From 3172d73b6c7969fe2f01664c495c641256418f39 Mon Sep 17 00:00:00 2001 From: Maciej Malarz Date: Thu, 7 May 2015 10:06:26 +0200 Subject: [PATCH] [DoctrineBridge][Form] Fix BC break in DoctrineType --- .../Doctrine/Form/Type/DoctrineType.php | 68 +++++++++++-------- .../Bridge/Doctrine/Form/Type/EntityType.php | 44 ++++++++++++ 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index acc6d52a92..81631b3ac1 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/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')); } /** diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php index 87b3ee42cb..486eca94ab 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php +++ b/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(), + ); + } }