Bugzilla – Attachment 122426 Details for
Bug 28211
Replace use of call_recursive() with call()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28211: Replace use of call_recursive() with call()
Bug-28211-Replace-use-of-callrecursive-with-call.patch (text/plain), 12.05 KB, created by
Kyle M Hall (khall)
on 2021-06-25 12:01:02 UTC
(
hide
)
Description:
Bug 28211: Replace use of call_recursive() with call()
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2021-06-25 12:01:02 UTC
Size:
12.05 KB
patch
obsolete
>From 1b5ee329aa35bc30ad0c4c431020d7e48342c806 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 23 Apr 2021 11:53:51 -0400 >Subject: [PATCH] Bug 28211: Replace use of call_recursive() with call() > >This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call. > >Test Plan: >1) Apply this patch >2) prove t/db_dependent/Koha/Plugins/Plugins.t >3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >--- > C4/Circulation.pm | 2 +- > C4/SIP/Sip/MsgType.pm | 16 ++++++------ > Koha/Patron.pm | 6 +++-- > Koha/Plugins.pm | 35 --------------------------- > circ/circulation.pl | 5 ++-- > members/memberentry.pl | 4 ++- > opac/sco/sco-main.pl | 2 +- > t/db_dependent/Koha/Plugins/Plugins.t | 22 ++++++++--------- > t/lib/Koha/Plugin/Test.pm | 8 +++--- > 9 files changed, 36 insertions(+), 64 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 80e05e70d4..52273deff5 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -176,7 +176,7 @@ sub barcodedecode { > my ($barcode, $filter) = @_; > my $branch = C4::Context::mybranch(); > $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter; >- ($barcode) = Koha::Plugins->call_recursive('item_barcode_transform', $barcode ); >+ Koha::Plugins->call('item_barcode_transform', \$barcode ); > $filter or return $barcode; # ensure filter is defined, else return untouched barcode > if ($filter eq 'whitespace') { > $barcode =~ s/\s//g; >diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm >index fccdb5b117..3cdf04b9c9 100644 >--- a/C4/SIP/Sip/MsgType.pm >+++ b/C4/SIP/Sip/MsgType.pm >@@ -521,7 +521,7 @@ sub handle_checkout { > $fields = $self->{fields}; > > $patron_id = $fields->{ (FID_PATRON_ID) }; >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > $item_id = $fields->{ (FID_ITEM_ID) }; > my $fee_ack = $fields->{ (FID_FEE_ACK) }; > >@@ -743,7 +743,7 @@ sub handle_block_patron { > $patron_id = $fields->{ (FID_PATRON_ID) }; > $terminal_pwd = $fields->{ (FID_TERMINAL_PWD) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > # Terminal passwords are different from account login > # passwords, but I have no idea what to do with them. So, >@@ -969,7 +969,7 @@ sub handle_patron_info { > $start = $fields->{ (FID_START_ITEM) }; > $end = $fields->{ (FID_END_ITEM) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > $patron = $ils->find_patron($patron_id); > >@@ -1128,7 +1128,7 @@ sub handle_fee_paid { > $fee_id = $fields->{ (FID_FEE_ID) }; > $trans_id = $fields->{ (FID_TRANSACTION_ID) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > $ils->check_inst_id( $inst_id, "handle_fee_paid" ); > >@@ -1326,7 +1326,7 @@ sub handle_patron_enable { > $patron_id = $fields->{ (FID_PATRON_ID) }; > $patron_pwd = $fields->{ (FID_PATRON_PWD) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > siplog( "LOG_DEBUG", "handle_patron_enable: patron_id: '%s', patron_pwd: '%s'", $patron_id, $patron_pwd ); > >@@ -1391,7 +1391,7 @@ sub handle_hold { > $title_id = $fields->{ (FID_TITLE_ID) } || ''; > $fee_ack = $fields->{ (FID_FEE_ACK) } || 'N'; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > if ( $hold_mode eq '+' ) { > $status = $ils->add_hold( $patron_id, $patron_pwd, $item_id, $title_id, $expiry_date, $pickup_locn, $hold_type, $fee_ack ); >@@ -1458,7 +1458,7 @@ sub handle_renew { > $item_props = $fields->{ (FID_ITEM_PROPS) }; > $fee_ack = $fields->{ (FID_FEE_ACK) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > $status = $ils->renew( $patron_id, $patron_pwd, $item_id, $title_id, $no_block, $nb_due_date, $third_party, $item_props, $fee_ack ); > >@@ -1541,7 +1541,7 @@ sub handle_renew_all { > $terminal_pwd = $fields->{ (FID_TERMINAL_PWD) }; > $fee_ack = $fields->{ (FID_FEE_ACK) }; > >- ($patron_id) = Koha::Plugins->call_recursive('patron_barcode_transform', $patron_id ); >+ Koha::Plugins->call('patron_barcode_transform', \$patron_id ); > > $status = $ils->renew_all( $patron_id, $patron_pwd, $fee_ack ); > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 79cbef6771..a5537e9a8e 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -112,7 +112,8 @@ Autogenerate next cardnumber from highest value found in database > sub fixup_cardnumber { > my ( $self ) = @_; > >- my ( $max ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $self->cardnumber ); >+ my $max = $self->cardnumber; >+ Koha::Plugins->call( 'patron_barcode_transform', \$max ); > > $max ||= Koha::Patrons->search({ > cardnumber => {-regexp => '^-?[0-9]+$'} >@@ -201,7 +202,8 @@ sub store { > > $self->trim_whitespaces; > >- my ( $new_cardnumber ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $self->cardnumber ); >+ my $new_cardnumber = $self->cardnumber; >+ Koha::Plugins->call( 'patron_barcode_transform', \$new_cardnumber ); > $self->cardnumber( $new_cardnumber ); > > # Set surname to uppercase if uppercasesurname is true >diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm >index bedd220b6f..36e6090415 100644 >--- a/Koha/Plugins.pm >+++ b/Koha/Plugins.pm >@@ -82,41 +82,6 @@ sub call { > return @responses; > } > >-=head2 call_recursive >- >-Calls a plugin method for all enabled plugins, >-passing the return value from the previous plugin >-to the next one. >- >- @response = Koha::Plugins->call_recursive($method, @args) >- >-=cut >- >-sub call_recursive { >- my ( $class, $method, @args ) = @_; >- >- my @responses; >- if ( C4::Context->config('enable_plugins') ) { >- >- my @plugins = $class->new( { enable_plugins => 1 } )->GetPlugins( { method => $method } ); >- @plugins = grep { $_->can($method) } @plugins; >- >- foreach my $plugin (@plugins) { >- my @response = eval { $plugin->$method(@args) }; >- >- if ($@) { >- warn sprintf( "Plugin error (%s): %s", $plugin->get_metadata->{name}, $@ ); >- next; >- } else { >- @args = @response; >- } >- } >- >- } >- >- return @args; >-} >- > =head2 GetPlugins > > This will return a list of all available plugins, optionally limited by >diff --git a/circ/circulation.pl b/circ/circulation.pl >index bf47a3408e..8fb2595ee7 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -81,7 +81,8 @@ my $autoswitched; > my $borrowernumber = $query->param('borrowernumber'); > > if (C4::Context->preference("AutoSwitchPatron") && $barcode) { >- my ( $new_barcode ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $findborrower ); >+ my $new_barcode = $findborrower; >+ Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); > if (Koha::Patrons->search( { cardnumber => $new_barcode} )->count() > 0) { > $findborrower = $barcode; > undef $barcode; >@@ -227,7 +228,7 @@ if ( @$barcodes == 0 && $charges eq 'yes' ) { > # > my $message; > if ($findborrower) { >- ( $findborrower ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $findborrower ); >+ Koha::Plugins->call( 'patron_barcode_transform', \$findborrower ); > my $patron = Koha::Patrons->find( { cardnumber => $findborrower } ); > if ( $patron ) { > $borrowernumber = $patron->borrowernumber; >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 0828893200..96eb68f7fb 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -325,7 +325,9 @@ if ($op eq 'save' || $op eq 'insert'){ > # If the cardnumber is blank, treat it as null. > $newdata{'cardnumber'} = undef if $newdata{'cardnumber'} =~ /^\s*$/; > >- my ( $new_barcode ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $newdata{'cardnumber'} ); >+ my $new_barcode = $newdata{'cardnumber'}; >+ Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode ); >+ > $newdata{'cardnumber'} = $new_barcode; > > if (my $error_code = checkcardnumber($newdata{cardnumber},$newdata{borrowernumber})){ >diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl >index 1b627c4a1c..018f2d7b03 100755 >--- a/opac/sco/sco-main.pl >+++ b/opac/sco/sco-main.pl >@@ -123,7 +123,7 @@ if (C4::Context->preference('SelfCheckoutByLogin') && !$patronid) { > > my ( $borrower, $patron ); > if ( $patronid ) { >- ( $patronid ) = Koha::Plugins->call_recursive( 'patron_barcode_transform', $patronid ); >+ Koha::Plugins->call( 'patron_barcode_transform', \$patronid ); > $patron = Koha::Patrons->find( { cardnumber => $patronid } ); > $borrower = $patron->unblessed if $patron; > } >diff --git a/t/db_dependent/Koha/Plugins/Plugins.t b/t/db_dependent/Koha/Plugins/Plugins.t >index 52938ad7d1..a5ad2010e5 100755 >--- a/t/db_dependent/Koha/Plugins/Plugins.t >+++ b/t/db_dependent/Koha/Plugins/Plugins.t >@@ -78,8 +78,8 @@ subtest 'call() tests' => sub { > $schema->storage->txn_rollback; > }; > >-subtest 'call_recursive() tests' => sub { >- plan tests => 6; >+subtest 'more call() tests' => sub { >+ plan tests => 3; > > $schema->storage->txn_begin; > # Temporarily remove any installed plugins data >@@ -92,18 +92,18 @@ subtest 'call_recursive() tests' => sub { > $plugin->enable(); > } > >- my (@responses) = Koha::Plugins->call_recursive('item_barcode_transform', 1); >- is( scalar @responses, 1, "Got back one element" ); >- is( $responses[0], 4, "Got expected response" ); >+ my $bc = 1; >+ Koha::Plugins->call('item_barcode_transform', \$bc); >+ is( $bc, 4, "Got expected response" ); > >- (@responses) = Koha::Plugins->call_recursive('item_barcode_transform', 'abcd'); >- is( scalar @responses, 1, "Got back one element" ); >- is( $responses[0], 'abcd', "Got expected response" ); >+ my $cn = 'abcd'; >+ Koha::Plugins->call('item_barcode_transform', \$cn); >+ is( $cn, 'abcd', "Got expected response" ); > > t::lib::Mocks::mock_config('enable_plugins', 0); >- (@responses) = Koha::Plugins->call_recursive('item_barcode_transform', 1); >- is( scalar @responses, 1, "Got back one element" ); >- is( $responses[0], 1, "call_recursive should return the original arguments if plugins are disabled" ); >+ $bc = 1; >+ Koha::Plugins->call('item_barcode_transform', \$bc); >+ is( $bc, 1, "call should return the original arguments if plugins are disabled" ); > > $schema->storage->txn_rollback; > }; >diff --git a/t/lib/Koha/Plugin/Test.pm b/t/lib/Koha/Plugin/Test.pm >index 00af419010..00a3d7171c 100644 >--- a/t/lib/Koha/Plugin/Test.pm >+++ b/t/lib/Koha/Plugin/Test.pm >@@ -95,13 +95,15 @@ sub intranet_js { > > sub item_barcode_transform { > my ( $self, $barcode ) = @_; >- Koha::Exceptions::Exception->throw("item_barcode_transform called with parameter: $barcode"); >+ my $param = $$barcode; >+ $$barcode = 4 if "$$barcode" eq 1; >+ Koha::Exceptions::Exception->throw("item_barcode_transform called with parameter: $param"); > } > > sub patron_barcode_transform { > my ( $self, $barcode ) = @_; >- $barcode //= ''; >- Koha::Exceptions::Exception->throw("patron_barcode_transform called with parameter: $barcode"); >+ $$barcode //= ''; >+ Koha::Exceptions::Exception->throw("patron_barcode_transform called with parameter: $$barcode"); > } > > sub configure { >-- >2.30.1 (Apple Git-130)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=122426">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28211
:
120120
|
120875
|
120882
|
120883
|
122425
|
122426
|
122427
|
125892
|
125926
|
125927
|
125928
|
126274
|
126471
|
126497