[Validator] Constraint validators now use the 2.5 API #11485
Conversation
| @@ -41,11 +41,10 @@ public function validate($value, Constraint $constraint) | |||
|
|
|||
| $context = $this->context; | |||
| $group = $context->getGroup(); | |||
| $validator = $context->getValidator()->inContext($context); | |||
There was a problem hiding this comment.
I know this is correct at the moment. But it looks kinda strange from an API point of view.
Would it not make more sense if $context->getValidator() automatically returns a ContextualValidatorInterface?
There was a problem hiding this comment.
No, because you don't always want to validate in the same context. For example, imagine validating some constraints and, depending on the result, adding one single combined violation. Then you need to validate in a different context.
This was in fact one of the motivations for the API change.
There was a problem hiding this comment.
See #9888 for example
…le validators, legacy validators were created
…rs for the different APIs
|
👍 |
|
I think the duplicated validation code for some new and legacy validators is not good. The validation code is the critical part. So it might make more sense to put that in an internal abstract class and then just change the implementation of adding a violation in the concrete class, i.e. |
|
@Tobion I was thinking about that, but then again I think this over-complicates the code (lots of new abstraction and classes). I think that the code - although duplicated - will be easy to maintain for two reasons:
For these reasons I would prefer to keep the duplicated code until we really need to combine it into a more complex abstraction. |
|
OK 👍 |
…ebmozart) This PR was squashed before being merged into the 2.5 branch (closes #11485). Discussion ---------- [Validator] Constraint validators now use the 2.5 API | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See the comments in #11049 for the origin of this PR. Currently, the 2.5 API needs to use `LegacyExecutionContextFactory` because the constraint validators rely on methods from the old `ExecutionContext` class (like `validate()`, `validateValue()`). Consequently it is impossible to switch to the pure 2.5 API and check whether all calls to deprecated methods were removed from application code. This is fixed now. This PR also introduces a complete test suite to test each constraint validator against all three APIs: 2.4, 2.5-BC and 2.5. Currently, some tests are not executed yet when running the complete test suite is run. I expect this to be fixed soon (ticket: sebastianbergmann/phpunit#529, pr: sebastianbergmann/phpunit#1327). Commits ------- 295e5bb [Validator] Fixed failing tests 3bd6d80 [Validator] CS fixes 870a41a [FrameworkBundle] Made ConstraintValidatorFactory aware of the legacy validators 7504448 [Validator] Added extensive test coverage for the constraint validators for the different APIs 8e461af [Validator] Constraint validators now use the 2.5 API. For incompatible validators, legacy validators were created
|
@webmozart tests are terribly broken: |
Both tests were introduced in symfony#11498, but symfony#11485 changed the approach to testing the validator and did not include these tests.
|
see #11562 |
See the comments in #11049 for the origin of this PR.
Currently, the 2.5 API needs to use
LegacyExecutionContextFactorybecause the constraint validators rely on methods from the oldExecutionContextclass (likevalidate(),validateValue()). Consequently it is impossible to switch to the pure 2.5 API and check whether all calls to deprecated methods were removed from application code. This is fixed now.This PR also introduces a complete test suite to test each constraint validator against all three APIs: 2.4, 2.5-BC and 2.5. Currently, some tests are not executed yet when running the complete test suite is run. I expect this to be fixed soon (ticket: sebastianbergmann/phpunit#529, pr: sebastianbergmann/phpunit#1327).