- Fixed a regression from 0.7 where the contextmanager feature

of :meth:`.Session.begin_nested` would fail to correctly
roll back the transaction when a flush error occurred, instead
raising its own exception while leaving the session still
pending a rollback.  [ticket:2718]

3 files changed, 77 insertions, 4 deletions

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index ed9539891..aa8f0f878 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -7,6 +7,16 @@
     :version: 0.8.2
 
     .. change::
+      :tags: bug, orm
+      :tickets: 2718
+
+      Fixed a regression from 0.7 where the contextmanager feature
+      of :meth:`.Session.begin_nested` would fail to correctly
+      roll back the transaction when a flush error occurred, instead
+      raising its own exception while leaving the session still
+      pending a rollback.
+
+    .. change::
       :tags: bug, mysql
 
       Updated mysqlconnector dialect to check for disconnect based
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index f7a5558f1..ffb8a4e03 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -169,6 +169,7 @@ class SessionTransaction(object):
 
     def _assert_active(self, prepared_ok=False,
                         rollback_ok=False,
+                        deactive_ok=False,
                         closed_msg="This transaction is closed"):
         if self._state is COMMITTED:
             raise sa_exc.InvalidRequestError(
@@ -182,7 +183,7 @@ class SessionTransaction(object):
                         "SQL can be emitted within this transaction."
                     )
         elif self._state is DEACTIVE:
-            if not rollback_ok:
+            if not deactive_ok and not rollback_ok:
                 if self._rollback_exception:
                     raise sa_exc.InvalidRequestError(
                         "This Session's transaction has been rolled back "
@@ -192,7 +193,7 @@ class SessionTransaction(object):
                         " Original exception was: %s"
                         % self._rollback_exception
                     )
-                else:
+                elif not deactive_ok:
                     raise sa_exc.InvalidRequestError(
                         "This Session's transaction has been rolled back "
                         "by a nested rollback() call.  To begin a new "
@@ -435,7 +436,7 @@ class SessionTransaction(object):
         return self
 
     def __exit__(self, type, value, traceback):
-        self._assert_active(prepared_ok=True)
+        self._assert_active(deactive_ok=True, prepared_ok=True)
         if self.session.transaction is None:
             return
         if type is None:
diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 64b05a131..2866ab4ab 100644
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -1,4 +1,4 @@
-
+from __future__ import with_statement
 from sqlalchemy.testing import eq_, assert_raises, \
     assert_raises_message, assert_warnings
 from sqlalchemy import *
@@ -83,6 +83,8 @@ class SessionTransactionTest(FixtureTest):
             conn.close()
             raise
 
+
+
     @testing.requires.savepoints
     def test_heavy_nesting(self):
         users = self.tables.users
@@ -620,6 +622,66 @@ class CleanSavepointTest(FixtureTest):
                                     synchronize_session='fetch')
         self._run_test(update_fn)
 
+class ContextManagerTest(FixtureTest):
+    run_inserts = None
+
+    @testing.requires.savepoints
+    @engines.close_open_connections
+    def test_contextmanager_nested_rollback(self):
+        users, User = self.tables.users, self.classes.User
+
+        mapper(User, users)
+
+        sess = Session()
+        def go():
+            with sess.begin_nested():
+                sess.add(User())   # name can't be null
+                sess.flush()
+
+        # and not InvalidRequestError
+        assert_raises(
+            sa_exc.DBAPIError,
+            go
+        )
+
+        with sess.begin_nested():
+            sess.add(User(name='u1'))
+
+        eq_(sess.query(User).count(), 1)
+
+    def test_contextmanager_commit(self):
+        users, User = self.tables.users, self.classes.User
+
+        mapper(User, users)
+
+        sess = Session(autocommit=True)
+        with sess.begin():
+            sess.add(User(name='u1'))
+
+        sess.rollback()
+        eq_(sess.query(User).count(), 1)
+
+    def test_contextmanager_rollback(self):
+        users, User = self.tables.users, self.classes.User
+
+        mapper(User, users)
+
+        sess = Session(autocommit=True)
+        def go():
+            with sess.begin():
+                sess.add(User())  # name can't be null
+        assert_raises(
+            sa_exc.DBAPIError,
+            go
+        )
+
+        eq_(sess.query(User).count(), 0)
+
+        with sess.begin():
+            sess.add(User(name='u1'))
+        eq_(sess.query(User).count(), 1)
+
+
 class AutoExpireTest(_LocalFixture):
 
     def test_expunge_pending_on_rollback(self):