From aba8b130f8b96e5000e8a1eccecfb53d7b371c30 Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Tue, 5 Mar 2019 11:04:55 +0200 Subject: [PATCH] Bug 22413: Improve query string and desc creation for Elasticsearch Test plan: After each of the following steps, verify that the results and query description looks ok. 1. Do a simple keyword query 2. Change sort order 3. Click a facet 4. Do an advanced search with title and author 5. Change sort order 6. Click a facet Signed-off-by: Josef Moravec Signed-off-by: Marjorie Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- .../Elasticsearch/QueryBuilder.pm | 26 ++++++++++++++----- .../SearchEngine/Elasticsearch/QueryBuilder.t | 13 +++++++--- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 1560d16ea0..084d5ad3db 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -227,8 +227,9 @@ sub build_query_compat { # would be to pass them separately into build_query and let it build # them into a structured ES query itself. Maybe later, though that'd be # more robust. + my $search_param_query_str = join( ' ', $self->_create_query_string(@search_params) ); my $query_str = join( ' AND ', - join( ' ', $self->_create_query_string(@search_params) ) || (), + $search_param_query_str || (), $self->_join_queries( $self->_convert_index_strings(@$limits) ) || () ); my @fields = '_all'; @@ -244,19 +245,32 @@ sub build_query_compat { $options{expanded_facet} = $params->{expanded_facet}; my $query = $self->build_query( $query_str, %options ); - #die Dumper($query); # We roughly emulate the CGI parameters of the zebra query builder - my $query_cgi; - $query_cgi = 'q=' . uri_escape_utf8( $operands->[0] ) if @$operands; + my $query_cgi = ''; + shift @$operators; # Shift out the one we unshifted before + $ea = each_array( @$operands, @$operators, @$indexes ); + while ( my ( $oand, $otor, $index ) = $ea->() ) { + $query_cgi .= '&' if $query_cgi; + $query_cgi .= 'idx=' . uri_escape_utf8( $index // '') . '&q=' . uri_escape_utf8( $oand ); + $query_cgi .= '&op=' . uri_escape_utf8( $otor ) if $otor; + } + $query_cgi .= '&scan=1' if ( $scan ); + my $simple_query; $simple_query = $operands->[0] if @$operands == 1; - my $query_desc = $simple_query; - my $limit = $self->_join_queries( $self->_convert_index_strings(@$limits)); + my $query_desc; + if ( $simple_query ) { + $query_desc = $simple_query; + } else { + $query_desc = $search_param_query_str; + } + my $limit = $self->_join_queries( $self->_convert_index_strings(@$limits)); my $limit_cgi = ( $orig_limits and @$orig_limits ) ? '&limit=' . join( '&limit=', map { uri_escape_utf8($_) } @$orig_limits ) : ''; my $limit_desc; $limit_desc = "$limit" if $limit; + return ( undef, $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, undef, undef diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index 03d4d36999..df2f323383 100644 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -169,7 +169,7 @@ subtest 'build_authorities_query_compat() tests' => sub { }; subtest 'build_query tests' => sub { - plan tests => 30; + plan tests => 33; my $qb; @@ -238,6 +238,11 @@ subtest 'build_query tests' => sub { 'multiple query terms are enclosed in parenthesis while a single one is not' ); + my ($simple_query, $query_cgi, $query_desc); + ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['"donald duck"', 'walt disney'], ['ti', 'au'] ); + is($query_cgi, 'idx=ti&q=%22donald%20duck%22&idx=au&q=walt%20disney', 'query cgi ok for multiterm query'); + is($query_desc, '(title:("donald duck")) (author:(walt disney))', 'query desc ok for multiterm query'); + t::lib::Mocks::mock_preference( 'QueryAutoTruncate', '1' ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'] ); @@ -362,14 +367,14 @@ subtest 'build_query tests' => sub { "query of specific field is added AND suppress:0" ); - my ($simple_query, $query_cgi); - ( undef, $query, $simple_query, $query_cgi ) = $qb->build_query_compat( undef, ['title:"donald duck"'], undef, undef, undef, undef, undef, { suppress => 0 } ); + ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['title:"donald duck"'], undef, undef, undef, undef, undef, { suppress => 0 } ); is( $query->{query}{query_string}{query}, '(title:"donald duck")', "query of specific field is not added AND suppress:0" ); - is($query_cgi, 'q=title%3A%22donald%20duck%22', 'query cgi'); + is($query_cgi, 'idx=&q=title%3A%22donald%20duck%22', 'query cgi'); + is($query_desc, 'title:"donald duck"', 'query desc ok'); }; -- 2.39.5