Avoid reference bean name collision with unrelated by-name lookups - #16405
Open
xiincs wants to merge 1 commit into
Open
Avoid reference bean name collision with unrelated by-name lookups#16405xiincs wants to merge 1 commit into
xiincs wants to merge 1 commit into
Conversation
@DubboReference registers its reference bean under the bare field name when no explicit id is given. That name is claimed at registration time only, so it can silently collide later with an unrelated bean looked up by the same default name elsewhere in the context (e.g. a @resource field), which then fails with a confusing BeanNotOfRequiredTypeException instead of falling back to by-type resolution, since Spring only takes that fallback path when no bean of that name exists yet. Add an opt-in dubbo.application.qualify-reference-bean-name property that suffixes auto-derived reference bean names so they can no longer claim a name an unrelated lookup might use. Defaults to false to keep existing naming behavior unchanged for applications that already rely on it. Fixes apache#12637
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16405 +/- ##
============================================
- Coverage 60.87% 60.86% -0.02%
+ Complexity 11766 11761 -5
============================================
Files 1953 1953
Lines 89273 89278 +5
Branches 13473 13474 +1
============================================
- Hits 54346 54340 -6
- Misses 29333 29343 +10
- Partials 5594 5595 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change?
Fixes #12637.
As I understand the issue, when
@DubboReferencehas no explicitid, the reference bean is registered under the bare field name. That name is only checked against bean definitions that already exist at registration time, so it can later collide with an unrelated bean looked up by the same default name elsewhere in the context (for example an independent@Resourcefield). When that happens, Spring throwsBeanNotOfRequiredTypeExceptioninstead of falling back to by-type resolution, becauseCommonAnnotationBeanPostProcessor#autowireResourceonly takes that fallback when no bean of that name exists yet.While looking into this, I noticed a previous attempt (#15889) touched
ReferenceBean#getObjectType(), but its own regression test still passed on unpatched master, so I don't think it ended up exercising the reported scenario - hopefully this PR's tests close that gap.My approach here is an opt-in
dubbo.application.qualify-reference-bean-nameproperty that suffixes auto-derived reference bean names so they no longer claim a name an unrelated lookup might use. It defaults tofalseso existing naming behavior is unchanged for anyone already relying on it - I wasn't confident a default-naming change would be safe to make unconditionally given how many downstream users this project has, but I'm happy to adjust the approach if maintainers see a better way to handle it.Added
ReferenceBeanNameCollisionTest(documents the collision under default behavior) andReferenceBeanNameCollisionFixedTest(confirms the property resolves it), and ran the existingReferenceAnnotationBeanPostProcessorTestlocally to check for regressions.This is my first contribution to Dubbo, so please let me know if I've missed any conventions.
Checklist