From a225d0077ad5d313d5b844e8ca12e32586caf078 Mon Sep 17 00:00:00 2001 From: walaniam Date: Thu, 24 Aug 2023 15:53:56 +0200 Subject: [PATCH 1/2] Bugfix: BoatJavaCodeGen and BoatSpringCodeGen - generate proper initialization for arrays --- .../oss/codegen/BoatDefaultGenerator.java | 6 - .../oss/codegen/java/BoatCodeGenUtils.java | 44 ++++++ .../oss/codegen/java/BoatJavaCodeGen.java | 30 +--- .../oss/codegen/java/BoatSpringCodeGen.java | 13 +- .../java/BoatCommonJavaCodeGenTests.java | 132 ++++++++++++++++++ .../codegen/java/BoatSpringCodeGenTests.java | 6 +- .../test/resources/boat-spring/openapi.yaml | 18 +++ 7 files changed, 210 insertions(+), 39 deletions(-) delete mode 100644 boat-scaffold/src/main/java/com/backbase/oss/codegen/BoatDefaultGenerator.java create mode 100644 boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatCodeGenUtils.java create mode 100644 boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatCommonJavaCodeGenTests.java diff --git a/boat-scaffold/src/main/java/com/backbase/oss/codegen/BoatDefaultGenerator.java b/boat-scaffold/src/main/java/com/backbase/oss/codegen/BoatDefaultGenerator.java deleted file mode 100644 index 956e8e00d..000000000 --- a/boat-scaffold/src/main/java/com/backbase/oss/codegen/BoatDefaultGenerator.java +++ /dev/null @@ -1,6 +0,0 @@ -package com.backbase.oss.codegen; - -import org.openapitools.codegen.DefaultGenerator; - -public class BoatDefaultGenerator extends DefaultGenerator { -} diff --git a/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatCodeGenUtils.java b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatCodeGenUtils.java new file mode 100644 index 000000000..c9bf4892a --- /dev/null +++ b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatCodeGenUtils.java @@ -0,0 +1,44 @@ +package com.backbase.oss.codegen.java; + +import io.swagger.v3.oas.models.media.Schema; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import lombok.AccessLevel; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; +import org.openapitools.codegen.CodegenProperty; +import org.openapitools.codegen.utils.ModelUtils; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +class BoatCodeGenUtils { + + /** + * @return {@link CodegenValueType} to be used or empty if given schema is not array + */ + public static Optional getCollectionCodegenValue(CodegenProperty cp, Schema schema, + boolean containerDefaultToNull, Map types) { + CodegenValueType valueType = null; + if (ModelUtils.isSet(schema) && (schema.getDefault() == null)) { + valueType = CodegenValueType.of( + formatValue(cp, containerDefaultToNull, types.getOrDefault("set", "LinkedHashSet"))); + } else if (ModelUtils.isArraySchema(schema) && (schema.getDefault() == null)) { + valueType = CodegenValueType.of( + formatValue(cp, containerDefaultToNull, types.getOrDefault("array", "ArrayList"))); + } + return Optional.ofNullable(valueType); + } + + private static String formatValue(CodegenProperty cp, boolean defaultToNull, String javaSimpleType) { + return (cp.required || !defaultToNull) + ? String.format(Locale.ROOT, "new %s<>()", javaSimpleType) + : null; + } + + @RequiredArgsConstructor(staticName = "of") + @Getter + static class CodegenValueType { + private final String value; + } +} diff --git a/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatJavaCodeGen.java b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatJavaCodeGen.java index 7cedd5478..6fe96ff6a 100644 --- a/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatJavaCodeGen.java +++ b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatJavaCodeGen.java @@ -1,7 +1,9 @@ package com.backbase.oss.codegen.java; +import static com.backbase.oss.codegen.java.BoatCodeGenUtils.getCollectionCodegenValue; + +import com.backbase.oss.codegen.java.BoatCodeGenUtils.CodegenValueType; import io.swagger.v3.oas.models.media.Schema; -import java.util.Locale; import lombok.Getter; import lombok.Setter; import org.openapitools.codegen.CliOption; @@ -120,27 +122,9 @@ private void processRestTemplateOpts() { @Override public String toDefaultValue(CodegenProperty cp, Schema schema) { - schema = ModelUtils.getReferencedSchema(this.openAPI, schema); - if (ModelUtils.isArraySchema(schema) && (schema.getDefault() == null)) { - return getArrayDefaultValue(cp, schema, containerDefaultToNull, - instantiationTypes().getOrDefault("set", "LinkedHashSet"), - instantiationTypes().getOrDefault("array", "ArrayList")); - } - return super.toDefaultValue(cp, schema); - } - - public static String getArrayDefaultValue(CodegenProperty cp, Schema schema, - boolean containerDefaultToNull, String setType, String arrayType) { - if (cp.isNullable || containerDefaultToNull) { - return null; - } else { - if (ModelUtils.isSet(schema)) { - return String.format(Locale.ROOT, "new %s<>()", - setType); - } else { - return String.format(Locale.ROOT, "new %s<>()", - arrayType); - } - } + final Schema referencedSchema = ModelUtils.getReferencedSchema(this.openAPI, schema); + return getCollectionCodegenValue(cp, referencedSchema, containerDefaultToNull, instantiationTypes()) + .map(CodegenValueType::getValue) + .orElseGet(() -> super.toDefaultValue(cp, referencedSchema)); } } diff --git a/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatSpringCodeGen.java b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatSpringCodeGen.java index e54728b2c..90a7a6e73 100644 --- a/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatSpringCodeGen.java +++ b/boat-scaffold/src/main/java/com/backbase/oss/codegen/java/BoatSpringCodeGen.java @@ -1,9 +1,11 @@ package com.backbase.oss.codegen.java; +import static com.backbase.oss.codegen.java.BoatCodeGenUtils.getCollectionCodegenValue; import static java.util.Arrays.stream; import static java.util.stream.Collectors.joining; import static org.openapitools.codegen.utils.StringUtils.camelize; +import com.backbase.oss.codegen.java.BoatCodeGenUtils.CodegenValueType; import com.samskivert.mustache.Mustache; import com.samskivert.mustache.Template.Fragment; import io.swagger.v3.oas.models.Operation; @@ -258,12 +260,9 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation @Override public String toDefaultValue(CodegenProperty cp, Schema schema) { - schema = ModelUtils.getReferencedSchema(this.openAPI, schema); - if (ModelUtils.isArraySchema(schema) && (schema.getDefault() == null)) { - return BoatJavaCodeGen.getArrayDefaultValue(cp, schema, containerDefaultToNull, - instantiationTypes().getOrDefault("set", "LinkedHashSet"), - instantiationTypes().getOrDefault("array", "ArrayList")); - } - return super.toDefaultValue(cp, schema); + final Schema referencedSchema = ModelUtils.getReferencedSchema(this.openAPI, schema); + return getCollectionCodegenValue(cp, referencedSchema, containerDefaultToNull, instantiationTypes()) + .map(CodegenValueType::getValue) + .orElseGet(() -> super.toDefaultValue(cp, referencedSchema)); } } diff --git a/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatCommonJavaCodeGenTests.java b/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatCommonJavaCodeGenTests.java new file mode 100644 index 000000000..428cb342a --- /dev/null +++ b/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatCommonJavaCodeGenTests.java @@ -0,0 +1,132 @@ +package com.backbase.oss.codegen.java; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.openapitools.codegen.languages.JavaClientCodegen.APACHE; +import static org.openapitools.codegen.languages.JavaClientCodegen.RESTTEMPLATE; + +import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.body.FieldDeclaration; +import com.github.javaparser.ast.body.VariableDeclarator; +import io.swagger.parser.OpenAPIParser; +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.parser.core.models.ParseOptions; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; +import lombok.SneakyThrows; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.openapitools.codegen.ClientOptInput; +import org.openapitools.codegen.DefaultGenerator; +import org.openapitools.codegen.languages.AbstractJavaCodegen; + +class BoatCommonJavaCodeGenTests { + + static final String PROP_BASE = BoatCommonJavaCodeGenTests.class.getSimpleName() + "."; + static final String TEST_OUTPUT = System.getProperty(PROP_BASE + "output", + "target/" + BoatCommonJavaCodeGenTests.class.getSimpleName()); + + @BeforeAll + static void before() throws IOException { + Files.createDirectories(Paths.get(TEST_OUTPUT)); + FileUtils.deleteDirectory(new File(TEST_OUTPUT)); + } + + @ParameterizedTest + @ValueSource(strings = {"true", "false", ""}) + void shouldGenerateArrayField(String containerDefaultToNull) { + generateAndAssert(javaCodeGenWithLib(RESTTEMPLATE), containerDefaultToNull); + generateAndAssert(javaCodeGenWithLib(APACHE), containerDefaultToNull); + generateAndAssert(springCodeGen(), containerDefaultToNull); + } + + private static BoatJavaCodeGen javaCodeGenWithLib(String library) { + var codeGen = new BoatJavaCodeGen(); + codeGen.setLibrary(library); + return codeGen; + } + + private static BoatSpringCodeGen springCodeGen() { + var codeGen = new BoatSpringCodeGen(); + codeGen.setLibrary("spring-boot"); + return codeGen; + } + + void generateAndAssert(AbstractJavaCodegen codegen, String containerDefaultToNull) { + + var input = new File("src/test/resources/boat-spring/openapi.yaml"); + var outputDir = TEST_OUTPUT + String.format("/generateAndAssert_%s_%s_%s", codegen.getClass().getSimpleName(), + codegen.getLibrary(), containerDefaultToNull); + + codegen.setInputSpec(input.getAbsolutePath()); + codegen.setOutputDir(outputDir); + + OpenAPI openApiInput = new OpenAPIParser() + .readLocation(input.getAbsolutePath(), null, new ParseOptions()) + .getOpenAPI(); + var clientOptInput = new ClientOptInput(); + clientOptInput.config(codegen); + clientOptInput.openAPI(openApiInput); + if (StringUtils.isNotBlank(containerDefaultToNull)) { + clientOptInput.getConfig().additionalProperties().put("containerDefaultToNull", containerDefaultToNull); + } + + List files = new DefaultGenerator().opts(clientOptInput).generate(); + + CompilationUnit multilinePaymentRequest = files.stream() + .filter(it -> it.getName().equals("MultiLinePaymentRequest.java")) + .findFirst() + .map(file -> { + try { + return StaticJavaParser.parse(file); + } catch (FileNotFoundException e) { + throw new RuntimeException(e); + } + }) + .orElseThrow(); + + if (Boolean.parseBoolean(containerDefaultToNull)) { + // containerDefaultToNull=true + assertVariableDeclarator(multilinePaymentRequest, "lines", "ArrayList"); + assertVariableDeclarator(multilinePaymentRequest, "uniqueLines", "LinkedHashSet"); + assertVariableDeclarator(multilinePaymentRequest, "optionalLines", ""); // optional field, therefore null + assertVariableDeclarator(multilinePaymentRequest, "optionalUniqueLines", ""); // optional field, therefore null + } else { + // containerDefaultToNull=false or default behaviour + assertVariableDeclarator(multilinePaymentRequest, "lines", "ArrayList"); + assertVariableDeclarator(multilinePaymentRequest, "uniqueLines", "LinkedHashSet"); + assertVariableDeclarator(multilinePaymentRequest, "optionalLines", "ArrayList"); + assertVariableDeclarator(multilinePaymentRequest, "optionalUniqueLines", "LinkedHashSet"); + } + } + + @SneakyThrows + private void assertVariableDeclarator(CompilationUnit requestClass, String fieldName, String declarationType) { + VariableDeclarator listDeclarator = requestClass + .findAll(FieldDeclaration.class) + .stream() + .flatMap(field -> field.getChildNodes().stream()) + .filter(node -> node instanceof VariableDeclarator) + .map(VariableDeclarator.class::cast) + .filter(declarator -> declarator.getName().getIdentifier().equals(fieldName)) + .findFirst() + .orElseThrow(); + + if (StringUtils.isNotBlank(declarationType)) { + assertTrue(listDeclarator.getInitializer().isPresent()); + assertEquals(listDeclarator.getInitializer().get().toString(), + String.format("new %s<>()", declarationType)); + } else { + assertFalse(listDeclarator.getInitializer().isPresent()); + } + } +} diff --git a/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatSpringCodeGenTests.java b/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatSpringCodeGenTests.java index 730c92aa4..e69294a6c 100644 --- a/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatSpringCodeGenTests.java +++ b/boat-scaffold/src/test/java/com/backbase/oss/codegen/java/BoatSpringCodeGenTests.java @@ -15,15 +15,15 @@ import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.body.Parameter; import com.samskivert.mustache.Template.Fragment; -import io.swagger.v3.oas.models.Operation; import io.swagger.parser.OpenAPIParser; +import io.swagger.v3.oas.models.Operation; import io.swagger.v3.parser.core.models.ParseOptions; import java.io.File; import java.io.IOException; import java.io.StringWriter; -import java.util.Arrays; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.Arrays; import java.util.List; import java.util.Map; import org.apache.commons.io.FileUtils; @@ -42,7 +42,7 @@ class BoatSpringCodeGenTests { @BeforeAll static void before() throws IOException { Files.createDirectories(Paths.get(TEST_OUTPUT)); - FileUtils.deleteDirectory(new File(TEST_OUTPUT, "src")); + FileUtils.deleteDirectory(new File(TEST_OUTPUT)); } @Test diff --git a/boat-scaffold/src/test/resources/boat-spring/openapi.yaml b/boat-scaffold/src/test/resources/boat-spring/openapi.yaml index 8ce4dd9fa..267107498 100644 --- a/boat-scaffold/src/test/resources/boat-spring/openapi.yaml +++ b/boat-scaffold/src/test/resources/boat-spring/openapi.yaml @@ -101,6 +101,7 @@ components: MultiLinePaymentRequest: required: - lines + - uniqueLines type: object properties: lines: @@ -108,6 +109,23 @@ components: description: Payment request details items: $ref: '#/components/schemas/PaymentRequestLine' + uniqueLines: + type: array + uniqueItems: true + description: Payment request details + items: + $ref: '#/components/schemas/PaymentRequestLine' + optionalLines: + type: array + description: Payment request optional details + items: + $ref: '#/components/schemas/PaymentRequestLine' + optionalUniqueLines: + type: array + uniqueItems: true + description: Payment request optional details + items: + $ref: '#/components/schemas/PaymentRequestLine' PaymentRequestLine: required: - accountId From 90e46f3ac41acd4e70796762b61723b51b9b6e96 Mon Sep 17 00:00:00 2001 From: walaniam Date: Fri, 25 Aug 2023 10:10:35 +0200 Subject: [PATCH 2/2] Bugfix: BoatJavaCodeGen and BoatSpringCodeGen - generate proper initialization for arrays (changelog updated) --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index b2a218874..b466e147b 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,9 @@ The project is very much Work In Progress and will be published on maven central # Release Notes BOAT is still under development and subject to change. +## 0.17.11 +* BoatJavaCodeGen, BoatSpringCodeGen + * Fix: always generate collection initializer when array is required in the schema (even if containerDefaultToNull=true) ## 0.17.10 * Boat maven plugin * Fix: When using Multipart, generate with `@RequestPart` instead of `@RequestParam`