[Form] Improved performance of ChoiceType and its subtypes

symfony · Tobion · Dec 30, 2015 · Nov 29, 2015 · Dec 30, 2015 · a0ef1018d697523f57056138d128602816bfe943
commit a0ef1018d697523f57056138d128602816bfe943
diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
@@ -94,12 +94,12 @@ public static function createChoiceName($choice, $key, $value)
     * Gets important parts from QueryBuilder that will allow to cache its results.
     * For instance in ORM two query builders with an equal SQL string and
     * equal parameters are considered to be equal.
-     * 
+     *
     * @param object $queryBuilder
-     * 
+     *
     * @return array|false Array with important QueryBuilder parts or false if
     *                     they can't be determined
-     * 
+     *
     * @internal This method is public to be usable as callback. It should not
     *           be used in user code.
     */
@@ -111,7 +111,12 @@ public function getQueryBuilderPartsForCachingHash($queryBuilder)
    public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null)
    {
        $this->registry = $registry;
-        $this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory(), $propertyAccessor);
+        $this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
+            new PropertyAccessDecorator(
+                new DefaultChoiceListFactory(),
+                $propertyAccessor
+            )
+        );
    }

    public function buildForm(FormBuilderInterface $builder, array $options)

diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php
@@ -12,6 +12,7 @@
 namespace Symfony\Bridge\Doctrine\Form\Type;

 use Doctrine\Common\Persistence\ObjectManager;
+use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\QueryBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
 use Symfony\Component\Form\Exception\UnexpectedTypeException;
@@ -64,19 +65,31 @@ public function getName()
    /**
     * We consider two query builders with an equal SQL string and
     * equal parameters to be equal.
-     * 
+     *
     * @param QueryBuilder $queryBuilder
-     * 
+     *
     * @return array
-     * 
+     *
     * @internal This method is public to be usable as callback. It should not
     *           be used in user code.
     */
    public function getQueryBuilderPartsForCachingHash($queryBuilder)
    {
        return array(
-                $queryBuilder->getQuery()->getSQL(),
-                $queryBuilder->getParameters()->toArray(),
+            $queryBuilder->getQuery()->getSQL(),
+            array_map(array($this, 'parameterToArray'), $queryBuilder->getParameters()->toArray()),
        );
    }
+
+    /**
+     * Converts a query parameter to an array.
+     *
+     * @param Parameter $parameter The query parameter
+     *
+     * @return array The array representation of the parameter
+     */
+    private function parameterToArray(Parameter $parameter)
+    {
+        return array($parameter->getName(), $parameter->getType(), $parameter->getValue());
+    }
 }
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
@@ -1114,6 +1114,69 @@ public function testLoaderCaching()
        $this->assertSame($choiceList1, $choiceList3);
    }

+    public function testLoaderCachingWithParameters()
+    {
+        $entity1 = new SingleIntIdEntity(1, 'Foo');
+        $entity2 = new SingleIntIdEntity(2, 'Bar');
+        $entity3 = new SingleIntIdEntity(3, 'Baz');
+
+        $this->persist(array($entity1, $entity2, $entity3));
+
+        $repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
+
+        $entityType = new EntityType(
+            $this->emRegistry,
+            PropertyAccess::createPropertyAccessor()
+        );
+
+        $entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
+
+        $factory = Forms::createFormFactoryBuilder()
+            ->addType($entityType)
+            ->addTypeGuesser($entityTypeGuesser)
+            ->getFormFactory();
+
+        $formBuilder = $factory->createNamedBuilder('form', 'form');
+
+        $formBuilder->add('property1', 'entity', array(
+            'em' => 'default',
+            'class' => self::SINGLE_IDENT_CLASS,
+            'query_builder' => $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1),
+        ));
+
+        $formBuilder->add('property2', 'entity', array(
+            'em' => 'default',
+            'class' => self::SINGLE_IDENT_CLASS,
+            'query_builder' => function (EntityRepository $repo) {
+                return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
+            },
+        ));
+
+        $formBuilder->add('property3', 'entity', array(
+            'em' => 'default',
+            'class' => self::SINGLE_IDENT_CLASS,
+            'query_builder' => function (EntityRepository $repo) {
+                return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
+            },
+        ));
+
+        $form = $formBuilder->getForm();
+
+        $form->submit(array(
+            'property1' => 1,
+            'property2' => 1,
+            'property3' => 2,
+        ));
+
+        $choiceList1 = $form->get('property1')->getConfig()->getOption('choice_list');
+        $choiceList2 = $form->get('property2')->getConfig()->getOption('choice_list');
+        $choiceList3 = $form->get('property3')->getConfig()->getOption('choice_list');
+
+        $this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
+        $this->assertSame($choiceList1, $choiceList2);
+        $this->assertSame($choiceList1, $choiceList3);
+    }
+
    public function testCacheChoiceLists()
    {
        $entity1 = new SingleIntIdEntity(1, 'Foo');

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml
@@ -57,6 +57,19 @@
        <!-- CoreExtension -->
        <service id="form.property_accessor" alias="property_accessor" public="false" />

+        <service id="form.choice_list_factory.default" class="Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory" public="false"/>
+
+        <service id="form.choice_list_factory.property_access" class="Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator" public="false">
+            <argument type="service" id="form.choice_list_factory.default"/>
+            <argument type="service" id="form.property_accessor"/>
+        </service>
+
+        <service id="form.choice_list_factory.cached" class="Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator" public="false">
+            <argument type="service" id="form.choice_list_factory.property_access"/>
+        </service>
+
+        <service id="form.choice_list_factory" alias="form.choice_list_factory.cached" public="false"/>
+
        <service id="form.type.form" class="Symfony\Component\Form\Extension\Core\Type\FormType">
            <argument type="service" id="form.property_accessor" />
            <tag name="form.type" alias="form" />
@@ -69,6 +82,7 @@
        </service>
        <service id="form.type.choice" class="Symfony\Component\Form\Extension\Core\Type\ChoiceType">
            <tag name="form.type" alias="choice" />
+            <argument type="service" id="form.choice_list_factory"/>
        </service>
        <service id="form.type.collection" class="Symfony\Component\Form\Extension\Core\Type\CollectionType">
            <tag name="form.type" alias="collection" />

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Form\Extension\Core\Type;

 use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
 use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
 use Symfony\Component\Form\ChoiceList\LegacyChoiceListAdapter;
 use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
@@ -46,7 +47,11 @@ class ChoiceType extends AbstractType

    public function __construct(ChoiceListFactoryInterface $choiceListFactory = null)
    {
-        $this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory());
+        $this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
+            new PropertyAccessDecorator(
+                new DefaultChoiceListFactory()
+            )
+        );
    }

    /**