7
\$\begingroup\$

I've created a "framework" or more of a library for a CQS implementation with a few decorators. Using ASP.NET Core for the front end, I wanted an opinion about how this looks/feels. I changed the default mvc approach and created a Features folder that stores both the Views and Controllers in the same folder. Then each command/query ends up being in its own folder in the implementation. I have the following decorators: Exception/Logging, Retry, Cache, Cache Invalidation, Validation, and Permission. Any input at all is welcomed!

This will be along post, but it'll be an interesting read, in my opinion. :)

First, CQS = Command Query Separation. Essentially, queries return data and do not modify state and commands modify state but return no data. There is a gray area here where you need to run a command but then return the data (i.e. Queues, Stacks, primary keys from identity fields). Thus, I created a "CommandQuery" for those scenarios where just a command won't be good enough. CQS normally has ICommandHandler<> and IQueryHandler<> interfaces to support decorators. Decorators are classes that can add functionality to these classes without changing them. i.e. The calls are chained. They implement the same interface, but I use SimpleInjector to make those classes run before/after the actual implementation. All code becomes narrow and easy to maintain. Also, you can add more decorators to all commands/queries with one line of code.

I tried to follow SOLID through all the layers here with the intention that every piece of code will be unit testable at a high level. The problems being solved inside may end up more difficult than that, but I digress. :)

(My mock implementation deals with Digimon World 2... Don't ask) :)

Controller

[Route(ApiConstants.RootApiUrlVersion1 + "DigimonWorld2Admin/Digimon/Create")]
public class CreateCommandQueryController : MetalKidApiControllerBase
{
    private readonly IResponseMediator _responseMediator;

    public CreateCommandQueryController(IResponseMediator responseMediator) =>
        _responseMediator = responseMediator;

    [HttpPost]
    public async Task<IActionResult> Post([FromBody] CreateCommandQuery commandQuery) =>
        await _responseMediator.ExecuteAsync(commandQuery).ConfigureAwait(false);
}

Since we can route the different verbs to different classes, we can focus on only 1 method at a time per class. I'm using Mediators everywhere so the code looks the same across all classes. Mediators tie a request 1:1 with an implementation via a specific generic interface. I thought this was really the Service Locator anti-pattern, but it isn't because you still know the mediator dependency. I also use async through the entire call stack because Task.CompletedTask, if needed, has very little overhead and not blocking threads is the way to go.

ResponseMediator

public class ResponseMediator : IResponseMediator
{
    private readonly ICqsMediator _mediator;
    private readonly ICommonResource _resource;

    public ResponseMediator(ICqsMediator mediator, ICommonResource resource)
    {
        Guard.IsNotNull(mediator, nameof(mediator));

        _mediator = mediator;
        _resource = resource;
    }

    public async Task<IActionResult> ExecuteAsync(
        ICommand command, CancellationToken token = default(CancellationToken)) =>
        HandleResult(await _mediator.ExecuteAsync(command, token).ConfigureAwait(false));

    public async Task<IActionResult> ExecuteAsync<TResponse>(
        ICommandQuery<TResponse> query, CancellationToken token = default(CancellationToken)) =>
        HandleResult(await _mediator.ExecuteAsync(query, token).ConfigureAwait(false));

    public async Task<IActionResult> ExecuteAsync<TResponse>(
        IQuery<TResponse> query, CancellationToken token = default(CancellationToken)) =>
        HandleResult(await _mediator.ExecuteAsync(query, token).ConfigureAwait(false));

    private IActionResult HandleResult<T>(IResult<T> result)
    {
        if (result.IsSuccessful)
        {
            return new OkObjectResult(result.Data);
        }
        return HandleResult((IResult)result);
    }

    private IActionResult HandleResult(IResult result)
    {
        if (result.IsSuccessful)
        {
            return new OkResult();
        }
        if (result.BrokenRules?.Any() == true)
        {
            return new BadRequestObjectResult(new {result.BrokenRules});
        }
        if (result.HasConcurrencyError)
        {
            return new BadRequestObjectResult(new {Message = _resource.ErrorConcurrency});
        }
        if (result.HasNoPermissionError)
        {
            return new UnauthorizedResult();
        }
        if (result.HasNoDataFoundError)
        {
            return new NotFoundResult();
        }
        if (!string.IsNullOrEmpty(result.ErrorMessage))
        {
            return new BadRequestObjectResult(new {Message = result.ErrorMessage});
        }
        return new BadRequestObjectResult(new {Message = _resource.ErrorGeneric});
    }
}

All my queries/commands/command queries end up returning IResult because I found throwing exceptions is really expensive. Thus, this lets me deal with any issues. IResult is returned by Command and doesn't return any info about the actual command itself, so it doesn't violate CQS. Query returns IResult< T > that stores the data.

I'll show an example here of the Create Command through the layers.

CommandQuery

public class CreateCommandQuery : ICommandQuery<int>, ICommandQueryRetry
{
    public string Name { get; set; }
    public int DigimonTypeId { get; set; }
    public int RankId { get; set; }
    public int? LearnedSkillId { get; set; }
    public int? SpecialtyId { get; set; }
    public string Found { get; set; }
    public bool? HasImage { get; set; }
    public int? HP { get; set; }
    public int? MP { get; set; }
    public int? Att { get; set; }
    public int? Def { get; set; }
    public int? Speed { get; set; }

    int ICommandQueryRetry.MaxRetries => 3;
}

ICommandQueryRetry will cause this to retry general exceptions up to 3 times before failing.

For caching, you just add a different interfaces (a few different ones for different timeframes) and it takes care of it for you.

public class ListQuery : IQuery<ICollection<ListDto>>, IQueryCacheableAbsoluteTimespan
{
    public string Name { get; set; }

    TimeSpan IQueryCacheableAbsoluteTimespan.ExpireTimeout => TimeSpan.FromDays(1);
}

CommandQueryHandler

public class CreateCommandQueryHandler : ICommandQueryHandler<CreateCommandQuery, int>
{
    private readonly IRepositoryMediator _repositoryMediator;

    public CreateCommandQueryHandler(IRepositoryMediator repositoryMediator) =>
        _repositoryMediator = repositoryMediator;

    public async Task<IResult<int>> ExecuteAsync(
        CreateCommandQuery commandQuery, CancellationToken token = default(CancellationToken))
    {
        var digimonId = await _repositoryMediator.ExecuteAsync<int>(commandQuery, token).ConfigureAwait(false);
        return ResultHelper.Successful(digimonId);
    }
}

Validation

The validation decorator will run validation on multiple threads (if you want) and async will not block. I didn't use expressions since a lot of times validation involves multiple properties and has a lot of overhead. The BrokenRule() parameter is the "Relation" and it can be used to tie the rule somewhere on the UI.

public class CreateCommandQueryValidator : CommandQueryValidatorBase<CreateCommandQuery, int>
{
    private readonly IValidationRepositoryMediator _validationRepositoryMediator;
    private readonly IDigimonWorld2AdminResources _resources;

    public CreateCommandQueryValidator(IValidationRepositoryMediator validationRepositoryMediator,
        IDigimonWorld2AdminResources resources)
    {
        _validationRepositoryMediator = validationRepositoryMediator;
        _resources = resources;
    }

    protected override void CreateRules(CancellationToken token = default(CancellationToken))
    {
        AddRules(
            () => Validate.If(string.IsNullOrEmpty(CommandQuery.Name))?.BrokenRule(nameof(CommandQuery.Name))
                .Required(_resources.DigimonCreateCommandName),
            () => Validate.If(CommandQuery.DigimonTypeId == 0)?.BrokenRule(nameof(CommandQuery.DigimonTypeId))
                .Required(_resources.DigimonCreateCommandDigimonTypeId),
            () => Validate.If(CommandQuery.RankId == 0)?.BrokenRule(nameof(CommandQuery.RankId))
                .Required(_resources.DigimonCreateCommandRankId)
        );

        AddRules(_validationRepositoryMediator.GetValidationRules(CommandQuery, token));
    }
}

public class CreateCommandQueryValidationRepository : IValidationRepository<CreateCommandQuery>
{
    private readonly IValidator _validate;
    private readonly IDigimonWorld2ContextFactory _contextFactory;
    private readonly IDigimonWorld2AdminResources _resources;

    public CreateCommandQueryValidationRepository(IValidator validate, IDigimonWorld2ContextFactory contextFactory,
        IDigimonWorld2AdminResources resources)
    {
        _validate = validate;
        _contextFactory = contextFactory;
        _resources = resources;
    }

    public ICollection<Func<Task<BrokenRule>>> GetValidationRules(
        CreateCommandQuery request, CancellationToken token = default(CancellationToken)) =>
        new List<Func<Task<BrokenRule>>>(1)
        {
            async () =>
            {
                using (var context = _contextFactory.Create(false))
                {
                    return _validate.If(
                            !string.IsNullOrEmpty(request.Name) &&
                            await context.Digimons
                                .AnyAsync(a => a.Name.Equals(request.Name, StringComparison.OrdinalIgnoreCase),
                                    token)
                                .ConfigureAwait(false))?.BrokenRule(nameof(request.Name))
                        .AlreadyInUse(_resources.DigimonCreateCommandName, request.Name);
                }
            }
        };
}

Validation uses ?. here so that the second part isn't run if there is no rule. .NET Core changed the way translations work, so I had to IoC inject a reference to them now, which is why you see that.

CreateCommandRepository

public class CreateCommandQueryRepository : IRepository<CreateCommandQuery, int>
{
    private const int ExpectedRowsAffected = 1;

    private readonly IDigimonWorld2ContextFactory _contextFactory;
    private readonly IMapperMediator _mapperMediator;

    public CreateCommandQueryRepository(
        IDigimonWorld2ContextFactory contextFactory,
        IMapperMediator mapperMediator)
    {
        _contextFactory = contextFactory;
        _mapperMediator = mapperMediator;
    }

    public async Task<int> ExecuteAsync(CreateCommandQuery request, CancellationToken token = default(CancellationToken))
    {
        using (var context = _contextFactory.Create())
        {
            var entity = _mapperMediator.Map<DigimonEntity>(request);
            context.Digimons.Add(entity);

            await context.SaveChangesAsync(ExpectedRowsAffected, token).ConfigureAwait(false);

            return entity.DigimonId;
        }
    }
}

My "repository" is a little different. The repository here is above the database technology being used. That way, if you want to change from EF to a webservice or use Dapper/EF interchangeably or something, you can change that implementation easily without affecting the handler above.

Mapper

public class CreateCommandQueryMapper : IMapper<CreateCommandQuery, DigimonEntity>
{
    public DigimonEntity Map(CreateCommandQuery command, DigimonEntity to = default(DigimonEntity))
    {
        to = to ?? new DigimonEntity();

        to.Name = command.Name;
        to.DigimonTypeId = command.DigimonTypeId;
        to.RankId = command.RankId;
        to.LearnedSkillId = command.LearnedSkillId;
        to.Att = command.Att;
        to.Def = command.Def;
        to.Found = command.Found;
        to.HasImage = command.HasImage;
        to.HP = command.HP;
        to.MP = command.MP;
        to.SpecialtyId = command.SpecialtyId;
        to.Speed = command.Speed;

        return to;
    }
}

I find if you do this in chunks together (all these classes, minus the controller) are all in the same folder as they are all related, that coding the mapping by hand isn't that big of a deal. Though, I made this generic so you could setup your own that just used AutoMapper or whatever instead.

Invalidate Cache

public class CreateCommandQueryInvalidateCache : CacheManagerCommandQueryInvalidateCache<CreateCommandQuery, int>
{
    public CreateCommandQueryInvalidateCache(
        IQueryCacheRegion queryCacheRegion, 
        ICacheManager<object> cache) : base(queryCacheRegion, cache) { }

    protected override Task<ICollection<Type>> GetTypeOfQueriesToInvalidateAsync(
        CancellationToken token = default(CancellationToken)) =>
        Task.FromResult<ICollection<Type>>(new[] {typeof(ListQuery)});
}

If you know the queries you affect, you just have to give it the query type and it'll take care of the rest. I made it async in case it took some IO to figure out all the types.

Decorators

The most important decorator I wanted to show was the exception handling one that is at the top of the chain:

public class CommandQueryHandlerExceptionDecorator<TCommandQuery, TResult> : ICommandQueryHandler<TCommandQuery, TResult>
    where TCommandQuery : ICommandQuery<TResult>
{
    private readonly ICommandQueryHandler<TCommandQuery, TResult> _commandQueryHandler;
    private readonly ILogger _logger;
    private readonly IUserContext _userContext;
    private readonly ICommonResource _resource;

    public CommandQueryHandlerExceptionDecorator(
        ICommandQueryHandler<TCommandQuery, TResult> commandQueryHandler, ILogger logger, IUserContext userContext,
        ICommonResource resource)
    {
        Guard.IsNotNull(commandQueryHandler, nameof(commandQueryHandler));
        Guard.IsNotNull(logger, nameof(logger));

        _commandQueryHandler = commandQueryHandler;
        _logger = logger;
        _userContext = userContext;
        _resource = resource;
    }

    public async Task<IResult<TResult>> ExecuteAsync(TCommandQuery commandQuery,
        CancellationToken token = default(CancellationToken))
    {
        try
        {
            return await _commandQueryHandler.ExecuteAsync(commandQuery, token).ConfigureAwait(false);
        }
        catch (UserFriendlyException ex)
        {
            await _logger.LogAsync(new LogEntry(LogTypeEnum.Error, _userContext,
                        "Friendly exception with command query: " + typeof(TCommandQuery).FullName, ex,
                        commandQuery),
                    token)
                .ConfigureAwait(false);
            return ResultHelper.Error<TResult>(ex.Message);
        }
        catch (DataNotFoundException ex)
        {
            await _logger.LogAsync(new LogEntry(LogTypeEnum.Error, _userContext,
                    "Data Not Found exception with command query: " + typeof(TCommandQuery).FullName, ex,
                    commandQuery), token)
                .ConfigureAwait(false);
            return ResultHelper.NoDataFoundError<TResult>();
        }
        catch (ConcurrencyException ex)
        {
            await _logger.LogAsync(new LogEntry(LogTypeEnum.Error, _userContext,
                        "Concurrency error with command query: " + typeof(TCommandQuery).FullName, ex,
                        commandQuery),
                    token)
                .ConfigureAwait(false);
            return ResultHelper.ConcurrencyError<TResult>();
        }
        catch (Exception ex)
        {
            await _logger.LogAsync(new LogEntry(LogTypeEnum.Error, _userContext,
                    "Error with command query: " + typeof(TCommandQuery).FullName, ex, commandQuery), token)
                .ConfigureAwait(false);
            return ResultHelper.Error<TResult>(_resource.ErrorGeneric);
        }
    }
}

If you are still reading and your eyes haven't glazed over yet, what are your thoughts? Is this too much? Thanks!

---Edit---

Retry Decorator

    public class CommandHandlerRetryDecorator<TCommand> : ICommandHandler<TCommand> where TCommand : ICommand
    {
        private readonly ICommandHandler<TCommand> _commandHandler;
        private int _maxRetries;
        private int _retryDelayMilliseconds;

        public CommandHandlerRetryDecorator(ICommandHandler<TCommand> commandHandler)
        {
            Guard.IsNotNull(commandHandler, nameof(commandHandler));

            _commandHandler = commandHandler;
        }

        public async Task<IResult> ExecuteAsync(TCommand command, CancellationToken token = default(CancellationToken))
        {
            if (!(command is ICqsRetry retry))
            {
                return await _commandHandler.ExecuteAsync(command, token).ConfigureAwait(false);
            }

            _maxRetries = retry.MaxRetries;
            _retryDelayMilliseconds = retry.RetryDelayMilliseconds;
            return await ExecuteWithRetryAsync(command, 0, token).ConfigureAwait(false);
        }

        private async Task<IResult> ExecuteWithRetryAsync(TCommand command, int tries,
            CancellationToken token = default(CancellationToken))
        {
            try
            {
                return await _commandHandler.ExecuteAsync(command, token).ConfigureAwait(false);
            }
            catch (BrokenRuleException)
            {
                throw; // Cannot retry this
            }
            catch (NoPermissionException)
            {
                throw; // Cannot retry this
            }
            catch (ConcurrencyException)
            {
                throw; // Cannot retry this
            }
            catch (DataNotFoundException)
            {
                throw; // Cannot retry this
            }
            catch (Exception ex)
            {
                if (tries >= _maxRetries) throw;

                if (command is ICqsRetrySpecific specific)
                {
                    if (!RetryHelper.HasAnyExceptionMatch(
                        specific.RetryCheckBaseTypes,
                        specific.RetryCheckInnerExceptions,
                        ex,
                        specific.OnlyRetryForExceptionsOfTheseTypes)) throw;
                }
                if (_retryDelayMilliseconds > 0)
                {
                    await Task.Delay(_retryDelayMilliseconds, token).ConfigureAwait(false);
                }
                return await ExecuteWithRetryAsync(command, ++tries, token).ConfigureAwait(false);
            }
        }
    }

Delete Command

public class DeleteCommand : ICommand, ICqsRetry
{
    public int DigimonId { get; set; }
    public byte[] Timestamp { get; set; }

    int ICqsRetry.MaxRetries => 3;
    int ICqsRetry.RetryDelayMilliseconds => 100;
}

public class DeleteCommandHandler : ICommandHandler<DeleteCommand>
{
    private readonly IRepositoryMediator _repositoryMediator;

    public DeleteCommandHandler(IRepositoryMediator repositoryMediator) =>
        _repositoryMediator = repositoryMediator;

    public async Task<IResult> ExecuteAsync(DeleteCommand command,
        CancellationToken token = default(CancellationToken))
    {
        await _repositoryMediator.ExecuteAsync(command, token).ConfigureAwait(false);
        return ResultHelper.Successful();
    }
}

public class DeleteCommandRepository : IRepository<DeleteCommand>
{
    private const int ExpectedRowsAffected = 1;

    private readonly IDigimonWorld2ContextFactory _contextFactory;
    private readonly IMapperMediator _mapperMediator;

    public DeleteCommandRepository(
        IDigimonWorld2ContextFactory contextFactory,
        IMapperMediator mapperMediator)
    {
        _contextFactory = contextFactory;
        _mapperMediator = mapperMediator;
    }

    public async Task ExecuteAsync(DeleteCommand request, CancellationToken token = default(CancellationToken))
    {
        using (var context = _contextFactory.Create())
        {
            var entity = _mapperMediator.Map<DigimonEntity>(request);
            context.Digimons.Remove(entity);
            await context.SaveChangesAsync(ExpectedRowsAffected, token).ConfigureAwait(false);
        }
    }
}

public class DeleteCommandMapper : IMapper<DeleteCommand, DigimonEntity>
{
    public DigimonEntity Map(DeleteCommand command, DigimonEntity to = default(DigimonEntity))
    {
        to = to ?? new DigimonEntity();

        to.DigimonId = command.DigimonId;
        to.Timestamp = command.Timestamp;

        return to;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Just looking for any opinions that this looks good or this looks like crap (and reasons why). \$\endgroup\$ – Daniel Lorenz Jan 22 '18 at 14:56
  • \$\begingroup\$ Also, I now added a Timing decorator that logs information about how long a command/query takes to run with an added ability to log a warning if it took longer than a specified threshold. \$\endgroup\$ – Daniel Lorenz Apr 9 '18 at 17:23
0
\$\begingroup\$

I have to play with the code in an IDE, but I really like that you’ve addressed caching at the outaet as it seems one of the more obvious benefits of havung well defined queries & results (rather than a generic repository which would then need a second layer to cache responses.

\$\endgroup\$
  • 1
    \$\begingroup\$ Moderator note: This answer does mention a positive thing about the code and thus I consider it to be an answer. \$\endgroup\$ – Simon Forsberg Apr 1 '18 at 14:33
  • \$\begingroup\$ I agree. Hopefully he'll have time to play with it, but then again, I left a lot of other code out so might be tough. :) \$\endgroup\$ – Daniel Lorenz Apr 2 '18 at 18:39
0
\$\begingroup\$

This looks great! The only things I can think of are the following:

Validation:

Your validation interface is called IValidator, but the instance is called _validate. Both make sense in their own context, but seeing IValidator _validate just doesn't seem right. Maybe rename the instance to _validator, or the interface to IValidationAction or IValidationRule?

Access modifiers:

It is hard to say without having a view on your project/solution structure, but do all methods need to be public?

\$\endgroup\$
  • \$\begingroup\$ Heh, yeah the naming itself needs to be updated to match correctly, I agree. Oversight. As for access modifiers, handler works with internal but repository does not for some reason. You could update the mediators to call these methods dynamically if you really didn't want them to be public, but you probably also want to unit test these things. The internal dependencies to these classes can be whatever you want, though. As for the ExceuteAsync methods, yes, they have to be public since they are interface driven. \$\endgroup\$ – Daniel Lorenz Aug 30 '18 at 15:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.