-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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.
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
[Codegen] Convert "any type" to oneOf model #6051
Changes from all commits
0c3f479
b1d60d5
5d746ee
2710959
fcc47b8
9ebddb5
9045d65
1791d16
6097031
af6496f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1774,6 +1774,10 @@ public String toDefaultValueWithParam(String name, Schema schema) { | |
**/ | ||
@SuppressWarnings("static-method") | ||
public String getSchemaType(Schema schema) { | ||
if (schema == null) { | ||
// This is indicative of a bug in codegen. | ||
throw new RuntimeException("getSchemaType schema argument is null"); | ||
} | ||
if (schema instanceof ComposedSchema) { // composed schema | ||
ComposedSchema cs = (ComposedSchema) schema; | ||
// Get the interfaces, i.e. the set of elements under 'allOf', 'anyOf' or 'oneOf'. | ||
|
@@ -1886,7 +1890,9 @@ public String toOneOfName(List<String> names, ComposedSchema composedSchema) { | |
*/ | ||
private String getSingleSchemaType(Schema schema) { | ||
Schema unaliasSchema = ModelUtils.unaliasSchema(this.openAPI, schema, importMapping); | ||
|
||
if (unaliasSchema == null) { | ||
throw new RuntimeException("Schema '" + schema.getName() + "' is invalid"); | ||
} | ||
if (StringUtils.isNotBlank(unaliasSchema.get$ref())) { // reference to another definition/schema | ||
// get the schema/model name from $ref | ||
String schemaName = ModelUtils.getSimpleRef(unaliasSchema.get$ref()); | ||
|
@@ -2010,15 +2016,15 @@ public String getTypeDeclaration(String name) { | |
} | ||
|
||
/** | ||
* Output the type declaration of the property | ||
* Output the language-specific type declaration of the property. | ||
* | ||
* @param schema property schema | ||
* @return a string presentation of the property type | ||
*/ | ||
public String getTypeDeclaration(Schema schema) { | ||
if (schema == null) { | ||
LOGGER.warn("Null schema found. Default type to `NULL_SCHMEA_ERR`"); | ||
return "NULL_SCHMEA_ERR"; | ||
LOGGER.warn("Null schema found. Default type to `NULL_SCHEMA_ERR`"); | ||
return "NULL_SCHEMA_ERR"; | ||
} | ||
|
||
String oasType = getSchemaType(schema); | ||
|
@@ -2100,6 +2106,70 @@ public String toModelName(final String name) { | |
return camelize(modelNamePrefix + "_" + name + "_" + modelNameSuffix); | ||
} | ||
|
||
/** | ||
* Returns a composed model that encapsulates the JSON schema "any type". | ||
* Its value can be any of null, integer, boolean, number, string, array or map. | ||
*/ | ||
protected Schema getAnyTypeSchema(String name, Schema schema) { | ||
if (!ModelUtils.isAnyTypeSchema(schema)) { | ||
throw new RuntimeException("Schema '" + name + "' is not 'any type'"); | ||
} | ||
ComposedSchema cs = (ComposedSchema) new ComposedSchema() | ||
.addAnyOfItem(new ObjectSchema().type("null")) | ||
.addAnyOfItem(new BooleanSchema()) | ||
.addAnyOfItem(new StringSchema() | ||
.minLength(schema.getMinLength()) | ||
.maxLength(schema.getMaxLength()) | ||
.pattern(schema.getPattern()) | ||
) | ||
.addAnyOfItem(new IntegerSchema() | ||
.minimum(schema.getMinimum()) | ||
.maximum(schema.getMaximum()) | ||
.exclusiveMinimum(schema.getExclusiveMinimum()) | ||
.exclusiveMaximum(schema.getExclusiveMaximum()) | ||
.multipleOf(schema.getMultipleOf()) | ||
) | ||
.addAnyOfItem(new NumberSchema() | ||
.minimum(schema.getMinimum()) | ||
.maximum(schema.getMaximum()) | ||
.exclusiveMinimum(schema.getExclusiveMinimum()) | ||
.exclusiveMaximum(schema.getExclusiveMaximum()) | ||
.multipleOf(schema.getMultipleOf()) | ||
) | ||
.name(name); | ||
|
||
// The map keys must be strings and the values can be anything. | ||
cs.addAnyOfItem(new MapSchema() | ||
.additionalProperties(true) | ||
.minProperties(schema.getMinProperties()) | ||
.maxProperties(schema.getMaxProperties()) | ||
.required(schema.getRequired()) | ||
); | ||
// The array items can be anything. | ||
cs.addAnyOfItem(new ArraySchema() | ||
.minItems(schema.getMinItems()) | ||
.maxItems(schema.getMaxItems()) | ||
.uniqueItems(schema.getUniqueItems()) | ||
); | ||
if (schema != null) { | ||
cs.setTitle(schema.getTitle()); | ||
cs.setDescription(schema.getDescription()); | ||
} | ||
return cs; | ||
} | ||
|
||
// Returns a model that encapsulates the JSON schema "any type". Its value | ||
// can be any of null, integer, boolean, number, string, array or map. | ||
// "Any type" is a schema that does not have the "type" attribute | ||
// specified in the OpenAPI schema. That means the value can be any valid | ||
// payload, i.e. the null value, boolean, string, integer, number, | ||
// array or map. | ||
// Numerical payloads may match more than one type, for example "2" may | ||
// match integer and number. Hence the use of 'anyOf'. | ||
public CodegenModel getAnyTypeModel(String name, Schema schema) { | ||
return fromModel(name, getAnyTypeSchema(name, schema)); | ||
} | ||
|
||
/** | ||
* Convert OAS Model object to Codegen Model object | ||
* | ||
|
@@ -2121,6 +2191,11 @@ public CodegenModel fromModel(String name, Schema schema) { | |
return null; | ||
} | ||
|
||
if (ModelUtils.isAnyTypeSchema(schema)) { | ||
// "Any type" means the payload can be any type, e.g. integer, number, object, array... | ||
return getAnyTypeModel(name, schema); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code only covers the case when the user include AnyType as a model schema. What if we have AnyTypeSchema in:
For example if AnyType is only in a model property AND/or an inline response definition there is no Model/Schema that it should be $ref connected to. In my opinion, we should make this model so it can be handled by all generators. How about instead in the InlineModelResolver
|
||
|
||
CodegenModel m = CodegenModelFactory.newInstance(CodegenModelType.MODEL); | ||
|
||
if (reservedWords.contains(name)) { | ||
|
@@ -2155,7 +2230,6 @@ public CodegenModel fromModel(String name, Schema schema) { | |
m.xmlNamespace = schema.getXml().getNamespace(); | ||
m.xmlName = schema.getXml().getName(); | ||
} | ||
|
||
if (ModelUtils.isArraySchema(schema)) { | ||
m.isArrayModel = true; | ||
m.arrayModelType = fromProperty(name, schema).complexType; | ||
|
@@ -2801,7 +2875,7 @@ public String getterAndSetterCapitalize(String name) { | |
* Convert OAS Property object to Codegen Property object | ||
* | ||
* @param name name of the property | ||
* @param p OAS property object | ||
* @param p OAS property schema | ||
* @return Codegen Property object | ||
*/ | ||
public CodegenProperty fromProperty(String name, Schema p) { | ||
|
@@ -2814,6 +2888,9 @@ public CodegenProperty fromProperty(String name, Schema p) { | |
// unalias schema | ||
p = ModelUtils.unaliasSchema(this.openAPI, p, importMapping); | ||
|
||
if (ModelUtils.isAnyTypeSchema(p)) { | ||
p = getAnyTypeSchema(name, p); | ||
} | ||
CodegenProperty property = CodegenModelFactory.newInstance(CodegenModelType.PROPERTY); | ||
|
||
ModelUtils.syncValidationProperties(p, property); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -910,7 +910,14 @@ public String getTypeString(Schema p, String prefix, String suffix) { | |
} else if (ModelUtils.isArraySchema(p)) { | ||
ArraySchema ap = (ArraySchema) p; | ||
Schema inner = ap.getItems(); | ||
return prefix + "[" + getTypeString(inner, "", "") + "]" + fullSuffix; | ||
if (inner == null) { | ||
// Per JSON schema specification, the array "items" attribute is optional. | ||
// When "items" is not specified, the elements of the array | ||
// may be anything at all. | ||
return prefix + "[bool, date, datetime, dict, float, int, list, str, none_type]" + fullSuffix; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @spacether , can you help review this? Is this the right way to fix the issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this will only happen if we are in an array, right?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also need the |
||
} else { | ||
return prefix + "[" + getTypeString(inner, "", "") + "]" + fullSuffix; | ||
} | ||
} | ||
if (ModelUtils.isFileSchema(p)) { | ||
return prefix + "file_type" + fullSuffix; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I thought I should create a single "AnyType" schema, but in reality there may be corner cases where constraints other than "type" have been specified:
title
pattern
required
enum
minimum
maximum
exclusiveMinimum
exclusiveMaximum
multipleOf
minLength
maxLength
minItems
maxItems
uniqueItems
minProperties
maxProperties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But aren't those constraints specific to types? If you had minItems it would only apply to dict and array, right? My take is if they have any of those constraints then they should be fully explicit and list all types. We are just trying to cover this one super general yoy said it could be anything case. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That way that constraint question is the headache of specific generators. I agree with you about adding this model/schema once and then using it multiple places if the writers used it multiple places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am planning to handle these edge cases later. Right now just trying to make the simple case work, i.e. no OAS attribute whatsoever is defined in the OAS schema, it's really any type. Even this simple case is not so simple.
Also, the good news is most of these constraints are specific to a type. The only constraints that apply to all types are type, enum and const: https://tools.ietf.org/html/draft-handrews-json-schema-validation-02