LP#? Hold targeter refactoring and optimization.

Signed-off-by: Bill Erickson <berickxx@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
index 1f8e098..ae522df 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
@@ -795,8 +795,13 @@ sub get_copies_at_loop_iter {
         }
     }
 
-    $self->log_hold(scalar(@iter_copies). " potential copies with an ".
-        "unfulfilled circ_lib target attempt count of $iter");
+    $self->log_hold(
+        sprintf("%d potential copies at max-loops iteration level $iter. ".
+            "%d remain to be tested at a higher loop iteration level.",
+            scalar(@iter_copies), 
+            scalar(@remaining_copies)
+        )
+    );
 
     return (\@iter_copies, \@remaining_copies);
 }
@@ -819,11 +824,24 @@ sub target_by_org_loops {
         order_by => [{class => 'aufhl', field => 'count'}]
     });
 
-    $self->log_hold(scalar(@$targeted_libs).
-        " libs have been targeted at least once");
+    my $max_tried = 0; # Highest per-lib target attempts.
+    foreach (@$targeted_libs) {
+        $max_tried = $_->{count} if $_->{count} > $max_tried;
+    }
+
+    $self->log_hold("Max lib attempts is $max_tried. ".
+        scalar(@$targeted_libs)." libs have been targeted at least once.");
 
-    my $loop_iter = 0;
-    while ($loop_iter++ < $max_loops) {
+    # $loop_iter represents per-lib target attemtps already made.
+    # When loop_iter equals max loops, all libs with targetable copies
+    # have been targeted the maximum number of times.  loop_iter starts
+    # at 0 to pick up libs that have never been targeted.
+    my $loop_iter = -1;
+    while (++$loop_iter < $max_loops) {
+
+        # Ran out of copies to try before exceeding max target loops.
+        # Nothing else to do here.
+        return undef unless @{$self->copies};
 
         my ($iter_copies, $remaining_copies) = 
             $self->get_copies_at_loop_iter($targeted_libs, $loop_iter);
@@ -842,16 +860,19 @@ sub target_by_org_loops {
         # No targetable copy at the current target loop.
         # Update our current copy set to the not-yet-tested copies.
         $self->copies($remaining_copies);
-
-        # We ran out of copies to try before exceeding max target loops.
-        # Nothing else to do here.
-        return undef unless @{$self->copies};
     }
 
-    # Max target loops has been exceeded.
+    # Avoid canceling the hold with exceeds-loops unless at least one
+    # lib has been targeted max_loops times.  Otherwise, the hold goes
+    # back to waiting for another copy (or retargets its current copy).
+    return undef if $max_tried < $max_loops;
+
+    # At least one lib has been targeted max-loops times and zero 
+    # other copies are targetable.  All options have been exhausted.
     return $self->handle_exceeds_target_loops;
 }
 
+# Cancel the hold, fire the no-target A/T event handler, and exit.
 sub handle_exceeds_target_loops {
     my $self = shift;
     my $e = $self->editor;
@@ -1125,9 +1146,15 @@ sub target {
     return unless $self->filter_copies_in_use;
     return unless $self->filter_closed_date_copies;
 
-    # Set aside the previously targeted copy for later use as needed
+    # Set aside the previously targeted copy for later use as needed.
+    # Code may exit here in retarget_nonviable mode if the existing
+    # current_copy value is still viable.
     return unless $self->inspect_previous_target;
 
+    # Log that the hold was not captured.  Logging this here is
+    # necessary for max-loops calculation.
+    return unless $self->log_unfulfilled_hold;
+
     # Confirm again we have something to work on.  If we have no
     # targetable copies now, there may be a copy that can be recalled.
     return unless $self->handle_no_copies(process_recalls => 1);
@@ -1146,8 +1173,6 @@ sub target {
     # effect of looking for a copy to target.
     return if $hold->cancel_time;
 
-    return unless $self->log_unfulfilled_hold;
-
     return $self->apply_copy_target($copy) if $copy;
 
     # No targetable copy was found.  Fire the no-copy handler.