HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

Returning a validation error from a command handler

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
errorvalidationreturningcommandfromhandler

Problem

public class AddPostCommand
{
    public string Title { get; set; }
    public string Body { get; set; }
    public bool Published { get; set; }
}

public class AddPostCommandHandler
{
    public string Handle(AddPostCommand command)
    {
        var param = new
        {
            Slug = command.Title.Replace(" ", "-"),
            PublishDate = command.Published ? DateTime.Now : DateTime.MinValue,
            command.Title,
            command.Body,
            command.Published
        };

        // I removed the data access code for brevity.

        return param.Slug;
    }
}


I have a command and I have a command handler. I also have a controller that invokes the command handler.

Within the command handler I generate a slug (definition). In the database, the slug column has a unique constraint - there must be no duplicate slugs.

I need to find a clean way to inform the caller that they have effectively provided to the command handler a duplicate slug.

I do not perform any validation in the command handler yet. This is the simplest solution I can come up with:

PostRequestHandler requestHandler = new PostRequestHandler();
AddPostCommandHandler commandHandler = new AddPostCommandHandler();

[HttpPost]
public ActionResult AddPost(AddPostCommand command)
{
    string expectedSlug = command.Title.Replace(" ", "-");
    PostRequest request = new PostRequest { Slug = expectedSlug };
    PostModel post = requestHandler.Handle(request);
    if (post != null)
    {
        ModelState.AddModelError("", "A post with this title already exists.");
        return View(post);
    }

    string actualSlug = commandHandler.Handle(command);
    return RedirectToAction("Index", "Post", new { actualSlug });
}


I abhor this code.

My most pressing concern is that I do not want the caller to have to generate the slug. I am also concerned that the controller action is becoming too big.

How might I communicate to the controller that an error has occurred (so th

Solution

This is an interesting question because I've never seen the MVC and the command pattern implemented together. Typically I think WebForms when I think of the command pattern. I'm still on the fence on whether or not it is a good fit but there are a couple of MVC concepts you can use to make your code cleaner. The most valuable of which in your case is to:

Leverage Custom Model Binding

I agree, your controller shouldn't have to do work to build the request model (slug, post request), this should all be happening in a custom model binder:

public class AddPostCommandModelBinder : DefaultModelBinder
{

    protected virtual void OnModelUpdated(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        var command = (AddPostCommand)bindingContext.Model;
        // Add the ExpectedSlug property to the request command and have the ModelBinder do this work (as well as any other post-binding logic
        command.ExpectedSlug = command.Title.Replace(" ", "-");
    }
}


In addition to this, your PostRequestHandler is a concern much better solved using Model Binding, as the responsibility of Model Binding is to take request parameters and map them to a strongly typed object as well as validating those inputs (by default using the DataAnnotations namespace: http://msdn.microsoft.com/en-us/library/system.componentmodel.dataannotations%28v=vs.110%29.aspx.

In yet another Model Binder, you can put any custom validation logic, for example:

public override Object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
    var newModel = (PostRequest)bindingContext.Model;
    newModel.AttemptedSlug = bindingContext.ValueProvider.GetValue("title").AttemptedValue;

    if(string.IsNullOrWhiteSpace(newModel.AttemptedSlug))
    {
        bindingContext.ModelState.AddModelError("AttemptedSlug", "The Attempted Slug is required.");
    }
    // Other custom logic

    return newModel;
}


A point on Injection

As with anything MVC, the code should be test-able. I'm not sure if you omitted it for brevity, but the command handlers should be constructor-injected interfaces bound to concrete types (ideally) using an IoC container. Also, with the changes above, your controller should now look like this:

public class PostController : Controller
{
    IPostCommandHandler _postCommandHandler;

    public PostController(IPostCommandHandler addCommandHandler)
    {
        this._postCommandHandler = addCommandHandler;
    }

    [HttpPost]
    public ActionResult AddPost(AddPostCommand command, PostRequest submitRequest)
    {
        ActionResult result = null;
        if (!submitRequest.IsValid)
        {
            result = View("AddPost", submitRequest);
        }
        else
        {
            string actualSlug = this._postCommandHandler.Handle(command);
            result = RedirectToAction("Index", "Post", new { actualSlug });
        }

        return result;
    }
}


Ultimately, I follow several rules of thumb as far as my separation of concerns that have kept me mostly out of trouble.

  • Model binders bind all request parameters to strongly typed request objects (with a few exceptions).



  • Model binders handle basic data-validation and error handling for user input



  • Controllers accept request models, check for validity, call external data services to build response view models, and determine action (redirect, view, etc...)



Other Notes

The usage of the word "post" as part of your domain (presumably you're using the term in the sense of a blog "post"), should be re-considered. POST carries a lot of context with it and some developers (myself included) will have to read around the common use of post when reading this code.

  • Consider renaming post to blogPost or forumPost or newsPost to make the code clearer.



Adding validation logic in the controller should be a red flag, but adding messaging in the controller is perfectly appropriate, as the messaging is directly related to the view (which is found, prepared and delivered by the controller). I wouldn't do a null check as you have done, but either use a validator in the model binder (ModelState.IsValid), or providing your own as I did above as part of the PostRequest object.

You may also want to consider adding an AddPostCommand property to your PostRequest, which would then require your controller to take only one argument. You can still have a custom model binder for each type.

Code Snippets

public class AddPostCommandModelBinder : DefaultModelBinder
{

    protected virtual void OnModelUpdated(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        var command = (AddPostCommand)bindingContext.Model;
        // Add the ExpectedSlug property to the request command and have the ModelBinder do this work (as well as any other post-binding logic
        command.ExpectedSlug = command.Title.Replace(" ", "-");
    }
}
public override Object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
    var newModel = (PostRequest)bindingContext.Model;
    newModel.AttemptedSlug = bindingContext.ValueProvider.GetValue("title").AttemptedValue;

    if(string.IsNullOrWhiteSpace(newModel.AttemptedSlug))
    {
        bindingContext.ModelState.AddModelError("AttemptedSlug", "The Attempted Slug is required.");
    }
    // Other custom logic

    return newModel;
}
public class PostController : Controller
{
    IPostCommandHandler _postCommandHandler;

    public PostController(IPostCommandHandler addCommandHandler)
    {
        this._postCommandHandler = addCommandHandler;
    }

    [HttpPost]
    public ActionResult AddPost(AddPostCommand command, PostRequest submitRequest)
    {
        ActionResult result = null;
        if (!submitRequest.IsValid)
        {
            result = View("AddPost", submitRequest);
        }
        else
        {
            string actualSlug = this._postCommandHandler.Handle(command);
            result = RedirectToAction("Index", "Post", new { actualSlug });
        }

        return result;
    }
}

Context

StackExchange Code Review Q#72166, answer score: 3

Revisions (0)

No revisions yet.