Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit acf39c5

Browse filesBrowse files
committed
[DoctrineBridge] DoctrineType now respects the "query_builder" option when caching the choice loader
1 parent fc61844 commit acf39c5
Copy full SHA for acf39c5

File tree

4 files changed

+58
-39
lines changed
Filter options

4 files changed

+58
-39
lines changed

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public function __construct($queryBuilder, $manager = null, $class = null)
5151
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
5252
}
5353

54+
// This block is not executed anymore since Symfony 2.7. The query
55+
// builder closure is already invoked in DoctrineType
5456
if ($queryBuilder instanceof \Closure) {
5557
if (!$manager instanceof EntityManager) {
5658
throw new UnexpectedTypeException($manager, 'Doctrine\ORM\EntityManager');

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
+38-20Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Doctrine\Common\Persistence\ManagerRegistry;
1515
use Doctrine\Common\Persistence\ObjectManager;
16+
use Doctrine\ORM\QueryBuilder;
1617
use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader;
1718
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
1819
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
@@ -23,6 +24,7 @@
2324
use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory;
2425
use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
2526
use Symfony\Component\Form\Exception\RuntimeException;
27+
use Symfony\Component\Form\Exception\UnexpectedTypeException;
2628
use Symfony\Component\Form\FormBuilderInterface;
2729
use Symfony\Component\OptionsResolver\Options;
2830
use Symfony\Component\OptionsResolver\OptionsResolver;
@@ -71,20 +73,24 @@ public function configureOptions(OptionsResolver $resolver)
7173
$choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {
7274
// Unless the choices are given explicitly, load them on demand
7375
if (null === $options['choices']) {
74-
// Don't cache if the query builder is constructed dynamically
75-
if ($options['query_builder'] instanceof \Closure) {
76-
$hash = null;
77-
} else {
78-
$hash = CachingFactoryDecorator::generateHash(array(
79-
$options['em'],
80-
$options['class'],
81-
$options['query_builder'],
82-
$options['loader'],
83-
));
84-
85-
if (isset($choiceLoaders[$hash])) {
86-
return $choiceLoaders[$hash];
87-
}
76+
// We consider two query builders with an equal SQL string and
77+
// equal parameters to be equal
78+
$qbParts = $options['query_builder']
79+
? array(
80+
$options['query_builder']->getQuery()->getSQL(),
81+
$options['query_builder']->getParameters()->toArray(),
82+
)
83+
: null;
84+
85+
$hash = CachingFactoryDecorator::generateHash(array(
86+
$options['em'],
87+
$options['class'],
88+
$qbParts,
89+
$options['loader'],
90+
));
91+
92+
if (isset($choiceLoaders[$hash])) {
93+
return $choiceLoaders[$hash];
8894
}
8995

9096
if ($options['loader']) {
@@ -96,18 +102,14 @@ public function configureOptions(OptionsResolver $resolver)
96102
$entityLoader = $type->getLoader($options['em'], $queryBuilder, $options['class']);
97103
}
98104

99-
$choiceLoader = new DoctrineChoiceLoader(
105+
$choiceLoaders[$hash] = new DoctrineChoiceLoader(
100106
$choiceListFactory,
101107
$options['em'],
102108
$options['class'],
103109
$entityLoader
104110
);
105111

106-
if (null !== $hash) {
107-
$choiceLoaders[$hash] = $choiceLoader;
108-
}
109-
110-
return $choiceLoader;
112+
return $choiceLoaders[$hash];
111113
}
112114
};
113115

@@ -199,6 +201,20 @@ public function configureOptions(OptionsResolver $resolver)
199201
return $entitiesById;
200202
};
201203

204+
// Invoke the query builder closure so that we can cache choice lists
205+
// for equal query builders
206+
$queryBuilderNormalizer = function (Options $options, $queryBuilder) {
207+
if (is_callable($queryBuilder)) {
208+
$queryBuilder = call_user_func($queryBuilder, $options['em']->getRepository($options['class']));
209+
210+
if (!$queryBuilder instanceof QueryBuilder) {
211+
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
212+
}
213+
}
214+
215+
return $queryBuilder;
216+
};
217+
202218
$resolver->setDefaults(array(
203219
'em' => null,
204220
'property' => null, // deprecated, use "choice_label"
@@ -216,9 +232,11 @@ public function configureOptions(OptionsResolver $resolver)
216232

217233
$resolver->setNormalizer('em', $emNormalizer);
218234
$resolver->setNormalizer('choices', $choicesNormalizer);
235+
$resolver->setNormalizer('query_builder', $queryBuilderNormalizer);
219236

220237
$resolver->setAllowedTypes('em', array('null', 'string', 'Doctrine\Common\Persistence\ObjectManager'));
221238
$resolver->setAllowedTypes('loader', array('null', 'Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface'));
239+
$resolver->setAllowedTypes('query_builder', array('null', 'callable', 'Doctrine\ORM\QueryBuilder'));
222240
}
223241

224242
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php
+1-5Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ class EntityType extends DoctrineType
2828
*/
2929
public function getLoader(ObjectManager $manager, $queryBuilder, $class)
3030
{
31-
return new ORMQueryBuilderLoader(
32-
$queryBuilder,
33-
$manager,
34-
$class
35-
);
31+
return new ORMQueryBuilderLoader($queryBuilder, $manager, $class);
3632
}
3733

3834
public function getName()

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
+17-14Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Doctrine\Common\Collections\ArrayCollection;
1515
use Doctrine\Common\Persistence\ManagerRegistry;
1616
use Doctrine\ORM\EntityManager;
17+
use Doctrine\ORM\EntityRepository;
1718
use Doctrine\ORM\Tools\SchemaTool;
1819
use Symfony\Bridge\Doctrine\Form\DoctrineOrmExtension;
1920
use Symfony\Bridge\Doctrine\Form\DoctrineOrmTypeGuesser;
@@ -28,6 +29,7 @@
2829
use Symfony\Component\Form\ChoiceList\View\ChoiceView;
2930
use Symfony\Component\Form\Forms;
3031
use Symfony\Component\Form\Test\TypeTestCase;
32+
use Symfony\Component\OptionsResolver\Options;
3133
use Symfony\Component\PropertyAccess\PropertyAccess;
3234

3335
class EntityTypeTest extends TypeTestCase
@@ -172,7 +174,7 @@ public function testSetDataToUninitializedEntityWithNonRequiredQueryBuilder()
172174
}
173175

174176
/**
175-
* @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException
177+
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
176178
*/
177179
public function testConfigureQueryBuilderWithNonQueryBuilderAndNonClosure()
178180
{
@@ -786,8 +788,7 @@ public function testLoaderCaching()
786788

787789
$this->persist(array($entity1, $entity2, $entity3));
788790

789-
$repository = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
790-
$qb = $repository->createQueryBuilder('e')->where('e.id IN (1, 2)');
791+
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
791792

792793
$entityType = new EntityType(
793794
$this->emRegistry,
@@ -806,19 +807,23 @@ public function testLoaderCaching()
806807
$formBuilder->add('property1', 'entity', array(
807808
'em' => 'default',
808809
'class' => self::SINGLE_IDENT_CLASS,
809-
'query_builder' => $qb,
810+
'query_builder' => $repo->createQueryBuilder('e')->where('e.id IN (1, 2)'),
810811
));
811812

812813
$formBuilder->add('property2', 'entity', array(
813814
'em' => 'default',
814815
'class' => self::SINGLE_IDENT_CLASS,
815-
'query_builder' => $qb,
816+
'query_builder' => function (EntityRepository $repo) {
817+
return $repo->createQueryBuilder('e')->where('e.id IN (1, 2)');
818+
},
816819
));
817820

818821
$formBuilder->add('property3', 'entity', array(
819822
'em' => 'default',
820823
'class' => self::SINGLE_IDENT_CLASS,
821-
'query_builder' => $qb,
824+
'query_builder' => function (EntityRepository $repo) {
825+
return $repo->createQueryBuilder('e')->where('e.id IN (1, 2)');
826+
},
822827
));
823828

824829
$form = $formBuilder->getForm();
@@ -829,15 +834,13 @@ public function testLoaderCaching()
829834
'property3' => 2,
830835
));
831836

832-
$reflectionClass = new \ReflectionObject($entityType);
833-
$reflectionProperty = $reflectionClass->getProperty('loaderCache');
834-
$reflectionProperty->setAccessible(true);
835-
836-
$loaders = $reflectionProperty->getValue($entityType);
837-
838-
$reflectionProperty->setAccessible(false);
837+
$choiceList1 = $form->get('property1')->getConfig()->getOption('choice_list');
838+
$choiceList2 = $form->get('property2')->getConfig()->getOption('choice_list');
839+
$choiceList3 = $form->get('property3')->getConfig()->getOption('choice_list');
839840

840-
$this->assertCount(1, $loaders);
841+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
842+
$this->assertSame($choiceList1, $choiceList2);
843+
$this->assertSame($choiceList1, $choiceList3);
841844
}
842845

843846
public function testCacheChoiceLists()

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.