Skip to content

Commit

Permalink
fix #17258 - use model class only if it is generated (#17490)
Browse files Browse the repository at this point in the history
* fix  #16797 and #15796 spring child constructor missing parent params

* root cause and update the DefaultCodegen.java to add missing property when with multi inheritance

* rollback SpringCodegen.java

* update samples

* rollback with master cause #16992 fixed this issue too

* still using orignal design

* catchup master

* catchup master

* catchup master

* fix

* add tests

---------

Co-authored-by: dabdirb <[email protected]>
  • Loading branch information
martin-mfg and dabdirb authored Jan 5, 2024
1 parent 6317796 commit 64f2cad
Show file tree
Hide file tree
Showing 228 changed files with 10,788 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3724,6 +3724,10 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir
return;
}
if (ModelUtils.isComposedSchema(schema)) {
// fix issue #16797 and #15796, constructor fail by missing parent required params
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
properties.putAll(schema.getProperties());
}

if (schema.getAllOf() != null) {
for (Object component : schema.getAllOf()) {
Expand All @@ -3747,6 +3751,11 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir
}
}

for (String r : required) {
if (!properties.containsKey(r)) {
LOGGER.error("Required var %s not in properties", r);
}
}
return;
}

Expand Down Expand Up @@ -7324,7 +7333,7 @@ protected void addBodyModelSchema(CodegenParameter codegenParameter, String name
}

protected void updateRequestBodyForMap(CodegenParameter codegenParameter, Schema schema, String name, Set<String> imports, String bodyParameterName) {
if (StringUtils.isNotBlank(name)) {
if (StringUtils.isNotBlank(name) && !(ModelUtils.isFreeFormObject(schema) && !ModelUtils.shouldGenerateFreeFormObjectModel(name, this))) {
this.addBodyModelSchema(codegenParameter, name, schema, imports, bodyParameterName, true);
} else {
Schema inner = ModelUtils.getAdditionalProperties(schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,16 +509,7 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
Schema schema = schemas.get(name);

if (ModelUtils.isFreeFormObject(schema)) { // check to see if it's a free-form object
// there are 3 free form use cases
// 1. free form with no validation that is not allOf included in any composed schemas
// 2. free form with validation
// 3. free form that is allOf included in any composed schemas
// this use case arises when using interface schemas
// generators may choose to make models for use case 2 + 3
Schema refSchema = new Schema();
refSchema.set$ref("#/components/schemas/" + name);
Schema unaliasedSchema = config.unaliasSchema(refSchema);
if (unaliasedSchema.get$ref() == null) {
if (!ModelUtils.shouldGenerateFreeFormObjectModel(name, config)) {
LOGGER.info("Model {} not generated since it's a free-form object", name);
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import io.swagger.v3.parser.util.SchemaTypeUtil;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.CodegenConfig;
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.IJsonSchemaValidationProperties;
import org.openapitools.codegen.config.GlobalSettings;
Expand Down Expand Up @@ -831,6 +832,19 @@ public static boolean isFreeFormObject(Schema schema) {
return false;
}

public static boolean shouldGenerateFreeFormObjectModel(String name, CodegenConfig config) {
// there are 3 free form use cases
// 1. free form with no validation that is not allOf included in any composed schemas
// 2. free form with validation
// 3. free form that is allOf included in any composed schemas
// this use case arises when using interface schemas
// generators may choose to make models for use case 2 + 3
Schema refSchema = new Schema();
refSchema.set$ref("#/components/schemas/" + name);
Schema unaliasedSchema = config.unaliasSchema(refSchema);
return unaliasedSchema.get$ref() != null;
}

/**
* If a Schema contains a reference to another Schema with '$ref', returns the referenced Schema if it is found or the actual Schema in the other cases.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4388,4 +4388,34 @@ public void givenMultipartForm_whenGenerateBlockedServer_thenParameterAreCreated
assertFileContains(Paths.get(outputPath + "/src/main/java/org/openapitools/api/PetApi.java"),
"@Valid @RequestParam(value = \"additionalMetadata\", required = false) String additionalMetadata");
}

@Test
public void testMultiInheritanceParentRequiredParams_issue16797() throws IOException {
final Map<String, File> output = generateFromContract("src/test/resources/3_0/spring/issue_16797.yaml", SPRING_BOOT);
// constructor should as
// public Object4(Type1 pageInfo, String responseType, String requestId, Boolean success) {
// super(responseType, requestId, success, pageInfo);
// }
JavaFileAssert.assertThat(output.get("Object4.java"))
.assertConstructor("Type1", "String", "String", "Boolean")
.hasParameter("responseType").toConstructor()
.hasParameter("requestId").toConstructor()
.hasParameter("success").toConstructor()
.hasParameter("pageInfo").toConstructor()
;
}

@Test
public void testMultiInheritanceParentRequiredParams_issue15796() throws IOException {
final Map<String, File> output = generateFromContract("src/test/resources/3_0/spring/issue_15796.yaml", SPRING_BOOT);
// constructor should as this
//public Poodle(String race, String type) {
// super(race, type);
//}
JavaFileAssert.assertThat(output.get("Poodle.java"))
.assertConstructor("String", "String")
.hasParameter("type").toConstructor()
.hasParameter("race").toConstructor()
;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1790,6 +1807,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1921,6 +1938,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1802,6 +1819,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1773,6 +1790,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1875,6 +1892,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1753,6 +1770,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1764,6 +1781,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,23 @@ paths:
required:
- param
- param2
/fake/additionalProperties-reference:
post:
tags:
- fake
summary: test referenced additionalProperties
description: ''
operationId: testAdditionalPropertiesReference
responses:
'200':
description: successful operation
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/FreeFormObject'
description: request body
required: true
/fake/inline-additionalProperties:
post:
tags:
Expand Down Expand Up @@ -1772,6 +1789,10 @@ components:
enum:
- fish
- crab
FreeFormObject:
type: object
description: A schema consisting only of additional properties
additionalProperties: true
OuterEnum:
nullable: true
type: string
Expand Down
Loading

0 comments on commit 64f2cad

Please sign in to comment.