From efc14f7354be5b5320f20e59fb4e2d167c4d2f46 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 13 Dec 2021 12:01:24 +0000 Subject: [PATCH] Bug 29685: Reduce item processing by calculating 'items any available' outside of loop See bug 24185, this avoids looping every each item of the record for every item of the record How to reproduce: 1) on freshly installed kohadevbox create/import one book, remember that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up a single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", that rule should match above book items/library/patron, check that rule to have a non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create a direct link by yourself, for example, in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time on old code, densely increasing for every hundred items added. In the case of this solution, it's fast, and time increases a little only, linear. In testing with 100 books I went from ~6.5 seconds to ~3.2 seconds Signed-off-by: Andrew Fuerste-Henry --- opac/opac-reserve.pl | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index a7391e2327..3b6ae8e2a3 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -24,7 +24,7 @@ use CGI qw ( -utf8 ); use C4::Auth qw( get_template_and_user ); use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); use C4::Circulation qw( GetBranchItemRule GetTransfers ); -use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch IsAvailableForItemLevelRequest ); +use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest ); use C4::Biblio qw( GetBiblioData GetFrameworkCode GetMarcBiblio ); use C4::Items qw( GetHostItemsInfo GetItemsInfo ); use C4::Output qw( output_html_with_http_headers ); @@ -470,6 +470,12 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{itemLoop} = []; my $numCopiesAvailable = 0; my $numCopiesOPACAvailable = 0; + # iterating through all items first to check if any of them available + # to pass this value further inside down to IsAvailableForItemLevelRequest to + # it's complicated logic to analyse. + # (before this loop was inside that sub loop so it was O(n^2) ) + my $items_any_available; + $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioNum, patron => $patron }) if $patron; foreach my $itemInfo (@{$biblioData->{itemInfos}}) { my $itemNum = $itemInfo->{itemnumber}; my $item = $visible_items->{$itemNum}; @@ -557,9 +563,11 @@ foreach my $biblioNum (@biblionumbers) { my $branch = GetReservesControlBranch( $itemInfo, $patron_unblessed ); my $policy_holdallowed = !$itemLoopIter->{already_reserved}; + # items_any_available defined outside of the current loop, + # so we avoiding loop inside IsAvailableForItemLevelRequest: $policy_holdallowed &&= - IsAvailableForItemLevelRequest($item, $patron) && - CanItemBeReserved( $borrowernumber, $itemNum )->{status} eq 'OK'; + CanItemBeReserved( $borrowernumber, $itemNum )->{status} eq 'OK' && + IsAvailableForItemLevelRequest($item, $patron, undef, $items_any_available); if ($policy_holdallowed) { my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } ); -- 2.20.1