Improve OneOf handling with new normalizer REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING (#23543)

* normalizer REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING

* Work in progress

* Improvements

* Merge master

* Fix invalid path

* Improve assertions

* Fix invalid discriminator value

* filename case

* filename case

* Rollback composed-oneof.yaml

* Improve normalization

* Fix building of allOf

* Fix hasParent

* Fix some cubic findings

* Fix some cubic findings

* Fix infinite recursion stopping too early

* Force build

* Use getReferencedSchema in search for properties

* Improve hasParent -> isParentReferencedInChild

* Cubic suggestions

* Add assertions for JsonSubTypes.Types

* Clean moved child

* Convert a ComposedSchema to a Schema if there is no oneOf/allOf/anyOf
-> avoid side effects with bugs in InlineModelResolver

* Undo changes in decompose Schema

* Inline model resolver for issue 22209

* Undo formatting in ModelUtils

* Merge master

Author: jpfinne

docs/customization.md

@@ -651,6 +651,13 @@ Example:
 java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i modules/openapi-generator/src/test/resources/3_0/required-properties.yaml -o /tmp/java-okhttp/ --openapi-normalizer REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT=true
 ```
 
+- `REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING`: when set to true, oneOf is removed and is converted into mappings in a discriminator mapping.
+
+Example:
+```
+java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g spring -i modules/openapi-generator/src/test/resources/3_0/spring/issue_23527.yaml -o /tmp/java-spring/ --openapi-normalizer REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING=true
+```
+
 - `FILTER`
 
 The `FILTER` parameter allows selective inclusion of API operations based on specific criteria. It applies the `x-internal: true` property to operations that do **not** match the specified values, preventing them from being generated. Multiple filters can be separated by a semicolon.

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

@@ -694,6 +694,10 @@ private void flattenComponents() {
             } else if (ModelUtils.isOneOf(model)) { // contains oneOf only
                 gatherInlineModels(model, modelName);
             } else if (ModelUtils.isComposedSchema(model)) {
+                // composed Schema can have properties!
+                if (ModelUtils.hasProperties(model)) {
+                    gatherInlineModels(model, modelName);
+                }
                 // inline child schemas
                 flattenComposedChildren(modelName + "_allOf", model.getAllOf(), !Boolean.TRUE.equals(this.refactorAllOfInlineSchemas));
                 flattenComposedChildren(modelName + "_anyOf", model.getAnyOf(), false);

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

@@ -78,6 +78,10 @@ public class OpenAPINormalizer {
     // are removed as most generators cannot handle such case at the moment
     final String REMOVE_ANYOF_ONEOF_AND_KEEP_PROPERTIES_ONLY = "REMOVE_ANYOF_ONEOF_AND_KEEP_PROPERTIES_ONLY";
 
+    // when set to true, oneOf is removed and is converted into mappings in a discriminator mapping
+    final String REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING = "REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING";
+
+
     // when set to true, oneOf/anyOf with either string or enum string as sub schemas will be simplified
     // to just string
     final String SIMPLIFY_ANYOF_STRING_AND_ENUM_STRING = "SIMPLIFY_ANYOF_STRING_AND_ENUM_STRING";
@@ -214,6 +218,7 @@ public OpenAPINormalizer(OpenAPI openAPI, Map<String, String> inputRules) {
         ruleNames.add(SIMPLIFY_ONEOF_ANYOF_ENUM);
         ruleNames.add(REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT);
         ruleNames.add(SORT_MODEL_PROPERTIES);
+        ruleNames.add(REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING);
 
         // rules that are default to true
         rules.put(SIMPLIFY_ONEOF_ANYOF, true);
@@ -639,6 +644,10 @@ protected void normalizeComponentsSchemas() {
 
                 // normalize the schemas
                 schemas.put(schemaName, normalizeSchema(schema, new HashSet<>()));
+
+                if (getRule(REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING)) {
+                    ensureInheritanceForDiscriminatorMappings(schema, schemaName);
+                }
             }
         }
     }
@@ -1069,6 +1078,7 @@ protected Schema normalizeOneOf(Schema schema, Set<Schema> visitedSchemas) {
                 // update sub-schema with the updated schema
                 schema.getOneOf().set(i, normalizeSchema((Schema) item, visitedSchemas));
             }
+            schema = processReplaceOneOfByMapping(schema);
         } else {
             // normalize it as it's no longer an oneOf
             schema = normalizeSchema(schema, visitedSchemas);
@@ -1564,6 +1574,245 @@ protected Schema processSimplifyOneOf(Schema schema) {
         return schema;
     }
 
+
+    /**
+     * Ensure inheritance is correctly defined for OneOf and Discriminators.
+     *
+     * For schemas containing oneOf and discriminator.propertyName:
+     * <ul>
+     *  <li>Create the mappings as $refs</li>
+     *  <li>Remove OneOf</li>
+     * </ul>
+     */
+    protected Schema processReplaceOneOfByMapping(Schema schema) {
+        if (!getRule(REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING) || schema.getOneOf() == null) {
+            return schema;
+        }
+        Discriminator discriminator = schema.getDiscriminator();
+        if (discriminator != null) {
+            boolean inlineSchema = isInlineSchema(schema);
+            if (inlineSchema) {
+                // the For referenced schemas, ensure that there is an allOf with this schema.
+                LOGGER.warn("Inline oneOf schema not supported by REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING normalization");
+                return schema;
+            }
+            if (discriminator.getMapping() == null && discriminator.getPropertyName() != null) {
+                List<Schema> oneOfs = schema.getOneOf();
+                if (oneOfs.stream().anyMatch(oneOf -> oneOf.get$ref() == null)) {
+                    LOGGER.warn("oneOf should only contain $ref for REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING normalization");
+                    return schema;
+                }
+                Map<String, String> mappings = new TreeMap<>();
+                // is the discriminator qttribute qlready in this schema?
+                // if yes, it will be deleted in references oneOf to avoid duplicates
+                boolean hasProperty = findProperty(schema, discriminator.getPropertyName(), false, new HashSet<>()) != null;
+                discriminator.setMapping(mappings);
+                for (Schema oneOf : oneOfs) {
+                    String refSchema = oneOf.get$ref();
+                    String name = getDiscriminatorValue(refSchema, discriminator.getPropertyName(), hasProperty, new HashSet<>(List.of(schema)));
+                    mappings.put(name, refSchema);
+
+                }
+                // remove oneOf and only keep the new discriminator mapping
+                schema.oneOf(null);
+            } else if (discriminator.getPropertyName() == null) {
+                LOGGER.warn("Missing property name in discriminator");
+            } else if (discriminator.getMapping() != null && discriminator.getMapping().size() != schema.getOneOf().size()) {
+                LOGGER.warn("Discriminator mapping size " + discriminator.getMapping().size() + " mismatch with oneOf size " + schema.getOneOf().size());
+            } else {
+                // remove oneOf and only keep the discriminator mapping
+                LOGGER.info("Removing oneOf, discriminator mapping takes precedences on OneOfs");
+                schema.oneOf(null);
+            }
+        }
+
+        return schema;
+    }
+
+    private boolean isInlineSchema(Schema schema) {
+        if (openAPI.getComponents()!=null && openAPI.getComponents().getSchemas()!=null) {
+            int identity = System.identityHashCode(schema);
+            for (Schema componentSchema: openAPI.getComponents().getSchemas().values()) {
+                if (System.identityHashCode(componentSchema) == identity) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Best effort to retrieve a good discriminator value.
+     * By order of precedence:
+     * <ul>
+     *  <li>x-discriminator-value</li>
+     *  <li>single enum value for attribute used by the discriminator.propertyName</li>
+     *  <li>hame of the schema</li>
+     * </ul>
+     *
+     * @param refSchema $ref value like #/components/schemas/Dog
+     * @param discriminatorPropertyName name of the property used in the discriminator mapping
+     * @param propertyAlreadyPresent if true, delete the property in the referenced schemas to avoid duplicates
+     *
+     * @return the name
+     */
+    protected String getDiscriminatorValue(String refSchema, String discriminatorPropertyName, boolean propertyAlreadyPresent, Set<Schema> visitedSchemas) {
+        String schemaName = ModelUtils.getSimpleRef(refSchema);
+        Schema schema = ModelUtils.getSchema(openAPI, schemaName);
+        Schema property = findProperty(schema, discriminatorPropertyName, propertyAlreadyPresent, visitedSchemas);
+        if (schema != null && schema.getExtensions() != null) {
+            Object discriminatorValue = schema.getExtensions().get("x-discriminator-value");
+            if (discriminatorValue != null) {
+                return discriminatorValue.toString();
+            }
+        }
+
+        // find the discriminator value as a unique enum value
+        property = ModelUtils.getReferencedSchema(openAPI, property);
+        if (property != null) {
+            List enums = property.getEnum();
+            if (enums != null && enums.size() == 1) {
+                return enums.get(0).toString();
+            }
+        }
+
+        return schemaName;
+    }
+
+    /**
+     * find a property under the schema.
+     *
+     * @param schema
+     * @param propertyName property to find
+     * @param toDelete if true delete the found property
+     * @param visitedSchemas avoid infinite recursion
+     * @return found property or null if not found.
+     */
+    private Schema findProperty(Schema schema, String propertyName, boolean toDelete, Set<Schema> visitedSchemas) {
+        schema = ModelUtils.getReferencedSchema(openAPI, schema);
+        if (propertyName == null || schema == null || visitedSchemas.contains(schema)) {
+            return null;
+        }
+        visitedSchemas.add(schema);
+        Map<String, Schema>  properties = schema.getProperties();
+        if (properties != null) {
+            Schema property = ModelUtils.getReferencedSchema(openAPI, properties.get(propertyName));
+            if (property != null) {
+                if (toDelete) {
+                    if (schema.getProperties().remove(propertyName) != null) {
+                        LOGGER.info("property " + propertyName + " has been removed in REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING normalization");
+                        if (schema.getProperties().isEmpty()) {
+                            schema.setProperties(null);
+                        }
+                    }
+                }
+                return property;
+            }
+        }
+        List<Schema> allOfs = schema.getAllOf();
+        if (allOfs != null) {
+            for (Schema child : allOfs) {
+                Schema found = findProperty(child, propertyName, toDelete, visitedSchemas);
+                if (found != null) {
+                    return found;
+                }
+            }
+        }
+
+        return null;
+    }
+
+
+    /**
+     * ensure that all schemas referenced in the discriminator mapping has an allOf to the parent schema.
+     *
+     * This allows DefaultCodeGen to detect inheritance.
+     *
+     * @param parent parent schma
+     * @param parentName name of the parent schema
+     */
+    protected void ensureInheritanceForDiscriminatorMappings(Schema parent, String parentName) {
+        Discriminator discriminator = parent.getDiscriminator();
+        if (discriminator != null && discriminator.getMapping() != null) {
+            for (String mapping : discriminator.getMapping().values()) {
+                String refSchemaName = ModelUtils.getSimpleRef(mapping);
+                Schema child = ModelUtils.getSchema(openAPI, refSchemaName);
+                if (child != null) {
+                    if (parentName != null) {
+                        ensureInheritanceForDiscriminatorMapping(parent, child, parentName, new HashSet<>());
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * If not already present, add in the child an allOf referencing the parent.
+     */
+    protected void ensureInheritanceForDiscriminatorMapping(Schema parent, Schema child, String parentName, Set<Schema> visitedSchemas) {
+        String reference = "#/components/schemas/" + parentName;
+        List<Schema> allOf = child.getAllOf();
+        if (allOf != null) {
+            if (isParentReferencedInChild(parent, child, reference, visitedSchemas)) {
+                // already done, so no need to add
+                return;
+            }
+            Schema refToParent = new Schema<>().$ref(reference);
+            allOf.add(refToParent);
+        } else {
+            allOf = new ArrayList<>();
+            child.setAllOf(allOf);
+            Schema refToParent = new Schema<>().$ref(reference);
+            allOf.add(refToParent);
+            Map<String, Schema> childProperties = child.getProperties();
+            if (childProperties != null) {
+                // move the properties inside the new allOf.
+                Schema newChildProperties = new Schema<>()
+                        .properties(childProperties)
+                        .additionalProperties(child.getAdditionalProperties());
+                ModelUtils.copyMetadata(child, newChildProperties);
+                allOf.add(newChildProperties);
+                child.properties(null)
+                        .type(null)
+                        .additionalProperties(null)
+                        .description(null)
+                        ._default(null)
+                        .deprecated(null)
+                        .example(null)
+                        .examples(null)
+                        .readOnly(null)
+                        .writeOnly(null)
+                        .title(null);
+            }
+        }
+    }
+
+    /**
+     * return true if the child as an allOf referencing the parent schema.
+     */
+    private boolean isParentReferencedInChild(Schema parent, Schema child, String reference, Set<Schema> visitedSchemas) {
+        if (child == null || visitedSchemas.contains(child)) {
+            return false;
+        }
+        if (child.get$ref() != null && child.get$ref().equals(reference)) {
+            return true;
+        }
+        child = ModelUtils.getReferencedSchema(openAPI, child);
+        if (visitedSchemas.contains(child)) {
+            return false;
+        }
+        visitedSchemas.add(child);
+        List<Schema> allOf = child.getAllOf();
+        if (allOf != null) {
+            for (Schema  schema : allOf) {
+                if (isParentReferencedInChild(parent, schema, reference, visitedSchemas)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     /**
      * Set nullable to true in array/set if needed.
      *

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

@@ -26,7 +26,6 @@
 import org.openapitools.codegen.utils.ModelUtils;
 import org.testng.annotations.Test;
 
-import java.lang.reflect.Array;
 import java.math.BigDecimal;
 import java.util.*;
 
@@ -1502,4 +1501,47 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
         }
     }
 
+    @Test
+    public void testREPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING() {
+        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_23527.yaml");
+
+        Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
+        OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
+        openAPINormalizer.normalize();
+
+        Schema geoJsonObject = openAPI.getComponents().getSchemas().get("GeoJsonObject");
+        Map<String, String> mapping = geoJsonObject.getDiscriminator().getMapping();
+        assertEquals(mapping, Map.of("MultiPolygon", "#/components/schemas/Multi-Polygon", "Polygon", "#/components/schemas/Polygon" ));
+    }
+
+    @Test
+    public void issue_14769() {
+        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_14769.yaml");
+        Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
+        OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
+        openAPINormalizer.normalize();
+//        ModelUtils.dumpAsYaml(openAPI);
+        Schema vehicle = openAPI.getComponents().getSchemas().get("Vehicle");
+        Map<String, String> mapping = vehicle.getDiscriminator().getMapping();
+        assertEquals(mapping, Map.of("car", "#/components/schemas/Car", "plane", "#/components/schemas/Plane" ));
+        Schema car = openAPI.getComponents().getSchemas().get("Car");
+        assertNull(car.getProperties());
+        assertEquals(car.getAllOf().size(), 2);
+        assertEquals(((Schema)car.getAllOf().get(0)).get$ref(), "#/components/schemas/Vehicle");
+        assertEquals(((Schema)car.getAllOf().get(1)).getProperties().size(), 1);
+        assertEquals(((Schema)car.getAllOf().get(1)).getProperties().keySet(), Set.of("has_4_wheel_drive"));
+    }
+
+    @Test
+    public void oneOf_issue_23276() {
+        OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/oneOf_issue_23276.yaml");
+        Map<String, String> inputRules = Map.of("REPLACE_ONE_OF_BY_DISCRIMINATOR_MAPPING", "true");
+        OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
+        openAPINormalizer.normalize();
+//         ModelUtils.dumpAsYaml(openAPI);
+        Schema payload = (Schema)openAPI.getComponents().getSchemas().get("DeviceLifecycleEvent").getProperties().get("payload");
+        // inline oneOf are not converted
+        assertNotNull(payload.getOneOf());
+    }
+
 }