Skip to content

Commit c09ad47

Browse files
HeahDudeHeahDude
committed
[Form] Added support for caching choice lists based on options
1 parent dab6732 commit c09ad47

25 files changed

+962
-102
lines changed

‎src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php‎

Lines changed: 42 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
2222
use Symfony\Bridge\Doctrine\Form\EventListener\MergeDoctrineCollectionListener;
2323
use Symfony\Component\Form\AbstractType;
24+
use Symfony\Component\Form\ChoiceList\ChoiceList;
2425
use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
2526
use Symfony\Component\Form\Exception\RuntimeException;
2627
use Symfony\Component\Form\FormBuilderInterface;
@@ -41,9 +42,9 @@ abstract class DoctrineType extends AbstractType implements ResetInterface
4142
private $idReaders = [];
4243

4344
/**
44-
* @var DoctrineChoiceLoader[]
45+
* @var EntityLoaderInterface[]
4546
*/
46-
private $choiceLoaders = [];
47+
private $entityLoaders = [];
4748

4849
/**
4950
* Creates the label for a choice.
@@ -114,44 +115,26 @@ public function configureOptions(OptionsResolver $resolver)
114115
$choiceLoader = function (Options $options) {
115116
// Unless the choices are given explicitly, load them on demand
116117
if (null === $options['choices']) {
117-
$hash = null;
118-
$qbParts = null;
118+
// If there is no QueryBuilder we can safely cache
119+
$vary = [$options['em'], $options['class']];
119120

120-
// If there is no QueryBuilder we can safely cache DoctrineChoiceLoader,
121121
// also if concrete Type can return important QueryBuilder parts to generate
122122
// hash key we go for it as well
123-
if (!$options['query_builder'] || null !== $qbParts = $this->getQueryBuilderPartsForCachingHash($options['query_builder'])) {
124-
$hash = CachingFactoryDecorator::generateHash([
125-
$options['em'],
126-
$options['class'],
127-
$qbParts,
128-
]);
129-
130-
if (isset($this->choiceLoaders[$hash])) {
131-
return $this->choiceLoaders[$hash];
132-
}
123+
if ($options['query_builder'] && null !== $qbParts = $this->getQueryBuilderPartsForCachingHash($options['query_builder'])) {
124+
$vary[] = $qbParts;
133125
}
134126

135-
if (null !== $options['query_builder']) {
136-
$entityLoader = $this->getLoader($options['em'], $options['query_builder'], $options['class']);
137-
} else {
138-
$queryBuilder = $options['em']->getRepository($options['class'])->createQueryBuilder('e');
139-
$entityLoader = $this->getLoader($options['em'], $queryBuilder, $options['class']);
140-
}
141-
142-
$doctrineChoiceLoader = new DoctrineChoiceLoader(
127+
return ChoiceList::loader($this, new DoctrineChoiceLoader(
143128
$options['em'],
144129
$options['class'],
145130
$options['id_reader'],
146-
$entityLoader,
147-
false
148-
);
149-
150-
if (null !== $hash) {
151-
$this->choiceLoaders[$hash] = $doctrineChoiceLoader;
152-
}
153-
154-
return $doctrineChoiceLoader;
131+
$this->getCachedEntityLoader(
132+
$options['em'],
133+
$options['query_builder'] ?? $options['em']->getRepository($options['class'])->createQueryBuilder('e'),
134+
$options['class'],
135+
$vary
136+
)
137+
), $vary);
155138
}
156139

157140
return null;
@@ -162,7 +145,7 @@ public function configureOptions(OptionsResolver $resolver)
162145
// field name. We can only use numeric IDs as names, as we cannot
163146
// guarantee that a non-numeric ID contains a valid form name
164147
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isIntId()) {
165-
return [__CLASS__, 'createChoiceName'];
148+
return ChoiceList::fieldName($this, [__CLASS__, 'createChoiceName'], $options['id_reader']);
166149
}
167150

168151
// Otherwise, an incrementing integer is used as name automatically
@@ -176,7 +159,7 @@ public function configureOptions(OptionsResolver $resolver)
176159
$choiceValue = function (Options $options) {
177160
// If the entity has a single-column ID, use that ID as value
178161
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isSingleId()) {
179-
return [$options['id_reader'], 'getIdValue'];
162+
return ChoiceList::value($this, [$options['id_reader'], 'getIdValue'], $options['id_reader']);
180163
}
181164

182165
// Otherwise, an incrementing integer is used as value automatically
@@ -215,35 +198,21 @@ public function configureOptions(OptionsResolver $resolver)
215198
// Set the "id_reader" option via the normalizer. This option is not
216199
// supposed to be set by the user.
217200
$idReaderNormalizer = function (Options $options) {
218-
$hash = CachingFactoryDecorator::generateHash([
219-
$options['em'],
220-
$options['class'],
221-
]);
222-
223201
// The ID reader is a utility that is needed to read the object IDs
224202
// when generating the field values. The callback generating the
225203
// field values has no access to the object manager or the class
226204
// of the field, so we store that information in the reader.
227205
// The reader is cached so that two choice lists for the same class
228206
// (and hence with the same reader) can successfully be cached.
229-
if (!isset($this->idReaders[$hash])) {
230-
$classMetadata = $options['em']->getClassMetadata($options['class']);
231-
$this->idReaders[$hash] = new IdReader($options['em'], $classMetadata);
232-
}
233-
234-
if ($this->idReaders[$hash]->isSingleId()) {
235-
return $this->idReaders[$hash];
236-
}
237-
238-
return null;
207+
return $this->getCachedIdReader($options['em'], $options['class']);
239208
};
240209

241210
$resolver->setDefaults([
242211
'em' => null,
243212
'query_builder' => null,
244213
'choices' => null,
245214
'choice_loader' => $choiceLoader,
246-
'choice_label' => [__CLASS__, 'createChoiceLabel'],
215+
'choice_label' => ChoiceList::label($this, [__CLASS__, 'createChoiceLabel']),
247216
'choice_name' => $choiceName,
248217
'choice_value' => $choiceValue,
249218
'id_reader' => null, // internal
@@ -273,6 +242,28 @@ public function getParent()
273242

274243
public function reset()
275244
{
276-
$this->choiceLoaders = [];
245+
$this->idReaders = [];
246+
$this->entityLoaders = [];
247+
}
248+
249+
private function getCachedIdReader(ObjectManager $manager, string $class): ?IdReader
250+
{
251+
$hash = CachingFactoryDecorator::generateHash([$manager, $class]);
252+
253+
if (isset($this->idReaders[$hash])) {
254+
return $this->idReaders[$hash];
255+
}
256+
257+
$idReader = new IdReader($manager, $manager->getClassMetadata($class));
258+
259+
// don't cache the instance for composite ids that cannot be optimized
260+
return $this->idReaders[$hash] = $idReader->isSingleId() ? $idReader : null;
261+
}
262+
263+
private function getCachedEntityLoader(ObjectManager $manager, QueryBuilder $queryBuilder, string $class, array $vary): EntityLoaderInterface
264+
{
265+
$hash = CachingFactoryDecorator::generateHash($vary);
266+
267+
return $this->entityLoaders[$hash] ?? ($this->entityLoaders[$hash] = $this->getLoader($manager, $queryBuilder, $class));
277268
}
278269
}

‎src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,11 @@ public function testSubmitSingleNonExpandedStringCastableIdentifier()
581581
$this->assertSame('2', $field->getViewData());
582582
}
583583

584+
/**
585+
* @group legacy
586+
*
587+
* @expectedDeprecation Implicitly cached choice lists are deprecated since 5.1. Use one of "Symfony\Component\Form\ChoiceList\ChoiceList" methods instead to define choice options as of 6.0.
588+
*/
584589
public function testSubmitSingleStringCastableIdentifierExpanded()
585590
{
586591
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
@@ -662,6 +667,11 @@ public function testSubmitMultipleNonExpandedStringCastableIdentifier()
662667
$this->assertSame(['1', '3'], $field->getViewData());
663668
}
664669

670+
/**
671+
* @group legacy
672+
*
673+
* @expectedDeprecation Implicitly cached choice lists are deprecated since 5.1. Use one of "Symfony\Component\Form\ChoiceList\ChoiceList" methods instead to define choice options as of 6.0.
674+
*/
665675
public function testSubmitMultipleStringCastableIdentifierExpanded()
666676
{
667677
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
@@ -716,6 +726,11 @@ public function testOverrideChoices()
716726
$this->assertSame('2', $field->getViewData());
717727
}
718728

729+
/**
730+
* @group legacy
731+
*
732+
* @expectedDeprecation Implicitly cached choice lists are deprecated since 5.1. No list will be cached in 6.0, use "Symfony\Component\Form\ChoiceList\Factory\Cache\ChoiceValue" instead to define the choice value.
733+
*/
719734
public function testOverrideChoicesValues()
720735
{
721736
$entity1 = new SingleIntIdEntity(1, 'Foo');
@@ -738,6 +753,11 @@ public function testOverrideChoicesValues()
738753
$this->assertSame('Bar', $field->getViewData());
739754
}
740755

756+
/**
757+
* @group legacy
758+
*
759+
* @expectedDeprecation Implicitly cached choice lists are deprecated since 5.1. No list will be cached in 6.0, use "Symfony\Component\Form\ChoiceList\Factory\Cache\ChoiceValue" instead to define the choice value.
760+
*/
741761
public function testOverrideChoicesValuesWithCallable()
742762
{
743763
$entity1 = new GroupableEntity(1, 'Foo', 'BazGroup');
@@ -1155,13 +1175,13 @@ public function testLoaderCaching()
11551175
'property3' => 2,
11561176
]);
11571177

1158-
$choiceLoader1 = $form->get('property1')->getConfig()->getOption('choice_loader');
1159-
$choiceLoader2 = $form->get('property2')->getConfig()->getOption('choice_loader');
1160-
$choiceLoader3 = $form->get('property3')->getConfig()->getOption('choice_loader');
1178+
$choiceList1 = $form->get('property1')->getConfig()->getAttribute('choice_list');
1179+
$choiceList2 = $form->get('property2')->getConfig()->getAttribute('choice_list');
1180+
$choiceList3 = $form->get('property3')->getConfig()->getAttribute('choice_list');
11611181

1162-
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface', $choiceLoader1);
1163-
$this->assertSame($choiceLoader1, $choiceLoader2);
1164-
$this->assertSame($choiceLoader1, $choiceLoader3);
1182+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\LazyChoiceList', $choiceList1);
1183+
$this->assertSame($choiceList1, $choiceList2);
1184+
$this->assertSame($choiceList1, $choiceList3);
11651185
}
11661186

11671187
public function testLoaderCachingWithParameters()
@@ -1215,13 +1235,13 @@ public function testLoaderCachingWithParameters()
12151235
'property3' => 2,
12161236
]);
12171237

1218-
$choiceLoader1 = $form->get('property1')->getConfig()->getOption('choice_loader');
1219-
$choiceLoader2 = $form->get('property2')->getConfig()->getOption('choice_loader');
1220-
$choiceLoader3 = $form->get('property3')->getConfig()->getOption('choice_loader');
1238+
$choiceList1 = $form->get('property1')->getConfig()->getAttribute('choice_list');
1239+
$choiceList2 = $form->get('property2')->getConfig()->getAttribute('choice_list');
1240+
$choiceList3 = $form->get('property3')->getConfig()->getAttribute('choice_list');
12211241

1222-
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface', $choiceLoader1);
1223-
$this->assertSame($choiceLoader1, $choiceLoader2);
1224-
$this->assertSame($choiceLoader1, $choiceLoader3);
1242+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\LazyChoiceList', $choiceList1);
1243+
$this->assertSame($choiceList1, $choiceList2);
1244+
$this->assertSame($choiceList1, $choiceList3);
12251245
}
12261246

12271247
protected function createRegistryMock($name, $em)

‎src/Symfony/Bridge/Doctrine/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"symfony/stopwatch": "^4.4|^5.0",
2828
"symfony/config": "^4.4|^5.0",
2929
"symfony/dependency-injection": "^4.4|^5.0",
30-
"symfony/form": "^5.0",
30+
"symfony/form": "^5.1",
3131
"symfony/http-kernel": "^5.0",
3232
"symfony/messenger": "^4.4|^5.0",
3333
"symfony/property-access": "^4.4|^5.0",

‎src/Symfony/Component/Form/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
5.1.0
5+
-----
6+
7+
* added a `ChoiceList` facade to leverage explicit choice list caching based on callback options or specific loaders
8+
49
5.0.0
510
-----
611

0 commit comments

Comments
 (0)