Skip to content

Commit f29141d

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Fix merging explicit tags and #[AsTaggeditem]
1 parent 0e10ab7 commit f29141d

File tree

2 files changed

+78
-61
lines changed

2 files changed

+78
-61
lines changed

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

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,34 @@ 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+
70+
if ($reflector && $definition->isAutoconfigured() && !$definition->hasTag('container.ignore_attributes') && $phpAttributes = $reflector->getAttributes(AsTaggedItem::class)) {
71+
$loadFromAttributes = $attributes !== $tagAttributes = array_filter($attributes);
72+
$attributes = [];
73+
foreach ($phpAttributes as $attribute) {
74+
$attribute = $attribute->newInstance();
75+
$attributes[] = [
76+
'priority' => $attribute->priority,
77+
$indexAttribute ?? '' => $attribute->index,
78+
];
79+
if (null === $defaultPriority) {
80+
$defaultPriority = $attribute->priority ?? 0;
81+
$defaultIndex = $attribute->index;
82+
}
83+
}
84+
$attributes = $loadFromAttributes ? array_merge($attributes, $tagAttributes) : $tagAttributes;
85+
}
6986

7087
foreach ($attributes as $attribute) {
7188
$index = $priority = null;
7289

7390
if (isset($attribute['priority'])) {
7491
$priority = $attribute['priority'];
75-
} elseif (null === $defaultPriority && $defaultPriorityMethod && $reflector) {
76-
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority', $checkTaggedItem);
92+
} elseif ($loadFromDefaultMethods) {
93+
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority') ?? $defaultPriority;
94+
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute) ?? $defaultIndex;
95+
$loadFromDefaultMethods = false;
7796
}
7897
$priority ??= $defaultPriority ??= 0;
7998

@@ -85,41 +104,26 @@ private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagNam
85104
if (null !== $indexAttribute && isset($attribute[$indexAttribute])) {
86105
$index = $parameterBag->resolveValue($attribute[$indexAttribute]);
87106
}
88-
if (null === $index && null === $defaultIndex && $defaultPriorityMethod && $reflector) {
89-
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute, $checkTaggedItem);
107+
if (null === $index && $loadFromDefaultMethods) {
108+
$defaultPriority = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultPriorityMethod, $tagName, 'priority') ?? $defaultPriority;
109+
$defaultIndex = PriorityTaggedServiceUtil::getDefault($serviceId, $reflector, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute) ?? $defaultIndex;
110+
$loadFromDefaultMethods = false;
90111
}
91112
$index ??= $defaultIndex ??= $definition->getTag('container.decorator')[0]['id'] ?? $serviceId;
92113

93114
$services[] = [$priority, ++$i, $index, $serviceId, $class];
94115
}
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-
}
109-
}
110116
}
111117

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

114120
$refs = [];
115121
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-
}
122+
$reference = match (true) {
123+
!$class => new Reference($serviceId),
124+
$index === $serviceId => new TypedReference($serviceId, $class),
125+
default => new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $index),
126+
};
123127

124128
if (null === $index) {
125129
$refs[] = $reference;
@@ -137,25 +141,16 @@ private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagNam
137141
*/
138142
class PriorityTaggedServiceUtil
139143
{
140-
public static function getDefault(string $serviceId, \ReflectionClass $r, string $defaultMethod, string $tagName, ?string $indexAttribute, bool $checkTaggedItem): string|int|null
144+
public static function getDefault(string $serviceId, \ReflectionClass $r, string $defaultMethod, string $tagName, ?string $indexAttribute): string|int|null
141145
{
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-
146+
if (!$r->hasMethod($defaultMethod)) {
153147
return null;
154148
}
155149

156150
if ($r->isInterface()) {
157151
return null;
158152
}
153+
$class = $r->name;
159154

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

‎src/Symfony/Component/DependencyInjection/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 testAttributesAreMergedBeforeTags()
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+
'multi_hello_0' => new TypedReference('service_attr_first', MultiTagHelloNamedService::class),
291+
'z' => 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)