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
- enable tests that #2445 was blocking
 - fix these and other tests expecting different `ModelState` entries
  • Loading branch information
@dougbu
dougbu committed Jun 18, 2015
1 parent c4fa402 commit a218289
Show file tree
Hide file tree
Showing 15 changed files with 233 additions and 210 deletions.
2 changes: 2 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs
View file
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(model, attemptedValue: null, culture: null);
bindingContext.ModelState.SetModelValue(modelBindingKey, valueProviderResult);

var validationNode = new ModelValidationNode(modelBindingKey, bindingContext.ModelMetadata, model)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs
View file
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
8 changes: 5 additions & 3 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs
View file
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 All @@ -46,6 +45,9 @@ public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingConte
model = new FormCollection(new Dictionary<string, string[]>());
}

var valueProviderResult = new ValueProviderResult(model, attemptedValue: null, culture: null);
bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);

var validationNode =
new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model);
return new ModelBindingResult(
Expand Down
36 changes: 23 additions & 13 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs
View file
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,25 +22,33 @@ public class FormFileModelBinder : IModelBinder
/// <inheritdoc />
public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingContext bindingContext)
{
var typeMatched = false;
object value = null;
if (bindingContext.ModelType == typeof(IFormFile))
{
typeMatched = true;
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))
{
typeMatched = true;
var postedFiles = await GetFormFilesAsync(bindingContext);
var value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles);
var validationNode =
new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value);
value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles);
}

if (typeMatched)
{
ModelValidationNode validationNode = null;
if (value != null)
{
validationNode =
new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value);

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

return new ModelBindingResult(
value,
bindingContext.ModelName,
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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, culture: null);
bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult);
}

return Task.FromResult(
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public async Task BindModel_NoInputFormatterFound_SetsModelStateError()

// Assert

// Returns true because it understands the metadata type.
// Returns non-null because it understands the metadata type.
Assert.NotNull(binderResult);
Assert.True(binderResult.IsFatalError);
Assert.False(binderResult.IsModelSet);
Expand Down Expand Up @@ -171,7 +171,7 @@ public async Task CustomFormatterDeserializationException_AddedToModelState()

// Assert

// Returns true because it understands the metadata type.
// Returns non-null because it understands the metadata type.
Assert.NotNull(binderResult);
Assert.True(binderResult.IsFatalError);
Assert.False(binderResult.IsModelSet);
Expand Down
7 changes: 5 additions & 2 deletions test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ public async Task BindModelSetsModelToNullOnNullOrEmptyString(string value)
Assert.Equal("foo", binderResult.Key);
Assert.Null(binderResult.Model);

Assert.Empty(bindingContext.ModelState); // No submitted value for "foo".
var modelState = Assert.Single(bindingContext.ModelState);
Assert.Equal("foo", modelState.Key);
Assert.NotNull(modelState.Value.Value);
Assert.Equal(value, modelState.Value.Value.RawValue);
}

[Fact]
Expand Down Expand Up @@ -64,7 +67,7 @@ public async Task BindModelAddsModelErrorsOnInvalidCharacters()
// Arrange
var expected = TestPlatformHelper.IsMono ?
"Invalid length." :
"The supplied value is invalid for foo.";
"The value '\"Fys1\"' is not valid for foo.";

var valueProvider = new SimpleHttpValueProvider()
{
Expand Down
68 changes: 29 additions & 39 deletions test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@

using System.ComponentModel.DataAnnotations;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.Framework.DependencyInjection;
using Xunit;

namespace Microsoft.AspNet.Mvc.IntegrationTests
Expand Down Expand Up @@ -71,10 +68,10 @@ public async Task FromBodyOnActionParameter_EmptyBody_BindsToNullValue()
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
var parameter = new ParameterDescriptor
{
Name = "Parameter1",
BindingInfo = new BindingInfo()
BindingInfo = new BindingInfo
{
BinderModelName = "CustomParameter",
BindingSource = BindingSource.Body
Expand All @@ -99,8 +96,11 @@ public async Task FromBodyOnActionParameter_EmptyBody_BindsToNullValue()
Assert.NotNull(modelBindingResult);
Assert.True(modelBindingResult.IsModelSet);
Assert.Null(modelBindingResult.Model);
Assert.Empty(modelState.Keys);

Assert.True(modelState.IsValid);
var entry = Assert.Single(modelState);
Assert.Empty(entry.Key);
Assert.Null(entry.Value.Value.RawValue);
}

private class Person4
Expand All @@ -115,10 +115,10 @@ public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_AddsModelStat
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
var parameter = new ParameterDescriptor
{
Name = "Parameter1",
BindingInfo = new BindingInfo()
BindingInfo = new BindingInfo
{
BinderModelName = "CustomParameter",
},
Expand All @@ -141,13 +141,11 @@ public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_AddsModelStat
Assert.NotNull(modelBindingResult);
Assert.True(modelBindingResult.IsModelSet);
var boundPerson = Assert.IsType<Person4>(modelBindingResult.Model);
Assert.NotNull(boundPerson);
Assert.False(modelState.IsValid);

// The error with an empty key is a bug(#2416) in our implementation which does not append the prefix and
// use that along with the path. The expected key here would be CustomParameter.Address.
var key = Assert.Single(modelState.Keys, k => k == "");
var error = Assert.Single(modelState[""].Errors);
Assert.False(modelState.IsValid);
var entry = Assert.Single(modelState);
Assert.Equal(string.Empty, entry.Key);
var error = Assert.Single(entry.Value.Errors);
Assert.StartsWith(
"No JSON content found and type 'System.Int32' is not nullable.",
error.Exception.Message);
Expand All @@ -167,16 +165,16 @@ private class Address2
public int Zip { get; set; }
}

[Theory(Skip = "There should be entries for all model properties which are bound. #2445")]
[Theory]
[InlineData("{ \"Zip\" : 123 }")]
[InlineData("{}")]
public async Task FromBodyOnTopLevelProperty_RequiredOnSubProperty_AddsModelStateError(string inputText)
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
var parameter = new ParameterDescriptor
{
BindingInfo = new BindingInfo()
BindingInfo = new BindingInfo
{
BinderModelName = "CustomParameter",
},
Expand All @@ -200,14 +198,15 @@ public async Task FromBodyOnTopLevelProperty_RequiredOnSubProperty_AddsModelStat
Assert.True(modelBindingResult.IsModelSet);
var boundPerson = Assert.IsType<Person2>(modelBindingResult.Model);
Assert.NotNull(boundPerson);

Assert.False(modelState.IsValid);
Assert.Equal(2, modelState.Keys.Count);
var zip = Assert.Single(modelState.Keys, k => k == "CustomParameter.Address.Zip");
Assert.Equal(ModelValidationState.Valid, modelState[zip].ValidationState);
var address = Assert.Single(modelState, kvp => kvp.Key == "CustomParameter.Address").Value;
Assert.Equal(ModelValidationState.Unvalidated, address.ValidationState);

var street = Assert.Single(modelState.Keys, k => k == "CustomParameter.Address.Street");
Assert.Equal(ModelValidationState.Invalid, modelState[street].ValidationState);
var error = Assert.Single(modelState[street].Errors);
var street = Assert.Single(modelState, kvp => kvp.Key == "CustomParameter.Address.Street").Value;
Assert.Equal(ModelValidationState.Invalid, street.ValidationState);
var error = Assert.Single(street.Errors);
Assert.Equal("The Street field is required.", error.ErrorMessage);
}

Expand All @@ -225,16 +224,16 @@ private class Address3
public int Zip { get; set; }
}

[Theory(Skip = "There should be entries for all model properties which are bound. #2445")]
[Theory]
[InlineData("{ \"Street\" : \"someStreet\" }")]
[InlineData("{}")]
public async Task FromBodyOnProperty_RequiredOnValueTypeSubProperty_AddsModelStateError(string inputText)
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
var parameter = new ParameterDescriptor
{
BindingInfo = new BindingInfo()
BindingInfo = new BindingInfo
{
BinderModelName = "CustomParameter",
},
Expand All @@ -255,20 +254,11 @@ public async Task FromBodyOnProperty_RequiredOnValueTypeSubProperty_AddsModelSta
// Assert
Assert.NotNull(modelBindingResult);
Assert.True(modelBindingResult.IsModelSet);
var boundPerson = Assert.IsType<Person3>(modelBindingResult.Model);
Assert.NotNull(boundPerson);
Assert.False(modelState.IsValid);
var street = Assert.Single(modelState.Keys, k => k == "CustomParameter.Address.Street");
Assert.Equal(ModelValidationState.Valid, modelState[street].ValidationState);

// The error with an empty key is a bug(#2416) in our implementation which does not append the prefix and
// use that along with the path. The expected key here would be Address.
var zip = Assert.Single(modelState.Keys, k => k == "CustomParameter.Address.Zip");
Assert.Equal(ModelValidationState.Valid, modelState[zip].ValidationState);
var error = Assert.Single(modelState[""].Errors);
Assert.StartsWith(
"Required property 'Zip' not found in JSON. Path ''",
error.Exception.Message);
Assert.IsType<Person3>(modelBindingResult.Model);

Assert.True(modelState.IsValid);
Assert.Equal(1, modelState.Count);
Assert.Single(modelState, kvp => kvp.Key == "CustomParameter.Address");
}
}
}
Loading

0 comments on commit a218289

Please sign in to comment.