Skip to content

Fix #19439 and generated Java code for oneOf/anyOf models and enums #19443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
beikov wants to merge 9 commits into from
Closed

Fix #19439 and generated Java code for oneOf/anyOf models and enums #19443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
62d2177
Test anyOf with array and oneOf parameter as well as map in request body
beikov Sep 28, 2024
97bd3b7
Fix oneOf/anyOf support for jersey2 and jersey3
beikov Aug 23, 2024
d1948aa
Remove type annotations in sanitizeGeneric
beikov Sep 29, 2024
6fca1ef
[#19439] Don't assume schema is null type if ref is not null in OpenA…
beikov Aug 24, 2024
38f93c9
Test boolean property with enum
beikov Sep 29, 2024
794bf4d
Fix invalid Java Enum generation
beikov Aug 24, 2024
644c46c
Update samples
beikov Aug 24, 2024
af257b1
Reproduce oneOf required request body issue
beikov Sep 29, 2024
9efe5ce
WIP attempt to fix oneOf required request body issue
beikov Sep 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3155,8 +3155,12 @@ protected void setAddProps(Schema schema, IJsonSchemaValidationProperties proper
}
} else {
// if additioanl properties is set (e.g. free form object, any type, string, etc)
addPropProp = fromProperty(getAdditionalPropertiesName(), (Schema) schema.getAdditionalProperties(), false);
additionalPropertiesIsAnyType = true;
Schema additionalPropertiesSchema = (Schema) schema.getAdditionalProperties();
// Only consider the schema if it is not explicitly false
if (!Boolean.FALSE.equals(additionalPropertiesSchema.getBooleanSchemaValue())) {
addPropProp = fromProperty( getAdditionalPropertiesName(), additionalPropertiesSchema, false );
additionalPropertiesIsAnyType = true;
}
}
if (additionalPropertiesIsAnyType) {
property.setAdditionalPropertiesIsAnyType(true);
Expand Down
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ public boolean isNullTypeSchema(Schema schema) {
}

if (schema instanceof JsonSchema) { // 3.1 spec
if (Boolean.TRUE.equals(schema.getNullable())) {
if (Boolean.TRUE.equals(schema.getNullable()) && schema.get$ref() == null) {

@martin-mfg martin-mfg Sep 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add to bin/configs/ a small example of a case where this change prevents the generation of invalid code?
Thanks.

@beikov beikov Sep 29, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried long and hard, but couldn't reproduce this within the sample specs. Can you take a look? Just take this spec and you will see the error in e.g. model.RepositoryWebhooks, where you will see AnyOf license instead of SimpleLicense license.

return true;
}

Expand Down
10 changes: 7 additions & 3 deletions ...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,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 @@ -1794,14 +1794,18 @@ public void postProcessResponseWithProperty(CodegenResponse response, CodegenPro

// the response data types should not contains 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.
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
16 changes: 9 additions & 7 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ public static boolean isMapSchema(Schema schema) {
}

// additionalProperties explicitly set to false
if (schema.getAdditionalProperties() instanceof Boolean && Boolean.FALSE.equals(schema.getAdditionalProperties())) {
if (getAdditionalProperties(schema) == null) {
return false;
}

Expand Down Expand Up @@ -794,8 +794,7 @@ public static boolean isModelWithPropertiesOnly(Schema schema) {
// has properties
(null != schema.getProperties() && !schema.getProperties().isEmpty()) &&
// no additionalProperties is set
(schema.getAdditionalProperties() == null ||
(schema.getAdditionalProperties() instanceof Boolean && !(Boolean) schema.getAdditionalProperties()));
getAdditionalProperties(schema) == null;
}

public static boolean hasValidation(Schema sc) {
Expand Down Expand Up @@ -863,9 +862,7 @@ public static boolean isFreeFormObject(Schema schema, OpenAPI openAPI) {
return false;
}

if (schema.getAdditionalProperties() instanceof Boolean && (Boolean) schema.getAdditionalProperties()) {
return true;
} else if (schema.getAdditionalProperties() instanceof JsonSchema) {
if (getAdditionalProperties(schema) != null) {
return true;
} else if (schema.getTypes() != null) {
if (schema.getTypes().size() == 1) { // types = [object]
Expand Down Expand Up @@ -1421,7 +1418,12 @@ public static Schema unaliasSchema(OpenAPI openAPI,
public static Schema getAdditionalProperties(Schema schema) {
Object addProps = schema.getAdditionalProperties();
if (addProps instanceof Schema) {
return (Schema) addProps;
Schema additionalPropertiesSchema = (Schema) addProps;
// Only consider the schema if it is not explicitly false
if (!Boolean.FALSE.equals(additionalPropertiesSchema.getBooleanSchemaValue())) {
return additionalPropertiesSchema;
}
return null;
}
if (addProps == null) {
// When reaching this code path, this should indicate the 'additionalProperties' keyword is
Expand Down
118 changes: 104 additions & 14 deletions modules/openapi-generator/src/main/resources/Java/libraries/jersey2/anyof_model.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import java.util.HashSet;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
Expand Down Expand Up @@ -68,19 +70,83 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return ret;
}
{{/discriminator}}
boolean typeCoercion = ctxt.isEnabled(MapperFeature.ALLOW_COERCION_OF_SCALARS);
JsonToken token = tree.traverse(jp.getCodec()).nextToken();
{{#composedSchemas}}
{{#anyOf}}
// deserialize {{{.}}}
// deserialize {{{dataType}}}{{#isNullable}} (nullable){{/isNullable}}
try {
deserialized = tree.traverse(jp.getCodec()).readValueAs({{{.}}}.class);
{{^isArray}}
boolean attemptParsing = true;
{{#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) {
{{#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.
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.
log.log(Level.FINER, "Input data matches schema '{{{dataType}}}'");
}
{{/isArray}}
{{classname}} ret = new {{classname}}();
ret.setActualInstance(deserialized);
return ret;
} catch (Exception e) {
// deserialization failed, continue, log to help debugging
log.log(Level.FINER, "Input data does not match '{{classname}}'", e);
// deserialization failed, continue
log.log(Level.FINER, "Input data does not match schema '{{{dataType}}}'", e);
}

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

Expand Down Expand Up @@ -119,13 +185,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 +235,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 +260,33 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
return super.getActualInstance();
}

{{#composedSchemas}}
{{#anyOf}}
/**
* Get the actual instance of `{{{.}}}`. If the actual instance is not `{{{.}}}`,
* the ClassCastException will be thrown.
*
* @return The actual instance of `{{{.}}}`
* @throws ClassCastException if the instance is not `{{{.}}}`
*/
public {{{.}}} get{{{.}}}() throws ClassCastException {
return ({{{.}}})super.getActualInstance();
* 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}}
{{^isMap}}
public {{{dataType}}} get{{{dataType}}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isMap}}
{{#isMap}}
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isMap}}
{{/isArray}}
{{#isArray}}
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isArray}}

{{/anyOf}}
{{/composedSchemas}}
}
17 changes: 15 additions & 2 deletions modules/openapi-generator/src/main/resources/Java/libraries/jersey2/oneof_model.mustache
View file
Open in desktop
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 @@ -274,14 +280,21 @@ public class {{classname}} extends AbstractOpenApiSchema{{#vendorExtensions.x-im
* @throws ClassCastException if the instance is not `{{{dataType}}}`
*/
{{^isArray}}
{{^isMap}}
public {{{dataType}}} get{{{dataType}}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isMap}}
{{#isMap}}
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
{{/isMap}}
{{/isArray}}
{{#isArray}}
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
public {{{dataType}}} get{{#sanitizeGeneric}}{{{dataType}}}{{/sanitizeGeneric}}() throws ClassCastException {
return ({{{dataType}}})super.getActualInstance();
}
}
{{/isArray}}

{{/oneOf}}
Expand Down
Loading