Merge pull request #35217 from MikeEdgar/issue-34765

OpenAPI: normalize disabled endpoint paths for OpenAPI model matching
quarkusio · Aug 7, 2023 · 80f7095 · 80f7095
2 parents 2960dcf + b42cdfb
commit 80f7095
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 29 deletions.
diff --git a/...oyment/src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/DisabledEndpointTestCase.java b/...oyment/src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/DisabledEndpointTestCase.java
@@ -26,13 +26,21 @@ public class DisabledEndpointTestCase {
     static QuarkusUnitTest runner = new QuarkusUnitTest()
             .withApplicationRoot((jar) -> jar
                     .addClasses(DisabledEndpoint.class,
+                            DisabledOtherEndpoint.class,
+                            DisabledRootEndpoint.class,
                             EnabledEndpoint.class)
-                    .add(new StringAsset("quarkus.http.root-path=/root\n"), "application.properties"));
+                    .add(new StringAsset(""), "application.properties"));
 
     @EndpointDisabled(name = "xxx", disableIfMissing = true, stringValue = "xxx")
     @Path("/disabled")
     public static class DisabledEndpoint {
 
+        @Path("/")
+        @GET
+        public String helloRoot() {
+            return null;
+        }
+
         @Path("/hello")
         @GET
         public String hello() {
@@ -53,11 +61,40 @@ public String hello5() {
 
     }
 
+    @EndpointDisabled(name = "xxx", disableIfMissing = true, stringValue = "xxx")
+    @Path("/enabled")
+    public static class DisabledOtherEndpoint {
+
+        @Path("/hello5")
+        @GET
+        public String hello5() {
+            return null;
+        }
+
+    }
+
+    @EndpointDisabled(name = "xxx", disableIfMissing = true, stringValue = "xxx")
+    @Path("/")
+    public static class DisabledRootEndpoint {
+
+        @GET
+        public String helloRoot() {
+            return null;
+        }
+
+    }
+
     @EndpointDisabled(name = "xxx", disableIfMissing = false, stringValue = "xxx")
     @Path("/enabled")
     public static class EnabledEndpoint {
 
-        @Path("/hello3")
+        @Path("/")
+        @GET
+        public String helloRoot() {
+            return null;
+        }
+
+        @Path("/hello3/")
         @GET
         public String hello() {
             return null;
@@ -82,12 +119,17 @@ public void testDisabledEndpoint() {
         RestAssured.given().header("Accept", "application/json")
                 .when().get("/q/openapi")
                 .prettyPeek().then()
-                .body("paths.\"/root/disabled/hello\".get", nullValue())
-                .body("paths.\"/root/disabled/hello2/{param1}\".get", nullValue())
-                .body("paths.\"/root/enabled/hello3\".get", notNullValue())
-                .body("paths.\"/root/enabled/hello4/{param1}\".get", notNullValue())
-                .body("paths.\"/root/enabled/hello5\".post", notNullValue())
-                .body("paths.\"/root/enabled/hello5\".put", nullValue());
+                // All paths from DisabledEndpoint missing
+                .body("paths.\"/disabled\"", nullValue())
+                // Paths from DisabledOtherEndpoint
+                .body("paths.\"/enabled/hello5\".get", nullValue())
+                // Paths from DisabledRootEndpoint
+                .body("paths.\"/\".get", nullValue())
+                // Paths from EnabledEndpoint
+                .body("paths.\"/enabled\".get", notNullValue())
+                .body("paths.\"/enabled/hello3\".get", notNullValue())
+                .body("paths.\"/enabled/hello4/{param1}\".get", notNullValue())
+                .body("paths.\"/enabled/hello5\".post", notNullValue());
     }
 
 }
diff --git a/...src/main/java/io/quarkus/smallrye/openapi/runtime/filter/DisabledRestEndpointsFilter.java b/...src/main/java/io/quarkus/smallrye/openapi/runtime/filter/DisabledRestEndpointsFilter.java
@@ -1,12 +1,15 @@
 package io.quarkus.smallrye.openapi.runtime.filter;
 
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Stream;
 
 import org.eclipse.microprofile.openapi.OASFilter;
 import org.eclipse.microprofile.openapi.models.OpenAPI;
 import org.eclipse.microprofile.openapi.models.PathItem;
+import org.eclipse.microprofile.openapi.models.Paths;
 
 import io.quarkus.runtime.rest.DisabledRestEndpoints;
 
@@ -17,28 +20,46 @@
  */
 public class DisabledRestEndpointsFilter implements OASFilter {
 
+    @Override
     public void filterOpenAPI(OpenAPI openAPI) {
-        Map<String, List<String>> disabledEndpointsMap = DisabledRestEndpoints.get();
-        if (disabledEndpointsMap != null) {
-            Map<String, PathItem> pathItems = openAPI.getPaths().getPathItems();
-            List<String> emptyPathItems = new ArrayList<>();
-            if (pathItems != null) {
-                for (Map.Entry<String, PathItem> entry : pathItems.entrySet()) {
-                    String path = entry.getKey();
-                    PathItem pathItem = entry.getValue();
-                    List<String> disabledMethodsForThisPath = disabledEndpointsMap.get(path);
-                    if (disabledMethodsForThisPath != null) {
-                        disabledMethodsForThisPath.forEach(method -> {
-                            pathItem.setOperation(PathItem.HttpMethod.valueOf(method), null);
-                        });
-                        // if the pathItem is now empty, remove it
-                        if (pathItem.getOperations().isEmpty()) {
-                            emptyPathItems.add(path);
-                        }
+        Paths paths = openAPI.getPaths();
+
+        disabledRestEndpoints()
+                // Skip paths that are not present in the OpenAPI model
+                .filter(pathMethods -> paths.hasPathItem(pathMethods.getKey()))
+                .forEach(pathMethods -> {
+                    String path = pathMethods.getKey();
+                    PathItem pathItem = paths.getPathItem(path);
+
+                    // Remove each operation identified as a disabled HTTP method
+                    Optional.ofNullable(pathMethods.getValue())
+                            .orElseGet(Collections::emptyList)
+                            .stream()
+                            .map(PathItem.HttpMethod::valueOf)
+                            .forEach(method -> pathItem.setOperation(method, null));
+
+                    if (pathItem.getOperations().isEmpty()) {
+                        paths.removePathItem(path);
                     }
-                }
-                emptyPathItems.forEach(openAPI.getPaths()::removePathItem);
-            }
+                });
+    }
+
+    static Stream<Map.Entry<String, List<String>>> disabledRestEndpoints() {
+        return Optional.ofNullable(DisabledRestEndpoints.get())
+                .orElseGet(Collections::emptyMap)
+                .entrySet()
+                .stream()
+                .map(pathMethods -> Map.entry(stripSlash(pathMethods.getKey()), pathMethods.getValue()));
+    }
+
+    /**
+     * Removes any trailing slash character from the path when it is not the root '/'
+     * path. This is necessary to align with the paths generated in the OpenAPI model.
+     */
+    static String stripSlash(String path) {
+        if (path.endsWith("/") && path.length() > 1) {
+            return path.substring(0, path.length() - 1);
         }
+        return path;
     }
 }