|
| 1 | +# Discriminator Validation Refactor Plan |
| 2 | + |
| 3 | +## Objectives |
| 4 | +- Document the intended single-path validation architecture and discriminator rules before touching implementation. |
| 5 | +- Additive-first: introduce tests and supporting guards ahead of removing legacy recursive validation. |
| 6 | +- Enforce RFC 8927 discriminator constraints at compilation while preserving stack-based runtime checks with the discriminator tag exemption. |
| 7 | +- Retire duplicate validation paths only after documentation and tests clearly describe the target behavior. |
| 8 | + |
| 9 | +## Sequenced Work Breakdown |
| 10 | + |
| 11 | +1. **Documentation First** ✅ |
| 12 | + - Expand `json-java21-jtd/ARCHITECTURE.md` with explicit guidance on single stack-based validation, discriminator tag exemption, and compile-time schema constraints. |
| 13 | + - Audit other affected docs (e.g., module README, AGENTS addenda) to ensure contributors receive clear instructions prior to code edits. |
| 14 | + |
| 15 | +2. **Reconnaissance & Impact Analysis** ✅ |
| 16 | + - Inventory all usages of `JtdSchema.*#validate` and catalogue callers/tests that rely on the recursive path. |
| 17 | + - Trace how `pushChildFrames` and `PropertiesSchema` cooperate today to support discriminator handling and identify touchpoints for additive changes. |
| 18 | + |
| 19 | +3. **Test Design (Additive Stage)** ✅ |
| 20 | + - Keep `JtdSpecIT` focused solely on `validation.json` scenarios so we only assert that published docs validate as expected. |
| 21 | + - Add `CompilerSpecIT` to execute `invalid_schemas.json`, asserting compilation failures with deterministic messages instead of skipping them. |
| 22 | + - 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). |
| 23 | + |
| 24 | +4. **Compile-Time Guards (Additive)** ✅ |
| 25 | + - Introduce validation during schema compilation to reject non-`PropertiesSchema` discriminator mappings, nullable mappings, and mappings that shadow the discriminator key. |
| 26 | + - Emit deterministic exception messages with precise schema paths to keep property-based tests stable. |
| 27 | + |
| 28 | +5. **Stack Engine Enhancements (Additive)** |
| 29 | + - Teach the stack-based validator to propagate an "exempt key" for discriminator payload evaluation before deleting any recursive logic. |
| 30 | + - Adjust `PropertiesSchema` handling to skip the exempt key when checking required/optional members and additionalProperties while preserving crumb/schemaPath reporting. |
| 31 | + |
| 32 | +6. **Retire Recursive `validate` Path (Removal Stage)** |
| 33 | + - After new tests pass with the enhanced stack engine, remove the per-schema `validate` implementations and their call sites. |
| 34 | + - Clean up dead code, imports, and update any remaining references discovered by static analysis. |
| 35 | + |
| 36 | +7. **Verification & Cleanup** |
| 37 | + - Run targeted tests with mandated JUL logging levels (FINE/FINEST for new cases) followed by the full suite at INFO to confirm conformity. |
| 38 | + - Revisit documentation to confirm implemented behavior matches the authored guidance and adjust if gaps remain. |
| 39 | + |
| 40 | +## Risks & Mitigations |
| 41 | +- **Hidden Recursive Callers**: Use `rg "\.validate\("` and compiler errors to surface stragglers before deleting the recursive path. |
| 42 | +- **Error Message Drift**: Lock expected `schemaPath`/`instancePath` outputs in tests once new guards land to prevent flakiness. |
| 43 | +- **Temporal Test Failures**: Stage code changes so new tests are introduced alongside the supporting implementation to avoid committing failing tests. |
| 44 | +- **Documentation Drift**: Re-review docs post-implementation to ensure instructions still match code. |
| 45 | + |
| 46 | +## Out of Scope |
| 47 | +- Remote reference compilation or runtime changes (handled by MVF initiative). |
| 48 | +- Adjustments to global logging frameworks beyond what is necessary for new tests. |
| 49 | +- Broader API redesigns outside discriminator handling and validation-path consolidation. |
| 50 | + |
| 51 | +## Documentation Targets ✅ |
| 52 | +- json-java21-jtd/ARCHITECTURE.md: add single-path validation, discriminator tag exemption, compile-time guard details. |
| 53 | +- README.md (module-level if present): summarize discriminator constraints and reference architecture section. |
| 54 | +- AGENTS.md references (if needed): reinforce documentation-before-code steps for discriminator work. |
| 55 | + |
| 56 | +## Test Targets |
| 57 | +- New compile-time rejection test suite ensuring discriminator mapping violations throw with deterministic schema paths. |
| 58 | +- Runtime tests for RFC 8927 §3.3.8 scenarios (missing tag, non-string tag, unknown mapping, payload mismatch, success). |
| 59 | +- Regression coverage ensuring tag exemption prevents additionalProperties violations when payload omits discriminator. |
| 60 | +- Removal/update of tests invoking `JtdSchema.validate` only after stack engine passes new scenarios. |
0 commit comments