From e714a724343384e7e845ea2f1ccf879b132387ea Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Wed, 18 Sep 2024 11:46:20 +0200 Subject: [PATCH 01/16] Fix enum discriminator default value --- .../openapitools/codegen/DefaultCodegen.java | 59 +++++++++++-------- .../languages/CSharpClientCodegen.java | 6 +- .../CSharpClientCodegenTest.java | 27 +++++++++ .../codegen/java/JavaClientCodegenTest.java | 17 ++++++ .../3_0/enum_discriminator_inheritance.yaml | 38 ++++++++++++ 5 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index dbd6ee46d960..b1b6c850258c 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3270,11 +3270,11 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, * @param sc The Schema that may contain the discriminator * @param visitedSchemas An array list of visited schemas */ - private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { + private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); - Discriminator foundDisc = refSchema.getDiscriminator(); - if (foundDisc != null) { - return foundDisc; + Discriminator foundRefDisc = refSchema.getDiscriminator(); + if (foundRefDisc != null) { + return new DiscriminatorAndReferenceSchema(foundRefDisc, refSchema); } if (this.getLegacyDiscriminatorBehavior()) { @@ -3288,17 +3288,15 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis } visitedSchemas.add(refSchema); - Discriminator disc = new Discriminator(); if (ModelUtils.isComposedSchema(refSchema)) { Schema composedSchema = refSchema; if (composedSchema.getAllOf() != null) { // If our discriminator is in one of the allOf schemas break when we find it for (Object allOf : composedSchema.getAllOf()) { - foundDisc = recursiveGetDiscriminator((Schema) allOf, visitedSchemas); + DiscriminatorAndReferenceSchema foundDisc = + recursiveGetDiscriminator((Schema) allOf, visitedSchemas); if (foundDisc != null) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; + return foundDisc; } } } @@ -3307,6 +3305,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis Integer hasDiscriminatorCnt = 0; Integer hasNullTypeCnt = 0; Set discriminatorsPropNames = new HashSet<>(); + DiscriminatorAndReferenceSchema foundDisc = null; for (Object oneOf : composedSchema.getOneOf()) { if (ModelUtils.isNullType((Schema) oneOf)) { // The null type does not have a discriminator. Skip. @@ -3315,7 +3314,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis } foundDisc = recursiveGetDiscriminator((Schema) oneOf, visitedSchemas); if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.getPropertyName()); + discriminatorsPropNames.add(foundDisc.discriminator.getPropertyName()); hasDiscriminatorCnt++; } } @@ -3324,9 +3323,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis "oneOf schemas must have the same property name, but found " + String.join(", ", discriminatorsPropNames)); } if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getOneOf().size() && discriminatorsPropNames.size() == 1) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; + return foundDisc; } // If the scenario when oneOf has two children and one of them is the 'null' type, // there is no need for a discriminator. @@ -3336,6 +3333,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis Integer hasDiscriminatorCnt = 0; Integer hasNullTypeCnt = 0; Set discriminatorsPropNames = new HashSet<>(); + DiscriminatorAndReferenceSchema foundDisc = null; for (Object anyOf : composedSchema.getAnyOf()) { if (ModelUtils.isNullType((Schema) anyOf)) { // The null type does not have a discriminator. Skip. @@ -3344,7 +3342,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis } foundDisc = recursiveGetDiscriminator((Schema) anyOf, visitedSchemas); if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.getPropertyName()); + discriminatorsPropNames.add(foundDisc.discriminator.getPropertyName()); hasDiscriminatorCnt++; } } @@ -3353,9 +3351,7 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis "anyOf schemas must have the same property name, but found " + String.join(", ", discriminatorsPropNames)); } if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getAnyOf().size() && discriminatorsPropNames.size() == 1) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; + return foundDisc; } // If the scenario when anyOf has two children and one of them is the 'null' type, // there is no need for a discriminator. @@ -3364,6 +3360,16 @@ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList vis return null; } + private static class DiscriminatorAndReferenceSchema{ + private final Discriminator discriminator; + private final Schema referenceSchema; + + private DiscriminatorAndReferenceSchema(Discriminator discriminator, Schema referenceSchema) { + this.discriminator = discriminator; + this.referenceSchema = referenceSchema; + } + } + /** * This function is only used for composed schemas which have a discriminator * Process oneOf and anyOf models in a composed schema and adds them into @@ -3497,10 +3503,12 @@ protected List getAllOfDescendants(String thisSchemaName) { } protected CodegenDiscriminator createDiscriminator(String schemaName, Schema schema) { - Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, new ArrayList()); - if (sourceDiscriminator == null) { + DiscriminatorAndReferenceSchema discriminatorAndReferenceSchema = + recursiveGetDiscriminator(schema, new ArrayList()); + if (discriminatorAndReferenceSchema == null) { return null; } + Discriminator sourceDiscriminator = discriminatorAndReferenceSchema.discriminator; CodegenDiscriminator discriminator = new CodegenDiscriminator(); String discriminatorPropertyName = sourceDiscriminator.getPropertyName(); discriminator.setPropertyName(toVarName(discriminatorPropertyName)); @@ -3524,11 +3532,14 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch discriminator.setPropertyType(propertyType); // check to see if the discriminator property is an enum string - if (schema.getProperties() != null && - schema.getProperties().get(discriminatorPropertyName) instanceof StringSchema) { - StringSchema s = (StringSchema) schema.getProperties().get(discriminatorPropertyName); - if (s.getEnum() != null && !s.getEnum().isEmpty()) { // it's an enum string - discriminator.setIsEnum(true); + // If it is not in the schema it should be in the reference schema of sourceDiscriminator. + for (Schema s : List.of(schema, discriminatorAndReferenceSchema.referenceSchema)) { + if (s.getProperties() != null && + s.getProperties().get(discriminatorPropertyName) instanceof StringSchema) { + s = (StringSchema) s.getProperties().get(discriminatorPropertyName); + if (s.getEnum() != null && !s.getEnum().isEmpty()) { // it's an enum string + discriminator.setIsEnum(true); + } } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java index 3012973bd8c7..1190b42afdfe 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java @@ -440,7 +440,11 @@ public CodegenModel fromModel(String name, Schema model) { for (final CodegenProperty property : codegenModel.readWriteVars) { if (property.defaultValue == null && parentCodegenModel.discriminator != null && property.name.equals(parentCodegenModel.discriminator.getPropertyName())) { - property.defaultValue = "\"" + name + "\""; + if (parentCodegenModel.discriminator.getIsEnum()) { + property.defaultValue = toEnumDefaultValue(property, name); + } else { + property.defaultValue = "\"" + name + "\""; + } } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java index 4617ec5044ad..d7ad71f97770 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java @@ -140,4 +140,31 @@ public void test31specAdditionalPropertiesOfOneOf() throws IOException { assertFileContains(modelFile.toPath(), " Dictionary results = default(Dictionary"); } + + @Test + public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { + File output = Files.createTempDirectory("test").toFile().getCanonicalFile(); + output.deleteOnExit(); + final OpenAPI openAPI = TestUtils.parseFlattenSpec( + "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); + final DefaultGenerator defaultGenerator = new DefaultGenerator(); + final ClientOptInput clientOptInput = new ClientOptInput(); + clientOptInput.openAPI(openAPI); + CSharpClientCodegen cSharpClientCodegen = new CSharpClientCodegen(); + cSharpClientCodegen.setOutputDir(output.getAbsolutePath()); + cSharpClientCodegen.setAutosetConstants(true); + clientOptInput.config(cSharpClientCodegen); + defaultGenerator.opts(clientOptInput); + + Map files = defaultGenerator.generate().stream() + .collect(Collectors.toMap(File::getPath, Function.identity())); + + File modelFile = files.get(Paths + .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", "FooEntity.cs") + .toString() + ); + assertNotNull(modelFile); + assertFileContains(modelFile.toPath(), + "public FooEntity(TypeEnum type = TypeEnum.FooEntity) : base(type)"); + } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index abab35febc4b..faf250bc71c0 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2287,6 +2287,23 @@ public void testAllOfWithSinglePrimitiveTypeRef() { assertNull(files.get("pom.xml")); } + @Test public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { + final Path output = newTempFolder(); + final OpenAPI openAPI = TestUtils.parseFlattenSpec( + "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); + JavaClientCodegen codegen = new JavaClientCodegen(); + codegen.setOutputDir(output.toString()); + + Map files = new DefaultGenerator().opts(new ClientOptInput().openAPI(openAPI).config(codegen)) + .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); + + File fooEntityFile = files.get("FooEntity.java"); + assertNotNull(fooEntityFile); + assertThat(fooEntityFile).content() + .doesNotContain("this.type = this.getClass().getSimpleName();"); + System.out.println(Files.readString(fooEntityFile.toPath())); + } + @Test public void testRestTemplateHandleURIEnum() { String[] expectedInnerEnumLines = new String[] { diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml new file mode 100644 index 000000000000..2cc58321fd84 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -0,0 +1,38 @@ +openapi: 3.0.3 +info: + title: Test entity with enum discriminator + version: v1 +paths: + "/event": + get: + responses: + '200': + description: OK + content: + application/json: + schema: + "$ref": "#/components/schemas/Entity" +components: + schemas: + Entity: + required: + - type + type: object + properties: + type: + type: string + enum: + - FOO + - BAR + description: Entity + discriminator: + propertyName: type + mapping: + FOO: "#/components/schemas/FooEntity" + FooEntity: + required: + - type + type: object + description: Foo entity + allOf: + - "$ref": "#/components/schemas/Entity" From ab8c2b2648e03f3148b75450bb2051d076d05bba Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Wed, 18 Sep 2024 16:12:39 +0200 Subject: [PATCH 02/16] Remove system out call --- .../org/openapitools/codegen/java/JavaClientCodegenTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index faf250bc71c0..f95da8820c27 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2301,7 +2301,6 @@ public void testAllOfWithSinglePrimitiveTypeRef() { assertNotNull(fooEntityFile); assertThat(fooEntityFile).content() .doesNotContain("this.type = this.getClass().getSimpleName();"); - System.out.println(Files.readString(fooEntityFile.toPath())); } @Test From 76c7a1666105c4c63df0fdda4234aa5321f31222 Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 19 Sep 2024 16:29:15 +0200 Subject: [PATCH 03/16] Add case when discriminator type is ref --- .../openapitools/codegen/DefaultCodegen.java | 11 +++-- .../CSharpClientCodegenTest.java | 5 +-- .../codegen/java/JavaClientCodegenTest.java | 4 +- .../3_0/enum_discriminator_inheritance.yaml | 44 +++++++++++-------- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index b1b6c850258c..a278359087b9 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3534,10 +3534,13 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch // check to see if the discriminator property is an enum string // If it is not in the schema it should be in the reference schema of sourceDiscriminator. for (Schema s : List.of(schema, discriminatorAndReferenceSchema.referenceSchema)) { - if (s.getProperties() != null && - s.getProperties().get(discriminatorPropertyName) instanceof StringSchema) { - s = (StringSchema) s.getProperties().get(discriminatorPropertyName); - if (s.getEnum() != null && !s.getEnum().isEmpty()) { // it's an enum string + Optional discriminatorSchema = Optional.ofNullable(s.getProperties()).map(p -> (Schema) p.get(discriminatorPropertyName)); + if (discriminatorSchema.isPresent()) { + Optional refSchema = Optional.ofNullable(discriminatorSchema.get().get$ref()) + .map(ModelUtils::getSimpleRef) + .map(n -> ModelUtils.getSchema(openAPI, n)); + Schema correctSchema = refSchema.orElse(s); + if (correctSchema instanceof StringSchema && correctSchema.getEnum() != null && !correctSchema.getEnum().isEmpty()) { discriminator.setIsEnum(true); } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java index d7ad71f97770..dc17c13c33ae 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java @@ -160,11 +160,10 @@ public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { .collect(Collectors.toMap(File::getPath, Function.identity())); File modelFile = files.get(Paths - .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", "FooEntity.cs") + .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", "Cat.cs") .toString() ); assertNotNull(modelFile); - assertFileContains(modelFile.toPath(), - "public FooEntity(TypeEnum type = TypeEnum.FooEntity) : base(type)"); + assertFileContains(modelFile.toPath(), "AnimalType petType = AnimalType.Cat"); } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index f95da8820c27..b75097882b71 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2297,10 +2297,10 @@ public void testAllOfWithSinglePrimitiveTypeRef() { Map files = new DefaultGenerator().opts(new ClientOptInput().openAPI(openAPI).config(codegen)) .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); - File fooEntityFile = files.get("FooEntity.java"); + File fooEntityFile = files.get("Cat.java"); assertNotNull(fooEntityFile); assertThat(fooEntityFile).content() - .doesNotContain("this.type = this.getClass().getSimpleName();"); + .doesNotContain("this.petType = this.getClass().getSimpleName();"); } @Test diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index 2cc58321fd84..06f4579e6415 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -1,9 +1,9 @@ openapi: 3.0.3 info: - title: Test entity with enum discriminator + title: Test schema with enum discriminator version: v1 paths: - "/event": + "/animal": get: responses: '200': @@ -11,28 +11,34 @@ paths: content: application/json: schema: - "$ref": "#/components/schemas/Entity" + "$ref": "#/components/schemas/Animal" components: schemas: - Entity: - required: - - type + Animal: type: object properties: - type: - type: string - enum: - - FOO - - BAR - description: Entity + petType: + $ref: '#/components/schemas/AnimalType' discriminator: - propertyName: type - mapping: - FOO: "#/components/schemas/FooEntity" - FooEntity: + propertyName: petType required: - - type + - petType + AnimalType: + type: string + enum: + - Dog + - Cat + Cat: + type: object + allOf: + - $ref: '#/components/schemas/Animal' + properties: + meow: + type: string + Dog: type: object - description: Foo entity allOf: - - "$ref": "#/components/schemas/Entity" + - $ref: '#/components/schemas/Animal' + properties: + bark: + type: string From 5079f3a30199bac94911bacc3551e113f65478df Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 19 Sep 2024 16:40:36 +0200 Subject: [PATCH 04/16] Use correct schema --- .../src/main/java/org/openapitools/codegen/DefaultCodegen.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index a278359087b9..876003b979fa 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3539,7 +3539,7 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch Optional refSchema = Optional.ofNullable(discriminatorSchema.get().get$ref()) .map(ModelUtils::getSimpleRef) .map(n -> ModelUtils.getSchema(openAPI, n)); - Schema correctSchema = refSchema.orElse(s); + Schema correctSchema = refSchema.orElse(discriminatorSchema.get()); if (correctSchema instanceof StringSchema && correctSchema.getEnum() != null && !correctSchema.getEnum().isEmpty()) { discriminator.setIsEnum(true); } From 8fe6bc06059b970e326d30ea3bc81a58d61426fb Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Fri, 20 Sep 2024 09:46:16 +0200 Subject: [PATCH 05/16] Handle different use cases of mappings --- .../languages/CSharpClientCodegen.java | 16 +++++++++--- .../CSharpClientCodegenTest.java | 25 +++++++++++++------ .../3_0/enum_discriminator_inheritance.yaml | 13 +++++++++- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java index 1190b42afdfe..c8f2342619ae 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java @@ -439,9 +439,19 @@ public CodegenModel fromModel(String name, Schema model) { } for (final CodegenProperty property : codegenModel.readWriteVars) { - if (property.defaultValue == null && parentCodegenModel.discriminator != null && property.name.equals(parentCodegenModel.discriminator.getPropertyName())) { - if (parentCodegenModel.discriminator.getIsEnum()) { - property.defaultValue = toEnumDefaultValue(property, name); + CodegenDiscriminator discriminator = parentCodegenModel.discriminator; + if (property.defaultValue == null && discriminator != null && property.name.equals(discriminator.getPropertyName())) { + if (discriminator.getIsEnum()) { + String enumValue = name; + Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); + for (Map.Entry e : mapping.entrySet()) { + String schemaName = e.getValue().indexOf('/') < 0 ? e.getValue() : ModelUtils.getSimpleRef(e.getValue()); + if (name.equals(schemaName)) { + enumValue = e.getKey(); + break; + } + } + property.defaultValue = toEnumDefaultValue(property, enumValue); } else { property.defaultValue = "\"" + name + "\""; } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java index dc17c13c33ae..37775ba46f60 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java @@ -146,7 +146,7 @@ public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { File output = Files.createTempDirectory("test").toFile().getCanonicalFile(); output.deleteOnExit(); final OpenAPI openAPI = TestUtils.parseFlattenSpec( - "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); + "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); final DefaultGenerator defaultGenerator = new DefaultGenerator(); final ClientOptInput clientOptInput = new ClientOptInput(); clientOptInput.openAPI(openAPI); @@ -156,14 +156,23 @@ public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { clientOptInput.config(cSharpClientCodegen); defaultGenerator.opts(clientOptInput); - Map files = defaultGenerator.generate().stream() - .collect(Collectors.toMap(File::getPath, Function.identity())); + Map files = defaultGenerator.generate().stream() + .collect(Collectors.toMap(File::getPath, Function.identity())); - File modelFile = files.get(Paths - .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", "Cat.cs") - .toString() + Map expectedMapping = Map.of( + "Catty", "Cat", + "Lizzy", "Lizard", + "Dog", "Dog" ); - assertNotNull(modelFile); - assertFileContains(modelFile.toPath(), "AnimalType petType = AnimalType.Cat"); + for (Map.Entry e : expectedMapping.entrySet()) { + String modelName = e.getValue(); + String enumValue = e.getKey(); + File file = files.get(Paths + .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", modelName + ".cs") + .toString() + ); + assertNotNull(file, "Could not find file for model: " + modelName); + assertFileContains(file.toPath(), "AnimalType petType = AnimalType." + enumValue); + } } } diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index 06f4579e6415..3c5231950b50 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -21,13 +21,17 @@ components: $ref: '#/components/schemas/AnimalType' discriminator: propertyName: petType + # Mapping with implicit (Dog), explicit ref (Catty -> Cat), and explicit schema name (Lizzy -> Lizard) + mapping: + Catty: '#/components/schemas/Cat' + Lizzy: 'Lizard' required: - petType AnimalType: type: string enum: - Dog - - Cat + - Catty Cat: type: object allOf: @@ -42,3 +46,10 @@ components: properties: bark: type: string + Lizard: + type: object + allOf: + - $ref: '#/components/schemas/Animal' + properties: + lovesRocks: + type: string From 2d112dae582579d315d9d03c04a1e2c4fe026d47 Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Fri, 20 Sep 2024 09:54:00 +0200 Subject: [PATCH 06/16] Add missing enum type Lizzy --- .../src/test/resources/3_0/enum_discriminator_inheritance.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index 3c5231950b50..666f0c470c69 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -32,6 +32,7 @@ components: enum: - Dog - Catty + - Lizzy Cat: type: object allOf: From e4781713b4ea196deeedf3aa2cff5def25aa122a Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Fri, 20 Sep 2024 15:05:40 +0200 Subject: [PATCH 07/16] Make it more robust --- .../openapitools/codegen/DefaultCodegen.java | 87 +++++++++--------- .../codegen/utils/ModelUtils.java | 11 ++- .../CSharpClientCodegenTest.java | 22 +++-- .../codegen/java/JavaClientCodegenTest.java | 18 +++- .../3_0/enum_discriminator_inheritance.yaml | 90 +++++++++++++++++-- 5 files changed, 164 insertions(+), 64 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 876003b979fa..0bc6c54d5c27 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3270,11 +3270,11 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, * @param sc The Schema that may contain the discriminator * @param visitedSchemas An array list of visited schemas */ - private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { + private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); - Discriminator foundRefDisc = refSchema.getDiscriminator(); - if (foundRefDisc != null) { - return new DiscriminatorAndReferenceSchema(foundRefDisc, refSchema); + Discriminator foundDisc = refSchema.getDiscriminator(); + if (foundDisc != null) { + return foundDisc; } if (this.getLegacyDiscriminatorBehavior()) { @@ -3288,15 +3288,17 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr } visitedSchemas.add(refSchema); + Discriminator disc = new Discriminator(); if (ModelUtils.isComposedSchema(refSchema)) { Schema composedSchema = refSchema; if (composedSchema.getAllOf() != null) { // If our discriminator is in one of the allOf schemas break when we find it for (Object allOf : composedSchema.getAllOf()) { - DiscriminatorAndReferenceSchema foundDisc = - recursiveGetDiscriminator((Schema) allOf, visitedSchemas); + foundDisc = recursiveGetDiscriminator((Schema) allOf, visitedSchemas); if (foundDisc != null) { - return foundDisc; + disc.setPropertyName(foundDisc.getPropertyName()); + disc.setMapping(foundDisc.getMapping()); + return disc; } } } @@ -3305,7 +3307,6 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr Integer hasDiscriminatorCnt = 0; Integer hasNullTypeCnt = 0; Set discriminatorsPropNames = new HashSet<>(); - DiscriminatorAndReferenceSchema foundDisc = null; for (Object oneOf : composedSchema.getOneOf()) { if (ModelUtils.isNullType((Schema) oneOf)) { // The null type does not have a discriminator. Skip. @@ -3314,7 +3315,7 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr } foundDisc = recursiveGetDiscriminator((Schema) oneOf, visitedSchemas); if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.discriminator.getPropertyName()); + discriminatorsPropNames.add(foundDisc.getPropertyName()); hasDiscriminatorCnt++; } } @@ -3323,7 +3324,9 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr "oneOf schemas must have the same property name, but found " + String.join(", ", discriminatorsPropNames)); } if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getOneOf().size() && discriminatorsPropNames.size() == 1) { - return foundDisc; + disc.setPropertyName(foundDisc.getPropertyName()); + disc.setMapping(foundDisc.getMapping()); + return disc; } // If the scenario when oneOf has two children and one of them is the 'null' type, // there is no need for a discriminator. @@ -3333,7 +3336,6 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr Integer hasDiscriminatorCnt = 0; Integer hasNullTypeCnt = 0; Set discriminatorsPropNames = new HashSet<>(); - DiscriminatorAndReferenceSchema foundDisc = null; for (Object anyOf : composedSchema.getAnyOf()) { if (ModelUtils.isNullType((Schema) anyOf)) { // The null type does not have a discriminator. Skip. @@ -3342,7 +3344,7 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr } foundDisc = recursiveGetDiscriminator((Schema) anyOf, visitedSchemas); if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.discriminator.getPropertyName()); + discriminatorsPropNames.add(foundDisc.getPropertyName()); hasDiscriminatorCnt++; } } @@ -3351,7 +3353,9 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr "anyOf schemas must have the same property name, but found " + String.join(", ", discriminatorsPropNames)); } if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getAnyOf().size() && discriminatorsPropNames.size() == 1) { - return foundDisc; + disc.setPropertyName(foundDisc.getPropertyName()); + disc.setMapping(foundDisc.getMapping()); + return disc; } // If the scenario when anyOf has two children and one of them is the 'null' type, // there is no need for a discriminator. @@ -3360,16 +3364,6 @@ private DiscriminatorAndReferenceSchema recursiveGetDiscriminator(Schema sc, Arr return null; } - private static class DiscriminatorAndReferenceSchema{ - private final Discriminator discriminator; - private final Schema referenceSchema; - - private DiscriminatorAndReferenceSchema(Discriminator discriminator, Schema referenceSchema) { - this.discriminator = discriminator; - this.referenceSchema = referenceSchema; - } - } - /** * This function is only used for composed schemas which have a discriminator * Process oneOf and anyOf models in a composed schema and adds them into @@ -3503,12 +3497,10 @@ protected List getAllOfDescendants(String thisSchemaName) { } protected CodegenDiscriminator createDiscriminator(String schemaName, Schema schema) { - DiscriminatorAndReferenceSchema discriminatorAndReferenceSchema = - recursiveGetDiscriminator(schema, new ArrayList()); - if (discriminatorAndReferenceSchema == null) { + Discriminator sourceDiscriminator = recursiveGetDiscriminator(schema, new ArrayList()); + if (sourceDiscriminator == null) { return null; } - Discriminator sourceDiscriminator = discriminatorAndReferenceSchema.discriminator; CodegenDiscriminator discriminator = new CodegenDiscriminator(); String discriminatorPropertyName = sourceDiscriminator.getPropertyName(); discriminator.setPropertyName(toVarName(discriminatorPropertyName)); @@ -3531,21 +3523,6 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch .orElseGet(() -> typeMapping.get("string")); discriminator.setPropertyType(propertyType); - // check to see if the discriminator property is an enum string - // If it is not in the schema it should be in the reference schema of sourceDiscriminator. - for (Schema s : List.of(schema, discriminatorAndReferenceSchema.referenceSchema)) { - Optional discriminatorSchema = Optional.ofNullable(s.getProperties()).map(p -> (Schema) p.get(discriminatorPropertyName)); - if (discriminatorSchema.isPresent()) { - Optional refSchema = Optional.ofNullable(discriminatorSchema.get().get$ref()) - .map(ModelUtils::getSimpleRef) - .map(n -> ModelUtils.getSchema(openAPI, n)); - Schema correctSchema = refSchema.orElse(discriminatorSchema.get()); - if (correctSchema instanceof StringSchema && correctSchema.getEnum() != null && !correctSchema.getEnum().isEmpty()) { - discriminator.setIsEnum(true); - } - } - } - discriminator.setMapping(sourceDiscriminator.getMapping()); List uniqueDescendants = new ArrayList<>(); if (sourceDiscriminator.getMapping() != null && !sourceDiscriminator.getMapping().isEmpty()) { @@ -3597,6 +3574,32 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch } discriminator.getMappedModels().addAll(uniqueDescendants); + // check current schema, all parents and descendants to see if the discriminator property is an enum string + List schemasToCheckForEnumDiscriminator = new ArrayList<>(); + schemasToCheckForEnumDiscriminator.add(schema); + if (ModelUtils.isComposedSchema(schema)) { + ModelUtils.getAllParentsName(schema, openAPI.getComponents().getSchemas(), true).stream() + .map(n -> ModelUtils.getSchema(openAPI, n)) + .forEach(schemasToCheckForEnumDiscriminator::add); + } + uniqueDescendants.stream().map(MappedModel::getModelName) + .map(n->ModelUtils.getSchema(openAPI, n)) + .forEach(schemasToCheckForEnumDiscriminator::add); + for (Schema schemaToCheck : schemasToCheckForEnumDiscriminator) { + if (schemaToCheck == null) { + continue; + } + boolean hasDiscriminatorEnum = Optional.ofNullable(schemaToCheck.getProperties()) + .map(p -> (Schema) p.get(discriminatorPropertyName)) + .map(s -> ModelUtils.getReferencedSchema(openAPI, s)) + .filter(s -> s instanceof StringSchema && s.getEnum() != null && !s.getEnum().isEmpty()) + .isPresent(); + if (hasDiscriminatorEnum) { + discriminator.setIsEnum(true); + break; + } + } + return discriminator; } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java index b8bf2bcd669a..94cf08327ddc 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java @@ -1595,6 +1595,11 @@ public static String getParentName(Schema composedSchema, Map al * @return the name of the parent model */ public static List getAllParentsName(Schema composedSchema, Map allSchemas, boolean includeAncestors) { + return getAllParentsName(composedSchema, allSchemas, includeAncestors, new HashSet<>()); + } + + public static List getAllParentsName( + Schema composedSchema, Map allSchemas, boolean includeAncestors, Set seenNames) { List interfaces = getInterfaces(composedSchema); List names = new ArrayList(); @@ -1603,6 +1608,10 @@ public static List getAllParentsName(Schema composedSchema, Map getAllParentsName(Schema composedSchema, Map files = defaultGenerator.generate().stream() .collect(Collectors.toMap(File::getPath, Function.identity())); - Map expectedMapping = Map.of( - "Catty", "Cat", - "Lizzy", "Lizard", - "Dog", "Dog" + Map expectedContents = Map.of( + "Cat", "PetTypeEnum petType = PetTypeEnum.Catty", + "Dog", "PetTypeEnum petType = PetTypeEnum.Dog", + "Gecko", "PetTypeEnum petType = PetTypeEnum.Gecko", + "Chameleon", "PetTypeEnum petType = PetTypeEnum.Camo", + "MiniVan", "CarType carType = CarType.MiniVan", + "CargoVan", "CarType carType = CarType.CargoVan", + "SUV", "CarType carType = CarType.SUV", + "Truck", "CarType carType = CarType.Truck" + ); - for (Map.Entry e : expectedMapping.entrySet()) { - String modelName = e.getValue(); - String enumValue = e.getKey(); + for (Map.Entry e : expectedContents.entrySet()) { + String modelName = e.getKey(); + String expectedContent = e.getValue(); File file = files.get(Paths .get(output.getAbsolutePath(), "src", "Org.OpenAPITools", "Model", modelName + ".cs") .toString() ); assertNotNull(file, "Could not find file for model: " + modelName); - assertFileContains(file.toPath(), "AnimalType petType = AnimalType." + enumValue); + assertFileContains(file.toPath(), expectedContent); } } } diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index b75097882b71..4538414f9bdf 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2297,10 +2297,20 @@ public void testAllOfWithSinglePrimitiveTypeRef() { Map files = new DefaultGenerator().opts(new ClientOptInput().openAPI(openAPI).config(codegen)) .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); - File fooEntityFile = files.get("Cat.java"); - assertNotNull(fooEntityFile); - assertThat(fooEntityFile).content() - .doesNotContain("this.petType = this.getClass().getSimpleName();"); + List entities = List.of( + "Cat", + "Dog", + "Gecko", + "Chameleon", + "MiniVan", + "CargoVan", + "SUV", + "Truck"); + for (String entity : entities) { + File entityFile = files.get(entity + ".java"); + assertNotNull(entityFile); + assertThat(entityFile).content().doesNotContain("Type = this.getClass().getSimpleName();"); + } } @Test diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index 666f0c470c69..e2a05d326869 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -17,22 +17,22 @@ components: Animal: type: object properties: + # Inline enum type petType: - $ref: '#/components/schemas/AnimalType' + type: string + enum: + - Dog + - Catty + - Gecko + - Camo discriminator: propertyName: petType - # Mapping with implicit (Dog), explicit ref (Catty -> Cat), and explicit schema name (Lizzy -> Lizard) + # Mapping with implicit (Dog, Gecko), explicit ref (Catty -> Cat), and explicit schema name (Camo -> Chameleon) mapping: Catty: '#/components/schemas/Cat' - Lizzy: 'Lizard' + Camo: 'Chameleon' required: - petType - AnimalType: - type: string - enum: - - Dog - - Catty - - Lizzy Cat: type: object allOf: @@ -48,9 +48,81 @@ components: bark: type: string Lizard: + oneOf: + - $ref: '#/components/schemas/Gecko' + - $ref: '#/components/schemas/Chameleon' + Gecko: type: object allOf: - $ref: '#/components/schemas/Animal' properties: lovesRocks: type: string + Chameleon: + type: object + allOf: + - $ref: '#/components/schemas/Animal' + properties: + currentColor: + type: string + # Car inheritance tree: Car -> Truck -> SUV + # Car -> Van -> MiniVan + # Car -> Van -> CargoVan + Car: + type: object + required: + - carType + # Discriminator carType not defined in Car properties, but in child properties + discriminator: + propertyName: carType + CarType: + type: string + enum: + - Truck + - SUV + - Sedan + - MiniVan + - CargoVan + Truck: + type: object + allOf: + - $ref: '#/components/schemas/Car' + properties: + carType: + $ref: '#/components/schemas/CarType' + required: + - carType + SUV: + type: object + # SUV gets its discriminator property from Truck + allOf: + - $ref: '#/components/schemas/Truck' + Sedan: + type: object + allOf: + - $ref: '#/components/schemas/Car' + properties: + carType: + $ref: '#/components/schemas/CarType' + Van: + oneOf: + - $ref: '#/components/schemas/MiniVan' + - $ref: '#/components/schemas/CargoVan' + MiniVan: + type: object + allOf: + - $ref: '#/components/schemas/Car' + properties: + carType: + $ref: '#/components/schemas/CarType' + required: + - carType + CargoVan: + type: object + allOf: + - $ref: '#/components/schemas/Car' + properties: + carType: + $ref: '#/components/schemas/CarType' + required: + - carType From 452e95b4939a6d0cd4fe203f6c9180e733a9919f Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Fri, 20 Sep 2024 16:04:20 +0200 Subject: [PATCH 08/16] Add missing test for Sedan --- .../codegen/csharpnetcore/CSharpClientCodegenTest.java | 3 ++- .../org/openapitools/codegen/java/JavaClientCodegenTest.java | 3 ++- .../src/test/resources/3_0/enum_discriminator_inheritance.yaml | 3 +++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java index fb2b89d63bab..2fe1bdb5e641 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java @@ -167,7 +167,8 @@ public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { "MiniVan", "CarType carType = CarType.MiniVan", "CargoVan", "CarType carType = CarType.CargoVan", "SUV", "CarType carType = CarType.SUV", - "Truck", "CarType carType = CarType.Truck" + "Truck", "CarType carType = CarType.Truck", + "Sedan", "CarType carType = CarType.Sedan" ); for (Map.Entry e : expectedContents.entrySet()) { diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index 4538414f9bdf..13f94f89c828 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2305,7 +2305,8 @@ public void testAllOfWithSinglePrimitiveTypeRef() { "MiniVan", "CargoVan", "SUV", - "Truck"); + "Truck", + "Sedan"); for (String entity : entities) { File entityFile = files.get(entity + ".java"); assertNotNull(entityFile); diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index e2a05d326869..e211a96edcac 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -68,6 +68,7 @@ components: # Car inheritance tree: Car -> Truck -> SUV # Car -> Van -> MiniVan # Car -> Van -> CargoVan + # Car -> Sedan Car: type: object required: @@ -101,6 +102,8 @@ components: type: object allOf: - $ref: '#/components/schemas/Car' + required: + - carType properties: carType: $ref: '#/components/schemas/CarType' From 9c0a126706f8b9e54c78984b3392cbcd2412d10a Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 26 Sep 2024 15:41:49 +0200 Subject: [PATCH 09/16] Refactor some code to make it cleaner --- .../openapitools/codegen/DefaultCodegen.java | 42 ++++++++----------- .../languages/CSharpClientCodegen.java | 27 ++++++------ .../codegen/utils/ModelUtils.java | 3 +- .../codegen/java/JavaClientCodegenTest.java | 7 ++-- 4 files changed, 38 insertions(+), 41 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 0bc6c54d5c27..324bdf4b7daf 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3575,30 +3575,24 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch discriminator.getMappedModels().addAll(uniqueDescendants); // check current schema, all parents and descendants to see if the discriminator property is an enum string - List schemasToCheckForEnumDiscriminator = new ArrayList<>(); - schemasToCheckForEnumDiscriminator.add(schema); - if (ModelUtils.isComposedSchema(schema)) { - ModelUtils.getAllParentsName(schema, openAPI.getComponents().getSchemas(), true).stream() - .map(n -> ModelUtils.getSchema(openAPI, n)) - .forEach(schemasToCheckForEnumDiscriminator::add); - } - uniqueDescendants.stream().map(MappedModel::getModelName) - .map(n->ModelUtils.getSchema(openAPI, n)) - .forEach(schemasToCheckForEnumDiscriminator::add); - for (Schema schemaToCheck : schemasToCheckForEnumDiscriminator) { - if (schemaToCheck == null) { - continue; - } - boolean hasDiscriminatorEnum = Optional.ofNullable(schemaToCheck.getProperties()) - .map(p -> (Schema) p.get(discriminatorPropertyName)) - .map(s -> ModelUtils.getReferencedSchema(openAPI, s)) - .filter(s -> s instanceof StringSchema && s.getEnum() != null && !s.getEnum().isEmpty()) - .isPresent(); - if (hasDiscriminatorEnum) { - discriminator.setIsEnum(true); - break; - } - } + Stream schemasToCheckForEnumDiscriminator = Stream.concat( + Stream.of(schema), + Stream.concat( + ModelUtils.isComposedSchema(schema) + ? ModelUtils.getAllParentsName(schema, openAPI.getComponents().getSchemas(), true).stream() + : Stream.of(), + uniqueDescendants.stream().map(MappedModel::getModelName) + ).flatMap(s -> Optional.ofNullable(ModelUtils.getSchema(openAPI, s)).stream()) + ); + + boolean isEnum = schemasToCheckForEnumDiscriminator.anyMatch(s -> Optional + .ofNullable(s.getProperties()) + .map(p -> (Schema) p.get(discriminatorPropertyName)) + .map(s1 -> ModelUtils.getReferencedSchema(openAPI, s1)) + .filter(s1 -> s1 instanceof StringSchema && s1.getEnum() != null && !s1.getEnum().isEmpty()) + .isPresent() + ); + discriminator.setIsEnum(isEnum); return discriminator; } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java index c8f2342619ae..a0e902f31bd9 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java @@ -440,21 +440,22 @@ public CodegenModel fromModel(String name, Schema model) { for (final CodegenProperty property : codegenModel.readWriteVars) { CodegenDiscriminator discriminator = parentCodegenModel.discriminator; - if (property.defaultValue == null && discriminator != null && property.name.equals(discriminator.getPropertyName())) { - if (discriminator.getIsEnum()) { - String enumValue = name; - Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); - for (Map.Entry e : mapping.entrySet()) { - String schemaName = e.getValue().indexOf('/') < 0 ? e.getValue() : ModelUtils.getSimpleRef(e.getValue()); - if (name.equals(schemaName)) { - enumValue = e.getKey(); - break; - } + if (property.defaultValue != null || discriminator == null || !property.name.equals(discriminator.getPropertyName())) { + continue; + } + if (discriminator.getIsEnum()) { + String enumValue = name; + Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); + for (Map.Entry e : mapping.entrySet()) { + String schemaName = e.getValue().indexOf('/') < 0 ? e.getValue() : ModelUtils.getSimpleRef(e.getValue()); + if (name.equals(schemaName)) { + enumValue = e.getKey(); + break; } - property.defaultValue = toEnumDefaultValue(property, enumValue); - } else { - property.defaultValue = "\"" + name + "\""; } + property.defaultValue = toEnumDefaultValue(property, enumValue); + } else { + property.defaultValue = "\"" + name + "\""; } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java index 94cf08327ddc..f869e0edfc07 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java @@ -1598,7 +1598,8 @@ public static List getAllParentsName(Schema composedSchema, Map()); } - public static List getAllParentsName( + // Use a set of seen names to avoid infinite recursion + private static List getAllParentsName( Schema composedSchema, Map allSchemas, boolean includeAncestors, Set seenNames) { List interfaces = getInterfaces(composedSchema); List names = new ArrayList(); diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index 13f94f89c828..8ffb0009ed82 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2287,15 +2287,16 @@ public void testAllOfWithSinglePrimitiveTypeRef() { assertNull(files.get("pom.xml")); } - @Test public void testEnumDiscriminatorDefaultValueIsNotString() throws IOException { + @Test + public void testEnumDiscriminatorDefaultValueIsNotString() { final Path output = newTempFolder(); final OpenAPI openAPI = TestUtils.parseFlattenSpec( - "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); + "src/test/resources/3_0/enum_discriminator_inheritance.yaml"); JavaClientCodegen codegen = new JavaClientCodegen(); codegen.setOutputDir(output.toString()); Map files = new DefaultGenerator().opts(new ClientOptInput().openAPI(openAPI).config(codegen)) - .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); + .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); List entities = List.of( "Cat", From 3f2a342ae64e454306dfa7937fbc7f941e54456e Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Tue, 22 Oct 2024 10:56:11 +0200 Subject: [PATCH 10/16] Initialize discriminator enum field --- .../openapitools/codegen/DefaultCodegen.java | 32 +++++++++++++++++++ .../languages/AbstractJavaCodegen.java | 1 + .../languages/CSharpClientCodegen.java | 18 ++--------- .../Java/libraries/okhttp-gson/pojo.mustache | 5 +++ .../codegen/java/JavaClientCodegenTest.java | 29 ++++++++++------- .../client/model/EnumStringDiscriminator.java | 1 + 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 324bdf4b7daf..437279747a23 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3080,6 +3080,7 @@ public CodegenModel fromModel(String name, Schema schema) { listOLists.add(m.requiredVars); listOLists.add(m.vars); listOLists.add(m.allVars); + listOLists.add(m.readWriteVars); for (List theseVars : listOLists) { for (CodegenProperty requiredVar : theseVars) { if (discPropName.equals(requiredVar.baseName)) { @@ -3113,6 +3114,37 @@ public CodegenModel fromModel(String name, Schema schema) { return m; } + protected static void setEnumDiscriminatorDefaultValue(CodegenModel model) { + if (model.discriminator == null) { + return; + } + String discPropName = model.discriminator.getPropertyBaseName(); + Stream.of(model.requiredVars, model.vars, model.allVars, model.readWriteVars) + .flatMap(List::stream) + .filter(v -> discPropName.equals(v.baseName)) + .forEach(v -> v.defaultValue = getEnumValueForProperty(model.schemaName, model.discriminator, v)); + } + + protected static String getEnumValueForProperty( + String modelName, CodegenDiscriminator discriminator, CodegenProperty var) { + if (!discriminator.getIsEnum() && !var.isEnum) { + return null; + } + Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); + for (Map.Entry e : mapping.entrySet()) { + String schemaName = e.getValue().indexOf('/') < 0 ? e.getValue() : ModelUtils.getSimpleRef(e.getValue()); + if (modelName.equals(schemaName)) { + return e.getKey(); + } + } + Object values = var.allowableValues.get("values"); + if (!(values instanceof List)) { + return null; + } + List valueList = (List) values; + return valueList.stream().filter(o -> o.equals(modelName)).map(o -> (String) o).findAny().orElse(null); + } + protected void SortModelPropertiesByRequiredFlag(CodegenModel model) { Comparator comparator = new Comparator() { @Override diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java index 6ae58f6bff4c..feeb814ee946 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java @@ -1700,6 +1700,7 @@ public CodegenModel fromModel(String name, Schema model) { // additional import for different cases addAdditionalImports(codegenModel, codegenModel.getComposedSchemas()); + setEnumDiscriminatorDefaultValue(codegenModel); return codegenModel; } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java index a0e902f31bd9..eb24d4e3ddf0 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CSharpClientCodegen.java @@ -425,6 +425,7 @@ public String apiTestFileFolder() { public CodegenModel fromModel(String name, Schema model) { Map allDefinitions = ModelUtils.getSchemas(this.openAPI); CodegenModel codegenModel = super.fromModel(name, model); + setEnumDiscriminatorDefaultValue(codegenModel); if (allDefinitions != null && codegenModel != null && codegenModel.parent != null) { final Schema parentModel = allDefinitions.get(toModelName(codegenModel.parent)); if (parentModel != null) { @@ -439,22 +440,7 @@ public CodegenModel fromModel(String name, Schema model) { } for (final CodegenProperty property : codegenModel.readWriteVars) { - CodegenDiscriminator discriminator = parentCodegenModel.discriminator; - if (property.defaultValue != null || discriminator == null || !property.name.equals(discriminator.getPropertyName())) { - continue; - } - if (discriminator.getIsEnum()) { - String enumValue = name; - Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); - for (Map.Entry e : mapping.entrySet()) { - String schemaName = e.getValue().indexOf('/') < 0 ? e.getValue() : ModelUtils.getSimpleRef(e.getValue()); - if (name.equals(schemaName)) { - enumValue = e.getKey(); - break; - } - } - property.defaultValue = toEnumDefaultValue(property, enumValue); - } else { + if (property.defaultValue == null && parentCodegenModel.discriminator != null && property.name.equals(parentCodegenModel.discriminator.getPropertyName())) { property.defaultValue = "\"" + name + "\""; } } diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache index 0a32ef099070..e8d3bbdb2778 100644 --- a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache +++ b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache @@ -80,6 +80,11 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens {{/parcelableModel}} {{/parent}} {{#discriminator}} + {{#discriminator.isEnum}} +{{#readWriteVars}}{{#isDiscriminator}}{{#defaultValue}} + this.{{name}} = {{defaultValue}}; +{{/defaultValue}}{{/isDiscriminator}}{{/readWriteVars}} + {{/discriminator.isEnum}} {{^discriminator.isEnum}} this.{{{discriminatorName}}} = this.getClass().getSimpleName(); {{/discriminator.isEnum}} diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java index 8ffb0009ed82..4a0fda4687b7 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java @@ -2298,20 +2298,25 @@ public void testEnumDiscriminatorDefaultValueIsNotString() { Map files = new DefaultGenerator().opts(new ClientOptInput().openAPI(openAPI).config(codegen)) .generate().stream().collect(Collectors.toMap(File::getName, Function.identity())); - List entities = List.of( - "Cat", - "Dog", - "Gecko", - "Chameleon", - "MiniVan", - "CargoVan", - "SUV", - "Truck", - "Sedan"); - for (String entity : entities) { - File entityFile = files.get(entity + ".java"); + Map expectedContents = Map.of( + "Cat", "this.petType = PetTypeEnum.CATTY", + "Dog", "this.petType = PetTypeEnum.DOG", + "Gecko", "this.petType = PetTypeEnum.GECKO", + "Chameleon", "this.petType = PetTypeEnum.CAMO", + "MiniVan", "this.carType = CarType.MINI_VAN", + "CargoVan", "this.carType = CarType.CARGO_VAN", + "SUV", "this.carType = CarType.SUV", + "Truck", "this.carType = CarType.TRUCK", + "Sedan", "this.carType = CarType.SEDAN" + + ); + for (Map.Entry e : expectedContents.entrySet()) { + String modelName = e.getKey(); + String expectedContent = e.getValue(); + File entityFile = files.get(modelName + ".java"); assertNotNull(entityFile); assertThat(entityFile).content().doesNotContain("Type = this.getClass().getSimpleName();"); + assertThat(entityFile).content().contains(expectedContent); } } diff --git a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/EnumStringDiscriminator.java b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/EnumStringDiscriminator.java index c2a94ec0d1de..d1be014aba93 100644 --- a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/EnumStringDiscriminator.java +++ b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/EnumStringDiscriminator.java @@ -107,6 +107,7 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti protected EnumStrTypeEnum enumStrType; public EnumStringDiscriminator() { + } public EnumStringDiscriminator enumStrType(EnumStrTypeEnum enumStrType) { From 8c3434a5e013614d676319c18438480a5b66a6ce Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 24 Oct 2024 12:43:20 +0200 Subject: [PATCH 11/16] Don't override existing default value --- .../main/java/org/openapitools/codegen/DefaultCodegen.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 437279747a23..fb8042268d4f 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3128,7 +3128,7 @@ protected static void setEnumDiscriminatorDefaultValue(CodegenModel model) { protected static String getEnumValueForProperty( String modelName, CodegenDiscriminator discriminator, CodegenProperty var) { if (!discriminator.getIsEnum() && !var.isEnum) { - return null; + return var.defaultValue; } Map mapping = Optional.ofNullable(discriminator.getMapping()).orElseGet(Collections::emptyMap); for (Map.Entry e : mapping.entrySet()) { @@ -3139,10 +3139,10 @@ protected static String getEnumValueForProperty( } Object values = var.allowableValues.get("values"); if (!(values instanceof List)) { - return null; + return var.defaultValue; } List valueList = (List) values; - return valueList.stream().filter(o -> o.equals(modelName)).map(o -> (String) o).findAny().orElse(null); + return valueList.stream().filter(o -> o.equals(modelName)).map(o -> (String) o).findAny().orElse(var.defaultValue); } protected void SortModelPropertiesByRequiredFlag(CodegenModel model) { From 398bad431de062a26b3b996eb628c4aa5962636f Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 24 Oct 2024 13:56:59 +0200 Subject: [PATCH 12/16] Fix issue with finding discriminators --- .../openapitools/codegen/DefaultCodegen.java | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index fb8042268d4f..ace913f3098e 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3215,15 +3215,17 @@ protected void setAddProps(Schema schema, IJsonSchemaValidationProperties proper * @param visitedSchemas A set of visited schema names */ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set visitedSchemas) { - if (visitedSchemas.contains(composedSchemaName)) { // recursive schema definition found + Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); + // Identify nameless schemas by their toString. + String schemaKey = Optional.ofNullable(refSchema.getName()).orElseGet(refSchema::toString); + if (visitedSchemas.contains(schemaKey)) { // recursive schema definition found return null; } else { - visitedSchemas.add(composedSchemaName); + visitedSchemas.add(schemaKey); } - Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); if (refSchema.getProperties() != null && refSchema.getProperties().get(discPropName) != null) { - Schema discSchema = (Schema) refSchema.getProperties().get(discPropName); + Schema discSchema = ModelUtils.getReferencedSchema(openAPI, (Schema)refSchema.getProperties().get(discPropName)); CodegenProperty cp = new CodegenProperty(); if (ModelUtils.isStringSchema(discSchema)) { cp.isString = true; @@ -3232,6 +3234,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, if (refSchema.getRequired() != null && refSchema.getRequired().contains(discPropName)) { cp.setRequired(true); } + cp.setIsEnum(discSchema.getEnum() != null && !discSchema.getEnum().isEmpty()); return cp; } if (ModelUtils.isComposedSchema(refSchema)) { @@ -3606,24 +3609,10 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch } discriminator.getMappedModels().addAll(uniqueDescendants); - // check current schema, all parents and descendants to see if the discriminator property is an enum string - Stream schemasToCheckForEnumDiscriminator = Stream.concat( - Stream.of(schema), - Stream.concat( - ModelUtils.isComposedSchema(schema) - ? ModelUtils.getAllParentsName(schema, openAPI.getComponents().getSchemas(), true).stream() - : Stream.of(), - uniqueDescendants.stream().map(MappedModel::getModelName) - ).flatMap(s -> Optional.ofNullable(ModelUtils.getSchema(openAPI, s)).stream()) - ); - - boolean isEnum = schemasToCheckForEnumDiscriminator.anyMatch(s -> Optional - .ofNullable(s.getProperties()) - .map(p -> (Schema) p.get(discriminatorPropertyName)) - .map(s1 -> ModelUtils.getReferencedSchema(openAPI, s1)) - .filter(s1 -> s1 instanceof StringSchema && s1.getEnum() != null && !s1.getEnum().isEmpty()) - .isPresent() - ); + boolean isEnum = Optional + .ofNullable(discriminatorFound(schemaName, schema, discriminatorPropertyName, new TreeSet<>())) + .map(CodegenProperty::getIsEnum) + .orElse(false); discriminator.setIsEnum(isEnum); return discriminator; From 9942e93e6c07f9d5c62ba60c92ce487ec372c8a6 Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 24 Oct 2024 14:34:17 +0200 Subject: [PATCH 13/16] Move setIsEnum back to its original location --- .../org/openapitools/codegen/DefaultCodegen.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index ace913f3098e..a2ef3c01e711 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3558,6 +3558,13 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch .orElseGet(() -> typeMapping.get("string")); discriminator.setPropertyType(propertyType); + // check to see if the discriminator property is an enum string + boolean isEnum = Optional + .ofNullable(discriminatorFound(schemaName, schema, discriminatorPropertyName, new TreeSet<>())) + .map(CodegenProperty::getIsEnum) + .orElse(false); + discriminator.setIsEnum(isEnum); + discriminator.setMapping(sourceDiscriminator.getMapping()); List uniqueDescendants = new ArrayList<>(); if (sourceDiscriminator.getMapping() != null && !sourceDiscriminator.getMapping().isEmpty()) { @@ -3609,12 +3616,6 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch } discriminator.getMappedModels().addAll(uniqueDescendants); - boolean isEnum = Optional - .ofNullable(discriminatorFound(schemaName, schema, discriminatorPropertyName, new TreeSet<>())) - .map(CodegenProperty::getIsEnum) - .orElse(false); - discriminator.setIsEnum(isEnum); - return discriminator; } From aa78c388707521a8991a571433c89c9067d40c4e Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 24 Oct 2024 15:24:28 +0200 Subject: [PATCH 14/16] Be smarter about figuring out the model name --- .../openapitools/codegen/DefaultCodegen.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index a2ef3c01e711..7f337d01f772 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3216,12 +3216,15 @@ protected void setAddProps(Schema schema, IJsonSchemaValidationProperties proper */ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set visitedSchemas) { Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); - // Identify nameless schemas by their toString. - String schemaKey = Optional.ofNullable(refSchema.getName()).orElseGet(refSchema::toString); - if (visitedSchemas.contains(schemaKey)) { // recursive schema definition found + ModelUtils.getSimpleRef(sc.get$ref()); + String schemaName = Optional.ofNullable(composedSchemaName) + .or(() -> Optional.ofNullable(refSchema.getName())) + .or(() -> Optional.ofNullable(sc.get$ref()).map(ModelUtils::getSimpleRef)) + .orElseGet(sc::toString); + if (visitedSchemas.contains(schemaName)) { // recursive schema definition found return null; } else { - visitedSchemas.add(schemaKey); + visitedSchemas.add(schemaName); } if (refSchema.getProperties() != null && refSchema.getProperties().get(discPropName) != null) { @@ -3242,7 +3245,8 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, if (composedSchema.getAllOf() != null) { // If our discriminator is in one of the allOf schemas break when we find it for (Object allOf : composedSchema.getAllOf()) { - CodegenProperty cp = discriminatorFound(composedSchemaName, (Schema) allOf, discPropName, visitedSchemas); + Schema allOfSchema = (Schema) allOf; + CodegenProperty cp = discriminatorFound(allOfSchema.getName(), allOfSchema, discPropName, visitedSchemas); if (cp != null) { return cp; } @@ -3252,8 +3256,9 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, // All oneOf definitions must contain the discriminator CodegenProperty cp = new CodegenProperty(); for (Object oneOf : composedSchema.getOneOf()) { - String modelName = ModelUtils.getSimpleRef(((Schema) oneOf).get$ref()); - CodegenProperty thisCp = discriminatorFound(composedSchemaName, (Schema) oneOf, discPropName, visitedSchemas); + Schema oneOfSchema = (Schema) oneOf; + String modelName = ModelUtils.getSimpleRef((oneOfSchema).get$ref()); + CodegenProperty thisCp = discriminatorFound(oneOfSchema.getName(), oneOfSchema, discPropName, visitedSchemas); if (thisCp == null) { once(LOGGER).warn( "'{}' defines discriminator '{}', but the referenced OneOf schema '{}' is missing {}", @@ -3275,8 +3280,9 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, // All anyOf definitions must contain the discriminator because a min of one must be selected CodegenProperty cp = new CodegenProperty(); for (Object anyOf : composedSchema.getAnyOf()) { - String modelName = ModelUtils.getSimpleRef(((Schema) anyOf).get$ref()); - CodegenProperty thisCp = discriminatorFound(composedSchemaName, (Schema) anyOf, discPropName, visitedSchemas); + Schema anyOfSchema = (Schema) anyOf; + String modelName = ModelUtils.getSimpleRef(anyOfSchema.get$ref()); + CodegenProperty thisCp = discriminatorFound(anyOfSchema.getName(), anyOfSchema, discPropName, visitedSchemas); if (thisCp == null) { once(LOGGER).warn( "'{}' defines discriminator '{}', but the referenced AnyOf schema '{}' is missing {}", From 1f8b90c4a660c8e6c6a7732847b2b964b5997c99 Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Fri, 25 Oct 2024 11:10:47 +0200 Subject: [PATCH 15/16] Fix final warnings --- .../java/org/openapitools/codegen/DefaultCodegen.java | 9 ++++++--- .../resources/3_0/enum_discriminator_inheritance.yaml | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index 7f337d01f772..e48a124aa7d7 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3216,7 +3216,6 @@ protected void setAddProps(Schema schema, IJsonSchemaValidationProperties proper */ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set visitedSchemas) { Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); - ModelUtils.getSimpleRef(sc.get$ref()); String schemaName = Optional.ofNullable(composedSchemaName) .or(() -> Optional.ofNullable(refSchema.getName())) .or(() -> Optional.ofNullable(sc.get$ref()).map(ModelUtils::getSimpleRef)) @@ -3258,7 +3257,9 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, for (Object oneOf : composedSchema.getOneOf()) { Schema oneOfSchema = (Schema) oneOf; String modelName = ModelUtils.getSimpleRef((oneOfSchema).get$ref()); - CodegenProperty thisCp = discriminatorFound(oneOfSchema.getName(), oneOfSchema, discPropName, visitedSchemas); + // Must use a copied set as the oneOf schemas can point to the same discriminator. + Set visitedSchemasCopy = new TreeSet<>(visitedSchemas); + CodegenProperty thisCp = discriminatorFound(oneOfSchema.getName(), oneOfSchema, discPropName, visitedSchemasCopy); if (thisCp == null) { once(LOGGER).warn( "'{}' defines discriminator '{}', but the referenced OneOf schema '{}' is missing {}", @@ -3282,7 +3283,9 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, for (Object anyOf : composedSchema.getAnyOf()) { Schema anyOfSchema = (Schema) anyOf; String modelName = ModelUtils.getSimpleRef(anyOfSchema.get$ref()); - CodegenProperty thisCp = discriminatorFound(anyOfSchema.getName(), anyOfSchema, discPropName, visitedSchemas); + // Must use a copied set as the anyOf schemas can point to the same discriminator. + Set visitedSchemasCopy = new TreeSet<>(visitedSchemas); + CodegenProperty thisCp = discriminatorFound(anyOfSchema.getName(), anyOfSchema, discPropName, visitedSchemasCopy); if (thisCp == null) { once(LOGGER).warn( "'{}' defines discriminator '{}', but the referenced AnyOf schema '{}' is missing {}", diff --git a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml index e211a96edcac..db89a48eeb09 100644 --- a/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/enum_discriminator_inheritance.yaml @@ -5,6 +5,7 @@ info: paths: "/animal": get: + operationId: animalGet responses: '200': description: OK From f8ebac151dab828ba2b68191cc6443311ce7337d Mon Sep 17 00:00:00 2001 From: David Riddervold Marconis Date: Thu, 2 Jan 2025 13:30:35 +0100 Subject: [PATCH 16/16] Add javadocs to introduced methods --- .../openapitools/codegen/DefaultCodegen.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index e48a124aa7d7..7a02beab158c 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -3114,6 +3114,19 @@ public CodegenModel fromModel(String name, Schema schema) { return m; } + /** + * Sets the default value for an enum discriminator property in the provided {@link CodegenModel}. + *

+ * If the model's discriminator is defined, this method identifies the discriminator properties among the model's + * variables and assigns the default value to reflect the corresponding enum value for the model type. + *

+ *

+ * Example: If the discriminator is for type `Animal`, and the model is `Cat`, the default value + * will be set to `Animal.Cat` for the properties that have the same name as the discriminator. + *

+ * + * @param model the {@link CodegenModel} whose discriminator property default value is to be set + */ protected static void setEnumDiscriminatorDefaultValue(CodegenModel model) { if (model.discriminator == null) { return; @@ -3125,6 +3138,19 @@ protected static void setEnumDiscriminatorDefaultValue(CodegenModel model) { .forEach(v -> v.defaultValue = getEnumValueForProperty(model.schemaName, model.discriminator, v)); } + /** + * Retrieves the appropriate default value for an enum discriminator property based on the model name. + *

+ * If the discriminator has a mapping defined, it attempts to find a mapping for the model name. + * Otherwise, it defaults to one of the allowable enum value associated with the property. + * If no suitable value is found, the original default value of the property is returned. + *

+ * + * @param modelName the name of the model to determine the default value for + * @param discriminator the {@link CodegenDiscriminator} containing the mapping and enum details + * @param var the {@link CodegenProperty} representing the discriminator property + * @return the default value for the enum discriminator property, or its original default value if none is found + */ protected static String getEnumValueForProperty( String modelName, CodegenDiscriminator discriminator, CodegenProperty var) { if (!discriminator.getIsEnum() && !var.isEnum) {