- :class:`.Connection` now associates a new

:class:`.RootTransaction` or :class:`.TwoPhaseTransaction`
with its immediate :class:`._ConnectionFairy` as a "reset handler"
for the span of that transaction, which takes over the task
of calling commit() or rollback() for the "reset on return" behavior
of :class:`.Pool` if the transaction was not otherwise completed.
This resolves the issue that a picky transaction
like that of MySQL two-phase will be
properly closed out when the connection is closed without an
explicit rollback or commit (e.g. no longer raises "XAER_RMFAIL"
in this case - note this only shows up in logging as the exception
is not propagated within pool reset).
This issue would arise e.g. when using an orm
:class:`.Session` with ``twophase`` set, and then
:meth:`.Session.close` is called without an explicit rollback or
commit.   The change also has the effect that you will now see
an explicit "ROLLBACK" in the logs when using a :class:`.Session`
object in non-autocommit mode regardless of how that session was
discarded.  Thanks to Jeff Dairiki and Laurence Rowe for isolating
the issue here. [ticket:2907]

1 files changed, 90 insertions, 0 deletions

diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py
index 10f490b48..2e4c2dc48 100644
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -1328,6 +1328,96 @@ class QueuePoolTest(PoolTestBase):
         c2 = p.connect()
         assert c2.connection is not None
 
+class ResetOnReturnTest(PoolTestBase):
+    def _fixture(self, **kw):
+        dbapi = Mock()
+        return dbapi, pool.QueuePool(creator=lambda: dbapi.connect('foo.db'), **kw)
+
+    def test_plain_rollback(self):
+        dbapi, p = self._fixture(reset_on_return='rollback')
+
+        c1 = p.connect()
+        c1.close()
+        assert dbapi.connect().rollback.called
+        assert not dbapi.connect().commit.called
+
+    def test_plain_commit(self):
+        dbapi, p = self._fixture(reset_on_return='commit')
+
+        c1 = p.connect()
+        c1.close()
+        assert not dbapi.connect().rollback.called
+        assert dbapi.connect().commit.called
+
+    def test_plain_none(self):
+        dbapi, p = self._fixture(reset_on_return=None)
+
+        c1 = p.connect()
+        c1.close()
+        assert not dbapi.connect().rollback.called
+        assert not dbapi.connect().commit.called
+
+    def test_agent_rollback(self):
+        dbapi, p = self._fixture(reset_on_return='rollback')
+
+        class Agent(object):
+            def __init__(self, conn):
+                self.conn = conn
+
+            def rollback(self):
+                self.conn.special_rollback()
+
+            def commit(self):
+                self.conn.special_commit()
+
+        c1 = p.connect()
+        c1._reset_agent = Agent(c1)
+        c1.close()
+
+        assert dbapi.connect().special_rollback.called
+        assert not dbapi.connect().special_commit.called
+
+        assert not dbapi.connect().rollback.called
+        assert not dbapi.connect().commit.called
+
+        c1 = p.connect()
+        c1.close()
+        eq_(dbapi.connect().special_rollback.call_count, 1)
+        eq_(dbapi.connect().special_commit.call_count, 0)
+
+        assert dbapi.connect().rollback.called
+        assert not dbapi.connect().commit.called
+
+    def test_agent_commit(self):
+        dbapi, p = self._fixture(reset_on_return='commit')
+
+        class Agent(object):
+            def __init__(self, conn):
+                self.conn = conn
+
+            def rollback(self):
+                self.conn.special_rollback()
+
+            def commit(self):
+                self.conn.special_commit()
+
+        c1 = p.connect()
+        c1._reset_agent = Agent(c1)
+        c1.close()
+        assert not dbapi.connect().special_rollback.called
+        assert dbapi.connect().special_commit.called
+
+        assert not dbapi.connect().rollback.called
+        assert not dbapi.connect().commit.called
+
+        c1 = p.connect()
+        c1.close()
+
+        eq_(dbapi.connect().special_rollback.call_count, 0)
+        eq_(dbapi.connect().special_commit.call_count, 1)
+        assert not dbapi.connect().rollback.called
+        assert dbapi.connect().commit.called
+
 class SingletonThreadPoolTest(PoolTestBase):
 
     @testing.requires.threading_with_mock