Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oscal-cli resolve --relative-to=URI #88

Conversation

david-waltermire
Copy link
Contributor

@david-waltermire david-waltermire commented Dec 10, 2024

Committer Notes

Added the --relative-to=URI argument to the resolve command to support relative generated links during profile resolution.

Resolves #84.
Depends on metaschema-framework/liboscal-java#94.

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all website and readme documentation affected by the changes you made?

Summary by CodeRabbit

  • New Features
    • Introduced a new command-line option for generating relative URI references.
    • Users can now specify a base URI for reference generation via the new RELATIVE_TO option.

@david-waltermire david-waltermire changed the base branch from main to develop December 10, 2024 04:23
Copy link

coderabbitai bot commented Dec 10, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The changes in AbstractResolveCommand.java introduce a new command-line option, RELATIVE_TO, which allows users to specify a base URI for generating relative URI references. The executeCommand method is updated to handle this new option, determining the relativeTo URI based on the presence of the command-line argument. The logic for resolving relative URIs is implemented using UriUtils.relativize, while maintaining existing error handling.

Changes

File Change Summary
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractResolveCommand.java Added a new static final option RELATIVE_TO for generating relative URIs; updated executeCommand method logic to handle this option.

Assessment against linked issues

Objective Addressed Explanation
Support relative paths in resolved profiles (#84)
Submit a PR that fully addresses the goals (#84)
CI-CD build process runs without errors (#84) Status of CI-CD build process is unclear.

🐇 In the meadow, I hop with glee,
A new option for paths, oh what a spree!
With RELATIVE_TO, we dance and play,
Generating URIs in a clever way.
Hopping through code, with joy I proclaim,
This change brings fun to our CLI game! 🌼


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 10

🧹 Outside diff range and nitpick comments (22)
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractDeprecatedOscalValidationSubcommand.java (1)

18-21: Consider adding @deprecated annotation

The documentation clearly indicates this is a deprecated class. Consider adding the @Deprecated annotation to provide compile-time deprecation warnings and IDE support.

+@Deprecated(since = "1.0", forRemoval = true)
/**
 * This abstract command implementation provides user feedback about extending
 * command being deprecated in favor of the {@link ValidateCommand}.
 */
public abstract class AbstractDeprecatedOscalValidationSubcommand
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ListAllowedValuesCommand.java (3)

Line range hint 91-93: Fix incorrect command description.

The current description "Generate a diagram for the provided Metaschema module" is incorrect for a command that lists allowed values constraints.

   @Override
   public String getDescription() {
-    return "Generate a diagram for the provided Metaschema module";
+    return "List allowed values constraints for the provided Metaschema module";
   }

Line range hint 127-132: Enhance error message for binding context initialization failure.

While the error handling is good, the error message could be more specific about what failed during initialization.

     } catch (RuntimeException ex) {
       throw new CommandExecutionException(ExitCode.RUNTIME_ERROR,
-          String.format("Unable to initialize the binding context. %s", ex.getLocalizedMessage()),
+          String.format("Failed to initialize OSCAL binding context with constraint sets. Details: %s", ex.getLocalizedMessage()),
           ex);
     }

Line range hint 185-193: Consider implementing or removing commented-out metapath filtering code.

The commented-out code block suggests incomplete functionality for metapath filtering. If this feature is planned, consider creating a tracking issue.

Would you like me to help create a GitHub issue to track the implementation of metapath filtering functionality?

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/poam/PlanOfActionsAndMilestonesCommand.java (1)

23-23: Consider documenting or implementing the commented-out RenderSubcommand

The commented-out RenderSubcommand suggests incomplete or planned functionality.

Would you like me to:

  1. Create an issue to track the implementation of RenderSubcommand?
  2. Help implement the RenderSubcommand if it's ready for implementation?
src/main/assembly/component.xml (4)

Line range hint 8-8: Consider removing the leading slash from outputDirectory

The leading slash in /lib might cause issues in Windows environments. Consider using lib instead for better cross-platform compatibility.

-			<outputDirectory>/lib</outputDirectory>
+			<outputDirectory>lib</outputDirectory>

Line range hint 19-46: Standardize path handling across fileSets

For consistency and cross-platform compatibility, consider removing leading slashes from all outputDirectory paths.

-			<outputDirectory>/</outputDirectory>
+			<outputDirectory></outputDirectory>

Line range hint 49-60: Remove commented configuration

The commented-out excludes section should be removed if it's not needed. If it's for future use, consider documenting it in a separate file or issue.

-<!-- 				<excludes> -->
-<!-- 					<exclude>${artifact}</exclude> -->
-<!-- 				</excludes> -->

Line range hint 1-61: Overall structure looks good with some architectural considerations

The transition to the component format and the overall structure are well-implemented. Consider the following architectural points:

  1. Document the rationale for the component format transition in the project's documentation
  2. Create a migration guide for downstream projects
  3. Consider adding a schema version property in the pom.xml for easier future upgrades
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentresults/AssessmentResultsCommand.java (2)

10-13: Consider enhancing class documentation with more details.

The documentation could be more informative by including:

  • List of supported operations (validate, convert)
  • Brief explanation of what OSCAL assessment results are
  • Link to OSCAL documentation for further reference
 /**
  * A parent command implementation that organizes commands related to an OSCAL
  * assessment results.
+ *
+ * Supported operations:
+ * - validate: Validates an OSCAL assessment results file
+ * - convert: Converts assessment results between different formats
+ *
+ * @see <a href="https://pages.nist.gov/OSCAL/concepts/layer/assessment/assessment-results/">OSCAL Assessment Results</a>
  */

18-20: Enhance constructor documentation with initialization details.

The constructor documentation should describe what command handlers are initialized and their purposes.

 /**
  * Construct a new parent command.
+ *
+ * Initializes the following command handlers:
+ * - ValidateSubcommand: For validating assessment results
+ * - ConvertSubcommand: For format conversion operations
  */
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/SystemSecurityPlanCommand.java (1)

Line range hint 21-25: Consider adding support for relative paths in subcommands.

Given that this PR aims to add --relative-to=URI support, consider whether any of the subcommands (ValidateSubcommand, ConvertSubcommand) need to be updated to handle relative paths consistently.

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/metaschema/MetaschemaCommand.java (1)

11-14: Enhance class-level documentation

While the Javadoc is clear, it could be more descriptive about the specific Metaschema operations this command organizes.

Consider expanding the documentation:

 /**
  * A parent command implementation that organizes commands related to Metaschema
  * operations.
+ * 
+ * This command serves as a container for various Metaschema-related subcommands,
+ * providing a hierarchical structure for operations such as validation,
+ * transformation, or other Metaschema-specific tasks.
  */
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractResolveCommand.java (1)

175-183: Enhance error message in URI relativization.

While the implementation is correct, consider improving the error handling in the lambda function to provide more context about which URI failed to relativize.

         (uri, src) -> {
           try {
             return UriUtils.relativize(relativeTo, src.resolve(uri), true);
           } catch (URISyntaxException ex) {
-            throw new IllegalArgumentException(ex);
+            throw new IllegalArgumentException(
+                String.format("Failed to relativize URI '%s' against base '%s': %s",
+                    uri, relativeTo, ex.getMessage()), ex);
           }
         });
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/catalog/CatalogCommand.java (1)

23-23: Document or remove commented code

The commented out RenderSubcommand line should either be removed if it's no longer needed or documented with a TODO explaining why it's commented out and when it will be implemented.

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentplan/AssessmentPlanCommand.java (1)

10-13: Consider enhancing shared documentation

The documentation pattern is consistent with other command classes, which is good. Consider enhancing it with:

  1. @since tag to track when these commands were introduced
  2. Examples of command usage
  3. Links to relevant OSCAL documentation

Also applies to: 18-20

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/profile/ProfileCommand.java (1)

10-13: Document the resolve command functionality

Since this PR adds new functionality to the resolve command, consider enhancing the class documentation to mention the profile resolution capabilities, including the new relative path feature.

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ComponentDefinitionCommand.java (1)

Line range hint 23-25: Address commented out code.

The commented out line // addCommandHandler(new RenderSubcommand()); should either be removed or implemented if needed.

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/poam/ConvertSubcommand.java (1)

13-18: Add @deprecated annotation to complement the Javadoc.

While the deprecation notice in the Javadoc is clear, it's recommended to also add the @Deprecated annotation to the class for better tooling support and compile-time warnings.

 /**
  * Provides an OSCAL content conversion command.
  * <p>
  * This executor provides user feedback stating that this command is deprecated
  * in favor of the {@link ConvertCommand}.
  */
+@Deprecated(since = "1.0.0", forRemoval = true)
 public class ConvertSubcommand
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentresults/ValidateSubcommand.java (1)

32-32: LGTM! Completes consistent framework update

The removal of SAXException completes the consistent pattern of exception handling changes across all validation commands. This simplification improves API clarity while maintaining the same validation functionality.

Consider documenting this exception handling change in the framework's technical documentation to help future contributors understand how XML parsing exceptions are now handled internally.

src/main/java-templates/gov/nist/secauto/oscal/tools/cli/core/OscalCliVersion.java (1)

36-42: LGTM: Improved encapsulation through visibility changes

Good change to make the constants private since they are already exposed through the interface methods. This follows the principle of encapsulation and information hiding.

Consider adding @Value annotations to document the Maven property placeholders:

  /** The application name */
  @Value("${project.name}")
  private static final String NAME = "oscal-cli";
CODE_OF_CONDUCT.md (1)

3-3: Enhance readability with minor grammatical improvements

The addition of this introductory statement effectively sets clear expectations for contributors. Consider these minor grammatical improvements:

-The following is offered as standard code of conduct to which all contributors are assumed to be committed.
+The following is offered as a standard code of conduct, to which all contributors are assumed to be committed.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~3-~3: You might be missing the article “a” here.
Context: ...of Conduct The following is offered as standard code of conduct to which all contributo...

(AI_EN_LECTOR_MISSING_DETERMINER_A)


[uncategorized] ~3-~3: Possible missing comma found.
Context: ...ollowing is offered as standard code of conduct to which all contributors are assumed t...

(AI_HYDRA_LEO_MISSING_COMMA)

🛑 Comments failed to post (10)
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentresults/AssessmentResultsCommand.java (1)

22-24: 💡 Codebase verification

Based on the gathered evidence, I can now provide a final response:

Remove commented-out RenderSubcommand code across all command classes

The RenderSubcommand appears to be commented out consistently across 7 different command classes, but there's no implementation of this subcommand in the codebase. The project's README also doesn't mention any rendering functionality in its feature list, which only includes converting, validating, and resolving OSCAL content. This appears to be dead code that should be removed to maintain code cleanliness.

  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentresults/AssessmentResultsCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/SystemSecurityPlanCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/profile/ProfileCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/poam/PlanOfActionsAndMilestonesCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/catalog/CatalogCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ComponentDefinitionCommand.java
  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentplan/AssessmentPlanCommand.java
🔗 Analysis chain

Clarify the status of the commented-out RenderSubcommand.

The presence of a commented-out command handler raises questions:

  1. Is this a planned feature?
  2. Should this comment be removed if the feature is not planned?
  3. If planned, should there be a TODO with a ticket reference?

Let's check if there are any related issues or implementations:


Let me search for the actual RenderSubcommand implementation to understand if this is a planned feature or dead code.


Based on the results so far, I can see that the commented-out RenderSubcommand appears consistently across multiple command classes, but there's no implementation found. Let me check if there's any documentation or related code that might explain this pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for RenderSubcommand implementations or references
rg -l "RenderSubcommand" 

# Search for related issues in comments
rg -i "render.*command|render.*subcommand" 

Length of output: 1827


Script:

#!/bin/bash
# Search for RenderSubcommand class definition
ast-grep --pattern 'class RenderSubcommand'

# Search for any render-related TODOs or implementation notes
rg -i "todo.*render|implement.*render"

Length of output: 86


Script:

#!/bin/bash
# Check README or documentation for render functionality
fd README -e md -X cat {}

# Look for render-related code or interfaces
rg -A 3 "interface.*Render|class.*Render"

Length of output: 4621

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractResolveCommand.java (1)

165-170: 💡 Codebase verification

Implementation needs test coverage for --relative-to option

The implementation of the --relative-to option is present but lacks dedicated test coverage. Specific concerns:

  • No test cases found validating the --relative-to functionality
  • Missing edge case tests for URI resolution scenarios
  • No verification of relative path generation behavior

Please add test cases to cover:

  • Basic relative path resolution
  • Edge cases with different URI formats
  • Error handling for invalid URI inputs
🔗 Analysis chain

Verify the implementation meets requirements.

The implementation of --relative-to option appears complete, but let's verify it handles all edge cases mentioned in issue #84.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for tests covering the --relative-to option

# Look for test files
echo "Searching for test files..."
fd -e java -e xml test

# Search for test cases using the --relative-to option
echo "Searching for test cases..."
rg -l "relative-to" --type java --type xml

Length of output: 516


Script:

#!/bin/bash
# Let's examine the test coverage and implementation details

# Check test class for resolution tests
echo "=== Checking test coverage ==="
rg -A 10 "class CLITest" src/test/java/gov/nist/secauto/oscal/tools/cli/core/CLITest.java

# Look for the original issue description
echo -e "\n=== Checking issue description ==="
gh issue view 84

# Check command line option definition
echo -e "\n=== Checking CLI option definition ==="
rg "RELATIVE_TO" src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractResolveCommand.java

Length of output: 1965


Script:

#!/bin/bash
# Let's check the complete option definition and linked issue

# Check the full option definition
echo "=== Checking complete RELATIVE_TO option definition ==="
rg -B 5 -A 5 "Option RELATIVE_TO" src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractResolveCommand.java

# Check the linked issue #78
echo -e "\n=== Checking linked issue #78 ==="
gh issue view 78

# Look for any resolution tests
echo -e "\n=== Checking for resolution tests ==="
rg -l "resolve|resolution" src/test/java/gov/nist/secauto/oscal/tools/cli/core/

Length of output: 3210

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/catalog/ConvertSubcommand.java (1)

13-18: 🛠️ Refactor suggestion

Add @deprecated annotation to match Javadoc.

While the Javadoc indicates this class is deprecated, the @Deprecated annotation is missing. This annotation is important for compile-time warnings and tooling support.

Add the annotation before the class declaration:

 /**
  * Provides an OSCAL content conversion command.
  * <p>
  * This executor provides user feedback stating that this command is deprecated
  * in favor of the {@link ConvertCommand}.
  */
+@Deprecated(since = "1.0.0", forRemoval = true)
 public class ConvertSubcommand

Committable suggestion skipped: line range outside the PR's diff.

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentplan/ConvertSubcommand.java (1)

13-18: 🛠️ Refactor suggestion

Add @deprecated annotation for consistency.

Similar to the catalog version, this class needs the @Deprecated annotation to match its Javadoc deprecation notice.

Add the annotation before the class declaration:

 /**
  * Provides an OSCAL content conversion command.
  * <p>
  * This executor provides user feedback stating that this command is deprecated
  * in favor of the {@link ConvertCommand}.
  */
+@Deprecated(since = "1.0.0", forRemoval = true)
 public class ConvertSubcommand
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

/**
 * Provides an OSCAL content conversion command.
 * <p>
 * This executor provides user feedback stating that this command is deprecated
 * in favor of the {@link ConvertCommand}.
 */
@Deprecated(since = "1.0.0", forRemoval = true)
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/ConvertSubcommand.java (1)

13-18: 🛠️ Refactor suggestion

Enhance deprecation documentation and add @deprecated annotation

While the Javadoc indicates this class is deprecated, the implementation is missing several key elements for proper deprecation:

  1. Add the @Deprecated annotation
  2. Include @since tag to indicate when it was deprecated
  3. Add @forRemoval to indicate if/when it will be removed
  4. Provide migration guidance in the Javadoc

Here's the suggested implementation:

/**
 * Provides an OSCAL content conversion command.
 * <p>
 * This executor provides user feedback stating that this command is deprecated
 * in favor of the {@link ConvertCommand}.
+ * 
+ * @deprecated since 1.0.0, for removal in 2.0.0. Use {@link ConvertCommand} instead.
+ * Migration guide: Replace usage of this class with {@link ConvertCommand} and
+ * specify the content type as "ssp" when executing the command.
 */
+@Deprecated(since = "1.0.0", forRemoval = true)
 public class ConvertSubcommand
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

/**
 * Provides an OSCAL content conversion command.
 * <p>
 * This executor provides user feedback stating that this command is deprecated
 * in favor of the {@link ConvertCommand}.
 * 
 * @deprecated since 1.0.0, for removal in 2.0.0. Use {@link ConvertCommand} instead.
 * Migration guide: Replace usage of this class with {@link ConvertCommand} and
 * specify the content type as "ssp" when executing the command.
 */
@Deprecated(since = "1.0.0", forRemoval = true)
public class ConvertSubcommand
src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/assessmentresults/ConvertSubcommand.java (1)

13-18: 💡 Codebase verification

🛠️ Refactor suggestion

Systematic deprecation inconsistencies found across ConvertSubcommand implementations

All ConvertSubcommand implementations (AssessmentPlan, AssessmentResults, Catalog, ComponentDefinition, POAM, SSP) mention deprecation in their Javadoc comments but lack the required @Deprecated annotation. Only the Profile implementation is missing both the annotation and deprecation documentation.

  • src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/profile/ConvertSubcommand.java: Missing both annotation and deprecation documentation
  • All other ConvertSubcommand implementations: Missing @Deprecated annotation while having deprecation documentation
🔗 Analysis chain

Verify and systematically update all ConvertSubcommand implementations

This class has the same deprecation documentation issues as the SSP implementation. We should ensure consistent deprecation handling across all command implementations.

Let's verify all similar implementations:

Apply the same deprecation enhancement pattern as suggested for the SSP implementation to maintain consistency across all command implementations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all ConvertSubcommand implementations and check their deprecation status

# Find all ConvertSubcommand files
echo "Checking all ConvertSubcommand implementations..."
fd -e java ConvertSubcommand.java | while read -r file; do
  echo "=== $file ==="
  # Check for @Deprecated annotation
  echo "Has @Deprecated annotation?"
  rg "@Deprecated" "$file" || echo "No @Deprecated annotation found"
  # Check for deprecation in Javadoc
  echo "Deprecation documentation:"
  rg -A 5 "@deprecated|deprecated" "$file" || echo "No deprecation documentation found"
  echo
done

Length of output: 6270

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ConvertSubcommand.java (1)

11-11: 💡 Codebase verification

Based on the gathered information, I can now generate the final response:

Documentation needed for the unified ConvertCommand

The new ConvertCommand lacks sufficient documentation and migration guidance:

  • The command description is minimal and doesn't explain the unified nature of the command
  • No deprecation notice is present in the old ConvertSubcommand
  • No migration examples are provided in the documentation
  • The README mentions conversion capabilities but doesn't detail the command usage

Required documentation updates:

  • Add @Deprecated annotation to ConvertSubcommand with migration instructions
  • Enhance ConvertCommand Javadoc with detailed usage examples
  • Update README.md with examples of the new unified convert command
  • Add a migration guide for users transitioning from specific subcommands
🔗 Analysis chain

Verify documentation and examples for the new ConvertCommand

While deprecating this command in favor of ConvertCommand, we should ensure the new command is well-documented with migration examples.

Let's verify the documentation of the new command:

Consider adding a dedicated migration guide in the project documentation to help users transition from the deprecated command-specific implementations to the new unified ConvertCommand.

Also applies to: 13-18

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check documentation of the new ConvertCommand

# Find and examine ConvertCommand documentation
echo "Checking ConvertCommand documentation..."
rg -l "class ConvertCommand" | while read -r file; do
  echo "=== $file ==="
  # Extract class documentation
  ast-grep --pattern 'class ConvertCommand {
    $$$
  }' "$file"
done

# Check for usage examples in documentation files
echo "Checking for migration examples..."
fd -e md -e adoc | xargs rg -l "ConvertCommand"

Length of output: 604


Script:

#!/bin/bash
# Let's try a different approach to check the documentation and implementation

# Check for Javadoc comments and class implementation
echo "=== Checking ConvertCommand implementation and documentation ==="
rg -A 20 "class ConvertCommand" 

# Check for usage examples in README or docs
echo -e "\n=== Checking README and documentation files ==="
fd "README" | xargs cat
fd -t f -d 1 "docs" | xargs cat

# Check for command-line help text
echo -e "\n=== Checking command-line help text ==="
rg -B 3 "@Command\(name = \"convert\"" 

Length of output: 8570


Script:

#!/bin/bash
# Let's check for command annotations and help text in both the old and new implementations

# Check command annotations and descriptions in both files
echo "=== Checking command annotations and descriptions ==="
rg -B 5 "@Command" src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ConvertSubcommand.java
echo -e "\n=== Checking new ConvertCommand annotations ==="
rg -B 5 "@Command" src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ConvertCommand.java

# Check for any migration or deprecation notices
echo -e "\n=== Checking for deprecation notices ==="
rg "@Deprecated|@deprecated" src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ConvertSubcommand.java

# Check for any usage examples in test files
echo -e "\n=== Checking for usage examples in tests ==="
fd -e java -p "test" | xargs rg "ConvertCommand|ConvertSubcommand"

Length of output: 2742

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/ValidateSubcommand.java (1)

32-32: 💡 Codebase verification

Inconsistent exception handling in validation hierarchy

The verification reveals an inconsistency in the exception handling. While SAXException has been removed from ValidateSubcommand.getOscalXmlSchemas(), it's still declared in the abstract base class AbstractOscalValidationCommand. This creates a mismatch in the exception contract between the base class and its implementation.

  • AbstractOscalValidationCommand.java: Method declares throws IOException, SAXException
  • ValidateSubcommand.java: Method only declares throws IOException

This change should either be reverted to maintain consistency with the base class, or the base class should be updated to remove the SAXException declaration.

🔗 Analysis chain

Verify exception handling consistency across validation commands

The removal of SAXException from the method signature suggests that XML parsing exceptions are now handled internally. This change aligns with the broader refactoring of exception handling in the validation framework.

Let's verify this change is consistent across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining SAXException declarations in validation commands
# and verify the exception handling pattern in AbstractOscalValidationCommand

# Search for any remaining SAXException declarations in validation commands
echo "Checking for remaining SAXException declarations:"
rg "throws.*SAXException" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands"

# Check the base class implementation for exception handling pattern
echo "Checking base class implementation:"
rg -A 10 "getOscalXmlSchemas" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractOscalValidationCommand.java"

Length of output: 1190


Script:

#!/bin/bash
# Let's check the implementation of getOscalXmlSchemas in the ValidateSubcommand class
# and its hierarchy to understand how SAXException is handled

# First, check the ValidateSubcommand implementation
echo "Checking ValidateSubcommand implementation:"
rg -A 10 "getOscalXmlSchemas.*throws" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/ValidateSubcommand.java"

# Check for any try-catch blocks handling SAXException in ValidateSubcommand
echo -e "\nChecking for SAXException handling in ValidateSubcommand:"
rg -B 2 -A 5 "catch.*SAXException" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ssp/ValidateSubcommand.java"

# Check for any try-catch blocks handling SAXException in AbstractOscalValidationCommand
echo -e "\nChecking for SAXException handling in AbstractOscalValidationCommand:"
rg -B 2 -A 5 "catch.*SAXException" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractOscalValidationCommand.java"

# Check if there are any other validation commands that might show the pattern
echo -e "\nChecking other validation commands implementation:"
fd -e java . "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands" --exec rg -l "getOscalXmlSchemas.*throws"

Length of output: 2385


Script:

#!/bin/bash
# Let's check the implementation of XmlSchemaContentValidator to see how SAXException is handled internally

# Check XmlSchemaContentValidator constructor and related methods
echo "Checking XmlSchemaContentValidator implementation:"
rg -B 2 -A 10 "class XmlSchemaContentValidator" --type java

# Check for SAXException handling in XmlSchemaContentValidator
echo -e "\nChecking for SAXException handling in XmlSchemaContentValidator:"
rg -B 2 -A 5 "catch.*SAXException" --type java

# Let's also check ValidateCommand implementation for context
echo -e "\nChecking ValidateCommand implementation:"
rg -A 10 "getOscalXmlSchemas.*throws" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/ValidateCommand.java"

Length of output: 1068

src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/componentdefinition/ValidateSubcommand.java (1)

32-32: 💡 Codebase verification

Inconsistent exception handling detected in validation command hierarchy

The parent class AbstractOscalValidationCommand still declares SAXException in its method signature while the child class ValidateSubcommand has removed it. This creates an inconsistency in the inheritance hierarchy.

  • AbstractOscalValidationCommand.java: Method declares throws IOException, SAXException
  • ValidateSubcommand.java: Method only declares throws IOException
🔗 Analysis chain

LGTM! Verify consistent exception handling across validation commands.

The removal of SAXException from the method signature aligns with the broader refactoring of validation commands.

Let's verify consistent exception handling across all validation commands:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent exception handling in validation commands
# Look for any remaining SAXException declarations in validation-related files

rg -l "throws.*SAXException" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands"

Length of output: 188


Script:

#!/bin/bash
# Let's check the actual usage of SAXException in AbstractOscalValidationCommand.java
rg -A 5 "throws.*SAXException" "src/main/java/gov/nist/secauto/oscal/tools/cli/core/commands/AbstractOscalValidationCommand.java"

# Also check if ValidateSubcommand extends AbstractOscalValidationCommand
ast-grep --pattern 'class ValidateSubcommand extends AbstractOscalValidationCommand'

Length of output: 328

src/test/java/gov/nist/secauto/oscal/tools/cli/core/CLITest.java (1)

231-232: ⚠️ Potential issue

Add test coverage for the new --relative-to functionality.

While the added test for metaschema metapath list-functions is good, there's no test coverage for the new --relative-to=URI argument being added to the resolve command.

Would you like me to help generate test cases for the new --relative-to functionality?

@david-waltermire david-waltermire changed the title Feature/84 relative to oscal-cli resolve --relative-to=URI Dec 10, 2024
@david-waltermire
Copy link
Contributor Author

david-waltermire commented Dec 10, 2024

@coderabbitai regenerate the PR summary

Copy link

coderabbitai bot commented Dec 10, 2024

@david-waltermire, I will regenerate the AI-generated summary for the pull request as you requested.

✅ Actions performed

Summary regeneration triggered.

Copy link

coderabbitai bot commented Dec 10, 2024

@david-waltermire, I will regenerate the PR summary as per your request.

✅ Actions performed

Summary regeneration triggered.

@david-waltermire david-waltermire marked this pull request as draft December 10, 2024 04:56
@david-waltermire david-waltermire marked this pull request as ready for review December 11, 2024 04:16
…t relative generated links during profile resolution. Resolves metaschema-framework#84.
@aj-stein-gsa aj-stein-gsa force-pushed the feature/84-relative-to branch from 46d5aa6 to f3f4d17 Compare December 11, 2024 15:09
@aj-stein-gsa aj-stein-gsa merged commit 9f449aa into metaschema-framework:develop Dec 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support relative paths in resolved profiles
2 participants