Bugzilla – Attachment 104371 Details for
Bug 24823
Drop Catmandu dependency
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24823: Drop Catmandu dependency
Bug-24823-Drop-Catmandu-dependency.patch (text/plain), 9.26 KB, created by
Martin Renvoize (ashimema)
on 2020-05-05 14:25:28 UTC
(
hide
)
Description:
Bug 24823: Drop Catmandu dependency
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-05-05 14:25:28 UTC
Size:
9.26 KB
patch
obsolete
>From e87c6afdc34e3f1ff1ec7abb298b07bbb696d270 Mon Sep 17 00:00:00 2001 >From: David Gustafsson <david.gustafsson@ub.gu.se> >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 <david@davidnind.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24823
:
100268
|
100308
|
100337
|
100617
|
104303
|
104304
|
104305
|
104306
|
104307
|
104371
|
104372
|
104373
|
104501
|
104502
|
104503
|
104504