LP#1664386: fix certain subrequests that control DB transactions user/gmcharlt/lp1664386_fix_db_xact_subrequests
authorGalen Charlton <gmc@equinoxinitiative.org>
Mon, 13 Feb 2017 22:18:28 +0000 (17:18 -0500)
committerGalen Charlton <gmc@equinoxinitiative.org>
Mon, 13 Feb 2017 22:18:28 +0000 (17:18 -0500)
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>
Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/actor.pm

index 652acc1..6e2ae2c 100644 (file)
@@ -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
index 06ba202..bcf4508 100644 (file)
@@ -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),