From ad18a0102cc2968914232814c6554763f15abbe3 Mon Sep 17 00:00:00 2001 From: Xavier Fernandez Date: Mon, 20 Feb 2023 14:28:21 +0100 Subject: [PATCH] Fixed #34355 -- Deprecated passing positional arguments to BaseConstraint. --- django/db/models/constraints.py | 27 +++++++++++++++++++++--- docs/internals/deprecation.txt | 3 +++ docs/ref/models/constraints.txt | 6 +++++- docs/releases/5.0.txt | 4 ++++ tests/constraints/tests.py | 37 +++++++++++++++++++++++++-------- 5 files changed, 64 insertions(+), 13 deletions(-) diff --git a/django/db/models/constraints.py b/django/db/models/constraints.py index 746d34d115..237b186050 100644 --- a/django/db/models/constraints.py +++ b/django/db/models/constraints.py @@ -1,3 +1,4 @@ +import warnings from enum import Enum from types import NoneType @@ -9,6 +10,7 @@ from django.db.models.lookups import Exact from django.db.models.query_utils import Q from django.db.models.sql.query import Query from django.db.utils import DEFAULT_DB_ALIAS +from django.utils.deprecation import RemovedInDjango60Warning from django.utils.translation import gettext_lazy as _ __all__ = ["BaseConstraint", "CheckConstraint", "Deferrable", "UniqueConstraint"] @@ -18,12 +20,31 @@ class BaseConstraint: default_violation_error_message = _("Constraint “%(name)s” is violated.") violation_error_message = None - def __init__(self, name, violation_error_message=None): + # RemovedInDjango60Warning: When the deprecation ends, replace with: + # def __init__(self, *, name, violation_error_message=None): + def __init__(self, *args, name=None, violation_error_message=None): + # RemovedInDjango60Warning. + if name is None and not args: + raise TypeError( + f"{self.__class__.__name__}.__init__() missing 1 required keyword-only " + f"argument: 'name'" + ) self.name = name if violation_error_message is not None: self.violation_error_message = violation_error_message else: self.violation_error_message = self.default_violation_error_message + # RemovedInDjango60Warning. + if args: + warnings.warn( + f"Passing positional arguments to {self.__class__.__name__} is " + f"deprecated.", + RemovedInDjango60Warning, + stacklevel=2, + ) + for arg, attr in zip(args, ["name", "violation_error_message"]): + if arg: + setattr(self, attr, arg) @property def contains_expressions(self): @@ -67,7 +88,7 @@ class CheckConstraint(BaseConstraint): raise TypeError( "CheckConstraint.check must be a Q instance or boolean expression." ) - super().__init__(name, violation_error_message=violation_error_message) + super().__init__(name=name, violation_error_message=violation_error_message) def _get_check_sql(self, model, schema_editor): query = Query(model=model, alias_cols=False) @@ -186,7 +207,7 @@ class UniqueConstraint(BaseConstraint): F(expression) if isinstance(expression, str) else expression for expression in expressions ) - super().__init__(name, violation_error_message=violation_error_message) + super().__init__(name=name, violation_error_message=violation_error_message) @property def contains_expressions(self): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 9ada56cf6c..b151020375 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -18,6 +18,9 @@ details on these changes. * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form renderers will be removed. +* Support for passing positional arguments to ``BaseConstraint`` will be + removed. + .. _deprecation-removed-in-5.1: 5.1 diff --git a/docs/ref/models/constraints.txt b/docs/ref/models/constraints.txt index 6ce00ae979..baaacd8754 100644 --- a/docs/ref/models/constraints.txt +++ b/docs/ref/models/constraints.txt @@ -48,12 +48,16 @@ option. ``BaseConstraint`` ================== -.. class:: BaseConstraint(name, violation_error_message=None) +.. class:: BaseConstraint(*, name, violation_error_message=None) Base class for all constraints. Subclasses must implement ``constraint_sql()``, ``create_sql()``, ``remove_sql()`` and ``validate()`` methods. + .. deprecated:: 5.0 + + Support for passing positional arguments is deprecated. + All constraints have the following parameters in common: ``name`` diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index 3e14e17972..14e767cfd8 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -267,6 +267,10 @@ Miscellaneous * The ``DjangoDivFormRenderer`` and ``Jinja2DivFormRenderer`` transitional form renderers are deprecated. +* Passing positional arguments ``name`` and ``violation_error_message`` to + :class:`~django.db.models.BaseConstraint` is deprecated in favor of + keyword-only arguments. + Features removed in 5.0 ======================= diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index e52d15233c..e486a35b7a 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -7,6 +7,8 @@ from django.db.models.constraints import BaseConstraint, UniqueConstraint from django.db.models.functions import Lower from django.db.transaction import atomic from django.test import SimpleTestCase, TestCase, skipIfDBFeature, skipUnlessDBFeature +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango60Warning from .models import ( ChildModel, @@ -26,48 +28,48 @@ def get_constraints(table): class BaseConstraintTests(SimpleTestCase): def test_constraint_sql(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") msg = "This method must be implemented by a subclass." with self.assertRaisesMessage(NotImplementedError, msg): c.constraint_sql(None, None) def test_contains_expressions(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") self.assertIs(c.contains_expressions, False) def test_create_sql(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") msg = "This method must be implemented by a subclass." with self.assertRaisesMessage(NotImplementedError, msg): c.create_sql(None, None) def test_remove_sql(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") msg = "This method must be implemented by a subclass." with self.assertRaisesMessage(NotImplementedError, msg): c.remove_sql(None, None) def test_validate(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") msg = "This method must be implemented by a subclass." with self.assertRaisesMessage(NotImplementedError, msg): c.validate(None, None) def test_default_violation_error_message(self): - c = BaseConstraint("name") + c = BaseConstraint(name="name") self.assertEqual( c.get_violation_error_message(), "Constraint “name” is violated." ) def test_custom_violation_error_message(self): c = BaseConstraint( - "base_name", violation_error_message="custom %(name)s message" + name="base_name", violation_error_message="custom %(name)s message" ) self.assertEqual(c.get_violation_error_message(), "custom base_name message") def test_custom_violation_error_message_clone(self): constraint = BaseConstraint( - "base_name", + name="base_name", violation_error_message="custom %(name)s message", ).clone() self.assertEqual( @@ -77,7 +79,7 @@ class BaseConstraintTests(SimpleTestCase): def test_deconstruction(self): constraint = BaseConstraint( - "base_name", + name="base_name", violation_error_message="custom %(name)s message", ) path, args, kwargs = constraint.deconstruct() @@ -88,6 +90,23 @@ class BaseConstraintTests(SimpleTestCase): {"name": "base_name", "violation_error_message": "custom %(name)s message"}, ) + def test_deprecation(self): + msg = "Passing positional arguments to BaseConstraint is deprecated." + with self.assertRaisesMessage(RemovedInDjango60Warning, msg): + BaseConstraint("name", "violation error message") + + def test_name_required(self): + msg = ( + "BaseConstraint.__init__() missing 1 required keyword-only argument: 'name'" + ) + with self.assertRaisesMessage(TypeError, msg): + BaseConstraint() + + @ignore_warnings(category=RemovedInDjango60Warning) + def test_positional_arguments(self): + c = BaseConstraint("name", "custom %(name)s message") + self.assertEqual(c.get_violation_error_message(), "custom name message") + class CheckConstraintTests(TestCase): def test_eq(self):