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] Add ability to define an index for service in an injected service locator argument #30348

Merged
merged 2 commits into from Mar 15, 2019

Conversation

Projects
None yet
5 participants
@XuruDragon
Copy link
Contributor

commented Feb 22, 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 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:

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 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:

// 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:

// 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

  • Support PHP loader/dumper
  • Support YAML loader/dumper
  • Support XML loader/dumper (and update XSD too)
  • Add tests
  • Documentation

@XuruDragon XuruDragon changed the title [DI] Allow tagged_locator tag to be used as an argument [DependencyInjection] Add ability to define an index for an injected service locator argument Feb 22, 2019

@XuruDragon XuruDragon changed the title [DependencyInjection] Add ability to define an index for an injected service locator argument [DependencyInjection] Add ability to define an index for service in an injected service locator argument Feb 22, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-locator branch from 15453f1 to 81739dc Feb 22, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-locator branch from 81739dc to 4cdcff7 Feb 22, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Feb 23, 2019

@nicolas-grekas
Copy link
Member

left a comment

(once @stof's comment is addressed)

@vudaltsov

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

Is there any possibility to index by a class name?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@vudaltsov sure: implement getDefautKeyName and make it return self::class.

@vudaltsov

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

That will be a new static method in the interface which all tagged services implement...
What if we could add such a possibility out of the box? It's quite common to index by a class name. Because it's automatically a unique identifier and a key that is easy to use in the code.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@vudaltsov then make all such classes extends some base class, and make the method of that class return static::class. That's almost what is done right now for form types.

@vudaltsov

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

@XuruDragon , what about default_index_method: '::class'?

@nicolas-grekas nicolas-grekas force-pushed the XuruDragon:di-tagged-locator branch from 4cc6c2c to 9618752 Mar 7, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@vudaltsov thanks for facts checking :) Indexing by FQCN is now implemented as a fallback when the attribute or the method are either not defined in the tag or not found on the service.

Anthony MARTIN and others added some commits Feb 15, 2019

Anthony MARTIN
[DI] Allow tagged_locator tag to be used as an argument
Signed-off-by: Anthony MARTIN <anthony.martin@sensiolabs.com>

@XuruDragon XuruDragon force-pushed the XuruDragon:di-tagged-locator branch from 1ae543b to cb3c56b Mar 11, 2019

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

branch has been rebase on up-to-date master

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

Thank you @XuruDragon.

@nicolas-grekas nicolas-grekas merged commit cb3c56b into symfony:master Mar 15, 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 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.