From 799d0e0298ba017443c1a0bc88af80da9b7ba1d6 Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Thu, 9 Jul 2015 14:16:48 +0200 Subject: [PATCH] [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key --- .../Doctrine/Form/ChoiceList/IdReader.php | 24 +++- .../SingleAssociationToIntIdEntity.php | 38 ++++++ .../Tests/Form/Type/EntityTypeTest.php | 111 ++++++++++++++++++ 3 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php index 43ca6a2fe7..6ae98b57f8 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php @@ -19,6 +19,7 @@ use Symfony\Component\Form\Exception\RuntimeException; * A utility for reading object IDs. * * @since 1.0 + * * @author Bernhard Schussek * * @internal This class is meant for internal use only. @@ -50,6 +51,11 @@ class IdReader */ private $idField; + /** + * @var IdReader|null + */ + private $associationIdReader; + public function __construct(ObjectManager $om, ClassMetadata $classMetadata) { $ids = $classMetadata->getIdentifierFieldNames(); @@ -60,6 +66,16 @@ class IdReader $this->singleId = 1 === count($ids); $this->intId = $this->singleId && in_array($idType, array('integer', 'smallint', 'bigint')); $this->idField = current($ids); + + // single field association are resolved, since the schema column could be an int + if ($this->singleId && $classMetadata->hasAssociation($this->idField)) { + $this->associationIdReader = new self($om, $om->getClassMetadata( + $classMetadata->getAssociationTargetClass($this->idField) + )); + + $this->singleId = $this->associationIdReader->isSingleId(); + $this->intId = $this->associationIdReader->isIntId(); + } } /** @@ -108,7 +124,13 @@ class IdReader $this->om->initializeObject($object); - return current($this->classMetadata->getIdentifierValues($object)); + $idValue = current($this->classMetadata->getIdentifierValues($object)); + + if ($this->associationIdReader) { + $idValue = $this->associationIdReader->getIdValue($idValue); + } + + return $idValue; } /** diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php new file mode 100644 index 0000000000..954de338d3 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Fixtures; + +use Doctrine\ORM\Mapping\Id; +use Doctrine\ORM\Mapping\Column; +use Doctrine\ORM\Mapping\Entity; +use Doctrine\ORM\Mapping\OneToOne; + +/** @Entity */ +class SingleAssociationToIntIdEntity +{ + /** @Id @OneToOne(targetEntity="SingleIntIdNoToStringEntity", cascade={"ALL"}) */ + protected $entity; + + /** @Column(type="string", nullable=true) */ + public $name; + + public function __construct(SingleIntIdNoToStringEntity $entity, $name) + { + $this->entity = $entity; + $this->name = $name; + } + + public function __toString() + { + return (string) $this->name; + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php index e22db0093c..2fca804ca6 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php @@ -30,12 +30,16 @@ use Symfony\Component\Form\ChoiceList\View\ChoiceView; use Symfony\Component\Form\Forms; use Symfony\Component\Form\Test\TypeTestCase; use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity; +use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity; class EntityTypeTest extends TypeTestCase { const ITEM_GROUP_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\GroupableEntity'; const SINGLE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity'; + const SINGLE_IDENT_NO_TO_STRING_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity'; const SINGLE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity'; + const SINGLE_ASSOC_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity'; const COMPOSITE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIntIdEntity'; const COMPOSITE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeStringIdEntity'; @@ -60,7 +64,9 @@ class EntityTypeTest extends TypeTestCase $classes = array( $this->em->getClassMetadata(self::ITEM_GROUP_CLASS), $this->em->getClassMetadata(self::SINGLE_IDENT_CLASS), + $this->em->getClassMetadata(self::SINGLE_IDENT_NO_TO_STRING_CLASS), $this->em->getClassMetadata(self::SINGLE_STRING_IDENT_CLASS), + $this->em->getClassMetadata(self::SINGLE_ASSOC_IDENT_CLASS), $this->em->getClassMetadata(self::COMPOSITE_IDENT_CLASS), $this->em->getClassMetadata(self::COMPOSITE_STRING_IDENT_CLASS), ); @@ -304,6 +310,31 @@ class EntityTypeTest extends TypeTestCase $this->assertSame('2', $field->getViewData()); } + public function testSubmitSingleNonExpandedSingleAssocIdentifier() + { + $innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo'); + $innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar'); + + $entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo'); + $entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar'); + + $this->persist(array($innerEntity1, $innerEntity2, $entity1, $entity2)); + + $field = $this->factory->createNamed('name', 'entity', null, array( + 'multiple' => false, + 'expanded' => false, + 'em' => 'default', + 'class' => self::SINGLE_ASSOC_IDENT_CLASS, + 'choice_label' => 'name', + )); + + $field->submit('2'); + + $this->assertTrue($field->isSynchronized()); + $this->assertSame($entity2, $field->getData()); + $this->assertSame('2', $field->getViewData()); + } + public function testSubmitSingleNonExpandedCompositeIdentifier() { $entity1 = new CompositeIntIdEntity(10, 20, 'Foo'); @@ -352,6 +383,35 @@ class EntityTypeTest extends TypeTestCase $this->assertSame(array('1', '3'), $field->getViewData()); } + public function testSubmitMultipleNonExpandedSingleAssocIdentifier() + { + $innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo'); + $innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar'); + $innerEntity3 = new SingleIntIdNoToStringEntity(3, 'InBaz'); + + $entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo'); + $entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar'); + $entity3 = new SingleAssociationToIntIdEntity($innerEntity3, 'Baz'); + + $this->persist(array($innerEntity1, $innerEntity2, $innerEntity3, $entity1, $entity2, $entity3)); + + $field = $this->factory->createNamed('name', 'entity', null, array( + 'multiple' => true, + 'expanded' => false, + 'em' => 'default', + 'class' => self::SINGLE_ASSOC_IDENT_CLASS, + 'choice_label' => 'name', + )); + + $field->submit(array('1', '3')); + + $expected = new ArrayCollection(array($entity1, $entity3)); + + $this->assertTrue($field->isSynchronized()); + $this->assertEquals($expected, $field->getData()); + $this->assertSame(array('1', '3'), $field->getViewData()); + } + public function testSubmitMultipleNonExpandedSingleIdentifierForExistingData() { $entity1 = new SingleIntIdEntity(1, 'Foo'); @@ -611,6 +671,29 @@ class EntityTypeTest extends TypeTestCase $this->assertNull($field->getData()); } + public function testDisallowChoicesThatAreNotIncludedChoicesSingleAssocIdentifier() + { + $innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo'); + $innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar'); + + $entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo'); + $entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar'); + + $this->persist(array($innerEntity1, $innerEntity2, $entity1, $entity2)); + + $field = $this->factory->createNamed('name', 'entity', null, array( + 'em' => 'default', + 'class' => self::SINGLE_ASSOC_IDENT_CLASS, + 'choices' => array($entity1, $entity2), + 'choice_label' => 'name', + )); + + $field->submit('3'); + + $this->assertFalse($field->isSynchronized()); + $this->assertNull($field->getData()); + } + public function testDisallowChoicesThatAreNotIncludedChoicesCompositeIdentifier() { $entity1 = new CompositeIntIdEntity(10, 20, 'Foo'); @@ -656,6 +739,34 @@ class EntityTypeTest extends TypeTestCase $this->assertNull($field->getData()); } + public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleAssocIdentifier() + { + $innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo'); + $innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar'); + $innerEntity3 = new SingleIntIdNoToStringEntity(3, 'InBaz'); + + $entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo'); + $entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar'); + $entity3 = new SingleAssociationToIntIdEntity($innerEntity3, 'Baz'); + + $this->persist(array($innerEntity1, $innerEntity2, $innerEntity3, $entity1, $entity2, $entity3)); + + $repository = $this->em->getRepository(self::SINGLE_ASSOC_IDENT_CLASS); + + $field = $this->factory->createNamed('name', 'entity', null, array( + 'em' => 'default', + 'class' => self::SINGLE_ASSOC_IDENT_CLASS, + 'query_builder' => $repository->createQueryBuilder('e') + ->where('e.entity IN (1, 2)'), + 'choice_label' => 'name', + )); + + $field->submit('3'); + + $this->assertFalse($field->isSynchronized()); + $this->assertNull($field->getData()); + } + public function testDisallowChoicesThatAreNotIncludedQueryBuilderAsClosureSingleIdentifier() { $entity1 = new SingleIntIdEntity(1, 'Foo');