From: Mike Bayer Date: Tue, 30 Aug 2022 14:25:47 +0000 (-0400) Subject: implement event for merge/load=False for mutable state setup X-Git-Tag: rel_1_4_41~8^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dce7050c1b3f1e8735070026e2d73abe70cb1f21;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..9f4cdfddd6 --- /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 cbec06a31f..45c96178a6 100644 --- 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 39659c7232..adff448f50 100644 --- 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 c6a91693e3..96a273a359 100644 --- 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 ff167b2536..70b076c55e 100644 --- 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"