diff --git a/CLAUDE.md b/CLAUDE.md index 9dc0a29..15a0909 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,18 +122,41 @@ The project follows clean architecture principles with three main layers: - Business logic and use case implementations - Service implementations for core functionality: - - **Commands**: Browse, Info, New, Project commands - - **FileSystem**: File operations and copying services - - **GitHub**: GitHub repository and template handling - - **Project**: Project model management and policy enforcement - - **Data**: Repository pattern implementations with multiple store types - - **Utility**: Serialization services (JSON, YAML, Environment files) + - **Commands**: Browse, Info, New, Project commands + - **FileSystem**: File operations and copying services + - **GitHub**: GitHub repository and template handling + - **Project**: Project model management and policy enforcement + - **Data**: Repository pattern implementations with multiple store types + - **Utility**: Serialization services (JSON, YAML, Environment files) #### Presentation Layer (`src/Keystone.Cli/Presentation/`) - Command controllers using Cocona framework +- Decoupled from Application layer—can swap CLI frameworks without affecting business logic - User interface and command-line interaction -- Controllers: `BrowseCommandController`, `InfoCommandController`, `NewCommandController`, `SubCommandsHostController` +- Controllers: `BrowseCommandController`, `InfoCommandController`, `NewCommandController`, + `SubCommandsHostController` + +##### Cocona Conventions + +Cocona exposes method parameters as CLI options by default. To keep help output clean: + +- **Never** add `CancellationToken` as a command method parameter—it appears as + `--cancellation-token` in help output +- Instead, inject `ICoconaAppContextAccessor` via constructor and access the token from context: + +```csharp +public class MyCommand(ICoconaAppContextAccessor contextAccessor) +{ + public async Task RunAsync() + { + var cancellationToken = contextAccessor.Current?.CancellationToken ?? CancellationToken.None; + // use cancellationToken... + } +} +``` + +This convention is enforced by `CoconaCommandMethodConventionsTests`. ### Key Components @@ -166,6 +189,15 @@ Tests mirror the source structure in `tests/Keystone.Cli.UnitTests/`: - Uses NUnit testing framework with NSubstitute for mocking - Tests cover service implementations and command logic +#### Cocona Test Utilities (`Presentation/Cocona/`) + +Framework-specific test infrastructure isolated from general test utilities: + +- `CoconaAppContextFactory` — Creates `CoconaAppContext` instances for testing commands + that need `ICoconaAppContextAccessor` +- `CoconaCommandMethodConventionsTests` — Enforces conventions (e.g., no `CancellationToken` + parameters in command methods) + ### Configuration - Application settings in `appsettings.json` and `appsettings.Development.json` diff --git a/src/Keystone.Cli/Presentation/NewCommandController.cs b/src/Keystone.Cli/Presentation/NewCommandController.cs index 840d927..04d70ee 100644 --- a/src/Keystone.Cli/Presentation/NewCommandController.cs +++ b/src/Keystone.Cli/Presentation/NewCommandController.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Cocona; +using Cocona.Application; using Keystone.Cli.Application.Commands.New; using Keystone.Cli.Application.Utility; using Keystone.Cli.Domain; @@ -12,7 +13,11 @@ namespace Keystone.Cli.Presentation; /// /// The "new" command controller. /// -public class NewCommandController(IConsole console, INewCommand newCommand) +public class NewCommandController( + ICoconaAppContextAccessor contextAccessor, + IConsole console, + INewCommand newCommand +) { [Command("new", Description = "Creates a new project from a template")] public async Task NewAsync( @@ -29,6 +34,8 @@ public async Task NewAsync( bool includeGitFiles = false ) { + var cancellationToken = contextAccessor.Current?.CancellationToken ?? CancellationToken.None; + var fullPath = string.IsNullOrWhiteSpace(projectPath) ? Path.Combine(Path.GetFullPath("."), ProjectNamePolicy.GetProjectDirectoryName(projectName)) : Path.GetFullPath(projectPath); @@ -40,7 +47,7 @@ await newCommand.CreateNewAsync( templateName, fullPath, includeGitFiles, - CancellationToken.None + cancellationToken ); return CliCommandResults.Success; diff --git a/src/Keystone.Cli/Presentation/Project/SwitchTemplateSubCommand.cs b/src/Keystone.Cli/Presentation/Project/SwitchTemplateSubCommand.cs index 51f149f..e3cb455 100644 --- a/src/Keystone.Cli/Presentation/Project/SwitchTemplateSubCommand.cs +++ b/src/Keystone.Cli/Presentation/Project/SwitchTemplateSubCommand.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Cocona; +using Cocona.Application; using Keystone.Cli.Application.Commands.Project; using Keystone.Cli.Application.Utility; using Keystone.Cli.Domain; @@ -12,7 +13,11 @@ namespace Keystone.Cli.Presentation.Project; /// /// The implementation of the "switch-template" sub-command for the project command. /// -public class SwitchTemplateSubCommand(IConsole console, IProjectCommand projectCommand) +public class SwitchTemplateSubCommand( + ICoconaAppContextAccessor contextAccessor, + IConsole console, + IProjectCommand projectCommand +) { public async Task SwitchTemplateAsync( [Argument(Description = "The name of the new template to switch to"), @@ -20,10 +25,11 @@ public async Task SwitchTemplateAsync( string newTemplateName, [Option(Description = "The path to the project where the template should be switched"), Path, NotPaddedWhitespace] - string? projectPath = null, - CancellationToken cancellationToken = default + string? projectPath = null ) { + var cancellationToken = contextAccessor.Current?.CancellationToken ?? CancellationToken.None; + var fullPath = string.IsNullOrWhiteSpace(projectPath) ? Path.GetFullPath(".") : Path.GetFullPath(projectPath); diff --git a/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaAppContextFactory.cs b/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaAppContextFactory.cs new file mode 100644 index 0000000..50384a3 --- /dev/null +++ b/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaAppContextFactory.cs @@ -0,0 +1,45 @@ +using System.Reflection; +using Cocona; +using Cocona.Command; + + +namespace Keystone.Cli.UnitTests.Presentation.Cocona; + +/// +/// Factory for creating instances in tests. +/// +public static class CoconaAppContextFactory +{ + /// + /// Creates a with the specified cancellation token. + /// + /// The cancellation token to use. + /// A new instance. + public static CoconaAppContext Create(CancellationToken cancellationToken = default) + { + var dummyMethod = typeof(CoconaAppContextFactory) + .GetMethod(nameof(DummyCommand), BindingFlags.NonPublic | BindingFlags.Static)!; + + var commandDescriptor = new CommandDescriptor( + dummyMethod, + target: null, + name: "dummy", + aliases: [], + description: string.Empty, + metadata: [], + parameters: [], + options: [], + arguments: [], + overloads: [], + optionLikeCommands: [], + flags: CommandFlags.None, + subCommands: null + ); + + return new CoconaAppContext(commandDescriptor, cancellationToken); + } + + private static void DummyCommand() + { + } +} diff --git a/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaCommandMethodConventionsTests.cs b/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaCommandMethodConventionsTests.cs new file mode 100644 index 0000000..a575fc2 --- /dev/null +++ b/tests/Keystone.Cli.UnitTests/Presentation/Cocona/CoconaCommandMethodConventionsTests.cs @@ -0,0 +1,104 @@ +using System.Reflection; +using Cocona; +using Cocona.Application; +using Keystone.Cli.Presentation; + + +namespace Keystone.Cli.UnitTests.Presentation.Cocona; + +[TestFixture, Parallelizable(ParallelScope.All)] +public class CoconaCommandMethodConventionsTests +{ + private const string PresentationNamespace = "Keystone.Cli.Presentation"; + + /// + /// Cocona exposes method parameters as CLI options by default. parameters + /// should not appear in help output as they are framework infrastructure, not user-actionable arguments. + /// + /// + /// To access in a command method, inject + /// via the constructor and use its property, + /// e.g., contextAccessor.Current?.CancellationToken. + /// + [Test] + public void CommandMethods_ShouldNotExposeCancellationTokenAsParameter() + { + MethodInfo[] commandMethods = [..DiscoverCommandMethods().Distinct()]; + + string[] violations = + [ + ..commandMethods.SelectMany(method => method + .GetParameters() + .Where(p => p.ParameterType == typeof(CancellationToken)) + .Select(p => $"{method.DeclaringType!.Name}.{method.Name}({p.Name})") + ), + ]; + + Assert.That( + violations, + Is.Empty, + $""" + {nameof(CancellationToken)} parameters are exposed as CLI options. + Use {nameof(ICoconaAppContextAccessor)} instead. + + Violations: + {string.Join(Environment.NewLine, violations.Select(violation => $"- {violation}"))} + """ + ); + } + + private static IEnumerable DiscoverCommandMethods() + { + Type[] controllerTypes = + [ + ..typeof(BrowseCommandController).Assembly + .GetTypes() + .Where(t => t.Namespace?.StartsWith(PresentationNamespace) == true) + .Where(t => t is { IsClass: true, IsPublic: true } && t.Name.EndsWith("Controller")), + ]; + + foreach (var controllerType in controllerTypes) + { + // Methods with [Command] attribute + foreach (var method in GetMethodsWithCommandAttribute(controllerType)) + { + yield return method; + } + + // Recursively resolve [HasSubCommands] targets + foreach (var method in GetSubCommandMethods(controllerType)) + { + yield return method; + } + } + } + + private static IEnumerable GetMethodsWithCommandAttribute(Type type) + => type + .GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly) + .Where(m => m.GetCustomAttribute() != null); + + private static IEnumerable GetSubCommandMethods(Type type) + { + foreach (var attr in type.GetCustomAttributes()) + { + var subCommandType = attr.Type; + + // Get public *Async methods from subcommand types + var asyncMethods = subCommandType + .GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly) + .Where(m => m.Name.EndsWith("Async")); + + foreach (var method in asyncMethods) + { + yield return method; + } + + // Recurse into nested [HasSubCommands] + foreach (var method in GetSubCommandMethods(subCommandType)) + { + yield return method; + } + } + } +} diff --git a/tests/Keystone.Cli.UnitTests/Presentation/NewCommandControllerTests.cs b/tests/Keystone.Cli.UnitTests/Presentation/NewCommandControllerTests.cs index ef8ab7e..b4a3149 100644 --- a/tests/Keystone.Cli.UnitTests/Presentation/NewCommandControllerTests.cs +++ b/tests/Keystone.Cli.UnitTests/Presentation/NewCommandControllerTests.cs @@ -1,8 +1,10 @@ +using Cocona.Application; using Keystone.Cli.Application.Commands.New; using Keystone.Cli.Domain; using Keystone.Cli.Domain.Policies; using Keystone.Cli.Presentation; using Keystone.Cli.UnitTests.Application.Utility; +using Keystone.Cli.UnitTests.Presentation.Cocona; using NSubstitute; @@ -11,8 +13,15 @@ namespace Keystone.Cli.UnitTests.Presentation; [TestFixture, Parallelizable(ParallelScope.All)] public class NewCommandControllerTests { - private static NewCommandController Ctor(INewCommand? newCommand = null) - => new(NullConsole.Instance, newCommand ?? Substitute.For()); + private static NewCommandController Ctor( + ICoconaAppContextAccessor? contextAccessor = null, + INewCommand? newCommand = null + ) + => new( + contextAccessor ?? Substitute.For(), + NullConsole.Instance, + newCommand ?? Substitute.For() + ); [Test] public async Task NewAsync_OnSuccess_ReturnsCliSuccessAsync() @@ -36,7 +45,7 @@ public async Task NewAsync_WhenPathIsNotProvided_UsesDefaultPathAsync(string? pa var newCommand = Substitute.For(); - var sut = Ctor(newCommand); + var sut = Ctor(newCommand: newCommand); await sut.NewAsync(name, templateName, path); await newCommand.Received(1).CreateNewAsync( @@ -44,7 +53,7 @@ await newCommand.Received(1).CreateNewAsync( templateName, Path.Combine(Path.GetFullPath("."), ProjectNamePolicy.GetProjectDirectoryName(name)), includeGitFiles: false, - CancellationToken.None + Arg.Any() ); } @@ -56,7 +65,7 @@ public async Task NewAsync_WhenRelativePathIsProvided_UsesFullPathAsync() var newCommand = Substitute.For(); - var sut = Ctor(newCommand); + var sut = Ctor(newCommand: newCommand); await sut.NewAsync(name, templateName, projectPath: "."); await newCommand.Received(1).CreateNewAsync( @@ -64,7 +73,7 @@ await newCommand.Received(1).CreateNewAsync( templateName, Path.GetFullPath("."), includeGitFiles: false, - CancellationToken.None + Arg.Any() ); } @@ -76,7 +85,7 @@ public async Task NewAsync_UsesIncludeGitFilesOptionAsync() var newCommand = Substitute.For(); - var sut = Ctor(newCommand); + var sut = Ctor(newCommand: newCommand); await sut.NewAsync(name, templateName, includeGitFiles: true); await newCommand.Received(1).CreateNewAsync( @@ -84,7 +93,7 @@ await newCommand.Received(1).CreateNewAsync( templateName, Arg.Any(), includeGitFiles: true, - CancellationToken.None + Arg.Any() ); } @@ -97,10 +106,10 @@ public async Task NewAsync_OnKeyNotFoundException_ReturnsCliErrorAsync() var newCommand = Substitute.For(); newCommand - .When(stub => stub.CreateNewAsync(name, templateName, Arg.Any(), Arg.Any(), CancellationToken.None)) + .When(stub => stub.CreateNewAsync(name, templateName, Arg.Any(), Arg.Any(), Arg.Any())) .Do(_ => throw new KeyNotFoundException()); - var sut = Ctor(newCommand); + var sut = Ctor(newCommand: newCommand); var actual = await sut.NewAsync(name, templateName); Assert.That(actual, Is.EqualTo(CliCommandResults.Error)); @@ -115,12 +124,40 @@ public async Task NewAsync_OnInvalidOperationException_ReturnsCliErrorAsync() var newCommand = Substitute.For(); newCommand - .When(stub => stub.CreateNewAsync(name, templateName, Arg.Any(), Arg.Any(), CancellationToken.None)) + .When(stub => stub.CreateNewAsync(name, templateName, Arg.Any(), Arg.Any(), Arg.Any())) .Do(_ => throw new InvalidOperationException()); - var sut = Ctor(newCommand); + var sut = Ctor(newCommand: newCommand); var actual = await sut.NewAsync(name, templateName); Assert.That(actual, Is.EqualTo(CliCommandResults.Error)); } + + [Test] + public async Task NewAsync_UsesCancellationTokenFromContextAsync() + { + const string name = "project-name"; + const string templateName = "template-name"; + + using var cts = new CancellationTokenSource(); + var expectedToken = cts.Token; + + var context = CoconaAppContextFactory.Create(expectedToken); + + var contextAccessor = Substitute.For(); + contextAccessor.Current.Returns(context); + + var newCommand = Substitute.For(); + + var sut = Ctor(contextAccessor, newCommand); + await sut.NewAsync(name, templateName); + + await newCommand.Received(1).CreateNewAsync( + name, + templateName, + Arg.Any(), + Arg.Any(), + expectedToken + ); + } } diff --git a/tests/Keystone.Cli.UnitTests/Presentation/Project/SwitchTemplateSubCommandTests.cs b/tests/Keystone.Cli.UnitTests/Presentation/Project/SwitchTemplateSubCommandTests.cs index 35356f7..5fc9352 100644 --- a/tests/Keystone.Cli.UnitTests/Presentation/Project/SwitchTemplateSubCommandTests.cs +++ b/tests/Keystone.Cli.UnitTests/Presentation/Project/SwitchTemplateSubCommandTests.cs @@ -1,8 +1,10 @@ +using Cocona.Application; using Keystone.Cli.Application.Commands.Project; using Keystone.Cli.Domain; using Keystone.Cli.Domain.Project; using Keystone.Cli.Presentation.Project; using Keystone.Cli.UnitTests.Application.Utility; +using Keystone.Cli.UnitTests.Presentation.Cocona; using NSubstitute; using NSubstitute.ExceptionExtensions; @@ -12,8 +14,15 @@ namespace Keystone.Cli.UnitTests.Presentation.Project; [TestFixture, Parallelizable(ParallelScope.All)] public class SwitchTemplateSubCommandTests { - private static SwitchTemplateSubCommand Ctor(IProjectCommand? projectCommand = null) - => new(NullConsole.Instance, projectCommand ?? Substitute.For()); + private static SwitchTemplateSubCommand Ctor( + ICoconaAppContextAccessor? contextAccessor = null, + IProjectCommand? projectCommand = null + ) + => new( + contextAccessor ?? Substitute.For(), + NullConsole.Instance, + projectCommand ?? Substitute.For() + ); [Test] public async Task SwitchTemplateAsync_OnSuccess_ReturnsCliSuccessAsync() @@ -27,7 +36,7 @@ public async Task SwitchTemplateAsync_OnSuccess_ReturnsCliSuccessAsync() .SwitchTemplateAsync(newTemplateName, projectPath, Arg.Any()) .Returns(true); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); var actual = await sut.SwitchTemplateAsync(newTemplateName, projectPath); Assert.That(actual, Is.EqualTo(CliCommandResults.Success)); @@ -45,7 +54,7 @@ public async Task SwitchTemplateAsync_TemplateNotFound_ReturnsCliFailureAsync() .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) .ThrowsAsync(new KeyNotFoundException($"Template '{newTemplateName}' not found.")); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); var actual = await sut.SwitchTemplateAsync(newTemplateName, projectPath); Assert.That(actual, Is.EqualTo(CliCommandResults.Error)); @@ -63,7 +72,7 @@ public async Task SwitchTemplateAsync_ProjectNotLoaded_ReturnsCliFailureAsync() .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) .ThrowsAsync(new ProjectNotLoadedException("Failed to load project.")); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); var actual = await sut.SwitchTemplateAsync(newTemplateName, projectPath); Assert.That(actual, Is.EqualTo(CliCommandResults.Error)); @@ -80,7 +89,7 @@ public async Task SwitchTemplateAsync_ProjectPathIsNull_UsesCurrentDirectoryAsyn .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) .Returns(true); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); await sut.SwitchTemplateAsync(newTemplateName, projectPath: null); await projectCommand.Received(1).SwitchTemplateAsync( @@ -101,7 +110,7 @@ public async Task SwitchTemplateAsync_ProjectPathIsEmpty_UsesCurrentDirectoryAsy .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) .Returns(true); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); await sut.SwitchTemplateAsync(newTemplateName, projectPath: string.Empty); await projectCommand.Received(1).SwitchTemplateAsync( @@ -123,7 +132,7 @@ public async Task SwitchTemplateAsync_ResolvesFullPathForProjectPathAsync() .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) .Returns(true); - var sut = Ctor(projectCommand); + var sut = Ctor(projectCommand: projectCommand); await sut.SwitchTemplateAsync(newTemplateName, projectPath); await projectCommand.Received(1).SwitchTemplateAsync( @@ -132,4 +141,33 @@ await projectCommand.Received(1).SwitchTemplateAsync( Arg.Any() ); } + + [Test] + public async Task SwitchTemplateAsync_UsesCancellationTokenFromContextAsync() + { + const string newTemplateName = "new-template"; + const string projectPath = "."; + + using var cts = new CancellationTokenSource(); + var expectedToken = cts.Token; + + var context = CoconaAppContextFactory.Create(expectedToken); + + var contextAccessor = Substitute.For(); + contextAccessor.Current.Returns(context); + + var projectCommand = Substitute.For(); + projectCommand + .SwitchTemplateAsync(newTemplateName, Arg.Any(), Arg.Any()) + .Returns(true); + + var sut = Ctor(contextAccessor, projectCommand); + await sut.SwitchTemplateAsync(newTemplateName, projectPath); + + await projectCommand.Received(1).SwitchTemplateAsync( + newTemplateName, + Arg.Any(), + expectedToken + ); + } }