Skip to content

Conversation

@sobychacko
Copy link
Contributor

  • 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.

- 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]>
@sobychacko sobychacko force-pushed the qodana-class-visiblity branch from 4cbe245 to fc0c834 Compare May 22, 2025 17:50

@Bean
@ConditionalOnMissingBean(OllamaConnectionDetails.class)
public PropertiesOllamaConnectionDetails ollamaConnectionDetails(OllamaConnectionProperties properties) {
Copy link
Contributor Author

@sobychacko sobychacko May 22, 2025

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(
Copy link
Contributor Author

@sobychacko sobychacko May 22, 2025

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 {
Copy link
Contributor Author

@sobychacko sobychacko May 22, 2025

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);
Copy link
Contributor Author

@sobychacko sobychacko May 22, 2025

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.

@markpollack markpollack self-assigned this May 29, 2025
@markpollack markpollack added this to the 1.1.x milestone May 29, 2025
@markpollack markpollack removed this from the 1.1.0.M1 milestone Sep 25, 2025
@markpollack markpollack removed their assignment Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants