Skip to content

Commit d353b23

Browse files
thcedclaude
andcommitted
fix: Address code review for schema-booleans parser
- Align NeverSchema validator stub with the spec: keyword is now "false" and the rejected value is passed through via fail() instead of require() - Drop residual Map cast in parseAdditionalProperties; parse(value) accepts Object directly so no unchecked cast is needed - Static-import assertThatThrownBy in SchemaParserTest for consistency with the existing assertThat static import - Add parsesArrayWithBooleanFalseItems test covering items: false -> NeverSchema - Add rejectsNullRawSchema test covering null raw input -> IllegalArgumentException Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 798598c commit d353b23

3 files changed

Lines changed: 18 additions & 3 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ private static AdditionalProperties parseAdditionalProperties(Object value) {
139139
case null -> new AdditionalProperties.Allowed();
140140
case Boolean b when b -> new AdditionalProperties.Allowed();
141141
case Boolean _ -> new AdditionalProperties.Forbidden();
142-
default -> new AdditionalProperties.SchemaConstraint(parse((Map<String, Object>) value));
142+
default -> new AdditionalProperties.SchemaConstraint(parse(value));
143143
};
144144
}
145145

src/main/java/com/retailsvc/http/validate/DefaultValidator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ case EnumSchema(List<Object> values) ->
6767
case ConstSchema(Object expected) ->
6868
require(Objects.equals(expected, value), pointer, "const", "value does not equal const");
6969
case AlwaysSchema _ -> {}
70-
case NeverSchema _ -> require(false, pointer, "schema", "value rejected by false schema");
70+
case NeverSchema _ -> fail(pointer, "false", "schema rejects all values", value);
7171
case OneOfSchema _ -> throw new UnsupportedOperationException("oneOf not yet supported");
7272
case AnyOfSchema _ -> throw new UnsupportedOperationException("anyOf not yet supported");
7373
case AllOfSchema _ -> throw new UnsupportedOperationException("allOf not yet supported");

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.retailsvc.http.spec.schema;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
45

56
import java.util.List;
67
import java.util.Map;
@@ -159,7 +160,7 @@ void parsesFalseAsNeverSchema() {
159160

160161
@Test
161162
void rejectsNonMapNonBooleanRawSchema() {
162-
org.assertj.core.api.Assertions.assertThatThrownBy(() -> SchemaParser.parse("oops"))
163+
assertThatThrownBy(() -> SchemaParser.parse("oops"))
163164
.isInstanceOf(IllegalArgumentException.class)
164165
.hasMessageContaining("schema must be a boolean or an object");
165166
}
@@ -181,4 +182,18 @@ void parsesArrayWithBooleanItemsSchema() {
181182
assertThat(s).isInstanceOf(ArraySchema.class);
182183
assertThat(((ArraySchema) s).items()).isInstanceOf(AlwaysSchema.class);
183184
}
185+
186+
@Test
187+
void parsesArrayWithBooleanFalseItems() {
188+
Schema s = SchemaParser.parse(Map.of("type", "array", "items", Boolean.FALSE));
189+
assertThat(s).isInstanceOf(ArraySchema.class);
190+
assertThat(((ArraySchema) s).items()).isInstanceOf(NeverSchema.class);
191+
}
192+
193+
@Test
194+
void rejectsNullRawSchema() {
195+
assertThatThrownBy(() -> SchemaParser.parse(null))
196+
.isInstanceOf(IllegalArgumentException.class)
197+
.hasMessageContaining("schema must be a boolean or an object");
198+
}
184199
}

0 commit comments

Comments
 (0)