Skip to content

refactor: remove Saxon-HE, JDOM2, and jaxen dependencies from schemagen#607

Merged
david-waltermire merged 7 commits intometaschema-framework:developfrom
david-waltermire:refactor/remove-saxon-jdom2
Dec 31, 2025
Merged

refactor: remove Saxon-HE, JDOM2, and jaxen dependencies from schemagen#607
david-waltermire merged 7 commits intometaschema-framework:developfrom
david-waltermire:refactor/remove-saxon-jdom2

Conversation

@david-waltermire
Copy link
Contributor

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

Summary

  • Replace Saxon-HE XSLT post-processing with IndentingXMLStreamWriter for XML output formatting
  • Replace JDOM2-based XML processing with standard Java DOM/XPath APIs
  • Remove three heavy dependencies from the schemagen module

Changes

New Classes

  • IndentingXMLStreamWriter - XMLStreamWriter wrapper that adds proper indentation to XML output
  • IndentingXMLStreamWriter2 - Extension that implements XMLStreamWriter2 interface
  • XmlSchemaLoader - Loads XML schemas using standard DOM/XPath APIs
  • DomDatatypeContent - Writes DOM elements to XMLStreamWriter

Removed Classes

  • JDom2DatatypeContent - Replaced by DomDatatypeContent
  • JDom2XmlSchemaLoader - Replaced by XmlSchemaLoader

Removed Files

  • identity.xsl - No longer needed; indentation now handled by IndentingXMLStreamWriter

Dependencies Removed

  • net.sf.saxon:Saxon-HE
  • org.jdom:jdom2
  • jaxen:jaxen

Test Plan

  • All existing schemagen tests pass (37 tests)
  • New unit tests for IndentingXMLStreamWriter (22 tests)
  • New unit tests for XmlSchemaLoader (7 tests)
  • New unit tests for DomDatatypeContent (8 tests)
  • Full build passes: mvn clean install -PCI -Prelease
  • Saxon-HE no longer appears in automodule warnings

Benefits

  1. ~30% reduction in overall dependency size - Removing Saxon-HE, JDOM2, and jaxen significantly reduces the dependency footprint
  2. Eliminates automodule warnings for Saxon-HE
  3. Uses standard Java APIs (DOM, XPath) instead of third-party libraries
  4. Better maintainability with fewer external dependencies

Summary by CodeRabbit

  • Breaking Changes

    • Public XML APIs now use standard DOM element types and the previous transformation-based generation path was removed — integrations may need updating.
  • Improvements

    • Replaced several external XML libraries with standard Java XML APIs.
    • Added context-aware pretty-print indentation for generated schemas.
    • Reduced transitive dependency footprint and cleaned build configuration.
  • Tests

    • Added comprehensive tests for DOM/XPath loading and indentation; updated existing test suites.
  • Documentation

    • Added PRD and implementation plan describing the migration and rollout.

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

Add Product Requirements Document and implementation plan for removing
Saxon-HE, xmlresolver, JDOM2, and jaxen dependencies from the schemagen
module by replacing them with standard Java XML APIs.

Key design decisions documented:
- IndentingXMLStreamWriter with stack-based mixed content detection
- Standard DOM/XPath APIs for XSD loading
- Transformer-based DOM element serialization
- TDD methodology with characterization tests first
Replace heavy XML processing dependencies with standard Java APIs:

- Add IndentingXMLStreamWriter for XML output formatting, replacing
  Saxon XSLT post-processing with identity.xsl
- Add XmlSchemaLoader using standard DOM/XPath APIs to replace
  JDom2XmlSchemaLoader
- Add DomDatatypeContent to write DOM elements to XMLStreamWriter,
  replacing JDom2DatatypeContent
- Update XmlSchemaGenerator to wrap writer with IndentingXMLStreamWriter2
- Update all datatype providers to use standard DOM APIs
- Update XmlSuiteTest to use standard DOM/XPath

Dependencies removed from schemagen module:
- net.sf.saxon:Saxon-HE
- org.jdom:jdom2
- jaxen:jaxen

This reduces the dependency footprint and eliminates automodule warnings
for Saxon-HE in the build output.
@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

Removes Saxon-HE/JDOM2/jaxen/xmlresolver from schemagen, replaces XML processing with standard Java APIs (DOM, XPath, StAX), adds IndentingXMLStreamWriter (+2), XmlSchemaLoader, DomDatatypeContent, updates providers and XmlSchemaGenerator, removes XSLT post-processing, and updates build/module and tests.

Changes

Cohort / File(s) Summary
Planning & Docs
PRDs/20251231-saxon-jdom2-removal/PRD.md, PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
New PRD and implementation plan describing goals, phased rollout, TDD workflow, technical requirements, risks, design decisions, and rollout phases for removing Saxon/JDOM2 and migrating to standard Java XML APIs.
Indenting Writers
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java, schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
New XMLStreamWriter wrappers that track depth and mixed-content to emit pretty-print indentation; IndentingXMLStreamWriter2 extends and delegates XMLStreamWriter2-specific methods.
DOM Schema Loader
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
New namespace-aware DOM + XPath loader with constructors for Path, InputStream, Document and getContent(String, Map<String,String>) returning List<org.w3c.dom.Element>.
DOM Datatype Content
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
New DOM-backed datatype content storing List<Element> and serializing elements (namespaces, attributes, text/CDATA/comments) to an XMLStreamWriter.
Removed JDOM2 Implementations
schemagen/src/main/java/.../JDom2XmlSchemaLoader.java, schemagen/src/main/java/.../JDom2DatatypeContent.java
Deleted legacy JDOM2-based loader and datatype content classes and associated constants/constructors/methods.
Provider API Updates
schemagen/src/main/java/.../AbstractXmlDatatypeProvider.java, .../AbstractXmlMarkupDatatypeProvider.java, .../XmlCoreDatatypeProvider.java, .../XmlProseBaseDatatypeProvider.java
Switched loader parameter types to XmlSchemaLoader, replaced org.jdom2.Element with org.w3c.dom.Element, adjusted XPath/exception handling, and replaced JDom2DatatypeContent usages with DomDatatypeContent.
Schema Generator
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
Removed XSLT-based post-processing and generateFromModule(...Writer...); now constructs IndentingXMLStreamWriter2 and writes directly, removing identity XSLT transform path and related imports.
Module & Build
schemagen/pom.xml, top-level pom.xml, schemagen/src/main/java/module-info.java
Removed properties and dependencies for jdom2, jaxen, Saxon-HE, xmlresolver; added/adjusted org.codehaus.stax2 module usage; updated dependency management and THIRD_PARTY_LICENSES.md.
Removed XSLT Resource
schemagen/src/main/resources/identity.xsl
Deleted identity XSLT stylesheet previously used for transformation-based formatting.
Tests
schemagen/src/test/java/.../XmlSuiteTest.java, .../IndentingXMLStreamWriterTest.java, .../XmlSchemaLoaderTest.java, .../DomDatatypeContentTest.java, .../AbstractSchemaGeneratorTestSuite.java
Updated tests to DOM/XPath parsing and new exception sets; added comprehensive tests for Indenting writer, XmlSchemaLoader, DomDatatypeContent serialization; small assertion cleanup in JSON schema test.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Gen as XmlSchemaGenerator
    participant Loader as XmlSchemaLoader
    participant DOM as DOM Document
    participant Provider as DatatypeProvider
    participant Writer as IndentingXMLStreamWriter2
    participant Out as Output Writer

    Gen->>Loader: new(resource Path/InputStream)
    Loader->>DOM: parse & normalize
    DOM-->>Loader: Document

    Gen->>Loader: getContent(xpath, nsMap)
    Loader->>Loader: evaluate XPath -> NodeList -> List<Element>
    Loader-->>Gen: List<Element>

    Gen->>Provider: queryElements(loader)
    Provider-->>Gen: element lists

    Gen->>Writer: new IndentingXMLStreamWriter2(delegate)
    Gen->>Writer: write schema elements
    loop per element
        Writer->>Out: writeStartElement (may emit indent)
        Writer->>Out: writeAttributes / namespaces
        Writer->>Out: writeCharacters / writeCData / writeComment
        Writer->>Out: writeEndElement (may emit indent)
    end
    Writer-->>Out: Final indented XML output
Loading
sequenceDiagram
    autonumber
    participant Provider as DatatypeProvider
    participant Loader as XmlSchemaLoader
    participant Content as DomDatatypeContent
    participant Writer as XMLStreamWriter

    Provider->>Loader: getContent(xpath, nsMap)
    Loader-->>Provider: List<Element>

    Provider->>Content: new DomDatatypeContent(name,elements,deps)
    Content->>Writer: write(Element) [recursive: start, attrs, children, end]
    Writer-->>Provider: emitted XML fragments
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • aj-stein-gsa
  • wandmagic

Poem

🐰 I hopped through nodes both old and new,
Farewelled Saxon, JDOM too,
DOM and StAX now guide my trail,
Indents tidy, no more XSLT sail,
Schemas sing in cleaner view.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.85% 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 primary change: removing Saxon-HE, JDOM2, and jaxen dependencies from the schemagen module. It directly reflects the main refactoring objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.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.


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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (9)
PRDs/20251231-saxon-jdom2-removal/PRD.md (2)

30-36: Performance benefit claim needs substantiation.

Line 35 asserts that streaming indentation is more efficient than buffered XSLT post-processing, but provides no benchmark data or rationale. While this is likely true, either remove the claim or provide supporting evidence (e.g., memory usage comparison, throughput measurements).


183-201: Specify test framework and coverage metrics for verification.

The Existing Test Coverage section lists required tests but does not specify:

  1. Test framework: Will new tests use JUnit 4, JUnit 5, or another framework? Should they be consistent with existing schemagen tests?
  2. Coverage metrics: How will "100% test coverage for public methods" (line 75) be verified? Will a tool like JaCoCo be configured? What is the minimum acceptable branch coverage?
  3. Integration scope: Will the schemagen integration tests be run against all sample metaschema files to ensure end-to-end equivalence?

These details will guide implementation and help define success criteria more precisely.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)

90-118: Consider extracting common DocumentBuilder configuration.

The parseDocument(Path) and parseDocument(InputStream) methods duplicate the DocumentBuilderFactory configuration logic. Consider extracting a common helper.

🔎 Proposed refactor
+ @NonNull
+ private static DocumentBuilder createDocumentBuilder() {
+   try {
+     DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+     factory.setNamespaceAware(true);
+     return factory.newDocumentBuilder();
+   } catch (ParserConfigurationException ex) {
+     throw new IllegalStateException("Failed to create document builder", ex);
+   }
+ }
+
  @NonNull
  private static Document parseDocument(@NonNull Path path) throws SAXException, IOException {
-   try {
-     DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-     factory.setNamespaceAware(true);
-     DocumentBuilder builder = factory.newDocumentBuilder();
-     Document doc = builder.parse(path.toFile());
-     // Normalize to ensure deferred DOM nodes are fully loaded for XPath evaluation
-     doc.normalizeDocument();
-     return doc;
-   } catch (ParserConfigurationException ex) {
-     throw new IllegalStateException("Failed to create document builder", ex);
-   }
+   Document doc = createDocumentBuilder().parse(path.toFile());
+   doc.normalizeDocument();
+   return doc;
  }

  @NonNull
  private static Document parseDocument(@NonNull InputStream is) throws SAXException, IOException {
-   try {
-     DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-     factory.setNamespaceAware(true);
-     DocumentBuilder builder = factory.newDocumentBuilder();
-     Document doc = builder.parse(is);
-     // Normalize to ensure deferred DOM nodes are fully loaded for XPath evaluation
-     doc.normalizeDocument();
-     return doc;
-   } catch (ParserConfigurationException ex) {
-     throw new IllegalStateException("Failed to create document builder", ex);
-   }
+   Document doc = createDocumentBuilder().parse(is);
+   doc.normalizeDocument();
+   return doc;
  }
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java (2)

81-84: Prefer assertTrue() over assertEquals(true, ...).

Using assertTrue() is more idiomatic and provides clearer failure messages.

🔎 Proposed fix
-      assertEquals(true, typeNames.contains("Base64Datatype"),
-          "Should contain Base64Datatype");
-      assertEquals(true, typeNames.contains("StringDatatype"),
-          "Should contain StringDatatype");
+      assertTrue(typeNames.contains("Base64Datatype"),
+          "Should contain Base64Datatype");
+      assertTrue(typeNames.contains("StringDatatype"),
+          "Should contain StringDatatype");

111-112: Same suggestion: prefer assertTrue().

🔎 Proposed fix
-      assertEquals(true, elementNames.contains("simpleType"),
-          "Should contain simpleType elements");
+      assertTrue(elementNames.contains("simpleType"),
+          "Should contain simpleType elements");
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java (1)

151-152: Prefer assertTrue() over assertEquals(true, ...).

Same pattern as noted in XmlSchemaLoaderTest.

🔎 Proposed fix
-      assertEquals(true, result.contains("<xs:schema"));
-      assertEquals(true, result.contains("<xs:element"));
+      assertTrue(result.contains("<xs:schema"));
+      assertTrue(result.contains("<xs:element"));

Apply similarly to lines 265, 313, 405, 428, and 541.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java (1)

52-71: Functional dependency analysis with standard XPath.

The XPath expression .//@base correctly finds all base attributes for restriction/extension type references. The deduplication logic works correctly for the expected small number of dependencies.

Minor optimization opportunity: the XPath instance could be cached as a static field since it's thread-safe after configuration, but this is not critical for schema generation which isn't a hot path.

schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java (1)

186-204: Functional NamespaceContext for test use.

The anonymous NamespaceContext implementation is sufficient for this test's XPath evaluation. Note that getPrefix() and getPrefixes() returning null technically violates the interface contract (should return null and Iterator.empty() respectively), but this works in practice since JDK's XPath only calls getNamespaceURI() during evaluation.

Consider using the MapNamespaceContext pattern from XmlSchemaLoader for consistency across the codebase, but this is optional for test code.

PRDs/20251231-saxon-jdom2-removal/implementation-plan.md (1)

346-355: Note: Implementation differs from design notes (intentionally).

The design notes at lines 348-349 describe checking for whitespace-only text before setting hasText = true. The actual implementation always sets hasText = true for any text content. This is a conservative choice that better preserves whitespace in mixed content scenarios and is validated by the passing tests.

Consider updating the design notes to reflect the actual implementation for documentation accuracy.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a5cfa4 and 19f2f56.

📒 Files selected for processing (20)
  • PRDs/20251231-saxon-jdom2-removal/PRD.md
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
  • schemagen/pom.xml
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2DatatypeContent.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2XmlSchemaLoader.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/resources/identity.xsl
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
💤 Files with no reviewable changes (4)
  • schemagen/src/main/resources/identity.xsl
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2DatatypeContent.java
  • schemagen/pom.xml
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2XmlSchemaLoader.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
📚 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:

  • PRDs/20251231-saxon-jdom2-removal/PRD.md
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
📚 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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
📚 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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.java
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/metapath/**/*.java : Metapath is an implementation of XPath 3.1. Use the XPath 3.1 specification (https://www.w3.org/TR/xpath-31/) and XPath Functions 3.1 (https://www.w3.org/TR/xpath-functions-31/) as authoritative reference when implementing new functions, fixing bugs, or understanding error handling. Raise clarification before making changes if implementation differs from spec.

Applied to files:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
📚 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:

  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.java : Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages

Applied to files:

  • schemagen/src/main/java/module-info.java
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/

Applied to files:

  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • PRDs/20251231-saxon-jdom2-removal/implementation-plan.md
📚 Learning: 2025-12-30T04:19:41.802Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 605
File: databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/typeinfo/DefaultMetaschemaClassFactory.java:480-484
Timestamp: 2025-12-30T04:19:41.802Z
Learning: In metaschema-framework/metaschema-java, when users configure custom superinterfaces via binding configuration, david-waltermire prefers both documentation (explaining how to add dependencies to the Maven plugin) and validation mechanisms (warnings when classes aren't on classpath) to prevent redundant IBoundObject interface additions during code generation.

Applied to files:

  • schemagen/src/main/java/module-info.java
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.{xmlbeans,antlr} : Generated code in *.xmlbeans and *.antlr packages is excluded from Javadoc and style checks. Generated sources are placed in target/generated-sources/

Applied to files:

  • schemagen/src/main/java/module-info.java
📚 Learning: 2024-11-14T18:19:40.200Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java:86-104
Timestamp: 2024-11-14T18:19:40.200Z
Learning: In the file `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IBooleanItem.java`, the 3-step approach in the `cast` method is consistent with the XPath 3.1 specification.

Applied to files:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java
🧬 Code graph analysis (4)
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java (2)
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java (2)
  • Nested (55-104)
  • Nested (106-253)
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java (3)
  • Nested (55-114)
  • Nested (116-157)
  • Nested (159-184)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)
  • XmlSchemaLoader (41-206)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java (2)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)
  • XmlSchemaLoader (41-206)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
  • ObjectUtils (18-135)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.java (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)
  • XmlSchemaLoader (41-206)
🪛 markdownlint-cli2 (0.18.1)
PRDs/20251231-saxon-jdom2-removal/implementation-plan.md

125-125: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


139-139: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


146-146: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


182-182: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


191-191: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


198-198: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Website
  • GitHub Check: Code
🔇 Additional comments (30)
PRDs/20251231-saxon-jdom2-removal/PRD.md (3)

80-95: Clarify mixed content detection edge cases before implementation.

The table correctly identifies text productions requiring testing, but the stack-based hasText detection strategy (referenced in lines 156–159) leaves critical edge cases undefined:

  1. Whitespace-only text nodes: Should writeCharacters("\n ") (formatting between elements) set hasText = true? If yes, this could suppress indentation unnecessarily.
  2. CDATA sections: Should CDATA trigger hasText the same way as text content?
  3. Comments and processing instructions: These are not text content but may appear between elements; should they affect indentation?

Document the expected behavior for each case before implementation to prevent whitespace corruption.


152-164: Define mixed content detection behavior for whitespace-only text nodes.

The mixed content detection strategy using hasText flag is sound in principle, but the implementation behavior is ambiguous for a critical edge case:

Question: When writeCharacters(text) is called, how should the code determine if text is:

  • Formatting whitespace (e.g., "\n " before a child element) → should not set hasText = true (allow indentation)
  • Content whitespace (e.g., " " between inline elements) → should set hasText = true (suppress indentation)

The current description does not specify how to distinguish between these cases. Suggest adding a helper method definition or algorithm before implementation (e.g., "only non-whitespace or whitespace containing non-space/tab characters triggers hasText").


4-14: Add design section for DomDatatypeContent serialization strategy.

Line 68 mentions testing DOM element serialization to XMLStreamWriter, but the PRD provides no design guidance on implementation strategy. Key questions are unaddressed:

  1. Deep traversal vs. shallow copy: Will the implementation recursively traverse DOM nodes (like the original StAXStreamOutputter) or copy the subtree?
  2. Namespace handling: How will xmlns declarations be preserved when copying elements? What about namespace prefix remapping?
  3. Special node types: How will CDATA sections, comments, processing instructions, and entity references be handled?
  4. Attributes: Will all attributes be copied? How are QNames handled if they reference external namespaces?

Either add a design section with a clear strategy before implementation, or explicitly document that these details will be discovered during TDD (lines 57–61). This will prevent implementation surprises and ensure test-first correctness.

Also applies to: 68-68

schemagen/src/main/java/module-info.java (1)

22-22: LGTM!

The addition of org.codehaus.stax2 correctly replaces the removed Saxon-HE and JDOM2 dependencies, providing the XMLStreamWriter2 interface needed by the new IndentingXMLStreamWriter2 implementation.

schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.java (1)

33-253: LGTM!

The test class is well-organized with clear @Nested groupings and descriptive @DisplayName annotations. Tests cover key scenarios including:

  • Basic property accessors (type name, dependencies)
  • Edge cases (empty dependencies, empty element list)
  • XML writing functionality with various content types

The helper methods createDocument() and createSchemaElement() effectively reduce test boilerplate.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java (1)

119-121: LGTM!

The refactored newWriter() method cleanly wraps the base XMLStreamWriter2 with IndentingXMLStreamWriter2, replacing the previous XSLT-based post-processing approach. This simplifies the generation pipeline while achieving the same indentation formatting goal with reduced dependencies.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.java (2)

47-62: LGTM!

The initSchema() method correctly migrates from JDOM2 to standard DOM APIs:

  • Uses XmlSchemaLoader for namespace-aware DOM parsing
  • Exception handling properly catches SAXException | IOException matching the DOM parsing contract
  • Lazy initialization pattern is preserved with proper synchronization

71-72: LGTM!

The abstract method signature update to List<Element> queryElements(XmlSchemaLoader loader) correctly reflects the migration from JDOM2 to standard DOM Element type.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java (2)

46-51: LGTM!

The queryElements implementation correctly uses the new XmlSchemaLoader API with the proper namespace mapping via XmlSchemaLoader.NS_XML_SCHEMA.


62-72: LGTM!

The handleResults method correctly:

  • Filters out xs:include elements using DOM's getLocalName()
  • Constructs DomDatatypeContent as the replacement for the removed JDom2DatatypeContent
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (2)

141-161: LGTM!

The getContent method correctly:

  • Creates a new XPath instance with the provided namespace context
  • Evaluates the expression and collects only Element nodes
  • Handles invalid XPath expressions by throwing IllegalArgumentException with a clear message

166-205: LGTM!

The MapNamespaceContext inner class correctly implements the NamespaceContext interface:

  • Validates null inputs as required by the contract
  • Returns XMLConstants.NULL_NS_URI for unknown prefixes (correct per spec)
  • Implements getPrefix and getPrefixes for reverse lookups
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java (1)

42-53: LGTM!

Using @BeforeAll to load schemas once is efficient for read-only test scenarios. The assertions verify resource availability before loading.

schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java (1)

27-565: LGTM!

Excellent test coverage for IndentingXMLStreamWriter with comprehensive scenarios:

  • Element structure and nesting at various depths
  • Critical text content preservation tests ensuring no whitespace corruption in mixed content
  • CDATA, comments, and processing instruction handling
  • Schema documentation with XHTML content
  • Edge cases with interleaved text and elements

The tests verify the key invariant that mixed content text is preserved exactly.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.java (4)

17-52: Well-designed streaming indentation wrapper.

The class Javadoc and constructor documentation are complete. The stack-based hasTextStack design correctly preserves parent mixed-content state across nested elements, which is essential for proper XHTML handling in xs:documentation.


137-148: Correct end-element indentation logic.

The combination of !hasText && !lastWasStart correctly handles all cases:

  • Mixed content (hasText): no indent to preserve text formatting
  • Empty elements (lastWasStart): no double newline for <elem></elem>
  • Normal case: proper indent before closing tag

110-135: Empty element handling is correct.

Empty elements correctly skip the stack push since they don't create a child scope. Setting lastWasStart = false prevents the subsequent element from being treated as immediately following a start tag.


252-264: Conservative approach to text handling.

The implementation marks hasText = true for all character content, which is more conservative than the PRD design (which suggested only for non-whitespace). This is actually correct behavior for preserving whitespace in mixed content like <p>text <b>bold</b> more</p>.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.java (4)

47-53: Proper defensive copy in constructor.

The constructor correctly creates an unmodifiable copy of the content list via new ArrayList<>(content) wrapped in CollectionUtil.unmodifiableList(), preventing external modification.


82-108: Robust namespace handling with fallbacks.

The code correctly handles both namespace-aware and non-namespace-aware DOM parsing by falling back to getTagName() when getLocalName() is null (line 95). The getPrefix() check prevents duplicate namespace declarations.


110-135: Correct attribute serialization.

The attribute handling correctly skips xmlns declarations (already handled in namespace logic) and provides appropriate fallbacks for non-namespace-aware DOMs.


137-167: Child node handling covers primary use cases.

The switch handles the main node types (elements, text, CDATA, comments). Processing instructions are silently ignored in the default case, which is acceptable for XSD content where PIs are uncommon.

The empty text check on line 147 (!text.isEmpty()) may skip whitespace-only text nodes, but this is typically desirable when serializing parsed DOM content since DOM parsers often create whitespace text nodes from source formatting.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.java (1)

21-51: Clean migration to DOM-based processing.

The changes correctly update the provider to use:

  • org.w3c.dom.Element instead of JDOM2 Element
  • XmlSchemaLoader instead of JDom2XmlSchemaLoader
  • XmlSchemaLoader.NS_XML_SCHEMA for namespace constant
  • DomDatatypeContent instead of JDom2DatatypeContent

Class-level Javadoc has been added as required by coding guidelines.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.java (2)

31-50: Proper migration of queryElements.

The method correctly uses XmlSchemaLoader and XmlSchemaLoader.NS_XML_SCHEMA for the namespace mapping. Class-level Javadoc has been added as required.


73-85: Behavioral note on getAttribute vs getAttributeValue.

Line 79 uses element.getAttribute("name") which returns an empty string (not null) if the attribute is missing, unlike JDOM2's getAttributeValue() which returns null. The ObjectUtils.requireNonNull() check would pass for an empty string.

For valid XSD xs:simpleType elements, the name attribute is required, so this edge case shouldn't occur in practice. The passing tests confirm expected behavior.

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java (2)

22-55: Well-structured XMLStreamWriter2 wrapper.

The class correctly extends IndentingXMLStreamWriter and implements XMLStreamWriter2 by storing a separate delegate2 reference. The Javadoc coverage is appropriate:

  • Class-level documentation explains the purpose
  • Constructor and accessor methods are documented
  • Override methods correctly rely on inherited Javadoc per the project conventions

136-317: Complete TypedXMLStreamWriter and Validatable delegation.

All typed content and validation methods are correctly delegated to the underlying XMLStreamWriter2. These methods handle inline typed values and validation, which don't require indentation adjustments.

schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.java (2)

155-183: Correct migration to standard DOM parsing.

The test now uses DocumentBuilderFactory with namespace awareness enabled (line 178), which is essential for XPath queries with namespace prefixes. The InputStream-based parsing is appropriate for schema files.


206-212: Correct XPath-based validation assertion.

The XPath query //xs:attribute[not(@type or xs:simpleType)] correctly identifies XML Schema attribute declarations missing a type specification. The assertion validates the regression test for liboscal-java#181.

Minor style note: assertEquals(0, result.getLength(), ...) would provide clearer failure diagnostics than assertTrue(... == 0, ...), but the current form is acceptable.

PRDs/20251231-saxon-jdom2-removal/implementation-plan.md (1)

1-11: Well-structured PRD documenting the refactor.

The implementation plan provides clear phase-by-phase documentation of the Saxon/JDOM2 removal. The TDD approach and characterization testing methodology are well-articulated.

Comment on lines +97 to +114
## Requirements

### Functional Requirements

1. **FR-1**: XML schemas generated after the change must be semantically equivalent to those generated before
2. **FR-2**: All existing schemagen tests must pass without modification (except whitespace assertions if any)
3. **FR-3**: XSD datatype resources must continue to be loaded and processed correctly
4. **FR-4**: Generated schemas must be properly indented for readability

### Technical Requirements

1. **TR-1**: Create `IndentingXMLStreamWriter` wrapper for streaming indentation
2. **TR-2**: Replace JDOM2 XML parsing with `javax.xml.parsers.DocumentBuilder`
3. **TR-3**: Replace JDOM2 XPath with `javax.xml.xpath.XPath`
4. **TR-4**: Replace JDOM2 element serialization with `javax.xml.transform.Transformer`
5. **TR-5**: Update `module-info.java` to remove Saxon.HE and org.jdom2 requirements
6. **TR-6**: Remove dependency declarations from pom.xml files

Copy link

@coderabbitai coderabbitai bot Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add technical requirements for namespace handling, encoding, and StAX configuration.

The technical requirements (TR-1 through TR-6) are functional but omit critical implementation details:

  1. TR-3 (XPath): The XSD files use namespace prefixes (xs:schema, xs:simpleType). How will the XPath instance be configured with a NamespaceContext to resolve these prefixes? This must be documented before implementation begins.
  2. Missing TR: Character encoding strategy—will output be UTF-8? How will the XMLStreamWriter factory be configured to enforce encoding?
  3. Missing TR: StAX provider selection—will the implementation use the JDK default or explicitly depend on Woodstox?

These are blocking details for the XmlSchemaLoader and IndentingXMLStreamWriter implementations.

🤖 Prompt for AI Agents
In PRDs/20251231-saxon-jdom2-removal/PRD.md around lines 97 to 114, the
Technical Requirements (TR-1..TR-6) omit critical implementation details for
namespace resolution, character encoding, and StAX provider selection; update
the PRD to (a) add a TR that mandates how XPath will be configured with a
NamespaceContext mapping the XSD prefixes (e.g., "xs") to the XML Schema
namespace and document the location where that mapping will be implemented for
XmlSchemaLoader, (b) add a TR that specifies the character encoding for all
schema output (use UTF-8) and requires XmlStreamWriter/Transformer factories to
be configured to enforce that encoding, and (c) add a TR specifying the StAX
provider policy—either rely on the JDK default or require an explicit dependency
(e.g., Woodstox) and document how the XMLInputFactory/XMLStreamWriter will be
created to guarantee consistent behavior; make these new TRs concise and
reference the affected components (XmlSchemaLoader, IndentingXMLStreamWriter,
and build POMs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai The implementation has already addressed these concerns:

  1. Namespace handling: XmlSchemaLoader uses a NamespaceContext implementation in the getContent() method that maps prefixes (including 'xs') to the appropriate namespaces from the provided namespace map.

  2. Character encoding: UTF-8 is enforced via the XMLStreamWriter factory configuration. The XmlSchemaGenerator.getXmlOutputFactory() configures the output factory with UTF-8 encoding.

  3. StAX provider: The project uses Woodstox (com.ctc.wstx) as specified in the module-info.java. This is a transitive dependency through the existing stax2-api dependency.

Since the PRD is a planning document and the implementation is complete and working, I'm leaving the PRD as-is to reflect the original planning state. The actual implementation details are documented in the code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@david-waltermire Thank you for the clarification. You're absolutely right—since the PRD is a planning document and the implementation is complete with all these concerns properly addressed in the code, it makes sense to preserve the PRD in its original planning state.

The implementation approach you've outlined (NamespaceContext in XmlSchemaLoader, UTF-8 configuration in the factory, and Woodstox via transitive dependency) is sound, and having those details documented in the actual implementation is the appropriate place for them.


🧠 Learnings used
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.

Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/metapath/**/*.java : Metapath is an implementation of XPath 3.1. Use the XPath 3.1 specification (https://www.w3.org/TR/xpath-31/) and XPath Functions 3.1 (https://www.w3.org/TR/xpath-functions-31/) as authoritative reference when implementing new functions, fixing bugs, or understanding error handling. Raise clarification before making changes if implementation differs from spec.

Remove version properties and dependencyManagement entries for:
- org.jdom:jdom2
- jaxen:jaxen
- net.sf.saxon:Saxon-HE
- org.xmlresolver:xmlresolver

These dependencies are no longer used by any module.
Add {@inheritdoc} Javadoc comments to all override methods in
IndentingXMLStreamWriter2 to address CodeRabbit review feedback.
@david-waltermire
Copy link
Contributor Author

@coderabbitai Addressed in commit 1e9b28a. Added {@inheritDoc} Javadoc comments to all override methods in IndentingXMLStreamWriter2 to inherit documentation from the interface.

@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

Tip

For best results, initiate chat on the files or code changes.

@david-waltermire: Thank you for addressing this! Adding {@inheritDoc} to all override methods in IndentingXMLStreamWriter2 is the appropriate approach for methods that implement interface methods, as it explicitly inherits the documentation from XMLStreamWriter2 while keeping the code well-documented. This aligns with Java best practices and the project's Javadoc requirements.


🧠 Learnings used
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:45.001Z
Learning: In Java code for metaschema-framework/metaschema-java, override methods (annotated with Override) that implement interface methods automatically inherit Javadoc from the interface and do not require redundant documentation in the implementing class unless there is implementation-specific behavior that needs to be documented beyond what the interface specifies.

Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.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)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67b7ca0 and 1e9b28a.

📒 Files selected for processing (1)
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.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)

Applied to files:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
📚 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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Code
  • GitHub Check: Website
🔇 Additional comments (4)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java (4)

22-45: LGTM: Class documentation and constructor are well-structured.

The class-level Javadoc clearly explains the wrapper's purpose, and the constructor properly documents its parameter while maintaining non-null safety.


60-458: Javadoc approach follows established project pattern.

The use of {@inheritDoc} for interface method overrides is consistent with project guidance. Based on learnings, interface overrides without implementation-specific behavior should rely on inherited documentation rather than duplicating @param, @return, and @throws tags.


52-54: Clean delegation pattern.

The protected accessor getDelegate2() and consistent delegation approach throughout the class maintain clean separation while allowing potential subclass customization.

Also applies to: 64-66, 96-98


160-163: No action required. The parent class IndentingXMLStreamWriter does not override writeFullEndElement() or writeCData(char[] text, int start, int len). These are XMLStreamWriter2-specific methods that do not exist in the base XMLStreamWriter interface, so there is no parent indentation logic being bypassed by these delegations.

Likely an incorrect or invalid review comment.

Replace XMLOutputFactory.newInstance() with direct WstxOutputFactory
instantiation to ensure Woodstox is always used, regardless of
assertions being enabled.

Also add rule about not excusing test failures as "pre-existing".
@david-waltermire david-waltermire added this to the v3.0.0 Milestone 2 milestone Dec 31, 2025
- Replace assertEquals(true, ...) with assertTrue() in tests
- Extract common createDocumentBuilder() in XmlSchemaLoader
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)

96-104: Consider enabling secure processing to prevent XXE attacks.

The DocumentBuilderFactory is created without explicit XXE protection. While the schemas loaded are likely from trusted sources, enabling secure processing provides defense-in-depth.

🔎 Proposed fix to enable secure processing
 @NonNull
 private static DocumentBuilder createDocumentBuilder() {
   try {
     DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
     factory.setNamespaceAware(true);
+    factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
     return factory.newDocumentBuilder();
   } catch (ParserConfigurationException ex) {
     throw new IllegalStateException("Failed to create document builder", ex);
   }
 }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad9bbc0 and f76475a.

📒 Files selected for processing (4)
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.java
🧠 Learnings (8)
📓 Common learnings
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.
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
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.
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/metapath/**/*.java : Metapath is an implementation of XPath 3.1. Use the XPath 3.1 specification (https://www.w3.org/TR/xpath-31/) and XPath Functions 3.1 (https://www.w3.org/TR/xpath-functions-31/) as authoritative reference when implementing new functions, fixing bugs, or understanding error handling. Raise clarification before making changes if implementation differs from spec.

Applied to files:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
📚 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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.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:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.java
📚 Learning: 2025-12-30T04:37:04.935Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2025-12-30T04:37:04.935Z
Learning: In metaschema-framework/metaschema-java, assert statements on loop variables (e.g., `assert item != null;`) are intentionally kept to satisfy Eclipse's null-safety analysis when the list elements have a `NonNull` contract, ensuring static analysis tools recognize the non-null guarantee for subsequent method calls.

Applied to files:

  • schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Code
  • GitHub Check: Website
🔇 Additional comments (2)
schemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.java (1)

9-9: LGTM!

Good refactor replacing assertEquals(true, ...) with assertTrue(...). This is more idiomatic and produces clearer assertion failure messages.

Also applies to: 269-270

schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)

34-163: Clean implementation of DOM/XPath-based schema loading.

The class provides a well-structured replacement for the JDOM2-based loader. Good use of:

  • Standard Java DOM/XPath APIs appropriate for Java 11 target
  • @NonNull annotations for null safety
  • @SuppressFBWarnings with justifications for intentional patterns
  • Comprehensive Javadoc with proper @param, @return, @throws tags
  • Document normalization to ensure deferred DOM nodes are ready for XPath evaluation

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (1)

197-209: LGTM! Null check properly addresses past review feedback.

The addition of the null check at lines 199-200 correctly addresses the previous review comment about consistency with the NamespaceContext contract. All three methods in MapNamespaceContext now properly validate null inputs and throw IllegalArgumentException as required by the interface specification.

🧹 Nitpick comments (2)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (2)

107-107: Consider adding @nonnull annotations to private method parameters for consistency.

The private parseDocument methods lack @NonNull annotations on their parameters (lines 107 and 115). While Javadoc is not required for private methods, the coding guidelines recommend using SpotBugs annotations for null safety throughout the code. Adding @NonNull to these parameters would improve consistency with the rest of the codebase.

🔎 Suggested enhancement
-  private static Document parseDocument(@NonNull Path path) throws SAXException, IOException {
+  @NonNull
+  private static Document parseDocument(@NonNull Path path) throws SAXException, IOException {
-  private static Document parseDocument(@NonNull InputStream is) throws SAXException, IOException {
+  @NonNull
+  private static Document parseDocument(@NonNull InputStream is) throws SAXException, IOException {

Also applies to: 115-115


141-141: Clarify the reason for the null-safety suppression.

The @SuppressWarnings("null") annotation at line 141 should be documented to explain which specific null-safety warning is being suppressed and why it's a false positive. This helps future maintainers understand the rationale.

Consider adding a comment above the annotation or within the method explaining what null-safety concern is being suppressed (e.g., XPath.evaluate cast, NodeList items, or return value inference).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f76475a and eb76c89.

📒 Files selected for processing (1)
  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
🧠 Learnings (4)
📓 Common learnings
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.
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
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.
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/metapath/**/*.java : Metapath is an implementation of XPath 3.1. Use the XPath 3.1 specification (https://www.w3.org/TR/xpath-31/) and XPath Functions 3.1 (https://www.w3.org/TR/xpath-functions-31/) as authoritative reference when implementing new functions, fixing bugs, or understanding error handling. Raise clarification before making changes if implementation differs from spec.

Applied to files:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
📚 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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.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:

  • schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Website
  • GitHub Check: Code

@david-waltermire david-waltermire merged commit 05bd02e into metaschema-framework:develop Dec 31, 2025
4 checks passed
@david-waltermire david-waltermire deleted the refactor/remove-saxon-jdom2 branch December 31, 2025 19:56
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