Updated nuget test adapter fixed a resolution bug

The bug happened when you register a service with one NON keyed component then again with KEYED components. The adapter incorrectly checked only the first returned service for KEYED and returns null. Identified after update to Orleans 8.1.0
castleproject · May 22, 2024 · 17d1010 · 17d1010
1 parent 36bbbf1
commit 17d1010
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 33 deletions.
diff --git a/src/Castle.Facilities.AspNet.Mvc.Tests/Castle.Facilities.AspNet.Mvc.Tests.csproj b/src/Castle.Facilities.AspNet.Mvc.Tests/Castle.Facilities.AspNet.Mvc.Tests.csproj
@@ -15,11 +15,11 @@
 
 	<ItemGroup>
 		<PackageReference Include="Microsoft.AspNet.Mvc" Version="5.2.3" />
-		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 		<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" />
 		<PackageReference Include="Microsoft.Web.Infrastructure" Version="1.0.0" />
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/src/Castle.Facilities.AspNet.SystemWeb.Tests/Castle.Facilities.AspNet.SystemWeb.Tests.csproj b/src/Castle.Facilities.AspNet.SystemWeb.Tests/Castle.Facilities.AspNet.SystemWeb.Tests.csproj
@@ -19,10 +19,10 @@
 	</PropertyGroup>
 
 	<ItemGroup>
-		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 		<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" />
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/src/Castle.Facilities.AspNet.WebApi.Tests/Castle.Facilities.AspNet.WebApi.Tests.csproj b/src/Castle.Facilities.AspNet.WebApi.Tests/Castle.Facilities.AspNet.WebApi.Tests.csproj
@@ -15,11 +15,11 @@
 
 	<ItemGroup>
 		<PackageReference Include="Microsoft.AspNet.WebApi" Version="5.2.3" />
-		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 		<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" />
 		<PackageReference Include="Microsoft.Web.Infrastructure" Version="1.0.0" />
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/src/Castle.Facilities.AspNetCore.Tests/Castle.Facilities.AspNetCore.Tests.csproj b/src/Castle.Facilities.AspNetCore.Tests/Castle.Facilities.AspNetCore.Tests.csproj
@@ -7,8 +7,8 @@
 	<ItemGroup>
 		<!-- This is an intentional upgrade to NUnit. This is the solution for https://github.com/castleproject/Windsor/issues/243 once we upgrade NUnit and make dotnet test a first class citizen-->
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
-		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
+		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 		<PackageReference Include="Microsoft.AspNetCore" Version="2.0.1" />
 		<PackageReference Include="Microsoft.AspNetCore.Mvc" Version="2.0.2" />
 		<PackageReference Include="Microsoft.AspNetCore.Mvc.Razor" Version="2.0.2" />

diff --git a/src/Castle.Facilities.WcfIntegration.Tests/Castle.Facilities.WcfIntegration.Tests.csproj b/src/Castle.Facilities.WcfIntegration.Tests/Castle.Facilities.WcfIntegration.Tests.csproj
@@ -12,9 +12,9 @@
 	</PropertyGroup>
 
 	<ItemGroup>
-		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/...ions.DependencyInjection.Tests/Castle.Windsor.Extensions.DependencyInjection.Tests.csproj b/...ions.DependencyInjection.Tests/Castle.Windsor.Extensions.DependencyInjection.Tests.csproj
@@ -11,7 +11,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
     <PackageReference Include="System.Reflection.TypeExtensions" Version="4.7.0" />
     <PackageReference Include="xunit" Version="2.4.2" />
     <PackageReference Include="xunit.assert" Version="2.4.2" />

diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs
@@ -1,6 +1,6 @@
 #if NET8_0_OR_GREATER
+using Castle.MicroKernel;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection.Specification.Fakes;
 using System;
 using System.Linq;
 using System.Threading;
@@ -110,6 +110,21 @@ public void Scoped_service_resolved_outside_scope()
 			Assert.Equal(resolvedOutsideScope, resolvedAgainOutsideScope);
 		}
 
+		[Fact]
+		public void Mix_of_keyed_and_not_keyed()
+		{
+			var serviceCollection = GetServiceCollection();
+			serviceCollection.AddSingleton<ITestService, TestService>();
+			serviceCollection.AddKeyedSingleton<ITestService, AnotherTestService>("bla");
+
+			_serviceProvider = BuildServiceProvider(serviceCollection);
+
+			//can resolve the non-keyed
+			var nonKeyed = _serviceProvider.GetRequiredService<ITestService>();
+			Assert.NotNull(nonKeyed);
+			Assert.IsType<TestService>(nonKeyed);
+		}
+
 		[Fact]
 		public void Scoped_service_resolved_outside_scope_in_another_thread()
 		{
@@ -167,7 +182,8 @@ public async void Simulate_async_timer_without_wait()
 			ITestService resolvedInThread = null;
 			async Task ExecuteAsync()
 			{
-				while (!stop)
+				DateTime start = DateTime.UtcNow;
+				while (!stop && DateTime.UtcNow.Subtract(start).TotalSeconds < 10)
 				{
 					await Task.Delay(100);
 					if (shouldResolve)
@@ -178,10 +194,7 @@ async Task ExecuteAsync()
 				}
 			}
 			//fire and forget
-#pragma warning disable CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed
 			var task = ExecuteAsync();
-#pragma warning restore CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed
-
 			await Task.Delay(500);
 
 			var serviceCollection = GetServiceCollection();
@@ -353,7 +366,6 @@ public void TryToResolveScopedInOtherThread()
 			Assert.True(task.Result);
 		}
 
-
 		protected override void Dispose(bool disposing)
 		{
 			base.Dispose(disposing);
@@ -364,16 +376,12 @@ protected override void Dispose(bool disposing)
 		}
 	}
 
-	internal class TestService : ITestService
-	{
-	}
+	internal class TestService : ITestService;
 
-	internal class AnotherTestService : ITestService
-	{
-	}
+	internal class AnotherTestService : ITestService;
 
-	internal interface ITestService
-	{
-	}
+	internal class ThirdTestService : ITestService;
+
+	internal interface ITestService;
 }
 #endif
diff --git a/...Extensions.DependencyInjection.Tests/WindsorKeyedDependencyInjectionSpecificationTests.cs b/...Extensions.DependencyInjection.Tests/WindsorKeyedDependencyInjectionSpecificationTests.cs
@@ -2,7 +2,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Specification;
 using System;
-using Xunit;
 
 namespace Castle.Windsor.Extensions.DependencyInjection.Tests
 {

diff --git a/...ions.DependencyInjection.Tests/WindsorScopedServiceProviderCustomWindsorContainerTests.cs b/...ions.DependencyInjection.Tests/WindsorScopedServiceProviderCustomWindsorContainerTests.cs
@@ -53,6 +53,29 @@ public void Dispose()
 			GC.SuppressFinalize(this);
 		}
 
+		///// <summary>
+		///// To verify when a single test failed, open the corresponding test in dotnet/runtime repository,
+		///// then copy the test here, change name and execute with debugging etc etc.
+		///// This helps because source link support seems to be not to easy to use from the test runner
+		///// and this tricks makes everything really simpler.
+		///// </summary>
+		//[Fact]
+		//public void ClosedServicesPreferredOverOpenGenericServices_custom()
+		//{
+		//	// Arrange
+		//	var collection = new TestServiceCollection();
+		//	collection.AddTransient(typeof(IFakeOpenGenericService<PocoClass>), typeof(FakeService));
+		//	collection.AddTransient(typeof(IFakeOpenGenericService<>), typeof(FakeOpenGenericService<>));
+		//	collection.AddSingleton<PocoClass>();
+		//	var provider = CreateServiceProvider(collection);
+
+		//	// Act
+		//	var service = provider.GetService<IFakeOpenGenericService<PocoClass>>();
+
+		//	// Assert
+		//	Assert.IsType<FakeService>(service);
+		//}
+
 #if NET6_0_OR_GREATER
 #endif
 

diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs
@@ -14,6 +14,7 @@
 
 namespace Castle.Windsor.Extensions.DependencyInjection
 {
+	using Castle.MicroKernel;
 	using Castle.MicroKernel.Handlers;
 	using Castle.Windsor;
 	using Castle.Windsor.Extensions.DependencyInjection.Scope;
@@ -99,15 +100,53 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional)
 				//this is complicated by the concept of keyed service, because if you are about to resolve WITHOUTH KEY you do not
 				//need to resolve keyed services. Now Keyed services are available only in version 8 but we register with an helper
 				//all registered services so we can know if a service was really registered with keyed service or not.
-				var componentRegistration = container.Kernel.GetHandler(serviceType);
-				if (componentRegistration.ComponentModel.Name.StartsWith(KeyedRegistrationHelper.KeyedRegistrationPrefix))
+				var componentRegistrations = container.Kernel.GetHandlers(serviceType);
+
+				//now since the caller requested a NON Keyed component, we need to skip all keyed components.
+				var realRegistrations = componentRegistrations.Where(x => !x.ComponentModel.Name.StartsWith(KeyedRegistrationHelper.KeyedRegistrationPrefix)).ToList();
+				string registrationName = null;
+				if (realRegistrations.Count == 1) 
+				{
+					registrationName = realRegistrations[0].ComponentModel.Name;
+				}
+				else if (realRegistrations.Count == 0)
+				{
+					//No component is registered for the interface without key, resolution cannot be done.
+					registrationName = null;
+				}
+				else if (realRegistrations.Count > 1)
+				{
+					//more than one component is registered for the interface without key, we have some ambiguity that is resolved, based on test
+					//found in framework with this rule.
+					//1. Last component win.
+					//2. closed service are preferred over open generic.
+
+					//take first non generic
+					for (int i = realRegistrations.Count - 1; i >= 0; i--)
+					{
+						if (!realRegistrations[i].ComponentModel.Implementation.IsGenericTypeDefinition)
+						{
+							registrationName = realRegistrations[i].ComponentModel.Name;
+							break;
+						}
+					}
+
+					//if we did not find any non generic, take the last one.
+					if (registrationName == null)
+					{
+						registrationName = realRegistrations[realRegistrations.Count - 1].ComponentModel.Name;
+					}
+				}
+
+				if (registrationName == null)
 				{
-					//Component was registered as keyed component, so we really need to resolve with null because this is the old interface
-					//so no key is provided.
 					return null;
 				}
-#endif
+				return container.Resolve(registrationName, serviceType);
+#else
+				//no keyed component in previous framework, just resolve.
 				return container.Resolve(serviceType);
+#endif
 			}
 
 			if (serviceType.GetTypeInfo().IsGenericType && serviceType.GetGenericTypeDefinition() == typeof(IEnumerable<>))

diff --git a/src/Castle.Windsor.Tests/Castle.Windsor.Tests.csproj b/src/Castle.Windsor.Tests/Castle.Windsor.Tests.csproj
@@ -47,7 +47,7 @@
 		<PackageReference Include="Castle.Core-log4net" Version="[5.1.0,6.0)" />
 		<PackageReference Include="Castle.Core-NLog" Version="[5.1.0,6.0)" />
 		<PackageReference Include="NUnit" Version="3.13.3" />
-		<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+		<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
 	</ItemGroup>
 
 	<ItemGroup Condition="'$(TargetFramework)'=='net462'">