[ISSUE #16381] Retain registry configs with different explicit ids - #16407
Open
cyforkk wants to merge 1 commit into
Open
[ISSUE #16381] Retain registry configs with different explicit ids#16407cyforkk wants to merge 1 commit into
cyforkk wants to merge 1 commit into
Conversation
When two RegistryConfig instances have different explicit ids but the same address, the previous dedup logic (which uses equals() that ignores id) incorrectly treated the second one as a duplicate of the first and dropped it, causing "Registry not found" when later looking up by the second id. Fix by treating configs with different explicit ids as distinct in both the fast-path check in addConfig() and the findConfigByValue() lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16407 +/- ##
=========================================
Coverage 60.87% 60.88%
- Complexity 11766 11769 +3
=========================================
Files 1953 1953
Lines 89273 89278 +5
Branches 13473 13474 +1
=========================================
+ Hits 54346 54356 +10
+ Misses 29333 29329 -4
+ Partials 5594 5593 -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
3 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.
Fixes #16381
What is the purpose of the change
When two
RegistryConfiginstances have different explicit ids (e.g.zk1,zk2) but the same address, the previous dedup logic incorrectly treated the second one as a duplicate of the first and dropped it. This causedIllegalStateException: Registry not found: zk2when later resolving the reference's registry id.The root cause is that
AbstractConfig.equals()intentionally ignores theidfield, so the dedup check inaddConfig()andfindConfigByValue()could not distinguish configs that the user explicitly identified as different.Brief changelog
AbstractConfigManager.addConfig()fast-path check: skip the early return when the existing config has a different explicit id than the new config.AbstractConfigManager.findConfigByValue(): exclude matches where both configs have non-null, different explicit ids.hasDifferentExplicitId()to encapsulate the comparison.ConfigManagerTest:testRegistryConfigWithDifferentIdsSameAddress: two registries with different ids and same address are both retained.testRegistryConfigWithSameIdSameAddress: same-id registries are still deduplicated (override).Verifying this change
ConfigManagerTest: 21 tests passed (including 2 new regression tests).