diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs index 84d4106b8e..ac06ef741f 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs @@ -39,16 +39,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [NotNull] string modelName, [NotNull] ModelMetadata modelMetadata) { - var modelBindingContext = new ModelBindingContext(); - modelBindingContext.ModelName = modelName; - modelBindingContext.ModelMetadata = modelMetadata; - modelBindingContext.ModelState = bindingContext.ModelState; - modelBindingContext.ValueProvider = bindingContext.ValueProvider; - modelBindingContext.OperationBindingContext = bindingContext.OperationBindingContext; + var modelBindingContext = new ModelBindingContext + { + ModelName = modelName, + ModelMetadata = modelMetadata, + + ModelState = bindingContext.ModelState, + ValueProvider = bindingContext.ValueProvider, + OperationBindingContext = bindingContext.OperationBindingContext, + + BindingSource = modelMetadata.BindingSource, + BinderModelName = modelMetadata.BinderModelName, + BinderType = modelMetadata.BinderType, + }; - modelBindingContext.BindingSource = modelMetadata.BindingSource; - modelBindingContext.BinderModelName = modelMetadata.BinderModelName; - modelBindingContext.BinderType = modelMetadata.BinderType; return modelBindingContext; } @@ -66,9 +70,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding string modelName) { var binderModelName = bindingInfo?.BinderModelName ?? metadata.BinderModelName; - var propertyPredicateProvider = + var propertyPredicateProvider = bindingInfo?.PropertyBindingPredicateProvider ?? metadata.PropertyBindingPredicateProvider; - return new ModelBindingContext() + return new ModelBindingContext { ModelMetadata = metadata, BindingSource = bindingInfo?.BindingSource ?? metadata.BindingSource, @@ -149,27 +153,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } /// - /// Gets or sets a model name which is explicitly set using an . + /// Gets or sets a model name which is explicitly set using an . /// . /// public string BinderModelName { get; set; } /// - /// Gets or sets a value which represents the associated with the + /// Gets or sets a value which represents the associated with the /// . /// public BindingSource BindingSource { get; set; } /// - /// Gets the of an associated with the + /// Gets the of an associated with the /// . /// public Type BinderType { get; set; } /// /// Gets or sets a value that indicates whether the binder should use an empty prefix to look up - /// values in when no values are found using the - /// prefix. + /// values in when no values are found using the prefix. /// public bool FallbackToEmptyPrefix { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs index c50ecc7ca3..941afaf25a 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs @@ -8,6 +8,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public class ModelBindingResult { + /// + /// Creates a new indicating a fatal error. + /// + /// The key using which was used to attempt binding the model. + public ModelBindingResult(string key) + { + Key = key; + IsFatalError = true; + } + /// /// Creates a new . /// @@ -16,7 +26,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// A value that represents if the model has been set by the /// . public ModelBindingResult(object model, string key, bool isModelSet) - : this (model, key, isModelSet, validationNode: null) + : this(model, key, isModelSet, validationNode: null) { } @@ -52,6 +62,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public string Key { get; } + /// + /// Gets a value indicating the caller should not attempt binding again. This attempt encountered a fatal + /// error. + /// + public bool IsFatalError { get; } + /// /// /// Gets a value indicating whether or not the value has been set. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs index 2b3c004abd..cc458805ae 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await modelBinder.BindModelAsync(bindingContext); var modelBindingResult = result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : + result : new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); // A model binder was specified by metadata and this binder handles all such cases. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs index f01a2c0135..440a202002 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs @@ -79,10 +79,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await BindModelCoreAsync(context); - var modelBindingResult = - result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : - new ModelBindingResult(model: null, key: context.ModelName, isModelSet: false); + var modelBindingResult = result != null ? + result : + new ModelBindingResult(model: null, key: context.ModelName, isModelSet: false); // This model binder is the only handler for its binding source. // Always tell the model binding system to skip other model binders i.e. return non-null. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs index eb9552b02c..e7775f01bb 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs @@ -5,7 +5,6 @@ using System; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.Core; -using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding @@ -28,36 +27,45 @@ namespace Microsoft.AspNet.Mvc.ModelBinding protected async override Task BindModelCoreAsync( [NotNull] ModelBindingContext bindingContext) { + // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of + // the parameter name/a custom name. In all other cases (like when binding body to a property) we + // consider the entire ModelName as a prefix. + var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; + var modelBindingKey = isTopLevelObject ? string.Empty : bindingContext.ModelName; + var httpContext = bindingContext.OperationBindingContext.HttpContext; - var formatters = bindingContext.OperationBindingContext.InputFormatters; var formatterContext = new InputFormatterContext( - httpContext, - bindingContext.ModelState, + httpContext, + bindingContext.ModelState, bindingContext.ModelType); + var formatters = bindingContext.OperationBindingContext.InputFormatters; var formatter = formatters.FirstOrDefault(f => f.CanRead(formatterContext)); if (formatter == null) { var unsupportedContentType = Resources.FormatUnsupportedContentType( bindingContext.OperationBindingContext.HttpContext.Request.ContentType); - bindingContext.ModelState.AddModelError(bindingContext.ModelName, unsupportedContentType); + bindingContext.ModelState.AddModelError(modelBindingKey, unsupportedContentType); - // This model binder is the only handler for the Body binding source. - // Always tell the model binding system to skip other model binders i.e. return non-null. - return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); + // This model binder is the only handler for the Body binding source and it cannot run twice. Always + // tell the model binding system to skip other model binders and never to fall back i.e. indicate a + // fatal error. + return new ModelBindingResult(modelBindingKey); } try { + var previousCount = bindingContext.ModelState.ErrorCount; var model = await formatter.ReadAsync(formatterContext); - var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; + if (bindingContext.ModelState.ErrorCount != previousCount) + { + // Formatter added an error. Do not use the model it returned. As above, tell the model binding + // system to skip other model binders and never to fall back. + return new ModelBindingResult(modelBindingKey); + } - // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of - // the parameter name/a custom name. In all other cases (like when binding body to a property) we - // consider the entire ModelName as a prefix. - var modelBindingKey = isTopLevelObject ? string.Empty : bindingContext.ModelName; var validationNode = new ModelValidationNode(modelBindingKey, bindingContext.ModelMetadata, model) { @@ -72,11 +80,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } catch (Exception ex) { - bindingContext.ModelState.AddModelError(bindingContext.ModelName, ex); + bindingContext.ModelState.AddModelError(modelBindingKey, ex); - // This model binder is the only handler for the Body binding source. - // Always tell the model binding system to skip other model binders i.e. return non-null. - return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); + // This model binder is the only handler for the Body binding source and it cannot run twice. Always + // tell the model binding system to skip other model binders and never to fall back i.e. indicate a + // fatal error. + return new ModelBindingResult(modelBindingKey); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs index d546a7d6d1..8272a1a0dc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs @@ -42,8 +42,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding !string.IsNullOrEmpty(bindingContext.ModelName)) { // Fall back to empty prefix. - newBindingContext = CreateNewBindingContext(bindingContext, - modelName: string.Empty); + newBindingContext = CreateNewBindingContext(bindingContext, modelName: string.Empty); modelBindingResult = await TryBind(newBindingContext); } @@ -99,13 +98,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); if (result != null) { - // Use returned ModelBindingResult if it either indicates the model was set or is related to a - // ModelState entry. The second condition is necessary because the ModelState entry would never be - // validated if caller fell back to the empty prefix, leading to an possibly-incorrect !IsValid. + // Use returned ModelBindingResult if it indicates the model was set, indicates the binder + // encountered a fatal error, or is related to a ModelState entry. // - // In most (hopefully all) cases, the ModelState entry exists because some binders add errors - // before returning a result with !IsModelSet. Those binders often cannot run twice anyhow. - if (result.IsModelSet || bindingContext.ModelState.ContainsKey(bindingContext.ModelName)) + // The second condition is necessary because the BodyModelBinder unconditionally binds during the + // first attempt and does not always create ModelState values using ModelName. + // + // The third condition is necessary because the ModelState entry would never be validated if + // caller fell back to the empty prefix, leading to an possibly-incorrect !IsValid. In most + // (hopefully all) cases, the ModelState entry exists because some binders add errors before + // returning a result with !IsModelSet. Those binders often cannot run twice anyhow. + if (result.IsFatalError || + result.IsModelSet || + bindingContext.ModelState.ContainsKey(bindingContext.ModelName)) { return result; } @@ -121,8 +126,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return null; } - private static ModelBindingContext CreateNewBindingContext(ModelBindingContext oldBindingContext, - string modelName) + private static ModelBindingContext CreateNewBindingContext( + ModelBindingContext oldBindingContext, + string modelName) { var newBindingContext = new ModelBindingContext { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs index 2997e7f8b1..83820ea56f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); var modelBindingResult = result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : + result : new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); // Were able to resolve a binder type. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs index 6d8c267dfb..f44fc94e7d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +#if DNXCORE50 using System.Reflection; +#endif using System.Threading.Tasks; using Microsoft.Framework.Internal; @@ -39,8 +41,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding model = value; } } - else if (typeof(IEnumerable).GetTypeInfo().IsAssignableFrom( - bindingContext.ModelType.GetTypeInfo())) + else if (typeof(IEnumerable).IsAssignableFrom(bindingContext.ModelType)) { var values = request.Headers.GetCommaSeparatedValues(headerName); if (values != null) diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index cb2fc38c40..53b40d0931 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1962,38 +1962,6 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ModelTypeIsWrong"), p0, p1); } - /// - /// The value '{0}' is not valid for {1}. - /// - internal static string ModelBinderUtil_ValueInvalid - { - get { return GetString("ModelBinderUtil_ValueInvalid"); } - } - - /// - /// The value '{0}' is not valid for {1}. - /// - internal static string FormatModelBinderUtil_ValueInvalid(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalid"), p0, p1); - } - - /// - /// The supplied value is invalid for {0}. - /// - internal static string ModelBinderUtil_ValueInvalidGeneric - { - get { return GetString("ModelBinderUtil_ValueInvalidGeneric"); } - } - - /// - /// The supplied value is invalid for {0}. - /// - internal static string FormatModelBinderUtil_ValueInvalidGeneric(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalidGeneric"), p0); - } - /// /// A value for the '{0}' property was not provided. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 1ba3c225fa..6a17f8b56a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -493,12 +493,6 @@ The binding context has a ModelType of '{0}', but this binder can only operate on models of type '{1}'. - - The value '{0}' is not valid for {1}. - - - The supplied value is invalid for {0}. - A value for the '{0}' property was not provided. diff --git a/src/Microsoft.AspNet.Mvc.Extensions/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Extensions/Properties/Resources.Designer.cs index 8ae8c62618..f3d168ad1b 100644 --- a/src/Microsoft.AspNet.Mvc.Extensions/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Extensions/Properties/Resources.Designer.cs @@ -1898,102 +1898,6 @@ namespace Microsoft.AspNet.Mvc.Extensions return GetString("KeyValuePair_BothKeyAndValueMustBePresent"); } - /// - /// The binding context has a null Model, but this binder requires a non-null model of type '{0}'. - /// - internal static string ModelBinderUtil_ModelCannotBeNull - { - get { return GetString("ModelBinderUtil_ModelCannotBeNull"); } - } - - /// - /// The binding context has a null Model, but this binder requires a non-null model of type '{0}'. - /// - internal static string FormatModelBinderUtil_ModelCannotBeNull(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ModelCannotBeNull"), p0); - } - - /// - /// The binding context has a Model of type '{0}', but this binder can only operate on models of type '{1}'. - /// - internal static string ModelBinderUtil_ModelInstanceIsWrong - { - get { return GetString("ModelBinderUtil_ModelInstanceIsWrong"); } - } - - /// - /// The binding context has a Model of type '{0}', but this binder can only operate on models of type '{1}'. - /// - internal static string FormatModelBinderUtil_ModelInstanceIsWrong(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ModelInstanceIsWrong"), p0, p1); - } - - /// - /// The binding context cannot have a null ModelMetadata. - /// - internal static string ModelBinderUtil_ModelMetadataCannotBeNull - { - get { return GetString("ModelBinderUtil_ModelMetadataCannotBeNull"); } - } - - /// - /// The binding context cannot have a null ModelMetadata. - /// - internal static string FormatModelBinderUtil_ModelMetadataCannotBeNull() - { - return GetString("ModelBinderUtil_ModelMetadataCannotBeNull"); - } - - /// - /// The binding context has a ModelType of '{0}', but this binder can only operate on models of type '{1}'. - /// - internal static string ModelBinderUtil_ModelTypeIsWrong - { - get { return GetString("ModelBinderUtil_ModelTypeIsWrong"); } - } - - /// - /// The binding context has a ModelType of '{0}', but this binder can only operate on models of type '{1}'. - /// - internal static string FormatModelBinderUtil_ModelTypeIsWrong(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ModelTypeIsWrong"), p0, p1); - } - - /// - /// The value '{0}' is not valid for {1}. - /// - internal static string ModelBinderUtil_ValueInvalid - { - get { return GetString("ModelBinderUtil_ValueInvalid"); } - } - - /// - /// The value '{0}' is not valid for {1}. - /// - internal static string FormatModelBinderUtil_ValueInvalid(object p0, object p1) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalid"), p0, p1); - } - - /// - /// The supplied value is invalid for {0}. - /// - internal static string ModelBinderUtil_ValueInvalidGeneric - { - get { return GetString("ModelBinderUtil_ValueInvalidGeneric"); } - } - - /// - /// The supplied value is invalid for {0}. - /// - internal static string FormatModelBinderUtil_ValueInvalidGeneric(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinderUtil_ValueInvalidGeneric"), p0); - } - /// /// The '{0}' property is required. /// diff --git a/src/Microsoft.AspNet.Mvc.Extensions/Resources.resx b/src/Microsoft.AspNet.Mvc.Extensions/Resources.resx index 7575c2a6b2..6f2c43bcec 100644 --- a/src/Microsoft.AspNet.Mvc.Extensions/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Extensions/Resources.resx @@ -1,17 +1,17 @@  - @@ -481,24 +481,6 @@ A value is required. - - The binding context has a null Model, but this binder requires a non-null model of type '{0}'. - - - The binding context has a Model of type '{0}', but this binder can only operate on models of type '{1}'. - - - The binding context cannot have a null ModelMetadata. - - - The binding context has a ModelType of '{0}', but this binder can only operate on models of type '{1}'. - - - The value '{0}' is not valid for {1}. - - - The supplied value is invalid for {0}. - The '{0}' property is required. diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs index 14f83ac6af..449a4281cd 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs @@ -77,10 +77,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Returns true because it understands the metadata type. Assert.NotNull(binderResult); + Assert.True(binderResult.IsFatalError); Assert.False(binderResult.IsModelSet); Assert.Null(binderResult.ValidationNode); Assert.Null(binderResult.Model); - Assert.True(bindingContext.ModelState.ContainsKey("someName")); + + // Key is empty because this was a top-level binding. + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal(string.Empty, entry.Key); + Assert.Single(entry.Value.Errors); } [Fact] @@ -99,6 +104,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert Assert.NotNull(binderResult); + Assert.True(binderResult.IsFatalError); Assert.False(binderResult.IsModelSet); Assert.Null(binderResult.ValidationNode); } @@ -167,11 +173,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Returns true because it understands the metadata type. Assert.NotNull(binderResult); + Assert.True(binderResult.IsFatalError); Assert.False(binderResult.IsModelSet); Assert.Null(binderResult.ValidationNode); Assert.Null(binderResult.Model); - Assert.True(bindingContext.ModelState.ContainsKey("someName")); - var errorMessage = bindingContext.ModelState["someName"].Errors[0].Exception.Message; + + // Key is empty because this was a top-level binding. + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal(string.Empty, entry.Key); + var errorMessage = Assert.Single(entry.Value.Errors).Exception.Message; Assert.Equal("Your input is bad!", errorMessage); } @@ -198,13 +208,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert - // Returns true because it understands the metadata type. + // Returns non-null result because it understands the metadata type. Assert.NotNull(binderResult); + Assert.True(binderResult.IsFatalError); Assert.False(binderResult.IsModelSet); Assert.Null(binderResult.Model); Assert.Null(binderResult.ValidationNode); - Assert.True(bindingContext.ModelState.ContainsKey("someName")); - var errorMessage = bindingContext.ModelState["someName"].Errors[0].ErrorMessage; + + // Key is empty because this was a top-level binding. + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal(string.Empty, entry.Key); + var errorMessage = Assert.Single(entry.Value.Errors).ErrorMessage; Assert.Equal("Unsupported content type 'text/xyz'.", errorMessage); } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs index e7a2e35054..439e0ac5e8 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -65,7 +65,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Act var response = await client.PostAsync("http://localhost/Validation/Index", content); - //Assert + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal("User has been registerd : " + sampleName, await response.Content.ReadAsStringAsync()); @@ -89,7 +89,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Act var response = await client.PostAsync("http://localhost/Validation/Index", content); - //Assert + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal("The field Id must be between 1 and 2000.," + "The field Name must be a string or array type with a minimum length of '5'.," + @@ -109,9 +109,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Act var response = await client.PostAsync("http://localhost/Validation/GetDeveloperName", content); - //Assert + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Equal("No model validation for developer, even though developer.Name is empty.", + Assert.Equal("No model validation for developer, even though developer.Name is empty.", await response.Content.ReadAsStringAsync()); } @@ -123,33 +123,33 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var client = server.CreateClient(); var requestData = "{\"Name\":\"Library Manager\", \"Suppliers\": [{\"Name\":\"Contoso Corp\"}]}"; var content = new StringContent(requestData, Encoding.UTF8, "application/json"); - var expectedModelStateErrorMessage + var expectedModelStateErrorMessage = "The field Suppliers must be a string or array type with a minimum length of '2'."; - var shouldNotContainMessage + var shouldNotContainMessage = "The field Name must be a string or array type with a maximum length of '5'."; // Act var response = await client.PostAsync("http://localhost/Validation/CreateProject", content); - //Assert + // Assert Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode); var responseContent = await response.Content.ReadAsStringAsync(); - var responseObject = JsonConvert.DeserializeObject>(responseContent); - var errorCollection = Assert.Single(responseObject, modelState => modelState.Value.Errors.Any()); - var error = Assert.Single(errorCollection.Value.Errors); - Assert.Equal(expectedModelStateErrorMessage, error.ErrorMessage); + var responseObject = JsonConvert.DeserializeObject>(responseContent); + var errorKeyValuePair = Assert.Single(responseObject, keyValuePair => keyValuePair.Value.Length > 0); + var errorMessage = Assert.Single(errorKeyValuePair.Value); + Assert.Equal(expectedModelStateErrorMessage, errorMessage); // verifies that the excluded type is not validated - Assert.NotEqual(shouldNotContainMessage, error.ErrorMessage); + Assert.NotEqual(shouldNotContainMessage, errorMessage); } [Theory] [MemberData(nameof(SimpleTypePropertiesModelRequestData))] - public async Task ShallowValidation_HappensOnExlcuded_SimpleTypeProperties( - string requestContent, - int expectedStatusCode, - string expectedModelStateErrorMessage) + public async Task ShallowValidation_HappensOnExcluded_SimpleTypeProperties( + string requestContent, + int expectedStatusCode, + string expectedModelStateErrorMessage) { // Arrange var server = TestHelper.CreateServer(_app, SiteName, _configureServices); @@ -157,17 +157,18 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var content = new StringContent(requestContent, Encoding.UTF8, "application/json"); // Act - var response = await client.PostAsync("http://localhost/Validation/CreateSimpleTypePropertiesModel", - content); + var response = await client.PostAsync( + "http://localhost/Validation/CreateSimpleTypePropertiesModel", + content); - //Assert + // Assert Assert.Equal(expectedStatusCode, (int)response.StatusCode); var responseContent = await response.Content.ReadAsStringAsync(); - var responseObject = JsonConvert.DeserializeObject>(responseContent); - var errorCollection = Assert.Single(responseObject, modelState => modelState.Value.Errors.Any()); - var error = Assert.Single(errorCollection.Value.Errors); - Assert.Equal(expectedModelStateErrorMessage, error.ErrorMessage); + var responseObject = JsonConvert.DeserializeObject>(responseContent); + var errorKeyValuePair = Assert.Single(responseObject, keyValuePair => keyValuePair.Value.Length > 0); + var errorMessage = Assert.Single(errorKeyValuePair.Value); + Assert.Equal(expectedModelStateErrorMessage, errorMessage); } [Fact] @@ -183,27 +184,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Act var response = await client.PostAsync("http://localhost/Validation/GetDeveloperAlias", content); - //Assert + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal("xyz", await response.Content.ReadAsStringAsync()); } - - private class ErrorCollection - { - public IEnumerable Errors - { - get; - set; - } - } - - private class Error - { - public string ErrorMessage - { - get; - set; - } - } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index 267a8e1d3f..0ed7266f7a 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -11,7 +11,6 @@ using System.Reflection; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Builder; -using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.Framework.DependencyInjection; using ModelBindingWebSite.Models; using ModelBindingWebSite.ViewModels; @@ -39,9 +38,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var response = await client.GetStringAsync("http://localhost/Validation/DoNotValidateParameter"); // Assert - var modelState = JsonConvert.DeserializeObject(response); - Assert.Empty(modelState); - Assert.True(modelState.IsValid); + Assert.Equal("true", response); } [Fact] @@ -52,13 +49,10 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var client = server.CreateClient(); // Act - var response = await client.GetAsync( - "http://localhost/Validation/AvoidRecursive?Name=selfish"); + var response = await client.GetStringAsync("http://localhost/Validation/AvoidRecursive?Name=selfish"); // Assert - var stringValue = await response.Content.ReadAsStringAsync(); - var json = JsonConvert.DeserializeObject(stringValue); - Assert.True(json.IsValid); + Assert.Equal("true", response); } [Theory] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlDataContractSerializerInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlDataContractSerializerInputFormatterTest.cs index 13188033d2..ffd50864fe 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlDataContractSerializerInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlDataContractSerializerInputFormatterTest.cs @@ -49,8 +49,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var data = await response.Content.ReadAsStringAsync(); Assert.Contains( string.Format( - "dummyObject:There was an error deserializing the object of type {0}", - typeof(DummyClass).FullName), + ":There was an error deserializing the object of type {0}.", + typeof(DummyClass).FullName), data); } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlSerializerInputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlSerializerInputFormatterTests.cs index 98095c2ad2..d5ead5dd3c 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlSerializerInputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlSerializerInputFormatterTests.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var data = await response.Content.ReadAsStringAsync(); - Assert.Contains("dummyObject:There is an error in XML document", data); + Assert.Contains(":There is an error in XML document", data); } } } \ No newline at end of file diff --git a/test/WebSites/FormatterWebSite/Controllers/InputFormatterController.cs b/test/WebSites/FormatterWebSite/Controllers/InputFormatterController.cs index 03617f9258..8ac944db6d 100644 --- a/test/WebSites/FormatterWebSite/Controllers/InputFormatterController.cs +++ b/test/WebSites/FormatterWebSite/Controllers/InputFormatterController.cs @@ -14,8 +14,10 @@ namespace FormatterWebSite.Controllers { if (!ActionContext.ModelState.IsValid) { - var parameterBindingErrors = ActionContext.ModelState["dummy"].Errors; - if (parameterBindingErrors.Count != 0) + // Body model binder normally reports errors for parameters using the empty name. + var parameterBindingErrors = ActionContext.ModelState["dummy"]?.Errors ?? + ActionContext.ModelState[string.Empty]?.Errors; + if (parameterBindingErrors != null && parameterBindingErrors.Count != 0) { return new ErrorInfo { diff --git a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs index 452b440f06..92bddc4f36 100644 --- a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.ComponentModel.DataAnnotations; -using System.Threading.Tasks; using Microsoft.AspNet.Mvc; namespace FormatterWebSite @@ -56,10 +54,10 @@ namespace FormatterWebSite } } - // 'Developer' type is excluded but the shallow validation on the + // 'Developer' type is excluded but the shallow validation on the // property Developers should happen [ModelStateValidationFilter] - public IActionResult CreateProject([FromBody]Project project) + public IActionResult CreateProject([FromBody] Project project) { return Json(project); } diff --git a/test/WebSites/FormatterWebSite/Filters/ModelStateValidationFilterAttribute.cs b/test/WebSites/FormatterWebSite/Filters/ModelStateValidationFilterAttribute.cs index e1d90ee5f0..9260275176 100644 --- a/test/WebSites/FormatterWebSite/Filters/ModelStateValidationFilterAttribute.cs +++ b/test/WebSites/FormatterWebSite/Filters/ModelStateValidationFilterAttribute.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNet.Mvc; -using Microsoft.AspNet.WebUtilities; namespace FormatterWebSite { @@ -12,10 +11,7 @@ namespace FormatterWebSite { if (!context.ModelState.IsValid) { - context.Result = new ObjectResult(context.ModelState) - { - StatusCode = StatusCodes.Status400BadRequest - }; + context.Result = new BadRequestObjectResult(context.ModelState); } } } diff --git a/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs b/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs index d493112353..b4d36ba3fb 100644 --- a/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs +++ b/test/WebSites/FormatterWebSite/ValidateBodyParameterAttribute.cs @@ -19,8 +19,10 @@ namespace FormatterWebSite parameter.BindingInfo?.BindingSource)); if (bodyParameter != null) { - var parameterBindingErrors = context.ModelState[bodyParameter.Name].Errors; - if (parameterBindingErrors.Count != 0) + // Body model binder normally reports errors for parameters using the empty name. + var parameterBindingErrors = context.ModelState[bodyParameter.Name]?.Errors ?? + context.ModelState[string.Empty]?.Errors; + if (parameterBindingErrors != null && parameterBindingErrors.Count != 0) { var errorInfo = new ErrorInfo { diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs index 99f6b4904e..13dcea73ca 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs @@ -1,10 +1,7 @@ // 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.Collections.Generic; using Microsoft.AspNet.Mvc; -using Microsoft.AspNet.Mvc.ModelBinding; -using ModelBindingWebSite.Models; namespace ModelBindingWebSite.Controllers { @@ -16,61 +13,19 @@ namespace ModelBindingWebSite.Controllers public object AvoidRecursive(SelfishPerson selfishPerson) { - return new SerializableModelStateDictionary(ModelState); + return ModelState.IsValid; } public object DoNotValidateParameter([FromServices] ITestService service) { - return ModelState; + return ModelState.IsValid; } } - public class SerializableModelStateDictionary : Dictionary - { - public bool IsValid { get; set; } - - public int ErrorCount { get; set; } - - public SerializableModelStateDictionary(ModelStateDictionary modelState) - { - var errorCount = 0; - foreach (var keyModelStatePair in modelState) - { - var key = keyModelStatePair.Key; - var value = keyModelStatePair.Value; - errorCount += value.Errors.Count; - var entry = new Entry() - { - Errors = value.Errors, - RawValue = value.Value.RawValue, - AttemptedValue = value.Value.AttemptedValue, - ValidationState = value.ValidationState - }; - - Add(key, entry); - } - - IsValid = modelState.IsValid; - ErrorCount = errorCount; - } - } - - public class Entry - { - public ModelValidationState ValidationState { get; set; } - - public ModelErrorCollection Errors { get; set; } - - public object RawValue { get; set; } - - public string AttemptedValue { get; set; } - - } - - public class SelfishPerson { public string Name { get; set; } + public SelfishPerson MySelf { get { return this; } } } } \ No newline at end of file