Prevent overriding of configuration classes with the same simple name in NamedContextFactory child contexts - #1738
Open
akenra wants to merge 1 commit into
Open
Prevent overriding of configuration classes with the same simple name in NamedContextFactory child contexts#1738akenra wants to merge 1 commit into
akenra wants to merge 1 commit into
Conversation
… in NamedContextFactory child contexts Configuration classes registered in a NamedContextFactory child context were named after their simple class name. As a result, a client-specific configuration class with the same simple name as the default configuration (e.g. a custom LoadBalancerClientConfiguration) was silently replaced by the default configuration registered later and its beans were ignored. Configuration classes are now registered with fully qualified bean names so that client-specific and default configuration classes can coexist in the child context and @ConditionalOnMissingBean in the default configuration can back off as intended. Fixes spring-cloudgh-1359 Signed-off-by: akenra <37288280+akenra@users.noreply.github.com>
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
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.
Context
NamedContextFactoryregisters configuration classes in the per-client child context viaAnnotationConfigApplicationContext.register(), which names the configuration-class bean after its simple class name. When a client-specific configuration class shares its simple name with the default configuration - for example a custom class namedLoadBalancerClientConfigurationin the user's own package, as reported in #1359 - both get registered under the same bean name, and the default configuration registered later silently replaces the custom definition. The custom configuration is ignored and its beans are missing from the child context.Reproduction from the issue: take the
feign-eurekacustom-loadbalancer-configsample and renameLoadBalancerClientConfigtoLoadBalancerClientConfiguration; the custom load balancer configuration stops being applied.Note that Spring Framework 7.1's
AnnotationBeanNameGeneratordisambiguates nested classes, but top-level classes in different packages still collide.What this PR does
NamedContextFactory.registerBeans(...)now registers through anAnnotatedBeanDefinitionReaderwith an explicitClassUtils.getQualifiedName(...)-based bean name for each configuration class, so distinct classes can no longer override each other in a child context. This fixes it for everyNamedContextFactoryconsumer, not only load balancing.default.*configurations and the default configuration type last; class-level conditions and common definition annotations (scope, lazy, primary, depends on, ...) are evaluated exactly as before, since registration still goes throughAnnotatedBeanDefinitionReader; the fail-fastAnnotationConfigRegistryassertion is preserved.@ConditionalOnMissingBeanin the default configuration backs off to the user's beans.Backward compatibility
@Beanmethods keep their names; only the configuration-class bean definitions themselves are renamed. AllNamedContextFactorylookups (getInstance,getInstances,getAnnotatedInstance, providers) are type-based, as are all in-repo consumers.LoadBalancerChildContextInitializercalls the sameregisterBeans(...), and the generated child-context initializers embed the (now fully qualified) configuration bean names consistently on both JVM and AOT paths.Testing
NamedContextFactoryTests.testClientConfigurationWithSameSimpleNameAsDefaultConfigurationin spring-cloud-context andLoadBalancerClientFactoryTests.shouldApplyClientConfigurationWithSameSimpleNameAsDefaultConfigurationin spring-cloud-loadbalancer. Both are red before the fix and green after; the load balancer test additionally asserts that the default configuration still applies.Connection refused: getsockopt:message assertion); both reproduce identically on clean5.0.x.Fixes #1359
Notes for reviewers