From 97e073cda5b0b5488d2bdc532cf687ca05339d96 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Thu, 5 Mar 2020 18:05:10 +0100 Subject: [PATCH] Bug 24823: Drop Catmandu dependency 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 --- Koha/SearchEngine/Elasticsearch.pm | 6 ++-- Koha/SearchEngine/Elasticsearch/Indexer.pm | 30 +++++++--------- Koha/SearchEngine/Elasticsearch/Search.pm | 34 +++++++++---------- t/00-load.t | 5 ++- t/db_dependent/Koha/Authorities.t | 6 ---- .../Koha/SearchEngine/Elasticsearch/Search.t | 14 +++++--- t/db_dependent/Koha/SearchEngine/Search.t | 5 --- 7 files changed, 43 insertions(+), 57 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm index 6616671325..6b64f97c59 100644 --- a/Koha/SearchEngine/Elasticsearch.pm +++ b/Koha/SearchEngine/Elasticsearch.pm @@ -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}; } @@ -1127,7 +1128,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 diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm index bf00dda1d8..1fe2d6cc17 100644 --- a/Koha/SearchEngine/Elasticsearch/Indexer.pm +++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm @@ -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 @@ -107,7 +101,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, { @@ -293,18 +287,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) diff --git a/Koha/SearchEngine/Elasticsearch/Search.pm b/Koha/SearchEngine/Elasticsearch/Search.pm index 0bc8c3ebc6..60af359227 100644 --- a/Koha/SearchEngine/Elasticsearch/Search.pm +++ b/Koha/SearchEngine/Elasticsearch/Search.pm @@ -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; } diff --git a/t/00-load.t b/t/00-load.t index c45f21a599..d518f34fd1 100644 --- a/t/00-load.t +++ b/t/00-load.t @@ -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' ); diff --git a/t/db_dependent/Koha/Authorities.t b/t/db_dependent/Koha/Authorities.t index 8027bdce03..a618bfabb8 100644 --- a/t/db_dependent/Koha/Authorities.t +++ b/t/db_dependent/Koha/Authorities.t @@ -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; diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t index 19c33bf6ec..82ab287aba 100644 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t @@ -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'); } diff --git a/t/db_dependent/Koha/SearchEngine/Search.t b/t/db_dependent/Koha/SearchEngine/Search.t index bbef2f10b6..e3780b34eb 100644 --- a/t/db_dependent/Koha/SearchEngine/Search.t +++ b/t/db_dependent/Koha/SearchEngine/Search.t @@ -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; -- 2.20.1