Attachment 164970 Details for Bug 35899 – Bug 35899: memoize C4::Circulation::GetBranchItemRule when building holds queue

[patch] Bug 35899: memoize C4::Circulation::GetBranchItemRule when building holds queue

Bug-35899-memoize-C4CirculationGetBranchItemRule-w.patch (text/plain), 6.25 KB, created by Andreas Jonsson on 2024-04-16 21:40:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Jonsson

Created: 2024-04-16 21:40:45 UTC

Size: 6.25 KB

patch

obsolete

>From 93be7b4a302064d3f38f42e827f3928d533c8ea9 Mon Sep 17 00:00:00 2001
>From: Andreas Jonsson <andreas.jonsson@kreablo.se>
>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 9033032366..9cad532678 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 87da308bdc..4c75a062e5 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

Actions: View | Diff | Splinter Review

Attachments on bug 35899: 161358 | 161359 | 161360 | 164969 | 164970