Skip to content

Add support for model binding DateTime as UTC #24893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
4 commits merged into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 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());
Expand Down
105 changes: 105 additions & 0 deletions src/Mvc/Mvc.Core/src/ModelBinding/Binders/DateTimeModelBinder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// 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;

namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
{
/// <summary>
/// An <see cref="IModelBinder"/> for <see cref="DateTime"/> and nullable <see cref="DateTime"/> models.
/// </summary>
public class DateTimeModelBinder : IModelBinder
{
private readonly DateTimeStyles _supportedStyles;
private readonly ILogger _logger;

/// <summary>
/// Initializes a new instance of <see cref="DateTimeModelBinder"/>.
/// </summary>
/// <param name="supportedStyles">The <see cref="DateTimeStyles"/>.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
public DateTimeModelBinder(DateTimeStyles supportedStyles, ILoggerFactory loggerFactory)
{
if (loggerFactory == null)
{
throw new ArgumentNullException(nameof(loggerFactory));
}

_supportedStyles = supportedStyles;
_logger = loggerFactory.CreateLogger<DateTimeModelBinder>();
}

/// <inheritdoc />
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;

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, valueProviderResult.Culture, _supportedStyles);
}
else
{
throw new NotSupportedException();
Copy link
Member

Choose a reason for hiding this comment

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

This type is constructable right? So it isn't really unreachable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe because this uncommon? IDK, removed it because it didn't really add much value.

}

// 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)
{
// Conversion failed.
modelState.TryAddModelError(modelName, exception, metadata);
}

_logger.DoneAttemptingToBindModel(bindingContext);
return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// An <see cref="IModelBinderProvider"/> for binding <see cref="DateTime" /> and nullable <see cref="DateTime"/> models.
/// </summary>
public class DateTimeModelBinderProvider : IModelBinderProvider
{
internal static readonly DateTimeStyles SupportedStyles = DateTimeStyles.AdjustToUniversal | DateTimeStyles.AllowWhiteSpaces;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can create a binder provider that configures the model binder (it's constructable). It's the pattern we use for other types.


/// <inheritdoc />
public IModelBinder GetBinder(ModelBinderProviderContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

var modelType = context.Metadata.UnderlyingOrModelType;
if (modelType == typeof(DateTime))
{
var loggerFactory = context.Services.GetRequiredService<ILoggerFactory>();
return new DateTimeModelBinder(SupportedStyles, loggerFactory);
}

return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
try
{
var value = valueProviderResult.FirstValue;
var culture = valueProviderResult.Culture;

object model;
if (string.IsNullOrWhiteSpace(value))
Expand All @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
try
{
var value = valueProviderResult.FirstValue;
var culture = valueProviderResult.Culture;

object model;
if (string.IsNullOrWhiteSpace(value))
Expand All @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
try
{
var value = valueProviderResult.FirstValue;
var culture = valueProviderResult.Culture;

object model;
if (string.IsNullOrWhiteSpace(value))
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

Any functional reason for inlining this or just code clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
throw new ArgumentNullException(nameof(bindingContext));
}

_logger.AttemptingToBindModel(bindingContext);
Copy link
Member

Choose a reason for hiding this comment

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

Do we need move the logging above the work in other binders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of the ones that relied on the value provider, Simple type was the only outlier.


var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName);
if (valueProviderResult == ValueProviderResult.None)
{
Expand All @@ -56,8 +58,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
return Task.CompletedTask;
}

_logger.AttemptingToBindModel(bindingContext);

bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);

try
Expand Down
6 changes: 6 additions & 0 deletions src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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<TKey, TValue>
Microsoft.AspNetCore.Mvc.ModelBinding.Binders.DictionaryModelBinderProvider
Expand Down Expand Up @@ -1464,6 +1467,9 @@ virtual Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.Visit
~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder.ComplexTypeModelBinder(System.Collections.Generic.IDictionary<Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder> propertyBinders, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void
~Microsoft.AspNetCore.Mvc.ModelBinding.Binders.ComplexTypeModelBinder.ComplexTypeModelBinder(System.Collections.Generic.IDictionary<Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder> 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<TKey, TValue>.DictionaryModelBinder(Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder keyBinder, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder valueBinder, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) -> void
Expand Down
Original file line number Diff line number Diff line change
@@ -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<DateTimeModelBinder>(result);
}

[Fact]
public void Create_ForNullableDateTime_ReturnsBinder()
{
// Arrange
var context = new TestModelBinderProviderContext(typeof(DateTime?));

// Act
var result = _provider.GetBinder(context);

// Assert
Assert.IsType<DateTimeModelBinder>(result);
}
}
}
Loading