[Form] Fix same choice loader with different choice values

symfony · HeahDude · Apr 18, 2022 · Apr 18, 2022 · 71048a6a79c89693ba745254f6e424d62e4b4935
commit 71048a6a79c89693ba745254f6e424d62e4b4935
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
@@ -152,8 +152,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()

diff --git a/src/Symfony/Bridge/Doctrine/composer.json b/src/Symfony/Bridge/Doctrine/composer.json
@@ -30,7 +30,7 @@
         "symfony/cache": "^5.4|^6.0",
         "symfony/config": "^4.4|^5.0|^6.0",
         "symfony/dependency-injection": "^4.4|^5.0|^6.0",
-        "symfony/form": "^5.1.3|^6.0",
+        "symfony/form": "^5.4.8|^6.0",
         "symfony/http-kernel": "^5.0|^6.0",
         "symfony/messenger": "^4.4|^5.0|^6.0",
         "symfony/doctrine-messenger": "^5.1|^6.0",

@@ -38,6 +38,11 @@ class LazyChoiceList implements ChoiceListInterface
      */
     private $value;
 
+    /**
+     * @var ChoiceListInterface|null
+     */
+    private $loadedList;
+
     /**
      * Creates a lazily-loaded list using the given loader.
      *
@@ -58,38 +63,58 @@ public function __construct(ChoiceLoaderInterface $loader, callable $value = nul
      */
     public function getChoices()
     {
-        return $this->loader->loadChoiceList($this->value)->getChoices();
+        if ($this->loadedList) {
+            return $this->loadedList->getChoices();
+        }
+
+        return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getChoices();
     }
 
     /**
      * {@inheritdoc}
      */
     public function getValues()
     {
-        return $this->loader->loadChoiceList($this->value)->getValues();
+        if ($this->loadedList) {
+            return $this->loadedList->getValues();
+        }
+
+        return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getValues();
     }
 
     /**
      * {@inheritdoc}
      */
     public function getStructuredValues()
     {
-        return $this->loader->loadChoiceList($this->value)->getStructuredValues();
+        if ($this->loadedList) {
+            return $this->loadedList->getStructuredValues();
+        }
+
+        return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getStructuredValues();
     }
 
     /**
      * {@inheritdoc}
      */
     public function getOriginalKeys()
     {
-        return $this->loader->loadChoiceList($this->value)->getOriginalKeys();
+        if ($this->loadedList) {
+            return $this->loadedList->getOriginalKeys();
+        }
+
+        return ($this->loadedList = $this->loader->loadChoiceList($this->value))->getOriginalKeys();
     }
 
     /**
      * {@inheritdoc}
      */
     public function getChoicesForValues(array $values)
     {
+        if ($this->loadedList) {
+            return $this->loadedList->getChoicesForValues($values);
+        }
+
         return $this->loader->loadChoicesForValues($values, $this->value);
     }
 
@@ -98,6 +123,10 @@ public function getChoicesForValues(array $values)
      */
     public function getValuesForChoices(array $choices)
     {
+        if ($this->loadedList) {
+            return $this->loadedList->getValuesForChoices($choices);
+        }
+
         return $this->loader->loadValuesForChoices($choices, $this->value);
     }
 }
@@ -20,11 +20,11 @@
 abstract class AbstractChoiceLoader implements ChoiceLoaderInterface
 {
     /**
-     * The loaded choice list.
+     * The loaded choices.
      *
-     * @var ArrayChoiceList
+     * @var iterable|null
      */
-    private $choiceList;
+    private $choices;
 
     /**
      * @final
@@ -33,7 +33,7 @@ abstract class AbstractChoiceLoader implements ChoiceLoaderInterface
      */
     public function loadChoiceList(callable $value = null): ChoiceListInterface
     {
-        return $this->choiceList ?? ($this->choiceList = new ArrayChoiceList($this->loadChoices(), $value));
+        return new ArrayChoiceList($this->choices ?? $this->choices = $this->loadChoices(), $value);
     }
 
     /**
@@ -45,10 +45,6 @@ public function loadChoicesForValues(array $values, callable $value = null)
             return [];
         }
 
-        if ($this->choiceList) {
-            return $this->choiceList->getChoicesForValues($values);
-        }
-
         return $this->doLoadChoicesForValues($values, $value);
     }
 
@@ -66,10 +62,6 @@ public function loadValuesForChoices(array $choices, callable $value = null)
             return array_map($value, $choices);
         }
 
-        if ($this->choiceList) {
-            return $this->choiceList->getValuesForChoices($choices);
-        }
-
         return $this->doLoadValuesForChoices($choices);
     }
 

@@ -98,7 +98,7 @@ public function testGetChoicesForValuesForwardsCallIfListNotLoaded()
 
         $this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
         $this->assertSame(['foo', 'bar'], $list->getChoicesForValues(['a', 'b']));
-        $this->assertSame(3, $calls);
+        $this->assertSame(6, $calls);
     }
 
     public function testGetChoicesForValuesUsesLoadedList()

@@ -67,11 +67,18 @@ public function testLoadChoiceList()
         $this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
     }
 
-    public function testLoadChoiceListOnlyOnce()
+    public function testLoadChoicesOnlyOnce()
     {
-        $loadedChoiceList = self::$loader->loadChoiceList(self::$value);
+        $calls = 0;
+        $loader = new CallbackChoiceLoader(function () use (&$calls) {
+            ++$calls;
 
-        $this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
+            return [1];
+        });
+        $loadedChoiceList = $loader->loadChoiceList();
+
+        $this->assertNotSame($loadedChoiceList, $loader->loadChoiceList());
+        $this->assertSame(1, $calls);
     }
 
     public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

@@ -68,11 +68,19 @@ public function testLoadChoiceList()
         $this->assertInstanceOf(ChoiceListInterface::class, self::$loader->loadChoiceList(self::$value));
     }
 
-    public function testLoadChoiceListOnlyOnce()
+    public function testLoadChoicesOnlyOnce()
     {
-        $loadedChoiceList = self::$loader->loadChoiceList(self::$value);
+        $calls = 0;
+        $loader = new IntlCallbackChoiceLoader(function () use (&$calls) {
+            ++$calls;
 
-        $this->assertSame($loadedChoiceList, self::$loader->loadChoiceList(self::$value));
+            return self::$choices;
+        });
+
+        $loadedChoiceList = $loader->loadChoiceList(self::$value);
+
+        $this->assertNotSame($loadedChoiceList, $loader->loadChoiceList(self::$value));
+        $this->assertSame(1, $calls);
     }
 
     public function testLoadChoicesForValuesLoadsChoiceListOnFirstCall()

@@ -2264,4 +2264,32 @@ public function testUsingDeprecatedChoiceListFactory()
 
         new ChoiceType(new DeprecatedChoiceListFactory());
     }
+
+    public function testWithSameLoaderAndDifferentChoiceValueCallbacks()
+    {
+        $choiceLoader = new CallbackChoiceLoader(function () {
+            return [1, 2, 3];
+        });
+
+        $view = $this->factory->create(FormTypeTest::TESTED_TYPE)
+            ->add('choice_one', self::TESTED_TYPE, [
+                'choice_loader' => $choiceLoader,
+            ])
+            ->add('choice_two', self::TESTED_TYPE, [
+                'choice_loader' => $choiceLoader,
+                'choice_value' => function ($choice) {
+                    return $choice ? (string) $choice * 10 : '';
+                },
+            ])
+            ->createView()
+        ;
+
+        $this->assertSame('1', $view['choice_one']->vars['choices'][0]->value);
+        $this->assertSame('2', $view['choice_one']->vars['choices'][1]->value);
+        $this->assertSame('3', $view['choice_one']->vars['choices'][2]->value);
+
+        $this->assertSame('10', $view['choice_two']->vars['choices'][0]->value);
+        $this->assertSame('20', $view['choice_two']->vars['choices'][1]->value);
+        $this->assertSame('30', $view['choice_two']->vars['choices'][2]->value);
+    }
 }