Skip to content

Commit cafa38d

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Fix merging explicit tags and #[AsTaggeditem]
1 parent 98af8bb commit cafa38d

File tree

3 files changed

+87
-66
lines changed

3 files changed

+87
-66
lines changed

‎Attribute/AsTaggedItem.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
class AsTaggedItem
2121
{
2222
/**
23-
* @param string|null $index The property or method to use to index the item in the iterator/locator
24-
* @param int|null $priority The priority of the item; the higher the number, the earlier the tagged service will be located in the iterator/locator
23+
* @param string|null $index The index at which the service will be found when consuming tagged iterators/locators
24+
* @param int|null $priority The priority of the service in iterators/locators; the higher the number, the earlier it will
2525
*/
2626
public function __construct(
2727
public ?string $index = null,

‎Compiler/PriorityTaggedServiceTrait.php‎

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -65,61 +65,69 @@ private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagNam
6565
$class = $definition->getClass();
6666
$class = $container->getParameterBag()->resolveValue($class) ?: null;
6767
$reflector = null !== $class ? $container->getReflectionClass($class) : null;
68-
$checkTaggedItem = !$definition->hasTag($definition->isAutoconfigured() ? 'container.ignore_attributes' : $tagName);
68+
$loadFromDefaultMethods = $reflector && null !== $defaultPriorityMethod;
69+
$phpAttributes = $definition->isAutoconfigured() && !$definition->hasTag('container.ignore_attributes') ? $reflector?->getAttributes(AsTaggedItem::class) : [];
70+
71+
foreach ($phpAttributes ??= [] as $i => $attribute) {
72+
$attribute = $attribute->newInstance();
73+
$phpAttributes[$i] = [
74+
'priority' => $attribute->priority,
75+
$indexAttribute ?? '' => $attribute->index,
76+
];
77+
if (null === $defaultPriority) {
78+
$defaultPriority = $attribute->priority ?? 0;
79+
$defaultIndex = $attribute->index;
80+
}
81+
}
82+
if (1 >= \count($phpAttributes)) {
83+
$phpAttributes = [];
84+
}
85+
86+
for ($i = 0; $i < \count($attributes); ++$i) {
87+
if (!($attribute = $attributes[$i]) && $phpAttributes) {
88+
array_splice($attributes, $i--, 1, $phpAttributes);
89+
continue;
90+
}
6991

70-
foreach ($attributes as $attribute) {
7192
$index = $priority = null;
7293

7394
if (isset($attribute['priority'])) {
7495
$priority = $attribute['priority'];
75-
} elseif (null === $defaultPriority && $defaultPriorityMethod && $reflector) {
76-
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority', $checkTaggedItem);
96+
} elseif ($loadFromDefaultMethods) {
97+
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority') ?? $defaultPriority;
98+
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute) ?? $defaultIndex;
99+
$loadFromDefaultMethods = false;
77100
}
78101
$priority ??= $defaultPriority ??= 0;
79102

80103
if (null === $indexAttribute && !$defaultIndexMethod && !$needsIndexes) {
81-
$services[] = [$priority, ++$i, null, $serviceId, null];
104+
$services[] = [$priority, $i, null, $serviceId, null];
82105
continue 2;
83106
}
84107

85108
if (null !== $indexAttribute && isset($attribute[$indexAttribute])) {
86109
$index = $parameterBag->resolveValue($attribute[$indexAttribute]);
87110
}
88-
if (null === $index && null === $defaultIndex && $defaultPriorityMethod && $reflector) {
89-
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute, $checkTaggedItem);
111+
if (null === $index && $loadFromDefaultMethods) {
112+
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority') ?? $defaultPriority;
113+
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute) ?? $defaultIndex;
114+
$loadFromDefaultMethods = false;
90115
}
91116
$index ??= $defaultIndex ??= $definition->getTag('container.decorator')[0]['id'] ?? $serviceId;
92117

93-
$services[] = [$priority, ++$i, $index, $serviceId, $class];
94-
}
95-
96-
if ($reflector) {
97-
$attributes = $reflector->getAttributes(AsTaggedItem::class);
98-
$attributeCount = \count($attributes);
99-
100-
foreach ($attributes as $attribute) {
101-
$instance = $attribute->newInstance();
102-
103-
if (!$instance->index && 1 < $attributeCount) {
104-
throw new InvalidArgumentException(\sprintf('Attribute "%s" on class "%s" cannot have an empty index when repeated.', AsTaggedItem::class, $class));
105-
}
106-
107-
$services[] = [$instance->priority ?? 0, ++$i, $instance->index ?? $serviceId, $serviceId, $class];
108-
}
118+
$services[] = [$priority, $i, $index, $serviceId, $class];
109119
}
110120
}
111121

112122
uasort($services, static fn ($a, $b) => $b[0] <=> $a[0] ?: $a[1] <=> $b[1]);
113123

114124
$refs = [];
115125
foreach ($services as [, , $index, $serviceId, $class]) {
116-
if (!$class) {
117-
$reference = new Reference($serviceId);
118-
} elseif ($index === $serviceId) {
119-
$reference = new TypedReference($serviceId, $class);
120-
} else {
121-
$reference = new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $index);
122-
}
126+
$reference = match (true) {
127+
!$class => new Reference($serviceId),
128+
$index === $serviceId => new TypedReference($serviceId, $class),
129+
default => new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $index),
130+
};
123131

124132
if (null === $index) {
125133
$refs[] = $reference;
@@ -137,25 +145,16 @@ private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagNam
137145
*/
138146
class PriorityTaggedServiceUtil
139147
{
140-
public static function getDefault(string $serviceId, \ReflectionClass $r, string $defaultMethod, string $tagName, ?string $indexAttribute, bool $checkTaggedItem): string|int|null
148+
public static function getDefault(string $serviceId, \ReflectionClass $r, string $defaultMethod, string $tagName, ?string $indexAttribute): string|int|null
141149
{
142-
$class = $r->getName();
143-
144-
if (!$checkTaggedItem && !$r->hasMethod($defaultMethod)) {
145-
return null;
146-
}
147-
148-
if ($checkTaggedItem && !$r->hasMethod($defaultMethod)) {
149-
foreach ($r->getAttributes(AsTaggedItem::class) as $attribute) {
150-
return 'priority' === $indexAttribute ? $attribute->newInstance()->priority : $attribute->newInstance()->index;
151-
}
152-
150+
if (!$r->hasMethod($defaultMethod)) {
153151
return null;
154152
}
155153

156154
if ($r->isInterface()) {
157155
return null;
158156
}
157+
$class = $r->name;
159158

160159
if (null !== $indexAttribute) {
161160
$service = $class !== $serviceId ? \sprintf('service "%s"', $serviceId) : 'on the corresponding service';

‎Tests/Compiler/PriorityTaggedServiceTraitTest.php‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -233,29 +233,14 @@ public function testTaggedItemAttributes()
233233
'hello' => new TypedReference('service2', HelloNamedService::class),
234234
'multi_hello_1' => new TypedReference('service6', MultiTagHelloNamedService::class),
235235
'service1' => new TypedReference('service1', FooTagClass::class),
236+
'multi_hello_0' => new TypedReference('service6', MultiTagHelloNamedService::class),
236237
];
237238

238239
$services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
239240
$this->assertSame(array_keys($expected), array_keys($services));
240241
$this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
241242
}
242243

243-
public function testTaggedItemAttributesRepeatedWithoutNameThrows()
244-
{
245-
$container = new ContainerBuilder();
246-
$container->register('service1', MultiNoNameTagHelloNamedService::class)
247-
->setAutoconfigured(true)
248-
->addTag('my_custom_tag');
249-
250-
(new ResolveInstanceofConditionalsPass())->process($container);
251-
$tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar', exclude: ['service4', 'service5']);
252-
253-
$this->expectException(InvalidArgumentException::class);
254-
$this->expectExceptionMessage('Attribute "Symfony\Component\DependencyInjection\Attribute\AsTaggedItem" on class "Symfony\Component\DependencyInjection\Tests\Compiler\MultiNoNameTagHelloNamedService" cannot have an empty index when repeated.');
255-
256-
(new PriorityTaggedServiceTraitImplementation())->test($tag, $container);
257-
}
258-
259244
public function testResolveIndexedTags()
260245
{
261246
$container = new ContainerBuilder();
@@ -283,6 +268,48 @@ public function testResolveIndexedTags()
283268
$this->assertSame(array_keys($expected), array_keys($services));
284269
$this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
285270
}
271+
272+
public function testAttributesAreMergedWithTags()
273+
{
274+
$container = new ContainerBuilder();
275+
$definition = $container->register('service_attr_first', MultiTagHelloNamedService::class);
276+
$definition->setAutoconfigured(true);
277+
$definition->addTag('my_custom_tag', ['foo' => 'z']);
278+
$definition->addTag('my_custom_tag', []);
279+
280+
(new ResolveInstanceofConditionalsPass())->process($container);
281+
282+
$priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
283+
284+
$tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar');
285+
$services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
286+
287+
$expected = [
288+
'multi_hello_2' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class),
289+
'multi_hello_1' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class),
290+
'z' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class),
291+
'multi_hello_0' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class),
292+
];
293+
$this->assertSame(array_keys($expected), array_keys($services));
294+
$this->assertEquals($expected, $services);
295+
}
296+
297+
public function testAttributesAreFallbacks()
298+
{
299+
$container = new ContainerBuilder();
300+
$definition = $container->register('service_attr_first', MultiTagHelloNamedService::class);
301+
$definition->setAutoconfigured(true);
302+
$definition->addTag('my_custom_tag', ['foo' => 'z']);
303+
304+
(new ResolveInstanceofConditionalsPass())->process($container);
305+
306+
$priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
307+
308+
$tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar');
309+
$services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
310+
311+
$this->assertEquals(['z' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class)], $services);
312+
}
286313
}
287314

288315
class PriorityTaggedServiceTraitImplementation
@@ -305,18 +332,13 @@ class HelloNamedService2
305332
{
306333
}
307334

335+
#[AsTaggedItem(index: 'multi_hello_0', priority: 0)]
308336
#[AsTaggedItem(index: 'multi_hello_1', priority: 1)]
309337
#[AsTaggedItem(index: 'multi_hello_2', priority: 2)]
310338
class MultiTagHelloNamedService
311339
{
312340
}
313341

314-
#[AsTaggedItem(priority: 1)]
315-
#[AsTaggedItem(priority: 2)]
316-
class MultiNoNameTagHelloNamedService
317-
{
318-
}
319-
320342
interface HelloInterface
321343
{
322344
public static function getFooBar(): string;

0 commit comments

Comments
 (0)