Skip to content

refactor(databind): add permissive document loading for non-validation contexts#600

Merged
david-waltermire merged 1 commit intometaschema-framework:developfrom
david-waltermire:feature/permissive-document-loading
Dec 29, 2025
Merged

refactor(databind): add permissive document loading for non-validation contexts#600
david-waltermire merged 1 commit intometaschema-framework:developfrom
david-waltermire:feature/permissive-document-loading

Conversation

@david-waltermire
Copy link
Contributor

@david-waltermire david-waltermire commented Dec 29, 2025

Summary

Refinements to PR #593 (parser validation for required fields):

  • Add newPermissiveBoundLoader() to IBindingContext for creating loaders with DESERIALIZE_VALIDATE_REQUIRED_FIELDS disabled
  • Update constraint validation to use permissive loaders for referenced documents
  • Update CLI convert command to use permissive loading
  • Update CLI metapath eval command to use permissive loading and set document loader on DynamicContext to enable doc() function support
  • Add tests for permissive loading behavior

This also includes a fix for the plural form bug ("propertys" → "properties") in validation error messages.

Test plan

  • All 179 databind tests pass
  • All 30 CLI tests pass
  • New PermissiveLoadingTest tests verify:
    • Permissive loader skips required field validation
    • Strict loader validates required fields
    • Permissive loader allows partial documents
    • newPermissiveBoundLoader() has validation disabled
    • newBoundLoader() has validation enabled by default

Summary by CodeRabbit

  • New Features

    • Permissive document loading enabled across conversion, evaluation, and validation flows so operations can proceed on incomplete documents without failing required-field checks.
  • Bug Fixes

    • Improved error message grammar to correctly choose singular/plural property type names and group labels for clearer, more accurate messages.
  • Tests

    • Added tests verifying permissive vs. strict loading behaviors and validation outcomes for partial documents.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

Adds a permissive bound loader to IBindingContext and switches callers to use it for loading referenced or converted documents; disables required-field and constraint validation during permissive loads; adds plural-aware property type naming in error messages; tests and CLI usages updated to use permissive loading.

Changes

Cohort / File(s) Summary
Binding context API
databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java
Adds default IBoundLoader newPermissiveBoundLoader() to produce loaders with DESERIALIZE_VALIDATE_REQUIRED_FIELDS disabled.
Validation & deserialization
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java, databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
Uses permissive loader when building DynamicContext for constraint evaluation; introduces plural-aware getFormatPropertyTypeName(..., boolean plural) and propagates plural handling through getPropertyTypeName() and message construction.
Validation call sites
databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java (callers: newValidator(...), validateWithConstraints(...))
Replaces direct newBoundLoader() usages with newPermissiveBoundLoader() and disables DESERIALIZE_VALIDATE_CONSTRAINTS on permissive loads used for referenced documents.
CLI command integrations
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java, metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java
Use permissive bound loader for conversion and Metapath evaluation; propagate loader into DynamicContext for doc()/module support.
Tests
databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
Adds PermissiveLoadingTest cases validating permissive vs. strict loader behaviors and DESERIALIZE_VALIDATE_REQUIRED_FIELDS default states.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI / Client
    participant BindCtx as IBindingContext
    participant Loader as IBoundLoader (permissive)
    participant Deser as Deserializer
    participant DynCtx as DynamicContext
    participant Evaluator as Constraint/Metapath Evaluator

    CLI->>BindCtx: request convert / validate / evaluate
    BindCtx->>Loader: newPermissiveBoundLoader()
    Note right of Loader `#f6f0e6`: DESERIALIZE_VALIDATE_REQUIRED_FIELDS=false
    CLI->>Deser: provide document
    Deser->>Loader: load(document)
    Note right of Deser `#e8f6ef`: permissive load — skip required-field checks
    Loader-->>Deser: partial/loaded model
    Deser->>DynCtx: build DynamicContext (attach loader)
    DynCtx->>Evaluator: evaluate constraints / metapath (may call doc())
    Evaluator->>Loader: load referenced documents permissively
    Evaluator-->>DynCtx: results
    DynCtx-->>CLI: final result / validation outcome
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐇 I nibble code where loaders sleep,

Permissive hops let partials creep,
Messages now plural sing with grace,
Constraints still check their proper place,
A rabbit's cheer for gentler loads — hip hip peep!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: introducing permissive document loading for non-validation contexts, which is the primary focus of this PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46488f7 and b660af1.

📒 Files selected for processing (6)
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.java: All code changes must follow the Javadoc style guide (docs/javadoc-style-guide.md). New code requires 100% Javadoc coverage on public/protected members. Modified code must add/update Javadoc on any members touched. All Javadoc must include @param, @return, @throws tags in the correct order (BLOCKING)
Java target version must be Java 11. Use SpotBugs annotations (@nonnull, @nullable) for null safety in code.
Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages
Follow Test-Driven Development (TDD) principles: write tests first before implementing functionality, verify tests fail with current implementation, implement minimal code to pass tests, then refactor while keeping tests green

Files:

  • databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java
  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
**/*Test.java

📄 CodeRabbit inference engine (CLAUDE.md)

@test methods do not require Javadoc if they use descriptive method names. Use JUnit 5 for testing with parallel execution enabled.

Files:

  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
🧠 Learnings (5)
📚 Learning: 2025-12-19T04:01:37.408Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 550
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/format/JsonPointerFormatter.java:56-100
Timestamp: 2025-12-19T04:01:37.408Z
Learning: When overriding Java interface methods, rely on inherited Javadoc from the interface. Do not duplicate documentation in the implementing class unless there is implementation-specific behavior that warrants additional notes beyond the interface contract.

Applied to files:

  • databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java
  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
📚 Learning: 2025-12-27T16:52:04.509Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 590
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java:482-492
Timestamp: 2025-12-27T16:52:04.509Z
Learning: In Java, UncheckedIOException.getCause() is declared to return IOException. In methods that declare throws IOException, you can rethrow the underlying cause with throw e.getCause() where e is an UncheckedIOException, without a cast. Ensure the surrounding method signature includes throws IOException. This does not apply to other unchecked exceptions; verify that e is actually an UncheckedIOException before using this pattern.

Applied to files:

  • databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java
  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.

Applied to files:

  • databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java
  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
📚 Learning: 2025-12-24T21:22:07.082Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:22:07.082Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.

Applied to files:

  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
📚 Learning: 2025-12-24T21:21:56.361Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:56.361Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.

Applied to files:

  • databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java
🔇 Additional comments (8)
databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java (3)

414-433: LGTM! Well-documented permissive loader API.

The new newPermissiveBoundLoader() method is well-designed with comprehensive Javadoc that clearly explains its purpose for use with Metapath functions where documents may be incomplete. The implementation correctly disables DESERIALIZE_VALIDATE_REQUIRED_FIELDS to achieve permissive loading behavior.


469-472: LGTM! Appropriate use of permissive loading in validation context.

The switch to newPermissiveBoundLoader() for loading referenced documents during constraint validation is correct. The comments clearly explain that permissive loading is needed for referenced documents, and disabling constraint validation during loading prevents redundant validation.


594-597: LGTM! Consistent permissive loading approach.

The use of newPermissiveBoundLoader() for both target and referenced documents is appropriate here, and the approach is consistent with the changes in newValidator(). The comments effectively document the rationale.

databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java (1)

150-151: LGTM! Correctly applies permissive loading for constraint validation.

The use of newPermissiveBoundLoader() when setting the document loader for constraint expression validation is appropriate. This ensures that documents referenced during constraint validation (e.g., via Metapath expressions) can be loaded without strict required-field validation, which is consistent with the broader permissive loading strategy introduced in this PR.

databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java (1)

464-547: LGTM! Comprehensive test coverage for permissive loading.

The new PermissiveLoadingTest nested class provides thorough coverage of the permissive loading feature:

  • Tests both permissive and strict loading behaviors
  • Verifies that newPermissiveBoundLoader() has validation disabled
  • Verifies that newBoundLoader() has validation enabled by default
  • Tests partial document loading scenarios

The test method names are descriptive and follow JUnit 5 best practices. Per the coding guidelines, @test methods do not require Javadoc when using descriptive names, which is the case here.

databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java (3)

271-289: LGTM! Plural-aware property type naming.

The addition of the plural parameter to getFormatPropertyTypeName() properly addresses the plural form bug mentioned in the PR description. The implementation correctly returns:

  • "attribute"/"attributes" for XML flags
  • "element"/"elements" for XML fields/assemblies
  • "property"/"properties" for JSON/YAML

The Javadoc clearly documents the new parameter and its effect on the returned values.


306-309: LGTM! Good refactoring to eliminate duplication.

The refactoring of getFormatPropertyGroupLabel() to delegate to getFormatPropertyTypeName(..., true) and capitalize the result eliminates hardcoded strings and follows the DRY principle. This ensures consistent formatting when new formats are added in the future.


214-224: LGTM! Correct plural handling in error messages.

The updates to getPropertyTypeName() and its usage in formatMissingPropertiesMessage() correctly apply the plural parameter:

  • Line 214: Uses false for singular form when reporting a single missing property
  • Line 222: Uses true for plural form when reporting multiple properties of the same type
  • Line 224: Correctly formats the message with the plural type name

This ensures grammatically correct error messages like "Missing required properties" instead of "Missing required propertys".

Also applies to: 346-352

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PMD (7.19.0)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java

[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java

[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java

[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

  • 3 others

Comment @coderabbitai help to get the list of available commands and usage tips.

@david-waltermire david-waltermire added this to the v3.0.0 Milestone 2 milestone Dec 29, 2025
…sages

Refinements to PR metaschema-framework#593 (parser validation for required fields):

Permissive Loading:
- Add newPermissiveBoundLoader() to IBindingContext for creating loaders
  with DESERIALIZE_VALIDATE_REQUIRED_FIELDS disabled
- Update constraint validation to use permissive loaders for referenced
  documents (enables doc() function during constraint evaluation)
- Update CLI convert command to use permissive loading
- Update CLI metapath eval command to use permissive loading and set
  document loader on DynamicContext to enable doc() function support
- Add tests for permissive loading behavior

Error Message Fixes:
- Add plural parameter to getFormatPropertyTypeName() and getPropertyTypeName()
- Fix plural forms: 'properties' instead of 'propertys' for JSON/YAML
- Consolidate getFormatPropertyGroupLabel() to delegate to
  getFormatPropertyTypeName() and capitalize, reducing code duplication
@david-waltermire david-waltermire force-pushed the feature/permissive-document-loading branch from 46488f7 to b660af1 Compare December 29, 2025 02:51
@david-waltermire david-waltermire merged commit d60c916 into metaschema-framework:develop Dec 29, 2025
4 checks passed
@david-waltermire david-waltermire deleted the feature/permissive-document-loading branch December 29, 2025 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant