[DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute #45834

Jean-Beru · 2022-03-24T08:25:11Z

Q	A
Branch?	6.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets
License	MIT
Doc PR	WIP

The aim f this PR is to allow decorator declaration with PHP8 attributes by using AsDecorator attribute on class and, optionally, InnerService parameter attribute to inject decorated service.

To use it :

interface FooInterface
{
    public function myMethod(): string;
}

final class Foo implements FooInterface
{
    public function myMethod(): string
    {
        return 'foo';
    }
}

#[AsDecorator(
    decorates: Foo::class, 
    priority: 5, 
    onInvalid: ContainerInterface::NULL_ON_INVALID_REFERENCE,
)]
final class Bar implements FooInterface
{
    private string $arg1;
    private ?FooInterface $foo;

    public function __construct(string $arg1, #[InnerService] FooInterface $foo = null)
    {
        $this->arg1 = $arg1;
        $this->foo = $foo;
    }

    public function myMethod(): string
    {
        if (null === $this->foo) {
            return 'bar';
        }

        return $this->foo->myMethod().' bar ';
    }
}

nicolas-grekas · 2022-03-24T10:51:40Z

Thanks for the PR.

I see several issues with the current approach:

loading all classes in the container is no-go (this could have a huge perf impact at compile-time)
unconditionally looking for attributes is no-go (this breaks their declarative property by making them imperative configuration)
we should avoid using a generic name (Service) - future needs can be handled by new attributes. We could use #[AsDecorator] maybe?
decorationArgumentKey and decorationInnerName don't make sense to me. We could use another attribute instead, put on an argument, to tell where the decorated service should be injected, eg function __construct(#[DecoratedService] $decorated), or #[Inner].

Instead of adding a new pass, I think the attributes should be looked for in AutowirePass, where there is already some logic that deals with decoration. This would fix all listed issues I believe.

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

src/Symfony/Component/DependencyInjection/Compiler/RegisterDecoratorAttributesPass.php

nicolas-grekas · 2022-04-11T17:20:08Z

(please mind the PR title+description also ;))

Jean-Beru · 2022-04-14T20:27:42Z

Thanks for your reviews ! Code and description have been updated :)

nicolas-grekas

LGTM. Can you please confirm that you ran this on a small project and that it worked as expected?

dunglas

👍

Jean-Beru · 2022-04-15T13:26:12Z

I tried from a new demo application with :

final class Foo implements FooInterface
{
    public function __invoke(): string
    {
        return 'new '.__CLASS__.'()';
    }
}

#[AsDecorator(decorates: Foo::class, decorationPriority: 10)]
final class Bar10 implements FooInterface
{
    private FooInterface $foo;

    public function __construct(#[InnerService] FooInterface $foo)
    {
        $this->foo = $foo;
    }

    public function __invoke(): string
    {
        return 'new '.__CLASS__.'('.($this->foo)().')';
    }
}

#[AsDecorator(decorates: Foo::class, decorationPriority: 20)]
final class Bar20 implements FooInterface
{
    private FooInterface $foo;

    public function __construct(#[InnerServicce] FooInterface $foo)
    {
        $this->foo = $foo;
    }

    public function __invoke(): string
    {
        return 'new '.__CLASS__.'('.($this->foo)().')';
    }
}

#[AsController]
class TestController
{
    private FooInterface $foo;

    public function __construct(#[Autowire(service: Foo::class)] FooInterface $foo)
    {
        $this->foo = $foo;
    }

    #[Route('/test/')]
    public function index(): Response
    {
        return new Response(($this->foo)());
    }
}

Calling /test/ returns new App\Services\Bar10(new App\Services\Bar20(new App\Services\Foo())).

nicolas-grekas

Thanks! I'm just bike-shedding on the naming, WDYT?
Esp Inner vs DecoratedService? (or just Decorated)?
Any opinion @symfony/mergers? (see #45834 (comment) for examples)

src/Symfony/Component/DependencyInjection/Attribute/AsDecorator.php

src/Symfony/Component/DependencyInjection/Attribute/Inner.php

…e parameter attribute

nicolas-grekas · 2022-04-19T13:20:13Z

Thank you @Jean-Beru.

…apDecorated]` (chalasr) This PR was merged into the 6.1 branch. Discussion ---------- [DependencyInjection] Rename `#[InnerService]` to `#[MapDecorated]` | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | no but tweaks one #45834 | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When talking about the decorator pattern outside of Symfony, the term `inner` is pretty much (if not totally) inexistent. Worse, it makes the concept harder to explain. Take "there is a decorator and a decorated object" vs "there is a decorator and an inner object": using the later form, one has to explain both what is the decorator and what is the inner. While using the former, explaining what the decorator makes it obvious what the decorated is. I propose to take the addition of this attribute as an opportunity to start making this special term go away. Also I removed the `Service` suffix. It is tempting to keep it for explicitness, but it feels kinda redundant and AFAIK no other core attribute has such redundant suffix, maybe because their namespace is enough to indicate their target. Commits ------- c0a7979 [DependencyInjection] Rename `#[InnerService]` to `#[MapDecorated]`

carsonbot added Status: Needs Review DependencyInjection Feature labels Mar 24, 2022

carsonbot added this to the 6.1 milestone Mar 24, 2022

Jean-Beru requested a review from dunglas as a code owner April 8, 2022 21:00

nicolas-grekas reviewed Apr 11, 2022

View reviewed changes

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Apr 11, 2022

View reviewed changes

Jean-Beru changed the title ~~[DependencyInjection] add Service class attribute~~ [DependencyInjection] add AsDecorator class attribute and Inner parameter attribute Apr 14, 2022

nicolas-grekas approved these changes Apr 14, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 14, 2022

dunglas approved these changes Apr 14, 2022

View reviewed changes

nicolas-grekas reviewed Apr 15, 2022

View reviewed changes

src/Symfony/Component/DependencyInjection/Attribute/AsDecorator.php Outdated Show resolved Hide resolved

src/Symfony/Component/DependencyInjection/Attribute/Inner.php Outdated Show resolved Hide resolved

Jean-Beru changed the title ~~[DependencyInjection] add AsDecorator class attribute and Inner parameter attribute~~ [DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute Apr 15, 2022

[DependencyInjection] add AsDecorator class attribute and InnerServic…

d8a4680

…e parameter attribute

nicolas-grekas force-pushed the di/add-service-attribute branch from c0c5468 to d8a4680 Compare April 19, 2022 13:20

nicolas-grekas merged commit 84d35a2 into symfony:6.1 Apr 19, 2022

nicolas-grekas mentioned this pull request Apr 19, 2022

[DependencyInjection] add AsDecorator class attribute and InnerService … symfony/symfony-docs#16731

Closed

chalasr mentioned this pull request Apr 19, 2022

[DependencyInjection] Rename #[InnerService] to #[MapDecorated] #46112

Merged

Jean-Beru deleted the di/add-service-attribute branch April 19, 2022 16:23

fabpot mentioned this pull request Apr 27, 2022

Release v6.1.0-BETA2 #46195

Merged

tjveldhuizen mentioned this pull request May 3, 2023

Allow multiple AsDecorator attributes on a decorating class #50228

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute #45834

[DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute #45834

Jean-Beru commented Mar 24, 2022 •

edited by nicolas-grekas

Loading

nicolas-grekas commented Mar 24, 2022 •

edited

Loading

nicolas-grekas commented Apr 11, 2022

Jean-Beru commented Apr 14, 2022

nicolas-grekas left a comment

dunglas left a comment

Jean-Beru commented Apr 15, 2022 •

edited by nicolas-grekas

Loading

nicolas-grekas left a comment •

edited

Loading

nicolas-grekas commented Apr 19, 2022

[DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute #45834

[DependencyInjection] add AsDecorator class attribute and InnerService parameter attribute #45834

Conversation

Jean-Beru commented Mar 24, 2022 • edited by nicolas-grekas Loading

nicolas-grekas commented Mar 24, 2022 • edited Loading

nicolas-grekas commented Apr 11, 2022

Jean-Beru commented Apr 14, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

dunglas left a comment

Choose a reason for hiding this comment

Jean-Beru commented Apr 15, 2022 • edited by nicolas-grekas Loading

nicolas-grekas left a comment • edited Loading

Choose a reason for hiding this comment

nicolas-grekas commented Apr 19, 2022

Jean-Beru commented Mar 24, 2022 •

edited by nicolas-grekas

Loading

nicolas-grekas commented Mar 24, 2022 •

edited

Loading

Jean-Beru commented Apr 15, 2022 •

edited by nicolas-grekas

Loading

nicolas-grekas left a comment •

edited

Loading