ModelState reported as invalid for private properties on a model in ASP.NET MVC 2

Question

I'm using ASP.NET MVC 2.0 and I am trying to take advantage of the model binding in my controller and also the modelstate validation. However I have come up against a problem and wanted to share it with people on here to see what you think.

Ok I have my clean User poco in my model class library...

namespace Model
{    
    public partial class User
    {
        public virtual int Id { get; private set; }
        public virtual string UserName { get; private set; }
        public virtual string DisplayName { get; set; }
        public virtual string Email { get; set; }

        public User(string displayName, string userName)
            : this()
        {
            DisplayName = displayName;
            UserName = userName;
        }
    }
}

The design I have gone for only allows certain properties to be edited, after the object has been constructed. The UserName for example can only be set when the object is constructed, to me this makes OO sense, but is the key to my problem, so I wanted to highlight it here.

I then have a 'buddy class' that defines the validation metadata for my User class...

namespace Model
{
[MetadataType(typeof(UserMetadata))]
public partial class User
{
    class UserMetadata
    {
        [Required]
        public virtual int Id { get; set; }

        [Required]
        public virtual string UserName { get; set; }

        [Required]
        public virtual string DisplayName { get; set; }

        [RegularExpression(@"^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,4})$", ErrorMessage = "Invalid address")]
        public virtual string Email { get; set; }
    }
}

}

Then up in my web layer I want to allow my users to be able to edit this object. So I have the following two action methods in my profile controller.

namespace Web.Controllers
{
    public class ProfileController : Controller
    {
        [Authorize]
        public ActionResult Edit()
        {
            var user = _session.Single<User>(x => x.UserName == HttpContext.User.Identity.Name );
            return View(user);
        }

        [HttpPost]
        [ValidateAntiForgeryToken]
        [Authorize]
        [TransactionFilter]
        public ActionResult Edit(User updatedUser)
        {
            // Get the current user to update.
            var user = _session.Single<User>(x => x.UserName == HttpContext.User.Identity.Name);

            if (ModelState.IsValid)
            {
                TryUpdateModel(user);
                // Update store...                
            }
            return View(updatedUser);
        }
    }
}

This has a strongly typed view to go with it...

<%@ Page Title="" Language="C#" MasterPageFile="~/Views/Shared/Site.Master" Inherits="System.Web.Mvc.ViewPage<Model.User>" %>

<asp:Content ID="Content1" ContentPlaceHolderID="TitleContent" runat="server">
    Edit
</asp:Content>

<asp:Content ID="Content2" ContentPlaceHolderID="MainContent" runat="server">
    <%=Html.Script("jquery.validate.js")%>
    <%=Html.Script("MicrosoftMvcJQueryValidation.js")%>
    <%=Html.Script("MvcFoolproofJQueryValidation.js")%>
    <div class="container">
        <div class="column span-14">
        <% using (Html.BeginForm()) {%>
            <%= Html.AntiForgeryToken() %>
            <fieldset>
                <%: Html.DisplayFor(model => model.UserName) %>
                <%= Html.Label("Display Name") %>
                <%= Html.HiddenFor(model => model.DisplayName)%>
                <%= Html.ValidationMessageFor(model => model.DisplayName)%>
                <%= Html.Label("Email address") %>
                <%= Html.EditorFor(model => model.Email)%>
                <%= Html.ValidationMessageFor(model => model.Email)%>
                <%= Html.HiddenFor(model => model.UserName)%>
                <p>
                    <input type="submit" value="Save" />
                </p>
            </fieldset>
        </div>
        <div class="clear"></div>
        <% } %>
    </div>
</asp:Content>

Ok so thats all the code out the way!!

So here's the problem, the view is rendered fine after the initial get request. But when the user posts the form back, say after editing their display name, the ModelState is NOT valid. This is because the UserName property has a private setter on it. However this is by design, for security and semantics I don't ever want them to change their username, so the setter is private. However, as I have added the Required attribute to the property, it is failing as it is not set!

The question is should the modelbinding be reporting this as a validation error or not?! As the property is private, I have designed for it to not be set, therefore by design I don't expect the model binder to be setting it, but I don't want a validation error. I think it should only produce validation errors for properties that it CAN set.

Ok so possible solutions I have come up with so far..

Make the property public.

If I do this I open myself up to allowing the username to be changed for existing user's. I would have to add extra logic somewhere to catch this, not really very nice. I would also have to add a Bind Exclude on the action method to stop any naughty people trying to set it via a post.

Remove the Error

I believe I can remove the error from the ModelState dictionary, this would be fine on this occasion, but I think this will introduce some code smell, as I would have to add this for all my objects that have private setters. I would probably forget!!

Strongly type my view against an interface

I have read that some people bind their view to an interface of their model, this is king of a ModelView interface onto the business model object. I like this idea, but I loose the auto binding and would need to duplicate my model objects with their constructors in my web layer, not sure about that?! Some info on this here http://www.codethinked.com/post/2010/04/12/Easy-And-Safe-Model-Binding-In-ASPNET-MVC.aspx.

Use Model Views

This just doesn't seem DRY to me?! I'm happy to use these if I don't have an existing model object that fits (for example I use a signup Model View).

CustomModelBinder

My preferred option, but I'm not sure I know what I'm doing!! If I could just get the binder to only bind to properties it can set, then I would be laughing!!

What do people think? Comments on the above options, any other solutions, am I just off the mark with my architecture?!

Thanks :)

Answer 1

jfar posted a good link to a post by Brad Wilson where Brad comments...

You can still do partial editing, but you cannot do partial validation any more. So if you exclude binding something with the [Required] attribute, then validation will fail. You have a few choices to work around this:

• Use a view model which exactly mirrors the form data

• Pre-fill in [Required] but unbound fields with data before calling (Try)UpdateModel so that the validation will succeed (even though you don't intend to do anything with that data)

• Allow the validation errors to occur, and then remove them from ModelState after validation is done, since they're inappropriate errors.

My case seems to fit into the 'partial editing' case, where I don't want certain fields to be updated.

I will look into these as solutions.

Answer 2

"I have designed for it to not be set, therefore by design I don't expect the model binder to be setting it, but I don't want a validation error. I think it should only produce validation errors for properties that it CAN set."

Read more about this design decision here:

http://bradwilson.typepad.com/blog/2010/01/input-validation-vs-model-validation-in-aspnet-mvc.html

Interestingly most people complained the complete opposite of what your complaining about about. ;)

Your basically telling the system that something that can't be set should always be set. So I wouldn't say MVC is working incorrectly or anything like that. Your just coding an impossible scenario.

---

Overall your just reaching the pain points of the metadatabuddy technique. Primarily the need to have different validation for new and edit scenarios.

"If I do this I open myself up to allowing the username to be changed for existing user's. I would have to add extra logic somewhere to catch this, not really very nice. I would also have to add a Bind Exclude on the action method to stop any naughty people trying to set it via a post."

IMHO your overeating to these code changes. You'd be adding a simple string to a single method call. Whats the big deal? I'd take the pragmatic approach here.

Answer 3

I'd use a view model because it's the best fit for the job. Don't think of DRY meaning you can't repeat properties on two objects, think of it as "don't duplicate logic or persist identical data in two places". In this case, the semantics of dealing with model binding are not matched to your domain model so you need a way to translate it.