From d68948da61c712689236dbca1740850eb225d0dc Mon Sep 17 00:00:00 2001 From: Nick Clemens 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, +to additional options. The defined options are: + +C, which if present and mapped to a true value, causes C to omit the call to save the MARC in C This option is provided B @@ -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 will be called. +C +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