From fd50e5dbf44568a2a2115bcf4e5523319c482030 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 13 Aug 2020 16:21:16 -0700 Subject: [PATCH 1/4] Add support for model binding DateTime as UTC Fixes https://github.com/dotnet/aspnetcore/issues/11584 --- .../Infrastructure/MvcCoreMvcOptionsSetup.cs | 1 + .../Binders/DateTimeModelBinder.cs | 117 +++++++++ .../Binders/DateTimeModelBinderProvider.cs | 36 +++ src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 6 + .../DateTimeModelBinderProviderTest.cs | 57 +++++ .../Binders/DateTimeModelBinderTest.cs | 226 ++++++++++++++++++ src/Mvc/Mvc/test/MvcOptionsSetupTest.cs | 1 + .../SimpleTypeModelBinderIntegrationTest.cs | 87 +++++++ 8 files changed, 531 insertions(+) create mode 100644 src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs create mode 100644 src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs create mode 100644 src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderProviderTest.cs create mode 100644 src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs index a2b94da1188e..c12f5af2a381 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 HeaderModelBinderProvider()); options.ModelBinderProviders.Add(new FloatingPointTypeModelBinderProvider()); options.ModelBinderProviders.Add(new EnumTypeModelBinderProvider(options)); + options.ModelBinderProviders.Add(new DateTimeModelBinderProvider()); options.ModelBinderProviders.Add(new SimpleTypeModelBinderProvider()); options.ModelBinderProviders.Add(new CancellationTokenModelBinderProvider()); options.ModelBinderProviders.Add(new ByteArrayModelBinderProvider()); diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs new file mode 100644 index 000000000000..51274b2d8077 --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs @@ -0,0 +1,117 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using System.Runtime.ExceptionServices; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + /// + /// An for and nullable models. + /// + public class DateTimeModelBinder : IModelBinder + { + private readonly DateTimeStyles _supportedStyles; + private readonly ILogger _logger; + + /// + /// Initializes a new instance of . + /// + /// The . + /// The . + public DateTimeModelBinder(DateTimeStyles supportedStyles, ILoggerFactory loggerFactory) + { + if (loggerFactory == null) + { + throw new ArgumentNullException(nameof(loggerFactory)); + } + + _supportedStyles = supportedStyles; + _logger = loggerFactory.CreateLogger(); + } + + /// + public Task BindModelAsync(ModelBindingContext bindingContext) + { + if (bindingContext == null) + { + throw new ArgumentNullException(nameof(bindingContext)); + } + + _logger.AttemptingToBindModel(bindingContext); + + var modelName = bindingContext.ModelName; + var valueProviderResult = bindingContext.ValueProvider.GetValue(modelName); + if (valueProviderResult == ValueProviderResult.None) + { + _logger.FoundNoValueInRequest(bindingContext); + + // no entry + _logger.DoneAttemptingToBindModel(bindingContext); + return Task.CompletedTask; + } + + var modelState = bindingContext.ModelState; + modelState.SetModelValue(modelName, valueProviderResult); + + var metadata = bindingContext.ModelMetadata; + var type = metadata.UnderlyingOrModelType; + try + { + var value = valueProviderResult.FirstValue; + var culture = valueProviderResult.Culture; + + object model; + if (string.IsNullOrWhiteSpace(value)) + { + // Parse() method trims the value (with common DateTimeSyles) then throws if the result is empty. + model = null; + } + else if (type == typeof(DateTime)) + { + model = DateTime.Parse(value, culture, _supportedStyles); + } + else + { + // unreachable + throw new NotSupportedException(); + } + + // When converting value, a null model 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 (model == null && !metadata.IsReferenceOrNullableType) + { + modelState.TryAddModelError( + modelName, + metadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor( + valueProviderResult.ToString())); + } + else + { + bindingContext.Result = ModelBindingResult.Success(model); + } + } + catch (Exception exception) + { + var isFormatException = exception is FormatException; + if (!isFormatException && exception.InnerException != null) + { + // Unlike TypeConverters, floating point types do not seem to wrap FormatExceptions. Preserve + // this code in case a cursory review of the CoreFx code missed something. + exception = ExceptionDispatchInfo.Capture(exception.InnerException).SourceException; + } + + modelState.TryAddModelError(modelName, exception, metadata); + + // Conversion failed. + } + + _logger.DoneAttemptingToBindModel(bindingContext); + return Task.CompletedTask; + } + } +} diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs new file mode 100644 index 000000000000..73da65bba8d7 --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + /// + /// An for binding and nullable models. + /// + public class DateTimeModelBinderProvider : IModelBinderProvider + { + internal static readonly DateTimeStyles SupportedStyles = DateTimeStyles.AdjustToUniversal | DateTimeStyles.AllowWhiteSpaces; + + /// + public IModelBinder GetBinder(ModelBinderProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var modelType = context.Metadata.UnderlyingOrModelType; + var loggerFactory = context.Services.GetRequiredService(); + if (modelType == typeof(DateTime)) + { + return new DateTimeModelBinder(SupportedStyles, loggerFactory); + } + + return null; + } + } +} diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index b122b73175a7..cff0b37416e6 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -463,6 +463,9 @@ Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexObjectModelBinderProvider.C Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinderProvider Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinderProvider.ComplexTypeModelBinderProvider() -> void +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinder +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinderProvider +Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinderProvider.DateTimeModelBinderProvider() -> void Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DecimalModelBinder Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DictionaryModelBinder Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DictionaryModelBinderProvider @@ -1464,6 +1467,9 @@ virtual Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.Visit ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder.ComplexTypeModelBinder(System.Collections.Generic.IDictionary propertyBinders, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder.ComplexTypeModelBinder(System.Collections.Generic.IDictionary propertyBinders, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, bool allowValidatingTopLevelNodes) -> void ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinderProvider.GetBinder(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderContext context) -> Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder +~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinder.BindModelAsync(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBindingContext bindingContext) -> System.Threading.Tasks.Task +~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinder.DateTimeModelBinder(System.Globalization.DateTimeStyles supportedStyles, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void +~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DateTimeModelBinderProvider.GetBinder(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderContext context) -> Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DecimalModelBinder.BindModelAsync(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBindingContext bindingContext) -> System.Threading.Tasks.Task ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DecimalModelBinder.DecimalModelBinder(System.Globalization.NumberStyles supportedStyles, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void ~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DictionaryModelBinder.DictionaryModelBinder(Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder keyBinder, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder valueBinder, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderProviderTest.cs new file mode 100644 index 000000000000..376d3b6765fc --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderProviderTest.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + public class DateTimeModelBinderProviderTest + { + private readonly DateTimeModelBinderProvider _provider = new DateTimeModelBinderProvider(); + + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(DateTimeOffset))] + [InlineData(typeof(DateTimeOffset?))] + [InlineData(typeof(TimeSpan))] + public void Create_ForNonDateTime_ReturnsNull(Type modelType) + { + // Arrange + var context = new TestModelBinderProviderContext(modelType); + + // Act + var result = _provider.GetBinder(context); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Create_ForDateTime_ReturnsBinder() + { + // Arrange + var context = new TestModelBinderProviderContext(typeof(DateTime)); + + // Act + var result = _provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + + [Fact] + public void Create_ForNullableDateTime_ReturnsBinder() + { + // Arrange + var context = new TestModelBinderProviderContext(typeof(DateTime?)); + + // Act + var result = _provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + } +} diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs new file mode 100644 index 000000000000..2602a27fd4bc --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs @@ -0,0 +1,226 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + public class DateTimeModelBinderTest + { + [Fact] + public async Task BindModel_ReturnsFailure_IfAttemptedValueCannotBeParsed() + { + // Arrange + var bindingContext = GetBindingContext(); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "some-value" } + }; + var binder = GetBinder(); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + } + + [Fact] + public async Task BindModel_CreatesError_IfAttemptedValueCannotBeParsed() + { + // Arrange + var message = "The value 'not a number' is not valid."; + var bindingContext = GetBindingContext(); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", "not a number" }, + }; + var binder = GetBinder(); + + // 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); + } + + [Fact] + public async Task BindModel_CreatesError_IfAttemptedValueCannotBeCompletelyParsed() + { + // Arrange + var bindingContext = GetBindingContext(); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("en-GB")) + { + { "theModelName", "2020-08-not-a-date" } + }; + var binder = GetBinder(); + + // 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 '2020-08-not-a-date' is not valid.", error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Fact] + public async Task BindModel_ReturnsFailed_IfValueProviderEmpty() + { + // Arrange + var bindingContext = GetBindingContext(typeof(DateTime)); + var binder = GetBinder(); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Equal(ModelBindingResult.Failed(), bindingContext.Result); + Assert.Empty(bindingContext.ModelState); + } + + [Fact] + public async Task BindModel_NullableDatetime_ReturnsFailed_IfValueProviderEmpty() + { + // Arrange + var bindingContext = GetBindingContext(typeof(DateTime?)); + var binder = GetBinder(); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Equal(ModelBindingResult.Failed(), bindingContext.Result); + Assert.Empty(bindingContext.ModelState); + } + + [Theory] + [InlineData("")] + [InlineData(" \t \r\n ")] + public async Task BindModel_CreatesError_IfTrimmedAttemptedValueIsEmpty_NonNullableDestination(string value) + { + // Arrange + var message = $"The value '{value}' is invalid."; + var bindingContext = GetBindingContext(); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value }, + }; + var binder = GetBinder(); + + // 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(message, error.ErrorMessage, StringComparer.Ordinal); + Assert.Null(error.Exception); + } + + [Theory] + [InlineData("")] + [InlineData(" \t \r\n ")] + public async Task BindModel_ReturnsNull_IfTrimmedAttemptedValueIsEmpty_NullableDestination(string value) + { + // Arrange + var bindingContext = GetBindingContext(typeof(DateTime?)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value } + }; + var binder = GetBinder(); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Null(bindingContext.Result.Model); + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal("theModelName", entry.Key); + } + + [Theory] + [InlineData(typeof(DateTime))] + [InlineData(typeof(DateTime?))] + public async Task BindModel_ReturnsModel_IfAttemptedValueIsValid(Type type) + { + // Arrange + var expected = new DateTime(2019, 06, 14, 2, 30, 4, 0, DateTimeKind.Utc); + var bindingContext = GetBindingContext(type); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) + { + { "theModelName", "2019-06-14T02:30:04.0Z" } + }; + var binder = GetBinder(); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var model = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(expected, model); + Assert.Equal(DateTimeKind.Utc, model.Kind); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + [Fact] + public async Task UsesSpecifiedStyleToParseModel() + { + // Arrange + var bindingContext = GetBindingContext(); + var expected = new DateTime(2019, 06, 13, 19, 30, 4, 0, DateTimeKind.Local); + bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) + { + { "theModelName", "2019-06-14T02:30:04.0Z" } + }; + var binder = GetBinder(DateTimeStyles.AssumeLocal); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var model = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(expected, model); + Assert.Equal(DateTimeKind.Local, model.Kind); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + private IModelBinder GetBinder(DateTimeStyles? dateTimeStyles = null) + { + return new DateTimeModelBinder(dateTimeStyles ?? DateTimeModelBinderProvider.SupportedStyles, NullLoggerFactory.Instance); + } + + private static DefaultModelBindingContext GetBindingContext(Type modelType = null) + { + modelType ??= typeof(DateTime); + return new DefaultModelBindingContext + { + ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(modelType), + ModelName = "theModelName", + ModelState = new ModelStateDictionary(), + ValueProvider = new SimpleValueProvider() // empty + }; + } + + private sealed class TestClass + { + } + } +} diff --git a/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs b/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs index eb8d0933b48c..b0a740c1f945 100644 --- a/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs +++ b/src/Mvc/Mvc/test/MvcOptionsSetupTest.cs @@ -58,6 +58,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/Mvc/test/Mvc.IntegrationTests/SimpleTypeModelBinderIntegrationTest.cs b/src/Mvc/test/Mvc.IntegrationTests/SimpleTypeModelBinderIntegrationTest.cs index 62f3c3eb9fa8..634ba9bcedbb 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/SimpleTypeModelBinderIntegrationTest.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/SimpleTypeModelBinderIntegrationTest.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -229,6 +231,91 @@ public async Task BindDecimalParameter_WithData_GetsBound() Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); } + [Fact] + [ReplaceCulture("en-GB", "en-GB")] + public async Task BindDateTimeParameter_WithData_GetsBound() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + ParameterType = typeof(DateTime), + BindingInfo = new BindingInfo(), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = QueryString.Create("Parameter1", "2020-02-01"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(new DateTime(2020, 02, 01, 0, 0, 0, DateTimeKind.Utc), model); + + // ModelState + Assert.True(modelState.IsValid); + + Assert.Single(modelState.Keys); + var key = Assert.Single(modelState.Keys); + Assert.Equal("Parameter1", key); + Assert.Equal("2020-02-01", modelState[key].AttemptedValue); + Assert.Equal("2020-02-01", modelState[key].RawValue); + Assert.Empty(modelState[key].Errors); + Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + } + + [Fact] + [ReplaceCulture("en-GB", "en-GB")] + public async Task BindDateTimeParameter_WithDataFromBody_GetsBound() + { + // Arrange + var input = "\"2020-02-01\""; + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + ParameterType = typeof(DateTime), + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Body, + } + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(input)); + request.ContentType = "application/json"; + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(new DateTime(2020, 02, 01, 0, 0, 0, DateTimeKind.Utc), model); + + // ModelState + Assert.True(modelState.IsValid); + } + [Fact] public async Task BindParameter_WithMultipleValues_GetsBoundToFirstValue() { From 559ad78ec8c2e7a8860947ebd54c4e467068fad8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 13 Aug 2020 19:10:48 -0700 Subject: [PATCH 2/4] Make test work in other TZs --- .../test/ModelBinding/Binders/DateTimeModelBinderTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs index 2602a27fd4bc..7de3204c6f80 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs @@ -184,7 +184,7 @@ public async Task UsesSpecifiedStyleToParseModel() { // Arrange var bindingContext = GetBindingContext(); - var expected = new DateTime(2019, 06, 13, 19, 30, 4, 0, DateTimeKind.Local); + var expected = DateTime.Parse("2019-06-14T02:30:04.0Z"); bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) { { "theModelName", "2019-06-14T02:30:04.0Z" } From efe27b0506ef8802c3687d8525a4de473563d64d Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 14 Aug 2020 07:23:14 -0700 Subject: [PATCH 3/4] Changes per PR comments * Cleanup unused exception code path, fix doc comments * Clean up usage of variables * Adjust logging to be consistent --- .../Binders/DateTimeModelBinder.cs | 20 +++++-------------- .../Binders/DateTimeModelBinderProvider.cs | 2 +- .../Binders/DecimalModelBinder.cs | 3 +-- .../ModelBinding/Binders/DoubleModelBinder.cs | 3 +-- .../ModelBinding/Binders/FloatModelBinder.cs | 3 +-- .../Binders/SimpleTypeModelBinder.cs | 4 ++-- .../Binders/DateTimeModelBinderTest.cs | 14 +++++-------- .../Binders/SimpleTypeModelBinderTest.cs | 2 +- 8 files changed, 17 insertions(+), 34 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs index 51274b2d8077..2e4c13341bfe 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs @@ -3,7 +3,6 @@ using System; using System.Globalization; -using System.Runtime.ExceptionServices; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -18,9 +17,9 @@ public class DateTimeModelBinder : IModelBinder private readonly ILogger _logger; /// - /// Initializes a new instance of . + /// Initializes a new instance of . /// - /// The . + /// The . /// The . public DateTimeModelBinder(DateTimeStyles supportedStyles, ILoggerFactory loggerFactory) { @@ -61,8 +60,8 @@ public Task BindModelAsync(ModelBindingContext bindingContext) var type = metadata.UnderlyingOrModelType; try { + var value = valueProviderResult.FirstValue; - var culture = valueProviderResult.Culture; object model; if (string.IsNullOrWhiteSpace(value)) @@ -72,7 +71,7 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } else if (type == typeof(DateTime)) { - model = DateTime.Parse(value, culture, _supportedStyles); + model = DateTime.Parse(value, valueProviderResult.Culture, _supportedStyles); } else { @@ -97,17 +96,8 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } catch (Exception exception) { - var isFormatException = exception is FormatException; - if (!isFormatException && exception.InnerException != null) - { - // Unlike TypeConverters, floating point types do not seem to wrap FormatExceptions. Preserve - // this code in case a cursory review of the CoreFx code missed something. - exception = ExceptionDispatchInfo.Capture(exception.InnerException).SourceException; - } - - modelState.TryAddModelError(modelName, exception, metadata); - // Conversion failed. + modelState.TryAddModelError(modelName, exception, metadata); } _logger.DoneAttemptingToBindModel(bindingContext); diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs index 73da65bba8d7..aad9323f193d 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinderProvider.cs @@ -24,9 +24,9 @@ public IModelBinder GetBinder(ModelBinderProviderContext context) } var modelType = context.Metadata.UnderlyingOrModelType; - var loggerFactory = context.Services.GetRequiredService(); if (modelType == typeof(DateTime)) { + var loggerFactory = context.Services.GetRequiredService(); return new DateTimeModelBinder(SupportedStyles, loggerFactory); } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DecimalModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DecimalModelBinder.cs index b8f27116b568..83001eb70627 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DecimalModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DecimalModelBinder.cs @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) try { var value = valueProviderResult.FirstValue; - var culture = valueProviderResult.Culture; object model; if (string.IsNullOrWhiteSpace(value)) @@ -73,7 +72,7 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } else if (type == typeof(decimal)) { - model = decimal.Parse(value, _supportedStyles, culture); + model = decimal.Parse(value, _supportedStyles, valueProviderResult.Culture); } else { diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DoubleModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DoubleModelBinder.cs index 64dd08301a4f..27e7417bd946 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DoubleModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DoubleModelBinder.cs @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) try { var value = valueProviderResult.FirstValue; - var culture = valueProviderResult.Culture; object model; if (string.IsNullOrWhiteSpace(value)) @@ -73,7 +72,7 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } else if (type == typeof(double)) { - model = double.Parse(value, _supportedStyles, culture); + model = double.Parse(value, _supportedStyles, valueProviderResult.Culture); } else { diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/FloatModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/FloatModelBinder.cs index 09a150213b08..733d8e28c92c 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/FloatModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/FloatModelBinder.cs @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) try { var value = valueProviderResult.FirstValue; - var culture = valueProviderResult.Culture; object model; if (string.IsNullOrWhiteSpace(value)) @@ -73,7 +72,7 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } else if (type == typeof(float)) { - model = float.Parse(value, _supportedStyles, culture); + model = float.Parse(value, _supportedStyles, valueProviderResult.Culture); } else { diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/SimpleTypeModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/SimpleTypeModelBinder.cs index 8d9b1eeee99a..2bf1af977b55 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/SimpleTypeModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/SimpleTypeModelBinder.cs @@ -46,6 +46,8 @@ public Task BindModelAsync(ModelBindingContext bindingContext) throw new ArgumentNullException(nameof(bindingContext)); } + _logger.AttemptingToBindModel(bindingContext); + var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName); if (valueProviderResult == ValueProviderResult.None) { @@ -56,8 +58,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) return Task.CompletedTask; } - _logger.AttemptingToBindModel(bindingContext); - bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); try diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs index 7de3204c6f80..2fd87e551ab1 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/DateTimeModelBinderTest.cs @@ -33,11 +33,11 @@ public async Task BindModel_ReturnsFailure_IfAttemptedValueCannotBeParsed() public async Task BindModel_CreatesError_IfAttemptedValueCannotBeParsed() { // Arrange - var message = "The value 'not a number' is not valid."; + var message = "The value 'not a date' is not valid."; var bindingContext = GetBindingContext(); bindingContext.ValueProvider = new SimpleValueProvider { - { "theModelName", "not a number" }, + { "theModelName", "not a date" }, }; var binder = GetBinder(); @@ -164,7 +164,7 @@ public async Task BindModel_ReturnsModel_IfAttemptedValueIsValid(Type type) var bindingContext = GetBindingContext(type); bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) { - { "theModelName", "2019-06-14T02:30:04.0Z" } + { "theModelName", "2019-06-14T02:30:04.0000000Z" } }; var binder = GetBinder(); @@ -184,10 +184,10 @@ public async Task UsesSpecifiedStyleToParseModel() { // Arrange var bindingContext = GetBindingContext(); - var expected = DateTime.Parse("2019-06-14T02:30:04.0Z"); + var expected = DateTime.Parse("2019-06-14T02:30:04.0000000Z"); bindingContext.ValueProvider = new SimpleValueProvider(new CultureInfo("fr-FR")) { - { "theModelName", "2019-06-14T02:30:04.0Z" } + { "theModelName", "2019-06-14T02:30:04.0000000Z" } }; var binder = GetBinder(DateTimeStyles.AssumeLocal); @@ -218,9 +218,5 @@ private static DefaultModelBindingContext GetBindingContext(Type modelType = nul ValueProvider = new SimpleValueProvider() // empty }; } - - private sealed class TestClass - { - } } } diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs index e50098b146f3..ee5c372cd095 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs @@ -194,7 +194,7 @@ public async Task BindModel_EmptyValueProviderResult_ReturnsFailedAndLogsSuccess // Assert Assert.Equal(ModelBindingResult.Failed(), bindingContext.Result); Assert.Empty(bindingContext.ModelState); - Assert.Equal(2, sink.Writes.Count()); + Assert.Equal(3, sink.Writes.Count()); } [Theory] From 70bb448bb2cd7fc75c657a8dba91799fb9537129 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 17 Aug 2020 11:17:04 -0700 Subject: [PATCH 4/4] Apply suggestions from code review --- .../Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs index 2e4c13341bfe..70f7403b064a 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs @@ -60,7 +60,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) var type = metadata.UnderlyingOrModelType; try { - var value = valueProviderResult.FirstValue; object model; @@ -75,7 +74,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } else { - // unreachable throw new NotSupportedException(); }