Bugzilla – Attachment 94823 Details for
Bug 23630
Elasticsearch indexing is removing field 999
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23630: Do not remove field 999 in Elasticsearch indexing
Bug-23630-Do-not-remove-field-999-in-Elasticsearch.patch (text/plain), 5.65 KB, created by
Martin Renvoize (ashimema)
on 2019-10-29 11:09:59 UTC
(
hide
)
Description:
Bug 23630: Do not remove field 999 in Elasticsearch indexing
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2019-10-29 11:09:59 UTC
Size:
5.65 KB
patch
obsolete
>From 0d787de23d39ec452414cccd60085ef27326be35 Mon Sep 17 00:00:00 2001 >From: Fridolin Somers <fridolin.somers@biblibre.com> >Date: Thu, 19 Sep 2019 16:55:51 +0200 >Subject: [PATCH] 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> >--- > Koha/SearchEngine/Elasticsearch.pm | 5 +-- > Koha/SearchEngine/Elasticsearch/Indexer.pm | 39 ++----------------- > .../Koha/SearchEngine/Elasticsearch/Indexer.t | 2 +- > 3 files changed, 6 insertions(+), 40 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm >index 16532018ba..f91f16c10b 100644 >--- 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 703f39f3ca..3610f0191d 100644 >--- 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 80bfe79956..675ddda65a 100644 >--- 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(), >-- >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 23630
:
92971
|
93242
| 94823 |
94912