Skip to content

Commit a54032b

Browse files
committed
Merge branch '5.1' into 5.2
* 5.1: [Notifier] [Twilio] Fix tests [Cache] Prevent notice on case matching metadata trick [Notifier][Slack] Remove :void from test methods [Notifier] Remove @internal annotation from notifier transports [Notifier][Twilio] Add tests [Notifier][Free Mobile] Tests [DI] The default index method wasn't used if the "index_by" attribute is missing
2 parents a293468 + cb3d5f9 commit a54032b

File tree

4 files changed

+90
-10
lines changed

4 files changed

+90
-10
lines changed

Argument/TaggedIteratorArgument.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ public function __construct(string $tag, string $indexAttribute = null, string $
4141

4242
$this->tag = $tag;
4343
$this->indexAttribute = $indexAttribute;
44-
$this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
44+
$this->defaultIndexMethod = $defaultIndexMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Name' : null);
4545
$this->needsIndexes = $needsIndexes;
46-
$this->defaultPriorityMethod = $defaultPriorityMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Priority');
46+
$this->defaultPriorityMethod = $defaultPriorityMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Priority' : null);
4747
}
4848

4949
public function getTag()

Compiler/PriorityTaggedServiceTrait.php

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
4646
$indexAttribute = $tagName->getIndexAttribute();
4747
$defaultIndexMethod = $tagName->getDefaultIndexMethod();
4848
$needsIndexes = $tagName->needsIndexes();
49-
$defaultPriorityMethod = $tagName->getDefaultPriorityMethod();
49+
$defaultPriorityMethod = $tagName->getDefaultPriorityMethod() ?? 'getDefaultPriority';
5050
$tagName = $tagName->getTag();
5151
}
5252

@@ -69,15 +69,15 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
6969
}
7070
$priority = $priority ?? $defaultPriority ?? $defaultPriority = 0;
7171

72-
if (null === $indexAttribute && !$needsIndexes) {
72+
if (null === $indexAttribute && !$defaultIndexMethod && !$needsIndexes) {
7373
$services[] = [$priority, ++$i, null, $serviceId, null];
7474
continue 2;
7575
}
7676

7777
if (null !== $indexAttribute && isset($attribute[$indexAttribute])) {
7878
$index = $attribute[$indexAttribute];
79-
} elseif (null === $defaultIndex && $defaultIndexMethod && $class) {
80-
$defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod, $tagName, $indexAttribute);
79+
} elseif (null === $defaultIndex && $defaultPriorityMethod && $class) {
80+
$defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute);
8181
}
8282
$index = $index ?? $defaultIndex ?? $defaultIndex = $serviceId;
8383

@@ -116,24 +116,31 @@ class PriorityTaggedServiceUtil
116116
/**
117117
* Gets the index defined by the default index method.
118118
*/
119-
public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, string $indexAttribute): ?string
119+
public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, ?string $indexAttribute): ?string
120120
{
121121
if (!($r = $container->getReflectionClass($class)) || !$r->hasMethod($defaultIndexMethod)) {
122122
return null;
123123
}
124124

125+
if (null !== $indexAttribute) {
126+
$service = $class !== $serviceId ? sprintf('service "%s"', $serviceId) : 'on the corresponding service';
127+
$message = [sprintf('Either method "%s::%s()" should ', $class, $defaultIndexMethod), sprintf(' or tag "%s" on %s is missing attribute "%s".', $tagName, $service, $indexAttribute)];
128+
} else {
129+
$message = [sprintf('Method "%s::%s()" should ', $class, $defaultIndexMethod), '.'];
130+
}
131+
125132
if (!($rm = $r->getMethod($defaultIndexMethod))->isStatic()) {
126-
throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be static or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
133+
throw new InvalidArgumentException(implode('be static', $message));
127134
}
128135

129136
if (!$rm->isPublic()) {
130-
throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be public or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
137+
throw new InvalidArgumentException(implode('be public', $message));
131138
}
132139

133140
$defaultIndex = $rm->invoke(null);
134141

135142
if (!\is_string($defaultIndex)) {
136-
throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should return a string (got "%s") or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, get_debug_type($defaultIndex), $tagName, $serviceId, $indexAttribute));
143+
throw new InvalidArgumentException(implode(sprintf('return a string (got "%s")', get_debug_type($defaultIndex)), $message));
137144
}
138145

139146
return $defaultIndex;

Tests/Compiler/PriorityTaggedServiceTraitTest.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@
1515
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
1616
use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
1717
use Symfony\Component\DependencyInjection\ContainerBuilder;
18+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1819
use Symfony\Component\DependencyInjection\Reference;
1920
use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass;
21+
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTagClass;
22+
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTaggedForInvalidDefaultMethodClass;
2023
use Symfony\Component\DependencyInjection\TypedReference;
2124

2225
class PriorityTaggedServiceTraitTest extends TestCase
@@ -132,6 +135,55 @@ public function testOnlyTheIndexedTagsAreListed()
132135
$this->assertSame(array_keys($expected), array_keys($services));
133136
$this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
134137
}
138+
139+
public function testTheIndexedTagsByDefaultIndexMethod()
140+
{
141+
$container = new ContainerBuilder();
142+
$container->register('service1', FooTagClass::class)->addTag('my_custom_tag');
143+
144+
$definition = $container->register('service2', BarTagClass::class);
145+
$definition->addTag('my_custom_tag', ['priority' => 100]);
146+
$definition->addTag('my_custom_tag', []);
147+
148+
$priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
149+
150+
$tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar');
151+
$expected = [
152+
'bar_tab_class_with_defaultmethod' => new TypedReference('service2', BarTagClass::class),
153+
'service1' => new TypedReference('service1', FooTagClass::class),
154+
];
155+
$services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
156+
$this->assertSame(array_keys($expected), array_keys($services));
157+
$this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
158+
}
159+
160+
/**
161+
* @dataProvider provideInvalidDefaultMethods
162+
*/
163+
public function testTheIndexedTagsByDefaultIndexMethodFailure(string $defaultIndexMethod, ?string $indexAttribute, string $expectedExceptionMessage)
164+
{
165+
$this->expectException(InvalidArgumentException::class);
166+
$this->expectExceptionMessage($expectedExceptionMessage);
167+
168+
$container = new ContainerBuilder();
169+
170+
$container->register('service1', FooTaggedForInvalidDefaultMethodClass::class)->addTag('my_custom_tag');
171+
172+
$priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
173+
174+
$tag = new TaggedIteratorArgument('my_custom_tag', $indexAttribute, $defaultIndexMethod);
175+
$priorityTaggedServiceTraitImplementation->test($tag, $container);
176+
}
177+
178+
public function provideInvalidDefaultMethods(): iterable
179+
{
180+
yield ['getMethodShouldBeStatic', null, sprintf('Method "%s::getMethodShouldBeStatic()" should be static.', FooTaggedForInvalidDefaultMethodClass::class)];
181+
yield ['getMethodShouldBeStatic', 'foo', sprintf('Either method "%s::getMethodShouldBeStatic()" should be static or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
182+
yield ['getMethodShouldBePublicInsteadProtected', null, sprintf('Method "%s::getMethodShouldBePublicInsteadProtected()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)];
183+
yield ['getMethodShouldBePublicInsteadProtected', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadProtected()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
184+
yield ['getMethodShouldBePublicInsteadPrivate', null, sprintf('Method "%s::getMethodShouldBePublicInsteadPrivate()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)];
185+
yield ['getMethodShouldBePublicInsteadPrivate', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadPrivate()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
186+
}
135187
}
136188

137189
class PriorityTaggedServiceTraitImplementation
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
4+
5+
class FooTaggedForInvalidDefaultMethodClass
6+
{
7+
public function getMethodShouldBeStatic()
8+
{
9+
return 'anonymous_foo_class_with_default_method';
10+
}
11+
12+
protected static function getMethodShouldBePublicInsteadProtected()
13+
{
14+
return 'anonymous_foo_class_with_default_method';
15+
}
16+
17+
private static function getMethodShouldBePublicInsteadPrivate()
18+
{
19+
return 'anonymous_foo_class_with_default_method';
20+
}
21+
}

0 commit comments

Comments
 (0)