diff --git a/.github/workflows/test-java-feign-jackson-oneof.yaml b/.github/workflows/test-java-feign-jackson-oneof.yaml new file mode 100644 index 000000000000..06340eb35574 --- /dev/null +++ b/.github/workflows/test-java-feign-jackson-oneof.yaml @@ -0,0 +1,32 @@ +name: Java Feign Jackson oneof +on: + push: + paths: + - 'modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java' + - 'modules/openapi-generator/src/main/resources/Java/**' + - 'modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml' + - 'modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java' + - '.github/workflows/test-java-feign-jackson-oneof.yaml' + pull_request: + paths: + - 'modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java' + - 'modules/openapi-generator/src/main/resources/Java/**' + - 'modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml' + - 'modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java' + - '.github/workflows/test-java-feign-jackson-oneof.yaml' +jobs: + runtime: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-java@v6 + with: + distribution: temurin + java-version: '17' + cache: maven + - run: ./mvnw -B -pl modules/openapi-generator-cli -am -DskipTests package + - run: | + java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java --library feign -i modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml -o /tmp/feign-test --additional-properties=serializationLibrary=jackson --global-property=apis,models,supportingFiles,apiTests=false,modelTests=false + mkdir -p /tmp/feign-test/src/test/java + cp modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java /tmp/feign-test/src/test/java/ + mvn -B -f /tmp/feign-test/pom.xml -Dtest=FeignJacksonOneOfRuntimeTest test diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java index c2b1a9aa93b0..71adac8d44ef 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java @@ -1239,6 +1239,9 @@ public CodegenModel fromModel(String name, Schema model) { @Override public Map postProcessAllModels(Map objs) { objs = super.postProcessAllModels(objs); + if ((isLibrary(FEIGN) || isLibrary(RESTTEMPLATE)) && jackson && !useOneOfInterfaces) { + configureJacksonTaggedOneOf(objs); + } if (isLibrary(OKHTTP_GSON)) { // gson refuses a class whose hierarchy declares two fields bound to one JSON name, so // the okhttp-gson additionalProperties bag is declared once, on the topmost ancestor @@ -1260,6 +1263,107 @@ public Map postProcessAllModels(Map objs) return objs; } + /** Preserve object oneOf variants that share a required single-value string tag. */ + private void configureJacksonTaggedOneOf(Map models) { + Map allModels = getAllModels(models); + for (CodegenModel union : allModels.values()) { + if (union.oneOf.size() < 2 || union.discriminator != null || union.isEnum + || openAPI == null || openAPI.getComponents() == null + || openAPI.getComponents().getSchemas() == null) { + continue; + } + Schema schema = openAPI.getComponents().getSchemas().get(union.schemaName); + if (schema == null || schema.getProperties() != null && !schema.getProperties().isEmpty() + || schema.getAllOf() != null && !schema.getAllOf().isEmpty()) { + continue; + } + List variants = new ArrayList<>(); + for (String name : union.oneOf) { + CodegenModel variant = allModels.get(name); + if (variant == null || variant.isEnum || !variant.oneOf.isEmpty() || !variant.anyOf.isEmpty()) { + break; + } + variants.add(variant); + } + if (variants.size() != union.oneOf.size()) { + continue; + } + Map> candidates = new LinkedHashMap<>(); + Map fallbackModels = new HashMap<>(); + for (CodegenProperty first : variants.get(0).allVars) { + if (!first.required || !first.isString) continue; + Map valuesByModel = new LinkedHashMap<>(); + Set excluded = null; + String fallback = null; + boolean valid = true; + for (CodegenModel variant : variants) { + CodegenProperty property = variant.allVars.stream() + .filter(p -> first.baseName.equals(p.baseName)).findFirst().orElse(null); + if (property == null || !property.required || !property.isString) { + valid = false; + break; + } + Object values = property.isEnum && property.allowableValues != null + ? property.allowableValues.get("values") : null; + if (values instanceof List && ((List) values).size() == 1 + && ((List) values).get(0) instanceof String) { + String value = (String) ((List) values).get(0); + if (valuesByModel.containsValue(value)) { valid = false; break; } + valuesByModel.put(variant.classname, value); + } else { + CodegenProperty not = property.getComposedSchemas() == null + ? null : property.getComposedSchemas().getNot(); + Object notValues = not == null || not.allowableValues == null + ? null : not.allowableValues.get("values"); + if (fallback != null || !(notValues instanceof List) + || ((List) notValues).stream().anyMatch(v -> !(v instanceof String))) { + valid = false; + break; + } + fallback = variant.classname; + excluded = new HashSet<>(); + for (Object value : (List) notValues) excluded.add((String) value); + } + } + if (valid && !valuesByModel.isEmpty() + && (fallback == null || excluded.equals(new HashSet<>(valuesByModel.values())))) { + candidates.put(first.baseName, valuesByModel); + if (fallback != null) fallbackModels.put(first.baseName, fallback); + } + } + if (candidates.isEmpty()) { + continue; + } + String tagName = candidates.containsKey(".tag") ? ".tag" : candidates.keySet().iterator().next(); + List> mappings = new ArrayList<>(); + for (CodegenModel variant : variants) { + String value = candidates.get(tagName).get(variant.classname); + if (value != null) { + Map mapping = new HashMap<>(); + mapping.put("model", variant.classname); + mapping.put("value", escapeText(value)); + mappings.add(mapping); + } + Object existing = variant.vendorExtensions.get(X_IMPLEMENTS); + List interfaces = new ArrayList<>(); + if (existing instanceof Collection) { + for (Object item : (Collection) existing) interfaces.add(String.valueOf(item)); + } else if (existing instanceof String) { + interfaces.add((String) existing); + } + if (!interfaces.contains(union.classname)) interfaces.add(union.classname); + variant.vendorExtensions.put(X_IMPLEMENTS, interfaces); + } + if (fallbackModels.containsKey(tagName)) { + union.vendorExtensions.put("x-feign-tag-default", fallbackModels.get(tagName)); + } + union.vendorExtensions.put(X_IS_ONE_OF_INTERFACE, true); + union.vendorExtensions.put("x-feign-tagged-oneof", true); + union.vendorExtensions.put("x-feign-tag-name", escapeText(tagName)); + union.vendorExtensions.put("x-feign-tag-mappings", mappings); + } + } + @Override public ModelsMap postProcessModelsEnum(ModelsMap objs) { objs = super.postProcessModelsEnum(objs); diff --git a/modules/openapi-generator/src/main/resources/Java/oneof_interface.mustache b/modules/openapi-generator/src/main/resources/Java/oneof_interface.mustache index 71888430c647..270d5f530931 100644 --- a/modules/openapi-generator/src/main/resources/Java/oneof_interface.mustache +++ b/modules/openapi-generator/src/main/resources/Java/oneof_interface.mustache @@ -1,3 +1,28 @@ +{{#vendorExtensions.x-feign-tagged-oneof}} +{{>additionalOneOfTypeAnnotations}}{{>generatedAnnotation}} + +@com.fasterxml.jackson.annotation.JsonTypeInfo(use = com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME, include = com.fasterxml.jackson.annotation.JsonTypeInfo.As.EXISTING_PROPERTY, property = "{{{vendorExtensions.x-feign-tag-name}}}", visible = true{{#vendorExtensions.x-feign-tag-default}}, defaultImpl = {{{modelPackage}}}.{{{vendorExtensions.x-feign-tag-default}}}.class{{/vendorExtensions.x-feign-tag-default}}{{^vendorExtensions.x-feign-tag-default}}, defaultImpl = {{classname}}.InvalidVariant.class{{/vendorExtensions.x-feign-tag-default}}) +@com.fasterxml.jackson.annotation.JsonSubTypes({ +{{#vendorExtensions.x-feign-tag-mappings}} + @com.fasterxml.jackson.annotation.JsonSubTypes.Type(value = {{{modelPackage}}}.{{model}}.class, name = "{{{value}}}"){{^-last}},{{/-last}} +{{/vendorExtensions.x-feign-tag-mappings}} +}) +{{>xmlAnnotation}} +{{#vendorExtensions.x-class-extra-annotation}} +{{{.}}} +{{/vendorExtensions.x-class-extra-annotation}} +public interface {{classname}} {{#vendorExtensions.x-implements}}{{#-first}}extends {{{.}}}{{/-first}}{{^-first}}, {{{.}}}{{/-first}}{{#-last}} {{/-last}}{{/vendorExtensions.x-implements}}{ + {{^vendorExtensions.x-feign-tag-default}} + class InvalidVariant implements {{classname}} { + @com.fasterxml.jackson.annotation.JsonCreator + public InvalidVariant() { + throw new IllegalArgumentException("Unknown or missing oneOf discriminator for {{classname}}"); + } + } + {{/vendorExtensions.x-feign-tag-default}} +} +{{/vendorExtensions.x-feign-tagged-oneof}} +{{^vendorExtensions.x-feign-tagged-oneof}} {{>additionalOneOfTypeAnnotations}}{{>generatedAnnotation}}{{#discriminator}}{{>typeInfoAnnotation}}{{/discriminator}}{{^discriminator}}{{^useDeductionForOneOfInterfaces}}{{>typeInfoAnnotation}}{{/useDeductionForOneOfInterfaces}}{{/discriminator}}{{>deductionAnnotation}}{{>xmlAnnotation}} {{#vendorExtensions.x-class-extra-annotation}} {{{.}}} @@ -7,3 +32,4 @@ public {{>sealed}}interface {{classname}} {{#vendorExtensions.x-implements}}{{#- public {{propertyType}} {{propertyGetter}}(); {{/discriminator}} } +{{/vendorExtensions.x-feign-tagged-oneof}} 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 435767b783ec..ac5edb9a5a7c 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 @@ -17,6 +17,8 @@ package org.openapitools.codegen.java; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; import com.github.javaparser.StaticJavaParser; import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.body.FieldDeclaration; @@ -52,8 +54,11 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -65,6 +70,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import javax.tools.ToolProvider; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; @@ -5946,4 +5952,81 @@ public void testRestTemplateGenerateClientAsBeanTrue() { .contains("import org.springframework.beans.factory.annotation.Autowired;") .contains("import org.springframework.stereotype.Component;"); } + @DataProvider(name = "taggedOneOfLibraries") + public static Object[][] taggedOneOfLibraries() { + return new Object[][]{{FEIGN}, {JavaClientCodegen.RESTTEMPLATE}}; + } + + @Test(dataProvider = "taggedOneOfLibraries") + public void testJacksonTaggedOneOf(String library) throws Exception { + final Path output = newTempFolder(); + final CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName(JAVA_GENERATOR) + .setLibrary(library) + .addAdditionalProperty("serializationLibrary", "jackson") + .setInputSpec("src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml") + .setOutputDir(output.toString()); + List files = new DefaultGenerator().opts(configurator.toClientOptInput()).generate(); + validateJavaSourceFiles(files); + Path java = output.resolve("src/main/java/org/openapitools/client"); + String fileModel = FEIGN.equals(library) ? "File" : "ModelFile"; + assertThat(java.resolve("model/Metadata.java")).content() + .contains("interface Metadata", "defaultImpl = Metadata.InvalidVariant.class", + "class InvalidVariant implements Metadata", + "Unknown or missing oneOf discriminator for Metadata", + "org.openapitools.client.model." + fileModel + ".class, name = \"file\"", + "org.openapitools.client.model.Folder.class, name = \"folder\"") + .doesNotContain("abstract class InvalidVariant"); + assertThat(java.resolve("model/OpenMetadata.java")).content() + .contains("defaultImpl = org.openapitools.client.model.Other.class"); + assertThat(java.resolve("model/" + fileModel + ".java")).content().contains("implements Metadata", "ModelClass", "OpenMetadata"); + assertThat(java.resolve("model/ModelClass.java")).content() + .contains("JsonTypeInfo", "@Deprecated", "interface ModelClass extends java.io.Serializable"); + verifyTaggedOneOfRuntime(output, java.resolve("model/Metadata.java"), fileModel); + } + + @Test(dataProvider = "taggedOneOfLibraries") + public void testTaggedOneOfAnnotations(String library) { + Path output = newTempFolder(); + CodegenConfigurator configurator = new CodegenConfigurator() + .setGeneratorName(JAVA_GENERATOR) + .setLibrary(library) + .addAdditionalProperty("serializationLibrary", "jackson") + .addAdditionalProperty("additionalOneOfTypeAnnotations", "@SuppressWarnings(\"unused\")") + .addAdditionalProperty("withXml", true) + .setInputSpec("src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml") + .setOutputDir(output.toString()); + new DefaultGenerator().opts(configurator.toClientOptInput()).generate(); + assertThat(output.resolve("src/main/java/org/openapitools/client/model/Metadata.java")).content() + .contains("@SuppressWarnings(\"unused\")", "@XmlRootElement", "@XmlAccessorType", "@JacksonXmlRootElement"); + } + + private static void verifyTaggedOneOfRuntime(Path output, Path generatedInterface, String fileModel) throws Exception { + Path runtime = Files.createDirectory(output.resolve("tagged-oneof-runtime")); + Path source = Files.createDirectories(runtime.resolve("org/openapitools/client/model")); + // Compile the generated interface with minimal variants; the variants' unrelated fields are not needed here. + String metadata = Files.readString(generatedInterface) + .replaceAll("(?m)^@javax\\.annotation\\.Generated\\([^\\n]*\\)\\R", ""); + Path metadataFile = Files.writeString(source.resolve("Metadata.java"), metadata); + String variant = "package org.openapitools.client.model; public class %s implements Metadata { " + + "@com.fasterxml.jackson.annotation.JsonProperty(\".tag\") public String tag; }"; + Path file = Files.writeString(source.resolve(fileModel + ".java"), String.format(Locale.ROOT, variant, fileModel)); + Path folder = Files.writeString(source.resolve("Folder.java"), String.format(Locale.ROOT, variant, "Folder")); + ByteArrayOutputStream diagnostics = new ByteArrayOutputStream(); + int result = ToolProvider.getSystemJavaCompiler().run(null, diagnostics, diagnostics, + "-classpath", System.getProperty("java.class.path"), "-d", runtime.toString(), + metadataFile.toString(), file.toString(), folder.toString()); + Assert.assertEquals(result, 0, diagnostics.toString(StandardCharsets.UTF_8)); + try (URLClassLoader loader = new URLClassLoader(new URL[]{runtime.toUri().toURL()}, JavaClientCodegenTest.class.getClassLoader())) { + Class type = loader.loadClass("org.openapitools.client.model.Metadata"); + ObjectMapper mapper = new ObjectMapper(); + Assert.assertEquals(mapper.readValue("{\".tag\":\"file\"}", type).getClass().getSimpleName(), fileModel); + Assert.assertEquals(mapper.readValue("{\".tag\":\"folder\"}", type).getClass().getSimpleName(), "Folder"); + for (String json : List.of("{\".tag\":\"unknown\"}", "{}")) { + JsonMappingException error = Assert.expectThrows(JsonMappingException.class, () -> mapper.readValue(json, type)); + assertThat(error.getMessage()).contains("Unknown or missing oneOf discriminator for Metadata"); + } + } + } + } diff --git a/modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java b/modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java new file mode 100644 index 000000000000..7bcdcb4eda7a --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/java/FeignJacksonOneOfRuntimeTest.java @@ -0,0 +1,20 @@ +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.openapitools.client.ApiClient; +import org.openapitools.client.model.Metadata; +import org.openapitools.client.model.OpenMetadata; +import org.openapitools.client.model.Folder; +import org.openapitools.client.model.Other; +import static org.junit.jupiter.api.Assertions.*; + +class FeignJacksonOneOfRuntimeTest { + @Test + void deserializesClosedAndOpenTaggedUnions() throws Exception { + ObjectMapper mapper = new ApiClient().getObjectMapper(); + assertInstanceOf(org.openapitools.client.model.File.class, mapper.readValue("{\".tag\":\"file\",\"name\":\"x\"}", Metadata.class)); + assertInstanceOf(Folder.class, mapper.readValue("{\".tag\":\"folder\",\"name\":\"x\"}", Metadata.class)); + assertThrows(Exception.class, () -> mapper.readValue("{\".tag\":\"other\"}", Metadata.class)); + assertInstanceOf(Other.class, mapper.readValue("{\".tag\":\"other\"}", OpenMetadata.class)); + assertInstanceOf(org.openapitools.client.model.File.class, mapper.readValue("{\".tag\":\"file\",\"name\":\"x\"}", OpenMetadata.class)); + } +} diff --git a/modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml b/modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml new file mode 100644 index 000000000000..f142ebed068d --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/java/feign-jackson-tagged-oneof.yaml @@ -0,0 +1,69 @@ +openapi: 3.1.0 +info: + title: Feign Jackson oneOf and JSON header + version: 1.0.0 +paths: + /metadata: + get: + operationId: getMetadata + responses: + '200': + description: metadata + content: + application/json: + schema: + $ref: '#/components/schemas/Metadata' +components: + schemas: + Metadata: + oneOf: + - $ref: '#/components/schemas/File' + - $ref: '#/components/schemas/Folder' + File: + allOf: + - type: object + required: [.tag] + properties: + .tag: + type: string + enum: [file] + - type: object + properties: + name: + type: string + size: + type: integer + Folder: + allOf: + - type: object + required: [.tag] + properties: + .tag: + type: string + enum: [folder] + - type: object + properties: + name: + type: string + OpenMetadata: + oneOf: + - $ref: '#/components/schemas/File' + - $ref: '#/components/schemas/Folder' + - $ref: '#/components/schemas/Other' + Other: + type: object + required: [.tag] + properties: + .tag: + type: string + not: + enum: [file, folder] + name: + type: string + class: + x-class-extra-annotation: '@Deprecated' + x-implements: + - java.io.Serializable + oneOf: + - $ref: '#/components/schemas/File' + - $ref: '#/components/schemas/Folder'