@@ -, +, @@ serialization format --- Koha/SearchEngine/Elasticsearch.pm | 63 +++++++-- Koha/SearchEngine/Elasticsearch/Indexer.pm | 205 +++++++++++++++++++++------- misc/search_tools/rebuild_elastic_search.pl | 6 +- t/Koha/SearchEngine/Elasticsearch.t | 27 +++- 4 files changed, 240 insertions(+), 61 deletions(-) --- a/Koha/SearchEngine/Elasticsearch.pm +++ a/Koha/SearchEngine/Elasticsearch.pm @@ -73,6 +73,15 @@ sub new { return $self; } +=head2 get_elasticsearch + + my $elasticsearch_client = $self->get_elasticsearch(); + +Returns a C client. The client is cached on a C +instance level and will be reused if method is called multiple times. + +=cut + sub get_elasticsearch { my $self = shift @_; unless (defined $self->{elasticsearch}) { @@ -145,8 +154,8 @@ sub get_elasticsearch_params { my $settings = $self->get_elasticsearch_settings(); -This provides the settings provided to elasticsearch when an index is created. -These can do things like define tokenisation methods. +This provides the settings provided to Elasticsearch when an index is created. +These can do things like define tokenization methods. A hashref containing the settings is returned. @@ -170,7 +179,7 @@ sub get_elasticsearch_settings { my $mappings = $self->get_elasticsearch_mappings(); -This provides the mappings that get passed to elasticsearch when an index is +This provides the mappings that get passed to Elasticsearch when an index is created. =cut @@ -232,7 +241,7 @@ sub get_elasticsearch_mappings { =head2 _get_elasticsearch_mapping -Get the ES mappings for the given purpose and data type +Get the Elasticsearch mappings for the given purpose and data type. $mapping = _get_elasticsearch_mapping('search', 'text'); @@ -301,13 +310,31 @@ sub sort_fields { return $self->_sort_fields_accessor(); } +=head2 marc_records_to_documents($marc_records) + + my @record_documents = $self->marc_records_to_documents($marc_records); + +Using mappings stored in databse convert C<$marc_records> to ElasticSearch documents. + +Returns array of hash references, representing ElasticSearch documents, +acceptable as body payload in C requests. + +=over 4 + +=item C<$marc_documents> + +Reference to array of C objects to be converted to ElasticSearch documents. + +=back + +=cut + sub marc_records_to_documents { my ($self, $records) = @_; - my $rules = $self->get_marc_mapping_rules(); + my $rules = $self->_get_marc_mapping_rules(); my $control_fields_rules = $rules->{control_fields}; my $data_fields_rules = $rules->{data_fields}; my $marcflavour = lc C4::Context->preference('marcflavour'); - my $serialization_format = C4::Context->preference('ElasticsearchMARCSerializationFormat'); my @record_documents; @@ -426,8 +453,28 @@ sub marc_records_to_documents { return \@record_documents; } -# Provides the rules for marc to Elasticsearch JSON document conversion. -sub get_marc_mapping_rules { +=head2 _get_marc_mapping_rules + + my $mapping_rules = $self->_get_marc_mapping_rules() + +Generates rules from mappings stored in databse for MARC records to Elasticsearch JSON document conversion. + +Since field retrieval is slow in C (all fields are itereted through for +each call to C->field) we create an optimized structure of mapping +rules keyed by MARC field tags holding all the mapping rules for that particular tag. + +We can then iterate through all MARC fields for each record and apply all relevant +rules once per fields instead of retreiving field multiple times for each mapping rule +wich is terribly slow. + +=cut + +# TODO: This structure can be used for processing multiple MARC::Records so is currently +# rebuilt for each batch. Since it is cacheable it could also be stored in an in +# memory cache which it is currently not. The performance gain of caching +# would probably be marginal, but to do this could be a further improvement. + +sub _get_marc_mapping_rules { my ($self) = @_; my $marcflavour = lc C4::Context->preference('marcflavour'); --- a/Koha/SearchEngine/Elasticsearch/Indexer.pm +++ a/Koha/SearchEngine/Elasticsearch/Indexer.pm @@ -44,45 +44,76 @@ Koha::SearchEngine::Elasticsearch::Indexer - handles adding new records to the i $indexer->drop_index(); $indexer->update_index(\@biblionumbers, \@records); -=head1 FUNCTIONS -=head2 $indexer->update_index($biblionums, $records); +=head1 CONSTANTS -C<$biblionums> is an arrayref containing the biblionumbers for the records. +=over 4 -C<$records> is an arrayref containing the Ls themselves. +=item C -The values in the arrays must match up, and the 999$c value in the MARC record -will be rewritten using the values in C<$biblionums> to ensure they are correct. -If C<$biblionums> is C, this won't happen, but you should be sure that -999$c is correct on your own then. +Represents an index state where index is created and in a working state. -Note that this will modify the original record if C<$biblionums> is supplied. -If that's a problem, clone them first. +=item C + +Not currently used, but could be useful later, for example if can detect when new field or mapping added. + +=item C + +Representings an index state where index needs to be recreated and is not in a working state. + +=back =cut use constant { INDEX_STATUS_OK => 0, - INDEX_STATUS_REINDEX_REQUIRED => 1, # Not currently used, but could be useful later, for example if can detect when new field or mapping added + INDEX_STATUS_REINDEX_REQUIRED => 1, INDEX_STATUS_RECREATE_REQUIRED => 2, }; +=head1 FUNCTIONS + +=head2 update_index($biblionums, $records) + + try { + $self->update_index($biblionums, $records); + } catch { + die("Something whent wrong trying to update index:" . $_[0]); + } + +Converts C C<$records> to Elasticsearch documents and perform +an update request for these records on the Elasticsearch index. + +The values in the arrays must match up, and the 999$c value in the MARC record +will be rewritten using the values in C<$biblionums> to ensure they are correct. +If C<$biblionums> is C, this won't happen, so in that case you should make +sure that 999$c is correct. + +Note that this will modify the original record if C<$biblionums> is supplied. +If that's a problem, clone them first. + +=over 4 + +=item C<$biblionums> + +Arrayref of biblio numbers for the C<$records>, the order must be the same +as C<$records>. + +=item C<$records> + +Arrayref of Cs. + +=back + +=cut + sub update_index { my ($self, $biblionums, $records) = @_; - # TODO should have a separate path for dealing with a large number - # of records at once where we use the bulk update functions in ES. if ($biblionums) { $self->_sanitise_records($biblionums, $records); } - $self->bulk_index($records); - return 1; -} - -sub bulk_index { - my ($self, $records) = @_; my $conf = $self->get_elasticsearch_params(); my $elasticsearch = $self->get_elasticsearch(); my $documents = $self->marc_records_to_documents($records); @@ -108,27 +139,83 @@ sub bulk_index { return 1; } -sub index_status_ok { +=head2 set_index_status_ok + +Convenience method for setting index status to C. + +=cut + +sub set_index_status_ok { my ($self, $set) = @_; - return defined $set ? - $self->index_status(INDEX_STATUS_OK) : - $self->index_status == INDEX_STATUS_OK; + $self->index_status(INDEX_STATUS_OK); } -sub index_status_reindex_required { - my ($self, $set) = @_; - return defined $set ? - $self->index_status(INDEX_STATUS_REINDEX_REQUIRED) : - $self->index_status == INDEX_STATUS_REINDEX_REQUIRED; +=head2 is_index_status_ok + +Convenience method for checking if index status is C. + +=cut + +sub is_index_status_ok { + return $self->index_status == INDEX_STATUS_OK; } -sub index_status_recreate_required { - my ($self, $set) = @_; - return defined $set ? - $self->index_status(INDEX_STATUS_RECREATE_REQUIRED) : - $self->index_status == INDEX_STATUS_RECREATE_REQUIRED; +=head2 set_index_status_reindex_required + +Convenience method for setting index status to C. + +=cut + +sub set_index_status_reindex_required { + $self->index_status(INDEX_STATUS_REINDEX_REQUIRED); +} + +=head2 is_index_status_reindex_required + +Convenience method for checking if index status is C. + +=cut + +sub is_index_status_reindex_required { + return $self->index_status == INDEX_STATUS_REINDEX_REQUIRED; +} + +=head2 set_index_status_recreate_required + +Convenience method for setting index status to C. + +=cut + +sub set_index_status_recreate_required { + $self->index_status(INDEX_STATUS_RECREATE_REQUIRED); +} + +=head2 is_index_status_recreate_required + +Convenience method for checking if index status is C. + +=cut + +sub is_index_status_recreate_required { + return $self->index_status == INDEX_STATUS_RECREATE_REQUIRED; } +=head2 index_status($status) + +Will either set the current index status to C<$status> and return C<$status>, +or return the current index status if called with no arguments. + +=over 4 + +=item C<$status> + +Optional argument. If passed will set current index status to C<$status> if C<$status> is +a valid status. See L. + +=back + +=cut + sub index_status { my ($self, $status) = @_; my $key = 'ElasticsearchIndexStatus_' . $self->index; @@ -150,6 +237,15 @@ sub index_status { } } +=head2 update_mappings + +Generate Elasticsearch mappings from mappings stored in database and +perform a request to update Elasticsearch index mappings. Will throw an +error and set index status to C if update +failes. + +=cut + sub update_mappings { my ($self) = @_; my $conf = $self->get_elasticsearch_params(); @@ -166,35 +262,35 @@ sub update_mappings { } ); } catch { - $self->index_status_recreate_required(1); + $self->set_index_status_recreate_required(); my $reason = $_[0]->{vars}->{body}->{error}->{reason}; Koha::Exceptions::Exception->throw( error => "Unable to update mappings for index \"$conf->{index_name}\". Reason was: \"$reason\". Index needs to be recreated and reindexed", ); }; } - $self->index_status_ok(1); + $self->set_index_status_ok(); } -=head2 $indexer->update_index_background($biblionums, $records) +=head2 update_index_background($biblionums, $records) -This has exactly the same API as C however it'll +This has exactly the same API as C however it'll return immediately. It'll start a background process that does the adding. If it fails to add to Elasticsearch then it'll add to a queue that will cause it to be updated by a regular index cron job in the future. +=cut + # TODO implement in the future - I don't know the best way of doing this yet. # If fork: make sure process group is changed so apache doesn't wait for us. -=cut - sub update_index_background { my $self = shift; $self->update_index(@_); } -=head2 $indexer->delete_index($biblionums) +=head2 delete_index($biblionums) C<$biblionums> is an arrayref of biblionumbers to delete from the index. @@ -206,7 +302,7 @@ sub delete_index { if ( !$self->store ) { my $params = $self->get_elasticsearch_params(); $self->store( - Catmandu::Store::ElasticSearch->new( + Catmandu::Store::Elasticsearch->new( %$params, index_settings => $self->get_elasticsearch_settings(), index_mappings => $self->get_elasticsearch_mappings(), @@ -217,23 +313,21 @@ sub delete_index { $self->store->bag->commit; } -=head2 $indexer->delete_index_background($biblionums) +=head2 delete_index_background($biblionums) -Identical to L, this will return immediately and start a -background process to do the actual deleting. +Identical to L =cut -# TODO implement in the future - +# TODO: Should be made async sub delete_index_background { my $self = shift; $self->delete_index(@_); } -=head2 $indexer->drop_index(); +=head2 drop_index -Drops the index from the elasticsearch server. +Drops the index from the Elasticsearch server. =cut @@ -243,10 +337,16 @@ sub drop_index { my $conf = $self->get_elasticsearch_params(); my $elasticsearch = $self->get_elasticsearch(); $elasticsearch->indices->delete(index => $conf->{index_name}); - $self->index_status_recreate_required(1); + $self->set_index_status_recreate_required(); } } +=head2 create_index + +Creates the index (including mappings) on the Elasticsearch server. + +=cut + sub create_index { my ($self) = @_; my $conf = $self->get_elasticsearch_params(); @@ -261,6 +361,13 @@ sub create_index { $self->update_mappings(); } +=head2 index_exists + +Checks if index has been created on the ElasticSearch server. Returns C<1> or the +empty string to indicate whether index exists or not. + +=cut + sub index_exists { my ($self) = @_; my $conf = $self->get_elasticsearch_params(); --- a/misc/search_tools/rebuild_elastic_search.pl +++ a/misc/search_tools/rebuild_elastic_search.pl @@ -165,13 +165,13 @@ sub do_reindex { $indexer->drop_index() if $indexer->index_exists(); $indexer->create_index(); } - elsif (!$indexer->index_exists()) { + elsif (!$indexer->index_exists) { # Create index if does not exist $indexer->create_index(); - } elsif ($indexer->index_status_ok) { + } elsif ($indexer->is_index_status_ok) { # Update mapping unless index is some kind of problematic state $indexer->update_mappings(); - } elsif ($indexer->index_status_recreate_required) { + } elsif ($indexer->is_index_status_recreate_required) { warn qq/Index "$index_name" has status "recreate required", suggesting it should be recreated/; } --- a/t/Koha/SearchEngine/Elasticsearch.t +++ a/t/Koha/SearchEngine/Elasticsearch.t @@ -115,7 +115,7 @@ subtest 'get_elasticsearch_mappings() tests' => sub { subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' => sub { - plan tests => 30; + plan tests => 32; t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); @@ -315,6 +315,8 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' ok(defined $docs->[0][1]->{marc_format}, 'First document marc_format field should be set'); + is($docs->[0][1]->{marc_format}, 'base64ISO2709', 'First document marc_format should be set correctly'); + is(scalar @{$docs->[0][1]->{type_of_record}}, 1, 'First document type_of_record field should have one value'); is_deeply( $docs->[0][1]->{type_of_record}, @@ -351,4 +353,27 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' ok(!(defined $docs->[0][1]->{unimarc_title}), "No mapping when marc_type doesn't match marc flavour"); + # Marc serialization format fallback for records exceeding ISO2709 max record size + + my $large_marc_record = MARC::Record->new(); + $large_marc_record->leader(' cam 22 a 4500'); + + $large_marc_record->append_fields( + MARC::Field->new('100', '', '', a => 'Author 1'), + MARC::Field->new('110', '', '', a => 'Corp Author'), + MARC::Field->new('210', '', '', a => 'Title 1'), + MARC::Field->new('245', '', '', a => 'Title:', b => 'large record'), + MARC::Field->new('999', '', '', c => '1234567'), + ); + + my $item_field = MARC::Field->new('952', '', '', o => '123456789123456789123456789', p => '123456789', z => 'test'); + my $items_count = 1638; + while(--$items_count) { + $large_marc_record->append_fields($item_field); + } + + $docs = $see->marc_records_to_documents([$large_marc_record]); + + is($docs->[0][1]->{marc_format}, 'MARCXML', 'For record exceeding max record size marc_format should be set correctly'); + }; --