fix: emit $ref for array items when cycle guard suppresses implementation processing#5205
Conversation
…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.
|
@ewaostrowska I don't think this should have the 'backlog' label... any array type within a schema that references a non-primitive (e.g. string etc) type is generated incorrectly at the moment. |
|
Thanks for the bump, @caoimhebyrne — agreed this hurts real payloads and shouldn't sit in the backlog. One scoping note so maintainers can size it accurately: the regression this PR fixes is specifically the cycle-guard branch added in #5004. When The fix is purely additive: it only fills If you're seeing arrays of non-primitive types dropped outside the recursive/cycle-guard case, that would be a different code path from what this PR touches — a minimal repro would be really helpful, and I'm happy to extend this fix or open a follow-up to cover it. For reference, the PR adds five focused regression tests and the full 692-test suite passes locally, so it's ready for review whenever a maintainer has a moment. |
Description
Fixes #5187.
AnnotationsUtils.getArraySchema(...)is invoked withprocessSchemaImplementation = falsewhenever the cycle guard added in #5004 detects that the array's annotated type is already being resolved further up the stack. In that branch bothsetItems(...)paths are skipped:So when a recursive model uses
@ArraySchema(schema = @Schema(implementation = X.class))and the guard fires, the resulting array schema loses itsitemsfield. In OAS 3.1 the existing schema passed in usually already haditemsfromcontext.resolve(...), but the cycle-guard branch itself contributes nothing — and when downstream code starts from an empty array schema (no pre-existing items), the property degrades to bare{ type: "array" }or gets pruned entirely. This is the regression #5187 reports against 2.2.41 — 2.2.49.Root cause
The cycle guard was introduced in #5004 to prevent
StackOverflowErroron recursive models. To avoid recursing it forcesprocessSchemaImplementation = false, but the recovery path then drops theitemsfield rather than reaching for the schema that the converter context is already (or about to be) holding for that implementation type.Fix
When the cycle guard fires AND
arraySchemaObject.getItems() == null, look up the implementation class inModelConverterContext.getDefinedModels()and emititems: { $ref: "#/components/schemas/<Name>" }. The lookup honours an explicit@Schema(name = "...")declaration on the implementation type and falls back to the simple class name (matching the default Jackson convention used elsewhere). If neither is registered we leaveitemsunset — so the fallback is purely additive: schemas that populateditemsthrough other paths are untouched.The implementation type is exactly the one the user named in
@Schema(implementation = X.class), so emitting a$refto its already-registered component preserves the StackOverflow fix while keeping the array schema valid.Tests
Added
Issue5187Testwith five focused regressions, exercising:$refto the registered implementation.$refto the registered implementation.@Schema(name = "Renamed")is honoured over the simple class name.itemson the array schema are preserved (the fallback only runs whenitemsis null).itemsis left unset (no invented refs).Without the fix three of these tests fail (confirmed by reverting
AnnotationsUtils.javalocally); with it the fullswagger-coresuite (692 tests) passes.Type of Change
Checklist
ModelResolvercycle guard dropsitemsfrom recursiveList<T>schemas (regression since 2.2.41) #5187)