Add new openapi-normalizer rule REFACTOR_ALLOF_WITH_PROPERTIES_ONLY (#15039)

* add new rule REFACTOR_ALLOF_WITH_PROPERTIES_ONLY

* update other attributes

* minor refactoring

Author: wing328

docs/customization.md

@@ -458,7 +458,6 @@ OpenAPI Normalizer (off by default) transforms the input OpenAPI doc/spec (which
 
 - `REF_AS_PARENT_IN_ALLOF`: when set to `true`, child schemas in `allOf` is considered a parent if it's a `$ref` (instead of inline schema).
 
-
 Example:
 ```
 java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i modules/openapi-generator/src/test/resources/3_0/allOf_extension_parent.yaml -o /tmp/java-okhttp/ --openapi-normalizer REF_AS_PARENT_IN_ALLOF=true
@@ -512,3 +511,11 @@ Example:
 ```
 java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i modules/openapi-generator/src/test/resources/3_0/addUnsignedToIntegerWithInvalidMaxValue_test.yaml -o /tmp/java-okhttp/ --openapi-normalizer ADD_UNSIGNED_TO_INTEGER_WITH_INVALID_MAX_VALUE=true
 ```
+
+- `REFACTOR_ALLOF_WITH_PROPERTIES_ONLY`: When set to true, refactor schema with allOf and properties in the same level to a schema with allOf only and, the allOf contains a new schema containing the properties in the top level.
+
+Example:
+```
+java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i modules/openapi-generator/src/test/resources/3_0/allOf_extension_parent.yaml -o /tmp/java-okhttp/ --openapi-normalizer REFACTOR_ALLOF_WITH_PROPERTIES_ONLY=true
+```
+

modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java

@@ -82,6 +82,11 @@ public class OpenAPINormalizer {
     final String ADD_UNSIGNED_TO_INTEGER_WITH_INVALID_MAX_VALUE = "ADD_UNSIGNED_TO_INTEGER_WITH_INVALID_MAX_VALUE";
     boolean addUnsignedToIntegerWithInvalidMaxValue;
 
+    // when set to true, refactor schema with allOf and properties in the same level to a schema with allOf only and
+    // the allOf contains a new schema containing the properties in the top level
+    final String REFACTOR_ALLOF_WITH_PROPERTIES_ONLY = "REFACTOR_ALLOF_WITH_PROPERTIES_ONLY";
+    boolean refactorAllOfWithPropertiesOnly;
+
     // ============= end of rules =============
 
     /**
@@ -141,6 +146,10 @@ public void parseRules(Map<String, String> rules) {
         if (enableAll || "true".equalsIgnoreCase(rules.get(ADD_UNSIGNED_TO_INTEGER_WITH_INVALID_MAX_VALUE))) {
             addUnsignedToIntegerWithInvalidMaxValue = true;
         }
+
+        if (enableAll || "true".equalsIgnoreCase(rules.get(REFACTOR_ALLOF_WITH_PROPERTIES_ONLY))) {
+            refactorAllOfWithPropertiesOnly = true;
+        }
     }
 
     /**
@@ -346,6 +355,9 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
             return normalizeOneOf(schema, visitedSchemas);
         } else if (ModelUtils.isAnyOf(schema)) { // anyOf
             return normalizeAnyOf(schema, visitedSchemas);
+        } else if (ModelUtils.isAllOfWithProperties(schema)) { // allOf with properties
+            schema = normalizeAllOfWithProperties(schema, visitedSchemas);
+            normalizeSchema(schema, visitedSchemas);
         } else if (ModelUtils.isAllOf(schema)) { // allOf
             return normalizeAllOf(schema, visitedSchemas);
         } else if (ModelUtils.isComposedSchema(schema)) { // composed schema
@@ -427,6 +439,20 @@ private Schema normalizeAllOf(Schema schema, Set<Schema> visitedSchemas) {
         return schema;
     }
 
+    private Schema normalizeAllOfWithProperties(Schema schema, Set<Schema> visitedSchemas) {
+        for (Object item : schema.getAllOf()) {
+            if (!(item instanceof Schema)) {
+                throw new RuntimeException("Error! allOf schema is not of the type Schema: " + item);
+            }
+            // normalize allOf sub schemas one by one
+            normalizeSchema((Schema) item, visitedSchemas);
+        }
+        // process rules here
+        schema = processRefactorAllOfWithPropertiesOnly(schema);
+
+        return schema;
+    }
+
     private Schema normalizeOneOf(Schema schema, Set<Schema> visitedSchemas) {
         for (Object item : schema.getOneOf()) {
             if (item == null) {
@@ -759,5 +785,46 @@ private void processAddUnsignedToIntegerWithInvalidMaxValue(Schema schema) {
         }
     }
 
+    /*
+     * When set to true, refactor schema with allOf and properties in the same level to a schema with allOf only and
+     * the allOf contains a new schema containing the properties in the top level.
+     *
+     * @param schema Schema
+     * @return Schema
+     */
+    private Schema processRefactorAllOfWithPropertiesOnly(Schema schema) {
+        if (!refactorAllOfWithPropertiesOnly && !enableAll) {
+            return schema;
+        }
+
+        ObjectSchema os = new ObjectSchema();
+        // set the properties, etc of the new schema to the properties of schema
+        os.setProperties(schema.getProperties());
+        os.setRequired(schema.getRequired());
+        os.setAdditionalProperties(schema.getAdditionalProperties());
+        os.setNullable(schema.getNullable());
+        os.setDescription(schema.getDescription());
+        os.setDeprecated(schema.getDeprecated());
+        os.setExample(schema.getExample());
+        os.setExamples(schema.getExamples());
+        os.setTitle(schema.getTitle());
+        schema.getAllOf().add(os); // move new schema as a child schema of allOf
+        // clean up by removing properties, etc
+        schema.setProperties(null);
+        schema.setRequired(null);
+        schema.setAdditionalProperties(null);
+        schema.setNullable(null);
+        schema.setDescription(null);
+        schema.setDeprecated(null);
+        schema.setExample(null);
+        schema.setExamples(null);
+        schema.setTitle(null);
+
+        // at this point the schema becomes a simple allOf (no properties) with an additional schema containing
+        // the properties
+
+        return schema;
+    }
+
     // ===================== end of rules =====================
 }

modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

@@ -1811,6 +1811,19 @@ public static boolean hasAllOf(Schema schema) {
         return false;
     }
 
+    /**
+     * Returns true if the schema contains allOf and properties,
+     * and no oneOf/anyOf defined.
+     *
+     * @param schema the schema
+     * @return true if the schema contains allOf but no properties/oneOf/anyOf defined.
+     */
+    public static boolean isAllOfWithProperties(Schema schema) {
+        return hasAllOf(schema) && (schema.getProperties() != null && !schema.getProperties().isEmpty()) &&
+                (schema.getOneOf() == null || schema.getOneOf().isEmpty()) &&
+                (schema.getAnyOf() == null || schema.getAnyOf().isEmpty());
+    }
+
     /**
      * Returns true if the schema contains oneOf but
      * no properties/allOf/anyOf defined.

modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java

@@ -278,4 +278,30 @@ public void testOpenAPINormalizerConvertEnumNullToNullable_test() {
         assertEquals(schema3.getAnyOf().size(), 2);
         assertTrue(schema3.getNullable());
     }
+
+    @Test
+    public void testOpenAPINormalizerRefactorAllOfWithPropertiesOnly() {
+        // to test the rule REFACTOR_ALLOF_WITH_PROPERTIES_ONLY
+        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/allOf_extension_parent.yaml");
+
+        ComposedSchema schema = (ComposedSchema) openAPI.getComponents().getSchemas().get("allOfWithProperties");
+        assertEquals(schema.getAllOf().size(), 1);
+        assertEquals(schema.getProperties().size(), 2);
+        assertEquals(((Schema) schema.getProperties().get("isParent")).getType(), "boolean");
+        assertEquals(((Schema) schema.getProperties().get("mum_or_dad")).getType(), "string");
+
+        Map<String, String> options = new HashMap<>();
+        options.put("REFACTOR_ALLOF_WITH_PROPERTIES_ONLY", "true");
+        OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
+        openAPINormalizer.normalize();
+
+        Schema schema2 = openAPI.getComponents().getSchemas().get("allOfWithProperties");
+        assertEquals(schema2.getAllOf().size(), 2);
+        assertNull(schema2.getProperties());
+
+        Schema newSchema = (Schema) (schema2.getAllOf().get(1));
+        assertEquals(((Schema) newSchema.getProperties().get("isParent")).getType(), "boolean");
+        assertEquals(((Schema) newSchema.getProperties().get("mum_or_dad")).getType(), "string");
+        assertEquals(newSchema.getRequired().get(0), "isParent");
+    }
 }

modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java

@@ -1746,7 +1746,7 @@ public void testJdkHttpClientWithAndWithoutParentExtension() throws Exception {
         generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "true");
         List<File> files = generator.opts(clientOptInput).generate();
 
-        Assert.assertEquals(files.size(), 30);
+        Assert.assertEquals(files.size(), 33);
         validateJavaSourceFiles(files);
 
         TestUtils.assertFileContains(Paths.get(output + "/src/main/java/xyz/abcdef/model/Child.java"),

modules/openapi-generator/src/test/resources/3_0/allOf_extension_parent.yaml

@@ -85,3 +85,15 @@ components:
           type: boolean
         mum_or_dad:
           type: string
+    allOfWithProperties:
+      description: parent object without x-parent extension
+      type: object
+      allOf:
+        - $ref: '#/components/schemas/AnotherParent'
+      properties:
+        isParent:
+          type: boolean
+        mum_or_dad:
+          type: string
+      required:
+        - isParent