[Form] Added an AbstractChoiceLoader to simplify implementations and handle global optimizations
This commit is contained in:
parent
9bfa25869a
commit
1394df2dea
@ -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);
|
||||
}
|
||||
}
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -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(
|
||||
|
@ -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",
|
||||
|
@ -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
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
@ -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)();
|
||||
}
|
||||
}
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user