Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4650] Fix StackOverflowError for recursive composite schemas #11620

Merged
merged 1 commit into from
Mar 1, 2022
Merged

[#4650] Fix StackOverflowError for recursive composite schemas #11620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2532,14 +2532,14 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
if (allDefinitions != null && refSchema != null) {
if (allParents.contains(ref) && supportsMultipleInheritance) {
// multiple inheritance
addProperties(allProperties, allRequired, refSchema);
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else if (parentName != null && parentName.equals(ref) && supportsInheritance) {
// single inheritance
addProperties(allProperties, allRequired, refSchema);
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else {
// composition
addProperties(properties, required, refSchema);
addProperties(allProperties, allRequired, refSchema);
addProperties(properties, required, refSchema, new HashSet<>());
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
}
}

Expand Down Expand Up @@ -2576,10 +2576,10 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
if (component.get$ref() == null) {
if (component != null) {
// component is the child schema
addProperties(properties, required, component);
addProperties(properties, required, component, new HashSet<>());

// includes child's properties (all, required) in allProperties, allRequired
addProperties(allProperties, allRequired, component);
addProperties(allProperties, allRequired, component, new HashSet<>());
}
break; // at most one child only
}
Expand Down Expand Up @@ -3260,17 +3260,21 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc
/**
* Add schema's properties to "properties" and "required" list
*
* @param properties all properties
* @param required required property only
* @param schema schema in which the properties will be added to the lists
* @param properties all properties
* @param required required property only
* @param schema schema in which the properties will be added to the lists
* @param visitedSchemas circuit-breaker - the schemas with which the method was called before for recursive structures
*/
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kthoms thanks for the PR. Please update the docstring of the method as well since it's causing issues to the build: https://github.com/OpenAPITools/openapi-generator/runs/5213864795?check_suite_focus=true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the code and rebased it. The build fails in the samples. I don't think that it is related, WDYT?

protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas) {
if (!visitedSchemas.add(schema)) {
return;
}
if (schema instanceof ComposedSchema) {
ComposedSchema composedSchema = (ComposedSchema) schema;

if (composedSchema.getAllOf() != null) {
for (Schema component : composedSchema.getAllOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

Expand All @@ -3280,13 +3284,13 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (composedSchema.getOneOf() != null) {
for (Schema component : composedSchema.getOneOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

if (composedSchema.getAnyOf() != null) {
for (Schema component : composedSchema.getAnyOf()) {
addProperties(properties, required, component);
addProperties(properties, required, component, visitedSchemas);
}
}

Expand All @@ -3295,7 +3299,7 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (StringUtils.isNotBlank(schema.get$ref())) {
Schema interfaceSchema = ModelUtils.getReferencedSchema(this.openAPI, schema);
addProperties(properties, required, interfaceSchema);
addProperties(properties, required, interfaceSchema, visitedSchemas);
return;
}
if (schema.getProperties() != null) {
Expand Down Expand Up @@ -6244,7 +6248,7 @@ public List<CodegenParameter> fromRequestBodyToFormParameters(RequestBody body,
// TODO in the future have this return one codegenParameter of type object or composed which includes all definition
// that will be needed for complex composition use cases
// https://github.com/OpenAPITools/openapi-generator/issues/10415
addProperties(properties, allRequired, schema);
addProperties(properties, allRequired, schema, new HashSet<>());

if (!properties.isEmpty()) {
for (Map.Entry<String, Schema> entry : properties.entrySet()) {
Expand Down
4 changes: 2 additions & 2 deletions ...enapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,14 +629,14 @@ private void fixComposedSchemaRequiredVars(Schema schema, CodegenModel result) {
}
for (Schema sc : oneOfanyOfSchemas) {
Schema refSchema = ModelUtils.getReferencedSchema(this.openAPI, sc);
addProperties(otherProperties, otherRequired, refSchema);
addProperties(otherProperties, otherRequired, refSchema, new HashSet<>());
}
Set<String> otherRequiredSet = new HashSet<>(otherRequired);

List<Schema> allOf = cs.getAllOf();
if ((schema.getProperties() != null && !schema.getProperties().isEmpty()) || allOf != null) {
// NOTE: this function also adds the allOf properties inside schema
addProperties(selfProperties, selfRequired, schema);
addProperties(selfProperties, selfRequired, schema, new HashSet<>());
}
if (result.discriminator != null) {
selfRequired.add(result.discriminator.getPropertyBaseName());
Expand Down
20 changes: 20 additions & 0 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultGeneratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,4 +744,24 @@ public void testProcessUserDefinedTemplatesWithConfig() throws IOException {
templates.toFile().delete();
}
}

@Test
public void testRecursionBug4650() {
OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/bugs/recursion-bug-4650.yaml");
ClientOptInput opts = new ClientOptInput();
opts.openAPI(openAPI);
DefaultCodegen config = new DefaultCodegen();
config.setStrictSpecBehavior(false);
opts.config(config);
final DefaultGenerator generator = new DefaultGenerator();
generator.opts(opts);
generator.configureGeneratorProperties();

List<File> files = new ArrayList<>();
List<String> filteredSchemas = ModelUtils.getSchemasUsedOnlyInFormParam(openAPI);
List<Object> allModels = new ArrayList<>();
// The bug causes a StackOverflowError when calling generateModels
generator.generateModels(files, allModels, filteredSchemas);
// all fine, we have passed
}
}
64 changes: 64 additions & 0 deletions modules/openapi-generator/src/test/resources/bugs/recursion-bug-4650.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
openapi: 3.0.1
info:
title: Test
version: v1
paths:
/api/v1/filters:
get:
responses:
'200':
description: default response
content:
'*/*':
schema:
$ref: '#/components/schemas/Filter'
components:
schemas:
AndFilter:
type: object
allOf:
- $ref: '#/components/schemas/Filter'
- type: object
properties:
operator:
type: string
default: and
enum:
- and
- or
filters:
type: array
items:
$ref: '#/components/schemas/Filter'
Filter:
type: object
properties:
operator:
type: string
enum:
- and
- or
example:
operator: eq
field: name
value: john
discriminator:
propertyName: operator
mapping:
and: '#/components/schemas/AndFilter'
or: '#/components/schemas/OrFilter'
oneOf:
- $ref: '#/components/schemas/AndFilter'
- $ref: '#/components/schemas/OrFilter'
OrFilter:
type: object
allOf:
- $ref: '#/components/schemas/Filter'
- type: object
properties:
operator:
type: string
default: or
enum:
- and
- or