Skip to content

Commit

Permalink
feature #48685 [DependencyInjection] Exclude referencing service (sel…
Browse files Browse the repository at this point in the history
…f) in `TaggedIteratorArgument` (chalasr)

This PR was merged into the 6.3 branch.

Discussion
----------

[DependencyInjection] Exclude referencing service (self) in `TaggedIteratorArgument`

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Suggested by `@OskarStark` in https://twitter.com/OskarStark/status/1594641457226436608?s=20&t=Wbqw_Mu2tMYQ0iouaG8yig.
This PR avoids the need to explicitly indicate to exclude the referencing service when using `#[TaggedIterator]` and variants.

Before:
```php
final class DelegatingErrorTracker implements ErrorTracker
{
    public function __construct(
        #TaggedIterator(ErrorTracker::class, exclude: self::class)]
        private iterable $trackers
    ) {}
}
```

After:
```php
final class DelegatingErrorTracker implements ErrorTracker
{
    public function __construct(
        #TaggedIterator(ErrorTracker::class]
        private iterable $trackers
    ) {}
}
```

I went with no deprecation as I think the breakage potential is close to zero, but happy to reconsider.

Commits
-------

1cadd46 [DependencyInjection] Auto exclude referencing service in `TaggedIteratorArgument`
  • Loading branch information
fabpot committed Jan 5, 2023
2 parents c301d9a + 1cadd46 commit a14eb6b
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class TaggedIteratorArgument extends IteratorArgument
private ?string $defaultPriorityMethod;
private bool $needsIndexes;
private array $exclude;
private bool $excludeSelf = true;

/**
* @param string $tag The name of the tag identifying the target services
Expand All @@ -32,8 +33,9 @@ class TaggedIteratorArgument extends IteratorArgument
* @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 each service's priority when their tag doesn't define the "priority" attribute
* @param array $exclude Services to exclude from the iterator
* @param bool $excludeSelf Whether to automatically exclude the referencing service from the iterator
*/
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null, array $exclude = [])
public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null, array $exclude = [], bool $excludeSelf = true)
{
parent::__construct([]);

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

public function getTag()
Expand Down Expand Up @@ -78,4 +81,9 @@ public function getExclude(): array
{
return $this->exclude;
}

public function excludeSelf(): bool
{
return $this->excludeSelf;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public function __construct(
public ?string $defaultIndexMethod = null,
public ?string $defaultPriorityMethod = null,
public string|array $exclude = [],
public bool $excludeSelf = true,
) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public function __construct(
public ?string $defaultIndexMethod = null,
public ?string $defaultPriorityMethod = null,
public string|array $exclude = [],
public bool $excludeSelf = true,
) {
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ CHANGELOG
* Deprecate using numeric parameter names
* Add support for tagged iterators/locators `exclude` option to the xml and yaml loaders/dumpers
* Allow injecting `string $env` into php config closures
* Add `excludeSelf` parameter to `TaggedIteratorArgument` with default value to `true`
to control whether the referencing service should be automatically excluded from the iterator

6.1
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
}

if ($value instanceof TaggedIterator) {
return new TaggedIteratorArgument($value->tag, $value->indexAttribute, $value->defaultIndexMethod, false, $value->defaultPriorityMethod, (array) $value->exclude);
return new TaggedIteratorArgument($value->tag, $value->indexAttribute, $value->defaultIndexMethod, false, $value->defaultPriorityMethod, (array) $value->exclude, $value->excludeSelf);
}

if ($value instanceof TaggedLocator) {
return new ServiceLocatorArgument(new TaggedIteratorArgument($value->tag, $value->indexAttribute, $value->defaultIndexMethod, true, $value->defaultPriorityMethod, (array) $value->exclude));
return new ServiceLocatorArgument(new TaggedIteratorArgument($value->tag, $value->indexAttribute, $value->defaultIndexMethod, true, $value->defaultPriorityMethod, (array) $value->exclude, $value->excludeSelf));
}

if ($value instanceof MapDecorated) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,16 @@ trait PriorityTaggedServiceTrait
*
* @return Reference[]
*/
private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagName, ContainerBuilder $container): array
private function findAndSortTaggedServices(string|TaggedIteratorArgument $tagName, ContainerBuilder $container, array $exclude = []): array
{
$exclude = [];
$indexAttribute = $defaultIndexMethod = $needsIndexes = $defaultPriorityMethod = null;

if ($tagName instanceof TaggedIteratorArgument) {
$indexAttribute = $tagName->getIndexAttribute();
$defaultIndexMethod = $tagName->getDefaultIndexMethod();
$needsIndexes = $tagName->needsIndexes();
$defaultPriorityMethod = $tagName->getDefaultPriorityMethod() ?? 'getDefaultPriority';
$exclude = $tagName->getExclude();
$exclude = array_merge($exclude, $tagName->getExclude());
$tagName = $tagName->getTag();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
return parent::processValue($value, $isRoot);
}

$value->setValues($this->findAndSortTaggedServices($value, $this->container));
$exclude = $value->getExclude();
if ($value->excludeSelf()) {
$exclude[] = $this->currentId;
}

$value->setValues($this->findAndSortTaggedServices($value, $this->container, $exclude));

return $value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ private function getArgumentsAsPhp(\DOMElement $node, string $name, string $file
$excludes = [$arg->getAttribute('exclude')];
}

$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, $excludes);
$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, $excludes, $arg->getAttribute('exclude-self') ?: true);

if ($forLocator) {
$arguments[$key] = new ServiceLocatorArgument($arguments[$key]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -824,11 +824,11 @@ private function resolveServices(mixed $value, string $file, bool $isParameter =
$forLocator = 'tagged_locator' === $value->getTag();

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

$argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null, $forLocator, $argument['default_priority_method'] ?? null, (array) ($argument['exclude'] ?? null));
$argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null, $forLocator, $argument['default_priority_method'] ?? null, (array) ($argument['exclude'] ?? null), $argument['exclude_self'] ?? true);
} elseif (\is_string($argument) && $argument) {
$argument = new TaggedIteratorArgument($argument, null, null, $forLocator);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@
<xsd:attribute name="default-index-method" type="xsd:string" />
<xsd:attribute name="default-priority-method" type="xsd:string" />
<xsd:attribute name="exclude" type="xsd:string" />
<xsd:attribute name="exclude-self" type="xsd:boolean" />
</xsd:complexType>

<xsd:complexType name="call">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public static function getSubscribedServices(): array
'autowired' => new ServiceClosureArgument(new Reference('service.id')),
'autowired.nullable' => new ServiceClosureArgument(new Reference('service.id', ContainerInterface::NULL_ON_INVALID_REFERENCE)),
'autowired.parameter' => new ServiceClosureArgument('foobar'),
'map.decorated' => new ServiceClosureArgument(new Reference('.service_locator.oZHAdom.inner', ContainerInterface::NULL_ON_INVALID_REFERENCE)),
'map.decorated' => new ServiceClosureArgument(new Reference('.service_locator.LnJLtj2.inner', ContainerInterface::NULL_ON_INVALID_REFERENCE)),
'target' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, 'someTarget', [new Target('someTarget')])),
];
$this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,36 @@ public function testProcessWithIndexes()
$expected->setValues(['1' => new TypedReference('service_a', 'stdClass'), '2' => new TypedReference('service_b', 'stdClass')]);
$this->assertEquals($expected, $properties['foos']);
}

public function testProcesWithAutoExcludeReferencingService()
{
$container = new ContainerBuilder();
$container->register('service_a', 'stdClass')->addTag('foo', ['key' => '1']);
$container->register('service_b', 'stdClass')->addTag('foo', ['key' => '2']);
$container->register('service_c', 'stdClass')->addTag('foo', ['key' => '3'])->setProperty('foos', new TaggedIteratorArgument('foo', 'key'));

(new ResolveTaggedIteratorArgumentPass())->process($container);

$properties = $container->getDefinition('service_c')->getProperties();

$expected = new TaggedIteratorArgument('foo', 'key');
$expected->setValues(['1' => new TypedReference('service_a', 'stdClass'), '2' => new TypedReference('service_b', 'stdClass')]);
$this->assertEquals($expected, $properties['foos']);
}

public function testProcesWithoutAutoExcludeReferencingService()
{
$container = new ContainerBuilder();
$container->register('service_a', 'stdClass')->addTag('foo', ['key' => '1']);
$container->register('service_b', 'stdClass')->addTag('foo', ['key' => '2']);
$container->register('service_c', 'stdClass')->addTag('foo', ['key' => '3'])->setProperty('foos', new TaggedIteratorArgument(tag: 'foo', indexAttribute: 'key', excludeSelf: false));

(new ResolveTaggedIteratorArgumentPass())->process($container);

$properties = $container->getDefinition('service_c')->getProperties();

$expected = new TaggedIteratorArgument(tag: 'foo', indexAttribute: 'key', excludeSelf: false);
$expected->setValues(['1' => new TypedReference('service_a', 'stdClass'), '2' => new TypedReference('service_b', 'stdClass'), '3' => new TypedReference('service_c', 'stdClass')]);
$this->assertEquals($expected, $properties['foos']);
}
}

0 comments on commit a14eb6b

Please sign in to comment.