Validation within Aggregate Roots - Part 2

CommandValidator.cs

  public interface IValidationHandler<in T> where T : Command {
    ValidationResult Validate(T cmd);
  }

  public interface ICommandValidator {
    ValidationResult Validate<T>(T command) where T : Command;
  }

  public class CommandValidator : ICommandValidator {
    private readonly IDictionary<Type, Func<object, ValidationResult>> _validationHandlers 
      = new Dictionary<Type, Func<object, ValidationResult>>();

    public void RegisterValidationHandler<T>(Func<T,ValidationResult> handler) where T : Command {
      _validationHandlers.Add(typeof(T), o => handler((T) o));
    }

    public ValidationResult Validate<T>(T command) where T : Command {
      Func<object, ValidationResult> handler;
      if (!_validationHandlers.TryGetValue(command.GetType(), out handler)) 
        return new ValidationResult();
      try {
        return handler(command);
      }
      catch(TargetInvocationException ex) {
        throw ex.InnerException;
      }
    }
  }

  public class ValidationResult {
    private readonly List<ValidationFailure> errors = new List<ValidationFailure>();

    public bool IsValid {
      get { return Errors.Count == 0; }
    }

    public IList<ValidationFailure> Errors {
      get { return errors; }
    }

    public ValidationResult() { }

    public ValidationResult(IEnumerable<ValidationFailure> failures) {
      errors.AddRange(failures.Where(failure => failure != null));
    }
  }

  public class ValidationFailure {
    public ValidationFailure(string propertyName, string error)
      : this(propertyName, error, null) {}

    public ValidationFailure(string propertyName, string error, object attemptedValue) {
      PropertyName = propertyName;
      ErrorMessage = error;
      AttemptedValue = attemptedValue;
    }

    public string PropertyName { get; private set; }
    public string ErrorMessage { get; private set; }
    public object AttemptedValue { get; private set; }

    public override string ToString() {
      return ErrorMessage;
    }
  }
  // Class to encapsulate all command validation rules for an Aggregate Root
  public class PatientValidationHandler : IValidationHandler<AddReferredPatient>, IValidationHandler<ChangePatientAddress> {
    public bool Validate(AddReferredPatient command) {
      var validationErrors = new List<ValidationFailure>();      
      
      if (string.IsNullOrWhitespace(command.Firstname))
        validationErrors.Add("firstname", "First Name is required");

      return new ValidationResult(validationErrors);
    }

gistfile1.cs

// Register your command validators as you might your command handlers (N.B. this could be done with an IoC or reflection to auto-wireup)
var patientValidator = new PatientValidationHandler();
validator.RegisterValidator<AddReferredPatient>(patientValidator.Validate);
validator.RegisterValidator<ChangePatientAddress>(patientValidator.Validate);

// Then on all command sending
protected void Send<T>(T cmd) where T : Command {
  // Validate the command
  var result = Validator.Validate(cmd);
  // if invalid set Model Error warnings and be done
  if (!result.IsValid) {
    SetModelErrors(result.Errors);
  }
  else { // if valid try a send
    try {
      Bus.Send(cmd);
    }
    catch (DomainValidationException ex) { //only catch business rule failures that can only be validated from within the AR
      SetModelErrors(ex.Errors);
    }
  }
}

// since both command and domain validation return the same error collection use the same process to strip them out
private void SetModelErrors(IEnumerable<ValidationFailure> errors) {
  foreach (var error in errors) {
    ModelState.AddModelError(error.PropertyName, error.ErrorMessage);
  }
}

Following on from our discussions last week, I think we can all agree that the pursuit of validation in one place is unachievable, as Chris pointed out. But what I think is more important is the realisation (at least on my part) that there are clearly two distinct types of validation. So, the above aims to represent my first pass at isolating the command validation logic outside of the aggregate root (and the mechanics of the message bus entirely) but also still allow for the consistent feedback to the user that the previous solution offered. Let me know your thoughts, am I solving problem that doesn't exist?

Couple of observations:

1. The Validate method in the interface IValidationHandler returns bool - but judging by the sample validation, it doesn't look like you intend to return anything other than true (failure cases are throwing).
2. Exceptions are being used for process flow - since you're expecting invalid data from time to time (since this is user input), this flow isn't exceptional, it's expected.

If you want to follow CQS, you could validate into an object (in a similar vain to asp.net MVC validating into ModelStateDictionary) rather than returning bool.

Apart from that, I like where this is going.

I'm glad you've made those points. I had the very same feeling once I'd isolated the command validation from the AR, that throwing felt wrong (esp. since the validation now occurs before the command is put on the bus and potentially crossing a system boundary). And that the return of true is totally redundant. Then I thought back to a post about not returning bools since whenever you need to, you usually also need context so return a Result object.

I feel an updated gist coming on...

Paul, if you're adding validation to a command object then I'd stick to the data annotation attributes, like [Required] and [StringLength]. That way anyone consuming the command can determine the rules easily. Not sure what classes would then validate your command outside of MVC, but they must exist.

That doesn't solve the problem of duplicating validation, but given that a command and domain classes validate on different levels I don't know if you can resolve this. For example, the command can only validate the primitive types for things like presence and length, but I'd expect more complex validation on the server. E.g a command that allocates a nurse onto a duty will supply just a nurseid and dutyid. It can check that the ids are not the empty guid, bout not that they are valid or that the nurse's contract is valid on the date of the duty.

In the interests of trying to get the code being shaped by the real world problem, what are the rules behind AddReferredPatient command?

Another option would be to declare the validation constraints outside of the command.

Have you considered using something like FluentValidation (http://fluentvalidation.codeplex.com/)? If the mechanics for validation aren't a core part of the projects exploration, it may be cheaper to "buy" vs build.

Chris - regarding "the nurse's contract is valid on the date of the duty", I consider this kind of checking to be outside the scope of validation as it appears to be a business rule. Validation is generally checking that the message is something that makes sense (simple format/schema checking), but not necessarily conforming to business rules and requirements. As a corollary to this, business rules define the correctness of an operation, and I would consider this type of check to happen in the domain model itself (or at least closer to the domain model than the validation piece of the puzzle)*.

The validation components could be deployed to both client and server components, which alleviates the duplication problem.

In regards to rules behind AddReferredPatient command, it maybe worth fleshing out the user story here (I'm still not sure the name of the command is right, but I would expect that to come out of conversations with domain experts). From my experience in this domain, some patients turn up at treatment centers after a referral from their GP. In this specific case (there are a ton of other ways patients end up at a treatment center) the patient will need be registered. This enables the treatment center to monitor the patients attendance. Attendance tracking is necessary to support the many KPI's that treatment centers need to report against to show compliance to contractual obligations. An example of this (from a few years back now) was a program called "18 week wait" which was an initiative to attempt to reduce patient waiting times. The general idea was that a patient would wait no longer than 18 weeks from being referred, to their initial treatment for the referred condition (of course there are a ton of exclusions to this SLA). Often the patients non-clinical details (name, address etc) are provided by the national patient records (think web service) and are looked up based on either the referral (think paper form rather than electronic record) or details provided by the patient (if the treatment center is a walk-in-clinic, patient walking in are treated as self-referrals). It appears that this command may have been intended to handle the registration of a patient in the treatment center's PAS (patient administration system).

*It sounds like staff contracts could be part of a saga, in which case, the NurseID might make up part of the correlation id mechanics.

Hi gents, sorry I'm late to the party on this. In my opinion, validation is a slightly overloaded term. I think that "validation" belongs in both the command (to protect the state of your domain) and for validating user input.
In my view is that it's perfectly OK to throw an exception to stop your domain from getting into a bad state, this is really should be an exceptional case as bad user input should be caught earlier than this.
Validation of user input should happen as soon as possible after they have input it, and this should not throw an exception as you would expect users to make plenty of mistakes (as you have all said).
In my opinion, this is not a violation of DRY as each form of "validation" has a different purpose. Also, the best defences are layered defences.

Based on the comments provided I've updated the gist to show a non-throwing command validation approach that returns a ValidationResult (which could be converted to use FluentValidation without much effort but I didn't like the dependency on a base class)

Regarding the validation issue in general, I think we can all agree that commands should be validated before sending and that the domain needs to protect itself from invalid state. Since commands should only embody primitive types the nature of command validation is only ever likely to be basic input validation (null checks etc.), whilst the domain will include business-driven validation to maintain a valid state.

BUT the question is - should the domain expect only valid commands or guard against the receipt of invalid commands? This is somewhat of a moot point in my mind for two reasons;

1. Since the above approach would allow for the definition of command validation in one place (DRY), the domain could re-validate the commands on receipt.
2. However, since commands only embody primitive types and the level of validation is only input/null checking, how likely is it for this input data to be the main driver behind AR state change? From my experience so far, the command/event themselves are the main catalyst for AR state change, not the data in the command. When the command data is not AR or other ref IDs, it's usually display data mainly consumed by the ReadModel.

I'm sorry, but I don't follow what you mean by commands/events being the main catalyst for change, not the data in the command.

I think Paul means that the aggregate often behaves like a state machine, where receiving a command is the trigger for changing state; the data in the command is less important to the state of the aggregate than the trigger caused by the actual receipt of the command.

Would you mind if I draw us back to the actual story, and see what real code that produces? Working to complete the story will force the completion of all the code from textbox to db. Seeing the whole thing, we can then decide which bits have value. I'd submit some code myself, but I don't have a windows machine.

So, what is the original story relating to referring patients?

Is it

1. As a GP I want to refer a Patient, so that we can give treatment
2. As a Patient I want to be referred by a GP, so that I can receive treatment

Difference being that #1 has a GP sat at a PC and #2 is a patient showing up at a surgery with details of a referral from a GP.

Feels like a Referral is the actual AR as it has some kind of approval/acceptance process, KPIs and record of Treatment and eventual Completion (which may include another referral). So it also feels like the main story is an epic, which needs breaking down.

As a GP I want to refer a Patient, so that we can give treatment

1. As a GP I want to create a Referral, so that a patient can receive treatment and we can record performance
2. As a Specialist I want to browse open referrals to my TreatmentCentre, so that I can accept/reject them
3. As a Specialist I want to accept/reject a referral
4. As a Specialist I want to record Treatment against a Referral, so that we can measure performance against KPIs
5. As a Specialist I want to complete a Referral, so that we can finalise the KPIs
6. As a GP I want to browse my Referrals, so that I can see their progress
7. As a GP I want to browse my Patient's Referrals, so that I can review treatment

So what ARs do we have? Patient, GP, Specialist, Referral & TreatmentCentre?

So, if we're looking at story 1.1 then it appears to me that we've got a story more complex than just validating primitive types, and it should highlight for us how the system will deal with things like referential integrity, and answer the question about where is Treatment recorded?

Thanks Craig, that was what I was getting at. Chris, I thought the original focus was on validation strategies so I'd extended my examples in that direction. I agree that getting a stable set of stories agreed is necessary for taking the discussion regarding the specifics of the aggregate roots forward.

Can I just caveat my responses with the fact that I'm currently working in a completely separate and unrelated domain and am transposing my examples over. Therefore loosing the true business context and fitting the above solutions to contrived examples in the healthcare domain. I appreciate that aggregate roots (once defined) in our shared healthcare context may not behave the same as mine in my current context, but I felt that the discussions regarding validation transcended these domain specifics.

Can I suggest that we start a new gist focused on clarifying the ARs?