Skip to content

Have a common way to collect services and inject it into a single one #12269

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
dawehner opened this issue Oct 20, 2014 · 17 comments
Closed

Have a common way to collect services and inject it into a single one #12269

dawehner opened this issue Oct 20, 2014 · 17 comments

Comments

@dawehner
Copy link
Contributor

Similar issue to (in the sense of having a similar context) than #11460

In Drupal we have a lot of tagged services which are injected into a single one. When we started most of them had custom Compiler Passed, but than we realized that we can easily extract that into a common service:

This would be the "collector" service:

  breadcrumb:
    class: Drupal\Core\Breadcrumb\BreadcrumbManager
    arguments: ['@module_handler']
    tags:
      - { name: service_collector, tag: breadcrumb_builder, call: addBuilder }

and then you do something like that in the client service:

  forum.breadcrumb.listing:
    class: Drupal\forum\Breadcrumb\ForumListingBreadcrumbBuilder
    arguments: ['@entity.manager', '@config.factory', '@forum_manager']
    tags:
      - { name: breadcrumb_builder, priority: 314 }

Any oppinions?

@Strate
Copy link
Contributor

Strate commented Oct 20, 2014

Very good idea! But I think it should be bundle.

@dawehner
Copy link
Contributor Author

@Strate
Well ... Drupal just uses the components of Symfony so bundles wouldn't be that good in case Drupal wants to reuse it.

@Strate
Copy link
Contributor

Strate commented Oct 20, 2014

Ok, it can be external library (with only one compiler pass, as I think), and separate bundle to integrate with symfony full stack apps.

@fabpot
Copy link
Member

fabpot commented Oct 21, 2014

@Strate Let's first talk about the problem at hand before talking about where/how to solve it.

@linaori
Copy link
Contributor

linaori commented Oct 21, 2014

I'm curious but missing the problem and I don't see a relation between the 2 examples.

@Strate
Copy link
Contributor

Strate commented Oct 21, 2014

As I see, this feature allows only passing service to method call, What about passing array of services to property or constructor's argument?

@stof
Copy link
Member

stof commented Nov 21, 2014

@dawehner could you paste (or link to) the code of the compiler pass implemented in Drupal, so that your code snippet is complete ? Seeing only the tags without the way they are processed does not make it easy to understand the solution you implemented.
And I recognize I don't want to spend time now searching it in the Drupal codebase myself as you probably know where the class is much better than me (not difficult given that I haven't looked at how Drupal is organizing its codebase)

@Crell
Copy link
Contributor

Crell commented Nov 24, 2014

@dosten
Copy link
Contributor

dosten commented Jun 24, 2015

Any feedback about this? I like the idea because is a common use case of the compiler passes, with this we can reduce the code necessary to do something like this.

@TomasVotruba
Copy link
Contributor

@Crell Thanks for implementation. This would be really great in standalone bundle.

Also it would be really nice to have this as an option. Though line to make it a bundle is thin.
Already done internally by:

  • event dispatcher
services:
    app.exception_subscriber:
        class: AppBundle\EventSubscriber\ExceptionSubscriber
        tags:
            - { name: kernel.event_subscriber }
  • console commands
services:
    app.command.my_command:
        class: AppBundle\Command\MyCommand
        tags:
            -  { name: console.command }

Externally by any tag system as written by @dosten and @dawehner

@patrick-mcdougle
Copy link
Contributor

Why aren't all the breadcrumb builders registered as listeners to a build breadcrumb event? If a listener returns a breadcrumb, can't you just stop propagation and return? Or am I misunderstanding the purpose of the breadcrumb manager?

@dawehner
Copy link
Contributor Author

@patrick-mcdougle Well, of course this could work as well, BUT conceptually I think having a custom interface for generating breadcrumbs for example is better than an event.

@patrick-mcdougle
Copy link
Contributor

But you're describing a custom dispatcher that has attached listeners, aren't you?

@chalasr
Copy link
Member

chalasr commented Feb 10, 2017

Now that we have support for custom tags in yaml (#21185) and maybe going to have true service locators (#21553), I think this could be great in the spirit of a lower entry bar.

Instead of having to know about compiler passes, based on #21553 it could be something like:

App\FooManager:
    arguments:
        - !service_locator [ !tagged 'foo']

@linaori
Copy link
Contributor

linaori commented Feb 10, 2017

While it increases complexity of the config like that, I do kinda like that feature. Though I'd love it to be lazy ;)

@chalasr
Copy link
Member

chalasr commented Feb 10, 2017

@iltar note that the example uses a !service_locator so it would be by nature :). If it was inside a !iterator the service would ends up with a RewindableGenerator, and if used as a direct argument then no laziness, just an array.

@slootjes
Copy link

In Drupal 8 I really like how I don't need to create compiler passes for simple service collection. Would love it is this would also be a part of Symfony Framework (maybe something for the SensioFrameworkExtraBundle?).

nicolas-grekas added a commit that referenced this issue Sep 28, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Reference tagged services in config

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

This is a proof of concept to reference a sequence of tagged services.

The problem bugs me for some time, and at first i thought the solution was to have some super generic compiler pass. If it could replace a lot of compilers in core.. perhaps worth it, but eventually each tag comes with it's own logic, including how to deal with tag attributes.

However, writing the passes over and over again becomes tedious for the most basic usecase. So given the recent developments, this idea came to mind.

```yml
services:
    a:
        class: stdClass
        properties: { a: true }
        tags: [foo]

    b:
        class: stdClass
        properties: { b: true }
        tags: [foo]

    c:
        class: stdClass
        properties:
            #stds: !tagged_services foo (see #22198)
            stds: !tagged_services
                foo
```

```
dump(iterator_to_array($this->get('c')->stds));
```

```
array:2 [▼
  0 => {#5052 ▼
    +"a": true
  }
  1 => {#4667 ▼
    +"b": true
  }
]
```

Given the _basic_ example at https://symfony.com/doc/current/service_container/tags.html, this could replace that.

Any thoughts?

Commits
-------

979e58f [DI] Reference tagged services in config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests