ASP.NET MVC Beta 1: DefaultModelBinder incorrectly saves parameter and check state between unrelated requests

Question

When I use the default model binding to bind the form parameters to a complex object, which is the parameter for the action, the structure remembers the values ​​passed to the first request, which means that any subsequent request to this action receives the same data as the first. Parameter values ​​and validation status are stored between unrelated web requests.

Here is my controller code ( service represents access to the back of the application):

  [AcceptVerbs(HttpVerbs.Get)] public ActionResult Create() { return View(RunTime.Default); } [AcceptVerbs(HttpVerbs.Post)] public ActionResult Create(RunTime newRunTime) { if (ModelState.IsValid) { service.CreateNewRun(newRunTime); TempData["Message"] = "New run created"; return RedirectToAction("index"); } return View(newRunTime); } 

My.aspx view (strongly typed as ViewPage<RunTime >) contains directives like:

 <%= Html.TextBox("newRunTime.Time", ViewData.Model.Time) %> 

This uses the DefaultModelBinder class, which is designed to automatically save the properties of my model .

I hit the page, enter valid data (e.g. time = 1). The application correctly saves the new object with time = 1. Then I hit it again, enter different valid data (for example, time = 2). However, the data that is saved is original (for example, time = 1). This also affects the validation, so if my original data was invalid, all the data that I entered in the future are considered invalid. Restarting IIS or restoring my code resets the saved state.

I can fix the problem by writing my own hard-coded middleware, the basic naive example of which is shown below.

  [AcceptVerbs(HttpVerbs.Post)] public ActionResult Create([ModelBinder(typeof (RunTimeBinder))] RunTime newRunTime) { if (ModelState.IsValid) { service.CreateNewRun(newRunTime); TempData["Message"] = "New run created"; return RedirectToAction("index"); } return View(newRunTime); } internal class RunTimeBinder : DefaultModelBinder { public override ModelBinderResult BindModel(ModelBindingContext bindingContext) { // Without this line, failed validation state persists between requests bindingContext.ModelState.Clear(); double time = 0; try { time = Convert.ToDouble(bindingContext.HttpContext.Request[bindingContext.ModelName + ".Time"]); } catch (FormatException) { bindingContext.ModelState.AddModelError(bindingContext.ModelName + ".Time", bindingContext.HttpContext.Request[bindingContext.ModelName + ".Time"] + "is not a valid number"); } var model = new RunTime(time); return new ModelBinderResult(model); } } 

Am I missing something? I do not think this is a browser session problem, since I can reproduce the problem if the first data is entered in one browser and the second in another.

Answer 1

Turns out the problem was that my controllers were reused between calls. One of the details that I selected from my initial post is that I use the Castle.Windsor container to create my controllers. I could not mark my controller with the Transient style, so I got the same instance for every request. Thus, the context used by the binder was reused and, of course, contained outdated data.

I found the problem by carefully analyzing the difference between Eilon code and mine, eliminating all other possibilities. As stated in the castle documentation , this is a “terrible mistake”! Let it be a warning to others!

Thanks for your reply. Eilon - sorry for your time.

Answer 2

I tried to reproduce this problem, but I do not see the same behavior. I created almost the same controller and views that you have (with some assumptions), and every time I created a new "RunTime", I added its value to TempData and sent it via Redirect. Then, on the landing page, I grabbed the value, and was always the value I entered in this query - never a deprecated value.

Here is my controller:

public class HomeController: Controller {public ActionResult Index () {ViewData ["Title"] = "Home"; string message = "Welcome:" + TempData ["Message"]; if (TempData.ContainsKey ("value")) {int theValue = (int) TempData ["value"]; message + = "+ theValue.ToString ();} ViewData [" Message "] = message; return View ();}

 [AcceptVerbs(HttpVerbs.Get)] public ActionResult Create() { return View(RunTime.Default); } [AcceptVerbs(HttpVerbs.Post)] public ActionResult Create(RunTime newRunTime) { if (ModelState.IsValid) { //service.CreateNewRun(newRunTime); TempData["Message"] = "New run created"; TempData["value"] = newRunTime.TheValue; return RedirectToAction("index"); } return View(newRunTime); } 

}

And here is my View (Create.aspx):

 <% using (Html.BeginForm()) { %> <%= Html.TextBox("newRunTime.TheValue", ViewData.Model.TheValue) %> <input type="submit" value="Save" /> <% } %> 

Also, I was not sure what the "RunTime" type looked like, so I did this:

  public class RunTime { public static readonly RunTime Default = new RunTime(-1); public RunTime() { } public RunTime(int theValue) { TheValue = theValue; } public int TheValue { get; set; } } 

Is it possible that your RunTime implementation includes some static values ​​or something else?

Thanks,

Eilon

Answer 3

I'm not sure if this is connected or not, but your call <% = Html.TextBox ("newRunTime.Time", ViewData.Model.Time)%> may actually choose the wrong overload (since Time is an integer, it will choose overload object htmlAttributes , not string value .

Checking the displayed HTML will let you know if this is happening. changing int to ViewData.Model.Time.ToString() will result in the correct overload.

It looks like your problem is something else, but I noticed it and was burned in the past.

Answer 4

Seb, I'm not sure what you mean by example. I don't know anything about Unity configuration. I will explain the situation with Castle.Windsor and maybe this will help you configure Unity correctly.

By default, Castle.Windsor returns the same object each time you request this type. This is a singleton lifestyle. There is a good explanation of the different lifestyle options in Bookmark. Bookmark .

In ASP.NET MVC, each instance of a controller class is bound to the context of the web request that it was created for maintenance. Therefore, if your IoC container returns the same instance of your controller class each time, you will always get the controller associated with the context of the first web request that this controller class used. In particular, ModelState and other objects used by DefaultModelBinder will be reused, so your related model object and validation messages in ModelState will become obsolete.

So you need your IoC to return a new instance every time MVC requests an instance of your controller class.

At Castle.Windsor, this is called a temporary lifestyle. To configure it, you have two options:

• XML configuration: you add lifestlye = "transitional" to each element of your configuration file that the controller represents.
• Configuration in code: you can tell the container to use a transient lifestyle during controller registration. This is what the MvcContrib helper Ben mentioned about does for you automatically — look at the RegisterControllers method in the MvcContrib source code .

I would suggest that Unity offers a similar lifestyle concept in Castle.Windsor, so you need to configure Unity to use its transitional equivalent for your controllers. MvcContrib seems to have Unity support - maybe you could look there.

Hope this helps.

Answer 5

If you encounter similar problems when trying to use the Wind Ior container in an ASP.NET MVC application, I had to go through the same opening flight to make it work. Here are some details that might help someone else.

Using this initial setup in Global.asax:

  if (_container == null) { _container = new WindsorContainer("config/castle.config"); ControllerBuilder.Current.SetControllerFactory(new WindsorControllerFactory(Container)); } 

And using WindsorControllerFactory, which when requesting a controller instance:

  return (IController)_container.Resolve(controllerType); 

While Windsor correctly connected all the controllers, for some reason the parameters were not transferred from the form to the corresponding controller action. Instead, they were all null, although they caused the correct action.

By default, single packets are sent back to the container, which is obviously bad for the controllers and the cause of the problem:

http://www.castleproject.org/monorail/documentation/trunk/integration/windsor.html

However, the documentation states that the lifestyle of the controllers can be changed to a transient, although it does not really tell you how to do this if you use a configuration file. It turns out quite easily:

 <component id="home.controller" type="DoYourStuff.Controllers.HomeController, DoYourStuff" lifestyle="transient" /> 

And without any code changes, it should work as expected (i.e. unique controllers each time provided by one container instance). You can then complete the entire IoC configuration in the configuration file, rather than the code, like a good boy / girl I know.