Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
for k in totals: totals[k]+=int(r.get(k,'0'))
except Exception:
pass
exp_tests=475
exp_tests=498
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}")
Expand Down
30 changes: 30 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
@@ -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.
46 changes: 38 additions & 8 deletions json-java21-jtd/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions json-java21-jtd/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`:
Expand Down
Loading