From a6042a1fef741b17ebc446d537f5649455f27d6f Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti 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