8000 LegacyValidator doesn't provide B/C for validatePropertyValue · Issue #11139 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

LegacyValidator doesn't provide B/C for validatePropertyValue #11139

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.

Sign up for GitHub

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

Jump to bottom
Closed
oleg-andreyev opened this issue Jun 17, 2014 · 3 comments
Closed

LegacyValidator doesn't provide B/C for validatePropertyValue #11139

oleg-andreyev opened this issue Jun 17, 2014 · 3 comments
Labels
Bug Validator

Comments

@oleg-andreyev
Copy link
Contributor

In 2.4 we can call validator on property this way:

$errors = $this->validator->validatePropertyValue(
    'My/CustomBundle/Entity/Test',
    'customProperty ',
    $value
);

but in 2.5 validatePropertyValue generates

PHP Warning:  spl_object_hash() expects parameter 1 to be object, string given in /vendor/symfony/symfony/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php on line 214

caused by $cacheKey = spl_object_hash($object);
@stof stof added Bug labels Jun 17, 2014
@stof
Copy link
Member
stof commented Jun 17, 2014

hmm, 2.4 indeed seem to accept both an object instance or a class name as first argument of validatePropertyValue but the BC layer only takes the first case into account.

oleg-andreyev pushed a commit to oleg-andreyev/symfony that referenced this issue Jul 25, 2014
symfony#11139 fixing B/C break: replaced spl_object_hash($object) wit…
b8e 8000 6c98 
…h spl_object_hash($classMetadata)
oleg-andreyev pushed a commit to oleg-andreyev/symfony that referenced this issue Jul 25, 2014
symfony#11139 fixing B/C break: replaced spl_object_hash($object) wit…
3cee6fc 
…h spl_object_hash($classMetadata)
@oleg-andreyev oleg-andreyev mentioned this issue Jul 25, 2014
Closed
@jakzal jakzal added the hasPR label Jul 26, 2014
@webmozart
Copy link
Contributor

I suppose customProperty is a static property?

@oleg-andreyev
Copy link
Contributor Author

@webmozart, no. It can be a regular property

@webmozart webmozart mentioned this issue Jul 28, 2014
Merged
webmozart added a commit that referenced this issue Aug 4, 2014
@webmozart
bug #11498 [Validator] Made it possible (again) to pass a class name …
c43253e 
…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()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Validator
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@webmozart @jakzal @stof @oleg-andreyev
0