Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#34319 closed Bug (fixed)

Model.validate_constraints() crashes when constraint's validate() raises ValidationError without a code.

Reported by: Mateusz Kurowski Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 4.1
Severity: Release blocker Keywords: Model, validate_constraints, ValidationError, code, message
Cc: Gagaro Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mateusz Kurowski)

Imagine scenario when i want to explicitly mark a field that model constraint should raise ValidationError for:

class CustomUniqueConstraint(UniqueConstraint):

    def validate(self, *args, **kwargs):
        try:
            value = super().validate(*args, **kwargs)
        except ValidationError as e:
            raise ValidationError(
                {
                    'email': e,
                }
            )
        return value


class AbstractUser(django.contrib.auth.models.AbstractUser):

    class Meta:
        abstract = True
        constraints = [
            CustomUniqueConstraint(
                Lower("email"),
                name="%(app_label)s_%(class)s_email_unique",
            )
        ]

This wont work because:

1425, in validate_constraints
    if e.code == "unique" and len(constraint.fields) == 1:
       ^^^^^^
AttributeError: 'ValidationError' object has no attribute 'code'

Maybe all unique constraints should allow raising validation error for specific field like ?

from django.core.exceptions import ValidationError
from django.db import models


class ViolationFieldNameMixin:
    """
    Mixin for BaseConstraint subclasses that builds custom
    ValidationError message for the `violation_field_name`.
    By this way we can bind the error to the field that caused it.
    This is useful in ModelForms where we can display the error
    message next to the field and also avoid displaying unique
    constraint violation error messages more than once for  the same field.
    """

    def __init__(self, *args, **kwargs):
        self.violation_field_name = kwargs.pop("violation_field_name", None)
        self.violation_code = kwargs.pop("violation_code", None)
        super().__init__(*args, **kwargs)

    def validate(self, *args, **kwargs):
        try:
            value = super().validate(*args, **kwargs)
        except ValidationError as e:
            e.code = self.violation_code
            # Create a new ValidationError with the violation_field_name attribute as the key
            e = ValidationError({self.violation_field_name: e})
            # Set the error code to None
            # See https://code.djangoproject.com/ticket/34319#ticket
            e.code = self.violation_code
            raise e
        return value

    def deconstruct(self):
        path, args, kwargs = super().deconstruct()
        kwargs["violation_field_name"] = self.violation_field_name
        kwargs["violation_code"] = self.violation_code
        return path, args, kwargs

    def __eq__(self, other):
        return (
                super().__eq__(other)
                and self.violation_field_name == getattr(other, "violation_field_name", None)
                and self.violation_code == getattr(other, "violation_code", None)
        )


class UniqueConstraint(ViolationFieldNameMixin, models.UniqueConstraint):
    ...

Change History (10)

comment:2 by Mateusz Kurowski, 13 months ago

Description: modified (diff)
Summary: Model.validate_constraints check for ValidationError codeValidationError handling during model.validate_constraints
Type: BugCleanup/optimization

comment:3 by Mateusz Kurowski, 13 months ago

Description: modified (diff)

comment:4 by Mateusz Kurowski, 13 months ago

Description: modified (diff)

in reply to:  description comment:5 by Mariusz Felisiak, 13 months ago

Cc: Gagaro added
Severity: NormalRelease blocker
Summary: ValidationError handling during model.validate_constraintsModel.validate_constraints() crashes when constraint's validate() raises ValidationError without a code.
Triage Stage: UnreviewedAccepted
Type: Cleanup/optimizationBug

Replying to Mateusz Kurowski:

This wont work because:

1425, in validate_constraints
    if e.code == "unique" and len(constraint.fields) == 1:
       ^^^^^^
AttributeError: 'ValidationError' object has no attribute 'code'

This is a regression in 667105877e6723c6985399803a364848891513cc, that we should fix.

Maybe all unique constraints should allow raising validation error for specific field like ?

I don't think that adding extra arguments is a step forward here, we should be able to recognized violated fields by inspecting expression, see #34007.

comment:6 by Mariusz Felisiak, 13 months ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:7 by Mariusz Felisiak, 13 months ago

Has patch: set

comment:8 by Carlton Gibson, 13 months ago

Triage Stage: AcceptedReady for checkin

comment:9 by GitHub <noreply@…>, 13 months ago

Resolution: fixed
Status: assignedclosed

In 2fd755b3:

Fixed #34319 -- Fixed Model.validate_constraints() crash on ValidationError with no code.

Thanks Mateusz Kurowski for the report.

Regression in 667105877e6723c6985399803a364848891513cc.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

In 836ae73:

[4.2.x] Fixed #34319 -- Fixed Model.validate_constraints() crash on ValidationError with no code.

Thanks Mateusz Kurowski for the report.

Regression in 667105877e6723c6985399803a364848891513cc.
Backport of 2fd755b361d3da2cd0440fc9839feb2bb69b027b from main

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

In 590a92e4:

[4.1.x] Fixed #34319 -- Fixed Model.validate_constraints() crash on ValidationError with no code.

Thanks Mateusz Kurowski for the report.

Regression in 667105877e6723c6985399803a364848891513cc.
Backport of 2fd755b361d3da2cd0440fc9839feb2bb69b027b from main

Note: See TracTickets for help on using tickets.
Back to Top