Fix premature exit from loop processing UI options; some general clea…

…n-up (helidon-io#6110)

integrations/openapi-ui/src/main/java/io/helidon/integrations/openapi/ui/OpenApiUiFull.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,7 +26,6 @@
 
 import io.helidon.common.http.Http;
 import io.helidon.common.http.MediaType;
-import io.helidon.config.Config;
 import io.helidon.openapi.OpenApiUi;
 import io.helidon.openapi.OpenApiUiBase;
 import io.helidon.webserver.Routing;
@@ -73,7 +72,7 @@ static OpenApiUiFull.Builder builder() {
 
     private OpenApiUiFull(Builder builder) {
         super(builder, builder.documentPreparer(), builder.openApiSupportWebContext());
-        Map<Option, String> options = new HashMap<>(builder.options);
+        Map<Option, String> options = builder.uiOptions();
 
         // Apply some Helidon-specific defaults.
         Map.of(Option.title, "Helidon OpenAPI UI",
@@ -138,15 +137,23 @@ public void update(Routing.Rules rules) {
      */
     public static class Builder extends OpenApiUiBase.Builder<Builder, OpenApiUiFull> {
 
-        private Map<Option, String> options = new HashMap<>();
-
         private Builder() {
             super();
         }
 
         @Override
         public OpenApiUiFull build() {
-            if (options.containsKey(Option.url)) {
+            return new OpenApiUiFull(this);
+        }
+
+        /**
+         * Converts the recorded options based on {@code String}s to ones based on {@link Option}s.
+         *
+         * @return {@code Option}-based map of UI options
+         */
+        Map<Option, String> uiOptions() {
+            // Package-private for visibility from tests.
+            if (options().containsKey(Option.url.name())) {
                 LOGGER.log(Level.WARNING,
                            """
                                    Unexpected setting for the OpenAPI URL; \
@@ -155,56 +162,25 @@ the actual endpoint of the Helidon OpenAPI service ({0})
                                    """,
                            new Object[] {
                                    openApiSupportWebContext() + OpenApiUi.UI_WEB_SUBCONTEXT,
-                                   options.get(Option.url)}
-                           );
+                                   options().get(Option.url.name())}
+                );
             }
-            return new OpenApiUiFull(this);
-        }
 
-        /**
-         * Sets the options map the UI should use. Other settings previously assigned will be respected unless the provided map
-         * sets the corresponding value.
-         *
-         * @param options UI options map
-         * @return updated builder
-         */
-        @Override
-        public Builder options(Map<String, String> options) {
-            this.options = convertOptions(options);
-            return this;
-        }
-
-        /**
-         * Assigns the settings using the provided OpenAPI UI {@code Config} node.
-         *
-         * @param uiConfig OpenAPI UI config node
-         * @return updated builder
-         */
-        @Override
-        public Builder config(Config uiConfig) {
-            super.config(uiConfig);
-            applyConfigToOptions(uiConfig.get(OPTIONS_CONFIG_KEY));
-            return this;
-        }
-
-        // For testing.
-        Map<Option, String> uiOptions() {
-            return options;
-        }
-
-        private Map<Option, String> convertOptions(Map<String, String> options) {
             Map<Option, String> result = new HashMap<>();
             List<String> unrecognizedKeys = new ArrayList<>();
 
-            nextKey:
-            for (Map.Entry<String, String> entry : options.entrySet()) {
+            for (Map.Entry<String, String> entry : options().entrySet()) {
+                boolean matched = false;
                 for (Option opt : Option.values()) {
                     if (opt.name().equals(entry.getKey())) {
                         result.put(opt, entry.getValue());
-                        break nextKey;
+                        matched = true;
+                        break;
                     }
                 }
-                unrecognizedKeys.add(entry.getKey());
+                if (!matched) {
+                    unrecognizedKeys.add(entry.getKey());
+                }
             }
             if (!unrecognizedKeys.isEmpty()) {
                 LOGGER.log(Level.WARNING,
@@ -213,16 +189,6 @@ private Map<Option, String> convertOptions(Map<String, String> options) {
             }
             return result;
         }
-
-        private void applyConfigToOptions(Config optionsConfig) {
-            if (!optionsConfig.exists() || optionsConfig.isLeaf()) {
-                return;
-            }
-            optionsConfig.detach()
-                    .asMap()
-                    .map(this::convertOptions)
-                    .ifPresent(options::putAll);
-        }
     }
 
     private void displayIndex(ServerRequest request, ServerResponse response) {

integrations/openapi-ui/src/test/java/io/helidon/integrations/openapi/ui/TestConfig.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,20 +34,23 @@ class TestConfig {
     void checkOptionsConfig() {
         String newTitle = "New Title";
         String newContext = "/overThere";
+        String newFooter = "New Footer";
         Map<String, String> settings = Map.of("openapi.ui.options.title", newTitle,
+                                              "openapi.ui.options.footer", newFooter,
                                               "openapi.ui.options.notThere", "anything",
                                               "openapi.web-context", newContext);
 
-        Config config = Config.create(ConfigSources.create(settings));
+        Config config = Config.just(ConfigSources.create(settings));
         Config openApiConfig = config.get(OpenAPISupport.Builder.CONFIG_KEY);
         OpenApiUiFull.Builder uiSupportBuilder = OpenApiUiFull.builder()
                 .config(openApiConfig.get(OpenApiUi.Builder.OPENAPI_UI_CONFIG_KEY));
-        OpenAPISupport openAPISupportBuilder = OpenAPISupport.builder()
+        OpenAPISupport.builder() // Side effect: trigger conversion of string options to SmallRye Options options.
                 .config(openApiConfig)
                 .ui(uiSupportBuilder)
                 .build();
 
         // Check a simple option setting.
         assertThat("Overridden title value", uiSupportBuilder.uiOptions().get(Option.title), is(newTitle));
+        assertThat("Overridden footer value", uiSupportBuilder.uiOptions().get(Option.footer), is(newFooter));
     }
 }

openapi/src/main/java/io/helidon/openapi/OpenApiUi.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -100,9 +100,9 @@ interface Builder<B extends Builder<B, T>, T extends OpenApiUi> extends io.helid
         String WEB_CONTEXT_CONFIG_KEY = "web-context";
 
         /**
-         * Sets implementation-specific UI options.
+         * Merges implementation-specific UI options.
          *
-         * @param options the options to set for the UI
+         * @param options the options to for the UI to merge
          * @return updated builder
          */
         @ConfiguredOption(kind = ConfiguredOption.Kind.MAP)
@@ -136,7 +136,7 @@ interface Builder<B extends Builder<B, T>, T extends OpenApiUi> extends io.helid
         default B config(Config uiConfig) {
             uiConfig.get(ENABLED_CONFIG_KEY).asBoolean().ifPresent(this::isEnabled);
             uiConfig.get(WEB_CONTEXT_CONFIG_KEY).asString().ifPresent(this::webContext);
-            uiConfig.get(OPTIONS_CONFIG_KEY).asMap().ifPresent(this::options);
+            uiConfig.get(OPTIONS_CONFIG_KEY).detach().asMap().ifPresent(this::options);
             return identity();
         }
 

openapi/src/main/java/io/helidon/openapi/OpenApiUiBase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -192,7 +192,6 @@ public abstract static class Builder<B extends Builder<B, T>, T extends OpenApiU
 
         @Override
         public B options(Map<String, String> options) {
-            this.options.clear();
             this.options.putAll(options);
             return identity();
         }
@@ -236,5 +235,9 @@ public String openApiSupportWebContext() {
         public Function<MediaType, String> documentPreparer() {
             return documentPreparer;
         }
+
+        protected Map<String, String> options() {
+            return options;
+        }
     }
 }