From 8c701ab84236a630fc335115ca47709371c7d74a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jan 2022 22:44:11 -0800 Subject: [PATCH 1/9] Inferring FromServices optionality --- .../Binders/ServicesModelBinder.cs | 11 +++- .../Binders/ServicesModelBinderProvider.cs | 14 +++-- .../ServicesModelBinderProviderTest.cs | 56 ++++++++++++++++++- .../TestModelBinderProviderContext.cs | 18 +++++- 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs index 6539f98754f8..f719b8ecdcfa 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 IsOptionalParameter { 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 = IsOptionalParameter ? + 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 e72eb38e0cdd..43d706305fbc 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs @@ -3,6 +3,7 @@ #nullable enable +using System.Reflection; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; @@ -11,9 +12,7 @@ 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 NullabilityInfoContext _nullabilityContext = new(); /// public IModelBinder? GetBinder(ModelBinderProviderContext context) @@ -26,9 +25,16 @@ public class ServicesModelBinderProvider : IModelBinderProvider if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services)) { - return _modelBinder; + return new ServicesModelBinder() + { + IsOptionalParameter = IsOptionalParameter(context.Metadata.Identity.ParameterInfo!) + }; } return null; } + + internal bool IsOptionalParameter(ParameterInfo parameterInfo) => + parameterInfo.HasDefaultValue || + _nullabilityContext.Create(parameterInfo).ReadState == NullabilityState.Nullable; } diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs index d35b46a7de6e..6023abd8231a 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,59 @@ 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.IsOptionalParameter); + } + private class IPersonService { } + + public static TheoryData ParameterInfoData() + { + return new TheoryData() + { + { ParameterInfos.NullableParameterInfo, true }, + { ParameterInfos.DefaultValueParameterInfo, true }, + { ParameterInfos.NonNullableParameterInfo, false }, + }; + } + + private class ParameterInfos + { + public void TestMethod([FromServices] IPersonService param1, [FromServices] IPersonService param2 = null) + { } + +#nullable enable + public void TestMethod2([FromServices] IPersonService? param2) + { } +#nullable restore + + public static ParameterInfo NullableParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod2)) + .GetParameters()[0]; + public static ParameterInfo NonNullableParameterInfo + = 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..40b55dba1365 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs @@ -1,10 +1,11 @@ -// 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 Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using System.Reflection; namespace Microsoft.AspNetCore.Mvc.ModelBinding; @@ -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; } From 4a66bcc55482df8f497dd48061a1e0cfa14ae0e0 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 27 Jan 2022 16:49:39 -0800 Subject: [PATCH 2/9] Using IsRequired --- .../src/ModelBinding/ModelMetadata.cs | 19 ++++++++++++++++++ .../Binders/ServicesModelBinder.cs | 4 ++-- .../Binders/ServicesModelBinderProvider.cs | 20 ++++++++++--------- .../ServicesModelBinderProviderTest.cs | 17 ++++++++++++---- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index 7f9e1d99a79e..6166ac4ee434 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 => nullabilityContext.Create(Identity.ParameterInfo!), + ModelMetadataKind.Property => nullabilityContext.Create(Identity.PropertyInfo!), + _ => 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 f719b8ecdcfa..d9ce45170d66 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; /// public class ServicesModelBinder : IModelBinder { - internal bool IsOptionalParameter { get; set; } + internal bool IsOptional { get; set; } /// public Task BindModelAsync(ModelBindingContext bindingContext) @@ -25,7 +25,7 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } var requestServices = bindingContext.HttpContext.RequestServices; - var model = IsOptionalParameter ? + var model = IsOptional ? requestServices.GetService(bindingContext.ModelType) : requestServices.GetRequiredService(bindingContext.ModelType); diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs index 43d706305fbc..bd419175e91b 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs @@ -12,7 +12,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; /// public class ServicesModelBinderProvider : IModelBinderProvider { - private readonly NullabilityInfoContext _nullabilityContext = new(); + private readonly ServicesModelBinder _optionalServicesBinder = new() { IsOptional = true }; + private readonly ServicesModelBinder _servicesBinder = new(); /// public IModelBinder? GetBinder(ModelBinderProviderContext context) @@ -25,16 +26,17 @@ public class ServicesModelBinderProvider : IModelBinderProvider if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services)) { - return new ServicesModelBinder() - { - IsOptionalParameter = IsOptionalParameter(context.Metadata.Identity.ParameterInfo!) - }; + // IsRequired will be false when 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; } - - internal bool IsOptionalParameter(ParameterInfo parameterInfo) => - parameterInfo.HasDefaultValue || - _nullabilityContext.Create(parameterInfo).ReadState == NullabilityState.Nullable; } diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs index 6023abd8231a..a4dc3213a743 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs @@ -67,7 +67,7 @@ public void Create_WhenBindingSourceIsNullableFromServices_ReturnsBinder(Paramet // Assert var binder = Assert.IsType(result); - Assert.Equal(isOptional, binder.IsOptionalParameter); + Assert.Equal(isOptional, binder.IsOptional); } private class IPersonService @@ -80,25 +80,34 @@ public static TheoryData ParameterInfoData() { { 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? param2) + public void TestMethod2([FromServices] IPersonService param1, [FromServices] IPersonService? param2) { } #nullable restore - public static ParameterInfo NullableParameterInfo + public static ParameterInfo NonNullableParameterInfo = typeof(ParameterInfos) .GetMethod(nameof(ParameterInfos.TestMethod2)) .GetParameters()[0]; - public static ParameterInfo NonNullableParameterInfo + public static ParameterInfo NullableParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.TestMethod2)) + .GetParameters()[1]; + + + public static ParameterInfo NonNullabilityContextParameterInfo = typeof(ParameterInfos) .GetMethod(nameof(ParameterInfos.TestMethod)) .GetParameters()[0]; From 5f0c160a0a18a1c4b1587c49f6a8d4939136b446 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 14:21:28 -0800 Subject: [PATCH 3/9] Adding integration tests --- .../ServicesModelBinderIntegrationTest.cs | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) 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; } From d1c4680e870a3f80e86f7e4b8ea793abd7b5b08c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 14:48:24 -0800 Subject: [PATCH 4/9] Update src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs Co-authored-by: Pranav K --- .../src/ModelBinding/Binders/ServicesModelBinderProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs index bd419175e91b..92d06f51414c 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs @@ -26,7 +26,7 @@ public class ServicesModelBinderProvider : IModelBinderProvider if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services)) { - // IsRequired will be false when a Reference Type + // 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 || From 3e298f31989e9268b870cf6698f8768302e3ab78 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 14:48:41 -0800 Subject: [PATCH 5/9] Update src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs Co-authored-by: Pranav K --- .../Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs index 40b55dba1365..52c35b37ddbc 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. // 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; From 723118551fa9df5a07591c8d6e1c0699354cc9c4 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 14:49:35 -0800 Subject: [PATCH 6/9] Update src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs Co-authored-by: Pranav K --- .../test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs index a4dc3213a743..20063e411fff 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs @@ -106,7 +106,6 @@ public static ParameterInfo NullableParameterInfo .GetMethod(nameof(ParameterInfos.TestMethod2)) .GetParameters()[1]; - public static ParameterInfo NonNullabilityContextParameterInfo = typeof(ParameterInfos) .GetMethod(nameof(ParameterInfos.TestMethod)) From 3eadb0e8840939e2e78e9789562d655708ebfda7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 14:50:30 -0800 Subject: [PATCH 7/9] Update src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs Co-authored-by: Pranav K --- .../Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs index 52c35b37ddbc..8dc685be93e3 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs @@ -6,7 +6,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using System.Reflection; namespace Microsoft.AspNetCore.Mvc.ModelBinding; From 14b53d747656be539947b161d57544b1ab247385 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 11:01:20 -0800 Subject: [PATCH 8/9] Fixing unit test --- src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index 6166ac4ee434..bbcf91a97eda 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -627,8 +627,8 @@ private void InitializeTypeInformation() var nullabilityContext = new NullabilityInfoContext(); var nullability = MetadataKind switch { - ModelMetadataKind.Parameter => nullabilityContext.Create(Identity.ParameterInfo!), - ModelMetadataKind.Property => nullabilityContext.Create(Identity.PropertyInfo!), + 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; From 69a9fd82a5ccfc6e47163be48424a05bc84bf1b9 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 11:23:01 -0800 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Pranav K --- .../Binders/ServicesModelBinderProviderTest.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs index 20063e411fff..3bb6b2296d41 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs @@ -77,12 +77,12 @@ private class IPersonService public static TheoryData ParameterInfoData() { return new TheoryData() - { - { ParameterInfos.NullableParameterInfo, true }, - { ParameterInfos.DefaultValueParameterInfo, true }, - { ParameterInfos.NonNullabilityContextParameterInfo, false }, - { ParameterInfos.NonNullableParameterInfo, false }, - }; + { + { ParameterInfos.NullableParameterInfo, true }, + { ParameterInfos.DefaultValueParameterInfo, true }, + { ParameterInfos.NonNullabilityContextParameterInfo, false }, + { ParameterInfos.NonNullableParameterInfo, false }, + }; } private class ParameterInfos @@ -114,6 +114,5 @@ public static ParameterInfo DefaultValueParameterInfo = typeof(ParameterInfos) .GetMethod(nameof(ParameterInfos.TestMethod)) .GetParameters()[1]; - } }