Bug 23630: Do not remove field 999 in Elasticsearch indexing

Elasticsearch indexing uses 999$c to store record id by deleting the all field first !
So you can not store anything in field 999, even in UNIMARC and even in authorities records.

Looks like it is quick fix code added to start Elasticsearch use.

This behavior is disturbing and very strange for UNIMARC flavour.

This patch corrects by defining record ids mandatory in Koha::SearchEngine::Elasticsearch::Indexer::update_index().
This ids array is actually always given (except in UT).
I think it is useless to allow adding a record without its id.

Test plan :
1) Use Elasticsearch as SearchEngine
2) Create a subfield 999$z in default framework
3) Create a record with default framework
4) Enter a random string (never used in catalog) like "tototata" in 999$z
5) In Search engine configuration, define search field "subject" for 999$z
6) Rebuild record : misc/search_tools/rebuild_elasticsearch.pl -b -bn <biblionumber> -v
7) Search for the random string => You get a result
8) Optionnaly look at records in ES : <es server>:9200/<es index name>/data/<biblionumber>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
index 1653201..f91f16c 100644 (file)
--- a/Koha/SearchEngine/Elasticsearch.pm
+++ b/Koha/SearchEngine/Elasticsearch.pm
@@ -414,7 +414,7 @@ sub _process_mappings {
 
 =head2 marc_records_to_documents($marc_records)
 
-    my @record_documents = $self->marc_records_to_documents($marc_records);
+    my $record_documents = $self->marc_records_to_documents($marc_records);
 
 Using mappings stored in database convert C<$marc_records> to Elasticsearch documents.
 
@@ -583,8 +583,7 @@ sub marc_records_to_documents {
                 $record_document->{'marc_format'} = 'base64ISO2709';
             }
         }
-        my $id = $record->subfield('999', 'c');
-        push @record_documents, [$id, $record_document];
+        push @record_documents, $record_document;
     }
     return \@record_documents;
 }

diff --git a/Koha/SearchEngine/Elasticsearch/Indexer.pm b/Koha/SearchEngine/Elasticsearch/Indexer.pm
index 703f39f..3610f01 100644 (file)
--- a/Koha/SearchEngine/Elasticsearch/Indexer.pm
+++ b/Koha/SearchEngine/Elasticsearch/Indexer.pm
@@ -84,14 +84,6 @@ use constant {
 Converts C<MARC::Records> C<$records> to Elasticsearch documents and performs
 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<undef>, 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>
@@ -110,17 +102,14 @@ Arrayref of C<MARC::Record>s.
 sub update_index {
     my ($self, $biblionums, $records) = @_;
 
-    if ($biblionums) {
-        $self->_sanitise_records($biblionums, $records);
-    }
-
     my $conf = $self->get_elasticsearch_params();
     my $elasticsearch = $self->get_elasticsearch();
     my $documents = $self->marc_records_to_documents($records);
     my @body;
 
-    foreach my $document_info (@{$documents}) {
-        my ($id, $document) = @{$document_info};
+    for (my $i=0; $i < scalar @$biblionums; $i++) {
+        my $id = $biblionums->[$i];
+        my $document = $documents->[$i];
         push @body, {
             index => {
                 _id => $id
@@ -382,28 +371,6 @@ sub index_exists {
     );
 }
 
-sub _sanitise_records {
-    my ($self, $biblionums, $records) = @_;
-
-    confess "Unequal number of values in \$biblionums and \$records." if (@$biblionums != @$records);
-
-    my $c = @$biblionums;
-    for (my $i=0; $i<$c; $i++) {
-        my $bibnum = $biblionums->[$i];
-        my $rec = $records->[$i];
-        # I've seen things you people wouldn't believe. Attack ships on fire
-        # off the shoulder of Orion. I watched C-beams glitter in the dark near
-        # the Tannhauser gate. MARC records where 999$c doesn't match the
-        # biblionumber column. All those moments will be lost in time... like
-        # tears in rain...
-        if ( $rec ) {
-            $rec->delete_fields($rec->field('999'));
-            # Make sure biblionumber is a string. Elasticsearch would consider int and string different IDs.
-            $rec->append_fields(MARC::Field->new('999','','','c' => "" . $bibnum, 'd' => "" . $bibnum));
-        }
-    }
-}
-
 1;
 
 __END__

diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
index 80bfe79..675ddda 100644 (file)
--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
@@ -59,7 +59,7 @@ subtest 'create_index() tests' => sub {
         MARC::Field->new('245', '', '', 'a' => 'Title')
     );
     my $records = [$marc_record];
-    ok($indexer->update_index(undef, $records), 'Update Index');
+    ok($indexer->update_index([1], $records), 'Update Index');
 
     is(
         $indexer->drop_index(),