From cdae83efae717238306167009fe0b311ba4737ad Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sun, 20 Oct 2019 06:30:58 -0700 Subject: [PATCH] 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 https://github.com/aspnet/AspNetCore/issues/14812 --- ....AspNetCore.Mvc.Abstractions.netcoreapp.cs | 3 + .../Metadata/ModelMetadataIdentity.cs | 45 ++- .../test/ModelBinding/ModelMetadataTest.cs | 16 +- .../Metadata/DefaultModelMetadataProvider.cs | 4 +- src/Mvc/Mvc.Core/test/BindAttributeTest.cs | 4 +- .../DefaultBindingMetadataProviderTest.cs | 30 +- .../Metadata/DefaultModelMetadataTest.cs | 77 +++-- .../DefaultValidationMetadataProviderTest.cs | 14 +- .../ExcludeBindingMetadataProviderTest.cs | 4 +- .../src/DataAnnotationsMetadataProvider.cs | 25 +- .../DataAnnotationsMetadataProviderTest.cs | 317 ++++++++++++++++-- ...mberRequiredBindingMetadataProviderTest.cs | 8 +- .../TestModelMetadataProvider.cs | 2 +- 13 files changed, 446 insertions(+), 103 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.netcoreapp.cs b/src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.netcoreapp.cs index a13240eca5..e41015f9e9 100644 --- a/src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.netcoreapp.cs +++ b/src/Mvc/Mvc.Abstractions/ref/Microsoft.AspNetCore.Mvc.Abstractions.netcoreapp.cs @@ -883,10 +883,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata public System.Type ModelType { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public string Name { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public System.Reflection.ParameterInfo ParameterInfo { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public System.Reflection.PropertyInfo PropertyInfo { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public bool Equals(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity other) { throw null; } public override bool Equals(object obj) { throw null; } public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForParameter(System.Reflection.ParameterInfo parameter) { throw null; } public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForParameter(System.Reflection.ParameterInfo parameter, System.Type modelType) { throw null; } + public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForProperty(System.Reflection.PropertyInfo propertyInfo, System.Type modelType, System.Type containerType) { throw null; } + [System.ObsoleteAttribute("This API is obsolete and may be removed in a future release.")] public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForProperty(System.Type modelType, string name, System.Type containerType) { throw null; } public static Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ModelMetadataIdentity ForType(System.Type modelType) { throw null; } public override int GetHashCode() { throw null; } diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/Metadata/ModelMetadataIdentity.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/Metadata/ModelMetadataIdentity.cs index 655bd9cb74..9f183c1cb2 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/Metadata/ModelMetadataIdentity.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/Metadata/ModelMetadataIdentity.cs @@ -17,12 +17,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata Type modelType, string name = null, Type containerType = null, - ParameterInfo parameterInfo = null) + ParameterInfo parameterInfo = null, + PropertyInfo propertyInfo = null) { ModelType = modelType; Name = name; ContainerType = containerType; ParameterInfo = parameterInfo; + PropertyInfo = propertyInfo; } /// @@ -47,6 +49,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// The name of the property. /// The container type of the model property. /// A . + [Obsolete("This API is obsolete and may be removed in a future release.")] public static ModelMetadataIdentity ForProperty( Type modelType, string name, @@ -70,6 +73,36 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata return new ModelMetadataIdentity(modelType, name, containerType); } + /// + /// Creates a for the provided property. + /// + /// The model type. + /// The property. + /// The container type of the model property. + /// A . + public static ModelMetadataIdentity ForProperty( + PropertyInfo propertyInfo, + Type modelType, + Type containerType) + { + if (propertyInfo == null) + { + throw new ArgumentNullException(nameof(propertyInfo)); + } + + if (modelType == null) + { + throw new ArgumentNullException(nameof(modelType)); + } + + if (containerType == null) + { + throw new ArgumentNullException(nameof(containerType)); + } + + return new ModelMetadataIdentity(modelType, propertyInfo.Name, containerType, propertyInfo: propertyInfo); + } + /// /// Creates a for the provided parameter. /// @@ -145,6 +178,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// public ParameterInfo ParameterInfo { get; } + /// + /// Gets a descriptor for the property, or null if this instance + /// does not represent a property. + /// + public PropertyInfo PropertyInfo { get; } + /// public bool Equals(ModelMetadataIdentity other) { @@ -152,7 +191,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata ContainerType == other.ContainerType && ModelType == other.ModelType && Name == other.Name && - ParameterInfo == other.ParameterInfo; + ParameterInfo == other.ParameterInfo && + PropertyInfo == other.PropertyInfo; } /// @@ -170,6 +210,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata hash.Add(ModelType); hash.Add(Name, StringComparer.Ordinal); hash.Add(ParameterInfo); + hash.Add(PropertyInfo); return hash; } } diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs index e72dea57e4..2f71dffe32 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs @@ -270,7 +270,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void ContainerType_ReturnExpectedMetadata_ForProperty() { // Arrange & Act - var metadata = new TestModelMetadata(typeof(int), nameof(string.Length), typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var metadata = new TestModelMetadata(property, typeof(int), typeof(string)); // Assert Assert.Equal(typeof(string), metadata.ContainerType); @@ -308,7 +309,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void Names_ReturnExpectedMetadata_ForProperty() { // Arrange & Act - var metadata = new TestModelMetadata(typeof(int), nameof(string.Length), typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var metadata = new TestModelMetadata(property, typeof(int), typeof(string)); // Assert Assert.Equal(nameof(string.Length), metadata.Name); @@ -322,7 +324,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void GetDisplayName_ReturnsDisplayName_IfSet() { // Arrange - var metadata = new TestModelMetadata(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var metadata = new TestModelMetadata(property, typeof(int), typeof(string)); metadata.SetDisplayName("displayName"); // Act @@ -351,7 +354,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void GetDisplayName_ReturnsPropertyName_WhenSetAndDisplayNameIsNull() { // Arrange - var metadata = new TestModelMetadata(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var metadata = new TestModelMetadata(property, typeof(int), typeof(string)); // Act var result = metadata.GetDisplayName(); @@ -419,8 +423,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { } - public TestModelMetadata(Type modelType, string propertyName, Type containerType) - : base(ModelMetadataIdentity.ForProperty(modelType, propertyName, containerType)) + public TestModelMetadata(PropertyInfo propertyInfo, Type modelType, Type containerType) + : base(ModelMetadataIdentity.ForProperty(propertyInfo, modelType, containerType)) { } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs index e7a2d454d8..710af66ebc 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs @@ -190,7 +190,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata private ModelMetadataCacheEntry GetCacheEntry(PropertyInfo property, Type modelType) { return _typeCache.GetOrAdd( - ModelMetadataIdentity.ForProperty(modelType, property.Name, property.DeclaringType), + ModelMetadataIdentity.ForProperty(property, modelType, property.DeclaringType), _cacheEntryFactory); } @@ -275,8 +275,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var propertyHelper = propertyHelpers[i]; var propertyKey = ModelMetadataIdentity.ForProperty( + propertyHelper.Property, propertyHelper.Property.PropertyType, - propertyHelper.Name, key.ModelType); var propertyEntry = CreateSinglePropertyDetails(propertyKey, propertyHelper); diff --git a/src/Mvc/Mvc.Core/test/BindAttributeTest.cs b/src/Mvc/Mvc.Core/test/BindAttributeTest.cs index 93b614e477..3dc6aa2fcb 100644 --- a/src/Mvc/Mvc.Core/test/BindAttributeTest.cs +++ b/src/Mvc/Mvc.Core/test/BindAttributeTest.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Moq; using Xunit; @@ -27,7 +25,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var context = new DefaultModelBindingContext(); +#pragma warning disable CS0618 // Type or member is obsolete var identity = ModelMetadataIdentity.ForProperty(typeof(int), property, typeof(string)); +#pragma warning restore CS0618 // Type or member is obsolete context.ModelMetadata = new Mock(identity).Object; // Act diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs index 727cd31705..3a0756391d 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs @@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -184,7 +184,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -207,7 +207,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -230,7 +230,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -253,7 +253,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -420,7 +420,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -438,7 +438,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { // Arrange var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindRequiredOnClass).GetProperty(nameof(BindRequiredOnClass.Property)), typeof(int), typeof(BindRequiredOnClass)), new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -456,7 +456,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { // Arrange var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -474,7 +474,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { // Arrange var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -497,7 +497,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -520,7 +520,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -543,7 +543,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)), + ModelMetadataIdentity.ForProperty(typeof(InheritedBindNeverOnClass).GetProperty(nameof(InheritedBindNeverOnClass.Property)), typeof(int), typeof(InheritedBindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -566,7 +566,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)), + ModelMetadataIdentity.ForProperty(typeof(BindRequiredOnClass).GetProperty(nameof(BindRequiredOnClass.Property)), typeof(int), typeof(BindRequiredOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -585,7 +585,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { // Arrange var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOverridesInheritedBindNever)), + ModelMetadataIdentity.ForProperty(typeof(BindRequiredOverridesInheritedBindNever).GetProperty(nameof(BindRequiredOverridesInheritedBindNever.Property)), typeof(int), typeof(BindRequiredOverridesInheritedBindNever)), new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -641,7 +641,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), + ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(typeAttributes, new object[0], null)); // These values shouldn't be changed since this is a Type-Metadata diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs index cace016898..b3d87dbead 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -103,7 +103,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new EmptyModelMetadataProvider(); var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); - var key = ModelMetadataIdentity.ForProperty(typeof(string), "Message", typeof(Exception)); + var key = ModelMetadataIdentity.ForProperty(typeof(Exception).GetProperty(nameof(Exception.Message)), typeof(string), typeof(Exception)); var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], new object[0], null)); // Act @@ -123,8 +123,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(TypeWithProperties).GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(string), - nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(TypeWithProperties)); var attributes = new ModelAttributes(Array.Empty(), Array.Empty(), null); @@ -160,8 +160,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(TypeWithProperties).GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(string), - nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(TypeWithProperties)); var attributes = new ModelAttributes(Array.Empty(), Array.Empty(), null); @@ -197,8 +197,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(TypeWithProperties).GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(string), - nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(TypeWithProperties)); var attributes = new ModelAttributes(Array.Empty(), Array.Empty(), null); @@ -393,19 +393,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new Mock(MockBehavior.Strict); var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); + var prop1 = typeof(Exception).GetProperty(nameof(Exception.Message)); + var prop2 = typeof(Exception).GetProperty(nameof(Exception.StackTrace)); + var expectedProperties = new DefaultModelMetadata[] { new DefaultModelMetadata( provider.Object, detailsProvider, new DefaultMetadataDetails( - ModelMetadataIdentity.ForProperty(typeof(int), "Prop1", typeof(string)), + ModelMetadataIdentity.ForProperty(prop1, typeof(int), typeof(string)), attributes: new ModelAttributes(new object[0], new object[0], null))), new DefaultModelMetadata( provider.Object, detailsProvider, new DefaultMetadataDetails( - ModelMetadataIdentity.ForProperty(typeof(int), "Prop2", typeof(string)), + ModelMetadataIdentity.ForProperty(prop2, typeof(int), typeof(string)), attributes: new ModelAttributes(new object[0], new object[0], null))), }; @@ -475,7 +478,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata provider.Object, detailsProvider, new DefaultMetadataDetails( +#pragma warning disable CS0618 // Using the obsolete overload does not affect the intent of this test, but fixing it requires a lot of code churn. ModelMetadataIdentity.ForProperty(typeof(int), originalName, typeof(string)), +#pragma warning restore CS0618 // Type or member is obsolete attributes: new ModelAttributes(new object[0], new object[0], null)))); } @@ -575,7 +580,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata foreach (var kvp in originalNamesAndOrders) { var propertyCache = new DefaultMetadataDetails( +#pragma warning disable CS0618 // Using the obsolete overload does not affect the intent of this test, but fixing it requires a lot of code churn. ModelMetadataIdentity.ForProperty(typeof(int), kvp.Key, typeof(string)), +#pragma warning restore CS0618 // Type or member is obsolete attributes: new ModelAttributes(new object[0], new object[0], null)) { DisplayMetadata = new DisplayMetadata(), @@ -934,7 +941,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata // Arrange var property = GetType() .GetProperty(nameof(CalculateHasValidators_PropertyMetadata_TypeHasNoValidatorsProperty), BindingFlags.Static | BindingFlags.NonPublic); - var modelIdentity = ModelMetadataIdentity.ForProperty(property.PropertyType, property.Name, GetType()); + var modelIdentity = ModelMetadataIdentity.ForProperty(property, property.PropertyType, GetType()); var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), false); // Act @@ -997,7 +1004,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var propertyIdentity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var property = typeof(TypeWithProperties).GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)); + var propertyIdentity = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(TypeWithProperties)); var propertyMetadata = new Mock(propertyIdentity); metadataProvider @@ -1021,10 +1029,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var property1Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var property1Identity = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(int), modelType); var property1Metadata = CreateModelMetadata(property1Identity, metadataProvider.Object, false); - var property2Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetProtectedSetProperty), typeof(string)); + var property2Identity = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(TypeWithProperties.PublicGetProtectedSetProperty)), typeof(int), modelType); var property2Metadata = CreateModelMetadata(property2Identity, metadataProvider.Object, true); metadataProvider @@ -1048,7 +1056,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var propertyIdentity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var propertyIdentity = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(int), modelType); var propertyMetadata = CreateModelMetadata(propertyIdentity, metadataProvider.Object, null); metadataProvider @@ -1072,10 +1080,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var property1Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), modelType); + var property1Identity = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(TypeWithProperties.PublicGetPublicSetProperty)), typeof(int), modelType); var property1Metadata = CreateModelMetadata(property1Identity, metadataProvider.Object, false); - var property2Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetProtectedSetProperty), modelType); + var property2Identity = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(TypeWithProperties.PublicGetProtectedSetProperty)), typeof(int), modelType); var property2Metadata = CreateModelMetadata(property2Identity, metadataProvider.Object, false); metadataProvider @@ -1099,18 +1107,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeId = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Id)), typeof(int), modelType); var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnit = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Unit)), typeof(BusinessUnit), modelType); var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); - var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManager = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Manager)), typeof(Employee), modelType); var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); - var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployees = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Employees)), typeof(List), modelType); var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); - var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitModel = typeof(BusinessUnit); + var unitHead = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Head)), typeof(Employee), unitModel); var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, false); - var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitId = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Id)), typeof(int), unitModel); var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, true); // BusinessUnit.Id has validators. metadataProvider @@ -1139,18 +1148,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeId = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Id)), typeof(int), modelType); var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnit = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Unit)), typeof(BusinessUnit), modelType); var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); - var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManager = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Manager)), typeof(Employee), modelType); var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); - var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployees = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Employees)), typeof(List), modelType); var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); - var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitModel = typeof(BusinessUnit); + var unitHead = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Head)), typeof(Employee), unitModel); var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, true); // BusinessUnit.Head has validators - var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitId = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Id)), typeof(int), unitModel); var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, false); metadataProvider @@ -1181,9 +1191,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeId = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Id)), typeof(int), modelType); var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployees = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Employees)), typeof(List), modelType); var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); metadataProvider @@ -1210,18 +1220,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var metadataProvider = new Mock(); var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeId = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Id)), typeof(int), modelType); var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); - var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnit = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Unit)), typeof(BusinessUnit), modelType); var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); - var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManager = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Manager)), typeof(Employee), modelType); var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); - var employeeEmployeesId = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployeesId = ModelMetadataIdentity.ForProperty(modelType.GetProperty(nameof(Employee.Employees)), typeof(List), modelType); var employeeEmployeesIdMetadata = CreateModelMetadata(employeeEmployeesId, metadataProvider.Object, false); - var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitModel = typeof(BusinessUnit); + var unitHead = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Head)), typeof(Employee), unitModel); var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, false); - var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitId = ModelMetadataIdentity.ForProperty(unitModel.GetProperty(nameof(BusinessUnit.Id)), typeof(int), unitModel); var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, false); metadataProvider diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs index 4c1c73544f..fde0032535 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultValidationMetadataProvider(); var attributes = new Attribute[] { new ValidateNeverAttribute() }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var key = ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultValidationMetadataProvider(); var attributes = new Attribute[] { new ValidateNeverAttribute() }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var key = ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0], null)); // Act @@ -71,8 +71,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultValidationMetadataProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(ValidateNeverClass).GetProperty(nameof(ValidateNeverClass.ClassName)), typeof(string), - nameof(ValidateNeverClass.ClassName), typeof(ValidateNeverClass)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); @@ -93,8 +93,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultValidationMetadataProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(ValidateNeverSubclass).GetProperty(nameof(ValidateNeverSubclass.SubclassName)), typeof(string), - nameof(ValidateNeverSubclass.SubclassName), typeof(ValidateNeverSubclass)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestClientModelValidationAttribute(); var attributes = new Attribute[] { attribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var key = ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act @@ -135,7 +135,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestModelValidationAttribute(); var attributes = new Attribute[] { attribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var key = ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act @@ -154,7 +154,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var key = ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.ValidationMetadata.ValidatorMetadata.Add(attribute); diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs index 8d2e6d10f3..908731445f 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs @@ -16,8 +16,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new ExcludeBindingMetadataProvider(typeof(string)); var key = ModelMetadataIdentity.ForProperty( + typeof(Person).GetProperty(nameof(Person.Age)), typeof(int), - nameof(Person.Age), typeof(Person)); var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); @@ -40,8 +40,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new ExcludeBindingMetadataProvider(typeof(int)); var key = ModelMetadataIdentity.ForProperty( + typeof(Person).GetProperty(nameof(Person.Age)), typeof(int), - nameof(Person.Age), typeof(Person)); var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); diff --git a/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs b/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs index 0e975e697f..a5c2b3f4c9 100644 --- a/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs +++ b/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs @@ -366,10 +366,27 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations } else if (context.Key.MetadataKind == ModelMetadataKind.Property) { - addInferredRequiredAttribute = IsNullableReferenceType( - context.Key.ContainerType, - member: null, - context.PropertyAttributes); + var property = context.Key.PropertyInfo; + if (property is null) + { + // PropertyInfo was unavailable on ModelIdentity prior to 3.1. + // Making a cogent argument about the nullability of the property requires inspecting the declared type, + // since looking at the runtime type may result in false positives: https://github.com/aspnet/AspNetCore/issues/14812 + // The only way we could arrive here is if the ModelMetadata was constructed using the non-default provider. + // We'll cursorily examine the attributes on the property, but not the ContainerType to make a decision about it's nullability. + + if (HasNullableAttribute(context.PropertyAttributes, out var propertyHasNullableAttribute)) + { + addInferredRequiredAttribute = propertyHasNullableAttribute; + } + } + else + { + addInferredRequiredAttribute = IsNullableReferenceType( + property.DeclaringType, + member: null, + context.PropertyAttributes); + } } else if (context.Key.MetadataKind == ModelMetadataKind.Parameter) { diff --git a/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs b/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs index f93ad363cc..9760421c9a 100644 --- a/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs +++ b/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs @@ -599,13 +599,13 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations provider.CreateDisplayMetadata(context); // Assert - using(new CultureReplacer("en-US", "en-US")) + using (new CultureReplacer("en-US", "en-US")) { Assert.Equal("name from localizer en-US", context.DisplayMetadata.DisplayName()); Assert.Equal("description from localizer en-US", context.DisplayMetadata.Description()); Assert.Equal("prompt from localizer en-US", context.DisplayMetadata.Placeholder()); } - using(new CultureReplacer("fr-FR", "fr-FR")) + using (new CultureReplacer("fr-FR", "fr-FR")) { Assert.Equal("name from localizer fr-FR", context.DisplayMetadata.DisplayName()); Assert.Equal("description from localizer fr-FR", context.DisplayMetadata.Description()); @@ -1031,12 +1031,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations // Assert var groupTwo = Assert.Single(enumNameAndGroup, e => e.Value.Equals("2", StringComparison.Ordinal)); - using(new CultureReplacer("en-US", "en-US")) + using (new CultureReplacer("en-US", "en-US")) { Assert.Equal("Loc_Two_Name", groupTwo.Key.Name); } - using(new CultureReplacer("fr-FR", "fr-FR")) + using (new CultureReplacer("fr-FR", "fr-FR")) { Assert.Equal("Loc_Two_Name", groupTwo.Key.Name); } @@ -1051,12 +1051,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations // Assert var groupTwo = Assert.Single(enumNameAndGroup, e => e.Value.Equals("2", StringComparison.Ordinal)); - using(new CultureReplacer("en-US", "en-US")) + using (new CultureReplacer("en-US", "en-US")) { Assert.Equal("Loc_Two_Name en-US", groupTwo.Key.Name); } - using(new CultureReplacer("fr-FR", "fr-FR")) + using (new CultureReplacer("fr-FR", "fr-FR")) { Assert.Equal("Loc_Two_Name fr-FR", groupTwo.Key.Name); } @@ -1071,12 +1071,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations // Assert var groupThree = Assert.Single(enumNameAndGroup, e => e.Value.Equals("3", StringComparison.Ordinal)); - using(new CultureReplacer("en-US", "en-US")) + using (new CultureReplacer("en-US", "en-US")) { Assert.Equal("type three name en-US", groupThree.Key.Name); } - using(new CultureReplacer("fr-FR", "fr-FR")) + using (new CultureReplacer("fr-FR", "fr-FR")) { Assert.Equal("type three name fr-FR", groupThree.Key.Name); } @@ -1091,12 +1091,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var groupThree = Assert.Single(enumNameAndGroup, e => e.Value.Equals("3", StringComparison.Ordinal)); // Assert - using(new CultureReplacer("en-US", "en-US")) + using (new CultureReplacer("en-US", "en-US")) { Assert.Equal("type three name en-US", groupThree.Key.Name); } - using(new CultureReplacer("fr-FR", "fr-FR")) + using (new CultureReplacer("fr-FR", "fr-FR")) { Assert.Equal("type three name fr-FR", groupThree.Key.Name); } @@ -1111,7 +1111,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var required = new RequiredAttribute(); var attributes = new Attribute[] { required }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); // Act @@ -1131,7 +1132,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var provider = CreateProvider(); var attributes = new Attribute[] { }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); context.ValidationMetadata.IsRequired = initialValue; @@ -1152,8 +1154,9 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations typeof(NullableReferenceTypes), typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType))); var key = ModelMetadataIdentity.ForProperty( - typeof(NullableReferenceTypes), - nameof(NullableReferenceTypes.NonNullableReferenceType), typeof(string)); + typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType)), + typeof(string), + typeof(NullableReferenceTypes)); var context = new ValidationMetadataProviderContext(key, attributes); // Act @@ -1174,9 +1177,11 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var attributes = ModelAttributes.GetAttributesForProperty( typeof(NullableReferenceTypes), typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceTypeWithRequired))); + var key = ModelMetadataIdentity.ForProperty( - typeof(NullableReferenceTypes), - nameof(NullableReferenceTypes.NonNullableReferenceTypeWithRequired), typeof(string)); + typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceTypeWithRequired)), + typeof(string), + typeof(NullableReferenceTypes)); var context = new ValidationMetadataProviderContext(key, attributes); // Act @@ -1201,9 +1206,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var attributes = ModelAttributes.GetAttributesForProperty( typeof(NullableReferenceTypes), typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType))); + var key = ModelMetadataIdentity.ForProperty( - typeof(NullableReferenceTypes), - nameof(NullableReferenceTypes.NonNullableReferenceType), typeof(string)); + typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType)), + typeof(string), + typeof(NullableReferenceTypes)); + var context = new ValidationMetadataProviderContext(key, attributes); // Act @@ -1214,6 +1222,189 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations Assert.DoesNotContain(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); } + [Theory] + [InlineData(nameof(DerivedTypeWithAllNonNullProperties.Property1))] + [InlineData(nameof(DerivedTypeWithAllNonNullProperties.Property2))] + public void CreateValidationMetadata_InfersRequiredAttributeOnDerivedType_BaseAnDerivedTypHaveAllNonNullProperties(string propertyName) + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithAllNonNullProperties); + var property = modelType.GetProperty(propertyName); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // This test verifies how MVC reads the NullableContextOptions. We expect the property to not have a Nullable attribute on, and for + // the types to have NullableContext. We'll encode our expectations as assertions so that we can catch if or when the compiler changes + // this behavior and the test needs to be tweaked. + Assert.False(DataAnnotationsMetadataProvider.HasNullableAttribute(context.PropertyAttributes, out _), "We do not expect NullableAttribute to be defined on the property"); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.IsRequired); + Assert.Contains(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_InfersRequiredAttributeOnDerivedType_PropertyDeclaredOnBaseType() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithAllNonNullProperties_WithNullableProperties); + var property = modelType.GetProperty(nameof(DerivedTypeWithAllNonNullProperties_WithNullableProperties.Property1)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.IsRequired); + Assert.Contains(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_InfersRequiredAttributeOnDerivedType_NullablePropertyDeclaredOnDerviedType() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithAllNonNullProperties_WithNullableProperties); + var property = modelType.GetProperty(nameof(DerivedTypeWithAllNonNullProperties_WithNullableProperties.Property2)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.IsRequired); + Assert.DoesNotContain(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Theory] + [InlineData(nameof(DerivedTypeWithNullableProperties.Property1))] + [InlineData(nameof(DerivedTypeWithNullableProperties.Property2))] + public void CreateValidationMetadata_BaseAnDerivedTypHaveAllNullableProperties_DoesNotInferRequiredAttribute(string propertyName) + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithNullableProperties); + var property = modelType.GetProperty(propertyName); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.IsRequired); + Assert.DoesNotContain(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_InfersRequiredAttribute_BaseTypeIsNullable_PropertyIsNotNull() + { + // Tests the scenario listed in https://github.com/aspnet/AspNetCore/issues/14812 + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithNullableProperties_WithNonNullProperties); + var property = modelType.GetProperty(nameof(DerivedTypeWithNullableProperties_WithNonNullProperties.Property2)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.IsRequired); + Assert.Contains(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_InfersRequiredAttribute_ShadowedPropertyIsNonNull() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(DerivedTypeWithNullableProperties_ShadowedProperty); + var property = modelType.GetProperty(nameof(DerivedTypeWithNullableProperties_ShadowedProperty.Property1)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.IsRequired); + Assert.Contains(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_DoesNotInfersRequiredAttribute_TypeImplementingNonNullAbstractClass() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(TypeImplementIInterfaceWithNonNullProperty); + var property = modelType.GetProperty(nameof(TypeImplementIInterfaceWithNonNullProperty.Property)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.IsRequired); + Assert.Contains(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_DoesNotInfersRequiredAttribute_TypeImplementingNonNullAbstractClass_NotNullable() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(TypeImplementIInterfaceWithNonNullProperty_AsNullable); + var property = modelType.GetProperty(nameof(TypeImplementIInterfaceWithNonNullProperty_AsNullable.Property)); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.IsRequired); + Assert.DoesNotContain(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + + [Fact] + public void CreateValidationMetadata_WithOldModelIdentity_DoesNotInferValueBasedOnContext() + { + // Arrange + var provider = CreateProvider(); + + var modelType = typeof(TypeWithAllNonNullProperties); + var property = modelType.GetProperty(nameof(TypeWithAllNonNullProperties.Property1)); +#pragma warning disable CS0618 // Type or member is obsolete + var key = ModelMetadataIdentity.ForProperty(property.PropertyType, property.Name, modelType); +#pragma warning restore CS0618 // Type or member is obsolete + var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.IsRequired); + Assert.DoesNotContain(context.ValidationMetadata.ValidatorMetadata, m => m is RequiredAttribute); + } + [Fact] public void CreateValidationMetadata_WillAddValidationAttributes_From_ValidationProviderAttribute() { @@ -1227,7 +1418,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations }); var attributes = new Attribute[] { new EmailAddressAttribute(), validationProviderAttribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(string), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); // Act @@ -1254,7 +1446,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var provider = CreateProvider(); var attributes = new Attribute[] { new RequiredAttribute() }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); context.BindingMetadata.IsBindingRequired = initialValue; @@ -1275,7 +1468,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var provider = CreateProvider(); var attributes = new Attribute[] { }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); context.BindingMetadata.IsReadOnly = initialValue; @@ -1294,7 +1488,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); // Act @@ -1313,7 +1508,29 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; - var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); + context.ValidationMetadata.ValidatorMetadata.Add(attribute); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + var validatorMetadata = Assert.Single(context.ValidationMetadata.ValidatorMetadata); + Assert.Same(attribute, validatorMetadata); + } + + [Fact] + public void CreateValidationDetails_ForProperty() + { + // Arrange + var provider = CreateProvider(); + + var attribute = new TestValidationAttribute(); + var attributes = new Attribute[] { attribute }; + var property = typeof(string).GetProperty(nameof(string.Length)); + var key = ModelMetadataIdentity.ForProperty(property, typeof(int), typeof(string)); var context = new ValidationMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); context.ValidationMetadata.ValidatorMetadata.Add(attribute); @@ -1479,7 +1696,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations public bool Equals(KeyValuePair x, KeyValuePair y) { - using(new CultureReplacer(string.Empty, string.Empty)) + using (new CultureReplacer(string.Empty, string.Empty)) { return x.Key.Name.Equals(y.Key.Name, StringComparison.Ordinal) && x.Key.Group.Equals(y.Key.Group, StringComparison.Ordinal); @@ -1488,7 +1705,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations public int GetHashCode(KeyValuePair obj) { - using(new CultureReplacer(string.Empty, string.Empty)) + using (new CultureReplacer(string.Empty, string.Empty)) { return obj.Key.GetHashCode(); } @@ -1657,6 +1874,56 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations { } } + + private class TypeWithAllNonNullProperties + { + public string Property1 { get; set; } = string.Empty; + } + + private class DerivedTypeWithAllNonNullProperties : TypeWithAllNonNullProperties + { + public string Property2 { get; set; } = string.Empty; + } + + private class DerivedTypeWithAllNonNullProperties_WithNullableProperties : TypeWithAllNonNullProperties + { + public string? Property2 { get; set; } = string.Empty; + } + + private class TypeWithNullableProperties + { + public string? Property1 { get; set; } + } + + private class DerivedTypeWithNullableProperties : TypeWithNullableProperties + { + public string? Property2 { get; set; } + } + + private class DerivedTypeWithNullableProperties_WithNonNullProperties : TypeWithNullableProperties + { + public string Property2 { get; set; } = string.Empty; + } + + private class DerivedTypeWithNullableProperties_ShadowedProperty : TypeWithNullableProperties + { + public new string Property1 { get; set; } = string.Empty; + } + + public abstract class AbstraceTypehNonNullProperty + { + public abstract string Property { get; set; } + } + + public class TypeImplementIInterfaceWithNonNullProperty : AbstraceTypehNonNullProperty + { + public override string Property { get; set; } = string.Empty; + } #nullable restore + + public class TypeImplementIInterfaceWithNonNullProperty_AsNullable : AbstraceTypehNonNullProperty + { + public override string Property { get; set; } + } } } diff --git a/src/Mvc/Mvc.DataAnnotations/test/DataMemberRequiredBindingMetadataProviderTest.cs b/src/Mvc/Mvc.DataAnnotations/test/DataMemberRequiredBindingMetadataProviderTest.cs index 6c04151e41..c374cca4f5 100644 --- a/src/Mvc/Mvc.DataAnnotations/test/DataMemberRequiredBindingMetadataProviderTest.cs +++ b/src/Mvc/Mvc.DataAnnotations/test/DataMemberRequiredBindingMetadataProviderTest.cs @@ -24,8 +24,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations }; var key = ModelMetadataIdentity.ForProperty( + typeof(ClassWithDataMemberIsRequiredTrue).GetProperty(nameof(ClassWithDataMemberIsRequiredTrue.StringProperty)), typeof(string), - nameof(ClassWithDataMemberIsRequiredTrue.StringProperty), typeof(ClassWithDataMemberIsRequiredTrue)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); @@ -50,8 +50,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations }; var key = ModelMetadataIdentity.ForProperty( + typeof(ClassWithDataMemberIsRequiredFalse).GetProperty(nameof(ClassWithDataMemberIsRequiredFalse.StringProperty)), typeof(string), - nameof(ClassWithDataMemberIsRequiredFalse.StringProperty), typeof(ClassWithDataMemberIsRequiredFalse)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); @@ -98,8 +98,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations var provider = new DataMemberRequiredBindingMetadataProvider(); var key = ModelMetadataIdentity.ForProperty( + typeof(ClassWithoutAttributes).GetProperty(nameof(ClassWithoutAttributes.StringProperty)), typeof(string), - nameof(ClassWithoutAttributes.StringProperty), typeof(ClassWithoutAttributes)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], new object[0])); @@ -126,8 +126,8 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations }; var key = ModelMetadataIdentity.ForProperty( + typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract).GetProperty(nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty)), typeof(string), - nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty), typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract)); var context = new BindingMetadataProviderContext(key, GetModelAttributes(new object[0], attributes)); diff --git a/src/Mvc/shared/Mvc.Core.TestCommon/TestModelMetadataProvider.cs b/src/Mvc/shared/Mvc.Core.TestCommon/TestModelMetadataProvider.cs index d284e4197d..43aa7e1d28 100644 --- a/src/Mvc/shared/Mvc.Core.TestCommon/TestModelMetadataProvider.cs +++ b/src/Mvc/shared/Mvc.Core.TestCommon/TestModelMetadataProvider.cs @@ -122,7 +122,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var property = containerType.GetRuntimeProperty(propertyName); Assert.NotNull(property); - var key = ModelMetadataIdentity.ForProperty(property.PropertyType, propertyName, containerType); + var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, containerType); var builder = new MetadataBuilder(key); _detailsProvider.Builders.Add(builder);