From 10e089dae76fdea11cc4676105bb95af3955ea1a Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Tue, 12 May 2026 10:57:40 +0200 Subject: [PATCH 1/2] fix: Add missing extensions arg to constructors in new coercion tests Tests added in the JSON Schema type-coercion fix used the pre-extensions record signatures and stopped compiling after the OpenAPI extensions feature merged into master. --- .../RequestPreparationFilterTest.java | 22 ++++++++++++++----- .../validate/StringIntegerNumberTest.java | 5 +++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index 2760dbf..429f98c 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -164,7 +164,8 @@ void invalidQueryParamThrowsValidation() { @Test void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception { - var intSchema = new IntegerSchema(Set.of(TypeName.INTEGER), 1L, 100L, null, null, null, null); + var intSchema = + new IntegerSchema(Set.of(TypeName.INTEGER), 1L, 100L, null, null, null, null, Map.of()); var op = new Operation( "a", @@ -172,6 +173,7 @@ void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("n", Parameter.Location.QUERY, true, intSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); @@ -184,7 +186,8 @@ void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception { @Test void integerQueryParamRejectsNonNumericString() { - var intSchema = new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null); + var intSchema = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null, Map.of()); var op = new Operation( "a", @@ -192,6 +195,7 @@ void integerQueryParamRejectsNonNumericString() { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("n", Parameter.Location.QUERY, true, intSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); @@ -205,7 +209,8 @@ void integerQueryParamRejectsNonNumericString() { @Test void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception { - var numSchema = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + var numSchema = + new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null, Map.of()); var op = new Operation( "a", @@ -213,6 +218,7 @@ void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("n", Parameter.Location.QUERY, true, numSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); @@ -225,7 +231,8 @@ void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception { @Test void numberQueryParamRejectsNonNumericString() { - var numSchema = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + var numSchema = + new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null, Map.of()); var op = new Operation( "a", @@ -233,6 +240,7 @@ void numberQueryParamRejectsNonNumericString() { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("n", Parameter.Location.QUERY, true, numSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); @@ -246,7 +254,7 @@ void numberQueryParamRejectsNonNumericString() { @Test void booleanQueryParamCoercesTrueAndFalse() throws Exception { - var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN)); + var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN), Map.of()); var op = new Operation( "a", @@ -254,6 +262,7 @@ void booleanQueryParamCoercesTrueAndFalse() throws Exception { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("b", Parameter.Location.QUERY, true, boolSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); @@ -270,7 +279,7 @@ void booleanQueryParamCoercesTrueAndFalse() throws Exception { @Test void booleanQueryParamRejectsNonBooleanString() { - var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN)); + var boolSchema = new BooleanSchema(Set.of(TypeName.BOOLEAN), Map.of()); var op = new Operation( "a", @@ -278,6 +287,7 @@ void booleanQueryParamRejectsNonBooleanString() { PathTemplate.compile("/x"), Optional.empty(), List.of(new Parameter("b", Parameter.Location.QUERY, true, boolSchema)), + Map.of(), Map.of()); Spec spec = specWith(op); Filter f = newFilter(spec); diff --git a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java index a87eddf..fc5d5b1 100644 --- a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java +++ b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java @@ -349,7 +349,7 @@ void numberFormatUnknownIsIgnored() { @Test void integerRejectsNumericLookingString() { IntegerSchema s = - new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null); + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null, Map.of()); assertThatThrownBy(() -> v.validate("42", s, "/v")) .extracting(t -> ((ValidationException) t).error().keyword()) .isEqualTo("type"); @@ -357,7 +357,8 @@ void integerRejectsNumericLookingString() { @Test void numberRejectsNumericLookingString() { - NumberSchema s = new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null); + NumberSchema s = + new NumberSchema(Set.of(TypeName.NUMBER), null, null, null, null, null, null, Map.of()); assertThatThrownBy(() -> v.validate("1234567890", s, "/v")) .extracting(t -> ((ValidationException) t).error().keyword()) .isEqualTo("type"); From f547ee679fd22dab8e8745daa350da694215b99a Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Tue, 12 May 2026 11:36:40 +0200 Subject: [PATCH 2/2] perf: Replace exception-driven control flow in DefaultValidator with Optional returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JFR profiling under load showed ~267 ValidationException throws/sec on the happy path — oneOf/anyOf branch selection was catching exceptions to determine which branch matched, paying allocation + stack-walk cost for every non-matching branch even when the request ultimately succeeded. Internal validation now returns Optional and branch selection inspects the result. ValidationException is constructed at most once per request, only at the public validate() boundary when real validation has failed. A static counter on ValidationException makes the contract observable and testable. --- .../retailsvc/http/ValidationException.java | 9 + .../http/validate/DefaultValidator.java | 289 ++++++++++-------- .../ValidatorNoThrowOnHappyPathTest.java | 101 ++++++ 3 files changed, 267 insertions(+), 132 deletions(-) create mode 100644 src/test/java/com/retailsvc/http/validate/ValidatorNoThrowOnHappyPathTest.java diff --git a/src/main/java/com/retailsvc/http/ValidationException.java b/src/main/java/com/retailsvc/http/ValidationException.java index a8b6c2b..6bbd505 100644 --- a/src/main/java/com/retailsvc/http/ValidationException.java +++ b/src/main/java/com/retailsvc/http/ValidationException.java @@ -1,13 +1,22 @@ package com.retailsvc.http; import com.retailsvc.http.validate.ValidationError; +import java.util.concurrent.atomic.AtomicLong; public final class ValidationException extends RuntimeException { + /** + * Counts every {@code ValidationException} ever constructed. Used to assert that the validator + * does not use exceptions as control flow on the happy path: a successful validation of a body + * containing {@code oneOf}/{@code anyOf} branches should leave this counter unchanged. + */ + public static final AtomicLong CONSTRUCTIONS = new AtomicLong(); + private final transient ValidationError error; public ValidationException(ValidationError error) { super(error.pointer() + " [" + error.keyword() + "] " + error.message()); this.error = error; + CONSTRUCTIONS.incrementAndGet(); } public ValidationError error() { diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index 53061e0..308ef73 100644 --- a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java +++ b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -46,6 +47,7 @@ public final class DefaultValidator implements Validator { private static final String FORMAT_KEYWORD = "format"; + private static final Optional OK = Optional.empty(); private record FormatCheck(Predicate isValid, String message) {} @@ -107,94 +109,111 @@ public DefaultValidator(Function refResolver) { @Override public void validate(Object value, Schema schema, String pointer) { + Optional result = check(value, schema, pointer); + if (result.isPresent()) { + throw new ValidationException(result.get()); + } + } + + /** + * Internal validation entry point. Returns the first {@link ValidationError} encountered, or + * {@link Optional#empty()} on success. This is what {@code anyOf} / {@code oneOf} / {@code not} + * branch-select against, so it must never throw {@link ValidationException} on a failed branch — + * exceptions would re-introduce the hot-path control-flow cost this refactor exists to remove. + */ + Optional check(Object value, Schema schema, String pointer) { if (value == null && schema.types().contains(TypeName.NULL)) { - return; - } - - switch (schema) { - case RefSchema(String ref, var _) -> validate(value, refResolver.apply(ref), pointer); - case BooleanSchema _ -> validateBoolean(value, pointer); - case NullSchema _ -> require(value == null, pointer, "type", "expected null"); - case StringSchema s -> validateString(value, s, pointer); - case IntegerSchema i -> validateInteger(value, i, pointer); - case NumberSchema n -> validateNumber(value, n, pointer); - case ObjectSchema o -> validateObject(value, o, pointer); - case ArraySchema a -> validateArray(value, a, pointer); + return OK; + } + + return switch (schema) { + case RefSchema(String ref, var _) -> check(value, refResolver.apply(ref), pointer); + case BooleanSchema _ -> checkBoolean(value, pointer); + case NullSchema _ -> value == null ? OK : err(pointer, "type", "expected null", value); + case StringSchema s -> checkString(value, s, pointer); + case IntegerSchema i -> checkInteger(value, i, pointer); + case NumberSchema n -> checkNumber(value, n, pointer); + case ObjectSchema o -> checkObject(value, o, pointer); + case ArraySchema a -> checkArray(value, a, pointer); case EnumSchema(List values, var _) -> - require(values.contains(value), pointer, "enum", "value not in enum"); + values.contains(value) ? OK : err(pointer, "enum", "value not in enum", value); case ConstSchema(Object expected, var _) -> - require(Objects.equals(expected, value), pointer, "const", "value does not equal const"); - case AllOfSchema(List parts, var _) -> { - for (Schema p : parts) { - validate(value, p, pointer); - } - } - case AnyOfSchema(List options, var _) -> validateAnyOf(value, options, pointer); - case OneOfSchema(List options, var _) -> validateOneOf(value, options, pointer); - case NotSchema(Schema inner, var _) -> validateNot(value, inner, pointer); - case AlwaysSchema _ -> { - /* accepts any value, including null */ - } - case NeverSchema _ -> fail(pointer, "false", "schema rejects all values", value); - } + Objects.equals(expected, value) + ? OK + : err(pointer, "const", "value does not equal const", value); + case AllOfSchema(List parts, var _) -> checkAllOf(value, parts, pointer); + case AnyOfSchema(List options, var _) -> checkAnyOf(value, options, pointer); + case OneOfSchema(List options, var _) -> checkOneOf(value, options, pointer); + case NotSchema(Schema inner, var _) -> checkNot(value, inner, pointer); + case AlwaysSchema _ -> OK; + case NeverSchema _ -> err(pointer, "false", "schema rejects all values", value); + }; + } + + private static Optional err( + String pointer, String keyword, String message, Object rejectedValue) { + return Optional.of(new ValidationError(pointer, keyword, message, rejectedValue)); } - private void validateBoolean(Object value, String pointer) { - require(value instanceof Boolean, pointer, "type", "expected boolean"); + private static Optional err(String pointer, String keyword, String message) { + return Optional.of(new ValidationError(pointer, keyword, message, null)); } - private void validateString(Object value, StringSchema s, String pointer) { - require(value instanceof String, pointer, "type", "expected string"); - String str = (String) value; + private static Optional checkBoolean(Object value, String pointer) { + return value instanceof Boolean ? OK : err(pointer, "type", "expected boolean", value); + } + + private Optional checkString(Object value, StringSchema s, String pointer) { + if (!(value instanceof String str)) { + return err(pointer, "type", "expected string", value); + } if (s.minLength() != null && str.length() < s.minLength()) { - fail(pointer, "minLength", "string shorter than " + s.minLength(), str); + return err(pointer, "minLength", "string shorter than " + s.minLength(), str); } if (s.maxLength() != null && str.length() > s.maxLength()) { - fail(pointer, "maxLength", "string longer than " + s.maxLength(), str); + return err(pointer, "maxLength", "string longer than " + s.maxLength(), str); } if (s.pattern() != null && !compiledPatterns .computeIfAbsent(s.pattern(), Pattern::compile) .matcher(str) .matches()) { - fail(pointer, "pattern", "does not match pattern " + s.pattern(), str); + return err(pointer, "pattern", "does not match pattern " + s.pattern(), str); } if (s.enumValues() != null && !s.enumValues().contains(str)) { - fail(pointer, "enum", "value not in enum", str); + return err(pointer, "enum", "value not in enum", str); } if (s.format() != null) { - validateStringFormat(str, s.format(), pointer); + return checkStringFormat(str, s.format(), pointer); } + return OK; } - private void validateStringFormat(String str, String format, String pointer) { + private static Optional checkStringFormat( + String str, String format, String pointer) { FormatCheck check = FORMAT_CHECKS.get(format); if (check == null) { - return; - } - if (!check.isValid().test(str)) { - fail(pointer, FORMAT_KEYWORD, check.message(), str); + return OK; } + return check.isValid().test(str) ? OK : err(pointer, FORMAT_KEYWORD, check.message(), str); } - private void validateIntegerFormat(long n, String format, String pointer) { + private static Optional checkIntegerFormat( + long n, String format, String pointer) { IntegerFormatCheck check = INTEGER_FORMAT_CHECKS.get(format); if (check == null) { - return; - } - if (!check.isValid().test(n)) { - fail(pointer, FORMAT_KEYWORD, check.message(), n); + return OK; } + return check.isValid().test(n) ? OK : err(pointer, FORMAT_KEYWORD, check.message(), n); } - private void validateNumberFormat(double n, String format, String pointer) { + private static Optional checkNumberFormat( + double n, String format, String pointer) { NumberFormatCheck check = NUMBER_FORMAT_CHECKS.get(format); if (check == null) { - return; - } - if (!check.isValid().test(n)) { - fail(pointer, FORMAT_KEYWORD, check.message(), n); + return OK; } + return check.isValid().test(n) ? OK : err(pointer, FORMAT_KEYWORD, check.message(), n); } private static boolean isUuid(String s) { @@ -346,58 +365,61 @@ private static boolean isHextet(String hextet) { return true; } - private void validateInteger(Object value, IntegerSchema s, String pointer) { + private static Optional checkInteger( + Object value, IntegerSchema s, String pointer) { if (!(value instanceof Number num)) { - fail(pointer, "type", "expected integer", value); - return; + return err(pointer, "type", "expected integer", value); } long n = num.longValue(); if (s.minimum() != null && n < s.minimum()) { - fail(pointer, "minimum", "integer below minimum " + s.minimum(), n); + return err(pointer, "minimum", "integer below minimum " + s.minimum(), n); } if (s.maximum() != null && n > s.maximum()) { - fail(pointer, "maximum", "integer above maximum " + s.maximum(), n); + return err(pointer, "maximum", "integer above maximum " + s.maximum(), n); } if (s.exclusiveMinimum() != null && n <= s.exclusiveMinimum()) { - fail(pointer, "exclusiveMinimum", "integer not greater than " + s.exclusiveMinimum(), n); + return err( + pointer, "exclusiveMinimum", "integer not greater than " + s.exclusiveMinimum(), n); } if (s.exclusiveMaximum() != null && n >= s.exclusiveMaximum()) { - fail(pointer, "exclusiveMaximum", "integer not less than " + s.exclusiveMaximum(), n); + return err(pointer, "exclusiveMaximum", "integer not less than " + s.exclusiveMaximum(), n); } if (s.multipleOf() != null && n % s.multipleOf() != 0) { - fail(pointer, "multipleOf", "not a multiple of " + s.multipleOf(), n); + return err(pointer, "multipleOf", "not a multiple of " + s.multipleOf(), n); } if (s.format() != null) { - validateIntegerFormat(n, s.format(), pointer); + return checkIntegerFormat(n, s.format(), pointer); } + return OK; } - private void validateNumber(Object value, NumberSchema s, String pointer) { + private static Optional checkNumber( + Object value, NumberSchema s, String pointer) { if (!(value instanceof Number num)) { - fail(pointer, "type", "expected number", value); - return; + return err(pointer, "type", "expected number", value); } double n = num.doubleValue(); if (s.minimum() != null && n < s.minimum().doubleValue()) { - fail(pointer, "minimum", "number below minimum " + s.minimum(), n); + return err(pointer, "minimum", "number below minimum " + s.minimum(), n); } if (s.maximum() != null && n > s.maximum().doubleValue()) { - fail(pointer, "maximum", "number above maximum " + s.maximum(), n); + return err(pointer, "maximum", "number above maximum " + s.maximum(), n); } if (s.exclusiveMinimum() != null && n <= s.exclusiveMinimum().doubleValue()) { - fail(pointer, "exclusiveMinimum", "number not greater than " + s.exclusiveMinimum(), n); + return err(pointer, "exclusiveMinimum", "number not greater than " + s.exclusiveMinimum(), n); } if (s.exclusiveMaximum() != null && n >= s.exclusiveMaximum().doubleValue()) { - fail(pointer, "exclusiveMaximum", "number not less than " + s.exclusiveMaximum(), n); + return err(pointer, "exclusiveMaximum", "number not less than " + s.exclusiveMaximum(), n); } if (s.multipleOf() != null && !isMultipleOf(n, s.multipleOf().doubleValue())) { - fail(pointer, "multipleOf", "not a multiple of " + s.multipleOf(), n); + return err(pointer, "multipleOf", "not a multiple of " + s.multipleOf(), n); } if (s.format() != null) { - validateNumberFormat(n, s.format(), pointer); + return checkNumberFormat(n, s.format(), pointer); } + return OK; } /** @@ -412,124 +434,127 @@ private static boolean isMultipleOf(double value, double divisor) { } @SuppressWarnings("unchecked") - private void validateObject(Object value, ObjectSchema s, String pointer) { - require(value instanceof Map, pointer, "type", "expected object"); + private Optional checkObject(Object value, ObjectSchema s, String pointer) { + if (!(value instanceof Map)) { + return err(pointer, "type", "expected object", value); + } Map map = (Map) value; for (String required : s.required()) { - require( - map.containsKey(required), - pointer + "/" + required, - "required", - "required property missing"); + if (!map.containsKey(required)) { + return err(pointer + "/" + required, "required", "required property missing"); + } } if (s.minProperties() != null && map.size() < s.minProperties()) { - fail(pointer, "minProperties", "fewer than " + s.minProperties() + " properties", map.size()); + return err( + pointer, "minProperties", "fewer than " + s.minProperties() + " properties", map.size()); } if (s.maxProperties() != null && map.size() > s.maxProperties()) { - fail(pointer, "maxProperties", "more than " + s.maxProperties() + " properties", map.size()); + return err( + pointer, "maxProperties", "more than " + s.maxProperties() + " properties", map.size()); } for (var entry : map.entrySet()) { String childPointer = pointer + "/" + entry.getKey(); Schema propSchema = s.properties().get(entry.getKey()); + Optional result; if (propSchema != null) { - validate(entry.getValue(), propSchema, childPointer); + result = check(entry.getValue(), propSchema, childPointer); } else { - switch (s.additionalProperties()) { - case AdditionalProperties.Allowed _ -> { - /* no-op: additional properties are permitted by default */ - } - case AdditionalProperties.Forbidden _ -> - fail( - childPointer, - "additionalProperties", - "additional property not allowed", - entry.getKey()); - case AdditionalProperties.SchemaConstraint(Schema constraint) -> - validate(entry.getValue(), constraint, childPointer); - } + result = + switch (s.additionalProperties()) { + case AdditionalProperties.Allowed _ -> OK; + case AdditionalProperties.Forbidden _ -> + err( + childPointer, + "additionalProperties", + "additional property not allowed", + entry.getKey()); + case AdditionalProperties.SchemaConstraint(Schema constraint) -> + check(entry.getValue(), constraint, childPointer); + }; + } + if (result.isPresent()) { + return result; } } + return OK; } - private void validateArray(Object value, ArraySchema s, String pointer) { - require(value instanceof Iterable, pointer, "type", "expected array"); - Iterable it = (Iterable) value; + private Optional checkArray(Object value, ArraySchema s, String pointer) { + if (!(value instanceof Iterable it)) { + return err(pointer, "type", "expected array", value); + } List elements = new ArrayList<>(); for (Object o : it) { elements.add(o); } if (s.minItems() != null && elements.size() < s.minItems()) { - fail(pointer, "minItems", "fewer than " + s.minItems() + " items", elements.size()); + return err(pointer, "minItems", "fewer than " + s.minItems() + " items", elements.size()); } if (s.maxItems() != null && elements.size() > s.maxItems()) { - fail(pointer, "maxItems", "more than " + s.maxItems() + " items", elements.size()); + return err(pointer, "maxItems", "more than " + s.maxItems() + " items", elements.size()); } if (s.uniqueItems()) { Set seen = new HashSet<>(); for (Object e : elements) { if (!seen.add(e)) { - fail(pointer, "uniqueItems", "duplicate item", e); + return err(pointer, "uniqueItems", "duplicate item", e); } } } for (int i = 0; i < elements.size(); i++) { - validate(elements.get(i), s.items(), pointer + "/" + i); + Optional result = check(elements.get(i), s.items(), pointer + "/" + i); + if (result.isPresent()) { + return result; + } } + return OK; } - private static void fail(String pointer, String keyword, String message, Object rejectedValue) { - throw new ValidationException(new ValidationError(pointer, keyword, message, rejectedValue)); - } - - static void require(boolean condition, String pointer, String keyword, String message) { - if (!condition) { - throw new ValidationException(new ValidationError(pointer, keyword, message, null)); + private Optional checkAllOf(Object value, List parts, String pointer) { + for (Schema p : parts) { + Optional result = check(value, p, pointer); + if (result.isPresent()) { + return result; + } } + return OK; } - private void validateAnyOf(Object value, List options, String pointer) { + private Optional checkAnyOf(Object value, List options, String pointer) { for (Schema o : options) { - try { - validate(value, o, pointer); - return; - } catch (ValidationException ignored) { - // try next branch + if (check(value, o, pointer).isEmpty()) { + return OK; } } - fail(pointer, "anyOf", "did not match any anyOf branch", value); + return err(pointer, "anyOf", "did not match any anyOf branch", value); } - private void validateOneOf(Object value, List options, String pointer) { + private Optional checkOneOf(Object value, List options, String pointer) { int matched = 0; for (Schema o : options) { - try { - validate(value, o, pointer); + if (check(value, o, pointer).isEmpty()) { matched++; - } catch (ValidationException ignored) { - // branch did not match; continue } } - if (matched != 1) { - fail( - pointer, - "oneOf", - "matched " + matched + " of " + options.size() + " oneOf branches", - value); + if (matched == 1) { + return OK; } + return err( + pointer, + "oneOf", + "matched " + matched + " of " + options.size() + " oneOf branches", + value); } - private void validateNot(Object value, Schema inner, String pointer) { - try { - validate(value, inner, pointer); - } catch (ValidationException expected) { - return; - } - fail(pointer, "not", "value matched 'not' schema", value); + private Optional checkNot(Object value, Schema inner, String pointer) { + return check(value, inner, pointer).isPresent() + ? OK + : err(pointer, "not", "value matched 'not' schema", value); } } diff --git a/src/test/java/com/retailsvc/http/validate/ValidatorNoThrowOnHappyPathTest.java b/src/test/java/com/retailsvc/http/validate/ValidatorNoThrowOnHappyPathTest.java new file mode 100644 index 0000000..7298009 --- /dev/null +++ b/src/test/java/com/retailsvc/http/validate/ValidatorNoThrowOnHappyPathTest.java @@ -0,0 +1,101 @@ +package com.retailsvc.http.validate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.ValidationException; +import com.retailsvc.http.spec.schema.AdditionalProperties; +import com.retailsvc.http.spec.schema.AnyOfSchema; +import com.retailsvc.http.spec.schema.NumberSchema; +import com.retailsvc.http.spec.schema.ObjectSchema; +import com.retailsvc.http.spec.schema.OneOfSchema; +import com.retailsvc.http.spec.schema.Schema; +import com.retailsvc.http.spec.schema.StringSchema; +import com.retailsvc.http.spec.schema.TypeName; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +/** + * Performance contract: branch-selecting schemas ({@code oneOf} / {@code anyOf}) must select + * matching branches by inspecting a return value, not by catching {@link ValidationException}. + * Constructing a {@link Throwable} on every non-matching branch was a measurable hot-path cost (see + * the issue that prompted this refactor). These tests fail if anyone reintroduces exception-driven + * control flow. + */ +class ValidatorNoThrowOnHappyPathTest { + + private final Validator validator = + new DefaultValidator( + name -> { + throw new AssertionError("no refs"); + }); + + private static Schema stringOrNumber() { + return new OneOfSchema( + List.of( + new StringSchema(Set.of(TypeName.STRING), null, null, null, null, null, Map.of()), + new NumberSchema( + Set.of(TypeName.NUMBER), null, null, null, null, null, null, Map.of())), + Map.of()); + } + + private static Schema attributesMap() { + return new ObjectSchema( + Set.of(TypeName.OBJECT), + Map.of(), + List.of(), + new AdditionalProperties.SchemaConstraint(stringOrNumber()), + null, + null, + Map.of()); + } + + @Test + void successfulOneOfDoesNotConstructValidationException() { + long before = ValidationException.CONSTRUCTIONS.get(); + + validator.validate("hello", stringOrNumber(), "/v"); + validator.validate(42, stringOrNumber(), "/v"); + validator.validate(Map.of("a", "x", "b", 7, "c", "1234567890"), attributesMap(), "/v"); + + assertThat(ValidationException.CONSTRUCTIONS.get() - before) + .as("no ValidationException should be constructed on a valid oneOf body") + .isZero(); + } + + @Test + void successfulAnyOfDoesNotConstructValidationException() { + Schema anyOfStringOrNumber = + new AnyOfSchema( + List.of( + new StringSchema(Set.of(TypeName.STRING), null, null, null, null, null, Map.of()), + new NumberSchema( + Set.of(TypeName.NUMBER), null, null, null, null, null, null, Map.of())), + Map.of()); + + long before = ValidationException.CONSTRUCTIONS.get(); + + validator.validate("hello", anyOfStringOrNumber, "/v"); + validator.validate(42, anyOfStringOrNumber, "/v"); + + assertThat(ValidationException.CONSTRUCTIONS.get() - before) + .as("no ValidationException should be constructed on a valid anyOf body") + .isZero(); + } + + @Test + void failingOneOfConstructsExactlyOneValidationException() { + long before = ValidationException.CONSTRUCTIONS.get(); + + assertThatThrownBy(() -> validator.validate(true, stringOrNumber(), "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("oneOf"); + + assertThat(ValidationException.CONSTRUCTIONS.get() - before) + .as("exactly one ValidationException — only at the boundary, not per branch") + .isOne(); + } +}