Skip to content

Commit

Permalink
[Form] Fixed regression: Choices are compared by their values if a va…
Browse files Browse the repository at this point in the history
…lue callback is given
  • Loading branch information
webmozart committed Mar 31, 2015
1 parent a289deb commit 26eba76
Show file tree
Hide file tree
Showing 11 changed files with 362 additions and 260 deletions.
Expand Up @@ -11,12 +11,10 @@

namespace Symfony\Bridge\Doctrine\Form\ChoiceList;

use Doctrine\Common\Persistence\Mapping\ClassMetadata;
use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
use Symfony\Component\Form\Exception\RuntimeException;

/**
* Loads choices using a Doctrine object manager.
Expand All @@ -41,67 +39,20 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
private $class;

/**
* @var ClassMetadata
* @var IdReader
*/
private $classMetadata;
private $idReader;

/**
* @var null|EntityLoaderInterface
*/
private $objectLoader;

/**
* The identifier field, unless the identifier is composite
*
* @var null|string
*/
private $idField = null;

/**
* Whether to use the identifier for value generation
*
* @var bool
*/
private $compositeId = true;

/**
* @var ChoiceListInterface
*/
private $choiceList;

/**
* Returns the value of the identifier field of an object.
*
* Doctrine must know about this object, that is, the object must already
* be persisted or added to the identity map before. Otherwise an
* exception is thrown.
*
* This method assumes that the object has a single-column identifier and
* will return a single value instead of an array.
*
* @param object $object The object for which to get the identifier
*
* @return int|string The identifier value
*
* @throws RuntimeException If the object does not exist in Doctrine's identity map
*
* @internal Should not be accessed by user-land code. This method is public
* only to be usable as callback.
*/
public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadata, $object)
{
if (!$om->contains($object)) {
throw new RuntimeException(
'Entities passed to the choice field must be managed. Maybe '.
'persist them in the entity manager?'
);
}

$om->initializeObject($object);

return current($classMetadata->getIdentifierValues($object));
}

/**
* Creates a new choice loader.
*
Expand All @@ -114,22 +65,17 @@ public static function getIdValue(ObjectManager $om, ClassMetadata $classMetadat
* @param ObjectManager $manager The object manager
* @param string $class The class name of the
* loaded objects
* @param IdReader $idReader The reader for the object
* IDs.
* @param null|EntityLoaderInterface $objectLoader The objects loader
*/
public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, EntityLoaderInterface $objectLoader = null)
public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, IdReader $idReader, EntityLoaderInterface $objectLoader = null)
{
$this->factory = $factory;
$this->manager = $manager;
$this->classMetadata = $manager->getClassMetadata($class);
$this->class = $this->classMetadata->getName();
$this->class = $manager->getClassMetadata($class)->getName();
$this->idReader = $idReader;
$this->objectLoader = $objectLoader;

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

if (1 === count($identifier)) {
$this->idField = $identifier[0];
$this->compositeId = false;
}
}

/**
Expand All @@ -145,23 +91,7 @@ public function loadChoiceList($value = null)
? $this->objectLoader->getEntities()
: $this->manager->getRepository($this->class)->findAll();

// If the class has a multi-column identifier, we cannot index the
// objects by their IDs
if ($this->compositeId) {
$this->choiceList = $this->factory->createListFromChoices($objects, $value);

return $this->choiceList;
}

// Index the objects by ID
$objectsById = array();

foreach ($objects as $object) {
$id = self::getIdValue($this->manager, $this->classMetadata, $object);
$objectsById[$id] = $object;
}

$this->choiceList = $this->factory->createListFromChoices($objectsById, $value);
$this->choiceList = $this->factory->createListFromChoices($objects, $value);

return $this->choiceList;
}
Expand Down Expand Up @@ -193,14 +123,14 @@ public function loadValuesForChoices(array $objects, $value = null)
// know that the IDs are used as values

// Attention: This optimization does not check choices for existence
if (!$this->choiceList && !$this->compositeId) {
if (!$this->choiceList && $this->idReader->isSingleId()) {
$values = array();

// Maintain order and indices of the given objects
foreach ($objects as $i => $object) {
if ($object instanceof $this->class) {
// Make sure to convert to the right format
$values[$i] = (string) self::getIdValue($this->manager, $this->classMetadata, $object);
$values[$i] = (string) $this->idReader->getIdValue($object);
}
}

Expand Down Expand Up @@ -240,8 +170,8 @@ public function loadChoicesForValues(array $values, $value = null)

// Optimize performance in case we have an object loader and
// a single-field identifier
if (!$this->choiceList && !$this->compositeId && $this->objectLoader) {
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idField, $values);
if (!$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
$objectsById = array();
$objects = array();

Expand All @@ -250,8 +180,7 @@ public function loadChoicesForValues(array $values, $value = null)
// "INDEX BY" clause to the Doctrine query in the loader,
// but I'm not sure whether that's doable in a generic fashion.
foreach ($unorderedObjects as $object) {
$id = self::getIdValue($this->manager, $this->classMetadata, $object);
$objectsById[$id] = $object;
$objectsById[$this->idReader->getIdValue($object)] = $object;
}

foreach ($values as $i => $id) {
Expand Down
125 changes: 125 additions & 0 deletions src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php
@@ -0,0 +1,125 @@
<?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\Form\ChoiceList;

use Doctrine\Common\Persistence\Mapping\ClassMetadata;
use Doctrine\Common\Persistence\ObjectManager;
use Symfony\Component\Form\Exception\RuntimeException;

/**
* A utility for reading object IDs.
*
* @since 1.0
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @internal This class is meant for internal use only.
*/
class IdReader
{
/**
* @var ObjectManager
*/
private $om;

/**
* @var ClassMetadata
*/
private $classMetadata;

/**
* @var bool
*/
private $singleId;

/**
* @var bool
*/
private $intId;

/**
* @var string
*/
private $idField;

public function __construct(ObjectManager $om, ClassMetadata $classMetadata)
{
$ids = $classMetadata->getIdentifierFieldNames();
$idType = $classMetadata->getTypeOfField(current($ids));

$this->om = $om;
$this->classMetadata = $classMetadata;
$this->singleId = 1 === count($ids);
$this->intId = $this->singleId && 1 === count($ids) && in_array($idType, array('integer', 'smallint', 'bigint'));
$this->idField = current($ids);
}

/**
* Returns whether the class has a single-column ID.
*
* @return bool Returns `true` if the class has a single-column ID and
* `false` otherwise.
*/
public function isSingleId()
{
return $this->singleId;
}

/**
* Returns whether the class has a single-column integer ID.
*
* @return bool Returns `true` if the class has a single-column integer ID
* and `false` otherwise.
*/
public function isIntId()
{
return $this->intId;
}

/**
* Returns the ID value for an object.
*
* This method assumes that the object has a single-column ID.
*
* @param object $object The object.
*
* @return mixed The ID value.
*/
public function getIdValue($object)
{
if (!$object) {
return null;
}

if (!$this->om->contains($object)) {
throw new RuntimeException(
'Entities passed to the choice field must be managed. Maybe '.
'persist them in the entity manager?'
);
}

$this->om->initializeObject($object);

return current($this->classMetadata->getIdentifierValues($object));
}

/**
* Returns the name of the ID field.
*
* This method assumes that the object has a single-column ID.
*
* @return string The name of the ID field.
*/
public function getIdField()
{
return $this->idField;
}
}

0 comments on commit 26eba76

Please sign in to comment.