From 6adaf28b2ce6f1b11c45773c450f86a19b29464c Mon Sep 17 00:00:00 2001 From: Gian Maria Ricci Date: Mon, 24 Jun 2024 17:20:21 +0200 Subject: [PATCH] Fixed IsDefault() usage in DependencyInjectionAdapter If multiple concrete classes are registered in castle, when you resolve castle resolves the first one. With Microsoft DI is the oposite, you want the last registered. The resolution is now fixed to honor IsDefault() because previous code registered every component with IsDefault() if it is registered from the adapter. --- .vscode/tasks.json | 15 ++-- .../CustomAssumptionTests.cs | 47 ++++++++++++ .../RegistrationAdapter.cs | 6 +- .../WindsorScopedServiceProvider.cs | 72 +++++++++++++------ 4 files changed, 110 insertions(+), 30 deletions(-) diff --git a/.vscode/tasks.json b/.vscode/tasks.json index e176868402..36904a4e05 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -1,16 +1,21 @@ { - "version": "0.1.0", + "version": "2.0.0", "command": "dotnet", - "isShellCommand": true, "args": [], "tasks": [ { - "taskName": "build", + "label": "build", + "type": "shell", + "command": "dotnet", "args": [ + "build", "${workspaceRoot}/src/Castle.Windsor.Tests/Castle.Windsor.Tests.csproj" ], - "isBuildCommand": true, - "problemMatcher": "$msCompile" + "problemMatcher": "$msCompile", + "group": { + "_id": "build", + "isDefault": false + } } ] } \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs index df34569513..5b2d6458d7 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/CustomAssumptionTests.cs @@ -1,5 +1,6 @@ #if NET8_0_OR_GREATER using Castle.MicroKernel; +using Castle.MicroKernel.Registration; using Microsoft.Extensions.DependencyInjection; using System; using System.Linq; @@ -366,6 +367,52 @@ public void TryToResolveScopedInOtherThread() Assert.True(task.Result); } + [Fact] + public void Resolve_order_in_castle() + { + var serviceCollection = GetServiceCollection(); + _factory = new WindsorServiceProviderFactory(); + _container = _factory.CreateBuilder(serviceCollection); + + _container.Register( + Component.For().ImplementedBy() + , Component.For().ImplementedBy()); + + var provider = _factory.CreateServiceProvider(_container); + + var resolvedWithCastle = _container.Resolve(); + var resolvedWithProvider = provider.GetRequiredService(); + + //SUper important: Assumption for resolve multiple services registerd with the same + //interface is different: castle resolves the first, Microsoft DI require you to + //resolve the latest. + Assert.IsType(resolvedWithCastle); + Assert.IsType(resolvedWithProvider); + } + + [Fact] + public void Resolve_order_in_castle_with_is_default() + { + var serviceCollection = GetServiceCollection(); + _factory = new WindsorServiceProviderFactory(); + _container = _factory.CreateBuilder(serviceCollection); + + _container.Register( + Component.For().ImplementedBy().IsDefault() + , Component.For().ImplementedBy()); + + var provider = _factory.CreateServiceProvider(_container); + + var resolvedWithCastle = _container.Resolve(); + var resolvedWithProvider = provider.GetRequiredService(); + + //SUper important: Assumption for resolve multiple services registerd with the same + //interface is different: castle resolves the first, Microsoft DI require you to + //resolve the latest. + Assert.IsType(resolvedWithCastle); + Assert.IsType(resolvedWithProvider); + } + protected override void Dispose(bool disposing) { base.Dispose(disposing); diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs index b4096e5317..0781d1b514 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs @@ -66,8 +66,7 @@ public static IRegistration FromOpenGenericServiceDescriptor( throw new System.ArgumentException("Unsupported ServiceDescriptor"); } #endif - return ResolveLifestyle(registration, service) - .IsDefault(); + return ResolveLifestyle(registration, service); } public static IRegistration FromServiceDescriptor( @@ -127,8 +126,7 @@ public static IRegistration FromServiceDescriptor( registration = UsingImplementation(registration, service); } #endif - return ResolveLifestyle(registration, service) - .IsDefault(); + return ResolveLifestyle(registration, service); } public static string OriginalComponentName(string uniqueComponentName) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs index 7e2c1a6966..1b8b846439 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs @@ -14,7 +14,6 @@ namespace Castle.Windsor.Extensions.DependencyInjection { - using Castle.MicroKernel; using Castle.MicroKernel.Handlers; using Castle.Windsor; using Castle.Windsor.Extensions.DependencyInjection.Scope; @@ -96,7 +95,6 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional) { if (container.Kernel.HasComponent(serviceType)) { -#if NET8_0_OR_GREATER //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. @@ -105,7 +103,7 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional) //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) + if (realRegistrations.Count == 1) { registrationName = realRegistrations[0].ComponentModel.Name; } @@ -116,25 +114,39 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional) } 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. + //Need to honor IsDefault for castle registrations. + var isDefaultRegistration = realRegistrations + .FirstOrDefault(dh => dh.ComponentModel.ExtendedProperties.Any(ComponentIsDefault)); - //take first non generic - for (int i = realRegistrations.Count - 1; i >= 0; i--) + //Remember that castle has a specific order of resolution, if someone registered something in castle with + //IsDefault() it Must be honored. + if (isDefaultRegistration != null) + { + registrationName = isDefaultRegistration.ComponentModel.Name; + } + else { - if (!realRegistrations[i].ComponentModel.Implementation.IsGenericTypeDefinition) + //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. In this situation we do not use the same rule of Castle where the first service win but + //we use the framework rule that: + //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--) { - registrationName = realRegistrations[i].ComponentModel.Name; - break; + 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 we did not find any non generic, take the last one. + if (registrationName == null) + { + registrationName = realRegistrations[realRegistrations.Count - 1].ComponentModel.Name; + } } } @@ -143,10 +155,6 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional) return null; } 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<>)) @@ -197,6 +205,28 @@ private object ResolveInstanceOrNull(Type serviceType, bool isOptional) return container.Resolve(serviceType); } + private static bool ComponentIsDefault(KeyValuePair property) + { + if (!Core.Internal.Constants.DefaultComponentForServiceFilter.Equals(property.Key)) + { + //not the property we are looking for + return false; + } + + if (property.Value is bool boolValue) + { + return boolValue; + } + + if (property.Value is Predicate predicate) + { + //this is a method info that we can invoke to get the value. + return predicate(null); + } + + return false; + } + #if NET6_0_OR_GREATER public bool IsService(Type serviceType)