Skip to content

Commit 76491e1

Browse files
ewaostrowskaewaostrowska
committed
⏺ fix: set @nullable on object ref fields at the property site, not the component schema
1 parent 96800d8 commit 76491e1

7 files changed

Lines changed: 196 additions & 14 deletions

File tree

‎modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,13 @@ public Schema resolve(AnnotatedType annotatedType, ModelConverterContext context
907907
}
908908
}
909909
}
910+
// A @Nullable object property must not mark the shared component schema as nullable,
911+
// because that would corrupt every other reference to the same component.
912+
// Instead, keep the component as type:object and express nullable only at this
913+
// property's reference: oneOf[$ref, {type:null}] for OAS 3.1, nullable+allOf[$ref] for OAS 3.0.
914+
if (property != null && property.get$ref() != null && hasNullableAnnotation(annotations)) {
915+
property = wrapNullableRef(property);
916+
}
910917
property.setName(propName);
911918
JAXBAnnotationsHelper.apply(propBeanDesc.getClassInfo(), annotations, property);
912919
if (property != null && io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED.equals(requiredMode)) {
@@ -3545,6 +3552,34 @@ private boolean isNullableSchema(Schema schema, io.swagger.v3.oas.annotations.me
35453552
return isObjectSchema(schema) && schema.getAdditionalProperties() != null;
35463553
}
35473554

3555+
private boolean hasNullableAnnotation(Annotation[] annotations) {
3556+
if (annotations == null) {
3557+
return false;
3558+
}
3559+
for (Annotation annotation : annotations) {
3560+
if (NULLABLE_ANNOTATIONS.contains(annotation.annotationType().getSimpleName())) {
3561+
return true;
3562+
}
3563+
}
3564+
return false;
3565+
}
3566+
3567+
private Schema wrapNullableRef(Schema refSchema) {
3568+
if (openapi31) {
3569+
Schema nullTypeSchema = new JsonSchema();
3570+
nullTypeSchema.addType("null");
3571+
Schema composed = new JsonSchema();
3572+
composed.addOneOfItem(refSchema);
3573+
composed.addOneOfItem(nullTypeSchema);
3574+
return composed;
3575+
} else {
3576+
Schema composed = new Schema();
3577+
composed.setNullable(true);
3578+
composed.addAllOfItem(refSchema);
3579+
return composed;
3580+
}
3581+
}
3582+
35483583
protected boolean isObjectSchema(Schema schema) {
35493584
return SchemaTypeUtils.isObjectSchema(schema);
35503585
}

‎modules/swagger-core/src/test/java/io/swagger/v3/core/issues/Issue5115Test.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,13 @@
1212

1313
import javax.annotation.Nullable;
1414
import java.io.IOException;
15+
import java.util.Collections;
16+
import java.util.LinkedHashSet;
1517

1618
import static org.testng.Assert.assertEquals;
19+
import static org.testng.Assert.assertFalse;
20+
import static org.testng.Assert.assertNotNull;
21+
import static org.testng.Assert.assertNull;
1722

1823
/**
1924
* Reproduces GitHub Issue #5115
@@ -79,6 +84,98 @@ public void testObjectKeepsInvalidNullableSchemaIfSetInSchemaAnnotationOAS30() t
7984
assertEquals(actualNode, expectedNode);
8085
}
8186

87+
@Test
88+
public void testComponentSchemaStableRegardlessOfNullableFieldOrderOAS31() {
89+
ResolvedSchema nullableFirst = ModelConverters.getInstance(true)
90+
.readAllAsResolvedSchema(ContainerNullableFirst.class);
91+
ResolvedSchema nonNullableFirst = ModelConverters.getInstance(true)
92+
.readAllAsResolvedSchema(ContainerNonNullableFirst.class);
93+
94+
io.swagger.v3.oas.models.media.Schema nestedFromNullableFirst = nullableFirst.referencedSchemas.get("NestedModel");
95+
io.swagger.v3.oas.models.media.Schema nestedFromNonNullableFirst = nonNullableFirst.referencedSchemas.get("NestedModel");
96+
97+
assertNotNull(nestedFromNullableFirst);
98+
assertNotNull(nestedFromNonNullableFirst);
99+
100+
LinkedHashSet<String> expectedTypes = new LinkedHashSet<>(Collections.singletonList("object"));
101+
assertFalse(nestedFromNullableFirst.getTypes().contains("null"),
102+
"NestedModel must not contain null type when nullable field is resolved first");
103+
assertEquals(nestedFromNullableFirst.getTypes(), expectedTypes);
104+
assertEquals(nestedFromNonNullableFirst.getTypes(), expectedTypes,
105+
"NestedModel component schema must be identical regardless of field resolution order");
106+
}
107+
108+
@Test
109+
public void testComponentSchemaStableRegardlessOfNullableFieldOrderOAS30() {
110+
ResolvedSchema nullableFirst = ModelConverters.getInstance()
111+
.readAllAsResolvedSchema(ContainerNullableFirst.class);
112+
ResolvedSchema nonNullableFirst = ModelConverters.getInstance()
113+
.readAllAsResolvedSchema(ContainerNonNullableFirst.class);
114+
115+
io.swagger.v3.oas.models.media.Schema nestedFromNullableFirst = nullableFirst.referencedSchemas.get("NestedModel");
116+
io.swagger.v3.oas.models.media.Schema nestedFromNonNullableFirst = nonNullableFirst.referencedSchemas.get("NestedModel");
117+
118+
assertNotNull(nestedFromNullableFirst);
119+
assertNotNull(nestedFromNonNullableFirst);
120+
121+
assertEquals(nestedFromNullableFirst.getType(), "object");
122+
assertNull(nestedFromNullableFirst.getNullable(),
123+
"NestedModel must not be nullable when nullable field is resolved first");
124+
assertEquals(nestedFromNonNullableFirst.getType(), "object");
125+
assertNull(nestedFromNonNullableFirst.getNullable(),
126+
"NestedModel component schema must be identical regardless of field resolution order");
127+
}
128+
129+
public static class ContainerNullableFirst {
130+
131+
@Nullable
132+
private NestedModel nullableModel;
133+
134+
private NestedModel model;
135+
136+
@Nullable
137+
public NestedModel getNullableModel() {
138+
return nullableModel;
139+
}
140+
141+
public void setNullableModel(@Nullable NestedModel nullableModel) {
142+
this.nullableModel = nullableModel;
143+
}
144+
145+
public NestedModel getModel() {
146+
return model;
147+
}
148+
149+
public void setModel(NestedModel model) {
150+
this.model = model;
151+
}
152+
}
153+
154+
public static class ContainerNonNullableFirst {
155+
156+
private NestedModel model;
157+
158+
@Nullable
159+
private NestedModel nullableModel;
160+
161+
public NestedModel getModel() {
162+
return model;
163+
}
164+
165+
public void setModel(NestedModel model) {
166+
this.model = model;
167+
}
168+
169+
@Nullable
170+
public NestedModel getNullableModel() {
171+
return nullableModel;
172+
}
173+
174+
public void setNullableModel(@Nullable NestedModel nullableModel) {
175+
this.nullableModel = nullableModel;
176+
}
177+
}
178+
82179
public static class ModelWithObject {
83180

84181
@Nullable

‎modules/swagger-core/src/test/resources/specFiles/NullableFieldsOAS30.json‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
"type" : "string"
1111
},
1212
"nullableModel" : {
13-
"$ref" : "#/components/schemas/Model"
13+
"nullable" : true,
14+
"allOf" : [ {
15+
"$ref" : "#/components/schemas/Model"
16+
} ]
1417
},
1518
"strings" : {
1619
"type" : "array",
@@ -48,7 +51,10 @@
4851
"type" : "string"
4952
},
5053
"nullableModel" : {
51-
"$ref" : "#/components/schemas/Model"
54+
"nullable" : true,
55+
"allOf" : [ {
56+
"$ref" : "#/components/schemas/Model"
57+
} ]
5258
},
5359
"strings" : {
5460
"type" : "array",

‎modules/swagger-core/src/test/resources/specFiles/NullableFieldsOAS31.json‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
"type" : "string"
1010
},
1111
"nullableModel" : {
12-
"$ref" : "#/components/schemas/Model"
12+
"oneOf" : [ {
13+
"$ref" : "#/components/schemas/Model"
14+
}, {
15+
"type" : "null"
16+
} ]
1317
},
1418
"strings" : {
1519
"type" : [ "array", "null" ],
@@ -44,7 +48,11 @@
4448
"type" : "string"
4549
},
4650
"nullableModel" : {
47-
"$ref" : "#/components/schemas/Model"
51+
"oneOf" : [ {
52+
"$ref" : "#/components/schemas/Model"
53+
}, {
54+
"type" : "null"
55+
} ]
4856
},
4957
"strings" : {
5058
"type" : [ "array", "null" ],

‎modules/swagger-core/src/test/resources/specFiles/NullableObjectFieldsOAS30.json‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
"type" : "object",
44
"properties" : {
55
"nullableModel" : {
6-
"$ref" : "#/components/schemas/Model"
6+
"nullable" : true,
7+
"allOf" : [ {
8+
"$ref" : "#/components/schemas/Model"
9+
} ]
710
},
811
"model" : {
912
"$ref" : "#/components/schemas/Model"
@@ -18,10 +21,16 @@
1821
"$ref" : "#/components/schemas/NestedModel"
1922
},
2023
"nullableNestedModel" : {
21-
"$ref" : "#/components/schemas/NestedModel"
24+
"nullable" : true,
25+
"allOf" : [ {
26+
"$ref" : "#/components/schemas/NestedModel"
27+
} ]
2228
},
2329
"nullableNestedModel2" : {
24-
"$ref" : "#/components/schemas/NestedModel2"
30+
"nullable" : true,
31+
"allOf" : [ {
32+
"$ref" : "#/components/schemas/NestedModel2"
33+
} ]
2534
},
2635
"nestedModel2" : {
2736
"$ref" : "#/components/schemas/NestedModel2"
@@ -32,7 +41,10 @@
3241
"type" : "object",
3342
"properties" : {
3443
"nullableModel" : {
35-
"$ref" : "#/components/schemas/Model"
44+
"nullable" : true,
45+
"allOf" : [ {
46+
"$ref" : "#/components/schemas/Model"
47+
} ]
3648
},
3749
"model" : {
3850
"$ref" : "#/components/schemas/Model"

‎modules/swagger-core/src/test/resources/specFiles/NullableObjectFieldsOAS31.json‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
"type" : "object",
44
"properties" : {
55
"nullableModel" : {
6-
"$ref" : "#/components/schemas/Model"
6+
"oneOf" : [ {
7+
"$ref" : "#/components/schemas/Model"
8+
}, {
9+
"type" : "null"
10+
} ]
711
},
812
"model" : {
913
"$ref" : "#/components/schemas/Model"
@@ -18,10 +22,18 @@
1822
"$ref" : "#/components/schemas/NestedModel"
1923
},
2024
"nullableNestedModel" : {
21-
"$ref" : "#/components/schemas/NestedModel"
25+
"oneOf" : [ {
26+
"$ref" : "#/components/schemas/NestedModel"
27+
}, {
28+
"type" : "null"
29+
} ]
2230
},
2331
"nullableNestedModel2" : {
24-
"$ref" : "#/components/schemas/NestedModel2"
32+
"oneOf" : [ {
33+
"$ref" : "#/components/schemas/NestedModel2"
34+
}, {
35+
"type" : "null"
36+
} ]
2537
},
2638
"nestedModel2" : {
2739
"$ref" : "#/components/schemas/NestedModel2"
@@ -32,7 +44,11 @@
3244
"type" : "object",
3345
"properties" : {
3446
"nullableModel" : {
35-
"$ref" : "#/components/schemas/Model"
47+
"oneOf" : [ {
48+
"$ref" : "#/components/schemas/Model"
49+
}, {
50+
"type" : "null"
51+
} ]
3652
},
3753
"model" : {
3854
"$ref" : "#/components/schemas/Model"

‎modules/swagger-core/src/test/resources/specFiles/NullableObjectFieldsSchemaAnnotationOAS31.json‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,18 @@
1919
"$ref" : "#/components/schemas/NestedModel"
2020
},
2121
"nullableNestedModel" : {
22-
"$ref" : "#/components/schemas/NestedModel"
22+
"oneOf" : [ {
23+
"$ref" : "#/components/schemas/NestedModel"
24+
}, {
25+
"type" : "null"
26+
} ]
2327
},
2428
"nullableNestedModel2" : {
25-
"$ref" : "#/components/schemas/NestedModel2"
29+
"oneOf" : [ {
30+
"$ref" : "#/components/schemas/NestedModel2"
31+
}, {
32+
"type" : "null"
33+
} ]
2634
},
2735
"nestedModel2" : {
2836
"$ref" : "#/components/schemas/NestedModel2"

0 commit comments

Comments
 (0)