diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #9469 [2.2][Propel1] re-factor Propel1 ModelChoiceList (havvg)

Browse Source 
This PR was merged into the 2.2 branch.

Discussion
----------

[2.2][Propel1] re-factor Propel1 ModelChoiceList

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | fixes propelorm/Propel#789
| License       | MIT

replaces #9458

Commits
-------

613b5f6 re-factor Propel1 ModelChoiceList
This commit is contained in:
Fabien Potencier 2013-11-14 14:42:38 +01:00
commit 43371bb754
5 changed files with 355 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

303
src/Symfony/Bridge/Propel1/Form/ChoiceList/ModelChoiceList.php
View File

@ -21,7 +21,7 @@ use Symfony\Component\Form\Extension\Core\ChoiceList\ObjectChoiceList;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
/**
* Widely inspired by the EntityChoiceList.
* A choice list for object choices based on Propel model.
*
* @author William Durand <william.durand1@gmail.com>
* @author Toni Uebernickel <tuebernickel@gmail.com>
@ -59,7 +59,7 @@ class ModelChoiceList extends ObjectChoiceList
protected $loaded = false;
/**
* Whether to use the identifier for index generation
* Whether to use the identifier for index generation.
*
* @var Boolean
*/
@ -68,7 +68,7 @@ class ModelChoiceList extends ObjectChoiceList
/**
* Constructor.
*
* @see Symfony\Bridge\Propel1\Form\Type\ModelType How to use the preferred choices.
* @see \Symfony\Bridge\Propel1\Form\Type\ModelType How to use the preferred choices.
*
* @param string $class The FQCN of the model class to be loaded.
* @param string $labelPath A property path pointing to the property used for the choice labels.
@ -87,8 +87,8 @@ class ModelChoiceList extends ObjectChoiceList
$queryClass = $this->class . 'Query';
$query = new $queryClass();
$this->identifier = $query->getTableMap()->getPrimaryKeys();
$this->query = $queryObject ?: $query;
$this->identifier = $this->query->getTableMap()->getPrimaryKeys();
$this->loaded = is_array($choices) || $choices instanceof \Traversable;
if ($preferred instanceof ModelCriteria) {
@ -102,7 +102,7 @@ class ModelChoiceList extends ObjectChoiceList
$preferred = array();
}
if (1 === count($this->identifier) && $this->isInteger(current($this->identifier))) {
if (1 === count($this->identifier) && $this->isScalar(current($this->identifier))) {
$this->identifierAsIndex = true;
}
@ -110,7 +110,7 @@ class ModelChoiceList extends ObjectChoiceList
}
/**
* Returns the class name
* Returns the class name of the model.
*
* @return string
*/
@ -120,162 +120,135 @@ class ModelChoiceList extends ObjectChoiceList
}
/**
* Returns the list of model objects
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getChoices()
{
if (!$this->loaded) {
$this->load();
}
$this->load();
return parent::getChoices();
}
/**
* Returns the values for the model objects
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getValues()
{
if (!$this->loaded) {
$this->load();
}
$this->load();
return parent::getValues();
}
/**
* Returns the choice views of the preferred choices as nested array with
* the choice groups as top-level keys.
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getPreferredViews()
{
if (!$this->loaded) {
$this->load();
}
$this->load();
return parent::getPreferredViews();
}
/**
* Returns the choice views of the choices that are not preferred as nested
* array with the choice groups as top-level keys.
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getRemainingViews()
{
if (!$this->loaded) {
$this->load();
}
$this->load();
return parent::getRemainingViews();
}
/**
* Returns the model objects corresponding to the given values.
*
* @param array $values
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getChoicesForValues(array $values)
{
if (empty($values)) {
return array();
}
/**
* This performance optimization reflects a common scenario:
* * A simple select of a model entry.
* * The choice option "expanded" is set to false.
* * The current request is the submission of the selected value.
*
* @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer::reverseTransform
* @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer::reverseTransform
*/
if (!$this->loaded) {
if (1 === count($this->identifier)) {
$filterBy = 'filterBy' . current($this->identifier)->getPhpName();
return (array) $this->query->create()
// The initial query is cloned, so all additional filters are applied correctly.
$query = clone $this->query;
$result = (array) $query
->$filterBy($values)
->find();
}
$this->load();
// Preserve the keys as provided by the values.
$models = array();
foreach ($values as $index => $value) {
foreach ($result as $eachModel) {
if ($value === $this->createValue($eachModel)) {
// Make sure to convert to the right format
$models[$index] = $this->fixChoice($eachModel);
// If all values have been assigned, skip further loops.
unset($values[$index]);
if (0 === count($values)) {
break 2;
}
}
}
}
return $models;
}
}
$this->load();
return parent::getChoicesForValues($values);
}
/**
* Returns the values corresponding to the given model objects.
*
* @param array $models
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getValuesForChoices(array $models)
{
if (!$this->loaded) {
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as values
if (empty($models)) {
return array();
}
// Attention: This optimization does not check choices for existence
if (!$this->loaded) {
/**
* This performance optimization assumes the validation of the respective values will be done by other means.
*
* It correlates with the performance optimization in {@link ModelChoiceList::getChoicesForValues()}
* as it won't load the actual entries from the database.
*
* @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer::transform
* @see \Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer::transform
*/
if (1 === count($this->identifier)) {
$values = array();
foreach ($models as $model) {
foreach ($models as $index => $model) {
if ($model instanceof $this->class) {
// Make sure to convert to the right format
$values[] = $this->fixValue(current($this->getIdentifierValues($model)));
$values[$index] = $this->fixValue(current($this->getIdentifierValues($model)));
}
}
return $values;
}
$this->load();
}
return parent::getValuesForChoices($models);
}
$this->load();
/**
* Returns the indices corresponding to the given models.
*
* @param array $models
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
*/
public function getIndicesForChoices(array $models)
{
$indices = array();
if (!$this->loaded) {
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as indices
// Attention: This optimization does not check choices for existence
if ($this->identifierAsIndex) {
foreach ($models as $model) {
if ($model instanceof $this->class) {
// Make sure to convert to the right format
$indices[] = $this->fixIndex(current($this->getIdentifierValues($model)));
}
}
return $indices;
}
$this->load();
}
$values = array();
$availableValues = $this->getValues();
/*
* Overwriting default implementation.
@ -287,12 +260,61 @@ class ModelChoiceList extends ObjectChoiceList
* The choicelist will retrieve the list of available related models with a different query, resulting in different objects.
*/
$choices = $this->fixChoices($models);
foreach ($this->getChoices() as $i => $choice) {
foreach ($choices as $j => $givenChoice) {
if (null !== $givenChoice && $this->getIdentifierValues($choice) === $this->getIdentifierValues($givenChoice)) {
$indices[] = $i;
unset($choices[$j]);
foreach ($choices as $i => $givenChoice) {
if (null === $givenChoice) {
continue;
}
foreach ($this->getChoices() as $j => $choice) {
if ($this->isEqual($choice, $givenChoice)) {
$values[$i] = $availableValues[$j];
// If all choices have been assigned, skip further loops.
unset($choices[$i]);
if (0 === count($choices)) {
break 2;
}
}
}
}
return $values;
}
/**
* {@inheritdoc}
*/
public function getIndicesForChoices(array $models)
{
if (empty($models)) {
return array();
}
$this->load();
$indices = array();
/*
* Overwriting default implementation.
*
* The two objects may represent the same entry in the database,
* but if they originated from different queries, there are not the same object within the code.
*
* This happens when using m:n relations with either sides model as data_class of the form.
* The choicelist will retrieve the list of available related models with a different query, resulting in different objects.
*/
$choices = $this->fixChoices($models);
foreach ($choices as $i => $givenChoice) {
if (null === $givenChoice) {
continue;
}
foreach ($this->getChoices() as $j => $choice) {
if ($this->isEqual($choice, $givenChoice)) {
$indices[$i] = $j;
// If all choices have been assigned, skip further loops.
unset($choices[$i]);
if (0 === count($choices)) {
break 2;
}
@ -304,28 +326,16 @@ class ModelChoiceList extends ObjectChoiceList
}
/**
* Returns the models corresponding to the given values.
*
* @param array $values
*
* @return array
*
* @see Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
* {@inheritdoc}
*/
public function getIndicesForValues(array $values)
{
if (!$this->loaded) {
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as indices and values
// Attention: This optimization does not check values for existence
if ($this->identifierAsIndex) {
return $this->fixIndices($values);
}
$this->load();
if (empty($values)) {
return array();
}
$this->load();
return parent::getIndicesForValues($values);
}
@ -363,7 +373,7 @@ class ModelChoiceList extends ObjectChoiceList
*/
protected function createValue($model)
{
if (1 === count($this->identifier)) {
if ($this->identifierAsIndex) {
return (string) current($this->getIdentifierValues($model));
}
@ -371,10 +381,16 @@ class ModelChoiceList extends ObjectChoiceList
}
/**
* Loads the list with model objects.
* Loads the complete choice list entries, once.
*
* If data has been loaded the choice list is initialized with the retrieved data.
*/
private function load()
{
if ($this->loaded) {
return;
}
$models = (array) $this->query->find();
$preferred = array();
@ -385,15 +401,15 @@ class ModelChoiceList extends ObjectChoiceList
try {
// The second parameter $labels is ignored by ObjectChoiceList
parent::initialize($models, array(), $preferred);
$this->loaded = true;
} catch (StringCastException $e) {
throw new StringCastException(str_replace('argument $labelPath', 'option "property"', $e->getMessage()), null, $e);
}
$this->loaded = true;
}
/**
* Returns the values of the identifier fields of an model
* Returns the values of the identifier fields of a model.
*
* Propel must know about this model, that is, the model must already
* be persisted or added to the idmodel map before. Otherwise an
@ -407,6 +423,10 @@ class ModelChoiceList extends ObjectChoiceList
*/
private function getIdentifierValues($model)
{
if (!$model instanceof $this->class) {
return array();
}
if ($model instanceof Persistent) {
return array($model->getPrimaryKey());
}
@ -416,7 +436,7 @@ class ModelChoiceList extends ObjectChoiceList
return array($model->getPrimaryKey());
}
if (null === $model) {
if (!method_exists($model, 'getPrimaryKeys')) {
return array();
}
@ -424,14 +444,39 @@ class ModelChoiceList extends ObjectChoiceList
}
/**
* Whether this column in an integer
* Whether this column contains scalar values (to be used as indices).
*
* @param \ColumnMap $column
*
* @return Boolean
*/
private function isInteger(\ColumnMap $column)
private function isScalar(\ColumnMap $column)
{
return $column->getPdoType() === \PDO::PARAM_INT;
return in_array($column->getPdoType(), array(
\PDO::PARAM_BOOL,
\PDO::PARAM_INT,
\PDO::PARAM_STR,
));
}
/**
* Check the given choices for equality.
*
* @param mixed $choice
* @param mixed $givenChoice
*
* @return Boolean
*/
private function isEqual($choice, $givenChoice)
{
if ($choice === $givenChoice) {
return true;
}
if ($this->getIdentifierValues($choice) === $this->getIdentifierValues($givenChoice)) {
return true;
}
return false;
}
}

17
src/Symfony/Bridge/Propel1/Tests/Fixtures/ItemQuery.php
View File

@ -20,12 +20,20 @@ class ItemQuery
'is_active' => \PropelColumnTypes::BOOLEAN,
'enabled' => \PropelColumnTypes::BOOLEAN_EMU,
'updated_at' => \PropelColumnTypes::TIMESTAMP,
'updated_at' => \PropelColumnTypes::TIMESTAMP,
'updated_at' => \PropelColumnTypes::TIMESTAMP,
'updated_at' => \PropelColumnTypes::TIMESTAMP,
);
public static $result = array();
public function find()
{
return self::$result;
}
public function filterById($id)
{
return $this;
}
public function getTableMap()
{
// Allows to define methods in this class
@ -37,6 +45,7 @@ class ItemQuery
{
$cm = new \ColumnMap('id', new \TableMap());
$cm->setType('INTEGER');
$cm->setPhpName('Id');
return array('id' => $cm);
}

128
src/Symfony/Bridge/Propel1/Tests/Form/ChoiceList/CompatModelChoiceListTest.php Normal file
View File

@ -0,0 +1,128 @@
<?php
namespace Symfony\Bridge\Propel1\Tests\Form\ChoiceList;
use Symfony\Bridge\Propel1\Form\ChoiceList\ModelChoiceList;
use Symfony\Bridge\Propel1\Tests\Fixtures\Item;
use Symfony\Bridge\Propel1\Tests\Fixtures\ItemQuery;
use Symfony\Component\Form\Tests\Extension\Core\ChoiceList\AbstractChoiceListTest;
class CompatModelChoiceListTest extends AbstractChoiceListTest
{
const ITEM_CLASS = '\Symfony\Bridge\Propel1\Tests\Fixtures\Item';
/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Symfony\Bridge\Propel1\Tests\Fixtures\ItemQuery
*/
protected $query;
protected $item1;
protected $item2;
protected $item3;
protected $item4;
public static function setUpBeforeClass()
{
if (!class_exists('\Propel')) {
self::markTestSkipped('Propel is not available.');
}
if (!class_exists('Symfony\Component\Form\Form')) {
self::markTestSkipped('The "Form" component is not available');
}
if (!class_exists('Symfony\Component\PropertyAccess\PropertyAccessor')) {
self::markTestSkipped('The "PropertyAccessor" component is not available');
}
parent::setUpBeforeClass();
}
public function testGetChoicesForValues()
{
$this->query
->expects($this->once())
->method('filterById')
->with(array(1, 2))
->will($this->returnSelf())
;
ItemQuery::$result = array(
$this->item2,
$this->item1,
);
parent::testGetChoicesForValues();
}
protected function setUp()
{
$this->query = $this->getMock('Symfony\Bridge\Propel1\Tests\Fixtures\ItemQuery', array(
'filterById',
), array(), '', true, true, true, false, true);
$this->createItems();
ItemQuery::$result = array(
$this->item1,
$this->item2,
$this->item3,
$this->item4,
);
parent::setUp();
}
protected function createItems()
{
$this->item1 = new Item(1, 'Foo');
$this->item2 = new Item(2, 'Bar');
$this->item3 = new Item(3, 'Baz');
$this->item4 = new Item(4, 'Cuz');
}
protected function createChoiceList()
{
return new ModelChoiceList(self::ITEM_CLASS, 'value', null, $this->query);
}
protected function getChoices()
{
return array(
1 => $this->item1,
2 => $this->item2,
3 => $this->item3,
4 => $this->item4,
);
}
protected function getLabels()
{
return array(
1 => 'Foo',
2 => 'Bar',
3 => 'Baz',
4 => 'Cuz',
);
}
protected function getValues()
{
return array(
1 => '1',
2 => '2',
3 => '3',
4 => '4',
);
}
protected function getIndices()
{
return array(
1,
2,
3,
4,
);
}
}

44
src/Symfony/Bridge/Propel1/Tests/Form/ChoiceList/ModelChoiceListTest.php
View File

@ -12,26 +12,34 @@
namespace Symfony\Bridge\Propel1\Tests\Form\ChoiceList;
use Symfony\Bridge\Propel1\Form\ChoiceList\ModelChoiceList;
use Symfony\Component\Form\Extension\Core\View\ChoiceView;
use Symfony\Bridge\Propel1\Tests\Fixtures\Item;
use Symfony\Bridge\Propel1\Tests\Fixtures\ReadOnlyItem;
use Symfony\Bridge\Propel1\Tests\Propel1TestCase;
use Symfony\Bridge\Propel1\Tests\Fixtures\Item;
use Symfony\Bridge\Propel1\Tests\Fixtures\ItemQuery;
use Symfony\Bridge\Propel1\Tests\Fixtures\ReadOnlyItem;
use Symfony\Component\Form\Extension\Core\View\ChoiceView;
class ModelChoiceListTest extends Propel1TestCase
{
const ITEM_CLASS = '\Symfony\Bridge\Propel1\Tests\Fixtures\Item';
protected function setUp()
public static function setUpBeforeClass()
{
parent::setUpBeforeClass();
if (!class_exists('Symfony\Component\Form\Form')) {
$this->markTestSkipped('The "Form" component is not available');
self::markTestSkipped('The "Form" component is not available');
}
if (!class_exists('Symfony\Component\PropertyAccess\PropertyAccessor')) {
$this->markTestSkipped('The "PropertyAccessor" component is not available');
self::markTestSkipped('The "PropertyAccessor" component is not available');
}
}
protected function setUp()
{
ItemQuery::$result = array();
}
public function testEmptyChoicesReturnsEmpty()
{
$choiceList = new ModelChoiceList(
@ -173,6 +181,11 @@ class ModelChoiceListTest extends Propel1TestCase
$item1 = new Item(1, 'Foo');
$item2 = new Item(2, 'Bar');
ItemQuery::$result = array(
$item1,
$item2,
);
$choiceList = new ModelChoiceList(
self::ITEM_CLASS,
'value',
@ -189,6 +202,11 @@ class ModelChoiceListTest extends Propel1TestCase
public function testDifferentEqualObjectsAreChoosen()
{
$item = new Item(1, 'Foo');
ItemQuery::$result = array(
$item,
);
$choiceList = new ModelChoiceList(
self::ITEM_CLASS,
'value',
@ -198,6 +216,7 @@ class ModelChoiceListTest extends Propel1TestCase
$choosenItem = new Item(1, 'Foo');
$this->assertEquals(array(1), $choiceList->getIndicesForChoices(array($choosenItem)));
$this->assertEquals(array('1'), $choiceList->getValuesForChoices(array($choosenItem)));
}
public function testGetIndicesForNullChoices()
@ -211,4 +230,17 @@ class ModelChoiceListTest extends Propel1TestCase
$this->assertEquals(array(), $choiceList->getIndicesForChoices(array(null)));
}
public function testDontAllowInvalidChoiceValues()
{
$item = new Item(1, 'Foo');
$choiceList = new ModelChoiceList(
self::ITEM_CLASS,
'value',
array($item)
);
$this->assertEquals(array(), $choiceList->getValuesForChoices(array(new Item(2, 'Bar'))));
$this->assertEquals(array(), $choiceList->getChoicesForValues(array(2)));
}
}

4
src/Symfony/Bridge/Propel1/Tests/Propel1TestCase.php
View File

@ -13,10 +13,10 @@ namespace Symfony\Bridge\Propel1\Tests;
abstract class Propel1TestCase extends \PHPUnit_Framework_TestCase
{
protected function setUp()
public static function setUpBeforeClass()
{
if (!class_exists('\Propel')) {
$this->markTestSkipped('Propel is not available.');
self::markTestSkipped('Propel is not available.');
}
}
}