diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

minor #30966 [DoctrineBridge] Deprecated using IdReader when optimization is not possible (HeahDude) 

This PR was merged into the 4.3-dev branch.

Discussion
----------

[DoctrineBridge] Deprecated using IdReader when optimization is not possible

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | ~
| License       | MIT
| Doc PR        | ~

Follow up of #30962. (Review only the second commit until #30962 is merged).

Commits
-------

a234c8913e [DoctrineBridge] Deprecated using IdReader when optimization is not possible
This commit is contained in:
Christophe Coevoet 2019-04-07 15:40:33 +02:00
parent c274dffc8b a234c8913e
commit c68ef4648c
6 changed files with 58 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
UPGRADE-4.3.md
View File

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

3
UPGRADE-5.0.md
View File

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

1
src/Symfony/Bridge/Doctrine/CHANGELOG.md
View File

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

13
src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
View File

@ -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()) {

18
src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
View File

@ -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([

71
src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
View File

@ -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);
}
}