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.
2024-06-19 11:15:33 +02:00 · 2024-06-19 11:15:33 +02:00 · 5bc7aa3cd6
commit 5bc7aa3cd6
parent da57701569
7 changed files with 169 additions and 5 deletions
--- 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);
            }
--- 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());
+    }
 }
--- a/modules/openapi-generator/src/test/resources/3_0/issue_15077.yaml
+++ 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
--- 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
--- 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
--- 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&lt;String&gt;** |  |  |
 |**tags** | [**List&lt;Tag&gt;**](Tag.md) |  |  [optional] |
--- 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()));
      }