Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Add ModelState entries for greedy and type-matching model binders
Browse files Browse the repository at this point in the history
- part II of II for #2445
- `FormCollectionModelBinder` is an exception because container is not user-provided
 - no `ModelState` entry added
- enable tests that #2445 was blocking
 - fix these and other tests expecting different `ModelState` entries
- simplify logic in `FormFileModelBinder`

`ValueProviderResult`
- remove `protected` setters and parameterless constructor
 - no scenario for their use in subclasses; however `ConvertTo()` remains `virtual`
- add single-parameter constructor
 - use in most of the greedy and type-matching model binders
- add doc comments throughout class

nits:
- use new `ValueProviderResult` constructor in many existing tests
- `""` -> `string.Empty` and `vpr` -> `valueProviderResult` in `ValueProviderResultTest`
- improve some test names in `BodyValidationIntegrationTests`
- do not check `Message` of a Json.NET `Exception`
  • Loading branch information
dougbu committed Jun 24, 2015
1 parent 4a4b8ec commit 715a0b6
Show file tree
Hide file tree
Showing 20 changed files with 360 additions and 348 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,83 @@

namespace Microsoft.AspNet.Mvc.ModelBinding
{
/// <summary>
/// Result of an <see cref="IValueProvider.GetValueAsync"/> operation.
/// </summary>
public class ValueProviderResult
{
private static readonly CultureInfo _staticCulture = CultureInfo.InvariantCulture;
private CultureInfo _instanceCulture;

// default constructor so that subclassed types can set the properties themselves
protected ValueProviderResult()
/// <summary>
/// Instantiates a new instance of the <see cref="ValueProviderResult"/> class with given
/// <paramref name="rawValue"/>. Initializes <see cref="Culture"/> to
/// <see cref="CultureInfo.InvariantCulture"/>.
/// </summary>
/// <param name="rawValue">The <see cref="RawValue"/> value of the new instance.</param>
public ValueProviderResult(object rawValue)
: this(rawValue, attemptedValue: null, culture: _staticCulture)
{
}

/// <summary>
/// Instantiates a new instance of the <see cref="ValueProviderResult"/> class with given
/// <paramref name="rawValue"/>, <paramref name="attemptedValue"/>, and <paramref name="culture"/>.
/// </summary>
/// <param name="rawValue">The <see cref="RawValue"/> value of the new instance.</param>
/// <param name="attemptedValue">The <see cref="AttemptedValue"/> value of the new instance.</param>
/// <param name="culture">The <see cref="Culture"/> value of the new instance.</param>
public ValueProviderResult(object rawValue, string attemptedValue, CultureInfo culture)
{
RawValue = rawValue;
AttemptedValue = attemptedValue;
Culture = culture;
Culture = culture ?? _staticCulture;
}

public string AttemptedValue { get; protected set; }

public CultureInfo Culture
{
get
{
if (_instanceCulture == null)
{
_instanceCulture = _staticCulture;
}
return _instanceCulture;
}
protected set { _instanceCulture = value; }
}

public object RawValue { get; protected set; }

/// <summary>
/// <see cref="string"/> conversion of <see cref="RawValue"/>.
/// </summary>
/// <remarks>
/// Used in helpers that generate <c>&lt;textarea&gt;</c> elements as well as some error messages.
/// </remarks>
public string AttemptedValue { get; }

/// <summary>
/// <see cref="CultureInfo"/> to use in <see cref="ConvertTo(Type)"/> or
/// <see cref="ConvertTo(Type, CultureInfo)"/> if passed <see cref="CultureInfo"/> is <c>null</c>.
/// </summary>
public CultureInfo Culture { get; }

/// <summary>
/// The provided <see cref="object"/>.
/// </summary>
public object RawValue { get; }

/// <summary>
/// Converts <see cref="RawValue"/> to the given <paramref name="type"/>. Uses <see cref="Culture"/> for
/// <see cref="TypeConverter"/> operations.
/// </summary>
/// <param name="type">The target <see cref="Type"/> of the conversion.</param>
/// <returns>
/// <see cref="RawValue"/> converted to the given <paramref name="type"/>. <c>null</c> if the conversion fails.
/// </returns>
public object ConvertTo(Type type)
{
return ConvertTo(type, culture: null);
}

/// <summary>
/// Converts <see cref="RawValue"/> to the given <paramref name="type"/> using the given
/// <paramref name="culture"/>.
/// </summary>
/// <param name="type">The target <see cref="Type"/> of the conversion.</param>
/// <param name="culture">
/// The <see cref="CultureInfo"/> to use for <see cref="TypeConverter"/> operations. Uses
/// <see cref="Culture"/> if this parameter is <c>null</c>.
/// </param>
/// <returns>
/// <see cref="RawValue"/> converted to the given <paramref name="type"/> using the given
/// <paramref name="culture"/>. <c>null</c> if the conversion fails.
/// </returns>
public virtual object ConvertTo([NotNull] Type type, CultureInfo culture)
{
var value = RawValue;
Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ protected async override Task<ModelBindingResult> BindModelCoreAsync(
return new ModelBindingResult(modelBindingKey);
}

var valueProviderResult = new ValueProviderResult(rawValue: model);
bindingContext.ModelState.SetModelValue(modelBindingKey, valueProviderResult);

var validationNode = new ModelValidationNode(modelBindingKey, bindingContext.ModelMetadata, model)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingConte
return null;
}

var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName);

// Check for missing data case 1: There was no <input ... /> element containing this data.
var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName);
if (valueProviderResult == null)
{
return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
}

var value = valueProviderResult.AttemptedValue;
bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);

// Check for missing data case 2: There was an <input ... /> element but it was left blank.
var value = valueProviderResult.AttemptedValue;
if (string.IsNullOrEmpty(value))
{
return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingConte
return null;
}

object model = null;
object model;
var request = bindingContext.OperationBindingContext.HttpContext.Request;
if (request.HasFormContentType)
{
Expand All @@ -36,8 +36,7 @@ public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingConte
}
else
{
var formValuesLookup = form.ToDictionary(p => p.Key,
p => p.Value);
var formValuesLookup = form.ToDictionary(p => p.Key, p => p.Value);
model = new FormCollection(formValuesLookup, form.Files);
}
}
Expand Down
43 changes: 25 additions & 18 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System;
using System.Collections.Generic;
using System.Linq;
#if DNXCORE50
using System.Reflection;
#endif
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.Framework.Internal;
Expand All @@ -20,33 +22,38 @@ public class FormFileModelBinder : IModelBinder
/// <inheritdoc />
public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingContext bindingContext)
{
object value;
if (bindingContext.ModelType == typeof(IFormFile))
{
var postedFiles = await GetFormFilesAsync(bindingContext);
var value = postedFiles.FirstOrDefault();
var validationNode =
new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value);
return new ModelBindingResult(
value,
bindingContext.ModelName,
isModelSet: value != null,
validationNode: validationNode);
value = postedFiles.FirstOrDefault();
}
else if (typeof(IEnumerable<IFormFile>).GetTypeInfo().IsAssignableFrom(
bindingContext.ModelType.GetTypeInfo()))
else if (typeof(IEnumerable<IFormFile>).IsAssignableFrom(bindingContext.ModelType))
{
var postedFiles = await GetFormFilesAsync(bindingContext);
var value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles);
var validationNode =
value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles);
}
else
{
// This binder does not support the requested type.
return null;
}

ModelValidationNode validationNode = null;
if (value != null)
{
validationNode =
new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value);
return new ModelBindingResult(
value,
bindingContext.ModelName,
isModelSet: value != null,
validationNode: validationNode);

var valueProviderResult = new ValueProviderResult(rawValue: value);
bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);
}

return null;
return new ModelBindingResult(
value,
bindingContext.ModelName,
isModelSet: value != null,
validationNode: validationNode);
}

private async Task<List<IFormFile>> GetFormFilesAsync(ModelBindingContext bindingContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Globalization;
#if DNXCORE50
using System.Reflection;
#endif
Expand Down Expand Up @@ -59,6 +60,10 @@ protected override Task<ModelBindingResult> BindModelCoreAsync([NotNull] ModelBi
bindingContext.ModelName,
bindingContext.ModelMetadata,
model);

var attemptedValue = (model as string) ?? request.Headers.Get(headerName);
var valueProviderResult = new ValueProviderResult(model, attemptedValue, CultureInfo.InvariantCulture);
bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);
}

return Task.FromResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Globalization;
using Microsoft.Framework.Internal;
using Xunit;
Expand Down Expand Up @@ -75,7 +74,7 @@ public void MarkFieldSkipped_Throws_IfStateIsInvalid()

// Act
var exception = Assert.Throws<InvalidOperationException>(() => source.MarkFieldSkipped("key"));

// Assert
Assert.Equal(
"A field previously marked invalid should not be marked skipped.",
Expand Down Expand Up @@ -138,7 +137,7 @@ public void MarkFieldValid_Throws_IfStateIsInvalid()

// Act
var exception = Assert.Throws<InvalidOperationException>(() => source.MarkFieldValid("key"));

// Assert
Assert.Equal(
"A field previously marked invalid should not be marked valid.",
Expand Down Expand Up @@ -278,7 +277,7 @@ public void GetFieldValidationState_ReturnsValidIfModelStateDoesNotContainErrors
// Arrange
var validState = new ModelState
{
Value = new ValueProviderResult(null, null, null),
Value = new ValueProviderResult(rawValue: null),
ValidationState = ModelValidationState.Valid
};
var msd = new ModelStateDictionary
Expand Down Expand Up @@ -317,7 +316,7 @@ public void GetFieldValidationState_IndexedPrefix_ReturnsValidIfModelStateDoesNo
// Arrange
var validState = new ModelState
{
Value = new ValueProviderResult(null, null, null),
Value = new ValueProviderResult(rawValue: null),
ValidationState = ModelValidationState.Valid
};
var msd = new ModelStateDictionary
Expand Down
Loading

0 comments on commit 715a0b6

Please sign in to comment.