Skip to content

Commit

Permalink
[JAVA] fix several anyOf/oneOf problems (#19817)
Browse files Browse the repository at this point in the history
* erasure duplicates

* sanitize beanValidation

* oneOf maps

* anyOf

* update samples
  • Loading branch information
martin-mfg authored Oct 10, 2024
1 parent 43fd189 commit b730e36
Show file tree
Hide file tree
Showing 137 changed files with 10,054 additions and 103 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/samples-java-client-jdk11.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ on:
- samples/openapi3/client/petstore/java/jersey2-java8-swagger2/**
- samples/openapi3/client/petstore/java/native**
- samples/client/others/java/okhttp-gson-oneOf/**
- samples/client/others/java/okhttp-gson-oneOf-array/**
- samples/client/others/java/resttemplate-useAbstractionForFiles/**
- samples/client/others/java/webclient-useAbstractionForFiles/**
- samples/client/others/java/jersey2-oneOf-duplicates/**
Expand All @@ -26,6 +27,7 @@ on:
- samples/openapi3/client/petstore/java/jersey2-java8-swagger2/**
- samples/openapi3/client/petstore/java/native**
- samples/client/others/java/okhttp-gson-oneOf/**
- samples/client/others/java/okhttp-gson-oneOf-array/**
- samples/client/others/java/resttemplate-useAbstractionForFiles/**
- samples/client/others/java/webclient-useAbstractionForFiles/**
- samples/client/others/java/jersey2-oneOf-duplicates/**
Expand Down Expand Up @@ -75,6 +77,7 @@ jobs:
- samples/client/petstore/java/resttemplate-swagger2/
- samples/openapi3/client/petstore/java/jersey2-java8-swagger2/
- samples/client/others/java/okhttp-gson-oneOf/
- samples/client/others/java/okhttp-gson-oneOf-array/
- samples/client/echo_api/java/okhttp-gson-user-defined-templates/
- samples/client/others/java/resttemplate-useAbstractionForFiles/
- samples/client/others/java/webclient-useAbstractionForFiles/
Expand Down
1 change: 1 addition & 0 deletions CI/circle_parallel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ else
(cd samples/client/petstore/java/jersey2-java8 && mvn integration-test)
(cd samples/openapi3/client/petstore/java/jersey2-java8 && mvn integration-test)
(cd samples/client/petstore/java/jersey3 && mvn integration-test)
(cd samples/client/petstore/java/jersey3-oneOf && mvn integration-test)
(cd samples/client/others/java/okhttp-gson-streaming && mvn integration-test)
(cd samples/client/petstore/java/okhttp-gson && mvn integration-test)
(cd samples/client/petstore/java/okhttp-gson-3.1 && mvn integration-test)
Expand Down
7 changes: 7 additions & 0 deletions bin/configs/java-jersey3-oneOf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
generatorName: java
outputDir: samples/client/petstore/java/jersey3-oneOf
library: jersey3
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_additionalProperties.yaml
templateDir: modules/openapi-generator/src/main/resources/Java
additionalProperties:
hideGenerationTimestamp: true
8 changes: 8 additions & 0 deletions bin/configs/java-okhttp-gson-oneOf-array.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
generatorName: java
outputDir: samples/client/others/java/okhttp-gson-oneOf-array
library: okhttp-gson
inputSpec: modules/openapi-generator/src/test/resources/3_0/oneOf_array.yaml
templateDir: modules/openapi-generator/src/main/resources/Java
additionalProperties:
hideGenerationTimestamp: "true"
useBeanValidation: "true"
Original file line number Diff line number Diff line change
Expand Up @@ -8444,6 +8444,7 @@ private List<CodegenProperty> getComposedProperties(List<Schema> xOfCollection,
}
List<CodegenProperty> xOf = new ArrayList<>();
Set<String> dataTypeSet = new HashSet<>(); // to keep track of dataType
Set<String> dataTypeSetIgnoringErasure = new HashSet<>();
int i = 0;
for (Schema xOfSchema : xOfCollection) {
CodegenProperty cp = fromProperty(collectionName + "_" + i, xOfSchema, false);
Expand All @@ -8452,7 +8453,7 @@ private List<CodegenProperty> getComposedProperties(List<Schema> xOfCollection,

if (dataTypeSet.contains(cp.dataType)
|| (isTypeErasedGenerics() && dataTypeSet.contains(cp.baseType))) {
// add "x-duplicated-data-type" to indicate if the dataType already occurs before
// add "x-duplicated-data-type" to indicate if the (base) dataType already occurs before
// in other sub-schemas of allOf/anyOf/oneOf
cp.vendorExtensions.putIfAbsent("x-duplicated-data-type", true);
} else {
Expand All @@ -8462,6 +8463,13 @@ private List<CodegenProperty> getComposedProperties(List<Schema> xOfCollection,
dataTypeSet.add(cp.dataType);
}
}
if (dataTypeSetIgnoringErasure.contains(cp.dataType)) {
// add "x-duplicated-data-type-ignoring-erasure" to indicate if the dataType already occurs before
// in other sub-schemas of allOf/anyOf/oneOf
cp.vendorExtensions.putIfAbsent("x-duplicated-data-type-ignoring-erasure", true);
} else {
dataTypeSetIgnoringErasure.add(cp.dataType);
}
}
return xOf;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ public void processOpts() {
convertPropertyToBooleanAndWriteBack(CONTAINER_DEFAULT_TO_NULL, this::setContainerDefaultToNull);

additionalProperties.put("sanitizeGeneric", (Mustache.Lambda) (fragment, writer) -> {
String content = fragment.execute();
String content = removeAnnotations(fragment.execute());
for (final String s: List.of("<", ">", ",", " ")) {
content = content.replace(s, "");
}
Expand Down Expand Up @@ -1793,16 +1793,20 @@ public void postProcessResponseWithProperty(CodegenResponse response, CodegenPro
return;
}

// the response data types should not contains a bean validation annotation.
// the response data types should not contain a bean validation annotation.
if (property.dataType.contains("@")) {
property.dataType = property.dataType.replaceAll("(?:(?i)@[a-z0-9]*+([(].*[)]|\\s*))*+", "");
property.dataType = removeAnnotations(property.dataType);
}
// the response data types should not contains a bean validation annotation.
// the response data types should not contain a bean validation annotation.
if (response.dataType.contains("@")) {
response.dataType = response.dataType.replaceAll("(?:(?i)@[a-z0-9]*+([(].*[)]|\\s*))*+", "");
response.dataType = removeAnnotations(response.dataType);
}
}

private String removeAnnotations(String type) {
return type.replaceAll("(?:(?i)@[a-z0-9]*+([(].*[)]|\\s*))*+", "");
}

@Override
public ModelsMap postProcessModels(ModelsMap objs) {
// recursively add import for mapping one type to multiple imports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,23 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return ret;
}
{{/discriminator}}
{{#composedSchemas}}
{{#anyOf}}
// deserialize {{{.}}}
// deserialize {{{dataType}}}{{#isNullable}} (nullable){{/isNullable}}
try {
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{.}}}.class);
{{^isArray}}
{{^isMap}}
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{dataType}}}.class);
{{/isMap}}
{{/isArray}}
{{#isArray}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isArray}}
{{#isMap}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isMap}}
{{classname}} ret = new {{classname}}();
ret.setActualInstance(deserialized);
return ret;
Expand All @@ -81,6 +94,7 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}

{{/anyOf}}
{{/composedSchemas}}
throw new IOException(String.format("Failed deserialization for {{classname}}: no match found"));
}

Expand Down Expand Up @@ -119,13 +133,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return Objects.hash(getActualInstance(), isNullable(), getSchemaType(), additionalProperties);
}
{{/additionalPropertiesType}}
{{#composedSchemas}}
{{#anyOf}}
public {{classname}}({{{.}}} o) {
{{^vendorExtensions.x-duplicated-data-type}}
public {{classname}}({{{baseType}}} o) {
super("anyOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
setActualInstance(o);
}

{{/vendorExtensions.x-duplicated-data-type}}
{{/anyOf}}
{{/composedSchemas}}
static {
{{#anyOf}}
schemas.put("{{{.}}}", new GenericType<{{{.}}}>() {
Expand Down Expand Up @@ -165,13 +183,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}

{{/isNullable}}
{{#composedSchemas}}
{{#anyOf}}
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}}
{{/anyOf}}
{{/composedSchemas}}
throw new RuntimeException("Invalid instance type. Must be {{#anyOf}}{{{.}}}{{^-last}}, {{/-last}}{{/anyOf}}");
}

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

{{#composedSchemas}}
{{#anyOf}}
{{^vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
/**
* 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();
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}

{{/vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
{{/anyOf}}
{{/composedSchemas}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}
{{/isPrimitiveType}}
if (attemptParsing) {
{{#isMap}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isMap}}
{{^isMap}}
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{dataType}}}.class);
{{/isMap}}
// 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.
Expand Down Expand Up @@ -266,24 +272,19 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im

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

{{/vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
{{/oneOf}}
{{/composedSchemas}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,23 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return ret;
}
{{/discriminator}}
{{#composedSchemas}}
{{#anyOf}}
// deserialize {{{.}}}
// deserialize {{{dataType}}}{{#isNullable}} (nullable){{/isNullable}}
try {
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{.}}}.class);
{{^isArray}}
{{^isMap}}
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{dataType}}}.class);
{{/isMap}}
{{/isArray}}
{{#isArray}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isArray}}
{{#isMap}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isMap}}
{{classname}} ret = new {{classname}}();
ret.setActualInstance(deserialized);
return ret;
Expand All @@ -81,6 +94,7 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}

{{/anyOf}}
{{/composedSchemas}}
throw new IOException(String.format("Failed deserialization for {{classname}}: no match found"));
}

Expand Down Expand Up @@ -119,13 +133,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return Objects.hash(getActualInstance(), isNullable(), getSchemaType(), additionalProperties);
}
{{/additionalPropertiesType}}
{{#composedSchemas}}
{{#anyOf}}
public {{classname}}({{{.}}} o) {
{{^vendorExtensions.x-duplicated-data-type}}
public {{classname}}({{{baseType}}} o) {
super("anyOf", {{#isNullable}}Boolean.TRUE{{/isNullable}}{{^isNullable}}Boolean.FALSE{{/isNullable}});
setActualInstance(o);
}

{{/vendorExtensions.x-duplicated-data-type}}
{{/anyOf}}
{{/composedSchemas}}
static {
{{#anyOf}}
schemas.put("{{{.}}}", new GenericType<{{{.}}}>() {
Expand Down Expand Up @@ -165,13 +183,17 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}

{{/isNullable}}
{{#composedSchemas}}
{{#anyOf}}
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}}
{{/anyOf}}
{{/composedSchemas}}
throw new RuntimeException("Invalid instance type. Must be {{#anyOf}}{{{.}}}{{^-last}}, {{/-last}}{{/anyOf}}");
}

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

{{#composedSchemas}}
{{#anyOf}}
{{^vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
/**
* 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();
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}

{{/vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
{{/anyOf}}
{{/composedSchemas}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
}
{{/isPrimitiveType}}
if (attemptParsing) {
{{#isMap}}
final TypeReference<{{{dataType}}}> ref = new TypeReference<{{{dataType}}}>(){};
deserialized = tree.traverse(jp.getCodec()).readValueAs(ref);
{{/isMap}}
{{^isMap}}
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{dataType}}}.class);
{{/isMap}}
// 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.
Expand Down Expand Up @@ -266,24 +272,19 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im

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

{{/vendorExtensions.x-duplicated-data-type-ignoring-erasure}}
{{/oneOf}}
{{/composedSchemas}}
}
Loading

0 comments on commit b730e36

Please sign in to comment.