[#4650] Fix StackOverflowError for recursive composite schemas #11620
Conversation
| @@ -3264,13 +3264,16 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc | |||
| * @param required required property only | |||
| * @param schema schema in which the properties will be added to the lists | |||
| */ | |||
| protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema) { | |||
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
I have updated the code and rebased it. The build fails in the samples. I don't think that it is related, WDYT?
|
cc @OpenAPITools/generator-core-team |
kthoms
force-pushed
the
issues/4650-recursive-composite-schemas
branch
2 times, most recently
from
afa57c9 to
1c9977b
Compare
|
Samples not up-to-date: https://github.com/OpenAPITools/openapi-generator/runs/5245407913?check_suite_focus=true Can you please follow step 3 in the PR checklist to update the samples so that the CI can verify the result? |
|
Thanks for the pointer. Will do. |
| **color** | **str** | | [optional] | ||
| **origin** | **str** | | [optional] | ||
| **cultivar** | **str** | | [optional] | ||
| **length_cm** | **float** | | [optional] |
There was a problem hiding this comment.
@kthoms looks like this fix converts properties from required to optional. Is that the intended behaviour?
cc @spacether
There was a problem hiding this comment.
No, this looks like an unwanted side effect. Thanks for pointing this out!
When I see this correctly the originating spec is
modules/openapi-generator/src/test/resources/3_0/python/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
There these both properties are declared as required.
I need to have a look.
There was a problem hiding this comment.
Updated the code. Now the samples are untouched.
kthoms
force-pushed
the
issues/4650-recursive-composite-schemas
branch
from
229f15d to
6c6396c
Compare
|
Build fails here: This is also reported here: odeke-em/drive#1128 |
|
The Go upgrade resolved the previous issue, but now it fails with This issue I have found here: |
|
Please revert e881b193682a71243297d35e295d0e7db81fd19e and I'll fix it in the master. |
kthoms
force-pushed
the
issues/4650-recursive-composite-schemas
branch
from
e881b19 to
6c6396c
Compare
|
@wing328 Thanks for your patience. I've reverted the last commit. |
…hemas The generator ran into a loop when a composite schema recursively added itself. This change provides a reproducing example and fixes the issue by extending DefaultCodegen#addProperties() by an additional circuit breaker parameter. When additionalProperties() is called with a schema instance for which the properties have already been added, the method directly returns and does not run into the loop.
kthoms
force-pushed
the
issues/4650-recursive-composite-schemas
branch
from
6c6396c to
46c489a
Compare
|
Tested locally to confirm the fix |
The generator ran into a loop when a composite schema recursively added itself. This change provides a reproducing example and fixes the issue by extending DefaultCodegen#addProperties() by an additional circuit breaker parameter.
When additionalProperties() is called with a schema instance for which the properties have already been added, the method directly returns and does not run into the loop.
To fix #4650