Fix spurious ROLLBACK statements when a TxnScopeGuard fails a deferred commit

This is a *temporary workaround*, and I am really unhappy about it... it just
feels wrong. See the comment in lib/DBIx/Class/Storage/TxnScopeGuard.pm

diff --git a/Changes b/Changes
index c9d9966..b6c82cb 100644 (file)
--- a/Changes
+++ b/Changes
@@ -24,6 +24,9 @@ Revision history for DBIx::Class
           and warned about (GH#15)
         - Fix corner case of stringify-only overloaded objects being used in
           create()/populate()
+        - Fix spurious ROLLBACK statements when a TxnScopeGuard fails a commit
+          of a transaction with deferred FK checks: a guard is now inactivated
+          immediately before the commit is attempted (RT#107159)
         - Fix several corner cases with Many2Many over custom relationships
         - Fix t/52leaks.t failures on compilerless systems (RT#104429)
         - Fix t/storage/quote_names.t failures on systems with specified Oracle

diff --git a/lib/DBIx/Class/Storage/TxnScopeGuard.pm b/lib/DBIx/Class/Storage/TxnScopeGuard.pm
index edf7205..392e354 100644 (file)
--- a/lib/DBIx/Class/Storage/TxnScopeGuard.pm
+++ b/lib/DBIx/Class/Storage/TxnScopeGuard.pm
@@ -45,8 +45,13 @@ sub commit {
   $self->{storage}->throw_exception("Refusing to execute multiple commits on scope guard $self")
     if $self->{inactivated};
 
-  $self->{storage}->txn_commit;
+  # FIXME - this assumption may be premature: a commit may fail and a rollback
+  # *still* be necessary. Currently I am not aware of such scenarious, but I
+  # also know the deferred constraint handling is *severely* undertested.
+  # Making the change of "fire txn and never come back to this" in order to
+  # address RT#107159, but this *MUST* be reevaluated later.
   $self->{inactivated} = 1;
+  $self->{storage}->txn_commit;
 }
 
 sub DESTROY {

diff --git a/t/72pg.t b/t/72pg.t
index e03cd2b..71213e8 100644 (file)
--- a/t/72pg.t
+++ b/t/72pg.t
@@ -5,6 +5,7 @@ use warnings;
 
 use Test::More;
 use Test::Exception;
+use Test::Warn;
 use Sub::Name;
 use Config;
 use DBIx::Class::Optional::Dependencies ();
@@ -485,7 +486,24 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs';
     $schema->resultset('Track')->create({
       trackid => 1, cd => 9999, position => 1, title => 'Track1'
     });
-  } qr/constraint/i, 'with_deferred_fk_checks is off';
+  } qr/violates foreign key constraint/i, 'with_deferred_fk_checks is off outside of TXN';
+
+  # rerun the same under with_deferred_fk_checks
+  # it is expected to fail, hence the eval
+  # but it also should not warn
+  warnings_like {
+    eval {
+      $schema->storage->with_deferred_fk_checks(sub {
+        $schema->resultset('Track')->create({
+          trackid => 1, cd => 9999, position => 1, title => 'Track1'
+        });
+      } )
+    };
+
+    like $@, qr/violates foreign key constraint/i,
+      "Still expected exception on deferred failure at commit time";
+
+  } [], 'No warnings on deferred rollback';
 }
 
 done_testing;