Skip to content

Commit

Permalink
Never create inline model for allOf with single $ref
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ReneZeidler committed Jun 19, 2024
1 parent 18b4bcb commit f1bea46
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ private boolean isModelNeeded(Schema schema, Set<Schema> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
53 changes: 53 additions & 0 deletions modules/openapi-generator/src/test/resources/3_0/issue_15077.yaml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ paths:
operationId: getParameterNameMapping
parameters:
- name: _type
in: header
in: header
description: _type
required: true
schema:
Expand All @@ -1230,7 +1230,7 @@ paths:
schema:
type: string
- name: type_
in: header
in: header
description: type_
required: true
schema:
Expand All @@ -1246,7 +1246,7 @@ paths:
operationId: getParameterStringNumber
parameters:
- name: string_number
in: header
in: header
description: string number
required: true
schema:
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions samples/client/petstore/java/okhttp-gson/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions samples/client/petstore/java/okhttp-gson/docs/NewPet.md
Original file line number Diff line number Diff line change
Expand Up @@ -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] |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) &&
Expand All @@ -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
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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()));
}
Expand Down

0 comments on commit f1bea46

Please sign in to comment.