Skip to content

Honor @Order for Map injection sorting order #34701

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

Closed
wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Apr 2, 2025

Fix GH-34688

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 2, 2025
@quaff
Copy link
Contributor Author

quaff commented Apr 2, 2025

The failed test is related to this PR, it passes when I run it locally.

java.lang.AssertionError: VerifySubscriber timed out on reactor.core.publisher.MonoFlatMap$FlatMapMain@4c12f54a
	at reactor.test.MessageFormatter.assertionError(MessageFormatter.java:115)
	at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.pollTaskEventOrComplete(DefaultStepVerifierBuilder.java:1728)
	at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.verify(DefaultStepVerifierBuilder.java:1298)
	at reactor.test.DefaultStepVerifierBuilder$DefaultStepVerifier.verify(DefaultStepVerifierBuilder.java:832)
	at org.springframework.web.reactive.function.MultipartRouterFunctionIntegrationTests.parts(MultipartRouterFunctionIntegrationTests.java:107)

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Apr 2, 2025
@sbrannen sbrannen changed the title Map injection should honor the sorting order Honor @Order for Map injection sorting order Apr 2, 2025
@jhoeller
Copy link
Contributor

jhoeller commented Apr 2, 2025

There is arguably a semantic problem: Intuitively, a sorted Map would be sorted by key, not by value - whereas in this case, it would be the bean instance values that are to be sorted. That's the main reason we have not implemented this initially but rather left it at bean registration order.

Also, from a modern-day perspective, we mean to discourage Map injection altogether. While it seemed attractive to align it with getBeansOfType in the early days, this is hard to handle in case of actual beans of type Map, and even getBeansOfType is semi-deprecated in favor of getBeanNamesForType plus individual getBean calls these days. It is usually recommendable to use ObjectProvider instead with a clear distinction between bean type and multi-element iteration.

Overall, we intend to refrain from adding further capabilities to Map injection. Thanks for the PR, in any case!

@jhoeller jhoeller closed this Apr 2, 2025
@jhoeller jhoeller added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map injection doesn't honor @Order
4 participants