From 287c39b9ea52cd26a713fb1e7fe17fbf2503938b Mon Sep 17 00:00:00 2001 From: Jules Pietri Date: Sun, 7 Apr 2019 11:28:32 +0200 Subject: [PATCH] [DoctrineBridge] Deprecated implicit optimization in DoctrineChoiceLoader --- src/Symfony/Bridge/Doctrine/CHANGELOG.md | 1 + .../Form/ChoiceList/DoctrineChoiceLoader.php | 14 +++- .../Doctrine/Form/Type/DoctrineType.php | 3 +- .../ChoiceList/DoctrineChoiceLoaderTest.php | 77 +++++++++++++++++++ 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md index 6b617825c9190..27cabfa558fd9 100644 --- a/src/Symfony/Bridge/Doctrine/CHANGELOG.md +++ b/src/Symfony/Bridge/Doctrine/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * changed guessing of DECIMAL to set the `input` option of `NumberType` to string + * deprecated not passing an `IdReader` to the `DoctrineChoiceLoader` when query can be optimized with a single id field 4.2.0 ----- diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php index 0ed66f8c44f41..fd891abb34b09 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php @@ -49,9 +49,19 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR { $classMetadata = $manager->getClassMetadata($class); + if ((5 > \func_num_args() || false !== func_get_arg(4)) && null === $idReader) { + $idReader = new IdReader($manager, $classMetadata); + + if ($idReader->isSingleId()) { + @trigger_error(sprintf('Not explicitly passing an instance of "%s" when it can optimize single id entity "%s" has been deprecated in 4.3 and will not apply any optimization in 5.0.', IdReader::class, $class), E_USER_DEPRECATED); + } else { + $idReader = null; + } + } + $this->manager = $manager; $this->class = $classMetadata->getName(); - $this->idReader = $idReader ?: new IdReader($manager, $classMetadata); + $this->idReader = $idReader; $this->objectLoader = $objectLoader; } @@ -120,7 +130,7 @@ public function loadChoicesForValues(array $values, $value = null) // Optimize performance in case we have an object loader and // a single-field identifier - $optimize = null === $value || \is_array($value) && $this->idReader === $value[0]; + $optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]); if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) { $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values); diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index cd3534d51253f..03bf6a0ba509d 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php @@ -150,7 +150,8 @@ public function configureOptions(OptionsResolver $resolver) $options['em'], $options['class'], $options['id_reader'], - $entityLoader + $entityLoader, + false ); if (null !== $hash) { diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php index de6f3a3aa970a..2fbb07d7f3406 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php @@ -18,6 +18,7 @@ use Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface; use Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader; +use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity; use Symfony\Component\Form\ChoiceList\ArrayChoiceList; use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface; @@ -393,4 +394,80 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueIsIdReader() $this->assertSame([$this->obj2], $loader->loadChoicesForValues(['2'], $value)); } + + /** + * @group legacy + * + * @expectedDeprecation Not explicitly passing an instance of "Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader" when it can optimize single id entity "%s" has been deprecated in 4.3 and will not apply any optimization in 5.0. + */ + public function testLoaderWithoutIdReaderCanBeOptimized() + { + $obj1 = new SingleIntIdEntity('1', 'one'); + $obj2 = new SingleIntIdEntity('2', 'two'); + + $metadata = $this->createMock(ClassMetadata::class); + $metadata->expects($this->once()) + ->method('getIdentifierFieldNames') + ->willReturn(['idField']) + ; + $metadata->expects($this->any()) + ->method('getIdentifierValues') + ->willReturnCallback(function ($obj) use ($obj1, $obj2) { + if ($obj === $obj1) { + return ['idField' => '1']; + } + if ($obj === $obj2) { + return ['idField' => '2']; + } + + return null; + }) + ; + + $this->om = $this->createMock(ObjectManager::class); + $this->om->expects($this->once()) + ->method('getClassMetadata') + ->with(SingleIntIdEntity::class) + ->willReturn($metadata) + ; + $this->om->expects($this->any()) + ->method('contains') + ->with($this->isInstanceOf(SingleIntIdEntity::class)) + ->willReturn(true) + ; + + $loader = new DoctrineChoiceLoader( + $this->om, + SingleIntIdEntity::class, + null, + $this->objectLoader + ); + + $choices = [$obj1, $obj2]; + + $this->idReader->expects($this->any()) + ->method('isSingleId') + ->willReturn(true); + + $this->idReader->expects($this->any()) + ->method('getIdField') + ->willReturn('idField'); + + $this->repository->expects($this->never()) + ->method('findAll'); + + $this->objectLoader->expects($this->once()) + ->method('getEntitiesByIds') + ->with('idField', ['1']) + ->willReturn($choices); + + $this->idReader->expects($this->any()) + ->method('getIdValue') + ->willReturnMap([ + [$obj1, '1'], + [$obj2, '2'], + ]); + + $this->assertSame([$obj1], $loader->loadChoicesForValues(['1'])); + } }