From 2c6cb9ebd4152b42d0b243ff6edaacf5f1a95c03 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 29 Jul 2019 16:35:39 -0300 Subject: [PATCH] Bug 23395: Add a $plugins parameter to *Biblio and *Item methods This patch adds a $plugin parameter (the way those methods already expect parameters) that is expected to contain an arrayref of plugin object instances. The target scenario are batch operations, in which we really don't want to fetch the available plugins on each iteration. This patch doesn't make batch-related scripts use this new option. It will be done on a follow-up patch. To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t => SUCCESS: All tests pass, new tests as well - Sign off :-D --- C4/Biblio.pm | 67 ++++++++++++++++++++++++++++++++++++---------------- C4/Items.pm | 45 +++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 32 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index a0af92453e..bf2361f9ef 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -183,15 +183,19 @@ 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, -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 -for the use of scripts such as C that may need -to do some manipulation of the MARC record for item parsing before -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. +to additional options. + +The C option, 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 for the use of scripts such as +C that may need to do some manipulation of the +MARC record for item parsing before 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. + +The C option, expects an arrayref of plugin objects, to be +passed to the plugin hooks to avoid unnecessary queries in the context +of batch operations (e.g. bulkmarcimport.pl) =cut @@ -200,12 +204,17 @@ sub AddBiblio { my $frameworkcode = shift; my $options = @_ ? shift : undef; my $defer_marc_save = 0; + my $plugins; + if (!$record) { carp('AddBiblio called with undefined record'); return; } - if ( defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'} ) { - $defer_marc_save = 1; + if ( defined $options ) { + $defer_marc_save = 1 + if exists $options->{defer_marc_save} and $options->{defer_marc_save}; + $plugins = $options->{plugins} + if exists $options->{plugins} and $options->{plugins}; } if (C4::Context->preference('BiblioAddsAuthorities')) { @@ -235,7 +244,7 @@ sub AddBiblio { C4::OAI::Sets::UpdateOAISetsBiblio($biblionumber, $record); } - _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber }); + _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber, plugins => $plugins }); logaction( "CATALOGUING", "ADD", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); return ( $biblionumber, $biblioitemnumber ); @@ -243,7 +252,7 @@ sub AddBiblio { =head2 ModBiblio - ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink); + ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink, $plugins); Replace an existing bib record identified by C<$biblionumber> with one supplied by the MARC::Record object C<$record>. The embedded @@ -263,12 +272,16 @@ Unless C<$disable_autolink> is passed ModBiblio will relink record headings to authorities based on settings in the system preferences. This flag allows us to not relink records when the authority linker is saving modifications. +The C parameter, expects an arrayref of plugin objects, to be +passed to the plugin hooks to avoid unnecessary queries in the context +of batch operations (e.g. bulkmarcimport.pl) + Returns 1 on success 0 on failure =cut sub ModBiblio { - my ( $record, $biblionumber, $frameworkcode, $disable_autolink ) = @_; + my ( $record, $biblionumber, $frameworkcode, $disable_autolink, $plugins ) = @_; if (!$record) { carp 'No record passed to ModBiblio'; return 0; @@ -322,7 +335,7 @@ sub ModBiblio { _koha_modify_biblio( $dbh, $oldbiblio, $frameworkcode ); _koha_modify_biblioitem_nonmarc( $dbh, $oldbiblio ); - _after_biblio_action_hooks({ action => 'modify', biblio_id => $biblionumber }); + _after_biblio_action_hooks({ action => 'modify', biblio_id => $biblionumber, plugins => $plugins }); # update OAI-PMH sets if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { @@ -366,10 +379,14 @@ Checks to make sure that the biblio has no items attached. return: C<$error> : undef unless an error occurs +The C option, expects an arrayref of plugin objects, to be +passed to the plugin hooks to avoid unnecessary queries in the context +of batch operations. + =cut sub DelBiblio { - my ($biblionumber) = @_; + my ($biblionumber, $plugins) = @_; my $biblio = Koha::Biblios->find( $biblionumber ); return unless $biblio; # Should we throw an exception instead? @@ -424,7 +441,7 @@ sub DelBiblio { # from being generated by _koha_delete_biblioitems $error = _koha_delete_biblio( $dbh, $biblionumber ); - _after_biblio_action_hooks({ action => 'delete', biblio_id => $biblionumber }); + _after_biblio_action_hooks({ action => 'delete', biblio_id => $biblionumber, plugins => $plugins }); logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); @@ -3460,12 +3477,22 @@ sub _after_biblio_action_hooks { my $biblio_id = $args->{biblio_id}; my $action = $args->{action}; + my $plugins = $args->{plugins}; if ( C4::Context->preference('UseKohaPlugins') && C4::Context->config("enable_plugins") ) { - my @plugins = Koha::Plugins->new->GetPlugins({ - method => 'after_biblio_action', - }); + my @plugins; + + if ( $plugins and + ref($plugins) eq 'ARRAY' ) { + @plugins = grep { $_->can('after_biblio_action') } @{ $plugins }; + } + else { + # We haven't been passed a plugins list, get it ourselves + @plugins = Koha::Plugins->new->GetPlugins({ + method => 'after_biblio_action', + }); + } if (@plugins) { diff --git a/C4/Items.pm b/C4/Items.pm index 797bda6244..b5e7da94c1 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -168,8 +168,8 @@ sub AddItemFromMarc { =head2 AddItem - my ($biblionumber, $biblioitemnumber, $itemnumber) - = AddItem($item, $biblionumber[, $dbh, $frameworkcode, $unlinked_item_subfields]); + my ( $biblionumber, $biblioitemnumber, $itemnumber ) + = AddItem( $item, $biblionumber[ , $dbh, $frameworkcode, $unlinked_item_subfields, $plugins ] ); Given a hash containing item column names as keys, create a new Koha item record. @@ -178,13 +178,17 @@ The first two optional parameters (C<$dbh> and C<$frameworkcode>) do not need to be supplied for general use; they exist simply to allow them to be picked up from AddItemFromMarc. -The final optional parameter, C<$unlinked_item_subfields>, contains +The optional C<$unlinked_item_subfields> parameter contains an arrayref containing subfields present in the original MARC representation of the item (e.g., from the item editor) that are not mapped to C columns directly but should instead be stored in C and included in the biblio items tag for display and indexing. +The optional C<$plugins> parameter contains an arrayref containing +a list of plugin objects. If it is not passed they will get queried +on the plugin hooks. + =cut sub AddItem { @@ -197,6 +201,7 @@ sub AddItem { if (@_) { $unlinked_item_subfields = shift; } + my $plugins = shift; # needs old biblionumber and biblioitemnumber $item->{'biblionumber'} = $biblionumber; @@ -225,7 +230,7 @@ sub AddItem { logaction( "CATALOGUING", "ADD", $itemnumber, "item" ) if C4::Context->preference("CataloguingLog"); - _after_item_action_hooks({ action => 'create', item_id => $itemnumber }); + _after_item_action_hooks({ action => 'create', item_id => $itemnumber, plugins => $plugins }); return ( $item->{biblionumber}, $item->{biblioitemnumber}, $itemnumber ); } @@ -443,8 +448,9 @@ ModItem( $biblionumber, $itemnumber, { - [ unlinked_item_subfields => $unlinked_item_subfields, ] - [ log_action => 1, ] + [ unlinked_item_subfields => $unlinked_item_subfields, ], + [ log_action => 1, ], + [ plugins => [ $plugin_1, ... ] ] } ); @@ -471,12 +477,16 @@ and set the value. If log_action is set to false, the action will not be logged. If log_action is true or undefined, the action will be logged. +If plugins is passed an arrayref, then it is passed to the plugin +hooks so no plugin query needs to take place. + =cut sub ModItem { my ( $item, $biblionumber, $itemnumber, $additional_params ) = @_; my $log_action = $additional_params->{log_action} // 1; my $unlinked_item_subfields = $additional_params->{unlinked_item_subfields}; + my $plugins = $additional_params->{plugins}; return unless %$item; $item->{'itemnumber'} = $itemnumber or return; @@ -531,7 +541,7 @@ sub ModItem { # item status is possible ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); - _after_item_action_hooks({ action => 'modify', item_id => $itemnumber }); + _after_item_action_hooks({ action => 'modify', item_id => $itemnumber, plugins => $plugins }); logaction( "CATALOGUING", "MODIFY", $itemnumber, "item " . Dumper($item) ) if $log_action && C4::Context->preference("CataloguingLog"); @@ -592,7 +602,7 @@ sub ModDateLastSeen { =head2 DelItem - DelItem({ itemnumber => $itemnumber, [ biblionumber => $biblionumber ] } ); + DelItem({ itemnumber => $itemnumber, [ biblionumber => $biblionumber, plugins => [ $plugin_1, ... ] ] } ); Exported function (core API) for deleting an item record in Koha. @@ -603,6 +613,7 @@ sub DelItem { my $itemnumber = $params->{itemnumber}; my $biblionumber = $params->{biblionumber}; + my $plugins = $params->{plugins}; unless ($biblionumber) { my $item = Koha::Items->find( $itemnumber ); @@ -617,7 +628,7 @@ sub DelItem { ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); - _after_item_action_hooks({ action => 'delete', item_id => $itemnumber }); + _after_item_action_hooks({ action => 'delete', item_id => $itemnumber, plugins => $plugins }); #search item field code logaction("CATALOGUING", "DELETE", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); @@ -2653,12 +2664,22 @@ sub _after_item_action_hooks { my $item_id = $args->{item_id}; my $action = $args->{action}; + my $plugins = $args->{plugins}; if ( C4::Context->preference('UseKohaPlugins') && C4::Context->config("enable_plugins") ) { - my @plugins = Koha::Plugins->new->GetPlugins({ - method => 'after_item_action', - }); + my @plugins; + + if ( $plugins and + ref($plugins) eq 'ARRAY' ) { + @plugins = grep { $_->can('after_item_action') } @{ $plugins }; + } + else { + # We haven't been passed a plugins list, get it ourselves + @plugins = Koha::Plugins->new->GetPlugins({ + method => 'after_item_action', + }); + } if (@plugins) { -- 2.22.0