Skip to content

Commit 088a6fa

Browse files
authored
Add support for binding record types (#23976)
* Add support for binding record types * PR feedback * PR changes * Changes per PR comments * Changes per PR comments * Update src/Mvc/Mvc.Core/src/Resources.resx * Apply suggestions from code review * Add some more tests * Undo blazor.server.js changes * Fixup test
1 parent b326be1 commit 088a6fa

File tree

59 files changed

+14347
-3888
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+14347
-3888
lines changed

src/Mvc/Mvc.Abstractions/src/ModelBinding/IPropertyFilterProvider.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,17 @@
66
namespace Microsoft.AspNetCore.Mvc.ModelBinding
77
{
88
/// <summary>
9-
/// Provides a predicate which can determines which model properties should be bound by model binding.
9+
/// Provides a predicate which can determines which model properties or parameters should be bound by model binding.
1010
/// </summary>
1111
public interface IPropertyFilterProvider
1212
{
1313
/// <summary>
14+
/// <para>
1415
/// Gets a predicate which can determines which model properties should be bound by model binding.
16+
/// </para>
17+
/// <para>
18+
/// This predicate is also used to determine which parameters are bound when a model's constructor is bound.
19+
/// </para>
1520
/// </summary>
1621
Func<ModelMetadata, bool> PropertyFilter { get; }
1722
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public abstract class ModelBindingMessageProvider
5151
/// <summary>
5252
/// Error message the model binding system adds when <see cref="ModelError.Exception"/> is of type
5353
/// <see cref="FormatException"/> or <see cref="OverflowException"/>, value is known, and error is associated
54-
/// with a collection element or action parameter.
54+
/// with a collection element or parameter.
5555
/// </summary>
5656
/// <value>Default <see cref="string"/> is "The value '{0}' is not valid.".</value>
5757
public virtual Func<string, string> NonPropertyAttemptedValueIsInvalidAccessor { get; } = default!;
@@ -67,7 +67,7 @@ public abstract class ModelBindingMessageProvider
6767
/// <summary>
6868
/// Error message the model binding system adds when <see cref="ModelError.Exception"/> is of type
6969
/// <see cref="FormatException"/> or <see cref="OverflowException"/>, value is unknown, and error is associated
70-
/// with a collection element or action parameter.
70+
/// with a collection element or parameter.
7171
/// </summary>
7272
/// <value>Default <see cref="string"/> is "The supplied value is invalid.".</value>
7373
public virtual Func<string> NonPropertyUnknownValueIsInvalidAccessor { get; } = default!;

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

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ private ModelMetadataIdentity(
1616
Type modelType,
1717
string? name = null,
1818
Type? containerType = null,
19-
object? fieldInfo = null)
19+
object? fieldInfo = null,
20+
ConstructorInfo? constructorInfo = null)
2021
{
2122
ModelType = modelType;
2223
Name = name;
2324
ContainerType = containerType;
2425
FieldInfo = fieldInfo;
26+
ConstructorInfo = constructorInfo;
2527
}
2628

2729
/// <summary>
@@ -130,6 +132,28 @@ public static ModelMetadataIdentity ForParameter(ParameterInfo parameter, Type m
130132
return new ModelMetadataIdentity(modelType, parameter.Name, fieldInfo: parameter);
131133
}
132134

135+
/// <summary>
136+
/// Creates a <see cref="ModelMetadataIdentity"/> for the provided parameter with the specified
137+
/// model type.
138+
/// </summary>
139+
/// <param name="constructor">The <see cref="ConstructorInfo" />.</param>
140+
/// <param name="modelType">The model type.</param>
141+
/// <returns>A <see cref="ModelMetadataIdentity"/>.</returns>
142+
public static ModelMetadataIdentity ForConstructor(ConstructorInfo constructor, Type modelType)
143+
{
144+
if (constructor == null)
145+
{
146+
throw new ArgumentNullException(nameof(constructor));
147+
}
148+
149+
if (modelType == null)
150+
{
151+
throw new ArgumentNullException(nameof(modelType));
152+
}
153+
154+
return new ModelMetadataIdentity(modelType, constructor.Name, constructorInfo: constructor);
155+
}
156+
133157
/// <summary>
134158
/// Gets the <see cref="Type"/> defining the model property represented by the current
135159
/// instance, or <c>null</c> if the current instance does not represent a property.
@@ -152,6 +176,10 @@ public ModelMetadataKind MetadataKind
152176
{
153177
return ModelMetadataKind.Parameter;
154178
}
179+
else if (ConstructorInfo != null)
180+
{
181+
return ModelMetadataKind.Constructor;
182+
}
155183
else if (ContainerType != null && Name != null)
156184
{
157185
return ModelMetadataKind.Property;
@@ -183,6 +211,12 @@ public ModelMetadataKind MetadataKind
183211
/// </summary>
184212
public PropertyInfo? PropertyInfo => FieldInfo as PropertyInfo;
185213

214+
/// <summary>
215+
/// Gets a descriptor for the constructor, or <c>null</c> if this instance
216+
/// does not represent a constructor.
217+
/// </summary>
218+
public ConstructorInfo? ConstructorInfo { get; }
219+
186220
/// <inheritdoc />
187221
public bool Equals(ModelMetadataIdentity other)
188222
{
@@ -191,7 +225,8 @@ public bool Equals(ModelMetadataIdentity other)
191225
ModelType == other.ModelType &&
192226
Name == other.Name &&
193227
ParameterInfo == other.ParameterInfo &&
194-
PropertyInfo == other.PropertyInfo;
228+
PropertyInfo == other.PropertyInfo &&
229+
ConstructorInfo == other.ConstructorInfo;
195230
}
196231

197232
/// <inheritdoc />
@@ -210,6 +245,7 @@ public override int GetHashCode()
210245
hash.Add(Name, StringComparer.Ordinal);
211246
hash.Add(ParameterInfo);
212247
hash.Add(PropertyInfo);
248+
hash.Add(ConstructorInfo);
213249
return hash.ToHashCode();
214250
}
215251
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,10 @@ public enum ModelMetadataKind
2222
/// Used for <see cref="ModelMetadata"/> for a parameter.
2323
/// </summary>
2424
Parameter,
25+
26+
/// <summary>
27+
/// <see cref="ModelMetadata"/> for a constructor.
28+
/// </summary>
29+
Constructor,
2530
}
26-
}
31+
}

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

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
using System;
55
using System.Collections;
66
using System.Collections.Generic;
7+
using System.Collections.ObjectModel;
78
using System.ComponentModel;
89
using System.Diagnostics;
10+
using System.Linq;
911
using System.Reflection;
1012
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
1113
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
@@ -24,7 +26,11 @@ public abstract class ModelMetadata : IEquatable<ModelMetadata?>, IModelMetadata
2426
/// </summary>
2527
public static readonly int DefaultOrder = 10000;
2628

29+
private static readonly IReadOnlyDictionary<ModelMetadata, ModelMetadata> EmptyParameterMapping = new Dictionary<ModelMetadata, ModelMetadata>(0);
30+
2731
private int? _hashCode;
32+
private IReadOnlyList<ModelMetadata>? _boundProperties;
33+
private IReadOnlyDictionary<ModelMetadata, ModelMetadata>? _parameterMapping;
2834

2935
/// <summary>
3036
/// Creates a new <see cref="ModelMetadata"/>.
@@ -83,7 +89,7 @@ public virtual ModelMetadata ContainerMetadata
8389
/// <summary>
8490
/// Gets the key for the current instance.
8591
/// </summary>
86-
protected ModelMetadataIdentity Identity { get; }
92+
protected internal ModelMetadataIdentity Identity { get; }
8793

8894
/// <summary>
8995
/// Gets a collection of additional information about the model.
@@ -95,6 +101,88 @@ public virtual ModelMetadata ContainerMetadata
95101
/// </summary>
96102
public abstract ModelPropertyCollection Properties { get; }
97103

104+
internal IReadOnlyList<ModelMetadata> BoundProperties
105+
{
106+
get
107+
{
108+
// In record types, each constructor parameter in the primary constructor is also a settable property with the same name.
109+
// Executing model binding on these parameters twice may have detrimental effects, such as duplicate ModelState entries,
110+
// or failures if a model expects to be bound exactly ones.
111+
// Consequently when binding to a constructor, we only bind and validate the subset of properties whose names
112+
// haven't appeared as parameters.
113+
if (BoundConstructor is null)
114+
{
115+
return Properties;
116+
}
117+
118+
if (_boundProperties is null)
119+
{
120+
var boundParameters = BoundConstructor.BoundConstructorParameters!;
121+
var boundProperties = new List<ModelMetadata>();
122+
123+
foreach (var metadata in Properties)
124+
{
125+
if (!boundParameters.Any(p =>
126+
string.Equals(p.ParameterName, metadata.PropertyName, StringComparison.Ordinal)
127+
&& p.ModelType == metadata.ModelType))
128+
{
129+
boundProperties.Add(metadata);
130+
}
131+
}
132+
133+
_boundProperties = boundProperties;
134+
}
135+
136+
return _boundProperties;
137+
}
138+
}
139+
140+
internal IReadOnlyDictionary<ModelMetadata, ModelMetadata> BoundConstructorParameterMapping
141+
{
142+
get
143+
{
144+
if (_parameterMapping != null)
145+
{
146+
return _parameterMapping;
147+
}
148+
149+
if (BoundConstructor is null)
150+
{
151+
_parameterMapping = EmptyParameterMapping;
152+
return _parameterMapping;
153+
}
154+
155+
var boundParameters = BoundConstructor.BoundConstructorParameters!;
156+
var parameterMapping = new Dictionary<ModelMetadata, ModelMetadata>();
157+
158+
foreach (var parameter in boundParameters)
159+
{
160+
var property = Properties.FirstOrDefault(p =>
161+
string.Equals(p.Name, parameter.ParameterName, StringComparison.Ordinal) &&
162+
p.ModelType == parameter.ModelType);
163+
164+
if (property != null)
165+
{
166+
parameterMapping[parameter] = property;
167+
}
168+
}
169+
170+
_parameterMapping = parameterMapping;
171+
return _parameterMapping;
172+
}
173+
}
174+
175+
/// <summary>
176+
/// Gets <see cref="ModelMetadata"/> instance for a constructor of a record type that is used during binding and validation.
177+
/// </summary>
178+
public virtual ModelMetadata? BoundConstructor { get; }
179+
180+
/// <summary>
181+
/// Gets the collection of <see cref="ModelMetadata"/> instances for parameters on a <see cref="BoundConstructor"/>.
182+
/// This is only available when <see cref="MetadataKind"/> is <see cref="ModelMetadataKind.Constructor"/>.
183+
/// </summary>
184+
public virtual IReadOnlyList<ModelMetadata>? BoundConstructorParameters { get; }
185+
98186
/// <summary>
99187
/// Gets the name of a model if specified explicitly using <see cref="IModelNameProvider"/>.
100188
/// </summary>
@@ -401,6 +489,11 @@ public virtual ModelMetadata ContainerMetadata
401489
/// </summary>
402490
public abstract Action<object, object> PropertySetter { get; }
403491

492+
/// <summary>
493+
/// Gets a delegate that invokes the bound constructor <see cref="BoundConstructor" /> if non-<see langword="null" />.
494+
/// </summary>
495+
public virtual Func<object[], object>? BoundConstructorInvoker => null;
496+
404497
/// <summary>
405498
/// Gets a display name for the model.
406499
/// </summary>
@@ -500,6 +593,8 @@ private string DebuggerToString()
500593
return $"ModelMetadata (Property: '{ContainerType!.Name}.{PropertyName}' Type: '{ModelType.Name}')";
501594
case ModelMetadataKind.Type:
502595
return $"ModelMetadata (Type: '{ModelType.Name}')";
596+
case ModelMetadataKind.Constructor:
597+
return $"ModelMetadata (Constructor: '{ModelType.Name}')";
503598
default:
504599
return $"Unsupported MetadataKind '{MetadataKind}'.";
505600
}

src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadataProvider.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// 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

44
using System;
@@ -54,5 +54,16 @@ public virtual ModelMetadata GetMetadataForProperty(PropertyInfo propertyInfo, T
5454
{
5555
throw new NotSupportedException();
5656
}
57+
58+
/// <summary>
59+
/// Supplies metadata describing a constructor.
60+
/// </summary>
61+
/// <param name="constructor">The <see cref="ConstructorInfo"/>.</param>
62+
/// <param name="modelType">The type declaring the constructor.</param>
63+
/// <returns>A <see cref="ModelMetadata"/> instance describing the <paramref name="constructor"/>.</returns>
64+
public virtual ModelMetadata GetMetadataForConstructor(ConstructorInfo constructor, Type modelType)
65+
{
66+
throw new NotSupportedException();
67+
}
5768
}
5869
}

src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelStateDictionary.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ public bool TryAddModelError(string key, Exception exception, ModelMetadata meta
298298
// "The value '' is not valid." (when no value was provided, not even an empty string) and
299299
// "The supplied value is invalid for Int32." (when error is for an element or parameter).
300300
var messageProvider = metadata.ModelBindingMessageProvider;
301+
301302
var name = metadata.DisplayName ?? metadata.PropertyName;
302303
string errorMessage;
303304
if (entry == null && name == null)

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesValuesFromBindingI
8383
// Arrange
8484
var attributes = new object[]
8585
{
86-
new ModelBinderAttribute { BinderType = typeof(ComplexTypeModelBinder), Name = "Test" },
86+
new ModelBinderAttribute { BinderType = typeof(ComplexObjectModelBinder), Name = "Test" },
8787
};
8888
var modelType = typeof(Guid);
8989
var provider = new TestModelMetadataProvider();
@@ -100,7 +100,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesValuesFromBindingI
100100

101101
// Assert
102102
Assert.NotNull(bindingInfo);
103-
Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
103+
Assert.Same(typeof(ComplexObjectModelBinder), bindingInfo.BinderType);
104104
Assert.Same("Test", bindingInfo.BinderModelName);
105105
}
106106

@@ -110,7 +110,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderNameFromMode
110110
// Arrange
111111
var attributes = new object[]
112112
{
113-
new ModelBinderAttribute(typeof(ComplexTypeModelBinder)),
113+
new ModelBinderAttribute(typeof(ComplexObjectModelBinder)),
114114
new ControllerAttribute(),
115115
new BindNeverAttribute(),
116116
};
@@ -129,7 +129,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderNameFromMode
129129

130130
// Assert
131131
Assert.NotNull(bindingInfo);
132-
Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
132+
Assert.Same(typeof(ComplexObjectModelBinder), bindingInfo.BinderType);
133133
Assert.Same("Different", bindingInfo.BinderModelName);
134134
Assert.Same(BindingSource.Custom, bindingInfo.BindingSource);
135135
}
@@ -143,7 +143,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesModelBinderFromMod
143143
var provider = new TestModelMetadataProvider();
144144
provider.ForType(modelType).BindingDetails(metadata =>
145145
{
146-
metadata.BinderType = typeof(ComplexTypeModelBinder);
146+
metadata.BinderType = typeof(ComplexObjectModelBinder);
147147
});
148148
var modelMetadata = provider.GetMetadataForType(modelType);
149149

@@ -152,7 +152,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesModelBinderFromMod
152152

153153
// Assert
154154
Assert.NotNull(bindingInfo);
155-
Assert.Same(typeof(ComplexTypeModelBinder), bindingInfo.BinderType);
155+
Assert.Same(typeof(ComplexObjectModelBinder), bindingInfo.BinderType);
156156
}
157157

158158
[Fact]
@@ -187,7 +187,7 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesPropertyPredicateP
187187
// Arrange
188188
var attributes = new object[]
189189
{
190-
new ModelBinderAttribute(typeof(ComplexTypeModelBinder)),
190+
new ModelBinderAttribute(typeof(ComplexObjectModelBinder)),
191191
new ControllerAttribute(),
192192
new BindNeverAttribute(),
193193
};

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,12 @@ public override Action<object, object> PropertySetter
728728
throw new NotImplementedException();
729729
}
730730
}
731+
732+
public override ModelMetadata BoundConstructor => throw new NotImplementedException();
733+
734+
public override Func<object[], object> BoundConstructorInvoker => throw new NotImplementedException();
735+
736+
public override IReadOnlyList<ModelMetadata> BoundConstructorParameters => throw new NotImplementedException();
731737
}
732738

733739
private class CollectionImplementation : ICollection<string>

src/Mvc/Mvc.Analyzers/test/TestFiles/TopLevelParameterNameAnalyzerTest/IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFi
44
{
55
public class IsProblematicParameter_ReturnsTrue_IfPropertyWithModelBindingAttributeHasSameNameAsParameter
66
{
7-
[ModelBinder(typeof(ComplexTypeModelBinder), Name = "model")]
7+
[ModelBinder(typeof(ComplexObjectModelBinder), Name = "model")]
88
public string Different { get; set; }
99

1010
public void ActionMethod(

0 commit comments

Comments
 (0)