From 0bbe1c6cdcecd201fe2d0f7eba0aa8d9b5d724f2 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Tue, 21 Jul 2015 19:34:49 +0200 Subject: [PATCH] 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 --- .../Macros/PartialViewMacroEngine.cs | 2 +- src/Umbraco.Web/ModelStateExtensions.cs | 52 +++++-------------- src/Umbraco.Web/Mvc/ControllerExtensions.cs | 19 +++---- .../MergeModelStateToChildActionAttribute.cs | 2 +- src/Umbraco.Web/Mvc/UmbracoPageResult.cs | 4 +- 5 files changed, 27 insertions(+), 52 deletions(-) diff --git a/src/Umbraco.Web/Macros/PartialViewMacroEngine.cs b/src/Umbraco.Web/Macros/PartialViewMacroEngine.cs index e1b57709f3..bf3586a174 100644 --- a/src/Umbraco.Web/Macros/PartialViewMacroEngine.cs +++ b/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(); diff --git a/src/Umbraco.Web/ModelStateExtensions.cs b/src/Umbraco.Web/ModelStateExtensions.cs index 29f4eff120..08868e4e5f 100644 --- a/src/Umbraco.Web/ModelStateExtensions.cs +++ b/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 { - /// - /// Safely merges ModelState - /// - /// - /// - /// The MVC5 System.Web.Mvc.ModelStateDictionary.Merge method is not safe - 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; - } - /// - /// Merges ModelState that has names matching the prefix - /// - /// - /// - /// - public static void Merge(this ModelStateDictionary state, ModelStateDictionary dictionary, string prefix) + /// + /// Merges ModelState that has names matching the prefix + /// + /// + /// + /// + 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; - } + } /// /// Checks if there are any model errors on any fields containing the prefix diff --git a/src/Umbraco.Web/Mvc/ControllerExtensions.cs b/src/Umbraco.Web/Mvc/ControllerExtensions.cs index 28626e70fb..f63cae1dcb 100644 --- a/src/Umbraco.Web/Mvc/ControllerExtensions.cs +++ b/src/Umbraco.Web/Mvc/ControllerExtensions.cs @@ -138,17 +138,18 @@ namespace Umbraco.Web.Mvc /// 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)) diff --git a/src/Umbraco.Web/Mvc/MergeModelStateToChildActionAttribute.cs b/src/Umbraco.Web/Mvc/MergeModelStateToChildActionAttribute.cs index 28eee44939..7d2df43962 100644 --- a/src/Umbraco.Web/Mvc/MergeModelStateToChildActionAttribute.cs +++ b/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); } } diff --git a/src/Umbraco.Web/Mvc/UmbracoPageResult.cs b/src/Umbraco.Web/Mvc/UmbracoPageResult.cs index 70200c627a..f23515c1d9 100644 --- a/src/Umbraco.Web/Mvc/UmbracoPageResult.cs +++ b/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 /// 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;