Skip to content

Commit 6143bee

Browse files
Mattias-SehlstedtMattias-Sehlstedt
authored
fix: calculate sibling properties based upon whether the schema is the array or the item in the array (#5055)
1 parent 6043dd5 commit 6143bee

File tree

10 files changed

+416
-42
lines changed

10 files changed

+416
-42
lines changed

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -550,9 +550,11 @@ public static Optional<Schema> getArraySchema(io.swagger.v3.oas.annotations.medi
550550

551551
if (arraySchema.schema() != null) {
552552
if (arraySchema.schema().implementation().equals(Void.class)) {
553-
getSchemaFromAnnotation(arraySchema.schema(), components, jsonViewAnnotation, openapi31, arraySchemaObject.getItems()).ifPresent(arraySchemaObject::setItems);
553+
getSchemaFromAnnotation(arraySchema.schema(), components, jsonViewAnnotation, openapi31, arraySchemaObject.getItems())
554+
.ifPresent(arraySchemaObject::setItems);
554555
} else if (processSchemaImplementation) {
555-
getSchema(arraySchema.schema(), arraySchema, false, arraySchema.schema().implementation(), components, jsonViewAnnotation, openapi31).ifPresent(arraySchemaObject::setItems);
556+
getSchema(arraySchema.schema(), arraySchema, false, arraySchema.schema().implementation(), components, jsonViewAnnotation, openapi31)
557+
.ifPresent(arraySchemaObject::setItems);
556558
}
557559
}
558560

@@ -1024,11 +1026,7 @@ public static Schema resolveSchemaFromType(Class<?> schemaImplementation, Compon
10241026
// Schema existingSchemaObject = clone(schemaObject, openapi31);
10251027
Schema existingSchemaObject = schemaObject;
10261028
if (openapi31) {
1027-
Optional<Schema> reResolvedSchema = getSchemaFromAnnotation(schemaAnnotation, components, jsonViewAnnotation, openapi31, existingSchemaObject);
1028-
if (reResolvedSchema.isPresent()) {
1029-
existingSchemaObject = reResolvedSchema.get();
1030-
}
1031-
reResolvedSchema = AnnotationsUtils.getArraySchema(arrayAnnotation, components, null, openapi31, existingSchemaObject);
1029+
Optional<Schema> reResolvedSchema = resolveSiblings(existingSchemaObject, components, jsonViewAnnotation, openapi31, schemaAnnotation, arrayAnnotation);
10321030
if (reResolvedSchema.isPresent()) {
10331031
existingSchemaObject = reResolvedSchema.get();
10341032
}
@@ -1040,6 +1038,29 @@ public static Schema resolveSchemaFromType(Class<?> schemaImplementation, Compon
10401038
return existingSchemaObject;
10411039
}
10421040

1041+
private static Optional<Schema> resolveSiblings(Schema existingSchemaObject,
1042+
Components components,
1043+
JsonView jsonViewAnnotation,
1044+
boolean openapi31,
1045+
io.swagger.v3.oas.annotations.media.Schema schemaAnnotation,
1046+
io.swagger.v3.oas.annotations.media.ArraySchema arrayAnnotation) {
1047+
Optional<Schema> reResolvedSchema;
1048+
reResolvedSchema = getSchemaFromAnnotation(schemaAnnotation, components, jsonViewAnnotation, openapi31, existingSchemaObject);
1049+
// Return the reResolvedSchema if the schemaAnnotation has incurred differences
1050+
if (reResolvedSchema.isPresent() && !reResolvedSchema.get().equals(existingSchemaObject)) {
1051+
return reResolvedSchema;
1052+
}
1053+
// If there were no changes based upon the schemaAnnotation, check the arrayAnnotation's schema
1054+
if (arrayAnnotation != null && arrayAnnotation.schema() != null) {
1055+
reResolvedSchema = getSchemaFromAnnotation(arrayAnnotation.schema(), components, jsonViewAnnotation, openapi31, existingSchemaObject);
1056+
if (reResolvedSchema.isPresent()) {
1057+
return reResolvedSchema;
1058+
}
1059+
}
1060+
// If neither of the schema annotations applied any changes, check the arrayAnnotation
1061+
return getArraySchema(arrayAnnotation, components, null, openapi31, existingSchemaObject);
1062+
}
1063+
10431064
public static Optional<Set<Tag>> getTags(io.swagger.v3.oas.annotations.tags.Tag[] tags, boolean skipOnlyName) {
10441065
if (tags == null) {
10451066
return Optional.empty();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public class ArrayOfSubclassTest {
2121
public void extractSubclassArray_oas31() throws Exception {
2222
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(ModelWithArrayOfSubclasses.Holder.class);
2323
assertNotNull(schema);
24-
String expectedJson = new String(Files.readAllBytes(Paths.get("src/test/java/io/swagger/v3/core/converting/ArrayOfSubclassTest_expected31.json")));
24+
String expectedJson = new String(Files.readAllBytes(Paths.get("src/test/resources/converting/ArrayOfSubclassTest_expected31.json")));
2525
String actualJson = Json31.pretty(schema);
2626
ObjectMapper mapper = new ObjectMapper();
2727
JsonNode expectedNode = mapper.readTree(expectedJson);
@@ -33,7 +33,7 @@ public void extractSubclassArray_oas31() throws Exception {
3333
public void extractSubclassArray_oas30() throws Exception {
3434
ResolvedSchema schema = ModelConverters.getInstance(false).readAllAsResolvedSchema(ModelWithArrayOfSubclasses.Holder.class);
3535
assertNotNull(schema);
36-
String expectedJson = new String(Files.readAllBytes(Paths.get("src/test/java/io/swagger/v3/core/converting/ArrayOfSubclassTest_expected30.json")));
36+
String expectedJson = new String(Files.readAllBytes(Paths.get("src/test/resources/converting/ArrayOfSubclassTest_expected30.json")));
3737
String actualJson = Json31.pretty(schema);
3838
ObjectMapper mapper = new ObjectMapper();
3939
JsonNode expectedNode = mapper.readTree(expectedJson);

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

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
package io.swagger.v3.core.converting;
2+
3+
import com.fasterxml.jackson.databind.JsonNode;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import io.swagger.v3.core.converter.ModelConverters;
6+
import io.swagger.v3.core.converter.ResolvedSchema;
7+
import io.swagger.v3.core.util.Json31;
8+
import io.swagger.v3.oas.annotations.media.ArraySchema;
9+
import org.testng.annotations.Test;
10+
11+
import java.util.List;
12+
13+
import static org.testng.Assert.*;
14+
15+
/**
16+
* test documenting the behavior of sibling @Schema and @ArraySchema annotations.
17+
*/
18+
public class Issue5055Test {
19+
20+
21+
@Test
22+
public void testArrayMetadataDoesNotLeakToItemsRef() throws Exception {
23+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
24+
TestModels.ArrayWithFullMetadata.class
25+
);
26+
27+
assertNotNull(schema, "Schema should resolve");
28+
String json = Json31.pretty(schema);
29+
assertNotNull(json);
30+
ObjectMapper mapper = new ObjectMapper();
31+
JsonNode root = mapper.readTree(json);
32+
33+
JsonNode itemsProp = root.at("/schema/properties/pets");
34+
assertFalse(itemsProp.isMissingNode(), "pets property should exist");
35+
36+
assertEquals(itemsProp.get("type").asText(), "array", "Should be array type");
37+
assertEquals(itemsProp.get("description").asText(), "Collection of pets", "Array should have description");
38+
assertEquals(itemsProp.get("minItems").asInt(), 1, "Array should have minItems");
39+
assertEquals(itemsProp.get("maxItems").asInt(), 100, "Array should have maxItems");
40+
assertTrue(itemsProp.get("uniqueItems").asBoolean(), "Array should have uniqueItems");
41+
42+
JsonNode items = itemsProp.get("items");
43+
assertNotNull(items, "Items should exist");
44+
assertTrue(items.has("$ref"), "Items should be a reference");
45+
46+
assertFalse(items.has("minItems"), "BUG: minItems leaked to items $ref");
47+
assertFalse(items.has("maxItems"), "BUG: maxItems leaked to items $ref");
48+
assertFalse(items.has("uniqueItems"), "BUG: uniqueItems leaked to items $ref");
49+
assertFalse(items.has("description") && items.get("description") != null,
50+
"BUG: array description leaked to items $ref");
51+
52+
if (items.has("type")) {
53+
assertTrue(items.get("type").isNull() || items.get("type").asText().isEmpty(),
54+
"BUG: type value leaked to items $ref");
55+
}
56+
}
57+
58+
@Test
59+
public void testComponentSchemaIsCleanWithoutLeakedProperties() throws Exception {
60+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
61+
TestModels.ArrayWithFullMetadata.class
62+
);
63+
64+
assertNotNull(schema);
65+
assertNotNull(schema.referencedSchemas);
66+
67+
io.swagger.v3.oas.models.media.Schema petSchema = schema.referencedSchemas.get("Pet");
68+
assertNotNull(petSchema, "Pet component schema should exist");
69+
70+
assertNull(petSchema.getMinItems(), "Component schema should not have minItems");
71+
assertNull(petSchema.getMaxItems(), "Component schema should not have maxItems");
72+
assertNull(petSchema.getUniqueItems(), "Component schema should not have uniqueItems");
73+
74+
if (petSchema.getDescription() != null) {
75+
assertNotEquals(petSchema.getDescription(), "Collection of pets",
76+
"Component should not have array's description");
77+
}
78+
}
79+
80+
@Test
81+
public void testArraySchemaAttributesSeparation() throws Exception {
82+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
83+
TestModels.BothArraySchemaAttributes.class
84+
);
85+
86+
assertNotNull(schema);
87+
String json = Json31.pretty(schema);
88+
89+
ObjectMapper mapper = new ObjectMapper();
90+
JsonNode root = mapper.readTree(json);
91+
92+
JsonNode dataField = root.at("/schema/properties/data");
93+
if (!dataField.isMissingNode()) {
94+
assertTrue(dataField.has("description"));
95+
assertEquals(dataField.get("type").asText(), "array");
96+
97+
JsonNode items = dataField.get("items");
98+
assertNotNull(items);
99+
assertTrue(items.has("description") || items.has("$ref"));
100+
}
101+
}
102+
103+
@Test
104+
public void testTypeInferenceWithNoImplementation() throws Exception {
105+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
106+
TestModels.NoImplementationSpecified.class
107+
);
108+
109+
assertNotNull(schema);
110+
String json = Json31.pretty(schema);
111+
112+
System.out.println("\n=== testTypeInferenceWithNoImplementation - Generated Spec ===");
113+
System.out.println(json);
114+
System.out.println("=== End Spec ===\n");
115+
116+
ObjectMapper mapper = new ObjectMapper();
117+
JsonNode root = mapper.readTree(json);
118+
119+
JsonNode dataField = root.at("/schema/properties/data");
120+
assertEquals(dataField.get("type").asText(), "array");
121+
122+
JsonNode items = dataField.get("items");
123+
assertNotNull(items);
124+
if (items.has("type")) {
125+
assertEquals(items.get("type").asText(), "string");
126+
}
127+
128+
}
129+
130+
@Test
131+
public void testSchemaEqualsSemanticsForPrecedence() {
132+
io.swagger.v3.oas.models.media.Schema unchanged = new io.swagger.v3.oas.models.media.Schema();
133+
unchanged.setType("array");
134+
135+
io.swagger.v3.oas.models.media.Schema alsoUnchanged = new io.swagger.v3.oas.models.media.Schema();
136+
alsoUnchanged.setType("array");
137+
138+
assertEquals(unchanged, alsoUnchanged, "Equal content should be detected");
139+
140+
io.swagger.v3.oas.models.media.Schema changed = new io.swagger.v3.oas.models.media.Schema();
141+
changed.setType("array");
142+
changed.setDescription("added");
143+
144+
assertNotEquals(unchanged, changed, "Changes should be detected by equals");
145+
}
146+
147+
@Test
148+
public void testSchemaEqualsIncludesExtensions() {
149+
io.swagger.v3.oas.models.media.Schema s1 = new io.swagger.v3.oas.models.media.Schema();
150+
s1.addExtension("x-prop", "value");
151+
152+
io.swagger.v3.oas.models.media.Schema s2 = new io.swagger.v3.oas.models.media.Schema();
153+
s2.addExtension("x-prop", "value");
154+
155+
assertEquals(s1, s2, "Same extensions should be equal");
156+
157+
io.swagger.v3.oas.models.media.Schema s3 = new io.swagger.v3.oas.models.media.Schema();
158+
s3.addExtension("x-prop", "different");
159+
160+
assertNotEquals(s1, s3, "Different extensions should not be equal");
161+
}
162+
163+
@Test
164+
public void testNoImplementationCase() throws Exception {
165+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
166+
TestModels.NoImplementationSpecified.class
167+
);
168+
169+
assertNotNull(schema);
170+
String json = Json31.pretty(schema);
171+
172+
ObjectMapper mapper = new ObjectMapper();
173+
JsonNode root = mapper.readTree(json);
174+
175+
JsonNode dataField = root.at("/schema/properties/data");
176+
assertFalse(dataField.isMissingNode(), "data property should exist");
177+
assertEquals(dataField.get("type").asText(), "array");
178+
assertEquals(dataField.get("description").asText(), "Inferred from type");
179+
180+
JsonNode items = dataField.get("items");
181+
assertNotNull(items);
182+
assertEquals(items.get("type").asText(), "string", "Items should infer string type");
183+
}
184+
185+
@Test
186+
public void testNoSchemaAtAllCase() throws Exception {
187+
ResolvedSchema schema = ModelConverters.getInstance(true).readAllAsResolvedSchema(
188+
TestModels.NoSchemaAnnotations.class
189+
);
190+
191+
assertNotNull(schema);
192+
String json = Json31.pretty(schema);
193+
194+
ObjectMapper mapper = new ObjectMapper();
195+
JsonNode root = mapper.readTree(json);
196+
197+
JsonNode dataField = root.at("/schema/properties/data");
198+
assertFalse(dataField.isMissingNode(), "data property should exist");
199+
assertEquals(dataField.get("type").asText(), "array");
200+
201+
JsonNode items = dataField.get("items");
202+
assertNotNull(items);
203+
assertEquals(items.get("type").asText(), "string", "Items should infer string type from List<String>");
204+
}
205+
206+
207+
public static class TestModels {
208+
209+
@io.swagger.v3.oas.annotations.media.Schema(description = "Model with full array metadata")
210+
public static class ArrayWithFullMetadata {
211+
private List<Pet> pets;
212+
213+
@ArraySchema(
214+
schema = @io.swagger.v3.oas.annotations.media.Schema(implementation = Pet.class),
215+
arraySchema = @io.swagger.v3.oas.annotations.media.Schema(
216+
type = "array",
217+
description = "Collection of pets"
218+
),
219+
minItems = 1,
220+
maxItems = 100,
221+
uniqueItems = true
222+
)
223+
public List<Pet> getPets() {
224+
return pets;
225+
}
226+
}
227+
228+
@io.swagger.v3.oas.annotations.media.Schema(description = "Pet model")
229+
public static class Pet {
230+
private String name;
231+
private String species;
232+
233+
public String getName() {
234+
return name;
235+
}
236+
237+
public String getSpecies() {
238+
return species;
239+
}
240+
}
241+
242+
@io.swagger.v3.oas.annotations.media.Schema(description = "Test precedence")
243+
public static class SchemaTakesPrecedence {
244+
private List<String> data;
245+
246+
@io.swagger.v3.oas.annotations.media.Schema(type = "array", description = "Schema description")
247+
@ArraySchema(minItems = 10)
248+
public List<String> getData() {
249+
return data;
250+
}
251+
}
252+
253+
@io.swagger.v3.oas.annotations.media.Schema(description = "Test both attributes")
254+
public static class BothArraySchemaAttributes {
255+
private List<String> data;
256+
257+
@ArraySchema(
258+
schema = @io.swagger.v3.oas.annotations.media.Schema(type = "string", description = "Item description"),
259+
arraySchema = @io.swagger.v3.oas.annotations.media.Schema(type = "array", description = "Array description")
260+
)
261+
public List<String> getData() {
262+
return data;
263+
}
264+
}
265+
266+
@io.swagger.v3.oas.annotations.media.Schema(description = "Test no implementation")
267+
public static class NoImplementationSpecified {
268+
private List<String> data;
269+
270+
@io.swagger.v3.oas.annotations.media.Schema(description = "Inferred from type")
271+
public List<String> getData() {
272+
return data;
273+
}
274+
}
275+
276+
@io.swagger.v3.oas.annotations.media.Schema(description = "Test no schema annotations")
277+
public static class NoSchemaAnnotations {
278+
private List<String> data;
279+
280+
public List<String> getData() {
281+
return data;
282+
}
283+
}
284+
}
285+
}

0 commit comments

Comments
 (0)