From 819fbe02ccd19b7a6fb86a6f34ffae3bf4e71891 Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Thu, 29 Sep 2022 12:25:37 +0200 Subject: [PATCH 1/2] Fix AbstractJavaCodegen ability to customize container types After https://github.com/OpenAPITools/openapi-generator/pull/5466 java codegen emits Set for array with uniqueItems=true which has its downsides - easy to lose track of element order (it can be important) - easy to lose ordering by switching between java Set flavors (HashSet vs LinkedHashSet) somewhere down the line - bigger divergence from original json types (either array or object) Luckily types can be customized/overriden by extending the generator: - extend JavaClientCodegen for example - in constructor do ```java super(); // import/interface type typeMapping().put("set", typeMapping().get("array")); // concrete type instantiationTypes().put("set", instantiationTypes().get("array")); ``` But sometimes it won't work because of missing ArrayList import, can be fixed by also doing ```java importMapping().put("LinkedHashSet", importMapping().get("ArrayList")); ``` Looks like a bug of assuming "set" fields are instantiated with LinkedHashSet, let's fix it by using type mappings. --- .../codegen/languages/AbstractJavaCodegen.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 254370429374..b97794e75aa0 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 @@ -1319,16 +1319,16 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert if (!fullJavaUtil) { if ("array".equals(property.containerType)) { - model.imports.add("ArrayList"); + model.imports.add(instantiationTypes.get("array")); } else if ("set".equals(property.containerType)) { - model.imports.add("LinkedHashSet"); + model.imports.add(instantiationTypes.get("set")); boolean canNotBeWrappedToNullable = !openApiNullable || !property.isNullable; if (canNotBeWrappedToNullable) { model.imports.add("JsonDeserialize"); - property.vendorExtensions.put("x-setter-extra-annotation", "@JsonDeserialize(as = LinkedHashSet.class)"); + property.vendorExtensions.put("x-setter-extra-annotation", "@JsonDeserialize(as = " + instantiationTypes.get("set") + ".class)"); } } else if ("map".equals(property.containerType)) { - model.imports.add("HashMap"); + model.imports.add(instantiationTypes.get("map")); } } From 8a62c9c9d98d02d5d4f8bac470cd1f4780a9077a Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Mon, 3 Oct 2022 13:55:24 +0200 Subject: [PATCH 2/2] add unit test --- .../codegen/java/AbstractJavaCodegenTest.java | 34 +++++++++++++++++++ .../src/test/resources/3_0/setSchema.yaml | 26 ++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 modules/openapi-generator/src/test/resources/3_0/setSchema.yaml diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java index 2edaa5024e70..1e11bb857da3 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java @@ -841,6 +841,40 @@ public void maplikeDefaultValueForModelWithStringToModelMapping() { Assert.assertEquals(defaultValue, "new HashMap<>()", "Expected string-ref map aliased model to default to new HashMap()"); } + @Test + public void uniqueItemsSet() { + final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/setSchema.yaml"); + codegen.additionalProperties().put(CodegenConstants.GENERATE_ALIAS_AS_MODEL, true); + codegen.setOpenAPI(openAPI); + + Schema schema = openAPI.getComponents().getSchemas().get("Response"); + CodegenModel cm = codegen.fromModel("Response", schema); + Assert.assertFalse(cm.getImports().contains("List"), "expect List import"); + Assert.assertFalse(cm.getImports().contains("ArrayList"), "expect ArrayList import"); + Assert.assertTrue(cm.getImports().contains("Set"), "expect no Set import"); + Assert.assertTrue(cm.getImports().contains("LinkedHashSet"), "expect no LinkedHashSet import"); + } + + @Test + public void uniqueItemsArray() { + final P_AbstractJavaCodegen codegen = new P_AbstractJavaCodegen(); + // overrides to keep uniqueItems=true arrays as arrays in generated code + codegen.typeMapping().put("set", codegen.typeMapping().get("array")); + codegen.instantiationTypes().put("set", codegen.instantiationTypes().get("array")); + + final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/setSchema.yaml"); + codegen.additionalProperties().put(CodegenConstants.GENERATE_ALIAS_AS_MODEL, true); + codegen.setOpenAPI(openAPI); + + Schema schema = openAPI.getComponents().getSchemas().get("Response"); + CodegenModel cm = codegen.fromModel("Response", schema); + Assert.assertTrue(cm.getImports().contains("List"), "expect List import"); + Assert.assertTrue(cm.getImports().contains("ArrayList"), "expect ArrayList import"); + Assert.assertFalse(cm.getImports().contains("Set"), "expect no Set import"); + Assert.assertFalse(cm.getImports().contains("LinkedHashSet"), "expect no LinkedHashSet import"); + } + @Test public void srcMainFolderShouldNotBeOperatingSystemSpecificPaths() { // it's not responsibility of the generator to fix OS-specific paths. This is left to template manager. diff --git a/modules/openapi-generator/src/test/resources/3_0/setSchema.yaml b/modules/openapi-generator/src/test/resources/3_0/setSchema.yaml new file mode 100644 index 000000000000..bdd176141969 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/setSchema.yaml @@ -0,0 +1,26 @@ +openapi: 3.0.0 +paths: + /examples: + get: + tags: + - Examples + summary: Get a list of transactions + operationId: getFilteredTransactions + responses: + 200: + description: A list of deleted consumers + content: + application/json: + schema: + $ref: '#/components/schemas/Response' + +components: + schemas: + Response: + type: object + properties: + something: + type: array + uniqueItems: true + items: + type: string