[Validator] Allow to use a property path to get value to compare in comparison constraints #22576

ogizanagi · 2017-04-28T14:57:58Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	N/A
License	MIT
Doc PR	todo

So we can simply declare something like:

class Activity 
{
    /**
     * @var \DateTime
     *
     * @Assert\DateTime()
     */
    private $startDate;

    /**
     * @var \DateTime
     *
     * @Assert\DateTime()
     * @Assert\GreaterThan(propertyPath="startDate")
     */
    private $endDate;

    // [...]

    public function getStartDate(): \DateTime
    {
        return $this->startDate;
    }

    public function getEndDate(): \DateTime
    {
        return $this->startDate;
    }
}

Of course, this is actually already possible by using an Expression constraint (or a callable), but it feels more natural to me to use proper comparison constraints for this.

ro0NL · 2017-04-28T15:02:41Z

propertyPath?

edit; perhaps for sake of the explicitness valuePropertyPath :)

ogizanagi · 2017-04-28T15:08:24Z

@ro0NL: I hesitated. It should be obvious it's the property path to the value to use in comparison, and not anything else. I can't really explain why I had some reservations on this naming. Let's see what people do prefer.

ogizanagi · 2017-06-25T16:35:48Z

Updated to use back propertyPath for the new option name, as it's the common naming we've got in the core for this (original proposition was valuePath). Unless there is a debate about another naming, this should be ready.

xabbuh · 2017-06-29T07:59:08Z

Can you add a new entry to the changelog please?

xabbuh · 2017-06-29T08:01:06Z

src/Symfony/Component/Validator/Constraints/AbstractComparison.php


    /**
     * {@inheritdoc}
     */
    public function __construct($options = null)
    {
-        if (is_array($options) && !isset($options['value'])) {
+        if (is_array($options) && !isset($options['value']) && !isset($options['propertyPath'])) {


We should also error if both the value and the propertyPath option are given.

xabbuh · 2017-06-29T08:02:47Z

src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php

@@ -36,7 +47,16 @@ public function validate($value, Constraint $constraint)
            return;
        }

-        $comparedValue = $constraint->value;
+        if ($path = $constraint->propertyPath) {
+            $object = $this->context->getObject();


we need to account for the case where $object is null

Hmm. Indeed. What should we do in such case? I guess we can only return; as there is no way to get the value to compare to from null and should not create a violation (it's the NotNull constraint role). Right?

xabbuh · 2017-06-29T08:04:17Z

src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php

+    private function getPropertyAccessor()
+    {
+        if (null === $this->propertyAccessor) {
+            if (!class_exists(PropertyAccess::class)) {


What about failing earlier instead when the constraint definition is evaluated (i.e. in the constructor of AbstractComparison)?

xabbuh · 2017-06-29T08:16:00Z

src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php

@@ -23,6 +27,13 @@
 */
 abstract class AbstractComparisonValidator extends ConstraintValidator
 {
+    private $propertyAccessor;
+
+    public function __construct(PropertyAccess $propertyAccessor = null)


PropertyAccessor

Oops. Good catch.

ogizanagi · 2017-07-03T18:01:21Z

Rebased and green.

fabpot · 2017-07-04T10:38:23Z

src/Symfony/Component/Validator/Constraints/AbstractComparison.php

+            }
+
+            if (isset($options['propertyPath']) && !class_exists(PropertyAccess::class)) {
+                throw new ConstraintDefinitionException(sprintf('The %s constraint requires the Symfony PropertyAccess component to use the "propertyPath" option.', get_class($this)));


Missing " around the class name (same above)

No double quotes were used in the existing one. (Fixed though)

fabpot · 2017-07-04T10:39:55Z

src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php

@@ -54,6 +54,14 @@ public function provideValidComparisons()
    /**
     * {@inheritdoc}
     */
+    public function provideValidComparisonsToPropertyPath()
+    {
+        yield array(5);


Why not use return array(5);. That would be more similar to what we already have and would avoid questions about why the new ones are different.

…omparison constraints

nicolas-grekas · 2017-07-12T12:11:44Z

Thank you @ogizanagi.

@var

…to compare in comparison constraints (ogizanagi) This PR was merged into the 3.4 branch. Discussion ---------- [Validator] Allow to use a property path to get value to compare in comparison constraints | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | todo So we can simply declare something like: ```php class Activity { /** * @var \DateTime * * @Assert\DateTime() */ private $startDate; /** * @var \DateTime * * @Assert\DateTime() * @Assert\GreaterThan(propertyPath="startDate") */ private $endDate; // [...] public function getStartDate(): \DateTime { return $this->startDate; } public function getEndDate(): \DateTime { return $this->startDate; } } ``` Of course, this is actually already possible by using an `Expression` constraint (or a callable), but it feels more natural to me to use proper comparison constraints for this. Commits ------- 07c5aa6 [Validator] Allow to use a property path to get value to compare in comparison constraints

…in range constraint (Lctrs) This PR was squashed before being merged into the 4.4 branch (closes #31511). Discussion ---------- [Validator] Allow to use property paths to get limits in range constraint | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | Part of #31503 | License | MIT | Doc PR | symfony/symfony-docs#11793 Similar as #22576, but for the `Range` constraint. Commits ------- 2b50990 [Validator] Allow to use property paths to get limits in range constraint

carsonbot added Status: Needs Review Validator Feature labels Apr 28, 2017

nicolas-grekas added this to the 3.4 milestone Apr 28, 2017

ogizanagi changed the base branch from master to 3.4 May 17, 2017 18:31

xabbuh reviewed Jun 29, 2017

View reviewed changes

xabbuh approved these changes Jul 4, 2017

View reviewed changes

fabpot requested changes Jul 4, 2017

View reviewed changes

fabpot approved these changes Jul 4, 2017

View reviewed changes

[Validator] Allow to use a property path to get value to compare in c…

07c5aa6

…omparison constraints

nicolas-grekas merged commit 07c5aa6 into symfony:3.4 Jul 12, 2017

ogizanagi deleted the feature/validator/comparisons_property_path branch July 12, 2017 12:12

xabbuh mentioned this pull request Jul 12, 2017

[Validator] propertyPath in comparison constraints symfony/symfony-docs#8150

Closed

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

Lctrs mentioned this pull request May 16, 2019

[Validator] Allow to use property paths to get limits in range constraint #31511

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] Allow to use a property path to get value to compare in comparison constraints #22576

[Validator] Allow to use a property path to get value to compare in comparison constraints #22576

ogizanagi commented Apr 28, 2017 •

edited

Loading

ro0NL commented Apr 28, 2017 •

edited

Loading

ogizanagi commented Apr 28, 2017

ogizanagi commented Jun 25, 2017 •

edited

Loading

xabbuh commented Jun 29, 2017

xabbuh Jun 29, 2017

ogizanagi Jun 29, 2017

xabbuh Jun 29, 2017

ogizanagi Jun 29, 2017

xabbuh Jun 29, 2017

ogizanagi Jun 29, 2017

xabbuh Jun 29, 2017

ogizanagi Jun 29, 2017

ogizanagi commented Jul 3, 2017

fabpot Jul 4, 2017

ogizanagi Jul 4, 2017

fabpot Jul 4, 2017

ogizanagi Jul 4, 2017

nicolas-grekas commented Jul 12, 2017

[Validator] Allow to use a property path to get value to compare in comparison constraints #22576

[Validator] Allow to use a property path to get value to compare in comparison constraints #22576

Conversation

ogizanagi commented Apr 28, 2017 • edited Loading

ro0NL commented Apr 28, 2017 • edited Loading

ogizanagi commented Apr 28, 2017

ogizanagi commented Jun 25, 2017 • edited Loading

xabbuh commented Jun 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi commented Jul 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Jul 12, 2017

ogizanagi commented Apr 28, 2017 •

edited

Loading

ro0NL commented Apr 28, 2017 •

edited

Loading

ogizanagi commented Jun 25, 2017 •

edited

Loading