From 636dcb2ffcb5c9ef5775d0ad4fbc2a8901d85e84 Mon Sep 17 00:00:00 2001 From: Bill Erickson Date: Tue, 7 Jun 2016 17:32:14 -0400 Subject: [PATCH] LP#? Hold targeter refactoring and optimization. * Ports hold targeter code to a Perl utility function, communicating w/ the DB via cstore instead of storage. * Adds a new global flag 'circ.holds.retarget_interval' for configuring the hold retarget interval in the database. * Adds a new DB function to regenerating hold copy maps to make map deletion and creation more efficient. * Adds targeter support for targeting holds in newest to oldest order. Signed-off-by: Bill Erickson --- .../lib/OpenILS/Application/Circ/Circulate.pm | 10 +- .../perlmods/lib/OpenILS/Application/Circ/Holds.pm | 18 +- .../src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm | 1133 ++++++++++++++++++++ Open-ILS/src/sql/Pg/090.schema.action.sql | 7 + Open-ILS/src/sql/Pg/950.data.seed-values.sql | 11 + .../sql/Pg/upgrade/XXXX.schema.hold_targeter.sql | 25 + Open-ILS/src/support-scripts/hold_targeter.pl | 105 +- 7 files changed, 1217 insertions(+), 92 deletions(-) create mode 100644 Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm index befe1bcb75..26c813909a 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm @@ -7,6 +7,7 @@ use OpenSRF::Utils::SettingsClient; use OpenSRF::Utils::Logger qw(:logger); use OpenILS::Const qw/:const/; use OpenILS::Application::AppUtils; +use OpenILS::Utils::HoldTargeter; use DateTime; my $U = "OpenILS::Application::AppUtils"; @@ -2299,7 +2300,10 @@ sub checkin_retarget { next if ($_->{hold_type} eq 'P'); } # So much for easy stuff, attempt a retarget! - my $tresult = $U->storagereq('open-ils.storage.action.hold_request.copy_targeter', undef, $_->{id}, $self->copy->id); + my $tresult = OpenILS::Utils::HoldTargeter->new->target( + hold => $_->{id}, + find_copy => $self->copy->id + ); if(ref $tresult eq "ARRAY" and scalar @$tresult) { last if(exists $tresult->[0]->{found_copy} and $tresult->[0]->{found_copy}); } @@ -3057,9 +3061,7 @@ sub do_hold_notify { sub retarget_holds { my $self = shift; $logger->info("circulator: retargeting holds @{$self->retarget} after opportunistic capture"); - my $ses = OpenSRF::AppSession->create('open-ils.storage'); - $ses->request('open-ils.storage.action.hold_request.copy_targeter', undef, $self->retarget); - # no reason to wait for the return value + OpenILS::Utils::HoldTargeter->new->target(hold => $self->retarget); return; } diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm index 65c3a6fe97..7949ab582c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm @@ -27,6 +27,7 @@ use OpenSRF::Utils; use OpenSRF::Utils::Logger qw(:logger); use OpenILS::Utils::CStoreEditor q/:funcs/; use OpenILS::Utils::PermitHold; +use OpenILS::Utils::HoldTargeter; use OpenSRF::Utils::SettingsClient; use OpenILS::Const qw/:const/; use OpenILS::Application::Circ::Transit; @@ -380,9 +381,8 @@ sub create_hold { $conn->respond_complete($hold->id); - $U->storagereq( - 'open-ils.storage.action.hold_request.copy_targeter', - undef, $hold->id ) unless $U->is_true($hold->frozen); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold->id) + unless $U->is_true($hold->frozen); return undef; } @@ -746,7 +746,7 @@ sub uncancel_hold { $e->update_action_hold_request($hold) or return $e->die_event; $e->commit; - $U->storagereq('open-ils.storage.action.hold_request.copy_targeter', undef, $hold_id); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold_id); return 1; } @@ -1064,15 +1064,14 @@ sub update_hold_impl { if(!$U->is_true($hold->frozen) && $U->is_true($orig_hold->frozen)) { $logger->info("Running targeter on activated hold ".$hold->id); - $U->storagereq( 'open-ils.storage.action.hold_request.copy_targeter', undef, $hold->id ); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold->id); } # a change to mint-condition changes the set of potential copies, so retarget the hold; if($U->is_true($hold->mint_condition) and !$U->is_true($orig_hold->mint_condition)) { _reset_hold($self, $e->requestor, $hold) } elsif($need_retarget && !defined $hold->capture_time()) { # If needed, retarget the hold due to changes - $U->storagereq( - 'open-ils.storage.action.hold_request.copy_targeter', undef, $hold->id ); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold->id); } return $hold->id; @@ -1160,7 +1159,7 @@ sub update_hold_if_frozen { } else { if($U->is_true($orig_hold->frozen)) { $logger->info("Running targeter on activated hold ".$hold->id); - $U->storagereq( 'open-ils.storage.action.hold_request.copy_targeter', undef, $hold->id ); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold->id); } } } @@ -1989,8 +1988,7 @@ sub _reset_hold { $e->update_action_hold_request($hold) or return $e->die_event; $e->commit; - $U->storagereq( - 'open-ils.storage.action.hold_request.copy_targeter', undef, $hold->id ); + OpenILS::Utils::HoldTargeter->new->target(hold => $hold->id); return undef; } diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm new file mode 100644 index 0000000000..b512bbc448 --- /dev/null +++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm @@ -0,0 +1,1133 @@ +package OpenILS::Utils::HoldTargeter; +# --------------------------------------------------------------- +# Copyright (C) 2016 King County Library System +# Author: Bill Erickson +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# --------------------------------------------------------------- +use strict; +use warnings; +use DateTime; +use OpenSRF::AppSession; +use OpenSRF::Utils::Logger qw(:logger); +use OpenSRF::Utils::JSON; +use OpenSRF::Utils qw/:datetime/; +use OpenILS::Application::AppUtils; +use OpenILS::Utils::CStoreEditor qw/:funcs/; + +our $U = "OpenILS::Application::AppUtils"; +our $dt_parser = DateTime::Format::ISO8601->new; + +# See target() for runtime arguments. +sub new { + my ($class, %args) = @_; + my $self = { + editor => new_editor(), + ou_setting_cache => {}, + %args, + }; + return bless($self, $class); +} + +# Target and retarget holds. +# By default, targets all holds that need targeting, meaning those that +# have either never been targeted or those whose prev_check_time exceeds +# the retarget interval. +# +# Optional parameters: +# +# hold => +# -- ID of a specific hold to target. +# +# return_count => 1 +# -- If set, avoid collecting result objects for every hold processed +# and simply return the number of holds processed. This is useful +# for batch processing to avoid storing result data in memory for +# potentially hundreds of thousands of holds. +# +# retarget_interval => +# -- If set, this overrides the value found in the +# 'circ.holds.retarget_interval' global flag. +# +# newest_first => 1 +# -- If set, holds will be targeted in reverse order of create_time. +# This is useful for targeting / re-targeting newer holds first. +# +# target_all => 1 +# -- Forces targeting / re-targeting of all active holds. +# This is primarily usefulf or testing. USE WITH CAUTION. +# +# Returns an array of hold targeter response objects, one response per hold. +sub target { + my ($self, %args) = @_; + + foreach (qw/hold retarget_interval newest_first target_all return_count/) { + $self->{$_} = $args{$_} if exists $args{$_}; + } + + $self->init; + + my $count = 0; + my @responses; + + for my $hold_id ($self->find_holds_to_target) { + my $single = OpenILS::Utils::HoldTargeter::Single->new(parent => $self); + $single->target($hold_id); + push(@responses, $single->result) unless $self->{return_count}; + $count++; + } + + return $self->{return_count} ? $count : \@responses; +} + +sub find_holds_to_target { + my $self = shift; + + return ($self->{hold}) if $self->{hold}; + + my $query = { + select => {ahr => ['id']}, + from => 'ahr', + where => { + capture_time => undef, + fulfillment_time => undef, + cancel_time => undef, + frozen => 'f' + }, + order_by => [ + {class => 'ahr', field => 'selection_depth', direction => 'DESC'}, + {class => 'ahr', field => 'request_time'}, + {class => 'ahr', field => 'prev_check_time'} + ] + }; + + if (!$self->{target_all}) { + # Unless we're retargeting all holds, limit to holds that have no + # prev_check_time or those whose prev_check_time occurred + # before the retarget interval. + + my $date = DateTime->now->subtract( + seconds => $self->{retarget_interval}); + + $query->{where}->{'-or'} = [ + {prev_check_time => undef}, + {prev_check_time => {'<=' => $date->strftime('%F %T%z')}} + ]; + } + + # Newest-first sorting cares only about hold create_time. + $query->{order_by} = + [{class => 'ahr', field => 'request_time', direction => 'DESC'}] + if $self->{newest_first}; + + my $holds = $self->editor->json_query($query, {substream => 1}); + + return map {$_->{id}} @$holds; +} + +sub editor { + my $self = shift; + return $self->{editor}; +} + +# Load startup data required by all targeter actions. +sub init { + my $self = shift; + my $e = $self->editor; + + my $closed_orgs_query = { + close_start => {'<=', 'now'}, + close_end => {'>=', 'now'} + }; + + if (!$self->{target_all}) { + + # See if the caller provided an interval + my $interval = $self->{retarget_interval}; + + if (!$interval) { + # See if we have a global flag value for the interval + + $interval = $e->search_config_global_flag({ + name => 'circ.holds.retarget_interval', + enabled => 't' + })->[0]; + + # If no flag is present, default to a 24-hour retarget interval. + $interval = $interval ? $interval->value : '24h'; + } + + # Convert the interval to seconds for current and future use. + $self->{retarget_interval} = interval_to_seconds($interval); + + # An org unit is considered closed for retargeting purposes + # if it's closed both now and at the next re-target date. + + my $next_check_time = + DateTime->now->add(seconds => $self->{retarget_interval}) + ->strftime('%F %T%z'); + + $closed_orgs_query = { + '-and' => [ + $closed_orgs_query, { + close_start => {'<=', $next_check_time}, + close_end => {'>=', $next_check_time} + } + ] + } + } + + my $closed = + $self->editor->search_actor_org_unit_closed_date($closed_orgs_query); + + # Map of org id to 1. Any org in the map is closed. + $self->{closed_orgs} = {map {$_->org_unit => 1} @$closed}; +} + +# Org unit setting fetch+cache +sub get_ou_setting { + my ($self, $org_id, $setting) = @_; + my $c = $self->{ou_setting_cache}; + + $c->{$org_id} = {} unless $c->{$org_id}; + + $c->{$org_id}->{$setting} = + $U->ou_ancestor_setting_value($org_id, $setting, $self->{editor}) + unless exists $c->{$org_id}->{$setting}; + + return $c->{$org_id}->{$setting}; +} + +# ----------------------------------------------------------------------- +# Knows how to target a single hold. +# ----------------------------------------------------------------------- +package OpenILS::Utils::HoldTargeter::Single; +use strict; +use warnings; +use DateTime; +use OpenSRF::AppSession; +use OpenSRF::Utils qw/:datetime/; +use OpenSRF::Utils::Logger qw(:logger); +use OpenILS::Application::AppUtils; +use OpenILS::Utils::CStoreEditor qw/:funcs/; + +sub new { + my ($class, %args) = @_; + my $self = { + %args, + editor => new_editor(), + error => 0, + success => 0 + }; + return bless($self, $class); +} + +# Parent targeter object. +sub parent { + my ($self, $parent) = @_; + $self->{parent} = $parent if $parent; + return $self->{parent}; +} + +sub hold_id { + my ($self, $hold_id) = @_; + $self->{hold_id} = $hold_id if $hold_id; + return $self->{hold_id}; +} + +sub hold { + my ($self, $hold) = @_; + $self->{hold} = $hold if $hold; + return $self->{hold}; +} + +# Debug message +sub message { + my ($self, $message) = @_; + $self->{message} = $message if $message; + return $self->{message} || ''; +} + +# True if the hold was successfully targeted. +sub success { + my ($self, $success) = @_; + $self->{success} = $success if defined $success; + return $self->{success}; +} + +# True if targeting exited early on an unrecoverable error. +sub error { + my ($self, $error) = @_; + $self->{error} = $error if defined $error; + return $self->{error}; +} + +sub editor { + my $self = shift; + return $self->{editor}; +} + +sub result { + my $self = shift; + + return { + hold => $self->hold_id, + error => $self->error, + success => $self->success, + message => $self->message, + target => $self->hold ? $self->hold->current_copy : undef, + old_target => $self->{previous_copy_id}, + found_copy => $self->{found_copy}, + eligible_copies => $self->{eligible_copy_count} + }; +} + +# List of potential copies in the form of slim hashes. This list +# evolves as copies are filtered as they are deemed non-targetable. +sub copies { + my ($self, $copies) = @_; + $self->{copies} = $copies if $copies; + return $self->{copies}; +} + +# Final set of potential copies, including those that may not be +# currently targetable, that may be eligible for recall processing. +sub recall_copies { + my ($self, $recall_copies) = @_; + $self->{recall_copies} = $recall_copies if $recall_copies; + return $self->{recall_copies}; +} + +# Maps copy ID's to their hold proximity +sub copy_prox_map { + my ($self, $copy_prox_map) = @_; + $self->{copy_prox_map} = $copy_prox_map if $copy_prox_map; + return $self->{copy_prox_map}; +} + +# Captures the exit message, rolls back the cstore transaction/connection, +# and returns false. +# is_error : log the final message and editor event at ERR level. +sub exit_targeter { + my ($self, $msg, $is_error) = @_; + + $self->message($msg); + my $log = "targeter: exiting hold ".$self->hold_id." : $msg"; + + if ($is_error) { + # On error, roll back and capture the last editor event for logging. + + my $evt = $self->editor->die_event; + $log .= " [".$evt->{textcode}."]" if $evt; + + $self->error(1); + $logger->error($log); + + } else { + # Attempt a rollback and disconnect when each hold exits + # to avoid the possibility of leaving cstore's pinned. + # Note: ->rollback is a no-op when a ->commit has already occured. + + $self->editor->rollback; + $logger->info($log); + } + + return 0; +} + +# Cancel expired holds and kick off the A/T no-target event. Returns +# true (i.e. keep going) if the hold is not expired. Returns false if +# the hold is canceled or a non-recoverable error occcurred. +sub handle_expired_hold { + my $self = shift; + my $hold = $self->hold; + + return 1 unless $hold->expire_time; + + my $ex_time = + $dt_parser->parse_datetime(cleanse_ISO8601($hold->expire_time)); + return 1 unless DateTime->compare($ex_time, DateTime->now) < 0; + + # Hold is expired -- + + $hold->cancel_time('now'); + $hold->cancel_cause(1); # == un-targeted expiration + + $self->editor->update_action_hold_request($hold) + or return $self->exit_targeter("Error canceling hold", 1); + + $self->editor->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 is expired"); +} + +# Find potential copies for hold mapping and targeting. +sub get_hold_copies { + my $self = shift; + my $e = $self->editor; + my $hold = $self->hold; + + my $hold_target = $hold->target; + my $hold_type = $hold->hold_type; + my $org_unit = $hold->selection_ou; + my $org_depth = $hold->selection_depth || 0; + + my $query = { + select => { + acp => ['id', 'status', 'circ_lib'], + ahr => ['current_copy'] + }, + from => { + acp => { + # Tag copies that are in use by other holds so we don't + # try to target them for our hold. + ahr => { + type => 'left', + fkey => 'id', # acp.id + field => 'current_copy', + filter => { + fulfillment_time => undef, + cancel_time => undef, + id => {'!=' => $self->hold_id} + } + } + } + }, + where => { + '+acp' => { + deleted => 'f', + circ_lib => { + in => { + select => { + aou => [{ + transform => 'actor.org_unit_descendants', + column => 'id', + result_field => 'id', + params => [$org_depth] + }], + }, + from => 'aou', + where => {id => $org_unit} + } + } + } + } + }; + + unless ($hold_type eq 'R' || $hold_type eq 'F') { + # Add the holdability filters to the copy query, unless + # we're processing a Recall or Force hold, which bypass most + # holdability checks. + + $query->{from}->{acp}->{acpl} = { + field => 'id', + filter => {holdable => 't', deleted => 'f'}, + fkey => 'location' + }; + + $query->{from}->{acp}->{ccs} = { + field => 'id', + filter => {holdable => 't'}, + fkey => 'status' + }; + + $query->{where}->{'+acp'}->{holdable} = 't'; + $query->{where}->{'+acp'}->{mint_condition} = 't' + if $U->is_true($hold->mint_condition); + } + + unless ($hold_type eq 'C' || $hold_type eq 'I' || $hold_type eq 'P') { + # For volume and higher level holds, avoid targeting copies that + # act as instances of monograph parts. + # TODO: Should this include 'R' holds? The original hold + # targeter does not include them either. + $query->{from}->{acp}->{acpm} = { + type => 'left', + field => 'target_copy', + fkey => 'id' + }; + + $query->{where}->{'+acpm'}->{id} = undef; + } + + if ($hold_type eq 'C' || $hold_type eq 'R' || $hold_type eq 'F') { + + $query->{where}->{'+acp'}->{id} = $hold_target; + + } elsif ($hold_type eq 'V') { + + $query->{where}->{'+acp'}->{call_number} = $hold_target; + + } elsif ($hold_type eq 'P') { + + $query->{from}->{acp}->{acpm} = { + field => 'target_copy', + fkey => 'id', + filter => {part => $hold_target}, + }; + + } elsif ($hold_type eq 'I') { + + $query->{from}->{acp}->{sitem} = { + field => 'unit', + fkey => 'id', + filter => {issuance => $hold_target}, + }; + + } elsif ($hold_type eq 'T') { + + $query->{from}->{acp}->{acn} = { + field => 'id', + fkey => 'call_number', + 'join' => { + bre => { + field => 'id', + filter => {id => $hold_target}, + fkey => 'record' + } + } + }; + + } else { # Metarecord hold + + $query->{from}->{acp}->{acn} = { + field => 'id', + fkey => 'call_number', + join => { + bre => { + field => 'id', + fkey => 'record', + join => { + mmrsm => { + field => 'source', + fkey => 'id', + filter => {metarecord => $hold_target}, + } + } + } + } + }; + } + + my $copies = $e->json_query($query, {substream => 1}); + $self->{eligible_copy_count} = scalar(@$copies); + + $logger->info("targeter: Hold ".$self->hold_id." has ". + $self->{eligible_copy_count}." potential copies"); + + # Let the caller know we encountered the copy they were interested in. + $self->{found_copy} = 1 if $self->{find_copy} + && grep {$_->{id} eq $self->{find_copy}} @$copies; + + $self->copies($copies); + + return 1; +} + +# Delete and rebuild copy maps +sub update_copy_maps { + my $self = shift; + my $e = $self->editor; + + my $resp = $e->json_query({from => [ + 'action.hold_request_regen_copy_maps', + $self->hold_id, + '{' . join(',', map {$_->{id}} @{$self->copies}) . '}' + ]}); + + # The above call can fail if another process is updating + # copy maps for this hold at the same time. + return 1 if $resp && @$resp; + + return $self->exit_targeter("Error creating hold copy maps", 1); +} + +# Returns a map of proximity values to arrays of copy hashes. +# The copy hash arrays are weighted consistent with the org unit hold +# target weight, meaning that a given copy may appear more than once +# in its proximity list. +sub compile_weighted_proximity_map { + my $self = shift; + + # Collect copy proximity info (generated via DB trigger) + # from our newly create copy maps. + my $hold_copy_maps = $self->editor->json_query({ + select => {ahcm => ['target_copy', 'proximity']}, + from => 'ahcm', + where => {hold => $self->hold_id} + }); + + my %copy_prox_map = + map {$_->{target_copy} => $_->{proximity}} @$hold_copy_maps; + + my %prox_map; + for my $copy_hash (@{$self->copies}) { + my $prox = $copy_prox_map{$copy_hash->{id}}; + $prox_map{$prox} ||= []; + + my $weight = $self->parent->get_ou_setting( + $copy_hash->{circ_lib}, + 'circ.holds.org_unit_target_weight') || 1; + + # Each copy is added to the list once per target weight. + push(@{$prox_map{$prox}}, $copy_hash) foreach (1 .. $weight); + } + + return $self->{weighted_prox_map} = \%prox_map; +} + +# Returns true if filtering completed without error, false otherwise. +sub filter_closed_date_copies { + my $self = shift; + + my @filtered_copies; + for my $copy_hash (@{$self->copies}) { + my $clib = $copy_hash->{circ_lib}; + + if ($self->parent->{closed_orgs}->{$clib}) { + # Org unit is currently closed. See if it matters. + + my $ous = $self->hold->pickup_lib eq $clib ? + 'circ.holds.target_when_closed_if_at_pickup_lib' : + 'circ.holds.target_when_closed'; + + unless ($self->parent->get_ou_setting($clib, $ous)) { + # Targeting not allowed at this circ lib when its closed + + $logger->info("targeter: skipping copy ". + $copy_hash->{id}."at closed org $clib"); + + next; + } + + } + + push(@filtered_copies, $copy_hash); + } + + # Update our in-progress list of copies to reflect the filtered set. + $self->copies(\@filtered_copies); + + return 1; +} + +# Limit the set of potential copies to those that are +# in a targetable status. +# Returns true if filtering completes without error, false otherwise. +sub filter_copies_by_status { + my $self = shift; + + $self->copies([ + grep {$_->{status} == 0 || $_->{status} == 7} @{$self->copies} + ]); + + # Track checked out copies for later recall + $self->recall_copies([grep {$_->{status} == 1} @{$self->copies}]); + + return 1; +} + +# Remove copies that are currently targeted by other holds. +# Returns true if filtering completes without error, false otherwise. +sub filter_copies_in_use { + my $self = shift; + + # A copy with a 'current_copy' value means it's in use by another hold. + $self->copies([ + grep {!$_->{current_copy}} @{$self->copies} + ]); + + return 1; +} + +# Returns true if inspection completed without error, false otherwise. +sub inspect_previous_target { + my $self = shift; + my $hold = $self->hold; + my @copies = @{$self->copies}; + + # no previous target + return 1 unless my $prev_id = $hold->current_copy; + + $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; + + # previous target is no longer valid. + return 1 unless $prev; + + # Previous copy is targetable. Keep it around for later. + $self->{valid_previous_copy} = $prev; + + # Remove the previous copy from the working set of potential copies. + # It will be revisited later if needed. + $self->copies([grep {$_->{id} ne $prev_id} @copies]); + + return 1; +} + +# Returns true if we have at least one potential copy remaining, thus +# targeting should continue. Otherwise, the hold is updated to reflect +# that there is no target and returns false to stop targeting. +sub handle_no_copies { + my ($self, %args) = @_; + + if (!$args{force}) { + # If 'force' is set, the caller is saying that all copies have + # failed. Otherwise, see if we have any copies left to inspect. + return 1 if @{$self->copies} || $self->{valid_previous_copy}; + } + + # At this point, all copies have been inspected and none + # have yielded a targetable item. + + if ($args{process_recalls}) { + # See if we have any copies/circs to recall. + return unless $self->process_recalls; + } + + my $hold = $self->hold; + $hold->clear_current_copy; + $hold->prev_check_time('now'); + + $self->editor->update_action_hold_request($hold) + or return $self->exit_targeter("Error updating hold request", 1); + + $self->editor->commit; + return $self->exit_targeter("Hold has no targetable copies"); +} + +# Force and recall holds bypass validity tests. Returns the first +# (and presumably only) copy in our list of valid copies when a +# F or R hold is encountered. Returns undef otherwise. +sub attempt_force_recall_target { + my $self = shift; + return $self->copies->[0] if + $self->hold->hold_type eq 'R' || $self->hold->hold_type eq 'F'; + return undef; +} + +# 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->copies}; + 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); + } + + $logger->info( + "targeter: no local targetable copies found for hold ".$self->hold_id); + + return undef; +} + +sub attempt_remote_copy_target { + my $self = shift; + + $logger->info("targeter: attempting to target a ". + "remote copy for hold ".$self->hold_id); + + return undef unless @{$self->copies}; + + 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 1 unless defined $max_loops; + + my @copies = @{$self->copies}; + 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->copies(\@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 + + $e->update_action_hold_request($hold) + or return $self->exit_targeter("Error updating hold request", 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; + + # attempt_remote_copy_target() can in some cases cancel the hold. + return undef if $self->hold->cancel_time; + + my $prev_copy = $self->{valid_previous_copy}; + return undef unless $prev_copy; + + $logger->info("targeter: attempting to re-target ". + "previously targeted copy for hold ".$self->hold_id); + + 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 +# targetable copy. +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))); + + while (my ($c) = splice(@copies, $rand, 1)) { + $rand = int(rand(scalar(@copies))); + next if $seen{$c->{id}}; + + return $c if $self->copy_is_permitted($c); + $seen{$c->{id}} = 1; + + last unless(@copies); + } + } + + return undef; +} + +# Returns true if the provided copy passes the hold permit test for our +# hold and can be used for targeting. +# When a copy fails the test, it is removed from $self->copies. +sub copy_is_permitted { + my ($self, $copy) = @_; + return 0 unless $copy; + + my $resp = $self->editor->json_query({ + from => [ + 'action.hold_retarget_permit_test', + $self->hold->request_lib, + $self->hold->pickup_lib, + $copy->{id}, + $self->hold->usr, + $self->hold->requestor + ] + }); + + return 1 if $U->is_true($resp->[0]->{success}); + + # Copy is confirmed non-viable. + # Remove it from our potentials list. + $self->copies([ + grep {$_->{id} ne $copy->{id}} @{$self->copies} + ]); + + return 0; +} + +# Sets hold.current_copy to the provided copy. +sub apply_copy_target { + my ($self, $copy) = @_; + my $e = $self->editor; + my $hold = $self->hold; + + $hold->current_copy($copy->{id}); + $hold->prev_check_time('now'); + + $e->update_action_hold_request($hold) + or return $self->exit_targeter("Error updating hold request", 1); + + $e->commit; + $self->{success} = 1; + return $self->exit_targeter("Hold successfully targeted"); +} + +# Creates a new row in action.unfulfilled_hold_list for our hold. +# 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); + + $e->create_action_unfulfilled_hold_list($unful) or + return $self->exit_targeter("Error creating unfulfilled_hold_list", 1); + + return 1; +} + +sub process_recalls { + my $self = shift; + my $e = $self->editor; + + my $pu_lib = $self->hold->pickup_lib; + + my $threshold = + $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_threshold') + or return 1; + + my $interval = + $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_return_interval') + or return 1; + + # Give me the ID of every checked out copy living at the hold + # pickup library. + my @copy_ids = map {$_->{id}} + grep {$_->{circ_lib} eq $pu_lib} @{$self->recall_copies}; + + return 1 unless @copy_ids; + + my $circ = $e->search_action_circulation([ + { target_copy => \@copy_ids, + checkin_time => undef, + duration => {'>' => $threshold} + }, { + order_by => 'due_date', + limit => 1 + } + ])->[0]; + + return unless $circ; + + $logger->info("targeter: recalling circ ".$circ->id); + + # Give the user a new due date of either a full recall threshold, + # or the return interval, whichever is further in the future. + my $threshold_date = DateTime::Format::ISO8601 + ->parse_datetime(cleanse_ISO8601($circ->xact_start)) + ->add(seconds => interval_to_seconds($threshold)) + ->iso8601(); + + my $return_date = DateTime->now(time_zone => 'local')->add( + seconds => interval_to_seconds($interval))->iso8601(); + + if (DateTime->compare( + DateTime::Format::ISO8601->parse_datetime($threshold_date), + DateTime::Format::ISO8601->parse_datetime($return_date)) == 1) { + $return_date = $threshold_date; + } + + my %update_fields = ( + due_date => $return_date, + renewal_remaining => 0, + ); + + my $fine_rules = + $self->parent->get_ou_setting($pu_lib, 'circ.holds.recall_fine_rules'); + + # If the OU hasn't defined new fine rules for recalls, keep them + # as they were + if ($fine_rules) { + $logger->info("targeter: applying recall fine rules: $fine_rules"); + my $rules = OpenSRF::Utils::JSON->JSON2perl($fine_rules); + $update_fields{recurring_fine} = $rules->[0]; + $update_fields{fine_interval} = $rules->[1]; + $update_fields{max_fine} = $rules->[2]; + } + + # Copy updated fields into circ object. + $circ->$_($update_fields{$_}) for keys %update_fields; + + $e->update_action_circulation($circ) + or return $self->exit_targeter( + "Error updating circulation object in process_recalls", 1); + + # Create trigger event for notifying current user + my $ses = OpenSRF::AppSession->create('open-ils.trigger'); + $ses->request('open-ils.trigger.event.autocreate', + 'circ.recall.target', $circ, $circ->circ_lib); + + return 1; +} + +# Target a single hold request +sub target { + my ($self, $hold_id) = @_; + + my $e = $self->editor; + $self->hold_id($hold_id); + + $logger->info("Processing hold $hold_id"); + + $e->xact_begin; + + my $hold = $e->retrieve_action_hold_request($hold_id) + or return $self->exit_targeter("No hold found", 1); + + return $self->exit_targeter("Hold is not eligible for targeting") + if $hold->capture_time || + $hold->cancel_time || + $hold->fulfillment_time || + $U->is_true($hold->frozen); + + $self->hold($hold); + + return unless $self->handle_expired_hold; + return unless $self->get_hold_copies; + return unless $self->update_copy_maps; + + # Confirm that we have something to work on. If we have no + # copies at this point, there's also nothing to recall. + return unless $self->handle_no_copies; + + # Trim the set of working copies down to those that are + # currently targetable. + return unless $self->filter_copies_by_status; + 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 + return unless $self->inspect_previous_target; + + # 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); + + # At this point, the working list of copies has been trimmed to + # those that are currently targetable. + + my $copy = $self->attempt_force_recall_target || + $self->attempt_local_copy_target || + $self->attempt_remote_copy_target || + $self->attempt_prev_copy_retarget; + + # See if one of the above attempt* calls canceled the hold as a side + # 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. + $self->handle_no_copies(force => 1, process_recalls => 1); +} + + + diff --git a/Open-ILS/src/sql/Pg/090.schema.action.sql b/Open-ILS/src/sql/Pg/090.schema.action.sql index c79793a321..184a49cbde 100644 --- a/Open-ILS/src/sql/Pg/090.schema.action.sql +++ b/Open-ILS/src/sql/Pg/090.schema.action.sql @@ -420,6 +420,13 @@ CREATE TRIGGER hold_request_clear_map_tgr ) EXECUTE PROCEDURE action.hold_request_clear_map(); +CREATE OR REPLACE FUNCTION + action.hold_request_regen_copy_maps( + hold_id INTEGER, copy_ids INTEGER[]) RETURNS VOID AS $$ + DELETE FROM action.hold_copy_map WHERE hold = $1; + INSERT INTO action.hold_copy_map (hold, target_copy) SELECT $1, UNNEST($2); +$$ LANGUAGE SQL; + CREATE INDEX hold_request_target_idx ON action.hold_request (target); CREATE INDEX hold_request_usr_idx ON action.hold_request (usr); CREATE INDEX hold_request_pickup_lib_idx ON action.hold_request (pickup_lib); diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql index 9c59fa321b..cba394274e 100644 --- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql +++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql @@ -16253,3 +16253,14 @@ INSERT INTO config.org_unit_setting_type 'coust', 'description'), 'integer'); +INSERT INTO config.global_flag (name, label, value, enabled) VALUES ( + 'circ.holds.retarget_interval', + oils_i18n_gettext( + 'circ.holds.retarget_interval', + 'Holds Retarget Interval', + 'cgf', + 'label' + ), + '24h', + TRUE +); diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql new file mode 100644 index 0000000000..ba584f0f57 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql @@ -0,0 +1,25 @@ +BEGIN; + +CREATE OR REPLACE FUNCTION + action.hold_request_regen_copy_maps( + hold_id INTEGER, copy_ids INTEGER[]) RETURNS VOID AS $$ + DELETE FROM action.hold_copy_map WHERE hold = $1; + INSERT INTO action.hold_copy_map (hold, target_copy) SELECT $1, UNNEST($2); +$$ LANGUAGE SQL; + +-- DATA + +INSERT INTO config.global_flag (name, label, value, enabled) VALUES ( + 'circ.holds.retarget_interval', + oils_i18n_gettext( + 'circ.holds.retarget_interval', + 'Holds Retarget Interval', + 'cgf', + 'label' + ), + '24h', + TRUE +); + +COMMIT; + diff --git a/Open-ILS/src/support-scripts/hold_targeter.pl b/Open-ILS/src/support-scripts/hold_targeter.pl index 2ca196e1ae..a046238276 100755 --- a/Open-ILS/src/support-scripts/hold_targeter.pl +++ b/Open-ILS/src/support-scripts/hold_targeter.pl @@ -1,99 +1,48 @@ #!/usr/bin/perl -# --------------------------------------------------------------------- -# Usage: -# hold_targeter.pl -# --------------------------------------------------------------------- - +#---------------------------------------------------------------- +# Batch hold targeter +#---------------------------------------------------------------- use strict; use warnings; -use OpenSRF::Utils::JSON; use OpenSRF::System; -use OpenSRF::Utils::SettingsClient; -use OpenSRF::MultiSession; -use OpenSRF::EX qw(:try); +use OpenILS::Utils::Fieldmapper; +use OpenILS::Utils::HoldTargeter; +#---------------------------------------------------------------- +# Usage: +# ./hold_targeter.pl /openils/conf/opensrf_core.xml +#---------------------------------------------------------------- -my $config = shift || die "bootstrap config required\n"; +my $osrf_config = shift || '/openils/conf/opensrf_core.xml'; my $lockfile = shift || "/tmp/hold_targeter-LOCK"; if (-e $lockfile) { die "I seem to be running already. If not remove $lockfile, try again\n"; } -open(F, ">$lockfile"); -print F $$; +open(F, ">$lockfile") or die "Cannot open lock file: $lockfile : $@\n"; +print F $$ or die "Cannot write to lock file: $lockfile : $@\n"; close F; -my $settings; -my $parallel; +eval { # Make sure we can delete the lock file. -try { - OpenSRF::System->bootstrap_client( config_file => $config ); - $settings = OpenSRF::Utils::SettingsClient->new; - $parallel = $settings->config_value( hold_targeter => 'parallel' ) || 1; -} otherwise { - my $e = shift; - warn "$e\n"; - unlink $lockfile; - exit 1; -}; + OpenSRF::System->bootstrap_client(config_file => $osrf_config); + Fieldmapper->import( + IDL => OpenSRF::Utils::SettingsClient->new->config_value("IDL")); + OpenILS::Utils::CStoreEditor::init(); -if ($parallel == 1) { + my $targeter = OpenILS::Utils::HoldTargeter->new; - try { - my $r = OpenSRF::AppSession - ->create( 'open-ils.storage' ) - ->request( 'open-ils.storage.action.hold_request.copy_targeter' => '24h' ); + my $start = time; + my $count = $targeter->target( + # Return only the number processed, + # instead of a result blob for each hold. + return_count => 1 + ); - while (!$r->complete) { - my $start = time; - $r->recv(timeout => 3600); - last if (time() - $start) >= 3600; - }; - } otherwise { - my $e = shift; - warn "Failure in single-session targeter:\n$e\n"; - }; + my $minutes = sprintf('%0.2f', (time - $start) / 60); -} else { - - try { - my $multi_targeter = OpenSRF::MultiSession->new( - app => 'open-ils.storage', - cap => $parallel, - api_level => 1, - session_hash_function => sub { - my $ses = shift; - my $req = shift; - return $_[-1]; # last parameter is the ID of the metarecord associated with the - # request's target; using this as the hash function value ensures - # that parallel targeters won't try to simultaneously handle two - # hold requests that have overlapping pools of copies that could - # fill those requests - } - ); - - my $storage = OpenSRF::AppSession->create("open-ils.storage"); - - my $r = $storage->request('open-ils.storage.action.hold_request.targetable_holds.id_list', '24h'); - while ( my $h = $r->recv ) { - if ($r->failed) { - print $r->failed->stringify . "\n"; - last; - } - if (my $hold = $h->content) { - $multi_targeter->request( 'open-ils.storage.action.hold_request.copy_targeter', '', $hold->[0], $hold->[1]); - } - } - - $storage->disconnect(); - - $multi_targeter->session_wait(1); - $multi_targeter->disconnect; - } otherwise { - my $e = shift; - warn "Failure in multi-session targeter:\n$e\n"; - } -} + print "Processed $count holds in $minutes minutes.\n"; +}; unlink $lockfile; -- 2.11.0