-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Support for map of tagged services as service decency #6102
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
Comments
This could indeed be a viable alternative to the proxy approach. The main draw back is that it needs modifications in the class using the dependency while the proxy approach works entirely on the DIC config level. BTW semi related there is also LiipContainerWrapperBundle |
I don't see it as a alternative, more as a supplement, slightly different but overlapping use cases. And the draw back you mention is the exact same as with the proxy proposal when dealing with existing code base, you have to remove some container use somewhere, for instance in your factories. The upside is that you avoid all the complex code generation you will need for the proxies, and the slight overhead when using your dependencies (function call overhead). But again, I don't see this as a full replacement for proxies, and I would actually like to have both in. ( LiipContainerWrapperBundle: interesting ) |
The proxy solution will allow you to use lazy loading without changing the service code .. all you need to do is change the service definition in the DIC. |
True, I was talking about existing code that is already doing lazy loading by being ContainerAware. |
As @andrerom said i think this is another use case, and not really a replacement for the proxy because in theory the tagged services could be proxies. |
I've added a similar feature to JMSDiExtraBundle recently in case you're using the Standard Edition: |
@schmittjoh why not adding it in core ? |
That's a good question, the answer to it is not that simple though. You can ofc work on a solution for the component, but I don't have the time. |
Closing as the discussion should now happen on #7527 |
@fabpot You closed to fast here, this has actually nothing to do with proxy services, it's about getting support for getting map of services, both eagerly and lazy loaded. Example: You have a bunch of services tagged which all implements a interface, another service depends on this list. Currently you have to create a Complier pass for every occurrence of this need, but this PR suggests adding a service definition and support for getting them as plain array (eagerly) or ArrayObject (with container hidden inside so lazy loaded). |
@andrerom but if we have native support to inject any service as a lazy loaded service do we really need this special feature? |
There is no support for getting out of the box list of tagged services. |
i am not really convinced that we need this in core. at this point we are just talking about being able to add items to a collection via a tag. that being said .. i am not saying that it would require a lot of code so maybe its useful syntax sugar to provide a standard compiler pass that does this. |
Well, this PR was still closed for the wrong reason :) The use case for eZ Publish is tagging all implementations of a SPI (Service Provider Interface), and other services in some cases depends on picking one of them using identifier. One example is FieldTypes (the different types of attributes you can have on a Content). An Article will need String, XML and Rating types, While a Folder only needs the first two by default, this is handled by our ContentTypeService. So from a pure dependency injection principle, we need a map (array/ArrayObject) of the different handlers( To get string instance, as simple: $this->fieldTypes['ezstring'] ), that is all the service should know about it's needs if you strip it down to the bare minimum. Option A "Inject Container": Bad, no decoupling, affects service |
If your service expects an array, there is no way to lazy-load it (there is no way to implement a lazy array as it is a native type) JMSDiExtraBundle provide a lazy implementation of the Map and Sequence of php-collection with the compiler pass to load them. See JMSSerializerBundle for an example using it. |
@stof Thanks, corrected my text to make it more clear the service would need to accept both native array and something like ArrayObject. I'm aware of JMSDiExtraBundle, will consider it in the future if this stays closed. |
This PR was squashed before being merged into the master branch (closes #7890). Discussion ---------- ProxyManager Bridge As of @beberlei's suggestion, I re-implemented #7527 as a new bridge to avoid possible hidden dependencies. Everything is like #7527 except that the new namespace (and possibly package/subtree split) `Symfony\Bridge\ProxyManager` is introduced | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #6140 (supersedes) #5012 #6102 (maybe) #7527 (supersedes) | License | MIT (attached code) - BSD-3-Clause (transitive dependency) | Doc PR | Please pester me to death so I do it This PR introduces lazy services along the lines of zendframework/zendframework#4146 It introduces an **OPTIONAL** dependency to [ProxyManager](https://github.com/Ocramius/ProxyManager) and transitively to [`"zendframework/zend-code": "2.*"`](https://github.com/zendframework/zf2/tree/master/library/Zend/Code). ## Lazy services: why? A comprehensive example For those who don't know what this is about, here's an example. Assuming you have a service class like following: ```php class MySuperSlowClass { public function __construct() { // inject large object graph or do heavy computation sleep(10); } public function doFoo() { echo 'Foo!'; } } ``` The DIC will hang for 10 seconds when calling: ```php $container->get('my_super_slow_class'); ``` With this PR, this can be avoided, and the following call will return a proxy immediately. ```php $container->getDefinitions('my_super_slow_class')->setLazy(true); $service = $container->get('my_super_slow_class'); ``` The 10 seconds wait time will be delayed until the object is actually used: ```php $service->doFoo(); // wait 10 seconds, then 'Foo!' ``` A more extensive description of the functionality can be found [here](https://github.com/Ocramius/ProxyManager/blob/master/docs/lazy-loading-value-holder.md). ## When do we need it? Lazy services can be used to optimize the dependency graph in cases like: * Webservice endpoints * Db connections * Objects that cause I/O in general * Large dependency graphs that are not always used This could also help in reducing excessive service location usage as I've explained [here](http://ocramius.github.com/blog/zf2-and-symfony-service-proxies-with-doctrine-proxies/). ## Implementation quirks of this PR There's a couple of quirks in the implementation: * `Symfony\Component\DependencyInjection\CompilerBuilder#createService` is now public because of the limitations of PHP 5.3 * `Symfony\Component\DependencyInjection\Dumper\PhpDumper` now with extra mess! * The proxies are dumped at the end of compiled containers, therefore the container class is not PSR compliant anymore Commits ------- 78e3710 ProxyManager Bridge
Closing again as there is an alternative implementation in an existing bundle. |
@stof if it requires an array cant i just be an array of Proxies? |
@henrikbjorn you could indeed pass it an array of lazy services, which are now supported (they were not at the time of my comment) |
Just like #5012 proposes a solution to avoid having to use container to be able to lazy load services, this one is a proposal to avoid injecting container in project code when you need a map of tagged services.
Overview
Service Container should be able to inject a map of services, by tag, where map key is the alias attribute, and value is the service.
Lazy loaded maps
If the proxy proposal is accepted, a lazy loaded map could be a plain array map of proxies. But proxie objects can be avoided in this case by instead returning an object that implements ArrayAccess + Traversable + Countable and behind the scenes uses container.
Known issue: This approach has the same downside as other ContainerAware code including proxies: serialization, var_dump and var_export.
The text was updated successfully, but these errors were encountered: