U4-6721 Error when submitting Macros, Collection was modified; enumer…

…ation operation may not execute. (after project has been updated to MVC5)

#U4-6721 In Progress

(cherry picked from commit 34f44b0)

src/Umbraco.Web/Macros/PartialViewMacroEngine.cs

@@ -143,7 +143,7 @@ public string Execute(MacroModel macro, IPublishedContent content)
 				//bubble up the model state from the main view context to our custom controller.
 				//when merging we'll create a new dictionary, otherwise you might run into an enumeration error
 				// caused from ModelStateDictionary
-				controller.ModelState.Merge(new ModelStateDictionary(viewContext.ViewData.ModelState));
+				controller.ModelState.MergeSafe(new ModelStateDictionary(viewContext.ViewData.ModelState));
 				controller.ControllerContext = new ControllerContext(request, controller);
 				//call the action to render
                 var result = controller.Index();

src/Umbraco.Web/ModelStateExtensions.cs

@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
 using System.Linq;
@@ -8,24 +7,51 @@ namespace Umbraco.Web
 {
 	internal static class ModelStateExtensions
 	{
+        /// <summary>
+        /// Safely merges ModelState
+        /// </summary>
+        /// <param name="state"></param>
+        /// <param name="dictionary"></param>
+        /// <remarks>The MVC5 System.Web.Mvc.ModelStateDictionary.Merge method is not safe</remarks>
+        public static void MergeSafe(this ModelStateDictionary state, ModelStateDictionary dictionary)
+        {
+            if (dictionary == null)
+                return;
+            // Need to stuff this into a temporary new dictionary that we're allowed to alter, 
+            // if we alter "state" in this enumeration, it fails with 
+            // "Collection was modified; enumeration operation may not execute"
+            var tempDictionary = new ModelStateDictionary(state);
+	        foreach (var entryKey in dictionary.Keys)
+            {
+                tempDictionary[entryKey] = dictionary[entryKey];
+            }
+            // Update state with updated dictionary
+	        state = tempDictionary;
+	    }
 
-		/// <summary>
-		/// Merges ModelState that has names matching the prefix
-		/// </summary>
-		/// <param name="state"></param>
-		/// <param name="dictionary"></param>
-		/// <param name="prefix"></param>
-		public static void Merge(this ModelStateDictionary state, ModelStateDictionary dictionary, string prefix)
+	    /// <summary>
+        /// Merges ModelState that has names matching the prefix
+        /// </summary>
+        /// <param name="state"></param>
+        /// <param name="dictionary"></param>
+        /// <param name="prefix"></param>
+        public static void Merge(this ModelStateDictionary state, ModelStateDictionary dictionary, string prefix)
 		{
-			if (dictionary == null)
-				return;
-			foreach (var keyValuePair in dictionary
+            if (dictionary == null)
+                return;
+            // Need to stuff this into a temporary new dictionary that we're allowed to alter, 
+            // if we alter "state" in this enumeration, it fails with 
+            // "Collection was modified; enumeration operation may not execute"
+            var tempDictionary = new ModelStateDictionary(state);
+            foreach (var keyValuePair in dictionary
                 //It can either equal the prefix exactly (model level errors) or start with the prefix. (property level errors)
                 .Where(keyValuePair => keyValuePair.Key == prefix || keyValuePair.Key.StartsWith(prefix + ".")))
 			{
-				state[keyValuePair.Key] = keyValuePair.Value;
+                tempDictionary[keyValuePair.Key] = keyValuePair.Value;
 			}
-		}
+            // Update state with updated dictionary
+            state = tempDictionary;
+        }
 
 		/// <summary>
 		/// Checks if there are any model errors on any fields containing the prefix

src/Umbraco.Web/Mvc/ControllerExtensions.cs

@@ -140,7 +140,7 @@ internal static void EnsureViewObjectDataOnResult(this ControllerBase controller
         {
 			//when merging we'll create a new dictionary, otherwise you might run into an enumeration error
 			// caused from ModelStateDictionary
-			result.ViewData.ModelState.Merge(new ModelStateDictionary(controller.ViewData.ModelState));
+			result.ViewData.ModelState.MergeSafe(new ModelStateDictionary(controller.ViewData.ModelState));
 
             // Temporarily copy the dictionary to avoid enumerator-modification errors
             var newViewDataDict = new ViewDataDictionary(controller.ViewData);

src/Umbraco.Web/Mvc/MergeModelStateToChildActionAttribute.cs

@@ -32,7 +32,7 @@ public override void OnActionExecuting(ActionExecutingContext filterContext)
 			{
 				if (filterContext.Controller.ControllerContext.IsChildAction)
 				{
-					filterContext.Controller.ViewData.ModelState.Merge(
+					filterContext.Controller.ViewData.ModelState.MergeSafe(
 						filterContext.Controller.ControllerContext.ParentActionViewContext.ViewData.ModelState);
 				}
 			}

src/Umbraco.Web/Mvc/UmbracoPageResult.cs

@@ -94,7 +94,7 @@ private static void EnsureViewContextForWebForms(ControllerContext context)
             tempDataDictionary.Save(context, new SessionStateTempDataProvider());
             var viewCtx = new ViewContext(context, new DummyView(), new ViewDataDictionary(), tempDataDictionary, new StringWriter());
 
-            viewCtx.ViewData.ModelState.Merge(context.Controller.ViewData.ModelState);
+            viewCtx.ViewData.ModelState.MergeSafe(context.Controller.ViewData.ModelState);
 
             foreach (var d in context.Controller.ViewData)
                 viewCtx.ViewData[d.Key] = d.Value;
@@ -108,7 +108,7 @@ private static void EnsureViewContextForWebForms(ControllerContext context)
         /// </summary>
         private static void CopyControllerData(ControllerContext context, ControllerBase controller)
         {
-            controller.ViewData.ModelState.Merge(context.Controller.ViewData.ModelState);
+            controller.ViewData.ModelState.MergeSafe(context.Controller.ViewData.ModelState);
 
             foreach (var d in context.Controller.ViewData)
                 controller.ViewData[d.Key] = d.Value;