[DependencyInjection] Added a way to define the priority of service decoration #15416
Conversation
|
@dosten can you give a use case for prioritizing service's decorator ? I fail to understand :-) |
|
@dupuchba controlling the order in which multiple decorators for the same service are applied. |
|
btw, the issue requesting the change explains it already |
| */ | ||
| class DecoratorServicePass implements CompilerPassInterface | ||
| { | ||
| public function process(ContainerBuilder $container) | ||
| { | ||
| $definitions = new \SplPriorityQueue(); |
There was a problem hiding this comment.
The usage of a priority query is a BC break here. Currently, decorators with the same priority (i.e. all of them for now) are applied in the order in which they appear in $container->getDefinitions(), which is more or less the order in which services have been defined (it is a bit more complex when a service gets overridden actually). but a SplPriorityQueue does not retain the order of insertion for the same priority.
And this is inconsistent with all other places using priorities in Symfony as we preserve the insertion order when the priority is the same.
There was a problem hiding this comment.
@stof Thanks for your feedback, I missed up that part of the documentation. Using arrays and krsort is the way to go?
There was a problem hiding this comment.
@stof Can you check again? I've implemented the same solution of ProfilerPass.
|
ping |
|
👍 looks good to me |
|
👍 Please submit a PR to the documentation repo to document this new feature for the 2.8 branch |
|
IMO this can be labeled as reviewed, right? @stof 👍 |
|
The docs PR can be found here. |
|
@dosten indeed. note that anyone can trigger the status change thanks to @carsonbot (the update of the doc to describe our goals with it is not yet ready unfortunately) status: reviewed |
|
@stof Yes, I know, but I feel unconfortable changing the status by myself. |
|
👍 |
|
@dosten ah yeah, being the PR author, this seems fair to let someone else change. We have plans to implement a restriction about that in the bot itself in the future. I missed the fact you are the author when I commented |
|
Thank you @dosten. |
…ty of service decoration (dosten) This PR was merged into the 2.8 branch. Discussion ---------- [DependencyInjection] Added a way to define the priority of service decoration | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #10634 | License | MIT | Doc PR | symfony/symfony-docs#5600 This PR adds a way to define the priority of service decoration, so, the service with the highest priority will be applied first (the default priority is zero). ```yaml services: foo: class: Foo bar: class: Bar arguments: ['@bar.inner'] decorates: foo public: false foobar: class: Foobar arguments: ['@foobar.inner'] decorates: foo decoration_priority: 1 public: false ``` This will result in this code: ```php $this->services['foo'] = new Bar(new Foobar(new Foo))); ``` Commits ------- 75c98cb Added a way to define the priority of service decoration
…the service decoration priority (dosten) This PR was merged into the 2.8 branch. Discussion ---------- [DependencyInjection] Documented the ability of define the service decoration priority | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#15416) | Applies to | 2.8+ Commits ------- 5868190 Documented the ability of define service decoration priority
This PR adds a way to define the priority of service decoration, so, the service with the highest priority will be applied first (the default priority is zero).
This will result in this code: