search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.25k stars 4.73k forks source link

Unable to use a enum as a key for keyed services in controller constructor #96140

Closed inayelle closed 3 months ago

inayelle commented 10 months ago

Is there an existing issue for this?

Describe the bug

Let's say we have the following services:

public enum WeatherProvider
{
    AccuWeather = 1,
    OpenWeatherMap = 2,
}

public interface IWeatherProvider
{
    string GetWeatherForCity(string city);
}

public sealed class AccuWeatherWeatherProvider : IWeatherProvider
{
    public string GetWeatherForCity(string city) => "AccuWeather";
}

public sealed class OpenWeatherMapWeatherProvider : IWeatherProvider
{
    public string GetWeatherForCity(string city) => "OpenWeather";
}

And their registration in the dependency container using enum keys:

var builder = WebApplication.CreateBuilder();

builder
   .Services
   .AddKeyedScoped<IWeatherProvider, OpenWeatherMapWeatherProvider>(WeatherProvider.OpenWeatherMap)
   .AddKeyedScoped<IWeatherProvider, AccuWeatherWeatherProvider>(WeatherProvider.AccuWeather);

Having this, we can resolve a weather provider by interface and key using method injection in a controller (which works fine):

    [HttpGet]
    public object GetWeather(
        [FromQuery] string city,
        [FromKeyedServices(WeatherProvider.OpenWeatherMap)] IWeatherProvider weatherProvider
    )
    {
        return new
        {
            City = city,
            Weather = weatherProvider.GetWeatherForCity(city),
        };
    }

However, if we wanted to inject a default weather provider via constructor, like:

[ApiController]
[Route("weather")]
public sealed class WeatherController : ControllerBase
{
    private readonly IWeatherProvider _defaultWeatherProvider;

    public WeatherController([FromKeyedServices(WeatherProvider.AccuWeather)] IWeatherProvider defaultWeatherProvider)
    {
        _defaultWeatherProvider = defaultWeatherProvider;
    }

    // actions omitted for brevity
}

We won't be able to query any of the actions of this controller due to a controller activation failure with an exception:

System.ArgumentException: Expression of type 'KeyedServicesExample.Api.Providers.WeatherProvider' cannot be used for parameter of type 'System.Object' of method 'System.Object GetService(System.IServiceProvider, System.Type, System.Type, Boolean, System.Object)' (Parameter 'arg4')
         at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index)
         at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3, Expression arg4)
         at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.BuildFactoryExpression(ConstructorInfo constructor, Nullable`1[] parameterMap, Expression serviceProvider, Expression factoryArgumentArray)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactoryInternal(Type instanceType, Type[] argumentTypes, ParameterExpression& provider, ParameterExpression& argumentArray, Expression& factoryExpressionBody)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes)
         at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.CreateActivator(ControllerActionDescriptor descriptor)
         at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.CreateControllerFactory(ControllerActionDescriptor descriptor)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvokerCache.GetCachedResult(ControllerContext controllerContext)
         at Microsoft.AspNetCore.Mvc.Routing.ControllerRequestDelegateFactory.<>c__DisplayClass12_0.<CreateRequestDelegate>b__0(HttpContext context)
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
         at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
         at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
         at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

I believe it doesn't work because the key in this example is an enum, which is a value type. If we replace the enum key with a string key, it all works like a charm.

Expected Behavior

It should be possible to resolve keyed services using FromKeyedServices attribute via controller constructor.

Steps To Reproduce

Link to an example project: https://github.com/inayelle/keyed-services-api-example

Exceptions (if any)

System.ArgumentException: Expression of type 'KeyedServicesExample.Api.Providers.WeatherProvider' cannot be used for parameter of type 'System.Object' of method 'System.Object GetService(System.IServiceProvider, System.Type, System.Type, Boolean, System.Object)' (Parameter 'arg4')
         at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index)
         at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3, Expression arg4)
         at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.BuildFactoryExpression(ConstructorInfo constructor, Nullable`1[] parameterMap, Expression serviceProvider, Expression factoryArgumentArray)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactoryInternal(Type instanceType, Type[] argumentTypes, ParameterExpression& provider, ParameterExpression& argumentArray, Expression& factoryExpressionBody)
         at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes)
         at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.CreateActivator(ControllerActionDescriptor descriptor)
         at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.CreateControllerFactory(ControllerActionDescriptor descriptor)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvokerCache.GetCachedResult(ControllerContext controllerContext)
         at Microsoft.AspNetCore.Mvc.Routing.ControllerRequestDelegateFactory.<>c__DisplayClass12_0.<CreateRequestDelegate>b__0(HttpContext context)
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
         at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
         at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
         at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

.NET Version

8.0.100

Anything else?

.NET SDK:
 Version:           8.0.100
 Commit:            57efcf1350
 Workload version:  8.0.100-manifests.6c33ef20

Runtime Environment:
 OS Name:     arch
 OS Version:
 OS Platform: Linux
 RID:         linux-x64
 Base Path:   /home/inayelle/local/lib/dotnet/sdk/8.0.100/

.NET workloads installed:
 Workload version: 8.0.100-manifests.6c33ef20
There are no installed workloads to display.

Host:
  Version:      8.0.0
  Architecture: x64
  Commit:       5535e31a71

.NET SDKs installed:
  7.0.404 [/home/inayelle/local/lib/dotnet/sdk]
  8.0.100 [/home/inayelle/local/lib/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 7.0.14 [/home/inayelle/local/lib/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0 [/home/inayelle/local/lib/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 7.0.14 [/home/inayelle/local/lib/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0 [/home/inayelle/local/lib/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/home/inayelle/local/lib/dotnet]

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
Kahbazi commented 10 months ago

I could reproduce this bug. MVC is using ActivatorUtilities which cause this bug. I think the issue should be transferred to the runtime repo.

captainsafia commented 10 months ago

@Kahbazi Thanks for confirming that this repros.

Tagging @benjaminpetit to confirm whether or not we should transfer this to the runtime repo for a fix.

benjaminpetit commented 10 months ago

Yes I think we should transfer it, and assign it to me.

ghost commented 10 months ago

Tagging subscribers to this area: @cston See info in area-owners.md if you want to be subscribed.

Issue Details
### Is there an existing issue for this? - [X] I have searched the existing issues ### Describe the bug Let's say we have the following services: ```csharp public enum WeatherProvider { AccuWeather = 1, OpenWeatherMap = 2, } public interface IWeatherProvider { string GetWeatherForCity(string city); } public sealed class AccuWeatherWeatherProvider : IWeatherProvider { public string GetWeatherForCity(string city) => "AccuWeather"; } public sealed class OpenWeatherMapWeatherProvider : IWeatherProvider { public string GetWeatherForCity(string city) => "OpenWeather"; } ``` And their registration in the dependency container using enum keys: ```csharp var builder = WebApplication.CreateBuilder(); builder .Services .AddKeyedScoped(WeatherProvider.OpenWeatherMap) .AddKeyedScoped(WeatherProvider.AccuWeather); ``` Having this, we can resolve a weather provider by interface and key using method injection in a controller (which works fine): ```csharp [HttpGet] public object GetWeather( [FromQuery] string city, [FromKeyedServices(WeatherProvider.OpenWeatherMap)] IWeatherProvider weatherProvider ) { return new { City = city, Weather = weatherProvider.GetWeatherForCity(city), }; } ``` However, if we wanted to inject a default weather provider via constructor, like: ``` [ApiController] [Route("weather")] public sealed class WeatherController : ControllerBase { private readonly IWeatherProvider _defaultWeatherProvider; public WeatherController([FromKeyedServices(WeatherProvider.AccuWeather)] IWeatherProvider defaultWeatherProvider) { _defaultWeatherProvider = defaultWeatherProvider; } // actions omitted for brevity } ``` We won't be able to query any of the actions of this controller due to a controller activation failure with an exception: ``` System.ArgumentException: Expression of type 'KeyedServicesExample.Api.Providers.WeatherProvider' cannot be used for parameter of type 'System.Object' of method 'System.Object GetService(System.IServiceProvider, System.Type, System.Type, Boolean, System.Object)' (Parameter 'arg4') at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index) at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3, Expression arg4) at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.BuildFactoryExpression(ConstructorInfo constructor, Nullable`1[] parameterMap, Expression serviceProvider, Expression factoryArgumentArray) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactoryInternal(Type instanceType, Type[] argumentTypes, ParameterExpression& provider, ParameterExpression& argumentArray, Expression& factoryExpressionBody) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes) at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.CreateActivator(ControllerActionDescriptor descriptor) at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.CreateControllerFactory(ControllerActionDescriptor descriptor) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvokerCache.GetCachedResult(ControllerContext controllerContext) at Microsoft.AspNetCore.Mvc.Routing.ControllerRequestDelegateFactory.<>c__DisplayClass12_0.b__0(HttpContext context) at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext) at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context) at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext) at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider) at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) ``` I believe it doesn't work because the key in this example is an enum, which is a value type. If we replace the enum key with a string key, it all works like a charm. ### Expected Behavior It should be possible to resolve keyed services using `FromKeyedServices` attribute via controller constructor. ### Steps To Reproduce Link to an example project: https://github.com/inayelle/keyed-services-api-example ### Exceptions (if any) ``` System.ArgumentException: Expression of type 'KeyedServicesExample.Api.Providers.WeatherProvider' cannot be used for parameter of type 'System.Object' of method 'System.Object GetService(System.IServiceProvider, System.Type, System.Type, Boolean, System.Object)' (Parameter 'arg4') at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index) at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1, Expression arg2, Expression arg3, Expression arg4) at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.BuildFactoryExpression(ConstructorInfo constructor, Nullable`1[] parameterMap, Expression serviceProvider, Expression factoryArgumentArray) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactoryInternal(Type instanceType, Type[] argumentTypes, ParameterExpression& provider, ParameterExpression& argumentArray, Expression& factoryExpressionBody) at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes) at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.CreateActivator(ControllerActionDescriptor descriptor) at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.CreateControllerFactory(ControllerActionDescriptor descriptor) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvokerCache.GetCachedResult(ControllerContext controllerContext) at Microsoft.AspNetCore.Mvc.Routing.ControllerRequestDelegateFactory.<>c__DisplayClass12_0.b__0(HttpContext context) at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext) at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context) at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext) at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider) at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) ``` ### .NET Version 8.0.100 ### Anything else? ``` .NET SDK: Version: 8.0.100 Commit: 57efcf1350 Workload version: 8.0.100-manifests.6c33ef20 Runtime Environment: OS Name: arch OS Version: OS Platform: Linux RID: linux-x64 Base Path: /home/inayelle/local/lib/dotnet/sdk/8.0.100/ .NET workloads installed: Workload version: 8.0.100-manifests.6c33ef20 There are no installed workloads to display. Host: Version: 8.0.0 Architecture: x64 Commit: 5535e31a71 .NET SDKs installed: 7.0.404 [/home/inayelle/local/lib/dotnet/sdk] 8.0.100 [/home/inayelle/local/lib/dotnet/sdk] .NET runtimes installed: Microsoft.AspNetCore.App 7.0.14 [/home/inayelle/local/lib/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.0 [/home/inayelle/local/lib/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 7.0.14 [/home/inayelle/local/lib/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0 [/home/inayelle/local/lib/dotnet/shared/Microsoft.NETCore.App] Other architectures found: None Environment variables: DOTNET_ROOT [/home/inayelle/local/lib/dotnet] global.json file: Not found Learn more: https://aka.ms/dotnet/info Download .NET: https://aka.ms/dotnet/download ```
Author: inayelle
Assignees: -
Labels: `bug`, `area-System.Linq.Expressions`
Milestone: -
inayelle commented 10 months ago

Hi @benjaminpetit

I filed this issue originally in the dotnet/aspnet repo because it reproduces specifically with injecting keyed services into aspnet controllers. If we hide the keyed injection behind a proxy class, like in the snippet below, then the resolution works as expected and no exceptions occur. I hope this fact may help.

public sealed class DefaultWeatherProviderProxy
{
    private readonly IWeatherProvider _weatherProvider;

    public DefaultWeatherProviderProxy([FromKeyedServices(WeatherProvider.OpenWeatherMap)] IWeatherProvider weatherProvider)
    {
        _weatherProvider = weatherProvider;
    }

    public string GetWeatherForCity(string city)
    {
        return _weatherProvider.GetWeatherForCity(city);
    }
}

[ApiController]
[Route("weather")]
public sealed class WeatherController : ControllerBase
{
    private readonly DefaultWeatherProviderProxy _defaultWeatherProvider;

    public WeatherController(DefaultWeatherProviderProxy defaultWeatherProvider)
    {
        _defaultWeatherProvider = defaultWeatherProvider;
    }

    // ...
}
dotnet-policy-service[bot] commented 3 months ago

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.

steveharter commented 3 months ago

The core issue is that ActivatorUtilities.CreateFactory() can't be used with keyed services if the key 's type is a value type, so the issue isn't just with enums.

lateapexearlyspeed commented 3 months ago

Hi @steveharter , just found that I had raised pr for this :)