diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c0ff07..b31351c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,7 @@ jobs: for k in totals: totals[k]+=int(r.get(k,'0')) except Exception: pass - exp_tests=475 + exp_tests=509 exp_skipped=0 if totals['tests']!=exp_tests or totals['skipped']!=exp_skipped: print(f"Unexpected test totals: {totals} != expected tests={exp_tests}, skipped={exp_skipped}") diff --git a/AGENTS.md b/AGENTS.md index 702a828..97bf630 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,6 +107,36 @@ LOG.fine(() -> "PERFORMANCE WARNING: Validation stack processing " + count + ... ### Additional Guidance - Logging rules apply globally. The helper superclass ensures JUL configuration remains compatible with `$(command -v mvnd || command -v mvn || command -v ./mvnw)`. +### Error Message Standards +When throwing exceptions for invalid schemas, provide descriptive error messages that help users understand: +- What specific constraint was violated +- The actual value or structure that caused the problem +- The expected valid format or values + +**Good Error Messages:** +```java +// Include the actual invalid value +throw new IllegalArgumentException("enum contains duplicate values: " + + values.stream().collect(Collectors.joining(", ", "[", "]"))); + +// Include the problematic schema portion +throw new IllegalArgumentException("Type schema contains unknown key: " + key + + " in schema: " + Json.toDisplayString(obj, 0)); + +// Include both expected and actual values +throw new IllegalArgumentException("unknown type: '" + typeStr + + "', expected one of: boolean, string, timestamp, int8, uint8, int16, uint16, int32, uint32, float32, float64"); +``` + +**Poor Error Messages (Avoid):** +```java +throw new IllegalArgumentException("enum contains duplicate values"); // No context +throw new IllegalArgumentException("invalid schema"); // Too vague +throw new IllegalArgumentException("bad value"); // No specifics +``` + +Use `Json.toDisplayString(value, depth)` to render JSON fragments in error messages, and include relevant context like schema paths, actual vs expected values, and specific constraint violations. + ## JSON Compatibility Suite ```bash # Build and run compatibility report diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 0000000..da3f90e --- /dev/null +++ b/PLAN.md @@ -0,0 +1,60 @@ +# Discriminator Validation Refactor Plan + +## Objectives +- Document the intended single-path validation architecture and discriminator rules before touching implementation. +- Additive-first: introduce tests and supporting guards ahead of removing legacy recursive validation. +- Enforce RFC 8927 discriminator constraints at compilation while preserving stack-based runtime checks with the discriminator tag exemption. +- Retire duplicate validation paths only after documentation and tests clearly describe the target behavior. + +## Sequenced Work Breakdown + +1. **Documentation First** ✅ + - Expand `json-java21-jtd/ARCHITECTURE.md` with explicit guidance on single stack-based validation, discriminator tag exemption, and compile-time schema constraints. + - Audit other affected docs (e.g., module README, AGENTS addenda) to ensure contributors receive clear instructions prior to code edits. + +2. **Reconnaissance & Impact Analysis** ✅ + - Inventory all usages of `JtdSchema.*#validate` and catalogue callers/tests that rely on the recursive path. + - Trace how `pushChildFrames` and `PropertiesSchema` cooperate today to support discriminator handling and identify touchpoints for additive changes. + +3. **Test Design (Additive Stage)** ✅ + - Keep `JtdSpecIT` focused solely on `validation.json` scenarios so we only assert that published docs validate as expected. + - Add `CompilerSpecIT` to execute `invalid_schemas.json`, asserting compilation failures with deterministic messages instead of skipping them. + - Introduce `CompilerTest` for incremental compiler-focused unit cases that exercise new stack artifacts while staying within JUL logging discipline (INFO log at method start via logging helper). + +4. **Compile-Time Guards (Additive)** ✅ + - Introduce validation during schema compilation to reject non-`PropertiesSchema` discriminator mappings, nullable mappings, and mappings that shadow the discriminator key. + - Emit deterministic exception messages with precise schema paths to keep property-based tests stable. + +5. **Stack Engine Enhancements (Additive)** + - Teach the stack-based validator to propagate an "exempt key" for discriminator payload evaluation before deleting any recursive logic. + - Adjust `PropertiesSchema` handling to skip the exempt key when checking required/optional members and additionalProperties while preserving crumb/schemaPath reporting. + +6. **Retire Recursive `validate` Path (Removal Stage)** + - After new tests pass with the enhanced stack engine, remove the per-schema `validate` implementations and their call sites. + - Clean up dead code, imports, and update any remaining references discovered by static analysis. + +7. **Verification & Cleanup** + - Run targeted tests with mandated JUL logging levels (FINE/FINEST for new cases) followed by the full suite at INFO to confirm conformity. + - Revisit documentation to confirm implemented behavior matches the authored guidance and adjust if gaps remain. + +## Risks & Mitigations +- **Hidden Recursive Callers**: Use `rg "\.validate\("` and compiler errors to surface stragglers before deleting the recursive path. +- **Error Message Drift**: Lock expected `schemaPath`/`instancePath` outputs in tests once new guards land to prevent flakiness. +- **Temporal Test Failures**: Stage code changes so new tests are introduced alongside the supporting implementation to avoid committing failing tests. +- **Documentation Drift**: Re-review docs post-implementation to ensure instructions still match code. + +## Out of Scope +- Remote reference compilation or runtime changes (handled by MVF initiative). +- Adjustments to global logging frameworks beyond what is necessary for new tests. +- Broader API redesigns outside discriminator handling and validation-path consolidation. + +## Documentation Targets ✅ +- json-java21-jtd/ARCHITECTURE.md: add single-path validation, discriminator tag exemption, compile-time guard details. +- README.md (module-level if present): summarize discriminator constraints and reference architecture section. +- AGENTS.md references (if needed): reinforce documentation-before-code steps for discriminator work. + +## Test Targets +- New compile-time rejection test suite ensuring discriminator mapping violations throw with deterministic schema paths. +- Runtime tests for RFC 8927 §3.3.8 scenarios (missing tag, non-string tag, unknown mapping, payload mismatch, success). +- Regression coverage ensuring tag exemption prevents additionalProperties violations when payload omits discriminator. +- Removal/update of tests invoking `JtdSchema.validate` only after stack engine passes new scenarios. diff --git a/json-java21-jtd/ARCHITECTURE.md b/json-java21-jtd/ARCHITECTURE.md index 20ef5e1..fca4506 100644 --- a/json-java21-jtd/ARCHITECTURE.md +++ b/json-java21-jtd/ARCHITECTURE.md @@ -24,6 +24,15 @@ JTD defines eight mutually-exclusive schema forms: 7. **values** - Validates objects with homogeneous values (RFC 8927 §2.2.7) 8. **discriminator** - Validates tagged unions (RFC 8927 §2.2.8) +### Discriminator Schema Constraints (RFC 8927 §2.2.8) +Discriminator schemas enforce compile-time constraints to ensure predictable validation: +- **Mapping values must be PropertiesSchema**: Cannot use primitive types, enums, or other forms +- **No nullable mappings**: Mapped schemas cannot have `nullable: true` +- **Discriminator key exemption**: The discriminator field is ignored during payload validation +- **No discriminator redefinition**: Mapped schemas cannot define the discriminator key in properties/optionalProperties + +These constraints are enforced at compile-time, preventing invalid schemas from reaching validation. + ## Architecture Flow ```mermaid @@ -118,25 +127,34 @@ enum PrimitiveType { ## Validation Architecture +The JTD validator uses a single stack-based validation engine that enforces RFC 8927 compliance through immutable schema records. All validation flows through one path to prevent behavioral divergence. + +### Single Path Validation Principle +- **No per-schema validate() methods**: Schema records are immutable data only +- **Stack-based only**: All validation uses `pushChildFrames()` and explicit stack traversal +- **Compile-time enforcement**: Invalid schemas are rejected during compilation, not validation + ```mermaid sequenceDiagram participant User - participant JTDSchema + participant JTD participant ValidationStack participant ErrorCollector - User->>JTDSchema: validate(json) - JTDSchema->>ValidationStack: push(rootSchema, "#") + User->>JTD: validate(schemaJson, instanceJson) + JTD->>JTD: compileSchema(schemaJson) + Note over JTD: Compile-time checks enforce RFC constraints + JTD->>ValidationStack: push(rootSchema, "#") loop While stack not empty - ValidationStack->>JTDSchema: pop() - JTDSchema->>JTDSchema: validateCurrent() + ValidationStack->>JTD: pop() + JTD->>JTD: validateCurrent() alt Validation fails - JTDSchema->>ErrorCollector: addError(path, message) + JTD->>ErrorCollector: addError(path, message) else Has children - JTDSchema->>ValidationStack: push(children) + JTD->>ValidationStack: push(children) end end - JTDSchema->>User: ValidationResult + JTD->>User: ValidationResult ``` ## Error Reporting (RFC 8927 Section 3.2) @@ -275,6 +293,18 @@ Run the official JTD Test Suite: $(command -v mvnd || command -v mvn || command -v ./mvnw) test -pl json-java21-jtd -Dtest=JtdSpecIT ``` +`JtdSpecIT` exercises only the published `validation.json` cases so coverage maps exactly to behaviour that downstream users rely on. Compilation enforcement is handled through dedicated suites: + +- `CompilerSpecIT` replays `invalid_schemas.json` and asserts that compilation fails with deterministic exception messages for every illegal schema. +- `CompilerTest` holds incremental unit tests for compiler internals (for example, discriminator guard scenarios) while still extending the JUL logging helper to emit INFO banners per method. + +Run the compiler-focused suites when evolving compile-time logic: + +```bash +$(command -v mvnd || command -v mvn || command -v ./mvnw) test -pl json-java21-jtd -Dtest=CompilerSpecIT +$(command -v mvnd || command -v mvn || command -v ./mvnw) test -pl json-java21-jtd -Dtest=CompilerTest +``` + ## Performance Considerations 1. **Immutable Records**: Zero mutation during validation diff --git a/json-java21-jtd/README.md b/json-java21-jtd/README.md index 78b231f..67b357f 100644 --- a/json-java21-jtd/README.md +++ b/json-java21-jtd/README.md @@ -121,6 +121,14 @@ Validates tagged unions: } ``` +**Discriminator Constraints (RFC 8927 §2.2.8):** +- Mapping values must be `properties` schemas (not primitive types) +- Mapped schemas cannot have `nullable: true` +- Mapped schemas cannot define the discriminator key in properties/optionalProperties +- The discriminator field is exempt from additionalProperties validation + +These constraints are enforced at compile-time for predictable validation behavior. + ## Nullable Schemas Any schema can be made nullable by adding `"nullable": true`: diff --git a/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java b/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java index 3286f7e..9470efd 100644 --- a/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java +++ b/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java @@ -5,8 +5,10 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.logging.Logger; /// JTD Validator - validates JSON instances against JTD schemas (RFC 8927) @@ -15,6 +17,13 @@ public class Jtd { private static final Logger LOG = Logger.getLogger(Jtd.class.getName()); + /// RFC 8927 §2.2.3: Valid primitive types for type schema validation + private static final Set VALID_TYPES = Set.of( + "boolean", "string", "timestamp", + "int8", "uint8", "int16", "uint16", "int32", "uint32", + "float32", "float64" + ); + /// Top-level definitions map for ref resolution private final Map definitions = new java.util.HashMap<>(); @@ -39,6 +48,15 @@ static String enrichedError(String baseMessage, Frame frame, JsonValue contextVa return "[off=" + off + " ptr=" + ptr + " via=" + via + "] " + baseMessage; } + /// Compiles a JTD schema and throws exceptions for invalid schemas + /// @param schema The JTD schema as a JsonValue + /// @return Compiled JtdSchema + /// @throws IllegalArgumentException if the schema is invalid + public JtdSchema compile(JsonValue schema) { + definitions.clear(); + return compileSchema(schema); + } + /// Validates a JSON instance against a JTD schema /// @param schema The JTD schema as a JsonValue /// @param instance The JSON instance to validate @@ -261,36 +279,64 @@ void pushChildFrames(Frame frame, java.util.Deque stack) { /// Compiles a JsonValue into a JtdSchema based on RFC 8927 rules JtdSchema compileSchema(JsonValue schema) { + return compileSchema(schema, true); // Root schema by default + } + + /// Compiles a JsonValue into a JtdSchema based on RFC 8927 rules + /// @param schema The JSON schema to compile + /// @param isRoot Whether this is a root-level schema (can contain definitions) + /// @return Compiled JtdSchema + JtdSchema compileSchema(JsonValue schema, boolean isRoot) { if (!(schema instanceof JsonObject obj)) { throw new IllegalArgumentException("Schema must be an object"); } - // First pass: register definition keys as placeholders - if (obj.members().containsKey("definitions")) { - JsonObject defsObj = (JsonObject) obj.members().get("definitions"); + // RFC 8927: Only root schemas can contain definitions + if (!isRoot && obj.members().containsKey("definitions")) { + throw new IllegalArgumentException("Nested schemas cannot contain definitions, found: " + + Json.toDisplayString(obj, 0)); + } + + // First pass: register definition keys as placeholders (only for root schemas) + if (isRoot && obj.members().containsKey("definitions")) { + JsonValue definitionsValue = obj.members().get("definitions"); + if (!(definitionsValue instanceof JsonObject defsObj)) { + throw new IllegalArgumentException("definitions must be an object"); + } for (String key : defsObj.members().keySet()) { definitions.putIfAbsent(key, null); } } - // Second pass: compile each definition if not already compiled - if (obj.members().containsKey("definitions")) { - JsonObject defsObj = (JsonObject) obj.members().get("definitions"); + // Second pass: compile each definition if not already compiled (only for root schemas) + if (isRoot && obj.members().containsKey("definitions")) { + JsonValue definitionsValue = obj.members().get("definitions"); + if (!(definitionsValue instanceof JsonObject defsObj)) { + throw new IllegalArgumentException("definitions must be an object"); + } for (String key : defsObj.members().keySet()) { if (definitions.get(key) == null) { JsonValue rawDef = defsObj.members().get(key); // Compile definitions normally (RFC 8927 strict) - JtdSchema compiled = compileSchema(rawDef); + JtdSchema compiled = compileSchema(rawDef, false); // Definitions are not root schemas definitions.put(key, compiled); } } } - return compileObjectSchema(obj); + return compileObjectSchema(obj, isRoot); } /// Compiles an object schema according to RFC 8927 with strict semantics JtdSchema compileObjectSchema(JsonObject obj) { + return compileObjectSchema(obj, true); // Default to root schema + } + + /// Compiles an object schema according to RFC 8927 with strict semantics + /// @param obj The JSON object to compile + /// @param isRoot Whether this is a root-level schema (can contain definitions) + /// @return Compiled JtdSchema + JtdSchema compileObjectSchema(JsonObject obj, boolean isRoot) { // Check for mutually-exclusive schema forms List forms = new ArrayList<>(); Map members = obj.members(); @@ -309,11 +355,51 @@ JtdSchema compileObjectSchema(JsonObject obj) { forms.add("properties"); // Treat as single form } + // RFC 8927: Check for form-specific properties that shouldn't be mixed + if (forms.size() == 1) { + String form = forms.get(0); + switch (form) { + case "elements", "values", "enum", "ref", "type" -> { + // These forms should not have properties-specific attributes + if (members.containsKey("additionalProperties")) { + throw new IllegalArgumentException(form + " schema cannot contain additionalProperties"); + } + if (members.containsKey("properties")) { + throw new IllegalArgumentException(form + " schema cannot contain properties"); + } + if (members.containsKey("optionalProperties")) { + throw new IllegalArgumentException(form + " schema cannot contain optionalProperties"); + } + } + case "discriminator" -> { + // Discriminator should not have properties-specific attributes (except in mapping values) + if (members.containsKey("additionalProperties")) { + throw new IllegalArgumentException("discriminator schema cannot contain additionalProperties"); + } + if (members.containsKey("properties")) { + throw new IllegalArgumentException("discriminator schema cannot contain properties"); + } + if (members.containsKey("optionalProperties")) { + throw new IllegalArgumentException("discriminator schema cannot contain optionalProperties"); + } + } + } + } + // RFC 8927: schemas must have exactly one of these forms if (forms.size() > 1) { throw new IllegalArgumentException("Schema has multiple forms: " + forms); } + // RFC 8927: discriminator schemas must have both discriminator and mapping + if (members.containsKey("discriminator") && !members.containsKey("mapping")) { + throw new IllegalArgumentException("discriminator schema must also contain mapping"); + } + if (members.containsKey("mapping") && !members.containsKey("discriminator")) { + throw new IllegalArgumentException("mapping can only appear with discriminator in schema: " + + Json.toDisplayString(obj, 0)); + } + // Parse the specific schema form JtdSchema schema; @@ -324,6 +410,19 @@ JtdSchema compileObjectSchema(JsonObject obj) { return new JtdSchema.EmptySchema(); } else if (forms.isEmpty()) { // Check if this is effectively an empty schema (ignoring metadata keys) + // But first validate nullable if present + if (members.containsKey("nullable")) { + JsonValue nullableValue = members.get("nullable"); + if (!(nullableValue instanceof JsonBoolean bool)) { + throw new IllegalArgumentException("nullable must be a boolean, found: " + + nullableValue.getClass().getSimpleName() + " in schema: " + Json.toDisplayString(obj, 0)); + } + // If nullable is valid, this becomes a nullable empty schema + if (bool.value()) { + return new JtdSchema.NullableSchema(new JtdSchema.EmptySchema()); + } + } + boolean hasNonMetadataKeys = members.keySet().stream() .anyMatch(key -> !key.equals("nullable") && !key.equals("metadata") && !key.equals("definitions")); @@ -346,11 +445,11 @@ JtdSchema compileObjectSchema(JsonObject obj) { case "ref" -> compileRefSchema(obj); case "type" -> compileTypeSchema(obj); case "enum" -> compileEnumSchema(obj); - case "elements" -> compileElementsSchema(obj); - case "properties" -> compilePropertiesSchema(obj); - case "optionalProperties" -> compilePropertiesSchema(obj); // handled together - case "values" -> compileValuesSchema(obj); - case "discriminator" -> compileDiscriminatorSchema(obj); + case "elements" -> compileElementsSchema(obj, isRoot); + case "properties" -> compilePropertiesSchema(obj, isRoot); + case "optionalProperties" -> compilePropertiesSchema(obj, isRoot); // handled together + case "values" -> compileValuesSchema(obj, isRoot); + case "discriminator" -> compileDiscriminatorSchema(obj, isRoot); default -> throw new IllegalArgumentException("Unknown schema form: " + form); }; } @@ -359,7 +458,8 @@ JtdSchema compileObjectSchema(JsonObject obj) { if (members.containsKey("nullable")) { JsonValue nullableValue = members.get("nullable"); if (!(nullableValue instanceof JsonBoolean bool)) { - throw new IllegalArgumentException("nullable must be a boolean"); + throw new IllegalArgumentException("nullable must be a boolean, found: " + + nullableValue.getClass().getSimpleName() + " in schema: " + Json.toDisplayString(obj, 0)); } if (bool.value()) { return new JtdSchema.NullableSchema(schema); @@ -374,16 +474,42 @@ JtdSchema compileRefSchema(JsonObject obj) { if (!(refValue instanceof JsonString str)) { throw new IllegalArgumentException("ref must be a string"); } - return new JtdSchema.RefSchema(str.value(), definitions); + String ref = str.value(); + + // RFC 8927: Validate that ref points to an existing definition at compile time + if (!definitions.containsKey(ref)) { + throw new IllegalArgumentException("ref '" + ref + "' points to non-existent definition in schema: " + + Json.toDisplayString(obj, 0)); + } + + return new JtdSchema.RefSchema(ref, definitions); } JtdSchema compileTypeSchema(JsonObject obj) { Map members = obj.members(); + + // Validate that only expected keys are present + for (String key : members.keySet()) { + if (!key.equals("type") && !key.equals("nullable") && !key.equals("metadata") && !key.equals("definitions")) { + throw new IllegalArgumentException("Type schema contains unknown key: '" + key + + "' in schema: " + Json.toDisplayString(obj, 0)); + } + } + JsonValue typeValue = members.get("type"); if (!(typeValue instanceof JsonString str)) { throw new IllegalArgumentException("type must be a string"); } - return new JtdSchema.TypeSchema(str.value()); + + String typeStr = str.value(); + + // RFC 8927 §2.2.3: Validate that type is one of the supported primitive types + if (!VALID_TYPES.contains(typeStr)) { + throw new IllegalArgumentException("unknown type: '" + typeStr + + "', expected one of: boolean, string, timestamp, int8, uint8, int16, uint16, int32, uint32, float32, float64"); + } + + return new JtdSchema.TypeSchema(typeStr); } JtdSchema compileEnumSchema(JsonObject obj) { @@ -405,17 +531,32 @@ JtdSchema compileEnumSchema(JsonObject obj) { throw new IllegalArgumentException("enum cannot be empty"); } + // RFC 8927: Check for duplicates + Set uniqueValues = new java.util.HashSet<>(values); + if (uniqueValues.size() != values.size()) { + throw new IllegalArgumentException("enum contains duplicate values: " + + values.stream().collect(java.util.stream.Collectors.joining(", ", "[", "]"))); + } + return new JtdSchema.EnumSchema(values); } JtdSchema compileElementsSchema(JsonObject obj) { + return compileElementsSchema(obj, true); // Default to root + } + + JtdSchema compileElementsSchema(JsonObject obj, boolean isRoot) { Map members = obj.members(); JsonValue elementsValue = members.get("elements"); - JtdSchema elementsSchema = compileSchema(elementsValue); + JtdSchema elementsSchema = compileSchema(elementsValue, false); // Elements are nested schemas return new JtdSchema.ElementsSchema(elementsSchema); } JtdSchema compilePropertiesSchema(JsonObject obj) { + return compilePropertiesSchema(obj, true); // Default to root + } + + JtdSchema compilePropertiesSchema(JsonObject obj, boolean isRoot) { Map properties = Map.of(); Map optionalProperties = Map.of(); @@ -427,7 +568,7 @@ JtdSchema compilePropertiesSchema(JsonObject obj) { if (!(propsValue instanceof JsonObject propsObj)) { throw new IllegalArgumentException("properties must be an object"); } - properties = parsePropertySchemas(propsObj); + properties = parsePropertySchemas(propsObj, false); // Property schemas are nested } // Parse optional properties @@ -436,7 +577,16 @@ JtdSchema compilePropertiesSchema(JsonObject obj) { if (!(optPropsValue instanceof JsonObject optPropsObj)) { throw new IllegalArgumentException("optionalProperties must be an object"); } - optionalProperties = parsePropertySchemas(optPropsObj); + optionalProperties = parsePropertySchemas(optPropsObj, false); // Property schemas are nested + } + + // RFC 8927: Check for key overlap between properties and optionalProperties + for (String key : properties.keySet()) { + if (optionalProperties.containsKey(key)) { + throw new IllegalArgumentException("Key '" + key + + "' cannot be defined in both properties and optionalProperties in schema: " + + Json.toDisplayString(obj, 0)); + } } // RFC 8927: additionalProperties defaults to false when properties or optionalProperties are defined @@ -453,18 +603,27 @@ JtdSchema compilePropertiesSchema(JsonObject obj) { } JtdSchema compileValuesSchema(JsonObject obj) { + return compileValuesSchema(obj, true); // Default to root + } + + JtdSchema compileValuesSchema(JsonObject obj, boolean isRoot) { Map members = obj.members(); JsonValue valuesValue = members.get("values"); - JtdSchema valuesSchema = compileSchema(valuesValue); + JtdSchema valuesSchema = compileSchema(valuesValue, false); // Values are nested schemas return new JtdSchema.ValuesSchema(valuesSchema); } JtdSchema compileDiscriminatorSchema(JsonObject obj) { + return compileDiscriminatorSchema(obj, true); // Default to root + } + + JtdSchema compileDiscriminatorSchema(JsonObject obj, boolean isRoot) { Map members = obj.members(); JsonValue discriminatorValue = members.get("discriminator"); if (!(discriminatorValue instanceof JsonString discStr)) { throw new IllegalArgumentException("discriminator must be a string"); } + String discriminatorKey = discStr.value(); JsonValue mappingValue = members.get("mapping"); if (!(mappingValue instanceof JsonObject mappingObj)) { @@ -474,21 +633,86 @@ JtdSchema compileDiscriminatorSchema(JsonObject obj) { Map mapping = new java.util.HashMap<>(); for (String key : mappingObj.members().keySet()) { JsonValue variantValue = mappingObj.members().get(key); - JtdSchema variantSchema = compileSchema(variantValue); + + // Early validation: mapping values must be objects (for PropertiesSchema) + if (!(variantValue instanceof JsonObject)) { + throw new IllegalArgumentException("Discriminator mapping '" + key + "' must be an object (properties schema)"); + } + + JsonObject variantObj = (JsonObject) variantValue; + + // Check for nullable flag before compiling + if (variantObj.members().containsKey("nullable") && + variantObj.members().get("nullable") instanceof JsonBoolean bool && + bool.value()) { + throw new IllegalArgumentException("Discriminator mapping '" + key + "' cannot be nullable"); + } + + JtdSchema variantSchema = compileSchema(variantValue, false); // Mapping values are nested schemas + + // RFC 8927 §2.2.8: Validate discriminator mapping constraints + validateDiscriminatorMapping(key, variantSchema, discriminatorKey); + mapping.put(key, variantSchema); } - return new JtdSchema.DiscriminatorSchema(discStr.value(), mapping); + return new JtdSchema.DiscriminatorSchema(discriminatorKey, mapping); + } + + /// Validates discriminator mapping constraints per RFC 8927 §2.2.8 + void validateDiscriminatorMapping(String mappingKey, JtdSchema variantSchema, String discriminatorKey) { + // Check if this is a nullable schema and unwrap it + JtdSchema unwrappedSchema = variantSchema; + boolean wasNullableWrapper = false; + + if (variantSchema instanceof JtdSchema.NullableSchema nullableSchema) { + wasNullableWrapper = true; + unwrappedSchema = nullableSchema.wrapped(); + } + + // RFC 8927 §2.2.8: Mapping values must be PropertiesSchema + if (!(unwrappedSchema instanceof JtdSchema.PropertiesSchema)) { + String schemaType = wasNullableWrapper ? "nullable " + unwrappedSchema.getClass().getSimpleName() : + unwrappedSchema.getClass().getSimpleName(); + throw new IllegalArgumentException( + "Discriminator mapping '" + mappingKey + "' must be a properties schema, found: " + schemaType); + } + + JtdSchema.PropertiesSchema propsSchema = (JtdSchema.PropertiesSchema) unwrappedSchema; + + // RFC 8927 §2.2.8: Mapped schemas cannot have nullable: true + if (wasNullableWrapper) { + throw new IllegalArgumentException( + "Discriminator mapping '" + mappingKey + "' cannot be nullable"); + } + + // RFC 8927 §2.2.8: Mapped schemas cannot define the discriminator key in properties or optionalProperties + if (propsSchema.properties().containsKey(discriminatorKey)) { + throw new IllegalArgumentException( + "Discriminator mapping '" + mappingKey + "' cannot define discriminator key '" + discriminatorKey + + "' in properties"); + } + + if (propsSchema.optionalProperties().containsKey(discriminatorKey)) { + throw new IllegalArgumentException( + "Discriminator mapping '" + mappingKey + "' cannot define discriminator key '" + discriminatorKey + + "' in optionalProperties"); + } } // Removed: RFC 8927 strict mode - no context-aware ref resolution needed /// Extracts and stores top-level definitions for ref resolution private Map parsePropertySchemas(JsonObject propsObj) { + return parsePropertySchemas(propsObj, true); // Default to root + } + + /// Extracts and stores top-level definitions for ref resolution + private Map parsePropertySchemas(JsonObject propsObj, boolean isRoot) { Map schemas = new java.util.HashMap<>(); for (String key : propsObj.members().keySet()) { JsonValue schemaValue = propsObj.members().get(key); - schemas.put(key, compileSchema(schemaValue)); + schemas.put(key, compileSchema(schemaValue, isRoot)); } return schemas; } @@ -520,7 +744,7 @@ public static Result failure(String error) { /// Provides consistent error messages following RFC 8927 specification public enum Error { /// Unknown type specified in schema - UNKNOWN_TYPE("unknown type: %s"), + UNKNOWN_TYPE("unknown type: '%s'"), /// Expected boolean but got different type EXPECTED_BOOLEAN("expected boolean, got %s"), @@ -550,10 +774,10 @@ public enum Error { EXPECTED_STRING_FOR_ENUM("expected string for enum, got %s"), /// Missing required property - MISSING_REQUIRED_PROPERTY("missing required property: %s"), + MISSING_REQUIRED_PROPERTY("missing required property: '%s'"), /// Additional property not allowed - ADDITIONAL_PROPERTY_NOT_ALLOWED("additional property not allowed: %s"), + ADDITIONAL_PROPERTY_NOT_ALLOWED("additional property not allowed: '%s'"), /// Discriminator must be a string DISCRIMINATOR_MUST_BE_STRING("discriminator '%s' must be a string"), diff --git a/json-java21-jtd/src/main/java/json/java21/jtd/JtdSchema.java b/json-java21-jtd/src/main/java/json/java21/jtd/JtdSchema.java index e42b555..d3773b7 100644 --- a/json-java21-jtd/src/main/java/json/java21/jtd/JtdSchema.java +++ b/json-java21-jtd/src/main/java/json/java21/jtd/JtdSchema.java @@ -4,42 +4,32 @@ import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; +import java.util.ArrayList; import java.util.List; /// JTD Schema interface - validates JSON instances against JTD schemas /// Following RFC 8927 specification with eight mutually-exclusive schema forms sealed interface JtdSchema { - /// Validates a JSON instance against this schema - /// @param instance The JSON value to validate - /// @return Result containing errors if validation fails - Jtd.Result validate(JsonValue instance); + /// Core frame-based validation that all schema variants must implement. + /// @param frame Current validation frame + /// @param errors Accumulates error messages + /// @param verboseErrors Whether to include verbose error details + /// @return true if valid, false otherwise + boolean validateWithFrame(Frame frame, java.util.List errors, boolean verboseErrors); - /// Validates a JSON instance against this schema using stack-based validation - /// @param frame The current validation frame containing schema, instance, path, and context - /// @param errors List to accumulate error messages - /// @param verboseErrors Whether to include full JSON values in error messages - /// @return true if validation passes, false if validation fails - default boolean validateWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { - // Default implementation delegates to existing validate method for backward compatibility - Jtd.Result result = validate(frame.instance(), verboseErrors); - if (!result.isValid()) { - errors.addAll(result.errors()); - return false; - } - return true; - } - - /// Validates a JSON instance against this schema with optional verbose errors - /// @param instance The JSON value to validate - /// @param verboseErrors Whether to include full JSON values in error messages - /// @return Result containing errors if validation fails + /// Default verbose-capable validation entrypoint constructing a root frame. default Jtd.Result validate(JsonValue instance, boolean verboseErrors) { - // Default implementation delegates to existing validate method - // Individual schema implementations can override for verbose error support - return validate(instance); + final var errors = new ArrayList(); + final var valid = validateWithFrame(new Frame(this, instance, "#", Crumbs.root()), errors, verboseErrors); + return valid ? Jtd.Result.success() : Jtd.Result.failure(errors); } - + + /// Non-verbose validation delegates to verbose variant. + default Jtd.Result validate(JsonValue instance) { + return validate(instance, false); + } + /// Nullable schema wrapper - allows null values record NullableSchema(JtdSchema wrapped) implements JtdSchema { @Override @@ -112,60 +102,51 @@ record TypeSchema(String type) implements JtdSchema { private static final java.util.regex.Pattern RFC3339 = java.util.regex.Pattern.compile( "^(\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:(\\d{2}|60)(\\.\\d+)?(Z|[+-]\\d{2}:\\d{2}))$" ); - - @Override - public Jtd.Result validate(JsonValue instance) { - return validate(instance, false); - } - - @Override - public Jtd.Result validate(JsonValue instance, boolean verboseErrors) { - return switch (type) { - case "boolean" -> validateBoolean(instance, verboseErrors); - case "string" -> validateString(instance, verboseErrors); - case "timestamp" -> validateTimestamp(instance, verboseErrors); - case "int8", "uint8", "int16", "uint16", "int32", "uint32" -> validateInteger(instance, type, verboseErrors); - case "float32", "float64" -> validateFloat(instance, type, verboseErrors); - default -> Jtd.Result.failure(Jtd.Error.UNKNOWN_TYPE.message(type)); - }; - } @SuppressWarnings("ClassEscapesDefinedScope") @Override public boolean validateWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { - Jtd.Result result = validate(frame.instance(), verboseErrors); - if (!result.isValid()) { - // Enrich errors with offset and path information - for (String error : result.errors()) { - String enrichedError = Jtd.enrichedError(error, frame, frame.instance()); - errors.add(enrichedError); + JsonValue instance = frame.instance(); + return switch (type) { + case "boolean" -> validateBooleanWithFrame(frame, errors, verboseErrors); + case "string" -> validateStringWithFrame(frame, errors, verboseErrors); + case "timestamp" -> validateTimestampWithFrame(frame, errors, verboseErrors); + case "int8", "uint8", "int16", "uint16", "int32", "uint32" -> validateIntegerWithFrame(frame, type, errors, verboseErrors); + case "float32", "float64" -> validateFloatWithFrame(frame, type, errors, verboseErrors); + default -> { + String error = Jtd.Error.UNKNOWN_TYPE.message(type); + errors.add(Jtd.enrichedError(error, frame, instance)); + yield false; } - return false; - } - return true; + }; } - - Jtd.Result validateBoolean(JsonValue instance, boolean verboseErrors) { + + boolean validateBooleanWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonBoolean) { - return Jtd.Result.success(); + return true; } String error = verboseErrors ? Jtd.Error.EXPECTED_BOOLEAN.message(instance, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_BOOLEAN.message(instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - Jtd.Result validateString(JsonValue instance, boolean verboseErrors) { + boolean validateStringWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonString) { - return Jtd.Result.success(); + return true; } String error = verboseErrors ? Jtd.Error.EXPECTED_STRING.message(instance, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_STRING.message(instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - Jtd.Result validateTimestamp(JsonValue instance, boolean verboseErrors) { + boolean validateTimestampWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonString str) { String value = str.value(); if (RFC3339.matcher(value).matches()) { @@ -173,111 +154,116 @@ Jtd.Result validateTimestamp(JsonValue instance, boolean verboseErrors) { // Replace :60 with :59 to allow leap seconds through parsing String normalized = value.replace(":60", ":59"); OffsetDateTime.parse(normalized, DateTimeFormatter.ISO_OFFSET_DATE_TIME); - return Jtd.Result.success(); + return true; } catch (Exception ignore) {} } } String error = verboseErrors ? Jtd.Error.EXPECTED_TIMESTAMP.message(instance, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_TIMESTAMP.message(instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - Jtd.Result validateInteger(JsonValue instance, String type, boolean verboseErrors) { + private boolean hasFractionalComponent(Number value) { + return switch (value) { + case null -> false; + case Double d -> d != Math.floor(d); + case Float f -> f != Math.floor(f); + case java.math.BigDecimal bd -> bd.remainder(java.math.BigDecimal.ONE).signum() != 0; + default -> + // Long, Integer, Short, Byte are always integers + false; + }; + } + + boolean validateIntegerWithFrame(Frame frame, String type, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonNumber num) { Number value = num.toNumber(); - // Check if the number is not integral (has fractional part) - if (value instanceof Double d && d != Math.floor(d)) { - return Jtd.Result.failure(Jtd.Error.EXPECTED_INTEGER.message()); + // Check for fractional component first (applies to all Number types) + if (hasFractionalComponent(value)) { + String error = Jtd.Error.EXPECTED_INTEGER.message(); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - // Handle BigDecimal - check if it has fractional component (not just scale > 0) - // RFC 8927 §2.2.3.1: "An integer value is a number without a fractional component" - // Values like 3.0 or 3.000 are valid integers despite positive scale, but 3.1 is not - if (value instanceof java.math.BigDecimal bd && bd.remainder(java.math.BigDecimal.ONE).signum() != 0) { - return Jtd.Result.failure(Jtd.Error.EXPECTED_INTEGER.message()); + // Handle precision loss for large Double values + if (value instanceof Double d) { + if (d > Long.MAX_VALUE || d < Long.MIN_VALUE) { + String error = verboseErrors + ? Jtd.Error.EXPECTED_NUMERIC_TYPE.message(instance, type, "out of range") + : Jtd.Error.EXPECTED_NUMERIC_TYPE.message(type, "out of range"); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; + } } - // Convert to long for range checking + // Now check if the value is within range for the specific integer type + // Convert to long for range checking (works for all Number types) long longValue = value.longValue(); - - // Check ranges according to RFC 8927 §2.2.3.1 - boolean valid = switch (type) { + boolean inRange = switch (type) { case "int8" -> longValue >= -128 && longValue <= 127; case "uint8" -> longValue >= 0 && longValue <= 255; case "int16" -> longValue >= -32768 && longValue <= 32767; case "uint16" -> longValue >= 0 && longValue <= 65535; - case "int32" -> longValue >= -2147483648L && longValue <= 2147483647L; + case "int32" -> longValue >= Integer.MIN_VALUE && longValue <= Integer.MAX_VALUE; case "uint32" -> longValue >= 0 && longValue <= 4294967295L; - default -> false; + default -> true; }; - if (valid) { - return Jtd.Result.success(); + if (!inRange) { + String error = verboseErrors + ? Jtd.Error.EXPECTED_NUMERIC_TYPE.message(instance, type, "out of range") + : Jtd.Error.EXPECTED_NUMERIC_TYPE.message(type, "out of range"); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - - // Range violation - String error = verboseErrors - ? Jtd.Error.EXPECTED_NUMERIC_TYPE.message(instance, type, instance.getClass().getSimpleName()) - : Jtd.Error.EXPECTED_NUMERIC_TYPE.message(type, instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + return true; } - String error = verboseErrors ? Jtd.Error.EXPECTED_NUMERIC_TYPE.message(instance, type, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_NUMERIC_TYPE.message(type, instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } - Jtd.Result validateFloat(JsonValue instance, String type, boolean verboseErrors) { + boolean validateFloatWithFrame(Frame frame, String type, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonNumber) { - return Jtd.Result.success(); + return true; } String error = verboseErrors ? Jtd.Error.EXPECTED_NUMERIC_TYPE.message(instance, type, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_NUMERIC_TYPE.message(type, instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } + } /// Enum schema - validates against a set of string values record EnumSchema(List values) implements JtdSchema { + @SuppressWarnings("ClassEscapesDefinedScope") @Override - public Jtd.Result validate(JsonValue instance) { - return validate(instance, false); - } - - @Override - public Jtd.Result validate(JsonValue instance, boolean verboseErrors) { + public boolean validateWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { + JsonValue instance = frame.instance(); if (instance instanceof JsonString str) { if (values.contains(str.value())) { - return Jtd.Result.success(); + return true; } String error = verboseErrors ? Jtd.Error.VALUE_NOT_IN_ENUM.message(instance, str.value(), values) : Jtd.Error.VALUE_NOT_IN_ENUM.message(str.value(), values); - return Jtd.Result.failure(error); + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } String error = verboseErrors ? Jtd.Error.EXPECTED_STRING_FOR_ENUM.message(instance, instance.getClass().getSimpleName()) : Jtd.Error.EXPECTED_STRING_FOR_ENUM.message(instance.getClass().getSimpleName()); - return Jtd.Result.failure(error); - } - - @SuppressWarnings("ClassEscapesDefinedScope") - @Override - public boolean validateWithFrame(Frame frame, java.util.List errors, boolean verboseErrors) { - Jtd.Result result = validate(frame.instance(), verboseErrors); - if (!result.isValid()) { - // Enrich errors with offset and path information - for (String error : result.errors()) { - String enrichedError = Jtd.enrichedError(error, frame, frame.instance()); - errors.add(enrichedError); - } - return false; - } - return true; + errors.add(Jtd.enrichedError(error, frame, instance)); + return false; } } @@ -315,9 +301,7 @@ public boolean validateWithFrame(Frame frame, java.util.List errors, boo JsonValue instance = frame.instance(); if (!(instance instanceof JsonArray)) { - String error = verboseErrors - ? Jtd.Error.EXPECTED_ARRAY.message(instance, instance.getClass().getSimpleName()) - : Jtd.Error.EXPECTED_ARRAY.message(instance.getClass().getSimpleName()); + String error = Jtd.Error.EXPECTED_ARRAY.message(instance, instance.getClass().getSimpleName()); String enrichedError = Jtd.enrichedError(error, frame, instance); errors.add(enrichedError); return false; @@ -515,7 +499,6 @@ public Jtd.Result validate(JsonValue instance, boolean verboseErrors) { return Jtd.Result.success(); } - // Otherwise, validate against the chosen variant schema return variantSchema.validate(instance, verboseErrors); } @@ -554,7 +537,7 @@ public boolean validateWithFrame(Frame frame, java.util.List errors, boo return false; } - // For DiscriminatorSchema, push the variant schema for validation + // Variant schema will be pushed by caller return true; } } diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/CompilerSpecIT.java b/json-java21-jtd/src/test/java/json/java21/jtd/CompilerSpecIT.java new file mode 100644 index 0000000..58ae42f --- /dev/null +++ b/json-java21-jtd/src/test/java/json/java21/jtd/CompilerSpecIT.java @@ -0,0 +1,178 @@ +package json.java21.jtd; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import jdk.sandbox.java.util.json.Json; +import jdk.sandbox.java.util.json.JsonValue; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.TestFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Map; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/// Runs the official JTD Test Suite invalid schema tests as JUnit dynamic tests. +/// +/// This test class loads and runs invalid schema tests from the official JTD specification. +/// Each test case is a schema that violates JTD rules and should cause compilation to fail +/// with a deterministic error message. +/// +/// Test Format: +/// ```json +/// { +/// "null schema": null, +/// "boolean schema": true, +/// "illegal keyword": {"foo": 123}, +/// "discriminator with non-properties mapping": { +/// "discriminator": "kind", +/// "mapping": { +/// "bool": {"type": "boolean"} +/// } +/// } +/// } +/// ``` +/// +/// The test suite is extracted from the embedded ZIP file and run as dynamic tests. +/// All tests must fail compilation for RFC 8927 compliance. +public class CompilerSpecIT extends JtdTestBase { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final Path INVALID_SCHEMAS_FILE = Paths.get("target/test-data/json-typedef-spec-2025-09-27/tests/invalid_schemas.json"); + + /// Metrics tracking for test results + private static int totalTests = 0; + private static int passedTests = 0; + private static int failedTests = 0; + + @AfterAll + static void printMetrics() { + LOG.info(() -> String.format("JTD-COMPILER-SPEC: total=%d passed=%d failed=%d", + totalTests, passedTests, failedTests)); + + // RFC compliance: all tests must fail compilation + assertEquals(totalTests, passedTests + failedTests, "Test accounting mismatch"); + } + + @TestFactory + Stream runInvalidSchemaTests() throws Exception { + LOG.info(() -> "Running JTD Invalid Schema Compilation Tests"); + + // Ensure test data is extracted + extractTestData(); + + return runInvalidSchemaCompilationTests(); + } + + private Stream runInvalidSchemaCompilationTests() throws Exception { + LOG.info(() -> "Running invalid schema compilation tests from: " + INVALID_SCHEMAS_FILE); + JsonNode invalidSchemas = loadTestFile(INVALID_SCHEMAS_FILE); + + return StreamSupport.stream(((Iterable>) invalidSchemas::fields).spliterator(), false) + .map(entry -> { + String testName = "invalid schema: " + entry.getKey(); + JsonNode schema = entry.getValue(); + return createInvalidSchemaTest(testName, schema); + }); + } + + private void extractTestData() throws IOException { + // Check if test data is already extracted + if (Files.exists(INVALID_SCHEMAS_FILE)) { + LOG.fine(() -> "JTD invalid schemas test suite already extracted at: " + INVALID_SCHEMAS_FILE); + return; + } + + // Extract the embedded test suite + Path zipFile = Paths.get("src/test/resources/jtd-test-suite.zip"); + Path targetDir = Paths.get("target/test-data"); + + if (!Files.exists(zipFile)) { + throw new RuntimeException("JTD test suite ZIP not found: " + zipFile.toAbsolutePath()); + } + + LOG.info(() -> "Extracting JTD test suite from: " + zipFile); + + // Create target directory + Files.createDirectories(targetDir); + + // Extract ZIP file + try (var zis = new java.util.zip.ZipInputStream(Files.newInputStream(zipFile))) { + java.util.zip.ZipEntry entry; + while ((entry = zis.getNextEntry()) != null) { + if (!entry.isDirectory() && entry.getName().startsWith("json-typedef-spec-")) { + Path outputPath = targetDir.resolve(entry.getName()); + Files.createDirectories(outputPath.getParent()); + Files.copy(zis, outputPath, java.nio.file.StandardCopyOption.REPLACE_EXISTING); + } + zis.closeEntry(); + } + } + + if (!Files.exists(INVALID_SCHEMAS_FILE)) { + throw new RuntimeException("Extraction completed but test file not found: " + INVALID_SCHEMAS_FILE); + } + } + + @SuppressWarnings("SameParameterValue") + private JsonNode loadTestFile(Path testFile) throws IOException { + if (!Files.exists(testFile)) { + throw new RuntimeException("JTD test file not found: " + testFile); + } + + LOG.fine(() -> "Loading JTD test file from: " + testFile); + return MAPPER.readTree(Files.newInputStream(testFile)); + } + + private DynamicTest createInvalidSchemaTest(String testName, JsonNode schemaNode) { + return DynamicTest.dynamicTest(testName, () -> { + totalTests++; + + // INFO level logging as required by AGENTS.md - announce test execution + LOG.info(() -> "EXECUTING: " + testName); + + try { + // FINE level logging for test details + LOG.fine(() -> String.format("Invalid schema test details - schema: %s", schemaNode)); + + // Convert to java.util.json format + JsonValue schema = Json.parse(schemaNode.toString()); + + // Create validator and attempt compilation - this should fail + Jtd validator = new Jtd(); + + // Expect compilation to fail with IllegalArgumentException + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for invalid schema" + ); + + // FINE level logging for compilation failure + LOG.fine(() -> String.format("Compilation failed as expected for %s - error: %s", testName, exception.getMessage())); + + passedTests++; + + } catch (AssertionError e) { + failedTests++; + // Log SEVERE for test failures with full details including the actual schema input + LOG.severe(() -> String.format("ERROR: Invalid schema test FAILED: %s\nSchema: %s\nExpected: compilation to fail\nActual: compilation succeeded\nAssertionError: %s", + testName, schemaNode, e.getMessage())); + throw new RuntimeException("Invalid schema test failed: " + testName, e); + } catch (Exception e) { + failedTests++; + // Log SEVERE for test failures with full details + LOG.severe(() -> String.format("ERROR: Invalid schema test FAILED: %s\nSchema: %s\nException: %s", + testName, schemaNode, e.getMessage())); + throw new RuntimeException("Invalid schema test failed: " + testName, e); + } + }); + } +} diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/CompilerTest.java b/json-java21-jtd/src/test/java/json/java21/jtd/CompilerTest.java new file mode 100644 index 0000000..dea21ba --- /dev/null +++ b/json-java21-jtd/src/test/java/json/java21/jtd/CompilerTest.java @@ -0,0 +1,701 @@ +package json.java21.jtd; + +import jdk.sandbox.java.util.json.Json; +import jdk.sandbox.java.util.json.JsonValue; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/// Unit tests for JTD schema compilation focusing on discriminator constraints and compile-time validation. +/// +/// These tests verify that the compiler correctly rejects invalid schemas according to RFC 8927 §2.2.8 +/// while allowing valid discriminator schemas to be compiled successfully. +public class CompilerTest extends JtdTestBase { + + @Test + void discriminatorMappingMustBePropertiesSchema() { + LOG.info(() -> "EXECUTING: discriminatorMappingMustBePropertiesSchema"); + + // Invalid: mapping value is a primitive type schema instead of properties schema + String invalidSchema = """ + { + "discriminator": "kind", + "mapping": { + "bool": {"type": "boolean"} + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for discriminator with non-properties mapping" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("mapping"), + "Error message should mention mapping constraint"); + } + + @Test + void discriminatorMappingCannotBeNullable() { + LOG.info(() -> "EXECUTING: discriminatorMappingCannotBeNullable"); + + // Invalid: mapping value has nullable: true + String invalidSchema = """ + { + "discriminator": "kind", + "mapping": { + "person": { + "properties": { + "name": {"type": "string"} + }, + "nullable": true + } + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for discriminator with nullable mapping" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("nullable"), + "Error message should mention nullable constraint"); + } + + @Test + void discriminatorMappingCannotRedefineDiscriminatorKey() { + LOG.info(() -> "EXECUTING: discriminatorMappingCannotRedefineDiscriminatorKey"); + + // Invalid: mapping schema defines the discriminator key in properties + String invalidSchema = """ + { + "discriminator": "kind", + "mapping": { + "person": { + "properties": { + "kind": {"type": "string"}, + "name": {"type": "string"} + } + } + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for discriminator with redefined key" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("discriminator") || exception.getMessage().contains("kind"), + "Error message should mention discriminator key conflict"); + } + + @Test + void discriminatorMappingCannotRedefineDiscriminatorKeyInOptional() { + LOG.info(() -> "EXECUTING: discriminatorMappingCannotRedefineDiscriminatorKeyInOptional"); + + // Invalid: mapping schema defines the discriminator key in optionalProperties + String invalidSchema = """ + { + "discriminator": "type", + "mapping": { + "person": { + "optionalProperties": { + "type": {"type": "string"}, + "name": {"type": "string"} + } + } + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for discriminator with redefined key in optional" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("discriminator") || exception.getMessage().contains("type"), + "Error message should mention discriminator key conflict"); + } + + @Test + void validDiscriminatorSchemaCompilesSuccessfully() { + LOG.info(() -> "EXECUTING: validDiscriminatorSchemaCompilesSuccessfully"); + + // Valid: discriminator with proper properties mapping + String validSchema = """ + { + "discriminator": "kind", + "mapping": { + "person": { + "properties": { + "name": {"type": "string"}, + "age": {"type": "int32"} + } + }, + "company": { + "properties": { + "name": {"type": "string"}, + "employees": {"type": "int32"} + } + } + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"kind\":\"person\",\"name\":\"Alice\",\"age\":30}"); + JsonValue validCompanyInstance = Json.parse("{\"kind\":\"company\",\"name\":\"Acme\",\"employees\":100}"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result1 = validator.validate(schema, validInstance); + Jtd.Result result2 = validator.validate(schema, validCompanyInstance); + + assertTrue(result1.isValid(), "Valid person instance should pass validation"); + assertTrue(result2.isValid(), "Valid company instance should pass validation"); + + LOG.fine(() -> "Valid discriminator schema compiled and validated successfully"); + } + + @Test + void discriminatorWithAdditionalPropertiesValidation() { + LOG.info(() -> "EXECUTING: discriminatorWithAdditionalPropertiesValidation"); + + // Valid: discriminator field should be exempt from additionalProperties validation + String validSchema = """ + { + "discriminator": "type", + "mapping": { + "data": { + "properties": { + "value": {"type": "string"} + }, + "additionalProperties": false + } + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"type\":\"data\",\"value\":\"test\"}"); + JsonValue invalidInstance = Json.parse("{\"type\":\"data\",\"value\":\"test\",\"extra\":\"field\"}"); + + Jtd validator = new Jtd(); + + // Valid instance should pass + Jtd.Result validResult = validator.validate(schema, validInstance); + assertTrue(validResult.isValid(), "Instance with only discriminator and defined properties should be valid"); + + // Invalid instance should fail due to extra field (not discriminator) + Jtd.Result invalidResult = validator.validate(schema, invalidInstance); + assertFalse(invalidResult.isValid(), "Instance with extra field should fail validation"); + + LOG.fine(() -> "Discriminator exemption from additionalProperties works correctly"); + } + + @Test + void emptySchemaCompilesAndAcceptsAllValues() { + LOG.info(() -> "EXECUTING: emptySchemaCompilesAndAcceptsAllValues"); + + // Valid: empty schema {} should accept any JSON value + String emptySchema = "{}"; + JsonValue schema = Json.parse(emptySchema); + + Jtd validator = new Jtd(); + + // Should accept various JSON values + assertTrue(validator.validate(schema, Json.parse("null")).isValid()); + assertTrue(validator.validate(schema, Json.parse("true")).isValid()); + assertTrue(validator.validate(schema, Json.parse("42")).isValid()); + assertTrue(validator.validate(schema, Json.parse("\"hello\"")).isValid()); + assertTrue(validator.validate(schema, Json.parse("[]")).isValid()); + assertTrue(validator.validate(schema, Json.parse("{}")).isValid()); + + LOG.fine(() -> "Empty schema correctly accepts all JSON values"); + } + + @Test + void nullableMustBeBoolean() { + LOG.info(() -> "EXECUTING: nullableMustBeBoolean"); + + // Invalid: nullable value is not a boolean + String invalidSchema = """ + { + "type": "string", + "nullable": 123 + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing nullable with non-boolean value: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for nullable with non-boolean value" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("nullable") && exception.getMessage().contains("boolean"), + "Error message should mention nullable must be boolean"); + } + + @Test + void definitionsMustBeObject() { + LOG.info(() -> "EXECUTING: definitionsMustBeObject"); + + // Invalid: definitions is not an object + String invalidSchema = """ + { + "definitions": 123 + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing definitions with non-object value: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for definitions with non-object value" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("definitions") && exception.getMessage().contains("object"), + "Error message should mention definitions must be object"); + } + + @Test + void refWithoutDefinitionsShouldFail() { + LOG.info(() -> "EXECUTING: refWithoutDefinitionsShouldFail"); + + // Invalid: ref points to definition but no definitions object exists + String invalidSchema = """ + { + "ref": "foo" + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing ref without definitions: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for ref without definitions" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("ref") || exception.getMessage().contains("definition"), + "Error message should mention ref or definition issues"); + } + + @Test + void refToNonExistentDefinitionShouldFail() { + LOG.info(() -> "EXECUTING: refToNonExistentDefinitionShouldFail"); + + // Invalid: ref points to non-existent definition + String invalidSchema = """ + { + "definitions": {}, + "ref": "foo" + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing ref to non-existent definition: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for ref to non-existent definition" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("ref") || exception.getMessage().contains("definition"), + "Error message should mention ref or definition issues"); + } + + @Test + void subSchemaRefToNonExistentDefinitionShouldFail() { + LOG.info(() -> "EXECUTING: subSchemaRefToNonExistentDefinitionShouldFail"); + + // Invalid: sub-schema ref points to non-existent definition + String invalidSchema = """ + { + "definitions": {}, + "elements": { + "ref": "foo" + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing sub-schema ref to non-existent definition: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for sub-schema ref to non-existent definition" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("ref") || exception.getMessage().contains("definition"), + "Error message should mention ref or definition issues"); + } + + @Test + void invalidTypeStringValueShouldFail() { + LOG.info(() -> "EXECUTING: invalidTypeStringValueShouldFail"); + + // Invalid: type value is not a valid primitive type + String invalidSchema = """ + { + "type": "foo" + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing invalid type string value: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for invalid type string value" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("type") || exception.getMessage().contains("unknown"), + "Error message should mention invalid type"); + } + + @Test + void enumWithDuplicatesShouldFail() { + LOG.info(() -> "EXECUTING: enumWithDuplicatesShouldFail"); + + // Invalid: enum contains duplicate values + String invalidSchema = """ + { + "enum": ["foo", "bar", "foo"] + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing enum with duplicates: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for enum with duplicates" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("enum") || exception.getMessage().contains("duplicate"), + "Error message should mention enum duplicates"); + } + + @Test + void elementsWithAdditionalPropertiesShouldFail() { + LOG.info(() -> "EXECUTING: elementsWithAdditionalPropertiesShouldFail"); + + // Invalid: elements form with additionalProperties (form-specific property mixing) + String invalidSchema = """ + { + "elements": {}, + "additionalProperties": true + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing elements with additionalProperties: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for elements with additionalProperties" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("form") || exception.getMessage().contains("additionalProperties"), + "Error message should mention form mixing or additionalProperties"); + } + + @Test + void unknownSchemaKeysCauseCompilationFailure() { + + // Invalid: schema contains unknown keys + String invalidSchema = """ + { + "type": "string", + "unknownKey": "value" + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for schema with unknown keys" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("unknown"), + "Error message should mention unknown keys"); + } + + @Test + void enumWithDuplicatesShouldFailAtCompileTime() { + LOG.info(() -> "EXECUTING: enumWithDuplicatesShouldFailAtCompileTime"); + + // Invalid: enum contains duplicate values - should fail at compile time + String invalidSchema = """ + { + "enum": ["red", "red"] + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing enum with duplicates: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for enum with duplicates" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("duplicate"), + "Error message should mention enum duplicates"); + } + + @Test + void propertiesAndOptionalPropertiesKeyOverlapShouldFail() { + LOG.info(() -> "EXECUTING: propertiesAndOptionalPropertiesKeyOverlapShouldFail"); + + // Invalid: same key defined in both properties and optionalProperties + String invalidSchema = """ + { + "properties": { + "name": {"type": "string"} + }, + "optionalProperties": { + "name": {"type": "string"} + } + } + """; + + JsonValue schema = Json.parse(invalidSchema); + Jtd validator = new Jtd(); + + LOG.fine(() -> "Testing properties and optionalProperties key overlap: " + schema); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> validator.compile(schema), + "Expected compilation to fail for overlapping keys" + ); + + LOG.fine(() -> "Compilation failed as expected: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("Key") && exception.getMessage().contains("name"), + "Error message should mention overlapping key 'name'"); + } + + @Test + void elementsWithNestedDefinitionsShouldWork() { + LOG.info(() -> "EXECUTING: elementsWithNestedDefinitionsShouldWork"); + + // Valid: elements schema with nested definitions + String validSchema = """ + { + "definitions": { + "item": {"type": "string"} + }, + "elements": { + "ref": "item" + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("[\"hello\", \"world\"]"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result = validator.validate(schema, validInstance); + + assertTrue(result.isValid(), "Elements with nested definitions should validate successfully"); + LOG.fine(() -> "Elements with nested definitions compiled and validated successfully"); + } + + @Test + void propertiesWithNestedDefinitionsShouldWork() { + LOG.info(() -> "EXECUTING: propertiesWithNestedDefinitionsShouldWork"); + + // Valid: properties schema with nested definitions + String validSchema = """ + { + "definitions": { + "name": {"type": "string"} + }, + "properties": { + "firstName": {"ref": "name"}, + "lastName": {"ref": "name"} + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"firstName\":\"John\",\"lastName\":\"Doe\"}"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result = validator.validate(schema, validInstance); + + assertTrue(result.isValid(), "Properties with nested definitions should validate successfully"); + LOG.fine(() -> "Properties with nested definitions compiled and validated successfully"); + } + + @Test + void optionalPropertiesWithNestedDefinitionsShouldWork() { + LOG.info(() -> "EXECUTING: optionalPropertiesWithNestedDefinitionsShouldWork"); + + // Valid: optionalProperties schema with nested definitions + String validSchema = """ + { + "definitions": { + "address": { + "properties": { + "street": {"type": "string"}, + "city": {"type": "string"} + } + } + }, + "optionalProperties": { + "homeAddress": {"ref": "address"}, + "workAddress": {"ref": "address"} + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"homeAddress\":{\"street\":\"123 Main St\",\"city\":\"Anytown\"}}"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result = validator.validate(schema, validInstance); + + assertTrue(result.isValid(), "OptionalProperties with nested definitions should validate successfully"); + LOG.fine(() -> "OptionalProperties with nested definitions compiled and validated successfully"); + } + + @Test + void valuesWithNestedDefinitionsShouldWork() { + LOG.info(() -> "EXECUTING: valuesWithNestedDefinitionsShouldWork"); + + // Valid: values schema with nested definitions + String validSchema = """ + { + "definitions": { + "user": { + "properties": { + "name": {"type": "string"} + } + } + }, + "values": { + "ref": "user" + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"user1\":{\"name\":\"Alice\"},\"user2\":{\"name\":\"Bob\"}}"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result = validator.validate(schema, validInstance); + + assertTrue(result.isValid(), "Values with nested definitions should validate successfully"); + LOG.fine(() -> "Values with nested definitions compiled and validated successfully"); + } + + @Test + void discriminatorMappingWithNestedDefinitionsShouldWork() { + LOG.info(() -> "EXECUTING: discriminatorMappingWithNestedDefinitionsShouldWork"); + + // Valid: discriminator mapping with nested definitions + String validSchema = """ + { + "definitions": { + "personName": {"type": "string"} + }, + "discriminator": "kind", + "mapping": { + "person": { + "properties": { + "name": {"ref": "personName"} + } + } + } + } + """; + + JsonValue schema = Json.parse(validSchema); + JsonValue validInstance = Json.parse("{\"kind\":\"person\",\"name\":\"Alice\"}"); + + Jtd validator = new Jtd(); + + // Should compile and validate successfully + Jtd.Result result = validator.validate(schema, validInstance); + + assertTrue(result.isValid(), "Discriminator mapping with nested definitions should validate successfully"); + LOG.fine(() -> "Discriminator mapping with nested definitions compiled and validated successfully"); + } +} \ No newline at end of file diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/JtdPropertyTest.java b/json-java21-jtd/src/test/java/json/java21/jtd/JtdPropertyTest.java index 36f694e..ea3ea39 100644 --- a/json-java21-jtd/src/test/java/json/java21/jtd/JtdPropertyTest.java +++ b/json-java21-jtd/src/test/java/json/java21/jtd/JtdPropertyTest.java @@ -261,7 +261,12 @@ private static Arbitrary jtdSchemaArbitrary(int depth) { } private static Arbitrary enumSchemaArbitrary() { - return Arbitraries.of(ENUM_VALUES).list().ofMinSize(1).ofMaxSize(4).map(values -> new EnumSchema(new ArrayList<>(values))); + // Ensure no duplicates by using distinct values + return Arbitraries.of(ENUM_VALUES).list().ofMinSize(1).ofMaxSize(4).map(values -> { + // Remove duplicates to ensure valid enum schema per RFC 8927 + List distinctValues = values.stream().distinct().toList(); + return new EnumSchema(new ArrayList<>(distinctValues)); + }); } private static Arbitrary elementsSchemaArbitrary(int depth) { @@ -314,26 +319,37 @@ private static Arbitrary simplePropertiesSchemaArbitrary() { // Create primitive schemas that don't recurse final var primitiveSchemas = Arbitraries.of(new EmptySchema(), new TypeSchema("boolean"), new TypeSchema("string"), new TypeSchema("int32"), new EnumSchema(List.of("red", "green", "blue"))); + // ======================== CHANGE: USE NON-DISCRIMINATOR PROPERTY NAMES ======================== + // RFC 8927 §2.2.8: Discriminator mapping schemas cannot define the discriminator key + // Use property names that won't conflict with discriminator keys + final var safePropertyNames = List.of("beta", "gamma", "delta", "epsilon"); // Exclude "alpha" + final var safePropertyPairs = List.of( + List.of("beta", "gamma"), + List.of("gamma", "delta"), + List.of("delta", "epsilon") + ); + // ==================== END CHANGE: USE NON-DISCRIMINATOR PROPERTY NAMES ==================== + return Arbitraries.oneOf( // Empty properties schema Arbitraries.of(new PropertiesSchema(Map.of(), Map.of(), false)), // Single required property with primitive schema - Combinators.combine(Arbitraries.of(PROPERTY_NAMES), primitiveSchemas).as((name, schema) -> { + Combinators.combine(Arbitraries.of(safePropertyNames), primitiveSchemas).as((name, schema) -> { Assertions.assertNotNull(name); Assertions.assertNotNull(schema); return new PropertiesSchema(Map.of(name, schema), Map.of(), false); }), // Single optional property with primitive schema - Combinators.combine(Arbitraries.of(PROPERTY_NAMES), primitiveSchemas).as((name, schema) -> { + Combinators.combine(Arbitraries.of(safePropertyNames), primitiveSchemas).as((name, schema) -> { Assertions.assertNotNull(name); Assertions.assertNotNull(schema); return new PropertiesSchema(Map.of(), Map.of(name, schema), false); }), // Required + optional property with primitive schemas - Combinators.combine(Arbitraries.of(PROPERTY_PAIRS), primitiveSchemas, primitiveSchemas).as((names, requiredSchema, optionalSchema) -> { + Combinators.combine(Arbitraries.of(safePropertyPairs), primitiveSchemas, primitiveSchemas).as((names, requiredSchema, optionalSchema) -> { Assertions.assertNotNull(names); Assertions.assertNotNull(requiredSchema); Assertions.assertNotNull(optionalSchema); @@ -343,17 +359,72 @@ private static Arbitrary simplePropertiesSchemaArbitrary() { private static Arbitrary discriminatorSchemaArbitrary() { - return Combinators.combine(Arbitraries.of(PROPERTY_NAMES), Arbitraries.of(DISCRIMINATOR_VALUES), Arbitraries.of(DISCRIMINATOR_VALUES), simplePropertiesSchemaArbitrary(), simplePropertiesSchemaArbitrary()).as((discriminatorKey, value1, value2, schema1, schema2) -> { + return Combinators.combine(Arbitraries.of(PROPERTY_NAMES), Arbitraries.of(DISCRIMINATOR_VALUES), Arbitraries.of(DISCRIMINATOR_VALUES)).as((discriminatorKey, value1, value2) -> { final var mapping = new LinkedHashMap(); + + // Generate properties schemas that avoid the discriminator key + final var schema1 = propertiesSchemaForDiscriminatorMapping(discriminatorKey).sample(); mapping.put(value1, schema1); + Assertions.assertNotNull(value1); if (!value1.equals(value2)) { + final var schema2 = propertiesSchemaForDiscriminatorMapping(discriminatorKey).sample(); mapping.put(value2, schema2); } return new DiscriminatorSchema(discriminatorKey, mapping); }); } + /// Creates PropertiesSchema instances specifically for discriminator mappings + /// RFC 8927 §2.2.8 requires mapping values to be PropertiesSchema (not EmptySchema) + /// and cannot define the discriminator key in properties or optionalProperties + private static Arbitrary propertiesSchemaForDiscriminatorMapping(String discriminatorKey) { + // Create primitive schemas that don't recurse + final var primitiveSchemas = Arbitraries.of(new TypeSchema("boolean"), new TypeSchema("string"), new TypeSchema("int32"), new EnumSchema(List.of("red", "green", "blue"))); + + // Create safe property names that exclude the discriminator key + final var allPropertyNames = List.of("alpha", "beta", "gamma", "delta", "epsilon"); + final var safePropertyNames = allPropertyNames.stream() + .filter(name -> !name.equals(discriminatorKey)) + .toList(); + + // If we removed too many names, add some backup names + final var effectivePropertyNames = safePropertyNames.isEmpty() + ? List.of("prop1", "prop2", "prop3") + : safePropertyNames; + + // Create safe property pairs + final var safePropertyPairs = effectivePropertyNames.stream() + .flatMap(name1 -> effectivePropertyNames.stream() + .filter(name2 -> !name1.equals(name2)) + .map(name2 -> List.of(name1, name2))) + .filter(pair -> !pair.get(0).equals(discriminatorKey) && !pair.get(1).equals(discriminatorKey)) + .toList(); + + return Arbitraries.oneOf( + // Single required property with primitive schema + Combinators.combine(Arbitraries.of(effectivePropertyNames), primitiveSchemas).as((name, schema) -> { + Assertions.assertNotNull(name); + Assertions.assertNotNull(schema); + return new PropertiesSchema(Map.of(name, schema), Map.of(), false); + }), + + // Single optional property with primitive schema + Combinators.combine(Arbitraries.of(effectivePropertyNames), primitiveSchemas).as((name, schema) -> { + Assertions.assertNotNull(name); + Assertions.assertNotNull(schema); + return new PropertiesSchema(Map.of(), Map.of(name, schema), false); + }), + + // Required + optional property with primitive schemas + Combinators.combine(Arbitraries.of(safePropertyPairs), primitiveSchemas, primitiveSchemas).as((names, requiredSchema, optionalSchema) -> { + Assertions.assertNotNull(names); + Assertions.assertNotNull(requiredSchema); + Assertions.assertNotNull(optionalSchema); + return new PropertiesSchema(Map.of(names.getFirst(), requiredSchema), Map.of(names.get(1), optionalSchema), false); + })); + } + private static Arbitrary nullableSchemaArbitrary(int depth) { return jtdSchemaArbitrary(depth - 1).map(NullableSchema::new); } @@ -388,7 +459,15 @@ void exhaustiveJtdValidation(@ForAll("jtdSchemas") JtdPropertyTest.JtdTestSchema final var validationResult = validator.validate(schemaJson, compliantDocument); if (!validationResult.isValid()) { - LOG.severe(() -> String.format("ERROR: Compliant document failed validation!%nSchema: %s%nDocument: %s%nErrors: %s", schemaJson, compliantDocument, validationResult.errors())); + String errorMessage = String.format( + "ERROR: Compliant document failed validation!%nSchema JSON: %s%nDocument JSON: %s%nValidation Errors: %s%nSchema Description: %s%nFull Schema Object: %s", + Json.toDisplayString(schemaJson, 2), + Json.toDisplayString(compliantDocument, 2), + validationResult.errors(), + schemaDescription, + schema + ); + LOG.severe(() -> errorMessage); } assertThat(validationResult.isValid()).as("Compliant JTD document should validate for schema %s", schemaDescription).isTrue(); @@ -410,7 +489,9 @@ void exhaustiveJtdValidation(@ForAll("jtdSchemas") JtdPropertyTest.JtdTestSchema final var failingResult = validator.validate(schemaJson, failing); if (failingResult.isValid()) { - LOG.severe(() -> String.format("UNEXPECTED: Failing document passed validation!%nSchema: %s%nDocument: %s%nExpected: FAILURE, Got: SUCCESS", schemaJson, failing)); + LOG.severe(() -> String.format("UNEXPECTED: Failing document passed validation!%nSchema JSON: %s%nDocument JSON: %s%nExpected: FAILURE, Got: SUCCESS", + Json.toDisplayString(schemaJson, 2), + Json.toDisplayString(failing, 2))); } assertThat(failingResult.isValid()).as("Expected JTD validation failure for %s against schema %s", failing, schemaDescription).isFalse(); diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/JtdSpecIT.java b/json-java21-jtd/src/test/java/json/java21/jtd/JtdSpecIT.java index 6ccdf35..e2edde5 100644 --- a/json-java21-jtd/src/test/java/json/java21/jtd/JtdSpecIT.java +++ b/json-java21-jtd/src/test/java/json/java21/jtd/JtdSpecIT.java @@ -28,9 +28,6 @@ /// - `instance`: A JSON value to validate against the schema /// - `errors`: Expected validation errors (empty array for valid instances) /// -/// 2. **invalid_schemas.json** - Contains schemas that should be rejected as invalid JTD schemas. -/// Each entry is a schema that violates JTD rules and should cause compilation to fail. -/// /// Test Format Examples: /// ```json /// // validation.json - Valid case @@ -51,13 +48,6 @@ /// } /// } /// -/// // invalid_schemas.json - Schema compilation should fail -/// { -/// "null schema": null, -/// "boolean schema": true, -/// "illegal keyword": {"foo": 123} -/// } -/// ``` /// /// The test suite is extracted from the embedded ZIP file and run as dynamic tests. /// All tests must pass for RFC 8927 compliance. @@ -88,11 +78,7 @@ Stream runJtdSpecSuite() throws Exception { // Ensure test data is extracted extractTestData(); - // Run both validation tests and invalid schema tests - Stream validationTests = runValidationTests(); - Stream invalidSchemaTests = runInvalidSchemaTests(); - - return Stream.concat(validationTests, invalidSchemaTests); + return runValidationTests(); } private Stream runValidationTests() throws Exception { @@ -107,18 +93,6 @@ private Stream runValidationTests() throws Exception { }); } - private Stream runInvalidSchemaTests() throws Exception { - LOG.info(() -> "Running invalid schema tests from: " + INVALID_SCHEMAS_FILE); - JsonNode invalidSchemas = loadTestFile(INVALID_SCHEMAS_FILE); - - return StreamSupport.stream(((Iterable>) invalidSchemas::fields).spliterator(), false) - .map(entry -> { - String testName = "invalid schema: " + entry.getKey(); - JsonNode schema = entry.getValue(); - return createInvalidSchemaTest(testName, schema); - }); - } - private void extractTestData() throws IOException { // Check if test data is already extracted if (Files.exists(VALIDATION_TEST_FILE)) { @@ -219,17 +193,4 @@ private DynamicTest createValidationTest(String testName, JsonNode testCase) { } }); } - - private DynamicTest createInvalidSchemaTest(String testName, JsonNode schema) { - return DynamicTest.dynamicTest(testName, () -> { - // FIXME: commenting out raised as gh issue #86 - Invalid schema test logic being ignored - // https://github.com/simbo1905/java.util.json.Java21/issues/86 - // - // These tests should fail because invalid schemas should be rejected during compilation, - // but currently they only log warnings and pass. Disabling until the issue is fixed. - LOG.info(() -> "SKIPPED (issue #86): " + testName + " - invalid schema validation not properly implemented"); - totalTests++; - passedTests++; // Count as passed for now to avoid CI failure - }); - } } diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java b/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java index bfd53e6..ec08efc 100644 --- a/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java +++ b/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java @@ -563,14 +563,12 @@ public void testDiscriminatorInElementsSchema() { "mapping": { "config": { "properties": { - "type": {}, "value": {"type": "string"} }, "additionalProperties": false }, "flag": { "properties": { - "type": {}, "enabled": {"type": "boolean"} }, "additionalProperties": false @@ -740,9 +738,7 @@ public void testDiscriminatorFormWithEmptySchemaProperty() { "discriminator": "alpha", "mapping": { "type1": { - "properties": { - "alpha": {} - } + "properties": {} } } } @@ -787,7 +783,6 @@ public void testDiscriminatorWithAdditionalRequiredProperties() { "mapping": { "user": { "properties": { - "type": {}, "name": {"type": "string"} }, "additionalProperties": false @@ -815,7 +810,7 @@ public void testDiscriminatorWithAdditionalRequiredProperties() { .isFalse(); assertThat(invalidResult.errors()) .as("Should report missing required property") - .anyMatch(error -> error.contains("missing required property: name")); + .anyMatch(error -> error.contains("missing required property: 'name'")); } /// Test discriminator form with optional properties @@ -827,9 +822,6 @@ public void testDiscriminatorWithOptionalProperties() { "discriminator": "kind", "mapping": { "circle": { - "properties": { - "kind": {} - }, "optionalProperties": { "radius": {"type": "float32"} }, @@ -868,7 +860,6 @@ public void testDiscriminatorInOptionalProperties() { "mapping": { "default": { "optionalProperties": { - "mode": {}, "config": {"type": "string"} }, "additionalProperties": false @@ -886,4 +877,283 @@ public void testDiscriminatorInOptionalProperties() { .as("Should accept discriminator field in optionalProperties") .isTrue(); } + + /// Test for the critical integer range validation bug + /// This test specifically targets the issue where Double values bypass range checks + /// JsonNumber.toNumber() commonly returns Double, which falls through validation + @Test + public void testInt8RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"int8\"}"); + + // These values should fail int8 validation (range: -128 to 127) + // But they pass through because Double values aren't handled in range checking + JsonValue[] outOfRangeValues = { + Json.parse("1000"), // Way above int8 max (127) + Json.parse("-500"), // Way below int8 min (-128) + Json.parse("300"), // Above int8 max + Json.parse("-200") // Below int8 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + LOG.fine(() -> "Testing int8 range with Double value: " + outOfRange + + " (JsonNumber.toNumber() type: " + + ((JsonNumber)outOfRange).toNumber().getClass().getSimpleName() + ")"); + + // This should fail but currently passes due to the bug + assertThat(result.isValid()) + .as("int8 should reject out-of-range value %s (likely parsed as Double)", outOfRange) + .isFalse(); + assertThat(result.errors()) + .as("Should have range validation errors for %s", outOfRange) + .isNotEmpty(); + } + } + + /// Test uint8 range validation with Double values + @Test + public void testUint8RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"uint8\"}"); + + // These should fail uint8 validation (range: 0 to 255) + JsonValue[] outOfRangeValues = { + Json.parse("1000"), // Way above uint8 max (255) + Json.parse("-1"), // Below uint8 min (0) + Json.parse("500"), // Above uint8 max + Json.parse("-100") // Below uint8 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + assertThat(result.isValid()) + .as("uint8 should reject out-of-range value %s", outOfRange) + .isFalse(); + assertThat(result.errors()) + .as("Should have range validation errors for %s", outOfRange) + .isNotEmpty(); + } + } + + /// Test int16 range validation with Double values + @Test + public void testInt16RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"int16\"}"); + + // These should fail int16 validation (range: -32768 to 32767) + JsonValue[] outOfRangeValues = { + Json.parse("100000"), // Way above int16 max + Json.parse("-100000"), // Way below int16 min + Json.parse("50000"), // Above int16 max + Json.parse("-50000") // Below int16 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + assertThat(result.isValid()) + .as("int16 should reject out-of-range value %s", outOfRange) + .isFalse(); + } + } + + /// Test uint16 range validation with Double values + @Test + public void testUint16RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"uint16\"}"); + + JsonValue[] outOfRangeValues = { + Json.parse("100000"), // Way above uint16 max (65535) + Json.parse("-1"), // Below uint16 min (0) + Json.parse("70000"), // Above uint16 max + Json.parse("-1000") // Below uint16 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + assertThat(result.isValid()) + .as("uint16 should reject out-of-range value %s", outOfRange) + .isFalse(); + } + } + + /// Test int32 range validation with Double values + @Test + public void testInt32RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"int32\"}"); + + // These should fail int32 validation (range: -2147483648 to 2147483647) + JsonValue[] outOfRangeValues = { + Json.parse("3000000000"), // Above int32 max + Json.parse("-3000000000"), // Below int32 min + Json.parse("2200000000"), // Above int32 max + Json.parse("-2200000000") // Below int32 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + assertThat(result.isValid()) + .as("int32 should reject out-of-range value %s", outOfRange) + .isFalse(); + } + } + + /// Test uint32 range validation with Double values + @Test + public void testUint32RangeValidationWithDoubleValues() { + JsonValue schema = Json.parse("{\"type\": \"uint32\"}"); + + // These should fail uint32 validation (range: 0 to 4294967295) + JsonValue[] outOfRangeValues = { + Json.parse("5000000000"), // Above uint32 max + Json.parse("-1"), // Below uint32 min + Json.parse("4500000000"), // Above uint32 max + Json.parse("-1000") // Below uint32 min + }; + + Jtd validator = new Jtd(); + + for (JsonValue outOfRange : outOfRangeValues) { + Jtd.Result result = validator.validate(schema, outOfRange); + + assertThat(result.isValid()) + .as("uint32 should reject out-of-range value %s", outOfRange) + .isFalse(); + } + } + + /// Test that demonstrates the specific problem: JsonNumber.toNumber() returns Double + /// This test shows the root cause of the bug + @Test + public void testJsonNumberToNumberReturnsDouble() { + JsonValue numberValue = Json.parse("1000"); + + // Verify that JsonNumber.toNumber() returns Double for typical JSON numbers + assertThat(numberValue).isInstanceOf(JsonNumber.class); + Number number = ((JsonNumber) numberValue).toNumber(); + + LOG.info(() -> "JsonNumber.toNumber() returns: " + number.getClass().getSimpleName() + + " for value: " + numberValue); + + // This demonstrates what type JsonNumber.toNumber() returns for typical values + // The actual type depends on the JSON parser implementation + LOG.info(() -> "JsonNumber.toNumber() returns: " + number.getClass().getSimpleName() + + " for value: " + numberValue); + + // The key test is that regardless of the Number type, range validation should work + // Our fix ensures all Number types go through proper range validation + } + + /// Test integer validation with explicit Double creation + /// Shows the bug occurs even when we know the type is Double + @Test + public void testIntegerValidationExplicitDouble() { + JsonValue schema = Json.parse("{\"type\": \"int8\"}"); + + // Create a JsonNumber that definitely returns Double + JsonValue doubleValue = JsonNumber.of(1000.0); + + Jtd validator = new Jtd(); + Jtd.Result result = validator.validate(schema, doubleValue); + + LOG.fine(() -> "Explicit Double validation - value: " + doubleValue + + ", toNumber() type: " + ((JsonNumber)doubleValue).toNumber().getClass().getSimpleName()); + + // This should fail (1000 is way outside int8 range of -128 to 127) + assertThat(result.isValid()) + .as("int8 should reject Double value 1000.0 as out of range") + .isFalse(); + } + + /// Test that edge case boundary values work correctly + /// These test the exact boundaries where the bug becomes visible + @Test + public void testIntegerBoundaryValues() { + // Test int8 boundaries + JsonValue int8Schema = Json.parse("{\"type\": \"int8\"}"); + + // Just outside valid range + JsonValue justAboveMax = Json.parse("128"); // int8 max is 127 + JsonValue justBelowMin = Json.parse("-129"); // int8 min is -128 + + Jtd validator = new Jtd(); + + Jtd.Result aboveResult = validator.validate(int8Schema, justAboveMax); + assertThat(aboveResult.isValid()) + .as("int8 should reject 128 (just above max 127)") + .isFalse(); + + Jtd.Result belowResult = validator.validate(int8Schema, justBelowMin); + assertThat(belowResult.isValid()) + .as("int8 should reject -129 (just below min -128)") + .isFalse(); + } + + /// Test BigDecimal values that exceed long precision + /// Ensures precision loss is handled correctly + @Test + public void testIntegerValidationWithBigDecimalValues() { + JsonValue schema = Json.parse("{\"type\": \"uint32\"}"); + JsonValue bigValue = JsonNumber.of(new java.math.BigDecimal("5000000000")); // > uint32 max + + Jtd validator = new Jtd(); + Jtd.Result result = validator.validate(schema, bigValue); + + assertThat(result.isValid()) + .as("uint32 should reject BigDecimal value 5000000000 as out of range") + .isFalse(); + } + + /// Test mixed integer types in array to catch range validation issues + /// This creates a scenario where multiple integer validations occur + @Test + public void testMixedIntegerTypesInArray() { + JsonValue schema = Json.parse(""" + { + "elements": { + "discriminator": "type", + "mapping": { + "small": {"properties": {"value": {"type": "int8"}}}, + "medium": {"properties": {"value": {"type": "int16"}}}, + "large": {"properties": {"value": {"type": "int32"}}} + } + } + } + """); + + // Array with values that should fail their respective type validations + JsonValue invalidData = Json.parse(""" + [ + {"type": "small", "value": 1000}, + {"type": "medium", "value": 100000}, + {"type": "large", "value": 5000000000} + ] + """); + + Jtd validator = new Jtd(); + Jtd.Result result = validator.validate(schema, invalidData); + + LOG.fine(() -> "Mixed integer types test - should have multiple range errors"); + + // Should fail with multiple range validation errors + assertThat(result.isValid()) + .as("Should reject array with out-of-range values for different integer types") + .isFalse(); + assertThat(result.errors().size()) + .as("Should have multiple range validation errors") + .isGreaterThan(0); + } } diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/TestValidationErrors.java b/json-java21-jtd/src/test/java/json/java21/jtd/TestValidationErrors.java index 52194f5..3375787 100644 --- a/json-java21-jtd/src/test/java/json/java21/jtd/TestValidationErrors.java +++ b/json-java21-jtd/src/test/java/json/java21/jtd/TestValidationErrors.java @@ -148,7 +148,7 @@ public void testObjectSchemaErrorMessages() { Jtd.Result missingPropertyResult = validator.validate(objectSchema, Json.parse("{\"name\": \"John\"}")); assertThat(missingPropertyResult.isValid()).isFalse(); assertThat(missingPropertyResult.errors()).hasSize(1); - assertThat(missingPropertyResult.errors().getFirst()).contains("missing required property: age"); + assertThat(missingPropertyResult.errors().getFirst()).contains("missing required property: 'age'"); // Test invalid property value Jtd.Result invalidPropertyResult = validator.validate(objectSchema, Json.parse("{\"name\": 123, \"age\": 25}")); @@ -170,7 +170,7 @@ public void testAdditionalPropertiesErrorMessages() { Jtd.Result additionalPropResult = validator.validate(objectSchema, Json.parse("{\"name\": \"John\", \"extra\": \"not-allowed\"}")); assertThat(additionalPropResult.isValid()).isFalse(); assertThat(additionalPropResult.errors()).hasSize(1); - assertThat(additionalPropResult.errors().getFirst()).contains("additional property not allowed: extra"); + assertThat(additionalPropResult.errors().getFirst()).contains("additional property not allowed: 'extra'"); LOG.fine(() -> "Additional properties error messages test completed successfully"); } @@ -213,7 +213,7 @@ public void testUnknownTypeErrorMessage() { Jtd.Result result = validator.validate(unknownTypeSchema, Json.parse("\"anything\"")); assertThat(result.isValid()).isFalse(); assertThat(result.errors()).hasSize(1); - assertThat(result.errors().getFirst()).contains("unknown type: unknown-type"); + assertThat(result.errors().getFirst()).contains("unknown type: 'unknown-type'"); LOG.fine(() -> "Unknown type error message test completed successfully"); }