From 6a742e49874e56116c4a667346ff99b0f8da4f65 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 11 Feb 2020 12:51:46 -0500 Subject: [PATCH] Begin to disallow back_populates with viewonly=True A viewonly=True relationship should not be mutated and ideally mutation itself would raise an error, but we're not there yet. Warn when a viewonly is to be the target of a back_populates as this only means that it should be locally mutated, which by definition will not work as expected because post-flush it will deliver doubled results, due to its state not being reset. Setting a relationship to viewonly=True which is also the target of a back_populates or backref configuration will now emit a warning and eventually be disallowed. back_populates refers specifically to mutation of an attribute or collection, which is disallowed when the attribute is subject to viewonly=True. The viewonly attribute is not subject to persistence behaviors which means it will not reflect correct results when it is locally mutated. Fixes: #5149 Change-Id: Ie51382a82e1a0ff5f3cf2cdbded780e77ace7f5f (cherry picked from commit 4376c2e5b016e8dfec7bc1b0d2ebbebae737c063) --- doc/build/changelog/unreleased_13/5149.rst | 11 +++++++++++ lib/sqlalchemy/orm/relationships.py | 21 +++++++++++++++++++++ test/orm/test_relationships.py | 18 ++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 doc/build/changelog/unreleased_13/5149.rst diff --git a/doc/build/changelog/unreleased_13/5149.rst b/doc/build/changelog/unreleased_13/5149.rst new file mode 100644 index 0000000000..9550a7aa08 --- /dev/null +++ b/doc/build/changelog/unreleased_13/5149.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm + :tickets: 5149 + + Setting a relationship to viewonly=True which is also the target of a + back_populates or backref configuration will now emit a warning and + eventually be disallowed. back_populates refers specifically to mutation + of an attribute or collection, which is disallowed when the attribute is + subject to viewonly=True. The viewonly attribute is not subject to + persistence behaviors which means it will not reflect correct results + when it is locally mutated. diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index b5e7c1d9b5..511e6c3ee7 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -835,6 +835,19 @@ class RelationshipProperty(StrategizedProperty): the full set of related objects, to prevent modifications of the collection from resulting in persistence operations. + .. warning:: The viewonly=True relationship should not be mutated + in Python; that means, elements should not be added or removed + from collections nor should a many-to-one or one-to-one attribute + be altered in Python. The viewonly=True relationship should only + be accessed via read. Towards this behavior, it is also not + appropriate for the viewonly=True relationship to have any kind + of persistence cascade settings, nor should it be the target of + either :paramref:`.relationship.backref` or + :paramref:`.relationship.back_populates`, as backrefs imply + in-Python mutation of the attribute. SQLAlchemy may emit + warnings for some or all of these conditions as of the 1.3 and + 1.4 series of SQLAlchemy and will eventually be disallowed. + :param omit_join: Allows manual control over the "selectin" automatic join optimization. Set to ``False`` to disable the "omit join" feature @@ -1847,6 +1860,14 @@ class RelationshipProperty(StrategizedProperty): def _add_reverse_property(self, key): other = self.mapper.get_property(key, _configure_mappers=False) + if other.viewonly: + util.warn_limited( + "Setting backref / back_populates on relationship %s to refer " + "to viewonly relationship %s will be deprecated in SQLAlchemy " + "1.4, and will be disallowed in a future release. " + "viewonly relationships should not be mutated", + (self, other), + ) self._reverse_property.add(other) other._reverse_property.add(self) diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 9f7735d2b4..fdfde2cb4a 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -2704,6 +2704,12 @@ class ViewOnlyHistoryTest(fixtures.MappedTest): ) mapper(B, self.tables.t2) + with testing.expect_warnings( + "Setting backref / back_populates on " + "relationship B.a to refer to viewonly relationship A.bs" + ): + configure_mappers() + a1 = A() b1 = B() a1.bs.append(b1) @@ -2735,6 +2741,12 @@ class ViewOnlyHistoryTest(fixtures.MappedTest): ) mapper(B, self.tables.t2) + with testing.expect_warnings( + "Setting backref / back_populates on " + "relationship A.bs to refer to viewonly relationship B.a" + ): + configure_mappers() + a1 = A() b1 = B() b1.a = a1 @@ -2835,6 +2847,12 @@ class ViewOnlyM2MBackrefTest(fixtures.MappedTest): ) mapper(B, t2) + with testing.expect_warnings( + "Setting backref / back_populates on " + "relationship A.bs to refer to viewonly relationship B.a" + ): + configure_mappers() + sess = create_session() a1 = A() b1 = B(as_=[a1]) -- 2.39.5