From 2260a07baa6d045fb78d574025d4784d3d553c15 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/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
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 ab03362e06..a48aac98c8 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -425,16 +425,17 @@ sub TooMany {
     my $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
     my $type = $item_object->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,
@@ -442,10 +443,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,
@@ -454,7 +453,7 @@ sub TooMany {
         }
     );
 
-    my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
+    my $maxonsiteissueqty = Koha::CirculationRules->get_effective_rule_value(
         {
             categorycode => $cat_borrower,
             itemtype     => $type,
@@ -468,19 +467,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
@@ -488,23 +486,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 });
@@ -523,7 +520,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;
@@ -542,20 +539,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;
             }
@@ -599,7 +596,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 };
     }
 
@@ -1926,28 +1923,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 01aa726322..bb0f530f52 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -333,7 +333,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)
@@ -346,16 +346,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(
             {
@@ -439,7 +439,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
@@ -480,8 +480,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');
 
@@ -498,31 +502,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" };
         }
@@ -546,8 +543,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} ) {
@@ -564,31 +561,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};
             }
@@ -598,25 +587,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' };
@@ -2347,13 +2336,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 43b53e17b0..a11e53fdf7 100644
--- a/Koha/Biblio.pm
+++ b/Koha/Biblio.pm
@@ -367,16 +367,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
@@ -390,21 +387,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 a9bbf0fe60..3815f6fa74 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 f96df496fb..f138a36375 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -300,13 +300,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;
 }
 
@@ -541,7 +544,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,
@@ -549,8 +552,6 @@ sub get_opacitemholds_policy {
             rule_name    => 'opacitemholds',
         }
     );
-
-    return $rule ? $rule->rule_value : undef;
 }
 
 =head3 get_onshelfholds_policy
@@ -564,15 +565,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
@@ -627,7 +627,7 @@ sub get_lostreturn_policy {
     my $rules = Koha::CirculationRules->get_effective_rules(
         {
             branchcode => $branch,
-            rules  => ['lostreturn','processingreturn']
+            rules  => ['lostreturn', 'processingreturn']
         }
     );
 
@@ -714,7 +714,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,
@@ -722,12 +722,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 2ad4c3fdcb..c8df57b585 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 2a8eecf9f6..3cabb307f5 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -1049,7 +1049,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,
@@ -1057,8 +1057,6 @@ sub can_request_article {
         }
     );
 
-    my $limit = ($rule) ? $rule->rule_value : undef;
-
     return 1 unless defined $limit;
 
     my $count = Koha::ArticleRequests->search(
@@ -1090,15 +1088,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 ea9ccd5267..4e8bd4aeed 100644
--- a/Koha/REST/V1/Checkouts.pm
+++ b/Koha/REST/V1/Checkouts.pm
@@ -354,19 +354,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 ab390962b2..7ae159ecc0 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->unblessed, $self->patron->unblessed );
     }
 
-    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 5526f7d529..b615c215da 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 $due_date = $timestamp->add( days => $due_interval );
         $checkout->update({ date_due => $due_date });
diff --git a/Koha/Template/Plugin/CirculationRules.pm b/Koha/Template/Plugin/CirculationRules.pm
index 5e00e45638..100a46f1a9 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.30.2