From d66fa38bdd1cb39efcd339bbeed4158c64d8e9ba Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Sun, 20 Oct 2013 13:25:21 -0700 Subject: [PATCH] Bug 11096: force XML syntax for Zconn This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba --- C4/Context.pm | 8 +++--- C4/Search.pm | 66 +++++++++++++++++++++++++++---------------------- catalogue/search.pl | 2 +- circ/circulation.pl | 2 +- t/db_dependent/Search.t | 40 +++++++++++++++--------------- 5 files changed, 62 insertions(+), 56 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 39927d4..bdc23bd 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -705,8 +705,6 @@ sub Zconn { $context->{"Zconn"}->{$server}->destroy() if defined($context->{"Zconn"}->{$server}); $context->{"Zconn"}->{$server} = &_new_Zconn($server,$async,$auth,$piggyback,$syntax); - $context->{ Zconn }->{ $server }->option( - preferredRecordSyntax => C4::Context->preference("marcflavour") ); return $context->{"Zconn"}->{$server}; } } @@ -730,8 +728,8 @@ sub _new_Zconn { my $tried=0; # first attempt my $Zconn; # connection object - $server = "biblioserver" unless $server; - $syntax = "usmarc" unless $syntax; + $server //= "biblioserver"; + $syntax //= "XML"; my $host = $context->{'listen'}->{$server}->{'content'}; my $servername = $context->{"config"}->{$server}; @@ -749,7 +747,7 @@ sub _new_Zconn { $o->option(cqlfile=> $context->{"server"}->{$server}->{"cql2rpn"}); $o->option(cclfile=> $context->{"serverinfo"}->{$server}->{"ccl2rpn"}); $o->option(preferredRecordSyntax => $syntax); - $o->option(elementSetName => "F"); # F for 'full' as opposed to B for 'brief' + $o->option(elementSetName => "marcxml"); $o->option(databaseName => ($servername?$servername:"biblios")); # create a new connection object diff --git a/C4/Search.pm b/C4/Search.pm index c9f2560..df538a5 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -144,8 +144,10 @@ sub FindDuplicate { my @results; if (!defined $error) { foreach my $possible_duplicate_record (@{$searchresults}) { - my $marcrecord = - MARC::Record->new_from_usmarc($possible_duplicate_record); + my $marcrecord = MARC::Record->new_from_xml( + $possible_duplicate_record, + 'UTF-8' + ); my $result = TransformMarcToKoha( $dbh, $marcrecord, '' ); # FIXME :: why 2 $biblionumber ? @@ -446,6 +448,7 @@ sub getRecords { else { $times = $size; } + for ( my $j = $offset ; $j < $times ; $j++ ) { my $records_hash; my $record; @@ -455,7 +458,7 @@ sub getRecords { my ( $term, $occ ) = $results[ $i - 1 ]->term($j); # here we create a minimal MARC record and hand it off to the - # template just like a normal result ... perhaps not ideal, but + # template just like a normal result ... serhaps not ideal, but # it works for now my $tmprecord = MARC::Record->new(); $tmprecord->encoding('UTF-8'); @@ -488,7 +491,6 @@ sub getRecords { # not an index scan else { $record = $results[ $i - 1 ]->record($j)->raw(); - # warn "RECORD $j:".$record; $results_hash->{'RECORDS'}[$j] = $record; } @@ -503,39 +505,41 @@ sub getRecords { $size > $facets_maxrecs ? $facets_maxrecs : $size; for my $facet (@$facets) { for ( my $j = 0 ; $j < $jmax ; $j++ ) { - my $render_record = - $results[ $i - 1 ]->record($j)->render(); + + my $marc_record = MARC::Record->new_from_xml( + $results[ $i - 1 ]->record($j)->raw(), + 'UTF-8' + ); my @used_datas = (); + foreach my $tag ( @{ $facet->{tags} } ) { # avoid first line my $tag_num = substr( $tag, 0, 3 ); - my $letters = substr( $tag, 3 ); - my $field_pattern = - '\n' . $tag_num . ' ([^z][^\n]+)'; - $field_pattern = '\n' . $tag_num . ' ([^\n]+)' - if ( int($tag_num) < 10 ); - my @field_tokens = - ( $render_record =~ /$field_pattern/g ); - foreach my $field_token (@field_tokens) { - my @subf = ( $field_token =~ - /\$([a-zA-Z0-9]) ([^\$]+)/g ); - my @values; - for ( my $i = 0 ; $i < @subf ; $i += 2 ) { - if ( $letters =~ $subf[$i] ) { - my $value = $subf[ $i + 1 ]; - $value =~ s/^ *//; - $value =~ s/ *$//; - push @values, $value; - } + my $subfield_letters = substr( $tag, 3 ); + # Removed when as_string fixed + my @subfields = $subfield_letters =~ /./sg; + + my @fields = $marc_record->field($tag_num); + foreach my $field (@fields) { + # TODO: Once as_string is capable of using a separator, use this + # my $data = $field->as_string( $subfield_letters, $facet->{sep} ); + + my @values = (); + foreach my $subfield (@subfields) { + push @values, $field->subfield( $subfield ) + if defined $field->subfield( $subfield ); } + my $data = join( $facet->{sep}, @values ); + unless ( $data ~~ @used_datas ) { - $facets_counter->{ $facet->{idx} } - ->{$data}++; push @used_datas, $data; } - } # fields + + $facets_counter->{ $facet->{idx} }->{$data}++; + + } # fields } # field codes } # records $facets_info->{ $facet->{idx} }->{label_value} = @@ -1693,13 +1697,17 @@ sub searchResults { $times = $hits; # FIXME: if $hits is undefined, why do we want to equal it? } - my $marcflavour = C4::Context->preference("marcflavour"); + my $marcflavour = C4::Context->preference("marcflavour"); # We get the biblionumber position in MARC my ($bibliotag,$bibliosubf)=GetMarcFromKohaField('biblio.biblionumber',''); # loop through all of the records we've retrieved for ( my $i = $offset ; $i <= $times - 1 ; $i++ ) { - my $marcrecord = eval { MARC::File::USMARC::decode( $marcresults->[$i] ); }; + my $marcrecord = eval { MARC::Record->new_from_xml( + $marcresults->[$i], + 'UTF-8' + ); }; + if ( $@ ) { warn "ERROR DECODING RECORD - $@: " . $marcresults->[$i]; next; diff --git a/catalogue/search.pl b/catalogue/search.pl index 892ff15..4ee0387 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -660,7 +660,7 @@ for (my $i=0;$i<@servers;$i++) { elsif ($server =~/authorityserver/) { # this is the local authority server my @inner_sup_results_array; for my $sup_record ( @{$results_hashref->{$server}->{"RECORDS"}} ) { - my $marc_record_object = MARC::Record->new_from_usmarc($sup_record); + my $marc_record_object = MARC::Record->new_from_xml($sup_record,'UTF-8'); # warn "Authority Found: ".$marc_record_object->as_formatted(); push @inner_sup_results_array, { 'title' => $marc_record_object->field(100)->subfield('a'), diff --git a/circ/circulation.pl b/circ/circulation.pl index 9769731..d587a00 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -309,7 +309,7 @@ if ($barcode) { foreach my $hit ( @{$results} ) { my $chosen = TransformMarcToKoha( C4::Context->dbh, - MARC::Record->new_from_usmarc($hit) ); + MARC::Record->new_from_xml($hit,'UTF-8') ); # offer all barcodes individually foreach my $barcode ( sort split(/\s*\|\s*/, $chosen->{barcode}) ) { diff --git a/t/db_dependent/Search.t b/t/db_dependent/Search.t index 88d965c..5e8be03 100644 --- a/t/db_dependent/Search.t +++ b/t/db_dependent/Search.t @@ -346,47 +346,47 @@ sub run_marc21_search_tests { ( undef, $results_hashref, $facets_loop ) = getRecords('salud', 'salud', [], [ 'biblioserver' ], '19', 0, undef, \%branches, \%itemtypes, 'ccl', undef); - ok(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper() =~ m/^Efectos del ambiente/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[7])->title_proper() eq 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies' && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[18])->title_proper() =~ m/^Indicadores de resultados identificados/ + ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper() =~ m/^Efectos del ambiente/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[7],'UTF-8')->title_proper() eq 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies' && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[18],'UTF-8')->title_proper() =~ m/^Indicadores de resultados identificados/ , "Simple relevance sorting in getRecords matches old behavior"); ( undef, $results_hashref, $facets_loop ) = getRecords('salud', 'salud', [ 'author_az' ], [ 'biblioserver' ], '38', 0, undef, \%branches, \%itemtypes, 'ccl', undef); - ok(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper() =~ m/la enfermedad laboral\^ies$/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[6])->title_proper() =~ m/^Indicadores de resultados identificados/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[18])->title_proper() eq 'World health statistics 2009^ien' + ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper() =~ m/la enfermedad laboral\^ies$/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[6],'UTF-8')->title_proper() =~ m/^Indicadores de resultados identificados/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[18],'UTF-8')->title_proper() eq 'World health statistics 2009^ien' , "Simple ascending author sorting in getRecords matches old behavior"); ( undef, $results_hashref, $facets_loop ) = getRecords('salud', 'salud', [ 'author_za' ], [ 'biblioserver' ], '38', 0, undef, \%branches, \%itemtypes, 'ccl', undef); - ok(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper() eq 'World health statistics 2009^ien' && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[12])->title_proper() =~ m/^Indicadores de resultados identificados/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[18])->title_proper() =~ m/la enfermedad laboral\^ies$/ + ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper() eq 'World health statistics 2009^ien' && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[12],'UTF-8')->title_proper() =~ m/^Indicadores de resultados identificados/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[18],'UTF-8')->title_proper() =~ m/la enfermedad laboral\^ies$/ , "Simple descending author sorting in getRecords matches old behavior"); ( undef, $results_hashref, $facets_loop ) = getRecords('salud', 'salud', [ 'pubdate_asc' ], [ 'biblioserver' ], '38', 0, undef, \%branches, \%itemtypes, 'ccl', undef); - ok(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper() eq 'Manual de higiene industrial^ies' && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[7])->title_proper() =~ m/seguridad e higiene del trabajo\^ies$/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[18])->title_proper() =~ m/^Indicadores de resultados identificados/ + ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper() eq 'Manual de higiene industrial^ies' && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[7],'UTF-8')->title_proper() =~ m/seguridad e higiene del trabajo\^ies$/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[18],'UTF-8')->title_proper() =~ m/^Indicadores de resultados identificados/ , "Simple ascending publication date sorting in getRecords matches old behavior"); ( undef, $results_hashref, $facets_loop ) = getRecords('salud', 'salud', [ 'pubdate_dsc' ], [ 'biblioserver' ], '38', 0, undef, \%branches, \%itemtypes, 'ccl', undef); - ok(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper() =~ m/^Estado de salud/ && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[7])->title_proper() eq 'World health statistics 2009^ien' && - MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[18])->title_proper() eq 'Manual de higiene industrial^ies' + ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper() =~ m/^Estado de salud/ && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[7],'UTF-8')->title_proper() eq 'World health statistics 2009^ien' && + MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[18],'UTF-8')->title_proper() eq 'Manual de higiene industrial^ies' , "Simple descending publication date sorting in getRecords matches old behavior"); - +TODO: { + local $TODO = "Switch relevance search to MARCXML too"; ( undef, $results_hashref, $facets_loop ) = getRecords('books', 'books', [ 'relevance' ], [ 'biblioserver' ], '20', 0, undef, \%branches, \%itemtypes, undef, 1); $record = MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0]); is($record->title_proper(), 'books', "Scan returned requested item"); is($record->subfield('100', 'a'), 2, "Scan returned correct number of records matching term"); - # Time to test buildQuery and searchResults too. - +} my ( $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, $stopwords_removed, $query_type ); @@ -526,10 +526,10 @@ sub run_marc21_search_tests { ($error, $results_hashref, $facets_loop) = getRecords($query,$simple_query,[ ], [ 'biblioserver' ],20,0,undef,\%branches,\%itemtypes,$query_type,0); is($results_hashref->{biblioserver}->{hits}, 19, "Weighted query returned correct number of results"); if ($indexing_mode eq 'grs1') { - is(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper(), 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies', "Weighted query returns best match first"); + is(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper(), 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies', "Weighted query returns best match first"); } else { local $TODO = "Query weighting does not behave exactly the same in DOM vs. GRS"; - is(MARC::Record::new_from_usmarc($results_hashref->{biblioserver}->{RECORDS}->[0])->title_proper(), 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies', "Weighted query returns best match first"); + is(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'UTF-8')->title_proper(), 'Salud y seguridad de los trabajadores del sector salud: manual para gerentes y administradores^ies', "Weighted query returns best match first"); } $QueryStemming = $QueryWeightFields = $QueryFuzzy = $QueryRemoveStopwords = 0; -- 1.8.3.2