From b8df91b1b3f095ab447e15207ce6ab10ade3b1a1 Mon Sep 17 00:00:00 2001 From: Adam Sickmiller Date: Thu, 31 Aug 2023 16:11:19 -0400 Subject: [PATCH] Issue 16394: Fix uncompileable java client code from composed schema discriminator enums --- .../openapitools/codegen/DefaultCodegen.java | 26 ++++++-- .../codegen/java/JavaClientCodegenTest.java | 29 +++++++++ .../src/test/resources/bugs/issue_16394.yaml | 63 +++++++++++++++++++ 3 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/bugs/issue_16394.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 d7d1fc34ef39..bc7d4b0a5161 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 @@ -561,6 +561,9 @@ public Map postProcessAllModels(Map objs) } } + + + return objs; } @@ -3576,12 +3579,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 - 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); + + // sometimes the discriminator is directly described in the properties + discriminator.setIsEnum(discriminatorIsEnumString(schema, discriminatorPropertyName)); + if (ModelUtils.isComposedSchema(schema)) { + // other times the properties are null and only the parent contains data about if the discriminator is an enum + for(Schema s: getModelNameToSchemaCache().values()) { + discriminator.setIsEnum(discriminatorIsEnumString(s, discriminatorPropertyName)); } } @@ -3639,6 +3643,16 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch return discriminator; } + private static boolean discriminatorIsEnumString(Schema schema, String discriminatorPropertyName) { + 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 + return true; + } + } + return false; + } + /** * Handle the model for the 'additionalProperties' keyword in the OAS schema. * 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 f5519605badc..4ca6ae29baa6 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 @@ -45,6 +45,7 @@ import org.openapitools.codegen.DefaultGenerator; import org.openapitools.codegen.TestUtils; import org.openapitools.codegen.config.CodegenConfigurator; +import org.openapitools.codegen.config.GlobalSettings; import org.openapitools.codegen.java.assertions.JavaFileAssert; import org.openapitools.codegen.languages.AbstractJavaCodegen; import org.openapitools.codegen.languages.JavaClientCodegen; @@ -2384,4 +2385,32 @@ public void testEnumCaseSensitive_issue8084() throws IOException { .bodyContainsLines("if (b.value.equals(value)) {"); } + @Test + public void testIfJavaCodeWithDiscriminatorWillCompile_issue16394() throws IOException { + Map properties = new HashMap<>(); + properties.put(CodegenConstants.API_PACKAGE, "xyz.abcdef.api"); + properties.put(JavaClientCodegen.OKHTTP_GSON, true); + + File output = Files.createTempDirectory("test").toFile(); + output.deleteOnExit(); + + final CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName("java") + .setInputSpec("src/test/resources/bugs/issue_16394.yaml") + .setOutputDir(output.getAbsolutePath().replace("\\", "/")); + + GlobalSettings.setProperty("debugModels", "true"); + DefaultGenerator generator = new DefaultGenerator(); + List files = generator.opts(configurator.toClientOptInput()).generate(); + files.forEach(File::deleteOnExit); + + for(File f: files) { + // a rather fragile way of determining if the code won't compile. a better approach would be a reusable + // utility to determine this, similar to how validateJavaSourceFiles checks the syntax. + if("Cat.java".equals(f.getName())) { + String fileContents = new String(Files.readAllBytes(f.toPath()), StandardCharsets.UTF_8); + Assert.assertFalse(fileContents.contains("this.petType = this.getClass().getSimpleName()")); + } + } + } } diff --git a/modules/openapi-generator/src/test/resources/bugs/issue_16394.yaml b/modules/openapi-generator/src/test/resources/bugs/issue_16394.yaml new file mode 100644 index 000000000000..f6da4c90a6f3 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/bugs/issue_16394.yaml @@ -0,0 +1,63 @@ +--- +openapi: 3.0.3 +info: + title: example of a parent schema with discriminator property that is an enum + version: 2.0 +paths: + "/pet/{petId}": + get: + summary: Find pet by id + operationId: getBetById + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + '200': + description: OK - The request has succeeded. + content: + application/json: + schema: + type: array + items: + "$ref": "#/components/schemas/Pet" +components: + schemas: + Pet: + type: object + required: + - petType + properties: + petType: + type: string + enum: [ Cat, Dog, Lizard ] + discriminator: + propertyName: petType + mapping: + dog: Dog + Cat: + allOf: + - $ref: '#/components/schemas/Pet' + - type: object + # all other properties specific to a `Cat` + properties: + name: + type: string + Dog: + allOf: + - $ref: '#/components/schemas/Pet' + - type: object + # all other properties specific to a `Dog` + properties: + bark: + type: string + Lizard: + allOf: + - $ref: '#/components/schemas/Pet' + - type: object + # all other properties specific to a `Lizard` + properties: + lovesRocks: + type: boolean