#36809: Allow EmailValidator to customize error messages depending on the part
that
failed validation
-------------------------------------+-------------------------------------
Reporter: Daniel E Onetti | Owner: jaffar
Type: | Khan
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: EmailValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mike Edmunds):
Natalia, in attempting to review jaffar Khan's PR, I realized that while I
agree with these statements in theory, I'm having trouble putting them
into practice:
Replying to [comment:3 Natalia Bidart]:
> … Looking at the other validators in this module, there is a clear
pattern of separating parsing, normalization, and validation into distinct
steps or methods, even when the final error surface remains simple.
`EmailValidator` is somewhat inconsistent here: it already decomposes the
value into user and domain components, but does so inline inside
`__call__`, with no clear extension points. Adding a single extra param
addresses the symptom rather than the structure.
>
> I think a more Django aligned approach would be to re-evaluate
`EmailValidator` as a whole and give it clearer, overridable validation
hooks, while keeping full backward compatibility. For example, introducing
explicit methods like `validate_recipient()` and `validate_domain()`,
called from `__call__`, would allow customization via subclassing without
re-parsing the email or wrapping errors. …
I couldn't tell which validators you were referring to that separate
parsing, normalization and validation. EmailValidator follows the pattern
of the RegexValidators: it has some (overridable) regex and list
properties, and it does pretty much ''everything'' in its `__call__()`
method. Unlike the RegexValidators, it also has one extension point
method: `validate_domain_part()` allows replacing the domain validation
logic (but not `domain_allowlist` exceptions or ValidationError
construction). I ''believe'' the intent was to support stricter domain
validation via subclassing (e.g., for users who felt DomainNameValidator
was too lax or wanted to disallow numeric domain literals).
If a refactored EmailValidator looks something like:
{{{#!python
class EmailValidator:
def __call__(self, value):
... # pre-validation omitted
username, domain = value.rsplit("@", 1) # or
self.parse_parts(value) ?
self.validate_username(username, value)
self.validate_domain(domain, value)
def validate_domain(self, domain, value):
if domain not in self.domain_allowlist and not
some_other_logic_on(domain):
raise ValidationError(self.message, code=self.code,
params={"value": value})
}}}
… then the original ticket request to have access to the domain in the
error message ''does'' seem to require wrapping errors (or duplicating
logic from the superclass):
{{{#!python
class EmailValidatorWithBetterErrorMessages(EmailValidator):
def validate_domain(self, domain, value):
try:
super().validate_domain(domain, value)
except ValidationError as error:
# change "code" and add domain to the error params
raise ValidationError(self.message, code="invalid-domain",
params={"value": value, "domain": domain}) from error
}}}
… and substituting the domain validation logic requires duplicating some
of the superclass code:
{{{#!python
class EmailValidatorWithCustomDomainValidation(EmailValidator):
def validate_domain(self, domain, value):
if domain in self.domain_allowlist:
return # duplicate allowlist logic from superclass
if not idna.utils.is_valid_domain(domain):
raise ValidationError(self.message, code=self.code,
params={"value": value})
}}}
… which makes me think I've misunderstood what you were envisioning in
reworking EmailValidator to improve subclassing hooks.
--
Ticket URL: <https://code.djangoproject.com/ticket/36809#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019c15c32954-1884af25-7ba3-439e-9a6a-95ed4a2d0fb7-000000%40eu-central-1.amazonses.com.
