121

Can anyone give me a succinct definition of the role of ModelState in Asp.net MVC (or a link to one). In particular I need to know in what situations it is necessary or desirable to call ModelState.Clear().

Bit open ended huh... sorry, I think it might help if tell you what I'm acutally doing:

I have an Action of Edit on a Controller called "Page". When I first see the form to change the Page's details everything loads up fine (binding to a "MyCmsPage" object). Then I click a button that generates a value for one of the MyCmsPage object's fields (MyCmsPage.SeoTitle). It generates fine and updates the object and I then return the action result with the newly modified page object and expect the relevant textbox (rendered using <%= Html.TextBox("seoTitle", page.SeoTitle)%>) to be updated ... but alas it displays the value from the old model that was loaded.

I've worked around it by using ModelState.Clear() but I need to know why / how it has worked so I'm not just doing it blindly.

PageController:

[AcceptVerbs("POST")]
public ActionResult Edit(MyCmsPage page, string submitButton)
{
    // add the seoTitle to the current page object
    page.GenerateSeoTitle();

    // why must I do this?
    ModelState.Clear();

    // return the modified page object
     return View(page);
 }

Aspx:

<%@ Page Language="C#" MasterPageFile="~/Views/Shared/Site.Master" Inherits="System.Web.Mvc.ViewPage<MyCmsPage>" %>
....
        <div class="c">
            <label for="seoTitle">
                Seo Title</label>
            <%= Html.TextBox("seoTitle", page.SeoTitle)%>
            <input type="submit" value="Generate Seo Title" name="submitButton" />
        </div>
Matt Kocaj
  • 11,278
  • 6
  • 51
  • 79
Mr Grok
  • 3,876
  • 5
  • 31
  • 40
  • Noob AspMVC, if it wanna cache old data, then what's the point in giving model to user again :@ i had same issue, thanks a lot bro – Hassan Faghihi Sep 01 '15 at 05:58

10 Answers10

145

I think is a bug in MVC. I struggled with this issue for hours today.

Given this:

public ViewResult SomeAction(SomeModel model) 
{
    model.SomeString = "some value";
    return View(model); 
}

The view renders with the original model, ignoring the changes. So I thought, maybe it does not like me using the same model, so I tried like this:

public ViewResult SomeAction(SomeModel model) 
{
    var newModel = new SomeModel { SomeString = "some value" };
    return View(newModel); 
}

And still the view renders with the original model. What's odd is, when I put a breakpoint in the view and examine the model, it has the changed value. But the response stream has the old values.

Eventually I discovered the same work around that you did:

public ViewResult SomeAction(SomeModel model) 
{
    var newModel = new SomeModel { SomeString = "some value" };
    ModelState.Clear();
    return View(newModel); 
}

Works as expected.

I don't think this is a "feature," is it?

Jason Coyne
  • 6,509
  • 8
  • 40
  • 70
Tim Scott
  • 15,106
  • 9
  • 65
  • 79
  • 35
    Just did nearly the exact same thing as you. Found out this isn't a bug, though. It's by design: [A Bug? EditorFor and DisplayFor don't display same value](http://forums.asp.net/p/1527149/3687407.aspx) and [ASP.NET MVC’s Html Helpers Render the Wrong Value](http://blogs.msdn.com/b/simonince/archive/2010/05/05/asp-net-mvc-s-html-helpers-render-the-wrong-value.aspx) – Metro Smurf Sep 07 '11 at 20:39
  • I believe that the reason is that the user may have entered incompatible data in the form. The underlying assumption is that if you are returning back to the original form it is because there was a validation failure. If the user entered the value 'Bob' in an int field, you want the value of 'Bob' to still be there so you can tell them that 'Bob' isn't an int. Something like that. – wcm Dec 29 '11 at 22:07
  • 8
    Man, I've already spent 2 hours fighting with it. Thanks for posting this answer! – Andrey Agibalov Mar 31 '12 at 13:52
  • 40
    this is still true and a lot of people, including me, are losing a lot of time because of this. bug or by design, i don't care, it's "unexpected". – Proviste Oct 02 '12 at 12:14
  • 8
    I agree with @Proviste, i hope this "feature" is removed in the future – Ben Jan 15 '13 at 13:16
  • 9
    I just spent four hours on this. Ugly. – Brian MacKay Apr 10 '13 at 21:52
  • 6
    I've lost at least two days of development time because of this nasty behavior. LET IT BE KNOWN: If you are serializing your form in javascript and submitting the content to your controller so that the controller can the subsequently modify the model AND RETURN THE RESULT, the result won't be what you expect. ENSURE YOU ISSUE ModelState.Clear() in the controller's action method. – Aaron Hudon Jun 06 '14 at 16:48
  • 2
    Damn! spent so much time for the same issue faced by ajhuddy – suresh2 Nov 17 '14 at 16:49
  • I am thinking that it is a feature albeit an obscure one. Have you guys noticed in forms that when you post back to the server and say a date field is invalid that the text box still has the invalid value along with a validation error telling the user to correct it. That happens because of this feature. So your code should be: if(ModelState.IsValid) // do extra serverside validation and save else ModelState.Clear() // all changes where comitted to the model so no need for the posted values. – John May 21 '15 at 06:58
  • ASP.NET MVC, you're a pretty evil – Zignd Dec 01 '15 at 12:34
  • 4
    This issue exists in ASP.NET MVC 5.2 – Evgeny Gorb Jan 25 '16 at 16:22
  • New issue inserted for new MVC 6 here too: https://github.com/aspnet/Mvc/issues/4486 – Miroslav Siska Apr 15 '16 at 17:23
  • 1
    Oct 2016 and this silly bug is still around... Soon it will be old enough to have a beer! – Jose Parra Oct 12 '16 at 02:22
  • For anyone wanting to clear a text box for entering a new value in multiple times - see @cottsak answer below – niico Oct 15 '16 at 16:31
  • this. is. not. a. bug. [see below](https://stackoverflow.com/questions/1775170/asp-net-mvc-modelstate-clear/1775185#1775185) – Matt Kocaj Dec 27 '16 at 04:45
  • Is it neccesary to create a new instance of the object? – Sana Ahmed Jan 28 '19 at 04:50
  • It's a bug by design. – BVernon May 28 '21 at 18:22
54

Update:

  • This is not a bug.
  • Please stop returning View() from a POST action. Use PRG instead and redirect to a GET if the action is a success.
  • If you are returning a View() from a POST action, do it for form validation, and do it the way MVC is designed using the built in helpers. If you do it this way then you shouldn't need to use .Clear()
  • If you're using this action to return ajax for a SPA, use a web api controller and forget about ModelState since you shouldn't be using it anyway.

Old answer:

ModelState in MVC is used primarily to describe the state of a model object largely with relation to whether that object is valid or not. This tutorial should explain a lot.

Generally you should not need to clear the ModelState as it is maintained by the MVC engine for you. Clearing it manually might cause undesired results when trying to adhere to MVC validation best practises.

It seems that you are trying to set a default value for the title. This should be done when the model object is instantiated (domain layer somewhere or in the object itself - parameterless ctor), on the get action such that it goes down to the page the 1st time or completely on the client (via ajax or something) so that it appears as if the user entered it and it comes back with the posted forms collection. Some how your approach of adding this value on the receiving of a forms collection (in the POST action // Edit) is causing this bizarre behaviour that might result in a .Clear() appearing to work for you. Trust me - you don't want to be using the clear. Try one of the other ideas.

Matt Kocaj
  • 11,278
  • 6
  • 51
  • 79
  • 1
    Does help me to rethink my services layer a bit (groan but thx) but as with a lot of stuff on the net it leans heavily towards the viewpoint of using ModelState for validation. – Mr Grok Nov 21 '09 at 11:17
  • Added more information to the question to show why I'm particularly interested in ModelState.Clear() and the reason for my query – Mr Grok Nov 21 '09 at 11:41
  • 10
    I don't really buy this argument to *stop* returning View(...) from an [HttpPost] function. If you are POSTing content via ajax and then updating the document with the resulting PartialView, the MVC ModelState has been shown to be incorrect. The only workaround I have found is to Clear it in the controller method. – Aaron Hudon Oct 20 '16 at 16:31
  • @AaronHudon PRG is pretty well established. – Matt Kocaj Oct 25 '16 at 04:11
  • If I POST with an AJAX call can I redirect to a GET action and return a model filled view like the OP wants to, all asynchronously? – MyiEye Aug 03 '18 at 16:40
  • @MyiEye not without doing more work in the client: an AJAX POST will be captured on the client and wont implicitly cause the whole page to reload. You could capture the response which might include the new `location` to redirect to, and then on the `.success()` handler of the AJAX call, redirect the whole page to the `location` causing a GET. – Matt Kocaj Aug 10 '18 at 01:14
  • Im returning a view from a post so I can update drop down lists. Im posting using AJAX. If there is a better way to do this let me know. – Darth Scitus Mar 11 '19 at 17:14
  • @CameronBelt is it a partial view with a block of HTML which includes the updated select elements? – Matt Kocaj Mar 15 '19 at 06:58
  • 1
    I might have to consider the 'stop returning View() from a POST'. Thx. – Kai Hartmann Apr 08 '22 at 08:29
  • 1
    MVC is overly complicated. If this many people, myself included, come here wondering why their View is showing wrong data after a POST, there's something wrong. I would argue if we aren't supposed to return a View from a POST, it should be prevented. Fortunately we have better options than MVC now. – Mmm Nov 30 '22 at 19:00
18

If you want to clear a value for an individual field then I found the following technique useful.

ModelState.SetModelValue("Key", new ValueProviderResult(null, string.Empty, CultureInfo.InvariantCulture));

Note: Change "Key" to the name of the field that you want to reset.

Carl Saunders
  • 181
  • 1
  • 2
  • I don't know why this worked differently for me (MVC4 perhaps)? But I had to also do model.Key = "" afterwards. Both lines are required. – TTT Mar 21 '13 at 16:18
  • 1
    I'd like to compliment you on the remove comment @PeterGluck. It's better than clearing the complete modelstate (since I've got errors on some fields that I'd like to keep). – Tjab Apr 20 '16 at 12:34
7

Well lots of us seem to have been bitten by this, and although the reason this happens makes sense I needed a way to ensure that the value on my Model was shown, and not ModelState.

Some have suggested ModelState.Remove(string key), but it's not obvious what key should be, especially for nested models. Here are a couple methods I came up with to assist with this.

The RemoveStateFor method will take a ModelStateDictionary, a Model, and an expression for the desired property, and remove it. HiddenForModel can be used in your View to create a hidden input field using only the value from the Model, by first removing its ModelState entry. (This could easily be expanded for the other helper extension methods).

/// <summary>
/// Returns a hidden input field for the specified property. The corresponding value will first be removed from
/// the ModelState to ensure that the current Model value is shown.
/// </summary>
public static MvcHtmlString HiddenForModel<TModel, TProperty>(this HtmlHelper<TModel> helper,
    Expression<Func<TModel, TProperty>> expression)
{
    RemoveStateFor(helper.ViewData.ModelState, helper.ViewData.Model, expression);
    return helper.HiddenFor(expression);
}

/// <summary>
/// Removes the ModelState entry corresponding to the specified property on the model. Call this when changing
/// Model values on the server after a postback, to prevent ModelState entries from taking precedence.
/// </summary>
public static void RemoveStateFor<TModel, TProperty>(this ModelStateDictionary modelState, TModel model,
    Expression<Func<TModel, TProperty>> expression)
{
    var key = ExpressionHelper.GetExpressionText(expression);

    modelState.Remove(key);
}

Call from a controller like this:

ModelState.RemoveStateFor(model, m => m.MySubProperty.MySubValue);

or from a view like this:

@Html.HiddenForModel(m => m.MySubProperty.MySubValue)

It uses System.Web.Mvc.ExpressionHelper to get the name of the ModelState property.

Tobias J
  • 19,813
  • 8
  • 81
  • 66
6

Well the ModelState basically holds the current State of the model in terms of validation, it holds

ModelErrorCollection: Represent the errors when the model try to bind the values. ex.

TryUpdateModel();
UpdateModel();

or like a parameter in the ActionResult

public ActionResult Create(Person person)

ValueProviderResult: Hold the details about the attempted bind to the model. ex. AttemptedValue, Culture, RawValue.

Clear() method must be use with caution because it can lead to unspected results. And you will lose some nice properties of the ModelState like AttemptedValue, this is used by MVC in the background to repopulate the form values in case of error.

ModelState["a"].Value.AttemptedValue
JOBG
  • 4,544
  • 4
  • 26
  • 47
  • 1
    hmmm... That might be where I'm getting the issue by the looks of it. I've inspected the value of the Model.SeoTitle property and it has changed but the attempted value has not. Looks as if it's sticking the value in as if there is an error on the page even though there isn't one (have checked the ModelState Dictionary and there are no errors). – Mr Grok Nov 22 '09 at 19:45
6

I had an instance where I wanted to update the model of a sumitted form, and did not want to 'Redirect To Action' for performanace reason. Previous values of hidden fields were being retained on my updated model - causing allsorts of issues!.

A few lines of code soon identified the elements within ModelState that I wanted to remove (after validation), so the new values were used in the form:-

while (ModelState.FirstOrDefault(ms => ms.Key.ToString().StartsWith("SearchResult")).Value != null)
{
    ModelState.Remove(ModelState.FirstOrDefault(ms => ms.Key.ToString().StartsWith("SearchResult")));
}
MaseBase
  • 800
  • 3
  • 8
  • 31
stevieg
  • 61
  • 1
  • 1
4

I wanted to update or reset a value if it didn't quite validate, and ran into this problem.

The easy answer, ModelState.Remove, is.. problematic.. because if you are using helpers you don't really know the name (unless you stick by the naming convention). Unless perhaps you create a function that both your custom helper and your controller can use to get a name.

This feature should have been implemented as an option on the helper, where by default is does not do this, but if you wanted the unaccepted input to redisplay you could just say so.

But at least I understand the issue now ;).

Gerard ONeill
  • 3,914
  • 39
  • 25
  • I needed to do exactly this; see my methods I posted below which helped me `Remove()` the correct key. – Tobias J May 18 '15 at 20:54
0

Got it in the end. My Custom ModelBinder which was not being registered and does this :

var mymsPage = new MyCmsPage();

NameValueCollection frm = controllerContext.HttpContext.Request.Form;

myCmsPage.SeoTitle = (!String.IsNullOrEmpty(frm["seoTitle"])) ? frm["seoTitle"] : null;

So something that the default model binding was doing must have been causing the problem. Not sure what, but my problem is at least fixed now that my custom model binder is being registered.

Mr Grok
  • 3,876
  • 5
  • 31
  • 40
  • Well i have no experience with a custom ModelBinder, the default one fits my needs so far =). – JOBG Nov 23 '09 at 03:25
0

Generally, when you find yourself fighting against a framework standard practices, it is time to reconsider your approach. In this case, the behavior of ModelState. For instance, when you don't want model state after a POST, consider a redirect to the get.

[HttpPost]
public ActionResult Edit(MyCmsPage page, string submitButton)
{
    if (ModelState.IsValid) {
        SomeRepository.SaveChanges(page);
        return RedirectToAction("GenerateSeoTitle",new { page.Id });
    }
    return View(page);
}

public ActionResult GenerateSeoTitle(int id) {
     var page = SomeRepository.Find(id);
     page.GenerateSeoTitle();
     return View("Edit",page);
}

EDITED to answer culture comment:

Here is what I use to handle a multi-cultural MVC application. First the route handler subclasses:

public class SingleCultureMvcRouteHandler : MvcRouteHandler {
    protected override IHttpHandler GetHttpHandler(RequestContext requestContext)
    {
        var culture = requestContext.RouteData.Values["culture"].ToString();
        if (string.IsNullOrWhiteSpace(culture))
        {
            culture = "en";
        }
        var ci = new CultureInfo(culture);
        Thread.CurrentThread.CurrentUICulture = ci;
        Thread.CurrentThread.CurrentCulture = CultureInfo.CreateSpecificCulture(ci.Name);
        return base.GetHttpHandler(requestContext);
    }
}

public class MultiCultureMvcRouteHandler : MvcRouteHandler
{
    protected override IHttpHandler GetHttpHandler(RequestContext requestContext)
    {
        var culture = requestContext.RouteData.Values["culture"].ToString();
        if (string.IsNullOrWhiteSpace(culture))
        {
            culture = "en";
        }
        var ci = new CultureInfo(culture);
        Thread.CurrentThread.CurrentUICulture = ci;
        Thread.CurrentThread.CurrentCulture = CultureInfo.CreateSpecificCulture(ci.Name);
        return base.GetHttpHandler(requestContext);
    }
}

public class CultureConstraint : IRouteConstraint
{
    private string[] _values;
    public CultureConstraint(params string[] values)
    {
        this._values = values;
    }

    public bool Match(HttpContextBase httpContext,Route route,string parameterName,
                        RouteValueDictionary values, RouteDirection routeDirection)
    {

        // Get the value called "parameterName" from the 
        // RouteValueDictionary called "value"
        string value = values[parameterName].ToString();
        // Return true is the list of allowed values contains 
        // this value.
        return _values.Contains(value);

    }

}

public enum Culture
{
    es = 2,
    en = 1
}

And here is how I wire up the routes. After creating the routes, I prepend my subagent (example.com/subagent1, example.com/subagent2, etc) then the culture code. If all you need is the culture, simply remove the subagent from the route handlers and routes.

    public static void RegisterRoutes(RouteCollection routes)
    {

        routes.IgnoreRoute("{resource}.axd/{*pathInfo}");
        routes.IgnoreRoute("Content/{*pathInfo}");
        routes.IgnoreRoute("Cache/{*pathInfo}");
        routes.IgnoreRoute("Scripts/{pathInfo}.js");
        routes.IgnoreRoute("favicon.ico");
        routes.IgnoreRoute("apple-touch-icon.png");
        routes.IgnoreRoute("apple-touch-icon-precomposed.png");

        /* Dynamically generated robots.txt */
        routes.MapRoute(
            "Robots.txt", "robots.txt",
            new { controller = "Robots", action = "Index", id = UrlParameter.Optional }
        );

        routes.MapRoute(
             "Sitemap", // Route name
             "{subagent}/sitemap.xml", // URL with parameters
             new { subagent = "aq", controller = "Default", action = "Sitemap"},  new[] { "aq3.Controllers" } // Parameter defaults
        );

        routes.MapRoute(
             "Rss Feed", // Route name
             "{subagent}/rss", // URL with parameters
             new { subagent = "aq", controller = "Default", action = "RSS"},  new[] { "aq3.Controllers" } // Parameter defaults
        );

        /* remap wordpress tags to mvc blog posts */
        routes.MapRoute(
            "Tag", "tag/{title}",
            new { subagent = "aq", controller = "Default", action = "ThreeOhOne", id = UrlParameter.Optional},  new[] { "aq3.Controllers" }
        ).RouteHandler = new MultiCultureMvcRouteHandler(); ;

        routes.MapRoute(
            "Custom Errors", "Error/{*errorType}",
            new { controller = "Error", action = "Index", id = UrlParameter.Optional},  new[] { "aq3.Controllers" }
        );

        /* dynamic images not loaded from content folder */
        routes.MapRoute(
            "Stock Images",
            "{subagent}/Images/{*filename}",
            new { subagent = "aq", controller = "Image", action = "Show", id = UrlParameter.Optional, culture = "en"},  new[] { "aq3.Controllers" }
        );

        /* localized routes follow */
        routes.MapRoute(
            "Localized Images",
            "Images/{*filename}",
            new { subagent = "aq", controller = "Image", action = "Show", id = UrlParameter.Optional},  new[] { "aq3.Controllers" }
        ).RouteHandler = new MultiCultureMvcRouteHandler();

        routes.MapRoute(
            "Blog Posts",
            "Blog/{*postname}",
            new { subagent = "aq", controller = "Blog", action = "Index", id = UrlParameter.Optional},  new[] { "aq3.Controllers" }
        ).RouteHandler = new MultiCultureMvcRouteHandler();

        routes.MapRoute(
            "Office Posts",
            "Office/{*address}",
            new { subagent = "aq", controller = "Offices", action = "Address", id = UrlParameter.Optional }, new[] { "aq3.Controllers" }
        ).RouteHandler = new MultiCultureMvcRouteHandler();

        routes.MapRoute(
             "Default", // Route name
             "{controller}/{action}/{id}", // URL with parameters
             new { subagent = "aq", controller = "Home", action = "Index", id = UrlParameter.Optional }, new[] { "aq3.Controllers" } // Parameter defaults
        ).RouteHandler = new MultiCultureMvcRouteHandler();

        foreach (System.Web.Routing.Route r in routes)
        {
            if (r.RouteHandler is MultiCultureMvcRouteHandler)
            {
                r.Url = "{subagent}/{culture}/" + r.Url;
                //Adding default culture 
                if (r.Defaults == null)
                {
                    r.Defaults = new RouteValueDictionary();
                }
                r.Defaults.Add("culture", Culture.en.ToString());

                //Adding constraint for culture param
                if (r.Constraints == null)
                {
                    r.Constraints = new RouteValueDictionary();
                }
                r.Constraints.Add("culture", new CultureConstraint(Culture.en.ToString(), Culture.es.ToString()));
            }
        }

    }
B2K
  • 2,541
  • 1
  • 22
  • 34
  • You are very right suggesting the POST REDIRECT practice, in fact i do this for almost every post action. However i had a very particular need: i have a filter form on top of the page, initially it submitted with get. But i ran into a problem with a date field not being bound and then discovered that GET requests don't carry the culture around (i use french for my app), so i had to switch the request to POST to successfully bind my date. Then came this problem, i'm a little stuck her.. – Souhaieb Besbes Jan 13 '16 at 09:03
  • @SouhaiebBesbes See my updates showing how I handle culture. – B2K Jan 13 '16 at 17:35
  • @SouhaiebBesbes perhaps a bit simpler would be to store your culture in TempData. See http://stackoverflow.com/questions/12422930/using-tempdata-in-asp-net-mvc-best-practice – B2K Jan 13 '16 at 17:40
  • I fail to see how this example of how to make MVC work is progress over WebForms. Sorry MS, you blew it. – Mmm Nov 30 '22 at 19:04
  • What does this have to do with WebForms? – B2K Dec 06 '22 at 22:19
0

Well, this seemed to work on my Razor Page and never even did a round trip to the .cs file. This is old html way. It might be useful.

<input type="reset" value="Reset">
JustJohn
  • 1,362
  • 2
  • 22
  • 44