[ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" #27697

nicolas-grekas · 2018-06-24T22:12:34Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#20656
License	MIT
Doc PR	-

By adding <tag name="proxy" interface="Some\ProxifiedInterface" /> to your service definitions, this PR allows generating interface-based proxies.

This would allow two things:

generating lazy proxies for final classes
wrapping a service into such proxy to forbid using the methods that are on a specific implementation but not on some interface - the proxy acting as a safe guard.

The generated proxies are always lazy, so that lazy=true/false makes no difference.

As a shortcut, you can also use lazy: Some\ProxifiedInterface to do the same (yaml example this time.)

unkind · 2018-06-24T22:41:09Z

The generated proxies are always lazy, so that lazy=true/false makes no difference.

Maybe throw exception for lazy: false then?

nicolas-grekas · 2018-06-25T06:30:03Z

throw exception for lazy: false

I'm going to do this good idea. I'm also wondering about allowing lazy: Some\Interface at the loader level. Would be equivalent to lazy: true + adding the tag. WDYT?

nicolas-grekas · 2018-06-25T15:50:01Z

Waiting for a resolution of Ocramius/ProxyManager#419

Status: needs work

nicolas-grekas · 2018-06-26T11:16:32Z

Waiting for a resolution of Ocramius/ProxyManager#419

Not needed actually, and won't be fixed in ProxyManager v2.1 which is in security-only fixes mode, but is the version compatible with PHP 7.1, thus Symfony 4.

This PR deals with the issue on its own. Ready.

Status: needs review

stof · 2018-06-26T11:37:44Z

src/Symfony/Bridge/ProxyManager/LazyProxy/Instantiator/LazyLoadingValueHolderFactoryV2.php

@@ -22,6 +23,11 @@ class LazyLoadingValueHolderFactoryV2 extends BaseFactory
 {
    private $generator;

+    public function getProxifiedClass(Definition $definition): ?string
+    {
+        return $this->getGenerator()->getProxifiedClass($definition);


that looks weird to me. getProxifiedClass(Definition $definition) is not part of ProxyGeneratorInterface used as return type.

I suggest keeping this logic in RuntimeInstantiator instead (or in a separate class if needed elsewhere)

stof · 2018-06-26T11:38:48Z

src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/LazyLoadingValueHolderGenerator.php

+        if (!$definition->isLazy()) {
+            throw new \InvalidArgumentException(sprintf('Invalid definition for service of class "%s": setting the "proxy" tag on a service requires it to be "lazy".', $definition->getClass()));
+        }
+        if (!isset($definition->getTag('proxy')[0]['interface'])) {


what if we have multiple tags ? Should we support implementing multiple interfaces ?

That's not supported by ProxyManager AFAIK (ping @Ocramius)
Should we throw?

No, implementing multiple interfaces is not supported. I tried doing so when playing around with DCI implementations, but never pursued, because the use-case is too narrow compared with the added complexity.

thanks for the confirmation, this situation is now throwing

I managed to make it work actually, now supports proxying several interfaces :)
When they're not compatible, a PHP fatal error is thrown at compile time, there is no other way around.

stof · 2018-06-26T11:39:49Z

src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/LazyTrait.php

     * @return $this
     */
-    final public function lazy(bool $lazy = true)
+    final public function lazy($lazy = true)
    {
        $this->definition->setLazy($lazy);


shouldn't this always set a bool here ?

same in all loaders btw

stof · 2018-06-26T11:41:55Z

src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

@@ -267,7 +267,10 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
        foreach (array('class', 'public', 'shared', 'synthetic', 'lazy', 'abstract') as $key) {
            if ($value = $service->getAttribute($key)) {
                $method = 'set'.$key;
-                $definition->$method(XmlUtils::phpize($value));
+                $definition->$method($value = XmlUtils::phpize($value));
+                if ('lazy' === $key && \is_string($value)) {


as lazy has a special logic, I would rather remove it from the loop and handle it on its own (as done for some other properties)

stof · 2018-06-26T11:45:30Z

Do we still need LazyLoadingValueHolderFactoryV1 in master ?

nicolas-grekas · 2018-06-26T17:48:48Z

@stof comments addressed, thanks.

…\ProxifiedInterface"

fabpot · 2018-07-09T14:47:57Z

Thank you @nicolas-grekas.

…ith "lazy: Some\ProxifiedInterface" (nicolas-grekas) This PR was merged into the 4.2-dev branch. Discussion ---------- [ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20656 | License | MIT | Doc PR | - By adding `<tag name="proxy" interface="Some\ProxifiedInterface" />` to your service definitions, this PR allows generating interface-based proxies. This would allow two things: - generating lazy proxies for final classes - wrapping a service into such proxy to forbid using the methods that are on a specific implementation but not on some interface - the proxy acting as a safe guard. The generated proxies are always lazy, so that lazy=true/false makes no difference. As a shortcut, you can also use `lazy: Some\ProxifiedInterface` to do the same (yaml example this time.) Commits ------- 1d9f1d1 [ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface"

…ers and pass it to child definitions (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [DependencyInjection] Fix "proxy" tag: resolve its parameters and pass it to child definitions | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Forgotten in #27697 "proxy" tags are special: they must follow like the "lazy" attribute of definitions. Commits ------- 4cc3b3d [DependencyInjection] Fix "proxy" tag: resolve its parameters and pass it to child definitions

…oxies out of the box (nicolas-grekas) This PR was merged into the 6.2 branch. Discussion ---------- [DependencyInjection] Use lazy-loading ghost object proxies out of the box | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #35345 | License | MIT | Doc PR | - This PR builds on #46751. It also replaces #46458. Instead of using ProxyManager to make lazy services actually lazy, using `LazyGhostObjectTrait` from #46751 allows doing so *out of the box* - aka without the need to install any optional dependencies. When a virtual proxy is required (typically when using [the `proxy` tag](#27697)), ProxyManager is still required (and the dep remains optional.) But for most services, using `LazyGhostObjectTrait` just works \o/ Commits ------- 58a1848 [DependencyInjection] Use lazy-loading ghost object proxies out of the box

…oxies out of the box (nicolas-grekas) This PR was merged into the 6.2 branch. Discussion ---------- [DependencyInjection] Use lazy-loading ghost object proxies out of the box | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #35345 | License | MIT | Doc PR | - This PR builds on #46751. It also replaces #46458. Instead of using ProxyManager to make lazy services actually lazy, using `LazyGhostObjectTrait` from #46751 allows doing so *out of the box* - aka without the need to install any optional dependencies. When a virtual proxy is required (typically when using [the `proxy` tag](symfony/symfony#27697)), ProxyManager is still required (and the dep remains optional.) But for most services, using `LazyGhostObjectTrait` just works \o/ Commits ------- 58a184826a [DependencyInjection] Use lazy-loading ghost object proxies out of the box

nicolas-grekas added this to the next milestone Jun 24, 2018

carsonbot added Status: Needs Review Feature labels Jun 24, 2018

nicolas-grekas mentioned this pull request Jun 24, 2018

[DI] Support generating interface-bound proxies #20656

Closed

nicolas-grekas force-pushed the di-proxy branch from f80f0f9 to 9078b38 Compare June 24, 2018 22:14

nicolas-grekas force-pushed the di-proxy branch from 9078b38 to 02c27ca Compare June 25, 2018 07:55

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jun 25, 2018

nicolas-grekas force-pushed the di-proxy branch from 02c27ca to def6c54 Compare June 25, 2018 18:45

nicolas-grekas changed the title ~~[ProxyManagerBridge] allow proxifying interfaces with <tag name="proxy" interface="Some\ProxifiedInterface" />~~ [ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" Jun 25, 2018

nicolas-grekas added DependencyInjection ProxyManagerBridge labels Jun 25, 2018

nicolas-grekas force-pushed the di-proxy branch 5 times, most recently from 287c039 to 534200c Compare June 26, 2018 10:06

carsonbot added Status: Needs Review and removed Status: Needs Work labels Jun 26, 2018

nicolas-grekas force-pushed the di-proxy branch 3 times, most recently from 3a2b6ce to 311cbd5 Compare June 26, 2018 11:41

stof reviewed Jun 26, 2018

View reviewed changes

nicolas-grekas force-pushed the di-proxy branch from 311cbd5 to 0614220 Compare June 26, 2018 17:48

nicolas-grekas force-pushed the di-proxy branch 4 times, most recently from d6a94d7 to 5094785 Compare June 28, 2018 05:17

[ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some…

1d9f1d1

…\ProxifiedInterface"

nicolas-grekas force-pushed the di-proxy branch from 5094785 to 1d9f1d1 Compare July 9, 2018 14:45

carsonbot removed the Status: Needs Review label Jul 9, 2018

fabpot approved these changes Jul 9, 2018

View reviewed changes

carsonbot added the Status: Reviewed label Jul 9, 2018

fabpot merged commit 1d9f1d1 into symfony:master Jul 9, 2018

nicolas-grekas deleted the di-proxy branch July 22, 2018 20:25

javiereguiluz mentioned this pull request Sep 7, 2018

[ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" symfony/symfony-docs#10295

Closed

vhejda mentioned this pull request Oct 22, 2018

Lazy services - Final classes cannot be proxied by ProxyManager schmittjoh/JMSSerializerBundle#690

Closed

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

This was referenced Nov 3, 2018

Release v4.2.0-BETA1 #29072

Closed

Release v4.2.0-BETA1 #29073

Merged

ogizanagi mentioned this pull request Dec 29, 2018

Using a parameterised boolean false for the lazy tag #29717

Closed

garak mentioned this pull request Dec 2, 2020

Issue with lazy Paginator and knp-components@3.0 KnpLabs/KnpPaginatorBundle#680

Closed

ausi mentioned this pull request Mar 22, 2021

remove final from drivers php-imagine/Imagine#760

Merged

nicolas-grekas mentioned this pull request May 24, 2022

[DependencyInjection] Fix "proxy" tag: resolve its parameters and pass it to child definitions #46448

Merged

barw4 mentioned this pull request Jun 8, 2022

IBX-2934: Proxified FieldsGroupsList with lazy ezsystems/ezplatform-kernel#314

Merged

6 tasks

nicolas-grekas mentioned this pull request Jun 23, 2022

[DependencyInjection] Use lazy-loading ghost object proxies out of the box #46752

Merged

Uh oh!

[ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" #27697

[ProxyManagerBridge][DI] allow proxifying interfaces with "lazy: Some\ProxifiedInterface" #27697

Uh oh!

Conversation

nicolas-grekas commented Jun 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

unkind commented Jun 24, 2018

Uh oh!

nicolas-grekas commented Jun 25, 2018

Uh oh!

nicolas-grekas commented Jun 25, 2018

Uh oh!

nicolas-grekas commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof commented Jun 26, 2018

Uh oh!

nicolas-grekas commented Jun 26, 2018

Uh oh!

fabpot commented Jul 9, 2018

Uh oh!

Uh oh!

nicolas-grekas commented Jun 24, 2018 •

edited

Loading

nicolas-grekas Jun 27, 2018 •

edited

Loading