From f1bea467df47fcda884d0ce78a259f4a04816569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Zeidler?= Date: Wed, 19 Jun 2024 11:00:43 +0200 Subject: [PATCH] Never create inline model for allOf with single $ref 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 7ccb89967959..9e1c96367b06 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 @@ private boolean isModelNeeded(Schema schema, Set visitedSchemas) { 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 0303e161a2f4..133f9b1c5129 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 void resolveOperationInlineEnumFormParameters() { ((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 000000000000..2c227604eca3 --- /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 c2d678c532bb..d131a1c5001a 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 979f2695d996..66b9001788ee 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 818b8db00792..3cfa8c0076fe 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 fa28e27b4b62..14475a9aed66 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 static void validateJsonElement(JsonElement jsonElement) throws IOExcepti 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 void setCategoryAllOfRef(Category categoryAllOfRef) { } + 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 boolean equals(Object o) { 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 boolean equals(Object o) { @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 String toString() { 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 @@ private String toIndentedString(Object o) { 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 static void validateJsonElement(JsonElement jsonElement) throws IOExcepti 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())); }