Skip to content

[DependencyInjection] Attribute : Decorator + Autoconfigure does not work #49931

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.

Sign up for GitHub

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

Jump to bottom
Closed
lyrixx opened this issue Apr 4, 2023 · 9 comments
Closed

[DependencyInjection] Attribute : Decorator + Autoconfigure does not work #49931

lyrixx opened this issue Apr 4, 2023 · 9 comments
Labels
Bug DependencyInjection Status: Needs Review

Comments

@lyrixx
Copy link
Member

lyrixx commented Apr 4, 2023 •
edited
Loading

Symfony version(s) affected

6.2 at least

Description

If we use both attribute, it does not work: the service is not tagged.

Trace

ReflectionException:
Trying to invoke abstract method App\Jerome\JeromeInterface::getNameStoredInDb()

at /home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/PriorityTaggedServiceTrait.php:151
at ReflectionMethod->invoke()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/PriorityTaggedServiceTrait.php:151)
at Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceUtil::getDefault()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/PriorityTaggedServiceTrait.php:83)
at Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass->findAndSortTaggedServices()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/ServiceLocatorTagPass.php:37)
at Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/ServiceLocatorTagPass.php:48)
at Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:91)
at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/ServiceLocatorTagPass.php:48)
at Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/ServiceLocatorTagPass.php:48)
at Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass->processValue()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:47)
at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->process()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/Compiler/Compiler.php:82)
at Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/dependency-injection/ContainerBuilder.php:757)
at Symfony\Component\DependencyInjection\ContainerBuilder->compile()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/http-kernel/Kernel.php:546)
at Symfony\Component\HttpKernel\Kernel->initializeContainer()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/http-kernel/Kernel.php:789)
at Symfony\Component\HttpKernel\Kernel->preBoot()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/http-kernel/Kernel.php:190)
at Symfony\Component\HttpKernel\Kernel->handle()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/autoload_runtime.php:35)
at require_once('/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/vendor/autoload_runtime.php')
(/home/gregoire/dev/labs/symfony/repro-dic-locator-factory/public/index.php:5)

How to reproduce

#[AsDecorator(HelloInterface::class)]
#[AutoconfigureTag('FOOBAR')]
class Decorator implements HelloInterface
{
    public function __construct(
        private readonly HelloInterface $hello,
    ) {
    }

    public function hello(): string
    {
        return '🎉🎉🎉 ' . $this->hello->hello() . ' 🎉🎉🎉';
    }
}

https://github.com/lyrixx/test/tree/sf-repro-tag-decorator

Possible Solution

No response

Additional Context

No response
@lyrixx lyrixx added the Bug label Apr 4, 2023
@krciga22
Copy link
Contributor

Hi @lyrixx, I've taken a look into the issue and was able to recreate it on v6.2 using your example within a test case. It looks like some code was added in v4.2 specifically to prevent auto-configured tags from being added to service decorators for specific reasons:

symfony/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php

Line 126 in 746c06b

// Don't add tags to service decorators

See Issue #30391 and PR #30417

It looks like one current solution is to add the FOOBAR tag to the parameter container.behavior_describing_tags in your services.yaml configuration. This would allow the tag to be added to any decorator service as needed using auto-configuration. See PR #38999

@HypeMC HypeMC mentioned this issue Jun 7, 2023
Merged
nicolas-grekas added a commit that referenced this issue Jul 7, 2023
@nicolas-grekas
bug #50595 [DependencyInjection] Don't ignore attributes on the actua…
7723dca 
…l decorator (HypeMC)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Don't ignore attributes on the actual decorator

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #49931
| License       | MIT
| Doc PR        | -

An attempt to fix #49931 without reintroducing the problem reported in #30391 and fixed by #30417. The idea here is that if the attribute is declared on the decorator itself, it shouldn't be ignored as it was explicitly added.

Commits
-------

fdd2ec2 [DependencyInjection] Don't ignore attributes on the actual decorator
@lyrixx
Copy link
Member Author

lyrixx commented Jul 31, 2023 •
edited
Loading

Hello,

Unfortunately, It still does not work as expected.

I have another exemple:

use Symfony\Component\Config\Loader\Loader;

#[Autoconfigure(tags: ['routing.loader'])]
#[AsDecorator('api_platform.route_loader')]
class RouteLoader extends Loader
{
    private const DOC_ROUTES = [
        'api_entrypoint',
        'api_doc',
    ];

    public function __construct(
        private readonly VisibilityDecider $visibilityDecider,
        private readonly ApiLoader $decorated,
        #[Autowire('%redirectionio.api_host%')]
        private readonly string $publicHost,
        #[Autowire('%redirectionio.frontend_host%')]
        private readonly string $privateHost,
    ) {
    }

When I look at the container.xml:

<service id="AppBundle\Api\Routing\RouteLoader" class="AppBundle\Api\Routing\RouteLoader" autowire="true" autoconfigure="true">
      <tag name="monolog.logger" channel="api"/>
      <argument type="service" id="AppBundle\Api\VisibilityDecider"/>
      <argument type="service" id="api_platform.route_loader.legacy"/>
      <argument>api.redirection-io.test</argument>
      <argument>redirection-io.test</argument>
    </service>

I miss the router.loader tag

@lyrixx lyrixx reopened this Jul 31, 2023
@lyrixx
Copy link
Member Author

lyrixx commented Jul 31, 2023

Note: If I add the tag from the YAML, it do works. So it should work via an attributes at some point!

@HypeMC
Copy link
Contributor

HypeMC commented Aug 1, 2023

@lyrixx I'm having trouble reproducing the problem, here's what I get:

<service id="App\Loader\RouteLoader" class="App\Loader\RouteLoader" autowire="true" autoconfigure="true">
  <tag name="routing.loader"/>
  <argument type="service" id="App\Loader\RouteLoader.inner"/>
</service>

and here's my example application: example-application.zip. Not sure what I'm doing differently.

@kissifrot
Copy link

Hello, I stumbled on this issue when trying to use attributes to decorate router and had no way to make it being called 😭

I also created a reproducer on https://github.com/kissifrot/sf-attribute-decorator-bug using 2 different services which should both be called (sf 6.4.3 is being used)

Same as @lyrixx , when using YAML, no issue, but nothing with attributes

@HypeMC

This comment was marked as outdated.

Sign in to view
@HypeMC
Copy link
Contributor

HypeMC commented Jan 31, 2024

@kissifrot Ignore my last response, I was wrong. I've double-checked and the reason the attribute is not working is because you are decorating a different service than in YAML. In YAML, you are decorating the router service, while in the attribute you are referencing RouterInterface:

-#[AsDecorator(decorates: RouterInterface::class, priority: 500)]
+#[AsDecorator(decorates: 'router', priority: 500)]
 class DecoratingUsingAttributesUrlGenerator implements RouterInterface
 {

The core problem here is that you have two different aliases: router, which is an alias for router.default, and RouterInterface, which is an alias for router. When you decorate RouterInterface, it becomes an alias for DecoratingUsingAttributesUrlGenerator, while router becomes an alias for DecoratingUsingServicesUrlGenerator.

Since Symfony internally uses router most of the time, you end up with DecoratingUsingServicesUrlGenerator.

@HypeMC
Copy link
Contributor

HypeMC commented Jan 31, 2024

I'm not sure if this is even a bug tbh, since you are decorating two different aliases. Maybe @nicolas-grekas can shed some light on this?

@lyrixx
Copy link
Member Author

lyrixx commented Aug 22, 2024

Just checked again, and it seems to work 🎉

@lyrixx lyrixx closed this as completed Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@kissifrot @nicolas-grekas @lyrixx @krciga22 @HypeMC @carsonbot