From 2de752afb4d28b156be7d13cc5618e3db9b4e198 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 22 Sep 2012 16:03:57 -0400 Subject: [PATCH] - [bug] Extra logic has been added to the "flush" that occurs within Session.commit(), such that the extra state added by an after_flush() or after_flush_postexec() hook is also flushed in a subsequent flush, before the "commit" completes. Subsequent calls to flush() will continue until the after_flush hooks stop adding new state. An "overflow" counter of 100 is also in place, in the event of a broken after_flush() hook adding new content each time. [ticket:2566] --- CHANGES | 11 ++++++++ lib/sqlalchemy/orm/session.py | 10 +++++++- test/orm/test_transaction.py | 47 +++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index ed87284e30..d3ef996894 100644 --- a/CHANGES +++ b/CHANGES @@ -34,6 +34,17 @@ CHANGES "version" column, when using the "version" feature. Tests courtesy Daniel Miller. [ticket:2539] + - [bug] Extra logic has been added to the "flush" + that occurs within Session.commit(), such that the + extra state added by an after_flush() or + after_flush_postexec() hook is also flushed in a + subsequent flush, before the "commit" completes. + Subsequent calls to flush() will continue until + the after_flush hooks stop adding new state. + An "overflow" counter of 100 is also in place, + in the event of a broken after_flush() hook + adding new content each time. [ticket:2566] + - sql - [bug] Fixed CTE bug whereby positional bound parameters present in the CTEs themselves diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 3710f32281..188c3b2e76 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -334,7 +334,15 @@ class SessionTransaction(object): subtransaction.commit() if not self.session._flushing: - self.session.flush() + for _flush_guard in xrange(100): + if self.session._is_clean(): + break + self.session.flush() + else: + raise exc.FlushError( + "Over 100 subsequent flushes have occurred within " + "session.commit() - is an after_flush() hook " + "creating new objects?") if self._parent is None and self.session.twophase: try: diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index 1fa5835b94..30bbd91946 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -318,6 +318,53 @@ class SessionTransactionTest(FixtureTest): eq_(len(sess.query(User).all()), 1) + def test_continue_flushing_on_commit(self): + """test that post-flush actions get flushed also if + we're in commit()""" + users, User = self.tables.users, self.classes.User + + mapper(User, users) + sess = Session() + + to_flush = [User(name='ed'), User(name='jack'), User(name='wendy')] + @event.listens_for(sess, "after_flush_postexec") + def add_another_user(session, ctx): + if to_flush: + session.add(to_flush.pop(0)) + + x = [1] + @event.listens_for(sess, "after_commit") + def add_another_user(session): + x[0] += 1 + + sess.add(to_flush.pop()) + sess.commit() + eq_(x, [2]) + eq_( + sess.scalar(select([func.count(users.c.id)])), 3 + ) + + def test_continue_flushing_guard(self): + users, User = self.tables.users, self.classes.User + + mapper(User, users) + sess = Session() + + @event.listens_for(sess, "after_flush_postexec") + def add_another_user(session, ctx): + session.add(User(name='x')) + sess.add(User(name='x')) + assert_raises_message( + orm_exc.FlushError, + "Over 100 subsequent flushes have occurred", + sess.commit + ) + + + + + + def test_error_on_using_inactive_session_commands(self): users, User = self.tables.users, self.classes.User -- 2.47.2