Attachment 60604 Details for Bug 9988 – Bug 9988: Refactor the cron script

[patch] Bug 9988: Refactor the cron script

Bug-9988-Refactor-the-cron-script.patch (text/plain), 12.04 KB, created by Marcel de Rooy on 2017-02-23 12:16:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2017-02-23 12:16:11 UTC

Size: 12.04 KB

patch

obsolete

>From d1ac545ddebd72978ebba5825e68a1e6b1f070dd Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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         |  91 +++++++++++++++++++++++++
> misc/migration_tools/merge_authority.pl    | 104 -----------------------------
> t/db_dependent/Authorities/Merge.t         |   2 +-
> t/db_dependent/Authorities/MergeRequests.t |  44 ++++++++++++
> 5 files changed, 172 insertions(+), 105 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..ce6456c
>--- /dev/null
>+++ b/misc/cronjobs/merge_authorities.pl
>@@ -0,0 +1,91 @@
>+#!/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' }}, # the order is VERY IMPORTANT
>+    );
>+    # Note on the order: The order is especially important when two auth
>+    # records are merged. This results in three merges: N to N, O to N,
>+    # and O to NULL (N=New, O=Old). The merge O to N should always
>+    # precede the merge O to NULL; the id should guarantee that.
>+
>+    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 e344be8..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 <<EOF
>-Script to merge an authority into another
>-parameters :
>-\th : this version/help screen
>-\tv : verbose mode (show many things on screen)
>-\tf : the authority number to merge (the one that can be deleted after the merge).
>-\tt : the authority number where to merge
>-\tn : don't ask for confirmation (useful for batch mergings, should not be used on command line)
>-\tb : batch Merging
>-
>-All biblios with the authority in -t will be modified to be "connected" to authority -f
>-SAMPLE :
>-./merge_authority.pl -f 2457 -t 531
>-
>-Before doing anything, the script will show both authorities and ask for confirmation. Of course, you can merge only 2 authorities of the same kind.
>-EOF
>-;#
>-die;
>-}#/'
>-
>-my $dbh = C4::Context->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 = <STDIN>;
>-    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($mergefrom) if ($mergefrom != $mergeto);
>-}
>-my $timeneeded = gettimeofday - $starttime;
>-print "Done in $timeneeded seconds" unless $noconfirm;
>diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t
>index e849de4..b813c9f 100755
>--- a/t/db_dependent/Authorities/Merge.t
>+++ b/t/db_dependent/Authorities/Merge.t
>@@ -275,7 +275,7 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub {
>     # Instead of going through DelAuthority, we manually delete the auth
>     # record and call merge afterwards.
>     # This mimics deleting an authority and calling merge later in the
>-    # merge_authority.pl cron job.
>+    # merge cron job.
>     # We use the biblionumbers parameter here and unmock linked_biblionumbers.
>     C4::Context->dbh->do( "DELETE FROM auth_header WHERE authid=?", undef, $authid1 );
>     @linkedrecords = ();
>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

Actions: View | Diff | Splinter Review

Attachments on bug 9988: 59158 | 59159 | 60256 | 60257 | 60258 | 60259 | 60594 | 60595 | 60596 | 60597 | 60598 | 60599 | 60600 | 60601 | 60602 | 60603 | 60604 | 60605 | 60606 | 60607 | 60668 | 60669 | 60670 | 60671 | 60672 | 60673 | 60674 | 60675 | 61306 | 61307 | 61308 | 61309 | 61779 | 61780 | 61781 | 61782 | 61783 | 61784 | 61785 | 61786 | 61787 | 61788 | 61789 | 61790 | 61791 | 61792 | 61890 | 61891 | 61892 | 61893 | 61894 | 61895 | 61897 | 61898 | 61899 | 61900 | 61901 | 61902 | 61903 | 61904 | 61912 | 61913 | 61914 | 61915 | 61916 | 61917 | 61918 | 61919 | 61920 | 61921 | 61922 | 61923 | 61924 | 61925 | 62055 | 62123 | 62124 | 62125 | 62126 | 62127 | 62128 | 62129 | 62130 | 62131 | 62132 | 62133 | 62134 | 62135 | 62136 | 62137