[Validator] Code property can't be populated to ConstraintViolation

[aeoris]

Following my (unanswered) question at StackOverflow I'm reporting this as a bug, as no one seems to know the answer.

Currently there's no way that I can see to populate the code property for the ConstraintViolation class, as it is not defined in the Constraint class. But as I see, it goes farther than that, as all the validators just ignore the parameters for the addViolation method, making it impossible for simple asserts to define a code for the violation.

What I expect it to allow is the following:

/**
 * @Assert\NotBlank(code=400)
 * @Assert\Email(code=401)
 */
protected $email;

Problem is, the current validators call addViolations without setting the 5th argument (code). Plus, the Constraint class doesn't implement such a code property as mentioned.

If this is indeed considered a bug, I would like to send a PR with a patch, provided that this is the way that it should be working.If it's not, some directions would be amazing!

[webmozart]

Why do you need this new property? Can you show me a problem that can't be solved with the currently existing functionality?

[stof]

@bschussek What is the goal of the code in the ConstraintViolation if it is never set ?

[aeoris]

@bschussek that's exactly the point, what @stof already mentioned. We now have an existing property that can't be populated via the usual ways (annotations et al).

I explained my particular needs in StackOverflow (the question is linked in the first message): I need to provide a constraint error code in addition to the violation message. It will allow an API to check which error was thrown comparing against an integer instead of the message - of course, I want both fields, not just one, as the message is nice to have and it may change for different users (i18n).

[webmozart]

@stof The point of the code property is to programmatically analyze where a violation comes from. Currently this is only used for "invalid" errors on forms.

One enhancement I see is to add a CODE constant to all constraint classes. Then you could test whether a violation originates from a specific constraint.

if (NotNull::CODE === $violation->getCode())

For constraints that have multiple error scenarios, multiple CODE_* constants could be added:

if (Length::CODE_MIN === $violation->getCode())

But for me it doesn't make sense to let the definition of the constraint set the code of the potential error. Imagine there is a generic module that relies on codes being set as in the examples I just gave. If you change the code in the constraint definition, this module will break.

@aeoris Would this solution also be acceptable for you?

[webmozart]

Obviously, all of these constants need to contain some sort of UUID.

[aeoris]

@bschussek Perfectly valid, indeed. As long as there's a way to compare against a constant value, that's fine for my use case.

I see your point on possible dependencies between modules, and I haven't thought about that.

[webmozart]

Ok. Before we implement this, let's work on the naming first. I think NotNull::CODE is semantically a bad name, because it's not really the "code of the NotNull constraint", but "the code of the error that results from failing NotNull validation".

Alternatives:

if (NotNull::CODE === $violation->getCode())
if (Length::CODE_MIN === $violation->getCode())

if (NotNull::ERROR === $violation->getCode())
if (Length::ERROR_MIN === $violation->getCode())

if (NotNull::ERR === $violation->getCode())
if (Length::ERR_MIN === $violation->getCode())

if (NotNull::VIOLATION === $violation->getCode())
if (Length::VIOLATION_MIN === $violation->getCode())

Which naming do you prefer?

[webmozart]

My preference would probably be

if (NotNull::ERROR === $violation->getCode())
if (Length::ERROR_MIN === $violation->getCode())

or even

if (NotNull::ERROR === $violation->getCode())
if (Length::MIN_ERROR === $violation->getCode())

[aeoris]

I would go with PHP's own (sort of) convention, although I don't know if it matches Symfony's:

if (NotNull::E_CODE === $violation->getCode())
if (Length::E_CODE_MIN === $violation->getCode())

Otherwise, I also like your first preference.

[webmozart]

@aeoris But to be really consistent with this convention it should probably be

if (NotNull::E_ERROR === $violation->getCode())
if (Length::E_MIN_ERROR === $violation->getCode())

[aeoris]

True. A possible drawback can be that NotNull::E_ERROR could be confused with a fatal error.

[webmozart]

Ok, let's go for that solution but without the E_ prefix then. Do you want to implement this?

[aeoris]

Sure! What about the possible values? Shall we go with UUID, 2^n, a regular incremental int?

[webmozart]

As mentioned before, every constraint that may result in an error should receive either one ERROR constant or multiple <case>_ERROR constants, where <case> refers to the different error conditions.

I suggest to use 128bit random values generated according to the Leach-Salz algorithm (RFC4122, as in Java) as values. For an online UUID generator with this algorithm, check http://www.famkruithof.net/uuid/uuidgen and set "type" to "Version 4: Random".

[webmozart]

#11657 was just merged. That means you can now access the constraint that caused a violation:

$violation->getConstraint()

The only thing that is missing now is to access the reason for constraints that can have multiple error paths (min, max, etc.).

[webmozart]

This was fixed in #12005. Additionally, #12021 adds error codes to all core validators with multiple error reasons.