Add overload for AddModelError without ModelMetadata (#7407)

Addresses #6102

Author: jbagga

src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs

@@ -174,10 +174,54 @@ public ModelStateEntry this[string key]
 
         /// <summary>
         /// Adds the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/> instance
-        /// that is associated with the specified <paramref name="key"/>.
+        /// that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
+        /// <remarks>
+        /// This method allows adding the <paramref name="exception"/> to the current <see cref="ModelStateDictionary"/>
+        /// when <see cref="ModelMetadata"/> is not available or the exact <paramref name="exception"/> 
+        /// must be maintained for later use (even if it is for example a <see cref="FormatException"/>).
+        /// Where <see cref="ModelMetadata"/> is available, use <see cref="AddModelError(string, Exception, ModelMetadata)"/> instead.
+        /// </remarks>
         /// <param name="key">The key of the <see cref="ModelStateEntry"/> to add errors to.</param>
         /// <param name="exception">The <see cref="Exception"/> to add.</param>
+        /// <returns>
+        /// <c>True</c> if the given error was added, <c>false</c> if the error was ignored.
+        /// See <see cref="MaxAllowedErrors"/>.
+        /// </returns>
+        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;
+        }
+
+        /// <summary>
+        /// Adds the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/> instance
+        /// that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
+        /// </summary>
+        /// <param name="key">The key of the <see cref="ModelStateEntry"/> to add errors to.</param>
+        /// <param name="exception">The <see cref="Exception"/> to add. Some exception types will be replaced with
+        /// a descriptive error message.</param>
         /// <param name="metadata">The <see cref="ModelMetadata"/> associated with the model.</param>
         public void AddModelError(string key, Exception exception, ModelMetadata metadata)
         {
@@ -202,10 +246,12 @@ public void AddModelError(string key, Exception exception, ModelMetadata metadat
         /// <summary>
         /// Attempts to add the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/>
         /// instance that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
-        /// errors has already been recorded, records a <see cref="TooManyModelErrorsException"/> exception instead.
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
         /// <param name="key">The key of the <see cref="ModelStateEntry"/> to add errors to.</param>
-        /// <param name="exception">The <see cref="Exception"/> to add.</param>
+        /// <param name="exception">The <see cref="Exception"/> to add. Some exception types will be replaced with
+        /// a descriptive error message.</param>
         /// <param name="metadata">The <see cref="ModelMetadata"/> associated with the model.</param>
         /// <returns>
         /// <c>True</c> if the given error was added, <c>false</c> if the error was ignored.
@@ -277,7 +323,9 @@ public bool TryAddModelError(string key, Exception exception, ModelMetadata meta
 
         /// <summary>
         /// Adds the specified <paramref name="errorMessage"/> to the <see cref="ModelStateEntry.Errors"/> instance
-        /// that is associated with the specified <paramref name="key"/>.
+        /// that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
         /// <param name="key">The key of the <see cref="ModelStateEntry"/> to add errors to.</param>
         /// <param name="errorMessage">The error message to add.</param>
@@ -299,7 +347,8 @@ public void AddModelError(string key, string errorMessage)
         /// <summary>
         /// Attempts to add the specified <paramref name="errorMessage"/> to the <see cref="ModelStateEntry.Errors"/>
         /// instance that is associated with the specified <paramref name="key"/>. If the maximum number of allowed
-        /// errors has already been recorded, records a <see cref="TooManyModelErrorsException"/> exception instead.
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
         /// <param name="key">The key of the <see cref="ModelStateEntry"/> to add errors to.</param>
         /// <param name="errorMessage">The error message to add.</param>

src/Microsoft.AspNetCore.Mvc.ViewFeatures/ModelStateDictionaryExtensions.cs

@@ -15,7 +15,9 @@ public static class ModelStateDictionaryExtensions
     {
         /// <summary>
         /// Adds the specified <paramref name="errorMessage"/> to the <see cref="ModelStateEntry.Errors"/> instance
-        /// that is associated with the specified <paramref name="expression"/>.
+        /// that is associated with the specified <paramref name="expression"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
         /// <typeparam name="TModel">The type of the model.</typeparam>
         /// <param name="modelState">The <see cref="ModelStateDictionary"/> instance this method extends.</param>
@@ -46,7 +48,42 @@ public static void AddModelError<TModel>(
 
         /// <summary>
         /// Adds the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/> instance
-        /// that is associated with the specified <paramref name="expression"/>.
+        /// that is associated with the specified <paramref name="expression"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
+        /// </summary>
+        /// <remarks>
+        /// This method allows adding the <paramref name="exception"/> to the current <see cref="ModelStateDictionary"/>
+        /// when <see cref="ModelMetadata"/> is not available or the exact <paramref name="exception"/> 
+        /// must be maintained for later use (even if it is for example a <see cref="FormatException"/>).
+        /// </remarks>
+        /// <typeparam name="TModel">The type of the model.</typeparam>
+        /// <param name="modelState">The <see cref="ModelStateDictionary"/> instance this method extends.</param>
+        /// <param name="expression">An expression to be evaluated against an item in the current model.</param>
+        /// <param name="exception">The <see cref="Exception"/> to add.</param>
+        public static void TryAddModelException<TModel>(
+            this ModelStateDictionary modelState,
+            Expression<Func<TModel, object>> expression,
+            Exception exception)
+        {
+            if (modelState == null)
+            {
+                throw new ArgumentNullException(nameof(modelState));
+            }
+
+            if (expression == null)
+            {
+                throw new ArgumentNullException(nameof(expression));
+            }
+
+            modelState.TryAddModelException(GetExpressionText(expression), exception);
+        }
+
+        /// <summary>
+        /// Adds the specified <paramref name="exception"/> to the <see cref="ModelStateEntry.Errors"/> instance
+        /// that is associated with the specified <paramref name="expression"/>. If the maximum number of allowed
+        /// errors has already been recorded, ensures that a <see cref="TooManyModelErrorsException"/> exception is
+        /// recorded instead.
         /// </summary>
         /// <typeparam name="TModel">The type of the model.</typeparam>
         /// <param name="modelState">The <see cref="ModelStateDictionary"/> instance this method extends.</param>

test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs

@@ -285,6 +285,23 @@ public void CopyConstructor_CopiesModelStateData()
             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 @@ public void AddModelError_WithErrorString_AddsTooManyModelErrors_WhenMaxErrorsIs
             };
             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 @@ public void AddModelError_WithErrorString_AddsTooManyModelErrors_WhenMaxErrorsIs
             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<TooManyModelErrorsException>(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 @@ public void ModelStateDictionary_HasReachedMaxErrors(int errorCount, bool expect
             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 @@ public void ModelStateDictionary_AddsCustomErrorMessage_WhenModelStateNotSet_Wit
             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 @@ public void ModelStateDictionary_NoErrorMessage_ForUnrecognizedException()
             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 @@ private class OptionsAccessor : IOptions<MvcOptions>
             public MvcOptions Value { get; } = new MvcOptions();
         }
     }
+
+    internal class TestException : Exception
+    {
+        public TestException()
+        {
+            Message = "This is a test exception";
+        }
+
+        public override string Message { get; }
+    }
 }