diff --git a/docs/adr/0032-dependency-injection-foundation.md b/docs/adr/0032-dependency-injection-foundation.md new file mode 100644 index 0000000..8402ba6 --- /dev/null +++ b/docs/adr/0032-dependency-injection-foundation.md @@ -0,0 +1,124 @@ +--- +number: '0032' +title: 'Dependency Injection Foundation' +status: accepted +date: 2026-01-16 +decision-makers: + - '@mcj-coder' + - '@martincjarvis' +context: Architecture +--- + +# ADR-0032: Dependency Injection Foundation + +## Status + +Accepted (2026-01-16) + +## Context + +The codebase had grown to include significant business logic in Program.cs (~560 lines) with direct +dependencies on static classes and the Console/Environment APIs. This made unit testing difficult: + +- Program.cs contained argument parsing, validation, help output, diagnostics +- ClaudeMonitor depended directly on Console, Environment, and File static APIs +- Static classes like LoggingConfiguration couldn't be mocked +- Test coverage was limited to integration-level testing + +We needed a testable architecture that: + +1. Enables unit testing of business logic in isolation +2. Reduces coupling between components +3. Establishes patterns for future development +4. Maintains backward compatibility with existing CLI behavior + +## Decision + +Implement dependency injection using `Microsoft.Extensions.Hosting` with the following structure: + +### Core Interfaces + +| Interface | Purpose | +| --------------------- | --------------------------------- | +| `IApplication` | Main application entry point | +| `IConsoleService` | Console I/O abstraction | +| `IEnvironmentService` | Environment and filesystem access | +| `IArgumentParser` | CLI argument parsing | +| `IClaudeMonitor` | Claude process monitoring | + +### Service Lifetimes + +| Service | Lifetime | Rationale | +| --------------------- | --------- | ----------------------------------- | +| `IConsoleService` | Singleton | Stateless, wraps static Console | +| `IEnvironmentService` | Singleton | Stateless, wraps static Environment | +| `IArgumentParser` | Singleton | Stateless | +| `IApplication` | Singleton | Single run per process | +| `ClaudeMonitor` | Transient | Stateful, disposable | + +### Program.cs Structure + +Minimal host setup (~70 lines): + +```csharp +var host = Host.CreateDefaultBuilder(args) + .ConfigureServices(services => + { + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }) + .UseSerilog() + .Build(); + +var app = host.Services.GetRequiredService(); +return await app.RunAsync(args); +``` + +## Consequences + +### Positive + +- **Testability**: All business logic can be unit tested with mocked dependencies +- **Separation of Concerns**: Clear boundaries between components +- **Maintainability**: Smaller, focused classes (Program.cs reduced from ~560 to ~70 lines) +- **Extensibility**: Easy to add new services or swap implementations +- **Consistency**: Standard Microsoft.Extensions.Hosting patterns + +### Negative + +- **Complexity**: More files and indirection than static approach +- **Learning Curve**: Contributors need to understand DI patterns +- **Bootstrap Limitation**: Some code (logging setup) must run before DI is available + +### Neutral + +- **Performance**: Negligible overhead for CLI application +- **Migration**: Existing tests updated to use mock services + +## Alternatives Considered + +### 1. Manual DI (No Framework) + +Pass dependencies through constructors without a DI container. + +**Rejected**: Missing lifetime management, service resolution, and host integration benefits. + +### 2. Third-Party DI (Autofac, etc.) + +Use a third-party DI container instead of Microsoft's. + +**Rejected**: Microsoft.Extensions.Hosting is sufficient and already bundled with Serilog.Extensions.Hosting. + +### 3. Static Factory Pattern + +Keep static classes but add factory methods returning interfaces. + +**Rejected**: Doesn't solve the fundamental testability problem. + +## Related + +- [Coding Standards: Dependency Injection](../standards/coding-standards/dependency-injection.md) +- [Design Document](../plans/2026-01-16-dependency-injection-foundation-design.md) +- Issue #116, #117, #118 diff --git a/docs/standards/coding-standards.md b/docs/standards/coding-standards.md index bd15fa3..88fe392 100644 --- a/docs/standards/coding-standards.md +++ b/docs/standards/coding-standards.md @@ -228,6 +228,46 @@ See [performance-examples.md](coding-standards/performance-examples.md) for patt See [security-examples.md](coding-standards/security-examples.md) for examples. +## Dependency Injection & Testability + +### DI Principles + +- **Constructor injection preferred** over property or method injection +- **Interface-first design**: Define interface before implementation +- **Single Responsibility**: One reason to change per class +- **Explicit dependencies**: All dependencies via constructor, no hidden statics + +### Service Lifetimes + +| Lifetime | Use For | Example | +| --------- | --------------------------------- | ----------------- | +| Singleton | Stateless services, shared state | `IConsoleService` | +| Scoped | Per-request/operation state | Database contexts | +| Transient | Lightweight, stateful, disposable | `ClaudeMonitor` | + +### Avoid Static Classes For + +- Business logic +- External resource access (file system, network, environment) +- Anything that needs to be mocked in tests +- Classes with state or dependencies + +### Static Classes Acceptable For + +- Pure extension methods +- Mathematical/string utilities with no side effects +- Constants-only classes +- Bootstrap code (before DI container is available) + +### Testability Requirements + +- New classes MUST be unit-testable in isolation +- External dependencies MUST be abstracted behind interfaces +- No direct `new` of classes with external dependencies in business logic +- Prefer constructor injection over service locator pattern + +See [dependency-injection.md](coding-standards/dependency-injection.md) for patterns. + ## Code Review Checklist - [ ] Follows naming conventions @@ -242,3 +282,7 @@ See [security-examples.md](coding-standards/security-examples.md) for examples. - [ ] No analyzer warnings - [ ] No hardcoded secrets - [ ] Input validation at boundaries +- [ ] Dependencies injected via constructor (no hidden statics) +- [ ] Interfaces defined for external dependencies +- [ ] New classes are unit-testable in isolation +- [ ] No static classes for business logic diff --git a/docs/standards/coding-standards/dependency-injection.md b/docs/standards/coding-standards/dependency-injection.md new file mode 100644 index 0000000..e3899d5 --- /dev/null +++ b/docs/standards/coding-standards/dependency-injection.md @@ -0,0 +1,267 @@ +# Dependency Injection Patterns + +Patterns and examples for dependency injection in the project. + +## Constructor Injection + +Prefer constructor injection for all dependencies: + +```csharp +internal sealed class ClaudeMonitor : IClaudeMonitor +{ + private readonly IConsoleService _console; + private readonly IEnvironmentService _environment; + private readonly ILogger _logger; + + public ClaudeMonitor( + IConsoleService console, + IEnvironmentService environment, + ILogger? logger = null) + { + _console = console; + _environment = environment; + _logger = logger ?? Log.Logger; + } +} +``` + +**Benefits:** + +- Dependencies are explicit and visible +- Immutable after construction +- Easy to mock in tests +- Compiler enforces required dependencies + +## Interface Design + +### Design Principles + +- One interface per responsibility +- Prefer small, focused interfaces (ISP) +- Name interfaces after what they do, not how + +### Example: Console Abstraction + +```csharp +internal interface IConsoleService +{ + void Write(string value); + void WriteLine(string message); + void WriteErrorLine(string message); + ConsoleColor ForegroundColor { get; set; } + void ResetColor(); + bool IsInputRedirected { get; } + int WindowWidth { get; } + int WindowHeight { get; } +} + +internal sealed class ConsoleService : IConsoleService +{ + public void Write(string value) => Console.Write(value); + public void WriteLine(string message) => Console.WriteLine(message); + // ... rest of implementation +} +``` + +## Service Registration + +Register services in Program.cs or dedicated extension methods: + +```csharp +var host = Host.CreateDefaultBuilder(args) + .ConfigureServices(services => + { + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }) + .Build(); +``` + +### Service Lifetimes + +| Lifetime | Registration | When to Use | +| --------- | ---------------------- | --------------------------------- | +| Singleton | `AddSingleton()` | Stateless services, shared state | +| Scoped | `AddScoped()` | Per-request state (web scenarios) | +| Transient | `AddTransient()` | Lightweight, stateful, disposable | + +### Choosing Lifetime + +- **Singleton**: `IConsoleService`, `IEnvironmentService` - wrap static APIs +- **Transient**: `ClaudeMonitor` - stateful, disposable per use +- **Scoped**: Database contexts, HTTP clients per request + +## Testing with Mocked Dependencies + +```csharp +public sealed class ClaudeMonitorTests : IDisposable +{ + private readonly Mock _mockConsole; + private readonly Mock _mockEnvironment; + private readonly ClaudeMonitor _monitor; + + public ClaudeMonitorTests() + { + _mockConsole = new Mock(); + _mockEnvironment = new Mock(); + + // Setup default mock behavior + _mockConsole.Setup(c => c.WindowWidth).Returns(120); + _mockEnvironment.Setup(e => e.CurrentDirectory).Returns("/test"); + + _monitor = new ClaudeMonitor( + WrapperConfig.Default, + _mockConsole.Object, + _mockEnvironment.Object); + } + + [Fact] + public void Method_Scenario_ExpectedBehaviour() + { + // Arrange - setup specific mock behavior + _mockEnvironment.Setup(e => e.GetEnvironmentVariable("PATH")) + .Returns("/usr/bin"); + + // Act + var result = _monitor.SomeMethod(); + + // Assert + result.Should().BeExpectedValue(); + _mockConsole.Verify(c => c.WriteLine(It.IsAny()), Times.Once); + } +} +``` + +## Anti-Patterns to Avoid + +### Static Dependencies + +```csharp +// BAD: Hidden static dependency +public class Service +{ + public void DoWork() + { + var path = Environment.GetEnvironmentVariable("PATH"); // Static! + Console.WriteLine("Working..."); // Static! + } +} + +// GOOD: Injected dependencies +public class Service +{ + private readonly IEnvironmentService _environment; + private readonly IConsoleService _console; + + public Service(IEnvironmentService environment, IConsoleService console) + { + _environment = environment; + _console = console; + } + + public void DoWork() + { + var path = _environment.GetEnvironmentVariable("PATH"); + _console.WriteLine("Working..."); + } +} +``` + +### Service Locator + +```csharp +// BAD: Service locator pattern +public class Service +{ + public void DoWork(IServiceProvider provider) + { + var console = provider.GetRequiredService(); + console.WriteLine("Working..."); + } +} + +// GOOD: Constructor injection +public class Service +{ + private readonly IConsoleService _console; + + public Service(IConsoleService console) + { + _console = console; + } + + public void DoWork() + { + _console.WriteLine("Working..."); + } +} +``` + +### Concrete Dependencies + +```csharp +// BAD: Depends on concrete class +public class Application +{ + private readonly ClaudeMonitor _monitor; // Concrete! + + public Application() + { + _monitor = new ClaudeMonitor(config); // Can't mock! + } +} + +// GOOD: Depends on interface +public class Application +{ + private readonly IClaudeMonitor _monitor; + + public Application(IClaudeMonitor monitor) + { + _monitor = monitor; + } +} +``` + +## When Static is Acceptable + +### Extension Methods + +```csharp +public static class StringExtensions +{ + public static string Truncate(this string value, int maxLength) + { + return value.Length <= maxLength ? value : value[..maxLength]; + } +} +``` + +### Pure Functions + +```csharp +public static class MathUtilities +{ + public static int Clamp(int value, int min, int max) + { + return Math.Max(min, Math.Min(max, value)); + } +} +``` + +### Bootstrap Code + +```csharp +// Before DI container is available +Log.Logger = new LoggerConfiguration() + .MinimumLevel.Warning() + .WriteTo.File(LoggingConfiguration.GetLogFilePath()) + .CreateBootstrapLogger(); +``` + +## Related + +- [Microsoft DI Documentation](https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection) +- [ADR-0022: Dependency Injection Foundation](../adr/0022-dependency-injection-foundation.md)