Fix remove unused schemas

Fixed #1736
micronaut-projects · Aug 28, 2024 · 2fcb91e · 2fcb91e
1 parent cb2072e
commit 2fcb91e
Show file tree

Hide file tree

Showing 4 changed files with 146 additions and 49 deletions.
diff --git a/openapi/src/main/java/io/micronaut/openapi/view/AbstractViewConfig.java b/openapi/src/main/java/io/micronaut/openapi/view/AbstractViewConfig.java
@@ -37,6 +37,7 @@
 import static io.micronaut.openapi.visitor.OpenApiConfigProperty.MICRONAUT_SERVER_CONTEXT_PATH;
 import static io.micronaut.openapi.visitor.StringUtil.PLACEHOLDER_POSTFIX;
 import static io.micronaut.openapi.visitor.StringUtil.PLACEHOLDER_PREFIX;
+import static io.micronaut.openapi.visitor.StringUtil.QUOTE;
 import static io.micronaut.openapi.visitor.StringUtil.SLASH;
 
 /**
@@ -295,7 +296,7 @@ static Object asString(String v) {
      * @return A quoted String.
      */
     static Object asQuotedString(String v) {
-        return v == null ? null : "\"" + v + '"';
+        return v == null ? null : QUOTE + v + QUOTE;
     }
 
     /**

diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java b/openapi/src/main/java/io/micronaut/openapi/visitor/OpenApiApplicationVisitor.java
@@ -114,6 +114,7 @@
 import static io.micronaut.openapi.visitor.SchemaUtils.setOperationOnPathItem;
 import static io.micronaut.openapi.visitor.StringUtil.PLACEHOLDER_POSTFIX;
 import static io.micronaut.openapi.visitor.StringUtil.PLACEHOLDER_PREFIX;
+import static io.micronaut.openapi.visitor.StringUtil.QUOTE;
 import static io.micronaut.openapi.visitor.Utils.resolveComponents;
 import static io.swagger.v3.oas.models.Components.COMPONENTS_SCHEMAS_REF;
 
@@ -125,6 +126,7 @@
  */
 public class OpenApiApplicationVisitor extends AbstractOpenApiVisitor implements TypeElementVisitor<Object, Object> {
 
+    private static final int MAX_ITERATIONS = 100;
     private ClassElement classElement;
     private int visitedElements = -1;
 
@@ -284,18 +286,12 @@ private static PropertyNamingStrategies.NamingBase fromName(String name) {
         }
         return switch (name.toUpperCase(Locale.US)) {
             case "LOWER_CAMEL_CASE" -> new LowerCamelCasePropertyNamingStrategy();
-            case "UPPER_CAMEL_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.UPPER_CAMEL_CASE;
-            case "SNAKE_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.SNAKE_CASE;
-            case "UPPER_SNAKE_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.UPPER_SNAKE_CASE;
-            case "LOWER_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.LOWER_CASE;
-            case "KEBAB_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.KEBAB_CASE;
-            case "LOWER_DOT_CASE" ->
-                (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.LOWER_DOT_CASE;
+            case "UPPER_CAMEL_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.UPPER_CAMEL_CASE;
+            case "SNAKE_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.SNAKE_CASE;
+            case "UPPER_SNAKE_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.UPPER_SNAKE_CASE;
+            case "LOWER_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.LOWER_CASE;
+            case "KEBAB_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.KEBAB_CASE;
+            case "LOWER_DOT_CASE" -> (PropertyNamingStrategies.NamingBase) PropertyNamingStrategies.LOWER_DOT_CASE;
             default -> null;
         };
     }
@@ -702,31 +698,7 @@ private OpenAPI postProcessOpenApi(OpenAPI openApi, VisitorContext context) {
         new JacksonDiscriminatorPostProcessor().addMissingDiscriminatorType(openApi);
         new OpenApiOperationsPostProcessor().processOperations(openApi);
 
-        // remove unused schemas
-        try {
-            if (openApi.getComponents() != null) {
-                var extraSchemas = OpenApiExtraSchemaVisitor.getExtraSchemas();
-                Map<String, Schema> schemas = openApi.getComponents().getSchemas();
-                if (CollectionUtils.isNotEmpty(schemas)) {
-                    String openApiJson = Utils.getJsonMapper().writeValueAsString(openApi);
-                    // Create a copy of the keySet so that we can modify the map while in a foreach
-                    var keySet = new HashSet<>(schemas.keySet());
-                    for (String schemaName : keySet) {
-                        if (!openApiJson.contains("\"" + COMPONENTS_SCHEMAS_REF + schemaName + '"')
-                                && !extraSchemas.containsKey(schemaName)
-                        ) {
-                            schemas.remove(schemaName);
-                        }
-                    }
-                    // check excluded extra schemas also
-                    for (String schemaName : OpenApiExtraSchemaVisitor.getExcludedExtraSchemas()) {
-                        schemas.remove(schemaName);
-                    }
-                }
-            }
-        } catch (JsonProcessingException e) {
-            // do nothing
-        }
+        removeUnusedSchemas(openApi);
 
         removeEmptyComponents(openApi);
         findAndRemoveDuplicates(openApi);
@@ -738,18 +710,60 @@ private OpenAPI postProcessOpenApi(OpenAPI openApi, VisitorContext context) {
         return openApi;
     }
 
+    public static void removeUnusedSchemas(OpenAPI openApi) {
+        int i = 0;
+        // remove unused schemas
+        while (removeUnusedSchemasIter(openApi) || i > MAX_ITERATIONS) {
+            i++;
+        }
+    }
+
+    public static boolean removeUnusedSchemasIter(OpenAPI openApi) {
+        if (openApi.getComponents() == null) {
+            return false;
+        }
+        Map<String, Schema> schemas = openApi.getComponents().getSchemas();
+        if (CollectionUtils.isEmpty(schemas)) {
+            return false;
+        }
+
+        var extraSchemas = OpenApiExtraSchemaVisitor.getExtraSchemas();
+        var removed = false;
+
+        try {
+            String openApiJson = Utils.getJsonMapper().writeValueAsString(openApi);
+            // Create a copy of the keySet so that we can modify the map while in a foreach
+            var keySet = new HashSet<>(schemas.keySet());
+            for (String schemaName : keySet) {
+                if (!openApiJson.contains(QUOTE + COMPONENTS_SCHEMAS_REF + schemaName + QUOTE)
+                    && !extraSchemas.containsKey(schemaName)
+                ) {
+                    schemas.remove(schemaName);
+                    removed = true;
+                }
+            }
+            // check excluded extra schemas also
+            for (String schemaName : OpenApiExtraSchemaVisitor.getExcludedExtraSchemas()) {
+                schemas.remove(schemaName);
+            }
+        } catch (JsonProcessingException e) {
+            // do nothing
+        }
+        return removed;
+    }
+
     private void addExtraSchemas(OpenAPI openApi, VisitorContext context) {
-       var extraSchemas = OpenApiExtraSchemaVisitor.getExtraSchemas();
-       if (CollectionUtils.isEmpty(extraSchemas)) {
-           return;
-       }
-       var schemas = resolveSchemas(openApi);
-       for (var entry : extraSchemas.entrySet()) {
-           if (schemas.containsKey(entry.getKey())) {
-               continue;
-           }
-           schemas.put(entry.getKey(), entry.getValue());
-       }
+        var extraSchemas = OpenApiExtraSchemaVisitor.getExtraSchemas();
+        if (CollectionUtils.isEmpty(extraSchemas)) {
+            return;
+        }
+        var schemas = resolveSchemas(openApi);
+        for (var entry : extraSchemas.entrySet()) {
+            if (schemas.containsKey(entry.getKey())) {
+                continue;
+            }
+            schemas.put(entry.getKey(), entry.getValue());
+        }
     }
 
     private void generateViews(@Nullable String documentTitle, @Nullable Map<Pair<String, String>, OpenApiInfo> openApiInfos, VisitorContext context) {

diff --git a/openapi/src/main/java/io/micronaut/openapi/visitor/StringUtil.java b/openapi/src/main/java/io/micronaut/openapi/visitor/StringUtil.java
@@ -40,6 +40,7 @@ public final class StringUtil {
     public static final String UNDERSCORE = "_";
     public static final String MINUS = "-";
     public static final String WILDCARD = "*";
+    public static final String QUOTE = "\"";
 
     private StringUtil() {
     }

diff --git a/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiGroupSpec.groovy b/openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiGroupSpec.groovy
@@ -1,6 +1,8 @@
 package io.micronaut.openapi.visitor
 
 import io.micronaut.openapi.AbstractOpenApiTypeElementSpec
+import io.micronaut.openapi.OpenApiUtils
+import io.swagger.v3.oas.models.OpenAPI
 import io.swagger.v3.oas.models.security.SecurityScheme
 import spock.util.environment.RestoreSystemProperties
 
@@ -436,4 +438,83 @@ public class MyBean {}
         apiV1.paths.'/demo'.get.responses.'200'.content.'application/json'.schema.$ref == '#/components/schemas/HelloResponseV1'
         apiV2.paths.'/demo'.get.responses.'200'.content.'application/json'.schema.$ref == '#/components/schemas/HelloResponseV2'
     }
+
+    void "test remove unused schemas"() {
+
+        when:
+        var openApiSpec = """
+openapi: 3.0.1
+info:
+  title: openapi-groups
+  version: "0.0"
+paths:
+  /visible:
+    get:
+      operationId: index
+      responses:
+        "200":
+          description: index 200 response
+          content:
+            application/json:
+              schema:
+                \$ref: "#/components/schemas/VisibleResponse"
+components:
+  schemas:
+    NotVisibleClassKO:
+      type: object
+      properties:
+        test:
+          type: string
+      description: "This class should not appear in the schema, but it does because\\
+        \\ it's a property of NotVisibleResponse"
+    NotVisibleEnumKO:
+      type: string
+      description: "This enum should not appear in the schema, but it does because\\
+        \\ it's a property of NotVisibleResponse"
+      enum:
+      - VALUE1
+      - VALUE2
+    NotVisibleRequest:
+      type: object
+      properties:
+        part:
+          \$ref: "#/components/schemas/NotVisibleRequestPartKO"
+    NotVisibleRequestPartKO:
+      type: object
+      properties:
+        test:
+          type: string
+      description: "This class should not appear in the schema, but it does because\\
+        \\ it's a property of NotVisibleResponse"
+    NotVisibleResponse:
+      type: object
+      properties:
+        test:
+          type: string
+        notVisibleEnumKO:
+          \$ref: "#/components/schemas/NotVisibleEnumKO"
+        notVisibleClassKO:
+          \$ref: "#/components/schemas/NotVisibleClassKO"
+        notVisibleInstantOK:
+          type: string
+          description: "If this getter is uncommented, nothing about Instant will\\
+            \\ be visible in the schema"
+          format: date-time
+    VisibleResponse:
+      required:
+      - visibleField
+      type: object
+      properties:
+        visibleField:
+          type: string
+"""
+        var openApi = OpenApiUtils.getYamlMapper().readValue(openApiSpec, OpenAPI.class)
+        OpenApiApplicationVisitor.removeUnusedSchemas(openApi)
+
+        then:
+        openApi.components
+        openApi.components.schemas
+        openApi.components.schemas.size() == 1
+        openApi.components.schemas.VisibleResponse
+    }
 }