Skip to content

Commit d9cf8b5

Browse files
committed
feat: change Schema.required from List to Set (#5027)
- Change internal storage from List<String> to Set<String> using TreeSet - Keep public API compatible (getRequired() returns a defensive copy as List<String>) - Make addRequiredItem() consistent with setRequired(): ignore items not present in properties - Add JavaDoc to getRequired() explaining the defensive copy behavior - Add unit tests for duplicate handling, sorting, and consisteny
1 parent ca8b4b5 commit d9cf8b5

2 files changed

Lines changed: 126 additions & 21 deletions

File tree

  • modules/swagger-models/src

modules/swagger-models/src/main/java/io/swagger/v3/oas/models/media/Schema.java

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010

1111
import java.math.BigDecimal;
1212
import java.util.ArrayList;
13-
import java.util.Collections;
1413
import java.util.LinkedHashMap;
1514
import java.util.LinkedHashSet;
1615
import java.util.List;
1716
import java.util.Map;
1817
import java.util.Objects;
1918
import java.util.Set;
19+
import java.util.TreeSet;
2020

2121
/**
2222
* Schema
@@ -91,7 +91,7 @@ public String toString() {
9191
private Boolean uniqueItems = null;
9292
private Integer maxProperties = null;
9393
private Integer minProperties = null;
94-
private List<String> required = null;
94+
private Set<String> required = null;
9595
@OpenAPI30
9696
private String type = null;
9797
private Schema not = null;
@@ -1229,42 +1229,49 @@ public Schema minProperties(Integer minProperties) {
12291229
}
12301230

12311231
/**
1232-
* returns the required property from a Schema instance.
1232+
* Returns a copy of the required property from a Schema instance.
12331233
*
1234-
* @return List&lt;String&gt; required
1234+
* <p>Note: the returned list is a snapshot copy of the internal set. Mutating
1235+
* the returned list (e.g. {@code schema.getRequired().add("x")}) does
1236+
* <em>not</em> modify this schema. Use {@link #addRequiredItem(String)} or
1237+
* {@link #setRequired(List)} to change the required fields.
1238+
*
1239+
* @return a new {@link List} containing the required field names, or {@code null} if none are set
12351240
**/
12361241

12371242
public List<String> getRequired() {
1238-
return required;
1243+
if (required == null) {
1244+
return null;
1245+
}
1246+
return new ArrayList<>(required);
12391247
}
12401248

12411249
public void setRequired(List<String> required) {
1242-
List<String> list = new ArrayList<>();
1243-
if (required != null) {
1244-
for (String req : required) {
1245-
if (this.properties == null || this.properties.containsKey(req)) {
1246-
list.add(req);
1247-
}
1248-
}
1250+
if (required == null) {
1251+
this.required = null;
1252+
return;
12491253
}
1250-
Collections.sort(list);
1251-
if (list.isEmpty()) {
1252-
list = null;
1254+
Set<String> set = new TreeSet<>();
1255+
for (String req : required) {
1256+
if (this.properties == null || this.properties.containsKey(req)) {
1257+
set.add(req);
1258+
}
12531259
}
1254-
this.required = list;
1260+
this.required = set.isEmpty() ? null : set;
12551261
}
12561262

12571263
public Schema required(List<String> required) {
1258-
this.required = required;
1264+
this.setRequired(required);
12591265
return this;
12601266
}
12611267

12621268
public Schema addRequiredItem(String requiredItem) {
1263-
if (this.required == null) {
1264-
this.required = new ArrayList<>();
1269+
if (this.properties == null || this.properties.containsKey(requiredItem)) {
1270+
if (this.required == null) {
1271+
this.required = new TreeSet<>();
1272+
}
1273+
this.required.add(requiredItem);
12651274
}
1266-
this.required.add(requiredItem);
1267-
Collections.sort(required);
12681275
return this;
12691276
}
12701277

modules/swagger-models/src/test/java/io/swagger/v3/oas/models/media/SchemaTest.java

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.math.BigDecimal;
77
import java.util.Arrays;
88
import java.util.HashMap;
9+
import java.util.List;
910
import java.util.Map;
1011

1112
public class SchemaTest {
@@ -178,4 +179,101 @@ private Schema<Object> createComplexSchema() {
178179
schema.setBooleanSchemaValue(true);
179180
return schema;
180181
}
182+
183+
@Test
184+
public void testRequiredNoDuplicates() {
185+
Schema<Object> schema = new Schema<>();
186+
schema.addRequiredItem("id");
187+
schema.addRequiredItem("name");
188+
schema.addRequiredItem("id"); // duplicate
189+
190+
List<String> required = schema.getRequired();
191+
192+
// Duplicates should be removed
193+
assertEquals(2, required.size());
194+
assertTrue(required.contains("id"));
195+
assertTrue(required.contains("name"));
196+
}
197+
198+
@Test
199+
public void testSetRequiredRemovesDuplicates() {
200+
Schema<Object> schema = new Schema<>();
201+
schema.setRequired(Arrays.asList("id", "name", "id", "email", "name"));
202+
203+
List<String> required = schema.getRequired();
204+
205+
// Should contain only unique values: id, name, email
206+
assertEquals(3, required.size());
207+
}
208+
209+
210+
@Test
211+
public void testRequiredIsSorted() {
212+
Schema<Object> schema = new Schema<>();
213+
schema.addRequiredItem("zebra");
214+
schema.addRequiredItem("apple");
215+
schema.addRequiredItem("mango");
216+
217+
List<String> required = schema.getRequired();
218+
219+
// Should be sorted alphabetically
220+
assertEquals(Arrays.asList("apple", "mango", "zebra"), required);
221+
}
222+
223+
@Test
224+
public void testSetRequiredWithNull() {
225+
Schema<Object> schema = new Schema<>();
226+
schema.addRequiredItem("id");
227+
schema.setRequired(null);
228+
229+
// Setting null should clear required
230+
assertNull(schema.getRequired());
231+
}
232+
233+
@Test
234+
public void testSetRequiredWithEmptyList() {
235+
Schema<Object> schema = new Schema<>();
236+
schema.setRequired(Arrays.asList());
237+
238+
// Empty list should result in null
239+
assertNull(schema.getRequired());
240+
}
241+
242+
@Test
243+
public void testAddRequiredItemIgnoresItemNotInProperties() {
244+
Schema<Object> schema = new Schema<>();
245+
Schema<String> nameSchema = new Schema<>();
246+
schema.addProperty("name", nameSchema);
247+
248+
schema.addRequiredItem("name"); // exists in properties → accepted
249+
schema.addRequiredItem("notAProp"); // not in properties → ignored
250+
251+
List<String> required = schema.getRequired();
252+
assertEquals(1, required.size());
253+
assertTrue(required.contains("name"));
254+
assertFalse(required.contains("notAProp"));
255+
}
256+
257+
@Test
258+
public void testAddRequiredItemAcceptsAllWhenNoProperties() {
259+
Schema<Object> schema = new Schema<>();
260+
// no properties set → all items accepted
261+
schema.addRequiredItem("id");
262+
schema.addRequiredItem("name");
263+
264+
List<String> required = schema.getRequired();
265+
assertEquals(2, required.size());
266+
}
267+
268+
@Test
269+
public void testGetRequiredReturnsCopy() {
270+
Schema<Object> schema = new Schema<>();
271+
schema.addRequiredItem("id");
272+
273+
List<String> copy = schema.getRequired();
274+
copy.add("shouldNotAffectSchema");
275+
276+
// original schema should be unchanged
277+
assertEquals(1, schema.getRequired().size());
278+
}
181279
}

0 commit comments

Comments
 (0)