bug #46098 [Form] Fix same choice loader with different choice values (HeahDude)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Form] Fix same choice loader with different choice values

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | #44655
| License       | MIT
| Doc PR        | ~

It appears that deprecating the caching in the `LazyChoiceList` (cf #18359) was a mistake. The bug went under the radar because in practice every choice field has its own loader instance.
However, optimizations made in #30994 then revealed the flaw (cf #42206) as the loaders were actually shared across many fields.
While working on a fix I ended up implementing something similar to what's proposed in #44655.

I'll send a PR for 5.4 as well.

Commits
-------

65cbf18ea8 [Form] Fix same choice loader with different choice values

Author: xabbuh

Form/ChoiceList/DoctrineChoiceLoader.php

@@ -13,7 +13,6 @@
 
 use Doctrine\Persistence\ObjectManager;
 use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
-use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface;
 
 /**
@@ -29,9 +28,9 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
     private $objectLoader;
 
     /**
-     * @var ChoiceListInterface
+     * @var array|null
      */
-    private $choiceList;
+    private $choices;
 
     /**
      * Creates a new choice loader.
@@ -74,15 +73,13 @@ public function __construct(ObjectManager $manager, string $class, IdReader $idR
      */
     public function loadChoiceList($value = null)
     {
-        if ($this->choiceList) {
-            return $this->choiceList;
+        if (null === $this->choices) {
+            $this->choices = $this->objectLoader
+                ? $this->objectLoader->getEntities()
+                : $this->manager->getRepository($this->class)->findAll();
         }
 
-        $objects = $this->objectLoader
-            ? $this->objectLoader->getEntities()
-            : $this->manager->getRepository($this->class)->findAll();
-
-        return $this->choiceList = new ArrayChoiceList($objects, $value);
+        return new ArrayChoiceList($this->choices, $value);
     }
 
     /**
@@ -100,7 +97,7 @@ public function loadValuesForChoices(array $choices, $value = null)
         $optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
 
         // Attention: This optimization does not check choices for existence
-        if ($optimize && !$this->choiceList && $this->idReader->isSingleId()) {
+        if ($optimize && !$this->choices && $this->idReader->isSingleId()) {
             $values = [];
 
             // Maintain order and indices of the given objects
@@ -136,7 +133,7 @@ public function loadChoicesForValues(array $values, $value = null)
         // a single-field identifier
         $optimize = $this->idReader && (null === $value || \is_array($value) && $this->idReader === $value[0]);
 
-        if ($optimize && !$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) {
+        if ($optimize && !$this->choices && $this->objectLoader && $this->idReader->isSingleId()) {
             $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values);
             $objectsById = [];
             $objects = [];

Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php

@@ -146,8 +146,7 @@ public function testLoadChoiceListUsesObjectLoaderIfAvailable()
         $this->assertEquals($choiceList, $loaded = $loader->loadChoiceList());
 
         // no further loads on subsequent calls
-
-        $this->assertSame($loaded, $loader->loadChoiceList());
+        $this->assertEquals($loaded, $loader->loadChoiceList());
     }
 
     public function testLoadValuesForChoices()

Tests/Form/Type/EntityTypeTest.php

@@ -1779,4 +1779,32 @@ public function testSubmitNullMultipleUsesDefaultEmptyData()
         $this->assertEquals($collection, $form->getNormData());
         $this->assertEquals($collection, $form->getData());
     }
+
+    public function testWithSameLoaderAndDifferentChoiceValueCallbacks()
+    {
+        $entity1 = new SingleIntIdEntity(1, 'Foo');
+        $entity2 = new SingleIntIdEntity(2, 'Bar');
+        $this->persist([$entity1, $entity2]);
+
+        $view = $this->factory->create(FormTypeTest::TESTED_TYPE)
+            ->add('entity_one', self::TESTED_TYPE, [
+                'em' => 'default',
+                'class' => self::SINGLE_IDENT_CLASS,
+            ])
+            ->add('entity_two', self::TESTED_TYPE, [
+                'em' => 'default',
+                'class' => self::SINGLE_IDENT_CLASS,
+                'choice_value' => function ($choice) {
+                    return $choice ? $choice->name : '';
+                },
+            ])
+            ->createView()
+        ;
+
+        $this->assertSame('1', $view['entity_one']->vars['choices'][1]->value);
+        $this->assertSame('2', $view['entity_one']->vars['choices'][2]->value);
+
+        $this->assertSame('Foo', $view['entity_two']->vars['choices']['Foo']->value);
+        $this->assertSame('Bar', $view['entity_two']->vars['choices']['Bar']->value);
+    }
 }

composer.json

@@ -29,7 +29,7 @@
         "symfony/stopwatch": "^3.4|^4.0|^5.0",
         "symfony/config": "^4.2|^5.0",
         "symfony/dependency-injection": "^3.4|^4.0|^5.0",
-        "symfony/form": "^4.4.11|^5.0.11",
+        "symfony/form": "^4.4.41|^5.0.11",
         "symfony/http-kernel": "^4.3.7",
         "symfony/messenger": "^4.4|^5.0",
         "symfony/property-access": "^3.4|^4.0|^5.0",