diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs index 8dc60b9420..647a76da5c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs @@ -174,10 +174,54 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Adds the specified to the instance - /// that is associated with the specified . + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// + /// + /// This method allows adding the to the current + /// when is not available or the exact + /// must be maintained for later use (even if it is for example a ). + /// Where is available, use instead. + /// /// The key of the to add errors to. /// The to add. + /// + /// True if the given error was added, false if the error was ignored. + /// See . + /// + public bool TryAddModelException(string key, Exception exception) + { + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + + if (exception == null) + { + throw new ArgumentNullException(nameof(exception)); + } + + if (ErrorCount >= MaxAllowedErrors - 1) + { + EnsureMaxErrorsReachedRecorded(); + return false; + } + + ErrorCount++; + AddModelErrorCore(key, exception); + return true; + } + + /// + /// Adds the specified to the instance + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. + /// + /// The key of the to add errors to. + /// The to add. Some exception types will be replaced with + /// a descriptive error message. /// The associated with the model. public void AddModelError(string key, Exception exception, ModelMetadata metadata) { @@ -202,10 +246,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Attempts to add the specified to the /// instance that is associated with the specified . If the maximum number of allowed - /// errors has already been recorded, records a exception instead. + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// /// The key of the to add errors to. - /// The to add. + /// The to add. Some exception types will be replaced with + /// a descriptive error message. /// The associated with the model. /// /// True if the given error was added, false if the error was ignored. @@ -277,7 +323,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Adds the specified to the instance - /// that is associated with the specified . + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// /// The key of the to add errors to. /// The error message to add. @@ -299,7 +347,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Attempts to add the specified to the /// instance that is associated with the specified . If the maximum number of allowed - /// errors has already been recorded, records a exception instead. + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// /// The key of the to add errors to. /// The error message to add. diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs index 7bba44132e..7e023c4b7f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs @@ -15,7 +15,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { /// /// Adds the specified to the instance - /// that is associated with the specified . + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// /// The type of the model. /// The instance this method extends. @@ -46,7 +48,42 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Adds the specified to the instance - /// that is associated with the specified . + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. + /// + /// + /// This method allows adding the to the current + /// when is not available or the exact + /// must be maintained for later use (even if it is for example a ). + /// + /// The type of the model. + /// The instance this method extends. + /// An expression to be evaluated against an item in the current model. + /// The to add. + public static void TryAddModelException( + this ModelStateDictionary modelState, + Expression> expression, + Exception exception) + { + if (modelState == null) + { + throw new ArgumentNullException(nameof(modelState)); + } + + if (expression == null) + { + throw new ArgumentNullException(nameof(expression)); + } + + modelState.TryAddModelException(GetExpressionText(expression), exception); + } + + /// + /// Adds the specified to the instance + /// that is associated with the specified . If the maximum number of allowed + /// errors has already been recorded, ensures that a exception is + /// recorded instead. /// /// The type of the model. /// The instance this method extends. diff --git a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs index c06668528c..7fb7de7464 100644 --- a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs @@ -285,6 +285,23 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal(entry.ValidationState, actual.ValidationState); } + [Fact] + public void TryAddModelException_Succeeds() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new TestException(); + + // Act + dictionary.TryAddModelException("some key", exception); + + // Assert + var kvp = Assert.Single(dictionary); + Assert.Equal("some key", kvp.Key); + var error = Assert.Single(kvp.Value.Errors); + Assert.Same(exception, error.Exception); + } + [Fact] public void AddModelErrorCreatesModelStateIfNotPresent() { @@ -695,13 +712,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding }; var provider = new EmptyModelMetadataProvider(); var metadata = provider.GetMetadataForProperty(typeof(string), nameof(string.Length)); + + // Act dictionary.AddModelError("key1", "error1"); dictionary.AddModelError("key2", new Exception(), metadata); dictionary.AddModelError("key3", new Exception(), metadata); dictionary.AddModelError("key4", "error4"); dictionary.AddModelError("key5", "error5"); - // Act and Assert + // Assert Assert.True(dictionary.HasReachedMaxErrors); Assert.Equal(5, dictionary.ErrorCount); var error = Assert.Single(dictionary[string.Empty].Errors); @@ -712,6 +731,35 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.DoesNotContain("key5", dictionary.Keys); } + [Fact] + public void TryAddModelException_ReturnsFalse_AndAddsMaxModelErrorMessage() + { + // Arrange + var expected = "The maximum number of allowed model errors has been reached."; + var dictionary = new ModelStateDictionary + { + MaxAllowedErrors = 3 + }; + + // Act and Assert + var result = dictionary.TryAddModelError("key1", "error1"); + Assert.True(result); + + result = dictionary.TryAddModelException("key2", new Exception()); + Assert.True(result); + + result = dictionary.TryAddModelException("key3", new Exception()); + Assert.False(result); + + Assert.Equal(3, dictionary.Count); + var error = Assert.Single(dictionary[string.Empty].Errors); + Assert.IsType(error.Exception); + Assert.Equal(expected, error.Exception.Message); + + // TooManyModelErrorsException added instead of key3 exception. + Assert.DoesNotContain("key3", dictionary.Keys); + } + [Fact] public void TryAddModelError_WithErrorString_ReturnsFalse_AndAddsMaxModelErrorMessage() { @@ -864,6 +912,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal(expected, canAdd); } + [Fact] + public void ModelStateDictionary_ReturnExceptionMessage_WhenModelStateNotSet() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new FormatException("The supplied value is invalid for Length."); + + // Act + dictionary.TryAddModelException("key", exception); + + // Assert + var error = Assert.Single(dictionary["key"].Errors); + Assert.Same(exception, error.Exception); + } + [Fact] public void ModelStateDictionary_ReturnGenericErrorMessage_WhenModelStateNotSet() { @@ -933,6 +996,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal(expected, error.ErrorMessage); } + [Fact] + public void TryAddModelException_ReturnExceptionMessage_WhenModelStateSet() + { + // Arrange + var dictionary = new ModelStateDictionary(); + dictionary.SetModelValue("key", new string[] { "some value" }, "some value"); + var exception = new FormatException("The value 'some value' is not valid for Length."); + + // Act + dictionary.TryAddModelException("key", exception); + + // Assert + var error = Assert.Single(dictionary["key"].Errors); + Assert.Same(exception, error.Exception); + } + [Fact] public void ModelStateDictionary_ReturnSpecificErrorMessage_WhenModelStateSet() { @@ -1022,6 +1101,23 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Empty(error.ErrorMessage); } + [Fact] + public void TryAddModelException_AddsErrorMessage_ForInputFormatterException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new InputFormatterException("This is an InputFormatterException."); + + // Act + dictionary.TryAddModelException("key", exception); + + // Assert + var entry = Assert.Single(dictionary); + Assert.Equal("key", entry.Key); + var error = Assert.Single(entry.Value.Errors); + Assert.Same(exception, error.Exception); + } + [Fact] public void ModelStateDictionary_AddsErrorMessage_ForInputFormatterException() { @@ -1371,4 +1467,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public MvcOptions Value { get; } = new MvcOptions(); } } + + internal class TestException : Exception + { + public TestException() + { + Message = "This is a test exception"; + } + + public override string Message { get; } + } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ModelStateDictionaryExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ModelStateDictionaryExtensionsTest.cs index aca11c043c..80c47900db 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ModelStateDictionaryExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ModelStateDictionaryExtensionsTest.cs @@ -78,7 +78,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void AddModelError_ForSingleExpression_AddsExpectedException() + public void TryAddModelException_ForSingleExpression_AddsExpectedException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new Exception(); + + // Act + dictionary.TryAddModelException(model => model.Text, exception); + + // Assert + var modelState = Assert.Single(dictionary); + var modelError = Assert.Single(modelState.Value.Errors); + + Assert.Equal("Text", modelState.Key); + Assert.Same(exception, modelError.Exception); + } + + [Fact] + public void AddModelError_ForSingleExpression_AddsExpectedException_WithModelMetadata() { // Arrange var dictionary = new ModelStateDictionary(); @@ -98,7 +116,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void AddModelError_ForRelationExpression_AddsExpectedException() + public void TryAddModelException_ForRelationExpression_AddsExpectedException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new Exception(); + + // Act + dictionary.TryAddModelException(model => model.Child.Text, exception); + + // Assert + var modelState = Assert.Single(dictionary); + var modelError = Assert.Single(modelState.Value.Errors); + + Assert.Equal("Child.Text", modelState.Key); + Assert.Same(exception, modelError.Exception); + } + + [Fact] + public void AddModelError_ForRelationExpression_AddsExpectedException_WithModelMetadata() { // Arrange var dictionary = new ModelStateDictionary(); @@ -118,7 +154,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void AddModelError_ForImplicitlyCastedToObjectExpression_AddsExpectedException() + public void TryAddModelException_ForImplicitlyCastedToObjectExpression_AddsExpectedException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var exception = new Exception(); + + // Act + dictionary.TryAddModelException(model => model.Child.Value, exception); + + // Assert + var modelState = Assert.Single(dictionary); + var modelError = Assert.Single(modelState.Value.Errors); + + Assert.Equal("Child.Value", modelState.Key); + Assert.Same(exception, modelError.Exception); + } + + [Fact] + public void AddModelError_ForImplicitlyCastedToObjectExpression_AddsExpectedException_WithModelMetadata() { // Arrange var dictionary = new ModelStateDictionary(); @@ -138,7 +192,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void AddModelError_ForNotModelsExpression_AddsExpectedException() + public void TryAddModelException_ForNotModelsExpression_AddsExpectedException() + { + // Arrange + var dictionary = new ModelStateDictionary(); + var variable = "Test"; + var exception = new Exception(); + + // Act + dictionary.TryAddModelException(model => variable, exception); + + // Assert + var modelState = Assert.Single(dictionary); + var modelError = Assert.Single(modelState.Value.Errors); + + Assert.Equal("variable", modelState.Key); + Assert.Same(exception, modelError.Exception); + } + + [Fact] + public void AddModelError_ForNotModelsExpression_AddsExpectedException_WithModelMetadata() { // Arrange var dictionary = new ModelStateDictionary();