From 4be093dc0291f10611aea262bae35d9c04df2f7d Mon Sep 17 00:00:00 2001 From: Kevin Carnes Date: Tue, 8 Feb 2022 14:14:11 +0100 Subject: [PATCH] Bug 25669: ElasticSearch 6: [types removal] Specifying types in put mapping requests is deprecated (incompatible with 7) Starting with version 7.0 of Elasticsearch type names should not be used and in version 6.8 there is a warning if you don't use include_type_name when using put_mapping (requires Search::Elasticsearch@6.8). To test: 1) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 2) Observe any deprecation warnings or errors about types 3) Apply patch 4) Install Search::Elasticsearch with the same version as Elasticsearch 5) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t 6) Observe no warning or errors about types 7) Sign off Sponsored-by: Lund University Library --- Koha/SearchEngine/Elasticsearch.pm | 15 ++-- Koha/SearchEngine/Elasticsearch/Indexer.pm | 54 +++++++----- .../Elasticsearch/QueryBuilder.pm | 4 +- .../Koha/SearchEngine/Elasticsearch.t | 2 +- .../SearchEngine/Elasticsearch/QueryBuilder.t | 86 +++++++++---------- .../Koha/SearchEngine/Elasticsearch/Search.t | 38 ++++---- 6 files changed, 102 insertions(+), 97 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index 203183138f..120f11e819 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -104,6 +104,7 @@ sub get_elasticsearch { $self->{elasticsearch} = Search::Elasticsearch->new( $self->get_elasticsearch_params() ); + $self->{es_version} = $self->{elasticsearch}->info->{version}{number}; } return $self->{elasticsearch}; } @@ -189,9 +190,7 @@ sub get_elasticsearch_mappings { if (!defined $all_mappings{$self->index}) { $sort_fields{$self->index} = {}; # Clone the general mapping to break ties with the original hash - my $mappings = { - data => clone(_get_elasticsearch_field_config('general', '')) - }; + my $mappings = clone(_get_elasticsearch_field_config('general', '')); my $marcflavour = lc C4::Context->preference('marcflavour'); $self->_foreach_mapping( sub { @@ -214,25 +213,25 @@ sub get_elasticsearch_mappings { } if ($search) { - $mappings->{data}{properties}{$name} = _get_elasticsearch_field_config('search', $es_type); + $mappings->{properties}{$name} = _get_elasticsearch_field_config('search', $es_type); } if ($facet) { - $mappings->{data}{properties}{ $name . '__facet' } = _get_elasticsearch_field_config('facet', $es_type); + $mappings->{properties}{ $name . '__facet' } = _get_elasticsearch_field_config('facet', $es_type); } if ($suggestible) { - $mappings->{data}{properties}{ $name . '__suggestion' } = _get_elasticsearch_field_config('suggestible', $es_type); + $mappings->{properties}{ $name . '__suggestion' } = _get_elasticsearch_field_config('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_field_config('sort', $es_type); + $mappings->{properties}{ $name . '__sort' } = _get_elasticsearch_field_config('sort', $es_type); $sort_fields{$self->index}{$name} = 1; } } ); - $mappings->{data}{properties}{ 'match-heading' } = _get_elasticsearch_field_config('search', 'text') if $self->index eq 'authorities'; + $mappings->{properties}{ 'match-heading' } = _get_elasticsearch_field_config('search', 'text') if $self->index eq 'authorities'; $all_mappings{$self->index} = $mappings; } $self->sort_fields(\%{$sort_fields{$self->index}}); diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm index 0a10832d2e..23d34f4218 100644 --- a/Koha/SearchEngine/Elasticsearch/Indexer.pm +++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm @@ -21,6 +21,7 @@ use Carp qw( carp croak ); use Modern::Perl; use Try::Tiny qw( catch try ); use List::Util qw( any ); +use version; use base qw(Koha::SearchEngine::Elasticsearch); use Koha::Exceptions; @@ -115,11 +116,14 @@ sub update_index { if (@body) { try{ my $elasticsearch = $self->get_elasticsearch(); - $response = $elasticsearch->bulk( + my %es_bulk = ( index => $self->index_name, - type => 'data', # is just hard coded in Indexer.pm? body => \@body ); + if (version->parse($self->{es_version}) < version->parse('7.0.0')) { + $es_bulk{'type'} = 'data'; + } + $response = $elasticsearch->bulk(%es_bulk); if ($response->{errors}) { carp "One or more ElasticSearch errors occurred when indexing documents"; } @@ -250,24 +254,27 @@ sub update_mappings { my $elasticsearch = $self->get_elasticsearch(); my $mappings = $self->get_elasticsearch_mappings(); - foreach my $type (keys %{$mappings}) { - try { - my $response = $elasticsearch->indices->put_mapping( - index => $self->index_name, - type => $type, - body => { - $type => $mappings->{$type} - } - ); - } catch { - $self->set_index_status_recreate_required(); - my $reason = $_[0]->{vars}->{body}->{error}->{reason}; - my $index_name = $self->index_name; - Koha::Exceptions::Exception->throw( - error => "Unable to update mappings for index \"$index_name\". Reason was: \"$reason\". Index needs to be recreated and reindexed", - ); - }; - } + try { + my %es_mapping = ( + index => $self->index_name, + body => $mappings + ); + if (version->parse($self->{es_version}) < version->parse('7.0.0')) { + if (version->parse($self->{es_version}) >= version->parse('6.8.0') && version->parse($elasticsearch->VERSION) >= version->parse('6.8.0')) { + $es_mapping{'include_type_name'} = \1; + } + $es_mapping{'type'} = 'data'; + $es_mapping{'body'} = {'data' => $mappings}; + } + my $response = $elasticsearch->indices->put_mapping(%es_mapping); + } catch { + $self->set_index_status_recreate_required(); + my $reason = $_[0]->{vars}->{body}->{error}->{reason}; + my $index_name = $self->index_name; + Koha::Exceptions::Exception->throw( + error => "Unable to update mappings for index \"$index_name\". Reason was: \"$reason\". Index needs to be recreated and reindexed", + ); + }; $self->set_index_status_ok(); } @@ -346,11 +353,14 @@ sub delete_index { my $elasticsearch = $self->get_elasticsearch(); my @body = map { { delete => { _id => "$_" } } } @{$biblionums}; - my $result = $elasticsearch->bulk( + my %es_bulk = ( index => $self->index_name, - type => 'data', body => \@body, ); + if (version->parse($self->{es_version}) < version->parse('7.0.0')) { + $es_bulk{'type'} = 'data'; + } + my $result = $elasticsearch->bulk(%es_bulk); if ($result->{errors}) { croak "An Elasticsearch error occurred during bulk delete"; } diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index 607634c405..52cd31b7d9 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -577,7 +577,7 @@ sub build_authorities_query_compat { $m = exists $koha_to_index_name->{$m} ? $koha_to_index_name->{$m} : $m; push @indexes, $m; - warn "Unknown search field $m in marclist" unless (defined $mappings->{data}->{properties}->{$m} || $m eq '' || $m eq 'match-heading'); + warn "Unknown search field $m in marclist" unless (defined $mappings->{properties}->{$m} || $m eq '' || $m eq 'match-heading'); } for ( my $i = 0 ; $i < @$value ; $i++ ) { next unless $value->[$i]; #clean empty form values, ES doesn't like undefined searches @@ -1123,7 +1123,7 @@ sub _sort_field { my ($self, $f) = @_; my $mappings = $self->get_elasticsearch_mappings(); - my $textField = defined $mappings->{data}{properties}{$f}{type} && $mappings->{data}{properties}{$f}{type} eq 'text'; + my $textField = defined $mappings->{properties}{$f}{type} && $mappings->{properties}{$f}{type} eq 'text'; if (!defined $self->sort_fields()->{$f} || $self->sort_fields()->{$f}) { $f .= '__sort'; } else { diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t index f29831f504..51c1430942 100755 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t @@ -135,7 +135,7 @@ subtest 'get_elasticsearch_mappings() tests' => sub { # test reading mappings my $es = Koha::SearchEngine::Elasticsearch->new( {index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX} ); $mappings = $es->get_elasticsearch_mappings(); - is( $mappings->{data}{properties}{isbn__sort}{index}, 'false', 'Field mappings parsed correctly' ); + is( $mappings->{properties}{isbn__sort}{index}, 'false', 'Field mappings parsed correctly' ); }; subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' => sub { diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index 2991ee39b6..b5e2ebdf5f 100755 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -39,50 +39,48 @@ $se->mock( 'get_elasticsearch_mappings', sub { my %all_mappings; my $mappings = { - data => { - properties => { - title => { - type => 'text' - }, - title__sort => { - type => 'text' - }, - subject => { - type => 'text', - facet => 1 - }, - 'subject-heading-thesaurus' => { - type => 'text', - facet => 1 - }, - itemnumber => { - type => 'integer' - }, - sortablenumber => { - type => 'integer' - }, - sortablenumber__sort => { - type => 'integer' - }, - heading => { - type => 'text' - }, - 'heading-main' => { - type => 'text' - }, - heading__sort => { - type => 'text' - }, - match => { - type => 'text' - }, - 'match-heading' => { - type => 'text' - }, - 'match-heading-see-from' => { - type => 'text' - }, - } + properties => { + title => { + type => 'text' + }, + title__sort => { + type => 'text' + }, + subject => { + type => 'text', + facet => 1 + }, + 'subject-heading-thesaurus' => { + type => 'text', + facet => 1 + }, + itemnumber => { + type => 'integer' + }, + sortablenumber => { + type => 'integer' + }, + sortablenumber__sort => { + type => 'integer' + }, + heading => { + type => 'text' + }, + 'heading-main' => { + type => 'text' + }, + heading__sort => { + type => 'text' + }, + match => { + type => 'text' + }, + 'match-heading' => { + type => 'text' + }, + 'match-heading-see-from' => { + type => 'text' + }, } }; $all_mappings{$self->index} = $mappings; diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t index e7d3b2ae62..59eabeb778 100755 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t @@ -31,26 +31,24 @@ $se->mock( 'get_elasticsearch_mappings', sub { my %all_mappings; my $mappings = { - data => { - properties => { - title => { - type => 'text' - }, - title__sort => { - type => 'text' - }, - subject => { - type => 'text' - }, - itemnumber => { - type => 'integer' - }, - sortablenumber => { - type => 'integer' - }, - sortablenumber__sort => { - type => 'integer' - } + properties => { + title => { + type => 'text' + }, + title__sort => { + type => 'text' + }, + subject => { + type => 'text' + }, + itemnumber => { + type => 'integer' + }, + sortablenumber => { + type => 'integer' + }, + sortablenumber__sort => { + type => 'integer' } } }; -- 2.17.1