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 48233b0

Browse filesBrowse files
fabpotfabpot
committed
bug #31463 [DI] default to service id - *not* FQCN - when building tagged locators (nicolas-grekas)
This PR was merged into the 4.3 branch. Discussion ---------- [DI] default to service id - *not* FQCN - when building tagged locators | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | no | Fixed tickets | - | License | MIT | Doc PR | - While reviewing #30926 I realized that defaulting to the FQCN is a shortcut that isn't useful enough. Defaulting to the service id provides the same experience in practice because service ids are FQCN by default. But when they aren't, the service id is the proper index to default to when building the locator. Commits ------- 52e827c [DI] default to service id - *not* FQCN - when building tagged locators
2 parents 3865b78 + 52e827c commit 48233b0
Copy full SHA for 48233b0

File tree

3 files changed

+58
-28
lines changed
Filter options

3 files changed

+58
-28
lines changed

‎src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php
+10-6Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,26 @@ class TaggedIteratorArgument extends IteratorArgument
2121
private $tag;
2222
private $indexAttribute;
2323
private $defaultIndexMethod;
24-
private $useFqcnAsFallback = false;
24+
private $needsIndexes = false;
2525

2626
/**
2727
* @param string $tag The name of the tag identifying the target services
2828
* @param string|null $indexAttribute The name of the attribute that defines the key referencing each service in the tagged collection
2929
* @param string|null $defaultIndexMethod The static method that should be called to get each service's key when their tag doesn't define the previous attribute
30-
* @param bool $useFqcnAsFallback Whether the FQCN of the service should be used as index when neither the attribute nor the method are defined
30+
* @param bool $needsIndexes Whether indexes are required and should be generated when computing the map
3131
*/
32-
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $useFqcnAsFallback = false)
32+
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false)
3333
{
3434
parent::__construct([]);
3535

36+
if (null === $indexAttribute && $needsIndexes) {
37+
$indexAttribute = preg_match('/[^.]++$/', $tag, $m) ? $m[0] : $tag;
38+
}
39+
3640
$this->tag = $tag;
3741
$this->indexAttribute = $indexAttribute;
3842
$this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
39-
$this->useFqcnAsFallback = $useFqcnAsFallback;
43+
$this->needsIndexes = $needsIndexes;
4044
}
4145

4246
public function getTag()
@@ -54,8 +58,8 @@ public function getDefaultIndexMethod(): ?string
5458
return $this->defaultIndexMethod;
5559
}
5660

57-
public function useFqcnAsFallback(): bool
61+
public function needsIndexes(): bool
5862
{
59-
return $this->useFqcnAsFallback;
63+
return $this->needsIndexes;
6064
}
6165
}

‎src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ trait PriorityTaggedServiceTrait
4141
*/
4242
private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
4343
{
44-
$indexAttribute = $defaultIndexMethod = $useFqcnAsFallback = null;
44+
$indexAttribute = $defaultIndexMethod = $needsIndexes = null;
4545

4646
if ($tagName instanceof TaggedIteratorArgument) {
4747
$indexAttribute = $tagName->getIndexAttribute();
4848
$defaultIndexMethod = $tagName->getDefaultIndexMethod();
49-
$useFqcnAsFallback = $tagName->useFqcnAsFallback();
49+
$needsIndexes = $tagName->needsIndexes();
5050
$tagName = $tagName->getTag();
5151
}
5252

@@ -55,7 +55,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
5555
foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
5656
$priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
5757

58-
if (null === $indexAttribute && !$useFqcnAsFallback) {
58+
if (null === $indexAttribute && !$needsIndexes) {
5959
$services[$priority][] = new Reference($serviceId);
6060

6161
continue;
@@ -77,8 +77,8 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
7777
$class = $r->name;
7878

7979
if (!$r->hasMethod($defaultIndexMethod)) {
80-
if ($useFqcnAsFallback) {
81-
$services[$priority][$class] = new TypedReference($serviceId, $class);
80+
if ($needsIndexes) {
81+
$services[$priority][$serviceId] = new TypedReference($serviceId, $class);
8282

8383
continue;
8484
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php
+43-17Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -292,22 +292,22 @@ public function testTaggedServiceWithIndexAttributeAndDefaultMethod()
292292
public function testTaggedServiceLocatorWithIndexAttribute()
293293
{
294294
$container = new ContainerBuilder();
295-
$container->register(BarTagClass::class)
295+
$container->register('bar_tag', BarTagClass::class)
296296
->setPublic(true)
297297
->addTag('foo_bar', ['foo' => 'bar'])
298298
;
299-
$container->register(FooTagClass::class)
299+
$container->register('foo_tag', FooTagClass::class)
300300
->setPublic(true)
301301
->addTag('foo_bar')
302302
;
303-
$container->register(FooBarTaggedClass::class)
303+
$container->register('foo_bar_tagged', FooBarTaggedClass::class)
304304
->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo')))
305305
->setPublic(true)
306306
;
307307

308308
$container->compile();
309309

310-
$s = $container->get(FooBarTaggedClass::class);
310+
$s = $container->get('foo_bar_tagged');
311311

312312
/** @var ServiceLocator $serviceLocator */
313313
$serviceLocator = $s->getParam();
@@ -317,28 +317,28 @@ public function testTaggedServiceLocatorWithIndexAttribute()
317317
'bar' => $serviceLocator->get('bar'),
318318
'foo_tag_class' => $serviceLocator->get('foo_tag_class'),
319319
];
320-
$this->assertSame(['bar' => $container->get(BarTagClass::class), 'foo_tag_class' => $container->get(FooTagClass::class)], $same);
320+
$this->assertSame(['bar' => $container->get('bar_tag'), 'foo_tag_class' => $container->get('foo_tag')], $same);
321321
}
322322

323323
public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
324324
{
325325
$container = new ContainerBuilder();
326-
$container->register(BarTagClass::class)
326+
$container->register('bar_tag', BarTagClass::class)
327327
->setPublic(true)
328328
->addTag('foo_bar')
329329
;
330-
$container->register(FooTagClass::class)
330+
$container->register('foo_tag', FooTagClass::class)
331331
->setPublic(true)
332332
->addTag('foo_bar', ['foo' => 'foo'])
333333
;
334-
$container->register(FooBarTaggedClass::class)
334+
$container->register('foo_bar_tagged', FooBarTaggedClass::class)
335335
->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar')))
336336
->setPublic(true)
337337
;
338338

339339
$container->compile();
340340

341-
$s = $container->get(FooBarTaggedClass::class);
341+
$s = $container->get('foo_bar_tagged');
342342

343343
/** @var ServiceLocator $serviceLocator */
344344
$serviceLocator = $s->getParam();
@@ -348,33 +348,59 @@ public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
348348
'bar_tab_class_with_defaultmethod' => $serviceLocator->get('bar_tab_class_with_defaultmethod'),
349349
'foo' => $serviceLocator->get('foo'),
350350
];
351-
$this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get(BarTagClass::class), 'foo' => $container->get(FooTagClass::class)], $same);
351+
$this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get('bar_tag'), 'foo' => $container->get('foo_tag')], $same);
352352
}
353353

354-
public function testTaggedServiceLocatorWithFqcnFallback()
354+
public function testTaggedServiceLocatorWithFallback()
355355
{
356356
$container = new ContainerBuilder();
357-
$container->register(BarTagClass::class)
357+
$container->register('bar_tag', BarTagClass::class)
358358
->setPublic(true)
359359
->addTag('foo_bar')
360360
;
361-
$container->register(FooBarTaggedClass::class)
361+
$container->register('foo_bar_tagged', FooBarTaggedClass::class)
362362
->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', null, null, true)))
363363
->setPublic(true)
364364
;
365365

366366
$container->compile();
367367

368-
$s = $container->get(FooBarTaggedClass::class);
368+
$s = $container->get('foo_bar_tagged');
369369

370370
/** @var ServiceLocator $serviceLocator */
371371
$serviceLocator = $s->getParam();
372372
$this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator)));
373373

374-
$same = [
375-
BarTagClass::class => $serviceLocator->get(BarTagClass::class),
374+
$expected = [
375+
'bar_tag' => $container->get('bar_tag'),
376+
];
377+
$this->assertSame($expected, ['bar_tag' => $serviceLocator->get('bar_tag')]);
378+
}
379+
380+
public function testTaggedServiceLocatorWithDefaultIndex()
381+
{
382+
$container = new ContainerBuilder();
383+
$container->register('bar_tag', BarTagClass::class)
384+
->setPublic(true)
385+
->addTag('app.foo_bar', ['foo_bar' => 'baz'])
386+
;
387+
$container->register('foo_bar_tagged', FooBarTaggedClass::class)
388+
->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('app.foo_bar', null, null, true)))
389+
->setPublic(true)
390+
;
391+
392+
$container->compile();
393+
394+
$s = $container->get('foo_bar_tagged');
395+
396+
/** @var ServiceLocator $serviceLocator */
397+
$serviceLocator = $s->getParam();
398+
$this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator)));
399+
400+
$expected = [
401+
'baz' => $container->get('bar_tag'),
376402
];
377-
$this->assertSame([BarTagClass::class => $container->get(BarTagClass::class)], $same);
403+
$this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]);
378404
}
379405
}
380406

0 commit comments

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