Skip to content

Commit be7f055

Browse files
webmozartwebmozart
committed
[Validator] Visitors may now abort the traversal by returning false from beforeTraversal()
1 parent 299c2dc commit be7f055

File tree

7 files changed

+280
-68
lines changed

7 files changed

+280
-68
lines changed

‎src/Symfony/Component/Validator/NodeTraverser/NodeTraverserInterface.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@
2525
* {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::visit()}
2626
* of each visitor is called. At the end of the traversal, the traverser invokes
2727
* {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::afterTraversal()}
28-
* on each visitor.
28+
* on each visitor. The visitors are called in the same order in which they are
29+
* added to the traverser.
2930
*
30-
* The visitors should be called in the same order in which they are added to
31-
* the traverser.
31+
* If the {@link traverse()} method is called recursively, the
32+
* {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::beforeTraversal()}
33+
* and {@link \Symfony\Component\Validator\NodeVisitor\NodeVisitorInterface::afterTraversal()}
34+
* methods of the visitors will be invoked for each call.
3235
*
3336
* The validation graph typically contains nodes of the following types:
3437
*
@@ -92,5 +95,5 @@ public function removeVisitor(NodeVisitorInterface $visitor);
9295
* @param Node[] $nodes The nodes to traverse
9396
* @param ExecutionContextInterface $context The validation context
9497
*/
95-
public function traverse(array $nodes, ExecutionContextInterface $context);
98+
public function traverse($nodes, ExecutionContextInterface $context);
9699
}

‎src/Symfony/Component/Validator/NodeTraverser/NonRecursiveNodeTraverser.php‎

Lines changed: 63 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,6 @@ class NonRecursiveNodeTraverser implements NodeTraverserInterface
6868
*/
6969
private $metadataFactory;
7070

71-
/**
72-
* @var Boolean
73-
*/
74-
private $traversalStarted = false;
75-
7671
/**
7772
* Creates a new traverser.
7873
*
@@ -104,20 +99,20 @@ public function removeVisitor(NodeVisitorInterface $visitor)
10499
/**
105100
* {@inheritdoc}
106101
*/
107-
public function traverse(array $nodes, ExecutionContextInterface $context)
102+
public function traverse($nodes, ExecutionContextInterface $context)
108103
{
109-
// beforeTraversal() and afterTraversal() are only executed for the
110-
// top-level call of traverse()
111-
$isTopLevelCall = !$this->traversalStarted;
104+
if (!is_array($nodes)) {
105+
$nodes = array($nodes);
106+
}
112107

113-
if ($isTopLevelCall) {
114-
// Remember that the traversal was already started for the case of
115-
// recursive calls to traverse()
116-
$this->traversalStarted = true;
108+
$numberOfInitializedVisitors = $this->beforeTraversal($nodes, $context);
117109

118-
foreach ($this->visitors as $visitor) {
119-
$visitor->beforeTraversal($nodes, $context);
120-
}
110+
// If any of the visitors requested to abort the traversal, do so, but
111+
// clean up before
112+
if ($numberOfInitializedVisitors < count($this->visitors)) {
113+
$this->afterTraversal($nodes, $context, $numberOfInitializedVisitors);
114+
115+
return;
121116
}
122117

123118
// This stack contains all the nodes that should be traversed
@@ -148,13 +143,60 @@ public function traverse(array $nodes, ExecutionContextInterface $context)
148143
}
149144
}
150145

151-
if ($isTopLevelCall) {
152-
foreach ($this->visitors as $visitor) {
153-
$visitor->afterTraversal($nodes, $context);
146+
$this->afterTraversal($nodes, $context);
147+
}
148+
149+
/**
150+
* Executes the {@link NodeVisitorInterface::beforeTraversal()} method of
151+
* each visitor.
152+
*
153+
* @param Node[] $nodes The traversed nodes
154+
* @param ExecutionContextInterface $context The current execution context
155+
*
156+
* @return integer The number of successful calls. This is lower than
157+
* the number of visitors if any of the visitors'
158+
* beforeTraversal() methods returned false
159+
*/
160+
private function beforeTraversal($nodes, ExecutionContextInterface $context)
161+
{
162+
$numberOfCalls = 1;
163+
164+
foreach ($this->visitors as $visitor) {
165+
if (false === $visitor->beforeTraversal($nodes, $context)) {
166+
break;
154167
}
155168

156-
// Put the traverser back into its initial state
157-
$this->traversalStarted = false;
169+
++$numberOfCalls;
170+
}
171+
172+
return $numberOfCalls;
173+
}
174+
175+
/**
176+
* Executes the {@link NodeVisitorInterface::beforeTraversal()} method of
177+
* each visitor.
178+
*
179+
* @param Node[] $nodes The traversed nodes
180+
* @param ExecutionContextInterface $context The current execution context
181+
* @param integer|null $limit Limits the number of visitors
182+
* on which beforeTraversal()
183+
* should be called. All visitors
184+
* will be called by default
185+
*/
186+
private function afterTraversal($nodes, ExecutionContextInterface $context, $limit = null)
187+
{
188+
if (null === $limit) {
189+
$limit = count($this->visitors);
190+
}
191+
192+
$numberOfCalls = 0;
193+
194+
foreach ($this->visitors as $visitor) {
195+
$visitor->afterTraversal($nodes, $context);
196+
197+
if (++$numberOfCalls === $limit) {
198+
return;
199+
}
158200
}
159201
}
160202

‎src/Symfony/Component/Validator/NodeVisitor/AbstractVisitor.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ abstract class AbstractVisitor implements NodeVisitorInterface
2727
/**
2828
* {@inheritdoc}
2929
*/
30-
public function beforeTraversal(array $nodes, ExecutionContextInterface $context)
30+
public function beforeTraversal($nodes, ExecutionContextInterface $context)
3131
{
3232
}
3333

3434
/**
3535
* {@inheritdoc}
3636
*/
37-
public function afterTraversal(array $nodes, ExecutionContextInterface $context)
37+
public function afterTraversal($nodes, ExecutionContextInterface $context)
3838
{
3939
}
4040

‎src/Symfony/Component/Validator/NodeVisitor/NodeValidationVisitor.php‎

Lines changed: 58 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,6 @@
2929
*/
3030
class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInterface
3131
{
32-
/**
33-
* Stores the hashes of each validated object together with the groups
34-
* in which that object was already validated.
35-
*
36-
* @var array
37-
*/
38-
private $validatedObjects = array();
39-
40-
private $validatedConstraints = array();
41-
4232
/**
4333
* @var ConstraintValidatorFactoryInterface
4434
*/
@@ -49,20 +39,37 @@ class NodeValidationVisitor extends AbstractVisitor implements GroupManagerInter
4939
*/
5040
private $nodeTraverser;
5141

42+
/**
43+
* The currently validated group.
44+
*
45+
* @var string
46+
*/
5247
private $currentGroup;
5348

49+
/**
50+
* Creates a new visitor.
51+
*
52+
* @param NodeTraverserInterface $nodeTraverser The node traverser
53+
* @param ConstraintValidatorFactoryInterface $validatorFactory The validator factory
54+
*/
5455
public function __construct(NodeTraverserInterface $nodeTraverser, ConstraintValidatorFactoryInterface $validatorFactory)
5556
{
5657
$this->validatorFactory = $validatorFactory;
5758
$this->nodeTraverser = $nodeTraverser;
5859
}
5960

60-
public function afterTraversal(array $nodes, ExecutionContextInterface $context)
61-
{
62-
$this->validatedObjects = array();
63-
$this->validatedConstraints = array();
64-
}
65-
61+
/**
62+
* Validates a node's value against the constraints defined in the node's
63+
* metadata.
64+
*
65+
* Objects and constraints that were validated before in the same context
66+
* will be skipped.
67+
*
68+
* @param Node $node The current node
69+
* @param ExecutionContextInterface $context The execution context
70+
*
71+
* @return Boolean Whether to traverse the successor nodes
72+
*/
6673
public function visit(Node $node, ExecutionContextInterface $context)
6774
{
6875
if ($node instanceof CollectionNode) {
@@ -84,21 +91,24 @@ public function visit(Node $node, ExecutionContextInterface $context)
8491
// simply continue traversal (if possible)
8592

8693
foreach ($node->groups as $key => $group) {
87-
// Remember which object was validated for which group
88-
// Skip validation if the object was already validated for this
89-
// group
94+
// Even if we remove the following clause, the constraints on an
95+
// object won't be validated again due to the measures taken in
96+
// validateNodeForGroup().
97+
// The following shortcut, however, prevents validatedNodeForGroup()
98+
// from being called at all and enhances performance a bit.
9099
if ($node instanceof ClassNode) {
91100
// Use the object hash for group sequences
92101
$groupHash = is_object($group) ? spl_object_hash($group) : $group;
93102

94103
if ($context->isObjectValidatedForGroup($objectHash, $groupHash)) {
95-
// Skip this group when validating properties
104+
// Skip this group when validating the successor nodes
105+
// (property and/or collection nodes)
96106
unset($node->groups[$key]);
97107

98108
continue;
99109
}
100110

101-
//$context->markObjectAsValidatedForGroup($objectHash, $groupHash);
111+
$context->markObjectAsValidatedForGroup($objectHash, $groupHash);
102112
}
103113

104114
// Validate normal group
@@ -108,27 +118,34 @@ public function visit(Node $node, ExecutionContextInterface $context)
108118
continue;
109119
}
110120

111-
// Skip the group sequence when validating properties
112-
unset($node->groups[$key]);
113-
114121
// Traverse group sequence until a violation is generated
115122
$this->traverseGroupSequence($node, $group, $context);
116123

117-
// Optimization: If the groups only contain the group sequence,
118-
// we can skip the traversal for the properties of the object
119-
if (1 === count($node->groups)) {
120-
return false;
121-
}
124+
// Skip the group sequence when validating successor nodes
125+
unset($node->groups[$key]);
122126
}
123127

124128
return true;
125129
}
126130

131+
/**
132+
* {@inheritdoc}
133+
*/
127134
public function getCurrentGroup()
128135
{
129136
return $this->currentGroup;
130137
}
131138

139+
/**
140+
* Validates a node's value in each group of a group sequence.
141+
*
142+
* If any of the groups' constraints generates a violation, subsequent
143+
* groups are not validated anymore.
144+
*
145+
* @param Node $node The validated node
146+
* @param GroupSequence $groupSequence The group sequence
147+
* @param ExecutionContextInterface $context The execution context
148+
*/
132149
private function traverseGroupSequence(Node $node, GroupSequence $groupSequence, ExecutionContextInterface $context)
133150
{
134151
$violationCount = count($context->getViolations());
@@ -150,6 +167,17 @@ private function traverseGroupSequence(Node $node, GroupSequence $groupSequence,
150167
}
151168
}
152169

170+
/**
171+
* Validates a node's value against all constraints in the given group.
172+
*
173+
* @param Node $node The validated node
174+
* @param string $group The group to validate
175+
* @param ExecutionContextInterface $context The execution context
176+
* @param string $objectHash The hash of the node's
177+
* object (if any)
178+
*
179+
* @throws \Exception
180+
*/
153181
private function validateNodeForGroup(Node $node, $group, ExecutionContextInterface $context, $objectHash)
154182
{
155183
try {
@@ -176,8 +204,6 @@ private function validateNodeForGroup(Node $node, $group, ExecutionContextInterf
176204

177205
$context->markPropertyConstraintAsValidated($objectHash, $propertyName, $constraintHash);
178206
}
179-
180-
$this->validatedConstraints[$objectHash][$constraintHash] = true;
181207
}
182208

183209
$validator = $this->validatorFactory->getInstance($constraint);
@@ -187,6 +213,7 @@ private function validateNodeForGroup(Node $node, $group, ExecutionContextInterf
187213

188214
$this->currentGroup = null;
189215
} catch (\Exception $e) {
216+
// Should be put into a finally block once we switch to PHP 5.5
190217
$this->currentGroup = null;
191218

192219
throw $e;

‎src/Symfony/Component/Validator/NodeVisitor/NodeVisitorInterface.php‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,31 @@
2929
*/
3030
interface NodeVisitorInterface
3131
{
32-
public function beforeTraversal(array $nodes, ExecutionContextInterface $context);
32+
/**
33+
* Called at the beginning of a traversal.
34+
*
35+
* @param Node[] $nodes A list of Node instances
36+
* @param ExecutionContextInterface $context The execution context
37+
*
38+
* @return Boolean Whether to continue the traversal
39+
*/
40+
public function beforeTraversal($nodes, ExecutionContextInterface $context);
3341

34-
public function afterTraversal(array $nodes, ExecutionContextInterface $context);
42+
/**
43+
* Called at the end of a traversal.
44+
*
45+
* @param Node[] $nodes A list of Node instances
46+
* @param ExecutionContextInterface $context The execution context
47+
*/
48+
public function afterTraversal($nodes, ExecutionContextInterface $context);
3549

50+
/**
51+
* Called for each node during a traversal.
52+
*
53+
* @param Node $node The current node
54+
* @param ExecutionContextInterface $context The execution context
55+
*
56+
* @return Boolean Whether to traverse the node's successor nodes
57+
*/
3658
public function visit(Node $node, ExecutionContextInterface $context);
3759
}

0 commit comments

Comments
 (0)