Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c68ef46

Browse filesBrowse files
stofstof
committed
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 ------- a234c89 [DoctrineBridge] Deprecated using IdReader when optimization is not possible
2 parents c274dff + a234c89 commit c68ef46
Copy full SHA for c68ef46

File tree

Expand file treeCollapse file tree

6 files changed

+58
-54
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+58
-54
lines changed

‎UPGRADE-4.3.md

Copy file name to clipboardExpand all lines: UPGRADE-4.3.md
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ DependencyInjection
3838
env(NAME): '1.5'
3939
```
4040
41+
Doctrine Bridge
42+
---------------
43+
44+
* Passing an `IdReader` to the `DoctrineChoiceLoader` when the query cannot be optimized with single id field has been deprecated, pass `null` instead
45+
* Not passing an `IdReader` to the `DoctrineChoiceLoader` when the query can be optimized with single id field has been deprecated
46+
4147
EventDispatcher
4248
---------------
4349

‎UPGRADE-5.0.md

Copy file name to clipboardExpand all lines: UPGRADE-5.0.md
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ DoctrineBridge
6060

6161
* Deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`, an instance of `EntityManagerInterface` should be
6262
injected instead
63+
* Passing an `IdReader` to the `DoctrineChoiceLoader` when the query cannot be optimized with single id field will throw an exception, pass `null` instead
64+
* Not passing an `IdReader` to the `DoctrineChoiceLoader` when the query can be optimized with single id field will throw an exception
65+
6366

6467
DomCrawler
6568
----------

‎src/Symfony/Bridge/Doctrine/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ CHANGELOG
66

77
* changed guessing of DECIMAL to set the `input` option of `NumberType` to string
88
* deprecated not passing an `IdReader` to the `DoctrineChoiceLoader` when query can be optimized with a single id field
9+
* deprecated passing an `IdReader` to the `DoctrineChoiceLoader` when entities have a composite id
910

1011
4.2.0
1112
-----

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php
+10-3Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,25 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
4242
*
4343
* @param ObjectManager $manager The object manager
4444
* @param string $class The class name of the loaded objects
45-
* @param IdReader $idReader The reader for the object IDs
45+
* @param IdReader|null $idReader The reader for the object IDs
4646
* @param EntityLoaderInterface|null $objectLoader The objects loader
4747
*/
4848
public function __construct(ObjectManager $manager, string $class, IdReader $idReader = null, EntityLoaderInterface $objectLoader = null)
4949
{
5050
$classMetadata = $manager->getClassMetadata($class);
5151

52+
if ($idReader && !$idReader->isSingleId()) {
53+
@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);
54+
55+
// In Symfony 5.0
56+
// 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__));
57+
}
58+
5259
if ((5 > \func_num_args() || false !== func_get_arg(4)) && null === $idReader) {
5360
$idReader = new IdReader($manager, $classMetadata);
5461

5562
if ($idReader->isSingleId()) {
56-
@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);
63+
@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);
5764
} else {
5865
$idReader = null;
5966
}
@@ -93,7 +100,7 @@ public function loadValuesForChoices(array $choices, $value = null)
93100

94101
// Optimize performance for single-field identifiers. We already
95102
// know that the IDs are used as values
96-
$optimize = null === $value || \is_array($value) && $value[0] === $this->idReader;
103+
$optimize = $this->idReader && (null === $value || \is_array($value) && $value[0] === $this->idReader);
97104

98105
// Attention: This optimization does not check choices for existence
99106
if ($optimize && !$this->choiceList && $this->idReader->isSingleId()) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
+8-10Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,10 @@ public function configureOptions(OptionsResolver $resolver)
163163
};
164164

165165
$choiceName = function (Options $options) {
166-
/** @var IdReader $idReader */
167-
$idReader = $options['id_reader'];
168-
169166
// If the object has a single-column, numeric ID, use that ID as
170167
// field name. We can only use numeric IDs as names, as we cannot
171168
// guarantee that a non-numeric ID contains a valid form name
172-
if ($idReader->isIntId()) {
169+
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isIntId()) {
173170
return [__CLASS__, 'createChoiceName'];
174171
}
175172

@@ -181,12 +178,9 @@ public function configureOptions(OptionsResolver $resolver)
181178
// are indexed by an incrementing integer.
182179
// Use the ID/incrementing integer as choice value.
183180
$choiceValue = function (Options $options) {
184-
/** @var IdReader $idReader */
185-
$idReader = $options['id_reader'];
186-
187181
// If the entity has a single-column ID, use that ID as value
188-
if ($idReader->isSingleId()) {
189-
return [$idReader, 'getIdValue'];
182+
if ($options['id_reader'] instanceof IdReader && $options['id_reader']->isSingleId()) {
183+
return [$options['id_reader'], 'getIdValue'];
190184
}
191185

192186
// Otherwise, an incrementing integer is used as value automatically
@@ -240,7 +234,11 @@ public function configureOptions(OptionsResolver $resolver)
240234
$this->idReaders[$hash] = new IdReader($options['em'], $classMetadata);
241235
}
242236

243-
return $this->idReaders[$hash];
237+
if ($this->idReaders[$hash]->isSingleId()) {
238+
return $this->idReaders[$hash];
239+
}
240+
241+
return null;
244242
};
245243

246244
$resolver->setDefaults([

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
+30-41Lines changed: 30 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ protected function setUp()
8181
$this->idReader = $this->getMockBuilder('Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader')
8282
->disableOriginalConstructor()
8383
->getMock();
84+
$this->idReader->expects($this->any())
85+
->method('isSingleId')
86+
->willReturn(true)
87+
;
88+
8489
$this->objectLoader = $this->getMockBuilder('Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface')->getMock();
8590
$this->obj1 = (object) ['name' => 'A'];
8691
$this->obj2 = (object) ['name' => 'B'];
@@ -151,7 +156,7 @@ public function testLoadValuesForChoices()
151156
$loader = new DoctrineChoiceLoader(
152157
$this->om,
153158
$this->class,
154-
$this->idReader
159+
null
155160
);
156161

157162
$choices = [$this->obj1, $this->obj2, $this->obj3];
@@ -189,10 +194,6 @@ public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
189194
$this->idReader
190195
);
191196

192-
$this->idReader->expects($this->any())
193-
->method('isSingleId')
194-
->willReturn(true);
195-
196197
$this->repository->expects($this->never())
197198
->method('findAll');
198199

@@ -215,10 +216,6 @@ public function testLoadValuesForChoicesLoadsIfSingleIntIdAndValueGiven()
215216
$choices = [$this->obj1, $this->obj2, $this->obj3];
216217
$value = function (\stdClass $object) { return $object->name; };
217218

218-
$this->idReader->expects($this->any())
219-
->method('isSingleId')
220-
->willReturn(true);
221-
222219
$this->repository->expects($this->once())
223220
->method('findAll')
224221
->willReturn($choices);
@@ -239,10 +236,6 @@ public function testLoadValuesForChoicesDoesNotLoadIfValueIsIdReader()
239236

240237
$value = [$this->idReader, 'getIdValue'];
241238

242-
$this->idReader->expects($this->any())
243-
->method('isSingleId')
244-
->willReturn(true);
245-
246239
$this->repository->expects($this->never())
247240
->method('findAll');
248241

@@ -303,10 +296,6 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfSingleIntId()
303296

304297
$choices = [$this->obj2, $this->obj3];
305298

306-
$this->idReader->expects($this->any())
307-
->method('isSingleId')
308-
->willReturn(true);
309-
310299
$this->idReader->expects($this->any())
311300
->method('getIdField')
312301
->willReturn('idField');
@@ -343,10 +332,6 @@ public function testLoadChoicesForValuesLoadsAllIfSingleIntIdAndValueGiven()
343332
$choices = [$this->obj1, $this->obj2, $this->obj3];
344333
$value = function (\stdClass $object) { return $object->name; };
345334

346-
$this->idReader->expects($this->any())
347-
->method('isSingleId')
348-
->willReturn(true);
349-
350335
$this->repository->expects($this->once())
351336
->method('findAll')
352337
->willReturn($choices);
@@ -369,10 +354,6 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueIsIdReader()
369354
$choices = [$this->obj2, $this->obj3];
370355
$value = [$this->idReader, 'getIdValue'];
371356

372-
$this->idReader->expects($this->any())
373-
->method('isSingleId')
374-
->willReturn(true);
375-
376357
$this->idReader->expects($this->any())
377358
->method('getIdField')
378359
->willReturn('idField');
@@ -398,7 +379,7 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueIsIdReader()
398379
/**
399380
* @group legacy
400381
*
401-
* @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.
382+
* @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.
402383
*/
403384
public function testLoaderWithoutIdReaderCanBeOptimized()
404385
{
@@ -445,14 +426,6 @@ public function testLoaderWithoutIdReaderCanBeOptimized()
445426

446427
$choices = [$obj1, $obj2];
447428

448-
$this->idReader->expects($this->any())
449-
->method('isSingleId')
450-
->willReturn(true);
451-
452-
$this->idReader->expects($this->any())
453-
->method('getIdField')
454-
->willReturn('idField');
455-
456429
$this->repository->expects($this->never())
457430
->method('findAll');
458431

@@ -461,13 +434,29 @@ public function testLoaderWithoutIdReaderCanBeOptimized()
461434
->with('idField', ['1'])
462435
->willReturn($choices);
463436

464-
$this->idReader->expects($this->any())
465-
->method('getIdValue')
466-
->willReturnMap([
467-
[$obj1, '1'],
468-
[$obj2, '2'],
469-
]);
470-
471437
$this->assertSame([$obj1], $loader->loadChoicesForValues(['1']));
472438
}
439+
440+
/**
441+
* @group legacy
442+
*
443+
* @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.
444+
*/
445+
public function testPassingIdReaderWithoutSingleIdEntity()
446+
{
447+
$idReader = $this->createMock(IdReader::class);
448+
$idReader->expects($this->once())
449+
->method('isSingleId')
450+
->willReturn(false)
451+
;
452+
453+
$loader = new DoctrineChoiceLoader(
454+
$this->om,
455+
$this->class,
456+
$idReader,
457+
$this->objectLoader
458+
);
459+
460+
$this->assertInstanceOf(DoctrineChoiceLoader::class, $loader);
461+
}
473462
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.