Bug 24823: Drop Catmandu dependency
authorDavid Gustafsson <david.gustafsson@ub.gu.se>
Thu, 5 Mar 2020 17:05:10 +0000 (18:05 +0100)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Tue, 12 May 2020 10:43:19 +0000 (11:43 +0100)
Replace remaining Catmandu dependant code with the Search::Elasticsearch
equivalent.

To test:
1) Apply patch
2) Run tests in t/Koha/SearchEngine/Elasticsearch.t, t/Koha/SearchEngine/ElasticSearch/*
and t/db_dependent/Koha/SearchEngine/*
3) All tests should pass

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Koha/SearchEngine/Elasticsearch.pm
Koha/SearchEngine/Elasticsearch/Indexer.pm
Koha/SearchEngine/Elasticsearch/Search.pm
t/00-load.t
t/db_dependent/Koha/Authorities.t
t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t
t/db_dependent/Koha/SearchEngine/Search.t

index 202ef7e..4a1ceea 100644 (file)
@@ -90,8 +90,9 @@ instance level and will be reused if method is called multiple times.
 sub get_elasticsearch {
     my $self = shift @_;
     unless (defined $self->{elasticsearch}) {
-        my $conf = $self->get_elasticsearch_params();
-        $self->{elasticsearch} = Search::Elasticsearch->new($conf);
+        $self->{elasticsearch} = Search::Elasticsearch->new(
+            $self->get_elasticsearch_params()
+        );
     }
     return $self->{elasticsearch};
 }
@@ -1129,7 +1130,6 @@ A string that indicates the MARC type that this mapping is for, e.g. 'marc21',
 =item C<$marc_field>
 
 A string that describes the MARC field that contains the data to extract.
-These are of a form suited to Catmandu's MARC fixers.
 
 =back
 
index 4c57445..a36a4e2 100644 (file)
@@ -24,15 +24,9 @@ use List::Util qw(any);
 use base qw(Koha::SearchEngine::Elasticsearch);
 use Data::Dumper;
 
-# For now just marc, but we can do anything here really
-use Catmandu::Importer::MARC;
-use Catmandu::Store::ElasticSearch;
-
 use Koha::Exceptions;
 use C4::Context;
 
-Koha::SearchEngine::Elasticsearch::Indexer->mk_accessors(qw( store ));
-
 =head1 NAME
 
 Koha::SearchEngine::Elasticsearch::Indexer - handles adding new records to the index
@@ -106,7 +100,7 @@ sub update_index {
     my $documents = $self->marc_records_to_documents($records);
     my @body;
 
-    for (my $i=0; $i < scalar @$biblionums; $i++) {
+    for (my $i = 0; $i < scalar @$biblionums; $i++) {
         my $id = $biblionums->[$i];
         my $document = $documents->[$i];
         push @body, {
@@ -292,18 +286,18 @@ C<$biblionums> is an arrayref of biblionumbers to delete from the index.
 sub delete_index {
     my ($self, $biblionums) = @_;
 
-    if ( !$self->store ) {
-        my $params  = $self->get_elasticsearch_params();
-        $self->store(
-            Catmandu::Store::ElasticSearch->new(
-                %$params,
-                index_settings => $self->get_elasticsearch_settings(),
-                index_mappings => $self->get_elasticsearch_mappings(),
-            )
-        );
+    my $elasticsearch = $self->get_elasticsearch();
+    my $conf  = $self->get_elasticsearch_params();
+
+    my @body = map { { delete => { _id => $_ } } } @{$biblionums};
+    my $result = $elasticsearch->bulk(
+        index => $conf->{index_name},
+        type => 'data',
+        body => \@body,
+    );
+    if ($result->{errors}) {
+        croak "An Elasticsearch error occured during bulk delete";
     }
-    $self->store->bag->delete("$_") foreach @$biblionums;
-    $self->store->bag->commit;
 }
 
 =head2 delete_index_background($biblionums)
index 0bc8c3e..60af359 100644 (file)
@@ -49,7 +49,6 @@ use Koha::SearchEngine::QueryBuilder;
 use Koha::SearchEngine::Search;
 use Koha::Exceptions::Elasticsearch;
 use MARC::Record;
-use Catmandu::Store::ElasticSearch;
 use MARC::File::XML;
 use Data::Dumper; #TODO remove
 use Carp qw(cluck);
@@ -117,15 +116,17 @@ faster than pulling all the data in, usually.
 
 sub count {
     my ( $self, $query ) = @_;
+    my $elasticsearch = $self->get_elasticsearch();
+    my $conf = $self->get_elasticsearch_params();
 
-    my $params = $self->get_elasticsearch_params();
-    $self->store(
-        Catmandu::Store::ElasticSearch->new( %$params, trace_calls => 0, ) )
-      unless $self->store;
+    # TODO: Probably possible to exclude results
+    # and just return number of hits
+    my $result = $elasticsearch->search(
+        index => $conf->{index_name},
+        body => $query
+    );
 
-    my $search = $self->store->bag->search( %$query);
-    my $count = $search->total() || 0;
-    return $count;
+    return $result->{hits}->{total};
 }
 
 =head2 search_compat
@@ -405,18 +406,17 @@ the default value for this setting in case it is not set)
 sub max_result_window {
     my ($self) = @_;
 
-    $self->store(
-        Catmandu::Store::ElasticSearch->new(%{ $self->get_elasticsearch_params })
-    ) unless $self->store;
+    my $elasticsearch = $self->get_elasticsearch();
+    my $conf = $self->get_elasticsearch_params();
 
-    my $index_name = $self->store->index_name;
-    my $settings = $self->store->es->indices->get_settings(
-        index  => $index_name,
-        params => { include_defaults => 'true', flat_settings => 'true' },
+    my $response = $elasticsearch->indices->get_settings(
+        index => $conf->{index_name},
+        flat_settings => 'true',
+        include_defaults => 'true'
     );
 
-    my $max_result_window = $settings->{$index_name}->{settings}->{'index.max_result_window'};
-    $max_result_window //= $settings->{$index_name}->{defaults}->{'index.max_result_window'};
+    my $max_result_window = $response->{$conf->{index_name}}->{settings}->{'index.max_result_window'};
+    $max_result_window //= $response->{$conf->{index_name}}->{defaults}->{'index.max_result_window'};
 
     return $max_result_window;
 }
index c45f21a..d518f34 100644 (file)
@@ -76,11 +76,10 @@ sub is_testable {
     my @needed_module_names;
     my $return_value = 1;
     if ( $module_name =~ /Koha::SearchEngine::Elasticsearch::Indexer/xsm ) {
-        @needed_module_names =
-          ( 'Catmandu::Importer::MARC', 'Catmandu::Store::ElasticSearch' );
+        @needed_module_names = ( 'Search::Elasticsearch' );
     }
     elsif ( $module_name =~ /Koha::SearchEngine::Elasticsearch::Search/xsm ) {
-        @needed_module_names = ( 'Catmandu::Store::ElasticSearch' );
+        @needed_module_names = ( 'Search::Elasticsearch' );
     }
     elsif ( $module_name =~ /Koha::SearchEngine::Elasticsearch/xsm ) {
         @needed_module_names = ( 'Search::Elasticsearch' );
index 8027bdc..a618bfa 100644 (file)
@@ -41,12 +41,6 @@ use Koha::Database;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
-BEGIN {
-    #TODO Helpful as long as we have issues here
-    my $mock = Test::MockObject->new();
-    $mock->fake_module( 'Catmandu::Store::ElasticSearch' );
-}
-
 my $schema = Koha::Database->new->schema;
 $schema->storage->txn_begin;
 
index 19c33bf..82ab287 100644 (file)
@@ -115,10 +115,14 @@ SKIP: {
     is ( $count = $searcher->count_auth_use($searcher,1), 0, 'Testing count_auth_use');
 
     is ($searcher->max_result_window, 10000, 'By default, max_result_window is 10000');
-    $searcher->store->es->indices->put_settings(index => $searcher->store->index_name, body => {
-        'index' => {
-            'max_result_window' => 12000,
-        },
-    });
+
+    $searcher->get_elasticsearch()->indices->put_settings(
+        index => $searcher->get_elasticsearch_params()->{index_name},
+        body => {
+            'index' => {
+                'max_result_window' => 12000,
+            },
+        }
+    );
     is ($searcher->max_result_window, 12000, 'max_result_window returns the correct value');
 }
index bbef2f1..e3780b3 100644 (file)
@@ -17,11 +17,6 @@ use t::lib::Mocks;
 use Koha::Database;
 use Koha::SearchEngine::Search;
 
-BEGIN {
-    my $mock = Test::MockObject->new();
-    $mock->fake_module( 'Catmandu::Store::ElasticSearch' );
-}
-
 my $schema  = Koha::Database->new->schema;
 $schema->storage->txn_begin;