From 9cc9da4f922e2bc75b94aaf0050f522a918a171a Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Tue, 23 Jan 2018 15:21:31 +0200 Subject: [PATCH] Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. Improvements: 1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml. 2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml. 3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml. 4) Default settings have been improved to remove punctuation from phrases used for sorting etc. 5) State variables are used for storing configuration to avoid parsing it multiple times. 6) A possibility to reset the fields too has been added to the reset operation of mappings administration. 7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. 8) An stdno field type has been added for standard identifiers. To test: 1) Run tests in t/Koha/SearchEngine/Elasticsearch.t 2) Clear tables search_fields and search_marc_map 3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields. 5) Index some records 6) Verify that you can find the records 7) Put non_existent to koha-conf.xml 8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent. 9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it. 10) Make a change in the new mappings.yaml. 11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248) 12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 13) Verify that the changes you made are now visible in the mappings UI --- Koha/SearchEngine/Elasticsearch.pm | 209 +++++++++------------ debian/templates/koha-conf-site.xml.in | 11 ++ etc/koha-conf.xml | 16 ++ etc/searchengine/elasticsearch/field_config.yaml | 61 ++++++ etc/searchengine/elasticsearch/index_config.yaml | 34 ++++ .../searchengine/elasticsearch/mappings.yaml | 7 +- installer/data/mysql/atomicupdate/bug_20073.perl | 8 + installer/data/mysql/kohastructure.sql | 2 +- .../admin/searchengine/elasticsearch/mappings.tt | 14 +- t/Koha/SearchEngine/Elasticsearch.t | 26 ++- 10 files changed, 262 insertions(+), 126 deletions(-) create mode 100644 etc/searchengine/elasticsearch/field_config.yaml create mode 100644 etc/searchengine/elasticsearch/index_config.yaml rename {admin => etc}/searchengine/elasticsearch/mappings.yaml (99%) create mode 100644 installer/data/mysql/atomicupdate/bug_20073.perl diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index d859a39..13888e9 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -34,8 +34,6 @@ use Search::Elasticsearch; use Try::Tiny; use YAML::Syck; -use Data::Dumper; # TODO remove - __PACKAGE__->mk_ro_accessors(qw( index )); __PACKAGE__->mk_accessors(qw( sort_fields )); @@ -137,24 +135,14 @@ A hashref containing the settings is returned. sub get_elasticsearch_settings { my ($self) = @_; - # Ultimately this should come from a file or something, and not be - # hardcoded. - my $settings = { - index => { - analysis => { - analyzer => { - analyser_phrase => { - tokenizer => 'icu_tokenizer', - filter => ['icu_folding'], - }, - analyser_standard => { - tokenizer => 'icu_tokenizer', - filter => ['icu_folding'], - }, - }, - } - } - }; + # Use state to speed up repeated calls + state $settings = undef; + if (!defined $settings) { + my $config_file = C4::Context->config('elasticsearch_index_config'); + $config_file ||= C4::Context->config('intranetdir') . '/etc/searchengine/elasticsearch/index_config.yaml'; + $settings = LoadFile( $config_file ); + } + return $settings; } @@ -170,116 +158,97 @@ created. sub get_elasticsearch_mappings { my ($self) = @_; - # 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", - }, - } - } - }; - my %sort_fields; - my $marcflavour = lc C4::Context->preference('marcflavour'); - $self->_foreach_mapping( - sub { - my ( $name, $type, $facet, $suggestible, $sort, $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. - - # TODO be aware of date formats, but this requires pre-parsing - # as ES will simply reject anything with an invalid date. - my $es_type = - $type eq 'boolean' - ? 'boolean' - : 'text'; - - if ($es_type eq 'boolean') { - $mappings->{data}{properties}{$name} = _elasticsearch_mapping_for_boolean( $name, $es_type, $facet, $suggestible, $sort, $marc_type ); - return; #Boolean cannot have facets nor sorting nor suggestions - } else { - $mappings->{data}{properties}{$name} = _elasticsearch_mapping_for_default( $name, $es_type, $facet, $suggestible, $sort, $marc_type ); - } + # Use state to speed up repeated calls + state %all_mappings; + state %sort_fields; + + if (!defined $all_mappings{$self->index}) { + $sort_fields{$self->index} = {}; + my $mappings = { + data => _get_elasticsearch_mapping('general', '') + }; + my $marcflavour = lc C4::Context->preference('marcflavour'); + $self->_foreach_mapping( + sub { + my ( $name, $type, $facet, $suggestible, $sort, $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. + + # TODO be aware of date formats, but this requires pre-parsing + # as ES will simply reject anything with an invalid date. + my $es_type = 'text'; + if ($type eq 'boolean') { + $es_type = 'boolean'; + } elsif ($type eq 'number' || $type eq 'sum') { + $es_type = 'integer'; + } elsif ($type eq 'isbn' || $type eq 'stdno') { + $es_type = 'stdno'; + } - if ($facet) { - $mappings->{data}{properties}{ $name . '__facet' } = { - type => "keyword", - }; - } - if ($suggestible) { - $mappings->{data}{properties}{ $name . '__suggestion' } = { - type => 'completion', - analyzer => 'simple', - search_analyzer => 'simple', - }; - } - # Sort is a bit special as it can be true, false, undef. - # We care about "true" or "undef", - # "undef" means to do the default thing, which is make it sortable. - if ($sort || !defined $sort) { - $mappings->{data}{properties}{ $name . '__sort' } = { - search_analyzer => "analyser_phrase", - analyzer => "analyser_phrase", - type => "text", - include_in_all => JSON::false, - fields => { - phrase => { - type => "keyword", - }, - }, - }; - $sort_fields{$name} = 1; + $mappings->{data}{properties}{$name} = _get_elasticsearch_mapping('search', $es_type); + + if ($facet) { + $mappings->{data}{properties}{ $name . '__facet' } = _get_elasticsearch_mapping('facet', $es_type); + } + if ($suggestible) { + $mappings->{data}{properties}{ $name . '__suggestion' } = _get_elasticsearch_mapping('suggestible', $es_type); + } + # Sort is a bit special as it can be true, false, undef. + # We care about "true" or "undef", + # "undef" means to do the default thing, which is make it sortable. + if (!defined $sort || $sort) { + $mappings->{data}{properties}{ $name . '__sort' } = _get_elasticsearch_mapping('sort', $es_type); + $sort_fields{$self->index}{$name} = 1; + } } - } - ); - $self->sort_fields(\%sort_fields); - return $mappings; + ); + $all_mappings{$self->index} = $mappings; + } + $self->sort_fields(\%{$sort_fields{$self->index}}); + + return $all_mappings{$self->index}; } -=head2 _elasticsearch_mapping_for_* +=head2 _get_elasticsearch_mapping -Get the ES mappings for the given data type or a special mapping case +Get the ES mappings for the given purpose and data type -Receives the same parameters from the $self->_foreach_mapping() dispatcher +$mapping = _get_elasticsearch_mapping('search', 'text'); =cut -sub _elasticsearch_mapping_for_boolean { - my ( $name, $type, $facet, $suggestible, $sort, $marc_type ) = @_; +sub _get_elasticsearch_mapping { - return { - type => $type, - null_value => 0, - }; -} + my ( $purpose, $type ) = @_; -sub _elasticsearch_mapping_for_default { - my ( $name, $type, $facet, $suggestible, $sort, $marc_type ) = @_; - - return { - search_analyzer => "analyser_standard", - analyzer => "analyser_standard", - type => $type, - fields => { - phrase => { - search_analyzer => "analyser_phrase", - analyzer => "analyser_phrase", - type => "text", - }, - raw => { - type => "keyword", - } - }, - }; + # Use state to speed up repeated calls + state $settings = undef; + if (!defined $settings) { + my $config_file = C4::Context->config('elasticsearch_field_config'); + $config_file ||= C4::Context->config('intranetdir') . '/etc/searchengine/elasticsearch/field_config.yaml'; + $settings = LoadFile( $config_file ); + } + + if (!defined $settings->{$purpose}) { + die "Field purpose $purpose not defined in field config"; + } + if ($type eq '') { + return $settings->{$purpose}; + } + if (defined $settings->{$purpose}{$type}) { + return $settings->{$purpose}{$type}; + } + if (defined $settings->{$purpose}{'default'}) { + return $settings->{$purpose}{'default'}; + } + return undef; } sub reset_elasticsearch_mappings { - my $mappings_yaml = C4::Context->config('intranetdir') . '/admin/searchengine/elasticsearch/mappings.yaml'; + my ( $reset_fields ) = @_; + my $mappings_yaml = C4::Context->config('elasticsearch_index_mappings'); + $mappings_yaml ||= C4::Context->config('intranetdir') . '/etc/searchengine/elasticsearch/mappings.yaml'; my $indexes = LoadFile( $mappings_yaml ); while ( my ( $index_name, $fields ) = each %$indexes ) { @@ -349,10 +318,10 @@ sub get_fixer_rules { if ($type eq 'sum' ) { push @rules, "sum('$name')"; } - if ($self->sort_fields()->{$name}) { - if ($sort || !defined $sort) { - push @rules, "marc_map('$marc_field','${name}__sort.\$append', $options)"; - } + my $sort_fields = $self->sort_fields(); + if (defined $sort_fields->{$name} && $sort_fields->{$name}) { + push @rules, "marc_map('$marc_field','${name}__sort.\$append')"; + push @rules, "join_field('${name}__sort',' ')"; } } ); diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index 85b54a9..98a04b0 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -319,10 +319,21 @@ __END_SRU_PUBLICSERVER__ 50 2 + localhost:9200 koha___KOHASITE__ + + + + + + + + __ELASTICSEARCH_SERVERS__ + __ELASTICSEARCH_INDEX__ + + + + + + + + diff --git a/etc/searchengine/elasticsearch/field_config.yaml b/etc/searchengine/elasticsearch/field_config.yaml new file mode 100644 index 0000000..8d8e715 --- /dev/null +++ b/etc/searchengine/elasticsearch/field_config.yaml @@ -0,0 +1,61 @@ +--- +# General field configuration +general: + _all: + type: string + analyzer: analyser_standard + properties: + record: + store: true + type: text +# Search fields +search: + boolean: + type: boolean + null_value: false + integer: + type: integer + null_value: 0 + stdno: + type: text + analyzer: analyser_stdno + search_analyzer: analyser_stdno + fields: + phrase: + type: text + analyzer: analyser_stdno + search_analyzer: analyser_stdno + raw: + type: keyword + copy_to: _all + default: + type: text + analyzer: analyser_standard + search_analyzer: analyser_standard + fields: + phrase: + type: text + analyzer: analyser_phrase + search_analyzer: analyser_phrase + raw: + type: keyword + copy_to: all +# Facets +facet: + default: + type: keyword +# Suggestible +suggestible: + default: + type: completion + analyzer: simple + search_analyzer: simple +# Sort +sort: + default: + type: text + analyzer: analyser_phrase + search_analyzer: analyser_phrase + fields: + phrase: + type: keyword diff --git a/etc/searchengine/elasticsearch/index_config.yaml b/etc/searchengine/elasticsearch/index_config.yaml new file mode 100644 index 0000000..bdfcdf5 --- /dev/null +++ b/etc/searchengine/elasticsearch/index_config.yaml @@ -0,0 +1,34 @@ +--- +# Index configuration that defines how different analyzers work. +index: + analysis: + analyzer: + # Phrase analyzer is used for phrases (phrase match, sorting) + analyser_phrase: + tokenizer: keyword + filter: + - icu_folding + char_filter: + - punctuation + analyser_standard: + tokenizer: icu_tokenizer + filter: + - icu_folding + analyser_stdno: + tokenizer: whitespace + filter: + - icu_folding + char_filter: + - punctuation + normalizer: + normalizer_keyword: + type: custom + filter: + - icu_folding + char_filter: + # The punctuation filter is used to remove any punctuation chars in fields that don't use icu_tokenizer. + punctuation: + type: pattern_replace + # The pattern contains all ASCII punctuation characters. + pattern: '([\x00-\x1F,\x21-\x2F,\x3A-\x40,\x5B-\x60,\x7B-\x89,\x8B,\x8D,\x8F,\x90-\x99,\x9B,\x9D,\xA0-\xBF,\xD7,\xF7])' + replacement: '' diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/etc/searchengine/elasticsearch/mappings.yaml similarity index 99% rename from admin/searchengine/elasticsearch/mappings.yaml rename to etc/searchengine/elasticsearch/mappings.yaml index 25a1cc7..aff4152 100644 --- a/admin/searchengine/elasticsearch/mappings.yaml +++ b/etc/searchengine/elasticsearch/mappings.yaml @@ -1,4 +1,5 @@ --- +# Basic mappings from MARC fields to Elasticsearch fields. authorities: Corporate-name-see-also-from: label: Corporate-name-see-also-from @@ -1549,7 +1550,7 @@ biblios: marc_type: unimarc sort: ~ suggestible: '' - type: '' + type: 'stdno' isbn: label: isbn mappings: @@ -1568,7 +1569,7 @@ biblios: marc_type: unimarc sort: ~ suggestible: '' - type: '' + type: 'isbn' issn: label: issn mappings: @@ -1587,7 +1588,7 @@ biblios: marc_type: unimarc sort: ~ suggestible: '' - type: '' + type: 'stdno' issues: label: issues mappings: diff --git a/installer/data/mysql/atomicupdate/bug_20073.perl b/installer/data/mysql/atomicupdate/bug_20073.perl new file mode 100644 index 0000000..0fd4b45 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20073.perl @@ -0,0 +1,8 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( "ALTER TABLE search_field CHANGE COLUMN type type ENUM('', 'string', 'date', 'number', 'boolean', 'sum', 'isbn', 'stdno') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine'" ); + + # Always end with this (adjust the bug info) + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 20073 - Add new types for Elasticsearch fields)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 8151a29..c919c35 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1455,7 +1455,7 @@ CREATE TABLE `search_field` ( `id` int(11) NOT NULL AUTO_INCREMENT, `name` varchar(255) NOT NULL COMMENT 'the name of the field as it will be stored in the search engine', `label` varchar(255) NOT NULL COMMENT 'the human readable name of the field, for display', - `type` ENUM('', 'string', 'date', 'number', 'boolean', 'sum') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine', + `type` ENUM('', 'string', 'date', 'number', 'boolean', 'sum', 'isbn', 'stdno') NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine', PRIMARY KEY (`id`), UNIQUE KEY (`name` (191)) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; 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 24327c5..b6b6aa2 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 @@ -1,3 +1,5 @@ +[% USE Koha %] +[% SET KOHA_VERSION = Koha.Preference('Version') %] [% INCLUDE 'doc-head-open.inc' %] Koha › Administration › Elastic Search mappings [% INCLUDE 'doc-head-close.inc' %] @@ -88,7 +90,7 @@ a.add, a.delete {

Search engine configuration

- Warning: Any modification in these configurations will need a total reindexation to be fully taken into account ! + Warning: Any modification of this configuration requires a complete index rebuild to be fully taken into account!
[% IF errors %]
@@ -163,6 +165,16 @@ a.add, a.delete { [% ELSE %] [% END %] + [% IF search_field.type == "isbn" %] + + [% ELSE %] + + [% END %] + [% IF search_field.type == "stdno" %] + + [% ELSE %] + + [% END %] diff --git a/t/Koha/SearchEngine/Elasticsearch.t b/t/Koha/SearchEngine/Elasticsearch.t index f7a7f27..b91fcba 100644 --- a/t/Koha/SearchEngine/Elasticsearch.t +++ b/t/Koha/SearchEngine/Elasticsearch.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 1; +use Test::More tests => 3; use Test::Exception; use t::lib::Mocks; @@ -84,3 +84,27 @@ subtest '_read_configuration() tests' => sub { is( $configuration->{index_name}, 'index', 'Index configuration parsed correctly' ); is_deeply( $configuration->{nodes}, \@servers , 'Server configuration parsed correctly' ); }; + +subtest 'get_elasticsearch_settings() tests' => sub { + + plan tests => 1; + + my $settings; + + # test reading index settings + my $es = Koha::SearchEngine::Elasticsearch->new( {index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX} ); + $settings = $es->get_elasticsearch_settings(); + is( $settings->{index}{analysis}{analyzer}{analyser_phrase}{tokenizer}, 'keyword', 'Index settings parsed correctly' ); +}; + +subtest 'get_elasticsearch_mappings() tests' => sub { + + plan tests => 1; + + my $mappings; + + # test reading mappings + my $es = Koha::SearchEngine::Elasticsearch->new( {index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX} ); + $mappings = $es->get_elasticsearch_mappings(); + is( $mappings->{data}{_all}{type}, 'string', 'Field mappings parsed correctly' ); +}; -- 2.7.4