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

wing328 · web-flow · commit b2f622cd983d · 2024-01-23T12:06:38.000+08:00
* fix map issue in 3.1 spec

* fix, add tests

* update samples

* update

* manully fix spec

* revert

* fix rust model
diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/IJsonSchemaValidationProperties.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/IJsonSchemaValidationProperties.java
@@ -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);
         }
diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
@@ -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());
+        }
     }
 
     /**
@@ -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());
+        }
     }
 
     /**
@@ -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);
diff --git a/modules/openapi-generator/src/main/resources/rust/model.mustache b/modules/openapi-generator/src/main/resources/rust/model.mustache
@@ -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}}
         }
     }
diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java
@@ -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")));
+    }
 }
diff --git a/modules/openapi-generator/src/test/resources/3_1/regression-16119.yaml b/modules/openapi-generator/src/test/resources/3_1/regression-16119.yaml
@@ -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:
diff --git a/modules/openapi-generator/src/test/resources/3_1/schema.yaml b/modules/openapi-generator/src/test/resources/3_1/schema.yaml
@@ -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
diff --git a/samples/client/others/rust/reqwest-regression-16119/Cargo.toml b/samples/client/others/rust/reqwest-regression-16119/Cargo.toml
@@ -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"] }
diff --git a/samples/client/others/rust/reqwest-regression-16119/src/models/parent.rs b/samples/client/others/rust/reqwest-regression-16119/src/models/parent.rs
@@ -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 {

Original file line number	Diff line number	Diff line change
`@@ -329,6 +329,8 @@ default void setTypeProperties(Schema p) {`
`329`	`329`	`setIsFreeFormObject(true);`
`330`	`330`	`// TODO: remove below later after updating generators to properly use isFreeFormObject`
`331`	`331`	`setIsMap(true);`
	`332`	`+ } else if (ModelUtils.isMapSchema(p)) {`
	`333`	`+ setIsMap(true);`
`332`	`334`	`} else if (ModelUtils.isTypeObjectSchema(p)) {`
`333`	`335`	`setIsMap(true);`
`334`	`336`	`}`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ impl {{{classname}}} {`
`82`	`82`	`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}}} {`
`83`	`83`	`{{{classname}}} {`
`84`	`84`	`{{#vars}}`
`85`		`- {{{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}},`
	`85`	`+ {{{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}},`
`86`	`86`	`{{/vars}}`
`87`	`87`	`}`
`88`	`88`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,8 @@`
`13`	`13`
`14`	`14`	`#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]`
`15`	`15`	`pub struct Parent {`
`16`		`- #[serde(rename = "child", default, with = "::serde_with::rust::double_option", skip_serializing_if = "Option::is_none")]`
`17`		`- pub child: Option<Option<::std::collections::HashMap<String, serde_json::Value>>>,`
	`16`	`+ #[serde(rename = "child", skip_serializing_if = "Option::is_none")]`
	`17`	`+ pub child: Option<::std::collections::HashMap<String, serde_json::Value>>,`
`18`	`18`	`}`
`19`	`19`
`20`	`20`	`impl Parent {`