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

Bug 21684: Fix UploadedFile[s]->delete 

Tests were failing with:
 #   Failed test 'Test delete via UploadedFile as well as UploadedFiles'
 #   at t/db_dependent/Upload.t line 193.
 DBIx::Class::Row::delete(): Not in database at /kohadevbox/koha/Koha/Object.pm line 219

I am not sure this patch is perfect, a set of uploaded files should be
deleted in a transaction, which would be rollback if something is wrong.
But it will be tricky to restore the files after they have been deleted.
It seems that we should deal with that with a more complicated process
and should be part of a separate bug.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Jonathan Druart 2020-01-07 12:10:05 +01:00 committed by Martin Renvoize
parent 7514ecb6c9
commit c66637670b
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
3 changed files with 16 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Koha/UploadedFile.pm
View File

@ -75,13 +75,6 @@ sub delete {
my $file = $self->full_path;
my $retval = $self->SUPER::delete;
if( !defined($retval) ) { # undef is Unknown (-1)
$retval = -1;
} elsif( $retval eq '0' ) { # 0 => 0E0
$retval = "0E0";
} elsif( $retval !~ /^(0E0|1)$/ ) { # Unknown too
$retval = -1;
}
return $retval if $params->{keep_file};
if( ! -e $file ) {

8
Koha/UploadedFiles.pm
View File

@ -72,8 +72,8 @@ sub delete {
# We use the individual delete on each resultset record
my $rv = 0;
while( my $row = $self->next ) {
my $delete= $row->delete( $params ); # 1, 0E0 or -1
$rv = ( $delete < 0 || $rv < 0 ) ? -1 : ( $rv + $delete );
my $deleted = eval { $row->delete( $params ) };
$rv++ if $deleted && !$@;
}
return $rv==0 ? "0E0" : $rv;
}
@ -135,8 +135,8 @@ sub delete_missing {
# We are passing keep_file since we already know that the file
# is missing and we do not want to see the warning
# Apply the same logic as in delete for the return value
my $delete = $row->delete({ keep_file => 1 }); # 1, 0E0 or -1
$rv = ( $delete < 0 || $rv < 0 ) ? -1 : ( $rv + $delete );
my $deleted = eval { $row->delete({ keep_file => 1 }) };
$rv++ if $deleted && !$@;
}
return $rv==0 ? "0E0" : $rv;
}

21
t/db_dependent/Upload.t
View File

@ -4,6 +4,7 @@ use Modern::Perl;
use File::Temp qw/ tempdir /;
use Test::More tests => 13;
use Test::Warn;
use Try::Tiny;
use Test::MockModule;
use t::lib::Mocks;
@ -171,7 +172,7 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub {
my $kohaobj = Koha::UploadedFiles->find( $upl->result );
$path = $kohaobj->full_path;
$delete = $kohaobj->delete;
ok( $delete=~/^-?1$/, 'Delete successful' );
ok( $delete, 'Delete successful' );
isnt( -e $path, 1, 'File no longer found after delete' );
# add another record with TestBuilder, so file does not exist
@ -180,16 +181,18 @@ subtest 'Test delete via UploadedFile as well as UploadedFiles' => sub {
warning_like { $delete = Koha::UploadedFiles->find( $upload01->{id} )->delete; }
qr/file was missing/,
'delete warns when file is missing';
ok( $delete=~/^-?1$/, 'Deleting record was successful' );
ok( $delete, 'Deleting record was successful' );
is( Koha::UploadedFiles->count, 4, 'Back to four uploads now' );
# add another one with TestBuilder and delete twice (file does not exist)
$upload01 = $builder->build({ source => 'UploadedFile' });
$kohaobj = Koha::UploadedFiles->find( $upload01->{id} );
$delete = $kohaobj->delete({ keep_file => 1 });
$delete = $kohaobj->delete({ keep_file => 1 });
ok( $delete =~ /^(0E0|-1)$/, 'Repeated delete unsuccessful' );
# NOTE: Koha::Object->delete does not return 0E0 (yet?)
try {
$delete = $kohaobj->delete({ keep_file => 1 });
} catch {
ok( $_->isa("DBIx::Class::Exception"), 'Repeated delete unsuccessful' );
}
};
subtest 'Test delete_missing' => sub {
@ -203,7 +206,7 @@ subtest 'Test delete_missing' => sub {
is( $deleted, 2, 'Expect two records with missing files' );
isnt( Koha::UploadedFiles->find( $upload01->{id} ), undef, 'Not deleted' );
$deleted = Koha::UploadedFiles->delete_missing;
ok( $deleted =~ /^(2|-1)$/, 'Deleted two records with missing files' );
ok( $deleted =~ /^(2)$/, 'Deleted two records with missing files' );
is( Koha::UploadedFiles->search({
id => [ $upload01->{id}, $upload02->{id} ],
})->count, 0, 'Records are gone' );
@ -300,17 +303,17 @@ subtest 'Testing delete_temporary' => sub {
# Now call delete_temporary with 6, 5 and 0
t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 6 );
my $delete = Koha::UploadedFiles->delete_temporary;
ok( $delete =~ /^(-1|0E0)$/, 'Check return value with 6' );
is( $delete, '0E0', 'Check return value with 6' );
is( Koha::UploadedFiles->search->count, 6, 'Delete with pref==6' );
# use override parameter
$delete = Koha::UploadedFiles->delete_temporary({ override_pref => 5 });
ok( $delete =~ /^(2|-1)$/, 'Check return value with 5' );
is( $delete, 2, 'Check return value with 5' );
is( Koha::UploadedFiles->search->count, 4, 'Delete with override==5' );
t::lib::Mocks::mock_preference('UploadPurgeTemporaryFilesDays', 0 );
$delete = Koha::UploadedFiles->delete_temporary;
ok( $delete =~ /^(-1|1)$/, 'Check return value with 0' );
is( $delete, 1, 'Check return value with 0' );
is( Koha::UploadedFiles->search->count, 3, 'Delete with pref==0 makes 3' );
is( Koha::UploadedFiles->search({ permanent => 1 })->count, 3,
'Still 3 permanent uploads' );