Activate and fix CA2000: Dispose objects before losing scope

Author: bkoelman

CodingGuidelines.ruleset

@@ -39,5 +39,6 @@
     <Rule Id="CA1065" Action="Warning" />
     <Rule Id="CA1066" Action="Warning" />
     <Rule Id="CA1849" Action="Warning" />
+    <Rule Id="CA2000" Action="Warning" />
   </Rules>
 </RuleSet>

benchmarks/Deserialization/DeserializationBenchmarkBase.cs

@@ -11,8 +11,10 @@
 
 namespace Benchmarks.Deserialization;
 
-public abstract class DeserializationBenchmarkBase
+public abstract class DeserializationBenchmarkBase : IDisposable
 {
+    private readonly ServiceContainer _serviceProvider = new();
+
     protected JsonSerializerOptions SerializerReadOptions { get; }
     protected DocumentAdapter DocumentAdapter { get; }
 
@@ -23,12 +25,11 @@ protected DeserializationBenchmarkBase()
         options.SerializerOptions.Converters.Add(new ResourceObjectConverter(resourceGraph));
         SerializerReadOptions = ((IJsonApiOptions)options).SerializerReadOptions;
 
-        var serviceContainer = new ServiceContainer();
-        var resourceFactory = new ResourceFactory(serviceContainer);
-        var resourceDefinitionAccessor = new ResourceDefinitionAccessor(resourceGraph, serviceContainer);
+        var resourceFactory = new ResourceFactory(_serviceProvider);
+        var resourceDefinitionAccessor = new ResourceDefinitionAccessor(resourceGraph, _serviceProvider);
 
-        serviceContainer.AddService(typeof(IResourceDefinitionAccessor), resourceDefinitionAccessor);
-        serviceContainer.AddService(typeof(IResourceDefinition<IncomingResource, int>), new JsonApiResourceDefinition<IncomingResource, int>(resourceGraph));
+        _serviceProvider.AddService(typeof(IResourceDefinitionAccessor), resourceDefinitionAccessor);
+        _serviceProvider.AddService(typeof(IResourceDefinition<IncomingResource, int>), new JsonApiResourceDefinition<IncomingResource, int>(resourceGraph));
 
         // ReSharper disable once VirtualMemberCallInConstructor
         JsonApiRequest request = CreateJsonApiRequest(resourceGraph);
@@ -53,6 +54,22 @@ protected DeserializationBenchmarkBase()
 
     protected abstract JsonApiRequest CreateJsonApiRequest(IResourceGraph resourceGraph);
 
+    public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
+
+#pragma warning disable CA1063 // Implement IDisposable Correctly
+    private void Dispose(bool disposing)
+#pragma warning restore CA1063 // Implement IDisposable Correctly
+    {
+        if (disposing)
+        {
+            _serviceProvider.Dispose();
+        }
+    }
+
     [UsedImplicitly(ImplicitUseTargetFlags.Members)]
     public sealed class IncomingResource : Identifiable<int>
     {

benchmarks/QueryString/QueryStringParserBenchmarks.cs

@@ -14,8 +14,9 @@ namespace Benchmarks.QueryString;
 [MarkdownExporter]
 [SimpleJob(3, 10, 20)]
 [MemoryDiagnoser]
-public class QueryStringParserBenchmarks
+public class QueryStringParserBenchmarks : IDisposable
 {
+    private readonly ServiceContainer _serviceProvider = new();
     private readonly FakeRequestQueryStringAccessor _queryStringAccessor = new();
     private readonly QueryStringReader _queryStringReader;
 
@@ -34,7 +35,7 @@ public QueryStringParserBenchmarks()
             IsCollection = true
         };
 
-        var resourceFactory = new ResourceFactory(new ServiceContainer());
+        var resourceFactory = new ResourceFactory(_serviceProvider);
 
         var includeParser = new IncludeParser(options);
         var includeReader = new IncludeQueryStringParameterReader(includeParser, request, resourceGraph);
@@ -92,4 +93,20 @@ public void ComplexQuery()
         _queryStringAccessor.SetQueryString(queryString);
         _queryStringReader.ReadAll(null);
     }
+
+    public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
+
+#pragma warning disable CA1063 // Implement IDisposable Correctly
+    private void Dispose(bool disposing)
+#pragma warning restore CA1063 // Implement IDisposable Correctly
+    {
+        if (disposing)
+        {
+            _serviceProvider.Dispose();
+        }
+    }
 }

src/Examples/JsonApiDotNetCoreExample/Controllers/NonJsonApiController.cs

@@ -16,7 +16,8 @@ public IActionResult Get()
     [HttpPost]
     public async Task<IActionResult> PostAsync()
     {
-        string name = await new StreamReader(Request.Body).ReadToEndAsync();
+        using var reader = new StreamReader(Request.Body, leaveOpen: true);
+        string name = await reader.ReadToEndAsync();
 
         if (string.IsNullOrEmpty(name))
         {

src/JsonApiDotNetCore/Diagnostics/CascadingCodeTimer.cs

@@ -3,6 +3,8 @@
 using System.Runtime.InteropServices;
 using System.Text;
 
+#pragma warning disable CA2000 // Dispose objects before losing scope
+
 namespace JsonApiDotNetCore.Diagnostics;
 
 /// <summary>

src/JsonApiDotNetCore/Middleware/JsonApiMiddleware.cs

@@ -53,7 +53,9 @@ public JsonApiMiddleware(RequestDelegate? next, IHttpContextAccessor httpContext
         _options = options;
         _logger = logger;
 
+#pragma warning disable CA2000 // Dispose objects before losing scope
         var session = new AspNetCodeTimerSession(httpContextAccessor);
+#pragma warning restore CA2000 // Dispose objects before losing scope
         CodeTimingSessionManager.Capture(session);
     }
 

test/DapperTests/IntegrationTests/DapperTestContext.cs

@@ -52,7 +52,10 @@ public DapperTestContext()
 
     private WebApplicationFactory<TodoItem> CreateFactory()
     {
+#pragma warning disable CA2000 // Dispose objects before losing scope
+        // Justification: The child factory returned by WithWebHostBuilder() is owned by the parent factory, which disposes it.
         return new WebApplicationFactory<TodoItem>().WithWebHostBuilder(builder =>
+#pragma warning restore CA2000 // Dispose objects before losing scope
         {
             builder.UseSetting("ConnectionStrings:DapperExamplePostgreSql",
                 $"Host=localhost;Database=DapperExample-{Guid.NewGuid():N};User ID=postgres;Password=postgres;Include Error Detail=true");

test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Mixed/AtomicLoggingTests.cs

@@ -19,28 +19,24 @@ public AtomicLoggingTests(IntegrationTestContext<TestableStartup<OperationsDbCon
 
         testContext.UseController<OperationsController>();
 
-        var loggerFactory = new FakeLoggerFactory(LogLevel.Information);
-
         testContext.ConfigureLogging(options =>
         {
-            options.ClearProviders();
-            options.AddProvider(loggerFactory);
+            var loggerProvider = new CapturingLoggerProvider(LogLevel.Information);
+            options.AddProvider(loggerProvider);
             options.SetMinimumLevel(LogLevel.Information);
-        });
 
-        testContext.ConfigureServices(services =>
-        {
-            services.AddSingleton(loggerFactory);
-            services.AddSingleton<IOperationsTransactionFactory, ThrowingOperationsTransactionFactory>();
+            options.Services.AddSingleton(loggerProvider);
         });
+
+        testContext.ConfigureServices(services => services.AddSingleton<IOperationsTransactionFactory, ThrowingOperationsTransactionFactory>());
     }
 
     [Fact]
     public async Task Logs_unhandled_exception_at_Error_level()
     {
         // Arrange
-        var loggerFactory = _testContext.Factory.Services.GetRequiredService<FakeLoggerFactory>();
-        loggerFactory.Logger.Clear();
+        var loggerProvider = _testContext.Factory.Services.GetRequiredService<CapturingLoggerProvider>();
+        loggerProvider.Clear();
 
         var transactionFactory = (ThrowingOperationsTransactionFactory)_testContext.Factory.Services.GetRequiredService<IOperationsTransactionFactory>();
         transactionFactory.ThrowOnOperationStart = true;
@@ -80,19 +76,18 @@ public async Task Logs_unhandled_exception_at_Error_level()
         error.Source.ShouldNotBeNull();
         error.Source.Pointer.Should().Be("/atomic:operations[0]");
 
-        IReadOnlyList<FakeLogMessage> logMessages = loggerFactory.Logger.GetMessages();
-        logMessages.ShouldNotBeEmpty();
+        IReadOnlyList<LogMessage> logMessages = loggerProvider.GetMessages();
 
-        logMessages.Should().ContainSingle(message => message.LogLevel == LogLevel.Error &&
-            message.Text.Contains("Simulated failure.", StringComparison.Ordinal));
+        logMessages.Should().ContainSingle(message =>
+            message.LogLevel == LogLevel.Error && message.Text.Contains("Simulated failure.", StringComparison.Ordinal));
     }
 
     [Fact]
     public async Task Logs_invalid_request_body_error_at_Information_level()
     {
         // Arrange
-        var loggerFactory = _testContext.Factory.Services.GetRequiredService<FakeLoggerFactory>();
-        loggerFactory.Logger.Clear();
+        var loggerProvider = _testContext.Factory.Services.GetRequiredService<CapturingLoggerProvider>();
+        loggerProvider.Clear();
 
         var transactionFactory = (ThrowingOperationsTransactionFactory)_testContext.Factory.Services.GetRequiredService<IOperationsTransactionFactory>();
         transactionFactory.ThrowOnOperationStart = false;
@@ -118,8 +113,7 @@ public async Task Logs_invalid_request_body_error_at_Information_level()
 
         responseDocument.Errors.ShouldHaveCount(1);
 
-        IReadOnlyList<FakeLogMessage> logMessages = loggerFactory.Logger.GetMessages();
-        logMessages.ShouldNotBeEmpty();
+        IReadOnlyList<LogMessage> logMessages = loggerProvider.GetMessages();
 
         logMessages.Should().ContainSingle(message => message.LogLevel == LogLevel.Information &&
             message.Text.Contains("Failed to deserialize request body", StringComparison.Ordinal));

test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Mixed/AtomicTraceLoggingTests.cs

@@ -19,25 +19,24 @@ public AtomicTraceLoggingTests(IntegrationTestContext<TestableStartup<Operations
 
         testContext.UseController<OperationsController>();
 
-        var loggerFactory = new FakeLoggerFactory(LogLevel.Trace);
-
         testContext.ConfigureLogging(options =>
         {
-            options.ClearProviders();
-            options.AddProvider(loggerFactory);
+            var loggerProvider = new CapturingLoggerProvider((category, level) =>
+                level >= LogLevel.Trace && category.StartsWith("JsonApiDotNetCore.", StringComparison.Ordinal));
+
+            options.AddProvider(loggerProvider);
             options.SetMinimumLevel(LogLevel.Trace);
-            options.AddFilter((category, _) => category != null && category.StartsWith("JsonApiDotNetCore.", StringComparison.Ordinal));
-        });
 
-        testContext.ConfigureServices(services => services.AddSingleton(loggerFactory));
+            options.Services.AddSingleton(loggerProvider);
+        });
     }
 
     [Fact]
     public async Task Logs_execution_flow_at_Trace_level_on_operations_request()
     {
         // Arrange
-        var loggerFactory = _testContext.Factory.Services.GetRequiredService<FakeLoggerFactory>();
-        loggerFactory.Logger.Clear();
+        var loggerProvider = _testContext.Factory.Services.GetRequiredService<CapturingLoggerProvider>();
+        loggerProvider.Clear();
 
         MusicTrack existingTrack = _fakers.MusicTrack.GenerateOne();
         existingTrack.Lyric = _fakers.Lyric.GenerateOne();
@@ -116,7 +115,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
 
         responseDocument.Should().BeEmpty();
 
-        IReadOnlyList<string> logLines = loggerFactory.Logger.GetLines();
+        IReadOnlyList<string> logLines = loggerProvider.GetLines();
 
         logLines.Should().BeEquivalentTo(new[]
         {

test/JsonApiDotNetCoreTests/IntegrationTests/CustomRoutes/ApiControllerAttributeLogTests.cs

@@ -8,34 +8,33 @@ namespace JsonApiDotNetCoreTests.IntegrationTests.CustomRoutes;
 
 public sealed class ApiControllerAttributeLogTests : IntegrationTestContext<TestableStartup<CustomRouteDbContext>, CustomRouteDbContext>, IAsyncDisposable
 {
-    private readonly FakeLoggerFactory _loggerFactory;
+    private readonly CapturingLoggerProvider _loggerProvider;
 
     public ApiControllerAttributeLogTests()
     {
         UseController<CiviliansController>();
 
-        _loggerFactory = new FakeLoggerFactory(LogLevel.Warning);
+        _loggerProvider = new CapturingLoggerProvider(LogLevel.Warning);
 
         ConfigureLogging(options =>
         {
-            options.ClearProviders();
-            options.AddProvider(_loggerFactory);
-        });
+            options.AddProvider(_loggerProvider);
 
-        ConfigureServices(services => services.AddSingleton(_loggerFactory));
+            options.Services.AddSingleton(_loggerProvider);
+        });
     }
 
     [Fact]
     public void Logs_warning_at_startup_when_ApiControllerAttribute_found()
     {
         // Arrange
-        _loggerFactory.Logger.Clear();
+        _loggerProvider.Clear();
 
         // Act
         _ = Factory;
 
         // Assert
-        IReadOnlyList<string> logLines = _loggerFactory.Logger.GetLines();
+        IReadOnlyList<string> logLines = _loggerProvider.GetLines();
         logLines.ShouldHaveCount(1);
 
         logLines[0].Should().Be(
@@ -44,7 +43,7 @@ public void Logs_warning_at_startup_when_ApiControllerAttribute_found()
 
     public override Task DisposeAsync()
     {
-        _loggerFactory.Dispose();
+        _loggerProvider.Dispose();
         return base.DisposeAsync();
     }