Skip to content

Commit b89eba6

Browse files
brunolins16pranavkm
brunolins16
and
pranavkm
authored
Inferring FromServices optionality (#39804)
* Inferring FromServices optionality * Using IsRequired * Adding integration tests * Update src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs Co-authored-by: Pranav K <[email protected]> * Update src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs Co-authored-by: Pranav K <[email protected]> * Update src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs Co-authored-by: Pranav K <[email protected]> * Update src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs Co-authored-by: Pranav K <[email protected]> * Fixing unit test * Apply suggestions from code review Co-authored-by: Pranav K <[email protected]> Co-authored-by: Pranav K <[email protected]>
1 parent 32fb3a6 commit b89eba6

File tree

6 files changed

+270
-8
lines changed

6 files changed

+270
-8
lines changed

‎src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics;
77
using System.Diagnostics.CodeAnalysis;
88
using System.Linq;
9+
using System.Reflection;
910
using Microsoft.AspNetCore.Mvc.Abstractions;
1011
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
1112
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
@@ -474,6 +475,15 @@ internal IReadOnlyDictionary<ModelMetadata, ModelMetadata> BoundConstructorPrope
474475
/// </remarks>
475476
public Type UnderlyingOrModelType { get; private set; } = default!;
476477

478+
/// <summary>
479+
/// Gets a value indicating the NullabilityState of the value or reference type.
480+
/// </summary>
481+
/// <remarks>
482+
/// The state will be set for Parameters and Properties <see cref="ModelMetadataKind"/>
483+
/// otherwise the state will be <c>NullabilityState.Unknown</c>
484+
/// </remarks>
485+
internal NullabilityState NullabilityState { get; set; }
486+
477487
/// <summary>
478488
/// Gets a property getter delegate to get the property value from a model object.
479489
/// </summary>
@@ -614,6 +624,15 @@ private void InitializeTypeInformation()
614624
var collectionType = ClosedGenericMatcher.ExtractGenericInterface(ModelType, typeof(ICollection<>));
615625
IsCollectionType = collectionType != null;
616626

627+
var nullabilityContext = new NullabilityInfoContext();
628+
var nullability = MetadataKind switch
629+
{
630+
ModelMetadataKind.Parameter => Identity.ParameterInfo != null ? nullabilityContext.Create(Identity.ParameterInfo!) : null,
631+
ModelMetadataKind.Property => Identity.PropertyInfo != null ? nullabilityContext.Create(Identity.PropertyInfo!) : null,
632+
_ => null
633+
};
634+
NullabilityState = nullability?.ReadState ?? NullabilityState.Unknown;
635+
617636
if (ModelType == typeof(string) || !typeof(IEnumerable).IsAssignableFrom(ModelType))
618637
{
619638
// Do nothing, not Enumerable.

‎src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinder.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
1414
/// </summary>
1515
public class ServicesModelBinder : IModelBinder
1616
{
17+
internal bool IsOptional { get; set; }
18+
1719
/// <inheritdoc />
1820
public Task BindModelAsync(ModelBindingContext bindingContext)
1921
{
@@ -23,9 +25,14 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
2325
}
2426

2527
var requestServices = bindingContext.HttpContext.RequestServices;
26-
var model = requestServices.GetRequiredService(bindingContext.ModelType);
28+
var model = IsOptional ?
29+
requestServices.GetService(bindingContext.ModelType) :
30+
requestServices.GetRequiredService(bindingContext.ModelType);
2731

28-
bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true });
32+
if (model != null)
33+
{
34+
bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true });
35+
}
2936

3037
bindingContext.Result = ModelBindingResult.Success(model);
3138
return Task.CompletedTask;

‎src/Mvc/Mvc.Core/src/ModelBinding/Binders/ServicesModelBinderProvider.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@
33

44
#nullable enable
55

6+
using System.Reflection;
7+
68
namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
79

810
/// <summary>
911
/// An <see cref="IModelBinderProvider"/> for binding from the <see cref="IServiceProvider"/>.
1012
/// </summary>
1113
public class ServicesModelBinderProvider : IModelBinderProvider
1214
{
13-
// ServicesModelBinder does not have any state. Re-use the same instance for binding.
14-
15-
private readonly ServicesModelBinder _modelBinder = new ServicesModelBinder();
15+
private readonly ServicesModelBinder _optionalServicesBinder = new() { IsOptional = true };
16+
private readonly ServicesModelBinder _servicesBinder = new();
1617

1718
/// <inheritdoc />
1819
public IModelBinder? GetBinder(ModelBinderProviderContext context)
@@ -25,7 +26,15 @@ public class ServicesModelBinderProvider : IModelBinderProvider
2526
if (context.BindingInfo.BindingSource != null &&
2627
context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services))
2728
{
28-
return _modelBinder;
29+
// IsRequired will be false for a Reference Type
30+
// without a default value in a oblivious nullability context
31+
// however, for services we shoud treat them as required
32+
var isRequired = context.Metadata.IsRequired ||
33+
(context.Metadata.Identity.ParameterInfo?.HasDefaultValue != true &&
34+
!context.Metadata.ModelType.IsValueType &&
35+
context.Metadata.NullabilityState == NullabilityState.Unknown);
36+
37+
return isRequired ? _servicesBinder : _optionalServicesBinder;
2938
}
3039

3140
return null;

‎src/Mvc/Mvc.Core/test/ModelBinding/Binders/ServicesModelBinderProviderTest.cs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Reflection;
5+
46
namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
57

68
public class ServicesModelBinderProviderTest
@@ -51,7 +53,66 @@ public void Create_WhenBindingSourceIsFromServices_ReturnsBinder()
5153
Assert.IsType<ServicesModelBinder>(result);
5254
}
5355

56+
[Theory]
57+
[MemberData(nameof(ParameterInfoData))]
58+
public void Create_WhenBindingSourceIsNullableFromServices_ReturnsBinder(ParameterInfo parameterInfo, bool isOptional)
59+
{
60+
// Arrange
61+
var provider = new ServicesModelBinderProvider();
62+
63+
var context = new TestModelBinderProviderContext(parameterInfo);
64+
65+
// Act
66+
var result = provider.GetBinder(context);
67+
68+
// Assert
69+
var binder = Assert.IsType<ServicesModelBinder>(result);
70+
Assert.Equal(isOptional, binder.IsOptional);
71+
}
72+
5473
private class IPersonService
5574
{
5675
}
76+
77+
public static TheoryData<ParameterInfo, bool> ParameterInfoData()
78+
{
79+
return new TheoryData<ParameterInfo, bool>()
80+
{
81+
{ ParameterInfos.NullableParameterInfo, true },
82+
{ ParameterInfos.DefaultValueParameterInfo, true },
83+
{ ParameterInfos.NonNullabilityContextParameterInfo, false },
84+
{ ParameterInfos.NonNullableParameterInfo, false },
85+
};
86+
}
87+
88+
private class ParameterInfos
89+
{
90+
#nullable disable
91+
public void TestMethod([FromServices] IPersonService param1, [FromServices] IPersonService param2 = null)
92+
{ }
93+
#nullable restore
94+
95+
#nullable enable
96+
public void TestMethod2([FromServices] IPersonService param1, [FromServices] IPersonService? param2)
97+
{ }
98+
#nullable restore
99+
100+
public static ParameterInfo NonNullableParameterInfo
101+
= typeof(ParameterInfos)
102+
.GetMethod(nameof(ParameterInfos.TestMethod2))
103+
.GetParameters()[0];
104+
public static ParameterInfo NullableParameterInfo
105+
= typeof(ParameterInfos)
106+
.GetMethod(nameof(ParameterInfos.TestMethod2))
107+
.GetParameters()[1];
108+
109+
public static ParameterInfo NonNullabilityContextParameterInfo
110+
= typeof(ParameterInfos)
111+
.GetMethod(nameof(ParameterInfos.TestMethod))
112+
.GetParameters()[0];
113+
public static ParameterInfo DefaultValueParameterInfo
114+
= typeof(ParameterInfos)
115+
.GetMethod(nameof(ParameterInfos.TestMethod))
116+
.GetParameters()[1];
117+
}
57118
}

‎src/Mvc/Mvc.Core/test/ModelBinding/TestModelBinderProviderContext.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Reflection;
45
using Microsoft.Extensions.DependencyInjection;
56
using Microsoft.Extensions.Logging;
67
using Microsoft.Extensions.Logging.Abstractions;
@@ -38,6 +39,21 @@ public TestModelBinderProviderContext(Type modelType, BindingInfo bindingInfo)
3839
(Services, MvcOptions) = GetServicesAndOptions();
3940
}
4041

42+
public TestModelBinderProviderContext(ParameterInfo parameterInfo)
43+
{
44+
Metadata = CachedMetadataProvider.GetMetadataForParameter(parameterInfo);
45+
MetadataProvider = CachedMetadataProvider;
46+
_bindingInfo = new BindingInfo
47+
{
48+
BinderModelName = Metadata.BinderModelName,
49+
BinderType = Metadata.BinderType,
50+
BindingSource = Metadata.BindingSource,
51+
PropertyFilterProvider = Metadata.PropertyFilterProvider,
52+
};
53+
54+
(Services, MvcOptions) = GetServicesAndOptions();
55+
}
56+
4157
public override BindingInfo BindingInfo => _bindingInfo;
4258

4359
public override ModelMetadata Metadata { get; }

‎src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using Microsoft.AspNetCore.Mvc.Abstractions;
5+
using Microsoft.AspNetCore.Mvc.Controllers;
56
using Microsoft.AspNetCore.Mvc.Infrastructure;
67
using Microsoft.AspNetCore.Mvc.ModelBinding;
78

@@ -180,6 +181,155 @@ public async Task BindParameterFromService_NoService_Throws()
180181
Assert.Contains(typeof(IActionResult).FullName, exception.Message);
181182
}
182183

184+
private class TestController
185+
{
186+
#nullable enable
187+
public void Action(IActionResult? service, ITypeActivatorCache? service2)
188+
{ }
189+
#nullable restore
190+
191+
public void ActionWithDefaultValue(IActionResult service = default, ITypeActivatorCache service2 = default)
192+
{ }
193+
}
194+
195+
[Fact]
196+
public async Task BindNullableParameterFromService_WithData_GetBounds()
197+
{
198+
// Arrange
199+
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
200+
var parameters = typeof(TestController).GetMethod(nameof(TestController.Action)).GetParameters();
201+
var parameter = new ControllerParameterDescriptor
202+
{
203+
Name = "ControllerProperty",
204+
BindingInfo = new BindingInfo
205+
{
206+
BindingSource = BindingSource.Services,
207+
},
208+
ParameterInfo = parameters[1],
209+
// Use a service type already in defaults.
210+
ParameterType = typeof(ITypeActivatorCache),
211+
};
212+
213+
var testContext = ModelBindingTestHelper.GetTestContext();
214+
var modelState = testContext.ModelState;
215+
216+
// Act
217+
var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
218+
219+
// Model
220+
var provider = Assert.IsAssignableFrom<ITypeActivatorCache>(modelBindingResult.Model);
221+
Assert.NotNull(provider);
222+
223+
// ModelState
224+
Assert.True(modelState.IsValid);
225+
Assert.Empty(modelState.Keys);
226+
}
227+
228+
[Fact]
229+
public async Task BindNullableParameterFromService_NoService_BindsToNull()
230+
{
231+
// Arrange
232+
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
233+
var parameters = typeof(TestController).GetMethod(nameof(TestController.Action)).GetParameters();
234+
var parameter = new ControllerParameterDescriptor
235+
{
236+
Name = "ControllerProperty",
237+
BindingInfo = new BindingInfo
238+
{
239+
BindingSource = BindingSource.Services,
240+
},
241+
ParameterInfo = parameters[0],
242+
// Use a service type not available in DI.
243+
ParameterType = typeof(IActionResult),
244+
};
245+
246+
var testContext = ModelBindingTestHelper.GetTestContext();
247+
var modelState = testContext.ModelState;
248+
249+
// Act
250+
var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
251+
252+
// Assert
253+
// ModelBindingResult
254+
Assert.True(modelBindingResult.IsModelSet);
255+
256+
// Model
257+
Assert.Null(modelBindingResult.Model);
258+
259+
// ModelState
260+
Assert.True(modelState.IsValid);
261+
Assert.Empty(modelState);
262+
}
263+
264+
[Fact]
265+
public async Task BindParameterWithDefaultValueFromService_WithData_GetBounds()
266+
{
267+
// Arrange
268+
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
269+
var parameters = typeof(TestController).GetMethod(nameof(TestController.ActionWithDefaultValue)).GetParameters();
270+
var parameter = new ControllerParameterDescriptor
271+
{
272+
Name = "ControllerProperty",
273+
BindingInfo = new BindingInfo
274+
{
275+
BindingSource = BindingSource.Services,
276+
},
277+
ParameterInfo = parameters[1],
278+
// Use a service type already in defaults.
279+
ParameterType = typeof(ITypeActivatorCache),
280+
};
281+
282+
var testContext = ModelBindingTestHelper.GetTestContext();
283+
var modelState = testContext.ModelState;
284+
285+
// Act
286+
var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
287+
288+
// Model
289+
var provider = Assert.IsAssignableFrom<ITypeActivatorCache>(modelBindingResult.Model);
290+
Assert.NotNull(provider);
291+
292+
// ModelState
293+
Assert.True(modelState.IsValid);
294+
Assert.Empty(modelState.Keys);
295+
}
296+
297+
[Fact]
298+
public async Task BindParameterWithDefaultValueFromService_NoService_BindsToDefaultValue()
299+
{
300+
// Arrange
301+
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
302+
var parameters = typeof(TestController).GetMethod(nameof(TestController.ActionWithDefaultValue)).GetParameters();
303+
var parameter = new ControllerParameterDescriptor
304+
{
305+
Name = "ControllerProperty",
306+
BindingInfo = new BindingInfo
307+
{
308+
BindingSource = BindingSource.Services,
309+
},
310+
ParameterInfo = parameters[0],
311+
// Use a service type not available in DI.
312+
ParameterType = typeof(IActionResult),
313+
};
314+
315+
var testContext = ModelBindingTestHelper.GetTestContext();
316+
var modelState = testContext.ModelState;
317+
318+
// Act
319+
var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
320+
321+
// Assert
322+
// ModelBindingResult
323+
Assert.True(modelBindingResult.IsModelSet);
324+
325+
// Model
326+
Assert.Null(modelBindingResult.Model);
327+
328+
// ModelState
329+
Assert.True(modelState.IsValid);
330+
Assert.Empty(modelState);
331+
}
332+
183333
private class Person
184334
{
185335
public ITypeActivatorCache Service { get; set; }

0 commit comments

Comments
 (0)