From d064d66f1507245ddd4f07dc83c476c331e9ddcd Mon Sep 17 00:00:00 2001 From: Andreas Jonsson Date: Wed, 24 Jan 2024 20:09:00 +0100 Subject: [PATCH] Bug 35899: memoize C4::Circulation::GetBranchItemRule when building holds queue Test plan: * Examine patches and make sure they do not alter the functionality (provided there are no unexpected side effects in called subroutines) * Run prove t/db_dependent/HoldsQueue.t --- C4/HoldsQueue.pm | 25 ++++++++++++++++++++++++- t/db_dependent/HoldsQueue.t | 13 +++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index e554b1257e..b15c4af1a2 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -29,6 +29,7 @@ use Koha::Hold::HoldsQueueItems; use Koha::Items; use Koha::Libraries; use Koha::Patrons; +use Koha::Cache::Memory::Lite; use List::Util qw( shuffle ); use List::MoreUtils qw( any ); @@ -297,6 +298,28 @@ to fill a hold request if and only if: =cut +sub _getBranchItemRule { + my ( $branchcode, $itemtype ) = @_; + + my $memory_cache = Koha::Cache::Memory::Lite->get_instance(); + my $key = + '_getBranchItemRule:' + . (defined $branchcode ? $branchcode : '*') . ':' + . (defined $itemtype ? $itemtype : '*'); + + my $result = $memory_cache->get_from_cache($key); + + if ( defined $result ) { + return $result; + } + + $result = C4::Circulation::GetBranchItemRule( $branchcode, $itemtype ); + + $memory_cache->set_in_cache( $key, $result ); + + return $result; +} + sub GetItemsAvailableToFillHoldRequestsForBib { my ($biblionumber, $branches_to_use) = @_; @@ -341,7 +364,7 @@ sub GetItemsAvailableToFillHoldRequestsForBib { my $itm = $sth->fetchall_arrayref({}); return [ grep { - my $rule = C4::Circulation::GetBranchItemRule($_->{homebranch}, $_->{itype}); + my $rule = _getBranchItemRule($_->{homebranch}, $_->{itype}); $_->{holdallowed} = $rule->{holdallowed}; $_->{hold_fulfillment_policy} = $rule->{hold_fulfillment_policy}; } @{$itm} ]; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 7875c69eed..5f5bf095cd 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -20,6 +20,7 @@ use Koha::DateUtils qw( dt_from_string ); use Koha::Items; use Koha::Holds; use Koha::CirculationRules; +use Koha::Cache::Memory::Lite; use t::lib::TestBuilder; use t::lib::Mocks; @@ -35,6 +36,7 @@ my $schema = Koha::Database->schema; $schema->storage->txn_begin; my $dbh = C4::Context->dbh; $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); my $builder = t::lib::TestBuilder->new; @@ -186,6 +188,7 @@ $schema->txn_begin; # Clear out existing rules relating to holdallowed $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); t::lib::Mocks::mock_preference('UseTransportCostMatrix', 0); @@ -306,6 +309,7 @@ $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 ) my $holds_queue; $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rule( { rule_name => 'holdallowed', @@ -343,6 +347,7 @@ is( scalar( @$holds_queue ), 1, "Holds not filled with items from closed librari t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 0); $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rule( { rule_name => 'holdallowed', @@ -370,6 +375,7 @@ t::lib::Mocks::mock_preference( 'HoldsQueueSkipClosed', 0 ); t::lib::Mocks::mock_preference('LocalHoldsPriority', 1); $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rule( { rule_name => 'holdallowed', @@ -646,6 +652,7 @@ $dbh->do(" # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rules( { branchcode => undef, @@ -707,6 +714,7 @@ Koha::Holds->find( $reserve_id )->cancel; # With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rules( { branchcode => undef, @@ -765,6 +773,7 @@ Koha::Holds->find( $reserve_id )->cancel; # With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rules( { branchcode => undef, @@ -856,6 +865,7 @@ $dbh->do(" # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch $dbh->do("DELETE FROM circulation_rules"); +Koha::Cache::Memory::Lite->flush(); Koha::CirculationRules->set_rules( { branchcode => undef, @@ -1207,6 +1217,7 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue $dbh->do("DELETE FROM hold_fill_targets"); $dbh->do("DELETE FROM reserves"); $dbh->do("DELETE FROM circulation_rules"); + Koha::Cache::Memory::Lite->flush(); Koha::Biblios->delete(); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); @@ -1274,6 +1285,7 @@ subtest "Item level holds info is preserved (Bug 25738)" => sub { $dbh->do("DELETE FROM hold_fill_targets"); $dbh->do("DELETE FROM reserves"); $dbh->do("DELETE FROM circulation_rules"); + Koha::Cache::Memory::Lite->flush(); my $library = $builder->build_object({ class => 'Koha::Libraries' }); my $category = $builder->build_object( { class => 'Koha::Patron::Categories', value => {exclude_from_local_holds_priority => 0} }); @@ -1482,6 +1494,7 @@ subtest "Test item group holds" => sub { $dbh->do("DELETE FROM hold_fill_targets"); $dbh->do("DELETE FROM reserves"); $dbh->do("DELETE FROM circulation_rules"); + Koha::Cache::Memory::Lite->flush(); t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 0); t::lib::Mocks::mock_preference('LocalHoldsPriority', 0); -- 2.39.2