diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 954e8c94a082..ca91d74468f2 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -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)); // The following code is generated if the parameter is required and // the method should not be matched. diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 0bbd48d1f9f3..7b56ccf6a70b 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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); diff --git a/src/Mvc/Mvc.Abstractions/src/Microsoft.AspNetCore.Mvc.Abstractions.csproj b/src/Mvc/Mvc.Abstractions/src/Microsoft.AspNetCore.Mvc.Abstractions.csproj index b277931317a3..b97d265a0fa3 100644 --- a/src/Mvc/Mvc.Abstractions/src/Microsoft.AspNetCore.Mvc.Abstractions.csproj +++ b/src/Mvc/Mvc.Abstractions/src/Microsoft.AspNetCore.Mvc.Abstractions.csproj @@ -13,6 +13,8 @@ Microsoft.AspNetCore.Mvc.IActionResult + + diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index bbcf91a97eda..f38550133f64 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -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; @@ -25,6 +27,8 @@ public abstract class ModelMetadata : IEquatable, IModelMetadata /// public static readonly int DefaultOrder = 10000; + private static readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); + private int? _hashCode; private IReadOnlyList? _boundProperties; private IReadOnlyDictionary? _parameterMapping; @@ -434,10 +438,10 @@ internal IReadOnlyDictionary BoundConstructorPrope /// /// /// A complex type is defined as a without a that can convert - /// from . Most POCO and types are therefore complex. Most, if - /// not all, BCL value types are simple types. + /// from and without a TryParse method. Most POCO and types are therefore complex. + /// Most, if not all, BCL value types are simple types. /// - public bool IsComplexType { get; private set; } + public bool IsComplexType => !IsConvertibleType && !IsParseableType; /// /// Gets a value indicating whether or not is a . @@ -475,6 +479,17 @@ internal IReadOnlyDictionary BoundConstructorPrope /// public Type UnderlyingOrModelType { get; private set; } = default!; + /// + /// Gets a value indicating whether or not has a TryParse method. + /// + internal virtual bool IsParseableType { get; private set; } + + /// + /// Gets a value indicating whether or not has a + /// from . + /// + internal bool IsConvertibleType { get; private set; } + /// /// Gets a value indicating the NullabilityState of the value or reference type. /// @@ -521,6 +536,22 @@ internal void ThrowIfRecordTypeHasValidationOnProperties() } } + internal static Func? FindTryParseMethod(Type modelType) + { + try + { + modelType = Nullable.GetUnderlyingType(modelType) ?? modelType; + return ParameterBindingMethodCache.FindTryParseMethod(modelType); + } + catch (InvalidOperationException) + { + // The ParameterBindingMethodCache.FindTryParseMethod throws an exception + // 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() { @@ -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; diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelStateDictionaryTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelStateDictionaryTest.cs index 70af69ea5a1d..6b8e91955734 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelStateDictionaryTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/ModelStateDictionaryTest.cs @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -1599,6 +1599,9 @@ public void GetModelStateForProperty_ReturnsModelStateForIndexedChildren() Assert.Equal("value1", property.RawValue); } + private DefaultBindingMetadataProvider CreateBindingMetadataProvider() + => new DefaultBindingMetadataProvider(); + private class OptionsAccessor : IOptions { public MvcOptions Value { get; } = new MvcOptions(); diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index b55223a8cef1..4b0491eb95ef 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -10,7 +10,6 @@ - diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs index 468018d37daa..82affa44c7b9 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs @@ -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()); diff --git a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj index a0b45ab9d83a..9d9b2ac58e91 100644 --- a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj @@ -58,7 +58,6 @@ Microsoft.AspNetCore.Mvc.RouteAttribute - diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinder.cs new file mode 100644 index 000000000000..5bb24d38f40e --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinder.cs @@ -0,0 +1,151 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Linq.Expressions; +using System.Reflection; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Core; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; + +/// +/// An for simple types. +/// +internal sealed class TryParseModelBinder : IModelBinder +{ + private static readonly MethodInfo AddModelErrorMethod = typeof(TryParseModelBinder).GetMethod(nameof(AddModelError), BindingFlags.NonPublic | BindingFlags.Static)!; + private static readonly MethodInfo SuccessBindingResultMethod = typeof(ModelBindingResult).GetMethod(nameof(ModelBindingResult.Success), BindingFlags.Public | BindingFlags.Static)!; + private static readonly ParameterExpression BindingContextExpression = Expression.Parameter(typeof(ModelBindingContext), "bindingContext"); + private static readonly ParameterExpression ValueProviderResultExpression = Expression.Parameter(typeof(ValueProviderResult), "valueProviderResult"); + private static readonly MemberExpression BindingResultExpression = Expression.Property(BindingContextExpression, nameof(ModelBindingContext.Result)); + private static readonly MemberExpression ValueExpression = Expression.Property(ValueProviderResultExpression, nameof(ValueProviderResult.FirstValue)); + private static readonly MemberExpression CultureExpression = Expression.Property(ValueProviderResultExpression, nameof(ValueProviderResult.Culture)); + + private readonly Func _tryParseOperation; + private readonly ILogger _logger; + + /// + /// Initializes a new instance of . + /// + /// The model type. + /// The . + public TryParseModelBinder(Type modelType, ILoggerFactory loggerFactory) + { + if (modelType == null) + { + throw new ArgumentNullException(nameof(modelType)); + } + + if (loggerFactory == null) + { + throw new ArgumentNullException(nameof(loggerFactory)); + } + + _tryParseOperation = CreateTryParseOperation(modelType); + _logger = loggerFactory.CreateLogger(); + } + + /// + public Task BindModelAsync(ModelBindingContext bindingContext) + { + if (bindingContext == null) + { + throw new ArgumentNullException(nameof(bindingContext)); + } + + _logger.AttemptingToBindModel(bindingContext); + + var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); + if (valueProviderResult == ValueProviderResult.None) + { + _logger.FoundNoValueInRequest(bindingContext); + + // no entry + _logger.DoneAttemptingToBindModel(bindingContext); + return Task.CompletedTask; + } + + bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); + + try + { + var value = valueProviderResult.FirstValue; + if (string.IsNullOrWhiteSpace(value)) + { + // Most TryParse() methods trim the value and fail if the result is empty. + + // When converting a null value may indicate a failed conversion for an otherwise required + // model (can't set a ValueType to null). This detects if a null model value is acceptable given the + // current bindingContext. If not, an error is logged. + if (!bindingContext.ModelMetadata.IsReferenceOrNullableType) + { + bindingContext.ModelState.TryAddModelError( + bindingContext.ModelName, + bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor( + valueProviderResult.ToString())); + } + else + { + bindingContext.Result = ModelBindingResult.Success(null); + } + } + else + { + _tryParseOperation(valueProviderResult, bindingContext); + } + } + catch (Exception exception) + { + // Conversion failed. + AddModelError(bindingContext, exception); + } + + _logger.DoneAttemptingToBindModel(bindingContext); + return Task.CompletedTask; + } + + private static void AddModelError(ModelBindingContext bindingContext, Exception exception) + { + // Conversion failed. + bindingContext.ModelState.TryAddModelError( + bindingContext.ModelName, + exception, + bindingContext.ModelMetadata); + } + + private static Func CreateTryParseOperation(Type modelType) + { + modelType = Nullable.GetUnderlyingType(modelType) ?? modelType; + var tryParseMethodExpession = ModelMetadata.FindTryParseMethod(modelType) + ?? throw new InvalidOperationException(Resources.FormatTryParseModelBinder_InvalidType(modelType, nameof(TryParseModelBinder))); + + // var tempSourceString = valueProviderResult.FirstValue; + // object model = null; + // if ([modeltype].TryParse(tempSourceString, [valueProviderResult.Culture,] out [modelType] parsedValue)) + // { + // model = (object)parsedValue; + // bindingContext.Result = ModelBindingResult.Success(model); + // } + // else + // { + // AddModelError(bindingContext, new FormatException()); + // } + // return model; + + var parsedValue = Expression.Variable(modelType, "parsedValue"); + var modelValue = Expression.Variable(typeof(object), "model"); + + var expression = Expression.Block( + new[] { parsedValue, modelValue, ParameterBindingMethodCache.TempSourceStringExpr }, + Expression.Assign(ParameterBindingMethodCache.TempSourceStringExpr, ValueExpression), + Expression.IfThenElse(tryParseMethodExpession(parsedValue, CultureExpression), + Expression.Block( + Expression.Assign(modelValue, Expression.Convert(parsedValue, modelValue.Type)), + Expression.Assign(BindingResultExpression, Expression.Call(SuccessBindingResultMethod, modelValue))), + Expression.Call(AddModelErrorMethod, BindingContextExpression, Expression.Constant(new FormatException()))), + modelValue); + + return Expression.Lambda>(expression, new[] { ValueProviderResultExpression, BindingContextExpression }).Compile(); + } +} diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinderProvider.cs new file mode 100644 index 000000000000..e41c21640f5c --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/TryParseModelBinderProvider.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; + +/// +/// An for binding types that have a TryParse method. +/// +public sealed class TryParseModelBinderProvider : IModelBinderProvider +{ + /// + public IModelBinder? GetBinder(ModelBinderProviderContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (context.Metadata.IsParseableType) + { + var loggerFactory = context.Services.GetRequiredService(); + return new TryParseModelBinder(context.Metadata.ModelType, loggerFactory); + } + + return null; + } +} diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index 5df7385255c2..e72c9d39ff92 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -3,6 +3,9 @@ Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.get -> bool Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.set -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService! serviceProviderIsService) -> void +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.GetBinder(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderContext! context) -> Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder? +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.TryParseModelBinderProvider() -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void diff --git a/src/Mvc/Mvc.Core/src/Resources.resx b/src/Mvc/Mvc.Core/src/Resources.resx index d3dc8246fc20..206a2f420b5c 100644 --- a/src/Mvc/Mvc.Core/src/Resources.resx +++ b/src/Mvc/Mvc.Core/src/Resources.resx @@ -1,17 +1,17 @@ - @@ -510,4 +510,7 @@ Could not parse '{0}'. Content types with wildcards are not supported. - + + The type '{0}' does not contain a TryParse method and the binder '{1}' cannot be used. + + \ No newline at end of file diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderProviderTest.cs new file mode 100644 index 000000000000..1cda72ab5fcd --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderProviderTest.cs @@ -0,0 +1,51 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using System.Net; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; + +public class TryParseModelBinderProviderTest +{ + [Fact] + public void Create_ForTypesWithoutTryParse_ReturnsNull() + { + // Arrange + var provider = new TryParseModelBinderProvider(); + var context = new TestModelBinderProviderContext(typeof(TestClass)); + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Create_ForTypesWithTryParse_ReturnsBinder() + { + // Arrange + var provider = new TryParseModelBinderProvider(); + var context = new TestModelBinderProviderContext(typeof(TestClassWithTryParse)); + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + + private class TestClass + { + } + + private class TestClassWithTryParse + { + public static bool TryParse(string s, out TestClassWithTryParse result) + { + result = new TestClassWithTryParse(); + return true; + } + } +} diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderTest.cs new file mode 100644 index 000000000000..73cb7bc55acd --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/TryParseTypeModelBinderTest.cs @@ -0,0 +1,559 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Logging.Testing; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders; + +public class TryParseTypeModelBinderTest +{ + public static TheoryData ConvertibleTypeData + { + get + { + var data = new TheoryData + { + typeof(byte), + typeof(short), + typeof(int), + typeof(long), + typeof(Guid), + typeof(double), + typeof(DayOfWeek), + }; + + // DateTimeOffset doesn't have a TypeConverter in Mono. + if (!TestPlatformHelper.IsMono) + { + data.Add(typeof(DateTimeOffset)); + } + + return data; + } + } + + [Theory] + [MemberData(nameof(ConvertibleTypeData))] + public async Task BindModel_ReturnsFailure_IfTypeCanBeConverted_AndConversionFails(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "some-value" } + }; + + var binder = CreateBinder(destinationType); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + } + + [Theory] + [MemberData(nameof(ConvertibleTypeData))] + public async Task BindModel_CreatesError_WhenTypeConversionIsNull(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", string.Empty } + }; + var binder = CreateBinder(destinationType); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal("The value '' is invalid.", error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Fact] + public async Task BindModel_Error_FormatExceptionsTurnedIntoStringsInModelState() + { + // Arrange + var message = "The value 'not an integer' is not valid."; + var bindingContext = GetBindingContext(typeof(int)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "not an integer" } + }; + + var binder = CreateBinder(typeof(int)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + Assert.False(bindingContext.ModelState.IsValid); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal(message, error.ErrorMessage); + } + + public static TheoryData IntegerModelMetadataDataSet + { + get + { + var metadataProvider = new EmptyModelMetadataProvider(); + var method = typeof(MetadataClass).GetMethod(nameof(MetadataClass.IsLovely)); + var parameter = method.GetParameters()[0]; // IsLovely(int parameter) + + return new TheoryData + { + metadataProvider.GetMetadataForParameter(parameter), + metadataProvider.GetMetadataForProperty(typeof(MetadataClass), nameof(MetadataClass.Property)), + metadataProvider.GetMetadataForType(typeof(int)), + }; + } + } + + [Theory] + [MemberData(nameof(IntegerModelMetadataDataSet))] + public async Task BindModel_EmptyValueProviderResult_ReturnsFailedAndLogsSuccessfully(ModelMetadata metadata) + { + // Arrange + var bindingContext = GetBindingContext(typeof(int)); + bindingContext.ModelMetadata = metadata; + + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var binder = CreateBinder(typeof(int), loggerFactory); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Equal(ModelBindingResult.Failed(), bindingContext.Result); + Assert.Empty(bindingContext.ModelState); + Assert.Equal(3, sink.Writes.Count()); + } + + [Fact] + public async Task BindModel_NullableIntegerValueProviderResult_ReturnsModel() + { + // Arrange + var bindingContext = GetBindingContext(typeof(int?)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "12" } + }; + + var binder = CreateBinder(typeof(int?)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal(12, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + [Fact] + public async Task BindModel_NullableDoubleValueProviderResult_ReturnsModel() + { + // Arrange + var bindingContext = GetBindingContext(typeof(double?)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "12.5" } + }; + + var binder = CreateBinder(typeof(double?)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal(12.5, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + [Theory] + [MemberData(nameof(IntegerModelMetadataDataSet))] + public async Task BindModel_ValidValueProviderResult_ReturnsModelAndLogsSuccessfully(ModelMetadata metadata) + { + // Arrange + var bindingContext = GetBindingContext(typeof(int)); + bindingContext.ModelMetadata = metadata; + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "42" } + }; + + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var binder = CreateBinder(typeof(int), loggerFactory); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal(42, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + Assert.Equal(2, sink.Writes.Count()); + } + + public static TheoryData BiggerNumericTypes + { + get + { + // Data set does not include bool, byte, sbyte, or char because they do not need thousands separators. + // Also does not include float point types (eg.: decimal, double, float) because for those we will use + // NumberStyles.Number that allow thousands operator + return new TheoryData + { + typeof(int), + typeof(long), + typeof(short), + typeof(uint), + typeof(ulong), + typeof(ushort), + }; + } + } + + [Theory] + [MemberData(nameof(BiggerNumericTypes))] + public async Task BindModel_ThousandsSeparators_LeadToErrors(Type type) + { + // Arrange + var bindingContext = GetBindingContext(type); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("en-GB")) + { + { "theModelName", "32,000" } + }; + + var binder = CreateBinder(type); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal("theModelName", entry.Key); + Assert.Equal("32,000", entry.Value.AttemptedValue); + Assert.Equal(ModelValidationState.Invalid, entry.Value.ValidationState); + + var error = Assert.Single(entry.Value.Errors); + Assert.Equal("The value '32,000' is not valid.", error.ErrorMessage); + Assert.Null(error.Exception); + } + + [Fact] + public async Task BindModel_ValidValueProviderResultWithProvidedCulture_ReturnsModel() + { + // Arrange + var bindingContext = GetBindingContext(typeof(decimal)); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) + { + { "theModelName", "12,5" } + }; + + var binder = CreateBinder(typeof(decimal)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal(12.5M, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + [Fact] + public async Task BindModel_CreatesErrorForFormatException_ValueProviderResultWithInvalidCulture() + { + // Arrange + var bindingContext = GetBindingContext(typeof(decimal)); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("en-GB")) + { + { "theModelName", "12-5" } + }; + + var binder = CreateBinder(typeof(decimal)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal("The value '12-5' is not valid.", error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Fact] + public async Task BindModel_BindsEnumModels_IfArrayElementIsStringKey() + { + // Arrange + var bindingContext = GetBindingContext(typeof(IntEnum)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", new object[] { "Value1" } } + }; + + var binder = CreateBinder(typeof(IntEnum)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(IntEnum.Value1, boundModel); + } + + [Fact] + public async Task BindModel_BindsEnumModels_IfArrayElementIsStringValue() + { + // Arrange + var bindingContext = GetBindingContext(typeof(IntEnum)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", new object[] { "1" } } + }; + + var binder = CreateBinder(typeof(IntEnum)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(IntEnum.Value1, boundModel); + } + + public static TheoryData EnumValues + { + get + { + return new TheoryData + { + { "0", 0 }, + { "1", 1 }, + { "13", 13 }, + { "Value1", 1 }, + { "Value1, Value2", 3 }, + }; + } + } + + [Theory] + [MemberData(nameof(EnumValues))] + public async Task BindModel_BindsIntEnumModels(string flagsEnumValue, int expected) + { + // Arrange + var bindingContext = GetBindingContext(typeof(IntEnum)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", flagsEnumValue } + }; + + var binder = CreateBinder(typeof(IntEnum)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal((IntEnum)expected, boundModel); + } + + [Theory] + [InlineData("32,015")] + [InlineData("32,128")] + public async Task BindModel_CreatesErrorForFormatException_BindsIntEnumModels(string flagsEnumValue) + { + // Arrange + var bindingContext = GetBindingContext(typeof(FlagsEnum)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", flagsEnumValue } + }; + + var binder = CreateBinder(typeof(FlagsEnum)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Different than the Converter when calling the TryParse the values will + // NOT be treat as two separate enum values that are or'd together + // Assert + Assert.False(bindingContext.Result.IsModelSet); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal($"The value '{flagsEnumValue}' is not valid.", error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Theory] + [InlineData("~10~", 10)] + [InlineData("~5", 5)] + [InlineData("aaaa~1~aaaa", 1)] + public async Task BindModel_BindClassWithTryParseMethod(string value, int expected) + { + // Arrange + var bindingContext = GetBindingContext(typeof(TestTryParseClass)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value } + }; + + var binder = CreateBinder(typeof(TestTryParseClass)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(expected, boundModel.Id); + } + + [Theory] + [InlineData("10")] + [InlineData("~~0")] + public async Task BindModel_CreatesErrorForFormatException_BindClassWithTryParseMethod(string value) + { + // Arrange + var bindingContext = GetBindingContext(typeof(TestTryParseClass)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value } + }; + + var binder = CreateBinder(typeof(TestTryParseClass)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Different than the Converter when calling the TryParse the values will + // NOT be treat as two separate enum values that are or'd together + // Assert + Assert.False(bindingContext.Result.IsModelSet); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal($"The value '{value}' is not valid.", error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Theory] + [MemberData(nameof(EnumValues))] + [InlineData("Value8, Value4", 12)] + public async Task BindModel_BindsFlagsEnumModels(string flagsEnumValue, int expected) + { + // Arrange + var bindingContext = GetBindingContext(typeof(FlagsEnum)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", flagsEnumValue } + }; + + var binder = CreateBinder(typeof(FlagsEnum)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal((FlagsEnum)expected, boundModel); + } + + [Fact] + public void BindModel_ThrowsInvalidOperationException_WhenTryParseNotFound() + { + // Act & assert + Assert.Throws(() => new TryParseModelBinder(typeof(TestClass), NullLoggerFactory.Instance)); + } + + private static DefaultModelBindingContext GetBindingContext(Type modelType) + { + return new DefaultModelBindingContext + { + ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(modelType), + ModelName = "theModelName", + ModelState = new ModelStateDictionary(), + ValueProvider = new SimpleValueProvider() // empty + }; + } + + private static IModelBinder CreateBinder(Type modelType, ILoggerFactory loggerFactory = null) => + new TryParseModelBinder(modelType, loggerFactory ?? NullLoggerFactory.Instance); + + private sealed class TestClass + { + } + + private sealed class TestTryParseClass + { + public int? Id { get; set; } + + public static bool TryParse(string s, out TestTryParseClass result) + { + result = new TestTryParseClass(); + + if (!string.IsNullOrWhiteSpace(s)) + { + var tokens = s.Split('~'); + + if (tokens.Length >= 2) + { + result.Id = int.Parse(tokens[1], CultureInfo.CurrentCulture); + return true; + } + } + + return false; + + } + } + + [Flags] + private enum FlagsEnum + { + Value1 = 1, + Value2 = 2, + Value4 = 4, + Value8 = 8, + } + + private enum IntEnum + { + Value0 = 0, + Value1 = 1, + Value2 = 2, + MaxValue = int.MaxValue + } + + private class MetadataClass + { + public int Property { get; set; } + + public bool IsLovely(int parameter) + { + return true; + } + } +} diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs index ac8cba12279d..ab2feac0f41c 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultBindingMetadataProviderTest.cs @@ -22,7 +22,7 @@ public void CreateBindingDetails_FindsBinderTypeProvider() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -46,7 +46,7 @@ public void CreateBindingDetails_FindsBinderTypeProvider_IfNullFallsBack() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -69,7 +69,7 @@ public void CreateBindingDetails_FindsModelName() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -93,7 +93,7 @@ public void CreateBindingDetails_FindsModelName_IfNullFallsBack() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -116,7 +116,7 @@ public void CreateBindingDetails_FindsBindingSource() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -140,7 +140,7 @@ public void CreateBindingDetails_FindsBindingSource_IfNullFallsBack() ModelMetadataIdentity.ForType(typeof(string)), new ModelAttributes(attributes, null, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -162,7 +162,7 @@ public void CreateBindingDetails_FindsBindingBehaviorNever_OnProperty() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -185,7 +185,7 @@ public void CreateBindingDetails_FindsBindNever_OnProperty() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -208,7 +208,7 @@ public void CreateBindingDetails_FindsBindingBehaviorOptional_OnProperty() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -231,7 +231,7 @@ public void CreateBindingDetails_FindsBindingBehaviorRequired_OnProperty() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -254,7 +254,7 @@ public void CreateBindingDetails_FindsBindRequired_OnProperty() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -277,7 +277,7 @@ public void CreateBindingDetails_FindsBindingBehaviorNever_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -300,7 +300,7 @@ public void CreateBindingDetails_FindsBindNever_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -323,7 +323,7 @@ public void CreateBindingDetails_FindsBindingBehaviorOptional_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -346,7 +346,7 @@ public void CreateBindingDetails_FindsBindingBehaviorRequired_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -369,7 +369,7 @@ public void CreateBindingDetails_FindsBindRequired_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -393,7 +393,7 @@ public void CreateBindingDetails_FindsCustomAttributes_OnParameter() ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), new ModelAttributes(Array.Empty(), null, parameterAttributes)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -421,7 +421,7 @@ public void CreateBindingDetails_UsesFirstAttribute() ModelMetadataIdentity.ForProperty(typeof(string).GetProperty(nameof(string.Length)), typeof(int), typeof(string)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -439,7 +439,7 @@ public void CreateBindingDetails_FindsBindRequired_OnContainerClass() 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(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -457,7 +457,7 @@ public void CreateBindingDetails_FindsBindNever_OnContainerClass() 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(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -475,7 +475,7 @@ public void CreateBindingDetails_FindsBindNever_OnBaseClass() 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(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -498,7 +498,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithOptional() ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -521,7 +521,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithRequired() ModelMetadataIdentity.ForProperty(typeof(BindNeverOnClass).GetProperty(nameof(BindNeverOnClass.Property)), typeof(int), typeof(BindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -544,7 +544,7 @@ public void CreateBindingDetails_OverrideInheritedBehaviorOnClass_OverrideWithRe ModelMetadataIdentity.ForProperty(typeof(InheritedBindNeverOnClass).GetProperty(nameof(InheritedBindNeverOnClass.Property)), typeof(int), typeof(InheritedBindNeverOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -567,7 +567,7 @@ public void CreateBindingDetails_OverrideBehaviorOnClass_OverrideWithNever() ModelMetadataIdentity.ForProperty(typeof(BindRequiredOnClass).GetProperty(nameof(BindRequiredOnClass.Property)), typeof(int), typeof(BindRequiredOnClass)), new ModelAttributes(new object[0], propertyAttributes, null)); - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -586,7 +586,7 @@ public void CreateBindingDetails_OverrideBehaviorOnBaseClass_OverrideWithRequire 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(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -615,7 +615,7 @@ public void CreateBindingDetails_BindingBehaviorLeftAlone_ForTypeMetadata(bool i context.BindingMetadata.IsBindingAllowed = initialValue; context.BindingMetadata.IsBindingRequired = initialValue; - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -646,7 +646,7 @@ public void CreateBindingDetails_BindingBehaviorLeftAlone_ForAttributeOnProperty context.BindingMetadata.IsBindingAllowed = initialValue; context.BindingMetadata.IsBindingRequired = initialValue; - var provider = new DefaultBindingMetadataProvider(); + var provider = CreateBindingMetadataProvider(); // Act provider.CreateBindingMetadata(context); @@ -913,4 +913,7 @@ private class CustomAttribute : Attribute { public string Identifier { get; set; } } + + private DefaultBindingMetadataProvider CreateBindingMetadataProvider() + => new DefaultBindingMetadataProvider(); } diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs index 2d7329995f31..9eb9682e6b96 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -339,6 +339,56 @@ public void IsBindingRequired_ReturnsFalse_ForTypes(Type modelType) Assert.False(isBindingRequired); } + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(int?))] + [InlineData(typeof(Guid))] + [InlineData(typeof(DateTime))] + [InlineData(typeof(DateTime?))] + [InlineData(typeof(System.Net.IPEndPoint))] + [InlineData(typeof(TypeWithTryParse))] + [InlineData(typeof(TypeWithTryParseAndIFormatProvider))] + public void IsParseableType_ReturnsTrue_ForParseableTypes(Type modelType) + { + // Arrange + var provider = new EmptyModelMetadataProvider(); + var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); + + var key = ModelMetadataIdentity.ForType(modelType); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(Array.Empty(), null, null)); + + var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); + + // Act + var isParseableType = metadata.IsParseableType; + + // Assert + Assert.True(isParseableType); + } + + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(object))] + [InlineData(typeof(TypeWithInvalidTryParse))] + [InlineData(typeof(TypeWithProperties))] + public void IsParseableType_ReturnsFalse_ForNonParseableTypes(Type modelType) + { + // Arrange + var provider = new EmptyModelMetadataProvider(); + var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); + + var key = ModelMetadataIdentity.ForType(modelType); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(Array.Empty(), null, null)); + + var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); + + // Act + var isParseableType = metadata.IsParseableType; + + // Assert + Assert.False(isParseableType); + } + [Theory] [InlineData(typeof(string))] [InlineData(typeof(IDisposable))] @@ -1543,6 +1593,32 @@ private class TypeWithProperties public int PublicGetPublicSetProperty { get; set; } } + private class TypeWithInvalidTryParse + { + public static bool TryParse(string s) + { + return true; + } + } + + private class TypeWithTryParse + { + public static bool TryParse(string s, out TypeWithTryParse result) + { + result = new TypeWithTryParse(); + return true; + } + } + + private class TypeWithTryParseAndIFormatProvider + { + public static bool TryParse(string s, IFormatProvider provider, out TypeWithTryParseAndIFormatProvider result) + { + result = new TypeWithTryParseAndIFormatProvider(); + return true; + } + } + public class Employee { public int Id { get; set; } diff --git a/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs b/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs index de091e37a918..79c83b8ec421 100644 --- a/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs +++ b/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs @@ -52,6 +52,7 @@ public void Setup_SetsUpModelBinderProviders() binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder), + binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder), diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 810dd7923618..2129988bb2d7 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -28,7 +28,7 @@ internal sealed class ParameterBindingMethodCache private readonly MethodInfo _enumTryParseMethod; // Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( - private readonly ConcurrentDictionary?> _stringMethodCallCache = new(); + private readonly ConcurrentDictionary?> _stringMethodCallCache = new(); private readonly ConcurrentDictionary?, int)> _bindAsyncMethodCallCache = new(); // If IsDynamicCodeSupported is false, we can't use the static Enum.TryParse since there's no easy way for @@ -52,9 +52,9 @@ public bool HasTryParseMethod(Type type) public bool HasBindAsyncMethod(ParameterInfo parameter) => FindBindAsyncMethod(parameter).Expression is not null; - public Func? FindTryParseMethod(Type type) + public Func? FindTryParseMethod(Type type) { - Func? Finder(Type type) + Func? Finder(Type type) { MethodInfo? methodInfo; @@ -64,10 +64,10 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => { methodInfo = _enumTryParseMethod.MakeGenericMethod(type); - return (expression) => Expression.Call(methodInfo!, TempSourceStringExpr, expression); + return (expression, formatProvider) => Expression.Call(methodInfo!, TempSourceStringExpr, expression); } - return (expression) => + return (expression, formatProvider) => { var enumAsObject = Expression.Variable(typeof(object), "enumAsObject"); var success = Expression.Variable(typeof(bool), "success"); @@ -108,21 +108,21 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => _ => DateTimeStyles.AllowWhiteSpaces }; - return (expression) => Expression.Call( + return (expression, formatProvider) => Expression.Call( methodInfo!, TempSourceStringExpr, - Expression.Constant(CultureInfo.InvariantCulture), + formatProvider, Expression.Constant(dateTimeStyles), expression); } if (TryGetNumberStylesTryGetMethod(type, out methodInfo, out var numberStyle)) { - return (expression) => Expression.Call( + return (expression, formatProvider) => Expression.Call( methodInfo!, TempSourceStringExpr, Expression.Constant(numberStyle), - Expression.Constant(CultureInfo.InvariantCulture), + formatProvider, expression); } @@ -130,10 +130,10 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => if (methodInfo is not null) { - return (expression) => Expression.Call( + return (expression, formatProvider) => Expression.Call( methodInfo, TempSourceStringExpr, - Expression.Constant(CultureInfo.InvariantCulture), + formatProvider, expression); } @@ -141,7 +141,7 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => if (methodInfo is not null) { - return (expression) => Expression.Call(methodInfo, TempSourceStringExpr, expression); + return (expression, formatProvider) => Expression.Call(methodInfo, TempSourceStringExpr, expression); } if (GetAnyMethodFromHierarchy(type, "TryParse") is MethodInfo invalidMethod)