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

Improve functionality & usefulness of tagging in DIC #2085

Closed
atrauzzi opened this issue Sep 4, 2011 · 16 comments
Closed

Improve functionality & usefulness of tagging in DIC #2085

atrauzzi opened this issue Sep 4, 2011 · 16 comments

Comments

@atrauzzi
Copy link

atrauzzi commented Sep 4, 2011

I got into a situation this weekend where it would have been nice to have all services with a specific tag be easily available to a controller. Whether through method invocation injection (via routing.yml), or via something like $this->get().

After a lot of google-digging, I found this: https://gist.github.com/785089

It's not an ideal solution, but it's close and accomplishes two things:

  • Can pick up on taggings from other bundles by aggregating late.
  • Actually allows me to make any kind of use of tagging.

It also however raised some concerns in my mind that Symfony's DIC tagging is underdeveloped:

  • There's no way I would have come up with this solution if I hadn't gotten lucky and stumbled upon it (and subsequently understood it!)
  • Nowhere does the documentation talk about what you can do with tags after you've gone around declaring them on your services. ( http://symfony.com/doc/2.0/book/service_container.html#tags-tags )
  • As an indicator that this isn't just a documentation issue: There's not a lot of use for tags beyond a few Symfony & Twig tags listed on the web site. Making the feature seem intended only for occasional internal use. ( http://symfony.com/doc/2.0/reference/dic_tags.html )

Tagging seems like the perfect opportunity to allow for bundles to make use of each others' features via the DIC. But it has no real public interface for bundle developers to grab onto.

As a solution, I would propose adding the ability to refer to tagged-service collections in config and routing files. When passed to other classes, they would be a PHP array. A loose idea of this can be found in my best-case-scenario code here: http://stackoverflow.com/questions/7301303/can-you-use-parameters-from-the-dic-in-routes-method-parameter-injection

If you'd like to see my 'bigger picture', feel free to check out Nerve at: http://www.github.com/atrauzzi/nerve

@jalliot
Copy link
Contributor

jalliot commented Sep 4, 2011

Maybe I misunderstood something in your use-case but why don't you simply declare your controller as a service and simply give an array of your tagged services as a constructor argument (by adding a new compiler pass of course)?
Basically something like what I did here but the service being a controller in your case (which doesn't change anything).

I agree that the documentation is far from complete regarding the DIC though :)

@stof
Copy link
Member

stof commented Sep 4, 2011

This is a documentation issue. Tags are used in many places both in Symfony (the doc misses more than half of the tags used by the core bundles) and in shared bundles.
Tags are a useful feature of the DI component but it is not documented at all, and it is not the only one. The DI component misses lots of documentation for its features.

For your point about the routing, it seems to me like you are confusing things that are totally different. The common point between the DI component and the routing component is that they both let you use YAML and XML files to configure them. But the end is here. The content of these file is totally different, and you cannot use features used in service definitions of the DIC in the routing files.

@atrauzzi
Copy link
Author

atrauzzi commented Sep 4, 2011

I think that's a great idea! And I suppose this is where the documentation or correct design just did not make it to my brain. ;)
I had tried the "make my controller a service" approach. But got stuck trying to figure out how to get all the tagged services passed to it. At which point I ran out of research resources and went back to investigating for other options/solutions/approaches.

Specifically in the case of tags, I think the circumstance of them only returning other services from the same bundle is a distant exception rather than the norm.
There are definitely some deficiencies in how it's working right now. But my improvements would likely solve them:

  • References to a service tag are conducted late after all bundles have had their configs compiled.
  • Tags can be passed around in config and routing files so that the DIC can offer them as parameters to constructors, actions and other methods.

In situations where you only want tagged things from the current bundle (when that happens, I don't know), it seems like all you'd have to do is pick a different name for the tag.

@atrauzzi
Copy link
Author

atrauzzi commented Sep 4, 2011

Stof: If I'm not mistaken, that's a slight contradiction given that controllers defined as services are made available with short-hand references in routing.ymls: http://symfony.com/doc/2.0/cookbook/controller/service.html

I don't have any illusions about the relationship between config and routing ymls. But it does seem like there is some interplay there. I still think that there's missing functionality and documentation here: I wouldn't object to having to define my controller as a service just to make it DIC aware. The problem is making tagging function like the rest of the DIC.

Right now, it's just too esoteric. With my current solution, what happens if I end up needing another one? All I should think one would ever want to do with tagged services is retrieve them as a collection!

@stof
Copy link
Member

stof commented Sep 4, 2011

@atrauzzi, the routing only gives a string for the _controller attribute. The object is created after that by the ControllerResolver.

Tags are meant to be used in compiler passes, which are executed after all DI extensions have been called. and they are meant to allow having different services tagged with the same tag (as many as you want).

If you have your controller as service, you don't need to do anything about tagging in the routing file. The compiler pass can add a method call on the service (or change a constructor argument) to inject the needed services according to the tag.

Also, your proposal about passing tags as arguments does not make sense. A tag is not something that can be passed as argument but a flag set on a service definition with some attributes (for instance the event name for the kernel.event_listener tag)

@atrauzzi
Copy link
Author

atrauzzi commented Sep 4, 2011

The proposal isn't to pass the tag-string as an argument. It's to pass an array containing all services that are marked with a particular tag where indicated in config files.

(as I said earlier, I don't mind having to make a controller-service, so the routing.yml is really not a big deal to me)

Is there no desire to make this functionality more simple, or to bring it in line with how the rest of the config files behave? At this rate, what's the reason why we aren't just writing compiler passes instead of DIC config files?

The idea here is to make things more intuitive to use. Not cause two days of crawling documentation and 3+ classes to be implemented just to pass a collection of instances to a controller (when supposedly the tags were there in the first place to make things easier).

@stof
Copy link
Member

stof commented Sep 4, 2011

tags are only available when building the container as they are dropped when optimizing it. So the router has no way to access them (and you don't want to search by tags at runtime anyway: it forces to parse all definitions to look for the tags so it would have really bad performances)

@atrauzzi
Copy link
Author

atrauzzi commented Sep 4, 2011

That can't be what's preventing this very useful bit of functionality from becoming a reality.

You can mitigate that concern by adding each instance to a 2 dimensional array with the tag name as the first index. Once cached, at runtime, requesting all services with the tag returns the array belonging to it. Which ends up being no different than requesting a bunch of services back to back.

@Seldaek
Copy link
Member

Seldaek commented Sep 5, 2011

@atrauzzi: BTW, somewhat unrelated to the issue at hand, but speaking of stuff that you may have just missed by accident, have you seen the SymfonyCMF project? Might be nice to join instead of doing your own CMS thing on the side :)

@jalliot
Copy link
Contributor

jalliot commented Sep 5, 2011

@stof Maybe it would be possible to implement a new type of argument in service definition files that would basically do the exact same thing that a basic compiler pass do (that could be a collection of the services). That would all still happen at compile-time and would avoid people to write compiler passes for basic stuff.

Maybe something like:

<service id="some_id" class="...">
    <argument type="tagged_collection" tag="some_tag" />
</service>

@atrauzzi
Copy link
Author

atrauzzi commented Sep 5, 2011

@Seldaek I am familiar with it and am watching it pretty closely. :)
If anything they make works for me, for sure I'll make use of it, but I doubt anything I make would be accepted by the project.

Keep in mind, they're making an "F" and I'm making an "S". :)

@atrauzzi
Copy link
Author

atrauzzi commented Sep 6, 2011

@jalliot That's more or less what my suggestion has been.

@Problematic
Copy link

What's the status of this? I've needed something simple like @jalliot's proposal too.

@TyrannicalPidgeon
Copy link

@Problematic, @atrauzzi, @jalliot

It seems what you are asking for can be acomplished in a few lines of code in your own bundles. This is just sample code(untested) but it should be plenty to get you started

Service definitions:

foo:
    class: Doctrine\Common\Collections\ArrayCollection
    tags: [{ name="tagged_collection" tag="some_tag" }

bar:
    class: ...
    tags: [{ name="some_tag" }]

baz:
    class: ...
    tags: [{ name="some_tag" }]

Compiler Pass

namespace Foo\BarBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;

class TaggedCollectionCompilerPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        foreach ($container->findTaggedServiceIds('tagged_collection') as $serviceId => $arguments) {
            $services = array();
            foreach ($container->findTaggedServiceIds($arguments['tag']) as $nestedServiceId => $_) {
                $services[] = new Reference($nestedServiceId);
            }
            $container->getDefinition($serviceId)->setArguments(array($services));
        }
    }
}

@atrauzzi
Copy link
Author

This is more than a few lines of code and probably a bit more involved than what some are able to handle. Tagging as it's presented doesn't to me imply rummaging around with compiler passes! I don't see the threat in making a simple method to fetch things of a particular tag. It's the only thing you could possibly want to do with it!

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

@atrauzzi closing this as tags are only intended to be used during the DIC construction as explained by @stof. What you need could probably be implemented in a generic compiler pass.

@vicb vicb closed this as completed Mar 8, 2012
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

7 participants