From b64f96830bbdd555eeed7137b4b473ac15218f0c Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Wed, 2 Feb 2022 13:53:48 +0100
Subject: [PATCH] Bug 30004: Prevent TooMany from executing too many SQL
 queries

If a maximum number of checkouts allowed is defined in circulation
rules, C4::Circulation::TooMany will loop over all patron's checkouts.
When a patron has several hundreds of checkouts, it can really slow down
the checkout process by several seconds (or even tens of seconds)

This patch does two things:
- Always prefetch item data so that `$c->item` does not execute an
  additional SQL query at every iteration of the loop. Item data is
  always needed at the first line of the loop, so there is really no
  downside for doing this.
- Build the `@types` array only once, out of the checkouts loop. Since
  it does not depend at all on patron's checkouts data, it does not make
  sense to build it inside the loop.

Test plan:
1. Before applying the patch, create a patron with a lot of checkouts.
   I tested with 1000 checkouts, but the slowness should be noticeable
   with less.
2. Make sure you have a circulation rule (one that apply to your patron
   and the item(s) you will check out for testing) with a maximum number
   of checkouts allowed
3. Check out an item for the patron with a lot of checkouts. Measure the
   time it takes.
4. Apply the patch
5. Check out another item (or check in and then check out the same item
   used in step 3). Measure the time it takes and compare it to step 3.
   It should be faster now.
6. Run `prove t/db_dependent/Circulation/TooMany.t`

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
---
 C4/Circulation.pm | 64 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 5537531db2..965e02ca6c 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -475,47 +475,55 @@ sub TooMany {
                 $checkouts = $patron->checkouts; # if branch is the patron's home branch, then count all loans by patron
             } else {
                 $checkouts = $patron->checkouts->search(
-                    { 'item.homebranch' => $maxissueqty_rule->branchcode },
-                    { prefetch          => 'item' } );
+                    { 'item.homebranch' => $maxissueqty_rule->branchcode } );
             }
         } else {
             $checkouts = $patron->checkouts; # if rule is not branch specific then count all loans by patron
         }
+        $checkouts = $checkouts->search(undef, { prefetch => 'item' });
+
         my $sum_checkouts;
         my $rule_itemtype = $maxissueqty_rule->itemtype;
-        while ( my $c = $checkouts->next ) {
-            my $itemtype = $c->item->effective_itemtype;
-            my @types;
-            unless ( $rule_itemtype ) {
-                # 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 ],
-                        itemtype  => { '!=' => undef },
-                        rule_name => 'maxissueqty'
-                    }
-                )->get_column('itemtype');
 
-                next if grep {$_ eq $itemtype} @types;
-            } else {
-                my @types;
-                if ( $parent_maxissueqty_rule ) {
+        my @types;
+        unless ( $rule_itemtype ) {
+            # 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 ],
+                    itemtype  => { '!=' => undef },
+                    rule_name => 'maxissueqty'
+                }
+            )->get_column('itemtype');
+        } else {
+            if ( $parent_maxissueqty_rule ) {
                 # 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 });
-                    @types = $children->get_column('itemtype');
-                    push @types, $parent_type;
-                } elsif ( $child_types ) {
+                my $children = Koha::ItemTypes->search({ parent_type => $parent_type });
+                @types = $children->get_column('itemtype');
+                push @types, $parent_type;
+            } elsif ( $child_types ) {
                 # If we are a parent type, we need to count all child types and our own type
-                    @types = $child_types->get_column('itemtype');
-                    push @types, $type; # And don't forget to count our own types
-                } else { push @types, $type; } # Otherwise only count the specific itemtype
+                @types = $child_types->get_column('itemtype');
+                push @types, $type; # And don't forget to count our own types
+            } else {
+                # Otherwise only count the specific itemtype
+                push @types, $type;
+            }
+        }
+
+        while ( my $c = $checkouts->next ) {
+            my $itemtype = $c->item->effective_itemtype;
 
+            unless ( $rule_itemtype ) {
+                next if grep {$_ eq $itemtype} @types;
+            } else {
                 next unless grep {$_ eq $itemtype} @types;
             }
+
             $sum_checkouts->{total}++;
             $sum_checkouts->{onsite_checkouts}++ if $c->onsite_checkout;
             $sum_checkouts->{itemtype}->{$itemtype}++;
-- 
2.39.5