Rick Strahl's Weblog  

Wind, waves, code and everything in between...
.NET • C# • Markdown • WPF • All Things Web
Contact   •   Articles   •   Products   •   Support   •   Advertise
Sponsored by:
Markdown Monster - The Markdown Editor for Windows

DropDownList and SelectListItem Array Item Updates in MVC


:P
On this page:

So I ran into an interesting behavior today as I deployed my first MVC 4 app tonight. I have a list form that has a filter drop down that allows selection of categories. This list is static and rarely changes so rather than loading these items from the database each time I load the items once and then cache the actual SelectListItem[] array in a static property.

DropDown

However, when we put the site online tonight we immediately noticed that the drop down list was coming up with pre-set values that randomly changed. Didn't take me long to trace this back to the cached list of SelectListItem[]. Clearly the list was getting updated - apparently through the model binding process in the selection postback.

To clarify the scenario here's the drop down list definition in the Razor View:

@Html.DropDownListFor(mod => mod.QueryParameters.Category, Model.CategoryList, "All Categories")

where Model.CategoryList gets set with:

[HttpPost]
[CompressContent]
public ActionResult List(MessageListViewModel model)
{
    InitializeViewModel(model);

    busEntry entryBus = new busEntry();
    var entries = entryBus.GetEntryList(model.QueryParameters);

    model.Entries = entries;
    model.DisplayMode = ApplicationDisplayModes.Standard;
       
    model.CategoryList = AppUtils.GetCachedCategoryList();
return View(model); }

The AppUtils.GetCachedCategoryList() method gets the cached list or loads the list on the first access. The code to load up the list is housed in a Web utility class. The method looks like this:

/// <summary>
/// Returns a static category list that is cached
/// </summary>
/// <returns></returns>
public static SelectListItem[] GetCachedCategoryList()
{
    if (_CategoryList != null)
        return _CategoryList;

    lock (_SyncLock)
    {
        if (_CategoryList != null)
            return _CategoryList;
        
        var catBus = new busCategory();
        var categories = catBus.GetCategories().ToList();


        // Turn list into a SelectItem list            
        var catList= categories
                         .Select(cat => new SelectListItem() { Text = cat.Name, Value = cat.Id.ToString() })
                         .ToList();

        catList.Insert(0, new SelectListItem()
        {
            Value = ((int)SpecialCategories.AllCategoriesButRealEstate).ToString(),
            Text = "All Categories except Real Estate"
        });
        catList.Insert(1, new SelectListItem()
        {
            Value = "-1",
            Text = "--------------------------------"                    
        });
        
        _CategoryList = catList.ToArray();
    }

    return _CategoryList;
}
private static SelectListItem[] _CategoryList ;

This seemed normal enough to me - I've been doing stuff like this forever caching smallish lists in memory to avoid an extra trip to the database. This list is used in various places throughout the application - for the list display and also when adding new items and setting up for notifications etc..

Watch that ModelBinder!

However, it turns out that this code is clearly causing a problem. It appears that the model binder on the [HttpPost] method is actually updating the list that's bound to and changing the actual entry item in the list and setting its selected value. If you look at the code above I'm not setting the SelectListItem.Selected value anywhere - the only place this value can get set is through ModelBinding. Sure enough when stepping through the code I see that when an item is selected the actual model - model.CategoryList[x].Selected - reflects that.

This is bad on several levels: First it's obviously affecting the application behavior - nobody wants to see their drop down list values jump all over the place randomly. But it's also a problem because the array is getting updated by multiple ASP.NET threads which likely would lead to odd crashes from time to time. Not good!

In retrospect the modelbinding behavior makes perfect sense. The actual items and the Selected property is the ModelBinder's way of keeping track of one or more selected values. So while I assumed the list to be read-only, the ModelBinder is actually updating it on a post back producing the rather surprising results. Totally missed this during testing and is another one of those little - "Did you know?" moments.

So, is there a way around this? Yes but it's maybe not quite obvious. I can't change the behavior of the ModelBinder, but I can certainly change the way that the list is generated. Rather than returning the cached list, I can return a brand new cloned list from the cached items like this:

/// <summary>
/// Returns a static category list that is cached
/// </summary>
/// <returns></returns>
public static SelectListItem[] GetCachedCategoryList()
{
    if (_CategoryList != null)
    {
        // Have to create new instances via projection
        // to avoid ModelBinding updates to affect this
        // globally
        return _CategoryList
                    .Select(cat => new SelectListItem()
                                   {
                                       Value = cat.Value,
                                       Text = cat.Text
                                   })
                    .ToArray();
    }
    …} 

The key is that newly created instances of SelectListItems are returned not just filtered instances of the original list. The key here is 'new instances' so that the ModelBinding updates do not update the actual static instance. The code above uses LINQ and a projection into new SelectListItem instances to create this array of fresh instances. And this code works correctly - no more cross-talk between users.

Unfortunately this code is also less efficient - it has to reselect the items and uses extra memory for the new array. Knowing what I know now I probably would have not cached the list and just take the hit to read from the database. If there is even a possibility of thread clashes I'm very wary of creating code like this. But since the method already exists and handles this load in one place this fix was easy enough to put in.

Live and learn. It's little things like this that can cause some interesting head scratchers sometimes…

Posted in MVC  ASP.NET  .NET  

The Voices of Reason


 

TheSaintr
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

Why don't you use Cache instead of static object..

Doing so you can "clear" that cache item to refresh... with static you have to restart/recycle your site

Rick Strahl
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

@TheSaintr - yes Cache would work, but as I mentioned that list so rarely changes that it really doesn't matter. It wouldn't really change the logistics and I prefer not tying things to the HttpContext pipeline if I can avoid it :-)

Vova
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

Interesting. So, is it MVC4 specific issue? And how ModelBinder would know about the cached list? Form what I can see, when it binds received (form) data to view model there are no items in SelectList, so it can set only SelectedValue property. How it managed to track depndencies between your static (cached) variable and SelectList in view model?

Amit
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

This is a typical problem even if using Cache. If the object from cache is returned as is (without cloning) and for some reason the returned object is modified (in this case by ModelBinder), the original object in the Cache is modified. I have also run into this.

On the side note, I prefer to cache the domain entity (in this case categories) instead of the select list, as then it can be used in other places as well.

Rick Strahl
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

@Vova - No it's an issue in all versions of MVC - just so happens this is the first time I've stumbled into this. The reason the ModelBinder 'knows about' the cached list is because a reference to it is on the model that I'm binding to - I'm passing it out of the GetCachedCategories() function which originally simply returned the static list. My assumption was that the SelectList[] was read only - but the ModelBinder actually updates the list if it's bound. Actually I'm not sure whether this is the binder's doing or the Html.DropDownlist() helper which assigns the value once it knows what the selected value/values from the model/Postback are. Something sets the Selected value of the item selected between the two. It'd be interesting to look into the code for ListBox/DropDownlist helpers.

@Amit - yes I was thinking about caching the domain object originally, but since I only needed a couple of values rather than a full entity and 100% of the use cases involve a drop down list I decided to return the SelectList[] instead.

Larry
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

could you not use the Action Methods Bind attribute to exclude the model.CategoryList property from the model binder, since you're refetching it from cache anyway?

Larry
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

could you not use the POST Action Method's Bind attribute to exclude the model.CategoryList property from the model binder, since you're refetching it from cache anyway?

would that prevent it from updating the CategoryList while still allowing it to set model.QueryParameters.Category?

Rick Strahl
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

@Larry - that might work, but the list is used in number of places so it becomes tedious and unobvious that this is required - I think it's better to fix this at the source rather than special casing in the actual controller methods which is easy to forget.

FWIW, I didn't know that you can actually do that or what the syntax for this looks like - good to know. This might actually make a lot of sense for other scenarios.

Larry
May 16, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

looks like there's also a [ReadOnly(true)] attribute you can put on the model itself, so you have it in one place rather than having to find every action method where it's used and use [Bind]:

http://odetocode.com/blogs/scott/archive/2012/03/11/complete-guide-to-mass-assignment-in-asp-net-mvc.aspx

but I agree, in your case, it's probably best to fix the source

Javier Callico
May 17, 2012

# re: DropDownList and SelectListItem Array Item Updates in MVC

I had a similar issue myself also caused by an static list of items been used across different requests (threads). Thanks for sharing your solution.

West Wind  © Rick Strahl, West Wind Technologies, 2005 - 2024