U4-6721 Error when submitting Macros, Collection was modified; enumeration operation may not execute. (after project has been updated to MVC5)

#U4-6721 In Progress

src/Umbraco.Web/Macros/PartialViewMacroEngine.cs

@@ -143,7 +143,7 @@ namespace Umbraco.Web.Macros
 				//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.MergeSafe(new ModelStateDictionary(viewContext.ViewData.ModelState));
+				controller.ModelState.Merge(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,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
 using System.Linq;
@@ -7,51 +8,24 @@ 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;
-            // 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
+			if (dictionary == null)
+				return;
+			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 + ".")))
 			{
-                tempDictionary[keyValuePair.Key] = keyValuePair.Value;
+				state[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

@@ -138,17 +138,18 @@ namespace Umbraco.Web.Mvc
         /// <param name="controller"></param>
         internal static void EnsureViewObjectDataOnResult(this ControllerBase controller, ViewResultBase result)
         {
-			//when merging we'll create a new dictionary, otherwise you might run into an enumeration error
-			// caused from ModelStateDictionary
-			result.ViewData.ModelState.MergeSafe(new ModelStateDictionary(controller.ViewData.ModelState));
-
-            // Temporarily copy the dictionary to avoid enumerator-modification errors
-            var newViewDataDict = new ViewDataDictionary(controller.ViewData);
-            foreach (var d in newViewDataDict)
-                result.ViewData[d.Key] = d.Value;
-
             result.TempData = controller.TempData;
 
+            var newViewDataDict = new ViewDataDictionary(controller.ViewData);
+            var viewDataDictionary = result.ViewData;
+            foreach (var d in newViewDataDict)
+                viewDataDictionary[d.Key] = d.Value;
+
+            result.ViewData = viewDataDictionary;
+
+            foreach (var keyValuePair in controller.ViewData.ModelState.Keys)
+                result.ViewData[keyValuePair] = keyValuePair;
+            
             if (result.View != null) return;
 
             if (string.IsNullOrEmpty(result.ViewName))

src/Umbraco.Web/Mvc/MergeModelStateToChildActionAttribute.cs

@@ -32,7 +32,7 @@ namespace Umbraco.Web.Mvc
 			{
 				if (filterContext.Controller.ControllerContext.IsChildAction)
 				{
-					filterContext.Controller.ViewData.ModelState.MergeSafe(
+					filterContext.Controller.ViewData.ModelState.Merge(
 						filterContext.Controller.ControllerContext.ParentActionViewContext.ViewData.ModelState);
 				}
 			}

src/Umbraco.Web/Mvc/UmbracoPageResult.cs

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