[DoctrineBridge] Improved performance of the EntityType when used with the "query_builder" option

src/Symfony/Bridge/Doctrine/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+2.1.5
+-----
+
+ * fixed caching of choice lists when EntityType is used with the "query_builder" option
+
 2.1.0
 -----
 

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

@@ -11,26 +11,53 @@
 
 namespace Symfony\Bridge\Doctrine\Form\Type;
 
-use Doctrine\Common\Persistence\ObjectManager;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
+use Doctrine\Common\Persistence\ObjectManager;
+use Doctrine\ORM\QueryBuilder;
 
 class EntityType extends DoctrineType
 {
+    /**
+     * @var array
+     */
+    private $loaderCache = array();
+
     /**
      * Return the default loader object.
      *
-     * @param ObjectManager $manager
+     * @param ObjectManager         $manager
-     * @param mixed         $queryBuilder
+     * @param QueryBuilder|\Closure $queryBuilder
-     * @param string        $class
+     * @param string                $class
+     *
      * @return ORMQueryBuilderLoader
+     *
+     * @throws UnexpectedTypeException If the passed $queryBuilder is no \Closure
+     *                                 and no QueryBuilder or if the closure
+     *                                 does not return a QueryBuilder.
      */
     public function getLoader(ObjectManager $manager, $queryBuilder, $class)
     {
-        return new ORMQueryBuilderLoader(
+        if ($queryBuilder instanceof \Closure) {
-            $queryBuilder,
+            $queryBuilder = $queryBuilder($manager->getRepository($class));
-            $manager,
+
-            $class
+            if (!$queryBuilder instanceof QueryBuilder) {
-        );
+                throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
+            }
+        } elseif (!$queryBuilder instanceof QueryBuilder) {
+            throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
+        }
+
+        // It is important to return the same loader for identical queries,
+        // otherwise the caching mechanism in DoctrineType does not work
+        // (which expects identical loaders for the cache to work)
+        $hash = md5($queryBuilder->getQuery()->getDQL());
+
+        if (!isset($this->loaderCache[$hash])) {
+            $this->loaderCache[$hash] = new ORMQueryBuilderLoader($queryBuilder);
+        }
+
+        return $this->loaderCache[$hash];
     }
 
     public function getName()

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

@@ -12,6 +12,7 @@
 namespace Symfony\Bridge\Doctrine\Tests\Form\Type;
 
 use Symfony\Component\Form\Tests\FormPerformanceTestCase;
+use Doctrine\ORM\EntityRepository;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIdentEntity;
 use Doctrine\ORM\Tools\SchemaTool;
 use Symfony\Bridge\Doctrine\Tests\DoctrineOrmTestCase;
@@ -36,6 +37,9 @@ class EntityTypePerformanceTest extends FormPerformanceTestCase
         $manager->expects($this->any())
             ->method('getManager')
             ->will($this->returnValue($this->em));
+        $manager->expects($this->any())
+            ->method('getManagerForClass')
+            ->will($this->returnValue($this->em));
 
         return array(
             new CoreExtension(),
@@ -109,4 +113,21 @@ class EntityTypePerformanceTest extends FormPerformanceTestCase
             $form->createView();
         }
     }
+
+    public function testCollapsedEntityFieldWithQueryBuilder()
+    {
+        $this->setMaxRunningTime(1);
+
+        for ($i = 0; $i < 20; ++$i) {
+            $form = $this->factory->create('entity', null, array(
+                'class' => self::ENTITY_CLASS,
+                'query_builder' => function (EntityRepository $repo) {
+                    return $repo->createQueryBuilder('e')->addOrderBy('e.id', 'DESC');
+                }
+            ));
+
+            // force loading of the choice list
+            $form->createView();
+        }
+    }
 }