Skip to content

Commit 2370012

Browse files
jbrinkmanjbrinkman
committed
Refactor CompareCommand to use proper dependency injection
- Change CompareCommand constructor from 2 to 4 parameters - Add IExitCodeManager and IGlobalExceptionHandler parameters - Eliminate service locator anti-pattern by using constructor injection - Update TypeRegistrar to provide new constructor parameters - Make Program.ConfigureServices public for testing - Add comprehensive DI validation tests (DependencyInjectionTests.cs) - Fix all unit tests to use new constructor parameters - All 363 tests now pass This improves performance, testability, and maintainability by using proper DI patterns.
1 parent c2de3bb commit 2370012

File tree

7 files changed

+291
-38
lines changed

7 files changed

+291
-38
lines changed

‎src/DotNetApiDiff/Commands/CompareCommand.cs‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,26 @@ public class CompareCommand : Command<CompareCommandSettings>
7979
{
8080
private readonly IServiceProvider _serviceProvider;
8181
private readonly ILogger<CompareCommand> _logger;
82+
private readonly IExitCodeManager _exitCodeManager;
83+
private readonly IGlobalExceptionHandler _exceptionHandler;
8284

8385
/// <summary>
8486
/// Initializes a new instance of the <see cref="CompareCommand"/> class.
8587
/// </summary>
8688
/// <param name="serviceProvider">The service provider.</param>
8789
/// <param name="logger">The logger.</param>
88-
public CompareCommand(IServiceProvider serviceProvider, ILogger<CompareCommand> logger)
90+
/// <param name="exitCodeManager">The exit code manager.</param>
91+
/// <param name="exceptionHandler">The global exception handler.</param>
92+
public CompareCommand(
93+
IServiceProvider serviceProvider,
94+
ILogger<CompareCommand> logger,
95+
IExitCodeManager exitCodeManager,
96+
IGlobalExceptionHandler exceptionHandler)
8997
{
9098
_serviceProvider = serviceProvider;
9199
_logger = logger;
100+
_exitCodeManager = exitCodeManager;
101+
_exceptionHandler = exceptionHandler;
92102
}
93103

94104
/// <summary>
@@ -171,8 +181,6 @@ public override ValidationResult Validate([NotNull] CommandContext context, [Not
171181
/// <returns>Exit code (0 for success, non-zero for failure)</returns>
172182
public override int Execute([NotNull] CommandContext context, [NotNull] CompareCommandSettings settings)
173183
{
174-
var exceptionHandler = _serviceProvider.GetRequiredService<IGlobalExceptionHandler>();
175-
176184
try
177185
{
178186
// Create a logging scope for this command execution
@@ -217,8 +225,7 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
217225
AnsiConsole.MarkupLine($"[red]Error loading configuration:[/] {ex.Message}");
218226

219227
// Use the ExitCodeManager to determine the appropriate exit code for errors
220-
var configErrorExitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
221-
return configErrorExitCodeManager.GetExitCodeForException(ex);
228+
return _exitCodeManager.GetExitCodeForException(ex);
222229
}
223230
}
224231
}
@@ -251,8 +258,7 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
251258
_logger.LogError(ex, "Failed to load source assembly: {Path}", settings.SourceAssemblyPath);
252259
AnsiConsole.MarkupLine($"[red]Error loading source assembly:[/] {ex.Message}");
253260

254-
var sourceAssemblyExitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
255-
return sourceAssemblyExitCodeManager.GetExitCodeForException(ex);
261+
return _exitCodeManager.GetExitCodeForException(ex);
256262
}
257263

258264
try
@@ -264,8 +270,7 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
264270
_logger.LogError(ex, "Failed to load target assembly: {Path}", settings.TargetAssemblyPath);
265271
AnsiConsole.MarkupLine($"[red]Error loading target assembly:[/] {ex.Message}");
266272

267-
var targetAssemblyExitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
268-
return targetAssemblyExitCodeManager.GetExitCodeForException(ex);
273+
return _exitCodeManager.GetExitCodeForException(ex);
269274
}
270275
}
271276

@@ -316,8 +321,7 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
316321
_logger.LogError(ex, "Error comparing assemblies");
317322
AnsiConsole.MarkupLine($"[red]Error comparing assemblies:[/] {ex.Message}");
318323

319-
var comparisonExitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
320-
return comparisonExitCodeManager.GetExitCodeForException(ex);
324+
return _exitCodeManager.GetExitCodeForException(ex);
321325
}
322326
}
323327

@@ -364,14 +368,12 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
364368
_logger.LogError(ex, "Error generating {Format} report", format);
365369
AnsiConsole.MarkupLine($"[red]Error generating report:[/] {ex.Message}");
366370

367-
var reportExitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
368-
return reportExitCodeManager.GetExitCodeForException(ex);
371+
return _exitCodeManager.GetExitCodeForException(ex);
369372
}
370373
}
371374

372375
// Use the ExitCodeManager to determine the appropriate exit code
373-
var exitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
374-
int exitCode = exitCodeManager.GetExitCode(comparison);
376+
int exitCode = _exitCodeManager.GetExitCode(comparison);
375377

376378
if (comparison.HasBreakingChanges)
377379
{
@@ -385,15 +387,15 @@ public override int Execute([NotNull] CommandContext context, [NotNull] CompareC
385387
_logger.LogInformation(
386388
"Exiting with code {ExitCode}: {Description}",
387389
exitCode,
388-
exitCodeManager.GetExitCodeDescription(exitCode));
390+
_exitCodeManager.GetExitCodeDescription(exitCode));
389391

390392
return exitCode;
391393
}
392394
}
393395
catch (Exception ex)
394396
{
395397
// Use our centralized exception handler for any unhandled exceptions
396-
return exceptionHandler.HandleException(ex, "Compare command execution");
398+
return _exceptionHandler.HandleException(ex, "Compare command execution");
397399
}
398400
}
399401

‎src/DotNetApiDiff/Commands/TypeRegistrar.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ public TypeRegistrar(IServiceProvider serviceProvider)
5959
_services.AddTransient<CompareCommand>(provider =>
6060
{
6161
var logger = provider.GetRequiredService<ILogger<CompareCommand>>();
62-
return new CompareCommand(serviceProvider, logger);
62+
var exitCodeManager = serviceProvider.GetRequiredService<IExitCodeManager>();
63+
var exceptionHandler = serviceProvider.GetRequiredService<IGlobalExceptionHandler>();
64+
return new CompareCommand(serviceProvider, logger, exitCodeManager, exceptionHandler);
6365
});
6466
}
6567

‎src/DotNetApiDiff/Program.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private static string GetBuildTime()
117117
/// Configures dependency injection services
118118
/// </summary>
119119
/// <param name="services">Service collection to configure</param>
120-
private static void ConfigureServices(IServiceCollection services)
120+
public static void ConfigureServices(IServiceCollection services)
121121
{
122122
// Configure logging with structured logging support
123123
services.AddLogging(builder =>
@@ -153,8 +153,8 @@ private static void ConfigureServices(IServiceCollection services)
153153
// Register core services
154154
services.AddScoped<IAssemblyLoader, AssemblyLoading.AssemblyLoader>();
155155
services.AddScoped<IApiExtractor, ApiExtraction.ApiExtractor>();
156-
services.AddScoped<ITypeAnalyzer, ApiExtraction.TypeAnalyzer>();
157156
services.AddScoped<IMemberSignatureBuilder, ApiExtraction.MemberSignatureBuilder>();
157+
services.AddScoped<ITypeAnalyzer, ApiExtraction.TypeAnalyzer>();
158158
services.AddScoped<IDifferenceCalculator, ApiExtraction.DifferenceCalculator>();
159159

160160
// Register the NameMapper

‎tests/DotNetApiDiff.Tests/Commands/CompareCommandErrorTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public CompareCommandErrorTests()
4646
services.AddSingleton(_loggerMock.Object);
4747

4848
_serviceProvider = services.BuildServiceProvider();
49-
_command = new CompareCommand(_serviceProvider, _loggerMock.Object);
49+
_command = new CompareCommand(_serviceProvider, _loggerMock.Object, _exitCodeManagerMock.Object, _exceptionHandlerMock.Object);
5050

5151
// Create temp directory for test files
5252
_tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());

‎tests/DotNetApiDiff.Tests/Commands/CompareCommandFilteringTests.cs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,22 @@ public CompareCommandFilteringTests()
8686
_serviceProvider = services.BuildServiceProvider();
8787
}
8888

89+
/// <summary>
90+
/// Creates a CompareCommand instance with properly resolved dependencies for testing
91+
/// </summary>
92+
/// <returns>CompareCommand instance ready for testing</returns>
93+
private CompareCommand CreateCompareCommand()
94+
{
95+
var exitCodeManager = _serviceProvider.GetRequiredService<IExitCodeManager>();
96+
var exceptionHandler = _serviceProvider.GetRequiredService<IGlobalExceptionHandler>();
97+
return new CompareCommand(_serviceProvider, _mockLogger.Object, exitCodeManager, exceptionHandler);
98+
}
99+
89100
[Fact]
90101
public void Execute_WithNamespaceFilters_AppliesFiltersToConfiguration()
91102
{
92103
// Arrange
93-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
104+
var command = CreateCompareCommand();
94105
var settings = new CompareCommandSettings
95106
{
96107
SourceAssemblyPath = "source.dll",
@@ -116,7 +127,7 @@ public void Execute_WithNamespaceFilters_AppliesFiltersToConfiguration()
116127
public void Execute_WithTypePatterns_AppliesFiltersToConfiguration()
117128
{
118129
// Arrange
119-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
130+
var command = CreateCompareCommand();
120131
var settings = new CompareCommandSettings
121132
{
122133
SourceAssemblyPath = "source.dll",
@@ -142,7 +153,7 @@ public void Execute_WithTypePatterns_AppliesFiltersToConfiguration()
142153
public void Execute_WithExcludePatterns_AppliesExclusionsToConfiguration()
143154
{
144155
// Arrange
145-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
156+
var command = CreateCompareCommand();
146157
var settings = new CompareCommandSettings
147158
{
148159
SourceAssemblyPath = "source.dll",
@@ -173,7 +184,7 @@ public void Execute_WithExcludePatterns_AppliesExclusionsToConfiguration()
173184
public void Execute_WithIncludeInternals_AppliesOptionToConfiguration()
174185
{
175186
// Arrange
176-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
187+
var command = CreateCompareCommand();
177188
var settings = new CompareCommandSettings
178189
{
179190
SourceAssemblyPath = "source.dll",
@@ -197,7 +208,7 @@ public void Execute_WithIncludeInternals_AppliesOptionToConfiguration()
197208
public void Execute_WithIncludeCompilerGenerated_AppliesOptionToConfiguration()
198209
{
199210
// Arrange
200-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
211+
var command = CreateCompareCommand();
201212
var settings = new CompareCommandSettings
202213
{
203214
SourceAssemblyPath = "source.dll",
@@ -229,7 +240,7 @@ public void Execute_WithConfigFile_LoadsConfigurationFromFile()
229240
config.Filters.IncludeNamespaces.Add("TestNamespace");
230241
config.SaveToJsonFile(tempConfigFile);
231242

232-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
243+
var command = CreateCompareCommand();
233244
var settings = new CompareCommandSettings
234245
{
235246
SourceAssemblyPath = "source.dll",
@@ -268,7 +279,7 @@ public void Execute_WithInvalidConfigFile_ReturnsErrorCode()
268279
// Create an invalid JSON file
269280
File.WriteAllText(tempConfigFile, "{ invalid json }");
270281

271-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
282+
var command = CreateCompareCommand();
272283
var settings = new CompareCommandSettings
273284
{
274285
SourceAssemblyPath = "source.dll",
@@ -291,7 +302,9 @@ public void Execute_WithInvalidConfigFile_ReturnsErrorCode()
291302
services.AddSingleton(Mock.Of<IGlobalExceptionHandler>());
292303

293304
var serviceProvider = services.BuildServiceProvider();
294-
command = new CompareCommand(serviceProvider, _mockLogger.Object);
305+
var exitCodeManager = serviceProvider.GetRequiredService<IExitCodeManager>();
306+
var exceptionHandler = serviceProvider.GetRequiredService<IGlobalExceptionHandler>();
307+
command = new CompareCommand(serviceProvider, _mockLogger.Object, exitCodeManager, exceptionHandler);
295308

296309
// Act
297310
var result = command.Execute(_commandContext, settings);
@@ -313,7 +326,7 @@ public void Execute_WithInvalidConfigFile_ReturnsErrorCode()
313326
public void Execute_WithCombinedFilters_AppliesAllFiltersToConfiguration()
314327
{
315328
// Arrange
316-
var command = new CompareCommand(_serviceProvider, _mockLogger.Object);
329+
var command = CreateCompareCommand();
317330
var settings = new CompareCommandSettings
318331
{
319332
SourceAssemblyPath = "source.dll",

‎tests/DotNetApiDiff.Tests/Commands/CompareCommandTests.cs‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,24 @@ namespace DotNetApiDiff.Tests.Commands;
1414

1515
public class CompareCommandTests
1616
{
17+
/// <summary>
18+
/// Creates a CompareCommand instance with properly mocked dependencies for testing
19+
/// </summary>
20+
/// <returns>CompareCommand instance ready for testing</returns>
21+
private static CompareCommand CreateCompareCommand()
22+
{
23+
var serviceProvider = Mock.Of<IServiceProvider>();
24+
var logger = Mock.Of<ILogger<CompareCommand>>();
25+
var exitCodeManager = Mock.Of<IExitCodeManager>();
26+
var exceptionHandler = Mock.Of<IGlobalExceptionHandler>();
27+
28+
return new CompareCommand(serviceProvider, logger, exitCodeManager, exceptionHandler);
29+
}
1730
[Fact]
1831
public void Validate_WithValidPaths_ReturnsSuccess()
1932
{
2033
// Arrange
21-
var command = new CompareCommand(Mock.Of<IServiceProvider>(), Mock.Of<ILogger<CompareCommand>>());
34+
var command = CreateCompareCommand();
2235
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
2336

2437
// Create temporary files for testing
@@ -55,7 +68,7 @@ public void Validate_WithValidPaths_ReturnsSuccess()
5568
public void Validate_WithInvalidSourceAssemblyPath_ReturnsError()
5669
{
5770
// Arrange
58-
var command = new CompareCommand(Mock.Of<IServiceProvider>(), Mock.Of<ILogger<CompareCommand>>());
71+
var command = CreateCompareCommand();
5972
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
6073

6174
// Create temporary file for target assembly
@@ -89,7 +102,7 @@ public void Validate_WithInvalidSourceAssemblyPath_ReturnsError()
89102
public void Validate_WithInvalidTargetAssemblyPath_ReturnsError()
90103
{
91104
// Arrange
92-
var command = new CompareCommand(Mock.Of<IServiceProvider>(), Mock.Of<ILogger<CompareCommand>>());
105+
var command = CreateCompareCommand();
93106
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
94107

95108
// Create temporary file for source assembly
@@ -123,7 +136,7 @@ public void Validate_WithInvalidTargetAssemblyPath_ReturnsError()
123136
public void Validate_WithInvalidConfigFile_ReturnsError()
124137
{
125138
// Arrange
126-
var command = new CompareCommand(Mock.Of<IServiceProvider>(), Mock.Of<ILogger<CompareCommand>>());
139+
var command = CreateCompareCommand();
127140
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
128141

129142
// Create temporary files for assemblies
@@ -162,7 +175,7 @@ public void Validate_WithInvalidConfigFile_ReturnsError()
162175
public void Validate_WithInvalidOutputFormat_ReturnsError()
163176
{
164177
// Arrange
165-
var command = new CompareCommand(Mock.Of<IServiceProvider>(), Mock.Of<ILogger<CompareCommand>>());
178+
var command = CreateCompareCommand();
166179
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
167180

168181
// Create temporary files for assemblies
@@ -252,7 +265,7 @@ public void Execute_WithValidSettings_ReturnsSuccessExitCode()
252265
mockReportGenerator.Setup(rg => rg.GenerateReport(It.IsAny<ComparisonResult>(), It.IsAny<ReportFormat>()))
253266
.Returns("Test Report");
254267

255-
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object);
268+
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object, mockExitCodeManager.Object, mockExceptionHandler.Object);
256269
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
257270

258271
// Create temporary files for testing
@@ -359,7 +372,7 @@ public void Execute_WithBreakingChanges_ReturnsNonZeroExitCode()
359372
mockReportGenerator.Setup(rg => rg.GenerateReport(It.IsAny<ComparisonResult>(), It.IsAny<ReportFormat>()))
360373
.Returns("Test Report");
361374

362-
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object);
375+
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object, mockExitCodeManager.Object, mockExceptionHandler.Object);
363376
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
364377

365378
// Create temporary files for testing
@@ -422,7 +435,7 @@ public void Execute_WithException_ReturnsErrorExitCode()
422435
mockAssemblyLoader.Setup(al => al.LoadAssembly(It.IsAny<string>()))
423436
.Throws(new InvalidOperationException("Test exception"));
424437

425-
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object);
438+
var command = new CompareCommand(mockServiceProvider.Object, mockLogger.Object, mockExitCodeManager.Object, mockExceptionHandler.Object);
426439
var context = new CommandContext(Mock.Of<IRemainingArguments>(), "compare", null);
427440

428441
// Create temporary files for testing

0 commit comments

Comments
 (0)