Skip to content

Commit

Permalink
[DoctrineBridge] DoctrineType now respects the "query_builder" option…
Browse files Browse the repository at this point in the history 
… when caching the choice loader
  • Loading branch information
@webmozart
webmozart committed Mar 31, 2015
1 parent 3846b37 commit e6739bf
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 39 deletions.
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php
View file
Expand Up @@ -51,6 +51,8 @@ public function __construct($queryBuilder, $manager = null, $class = null)
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure');
}

// This block is not executed anymore since Symfony 2.7. The query
// builder closure is already invoked in DoctrineType
if ($queryBuilder instanceof \Closure) {
if (!$manager instanceof EntityManager) {
throw new UnexpectedTypeException($manager, 'Doctrine\ORM\EntityManager');
Expand Down
58 changes: 38 additions & 20 deletions src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
View file
Expand Up @@ -13,6 +13,7 @@

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\DataTransformer\CollectionToArrayTransformer;
Expand All @@ -23,6 +24,7 @@
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;
Expand Down Expand Up @@ -71,20 +73,24 @@ public function configureOptions(OptionsResolver $resolver)
$choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) {
// Unless the choices are given explicitly, load them on demand
if (null === $options['choices']) {
// Don't cache if the query builder is constructed dynamically
if ($options['query_builder'] instanceof \Closure) {
$hash = null;
} else {
$hash = CachingFactoryDecorator::generateHash(array(
$options['em'],
$options['class'],
$options['query_builder'],
$options['loader'],
));

if (isset($choiceLoaders[$hash])) {
return $choiceLoaders[$hash];
}
// 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'],
));

if (isset($choiceLoaders[$hash])) {
return $choiceLoaders[$hash];
}

if ($options['loader']) {
Expand All @@ -96,18 +102,14 @@ public function configureOptions(OptionsResolver $resolver)
$entityLoader = $type->getLoader($options['em'], $queryBuilder, $options['class']);
}

$choiceLoader = new DoctrineChoiceLoader(
$choiceLoaders[$hash] = new DoctrineChoiceLoader(
$choiceListFactory,
$options['em'],
$options['class'],
$entityLoader
);

if (null !== $hash) {
$choiceLoaders[$hash] = $choiceLoader;
}

return $choiceLoader;
return $choiceLoaders[$hash];
}
};

Expand Down Expand Up @@ -199,6 +201,20 @@ public function configureOptions(OptionsResolver $resolver)
return $entitiesById;
};

// 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->setDefaults(array(
'em' => null,
'property' => null, // deprecated, use "choice_label"
Expand All @@ -216,9 +232,11 @@ public function configureOptions(OptionsResolver $resolver)

$resolver->setNormalizer('em', $emNormalizer);
$resolver->setNormalizer('choices', $choicesNormalizer);
$resolver->setNormalizer('query_builder', $queryBuilderNormalizer);

$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'));
}

/**
Expand Down
6 changes: 1 addition & 5 deletions src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php
View file
Expand Up @@ -28,11 +28,7 @@ class EntityType extends DoctrineType
*/
public function getLoader(ObjectManager $manager, $queryBuilder, $class)
{
return new ORMQueryBuilderLoader(
$queryBuilder,
$manager,
$class
);
return new ORMQueryBuilderLoader($queryBuilder, $manager, $class);
}

public function getName()
Expand Down
31 changes: 17 additions & 14 deletions src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
View file
Expand Up @@ -14,6 +14,7 @@
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Tools\SchemaTool;
use Symfony\Bridge\Doctrine\Form\DoctrineOrmExtension;
use Symfony\Bridge\Doctrine\Form\DoctrineOrmTypeGuesser;
Expand All @@ -28,6 +29,7 @@
use Symfony\Component\Form\ChoiceList\View\ChoiceView;
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\Test\TypeTestCase;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\PropertyAccess\PropertyAccess;

class EntityTypeTest extends TypeTestCase
Expand Down Expand Up @@ -172,7 +174,7 @@ public function testSetDataToUninitializedEntityWithNonRequiredQueryBuilder()
}

/**
* @expectedException \Symfony\Component\Form\Exception\UnexpectedTypeException
* @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException
*/
public function testConfigureQueryBuilderWithNonQueryBuilderAndNonClosure()
{
Expand Down Expand Up @@ -786,8 +788,7 @@ public function testLoaderCaching()

$this->persist(array($entity1, $entity2, $entity3));

$repository = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
$qb = $repository->createQueryBuilder('e')->where('e.id IN (1, 2)');
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);

$entityType = new EntityType(
$this->emRegistry,
Expand All @@ -806,19 +807,23 @@ public function testLoaderCaching()
$formBuilder->add('property1', 'entity', array(
'em' => 'default',
'class' => self::SINGLE_IDENT_CLASS,
'query_builder' => $qb,
'query_builder' => $repo->createQueryBuilder('e')->where('e.id IN (1, 2)'),
));

$formBuilder->add('property2', 'entity', array(
'em' => 'default',
'class' => self::SINGLE_IDENT_CLASS,
'query_builder' => $qb,
'query_builder' => function (EntityRepository $repo) {
return $repo->createQueryBuilder('e')->where('e.id IN (1, 2)');
},
));

$formBuilder->add('property3', 'entity', array(
'em' => 'default',
'class' => self::SINGLE_IDENT_CLASS,
'query_builder' => $qb,
'query_builder' => function (EntityRepository $repo) {
return $repo->createQueryBuilder('e')->where('e.id IN (1, 2)');
},
));

$form = $formBuilder->getForm();
Expand All @@ -829,15 +834,13 @@ public function testLoaderCaching()
'property3' => 2,
));

$reflectionClass = new \ReflectionObject($entityType);
$reflectionProperty = $reflectionClass->getProperty('loaderCache');
$reflectionProperty->setAccessible(true);

$loaders = $reflectionProperty->getValue($entityType);

$reflectionProperty->setAccessible(false);
$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->assertCount(1, $loaders);
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
$this->assertSame($choiceList1, $choiceList2);
$this->assertSame($choiceList1, $choiceList3);
}

public function testCacheChoiceLists()
Expand Down

0 comments on commit e6739bf

Please sign in to comment.