diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index 7f9e1d99a79e..bbcf91a97eda 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -474,6 +475,15 @@ internal IReadOnlyDictionary BoundConstructorPrope /// public Type UnderlyingOrModelType { get; private set; } = default!; + /// + /// Gets a value indicating the NullabilityState of the value or reference type. + /// + /// + /// The state will be set for Parameters and Properties + /// otherwise the state will be NullabilityState.Unknown + /// + internal NullabilityState NullabilityState { get; set; } + /// /// Gets a property getter delegate to get the property value from a model object. /// @@ -614,6 +624,15 @@ private void InitializeTypeInformation() var collectionType = ClosedGenericMatcher.ExtractGenericInterface(ModelType, typeof(ICollection<>)); IsCollectionType = collectionType != null; + var nullabilityContext = new NullabilityInfoContext(); + var nullability = MetadataKind switch + { + ModelMetadataKind.Parameter => Identity.ParameterInfo != null ? nullabilityContext.Create(Identity.ParameterInfo!) : null, + ModelMetadataKind.Property => Identity.PropertyInfo != null ? nullabilityContext.Create(Identity.PropertyInfo!) : null, + _ => null + }; + NullabilityState = nullability?.ReadState ?? NullabilityState.Unknown; + if (ModelType == typeof(string) || !typeof(IEnumerable).IsAssignableFrom(ModelType)) { // Do nothing, not Enumerable. diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs index 6539f98754f8..d9ce45170d66 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs @@ -14,6 +14,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; /// public class ServicesModelBinder : IModelBinder { + internal bool IsOptional { get; set; } + /// public Task BindModelAsync(ModelBindingContext bindingContext) { @@ -23,9 +25,14 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } var requestServices = bindingContext.HttpContext.RequestServices; - var model = requestServices.GetRequiredService(bindingContext.ModelType); + var model = IsOptional ? + requestServices.GetService(bindingContext.ModelType) : + requestServices.GetRequiredService(bindingContext.ModelType); - bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true }); + if (model != null) + { + bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true }); + } bindingContext.Result = ModelBindingResult.Success(model); return Task.CompletedTask; diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs index b630c8adb951..92d06f51414c 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs @@ -3,6 +3,8 @@ #nullable enable +using System.Reflection; + namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; /// @@ -10,9 +12,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; /// public class ServicesModelBinderProvider : IModelBinderProvider { - // ServicesModelBinder does not have any state. Re-use the same instance for binding. - - private readonly ServicesModelBinder _modelBinder = new ServicesModelBinder(); + private readonly ServicesModelBinder _optionalServicesBinder = new() { IsOptional = true }; + private readonly ServicesModelBinder _servicesBinder = new(); /// public IModelBinder? GetBinder(ModelBinderProviderContext context) @@ -25,7 +26,15 @@ public class ServicesModelBinderProvider : IModelBinderProvider if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services)) { - return _modelBinder; + // IsRequired will be false for a Reference Type + // without a default value in a oblivious nullability context + // however, for services we shoud treat them as required + var isRequired = context.Metadata.IsRequired || + (context.Metadata.Identity.ParameterInfo?.HasDefaultValue != true && + !context.Metadata.ModelType.IsValueType && + context.Metadata.NullabilityState == NullabilityState.Unknown); + + return isRequired ? _servicesBinder : _optionalServicesBinder; } return null; diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs index d35b46a7de6e..3bb6b2296d41 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs @@ -1,6 +1,8 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Reflection; + namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; public class ServicesModelBinderProviderTest @@ -51,7 +53,66 @@ public void Create_WhenBindingSourceIsFromServices_ReturnsBinder() Assert.IsType(result); } + [Theory] + [MemberData(nameof(ParameterInfoData))] + public void Create_WhenBindingSourceIsNullableFromServices_ReturnsBinder(ParameterInfo parameterInfo, bool isOptional) + { + // Arrange + var provider = new ServicesModelBinderProvider(); + + var context = new TestModelBinderProviderContext(parameterInfo); + + // Act + var result = provider.GetBinder(context); + + // Assert + var binder = Assert.IsType(result); + Assert.Equal(isOptional, binder.IsOptional); + } + private class IPersonService { } + + public static TheoryData ParameterInfoData() + { + return new TheoryData() + { + { ParameterInfos.NullableParameterInfo, true }, + { ParameterInfos.DefaultValueParameterInfo, true }, + { ParameterInfos.NonNullabilityContextParameterInfo, false }, + { ParameterInfos.NonNullableParameterInfo, false }, + }; + } + + private class ParameterInfos + { +#nullable disable + public void TestMethod([FromServices] IPersonService param1, [FromServices] IPersonService param2 = null) + { } +#nullable restore + +#nullable enable + public void TestMethod2([FromServices] IPersonService param1, [FromServices] IPersonService? param2) + { } +#nullable restore + + public static ParameterInfo NonNullableParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod2)) + .GetParameters()[0]; + public static ParameterInfo NullableParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod2)) + .GetParameters()[1]; + + public static ParameterInfo NonNullabilityContextParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod)) + .GetParameters()[0]; + public static ParameterInfo DefaultValueParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod)) + .GetParameters()[1]; + } } diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs index 2c26818dea88..8dc685be93e3 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs @@ -1,6 +1,7 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Reflection; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -38,6 +39,21 @@ public TestModelBinderProviderContext(Type modelType, BindingInfo bindingInfo) (Services, MvcOptions) = GetServicesAndOptions(); } + public TestModelBinderProviderContext(ParameterInfo parameterInfo) + { + Metadata = CachedMetadataProvider.GetMetadataForParameter(parameterInfo); + MetadataProvider = CachedMetadataProvider; + _bindingInfo = new BindingInfo + { + BinderModelName = Metadata.BinderModelName, + BinderType = Metadata.BinderType, + BindingSource = Metadata.BindingSource, + PropertyFilterProvider = Metadata.PropertyFilterProvider, + }; + + (Services, MvcOptions) = GetServicesAndOptions(); + } + public override BindingInfo BindingInfo => _bindingInfo; public override ModelMetadata Metadata { get; } diff --git a/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs b/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs index 0c67a6eb59f4..3dfb12f23e73 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -180,6 +181,155 @@ public async Task BindParameterFromService_NoService_Throws() Assert.Contains(typeof(IActionResult).FullName, exception.Message); } + private class TestController + { +#nullable enable + public void Action(IActionResult? service, ITypeActivatorCache? service2) + { } +#nullable restore + + public void ActionWithDefaultValue(IActionResult service = default, ITypeActivatorCache service2 = default) + { } + } + + [Fact] + public async Task BindNullableParameterFromService_WithData_GetBounds() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameters = typeof(TestController).GetMethod(nameof(TestController.Action)).GetParameters(); + var parameter = new ControllerParameterDescriptor + { + Name = "ControllerProperty", + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Services, + }, + ParameterInfo = parameters[1], + // Use a service type already in defaults. + ParameterType = typeof(ITypeActivatorCache), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Model + var provider = Assert.IsAssignableFrom(modelBindingResult.Model); + Assert.NotNull(provider); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Empty(modelState.Keys); + } + + [Fact] + public async Task BindNullableParameterFromService_NoService_BindsToNull() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameters = typeof(TestController).GetMethod(nameof(TestController.Action)).GetParameters(); + var parameter = new ControllerParameterDescriptor + { + Name = "ControllerProperty", + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Services, + }, + ParameterInfo = parameters[0], + // Use a service type not available in DI. + ParameterType = typeof(IActionResult), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + Assert.Null(modelBindingResult.Model); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + + [Fact] + public async Task BindParameterWithDefaultValueFromService_WithData_GetBounds() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameters = typeof(TestController).GetMethod(nameof(TestController.ActionWithDefaultValue)).GetParameters(); + var parameter = new ControllerParameterDescriptor + { + Name = "ControllerProperty", + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Services, + }, + ParameterInfo = parameters[1], + // Use a service type already in defaults. + ParameterType = typeof(ITypeActivatorCache), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Model + var provider = Assert.IsAssignableFrom(modelBindingResult.Model); + Assert.NotNull(provider); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Empty(modelState.Keys); + } + + [Fact] + public async Task BindParameterWithDefaultValueFromService_NoService_BindsToDefaultValue() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameters = typeof(TestController).GetMethod(nameof(TestController.ActionWithDefaultValue)).GetParameters(); + var parameter = new ControllerParameterDescriptor + { + Name = "ControllerProperty", + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Services, + }, + ParameterInfo = parameters[0], + // Use a service type not available in DI. + ParameterType = typeof(IActionResult), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + Assert.Null(modelBindingResult.Model); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + private class Person { public ITypeActivatorCache Service { get; set; }