diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs index d54cd73..fdbccdc 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs @@ -123,6 +123,45 @@ public void Dispose() } } + public class TransientAsyncDisposable : IAsyncDisposable + { + public ValueTask DisposeAsync() + { + GC.SuppressFinalize(this); + return default; + } + } + + public class AsyncDisposableConsumer : IAsyncDisposable + { + public TransientAsyncDisposable TransientAsyncDisposable { get; } + + public AsyncDisposableConsumer(TransientAsyncDisposable transientAsyncDisposable) + { + TransientAsyncDisposable = transientAsyncDisposable; + } + + public ValueTask DisposeAsync() + { + GC.SuppressFinalize(this); + return default; + } + } + + public class AsyncDisposableConsumerFactory + { +#pragma warning disable IDE0079 // Remove unnecessary suppression +#pragma warning disable CA1822 // Mark members as static +#pragma warning disable S2325 // Methods and properties that don't access instance data should be static + public AsyncDisposableConsumer Build(IServiceProvider sp) +#pragma warning restore S2325 // Methods and properties that don't access instance data should be static +#pragma warning restore CA1822 // Mark members as static +#pragma warning restore IDE0079 // Remove unnecessary suppression + { + return new AsyncDisposableConsumer(sp.GetRequiredService()); + } + } + private static ServiceCollection CreateServiceCollection() { var serviceCollection = new ServiceCollection(); @@ -605,7 +644,118 @@ public void Register_OpenGeneric_TransientDisposable_WithoutValidation_LogsError Assert.AreEqual(LogLevel.Warning, fakeLogger.LatestRecord.Level); Assert.AreEqual("Open generic transient disposable registration detected, ServiceKey: (null), ServiceType: Mammoth.Extensions.DependencyInjection.Tests.DetectIncorrectUsageOfTransientDisposablesTests+ITransientOpenGeneric`1[T], ImplementationType: Mammoth.Extensions.DependencyInjection.Tests.DetectIncorrectUsageOfTransientDisposablesTests+TransientOpenGeneric`1[T]", fakeLogger.LatestRecord.Message); } - } -} - + [TestMethod] + public void Resolve_TransientAsyncDisposable_InRootScope_WithValidation_Throws() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + Assert.ThrowsExactly(() => sp.GetService()); + } + + [TestMethod] + public async Task Resolve_TransientAsyncDisposable_InScope_WithValidation_NoMemoryLeak() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + await using (sp) + { + await using var scope = sp.CreateAsyncScope(); + var consumer = scope.ServiceProvider.GetService(); + Assert.IsNotNull(consumer); + Assert.IsFalse(scope.ServiceProvider.GetIsRootScope()); + Assert.IsTrue(scope.ServiceProvider.GetDisposables().Contains(consumer)); + Assert.IsTrue(sp.GetIsRootScope()); + Assert.IsFalse(sp.GetDisposables().Contains(consumer)); + } + } + + [TestMethod] + public void Resolve_AsyncDisposableConsumer_using_factory_InRootScope_WithValidation_Throws() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + serviceCollection.AddSingleton(); + serviceCollection.AddTransient(sp => sp.GetRequiredService().Build(sp)); + using var spProvider = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + Assert.ThrowsExactly(() => spProvider.GetService()); + } + + [TestMethod] + public async Task Resolve_AsyncDisposableConsumer_using_factory_InScope_WithValidation_NoMemoryLeak() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + serviceCollection.AddSingleton(); + serviceCollection.AddTransient(sp => sp.GetRequiredService().Build(sp)); + var spProvider = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + await using (spProvider) + { + await using var scope = spProvider.CreateAsyncScope(); + var consumer = scope.ServiceProvider.GetService(); + Assert.IsNotNull(consumer); + Assert.IsFalse(scope.ServiceProvider.GetIsRootScope()); + Assert.IsTrue(scope.ServiceProvider.GetDisposables().Contains(consumer)); + Assert.IsTrue(spProvider.GetIsRootScope()); + Assert.IsFalse(spProvider.GetDisposables().Contains(consumer)); + } + } + + [TestMethod] + public void Resolve_KeyedTransientAsyncDisposable_InRootScope_WithValidation_Throws() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddKeyedTransient("key"); + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + Assert.ThrowsExactly(() => sp.GetKeyedService("key")); + } + + [TestMethod] + public void Resolve_KeyedTransientAsyncDisposable_using_factory_InRootScope_WithValidation_Throws() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddKeyedTransient("key", (sp, key) => new TransientAsyncDisposable()); + using var spProvider = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = true, + ValidateScopes = true + }); + Assert.ThrowsExactly(() => spProvider.GetKeyedService("key")); + } + } +} + #pragma warning restore S3881 // "IDisposable" should be implemented correctly \ No newline at end of file diff --git a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs index 358b309..fb6b4c1 100644 --- a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs +++ b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs @@ -158,8 +158,8 @@ public static (IServiceCollection ServiceCollection, List Ope case ServiceLifetime.Transient when (descriptor.IsKeyedService && descriptor.KeyedImplementationType?.IsGenericTypeDefinition == true) || (!descriptor.IsKeyedService && descriptor.ImplementationType?.IsGenericTypeDefinition == true): - if ((descriptor.IsKeyedService && typeof(IDisposable).IsAssignableFrom(descriptor.KeyedImplementationType)) - || (!descriptor.IsKeyedService && typeof(IDisposable).IsAssignableFrom(descriptor.ImplementationType))) + if ((descriptor.IsKeyedService && IsDisposableType(descriptor.KeyedImplementationType)) + || (!descriptor.IsKeyedService && IsDisposableType(descriptor.ImplementationType))) { if (throwOnOpenGenericTransientDisposable) { @@ -175,9 +175,9 @@ public static (IServiceCollection ServiceCollection, List Ope break; case ServiceLifetime.Transient when (descriptor is { IsKeyedService: true, KeyedImplementationType: not null } - && typeof(IDisposable).IsAssignableFrom(descriptor.KeyedImplementationType)) + && IsDisposableType(descriptor.KeyedImplementationType)) || (descriptor is { IsKeyedService: false, ImplementationType: not null } - && typeof(IDisposable).IsAssignableFrom(descriptor.ImplementationType)): + && IsDisposableType(descriptor.ImplementationType)): collection.Add(CreatePatchedDescriptor(descriptor, allowSingletonToResolveTransientDisposables)); break; case ServiceLifetime.Transient @@ -214,10 +214,10 @@ bool allowSingletonToResolveTransientDisposables //check the ResolutionContext to see if the service is being resolved by a singleton //if it is, then it's safe to resolve the transient disposable service if (sp.GetIsRootScope() - && originalResult is IDisposable d + && (originalResult is IDisposable || originalResult is IAsyncDisposable) && !IsResolvedBySingleton(sp, allowSingletonToResolveTransientDisposables)) { - ThrowTransientDisposableException(original.ServiceKey, original.ServiceType, d.GetType(), isFactory: true); + ThrowTransientDisposableException(original.ServiceKey, original.ServiceType, originalResult.GetType(), isFactory: true); } return originalResult; @@ -243,10 +243,10 @@ bool allowSingletonToResolveTransientDisposables //check the ResolutionContext to see if the service is being resolved by a singleton //if it is, then it's safe to resolve the transient disposable service if (sp.GetIsRootScope() - && originalResult is IDisposable d + && (originalResult is IDisposable || originalResult is IAsyncDisposable) && !IsResolvedBySingleton(sp, allowSingletonToResolveTransientDisposables)) { - ThrowTransientDisposableException(original.ServiceKey, original.ServiceType, d.GetType(), isFactory: true); + ThrowTransientDisposableException(original.ServiceKey, original.ServiceType, originalResult.GetType(), isFactory: true); } return originalResult; @@ -390,6 +390,9 @@ private static void ThrowTransientDisposableException(object? serviceKey, Type? } #endif + private static bool IsDisposableType(Type? type) => + type != null && (typeof(IDisposable).IsAssignableFrom(type) || typeof(IAsyncDisposable).IsAssignableFrom(type)); + private static bool IsResolvedBySingleton( IServiceProvider sp, bool allowSingletonToResolveTransientDisposables