Fix bug in ModelUtils.getParentName resulting in wrong inner Models for oneOf-composed schemas (#21799)

* Test Cases for more than two oneOf-Options (both passing, but important to narrow down observed bug)

* fix language-specific tests broken by adding a third fruit to oneOf test

* create reproducer unit test for java client codegen

* fix typo in test yaml

* fix ModelUtils.getParentName returning name of first element in composed schema instead of null when there are multiple elements and it is not clear which one should be parent

* rename test yaml and added tests for clarity

* update samples

* update samples again

Author: fkellner

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

@@ -1573,17 +1573,18 @@ public static String getParentName(Schema composedSchema, Map<String, Schema> al
         List<String> refedWithoutDiscriminator = new ArrayList<>();
 
         if (interfaces != null && !interfaces.isEmpty()) {
+            List<String> parentNameCandidates = new ArrayList<>(interfaces.size());
             for (Schema schema : interfaces) {
                 // get the actual schema
                 if (StringUtils.isNotEmpty(schema.get$ref())) {
                     String parentName = getSimpleRef(schema.get$ref());
                     Schema s = allSchemas.get(parentName);
                     if (s == null) {
                         LOGGER.error("Failed to obtain schema from {}", parentName);
-                        return "UNKNOWN_PARENT_NAME";
+                        parentNameCandidates.add("UNKNOWN_PARENT_NAME");
                     } else if (hasOrInheritsDiscriminator(s, allSchemas, new ArrayList<Schema>())) {
                         // discriminator.propertyName is used or x-parent is used
-                        return parentName;
+                        parentNameCandidates.add(parentName);
                     } else {
                         // not a parent since discriminator.propertyName or x-parent is not set
                         hasAmbiguousParents = true;
@@ -1600,6 +1601,12 @@ public static String getParentName(Schema composedSchema, Map<String, Schema> al
                     }
                 }
             }
+            if (parentNameCandidates.size() > 1) {
+                // unclear which one should be the parent
+                return null;
+            } else if (parentNameCandidates.size() == 1) {
+                return parentNameCandidates.get(0);
+            }
             if (refedWithoutDiscriminator.size() == 1 && nullSchemaChildrenCount == 1) {
                 // One schema is a $ref and the other is the 'null' type, so the parent is obvious.
                 // In this particular case there is no need to specify a discriminator.

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

@@ -766,9 +766,10 @@ public void testComposedSchemaOneOfWithProperties() {
         Set<String> oneOf = new TreeSet<>();
         oneOf.add("Apple");
         oneOf.add("Banana");
+        oneOf.add("Orange");
         assertEquals(fruit.oneOf, oneOf);
-        assertEquals(3, fruit.optionalVars.size());
-        assertEquals(3, fruit.vars.size());
+        assertEquals(4, fruit.optionalVars.size());
+        assertEquals(4, fruit.vars.size());
         // make sure that fruit has the property color
         boolean colorSeen = false;
         for (CodegenProperty cp : fruit.vars) {
@@ -788,6 +789,32 @@ public void testComposedSchemaOneOfWithProperties() {
         assertTrue(colorSeen);
     }
 
+    @Test
+    public void testComposedSchemaOneOfWithInnerModel() {
+        final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/oneOf_innerModel.yaml");
+        final DefaultCodegen codegen = new DefaultCodegen();
+
+        final Schema schema = openAPI.getComponents().getSchemas().get("RandomAnimalsResponse_animals_inner");
+        codegen.setOpenAPI(openAPI);
+        CodegenModel randomAnimalsResponseInner = codegen.fromModel("RandomAnimalsResponse_animals_inner", schema);
+
+        Set<String> oneOf = new TreeSet<>();
+        oneOf.add("Mouse");
+        oneOf.add("Cat");
+        oneOf.add("Dog");
+        assertEquals(oneOf, randomAnimalsResponseInner.oneOf);
+        assertEquals(4, randomAnimalsResponseInner.vars.size());
+        // make sure that RandomAnimalsResponseInner has the property species
+        boolean speciesSeen = false;
+        for (CodegenProperty cp : randomAnimalsResponseInner.vars) {
+            if ("species".equals(cp.name)) {
+                speciesSeen = true;
+                break;
+            }
+        }
+        assertTrue(speciesSeen);
+    }
+
     @Test
     public void testEscapeText() {
         final DefaultCodegen codegen = new DefaultCodegen();

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

@@ -40,12 +40,14 @@
 import org.openapitools.codegen.java.assertions.JavaFileAssert;
 import org.openapitools.codegen.languages.AbstractJavaCodegen;
 import org.openapitools.codegen.languages.JavaClientCodegen;
+import org.openapitools.codegen.languages.RubyClientCodegen;
 import org.openapitools.codegen.languages.features.BeanValidationFeatures;
 import org.openapitools.codegen.languages.features.CXFServerFeatures;
 import org.openapitools.codegen.meta.features.SecurityFeature;
 import org.openapitools.codegen.model.OperationMap;
 import org.openapitools.codegen.model.OperationsMap;
 import org.openapitools.codegen.testutils.ConfigAssert;
+import org.testng.Assert;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Parameters;
 import org.testng.annotations.Test;
@@ -67,6 +69,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
 import static org.assertj.core.api.InstanceOfAssertFactories.FILE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.openapitools.codegen.CodegenConstants.*;
 import static org.openapitools.codegen.TestUtils.*;
 import static org.openapitools.codegen.languages.JavaClientCodegen.*;
@@ -3837,4 +3841,49 @@ public void queryParameterJsonSerialization(String library) {
     public static Object[] springClients() {
         return new Object[]{RESTCLIENT, WEBCLIENT};
     }
+
+    @Test(description = "test oneOf (OAS3)")
+    public void oneOfTest() {
+        final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/oneOf.yaml");
+        final JavaClientCodegen codegen = new JavaClientCodegen();
+
+        final Schema schema = openAPI.getComponents().getSchemas().get("fruit");
+        codegen.setOpenAPI(openAPI);
+        CodegenModel fruit = codegen.fromModel("Fruit", schema);
+
+        Set<String> oneOf = new TreeSet<String>();
+        oneOf.add("Apple");
+        oneOf.add("Banana");
+        oneOf.add("Orange");
+        Assert.assertEquals(fruit.oneOf, oneOf);
+
+        assertEquals(4, fruit.optionalVars.size());
+        assertEquals(4, fruit.vars.size());
+    }
+
+    @Test
+    public void oneOfWithInnerModelTest() {
+        final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/oneOf_innerModel.yaml");
+        final JavaClientCodegen codegen = new JavaClientCodegen();
+
+        final Schema schema = openAPI.getComponents().getSchemas().get("RandomAnimalsResponse_animals_inner");
+        codegen.setOpenAPI(openAPI);
+        CodegenModel randomAnimalsResponseInner = codegen.fromModel("RandomAnimalsResponse_animals_inner", schema);
+
+        Set<String> oneOf = new TreeSet<>();
+        oneOf.add("Mouse");
+        oneOf.add("Cat");
+        oneOf.add("Dog");
+        assertEquals(oneOf, randomAnimalsResponseInner.oneOf);
+        assertEquals(4, randomAnimalsResponseInner.vars.size());
+        // make sure that RandomAnimalsResponseInner has the property species
+        boolean speciesSeen = false;
+        for (CodegenProperty cp : randomAnimalsResponseInner.vars) {
+            if ("species".equals(cp.name)) {
+                speciesSeen = true;
+                break;
+            }
+        }
+        assertTrue(speciesSeen);
+    }
 }

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

@@ -368,6 +368,7 @@ public void oneOfTest() {
         Set<String> oneOf = new TreeSet<String>();
         oneOf.add("Apple");
         oneOf.add("Banana");
+        oneOf.add("Orange");
         Assert.assertEquals(fruit.oneOf, oneOf);
     }
 

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

@@ -675,4 +675,12 @@ public void testModelWithPropertiesOnly() {
         testSchema.setAdditionalProperties(new Schema().type("string"));
         assertFalse(ModelUtils.isModelWithPropertiesOnly(testSchema));
     }
+
+    @Test
+    public void getParentNameMultipleInterfacesTest() {
+        OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/oneOf_innerModel.yaml");
+        Map<String, Schema> allSchemas = openAPI.getComponents().getSchemas();
+        Schema composedSchema = allSchemas.get("RandomAnimalsResponse_animals_inner");
+        assertNull(ModelUtils.getParentName(composedSchema, allSchemas));
+    }
 }

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

@@ -22,6 +22,7 @@ components:
          oneOf:
             - $ref: '#/components/schemas/apple'
             - $ref: '#/components/schemas/banana'
+            - $ref: '#/components/schemas/orange'
          # additionalProperties:
          #    type: string
          # uncomment this when https://github.com/swagger-api/swagger-parser/issues/1252 is resolved
@@ -37,3 +38,9 @@ components:
          properties:
             count:
                type: number
+      orange:
+         title: orange
+         type: object
+         properties:
+            sweet:
+               type: boolean

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

@@ -0,0 +1,70 @@
+---
+openapi: 3.0.3
+info:
+  title: randimals
+  version: "1.0"
+paths:
+  /random-animals:
+    get:
+      tags:
+        - Random Animals Resource
+      responses:
+        "200":
+          description: OK
+          content:
+            '*/*':
+              schema:
+                $ref: '#/components/schemas/RandomAnimalsResponse'
+components:
+  schemas:
+    RandomAnimalsResponse:
+      type: object
+      properties:
+        animals:
+          type: array
+          items:
+            oneOf:
+              - $ref: '#/components/schemas/Dog'
+              - $ref: '#/components/schemas/Cat'
+              - $ref: '#/components/schemas/Mouse'
+      required:
+        - animals
+    Dog:
+      allOf:
+        - $ref: '#/components/schemas/Animal'
+        - type: object
+          properties:
+            dogId:
+              type: string
+      required:
+        - dogId
+    Animal:
+      discriminator:
+        propertyName: species
+        mapping:
+          Dog: '#/components/schemas/Dog'
+          Cat: '#/components/schemas/Cat'
+          Mouse: '#/components/schemas/Mouse'
+      properties:
+        species:
+          type: string
+      required:
+        - species
+    Cat:
+      allOf:
+        - $ref: '#/components/schemas/Animal'
+        - type: object
+          properties:
+            catId:
+              type: string
+      required:
+        - catId
+    Mouse:
+      allOf:
+        - $ref: '#/components/schemas/Animal'
+        - type: object
+          properties:
+            mouseId:
+              type: string
+      required:
+        - mouseId

modules/openapi-generator/src/test/resources/3_0/protobuf-schema/fruitOneOf.proto

@@ -14,14 +14,13 @@ package openapitools;
 
 import public "models/apple.proto";
 import public "models/banana.proto";
+import public "models/orange.proto";
 
 message Fruit {
 
     oneof fruit {
         Apple apple = 93029210;
-
         Banana banana = 322613405;
-
+        Orange orange = 471980499;
     }
-
 }

samples/client/petstore/csharp/generichost/net4.7/OneOf/.openapi-generator/FILES

@@ -7,6 +7,7 @@ docs/apis/DefaultApi.md
 docs/models/Apple.md
 docs/models/Banana.md
 docs/models/Fruit.md
+docs/models/Orange.md
 docs/scripts/git_push.ps1
 docs/scripts/git_push.sh
 src/Org.OpenAPITools.Test/Api/DependencyInjectionTests.cs
@@ -36,5 +37,6 @@ src/Org.OpenAPITools/Extensions/IServiceCollectionExtensions.cs
 src/Org.OpenAPITools/Model/Apple.cs
 src/Org.OpenAPITools/Model/Banana.cs
 src/Org.OpenAPITools/Model/Fruit.cs
+src/Org.OpenAPITools/Model/Orange.cs
 src/Org.OpenAPITools/Org.OpenAPITools.csproj
 src/Org.OpenAPITools/README.md

samples/client/petstore/csharp/generichost/net4.7/OneOf/api/openapi.yaml

@@ -22,6 +22,7 @@ components:
       oneOf:
       - $ref: "#/components/schemas/apple"
       - $ref: "#/components/schemas/banana"
+      - $ref: "#/components/schemas/orange"
       properties:
         color:
           type: string
@@ -38,4 +39,10 @@ components:
           type: number
       title: banana
       type: object
+    orange:
+      properties:
+        sweet:
+          type: boolean
+      title: orange
+      type: object