From 444a9b435d5ac571ff4147382594369f20bee479 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 2 Oct 2019 11:17:13 +0000 Subject: [PATCH] Bug 23719: [19.05.x] Allow searching specific fields for matching authorities in ES To test: 1 - Export your authorities via Tools->Export data 2 - Define a record matching rule in Admin->Record matchign rules Use index: LC-card-number field: 010$a 3 - Stage the exported records for import and use the rule created above for matching 4 - The process does not complete 5 - Check intranet error logs - exception on unknown marclist 6 - Apply patch 7 - Repeat 8 - Success! 9 - prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Jonathan Druart Bug 23719: (follow-up) Add warn when passed invalid search field in marclist Note: I also remove warnings for undefined operation in this patch, is a trivial fix To test: prove -v t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t Signed-off-by: Jonathan Druart Signed-off-by: Lucas Gass --- .../Elasticsearch/QueryBuilder.pm | 14 ++++-- .../SearchEngine/Elasticsearch/QueryBuilder.t | 46 +++++++++++++++---- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 6bee35f8cb..0f88e7fbab 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -315,7 +315,7 @@ sub build_authorities_query { foreach my $s ( @{ $search->{searches} } ) { my ( $wh, $op, $val ) = @{$s}{qw(where operator value)}; $wh = '_all' if $wh eq ''; - if ( $op eq 'is' || $op eq '=' || $op eq 'exact' ) { + if ( defined $op && ($op eq 'is' || $op eq '=' || $op eq 'exact') ) { # look for something that matches a term completely # note, '=' is about numerical vals. May need special handling. @@ -324,7 +324,7 @@ sub build_authorities_query { # search analyzer applied to them. push @query_parts, { match_phrase => {"$wh.phrase" => lc $val} }; } - elsif ( $op eq 'start' ) { + elsif ( defined $op && $op eq 'start' ) { # startswith search, uses lowercase untokenized version of heading push @query_parts, { match_phrase_prefix => {"$wh.phrase" => lc $val} }; } @@ -449,21 +449,25 @@ sub build_authorities_query_compat { # This turns the old-style many-options argument form into a more # extensible hash form that is understood by L. my @searches; + my $mappings = $self->get_elasticsearch_mappings(); # Convert to lower case $marclist = [map(lc, @{$marclist})]; $orderby = lc $orderby; + my @indexes; # Make sure everything exists foreach my $m (@$marclist) { - Koha::Exceptions::WrongParameter->throw("Invalid marclist field provided: $m") - unless exists $koha_to_index_name->{$m}; + + $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 ''); } for ( my $i = 0 ; $i < @$value ; $i++ ) { next unless $value->[$i]; #clean empty form values, ES doesn't like undefined searches push @searches, { - where => $koha_to_index_name->{$marclist->[$i]}, + where => $indexes[$i], operator => $operator->[$i], value => $value->[$i], }; diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index f0231f8476..2e9a356ba9 100644 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -19,6 +19,7 @@ use Modern::Perl; use C4::Context; use Test::Exception; +use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; use Test::More tests => 6; @@ -47,6 +48,10 @@ $se->mock( 'get_elasticsearch_mappings', sub { subject => { type => 'text' }, + 'subject-heading-thesaurus' => { + type => 'text', + facet => 1 + }, itemnumber => { type => 'integer' }, @@ -56,12 +61,24 @@ $se->mock( 'get_elasticsearch_mappings', sub { sortablenumber__sort => { type => 'integer' }, - Heading => { + heading => { type => 'text' }, - Heading__sort => { + 'heading-main' => { type => 'text' - } + }, + heading__sort => { + type => 'text' + }, + match => { + type => 'text' + }, + 'match-heading' => { + type => 'text' + }, + 'match-heading-see-from' => { + type => 'text' + }, } } }; @@ -82,7 +99,7 @@ $se->mock( 'get_elasticsearch_mappings', sub { }); subtest 'build_authorities_query_compat() tests' => sub { - plan tests => 45; + plan tests => 47; my $qb; @@ -169,12 +186,23 @@ subtest 'build_authorities_query_compat() tests' => sub { "authorities type code is used as filter" ); - # Failing case - throws_ok { - $qb->build_authorities_query_compat( [ 'tomas' ], undef, undef, ['contains'], [$search_term], 'AUTH_TYPE', 'asc' ); + # Authorities marclist check + warning_like { + $query = $qb->build_authorities_query_compat( [ 'tomas','mainentry' ], undef, undef, ['contains'], [$search_term,$search_term], 'AUTH_TYPE', 'asc' ) } - 'Koha::Exceptions::WrongParameter', - 'Exception thrown on invalid value in the marclist param'; + qr/Unknown search field tomas/, + "Warning for unknown field in marclist"; + is_deeply( + $query->{query}->{bool}->{must}[0]->{query_string}->{default_field}, + 'tomas', + "If no mapping for marclist the index is passed through as defined" + ); + is_deeply( + $query->{query}->{bool}->{must}[1]->{query_string}{default_field}, + 'heading', + "If mapping found for marclist the index is passed through converted" + ); + }; subtest 'build_query tests' => sub { -- 2.39.5