Skip to content

[Validator] New NodeTraverser implementation #10287

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 86 commits into from
Mar 31, 2014
Merged

Conversation

webmozart
Copy link
Contributor

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

This PR is ready for review.

Todo

  • Test extensively to avoid regressions
  • Test more extensively
  • Finish inline documentation
  • Provide a layer to choose the desired API through ValidatorBuilder
  • Provide a layer to choose the desired API through FrameworkBundle
  • Update UPGRADE file
  • Update CHANGELOG
  • Update user documentation

Goal

The goal of this PR is to be able to fix the following tickets:

The following tickets are probably fixed, but require testing first:

Of course, full backwards compatibility must be guaranteed.

Other tickets I want to fix in follow-up PRs:

In a nutshell

The implementation removes the Visitor pattern, which was implemented badly. I tried fixing it via a NodeTraverser/NodeVisitor implementation, but performance degraded too much so I decided to remove the pattern altogether.

A couple of new features and bug fixes are possible thanks to the new implementation. See below for details.

PHP Versions

PHP 5.3.8 and older does not allow to implement two different interfaces which both contain a method with the same name. This is used in the compatibility layer that supports both the old and the new API.

For this reason, the compatibility layer is disabled on PHP < 5.3.9. Older PHP versions need to decide on the old API or the new API (without compatibility layer).

Choosing the API Version

The API version can be specified by one of Validation::API_VERSION_2_4, Validation::API_VERSION_2_5 and Validation::API_VERSION_2_5_BC to setApiVersion() when building the validator:

// Old implementation
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_4)
    ->getValidator();

// New implementation without BC API
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_5)
    ->getValidator();

// New implementation with BC API
// Does not work on PHP < 5.3.9
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_5_BC)
    ->getValidator();

Features

Constraint validation as first-class citizen

The new API merges validateValue() and validate(). The idea is that the validation of values against constraints should be as simple as possible. Object validation is a special case where an object is tested against the Valid constraint. A backwards compatibility layer is provided to use both validate() and validateValue() with the old signature.

// Validate against explicit constraints
$violations = $validator->validate($firstName, array(
    new NotNull(),
    new Length(array('min' => 3)),
));

// Validate against metadata
$violations = $validator->validate($object);

// Same, more expressive notation
$violations = $validator->validate($object, new Valid());

// Validate each entry against its metadata
$violations = $validator->validate($array);
Aggregate violations

It is now possible to call the methods of the validator multiple times and aggregate the violations to a common violation list. To do so, call startContext(), execute the calls and call getViolations() in the end to retrieve the violations:

$violations = $validator->startContext()
    ->validate($title, new NotNull())
    ->validate($text, new NotNull())
    ->validate($author->getName(), new NotNull())
    ->getViolations()

Most of the time, you will want to specify a property path for each validation. Use the method atPath() for that:

$violations = $validator->startContext()
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
    ->atPath('author.name')->validate($author->getName(), new NotNull())
    ->getViolations()
Control the context of nested validations

In Symfony <= 2.4, you can validate objects or constraints from within a constraint validator like this:

$this->context->validate($object);
$this->context->validateValue($value, new Length(array('min' => 3)));

The validations will run and all violations will be added to the current context.

This way, it is impossible though to validate something, inspect the result and then decide what kind of violations to add to the context. This is needed, for example, for the Some constraint (proposed in #9888), which should succeed if any of the validated values did not generate violations.

For this reason, the new context API features a method getValidator(). This method returns the validator instance, you can use it to validate anything in a new context (as the validator always does):

$validator = $this->context->getValidator();
$violations = $validator->validate($object);

if (count($violations)  > 0) {
    $this->context->addViolation('The validation did not pass');
}

You can also explicitly start a new context:

$validator = $this->context->getValidator();
$violations = $validator->startContext()
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
    ->getViolations()

If you want to execute the validation in the current context, use the inContext() method of the validator instead:

// violations are added to $this->context
$validator->inContext($this->context)
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
;

With this feature, #9888 (especially the PR for it: #9988) can be finished.

Custom group sequences (#7146)

It is now possible to pass GroupSequence instances whenever you can pass a group to the validator. For example:

$violations = $validator->validate($object, new Valid(), new GroupSequence('Basic', 'Strict'));

Or in the context of the Form component:

$form = $this->createForm(new BlogType(), new Blog(), array(
    'validation_groups' => new GroupSequence('Basic', 'Strict'),
));
Constraint violation builders (#6138)

The API for adding constraint violations was simplified:

$this->context->addViolation('message', array('param' => 'value'));

// or

$this->context->buildViolation('message')
    ->atPath('property')
    ->setParameter('param', 'value')
    ->setTranslationDomain('validation_strict')
    ->addViolation();
Control traversal at class level (#8617)

Currently, it is possible whether to traverse a Traversable object or not in the Valid constraint:

/**
 * @Assert\Valid(traverse=true)
 */
private $tags = new TagList();

(actually, true is the default)

In this way, the validator will iterate the TagList instance and validate each of the contained objects. You can also set "traverse" to false to disable iteration.

What if you want to specify, that TagList instances should always (or never) be traversed? That's currently not possible.

With this PR, you can do the following:

/**
 * @Assert\Traverse(false)
 */
class TagList implements \IteratorAggregate
{
    // ...
}

Follow-up features

Features of the follow-up PRs will be described directly there.

Backwards compatibility

I implemented a new AbstractValidatorTest which tests both the old and the new implementation for compatibility. I still want to extend this test to make sure we don't introduce any regressions.

Almost none of the existing classes were modified (or only slightly). If users depend on the current (now "legacy") implementation, they will have the choice to continue using it until 3.0 (of course, without the new features).

Your task

Congrats, you made it till here :) If you have time, please skim over the code and give me feedback on the overall implementation and the class/method names. Again, no feedback on details yet, there are quite a few areas in the code that are still work in progress.

Thanks,
Bernhard

[1] That means that only the nodes from the root of the graph until the currently validated node are held in memory.


public function offsetGet($offset)
{
return $this->groups[$offset];
Copy link
Member

Choose a reason for hiding this comment

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

you need to take care of error handling here to avoid a notice on invalid access

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are quite a few areas in the code that are still work in progress

:)

@lyrixx
Copy link
Member

lyrixx commented Feb 18, 2014

interface NodeTraverserInterface
{
    public function addVisitor(NodeVisitorInterface $visitor);
    public function removeVisitor(NodeVisitorInterface $visitor);
    public function traverse(array $nodes);
}

interface NodeVisitorInterface
{
    public function beforeTraversal(array $nodes);
    public function afterTraversal(array $nodes);
    public function enterNode(Node $node);
    public function leaveNode(Node $node);
}

You type hinted some methods with array. It's preferable to not type hint to allow a refactoring to give a Traversable objectinstead. I'm not why someone would like to change it, but...

I know @fabpot avoid type hinting array as much as possible.

@yahuarkuntur
Copy link
Contributor

CS = Computer Science? because this looks amazing! 😄

* }
*
* Whenever you validate that object in the "Default" group, the group sequence
* will be validated:
Copy link
Member

Choose a reason for hiding this comment

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

This point was, and stays a bit hard to understand.

  • What constraints are in the Default group?
  • What constraints are in the Address group?

I want to highlight a naming issue here. In the past, I was a bit lost between Default and ClassName group. Acutally, I'm not sure I totally understand it ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, this is one of the confusing parts of JSR-303. Suppose you have constraints in the "Default" group:

class Address
{
    /**
     * @Length(min=3, groups="Default")
     */
    private $street;
}

Then these two calls will be 100% equivalent:

$violations = $validator->validate($address, "Default");
$violations = $validator->validate($address, "Address");

We say that "Address" is the implicit group name of those constraints, i.e. a group name that is equivalent to "Default".

Let's override the default group with a sequence:

/**
 * @GroupSequence({"Address", "Strict"})
 */
class Address
{
    // ... rest stays the same ...
}

Now the calls are not equivalent anymore:

// validates the group sequence
$violations = $validator->validate($address, "Default");

// validates the constraints in "Default"
$violations = $validator->validate($address, "Address");

That is also why you need to refer to the "Default" group as "" in the sequence. The following would create a recursion and is invalid (remember that "Default" === the group sequence now):

/**
 * @GroupSequence({"Default", "Strict"})
 */
class Address
{
    // ... rest stays the same ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw, if you think you can improve the documentation about this functionality, please with sugar on top do so :)

Copy link
Member

Choose a reason for hiding this comment

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

Ok understood. Thanks. I think we can copy/paste your explanation for the doc. ping @wouterj

Copy link
Member

Choose a reason for hiding this comment

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

@webmozart
Copy link
Contributor Author

CS = Computer Science?

Coding Style

$traversalStrategy
);
} elseif ($cascadingStrategy & CascadingStrategy::CASCADE) {
$this->cascadeObject(
Copy link
Member

Choose a reason for hiding this comment

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

what if the value is a scalar ? Currently, it would go through cascadeObject which looks weird to me. Is it a mistake in the condition or in the naming of the method ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's BC behavior. Currently, the cascading of scalars leads to NoSuchMetadataExceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment that explains this.

@webmozart
Copy link
Contributor Author

The PR is pretty far now. #6138, #7146, #7432 and #8617 are fixed. The Validator component will come with three different implementations:

  • legacy: the implementation we had before 2.5 (untouched)
  • legacy compatible: the new implementation with a compatibility layer for <2.5 (will be the default)
  • 2.5+: the new implementation, without the compatibility layer

I stumbled upon a problem though: PHP < 5.3.9 does not like classes that implement two interfaces which each contain the same method. That's a problem, because the legacy compatibility layer implements both the old and the new interfaces, for obvious reasons.

I don't know how to deal with this yet.

@stof
Copy link
Member

stof commented Feb 19, 2014

@webmozart is it only PHP < 5.3.9 ? I thought newer implementation are forbidding it as well (5.3.9 fixed the redeclaration of an abstract method in a child, but I don't think it fixed something related to 2 separate interfaces)

And your tests has a fatal error on PHP 5.3

@docteurklein
Copy link
Contributor

@stof @webmozart it works between 5.3.9 and current:

http://3v4l.org/D4c1I

@webmozart
Copy link
Contributor Author

@stof Yes, it works with 5.3.9+.

Btw. I added new feature descriptions above:

  • "Constraint validation as first-class citizen"
  • "Aggregate violations"

I also updated the description of "Control the context of nested validations".

Validator
---------

* `ClassMetadata::getGroupSequence()` now returns `GroupSequence` instances
Copy link
Contributor

Choose a reason for hiding this comment

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

"now returns a GroupSequence instance" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please clarify your question?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it return a single one or multiple? The first line makes me think it returns a list of GroupSequence's but the following line talks as it returns a single instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, got it now. It returns one only, will fix :)

@mvrhov
Copy link

mvrhov commented Feb 20, 2014

Does the 5.3.3 really present huge problem?
IMO The users with that old version are usually huge corporations and those should really use the 2.3 LTS. And By the time next LTS comes around most of the distributions will be on 5.4.x (Yeah go and complain to Redhat they publicly said that their next version will use 5.4 and not 5.5, by the time they'll release the redHat 7 the 5.4 will be unsupported)

@webmozart
Copy link
Contributor Author

I found a way to fix the tests for PHP < 5.3.9 now. The new implementation cannot be used on those PHP versions. The old implementation, however, continues to work fine in that case (without the new features, obviously).

@webmozart
Copy link
Contributor Author

I also enabled the new API on PHP < 5.3.9 now. The compatibility layer, that supports both the old and the new API at the same time, continues to be broken under PHP < 5.3.9 though.

@webmozart
Copy link
Contributor Author

Except for the open todos mentioned in the description, this PR is complete now. Feedback welcome. :)

@stof
Copy link
Member

stof commented Feb 21, 2014

@webmozart I suggest you remove the unused use statement reported by fabbot (it provides a patch) so that the PR is green

* instance will be returned.
*
* You can optionally pass a {@link LoaderInterface} instance to the constructor.
* Whenever a new metadata instance, it will be passed to the loader, which can
Copy link
Member

Choose a reason for hiding this comment

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

missing verb in Whenever a new metadata instance

weaverryan added a commit to symfony/symfony-docs that referenced this pull request Mar 8, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

Clarified Default and ClassName groups

This is still the most difficult part of the validation groups and we
don't put much attention on it. I've refactored it a bit after the
really great explaination of @webmozart.

For more information see: symfony/symfony#10287 (comment)

| Q   | A
| --- | ---
| Doc fix? | yes
| New docs? | yes
| Applies to | all
| Fixed tickets | -

Commits
-------

b6357fa Updated after review of @webmozart
2330cd8 Clarified Default vs ClassName groups
@webmozart
Copy link
Contributor Author

I rebased onto master now, optimized some code parts and did performance comparisons. I used the following simple benchmark: https://gist.github.com/webmozart/9486236

With the benchmark, I get the following results:

API Version Duration (ms) Relative Duration (%)
2.4 ~7.8ms -
2.5 (recursive implementation) ~11.8ms +51%
2.5 (traverser implementation) ~14.7ms +88%

The recursive 2.5 implementation is very similar to the 2.4 implementation, except that the features and bug fixes mentioned above are supported. I'm still trying to improve this to reduce the performance loss.

The traverser 2.5 implementation is identical to the recursive one, only that it uses a NodeTraverser/NodeVisitor architecture to be more extensible. The question is whether we really need this flexibility and whether it warrants the performance loss compared to the recursive implementation.

Any opinions?

}

/**
* Alias of {@link getMessageParameters()}.
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reason for an alias? should the old one not get deprecated if the new one is better named?

Copy link
Contributor

Choose a reason for hiding this comment

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

@webmozart ^^ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Tobion Yes, but we can't add the new version to ConstraintViolationInterface because that is marked as API. I don't want to simply deprecate a method without offering an alternative either.

The long versions are going to be removed in Symfony 3.0 when we can change the interface (and move it to the Violation sub-namespace, for that sake).

@webmozart
Copy link
Contributor Author

Apart from updating the manual, this PR should be ready to merge now.

@fabpot fabpot changed the title [WIP][Validator] New NodeTraverser implementation [Validator] New NodeTraverser implementation Mar 31, 2014
fabpot added a commit that referenced this pull request Mar 31, 2014
…mozart)

This PR was merged into the 2.5-dev branch.

Discussion
----------

[WIP][Validator] New NodeTraverser implementation

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

This PR is ready for review.

#### Todo

- [x] Test extensively to avoid regressions
- [x] Test more extensively
- [x] Finish inline documentation
- [x] Provide a layer to choose the desired API through ValidatorBuilder
- [x] Provide a layer to choose the desired API through FrameworkBundle
- [x] Update UPGRADE file
- [x] Update CHANGELOG
- [ ] Update user documentation

#### Goal

The goal of this PR is to be able to fix the following tickets:

- [x] #6138 Simplify adding of constraint violations
- [x] #7146 Support group sequences in Validator API
- [x] #7432 Poorly implemented Visitor Pattern
- [x] #8617 Control traversal on class level
- [x] #9888 Improve support for collection validation (PR: #9988)

The following tickets are probably fixed, but require testing first:

- [ ] #8376 Using validation_group causes error message to display multiple times
- [ ] #9939 GroupSequences still execute next group if first fail

Of course, full backwards compatibility **must** be guaranteed.

Other tickets I want to fix in follow-up PRs:

* #3622 Constraints\Valid does not respect "groups" option
* #4453 walk constraints by groups
* #7700 Propagate implicit group names in constraints
* #9051 Always ask value event if field isn't in the validating group
* #10163 poor collection validation test coverage
* #10221 TypeValidator does not enforce desired type when value is NULL
* #10495 Class Valid Constraint can't be used on a Form Type

#### In a nutshell

The implementation removes the Visitor pattern, which was implemented badly. I tried fixing it via a NodeTraverser/NodeVisitor implementation, but performance degraded too much so I decided to remove the pattern altogether.

A couple of new features and bug fixes are possible thanks to the new implementation. See below for details.

#### PHP Versions

PHP 5.3.8 and older does not allow to implement two different interfaces which both contain a method with the same name. This is used in the compatibility layer that supports both the old and the new API.

For this reason, the compatibility layer is disabled on PHP < 5.3.9. Older PHP versions need to decide on the old API or the new API (without compatibility layer).

#### Choosing the API Version

The API version can be specified by one of `Validation::API_VERSION_2_4`, `Validation::API_VERSION_2_5` and `Validation::API_VERSION_2_5_BC` to `setApiVersion()` when building the validator:

```php
// Old implementation
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_4)
    ->getValidator();

// New implementation with BC API
// Does not work on PHP < 5.3.9
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_5)
    ->getValidator();

// New implementation without BC API
$validator = Validation::createValidatorBuilder()
    ->setApiVersion(Validation::API_VERSION_2_5_BC)
    ->getValidator();
```

#### Features

##### Constraint validation as first-class citizen

The new API merges `validateValue()` and `validate()`. The idea is that the validation of values against constraints should be as simple as possible. Object validation is a special case where an object is tested against the `Valid` constraint. A backwards compatibility layer is provided to use both `validate()` and `validateValue()` with the old signature.

```php
// Validate against explicit constraints
$violations = $validator->validate($firstName, array(
    new NotNull(),
    new Length(array('min' => 3)),
));

// Validate against metadata
$violations = $validator->validate($object);

// Same, more expressive notation
$violations = $validator->validate($object, new Valid());

// Validate each entry against its metadata
$violations = $validator->validate($array);
```

##### Aggregate violations

It is now possible to call the methods of the validator multiple times and aggregate the violations to a common violation list. To do so, call `startContext()`, execute the calls and call `getViolations()` in the end to retrieve the violations:

```php
$violations = $validator->startContext()
    ->validate($title, new NotNull())
    ->validate($text, new NotNull())
    ->validate($author->getName(), new NotNull())
    ->getViolations()
```

Most of the time, you will want to specify a property path for each validation. Use the method `atPath()` for that:

```php
$violations = $validator->startContext()
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
    ->atPath('author.name')->validate($author->getName(), new NotNull())
    ->getViolations()
```

##### Control the context of nested validations

In Symfony <= 2.4, you can validate objects or constraints from within a constraint validator like this:

```php
$this->context->validate($object);
$this->context->validateValue($value, new Length(array('min' => 3)));
```

The validations will run and all violations will be added to the current context.

This way, it is impossible though to validate something, inspect the result and then decide what kind of violations to add to the context. This is needed, for example, for the `Some` constraint (proposed in #9888), which should succeed if any of the validated values did *not* generate violations.

For this reason, the new context API features a method `getValidator()`. This method returns the validator instance, you can use it to validate anything in a new context (as the validator always does):

```php
$validator = $this->context->getValidator();
$violations = $validator->validate($object);

if (count($violations)  > 0) {
    $this->context->addViolation('The validation did not pass');
}
```

You can also explicitly start a new context:

```php
$validator = $this->context->getValidator();
$violations = $validator->startContext()
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
    ->getViolations()
```

If you want to execute the validation in the current context, use the `inContext()` method of the validator instead:

```php
// violations are added to $this->context
$validator->inContext($this->context)
    ->atPath('title')->validate($title, new NotNull())
    ->atPath('text')->validate($text, new NotNull())
;
```

With this feature, #9888 (especially the PR for it: #9988) can be finished.

##### Custom group sequences (#7146)

It is now possible to pass `GroupSequence` instances whenever you can pass a group to the validator. For example:

```php
$violations = $validator->validate($object, new Valid(), new GroupSequence('Basic', 'Strict'));
```

Or in the context of the Form component:

```php
$form = $this->createForm(new BlogType(), new Blog(), array(
    'validation_groups' => new GroupSequence('Basic', 'Strict'),
));
```

##### Constraint violation builders (#6138)

The API for adding constraint violations was simplified:

```php
$this->context->addViolation('message', array('param' => 'value'));

// or

$this->context->buildViolation('message')
    ->atPath('property')
    ->setParameter('param', 'value')
    ->setTranslationDomain('validation_strict')
    ->addViolation();
```

##### Control traversal at class level (#8617)

Currently, it is possible whether to traverse a `Traversable` object or not in the `Valid` constraint:

```php
/**
 * @Assert\Valid(traverse=true)
 */
private $tags = new TagList();
```

(actually, `true` is the default)

In this way, the validator will iterate the `TagList` instance and validate each of the contained objects. You can also set "traverse" to `false` to disable iteration.

What if you want to specify, that `TagList` instances should always (or never) be traversed? That's currently not possible.

With this PR, you can do the following:

```php
/**
 * @Assert\Traverse(false)
 */
class TagList implements \IteratorAggregate
{
    // ...
}
```

#### Follow-up features

Features of the follow-up PRs will be described directly there.

#### Backwards compatibility

I implemented a new `AbstractValidatorTest` which tests both the old and the new implementation for compatibility. I still want to extend this test to make sure we don't introduce any regressions.

Almost none of the existing classes were modified (or only slightly). If users depend on the current (now "legacy") implementation, they will have the choice to continue using it until 3.0 (of course, without the new features).

#### Your task

Congrats, you made it till here :) If you have time, please skim over the code and give me feedback on the overall implementation and the class/method names. Again, no feedback on details yet, there are quite a few areas in the code that are still work in progress.

Thanks,
Bernhard

[1] That means that only the nodes from the root of the graph until the currently validated node are held in memory.

Commits
-------

ca6a722 [Validator] Converted `@deprecate` doc comment into regular doc comment
68d8018 [Validator] Documented changes in the UPGRADE files
b1badea [Validator] Fixed failing CsrfFormLoginTest
0bfde4a [Validator] Fixed misnamed method calls in FrameworkExtension
3dc2b4d [Validator] Made "symfony/property-access" an optional dependency
c5629bb [Validator] Added getObject() to ExecutionContextInterface
9b204c9 [FrameworkBundle] Implemented configuration to select the desired Validator API
0946dbe [Validator] Adapted CHANGELOG
1b111d0 [Validator] Fixed typos pointed out by @cordoval
7bc952d [Validator] Improved inline documentation of RecursiveContextualValidator
166d71a [Validator] Removed unused property
90c27bb [Validator] Removed traverser implementation
3183aed [Validator] Improved performance of cache key generation
029a716 [Validator] Moved logic of replaceDefaultGroup() to validateNode()
2f23d97 [Validator] Reduced number of method calls on the execution context
73c9cc5 [Validator] Optimized performance by calling spl_object_hash() only once per object
94ef21e [Validator] Optimized use statements
1622eb3 [Validator] Fixed reference to removed class in ValidatorBuilder
be508e0 [Validator] Merged DefaultGroupReplacingVisitor and ContextUpdateVisitor into NodeValidationVisitor
50bb84d [Validator] Optimized RecursiveContextualValidator
eed29d8 [Validator] Improved performance of *ContextualValidator::validate()
5c479d8 [Validator] Simplified validateNodeForGroup
eeed509 [Validator] Improved phpdoc of RecursiveValidator
274d4e6 [Validator] Changed ValidatorBuilder to always use LegacyExecutionContext
38e26fb [Validator] Decoupled RecursiveContextualValidator from Node
23534ca [Validator] Added a recursive clone of the new implementation for speed comparison
f61d31e [Validator] Fixed grammar
886e05e [Validator] Removed unused use statement
93fdff7 [Validator] The supported API versions can now be passed to the ValidatorBuilder
987313d [Validator] Improved inline documentation of the violation builder
79387a7 [Validator] Improved inline documentation of the metadata classes
01ceeda [Validator] Improved test coverage of the Traverse constraint
9ca61df [Validator] Improved inline documentation of CascadingStrategy and TraversalStrategy
524a953 [Validator] Improved inline documentation of the validators
9986f03 [Validator] Added inline documentation for the PropertyPath utility class
be7f055 [Validator] Visitors may now abort the traversal by returning false from beforeTraversal()
299c2dc [Validator] Improved test coverage and prevented duplicate validation of constraints
186c115 [Validator] Improved test coverage of NonRecursiveNodeTraverser
822fe47 [Validator] Completed inline documentation of the Node classes and the NodeTraverser
dbce5a2 [Validator] Updated outdated doc blocks
8558377 [Validator] Added deprecation notes
e8fa15b [Validator] Fixed the new validator API under PHP < 5.3.9
2936d10 [Validator] Removed unused use statement
6fc6ecd [Validator] Fixed tests under PHP<5.3.9
778ec24 [Validator] Removed helper class Traversal
76d8c9a [Validator] Fixed typos
4161371 [Validator] Removed unused use statements
aeb6822 [Validator] Improved visitor names
08172bf [Validator] Merged validate(), validateObject() and validateObjects() to simplify usage
51197f6 [Validator] Made traversal of Traversables consistent
117b1b9 [Validator] Wrapped collections into CollectionNode instances
94583a9 [Validator] Changed NodeTraverser to traverse nodes iteratively, not recursively
cf1281f [Validator] Added "Visitor" suffix to all node visitors
230f2a7 [Validator] Fixed exception message
e057b19 [Validator] Decoupled ContextRefresher from ExecutionContext
e440690 [Validator] Renamed validateCollection() to validateObjects()
df41974 [Validator] Changed context manager to context factory
26eafa4 [Validator] Removed unused use statements
bc29591 [Validator] Clearly separated classes supporting the API <2.5/2.5+
a3555fb [Validator] Fixed: Objects are not traversed unless they are instances of Traversable
2c65a28 [Validator] Completed test coverage and documentation of the Node classes
9c9e715 [Validator] Completed documentation of GroupManagerInterface
1e81f3b [Validator] Finished test coverage and documentation of ExecutionContextManager
feb3d6f [Validator] Tested the validation in a separate context
718601c [Validator] Changed validateValue() to validate() in the new API
ee1adad [Validator] Implemented handling of arrays and Traversables in LegacyExecutionContext::validate()
09f744b [Validator] Implemented BC traversal of traversables through validate()
297ba4f [Validator] Added a note why scalars are passed to cascadeObject() in NodeTraverser
9b07b0c [Validator] Implemented BC validation of arrays through validate()
405a03b [Validator] Updated deprecation notes in GroupSequence
499b2bb [Validator] Completed test coverage of ExecutionContext
adc1437 [Validator] Fixed failing tests
4ea3ff6 [Validator] Finished inline documentation of ExecutionContext[Interface]
f6b7288 [Validator] Removed unused use statement
8318286 [Validator] Completed GroupSequence implementation
5fbf848 [Validator] Added note about Callback constraint to CHANGELOG
c1b1e03 [Validator] Added TODO reminder
8ae68c9 [Validator] Made tests green (yay!)
680f1ee [Validator] Renamed $params to $parameters
321d5bb [Validator] Throw exception if ObjectInitializer is constructed without visitors
1156bde [Validator] Extracted code for group sequence resolving into GroupSequenceResolver
b1a9477 [Validator] Added ObjectInitializer visitor
7e3a41d [Validator] Moved visitors to NodeVisitor namespace
a40189c [Validator] Decoupled the new classes a bit
a6ed4ca [Validator] Prototype of the traverser implementation
25cdc68 [Validator] Refactored ValidatorTest and ValidationVisitorTest into an abstract validator test class
@fabpot fabpot merged commit ca6a722 into symfony:master Mar 31, 2014
private $propertyAccessor;

public function __construct(PropertyAccessorInterface $propertyAccessor = null)
public function __construct($propertyAccessor = null)
Copy link
Member

Choose a reason for hiding this comment

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

@webmozart was it intended to remove the typehint ? what else could be passed now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose was to remove the hard dependency on PropertyAccess. I need to verify whether the class is indeed not loaded when we keep the type hint and null is passed.

Copy link
Member

Choose a reason for hiding this comment

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

it is not loaded (exactly like for instanceof, as it is equivalent)

@webmozart webmozart deleted the issue7432 branch April 2, 2014 13:51
@romainneutron romainneutron mentioned this pull request Apr 2, 2014
fabpot added a commit that referenced this pull request Apr 2, 2014
This PR was merged into the 2.5-dev branch.

Discussion
----------

Fix travis build

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT

Since #10287 and #10606 have been merged, errors occur in FrameworkBundle and Console component. This patch solves this. After merging 2.4 in master (that would fix Process suite), everything should be green.

Commits
-------

099e480 Fix travis build
craue added a commit to craue/CraueFormFlowBundle that referenced this pull request Apr 4, 2014
$container->loadFromExtension('framework', array(
'secret' => 's3cr3t',
'validation' => array(
'enabled' => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra spaces.

@webdevilopers
Copy link

👍

@fabpot
Copy link
Member

fabpot commented Jan 21, 2015

@webdevilopers Can you please refrain from commenting on merged PRs? It's sending an email to a lot of people, and takes time to manage. Thank you.

@webdevilopers
Copy link

Sorry, my fault. Was mixing up some open tabs and commented the wrong one.

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.