Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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=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}")
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