unit-of-work flush didn't close the failed transaction when the session was not in...

diff --git a/CHANGES b/CHANGES
index 1eb5d504a68087c9a168b8586154aabea2cd9067..d699e3384cc7f1f12a836ba2dc69fb9703127670 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -177,6 +177,10 @@ CHANGES
         - autoflush for commit() wasn't flushing for simple
           subtransactions.
 
+        - unitofwork flush didn't close the failed transaction
+          when the session was not in a transaction and commiting
+          the transaction failed.
+
     - Miscellaneous tickets: [ticket:940] [ticket:964]
 
 - general

diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index 05c9388b309fefdaf9a40d090bd692de415220b4..6c9e989b05e17665414a4c3226331053d7feef28 100644 (file)
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -216,10 +216,10 @@ class UnitOfWork(object):
             
             if session.extension is not None:
                 session.extension.after_flush(session, flush_context)
+            session.commit()
         except:
             session.rollback()
             raise
-        session.commit()
 
         flush_context.post_exec()
 

diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py
index 92d526f789fe363bd75e583062592d5254a9d154..22c8bbe8bdc9de97b5c19f80ae7f562e4dd3e900 100644 (file)
--- a/test/orm/unitofwork.py
+++ b/test/orm/unitofwork.py
@@ -15,6 +15,7 @@ from testlib import engines, tables, fixtures
 # TODO: convert suite to not use Session.mapper, use fixtures.Base
 # with explicit session.save()
 Session = scoped_session(sessionmaker(autoflush=True, transactional=True))
+orm_mapper = mapper
 mapper = Session.mapper
 
 class UnitOfWorkTest(object):
@@ -1967,7 +1968,45 @@ class RowSwitchTest(ORMTest):
         assert list(sess.execute(t1.select(), mapper=T1)) == [(2, 'some other t1')]
         assert list(sess.execute(t2.select(), mapper=T1)) == [(1, 'some other t2', 2)]
 
+class TransactionTest(ORMTest):
+    """This is in fact a core test, but currently the only known way
+    to make COMMIT repeatably fail is on postgresql with deferrable FKs"""
+    __only_on__ = 'postgres'
+    def define_tables(self, metadata):
+        global t1, T1, t2, T2
+
+        Session.remove()
+
+        t1 = Table('t1', metadata,
+            Column('id', Integer, primary_key=True))
+        
+        t2 = Table('t2', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('t1_id', Integer))
+        deferred_constraint = DDL("ALTER TABLE t2 ADD CONSTRAINT t2_t1_id_fk FOREIGN KEY (t1_id) "\
+                                  "REFERENCES t1 (id) DEFERRABLE INITIALLY DEFERRED")
+        deferred_constraint.execute_at('after-create', t2)
+        
+        class T1(fixtures.Base):
+            pass
+        
+        class T2(fixtures.Base):
+            pass
+
+        orm_mapper(T1, t1)
+        orm_mapper(T2, t2)
 
+    def test_close_transaction_on_commit_fail(self):
+        Session = sessionmaker(autoflush=False, transactional=False)
+        sess = Session()
+        
+        sess.save(T2(t1_id=123))
+        try:
+            sess.flush()
+            assert False
+        except:
+            # Flush needs to rollback also when commit fails
+            assert sess.transaction is None
 
 if __name__ == "__main__":
     testenv.main()