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

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 1d6cf8a858303e83ddce3e03f8a1bdee384c62a5..1f102cb36f914f5545ee3554659f1d852f8f0f64 100644 (file)
--- a/lib/sqlalchemy/ext/mutable.py
+++ b/lib/sqlalchemy/ext/mutable.py
@@ -511,6 +511,14 @@ class MutableBase:
                     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 c17ea1abedb29d8fa5abc2640515a5e61ad3d336..41348a0aa6e17931780b449916a1d4ae49335ad3 100644 (file)
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -343,6 +343,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 45272caa248585fb0b6f51fe9cd7f122a834efa0..20ffb385d1d9d0bfc771b7d9af7719b462841b70 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -3590,6 +3590,9 @@ class Session(_SessionClassMethods, EventTarget):
         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 d130ab11f6682c8501212065f2a47fda316614f1..e117710e750875ee139fbdace05bbbf68d70ed59 100644 (file)
--- a/test/ext/test_mutable.py
+++ b/test/ext/test_mutable.py
@@ -7,6 +7,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
@@ -198,6 +200,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"