Skip to content

Commit e73f28b

Browse files
webmozartwebmozart
committed
[Form] Improved performance of ChoiceType and its subtypes
1 parent d4c8c13 commit e73f28b

File tree

5 files changed

+117
-13
lines changed

5 files changed

+117
-13
lines changed

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

+15-10
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@
3131

3232
abstract class DoctrineType extends AbstractType
3333
{
34+
/**
35+
* @var DoctrineChoiceLoader[]
36+
*/
37+
private static $choiceLoaders = array();
38+
3439
/**
3540
* @var ManagerRegistry
3641
*/
@@ -46,11 +51,6 @@ abstract class DoctrineType extends AbstractType
4651
*/
4752
private $idReaders = array();
4853

49-
/**
50-
* @var DoctrineChoiceLoader[]
51-
*/
52-
private $choiceLoaders = array();
53-
5454
/**
5555
* Creates the label for a choice.
5656
*
@@ -94,12 +94,12 @@ public static function createChoiceName($choice, $key, $value)
9494
* Gets important parts from QueryBuilder that will allow to cache its results.
9595
* For instance in ORM two query builders with an equal SQL string and
9696
* equal parameters are considered to be equal.
97-
*
97+
*
9898
* @param object $queryBuilder
99-
*
99+
*
100100
* @return array|false Array with important QueryBuilder parts or false if
101101
* they can't be determined
102-
*
102+
*
103103
* @internal This method is public to be usable as callback. It should not
104104
* be used in user code.
105105
*/
@@ -111,7 +111,12 @@ public function getQueryBuilderPartsForCachingHash($queryBuilder)
111111
public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null)
112112
{
113113
$this->registry = $registry;
114-
$this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory(), $propertyAccessor);
114+
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
115+
new PropertyAccessDecorator(
116+
new DefaultChoiceListFactory(),
117+
$propertyAccessor
118+
)
119+
);
115120
}
116121

117122
public function buildForm(FormBuilderInterface $builder, array $options)
@@ -129,7 +134,7 @@ public function configureOptions(OptionsResolver $resolver)
129134
$registry = $this->registry;
130135
$choiceListFactory = $this->choiceListFactory;
131136
$idReaders = &$this->idReaders;
132-
$choiceLoaders = &$this->choiceLoaders;
137+
$choiceLoaders = &self::$choiceLoaders;
133138
$type = $this;
134139

135140
$choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bridge\Doctrine\Form\Type;
1313

1414
use Doctrine\Common\Persistence\ObjectManager;
15+
use Doctrine\ORM\Query\Parameter;
1516
use Doctrine\ORM\QueryBuilder;
1617
use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
1718
use Symfony\Component\Form\Exception\UnexpectedTypeException;
@@ -75,8 +76,23 @@ public function getName()
7576
public function getQueryBuilderPartsForCachingHash($queryBuilder)
7677
{
7778
return array(
78-
$queryBuilder->getQuery()->getSQL(),
79-
$queryBuilder->getParameters()->toArray(),
79+
$queryBuilder->getQuery()->getSQL(),
80+
array_map(array($this, 'parameterToArray'), $queryBuilder->getParameters()->toArray()),
8081
);
8182
}
83+
84+
/**
85+
* Converts a query parameter to an array.
86+
*
87+
* @param Parameter $parameter The query parameter
88+
*
89+
* @return array The array representation of the parameter
90+
*
91+
* @internal This method is public to be usable as callback. It should not
92+
* be used in user code.
93+
*/
94+
public function parameterToArray(Parameter $parameter)
95+
{
96+
return array($parameter->getName(), $parameter->getType(), $parameter->getValue());
97+
}
8298
}

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

+63
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,69 @@ public function testLoaderCaching()
978978
$this->assertSame($choiceList1, $choiceList3);
979979
}
980980

981+
public function testLoaderCachingWithParameters()
982+
{
983+
$entity1 = new SingleIntIdEntity(1, 'Foo');
984+
$entity2 = new SingleIntIdEntity(2, 'Bar');
985+
$entity3 = new SingleIntIdEntity(3, 'Baz');
986+
987+
$this->persist(array($entity1, $entity2, $entity3));
988+
989+
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
990+
991+
$entityType = new EntityType(
992+
$this->emRegistry,
993+
PropertyAccess::createPropertyAccessor()
994+
);
995+
996+
$entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
997+
998+
$factory = Forms::createFormFactoryBuilder()
999+
->addType($entityType)
1000+
->addTypeGuesser($entityTypeGuesser)
1001+
->getFormFactory();
1002+
1003+
$formBuilder = $factory->createNamedBuilder('form', 'form');
1004+
1005+
$formBuilder->add('property1', 'entity', array(
1006+
'em' => 'default',
1007+
'class' => self::SINGLE_IDENT_CLASS,
1008+
'query_builder' => $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1),
1009+
));
1010+
1011+
$formBuilder->add('property2', 'entity', array(
1012+
'em' => 'default',
1013+
'class' => self::SINGLE_IDENT_CLASS,
1014+
'query_builder' => function (EntityRepository $repo) {
1015+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1016+
},
1017+
));
1018+
1019+
$formBuilder->add('property3', 'entity', array(
1020+
'em' => 'default',
1021+
'class' => self::SINGLE_IDENT_CLASS,
1022+
'query_builder' => function (EntityRepository $repo) {
1023+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1024+
},
1025+
));
1026+
1027+
$form = $formBuilder->getForm();
1028+
1029+
$form->submit(array(
1030+
'property1' => 1,
1031+
'property2' => 1,
1032+
'property3' => 2,
1033+
));
1034+
1035+
$choiceList1 = $form->get('property1')->getConfig()->getOption('choice_list');
1036+
$choiceList2 = $form->get('property2')->getConfig()->getOption('choice_list');
1037+
$choiceList3 = $form->get('property3')->getConfig()->getOption('choice_list');
1038+
1039+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
1040+
$this->assertSame($choiceList1, $choiceList2);
1041+
$this->assertSame($choiceList1, $choiceList3);
1042+
}
1043+
9811044
public function testCacheChoiceLists()
9821045
{
9831046
$entity1 = new SingleIntIdEntity(1, 'Foo');

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml

+14
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@
5757
<!-- CoreExtension -->
5858
<service id="form.property_accessor" alias="property_accessor" public="false" />
5959

60+
<service id="form.choice_list_factory.default" class="Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory" public="false"/>
61+
62+
<service id="form.choice_list_factory.property_access" class="Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator" public="false">
63+
<argument type="service" id="form.choice_list_factory.default"/>
64+
<argument type="service" id="property_accessor"/>
65+
</service>
66+
67+
<service id="form.choice_list_factory.cached" class="Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator" public="false">
68+
<argument type="service" id="form.choice_list_factory.property_access"/>
69+
</service>
70+
71+
<service id="form.choice_list_factory" alias="form.choice_list_factory.cached" public="false"/>
72+
6073
<service id="form.type.form" class="Symfony\Component\Form\Extension\Core\Type\FormType">
6174
<argument type="service" id="form.property_accessor" />
6275
<tag name="form.type" alias="form" />
@@ -69,6 +82,7 @@
6982
</service>
7083
<service id="form.type.choice" class="Symfony\Component\Form\Extension\Core\Type\ChoiceType">
7184
<tag name="form.type" alias="choice" />
85+
<argument type="service" id="form.choice_list_factory"/>
7286
</service>
7387
<service id="form.type.collection" class="Symfony\Component\Form\Extension\Core\Type\CollectionType">
7488
<tag name="form.type" alias="collection" />

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

+7-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Form\Extension\Core\Type;
1313

1414
use Symfony\Component\Form\AbstractType;
15+
use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
1516
use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
1617
use Symfony\Component\Form\ChoiceList\LegacyChoiceListAdapter;
1718
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
@@ -34,6 +35,7 @@
3435
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer;
3536
use Symfony\Component\OptionsResolver\Options;
3637
use Symfony\Component\OptionsResolver\OptionsResolver;
38+
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
3739

3840
class ChoiceType extends AbstractType
3941
{
@@ -46,7 +48,11 @@ class ChoiceType extends AbstractType
4648

4749
public function __construct(ChoiceListFactoryInterface $choiceListFactory = null)
4850
{
49-
$this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory());
51+
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
52+
new PropertyAccessDecorator(
53+
new DefaultChoiceListFactory()
54+
)
55+
);
5056
}
5157

5258
/**

0 commit comments

Comments
 (0)