Custom model binding, model state, and data annotations

Question

I have a few questions regarding custom model binding, model state, and data annotations.

1) Is it redundant to do validation in the custom model binder if I have data annotations on my model, because that's what I thought the point of data annotations were.

2) Why is my controller treating the model state as valid even when it's not, mainly I make the Name property null or too short.

3) Is it ok to think of custom model binders as constructor methods, because that's what they remind me of.

First here is my model.

public class Projects
{     
    [Key]
    [Required]
    public Guid ProjectGuid { get; set; }

    [Required]
    public string AccountName { get; set; }

    [Required(ErrorMessage = "Project name required")]
    [StringLength(128, ErrorMessage = "Project name cannot exceed 128 characters")]
    [MinLength(3, ErrorMessage = "Project name must be at least 3 characters")]
    public string Name { get; set; }

    [Required]
    public long TotalTime { get; set; }
}

Then I'm using a custom model binder to bind some properties of the model. Please don't mind that it's quick and dirty just trying to get it functioning and then refactoring it.

public class ProjectModelBinder : IModelBinder
{
     public object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        if (controllerContext == null)
        {
            throw new ArgumentNullException("controllerContext");
        }
        if (bindingContext == null)
        {
            throw new ArgumentNullException("bindingContext");
        }
        var p = new Project();
        p.ProjectGuid = System.Guid.NewGuid();
        p.AccountName = controllerContext.HttpContext.User.Identity.Name;
        p.Name = controllerContext.HttpContext.Request.Form.Get("Name");
        p.TotalTime = 0;

        //
        // Is this redundant because of the data annotations?!?!
        //
        if (p.AccountName == null)
            bindingContext.ModelState.AddModelError("Name", "Name is required");

        if (p.AccountName.Length < 3)
            bindingContext.ModelState.AddModelError("Name", "Minimum length is 3 characters");

        if (p.AccountName.Length > 128)
            bindingContext.ModelState.AddModelError("Name", "Maximum length is 128 characters");

        return p;
    }
}

Now my controller action.

    [HttpPost]
    public ActionResult CreateProject([ModelBinder(typeof(ProjectModelBinder))]Project project)
    {
        //
        // For some reason the model state comes back as valid even when I force an error
        //
        if (!ModelState.IsValid)
            return Content(Boolean.FalseString);

        //_projectRepository.CreateProject(project);

        return Content(Boolean.TrueString);

    }

EDIT

I Found some code on another stackoverflow question but I'm not sure at which point I would inject the following values into this possible solution.

What I want to inject when a new object is created:

        var p = new Project();
        p.ProjectGuid = System.Guid.NewGuid();
        p.AccountName = controllerContext.HttpContext.User.Identity.Name;
        p.Name = controllerContext.HttpContext.Request.Form.Get("Name");
        p.TotalTime = 0;

How do I get the above code into what's below (Possible solution):

    public class ProjectModelBinder : DefaultModelBinder
    {
        public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
        {
            if (bindingContext.ModelType == typeof(Project))
            {
                ModelBindingContext newBindingContext = new ModelBindingContext()
                {

                    ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(
                        () => new Project(), // construct a Project object,
                        typeof(Project)         // using the Project metadata
                    ),
                    ModelState = bindingContext.ModelState,
                    ValueProvider = bindingContext.ValueProvider

                };

                // call the default model binder this new binding context
                return base.BindModel(controllerContext, newBindingContext);
            }
            else
            {
                return base.BindModel(controllerContext, bindingContext);
            }
        }

    }

}

Sigmatism

Answer 1

You will find things work much easier if you inherit from the DefaultModelBinder, override the BindModel method, call the base.BindModel method and then make the manual changes (setting the guid, account name and total time).

1) It is redundant to validate as you have done it. You could write code to reflect the validation metadata much like the default does, or just remove the data annotations validation since you are not using it in your model binder.

2) I don't know, it seems correct, you should step through the code and make sure your custom binder is populating all of the applicable rules.

3) It's a factory for sure, but not so much a constructor.

---

EDIT: you couldn't be any closer to the solution, just set the properties you need in the model factory function

public class ProjectModelBinder : DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        if (bindingContext.ModelType == typeof(Project))
        {
            ModelBindingContext newBindingContext = new ModelBindingContext()
            {

                ModelMetadata = ModelMetadataProviders.Current.GetMetadataForType(
                    () => new Project()  // construct a Project object
                    {
                        ProjectGuid = System.Guid.NewGuid(),
                        AccountName = controllerContext.HttpContext.User.Identity.Name,
                        // don't set name, thats the default binder's job
                        TotalTime = 0,
                    }, 
                    typeof(Project)         // using the Project metadata
                ),
                ModelState = bindingContext.ModelState,
                ValueProvider = bindingContext.ValueProvider

            };

            // call the default model binder this new binding context
            return base.BindModel(controllerContext, newBindingContext);
        }
        else
        {
            return base.BindModel(controllerContext, bindingContext);
        }
    }

}

Or you could alternately override the CreateModel method:

protected override object CreateModel(ControllerContext controllerContext, ModelBindingContext bindingContext, System.Type modelType)
{
    if (modelType == typeof(Project))
    {
        Project model = new Project()
        {
            ProjectGuid = System.Guid.NewGuid(),
            AccountName = controllerContext.HttpContext.User.Identity.Name,
            // don't set name, thats the default binder's job
            TotalTime = 0,
        };

        return model;
    }

    throw new NotSupportedException("You can only use the ProjectModelBinder on parameters of type Project.");
}

Polacre

Hey do you mind checking my post I made an edit to it at the bottom with a possible solution, I just don't know how to do the next step. Modelbinding is still really new to me. – Sigmatism

Or perhaps you have a simpler way of achieving the same thing. – Sigmatism

@Odnxe: it's hard to get much closer than you were, I implemented this in MVC3 to double check it works as intended (or at least my understanding of your intention). – Polacre

Awesome, Thank you for that last little bit of help! I knew I was close I'm just quite new to model binding and MVC in general. You saved me hours and hours of headache. – Sigmatism

Both solutions work, is it just a preference thing to go with one or the other? Or is one approach inherently better than the other? – Sigmatism

@Odnxe: For your situation I would go with the CreateModel method since that is exactly what you are trying to override. In the other question you linked, the person was trying to use the metadata from an interface to validate a concrete implementation, and hence needed to override the metadata provider. The CreateModel code is also less confusing. – Polacre