From 872b2d9ef323e7b4f5c55ce8caffaca60d98fe05 Mon Sep 17 00:00:00 2001
From: Michael Bayne <mdb@samskivert.com>
Date: Wed, 1 Oct 2014 14:27:55 -0700
Subject: [PATCH] Some robustness bits.

It's not clear what the right thing to do is if the database connection fails
during the middle of a transaction. In general, one can't talk to the database
after that, so I'm assuming the transaction is aborted (or eventually aborted)
and rolled back, and everything has to be retried.

I'm not currently tracking state inside a Transaction, which means that I don't
freak out if you call commit() then call rollback(), I just ignore the latter
call, and it's also possible to do something stupid like:

  Transaction tx = Transaction.start(ctx);
  try {
    _somRepo.doOp(..);
  } catch (DatabaseException de) {
    // no problem!, keep on keepin' on
  }
  _someRepo.doAnotherOp(...);
  tx.commit();

If the first repo op fails due to a database connection failure, the
transaction will get a new database connection for the second op and do the
last half of the op in a new connection.

I guess I'll have to do some state tracking, because stupid as the above code
is, I'm sure some circumstance will arise where it seems like a reasonable
thing to do, and hilarity will ensue. Blah.
---
 .../samskivert/depot/PersistenceContext.java  | 10 +++-
 .../com/samskivert/depot/Transaction.java     | 49 +++++++++----------
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/src/main/java/com/samskivert/depot/PersistenceContext.java b/src/main/java/com/samskivert/depot/PersistenceContext.java
index 310e3b0..8e10b59 100644
--- a/src/main/java/com/samskivert/depot/PersistenceContext.java
+++ b/src/main/java/com/samskivert/depot/PersistenceContext.java
@@ -585,8 +585,16 @@ public class PersistenceContext
         checkAreInitialized(); // le check du sanity
 
         boolean isReadOnly = op.isReadOnly();
+        ConnOp connop;
         Transaction tx = Transaction.get();
-        ConnOp connop = (tx == null) ? new NonTxOp(isReadOnly) : new TxOp(tx);
+        if (tx != null) {
+            if (tx.ctx != this) throw new IllegalStateException(
+                "Cannot perform database ops on multiple persistence contexts in a transaction " +
+                "[txCtx=" + tx.ctx._ident + ", thisCtx=" + _ident + "].");
+            connop = new TxOp(tx);
+        } else {
+            connop = new NonTxOp(isReadOnly);
+        }
 
         long preConnect = System.nanoTime();
         Connection conn;
diff --git a/src/main/java/com/samskivert/depot/Transaction.java b/src/main/java/com/samskivert/depot/Transaction.java
index 62d3e2b..883b46c 100644
--- a/src/main/java/com/samskivert/depot/Transaction.java
+++ b/src/main/java/com/samskivert/depot/Transaction.java
@@ -92,20 +92,26 @@ public class Transaction {
         }
     }
 
+    /** The persistence context in which this transaction is operating. */
+    public final PersistenceContext ctx;
+
     /**
      * Commits this transaction.
      */
     public void commit ()
     {
-        if (_conn != null) {
-            checkActive("commit");
-            try {
+        checkActive("commit");
+        try {
+            if (_conn != null) {
                 _conn.commit();
-            } catch (SQLException sqe) {
-                throw new DatabaseException("Transaction commit failure", sqe);
-            } finally {
-                release();
+                ctx._conprov.releaseTxConnection(ctx._ident, _conn);
             }
+        } catch (SQLException sqe) {
+            throw new DatabaseException("Transaction commit failure", sqe);
+            connectionFailed(sqe);
+        } finally {
+            _conn = null;
+            _activeTx.set(null);
         }
     }
 
@@ -114,23 +120,22 @@ public class Transaction {
      */
     public void rollback ()
     {
-        if (_conn != null) {
-            checkActive("rollback");
-            try {
+        try {
+            if (_conn != null) {
+                checkActive("rollback");
                 _conn.rollback();
-            } catch (SQLException sqe) {
-                throw new DatabaseException("Transaction rollback failure", sqe);
-            } finally {
-                release();
+                ctx._conprov.releaseTxConnection(ctx._ident, _conn);
             }
+        } catch (SQLException sqe) {
+            throw new DatabaseException("Transaction rollback failure", sqe);
+            connectionFailed(sqe);
+        } finally {
+            _conn = null;
+            _activeTx.set(null);
         }
     }
 
-    /** The persistence context in which this transaction is operating. */
-    public final PersistenceContext ctx;
-
-    Connection getConnection ()
-        throws PersistenceException
+    Connection getConnection () throws PersistenceException
     {
         if (_conn == null) _conn = ctx._conprov.getTxConnection(ctx._ident);
         return _conn;
@@ -151,12 +156,6 @@ public class Transaction {
             "Attempted to " + action + " non-active transaction");
     }
 
-    protected void release () {
-        ctx._conprov.releaseTxConnection(ctx._ident, _conn);
-        _conn = null;
-        _activeTx.set(null);
-    }
-
     protected static final ThreadLocal<Transaction> _activeTx = new ThreadLocal<Transaction>();
 
     /** The connection being used for this transaction. */