Skip to content

Commit a7535bb

Browse files
MatTheCatMatTheCat
committed
[DependencyInjection] Fix ServiceLocatorTagPass indexes handling
1 parent 61e6239 commit a7535bb

File tree

4 files changed

+100
-39
lines changed

4 files changed

+100
-39
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 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 locator
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
2525
*/
2626
public function __construct(
2727
public ?string $index = null,

‎Compiler/PriorityTaggedServiceTrait.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagNam
8787
if (null === $index && null === $defaultIndex && $defaultPriorityMethod && $class) {
8888
$defaultIndex = PriorityTaggedServiceUtil::getDefault($container, $serviceId, $class, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute, $checkTaggedItem);
8989
}
90-
$decorated = $definition->getTag('container.decorator')[0]['id'] ?? null;
91-
$index = $index ?? $defaultIndex ?? $defaultIndex = $decorated ?? $serviceId;
90+
$index ??= $defaultIndex ??= $definition->getTag('container.decorator')[0]['id'] ?? $serviceId;
9291

9392
$services[] = [$priority, ++$i, $index, $serviceId, $class];
9493
}

‎Compiler/ServiceLocatorTagPass.php‎

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,41 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
5454
$value->setClass(ServiceLocator::class);
5555
}
5656

57-
$services = $value->getArguments()[0] ?? null;
57+
$values = $value->getArguments()[0] ?? null;
58+
$services = [];
5859

59-
if ($services instanceof TaggedIteratorArgument) {
60-
$services = $this->findAndSortTaggedServices($services, $this->container);
61-
}
62-
63-
if (!\is_array($services)) {
60+
if ($values instanceof TaggedIteratorArgument) {
61+
foreach ($this->findAndSortTaggedServices($values, $this->container) as $k => $v) {
62+
$services[$k] = new ServiceClosureArgument($v);
63+
}
64+
} elseif (!\is_array($values)) {
6465
throw new InvalidArgumentException(\sprintf('Invalid definition for service "%s": an array of references is expected as first argument when the "container.service_locator" tag is set.', $this->currentId));
66+
} else {
67+
$i = 0;
68+
69+
foreach ($values as $k => $v) {
70+
if ($v instanceof ServiceClosureArgument) {
71+
$services[$k] = $v;
72+
continue;
73+
}
74+
75+
if ($i === $k) {
76+
if ($v instanceof Reference) {
77+
$k = (string) $v;
78+
}
79+
++$i;
80+
} elseif (\is_int($k)) {
81+
$i = null;
82+
}
83+
84+
$services[$k] = new ServiceClosureArgument($v);
85+
}
86+
if (\count($services) === $i) {
87+
ksort($services);
88+
}
6589
}
6690

67-
$value->setArgument(0, self::map($services));
91+
$value->setArgument(0, $services);
6892

6993
$id = '.service_locator.'.ContainerBuilder::hash($value);
7094

@@ -83,8 +107,12 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
83107

84108
public static function register(ContainerBuilder $container, array $map, ?string $callerId = null): Reference
85109
{
110+
foreach ($map as $k => $v) {
111+
$map[$k] = new ServiceClosureArgument($v);
112+
}
113+
86114
$locator = (new Definition(ServiceLocator::class))
87-
->addArgument(self::map($map))
115+
->addArgument($map)
88116
->addTag('container.service_locator');
89117

90118
if (null !== $callerId && $container->hasDefinition($callerId)) {
@@ -109,29 +137,4 @@ public static function register(ContainerBuilder $container, array $map, ?string
109137

110138
return new Reference($id);
111139
}
112-
113-
public static function map(array $services): array
114-
{
115-
$i = 0;
116-
117-
foreach ($services as $k => $v) {
118-
if ($v instanceof ServiceClosureArgument) {
119-
continue;
120-
}
121-
122-
if ($i === $k) {
123-
if ($v instanceof Reference) {
124-
unset($services[$k]);
125-
$k = (string) $v;
126-
}
127-
++$i;
128-
} elseif (\is_int($k)) {
129-
$i = null;
130-
}
131-
132-
$services[$k] = new ServiceClosureArgument($v);
133-
}
134-
135-
return $services;
136-
}
137140
}

‎Tests/Compiler/ServiceLocatorTagPassTest.php‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,26 @@ public function testProcessValue()
8686
$this->assertSame(CustomDefinition::class, \get_class($locator('inlines.service')));
8787
}
8888

89+
public function testServiceListIsOrdered()
90+
{
91+
$container = new ContainerBuilder();
92+
93+
$container->register('bar', CustomDefinition::class);
94+
$container->register('baz', CustomDefinition::class);
95+
96+
$container->register('foo', ServiceLocator::class)
97+
->setArguments([[
98+
new Reference('baz'),
99+
new Reference('bar'),
100+
]])
101+
->addTag('container.service_locator')
102+
;
103+
104+
(new ServiceLocatorTagPass())->process($container);
105+
106+
$this->assertSame(['bar', 'baz'], array_keys($container->getDefinition('foo')->getArgument(0)));
107+
}
108+
89109
public function testServiceWithKeyOverwritesPreviousInheritedKey()
90110
{
91111
$container = new ContainerBuilder();
@@ -170,6 +190,27 @@ public function testTaggedServices()
170190
$this->assertSame(TestDefinition2::class, $locator('baz')::class);
171191
}
172192

193+
public function testTaggedServicesKeysAreKept()
194+
{
195+
$container = new ContainerBuilder();
196+
197+
$container->register('bar', TestDefinition1::class)->addTag('test_tag', ['index' => 0]);
198+
$container->register('baz', TestDefinition2::class)->addTag('test_tag', ['index' => 1]);
199+
200+
$container->register('foo', ServiceLocator::class)
201+
->setArguments([new TaggedIteratorArgument('test_tag', 'index', null, true)])
202+
->addTag('container.service_locator')
203+
;
204+
205+
(new ServiceLocatorTagPass())->process($container);
206+
207+
/** @var ServiceLocator $locator */
208+
$locator = $container->get('foo');
209+
210+
$this->assertSame(TestDefinition1::class, $locator(0)::class);
211+
$this->assertSame(TestDefinition2::class, $locator(1)::class);
212+
}
213+
173214
public function testIndexedByServiceIdWithDecoration()
174215
{
175216
$container = new ContainerBuilder();
@@ -201,15 +242,33 @@ public function testIndexedByServiceIdWithDecoration()
201242
static::assertInstanceOf(DecoratedService::class, $locator->get(Service::class));
202243
}
203244

204-
public function testDefinitionOrderIsTheSame()
245+
public function testServicesKeysAreKept()
205246
{
206247
$container = new ContainerBuilder();
207248
$container->register('service-1');
208249
$container->register('service-2');
250+
$container->register('service-3');
209251

210252
$locator = ServiceLocatorTagPass::register($container, [
211-
new Reference('service-2'),
212253
new Reference('service-1'),
254+
'service-2' => new Reference('service-2'),
255+
'foo' => new Reference('service-3'),
256+
]);
257+
$locator = $container->getDefinition($locator);
258+
$factories = $locator->getArguments()[0];
259+
260+
static::assertSame([0, 'service-2', 'foo'], array_keys($factories));
261+
}
262+
263+
public function testDefinitionOrderIsTheSame()
264+
{
265+
$container = new ContainerBuilder();
266+
$container->register('service-1');
267+
$container->register('service-2');
268+
269+
$locator = ServiceLocatorTagPass::register($container, [
270+
'service-2' => new Reference('service-2'),
271+
'service-1' => new Reference('service-1'),
213272
]);
214273
$locator = $container->getDefinition($locator);
215274
$factories = $locator->getArguments()[0];

0 commit comments

Comments
 (0)