Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
df4b6c8
New tryparse modelbinder
brunolins16 Feb 11, 2022
a695fec
Adding IformatProvider parameter
brunolins16 Feb 11, 2022
f1712a2
Adding tryparse binder
brunolins16 Feb 15, 2022
d088379
Adding IFormatProvider parameter
brunolins16 Feb 15, 2022
df3a6a4
Updating unittests and PublicAPI.txt
brunolins16 Feb 15, 2022
8c54730
Changing modelbinder to generics
brunolins16 Feb 15, 2022
4cc18a7
Updating comment
brunolins16 Feb 15, 2022
5bff268
Adding empty line
brunolins16 Feb 15, 2022
81a2408
removing unexpected VS change
brunolins16 Feb 15, 2022
17866fd
Updating csprojs
brunolins16 Feb 15, 2022
c89d9e2
Merge branch 'main' into brunolins16/issues/39682
brunolins16 Feb 15, 2022
fe3d122
Changing property name to IsParseableType
brunolins16 Feb 16, 2022
b3cff44
Changing property name to IsParseableType
brunolins16 Feb 16, 2022
e73543d
fix typo
brunolins16 Feb 16, 2022
248dc11
PR Feeback
brunolins16 Feb 16, 2022
dcc5fae
Rolling back to non-generic class
brunolins16 Feb 17, 2022
d2ef7b8
Adding SuppressParseableTypesBinding option
brunolins16 Feb 17, 2022
d9ff2d0
Updating tests
brunolins16 Feb 17, 2022
50dfb9d
Fix typo
brunolins16 Feb 17, 2022
748b095
fix commit style
brunolins16 Feb 17, 2022
83a28d2
Adding missed file
brunolins16 Feb 17, 2022
a6e3939
Adding one more missed file
brunolins16 Feb 18, 2022
c914e70
PR Feeback
brunolins16 Feb 22, 2022
d6a8c52
Fixing unit test
brunolins16 Feb 23, 2022
01c0dd6
Update src/Mvc/Mvc.Core/src/Resources.resx
brunolins16 Feb 23, 2022
b730ad4
Updating test
brunolins16 Feb 23, 2022
2d6ca53
Merge branch 'brunolins16/issues/39682' of https://github.com/brunoli…
brunolins16 Feb 23, 2022
d1203e6
Moving TryParse method check to DefaultModelMetadata
brunolins16 Feb 24, 2022
47708fe
Fixing unittest
brunolins16 Feb 24, 2022
5d3c7b1
API Review: Removing SuppressParseableTypeBinding option
brunolins16 Feb 28, 2022
49fdd21
PR feeback
brunolins16 Feb 28, 2022
92dd9c5
Update src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinder.cs
brunolins16 Mar 3, 2022
7a07a9a
PR Feeback
brunolins16 Mar 3, 2022
f292992
Merge branch 'brunolins16/issues/39682' of https://github.com/brunoli…
brunolins16 Mar 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
HttpContextExpr, parameterTypeNameConstant, parameterNameConstant,
TempSourceStringExpr, Expression.Constant(factoryContext.ThrowOnBadRequest)));

var tryParseCall = tryParseMethodCall(parsedValue);
var tryParseCall = tryParseMethodCall(parsedValue, Expression.Constant(CultureInfo.InvariantCulture));
Comment thread
pranavkm marked this conversation as resolved.

// The following code is generated if the parameter is required and
// the method should not be matched.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void FindTryParseStringMethod_ReturnsTheExpectedTryParseMethodWithInvaria

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue")) as MethodCallExpression;
var call = methodFound!(Expression.Variable(type, "parsedValue"), Expression.Constant(CultureInfo.InvariantCulture)) as MethodCallExpression;
Assert.NotNull(call);
var parameters = call!.Method.GetParameters();

Expand All @@ -53,7 +53,7 @@ public void FindTryParseStringMethod_ReturnsTheExpectedTryParseMethodWithInvaria

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue")) as MethodCallExpression;
var call = methodFound!(Expression.Variable(type, "parsedValue"), Expression.Constant(CultureInfo.InvariantCulture)) as MethodCallExpression;
Assert.NotNull(call);
var parameters = call!.Method.GetParameters();

Expand Down Expand Up @@ -85,7 +85,7 @@ public void FindTryParseStringMethod_ReturnsTheExpectedTryParseMethodWithInvaria

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue")) as MethodCallExpression;
var call = methodFound!(Expression.Variable(type, "parsedValue"), Expression.Constant(CultureInfo.InvariantCulture)) as MethodCallExpression;
Assert.NotNull(call);
var parameters = call!.Method.GetParameters();

Expand All @@ -109,7 +109,7 @@ public void FindTryParseMethod_WithNoFormatProvider(Type type)
var methodFound = new ParameterBindingMethodCache().FindTryParseMethod(@type);
Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue")) as MethodCallExpression;
var call = methodFound!(Expression.Variable(type, "parsedValue"), Expression.Constant(CultureInfo.InvariantCulture)) as MethodCallExpression;
Assert.NotNull(call);
var parameters = call!.Method.GetParameters();

Expand Down Expand Up @@ -155,7 +155,7 @@ public void FindTryParseStringMethod_WorksForEnums()

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue")) as MethodCallExpression;
var call = methodFound!(Expression.Variable(type, "parsedValue"), Expression.Constant(CultureInfo.InvariantCulture)) as MethodCallExpression;
Assert.NotNull(call);
var method = call!.Method;
var parameters = method.GetParameters();
Expand All @@ -177,7 +177,7 @@ public void FindTryParseStringMethod_WorksForEnumsWhenNonGenericEnumParseIsUsed(
Assert.NotNull(methodFound);

var parsedValue = Expression.Variable(type, "parsedValue");
var block = methodFound!(parsedValue) as BlockExpression;
var block = methodFound!(parsedValue, Expression.Constant(CultureInfo.InvariantCulture)) as BlockExpression;
Assert.NotNull(block);
Assert.Equal(typeof(bool), block!.Type);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Microsoft.AspNetCore.Mvc.IActionResult</Description>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)ClosedGenericMatcher\*.cs" />
<Compile Include="$(SharedSourceRoot)\ParameterBindingMethodCache.cs" Link="ModelBinding\ParameterBindingMethodCache.cs" />
<Compile Include="$(SharedSourceRoot)\TypeNameHelper\TypeNameHelper.cs" Link="ModelBinding\TypeNameHelper.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
40 changes: 36 additions & 4 deletions src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
Expand All @@ -25,6 +27,8 @@ public abstract class ModelMetadata : IEquatable<ModelMetadata?>, IModelMetadata
/// </summary>
public static readonly int DefaultOrder = 10000;

private static readonly ParameterBindingMethodCache ParameterBindingMethodCache = new();

private int? _hashCode;
private IReadOnlyList<ModelMetadata>? _boundProperties;
private IReadOnlyDictionary<ModelMetadata, ModelMetadata>? _parameterMapping;
Expand Down Expand Up @@ -434,10 +438,10 @@ internal IReadOnlyDictionary<ModelMetadata, ModelMetadata> BoundConstructorPrope
/// </summary>
/// <remarks>
/// A complex type is defined as a <see cref="Type"/> without a <see cref="TypeConverter"/> that can convert
/// from <see cref="string"/>. Most POCO and <see cref="IEnumerable"/> types are therefore complex. Most, if
/// not all, BCL value types are simple types.
/// from <see cref="string"/> and without a <c>TryParse</c> method. Most POCO and <see cref="IEnumerable"/> types are therefore complex.
/// Most, if not all, BCL value types are simple types.
/// </remarks>
public bool IsComplexType { get; private set; }
public bool IsComplexType => !IsConvertibleType && !IsParseableType;

/// <summary>
/// Gets a value indicating whether or not <see cref="ModelType"/> is a <see cref="Nullable{T}"/>.
Expand Down Expand Up @@ -475,6 +479,17 @@ internal IReadOnlyDictionary<ModelMetadata, ModelMetadata> BoundConstructorPrope
/// </remarks>
public Type UnderlyingOrModelType { get; private set; } = default!;

/// <summary>
/// Gets a value indicating whether or not <see cref="ModelType"/> has a TryParse method.
/// </summary>
internal virtual bool IsParseableType { get; private set; }

/// <summary>
/// Gets a value indicating whether or not <see cref="ModelType"/> has a <see cref="TypeConverter"/>
/// from <see cref="string"/>.
/// </summary>
internal bool IsConvertibleType { get; private set; }

/// <summary>
/// Gets a value indicating the NullabilityState of the value or reference type.
/// </summary>
Expand Down Expand Up @@ -521,6 +536,22 @@ internal void ThrowIfRecordTypeHasValidationOnProperties()
}
}

internal static Func<ParameterExpression, Expression, Expression>? FindTryParseMethod(Type modelType)
{
try
{
modelType = Nullable.GetUnderlyingType(modelType) ?? modelType;
return ParameterBindingMethodCache.FindTryParseMethod(modelType);
}
catch (InvalidOperationException)
{
// The ParameterBindingMethodCache.FindTryParseMethod throws an exception
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion - we could update FindTryParseMethod to not throw via an additional parameter. Not catching exceptions is certainly a lot nicer since it avoids things like first-chance exceptions in VS.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I will open a new PR just for that, ok?

// when an wrong try/parse method is detected
// but we don't need this behavior here and return null is enough
return null;
}
}

[MemberNotNull(nameof(_parameterMapping), nameof(_boundConstructorPropertyMapping))]
private void CalculateRecordTypeConstructorDetails()
{
Expand Down Expand Up @@ -616,7 +647,8 @@ private void InitializeTypeInformation()
{
Debug.Assert(ModelType != null);

IsComplexType = !TypeDescriptor.GetConverter(ModelType).CanConvertFrom(typeof(string));
IsConvertibleType = TypeDescriptor.GetConverter(ModelType).CanConvertFrom(typeof(string));
IsParseableType = ModelMetadata.FindTryParseMethod(ModelType) is not null;
IsNullableValueType = Nullable.GetUnderlyingType(ModelType) != null;
IsReferenceOrNullableType = !ModelType.IsValueType || IsNullableValueType;
UnderlyingOrModelType = Nullable.GetUnderlyingType(ModelType) ?? ModelType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateNotSet()
var expected = "Hmm, the supplied value is not valid for Length.";
var dictionary = new ModelStateDictionary();

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetUnknownValueIsInvalidAccessor(
Expand All @@ -1020,7 +1020,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateNotSet_Wit
var expected = "Hmm, the supplied value is not valid.";
var dictionary = new ModelStateDictionary();

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetNonPropertyUnknownValueIsInvalidAccessor(
Expand Down Expand Up @@ -1048,7 +1048,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateNotSet_Wit
var expected = "Hmm, the supplied value is not valid.";
var dictionary = new ModelStateDictionary();

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetNonPropertyUnknownValueIsInvalidAccessor(
Expand Down Expand Up @@ -1109,7 +1109,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateSet()
var dictionary = new ModelStateDictionary();
dictionary.SetModelValue("key", new string[] { "some value" }, "some value");

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetAttemptedValueIsInvalidAccessor(
Expand All @@ -1136,7 +1136,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateSet_WithPa
var dictionary = new ModelStateDictionary();
dictionary.SetModelValue("key", new string[] { "some value" }, "some value");

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetNonPropertyAttemptedValueIsInvalidAccessor(
Expand Down Expand Up @@ -1165,7 +1165,7 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateSet_WithTy
var dictionary = new ModelStateDictionary();
dictionary.SetModelValue("key", new string[] { "some value" }, "some value");

var bindingMetadataProvider = new DefaultBindingMetadataProvider();
var bindingMetadataProvider = CreateBindingMetadataProvider();
var compositeProvider = new DefaultCompositeMetadataDetailsProvider(new[] { bindingMetadataProvider });
var optionsAccessor = new OptionsAccessor();
optionsAccessor.Value.ModelBindingMessageProvider.SetNonPropertyAttemptedValueIsInvalidAccessor(
Expand Down Expand Up @@ -1599,6 +1599,9 @@ public void GetModelStateForProperty_ReturnsModelStateForIndexedChildren()
Assert.Equal("value1", property.RawValue);
}

private DefaultBindingMetadataProvider CreateBindingMetadataProvider()
=> new DefaultBindingMetadataProvider();

private class OptionsAccessor : IOptions<MvcOptions>
{
public MvcOptions Value { get; } = new MvcOptions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
</PropertyGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)ParameterBindingMethodCache.cs" />
<Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public void Configure(MvcOptions options)
options.ModelBinderProviders.Add(new FloatingPointTypeModelBinderProvider());
options.ModelBinderProviders.Add(new EnumTypeModelBinderProvider(options));
options.ModelBinderProviders.Add(new DateTimeModelBinderProvider());
options.ModelBinderProviders.Add(new TryParseModelBinderProvider());
options.ModelBinderProviders.Add(new SimpleTypeModelBinderProvider());
options.ModelBinderProviders.Add(new CancellationTokenModelBinderProvider());
options.ModelBinderProviders.Add(new ByteArrayModelBinderProvider());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ Microsoft.AspNetCore.Mvc.RouteAttribute</Description>
<ItemGroup Label="Sources packages">
<Compile Include="$(SharedSourceRoot)ValueStopwatch\*.cs" />
<Compile Include="$(SharedSourceRoot)ParameterDefaultValue\*.cs" />
<Compile Include="$(SharedSourceRoot)TypeNameHelper\*.cs" />
</ItemGroup>

</Project>
Loading