From d9cf8b573b0fbba9aef4d5ab39ac74902ecc988b Mon Sep 17 00:00:00 2001 From: yht0827 Date: Tue, 13 Jan 2026 11:34:22 +0900 Subject: [PATCH] feat: change Schema.required from List to Set (#5027) - Change internal storage from List to Set using TreeSet - Keep public API compatible (getRequired() returns a defensive copy as List) - 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 --- .../swagger/v3/oas/models/media/Schema.java | 49 ++++++---- .../v3/oas/models/media/SchemaTest.java | 98 +++++++++++++++++++ 2 files changed, 126 insertions(+), 21 deletions(-) diff --git a/modules/swagger-models/src/main/java/io/swagger/v3/oas/models/media/Schema.java b/modules/swagger-models/src/main/java/io/swagger/v3/oas/models/media/Schema.java index 5edd44602c..35f7979643 100644 --- a/modules/swagger-models/src/main/java/io/swagger/v3/oas/models/media/Schema.java +++ b/modules/swagger-models/src/main/java/io/swagger/v3/oas/models/media/Schema.java @@ -10,13 +10,13 @@ import java.math.BigDecimal; import java.util.ArrayList; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; /** * Schema @@ -91,7 +91,7 @@ public String toString() { private Boolean uniqueItems = null; private Integer maxProperties = null; private Integer minProperties = null; - private List required = null; + private Set required = null; @OpenAPI30 private String type = null; private Schema not = null; @@ -1229,42 +1229,49 @@ public Schema minProperties(Integer minProperties) { } /** - * returns the required property from a Schema instance. + * Returns a copy of the required property from a Schema instance. * - * @return List<String> required + *

Note: the returned list is a snapshot copy of the internal set. Mutating + * the returned list (e.g. {@code schema.getRequired().add("x")}) does + * not modify this schema. Use {@link #addRequiredItem(String)} or + * {@link #setRequired(List)} to change the required fields. + * + * @return a new {@link List} containing the required field names, or {@code null} if none are set **/ public List getRequired() { - return required; + if (required == null) { + return null; + } + return new ArrayList<>(required); } public void setRequired(List required) { - List list = new ArrayList<>(); - if (required != null) { - for (String req : required) { - if (this.properties == null || this.properties.containsKey(req)) { - list.add(req); - } - } + if (required == null) { + this.required = null; + return; } - Collections.sort(list); - if (list.isEmpty()) { - list = null; + Set set = new TreeSet<>(); + for (String req : required) { + if (this.properties == null || this.properties.containsKey(req)) { + set.add(req); + } } - this.required = list; + this.required = set.isEmpty() ? null : set; } public Schema required(List required) { - this.required = required; + this.setRequired(required); return this; } public Schema addRequiredItem(String requiredItem) { - if (this.required == null) { - this.required = new ArrayList<>(); + if (this.properties == null || this.properties.containsKey(requiredItem)) { + if (this.required == null) { + this.required = new TreeSet<>(); + } + this.required.add(requiredItem); } - this.required.add(requiredItem); - Collections.sort(required); return this; } diff --git a/modules/swagger-models/src/test/java/io/swagger/v3/oas/models/media/SchemaTest.java b/modules/swagger-models/src/test/java/io/swagger/v3/oas/models/media/SchemaTest.java index a3e8a7ec54..c3abc678d0 100644 --- a/modules/swagger-models/src/test/java/io/swagger/v3/oas/models/media/SchemaTest.java +++ b/modules/swagger-models/src/test/java/io/swagger/v3/oas/models/media/SchemaTest.java @@ -6,6 +6,7 @@ import java.math.BigDecimal; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; public class SchemaTest { @@ -178,4 +179,101 @@ private Schema createComplexSchema() { schema.setBooleanSchemaValue(true); return schema; } + + @Test + public void testRequiredNoDuplicates() { + Schema schema = new Schema<>(); + schema.addRequiredItem("id"); + schema.addRequiredItem("name"); + schema.addRequiredItem("id"); // duplicate + + List required = schema.getRequired(); + + // Duplicates should be removed + assertEquals(2, required.size()); + assertTrue(required.contains("id")); + assertTrue(required.contains("name")); + } + + @Test + public void testSetRequiredRemovesDuplicates() { + Schema schema = new Schema<>(); + schema.setRequired(Arrays.asList("id", "name", "id", "email", "name")); + + List required = schema.getRequired(); + + // Should contain only unique values: id, name, email + assertEquals(3, required.size()); + } + + + @Test + public void testRequiredIsSorted() { + Schema schema = new Schema<>(); + schema.addRequiredItem("zebra"); + schema.addRequiredItem("apple"); + schema.addRequiredItem("mango"); + + List required = schema.getRequired(); + + // Should be sorted alphabetically + assertEquals(Arrays.asList("apple", "mango", "zebra"), required); + } + + @Test + public void testSetRequiredWithNull() { + Schema schema = new Schema<>(); + schema.addRequiredItem("id"); + schema.setRequired(null); + + // Setting null should clear required + assertNull(schema.getRequired()); + } + + @Test + public void testSetRequiredWithEmptyList() { + Schema schema = new Schema<>(); + schema.setRequired(Arrays.asList()); + + // Empty list should result in null + assertNull(schema.getRequired()); + } + + @Test + public void testAddRequiredItemIgnoresItemNotInProperties() { + Schema schema = new Schema<>(); + Schema nameSchema = new Schema<>(); + schema.addProperty("name", nameSchema); + + schema.addRequiredItem("name"); // exists in properties → accepted + schema.addRequiredItem("notAProp"); // not in properties → ignored + + List required = schema.getRequired(); + assertEquals(1, required.size()); + assertTrue(required.contains("name")); + assertFalse(required.contains("notAProp")); + } + + @Test + public void testAddRequiredItemAcceptsAllWhenNoProperties() { + Schema schema = new Schema<>(); + // no properties set → all items accepted + schema.addRequiredItem("id"); + schema.addRequiredItem("name"); + + List required = schema.getRequired(); + assertEquals(2, required.size()); + } + + @Test + public void testGetRequiredReturnsCopy() { + Schema schema = new Schema<>(); + schema.addRequiredItem("id"); + + List copy = schema.getRequired(); + copy.add("shouldNotAffectSchema"); + + // original schema should be unchanged + assertEquals(1, schema.getRequired().size()); + } }