Bugzilla – Attachment 110771 Details for
Bug 26543
Elasticsearch - Importing biblios with multiple items can cause incorrect index
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26543: Refactor import code to reindex once per bib
Bug-26543-Refactor-import-code-to-reindex-once-per.patch (text/plain), 13.82 KB, created by
Nick Clemens
on 2020-09-25 18:53:55 UTC
(
hide
)
Description:
Bug 26543: Refactor import code to reindex once per bib
Filename:
MIME Type:
Creator:
Nick Clemens
Created:
2020-09-25 18:53:55 UTC
Size:
13.82 KB
patch
obsolete
>From d68948da61c712689236dbca1740850eb225d0dc Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 23 Sep 2020 13:30:36 +0000 >Subject: [PATCH] Bug 26543: Refactor import code to reindex once per bib > >To test: >0 - Be using Elasticsearch, apply patches, reindex >1 - Add multiple items to several records in your catalog >2 - Export those records >3 - Update the items on the records in Koha to be 'damaged' >4 - Import the file exported above, matchign on biblionumber and overlaying items >5 - Perform a search to return the imported records >6 - Confirm all items have damaged correctly removed in search results >7 - Import without matching and ignoring items to confirm new records are added >8 - Search to return records and verify they are indexed (shoudl see 2 copies) >9 - Alter the barcodes of the items in koha >10 - Import the file matching on biblionumber and adding items >11 - Search tp return records, confirm the new items are indexed >12 - Export some authority records >13 - Import them without matching and confirm new records are searchable >14 - Alter the existing records, and match on AuthID, you will need to create a matching rule for this >15 - Import and match and confirm the records are searchable and reflect the change back to original values >--- > C4/Biblio.pm | 41 ++++++++++++++++++++++++++++++++--------- > C4/ImportBatch.pm | 32 ++++++++++++++++++++------------ > 2 files changed, 52 insertions(+), 21 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 1b5560b626..b729958ff6 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -181,7 +181,9 @@ bib to add, while the second argument is the desired MARC > framework code. > > This function also accepts a third, optional argument: a hashref >-to additional options. The only defined option is C<defer_marc_save>, >+to additional options. The defined options are: >+ >+C<defer_marc_save>, > which if present and mapped to a true value, causes C<AddBiblio> > to omit the call to save the MARC in C<biblio_metadata.metadata> > This option is provided B<only> >@@ -191,6 +193,11 @@ saving it and which cannot afford the performance hit of saving > the MARC record twice. Consequently, do not use that option > unless you can guarantee that C<ModBiblioMarc> will be called. > >+C<skip_record_index> >+This prevents calling of index_records to add the record to the search engine(s) >+If the caller is performing more actions to the record, or acting on a batch of records >+this prevents multiple indexing calls. >+ > =cut > > sub AddBiblio { >@@ -198,6 +205,7 @@ sub AddBiblio { > my $frameworkcode = shift; > my $options = @_ ? shift : undef; > my $defer_marc_save = 0; >+ my $skip_record_index = 0; > if (!$record) { > carp('AddBiblio called with undefined record'); > return; >@@ -205,6 +213,9 @@ sub AddBiblio { > if ( defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'} ) { > $defer_marc_save = 1; > } >+ if ( defined $options and exists $options->{'skip_record_index'} and $options->{'skip_record_index'} ) { >+ $skip_record_index = 1; >+ } > > if (C4::Context->preference('BiblioAddsAuthorities')) { > BiblioAutoLink( $record, $frameworkcode ); >@@ -226,7 +237,7 @@ sub AddBiblio { > _koha_marc_update_biblioitem_cn_sort( $record, $olddata, $frameworkcode ); > > # now add the record >- ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; >+ ModBiblioMarc( $record, $biblionumber, $frameworkcode, { skip_record_index => $skip_record_index } ) unless $defer_marc_save; > > # update OAI-PMH sets > if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { >@@ -241,7 +252,7 @@ sub AddBiblio { > > =head2 ModBiblio > >- ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink); >+ ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink,[$params]); > > Replace an existing bib record identified by C<$biblionumber> > with one supplied by the MARC::Record object C<$record>. The embedded >@@ -263,10 +274,15 @@ us to not relink records when the authority linker is saving modifications. > > Returns 1 on success 0 on failure > >+The final optional parameter, C<$params>, expected to contain >+'skip_record_index' key, which prevents calling of index_records. >+If the caller is performing more actions to the record, or acting on a batch of records >+this prevents multiple indexing calls. >+ > =cut > > sub ModBiblio { >- my ( $record, $biblionumber, $frameworkcode, $disable_autolink ) = @_; >+ my ( $record, $biblionumber, $frameworkcode, $disable_autolink, $params ) = @_; > if (!$record) { > carp 'No record passed to ModBiblio'; > return 0; >@@ -314,7 +330,7 @@ sub ModBiblio { > _koha_marc_update_biblioitem_cn_sort( $record, $oldbiblio, $frameworkcode ); > > # update the MARC record (that now contains biblio and items) with the new record data >- &ModBiblioMarc( $record, $biblionumber, $frameworkcode ); >+ &ModBiblioMarc( $record, $biblionumber, $frameworkcode, { skip_record_index => $params->{skip_record_index } } ); > > # modify the other koha tables > _koha_modify_biblio( $dbh, $oldbiblio, $frameworkcode ); >@@ -3035,18 +3051,23 @@ sub _koha_delete_biblio_metadata { > > =head2 ModBiblioMarc > >- &ModBiblioMarc($newrec,$biblionumber,$frameworkcode); >+ &ModBiblioMarc($newrec,$biblionumber,$frameworkcode, [$params]); > > Add MARC XML data for a biblio to koha > > Function exported, but should NOT be used, unless you really know what you're doing > >+The final optional parameter, C<$params>, expected to contain >+'skip_record_index' key, which prevents calling of index_records. >+If the caller is performing more actions to the record, or acting on a batch of records >+this prevents multiple indexing calls. >+ > =cut > > sub ModBiblioMarc { > # pass the MARC::Record to this function, and it will create the records in > # the marcxml field >- my ( $record, $biblionumber, $frameworkcode ) = @_; >+ my ( $record, $biblionumber, $frameworkcode, $params ) = @_; > if ( !$record ) { > carp 'ModBiblioMarc passed an undefined record'; > return; >@@ -3117,8 +3138,10 @@ sub ModBiblioMarc { > $m_rs->metadata( $record->as_xml_record($encoding) ); > $m_rs->store; > >- my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >- $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" ); >+ unless( $params->{skip_record_index} ){ >+ my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >+ $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" ); >+ } > > return $biblionumber; > } >diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm >index 7d0d7bab5a..51ddab8767 100644 >--- a/C4/ImportBatch.pm >+++ b/C4/ImportBatch.pm >@@ -577,9 +577,10 @@ sub BatchCommitRecords { > my $num_ignored = 0; > # commit (i.e., save, all records in the batch) > SetImportBatchStatus('importing'); >- my $overlay_action = GetImportBatchOverlayAction($batch_id); >- my $nomatch_action = GetImportBatchNoMatchAction($batch_id); >- my $item_action = GetImportBatchItemAction($batch_id); >+ my $batch = GetImportBatch( $batch_id ); >+ my $overlay_action = $batch->{overlay_action}; >+ my $nomatch_action = $batch->{nomatch_action}; >+ my $item_action = $batch->{item_action}; > my $item_tag; > my $item_subfield; > my $dbh = C4::Context->dbh; >@@ -591,6 +592,10 @@ sub BatchCommitRecords { > $sth->execute($batch_id); > my $marcflavour = C4::Context->preference('marcflavour'); > my $rec_num = 0; >+ my $index; >+ $index = $batch->{record_type} eq 'biblio' ? >+ $Koha::SearchEngine::BIBLIOS_INDEX : $Koha::SearchEngine::AUTHORITIES_INDEX; >+ my $indexer = Koha::SearchEngine::Indexer->new({ index => $index }); > while (my $rowref = $sth->fetchrow_hashref) { > $record_type = $rowref->{'record_type'}; > $rec_num++; >@@ -630,14 +635,15 @@ sub BatchCommitRecords { > $num_added++; > if ($record_type eq 'biblio') { > my $biblioitemnumber; >- ($recordid, $biblioitemnumber) = AddBiblio($marc_record, $framework); >+ ($recordid, $biblioitemnumber) = AddBiblio($marc_record, $framework, { skip_record_index => 1 }); > $query = "UPDATE import_biblios SET matched_biblionumber = ? WHERE import_record_id = ?"; > if ($item_result eq 'create_new' || $item_result eq 'replace') { >- my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); >+ my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result, { skip_record_index => 1 }); > $num_items_added += $bib_items_added; > $num_items_replaced += $bib_items_replaced; > $num_items_errored += $bib_items_errored; > } >+ $indexer->index_records( $recordid, "specialUpdate", "biblioserver" ); > } else { > $recordid = AddAuthority($marc_record, undef, GuessAuthTypeCode($marc_record)); > $query = "UPDATE import_auths SET matched_authid = ? WHERE import_record_id = ?"; >@@ -663,19 +669,20 @@ sub BatchCommitRecords { > } > $oldxml = $old_marc->as_xml($marc_type); > >- ModBiblio($marc_record, $recordid, $oldbiblio->frameworkcode); >+ ModBiblio($marc_record, $recordid, $oldbiblio->frameworkcode, { skip_record_index => 1 }); > $query = "UPDATE import_biblios SET matched_biblionumber = ? WHERE import_record_id = ?"; > > if ($item_result eq 'create_new' || $item_result eq 'replace') { >- my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); >+ my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result, { skip_record_index => 1}); > $num_items_added += $bib_items_added; > $num_items_replaced += $bib_items_replaced; > $num_items_errored += $bib_items_errored; > } >+ $indexer->index_records( $recordid, "specialUpdate", "biblioserver" ); > } else { > $oldxml = GetAuthorityXML($recordid); > >- ModAuthority($recordid, $marc_record, GuessAuthTypeCode($marc_record)); >+ ModAuthority($recordid, $marc_record, GuessAuthTypeCode($marc_record) ); > $query = "UPDATE import_auths SET matched_authid = ? WHERE import_record_id = ?"; > } > my $sth = $dbh->prepare_cached("UPDATE import_records SET marcxml_old = ? WHERE import_record_id = ?"); >@@ -691,7 +698,7 @@ sub BatchCommitRecords { > $num_ignored++; > $recordid = $record_match; > if ($record_type eq 'biblio' and defined $recordid and ( $item_result eq 'create_new' || $item_result eq 'replace' ) ) { >- my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); >+ my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result, { skip_record_index =>1 }); > $num_items_added += $bib_items_added; > $num_items_replaced += $bib_items_replaced; > $num_items_errored += $bib_items_errored; >@@ -700,6 +707,7 @@ sub BatchCommitRecords { > my $sth2 = $dbh->prepare_cached("UPDATE import_biblios SET matched_biblionumber = ? WHERE import_record_id = ?"); > $sth2->execute($recordid, $rowref->{'import_record_id'}); > SetImportRecordOverlayStatus($rowref->{'import_record_id'}, 'match_applied'); >+ $indexer->index_records( $recordid, "specialUpdate", "biblioserver" ); > } > SetImportRecordStatus($rowref->{'import_record_id'}, 'ignored'); > } >@@ -750,7 +758,7 @@ sub BatchCommitItems { > my $updsth = $dbh->prepare("UPDATE import_items SET status = ?, itemnumber = ? WHERE import_items_id = ?"); > if ( $action eq "replace" && $duplicate_itemnumber ) { > # Duplicate itemnumbers have precedence, that way we can update barcodes by overlaying >- ModItemFromMarc( $item_marc, $biblionumber, $item->{itemnumber} ); >+ ModItemFromMarc( $item_marc, $biblionumber, $item->{itemnumber},{ skip_record_index => 1 } ); > $updsth->bind_param( 1, 'imported' ); > $updsth->bind_param( 2, $item->{itemnumber} ); > $updsth->bind_param( 3, $row->{'import_items_id'} ); >@@ -759,7 +767,7 @@ sub BatchCommitItems { > $num_items_replaced++; > } elsif ( $action eq "replace" && $duplicate_barcode ) { > my $itemnumber = $duplicate_barcode->itemnumber; >- ModItemFromMarc( $item_marc, $biblionumber, $itemnumber ); >+ ModItemFromMarc( $item_marc, $biblionumber, $itemnumber, { skip_record_index => 1} ); > $updsth->bind_param( 1, 'imported' ); > $updsth->bind_param( 2, $item->{itemnumber} ); > $updsth->bind_param( 3, $row->{'import_items_id'} ); >@@ -773,7 +781,7 @@ sub BatchCommitItems { > $updsth->execute(); > $num_items_errored++; > } else { >- my ( $item_biblionumber, $biblioitemnumber, $itemnumber ) = AddItemFromMarc( $item_marc, $biblionumber ); >+ my ( $item_biblionumber, $biblioitemnumber, $itemnumber ) = AddItemFromMarc( $item_marc, $biblionumber, { skip_record_index => 1 } ); > if( $itemnumber ) { > $updsth->bind_param( 1, 'imported' ); > $updsth->bind_param( 2, $itemnumber ); >-- >2.11.0
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 26543
: 110771 |
110772