Don't run pending_to_persistent for non-new objects

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.

Fixes: #4489
Change-Id: Ibe147020aa62f7d605cb1029b7f3b776f42e6b43
(cherry picked from commit b2afef966dcc44e991f22a8fb68de4f1220bd674)

diff --git a/doc/build/changelog/unreleased_12/4489.rst b/doc/build/changelog/unreleased_12/4489.rst
new file mode 100644 (file)
index 0000000..6f50c4b
--- /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 ceb18da7b0d526a6dd785b4397c4b6fa57f3afe7..c3bb23ce038c8c6058de1f6e8c7db2dee1981fa5 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1822,7 +1822,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)
@@ -1866,6 +1873,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
 
@@ -1876,7 +1886,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 1efc5105e852e008fbc1a22182099110ca089fe1..831a6013985c0454225048c4dd6492a8fa9e26ef 100644 (file)
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -434,7 +434,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 39ccfefc31a30e48bcff7e9b8da4a6305c89cca6..3ac134f5a32c26589253bb3bb59299cb7a8e050e 100644 (file)
--- a/test/orm/test_events.py
+++ b/test/orm/test_events.py
@@ -1940,6 +1940,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()