From 7d525d9ff97df189175f42dc4a7ac6259677e80a Mon Sep 17 00:00:00 2001
From: David Gustafsson <david.gustafsson@ub.gu.se>
Date: Thu, 3 Nov 2022 14:26:34 +0100
Subject: [PATCH] Bug 32092: Improve circulation rules cache utilization

To test:
1) Go to the page for placing a hold for a biblio with a large
   number of items
2) Depending on the number of items this page can be very slow
   to load
3) Ensure the following tests pass:
   t/db_dependent/Circulation.t
   t/db_dependent/Reserves.t
   t/db_dependent/Biblio.t
   t/db_dependent/Koha/Charges/Fees.t
   t/db_dependent/Items.t
   t/db_dependent/Koha/Patron.t
   t/db_dependent/api/v1/checkouts.t
   t/db_dependent/Koha/Template/Plugin/CirculationRules.t
   t/db_dependent/Template/Plugin/CirculationRules.t
   t/db_dependent/Koha/CirculationRules.t
   t/db_dependent/Circulation/Branch.t
   t/db_dependent/Circulation/TooMany.t
3) Apply the patch
4) The page loading time should now be significantly faster
5) Ensure all the tests in 3) still pass

Sponsored-by: Gothenburg University Library
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Circulation.pm                        | 79 ++++++++-----------
 C4/Reserves.pm                           | 96 +++++++++++-------------
 Koha/Biblio.pm                           | 18 ++---
 Koha/Charges/Fees.pm                     |  7 +-
 Koha/CirculationRules.pm                 | 37 ++++-----
 Koha/Item.pm                             |  5 +-
 Koha/Patron.pm                           | 10 +--
 Koha/REST/V1/Checkouts.pm                |  6 +-
 Koha/Recall.pm                           | 10 +--
 Koha/Recalls.pm                          |  5 +-
 Koha/Template/Plugin/CirculationRules.pm |  4 +-
 11 files changed, 112 insertions(+), 165 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 2ebe5cbe6e3..6f92b3e6001 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -423,16 +423,17 @@ sub TooMany {
     my $branch = _GetCircControlBranch($item, $patron);
     my $type = $item->effective_itemtype;
 
-    my ($type_object, $parent_type, $parent_maxissueqty_rule);
-    $type_object = Koha::ItemTypes->find( $type );
-    $parent_type = $type_object->parent_type if $type_object;
+    my $type_object = Koha::ItemTypes->find( $type );
+    my $parent_type = $type_object->parent_type if $type_object;
     my $child_types = Koha::ItemTypes->search({ parent_type => $type });
     # Find any children if we are a parent_type;
 
     # given branch, patron category, and item type, determine
     # applicable issuing rule
 
-    $parent_maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
+    # If the parent rule is for default type we discount it
+    my $parent_maxissueqty;
+    $parent_maxissueqty = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $cat_borrower,
             itemtype     => $parent_type,
@@ -440,10 +441,8 @@ sub TooMany {
             rule_name    => 'maxissueqty',
         }
     ) if $parent_type;
-    # If the parent rule is for default type we discount it
-    $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && !defined $parent_maxissueqty_rule->itemtype;
 
-    my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule(
+    my $maxissueqty = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $cat_borrower,
             itemtype     => $type,
@@ -452,7 +451,7 @@ sub TooMany {
         }
     );
 
-    my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
+    my $maxonsiteissueqty = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $cat_borrower,
             itemtype     => $type,
@@ -464,19 +463,18 @@ sub TooMany {
     # if a rule is found and has a loan limit set, count
     # how many loans the patron already has that meet that
     # rule
-    if (defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne "") {
+    if (defined $maxissueqty and $maxissueqty ne "") {
 
         my $checkouts;
-        if ( $maxissueqty_rule->branchcode ) {
+        if ( $branch ) {
             if ( C4::Context->preference('CircControl') eq 'PickupLibrary' ) {
                 $checkouts = $patron->checkouts->search(
-                    { 'me.branchcode' => $maxissueqty_rule->branchcode } );
+                    { 'me.branchcode' => $branch } );
             } elsif (C4::Context->preference('CircControl') eq 'PatronLibrary') {
                 $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
             } else {
                 my $branch_type = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
-                $checkouts = $patron->checkouts->search(
-                    { "item.$branch_type" => $maxissueqty_rule->branchcode } );
+                $checkouts = $patron->checkouts->search( { "item.$branch_type" => $branch } );
             }
         } else {
             $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron
@@ -484,23 +482,22 @@ sub TooMany {
         $checkouts = $checkouts->search(undef, { prefetch => 'item' });
 
         my $sum_checkouts;
-        my $rule_itemtype = $maxissueqty_rule->itemtype;
 
         my @types;
-        unless ( $rule_itemtype ) {
+        unless ( $type ) {
             # matching rule has the default item type, so count only
             # those existing loans that don't fall under a more
             # specific rule
             @types = Koha::CirculationRules->search(
                 {
-                    branchcode => $maxissueqty_rule->branchcode,
-                    categorycode => [ $maxissueqty_rule->categorycode, $cat_borrower ],
+                    branchcode => $branch,
+                    categorycode => $cat_borrower,
                     itemtype  => { '!=' => undef },
                     rule_name => 'maxissueqty'
                 }
             )->get_column('itemtype');
         } else {
-            if ( $parent_maxissueqty_rule ) {
+            if ( $parent_maxissueqty ) {
                 # if we have a parent item type then we count loans of the
                 # specific item type or its siblings or parent
                 my $children = Koha::ItemTypes->search({ parent_type => $parent_type });
@@ -519,7 +516,7 @@ sub TooMany {
         while ( my $c = $checkouts->next ) {
             my $itemtype = $c->item->effective_itemtype;
 
-            unless ( $rule_itemtype ) {
+            unless ( $type ) {
                 next if grep {$_ eq $itemtype} @types;
             } else {
                 next unless grep {$_ eq $itemtype} @types;
@@ -538,20 +535,20 @@ sub TooMany {
             checkout_count               => $checkout_count,
             onsite_checkout_count        => $onsite_checkout_count,
             onsite_checkout              => $onsite_checkout,
-            max_checkouts_allowed        => $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef,
-            max_onsite_checkouts_allowed => $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef,
+            max_checkouts_allowed        => $maxissueqty,,
+            max_onsite_checkouts_allowed => $maxonsiteissueqty,
             switch_onsite_checkout       => $switch_onsite_checkout,
         };
         # If parent rules exists
-        if ( defined($parent_maxissueqty_rule) and defined($parent_maxissueqty_rule->rule_value) ){
-            $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty_rule ? $parent_maxissueqty_rule->rule_value : undef;
+        if ( defined $parent_maxissueqty ){
+            $checkout_rules->{max_checkouts_allowed} = $parent_maxissueqty;
             my $qty_over = _check_max_qty($checkout_rules);
             return $qty_over if defined $qty_over;
 
             # If the parent rule is less than or equal to the child, we only need check the parent
-            if( $maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value && defined($maxissueqty_rule->itemtype) ) {
+            if( $maxissueqty < $parent_maxissueqty && defined($type) ) {
                 $checkout_rules->{checkout_count} = $checkout_count_type;
-                $checkout_rules->{max_checkouts_allowed} = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
+                $checkout_rules->{max_checkouts_allowed} = $maxissueqty;
                 my $qty_over = _check_max_qty($checkout_rules);
                 return $qty_over if defined $qty_over;
             }
@@ -595,7 +592,7 @@ sub TooMany {
         return $qty_over if defined $qty_over;
     }
 
-    if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) {
+    unless ( defined $maxissueqty || defined $branch_borrower_circ_rule->{patron_maxissueqty} ) {
         return { reason => 'NO_RULE_DEFINED', max_allowed => 0 };
     }
 
@@ -1922,28 +1919,14 @@ wildcards.
 
 sub GetBranchBorrowerCircRule {
     my ( $branchcode, $categorycode ) = @_;
-
-    # Initialize default values
-    my $rules = {
-        patron_maxissueqty       => undef,
-        patron_maxonsiteissueqty => undef,
-    };
-
-    # Search for rules!
-    foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) {
-        my $rule = Koha::CirculationRules->get_effective_rule(
-            {
-                categorycode => $categorycode,
-                itemtype     => undef,
-                branchcode   => $branchcode,
-                rule_name    => $rule_name,
-            }
-        );
-
-        $rules->{$rule_name} = $rule->rule_value if defined $rule;
-    }
-
-    return $rules;
+    return Koha::CirculationRules->get_effective_rules(
+        {
+            categorycode => $categorycode,
+            itemtype     => undef,
+            branchcode   => $branchcode,
+            rules => ['patron_maxissueqty', 'patron_maxonsiteissueqty']
+        }
+    );
 }
 
 =head2 GetBranchItemRule
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 36257378852..6c3ed96f65a 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -332,7 +332,7 @@ See CanItemBeReserved() for possible return values.
 
 =cut
 
-sub CanBookBeReserved{
+sub CanBookBeReserved {
     my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_;
 
     # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
@@ -345,16 +345,16 @@ sub CanBookBeReserved{
 
         # biblio-level, item type-contrained
         my $patron          = Koha::Patrons->find($borrowernumber);
-        my $reservesallowed = Koha::CirculationRules->get_effective_rule(
+        my $reservesallowed = Koha::CirculationRules->get_effective_rule_value(
             {
                 itemtype     => $params->{itemtype},
                 categorycode => $patron->categorycode,
                 branchcode   => $pickup_branchcode,
                 rule_name    => 'reservesallowed',
             }
-        )->rule_value;
+        );
 
-        $reservesallowed = ( $reservesallowed eq '' ) ? undef : $reservesallowed;
+        $reservesallowed = $reservesallowed eq '' ? undef : $reservesallowed;
 
         my $count = $patron->holds->search(
             {
@@ -438,7 +438,7 @@ sub CanItemBeReserved {
     }
 
     my $dbh = C4::Context->dbh;
-    my $ruleitemtype;    # itemtype of the matching issuing rule
+    my $effective_itemtype = $item->effective_itemtype;
     my $allowedreserves  = 0; # Total number of holds allowed across all records, default to none
 
     # We check item branch if IndependentBranches is ON
@@ -479,8 +479,12 @@ sub CanItemBeReserved {
     }
 
     # check if a recall exists on this item from this borrower
-    return _cache { status => 'recall' }
-      if $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count;
+    if (
+        C4::Context->preference('UseRecalls') &&
+        $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count
+    ) {
+        return _cache { status => 'recall' };
+    }
 
     my $controlbranch = C4::Context->preference('ReservesControlBranch');
 
@@ -497,31 +501,24 @@ sub CanItemBeReserved {
     }
 
     # we retrieve rights
-    if (
-        my $reservesallowed = Koha::CirculationRules->get_effective_rule({
-                itemtype     => $item->effective_itemtype,
-                categorycode => $borrower->{categorycode},
-                branchcode   => $reserves_control_branch,
-                rule_name    => 'reservesallowed',
-        })
-    ) {
-        $ruleitemtype     = $reservesallowed->itemtype;
-        $allowedreserves  = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited
-    }
-    else {
-        $ruleitemtype = undef;
-    }
+    #undefined is 0, blank is unlimited
+    $allowedreserves = Koha::CirculationRules->get_effective_rule_value({
+        itemtype     => $effective_itemtype,
+        categorycode => $borrower->{categorycode},
+        branchcode   => $reserves_control_branch,
+        rule_name    => 'reservesallowed'
+    }) // 0;
 
     my $rights = Koha::CirculationRules->get_effective_rules({
         categorycode => $borrower->{'categorycode'},
-        itemtype     => $item->effective_itemtype,
+        itemtype     => $effective_itemtype,
         branchcode   => $reserves_control_branch,
-        rules        => ['holds_per_record','holds_per_day']
+        rules        => ['holds_per_record', 'holds_per_day']
     });
     my $holds_per_record = $rights->{holds_per_record} // 1;
     my $holds_per_day    = $rights->{holds_per_day};
 
-    if (   defined $holds_per_record && $holds_per_record ne '' ){
+    if ( defined $holds_per_record && $holds_per_record ne '' ){
         if ( $holds_per_record == 0 ) {
             return _cache { status => "noReservesAllowed" };
         }
@@ -545,8 +542,8 @@ sub CanItemBeReserved {
     }
 
     # we check if it's ok or not
-    if ( defined $allowedreserves && $allowedreserves ne '' ){
-        if( $allowedreserves == 0 ){
+    if ( $allowedreserves ne '' ) {
+        if( $allowedreserves == 0 ) {
             return _cache { status => 'noReservesAllowed' };
         }
         if ( !$params->{ignore_hold_counts} ) {
@@ -563,31 +560,23 @@ sub CanItemBeReserved {
 
             # If using item-level itypes, fall back to the record
             # level itemtype if the hold has no associated item
-            if ( defined $ruleitemtype ) {
-                if ( C4::Context->preference('item-level_itypes') ) {
-                    $querycount .= q{
-                        AND ( COALESCE( items.itype, biblioitems.itemtype ) = ?
-                           OR reserves.itemtype = ? )
-                    };
-                }
-                else {
-                    $querycount .= q{
-                        AND ( biblioitems.itemtype = ?
-                           OR reserves.itemtype = ? )
-                    };
-                }
-            }
-
-            my $sthcount = $dbh->prepare($querycount);
-
-            if ( defined $ruleitemtype ) {
-                $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype, $ruleitemtype );
+            if ( C4::Context->preference('item-level_itypes') ) {
+                $querycount .= q{
+                    AND ( COALESCE( items.itype, biblioitems.itemtype ) = ?
+                    OR reserves.itemtype = ? )
+                };
             }
             else {
-                $sthcount->execute( $patron->borrowernumber, $reserves_control_branch );
+                $querycount .= q{
+                    AND ( biblioitems.itemtype = ?
+                    OR reserves.itemtype = ? )
+                };
             }
 
-            my $reservecount = "0";
+            my $sthcount = $dbh->prepare($querycount);
+            $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $effective_itemtype, $effective_itemtype );
+
+            my $reservecount = 0;
             if ( my $rowcount = $sthcount->fetchrow_hashref() ) {
                 $reservecount = $rowcount->{count};
             }
@@ -597,25 +586,25 @@ sub CanItemBeReserved {
     }
 
     # Now we need to check hold limits by patron category
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    my $max_holds = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $patron->categorycode,
             branchcode   => $reserves_control_branch,
             rule_name    => 'max_holds',
         }
     );
-    if (!$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) {
+    if (!$params->{ignore_hold_counts} && defined $max_holds && $max_holds ne '' ) {
         my $total_holds_count = Koha::Holds->search(
             {
                 borrowernumber => $patron->borrowernumber
             }
         )->count();
 
-        return _cache { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value;
+        return _cache { status => 'tooManyReserves', limit => $max_holds } if $total_holds_count >= $max_holds;
     }
 
     my $branchitemrule =
-      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
+      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $effective_itemtype );
 
     if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) {
         return _cache { status => 'notReservable' };
@@ -2281,13 +2270,12 @@ sub GetMaxPatronHoldsForRecord {
 
         $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" );
 
-        my $rule = Koha::CirculationRules->get_effective_rule({
+        my $holds_per_record = Koha::CirculationRules->get_effective_rule_value({
             categorycode => $categorycode,
             itemtype     => $itemtype,
             branchcode   => $branchcode,
             rule_name    => 'holds_per_record'
-        });
-        my $holds_per_record = $rule ? $rule->rule_value : 0;
+        }) // 0;
         $max = $holds_per_record if $holds_per_record > $max;
     }
 
diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
index 7f0f2ecb4f6..6cc40d7f5db 100644
--- a/Koha/Biblio.pm
+++ b/Koha/Biblio.pm
@@ -369,16 +369,13 @@ $borrower must be a Koha::Patron object
 sub article_request_type {
     my ( $self, $borrower ) = @_;
 
-    return q{} unless $borrower;
+    return unless $borrower;
 
-    my $rule = $self->article_request_type_for_items( $borrower );
-    return $rule if $rule;
+    my $type = $self->article_request_type_for_items( $borrower );
+    return $type if $type;
 
     # If the record has no items that are requestable, go by the record itemtype
-    $rule = $self->article_request_type_for_bib($borrower);
-    return $rule if $rule;
-
-    return q{};
+    return $self->article_request_type_for_bib($borrower);
 }
 
 =head3 article_request_type_for_bib
@@ -392,21 +389,18 @@ Returns the article request type 'yes', 'no', 'item_only', 'bib_only', for the g
 sub article_request_type_for_bib {
     my ( $self, $borrower ) = @_;
 
-    return q{} unless $borrower;
+    return unless $borrower;
 
     my $borrowertype = $borrower->categorycode;
     my $itemtype     = $self->itemtype();
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    return Koha::CirculationRules->get_effective_rule_value(
         {
             rule_name    => 'article_requests',
             categorycode => $borrowertype,
             itemtype     => $itemtype,
         }
     );
-
-    return q{} unless $rule;
-    return $rule->rule_value || q{}
 }
 
 =head3 article_request_type_for_items
diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm
index a9bbf0fe601..3815f6fa747 100644
--- a/Koha/Charges/Fees.pm
+++ b/Koha/Charges/Fees.pm
@@ -89,8 +89,8 @@ sub new {
 sub accumulate_rentalcharge {
     my ($self) = @_;
 
-    my $itemtype        = Koha::ItemTypes->find( $self->item->effective_itemtype );
-    my $lengthunit_rule = Koha::CirculationRules->get_effective_rule(
+    my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype );
+    my $units = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $self->patron->categorycode,
             itemtype     => $itemtype->id,
@@ -98,9 +98,8 @@ sub accumulate_rentalcharge {
             rule_name    => 'lengthunit',
         }
     );
-    return 0 unless $lengthunit_rule;
+    return 0 unless $units;
 
-    my $units = $lengthunit_rule->rule_value;
     my $rentalcharge_increment =
       ( $units eq 'days' )
       ? $itemtype->rentalcharge_daily
diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index c29b83d0413..180ea807765 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -307,13 +307,16 @@ sub get_effective_rule_value {
     my $cache_key = sprintf "CircRules:%s:%s:%s:%s", $rule_name // q{},
       $categorycode // q{}, $branchcode // q{}, $itemtype // q{};
 
-    my $cached       = $memory_cache->get_from_cache($cache_key);
-    return $cached if $cached;
-
-    my $rule = $self->get_effective_rule($params);
-
-    my $value= $rule ? $rule->rule_value : undef;
-    $memory_cache->set_in_cache( $cache_key, $value );
+    my $value = $memory_cache->get_from_cache($cache_key);
+    unless (defined $value) {
+        my $rule = $self->get_effective_rule($params);
+        if ($rule) {
+            $value = $rule->rule_value;
+        }
+        $value //= 'undef';
+        $memory_cache->set_in_cache( $cache_key, $value );
+    }
+    return if $value eq 'undef';
     return $value;
 }
 
@@ -551,7 +554,7 @@ sub get_opacitemholds_policy {
 
     return unless $item or $patron;
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    return Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $patron->categorycode,
             itemtype     => $item->effective_itemtype,
@@ -559,8 +562,6 @@ sub get_opacitemholds_policy {
             rule_name    => 'opacitemholds',
         }
     );
-
-    return $rule ? $rule->rule_value : undef;
 }
 
 =head3 get_onshelfholds_policy
@@ -574,15 +575,14 @@ sub get_onshelfholds_policy {
     my $item = $params->{item};
     my $itemtype = $item->effective_itemtype;
     my $patron = $params->{patron};
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    return Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => ( $patron ? $patron->categorycode : undef ),
             itemtype     => $itemtype,
             branchcode   => $item->holdingbranch,
             rule_name    => 'onshelfholds',
         }
-    );
-    return $rule ? $rule->rule_value : 0;
+    ) // 0;
 }
 
 =head3 get_lostreturn_policy
@@ -637,7 +637,7 @@ sub get_lostreturn_policy {
     my $rules = Koha::CirculationRules->get_effective_rules(
         {
             branchcode => $branch,
-            rules  => ['lostreturn','processingreturn']
+            rules  => ['lostreturn', 'processingreturn']
         }
     );
 
@@ -724,7 +724,7 @@ sub get_effective_daysmode {
     my $itemtype         = $params->{itemtype};
     my $branchcode       = $params->{branchcode};
 
-    my $daysmode_rule = $class->get_effective_rule(
+    my $daysmode = $class->get_effective_rule_value(
         {
             categorycode => $categorycode,
             itemtype     => $itemtype,
@@ -732,12 +732,7 @@ sub get_effective_daysmode {
             rule_name    => 'daysmode',
         }
     );
-
-    return ( defined($daysmode_rule)
-          and $daysmode_rule->rule_value ne '' )
-      ? $daysmode_rule->rule_value
-      : C4::Context->preference('useDaysMode');
-
+    return $daysmode || C4::Context->preference('useDaysMode');
 }
 
 
diff --git a/Koha/Item.pm b/Koha/Item.pm
index 854b6cfd58c..76844aaf02c 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -818,7 +818,7 @@ sub article_request_type {
       :                                      undef;
     my $borrowertype = $borrower->categorycode;
     my $itemtype = $self->effective_itemtype();
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    return Koha::CirculationRules->get_effective_rule_value(
         {
             rule_name    => 'article_requests',
             categorycode => $borrowertype,
@@ -826,9 +826,6 @@ sub article_request_type {
             branchcode   => $branchcode
         }
     );
-
-    return q{} unless $rule;
-    return $rule->rule_value || q{}
 }
 
 =head3 current_holds
diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index 73212772340..cb5419c1cd6 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -1217,7 +1217,7 @@ sub can_request_article {
 
     $library_id //= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    my $limit = Koha::CirculationRules->get_effective_rule_value(
         {
             branchcode   => $library_id,
             categorycode => $self->categorycode,
@@ -1225,8 +1225,6 @@ sub can_request_article {
         }
     );
 
-    my $limit = ($rule) ? $rule->rule_value : undef;
-
     return 1 unless defined $limit;
 
     my $count = Koha::ArticleRequests->search(
@@ -1258,15 +1256,13 @@ sub article_request_fee {
 
     $library_id //= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    my $fee = Koha::CirculationRules->get_effective_rule_value(
         {
             branchcode   => $library_id,
             categorycode => $self->categorycode,
             rule_name    => 'article_request_fee'
         }
-    );
-
-    my $fee = ($rule) ? $rule->rule_value + 0 : 0;
+    ) + 0;
 
     return $fee;
 }
diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm
index a7749f1210e..7b6e168e69b 100644
--- a/Koha/REST/V1/Checkouts.pm
+++ b/Koha/REST/V1/Checkouts.pm
@@ -396,19 +396,19 @@ sub allows_renewal {
         my $renewable = Mojo::JSON->false;
         $renewable = Mojo::JSON->true if $can_renew;
 
-        my $rule = Koha::CirculationRules->get_effective_rule(
+        my $renewalsallowed = Koha::CirculationRules->get_effective_rule_value(
             {
                 categorycode => $checkout->patron->categorycode,
                 itemtype     => $checkout->item->effective_itemtype,
                 branchcode   => $checkout->branchcode,
-                rule_name    => 'renewalsallowed',
+                rule_name    => 'renewalsallowed'
             }
         );
         return $c->render(
             status => 200,
             openapi => {
                 allows_renewal => $renewable,
-                max_renewals => $rule->rule_value,
+                max_renewals => $renewalsallowed,
                 current_renewals => $checkout->renewals_count,
                 unseen_renewals => $checkout->unseen_renewals,
                 error => $error
diff --git a/Koha/Recall.pm b/Koha/Recall.pm
index c70acb89dd1..01928001bcb 100644
--- a/Koha/Recall.pm
+++ b/Koha/Recall.pm
@@ -115,13 +115,13 @@ sub checkout {
         my @items = Koha::Items->search({ biblionumber => $self->biblio_id })->as_list;
         my @itemnumbers;
         foreach (@items) {
-            my $recalls_allowed = Koha::CirculationRules->get_effective_rule({
+            my $recalls_allowed = Koha::CirculationRules->get_effective_rule_value({
                 branchcode => C4::Context->userenv->{'branch'},
                 categorycode => $self->patron->categorycode,
                 itemtype => $_->effective_itemtype,
                 rule_name => 'recalls_allowed',
             });
-            if ( defined $recalls_allowed and $recalls_allowed->rule_value > 0 ) {
+            if ( defined $recalls_allowed and $recalls_allowed > 0 ) {
                 push ( @itemnumbers, $_->itemnumber );
             }
         }
@@ -261,14 +261,12 @@ sub calc_expirationdate {
         $branchcode = C4::Circulation::_GetCircControlBranch( $item, $self->patron );
     }
 
-    my $rule = Koha::CirculationRules->get_effective_rule({
+    my $shelf_time = Koha::CirculationRules->get_effective_rule_value({
         categorycode => $self->patron->categorycode,
         branchcode => $branchcode,
         itemtype => $item ? $item->effective_itemtype : undef,
         rule_name => 'recall_shelf_time'
-    });
-
-    my $shelf_time = defined $rule ? $rule->rule_value : C4::Context->preference('RecallsMaxPickUpDelay');
+    }) // C4::Context->preference('RecallsMaxPickUpDelay');
 
     my $expirationdate = dt_from_string->add( days => $shelf_time );
     return $expirationdate;
diff --git a/Koha/Recalls.pm b/Koha/Recalls.pm
index 29be68ce05a..37e407261e0 100644
--- a/Koha/Recalls.pm
+++ b/Koha/Recalls.pm
@@ -134,13 +134,12 @@ sub add_recall {
 
         # get checkout and adjust due date based on circulation rules
         my $checkout = $recall->checkout;
-        my $recall_due_date_interval = Koha::CirculationRules->get_effective_rule({
+        my $due_interval = Koha::CirculationRules->get_effective_rule_value({
             categorycode => $checkout->patron->categorycode,
             itemtype => $checkout->item->effective_itemtype,
             branchcode => $branchcode,
             rule_name => 'recall_due_date_interval',
-        });
-        my $due_interval = defined $recall_due_date_interval ? $recall_due_date_interval->rule_value : 5;
+        }) // 5;
         my $timestamp = dt_from_string( $recall->timestamp );
         my $checkout_timestamp = dt_from_string( $checkout->date_due );
         my $due_date = $timestamp->set(
diff --git a/Koha/Template/Plugin/CirculationRules.pm b/Koha/Template/Plugin/CirculationRules.pm
index 5e00e456380..100a46f1a96 100644
--- a/Koha/Template/Plugin/CirculationRules.pm
+++ b/Koha/Template/Plugin/CirculationRules.pm
@@ -46,7 +46,7 @@ sub Get {
     $categorycode = undef if $categorycode eq q{} or $categorycode eq q{*};
     $itemtype     = undef if $itemtype eq q{}     or $itemtype  eq q{*};
 
-    my $rule = Koha::CirculationRules->get_effective_rule(
+    return Koha::CirculationRules->get_effective_rule_value(
         {
             branchcode   => $branchcode,
             categorycode => $categorycode,
@@ -54,8 +54,6 @@ sub Get {
             rule_name    => $rule_name,
         }
     );
-
-    return $rule->rule_value if $rule;
 }
 
 =head3 Search
-- 
2.42.0