implement event for merge/load=False for mutable state setup

Fixed issue in :mod:`sqlalchemy.ext.mutable` extension where collection
links to the parent object would be lost if the object were merged with
:meth:`.Session.merge` while also passing :paramref:`.Session.merge.load`
as False.

The event added here is currently private for expediency, but
is acceptable to become a public event at some point.

Fixes: #8446
Change-Id: I9e5b9f1f5a0c5a9781f51635d5e57b1134c9e866
(cherry picked from commit e15cf451affdef95b3248d1ea5c31ac923e661c3)

diff --git a/doc/build/changelog/unreleased_14/8446.rst b/doc/build/changelog/unreleased_14/8446.rst
new file mode 100644 (file)
index 0000000..9f4cdfd
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/8446.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, ext
+    :tickets: 8446
+
+    Fixed issue in :mod:`sqlalchemy.ext.mutable` extension where collection
+    links to the parent object would be lost if the object were merged with
+    :meth:`.Session.merge` while also passing :paramref:`.Session.merge.load`
+    as False.

diff --git a/lib/sqlalchemy/ext/mutable.py b/lib/sqlalchemy/ext/mutable.py
index cbec06a31fec353cf9d8386903c3dce2a9d00e40..45c96178a6521c810bd71daf68c2dbd96a7ac874 100644 (file)
--- a/lib/sqlalchemy/ext/mutable.py
+++ b/lib/sqlalchemy/ext/mutable.py
@@ -511,6 +511,14 @@ class MutableBase(object):
                     for val in state_dict["ext.mutable.values"][key]:
                         val._parents[state] = key
 
+        event.listen(
+            parent_cls,
+            "_sa_event_merge_wo_load",
+            load,
+            raw=True,
+            propagate=True,
+        )
+
         event.listen(parent_cls, "load", load, raw=True, propagate=True)
         event.listen(
             parent_cls, "refresh", load_attrs, raw=True, propagate=True

diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index 39659c72325d1ea54480ac8cccda2e09b8f54428..adff448f5040bb48cb4a7a5cb7d5cf05e666e212 100644 (file)
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -325,6 +325,23 @@ class InstanceEvents(event.Events):
 
         """
 
+    def _sa_event_merge_wo_load(self, target, context):
+        """receive an object instance after it was the subject of a merge()
+        call, when load=False was passed.
+
+        The target would be the already-loaded object in the Session which
+        would have had its attributes overwritten by the incoming object. This
+        overwrite operation does not use attribute events, instead just
+        populating dict directly. Therefore the purpose of this event is so
+        that extensions like sqlalchemy.ext.mutable know that object state has
+        changed and incoming state needs to be set up for "parents" etc.
+
+        This functionality is acceptable to be made public in a later release.
+
+        .. versionadded:: 1.4.41
+
+        """
+
     def load(self, target, context):
         """Receive an object instance after it has been created via
         ``__new__``, and after initial attribute population has

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index c6a91693e30253cb002db24a5825a1b0eb37a466..96a273a3598ff75e2868c3b18d3cb47d0192a169 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -3151,6 +3151,9 @@ class Session(_SessionClassMethods):
         if not load:
             # remove any history
             merged_state._commit_all(merged_dict, self.identity_map)
+            merged_state.manager.dispatch._sa_event_merge_wo_load(
+                merged_state, None
+            )
 
         if new_instance:
             merged_state.manager.dispatch.load(merged_state, None)

diff --git a/test/ext/test_mutable.py b/test/ext/test_mutable.py
index ff167b25365f0e36acf1226c0de37816fa75ab14..70b076c55eaeabe263fd751f91a47f3f7785a77d 100644 (file)
--- a/test/ext/test_mutable.py
+++ b/test/ext/test_mutable.py
@@ -6,6 +6,7 @@ from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import inspect
 from sqlalchemy import Integer
+from sqlalchemy import JSON
 from sqlalchemy import select
 from sqlalchemy import String
 from sqlalchemy import testing
@@ -26,6 +27,7 @@ from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
+from sqlalchemy.testing import is_true
 from sqlalchemy.testing import mock
 from sqlalchemy.testing.fixtures import fixture_session
 from sqlalchemy.testing.schema import Column
@@ -160,6 +162,49 @@ class MiscTest(fixtures.TestBase):
                     b = ((), [data[other_attr]], ())
                     eq_(a, b)
 
+    @testing.combinations("key_present", "key_non_present", argnames="present")
+    @testing.combinations(
+        ("transient", True),
+        ("detached", True),
+        ("detached", False),
+        argnames="merge_subject, load",
+    )
+    @testing.requires.json_type
+    def test_session_merge(
+        self, decl_base, connection, present, load, merge_subject
+    ):
+        """test #8446"""
+
+        class Thing(decl_base):
+            __tablename__ = "thing"
+            id = Column(Integer, primary_key=True)
+            data = Column(MutableDict.as_mutable(JSON))
+
+        decl_base.metadata.create_all(connection)
+
+        with Session(connection) as sess:
+            sess.add(Thing(id=1, data={"foo": "bar"}))
+            sess.commit()
+
+        if merge_subject == "transient":
+            t1_to_merge = Thing(id=1, data={"foo": "bar"})
+        elif merge_subject == "detached":
+            with Session(connection) as sess:
+                t1_to_merge = sess.get(Thing, 1)
+
+        with Session(connection) as sess:
+            already_present = None
+            if present == "key_present":
+                already_present = sess.get(Thing, 1)
+
+            t1_merged = sess.merge(t1_to_merge, load=load)
+
+            t1_merged.data["foo"] = "bat"
+            if present == "key_present":
+                is_(t1_merged, already_present)
+
+            is_true(inspect(t1_merged).attrs.data.history.added)
+
 
 class _MutableDictTestBase(_MutableDictTestFixture):
     run_define_tables = "each"