Conditional ModelState Merge

Question

I implemented the 2nd response to the "Preserve ModelState Errors Across RedirectToAction?" question which involves using two custom ActionFilterAttributes. I like the solution, it keeps the code clean by just adding an attribute to methods that need the functionality.

The solution works well in most cases, but I've run across an issue with a repeating Partial View. Basically I have Partial View that uses it's own Model, separate from the Model that parent view uses.

The simplified version of my code from the main View:

@for (int i = 0; i < Model.Addresses.Count; i++)
{
        address = (Address)Model.Addresses[i];
        @Html.Partial("_AddressModal", address);
}

The Partial View "_AddressModal":

@model Acme.Domain.Models.Address
[...]
@Html.TextBoxFor(model => model.Address1, new { @class = "form-control" } )
[...]

When not using the custom ActionFilterAttributes everything works as expected. With each execution of the Partial View, the lamba expressions such "model => model.Address1" pull the correct value from ModelState.

The problem is when I get redirect and have used the custom ActionFilterAttributes. The core problem is that not only is the ModelState for the one instance of Address updated, but the ModelState of all the Addresses built by the Partial View get overwritten so they contain the same values, instead of the correct instance values.

My question is how do I modify the custom ActionFilterAttributes so that it only updates the ModelState of the one affected Address instance, not all the ModelStates? I want to avoid adding anything to the methods that use the attribute, to keep the clean implementation.

Here is the custom ActionFilterAttributes code from the other question:

public class SetTempDataModelStateAttribute : ActionFilterAttribute
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        base.OnActionExecuted(filterContext);         
        filterContext.Controller.TempData["ModelState"] = 
           filterContext.Controller.ViewData.ModelState;
    }
}

public class RestoreModelStateFromTempDataAttribute : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        base.OnActionExecuting(filterContext);
        if (filterContext.Controller.TempData.ContainsKey("ModelState"))
        {
            filterContext.Controller.ViewData.ModelState.Merge(
                (ModelStateDictionary)filterContext.Controller.TempData["ModelState"]);
        }
    }
}

Answer 1

Check out if this implementation (ben foster) does work : I used it heavily and never had a problem.

Are you setting the attributes correctly ? ' RestoreModelStateFromTempDataAttribute on the get action and SetTempDataModelState on your post action ?

Here are the 4 classes needed (Export, Import, Transfer and Validate)ModelState

 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
    public class ExportModelStateToTempDataAttribute : ModelStateTempDataTransfer
    {
        public override void OnActionExecuted(ActionExecutedContext filterContext)
        {
            // Only copy when ModelState is invalid and we're performing a Redirect (i.e. PRG)
            if (!filterContext.Controller.ViewData.ModelState.IsValid &&
                (filterContext.Result is RedirectResult || filterContext.Result is RedirectToRouteResult)) 
            {
                ExportModelStateToTempData(filterContext);
            }

            base.OnActionExecuted(filterContext);
        }
    }


 /// <summary>
    /// An Action Filter for importing ModelState from TempData.
    /// You need to decorate your GET actions with this when using the <see cref="ValidateModelStateAttribute"/>.
    /// </summary>
    /// <remarks>
    /// Useful when following the PRG (Post, Redirect, Get) pattern.
    /// </remarks>
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
    public class ImportModelStateFromTempDataAttribute : ModelStateTempDataTransfer
    {
        public override void OnActionExecuted(ActionExecutedContext filterContext)
        {
            // Only copy from TempData if we are rendering a View/Partial
            if (filterContext.Result is ViewResult)
            {
                ImportModelStateFromTempData(filterContext);
            }
            else 
            {
                // remove it
                RemoveModelStateFromTempData(filterContext);
            }

            base.OnActionExecuted(filterContext);
        }
    }

 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
    public abstract class ModelStateTempDataTransfer : ActionFilterAttribute
    {
        protected static readonly string Key = typeof(ModelStateTempDataTransfer).FullName;

        /// <summary>
        /// Exports the current ModelState to TempData (available on the next request).
        /// </summary>       
        protected static void ExportModelStateToTempData(ControllerContext context)
        {
            context.Controller.TempData[Key] = context.Controller.ViewData.ModelState;
        }

        /// <summary>
        /// Populates the current ModelState with the values in TempData
        /// </summary>
        protected static void ImportModelStateFromTempData(ControllerContext context)
        {
            var prevModelState = context.Controller.TempData[Key] as ModelStateDictionary;
            context.Controller.ViewData.ModelState.Merge(prevModelState);
        }

        /// <summary>
        /// Removes ModelState from TempData
        /// </summary>
        protected static void RemoveModelStateFromTempData(ControllerContext context)
        {
            context.Controller.TempData[Key] = null;
        }
    }

  /// <summary>
    /// An ActionFilter for automatically validating ModelState before a controller action is executed.
    /// Performs a Redirect if ModelState is invalid. Assumes the <see cref="ImportModelStateFromTempDataAttribute"/> is used on the GET action.
    /// </summary>
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
    public class ValidateModelStateAttribute : ModelStateTempDataTransfer
    {
        public override void OnActionExecuting(ActionExecutingContext filterContext)
        {
            if (!filterContext.Controller.ViewData.ModelState.IsValid)
            {
                if (filterContext.HttpContext.Request.IsAjaxRequest())
                {
                    ProcessAjax(filterContext);
                }
                else
                {
                    ProcessNormal(filterContext);
                }
            }

            base.OnActionExecuting(filterContext);
        }

        protected virtual void ProcessNormal(ActionExecutingContext filterContext)
        {
            // Export ModelState to TempData so it's available on next request
            ExportModelStateToTempData(filterContext);

            // redirect back to GET action
            filterContext.Result = new RedirectToRouteResult(filterContext.RouteData.Values);
        }

        protected virtual void ProcessAjax(ActionExecutingContext filterContext)
        {
            var errors = filterContext.Controller.ViewData.ModelState.ToSerializableDictionary();
            var json = new JavaScriptSerializer().Serialize(errors);

            // send 400 status code (Bad Request)
            filterContext.Result = new HttpStatusCodeResult((int)HttpStatusCode.BadRequest, json);
        }
    }

EDIT

This is a normal (non action filter) PRG pattern :

    [HttpGet]
    public async Task<ActionResult> Edit(Guid id)
    {
        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (calendarEvent == null) return this.RedirectToAction<CalendarController>(c => c.Index());
        var model = new CalendarEditViewModel(calendarEvent);
        ViewData.Model = model;
        return View();
    }

    [HttpPost]
    public async Task<ActionResult> Edit(Guid id, CalendarEventBindingModel binding)
    {
        if (!ModelState.IsValid) return await Edit(id);

        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (calendarEvent != null)
        {
            CalendarEvent model = calendarService.Update(calendarEvent, binding);
            await context.SaveChangesAsync();
        }
        return this.RedirectToAction<CalendarController>(c => c.Index());
    }

What do you want to avoid with action filters (or their purpose) is to remove the ModelState.IsValid check on every post Action, so the same (with action filters) would be :

    [HttpGet, ImportModelStateFromTempData]
    public async Task<ActionResult> Edit(Guid id)
    {
        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (calendarEvent == null) return this.RedirectToAction<CalendarController>(c => c.Index());
        var model = new CalendarEditViewModel(calendarEvent);
        ViewData.Model = model;
        return View();
    }

    // ActionResult changed to RedirectToRouteResult
    [HttpPost, ValidateModelState]
    public async Task<RedirectToRouteResult> Edit(Guid id, CalendarEventBindingModel binding)
    {
        // removed ModelState.IsValid check
        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (calendarEvent != null)
        {
            CalendarEvent model = calendarService.Update(calendarEvent, binding);
            await context.SaveChangesAsync();
        }
        return this.RedirectToAction<CalendarController>(c => c.Index());
    }

there's no much more happening here. So, if you only use ExportModelState action filter, you will end up with a post action like this :

    [HttpPost, ExportModelStateToTempData]
    public async Task<RedirectToRouteResult> Edit(Guid id, CalendarEventBindingModel binding)
    {
        if (!ModelState.IsValid) return RedirectToAction("Edit", new { id });
        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (calendarEvent != null)
        {
            CalendarEvent model = calendarService.Update(calendarEvent, binding);
            await context.SaveChangesAsync();
        }
        return this.RedirectToAction<CalendarController>(c => c.Index());
    }

Which makes me ask you, why you even bother with ActionFilters in the first place ? While I do like the ValidateModelState pattern, (lots of people doesn't), I don't really see any benefit if you are redirecting in your controller except for one scenario, where you have additional modelstate errors, for completeness let me give you an example:

    [HttpPost, ValidateModelState, ExportModelStateToTempData]
    public async Task<RedirectToRouteResult> Edit(Guid id, CalendarEventBindingModel binding)
    {

        var calendarEvent = await calendarService.FindByIdAsync(id);
        if (!(calendarEvent.DateStart > DateTime.UtcNow.AddDays(7))
            && binding.DateStart != calendarEvent.DateStart)
        {
            ModelState.AddModelError("id", "Sorry, Date start cannot be updated with less than 7 days of event.");
            return RedirectToAction("Edit", new { id });
        }
        if (calendarEvent != null)
        {
            CalendarEvent model = calendarService.Update(calendarEvent, binding);
            await context.SaveChangesAsync();
        }
        return this.RedirectToAction<CalendarController>(c => c.Index());
    }

In the last example, I used both ValidateModelState and ExportModelState, this is because ValidateModelState runs on ActionExecuting so it validates before entering the body of the method, if the binding have some validation error it will redirect automatically. Then I have another check that can't be in the data annotations because it deals with loading an entity and seeing if it has the correct requirements (I know is not the best example, think of it as looking if provided username is available when registration, I know about Remote data annotation but doesn't cover all cases) then I just update the ModelState with my own errors depending on external factors others than the binding. Since ExportModelState runs on ActionExecuted, all my modifications to ModelState are persisted on TempData so I will have them on HttpGet Edit action.

I know all this can confuse some of us, there are no really good indications on how to do MVC on the Controller / PRG side. I was thinking hard in making a blog post to cover all the scenarios and solutions. This is only the 1% of it.

I hope at least I cleared few key points of the POST - GET workflow. If this confuses more than it helps, please let me know. Sorry for the long post.

I wanted to note also that there is ONE subtle difference in the PRG returning an ActionResult that the ones returning a RedirectToRouteResult. If you refresh the page (F5) after having a ValidationError, with the RedirectToRouteResult, the errors will NOT be persisted and you get a clean view as if you entered for the first time. With the ActionResult ones, you refresh and see the exact same page including the errors. This has nothing to do with the ActionResult or RedirectToRouteResult return types, its because in one scenario you redirect always on POST while the other you redirect only on success POST. PRG does not suggest to blinding redirect on unsucessfully POST, yet some people prefer to do redirect on every post, which requires TempData transfer.