Skip to content

Commit 0c33d97

Browse files
webmozartwebmozart
committed
[Form] Improved performance of ChoiceType and its subtypes
1 parent 77f4b23 commit 0c33d97

File tree

5 files changed

+114
-14
lines changed

5 files changed

+114
-14
lines changed

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,19 @@ abstract class DoctrineType extends AbstractType
3737
protected $registry;
3838

3939
/**
40-
* @var ChoiceListFactoryInterface
40+
* @var DoctrineChoiceLoader[]
4141
*/
42-
private $choiceListFactory;
42+
private $choiceLoaders = array();
4343

4444
/**
4545
* @var IdReader[]
4646
*/
4747
private $idReaders = array();
4848

4949
/**
50-
* @var DoctrineChoiceLoader[]
50+
* @var ChoiceListFactoryInterface
5151
*/
52-
private $choiceLoaders = array();
52+
private $choiceListFactory;
5353

5454
/**
5555
* Creates the label for a choice.
@@ -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)

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

Lines changed: 18 additions & 5 deletions
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;
@@ -64,19 +65,31 @@ public function getName()
6465
/**
6566
* We consider two query builders with an equal SQL string and
6667
* equal parameters to be equal.
67-
*
68+
*
6869
* @param QueryBuilder $queryBuilder
69-
*
70+
*
7071
* @return array
71-
*
72+
*
7273
* @internal This method is public to be usable as callback. It should not
7374
* be used in user code.
7475
*/
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+
private function parameterToArray(Parameter $parameter)
92+
{
93+
return array($parameter->getName(), $parameter->getType(), $parameter->getValue());
94+
}
8295
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,6 +1114,69 @@ public function testLoaderCaching()
11141114
$this->assertSame($choiceList1, $choiceList3);
11151115
}
11161116

1117+
public function testLoaderCachingWithParameters()
1118+
{
1119+
$entity1 = new SingleIntIdEntity(1, 'Foo');
1120+
$entity2 = new SingleIntIdEntity(2, 'Bar');
1121+
$entity3 = new SingleIntIdEntity(3, 'Baz');
1122+
1123+
$this->persist(array($entity1, $entity2, $entity3));
1124+
1125+
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
1126+
1127+
$entityType = new EntityType(
1128+
$this->emRegistry,
1129+
PropertyAccess::createPropertyAccessor()
1130+
);
1131+
1132+
$entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
1133+
1134+
$factory = Forms::createFormFactoryBuilder()
1135+
->addType($entityType)
1136+
->addTypeGuesser($entityTypeGuesser)
1137+
->getFormFactory();
1138+
1139+
$formBuilder = $factory->createNamedBuilder('form', 'form');
1140+
1141+
$formBuilder->add('property1', 'entity', array(
1142+
'em' => 'default',
1143+
'class' => self::SINGLE_IDENT_CLASS,
1144+
'query_builder' => $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1),
1145+
));
1146+
1147+
$formBuilder->add('property2', 'entity', array(
1148+
'em' => 'default',
1149+
'class' => self::SINGLE_IDENT_CLASS,
1150+
'query_builder' => function (EntityRepository $repo) {
1151+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1152+
},
1153+
));
1154+
1155+
$formBuilder->add('property3', 'entity', array(
1156+
'em' => 'default',
1157+
'class' => self::SINGLE_IDENT_CLASS,
1158+
'query_builder' => function (EntityRepository $repo) {
1159+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1160+
},
1161+
));
1162+
1163+
$form = $formBuilder->getForm();
1164+
1165+
$form->submit(array(
1166+
'property1' => 1,
1167+
'property2' => 1,
1168+
'property3' => 2,
1169+
));
1170+
1171+
$choiceList1 = $form->get('property1')->getConfig()->getOption('choice_list');
1172+
$choiceList2 = $form->get('property2')->getConfig()->getOption('choice_list');
1173+
$choiceList3 = $form->get('property3')->getConfig()->getOption('choice_list');
1174+
1175+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
1176+
$this->assertSame($choiceList1, $choiceList2);
1177+
$this->assertSame($choiceList1, $choiceList3);
1178+
}
1179+
11171180
public function testCacheChoiceLists()
11181181
{
11191182
$entity1 = new SingleIntIdEntity(1, 'Foo');

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

Lines changed: 14 additions & 0 deletions
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="form.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

Lines changed: 6 additions & 1 deletion
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;
@@ -46,7 +47,11 @@ class ChoiceType extends AbstractType
4647

4748
public function __construct(ChoiceListFactoryInterface $choiceListFactory = null)
4849
{
49-
$this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory());
50+
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
51+
new PropertyAccessDecorator(
52+
new DefaultChoiceListFactory()
53+
)
54+
);
5055
}
5156

5257
/**

0 commit comments

Comments
 (0)