Bugzilla – Attachment 61809 Details for
Bug 17669
Add purging temporary uploads to cleanup_database
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17669: [QA Follow-up] More consistency in return values of delete
Bug-17669-QA-Follow-up-More-consistency-in-return-.patch (text/plain), 4.11 KB, created by
Marcel de Rooy
on 2017-04-03 19:31:44 UTC
(
hide
)
Description:
Bug 17669: [QA Follow-up] More consistency in return values of delete
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-04-03 19:31:44 UTC
Size:
4.11 KB
patch
obsolete
>From d6ec7508f748029ecddc5671ea2cdf3c467547f4 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Mon, 3 Apr 2017 19:14:52 +0200 >Subject: [PATCH] Bug 17669: [QA Follow-up] More consistency in return values > of delete >Content-Type: text/plain; charset=utf-8 > >See Bugzilla comment36 (QA request). > >Koha::UploadedFile->delete >Returns number of deleted records. POD corrected. Handles a possible or >fictive undef from the storage backend. 0E0 means zero (like DBI), -1 >means unknown. > >Koha::UploadedFile->delete >Returns number of deleted records (just as above), and no longer true or >false depending on errors encountered. POD corrected. > >Koha::UploadedFile->delete_temporary >Returns number of deleted records (just as above). Does no longer return >true when the associated pref contains NULL or empty string. POD corrected. > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >As often in Koha, the return values of delete operations are ignored. >The test t/db_dependent/Upload.t still passes. >--- > Koha/UploadedFile.pm | 8 +++++--- > Koha/UploadedFiles.pm | 16 ++++++++++------ > 2 files changed, 15 insertions(+), 9 deletions(-) > >diff --git a/Koha/UploadedFile.pm b/Koha/UploadedFile.pm >index 160c2d3..ffdd7d7 100644 >--- a/Koha/UploadedFile.pm >+++ b/Koha/UploadedFile.pm >@@ -61,7 +61,10 @@ Delete uploaded file. > It deletes not only the record, but also the actual file (unless you pass > the keep_file parameter). > >-Returns filename on successful delete or undef. >+Returns number of deleted records (1 or 0E0), or -1 for unknown. Note that this >+depends on the storage backend. And keep in mind that a deleted record does not >+automatically imply a deleted file; a warning may have been raised. >+(TODO: Use exceptions.) > > =cut > >@@ -71,14 +74,13 @@ sub delete { > my $name = $self->filename; > my $file = $self->full_path; > >- my $retval = $self->SUPER::delete; >+ my $retval = $self->SUPER::delete // -1; # translate (fictive) undef > return $retval if $params->{keep_file}; > > if( ! -e $file ) { > warn "Removing record for $name within category ". > $self->uploadcategorycode. ", but file was missing."; > } elsif( ! unlink($file) ) { >- $retval = 0; > warn "Problem while deleting: $file"; > } > return $retval; >diff --git a/Koha/UploadedFiles.pm b/Koha/UploadedFiles.pm >index b75aee8..83e2d20 100644 >--- a/Koha/UploadedFiles.pm >+++ b/Koha/UploadedFiles.pm >@@ -57,21 +57,25 @@ provides a wrapper around search to look for a term in multiple columns. > =head3 delete > > Delete uploaded files. >-Returns true if no errors occur. (So, false may mean partial success.) > > Parameter keep_file may be used to delete records, but keep files. > >+Returns the number of deleted records (depending on storage backend). An >+unknown value may be represented as -1; zero as 0E0. (Note: the number of >+deleted records is not automatically the same as the number of files.) >+ > =cut > > sub delete { > my ( $self, $params ) = @_; > $self = Koha::UploadedFiles->new if !ref($self); # handle class call > # We use the individual delete on each resultset record >- my $err = 0; >+ my $rv = 0; > while( my $row = $self->next ) { >- $err++ if !$row->delete( $params ); >+ my $delete= $row->delete( $params ) // -1; # handle fictive undef >+ $rv = ( $delete < 0 || $rv < 0 ) ? -1 : ( $rv + $delete ); > } >- return $err==0; >+ return $rv==0 ? "0E0" : $rv; > } > > =head3 delete_temporary >@@ -80,7 +84,7 @@ Delete_temporary is called by cleanup_database and only removes temporary > uploads older than [pref UploadPurgeTemporaryFilesDays] days. > It is possible to override the pref with the override_pref parameter. > >-Returns true if no errors occur. (Even when no files had to be deleted.) >+Return value: see delete. > > =cut > >@@ -90,7 +94,7 @@ sub delete_temporary { > if( exists $params->{override_pref} ) { > $days = $params->{override_pref}; > } elsif( !defined($days) || $days eq '' ) { # allow 0, not NULL or "" >- return 1; >+ return "0E0"; # in analogy with DBI (passed thru by DBIx) > } > my $dt = dt_from_string(); > $dt->subtract( days => $days ); >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17669
:
58176
|
58177
|
58178
|
58179
|
58810
|
58811
|
58812
|
61239
|
61240
|
61241
|
61242
|
61243
|
61465
|
61466
|
61467
|
61468
|
61469
|
61727
|
61729
|
61730
|
61742
|
61743
|
61805
|
61809
|
61818
|
61819
|
62486
|
62487
|
62488
|
62489
|
62490
|
62491
|
62492
|
62493
|
62494
|
62495