From e3ddffe8d0281595e9ffcfc7fea716e2a51b0cd6 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Sat, 30 Nov 2019 14:16:54 +0100 Subject: [PATCH] Bug 24153: Add a confirm flag to cleanup_database How handy this will be, right? This patch will change the behavior of this script as the --confirm flag will now be required for effective changes. Without --confirm and with --verbose you will see what could have done the script in non dry-run mode. Test plan: Use different options of the script without the --confirm flag and have a look at the output. It now tells you what would have been deleted if the flag was passed. Now use the --confirm flag and confirm that the changes are now effective. Sponsored-by: Association KohaLa - https://koha-fr.org/ --- misc/cronjobs/cleanup_database.pl | 199 +++++++++++++++++++++++++------------- 1 file changed, 133 insertions(+), 66 deletions(-) diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl index 2259cabfa5..ef023d9ba3 100755 --- a/misc/cronjobs/cleanup_database.pl +++ b/misc/cronjobs/cleanup_database.pl @@ -50,10 +50,11 @@ use Koha::PseudonymizedTransactions; sub usage { print STDERR < \$help, + 'confirm' => \$confirm, 'sessions' => \$sessions, 'sessdays:i' => \$sess_days, 'v|verbose' => \$verbose, @@ -216,7 +219,9 @@ if ($pDebarments && $allDebarments) { usage(1); } -cronlogaction(); +say "Confirm flag not passed, running in dry-run mode..." unless $confirm; + +cronlogaction() unless $confirm; my $dbh = C4::Context->dbh(); my $sth; @@ -231,15 +236,17 @@ if ( $sessions && !$sess_days ) { my @count_arr = $sth->fetchrow_array; print "$count_arr[0] entries will be deleted.\n"; } - $sth = $dbh->prepare(q{ TRUNCATE sessions }); - $sth->execute() or die $dbh->errstr; + if ( $confirm ) { + $sth = $dbh->prepare(q{ TRUNCATE sessions }); + $sth->execute() or die $dbh->errstr; + } if ($verbose) { print "Done with session purge.\n"; } } elsif ( $sessions && $sess_days > 0 ) { print "Session purge triggered with days>$sess_days.\n" if $verbose; - RemoveOldSessions(); + RemoveOldSessions() if $confirm; print "Done with session purge with days>$sess_days.\n" if $verbose; } @@ -253,10 +260,14 @@ if ($zebraqueue_days) { WHERE done=1 AND time < date_sub(curdate(), INTERVAL ? DAY) } ); - $sth->execute($zebraqueue_days) or die $dbh->errstr; + if ( $confirm ) { + $sth->execute($zebraqueue_days) or die $dbh->errstr; + } $sth2 = $dbh->prepare(q{ DELETE FROM zebraqueue WHERE id=? }); while ( my $record = $sth->fetchrow_hashref ) { - $sth2->execute( $record->{id} ) or die $dbh->errstr; + if ( $confirm ) { + $sth2->execute( $record->{id} ) or die $dbh->errstr; + } $count++; } print "$count records were deleted.\nDone with zebraqueue purge.\n" if $verbose; @@ -270,28 +281,31 @@ if ($mail) { WHERE time_queued < date_sub(curdate(), INTERVAL ? DAY) } ); - $sth->execute($mail) or die $dbh->errstr; - $count = $sth->rows; - $sth->finish; + if ( $confirm ) { + $sth->execute($mail) or die $dbh->errstr; + $count = $sth->rows; + } print "$count messages were deleted from the mail queue.\nDone with message_queue purge.\n" if $verbose; } if ($purge_merged) { print "Purging completed entries from need_merge_authorities.\n" if $verbose; - $sth = $dbh->prepare(q{ DELETE FROM need_merge_authorities WHERE done=1 }); - $sth->execute() or die $dbh->errstr; + if ( $confirm ) { + $sth = $dbh->prepare(q{ DELETE FROM need_merge_authorities WHERE done=1 }); + $sth->execute() or die $dbh->errstr; + } print "Done with purging need_merge_authorities.\n" if $verbose; } if ($pImport) { print "Purging records from import tables.\n" if $verbose; - PurgeImportTables(); + PurgeImportTables() if $confirm; print "Done with purging import tables.\n" if $verbose; } if ($pZ3950) { print "Purging Z39.50 records from import tables.\n" if $verbose; - PurgeZ3950(); + PurgeZ3950() if $confirm; print "Done with purging Z39.50 records from import tables.\n" if $verbose; } @@ -303,19 +317,21 @@ if ($pLogs) { WHERE timestamp < date_sub(curdate(), INTERVAL ? DAY) } ); - $sth->execute($pLogs) or die $dbh->errstr; + if ( $confirm ) { + $sth->execute($pLogs) or die $dbh->errstr; + } print "Done with purging action_logs.\n" if $verbose; } if ($fees_days) { print "Purging records from accountlines.\n" if $verbose; - purge_zero_balance_fees( $fees_days ); + purge_zero_balance_fees( $fees_days ) if $confirm; print "Done purging records from accountlines.\n" if $verbose; } if ($pSearchhistory) { print "Purging records older than $pSearchhistory from search_history.\n" if $verbose; - C4::Search::History::delete({ interval => $pSearchhistory }); + C4::Search::History::delete({ interval => $pSearchhistory }) if $confirm; print "Done with purging search_history.\n" if $verbose; } @@ -328,142 +344,193 @@ if ($pListShareInvites) { AND (sharedate + INTERVAL ? DAY) < NOW() } ); - $sth->execute($pListShareInvites); + if ( $confirm ) { + $sth->execute($pListShareInvites); + } print "Done with purging unaccepted list share invites.\n" if $verbose; } if ($pDebarments) { print "Expired patrons restrictions purge triggered for $pDebarments days.\n" if $verbose; - $count = PurgeDebarments($pDebarments); + $count = PurgeDebarments($pDebarments, $confirm); print "$count restrictions were deleted.\nDone with restrictions purge.\n" if $verbose; } if($allDebarments) { print "All expired patrons restrictions purge triggered.\n" if $verbose; - $count = PurgeDebarments(0); + $count = PurgeDebarments(0, $confirm); print "$count restrictions were deleted.\nDone with all restrictions purge.\n" if $verbose; } # Handle unsubscribe requests from GDPR consent form, depends on UnsubscribeReflectionDelay preference my $unsubscribed_patrons = Koha::Patrons->search_unsubscribed; $count = $unsubscribed_patrons->count; -$unsubscribed_patrons->lock( { expire => 1, remove => 1 } ); +$unsubscribed_patrons->lock( { expire => 1, remove => 1 } ) if $confirm; say sprintf "Locked %d patrons", $count if $verbose; # Anonymize patron data, depending on PatronAnonymizeDelay my $anonymize_candidates = Koha::Patrons->search_anonymize_candidates( { locked => 1 } ); $count = $anonymize_candidates->count; -$anonymize_candidates->anonymize; +$anonymize_candidates->anonymize if $confirm; say sprintf "Anonymized %s patrons", $count if $verbose; # Remove patron data, depending on PatronRemovalDelay (will raise an exception if problem encountered my $anonymized_patrons = Koha::Patrons->search_anonymized; $count = $anonymized_patrons->count; -$anonymized_patrons->delete( { move => 1 } ); -if ($@) { - warn $@; -} -elsif ($verbose) { +if ( $confirm ) { + $anonymized_patrons->delete( { move => 1 } ); + if ($@) { + warn $@; + } + elsif ($verbose) { + say sprintf "Deleted %d patrons", $count; + } +} else { say sprintf "Deleted %d patrons", $count; } +# FIXME The output for dry-run mode needs to be improved +# But non trivial changes to C4::Members need to be done before. if( $pExpSelfReg ) { - DeleteExpiredSelfRegs(); + if ( $confirm ) { + DeleteExpiredSelfRegs(); + } elsif ( $verbose ) { + say "self-registered borrowers may be deleted"; + } } if( $pUnvSelfReg ) { - DeleteUnverifiedSelfRegs( $pUnvSelfReg ); + if ( $confirm ) { + DeleteUnverifiedSelfRegs( $pUnvSelfReg ); + } elsif ( $verbose ) { + say "unverified self-registrations may be deleted"; + } } if ($special_holidays_days) { - DeleteSpecialHolidays( abs($special_holidays_days) ); + if ( $confirm ) { + DeleteSpecialHolidays( abs($special_holidays_days) ); + } elsif ( $verbose ) { + say "self-registered borrowers may be deleted"; + } } if( $temp_uploads ) { # Delete temporary uploads, governed by a pref (unless you override) print "Purging temporary uploads.\n" if $verbose; - Koha::UploadedFiles->delete_temporary({ - defined($temp_uploads_days) - ? ( override_pref => $temp_uploads_days ) - : () - }); + if ( $confirm ) { + Koha::UploadedFiles->delete_temporary({ + defined($temp_uploads_days) + ? ( override_pref => $temp_uploads_days ) + : () + }); + } print "Done purging temporary uploads.\n" if $verbose; } if( defined $uploads_missing ) { print "Looking for missing uploads\n" if $verbose; - my $keep = $uploads_missing == 1 ? 0 : 1; - my $count = Koha::UploadedFiles->delete_missing({ keep_record => $keep }); - if( $keep ) { - print "Counted $count missing uploaded files\n"; + if ( $confirm ) { + my $keep = $uploads_missing == 1 ? 0 : 1; + my $count = Koha::UploadedFiles->delete_missing({ keep_record => $keep }); + if( $keep ) { + print "Counted $count missing uploaded files\n"; + } else { + print "Removed $count records for missing uploads\n"; + } } else { - print "Removed $count records for missing uploads\n"; + # FIXME need to create a filter_by_missing method (then call ->delete) instead of delete_missing + say "Dry-run mode cannot guess how many uploads would have been deleted"; } } if ($oauth_tokens) { require Koha::OAuthAccessTokens; - my $count = int Koha::OAuthAccessTokens->search({ expires => { '<=', time } })->delete; - say "Removed $count expired OAuth2 tokens" if $verbose; + my $tokens = Koha::OAuthAccessTokens->search({ expires => { '<=', time } }); + my $count = $tokens->count; + $tokens->delete if $confirm; + say sprintf "Removed %s expired OAuth2 tokens", $count if $verbose; } if ($pStatistics) { print "Purging statistics older than $pStatistics days.\n" if $verbose; - Koha::Statistics->filter_by_last_update( - { timestamp_column_name => 'datetime', days => $pStatistics } )->delete; - print "Done with purging statistics.\n" if $verbose; + my $statistics = Koha::Statistics->filter_by_last_update( + { timestamp_column_name => 'datetime', days => $pStatistics } ); + my $count = $statistics->count; + $statistics->delete if $confirm; + say sprintf "Done with purging %s statistics.", $count if $verbose; } if ($pDeletedCatalog) { - print "Purging deleted catalog older than $pDeletedCatalog days.\n" if $verbose; - Koha::Old::Items ->filter_by_last_update( { days => $pDeletedCatalog } )->delete; - Koha::Old::Biblioitems->filter_by_last_update( { days => $pDeletedCatalog } )->delete; - Koha::Old::Biblios ->filter_by_last_update( { days => $pDeletedCatalog } )->delete; - print "Done with purging deleted catalog.\n" if $verbose; + print "Purging deleted catalog older than $pDeletedCatalog days.\n" + if $verbose; + my $old_items = Koha::Old::Items->filter_by_last_update( { days => $pDeletedCatalog } ); + my $old_biblioitems = Koha::Old::Biblioitems->filter_by_last_update( { days => $pDeletedCatalog } ); + my $old_biblios = Koha::Old::Biblios->filter_by_last_update( { days => $pDeletedCatalog } ); + my ( $c_i, $c_bi, $c_b ) = + ( $old_items->count, $old_biblioitems->count, $old_biblios->count ); + if ($confirm) { + $old_items->delete; + $old_biblioitems->delete; + $old_biblios->delete; + } + say sprintf + "Done with purging deleted catalog (%d items, %d biblioitems, %d biblios).", + $c_i, $c_bi, $c_b + if $verbose; } if ($pDeletedPatrons) { print "Purging deleted patrons older than $pDeletedPatrons days.\n" if $verbose; - Koha::Old::Patrons->filter_by_last_update( - { timestamp_column_name => 'updated_on', days => $pDeletedPatrons } ) - ->delete; - print "Done with purging deleted patrons.\n" if $verbose; + my $old_patrons = Koha::Old::Patrons->filter_by_last_update( + { timestamp_column_name => 'updated_on', days => $pDeletedPatrons } ); + my $count = $old_patrons->count; + $old_patrons->delete if $confirm; + say sprintf "Done with purging %d deleted patrons.", $count if $verbose; } if ($pOldIssues) { print "Purging old issues older than $pOldIssues days.\n" if $verbose; - Koha::Old::Checkouts->filter_by_last_update( { days => $pOldIssues } )->delete; - print "Done with purging old issues.\n" if $verbose; + my $old_checkouts = Koha::Old::Checkouts->filter_by_last_update( { days => $pOldIssues } ); + my $count = $old_checkouts->count; + $old_checkouts->delete if $confirm; + say sprintf "Done with purging %d old issues.", $count if $verbose; } if ($pOldReserves) { print "Purging old reserves older than $pOldReserves days.\n" if $verbose; - Koha::Old::Holds->filter_by_last_update( { days => $pOldReserves } )->delete; - print "Done with purging old reserves.\n" if $verbose; + my $old_reserves = Koha::Old::Holds->filter_by_last_update( { days => $pOldReserves } ); + my $count = $old_reserves->count; + $old_reserves->delete if $verbose; + say sprintf "Done with purging %d old reserves.", $count if $verbose; } if ($pTransfers) { print "Purging arrived item transfers older than $pTransfers days.\n" if $verbose; - Koha::Item::Transfers->filter_by_last_update( + my $transfers = Koha::Item::Transfers->filter_by_last_update( { timestamp_column_name => 'datearrived', days => $pTransfers, } - )->delete; - print "Done with purging transfers.\n" if $verbose; + ); + my $count = $transfers->count; + $transfers->delete if $verbose; + say sprintf "Done with purging %d transfers.", $count if $verbose; } if (defined $pPseudoTransactions) { print "Purging pseudonymized transactions older than $pPseudoTransactions days.\n" if $verbose; - Koha::Anonymized::Transactions->filter_by_last_update( + my $anonymized_transactions = Koha::Anonymized::Transactions->filter_by_last_update( { timestamp_column_name => 'datetime', ( $pPseudoTransactions ? ( days => $pPseudoTransactions ) : () ), ( $pPseudoTransactionsFrom ? ( from => $pPseudoTransactionsFrom ) : () ), ( $pPseudoTransactionsTo ? ( to => $pPseudoTransactionsTo ) : () ), } - )->delete; - print "Done with purging pseudonymized transactions.\n" if $verbose; + ); + my $count = $anonymized_transactions->count; + $anonymized_transactions->delete if $confirm; + say sprintf "Done with purging %d pseudonymized transactions.", $count if $verbose; } exit(0); @@ -537,7 +604,7 @@ sub PurgeZ3950 { sub PurgeDebarments { require Koha::Patron::Debarments; - my $days = shift; + my ( $days, $doit ) = @_; $count = 0; $sth = $dbh->prepare( q{ @@ -548,7 +615,7 @@ sub PurgeDebarments { ); $sth->execute($days) or die $dbh->errstr; while ( my ($borrower_debarment_id) = $sth->fetchrow_array ) { - Koha::Patron::Debarments::DelDebarment($borrower_debarment_id); + Koha::Patron::Debarments::DelDebarment($borrower_debarment_id) if $doit; $count++; } return $count; -- 2.11.0