Attachment 131037 Details for Bug 25669 – Bug 25669: ElasticSearch 6: [types removal] Specifying types in put mapping requests is deprecated (incompatible with 7)

[patch] Bug 25669: ElasticSearch 6: [types removal] Specifying types in put mapping requests is deprecated (incompatible with 7)

Bug-25669-ElasticSearch-6-types-removal-Specifying.patch (text/plain), 15.31 KB, created by Kevin Carnes on 2022-02-23 14:45:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kevin Carnes

Created: 2022-02-23 14:45:54 UTC

Size: 15.31 KB

patch

obsolete

>From d3770302f7d1f682ce54289a3417ef5dd711aff8 Mon Sep 17 00:00:00 2001
>From: Kevin Carnes <kevin.carnes@ub.lu.se>
>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) If testing with ES7, enable elasticsearch_no_type in koha-conf.xml
>6) Run prove t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
>7) Observe no warning or errors about types
>8) Sign off
>
>Sponsored-by: Lund University Library
>---
> Koha/SearchEngine/Elasticsearch.pm            | 16 ++--
> Koha/SearchEngine/Elasticsearch/Indexer.pm    | 69 ++++++++++-----
> .../Elasticsearch/QueryBuilder.pm             |  4 +-
> etc/koha-conf.xml                             |  2 +
> .../Koha/SearchEngine/Elasticsearch.t         |  2 +-
> .../SearchEngine/Elasticsearch/QueryBuilder.t | 86 +++++++++----------
> .../Koha/SearchEngine/Elasticsearch/Search.t  | 38 ++++----
> 7 files changed, 119 insertions(+), 98 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
>index 203183138f..c8ed15b0d7 100644
>--- a/Koha/SearchEngine/Elasticsearch.pm
>+++ b/Koha/SearchEngine/Elasticsearch.pm
>@@ -104,6 +104,8 @@ sub get_elasticsearch {
>         $self->{elasticsearch} = Search::Elasticsearch->new(
>             $self->get_elasticsearch_params()
>         );
>+        # TODO Remove no_type when support for Elasticsearch 6 is dropped
>+        $self->{no_type} = C4::Context->config('elasticsearch_no_type');
>     }
>     return $self->{elasticsearch};
> }
>@@ -189,9 +191,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 +214,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..79ee52b66c 100644
>--- a/Koha/SearchEngine/Elasticsearch/Indexer.pm
>+++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm
>@@ -115,11 +115,20 @@ sub update_index {
>     if (@body) {
>         try{
>             my $elasticsearch = $self->get_elasticsearch();
>-            $response = $elasticsearch->bulk(
>-                index => $self->index_name,
>-                type => 'data', # is just hard coded in Indexer.pm?
>-                body => \@body
>-            );
>+            #TODO Remove check for no_type and use bulk without type when support for Elasticsearch 6 is dropped
>+            if ($self->{no_type}) {
>+                $response = $elasticsearch->bulk(
>+                    index => $self->index_name,
>+                    body => \@body
>+                );
>+            }
>+            else {
>+                $response = $elasticsearch->bulk(
>+                    index => $self->index_name,
>+                    type => 'data',
>+                    body => \@body
>+                );
>+            }
>             if ($response->{errors}) {
>                 carp "One or more ElasticSearch errors occurred when indexing documents";
>             }
>@@ -250,24 +259,31 @@ sub update_mappings {
>     my $elasticsearch = $self->get_elasticsearch();
>     my $mappings = $self->get_elasticsearch_mappings();
> 
>-    foreach my $type (keys %{$mappings}) {
>-        try {
>+    try {
>+        #TODO Remove check for no_type and use put_mapping without type when support for Elasticsearch 6 is dropped
>+        if ($self->{no_type}) {
>+            my $response = $elasticsearch->indices->put_mapping(
>+                index => $self->index_name,
>+                body => $mappings
>+            );
>+        }
>+        else {
>             my $response = $elasticsearch->indices->put_mapping(
>                 index => $self->index_name,
>-                type => $type,
>+                type => 'data',
>                 body => {
>-                    $type => $mappings->{$type}
>+                    data => $mappings
>                 }
>             );
>-        } 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",
>-            );
>-        };
>-    }
>+        }
>+    } 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 +362,18 @@ sub delete_index {
> 
>     my $elasticsearch = $self->get_elasticsearch();
>     my @body = map { { delete => { _id => "$_" } } } @{$biblionums};
>-    my $result = $elasticsearch->bulk(
>-        index => $self->index_name,
>-        type => 'data',
>-        body => \@body,
>-    );
>+    #TODO Remove check for no_type and use bulk without type when support for Elasticsearch 6 is dropped
>+    my $result = $self->{no_type} ?
>+        $elasticsearch->bulk(
>+            index => $self->index_name,
>+            body => \@body,
>+        )
>+        :
>+        $elasticsearch->bulk(
>+            index => $self->index_name,
>+            type => 'data',
>+            body => \@body,
>+        );
>     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/etc/koha-conf.xml b/etc/koha-conf.xml
>index 118b6b8207..7f5d82040b 100644
>--- a/etc/koha-conf.xml
>+++ b/etc/koha-conf.xml
>@@ -177,6 +177,8 @@
>       Resetting mappings will override any changes made in the Search engine configuration UI.
>  -->
>  
>+ 
>+ 
> 
>  <interlibrary_loans>
>      <!-- Path to where Illbackends are located on the system
>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

Actions: View | Diff | Splinter Review

Attachments on bug 25669: 130288 | 130293 | 130341 | 130811 | 131009 | 131037 | 131484 | 132321 | 135335 | 135542 | 135543 | 136417 | 136418 | 136452 | 136453 | 139531 | 139532