From d7fa0fb92caab54f602cf4cd37e9971bd0347955 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 22 Feb 2017 16:51:58 +0100 Subject: [PATCH] Bug 9988: Refactor the cron script Content-Type: text/plain; charset=utf-8 The cron job is moved from migration tools to cronjobs. And renamed to a plural form. The script is now based on Koha objects. It does no longer include the code to merge one record. This can be done via the interface, and will be added to a maintenance script on bug 18071. Should not be part of this cron job. Adding a cron_cleanup method to MergeRequests; this method is called from the cron script to reset older entries still marked in progress and to also remove old processed entries. Tested in a separate unit test. Test plan: [1] Run t/db_dependent/Authorities/MergeRequests.t [2] Set AuthorityMergeLimit to 0. (All merges are postponed.) [3] Modify an authority linked to a few records. [4] Delete an authority linked to a few records with batch delete tool. [5] And select two auth records with linked records. Merge these two records with authority/merge.pl. Note: Do not select Default. See also bug 17380. [6] Check the need_merge_authorities table for inserted records. [7] Run misc/cronjobs/merge_authorities.pl -b and inspect the linked records and the record status in need_merge_authorities. --- Koha/Authority/MergeRequests.pm | 36 ++++++++++ misc/cronjobs/merge_authorities.pl | 89 ++++++++++++++++++++++++ misc/migration_tools/merge_authority.pl | 104 ----------------------------- t/db_dependent/Authorities/MergeRequests.t | 44 ++++++++++++ 4 files changed, 169 insertions(+), 104 deletions(-) create mode 100755 misc/cronjobs/merge_authorities.pl delete mode 100755 misc/migration_tools/merge_authority.pl create mode 100644 t/db_dependent/Authorities/MergeRequests.t diff --git a/Koha/Authority/MergeRequests.pm b/Koha/Authority/MergeRequests.pm index 469982b..dd274cf 100644 --- a/Koha/Authority/MergeRequests.pm +++ b/Koha/Authority/MergeRequests.pm @@ -23,6 +23,8 @@ use MARC::Record; use C4::Context; use Koha::Authority::MergeRequest; +use Koha::Database; +use Koha::DateUtils; use parent qw(Koha::Objects); @@ -69,6 +71,40 @@ sub reporting_tag_xml { ); } +=head3 cron_cleanup + + Koha::Authority::MergeRequests->cron_cleanup({ + reset_hours => 24, remove_days => 90, + }); + + Removes all entries with status "done" older than remove_days. + Set all entries with status "in progress" back to 0 when the timestamp + is older than reset_hours. + Defaults: reset_hours = 1, remove_days = 30. + +=cut + +sub cron_cleanup { + my ( $class_or_self, $params ) = @_; + my $reset_hours = $params->{reset_hours} || 1; + my $remove_days = $params->{remove_days} || 30; + my $parser = Koha::Database->new->schema->storage->datetime_parser; + + my $dt = dt_from_string; + $dt->subtract( hours => $reset_hours ); + $class_or_self->search({ + done => 2, + timestamp => { '<' => $parser->format_datetime($dt) }, + })->update({ done => 0 }); + + $dt = dt_from_string; + $dt->subtract( days => $remove_days ); + $class_or_self->search({ + done => 1, + timestamp => { '<' => $parser->format_datetime($dt) }, + })->delete; +} + =head3 _type Returns name of corresponding DBIC resultset diff --git a/misc/cronjobs/merge_authorities.pl b/misc/cronjobs/merge_authorities.pl new file mode 100755 index 0000000..4682694 --- /dev/null +++ b/misc/cronjobs/merge_authorities.pl @@ -0,0 +1,89 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Getopt::Long; +use Pod::Usage; +use Time::HiRes qw(gettimeofday); + +use C4::AuthoritiesMarc; +use Koha::Authority::MergeRequests; + +use constant RESET_HOURS => 24; +use constant REMOVE_DAYS => 30; + +my ( $params ); +GetOptions( + 'h' => \$params->{help}, + 'v' => \$params->{verbose}, + 'b' => \$params->{batch}, +); + +$|=1; # flushes output +if( $params->{batch} ) { + handle_batch( $params ); +} else { + pod2usage(1); +} + +sub handle_batch { + my $params = shift; + my $verbose = $params->{verbose}; + + my $starttime = gettimeofday; + print "Started merging\n" if $verbose; + + Koha::Authority::MergeRequests->cron_cleanup({ reset_hours => RESET_HOURS, remove_days => REMOVE_DAYS }); + my $rs = Koha::Authority::MergeRequests->search( + { done => 0 }, + { order_by => { -asc => 'id' }}, # IMPORTANT + ); + # For best results, postponed merges should be applied in right order. + # Similarly, we do not only select the last one for a specific id. + + while( my $req = $rs->next ) { + $req->done(2)->store; + print "Merging auth " . $req->authid . " to " . ( $req->authid_new // 'NULL' ) . ".\n" if $verbose; + my $newmarc = $req->authid_new + ? GetAuthority( $req->authid_new ) + : undef; + # Following merge call handles both modifications and deletes + merge({ + mergefrom => $req->authid, + MARCfrom => scalar $req->oldmarc, + mergeto => $req->authid_new, + MARCto => $newmarc, + override_limit => 1, + }); + $req->done(1)->store; + } + my $timeneeded = gettimeofday - $starttime; + print "Done in $timeneeded seconds\n" if $verbose; +} + +=head1 NAME + +merge_authorities.pl + +=head1 DESCRIPTION + +Cron script to handle authority merge requests + +=head1 SYNOPSIS + +merge_authorities.pl -h + +merge_authorities.pl -b -v + +=head1 OPTIONS + +-b : batch mode (You need to pass this parameter from crontab file) + +-h : print usage statement + +-v : verbose mode + +=head1 AUTHOR + +Koha Development Team + +=cut diff --git a/misc/migration_tools/merge_authority.pl b/misc/migration_tools/merge_authority.pl deleted file mode 100755 index 9b755b5..0000000 --- a/misc/migration_tools/merge_authority.pl +++ /dev/null @@ -1,104 +0,0 @@ -#!/usr/bin/perl -# script that rebuild thesaurus from biblio table. - -use strict; -#use warnings; FIXME - Bug 2505 -BEGIN { - # find Koha's Perl modules - # test carefully before changing this - use FindBin; - eval { require "$FindBin::Bin/kohalib.pl" }; -} - -# Koha modules used -use C4::Context; -use C4::Search; -use C4::Biblio; -use C4::AuthoritiesMarc; -use Koha::Authorities; -use Koha::Authority::MergeRequests; -use Time::HiRes qw(gettimeofday); - -use Getopt::Long; -my ($version, $verbose, $mergefrom,$mergeto,$noconfirm,$batch); -GetOptions( - 'h' => \$version, - 'f:s' => \$mergefrom, - 't:s' => \$mergeto, - 'v' => \$verbose, - 'n' => \$noconfirm, - 'b' => \$batch, -); - -if ($version || ($mergefrom eq '' && !$batch)) { - print <dbh; - -$|=1; # flushes output -my $authfrom = GetAuthority($mergefrom); -my $authto = GetAuthority($mergeto); - -die "Authority $mergefrom does not exist" unless $authfrom; -die "Authority $mergeto does not exist" unless $authto; - -my $authtypecodefrom = Koha::Authorities->find($mergefrom)->authtypecode; -my $authtypecodeto = Koha::Authorities->find($mergeto)->authtypecode; - -unless ($noconfirm || $batch) { - print "************\n"; - print "You will merge authority : $mergefrom ($authtypecodefrom)\n".$authfrom->as_formatted; - print "\n*************\n"; - print "Into authority : $mergeto ($authtypecodeto)\n".$authto->as_formatted; - print "\n\nDo you confirm (enter YES)?"; - my $confirm = ; - chop $confirm; - unless (uc($confirm) eq 'YES' and $authtypecodefrom eq $authtypecodeto) { - print "IMPOSSIBLE : authorities are not of the same type ($authtypecodefrom vs $authtypecodeto) !!!\n" if $authtypecodefrom ne $authtypecodeto; - print "Merge cancelled\n"; - exit; - } -} -my $starttime = gettimeofday; -print "Merging\n" unless $noconfirm; -if ($batch) { - my $authref; - $dbh->do("update need_merge_authorities set done=2 where done=0"); #temporary status 2 means: selected for merge - $authref=$dbh->selectall_arrayref("select id,authid,authid_new from need_merge_authorities where done=2"); - foreach my $row ( @$authref ) { - my $req = Koha::Authority::MergeRequests->find( $row->[0] ); - my $marc = $req ? $req->oldmarc : undef; - my $authid = $row->[1]; - my $authid_new = $row->[2]; - print "managing $authid\n" if $verbose; - # Following merge call handles both modifications and deletes - merge({ mergefrom => $authid, MARCfrom => $marc, mergeto => $authid_new, MARCto => GetAuthority($authid_new), override_limit => 1 }); - } - $dbh->do("update need_merge_authorities set done=1 where done=2"); #DONE -} else { - my $MARCfrom = GetAuthority($mergefrom); - my $MARCto = GetAuthority($mergeto); - &merge({ mergefrom => $mergefrom, MARCfrom => $MARCfrom, mergeto => $mergeto, MARCto => $MARCto }); - #Could add mergefrom authority to mergeto rejected forms before deletion - DelAuthority({ authid => $mergefrom }) if ($mergefrom != $mergeto); -} -my $timeneeded = gettimeofday - $starttime; -print "Done in $timeneeded seconds" unless $noconfirm; diff --git a/t/db_dependent/Authorities/MergeRequests.t b/t/db_dependent/Authorities/MergeRequests.t new file mode 100644 index 0000000..bba7919 --- /dev/null +++ b/t/db_dependent/Authorities/MergeRequests.t @@ -0,0 +1,44 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::More tests => 1; + +use Koha::Authority::MergeRequest; +use Koha::Authority::MergeRequests; +use Koha::Database; +use Koha::DateUtils qw/dt_from_string/; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +subtest "Tests for cron_cleanup" => sub { + plan tests => 3; + + my $dt = dt_from_string; + $dt->subtract( hours => 2 ); + my $req1 = Koha::Authority::MergeRequest->new({ + authid => 1, + done => 2, + timestamp => $dt, + })->store; + + $dt->subtract( days => 30 ); + my $req2 = Koha::Authority::MergeRequest->new({ + authid => 2, + done => 1, + timestamp => $dt, + })->store; + + # Now test two cleanup calls + # First call should only remove req2; second call should reset req1 + Koha::Authority::MergeRequests->cron_cleanup({ reset_hours => 3 }); + $req1->discard_changes; # requery + is( $req1->done, 2, 'My request was not touched' ); + $req2->discard_changes; # requery + is( $req2->in_storage, 0, 'Second request removed' ); + Koha::Authority::MergeRequests->cron_cleanup({ reset_hours => 1 }); + $req1->discard_changes; # requery + is( $req1->done, 0, 'Yes, we got a reset' ); +}; + +$schema->storage->txn_rollback; -- 2.1.4