Skip to content

Commit

Permalink
[Java][jersey2] Fix generated client code for oneOf models if datatyp…
Browse files Browse the repository at this point in the history
…e includes arrays (#18042)

* Add java-jersey2 sample with mixed oneOf

* [java][jersey2]Fix client generation if oneOf contains an array type

Changes:
* Change jersey2/oneof_model template to use composed schema data
* Change adding of imports in AbstractJavaCodegen to use composed schema data
* Add escapedDataType property to CodegenProperty so that the data type
  may be part of identifiers (e.g. in getters)
* Update samples

* Add sample for multiple array in oneOf

* Fix generation of constructors with same erasures

* Update samples again

* Version bump

* Add new sample folders to CI

* Make primitive handling more explicit

* Replace escapedDataType property with Mustache lambda

* Update samples with new primitive handling and sanitization lambda
  • Loading branch information
B4ckslash authored Mar 30, 2024
1 parent f73db59 commit 807aa5d
Show file tree
Hide file tree
Showing 107 changed files with 8,636 additions and 366 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/samples-java-client-jdk11.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ on:
- samples/client/others/java/okhttp-gson-oneOf/**
- samples/client/others/java/resttemplate-useAbstractionForFiles/**
- samples/client/others/java/webclient-useAbstractionForFiles/**
- samples/client/others/java/jersey2-oneOf-duplicates/**
- samples/client/others/java/jersey2-oneOf-Mixed/**
pull_request:
paths:
- 'samples/client/petstore/java/**'
Expand All @@ -25,6 +27,8 @@ on:
- samples/client/others/java/okhttp-gson-oneOf/**
- samples/client/others/java/resttemplate-useAbstractionForFiles/**
- samples/client/others/java/webclient-useAbstractionForFiles/**
- samples/client/others/java/jersey2-oneOf-duplicates/**
- samples/client/others/java/jersey2-oneOf-Mixed/**
jobs:
build:
name: Build Java Client JDK11
Expand Down Expand Up @@ -71,6 +75,8 @@ jobs:
- samples/client/others/java/okhttp-gson-oneOf/
- samples/client/others/java/resttemplate-useAbstractionForFiles/
- samples/client/others/java/webclient-useAbstractionForFiles/
- samples/client/others/java/jersey2-oneOf-duplicates/
- samples/client/others/java/jersey2-oneOf-Mixed/
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand Down
7 changes: 7 additions & 0 deletions bin/configs/java-jersey2-8-oneOfDuplicateList.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
generatorName: java
outputDir: samples/client/others/java/jersey2-oneOf-duplicates
library: jersey2
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_duplicateArray.yaml
templateDir: modules/openapi-generator/src/main/resources/Java
additionalProperties:
hideGenerationTimestamp: "true"
7 changes: 7 additions & 0 deletions bin/configs/java-jersey2-8-oneOfMixed.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
generatorName: java
outputDir: samples/client/others/java/jersey2-oneOf-Mixed
library: jersey2
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_primitiveAndArray.yaml
templateDir: modules/openapi-generator/src/main/resources/Java
additionalProperties:
hideGenerationTimestamp: "true"
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ public interface CodegenConfig {
*/
String generatorLanguageVersion();

boolean isTypeErasedGenerics();

List<VendorExtension> getSupportedVendorExtensions();

boolean getUseInlineModelResolver();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,7 @@ public void setIsEnum(boolean isEnum) {
this.isEnum = isEnum;
}


@Override
public String toString() {
final StringBuilder sb = new StringBuilder("CodegenProperty{");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8437,12 +8437,17 @@ private List<CodegenProperty> getComposedProperties(List<Schema> xOfCollection,
xOf.add(cp);
i += 1;

if (dataTypeSet.contains(cp.dataType)) {
if (dataTypeSet.contains(cp.dataType)
|| (isTypeErasedGenerics() && dataTypeSet.contains(cp.baseType))) {
// add "x-duplicated-data-type" to indicate if the dataType already occurs before
// in other sub-schemas of allOf/anyOf/oneOf
cp.vendorExtensions.putIfAbsent("x-duplicated-data-type", true);
} else {
dataTypeSet.add(cp.dataType);
if(isTypeErasedGenerics()) {
dataTypeSet.add(cp.baseType);
} else {
dataTypeSet.add(cp.dataType);
}
}
}
return xOf;
Expand Down Expand Up @@ -8479,11 +8484,16 @@ public Set<String> getOpenAPIGeneratorIgnoreList() {
return openapiGeneratorIgnoreList;
}

@Override
public boolean isTypeErasedGenerics() {
return false;
}

/*
A function to convert yaml or json ingested strings like property names
And convert special characters like newline, tab, carriage return
Into strings that can be rendered in the language that the generator will output to
*/
A function to convert yaml or json ingested strings like property names
And convert special characters like newline, tab, carriage return
Into strings that can be rendered in the language that the generator will output to
*/
protected String handleSpecialCharacters(String name) { return name; }

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.samskivert.mustache.Mustache;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.Operation;
import io.swagger.v3.oas.models.PathItem;
Expand Down Expand Up @@ -666,6 +667,14 @@ public void processOpts() {
this.setContainerDefaultToNull(Boolean.parseBoolean(additionalProperties.get(CONTAINER_DEFAULT_TO_NULL).toString()));
}
additionalProperties.put(CONTAINER_DEFAULT_TO_NULL, containerDefaultToNull);

additionalProperties.put("sanitizeGeneric", (Mustache.Lambda) (fragment, writer) -> {
String content = fragment.execute();
for (final String s: List.of("<", ">", ",", " ")) {
content = content.replace(s, "");
}
writer.write(content);
});
}

@Override
Expand Down Expand Up @@ -1592,15 +1601,29 @@ public CodegenModel fromModel(String name, Schema model) {
}

// additional import for different cases
addAdditionalImports(codegenModel, codegenModel.oneOf);
addAdditionalImports(codegenModel, codegenModel.anyOf);
addAdditionalImports(codegenModel, codegenModel.getComposedSchemas());
return codegenModel;
}

private void addAdditionalImports(CodegenModel model, Set<String> dataTypeSet) {
for (String dataType : dataTypeSet) {
if (null != importMapping().get(dataType)) {
model.imports.add(dataType);
private void addAdditionalImports(CodegenModel model, CodegenComposedSchemas composedSchemas) {
if(composedSchemas == null) {
return;
}

final List<List<CodegenProperty>> propertyLists = Arrays.asList(
composedSchemas.getAnyOf(),
composedSchemas.getOneOf(),
composedSchemas.getAllOf());
for(final List<CodegenProperty> propertyList : propertyLists){
if(propertyList == null)
{
continue;
}
for (CodegenProperty cp : propertyList) {
final String dataType = cp.baseType;
if (null != importMapping().get(dataType)) {
model.imports.add(dataType);
}
}
}
}
Expand Down Expand Up @@ -2507,4 +2530,9 @@ public static void addImports(List<Map<String, String>> imports, CodegenModel cm
imports.add(importsItem);
}
}

@Override
public boolean isTypeErasedGenerics() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,37 +78,74 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
boolean typeCoercion = ctxt.isEnabled(MapperFeature.ALLOW_COERCION_OF_SCALARS);
int match = 0;
JsonToken token = tree.traverse(jp.getCodec()).nextToken();
{{#composedSchemas}}
{{#oneOf}}
// deserialize {{{.}}}
// deserialize {{{dataType}}}{{#isNullable}} (nullable){{/isNullable}}
try {
{{^isArray}}
boolean attemptParsing = true;
// ensure that we respect type coercion as set on the client ObjectMapper
if ({{{.}}}.class.equals(Integer.class) || {{{.}}}.class.equals(Long.class) || {{{.}}}.class.equals(Float.class) || {{{.}}}.class.equals(Double.class) || {{{.}}}.class.equals(Boolean.class) || {{{.}}}.class.equals(String.class)) {
attemptParsing = typeCoercion;
if (!attemptParsing) {
attemptParsing |= (({{{.}}}.class.equals(Integer.class) || {{{.}}}.class.equals(Long.class)) && token == JsonToken.VALUE_NUMBER_INT);
attemptParsing |= (({{{.}}}.class.equals(Float.class) || {{{.}}}.class.equals(Double.class)) && token == JsonToken.VALUE_NUMBER_FLOAT);
attemptParsing |= ({{{.}}}.class.equals(Boolean.class) && (token == JsonToken.VALUE_FALSE || token == JsonToken.VALUE_TRUE));
attemptParsing |= ({{{.}}}.class.equals(String.class) && token == JsonToken.VALUE_STRING);
{{#isNullable}}
attemptParsing |= (token == JsonToken.VALUE_NULL);
{{/isNullable}}
}
{{#isPrimitiveType}}
attemptParsing = typeCoercion; //respect type coercion setting
if (!attemptParsing) {
{{#isString}}
attemptParsing |= (token == JsonToken.VALUE_STRING);
{{/isString}}
{{#isInteger}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_INT);
{{/isInteger}}
{{#isLong}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_INT);
{{/isLong}}
{{#isShort}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_INT);
{{/isShort}}
{{#isFloat}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_FLOAT);
{{/isFloat}}
{{#isDouble}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_FLOAT);
{{/isDouble}}
{{#isNumber}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_FLOAT);
{{/isNumber}}
{{#isDecimal}}
attemptParsing |= (token == JsonToken.VALUE_NUMBER_FLOAT);
{{/isDecimal}}
{{#isBoolean}}
attemptParsing |= (token == JsonToken.VALUE_FALSE || token == JsonToken.VALUE_TRUE));
{{/isBoolean}}
{{#isNullable}}
attemptParsing |= (token == JsonToken.VALUE_NULL);
{{/isNullable}}
}
{{/isPrimitiveType}}
if (attemptParsing) {
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{.}}}.class);
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{dataType}}}.class);
// TODO: there is no validation against JSON schema constraints
// (min, max, enum, pattern...), this does not perform a strict JSON
// validation, which means the 'match' count may be higher than it should be.
match++;
log.log(Level.FINER, "Input data matches schema '{{{.}}}'");
log.log(Level.FINER, "Input data matches schema '{{{dataType}}}'");
}
{{/isArray}}
{{#isArray}}
if (token == JsonToken.START_ARRAY) {
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
// TODO: there is no validation against JSON schema constraints
// (min, max, enum, pattern...), this does not perform a strict JSON
// validation, which means the 'match' count may be higher than it should be.
match++;
log.log(Level.FINER, "Input data matches schema '{{{dataType}}}'");
}
{{/isArray}}
} catch (Exception e) {
// deserialization failed, continue
log.log(Level.FINER, "Input data does not match schema '{{{.}}}'", e);
log.log(Level.FINER, "Input data does not match schema '{{{dataType}}}'", e);
}

{{/oneOf}}
{{/composedSchemas}}
if (match == 1) {
{{classname}} ret = new {{classname}}();
ret.setActualInstance(deserialized);
Expand Down Expand Up @@ -152,13 +189,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return Objects.hash(getActualInstance(), isNullable(), getSchemaType(), additionalProperties);
}
{{/additionalPropertiesType}}
{{#composedSchemas}}
{{#oneOf}}
public {{classname}}({{{.}}} o) {
{{^vendorExtensions.x-duplicated-data-type}}
public {{classname}}({{{baseType}}} o) {
super("oneOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
setActualInstance(o);
}
{{/vendorExtensions.x-duplicated-data-type}}

{{/oneOf}}
{{/composedSchemas}}
static {
{{#oneOf}}
schemas.put("{{{.}}}", new GenericType<{{{.}}}>() {
Expand Down Expand Up @@ -198,13 +239,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}

{{/isNullable}}
{{#composedSchemas}}
{{#oneOf}}
if (JSON.isInstanceOf({{{.}}}.class, instance, new HashSet<>())) {
{{^vendorExtensions.x-duplicated-data-type}}
if (JSON.isInstanceOf({{{baseType}}}.class, instance, new HashSet<>())) {
super.setActualInstance(instance);
return;
}

{{/vendorExtensions.x-duplicated-data-type}}
{{/oneOf}}
{{/composedSchemas}}
throw new RuntimeException("Invalid instance type. Must be {{#oneOf}}{{{.}}}{{^-last}}, {{/-last}}{{/oneOf}}");
}

Expand All @@ -219,17 +264,26 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return super.getActualInstance();
}

{{#composedSchemas}}
{{#oneOf}}
/**
* Get the actual instance of `{{{.}}}`. If the actual instance is not `{{{.}}}`,
* Get the actual instance of `{{{dataType}}}`. If the actual instance is not `{{{dataType}}}`,
* the ClassCastException will be thrown.
*
* @return The actual instance of `{{{.}}}`
* @throws ClassCastException if the instance is not `{{{.}}}`
* @return The actual instance of `{{{dataType}}}`
* @throws ClassCastException if the instance is not `{{{dataType}}}`
*/
public {{{.}}} get{{{.}}}() throws ClassCastException {
return ({{{.}}})super.getActualInstance();
{{^isArray}}
public {{{dataType}}} get{{{dataType}}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isArray}}
{{#isArray}}
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isArray}}

{{/oneOf}}
{{/composedSchemas}}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
openapi: 3.0.1
info:
version: 1.0.0
title: Example - oneOf data type
license:
name: MIT
servers:
- url: http://api.example.xyz/v1
paths:
/example:
get:
operationId: list
responses:
"200":
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/Example"
components:
schemas:
Example:
oneOf:
- type: array
items:
type: number
- type: array
items:
type: integer
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
openapi: 3.0.1
info:
version: 1.0.0
title: Example - oneOf data type
license:
name: MIT
servers:
- url: http://api.example.xyz/v1
paths:
/example:
get:
operationId: list
responses:
"200":
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/Example"
components:
schemas:
Example:
oneOf:
- type: string
format: uuid
- type: array
items:
type: integer
Loading

0 comments on commit 807aa5d

Please sign in to comment.