[Bps-public-commit] dbix-searchbuilder branch, warn-on-mixed-rollback-and-commit, created. 1.63_01-40-g46cf59d

[Ruslan Zakirov]

The branch, warn-on-mixed-rollback-and-commit has been created
        at  46cf59d92560a17f1b0c214ea12e53873295d54a (commit)

- Log -----------------------------------------------------------------
commit 46cf59d92560a17f1b0c214ea12e53873295d54a
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Apr 16 13:17:04 2013 +0400

    warn when rollback and commit are mixed
    
    Nested transactions are faked, so the only reasonable
    way to escape them is to use the action - either all commits
    or all rollbacks. Anything else may indicate a bug in software.
    Let's warn to catch such situations sooner.

diff --git a/lib/DBIx/SearchBuilder/Handle.pm b/lib/DBIx/SearchBuilder/Handle.pm
index fc4ff5e..01b78f5 100755
--- a/lib/DBIx/SearchBuilder/Handle.pm
+++ b/lib/DBIx/SearchBuilder/Handle.pm
@@ -11,7 +11,7 @@ use Encode qw();
 
 use DBIx::SearchBuilder::Util qw/ sorted_values /;
 
-use vars qw(@ISA %DBIHandle $PrevHandle $DEBUG %TRANSDEPTH %FIELDS_IN_TABLE);
+use vars qw(@ISA %DBIHandle $PrevHandle $DEBUG %TRANSDEPTH %TRANSROLLBACK %FIELDS_IN_TABLE);
 
 
 =head1 NAME
@@ -841,15 +841,23 @@ sub EndTransaction {
     $depth = 0 if $args{'Force'};
 
     $self->TransactionDepth( $depth );
+
+    my $dbh = $self->dbh;
+    $TRANSROLLBACK{ $dbh }{ $action }++;
+    if ( $TRANSROLLBACK{ $dbh }{ $action eq 'commit'? 'rollback' : 'commit' } ) {
+        warn "Rollback and commit are mixed while escaping nested transaction";
+    }
     return 1 if $depth;
 
+    delete $TRANSROLLBACK{ $dbh };
+
     if ($action eq 'commit') {
-        return $self->dbh->commit;
+        return $dbh->commit;
     }
     else {
         DBIx::SearchBuilder::Record::Cachable->FlushCache
             if DBIx::SearchBuilder::Record::Cachable->can('FlushCache');
-        return $self->dbh->rollback;
+        return $dbh->rollback;
     }
 }
 
diff --git a/t/03transactions.t b/t/03transactions.t
index c0a486d..e641c19 100644
--- a/t/03transactions.t
+++ b/t/03transactions.t
@@ -7,7 +7,7 @@ use Test::More;
 BEGIN { require "t/utils.pl" }
 our (@AvailableDrivers);
 
-use constant TESTS_PER_DRIVER => 42;
+use constant TESTS_PER_DRIVER => 52;
 
 my $total = scalar(@AvailableDrivers) * TESTS_PER_DRIVER;
 plan tests => $total;
@@ -95,6 +95,27 @@ diag("init schema in transaction and commit") if $ENV{'TEST_VERBOSE'};
     ok($handle->Commit, "commit successed");
     is($handle->TransactionDepth, 0, "transaction depth is 0");
 
+diag("nested txns with mixed escaping actions") if $ENV{'TEST_VERBOSE'};
+    ok($handle->BeginTransaction, "begin transaction");
+    ok($handle->BeginTransaction, "begin nested transaction");
+    ok($handle->Rollback, "rollback successed");
+    {
+        my $warn = 0;
+        local $SIG{__WARN__} = sub{ $_[0] =~ /Rollback and commit are mixed/? $warn++: warn @_ };
+        ok($handle->Commit, "commit successed");
+        is($warn, 1, "not forced rollback fires warning");
+    }
+
+    ok($handle->BeginTransaction, "begin transaction");
+    ok($handle->BeginTransaction, "begin nested transaction");
+    ok($handle->Commit, "rollback successed");
+    {
+        my $warn = 0;
+        local $SIG{__WARN__} = sub{ $_[0] =~ /Rollback and commit are mixed/? $warn++: warn @_ };
+        ok($handle->Rollback, "commit successed");
+        is($warn, 1, "not forced rollback fires warning");
+    }
+
 	cleanup_schema( 'TestApp::Address', $handle );
 }} # SKIP, foreach blocks
 

-----------------------------------------------------------------------