[Validator] Fixed annotation loader to not add parent constraints twice

symfony · Nov 12, 2010 · 69cd21d · 69cd21d
1 parent 48b3e92
commit 69cd21d
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 17 deletions.
diff --git a/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php
@@ -34,6 +34,7 @@ public function __construct()
     public function loadClassMetadata(ClassMetadata $metadata)
     {
         $reflClass = $metadata->getReflectionClass();
+        $className = $reflClass->getName();
         $loaded = false;
 
         foreach ($this->reader->getClassAnnotations($reflClass) as $constraint) {
@@ -49,33 +50,37 @@ public function loadClassMetadata(ClassMetadata $metadata)
         }
 
         foreach ($reflClass->getProperties() as $property) {
-            foreach ($this->reader->getPropertyAnnotations($property) as $constraint) {
-                if ($constraint instanceof Validation) {
-                    foreach ($constraint->constraints as $constraint) {
+            if ($property->getDeclaringClass()->getName() == $className) {
+                foreach ($this->reader->getPropertyAnnotations($property) as $constraint) {
+                    if ($constraint instanceof Validation) {
+                        foreach ($constraint->constraints as $constraint) {
+                            $metadata->addPropertyConstraint($property->getName(), $constraint);
+                        }
+                    } elseif ($constraint instanceof Constraint) {
                         $metadata->addPropertyConstraint($property->getName(), $constraint);
                     }
-                } elseif ($constraint instanceof Constraint) {
-                    $metadata->addPropertyConstraint($property->getName(), $constraint);
-                }
 
-                $loaded = true;
+                    $loaded = true;
+                }
             }
         }
 
         foreach ($reflClass->getMethods() as $method) {
-            foreach ($this->reader->getMethodAnnotations($method) as $constraint) {
-                // TODO: clean this up
-                $name = lcfirst(substr($method->getName(), 0, 3)=='get' ? substr($method->getName(), 3) : substr($method->getName(), 2));
+            if ($method->getDeclaringClass()->getName() ==  $className) {
+                foreach ($this->reader->getMethodAnnotations($method) as $constraint) {
+                    // TODO: clean this up
+                    $name = lcfirst(substr($method->getName(), 0, 3)=='get' ? substr($method->getName(), 3) : substr($method->getName(), 2));
 
-                if ($constraint instanceof Validation) {
-                    foreach ($constraint->constraints as $constraint) {
+                    if ($constraint instanceof Validation) {
+                        foreach ($constraint->constraints as $constraint) {
+                            $metadata->addGetterConstraint($name, $constraint);
+                        }
+                    } elseif ($constraint instanceof Constraint) {
                         $metadata->addGetterConstraint($name, $constraint);
                     }
-                } elseif ($constraint instanceof Constraint) {
-                    $metadata->addGetterConstraint($name, $constraint);
-                }
 
-                $loaded = true;
+                    $loaded = true;
+                }
             }
         }
 

diff --git a/tests/Symfony/Tests/Component/Validator/Fixtures/Entity.php b/tests/Symfony/Tests/Component/Validator/Fixtures/Entity.php
@@ -25,7 +25,6 @@ class Entity extends EntityParent implements EntityInterface
      * @validation:Choice(choices={"A", "B"}, message="Must be one of %choices%")
      */
     protected $firstName;
-
     protected $lastName;
 
     private $internal;

diff --git a/tests/Symfony/Tests/Component/Validator/Fixtures/EntityParent.php b/tests/Symfony/Tests/Component/Validator/Fixtures/EntityParent.php
@@ -6,4 +6,9 @@ class EntityParent
 {
     protected $firstName;
     private $internal;
+
+    /**
+     * @validation:NotNull
+     */
+    protected $other;
 }
diff --git a/tests/Symfony/Tests/Component/Validator/Mapping/Loader/AnnotationLoaderTest.php b/tests/Symfony/Tests/Component/Validator/Mapping/Loader/AnnotationLoaderTest.php
@@ -61,4 +61,69 @@ public function testLoadClassMetadata()
 
         $this->assertEquals($expected, $metadata);
     }
+
+    /**
+     * Test MetaData merge with parent annotation.
+     */
+    public function testLoadParentClassMetadata()
+    {
+        $loader = new AnnotationLoader();
+
+        // Load Parent MetaData
+        $parent_metadata = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\EntityParent');
+        $loader->loadClassMetadata($parent_metadata);
+
+        $expected_parent = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\EntityParent');
+        $expected_parent->addPropertyConstraint('other', new NotNull());
+        $expected_parent->getReflectionClass();
+
+        $this->assertEquals($expected_parent, $parent_metadata);
+    }
+    /**
+     * Test MetaData merge with parent annotation.
+     */
+    public function testLoadClassMetadataAndMerge()
+    {
+        $loader = new AnnotationLoader();
+
+        // Load Parent MetaData
+        $parent_metadata = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\EntityParent');
+        $loader->loadClassMetadata($parent_metadata);
+
+        $metadata = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\Entity');
+
+        // Merge parent metaData.
+        $metadata->mergeConstraints($parent_metadata);
+
+        $loader->loadClassMetadata($metadata);
+
+        $expected_parent = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\EntityParent');
+        $expected_parent->addPropertyConstraint('other', new NotNull());
+        $expected_parent->getReflectionClass();
+
+        $expected = new ClassMetadata('Symfony\Tests\Component\Validator\Fixtures\Entity');
+        $expected->mergeConstraints($expected_parent);
+
+        $expected->addConstraint(new NotNull());
+        $expected->addConstraint(new ConstraintA());
+        $expected->addConstraint(new Min(3));
+        $expected->addConstraint(new Choice(array('A', 'B')));
+        $expected->addConstraint(new All(array(new NotNull(), new Min(3))));
+        $expected->addConstraint(new All(array('constraints' => array(new NotNull(), new Min(3)))));
+        $expected->addConstraint(new Collection(array('fields' => array(
+            'foo' => array(new NotNull(), new Min(3)),
+            'bar' => new Min(5),
+        ))));
+        $expected->addPropertyConstraint('firstName', new Choice(array(
+            'message' => 'Must be one of %choices%',
+            'choices' => array('A', 'B'),
+        )));
+        $expected->addGetterConstraint('lastName', new NotNull());
+
+        // load reflection class so that the comparison passes
+        $expected->getReflectionClass();
+
+        $this->assertEquals($expected, $metadata);
+    }
+
 }