bug symfony#21288 [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys (dmaicher, HeahDude)

fabpot · fabpot · commit d228d347b4bd · 2017-01-16T08:52:47.000-08:00
This PR was merged into the 3.1 branch. Discussion ---------- [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys | Q | A | ------------- | --- | Branch? | master / 3.1 | Bug fix? | yes | New feature? |no | BC breaks? | no | Deprecations? |no | Tests pass? | yes (fail on php 7.1 unrelated?) | Fixed tickets | symfony#21274 | License | MIT | Doc PR | - This PR fixes an issue with the UniqueEntityValidator in case the entity being validated uses a composite primary key via relations to other entities whose classes do not have a `__toString` method. Commits ------- b3ced86 [DoctrineBridge] Fixed invalid unique value as composite key 5aadce3 [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/CompositeObjectNoToStringIdEntity.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace Symfony\Bridge\Doctrine\Tests\Fixtures;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * an entity that has two objects (class without toString methods) as primary key.
+ *
+ * @ORM\Entity
+ */
+class CompositeObjectNoToStringIdEntity
+{
+    /**
+     * @var SingleIntIdNoToStringEntity
+     *
+     * @ORM\Id
+     * @ORM\ManyToOne(targetEntity="SingleIntIdNoToStringEntity", cascade={"persist"})
+     * @ORM\JoinColumn(name="object_one_id")
+     */
+    protected $objectOne;
+
+    /**
+     * @var SingleIntIdNoToStringEntity
+     *
+     * @ORM\Id
+     * @ORM\ManyToOne(targetEntity="SingleIntIdNoToStringEntity", cascade={"persist"})
+     * @ORM\JoinColumn(name="object_two_id")
+     */
+    protected $objectTwo;
+
+    public function __construct(SingleIntIdNoToStringEntity $objectOne, SingleIntIdNoToStringEntity $objectTwo)
+    {
+        $this->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
@@ -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'),
         ));
     }
 
@@ -173,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();
     }
@@ -198,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();
     }
@@ -417,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();
@@ -450,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();
     }
@@ -561,4 +563,38 @@ 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');
+
+        $this->em->persist($objectOne);
+        $this->em->persist($objectTwo);
+        $this->em->flush();
+
+        $entity = new CompositeObjectNoToStringIdEntity($objectOne, $objectTwo);
+
+        $this->em->persist($entity);
+        $this->em->flush();
+
+        $newEntity = new CompositeObjectNoToStringIdEntity($objectOne, $objectTwo);
+
+        $this->validator->validate($newEntity, $constraint);
+
+        $expectedValue = 'object("Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity") identified by (id => 1)';
+
+        $this->buildViolation('myMessage')
+            ->atPath('property.path.objectOne')
+            ->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
@@ -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;
@@ -127,15 +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 = sprintf('Object of class "%s" identified by "%s"', get_class($entity), implode(', ', $class->getIdentifierValues($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();
     }
+
+    private function formatWithIdentifiers(ObjectManager $em, ClassMetadata $class, $value)
+    {
+        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));
+            }
+
+            $id = sprintf('%s => %s', $field, $idAsString);
+        });
+
+        return sprintf('object("%s") identified by (%s)', $idClass, implode(', ', $identifiers));
+    }
 }
