[RFC][DI] tagging system like event subscribers (autoconfigure) #25327

linaori · 2017-12-05T08:25:07Z

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes
Symfony version	4.1

Still being on 3.3, my first preparation in my application is to change everything to autowiring. Now that we have autoconfigure and resource loading, it's a lot easier to get things started with ~1900 services in my app.

However, there are some obstacles I'm hitting that still require me to define a lot of services in my config. While we can currently already use !tagged to avoid using compiler passes in 3.4, I'm still required to configure a priority when this is needed. In some cases, I have ~20 service definitions that I need to sort and inject into another.

Now my autowire.yml looks something like this:

services:
    #...
    App\Option\One:
        tags:
            - { name: app.option, priority: 10 }
            - { name: features.tag, tag: some_feature_name }
    App\Option\Two: { tags: [{name: app.option, priority: 15}] }
    App\Option\Three: { tags: [{name: app.option, priority: 100}] }
    App\Option\Four: { tags: [{name: app.option, priority: -10}] }

    App\Feature\EnvironmentResolver:
        tags: [{ name: features.resolver, config-key: environment }]
    #...

Ideally I'd only define the service definition that collects these options, so I've been wondering if a tag subscriber would be the solution. Similar to event listeners vs event subscribers, this would allow a class to subscribe to specific tags, which can be picked up by autoconfigure:

interface TagSubscriberInterface
{
    public static function getTags(): array;
}

final class One implements OptionInterface, TagSubscriberInterface
{
    // ...
    public static function getTags()
    {
        // multiple tags
        return [
            ['name' => 'app.option', 'priority' => 10],
            ['name' => 'features.tag', 'tag' => 'some_feature_name'],
        ];
        // different style in case of single tag
        return ['name' => 'app.option', 'priority' => 10];
    }
}

final class EnvironmentResolver implements FeatureResolverInterface, TagSubscriberInterface
{
    // ...
    public static function getTags()
    {
        return ['name' => 'features.resolver', 'config-key' => 'environment'];
    }
}

WDYT?

@Simperfit 😄

The text was updated successfully, but these errors were encountered:

stof · 2017-12-05T08:39:04Z

!tagged already supports priority (it does not support other custom attributes though, as it would not know what to do with them). This might just be missing in the documentation of the feature.

I'm -1 on adding TagSubscriberInterface in the core. Making classes exposing their DI tags themselves introduces a coupling to the config component again (as the concept of tags is purely a DI one).
You should rather define an interface for your class allowing a compiler pass to avoid requiring mandatory tag attributes, but getting the info from the class instead (which woul 8000 d be the actual comparison of event listener vs event subscriber, unlike your proposal to expose tags themselves in the class).

On a side note, you can implement TagSubscriberInterface in your own code if you really want it in your project:

write a compiler pass (registered with a high priority so that it runs before most other passes run, but still running after the autoconfiguration pass) which would get services tagged as app.tag_subscriber) and inject needed tags in your definition (be careful to avoid dropping tags added by autoconfiguration though)
register the autoconfiguration rule for your interface, adding the app.tag_subscriber tag.

linaori · 2017-12-05T08:57:06Z

!tagged already supports priority (it does not support other custom attributes though, as it would not know what to do with them). This might just be missing in the documentation of the feature.

Indeed, that's why it's a really nice thing to start with. However, I still have to define the priority (and other attributes) somewhere, which beats the purpose of the resource loading and makes my service config huge.

Making classes exposing their DI tags themselves introduces a coupling to the config component again (as the concept of tags is purely a DI one).

It's optional and not any different from how the ServiceSubscriberInterface or EventSubscriberInterface work. Additionally, it would mean that a lot of bundles will provide a similar solution for just their own tags to enable some form of autoconfigure.

You should rather define an interface for your class allowing a compiler pass to avoid requiring mandatory tag attributes, but getting the info from the class instead (which would be the actual comparison of event listener vs event subscriber, unlike your proposal to expose tags themselves in the class).

I would like to solve this problem in the core (if possible) with a generic solution to prevent a wild spread of similar implementations and therefore bugs or code duplication.

Is there currently a neat way of collecting this static information from a service definition? The EventSubscriberInterface implementation looked kinda complex to be able to just hook in.

On a side note, you can implement TagSubscriberInterface in your own code if you really want it in your project:

Yes, I can certainly add it myself, that's not really the problem, but I'm pretty sure I'm not the only one running against this particular issue. I can always create a bundle to provide this feature for more so that's not really a problem either.

So based on your feedback, what about something that allows people to write attribute targeting interfaces?

class FooBundle
{
    public function load(ContainerBuilder $builder)
    {
        // just and example
        $container->mapInterfaceToTagAttribute(PriorityInterface::class, 'priority', [
            'app.option', 'app.selection'
        ]);
        $container->mapInterfaceToTagAttribute(FeatureConfigKeyInterface::class, 'config-key', [
            'features.resolver'
        ]);
        $container->mapInterfaceToTagAttribute(FeatureTagKeyInterface::class, 'tag', [
            'features.tag'
        ]);
    }
}

class Foo implements PriorityInterface, FeatureConfigKeyInterface, FeatureTagKeyInterface
{
    public static function getPriority(): int { return 10; }
    public static function getConfigKey(): string { return 'environment'; }
    public static function getFeatureTag(): string { return 'some_feature_name'; }
}

chalasr · 2017-12-05T09:02:35Z

For the record, TagSubscriberInterface was rejected in #22649

stof · 2017-12-05T09:18:52Z

It's optional and not any different from how the ServiceSubscriberInterface or EventSubscriberInterface

There is a big difference with EventSubscriberInterface. The API exposed by EventSubscriberInterface is related to the feature of the event dispatcher component (btw, it is even usable without using DI at all).
ServiceSubscriberInterface is exposing details about the PSR-11 container that the class itself is using (expliciting what the class expects to find in it).
Your TagSubscriberInterface asks the class to expose some DI related thing, not something related to the behavior of the class itself.

So based on your feedback, what about something that allows people to write attribute targeting interfaces?

if you need custom attributes on your tags, you already need a custom compiler pass to process them (!tagged will never do anything with your config-key attribute). And then, it is much easier to call getConfigKey directly from your compiler pass rather than adding new features in the DI component for that (which has already been suggested in the past and rejected precisely for this reason).

linaori · 2017-12-05T09:31:05Z

Alright, so the best course is to create something myself and make this publicly available in case others will need it as well.

weaverryan · 2017-12-06T03:39:01Z

And then, if it’s widely used, it would have a better chance in core. Your use-case makes sense to me, but I don’t how common it is :).

nicolas-grekas · 2017-12-06T10:34:49Z

👎 also: this inlines some important logic/knowledge in a DI extension, instead of a compiler pass. This means the logic is not reusable anymore. That's a serious regression as far as design is concerned.

backbone87 · 2017-12-11T15:30:20Z

how about reducing the verbosity by adding "service template parameters":

services:
    #...
    _instanceof:
      App\OptionInterface:
        tags:
          - { name: app.option, priority: "%var(app.option.priority:10)%" }

    #App\Option\One: # default value of 10 applies
    App\Option\Two:
      vars:
        app.option.priority: 15
    App\Option\Three:
      vars:
        app.option.priority: 100
    App\Option\Four:
      vars:
        app.option.priority: -10

    #...

linaori · 2017-12-11T18:50:33Z

@backbone87 then I'd still have to define each and every dependency, while I would want to utilize autoconfigure for this

javiereguiluz · 2018-01-03T08:34:12Z

@iltar sadly I must close this proposal as "won't fix" because of the downvotes from the Core Team, but I hope you keep sending proposals to improve Symfony. Thanks!

xabbuh added DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Dec 5, 2017

linaori mentioned this issue Dec 28, 2017

[RFC][DependencyInjection] Allow set DI attributes using annotations #25592

Closed

javiereguiluz closed this as completed Jan 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[RFC][DI] tagging system like event subscribers (autoconfigure) #25327

[RFC][DI] tagging system like event subscribers (autoconfigure) #25327

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!