diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/IPropertyFilterProvider.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/IPropertyFilterProvider.cs index 6494d63ffd0f..0c364c5cc266 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/IPropertyFilterProvider.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/IPropertyFilterProvider.cs @@ -6,12 +6,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { /// - /// Provides a predicate which can determines which model properties should be bound by model binding. + /// Provides a predicate which can determines which model properties or parameters should be bound by model binding. /// public interface IPropertyFilterProvider { /// + /// /// Gets a predicate which can determines which model properties should be bound by model binding. + /// /// /// This predicate is also used to determine which parameters are bound when a model's constructor is bound. /// diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index 23977b49267b..097476b6a15f 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -105,11 +105,10 @@ internal IReadOnlyList BoundProperties { get { - // An item may appear as both a constructor parameter and a property. For instance, in record types, - // each constructor parameter is also a settable property and will have the same name, possibly with a difference in case. + // In record types, each constructor parameter in the primary constructor is also a settable property with the same name. // Executing model binding on these parameters twice may have detrimental effects, such as duplicate validation entries, // or failures if a model expects to be bound exactly ones. - // Consequently when a bound constructor is present, we only bind and validate the subset of properties whose names + // Consequently when binding to a constructor, we only bind and validate the subset of properties whose names // haven't appeared as parameters. if (BoundConstructor is null) { @@ -118,7 +117,7 @@ internal IReadOnlyList BoundProperties if (_boundProperties is null) { - var boundParameters = BoundConstructor.Parameters!; + var boundParameters = BoundConstructor.BoundConstructorParameters!; var boundProperties = new List(); foreach (var metadata in Properties) @@ -138,7 +137,7 @@ internal IReadOnlyList BoundProperties } } - internal IReadOnlyDictionary ParameterMapping + internal IReadOnlyDictionary BoundConstructorParameterMapping { get { @@ -153,7 +152,7 @@ internal IReadOnlyDictionary ParameterMapping return _parameterMapping; } - var boundParameters = BoundConstructor.Parameters!; + var boundParameters = BoundConstructor.BoundConstructorParameters!; var parameterMapping = new Dictionary(); foreach (var parameter in boundParameters) @@ -179,10 +178,10 @@ internal IReadOnlyDictionary ParameterMapping public virtual ModelMetadata? BoundConstructor { get; } /// - /// Gets the collection of instances for a constructor's parameters. + /// Gets the collection of instances for parameters on a . /// This is only available when is . /// - public virtual IReadOnlyList? Parameters { get; } + public virtual IReadOnlyList? BoundConstructorParameters { get; } /// /// Gets the name of a model if specified explicitly using . @@ -491,9 +490,9 @@ internal IReadOnlyDictionary ParameterMapping public abstract Action PropertySetter { get; } /// - /// Gets a delegate that invokes the constructor. + /// Gets a delegate that invokes the bound constructor if non-. /// - public virtual Func? ConstructorInvoker => null; + public virtual Func? BoundConstructorInvoker => null; /// /// Gets a display name for the model. diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs index b75ea9c172aa..61ef369e37ce 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs @@ -9,7 +9,6 @@ using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; -using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Mvc.ModelBinding diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs index c1d46cd3dd63..f9e24ea41546 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelMetadataTest.cs @@ -731,9 +731,9 @@ public override Action PropertySetter public override ModelMetadata BoundConstructor => throw new NotImplementedException(); - public override Func ConstructorInvoker => throw new NotImplementedException(); + public override Func BoundConstructorInvoker => throw new NotImplementedException(); - public override IReadOnlyList Parameters => throw new NotImplementedException(); + public override IReadOnlyList BoundConstructorParameters => throw new NotImplementedException(); } private class CollectionImplementation : ICollection diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinder.cs index b75b09a3b9c8..979fa31b932e 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinder.cs @@ -82,11 +82,11 @@ private async Task BindModelCoreAsync(ModelBindingContext bindingContext, int pr var boundConstructor = modelMetadata.BoundConstructor; if (boundConstructor != null) { - var values = new object[boundConstructor.Parameters.Count]; + var values = new object[boundConstructor.BoundConstructorParameters.Count]; var (attemptedParameterBinding, parameterBindingSucceeded) = await BindParameters( bindingContext, propertyData, - boundConstructor.Parameters, + boundConstructor.BoundConstructorParameters, values); attemptedBinding |= attemptedParameterBinding; @@ -162,7 +162,7 @@ internal static bool CreateModel(ModelBindingContext bindingContext, ModelMetada { try { - bindingContext.Model = boundConstructor.ConstructorInvoker(values); + bindingContext.Model = boundConstructor.BoundConstructorInvoker(values); return true; } catch (Exception ex) @@ -552,7 +552,7 @@ private int CanBindAnyModelItem(ModelBindingContext bindingContext) var performsConstructorBinding = bindingContext.Model == null && modelMetadata.BoundConstructor != null; if (modelMetadata.Properties.Count == 0 && - (!performsConstructorBinding || modelMetadata.BoundConstructor.Parameters.Count == 0)) + (!performsConstructorBinding || modelMetadata.BoundConstructor.BoundConstructorParameters.Count == 0)) { Log.NoPublicSettableItems(_logger, bindingContext); return NoDataAvailable; @@ -617,7 +617,7 @@ private int CanBindAnyModelItem(ModelBindingContext bindingContext) if (performsConstructorBinding) { - var parameters = bindingContext.ModelMetadata.BoundConstructor.Parameters; + var parameters = bindingContext.ModelMetadata.BoundConstructor.BoundConstructorParameters; for (var i = 0; i < parameters.Count; i++) { var parameterMetadata = parameters[i]; diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinderProvider.cs index 57cb19ecb8b4..62151794f287 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinderProvider.cs @@ -48,11 +48,11 @@ private static IReadOnlyList GetParameterBinders(ModelBinderProvid return Array.Empty(); } - var parameterBinders = boundConstructor.Parameters.Count == 0 ? Array.Empty() : new IModelBinder[boundConstructor.Parameters.Count]; + var parameterBinders = boundConstructor.BoundConstructorParameters.Count == 0 ? Array.Empty() : new IModelBinder[boundConstructor.BoundConstructorParameters.Count]; for (var i = 0; i < parameterBinders.Length; i++) { - parameterBinders[i] = context.CreateBinder(boundConstructor.Parameters[i]); + parameterBinders[i] = context.CreateBinder(boundConstructor.BoundConstructorParameters[i]); } return parameterBinders; diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultBindingMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultBindingMetadataProvider.cs index 62bc75f505c7..dd7fc3ee87cf 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultBindingMetadataProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultBindingMetadataProvider.cs @@ -104,8 +104,9 @@ private static ConstructorInfo GetRecordTypeConstructor(Type type, ConstructorIn } // For record types, we will support binding and validating the primary constructor. - // There isn't metadata to identify a primary constructor. We require that at most one constructor is defined on the type, - // and that every parameter on the constructor has a corresponding property. + // There isn't metadata to identify a primary constructor. Our heuristic is: + // We require exactly one constructor to be defined on the type, and that every parameter on + // that constructor is mapped to a property with the same name and type. if (constructors.Length > 1) { @@ -135,7 +136,9 @@ private static ConstructorInfo GetRecordTypeConstructor(Type type, ConstructorIn static bool IsRecordType(Type type) { - return type.GetProperty("EqualityContract", BindingFlags.NonPublic | BindingFlags.Instance) != null; + // Based on the state of the art as described in https://github.com/dotnet/roslyn/issues/45777 + var cloneMethod = type.GetMethod("<>Clone", BindingFlags.Public | BindingFlags.Instance); + return cloneMethod != null && cloneMethod.ReturnType == type; } } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultMetadataDetails.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultMetadataDetails.cs index 84a13db644ba..5290766553ab 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultMetadataDetails.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultMetadataDetails.cs @@ -57,7 +57,7 @@ public DefaultMetadataDetails(ModelMetadataIdentity key, ModelAttributes attribu /// /// Gets or sets the entries for constructor parameters. /// - public ModelMetadata[] ConstructorParameters { get; set; } + public ModelMetadata[] BoundConstructorParameters { get; set; } /// /// Gets or sets a property getter delegate to get the property value from a model object. @@ -70,7 +70,7 @@ public DefaultMetadataDetails(ModelMetadataIdentity key, ModelAttributes attribu public Action PropertySetter { get; set; } /// - /// Gets or sets a delegate used to construct an object using the model binding constructor. + /// Gets or sets a delegate used to invoke the bound constructor for record types. /// public Func BoundConstructorInvoker { get; set; } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs index 2b6eb4e20ffb..7d57a10f3939 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -407,7 +407,7 @@ public override ModelMetadata BoundConstructor } } - public override IReadOnlyList Parameters => _details.ConstructorParameters; + public override IReadOnlyList BoundConstructorParameters => _details.BoundConstructorParameters; /// public override IPropertyFilterProvider PropertyFilterProvider => BindingMetadata.PropertyFilterProvider; @@ -517,7 +517,7 @@ internal static bool CalculateHasValidators(HashSet visite } else if (defaultModelMetadata.IsComplexType) { - var parameters = defaultModelMetadata.BoundConstructor?.Parameters ?? Array.Empty(); + var parameters = defaultModelMetadata.BoundConstructor?.BoundConstructorParameters ?? Array.Empty(); foreach (var parameter in parameters) { if (CalculateHasValidators(visited, parameter)) @@ -559,7 +559,7 @@ public override IReadOnlyList ValidatorMetadata /// public override Action PropertySetter => _details.PropertySetter; - public override Func ConstructorInvoker => _details.BoundConstructorInvoker; + public override Func BoundConstructorInvoker => _details.BoundConstructorInvoker; internal DefaultMetadataDetails Details => _details; diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs index 55124e8eab79..8a76ab68dcb9 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadataProvider.cs @@ -5,11 +5,8 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Linq.Expressions; using System.Reflection; -using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Options; @@ -275,7 +272,7 @@ private DefaultMetadataDetails CreateConstructorDetails(ModelMetadataIdentity co } var constructorDetails = new DefaultMetadataDetails(constructorKey, ModelAttributes.Empty); - constructorDetails.ConstructorParameters = parameterMetadata; + constructorDetails.BoundConstructorParameters = parameterMetadata; constructorDetails.BoundConstructorInvoker = CreateObjectFactory(constructor); return constructorDetails; @@ -453,13 +450,5 @@ public ModelMetadataCacheEntry(ModelMetadata metadata, DefaultMetadataDetails de public DefaultMetadataDetails Details { get; } } - - private class NullServiceProvider : IServiceProvider - { - public static readonly NullServiceProvider Instance = new NullServiceProvider(); - - // We do not expect this to be invoked at all. - public object GetService(Type serviceType) => throw new NotSupportedException(); - } } } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultComplexObjectValidationStrategy.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultComplexObjectValidationStrategy.cs index 36280cee44e5..e9a17b5a4beb 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultComplexObjectValidationStrategy.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultComplexObjectValidationStrategy.cs @@ -4,10 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Linq; -using System.Reflection; using Microsoft.AspNetCore.Mvc.Core; -using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { @@ -19,7 +16,7 @@ internal class DefaultComplexObjectValidationStrategy : IValidationStrategy /// /// Gets an instance of . /// - public static readonly DefaultComplexObjectValidationStrategy Instance = new DefaultComplexObjectValidationStrategy(); + public static readonly IValidationStrategy Instance = new DefaultComplexObjectValidationStrategy(); private DefaultComplexObjectValidationStrategy() { @@ -61,7 +58,7 @@ public Enumerator( } else { - _parameters = _modelMetadata.BoundConstructor.Parameters; + _parameters = _modelMetadata.BoundConstructor.BoundConstructorParameters; } _properties = _modelMetadata.BoundProperties; @@ -95,7 +92,7 @@ public bool MoveNext() } else { - if (!_modelMetadata.ParameterMapping.TryGetValue(parameter, out var property)) + if (!_modelMetadata.BoundConstructorParameterMapping.TryGetValue(parameter, out var property)) { throw new InvalidOperationException( Resources.FormatValidationStrategy_MappedPropertyNotFound(parameter, _modelMetadata.ModelType)); diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs index 098d2b1873aa..80e6c2fec917 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -1275,7 +1275,7 @@ public void CalculateHasValidators_RecordType_ParametersWithNoValidators() var parameterMetadata = CreateModelMetadata(parameterId, metadataProvider.Object, hasValidators: false); var constructorMetadata = CreateModelMetadata(ModelMetadataIdentity.ForConstructor(constructor, modelType), metadataProvider.Object, null); - constructorMetadata.Details.ConstructorParameters = new[] + constructorMetadata.Details.BoundConstructorParameters = new[] { parameterMetadata, }; @@ -1316,7 +1316,7 @@ public void CalculateHasValidators_RecordType_ParametersWithValidators() var parameterMetadata = CreateModelMetadata(parameterId, metadataProvider.Object, hasValidators: true); var constructorMetadata = CreateModelMetadata(ModelMetadataIdentity.ForConstructor(constructor, modelType), metadataProvider.Object, null); - constructorMetadata.Details.ConstructorParameters = new[] + constructorMetadata.Details.BoundConstructorParameters = new[] { parameterMetadata, }; @@ -1365,7 +1365,7 @@ public void CalculateHasValidators_RecordTypeWithProperty_NoValidators() var parameterMetadata = CreateModelMetadata(parameterId, metadataProvider.Object, hasValidators: false); var constructorMetadata = CreateModelMetadata(ModelMetadataIdentity.ForConstructor(constructor, modelType), metadataProvider.Object, null); - constructorMetadata.Details.ConstructorParameters = new[] + constructorMetadata.Details.BoundConstructorParameters = new[] { parameterMetadata, }; @@ -1409,7 +1409,7 @@ public void CalculateHasValidators_RecordTypeWithProperty_PropertyHasValidators( var parameterMetadata = CreateModelMetadata(parameterId, metadataProvider.Object, hasValidators: true); var constructorMetadata = CreateModelMetadata(ModelMetadataIdentity.ForConstructor(constructor, modelType), metadataProvider.Object, null); - constructorMetadata.Details.ConstructorParameters = new[] + constructorMetadata.Details.BoundConstructorParameters = new[] { parameterMetadata, }; @@ -1453,7 +1453,7 @@ public void CalculateHasValidators_RecordTypeWithProperty_MappedPropertyHasValid var parameterMetadata = CreateModelMetadata(parameterId, metadataProvider.Object, hasValidators: false); var constructorMetadata = CreateModelMetadata(ModelMetadataIdentity.ForConstructor(constructor, modelType), metadataProvider.Object, null); - constructorMetadata.Details.ConstructorParameters = new[] + constructorMetadata.Details.BoundConstructorParameters = new[] { parameterMetadata, };