refactor: remove Saxon-HE, JDOM2, and jaxen dependencies from schemagen#607
Conversation
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.
📝 WalkthroughWalkthroughRemoves 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
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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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:
- Test framework: Will new tests use JUnit 4, JUnit 5, or another framework? Should they be consistent with existing schemagen tests?
- 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?
- 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)andparseDocument(InputStream)methods duplicate theDocumentBuilderFactoryconfiguration 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: PreferassertTrue()overassertEquals(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: preferassertTrue().🔎 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: PreferassertTrue()overassertEquals(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
.//@basecorrectly finds allbaseattributes for restriction/extension type references. The deduplication logic works correctly for the expected small number of dependencies.Minor optimization opportunity: the
XPathinstance 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
NamespaceContextimplementation is sufficient for this test's XPath evaluation. Note thatgetPrefix()andgetPrefixes()returningnulltechnically violates the interface contract (should returnnullandIterator.empty()respectively), but this works in practice since JDK's XPath only callsgetNamespaceURI()during evaluation.Consider using the
MapNamespaceContextpattern fromXmlSchemaLoaderfor 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 setshasText = truefor 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
📒 Files selected for processing (20)
PRDs/20251231-saxon-jdom2-removal/PRD.mdPRDs/20251231-saxon-jdom2-removal/implementation-plan.mdschemagen/pom.xmlschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2DatatypeContent.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/JDom2XmlSchemaLoader.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseBaseDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.javaschemagen/src/main/java/module-info.javaschemagen/src/main/resources/identity.xslschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.javaschemagen/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.javaschemagen/src/main/java/module-info.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.javaschemagen/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.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/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.mdschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.javaschemagen/src/main/java/module-info.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaPRDs/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.javaschemagen/src/main/java/module-info.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.javaPRDs/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.javaschemagen/src/main/java/module-info.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.javaschemagen/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContent.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.javaschemagen/src/main/java/module-info.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlDatatypeProvider.javaschemagen/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.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlCoreDatatypeProvider.javaPRDs/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/DomDatatypeContentTest.javaPRDs/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/XmlSuiteTest.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.javaPRDs/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.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.javaschemagen/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
hasTextdetection strategy (referenced in lines 156–159) leaves critical edge cases undefined:
- Whitespace-only text nodes: Should
writeCharacters("\n ")(formatting between elements) sethasText = true? If yes, this could suppress indentation unnecessarily.- CDATA sections: Should CDATA trigger
hasTextthe same way as text content?- 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
hasTextflag 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 iftextis:
- Formatting whitespace (e.g.,
"\n "before a child element) → should not sethasText = true(allow indentation)- Content whitespace (e.g.,
" "between inline elements) → should sethasText = 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:
- Deep traversal vs. shallow copy: Will the implementation recursively traverse DOM nodes (like the original
StAXStreamOutputter) or copy the subtree?- Namespace handling: How will
xmlnsdeclarations be preserved when copying elements? What about namespace prefix remapping?- Special node types: How will CDATA sections, comments, processing instructions, and entity references be handled?
- 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.stax2correctly replaces the removed Saxon-HE and JDOM2 dependencies, providing theXMLStreamWriter2interface needed by the newIndentingXMLStreamWriter2implementation.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
@Nestedgroupings and descriptive@DisplayNameannotations. 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()andcreateSchemaElement()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 baseXMLStreamWriter2withIndentingXMLStreamWriter2, 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
XmlSchemaLoaderfor namespace-aware DOM parsing- Exception handling properly catches
SAXException | IOExceptionmatching 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 DOMElementtype.schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/AbstractXmlMarkupDatatypeProvider.java (2)
46-51: LGTM!The
queryElementsimplementation correctly uses the newXmlSchemaLoaderAPI with the proper namespace mapping viaXmlSchemaLoader.NS_XML_SCHEMA.
62-72: LGTM!The
handleResultsmethod correctly:
- Filters out
xs:includeelements using DOM'sgetLocalName()- Constructs
DomDatatypeContentas the replacement for the removedJDom2DatatypeContentschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java (2)
141-161: LGTM!The
getContentmethod correctly:
- Creates a new XPath instance with the provided namespace context
- Evaluates the expression and collects only
Elementnodes- Handles invalid XPath expressions by throwing
IllegalArgumentExceptionwith a clear message
166-205: LGTM!The
MapNamespaceContextinner class correctly implements theNamespaceContextinterface:
- Validates null inputs as required by the contract
- Returns
XMLConstants.NULL_NS_URIfor unknown prefixes (correct per spec)- Implements
getPrefixandgetPrefixesfor reverse lookupsschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoaderTest.java (1)
42-53: LGTM!Using
@BeforeAllto 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
IndentingXMLStreamWriterwith 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
hasTextStackdesign correctly preserves parent mixed-content state across nested elements, which is essential for proper XHTML handling inxs:documentation.
137-148: Correct end-element indentation logic.The combination of
!hasText && !lastWasStartcorrectly 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 = falseprevents the subsequent element from being treated as immediately following a start tag.
252-264: Conservative approach to text handling.The implementation marks
hasText = truefor 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 inCollectionUtil.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()whengetLocalName()is null (line 95). ThegetPrefix()check prevents duplicate namespace declarations.
110-135: Correct attribute serialization.The attribute handling correctly skips
xmlnsdeclarations (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.Elementinstead of JDOM2 ElementXmlSchemaLoaderinstead ofJDom2XmlSchemaLoaderXmlSchemaLoader.NS_XML_SCHEMAfor namespace constantDomDatatypeContentinstead ofJDom2DatatypeContentClass-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
XmlSchemaLoaderandXmlSchemaLoader.NS_XML_SCHEMAfor 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'sgetAttributeValue()which returns null. TheObjectUtils.requireNonNull()check would pass for an empty string.For valid XSD
xs:simpleTypeelements, thenameattribute 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
IndentingXMLStreamWriterand implementsXMLStreamWriter2by storing a separatedelegate2reference. 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
DocumentBuilderFactorywith 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 thanassertTrue(... == 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.
| ## 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 | ||
|
|
There was a problem hiding this comment.
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:
- TR-3 (XPath): The XSD files use namespace prefixes (
xs:schema,xs:simpleType). How will theXPathinstance be configured with aNamespaceContextto resolve these prefixes? This must be documented before implementation begins. - Missing TR: Character encoding strategy—will output be UTF-8? How will the
XMLStreamWriterfactory be configured to enforce encoding? - 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).
There was a problem hiding this comment.
@coderabbitai The implementation has already addressed these concerns:
-
Namespace handling:
XmlSchemaLoaderuses aNamespaceContextimplementation in thegetContent()method that maps prefixes (including 'xs') to the appropriate namespaces from the provided namespace map. -
Character encoding: UTF-8 is enforced via the
XMLStreamWriterfactory configuration. TheXmlSchemaGenerator.getXmlOutputFactory()configures the output factory with UTF-8 encoding. -
StAX provider: The project uses Woodstox (
com.ctc.wstx) as specified in themodule-info.java. This is a transitive dependency through the existingstax2-apidependency.
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.
There was a problem hiding this comment.
@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.
.../src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
Show resolved
Hide resolved
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.
|
@coderabbitai Addressed in commit 1e9b28a. Added |
|
Tip For best results, initiate chat on the files or code changes. @david-waltermire: Thank you for addressing this! Adding 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 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 classIndentingXMLStreamWriterdoes not overridewriteFullEndElement()orwriteCData(char[] text, int start, int len). These are XMLStreamWriter2-specific methods that do not exist in the baseXMLStreamWriterinterface, so there is no parent indentation logic being bypassed by these delegations.Likely an incorrect or invalid review comment.
.../src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriter2.java
Show resolved
Hide resolved
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".
- Replace assertEquals(true, ...) with assertTrue() in tests - Extract common createDocumentBuilder() in XmlSchemaLoader
There was a problem hiding this comment.
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
DocumentBuilderFactoryis 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
📒 Files selected for processing (4)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/AbstractSchemaGeneratorTestSuite.javaschemagen/src/test/java/gov/nist/secauto/metaschema/schemagen/xml/impl/IndentingXMLStreamWriterTest.javaschemagen/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.javaschemagen/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.javaschemagen/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.javaschemagen/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, ...)withassertTrue(...). 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
@NonNullannotations for null safety@SuppressFBWarningswith justifications for intentional patterns- Comprehensive Javadoc with proper
@param,@return,@throwstags- Document normalization to ensure deferred DOM nodes are ready for XPath evaluation
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlSchemaLoader.java
Show resolved
Hide resolved
There was a problem hiding this comment.
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
NamespaceContextcontract. All three methods inMapNamespaceContextnow properly validate null inputs and throwIllegalArgumentExceptionas 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
parseDocumentmethods lack@NonNullannotations 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@NonNullto 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
📒 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
05bd02e
into
metaschema-framework:develop
Summary
IndentingXMLStreamWriterfor XML output formattingChanges
New Classes
IndentingXMLStreamWriter- XMLStreamWriter wrapper that adds proper indentation to XML outputIndentingXMLStreamWriter2- Extension that implements XMLStreamWriter2 interfaceXmlSchemaLoader- Loads XML schemas using standard DOM/XPath APIsDomDatatypeContent- Writes DOM elements to XMLStreamWriterRemoved Classes
JDom2DatatypeContent- Replaced byDomDatatypeContentJDom2XmlSchemaLoader- Replaced byXmlSchemaLoaderRemoved Files
identity.xsl- No longer needed; indentation now handled byIndentingXMLStreamWriterDependencies Removed
net.sf.saxon:Saxon-HEorg.jdom:jdom2jaxen:jaxenTest Plan
IndentingXMLStreamWriter(22 tests)XmlSchemaLoader(7 tests)DomDatatypeContent(8 tests)mvn clean install -PCI -PreleaseBenefits
Summary by CodeRabbit
Breaking Changes
Improvements
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.