Bug 9988: Remove the Zebra code from sub merge
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tue, 21 Feb 2017 08:22:59 +0000 (09:22 +0100)
committerKyle M Hall <kyle@bywatersolutions.com>
Thu, 13 Apr 2017 12:53:46 +0000 (08:53 -0400)
Since we can now call linked_biblionumbers, we can now remove all Zebra
related code from merge. We also add a parameter biblionumbers; we use it
in the test now, but it may be handy too later in the maintenance script
when we want to trigger a merge for specific biblionumber(s). See bug
report 18071.

All mocks for ZOOM, Context::Zconn, Search::new_record_for_zebra in the
merge test can now be replaced by one mock for linked_biblionumbers. Note
that we test the biblionumbers parameter in the last subtest without that
mock.

Remove unused vars $countunmodifiedbiblio, $counterrors from merge.
Renamed zebrarecords to linkedrecords in the Merge test.

Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Modify an authority record. Check the linked biblio records.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
C4/AuthoritiesMarc.pm
t/db_dependent/Authorities/Merge.t

index 9baaa26..4aa8845 100644 (file)
@@ -1394,6 +1394,7 @@ sub AddAuthorityTrees{
         MARCfrom => $MARCfrom,
         [ mergeto => $mergeto, ]
         [ MARCto => $MARCto, ]
+        [ biblionumbers => [ $a, $b, $c ], ]
     });
 
 Merge biblios linked to authority $mergefrom.
@@ -1401,6 +1402,9 @@ If $mergeto equals mergefrom, the linked biblio field is updated.
 If $mergeto is different, the biblio field will be linked to $mergeto.
 If $mergeto is missing, the biblio field is deleted.
 
+Normally all biblio records linked to $mergefrom, will be considered. But
+you can pass specific numbers via the biblionumbers parameter.
+
 Note: Although $mergefrom and $mergeto will normally be of the same
 authority type, merge also supports moving to another authority type.
 
@@ -1408,14 +1412,18 @@ authority type, merge also supports moving to another authority type.
 
 sub merge {
     my ( $params ) = @_;
-    my $mergefrom = $params->{mergefrom};
+    my $mergefrom = $params->{mergefrom} || return;
     my $MARCfrom = $params->{MARCfrom};
     my $mergeto = $params->{mergeto};
     my $MARCto = $params->{MARCto};
+    my @biblionumbers = $params->{biblionumbers}
+        # If we do not have biblionumbers, we get all linked biblios
+        ? @{ $params->{biblionumbers} }
+        : Koha::Authorities->linked_biblionumbers({ authid => $mergefrom });
+    return 0 if !@biblionumbers;
 
-    return 0 unless $mergefrom > 0; # prevent abuse
-    my ($counteditedbiblio,$countunmodifiedbiblio,$counterrors)=(0,0,0);        
-    my $dbh=C4::Context->dbh;
+    my $counteditedbiblio = 0;
+    my $dbh = C4::Context->dbh;
     my $authfrom = Koha::Authorities->find($mergefrom);
     my $authto = Koha::Authorities->find($mergeto);
     my $authtypefrom = $authfrom ? Koha::Authority::Types->find($authfrom->authtypecode) : undef;
@@ -1429,36 +1437,7 @@ sub merge {
     @record_to = $MARCto->field($auth_tag_to_report_to)->subfields() if $auth_tag_to_report_to && $MARCto && $MARCto->field($auth_tag_to_report_to);
     my @record_from;
     @record_from = $MARCfrom->field($auth_tag_to_report_from)->subfields() if $auth_tag_to_report_from && $MARCfrom && $MARCfrom->field($auth_tag_to_report_from);
-    
-    my @reccache;
-    # search all biblio tags using this authority.
-    #Getting marcbiblios impacted by the change.
-    #zebra connection
-    my $oConnection=C4::Context->Zconn("biblioserver",0);
-    # We used to use XML syntax here, but that no longer works.
-    # Thankfully, we don't need it.
-    my $query;
-    $query= "an=".$mergefrom;
-    my $oResult = $oConnection->search(new ZOOM::Query::CCL2RPN( $query, $oConnection ));
-    my $count = 0;
-    if  ($oResult) {
-        $count=$oResult->size();
-    }
-    my $z=0;
-    while ( $z<$count ) {
-        my $marcrecordzebra = C4::Search::new_record_from_zebra(
-            'biblioserver',
-            $oResult->record($z)->raw()
-        );
-        my ( $biblionumbertagfield, $biblionumbertagsubfield ) = &GetMarcFromKohaField( "biblio.biblionumber", '' );
-        my $i = ($biblionumbertagfield < 10)
-            ? $marcrecordzebra->field( $biblionumbertagfield )->data
-            : $marcrecordzebra->subfield( $biblionumbertagfield, $biblionumbertagsubfield );
-        my $marcrecorddb = GetMarcBiblio($i);
-        push @reccache, $marcrecorddb;
-        $z++;
-    }
-    $oResult->destroy();
+
     # Get All candidate Tags for the change 
     # (This will reduce the search scope in marc records).
     # For a deleted authority record, we scan all auth controlled fields
@@ -1479,7 +1458,9 @@ sub merge {
     # And we need to add $9 in order not to duplicate
     $skip_subfields->{9} = 1 if !$overwrite;
 
-    foreach my $marcrecord(@reccache){
+    foreach my $biblionumber ( @biblionumbers ) {
+        my $marcrecord = GetMarcBiblio( $biblionumber );
+        next if !$marcrecord;
         my $update = 0;
         foreach my $tagfield (@$tags_using_authtype) {
             my $countfrom = 0;    # used in strict mode to remove duplicates
@@ -1523,25 +1504,11 @@ sub merge {
                 $update = 1;
             }
         }
-        my ($bibliotag,$bibliosubf) = GetMarcFromKohaField("biblio.biblionumber","") ;
-        my $biblionumber;
-        if ($bibliotag<10){
-            $biblionumber=$marcrecord->field($bibliotag)->data;
-        }
-        else {
-            $biblionumber=$marcrecord->subfield($bibliotag,$bibliosubf);
-        }
-        unless ($biblionumber){
-            warn "pas de numéro de notice bibliographique dans : ".$marcrecord->as_formatted;
-            next;
-        }
-        if ($update==1){
-            &ModBiblio($marcrecord,$biblionumber,GetFrameworkCode($biblionumber)) ;
-            $counteditedbiblio++;
-            warn $counteditedbiblio if (($counteditedbiblio % 10) and $ENV{DEBUG});
-        }    
+        next if !$update;
+        ModBiblio($marcrecord, $biblionumber, GetFrameworkCode($biblionumber));
+        $counteditedbiblio++;
     }
-    return $counteditedbiblio;  
+    return $counteditedbiblio;
 }
 
 sub _merge_newtag {
index be3e420..5e8bcf9 100755 (executable)
@@ -9,12 +9,12 @@ use Test::More tests => 5;
 use Getopt::Long;
 use MARC::Record;
 use Test::MockModule;
-use Test::MockObject;
 
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
 use C4::Biblio;
+use Koha::Authorities;
 use Koha::Database;
 
 BEGIN {
@@ -30,7 +30,7 @@ my $schema  = Koha::Database->new->schema;
 $schema->storage->txn_begin;
 
 # Global variables, mocking and framework modifications
-our ( @zebrarecords, $index );
+our @linkedrecords;
 my $mocks = set_mocks();
 our ( $authtype1, $authtype2 ) = modify_framework();
 
@@ -58,8 +58,7 @@ subtest 'Test merge A1 to A2 (within same authtype)' => sub {
     my ( $biblionumber2 ) = AddBiblio($biblio2, '');
 
     # Time to merge
-    @zebrarecords = ( $biblio1, $biblio2 );
-    $index = 0;
+    @linkedrecords = ( $biblionumber1, $biblionumber2 );
     my $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid2, MARCfrom => $auth2, mergeto => $authid1, MARCto => $auth1 });
     is( $rv, 1, 'We expect one biblio record (out of two) to be updated' );
 
@@ -102,8 +101,7 @@ subtest 'Test merge A1 to modified A1, test strict mode' => sub {
     my ( $biblionumber2 ) = AddBiblio( $MARC2, '');
 
     # Time to merge in loose mode first
-    @zebrarecords = ( $MARC1, $MARC2 );
-    $index = 0;
+    @linkedrecords = ( $biblionumber1, $biblionumber2 );
     t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
     my $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1old, mergeto => $authid1, MARCto => $auth1new });
     is( $rv, 2, 'Both records are updated now' );
@@ -123,8 +121,7 @@ subtest 'Test merge A1 to modified A1, test strict mode' => sub {
     # Merge again in strict mode
     t::lib::Mocks::mock_preference('AuthorityMergeMode', 'strict');
     ModBiblio( $MARC1, $biblionumber1, '' );
-    @zebrarecords = ( $MARC1 );
-    $index = 0;
+    @linkedrecords = ( $biblionumber1 );
     $rv = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1old, mergeto => $authid1, MARCto => $auth1new });
     $biblio1 = GetMarcBiblio($biblionumber1);
     is( $biblio1->field(109)->subfield('b'), undef, 'Subfield overwritten in strict mode' );
@@ -170,8 +167,7 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub {
     my $oldbiblio = C4::Biblio::GetMarcBiblio( $biblionumber );
 
     # Time to merge
-    @zebrarecords = ( $marc );
-    $index = 0;
+    @linkedrecords = ( $biblionumber );
     my $retval = C4::AuthoritiesMarc::merge({ mergefrom => $authid1, MARCfrom => $auth1, mergeto => $authid2, MARCto => $auth2 });
     is( $retval, 1, 'We touched only one biblio' );
 
@@ -232,8 +228,7 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub {
         MARC::Field->new( '609', '', '', a => 'DEL', 9 => "$authid1" ),
     );
     my ( $biblionumber ) = C4::Biblio::AddBiblio( $bib1, '' );
-    @zebrarecords = ( $bib1 );
-    $index = 0;
+    @linkedrecords = ( $biblionumber );
     DelAuthority({ authid => $authid1 }); # this triggers a merge call
 
     # See what happened in the biblio record
@@ -253,41 +248,25 @@ subtest 'Merging authorities should handle deletes (BZ 18070)' => sub {
     # record and call merge afterwards.
     # This mimics deleting an authority and calling merge later in the
     # merge_authority.pl cron job (when dontmerge is enabled).
+    # We use the biblionumbers parameter here and unmock linked_biblionumbers.
     C4::Context->dbh->do( "DELETE FROM auth_header WHERE authid=?", undef, $authid1 );
-    @zebrarecords = ( $marc1 );
-    $index = 0;
-    merge({ mergefrom => $authid1 });
+    @linkedrecords = ();
+    $mocks->{auth_mod}->unmock_all;
+    merge({ mergefrom => $authid1, biblionumbers => [ $biblionumber ] });
     # Final check
     $marc1 = C4::Biblio::GetMarcBiblio( $biblionumber );
     is( $marc1->field('609'), undef, 'Merge removed the 609 again even after deleting the authority record' );
 };
 
 sub set_mocks {
-    # Mock ZOOM objects: They do nothing actually
-    # Get new_record_from_zebra to return the records
+    # After we removed the Zebra code from merge, we only need to mock
+    # linked_biblionumbers here.
 
     my $mocks;
-    $mocks->{context_mod} = Test::MockModule->new( 'C4::Context' );
-    $mocks->{search_mod} = Test::MockModule->new( 'C4::Search' );
-    $mocks->{zoom_mod} = Test::MockModule->new( 'ZOOM::Query::CCL2RPN', no_auto => 1 );
-    $mocks->{conn_obj} = Test::MockObject->new;
-    $mocks->{zoom_obj} = Test::MockObject->new;
-    $mocks->{zoom_record_obj} = Test::MockObject->new;
-
-    $mocks->{context_mod}->mock( 'Zconn', sub { $mocks->{conn_obj}; } );
-    $mocks->{search_mod}->mock( 'new_record_from_zebra', sub {
-         return if $index >= @zebrarecords;
-         return $zebrarecords[ $index++ ];
+    $mocks->{auth_mod} = Test::MockModule->new( 'Koha::Authorities' );
+    $mocks->{auth_mod}->mock( 'linked_biblionumbers', sub {
+         return @linkedrecords;
     });
-    $mocks->{zoom_mod}->mock( 'new', sub {} );
-
-    $mocks->{conn_obj}->mock( 'search', sub { $mocks->{zoom_obj}; } );
-    $mocks->{zoom_obj}->mock( 'destroy', sub {} );
-    $mocks->{zoom_obj}->mock( 'record', sub { $mocks->{zoom_record_obj}; } );
-    $mocks->{zoom_obj}->mock( 'search', sub {} );
-    $mocks->{zoom_obj}->mock( 'size', sub { @zebrarecords } );
-    $mocks->{zoom_record_obj}->mock( 'raw', sub {} );
-
     return $mocks;
 }