Skip to content

Commit cdae83e

Browse files
authored
Use the declared type to infer NullableContextOptions (#15134)
* Use the declared type to infer NullableContextOptions Prior to this change MVC used the runtime type rather than the declared type to determine the nullability of a property based on state. In the case of inheritance, this can be erroneous since the declared type may have a different nullability context than the model type. This change addresses this by adding content to `ModelIdentity` that allows inspecting the declared type. * Add an overload to `ModelMetadataIdentity` that allows flowing `PropertyInfo` * Use the declared type in `DataAnnotationsMetadataProvider` to determine nullability based on context. Fixes #14812
1 parent 0de5791 commit cdae83e

13 files changed

+446
-103
lines changed

src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.netcoreapp.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,10 +883,13 @@ protected ModelBindingMessageProvider() { }
883883
public System.Type ModelType { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
884884
public string Name { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
885885
public System.Reflection.ParameterInfo ParameterInfo { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
886+
public System.Reflection.PropertyInfo PropertyInfo { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
886887
public bool Equals(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity other) { throw null; }
887888
public override bool Equals(object obj) { throw null; }
888889
public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForParameter(System.Reflection.ParameterInfo parameter) { throw null; }
889890
public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForParameter(System.Reflection.ParameterInfo parameter, System.Type modelType) { throw null; }
891+
public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForProperty(System.Reflection.PropertyInfo propertyInfo, System.Type modelType, System.Type containerType) { throw null; }
892+
[System.ObsoleteAttribute("This API is obsolete and may be removed in a future release.")]
890893
public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForProperty(System.Type modelType, string name, System.Type containerType) { throw null; }
891894
public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForType(System.Type modelType) { throw null; }
892895
public override int GetHashCode() { throw null; }

src/Mvc/Mvc.Abstractions/src/ModelBinding/Metadata/ModelMetadataIdentity.cs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ private ModelMetadataIdentity(
1717
Type modelType,
1818
string name = null,
1919
Type containerType = null,
20-
ParameterInfo parameterInfo = null)
20+
ParameterInfo parameterInfo = null,
21+
PropertyInfo propertyInfo = null)
2122
{
2223
ModelType = modelType;
2324
Name = name;
2425
ContainerType = containerType;
2526
ParameterInfo = parameterInfo;
27+
PropertyInfo = propertyInfo;
2628
}
2729

2830
/// <summary>
@@ -47,6 +49,7 @@ public static ModelMetadataIdentity ForType(Type modelType)
4749
/// <param name="name">The name of the property.</param>
4850
/// <param name="containerType">The container type of the model property.</param>
4951
/// <returns>A <see cref="ModelMetadataIdentity"/>.</returns>
52+
[Obsolete("This API is obsolete and may be removed in a future release.")]
5053
public static ModelMetadataIdentity ForProperty(
5154
Type modelType,
5255
string name,
@@ -70,6 +73,36 @@ public static ModelMetadataIdentity ForProperty(
7073
return new ModelMetadataIdentity(modelType, name, containerType);
7174
}
7275

76+
/// <summary>
77+
/// Creates a <see cref="ModelMetadataIdentity"/> for the provided property.
78+
/// </summary>
79+
/// <param name="modelType">The model type.</param>
80+
/// <param name="propertyInfo">The property.</param>
81+
/// <param name="containerType">The container type of the model property.</param>
82+
/// <returns>A <see cref="ModelMetadataIdentity"/>.</returns>
83+
public static ModelMetadataIdentity ForProperty(
84+
PropertyInfo propertyInfo,
85+
Type modelType,
86+
Type containerType)
87+
{
88+
if (propertyInfo == null)
89+
{
90+
throw new ArgumentNullException(nameof(propertyInfo));
91+
}
92+
93+
if (modelType == null)
94+
{
95+
throw new ArgumentNullException(nameof(modelType));
96+
}
97+
98+
if (containerType == null)
99+
{
100+
throw new ArgumentNullException(nameof(containerType));
101+
}
102+
103+
return new ModelMetadataIdentity(modelType, propertyInfo.Name, containerType, propertyInfo: propertyInfo);
104+
}
105+
73106
/// <summary>
74107
/// Creates a <see cref="ModelMetadataIdentity"/> for the provided parameter.
75108
/// </summary>
@@ -145,14 +178,21 @@ public ModelMetadataKind MetadataKind
145178
/// </summary>
146179
public ParameterInfo ParameterInfo { get; }
147180

181+
/// <summary>
182+
/// Gets a descriptor for the property, or <c>null</c> if this instance
183+
/// does not represent a property.
184+
/// </summary>
185+
public PropertyInfo PropertyInfo { get; }
186+
148187
/// <inheritdoc />
149188
public bool Equals(ModelMetadataIdentity other)
150189
{
151190
return
152191
ContainerType == other.ContainerType &&
153192
ModelType == other.ModelType &&
154193
Name == other.Name &&
155-
ParameterInfo == other.ParameterInfo;
194+
ParameterInfo == other.ParameterInfo &&
195+
PropertyInfo == other.PropertyInfo;
156196
}
157197

158198
/// <inheritdoc />
@@ -170,6 +210,7 @@ public override int GetHashCode()
170210
hash.Add(ModelType);
171211
hash.Add(Name, StringComparer.Ordinal);
172212
hash.Add(ParameterInfo);
213+
hash.Add(PropertyInfo);
173214
return hash;
174215
}
175216
}

src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,8 @@ public void ContainerType_IsNull_ForParameter()
270270
public void ContainerType_ReturnExpectedMetadata_ForProperty()
271271
{
272272
// Arrange & Act
273-
var metadata = new TestModelMetadata(typeof(int), nameof(string.Length), typeof(string));
273+
var property = typeof(string).GetProperty(nameof(string.Length));
274+
var metadata = new TestModelMetadata(property, typeof(int), typeof(string));
274275

275276
// Assert
276277
Assert.Equal(typeof(string), metadata.ContainerType);
@@ -308,7 +309,8 @@ public void Names_ReturnExpectedMetadata_ForParameter()
308309
public void Names_ReturnExpectedMetadata_ForProperty()
309310
{
310311
// Arrange & Act
311-
var metadata = new TestModelMetadata(typeof(int), nameof(string.Length), typeof(string));
312+
var property = typeof(string).GetProperty(nameof(string.Length));
313+
var metadata = new TestModelMetadata(property, typeof(int), typeof(string));
312314

313315
// Assert
314316
Assert.Equal(nameof(string.Length), metadata.Name);
@@ -322,7 +324,8 @@ public void Names_ReturnExpectedMetadata_ForProperty()
322324
public void GetDisplayName_ReturnsDisplayName_IfSet()
323325
{
324326
// Arrange
325-
var metadata = new TestModelMetadata(typeof(int), "Length", typeof(string));
327+
var property = typeof(string).GetProperty(nameof(string.Length));
328+
var metadata = new TestModelMetadata(property, typeof(int), typeof(string));
326329
metadata.SetDisplayName("displayName");
327330

328331
// Act
@@ -351,7 +354,8 @@ public void GetDisplayName_ReturnsParameterName_WhenSetAndDisplayNameIsNull()
351354
public void GetDisplayName_ReturnsPropertyName_WhenSetAndDisplayNameIsNull()
352355
{
353356
// Arrange
354-
var metadata = new TestModelMetadata(typeof(int), "Length", typeof(string));
357+
var property = typeof(string).GetProperty(nameof(string.Length));
358+
var metadata = new TestModelMetadata(property, typeof(int), typeof(string));
355359

356360
// Act
357361
var result = metadata.GetDisplayName();
@@ -419,8 +423,8 @@ public TestModelMetadata(ParameterInfo parameter)
419423
{
420424
}
421425

422-
public TestModelMetadata(Type modelType, string propertyName, Type containerType)
423-
: base(ModelMetadataIdentity.ForProperty(modelType, propertyName, containerType))
426+
public TestModelMetadata(PropertyInfo propertyInfo, Type modelType, Type containerType)
427+
: base(ModelMetadataIdentity.ForProperty(propertyInfo, modelType, containerType))
424428
{
425429
}
426430

src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private ModelMetadataCacheEntry GetCacheEntry(ParameterInfo parameter, Type mode
190190
private ModelMetadataCacheEntry GetCacheEntry(PropertyInfo property, Type modelType)
191191
{
192192
return _typeCache.GetOrAdd(
193-
ModelMetadataIdentity.ForProperty(modelType, property.Name, property.DeclaringType),
193+
ModelMetadataIdentity.ForProperty(property, modelType, property.DeclaringType),
194194
_cacheEntryFactory);
195195
}
196196

@@ -275,8 +275,8 @@ protected virtual DefaultMetadataDetails[] CreatePropertyDetails(ModelMetadataId
275275
var propertyHelper = propertyHelpers[i];
276276

277277
var propertyKey = ModelMetadataIdentity.ForProperty(
278+
propertyHelper.Property,
278279
propertyHelper.Property.PropertyType,
279-
propertyHelper.Name,
280280
key.ModelType);
281281

282282
var propertyEntry = CreateSinglePropertyDetails(propertyKey, propertyHelper);

src/Mvc/Mvc.Core/test/BindAttributeTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
5-
using Microsoft.AspNetCore.Http;
64
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
75
using Moq;
86
using Xunit;
@@ -27,7 +25,9 @@ public void BindAttribute_Include(string property, bool isIncluded)
2725

2826
var context = new DefaultModelBindingContext();
2927

28+
#pragma warning disable CS0618 // Type or member is obsolete
3029
var identity = ModelMetadataIdentity.ForProperty(typeof(int), property, typeof(string));
30+
#pragma warning restore CS0618 // Type or member is obsolete
3131
context.ModelMetadata = new Mock<ModelMetadata>(identity).Object;
3232

3333
// Act

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public void CreateBindingDetails_FindsBindingBehaviorNever_OnProperty()
161161
};
162162

163163
var context = new BindingMetadataProviderContext(
164-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
164+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
165165
new ModelAttributes(new object[0], propertyAttributes, null));
166166

167167
var provider = new DefaultBindingMetadataProvider();
@@ -184,7 +184,7 @@ public void CreateBindingDetails_FindsBindNever_OnProperty()
184184
};
185185

186186
var context = new BindingMetadataProviderContext(
187-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
187+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
188188
new ModelAttributes(new object[0], propertyAttributes, null));
189189

190190
var provider = new DefaultBindingMetadataProvider();
@@ -207,7 +207,7 @@ public void CreateBindingDetails_FindsBindingBehaviorOptional_OnProperty()
207207
};
208208

209209
var context = new BindingMetadataProviderContext(
210-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
210+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
211211
new ModelAttributes(new object[0], propertyAttributes, null));
212212

213213
var provider = new DefaultBindingMetadataProvider();
@@ -230,7 +230,7 @@ public void CreateBindingDetails_FindsBindingBehaviorRequired_OnProperty()
230230
};
231231

232232
var context = new BindingMetadataProviderContext(
233-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
233+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
234234
new ModelAttributes(new object[0], propertyAttributes, null));
235235

236236
var provider = new DefaultBindingMetadataProvider();
@@ -253,7 +253,7 @@ public void CreateBindingDetails_FindsBindRequired_OnProperty()
253253
};
254254

255255
var context = new BindingMetadataProviderContext(
256-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
256+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
257257
new ModelAttributes(new object[0], propertyAttributes, null));
258258

259259
var provider = new DefaultBindingMetadataProvider();
@@ -420,7 +420,7 @@ public void CreateBindingDetails_UsesFirstAttribute()
420420
};
421421

422422
var context = new BindingMetadataProviderContext(
423-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
423+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
424424
new ModelAttributes(new object[0], propertyAttributes, null));
425425

426426
var provider = new DefaultBindingMetadataProvider();
@@ -438,7 +438,7 @@ public void CreateBindingDetails_FindsBindRequired_OnContainerClass()
438438
{
439439
// Arrange
440440
var context = new BindingMetadataProviderContext(
441-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)),
441+
ModelMetadataIdentity.ForProperty(typeof(BindRequiredOnClass).GetProperty(nameof(BindRequiredOnClass.Property)), typeof(int), typeof(BindRequiredOnClass)),
442442
new ModelAttributes(new object[0], new object[0], null));
443443

444444
var provider = new DefaultBindingMetadataProvider();
@@ -456,7 +456,7 @@ public void CreateBindingDetails_FindsBindNever_OnContainerClass()
456456
{
457457
// Arrange
458458
var context = new BindingMetadataProviderContext(
459-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
459+
ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)),
460460
new ModelAttributes(new object[0], new object[0], null));
461461

462462
var provider = new DefaultBindingMetadataProvider();
@@ -474,7 +474,7 @@ public void CreateBindingDetails_FindsBindNever_OnBaseClass()
474474
{
475475
// Arrange
476476
var context = new BindingMetadataProviderContext(
477-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)),
477+
ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)),
478478
new ModelAttributes(new object[0], new object[0], null));
479479

480480
var provider = new DefaultBindingMetadataProvider();
@@ -497,7 +497,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithOptional()
497497
};
498498

499499
var context = new BindingMetadataProviderContext(
500-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
500+
ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)),
501501
new ModelAttributes(new object[0], propertyAttributes, null));
502502

503503
var provider = new DefaultBindingMetadataProvider();
@@ -520,7 +520,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithRequired()
520520
};
521521

522522
var context = new BindingMetadataProviderContext(
523-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
523+
ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)),
524524
new ModelAttributes(new object[0], propertyAttributes, null));
525525

526526
var provider = new DefaultBindingMetadataProvider();
@@ -543,7 +543,7 @@ public void CreateBindingDetails_OverrideInheritedBehaviorOnClass_OverrideWithRe
543543
};
544544

545545
var context = new BindingMetadataProviderContext(
546-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)),
546+
ModelMetadataIdentity.ForProperty(typeof(InheritedBindNeverOnClass).GetProperty(nameof(InheritedBindNeverOnClass.Property)), typeof(int), typeof(InheritedBindNeverOnClass)),
547547
new ModelAttributes(new object[0], propertyAttributes, null));
548548

549549
var provider = new DefaultBindingMetadataProvider();
@@ -566,7 +566,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithNever()
566566
};
567567

568568
var context = new BindingMetadataProviderContext(
569-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)),
569+
ModelMetadataIdentity.ForProperty(typeof(BindRequiredOnClass).GetProperty(nameof(BindRequiredOnClass.Property)), typeof(int), typeof(BindRequiredOnClass)),
570570
new ModelAttributes(new object[0], propertyAttributes, null));
571571

572572
var provider = new DefaultBindingMetadataProvider();
@@ -585,7 +585,7 @@ public void CreateBindingDetails_OverrideBehaviorOnBaseClass_OverrideWithRequire
585585
{
586586
// Arrange
587587
var context = new BindingMetadataProviderContext(
588-
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOverridesInheritedBindNever)),
588+
ModelMetadataIdentity.ForProperty(typeof(BindRequiredOverridesInheritedBindNever).GetProperty(nameof(BindRequiredOverridesInheritedBindNever.Property)), typeof(int), typeof(BindRequiredOverridesInheritedBindNever)),
589589
new ModelAttributes(new object[0], new object[0], null));
590590

591591
var provider = new DefaultBindingMetadataProvider();
@@ -641,7 +641,7 @@ public void CreateBindingDetails_BindingBehaviorLeftAlone_ForAttributeOnProperty
641641
};
642642

643643
var context = new BindingMetadataProviderContext(
644-
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
644+
ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)),
645645
new ModelAttributes(typeAttributes, new object[0], null));
646646

647647
// These values shouldn't be changed since this is a Type-Metadata

0 commit comments

Comments
 (0)