8000 Support for map of tagged services as service decency · Issue #6102 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

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

Closed
andrerom opened this issue Nov 24, 2012 · 19 comments
Closed

Support for map of tagged services as service decency #6102

andrerom opened this issue Nov 24, 2012 · 19 comments

Comments

@andrerom
Copy link
Contributor

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.

@lsmith77
Copy link
Contributor

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

@andrerom
Copy link
Contributor Author

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 )

@lsmith77
Copy link
Contributor

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.

@andrerom
Copy link
Contributor Author

True, I was talking about existing code that is already doing lazy loading by being ContainerAware.

@henrikbjorn
Copy link
Contributor

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.

@schmittjoh
Copy link
Contributor

I've added a similar feature to JMSDiExtraBundle recently in case you're using the Standard Edition:
http://jmsyst.com/bundles/JMSDiExtraBundle/master/lazy_service_collections

@stof
Copy link
Member
stof commented Dec 4, 2012

@schmittjoh why not adding it in core ?

@schmittjoh
Copy link
Contributor

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.

@fabpot
Copy link
Member
fabpot commented Apr 21, 2013

Closing as the discussion should now happen on #7527

@fabpot fabpot closed this as completed Apr 21, 2013
@andrerom
Copy link
Contributor Author

@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).

@lsmith77
Copy link
Contributor

@andrerom but if we have native support to inject any service as a lazy loaded service do we really need this special feature?

@andrerom
Copy link
Contributor Author

There is no support for getting out of the box list of tagged services.

@lsmith77
Copy link
Contributor

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.

@andrerom
Copy link
Contributor Author

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
Option B "Inject Factory": Better, but still affects the service
Option C "Compiler pass": Better, this is what we currently do, but extra code needed for a basic Dependency injection need: get map of tagged services
Option D "Native" A very basic POC can be found here, but unfortunately not using the Symfony DIC: https://github.com/andrerom/HiMVC/blob/master/HiMVC/Core/Common/DependencyInjection/ArrayObject.php

@stof
Copy link
Member
stof commented Apr 22, 2013

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.

@andrerom
Copy link
Contributor Author

@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.

@fabpot fabpot reopened this Apr 23, 2013
fabpot added a commit that referenced this issue May 6, 2013
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
@fabpot
Copy link
Member
fabpot commented Sep 12, 2013

Closing again as there is an alternative implementation in an existing bundle.

@fabpot fabpot closed this as completed Sep 12, 2013
@henrikbjorn
Copy link
Contributor

@stof if it requires an array cant i just be an array of Proxies?

@stof
Copy link
Member
stof commented Sep 13, 2013

@henrikbjorn you could indeed pass it an array of lazy services, which are now supported (they were not at the time of my comment)

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

6 participants
0