8000 [DependencyInjection] Tagged services with negative priority · Issue #50900 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[DependencyInjection] Tagged services with negative priority #50900

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
Phenix789 opened this issue Jul 6, 2023 · 11 comments
Closed

[DependencyInjection] Tagged services with negative priority #50900

Phenix789 opened this issue Jul 6, 2023 · 11 comments
Labels
Bug DependencyInjection Stalled Status: Needs Review

Comments

@Phenix789
Copy link

Symfony version(s) affected

6.3

Description

By upgrading from 6.2 to 6.3, services implemente an registerForAutoconfiguration() interface with negative priority are not sorted correctly.
This seems happen only when $needsIndexes is set to true (see below).

How to reproduce

  1. Define a tagged service with a negative priority
$services
  ->set(MyService::class)->tag('controller.argument_value_resolver', ['priority' => -70]);
  1. ValueResolverInterface is registerForAutoconfiguration
  2. Now ControllerArgumentValueResolverPass call findAndSortTaggedServices() with a TaggedIteratorArgument and $needsIndexes to true instead of a simple string.
    The behavior of this method is changed and instead of register service only once by continue 2, the service is registered twice one with -70 priority and one with 0 priority.
  3. uasort() sort the service with priority 0 before the -70 one (normal)
  4. The $refs construction add the priority 0 first and the -70 only rewrite it.

Possible Solution

Locally, I need to register my service with autoconfigure(false).

$services
  ->set(MyService::class)->autoconfigure(false)->tag('controller.argument_value_resolver', ['priority' => -70]);

Additional Context

No response
@Phenix789 Phenix789 added the Bug label Jul 6, 2023
@xabbuh
Copy link
Member
xabbuh commented Jul 6, 2023

Can you create a small example application that allows to reproduce your issue?

@Phenix789
Copy link
Author

I will try to do it tomorrow with simple test.
Thanks

@Phenix789
Copy link
Author

@xabbuh Hi, you can find a project with the bug here : Phenix789/symfony-project#1
I did a pull request to comment some piece of code if needed.
Thanks

@Phenix789
Copy link
Author
Phenix789 commented Jul 8, 2023 •
edited
Loading

I also see something.
If you try to inject services via #[TaggedLocator], services are sorted correctly.

public function __invoke(
  #[TaggedIterator('controller.argument_value_resolver')
  iterable $resolvers,
) {
  dump(iterator_to_array($resolvers));
  exit();
}

The bug seems really be in how ControllerArgumentValueResolverPass sort tagged services.

@MatTheCat
Copy link
Contributor

May be a duplicate of #48019. In that case it would be a documentation issue because duplicated tags caused by autoconfiguration were deemed a feature. As you figured out, disabling autoconfiguration would then be the solution.

@Phenix789
Copy link
Author

The trouble with this, it's it works in 6.2 and not anymore in 6.3.
In addition, If you ask tagged services via #[TaggedIterator] is work fine.
Like say in issue description, bug only appear when $needsIndexes is set to true and priority is negative.
I think it's a break change, at least for this tag (controller.argument_value_resolver) and without documentation/changelogs.

And disable autoconfiguration for a service can be a bit "painful" when it implement several autoconfigured interfaces (like LoggerAwareInterface and others aware interfaces)

@MatTheCat
Copy link
Contributor

The change was the introduction of the ValueResolver attribute, which required to enable needsIndexes. Nobody thought it would be a BC break..!

@nicolas-grekas do you know why #35957 only ignores extra autoconfigured tags when needsIndexes is false?

@Phenix789
Copy link
Author

I allow myself to up this issue.
Have you any news ?
Thanks in advance.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Hello? This issue is about to be closed if nobody replies.

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

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 Stalled Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Phenix789 @MatTheCat @xabbuh @carsonbot
0