From be6a6c98363d9c48850993147cb5044b3df3abee 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. --- Koha/SearchEngine/Elasticsearch.pm | 208 +++++++++------------ admin/searchengine/elasticsearch/mappings.pl | 2 + debian/templates/koha-conf-site.xml.in | 11 ++ etc/koha-conf.xml | 109 +++++++++++ etc/searchengine/elasticsearch/field_config.yaml | 61 ++++++ etc/searchengine/elasticsearch/index_config.yaml | 34 ++++ .../searchengine/elasticsearch/mappings.yaml | 7 +- .../admin/searchengine/elasticsearch/mappings.tt | 2 +- t/Koha/SearchEngine/Elasticsearch.t | 36 +++- 9 files changed, 340 insertions(+), 130 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%) diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index d859a39..1fc1a59 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,96 @@ 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('intranetdir') . '/etc/searchengine/elasticsearch/mappings.yaml'; my $indexes = LoadFile( $mappings_yaml ); while ( my ( $index_name, $fields ) = each %$indexes ) { @@ -349,10 +317,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/admin/searchengine/elasticsearch/mappings.pl b/admin/searchengine/elasticsearch/mappings.pl index 7d63002..a1bb202 100755 --- a/admin/searchengine/elasticsearch/mappings.pl +++ b/admin/searchengine/elasticsearch/mappings.pl @@ -102,6 +102,8 @@ elsif( $op eq 'reset' ) { my $sure = $input->param('i_know_what_i_am_doing'); if ( $sure ) { Koha::SearchMarcMaps->search->delete; + my $reset_fields = $input->param('reset_fields'); + Koha::SearchFields->delete if ( $reset_fields ); Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; } } diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index 85b54a9..453dcd1 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__ + + + + + + + + @@ -171,5 +187,98 @@ __PAZPAR2_TOGGLE_XML_POST__ ILLLIBS + + + + http://__WEBSERVER_IP__:__WEBSERVER_PORT__ + + + http://__WEBSERVER_IP__:__WEBSERVER_PORT_LIBRARIAN__ + + + http://__WEBSERVER_IP__:__WEBSERVER_PORT_LIBRARIAN__ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/etc/searchengine/elasticsearch/field_config.yaml b/etc/searchengine/elasticsearch/field_config.yaml new file mode 100644 index 0000000..53b6410 --- /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..10aceb0 --- /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..81cc507 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/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 25535d9..4f406c0 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 @@ -88,7 +88,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 %]
diff --git a/t/Koha/SearchEngine/Elasticsearch.t b/t/Koha/SearchEngine/Elasticsearch.t index f7a7f27..b4084fc8 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; @@ -33,7 +33,7 @@ subtest '_read_configuration() tests' => sub { # 'elasticsearch' missing in configuration throws_ok { - $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration; + $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration(); } 'Koha::Exceptions::Config::MissingEntry', 'Configuration problem, exception thrown'; @@ -46,7 +46,7 @@ subtest '_read_configuration() tests' => sub { # 'elasticsearch' present but no 'server' entry t::lib::Mocks::mock_config( 'elasticsearch', {} ); throws_ok { - $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration; + $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration(); } 'Koha::Exceptions::Config::MissingEntry', 'Configuration problem, exception thrown'; @@ -59,7 +59,7 @@ subtest '_read_configuration() tests' => sub { # 'elasticsearch' and 'server' entries present, but no 'index_name' t::lib::Mocks::mock_config( 'elasticsearch', { server => 'a_server' } ); throws_ok { - $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration; + $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration(); } 'Koha::Exceptions::Config::MissingEntry', 'Configuration problem, exception thrown'; @@ -72,7 +72,7 @@ subtest '_read_configuration() tests' => sub { # Correct configuration, only one server t::lib::Mocks::mock_config( 'elasticsearch', { server => 'a_server', index_name => 'index' } ); - $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration; + $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration(); is( $configuration->{index_name}, 'index', 'Index configuration parsed correctly' ); is_deeply( $configuration->{nodes}, ['a_server'], 'Server configuration parsed correctly' ); @@ -80,7 +80,31 @@ subtest '_read_configuration() tests' => sub { my @servers = ('a_server', 'another_server'); t::lib::Mocks::mock_config( 'elasticsearch', { server => \@servers, index_name => 'index' } ); - $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration; + $configuration = Koha::SearchEngine::Elasticsearch::_read_configuration(); 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