]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Begin to disallow back_populates with viewonly=True
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 11 Feb 2020 17:51:46 +0000 (12:51 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Tue, 11 Feb 2020 17:54:27 +0000 (12:54 -0500)
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 [new file with mode: 0644]
lib/sqlalchemy/orm/relationships.py
test/orm/test_relationships.py

diff --git a/doc/build/changelog/unreleased_13/5149.rst b/doc/build/changelog/unreleased_13/5149.rst
new file mode 100644 (file)
index 0000000..9550a7a
--- /dev/null
@@ -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.
index b5e7c1d9b509cc73705ae416e534f5a379e059c9..511e6c3ee77f0ba85929be85ae018a63c07100e9 100644 (file)
@@ -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)
 
index 9f7735d2b4922d5ffd8605feabfc5c20fb57f383..fdfde2cb4a3f9a8f3339a1f033f673f81ed0fe73 100644 (file)
@@ -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])