Skip to content

Refactored usage of Optional<> as method parameters#2720

Open
CptBartender wants to merge 1 commit into
apache:mainfrom
CptBartender:refactor-optional-parameters
Open

Refactored usage of Optional<> as method parameters#2720
CptBartender wants to merge 1 commit into
apache:mainfrom
CptBartender:refactor-optional-parameters

Conversation

@CptBartender

Copy link
Copy Markdown
Contributor

There's only one place that uses Optional as method parameters, and it is generally recommended to be avioded. I've reworked the use case so that the Optionals are no longer used as method parameters or generic parameters for other functional interfaces

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the config metatype lookup helper and its consumers to avoid using Optional as a method parameter (and as a generic parameter for functional interface callbacks), simplifying callback usage across the config command code.

Changes:

  • Updated MetaServiceCaller.doWithMetaType to accept Function<MetaInfo, T> instead of Function<Optional<MetaInfo>, T>.
  • Updated MetaCommand callbacks to work with MetaInfo directly (with null handling).
  • Refactored ConfigurationPropertyCompleter metatype property collection to avoid Optional<MetaInfo> parameters and use a stream-based collection approach.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
config/core/src/main/java/org/apache/karaf/config/core/MetaServiceCaller.java Changes the metatype callback signature away from Optional and adjusts invocation behavior.
config/command/src/main/java/org/apache/karaf/config/command/MetaCommand.java Updates command implementation callbacks to match the new doWithMetaType function signature.
config/command/src/main/java/org/apache/karaf/config/command/completers/ConfigurationPropertyCompleter.java Updates metatype-property extraction logic to avoid Optional parameters and collect properties via streams.
Comments suppressed due to low confidence (1)

config/core/src/main/java/org/apache/karaf/config/core/MetaServiceCaller.java:89

  • doWithMetaType no longer invokes the callback when no metatype is found (or when MetaTypeService is unavailable) and returns null instead. Call sites currently rely on the callback being invoked with a "missing" value (e.g., ConfigurationPropertyCompleter expects a non-null List to addAll, and MetaCommand expects to print a "No meta type definition" message). This change can lead to a NullPointerException and/or silently skipping output.
    public static <T> T doWithMetaType(BundleContext context, String pid, Function<MetaInfo, T> function) {
        ServiceReference<MetaTypeService> ref = context.getServiceReference(MetaTypeService.class);
        if (ref != null) {
            try {
                MetaTypeService metaService = context.getService(ref);
                MetaInfo metaInfo = getMetatype(context, metaService, pid);
                if (metaInfo != null) {
                    return function.apply(metaInfo);
                }
            } finally {
                context.ungetService(ref);
            }
        }
        return null;
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants