Skip to content

Commit

Permalink
[Validator] Fix regression with class metadatada on parent classes
Browse files Browse the repository at this point in the history
  • Loading branch information
rmikalkenas authored and nicolas-grekas committed Jul 20, 2023
1 parent 62b6cd0 commit a58fc32
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 33 deletions.
6 changes: 3 additions & 3 deletions Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,11 @@ public function mergeConstraints(self $source)

if ($member instanceof MemberMetadata && !$member->isPrivate($this->name)) {
$property = $member->getPropertyName();
$this->members[$property] = [$member];
$this->members[$property][] = $member;

if ($member instanceof PropertyMetadata) {
if ($member instanceof PropertyMetadata && !isset($this->properties[$property])) {
$this->properties[$property] = $member;
} elseif ($member instanceof GetterMetadata) {
} elseif ($member instanceof GetterMetadata && !isset($this->getters[$property])) {
$this->getters[$property] = $member;
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions Tests/Fixtures/Annotation/Entity.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class Entity extends EntityParent implements EntityInterfaceB
private $internal;
public $data = 'Overridden data';
public $initialized = false;
/**
* @Assert\Type("integer")
*/
protected $other;

public function __construct($internal = null)
{
Expand Down
2 changes: 2 additions & 0 deletions Tests/Fixtures/Attribute/Entity.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class Entity extends EntityParent implements EntityInterfaceB
private $internal;
public $data = 'Overridden data';
public $initialized = false;
#[Assert\Type('integer')]
protected $other;

public function __construct($internal = null)
{
Expand Down
2 changes: 2 additions & 0 deletions Tests/Fixtures/NestedAttribute/Entity.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class Entity extends EntityParent implements EntityInterfaceB
private $internal;
public $data = 'Overridden data';
public $initialized = false;
#[Assert\Type('integer')]
protected $other;

public function __construct($internal = null)
{
Expand Down
48 changes: 21 additions & 27 deletions Tests/Mapping/ClassMetadataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ public function testMergeConstraintsMergesMemberConstraints()
$parent->addPropertyConstraint('firstName', new ConstraintA());
$parent->addPropertyConstraint('firstName', new ConstraintB(['groups' => 'foo']));

$this->metadata->mergeConstraints($parent);
$this->metadata->addPropertyConstraint('firstName', new ConstraintA());
$this->metadata->mergeConstraints($parent);

$constraintA1 = new ConstraintA(['groups' => [
'Default',
Expand All @@ -179,35 +179,29 @@ public function testMergeConstraintsMergesMemberConstraints()
'groups' => ['foo'],
]);

$constraints = [
$constraintA1,
$constraintB,
$constraintA2,
];
$members = $this->metadata->getPropertyMetadata('firstName');

$constraintsByGroup = [
'Default' => [
$constraintA1,
$constraintA2,
],
'EntityParent' => [
$constraintA1,
],
'Entity' => [
$constraintA1,
$constraintA2,
$this->assertCount(2, $members);
$this->assertEquals(self::CLASSNAME, $members[0]->getClassName());
$this->assertEquals([$constraintA2], $members[0]->getConstraints());
$this->assertEquals(
[
'Default' => [$constraintA2],
'Entity' => [$constraintA2],
],
'foo' => [
$constraintB,
$members[0]->constraintsByGroup
);
$this->assertEquals(self::PARENTCLASS, $members[1]->getClassName());
$this->assertEquals([$constraintA1, $constraintB], $members[1]->getConstraints());
$this->assertEquals(
[
'Default' => [$constraintA1],
'Entity' => [$constraintA1],
'EntityParent' => [$constraintA1],
'foo' => [$constraintB],
],
];

$members = $this->metadata->getPropertyMetadata('firstName');

$this->assertCount(1, $members);
$this->assertEquals(self::PARENTCLASS, $members[0]->getClassName());
$this->assertEquals($constraints, $members[0]->getConstraints());
$this->assertEquals($constraintsByGroup, $members[0]->constraintsByGroup);
$members[1]->constraintsByGroup
);
}

public function testMemberMetadatas()
Expand Down
13 changes: 10 additions & 3 deletions Tests/Mapping/Loader/AnnotationLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Symfony\Component\Validator\Constraints\Range;
use Symfony\Component\Validator\Constraints\Required;
use Symfony\Component\Validator\Constraints\Sequentially;
use Symfony\Component\Validator\Constraints\Type;
use Symfony\Component\Validator\Constraints\Valid;
use Symfony\Component\Validator\Mapping\ClassMetadata;
use Symfony\Component\Validator\Mapping\Loader\AnnotationLoader;
Expand Down Expand Up @@ -98,6 +99,7 @@ public function testLoadClassMetadata(string $namespace)
$expected->addGetterConstraint('lastName', new NotNull());
$expected->addGetterMethodConstraint('valid', 'isValid', new IsTrue());
$expected->addGetterConstraint('permissions', new IsTrue());
$expected->addPropertyConstraint('other', new Type('integer'));

// load reflection class so that the comparison passes
$expected->getReflectionClass();
Expand Down Expand Up @@ -139,18 +141,16 @@ public function testLoadClassMetadataAndMerge(string $namespace)
$loader->loadClassMetadata($parent_metadata);

$metadata = new ClassMetadata($namespace.'\Entity');
$loader->loadClassMetadata($metadata);

// Merge parent metaData.
$metadata->mergeConstraints($parent_metadata);

$loader->loadClassMetadata($metadata);

$expected_parent = new ClassMetadata($namespace.'\EntityParent');
$expected_parent->addPropertyConstraint('other', new NotNull());
$expected_parent->getReflectionClass();

$expected = new ClassMetadata($namespace.'\Entity');
$expected->mergeConstraints($expected_parent);

$expected->setGroupSequence(['Foo', 'Entity']);
$expected->addConstraint(new ConstraintA());
Expand Down Expand Up @@ -187,11 +187,18 @@ public function testLoadClassMetadataAndMerge(string $namespace)
$expected->addGetterConstraint('lastName', new NotNull());
$expected->addGetterMethodConstraint('valid', 'isValid', new IsTrue());
$expected->addGetterConstraint('permissions', new IsTrue());
$expected->addPropertyConstraint('other', new Type('integer'));

// load reflection class so that the comparison passes
$expected->getReflectionClass();
$expected->mergeConstraints($expected_parent);

$this->assertEquals($expected, $metadata);

$otherMetadata = $metadata->getPropertyMetadata('other');
$this->assertCount(2, $otherMetadata);
$this->assertInstanceOf(Type::class, $otherMetadata[0]->getConstraints()[0]);
$this->assertInstanceOf(NotNull::class, $otherMetadata[1]->getConstraints()[0]);
}

/**
Expand Down

0 comments on commit a58fc32

Please sign in to comment.