From: Brandon L. Black <blblack@gmail.com>
Date: Wed, 18 Jul 2007 21:23:29 +0000 (+0000)
Subject: fix a few txn issues, including the "transaction depth not resetting" which was being... 
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=57c18b65cf1c7d708efd3717f54fbbc3def07794;p=dbsrgits%2FDBIx-Class-Historic.git

fix a few txn issues, including the "transaction depth not resetting" which was being caused by the unexpected (but documented) volatility of $dbh->{AutoCommit}
nested rollback exceptions have never had any tests, and that part still looks wrong
---

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 336ab1f..f676ec1 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -14,7 +14,7 @@ use Scalar::Util qw/blessed weaken/;
 __PACKAGE__->mk_group_accessors('simple' =>
     qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts
        _conn_pid _conn_tid disable_sth_caching cursor on_connect_do
-       transaction_depth unsafe/
+       transaction_depth unsafe _dbh_autocommit/
 );
 
 BEGIN {
@@ -585,6 +585,8 @@ sub txn_do {
   ref $coderef eq 'CODE' or $self->throw_exception
     ('$coderef must be a CODE reference');
 
+  return $coderef->(@_) if $self->{transaction_depth};
+
   local $self->{_in_dbh_do} = 1;
 
   my @result;
@@ -645,7 +647,7 @@ sub disconnect {
   my ($self) = @_;
 
   if( $self->connected ) {
-    $self->_dbh->rollback unless $self->_dbh->{AutoCommit};
+    $self->_dbh->rollback unless $self->_dbh_autocommit;
     $self->_dbh->disconnect;
     $self->_dbh(undef);
     $self->{_dbh_gen}++;
@@ -726,7 +728,7 @@ sub _populate_dbh {
 
   # Always set the transaction depth on connect, since
   #  there is no transaction in progress by definition
-  $self->{transaction_depth} = $self->_dbh->{AutoCommit} ? 0 : 1;
+  $self->{transaction_depth} = $self->_dbh_autocommit ? 0 : 1;
 
   if(ref $self eq 'DBIx::Class::Storage::DBI') {
     my $driver = $self->_dbh->{Driver}->{Name};
@@ -785,6 +787,8 @@ sub _connect {
   $self->throw_exception("DBI Connection failed: " . ($@||$DBI::errstr))
     if !$dbh || $@;
 
+  $self->_dbh_autocommit($dbh->{AutoCommit});
+
   $dbh;
 }
 
@@ -792,7 +796,7 @@ sub _connect {
 sub txn_begin {
   my $self = shift;
   $self->ensure_connected();
-  if($self->{transaction_depth}++ == 0) {
+  if($self->{transaction_depth} == 0) {
     $self->debugobj->txn_begin()
       if $self->debug;
     # this isn't ->_dbh-> because
@@ -800,6 +804,7 @@ sub txn_begin {
     #  for AutoCommit users
     $self->dbh->begin_work;
   }
+  $self->{transaction_depth}++;
 }
 
 sub txn_commit {
@@ -810,7 +815,7 @@ sub txn_commit {
       if ($self->debug);
     $dbh->commit;
     $self->{transaction_depth} = 0
-      if $dbh->{AutoCommit};
+      if $self->_dbh_autocommit;
   }
   elsif($self->{transaction_depth} > 1) {
     $self->{transaction_depth}--
@@ -820,15 +825,13 @@ sub txn_commit {
 sub txn_rollback {
   my $self = shift;
   my $dbh = $self->_dbh;
-  my $autocommit;
   eval {
-    $autocommit = $dbh->{AutoCommit};
     if ($self->{transaction_depth} == 1) {
       $self->debugobj->txn_rollback()
         if ($self->debug);
-      $dbh->rollback;
       $self->{transaction_depth} = 0
-        if $autocommit;
+        if $self->_dbh_autocommit;
+      $dbh->rollback;
     }
     elsif($self->{transaction_depth} > 1) {
       $self->{transaction_depth}--;
@@ -842,7 +845,7 @@ sub txn_rollback {
     my $exception_class = "DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION";
     $error =~ /$exception_class/ and $self->throw_exception($error);
     # ensure that a failed rollback resets the transaction depth
-    $self->{transaction_depth} = $autocommit ? 0 : 1;
+    $self->{transaction_depth} = $self->_dbh_autocommit ? 0 : 1;
     $self->throw_exception($error);
   }
 }
diff --git a/t/81transactions.t b/t/81transactions.t
index e16ff2b..b0054af 100644
--- a/t/81transactions.t
+++ b/t/81transactions.t
@@ -7,7 +7,7 @@ use DBICTest;
 
 my $schema = DBICTest->init_schema();
 
-plan tests => 40;
+plan tests => 54;
 
 my $code = sub {
   my ($artist, @cd_titles) = @_;
@@ -34,6 +34,8 @@ my $code = sub {
 
 # Test successful txn_do() - scalar context
 {
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my @titles = map {'txn_do test CD ' . $_} (1..5);
   my $artist = $schema->resultset('Artist')->find(1);
   my $count_before = $artist->cds->count;
@@ -42,10 +44,14 @@ my $code = sub {
   is($artist->cds({
     title => "txn_do test CD $_",
   })->first->year, 2006, "new CD $_ year correct") for (1..5);
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth has been reset');
 }
 
 # Test successful txn_do() - list context
 {
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my @titles = map {'txn_do test CD ' . $_} (6..10);
   my $artist = $schema->resultset('Artist')->find(1);
   my $count_before = $artist->cds->count;
@@ -54,10 +60,14 @@ my $code = sub {
   is($artist->cds({
     title => "txn_do test CD $_",
   })->first->year, 2006, "new CD $_ year correct") for (6..10);
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth has been reset');
 }
 
 # Test nested successful txn_do()
 {
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my $nested_code = sub {
     my ($schema, $artist, $code) = @_;
 
@@ -82,6 +92,8 @@ my $code = sub {
     title => 'nested txn_do test CD '.$_,
   })->first->year, 2006, qq{nested txn_do CD$_ year ok}) for (1..10);
   is($artist->cds->count, $count_before+10, 'nested txn_do added all CDs');
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth has been reset');
 }
 
 my $fail_code = sub {
@@ -95,6 +107,31 @@ my $fail_code = sub {
 
 # Test failed txn_do()
 {
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
+  my $artist = $schema->resultset('Artist')->find(3);
+
+  eval {
+    $schema->txn_do($fail_code, $artist);
+  };
+
+  my $error = $@;
+
+  like($error, qr/the sky is falling/, 'failed txn_do threw an exception');
+  my $cd = $artist->cds({
+    title => 'this should not exist',
+    year => 2005,
+  })->first;
+  ok(!defined($cd), q{failed txn_do didn't change the cds table});
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth has been reset');
+}
+
+# do the same transaction again
+{
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my $artist = $schema->resultset('Artist')->find(3);
 
   eval {
@@ -109,16 +146,26 @@ my $fail_code = sub {
     year => 2005,
   })->first;
   ok(!defined($cd), q{failed txn_do didn't change the cds table});
+
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth has been reset');
 }
 
 # Test failed txn_do() with failed rollback
 {
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my $artist = $schema->resultset('Artist')->find(3);
 
   # Force txn_rollback() to throw an exception
   no warnings 'redefine';
   no strict 'refs';
-  local *{"DBIx::Class::Storage::DBI::SQLite::txn_rollback"} = sub{die 'FAILED'};
+
+  # die in rollback, but maintain sanity for further tests ...
+  local *{"DBIx::Class::Storage::DBI::SQLite::txn_rollback"} = sub{
+    my $storage = shift;
+    $storage->{transaction_depth}--;
+    die 'FAILED';
+  };
 
   eval {
     $schema->txn_do($fail_code, $artist);
@@ -143,12 +190,13 @@ my $fail_code = sub {
     year => 2005,
   })->first;
   ok(!defined($cd), q{deleted the failed txn's cd});
-  $schema->storage->{transaction_depth} = 0; # Must reset this or further tests
-                                             # will fail
+  $schema->storage->_dbh->rollback;
 }
 
 # Test nested failed txn_do()
 {
+  is( $schema->storage->{transaction_depth}, 0, 'txn depth starts at 0');
+
   my $nested_fail_code = sub {
     my ($schema, $artist, $code1, $code2) = @_;