From 0e5b442d992a298cc6f56813807607d2b397a3f8 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Fri, 13 Apr 2018 15:46:55 +0200 Subject: [PATCH] Bug 20589: Add field boosting and use query_string fields parameter Generate a list of fields for the query_string query fields parameter, with possible boosts, instead of using "_all"-field. Also add "search" flag in search_marc_to_field table so that certain mappings can be excluded from searches. Add option to include/exclude fields in query_string "fields" parameter depending on searching in OPAC or staff client. Refactor code to remove all other dependencies on "_all"-field. How to test: 1) Reindex authorities and biblios. 2) Search biblios and try to verify that this works as expected. 3) Search authorities and try to verify that this works as expected. 4) Go to "Search engine configuration" 5) Change some "Boost", "Staff client", and "OPAC" settings and save. 6) Verify that those settings where saved accordingly. 7) Click the "Biblios" or "Authorities" tab and change one or more "Searchable" settings 8) Verfiy that those settings where saved accordingly. 9) Try to verify that these settings has taken effect by peforming some biblios and/or authorities searches. Sponsorded-by: Gothenburg Univesity Library --- Koha/Schema/Result/SearchField.pm | 4 + Koha/Schema/Result/SearchMarcToField.pm | 2 + Koha/SearchEngine/Elasticsearch.pm | 72 +++++-- Koha/SearchEngine/Elasticsearch/QueryBuilder.pm | 230 +++++++++++++++++---- Koha/SearchField.pm | 1 - Koha/SearchFields.pm | 15 -- admin/searchengine/elasticsearch/field_config.yaml | 34 ++- admin/searchengine/elasticsearch/index_config.yaml | 12 +- admin/searchengine/elasticsearch/mappings.pl | 76 +++++-- catalogue/search.pl | 7 +- .../mysql/atomicupdate/bug_20589_add_columns.perl | 14 ++ .../admin/searchengine/elasticsearch/mappings.tt | 66 +++++- opac/opac-search.pl | 25 ++- .../Koha/SearchEngine/Elasticsearch/QueryBuilder.t | 62 ++++-- 14 files changed, 453 insertions(+), 167 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_20589_add_columns.perl diff --git a/Koha/Schema/Result/SearchField.pm b/Koha/Schema/Result/SearchField.pm index a347da08bb..3374905661 100644 --- a/Koha/Schema/Result/SearchField.pm +++ b/Koha/Schema/Result/SearchField.pm @@ -87,6 +87,10 @@ __PACKAGE__->add_columns( { data_type => "decimal", is_nullable => 1, size => [5, 2] }, "facet_order", { data_type => "tinyint", is_nullable => 1 }, + "staff_client", + { data_type => "tinyint", size => 1, is_nullable => 0, default => 1 }, + "opac", + { data_type => "tinyint", size => 1, is_nullable => 0, default => 1 }, ); =head1 PRIMARY KEY diff --git a/Koha/Schema/Result/SearchMarcToField.pm b/Koha/Schema/Result/SearchMarcToField.pm index bfd27eab48..5f3ca053e9 100644 --- a/Koha/Schema/Result/SearchMarcToField.pm +++ b/Koha/Schema/Result/SearchMarcToField.pm @@ -71,6 +71,8 @@ __PACKAGE__->add_columns( { data_type => "tinyint", default_value => 0, is_nullable => 1 }, "sort", { data_type => "tinyint", is_nullable => 1 }, + "search", + { data_type => "tinyint", size => 1, is_nullable => 0, default => 1 }, ); =head1 PRIMARY KEY diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index 3104e2ab04..bc62daad07 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -200,8 +200,7 @@ sub get_elasticsearch_mappings { my $marcflavour = lc C4::Context->preference('marcflavour'); $self->_foreach_mapping( sub { - my ( $name, $type, $facet, $suggestible, $sort, $marc_type ) = @_; - + my ( $name, $type, $facet, $suggestible, $sort, $search, $marc_type ) = @_; return if $marc_type ne $marcflavour; # TODO if this gets any sort of complexity to it, it should # be broken out into its own function. @@ -217,7 +216,9 @@ sub get_elasticsearch_mappings { $es_type = 'stdno'; } - $mappings->{data}{properties}{$name} = _get_elasticsearch_field_config('search', $es_type); + if ($search) { + $mappings->{data}{properties}{$name} = _get_elasticsearch_field_config('search', $es_type); + } if ($facet) { $mappings->{data}{properties}{ $name . '__facet' } = _get_elasticsearch_field_config('facet', $es_type); @@ -284,15 +285,41 @@ sub reset_elasticsearch_mappings { while ( my ( $index_name, $fields ) = each %$indexes ) { while ( my ( $field_name, $data ) = each %$fields ) { - my %sf_params = map { $_ => $data->{$_} } grep { exists $data->{$_} } qw/ type label weight facet_order /; - $sf_params{name} = $field_name; - - my $search_field = Koha::SearchFields->find_or_create( \%sf_params, { key => 'name' } ); - + my $field_type = $data->{type}; + my $field_label = $data->{label}; + my $field_weight = $data->{weight}; + my $staff_client = exists $data->{staff_client} ? $data->{staff_client} : 1; + my $opac = exists $data->{opac} ? $data->{opac} : 1; my $mappings = $data->{mappings}; + my $facet_order = $data->{facet_order}; + my $search_field = Koha::SearchFields->find_or_create({ + name => $field_name, + label => $field_label, + weight => $field_weight, + type => $field_type, + staff_client => $staff_client, + opac => $opac + }, + { + key => 'name' + }); + $search_field->update( + { + facet_order => $facet_order + } + ); for my $mapping ( @$mappings ) { - my $marc_field = Koha::SearchMarcMaps->find_or_create({ index_name => $index_name, marc_type => $mapping->{marc_type}, marc_field => $mapping->{marc_field} }); - $search_field->add_to_search_marc_maps($marc_field, { facet => $mapping->{facet} || 0, suggestible => $mapping->{suggestible} || 0, sort => $mapping->{sort} } ); + my $marc_field = Koha::SearchMarcMaps->find_or_create({ + index_name => $index_name, + marc_type => $mapping->{marc_type}, + marc_field => $mapping->{marc_field} + }); + $search_field->add_to_search_marc_maps($marc_field, { + facet => $mapping->{facet} || 0, + suggestible => $mapping->{suggestible} || 0, + sort => $mapping->{sort}, + search => $mapping->{search} || 1 + }); } } } @@ -650,9 +677,9 @@ sub _array_to_marc { return $record; } -=head2 _field_mappings($facet, $suggestible, $sort, $target_name, $target_type, $range) +=head2 _field_mappings($facet, $suggestible, $sort, $search, $target_name, $target_type, $range) - my @mappings = _field_mappings($facet, $suggestible, $sort, $target_name, $target_type, $range) + my @mappings = _field_mappings($facet, $suggestible, $sort, $search, $target_name, $target_type, $range) Get mappings, an internal data structure later used by L<_process_mappings($mappings, $data, $record_document, $altscript)> to process MARC target @@ -681,6 +708,10 @@ Boolean indicating whether to create a suggestion field for this mapping. Boolean indicating whether to create a sort field for this mapping. +=item C<$search> + +Boolean indicating whether to create a search field for this mapping. + =item C<$target_name> Elasticsearch document target field name. @@ -706,7 +737,7 @@ be extracted. =cut sub _field_mappings { - my ($_self, $facet, $suggestible, $sort, $target_name, $target_type, $range) = @_; + my ($_self, $facet, $suggestible, $sort, $search, $target_name, $target_type, $range) = @_; my %mapping_defaults = (); my @mappings; @@ -734,8 +765,10 @@ sub _field_mappings { }; } - my $mapping = [$target_name, $default_options]; - push @mappings, $mapping; + if ($search) { + my $mapping = [$target_name, $default_options]; + push @mappings, $mapping; + } my @suffixes = (); push @suffixes, 'facet' if $facet; @@ -792,7 +825,7 @@ sub _get_marc_mapping_rules { }; $self->_foreach_mapping(sub { - my ($name, $type, $facet, $suggestible, $sort, $marc_type, $marc_field) = @_; + my ($name, $type, $facet, $suggestible, $sort, $search, $marc_type, $marc_field) = @_; return if $marc_type ne $marcflavour; if ($type eq 'sum') { @@ -855,7 +888,7 @@ sub _get_marc_mapping_rules { } my $range = defined $3 ? $3 : undef; - my @mappings = $self->_field_mappings($facet, $suggestible, $sort, $name, $type, $range); + my @mappings = $self->_field_mappings($facet, $suggestible, $sort, $search, $name, $type, $range); if ($field_tag < 10) { $rules->{control_fields}->{$field_tag} //= []; @@ -875,7 +908,7 @@ sub _get_marc_mapping_rules { } elsif ($marc_field =~ $leader_regexp) { my $range = defined $1 ? $1 : undef; - my @mappings = $self->_field_mappings($facet, $suggestible, $sort, $name, $type, $range); + my @mappings = $self->_field_mappings($facet, $suggestible, $sort, $search, $name, $type, $range); push @{$rules->{leader}}, @mappings; } else { @@ -954,6 +987,7 @@ sub _foreach_mapping { 'search_marc_to_fields.facet', 'search_marc_to_fields.suggestible', 'search_marc_to_fields.sort', + 'search_marc_to_fields.search', 'search_marc_map.marc_type', 'search_marc_map.marc_field', ], @@ -961,6 +995,7 @@ sub _foreach_mapping { 'facet', 'suggestible', 'sort', + 'search', 'marc_type', 'marc_field', ], @@ -976,6 +1011,7 @@ sub _foreach_mapping { $search_field->get_column('facet'), $search_field->get_column('suggestible'), $search_field->get_column('sort'), + $search_field->get_column('search'), $search_field->get_column('marc_type'), $search_field->get_column('marc_field'), ); diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 5c4eb62463..75e6ef782f 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -48,6 +48,7 @@ use URI::Escape; use C4::Context; use Koha::Exceptions; +use Koha::Caches; =head2 build_query @@ -90,7 +91,7 @@ sub build_query { query => $query, fuzziness => $fuzzy_enabled ? 'auto' : '0', default_operator => 'AND', - default_field => '_all', + fields => $self->_search_fields({ is_opac => $options{is_opac}, weighted_fields => $options{weighted_fields} }), lenient => JSON::true, analyze_wildcard => JSON::true, fields => $options{fields} || [], @@ -231,16 +232,12 @@ sub build_query_compat { $search_param_query_str || (), $self->_join_queries( $self->_convert_index_strings(@$limits) ) || () ); - my @fields = '_all'; - if ( defined($params->{weighted_fields}) && $params->{weighted_fields} ) { - push @fields, sprintf("%s^%s", $_->name, $_->weight) for Koha::SearchFields->weighted_fields; - } - # If there's no query on the left, let's remove the junk left behind $query_str =~ s/^ AND //; my %options; - $options{fields} = \@fields; $options{sort} = \@sort_params; + $options{is_opac} = $params->{is_opac}; + $options{weighted_fields} = $params->{weighted_fields}; my $query = $self->build_query( $query_str, %options ); # We roughly emulate the CGI parameters of the zebra query builder @@ -314,22 +311,57 @@ 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' ) { - - # look for something that matches a term completely - # note, '=' is about numerical vals. May need special handling. - # Also, we lowercase our search because the ES - # index lowercases its values, and term searches don't get the - # search analyzer applied to them. - push @query_parts, { match_phrase => {"$wh.phrase" => lc $val} }; + if ( $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 } }; + } + else { + # Match the whole field for all searchable fields, case insensitive, + # UTF normalized. + # Given that field data is "The quick brown fox" + # "The quick brown fox" and "the quick brown fox" will match + # but not "quick brown fox". + push @query_parts, { + multi_match => { + query => $val, + fields => $self->_search_fields({ subfield => 'ci_raw' }), + } + }; + } } - elsif ( $op eq 'start' ) { - # startswith search, uses lowercase untokenized version of heading - push @query_parts, { match_phrase_prefix => {"$wh.phrase" => lc $val} }; + elsif ( $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" + + # Does not seems to be a multi prefix query + # so we need to create one + if ($wh) { + # Match prefix of the field. + push @query_parts, { prefix => {"$wh.ci_raw" => $val} }; + } + else { + my @prefix_queries; + foreach my $field (@{$self->_search_fields()}) { + push @prefix_queries, { + prefix => { "$field.ci_raw" => $val } + }; + } + push @query_parts, { + 'bool' => { + 'should' => \@prefix_queries, + 'minimum_should_match' => 1 + } + }; + } } else { - # regular wordlist stuff + # Query all searchable fields. + # Given that field data is "The quick brown fox" + # a search containing any of the words will match, regardless + # of order. + my @tokens = $self->_split_query( $val ); foreach my $token ( @tokens ) { $token = $self->_truncate_terms( @@ -337,36 +369,47 @@ sub build_authorities_query { ); } my $query = $self->_join_queries( @tokens ); - push @query_parts, { query_string => { default_field => $wh, query => $query } }; + + if ($wh) { + push @query_parts, { query_string => { default_field => $wh, query => $query } }; + } + else { + push @query_parts, { + query_string => { + query => $query, + fields => $self->_search_fields(), + } + }; + } } } # Merge the query parts appropriately # 'should' behaves like 'or' # 'must' behaves like 'and' - # Zebra results seem to match must so using that here - my $query = { query => - { bool => - { must => \@query_parts } - } - }; - - my %s; - if ( exists $search->{sort} ) { - foreach my $k ( keys %{ $search->{sort} } ) { - my $f = $self->_sort_field($k); - $s{$f} = $search->{sort}{$k}; - } - $search->{sort} = \%s; + # Zebra behaviour seem to match must so using that here + my $elastic_query = {}; + $elastic_query->{bool}->{must} = \@query_parts; + + # Filter by authtypecode if set + if ($search->{authtypecode}) { + $elastic_query->{bool}->{filter} = { + term => { + "authtype.raw" => $search->{authtypecode} + } + }; } - # add the sort stuff - $query->{sort} = [ $search->{sort} ] if exists $search->{sort}; + my $query = { + query => $elastic_query + }; + + # Add the sort stuff + $query->{sort} = [ $search->{sort} ] if exists $search->{sort}; return $query; } - =head2 build_authorities_query_compat my ($query) = @@ -464,8 +507,8 @@ sub build_authorities_query_compat { my %sort; my $sort_field = - ( $orderby =~ /^heading/ ) ? 'heading' - : ( $orderby =~ /^auth/ ) ? 'local-number' + ( $orderby =~ /^heading/ ) ? 'heading__sort' + : ( $orderby =~ /^auth/ ) ? 'local-number__sort' : undef; if ($sort_field) { my $sort_order = ( $orderby =~ /asc$/ ) ? 'asc' : 'desc'; @@ -532,7 +575,7 @@ types. =cut our %index_field_convert = ( - 'kw' => '_all', + 'kw' => '', 'ab' => 'abstract', 'au' => 'author', 'lcn' => 'local-classification', @@ -626,7 +669,7 @@ sub _convert_index_fields { # If a field starts with mc- we save it as it's used (and removed) later # when joining things, to indicate we make it an 'OR' join. # (Sorry, this got a bit ugly after special cases were found.) - grep { $_->{field} } map { + map { # Lower case all field names my ( $f, $t ) = map(lc, split /,/); my $mc = ''; @@ -639,7 +682,7 @@ sub _convert_index_fields { type => $index_type_convert{ $t // '__default' } }; $r->{field} = ($mc . $r->{field}) if $mc && $r->{field}; - $r; + $r->{field} ? $r : undef; } @indexes; } @@ -668,8 +711,8 @@ sub _convert_index_strings { push @res, $s; next; } - push @res, $conv->{field} . ":" - . $self->_modify_string_by_type( %$conv, operand => $term ); + push @res, ($conv->{field} ? $conv->{field} . ':' : '') + . $self->_modify_string_by_type( %$conv, operand => $term ); } return @res; } @@ -966,4 +1009,101 @@ sub _split_query { return @tokens; } +=head2 _search_fields + my $weighted_fields = $self->_search_fields({ + is_opac => 0, + weighted_fields => 1, + subfield => 'raw' + }); + +Generate a list of searchable fields to be used for Elasticsearch queries +applied to multiple fields. + +Returns an arrayref of field names for either OPAC or Staff client, with +possible weights and subfield appended to each field name depending on the +options provided. + +=over 4 + +=item C<$params> + +Hashref with options. The parameter C indicates whether the searchable +fields for OPAC or Staff client should be retrieved. If C is set +fields weights will be applied on returned fields. C can be used to +provide a subfield that will be appended to fields as "C.C". + +=back + +=cut + +sub _search_fields { + my ($self, $params) = @_; + $params //= { + is_opac => 0, + weighted_fields => 0, + # This is a hack for authorities build_authorities_query + # can hopefully be removed in the future + subfield => undef, + }; + + my $cache = Koha::Caches->get_instance(); + my $cache_key = 'elasticsearch_search_fields' . ($params->{is_opac} ? '_opac' : '_staff_client'); + my $search_fields = $cache->get_from_cache($cache_key, { unsafe => 1 }); + + if (!$search_fields) { + # The reason we don't use Koha::SearchFields->search here is we don't + # want or need resultset wrapped as Koha::SearchField object. + # It does not make any sense in this context and would cause + # unnecessary overhead sice we are only querying for data + # Also would not work, or produce strange results, with the "columns" + # option. + my $schema = Koha::Database->schema; + my $result = $schema->resultset('SearchField')->search( + { + $params->{is_opac} ? ( + 'opac' => 1, + ) : ( + 'staff_client' => 1 + ), + 'search_marc_map.index_name' => $self->index, + 'search_marc_map.marc_type' => C4::Context->preference('marcflavour'), + 'search_marc_to_fields.search' => 1, + }, + { + columns => [qw/name weight/], + collapse => 1, + join => {search_marc_to_fields => 'search_marc_map'}, + } + ); + my @search_fields; + while (my $search_field = $result->next) { + push @search_fields, [ + $search_field->name, + $search_field->weight ? $search_field->weight : () + ]; + } + $search_fields = \@search_fields; + $cache->set_in_cache($cache_key, $search_fields); + } + if ($params->{subfield}) { + my $subfield = $params->{subfield}; + $search_fields = [ + map { + # Copy values to avoid mutating cached + # data (since unsafe is used) + my ($field, $weight) = @{$_}; + ["${field}.${subfield}", $weight]; + } @{$search_fields} + ]; + } + + if ($params->{weighted_fields}) { + return [map { join('^', @{$_}) } @{$search_fields}]; + } + else { + # Exclude weight from field + return [map { $_->[0] } @{$search_fields}]; + } +} + 1; diff --git a/Koha/SearchField.pm b/Koha/SearchField.pm index efabb8db7d..bd19717b71 100644 --- a/Koha/SearchField.pm +++ b/Koha/SearchField.pm @@ -20,7 +20,6 @@ use Modern::Perl; use Carp; use Koha::Database; -use Koha::SearchMarcMaps; use base qw(Koha::Object); diff --git a/Koha/SearchFields.pm b/Koha/SearchFields.pm index b2c985f526..7af3c0b162 100644 --- a/Koha/SearchFields.pm +++ b/Koha/SearchFields.pm @@ -35,21 +35,6 @@ Koha::SearchFields - Koha SearchField Object set class =cut -=head3 weighted_fields - -my (@w_fields, @weight) = Koha::SearchFields->weighted_fields(); - -=cut - -sub weighted_fields { - my ($self) = @_; - - return $self->search( - { weight => { '>' => 0, '!=' => undef } }, - { order_by => { -desc => 'weight' } } - ); -} - =head3 type =cut diff --git a/admin/searchengine/elasticsearch/field_config.yaml b/admin/searchengine/elasticsearch/field_config.yaml index 73af9773e0..3494da45cf 100644 --- a/admin/searchengine/elasticsearch/field_config.yaml +++ b/admin/searchengine/elasticsearch/field_config.yaml @@ -1,9 +1,6 @@ --- # General field configuration general: - _all: - type: string - analyzer: analyser_standard properties: marc_data: store: true @@ -29,31 +26,30 @@ search: null_value: 0 stdno: type: text - analyzer: analyser_stdno - search_analyzer: analyser_stdno + analyzer: analyzer_stdno + search_analyzer: analyzer_stdno fields: phrase: type: text - analyzer: analyser_stdno - search_analyzer: analyser_stdno + analyzer: analyzer_phrase + search_analyzer: analyzer_phrase raw: type: keyword - copy_to: _all default: type: text - analyzer: analyser_standard - search_analyzer: analyser_standard + analyzer: analyzer_standard + search_analyzer: analyzer_standard fields: phrase: type: text - analyzer: analyser_phrase - search_analyzer: analyser_phrase + analyzer: analyzer_phrase + search_analyzer: analyzer_phrase raw: type: keyword - lc_raw: + normalizer: nfkc_cf_normalizer + ci_raw: type: keyword - normalizer: my_normalizer - copy_to: _all + normalizer: icu_folding_normalizer # Facets facet: default: @@ -67,9 +63,5 @@ suggestible: # Sort sort: default: - type: text - analyzer: analyser_phrase - search_analyzer: analyser_phrase - fields: - phrase: - type: keyword + type: icu_collation_keyword + index: false diff --git a/admin/searchengine/elasticsearch/index_config.yaml b/admin/searchengine/elasticsearch/index_config.yaml index 4b8627b862..7a8d9052b4 100644 --- a/admin/searchengine/elasticsearch/index_config.yaml +++ b/admin/searchengine/elasticsearch/index_config.yaml @@ -3,29 +3,29 @@ index: analysis: analyzer: - # Phrase analyzer is used for phrases (phrase match, sorting) - analyser_phrase: + # Phrase analyzer is used for phrases (exact phrase match) + analyzer_phrase: tokenizer: keyword filter: - icu_folding char_filter: - punctuation - analyser_standard: + analyzer_standard: tokenizer: icu_tokenizer filter: - icu_folding - analyser_stdno: + analyzer_stdno: tokenizer: whitespace filter: - icu_folding char_filter: - punctuation normalizer: - normalizer_keyword: + icu_folding_normalizer: type: custom filter: - icu_folding - my_normalizer: + nfkc_cf_normalizer: type: custom char_filter: icu_normalizer char_filter: diff --git a/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl index f91399aa9d..28802f6586 100755 --- a/admin/searchengine/elasticsearch/mappings.pl +++ b/admin/searchengine/elasticsearch/mappings.pl @@ -27,6 +27,7 @@ use Koha::SearchEngine::Elasticsearch; use Koha::SearchEngine::Elasticsearch::Indexer; use Koha::SearchMarcMaps; use Koha::SearchFields; +use Koha::Caches; use Try::Tiny; @@ -68,6 +69,12 @@ my $update_mappings = sub { } }; +my $cache = Koha::Caches->get_instance(); +my $clear_cache = sub { + $cache->clear_from_cache('elasticsearch_search_fields_staff_client'); + $cache->clear_from_cache('elasticsearch_search_fields_opac'); +}; + if ( $op eq 'edit' ) { $schema->storage->txn_begin; @@ -76,12 +83,15 @@ if ( $op eq 'edit' ) { my @field_label = $input->multi_param('search_field_label'); my @field_type = $input->multi_param('search_field_type'); my @field_weight = $input->multi_param('search_field_weight'); + my @field_staff_client = $input->multi_param('search_field_staff_client'); + my @field_opac = $input->multi_param('search_field_opac'); my @index_name = $input->multi_param('mapping_index_name'); - my @search_field_name = $input->multi_param('mapping_search_field_name'); + my @search_field_name = $input->multi_param('mapping_search_field_name'); my @mapping_sort = $input->multi_param('mapping_sort'); my @mapping_facet = $input->multi_param('mapping_facet'); my @mapping_suggestible = $input->multi_param('mapping_suggestible'); + my @mapping_search = $input->multi_param('mapping_search'); my @mapping_marc_field = $input->multi_param('mapping_marc_field'); my @faceted_field_names = $input->multi_param('display_facet'); @@ -92,6 +102,8 @@ if ( $op eq 'edit' ) { my $field_label = $field_label[$i]; my $field_type = $field_type[$i]; my $field_weight = $field_weight[$i]; + my $field_staff_client = $field_staff_client[$i]; + my $field_opac = $field_opac[$i]; my $search_field = Koha::SearchFields->find( { name => $field_name }, { key => 'name' } ); $search_field->label($field_label); @@ -106,6 +118,8 @@ if ( $op eq 'edit' ) { else { $search_field->weight($field_weight); } + $search_field->staff_client($field_staff_client ? 1 : 0); + $search_field->opac($field_opac ? 1 : 0); my $facet_order = first { $faceted_field_names[$_] eq $field_name } 0 .. $#faceted_field_names; $search_field->facet_order(defined $facet_order ? $facet_order + 1 : undef); @@ -118,19 +132,27 @@ if ( $op eq 'edit' ) { for my $i ( 0 .. scalar(@index_name) - 1 ) { my $index_name = $index_name[$i]; - my $search_field_name = $search_field_name[$i]; + my $search_field_name = $search_field_name[$i]; my $mapping_marc_field = $mapping_marc_field[$i]; my $mapping_facet = $mapping_facet[$i]; - my $mapping_suggestible = $mapping_suggestible[$i]; - my $mapping_sort = $mapping_sort[$i]; - $mapping_sort = undef if $mapping_sort eq 'undef'; $mapping_facet = ( grep {/^$search_field_name$/} @facetable_field_names ) ? $mapping_facet : 0; + my $mapping_suggestible = $mapping_suggestible[$i]; + my $mapping_sort = $mapping_sort[$i] eq 'undef' ? undef : $mapping_sort[$i]; + my $mapping_search = $mapping_search[$i]; my $search_field = Koha::SearchFields->find({ name => $search_field_name }, { key => 'name' }); # TODO Check mapping format - my $marc_field = Koha::SearchMarcMaps->find_or_create({ index_name => $index_name, marc_type => $marc_type, marc_field => $mapping_marc_field }); - $search_field->add_to_search_marc_maps($marc_field, { facet => $mapping_facet, suggestible => $mapping_suggestible, sort => $mapping_sort } ); - + my $marc_field = Koha::SearchMarcMaps->find_or_create({ + index_name => $index_name, + marc_type => $marc_type, + marc_field => $mapping_marc_field + }); + $search_field->add_to_search_marc_maps($marc_field, { + facet => $mapping_facet, + suggestible => $mapping_suggestible, + sort => $mapping_sort, + search => $mapping_search + }); } }; if ($@) { @@ -139,6 +161,7 @@ if ( $op eq 'edit' ) { } else { push @messages, { type => 'message', code => 'success_on_update' }; $schema->storage->txn_commit; + $clear_cache->(); $update_mappings->(); } } @@ -146,13 +169,13 @@ elsif( $op eq 'reset_confirmed' ) { Koha::SearchMarcMaps->delete; Koha::SearchFields->delete; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; + $clear_cache->(); push @messages, { type => 'message', code => 'success_on_reset' }; } elsif( $op eq 'reset_confirm' ) { $template->param( reset_confirm => 1 ); } - my @indexes; for my $index_name (@index_names) { @@ -179,29 +202,46 @@ for my $index_name (@index_names) { my @facetable_fields = Koha::SearchEngine::Elasticsearch->get_facetable_fields(); for my $index_name (@index_names) { my $search_fields = Koha::SearchFields->search( - { 'search_marc_map.index_name' => $index_name, 'search_marc_map.marc_type' => $marc_type, }, - { join => { search_marc_to_fields => 'search_marc_map' }, - '+select' => [ 'search_marc_to_fields.facet', 'search_marc_to_fields.suggestible', 'search_marc_to_fields.sort', 'search_marc_map.marc_field' ], - '+as' => [ 'facet', 'suggestible', 'sort', 'marc_field' ], + { + 'search_marc_map.index_name' => $index_name, + 'search_marc_map.marc_type' => $marc_type, + }, + { + join => { search_marc_to_fields => 'search_marc_map' }, + '+select' => [ + 'search_marc_to_fields.facet', + 'search_marc_to_fields.suggestible', + 'search_marc_to_fields.sort', + 'search_marc_to_fields.search', + 'search_marc_map.marc_field' + ], + '+as' => [ + 'facet', + 'suggestible', + 'sort', + 'search', + 'marc_field' + ], order_by => { -asc => [qw/name marc_field/] } - } - ); + } + ); my @mappings; my @facetable_field_names = map { $_->name } @facetable_fields; while ( my $s = $search_fields->next ) { my $name = $s->name; - push @mappings, - { search_field_name => $name, + push @mappings, { + search_field_name => $name, search_field_label => $s->label, search_field_type => $s->type, marc_field => $s->get_column('marc_field'), sort => $s->get_column('sort') // 'undef', # To avoid warnings "Use of uninitialized value in lc" suggestible => $s->get_column('suggestible'), + search => $s->get_column('search'), facet => $s->get_column('facet'), is_facetable => ( grep {/^$name$/} @facetable_field_names ) ? 1 : 0, - }; + }; } push @indexes, { index_name => $index_name, mappings => \@mappings }; diff --git a/catalogue/search.pl b/catalogue/search.pl index a3a7015efc..25ec0dbf08 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -463,11 +463,6 @@ my $page = $cgi->param('page') || 1; # Define some global variables my ( $error,$query,$simple_query,$query_cgi,$query_desc,$limit,$limit_cgi,$limit_desc,$query_type); -my $build_params; -unless ( $cgi->param('advsearch') ) { - $build_params->{weighted_fields} = 1; -} - my $builder = Koha::SearchEngine::QueryBuilder->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); my $searcher = Koha::SearchEngine::Search->new( @@ -480,7 +475,7 @@ my $searcher = Koha::SearchEngine::Search->new( $query_type ) = $builder->build_query_compat( \@operators, \@operands, \@indexes, \@limits, - \@sort_by, $scan, $lang, $build_params ); + \@sort_by, $scan, $lang, { weighted_fields => !$cgi->param('advsearch') }); ## parse the query_cgi string and put it into a form suitable for s my @query_inputs; diff --git a/installer/data/mysql/atomicupdate/bug_20589_add_columns.perl b/installer/data/mysql/atomicupdate/bug_20589_add_columns.perl new file mode 100644 index 0000000000..73ddd1becb --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20589_add_columns.perl @@ -0,0 +1,14 @@ +$DBversion = 'XXX'; +if (CheckVersion($DBversion)) { + if (!column_exists('search_marc_to_field', 'search')) { + $dbh->do("ALTER TABLE `search_marc_to_field` ADD COLUMN `search` tinyint(1) NOT NULL DEFAULT 1"); + } + if (!column_exists('search_field', 'staff_client')) { + $dbh->do("ALTER TABLE `search_field` ADD COLUMN `staff_client` tinyint(1) NOT NULL DEFAULT 1"); + } + if (!column_exists('search_field', 'opac')) { + $dbh->do("ALTER TABLE `search_field` ADD COLUMN `opac` tinyint(1) NOT NULL DEFAULT 1"); + } + SetVersion($DBversion) +} +print "Upgrade to $DBversion done (Bug 20589 - Add field boosting and use elastic query fields parameter instead of depricated _all)\n"; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt index 33df1fbd8a..e3a207db96 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt @@ -160,8 +160,15 @@ a.add, a.delete { Name Label Type + Searchable Weight + +   + Staff client + OPAC +   + [% FOREACH search_field IN all_search_fields %] @@ -169,7 +176,9 @@ a.add, a.delete { - + + + - [% IF search_field.mapped_biblios %] - - [% ELSE %] - - [% END %] + + + + + + + [% IF search_field.mapped_biblios %] + + [% ELSE %] + + [% END %] [% END %] @@ -231,6 +262,7 @@ a.add, a.delete { Sortable Facetable Suggestible + Searchable Mapping @@ -290,6 +322,17 @@ a.add, a.delete { + + + Delete @@ -335,6 +378,17 @@ a.add, a.delete { [% END %] + + + Add diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 27d1f2ce5c..a58cb8a2c5 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -551,18 +551,23 @@ if (C4::Context->preference('OpacSuppression')) { } } -my $build_params = { - suppress => $suppress -}; - -unless ( $cgi->param('advsearch') ) { - $build_params->{weighted_fields} = 1; -} - ## I. BUILD THE QUERY ( $error,$query,$simple_query,$query_cgi,$query_desc,$limit,$limit_cgi,$limit_desc,$query_type) - = $builder->build_query_compat( \@operators, \@operands, - \@indexes, \@limits, \@sort_by, 0, $lang, $build_params); + = $builder->build_query_compat( + \@operators, + \@operands, + \@indexes, + \@limits, + \@sort_by, + 0, + $lang, + { + expanded_facet => $expanded_facet, + suppress => $suppress, + is_opac => 1, + weighted_fields => !$cgi->param('advsearch') + } +); sub _input_cgi_parse { my @elements; diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index d3656eabc9..ac92eaba41 100644 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -23,6 +23,8 @@ use t::lib::Mocks; use t::lib::TestBuilder; use Test::More tests => 6; +use List::Util qw( all ); + use Koha::Database; use Koha::SearchEngine::Elasticsearch::QueryBuilder; @@ -81,8 +83,14 @@ $se->mock( 'get_elasticsearch_mappings', sub { return $all_mappings{$self->index}; }); +my $cache = Koha::Caches->get_instance(); +my $clear_search_fields_cache = sub { + $cache->clear_from_cache('elasticsearch_search_fields_staff_client'); + $cache->clear_from_cache('elasticsearch_search_fields_opac'); +}; + subtest 'build_authorities_query_compat() tests' => sub { - plan tests => 36; + plan tests => 46; my $qb; @@ -107,34 +115,40 @@ subtest 'build_authorities_query_compat() tests' => sub { $search_term = 'Donald Duck'; foreach my $koha_name ( keys %{ $koha_to_index_name } ) { my $query = $qb->build_authorities_query_compat( [ $koha_name ], undef, undef, ['contains'], [$search_term], 'AUTH_TYPE', 'asc' ); + is( $query->{query}->{bool}->{must}[0]->{query_string}->{query}, "(Donald*) AND (Duck*)" ); if ( $koha_name eq 'all' || $koha_name eq 'any' ) { - is( $query->{query}->{bool}->{must}[0]->{query_string}->{query}, - "(Donald*) AND (Duck*)"); + isa_ok( $query->{query}->{bool}->{must}[0]->{query_string}->{fields}, 'ARRAY') } else { - is( $query->{query}->{bool}->{must}[0]->{query_string}->{query}, - "(Donald*) AND (Duck*)"); + is( $query->{query}->{bool}->{must}[0]->{query_string}->{default_field}, $koha_to_index_name->{$koha_name} ); } } foreach my $koha_name ( keys %{ $koha_to_index_name } ) { my $query = $qb->build_authorities_query_compat( [ $koha_name ], undef, undef, ['is'], [$search_term], 'AUTH_TYPE', 'asc' ); if ( $koha_name eq 'all' || $koha_name eq 'any' ) { - is( $query->{query}->{bool}->{must}[0]->{match_phrase}->{"_all.phrase"}, - "donald duck"); + is( + $query->{query}->{bool}->{must}[0]->{multi_match}->{query}, + "Donald Duck" + ); + my $all_matches = all { /\.ci_raw$/ } + @{$query->{query}->{bool}->{must}[0]->{multi_match}->{fields}}; + ok( $all_matches, 'Correct fields parameter for "is" query in "any" or "all"' ); } else { - is( $query->{query}->{bool}->{must}[0]->{match_phrase}->{$koha_to_index_name->{$koha_name}.".phrase"}, - "donald duck"); + is( + $query->{query}->{bool}->{must}[0]->{term}->{$koha_to_index_name->{$koha_name} . ".ci_raw"}, + "Donald Duck" + ); } } foreach my $koha_name ( keys %{ $koha_to_index_name } ) { my $query = $qb->build_authorities_query_compat( [ $koha_name ], undef, undef, ['start'], [$search_term], 'AUTH_TYPE', 'asc' ); if ( $koha_name eq 'all' || $koha_name eq 'any' ) { - is( $query->{query}->{bool}->{must}[0]->{match_phrase_prefix}->{"_all.phrase"}, - "donald duck"); + my $all_matches = all { (%{$_->{prefix}})[0] =~ /\.ci_raw$/ && (%{$_->{prefix}})[1] eq "Donald Duck" } + @{$query->{query}->{bool}->{must}[0]->{bool}->{should}}; + ok( $all_matches, "Correct multiple prefix query" ); } else { - is( $query->{query}->{bool}->{must}[0]->{match_phrase_prefix}->{$koha_to_index_name->{$koha_name}.".phrase"}, - "donald duck"); + is( $query->{query}->{bool}->{must}[0]->{prefix}->{$koha_to_index_name->{$koha_name} . ".ci_raw"}, "Donald Duck" ); } } @@ -444,19 +458,21 @@ subtest 'build query from form subtests' => sub { }; subtest 'build_query with weighted fields tests' => sub { - plan tests => 4; + plan tests => 2; my $qb = Koha::SearchEngine::Elasticsearch::QueryBuilder->new( { index => 'mydb' } ); my $db_builder = t::lib::TestBuilder->new(); Koha::SearchFields->search({})->delete; + $clear_search_fields_cache->(); $db_builder->build({ source => 'SearchField', value => { name => 'acqdate', label => 'acqdate', - weight => undef + weight => undef, + staff_client => 1 } }); @@ -465,7 +481,8 @@ subtest 'build_query with weighted fields tests' => sub { value => { name => 'title', label => 'title', - weight => 25 + weight => 25, + staff_client => 1 } }); @@ -474,7 +491,8 @@ subtest 'build_query with weighted fields tests' => sub { value => { name => 'subject', label => 'subject', - weight => 15 + weight => 15, + staff_client => 1 } }); @@ -482,10 +500,12 @@ subtest 'build_query with weighted fields tests' => sub { undef, undef, undef, { weighted_fields => 1 }); my $fields = $query->{query}{query_string}{fields}; - is(scalar(@$fields), 3, 'Search is done on 3 fields'); - is($fields->[0], '_all', 'First search field is _all'); - is($fields->[1], 'title^25.00', 'Second search field is title'); - is($fields->[2], 'subject^15.00', 'Third search field is subject'); + + my @found = grep { $_ eq 'title^25.00' } @{$fields}; + is(@found, 1, 'Search field is title has correct weight'); # Fails + + @found = grep { $_ eq 'subject^15.00' } @{$fields}; + is(@found, 1, 'Search field subject has correct weight'); # Fails }; subtest "_convert_sort_fields() tests" => sub { -- 2.11.0