feat: support any in Java binding annotations (#220)#657
Conversation
📝 WalkthroughWalkthroughAdds end-to-end support for metaschema : core any-model interfaces and visitor hook, databind Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Reader as Reader (XML/JSON)
participant Parser as Parser (StAX / JSON parser)
participant Model as AssemblyModel (IAnyInstance)
participant AnyContent as AnyContent (XmlAnyContent/JsonAnyContent)
participant Writer as Writer (XML/JSON)
participant Output
Client->>Reader: deserialize(data)
Reader->>Model: check hasAnyInstance?
alt any-instance present
Reader->>Parser: parse known instances
Parser->>AnyContent: capture unmodeled elements/properties
Reader->>Model: boundAny.setAnyContent(AnyContent)
else
Reader->>Parser: skip or warn on unmodeled content
end
Client->>Writer: serialize(object)
Writer->>Model: get bound any content
alt any-content present
loop per item
AnyContent->>Writer: provide element/property
Writer->>Output: write item
end
end
Writer->>Output: finalize stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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.21.0)core/src/main/java/dev/metaschema/core/model/DefaultAssemblyModelBuilder.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. core/src/main/java/dev/metaschema/core/model/IAnyContent.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. core/src/main/java/dev/metaschema/core/model/IModelElementVisitor.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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/main/java/dev/metaschema/core/model/DefaultAssemblyModelBuilder.java (1)
122-134:⚠️ Potential issue | 🔴 CriticalFix: anyInstance is lost when buildAssembly() returns empty().
When
getModelInstances().isEmpty()returns true, the method returns the sharedIContainerModelAssemblySupport.empty()instance regardless of whetheranyInstanceis set. If an assembly has only an<any/>instance without any other model instances, this discards the anyInstance. Instead, create a newDefaultContainerModelAssemblySupportinstance with the anyInstance even when modelInstances is empty, since the code elsewhere treats anyInstance-only scenarios as valid (e.g.,XmlComplexTypeAssemblyDefinitionchecksif (hasModelContent || hasAny)).
🤖 Fix all issues with AI agents
In @.claude/rules/unit-testing.md:
- Line 31: The guideline references a non‑existent skill identifier
"superpowers:systematic-debugging", which is undocumented and unclear; either
document that capability or replace it with concrete steps. Fix by (a) adding a
short description and usage docs for the "superpowers:systematic-debugging"
skill where other "superpowers:" capabilities are documented (include examples
and expected outcomes) and link to that doc from the unit-testing guideline, or
(b) remove "superpowers:systematic-debugging" from the line and replace it with
a bullet list of concrete debugging steps (e.g., reproduce failure, run targeted
test with --run, collect logs, bisect recent changes, add failing example) so
readers understand exactly what to do; ensure the change updates the same
guideline entry so references like "superpowers:systematic-debugging" are no
longer ambiguous.
In
`@databind/src/main/java/dev/metaschema/databind/io/json/MetaschemaJsonReader.java`:
- Around line 607-697: The code uses parser.getLongValue() in buildJsonValue for
VALUE_NUMBER_INT which will throw for integers outside long range; change the
VALUE_NUMBER_INT branch in buildJsonValue to obtain a BigInteger (e.g.
parser.getBigIntegerValue() or parser.getNumberValue() cast to BigInteger) and
pass that BigInteger to nodeFactory.numberNode(...) so integer values of
arbitrary size are preserved instead of failing.
In
`@databind/src/test/java/dev/metaschema/databind/io/xml/AnyXmlRoundTripTest.java`:
- Around line 104-148: The test testWriteSerializesAnyContent uses Java assert
statements that are ignored in CI; replace the two plain "assert
xmlOutput.contains(...)" checks with JUnit assertions (e.g.,
assertTrue(xmlOutput.contains("extra"), "...") and
assertTrue(xmlOutput.contains("foreign-value"), "...")) so they run under
surefire. Update imports if needed to use
org.junit.jupiter.api.Assertions.assertTrue and keep the existing xmlOutput
variable and test method unchanged otherwise.
In
`@databind/src/test/java/dev/metaschema/databind/model/metaschema/AnyInstanceLoadingTest.java`:
- Around line 34-40: The test currently casts rootDef to
DefinitionAssemblyGlobal without checking its type which can throw
ClassCastException; update AnyInstanceLoadingTest to
assertInstanceOf(DefinitionAssemblyGlobal.class, rootDef, ...) (or use
instanceof + fail) before performing the cast, then assign the cast result to a
DefinitionAssemblyGlobal variable and continue calling
getModelContainer().getAnyInstance() so failures show a clear assertion error
instead of a ClassCastException.
In `@PRDs/20260206-any-instance-support/implementation-plan.md`:
- Around line 433-872: Add a new security-hardening task (e.g., "Task 14A:
Security Review and Hardening") to Phase 2 and update Tasks 10–13 to include
concrete mitigations: when converting StAX to DOM in
XmlEventUtil/DocumentBuilderFactory used by
MetaschemaXmlReader/MetaschemaXmlWriter disable external entity resolution and
DTD processing, set entity‑expansion limits and namespace validation; when
capturing JSON into JsonAnyContent/MetaschemaJsonReader/MetaschemaJsonWriter
enforce maximum payload size and maximum nesting depth, reject or truncate
oversized ObjectNode captures, and ensure proper escaping/encoding when
serializing captured content back; add documentation that captured content is
untrusted and add automated security tests (XXE, billion laughs, large payloads,
deep nesting, malicious namespaces) and CI checks to validate these protections.
- Around line 986-1001: Add explicit documentation sub-tasks to Phase 4 by
extending Task 17 with Task 17A/B/C: Task 17A "Update User Documentation" should
add a user guide section for the `@BoundAny` annotation, document the IAnyContent
interface and its format-specific implementations, provide example code for
capturing and serializing any content, and update module reference docs to cover
<any/> declarations; Task 17B "Update API Documentation" should ensure Javadoc
completeness for all new public interfaces/classes, document namespace behavior
for XML any content, and document value-key interaction for JSON any content;
Task 17C "Update Website/Changelog" should add a feature announcement to the
website, update changelog/release notes, and link to issue `#220` so documentation
work is explicitly tracked before PR submission.
- Around line 621-643: The implementation lacks documented error handling and
validation for captured "any" content in the XML/JSON parsers (code paths around
definition.getAnyInstance(), staxToDom(), XmlAnyContent, anyInstance.setValue()
and the fallback XmlEventUtil.skipElement()); update the implementation plan to
specify: a) on malformed content or JSON parse failures decide whether to fail
the whole parse or attach an error object to the any container (recommend
attaching an error marker plus original raw content), b) whether any content is
validated or treated as opaque (recommend default opaque with optional
schema/constraint hooks), c) enforce resource limits (max bytes/elements per any
capture and abort with a clear error if exceeded), and d) how errors are
surfaced to users (structured error codes/messages and logs) and ensure both XML
(staxToDom path) and JSON capture paths follow this behavior and are explicitly
documented for consistency.
- Line 3: Remove the AI assistant directive line that reads "For Claude:
REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan
task-by-task" from the implementation plan
(PRDs/20260206-any-instance-support/implementation-plan.md) so the document is
tool-agnostic; simply delete that line or replace it with a human-facing
instruction or heading that describes the expected developer workflow without
referencing any AI agent or sub-skill.
- Around line 613-615: Replace the plan's new utility with a reference to the
existing XmlDomUtil.staxToElement() and modify XmlDomUtil (specifically
newDocument() and any use of TransformerFactory during StAX-to-DOM conversion)
to harden XML parsing against XXE by setting the DocumentBuilderFactory and
TransformerFactory external-access attributes: call
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "") and
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "") (and the equivalent on
TransformerFactory) so external DTD/schema access is denied; update any callers
of staxToElement() if necessary to use the hardened newDocument()/factory.
🧹 Nitpick comments (3)
PRDs/20260206-any-instance-support/implementation-plan.md (1)
168-189: Provide explicit strategy for discovering all interface implementors.Adding a type parameter to
IContainerModelAssemblySupportis correctly identified as a breaking change. However, the plan could be more prescriptive about discovering all affected implementors to reduce implementation risk.Consider adding:
- Command to discover implementors:
rg "implements.*IContainerModelAssemblySupport|extends.*IContainerModelAssemblySupport" --type java- Expected count of affected classes across modules
- Checklist of known critical implementors (if applicable)
This would make the breaking change more manageable and reduce the chance of missing an implementor during compilation fix passes.
core/src/main/java/dev/metaschema/core/model/IModelElementVisitor.java (1)
101-110: LGTM! Minor inconsistency in parameter naming.The
visitAnycallback is correctly added to support visitingIAnyInstanceelements. The Javadoc is complete and follows the existing pattern.Nit: The parameter is named
instancewhile all other visitor methods useitem(e.g.,visitFlagInstance(IFlagInstance item, ...)). Consider renaming for consistency:🔧 Optional: Align parameter naming
- RESULT visitAny(`@NonNull` IAnyInstance instance, CONTEXT context); + RESULT visitAny(`@NonNull` IAnyInstance item, CONTEXT context);,
databind/src/main/java/dev/metaschema/databind/io/xml/XmlDomUtil.java (1)
193-216: Consider: Deeply nested XML could cause stack overflow.The
readChildrenmethod uses recursion to process nested elements. For extremely deep XML structures, this could potentially cause aStackOverflowError. This is a known trade-off with recursive XML processing and is unlikely to be a problem in practice for typical Metaschema content.
databind/src/main/java/dev/metaschema/databind/io/json/MetaschemaJsonReader.java
Show resolved
Hide resolved
databind/src/test/java/dev/metaschema/databind/io/xml/AnyXmlRoundTripTest.java
Show resolved
Hide resolved
databind/src/test/java/dev/metaschema/databind/model/metaschema/AnyInstanceLoadingTest.java
Show resolved
Hide resolved
- IAnyInstance.isEffectiveValueWrappedInXml() returns false instead of throwing UnsupportedOperationException - Use BigInteger for integer any-content parsing to handle values outside long range - Replace Java assert statements with JUnit assertTrue in AnyXmlRoundTripTest - Use assertInstanceOf instead of cast in AnyInstanceLoadingTest - Remove AI directive from implementation-plan.md
|
@coderabbitai Addressed outside-diff-range and nitpick comments in commit 63d733a: Critical (outside diff range): Nit: Nit (XmlDomUtil recursive depth): Acknowledged — unlikely to be an issue in practice for typical Metaschema content. No action taken. Nit (implementation-plan.md implementor discovery): PRD is completed; no update needed. |
|
Tip For best results, initiate chat on the files or code changes.
✅ Critical fix: Great catch on the empty singleton logic—ensuring ✅ Consistency improvement: Renaming the parameter to ✅ Pragmatic decision: Acknowledged on the recursive depth concern—makes sense that typical Metaschema content won't hit problematic depths. ✅ PRD status: Understood that the PRD is complete. The fixes look good! 🎉 |
|
Addressed remaining review feedback in commit 9f310f1: Code changes:
Implementation plan updates:
Previously addressed (commits 44247ba, 63d733a):
All 30 any-instance tests passing. |
…ramework#220) Add full support for the Metaschema <any/> instance type, enabling assemblies to capture and round-trip unmodeled content across XML and JSON/YAML formats. Core model layer: - IAnyContent interface for format-neutral unmodeled content - IAnyInstance interface with ModelType.ANY enum constant - Container model updates (IContainerModelAssemblySupport, DefaultContainerModelAssemblySupport, DefaultAssemblyModelBuilder) - IModelElementVisitor.visitAny() callback Databind binding layer: - @BoundAny annotation for marking IAnyContent fields - IBoundInstanceModelAny and InstanceModelAny implementation - AssemblyModelGenerator scans for @BoundAny during introspection - AnyInstanceImpl for module-loaded definitions XML parsing and writing: - XmlAnyContent wraps captured DOM elements - XmlDomUtil for StAX-to-DOM and DOM-to-StAX conversion with XXE-hardened DocumentBuilderFactory - MetaschemaXmlReader captures unknown child elements - MetaschemaXmlWriter serializes captured elements back JSON/YAML parsing and writing: - JsonAnyContent wraps captured Jackson ObjectNode - MetaschemaJsonReader captures unmatched properties using BigInteger for arbitrary-size integers - MetaschemaJsonWriter serializes captured properties back Schema generation: - XML Schema: emits xs:any namespace="##other" processContents="lax" - JSON Schema: sets additionalProperties: true Tests: 56 new tests covering container model, module loading, content wrappers, XML/JSON round-trips, json-value-key interaction, and schema generation.
9f310f1 to
8e27d5d
Compare
aj-stein
left a comment
There was a problem hiding this comment.
Small question, but this is a very exciting return of any!!!
9fef227
into
metaschema-framework:develop
…framework#220) The code generator now emits @BoundAny-annotated IAnyContent fields when an assembly definition contains <any/> in its model. This complements the runtime <any/> support added in PR metaschema-framework#657 by enabling generated binding classes to capture unmodeled content. Also adds a Claude rule documenting the full scope of changes required when adding new Metaschema model features.
…framework#220) The code generator now emits @BoundAny-annotated IAnyContent fields when an assembly definition contains <any/> in its model. This complements the runtime <any/> support added in PR metaschema-framework#657 by enabling generated binding classes to capture unmodeled content. Also adds a Claude rule documenting the full scope of changes required when adding new Metaschema model features.
The code generator now emits @BoundAny-annotated IAnyContent fields when an assembly definition contains <any/> in its model. This complements the runtime <any/> support added in PR #657 by enabling generated binding classes to capture unmodeled content. Also adds a Claude rule documenting the full scope of changes required when adding new Metaschema model features.
Summary
IAnyContentandIAnyInstancecore model interfaces for the Metaschema<any/>instance type@BoundAnyannotation andIBoundInstanceModelAnydatabind binding layerElementinstances via StAX-to-DOM conversion, round-trips with full fidelityObjectNode, round-trips with full fidelityxs:any namespace="##other" processContents="lax"in XML Schema for assemblies with<any/>additionalProperties: truein JSON Schema for assemblies with<any/>XmlDomUtil.newDocument()against XXE (defense-in-depth)@DisabledSpecification Documentation: Metaschema language-level documentation for
<any/>is tracked in companion PR metaschema-framework/metaschema#171.Test plan
ContainerModelAnyInstanceTest— 8 tests for core container modelgetAnyInstance()behaviorAnyInstanceLoadingTest— verifies<any/>is processed during Metaschema module loadingXmlAnyContentTest— 6 unit tests forXmlAnyContentwrapperJsonAnyContentTest— 10 unit tests forJsonAnyContentwrapperAnyXmlRoundTripTest— 5 XML parsing/writing round-trip tests including nested elements and attributesAnyJsonRoundTripTest— 8 JSON parsing/writing round-trip tests including nested objects and arraysAnyJsonValueKeyTest— 5 tests verifying correct interaction betweenjson-keyflags and@BoundAnycaptureAnyXmlSchemaGenerationTest— 2 tests for XML Schemaxs:anyoutputAnyJsonSchemaGenerationTest— 2 tests for JSON SchemaadditionalPropertiesoutputmvn clean install -PCI -PreleaseCloses #220
Summary by CodeRabbit
New Features
@BoundAnyannotation enables fields to receive unmodeled content from assemblies.Documentation