-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: correct visibility scope issues in Spring AI components #3284
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
base: main
Are you sure you want to change the base?
Conversation
- Change return types from implementation to interface in AutoConfiguration beans - OllamaConnectionDetails in OllamaApiAutoConfiguration - WeaviateConnectionDetails in WeaviateVectorStoreAutoConfiguration - Fix visibility modifiers for: - Make Aws class public in OpenSearchVectorStoreProperties - Set OpenAIAutoConfigurationUtil.resolveConnectionProperties to package-private - Change static NumericBoundary constants to private in RedisFilterExpressionConverter - Explicitly mark metadataFields as final in RedisFilterExpressionConverter These changes ensure components are properly encapsulated and follow API design best practices by exposing only what's necessary through interfaces rather than implementations. Signed-off-by: Soby Chacko <[email protected]>
4cbe245 to
fc0c834
Compare
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean(OllamaConnectionDetails.class) | ||
| public PropertiesOllamaConnectionDetails ollamaConnectionDetails(OllamaConnectionProperties properties) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PropertiesOllamaConnectionDetails is a package-private inner class that was marked as the return type from a public API method. Although the compiler works with that, the IDE throws a visibility issue warning.
| // Avoids instantiation | ||
| } | ||
|
|
||
| public static @NotNull ResolvedConnectionProperties resolveConnectionProperties( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Types of the two arguments in this public method are of classes where they are package-private. This breaks the visibility scope. Since this utility method is only used from the package, we may want to consider marking this as package-protected, matching the same scope of the provided method arg types.
| this.aws = aws; | ||
| } | ||
|
|
||
| static class Aws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We provide public accessor/setter for this although this is a package-private inner class. Since we can't change those public API's, we should make this inner AWS class as public.
| */ | ||
| public class RedisFilterExpressionConverter extends AbstractFilterExpressionConverter { | ||
|
|
||
| public static final NumericBoundary POSITIVE_INFINITY = new NumericBoundary(Double.POSITIVE_INFINITY, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These public constants cause visibility issues since the underlying types are package-private.
Change return types from implementation to interface in AutoConfiguration beans
Fix visibility modifiers for:
These changes ensure components are properly encapsulated and follow API design best practices by exposing only what's necessary through interfaces rather than implementations.