From: Bill Erickson <berickxx@gmail.com>
Date: Wed, 8 Jun 2016 21:36:23 +0000 (-0400)
Subject: hold targeter reify experiment
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=3840ce6d81e9de58f2d7019b7faeb40454290507;p=working%2FEvergreen.git

hold targeter reify experiment

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 9375d67e60..7384ef6921 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm
@@ -131,11 +131,13 @@ sub editor {
 
 sub result {
     my $self = shift;
+
     return {
         hold_id => $self->hold_id,
         error   => $self->error,
         success => $self->success,
-        message => $self->message
+        message => $self->message,
+        targeted_copy => $self->{targeted_copy}
     };
 }
 
@@ -156,6 +158,7 @@ sub copy_prox_map {
 sub exit_targeter {
     my ($self, $msg, $is_error) = @_;
     $self->message($msg);
+    $self->{done} = 1;
 
     # Force a rollback when exiting.
     # This is a no-op if a commit or rollback have already occurred.
@@ -525,7 +528,7 @@ sub inspect_previous_target {
     # no previous target
     return 1 unless my $prev_id = $hold->current_copy;
 
-    $self->{had_previous_copy} = 1;
+    $self->{previous_copy_id} = $prev_id;
 
     # See if the previous copy is in our list of valid copies.
     my ($prev) = grep {$_->{id} eq $prev_id} @copies;
@@ -550,8 +553,13 @@ sub inspect_previous_target {
 # there is no target and returns false, to stop targeting.
 sub handle_no_copies {
     my $self = shift;
+    my $force = shift;
 
-    return 1 if @{$self->copy_hashes} || $self->{valid_previous_copy};
+    if (!$force) {
+        # Force just says don't bother checking the copies, because
+        # other code already has.
+        return 1 if @{$self->copy_hashes} || $self->{valid_previous_copy};
+    }
     
     my $hold = $self->hold;
     $hold->clear_current_copy;
@@ -573,22 +581,154 @@ sub handle_no_copies {
 sub attempt_force_recall_target {
     my $self = shift;
     return $self->copy_hashes->[0] if 
-        $self->hold->hold_type eq 'R' || $self->hold->hold_type = 'F';
+        $self->hold->hold_type eq 'R' || $self->hold->hold_type eq 'F';
     return undef;
 }
 
-sub attempt_min_proxy_copy_target {
+# Copies at the pickup lib are allowed to bypass a lot of extra logic.
+# See if we have any that are targetable.
+sub attempt_local_copy_target {
     my $self = shift;
+
+    my $pu_lib = $self->hold->pickup_lib;
+    my @copies = @{$self->copy_hashes};
+    my @locals = grep {$_->{circ_lib} eq $pu_lib} @copies;
+
+    return undef unless @locals;
+
+    for my $copy (@locals) {
+        return $copy if $self->copy_is_permitted($copy);
+    }
+
+    return undef;
 }
 
 sub attempt_remote_copy_target {
     my $self = shift;
+
+    return undef unless @{$self->copy_hashes};
+
+    return undef unless $self->trim_copies_by_target_loop;
+
+    $self->compile_weighted_proximity_map;
+
+    return $self->find_nearest_copy;
+}
+
+sub trim_copies_by_target_loop {
+    my $self = shift;
+
+    my $max_loops = $self->parent->get_ou_setting(
+        $self->hold->pickup_lib,
+        'circ.holds.max_org_unit_target_loops'
+    );
+
+    return unless defined $max_loops;
+
+    my @copies = @{$self->copy_hashes};
+    my $e = $self->editor;
+
+    my %circ_lib_map =  map {$_->{circ_lib} => 1} @copies;
+    my @circ_lib_list = keys %circ_lib_map;
+
+    # Which target loop iteration are we currently on?
+    my $current_loop = $e->json_query({ 
+        distinct => 1,
+        select => {aufhmxl => ['max']},
+        from => 'aufhmxl',
+        where => {hold => $self->hold_id}
+    })->[0];
+
+    $current_loop = $current_loop ? $current_loop->{max} : 1;
+
+    # List of org units we've already tried targeting 
+    # within the current target loop.
+    my $exclude_libs = $e->json_query({
+        distinct => 1,
+        select => {aufhol => ['circ_lib']},
+        from => 'aufhol',
+        where => {hold => $self->hold_id}
+    });
+
+    my @keep_libs;
+    if (@$exclude_libs) {
+        my %exclude = map {$_->{circ_lib} => 1} @$exclude_libs;
+        for my $lib (@circ_lib_list) {
+            push(@keep_libs, $lib) unless $exclude{$lib};
+        }
+    } else {
+        @keep_libs = @circ_lib_list;
+    }
+
+    # If we have exhausted every org unit within the current
+    # loop iteration, jump to the next loop iteration.
+    $current_loop++ unless @keep_libs;
+
+    return $self->handle_exceeds_target_loops
+        if $current_loop > $max_loops;
+
+    # Max hold loops not exceeded.  Trim the set of targetable copies
+    # to those that at circ libs that have not been visited within
+    # the current target loop.
+
+    # New loop iteration.  All copies are on the table.
+    return 1 unless @keep_libs;
+
+    my @new_copies;
+    for my $copy (@copies) {
+        push(@new_copies, $copy) 
+            if grep {$copy->{circ_lib} eq $_} @keep_libs;
+    }
+
+    $self->copy_hashes(\@new_copies);
+
+    return 1;
+}
+
+sub handle_exceeds_target_loops {
+    my $self = shift;
+    my $e = $self->editor;
+    my $hold = $self->hold;
+
+    $hold->cancel_time('now');
+    $hold->cancel_cause(1); # = un-targeted expiration
+
+    if (!$e->update_action_hold_request($hold)) {
+        my $evt = $e->die_event;
+        return $self->exit_targeter(
+            "Error updating hold request: ".$evt->{textcode}, 1);
+    }
+
+    $e->commit;
+
+    # Fire the A/T handler, but don't wait for a response.
+    OpenSRF::AppSession->create('open-ils.trigger')->request(
+        'open-ils.trigger.event.autocreate',
+        'hold_request.cancel.expire_no_target',
+        $hold, $hold->pickup_lib
+    );
+
+    return $self->exit_targeter("Hold exceeded max target loops");
 }
 
+# When all else fails, see if we can reuse the previously targeted copy.
 sub attempt_prev_copy_retarget {
     my $self = shift;
 
-    $self->{valid_previous_copy} = undef;
+    # attempt_remote_copy_target() can in some cases cancel the hold.
+    # Check our global 'done' flag to confirm we're still going.
+    return undef if $self->{done};
+
+    my $prev_copy = $self->{valid_previous_copy};
+    return undef unless $prev_copy;
+
+    if ($self->copy_is_permitted($prev_copy)) {
+        $logger->debug("targeter: retargeting the ".
+            "previously targeted copy [".$prev_copy->{id}."]" );
+        return $prev_copy;
+    }
+    
+    return undef;
 }
 
 # Returns the closest copy by proximity that is a confirmed valid
@@ -597,26 +737,21 @@ sub find_nearest_copy {
     my $self = shift;
     my %prox_map = %{$self->{weighted_prox_map}};
     my $hold = $self->hold;
+    my %seen;
 
+    # Pick a copy at random from each tier of the proximity map,
+    # starting at the lowest proximity and working up, until a 
+    # copy is found that is suitable for targeting.
     for my $prox (sort {$a <=> $b} keys %prox_map) {
         my @copies = @{$prox_map{$prox}};
         next unless @copies;
 
         my $rand = int(rand(scalar(@copies)));
-        my %seen = ();
 
         while (my ($c) = splice(@copies, $rand, 1)) {
             next if $seen{$c->{id}};
 
-            return $c if OpenILS::Utils::PermitHold::permit_copy_hold({ 
-                patron_id => $hold->usr,
-                copy_id => $c->{id},
-                requestor => $hold->requestor,
-                request_lib => $hold->request_lib,
-                pickup_lib => $hold->pickup_lib,
-                retarget => 1
-            });
-
+            return $c if $self->copy_is_permitted($c);
             $seen{$c->{id}} = 1;
 
             last unless(@copies);
@@ -627,6 +762,30 @@ sub find_nearest_copy {
     return undef;
 }
 
+sub copy_is_permitted {
+    my ($self, $copy) = @_;
+    return 0 unless $copy;
+
+    my $permitted = OpenILS::Utils::PermitHold::permit_copy_hold({ 
+        patron_id => $self->hold->usr,
+        copy_id => $copy->{id},
+        requestor => $self->hold->requestor,
+        request_lib => $self->hold->request_lib,
+        pickup_lib => $self->hold->pickup_lib,
+        retarget => 1
+    });
+
+    return 1 if $permitted; # found a targetable copy.
+
+    # Copy is confirmed non-viable.  
+    # Remove it from our potentials list.
+    $self->copy_hashes([
+        grep {$_->{id} ne $copy->{id}} @{$self->copy_hashes}
+    ]);
+
+    return 0;
+}
+
 sub apply_copy_target {
     my ($self, $copy) = @_;
     my $e = $self->editor;
@@ -636,16 +795,50 @@ sub apply_copy_target {
     $hold->prev_check_time('now');
 
     if (!$e->update_action_hold_request($hold)) {
-        my $evt = $self->editor->die_event;
+        my $evt = $e->die_event;
         return $self->exit_targeter(
             "Error updating hold request: ".$evt->{textcode}, 1);
     }
 
     $e->commit;
     $self->{success} = 1;
+    $self->{targeted_copy} = $hold->current_copy;
     return $self->exit_targeter("Hold successfully targeted");
 }
 
+# Returns 1 if all is OK, false on error.
+sub log_unfulfilled_hold {
+    my $self = shift;
+    return 1 unless my $prev_id = $self->{previous_copy_id};
+    my $e = $self->editor;
+
+    $logger->info("targeter: hold was not ".
+        "(but should have been) fulfilled by $prev_id");
+
+    my $circ_lib;
+    if ($self->{valid_previous_copy}) {
+        $circ_lib = $self->{valid_previous_copy}->{circ_lib};
+
+    } else {
+        # We don't have a handle on the previous copy to get its
+        # circ lib.  Fetch it.
+        $circ_lib = $e->retrieve_asset_copy($prev_id)->circ_lib;
+    }
+
+    my $unful = Fieldmapper::action::unfulfilled_hold_list->new;
+    $unful->hold($self->hold_id);
+    $unful->circ_lib($circ_lib);
+    $unful->current_copy($prev_id);
+
+    if (!$e->create_action_unfulfilled_hold_list($unful)) {
+        my $evt = $e->die_event;
+        return $self->exit_targeter(
+            "Error creating unfulfilled_hold_list: " . $e->{textcode}, 1);
+    }
+
+    return 1;
+}
+
 # Target a single hold request
 sub target {
     my ($self, $hold_id) = @_;
@@ -673,19 +866,25 @@ sub target {
     return unless $self->filter_copies_by_status;
     return unless $self->filter_closed_date_copies;
     return unless $self->handle_no_copies;
-    return unless $self->compile_weighted_proximity_map;
 
     my $copy = $self->attempt_force_recall_target ||
-               $self->attempt_min_prox_copy_target   ||
+               $self->attempt_local_copy_target   ||
                $self->attempt_remote_copy_target  ||
                $self->attempt_prev_copy_retarget;
 
+    # Be sure none of the above attempt_* calls set the exit/done flag.
+    # This can happen if the hold has to be forceably canceled.
+    return if $self->{done};
+
+    return unless $self->log_unfulfilled_hold;
+
     return $self->apply_copy_target($copy) if $copy;
 
-    # No targetable copy was found.  Remove he copy data and fire the
-    # no-copy handler to update the hold accordingly.
-    $self->copy_hashes([]);
-    $self->handle_no_copies;
+    # No targetable copy was found.  Fire the no-copy 
+    # handler to update the hold accordingly.
+    
+    # TODO: process_recall()
+    $self->handle_no_copies(1);
 }