From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 22 May 2023 17:12:58 +0000 (-0400)
Subject: restore and test correct defaults for validates() keyword args
X-Git-Tag: rel_2_0_16~33
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=dfb09c0e424370026220061bbb9f287bdea0eff3;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

restore and test correct defaults for validates() keyword args

Fixed regression in the 2.0 series where the default value of
:paramref:`_orm.validates.include_backrefs` got changed to ``False`` for
the :func:`_orm.validates` function. This default is now restored to
``True``.

Fixes: #9820
Change-Id: I9b7257b7c6e94c12fc894c7b78e5c1cb92acad67
---

diff --git a/doc/build/changelog/unreleased_20/9820.rst b/doc/build/changelog/unreleased_20/9820.rst
new file mode 100644
index 0000000000..84a0561d42
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9820.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm, regression
+    :tickets: 9820
+
+    Fixed regression in the 2.0 series where the default value of
+    :paramref:`_orm.validates.include_backrefs` got changed to ``False`` for
+    the :func:`_orm.validates` function. This default is now restored to
+    ``True``.
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 731983ff47..0533c82a69 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -4300,7 +4300,7 @@ def reconstructor(fn):
 
 
 def validates(
-    *names: str, include_removes: bool = False, include_backrefs: bool = False
+    *names: str, include_removes: bool = False, include_backrefs: bool = True
 ) -> Callable[[_Fn], _Fn]:
     r"""Decorate a method as a 'validator' for one or more named properties.
 
@@ -4329,6 +4329,10 @@ def validates(
      :func:`.validates` usage where only one validator should emit per
      attribute operation.
 
+     .. versionchanged:: 2.0.16 This paramter inadvertently defaulted to
+        ``False `` for releases 2.0.0 through 2.0.15.  Its correct default
+        of ``True`` is restored in 2.0.16.
+
     .. seealso::
 
       :ref:`simple_validators` - usage examples for :func:`.validates`
diff --git a/test/orm/test_validators.py b/test/orm/test_validators.py
index 659ee3ca0c..c783b83668 100644
--- a/test/orm/test_validators.py
+++ b/test/orm/test_validators.py
@@ -2,6 +2,7 @@ from unittest.mock import call
 from unittest.mock import Mock
 
 from sqlalchemy import exc
+from sqlalchemy import testing
 from sqlalchemy.orm import collections
 from sqlalchemy.orm import relationship
 from sqlalchemy.orm import validates
@@ -309,61 +310,56 @@ class ValidatorTest(_fixtures.FixtureTest):
             users,
         )
 
-    def test_validator_wo_backrefs_wo_removes(self):
-        self._test_validator_backrefs(False, False)
-
-    def test_validator_wo_backrefs_w_removes(self):
-        self._test_validator_backrefs(False, True)
-
-    def test_validator_w_backrefs_wo_removes(self):
-        self._test_validator_backrefs(True, False)
-
-    def test_validator_w_backrefs_w_removes(self):
-        self._test_validator_backrefs(True, True)
-
-    def _test_validator_backrefs(self, include_backrefs, include_removes):
+    @testing.variation("include_backrefs", [True, False, "default"])
+    @testing.variation("include_removes", [True, False, "default"])
+    def test_validator_backrefs(self, include_backrefs, include_removes):
         users, addresses = (self.tables.users, self.tables.addresses)
         canary = Mock()
 
+        need_remove_param = (
+            bool(include_removes) and not include_removes.default
+        )
+        validate_kw = {}
+        if not include_removes.default:
+            validate_kw["include_removes"] = bool(include_removes)
+        if not include_backrefs.default:
+            validate_kw["include_backrefs"] = bool(include_backrefs)
+
+        expect_include_backrefs = include_backrefs.default or bool(
+            include_backrefs
+        )
+        expect_include_removes = (
+            bool(include_removes) and not include_removes.default
+        )
+
         class User(fixtures.ComparableEntity):
 
-            if include_removes:
+            if need_remove_param:
 
-                @validates(
-                    "addresses",
-                    include_removes=True,
-                    include_backrefs=include_backrefs,
-                )
+                @validates("addresses", **validate_kw)
                 def validate_address(self, key, item, remove):
                     canary(key, item, remove)
                     return item
 
             else:
 
-                @validates(
-                    "addresses",
-                    include_removes=False,
-                    include_backrefs=include_backrefs,
-                )
+                @validates("addresses", **validate_kw)
                 def validate_address(self, key, item):
                     canary(key, item)
                     return item
 
         class Address(fixtures.ComparableEntity):
-            if include_removes:
 
-                @validates(
-                    "user",
-                    include_backrefs=include_backrefs,
-                    include_removes=True,
-                )
+            if need_remove_param:
+
+                @validates("user", **validate_kw)
                 def validate_user(self, key, item, remove):
                     canary(key, item, remove)
                     return item
 
             else:
 
-                @validates("user", include_backrefs=include_backrefs)
+                @validates("user", **validate_kw)
                 def validate_user(self, key, item):
                     canary(key, item)
                     return item
@@ -390,8 +386,8 @@ class ValidatorTest(_fixtures.FixtureTest):
         # comparisons don't get caught
         calls = list(canary.mock_calls)
 
-        if include_backrefs:
-            if include_removes:
+        if expect_include_backrefs:
+            if expect_include_removes:
                 eq_(
                     calls,
                     [
@@ -433,7 +429,7 @@ class ValidatorTest(_fixtures.FixtureTest):
                     ],
                 )
         else:
-            if include_removes:
+            if expect_include_removes:
                 eq_(
                     calls,
                     [