Skip to content

[DependencyInjection] Define default priority inside service class #31943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ class TaggedIteratorArgument extends IteratorArgument
private $indexAttribute;
private $defaultIndexMethod;
private $needsIndexes = false;
private $defaultPriorityMethod;

/**
* @param string $tag The name of the tag identifying the target services
* @param string|null $indexAttribute The name of the attribute that defines the key referencing each service in the tagged collection
* @param string|null $defaultIndexMethod The static method that should be called to get each service's key when their tag doesn't define the previous attribute
* @param bool $needsIndexes Whether indexes are required and should be generated when computing the map
* @param string $tag The name of the tag identifying the target services
* @param string|null $indexAttribute The name of the attribute that defines the key referencing each service in the tagged collection
* @param string|null $defaultIndexMethod The static method that should be called to get each service's key when their tag doesn't define the previous attribute
* @param bool $needsIndexes Whether indexes are required and should be generated when computing the map
* @param string|null $defaultPriorityMethod The static method that should be called to get the default priority of the service
*/
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false)
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null)
{
parent::__construct([]);

Expand All @@ -41,6 +43,7 @@ public function __construct(string $tag, string $indexAttribute = null, string $
$this->indexAttribute = $indexAttribute;
$this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
$this->needsIndexes = $needsIndexes;
$this->defaultPriorityMethod = $defaultPriorityMethod;
}

public function getTag()
Expand All @@ -62,4 +65,9 @@ public function needsIndexes(): bool
{
return $this->needsIndexes;
}

public function getDefaultPriorityMethod(): ?string
{
return $this->defaultPriorityMethod;
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* deprecated support for short factories and short configurators in Yaml
* deprecated `tagged` in favor of `tagged_iterator`
* added ability to define a priority method for a tagged collection

4.3.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,61 @@ trait PriorityTaggedServiceTrait
*/
private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
{
$indexAttribute = $defaultIndexMethod = $needsIndexes = null;
$indexAttribute = $defaultIndexMethod = $needsIndexes = $defaultPriorityMethod = null;

if ($tagName instanceof TaggedIteratorArgument) {
$indexAttribute = $tagName->getIndexAttribute();
$defaultIndexMethod = $tagName->getDefaultIndexMethod();
$needsIndexes = $tagName->needsIndexes();
$defaultPriorityMethod = $tagName->getDefaultPriorityMethod();
$tagName = $tagName->getTag();
}

$services = [];

foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
$priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
$class = $container->getDefinition($serviceId)->getClass();
$class = $container->getParameterBag()->resolveValue($class) ?: null;
$reflectionClass = $container->getReflectionClass($class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line must be executed only if null !== $defaultPriorityMethod and $attributes[0]['priority'] is not set, so that we don't autoload a class for nothing when not needed


$priority = 0;
if (null !== $defaultPriorityMethod && null !== $reflectionClass && $reflectionClass->hasMethod($defaultPriorityMethod)) {
if (!($priorityReflMethod = $reflectionClass->getMethod($defaultPriorityMethod))->isStatic()) {
throw new InvalidArgumentException(sprintf('Default priority method "%s::%s()" of the "%s"-tagged collection on service "%s" must be static.', $class, $defaultPriorityMethod, $tagName, $serviceId));
}

if (!$priorityReflMethod->isPublic()) {
throw new InvalidArgumentException(sprintf('Default priority method "%s::%s()" of the "%s"-tagged collection on service "%s" should be public.', $class, $defaultPriorityMethod, $tagName, $serviceId));
}

$priority = $priorityReflMethod->invoke(null);

if (!\is_int($priority)) {
throw new InvalidArgumentException(sprintf('Default priority method "%s::%s()" of the "%s"-tagged collection on service "%s" should return an integer, got %s.', $class, $defaultPriorityMethod, $tagName, $serviceId, \gettype($priority)));
}
}

$priority = $attributes[0]['priority'] ?? $priority;

if (null === $indexAttribute && !$needsIndexes) {
$services[$priority][] = new Reference($serviceId);

continue;
}

$class = $container->getDefinition($serviceId)->getClass();
$class = $container->getParameterBag()->resolveValue($class) ?: null;

if (null !== $indexAttribute && isset($attributes[0][$indexAttribute])) {
$services[$priority][$attributes[0][$indexAttribute]] = new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $attributes[0][$indexAttribute]);

continue;
}

if (!$r = $container->getReflectionClass($class)) {
if (null === $reflectionClass) {
throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $serviceId));
}

$class = $r->name;
$class = $reflectionClass->name;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep $r as the name here, that will reduce the diff, thus reduce future merge conflicts


if (!$r->hasMethod($defaultIndexMethod)) {
if (!$reflectionClass->hasMethod($defaultIndexMethod)) {
if ($needsIndexes) {
$services[$priority][$serviceId] = new TypedReference($serviceId, $class);

Expand All @@ -86,7 +105,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
throw new InvalidArgumentException(sprintf('Method "%s::%s()" not found: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
}

if (!($rm = $r->getMethod($defaultIndexMethod))->isStatic()) {
if (!($rm = $reflectionClass->getMethod($defaultIndexMethod))->isStatic()) {
throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be static: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ private function convertParameters(array $parameters, $type, \DOMElement $parent
$element->setAttribute('default-index-method', $tag->getDefaultIndexMethod());
}
}

if (null !== $tag->getDefaultPriorityMethod()) {
$element->setAttribute('default-priority-method', $tag->getDefaultPriorityMethod());
}
} elseif ($value instanceof IteratorArgument) {
$element->setAttribute('type', 'iterator');
$this->convertParameters($value->getValues(), $type, $element, 'key');
Expand Down
19 changes: 12 additions & 7 deletions src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,19 +235,24 @@ private function dumpValue($value)
$tag = $value;

if ($value instanceof TaggedIteratorArgument || ($value instanceof ServiceLocatorArgument && $tag = $value->getTaggedIteratorArgument())) {
if (null === $tag->getIndexAttribute()) {
$content = $tag->getTag();
} else {
$content = [
'tag' => $tag->getTag(),
'index_by' => $tag->getIndexAttribute(),
];
$content = ['tag' => $tag->getTag()];

if (null !== $tag->getIndexAttribute()) {
$content['index_by'] = $tag->getIndexAttribute();

if (null !== $tag->getDefaultIndexMethod()) {
$content['default_index_method'] = $tag->getDefaultIndexMethod();
}
}

if (null !== $tag->getDefaultPriorityMethod()) {
$content['default_priority_method'] = $tag->getDefaultPriorityMethod();
}

if (1 === \count($content)) {
$content = $tag->getTag();
}

return new TaggedValue($value instanceof TaggedIteratorArgument ? 'tagged_iterator' : 'tagged_locator', $content);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,27 @@ function iterator(array $values): IteratorArgument
*
* @deprecated since Symfony 4.4, to be removed in 5.0, use "tagged_iterator" instead.
*/
function tagged(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null): TaggedIteratorArgument
function tagged(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null): TaggedIteratorArgument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be reverted, this function is deprecated, no need to alter it

{
@trigger_error(__NAMESPACE__.'\tagged() is deprecated since Symfony 4.4 and will be removed in 5.0, use '.__NAMESPACE__.'\tagged_iterator() instead.', E_USER_DEPRECATED);

return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod);
return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, $needsIndexes, $defaultPriorityMethod);
}

/**
* Creates a lazy iterator by tag name.
*/
function tagged_iterator(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null): TaggedIteratorArgument
function tagged_iterator(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null): TaggedIteratorArgument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$needsIndexes should be removed, it's always false for iterators

{
return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod);
return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, $needsIndexes, $defaultPriorityMethod);
}

/**
* Creates a service locator by tag name.
*/
function tagged_locator(string $tag, string $indexAttribute, string $defaultIndexMethod = null): ServiceLocatorArgument
function tagged_locator(string $tag, string $indexAttribute, string $defaultIndexMethod = null, bool $needsIndexes = true, string $defaultPriorityMethod = null): ServiceLocatorArgument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$needsIndexes should be removed, it's always true for locators

{
return new ServiceLocatorArgument(new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, true));
return new ServiceLocatorArgument(new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, $needsIndexes, $defaultPriorityMethod));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $file, $lowercase =
throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="%s" has no or empty "tag" attribute in "%s".', $name, $type, $file));
}

$arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'), $arg->getAttribute('index-by') ?: null, $arg->getAttribute('default-index-method') ?: null, $forLocator);
$arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'), $arg->getAttribute('index-by') ?: null, $arg->getAttribute('default-index-method') ?: null, $forLocator, $arg->getAttribute('default-priority-method') ?: null);

if ($forLocator) {
$arguments[$key] = new ServiceLocatorArgument($arguments[$key]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,11 @@ private function resolveServices($value, $file, $isParameter = false)
}

if (\is_array($argument) && isset($argument['tag']) && $argument['tag']) {
if ($diff = array_diff(array_keys($argument), ['tag', 'index_by', 'default_index_method'])) {
throw new InvalidArgumentException(sprintf('"!%s" tag contains unsupported key "%s"; supported ones are "tag", "index_by" and "default_index_method".', $value->getTag(), implode('"", "', $diff)));
if ($diff = array_diff(array_keys($argument), ['tag', 'index_by', 'default_index_method', 'default_priority_method'])) {
throw new InvalidArgumentException(sprintf('"!%s" tag contains unsupported key "%s"; supported ones are "tag", "index_by", "default_index_method" and "default_priority_method".', $value->getTag(), implode('"", "', $diff)));
}

$argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'], $argument['default_index_method'] ?? null, $forLocator);
$argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null, $forLocator, $argument['default_priority_method'] ?? null);

if ($forLocator) {
$argument = new ServiceLocatorArgument($argument);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@
<xsd:attribute name="tag" type="xsd:string" />
<xsd:attribute name="index-by" type="xsd:string" />
<xsd:attribute name="default-index-method" type="xsd:string" />
<xsd:attribute name="default-priority-method" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="call">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,30 @@ public function testTaggedServiceLocatorWithDefaultIndex()
];
$this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]);
}

public function testTaggedServiceWithPriorityMethod()
{
$container = new ContainerBuilder();
$container->register(BarTagClass::class)
->setPublic(true)
->addTag('foo_bar', ['priority' => 10])
;
$container->register(FooTagClass::class)
->setPublic(true)
->addTag('foo_bar')
;
$container->register(FooBarTaggedClass::class)
->addArgument(new TaggedIteratorArgument('foo_bar', null, null, false, 'getDefaultPriority'))
->setPublic(true)
;

$container->compile();

$s = $container->get(FooBarTaggedClass::class);

$param = iterator_to_array($s->getParam()->getIterator());
$this->assertSame([0 => $container->get(FooTagClass::class), 1 => $container->get(BarTagClass::class)], $param);
}
}

class ServiceSubscriberStub implements ServiceSubscriberInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,24 @@ public function testTaggedArguments()
$this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_arguments.xml', $dumper->dump());
}

public function testTaggedWithDefaultPriorityMethod()
{
$taggedIterator = new TaggedIteratorArgument('foo_tag', null, null, false, 'foopriority');
$container = new ContainerBuilder();
$container->register('foo', 'Foo')->addTag('foo_tag');
$container->register('foo_tagged_iterator', 'Bar')
->setPublic(true)
->addArgument($taggedIterator)
;
$container->register('foo_tagged_locator', 'Bar')
->setPublic(true)
->addArgument(new ServiceLocatorArgument($taggedIterator))
;

$dumper = new XmlDumper($container);
$this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_priority_method.xml', $dumper->dump());
}

public function testDumpAbstractServices()
{
$container = include self::$fixturesPath.'/containers/container_abstract.php';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,18 @@ public function testTaggedArguments()
$this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_argument.yml', $dumper->dump());
}

public function testTaggedWithDefaultPriorityMethod()
{
$taggedIterator = new TaggedIteratorArgument('foo', null, null, false, 'foopriority');
$container = new ContainerBuilder();
$container->register('foo_service', 'Foo')->addTag('foo');
$container->register('foo_service_tagged_iterator', 'Bar')->addArgument($taggedIterator);
$container->register('foo_service_tagged_locator', 'Bar')->addArgument(new ServiceLocatorArgument($taggedIterator));

$dumper = new YamlDumper($container);
$this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_priority_method.yml', $dumper->dump());
}

private function assertEqualYamlStructure($expected, $yaml, $message = '')
{
$parser = new Parser();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ public static function getDefaultFooName()
{
return 'foo_tag_class';
}

public static function getDefaultPriority()
{
return 20;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
<service id="foo" class="Foo">
<tag name="foo_tag"/>
</service>
<service id="foo_tagged_iterator" class="Bar" public="true">
<argument type="tagged_iterator" tag="foo_tag" default-priority-method="foopriority"/>
</service>
<service id="foo_tagged_locator" class="Bar" public="true">
<argument type="tagged_locator" tag="foo_tag" default-priority-method="foopriority"/>
</service>
<service id="Psr\Container\ContainerInterface" alias="service_container" public="false"/>
<service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" public="false"/>
</services>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

services:
service_container:
class: Symfony\Component\DependencyInjection\ContainerInterface
public: true
synthetic: true
foo_service:
class: Foo
tags:
- { name: foo }
foo_service_tagged_iterator:
class: Bar
arguments: [!tagged_iterator { tag: foo, default_priority_method: foopriority }]
foo_service_tagged_locator:
class: Bar
arguments: [!tagged_locator { tag: foo, default_priority_method: foopriority }]
Psr\Container\ContainerInterface:
alias: service_container
public: false
Symfony\Component\DependencyInjection\ContainerInterface:
alias: service_container
public: false
Loading