feature #13864 Entity type: loader caching by query builder instance (dominikzogg)

This PR was merged into the 2.7 branch.

Discussion
----------

Entity type: loader caching by query builder instance

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Using entity types within a collection, can get very slow, if a query_builder option is set.

Without this option, the doctrine type handles everything itself, and makes it calls against the manager registry, but using this, calls the getLoader method, which ends in build the choice list again for each collection element, which makes more than twice the time for a page load within my energycalculator.

https://github.com/dominikzogg/symfony/blob/2.7/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php#L68

https://github.com/dominikzogg/symfony/blob/2.7/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php#L108

https://github.com/dominikzogg/energycalculator/blob/master/src/Dominikzogg/EnergyCalculator/Form/ComestibleWithinDayType.php

Commits
-------

42b682b allow to cache entity loader based on query builder

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

@@ -12,10 +12,16 @@
 namespace Symfony\Bridge\Doctrine\Form\Type;
 
 use Doctrine\Common\Persistence\ObjectManager;
+use Doctrine\ORM\QueryBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
 
 class EntityType extends DoctrineType
 {
+    /**
+     * @var ORMQueryBuilderLoader[]
+     */
+    private $loaderCache = array();
+
     /**
      * Return the default loader object.
      *
@@ -27,11 +33,55 @@ class EntityType extends DoctrineType
      */
     public function getLoader(ObjectManager $manager, $queryBuilder, $class)
     {
-        return new ORMQueryBuilderLoader(
-            $queryBuilder,
-            $manager,
-            $class
-        );
+        if (!$queryBuilder instanceof QueryBuilder) {
+            return new ORMQueryBuilderLoader(
+                $queryBuilder,
+                $manager,
+                $class
+            );
+        }
+
+        $queryBuilderHash = $this->getQueryBuilderHash($queryBuilder);
+        $loaderHash = $this->getLoaderHash($manager, $queryBuilderHash, $class);
+
+        if (!isset($this->loaderCache[$loaderHash])) {
+            $this->loaderCache[$loaderHash] = new ORMQueryBuilderLoader(
+                $queryBuilder,
+                $manager,
+                $class
+            );
+        }
+
+        return $this->loaderCache[$loaderHash];
+    }
+
+    /**
+     * @param QueryBuilder $queryBuilder
+     *
+     * @return string
+     */
+    private function getQueryBuilderHash(QueryBuilder $queryBuilder)
+    {
+        return hash('sha256', json_encode(array(
+            'sql' => $queryBuilder->getQuery()->getSQL(),
+            'parameters' => $queryBuilder->getParameters(),
+        )));
+    }
+
+    /**
+     * @param ObjectManager $manager
+     * @param string        $queryBuilderHash
+     * @param string        $class
+     *
+     * @return string
+     */
+    private function getLoaderHash(ObjectManager $manager, $queryBuilderHash, $class)
+    {
+        return hash('sha256', json_encode(array(
+            'manager' => spl_object_hash($manager),
+            'queryBuilder' => $queryBuilderHash,
+            'class' => $class,
+        )));
     }
 
     public function getName()

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

@@ -11,7 +11,11 @@
 
 namespace Symfony\Bridge\Doctrine\Tests\Form\Type;
 
+use Symfony\Bridge\Doctrine\Form\DoctrineOrmTypeGuesser;
+use Symfony\Bridge\Doctrine\Form\Type\EntityType;
 use Symfony\Bridge\Doctrine\Test\DoctrineTestHelper;
+use Symfony\Component\Form\FormBuilder;
+use Symfony\Component\Form\Forms;
 use Symfony\Component\Form\Test\TypeTestCase;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\GroupableEntity;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity;
@@ -22,6 +26,7 @@ use Symfony\Bridge\Doctrine\Form\DoctrineOrmExtension;
 use Doctrine\ORM\Tools\SchemaTool;
 use Doctrine\Common\Collections\ArrayCollection;
 use Symfony\Component\Form\Extension\Core\View\ChoiceView;
+use Symfony\Component\PropertyAccess\PropertyAccess;
 
 class EntityTypeTest extends TypeTestCase
 {
@@ -656,7 +661,7 @@ class EntityTypeTest extends TypeTestCase
             'class' => self::SINGLE_IDENT_CLASS,
             'query_builder' => function ($repository) {
                 return $repository->createQueryBuilder('e')
-                        ->where('e.id IN (1, 2)');
+                    ->where('e.id IN (1, 2)');
             },
             'property' => 'name',
         ));
@@ -680,7 +685,7 @@ class EntityTypeTest extends TypeTestCase
             'class' => self::COMPOSITE_IDENT_CLASS,
             'query_builder' => function ($repository) {
                 return $repository->createQueryBuilder('e')
-                        ->where('e.id1 IN (10, 50)');
+                    ->where('e.id1 IN (10, 50)');
             },
             'property' => 'name',
         ));
@@ -766,13 +771,75 @@ class EntityTypeTest extends TypeTestCase
         ));
     }
 
+    public function testLoaderCaching()
+    {
+        $entity1 = new SingleIntIdEntity(1, 'Foo');
+        $entity2 = new SingleIntIdEntity(2, 'Bar');
+        $entity3 = new SingleIntIdEntity(3, 'Baz');
+
+        $this->persist(array($entity1, $entity2, $entity3));
+
+        $repository = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
+        $qb = $repository->createQueryBuilder('e')->where('e.id IN (1, 2)');
+
+        $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' => $qb,
+        ));
+
+        $formBuilder->add('property2', 'entity', array(
+            'em' => 'default',
+            'class' => self::SINGLE_IDENT_CLASS,
+            'query_builder' => $qb,
+        ));
+
+        $formBuilder->add('property3', 'entity', array(
+            'em' => 'default',
+            'class' => self::SINGLE_IDENT_CLASS,
+            'query_builder' => $qb,
+        ));
+
+        $form = $formBuilder->getForm();
+
+        $form->submit(array(
+            'property1' => 1,
+            'property2' => 1,
+            'property3' => 2,
+        ));
+
+        $reflectionClass = new \ReflectionObject($entityType);
+        $reflectionProperty = $reflectionClass->getProperty('loaderCache');
+        $reflectionProperty->setAccessible(true);
+
+        $loaders = $reflectionProperty->getValue($entityType);
+
+        $reflectionProperty->setAccessible(false);
+
+        $this->assertCount(1, $loaders);
+    }
+
     protected function createRegistryMock($name, $em)
     {
         $registry = $this->getMock('Doctrine\Common\Persistence\ManagerRegistry');
         $registry->expects($this->any())
-                 ->method('getManager')
-                 ->with($this->equalTo($name))
-                 ->will($this->returnValue($em));
+            ->method('getManager')
+            ->with($this->equalTo($name))
+            ->will($this->returnValue($em));
 
         return $registry;
     }