Skip to content

[DI] Fix not working if only "default_index_method" used #39203

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.

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

Merged
merged 1 commit into from
Dec 10, 2020
Merged

[DI] Fix not working if only "default_index_method" used #39203

merged 1 commit into from
Dec 10, 2020

Conversation

malteschlueter
Copy link
Contributor

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #35349
License MIT

The default index method wasn't used if the "index_by" attribute is missing. The documentation is showing an example, see https://symfony.com/doc/current/service_container/tags.html#tagged-services-with-index.

This problem also appears in symfony 5.

I created two example projects, the first in the current behaviour and the second with my bugfix branch.

Current 4.4: https://github.com/malteschlueter/symfony-reproducers/blob/bugfix/dependency-injection-default-index-method-not-working--not-fixed/tests/HandlerCollectionTest.php

This bugfix branch: https://github.com/malteschlueter/symfony-reproducers/blob/bugfix/dependency-injection-default-index-method-not-working--with-fix/tests/HandlerCollectionTest.php

@nicolas-grekas nicolas-grekas changed the title [DependencyInjection] Fix not working if only "default_index_method" used [DI] Fix not working if only "default_index_method" used Dec 8, 2020
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the implementation to improve the error messages and fix the behavior specifically when an index method is defined.
Tests are now green.

@carsonbot carsonbot changed the title [DI] Fix not working if only "default_index_method" used [DependencyInjection] [DI] Fix not working if only "default_index_method" used Dec 8, 2020
@nicolas-grekas nicolas-grekas changed the title [DependencyInjection] [DI] Fix not working if only "default_index_method" used [DI] Fix not working if only "default_index_method" used Dec 8, 2020
@malteschlueter
Copy link
Contributor Author

Thank you very much for your support @nicolas-grekas!

@fabpot
Copy link
Member

fabpot commented Dec 10, 2020

Thank you @malteschlueter.

@fabpot fabpot merged commit 2dd4561 into symfony:4.4 Dec 10, 2020
This was referenced Dec 18, 2020
@malteschlueter malteschlueter deleted the bugfix/dependency-injection-default-index-method-not-working branch November 24, 2021 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants