Skip to content

Commit d5754ff

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

File tree

2 files changed

+54
-63
lines changed

2 files changed

+54
-63
lines changed

‎Form/Type/DoctrineType.php‎

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

4243
/**
43-
* @var DoctrineChoiceLoader[]
44+
* @var EntityLoaderInterface[]
4445
*/
45-
private $choiceLoaders = [];
46+
private $entityLoaders = [];
4647

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

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

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

157141
return null;
@@ -162,7 +146,7 @@ public function configureOptions(OptionsResolver $resolver)
162146
// field name. We can only use numeric IDs as names, as we cannot
163147
// guarantee that a non-numeric ID contains a valid form name
164148
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isIntId()) {
165-
return [__CLASS__, 'createChoiceName'];
149+
return ChoiceList::fieldName($this, [__CLASS__, 'createChoiceName']);
166150
}
167151

168152
// Otherwise, an incrementing integer is used as name automatically
@@ -176,7 +160,7 @@ public function configureOptions(OptionsResolver $resolver)
176160
$choiceValue = function (Options $options) {
177161
// If the entity has a single-column ID, use that ID as value
178162
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isSingleId()) {
179-
return [$options['id_reader'], 'getIdValue'];
163+
return ChoiceList::value($this, [$options['id_reader'], 'getIdValue'], $options['id_reader']);
180164
}
181165

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

240210
$resolver->setDefaults([
241211
'em' => null,
242212
'query_builder' => null,
243213
'choices' => null,
244214
'choice_loader' => $choiceLoader,
245-
'choice_label' => [__CLASS__, 'createChoiceLabel'],
215+
'choice_label' => ChoiceList::label($this, [__CLASS__, 'createChoiceLabel']),
246216
'choice_name' => $choiceName,
247217
'choice_value' => $choiceValue,
248218
'id_reader' => null, // internal
@@ -274,6 +244,27 @@ public function getParent()
274244

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

‎Tests/Form/Type/EntityTypeTest.php‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,13 +1205,13 @@ public function testLoaderCaching()
12051205
'property3' => 2,
12061206
]);
12071207

1208-
$choiceLoader1 = $form->get('property1')->getConfig()->getOption('choice_loader');
1209-
$choiceLoader2 = $form->get('property2')->getConfig()->getOption('choice_loader');
1210-
$choiceLoader3 = $form->get('property3')->getConfig()->getOption('choice_loader');
1208+
$choiceList1 = $form->get('property1')->getConfig()->getAttribute('choice_list');
1209+
$choiceList2 = $form->get('property2')->getConfig()->getAttribute('choice_list');
1210+
$choiceList3 = $form->get('property3')->getConfig()->getAttribute('choice_list');
12111211

1212-
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface', $choiceLoader1);
1213-
$this->assertSame($choiceLoader1, $choiceLoader2);
1214-
$this->assertSame($choiceLoader1, $choiceLoader3);
1212+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\LazyChoiceList', $choiceList1);
1213+
$this->assertSame($choiceList1, $choiceList2);
1214+
$this->assertSame($choiceList1, $choiceList3);
12151215
}
12161216

12171217
public function testLoaderCachingWithParameters()
@@ -1265,13 +1265,13 @@ public function testLoaderCachingWithParameters()
12651265
'property3' => 2,
12661266
]);
12671267

1268-
$choiceLoader1 = $form->get('property1')->getConfig()->getOption('choice_loader');
1269-
$choiceLoader2 = $form->get('property2')->getConfig()->getOption('choice_loader');
1270-
$choiceLoader3 = $form->get('property3')->getConfig()->getOption('choice_loader');
1268+
$choiceList1 = $form->get('property1')->getConfig()->getAttribute('choice_list');
1269+
$choiceList2 = $form->get('property2')->getConfig()->getAttribute('choice_list');
1270+
$choiceList3 = $form->get('property3')->getConfig()->getAttribute('choice_list');
12711271

1272-
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface', $choiceLoader1);
1273-
$this->assertSame($choiceLoader1, $choiceLoader2);
1274-
$this->assertSame($choiceLoader1, $choiceLoader3);
1272+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\LazyChoiceList', $choiceList1);
1273+
$this->assertSame($choiceList1, $choiceList2);
1274+
$this->assertSame($choiceList1, $choiceList3);
12751275
}
12761276

12771277
protected function createRegistryMock($name, $em)

0 commit comments

Comments
 (0)