Skip to content

Commit

Permalink
[DoctrineBridge][Form] Fix EntityChoiceList when indexing by primary …
Browse files Browse the repository at this point in the history
…foreign key
  • Loading branch information
giosh94mhz committed Jul 9, 2015
1 parent 6f8a37c commit fe4246a
Show file tree
Hide file tree
Showing 7 changed files with 310 additions and 15 deletions.
97 changes: 83 additions & 14 deletions src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php
Expand Up @@ -40,6 +40,13 @@ class EntityChoiceList extends ObjectChoiceList
*/
private $classMetadata;

/**
* Metadata for target class of primary key association.
*
* @var ClassMetadata
*/
private $idClassMetadata;

/**
* Contains the query builder that builds the query for fetching the
* entities.
Expand Down Expand Up @@ -107,16 +114,21 @@ public function __construct(ObjectManager $manager, $class, $labelPath = null, E
$this->class = $this->classMetadata->getName();
$this->loaded = is_array($entities) || $entities instanceof \Traversable;
$this->preferredEntities = $preferredEntities;
list(
$this->idAsIndex,
$this->idAsValue,
$this->idField
) = $this->getIdentifierInfoForClass($this->classMetadata);

if (null !== $this->idField && $this->classMetadata->hasAssociation($this->idField)) {
$this->idClassMetadata = $this->em->getClassMetadata(
$this->classMetadata->getAssociationTargetClass($this->idField)
);

$identifier = $this->classMetadata->getIdentifierFieldNames();

if (1 === count($identifier)) {
$this->idField = $identifier[0];
$this->idAsValue = true;

if (in_array($this->classMetadata->getTypeOfField($this->idField), array('integer', 'smallint', 'bigint'))) {
$this->idAsIndex = true;
}
list(
$this->idAsIndex,
$this->idAsValue
) = $this->getIdentifierInfoForClass($this->idClassMetadata);
}

if (!$this->loaded) {
Expand Down Expand Up @@ -228,7 +240,7 @@ public function getChoicesForValues(array $values)
// "INDEX BY" clause to the Doctrine query in the loader,
// but I'm not sure whether that's doable in a generic fashion.
foreach ($unorderedEntities as $entity) {
$value = $this->fixValue(current($this->getIdentifierValues($entity)));
$value = $this->fixValue($this->getSingleIdentifierValue($entity));
$entitiesByValue[$value] = $entity;
}

Expand Down Expand Up @@ -274,7 +286,7 @@ public function getValuesForChoices(array $entities)
foreach ($entities as $i => $entity) {
if ($entity instanceof $this->class) {
// Make sure to convert to the right format
$values[$i] = $this->fixValue(current($this->getIdentifierValues($entity)));
$values[$i] = $this->fixValue($this->getSingleIdentifierValue($entity));
}
}

Expand Down Expand Up @@ -314,7 +326,7 @@ public function getIndicesForChoices(array $entities)
foreach ($entities as $i => $entity) {
if ($entity instanceof $this->class) {
// Make sure to convert to the right format
$indices[$i] = $this->fixIndex(current($this->getIdentifierValues($entity)));
$indices[$i] = $this->fixIndex($this->getSingleIdentifierValue($entity));
}
}

Expand Down Expand Up @@ -372,7 +384,7 @@ public function getIndicesForValues(array $values)
protected function createIndex($entity)
{
if ($this->idAsIndex) {
return $this->fixIndex(current($this->getIdentifierValues($entity)));
return $this->fixIndex($this->getSingleIdentifierValue($entity));
}

return parent::createIndex($entity);
Expand All @@ -392,7 +404,7 @@ protected function createIndex($entity)
protected function createValue($entity)
{
if ($this->idAsValue) {
return (string) current($this->getIdentifierValues($entity));
return (string) $this->getSingleIdentifierValue($entity);
}

return parent::createValue($entity);
Expand All @@ -415,6 +427,36 @@ protected function fixIndex($index)
return $index;
}

/**
* Get identifier information for a class.
*
* @param ClassMetadata $classMetadata The entity metadata
*
* @return array Return an array with idAsIndex, idAsValue and identifier
*/
private function getIdentifierInfoForClass(ClassMetadata $classMetadata)
{
$identifier = null;
$idAsIndex = false;
$idAsValue = false;

$identifiers = $classMetadata->getIdentifierFieldNames();

if (1 === count($identifiers)) {
$identifier = $identifiers[0];

if (!$classMetadata->hasAssociation($identifier)) {
$idAsValue = true;

if (in_array($classMetadata->getTypeOfField($identifier), array('integer', 'smallint', 'bigint'))) {
$idAsIndex = true;
}
}
}

return array($idAsIndex, $idAsValue, $identifier);
}

/**
* Loads the list with entities.
*
Expand All @@ -438,6 +480,33 @@ private function load()
$this->loaded = true;
}

/**
* Returns the first (and only) value of the identifier fields of an entity.
*
* Doctrine must know about this entity, that is, the entity must already
* be persisted or added to the identity map before. Otherwise an
* exception is thrown.
*
* @param object $entity The entity for which to get the identifier
*
* @return array The identifier values
*
* @throws RuntimeException If the entity does not exist in Doctrine's identity map
*/
private function getSingleIdentifierValue($entity)
{
$value = current($this->getIdentifierValues($entity));

if ($this->idClassMetadata) {
$class = $this->idClassMetadata->getName();
if ($value instanceof $class) {
$value = current($this->idClassMetadata->getIdentifierValues($value));
}
}

return $value;
}

/**
* Returns the values of the identifier fields of an entity.
*
Expand Down
@@ -0,0 +1,38 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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;
}
}
@@ -0,0 +1,86 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList;

use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;

/**
* Test choices generated from an entity with a primary foreign key.
*
* @author Premi Giorgio <giosh94mhz@gmail.com>
* @author Bernhard Schussek <bschussek@gmail.com>
*/
abstract class AbstractEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListTest
{
protected function getEntityClass()
{
return 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity';
}

protected function getClassesMetadata()
{
return array(
$this->em->getClassMetadata($this->getEntityClass()),
$this->em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity'),
);
}

protected function createChoiceList()
{
return new EntityChoiceList($this->em, $this->getEntityClass(), 'name');
}

/**
* @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
*/
protected function createObjects()
{
$innerA = new SingleIntIdNoToStringEntity(-10, 'inner_A');
$innerB = new SingleIntIdNoToStringEntity(10, 'inner_B');
$innerC = new SingleIntIdNoToStringEntity(20, 'inner_C');
$innerD = new SingleIntIdNoToStringEntity(30, 'inner_D');

$this->em->persist($innerA);
$this->em->persist($innerB);
$this->em->persist($innerC);
$this->em->persist($innerD);

return array(
new SingleAssociationToIntIdEntity($innerA, 'A'),
new SingleAssociationToIntIdEntity($innerB, 'B'),
new SingleAssociationToIntIdEntity($innerC, 'C'),
new SingleAssociationToIntIdEntity($innerD, 'D'),
);
}

protected function getChoices()
{
return array('_10' => $this->obj1, 10 => $this->obj2, 20 => $this->obj3, 30 => $this->obj4);
}

protected function getLabels()
{
return array('_10' => 'A', 10 => 'B', 20 => 'C', 30 => 'D');
}

protected function getValues()
{
return array('_10' => '-10', 10 => '10', 20 => '20', 30 => '30');
}

protected function getIndices()
{
return array('_10', 10, 20, 30);
}
}
Expand Up @@ -39,7 +39,7 @@ protected function setUp()
$this->em = DoctrineTestHelper::createTestEntityManager();

$schemaTool = new SchemaTool($this->em);
$classes = array($this->em->getClassMetadata($this->getEntityClass()));
$classes = $this->getClassesMetadata();

try {
$schemaTool->dropSchema($classes);
Expand Down Expand Up @@ -73,6 +73,11 @@ abstract protected function getEntityClass();

abstract protected function createObjects();

protected function getClassesMetadata()
{
return array($this->em->getClassMetadata($this->getEntityClass()));
}

/**
* @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
*/
Expand Down
@@ -0,0 +1,32 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList;

/**
* @author Premi Giorgio <giosh94mhz@gmail.com>
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class LoadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest
{
/**
* @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
*/
protected function createChoiceList()
{
$list = parent::createChoiceList();

// load list
$list->getChoices();

return $list;
}
}
@@ -0,0 +1,32 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList;

/**
* @author Premi Giorgio <giosh94mhz@gmail.com>
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class UnloadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest
{
public function testGetIndicesForValuesIgnoresNonExistingValues()
{
$this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.');
}

/**
* @group legacy
*/
public function testLegacyGetIndicesForValuesIgnoresNonExistingValues()
{
$this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.');
}
}

0 comments on commit fe4246a

Please sign in to comment.