Refactored usage of Optional<> as method parameters#2720
Open
CptBartender wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
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.doWithMetaTypeto acceptFunction<MetaInfo, T>instead ofFunction<Optional<MetaInfo>, T>. - Updated
MetaCommandcallbacks to work withMetaInfodirectly (with null handling). - Refactored
ConfigurationPropertyCompletermetatype property collection to avoidOptional<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
doWithMetaTypeno longer invokes the callback when no metatype is found (or whenMetaTypeServiceis unavailable) and returnsnullinstead. Call sites currently rely on the callback being invoked with a "missing" value (e.g.,ConfigurationPropertyCompleterexpects a non-nullListtoaddAll, andMetaCommandexpects to print a "No meta type definition" message). This change can lead to aNullPointerExceptionand/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.
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.
There's only one place that uses
Optionalas method parameters, and it is generally recommended to be avioded. I've reworked the use case so that theOptionals are no longer used as method parameters or generic parameters for other functional interfaces