Attachment 105435 Details for Bug 25273 – Bug 25273: Make match-heading rely on authority type configuration

[patch] Bug 25273: Make match-heading rely on authority type configuration

Bug-25273-Make-match-heading-rely-on-authority-typ.patch (text/plain), 15.92 KB, created by Nick Clemens (kidclamp) on 2020-05-29 11:53:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2020-05-29 11:53:42 UTC

Size: 15.92 KB

patch

obsolete

>From cd2a7dd9e1824913c5dbafee658679d2e16b9fcf Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Tue, 28 Apr 2020 12:19:56 +0000
>Subject: [PATCH] Bug 25273: Make match-heading rely on authority type
> configuration
>
>The match-heading field is a special field used only by the linker, not accessible
>to staff or patrons via the interface. This field is used to store the constructed
>'search form' used for matching bib headings to authority fields.
>
>In bug 24269 I attempted to use the mappings defined in the inferface and also inject the search term.
>This did not work as too many subfields were indexed on their own and leading to false matches.
>In this bug we remove the mappings for this field, and create it ourselves during
>the indexing process. The C4::Headings module is still used to generate the correct form,
>however, the mappings are set based on the authority types in the system. This gives the user
>the ability to add new typoes, but prevents mapping changes from breaking linker functionality
>
>To test:
> 1 - Start form a sample database with ElasticSearch working
> 2 - Download via Z39.50 2 authorities, one of which is a narrower heading of the other, e.g.:
>    Waterworks
>    Waterworks - Costs
> 3 - Place a heading for the broader term in a record. e.g. Waterworks
>       In 650$a, without the cataloguing authority plugin. We don't want
>       the link created now.
>       You need syspref BiblioAddsAuthorities => allow
> 4 - Make sure linker is set to default
> 5 - Attempt to link the records
>       misc/link_bibs_to_authorities.pl
> 6 - Linking fails
> 7 - Apply patch
> 8 - refresh index settings (if using a custom file, remove 'match-heading')
>       You can reset mappings in the UI or run this:
>       misc/search_tools/rebuild_elasticsearch.pl -v -d -r
> 9 - Reindex ES
>10 - Try to link again
>11 - It succeeds!
>12 - Run the tests
>     prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t
>
>Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>---
> C4/AuthoritiesMarc.pm                              |  2 +-
> C4/Biblio.pm                                       | 11 +--
> Koha/SearchEngine/Elasticsearch.pm                 | 30 ++++----
> Koha/SearchEngine/Elasticsearch/QueryBuilder.pm    |  2 +-
> admin/searchengine/elasticsearch/mappings.yaml     | 24 -------
> misc/search_tools/rebuild_elasticsearch.pl         |  8 ++-
> .../Koha/SearchEngine/Elasticsearch.t              | 81 +++++++---------------
> 7 files changed, 54 insertions(+), 104 deletions(-)
> rename t/{ => db_dependent}/Koha/SearchEngine/Elasticsearch.t (94%)
>
>diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm
>index c2aade898c..6bd2e0baf8 100644
>--- a/C4/AuthoritiesMarc.pm
>+++ b/C4/AuthoritiesMarc.pm
>@@ -629,7 +629,7 @@ sub AddAuthority {
>     $record->insert_fields_ordered( MARC::Field->new( '001', $authid ) );
>     # Update
>     $dbh->do( "UPDATE auth_header SET authtypecode=?, marc=?, marcxml=? WHERE authid=?", undef, $authtypecode, $record->as_usmarc, $record->as_xml_record($format), $authid ) or die $DBI::errstr;
>-    ModZebra( $authid, 'specialUpdate', 'authorityserver', $record );
>+    ModZebra( $authid, 'specialUpdate', 'authorityserver', { record => $record, authtypecode => $authtypecode } );
> 
>     return ( $authid );
> }
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index 9eb3696a54..2e991427b8 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -2546,8 +2546,8 @@ $op is specialUpdate or recordDelete, and is used to know what we want to do
> 
> $server is the server that we want to update
> 
>-$record is the update MARC record if it's available. If it's not supplied
>-and is needed, it'll be loaded from the database.
>+$record is a hash including the authtypecode and updated MARC record. This is only used for
>+Elasticsearch authorities. If it's not supplied a biblio will be loaded from the database.
> 
> =cut
> 
>@@ -2567,13 +2567,14 @@ sub ModZebra {
>             }
>         );
>         if ( $op eq 'specialUpdate' ) {
>-            unless ($record) {
>+            if ($record) {
>+                $indexer->update_index_background( [$biblionumber], [$record] );
>+            } else {
>                 $record = GetMarcBiblio({
>                     biblionumber => $biblionumber,
>                     embed_items  => 1 });
>+                $indexer->update_index_background( [$biblionumber], [{ record => $record }] );
>             }
>-            my $records = [$record];
>-            $indexer->update_index_background( [$biblionumber], [$record] );
>         }
>         elsif ( $op eq 'recordDelete' ) {
>             $indexer->delete_index_background( [$biblionumber] );
>diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
>index a43b864145..f3e663e54a 100644
>--- a/Koha/SearchEngine/Elasticsearch.pm
>+++ b/Koha/SearchEngine/Elasticsearch.pm
>@@ -234,10 +234,10 @@ sub get_elasticsearch_mappings {
>                 }
>             }
>         );
>+        $mappings->{data}{properties}{ 'match-heading' } = _get_elasticsearch_field_config('search', 'text') if $self->index eq 'authorities';
>         $all_mappings{$self->index} = $mappings;
>     }
>     $self->sort_fields(\%{$sort_fields{$self->index}});
>-
>     return $all_mappings{$self->index};
> }
> 
>@@ -527,8 +527,22 @@ sub marc_records_to_documents {
> 
>     my @record_documents;
> 
>+    my %auth_match_headings;
>+    if( $self->index eq 'authorities' ){
>+        my @auth_types = Koha::Authority::Types->search();
>+        %auth_match_headings = map { $_->authtypecode => $_->auth_tag_to_report } @auth_types;
>+    }
>+
>     foreach my $record (@{$records}) {
>         my $record_document = {};
>+
>+        if ( defined $record->{authtypecode} ){
>+            my $field = $record->{record}->field( $auth_match_headings{ $record->{authtypecode} } );
>+            my $heading = C4::Heading->new_from_field( $field, undef, 1 ); #new auth heading
>+            push @{$record_document->{'match-heading'}}, $heading->search_form if $heading;
>+        }
>+        $record = $record->{record};
>+
>         my $mappings = $rules->{leader};
>         if ($mappings) {
>             $self->_process_mappings($mappings, $record->leader(), $record_document, {
>@@ -580,13 +594,6 @@ sub marc_records_to_documents {
>                                 }
>                             );
>                         }
>-                        if ( @{$mappings} && grep { $_->[0] eq 'match-heading'} @{$mappings} ){
>-                            # Used by the authority linker the match-heading field requires a specific syntax
>-                            # that is specified in C4/Heading
>-                            my $heading = C4::Heading->new_from_field( $field, undef, 1 ); #new auth heading
>-                            next unless $heading;
>-                            push @{$record_document->{'match-heading'}}, $heading->search_form;
>-                        }
>                     }
> 
>                     my $subfields_join_mappings = $data_field_rules->{subfields_join};
>@@ -609,13 +616,6 @@ sub marc_records_to_documents {
>                                     }
>                                 );
>                             }
>-                            if ( grep { $_->[0] eq 'match-heading' } @{$subfields_join_mappings->{$subfields_group}} ){
>-                                # Used by the authority linker the match-heading field requires a specific syntax
>-                                # that is specified in C4/Heading
>-                                my $heading = C4::Heading->new_from_field( $field, undef, 1 ); #new auth heading
>-                                next unless $heading;
>-                                push @{$record_document->{'match-heading'}}, $heading->search_form;
>-                            }
>                         }
>                     }
>                 }
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index 871f81d027..d72f320698 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -476,7 +476,7 @@ sub build_authorities_query_compat {
> 
>         $m = exists $koha_to_index_name->{$m} ? $koha_to_index_name->{$m} : $m;
>         push @indexes, $m;
>-        warn "Unknown search field $m in marclist" unless (defined $mappings->{data}->{properties}->{$m} || $m eq '');
>+        warn "Unknown search field $m in marclist" unless (defined $mappings->{data}->{properties}->{$m} || $m eq '' || $m eq 'match-heading');
>     }
>     for ( my $i = 0 ; $i < @$value ; $i++ ) {
>         next unless $value->[$i]; #clean empty form values, ES doesn't like undefined searches
>diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml
>index b72fe18be4..847647e0fd 100644
>--- a/admin/searchengine/elasticsearch/mappings.yaml
>+++ b/admin/searchengine/elasticsearch/mappings.yaml
>@@ -695,30 +695,6 @@ authorities:
>         sort: ~
>         suggestible: ''
>     type: ''
>-  Match-heading:
>-    label: Match-heading
>-    mappings:
>-      - facet: ''
>-        marc_field: 100(abcdefghjklmnopqrstvxyz)
>-        marc_type: marc21
>-        sort: ~
>-        suggestible: ''
>-      - facet: ''
>-        marc_field: 111(acdefghjklnpqstvxyz)
>-        marc_type: marc21
>-        sort: ~
>-        suggestible: ''
>-      - facet: ''
>-        marc_field: 100(abcdefghjklmnopqrstvxyz)
>-        marc_type: normarc
>-        sort: ~
>-        suggestible: ''
>-      - facet: ''
>-        marc_field: 111(acdefghjklnpqstvxyz)
>-        marc_type: normarc
>-        sort: ~
>-        suggestible: ''
>-    type: ''
>   Match-heading-see-from:
>     label: Match-heading-see-from
>     mappings:
>diff --git a/misc/search_tools/rebuild_elasticsearch.pl b/misc/search_tools/rebuild_elasticsearch.pl
>index 6cff2eae2e..8bb6110ba1 100755
>--- a/misc/search_tools/rebuild_elasticsearch.pl
>+++ b/misc/search_tools/rebuild_elasticsearch.pl
>@@ -275,8 +275,10 @@ sub _do_reindex {
>     my $commit_count = $commit;
>     my ( @id_buffer, @commit_buffer );
>     while ( my $record = $next->() ) {
>-        my $id     = $record->id // $record->authid;
>-        my $record = $record->record;
>+        my $id     = $record->id;
>+        my $record_prepped;
>+        $record_prepped->{authtypecode} = $record->authtypecode if ref $record eq 'Koha::MetadataRecord::Authority';
>+        $record_prepped->{record} = $record->record;
>         $count++;
>         if ( $verbose == 1 ) {
>             _log( 1, "$count records processed\n" ) if ( $count % 1000 == 0);
>@@ -285,7 +287,7 @@ sub _do_reindex {
>         }
> 
>         push @id_buffer,     $id;
>-        push @commit_buffer, $record;
>+        push @commit_buffer, $record_prepped;
>         if ( !( --$commit_count ) ) {
>             _log( 1, "Committing $commit records...\n" );
>             my $response = $indexer->update_index( \@id_buffer, \@commit_buffer );
>diff --git a/t/Koha/SearchEngine/Elasticsearch.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t
>similarity index 94%
>rename from t/Koha/SearchEngine/Elasticsearch.t
>rename to t/db_dependent/Koha/SearchEngine/Elasticsearch.t
>index fede84ca34..6ae3d392fa 100644
>--- a/t/Koha/SearchEngine/Elasticsearch.t
>+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t
>@@ -21,6 +21,7 @@ use Test::More tests => 6;
> use Test::Exception;
> 
> use t::lib::Mocks;
>+use t::lib::TestBuilder;
> 
> use Test::MockModule;
> 
>@@ -31,6 +32,9 @@ use List::Util qw( any );
> use Koha::SearchEngine::Elasticsearch;
> use Koha::SearchEngine::Elasticsearch::Search;
> 
>+my $schema = Koha::Database->new->schema;
>+$schema->storage->txn_begin;
>+
> subtest '_read_configuration() tests' => sub {
> 
>     plan tests => 10;
>@@ -336,7 +340,10 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
>         MARC::Field->new('999', '', '', c => '1234568'),
>         MARC::Field->new('952', '', '', 0 => 1, g => 'string where should be numeric', o => $long_callno),
>     );
>-    my $records = [$marc_record_1, $marc_record_2];
>+    my $records = [
>+        { record => $marc_record_1 },
>+        { record => $marc_record_2 },
>+    ];
> 
>     $see->get_elasticsearch_mappings(); #sort_fields will call this and use the actual db values unless we call it first
> 
>@@ -503,7 +510,7 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
>         $large_marc_record->append_fields($item_field);
>     }
> 
>-    $docs = $see->marc_records_to_documents([$large_marc_record]);
>+    $docs = $see->marc_records_to_documents([{ record => $large_marc_record }]);
> 
>     is($docs->[0]->{marc_format}, 'MARCXML', 'For record exceeding max record size marc_format should be set correctly');
> 
>@@ -616,7 +623,10 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents_array () t
>         MARC::Field->new('999', '', '', c => '1234568'),
>         MARC::Field->new('952', '', '', 0 => 1, g => 'string where should be numeric'),
>     );
>-    my $records = [$marc_record_1, $marc_record_2];
>+    my $records = [
>+        { record => $marc_record_1 },
>+        { record => $marc_record_2 },
>+    ];
> 
>     $see->get_elasticsearch_mappings(); #sort_fields will call this and use the actual db values unless we call it first
> 
>@@ -642,6 +652,12 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () authori
>     t::lib::Mocks::mock_preference('marcflavour', 'MARC21');
>     t::lib::Mocks::mock_preference('ElasticsearchMARCFormat', 'ISO2709');
> 
>+    my $builder = t::lib::TestBuilder->new;
>+    my $auth_type = $builder->build_object({ class => 'Koha::Authority::Types', value =>{
>+            auth_tag_to_report => '150'
>+        }
>+    });
>+
>     my @mappings = (
>         {
>             name => 'match',
>@@ -652,57 +668,7 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () authori
>             sort => 0,
>             marc_type => 'marc21',
>             marc_field => '150(ae)',
>-        },
>-        {
>-            name => 'heading',
>-            type => 'string',
>-            facet => 0,
>-            suggestible => 0,
>-            searchable => 1,
>-            sort => 0,
>-            marc_type => 'marc21',
>-            marc_field => '150a',
>-        },
>-        {
>-            name => 'heading',
>-            type => 'string',
>-            facet => 0,
>-            suggestible => 0,
>-            searchable => 1,
>-            sort => 0,
>-            marc_type => 'marc21',
>-            marc_field => '150(ae)',
>-        },
>-        {
>-            name => 'heading-main',
>-            type => 'string',
>-            facet => 0,
>-            suggestible => 0,
>-            searchable => 1,
>-            sort => 0,
>-            marc_type => 'marc21',
>-            marc_field => '150a',
>-        },
>-        {
>-            name => 'heading',
>-            type => 'string',
>-            facet => 0,
>-            suggestible => 0,
>-            searchable => 1,
>-            sort => 0,
>-            marc_type => 'marc21',
>-            marc_field => '150',
>-        },
>-        {
>-            name => 'match-heading',
>-            type => 'string',
>-            facet => 0,
>-            suggestible => 0,
>-            searchable => 1,
>-            sort => 0,
>-            marc_type => 'marc21',
>-            marc_field => '150',
>-        },
>+        }
>     );
> 
>     my $se = Test::MockModule->new('Koha::SearchEngine::Elasticsearch');
>@@ -735,7 +701,10 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () authori
>     $marc_record_2->append_fields(
>         MARC::Field->new('150', '', '', a => 'Subject', v => 'Genresubdiv', z => 'Geosubdiv', x => 'Generalsubdiv', e => 'wrongsubdiv' ),
>     );
>-    my $records = [$marc_record_1, $marc_record_2];
>+    my $records = [
>+        { record => $marc_record_1, authtypecode=> $auth_type->authtypecode },
>+        { record => $marc_record_2, authtypecode=> $auth_type->authtypecode },
>+    ];
> 
>     $see->get_elasticsearch_mappings(); #sort_fields will call this and use the actual db values unless we call it first
> 
>@@ -752,3 +721,5 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () authori
>         "Second record match-heading should contain the correctly formatted heading without wrong subfield"
>     );
> };
>+
>+$schema->storage->txn_rollback;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 25273: 103801 | 103854 | 104363 | 105435 | 106759 | 107434 | 108989 | 109261 | 109262 | 109263