From a6042a1fef741b17ebc446d537f5649455f27d6f Mon Sep 17 00:00:00 2001
From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Date: Tue, 22 Jul 2014 11:49:46 +0300
Subject: [PATCH 1/3] Bug 12586 - Record matching rules - Required match
 checks doesn't work with MARCXML

When trying to Match MARCXML-records from Zebra using Required match checks, C4::Matcher::_passes_required_checks()
tries to make a MARC::Record out of MARCXML while thinking it is USMARC.
This leads to matching records getting not-matched, and subsequently importing the same records again and again.

Very hard to debug or create a test case.
I accidentally noticed this while building an automatic acquisitions record overlay mechanism.
There are no big errors, simply no match is found, even though a match exists.

Also refactored the Matcher a bit to prevent excessive MARC::Record creation for each "Required match check"
for each search results. This probably would cripple any reasonably sized batch Matching operations.

---------
TEST PLAN
---------
1. Create a Matcher with "Required match checks" in addition to normal checks.
2. Find matches using the C4::Matcher->get_matches() and notice that you don't find matches.
3. Somebody who is actually using this Matcher could help me out here with defining a test plan.
---
 C4/Matcher.pm |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/C4/Matcher.pm b/C4/Matcher.pm
index 8caab0a..30f22fd 100644
--- a/C4/Matcher.pm
+++ b/C4/Matcher.pm
@@ -689,37 +689,48 @@ sub get_matches {
             }
         }
 
+        #Collect the results
         if ( defined $error ) {
             warn "search failed ($query) $error";
         }
         else {
-            foreach my $matched ( @{$searchresults} ) {
-                $matches{$matched} += $matchpoint->{'score'};
+            if ($self->{'record_type'} eq 'authority') {
+                foreach my $matched ( @{$searchresults} ) {
+                    $matches{$matched}->{score} += $matchpoint->{'score'};
+                }
+            }
+            elsif ($self->{'record_type'} eq 'biblio') {
+                foreach my $matched ( @{$searchresults} ) {
+                    my $record = C4::Search::new_record_from_zebra( 'biblioserver', $matched );
+                    $matches{$record}->{score} += $matchpoint->{'score'}; #Using $record HASH string representation as the key :)
+                    $matches{$record}->{record} = $record;
+                }
             }
+
         }
     }
 
     # get rid of any that don't meet the threshold
-    %matches = map { ($matches{$_} >= $self->{'threshold'}) ? ($_ => $matches{$_}) : () } keys %matches;
+    %matches = map { ($matches{$_}->{score} >= $self->{'threshold'}) ? ($_ => $matches{$_}) : () } keys %matches;
 
     # get rid of any that don't meet the required checks
-    %matches = map { _passes_required_checks($source_record, $_, $self->{'required_checks'}) ?  ($_ => $matches{$_}) : () } 
+    %matches = map { _passes_required_checks($source_record, $matches{$_}->{record}, $self->{'required_checks'}) ?  ($_ => $matches{$_}) : () }
                 keys %matches unless ($self->{'record_type'} eq 'auth');
 
     my @results = ();
     if ($self->{'record_type'} eq 'biblio') {
         require C4::Biblio;
-        foreach my $marcblob (keys %matches) {
-            my $target_record = C4::Search::new_record_from_zebra('biblioserver',$marcblob);
+        foreach my $hashkey (keys %matches) {
+            my $target_record = $matches{$hashkey}->{record};
             my $record_number;
             my $result = C4::Biblio::TransformMarcToKoha(C4::Context->dbh, $target_record, '');
             $record_number = $result->{'biblionumber'};
-            push @results, { 'record_id' => $record_number, 'score' => $matches{$marcblob} };
+            push @results, { 'record_id' => $record_number, 'score' => $matches{$hashkey}->{score}, 'target_record' => $target_record, 'target_biblio' => $result };
         }
     } elsif ($self->{'record_type'} eq 'authority') {
         require C4::AuthoritiesMarc;
         foreach my $authid (keys %matches) {
-            push @results, { 'record_id' => $authid, 'score' => $matches{$authid} };
+            push @results, { 'record_id' => $authid, 'score' => $matches{$authid}->{score} };
         }
     }
     @results = sort { $b->{'score'} cmp $a->{'score'} } @results;
@@ -763,8 +774,7 @@ sub dump {
 }
 
 sub _passes_required_checks {
-    my ($source_record, $target_blob, $matchchecks) = @_;
-    my $target_record = MARC::Record->new_from_usmarc($target_blob); # FIXME -- need to avoid parsing record twice
+    my ($source_record, $target_record, $matchchecks) = @_;
 
     # no checks supplied == automatic pass
     return 1 if $#{ $matchchecks } == -1;
-- 
1.7.9.5