Friday, November 12, 2010

But you can't do that! Command Validators in the MVC stack

 

 

So, yeah, DataAnnotations are great.  I use them for Required, StringLength, Regex, and such. I use them, and you should too. In a CQRS system, you want to avoid issuing commands that won't succeed at all costs (uhhhh... duhhhhh).  I keep hearing Greg and Udi talk about how a shared library that can used that checks rules to check the validity of a command before issuing it. I can't find one, so I'm gunna do it. I'm thinking we just need a simple interface,  like so.
   1:  using myDojo.Infrastructure.CQRS.Commands;
   2:   
   3:  namespace myDojo.Infrastructure.CQRS.Validation
   4:  {
   5:      public interface IValidate<TCommand> where TCommand : ICommand
   6:      {
   7:          bool IsValid(TCommand command);
   8:      }
   9:  }

What I'm thinking is that I'll just use my IOC container to go get all these, execute each of them for a command. If any of them fails, we don't issue the command, and by checking what IValidate failed, we know why, and can communicate that to the user.

I think something like this...

   1:  using System.Collections.Generic;
   2:  using myDojo.Infrastructure.CQRS.Commands;
   3:   
   4:  namespace myDojo.Infrastructure.CQRS.Validation
   5:  {
   6:      public interface ICommandValidator<TCommand> where TCommand : ICommand
   7:      {
   8:          bool IsValid<TCommand>(TCommand command);
   9:          IEnumerable<IValidate<TCommand>> FailedRules { get; set; }
  10:      }
  11:  }



I guess that looks good... Lets try and make an implementation.  Test first.

   1:  using myDojo.Infrastructure.CQRS.Commands;
   2:  using myDojo.Infrastructure.CQRS.Validation;
   3:  using NUnit.Framework;
   4:  using Rhino.Mocks;
   5:  using StructureMap;
   6:   
   7:  namespace myDojo.Domain.UnitTests.Infrastructure
   8:  {
   9:      [TestFixture]
  10:      public class when_using_the_command_validator
  11:      {
  12:          
  13:          [Test]
  14:          public void if_a_validator_fails_is_valid_will_return_false()
  15:          {
  16:              if_the_command_is_not_valid();
  17:              when_Validate_Command_is_called();
  18:              _isValid.ShouldBeFalse();
  19:          }
  20:          
  21:          private void if_the_command_is_not_valid()
  22:          {
  23:              var validatorThatWillReturnFalse = MockRepository.GenerateMock<IValidate<SomeTestCommand>>();
  24:              validatorThatWillReturnFalse.Stub(v => v.IsValid(_command)).Return(false);
  25:              ObjectFactory.Inject(validatorThatWillReturnFalse);
  26:          }
  27:   
  28:          private CommandValidator<SomeTestCommand> _validator;
  29:          private SomeTestCommand _command;
  30:          private bool _isValid;
  31:   
  32:   
  33:          public void when_Validate_Command_is_called()
  34:          {
  35:              _validator = new CommandValidator<SomeTestCommand>(ObjectFactory.Container);
  36:              _isValid = _validator.IsValid(_command);
  37:          }
  38:          [SetUp]
  39:          public void establish_context()
  40:          {
  41:              ObjectFactory.Initialize(a => { });
  42:              _command = new SomeTestCommand();
  43:              
  44:          }
  45:      }
  46:      public class SomeTestCommand : ICommand
  47:      {
  48:          
  49:      }
  50:  }

Ok, so I'm just mocking up a validator that will fail, and making sure IsValid returns false. Here's what I did to make it pass.

   1:  using System;
   2:  using System.Collections.Generic;
   3:  using System.Linq;
   4:  using myDojo.Infrastructure.CQRS.Commands;
   5:  using StructureMap;
   6:   
   7:  namespace myDojo.Infrastructure.CQRS.Validation
   8:  {
   9:      public class CommandValidator<TCommand> : ICommandValidator<TCommand> where TCommand : ICommand
  10:      {
  11:          private readonly IContainer _container;
  12:   
  13:          public CommandValidator(IContainer container)
  14:          {
  15:              _container = container;
  16:          }
  17:   
  18:          public bool IsValid(TCommand command)
  19:          {
  20:              foreach(var validator in _container.GetAllInstances<IValidate<TCommand>>())
  21:              {
  22:                  if (!validator.IsValid(command))
  23:                      return false;
  24:              }
  25:              return true;
  26:          }
  27:   
  28:          public IEnumerable<IValidate<TCommand>> FailedRules
  29:          {
  30:              get { throw new NotImplementedException(); }
  31:              set { throw new NotImplementedException(); }
  32:          }
  33:      }
  34:  }
So, that one is good. Lets go add a test to make sure the positive is good (when there's only valid commands). Here she is.
 

   1:  [Test]
   2:          public void if_the_validator_passes_is_valid_will_return_false()
   3:          {
   4:              if_the_command_is_valid();
   5:              when_Validate_Command_is_called();
   6:              _isValid.ShouldBeTrue();
   7:          }
   8:          private void if_the_command_is_valid()
   9:          {
  10:              var validatorThatWillReturnFalse = MockRepository.GenerateMock<IValidate<SomeTestCommand>>();
  11:              validatorThatWillReturnFalse.Stub(v => v.IsValid(_command)).Return(true);
  12:              ObjectFactory.Inject(validatorThatWillReturnFalse);
  13:          }



Next up, lets get the failed command.

   1:    [Test]
   2:          public void failed_rules_will_contain_the_failed_validator()
   3:          {
   4:              if_the_command_is_not_valid();
   5:              when_Validate_Command_is_called();
   6:              _validator.FailedRules.ShouldContain(ObjectFactory.GetInstance<IValidate<SomeTestCommand>>());
   7:          }



We just go inject a failing command, then go make sure it gets added to FaildRules. Fails now (because we didn't write the code)... Lets go fix it.

Couple changes here, but pretty simple.

   1:  using System;
   2:  using System.Collections.Generic;
   3:  using System.Linq;
   4:  using myDojo.Infrastructure.CQRS.Commands;
   5:  using StructureMap;
   6:   
   7:  namespace myDojo.Infrastructure.CQRS.Validation
   8:  {
   9:      public class CommandValidator<TCommand> : ICommandValidator<TCommand> where TCommand : ICommand
  10:      {
  11:          private readonly IContainer _container;
  12:          private List<IValidate<TCommand>> _failedRules;
  13:   
  14:          public CommandValidator(IContainer container)
  15:          {
  16:              _container = container;
  17:              _failedRules = new List<IValidate<TCommand>>();
  18:          }
  19:   
  20:          public bool IsValid(TCommand command)
  21:          {
  22:              foreach(var validator in _container.GetAllInstances<IValidate<TCommand>>())
  23:              {
  24:                  if (!validator.IsValid(command))
  25:                      _failedRules.Add(validator);
  26:              }
  27:              return _failedRules.Count() == 0;
  28:          }
  29:   
  30:          public IEnumerable<IValidate<TCommand>> FailedRules { get { return _failedRules; } }
  31:      }
  32:  }
 
Instead of just short-circuiting the check for failed validators in the loop, now I just add each failed one to the collection, and then just return that true if the collection count is 0. Oh yeah, I removed the setter on FailedRules from the interface, since we won't need that.
 
And she passes!
 
Ok, let's go use it now. What I'm thinking is that (for now), I'll just plug it into my CommandActionResult. I probably could get prettier, but lets just make it work first.
 
Here's what CommandActionResult.ExecuteResult looks like now.
 

   1:   public override void ExecuteResult(ControllerContext context)
   2:          {
   3:              
   4:              if (!context.Controller.ViewData.ModelState.IsValid)
   5:              {
   6:                  ValidationFailedResult().ExecuteResult(context);
   7:                  return;
   8:              }
   9:              ICommandHandlerResult result = null;
  10:             
  11:              var handler = Container.GetInstance<ICommandHandler<TCommand>>();
  12:              result = handler.Handle(Command);
  13:              if(result is Success)
  14:              {
  15:                  Success().ExecuteResult(context);
  16:                  return;
  17:              }
  18:              CommandFailedResult(result).ExecuteResult(context);
  19:          }


So, before we even get the handler (line 11), lets just go get the CommandValidator and check it first.


Here's what I got now.

   1:  public override void ExecuteResult(ControllerContext context)
   2:          {
   3:   
   4:              if (!context.Controller.ViewData.ModelState.IsValid)
   5:              {
   6:                  ValidationFailedResult().ExecuteResult(context);
   7:                  return;
   8:              }
   9:   
  10:              var validator = Container.GetInstance<ICommandValidator<TCommand>>();
  11:   
  12:              if (!validator.IsValid(Command))
  13:              {
  14:                  ValidationFailedResult().ExecuteResult(context);
  15:                  return;
  16:              }
  17:   
  18:              ICommandHandlerResult result = null;
  19:             
  20:              var handler = Container.GetInstance<ICommandHandler<TCommand>>();
  21:              result = handler.Handle(Command);
  22:              if(result is Success)
  23:              {
  24:                  Success().ExecuteResult(context);
  25:                  return;
  26:              }
  27:              CommandFailedResult(result).ExecuteResult(context);
  28:          }
 
Kinda ugly... We've got 2 places where we're executing that ValidationFailedResult. That kinda smells, but I'd hate to go get the validator if I don't need it right? Screw it... I'm gunna go try this puppy out, then try to clean it up. 
 
Lets go make a validator to make sure we don't create dupe email addresses. So, I'm actually making a new assembly for my validators because validation will use both Commands and Queries - and I don't want either of those having a ref to each other. 
 
Here's my validator.
 

   1:  using myDojo.Infrastructure;
   2:  using myDojo.Infrastructure.CQRS.Validation;
   3:  using MyDojo.Query.ViewModels;
   4:   
   5:  namespace myDojo.Commands.Users.Validation
   6:  {
   7:      public class EmailAddressMustBeUnique : IValidate<RegisterUser>
   8:      {
   9:          private readonly IReadModelRepository<MartialArtistDetails> _readModelRepository;
  10:   
  11:          public EmailAddressMustBeUnique(IReadModelRepository<MartialArtistDetails> readModelRepository)
  12:          {
  13:              _readModelRepository = readModelRepository;
  14:          }
  15:   
  16:          public bool IsValid(RegisterUser command)
  17:          {
  18:              return _readModelRepository.GetSingle(m => m.EmailAddress == command.EmailAddress) == null;
  19:          }
  20:      }
  21:  }
 
 
Give it a shot..
 

Server Error in '/' Application.



StructureMap Exception Code:  202
No Default Instance defined for PluginFamily myDojo.Infrastructure.CQRS.Validation.ICommandValidator`1[[myDojo.Commands.Users.RegisterUser, myDojo.Commands, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], myDojo.Infrastructure, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

 
I actually knew that was gunna happen, but I wanted my blog to be a bit longer so I could show off about structure map. Gotta go tell SM to add all those validators. Simple enough....
 
Here's my quick integration test for my registry.
 

   1:   [Test]
   2:          public void get_a_EmailIsUnique_validator()
   3:          {
   4:              _container.GetAllInstances<IValidate<RegisterUser>>()
   5:                  .FirstOrDefault(v => v is EmailAddressMustBeUnique).ShouldNotBeNull();
   6:          }
 
Registry to make it pass looks like so.
 

   1:   
   2:   
   3:  using myDojo.Infrastructure.CQRS.Validation;
   4:  using StructureMap.Configuration.DSL;
   5:   
   6:  namespace myDojo.Commands.Users.Validation
   7:  {
   8:      public class CommandValidationRegistry : Registry 
   9:      {
  10:          public CommandValidationRegistry()
  11:          {
  12:              Scan(s =>
  13:                       {
  14:                           s.AssemblyContainingType(GetType());
  15:                           s.ConnectImplementationsToTypesClosing(typeof (IValidate<>));
  16:                           s.ConnectImplementationsToTypesClosing(typeof (ICommandValidator<>));
  17:                       });
  18:          }
  19:      }
  20:  }


Lets try the web again (that's what I get for not TDDing, huh?)...


Same freeking thing?! Oh yeah, that's because I've been working all day and my brain is fried. I forgot to add ref to the new assembly to my web project. Ok... add it, and...

What the??! Same thing... Time to go look at that test again. I'm missing something


Duh! I only tested that we could get a IValidate<RegisterUser>, not an ICommandValidator. New test looks like so.

   1:   [Test]
   2:          public void get_a_RegisterUser_CommandValidator()
   3:          {
   4:              _container.GetInstance<ICommandValidator<RegisterUser>>().ShouldNotBeNull();
   5:          }


Yeah, there, got the error. Now lets go fix it. Something to do with open generics. I think this'll fix it.


   1:   

   2:   

   3:  using myDojo.Infrastructure.CQRS.Validation;

   4:  using StructureMap.Configuration.DSL;

   5:   

   6:  namespace myDojo.Commands.Users.Validation

   7:  {

   8:      public class CommandValidationRegistry : Registry 

   9:      {

  10:          public CommandValidationRegistry()

  11:          {

  12:              Scan(s =>

  13:                       {

  14:                           s.AssemblyContainingType(GetType());

  15:                           s.ConnectImplementationsToTypesClosing(typeof (IValidate<>));

  16:                           s.ConnectImplementationsToTypesClosing(typeof (ICommandValidator<>));

  17:   

  18:                       });

  19:              For(typeof (ICommandValidator<>)).Use(typeof(CommandValidator<>));

  20:          }

  21:      }

  22:  }

 

Finally! Actually we don't need line 16. Why? You know, I'm gunna email the SM user group and find out, cause I ain't quite sure. Remove it and re-run test... Yep, I'm good.
Ok, lets hit website again.

 

And we're good!

 

We're not yet communicating to the user why the hell the command failed though. Lets see what we can do there.

   1:  public override void ExecuteResult(ControllerContext context)
   2:          {
   3:   
   4:              if (!context.Controller.ViewData.ModelState.IsValid)
   5:              {
   6:                  ValidationFailedResult().ExecuteResult(context);
   7:                  return;
   8:              }
   9:   
  10:              var validator = Container.GetInstance<ICommandValidator<TCommand>>();
  11:   
  12:              if (!validator.IsValid(Command))
  13:              {
  14:                  foreach (var failedRule in validator.FailedRules)
  15:                  {
  16:                      context.Controller.ViewData.ModelState.AddModelError(failedRule.GetType().Name,failedRule.ToString());
  17:                  }
  18:                  ValidationFailedResult().ExecuteResult(context);
  19:   
  20:                  return;
  21:              }
  22:   
  23:              ICommandHandlerResult result = null;
  24:             
  25:              var handler = Container.GetInstance<ICommandHandler<TCommand>>();
  26:              result = handler.Handle(Command);
  27:              if(result is Success)
  28:              {
  29:                  Success().ExecuteResult(context);
  30:                  return;
  31:              }
  32:              CommandFailedResult(result).ExecuteResult(context);
  33:          }

 

Now we just override ToString() on the validation message, and they get added to the Model Errors. Here's what my view looks like - oh yeah, it's Razor.


 

   1:  @inherits System.Web.Mvc.WebViewPage<myDojo.Web.Models.RegisterUserForm>
   2:   
   3:  @{
   4:      View.Title = "Register";
   5:      LayoutPage = "~/Views/Shared/_Layout.cshtml";
   6:  }
   7:   
   8:      <h2>Create your account</h2>
   9:      <form action='@Url.Action("Register","User")' method='POST'>
  10:              @Html.ValidationSummary()
  11:          <div>
  12:              @Html.LabelFor(m => m.EmailAddress)
  13:              @Html.TextBoxFor(m => m.EmailAddress)
  14:              @Html.ValidationMessageFor(m => m.EmailAddress)
  15:          </div>
  16:          <div>
  17:              <input type="submit" value="ok" />
  18:          </div>
  19:      </form>

 

And when I try to register with a pre-existing email address I see this.

 
blog
 
I think we can clean up that CommandActionResult a lot, and I probably will - just not tonight.
 
I like having my command validation in it's own assembly. This gives me some DRY around my validation, not sure how much I'll reuse it yet, but we'll find out, huh?
 
Have fun with it, tell me why it sucks, or why it doesn't.



 
 

5 comments:

  1. You seriously should consider using FluentValidation, by Jeremy Skinner. http://fluentvalidation.codeplex.com/

    ReplyDelete
  2. Wow! a reader from the DDD/CQRS group. I hope my shameless self promotion in that answer about problems with EC didn't offend anyone :)

    Thanks @seagile, I'll check it out.

    /E

    ReplyDelete
  3. I need to add this to the ugly "sign up with credentials from your favorite social site" code I crammed into our AccountController this past week. But that assumes we can break our commands out from the "all-in-one" facade pattern we're using for our commands into separate command classes. Poor me.

    ReplyDelete