Tagged Service Locator - config option to enforce unique index

[wucdbm]

Description

Hey,

As per https://symfony.com/doc/current/service_container/service_subscribers_locators.html#indexing-the-collection-of-services

It would be awesome if we could enforce indexes to be unique for tagged service locators when using index method.

The issue with the current behaviour is that if I register another service, and its static method that is responsible for returning the (expeced to be unique) key returns a key that already exists, the existing service in the locator is replaced.
This may not always be the desired behavour. Sometimes you may want to be able to do so and could come in handy, but other times you may need to enforce uniqueness and disallow registering the same thing twice and consider it a mistake.

My suggestion is to add another config option, such as unique="true" (in XML), and upon a duplicate, a RuntimeException would be thrown, preventing you from running anything in the project until fixed.

Example
Before

<service id="App\HandlerCollection">
        <argument type="tagged_locator" tag="app.handler" index-by="key" 
            default-index-method="myOwnMethodName"/>
</service>

No error, another app.handler may replace an existing one.

After

<service id="App\HandlerCollection">
        <argument type="tagged_locator" tag="app.handler" index-by="key" 
            default-index-method="myOwnMethodName" unique="true"/>
</service>

A RuntimeError is thrown, with a message such as "Service for index %s already exists in this locator as %s. Adding %s with the same key is impossible for tagged locators with unique indexes."
1 - index value
2 - existing service id
3 - next service id to be added to the locator

I assume (didn't do the research yet) that tagged locators are computed in a compiler pass, so throwing the exception there would make sense? This is how I currently solve this.

This allows you to know immediately when something isn't quite right in the project during development, as soon as you make a mistake, rather than spending hours in debugging and figuring out why your unique collection is misbehaving.

WDYT?

[reyostallenberg]

Isn't a unique tagged service the same as getting a service by the service id? Or am I missing a use case in which you just can't use the service id or in which this proposal is more convenient?

[wucdbm]

@reyostallenberg Our use-case involves defining implementations with a fixed code (read: index) which are afterwards configured via a user interface, as per the user's needs. The goal of this feature is to reduce developer errors by not allowing them to add an implementation which has the same code by mistake.

Implementations are afterwards fetched dynamically from the locator, based on aforementioned user configs. If I were to add an implementation with the same code (index in terms of service locator uniqueness) of an existing one, I'd be introducing a hard-to-find bug, it'll be found too late, and will definitely be hard to debug.

As already mentioned, I'm solving this using a custom compiler pass of about 60 lines of code that guards against duplicates and throws a RuntimeError with a nice message. There's also another one that does a similar thing.

In general, that's not a big deal to support ourselves. But I thought it would be a nice convenience feature if we could achieve this with just config, where we'd need just an additional option to make the collection unique.

I hope that clarifies the situation. I'd like to get some feedback from other people and/or core members as to whether this could be useful to them too?

[nicolas-grekas]

I'm not really convinced we should do this personally. All services "global" services can be redefined at will and this is fine. I'm not sure scoped locators should be different. At least I'm not sure this is worth the extra technical and cognitive complexity.

[wucdbm]

@nicolas-grekas Perhaps I didn't explain it very well, but the main point here is not to prevent redefining, you can still do that, but to prevent you from having two services with the same index in the locator, in use-cases where that's actually a bug.

IMO it adds up for nice DX when you can count on the framework catching mistakes with some sort of a runtime exception during development. In prod, that doesn't affect speed as the container is compiled and only read.

Your call in the end, I could do the implementation if you guys would accept such feature. If not, I guess we can close the issue.

[stof]

This would prevent redefining the service corresponding to a given index (using priorities to control which one wins)

[wucdbm]

@stof Good point. I'll spend time on that next week.

In the end, maybe I could scribble a package that does that.

[wucdbm]

Giving up on this one. After giving it some thought, it's no big deal supporting a simple custom implementation, as opposed to supporting it in the framework for years to come with no big interest, so I come to agree @nicolas-grekas

If anybody wants to pick up on the idea, feel free to re-open