Skip to content

Commit 42ef86b

Browse files
seonwooj0810seonwooj0810
authored
fix: preserve array items via $ref when cycle guard suppresses implementation 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.
1 parent be0548d commit 42ef86b

2 files changed

Lines changed: 219 additions & 0 deletions

File tree

‎modules/swagger-core/src/main/java/io/swagger/v3/core/util/AnnotationsUtils.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,17 @@ public static Optional<Schema> getArraySchema(io.swagger.v3.oas.annotations.medi
559559
} else if (processSchemaImplementation) {
560560
getSchema(arraySchema.schema(), arraySchema, false, arraySchema.schema().implementation(), components, jsonViewAnnotation, openapi31, context)
561561
.ifPresent(arraySchemaObject::setItems);
562+
} else if (arraySchemaObject.getItems() == null && context != null) {
563+
// Cycle guard active: the implementation type is currently being
564+
// resolved up the call stack, so we cannot recurse into it.
565+
// The type has already been registered as a component, so emit a
566+
// $ref instead of leaving the array's items unset (gh-5187).
567+
String implName = findRegisteredSchemaName(arraySchema.schema().implementation(), context);
568+
if (implName != null) {
569+
Schema itemsRef = openapi31 ? new JsonSchema() : new Schema();
570+
itemsRef.set$ref(COMPONENTS_REF + implName);
571+
arraySchemaObject.setItems(itemsRef);
572+
}
562573
}
563574
}
564575

@@ -2104,6 +2115,37 @@ public static io.swagger.v3.oas.annotations.media.Schema getSchemaAnnotation(Ann
21042115
return null;
21052116
}
21062117

2118+
/**
2119+
* Looks up the registered schema name for an implementation class in the
2120+
* given {@link ModelConverterContext}.
2121+
* <p>
2122+
* Prefers an explicit {@code @Schema(name = "...")} declaration on the
2123+
* class, then the type-name resolved by the underlying {@code ObjectMapper}
2124+
* (when available), and finally the simple class name. Returns {@code null}
2125+
* if no matching schema is registered.
2126+
* <p>
2127+
* Used to recover a usable {@code $ref} when array-schema processing is
2128+
* suppressed by a cycle guard (gh-5187).
2129+
*/
2130+
static String findRegisteredSchemaName(Class<?> impl, ModelConverterContext context) {
2131+
if (impl == null || impl.equals(Void.class) || context == null) {
2132+
return null;
2133+
}
2134+
Map<String, Schema> defined = context.getDefinedModels();
2135+
if (defined == null || defined.isEmpty()) {
2136+
return null;
2137+
}
2138+
io.swagger.v3.oas.annotations.media.Schema annotated = impl.getAnnotation(io.swagger.v3.oas.annotations.media.Schema.class);
2139+
if (annotated != null && StringUtils.isNotBlank(annotated.name()) && defined.containsKey(annotated.name())) {
2140+
return annotated.name();
2141+
}
2142+
String simpleName = impl.getSimpleName();
2143+
if (StringUtils.isNotBlank(simpleName) && defined.containsKey(simpleName)) {
2144+
return simpleName;
2145+
}
2146+
return null;
2147+
}
2148+
21072149
public static io.swagger.v3.oas.annotations.media.ArraySchema getArraySchemaAnnotation(Annotation... annotations) {
21082150
if (annotations == null) {
21092151
return null;

‎modules/swagger-core/src/test/java/io/swagger/v3/core/converting/Issue5187Test.java‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
package io.swagger.v3.core.converting;
2+
3+
import io.swagger.v3.core.converter.AnnotatedType;
4+
import io.swagger.v3.core.converter.ModelConverter;
5+
import io.swagger.v3.core.converter.ModelConverterContext;
6+
import io.swagger.v3.core.util.AnnotationsUtils;
7+
import io.swagger.v3.oas.models.media.ArraySchema;
8+
import io.swagger.v3.oas.models.media.JsonSchema;
9+
import io.swagger.v3.oas.models.media.Schema;
10+
import org.testng.annotations.Test;
11+
12+
import java.lang.annotation.Annotation;
13+
import java.util.HashMap;
14+
import java.util.Iterator;
15+
import java.util.Map;
16+
import java.util.Optional;
17+
18+
import static org.testng.Assert.assertEquals;
19+
import static org.testng.Assert.assertNotNull;
20+
import static org.testng.Assert.assertNull;
21+
22+
/**
23+
* Regression for <a href="https://github.com/swagger-api/swagger-core/issues/5187">gh-5187</a>:
24+
* when the cycle guard added in #5004 fires, {@code AnnotationsUtils.getArraySchema}
25+
* is invoked with {@code processSchemaImplementation = false} and used to leave the
26+
* array's {@code items} unset entirely (degrading the schema to {@code type: array}
27+
* with no items, or dropping the property when downstream code stripped the empty
28+
* schema). The fallback should emit a {@code $ref} to the implementation type that
29+
* is already registered in the {@link ModelConverterContext}.
30+
*/
31+
public class Issue5187Test {
32+
33+
@io.swagger.v3.oas.annotations.media.Schema
34+
static class Inner {
35+
public String name;
36+
}
37+
38+
@io.swagger.v3.oas.annotations.media.Schema(name = "RenamedInner")
39+
static class RenamedInner {
40+
public String name;
41+
}
42+
43+
@io.swagger.v3.oas.annotations.media.ArraySchema(
44+
schema = @io.swagger.v3.oas.annotations.media.Schema(implementation = Inner.class))
45+
static class HolderUsage {
46+
}
47+
48+
@io.swagger.v3.oas.annotations.media.ArraySchema(
49+
schema = @io.swagger.v3.oas.annotations.media.Schema(implementation = RenamedInner.class))
50+
static class RenamedHolderUsage {
51+
}
52+
53+
@Test
54+
public void cycleGuard_oas31_emitsRefToRegisteredImplementation() {
55+
ModelConverterContext context = new StubContext();
56+
context.defineModel("Inner", new JsonSchema());
57+
58+
io.swagger.v3.oas.annotations.media.ArraySchema ann =
59+
HolderUsage.class.getAnnotation(io.swagger.v3.oas.annotations.media.ArraySchema.class);
60+
61+
Schema existing = new JsonSchema().typesItem("array");
62+
Optional<Schema> result = AnnotationsUtils.getArraySchema(
63+
ann, null, null, /* openapi31 */ true, existing,
64+
/* processSchemaImplementation */ false, context);
65+
66+
assertNotNull(result.orElse(null), "getArraySchema should produce a schema");
67+
Schema items = result.get().getItems();
68+
assertNotNull(items, "items must be populated via the cycle-guard fallback");
69+
assertEquals(items.get$ref(), "#/components/schemas/Inner");
70+
}
71+
72+
@Test
73+
public void cycleGuard_oas30_emitsRefToRegisteredImplementation() {
74+
ModelConverterContext context = new StubContext();
75+
context.defineModel("Inner", new Schema());
76+
77+
io.swagger.v3.oas.annotations.media.ArraySchema ann =
78+
HolderUsage.class.getAnnotation(io.swagger.v3.oas.annotations.media.ArraySchema.class);
79+
80+
Optional<Schema> result = AnnotationsUtils.getArraySchema(
81+
ann, null, null, /* openapi31 */ false, /* existingSchema */ null,
82+
/* processSchemaImplementation */ false, context);
83+
84+
assertNotNull(result.orElse(null), "getArraySchema should produce a schema");
85+
Schema items = result.get().getItems();
86+
assertNotNull(items, "items must be populated via the cycle-guard fallback");
87+
assertEquals(items.get$ref(), "#/components/schemas/Inner");
88+
}
89+
90+
@Test
91+
public void cycleGuard_honoursExplicitSchemaName() {
92+
ModelConverterContext context = new StubContext();
93+
context.defineModel("RenamedInner", new Schema());
94+
95+
io.swagger.v3.oas.annotations.media.ArraySchema ann =
96+
RenamedHolderUsage.class.getAnnotation(io.swagger.v3.oas.annotations.media.ArraySchema.class);
97+
98+
Optional<Schema> result = AnnotationsUtils.getArraySchema(
99+
ann, null, null, false, null, false, context);
100+
101+
Schema items = result.get().getItems();
102+
assertNotNull(items);
103+
assertEquals(items.get$ref(), "#/components/schemas/RenamedInner");
104+
}
105+
106+
@Test
107+
public void cycleGuard_doesNotOverwriteExistingItems() {
108+
ModelConverterContext context = new StubContext();
109+
context.defineModel("Inner", new Schema());
110+
111+
io.swagger.v3.oas.annotations.media.ArraySchema ann =
112+
HolderUsage.class.getAnnotation(io.swagger.v3.oas.annotations.media.ArraySchema.class);
113+
114+
Schema preExisting = new JsonSchema().typesItem("array");
115+
preExisting.setItems(new JsonSchema().$ref("#/components/schemas/SomethingElse"));
116+
117+
Optional<Schema> result = AnnotationsUtils.getArraySchema(
118+
ann, null, null, true, preExisting, false, context);
119+
120+
// The fallback only runs when items is null; pre-existing items must be preserved.
121+
assertEquals(result.get().getItems().get$ref(), "#/components/schemas/SomethingElse");
122+
}
123+
124+
@Test
125+
public void cycleGuard_doesNothingWhenTypeNotRegistered() {
126+
ModelConverterContext context = new StubContext();
127+
// Note: do not define Inner in the context.
128+
129+
io.swagger.v3.oas.annotations.media.ArraySchema ann =
130+
HolderUsage.class.getAnnotation(io.swagger.v3.oas.annotations.media.ArraySchema.class);
131+
132+
Optional<Schema> result = AnnotationsUtils.getArraySchema(
133+
ann, null, null, false, null, false, context);
134+
135+
assertNull(result.get().getItems(),
136+
"without a registered schema we cannot invent a $ref; leave items unset");
137+
}
138+
139+
/**
140+
* Minimal {@link ModelConverterContext} that records defined models — enough to
141+
* exercise the lookup used by the cycle-guard fallback without spinning up a
142+
* full {@code ModelConverters} chain.
143+
*/
144+
private static final class StubContext implements ModelConverterContext {
145+
private final Map<String, Schema> models = new HashMap<>();
146+
147+
@Override
148+
public void defineModel(String name, Schema model) {
149+
models.put(name, model);
150+
}
151+
152+
@Override
153+
public void defineModel(String name, Schema model, AnnotatedType type, String previousName) {
154+
models.put(name, model);
155+
}
156+
157+
@Override
158+
public void defineModel(String name, Schema model, java.lang.reflect.Type type, String previousName) {
159+
models.put(name, model);
160+
}
161+
162+
@Override
163+
public Map<String, Schema> getDefinedModels() {
164+
return models;
165+
}
166+
167+
@Override
168+
public Schema resolve(AnnotatedType type) {
169+
return null;
170+
}
171+
172+
@Override
173+
public Iterator<ModelConverter> getConverters() {
174+
return java.util.Collections.<ModelConverter>emptyList().iterator();
175+
}
176+
}
177+
}

0 commit comments

Comments
 (0)