Fix for #2414 - Remove [DefaultValue] support from ModelBinding
This part of the change removes default value support from ModelBinding. Updated some unit tests to verify that it does nothing in that case. Deleted a functional test as it was pure duplication of another (supported) case where the property has a pre-initialized value.
This commit is contained in:
parent
1ea1cc4338
commit
2fc983b23a
|
|
@ -324,21 +324,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
};
|
||||
}
|
||||
|
||||
private static bool TryGetPropertyDefaultValue(PropertyInfo propertyInfo, out object value)
|
||||
{
|
||||
var attribute = propertyInfo.GetCustomAttribute<DefaultValueAttribute>();
|
||||
if (attribute == null)
|
||||
{
|
||||
value = null;
|
||||
return false;
|
||||
}
|
||||
else
|
||||
{
|
||||
value = attribute.Value;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingContext bindingContext)
|
||||
{
|
||||
var validationInfo = new PropertyValidationInfo();
|
||||
|
|
@ -473,16 +458,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
return;
|
||||
}
|
||||
|
||||
object value;
|
||||
var hasDefaultValue = false;
|
||||
object value = null;
|
||||
if (dtoResult.IsModelSet)
|
||||
{
|
||||
value = dtoResult.Model;
|
||||
}
|
||||
else
|
||||
{
|
||||
hasDefaultValue = TryGetPropertyDefaultValue(property, out value);
|
||||
}
|
||||
|
||||
// 'Required' validators need to run first so that we can provide useful error messages if
|
||||
// the property setters throw, e.g. if we're setting entity keys to null.
|
||||
|
|
@ -504,7 +484,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
}
|
||||
}
|
||||
|
||||
if (!dtoResult.IsModelSet && !hasDefaultValue)
|
||||
if (!dtoResult.IsModelSet)
|
||||
{
|
||||
// If we don't have a value, don't set it on the model and trounce a pre-initialized
|
||||
// value.
|
||||
|
|
|
|||
|
|
@ -1057,8 +1057,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties);
|
||||
var testableBinder = new TestableMutableObjectModelBinder();
|
||||
|
||||
// ValueTypeRequiredWithDefaultValue value comes from [DefaultValue] when !isModelSet.
|
||||
var expectedValue = isModelSet ? 0 : 42;
|
||||
// The [DefaultValue] on ValueTypeRequiredWithDefaultValue is ignored by model binding.
|
||||
var expectedValue = 0;
|
||||
|
||||
// Make ValueTypeRequired invalid.
|
||||
var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == nameof(Person.ValueTypeRequired));
|
||||
|
|
@ -1160,7 +1160,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
// Model gets provided values.
|
||||
Assert.Equal(41, model.ValueTypeRequired);
|
||||
Assert.Equal(57, model.ValueTypeRequiredWithDefaultValue);
|
||||
Assert.Equal(123.456m, model.PropertyWithDefaultValue); // from [DefaultValue]
|
||||
Assert.Equal(0m, model.PropertyWithDefaultValue); // [DefaultValue] has no effect
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -1205,7 +1205,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public void SetProperty_PropertyHasDefaultValue_SetsDefaultValue()
|
||||
public void SetProperty_PropertyHasDefaultValue_DefaultValueAttributeDoesNothing()
|
||||
{
|
||||
// Arrange
|
||||
var model = new Person();
|
||||
|
|
@ -1238,7 +1238,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
|
||||
// Assert
|
||||
var person = Assert.IsType<Person>(bindingContext.Model);
|
||||
Assert.Equal(123.456m, person.PropertyWithDefaultValue);
|
||||
Assert.Equal(0m, person.PropertyWithDefaultValue);
|
||||
Assert.True(bindingContext.ModelState.IsValid);
|
||||
}
|
||||
|
||||
|
|
@ -1276,7 +1276,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public void SetProperty_PropertyIsPreinitialized_WithDefaultValue_NoValue_CallsSetter()
|
||||
public void SetProperty_PropertyIsPreinitialized_DefaultValueAttributeDoesNothing()
|
||||
{
|
||||
// Arrange
|
||||
var model = new Person();
|
||||
|
|
@ -1304,7 +1304,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
|
||||
// Assert
|
||||
var person = Assert.IsType<Person>(bindingContext.Model);
|
||||
Assert.Equal("default", person.PropertyWithInitializedValueAndDefault);
|
||||
Assert.Equal("preinitialized", person.PropertyWithInitializedValueAndDefault);
|
||||
Assert.True(bindingContext.ModelState.IsValid);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -325,39 +325,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
|
|||
Assert.Equal("Title", error);
|
||||
}
|
||||
|
||||
// This model uses [DefaultValueAttribute(...)] for 'Title', and the model binder won't
|
||||
// trounce it.
|
||||
//
|
||||
// There's a validation error because we validate the value in the request (not present).
|
||||
[Fact]
|
||||
public async Task FromHeader_BindHeader_ToModel_NoValues_DefaultValueAttribute_ValidationError()
|
||||
{
|
||||
// Arrange
|
||||
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
|
||||
var client = server.CreateClient();
|
||||
|
||||
var request = new HttpRequestMessage(
|
||||
HttpMethod.Get,
|
||||
"http://localhost/Blog/BindToModelWithDefaultValue?author=Marvin");
|
||||
|
||||
// Intentionally not setting a title or tags
|
||||
|
||||
// Act
|
||||
var response = await client.SendAsync(request);
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
|
||||
var body = await response.Content.ReadAsStringAsync();
|
||||
var result = JsonConvert.DeserializeObject<Result>(body);
|
||||
|
||||
Assert.Equal("How to Make Soup", result.HeaderValue);
|
||||
Assert.Equal<string>(new[] { "Cooking" }, result.HeaderValues);
|
||||
|
||||
var error = Assert.Single(result.ModelStateErrors);
|
||||
Assert.Equal("Title", error);
|
||||
}
|
||||
|
||||
private class Result
|
||||
{
|
||||
public string HeaderValue { get; set; }
|
||||
|
|
|
|||
|
|
@ -87,18 +87,6 @@ namespace ModelBindingWebSite.Controllers
|
|||
};
|
||||
}
|
||||
|
||||
[HttpGet("BindToModelWithDefaultValue")]
|
||||
public object BindToModelWithDefaultValue(BlogPostWithDefaultValue blogPost)
|
||||
{
|
||||
return new Result()
|
||||
{
|
||||
HeaderValue = blogPost.Title,
|
||||
HeaderValues = blogPost.Tags,
|
||||
ModelStateErrors = ModelState.Where(kvp => kvp.Value.Errors.Count > 0).Select(kvp => kvp.Key).ToArray(),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
private class Result
|
||||
{
|
||||
public string HeaderValue { get; set; }
|
||||
|
|
@ -144,19 +132,5 @@ namespace ModelBindingWebSite.Controllers
|
|||
|
||||
public string Author { get; set; }
|
||||
}
|
||||
|
||||
public class BlogPostWithDefaultValue
|
||||
{
|
||||
[Required]
|
||||
[FromHeader]
|
||||
[DefaultValue("How to Make Soup")]
|
||||
public string Title { get; set; }
|
||||
|
||||
[FromHeader]
|
||||
[DefaultValue(new string[] { "Cooking" })]
|
||||
public string[] Tags { get; set; }
|
||||
|
||||
public string Author { get; set; }
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue