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

[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations

This commit is contained in:
Jules Pietri 2019-04-07 15:11:26 +02:00
parent 9bfa25869a
commit 1394df2dea
8 changed files with 174 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
View File

@ -12,27 +12,20 @@
namespace Symfony\Bridge\Doctrine\Form\ChoiceList;
use Doctrine\Persistence\ObjectManager;
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
use Symfony\Component\Form\ChoiceList\Loader\AbstractChoiceLoader;
/**
* Loads choices using a Doctrine object manager.
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class DoctrineChoiceLoader implements ChoiceLoaderInterface
class DoctrineChoiceLoader extends AbstractChoiceLoader
{
private $manager;
private $class;
private $idReader;
private $objectLoader;
/**
* @var ChoiceListInterface
*/
private $choiceList;
/**
* Creates a new choice loader.
*
@ -59,81 +52,57 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
/**
* {@inheritdoc}
*/
public function loadChoiceList(callable $value = null)
protected function loadChoices(): iterable
{
if ($this->choiceList) {
return $this->choiceList;
}
$objects = $this->objectLoader
return $this->objectLoader
? $this->objectLoader->getEntities()
: $this->manager->getRepository($this->class)->findAll();
return $this->choiceList = new ArrayChoiceList($objects, $value);
}
/**
* {@inheritdoc}
* @internal to be remove in Symfony 6
*/
public function loadValuesForChoices(array $choices, callable $value = null)
protected function doLoadValuesForChoices(array $choices): array
{
// Performance optimization
if (empty($choices)) {
return [];
}
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as values
$optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
// Attention: This optimization does not check choices for existence
if ($optimize && !$this->choiceList) {
$values = [];
if ($this->idReader) {
trigger_deprecation('symfony/doctrine-bridge', '5.1', 'Not defining explicitly the IdReader as value callback when query can be optimized is deprecated. Don\'t pass the IdReader to "%s" or define the "choice_value" option instead.', __CLASS__);
// Maintain order and indices of the given objects
$values = [];
foreach ($choices as $i => $object) {
if ($object instanceof $this->class) {
// Make sure to convert to the right format
$values[$i] = (string) $this->idReader->getIdValue($object);
$values[$i] = $this->idReader->getIdValue($object);
}
}
return $values;
}
return $this->loadChoiceList($value)->getValuesForChoices($choices);
return parent::doLoadValuesForChoices($choices);
}
/**
* {@inheritdoc}
*/
public function loadChoicesForValues(array $values, callable $value = null)
protected function doLoadChoicesForValues(array $values, ?callable $value): array
{
// Performance optimization
// Also prevents the generation of "WHERE id IN ()" queries through the
// object loader. At least with MySQL and on the development machine
// this was tested on, no exception was thrown for such invalid
// statements, consequently no test fails when this code is removed.
// https://github.com/symfony/symfony/pull/8981#issuecomment-24230557
if (empty($values)) {
return [];
$legacy = $this->idReader && null === $value;
if ($legacy) {
trigger_deprecation('symfony/doctrine-bridge', '5.1', 'Not defining explicitly the IdReader as value callback when query can be optimized is deprecated. Don\'t pass the IdReader to "%s" or define the "choice_value" option instead.', __CLASS__);
}
// Optimize performance in case we have an object loader and
// a single-field identifier
$optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
if ($optimize && !$this->choiceList && $this->objectLoader) {
$unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
$objectsById = [];
if (($legacy || \is_array($value) && $this->idReader === $value[0]) && $this->objectLoader) {
$objects = [];
$objectsById = [];
// Maintain order and indices from the given $values
// An alternative approach to the following loop is to add the
// "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) {
$objectsById[(string) $this->idReader->getIdValue($object)] = $object;
foreach ($this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values) as $object) {
$objectsById[$this->idReader->getIdValue($object)] = $object;
}
foreach ($values as $i => $id) {
@ -145,6 +114,6 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
return $objects;
}
return $this->loadChoiceList($value)->getChoicesForValues($values);
return parent::doLoadChoicesForValues($values, $value);
}
}

6
src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php
View File

@ -84,12 +84,12 @@ class IdReader
*
* This method assumes that the object has a single-column ID.
*
* @return mixed The ID value
* @return string The ID value
*/
public function getIdValue(object $object = null)
{
if (!$object) {
return null;
return '';
}
if (!$this->om->contains($object)) {
@ -104,7 +104,7 @@ class IdReader
$idValue = $this->associationIdReader->getIdValue($idValue);
}
return $idValue;
return (string) $idValue;
}
/**

59
src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
View File

@ -100,6 +100,10 @@ class DoctrineChoiceLoaderTest extends TestCase
->method('getClassMetadata')
->with($this->class)
->willReturn(new ClassMetadata($this->class));
$this->repository->expects($this->any())
->method('findAll')
->willReturn([$this->obj1, $this->obj2, $this->obj3])
;
}
public function testLoadChoiceList()
@ -186,6 +190,11 @@ class DoctrineChoiceLoaderTest extends TestCase
$this->assertSame([], $loader->loadValuesForChoices([]));
}
/**
* @group legacy
*
* @expectedDeprecation Since symfony/doctrine-bridge 5.1: Not defining explicitly the IdReader as value callback when query can be optimized is deprecated. Don't pass the IdReader to "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader" or define the "choice_value" option instead.
*/
public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
{
$loader = new DoctrineChoiceLoader(
@ -205,7 +214,7 @@ class DoctrineChoiceLoaderTest extends TestCase
$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2]));
}
public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
public function testLoadValuesForChoicesDoesNotLoadIfSingleIntIdAndValueGiven()
{
$loader = new DoctrineChoiceLoader(
$this->om,
@ -216,7 +225,7 @@ class DoctrineChoiceLoaderTest extends TestCase
$choices = [$this->obj1, $this->obj2, $this->obj3];
$value = function (\stdClass $object) { return $object->name; };
$this->repository->expects($this->once())
$this->repository->expects($this->never())
->method('findAll')
->willReturn($choices);
@ -254,8 +263,7 @@ class DoctrineChoiceLoaderTest extends TestCase
{
$loader = new DoctrineChoiceLoader(
$this->om,
$this->class,
$this->idReader
$this->class
);
$choices = [$this->obj1, $this->obj2, $this->obj3];
@ -285,7 +293,12 @@ class DoctrineChoiceLoaderTest extends TestCase
$this->assertSame([], $loader->loadChoicesForValues([]));
}
public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
/**
* @group legacy
*
* @expectedDeprecation Not defining explicitly the IdReader as value callback when query can be optimized has been deprecated in 5.1. Don't pass the IdReader to "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader" or define the choice_value instead.
*/
public function legacyTestLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
{
$loader = new DoctrineChoiceLoader(
$this->om,
@ -321,6 +334,42 @@ class DoctrineChoiceLoaderTest extends TestCase
));
}
public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
{
$loader = new DoctrineChoiceLoader(
$this->om,
$this->class,
$this->idReader,
$this->objectLoader
);
$choices = [$this->obj2, $this->obj3];
$this->idReader->expects($this->any())
->method('getIdField')
->willReturn('idField');
$this->repository->expects($this->never())
->method('findAll');
$this->objectLoader->expects($this->once())
->method('getEntitiesByIds')
->with('idField', [4 => '3', 7 => '2'])
->willReturn($choices);
$this->idReader->expects($this->any())
->method('getIdValue')
->willReturnMap([
[$this->obj2, '2'],
[$this->obj3, '3'],
]);
$this->assertSame(
[4 => $this->obj3, 7 => $this->obj2],
$loader->loadChoicesForValues([4 => '3', 7 => '2'], [$this->idReader, 'getIdValue']
));
}
public function testLoadChoicesForValuesLoadsAllIfSingleIntIdAndValueGiven()
{
$loader = new DoctrineChoiceLoader(

4
src/Symfony/Bridge/Doctrine/composer.json
View File

@ -27,7 +27,7 @@
"symfony/stopwatch": "^4.4|^5.0",
"symfony/config": "^4.4|^5.0",
"symfony/dependency-injection": "^4.4|^5.0",
"symfony/form": "^5.0",
"symfony/form": "^5.1",
"symfony/http-kernel": "^5.0",
"symfony/messenger": "^4.4|^5.0",
"symfony/property-access": "^4.4|^5.0",
@ -49,7 +49,7 @@
"conflict": {
"phpunit/phpunit": "<5.4.3",
"symfony/dependency-injection": "<4.4",
"symfony/form": "<5",
"symfony/form": "<5.1",
"symfony/http-kernel": "<5",
"symfony/messenger": "<4.4",
"symfony/property-info": "<5",

1
src/Symfony/Component/Form/CHANGELOG.md
View File

@ -4,6 +4,7 @@ CHANGELOG
5.1.0
-----
* Added an `AbstractChoiceLoader` to simplify implementations and handle global optimizations
* The `view_timezone` option defaults to the `model_timezone` if no `reference_date` is configured.
* Added default `inputmode` attribute to Search, Email and Tel form types.
* Implementing the `FormConfigInterface` without implementing the `getIsEmptyCallback()` method

86
src/Symfony/Component/Form/ChoiceList/Loader/AbstractChoiceLoader.php Normal file
View File

@ -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\Component\Form\ChoiceList\Loader;
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
/**
* @author Jules Pietri <jules@heahprod.com>
*/
abstract class AbstractChoiceLoader implements ChoiceLoaderInterface
{
/**
* The loaded choice list.
*
* @var ArrayChoiceList
*/
private $choiceList;
/**
* @final
*
* {@inheritdoc}
*/
public function loadChoiceList(callable $value = null)
{
return $this->choiceList ?? ($this->choiceList = new ArrayChoiceList($this->loadChoices(), $value));
}
/**
* {@inheritdoc}
*/
public function loadChoicesForValues(array $values, callable $value = null)
{
if (!$values) {
return [];
}
if ($this->choiceList) {
return $this->choiceList->getChoicesForValues($values);
}
return $this->doLoadChoicesForValues($values, $value);
}
/**
* {@inheritdoc}
*/
public function loadValuesForChoices(array $choices, callable $value = null)
{
if (!$choices) {
return [];
}
if ($value) {
// if a value callback exists, use it
return array_map($value, $choices);
}
if ($this->choiceList) {
return $this->choiceList->getValuesForChoices($choices);
}
return $this->doLoadValuesForChoices($choices);
}
abstract protected function loadChoices(): iterable;
protected function doLoadChoicesForValues(array $values, ?callable $value): array
{
return $this->loadChoiceList($value)->getChoicesForValues($values);
}
protected function doLoadValuesForChoices(array $choices): array
{
return $this->loadChoiceList()->getValuesForChoices($choices);
}
}

52
src/Symfony/Component/Form/ChoiceList/Loader/CallbackChoiceLoader.php
View File

@ -11,67 +11,25 @@
namespace Symfony\Component\Form\ChoiceList\Loader;
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
/**
* Loads an {@link ArrayChoiceList} instance from a callable returning an array of choices.
* Loads an {@link ArrayChoiceList} instance from a callable returning iterable choices.
*
* @author Jules Pietri <jules@heahprod.com>
*/
class CallbackChoiceLoader implements ChoiceLoaderInterface
class CallbackChoiceLoader extends AbstractChoiceLoader
{
private $callback;
/**
* The loaded choice list.
*
* @var ArrayChoiceList
*/
private $choiceList;
/**
* @param callable $callback The callable returning an array of choices
* @param callable $callback The callable returning iterable choices
*/
public function __construct(callable $callback)
{
$this->callback = $callback;
}
/**
* {@inheritdoc}
*/
public function loadChoiceList(callable $value = null)
protected function loadChoices(): iterable
{
if (null !== $this->choiceList) {
return $this->choiceList;
}
return $this->choiceList = new ArrayChoiceList(($this->callback)(), $value);
}
/**
* {@inheritdoc}
*/
public function loadChoicesForValues(array $values, callable $value = null)
{
// Optimize
if (empty($values)) {
return [];
}
return $this->loadChoiceList($value)->getChoicesForValues($values);
}
/**
* {@inheritdoc}
*/
public function loadValuesForChoices(array $choices, callable $value = null)
{
// Optimize
if (empty($choices)) {
return [];
}
return $this->loadChoiceList($value)->getValuesForChoices($choices);
return ($this->callback)();
}
}

14
src/Symfony/Component/Form/ChoiceList/Loader/IntlCallbackChoiceLoader.php
View File

@ -24,13 +24,7 @@ class IntlCallbackChoiceLoader extends CallbackChoiceLoader
*/
public function loadChoicesForValues(array $values, callable $value = null)
{
// Optimize
$values = array_filter($values);
if (empty($values)) {
return [];
}
return $this->loadChoiceList($value)->getChoicesForValues($values);
return parent::loadChoicesForValues(array_filter($values), $value);
}
/**
@ -38,17 +32,13 @@ class IntlCallbackChoiceLoader extends CallbackChoiceLoader
*/
public function loadValuesForChoices(array $choices, callable $value = null)
{
// Optimize
$choices = array_filter($choices);
if (empty($choices)) {
return [];
}
// If no callable is set, choices are the same as values
if (null === $value) {
return $choices;
}
return $this->loadChoiceList($value)->getValuesForChoices($choices);
return parent::loadValuesForChoices($choices, $value);
}
}