From 5bc7aa3cd603abff2e320e7a5cefa9abfa5ea936 Mon Sep 17 00:00:00 2001 From: ReneZeidler Date: Wed, 19 Jun 2024 11:15:33 +0200 Subject: [PATCH] Never create inline model for allOf with single $ref (#18945) Fixes #15077 The previous fix for this in #16096 is incomplete because it still generates unnecessary inline models when readOnly or nullable is used in conjunction with other properties like description. This commit fixes the logic error and adds testcases. --- .../codegen/InlineModelResolver.java | 4 +- .../codegen/InlineModelResolverTest.java | 26 ++++++++ .../src/test/resources/3_0/issue_15077.yaml | 53 ++++++++++++++++ ...points-models-for-testing-okhttp-gson.yaml | 17 +++++- .../java/okhttp-gson/api/openapi.yaml | 11 ++++ .../petstore/java/okhttp-gson/docs/NewPet.md | 2 + .../org/openapitools/client/model/NewPet.java | 61 ++++++++++++++++++- 7 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 modules/openapi-generator/src/test/resources/3_0/issue_15077.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 7ccb8996795..9e1c96367b0 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 @@ -231,7 +231,9 @@ public class InlineModelResolver { if (schema.equals(c)) { return isModelNeeded((Schema) schema.getAllOf().get(0), visitedSchemas); } - } else if (isSingleAllOf && StringUtils.isNotEmpty(((Schema) schema.getAllOf().get(0)).get$ref())) { + } + + if (isSingleAllOf && StringUtils.isNotEmpty(((Schema) schema.getAllOf().get(0)).get$ref())) { // single allOf and it's a ref return isModelNeeded((Schema) schema.getAllOf().get(0), visitedSchemas); } 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 0303e161a2f..133f9b1c512 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 @@ -1171,4 +1171,30 @@ public class InlineModelResolverTest { ((Schema) inlineFormParaemter.getProperties().get("enum_form_string")).get$ref()); } + + @Test + public void doNotWrapSingleAllOfRefs() { + OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue_15077.yaml"); + new InlineModelResolver().flatten(openAPI); + + // None of these cases should be wrapped in an inline schema and should reference the original schema "NumberRange" + Schema limitsModel = (Schema) openAPI.getComponents().getSchemas().get("Limits"); + final String numberRangeRef = "#/components/schemas/NumberRange"; + + Schema allOfRef = (Schema) limitsModel.getProperties().get("allOfRef"); + assertNotNull(allOfRef.getAllOf()); + assertEquals(numberRangeRef, ((Schema) allOfRef.getAllOf().get(0)).get$ref()); + + Schema allOfRefWithDescription = (Schema) limitsModel.getProperties().get("allOfRefWithDescription"); + assertNotNull(allOfRefWithDescription.getAllOf()); + assertEquals(numberRangeRef, ((Schema) allOfRefWithDescription.getAllOf().get(0)).get$ref()); + + Schema allOfRefWithReadonly = (Schema) limitsModel.getProperties().get("allOfRefWithReadonly"); + assertNotNull(allOfRefWithReadonly.getAllOf()); + assertEquals(numberRangeRef, ((Schema) allOfRefWithReadonly.getAllOf().get(0)).get$ref()); + + Schema allOfRefWithDescriptionAndReadonly = (Schema) limitsModel.getProperties().get("allOfRefWithDescriptionAndReadonly"); + assertNotNull(allOfRefWithDescriptionAndReadonly.getAllOf()); + assertEquals(numberRangeRef, ((Schema) allOfRefWithDescriptionAndReadonly.getAllOf().get(0)).get$ref()); + } } diff --git a/modules/openapi-generator/src/test/resources/3_0/issue_15077.yaml b/modules/openapi-generator/src/test/resources/3_0/issue_15077.yaml new file mode 100644 index 00000000000..2c227604eca --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_0/issue_15077.yaml @@ -0,0 +1,53 @@ +openapi: 3.0.0 +info: + version: 1.0.0 + title: Property refs using allOf +paths: + /limits: + get: + operationId: getLimits + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Limits' +components: + schemas: + NumberRange: + type: object + properties: + min: + type: number + max: + type: number + required: + - min + - max + Limits: + type: object + properties: + allOfRef: + allOf: + - $ref: '#/components/schemas/NumberRange' + allOfRefWithDescription: + description: | + Description for this property + allOf: + - $ref: '#/components/schemas/NumberRange' + allOfRefWithReadonly: + readOnly: true + allOf: + - $ref: '#/components/schemas/NumberRange' + allOfRefWithDescriptionAndReadonly: + description: | + Description for this readonly property + readOnly: true + allOf: + - $ref: '#/components/schemas/NumberRange' + required: + - allOfRef + - allOfRefWithDescription + - allOfRefWithReadonly + - allOfRefWithDescriptionAndReadonly \ No newline at end of file diff --git a/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-okhttp-gson.yaml b/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-okhttp-gson.yaml index c2d678c532b..d131a1c5001 100644 --- a/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-okhttp-gson.yaml +++ b/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-okhttp-gson.yaml @@ -1217,7 +1217,7 @@ paths: operationId: getParameterNameMapping parameters: - name: _type - in: header + in: header description: _type required: true schema: @@ -1230,7 +1230,7 @@ paths: schema: type: string - name: type_ - in: header + in: header description: type_ required: true schema: @@ -1246,7 +1246,7 @@ paths: operationId: getParameterStringNumber parameters: - name: string_number - in: header + in: header description: string number required: true schema: @@ -2600,6 +2600,17 @@ components: category_allOf_ref: allOf: - $ref: '#/components/schemas/Category' + category_allOf_ref_description: + description: | + Adding description to property using allOf + allOf: + - $ref: '#/components/schemas/Category' + category_allOf_ref_description_readonly: + description: | + Adding description to readonly property using allOf + readOnly: true + allOf: + - $ref: '#/components/schemas/Category' name: type: string example: doggie diff --git a/samples/client/petstore/java/okhttp-gson/api/openapi.yaml b/samples/client/petstore/java/okhttp-gson/api/openapi.yaml index 979f2695d99..66b9001788e 100644 --- a/samples/client/petstore/java/okhttp-gson/api/openapi.yaml +++ b/samples/client/petstore/java/okhttp-gson/api/openapi.yaml @@ -2640,6 +2640,17 @@ components: category_allOf_ref: allOf: - $ref: '#/components/schemas/Category' + category_allOf_ref_description: + allOf: + - $ref: '#/components/schemas/Category' + description: | + Adding description to property using allOf + category_allOf_ref_description_readonly: + allOf: + - $ref: '#/components/schemas/Category' + description: | + Adding description to readonly property using allOf + readOnly: true name: example: doggie type: string diff --git a/samples/client/petstore/java/okhttp-gson/docs/NewPet.md b/samples/client/petstore/java/okhttp-gson/docs/NewPet.md index 818b8db0079..3cfa8c0076f 100644 --- a/samples/client/petstore/java/okhttp-gson/docs/NewPet.md +++ b/samples/client/petstore/java/okhttp-gson/docs/NewPet.md @@ -10,6 +10,8 @@ |**id** | **Long** | | [optional] | |**categoryInlineAllof** | [**NewPetCategoryInlineAllof**](NewPetCategoryInlineAllof.md) | | [optional] | |**categoryAllOfRef** | [**Category**](Category.md) | | [optional] | +|**categoryAllOfRefDescription** | [**Category**](Category.md) | Adding description to property using allOf | [optional] | +|**categoryAllOfRefDescriptionReadonly** | [**Category**](Category.md) | Adding description to readonly property using allOf | [optional] [readonly] | |**name** | **String** | | | |**photoUrls** | **List<String>** | | | |**tags** | [**List<Tag>**](Tag.md) | | [optional] | diff --git a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NewPet.java b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NewPet.java index fa28e27b4b6..14475a9aed6 100644 --- a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NewPet.java +++ b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/model/NewPet.java @@ -68,6 +68,14 @@ public class NewPet { @SerializedName(SERIALIZED_NAME_CATEGORY_ALL_OF_REF) private Category categoryAllOfRef; + public static final String SERIALIZED_NAME_CATEGORY_ALL_OF_REF_DESCRIPTION = "category_allOf_ref_description"; + @SerializedName(SERIALIZED_NAME_CATEGORY_ALL_OF_REF_DESCRIPTION) + private Category categoryAllOfRefDescription; + + public static final String SERIALIZED_NAME_CATEGORY_ALL_OF_REF_DESCRIPTION_READONLY = "category_allOf_ref_description_readonly"; + @SerializedName(SERIALIZED_NAME_CATEGORY_ALL_OF_REF_DESCRIPTION_READONLY) + private Category categoryAllOfRefDescriptionReadonly; + public static final String SERIALIZED_NAME_NAME = "name"; @SerializedName(SERIALIZED_NAME_NAME) private String name; @@ -141,6 +149,13 @@ public class NewPet { public NewPet() { } + public NewPet( + Category categoryAllOfRefDescriptionReadonly + ) { + this(); + this.categoryAllOfRefDescriptionReadonly = categoryAllOfRefDescriptionReadonly; + } + public NewPet id(Long id) { this.id = id; return this; @@ -198,6 +213,36 @@ public class NewPet { } + public NewPet categoryAllOfRefDescription(Category categoryAllOfRefDescription) { + this.categoryAllOfRefDescription = categoryAllOfRefDescription; + return this; + } + + /** + * Adding description to property using allOf + * @return categoryAllOfRefDescription + */ + @javax.annotation.Nullable + public Category getCategoryAllOfRefDescription() { + return categoryAllOfRefDescription; + } + + public void setCategoryAllOfRefDescription(Category categoryAllOfRefDescription) { + this.categoryAllOfRefDescription = categoryAllOfRefDescription; + } + + + /** + * Adding description to readonly property using allOf + * @return categoryAllOfRefDescriptionReadonly + */ + @javax.annotation.Nullable + public Category getCategoryAllOfRefDescriptionReadonly() { + return categoryAllOfRefDescriptionReadonly; + } + + + public NewPet name(String name) { this.name = name; return this; @@ -347,6 +392,8 @@ public class NewPet { return Objects.equals(this.id, newPet.id) && Objects.equals(this.categoryInlineAllof, newPet.categoryInlineAllof) && Objects.equals(this.categoryAllOfRef, newPet.categoryAllOfRef) && + Objects.equals(this.categoryAllOfRefDescription, newPet.categoryAllOfRefDescription) && + Objects.equals(this.categoryAllOfRefDescriptionReadonly, newPet.categoryAllOfRefDescriptionReadonly) && Objects.equals(this.name, newPet.name) && Objects.equals(this.photoUrls, newPet.photoUrls) && Objects.equals(this.tags, newPet.tags) && @@ -356,7 +403,7 @@ public class NewPet { @Override public int hashCode() { - return Objects.hash(id, categoryInlineAllof, categoryAllOfRef, name, photoUrls, tags, status, additionalProperties); + return Objects.hash(id, categoryInlineAllof, categoryAllOfRef, categoryAllOfRefDescription, categoryAllOfRefDescriptionReadonly, name, photoUrls, tags, status, additionalProperties); } @Override @@ -366,6 +413,8 @@ public class NewPet { sb.append(" id: ").append(toIndentedString(id)).append("\n"); sb.append(" categoryInlineAllof: ").append(toIndentedString(categoryInlineAllof)).append("\n"); sb.append(" categoryAllOfRef: ").append(toIndentedString(categoryAllOfRef)).append("\n"); + sb.append(" categoryAllOfRefDescription: ").append(toIndentedString(categoryAllOfRefDescription)).append("\n"); + sb.append(" categoryAllOfRefDescriptionReadonly: ").append(toIndentedString(categoryAllOfRefDescriptionReadonly)).append("\n"); sb.append(" name: ").append(toIndentedString(name)).append("\n"); sb.append(" photoUrls: ").append(toIndentedString(photoUrls)).append("\n"); sb.append(" tags: ").append(toIndentedString(tags)).append("\n"); @@ -396,6 +445,8 @@ public class NewPet { openapiFields.add("id"); openapiFields.add("category_inline_allof"); openapiFields.add("category_allOf_ref"); + openapiFields.add("category_allOf_ref_description"); + openapiFields.add("category_allOf_ref_description_readonly"); openapiFields.add("name"); openapiFields.add("photoUrls"); openapiFields.add("tags"); @@ -435,6 +486,14 @@ public class NewPet { if (jsonObj.get("category_allOf_ref") != null && !jsonObj.get("category_allOf_ref").isJsonNull()) { Category.validateJsonElement(jsonObj.get("category_allOf_ref")); } + // validate the optional field `category_allOf_ref_description` + if (jsonObj.get("category_allOf_ref_description") != null && !jsonObj.get("category_allOf_ref_description").isJsonNull()) { + Category.validateJsonElement(jsonObj.get("category_allOf_ref_description")); + } + // validate the optional field `category_allOf_ref_description_readonly` + if (jsonObj.get("category_allOf_ref_description_readonly") != null && !jsonObj.get("category_allOf_ref_description_readonly").isJsonNull()) { + Category.validateJsonElement(jsonObj.get("category_allOf_ref_description_readonly")); + } if (!jsonObj.get("name").isJsonPrimitive()) { throw new IllegalArgumentException(String.format("Expected the field `name` to be a primitive type in the JSON string but got `%s`", jsonObj.get("name").toString())); }