From 1da0237b527125bdf37828daaa134f8ff889a4d6 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) No reindex is really required, but still, reindex authorities and biblios to verify that this still works. 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 | 6 + Koha/Schema/Result/SearchMarcToField.pm | 2 + Koha/SearchEngine/Elasticsearch.pm | 9 +- Koha/SearchEngine/Elasticsearch/QueryBuilder.pm | 146 +++++++++++++++++---- admin/searchengine/elasticsearch/mappings.pl | 74 ++++++++--- installer/data/mysql/atomicupdate/bug_xxxxx.sql | 13 ++ .../admin/searchengine/elasticsearch/mappings.tt | 60 ++++++++- opac/opac-search.pl | 2 +- 8 files changed, 263 insertions(+), 49 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_xxxxx.sql diff --git a/Koha/Schema/Result/SearchField.pm b/Koha/Schema/Result/SearchField.pm index ee32f42cf2..ac4f2aa073 100644 --- a/Koha/Schema/Result/SearchField.pm +++ b/Koha/Schema/Result/SearchField.pm @@ -68,6 +68,12 @@ __PACKAGE__->add_columns( extra => { list => ["", "string", "date", "number", "boolean", "sum"] }, is_nullable => 0, }, + "boost", + { data_type => "decimal", size => [ 5, 2 ], 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 d859a39362..c747faa9e6 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -173,11 +173,9 @@ sub get_elasticsearch_mappings { # TODO cache in the object? my $mappings = { data => { - _all => {type => "string", analyzer => "analyser_standard"}, properties => { record => { store => "true", - include_in_all => JSON::false, type => "text", }, } @@ -226,7 +224,6 @@ sub get_elasticsearch_mappings { search_analyzer => "analyser_phrase", analyzer => "analyser_phrase", type => "text", - include_in_all => JSON::false, fields => { phrase => { type => "keyword", @@ -286,11 +283,13 @@ sub reset_elasticsearch_mappings { while ( my ( $field_name, $data ) = each %$fields ) { my $field_type = $data->{type}; my $field_label = $data->{label}; + my $staff_client = exists $data->{staff_client} ? $data->{staff_client} : 1; + my $opac = exists $data->{opac} ? $data->{opac} : 1; my $mappings = $data->{mappings}; - my $search_field = Koha::SearchFields->find_or_create({ name => $field_name, label => $field_label, type => $field_type }, { key => 'name' }); + my $search_field = Koha::SearchFields->find_or_create({ name => $field_name, label => $field_label, type => $field_type, staff_client => $staff_client, opac => $opac }, { key => 'name' }); 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} } ); + $search_field->add_to_search_marc_maps($marc_field, { facet => $mapping->{facet} || 0, suggestible => $mapping->{suggestible} || 0, sort => $mapping->{sort}, search => $mapping->{search} || 1 } ); } } } diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 95cc86ab78..2cf59ce456 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->_query_string_fields({ is_opac => $options{is_opac} }), lenient => JSON::true, } }; @@ -233,6 +234,7 @@ sub build_query_compat { my %options; $options{sort} = \@sort_params; $options{expanded_facet} = $params->{expanded_facet}; + $options{is_opac} = exists $params->{is_opac} ? $params->{is_opac} : 0; my $query = $self->build_query( $query_str, %options ); #die Dumper($query); @@ -293,16 +295,55 @@ sub build_authorities_query { my @filter_parts; 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 '=' ) { - + if (!$wh) { + my %query_string_params = ( + default_operator => 'AND', + ); + if ( $op eq 'is' || $op eq '=' ) { + push @query_parts, { + multi_match => { + query => $val, + fields => $self->_query_string_fields({ subfield => 'raw' }), + } + }; + } + elsif ( $op eq 'exact' ) { + # left and right truncation, otherwise an exact phrase + push @query_parts, { + query_string => { + query => qq("$val"), + fields => $self->_query_string_fields({ subfield => 'phrase' }), + %query_string_params, + } + }; + } + elsif ( $op eq 'start') { + push @query_parts, { + query_string => { + query => "$val*", + $self->_query_string_fields({ subfield => 'phrase' }), + %query_string_params, + } + }; + } + else { + push @query_parts, { + query_string => { + query => $val, + fields => $self->_query_string_fields(), + %query_string_params, + } + }; + } + } + elsif ( $op eq 'is' || $op eq '=' ) { # look for something that matches completely # note, '=' is about numerical vals. May need special handling. - # _allphrase is a special field that only groups the exact + # raw is a special subfield that only groups the exact # matches. 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 @filter_parts, { term => { "$wh.phrase" => lc $val } }; + push @query_parts, { term => { "$wh.raw" => lc $val } }; } elsif ( $op eq 'exact' ) { @@ -320,13 +361,10 @@ sub build_authorities_query { } } - # Merge the query and filter parts appropriately - # 'should' behaves like 'or', if we want 'and', use 'must' - my $query_part = { bool => { should => \@query_parts } }; - my $filter_part = { bool => { should => \@filter_parts } }; - # We need to add '.phrase' to all the sort headings otherwise it'll sort # based on the tokenised form. + # TODO: .phrase currently contains the tokenized form, so this will not work + # instead .raw should be used? my %s; if ( exists $search->{sort} ) { foreach my $k ( keys %{ $search->{sort} } ) { @@ -339,21 +377,24 @@ sub build_authorities_query { # extract the sort stuff my %sort; %sort = ( sort => [ $search->{sort} ] ) if exists $search->{sort}; - my $query; + + # Merge the query and filter parts appropriately + # 'should' behaves like 'or', if we want 'and', use 'must' + my $elastic_query = {}; if (@filter_parts) { - $query = - { query => - { filtered => { filter => $filter_part, query => $query_part } } - }; - } - else { - $query = { query => $query_part }; + $elastic_query->{bool}->{filter} = { + bool => { + must => \@filter_parts, + } + }; } - $query = { %$query, %sort }; - return $query; + if (@query_parts) { + $elastic_query->{bool}->{should} = \@query_parts; + }; + my $params = { query => $elastic_query }; + return { %$params, %sort }; } - =head2 build_authorities_query_compat my ($query) = @@ -513,7 +554,7 @@ types. =cut our %index_field_convert = ( - 'kw' => '_all', + 'kw' => '', 'ti' => 'title', 'au' => 'author', 'su' => 'subject', @@ -539,7 +580,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 { my ( $f, $t ) = split /,/; my $mc = ''; if ($f =~ /^mc-/) { @@ -551,7 +592,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; } @@ -580,8 +621,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; } @@ -813,4 +854,55 @@ sub _truncate_terms { return join ' ', @terms; } +sub _query_string_fields { + my ($self, $params) = @_; + $params //= { + is_opac => 0, + # This is a terrible 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) { + 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, + }, + { + 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->boost ? '^' . $search_field->boost : ''); + } + $search_fields = \@search_fields; + $cache->set_in_cache($cache_key, $search_fields); + } + if ($params->{subfield}) { + my $subfield = $params->{subfield}; + $search_fields = [ + map { + my ($field, $boost) = $_ =~ /^([^\^]+)(?:\^(.+))?$/; + "${field}.${subfield}" . ($boost ? "^$boost" : ''); + } @{$search_fields} + ]; + } + return $search_fields; +} + 1; diff --git a/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl index 7d63002919..72f0fa200a 100755 --- a/admin/searchengine/elasticsearch/mappings.pl +++ b/admin/searchengine/elasticsearch/mappings.pl @@ -16,6 +16,7 @@ # along with Koha; if not, see . use Modern::Perl; +use Scalar::Util qw(looks_like_number); use CGI; use C4::Koha; use C4::Output; @@ -24,6 +25,7 @@ use C4::Auth; use Koha::SearchEngine::Elasticsearch; use Koha::SearchMarcMaps; use Koha::SearchFields; +use Koha::Caches; my $input = new CGI; my ( $template, $borrowernumber, $cookie ) = get_template_and_user( @@ -44,6 +46,12 @@ my $schema = $database->schema; my $marc_type = lc C4::Context->preference('marcflavour'); +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; @@ -51,12 +59,16 @@ if ( $op eq 'edit' ) { my @field_name = $input->param('search_field_name'); my @field_label = $input->param('search_field_label'); my @field_type = $input->param('search_field_type'); + my @field_boost = $input->param('search_field_boost'); + my @field_staff_client = $input->param('search_field_staff_client'); + my @field_opac = $input->param('search_field_opac'); my @index_name = $input->param('mapping_index_name'); - my @search_field_name = $input->param('mapping_search_field_name'); + my @search_field_name = $input->param('mapping_search_field_name'); my @mapping_sort = $input->param('mapping_sort'); my @mapping_facet = $input->param('mapping_facet'); my @mapping_suggestible = $input->param('mapping_suggestible'); + my @mapping_search = $input->param('mapping_search'); my @mapping_marc_field = $input->param('mapping_marc_field'); eval { @@ -65,9 +77,15 @@ if ( $op eq 'edit' ) { my $field_name = $field_name[$i]; my $field_label = $field_label[$i]; my $field_type = $field_type[$i]; + my $field_boost = $field_boost[$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); $search_field->type($field_type); + $search_field->boost($field_boost) if looks_like_number($field_boost); + $search_field->staff_client($field_staff_client ? 1 : 0); + $search_field->opac($field_opac ? 1 : 0); $search_field->store; } @@ -75,18 +93,26 @@ 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'; + 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 ($@) { @@ -96,6 +122,7 @@ if ( $op eq 'edit' ) { push @messages, { type => 'message', code => 'success_on_update' }; $schema->storage->txn_commit; } + $clear_cache->(); } elsif( $op eq 'reset' ) { # TODO Move this feature to the interface @@ -104,32 +131,49 @@ elsif( $op eq 'reset' ) { Koha::SearchMarcMaps->search->delete; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; } + $clear_cache->(); } my @indexes; for my $index_name (qw| biblios authorities |) { 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', + ], } ); my @mappings; while ( my $s = $search_fields->next ) { - push @mappings, - { search_field_name => $s->name, + push @mappings, { + search_field_name => $s->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'), facet => $s->get_column('facet'), - }; + search => $s->get_column('search'), + }; } - push @indexes, { index_name => $index_name, mappings => \@mappings }; } diff --git a/installer/data/mysql/atomicupdate/bug_xxxxx.sql b/installer/data/mysql/atomicupdate/bug_xxxxx.sql new file mode 100644 index 0000000000..50d60781e4 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_xxxxx.sql @@ -0,0 +1,13 @@ +DELIMITER // +CREATE PROCEDURE `alter_if_not_exists`() +BEGIN + IF NOT EXISTS(SELECT NULL FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = 'search_marc_to_field' AND table_schema = DATABASE() AND column_name = 'search') THEN + ALTER TABLE `search_marc_to_field` ADD COLUMN `search` tinyint(1) NOT NULL DEFAULT 1; + ALTER TABLE `search_field` ADD COLUMN `boost` decimal(5,2) DEFAULT NULL; + ALTER TABLE `search_field` ADD COLUMN `staff_client` tinyint(1) NOT NULL DEFAULT 1; + ALTER TABLE `search_field` ADD COLUMN `opac` tinyint(1) NOT NULL DEFAULT 1; + END IF; +END // +DELIMITER ; +CALL alter_if_not_exists(); +DROP PROCEDURE `alter_if_not_exists`; 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 25535d9c2c..78558cc7f1 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 @@ -126,6 +126,14 @@ a.add, a.delete { Name Label Type + Searchable + Boost + + +   + Staff client + OPAC +   @@ -134,7 +142,9 @@ a.add, a.delete { - + + + + + + + + + + + + [% END %] @@ -179,6 +214,7 @@ a.add, a.delete { Sortable Facetable Suggestible + Searchable Mapping @@ -233,6 +269,17 @@ a.add, a.delete { + + + Delete @@ -278,6 +325,17 @@ a.add, a.delete { [% END %] + + + Add diff --git a/opac/opac-search.pl b/opac/opac-search.pl index ccde617022..bc3376cb4c 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -555,7 +555,7 @@ if (C4::Context->preference('OpacSuppression')) { \@sort_by, 0, $lang, - { expanded_facet => $expanded_facet, suppress => $suppress } + { expanded_facet => $expanded_facet, suppress => $suppress, is_opac => 1 } ); sub _input_cgi_parse { -- 2.11.0