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

Fix map and free form object detection issue in 3.1 spec #17624

Merged
merged 7 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -329,6 +329,8 @@ default void setTypeProperties(Schema p) {
setIsFreeFormObject(true);
// TODO: remove below later after updating generators to properly use isFreeFormObject
setIsMap(true);
} else if (ModelUtils.isMapSchema(p)) {
setIsMap(true);
} else if (ModelUtils.isTypeObjectSchema(p)) {
setIsMap(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,16 @@ public static String getSimpleRef(String ref) {
* @return true if the specified schema is an Object schema.
*/
public static boolean isTypeObjectSchema(Schema schema) {
return SchemaTypeUtil.OBJECT_TYPE.equals(schema.getType());
if (schema instanceof JsonSchema) { // 3.1 spec
if (schema.getTypes() != null && schema.getTypes().size() == 1) {
return SchemaTypeUtil.OBJECT_TYPE.equals(schema.getTypes().iterator().next());
} else {
// null type or multiple types, e.g. [string, integer]
return false;
}
} else { // 3.0.x or 2.0 spec
return SchemaTypeUtil.OBJECT_TYPE.equals(schema.getType());
}
}

/**
Expand Down Expand Up @@ -567,9 +576,14 @@ public static boolean isMapSchema(Schema schema) {
return false;
}

return (schema instanceof MapSchema) ||
(schema.getAdditionalProperties() instanceof Schema) ||
(schema.getAdditionalProperties() instanceof Boolean && (Boolean) schema.getAdditionalProperties());
if (schema instanceof JsonSchema) { // 3.1 spec
return ((schema.getAdditionalProperties() instanceof JsonSchema) ||
(schema.getAdditionalProperties() instanceof Boolean && (Boolean) schema.getAdditionalProperties()));
} else { // 3.0 or 2.x spec
return (schema instanceof MapSchema) ||
(schema.getAdditionalProperties() instanceof Schema) ||
(schema.getAdditionalProperties() instanceof Boolean && (Boolean) schema.getAdditionalProperties());
}
}

/**
Expand Down Expand Up @@ -790,6 +804,31 @@ public static boolean isFreeFormObject(Schema schema) {
return false;
}

if (schema instanceof JsonSchema) { // 3.1 spec
if (isComposedSchema(schema)) { // composed schema, e.g. allOf, oneOf, anyOf
return false;
}

if (schema.getProperties() != null && !schema.getProperties().isEmpty()) { // has properties
return false;
}

if (schema.getAdditionalProperties() instanceof Boolean && (Boolean) schema.getAdditionalProperties()) {
return true;
} else if (schema.getAdditionalProperties() instanceof JsonSchema) {
return true;
} else if (schema.getTypes() != null) {
if (schema.getTypes().size() == 1) { // types = [object]
return SchemaTypeUtil.OBJECT_TYPE.equals(schema.getTypes().iterator().next());
} else { // has more than 1 type, e.g. types = [integer, string]
return false;
}
}

return false;
}

// 3.0.x spec or 2.x spec
// not free-form if allOf, anyOf, oneOf is not empty
if (isComposedSchema(schema)) {
List<Schema> interfaces = ModelUtils.getInterfaces(schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl {{{classname}}} {
pub fn new({{#requiredVars}}{{{name}}}: {{#isNullable}}Option<{{/isNullable}}{{#isEnum}}{{#isArray}}{{#uniqueItems}}std::collections::HashSet<{{/uniqueItems}}{{^uniqueItems}}Vec<{{/uniqueItems}}{{/isArray}}{{{enumName}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isNullable}}>{{/isNullable}}{{^-last}}, {{/-last}}{{/requiredVars}}) -> {{{classname}}} {
{{{classname}}} {
{{#vars}}
{{{name}}}{{^required}}{{#isContainer}}{{#isArray}}: None{{/isArray}}{{#isMap}}: None{{/isMap}}{{^isArray}}{{^isMap}}{{#isNullable}}: None{{/isNullable}}{{/isMap}}{{/isArray}}{{/isContainer}}{{^isContainer}}: None{{/isContainer}}{{/required}}{{#required}}{{#isModel}}: {{^isNullable}}Box::new({{{name}}}){{/isNullable}}{{#isNullable}}if let Some(x) = {{{name}}} {Some(Box::new(x))} else {None}{{/isNullable}}{{/isModel}}{{/required}},
{{{name}}}{{^required}}{{#isFreeFormObject}}: None{{/isFreeFormObject}}{{#isContainer}}{{#isArray}}: None{{/isArray}}{{#isMap}}: None{{/isMap}}{{^isArray}}{{^isMap}}{{#isNullable}}: None{{/isNullable}}{{/isMap}}{{/isArray}}{{/isContainer}}{{^isContainer}}: None{{/isContainer}}{{/required}}{{#required}}{{#isModel}}: {{^isNullable}}Box::new({{{name}}}){{/isNullable}}{{#isNullable}}if let Some(x) = {{{name}}} {Some(Box::new(x))} else {None}{{/isNullable}}{{/isModel}}{{/required}},
{{/vars}}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,4 +294,19 @@ public void testSimpleRefDecoding() {
String decoded = ModelUtils.getSimpleRef("#/components/~01%20Hallo~1Welt");
Assert.assertEquals(decoded, "~1 Hallo/Welt");
}

// 3.1 spec test
@Test
public void testIsMapSchema() {
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_1/schema.yaml");
Schema misc = ModelUtils.getSchema(openAPI, "Misc");

// test map
Assert.assertTrue(ModelUtils.isMapSchema((Schema) misc.getProperties().get("map1")));

// test free form object
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_1")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_2")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_3")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ openapi: 3.1.0
info:
title: double-option-hashmap
version: 0.1.0
license:
name: MIT
identifier: MIT
servers:
- url: http://api.example.xyz/v1
paths:
Expand Down
53 changes: 53 additions & 0 deletions modules/openapi-generator/src/test/resources/3_1/schema.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
openapi: 3.1.0
servers:
- url: 'http://petstore.swagger.io/v2'
info:
description: >-
This is a sample server Petstore server. For this sample, you can use the api key
`special-key` to test the authorization filters.
version: 1.0.0
title: OpenAPI Petstore
license:
name: Apache-2.0
url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
tags:
- name: pet
description: Everything about your Pets
- name: store
description: Access to Petstore orders
- name: user
description: Operations about user
paths:
/dummy:
post:
tags:
- dummy
summary: dummy operation
description: ''
operationId: dummy_operation
responses:
'200':
description: successful operation
'405':
description: Invalid input
externalDocs:
description: Find out more about Swagger
url: 'http://swagger.io'
components:
schemas:
Misc:
description: Schema tests
type: object
properties:
free_form_object_1:
type: object
free_form_object_2:
type: object
additionalProperties: true
free_form_object_3:
type: object
additionalProperties: {}
map1:
type: object
additionalProperties:
type: string
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ name = "regression-16119-reqwest"
version = "0.1.0"
authors = ["OpenAPI Generator team and contributors"]
description = "No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)"
license = "MIT"
# Override this license by providing a License Object in the OpenAPI.
license = "Unlicense"
edition = "2018"

[dependencies]
serde = "^1.0"
serde_derive = "^1.0"
serde_with = "^2.0"
serde_json = "^1.0"
url = "^2.2"
uuid = { version = "^1.0", features = ["serde", "v4"] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct Parent {
#[serde(rename = "child", default, with = "::serde_with::rust::double_option", skip_serializing_if = "Option::is_none")]
pub child: Option<Option<::std::collections::HashMap<String, serde_json::Value>>>,
#[serde(rename = "child", skip_serializing_if = "Option::is_none")]
pub child: Option<::std::collections::HashMap<String, serde_json::Value>>,
}

impl Parent {
Expand Down
Loading