From b9e4f660e6137756de9905b2b3463a9ea6e2de12 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 29 Nov 2022 22:55:16 -0800 Subject: [PATCH] [release/7.0] Infer parameter source from ElementType for IEnumerable (#45256) * Infer ElementType for IEnumerable * Update src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs * Update src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs * Fix SignalR fromservices inference Co-authored-by: Bruno Oliveira Co-authored-by: Bruno Oliveira Co-authored-by: Safia Abdalla --- .../InferParameterBindingInfoConvention.cs | 21 +++++++- ...InferParameterBindingInfoConventionTest.cs | 54 +++++++++++++++++++ .../Core/src/Internal/HubMethodDescriptor.cs | 16 +++++- .../HubConnectionHandlerTestUtils/Hubs.cs | 5 ++ .../SignalR/test/HubConnectionHandlerTests.cs | 41 ++++++++++++++ 5 files changed, 135 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index 23b302191651..6642445e37de 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -120,7 +120,7 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { if (IsComplexTypeParameter(parameter)) { - if (_serviceProviderIsService?.IsService(parameter.ParameterType) is true) + if (IsService(parameter.ParameterType)) { return BindingSource.Services; } @@ -136,6 +136,25 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) return BindingSource.Query; } + private bool IsService(Type type) + { + if (_serviceProviderIsService == null) + { + return false; + } + + // IServiceProviderIsService will special case IEnumerable<> and always return true + // so, in this case checking the element type instead + if (type.IsConstructedGenericType && + type.GetGenericTypeDefinition() is Type genericDefinition && + genericDefinition == typeof(IEnumerable<>)) + { + type = type.GenericTypeArguments[0]; + } + + return _serviceProviderIsService.IsService(type); + } + private static bool ParameterExistsInAnyRoute(ActionModel action, string parameterName) { foreach (var selector in ActionAttributeRouteModel.FlattenSelectors(action)) diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index 68513623fffd..6b475eeba66a 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -543,6 +543,21 @@ public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfSimpleTypes Assert.Same(BindingSource.Body, result); } + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForIEnumerableOfSimpleTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.IEnumerableOfSimpleTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Body, result); + } + [Fact] public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfComplexTypes() { @@ -558,6 +573,21 @@ public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfComplexType Assert.Same(BindingSource.Body, result); } + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForIEnumerableOfComplexTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.IEnumerableOfComplexTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Body, result); + } + [Fact] public void InferBindingSourceForParameter_ReturnsServicesForComplexTypesRegisteredInDI() { @@ -576,6 +606,24 @@ public void InferBindingSourceForParameter_ReturnsServicesForComplexTypesRegiste Assert.Same(BindingSource.Services, result); } + [Fact] + public void InferBindingSourceForParameter_ReturnsServicesForIEnumerableOfComplexTypesRegisteredInDI() + { + // Arrange + var actionName = nameof(ParameterBindingController.IEnumerableServiceParameter); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + // Using any built-in type defined in the Test action + var serviceProvider = Mock.Of(s => s.IsService(typeof(IApplicationModelProvider)) == true); + var convention = GetConvention(serviceProviderIsService: serviceProvider); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.True(convention.IsInferForServiceParametersEnabled); + Assert.Same(BindingSource.Services, result); + } + [Fact] public void PreservesBindingSourceInference_ForFromQueryParameter_WithDefaultName() { @@ -982,9 +1030,15 @@ private class ParameterBindingController public IActionResult CollectionOfSimpleTypes(IList parameter) => null; + public IActionResult IEnumerableOfSimpleTypes(IEnumerable parameter) => null; + public IActionResult CollectionOfComplexTypes(IList parameter) => null; + public IActionResult IEnumerableOfComplexTypes(IEnumerable parameter) => null; + public IActionResult ServiceParameter(IApplicationModelProvider parameter) => null; + + public IActionResult IEnumerableServiceParameter(IEnumerable parameter) => null; } [ApiController] diff --git a/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs b/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs index 916cc4b6ba2e..bf0d81cbdf9a 100644 --- a/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs +++ b/src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs @@ -80,7 +80,7 @@ public HubMethodDescriptor(ObjectMethodExecutor methodExecutor, IServiceProvider return false; } else if (p.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)) || - serviceProviderIsService?.IsService(p.ParameterType) == true) + serviceProviderIsService?.IsService(GetServiceType(p.ParameterType)) == true) { if (index >= 64) { @@ -160,4 +160,18 @@ private static Func> Compile var lambda = Expression.Lambda>>(methodCall, parameters); return lambda.Compile(); } + + private static Type GetServiceType(Type type) + { + // IServiceProviderIsService will special case IEnumerable<> and always return true + // so, in this case checking the element type instead + if (type.IsConstructedGenericType && + type.GetGenericTypeDefinition() is Type genericDefinition && + genericDefinition == typeof(IEnumerable<>)) + { + return type.GenericTypeArguments[0]; + } + + return type; + } } diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs index b49e22583229..53e0915d16e1 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs @@ -1372,6 +1372,11 @@ public int ServiceWithAndWithoutAttribute(Service1 service, [FromService] Servic return 1; } + public int IEnumerableOfServiceWithoutAttribute(IEnumerable services) + { + return 1; + } + public async Task Stream(ChannelReader channelReader) { while (await channelReader.WaitToReadAsync()) diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs index 32af4cfe6246..b3724d513306 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs @@ -4749,6 +4749,27 @@ public async Task ServiceResolvedWithoutAttribute() } } + [Fact] + public async Task ServiceResolvedForIEnumerableParameter() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(provider => + { + provider.AddSignalR(options => + { + options.EnableDetailedErrors = true; + }); + provider.AddSingleton(); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + var res = await client.InvokeAsync(nameof(ServicesHub.IEnumerableOfServiceWithoutAttribute)).DefaultTimeout(); + Assert.Equal(1L, res.Result); + } + } + [Fact] public async Task ServiceResolvedWithoutAttribute_WithHubSpecificSettingEnabled() { @@ -4839,6 +4860,26 @@ public async Task ServiceNotResolvedIfNotInDI() } } + [Fact] + public async Task ServiceNotResolvedForIEnumerableParameterIfNotInDI() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(provider => + { + provider.AddSignalR(options => + { + options.EnableDetailedErrors = true; + }); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + var res = await client.InvokeAsync(nameof(ServicesHub.IEnumerableOfServiceWithoutAttribute)).DefaultTimeout(); + Assert.Equal("Failed to invoke 'IEnumerableOfServiceWithoutAttribute' due to an error on the server. InvalidDataException: Invocation provides 0 argument(s) but target expects 1.", res.Error); + } + } + [Fact] public void TooManyParametersWithServiceThrows() {