[DoctrineBridge] Deprecated implicit optimization in DoctrineChoiceLoader

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
 -----

src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php

@@ -49,9 +49,19 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
     {
         $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 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
 
         // 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);

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

@@ -150,7 +150,8 @@ abstract class DoctrineType extends AbstractType implements ResetInterface
                     $options['em'],
                     $options['class'],
                     $options['id_reader'],
-                    $entityLoader
+                    $entityLoader,
+                    false
                 );
 
                 if (null !== $hash) {

src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php

@@ -18,6 +18,7 @@ use PHPUnit\Framework\TestCase;
 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 @@ class DoctrineChoiceLoaderTest extends TestCase
 
         $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']));
+    }
 }