Skip to content
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

[DependencyInjection] Allow to choose an index for tagged collection #30257

Merged
merged 2 commits into from Feb 22, 2019

Conversation

@XuruDragon
Copy link
Contributor

commented Feb 15, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29203
License MIT
Doc PR symfony/symfony-docs#11009

This is the continuity of the PR #29598

Add a way to specify an index based on a tag attribute when injecting a tag collection into services, but also a a way to fallback to a static method on the service class.

services:
  foo_service:
    class: Foo
    tags:
      - foo
  foo_service_tagged:
    class: Bar
    arguments:
      - !tagged
          tag: 'foo'
          index_by: 'tag_attribute_name'
          default_index_method: 'static_method'
<?xml version="1.0" ?>

<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 http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="foo" class="Foo">
        <tag name="foo_tag" />
    </service>
    <service id="foo_tagged_iterator" class="Bar" public="true">
      <argument type="tagged" tag="foo_tag" index-by="tag_attribute_name" default-index-method="static_method" />
    </service>
  </services>
</container>

Tasks

  • Support PHP loader/dumper
  • Support YAML loader/dumper
  • Support XML loader/dumper (and update XSD too)
  • Add tests
  • Documentation
@nicolas-grekas
Copy link
Member

left a comment

Cool, almost ready! Just a few comments to improve tests.
Please see fabbot report also.

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch 2 times, most recently from c84f9b9 to d18fd9a Feb 15, 2019

@nicolas-grekas nicolas-grekas force-pushed the XuruDragon:di-tagged-iterrator branch 2 times, most recently from 22dfaf3 to a6ffe83 Feb 15, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

(FYI, failures unrelated)

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch 2 times, most recently from 93d55a0 to ed131b4 Feb 18, 2019

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 18, 2019

PR to the documentation added

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch from ed131b4 to d159479 Feb 18, 2019

@deguif

deguif approved these changes Feb 18, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch 3 times, most recently from 57937e4 to b28017e Feb 21, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch from b28017e to e04713f Feb 22, 2019

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

Code rebased on master, CI fixed

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch from e04713f to e7d0312 Feb 22, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch from e7d0312 to e27a9c1 Feb 22, 2019

@xabbuh

xabbuh approved these changes Feb 22, 2019

Copy link
Member

left a comment

👍 with a minor comment

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-iterrator branch from e27a9c1 to 101bfd7 Feb 22, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Thank you @XuruDragon.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

And thank you @deguif!

@nicolas-grekas nicolas-grekas merged commit 101bfd7 into symfony:master Feb 22, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Feb 22, 2019

feature #30257 [DependencyInjection] Allow to choose an index for tag…
…ged collection (deguif, XuruDragon)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[DependencyInjection] Allow to choose an index for tagged collection

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29203
| License       | MIT
| Doc PR        | symfony/symfony-docs#11009

This is the continuity of the PR #29598

Add a way to specify an index based on a tag attribute when injecting a tag collection into services, but also a a way to fallback to a static method on the service class.

```yaml
services:
  foo_service:
    class: Foo
    tags:
      - foo
  foo_service_tagged:
    class: Bar
    arguments:
      - !tagged
          tag: 'foo'
          index_by: 'tag_attribute_name'
          default_index_method: 'static_method'
```
```xml
<?xml version="1.0" ?>

<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 http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="foo" class="Foo">
        <tag name="foo_tag" />
    </service>
    <service id="foo_tagged_iterator" class="Bar" public="true">
      <argument type="tagged" tag="foo_tag" index-by="tag_attribute_name" default-index-method="static_method" />
    </service>
  </services>
</container>
```

Tasks

* [x]  Support PHP loader/dumper
* [x]  Support YAML loader/dumper
* [x]  Support XML loader/dumper (and update XSD too)
* [x]  Add tests
* [x]  Documentation

Commits
-------

101bfd7 [DI] change name to tag + add XMl support + adding yaml/xml tests
845d3a6 Allow to choose an index for tagged collection
@enumag

This comment has been minimized.

@stof

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@enumag the use case you described in your comment is the use case where a ServiceLocator is what you need. Se #30348 which uses the feature of that PR to simplify configuring the ServiceLocator.

@enumag

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

@stof Thanks! Indeed that's exactly what I need.

nicolas-grekas added a commit that referenced this pull request Mar 15, 2019

feature #30348 [DependencyInjection] Add ability to define an index f…
…or service in an injected service locator argument (XuruDragon, nicolas-grekas)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[DependencyInjection] Add ability to define an index for service in an injected service locator argument

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | in progress /  symfony/symfony-docs#...

It's more or less the same thing then the PR #30257 but for a service locator argument

Add a simple way to specify an index based on a tag attribute to simplify retrieving a specific service when injecting a service locator as argument into services, but also a way to fallback to a static method on the service class.

Yaml:
```yaml
services:
  foo_service:
    class: Foo
    tags:
      - {name: foo_tag, key: foo_service}
  foo_service_tagged:
    class: Bar
    arguments:
      - !tagged_locator
          tag: 'foo_tag'
          index_by: 'key'
          default_index_method: 'static_method'
```
XML:
```xml
<?xml version="1.0" ?>

<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
    http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="foo" class="Foo">
        <tag name="foo_tag" key="foo_service" />
    </service>
    <service id="foo_tagged_iterator" class="Bar" public="true">
      <argument type="tagged_locator" tag="foo_tag" index-by="key" default-index-method="static_method" />
    </service>
  </services>
</container>
```
PHP:
```php
// config/services.php
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;

$container->register(Foo::class)
        ->addTag('foo_tag', ['key' => 'foo_service']);

$container->register(App\Handler\HandlerCollection::class)
         // inject all services tagged with app.handler as first argument
         ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('app.handler', 'key')));
```

Usage:
```php
// src/Handler/HandlerCollection.php
namespace App\Handler;

use Symfony\Component\DependencyInjection\ServiceLocator;

class HandlerCollection
{
     public function __construct(ServiceLocator $serviceLocator)
     {
           $foo = $serviceLocator->get('foo_service'):
     }
}
```

Tasks

* [x]  Support PHP loader/dumper
* [x]  Support YAML loader/dumper
* [x]  Support XML loader/dumper (and update XSD too)
* [x]  Add tests
* [x]  Documentation

Commits
-------

cb3c56b Support indexing tagged locators by FQCN as fallback
250a2c8 [DI] Allow tagged_locator tag to be used as an argument

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.