now to test middle layer

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

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 d713fd3..a1728aa 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
@@ -2959,7 +2959,7 @@ sub find_nearest_permitted_hold {
        # search for what should be the best holds for this copy to fulfill
        my $best_holds = $U->storagereq(
         "open-ils.storage.action.hold_request.nearest_hold.atomic", 
-               $user->ws_ou, $copy->id, 100, $hold_stall_interval, $fifo );
+               $user->ws_ou, $copy, 100, $hold_stall_interval, $fifo );
 
        # Add any pre-targeted holds to the list too? Unless they are already there, anyway.
        if ($old_holds) {

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
index 49f0d64..32a519c 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
@@ -342,14 +342,15 @@ sub get_hold_sort_order {
     ];
 }
 
-# Returns an ORDER BY clause *and* a string with a CTE expression to precede
-# the nearest-hold SQL query
+# Returns an ORDER BY clause
+# *and* a string with a CTE expression to precede the nearest-hold SQL query
+# *and* a string with extra JOIN statements needed
 sub build_hold_sort_clause {
     my ($columns, $cp, $here) = @_;
 
-    my %sprintf_args = (
+    my %order_by_sprintf_args = (
         hprox => [$cp->circ_lib],
-        approx => [$cp->id, $here->id],
+        approx => [$cp->id, $here],
         htime => [$cp->circ_lib],
         shtime => [$cp->circ_lib]
     );
@@ -364,7 +365,8 @@ sub build_hold_sort_clause {
         }
 
         my @args;
-        @args = @{$sprintf_args{$col}} if exists $sprintf_args{$col};
+        @args = @{$order_by_sprintf_args{$col}} if
+            exists $order_by_sprintf_args{$col};
 
         push @clauses, sprintf($HOLD_SORT_ORDER_BY{$col}, @args);
 
@@ -378,36 +380,42 @@ sub build_hold_sort_clause {
         # our copy been circulating away from home too long?" Two parts to
         # answer this question.
         #
-        # 1: Have their been no checkouts at the copy's circ_lib since the
+        # part 1: Have their been no checkouts at the copy's circ_lib since the
         # beginning of our go-home interval?
-        # 2: Was the last transit to affect our copy before the beginning
+        # part 2: Was the last transit to affect our copy before the beginning
         # of our go-home interval an outbound transit? i.e. away from circ-lib
 
-        $ctes .= q!
+        # [We use sprintf because the outer function that's going to send one
+        # big query through DBI is blind to our process of dynamically building
+        # these CTEs, and it wouldn't know what bind parameters to pass unless
+        # we did a lot more work here. This is injection-safe because we only
+        # use the %d formatter.]
+        $ctes .= sprintf(q!
 , copy_has_not_been_home AS (
     SELECT (
         -- part 1
         SELECT circ.id FROM action.circulation circ
         JOIN go_home_interval ON (true)
         WHERE
-            circ.target_copy = %d AND -- $cp->id
-            circ.circ_lib = %d AND  -- $cp->circ_lib
+            circ.target_copy = %d AND
+            circ.circ_lib = %d AND
             circ.xact_start >= NOW() - go_home_interval.value
     ) IS NULL AND (
         -- part 2
-        SELECT atc.dest <> $cp->circ_lib FROM action.transit_copy atc
+        SELECT atc.dest <> %d FROM action.transit_copy atc
         JOIN go_home_interval ON (true)
         WHERE
             atc.id = (
                 SELECT MAX(id) FROM action.transit_copy atc_inner
                 WHERE
-                    atc_inner.target_copy = $cp AND
+                    atc_inner.target_copy = %d AND
                     atc_inner.source_send_time < NOW() - go_home_interval.value
             )
     ) AS result
-) !;   # TODO sprintf
+) !, $cp->id, $cp->circ_lib, $cp->circ_lib, $cp->id);
         $joins .= " JOIN copy_has_not_been_home ON (true) ";
     }
+
     if ($ctes_needed == 2) {
         # In this auxiliary query, we ask the question, "has our copy come home
         # by any means that we can determine, even if it didn't circulate once
@@ -420,7 +428,7 @@ sub build_hold_sort_clause {
         # 2: there have been no checkins at home since the beginning of
         # the go-home interval for this copy
 
-        $ctes .= q!
+        $ctes .= sprintf(q!
 , copy_has_not_been_home_even_to_idle AS (
     SELECT
         copy_has_not_been_home.response AND (
@@ -428,20 +436,20 @@ sub build_hold_sort_clause {
             SELECT atc.id FROM action.transit_copy atc
             JOIN go_home_interval ON (true)
             WHERE
-                atc.target_copy = %d AND -- $cp
-                atc.dest = %d AND  -- $cp->circ_lib
+                atc.target_copy = %d AND
+                atc.dest = %d AND
                 atc.dest_recv_time >= NOW() - go_home_interval.value
         ) IS NULL AND (
             -- part 2
             SELECT circ.id FROM action.circulation circ
             JOIN go_home_interval ON (true)
             WHERE
-                circ.target_copy = $cp AND
-                circ.checkin_lib = $cp->circ_lib AND
+                circ.target_copy = %d AND
+                circ.checkin_lib = %d AND
                 circ.checkin_time >= NOW() - go_home_interval.value
         ) IS NULL
     AS result
-) !;  # TODO sprintf
+) !, $cp->id, $cp->circ_lib, $cp->id, $cp->circ_lib);
         $joins .= " JOIN copy_has_not_been_home_even_to_idle ON (true) ";
     }
 
@@ -455,21 +463,20 @@ sub build_hold_sort_clause {
 sub nearest_hold {
        my $self = shift;
        my $client = shift;
-       my $here = shift;
-       my $cp = shift;
+       my $here = shift;   # just the ID
+       my $cp = shift;     # now an object, formerly just the ID
        my $limit = int(shift()) || 10;
        my $age = shift() || '0 seconds';
        my $fifo = shift();
 
     $log->info("deprecated 'fifo' param true, but ignored") if isTrue $fifo;
 
-    my ($holdsort, $addl_cte, $addl_join) = build_hold_sort_clause(
-        get_hold_sort_order($here), $cp, $here
-    );
+    my ($holdsort, $addl_cte, $addl_join) =
+        build_hold_sort_clause(get_hold_sort_order($here), $cp, $here);
 
        local $OpenILS::Application::Storage::WRITE = 1;
 
-       my $ids = action::hold_request->db_Main->selectcol_arrayref(<<" SQL", {}, $here, $cp, $age);
+       my $ids = action::hold_request->db_Main->selectcol_arrayref(<<" SQL", {}, $here, $cp->id, $age);
         WITH go_home_interval AS (
             SELECT OILS_JSON_TO_TEXT(
                 (SELECT value FROM actor.org_unit_ancestor_setting(