From 5aadce39891a916112436dc779057c7b9dd75a97 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Sat, 14 Jan 2017 12:51:35 +0100 Subject: [PATCH 1/2] [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys --- .../CompositeObjectNoToStringIdEntity.php | 53 +++++++++++++++++++ .../Constraints/UniqueEntityValidatorTest.php | 31 +++++++++++ .../Constraints/UniqueEntityValidator.php | 25 ++++++++- 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php new file mode 100644 index 0000000000000..ac97367094bd5 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php @@ -0,0 +1,53 @@ +objectOne = $objectOne; + $this->objectTwo = $objectTwo; + } + + /** + * @return SingleIntIdNoToStringEntity + */ + public function getObjectOne() + { + return $this->objectOne; + } + + /** + * @return SingleIntIdNoToStringEntity + */ + public function getObjectTwo() + { + return $this->objectTwo; + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php index f141410af83c6..5d903a12ebda2 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php @@ -17,6 +17,7 @@ use Doctrine\Common\Persistence\ObjectRepository; use Symfony\Bridge\Doctrine\Test\DoctrineTestHelper; use Symfony\Bridge\Doctrine\Test\TestRepositoryFactory; +use Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity; use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity; use Symfony\Bridge\Doctrine\Tests\Fixtures\DoubleNameEntity; use Symfony\Bridge\Doctrine\Tests\Fixtures\AssociationEntity; @@ -140,6 +141,7 @@ private function createSchema(ObjectManager $em) $em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIntIdEntity'), $em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\AssociationEntity'), $em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\AssociationEntity2'), + $em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity'), )); } @@ -561,4 +563,33 @@ public function testEntityManagerNullObject() $this->validator->validate($entity, $constraint); } + + public function testValidateUniquenessWithCompositeObjectNoToStringIdEntity() + { + $constraint = new UniqueEntity(array( + 'message' => 'myMessage', + 'fields' => array('objectOne', 'objectTwo'), + 'em' => self::EM_NAME, + )); + + $objectOne = new SingleIntIdNoToStringEntity(1, 'foo'); + $objectTwo = new SingleIntIdNoToStringEntity(2, 'bar'); + $entity = new CompositeObjectNoToStringIdEntity($objectOne, $objectTwo); + + $this->em->persist($entity); + $this->em->flush(); + + $newEntity = new CompositeObjectNoToStringIdEntity($objectOne, $objectTwo); + + $this->validator->validate($newEntity, $constraint); + + $expectedValue = 'Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity" identified by "(Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "1"), (Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "2")"'; + + $this->buildViolation('myMessage') + ->atPath('property.path.objectOne') + ->setParameter('{{ value }}', '"'.$expectedValue.'"') + ->setInvalidValue($expectedValue) + ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) + ->assertRaised(); + } } diff --git a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php index eb90b78af2f7d..68ea4a6709b2a 100644 --- a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php +++ b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php @@ -12,6 +12,8 @@ namespace Symfony\Bridge\Doctrine\Validator\Constraints; use Doctrine\Common\Persistence\ManagerRegistry; +use Doctrine\Common\Persistence\Mapping\ClassMetadata; +use Doctrine\Common\Persistence\ObjectManager; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Exception\UnexpectedTypeException; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; @@ -128,7 +130,7 @@ public function validate($entity, Constraint $constraint) $invalidValue = isset($criteria[$errorPath]) ? $criteria[$errorPath] : $criteria[$fields[0]]; if (is_object($invalidValue) && !method_exists($invalidValue, '__toString')) { - $invalidValue = sprintf('Object of class "%s" identified by "%s"', get_class($entity), implode(', ', $class->getIdentifierValues($entity))); + $invalidValue = $this->buildInvalidValueString($em, $class, $entity); } $this->context->buildViolation($constraint->message) @@ -138,4 +140,25 @@ public function validate($entity, Constraint $constraint) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->addViolation(); } + + /** + * @param ObjectManager $em + * @param ClassMetadata $class + * @param object $entity + * + * @return string + */ + private function buildInvalidValueString(ObjectManager $em, ClassMetadata $class, $entity) + { + $identifiers = array_map(function ($identifier) use ($em) { + // identifiers can be objects (without any __toString method) if its a composite PK + if (is_object($identifier) && !method_exists($identifier, '__toString')) { + return sprintf('(%s)', $this->buildInvalidValueString($em, $em->getClassMetadata(get_class($identifier)), $identifier)); + } + + return $identifier; + }, $class->getIdentifierValues($entity)); + + return sprintf('Object of class "%s" identified by "%s"', get_class($entity), implode(', ', $identifiers)); + } } From b3ced8608b8229c1ec54181819d869d47ca92a7d Mon Sep 17 00:00:00 2001 From: HeahDude Date: Sat, 14 Jan 2017 19:07:55 +0100 Subject: [PATCH 2/2] [DoctrineBridge] Fixed invalid unique value as composite key Fixed UniqueEntityValidator tests --- .../Constraints/UniqueEntityValidatorTest.php | 23 ++++++---- .../Constraints/UniqueEntityValidator.php | 45 ++++++++++--------- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php index 5d903a12ebda2..efd9c54374181 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Validator/Constraints/UniqueEntityValidatorTest.php @@ -175,7 +175,7 @@ public function testValidateUniqueness() $this->buildViolation('myMessage') ->atPath('property.path.name') ->setParameter('{{ value }}', '"Foo"') - ->setInvalidValue('Foo') + ->setInvalidValue($entity2) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->assertRaised(); } @@ -200,7 +200,7 @@ public function testValidateCustomErrorPath() $this->buildViolation('myMessage') ->atPath('property.path.bar') ->setParameter('{{ value }}', '"Foo"') - ->setInvalidValue('Foo') + ->setInvalidValue($entity2) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->assertRaised(); } @@ -419,7 +419,7 @@ public function testAssociatedEntity() $this->buildViolation('myMessage') ->atPath('property.path.single') - ->setParameter('{{ value }}', $entity1) + ->setParameter('{{ value }}', 'object("Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity") identified by (id => 1)') ->setInvalidValue($entity1) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->assertRaised(); @@ -452,12 +452,12 @@ public function testValidateUniquenessNotToStringEntityWithAssociatedEntity() $this->validator->validate($associated2, $constraint); - $expectedValue = 'Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\AssociationEntity2" identified by "2"'; + $expectedValue = 'object("Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity") identified by (id => 1)'; $this->buildViolation('myMessage') ->atPath('property.path.single') - ->setParameter('{{ value }}', '"'.$expectedValue.'"') - ->setInvalidValue($expectedValue) + ->setParameter('{{ value }}', $expectedValue) + ->setInvalidValue($entity1) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->assertRaised(); } @@ -574,6 +574,11 @@ public function testValidateUniquenessWithCompositeObjectNoToStringIdEntity() $objectOne = new SingleIntIdNoToStringEntity(1, 'foo'); $objectTwo = new SingleIntIdNoToStringEntity(2, 'bar'); + + $this->em->persist($objectOne); + $this->em->persist($objectTwo); + $this->em->flush(); + $entity = new CompositeObjectNoToStringIdEntity($objectOne, $objectTwo); $this->em->persist($entity); @@ -583,12 +588,12 @@ public function testValidateUniquenessWithCompositeObjectNoToStringIdEntity() $this->validator->validate($newEntity, $constraint); - $expectedValue = 'Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity" identified by "(Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "1"), (Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "2")"'; + $expectedValue = 'object("Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity") identified by (id => 1)'; $this->buildViolation('myMessage') ->atPath('property.path.objectOne') - ->setParameter('{{ value }}', '"'.$expectedValue.'"') - ->setInvalidValue($expectedValue) + ->setParameter('{{ value }}', $expectedValue) + ->setInvalidValue($objectOne) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->assertRaised(); } diff --git a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php index 68ea4a6709b2a..16b74b98c3dc0 100644 --- a/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php +++ b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php @@ -129,36 +129,41 @@ public function validate($entity, Constraint $constraint) $errorPath = null !== $constraint->errorPath ? $constraint->errorPath : $fields[0]; $invalidValue = isset($criteria[$errorPath]) ? $criteria[$errorPath] : $criteria[$fields[0]]; - if (is_object($invalidValue) && !method_exists($invalidValue, '__toString')) { - $invalidValue = $this->buildInvalidValueString($em, $class, $entity); - } - $this->context->buildViolation($constraint->message) ->atPath($errorPath) - ->setParameter('{{ value }}', $this->formatValue($invalidValue, static::OBJECT_TO_STRING | static::PRETTY_DATE)) + ->setParameter('{{ value }}', $this->formatWithIdentifiers($em, $class, $invalidValue)) ->setInvalidValue($invalidValue) ->setCode(UniqueEntity::NOT_UNIQUE_ERROR) ->addViolation(); } - /** - * @param ObjectManager $em - * @param ClassMetadata $class - * @param object $entity - * - * @return string - */ - private function buildInvalidValueString(ObjectManager $em, ClassMetadata $class, $entity) + private function formatWithIdentifiers(ObjectManager $em, ClassMetadata $class, $value) { - $identifiers = array_map(function ($identifier) use ($em) { - // identifiers can be objects (without any __toString method) if its a composite PK - if (is_object($identifier) && !method_exists($identifier, '__toString')) { - return sprintf('(%s)', $this->buildInvalidValueString($em, $em->getClassMetadata(get_class($identifier)), $identifier)); + if (!is_object($value) || $value instanceof \DateTimeInterface) { + return $this->formatValue($value, self::PRETTY_DATE); + } + + // non unique value is a composite PK + if ($class->getName() !== $idClass = get_class($value)) { + $identifiers = $em->getClassMetadata($idClass)->getIdentifierValues($value); + } else { + $identifiers = $class->getIdentifierValues($value); + } + + if (!$identifiers) { + return sprintf('object("%s")', $idClass); + } + + array_walk($identifiers, function (&$id, $field) { + if (!is_object($id) || $id instanceof \DateTimeInterface) { + $idAsString = $this->formatValue($id, self::PRETTY_DATE); + } else { + $idAsString = sprintf('object("%s")', get_class($id)); } - return $identifier; - }, $class->getIdentifierValues($entity)); + $id = sprintf('%s => %s', $field, $idAsString); + }); - return sprintf('Object of class "%s" identified by "%s"', get_class($entity), implode(', ', $identifiers)); + return sprintf('object("%s") identified by (%s)', $idClass, implode(', ', $identifiers)); } }