From 4a36c1ac044ed9da3b67d32e9c315e61d2adc3bd Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 31 Oct 2019 14:34:53 +0000 Subject: [PATCH] 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: Martin Renvoize --- .../Elasticsearch/QueryBuilder.pm | 10 ++++-- .../SearchEngine/Elasticsearch/QueryBuilder.t | 31 ++++++++++++++++--- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index daf7e65688..4a7cede380 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -285,7 +285,7 @@ sub build_authorities_query { foreach my $s ( @{ $search->{searches} } ) { my ( $wh, $op, $val ) = @{$s}{qw(where operator value)}; - if ( $op eq 'is' || $op eq '=' || $op eq 'exact') { + if ( defined $op && ($op eq 'is' || $op eq '=' || $op eq 'exact') ) { if ($wh) { # Match the whole field, case insensitive, UTF normalized. push @query_parts, { term => { "$wh.ci_raw" => $val } }; @@ -304,7 +304,7 @@ sub build_authorities_query { }; } } - elsif ( $op eq 'start') { + elsif ( defined $op && $op eq 'start') { # Match the prefix within a field for all searchable fields. # Given that field data is "The quick brown fox" # "The quick bro" will match, but not "quick bro" @@ -464,6 +464,7 @@ 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})]; @@ -472,7 +473,10 @@ sub build_authorities_query_compat { my @indexes; # Make sure everything exists foreach my $m (@$marclist) { - push @indexes, exists $koha_to_index_name->{$m} ? $koha_to_index_name->{$m} : $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 diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index ccde4d911b..875dee981c 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; @@ -50,6 +51,10 @@ $se->mock( 'get_elasticsearch_mappings', sub { type => 'text', facet => 1 }, + 'subject-heading-thesaurus' => { + type => 'text', + facet => 1 + }, itemnumber => { type => 'integer' }, @@ -59,12 +64,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' + }, } } }; @@ -92,7 +109,7 @@ my $clear_search_fields_cache = sub { subtest 'build_authorities_query_compat() tests' => sub { - plan tests => 56; + plan tests => 57; my $qb; @@ -186,7 +203,11 @@ subtest 'build_authorities_query_compat() tests' => sub { ); # Authorities marclist check - $query = $qb->build_authorities_query_compat( [ 'tomas','mainentry' ], undef, undef, ['contains'], [$search_term,$search_term], 'AUTH_TYPE', 'asc' ); + warning_like { + $query = $qb->build_authorities_query_compat( [ 'tomas','mainentry' ], undef, undef, ['contains'], [$search_term,$search_term], 'AUTH_TYPE', 'asc' ) + } + qr/Unknown search field tomas/, + "Warning for unknown field in marclist"; is_deeply( $query->{query}->{bool}->{must}[0]->{query_string}->{default_field}, 'tomas', -- 2.39.5