From 56bd88c6e2f15088c427ca20aadc0e724345071e Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Wed, 17 Dec 2025 16:38:52 -0500 Subject: [PATCH 01/15] Add design document for CLIProcessor refactoring Document the approach for issue #252 to refactor the processCommand method in CLIProcessor.java. Key decisions: - Extract CallingContext to top-level package-private class - Use Optional-based result chaining for phase flow - Add both integration and unit tests - Use protected methods for testability --- .../20251217-cli-processor-refactor/design.md | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 PRDs/20251217-cli-processor-refactor/design.md diff --git a/PRDs/20251217-cli-processor-refactor/design.md b/PRDs/20251217-cli-processor-refactor/design.md new file mode 100644 index 000000000..f74a0334b --- /dev/null +++ b/PRDs/20251217-cli-processor-refactor/design.md @@ -0,0 +1,271 @@ +# Design: Refactor processCommand in CLIProcessor + +**Issue:** [#252](https://github.com/metaschema-framework/metaschema-java/issues/252) +**Date:** 2025-12-17 +**Status:** Approved + +## Problem Statement + +The `processCommand` method in `CLIProcessor.java` has high cyclomatic and NPath complexity, requiring PMD suppressions. The `CallingContext` inner class is also flagged as a GodClass. This makes the code harder to test, understand, and maintain. + +## Goals + +1. Reduce complexity metrics to remove PMD suppressions +2. Improve testability with comprehensive unit and integration tests +3. Extract `CallingContext` to a top-level package-private class +4. Use result-chaining pattern for clean flow control +5. Allow minor improvements with explicit approval for functional changes + +## Design Decisions + +| Decision | Choice | Rationale | +|----------|--------|-----------| +| Test coverage | Both integration + unit tests | Most comprehensive coverage | +| Method visibility | Protected | Allows subclassing for testing/extension | +| CallingContext location | Top-level, package-private | Clean separation, proper encapsulation | +| Flow control pattern | Optional-based chaining | Idiomatic Java, clean flow, easy to test | + +## Architecture + +### File Structure + +``` +cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/ +├── CLIProcessor.java (simplified, delegates to CallingContext) +├── CallingContext.java (NEW - extracted, package-private) +└── ... (existing files unchanged) + +cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/ +├── CLIProcessorTest.java (NEW - integration tests) +├── CallingContextTest.java (NEW - unit tests for phases) +└── ExitCodeTest.java (existing) +``` + +### Phase Flow + +``` +processCommand() + → checkHelpAndVersion() : Optional + → parseOptions() : CommandLine (throws ParseException) + → validateExtraArguments() : Optional + → validateCalledCommands() : Optional + → applyGlobalOptions() : void + → invokeCommand() : ExitStatus +``` + +## Implementation Details + +### Refactored processCommand() + +```java +@NonNull +public ExitStatus processCommand() { + // Phase 1: Check help/version before full parsing + Optional earlyExit = checkHelpAndVersion(); + if (earlyExit.isPresent()) { + return earlyExit.get(); + } + + // Phase 2: Parse all options + CommandLine cmdLine; + try { + cmdLine = parseOptions(); + } catch (ParseException ex) { + return handleInvalidCommand(ex.getMessage()); + } + + // Phase 3-4: Validate arguments and options + Optional validationResult = validateExtraArguments(cmdLine) + .or(() -> validateCalledCommands(cmdLine)); + if (validationResult.isPresent()) { + return validationResult.get(); + } + + // Phase 5: Apply global options and execute + applyGlobalOptions(cmdLine); + return invokeCommand(cmdLine); +} +``` + +### Phase Method Signatures + +```java +// Phase 1: Check --help and --version (before full parsing) +protected Optional checkHelpAndVersion() + +// Phase 2: Parse all command line options +protected CommandLine parseOptions() throws ParseException + +// Phase 3: Validate target command's extra arguments +protected Optional validateExtraArguments(@NonNull CommandLine cmdLine) + +// Phase 4: Validate options for all called commands +protected Optional validateCalledCommands(@NonNull CommandLine cmdLine) + +// Phase 5: Apply global options (--no-color, --quiet) +protected void applyGlobalOptions(@NonNull CommandLine cmdLine) +``` + +### Phase Implementations + +**Phase 1 - checkHelpAndVersion():** + +```java +protected Optional checkHelpAndVersion() { + Options phase1Options = new Options(); + phase1Options.addOption(HELP_OPTION); + phase1Options.addOption(VERSION_OPTION); + + try { + CommandLine cmdLine = new DefaultParser() + .parse(phase1Options, getExtraArgs().toArray(new String[0]), true); + + if (cmdLine.hasOption(VERSION_OPTION)) { + getCLIProcessor().showVersion(); + return Optional.of(ExitCode.OK.exit()); + } + if (cmdLine.hasOption(HELP_OPTION)) { + showHelp(); + return Optional.of(ExitCode.OK.exit()); + } + } catch (ParseException ex) { + return Optional.of(handleInvalidCommand(ex.getMessage())); + } + return Optional.empty(); +} +``` + +**Phase 2 - parseOptions():** + +```java +protected CommandLine parseOptions() throws ParseException { + return new DefaultParser().parse(toOptions(), getExtraArgs().toArray(new String[0])); +} +``` + +**Phase 3 - validateExtraArguments():** + +```java +protected Optional validateExtraArguments(@NonNull CommandLine cmdLine) { + ICommand target = getTargetCommand(); + if (target == null) { + return Optional.empty(); + } + try { + target.validateExtraArguments(this, cmdLine); + return Optional.empty(); + } catch (InvalidArgumentException ex) { + return Optional.of(handleError( + ExitCode.INVALID_ARGUMENTS.exitMessage(ex.getLocalizedMessage()), + cmdLine, true)); + } +} +``` + +**Phase 4 - validateCalledCommands():** + +```java +protected Optional validateCalledCommands(@NonNull CommandLine cmdLine) { + for (ICommand cmd : getCalledCommands()) { + try { + cmd.validateOptions(this, cmdLine); + } catch (InvalidArgumentException ex) { + return Optional.of(handleInvalidCommand(ex.getMessage())); + } + } + return Optional.empty(); +} +``` + +**Phase 5 - applyGlobalOptions():** + +```java +protected void applyGlobalOptions(@NonNull CommandLine cmdLine) { + if (cmdLine.hasOption(NO_COLOR_OPTION)) { + handleNoColor(); + } + if (cmdLine.hasOption(QUIET_OPTION)) { + handleQuiet(); + } +} +``` + +## Testing Strategy + +### Integration Tests (CLIProcessorTest.java) + +Test the public API through `process(String... args)`: + +- `--version` shows version info and returns OK +- `--help` shows help and returns OK +- Invalid command returns INVALID_COMMAND +- Invalid option returns INVALID_COMMAND +- Valid command executes successfully +- `--quiet` option works +- `--no-color` option works + +### Unit Tests (CallingContextTest.java) + +Test individual phases via protected methods: + +**checkHelpAndVersion():** +- Returns ExitStatus for --version +- Returns ExitStatus for --help +- Returns empty for other args + +**parseOptions():** +- Parses valid options +- Throws on invalid option + +**validateExtraArguments():** +- Returns empty when no target command +- Returns empty when arguments valid +- Returns error when arguments invalid + +**validateCalledCommands():** +- Returns empty when all commands valid +- Returns error on first invalid command + +**applyGlobalOptions():** +- Applies --no-color without error +- Applies --quiet without error + +### Test Fixtures + +```java +// Minimal command for basic tests +class TestCommand implements ICommand { ... } + +// Command that accepts extra arguments +class TestCommandWithArgs implements ICommand { ... } + +// Command that requires extra arguments +class TestCommandRequiringArgs implements ICommand { ... } + +// Command with required option +class TestCommandWithRequiredOption implements ICommand { ... } +``` + +## Risks & Mitigations + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Breaking existing CLI behavior | High | Write characterization tests first, run full test suite after each change | +| CallingContext extraction breaks internal references | Medium | `CLIProcessor.this` references need updating; search for all usages | +| Test fixtures become complex/fragile | Medium | Keep test commands minimal; use builder pattern if needed | +| PMD/Checkstyle finds new issues in refactored code | Low | Run `mvn checkstyle:check` incrementally | + +## Success Criteria + +- [ ] All existing tests pass (ExitCodeTest + any integration tests) +- [ ] New tests provide coverage for all phases +- [ ] `@SuppressWarnings` for PMD complexity removed from `processCommand()` +- [ ] `@SuppressWarnings("PMD.GodClass")` removed from `CallingContext` +- [ ] `mvn install -PCI -Prelease` passes +- [ ] No functional behavior changes (unless explicitly approved) + +## Out of Scope + +- Refactoring `invokeCommand()` (already reasonably sized) +- Refactoring help/footer building methods (not complexity issues) +- Changes to `ICommand` interface or other classes From 1f84c9c811840e7119e4de32d459c351b6a3ff5c Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Wed, 17 Dec 2025 16:46:50 -0500 Subject: [PATCH 02/15] docs: add implementation plan for CLIProcessor refactoring Detailed bite-sized tasks for: - Test infrastructure setup - CallingContext extraction - Phase method extraction - processCommand refactoring - PMD warning removal --- .../20251217-cli-processor-refactor/design.md | 22 + .../implementation-plan.md | 1625 +++++++++++++++++ 2 files changed, 1647 insertions(+) create mode 100644 PRDs/20251217-cli-processor-refactor/implementation-plan.md diff --git a/PRDs/20251217-cli-processor-refactor/design.md b/PRDs/20251217-cli-processor-refactor/design.md index f74a0334b..c34dc88f0 100644 --- a/PRDs/20251217-cli-processor-refactor/design.md +++ b/PRDs/20251217-cli-processor-refactor/design.md @@ -3,6 +3,7 @@ **Issue:** [#252](https://github.com/metaschema-framework/metaschema-java/issues/252) **Date:** 2025-12-17 **Status:** Approved +**Depends On:** [PR #551](https://github.com/metaschema-framework/metaschema-java/pull/551) (shell completion) ## Problem Statement @@ -246,12 +247,33 @@ class TestCommandRequiringArgs implements ICommand { ... } class TestCommandWithRequiredOption implements ICommand { ... } ``` +## Dependencies + +### PR #551 Impact + +This refactoring must be based on PR #551 (shell completion). That PR introduces: + +1. **`getTopLevelCommands()` visibility change** - Changed from `protected` to `public`. No impact on our design. + +2. **`ShellCompletionCommand`** - New command that imports `CLIProcessor.CallingContext`. When we extract `CallingContext` to a top-level class, we must update this import: + +```java +// Before (PR #551) +import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; + +// After (our refactor) +import gov.nist.secauto.metaschema.cli.processor.CallingContext; +``` + +3. **`ExtraArgument.getType()`** - New method for completion hints. No impact on our design. + ## Risks & Mitigations | Risk | Impact | Mitigation | |------|--------|------------| | Breaking existing CLI behavior | High | Write characterization tests first, run full test suite after each change | | CallingContext extraction breaks internal references | Medium | `CLIProcessor.this` references need updating; search for all usages | +| ShellCompletionCommand import breaks | Low | Update import when extracting CallingContext | | Test fixtures become complex/fragile | Medium | Keep test commands minimal; use builder pattern if needed | | PMD/Checkstyle finds new issues in refactored code | Low | Run `mvn checkstyle:check` incrementally | diff --git a/PRDs/20251217-cli-processor-refactor/implementation-plan.md b/PRDs/20251217-cli-processor-refactor/implementation-plan.md new file mode 100644 index 000000000..a9c96dae0 --- /dev/null +++ b/PRDs/20251217-cli-processor-refactor/implementation-plan.md @@ -0,0 +1,1625 @@ +# CLIProcessor Refactoring Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Refactor `processCommand` method and extract `CallingContext` to reduce complexity and improve testability. + +**Architecture:** Extract `CallingContext` inner class to a package-private top-level class. Refactor `processCommand()` into discrete phases using Optional-based result chaining. Each phase either returns an exit status (stop) or empty (continue). + +**Tech Stack:** Java 11, JUnit 5, Apache Commons CLI, Maven + +--- + +## Pre-Implementation Setup + +### Task 0: Rebase on PR #551 + +**Prerequisites:** PR #551 must be merged into `develop` + +**Step 1: Fetch latest develop** + +```bash +cd ../metaschema-java-252 +git fetch origin develop +``` + +**Step 2: Rebase branch** + +```bash +git rebase origin/develop +``` + +**Step 3: Verify build** + +```bash +mvn -pl cli-processor test +``` + +Expected: BUILD SUCCESS + +--- + +## Phase 1: Test Infrastructure + +### Task 1: Create Test Fixtures + +**Files:** +- Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java` +- Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java` + +**Step 1: Create TestVersionInfo** + +```java +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.core.util.IVersionInfo; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A simple version info implementation for testing. + */ +class TestVersionInfo implements IVersionInfo { + + @Override + @NonNull + public String getName() { + return "test-cli"; + } + + @Override + @NonNull + public String getVersion() { + return "1.0.0-test"; + } + + @Override + @NonNull + public String getBuildTimestamp() { + return "2025-01-01T00:00:00Z"; + } + + @Override + @NonNull + public String getGitOriginUrl() { + return "https://example.com/test.git"; + } + + @Override + @NonNull + public String getGitBranch() { + return "test-branch"; + } + + @Override + @NonNull + public String getGitCommit() { + return "abc1234"; + } + + @Override + @NonNull + public String getGitClosestTag() { + return "v1.0.0"; + } +} +``` + +**Step 2: Create TestCommand** + +```java +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; +import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; + +import org.apache.commons.cli.CommandLine; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A minimal command implementation for testing. + */ +class TestCommand extends AbstractTerminalCommand { + + @Override + @NonNull + public String getName() { + return "test-cmd"; + } + + @Override + @NonNull + public String getDescription() { + return "A test command"; + } + + @Override + public ICommandExecutor newExecutor( + @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CommandLine cmdLine) { + return ICommandExecutor.using(callingContext, cmdLine, this::executeCommand); + } + + @NonNull + private void executeCommand( + @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CommandLine cmdLine) { + // Do nothing - success + } +} +``` + +**Step 3: Verify compilation** + +```bash +mvn -pl cli-processor test-compile +``` + +Expected: BUILD SUCCESS + +**Step 4: Commit** + +```bash +git add cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java +git add cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java +git commit -m "test: add test fixtures for CLIProcessor testing" +``` + +--- + +### Task 2: Create Integration Tests for Existing Behavior + +**Files:** +- Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java` + +**Step 1: Write integration test class with version test** + +```java +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +/** + * Integration tests for {@link CLIProcessor}. + *

+ * Tests the public API through {@code process(String... args)}. + */ +@DisplayName("CLIProcessor Integration Tests") +class CLIProcessorTest { + + private CLIProcessor processor; + private ByteArrayOutputStream outputCapture; + + @BeforeEach + void setUp() { + outputCapture = new ByteArrayOutputStream(); + PrintStream printStream = new PrintStream(outputCapture, true, StandardCharsets.UTF_8); + processor = new CLIProcessor( + "test-cli", + Map.of(CLIProcessor.COMMAND_VERSION, new TestVersionInfo()), + printStream); + } + + @Nested + @DisplayName("Global Options") + class GlobalOptionsTests { + + @Test + @DisplayName("--version shows version info and returns OK") + void testVersionOption() { + ExitStatus status = processor.process("--version"); + + assertAll( + () -> assertEquals(ExitCode.OK, status.getExitCode()), + () -> assertThat(outputCapture.toString(StandardCharsets.UTF_8)).contains("test-cli"), + () -> assertThat(outputCapture.toString(StandardCharsets.UTF_8)).contains("1.0.0-test")); + } + + @Test + @DisplayName("--help shows help and returns OK") + void testHelpOption() { + ExitStatus status = processor.process("--help"); + + assertAll( + () -> assertEquals(ExitCode.OK, status.getExitCode()), + () -> assertThat(outputCapture.toString(StandardCharsets.UTF_8)).contains("--help")); + } + + @Test + @DisplayName("--quiet option is accepted") + void testQuietOption() { + processor.addCommandHandler(new TestCommand()); + + ExitStatus status = processor.process("--quiet", "test-cmd"); + + assertEquals(ExitCode.OK, status.getExitCode()); + } + } + + @Nested + @DisplayName("Command Execution") + class CommandExecutionTests { + + @Test + @DisplayName("Valid command executes successfully") + void testValidCommandExecution() { + processor.addCommandHandler(new TestCommand()); + + ExitStatus status = processor.process("test-cmd"); + + assertEquals(ExitCode.OK, status.getExitCode()); + } + + @Test + @DisplayName("Unknown command returns INVALID_COMMAND") + void testUnknownCommand() { + ExitStatus status = processor.process("nonexistent-command"); + + assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()); + } + + @Test + @DisplayName("Invalid option returns INVALID_COMMAND") + void testInvalidOption() { + ExitStatus status = processor.process("--invalid-option-xyz"); + + assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()); + } + + @Test + @DisplayName("Empty args returns INVALID_COMMAND with help") + void testEmptyArgs() { + ExitStatus status = processor.process(); + + assertAll( + () -> assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()), + () -> assertThat(outputCapture.toString(StandardCharsets.UTF_8)).contains("--help")); + } + } +} +``` + +**Step 2: Run tests to verify they pass with existing code** + +```bash +mvn -pl cli-processor test -Dtest=CLIProcessorTest +``` + +Expected: BUILD SUCCESS (all tests pass - these characterize existing behavior) + +**Step 3: Commit** + +```bash +git add cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java +git commit -m "test: add integration tests for CLIProcessor existing behavior" +``` + +--- + +## Phase 2: Extract CallingContext + +### Task 3: Create CallingContext Top-Level Class (Copy) + +**Files:** +- Create: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Create new CallingContext.java file** + +Copy the inner class to a new file, adding package-private visibility and updating `CLIProcessor.this` references: + +```java +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import static org.fusesource.jansi.Ansi.ansi; + +import gov.nist.secauto.metaschema.cli.processor.command.ExtraArgument; +import gov.nist.secauto.metaschema.cli.processor.command.ICommand; +import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; +import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; +import gov.nist.secauto.metaschema.core.util.AutoCloser; +import gov.nist.secauto.metaschema.core.util.CollectionUtil; +import gov.nist.secauto.metaschema.core.util.ObjectUtils; + +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.CommandLineParser; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.fusesource.jansi.AnsiPrintStream; + +import java.io.PrintStream; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; + +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Records information about the command line options and called command + * hierarchy. + */ +@SuppressWarnings("PMD.GodClass") +class CallingContext { + @NonNull + private final CLIProcessor cliProcessor; + @NonNull + private final List

+ * This is phase 1 of command processing. + * + * @return an exit status if help or version was requested, or empty to continue + */ +@NonNull +protected Optional checkHelpAndVersion() { + Options phase1Options = new Options(); + phase1Options.addOption(CLIProcessor.HELP_OPTION); + phase1Options.addOption(CLIProcessor.VERSION_OPTION); + + try { + CommandLine cmdLine = new DefaultParser() + .parse(phase1Options, getExtraArgs().toArray(new String[0]), true); + + if (cmdLine.hasOption(CLIProcessor.VERSION_OPTION)) { + cliProcessor.showVersion(); + return Optional.of(ExitCode.OK.exit()); + } + if (cmdLine.hasOption(CLIProcessor.HELP_OPTION)) { + showHelp(); + return Optional.of(ExitCode.OK.exit()); + } + } catch (ParseException ex) { + return Optional.of(handleInvalidCommand(ObjectUtils.notNull(ex.getMessage()))); + } + return Optional.empty(); +} +``` + +**Step 2: Add import for Optional** + +```java +import java.util.Optional; +``` + +**Step 3: Run tests for checkHelpAndVersion** + +```bash +mvn -pl cli-processor test -Dtest=CallingContextTest#*checkHelpAndVersion* +``` + +Expected: BUILD SUCCESS + +**Step 4: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: extract checkHelpAndVersion phase method" +``` + +--- + +### Task 8: Extract parseOptions Method + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Add parseOptions method** + +```java +/** + * Parse all command line options. + *

+ * This is phase 2 of command processing. + * + * @return the parsed command line + * @throws ParseException if parsing fails + */ +@NonNull +protected CommandLine parseOptions() throws ParseException { + return ObjectUtils.notNull( + new DefaultParser().parse(toOptions(), getExtraArgs().toArray(new String[0]))); +} +``` + +**Step 2: Run tests for parseOptions** + +```bash +mvn -pl cli-processor test -Dtest=CallingContextTest#*parseOptions* +``` + +Expected: BUILD SUCCESS + +**Step 3: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: extract parseOptions phase method" +``` + +--- + +### Task 9: Extract validateExtraArguments Method + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Add validateExtraArguments method** + +```java +/** + * Validate extra arguments for the target command. + *

+ * This is phase 3 of command processing. + * + * @param cmdLine the parsed command line + * @return an exit status if validation failed, or empty to continue + */ +@NonNull +protected Optional validateExtraArguments(@NonNull CommandLine cmdLine) { + ICommand target = getTargetCommand(); + if (target == null) { + return Optional.empty(); + } + try { + target.validateExtraArguments(this, cmdLine); + return Optional.empty(); + } catch (InvalidArgumentException ex) { + return Optional.of(handleError( + ExitCode.INVALID_ARGUMENTS.exitMessage(ex.getLocalizedMessage()), + cmdLine, + true)); + } +} +``` + +**Step 2: Run tests for validateExtraArguments** + +```bash +mvn -pl cli-processor test -Dtest=CallingContextTest#*validateExtraArguments* +``` + +Expected: BUILD SUCCESS + +**Step 3: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: extract validateExtraArguments phase method" +``` + +--- + +### Task 10: Extract validateCalledCommands Method + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Add validateCalledCommands method** + +```java +/** + * Validate options for all called commands in the chain. + *

+ * This is phase 4 of command processing. + * + * @param cmdLine the parsed command line + * @return an exit status if validation failed, or empty to continue + */ +@NonNull +protected Optional validateCalledCommands(@NonNull CommandLine cmdLine) { + for (ICommand cmd : getCalledCommands()) { + try { + cmd.validateOptions(this, cmdLine); + } catch (InvalidArgumentException ex) { + String msg = ex.getMessage(); + assert msg != null; + return Optional.of(handleInvalidCommand(msg)); + } + } + return Optional.empty(); +} +``` + +**Step 2: Run tests for validateCalledCommands** + +```bash +mvn -pl cli-processor test -Dtest=CallingContextTest#*validateCalledCommands* +``` + +Expected: BUILD SUCCESS + +**Step 3: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: extract validateCalledCommands phase method" +``` + +--- + +### Task 11: Extract applyGlobalOptions Method + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Add applyGlobalOptions method** + +```java +/** + * Apply global options like --no-color and --quiet. + *

+ * This is phase 5 of command processing. + * + * @param cmdLine the parsed command line + */ +protected void applyGlobalOptions(@NonNull CommandLine cmdLine) { + if (cmdLine.hasOption(CLIProcessor.NO_COLOR_OPTION)) { + CLIProcessor.handleNoColor(); + } + if (cmdLine.hasOption(CLIProcessor.QUIET_OPTION)) { + CLIProcessor.handleQuiet(); + } +} +``` + +**Step 2: Run tests for applyGlobalOptions** + +```bash +mvn -pl cli-processor test -Dtest=CallingContextTest#*applyGlobalOptions* +``` + +Expected: BUILD SUCCESS + +**Step 3: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: extract applyGlobalOptions phase method" +``` + +--- + +### Task 12: Refactor processCommand to Use Phase Methods + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Replace processCommand implementation** + +Replace the entire `processCommand()` method with: + +```java +/** + * Process the command identified by the CLI arguments. + * + * @return the result of processing the command + */ +@NonNull +public ExitStatus processCommand() { + // Phase 1: Check help/version before full parsing + Optional earlyExit = checkHelpAndVersion(); + if (earlyExit.isPresent()) { + return earlyExit.get(); + } + + // Phase 2: Parse all options + CommandLine cmdLine; + try { + cmdLine = parseOptions(); + } catch (ParseException ex) { + String msg = ex.getMessage(); + assert msg != null; + return handleInvalidCommand(msg); + } + + // Phase 3-4: Validate arguments and options + Optional validationResult = validateExtraArguments(cmdLine) + .or(() -> validateCalledCommands(cmdLine)); + if (validationResult.isPresent()) { + return validationResult.get(); + } + + // Phase 5: Apply global options and execute + applyGlobalOptions(cmdLine); + return invokeCommand(cmdLine); +} +``` + +**Step 2: Remove PMD suppressions from processCommand** + +Remove this annotation from processCommand: + +```java +@SuppressWarnings({ + "PMD.OnlyOneReturn", + "PMD.NPathComplexity", + "PMD.CyclomaticComplexity" +}) +``` + +**Step 3: Run all tests** + +```bash +mvn -pl cli-processor test +``` + +Expected: BUILD SUCCESS + +**Step 4: Run full CI build** + +```bash +mvn -pl cli-processor install -PCI +``` + +Expected: BUILD SUCCESS (no PMD violations) + +**Step 5: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: simplify processCommand using extracted phase methods + +Reduces cyclomatic complexity by delegating to focused phase methods. +Removes PMD suppressions for complexity warnings." +``` + +--- + +## Phase 4: Remove GodClass Warning + +### Task 13: Verify GodClass Warning Resolved + +**Files:** +- Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` + +**Step 1: Remove GodClass suppression** + +Remove this line from `CallingContext.java`: + +```java +@SuppressWarnings("PMD.GodClass") +``` + +**Step 2: Run PMD check** + +```bash +mvn -pl cli-processor pmd:check +``` + +Expected: BUILD SUCCESS (if GodClass warning persists, we may need additional extraction - but try first) + +**Step 3: Run full CI build** + +```bash +mvn install -PCI -Prelease +``` + +Expected: BUILD SUCCESS + +**Step 4: Commit** + +```bash +git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +git commit -m "refactor: remove PMD GodClass suppression from CallingContext" +``` + +--- + +## Phase 5: Final Verification + +### Task 14: Full Build Verification + +**Step 1: Run complete CI build** + +```bash +mvn clean install -PCI -Prelease +``` + +Expected: BUILD SUCCESS + +**Step 2: Verify all tests pass** + +```bash +mvn test +``` + +Expected: All tests pass + +**Step 3: Verify no checkstyle issues** + +```bash +mvn checkstyle:check +``` + +Expected: BUILD SUCCESS + +**Step 4: Review commit history** + +```bash +git log --oneline -15 +``` + +Verify commits are clean and well-organized. + +--- + +## Summary + +| Task | Description | Files | +|------|-------------|-------| +| 0 | Rebase on PR #551 | - | +| 1 | Create test fixtures | TestVersionInfo.java, TestCommand.java | +| 2 | Create integration tests | CLIProcessorTest.java | +| 3 | Extract CallingContext | CallingContext.java (new) | +| 4 | Update CLIProcessor | CLIProcessor.java | +| 5 | Update ShellCompletionCommand | ShellCompletionCommand.java | +| 6 | Add unit tests | CallingContextTest.java | +| 7-11 | Extract phase methods | CallingContext.java | +| 12 | Refactor processCommand | CallingContext.java | +| 13 | Remove GodClass warning | CallingContext.java | +| 14 | Final verification | - | + +**Estimated commits:** 14 +**Estimated time:** 2-3 hours From aca00f8202c084f97b7f762a1dbf9734532c9703 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 08:19:23 -0500 Subject: [PATCH 03/15] test: add test fixtures for CLIProcessor testing --- .../metaschema/cli/processor/TestCommand.java | 45 ++++++++++++++ .../cli/processor/TestVersionInfo.java | 58 +++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java create mode 100644 cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java new file mode 100644 index 000000000..81c9f0440 --- /dev/null +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java @@ -0,0 +1,45 @@ +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; +import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; + +import org.apache.commons.cli.CommandLine; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A minimal command implementation for testing. + */ +class TestCommand + extends AbstractTerminalCommand { + + @Override + @NonNull + public String getName() { + return "test-cmd"; + } + + @Override + @NonNull + public String getDescription() { + return "A test command"; + } + + @Override + public ICommandExecutor newExecutor( + @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CommandLine cmdLine) { + return ICommandExecutor.using(callingContext, cmdLine, this::executeCommand); + } + + private void executeCommand( + @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CommandLine cmdLine) { + // Do nothing - success + } +} diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java new file mode 100644 index 000000000..b3467945a --- /dev/null +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java @@ -0,0 +1,58 @@ +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.core.util.IVersionInfo; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A simple version info implementation for testing. + */ +class TestVersionInfo implements IVersionInfo { + + @Override + @NonNull + public String getName() { + return "test-cli"; + } + + @Override + @NonNull + public String getVersion() { + return "1.0.0-test"; + } + + @Override + @NonNull + public String getBuildTimestamp() { + return "2025-01-01T00:00:00Z"; + } + + @Override + @NonNull + public String getGitOriginUrl() { + return "https://example.com/test.git"; + } + + @Override + @NonNull + public String getGitBranch() { + return "test-branch"; + } + + @Override + @NonNull + public String getGitCommit() { + return "abc1234"; + } + + @Override + @NonNull + public String getGitClosestTag() { + return "v1.0.0"; + } +} From 9ce954737889a3390f6d41072fc028f8152cc72e Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 08:35:42 -0500 Subject: [PATCH 04/15] refactor: extract CallingContext to top-level public class - Create CallingContext.java as a separate top-level class - Update CLIProcessor to use the extracted CallingContext - Add getOutputStream() method to CLIProcessor for CallingContext access - Make OPTIONS and handleNoColor/handleQuiet package-private - Update all imports from CLIProcessor.CallingContext to CallingContext - Fixes part of #252 --- .../cli/processor/CLIProcessor.java | 510 +----------------- .../cli/processor/CallingContext.java | 502 +++++++++++++++++ .../command/AbstractCommandExecutor.java | 2 +- .../command/AbstractParentCommand.java | 2 +- .../cli/processor/command/ICommand.java | 2 +- .../processor/command/ICommandExecutor.java | 2 +- .../command/ShellCompletionCommand.java | 2 +- .../metaschema/cli/processor/TestCommand.java | 4 +- .../CompletionScriptGeneratorTest.java | 2 +- 9 files changed, 524 insertions(+), 504 deletions(-) create mode 100644 cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java index 9adc1d632..c9523dd0c 100644 --- a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java +++ b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java @@ -7,23 +7,13 @@ import static org.fusesource.jansi.Ansi.ansi; -import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; import gov.nist.secauto.metaschema.cli.processor.command.CommandService; -import gov.nist.secauto.metaschema.cli.processor.command.ExtraArgument; import gov.nist.secauto.metaschema.cli.processor.command.ICommand; -import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; -import gov.nist.secauto.metaschema.core.util.AutoCloser; import gov.nist.secauto.metaschema.core.util.CollectionUtil; import gov.nist.secauto.metaschema.core.util.IVersionInfo; import gov.nist.secauto.metaschema.core.util.ObjectUtils; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.CommandLineParser; -import org.apache.commons.cli.DefaultParser; -import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; -import org.apache.commons.cli.ParseException; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -31,23 +21,17 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.LoggerConfig; import org.fusesource.jansi.AnsiConsole; -import org.fusesource.jansi.AnsiPrintStream; import java.io.PrintStream; -import java.io.PrintWriter; -import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.Collection; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** * Processes command line arguments and dispatches called commands. @@ -102,7 +86,7 @@ public class CLIProcessor { .get()); @NonNull - private static final List

+ * Tests the public API through {@code process(String... args)}. + */ +@DisplayName("CLIProcessor Integration Tests") +class CLIProcessorTest { + + private CLIProcessor processor; + private ByteArrayOutputStream outputCapture; + + @BeforeEach + void setUp() { + outputCapture = new ByteArrayOutputStream(); + PrintStream printStream = new PrintStream(outputCapture, true, StandardCharsets.UTF_8); + processor = new CLIProcessor( + "test-cli", + Map.of(CLIProcessor.COMMAND_VERSION, new TestVersionInfo()), + printStream); + } + + @Nested + @DisplayName("Global Options") + class GlobalOptionsTests { + + @Test + @DisplayName("--version shows version info and returns OK") + void testVersionOption() { + ExitStatus status = processor.process("--version"); + + String output = outputCapture.toString(StandardCharsets.UTF_8); + assertAll( + () -> assertEquals(ExitCode.OK, status.getExitCode()), + () -> assertTrue(output.contains("test-cli"), "Output should contain 'test-cli'"), + () -> assertTrue(output.contains("1.0.0-test"), "Output should contain '1.0.0-test'")); + } + + @Test + @DisplayName("--help shows help and returns OK") + void testHelpOption() { + ExitStatus status = processor.process("--help"); + + String output = outputCapture.toString(StandardCharsets.UTF_8); + assertAll( + () -> assertEquals(ExitCode.OK, status.getExitCode()), + () -> assertTrue(output.contains("--help"), "Output should contain '--help'")); + } + + @Test + @DisplayName("--quiet option is accepted") + void testQuietOption() { + processor.addCommandHandler(new TestCommand()); + + ExitStatus status = processor.process("--quiet", "test-cmd"); + + assertEquals(ExitCode.OK, status.getExitCode()); + } + } + + @Nested + @DisplayName("Command Execution") + class CommandExecutionTests { + + @Test + @DisplayName("Valid command executes successfully") + void testValidCommandExecution() { + processor.addCommandHandler(new TestCommand()); + + ExitStatus status = processor.process("test-cmd"); + + assertEquals(ExitCode.OK, status.getExitCode()); + } + + @Test + @DisplayName("Unknown command returns INVALID_COMMAND") + void testUnknownCommand() { + ExitStatus status = processor.process("nonexistent-command"); + + assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()); + } + + @Test + @DisplayName("Invalid option returns INVALID_COMMAND") + void testInvalidOption() { + ExitStatus status = processor.process("--invalid-option-xyz"); + + assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()); + } + + @Test + @DisplayName("Empty args returns INVALID_COMMAND with help") + void testEmptyArgs() { + ExitStatus status = processor.process(); + + String output = outputCapture.toString(StandardCharsets.UTF_8); + assertAll( + () -> assertEquals(ExitCode.INVALID_COMMAND, status.getExitCode()), + () -> assertTrue(output.contains("--help"), "Output should contain '--help'")); + } + } +} From 49049e2e0f92f61cacefc2a8b4a684749be86c06 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 09:21:22 -0500 Subject: [PATCH 06/15] test: add unit tests for CallingContext phase methods (TDD - red) Tests for phase methods that will be extracted: - checkHelpAndVersion() - check for --help/--version before parsing - parseOptions() - parse all command line options - validateExtraArguments() - validate extra arguments for target command - validateCalledCommands() - validate options for all called commands - applyGlobalOptions() - apply --no-color and --quiet options --- .../cli/processor/CallingContextTest.java | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java new file mode 100644 index 000000000..72e73c54f --- /dev/null +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java @@ -0,0 +1,172 @@ +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.ParseException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import java.io.OutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Map; +import java.util.Optional; + +/** + * Unit tests for {@link CallingContext} phase methods. + */ +@DisplayName("CallingContext Unit Tests") +class CallingContextTest { + + private CLIProcessor processor; + + @BeforeEach + void setUp() { + PrintStream nullOutput = new PrintStream(OutputStream.nullOutputStream(), true, StandardCharsets.UTF_8); + processor = new CLIProcessor( + "test-cli", + Map.of(CLIProcessor.COMMAND_VERSION, new TestVersionInfo()), + nullOutput); + } + + private CallingContext createContext(String... args) { + return new CallingContext(processor, Arrays.asList(args)); + } + + @Nested + @DisplayName("checkHelpAndVersion()") + class CheckHelpAndVersionTests { + + @Test + @DisplayName("returns ExitStatus for --version") + void returnsExitStatusForVersionOption() { + CallingContext ctx = createContext("--version"); + + Optional result = ctx.checkHelpAndVersion(); + + assertAll( + () -> assertTrue(result.isPresent()), + () -> assertEquals(ExitCode.OK, result.get().getExitCode())); + } + + @Test + @DisplayName("returns ExitStatus for --help") + void returnsExitStatusForHelpOption() { + CallingContext ctx = createContext("--help"); + + Optional result = ctx.checkHelpAndVersion(); + + assertAll( + () -> assertTrue(result.isPresent()), + () -> assertEquals(ExitCode.OK, result.get().getExitCode())); + } + + @Test + @DisplayName("returns empty for other args") + void returnsEmptyForOtherArgs() { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd"); + + Optional result = ctx.checkHelpAndVersion(); + + assertTrue(result.isEmpty()); + } + } + + @Nested + @DisplayName("parseOptions()") + class ParseOptionsTests { + + @Test + @DisplayName("parses valid options") + void parsesValidOptions() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd", "--quiet"); + + CommandLine cmdLine = ctx.parseOptions(); + + assertTrue(cmdLine.hasOption(CLIProcessor.QUIET_OPTION)); + } + + @Test + @DisplayName("throws on invalid option") + void throwsOnInvalidOption() { + CallingContext ctx = createContext("--invalid-option-xyz"); + + assertThrows(ParseException.class, ctx::parseOptions); + } + } + + @Nested + @DisplayName("validateExtraArguments()") + class ValidateExtraArgumentsTests { + + @Test + @DisplayName("returns empty when no target command") + void returnsEmptyWhenNoTargetCommand() throws ParseException { + CallingContext ctx = createContext("--help"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateExtraArguments(cmdLine); + + assertTrue(result.isEmpty()); + } + + @Test + @DisplayName("returns empty when arguments valid") + void returnsEmptyWhenArgumentsValid() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateExtraArguments(cmdLine); + + assertTrue(result.isEmpty()); + } + } + + @Nested + @DisplayName("validateCalledCommands()") + class ValidateCalledCommandsTests { + + @Test + @DisplayName("returns empty when all commands valid") + void returnsEmptyWhenAllCommandsValid() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateCalledCommands(cmdLine); + + assertTrue(result.isEmpty()); + } + } + + @Nested + @DisplayName("applyGlobalOptions()") + class ApplyGlobalOptionsTests { + + @Test + @DisplayName("applies --quiet without error") + void appliesQuietOption() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd", "--quiet"); + CommandLine cmdLine = ctx.parseOptions(); + + assertDoesNotThrow(() -> ctx.applyGlobalOptions(cmdLine)); + } + } +} From d31cfe55e5d2c36db1469a007b2125c54c344ba1 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 09:24:04 -0500 Subject: [PATCH 07/15] refactor: add phase methods to CallingContext (TDD - green) Add protected phase methods that will be used by refactored processCommand: - checkHelpAndVersion() - check for --help/--version before parsing - parseOptions() - parse all command line options - validateExtraArguments() - validate extra arguments for target command - validateCalledCommands() - validate options for all called commands - applyGlobalOptions() - apply --no-color and --quiet options All CallingContextTest unit tests now pass. --- .../cli/processor/CallingContext.java | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java index 49497f0f7..63a3507f6 100644 --- a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +++ b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -30,6 +30,7 @@ import java.util.Collection; import java.util.LinkedList; import java.util.List; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; @@ -148,6 +149,118 @@ Options toOptions() { return retval; } + /** + * Check for --help and --version options before full parsing. + *

+ * This is phase 1 of command processing. + * + * @return an exit status if help or version was requested, or empty to continue + */ + @NonNull + protected Optional checkHelpAndVersion() { + Options phase1Options = new Options(); + phase1Options.addOption(CLIProcessor.HELP_OPTION); + phase1Options.addOption(CLIProcessor.VERSION_OPTION); + + try { + CommandLine cmdLine = new DefaultParser() + .parse(phase1Options, getExtraArgs().toArray(new String[0]), true); + + if (cmdLine.hasOption(CLIProcessor.VERSION_OPTION)) { + cliProcessor.showVersion(); + return Optional.of(ExitCode.OK.exit()); + } + if (cmdLine.hasOption(CLIProcessor.HELP_OPTION)) { + showHelp(); + return Optional.of(ExitCode.OK.exit()); + } + } catch (ParseException ex) { + return Optional.of(handleInvalidCommand(ObjectUtils.notNull(ex.getMessage()))); + } + return Optional.empty(); + } + + /** + * Parse all command line options. + *

+ * This is phase 2 of command processing. + * + * @return the parsed command line + * @throws ParseException + * if parsing fails + */ + @NonNull + protected CommandLine parseOptions() throws ParseException { + return ObjectUtils.notNull( + new DefaultParser().parse(toOptions(), getExtraArgs().toArray(new String[0]))); + } + + /** + * Validate extra arguments for the target command. + *

+ * This is phase 3 of command processing. + * + * @param cmdLine + * the parsed command line + * @return an exit status if validation failed, or empty to continue + */ + @NonNull + protected Optional validateExtraArguments(@NonNull CommandLine cmdLine) { + ICommand target = getTargetCommand(); + if (target == null) { + return Optional.empty(); + } + try { + target.validateExtraArguments(this, cmdLine); + return Optional.empty(); + } catch (InvalidArgumentException ex) { + return Optional.of(handleError( + ExitCode.INVALID_ARGUMENTS.exitMessage(ex.getLocalizedMessage()), + cmdLine, + true)); + } + } + + /** + * Validate options for all called commands in the chain. + *

+ * This is phase 4 of command processing. + * + * @param cmdLine + * the parsed command line + * @return an exit status if validation failed, or empty to continue + */ + @NonNull + protected Optional validateCalledCommands(@NonNull CommandLine cmdLine) { + for (ICommand cmd : getCalledCommands()) { + try { + cmd.validateOptions(this, cmdLine); + } catch (InvalidArgumentException ex) { + String msg = ex.getMessage(); + assert msg != null; + return Optional.of(handleInvalidCommand(msg)); + } + } + return Optional.empty(); + } + + /** + * Apply global options like --no-color and --quiet. + *

+ * This is phase 5 of command processing. + * + * @param cmdLine + * the parsed command line + */ + protected void applyGlobalOptions(@NonNull CommandLine cmdLine) { + if (cmdLine.hasOption(CLIProcessor.NO_COLOR_OPTION)) { + CLIProcessor.handleNoColor(); + } + if (cmdLine.hasOption(CLIProcessor.QUIET_OPTION)) { + CLIProcessor.handleQuiet(); + } + } + /** * Process the command identified by the CLI arguments. * From 6b85676505bd8191b894947f552f039c727f5ebc Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 10:10:42 -0500 Subject: [PATCH 08/15] refactor: simplify processCommand using extracted phase methods Replace inline logic with calls to focused phase methods: - checkHelpAndVersion() for early exit on --help/--version - parseOptions() for full option parsing - validateExtraArguments() and validateCalledCommands() chained with Optional.or() - applyGlobalOptions() for --no-color/--quiet handling Removes PMD suppressions for NPathComplexity and CyclomaticComplexity. --- .../cli/processor/CallingContext.java | 72 ++++--------------- 1 file changed, 14 insertions(+), 58 deletions(-) diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java index 63a3507f6..90bb43ada 100644 --- a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +++ b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -266,77 +266,33 @@ protected void applyGlobalOptions(@NonNull CommandLine cmdLine) { * * @return the result of processing the command */ - @SuppressWarnings({ - "PMD.OnlyOneReturn", - "PMD.NPathComplexity", - "PMD.CyclomaticComplexity" - }) @NonNull public ExitStatus processCommand() { - CommandLineParser parser = new DefaultParser(); - - // phase 1 - CommandLine cmdLine; - try { - Options phase1Options = new Options(); - phase1Options.addOption(CLIProcessor.HELP_OPTION); - phase1Options.addOption(CLIProcessor.VERSION_OPTION); - - cmdLine = ObjectUtils.notNull(parser.parse(phase1Options, getExtraArgs().toArray(new String[0]), true)); - } catch (ParseException ex) { - String msg = ex.getMessage(); - assert msg != null; - return handleInvalidCommand(msg); - } - - if (cmdLine.hasOption(CLIProcessor.VERSION_OPTION)) { - cliProcessor.showVersion(); - return ExitCode.OK.exit(); - } - if (cmdLine.hasOption(CLIProcessor.HELP_OPTION)) { - showHelp(); - return ExitCode.OK.exit(); + // Phase 1: Check help/version before full parsing + Optional earlyExit = checkHelpAndVersion(); + if (earlyExit.isPresent()) { + return earlyExit.get(); } - // phase 2 + // Phase 2: Parse all options + CommandLine cmdLine; try { - cmdLine = ObjectUtils.notNull(parser.parse(toOptions(), getExtraArgs().toArray(new String[0]))); + cmdLine = parseOptions(); } catch (ParseException ex) { String msg = ex.getMessage(); assert msg != null; return handleInvalidCommand(msg); } - ICommand targetCommand = getTargetCommand(); - if (targetCommand != null) { - try { - targetCommand.validateExtraArguments(this, cmdLine); - } catch (InvalidArgumentException ex) { - return handleError( - ExitCode.INVALID_ARGUMENTS.exitMessage(ex.getLocalizedMessage()), - cmdLine, - true); - } - } - - for (ICommand cmd : getCalledCommands()) { - try { - cmd.validateOptions(this, cmdLine); - } catch (InvalidArgumentException ex) { - String msg = ex.getMessage(); - assert msg != null; - return handleInvalidCommand(msg); - } - } - - // phase 3 - if (cmdLine.hasOption(CLIProcessor.NO_COLOR_OPTION)) { - CLIProcessor.handleNoColor(); + // Phase 3-4: Validate arguments and options + Optional validationResult = validateExtraArguments(cmdLine) + .or(() -> validateCalledCommands(cmdLine)); + if (validationResult.isPresent()) { + return validationResult.get(); } - if (cmdLine.hasOption(CLIProcessor.QUIET_OPTION)) { - CLIProcessor.handleQuiet(); - } + // Phase 5: Apply global options and execute + applyGlobalOptions(cmdLine); return invokeCommand(cmdLine); } From 9deecd933d27445a28a1e531c37dc98ff77453ba Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Fri, 19 Dec 2025 23:41:49 -0500 Subject: [PATCH 09/15] refactor: remove PMD GodClass suppression from CallingContext The GodClass warning is now a priority 3 (non-blocking) violation. The refactoring reduced complexity significantly by extracting phase methods. --- .../nist/secauto/metaschema/cli/processor/CallingContext.java | 1 - 1 file changed, 1 deletion(-) diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java index 90bb43ada..80a266e8c 100644 --- a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java +++ b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -42,7 +42,6 @@ * Records information about the command line options and called command * hierarchy. */ -@SuppressWarnings("PMD.GodClass") public class CallingContext { @NonNull private final CLIProcessor cliProcessor; From 9e414d635eb4a8f4d14aeb7e3052c9d498065882 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 21 Dec 2025 10:48:52 -0500 Subject: [PATCH 10/15] fix: update CallingContext imports in metaschema-cli module Update 9 files in metaschema-cli to use the new top-level CallingContext class instead of the removed inner class CLIProcessor.CallingContext. --- .../metaschema/cli/commands/AbstractConvertSubcommand.java | 2 +- .../metaschema/cli/commands/AbstractValidateContentCommand.java | 2 +- .../cli/commands/ConvertContentUsingModuleCommand.java | 2 +- .../secauto/metaschema/cli/commands/GenerateDiagramCommand.java | 2 +- .../secauto/metaschema/cli/commands/GenerateSchemaCommand.java | 2 +- .../cli/commands/ValidateContentUsingModuleCommand.java | 2 +- .../secauto/metaschema/cli/commands/ValidateModuleCommand.java | 2 +- .../cli/commands/metapath/EvaluateMetapathCommand.java | 2 +- .../cli/commands/metapath/ListFunctionsSubcommand.java | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java index c6bad16a1..dc92a9b98 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.AbstractCommandExecutor; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java index 81d0ebc29..fc7428466 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java @@ -6,7 +6,7 @@ package gov.nist.secauto.metaschema.cli.commands; import gov.nist.secauto.metaschema.cli.processor.CLIProcessor; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.AbstractCommandExecutor; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ConvertContentUsingModuleCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ConvertContentUsingModuleCommand.java index 1c477a6f1..18ea15bd9 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ConvertContentUsingModuleCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ConvertContentUsingModuleCommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; import gov.nist.secauto.metaschema.core.model.IBoundObject; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateDiagramCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateDiagramCommand.java index 815f49411..93b8c42b7 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateDiagramCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateDiagramCommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java index a2d5444cc..192796751 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateContentUsingModuleCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateContentUsingModuleCommand.java index d17848f1d..f3c030f63 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateContentUsingModuleCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateContentUsingModuleCommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; import gov.nist.secauto.metaschema.core.configuration.DefaultConfiguration; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateModuleCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateModuleCommand.java index 08cee5a0e..0658a5453 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateModuleCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/ValidateModuleCommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java index 6f1902f4b..ce501a6c4 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java @@ -6,7 +6,7 @@ package gov.nist.secauto.metaschema.cli.commands.metapath; import gov.nist.secauto.metaschema.cli.commands.MetaschemaCommands; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; import gov.nist.secauto.metaschema.cli.processor.command.CommandExecutionException; diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/ListFunctionsSubcommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/ListFunctionsSubcommand.java index 4c689a6f8..eb07e00b9 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/ListFunctionsSubcommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/ListFunctionsSubcommand.java @@ -5,7 +5,7 @@ package gov.nist.secauto.metaschema.cli.commands.metapath; -import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext; +import gov.nist.secauto.metaschema.cli.processor.CallingContext; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.ExitStatus; import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; From fe95eabbecb5eeb011d6d9e81701606dcb48b7aa Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 21 Dec 2025 11:25:01 -0500 Subject: [PATCH 11/15] fix: address PR review feedback - Fix typo 'occured' -> 'occurred' in error message - Remove AI instruction metadata from implementation plan - Convert bold step headers to markdown headings --- .../implementation-plan.md | 112 +++++++++--------- .../cli/processor/CallingContext.java | 2 +- 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/PRDs/20251217-cli-processor-refactor/implementation-plan.md b/PRDs/20251217-cli-processor-refactor/implementation-plan.md index a9c96dae0..12a800231 100644 --- a/PRDs/20251217-cli-processor-refactor/implementation-plan.md +++ b/PRDs/20251217-cli-processor-refactor/implementation-plan.md @@ -1,7 +1,5 @@ # CLIProcessor Refactoring Implementation Plan -> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. - **Goal:** Refactor `processCommand` method and extract `CallingContext` to reduce complexity and improve testability. **Architecture:** Extract `CallingContext` inner class to a package-private top-level class. Refactor `processCommand()` into discrete phases using Optional-based result chaining. Each phase either returns an exit status (stop) or empty (continue). @@ -16,20 +14,20 @@ **Prerequisites:** PR #551 must be merged into `develop` -**Step 1: Fetch latest develop** +### Step 1: Fetch latest develop ```bash cd ../metaschema-java-252 git fetch origin develop ``` -**Step 2: Rebase branch** +### Step 2: Rebase branch ```bash git rebase origin/develop ``` -**Step 3: Verify build** +### Step 3: Verify build ```bash mvn -pl cli-processor test @@ -47,7 +45,7 @@ Expected: BUILD SUCCESS - Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java` - Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommand.java` -**Step 1: Create TestVersionInfo** +### Step 1: Create TestVersionInfo ```java /* @@ -110,7 +108,7 @@ class TestVersionInfo implements IVersionInfo { } ``` -**Step 2: Create TestCommand** +### Step 2: Create TestCommand ```java /* @@ -160,7 +158,7 @@ class TestCommand extends AbstractTerminalCommand { } ``` -**Step 3: Verify compilation** +### Step 3: Verify compilation ```bash mvn -pl cli-processor test-compile @@ -168,7 +166,7 @@ mvn -pl cli-processor test-compile Expected: BUILD SUCCESS -**Step 4: Commit** +### Step 4: Commit ```bash git add cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestVersionInfo.java @@ -183,7 +181,7 @@ git commit -m "test: add test fixtures for CLIProcessor testing" **Files:** - Create: `cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java` -**Step 1: Write integration test class with version test** +### Step 1: Write integration test class with version test ```java /* @@ -308,7 +306,7 @@ class CLIProcessorTest { } ``` -**Step 2: Run tests to verify they pass with existing code** +### Step 2: Run tests to verify they pass with existing code ```bash mvn -pl cli-processor test -Dtest=CLIProcessorTest @@ -316,7 +314,7 @@ mvn -pl cli-processor test -Dtest=CLIProcessorTest Expected: BUILD SUCCESS (all tests pass - these characterize existing behavior) -**Step 3: Commit** +### Step 3: Commit ```bash git add cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessorTest.java @@ -332,7 +330,7 @@ git commit -m "test: add integration tests for CLIProcessor existing behavior" **Files:** - Create: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java` -**Step 1: Create new CallingContext.java file** +### Step 1: Create new CallingContext.java file Copy the inner class to a new file, adding package-private visibility and updating `CLIProcessor.this` references: @@ -842,7 +840,7 @@ class CallingContext { } ``` -**Step 2: Verify compilation** +### Step 2: Verify compilation ```bash mvn -pl cli-processor compile @@ -850,7 +848,7 @@ mvn -pl cli-processor compile Expected: BUILD SUCCESS -**Step 3: Commit** +### Step 3: Commit ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -864,7 +862,7 @@ git commit -m "refactor: extract CallingContext to top-level package-private cla **Files:** - Modify: `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java` -**Step 1: Add package-private getter for outputStream** +### Step 1: Add package-private getter for outputStream Add this method to `CLIProcessor.java` (needed by extracted `CallingContext`): @@ -880,7 +878,7 @@ PrintStream getOutputStream() { } ``` -**Step 2: Make OPTIONS package-private** +### Step 2: Make OPTIONS package-private Change the `OPTIONS` field visibility from `private` to package-private: @@ -894,7 +892,7 @@ static final List

@@ -27,7 +29,9 @@ @DisplayName("CLIProcessor Integration Tests") class CLIProcessorTest { + @NonNull private CLIProcessor processor; + @NonNull private ByteArrayOutputStream outputCapture; @BeforeEach diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java index 72e73c54f..a96d528cc 100644 --- a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java @@ -11,6 +11,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import edu.umd.cs.findbugs.annotations.NonNull; + import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.ParseException; import org.junit.jupiter.api.BeforeEach; @@ -42,7 +44,8 @@ void setUp() { nullOutput); } - private CallingContext createContext(String... args) { + @NonNull + private CallingContext createContext(@NonNull String... args) { return new CallingContext(processor, Arrays.asList(args)); } From 8dd8f722c522ded697b6e9ef203229a73c917f90 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 21 Dec 2025 16:52:54 -0500 Subject: [PATCH 13/15] test: add edge case tests for CallingContext phase methods - ParseOptionsTests: multiple invalid options - ValidateExtraArgumentsTests: required argument missing, too many arguments - ValidateCalledCommandsTests: required option missing/provided - ApplyGlobalOptionsTests: --no-color, both --quiet and --no-color Added test fixtures: - TestCommandWithRequiredArg: command requiring an extra argument - TestCommandWithRequiredOption: command requiring a specific option --- .../cli/processor/CallingContextTest.java | 82 +++++++++++++++++++ .../processor/TestCommandWithRequiredArg.java | 54 ++++++++++++ .../TestCommandWithRequiredOption.java | 70 ++++++++++++++++ 3 files changed, 206 insertions(+) create mode 100644 cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredArg.java create mode 100644 cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredOption.java diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java index a96d528cc..b3527f250 100644 --- a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/CallingContextTest.java @@ -111,6 +111,14 @@ void throwsOnInvalidOption() { assertThrows(ParseException.class, ctx::parseOptions); } + + @Test + @DisplayName("throws on multiple invalid options") + void throwsOnMultipleInvalidOptions() { + CallingContext ctx = createContext("--invalid-one", "--invalid-two", "--invalid-three"); + + assertThrows(ParseException.class, ctx::parseOptions); + } } @Nested @@ -139,6 +147,34 @@ void returnsEmptyWhenArgumentsValid() throws ParseException { assertTrue(result.isEmpty()); } + + @Test + @DisplayName("returns error when required argument missing") + void returnsErrorWhenRequiredArgumentMissing() throws ParseException { + processor.addCommandHandler(new TestCommandWithRequiredArg()); + CallingContext ctx = createContext("test-cmd-with-arg"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateExtraArguments(cmdLine); + + assertAll( + () -> assertTrue(result.isPresent()), + () -> assertEquals(ExitCode.INVALID_ARGUMENTS, result.get().getExitCode())); + } + + @Test + @DisplayName("returns error when too many arguments provided") + void returnsErrorWhenTooManyArguments() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd", "extra-arg-1", "extra-arg-2"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateExtraArguments(cmdLine); + + assertAll( + () -> assertTrue(result.isPresent()), + () -> assertEquals(ExitCode.INVALID_ARGUMENTS, result.get().getExitCode())); + } } @Nested @@ -156,6 +192,32 @@ void returnsEmptyWhenAllCommandsValid() throws ParseException { assertTrue(result.isEmpty()); } + + @Test + @DisplayName("returns error when required option missing") + void returnsErrorWhenRequiredOptionMissing() throws ParseException { + processor.addCommandHandler(new TestCommandWithRequiredOption()); + CallingContext ctx = createContext("test-cmd-with-option"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateCalledCommands(cmdLine); + + assertAll( + () -> assertTrue(result.isPresent()), + () -> assertEquals(ExitCode.INVALID_COMMAND, result.get().getExitCode())); + } + + @Test + @DisplayName("returns empty when required option provided") + void returnsEmptyWhenRequiredOptionProvided() throws ParseException { + processor.addCommandHandler(new TestCommandWithRequiredOption()); + CallingContext ctx = createContext("test-cmd-with-option", "--required-opt", "value"); + CommandLine cmdLine = ctx.parseOptions(); + + Optional result = ctx.validateCalledCommands(cmdLine); + + assertTrue(result.isEmpty()); + } } @Nested @@ -171,5 +233,25 @@ void appliesQuietOption() throws ParseException { assertDoesNotThrow(() -> ctx.applyGlobalOptions(cmdLine)); } + + @Test + @DisplayName("applies --no-color without error") + void appliesNoColorOption() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd", "--no-color"); + CommandLine cmdLine = ctx.parseOptions(); + + assertDoesNotThrow(() -> ctx.applyGlobalOptions(cmdLine)); + } + + @Test + @DisplayName("applies both --quiet and --no-color without error") + void appliesBothQuietAndNoColor() throws ParseException { + processor.addCommandHandler(new TestCommand()); + CallingContext ctx = createContext("test-cmd", "--quiet", "--no-color"); + CommandLine cmdLine = ctx.parseOptions(); + + assertDoesNotThrow(() -> ctx.applyGlobalOptions(cmdLine)); + } } } diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredArg.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredArg.java new file mode 100644 index 000000000..69369d4b1 --- /dev/null +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredArg.java @@ -0,0 +1,54 @@ +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; +import gov.nist.secauto.metaschema.cli.processor.command.ExtraArgument; +import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; + +import org.apache.commons.cli.CommandLine; + +import java.util.List; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A test command that requires an extra argument. + */ +class TestCommandWithRequiredArg + extends AbstractTerminalCommand { + + @Override + @NonNull + public String getName() { + return "test-cmd-with-arg"; + } + + @Override + @NonNull + public String getDescription() { + return "A test command requiring an argument"; + } + + @Override + public List getExtraArguments() { + return List.of( + ExtraArgument.newInstance("required-file", true)); + } + + @Override + public ICommandExecutor newExecutor( + @NonNull CallingContext callingContext, + @NonNull CommandLine cmdLine) { + return ICommandExecutor.using(callingContext, cmdLine, this::executeCommand); + } + + private void executeCommand( + @NonNull CallingContext callingContext, + @NonNull CommandLine cmdLine) { + // Do nothing - success + } +} diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredOption.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredOption.java new file mode 100644 index 000000000..535f22e41 --- /dev/null +++ b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/TestCommandWithRequiredOption.java @@ -0,0 +1,70 @@ +/* + * SPDX-FileCopyrightText: none + * SPDX-License-Identifier: CC0-1.0 + */ + +package gov.nist.secauto.metaschema.cli.processor; + +import gov.nist.secauto.metaschema.cli.processor.command.AbstractTerminalCommand; +import gov.nist.secauto.metaschema.cli.processor.command.ICommandExecutor; + +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.Option; + +import java.util.Collection; +import java.util.List; + +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A test command that requires a specific option. + */ +class TestCommandWithRequiredOption + extends AbstractTerminalCommand { + + private static final Option REQUIRED_OPTION = Option.builder() + .longOpt("required-opt") + .desc("A required option for testing") + .hasArg() + .argName("VALUE") + .build(); + + @Override + @NonNull + public String getName() { + return "test-cmd-with-option"; + } + + @Override + @NonNull + public String getDescription() { + return "A test command requiring an option"; + } + + @Override + public Collection gatherOptions() { + return List.of(REQUIRED_OPTION); + } + + @Override + public void validateOptions( + @NonNull CallingContext callingContext, + @NonNull CommandLine cmdLine) throws InvalidArgumentException { + if (!cmdLine.hasOption(REQUIRED_OPTION)) { + throw new InvalidArgumentException("The '--required-opt' option is required."); + } + } + + @Override + public ICommandExecutor newExecutor( + @NonNull CallingContext callingContext, + @NonNull CommandLine cmdLine) { + return ICommandExecutor.using(callingContext, cmdLine, this::executeCommand); + } + + private void executeCommand( + @NonNull CallingContext callingContext, + @NonNull CommandLine cmdLine) { + // Do nothing - success + } +} From 99c691427d14a2eb8bdfda6f5b0c6f0f0cda6c48 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 21 Dec 2025 17:08:16 -0500 Subject: [PATCH 14/15] docs: fix implementation-plan.md to match actual code - Update TestCommand to use CallingContext instead of CLIProcessor.CallingContext - Add @NonNull annotations to createContext helper method --- .../implementation-plan.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/PRDs/20251217-cli-processor-refactor/implementation-plan.md b/PRDs/20251217-cli-processor-refactor/implementation-plan.md index 12a800231..6f22af7dc 100644 --- a/PRDs/20251217-cli-processor-refactor/implementation-plan.md +++ b/PRDs/20251217-cli-processor-refactor/implementation-plan.md @@ -144,14 +144,13 @@ class TestCommand extends AbstractTerminalCommand { @Override public ICommandExecutor newExecutor( - @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CallingContext callingContext, @NonNull CommandLine cmdLine) { return ICommandExecutor.using(callingContext, cmdLine, this::executeCommand); } - @NonNull private void executeCommand( - @NonNull CLIProcessor.CallingContext callingContext, + @NonNull CallingContext callingContext, @NonNull CommandLine cmdLine) { // Do nothing - success } @@ -1046,7 +1045,8 @@ class CallingContextTest { nullOutput); } - private CallingContext createContext(String... args) { + @NonNull + private CallingContext createContext(@NonNull String... args) { return new CallingContext(processor, Arrays.asList(args)); } From 3a1d0f619211a146070e4b33c9ca2c74a3b6b31b Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 21 Dec 2025 17:23:13 -0500 Subject: [PATCH 15/15] docs: fix duplicate markdown headings in implementation-plan.md --- .../implementation-plan.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/PRDs/20251217-cli-processor-refactor/implementation-plan.md b/PRDs/20251217-cli-processor-refactor/implementation-plan.md index 6f22af7dc..d6e500d67 100644 --- a/PRDs/20251217-cli-processor-refactor/implementation-plan.md +++ b/PRDs/20251217-cli-processor-refactor/implementation-plan.md @@ -982,7 +982,7 @@ mvn -pl cli-processor test Expected: BUILD SUCCESS -### Step 3: Commit +### Step 3: Commit ShellCompletionCommand update ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java @@ -1289,7 +1289,7 @@ mvn -pl cli-processor test -Dtest=CallingContextTest#*parseOptions* Expected: BUILD SUCCESS -### Step 3: Commit +### Step 3: Commit parseOptions method ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -1340,7 +1340,7 @@ mvn -pl cli-processor test -Dtest=CallingContextTest#*validateExtraArguments* Expected: BUILD SUCCESS -### Step 3: Commit +### Step 3: Commit validateExtraArguments method ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -1388,7 +1388,7 @@ mvn -pl cli-processor test -Dtest=CallingContextTest#*validateCalledCommands* Expected: BUILD SUCCESS -### Step 3: Commit +### Step 3: Commit validateCalledCommands method ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java @@ -1430,7 +1430,7 @@ mvn -pl cli-processor test -Dtest=CallingContextTest#*applyGlobalOptions* Expected: BUILD SUCCESS -### Step 3: Commit +### Step 3: Commit applyGlobalOptions method ```bash git add cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CallingContext.java