From: Galen Charlton <gmc@equinoxinitiative.org>
Date: Mon, 13 Feb 2017 22:18:28 +0000 (-0500)
Subject: LP#1664386: fix certain subrequests that control DB transactions
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=fe29f169ab302c2b728a78647f589bb01358e2ab;p=working%2FEvergreen.git

LP#1664386: fix certain subrequests that control DB transactions

This patch removes all cases where the current OpenSRF client
object is passed to

$self->method_lookup('open-ils.storage.transaction.{begin/rollback}')->run()

This idiom is no longer needed, as session information required to
generate a transaction ID is reliably passed to subrequests. Without
this patch, current Evergreen master and OpenSRF master will cause
the following methods to return not only their results, but also
"1" from each use of this idiom:

open-ils.storage.actor.user.checked_out
open-ils.storage.booking.reservation.resource_targeter
open-ils.storage.action.hold_request.copy_targeter

To test
-------
[1] Be running current Evergreen master and OpenSRF master
    (or the 2.5 alpha)
[2] Run (say) open-ils.storage.actor.user.checked_out in srfsh:

srfsh# request open-ils.storage open-ils.storage.actor.user.checked_out 13

Received Data: 1

Received Data: 1

Received Data: {
  "out":[

  ],
  "claims_returned":[

  ],
  "long_overdue":[

  ],
  "overdue":[
    "55",
    "56",
    "57",
    "58",
    "59",
    "60"
  ],
  "lost":[

  ]
}

[3] Note the extraneous 'Received Data: 1' sent prior to the substantive
    response.
[4] Apply the patch and rerun step 2; note that this time
    only the desired response is sent.

Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>
---

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 652acc1883..6e2ae2cdf7 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
@@ -1098,7 +1098,7 @@ sub new_hold_copy_targeter {
 
     try {
         if ($one_hold) {
-            $self->method_lookup('open-ils.storage.transaction.begin')->run( $client );
+            $self->method_lookup('open-ils.storage.transaction.begin')->run();
             $holds = [ action::hold_request->search_where( { id => $one_hold, fulfillment_time => undef, cancel_time => undef, frozen => 'f' } ) ];
         } elsif ( $check_expire ) {
 
@@ -1195,7 +1195,7 @@ sub new_hold_copy_targeter {
                 $log->debug("Cleaning up after previous transaction\n");
                 $self->method_lookup('open-ils.storage.transaction.rollback')->run;
             }
-            $self->method_lookup('open-ils.storage.transaction.begin')->run( $client );
+            $self->method_lookup('open-ils.storage.transaction.begin')->run();
             $log->info("Processing hold ".$hold->id."...\n");
 
             #first, re-fetch the hold, to make sure it's not captured already
@@ -1710,7 +1710,7 @@ sub reservation_targeter {
 
     try {
         if ($one_reservation) {
-            $self->method_lookup('open-ils.storage.transaction.begin')->run( $client );
+            $self->method_lookup('open-ils.storage.transaction.begin')->run();
             $reservations = [ booking::reservation->search_where( { id => $one_reservation, capture_time => undef, cancel_time => undef } ) ];
         } else {
 
@@ -1738,7 +1738,7 @@ sub reservation_targeter {
                 $log->debug("Cleaning up after previous transaction\n");
                 $self->method_lookup('open-ils.storage.transaction.rollback')->run;
             }
-            $self->method_lookup('open-ils.storage.transaction.begin')->run( $client );
+            $self->method_lookup('open-ils.storage.transaction.begin')->run();
             $log->info("Processing reservation ".$bresv->id."...\n");
 
             #first, re-fetch the hold, to make sure it's not captured already
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/actor.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/actor.pm
index 06ba202264..bcf45087e0 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/actor.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/actor.pm
@@ -83,7 +83,7 @@ sub usr_breakdown_out {
     my $client = shift;
     my $usr = shift;
 
-    $self->method_lookup('open-ils.storage.transaction.begin')->run($client);
+    $self->method_lookup('open-ils.storage.transaction.begin')->run();
 
     my $out_sql = <<"    SQL";
             SELECT  id
@@ -135,7 +135,7 @@ sub usr_breakdown_out {
 
     my $lo = actor::user->db_Main->selectcol_arrayref($lo_sql, {}, $usr);
 
-    $self->method_lookup('open-ils.storage.transaction.rollback')->run($client);
+    $self->method_lookup('open-ils.storage.transaction.rollback')->run();
 
     if ($self->api_name =~/count$/o) {
         return {    total   => scalar(@$out) + scalar(@$od) + scalar(@$lost) + scalar(@$cl) + scalar(@$lo),