[Validator] Fixed: GraphWalker does not add constraint violation if e…

…rror message is empty
symfony · Jan 30, 2012 · f904a9e · f904a9e
1 parent 1dd302c
commit f904a9e
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 26 deletions.
diff --git a/src/Symfony/Component/Validator/ConstraintValidator.php b/src/Symfony/Component/Validator/ConstraintValidator.php
@@ -11,7 +11,11 @@
 
 namespace Symfony\Component\Validator;
 
-/*
+/**
+ * Base class for constraint validators
+ *
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ *
  * @api
  */
 abstract class ConstraintValidator implements ConstraintValidatorInterface

diff --git a/src/Symfony/Component/Validator/ConstraintViolationList.php b/src/Symfony/Component/Validator/ConstraintViolationList.php
@@ -12,14 +12,33 @@
 namespace Symfony\Component\Validator;
 
 /**
- * An array-acting object that holds many ConstrainViolation instances.
+ * An list of ConstrainViolation objects.
+ *
+ * @author Bernhard Schussek <bschussek@gmail.com>
  *
  * @api
  */
 class ConstraintViolationList implements \IteratorAggregate, \Countable, \ArrayAccess
 {
+    /**
+     * The constraint violations
+     *
+     * @var array
+     */
     protected $violations = array();
 
+    /**
+     * Creates a new constraint violation list
+     *
+     * @param array $violations The constraint violations to add to the list
+     */
+    public function __construct(array $violations = array())
+    {
+        foreach ($violations as $violation) {
+            $this->add($violation);
+        }
+    }
+
     /**
      * @return string
      */

diff --git a/src/Symfony/Component/Validator/GraphWalker.php b/src/Symfony/Component/Validator/GraphWalker.php
@@ -174,16 +174,20 @@ public function walkConstraint(Constraint $constraint, $value, $group, $property
         $validator->initialize($this->context);
 
         if (!$validator->isValid($value, $constraint)) {
-            // Resetting the property path. This is needed because some
-            // validators, like CollectionValidator, use the walker internally
-            // and so change the context.
-            $this->context->setPropertyPath($propertyPath);
-
-            $this->context->addViolation(
-                $validator->getMessageTemplate(),
-                $validator->getMessageParameters(),
-                $value
-            );
+            $messageTemplate = $validator->getMessageTemplate();
+            $messageParams = $validator->getMessageParameters();
+
+            // Somewhat ugly hack: Don't add a violation if no message is set.
+            // This is required if the validator added its violations directly
+            // to the context already
+            if (!empty($messageTemplate)) {
+                // Resetting the property path. This is needed because some
+                // validators, like CollectionValidator, use the walker internally
+                // and so change the context.
+                $this->context->setPropertyPath($propertyPath);
+
+                $this->context->addViolation($messageTemplate, $messageParams, $value);
+            }
         }
     }
 }
diff --git a/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php b/tests/Symfony/Tests/Component/Validator/ExecutionContextTest.php
@@ -98,9 +98,9 @@ public function testViolationsAsString()
         $violations = $this->context->getViolations();
 
         $expected = <<<EOF
-Root.:
+Root:
     Message 1
-Root.:
+Root:
     Message 2
 
 EOF;

diff --git a/tests/Symfony/Tests/Component/Validator/Fixtures/FailingConstraint.php b/tests/Symfony/Tests/Component/Validator/Fixtures/FailingConstraint.php
@@ -6,7 +6,7 @@
 
 class FailingConstraint extends Constraint
 {
-    public $message = '';
+    public $message = 'Failed';
 
     public function getTargets()
     {

diff --git a/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php b/tests/Symfony/Tests/Component/Validator/GraphWalkerTest.php
@@ -143,7 +143,7 @@ public function testWalkObjectInDefaultGroupTraversesGroupSequence()
         // validated
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'firstName',
@@ -175,7 +175,7 @@ public function testWalkObjectInGroupSequencePropagatesDefaultGroup()
         // "Default" was launched
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'reference',
@@ -202,7 +202,7 @@ public function testWalkObjectInOtherGroupTraversesNoGroupSequence()
         // Only group "Second" was validated
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'lastName',
@@ -254,7 +254,7 @@ public function testWalkCascadedPropertyValidatesReferences()
 
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'path',
@@ -286,7 +286,7 @@ public function testWalkCascadedPropertyValidatesArraysByDefault()
 
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'path[key]',
@@ -319,7 +319,7 @@ public function testWalkCascadedPropertyValidatesTraversableByDefault()
 
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             'Root',
             'path[key]',

diff --git a/tests/Symfony/Tests/Component/Validator/ValidatorTest.php b/tests/Symfony/Tests/Component/Validator/ValidatorTest.php
@@ -55,7 +55,7 @@ public function testValidate_defaultGroup()
         // Only the constraint of group "Default" failed
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             $entity,
             'firstName',
@@ -78,7 +78,7 @@ public function testValidate_oneGroup()
         // Only the constraint of group "Custom" failed
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             $entity,
             'lastName',
@@ -103,14 +103,14 @@ public function testValidate_multipleGroups()
         // The constraints of both groups failed
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             $entity,
             'firstName',
             ''
         ));
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             $entity,
             'lastName',
@@ -150,7 +150,7 @@ public function testValidateValue()
     {
         $violations = new ConstraintViolationList();
         $violations->add(new ConstraintViolation(
-            '',
+            'Failed',
             array(),
             '',
             '',