Add comprehensive Javadoc documentation to core module#544
Conversation
This commit adds Javadoc documentation to address warnings generated during CI builds with the -PCI -Prelease profiles: - Add class/interface-level Javadocs to model interfaces (IContainer, INamed, IDefinition, IAssemblyDefinition, IFieldDefinition, etc.) - Add Javadocs to abstract base classes (AbstractInstance, AbstractLoader, AbstractNamedInstance, etc.) - Document constraint-related classes (IConstraint enum constants, IConstraintValidator, ConstraintValidationException) - Add Javadocs to Metapath function and exception classes - Document node item interfaces and implementations - Add Javadocs to utility classes (ExceptionUtils, ModuleUtils, etc.) All documentation follows the project's Javadoc style guide with concise, meaningful descriptions.
Enhanced package-info.java files across the core module with: - Comprehensive descriptions of package purpose and scope - Key interfaces and classes with brief descriptions - Usage context and cross-references to related packages - References to XPath 3.1 specifications where applicable - Proper Javadoc formatting with HTML tags and @see references Packages updated: - core: configuration, util - datatype: adapter, markup, markup/flexmark, object - metapath: antlr, cst, cst/math, cst/path, cst/type, format, function, function/library, function/regex, item, item/atomic, item/node - model: constraint, constraint/impl, util, validation
Created package-level Javadoc documentation for 14 packages that were previously missing package-info.java files: - mdm, mdm/impl - Metaschema Document Model interfaces and implementations - metapath/impl - Metapath expression engine internals - metapath/cst/items - CST nodes for item and sequence expressions - metapath/cst/logic - CST nodes for logical and comparison expressions - metapath/function/impl - Function implementation support - metapath/item/atomic/impl - Atomic item type implementations - metapath/item/function - Array and map item interfaces - metapath/item/function/impl - Array and map implementations - metapath/type - Type system interfaces - metapath/type/impl - Type system implementations - model/impl - Model container support implementations - datatype/markup/flexmark/impl - Flexmark markup processing internals - qname - Enhanced qualified name support with caching
Added -J-Dfile.encoding=UTF-8 to maven-javadoc-plugin configuration to ensure UTF-8 encoding is used when javadoc runs in a forked JVM process. This fixes "unmappable character (0x9D) for encoding windows-1252" errors that occurred because JDK 17 defaults to Windows-1252 on Windows systems. Also fixed invalid @see references in package-info.java files that were pointing to packages instead of classes.
- Fixed import order in AbstractRecursionPreventingNodeItemVisitorTest, RecursionCollectingNodeItemVisitorTest, and MermaidErDiagramGeneratorTest - Made baseType variables final in ExpressionUtilsTest to address VariableDeclarationUsageDistance warnings
📝 WalkthroughWalkthroughWidespread documentation additions and package-info files across the core module; API edits include Metapath exception evaluation-stack capture, removal of ISequence.getValue(), added enum constants and default interface methods in model APIs, annotation tweaks for deprecated NCNAME types, ExceptionUtils enhancements, test helpers, and Maven javadoc UTF‑8 configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Eval as Metapath Evaluator
participant Dyn as DynamicContext
participant Expr as IExpression
participant Ex as MetapathException
Eval->>Dyn: evaluation begins (maintain execution stack)
Dyn-->>Eval: execution stack (Deque<IExpression>)
Eval->>Expr: evaluate node (push/pop expression)
alt error during evaluation
Eval->>Ex: instantiate InvalidTreatTypeDynamicMetapathException(evaluationStack, message)
Ex->>Ex: registerEvaluationContext(evaluationStack) / registerEvaluationContext(Dyn)
Ex-->>Eval: throw MetapathException (carries evaluation stack)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java (1)
65-72: Documentation inconsistency in existing method.The Javadoc for
getEffectiveJsonKey()at line 69 contains a contradictory phrase: "or {@code null} if a JSON key is configured." Based on the method logic (comparing withhasJsonKey()which returnstruewhengetEffectiveJsonKey() != null), this should likely read "or {@code null} if a JSON key is NOT configured."Apply this diff to fix the documentation:
/** * Get the JSON key flag instance for this model instance, if one is configured. * * @return the JSON key flag instance or {@code null} if a JSON key is - * configured + * not configured */core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java (1)
20-23: Fix typos/grammar inFLAGJavadoc (“idenfied”, “who’s”).
This is user-facing documentation; the current wording reads as incorrect English./** - * A flag is idenfied as the value key, who's value will be used. + * A flag is identified as the value key, whose value will be used. */ FLAG;core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (2)
91-105: Constructor should enforce non-null cause (don’t rely on assertions).
BecauseObjectUtils.notNullusesassert, a null cause can slip through in production if callers violate the contract, and failures will happen later (or as NPEs). Fail fast in the constructor.+import java.util.Objects; @@ public WrappedException(@NonNull Throwable cause) { - super(cause); + super(Objects.requireNonNull(cause, "cause")); }Also applies to: 111-114
116-139: Fix typo in exception message; consider avoiding duplicate suppressed entries.
Message currently says “excpeted”. Also, repeated unwrapping will add the same wrapper multiple times tocause.getSuppressed(); consider guarding if this is expected to happen.throw new IllegalArgumentException( - String.format("Wrapped exception '%s' did not match excpeted type '%s'.", + String.format("Wrapped exception '%s' did not match expected type '%s'.", cause.getClass().getName(), wrappedExceptionClass.getName()));core/src/main/java/gov/nist/secauto/metaschema/core/model/util/ModuleUtils.java (1)
44-65: Fix typo in method documentation.Line 45 contains a spelling error: "assemvly" should be "assembly".
Apply this diff to correct the typo:
/** * Parse the name of a field or assemvly. * <p>Correct to:
/** * Parse the name of a field or assembly. * <p>core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitor.java (1)
37-46: Method nameisDecendant()should be corrected toisDescendant().The spelling error in this protected method would require updating the call site at
RecursionCollectingNodeItemVisitor.java:68and within the same class hierarchy. Since the method is protected and called externally, this is a breaking change for any subclasses or external callers. Consider addressing this alongside the method rename to maintain API consistency.core/src/main/java/gov/nist/secauto/metaschema/core/model/IAssemblyDefinition.java (1)
41-101: Add missing@Nullablewhere defaults can return null; consider removing the commented-out dead code block.@@ import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; @@ + @Nullable default IEnhancedQName getRootQName() { @@ + @Nullable default String getRootJsonName() { return getRootName(); } @@ @Override + @Nullable default IAssemblyInstance getInlineInstance() { // not inline by default return null; }(Commented-out
getNodeItem()block at Lines 107-112 is also a good candidate for deletion.)Also applies to: 107-112
🧹 Nitpick comments (29)
core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldDefinition.java (1)
56-61: Fix grammar in Javadoc (“who’s” → “whose”).
Line 57: “who’s value” should be “whose value”.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/UnidentifiedFunctionError.java (1)
13-16: Javadoc addition is consistent with the implemented error code (FOER0000). The description matchesIErrorCode.of("FOER", 0)and doesn’t imply behavior that isn’t present. Consider formatting the code as{@code FOER0000}(and optionally adding an@seeto the relevant spec/registry if the project has a canonical reference), but this is optional.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IFeatureNoDataAtomicValuedItem.java (1)
10-13: Javadoc reads fine; consider explicitly documenting the thrown conversion behavior.Since this type still presents as
IAtomicValuedItembut “has no associated value data”, it may help to mention (briefly) thattoAtomicItem()is not supported / always throwsInvalidTypeFunctionException, or clarify the intended distinction vsIFeatureNoDataValuedItem.core/src/main/java/gov/nist/secauto/metaschema/core/model/IModelElementVisitable.java (1)
10-15: Nice addition; consider tightening Javadoc around nullability/contracts.The interface-level Javadoc reads well. Since the method uses
@NonNullforvisitorbut leavescontextandRESULTunconstrained, consider adding a brief note clarifying whethercontextmay be null and whether implementations may return null (or annotate accordingly), to avoid ambiguity for implementers/callers.core/src/main/java/gov/nist/secauto/metaschema/core/model/IValuedInstance.java (1)
10-13: Nice Javadoc; consider{@link ...}cross-references (and “e.g.” vs “i.e.”).
If this interface can represent more than just field/flag instances, “e.g.” is safer than “i.e.”; also consider linking the mentioned concepts/types (e.g.,IValuedDefinitionand the field/flag instance interfaces/classes) for easier IDE navigation.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractInstanceNodeItem.java (2)
10-19: Clarify terminology and add links in class Javadoc“Metaschema instance” can be read as an instance document; here it’s specifically an
INamedInstance. Consider tightening wording and adding{@link ...}references for quick navigation (e.g.,INamedInstance,IDefinition,IModelNodeItem).
32-45: Document non-null contract in constructor JavadocSince both params are annotated
@NonNull(Line 41-42), consider stating “must not be null” in the@paramdescriptions to make the contract explicit for readers who don’t notice annotations.core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureDefinitionReferenceInstance.java (1)
13-25: Clarify the “inline instance” type-parameter wording (reads contradictory withisInlineDefinition() == false).The class Javadoc says
INSTANCEis “the Java type of the inline instance (when definition is inline)”, but this interface’s defaults hard-code non-inline behavior (isInlineDefinition()returnsfalse;getInlineInstance()returnsnull). Suggest rephrasing to something like “(when an implementation supports inline definitions)” to avoid confusion.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractMetapathExpression.java (1)
53-64:getExpression()Javadoc is accurate; consider clarifying “compiled” implies stable/cached node. If implementations can return different nodes per call, “compiled” may over-promise—either ensure caching or tweak wording.core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/JsonSchemaContentValidator.java (1)
31-33: Javadoc is correct; consider adding library + schema dialect/version expectations.
Since this is backed by Everit JSON Schema, it may help to mention the underlying implementation (and, if known/assumed, the JSON Schema draft/dialect supported) and whether instances are intended to be reusable/thread-safe.core/src/main/java/gov/nist/secauto/metaschema/core/model/IFlagDefinition.java (1)
10-16: Good interface-level Javadoc; consider adding a couple of cross-references for navigation.
The description reads well. As an optional enhancement, add@see IFlag,@see IFlagInstance, and/or@see IValuedDefinition(or inline{@link ...}) so generated docs are easier to traverse.core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java (1)
8-10: Enum-level Javadoc looks good; consider briefly enumerating the supported key types.
Optional: add a short phrase like “Supported types: NONE, STATIC_LABEL, FLAG” to make the Javadoc more immediately scannable.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/regex/package-info.java (1)
7-43: Package-level Javadoc is clear and discoverable (key classes + supported flags + usage).
Optional: consider using{@link gov.nist.secauto.metaschema.core.metapath.function.library}instead of a bare@seepackage name if you want consistent link rendering.core/src/main/java/gov/nist/secauto/metaschema/core/model/IJsonInstance.java (2)
10-15: ConsiderIJsonInstance extends IJsonNamedto avoid duplicated contracts.At Line 10-16, since
IJsonNamedalready definesgetJsonName(), havingIJsonInstanceextend it would remove duplication and make the type relationship explicit (while staying source/binary compatible for existing implementers).
17-21: Method Javadoc aligns with signature; minor nit: mention non-null contract.At Line 17-21, consider noting the return is non-null (since
@NonNullis part of the contract).core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/JsonFunctionException.java (1)
12-14: Documentation added; consider enhancing with spec reference.The Javadoc provides a basic overview of the exception class. Consider enhancing it with a reference to the XPath Functions 3.1 specification for additional context, similar to the detailed documentation provided for the individual error codes below.
Example enhancement:
/** - * FOJS: Exceptions related to JSON function operations in XPath. + * FOJS: Exceptions related to JSON function operations in XPath 3.1. + * <p> + * This class defines error codes for JSON-related operations as specified in + * <a href="https://www.w3.org/TR/xpath-functions-31/#json-to-xml-mapping"> + * XPath and XQuery Functions and Operators 3.1</a>. */core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/package-info.java (1)
6-45: Minor Javadoc polish: tighten links + list punctuation.Since this is in
gov.nist.secauto.metaschema.core.model.validation, you can simplify{@link ...}to simple type names (e.g.,{@link IContentValidator}), and consider adding consistent punctuation/parallel phrasing in the<li>items for readability.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IArgument.java (1)
75-90: Consider enhancing Javadoc with usage context.The implementation is correct, but the Javadoc could benefit from additional context explaining that this method serves as a resolver callback for
EQNameFactory.parseName()(as used on line 118) and clarifying what "lexical qualified names" means in this context or referencing the relevant specification.Example enhancement:
/** - * Resolve an argument name from a prefix. + * Resolve an argument name from a prefix. This method is used as a resolver + * callback by {@link EQNameFactory#parseName(String, java.util.function.Function)} + * to enforce that argument names must not use lexical qualified names (prefix:localName). * * @param prefix * the prefix to resolve - * @return the resolved argument name + * @return an empty string for the empty prefix * @throws UnsupportedOperationException * if a non-empty prefix is provided */core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/package-info.java (1)
27-27: Consider linking to a specific class instead of the package.While linking to the package is valid, Javadoc links are typically more useful when they point to a specific class or interface. Consider linking to a key interface or class from the
datatypepackage instead.core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java (1)
44-46: Consider inline formatting for the{@code}block.The
{@code <p>}spanning three lines is syntactically valid but could be cleaner on a single line for readability.-* <li>{@link SuppressPTagExtension} - Suppresses {@code -* <p> -* } paragraph tags in single-line markup content</li> +* <li>{@link SuppressPTagExtension} - Suppresses {@code <p>} paragraph tags +* in single-line markup content</li>core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ModelTargetedConstraints.java (1)
17-20: Class-level Javadoc is clear and accurate.Minor nit: consider expanding the doc to mention that this targets definitions (not instances) and that it applies index/unique/cardinality constraints (based on
applyToimplementation).core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/package-info.java (1)
7-49: Strong package overview; links and structure look good.If this PR also introduced new constraint types (the PR summary mentions
IConstraint.Typeexpansion), please ensure the authoritative constraint-type lists/docs are updated too (metaschema-module-constraints.xml and website constraints syntax docs). Based on learnings, this is easy to miss.core/src/main/java/gov/nist/secauto/metaschema/core/model/ILoader.java (1)
19-27: Good high-level contract documentation for the loader abstraction.Optional: consider stating whether
getLoadedResources()returns a live/mutable view vs snapshot/unmodifiable collection (helps callers avoid accidental mutation/assumptions).core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractDefinitionNodeItem.java (1)
34-37: Add explicit nullness contract ongetDefinition()(and ensure public exposure is intended).
Sincedefinitionis constructed@NonNull, consider annotating the return as@NonNull(or documenting nullness via Javadoc) so downstream nullness tooling matches reality. Also worth a quick sanity check that exposing the backing definition via a new public getter doesn’t bypass any intended encapsulation.core/src/test/java/gov/nist/secauto/metaschema/core/metapath/TestUtils.java (1)
309-335: Consider an overload for “no namespace” and (optionally)localNamenaming.
If callers commonly need a “no namespace” QName, an overload likeeqname(@NonNull String localname)(using""internally) or accepting@Nullable namespacecan make tests less noisy. Also,localNamewould better match typical Java conventions (low impact since test-only).core/src/main/java/gov/nist/secauto/metaschema/core/qname/package-info.java (1)
6-99: Double-check the doc’s concrete guarantees match the actual cache implementation.
This Javadoc states specifics like singleton caches,ConcurrentHashMapusage, and index-based lookup semantics—please confirm these statements reflect current behavior (or soften wording) so it doesn’t accidentally become an incorrect contract.core/src/main/java/gov/nist/secauto/metaschema/core/model/IFlagInstance.java (1)
19-53: DefaultisRequired()returningfalseis safe but may hide missing overrides.If “required” is meaningful per-flag-instance in this model, consider ensuring concrete implementations explicitly override
isRequired()(or add stronger Javadoc wording that this is a default/fallback).core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureContainerModelGrouped.java (1)
12-79: Align nullability annotations withIContainerModelGroupedand consider whether returning livemap.values()views is intended.
Also, the@SuppressWarnings("null")hints the nullness contracts may not be expressed strongly enough onIContainerModelSupport/ its map getters.@@ import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; @@ @Override + @Nullable default NMI getNamedModelInstanceByName(Integer name) { return getModelContainer().getNamedModelInstanceMap().get(name); } @@ @Override + @NonNull default Collection<NMI> getNamedModelInstances() { return getModelContainer().getNamedModelInstanceMap().values(); } @@ @Override + @Nullable default FI getFieldInstanceByName(Integer name) { return getModelContainer().getFieldInstanceMap().get(name); } @@ @Override + @NonNull default Collection<FI> getFieldInstances() { return getModelContainer().getFieldInstanceMap().values(); } @@ @Override + @Nullable default AI getAssemblyInstanceByName(Integer name) { return getModelContainer().getAssemblyInstanceMap().get(name); } @@ @Override + @NonNull default Collection<AI> getAssemblyInstances() { return getModelContainer().getAssemblyInstanceMap().values(); }core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelAbsolute.java (1)
10-38: Add nullability annotations (and consider “index” terminology) for clarity/consistency.@@ import java.util.Collection; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; @@ @Override + @NonNull Collection<? extends IModelInstanceAbsolute> getModelInstances(); @@ @Override + @NonNull Collection<? extends INamedModelInstanceAbsolute> getNamedModelInstances(); @@ @Override + @Nullable INamedModelInstanceAbsolute getNamedModelInstanceByName(Integer name); @@ @Override + @NonNull Collection<? extends IFieldInstanceAbsolute> getFieldInstances(); @@ @Override + @Nullable IFieldInstanceAbsolute getFieldInstanceByName(Integer name); @@ @Override + @NonNull Collection<? extends IAssemblyInstanceAbsolute> getAssemblyInstances(); @@ @Override + @Nullable IAssemblyInstanceAbsolute getAssemblyInstanceByName(Integer name);Also consider tweaking the Javadoc to avoid “name (index)” ambiguity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (144)
core/src/main/java/gov/nist/secauto/metaschema/core/configuration/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/MetaschemaDataTypeProvider.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/NcNameAdapter.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/QNameAdapter.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/object/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/mdm/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/mdm/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/IErrorCode.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/StaticContext.java(0 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/antlr/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/logic/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/math/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/format/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionMetapathError.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IArgument.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionLibrary.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/InvalidValueForCastFunctionException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/JsonFunctionException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/UnidentifiedFunctionError.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/library/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/regex/RegexUtil.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/regex/RegularExpressionMetapathException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/regex/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractMetapathExpression.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractSequence.java(0 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/ISequence.java(0 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/ItemUtils.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/IMapKey.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractDefinitionNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractFlagInstanceNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractInstanceNodeItem.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractNodeItemFactory.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractOrphanedDefinitionNodeItem.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitor.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IAssemblyInstanceGroupedNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IAtomicValuedNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IDocumentBasedNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IFeatureNoDataAtomicValuedItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IModelNodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/INodeItem.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/INodeItemGenerator.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/INodeItemVisitable.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/NodeItemKind.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitor.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/InvalidTypeMetapathException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractLoader.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractNamedInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractResourceResolver.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/FlagContainerBuilder.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IAssemblyDefinition.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IAssemblyInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IAssemblyInstanceAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IAssemblyInstanceGrouped.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IConstraintLoader.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainer.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelAssembly.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelGrouped.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IDefinition.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IDescribable.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureContainerModelAssembly.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureContainerModelGrouped.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureDefinitionReferenceInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFeatureValueless.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldDefinition.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldInstanceAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldInstanceGrouped.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFlagContainerBuilder.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFlagDefinition.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFlagInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IInstanceAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IJsonInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IJsonNamed.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/ILoader.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IMetapathQueryable.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IMetaschemaData.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IModelElementVisitable.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IModelInstanceAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IModuleLoader.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/INamed.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstanceAbsolute.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IUriResolver.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IValuedInstance.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonGroupAsBehavior.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/MetaschemaException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/ModelType.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/XmlGroupAsBehavior.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ConstraintValidationException.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidator.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ModelTargetedConstraints.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/DefaultDiagramNode.java(3 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/IDiagramNode.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/IDiagramNodeVisitor.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/ModuleUtils.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/XmlUtil.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/IValidationResult.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/JsonSchemaContentValidator.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/qname/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java(4 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/util/package-info.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/ExpressionUtilsTest.java(3 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/TestUtils.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/RecursionCollectingNodeItemVisitorTest.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/util/MermaidErDiagramGeneratorTest.java(1 hunks)pom.xml(1 hunks)
💤 Files with no reviewable changes (3)
- core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/ISequence.java
- core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractSequence.java
- core/src/main/java/gov/nist/secauto/metaschema/core/metapath/StaticContext.java
🧰 Additional context used
🧠 Learnings (14)
📚 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:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IAtomicValuedNodeItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/INodeItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IFeatureNoDataAtomicValuedItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/logic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/package-info.java
📚 Learning: 2024-11-14T17:09:05.819Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java:116-124
Timestamp: 2024-11-14T17:09:05.819Z
Learning: In the interface `INonNegativeIntegerItem` (file `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java`), the casting logic in the `cast` method is intentionally designed this way due to earlier discrepancies.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IAtomicValuedNodeItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IFeatureNoDataAtomicValuedItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/InvalidValueForCastFunctionException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/function/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/INodeItemGenerator.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/InvalidTypeMetapathException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java
📚 Learning: 2024-11-28T04:53:23.842Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 266
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/DynamicFunctionCall.java:73-77
Timestamp: 2024-11-28T04:53:23.842Z
Learning: In the `DynamicFunctionCall` class in `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/DynamicFunctionCall.java`, it's acceptable to rely on `toAtomicItem()` to throw an exception if the specifier is not an `IAtomicItem`, rather than checking the instance type before calling `toAtomicItem()`.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IAtomicValuedNodeItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/MetaschemaDataTypeProvider.javacore/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.javacore/src/main/java/gov/nist/secauto/metaschema/core/datatype/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/type/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.java
📚 Learning: 2024-11-14T17:08:15.227Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java:66-67
Timestamp: 2024-11-14T17:08:15.227Z
Learning: In the `INonNegativeIntegerItem.valueOf(NonNull INumericItem value)` method (file: `core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/INonNegativeIntegerItem.java`), the loss of precision when converting an `INumericItem` to an integer using `value.asInteger()` is intentional, as this method is meant for converting the provided numeric into an integer value.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.java
📚 Learning: 2024-11-14T23:37:29.087Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/DateAdapter.java:41-48
Timestamp: 2024-11-14T23:37:29.087Z
Learning: In the `DateAdapter` class (`core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/DateAdapter.java`), using a regular expression for date validation is necessary because `DateTimeFormatter` does not properly parse dates with timezones in this context.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/datatype/object/package-info.java
📚 Learning: 2025-01-07T00:51:17.257Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/ArraySequenceConstructor.java:63-63
Timestamp: 2025-01-07T00:51:17.257Z
Learning: In the Metapath implementation, expression evaluation must go through the `accept()` method rather than calling `evaluate()` directly, as `accept()` handles the push/pop operations on the execution stack which is used for debugging and error reporting.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractMetapathExpression.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2025-01-07T00:50:46.218Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java:87-88
Timestamp: 2025-01-07T00:50:46.218Z
Learning: In the Metapath implementation, child expressions should be called using `accept()` rather than `evaluate()` to ensure proper management of the execution stack through push/pop operations. The `evaluate()` method is an internal implementation detail that performs the actual evaluation.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/impl/AbstractMetapathExpression.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2024-11-14T05:15:46.640Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NonNegativeIntegerItemImpl.java:24-31
Timestamp: 2024-11-14T05:15:46.640Z
Learning: In the `NonNegativeIntegerItemImpl` class (`core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NonNegativeIntegerItemImpl.java`), the validation for non-negative values is performed in the `INonNegativeIntegerItem.valueOf(BigInteger)` method. Therefore, it's not necessary to duplicate this validation in the constructor of `NonNegativeIntegerItemImpl`.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractInstanceNodeItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/node/IFeatureNoDataAtomicValuedItem.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java
📚 Learning: 2025-12-08T16:52:33.199Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:52:33.199Z
Learning: When adding new constraint types, update the constraint type lists for `define-flag`, `define-field`, and `define-assembly` in `schema/metaschema/metaschema-module-constraints.xml` and add a new section with syntax table and examples in `website/content/specification/syntax/constraints.md`.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraintValidator.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/ModelTargetedConstraints.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java
📚 Learning: 2024-11-14T05:20:24.045Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IYearMonthDurationItem.java:87-94
Timestamp: 2024-11-14T05:20:24.045Z
Learning: In the `IYearMonthDurationItem` interface (`core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IYearMonthDurationItem.java`), the `cast` method uses `item.asString()` as intended. Do not suggest replacing it with `FunctionUtils.toString(item)` in this context.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/CastFunctionException.java
📚 Learning: 2025-12-08T16:52:33.199Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:52:33.199Z
Learning: Documentation for datatypes in `website/content/specification/datatypes.md` must stay in sync with schema files (`schema/json/metaschema-datatypes.json` and `schema/xml/metaschema-datatypes.xsd`).
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/package-info.javacore/src/main/java/gov/nist/secauto/metaschema/core/datatype/package-info.java
📚 Learning: 2025-12-08T16:52:33.199Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:52:33.199Z
Learning: When adding or modifying datatypes, update three files in sync: `schema/json/metaschema-datatypes.json` (JSON Schema definition), `schema/xml/metaschema-datatypes.xsd` (XML Schema definition), and `website/content/specification/datatypes.md` (documentation).
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/package-info.java
📚 Learning: 2025-12-08T16:52:33.199Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:52:33.199Z
Learning: Applies to {schema/json/metaschema-datatypes.json,schema/xml/metaschema-datatypes.xsd} : Regex patterns in datatype definitions must be valid for both ECMA-262 (JSON Schema) and XML Schema dialects. Both support Unicode property escapes (`\p{L}`, `\p{N}`).
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/regex/package-info.java
🧬 Code graph analysis (5)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/NcNameAdapter.java (3)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/MetaschemaDataTypeProvider.java (1)
SuppressWarnings(17-249)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java (1)
SuppressWarnings(18-38)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnAvg.java (1)
SuppressWarnings(42-239)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/MetaschemaDataTypeProvider.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/NcNameAdapter.java (1)
SuppressWarnings(24-70)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java (1)
SuppressWarnings(18-38)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/impl/NcNameItemImpl.java (2)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/MetaschemaDataTypeProvider.java (1)
SuppressWarnings(17-249)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/adapter/NcNameAdapter.java (1)
SuppressWarnings(24-70)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/item/node/AbstractRecursionPreventingNodeItemVisitorTest.java (1)
core/src/test/java/gov/nist/secauto/metaschema/core/testsupport/MockedModelTestSupport.java (1)
MockedModelTestSupport(15-25)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java
Show resolved
Hide resolved
...java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.java
Show resolved
Hide resolved
core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelGrouped.java
Show resolved
Hide resolved
core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/package-info.java
Show resolved
Hide resolved
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java
Show resolved
Hide resolved
- Fix HTML tag formatting in flexmark package-info.java files - Simplify ISequence access pattern in FnDataTest - Remove unnecessary deprecation suppression in DefaultBindingContext
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/DefaultBindingContext.java (1)
272-277: Deprecation warning will now surface for the finalize() method.Removing the
"deprecation"suppression is a reasonable cleanup decision. The emptyfinalize()method follows SEI CERT OBJ-11 to prevent finalizer attacks in this non-final class with a throwing constructor.If you plan to eliminate the finalize() method entirely, consider making the class
final—no subclasses currently exist and all consumers reference theIBindingContextinterface rather than the concrete class, making this API-safe. However, this refactoring is optional and separate from this cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/package-info.java(1 hunks)core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDataTest.java(1 hunks)databind/src/main/java/gov/nist/secauto/metaschema/databind/DefaultBindingContext.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/package-info.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 (7)
core/src/test/java/gov/nist/secauto/metaschema/core/metapath/function/library/FnDataTest.java (1)
105-107: LGTM! Refactor correctly addresses checkstyle warning.The direct access to
result.get(i)eliminates the intermediate list variable, resolving the VariableDeclarationUsageDistance checkstyle warning mentioned in the PR objectives. The assertions remain functionally equivalent and correctly validate the atomized sequence values.core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java (6)
1-5: LGTM!The SPDX license headers are properly formatted and appropriate for documentation.
6-18: LGTM!The package overview and Configuration section are well-documented and clearly describe the purpose of this internal implementation package.
19-27: LGTM!The Visitors section clearly documents the visitor pattern implementation for AST traversal.
28-39: LGTM!The Writers section comprehensively documents the markup writing interfaces and implementations.
52-80: LGTM!The Design Patterns, Thread Safety, and Usage Context sections provide excellent architectural documentation and clearly explain the package's role and relationships.
82-86: All@linkreferences in the Javadoc are valid. Every referenced class (FlexmarkConfiguration, IMarkupVisitor, MarkupVisitor, AstCollectingVisitor, IMarkupWriter, AbstractMarkupWriter, MarkupXmlStreamWriter, MarkupXmlEventWriter, HtmlCodeRenderExtension, SuppressPTagExtension, FixedEmphasisDelimiterProcessor) exists as a Java file in this package. There are no broken Javadoc links.Likely an incorrect or invalid review comment.
...c/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java
Outdated
Show resolved
Hide resolved
- Fix IFunctionResolver Javadoc to remove incorrect null return mention (method is @nonnull and throws exception on no match) - Fix InvalidTreatTypeDynamicMetapathException to register the evaluationStack parameter by calling registerEvaluationContext() - Add registerEvaluationContext(Deque) method to MetapathException for registering evaluation stack directly - Fix IContainerModelGrouped Javadoc to correctly describe Collection return types instead of "ordered mapping" - Fix validation/package-info.java to link to IConstraint type instead of package (avoids doclint warnings) - Add Objects.requireNonNull in ExceptionUtils.wrap() and have wrapAndThrow() delegate to wrap() to avoid duplication - Fix flexmark/impl/package-info.java Javadoc formatting for SuppressPTagExtension
There was a problem hiding this comment.
Actionable comments posted: 0
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/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
138-138: Fix typo in exception message."excpeted" should be "expected".
Apply this diff:
- String.format("Wrapped exception '%s' did not match excpeted type '%s'.", + String.format("Wrapped exception '%s' did not match expected type '%s'.",
♻️ Duplicate comments (3)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java (1)
27-33: Doc/annotation contract is now consistent: non-null return, and miss indicated via exception.
This also addresses the earlier review concern about “or null” conflicting with@NonNull.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.java (1)
24-38: Good fix:evaluationStackis now actually registered on the exception.
This addresses the earlier issue where the new ctor parameter could be silently ignored.core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java (1)
42-45: MakeSuppressPTagExtensionbehavior explicit (mention literal<p>safely).
Current text says “Suppresses paragraph tags”; consider explicitly calling out the HTML tag with{@literal <p>}to avoid ambiguity and keep Javadoc HTML-safe.- * <li>{@link SuppressPTagExtension} - Suppresses paragraph tags in single-line markup content</li> + * <li>{@link SuppressPTagExtension} - Suppresses {@literal <p>} paragraph tags in single-line markup content</li>
🧹 Nitpick comments (4)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java (1)
19-30: Fix typo/ambiguity: “signature of the provided methods” doesn’t make sense here.
Suggest rewording to something like “supports the signature with the provided arity” (or “provided arguments”).- * Retrieve the function with the provided name that supports the signature of - * the provided methods. + * Retrieve the function with the provided name that supports the signature + * for the provided arity.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java (2)
31-37: Consider makingevaluationStacktransient(exception serialization safety).
IfIExpressionisn’t serializable, serializingMetapathExceptioncan fail at runtime.- private Deque<IExpression> evaluationStack = null; + private transient Deque<IExpression> evaluationStack = null;
89-103: Snapshot the DynamicContext stack (avoid post-throw mutation / confusing diagnostics).
Right now this keeps a live reference (dynamicContext.getExecutionStack()), while the Deque overload snapshots. Consider aligning behavior.public MetapathException registerEvaluationContext(@NonNull DynamicContext dynamicContext) { if (evaluationStack == null) { - evaluationStack = dynamicContext.getExecutionStack(); + evaluationStack = new ArrayDeque<>(dynamicContext.getExecutionStack()); } return this; }core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
118-141: Document the side effect of adding to suppressed exceptions.The method adds the wrapper exception to the unwrapped exception's suppressed list (line 134), but this behavior is not mentioned in the Javadoc. This side effect should be documented so callers are aware of the mutation.
Consider updating the Javadoc to mention this:
/** * Get the wrapped exception, casting it to the expected type. + * <p> + * This method adds the wrapper exception to the unwrapped exception's + * suppressed exceptions list for debugging purposes. * * @param <E> * the expected exception type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelGrouped.java(4 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/src/main/java/gov/nist/secauto/metaschema/core/model/validation/package-info.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-01-07T00:51:17.257Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/ArraySequenceConstructor.java:63-63
Timestamp: 2025-01-07T00:51:17.257Z
Learning: In the Metapath implementation, expression evaluation must go through the `accept()` method rather than calling `evaluate()` directly, as `accept()` handles the push/pop operations on the execution stack which is used for debugging and error reporting.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2025-01-07T00:50:46.218Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java:87-88
Timestamp: 2025-01-07T00:50:46.218Z
Learning: In the Metapath implementation, child expressions should be called using `accept()` rather than `evaluate()` to ensure proper management of the execution stack through push/pop operations. The `evaluate()` method is an internal implementation detail that performs the actual evaluation.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.javacore/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java
🧬 Code graph analysis (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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). (1)
- GitHub Check: Code
🔇 Additional comments (11)
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/IFunctionResolver.java (1)
13-15: New interface Javadoc is clear and matches the resolver’s responsibility.core/src/main/java/gov/nist/secauto/metaschema/core/model/IContainerModelGrouped.java (2)
13-19: Well-written class documentation.The class-level Javadoc clearly explains the concept of grouped instances and the interface's role in narrowing return types. This provides valuable context for API consumers.
34-34: Past review feedback successfully incorporated.The simplified
@returndescriptions accurately reflect theCollection<?>return types and consistently remove the misleading "ordered mapping" and "ordered collection" terminology across all three methods.Also applies to: 56-56, 78-78
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/InvalidTreatTypeDynamicMetapathException.java (1)
13-18: Javadoc clarification looks correct and aligns with XPath error mapping.
No concerns.core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java (3)
105-119: Deque overload snapshots context—good.
This is the safest of the overloads (stable diagnostics).
121-135: Single-expression context capture looks good.
No concerns.
137-145: Getter scope (protected) is appropriate.
No concerns.core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (3)
20-45: Previous feedback addressed correctly.The null-check with
Objects.requireNonNullhas been added, andwrapAndThrownow properly delegates towrap. These changes resolve the concerns raised in the previous review.
47-78: LGTM!The static unwrap methods provide convenient access to the instance methods and are properly documented.
143-165: LGTM!The
unwrapAndThrowmethods provide convenient helpers for unwrapping and re-throwing exceptions, and are properly documented.core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java (1)
79-80: The@seereferences to package names at lines 79-80 are valid and consistent with how the codebase documents related packages. The parentflexmark/package-info.javasuccessfully uses@linkreferences to the impl package, demonstrating that javadoc resolution works correctly in this build. Both referenced packages are part of the same documentation build, so package-level@seetags should resolve without warnings.Additionally, the suggested replacement class name
FlexmarkMarkupParserdoes not exist in the codebase, making the proposed fix incorrect.Likely an incorrect or invalid review comment.
- Make registerEvaluationContext methods final to fix PMD ConstructorCallsOverridableMethod violation - Add documentation clarifying snapshot behavior to avoid post-throw mutation confusion - Fix typo 'excpeted' -> 'expected' in ExceptionUtils.java - Fix typo 'idenfied' -> 'identified' in JsonValueKeyTypeEnum.java - Fix typo 'assemvly' -> 'assembly' in ModuleUtils.java - Fix grammar 'who's' -> 'whose' in 5 model files
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java (1)
20-23: FLAG Javadoc grammar fix looks good; optional tighten for precision.
If you want it a bit crisper: “A flag is used as the value key; its value will be used.”core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java (1)
69-69: Optional: Consider clarifying the return documentation.The Javadoc states
{@code null}is returned "if a JSON key is configured", but the logic suggests it should returnnullwhen a JSON key is not configured. While this is pre-existing and not part of your changes, you may want to correct it in this documentation-focused PR.Apply this diff if you choose to address it:
- * @return the JSON key flag instance or {@code null} if a JSON key is - * configured + * @return the JSON key flag instance or {@code null} if no JSON key is + * configuredcore/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (2)
8-9: Consider also null-guardingWrappedException’s public constructor (not justwrap).
wrap(...)now fails fast, but callers can still donew WrappedException(null)directly and later trip overgetCause()assumptions.public WrappedException(@NonNull Throwable cause) { - super(cause); + super(Objects.requireNonNull(cause, "cause")); }Also applies to: 28-30
129-141: Potential duplicate/noisy suppressed exceptions on repeated unwraps.
If the same wrapped cause is unwrapped multiple times,addSuppressed(this)can stack up duplicates on the original exception.Optional guard:
E unwrappedEx = wrappedExceptionClass.cast(cause); -unwrappedEx.addSuppressed(this); +boolean alreadySuppressed = java.util.Arrays.stream(unwrappedEx.getSuppressed()) + .anyMatch(s -> s == this); +if (!alreadySuppressed) { + unwrappedEx.addSuppressed(this); +} return unwrappedEx;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldDefinition.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/IModelDefinition.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/impl/DefaultContainerFlagSupport.java(1 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/model/util/ModuleUtils.java(2 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/package-info.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-01-07T00:51:17.257Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/ArraySequenceConstructor.java:63-63
Timestamp: 2025-01-07T00:51:17.257Z
Learning: In the Metapath implementation, expression evaluation must go through the `accept()` method rather than calling `evaluate()` directly, as `accept()` handles the push/pop operations on the execution stack which is used for debugging and error reporting.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2025-01-07T00:50:46.218Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 336
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java:87-88
Timestamp: 2025-01-07T00:50:46.218Z
Learning: In the Metapath implementation, child expressions should be called using `accept()` rather than `evaluate()` to ensure proper management of the execution stack through push/pop operations. The `evaluate()` method is an internal implementation detail that performs the actual evaluation.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java
🧬 Code graph analysis (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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 (15)
core/src/main/java/gov/nist/secauto/metaschema/core/model/JsonValueKeyTypeEnum.java (1)
8-10: Enum-level Javadoc addition is clear and accurate.
No functional impact; improves API discoverability.core/src/main/java/gov/nist/secauto/metaschema/core/model/util/ModuleUtils.java (2)
16-19: LGTM!Clear and concise class-level documentation that accurately describes the utility's purpose.
44-55: LGTM!Good catch on the typo fix ("assemvly" → "assembly"). The Javadoc now accurately describes the method's purpose.
core/src/main/java/gov/nist/secauto/metaschema/core/model/IFieldDefinition.java (2)
11-17: Excellent interface documentation!The Javadoc clearly explains the purpose of
IFieldDefinitionand its key concepts (flags, value type, JSON serialization). The documentation aligns well with the interface's methods and provides helpful context for users of this API.
57-57: Good catch on the grammar fix.Correctly changed "who's" to "whose" for the possessive form.
core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java (2)
13-18: LGTM! Clear and concise interface documentation.The interface-level Javadoc accurately describes the purpose of
INamedModelInstanceand highlights its key responsibilities: providing access to the instance's definition and JSON key configuration.
50-50: Good grammar correction.The change from "who's" to "whose" is grammatically correct and improves the documentation quality.
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (3)
32-45:wrapAndThrowdelegating towrapis clean and avoids drift.
47-78: Unwrap helpers are straightforward delegations.
103-106: WrappedException behavior is coherent (cause locked-in; unwrap/throw helpers are clear).Also applies to: 143-165
core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathException.java (5)
31-36: LGTM: Clear field documentation.The Javadoc clearly explains the purpose of the
evaluationStackfield for capturing the evaluation context when exceptions occur.
89-106: LGTM: Solid implementation with defensive copying.The method correctly implements:
- First-writer-wins pattern to capture the original exception context
- Defensive copying via
dynamicContext.getExecutionStack()- Method chaining for fluent API usage
- Final modifier to prevent subclass override
The documentation clearly explains the snapshot behavior and prevents confusion about post-throw mutations.
108-122: LGTM: Well-designed overload with proper defensive copying.The new overload correctly:
- Creates a defensive copy using
new ArrayDeque<>(stack)- Uses wildcard
<? extends IExpression>for flexibility- Maintains consistency with other overloads (first-writer-wins, final, chaining)
The documentation clearly explains the snapshot behavior.
124-138: LGTM: Clear documentation for single-expression overload.The enhanced documentation clearly explains how a single metapath expression is recorded as the evaluation context. The implementation correctly creates a defensive copy via
new ArrayDeque<>(Collections.singleton(metapath)).
140-148: LGTM: Clear documentation for protected getter.The Javadoc clearly documents the getter's purpose and nullable return. The protected visibility is appropriate for allowing subclasses to access the evaluation stack while keeping it encapsulated from external callers.
core/src/main/java/gov/nist/secauto/metaschema/core/model/IModelDefinition.java
Show resolved
Hide resolved
core/src/main/java/gov/nist/secauto/metaschema/core/model/impl/DefaultContainerFlagSupport.java
Show resolved
Hide resolved
- Fix INamedModelInstance Javadoc: null returned when no JSON key configured - Add null-guard to WrappedException constructor for fail-fast behavior - Prevent duplicate suppressed exceptions on repeated unwraps
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java(3 hunks)core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/gov/nist/secauto/metaschema/core/model/INamedModelInstance.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-14T17:07:03.586Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/item/atomic/IIPv4AddressItem.java:66-73
Timestamp: 2024-11-14T17:07:03.586Z
Learning: In the Metaschema Java codebase, differences in casting patterns across atomic type implementations are intentional and required; any differences in approach are significant and necessary.
Applied to files:
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java
🧬 Code graph analysis (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
⏰ 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 (7)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java (7)
8-9: LGTM! Necessary imports.The
ArraysandObjectsimports are properly used throughout the file for stream operations and null validation.
21-31: LGTM! Previous review feedback addressed.The
wrapmethod now enforces the non-null contract at runtime withObjects.requireNonNulland includes comprehensive Javadoc.
33-46: LGTM! Previous review feedback addressed.The
wrapAndThrowmethod now delegates towrap(ex)to avoid code duplication and includes clear Javadoc explaining its purpose.
48-79: LGTM! Clean API design.The static
unwrapmethods provide convenient delegation to instance methods with clear documentation.
81-102: LGTM! Constructor properly enforces non-null constraint.The
WrappedExceptionclass includes comprehensive documentation and the constructor correctly validates the cause parameter withObjects.requireNonNull.
109-117: LGTM! Appropriate use of ObjectUtils.notNull.The basic
unwrap()method correctly usesObjectUtils.notNullsincegetCause()is guaranteed non-null by the constructor's validation.
149-171: LGTM! Convenient throw helpers.The
unwrapAndThrowmethods provide expressive convenience wrappers for unwrap-and-throw operations with clear documentation.
core/src/main/java/gov/nist/secauto/metaschema/core/util/ExceptionUtils.java
Show resolved
Hide resolved
david-waltermire
merged commit fe7ef2b
into
metaschema-framework:develop
1 participant
Summary
-J-Dfile.encoding=UTF-8)Changes
Javadoc Documentation (99 files)
Package Documentation (43 files)
Build Fixes
additionalJOption-J-Dfile.encoding=UTF-8to maven-javadoc-pluginCheckstyle Fixes
Test plan
mvn clean install -PCI -Prelease -DskipTestspassesmvn -pl core checkstyle:checkpasses with 0 violationsmvn -pl core javadoc:javadocgenerates without encoding errorsSummary by CodeRabbit
Documentation
New Features
API Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.