Adding fix for 2756 and test cases. Skipping the test cases for 2793.
This commit is contained in:
parent
35ee0e3a49
commit
933a13608f
|
|
@ -32,7 +32,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
var mutableObjectBinderContext = new MutableObjectBinderContext()
|
var mutableObjectBinderContext = new MutableObjectBinderContext()
|
||||||
{
|
{
|
||||||
ModelBindingContext = bindingContext,
|
ModelBindingContext = bindingContext,
|
||||||
PropertyMetadata = GetMetadataForProperties(bindingContext),
|
PropertyMetadata = GetMetadataForProperties(bindingContext).ToArray(),
|
||||||
};
|
};
|
||||||
|
|
||||||
if (!(await CanCreateModel(mutableObjectBinderContext)))
|
if (!(await CanCreateModel(mutableObjectBinderContext)))
|
||||||
|
|
@ -98,7 +98,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2. Any of the model properties can be bound using a value provider.
|
// 2. If it is top level object and there are no properties to bind
|
||||||
|
if (isTopLevelObject && context.PropertyMetadata != null && context.PropertyMetadata.Count == 0)
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 3. Any of the model properties can be bound using a value provider.
|
||||||
if (await CanValueBindAnyModelProperties(context))
|
if (await CanValueBindAnyModelProperties(context))
|
||||||
{
|
{
|
||||||
return true;
|
return true;
|
||||||
|
|
@ -109,6 +115,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
private async Task<bool> CanValueBindAnyModelProperties(MutableObjectBinderContext context)
|
private async Task<bool> CanValueBindAnyModelProperties(MutableObjectBinderContext context)
|
||||||
{
|
{
|
||||||
|
// If there are no properties on the model, there is nothing to bind. We are here means this is not a top
|
||||||
|
// level object. So we return false.
|
||||||
|
if (context.PropertyMetadata == null || context.PropertyMetadata.Count == 0)
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
// We want to check to see if any of the properties of the model can be bound using the value providers,
|
// We want to check to see if any of the properties of the model can be bound using the value providers,
|
||||||
// because that's all that MutableObjectModelBinder can handle.
|
// because that's all that MutableObjectModelBinder can handle.
|
||||||
//
|
//
|
||||||
|
|
@ -202,6 +215,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (modelMetadata.IsCollectionType)
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
if (modelMetadata.ModelType == typeof(ComplexModelDto))
|
if (modelMetadata.ModelType == typeof(ComplexModelDto))
|
||||||
{
|
{
|
||||||
// forbidden type - will cause a stack overflow if we try binding this type
|
// forbidden type - will cause a stack overflow if we try binding this type
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
{
|
{
|
||||||
public ModelBindingContext ModelBindingContext { get; set; }
|
public ModelBindingContext ModelBindingContext { get; set; }
|
||||||
|
|
||||||
public IEnumerable<ModelMetadata> PropertyMetadata { get; set; }
|
public IReadOnlyList<ModelMetadata> PropertyMetadata { get; set; }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -74,7 +74,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
var mutableBinder = new TestableMutableObjectModelBinder();
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
bindingContext.ModelBindingContext);
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var canCreate = await mutableBinder.CanCreateModel(bindingContext);
|
var canCreate = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
@ -167,7 +167,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
var mutableBinder = new TestableMutableObjectModelBinder();
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
bindingContext.ModelBindingContext);
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
@ -217,6 +217,55 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
Assert.Equal(valueAvailable, result);
|
Assert.Equal(valueAvailable, result);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task CanCreateModel_ReturnsFalse_IfNotIsTopLevelObjectAndModelHasNoProperties()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var bindingContext = new MutableObjectBinderContext
|
||||||
|
{
|
||||||
|
ModelBindingContext = new ModelBindingContext
|
||||||
|
{
|
||||||
|
IsTopLevelObject = false,
|
||||||
|
|
||||||
|
ModelMetadata = GetMetadataForType(typeof(PersonWithNoProperties))
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var canCreate = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.False(canCreate);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task CanCreateModel_ReturnsTrue_IfIsTopLevelObjectAndModelHasNoProperties()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var bindingContext = new MutableObjectBinderContext
|
||||||
|
{
|
||||||
|
ModelBindingContext = new ModelBindingContext
|
||||||
|
{
|
||||||
|
IsTopLevelObject = true,
|
||||||
|
ModelMetadata = GetMetadataForType(typeof(PersonWithNoProperties))
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.True(retModel);
|
||||||
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData(typeof(TypeWithNoBinderMetadata), false)]
|
[InlineData(typeof(TypeWithNoBinderMetadata), false)]
|
||||||
[InlineData(typeof(TypeWithNoBinderMetadata), true)]
|
[InlineData(typeof(TypeWithNoBinderMetadata), true)]
|
||||||
|
|
@ -251,7 +300,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
var mutableBinder = new TestableMutableObjectModelBinder();
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
bindingContext.ModelBindingContext);
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
@ -311,7 +360,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
var mutableBinder = new TestableMutableObjectModelBinder();
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
bindingContext.ModelBindingContext);
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
@ -354,7 +403,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
|
|
||||||
var mutableBinder = new TestableMutableObjectModelBinder();
|
var mutableBinder = new TestableMutableObjectModelBinder();
|
||||||
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties(
|
||||||
bindingContext.ModelBindingContext);
|
bindingContext.ModelBindingContext).ToArray();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
var retModel = await mutableBinder.CanCreateModel(bindingContext);
|
||||||
|
|
@ -1753,6 +1802,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
||||||
public string PropertyWithInitializedValueAndDefault { get; set; } = "preinitialized";
|
public string PropertyWithInitializedValueAndDefault { get; set; } = "preinitialized";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private class PersonWithNoProperties
|
||||||
|
{
|
||||||
|
public string name;
|
||||||
|
}
|
||||||
|
|
||||||
private class PersonWithBindExclusion
|
private class PersonWithBindExclusion
|
||||||
{
|
{
|
||||||
[BindNever]
|
[BindNever]
|
||||||
|
|
|
||||||
|
|
@ -693,6 +693,47 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
|
||||||
Assert.Null(company.Employees);
|
Assert.Null(company.Employees);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PocoGetsCreated_IfTopLevelNoProperties()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
|
||||||
|
var client = server.CreateClient();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var response = await
|
||||||
|
client.GetAsync("http://localhost/Properties" +
|
||||||
|
"/GetPerson");
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||||
|
var person = JsonConvert.DeserializeObject<PersonWithNoProperties>(
|
||||||
|
await response.Content.ReadAsStringAsync());
|
||||||
|
Assert.NotNull(person);
|
||||||
|
Assert.Null(person.Name);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ArrayOfPocoGetsCreated_PoCoWithNoProperties()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
|
||||||
|
var client = server.CreateClient();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
var response = await
|
||||||
|
client.GetAsync("http://localhost/Properties" +
|
||||||
|
"/GetPeople?people[0].Name=asdf");
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||||
|
var arrperson = JsonConvert.DeserializeObject<ArrayOfPersonWithNoProperties>(
|
||||||
|
await response.Content.ReadAsStringAsync());
|
||||||
|
Assert.NotNull(arrperson);
|
||||||
|
Assert.NotNull(arrperson.people);
|
||||||
|
Assert.Equal(0, arrperson.people.Length);
|
||||||
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("http://localhost/Home/ActionWithPersonFromUrlWithPrefix/Javier/26")]
|
[InlineData("http://localhost/Home/ActionWithPersonFromUrlWithPrefix/Javier/26")]
|
||||||
[InlineData("http://localhost/Home/ActionWithPersonFromUrlWithoutPrefix/Javier/26")]
|
[InlineData("http://localhost/Home/ActionWithPersonFromUrlWithoutPrefix/Javier/26")]
|
||||||
|
|
|
||||||
|
|
@ -77,7 +77,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
public CustomReadOnlyCollection<Address> Address { get; set; }
|
public CustomReadOnlyCollection<Address> Address { get; set; }
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
|
||||||
public async Task ActionParameter_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
|
public async Task ActionParameter_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|
@ -252,7 +252,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
|
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
|
||||||
public async Task ActionParameter_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
|
public async Task ActionParameter_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|
|
||||||
|
|
@ -466,7 +466,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
var parameter = new ParameterDescriptor()
|
var parameter = new ParameterDescriptor()
|
||||||
{
|
{
|
||||||
Name = "parameter",
|
Name = "parameter",
|
||||||
ParameterType = typeof(Collection<KeyValuePair<string, int>>)
|
ParameterType = typeof(ICollection<KeyValuePair<string, int>>)
|
||||||
};
|
};
|
||||||
|
|
||||||
var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request =>
|
var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request =>
|
||||||
|
|
@ -483,7 +483,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
Assert.NotNull(modelBindingResult);
|
Assert.NotNull(modelBindingResult);
|
||||||
Assert.True(modelBindingResult.IsModelSet);
|
Assert.True(modelBindingResult.IsModelSet);
|
||||||
|
|
||||||
var model = Assert.IsType<Collection<KeyValuePair<string, int>>>(modelBindingResult.Model);
|
var model = Assert.IsType<List<KeyValuePair<string, int>>>(modelBindingResult.Model);
|
||||||
Assert.NotNull(model);
|
Assert.NotNull(model);
|
||||||
Assert.Empty(model);
|
Assert.Empty(model);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -173,7 +173,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
public CustomReadOnlyCollection<Address> Address { get; set; }
|
public CustomReadOnlyCollection<Address> Address { get; set; }
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
|
||||||
public async Task TryUpdateModel_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
|
public async Task TryUpdateModel_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|
@ -409,7 +409,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
|
||||||
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
|
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
|
||||||
public async Task TryUpdateModel_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
|
public async Task TryUpdateModel_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
|
|
|
||||||
|
|
@ -12,5 +12,15 @@ namespace ModelBindingWebSite.Controllers
|
||||||
{
|
{
|
||||||
return c;
|
return c;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public PersonWithNoProperties GetPerson(PersonWithNoProperties p)
|
||||||
|
{
|
||||||
|
return p;
|
||||||
|
}
|
||||||
|
|
||||||
|
public ArrayOfPersonWithNoProperties GetPeople(ArrayOfPersonWithNoProperties p)
|
||||||
|
{
|
||||||
|
return p;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -0,0 +1,10 @@
|
||||||
|
// 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.
|
||||||
|
|
||||||
|
namespace ModelBindingWebSite.Models
|
||||||
|
{
|
||||||
|
public class ArrayOfPersonWithNoProperties
|
||||||
|
{
|
||||||
|
public PersonWithNoProperties[] people { get; set; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,10 @@
|
||||||
|
// 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.
|
||||||
|
|
||||||
|
namespace ModelBindingWebSite.Models
|
||||||
|
{
|
||||||
|
public class PersonWithNoProperties
|
||||||
|
{
|
||||||
|
public string Name;
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue