From e38ea578f879d1a86972ee3c70dfc0d4e34e10eb Mon Sep 17 00:00:00 2001 From: William Cheng Date: Tue, 7 Mar 2023 15:53:53 +0800 Subject: [PATCH] Better support of inline allOf/anyOf/oneOf schemas (#14887) * better support of inilne allof schemas * add test file * better handling of anyof, oneof inline schemas * update samples * add tests for nested anyof * better code format --- .../codegen/InlineModelResolver.java | 29 +++--- .../codegen/utils/ModelUtils.java | 64 +++++++++++++ .../codegen/InlineModelResolverTest.java | 25 +++++ .../inline_model_allof_propertyof_oneof.yaml | 56 +++++++++++ .../src/test/resources/3_0/nested_anyof.yaml | 16 ++++ .../openapi-v3/.openapi-generator/FILES | 2 - .../rust-server/output/openapi-v3/README.md | 2 - .../output/openapi-v3/api/openapi.yaml | 24 ++--- .../output/openapi-v3/src/models.rs | 92 ------------------- 9 files changed, 188 insertions(+), 122 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/inline_model_allof_propertyof_oneof.yaml create mode 100644 modules/openapi-generator/src/test/resources/3_0/nested_anyof.yaml diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java index f8b67f47cd76..1d11972b44d9 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java @@ -224,7 +224,7 @@ private boolean isModelNeeded(Schema schema, Set visitedSchemas) { } if (m.getAllOf() != null && !m.getAllOf().isEmpty()) { - // check to ensure at least of the allOf item is model + // check to ensure at least one of the allOf item is model for (Schema inner : m.getAllOf()) { if (isModelNeeded(ModelUtils.getReferencedSchema(openAPI, inner), visitedSchemas)) { return true; @@ -385,6 +385,7 @@ private void gatherInlineModels(Schema schema, String modelPrefix) { } } m.setAnyOf(newAnyOf); + } if (m.getOneOf() != null) { List newOneOf = new ArrayList(); @@ -541,15 +542,15 @@ private void flattenResponses(String modelName, Operation operation) { * allOf: * - $ref: '#/components/schemas/Animal' * - type: object - * properties: - * name: - * type: string - * age: - * type: string + * properties: + * name: + * type: string + * age: + * type: string * - type: object - * properties: - * breed: - * type: string + * properties: + * breed: + * type: string * * @param key a unique name ofr the composed schema. * @param children the list of nested schemas within a composed schema (allOf, anyOf, oneOf). @@ -577,6 +578,8 @@ private void flattenComposedChildren(String key, List children) { // instead of inline. String innerModelName = resolveModelName(component.getTitle(), key); Schema innerModel = modelFromProperty(openAPI, component, innerModelName); + // Recurse to create $refs for inner models + gatherInlineModels(innerModel, innerModelName); String existing = matchGenerated(innerModel); if (existing == null) { innerModelName = addSchemas(innerModelName, innerModel); @@ -604,13 +607,17 @@ private void flattenComponents() { List modelNames = new ArrayList(models.keySet()); for (String modelName : modelNames) { Schema model = models.get(modelName); - if (ModelUtils.isComposedSchema(model)) { + if (ModelUtils.isAnyOf(model)) { // contains anyOf only + gatherInlineModels(model, modelName); + } else if (ModelUtils.isOneOf(model)) { // contains oneOf only + gatherInlineModels(model, modelName); + } else if (ModelUtils.isComposedSchema(model)) { ComposedSchema m = (ComposedSchema) model; // inline child schemas flattenComposedChildren(modelName + "_allOf", m.getAllOf()); flattenComposedChildren(modelName + "_anyOf", m.getAnyOf()); flattenComposedChildren(modelName + "_oneOf", m.getOneOf()); - } else if (model instanceof Schema) { + } else { gatherInlineModels(model, modelName); } } 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 index 580e53121e50..a392325eb815 100644 --- 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 @@ -1885,6 +1885,70 @@ public static boolean hasAllOf(Schema schema) { return false; } + /** + * Returns true if the schema contains oneOf but + * no properties/allOf/anyOf defined. + * + * @param schema the schema + * @return true if the schema contains oneOf but no properties/allOf/anyOf defined. + */ + public static boolean isOneOf(Schema schema) { + if (hasOneOf(schema) && (schema.getProperties() == null || schema.getProperties().isEmpty()) && + (schema.getAllOf() == null || schema.getAllOf().isEmpty()) && + (schema.getAnyOf() == null || schema.getAnyOf().isEmpty())) { + return true; + } + + return false; + } + + /** + * Returns true if the schema contains oneOf and may or may not have + * properties/allOf/anyOf defined. + * + * @param schema the schema + * @return true if allOf is not empty + */ + public static boolean hasOneOf(Schema schema) { + if (schema.getOneOf() != null && !schema.getOneOf().isEmpty()) { + return true; + } + + return false; + } + + /** + * Returns true if the schema contains anyOf but + * no properties/allOf/anyOf defined. + * + * @param schema the schema + * @return true if the schema contains oneOf but no properties/allOf/anyOf defined. + */ + public static boolean isAnyOf(Schema schema) { + if (hasAnyOf(schema) && (schema.getProperties() == null || schema.getProperties().isEmpty()) && + (schema.getAllOf() == null || schema.getAllOf().isEmpty()) && + (schema.getOneOf() == null || schema.getOneOf().isEmpty())) { + return true; + } + + return false; + } + + /** + * Returns true if the schema contains anyOf and may or may not have + * properties/allOf/oneOf defined. + * + * @param schema the schema + * @return true if anyOf is not empty + */ + public static boolean hasAnyOf(Schema schema) { + if (schema.getAnyOf() != null && !schema.getAnyOf().isEmpty()) { + return true; + } + + return false; + } + /** * Returns true if any of the common attributes of the schema (e.g. readOnly, default, maximum, etc) is defined. * diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java index f21dcb2202e5..c34cd38df425 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java @@ -1103,4 +1103,29 @@ public void resolveInlineRequestBodyAllOf() { //RequestBody referencedRequestBody = ModelUtils.getReferencedRequestBody(openAPI, requestBodyReference); //assertTrue(referencedRequestBody.getRequired()); } + + @Test + public void testInlineSchemaAllOfPropertyOfOneOf() { + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/inline_model_allof_propertyof_oneof.yaml"); + InlineModelResolver resolver = new InlineModelResolver(); + resolver.flatten(openAPI); + + Schema schema = openAPI.getComponents().getSchemas().get("Order_allOf_inline_oneof"); + assertEquals(((Schema) schema.getOneOf().get(0)).get$ref(), "#/components/schemas/Tag"); + assertEquals(((Schema) schema.getOneOf().get(1)).get$ref(), "#/components/schemas/Filter"); + + Schema schema2 = openAPI.getComponents().getSchemas().get("Order_allOf_inline_model"); + assertTrue(schema2.getProperties().get("something") instanceof StringSchema); + } + + @Test + public void testNestedAnyOf() { + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/nested_anyof.yaml"); + InlineModelResolver resolver = new InlineModelResolver(); + resolver.flatten(openAPI); + + Schema schema = openAPI.getComponents().getSchemas().get("SomeData_anyOf"); + assertTrue((Schema) schema.getAnyOf().get(0) instanceof StringSchema); + assertTrue((Schema) schema.getAnyOf().get(1) instanceof IntegerSchema); + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/inline_model_allof_propertyof_oneof.yaml b/modules/openapi-generator/src/test/resources/3_0/inline_model_allof_propertyof_oneof.yaml new file mode 100644 index 000000000000..c8869707fef9 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/inline_model_allof_propertyof_oneof.yaml @@ -0,0 +1,56 @@ +openapi: 3.0.1 +info: + title: ping test + version: '1.0' +servers: + - url: 'http://localhost:8000/' +paths: + /ping: + get: + operationId: pingGet + responses: + '201': + description: OK +components: + schemas: + Order: + title: Pet Order + description: An order for a pets from the pet store + allOf: + - $ref: "#/components/schemas/Tag" + - type: object + properties: + length: + type: integer + format: int64 + shipDate: + type: string + format: date-time + inline_oneof: + oneOf: + - $ref: "#/components/schemas/Tag" + - $ref: "#/components/schemas/Filter" + inline_model: + properties: + something: + type: string + Tag: + title: Pet Tag + description: A tag for a pet + type: object + properties: + id: + type: integer + format: int64 + name: + type: string + Filter: + title: Pet Tag + description: A tag for a pet + type: object + properties: + fid: + type: integer + format: int64 + fname: + type: string diff --git a/modules/openapi-generator/src/test/resources/3_0/nested_anyof.yaml b/modules/openapi-generator/src/test/resources/3_0/nested_anyof.yaml new file mode 100644 index 000000000000..da3c88bd0380 --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/nested_anyof.yaml @@ -0,0 +1,16 @@ +openapi: 3.0.0 +info: + version: '1.0.0' + title: 'Problem example with nested anyOf' +paths: {} +components: + schemas: + NullOne: + enum: + - null + SomeData: + anyOf: + - anyOf: + - type: string + - type: integer + - $ref: '#/components/schemas/NullOne' diff --git a/samples/server/petstore/rust-server/output/openapi-v3/.openapi-generator/FILES b/samples/server/petstore/rust-server/output/openapi-v3/.openapi-generator/FILES index b0d6b6c83950..995168af5ff5 100644 --- a/samples/server/petstore/rust-server/output/openapi-v3/.openapi-generator/FILES +++ b/samples/server/petstore/rust-server/output/openapi-v3/.openapi-generator/FILES @@ -9,14 +9,12 @@ docs/AnotherXmlInner.md docs/AnotherXmlObject.md docs/AnyOfGet202Response.md docs/AnyOfObject.md -docs/AnyOfObjectAnyOf.md docs/AnyOfProperty.md docs/DuplicateXmlObject.md docs/EnumWithStarObject.md docs/Err.md docs/Error.md docs/Model12345AnyOfObject.md -docs/Model12345AnyOfObjectAnyOf.md docs/MultigetGet201Response.md docs/MyId.md docs/MyIdList.md diff --git a/samples/server/petstore/rust-server/output/openapi-v3/README.md b/samples/server/petstore/rust-server/output/openapi-v3/README.md index b5bbb27bdf3a..fabb77958747 100644 --- a/samples/server/petstore/rust-server/output/openapi-v3/README.md +++ b/samples/server/petstore/rust-server/output/openapi-v3/README.md @@ -155,14 +155,12 @@ Method | HTTP request | Description - [AnotherXmlObject](docs/AnotherXmlObject.md) - [AnyOfGet202Response](docs/AnyOfGet202Response.md) - [AnyOfObject](docs/AnyOfObject.md) - - [AnyOfObjectAnyOf](docs/AnyOfObjectAnyOf.md) - [AnyOfProperty](docs/AnyOfProperty.md) - [DuplicateXmlObject](docs/DuplicateXmlObject.md) - [EnumWithStarObject](docs/EnumWithStarObject.md) - [Err](docs/Err.md) - [Error](docs/Error.md) - [Model12345AnyOfObject](docs/Model12345AnyOfObject.md) - - [Model12345AnyOfObjectAnyOf](docs/Model12345AnyOfObjectAnyOf.md) - [MultigetGet201Response](docs/MultigetGet201Response.md) - [MyId](docs/MyId.md) - [MyIdList](docs/MyIdList.md) diff --git a/samples/server/petstore/rust-server/output/openapi-v3/api/openapi.yaml b/samples/server/petstore/rust-server/output/openapi-v3/api/openapi.yaml index 43da25ffc94e..10519bee5f9f 100644 --- a/samples/server/petstore/rust-server/output/openapi-v3/api/openapi.yaml +++ b/samples/server/petstore/rust-server/output/openapi-v3/api/openapi.yaml @@ -478,13 +478,20 @@ components: - requiredAnyOf AnyOfObject: anyOf: - - $ref: '#/components/schemas/AnyOfObject_anyOf' + - enum: + - FOO + - BAR + type: string - description: Alternate option type: string description: Test a model containing an anyOf "12345AnyOfObject": anyOf: - - $ref: '#/components/schemas/_12345AnyOfObject_anyOf' + - enum: + - FOO + - BAR + - '*' + type: string - description: Alternate option type: string description: Test a model containing an anyOf that starts with a number @@ -678,19 +685,6 @@ components: anyOf: - $ref: '#/components/schemas/StringObject' - $ref: '#/components/schemas/UuidObject' - AnyOfObject_anyOf: - enum: - - FOO - - BAR - type: string - example: null - _12345AnyOfObject_anyOf: - enum: - - FOO - - BAR - - '*' - type: string - example: null securitySchemes: authScheme: flows: diff --git a/samples/server/petstore/rust-server/output/openapi-v3/src/models.rs b/samples/server/petstore/rust-server/output/openapi-v3/src/models.rs index 4ffae09b4e18..f3ea23bb3b79 100644 --- a/samples/server/petstore/rust-server/output/openapi-v3/src/models.rs +++ b/samples/server/petstore/rust-server/output/openapi-v3/src/models.rs @@ -638,50 +638,6 @@ impl AnyOfObject { } } -/// Enumeration of values. -/// Since this enum's variants do not hold data, we can easily define them as `#[repr(C)]` -/// which helps with FFI. -#[allow(non_camel_case_types)] -#[repr(C)] -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize)] -#[cfg_attr(feature = "conversion", derive(frunk_enum_derive::LabelledGenericEnum))] -pub enum AnyOfObjectAnyOf { - #[serde(rename = "FOO")] - Foo, - #[serde(rename = "BAR")] - Bar, -} - -impl std::fmt::Display for AnyOfObjectAnyOf { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match *self { - AnyOfObjectAnyOf::Foo => write!(f, "FOO"), - AnyOfObjectAnyOf::Bar => write!(f, "BAR"), - } - } -} - -impl std::str::FromStr for AnyOfObjectAnyOf { - type Err = String; - - fn from_str(s: &str) -> std::result::Result { - match s { - "FOO" => std::result::Result::Ok(AnyOfObjectAnyOf::Foo), - "BAR" => std::result::Result::Ok(AnyOfObjectAnyOf::Bar), - _ => std::result::Result::Err(format!("Value not valid: {}", s)), - } - } -} - -impl AnyOfObjectAnyOf { - /// Helper function to allow us to convert this model to an XML string. - /// Will panic if serialisation fails. - #[allow(dead_code)] - pub(crate) fn as_xml(&self) -> String { - serde_xml_rs::to_string(&self).expect("impossible to fail to serialize") - } -} - /// Test containing an anyOf object #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "conversion", derive(frunk::LabelledGeneric))] @@ -1242,54 +1198,6 @@ impl Model12345AnyOfObject { } } -/// Enumeration of values. -/// Since this enum's variants do not hold data, we can easily define them as `#[repr(C)]` -/// which helps with FFI. -#[allow(non_camel_case_types)] -#[repr(C)] -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize)] -#[cfg_attr(feature = "conversion", derive(frunk_enum_derive::LabelledGenericEnum))] -pub enum Model12345AnyOfObjectAnyOf { - #[serde(rename = "FOO")] - Foo, - #[serde(rename = "BAR")] - Bar, - #[serde(rename = "*")] - Star, -} - -impl std::fmt::Display for Model12345AnyOfObjectAnyOf { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match *self { - Model12345AnyOfObjectAnyOf::Foo => write!(f, "FOO"), - Model12345AnyOfObjectAnyOf::Bar => write!(f, "BAR"), - Model12345AnyOfObjectAnyOf::Star => write!(f, "*"), - } - } -} - -impl std::str::FromStr for Model12345AnyOfObjectAnyOf { - type Err = String; - - fn from_str(s: &str) -> std::result::Result { - match s { - "FOO" => std::result::Result::Ok(Model12345AnyOfObjectAnyOf::Foo), - "BAR" => std::result::Result::Ok(Model12345AnyOfObjectAnyOf::Bar), - "*" => std::result::Result::Ok(Model12345AnyOfObjectAnyOf::Star), - _ => std::result::Result::Err(format!("Value not valid: {}", s)), - } - } -} - -impl Model12345AnyOfObjectAnyOf { - /// Helper function to allow us to convert this model to an XML string. - /// Will panic if serialisation fails. - #[allow(dead_code)] - pub(crate) fn as_xml(&self) -> String { - serde_xml_rs::to_string(&self).expect("impossible to fail to serialize") - } -} - #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "conversion", derive(frunk::LabelledGeneric))] pub struct MultigetGet201Response {