Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complex validations #39

Open
AnasKiron opened this issue Nov 27, 2022 · 13 comments
Open

Complex validations #39

AnasKiron opened this issue Nov 27, 2022 · 13 comments
Labels

Comments

@AnasKiron
Copy link

Dear Steven,
I hope my question finds you well,

We had complex validations on the commands, for example: one field is required based on the value of other fields.

In your provided SOLID Services template there is DataAnnotationsValidator. I wounder if we can extend it or do something to enable the complex validation I mentioed.

Please assit us.
Yours,
Anas

@dotnetjunkie
Copy link
Owner

Data Annotations is typically suited for simple validations that can verify a single property in complete isolation, and in some cases to check two properties on the same model, without having to interact with the outside world. For everything else, Data Annotations is (imo) unsuited. This means it is unsuited for:

  • usage of Volatile Dependencies. For instance comparing the property with a current system time, is not a good idea.
  • going through larger set of objects
  • querying the database or any other external service.

Typically, it means that DataAnnotation validations should be Stable Dependencies.

For any other validation, it'd be good to define a separate abstraction, e.g. IValidator<T>. Things great benefits:

  • Each individual validation can be placed in a separate class. This class can have dependencies, a logical name, and can be tested individually.
  • You can have multiple validators on a single command. e.g. two implementations for IValidator<ShipOrder>.
  • Using a DI Container, these IValidator<T> implementations can be Auto-Registered and used as a collection of validators. For instance, while validating ShipOrder, you can iterate an IEnumerable<IValidator<ShipOrder>>.
  • You can force validation of command handlers by wrapping them in a decorator. This decorator can ensure that both the DataAnnotations validation and the more complex IValidator<T> implementations are executed.

Such IValidator<T> could look like this:

public interface IValidator<in T>
{
    IEnumerable<ValidationResult> Validate(T instance);
}

And you could have implementations such as:

public sealed record InstrumentShouldNotBeLent(IEntityLoader<InstrumentEntity> Repository) : IValidator<LendInstrument>
{
    public IEnumerable<ValidationResult> Validate(LendInstrument command)
    {
        var instrument = this.Repository.GetById(command.LendingInformation.InstrumentId);

        if (instrument.TemporaryLocation != null)
        {
            yield return new ValidationResult("The instrument has already been lent.");
        }
    }
}

and:

public sealed record NextCalibrationDateMustBeAFutureDate(ITimeProvider TimeProvider) : IValidator<AddCalibration>
{
    public IEnumerable<ValidationResult> Validate(AddCalibration instance) => this.Validate(instance.Calibration);

    public IEnumerable<ValidationResult> Validate(Calibration instance)
    {
        DateTime tomorrow = this.TimeProvider.UtcNow.Date.AddDays(1.0);

        if (instance.NextCalibrationDate < tomorrow)
        {
            yield return new ValidationResult("The next calibration date should be a future date.");
        }
    }
}

Finally, you can create a decorator for your command handlers that ensures validation is performed:

public sealed record ValidationCommandHandlerDecorator<TCommand>(
    IEnumerable<IValidator<TCommand>> Validators,
    ICommandHandler<TCommand> Decoratee)
    : ICommandHandler<TCommand>
{
    public void Handle(TCommand command)
    {
        this.Validate(command);
        this.Decoratee.Handle(command);
    }

    private void Validate(TCommand command)
    {
        var context = new ValidationContext(command, null, null);

        // Execute DataAnnotations validation first. This will throw a ValidationException
        // when there is an error.
        Validator.ValidateObject(command, context, validateAllProperties: true);

        // Validate using IValidator<T>. For simplicity, I just pick the first error here.
        // In practice, you might want to push all validation errors back to the client.
        var error = (
            from validator in this.Validators
            from result in validator.Validate(command)
            select result)
            .FirstOrDefault();

        if (error != null)
        {
            throw new ValidationException(error, null, null);
        }
    }
}

This decorator can be wrapped around all command handlers, which ensures no command is executed without proper validation.

Besides this validation, you might want other types of validation, such a security validation. That allows you to implement row-based security; that deserves an interface of its own.

You might want to do validation of queries as well. That would mean adding a decorator for your query handlers. The concepts stay the same.

I hope this helps

@AnasKiron
Copy link
Author

Thanks Steven,
It works well.

And here is how I registered the collection of validators into the Simple Injector container:

container.Collection.Register(typeof(IValidator<>), BusinessLayerAssemblies);

@MarkusRodler
Copy link

@dotnetjunkie I agree with you that Data Annotations are not the silver bullet to handle all validations, but unfortunately in my opinion your Validator approach isn't either.
The reason is because there is also always the possibility that your validation does not run. You separate the validation logic from the domain. With this everyone can call the domain without the validation logic executed first.
In my opinion that can work in a small setup but as it grows there is always that one new developer that oversees the validation requirement and then it breaks.

So I think it is much saver to not separate the validation from the domain. The commands should only be messages that can fail or succeed. Maybe they are wrong, maybe they are not.

@dotnetjunkie
Copy link
Owner

@hi @MarkusRodler,

there is also always the possibility that your validation does not run [...] there is always that one new developer that oversees the validation requirement and then it breaks

It's up to you to decide what's the best design for your given context. In the contexts I've been working with, however, this design suited us very well. I implemented this in smaller and bigger teams even that were working with newer developers, and, using this design, never had the problem of validation logic that didn't run. This is caused by a mix of education, code reviews, but especially the design of using ICommandHandler<TCommand> itself.

I found that I can educate even newer developers in a matter of minutes that calls from, for instance, a MVC controller to the business logic must always go through either an ICommandHandler<TCommand> or an IQueryHandler<TQuery, TResult>. As a matter of fact, if you really wish (but I never had to do this) you can even make it architectural impossible to call those concrete classes themselves. You can do this by moving that business logic to an assembly that isn't referenced by the presentation layer, and you can make sure that the solution stops compiling when someone accidentally adds a reference from presentation to that assembly. But again, I never had to do this.

When all these calls to the business layer go through these two abstractions, you can guarantee that all validations are executed. This can be done in 3 simple steps:

  1. Create a decorator (see my previous ValidationCommandHandlerDecorator for instance)
  2. Write a unit test that tests the correctness of that decorator
  3. Write a unit test that verifies that command handlers are wrapped with that decorator

You separate the validation logic from the domain.

It depends on how you view commands. I view commands as intrinsic part of my domain. They are the verbs in my domain. They are named in a way that I can typically communicate with business users about them. They are not called UseCase_025_Command, but rather given names the business understands, like ShipOrder. From that perspective, their validations are part of the domain. From a technical perspective, you can place those validator implementations anywhere you like. You can place them in the same folder as the commands, you can place them near the handlers, in a different project, or as close to the rest of your domain as you want. If you wish, you can even place those validator classes in the same file, or even within a handler class, or even have a single class implement multiple IValidator<T> interfaces (not that I endorse this, but the options are endless).

But that said, validation on the level of the command is different from validation on the Domain Entity. My experience is that, for more complex systems, I tend to need both. And how to validate those Domain Entities is a topic of its own. You can prevent Domain Entities from getting in an invalid state by making them immutable and verify their preconditions, but in that case you still don't want to implement any validations that have to go to the database, or any other Volatile Dependency. Those precondition checks should typically not much more than date must always be after the year 2000, or end date must always be after start date. For validations that require Volatile Dependencies, IMO you still want to pull the validation out, preferably in a structure similar to the one given above. And even in that case, you can ensure that those validations are always triggered, independently to what a individual developers does. My typical solution is a decorator icw an ORM that can spot which entities are changed, because in that case I can guarantee that the validation is executed.

But again, as always, your mileage may vary.

@MarkusRodler
Copy link

MarkusRodler commented Nov 28, 2022

Well I can understand that the approach can work if trained accordingly and various checks are made. But I just wanted to show the dangers that can happen. I'm glad it fits in your case.

@dotnetjunkie
Copy link
Owner

I'm glad we can have a open conversation about this. There are multiple ways to skin a cat, and the community will benefit the most from these conversations and reading about different point of views.

That said, can you demonstrate in a few lines of code what your approach would be? This would give readers a better insight in what the options are.

@MarkusRodler
Copy link

MarkusRodler commented Nov 28, 2022

For sure. I'm also glad that we can have a open conversation. In the end, everyone can only win.
In principle it is like this dummy code (CQRS + ES):

public sealed record ImproveArticleTitleCommand(Guid ArticleId, string NewTitle) : Command;
public sealed class CommandHandler
{
    public static async Task Handle(ImproveArticleTitleCommand command, EventStore eventStore)
    {
        // 1. Make new title a value object
        // The Value Object may enforce a minimum of at least 2 characters within the constructor
        var newTitle = TitleValueObject.Create(command.NewTitle);
        // (Maybe ask an external service, the database or whatever if the title is unique? Then do it here)

        // 2. Get the aggregate and call the appropriate action
        var articleAggregate = eventStore.Read("Article", command.ArticleId);
        articleAggregate.ImproveTitle(newTitle);

        // 3. Persist new state
        // OriginalVersion to detect if it was updated before
        eventStore.Write("Article", command.ArticleId, articleAggregate.EventLog, articleAggregate.OriginalVersion);
    }
}

So the rules are:

  1. Don't fall to primitive obsession within the domain (e.g. TitleValueObject should be a value object and not a string)
  2. Don't use value objects in commands and events

@dotnetjunkie
Copy link
Owner

And can you show an example of validation as well?

@MarkusRodler
Copy link

public sealed class TitleValueObject
{
    public string Title { get; }

    public TitleValueObject(string title)
    {
        var trimmedTitle = title.Trim();
        EnsureTitleContainsAtLeast2Characters(trimmedTitle);
        Title = trimmedTitle;
    }

    public static new TitleValueObject Create(string title) => new(title);

    public override string ToString() => Title;

    static void EnsureTitleContainsAtLeast2Characters(string title)
    {
        if (string.IsNullOrWhiteSpace(title))
        {
            throw new ArgumentException("Title must be at least 2 characters long");
        }
    }
}

@dotnetjunkie
Copy link
Owner

And can you show an example of a validation that depends on a Volatile Dependency? For instance, requires a call to the database?

@MarkusRodler
Copy link

After creating the value object var newTitle from above (articleTitleDatabase would be a third parameter in the Handle method:

articleTitleDatabase.EnsureTitleIsUnique(newTitle);

EnsureTitleIsUnique method within ArticleTitleDatabase:

public void EnsureTitleIsUnique(TitleValueObject newTitle)
{
    var titles = this.QueryAll();
    var match = titles.Where(title => newTitle);
    if (match is not null)
    {
        throw new Exception("Title already chosen");
    }
}

Disclaimer: Not production code. Only intended to demonstrate the principle.

@dotnetjunkie
Copy link
Owner

Good stuff. Thanks for sharing this.

@MarkusRodler
Copy link

You're welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants