Skip to content

[Validator] Made it possible (again) to pass a class name to validatePropertyValue() #11498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 4, 2014

Conversation

webmozart
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11139
License MIT
Doc PR -

In the 2.4 API it was possible to do both:

$validator->validatePropertyValue($object, 'propertyName', $myValue);
$validator->validatePropertyValue('\Vendor\Namespace\ClassName', 'propertyName', $myValue);

In the 2.5 API, the second case was not supported anymore. This is fixed now.

Together with the fix comes also a small change (also in the 2.4 API) which I'll demonstrate with a code snippet:

$metadata->addPropertyConstraint('ClassName', 'propertyName', new Callback(
    function ($value, $context) {
        var_dump($context->getRoot());
        var_dump($context->getPropertyPath());
    }
));

$validator->validatePropertyValue('ClassName', 'propertyName', 'foobar');

Before this PR, the output would be:

string(9) "ClassName"
string(12) "propertyName"

This doesn't make a lot of sense, because usually the following condition holds during validation:

'' === $context->getPropertyPath() || $value === $propertyAccessor->getValue($context->getRoot(), $context->getPropertyPath())

which obviously cannot work if root is a class name. Thus I changed the root and property path to become:

string(6) "foobar"
string(0) ""

With this change, the condition holds also in this case.

@fabpot
Copy link
Member

fabpot commented Aug 4, 2014

👍

@@ -254,10 +256,10 @@ public function validatePropertyValue($object, $propertyName, $value, $groups =
foreach ($propertyMetadatas as $propertyMetadata) {
$this->validateGenericNode(
$value,
$object,
$objectOrClass,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also means that some more methods accept object or class with changed phpdoc, like validateGenericNode, stepThroughGroupSequence, ExecutionContextInterface::setNode and ExecutionContextInterface::getObject

@Tobion
Copy link
Contributor

Tobion commented Aug 4, 2014

@webmozart
Copy link
Contributor Author

Thanks @Tobion. I changed RecursiveContextualValidator::validatePropertyValue() not to pass the class name to the private methods, this way we can leave the other methods unchanged.

@Tobion
Copy link
Contributor

Tobion commented Aug 4, 2014

👍

@webmozart webmozart merged commit 2bf1b37 into symfony:2.5 Aug 4, 2014
webmozart added a commit that referenced this pull request Aug 4, 2014
…to validatePropertyValue() (webmozart)

This PR was merged into the 2.5 branch.

Discussion
----------

[Validator] Made it possible (again) to pass a class name to validatePropertyValue()

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #11139
| License       | MIT
| Doc PR        | -

In the 2.4 API it was possible to do both:

```php
$validator->validatePropertyValue($object, 'propertyName', $myValue);
$validator->validatePropertyValue('\Vendor\Namespace\ClassName', 'propertyName', $myValue);
```

In the 2.5 API, the second case was not supported anymore. This is fixed now.

Together with the fix comes also a small change (also in the 2.4 API) which I'll demonstrate with a code snippet:

```php
$metadata->addPropertyConstraint('ClassName', 'propertyName', new Callback(
    function ($value, $context) {
        var_dump($context->getRoot());
        var_dump($context->getPropertyPath());
    }
));

$validator->validatePropertyValue('ClassName', 'propertyName', 'foobar');
```

Before this PR, the output would be:

```
string(9) "ClassName"
string(12) "propertyName"
```

This doesn't make a lot of sense, because usually the following condition holds during validation:

```php
'' === $context->getPropertyPath() || $value === $propertyAccessor->getValue($context->getRoot(), $context->getPropertyPath())
```

which obviously cannot work if root is a class name. Thus I changed the root and property path to become:

```
string(6) "foobar"
string(0) ""
```

With this change, the condition holds also in this case.

Commits
-------

2bf1b37 [Validator] Fixed ExpressionValidator when the validation root is not an object
ef6f5f5 [Validator] Fixed: Made it possible (again) to pass a class name to Validator::validatePropertyValue()
@webmozart webmozart deleted the issue11139 branch August 4, 2014 14:42
jakzal added a commit to jakzal/symfony that referenced this pull request Aug 4, 2014
Both tests were introduced in symfony#11498, but symfony#11485 changed the approach to testing the validator and did not include these tests.
@jakzal jakzal mentioned this pull request Aug 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants