#4852 Unit test and fix to allow recursive models to process without StackOverflow - #5004
Conversation
Update from master
|
hi @Yaytay , One issue that I see here is that the test ReaderTest.testArraySchemaImplementation is failing with This indicates that items are not resolved correctly after the change. That extra true argument in 2.2.28 wasn't random — it was almost certainly introduced to fix some other array-handling edge case in the OpenAPI 3.1 path (for example, making sure annotated @ArraySchema metadata such as minItems, uniqueItems, description, proper items resolution, etc., is actually reflected in the generated schema for arrays of complex/polymorphic types). We need to verify as well how does it influence the OAS 3.x schema as well. Thanks again for your contribution! We're happy to see any updates from you on this topic :) |
|
My apologies for making this a PR when the unit tests fail - I didn't realise the tests in the other module were affected, entirely my bad. What I don't understand is why there would be any circumstances in which you'd want to reset the ModelConverterContext while processing a single model. I've updated the PR so that it tracks the AnnotatedTypes that it is currently resolving. |
|
@Yaytay, Thanks for your contribution! |
…entation processing (swagger-api#5187) `AnnotationsUtils.getArraySchema(...)` is called with `processSchemaImplementation = false` whenever the cycle guard added in swagger-api#5004 detects that the array's annotated type is already being resolved further up the stack. In that branch the existing `setItems(...)` paths are skipped, so the resulting array schema ends up with no `items` field at all — degrading to `type: array` (and in deeper cycles getting dropped entirely by downstream pruning). The implementation class itself has already been (or will be) registered in the `ModelConverterContext`, so we can recover a valid schema by emitting a `$ref` to that registered component. This preserves the StackOverflow fix while restoring `items: { $ref: ... }` for the recursive case. The lookup honours an explicit `@Schema(name = "...")` declaration on the implementation type and otherwise falls back to the simple class name (matching the default Jackson convention used elsewhere in swagger-core). If neither is registered we leave `items` unset, so the fallback is purely additive — schemas that already populated `items` through other paths are not modified.
…entation processing (#5187) (#5205) `AnnotationsUtils.getArraySchema(...)` is called with `processSchemaImplementation = false` whenever the cycle guard added in #5004 detects that the array's annotated type is already being resolved further up the stack. In that branch the existing `setItems(...)` paths are skipped, so the resulting array schema ends up with no `items` field at all — degrading to `type: array` (and in deeper cycles getting dropped entirely by downstream pruning). The implementation class itself has already been (or will be) registered in the `ModelConverterContext`, so we can recover a valid schema by emitting a `$ref` to that registered component. This preserves the StackOverflow fix while restoring `items: { $ref: ... }` for the recursive case. The lookup honours an explicit `@Schema(name = "...")` declaration on the implementation type and otherwise falls back to the simple class name (matching the default Jackson convention used elsewhere in swagger-core). If neither is registered we leave `items` unset, so the fallback is purely additive — schemas that already populated `items` through other paths are not modified.
4 participants
Pull Request
Description
Bug fix ton undo a tiny part of v2.2.27...v2.2.28#diff-bed6aab5e29942b1a575bded333604f9dfaeb2ef9475f1433266ece4e49f4fcb that introduced a StackOverflow exception on recursive data models with OpenAPI 3.1.
Fixes: #4852
Type of Change
Checklist