8000 bug #58000 [DependencyInjection] Fix issue between decorator and serv… · core23/symfony@86ccbdb · GitHub
[go: up one dir, main page]
Skip to content

Commit 86ccbdb

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#58000 [DependencyInjection] Fix issue between decorator and service locator index (lyrixx)
This PR was merged into the 5.4 branch. Discussion ---------- [DependencyInjection] Fix issue between decorator and service locator index | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#53174 | License | MIT This PR is not finished yet. I want to gather feedback before writing tests. To explain what is wrong, I create a [reproducer](https://github.com/lyrixx/test/tree/symfony/symfony/pull/58000). Basically, when we decorates a services, we replace the old ID with a new ID in the DIC. Then, when we build a service locator, we use the new ID. This is really not handy because our code is dependant on the new name or old name, depending if the decorator is here or not! This issue discribe very well the issue: symfony#53174 So in this PR, I save the original ID in the decorator definition. Then I used this initial ID to build the `$index` of the service locator. --- With this PR and the reproducer, I got what would be expected ![image](https://github.com/user-attachments/assets/73f6321d-d711-452b-9d20-577e82e062d8) * Original ID in the locator * But we retrieve the decorated * Works well with decorator of decorator (etc) Commits ------- b2e8399 [DependencyInjection] Fix issue between decorator and service locator index
2 parents fb5e0e8 + b2e8399 commit 86ccbdb

File tree

6 files changed

+19
-6
lines changed

6 files changed

+19
-6
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ public function process(ContainerBuilder $container)
120120

121121
$container->setAlias($inner, $id)->setPublic($public);
122122
}
123+
124+
foreach ($decoratingDefinitions as $inner => $definition) {
125+
$definition->addTag('container.decorator', ['id' => $inner]);
126+
}
123127
}
124128

125129
protected function processValue($value, bool $isRoot = false)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
8282
} elseif (null === $defaultIndex && $defaultPriorityMethod && $class) {
8383
$defaultIndex = PriorityTaggedServiceUtil::getDefault($container, $serviceId, $class, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute, $checkTaggedItem);
8484
}
85-
$index = $index ?? $defaultIndex ?? $defaultIndex = $serviceId;
85+
$decorated = $definition->getTag('container.decorator')[0]['id'] ?? null;
86+
$index = $index ?? $defaultIndex ?? $defaultIndex = $decorated ?? $serviceId;
8687

8788
$services[] = [$priority, ++$i, $index, $serviceId, $class];
8889
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitio
198198
$this->process($container);
199199

200200
$this->assertEmpty($container->getDefinition('baz.inner')->getTags());
201-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar']], $container->getDefinition('baz')->getTags());
201+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
202202
}
203203

204204
public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitionMultipleTimes()
@@ -221,7 +221,7 @@ public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitio
221221
$this->process($container);
222222

223223
$this->assertEmpty($container->getDefinition('deco1')->getTags());
224-
$this->assertEquals(['bar' => ['attr' => 'baz']], $container->getDefinition('deco2')->getTags());
224+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('deco2')->getTags());
225225
}
226226

227227
public function testProcessLeavesServiceLocatorTagOnOriginalDefinition()
@@ -240,7 +240,7 @@ public function testProcessLeavesServiceLocatorTagOnOriginalDefinition()
240240
$this->process($container);
241241

242242
$this->assertEquals(['container.service_locator' => [0 => []]], $container->getDefinition('baz.inner')->getTags());
243-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar']], $container->getDefinition('baz')->getTags());
243+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
244244
}
245245

246246
public function testProcessLeavesServiceSubscriberTagOnOriginalDefinition()
@@ -259,7 +259,7 @@ public function testProcessLeavesServiceSubscriberTagOnOriginalDefinition()
259259
$this->process($container);
260260

261261
$this->assertEquals(['container.service_subscriber' => [], 'container.service_subscriber.locator' => []], $container->getDefinition('baz.inner')->getTags());
262-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar']], $container->getDefinition('baz')->getTags());
262+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
263263
}
264264

265265
public function testProcessLeavesProxyTagOnOriginalDefinition()
@@ -278,7 +278,7 @@ public function testProcessLeavesProxyTagOnOriginalDefinition()
278278
$this->process($container);
279279

280280
$this->assertEquals(['proxy' => 'foo'], $container->getDefinition('baz.inner')->getTags());
281-
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar']], $container->getDefinition('baz')->getTags());
281+
$this->assertEquals(['bar' => ['attr' => 'baz'], 'foobar' => ['attr' => 'bar'], 'container.decorator' => [['id' => 'foo']]], $container->getDefinition('baz')->getTags());
282282
}
283283

284284
public function testCannotDecorateSyntheticService()

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ public function testTheIndexedTagsByDefaultIndexMethod()
153153

154154
$container->register('service4', HelloInterface::class)->addTag('my_custom_tag');
155155

156+
$definition = $container->register('debug.service5', \stdClass::class)->addTag('my_custom_tag');
157+
$definition->addTag('container.decorator', ['id' => 'service5']);
158+
156159
$priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
157160

158161
$tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar');
@@ -161,6 +164,7 @@ public function testTheIndexedTagsByDefaultIndexMethod()
161164
'service1' => new TypedReference('service1', FooTagClass::class),
162165
'10' => new TypedReference('service3', IntTagClass::class),
163166
'service4' => new TypedReference('service4', HelloInterface::class),
167+
'service5' => new TypedReference('debug.service5', \stdClass::class),
164168
];
165169
$services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
166170
$this->assertSame(array_keys($expected), array_keys($services));

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.expected.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,6 @@ services:
1515
decorated:
1616
class: Symfony\Component\DependencyInjection\Tests\Fixtures\StdClassDecorator
1717
public: true
18+
tags:
19+
- container.decorator: { id: decorated }
1820
arguments: [!service { class: stdClass }]

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ services:
77
foo:
88
class: Class2
99
public: true
10+
tags:
11+
- container.decorator: { id: bar }
1012
file: file.php
1113
lazy: true
1214
arguments: [!service { class: Class1 }]

0 commit comments

Comments
 (0)
0