diff --git a/UPGRADE-4.3.md b/UPGRADE-4.3.md index 4cbf221623..f67720fae0 100644 --- a/UPGRADE-4.3.md +++ b/UPGRADE-4.3.md @@ -38,6 +38,12 @@ DependencyInjection env(NAME): '1.5' ``` +Doctrine Bridge +--------------- + + * Passing an `IdReader` to the `DoctrineChoiceLoader` when the query cannot be optimized with single id field has been deprecated, pass `null` instead + * Not passing an `IdReader` to the `DoctrineChoiceLoader` when the query can be optimized with single id field has been deprecated + EventDispatcher --------------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index aa44884623..7e338df2f4 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -60,6 +60,9 @@ DoctrineBridge * Deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`, an instance of `EntityManagerInterface` should be injected instead + * Passing an `IdReader` to the `DoctrineChoiceLoader` when the query cannot be optimized with single id field will throw an exception, pass `null` instead + * Not passing an `IdReader` to the `DoctrineChoiceLoader` when the query can be optimized with single id field will throw an exception + DomCrawler ---------- diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md index 27cabfa558..3bcf9a77df 100644 --- a/src/Symfony/Bridge/Doctrine/CHANGELOG.md +++ b/src/Symfony/Bridge/Doctrine/CHANGELOG.md @@ -6,6 +6,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 + * deprecated passing an `IdReader` to the `DoctrineChoiceLoader` when entities have a composite id 4.2.0 ----- diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php index fd891abb34..cd040d12a9 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php @@ -42,18 +42,25 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface * * @param ObjectManager $manager The object manager * @param string $class The class name of the loaded objects - * @param IdReader $idReader The reader for the object IDs + * @param IdReader|null $idReader The reader for the object IDs * @param EntityLoaderInterface|null $objectLoader The objects loader */ public function __construct(ObjectManager $manager, string $class, IdReader $idReader = null, EntityLoaderInterface $objectLoader = null) { $classMetadata = $manager->getClassMetadata($class); + if ($idReader && !$idReader->isSingleId()) { + @trigger_error(sprintf('Passing an instance of "%s" to "%s" with an entity class "%s" that has a composite id is deprecated since Symfony 4.3 and will throw an exception in 5.0.', IdReader::class, __CLASS__, $class), E_USER_DEPRECATED); + + // In Symfony 5.0 + // throw new \InvalidArgumentException(sprintf('The second argument `$idReader` of "%s" must be null when the query cannot be optimized because of composite id fields.', __METHOD__)); + } + 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); + @trigger_error(sprintf('Not explicitly passing an instance of "%s" to "%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__, $class), E_USER_DEPRECATED); } else { $idReader = null; } @@ -93,7 +100,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface // Optimize performance for single-field identifiers. We already // know that the IDs are used as values - $optimize = null === $value || \is_array($value) && $value[0] === $this->idReader; + $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()) { diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index 03bf6a0ba5..88f9cf9101 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php @@ -163,13 +163,10 @@ abstract class DoctrineType extends AbstractType implements ResetInterface }; $choiceName = function (Options $options) { - /** @var IdReader $idReader */ - $idReader = $options['id_reader']; - // If the object has a single-column, numeric ID, use that ID as // field name. We can only use numeric IDs as names, as we cannot // guarantee that a non-numeric ID contains a valid form name - if ($idReader->isIntId()) { + if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isIntId()) { return [__CLASS__, 'createChoiceName']; } @@ -181,12 +178,9 @@ abstract class DoctrineType extends AbstractType implements ResetInterface // are indexed by an incrementing integer. // Use the ID/incrementing integer as choice value. $choiceValue = function (Options $options) { - /** @var IdReader $idReader */ - $idReader = $options['id_reader']; - // If the entity has a single-column ID, use that ID as value - if ($idReader->isSingleId()) { - return [$idReader, 'getIdValue']; + if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isSingleId()) { + return [$options['id_reader'], 'getIdValue']; } // Otherwise, an incrementing integer is used as value automatically @@ -240,7 +234,11 @@ abstract class DoctrineType extends AbstractType implements ResetInterface $this->idReaders[$hash] = new IdReader($options['em'], $classMetadata); } - return $this->idReaders[$hash]; + if ($this->idReaders[$hash]->isSingleId()) { + return $this->idReaders[$hash]; + } + + return null; }; $resolver->setDefaults([ diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php index 2fbb07d7f3..5a5fba5afa 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php @@ -81,6 +81,11 @@ class DoctrineChoiceLoaderTest extends TestCase $this->idReader = $this->getMockBuilder('Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader') ->disableOriginalConstructor() ->getMock(); + $this->idReader->expects($this->any()) + ->method('isSingleId') + ->willReturn(true) + ; + $this->objectLoader = $this->getMockBuilder('Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface')->getMock(); $this->obj1 = (object) ['name' => 'A']; $this->obj2 = (object) ['name' => 'B']; @@ -151,7 +156,7 @@ class DoctrineChoiceLoaderTest extends TestCase $loader = new DoctrineChoiceLoader( $this->om, $this->class, - $this->idReader + null ); $choices = [$this->obj1, $this->obj2, $this->obj3]; @@ -189,10 +194,6 @@ class DoctrineChoiceLoaderTest extends TestCase $this->idReader ); - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->repository->expects($this->never()) ->method('findAll'); @@ -215,10 +216,6 @@ class DoctrineChoiceLoaderTest extends TestCase $choices = [$this->obj1, $this->obj2, $this->obj3]; $value = function (\stdClass $object) { return $object->name; }; - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->repository->expects($this->once()) ->method('findAll') ->willReturn($choices); @@ -239,10 +236,6 @@ class DoctrineChoiceLoaderTest extends TestCase $value = [$this->idReader, 'getIdValue']; - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->repository->expects($this->never()) ->method('findAll'); @@ -303,10 +296,6 @@ class DoctrineChoiceLoaderTest extends TestCase $choices = [$this->obj2, $this->obj3]; - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->idReader->expects($this->any()) ->method('getIdField') ->willReturn('idField'); @@ -343,10 +332,6 @@ class DoctrineChoiceLoaderTest extends TestCase $choices = [$this->obj1, $this->obj2, $this->obj3]; $value = function (\stdClass $object) { return $object->name; }; - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->repository->expects($this->once()) ->method('findAll') ->willReturn($choices); @@ -369,10 +354,6 @@ class DoctrineChoiceLoaderTest extends TestCase $choices = [$this->obj2, $this->obj3]; $value = [$this->idReader, 'getIdValue']; - $this->idReader->expects($this->any()) - ->method('isSingleId') - ->willReturn(true); - $this->idReader->expects($this->any()) ->method('getIdField') ->willReturn('idField'); @@ -398,7 +379,7 @@ class DoctrineChoiceLoaderTest extends TestCase /** * @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. + * @expectedDeprecation Not explicitly passing an instance of "Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader" to "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader" 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() { @@ -445,14 +426,6 @@ class DoctrineChoiceLoaderTest extends TestCase $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'); @@ -461,13 +434,29 @@ class DoctrineChoiceLoaderTest extends TestCase ->with('idField', ['1']) ->willReturn($choices); - $this->idReader->expects($this->any()) - ->method('getIdValue') - ->willReturnMap([ - [$obj1, '1'], - [$obj2, '2'], - ]); - $this->assertSame([$obj1], $loader->loadChoicesForValues(['1'])); } + + /** + * @group legacy + * + * @deprecationMessage Passing an instance of "Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader" to "Symfony\Bridge\Doctrine\Form\ChoiceList\DoctrineChoiceLoader" with an entity class "stdClass" that has a composite id is deprecated since Symfony 4.3 and will throw an exception in 5.0. + */ + public function testPassingIdReaderWithoutSingleIdEntity() + { + $idReader = $this->createMock(IdReader::class); + $idReader->expects($this->once()) + ->method('isSingleId') + ->willReturn(false) + ; + + $loader = new DoctrineChoiceLoader( + $this->om, + $this->class, + $idReader, + $this->objectLoader + ); + + $this->assertInstanceOf(DoctrineChoiceLoader::class, $loader); + } }