From 667105877e6723c6985399803a364848891513cc Mon Sep 17 00:00:00 2001 From: Gagaro Date: Mon, 31 Jan 2022 16:04:13 +0100 Subject: [PATCH] Fixed #30581 -- Added support for Meta.constraints validation. Thanks Simon Charette, Keryn Knight, and Mariusz Felisiak for reviews. --- django/contrib/postgres/constraints.py | 50 +++++- django/db/models/base.py | 93 ++++++++-- django/db/models/constraints.py | 92 +++++++++- django/db/models/expressions.py | 12 ++ django/forms/models.py | 3 +- docs/ref/checks.txt | 2 + docs/ref/contrib/postgres/constraints.txt | 19 +- docs/ref/models/constraints.txt | 74 ++++++-- docs/ref/models/instances.txt | 59 +++++-- docs/releases/4.1.txt | 13 ++ tests/constraints/models.py | 4 + tests/constraints/tests.py | 203 ++++++++++++++++++++-- tests/invalid_models_tests/test_models.py | 60 +++++++ tests/postgres_tests/test_constraints.py | 101 ++++++++--- tests/validation/models.py | 56 ++++++ tests/validation/test_constraints.py | 95 ++++++++++ tests/validation/test_unique.py | 4 - 17 files changed, 852 insertions(+), 88 deletions(-) create mode 100644 tests/validation/test_constraints.py diff --git a/django/contrib/postgres/constraints.py b/django/contrib/postgres/constraints.py index fa62379f67..8b59704063 100644 --- a/django/contrib/postgres/constraints.py +++ b/django/contrib/postgres/constraints.py @@ -1,11 +1,13 @@ import warnings from django.contrib.postgres.indexes import OpClass -from django.db import NotSupportedError +from django.core.exceptions import ValidationError +from django.db import DEFAULT_DB_ALIAS, NotSupportedError from django.db.backends.ddl_references import Expressions, Statement, Table from django.db.models import BaseConstraint, Deferrable, F, Q -from django.db.models.expressions import ExpressionList +from django.db.models.expressions import Exists, ExpressionList from django.db.models.indexes import IndexExpression +from django.db.models.lookups import PostgresOperatorLookup from django.db.models.sql import Query from django.utils.deprecation import RemovedInDjango50Warning @@ -32,6 +34,7 @@ class ExclusionConstraint(BaseConstraint): deferrable=None, include=None, opclasses=(), + violation_error_message=None, ): if index_type and index_type.lower() not in {"gist", "spgist"}: raise ValueError( @@ -78,7 +81,7 @@ class ExclusionConstraint(BaseConstraint): category=RemovedInDjango50Warning, stacklevel=2, ) - super().__init__(name=name) + super().__init__(name=name, violation_error_message=violation_error_message) def _get_expressions(self, schema_editor, query): expressions = [] @@ -197,3 +200,44 @@ class ExclusionConstraint(BaseConstraint): "" if not self.include else " include=%s" % repr(self.include), "" if not self.opclasses else " opclasses=%s" % repr(self.opclasses), ) + + def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS): + queryset = model._default_manager.using(using) + replacement_map = instance._get_field_value_map( + meta=model._meta, exclude=exclude + ) + lookups = [] + for idx, (expression, operator) in enumerate(self.expressions): + if isinstance(expression, str): + expression = F(expression) + if isinstance(expression, F): + if exclude and expression.name in exclude: + return + rhs_expression = replacement_map.get(expression.name, expression) + else: + rhs_expression = expression.replace_references(replacement_map) + if exclude: + for expr in rhs_expression.flatten(): + if isinstance(expr, F) and expr.name in exclude: + return + # Remove OpClass because it only has sense during the constraint + # creation. + if isinstance(expression, OpClass): + expression = expression.get_source_expressions()[0] + if isinstance(rhs_expression, OpClass): + rhs_expression = rhs_expression.get_source_expressions()[0] + lookup = PostgresOperatorLookup(lhs=expression, rhs=rhs_expression) + lookup.postgres_operator = operator + lookups.append(lookup) + queryset = queryset.filter(*lookups) + model_class_pk = instance._get_pk_val(model._meta) + if not instance._state.adding and model_class_pk is not None: + queryset = queryset.exclude(pk=model_class_pk) + if not self.condition: + if queryset.exists(): + raise ValidationError(self.get_violation_error_message()) + else: + if (self.condition & Exists(queryset.filter(self.condition))).check( + replacement_map, using=using + ): + raise ValidationError(self.get_violation_error_message()) diff --git a/django/db/models/base.py b/django/db/models/base.py index 9aa6d7596a..a2f594041e 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -28,6 +28,7 @@ from django.db.models import NOT_PROVIDED, ExpressionWrapper, IntegerField, Max, from django.db.models.constants import LOOKUP_SEP from django.db.models.constraints import CheckConstraint, UniqueConstraint from django.db.models.deletion import CASCADE, Collector +from django.db.models.expressions import RawSQL from django.db.models.fields.related import ( ForeignObjectRel, OneToOneField, @@ -1189,6 +1190,16 @@ class Model(metaclass=ModelBase): setattr(self, cachename, obj) return getattr(self, cachename) + def _get_field_value_map(self, meta, exclude=None): + if exclude is None: + exclude = set() + meta = meta or self._meta + return { + field.name: Value(getattr(self, field.attname), field) + for field in meta.local_concrete_fields + if field.name not in exclude + } + def prepare_database_save(self, field): if self.pk is None: raise ValueError( @@ -1221,7 +1232,7 @@ class Model(metaclass=ModelBase): if errors: raise ValidationError(errors) - def _get_unique_checks(self, exclude=None): + def _get_unique_checks(self, exclude=None, include_meta_constraints=False): """ Return a list of checks to perform. Since validate_unique() could be called from a ModelForm, some fields may have been excluded; we can't @@ -1234,13 +1245,15 @@ class Model(metaclass=ModelBase): unique_checks = [] unique_togethers = [(self.__class__, self._meta.unique_together)] - constraints = [(self.__class__, self._meta.total_unique_constraints)] + constraints = [] + if include_meta_constraints: + constraints = [(self.__class__, self._meta.total_unique_constraints)] for parent_class in self._meta.get_parent_list(): if parent_class._meta.unique_together: unique_togethers.append( (parent_class, parent_class._meta.unique_together) ) - if parent_class._meta.total_unique_constraints: + if include_meta_constraints and parent_class._meta.total_unique_constraints: constraints.append( (parent_class, parent_class._meta.total_unique_constraints) ) @@ -1251,10 +1264,11 @@ class Model(metaclass=ModelBase): # Add the check if the field isn't excluded. unique_checks.append((model_class, tuple(check))) - for model_class, model_constraints in constraints: - for constraint in model_constraints: - if not any(name in exclude for name in constraint.fields): - unique_checks.append((model_class, constraint.fields)) + if include_meta_constraints: + for model_class, model_constraints in constraints: + for constraint in model_constraints: + if not any(name in exclude for name in constraint.fields): + unique_checks.append((model_class, constraint.fields)) # These are checks for the unique_for_. date_checks = [] @@ -1410,10 +1424,35 @@ class Model(metaclass=ModelBase): params=params, ) - def full_clean(self, exclude=None, validate_unique=True): + def get_constraints(self): + constraints = [(self.__class__, self._meta.constraints)] + for parent_class in self._meta.get_parent_list(): + if parent_class._meta.constraints: + constraints.append((parent_class, parent_class._meta.constraints)) + return constraints + + def validate_constraints(self, exclude=None): + constraints = self.get_constraints() + using = router.db_for_write(self.__class__, instance=self) + + errors = {} + for model_class, model_constraints in constraints: + for constraint in model_constraints: + try: + constraint.validate(model_class, self, exclude=exclude, using=using) + except ValidationError as e: + if e.code == "unique" and len(constraint.fields) == 1: + errors.setdefault(constraint.fields[0], []).append(e) + else: + errors = e.update_error_dict(errors) + if errors: + raise ValidationError(errors) + + def full_clean(self, exclude=None, validate_unique=True, validate_constraints=True): """ - Call clean_fields(), clean(), and validate_unique() on the model. - Raise a ValidationError for any errors that occur. + Call clean_fields(), clean(), validate_unique(), and + validate_constraints() on the model. Raise a ValidationError for any + errors that occur. """ errors = {} if exclude is None: @@ -1443,6 +1482,16 @@ class Model(metaclass=ModelBase): except ValidationError as e: errors = e.update_error_dict(errors) + # Run constraints checks, but only for fields that passed validation. + if validate_constraints: + for name in errors: + if name != NON_FIELD_ERRORS and name not in exclude: + exclude.add(name) + try: + self.validate_constraints(exclude=exclude) + except ValidationError as e: + errors = e.update_error_dict(errors) + if errors: raise ValidationError(errors) @@ -2339,8 +2388,28 @@ class Model(metaclass=ModelBase): connection.features.supports_table_check_constraints or "supports_table_check_constraints" not in cls._meta.required_db_features - ) and isinstance(constraint.check, Q): - references.update(cls._get_expr_references(constraint.check)) + ): + if isinstance(constraint.check, Q): + references.update( + cls._get_expr_references(constraint.check) + ) + if any( + isinstance(expr, RawSQL) + for expr in constraint.check.flatten() + ): + errors.append( + checks.Warning( + f"Check constraint {constraint.name!r} contains " + f"RawSQL() expression and won't be validated " + f"during the model full_clean().", + hint=( + "Silence this warning if you don't care about " + "it." + ), + obj=cls, + id="models.W045", + ), + ) for field_name, *lookups in references: # pk is an alias that won't be found by opts.get_field. if field_name != "pk": diff --git a/django/db/models/constraints.py b/django/db/models/constraints.py index 3577c638ef..9949b50b1e 100644 --- a/django/db/models/constraints.py +++ b/django/db/models/constraints.py @@ -1,16 +1,25 @@ from enum import Enum -from django.db.models.expressions import ExpressionList, F +from django.core.exceptions import FieldError, ValidationError +from django.db import connections +from django.db.models.expressions import Exists, ExpressionList, F from django.db.models.indexes import IndexExpression +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.translation import gettext_lazy as _ __all__ = ["BaseConstraint", "CheckConstraint", "Deferrable", "UniqueConstraint"] class BaseConstraint: - def __init__(self, name): + violation_error_message = _("Constraint “%(name)s” is violated.") + + def __init__(self, name, violation_error_message=None): self.name = name + if violation_error_message is not None: + self.violation_error_message = violation_error_message @property def contains_expressions(self): @@ -25,6 +34,12 @@ class BaseConstraint: def remove_sql(self, model, schema_editor): raise NotImplementedError("This method must be implemented by a subclass.") + def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS): + raise NotImplementedError("This method must be implemented by a subclass.") + + def get_violation_error_message(self): + return self.violation_error_message % {"name": self.name} + def deconstruct(self): path = "%s.%s" % (self.__class__.__module__, self.__class__.__name__) path = path.replace("django.db.models.constraints", "django.db.models") @@ -36,13 +51,13 @@ class BaseConstraint: class CheckConstraint(BaseConstraint): - def __init__(self, *, check, name): + def __init__(self, *, check, name, violation_error_message=None): self.check = check if not getattr(check, "conditional", False): raise TypeError( "CheckConstraint.check must be a Q instance or boolean expression." ) - super().__init__(name) + super().__init__(name, violation_error_message=violation_error_message) def _get_check_sql(self, model, schema_editor): query = Query(model=model, alias_cols=False) @@ -62,6 +77,14 @@ class CheckConstraint(BaseConstraint): def remove_sql(self, model, schema_editor): return schema_editor._delete_check_sql(model, self.name) + def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS): + against = instance._get_field_value_map(meta=model._meta, exclude=exclude) + try: + if not Q(self.check).check(against, using=using): + raise ValidationError(self.get_violation_error_message()) + except FieldError: + pass + def __repr__(self): return "<%s: check=%s name=%s>" % ( self.__class__.__qualname__, @@ -99,6 +122,7 @@ class UniqueConstraint(BaseConstraint): deferrable=None, include=None, opclasses=(), + violation_error_message=None, ): if not name: raise ValueError("A unique constraint must be named.") @@ -148,7 +172,7 @@ class UniqueConstraint(BaseConstraint): F(expression) if isinstance(expression, str) else expression for expression in expressions ) - super().__init__(name) + super().__init__(name, violation_error_message=violation_error_message) @property def contains_expressions(self): @@ -265,3 +289,61 @@ class UniqueConstraint(BaseConstraint): if self.opclasses: kwargs["opclasses"] = self.opclasses return path, self.expressions, kwargs + + def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS): + queryset = model._default_manager.using(using) + if self.fields: + lookup_kwargs = {} + for field_name in self.fields: + if exclude and field_name in exclude: + return + field = model._meta.get_field(field_name) + lookup_value = getattr(instance, field.attname) + if lookup_value is None or ( + lookup_value == "" + and connections[using].features.interprets_empty_strings_as_nulls + ): + # A composite constraint containing NULL value cannot cause + # a violation since NULL != NULL in SQL. + return + lookup_kwargs[field.name] = lookup_value + queryset = queryset.filter(**lookup_kwargs) + else: + # Ignore constraints with excluded fields. + if exclude: + for expression in self.expressions: + for expr in expression.flatten(): + if isinstance(expr, F) and expr.name in exclude: + return + replacement_map = instance._get_field_value_map( + meta=model._meta, exclude=exclude + ) + expressions = [ + Exact(expr, expr.replace_references(replacement_map)) + for expr in self.expressions + ] + queryset = queryset.filter(*expressions) + model_class_pk = instance._get_pk_val(model._meta) + if not instance._state.adding and model_class_pk is not None: + queryset = queryset.exclude(pk=model_class_pk) + if not self.condition: + if queryset.exists(): + if self.expressions: + raise ValidationError(self.get_violation_error_message()) + # When fields are defined, use the unique_error_message() for + # backward compatibility. + for model, constraints in instance.get_constraints(): + for constraint in constraints: + if constraint is self: + raise ValidationError( + instance.unique_error_message(model, self.fields) + ) + else: + against = instance._get_field_value_map(meta=model._meta, exclude=exclude) + try: + if (self.condition & Exists(queryset.filter(self.condition))).check( + against, using=using + ): + raise ValidationError(self.get_violation_error_message()) + except FieldError: + pass diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index add7b20ba3..4bc55a1c89 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -387,6 +387,18 @@ class BaseExpression: ) return clone + def replace_references(self, references_map): + clone = self.copy() + clone.set_source_expressions( + [ + references_map.get(expr.name, expr) + if isinstance(expr, F) + else expr.replace_references(references_map) + for expr in self.get_source_expressions() + ] + ) + return clone + def copy(self): return copy.copy(self) diff --git a/django/forms/models.py b/django/forms/models.py index 68608f4aa7..8a4390fc67 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -806,7 +806,8 @@ class BaseModelFormSet(BaseFormSet): for form in valid_forms: exclude = form._get_validation_exclusions() unique_checks, date_checks = form.instance._get_unique_checks( - exclude=exclude + exclude=exclude, + include_meta_constraints=True, ) all_unique_checks.update(unique_checks) all_date_checks.update(date_checks) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 3228d71f08..233d999fe7 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -395,6 +395,8 @@ Models * **models.W043**: ```` does not support indexes on expressions. * **models.W044**: ```` does not support unique constraints on expressions. +* **models.W045**: Check constraint ```` contains ``RawSQL()`` + expression and won't be validated during the model ``full_clean()``. Security -------- diff --git a/docs/ref/contrib/postgres/constraints.txt b/docs/ref/contrib/postgres/constraints.txt index 1c283c367e..26c6356693 100644 --- a/docs/ref/contrib/postgres/constraints.txt +++ b/docs/ref/contrib/postgres/constraints.txt @@ -12,7 +12,7 @@ PostgreSQL supports additional data integrity constraints available from the ``ExclusionConstraint`` ======================= -.. class:: ExclusionConstraint(*, name, expressions, index_type=None, condition=None, deferrable=None, include=None, opclasses=()) +.. class:: ExclusionConstraint(*, name, expressions, index_type=None, condition=None, deferrable=None, include=None, opclasses=(), violation_error_message=None) Creates an exclusion constraint in the database. Internally, PostgreSQL implements exclusion constraints using indexes. The default index type is @@ -27,6 +27,14 @@ PostgreSQL supports additional data integrity constraints available from the :exc:`~django.db.IntegrityError` is raised. Similarly, when update conflicts with an existing row. + Exclusion constraints are checked during the :ref:`model validation + `. + + .. versionchanged:: 4.1 + + In older versions, exclusion constraints were not checked during model + validation. + ``name`` -------- @@ -165,6 +173,15 @@ creates an exclusion constraint on ``circle`` using ``circle_ops``. :class:`OpClass() ` in :attr:`~ExclusionConstraint.expressions`. +``violation_error_message`` +--------------------------- + +.. versionadded:: 4.1 + +The error message used when ``ValidationError`` is raised during +:ref:`model validation `. Defaults to +:attr:`.BaseConstraint.violation_error_message`. + Examples -------- diff --git a/docs/ref/models/constraints.txt b/docs/ref/models/constraints.txt index 52bb8b48e0..9b1e110f85 100644 --- a/docs/ref/models/constraints.txt +++ b/docs/ref/models/constraints.txt @@ -31,24 +31,21 @@ option. .. admonition:: Validation of Constraints - In general constraints are **not** checked during ``full_clean()``, and do - not raise ``ValidationError``\s. Rather you'll get a database integrity - error on ``save()``. ``UniqueConstraint``\s without a - :attr:`~UniqueConstraint.condition` (i.e. non-partial unique constraints) - and :attr:`~UniqueConstraint.expressions` (i.e. non-functional unique - constraints) are different in this regard, in that they leverage the - existing ``validate_unique()`` logic, and thus enable two-stage validation. - In addition to ``IntegrityError`` on ``save()``, ``ValidationError`` is - also raised during model validation when the ``UniqueConstraint`` is - violated. + Constraints are checked during the :ref:`model validation + `. + +.. versionchanged:: 4.1 + + In older versions, constraints were not checked during model validation. ``BaseConstraint`` ================== -.. class:: BaseConstraint(name) +.. class:: BaseConstraint(name, violation_error_message=None) Base class for all constraints. Subclasses must implement - ``constraint_sql()``, ``create_sql()``, and ``remove_sql()`` methods. + ``constraint_sql()``, ``create_sql()``, ``remove_sql()`` and + ``validate()`` methods. All constraints have the following parameters in common: @@ -60,10 +57,37 @@ All constraints have the following parameters in common: The name of the constraint. You must always specify a unique name for the constraint. +``violation_error_message`` +--------------------------- + +.. versionadded:: 4.1 + +.. attribute:: BaseConstraint.violation_error_message + +The error message used when ``ValidationError`` is raised during +:ref:`model validation `. Defaults to +``"Constraint “%(name)s” is violated."``. + +``validate()`` +-------------- + +.. versionadded:: 4.1 + +.. method:: BaseConstraint.validate(model, instance, exclude=None, using=DEFAULT_DB_ALIAS) + +Validates that the constraint, defined on ``model``, is respected on the +``instance``. This will do a query on the database to ensure that the +constraint is respected. If fields in the ``exclude`` list are needed to +validate the constraint, the constraint is ignored. + +Raise a ``ValidationError`` if the constraint is violated. + +This method must be implemented by a subclass. + ``CheckConstraint`` =================== -.. class:: CheckConstraint(*, check, name) +.. class:: CheckConstraint(*, check, name, violation_error_message=None) Creates a check constraint in the database. @@ -78,10 +102,14 @@ specifies the check you want the constraint to enforce. For example, ``CheckConstraint(check=Q(age__gte=18), name='age_gte_18')`` ensures the age field is never less than 18. +.. versionchanged:: 4.1 + + The ``violation_error_message`` argument was added. + ``UniqueConstraint`` ==================== -.. class:: UniqueConstraint(*expressions, fields=(), name=None, condition=None, deferrable=None, include=None, opclasses=()) +.. class:: UniqueConstraint(*expressions, fields=(), name=None, condition=None, deferrable=None, include=None, opclasses=(), violation_error_message=None) Creates a unique constraint in the database. @@ -203,3 +231,21 @@ For example:: creates a unique index on ``username`` using ``varchar_pattern_ops``. ``opclasses`` are ignored for databases besides PostgreSQL. + +``violation_error_message`` +--------------------------- + +.. versionadded:: 4.1 + +.. attribute:: UniqueConstraint.violation_error_message + +The error message used when ``ValidationError`` is raised during +:ref:`model validation `. Defaults to +:attr:`.BaseConstraint.violation_error_message`. + +This message is *not used* for :class:`UniqueConstraint`\s with +:attr:`~UniqueConstraint.fields` and without a +:attr:`~UniqueConstraint.condition`. Such :class:`~UniqueConstraint`\s show the +same message as constraints defined with +:attr:`.Field.unique` or in +:attr:`Meta.unique_together `. diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index e98377b8bf..b3323eaa0e 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -198,9 +198,10 @@ There are three steps involved in validating a model: 1. Validate the model fields - :meth:`Model.clean_fields()` 2. Validate the model as a whole - :meth:`Model.clean()` 3. Validate the field uniqueness - :meth:`Model.validate_unique()` +4. Validate the constraints - :meth:`Model.validate_constraints` -All three steps are performed when you call a model's -:meth:`~Model.full_clean()` method. +All four steps are performed when you call a model's :meth:`~Model.full_clean` +method. When you use a :class:`~django.forms.ModelForm`, the call to :meth:`~django.forms.Form.is_valid()` will perform these validation steps for @@ -210,12 +211,18 @@ need to call a model's :meth:`~Model.full_clean()` method if you plan to handle validation errors yourself, or if you have excluded fields from the :class:`~django.forms.ModelForm` that require validation. -.. method:: Model.full_clean(exclude=None, validate_unique=True) +.. versionchanged:: 4.1 -This method calls :meth:`Model.clean_fields()`, :meth:`Model.clean()`, and -:meth:`Model.validate_unique()` (if ``validate_unique`` is ``True``), in that -order and raises a :exc:`~django.core.exceptions.ValidationError` that has a -``message_dict`` attribute containing errors from all three stages. + In older versions, constraints were not checked during the model + validation. + +.. method:: Model.full_clean(exclude=None, validate_unique=True, validate_constraints=True) + +This method calls :meth:`Model.clean_fields()`, :meth:`Model.clean()`, +:meth:`Model.validate_unique()` (if ``validate_unique`` is ``True``), and +:meth:`Model.validate_constraints()` (if ``validate_constraints`` is ``True``) +in that order and raises a :exc:`~django.core.exceptions.ValidationError` that +has a ``message_dict`` attribute containing errors from all four stages. The optional ``exclude`` argument can be used to provide a list of field names that can be excluded from validation and cleaning. @@ -238,6 +245,10 @@ models. For example:: The first step ``full_clean()`` performs is to clean each individual field. +.. versionchanged:: 4.1 + + The ``validate_constraints`` argument was added. + .. method:: Model.clean_fields(exclude=None) This method will validate all fields on your model. The optional ``exclude`` @@ -306,7 +317,7 @@ pass a dictionary mapping field names to errors:: 'pub_date': ValidationError(_('Invalid date.'), code='invalid'), }) -Finally, ``full_clean()`` will check any unique constraints on your model. +Then, ``full_clean()`` will check unique constraints on your model. .. admonition:: How to raise field-specific validation errors if those fields don't appear in a ``ModelForm`` @@ -339,16 +350,40 @@ Finally, ``full_clean()`` will check any unique constraints on your model. .. method:: Model.validate_unique(exclude=None) -This method is similar to :meth:`~Model.clean_fields`, but validates all -uniqueness constraints on your model instead of individual field values. The -optional ``exclude`` argument allows you to provide a list of field names to -exclude from validation. It will raise a +This method is similar to :meth:`~Model.clean_fields`, but validates +uniqueness constraints defined via :attr:`.Field.unique`, +:attr:`.Field.unique_for_date`, :attr:`.Field.unique_for_month`, +:attr:`.Field.unique_for_year`, or :attr:`Meta.unique_together +` on your model instead of individual +field values. The optional ``exclude`` argument allows you to provide a list of +field names to exclude from validation. It will raise a :exc:`~django.core.exceptions.ValidationError` if any fields fail validation. +:class:`~django.db.models.UniqueConstraint`\s defined in the +:attr:`Meta.constraints ` are validated +by :meth:`Model.validate_constraints`. + Note that if you provide an ``exclude`` argument to ``validate_unique()``, any :attr:`~django.db.models.Options.unique_together` constraint involving one of the fields you provided will not be checked. +Finally, ``full_clean()`` will check any other constraints on your model. + +.. versionchanged:: 4.1 + + In older versions, :class:`~django.db.models.UniqueConstraint`\s were + validated by ``validate_unique()``. + +.. method:: Model.validate_constraints(exclude=None) + +.. versionadded:: 4.1 + +This method validates all constraints defined in +:attr:`Meta.constraints `. The +optional ``exclude`` argument allows you to provide a list of field names to +exclude from validation. It will raise a +:exc:`~django.core.exceptions.ValidationError` if any constraints fail +validation. Saving objects ============== diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 1bd3fe68b0..fa5990393b 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -65,6 +65,15 @@ advantage of developments in the ORM's asynchronous support as it evolves. See :ref:`async-queries` for details and limitations. +Validation of Constraints +------------------------- + +:class:`Check `, +:class:`unique `, and :class:`exclusion +` constraints defined +in the :attr:`Meta.constraints ` option +are now checked during :ref:`model validation `. + .. _csrf-cookie-masked-usage: ``CSRF_COOKIE_MASKED`` setting @@ -551,6 +560,10 @@ Miscellaneous * The undocumented ``django.contrib.auth.views.SuccessURLAllowedHostsMixin`` mixin is replaced by ``RedirectURLMixin``. +* :class:`~django.db.models.BaseConstraint` subclasses must implement + :meth:`~django.db.models.BaseConstraint.validate` method to allow those + constraints to be used for validation. + .. _deprecated-features-4.1: Features deprecated in 4.1 diff --git a/tests/constraints/models.py b/tests/constraints/models.py index 245693e847..3b349d204e 100644 --- a/tests/constraints/models.py +++ b/tests/constraints/models.py @@ -38,6 +38,10 @@ class UniqueConstraintProduct(models.Model): ] +class ChildUniqueConstraintProduct(UniqueConstraintProduct): + pass + + class UniqueConstraintConditionProduct(models.Model): name = models.CharField(max_length=255) color = models.CharField(max_length=32, null=True) diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index 58960fa5a3..d9e377438e 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -10,6 +10,7 @@ from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature from .models import ( ChildModel, + ChildUniqueConstraintProduct, Product, UniqueConstraintConditionProduct, UniqueConstraintDeferrable, @@ -46,6 +47,24 @@ class BaseConstraintTests(SimpleTestCase): with self.assertRaisesMessage(NotImplementedError, msg): c.remove_sql(None, None) + def test_validate(self): + c = BaseConstraint("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") + 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" + ) + self.assertEqual(c.get_violation_error_message(), "custom base_name message") + class CheckConstraintTests(TestCase): def test_eq(self): @@ -122,16 +141,60 @@ class CheckConstraintTests(TestCase): constraints = get_constraints(ChildModel._meta.db_table) self.assertIn("constraints_childmodel_adult", constraints) + def test_validate(self): + check = models.Q(price__gt=models.F("discounted_price")) + constraint = models.CheckConstraint(check=check, name="price") + # Invalid product. + invalid_product = Product(price=10, discounted_price=42) + with self.assertRaises(ValidationError): + constraint.validate(Product, invalid_product) + with self.assertRaises(ValidationError): + constraint.validate(Product, invalid_product, exclude={"unit"}) + # Fields used by the check constraint are excluded. + constraint.validate(Product, invalid_product, exclude={"price"}) + constraint.validate(Product, invalid_product, exclude={"discounted_price"}) + constraint.validate( + Product, + invalid_product, + exclude={"discounted_price", "price"}, + ) + # Valid product. + constraint.validate(Product, Product(price=10, discounted_price=5)) + + def test_validate_boolean_expressions(self): + constraint = models.CheckConstraint( + check=models.expressions.ExpressionWrapper( + models.Q(price__gt=500) | models.Q(price__lt=500), + output_field=models.BooleanField(), + ), + name="price_neq_500_wrap", + ) + msg = f"Constraint “{constraint.name}” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate(Product, Product(price=500, discounted_price=5)) + constraint.validate(Product, Product(price=501, discounted_price=5)) + constraint.validate(Product, Product(price=499, discounted_price=5)) + + def test_validate_rawsql_expressions_noop(self): + constraint = models.CheckConstraint( + check=models.expressions.RawSQL( + "price < %s OR price > %s", + (500, 500), + output_field=models.BooleanField(), + ), + name="price_neq_500_raw", + ) + # RawSQL can not be checked and is always considered valid. + constraint.validate(Product, Product(price=500, discounted_price=5)) + constraint.validate(Product, Product(price=501, discounted_price=5)) + constraint.validate(Product, Product(price=499, discounted_price=5)) + class UniqueConstraintTests(TestCase): @classmethod def setUpTestData(cls): - cls.p1, cls.p2 = UniqueConstraintProduct.objects.bulk_create( - [ - UniqueConstraintProduct(name="p1", color="red"), - UniqueConstraintProduct(name="p2"), - ] - ) + cls.p1 = UniqueConstraintProduct.objects.create(name="p1", color="red") + cls.p2 = UniqueConstraintProduct.objects.create(name="p2") def test_eq(self): self.assertEqual( @@ -415,15 +478,135 @@ class UniqueConstraintTests(TestCase): with self.assertRaisesMessage(ValidationError, msg): UniqueConstraintProduct( name=self.p1.name, color=self.p1.color - ).validate_unique() + ).validate_constraints() @skipUnlessDBFeature("supports_partial_indexes") def test_model_validation_with_condition(self): - """Partial unique constraints are ignored by Model.validate_unique().""" + """ + Partial unique constraints are not ignored by + Model.validate_constraints(). + """ obj1 = UniqueConstraintConditionProduct.objects.create(name="p1", color="red") obj2 = UniqueConstraintConditionProduct.objects.create(name="p2") - UniqueConstraintConditionProduct(name=obj1.name, color="blue").validate_unique() - UniqueConstraintConditionProduct(name=obj2.name).validate_unique() + UniqueConstraintConditionProduct( + name=obj1.name, color="blue" + ).validate_constraints() + msg = "Constraint “name_without_color_uniq” is violated." + with self.assertRaisesMessage(ValidationError, msg): + UniqueConstraintConditionProduct(name=obj2.name).validate_constraints() + + def test_validate(self): + constraint = UniqueConstraintProduct._meta.constraints[0] + msg = "Unique constraint product with this Name and Color already exists." + non_unique_product = UniqueConstraintProduct( + name=self.p1.name, color=self.p1.color + ) + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate(UniqueConstraintProduct, non_unique_product) + # Null values are ignored. + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name=self.p2.name, color=None), + ) + # Existing instances have their existing row excluded. + constraint.validate(UniqueConstraintProduct, self.p1) + # Unique fields are excluded. + constraint.validate( + UniqueConstraintProduct, + non_unique_product, + exclude={"name"}, + ) + constraint.validate( + UniqueConstraintProduct, + non_unique_product, + exclude={"color"}, + ) + constraint.validate( + UniqueConstraintProduct, + non_unique_product, + exclude={"name", "color"}, + ) + # Validation on a child instance. + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate( + UniqueConstraintProduct, + ChildUniqueConstraintProduct(name=self.p1.name, color=self.p1.color), + ) + + @skipUnlessDBFeature("supports_partial_indexes") + def test_validate_condition(self): + p1 = UniqueConstraintConditionProduct.objects.create(name="p1") + constraint = UniqueConstraintConditionProduct._meta.constraints[0] + msg = "Constraint “name_without_color_uniq” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate( + UniqueConstraintConditionProduct, + UniqueConstraintConditionProduct(name=p1.name, color=None), + ) + # Values not matching condition are ignored. + constraint.validate( + UniqueConstraintConditionProduct, + UniqueConstraintConditionProduct(name=p1.name, color="anything-but-none"), + ) + # Existing instances have their existing row excluded. + constraint.validate(UniqueConstraintConditionProduct, p1) + # Unique field is excluded. + constraint.validate( + UniqueConstraintConditionProduct, + UniqueConstraintConditionProduct(name=p1.name, color=None), + exclude={"name"}, + ) + + def test_validate_expression(self): + constraint = models.UniqueConstraint(Lower("name"), name="name_lower_uniq") + msg = "Constraint “name_lower_uniq” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name=self.p1.name.upper()), + ) + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name="another-name"), + ) + # Existing instances have their existing row excluded. + constraint.validate(UniqueConstraintProduct, self.p1) + # Unique field is excluded. + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name=self.p1.name.upper()), + exclude={"name"}, + ) + + def test_validate_expression_condition(self): + constraint = models.UniqueConstraint( + Lower("name"), + name="name_lower_without_color_uniq", + condition=models.Q(color__isnull=True), + ) + non_unique_product = UniqueConstraintProduct(name=self.p2.name.upper()) + msg = "Constraint “name_lower_without_color_uniq” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate(UniqueConstraintProduct, non_unique_product) + # Values not matching condition are ignored. + constraint.validate( + UniqueConstraintProduct, + UniqueConstraintProduct(name=self.p1.name, color=self.p1.color), + ) + # Existing instances have their existing row excluded. + constraint.validate(UniqueConstraintProduct, self.p2) + # Unique field is excluded. + constraint.validate( + UniqueConstraintProduct, + non_unique_product, + exclude={"name"}, + ) + # Field from a condition is excluded. + constraint.validate( + UniqueConstraintProduct, + non_unique_product, + exclude={"color"}, + ) def test_name(self): constraints = get_constraints(UniqueConstraintProduct._meta.db_table) diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 5ea830d0ec..08aca62850 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -2198,6 +2198,66 @@ class ConstraintsTests(TestCase): ] self.assertCountEqual(errors, expected_errors) + def test_check_constraint_raw_sql_check(self): + class Model(models.Model): + class Meta: + required_db_features = {"supports_table_check_constraints"} + constraints = [ + models.CheckConstraint(check=models.Q(id__gt=0), name="q_check"), + models.CheckConstraint( + check=models.ExpressionWrapper( + models.Q(price__gt=20), + output_field=models.BooleanField(), + ), + name="expression_wrapper_check", + ), + models.CheckConstraint( + check=models.expressions.RawSQL( + "id = 0", + params=(), + output_field=models.BooleanField(), + ), + name="raw_sql_check", + ), + models.CheckConstraint( + check=models.Q( + models.ExpressionWrapper( + models.Q( + models.expressions.RawSQL( + "id = 0", + params=(), + output_field=models.BooleanField(), + ) + ), + output_field=models.BooleanField(), + ) + ), + name="nested_raw_sql_check", + ), + ] + + expected_warnings = ( + [ + Warning( + "Check constraint 'raw_sql_check' contains RawSQL() expression and " + "won't be validated during the model full_clean().", + hint="Silence this warning if you don't care about it.", + obj=Model, + id="models.W045", + ), + Warning( + "Check constraint 'nested_raw_sql_check' contains RawSQL() " + "expression and won't be validated during the model full_clean().", + hint="Silence this warning if you don't care about it.", + obj=Model, + id="models.W045", + ), + ] + if connection.features.supports_table_check_constraints + else [] + ) + self.assertEqual(Model.check(databases=self.databases), expected_warnings) + def test_unique_constraint_with_condition(self): class Model(models.Model): age = models.IntegerField() diff --git a/tests/postgres_tests/test_constraints.py b/tests/postgres_tests/test_constraints.py index 377af41042..b8e53eb4a0 100644 --- a/tests/postgres_tests/test_constraints.py +++ b/tests/postgres_tests/test_constraints.py @@ -2,6 +2,7 @@ import datetime from unittest import mock from django.contrib.postgres.indexes import OpClass +from django.core.exceptions import ValidationError from django.db import IntegrityError, NotSupportedError, connection, transaction from django.db.models import ( CheckConstraint, @@ -612,18 +613,26 @@ class ExclusionConstraintTests(PostgreSQLTestCase): timezone.datetime(2018, 6, 28), timezone.datetime(2018, 6, 29), ] - HotelReservation.objects.create( + reservation = HotelReservation.objects.create( datespan=DateRange(datetimes[0].date(), datetimes[1].date()), start=datetimes[0], end=datetimes[1], room=room102, ) + constraint.validate(HotelReservation, reservation) HotelReservation.objects.create( datespan=DateRange(datetimes[1].date(), datetimes[3].date()), start=datetimes[1], end=datetimes[3], room=room102, ) + HotelReservation.objects.create( + datespan=DateRange(datetimes[3].date(), datetimes[4].date()), + start=datetimes[3], + end=datetimes[4], + room=room102, + cancelled=True, + ) # Overlap dates. with self.assertRaises(IntegrityError), transaction.atomic(): reservation = HotelReservation( @@ -632,33 +641,58 @@ class ExclusionConstraintTests(PostgreSQLTestCase): end=datetimes[2], room=room102, ) + msg = f"Constraint “{constraint.name}” is violated." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate(HotelReservation, reservation) reservation.save() # Valid range. - HotelReservation.objects.bulk_create( - [ - # Other room. - HotelReservation( - datespan=(datetimes[1].date(), datetimes[2].date()), - start=datetimes[1], - end=datetimes[2], - room=room101, - ), - # Cancelled reservation. - HotelReservation( - datespan=(datetimes[1].date(), datetimes[1].date()), - start=datetimes[1], - end=datetimes[2], - room=room102, - cancelled=True, - ), - # Other adjacent dates. - HotelReservation( - datespan=(datetimes[3].date(), datetimes[4].date()), - start=datetimes[3], - end=datetimes[4], - room=room102, - ), - ] + other_valid_reservations = [ + # Other room. + HotelReservation( + datespan=(datetimes[1].date(), datetimes[2].date()), + start=datetimes[1], + end=datetimes[2], + room=room101, + ), + # Cancelled reservation. + HotelReservation( + datespan=(datetimes[1].date(), datetimes[1].date()), + start=datetimes[1], + end=datetimes[2], + room=room102, + cancelled=True, + ), + # Other adjacent dates. + HotelReservation( + datespan=(datetimes[3].date(), datetimes[4].date()), + start=datetimes[3], + end=datetimes[4], + room=room102, + ), + ] + for reservation in other_valid_reservations: + constraint.validate(HotelReservation, reservation) + HotelReservation.objects.bulk_create(other_valid_reservations) + # Excluded fields. + constraint.validate( + HotelReservation, + HotelReservation( + datespan=(datetimes[1].date(), datetimes[2].date()), + start=datetimes[1], + end=datetimes[2], + room=room102, + ), + exclude={"room"}, + ) + constraint.validate( + HotelReservation, + HotelReservation( + datespan=(datetimes[1].date(), datetimes[2].date()), + start=datetimes[1], + end=datetimes[2], + room=room102, + ), + exclude={"datespan", "start", "end", "room"}, ) @ignore_warnings(category=RemovedInDjango50Warning) @@ -731,6 +765,21 @@ class ExclusionConstraintTests(PostgreSQLTestCase): constraint_name, self.get_constraints(RangesModel._meta.db_table) ) + def test_validate_range_adjacent(self): + constraint = ExclusionConstraint( + name="ints_adjacent", + expressions=[("ints", RangeOperators.ADJACENT_TO)], + violation_error_message="Custom error message.", + ) + range_obj = RangesModel.objects.create(ints=(20, 50)) + constraint.validate(RangesModel, range_obj) + msg = "Custom error message." + with self.assertRaisesMessage(ValidationError, msg): + constraint.validate(RangesModel, RangesModel(ints=(10, 20))) + constraint.validate(RangesModel, RangesModel(ints=(10, 19))) + constraint.validate(RangesModel, RangesModel(ints=(51, 60))) + constraint.validate(RangesModel, RangesModel(ints=(10, 20)), exclude={"ints"}) + def test_expressions_with_params(self): constraint_name = "scene_left_equal" self.assertNotIn(constraint_name, self.get_constraints(Scene._meta.db_table)) diff --git a/tests/validation/models.py b/tests/validation/models.py index 6934fa017c..8919a69310 100644 --- a/tests/validation/models.py +++ b/tests/validation/models.py @@ -161,3 +161,59 @@ class UniqueFuncConstraintModel(models.Model): constraints = [ models.UniqueConstraint(Lower("field"), name="func_lower_field_uq"), ] + + +class Product(models.Model): + price = models.IntegerField(null=True) + discounted_price = models.IntegerField(null=True) + + class Meta: + required_db_features = { + "supports_table_check_constraints", + } + constraints = [ + models.CheckConstraint( + check=models.Q(price__gt=models.F("discounted_price")), + name="price_gt_discounted_price_validation", + ), + ] + + +class ChildProduct(Product): + class Meta: + required_db_features = { + "supports_table_check_constraints", + } + + +class UniqueConstraintProduct(models.Model): + name = models.CharField(max_length=255) + color = models.CharField(max_length=32) + rank = models.IntegerField() + + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["name", "color"], name="name_color_uniq_validation" + ), + models.UniqueConstraint(fields=["rank"], name="rank_uniq_validation"), + ] + + +class ChildUniqueConstraintProduct(UniqueConstraintProduct): + pass + + +class UniqueConstraintConditionProduct(models.Model): + name = models.CharField(max_length=255) + color = models.CharField(max_length=31, null=True, blank=True) + + class Meta: + required_db_features = {"supports_partial_indexes"} + constraints = [ + models.UniqueConstraint( + fields=["name"], + name="name_without_color_uniq_validation", + condition=models.Q(color__isnull=True), + ), + ] diff --git a/tests/validation/test_constraints.py b/tests/validation/test_constraints.py new file mode 100644 index 0000000000..0b1ee6518e --- /dev/null +++ b/tests/validation/test_constraints.py @@ -0,0 +1,95 @@ +from django.core.exceptions import ValidationError +from django.test import TestCase, skipUnlessDBFeature + +from .models import ( + ChildProduct, + ChildUniqueConstraintProduct, + Product, + UniqueConstraintConditionProduct, + UniqueConstraintProduct, +) + + +class PerformConstraintChecksTest(TestCase): + @skipUnlessDBFeature("supports_table_check_constraints") + def test_full_clean_with_check_constraints(self): + product = Product(price=10, discounted_price=15) + with self.assertRaises(ValidationError) as cm: + product.full_clean() + self.assertEqual( + cm.exception.message_dict, + { + "__all__": [ + "Constraint “price_gt_discounted_price_validation” is violated." + ] + }, + ) + + @skipUnlessDBFeature("supports_table_check_constraints") + def test_full_clean_with_check_constraints_on_child_model(self): + product = ChildProduct(price=10, discounted_price=15) + with self.assertRaises(ValidationError) as cm: + product.full_clean() + self.assertEqual( + cm.exception.message_dict, + { + "__all__": [ + "Constraint “price_gt_discounted_price_validation” is violated." + ] + }, + ) + + @skipUnlessDBFeature("supports_table_check_constraints") + def test_full_clean_with_check_constraints_disabled(self): + product = Product(price=10, discounted_price=15) + product.full_clean(validate_constraints=False) + + def test_full_clean_with_unique_constraints(self): + UniqueConstraintProduct.objects.create(name="product", color="yellow", rank=1) + tests = [ + UniqueConstraintProduct(name="product", color="yellow", rank=1), + # Child model. + ChildUniqueConstraintProduct(name="product", color="yellow", rank=1), + ] + for product in tests: + with self.subTest(model=product.__class__.__name__): + with self.assertRaises(ValidationError) as cm: + product.full_clean() + self.assertEqual( + cm.exception.message_dict, + { + "__all__": [ + "Unique constraint product with this Name and Color " + "already exists." + ], + "rank": [ + "Unique constraint product with this Rank already exists." + ], + }, + ) + + def test_full_clean_with_unique_constraints_disabled(self): + UniqueConstraintProduct.objects.create(name="product", color="yellow", rank=1) + product = UniqueConstraintProduct(name="product", color="yellow", rank=1) + product.full_clean(validate_constraints=False) + + @skipUnlessDBFeature("supports_partial_indexes") + def test_full_clean_with_partial_unique_constraints(self): + UniqueConstraintConditionProduct.objects.create(name="product") + product = UniqueConstraintConditionProduct(name="product") + with self.assertRaises(ValidationError) as cm: + product.full_clean() + self.assertEqual( + cm.exception.message_dict, + { + "__all__": [ + "Constraint “name_without_color_uniq_validation” is violated." + ] + }, + ) + + @skipUnlessDBFeature("supports_partial_indexes") + def test_full_clean_with_partial_unique_constraints_disabled(self): + UniqueConstraintConditionProduct.objects.create(name="product") + product = UniqueConstraintConditionProduct(name="product") + product.full_clean(validate_constraints=False) diff --git a/tests/validation/test_unique.py b/tests/validation/test_unique.py index b8fd95abcb..4a8b3894f0 100644 --- a/tests/validation/test_unique.py +++ b/tests/validation/test_unique.py @@ -146,10 +146,6 @@ class PerformUniqueChecksTest(TestCase): mtv = ModelToValidate(number=10, name="Some Name") mtv.full_clean() - def test_func_unique_check_not_performed(self): - with self.assertNumQueries(0): - UniqueFuncConstraintModel(field="some name").full_clean() - def test_unique_for_date(self): Post.objects.create( title="Django 1.0 is released",