bug #14576 [DoctrineBridge][Form] Fix BC break in DoctrineType (malarzm)

This PR was squashed before being merged into the 2.7 branch (closes #14576).

Discussion
----------

[DoctrineBridge][Form] Fix BC break in DoctrineType

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

Commits
-------

3172d73 [DoctrineBridge][Form] Fix BC break in DoctrineType

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

@@ -13,7 +13,6 @@ namespace Symfony\Bridge\Doctrine\Form\Type;
 
 use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\Common\Persistence\ObjectManager;
-use Doctrine\ORM\QueryBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader;
@@ -25,7 +24,6 @@ use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;
 use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory;
 use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
 use Symfony\Component\Form\Exception\RuntimeException;
-use Symfony\Component\Form\Exception\UnexpectedTypeException;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
@@ -92,6 +90,24 @@ abstract class DoctrineType extends AbstractType
         return (string) $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.
+     */
+    public function getQueryBuilderPartsForCachingHash($queryBuilder)
+    {
+        return false;
+    }
+
     public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null)
     {
         $this->registry = $registry;
@@ -117,29 +133,28 @@ abstract class DoctrineType extends AbstractType
         $type = $this;
 
         $choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {
-            // This closure and the "query_builder" options should be pushed to
-            // EntityType in Symfony 3.0 as they are specific to the ORM
 
             // Unless the choices are given explicitly, load them on demand
             if (null === $options['choices']) {
-                // We consider two query builders with an equal SQL string and
-                // equal parameters to be equal
-                $qbParts = $options['query_builder']
-                    ? array(
-                        $options['query_builder']->getQuery()->getSQL(),
-                        $options['query_builder']->getParameters()->toArray(),
-                    )
-                    : null;
 
-                $hash = CachingFactoryDecorator::generateHash(array(
-                    $options['em'],
-                    $options['class'],
-                    $qbParts,
-                    $options['loader'],
-                ));
+                $hash = null;
+                $qbParts = null;
 
-                if (isset($choiceLoaders[$hash])) {
-                    return $choiceLoaders[$hash];
+                // If there is no QueryBuilder we can safely cache DoctrineChoiceLoader,
+                // also if concrete Type can return important QueryBuilder parts to generate
+                // hash key we go for it as well
+                if (!$options['query_builder'] || false !== ($qbParts = $type->getQueryBuilderPartsForCachingHash($options['query_builder']))) {
+
+                    $hash = CachingFactoryDecorator::generateHash(array(
+                        $options['em'],
+                        $options['class'],
+                        $qbParts,
+                        $options['loader'],
+                    ));
+
+                    if (isset($choiceLoaders[$hash])) {
+                        return $choiceLoaders[$hash];
+                    }
                 }
 
                 if ($options['loader']) {
@@ -151,7 +166,7 @@ abstract class DoctrineType extends AbstractType
                     $entityLoader = $type->getLoader($options['em'], $queryBuilder, $options['class']);
                 }
 
-                $choiceLoaders[$hash] = new DoctrineChoiceLoader(
+                $doctrineChoiceLoader = new DoctrineChoiceLoader(
                     $choiceListFactory,
                     $options['em'],
                     $options['class'],
@@ -159,7 +174,11 @@ abstract class DoctrineType extends AbstractType
                     $entityLoader
                 );
 
-                return $choiceLoaders[$hash];
+                if ($hash !== null) {
+                    $choiceLoaders[$hash] = $doctrineChoiceLoader;
+                }
+
+                return $doctrineChoiceLoader;
             }
         };
 
@@ -240,10 +259,6 @@ abstract class DoctrineType extends AbstractType
         $queryBuilderNormalizer = function (Options $options, $queryBuilder) {
             if (is_callable($queryBuilder)) {
                 $queryBuilder = call_user_func($queryBuilder, $options['em']->getRepository($options['class']));
-
-                if (!$queryBuilder instanceof QueryBuilder) {
-                    throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
-                }
             }
 
             return $queryBuilder;
@@ -305,7 +320,6 @@ abstract class DoctrineType extends AbstractType
 
         $resolver->setAllowedTypes('em', array('null', 'string', 'Doctrine\Common\Persistence\ObjectManager'));
         $resolver->setAllowedTypes('loader', array('null', 'Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface'));
-        $resolver->setAllowedTypes('query_builder', array('null', 'callable', 'Doctrine\ORM\QueryBuilder'));
     }
 
     /**

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

@@ -14,9 +14,34 @@ namespace Symfony\Bridge\Doctrine\Form\Type;
 use Doctrine\Common\Persistence\ObjectManager;
 use Doctrine\ORM\QueryBuilder;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
+use Symfony\Component\OptionsResolver\Options;
+use Symfony\Component\OptionsResolver\OptionsResolver;
 
 class EntityType extends DoctrineType
 {
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        parent::configureOptions($resolver);
+
+        // Invoke the query builder closure so that we can cache choice lists
+        // for equal query builders
+        $queryBuilderNormalizer = function (Options $options, $queryBuilder) {
+            if (is_callable($queryBuilder)) {
+                $queryBuilder = call_user_func($queryBuilder, $options['em']->getRepository($options['class']));
+
+                if (!$queryBuilder instanceof QueryBuilder) {
+                    throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder');
+                }
+            }
+
+            return $queryBuilder;
+        };
+
+        $resolver->setNormalizer('query_builder', $queryBuilderNormalizer);
+        $resolver->setAllowedTypes('query_builder', array('null', 'callable', 'Doctrine\ORM\QueryBuilder'));
+    }
+
     /**
      * Return the default loader object.
      *
@@ -35,4 +60,23 @@ class EntityType extends DoctrineType
     {
         return 'entity';
     }
+
+    /**
+     * 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(),
+        );
+    }
 }