Skip to content
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][Go] Fix compilation error of generated go code when schema is free form object #5391

Merged
merged 37 commits into from
Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5721959
Fix code generation for free-form objects in go-experimental
sebastien-rosset Feb 20, 2020
40470a6
Execute scripts in bin directory
sebastien-rosset Feb 20, 2020
1d12a00
Add more use cases for open-ended types
sebastien-rosset Feb 21, 2020
128b6ca
Add more use cases for open-ended types
sebastien-rosset Feb 21, 2020
946bdc6
Add more use cases for open-ended types
sebastien-rosset Feb 21, 2020
7af5f83
add code comments
sebastien-rosset Feb 21, 2020
f2e4418
Better name for test properties
sebastien-rosset Feb 21, 2020
de43542
handle scenario when type is arbitrary
sebastien-rosset Feb 21, 2020
0c117c4
handle interface{} scenario
sebastien-rosset Feb 21, 2020
9ba18ef
handle interface{} scenario
sebastien-rosset Feb 21, 2020
c5a29f5
add helper function isAnyType
sebastien-rosset Feb 21, 2020
821eaeb
isAnyType function
sebastien-rosset Feb 21, 2020
16259c7
implementation of isAnyType function
sebastien-rosset Feb 21, 2020
b0d0f4a
fix javadoc issue
sebastien-rosset Feb 21, 2020
1f33e83
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Feb 21, 2020
9471954
handle interface{} scenario
sebastien-rosset Feb 22, 2020
48fac66
Merge remote-tracking branch 'origin' into go-experimental-free-form
sebastien-rosset Feb 26, 2020
1661128
Merge remote-tracking branch 'origin' into go-experimental-free-form
sebastien-rosset Feb 29, 2020
ee2c1cc
use equals comparison instead of ==
sebastien-rosset Mar 1, 2020
1323017
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Mar 12, 2020
60529ee
resolve merge conflicts
sebastien-rosset Mar 17, 2020
840a209
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Mar 24, 2020
1668293
merge from master
sebastien-rosset Mar 24, 2020
76ffa9e
merge from master
sebastien-rosset Mar 24, 2020
5d7be16
Merge remote-tracking branch 'origin' into go-experimental-free-form
sebastien-rosset Mar 24, 2020
4b714ad
Add code documentation
sebastien-rosset Mar 24, 2020
6c5afdd
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Mar 25, 2020
5cb8407
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Mar 25, 2020
4db388f
add code comments, remove unused min/max attribute, fix equals method
sebastien-rosset Mar 25, 2020
42cc09a
Handle 'anytype' use case
sebastien-rosset Mar 25, 2020
c70f531
add code comments
sebastien-rosset Mar 25, 2020
49d833c
override postProcessModelProperty to set vendor extension
sebastien-rosset Mar 25, 2020
2eeb4e5
Use vendorExtensions.x-golang-is-container
sebastien-rosset Mar 25, 2020
6c4c5fc
fix compilation error of generated code
sebastien-rosset Mar 25, 2020
9051376
Merge branch 'master' of github.com:CiscoM31/openapi-generator into g…
sebastien-rosset Mar 25, 2020
f736657
fix compilation error of generated code
sebastien-rosset Mar 25, 2020
5d4796c
fix compilation error of generated code
sebastien-rosset Mar 25, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,15 @@ public class CodegenProperty implements Cloneable, IJsonSchemaValidationProperti
public boolean isUuid;
public boolean isUri;
public boolean isEmail;
/**
* The type is a free-form object, i.e. it is a map of string to values with no declared properties
*/
public boolean isFreeFormObject;
/**
* The 'type' in the OAS schema is unspecified (i.e. not set). The value can be number, integer, string, object or array.
* If the nullable attribute is set to true, the 'null' value is valid.
*/
public boolean isAnyType;
public boolean isListContainer;
public boolean isMapContainer;
public boolean isEnum;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1884,14 +1884,19 @@ private String getPrimitiveType(Schema schema) {
}
return "string";
} else if (ModelUtils.isFreeFormObject(schema)) {
// Note: the value of a free-form object cannot be an arbitrary type. Per OAS specification,
// it must be a map of string to values.
return "object";
} else if (schema.getProperties() != null && !schema.getProperties().isEmpty()) { // having property implies it's a model
return "object";
} else if (StringUtils.isNotEmpty(schema.getType())) {
LOGGER.warn("Unknown type found in the schema: " + schema.getType());
return schema.getType();
}

// The 'type' attribute has not been set in the OAS schema, which means the value
// can be an arbitrary type, e.g. integer, string, object, array, number...
// TODO: we should return a different value to distinguish between free-form object
// and arbitrary type.
return "object";
}

Expand Down Expand Up @@ -2647,6 +2652,9 @@ public CodegenProperty fromProperty(String name, Schema p) {
setNonArrayMapProperty(property, type);
Schema refOrCurrent = ModelUtils.getReferencedSchema(this.openAPI, p);
property.isModel = (ModelUtils.isComposedSchema(refOrCurrent) || ModelUtils.isObjectSchema(refOrCurrent)) && ModelUtils.isModel(refOrCurrent);
if (ModelUtils.isAnyTypeSchema(p)) {
property.isAnyType = true;
}
}

LOGGER.debug("debugging from property return: " + property);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ public AbstractGoCodegen() {
"complex64",
"complex128",
"rune",
"byte")
"byte",
"map[string]interface{}",
"interface{}"
)
);

instantiationTypes.clear();
Expand All @@ -116,7 +119,17 @@ public AbstractGoCodegen() {
typeMapping.put("file", "*os.File");
typeMapping.put("binary", "*os.File");
typeMapping.put("ByteArray", "string");
// A 'type: object' OAS schema without any declared property is
// (per JSON schema specification) "an unordered set of properties
// mapping a string to an instance".
// Hence map[string]interface{} is the proper implementation in golang.
// Note: OpenAPITools uses the same token 'object' for free-form objects
// and arbitrary types. A free form object is implemented in golang as
// map[string]interface{}, whereas an arbitrary type is implemented
// in golang as interface{}.
// See issue #5387 for more details.
typeMapping.put("object", "map[string]interface{}");
typeMapping.put("interface{}", "interface{}");

numberTypes = new HashSet<String>(
Arrays.asList(
Expand Down Expand Up @@ -286,6 +299,12 @@ public String toApiFilename(String name) {
return name;
}

/**
* Return the golang implementation type for the specified property.
*
* @param the OAS property.
* @return the golang implementation type.
*/
@Override
public String getTypeDeclaration(Schema p) {
if (ModelUtils.isArraySchema(p)) {
Expand Down Expand Up @@ -325,6 +344,12 @@ public String getTypeDeclaration(Schema p) {
return toModelName(openAPIType);
}

/**
* Return the OpenAPI type for the property.
*
* @param p the OAS property.
* @return the OpenAPI type.
*/
@Override
public String getSchemaType(Schema p) {
String openAPIType = super.getSchemaType(p);
Expand All @@ -333,6 +358,9 @@ public String getSchemaType(Schema p) {

if (ref != null && !ref.isEmpty()) {
type = openAPIType;
} else if (openAPIType == "object" && ModelUtils.isAnyTypeSchema(p)) {
sebastien-rosset marked this conversation as resolved.
Show resolved Hide resolved
// Arbitrary type. Note this is not the same thing as free-form object.
type = "interface{}";
} else if (typeMapping.containsKey(openAPIType)) {
type = typeMapping.get(openAPIType);
if (languageSpecificPrimitives.contains(type))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ public Map<String, Object> postProcessModels(Map<String, Object> objs) {
}

for (CodegenProperty param : model.vars) {
if (!param.isNullable || param.isMapContainer || param.isListContainer) {
if (!param.isNullable || param.isMapContainer || param.isListContainer ||
param.isFreeFormObject || param.isAnyType) {
continue;
}
if (param.isDateTime) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ public static boolean isEmailSchema(Schema schema) {
}

/**
* Check to see if the schema is a model with at least one properties
* Check to see if the schema is a model with at least one property.
*
* @param schema potentially containing a '$ref'
* @return true if it's a model with at least one properties
Expand All @@ -658,13 +658,68 @@ public static boolean isModel(Schema schema) {
}

/**
* Check to see if the schema is a free form object.
* Return true if the schema value can be any type, i.e. it can be
* the null value, integer, number, string, object or array.
* One use case is when the "type" attribute in the OAS schema is unspecified.
*
* Examples:
*
* arbitraryTypeValue:
* description: This is an arbitrary type schema.
* It is not a free-form object.
* The value can be any type except the 'null' value.
* arbitraryTypeNullableValue:
* description: This is an arbitrary type schema.
* It is not a free-form object.
* The value can be any type, including the 'null' value.
* nullable: true
*
* @param schema the OAS schema.
* @return true if the schema value can be an arbitrary type.
*/
public static boolean isAnyTypeSchema(Schema schema) {
if (schema == null) {
once(LOGGER).error("Schema cannot be null in isAnyTypeSchema check");
return false;
}
if (schema.getClass().equals(Schema.class) && schema.get$ref() == null && schema.getType() == null &&
(schema.getProperties() == null || schema.getProperties().isEmpty()) &&
schema.getAdditionalProperties() == null && schema.getNot() == null &&
schema.getEnum() == null) {
return true;
// If and when type arrays are supported in a future OAS specification,
// we could return true if the type array includes all possible JSON schema types.
}
return false;
}

/**
* Check to see if the schema is a free-form object.
*
* A free form object is an object (i.e. 'type: object' in a OAS document) that:
* 1) Does not define properties, and
* 2) Is not a composed schema (no anyOf, oneOf, allOf), and
* 3) additionalproperties is not defined, or additionalproperties: true, or additionalproperties: {}.
*
* Examples:
*
* components:
* schemas:
* arbitraryObject:
* type: object
* description: This is a free-form object.
* The value must be a map of strings to values. The value cannot be 'null'.
* It cannot be array, string, integer, number.
* arbitraryNullableObject:
* type: object
* description: This is a free-form object.
* The value must be a map of strings to values. The value can be 'null',
* It cannot be array, string, integer, number.
* nullable: true
* arbitraryTypeValue:
* description: This is NOT a free-form object.
* The value can be any type except the 'null' value.
*
* @param schema potentially containing a '$ref'
* @return true if it's a free-form object
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,23 @@ components:
type: integer
format: int32
description: User Status
arbitraryObject:
type: object
description: test code generation for objects
Value must be a map of strings to values. It cannot be the 'null' value.
arbitraryNullableObject:
type: object
description: test code generation for nullable objects.
Value must be a map of strings to values or the 'null' value.
nullable: true
arbitraryTypeValue:
description: test code generation for any type
Value can be any type - string, number, boolean, array or object.
arbitraryNullableTypeValue:
description: test code generation for any type
Value can be any type - string, number, boolean, array, object or
the 'null' value.
nullable: true
xml:
name: User
Tag:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1299,9 +1299,13 @@ components:
lastName: lastName
password: password
userStatus: 6
arbitraryTypeValue: ""
arbitraryNullableTypeValue: ""
phone: phone
id: 0
arbitraryObject: '{}'
email: email
arbitraryNullableObject: '{}'
username: username
properties:
id:
Expand All @@ -1324,6 +1328,22 @@ components:
description: User Status
format: int32
type: integer
arbitraryObject:
description: test code generation for objects Value must be a map of strings
to values. It cannot be the 'null' value.
type: object
arbitraryNullableObject:
description: test code generation for nullable objects. Value must be a
map of strings to values or the 'null' value.
nullable: true
type: object
arbitraryTypeValue:
description: test code generation for any type Value can be any type - string,
number, boolean, array or object.
arbitraryNullableTypeValue:
description: test code generation for any type Value can be any type - string,
number, boolean, array, object or the 'null' value.
nullable: true
type: object
xml:
name: User
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ Name | Type | Description | Notes
**StringProp** | Pointer to **NullableString** | | [optional]
**DateProp** | Pointer to **NullableString** | | [optional]
**DatetimeProp** | Pointer to [**NullableTime**](time.Time.md) | | [optional]
**ArrayNullableProp** | Pointer to [**[]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ArrayAndItemsNullableProp** | Pointer to [**[]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ArrayItemsNullable** | Pointer to [**[]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ObjectNullableProp** | Pointer to [**map[string]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ObjectAndItemsNullableProp** | Pointer to [**map[string]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ObjectItemsNullable** | Pointer to [**map[string]map[string]interface{}**](map[string]interface{}.md) | | [optional]
**ArrayNullableProp** | Pointer to **[]map[string]interface{}** | | [optional]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug before this PR. There is no markdown file "map[string]interface{}.md"

**ArrayAndItemsNullableProp** | Pointer to **[]map[string]interface{}** | | [optional]
**ArrayItemsNullable** | Pointer to **[]map[string]interface{}** | | [optional]
**ObjectNullableProp** | Pointer to **map[string]map[string]interface{}** | | [optional]
**ObjectAndItemsNullableProp** | Pointer to **map[string]map[string]interface{}** | | [optional]
**ObjectItemsNullable** | Pointer to **map[string]map[string]interface{}** | | [optional]

## Methods

Expand Down
Loading