diff --git a/Changelog.md b/Changelog.md index 2c116cf..6224976 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,17 @@ ## vNext +## 0.7.1 + +### Bug Fixes + +- `IServiceCollection` lifetime check methods (`IsTransientServiceRegistered`, `IsScopedServiceRegistered`, `IsSingletonServiceRegistered`, and their keyed variants) now return `false` instead of throwing `InvalidOperationException` when the service is not registered. +- Fixed `ResolutionContext` stack corruption in `PatchForResolutionContextTracking` when a factory delegate throws an exception. Push/Pop calls are now wrapped in `try/finally` for all 4 registration paths (non-keyed/keyed × factory/type). +- Fixed bidirectional `IsAssignableFrom` check in `GetServiceDescriptors` method that incorrectly returned unrelated base-type registrations [#19](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/19). + - The method now uses unidirectional matching: `serviceType == serviceDescriptor.ServiceType || serviceType.IsAssignableFrom(serviceDescriptor.ServiceType)`. + - This fixes incorrect lifetime checks in methods like `IsTransientServiceRegistered`, `IsSingletonServiceRegistered`, etc., which depend on `.Last()` to select the correct registration. +- Fixed missing `return` statements in `TryAdd*` overload methods with empty `DependsOn` array, which caused unnecessary reflection calls and potential double-registration attempts [#18](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/18). + ## 0.7.0 - Decorators: removed reflection-based proxy creation (`Reflection.Emit`) and allow class decoration [#11](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/11). diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposablesTests.cs index d54cd73..95d0ac1 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,6 +644,198 @@ 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")); + } + + [TestMethod] + public void PatchForResolutionContextTracking_FactoryThrows_StackIsClean() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(sp => throw new InvalidOperationException("factory error")); + + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = false, + ValidateScopes = true + }); + + Assert.ThrowsExactly(() => sp.GetService()); + Assert.AreEqual(0, ResolutionContext.CurrentStack.Count); + } + + [TestMethod] + public void PatchForResolutionContextTracking_KeyedFactoryThrows_StackIsClean() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddKeyedTransient("key", (sp, key) => throw new InvalidOperationException("keyed factory error")); + + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = false, + ValidateScopes = true + }); + + Assert.ThrowsExactly(() => sp.GetKeyedService("key")); + Assert.AreEqual(0, ResolutionContext.CurrentStack.Count); + } + + [TestMethod] + public void PatchForResolutionContextTracking_TypeCtorThrows_StackIsClean() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = false, + ValidateScopes = true + }); + + Assert.ThrowsExactly(() => sp.GetService()); + Assert.AreEqual(0, ResolutionContext.CurrentStack.Count); + } + + [TestMethod] + public void PatchForResolutionContextTracking_KeyedTypeCtorThrows_StackIsClean() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddKeyedTransient("key"); + + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection, + new ExtendedServiceProviderOptions + { + DetectIncorrectUsageOfTransientDisposables = true, + ValidateOnBuild = false, + ValidateScopes = true + }); + + Assert.ThrowsExactly(() => sp.GetKeyedService("key")); + Assert.AreEqual(0, ResolutionContext.CurrentStack.Count); + } + + private sealed class ThrowingConstructor + { + public ThrowingConstructor() + { + throw new InvalidOperationException("constructor error"); + } + } } } diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Tests.cs index 5f87218..dee436b 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Tests.cs @@ -80,6 +80,65 @@ public void Singleton_Resolve_DependsOn_Value() Assert.AreEqual("val1", service.Dep); } + [TestMethod] + public void TryAddSingleton_EmptyDependsOn_RegistersService() + { + var serviceCollection = new ServiceCollection(); + var emptyDependsOn = Array.Empty(); + serviceCollection.TryAddSingleton(typeof(SimpleService), emptyDependsOn); + + Assert.AreEqual(1, serviceCollection.Count); + Assert.IsTrue(serviceCollection.IsServiceRegistered()); + + using var serviceProvider = serviceCollection.BuildServiceProvider(); + var service = serviceProvider.GetRequiredService(); + Assert.IsNotNull(service); + } + + [TestMethod] + public void TryAddScoped_EmptyDependsOn_RegistersService() + { + var serviceCollection = new ServiceCollection(); + var emptyDependsOn = Array.Empty(); + serviceCollection.TryAddScoped(emptyDependsOn); + + Assert.AreEqual(1, serviceCollection.Count); + Assert.IsTrue(serviceCollection.IsServiceRegistered()); + + using var serviceProvider = serviceCollection.BuildServiceProvider(); + var service = serviceProvider.GetRequiredService(); + Assert.IsNotNull(service); + } + + [TestMethod] + public void TryAddTransient_EmptyDependsOn_RegistersService() + { + var serviceCollection = new ServiceCollection(); + var emptyDependsOn = Array.Empty(); + serviceCollection.TryAddTransient(emptyDependsOn); + + Assert.AreEqual(1, serviceCollection.Count); + Assert.IsTrue(serviceCollection.IsServiceRegistered()); + + using var serviceProvider = serviceCollection.BuildServiceProvider(); + var service = serviceProvider.GetRequiredService(); + Assert.IsNotNull(service); + } + + [TestMethod] + public void TryAddKeyedSingleton_EmptyDependsOn_RegistersService() + { + var serviceCollection = new ServiceCollection(); + var emptyDependsOn = Array.Empty(); + serviceCollection.TryAddKeyedSingleton("key1", emptyDependsOn); + + Assert.AreEqual(1, serviceCollection.Count); + + using var serviceProvider = serviceCollection.BuildServiceProvider(); + var service = serviceProvider.GetRequiredKeyedService("key1"); + Assert.IsNotNull(service); + } + [TestMethod] public void Resolve_KeyedService_DependsOn_Parameter() { @@ -295,6 +354,10 @@ public OpenGenericServiceWithKeyedDep(IKeyedService keyedService) public IKeyedService KeyedService { get; } } + + public class SimpleService + { + } } } diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Registration.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Registration.Tests.cs new file mode 100644 index 0000000..cd17314 --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Registration.Tests.cs @@ -0,0 +1,132 @@ +using Microsoft.Extensions.DependencyInjection; +using Mammoth.Cqrs.Infrastructure.Tests.Infrastructure; + +namespace Mammoth.Extensions.DependencyInjection.Tests +{ + [TestClass] + public class ServiceCollectionExtensionsRegistrationTests + { + [TestMethod] + public void IsTransientServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsTransientServiceRegistered(typeof(TestService))); + } + + [TestMethod] + public void IsScopedServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsScopedServiceRegistered()); + Assert.IsFalse(serviceCollection.IsScopedServiceRegistered(typeof(TestService))); + } + + [TestMethod] + public void IsSingletonServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered(typeof(TestService))); + } + + [TestMethod] + public void IsKeyedSingletonServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsKeyedSingletonServiceRegistered("key")); + Assert.IsFalse(serviceCollection.IsKeyedSingletonServiceRegistered(typeof(TestService), "key")); + } + + [TestMethod] + public void IsKeyedScopedServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsKeyedScopedServiceRegistered("key")); + Assert.IsFalse(serviceCollection.IsKeyedScopedServiceRegistered(typeof(TestService), "key")); + } + + [TestMethod] + public void IsKeyedTransientServiceRegistered_Returns_False_When_Service_Not_Registered() + { + var serviceCollection = new ServiceCollection(); + + Assert.IsFalse(serviceCollection.IsKeyedTransientServiceRegistered("key")); + Assert.IsFalse(serviceCollection.IsKeyedTransientServiceRegistered(typeof(TestService), "key")); + } + + [TestMethod] + public void IsTransientServiceRegistered_Returns_True_When_Registered_As_Transient() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + + Assert.IsTrue(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsScopedServiceRegistered()); + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void IsScopedServiceRegistered_Returns_True_When_Registered_As_Scoped() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddScoped(); + + Assert.IsFalse(serviceCollection.IsTransientServiceRegistered()); + Assert.IsTrue(serviceCollection.IsScopedServiceRegistered()); + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void IsSingletonServiceRegistered_Returns_True_When_Registered_As_Singleton() + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + + Assert.IsFalse(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsScopedServiceRegistered()); + Assert.IsTrue(serviceCollection.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void IsKeyedTransientServiceRegistered_Returns_True_When_Registered_As_Keyed_Transient() + { + var serviceCollection = new ServiceCollection(); + var key = Guid.NewGuid(); + serviceCollection.AddKeyedTransient(key); + + Assert.IsTrue(serviceCollection.IsKeyedTransientServiceRegistered(key)); + Assert.IsFalse(serviceCollection.IsKeyedScopedServiceRegistered(key)); + Assert.IsFalse(serviceCollection.IsKeyedSingletonServiceRegistered(key)); + } + + [TestMethod] + public void IsKeyedScopedServiceRegistered_Returns_True_When_Registered_As_Keyed_Scoped() + { + var serviceCollection = new ServiceCollection(); + var key = Guid.NewGuid(); + serviceCollection.AddKeyedScoped(key); + + Assert.IsFalse(serviceCollection.IsKeyedTransientServiceRegistered(key)); + Assert.IsTrue(serviceCollection.IsKeyedScopedServiceRegistered(key)); + Assert.IsFalse(serviceCollection.IsKeyedSingletonServiceRegistered(key)); + } + + [TestMethod] + public void IsKeyedSingletonServiceRegistered_Returns_True_When_Registered_As_Keyed_Singleton() + { + var serviceCollection = new ServiceCollection(); + var key = Guid.NewGuid(); + serviceCollection.AddKeyedSingleton(key); + + Assert.IsFalse(serviceCollection.IsKeyedTransientServiceRegistered(key)); + Assert.IsFalse(serviceCollection.IsKeyedScopedServiceRegistered(key)); + Assert.IsTrue(serviceCollection.IsKeyedSingletonServiceRegistered(key)); + } + } +} diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderExtensions.Registration.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderExtensions.Registration.Tests.cs index 6ca675a..2bf085d 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderExtensions.Registration.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderExtensions.Registration.Tests.cs @@ -272,6 +272,141 @@ public void Can_Mix_Object_And_String_Keys() Assert.IsTrue(sp.IsKeyedServiceRegistered(objectKey)); Assert.IsTrue(sp.IsKeyedTransientServiceRegistered(objectKey)); } + + #region Issue #19 - GetServiceDescriptors Bidirectional IsAssignableFrom Bug + + [TestMethod] + public void Issue19_BaseTypeIsolation_Register_Object_As_Singleton_And_TestService_As_Transient_Verify_No_CrossMatch() + { + // Arrange: Register object as Singleton and TestService as Transient + // With the buggy code, object.IsAssignableFrom(TestService) = true, so GetServiceDescriptors for TestService + // would incorrectly include the object registration, causing .Last() to return the wrong (Singleton) descriptor. + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(typeof(object)); // Register base type as singleton + serviceCollection.AddTransient(); // Register specific type as transient + + // Act: directly test GetServiceDescriptors - the code path that was fixed + var testServiceDescriptors = serviceCollection.GetServiceDescriptors(typeof(TestService)); + + // Assert: GetServiceDescriptors for TestService should NOT include the object registration + Assert.AreEqual(1, testServiceDescriptors.Length, "Should only return the TestService registration, not the object registration"); + Assert.AreEqual(typeof(TestService), testServiceDescriptors[0].ServiceType); + Assert.AreEqual(ServiceLifetime.Transient, testServiceDescriptors[0].Lifetime); + + // Also verify using IServiceCollection lifetime helpers (which use GetServiceDescriptors internally) + Assert.IsTrue(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void Issue19_InterfaceHierarchy_Register_Interface_And_Implementation_With_Different_Lifetimes() + { + // Arrange: Register ITestService as Singleton and TestService as Transient + // This tests that interface and implementation registrations are correctly distinguished + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(typeof(ITestService), typeof(TestService)); // ITestService -> TestService as Singleton + serviceCollection.AddTransient(); // Direct TestService as Transient + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection); + + // Act & Assert + // ITestService should be singleton (first registration) + Assert.IsTrue(sp.IsSingletonServiceRegistered(typeof(ITestService))); + + // TestService direct registration should be transient (last registration) + Assert.IsTrue(sp.IsTransientServiceRegistered()); + Assert.IsFalse(sp.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void Issue19_LastDescriptorSelection_Multiple_Registrations_Same_Type_Last_Wins() + { + // Arrange: Register base type as singleton, then TestService multiple times with different lifetimes. + // With the buggy bidirectional check, querying GetServiceDescriptors(TestService) would also include + // the object singleton, causing .Last() to return the wrong (Singleton) lifetime. + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(typeof(object)); // Base type - previously caused false positive matches + serviceCollection.AddSingleton(); + serviceCollection.AddScoped(); + serviceCollection.AddTransient(); + + // Act + var descriptors = serviceCollection.GetServiceDescriptors(typeof(TestService)); + + // Assert - only the 3 TestService registrations should be present (not the object singleton) + Assert.AreEqual(3, descriptors.Length, "Should only return the 3 TestService registrations, not include the object singleton"); + + // The last one is transient + Assert.AreEqual(ServiceLifetime.Transient, descriptors.Last().Lifetime); + + // When checking using IServiceCollection extension (uses GetServiceDescriptors internally), the last registration wins + Assert.IsTrue(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsScopedServiceRegistered()); // Last is not scoped + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); // Last is not singleton + } + + [TestMethod] + public void Issue19_UnrelatedTypes_Do_Not_Cross_Match_Even_With_Interface() + { + // Arrange: Both TestService and AnotherTestService implement IAnotherInterface + // The bug could cause them to cross-match due to bidirectional IsAssignableFrom + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(typeof(IAnotherInterface), typeof(AnotherTestService)); + serviceCollection.AddTransient(); + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection); + + // Act & Assert + // Querying for TestService should NOT return the IAnotherInterface registration + Assert.IsTrue(sp.IsTransientServiceRegistered()); + Assert.IsFalse(sp.IsSingletonServiceRegistered()); + + // Querying for IAnotherInterface should return the singleton + Assert.IsTrue(sp.IsSingletonServiceRegistered(typeof(IAnotherInterface))); + } + + [TestMethod] + public void Issue19_Exact_Type_Match_Only() + { + // Arrange: Register TestService as singleton + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); + using var sp = ServiceProviderFactory.CreateServiceProvider(serviceCollection); + + // Act + var descriptors = serviceCollection.GetServiceDescriptors(typeof(TestService)); + + // Assert - should find exact match only + Assert.AreEqual(1, descriptors.Length); + Assert.AreEqual(typeof(TestService), descriptors[0].ServiceType); + Assert.AreEqual(ServiceLifetime.Singleton, descriptors[0].Lifetime); + + // Should be registered as singleton + Assert.IsTrue(sp.IsSingletonServiceRegistered()); + } + + [TestMethod] + public void Issue19_Implementation_Type_Matches_Interface_Registration() + { + // Arrange: Register interface with implementation as singleton and implementation directly as transient. + // With the buggy bidirectional check, GetServiceDescriptors(TestService) could incorrectly include + // the ITestService registration because TestService.IsAssignableFrom(ITestService) would be checked. + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(); // Register interface->implementation as singleton + serviceCollection.AddTransient(); // Register implementation directly as transient + + // Act: directly test GetServiceDescriptors for TestService - the code path that was fixed + var testServiceDescriptors = serviceCollection.GetServiceDescriptors(typeof(TestService)); + + // Assert: GetServiceDescriptors for TestService should NOT include the ITestService registration + Assert.AreEqual(1, testServiceDescriptors.Length, "Should only return TestService registration, not ITestService"); + Assert.AreEqual(typeof(TestService), testServiceDescriptors[0].ServiceType); + Assert.AreEqual(ServiceLifetime.Transient, testServiceDescriptors[0].Lifetime); + + // Also verify using IServiceCollection lifetime helpers (which use GetServiceDescriptors internally) + Assert.IsTrue(serviceCollection.IsTransientServiceRegistered()); + Assert.IsFalse(serviceCollection.IsSingletonServiceRegistered()); + } + + #endregion } } diff --git a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs index 358b309..55986a4 100644 --- a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs +++ b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs @@ -44,9 +44,14 @@ public static IServiceCollection PatchForResolutionContextTracking(IServiceColle { // track the object we are about to resolve ResolutionContext.CurrentStack.Push(new ServiceIdentifier(descriptor.ServiceKey, descriptor.ServiceType)); - var instance = originalFactory(sp); - ResolutionContext.CurrentStack.Pop(); - return instance; + try + { + return originalFactory(sp); + } + finally + { + ResolutionContext.CurrentStack.Pop(); + } }, descriptor.Lifetime); collection.Add(d); @@ -61,9 +66,14 @@ public static IServiceCollection PatchForResolutionContextTracking(IServiceColle sp => { ResolutionContext.CurrentStack.Push(new ServiceIdentifier(descriptor.ServiceKey, descriptor.ServiceType)); - var instance = ActivatorUtilities.CreateInstance(sp, implementationType); - ResolutionContext.CurrentStack.Pop(); - return instance; + try + { + return ActivatorUtilities.CreateInstance(sp, implementationType); + } + finally + { + ResolutionContext.CurrentStack.Pop(); + } }, descriptor.Lifetime); collection.Add(d); @@ -91,9 +101,14 @@ public static IServiceCollection PatchForResolutionContextTracking(IServiceColle { // track the object we are about to resolve ResolutionContext.CurrentStack.Push(new ServiceIdentifier(descriptor.ServiceKey, descriptor.ServiceType)); - var instance = originalFactory(sp, key); - ResolutionContext.CurrentStack.Pop(); - return instance; + try + { + return originalFactory(sp, key); + } + finally + { + ResolutionContext.CurrentStack.Pop(); + } }, descriptor.Lifetime); collection.Add(d); @@ -109,9 +124,14 @@ public static IServiceCollection PatchForResolutionContextTracking(IServiceColle (sp, _) => { ResolutionContext.CurrentStack.Push(new ServiceIdentifier(descriptor.ServiceKey, descriptor.ServiceType)); - var instance = ActivatorUtilities.CreateInstance(sp, implementationType); - ResolutionContext.CurrentStack.Pop(); - return instance; + try + { + return ActivatorUtilities.CreateInstance(sp, implementationType); + } + finally + { + ResolutionContext.CurrentStack.Pop(); + } }, descriptor.Lifetime); collection.Add(d); @@ -158,8 +178,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 +195,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 +234,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 +263,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 +410,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 diff --git a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.KeyedService.cs b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.KeyedService.cs index eefa270..3db6869 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.KeyedService.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.KeyedService.cs @@ -38,6 +38,7 @@ public static void TryAddSingleton(this IServiceCollection services, Type servic if (dependsOn.Length == 0) { services.TryAddSingleton(serviceType); + return; } GetConstructorAndParameters(serviceType, out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -69,6 +70,7 @@ public static void TryAddSingleton(this IServiceCollection services, Type servic if (dependsOn.Length == 0) { services.TryAddSingleton(serviceType, implementationType); + return; } GetConstructorAndParameters(implementationType, out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -102,6 +104,7 @@ public static void TryAddSingleton(this IServiceCollection services, D if (dependsOn.Length == 0) { services.TryAddSingleton(); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -150,6 +153,7 @@ public static void TryAddScoped(this IServiceCollection services, Type serviceTy if (dependsOn.Length == 0) { services.TryAddScoped(serviceType); + return; } GetConstructorAndParameters(serviceType, out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -181,6 +185,7 @@ public static void TryAddScoped(this IServiceCollection services, Type serviceTy if (dependsOn.Length == 0) { services.TryAddScoped(serviceType, implementationType); + return; } GetConstructorAndParameters(implementationType, out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -214,6 +219,7 @@ public static void TryAddScoped(this IServiceCollection services, Depe if (dependsOn.Length == 0) { services.TryAddScoped(); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -277,6 +283,7 @@ public static void TryAddTransient(this IServiceCollection services, Type servic if (dependsOn.Length == 0) { services.TryAddTransient(serviceType, implementationType); + return; } GetConstructorAndParameters(implementationType, out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -310,6 +317,7 @@ public static void TryAddTransient(this IServiceCollection services, D if (dependsOn.Length == 0) { services.TryAddTransient(); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -360,6 +368,7 @@ public static void TryAddKeyedSingleton(this IServiceCollection servic if (dependsOn.Length == 0) { services.TryAddKeyedSingleton(serviceKey); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -395,6 +404,7 @@ public static void TryAddKeyedSingleton(this IService if (dependsOn.Length == 0) { services.TryAddKeyedSingleton(serviceKey); + return; } GetConstructorAndParameters(typeof(TImplementation), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -428,6 +438,7 @@ public static void TryAddKeyedScoped(this IServiceCollection services, if (dependsOn.Length == 0) { services.TryAddKeyedScoped(serviceKey); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -463,6 +474,7 @@ public static void TryAddKeyedScoped(this IServiceCol if (dependsOn.Length == 0) { services.TryAddKeyedScoped(serviceKey); + return; } GetConstructorAndParameters(typeof(TImplementation), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -496,6 +508,7 @@ public static void TryAddKeyedTransient(this IServiceCollection servic if (dependsOn.Length == 0) { services.TryAddKeyedTransient(serviceKey); + return; } GetConstructorAndParameters(typeof(TService), out ConstructorInfo ctor, out ParameterInfo[] parameter); @@ -531,6 +544,7 @@ public static void TryAddKeyedTransient(this IService if (dependsOn.Length == 0) { services.TryAddKeyedTransient(serviceKey); + return; } GetConstructorAndParameters(typeof(TImplementation), out ConstructorInfo ctor, out ParameterInfo[] parameter); diff --git a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs index 72c9030..9ddbbbe 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs @@ -51,7 +51,7 @@ public static bool IsKeyedServiceRegistered(this IServiceCollection services, ob } /// - /// Gets an array of objects for services that are assignable to or from the specified . + /// Gets an array of objects for services that are assignable from the specified . /// /// The service collection. /// The type of the service. @@ -66,7 +66,7 @@ public static ServiceDescriptor[] GetServiceDescriptors(this IServiceCollection return services.Where(serviceDescriptor => (isKeyedService == null || serviceDescriptor.IsKeyedService == isKeyedService) - && (serviceType.IsAssignableFrom(serviceDescriptor.ServiceType) || serviceDescriptor.ServiceType.IsAssignableFrom(serviceType))) + && serviceType.IsAssignableFrom(serviceDescriptor.ServiceType)) .ToArray(); } @@ -84,7 +84,7 @@ public static bool IsTransientServiceRegistered(this IServiceCollection services } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: false); - return descriptors.Last().Lifetime == ServiceLifetime.Transient; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Transient; } /// @@ -112,7 +112,7 @@ public static bool IsScopedServiceRegistered(this IServiceCollection services, T } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: false); - return descriptors.Last().Lifetime == ServiceLifetime.Scoped; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Scoped; } /// @@ -140,7 +140,7 @@ public static bool IsSingletonServiceRegistered(this IServiceCollection services } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: false); - return descriptors.Last().Lifetime == ServiceLifetime.Singleton; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Singleton; } /// @@ -174,9 +174,9 @@ public static bool IsKeyedSingletonServiceRegistered(this IServiceCollection ser } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: true) - .Where(d => d.ServiceKey == serviceKey) + .Where(d => Equals(d.ServiceKey, serviceKey)) .ToArray(); - return descriptors.Last().Lifetime == ServiceLifetime.Singleton; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Singleton; } /// @@ -210,9 +210,9 @@ public static bool IsKeyedScopedServiceRegistered(this IServiceCollection servic } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: true) - .Where(d => d.ServiceKey == serviceKey) + .Where(d => Equals(d.ServiceKey, serviceKey)) .ToArray(); - return descriptors.Last().Lifetime == ServiceLifetime.Scoped; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Scoped; } /// @@ -246,9 +246,9 @@ public static bool IsKeyedTransientServiceRegistered(this IServiceCollection ser } var descriptors = services.GetServiceDescriptors(serviceType, isKeyedService: true) - .Where(d => d.ServiceKey == serviceKey) + .Where(d => Equals(d.ServiceKey, serviceKey)) .ToArray(); - return descriptors.Last().Lifetime == ServiceLifetime.Transient; + return descriptors.Length > 0 && descriptors[descriptors.Length - 1].Lifetime == ServiceLifetime.Transient; } ///