feat: add complete Javadoc coverage to schemagen, databind, and maven-plugin modules#601
Conversation
Add Javadoc documentation to all public/protected methods and create package-info.java files for all packages in the schemagen module. - Document 35 source files with method-level Javadoc - Add 7 package-info.java files with package descriptions - Achieves 0 Checkstyle MissingJavadocMethod violations
Add Javadoc documentation to all public/protected methods and create/enhance package-info.java files across databind and maven-plugin modules. databind module: - Document codegen, io, and model packages with method-level Javadoc - Enhance package-info.java files with detailed descriptions - Excludes generated binding classes per project conventions metaschema-maven-plugin module: - Document all Mojo classes and inner classes - Add package-info.java with plugin goal documentation Achieves 0 Checkstyle MissingJavadocMethod violations in target packages.
📝 WalkthroughWalkthroughAdds PRD and implementation-plan documents for full Javadoc coverage and applies widespread API additions and Javadoc across modules (databind, schemagen, metaschema-maven-plugin). Introduces new module post-processing loader, enhanced code-generation and compilation support, expanded schema-generation state/APIs, and many package-level docs. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (49)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/DefaultGroupAs.java (1)
33-46: Add Javadoc documentation to the public constructor.The public constructor lacks Javadoc documentation. Per the PR objective to achieve 100% Javadoc coverage on public members and per the coding guidelines, the constructor should be documented with
@paramand@throwstags describing the parameters and any exceptions.🔎 Suggested Javadoc for the constructor
+ /** + * Constructs a DefaultGroupAs instance with the specified annotation configuration. + * + * @param annotation + * the {@link GroupAs} annotation containing the group-as configuration + * @param module + * the {@link IModule} context for resolving the group name + * @throws IllegalStateException + * if the group name from the annotation resolves to null + */ @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Use of final fields") public DefaultGroupAs( @NonNull GroupAs annotation, @NonNull IModule module) {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/AbstractModelInstanceCollectionInfo.java (1)
28-31: Add Javadoc for the public constructor.The public constructor lacks documentation despite the PR's goal of eliminating Checkstyle MissingJavadocMethod violations. As a public member, it requires comprehensive Javadoc per the coding guidelines.
🔎 Proposed fix to add constructor Javadoc
@NonNull private final IBoundInstanceModel<ITEM> instance; + /** + * Constructs an instance with the provided bound model instance. + * + * @param instance + * the bound model instance managing the collection + */ public AbstractModelInstanceCollectionInfo( @NonNull IBoundInstanceModel<ITEM> instance) { this.instance = instance; }databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingConstraintLoader.java (1)
51-65: Remove duplicate class-level Javadoc block.The class has two Javadoc blocks—one at lines 51–58 and another at lines 59–65. Java permits only one class-level Javadoc comment; the second will be ignored by the compiler and creates confusion. The first block (51–58) is more detailed and informative, describing caching behavior and import handling. Remove the duplicate second block (59–65).
🔎 Proposed fix to remove duplicate Javadoc
/** * Provides methods to load a constraint set expressed in any supported * Metaschema format. * <p> * Loaded constraint instances are cached to avoid the need to load them for * every use. Any constraint set imported is also loaded and cached * automatically. */ -/** - * Loads Metaschema constraints from external constraint files using data - * binding. - * <p> - * This class provides functionality to parse constraint files and apply them to - * Metaschema modules. - */ public class BindingConstraintLoader extends AbstractLoader<List<IConstraintSet>> implements IConstraintLoader {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/AbstractAbsoluteModelGenerator.java (1)
28-49: Remove duplicate Javadoc block for the class declaration.The class has two consecutive Javadoc blocks (lines 28–40 and lines 44–49). Java allows only one Javadoc block immediately preceding a class declaration; the second block will be ignored by the Javadoc tool or cause documentation generation issues. Retain the original, more comprehensive Javadoc (lines 28–40) which documents the purpose, thread safety, and generic type parameters. Remove the redundant block at lines 44–49.
🔎 Proposed fix
/** * Supports generating the model contents of a Metaschema instance with a * complex structure. * <p> * This class is not thread safe. * * @param <PARENT> * the Java type of the parent Metaschema object that contains the * generated model * @param <BUILDER> * the Java type of the builder to use to gather the container * instances */ @SuppressWarnings({ "PMD.AbstractClassWithoutAbstractMethod", "PMD.UseConcurrentHashMap" }) -/** - * Abstract base class for generating absolute model structures from bindings. - * <p> - * This class provides common functionality for building model containers from - * Metaschema module bindings. - */ public abstract class AbstractAbsoluteModelGenerator<databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/BoundInstanceModelChoice.java (1)
27-39: Critical: Duplicate Javadoc blocks will prevent compilation.The code contains two consecutive Javadoc blocks (lines 27–33 and lines 34–39) for the same class. Java allows only one Javadoc comment immediately before a class declaration. The second block would be interpreted as a regular comment, causing the class to lack proper Javadoc documentation. The build will fail with a compilation error.
The intent appears to be replacing the original Javadoc with updated documentation, but the old block was not removed. You must remove the first Javadoc block (lines 27–33) to keep only the new one.
🔎 Proposed fix
Remove the first Javadoc block:
-/** - * Represents a choice instance for annotation-based bound definitions. - * <p> - * A choice contains mutually exclusive model instance alternatives. This class - * is used for annotation-based bindings (classes with {@code @BoundChoice} - * annotations). - */ /** * Implementation of a choice instance within a bound model. * <p> * This class represents a choice between multiple model instances in a * Metaschema assembly. */databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/MapCollectionInfo.java (3)
36-38: Public constructor missing Javadoc documentation.The public constructor lacks Javadoc, violating the PR objective to achieve 100% Javadoc coverage on all public/protected members and eliminate Checkstyle MissingJavadocMethod violations. Per the coding guidelines, all public members must be documented.
🔎 Proposed Javadoc for the constructor
extends AbstractModelInstanceCollectionInfo<ITEM> { + /** + * Constructs a new {@code MapCollectionInfo} for the provided bound instance model. + * + * @param instance + * the bound instance model for the map-based collection + */ public MapCollectionInfo(@NonNull IBoundInstanceModel<ITEM> instance) { super(instance); }
56-62: Public methodgetKeyType()missing Javadoc documentation.This public method lacks Javadoc documentation, violating the PR objective and coding guidelines requiring 100% Javadoc coverage on public members. The method extracts the key type from the parameterized Map type and should be documented.
🔎 Proposed Javadoc for getKeyType()
} + /** + * Gets the Java type for map keys. + * + * @return + * the key type of this map-based collection + */ @SuppressWarnings("null") @NonNull public Class<?> getKeyType() {
69-75: Public methodgetValueType()missing Javadoc documentation.This public method lacks Javadoc documentation, violating the PR objective and coding guidelines. The method retrieves the value type from the parameterized Map and should be documented to clarify its purpose and return value.
🔎 Proposed Javadoc for getValueType()
} + /** + * Gets the Java type for map values. + * + * @return + * the value type of this map-based collection + */ @SuppressWarnings({ "null", "unchecked" }) @NonNull public Class<? extends ITEM> getValueType() {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IItemValueHandler.java (1)
16-31: Remove duplicate Javadoc: only one comment block should precede the interface declaration.The interface has two Javadoc blocks (lines 16–22 and 23–31), but Java only allows a single Javadoc comment immediately before a class, interface, or method. This creates ambiguity about which documentation javadoc will process and violates Java conventions.
Retain the more detailed second block (lines 23–31) and remove the first (lines 16–22).
🔎 Proposed fix
-/** - * A feature interface for handling read, writing, and copying item objects, - * which are the data building blocks of a Metaschema module instance. - * - * @param <TYPE> - * the Java type of the item - */ /** * Handler interface for processing bound item values. * <p> * This interface provides methods for reading and writing items, as well as * deep copying items during binding operations. * * @param <TYPE> * the Java type of the handled item value */databind/src/main/java/gov/nist/secauto/metaschema/databind/model/IBoundFieldValue.java (1)
32-38: Fix typo in Javadoc.Line 33 contains a grammatical error: "contain's" should be "contains" (no apostrophe).
🔎 Proposed fix
/** - * Get the field definition that contain's the field value. + * Get the field definition that contains the field value. * * @return the parent field definition */databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceFlagReference.java (2)
54-69: Public constructor is missing Javadoc documentation.The constructor accepts 5 parameters but lacks any documentation. According to the coding guidelines, all public/protected members must have 100% Javadoc coverage with @param, @return, and @throws tags in the correct order (BLOCKING requirement).
🔎 Proposed Javadoc documentation for the constructor
+ /** + * Constructs an InstanceFlagReference with the provided binding and definition data. + * + * @param binding the flag reference binding data (must not be null) + * @param bindingInstance the parent bound instance model grouped assembly (must not be null) + * @param position the position of this reference within the parent instance + * @param definition the flag definition being referenced (must not be null) + * @param parent the parent binding definition model (must not be null) + */ public InstanceFlagReference( @NonNull FlagReference binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IFlagDefinition definition, @NonNull IBindingDefinitionModel parent) {
71-74: Protected method is missing Javadoc documentation.The
getBinding()method lacks documentation despite being a protected member. This should include a @return tag describing what binding object is returned.🔎 Proposed Javadoc documentation for the method
+ /** + * Gets the flag reference binding data. + * + * @return the flag reference binding (never null) + */ @NonNull protected FlagReference getBinding() { return binding; }databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/SingletonCollectionInfo.java (1)
32-34: Public constructor lacks Javadoc documentation.Per the PR objectives to add Javadoc to all public/protected members, and per coding guidelines requiring 100% Javadoc coverage on public members, the public constructor should be documented. The constructor accepts a required non-null parameter and may throw BindingException indirectly through inherited behavior.
🔎 Proposed Javadoc for the constructor
} + /** + * Create a new singleton collection information instance. + * + * @param instance + * the bound instance model for the singleton item + */ public SingletonCollectionInfo(@NonNull IBoundInstanceModel<ITEM> instance) { super(instance); }databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/ChoiceModelGenerator.java (1)
31-41: Remove duplicate Javadoc blocks.There are two consecutive Javadoc blocks before the class declaration. Java will only recognize the last one (lines 36-41), causing the important thread-safety note from the first block (lines 31-35) to be lost. Merge both blocks into a single comprehensive Javadoc that preserves all information.
🔎 Proposed fix merging both Javadoc blocks
-/** - * Supports building the model contents of a Metaschema choice instance. - * <p> - * This class is not thread safe. - */ -/** - * Generates choice model structures from binding data. - * <p> - * This class handles the creation of choice containers for assemblies that - * contain exclusive model alternatives. - */ +/** + * Generates choice model structures from binding data. + * <p> + * This class handles the creation of choice containers for assemblies that + * contain exclusive model alternatives. It supports building the model contents + * of a Metaschema choice instance. + * <p> + * This class is not thread safe. + */databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/AssemblyModelGenerator.java (1)
33-44: Merge duplicate Javadoc blocks and preserve thread-safety note.Two Javadoc blocks exist before the class declaration. In Java, only the Javadoc immediately preceding a declaration is associated with it. The first Javadoc (lines 33-37) is orphaned by the
@SuppressWarningsannotation and will not be included in generated documentation. Additionally, the important thread-safety note from the original Javadoc is being lost.🔎 Proposed fix to merge the Javadoc blocks
-/** - * Supports building the model contents of a Metaschema assembly definition. - * <p> - * This class is not thread safe. - */ @SuppressWarnings("PMD.UseConcurrentHashMap") /** - * Generates assembly model structures from binding data. + * Supports building the model contents of a Metaschema assembly definition. * <p> - * This class handles the creation of model containers for assembly definitions - * loaded from Metaschema sources. + * Generates assembly model structures from binding data, handling the creation + * of model containers for assembly definitions loaded from Metaschema sources. + * <p> + * This class is not thread safe. */ public final class AssemblyModelGeneratorAs per coding guidelines, all Javadoc must follow the style guide and provide complete coverage.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IBindingModelElement.java (1)
23-24: Missing required Javadoc on new public method.The new
getSourceNodeItem()method lacks Javadoc documentation. Per coding guidelines, all new public/protected members require 100% Javadoc coverage with appropriate@returntags (BLOCKING).🔎 Proposed fix
+ /** + * Gets the source node item for this model element. + * + * @return the assembly node item representing the source of this model element + */ @NonNull IAssemblyNodeItem getSourceNodeItem();Based on coding guidelines: All new code requires 100% Javadoc coverage on public/protected members with @param, @return, @throws tags in correct order (BLOCKING).
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ClassIntrospector.java (1)
25-25: Add Javadoc to public utility methods.Methods
getMatchingMethods(line 25) andgetMatchingMethod(line 41) lack required Javadoc documentation. Both are public static methods and must include complete Javadoc with@param,@return, and@throwstags per coding guidelines.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelGroupedAssemblyReference.java (1)
51-65: Missing Javadoc on public constructor.The public constructor lacks Javadoc documentation, which contradicts the PR objective to eliminate all MissingJavadocMethod violations. According to the coding guidelines, all public/protected members require 100% Javadoc coverage with appropriate
@paramtags.🔎 Proposed fix
+ /** + * Constructs a new grouped assembly reference instance. + * + * @param binding + * the binding data for this assembly reference + * @param bindingInstance + * the bound instance model for the grouped assembly + * @param position + * the position of this instance within the group + * @param definition + * the assembly definition being referenced + * @param parent + * the containing choice group instance + */ public InstanceModelGroupedAssemblyReference( @NonNull AssemblyModel.ChoiceGroup.Assembly binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IAssemblyDefinition definition, @NonNull IChoiceGroupInstance parent) {As per coding guidelines, this is a BLOCKING issue that must be resolved to meet the PR's Javadoc coverage goals.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/FlagContainerSupport.java (1)
39-87: Public methodnewFlagContainermissing Javadoc documentation.This public static method lacks the required Javadoc with @param, @return, and @throws tags as mandated by the coding guidelines. The method has multiple parameters (some nullable), throws
UnsupportedOperationException, and complex logic that requires documentation. Add comprehensive Javadoc following the style guide:/** * Creates a new flag container from binding data. * <p> * Discovers and organizes flag instances from the provided binding list, * supporting both inline flag definitions and flag references. * * @param flags the list of flag binding objects to process; may be null or empty * @param bindingInstance the grouped assembly binding instance; must not be null * @param parent the parent binding definition model; must not be null * @param jsonKeyName the optional JSON key name for flag lookup; may be null * @return a flag container with organized flag instances; never null * @throws UnsupportedOperationException if an unknown flag instance type is encountered */databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelChoice.java (1)
51-67: Add Javadoc documentation to the public constructor.The public constructor lacks Javadoc documentation with
@paramtags for its five parameters. Per coding guidelines, all public/protected members require complete Javadoc coverage.🔎 Suggested Javadoc for the constructor
+ /** + * Construct a new choice instance from binding data. + * + * @param binding + * the binding object representing the choice + * @param bindingInstance + * the grouped assembly instance containing this choice + * @param position + * the position of this choice within the parent + * @param parent + * the parent assembly definition + * @param nodeItemFactory + * the factory for creating node items + */ public InstanceModelChoice( @NonNull AssemblyModel.Choice binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IBindingDefinitionModelAssembly parent, @NonNull INodeItemFactory nodeItemFactory) {Based on coding guidelines requiring 100% Javadoc coverage on public/protected members.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/InstanceModelChoiceGroup.java (1)
41-50: Remove duplicate class-level Javadoc.Two class-level Javadoc blocks are present (lines 41-43 and lines 45-50). The original Javadoc at lines 41-43 should be removed, leaving only the enhanced version at lines 45-50.
🔎 Proposed fix to remove the duplicate
-/** - * Implements a Metaschema module choice group instance bound to a Java field. - */ @SuppressWarnings("PMD.CouplingBetweenObjects") /** * Implementation of a choice group instance bound to a Java field. * <p> * This class handles the binding of a field that can contain multiple types of * model instances as members of a choice group. */ public final class InstanceModelChoiceGroupdatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/InstanceModelGroupedAssembly.java (1)
31-40: Remove duplicate class-level Javadoc.Two class-level Javadoc blocks are present (lines 31-34 and lines 35-40). The original Javadoc at lines 31-34 should be removed, leaving only the enhanced version at lines 35-40.
🔎 Proposed fix to remove the duplicate
-/** - * Represents an assembly model instance that is a member of a choice group - * instance. - */ /** * Implementation of an assembly instance within a choice group. * <p> * This class represents an assembly that is a member of a choice group, * allowing polymorphic model content. */ public class InstanceModelGroupedAssemblydatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelChoiceGroup.java (1)
57-75: Add Javadoc documentation to the public constructor.The public constructor lacks Javadoc documentation with
@paramtags for its five parameters. Per coding guidelines, all public/protected members require complete Javadoc coverage.🔎 Suggested Javadoc for the constructor
+ /** + * Construct a new choice group instance from binding data. + * + * @param binding + * the binding object representing the choice group + * @param bindingInstance + * the grouped assembly instance containing this choice group + * @param position + * the position of this choice group within the parent + * @param parent + * the parent assembly definition + * @param nodeItemFactory + * the factory for creating node items + */ public InstanceModelChoiceGroup( @NonNull AssemblyModel.ChoiceGroup binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IBindingDefinitionModelAssembly parent, @NonNull INodeItemFactory nodeItemFactory) {Based on coding guidelines requiring 100% Javadoc coverage on public/protected members.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelFieldInline.java (1)
77-116: Missing Javadoc on public constructor.The public constructor at line 77 lacks required Javadoc documentation, violating the coding guideline that requires 100% Javadoc coverage on public members. This would result in a Checkstyle MissingJavadocMethod violation, since the file is not excluded from checkstyle checks and the constructor is not marked with
@Override.Add a Javadoc block with
@paramtags for each parameter:/** * Constructs an inline field instance from binding data. * * @param binding the field binding definition * @param bindingInstance the bound grouped assembly instance * @param position the position of this field in its containing assembly * @param parent the containing model */ public InstanceModelFieldInline( @NonNull InlineDefineField binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IContainerModelAbsolute parent) {databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/JavaCompilerSupport.java (1)
136-146: Fix platform-specific path separator hardcoding.The path separator is hardcoded as
":"on lines 139 and 145, which only works on Unix-like systems. On Windows, the path separator is";". This will cause compilation failures when multiple classpath or module path entries are configured on Windows systems.🔎 Proposed fix using platform-independent separator
+import java.io.File; + ... if (!classPath.isEmpty()) { options.add("-classpath"); options.add(classPath.stream() - .collect(Collectors.joining(":"))); + .collect(Collectors.joining(File.pathSeparator))); } if (!modulePath.isEmpty()) { options.add("-p"); options.add(modulePath.stream() - .collect(Collectors.joining(":"))); + .collect(Collectors.joining(File.pathSeparator))); }databind/src/main/java/gov/nist/secauto/metaschema/databind/model/annotations/ModelUtil.java (4)
287-299: Missing Javadoc on public methodtoPropertyEntry(Property).This public static method at lines 287–299 lacks Javadoc documentation. It requires a complete Javadoc block with @param and @return tags to satisfy the 100% coverage requirement for new code.
Proposed Javadoc:
+ /** + * Convert a Property annotation to a map entry with an attributable key and set + * of string values. + * + * @param property + * the Property annotation to convert + * @return a map entry containing the property's attributable key and an + * unmodifiable set of property values + */ public static Map.Entry<IAttributable.Key, Set<String>> toPropertyEntry(@NonNull Property property) {
42-51: Add Javadoc to public static constants.Lines 42–43 define public static constants (
NO_STRING_VALUE,DEFAULT_STRING_VALUE) without Javadoc, which violates the 100% Javadoc coverage requirement for new code. The Javadoc style guide explicitly requires field documentation for all public members; this is blocking. The third constant (NULL_VALUE) is properly documented (lines 44–51), establishing an inconsistency.
277-285: Add missing Javadoc to public methodtoLocation(IBoundObject, URI)at lines 277–285.The overloaded public static method lacks Javadoc documentation. Per the coding guidelines, new code requires 100% Javadoc coverage on public/protected members with @param, @return, and @throws tags (where applicable). This must be addressed to satisfy the Checkstyle requirement.
Additionally, the single-argument overload
toLocation(IBoundObject)at line 264 is also missing Javadoc and requires the same treatment.
264-275: Add Javadoc to public methodtoLocation(IBoundObject).The public static method at lines 264–275 lacks required Javadoc documentation. Per the coding guidelines requiring 100% Javadoc coverage on public/protected members and the Javadoc style guide, this method must include a complete Javadoc block with @param and @return tags in the specified order.
+ /** + * Generate a location string for the provided bound object. + * <p> + * If metaschema data is available on the object, the location string includes + * line and column information. Otherwise, an empty string is returned. + * + * @param obj + * the bound object to generate the location for + * @return the location string in the format "line:column", or an empty string + * if location information is unavailable + */ public static String toLocation(@NonNull IBoundObject obj) {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IModelInstanceWriteHandler.java (2)
33-40: The @throws documentation message appears inconsistent with method semantics.The @throws comment states "if an error occurred while parsing the input," but this method is for writing items, not parsing. The message should reflect the actual operation or be made more general (e.g., "if an I/O error occurs during writing").
🔎 Suggested fix for the @throws documentation
/** * Write the next item in the collection of items represented by the instance. * * @param item * the item Java object to write * @throws IOException - * if an error occurred while parsing the input + * if an I/O error occurs during serialization */ void writeItem(@NonNull ITEM item) throws IOException;
25-31: Add Javadoc to public interface methods.The methods
writeSingleton(),writeList(), andwriteMap()are public interface members and require Javadoc per the project's style guide (Coverage Requirements: "Javadoc is required for allprotectedandpublicmembers: Methods"). Each should include a brief summary and@param,@return(if applicable), and@throwstags in that order. ThewriteItem()method on the same interface provides an example of the expected documentation format.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/DefinitionFieldGlobal.java (1)
61-66: Add Javadoc to public constructor and protected method.The public constructor (lines 61–66) and protected method
getBinding()(lines 99–101) require Javadoc per the style guide. Public constructors must document all parameters; protected methods must include@returnand@throwstags where applicable. These are not@Overridemethods, so inheritance does not apply.
- Constructor: add summary +
@paramtags for all parameters (binding,bindingInstance,position,module)getBinding(): add summary +@returntagAlso applies to: 99-101
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IFeatureComplexItemValueHandler.java (1)
110-117: Missing Javadoc on public interface methods in new file.The file was newly added in this PR, and the methods
callBeforeDeserializeandcallAfterDeserialize(lines 110-117) lack Javadoc documentation. These are public interface methods that require documentation to meet the coding guideline of 100% Javadoc coverage on public/protected members. Additionally, the PR commit message explicitly states this work achieves "0 Checkstyle MissingJavadocMethod violations in target packages," but these methods would trigger violations.Add Javadoc documentation to both methods including
@paramtags for the parameters and an@throwstag for theBindingException, consistent with the style shown in thegetDefinition()method above.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/BindingModuleLoader.java (1)
34-53: Critical: Duplicate Javadoc blocks detected.Two Javadoc blocks appear consecutively before the class declaration. Only the last block (lines 48-53) will be recognized by the Javadoc tool, causing the more detailed context in lines 34-47 to be lost.
🔎 Recommended fix: merge the two Javadoc blocks
-/** - * A module loader implementation that parses Metaschema modules from a - * specified resource using the built-in model {@link MetaschemaModelModule} - * binding. - * <p> - * Metaschema modules loaded this way are automatically registered with the - * {@link IBindingContext}. - * <p> - * Use of this Metaschema module loader requires that the associated binding - * context is initialized using a {@link IModuleLoaderStrategy} that supports - * dynamic bound module loading. This can be accomplished using the - * {@link SimpleModuleLoaderStrategy} initialized using the - * {@link DefaultModuleBindingGenerator}. - */ /** * Loads Metaschema modules from XML or YAML sources using data binding. * <p> * This class provides functionality to parse Metaschema module files and * construct the corresponding module model. + * <p> + * Metaschema modules loaded this way are automatically registered with the + * {@link IBindingContext}. + * <p> + * Use of this Metaschema module loader requires that the associated binding + * context is initialized using a {@link IModuleLoaderStrategy} that supports + * dynamic bound module loading. This can be accomplished using the + * {@link SimpleModuleLoaderStrategy} initialized using the + * {@link DefaultModuleBindingGenerator}. */ public class BindingModuleLoaderdatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IBindingModuleLoader.java (1)
18-36: Critical: Duplicate Javadoc blocks detected.Two Javadoc blocks appear consecutively before the interface declaration. Only the last block (lines 31-36) will be recognized by the Javadoc tool, causing the important context about IBindingContext registration and IModuleLoaderStrategy requirements in lines 18-30 to be lost.
🔎 Recommended fix: merge the two Javadoc blocks
-/** - * Supports loading a Metaschema module from a specified resource. - * <p> - * Metaschema modules loaded this way are automatically registered with the - * {@link IBindingContext}. - * <p> - * Use of this Metaschema module loader requires that the associated binding - * context is initialized using a {@link IModuleLoaderStrategy} that supports - * dynamic bound module loading. This can be accomplished using the - * {@link SimpleModuleLoaderStrategy} initialized using the - * {@link DefaultModuleBindingGenerator}. - * - */ /** * Loader interface for Metaschema modules using data binding. * <p> * This interface defines the contract for loading Metaschema modules from * various sources (files, URIs, etc.) using the data binding layer. + * <p> + * Metaschema modules loaded this way are automatically registered with the + * {@link IBindingContext}. + * <p> + * Use of this Metaschema module loader requires that the associated binding + * context is initialized using a {@link IModuleLoaderStrategy} that supports + * dynamic bound module loading. This can be accomplished using the + * {@link SimpleModuleLoaderStrategy} initialized using the + * {@link DefaultModuleBindingGenerator}. */ public interface IBindingModuleLoaderdatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/InstanceModelFieldScalar.java (1)
44-53: Critical: Duplicate Javadoc blocks detected.Two Javadoc blocks appear consecutively before the class declaration. Only the last block (lines 48-53) will be recognized by the Javadoc tool, causing the context from lines 44-47 to be lost.
🔎 Recommended fix: merge the two Javadoc blocks
-/** - * Implements a Metaschema module field instance bound to a scalar valued Java - * field. - */ /** * Implementation of a scalar field instance bound to a Java field. * <p> * This class handles the binding of a field with only a scalar value (no flags) * to the containing assembly's model. */ public final class InstanceModelFieldScalardatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/InstanceModelFieldComplex.java (1)
40-56: Remove duplicate Javadoc block.There are two consecutive Javadoc blocks before the class declaration. The existing comment on lines 40-43 should be removed since the new comprehensive Javadoc on lines 44-49 supersedes it with more detailed information about complex field binding.
🔎 Proposed fix
import nl.talsmasoftware.lazy4j.Lazy; -/** - * Implements a Metaschema module field instance bound to a Java field, - * supported by a bound definition class. - */ /** * Implementation of a complex field instance bound to a Java field. * <p> * This class handles the binding of a field referencing a complex field * definition (one that can contain flags) to the containing assembly's model. */ public final class InstanceModelFieldComplexdatabind/src/main/java/gov/nist/secauto/metaschema/databind/SimpleModuleLoaderStrategy.java (1)
27-33: Fix typo in error message: "IBindignContext" → "IBindingContext".There's a typo in the exception message.
🔎 Proposed fix
@NonNull private static final IModuleBindingGenerator COMPILATION_DISABLED_GENERATOR = module -> { throw new UnsupportedOperationException( "Dynamic compilation of Metaschema modules is not enabled by default." + " Configure a different IModuleBindingGenerator with the IModuleLoaderStrategy" + - " used with the IBindignContext."); + " used with the IBindingContext."); };databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IModelInstanceReadHandler.java (4)
27-29: BLOCKING: Missing Javadoc on public methodreadSingleton().This method lacks Javadoc documentation. Per coding guidelines, all public/protected members require complete Javadoc with
@returnand@throwstags.
32-32: BLOCKING: Missing Javadoc on public methodreadList().This method lacks Javadoc documentation. Per coding guidelines, all public/protected members require complete Javadoc with
@returnand@throwstags.
35-35: BLOCKING: Missing Javadoc on public methodreadMap().This method lacks Javadoc documentation. Per coding guidelines, all public/protected members require complete Javadoc with
@returnand@throwstags.
49-49: BLOCKING: Missing Javadoc on public methodgetJsonKeyFlagName().This method lacks Javadoc documentation. Per coding guidelines, all public/protected members require complete Javadoc with
@returntag.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IModelConstraintsBase.java (1)
10-20: Duplicate Javadoc blocks detected.There are two consecutive Javadoc comment blocks for
IModelConstraintsBase. Only the block immediately preceding the interface declaration (lines 14-19) will be used by Javadoc tooling; the earlier block (lines 10-13) will be orphaned.🔎 Proposed fix: Remove the duplicate Javadoc block
-/** - * Provides a common interface for model (assembly/field) constraint binding - * objects. - */ /** * Base interface for model-level constraint bindings. * <p> * This interface provides access to constraints that apply to assemblies and * their model content. */ public interface IModelConstraintsBase extends IValueConstraintsBase {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/ITargetedConstraintBase.java (1)
10-20: Duplicate Javadoc blocks - remove or merge the first one.Two consecutive Javadoc blocks precede the interface declaration. The first block (lines 10-13) is orphaned and will be ignored by documentation tools.
🔎 Proposed fix
-/** - * Represents constraint metadata that is common to all constraints that are - * targeted at a specific set of nodes matching the target. - */ /** * Base interface for targeted constraint bindings. * <p> - * This interface provides access to constraints that target specific nodes - * within a Metaschema model using Metapath expressions. + * This interface represents constraint metadata common to all constraints + * targeting a specific set of nodes matching the target, providing access to + * constraints that target specific nodes within a Metaschema model using + * Metapath expressions. */ public interface ITargetedConstraintBase extends IConstraintBase {databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlWriter.java (1)
55-61: Constructor Javadoc contains copy-paste error from JSON writer.The constructor documentation incorrectly states "Construct a new Module-aware JSON writer" and references
DefaultJsonProblemHandler, but this is an XML writer class.🔎 Proposed fix
/** - * Construct a new Module-aware JSON writer. + * Construct a new Module-aware XML writer. * * @param writer * the XML stream writer to write with - * @see DefaultJsonProblemHandler */ public MetaschemaXmlWriter(databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/ModuleLoadingPostProcessor.java (1)
13-24: Merge duplicate Javadoc blocks.There are two separate Javadoc blocks before the interface declaration. Only the second block (lines 19-24) will be attached to the interface; the first block (lines 13-17) will be orphaned. The
@sincetag from the first block should be preserved by merging both blocks.🔎 Proposed fix
-/** - * Performs post-processing on a loaded module. - * - * @since 2.0.0 - */ -@FunctionalInterface /** - * Post-processor interface for module loading operations. + * Performs post-processing on a loaded module. * <p> * Implementations can perform additional processing on modules after they have * been loaded, such as applying external constraints. + * + * @since 2.0.0 */ +@FunctionalInterface public interface ModuleLoadingPostProcessor {schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/XmlProseCompositDatatypeProvider.java (1)
40-53: Combine the return value fromproseBaseProvider.generateDatatypeswith the result.The method should aggregate datatypes provided by
proseBaseProviderinto the returned set. According to the contract inXmlDatatypeManager(line 70), callers remove provided datatypes fromrequiredTypesusing the return value. Currently, the prose base types generated on line 50 are not included in the returned set, which means callers won't know that those types have been provided. This should follow the same pattern asCompositeDatatypeProvider: aggregate results withresult.addAll(proseBaseProvider.generateDatatypes(proseBaseTypes, writer))before returning.schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/FlagInstanceFilter.java (1)
51-89: Add unit tests for FlagInstanceFilter before merging.The Javadoc and implementation are correct, but both
filterFlagsmethods lack test coverage. The coding guidelines require Test-Driven Development (TDD) as a blocking requirement: tests must be written first before implementing functionality. Add unit tests that verify:
- The two-parameter method correctly filters only the JSON key flag
- The three-parameter method correctly excludes both the JSON key flag and JSON value key flag
- Null flag parameters are handled properly
- Edge cases (empty collections, all flags filtered, no flags filtered)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelAssemblyInline.java (1)
150-153: Add Javadoc to the protectedgetBinding()method.Per the coding guidelines, all public and protected members must have 100% Javadoc coverage. The
getBinding()method is protected and not an @OverRide, so it does not inherit interface documentation and requires explicit Javadoc with @return tag.@NonNull + /** + * Get the binding instance for this inline assembly. + * + * @return the inline assembly binding + */ protected InlineDefineAssembly getBinding() { getContainingDefinition(); return binding; }
🧹 Nitpick comments (16)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/IBoundFieldValue.java (1)
88-91: Formalize the REFACTOR comment with a tracked issue.Line 89 contains a speculative comment ("// REFACTOR: Is this correct?") that expresses uncertainty about the implementation's correctness. This should either be resolved with a clear explanation or tracked as a formal GitHub issue rather than left as an inline question. If the implementation is correct, add a brief explanatory comment; if uncertain, file a follow-up issue.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/BindingConstants.java (1)
16-22: Consider documenting individual constants for completeness.While the class-level documentation explains the general purpose, each constant's role could be clarified with brief inline Javadoc comments. For example, constants like
METASCHEMA_ASSEMBLY_REFERENCE_NAME = "assembly"would benefit from a short comment describing when and why that value is used. This is a nice-to-have improvement that would enhance API clarity without being strictly required by the current checkstyle configuration.databind/src/main/java/gov/nist/secauto/metaschema/databind/model/impl/ConstraintSupport.java (1)
68-71: Pre-existing copy-paste error in method Javadoc.The description references
{@link ValueConstraints}but this overload processesAssemblyConstraints(as correctly stated in the@paramtag). Since this PR focuses on Javadoc quality, consider correcting this while you're here.🔎 Suggested fix
/** - * Generate constraints from a {@link ValueConstraints} annotation on a valued - * object (i.e., fields and flags). + * Generate constraints from an {@link AssemblyConstraints} annotation on an + * assembly. * * @param assemblyAnnotation * the annotation where the constraints are defineddatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/DefinitionFlagGlobal.java (1)
96-99: Consider adding Javadoc to non-override public/protected methods.The protected
getBinding()and publicgetNodeItem()methods are not overrides and lack Javadoc. Since Checkstyle passes, these may be excluded by configuration, but documenting them would improve API clarity—especiallygetNodeItem()which is part of the public API.🔎 Suggested documentation
+ /** + * Gets the underlying bound class for this flag definition. + * + * @return the bound flag definition + */ @NonNull protected METASCHEMA.DefineFlag getBinding() { return binding; }+ /** + * Gets the node item bound to this flag definition. + * + * @return the assembly node item representing this definition in the node tree + */ @NonNull public INodeItem getNodeItem() { return ObjectUtils.notNull(boundNodeItem.get()); }Also applies to: 161-164
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelGroupedFieldReference.java (1)
51-65: Consider adding Javadoc to the protected constructor for completeness.While the PR scope focuses on methods and Checkstyle passes without violations, documenting the protected constructor would improve overall documentation completeness and help future developers understand the initialization parameters.
🔎 Suggested constructor Javadoc
+ /** + * Constructs a new grouped field reference instance. + * + * @param binding + * the binding data for this field reference + * @param bindingInstance + * the grouped assembly instance containing this reference + * @param position + * the position of this reference within the parent group + * @param definition + * the field definition being referenced + * @param parent + * the choice group instance containing this reference + */ protected InstanceModelGroupedFieldReference( @NonNull AssemblyModel.ChoiceGroup.Field binding, @NonNull IBoundInstanceModelGroupedAssembly bindingInstance, int position, @NonNull IFieldDefinition definition, @NonNull IChoiceGroupInstance parent) {databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelAssemblyReference.java (1)
94-97: Protected methodgetBinding()lacks Javadoc documentation.The protected accessor method
getBinding()has no Javadoc. While the majority of methods in this class appropriately omit redundant documentation via inheritance from interfaces (marked with@Override), this method lacks an@Overrideannotation and should be documented to support the PR's goal of complete Javadoc coverage in the databind module.Consider adding brief documentation:
🔎 Proposed Javadoc for getBinding()
@NonNull protected AssemblyReference getBinding() { + /** + * Gets the assembly reference binding data. + * + * @return the assembly reference instance bound to a Java class + */ return binding;databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/InstanceModelGroupedAssemblyInline.java (1)
82-120: Consider adding constructor Javadoc for completeness.While the constructor wasn't modified in this PR and Checkstyle passes, adding Javadoc with
@paramtags would complete the documentation coverage for this class's public API. This is optional since the constructor wasn't in the touched code scope.🔎 Suggested constructor documentation
+ /** + * Constructs a new inline grouped assembly instance from binding data. + * + * @param binding + * the assembly definition binding data + * @param bindingInstance + * the bound instance for this grouped assembly + * @param position + * the zero-based position of this instance within the choice group + * @param parent + * the parent choice group instance + * @param nodeItemFactory + * the factory for creating node items + */ public InstanceModelGroupedAssemblyInline(databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/JavaCompilerSupport.java (1)
242-272: Add null safety annotations to Logger interface parameters.The
Loggerinterface method parameters (msgon lines 263 and 271) lack@NonNullor@Nullableannotations, which is inconsistent with the rest of the class and the project's null-safety conventions using SpotBugs annotations. Based on the usage patterns in this class,@NonNullis appropriate.🔎 Proposed fix for null safety annotations
public interface Logger { /** * Check if debug logging is enabled. * * @return {@code true} if debug logging is enabled */ boolean isDebugEnabled(); /** * Check if info logging is enabled. * * @return {@code true} if info logging is enabled */ boolean isInfoEnabled(); /** * Log a debug message. * * @param msg * the message to log */ - void debug(String msg); + void debug(@NonNull String msg); /** * Log an info message. * * @param msg * the message to log */ - void info(String msg); + void info(@NonNull String msg); }schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/datatype/AbstractDatatypeManager.java (2)
61-70: Cache the unmodifiable wrapper to avoid repeated allocations.The method creates a new
Collections.unmodifiableMap()wrapper on every call. Since the underlying map is populated once in the static initializer and never modified, consider wrapping it once and caching the unmodifiable view as a static final constant.🔎 Suggested refactor
@NonNull private static final Map<String, String> DATATYPE_TRANSLATION_MAP // NOPMD - intentional = new LinkedHashMap<>(); + + @NonNull + private static final Map<String, String> UNMODIFIABLE_DATATYPE_TRANSLATION_MAP; static { DATATYPE_TRANSLATION_MAP.put("base64", "Base64Datatype"); DATATYPE_TRANSLATION_MAP.put("boolean", "BooleanDatatype"); // ... rest of the mappings ... DATATYPE_TRANSLATION_MAP.put("uuid", "UUIDDatatype"); DATATYPE_TRANSLATION_MAP.put("year-month-duration", "YearMonthDurationDatatype"); + UNMODIFIABLE_DATATYPE_TRANSLATION_MAP = Collections.unmodifiableMap(DATATYPE_TRANSLATION_MAP); } /** * Get the mapping of Metaschema datatype names to schema type names. * * @return an unmodifiable map of datatype name translations */ - @SuppressWarnings("null") @NonNull protected static Map<String, String> getDatatypeTranslationMap() { - return Collections.unmodifiableMap(DATATYPE_TRANSLATION_MAP); + return UNMODIFIABLE_DATATYPE_TRANSLATION_MAP; }
77-84: Avoid wrapper allocation in hot path.The lambda in
computeIfAbsentcallsgetDatatypeTranslationMap()which creates a new unmodifiable wrapper on every cache miss. Since both methods are in the same class, you can either accessDATATYPE_TRANSLATION_MAPdirectly or use the cached unmodifiable view suggested in the previous comment.🔎 Suggested refactor
If you implement the cached unmodifiable wrapper from the previous comment, update this method to:
@SuppressWarnings("null") @Override @NonNull public String getTypeNameForDatatype(@NonNull IDataTypeAdapter<?> datatype) { return datatypeToTypeMap.computeIfAbsent( datatype, - key -> getDatatypeTranslationMap().get(key.getPreferredName().getLocalName())); + key -> UNMODIFIABLE_DATATYPE_TRANSLATION_MAP.get(key.getPreferredName().getLocalName())); }Or access the mutable map directly since we're within the same class and it's effectively immutable after initialization:
@SuppressWarnings("null") @Override @NonNull public String getTypeNameForDatatype(@NonNull IDataTypeAdapter<?> datatype) { return datatypeToTypeMap.computeIfAbsent( datatype, - key -> getDatatypeTranslationMap().get(key.getPreferredName().getLocalName())); + key -> DATATYPE_TRANSLATION_MAP.get(key.getPreferredName().getLocalName())); }databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/DefaultXmlSerializer.java (1)
68-71: Consider adding brief implementation note to override.The
configurationChanged()override adds XML-specific factory reset behavior. A brief Javadoc comment explaining that this implementation refreshes the XML output factory when configuration changes would improve clarity.🔎 Suggested Javadoc addition
+ /** + * {@inheritDoc} + * <p> + * This implementation resets the XML output factory to reflect configuration changes. + */ @Override protected void configurationChanged(IMutableConfiguration<SerializationFeature<?>> config) {databind/src/main/java/gov/nist/secauto/metaschema/databind/codegen/PackageMetadata.java (1)
60-68: Consider returning an unmodifiable view for defensive programming.The
getModuleProductions()method returns the internal mutable list directly. While this is a protected method in a package-private class, returning an unmodifiable view would be more defensive. This is optional given the internal scope.🔎 Proposed fix
@NonNull protected List<IGeneratedModuleClass> getModuleProductions() { - return moduleProductions; + return CollectionUtil.unmodifiableList(moduleProductions); }This would require importing
gov.nist.secauto.metaschema.core.util.CollectionUtil.schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/XmlSchemaGenerator.java (1)
86-92: Consider replacing assertion with explicit validation.The
assertstatement on line 88 will only execute when assertions are enabled, meaning this type check could silently pass in production with a different StAX implementation on the classpath, potentially causing issues later when Woodstox-specific features are used.🔎 Proposed fix
@NonNull private static XMLOutputFactory2 defaultXMLOutputFactory() { XMLOutputFactory2 xmlOutputFactory = (XMLOutputFactory2) XMLOutputFactory.newInstance(); - assert xmlOutputFactory instanceof WstxOutputFactory; + if (!(xmlOutputFactory instanceof WstxOutputFactory)) { + throw new IllegalStateException("Expected WstxOutputFactory but got: " + xmlOutputFactory.getClass().getName()); + } xmlOutputFactory.configureForSpeed(); xmlOutputFactory.setProperty(XMLOutputFactory.IS_REPAIRING_NAMESPACES, true); return xmlOutputFactory; }schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/ModuleIndex.java (2)
102-111: Consider returning an unmodifiable view forgetDefinitions().The Javadoc states "an unmodifiable collection" but
index.values()returns a view backed by the map. While external code cannot add/remove entries directly, if anIDefinitionkey is removed from the map elsewhere, the returned collection changes. For true immutability, consider wrapping withCollectionUtil.unmodifiableCollection().
275-282:getReferences()exposes the internal mutable Set.Returning the internal
referencesset directly allows callers to modify it, breaking encapsulation. Consider returningCollectionUtil.unmodifiableSet(references)to protect the internal state.🔎 Proposed fix
+ @NonNull public Set<INamedInstance> getReferences() { - return references; + return CollectionUtil.unmodifiableSet(references); }metaschema-maven-plugin/src/main/java/gov/nist/secauto/metaschema/maven/plugin/AbstractMetaschemaMojo.java (1)
532-542: Redundant close() call within try-with-resources.The explicit
os.close()on line 535 is unnecessary since the try-with-resources statement will automatically close theOutputStreamwhen the block exits.🔎 Proposed fix
try (OutputStream os = Files.newOutputStream(staleFile.toPath(), StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { - os.close(); if (getLog().isInfoEnabled()) { getLog().info("Created stale file: " + staleFile); }
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
Outdated
Show resolved
Hide resolved
...ain/java/gov/nist/secauto/metaschema/databind/model/metaschema/IBindingMetaschemaModule.java
Outdated
Show resolved
Hide resolved
...ov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.java
Outdated
Show resolved
Hide resolved
...ind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.java
Outdated
Show resolved
Hide resolved
...c/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.java
Outdated
Show resolved
Hide resolved
...ava/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.java
Outdated
Show resolved
Hide resolved
.../nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.java
Show resolved
Hide resolved
- Fix misleading inline comment (32 KB not 2 GB) in DeserializationFeature.java - Remove duplicate Javadoc blocks in IBindingMetaschemaModule.java - Remove duplicate Javadoc blocks in IConfigurableMessageConstraintBase.java - Remove duplicate Javadoc blocks in IConstraintBase.java - Remove duplicate Javadoc blocks in IValueConstraintsBase.java - Remove duplicate Javadoc blocks in IValueTargetedConstraintsBase.java - Add @nonnull annotation to getAllowedValuesCollection() in XmlSimpleTypeDataTypeRestriction.java
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)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.java (1)
66-76: Fix grammatical error in Javadoc.Line 67 contains a grammatical error: "explanation" should be "explaining".
🔎 Proposed fix
* Get the optional remarks that provide additional details explanation the + * Get the optional remarks that provide additional details explaining the
♻️ Duplicate comments (1)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java (1)
31-34: Inline comment has been corrected.The misleading inline comment mentioned in the previous review has been fixed. The comment now correctly states "// 32 KB" for the value 32_768.
🧹 Nitpick comments (1)
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.java (1)
97-99: Consider removing commented-out debug code.The commented-out LOGGER statement is no longer needed and can be removed to keep the codebase clean.
🔎 Proposed fix
- // LOGGER.info(String.format("Field:%s:%s: %s", - // definition.getContainingMetaschema().getLocation(), - // definition.getName(), allowedValue.getValue()));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IBindingMetaschemaModule.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.java
🚧 Files skipped from review as they are similar to previous changes (1)
- databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IBindingMetaschemaModule.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.java: All code changes must follow the Javadoc style guide (docs/javadoc-style-guide.md). New code requires 100% Javadoc coverage on public/protected members. Modified code must add/update Javadoc on any members touched. All Javadoc must include @param, @return, @throws tags in the correct order (BLOCKING)
Java target version must be Java 11. Use SpotBugs annotations (@nonnull, @nullable) for null safety in code.
Follow package naming convention gov.nist.secauto.metaschema.* for all Java packages
Follow Test-Driven Development (TDD) principles: write tests first before implementing functionality, verify tests fail with current implementation, implement minimal code to pass tests, then refactor while keeping tests green
Files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
🧠 Learnings (7)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.java : All code changes must follow the Javadoc style guide (docs/javadoc-style-guide.md). New code requires 100% Javadoc coverage on public/protected members. Modified code must add/update Javadoc on any members touched. All Javadoc must include param, return, throws tags in the correct order (BLOCKING)
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:22:07.082Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:56.361Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to core/metaschema/schema/xml/** : XMLBeans code is generated from XSD schemas in core/metaschema/schema/xml during Maven build. Generated sources are placed in target/generated-sources/
📚 Learning: 2025-12-24T21:21:40.208Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/csrc/ns/metaschema/test_suite/_1_0/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:40.208Z
Learning: In metaschema-framework/metaschema-java, generated binding classes in package gov.nist.csrc.ns.metaschema.test_suite._1_0 (and similar generated binding packages) are pre-generated by metaschema-maven-plugin and checked into source control. Javadoc coverage issues in these generated classes should be tracked as code generator improvements rather than file-level issues, and improvements are deferred to generator enhancements.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
📚 Learning: 2025-12-19T04:01:37.408Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 550
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/format/JsonPointerFormatter.java:56-100
Timestamp: 2025-12-19T04:01:37.408Z
Learning: When overriding Java interface methods, rely on inherited Javadoc from the interface. Do not duplicate documentation in the implementing class unless there is implementation-specific behavior that warrants additional notes beyond the interface contract.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
📚 Learning: 2025-12-24T21:21:56.361Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/Metaschema.java:41-47
Timestamp: 2025-12-24T21:21:56.361Z
Learning: In metaschema-testing, generated binding classes under gov.nist.secauto.metaschema.model.testing.testsuite are produced by metaschema-maven-plugin from YAML metaschema definitions. Javadoc issues in these generated classes should not be flagged for manual fixes; improvements are tracked and handled through code generator enhancements rather than manual edits to the generated source.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
📚 Learning: 2025-12-27T16:52:04.509Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 590
File: core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java:482-492
Timestamp: 2025-12-27T16:52:04.509Z
Learning: In Java, UncheckedIOException.getCause() is declared to return IOException. In methods that declare throws IOException, you can rethrow the underlying cause with throw e.getCause() where e is an UncheckedIOException, without a cast. Ensure the surrounding method signature includes throws IOException. This does not apply to other unchecked exceptions; verify that e is actually an UncheckedIOException before using this pattern.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.javaschemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: Applies to **/*.java : Java target version must be Java 11. Use SpotBugs annotations (NonNull, Nullable) for null safety in code.
Applied to files:
schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.java
📚 Learning: 2025-12-24T21:22:07.082Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 567
File: metaschema-testing/src/main/java/gov/nist/secauto/metaschema/model/testing/testsuite/GenerationCase.java:74-80
Timestamp: 2025-12-24T21:22:07.082Z
Learning: Files in the package gov.nist.secauto.metaschema.model.testing.testsuite in metaschema-testing are generated binding classes created from Metaschema definitions. Documentation and style improvements for these files should be made at the code generator level (metaschema-maven-plugin) rather than by manually editing the generated code.
Applied to files:
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.javadatabind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.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 (12)
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueConstraintsBase.java (1)
16-21: LGTM! Previous duplicate Javadoc issue resolved.The orphaned Javadoc block mentioned in the previous review has been successfully removed. The remaining interface-level documentation is clear, well-structured, and appropriately describes the purpose of this interface. The Javadoc follows the style guide and provides adequate context for value-level constraint bindings.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IValueTargetedConstraintsBase.java (1)
10-15: LGTM! Duplicate Javadoc issue resolved.The duplicate Javadoc blocks identified in the previous review have been successfully merged into a single, well-structured documentation block. The content clearly describes the interface purpose and follows the Javadoc style guidelines.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConfigurableMessageConstraintBase.java (2)
10-15: LGTM! Duplicate Javadoc issue resolved.The duplicate Javadoc blocks identified in the previous review have been removed. The interface-level documentation is now clear, accurate, and properly describes the purpose of this interface.
17-26: LGTM! Method documentation is complete.The method documentation includes all required elements: clear description, proper @return tag documenting the nullable return value, and correct @nullable annotation for null safety.
databind/src/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/IConstraintBase.java (1)
17-22: Duplicate Javadoc issue resolved.The duplicate Javadoc block flagged in the previous review has been successfully removed. The interface-level documentation is now clear and follows the Javadoc style guide.
databind/src/main/java/gov/nist/secauto/metaschema/databind/io/DeserializationFeature.java (3)
13-22: Excellent class-level Javadoc.The class documentation is complete and properly formatted, including a clear description and the generic type parameter
@param <V>.
26-30: Well-documented constant declarations.Both constant fields have clear Javadoc descriptions with accurate inline comments: "// 2 GB" for Integer.MAX_VALUE - 1 and "// 32 KB" for 32_768.
Also applies to: 31-34
91-100: Complete constructor documentation.The private constructor Javadoc is thorough with all
@paramtags properly documented, following the Javadoc style guide requirements.schemagen/src/main/java/gov/nist/secauto/metaschema/schemagen/xml/impl/schematype/XmlSimpleTypeDataTypeRestriction.java (4)
24-31: LGTM! Comprehensive class documentation.The class-level Javadoc clearly describes the purpose and provides helpful implementation details about the XML Schema generation strategy.
37-53: LGTM! Constructor properly documented.The constructor Javadoc includes all required @param tags in the correct order, and the implementation correctly initializes the base type and stores the allowed values collection.
55-63: LGTM! Getter properly documented with null-safety annotation.The getter includes appropriate Javadoc with a @return tag, and the @nonnull annotation (line 60) has been added as requested in the previous review, ensuring null-safety consistency with the field declaration.
111-141: LGTM! Static helper method properly documented.The
generateDescriptionAnnotationmethod includes complete Javadoc with all @param tags in the correct order and an appropriate @throws tag. The implementation correctly generates XML Schema annotation/documentation elements.
david-waltermire
merged commit ac5f680
into
metaschema-framework:develop
1 participant
Summary
Changes by Module
schemagen (35 files, 7 package-info files)
databind (114 files, 4 package-info files)
metaschema-maven-plugin (3 files, 1 package-info file)
Verification
mvn clean install -PCI -PreleaseTest plan
mvn -pl schemagen checkstyle:check- 0 violationsmvn -pl databind checkstyle:check- 0 violationsmvn -pl metaschema-maven-plugin checkstyle:check- 0 violationsmvn clean install -PCI -Prelease -DskipTests- SUCCESSSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.