Use MP OpenAPI 1.2 and adapt to SmallRye OpenAPI 2.0.26 which supports it (helidon-io#3294)

* Use MP OpenAPI 1.2 and adapt to SmallRye OpenAPI 2.0.26 which supports it

Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>

* Allow but warn of unquoted HTTP status values in incoming OpenAPI documents (they should be quoted)

* Add logging (and refactor a bit for it), correct a logging typo, and set the 'openapi' element when merging documents (because SmallRye skips that during merge)

* Prevent false prefix matches in regex used for selecting classes for annotation processing, add some logging, and use method reference instead of a lambda

* Rewrite a test to use Helidon test support and Hamcrest, enhance error reporting in TestUtil, and update pom.xml for testing changes

* Updated pom.xml

Author: tjquinno

dependencies/pom.xml

@@ -105,7 +105,7 @@
         <version.lib.microprofile-health>2.2</version.lib.microprofile-health>
         <version.lib.microprofile-jwt>1.1.1</version.lib.microprofile-jwt>
         <version.lib.microprofile-metrics-api>2.3.2</version.lib.microprofile-metrics-api>
-        <version.lib.microprofile-openapi-api>1.1.2</version.lib.microprofile-openapi-api>
+        <version.lib.microprofile-openapi-api>1.2</version.lib.microprofile-openapi-api>
         <version.lib.microprofile-reactive-messaging-api>1.0</version.lib.microprofile-reactive-messaging-api>
         <version.lib.microprofile-reactive-streams-operators-api>1.0.1</version.lib.microprofile-reactive-streams-operators-api>
         <version.lib.microprofile-reactive-streams-operators-core>1.0.1</version.lib.microprofile-reactive-streams-operators-core>
@@ -130,7 +130,7 @@
         <version.lib.postgresql>42.2.18</version.lib.postgresql>
         <version.lib.prometheus>0.9.0</version.lib.prometheus>
         <version.lib.slf4j>1.7.30</version.lib.slf4j>
-        <version.lib.smallrye-openapi>1.2.3</version.lib.smallrye-openapi>
+        <version.lib.smallrye-openapi>2.0.26</version.lib.smallrye-openapi>
         <version.lib.snakeyaml>1.27</version.lib.snakeyaml>
         <version.lib.transaction-api>1.3.3</version.lib.transaction-api>
         <version.lib.typesafe-config>1.4.1</version.lib.typesafe-config>
@@ -740,6 +740,7 @@
                 <groupId>io.smallrye</groupId>
                 <artifactId>smallrye-open-api</artifactId>
                 <version>${version.lib.smallrye-openapi}</version>
+                <type>pom</type>
             </dependency>
 
             <!-- Integrations related -->

microprofile/openapi/pom.xml

@@ -122,6 +122,16 @@
             <artifactId>snakeyaml</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.microprofile.tests</groupId>
+            <artifactId>helidon-microprofile-tests-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>

microprofile/openapi/src/main/java/io/helidon/microprofile/openapi/MPOpenAPIBuilder.java

@@ -27,6 +27,7 @@
 import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import javax.enterprise.inject.spi.CDI;
@@ -173,19 +174,41 @@ private FilteredIndexView appRelatedClassesToFilteredIndexView(Set<Class<?>> app
          * Create an OpenAPIConfig instance to limit scanning to this app's classes by overriding any inclusions of classes or
          * packages specified in the config with our own inclusions based on this app's classes.
          */
-        OpenApiConfigImpl openAPIFilteringConfig = new OpenApiConfigImpl(mpConfig);
-        Set<String> scanClasses = openAPIFilteringConfig.scanClasses();
-        scanClasses.clear();
-        openAPIFilteringConfig.scanPackages().clear();
-
-        appRelatedClassesToScan.stream()
-                .map(Class::getName)
-                .forEach(scanClasses::add);
+        Pattern appRelatedClassesPattern = Pattern.compile(
+                appRelatedClassesToScan.stream()
+                        .map(Class::getName)
+                        .map(Pattern::quote)
+                        .map(name -> "^" + name + "$") // avoid false prefix matches
+                        .collect(Collectors.joining("|", "(", ")")));
+
+        FilteredIndexView result = new FilteredIndexView(singleIndexViewSupplier.get(),
+                new FilteringOpenApiConfigImpl(mpConfig, appRelatedClassesPattern));
+        if (LOGGER.isLoggable(Level.FINE)) {
+            LOGGER.log(Level.FINE, String.format("FilteredIndexView for %n"
+                    + "  application classes %s%n"
+                    + "  will use pattern: %s%n"
+                    + "  with known classes %s",
+                    appRelatedClassesToScan, appRelatedClassesPattern, result.getKnownClasses()));
+        }
 
-        FilteredIndexView result = new FilteredIndexView(singleIndexViewSupplier.get(), openAPIFilteringConfig);
         return result;
     }
 
+    private static class FilteringOpenApiConfigImpl extends OpenApiConfigImpl {
+
+        private final Pattern appRelatedClassNamesToScan;
+
+        FilteringOpenApiConfigImpl(Config config, Pattern appRelatedClassNamesToScan) {
+            super(config);
+            this.appRelatedClassNamesToScan = appRelatedClassNamesToScan;
+        }
+
+        @Override
+        public Pattern scanClasses() {
+            return appRelatedClassNamesToScan;
+        }
+    }
+
     /**
      * Sets the OpenApiConfig instance to use in governing the behavior of the
      * smallrye OpenApi implementation.
@@ -211,7 +234,7 @@ MPOpenAPIBuilder singleIndexViewSupplier(Supplier<? extends IndexView> singleInd
 
     @Override
     protected Supplier<List<? extends IndexView>> indexViewsSupplier() {
-        return () -> buildPerAppFilteredIndexViews();
+        return this::buildPerAppFilteredIndexViews;
     }
 
     @Override

microprofile/openapi/src/main/java/module-info.java

@@ -22,7 +22,7 @@
 module io.helidon.microprofile.openapi {
     requires java.logging;
 
-    requires smallrye.open.api;
+    requires smallrye.open.api.core;
 
     requires microprofile.config.api;
     requires io.helidon.microprofile.server;

microprofile/openapi/src/test/java/io/helidon/microprofile/openapi/BasicServerTest.java

@@ -16,61 +16,63 @@
  */
 package io.helidon.microprofile.openapi;
 
-import java.net.HttpURLConnection;
 import java.util.Map;
 
-import io.helidon.common.http.MediaType;
-import io.helidon.config.Config;
-import io.helidon.microprofile.openapi.other.TestApp2;
-import io.helidon.microprofile.server.Server;
+import javax.inject.Inject;
+import javax.ws.rs.client.WebTarget;
+import javax.ws.rs.core.Response;
+
+import io.helidon.common.http.Http;
+import io.helidon.microprofile.tests.junit5.AddBean;
+import io.helidon.microprofile.tests.junit5.HelidonTest;
+import io.helidon.openapi.OpenAPISupport;
 
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
+import org.yaml.snakeyaml.Yaml;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
  * Test that MP OpenAPI support works when retrieving the OpenAPI document
  * from the server's /openapi endpoint.
  */
+@HelidonTest
+@AddBean(TestApp.class)
+@AddBean(TestApp3.class)
 public class BasicServerTest {
 
-    private static final String OPENAPI_PATH = "/openapi";
-
-    private static Server server;
-
-    private static HttpURLConnection cnx;
-
     private static Map<String, Object> yaml;
 
-    public BasicServerTest() {
+    @Inject
+    WebTarget webTarget;
+
+    private static Map<String, Object> retrieveYaml(WebTarget webTarget) {
+        try (Response response = webTarget
+                .path(OpenAPISupport.DEFAULT_WEB_CONTEXT)
+                .request(OpenAPISupport.DEFAULT_RESPONSE_MEDIA_TYPE.toString())
+                .get()) {
+            assertThat("Fetch of OpenAPI document from server status", response.getStatus(),
+                    is(equalTo(Http.Status.OK_200.code())));
+            String yamlText = response.readEntity(String.class);
+            return new Yaml().load(yamlText);
+        }
     }
 
-    /**
-     * Start the server to run the test app and read the response from the
-     * /openapi endpoint into a map that all tests can use.
-     *
-     * @throws Exception in case of error reading the response as yaml
-     */
-    @BeforeAll
-    public static void startServer() throws Exception {
-        server = TestUtil.startServer(Config.create(), TestApp.class, TestApp3.class);
-        cnx = TestUtil.getURLConnection(
-                server.port(),
-                "GET",
-                OPENAPI_PATH,
-                MediaType.APPLICATION_OPENAPI_YAML);
+    private static Map<String, Object> yaml(WebTarget webTarget) {
+        if (yaml == null) {
+            yaml = retrieveYaml(webTarget);
+        }
+        return yaml;
+    }
 
-        yaml = TestUtil.yamlFromResponse(cnx);
+    private Map<String, Object> yaml() {
+        return yaml(webTarget);
     }
 
-    /**
-     * Stop the server.
-     */
-    @AfterAll
-    public static void stopServer() {
-        TestUtil.cleanup(server, cnx);
+    public BasicServerTest() {
     }
 
     /**
@@ -79,16 +81,18 @@ public static void stopServer() {
      *
      * @throws Exception in case of errors reading the HTTP response
      */
-    @SuppressWarnings("unchecked")
     @Test
     public void simpleTest() throws Exception {
-        String goSummary = TestUtil.fromYaml(yaml, "paths./testapp/go.get.summary", String.class);
-        assertEquals(TestApp.GO_SUMMARY, goSummary);
+        checkPathValue("paths./testapp/go.get.summary", TestApp.GO_SUMMARY);
     }
 
     @Test
     public void testMultipleApps() {
-        String goSummary3 = TestUtil.fromYaml(yaml, "paths./testapp3/go3.get.summary", String.class);
-        assertEquals(TestApp3.GO_SUMMARY, goSummary3);
+        checkPathValue("paths./testapp3/go3.get.summary", TestApp3.GO_SUMMARY);
+    }
+
+    private void checkPathValue(String pathExpression, String expected) {
+        String result = TestUtil.fromYaml(yaml(), pathExpression, String.class);
+        assertThat(pathExpression, result, is(equalTo(expected)));
     }
 }

microprofile/openapi/src/test/java/io/helidon/microprofile/openapi/TestUtil.java

@@ -34,6 +34,7 @@
 import org.yaml.snakeyaml.Yaml;
 
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * Useful utility methods during testing.
@@ -198,9 +199,14 @@ public static Map<String, Object> yamlFromResponse(HttpURLConnection cnx) throws
      */
     @SuppressWarnings(value = "unchecked")
     public static <T> T fromYaml(Map<String, Object> map, String dottedPath, Class<T> cl) {
+        Map<String, Object> originalMap = map;
         String[] segments = dottedPath.split("\\.");
         for (int i = 0; i < segments.length - 1; i++) {
             map = (Map<String, Object>) map.get(segments[i]);
+            if (map == null) {
+                fail("Traversing dotted path " + dottedPath + " segment " + segments[i] + " not found in parsed map "
+                        + originalMap);
+            }
         }
         return cl.cast(map.get(segments[segments.length - 1]));
     }

openapi/pom.xml

@@ -73,7 +73,7 @@
                             <failOnMissingClassifierArtifact>true</failOnMissingClassifierArtifact>
                             <outputDirectory>${openapi-impls-dir}</outputDirectory>
                             <includeGroupIds>io.smallrye</includeGroupIds>
-                            <includeArtifactIds>smallrye-open-api</includeArtifactIds>
+                            <includeArtifactIds>smallrye-open-api-core</includeArtifactIds>
                             <includes>io/smallrye/openapi/api/models/**/*.java</includes>
                         </configuration>
                     </execution>
@@ -123,6 +123,7 @@
         <dependency>
             <groupId>io.smallrye</groupId>
             <artifactId>smallrye-open-api</artifactId>
+            <type>pom</type>
             <exclusions>
                 <exclusion>
                     <groupId>com.fasterxml.jackson.core</groupId>

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

@@ -16,8 +16,12 @@
  */
 package io.helidon.openapi;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import org.eclipse.microprofile.openapi.models.PathItem;
 import org.eclipse.microprofile.openapi.models.Paths;
@@ -32,10 +36,12 @@
 import org.eclipse.microprofile.openapi.models.servers.ServerVariables;
 import org.yaml.snakeyaml.TypeDescription;
 import org.yaml.snakeyaml.constructor.Constructor;
+import org.yaml.snakeyaml.error.Mark;
 import org.yaml.snakeyaml.nodes.MappingNode;
 import org.yaml.snakeyaml.nodes.Node;
 import org.yaml.snakeyaml.nodes.NodeId;
 import org.yaml.snakeyaml.nodes.SequenceNode;
+import org.yaml.snakeyaml.nodes.Tag;
 
 /**
  * Specialized SnakeYAML constructor for modifying {@code Node} objects for OpenAPI types that extend {@code Map} to adjust the
@@ -64,6 +70,8 @@ final class CustomConstructor extends Constructor {
     // maps OpenAPI interfaces which extend Map<?, List<type>> to the type that appears in the list
     private static final Map<Class<?>, Class<?>> CHILD_MAP_OF_LIST_TYPES = new HashMap<>();
 
+    private static final Logger LOGGER = Logger.getLogger(CustomConstructor.class.getName());
+
     static {
         CHILD_MAP_TYPES.put(Paths.class, PathItem.class);
         CHILD_MAP_TYPES.put(Callback.class, PathItem.class);
@@ -99,6 +107,28 @@ protected void constructMapping2ndStep(MappingNode node, Map<Object, Object> map
                 }
             });
         }
+
+        // Older releases silently accepted numbers for APIResponse status values; they should be strings.
+        if (parentType.equals(APIResponses.class)) {
+            convertIntHttpStatuses(node);
+        }
         super.constructMapping2ndStep(node, mapping);
     }
+
+    private void convertIntHttpStatuses(MappingNode node) {
+        List<Mark> numericHttpStatusMarks = new ArrayList<>();
+        node.getValue().forEach(t -> {
+            Node keyNode = t.getKeyNode();
+            if (keyNode.getTag().equals(Tag.INT)) {
+                numericHttpStatusMarks.add(keyNode.getStartMark());
+                keyNode.setTag(Tag.STR);
+            }
+        });
+        if (!numericHttpStatusMarks.isEmpty()) {
+            LOGGER.log(Level.WARNING,
+                    "Numeric HTTP status value(s) should be quoted. "
+                    + "Please change the following; unquoted numeric values might be rejected in a future release: {0}",
+                    numericHttpStatusMarks);
+        }
+    }
 }