diff options
| -rw-r--r-- | doc/build/changelog/unreleased_12/4489.rst | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 14 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/unitofwork.py | 2 | ||||
| -rw-r--r-- | test/orm/test_events.py | 9 |
4 files changed, 31 insertions, 3 deletions
diff --git a/doc/build/changelog/unreleased_12/4489.rst b/doc/build/changelog/unreleased_12/4489.rst new file mode 100644 index 000000000..6f50c4bdb --- /dev/null +++ b/doc/build/changelog/unreleased_12/4489.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 4489 + + Fixed fairly simple but critical issue where the + :meth:`.SessionEvents.pending_to_persistent` event would be invoked for + objects not just when they move from pending to persistent, but when they + were also already persistent and just being updated, thus causing the event + to be invoked for all objects on every update. diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 6d4198a4e..0b1a3b101 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1827,7 +1827,14 @@ class Session(_SessionClassMethods): states, self, to_transient=to_transient ) - def _register_newly_persistent(self, states): + def _register_persistent(self, states): + """Register all persistent objects from a flush. + + This is used both for pending objects moving to the persistent + state as well as already persistent objects. + + """ + pending_to_persistent = self.dispatch.pending_to_persistent or None for state in states: mapper = _state_mapper(state) @@ -1871,6 +1878,9 @@ class Session(_SessionClassMethods): ) state.key = instance_key + # there can be an existing state in the identity map + # that is replaced when the primary keys of two instances + # are swapped; see test/orm/test_naturalpks.py -> test_reverse self.identity_map.replace(state) state._orphaned_outside_of_session = False @@ -1881,7 +1891,7 @@ class Session(_SessionClassMethods): self._register_altered(states) if pending_to_persistent is not None: - for state in states: + for state in states.intersection(self._new): pending_to_persistent(self, state.obj()) # remove from new last, might be the last strong ref diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 3b91c8990..1f2221ea9 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -440,7 +440,7 @@ class UOWTransaction(object): if isdel: self.session._remove_newly_deleted(isdel) if other: - self.session._register_newly_persistent(other) + self.session._register_persistent(other) class IterateMappersMixin(object): diff --git a/test/orm/test_events.py b/test/orm/test_events.py index fc550eb83..d6d087d75 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -2004,6 +2004,15 @@ class SessionLifecycleEventsTest(_RemoveListeners, _fixtures.FixtureTest): [call.pending_to_persistent(sess, u1), call.flag_checked(u1)], ) + u1.name = 'u2' + sess.flush() + + # event was not called again + eq_( + listener.mock_calls, + [call.pending_to_persistent(sess, u1), call.flag_checked(u1)], + ) + def test_pending_to_persistent_del(self): sess, User, start_events = self._fixture() |