Skip to content

Commit 5e9b32c

Browse files
thcedclaude
thced
and
claude
committed
feat: Compose combinators with sibling base assertions in SchemaParser
Previously the parser dispatched in priority order and emitted exactly one schema record, silently discarding combinators that coexisted with type / properties / etc. Now each combinator and any base assertion contribute to an assertions list; multiple assertions wrap in an implicit AllOfSchema. allOf branches flatten into the outer list. Adds 7 parser tests for the composition path and a regression guard that a lone combinator still returns the bare combinator record. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2fc24b5 commit 5e9b32c

2 files changed

Lines changed: 169 additions & 7 deletions

File tree

‎src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java‎

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,35 @@ public static Schema parse(Map<String, Object> raw) {
1818
return new RefSchema((String) raw.get("$ref"));
1919
}
2020

21-
if (raw.containsKey("oneOf")) {
22-
return new OneOfSchema(parseList(raw, "oneOf"));
21+
List<Schema> assertions = new ArrayList<>();
22+
23+
Schema base = parseBaseIfPresent(raw);
24+
if (base != null) {
25+
assertions.add(base);
26+
}
27+
28+
if (raw.containsKey("allOf")) {
29+
assertions.addAll(parseList(raw, "allOf"));
2330
}
2431
if (raw.containsKey("anyOf")) {
25-
return new AnyOfSchema(parseList(raw, "anyOf"));
32+
assertions.add(new AnyOfSchema(parseList(raw, "anyOf")));
2633
}
27-
if (raw.containsKey("allOf")) {
28-
return new AllOfSchema(parseList(raw, "allOf"));
34+
if (raw.containsKey("oneOf")) {
35+
assertions.add(new OneOfSchema(parseList(raw, "oneOf")));
2936
}
3037
if (raw.containsKey("not")) {
31-
return new NotSchema(parse((Map<String, Object>) raw.get("not")));
38+
assertions.add(new NotSchema(parse((Map<String, Object>) raw.get("not"))));
3239
}
40+
41+
return switch (assertions.size()) {
42+
case 0 -> permissiveObject();
43+
case 1 -> assertions.getFirst();
44+
default -> new AllOfSchema(List.copyOf(assertions));
45+
};
46+
}
47+
48+
@SuppressWarnings("unchecked")
49+
private static Schema parseBaseIfPresent(Map<String, Object> raw) {
3350
if (raw.containsKey("const")) {
3451
return new ConstSchema(raw.get("const"));
3552
}
@@ -38,11 +55,20 @@ public static Schema parse(Map<String, Object> raw) {
3855
}
3956

4057
Set<TypeName> types = parseTypes(raw);
58+
if (types.isEmpty() && !hasObjectShapeKeywords(raw) && !hasArrayShapeKeywords(raw)) {
59+
return null;
60+
}
4161

42-
// Pick primary (non-null) type for record dispatch.
4362
TypeName primary =
4463
types.stream().filter(t -> t != TypeName.NULL).findFirst().orElse(TypeName.NULL);
4564

65+
if (types.isEmpty() && hasObjectShapeKeywords(raw)) {
66+
return parseObject(raw, types);
67+
}
68+
if (types.isEmpty() && hasArrayShapeKeywords(raw)) {
69+
return parseArray(raw, types);
70+
}
71+
4672
return switch (primary) {
4773
case STRING -> parseString(raw, types);
4874
case INTEGER -> parseInteger(raw, types);
@@ -54,6 +80,26 @@ public static Schema parse(Map<String, Object> raw) {
5480
};
5581
}
5682

83+
private static boolean hasObjectShapeKeywords(Map<String, Object> raw) {
84+
return raw.containsKey("properties")
85+
|| raw.containsKey("required")
86+
|| raw.containsKey("additionalProperties")
87+
|| raw.containsKey("minProperties")
88+
|| raw.containsKey("maxProperties");
89+
}
90+
91+
private static boolean hasArrayShapeKeywords(Map<String, Object> raw) {
92+
return raw.containsKey("items")
93+
|| raw.containsKey("minItems")
94+
|| raw.containsKey("maxItems")
95+
|| raw.containsKey("uniqueItems");
96+
}
97+
98+
private static Schema permissiveObject() {
99+
return new ObjectSchema(
100+
Set.of(), Map.of(), List.of(), new AdditionalProperties.Allowed(), null, null);
101+
}
102+
57103
private static Set<TypeName> parseTypes(Map<String, Object> raw) {
58104
Object t = raw.get("type");
59105
EnumSet<TypeName> out = EnumSet.noneOf(TypeName.class);

‎src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,15 @@ void parsesOneOf() {
121121
void parsesAnyOfAllOfNot() {
122122
assertThat(SchemaParser.parse(Map.of("anyOf", List.of(Map.of("type", "string")))))
123123
.isInstanceOf(AnyOfSchema.class);
124+
// allOf with a single branch flattens to the branch itself (no wrapper AllOfSchema).
124125
assertThat(SchemaParser.parse(Map.of("allOf", List.of(Map.of("type", "string")))))
126+
.isInstanceOf(StringSchema.class);
127+
// allOf with multiple branches flattens into AllOfSchema.
128+
assertThat(
129+
SchemaParser.parse(
130+
Map.of(
131+
"allOf",
132+
List.of(Map.of("type", "string"), Map.of("type", "string", "minLength", 1)))))
125133
.isInstanceOf(AllOfSchema.class);
126134
assertThat(SchemaParser.parse(Map.of("not", Map.of("type", "null"))))
127135
.isInstanceOf(NotSchema.class);
@@ -146,4 +154,112 @@ void enumOnStringStaysAsStringSchema() {
146154
assertThat(s).isInstanceOf(StringSchema.class);
147155
assertThat(((StringSchema) s).enumValues()).containsExactly("a", "b");
148156
}
157+
158+
@Test
159+
void allOfWithSiblingTypeWrapsInImplicitAllOf() {
160+
Schema s =
161+
SchemaParser.parse(
162+
Map.of(
163+
"type", "object",
164+
"required", List.of("x"),
165+
"allOf", List.of(Map.of("type", "object", "required", List.of("y")))));
166+
assertThat(s).isInstanceOf(AllOfSchema.class);
167+
AllOfSchema all = (AllOfSchema) s;
168+
assertThat(all.parts()).hasSize(2);
169+
assertThat(all.parts().get(0)).isInstanceOf(ObjectSchema.class);
170+
assertThat(((ObjectSchema) all.parts().get(0)).required()).containsExactly("x");
171+
assertThat(all.parts().get(1)).isInstanceOf(ObjectSchema.class);
172+
assertThat(((ObjectSchema) all.parts().get(1)).required()).containsExactly("y");
173+
}
174+
175+
@Test
176+
void anyOfWithSiblingTypeWrapsInImplicitAllOf() {
177+
Schema s =
178+
SchemaParser.parse(
179+
Map.of(
180+
"type",
181+
"string",
182+
"anyOf",
183+
List.of(
184+
Map.of("type", "string", "minLength", 1),
185+
Map.of("type", "string", "maxLength", 10))));
186+
assertThat(s).isInstanceOf(AllOfSchema.class);
187+
AllOfSchema all = (AllOfSchema) s;
188+
assertThat(all.parts()).hasSize(2);
189+
assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class);
190+
assertThat(all.parts().get(1)).isInstanceOf(AnyOfSchema.class);
191+
assertThat(((AnyOfSchema) all.parts().get(1)).options()).hasSize(2);
192+
}
193+
194+
@Test
195+
void oneOfWithSiblingTypeWrapsInImplicitAllOf() {
196+
Schema s =
197+
SchemaParser.parse(
198+
Map.of(
199+
"type",
200+
"string",
201+
"oneOf",
202+
List.of(
203+
Map.of("type", "string", "minLength", 1),
204+
Map.of("type", "string", "maxLength", 10))));
205+
assertThat(s).isInstanceOf(AllOfSchema.class);
206+
AllOfSchema all = (AllOfSchema) s;
207+
assertThat(all.parts()).hasSize(2);
208+
assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class);
209+
assertThat(all.parts().get(1)).isInstanceOf(OneOfSchema.class);
210+
}
211+
212+
@Test
213+
void notWithSiblingTypeWrapsInImplicitAllOf() {
214+
Schema s =
215+
SchemaParser.parse(
216+
Map.of("type", "string", "not", Map.of("type", "string", "maxLength", 2)));
217+
assertThat(s).isInstanceOf(AllOfSchema.class);
218+
AllOfSchema all = (AllOfSchema) s;
219+
assertThat(all.parts()).hasSize(2);
220+
assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class);
221+
assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class);
222+
}
223+
224+
@Test
225+
void multipleCombinatorsInOneSchemaWrapInAllOf() {
226+
Schema s =
227+
SchemaParser.parse(
228+
Map.of(
229+
"anyOf", List.of(Map.of("type", "string"), Map.of("type", "integer")),
230+
"not", Map.of("type", "boolean")));
231+
assertThat(s).isInstanceOf(AllOfSchema.class);
232+
AllOfSchema all = (AllOfSchema) s;
233+
assertThat(all.parts()).hasSize(2);
234+
assertThat(all.parts().get(0)).isInstanceOf(AnyOfSchema.class);
235+
assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class);
236+
}
237+
238+
@Test
239+
void allOfBranchesFlattenIntoOuterAllOf() {
240+
Schema s =
241+
SchemaParser.parse(
242+
Map.of(
243+
"type",
244+
"string",
245+
"allOf",
246+
List.of(
247+
Map.of("type", "string", "minLength", 1),
248+
Map.of("type", "string", "maxLength", 10))));
249+
assertThat(s).isInstanceOf(AllOfSchema.class);
250+
AllOfSchema all = (AllOfSchema) s;
251+
// Base + the two allOf branches flattened.
252+
assertThat(all.parts()).hasSize(3);
253+
}
254+
255+
@Test
256+
void aloneCombinatorStillReturnsCombinatorRecord() {
257+
// Regression: when no base assertions are present, the result is still the
258+
// single combinator record, not an AllOfSchema with a single child.
259+
Schema s =
260+
SchemaParser.parse(
261+
Map.of("oneOf", List.of(Map.of("type", "string"), Map.of("type", "integer"))));
262+
assertThat(s).isInstanceOf(OneOfSchema.class);
263+
assertThat(((OneOfSchema) s).options()).hasSize(2);
264+
}
149265
}

0 commit comments

Comments
 (0)