gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 17501: Move Koha::Upload::delete to Koha::UploadedFile[s] 

Since delete is not part of the upload process, we will move it now
to Koha::UploadedFile[s].
Deleting the file will be done in UploadedFile.
The (multiple) delete method in UploadedFiles refers to the single delete.

Test plan:
[1] Run t/db_dependent/Upload.t
    The warning ("but file was missing") in the last subtest is fine;
    the file did not exist. Will be addressed in a follow-up.
[2] Search for uploads on Tools/Upload. Clone this tab (repeat search on
    a new tab in your browser).
[3] Delete an existing upload on the first tab.
[4] Try to delete it again on the second tab. Error message?
[5] Bonus points:
    Add an upload. Mark the file immutable with chattr +i. Try to delete
    the file. You should see a "Could not be deleted"-message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2016-11-21 14:53:27 +01:00 committed by Kyle M Hall
parent 42f0bdf9f1
commit 7a62005468
6 changed files with 118 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
Koha/Upload.pm
View File

@ -46,9 +46,6 @@ Koha::Upload - Facilitate file uploads (temporary and permanent)
while( <$fh> ) { print $_; }
$fh->close;
# delete an upload
my ( $fn ) = Koha::Upload->new->delete({ id => $id });
=head1 DESCRIPTION
This module is a refactored version of C4::UploadedFile but adds on top
@ -56,6 +53,9 @@ Koha::Upload - Facilitate file uploads (temporary and permanent)
That report added module UploadedFiles.pm. This module contains the
functionality of both.
The module has been revised to use Koha::Object[s]; the delete method
has been moved to Koha::UploadedFile[s].
=head1 METHODS
=cut
@ -192,19 +192,6 @@ sub get {
return wantarray? @rv: $res;
}
=head2 delete
Returns array of deleted filenames or undef.
Since it now only accepts id as parameter, you should not expect more
than one filename.
=cut
sub delete {
my ( $self, $params ) = @_;
return $self->_delete( $params->{id} );
}
=head1 CLASS METHODS
=head2 getCategories
@ -262,8 +249,8 @@ sub allows_add_by {
sub _init {
my ( $self, $params ) = @_;
$self->{rootdir} = C4::Context->config('upload_path');
$self->{tmpdir} = File::Spec->tmpdir;
$self->{rootdir} = Koha::UploadedFile->permanent_directory;
$self->{tmpdir} = Koha::UploadedFile->temporary_directory;
$params->{tmp} = $params->{temp} if !exists $params->{tmp};
$self->{temporary} = $params->{tmp}? 1: 0; #default false
@ -402,30 +389,6 @@ sub _lookup {
# Does always return an arrayref (perhaps an empty one)
}
sub _delete {
my ( $self, $id ) = @_;
my $rec = Koha::UploadedFiles->find($id) || return;
my $filename = $rec->filename;
my $file = $self->_full_fname({
permanent => $rec->permanent,
dir => $rec->dir,
hashvalue => $rec->hashvalue,
filename => $filename,
});
if( !-e $file ) { # we will just delete the record
# TODO Should we add a trace here for the missing file?
$rec->delete;
return $filename;
} elsif( unlink($file) ) {
$rec->delete;
return $filename;
}
$self->{files}->{ $rec->{filename} }->{errcode} = 7;
#NOTE: errcode=6 is used to report successful delete (see template)
return;
}
sub _compute {
# Computes hash value when sub hook feeds the first block
# For temporary files, the id is made unique with time

58
Koha/UploadedFile.pm
View File

@ -18,6 +18,7 @@ package Koha::UploadedFile;
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
use Modern::Perl;
use File::Spec;
#use Koha::Database;
@ -41,17 +42,68 @@ Description
=head3 delete
Delete uploaded file (to be extended)
Delete uploaded file.
It deletes not only the record, but also the actual file.
Returns filename on successful delete or undef.
=cut
sub delete {
my ( $self, $params ) = @_;
$self->SUPER::delete( $params );
my ( $self ) = @_;
my $name = $self->filename;
my $file = $self->full_path;
if( !-e $file ) { # we will just delete the record
warn "Removing record for $name within category ".
$self->uploadcategorycode. ", but file was missing.";
return $name if $self->SUPER::delete;
} elsif( unlink($file) ) {
return $name if $self->SUPER::delete;
} else {
warn "Problem while deleting: $file";
}
return; # something went wrong
}
=head3 full_path
Returns the fully qualified path name for an uploaded file.
=cut
sub full_path {
my ( $self ) = @_;
my $path = File::Spec->catfile(
$self->permanent?
$self->permanent_directory: $self->temporary_directory,
$self->dir,
$self->hashvalue. '_'. $self->filename,
);
return $path;
}
=head2 CLASS METHODS
=head3 root_directory
=cut
sub permanent_directory {
my ( $class ) = @_;
return C4::Context->config('upload_path');
}
=head3 tmp_directory
=cut
sub temporary_directory {
my ( $class ) = @_;
return File::Spec->tmpdir;
}
=head3 _type
Returns name of corresponding DBIC resultset

22
Koha/UploadedFiles.pm
View File

@ -40,15 +40,29 @@ Description
=head2 INSTANCE METHODS
=head3 delete
=head3 delete, delete_errors
Delete uploaded files
Delete uploaded files.
Returns true if no errors occur.
Delete_errors returns the number of errors when deleting files.
=cut
sub delete {
my ( $self, $params ) = @_;
$self->SUPER::delete( $params );
my ( $self ) = @_;
# We use the individual delete on each resultset record
my $err = 0;
while( my $row = $self->_resultset->next ) {
my $kohaobj = Koha::UploadedFile->_new_from_dbic( $row );
$err++ if !$kohaobj->delete;
}
$self->{delete_errors} = $err;
return $err==0;
}
sub delete_errors {
my ( $self ) = @_;
return $self->{delete_errors};
}
=head2 CLASS METHODS

2
koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt
View File

@ -201,7 +201,7 @@
_("No temporary directory found."),
_("File could not be read."),
_("File has been deleted."),
_("File could not be deleted."),
_("File or upload record could not be deleted."),
];
//]]>
</script>

34
t/db_dependent/Upload.t
View File

@ -36,6 +36,9 @@ our $uploads = [
[
{ name => 'file4', cat => undef, size => 5000 }, # temp duplicate
],
[
{ name => 'file5', cat => undef, size => 7000 }, # temp duplicate
],
];
# Redirect upload dir structure and mock File::Spec and CGI
@ -48,7 +51,7 @@ $cgimod->mock( 'new' => \&newCGI );
# Start testing
subtest 'Test01' => sub {
plan tests => 7;
plan tests => 9;
test01();
};
subtest 'Test02' => sub {
@ -64,7 +67,7 @@ subtest 'Test04' => sub {
test04();
};
subtest 'Test05' => sub {
plan tests => 5;
plan tests => 6;
test05();
};
subtest 'Test06' => sub {
@ -85,6 +88,12 @@ sub test01 {
# Delete existing records (for later tests)
$dbh->do( "DELETE FROM uploaded_files" );
# Check mocked directories
is( Koha::UploadedFile->permanent_directory, $tempdir,
'Check permanent directory' );
is( Koha::UploadedFile->temporary_directory, $tempdir,
'Check temporary directory' );
my $upl = Koha::Upload->new({
category => $uploads->[$current_upload]->[0]->{cat},
});
@ -143,11 +152,24 @@ sub test05 { # add temporary file with same name and contents, delete it
is( $upl->count, 1, 'Upload 5 adds duplicate temporary file' );
my $id = $upl->result;
my $r = $upl->get({ id => $id });
my @d = $upl->delete({ id => $id });
is( $d[0], $r->{name}, 'Delete successful' );
is( -e $r->{path}? 1: 0, 0, 'File no longer found after delete' );
# testing delete via UploadedFiles (plural)
my $delete = Koha::UploadedFiles->search({ id => $id })->delete;
is( $delete, 1, 'Delete successful' );
isnt( -e $r->{path}, 1, 'File no longer found after delete' );
is( scalar $upl->get({ id => $id }), undef, 'Record also gone' );
is( $upl->delete({ id => $id }), undef, 'Repeated delete failed' );
# testing delete via UploadedFile (singular)
# Note that find returns a Koha::Object
$upl = Koha::Upload->new({ tmp => 1 });
$upl->cgi;
$id = $upl->result;
my $kohaobj = Koha::UploadedFiles->find( $id );
my $name = $kohaobj->filename;
my $path = $kohaobj->full_path;
$delete = $kohaobj->delete;
is( $delete, $name, 'Delete successful' );
isnt( -e $path, 1, 'File no longer found after delete' );
}
sub test06 { #some extra tests for get

19
tools/upload.pl
View File

@ -68,17 +68,20 @@ if ( $op eq 'new' ) {
} elsif ( $op eq 'delete' ) {
# delete only takes the id parameter
my $upl = Koha::Upload->new($upar);
my ($fn) = $upl->delete( { id => $id } );
my $e = $upl->err;
my $msg =
$fn ? JSON::to_json( { $fn => 6 } )
: $e ? JSON::to_json($e)
: undef;
my $upload = $plugin?
Koha::UploadedFiles->search({ public => 1, id => $id })->next:
Koha::UploadedFiles->find($id);
my $fn = $upload? $upload->delete: undef;
#TODO Improve error handling
my $msg = $fn?
JSON::to_json({ $fn => 6 }):
JSON::to_json({
$upload? $upload->filename: ( $id? "id $id": '[No id]' ), 7,
});
$template->param(
mode => 'deleted',
msg => $msg,
uploadcategories => $upl->getCategories,
uploadcategories => Koha::Upload->getCategories,
);
output_html_with_http_headers $input, $cookie, $template->output;