How many times have you seen or written code like this:
public ActionResult Index(SomeModel model) {
if (ModelState.IsValid)
{
return View();
}
// do something
return RedirectToAction("index");
}
What’s the acceptable amount of times to repeat the same piece of code before we decide to encapsulate the behaviour into something reusable?
Considerations
A while ago I posted a solution to this using a custom view result. The thing I don’t like about my original approach is that it means I need to write my “Success” code inside a delegate. It’s only a few extra brackets and curly braces but I still don’t like it.
As far as I’m concerned, a controller action should not be executed if ModelState is invalid.
How is this type of request validation any different to decorating your action with a [HttpGet] attribute? It’s not. Therefore, an Action Filter is the way to go.
It actually only took 5 minutes or so to knock up a prototype:
public class ValidateModelStateAttribute : ActionFilterAttribute
{
public override void OnActionExecuting(ActionExecutingContext filterContext)
{
var viewData = filterContext.Controller.ViewData;
if (!viewData.ModelState.IsValid)
{
filterContext.Result = new ViewResult
{
ViewData = viewData,
TempData = filterContext.Controller.TempData
};
}
base.OnActionExecuting(filterContext);
}
}
This works fine, most of the time.
Rebuilding View Models
The above ActionFilter does more or less the same job as the code at the beginning of this article. The problem is that ModelState only contains the properties of the Model that were POSTed to the server and handled by the MVC Model Binder. Even in simple ASP.NET MVC applications your GET “View” is likely to be more complex than your POST “command” - perhaps you have a few SelectList properties needed to build your form.
So how to we solve this problem - we need to “rebuild” our view model. Looking on the internet, this is what we should do right?
public ActionResult Index(SomeModel model) {
if (ModelState.IsValid)
{
// gotta rebuild the model
RebuildModel(model);
return View(model);
}
// do something
return RedirectToAction("index");
}
private void RebuildModel(SomeModel model) {
model.Countries = new SelectList(new[] { "England", "France" });
}
Great, now if ModelState is invalid, we can rebuild the model and then pass it back to the View.
But wait, we’re doing ModelState checks in the controller action again!
There is a simple solution to this, but you need to make a compromise.
I made various attempts to rebuild the view model inside OnActionExecuting. The first issue is that ViewData.Model happens to be null at this stage. We can grab it from ControllerBase.ActionParameters but we’re presented with issue two, it’s stored as an instance of System.Object. Locating the appropriate “model builder” is possible but requires a fair bit of reflection, and to be honest, is totally unecessary.
Solution
The solution (about time, you say) is to accept that a GET is different to a POST and embrace PRG (Post, Redirect, Get).
In most cases, our GET actions are responsible for returning a View, usually represented as HTML but could easily be JSON or XML.
When we make a POST, are we posting a View? No, we’re issuing a command. A command is often more simple in its structure than a View. For example, it probably doesn’t contain any SelectList properties - after all, these are just pieces of metadata that enable you to build the interface that allows the user to issue a command.
So what we’re actually saying here is that a POST action should not return a View as doing so would mean it has to know how to build the view, which is not its responsbility.
Embracing PRG, when a client POSTs to the server, we should issue a Redirect which in turn makes a GET request. You’re probably already doing this when your POST is successful, but we’ve been encourage NOT to do it when ModelState validation fails.
One possible reason for this is that ModelState is not persisted between requests. It can be, but you need to copy it somewhere more permanent. TempData is a great place for this and since I’m not the first person to be having this moment of clarity, there is already code available for doing it (in MvcContrib and by Kazi Manzur Rashid).
By using these attributes we no longer need to worry about rebuilding the view model in our POST action. All we do is issue a redirect back to the GET action and our model state will be preserved.
The ValidateModelStateAttribute from my Fabrik.Common project does just this.
Here’s an example using the attribute:
public class HomeController : Controller
{
[ImportModelStateFromTempData]
public ActionResult Index()
{
var homeView = new HomeViewFactory().CreateView();
return View(homeView);
}
[HttpPost]
[ValidateModelState]
public ActionResult Index(HomeCommand command)
{
// if we get here, ModelState is valid
// save to db etc.
return RedirectToAction("index");
}
}
public class HomeView : HomeCommand
{
public string Message { get; set; }
public SelectList SubscriptionTypes { get; set; }
}
public class HomeCommand : IValidatableObject
{
[Required]
public string Name { get; set; }
[Required]
[Range(18, int.MaxValue, ErrorMessage = "You must be over 18 or over to subscribe")]
public int Age { get; set; }
public string SubscriptionType { get; set; }
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (Age < 60 && SubscriptionType == "Gold")
yield return new ValidationResult(
"Sorry, Gold Subscription are only available if you are over 60.",
new[] { "SubscriptionType" });
}
}
Note that we have the [ValidateModelState] attribute on the POST action and an [ImportModelStateFromTempData] attribute on the GET action. As the same suggests, this is instructing the server to populate ModelState from TempData (if it exists).
What I particularly like about this is that I can now POST a different (simplified) type of object since I don’t need to worry about rebuilding the View in my POST action. This is great if you’re using some kind of message bus as it keeps your messages small and easily serializable.
What about AJAX requests?
The ValidateModelState filter detects if a request is an AJAX request and returns any ModelState errors as JSON instead, which you can then process on the client:
$(function () {
// hijax the form
$("form").submit(function (e) {
var $form = $(this);
var validator = $form.data("validator");
if (!validator || !$form.valid())
return;
e.preventDefault();
$.ajax({
url: "@Url.Action("index")",
type: "POST",
data: $form.serialize(),
statusCode: {
400: function(xhr, status, err) {
var errors = $.parseJSON(err);
validator.showErrors(errors);
}
},
success: function() {
// just reload the page for now
location.reload(true);
}
});
});
});
This makes is really easy to AJAX-enable your existing ASP.NET MVC forms.
The server will return a 400 status code (Bad Request) if ModelState is invalid so we need to catch this. You can do this like the example above, or just provide an “error” function. In the above example we parse the response and pass it to the jQuery validate plugin to display the errors.
So now you have no excuse for writing that dreaded ModelState check. Download the full source on GitHub.