Everything wrong with eShopOnContainers

iamprovidence

19 min read

·

Feb 27, 2024

--

6

Listen

Share

Press enter or click to view image in full size

In the dynamic realm of modern software development, the quest for building scalable, resilient, and efficient applications has led to the emergence of microservices architectures.

Microsoft’s eShopOnContainers has garnered attention for its attempt to showcase best practices and guide developers to embrace development methodologies.

If you’re living under a rock, eShopOnContainers is an open-source project that Microsoft developed to showcase and demonstrate the architecture of modern microservices-based applications. There is a dedicated GitHub repository, documentation, and e-book.

However, as with any ambitious project, eShopOnContainers is not without its fair share of strengths and weaknesses.

In this article, we will see what is wrong with eShopOnContainers. We will mainly focus on how domain events, integration events, and idempotency are implemented. Stuff that can be improved. Not only that, you will see what Microsoft did right. But let’s not admire them too much. Just enough to give appropriate respect to all those decent engineers working hard for their families. After all, Microsoft is just a soulless corporation of evil not caring about you or me, just trying to increase their profit 😒.

Since the project is quite big and still in the evolving phase, we will mainly focus on Ordering microservice from this archived code. I won’t be able to fit everything anyway, (otherwise, you will never finish this article), but if this article gets popular expect a second part.

That was enough for the introductory part. You know what to expect, so I think we can start 🙃.

Architecture

As with other projects, the easiest way to explore them is by starting from architecture and slowly deep into the code.

For Ordering microservice Microsoft uses layers from DDD architecture.

Press enter or click to view image in full size

That is where the first problem arrives. Ordering.API project is a mixture of Presentation and Application layers. It is even more visible when you unfold it. There is a folder called Application that logically represents the Application layer. Not having it in a separate project can quickly lead to mixed responsibilities.

The difference between a separate project and a folder is that the project allows you to manage dependencies between layers.

I don’t mind that some small microservices have logical layers separation with folders and not physical with projects. But it is just not consistent when it is already done for other layers. So let’s address this issue and do it properly:

Press enter or click to view image in full size

One can argue how an API project can represent Presentation layers if it only contains endpoints returning raw data. That is the point!

Presentation does not always mean shiny buttons and fabulous animations. The presentation layer presents your application to the outside world in the form of a graphical user interface (GUI) for users, a command line interface (CLI) for developers, an application programming interface (API) for programs, etc.

I know this section was not originally planned, but it just triggers me all the time 😤. Now, when we get that out of the way, let’s continue.

Domain events

Overuse of MediatR

Now, when we fixed it from an aerial view it is time to start digging into😌. As always, we will start with the most important project, business logic.

It is important to keep each project clean and don’t add dependencies that are not needed. This is why I was surprised to see the Domain project referencing MediatR. MediatR is an Application layer nuget used to embody slice architecture. What the hell it is doing in the Domain?

If we remove that nuget and recompile the solution, we will quickly see why it was added. It seems like all domain events inherit INotification interface.

public class OrderCancelledDomainEvent: MediatR.INotification
{
   public Order Order { get; }

   public OrderCancelledDomainEvent(Order order)
   {
       Order = order;
   }
}

It is strange. I would expect to see something like IDomainEvent. But let’s not judge beforehand. For that, we need to investigate further: a full flow of domain events processing. Maybe there is an explanation for it.

Domain events are raised and stored in the aggregate when the corresponding action is performed. Seems to be correct.

public abstract class Entity
{
                    . . .
    private List<INotification> _domainEvents = new List<INotification>();
    public IReadOnlyCollection<INotification> DomainEvents => _domainEvents.AsReadOnly();

    public void AddDomainEvent(INotification eventItem)
    {
        _domainEvents.Add(eventItem);
    }
                    . . .
}

public class Order: Entity, IAggregateRoot
{
                    . . .
    public void SetCancelledStatus()
    {
        _orderStatusId = OrderStatus.Cancelled.Id;
        
        AddDomainEvent(new OrderCancelledDomainEvent(this));
    }
                      . . .
}

The aggregate is loaded and updated at the application level inside the handler. Yeap. That is correct too🧐.

public class ShipOrderCommandHandler : IRequestHandler<ShipOrderCommand>
{
    private readonly IOrderRepository _orderRepository;

    public ShipOrderCommandHandler(IOrderRepository orderRepository)
    {
        _orderRepository = orderRepository;
    }

    public async Task Handle(ShipOrderCommand command, CancellationToken cancellationToken)
    {
        var orderToUpdate = await _orderRepository.GetAsync(command.OrderNumber);

        orderToUpdate.SetShippedStatus();

        await _orderRepository.UnitOfWork.SaveEntitiesAsync(cancellationToken);
    }
}

I think we need to explore SaveEntitiesAsync(). In the infrastructure layer, right before saving updated records, you can find code that publishes domain events through the MediatR:

public class OrderingContext : DbContext, IUnitOfWork
{
                    . . .

    private readonly IMediator _mediator;

    public OrderingContext(DbContextOptions<OrderingContext> options, IMediator mediator) : base(options)
    {
        _mediator = mediator;
    }

    public async Task<bool> SaveEntitiesAsync(CancellationToken cancellationToken = default(CancellationToken))
    {
        await _mediator.DispatchDomainEventsAsync(this);

        await base.SaveChangesAsync(cancellationToken);

        return true;
    }
                    . . .
}

There is an extension class to publish domain events. It does follow:

• gets updated Entities from DbContext (didn’t we just come from there? 🤔)
• retrieve their domain events as INotification
• clear domain events so they are not published multiple times
• publish them via IMediator:

static class MediatorExtension
{
    public static async Task DispatchDomainEventsAsync(this IMediator mediator, OrderingContext ctx)
    {
        // 1
        var domainEntities = ctx
            .ChangeTracker
            .Entries<Entity>()
            .Where(x => x.Entity.DomainEvents.Any());

        // 2
        var domainEvents = domainEntities
            .SelectMany(x => x.Entity.DomainEvents)
            .ToList();

        // 3
        domainEntities
            .ToList()
            .ForEach(entity => entity.Entity.ClearDomainEvents());

        // 4
        foreach (var domainEvent in domainEvents)
            await mediator.Publish(domainEvent);
    }
}

Later in the application layer, you can find INotificationHandler which is in fact domain event handler:

public class OrderCancelledDomainEventHandler
    : INotificationHandler<OrderCancelledDomainEvent>
{
    public async Task Handle(
      OrderCancelledDomainEvent orderCancelledDomainEvent, 
      CancellationToken cancellationToken)
    {
                    .  .  .
    }
}

The flow is clear to me. They inherit INotification to leverage the MediatR notification system.

There are also other issues. We will address them one by one. However, first, we should establish was the usage of MediatR in the Domain layer justified.

On the one side:

• it is a pragmatic approach that simplifies our code and saves time
• we know that our Application layer uses MediatR and it won’t change
• we also know that Ordering.Domain is only used by Ordering microservice and other microservices won’t reference it directly

On the other:

• not making code clean it is just being lazy
• we make an assumption about the implementation of the underlying layer ignoring the whole point of separation of concern
• in the case of a monolith, where business logic is shared are used across different layers, this implementation may create additional obstacles

In this particular example, we can leave stuff as is. However, there is always a space for improvement. So, let’s see what could be done better.

This is where I like Jason Taylor and his CleanArchitecture template more. He faced the same problem but solved it differently. He has… wait.. where it was… Seriously Jason? You were supposed to destroy the evil, not join them! Don’t do it to me, Jason, don’t be like that or I will review your Clean Architecture template too 😤. Fine, I’ll do it myself.

First, we need to create a marker interface IDomainEvent to replace INotification:

public interface IDomainEvent { }

public class OrderCancelledDomainEvent : IDomainEvent
{
      .  .  .
}

We also need to replace it in the abstract class:

public abstract class Entity
{
            .  .  .
    private List<IDomainEvent> _domainEvents = new List<IDomainEvent>();;
    public IReadOnlyCollection<IDomainEvent> DomainEvents => _domainEvents?.AsReadOnly();

    public void AddDomainEvent(IDomainEvent eventItem)
    {
        _domainEvents.Add(eventItem);
    } 
            .  .  .
}

Everything up to the point where events are published will stay the same. The only thing we need to update would be DispatchDomainEventsAsync() method. But even these changes will be minimal:

• gets updated Entities from DbContext
• retrieve their domain events as IDomainEvent
• clear domain events so they are not published multiple times
• wrap domain events with the DomainEventNotification class
• publish them via IMediator:

static class MediatorExtension
{
    public static async Task DispatchDomainEventsAsync(this IMediator mediator, OrderingContext ctx)
    {
        // 1
        var domainEntities = ctx
            .ChangeTracker
            .Entries<Entity>()
            .Where(x => x.Entity.DomainEvents.Any());

        // 2
        var domainEvents = domainEntities
            .SelectMany(x => x.Entity.DomainEvents)
            .ToList();

        // 3
        domainEntities
            .ToList()
            .ForEach(entity => entity.Entity.ClearDomainEvents());

        // 4
        var domainEventNotifications = domainEvents
            .Select(CreateDomainEventNotification);

        // 5
        foreach (var notification in domainEventNotifications)
            await mediator.Publish(notification);
    }

    private static INotification CreateDomainEventNotification(IDomainEvent domainEvent)
    {
        var genericType = typeof(DomainEventNotification<>).MakeGenericType(domainEvent.GetType());

        return (INotification)Activator.CreateInstance(genericType, domainEvent);
    }
}

Oh yes, I forget to tell you. In step 5, we can not publish domain events in the mediator as it used to be, because they no longer inherit INotification. Therefore we had to introduce a new class:

public class DomainEventNotification<TDomainEvent> : INotification
    where TDomainEvent : IDomainEvent
{
    public TDomainEvent DomainEvent { get; }

    public DomainEventNotification(TDomainEvent domainEvent)
    {
        DomainEvent = domainEvent;
    }
}

It is nothing but a dummy wrapper over IDomainEvent that inherits INotification to make our little trick possible.

We also need to unwrap it later in the domain event handler.

public class OrderCancelledDomainEventHandler
    : INotificationHandler<DomainEventNotification<OrderCancelledDomainEvent>>
{
    public Task Handle(DomainEventNotification<OrderCancelledDomainEvent> notification, CancellationToken cancellationToken)
    {
        var domainEvent = notification.DomainEvent;
                        .  .  .
    }
}

Not a big of a price for a clean architecture, is it? 😉

Overloaded DbContext

Have you noticed how disordered the code is? Domain events were published in the Application layer, through Infrastructure layer facilities, which called the Application layer back again 🤢.

That’s where the shoe pinches:

public class OrderingContext : DbContext, IUnitOfWork
{
                    . . .

    private readonly IMediator _mediator;

    public OrderingContext(DbContextOptions<OrderingContext> options, IMediator mediator) : base(options)
    {
        _mediator = mediator;
    }

    public async Task<bool> SaveEntitiesAsync(CancellationToken cancellationToken = default(CancellationToken))
    {
        await _mediator.DispatchDomainEventsAsync(this);

        await base.SaveChangesAsync(cancellationToken);

        return true;
    }
                    . . .
}

Microsoft injected IMeditor (an Application layer interface) into DbContext (an Infrastructure layer utility). They wanted to solve the following issues:

• make sure domain events are published before a transaction is submitted
• make the process transparent for developers

It works but at what cost:

• you are breaking a single responsibility principle coupling data storage and domain event handling logic together
• the infrastructure layer, should not contain any logic, not domain, neither application. It is just pure utility classes, that could be reused elsewhere in the code. Which is not the case for the code above. This is why they have two methods SaveChanges() and SaveEntities() and the developer is supposed to know in which circumstances they should be called
• the infrastructure layer knows and relies on the implementation details of the Application layer
• it can cause cyclic dependencies
• to instantiate DbContext without IMediator they had to add this dummy class. They also need to support and update it every time a new version of MediatR is released

class NoMediator : IMediator
{
    public IAsyncEnumerable<TResponse> CreateStream<TResponse>(IStreamRequest<TResponse> request, CancellationToken cancellationToken = default)
    {
        return default(IAsyncEnumerable<TResponse>);
    }

    public Task Publish(object notification, CancellationToken cancellationToken = default)
    {
        return Task.CompletedTask;
    }
          .  .  .
}

So, how do we achieve Microsoft’s needs without bringing more issues? Let’s see how other experts do it.

Jason Taylor in the Clean Architecture for .netcore3.1 did something similar.

public class ApplicationDbContext : IApplicationDbContext
{
    private readonly IDomainEventService _domainEventService;

    public ApplicationDbContext(
        DbContextOptions options,
        IDomainEventService domainEventService) 
        : base(options, operationalStoreOptions)
    {
        _domainEventService = domainEventService;
    }

    public override async Task<int> SaveChangesAsync(CancellationToken cancellationToken = new CancellationToken())
    {
        var result = await base.SaveChangesAsync(cancellationToken);

        await DispatchEvents();

        return result;
    }

    private async Task DispatchEvents()
    {
          var domainEvents = ChangeTracker
              .Entries<Entity>()
              .SelectMany(x => x.Entity.DomainEvents)

          await _domainEventService.Publish(domainEvents);
    }
                    .  .  .  
}

Domain events are still published in the DbContext however worth noticing he does not add a new method as Microsoft did, but overrides SaveChangesAsync(). It is such a simple approach, still not many people would know it is possible. Not a surprise. You should have either deeply investigated the framework or have a special mindset. Just two words. Bravo 👏 Jason.

Under the influence of Microsoft in the version for .net7.0 he also removes additional dependencies as IDomainEventService and injects IMediator directly in the DbContext. If I refer the Domain in the Azure function, where the Application layer is different and does not rely on MediatR, I have no way to raise domain events. What a pity! 😒

It gets more interesting in the current version of the repository where he gets use of EF’s newly introduced feature called interceptors. It allows to move cross-cutting concerns into a separate class which Jason did to clean up the SaveChanges() method:

public class DispatchDomainEventsInterceptor : SaveChangesInterceptor
{
    private readonly IMediator _mediator;

    public DispatchDomainEventsInterceptor(IMediator mediator)
    {
        _mediator = mediator;
    }

    public override InterceptionResult<int> SavingChanges(DbContextEventData eventData, InterceptionResult<int> result)
    {
        DispatchDomainEvents(eventData.Context).GetAwaiter().GetResult();

        return base.SavingChanges(eventData, result);
    }

    public override async ValueTask<InterceptionResult<int>> SavingChangesAsync(DbContextEventData eventData, InterceptionResult<int> result, CancellationToken cancellationToken = default)
    {
        await DispatchDomainEvents(eventData.Context);

        return await base.SavingChangesAsync(eventData, result, cancellationToken);
    }

    public async Task DispatchDomainEvents(DbContext? context)
    {
        if (context == null) return;

        var entities = context.ChangeTracker
            .Entries<BaseEntity>()
            .Where(e => e.Entity.DomainEvents.Any())
            .Select(e => e.Entity);

        var domainEvents = entities
            .SelectMany(e => e.DomainEvents)
            .ToList();

        entities.ToList().ForEach(e => e.ClearDomainEvents());

        foreach (var domainEvent in domainEvents)
            await _mediator.Publish(domainEvent);
    }
}

This is all interesting and everything, but despite it, we still have not managed to get off the ground.

The question is: how do we move logic from the SaveChanges() method to the Application layer and still keep it transparent and reliable? Simple! In the Application layer, we already have a class that is similar to SaveChanges() and responsible for transaction management, called TransactionBehaviour:

class TransactionBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse> where TRequest : IRequest<TResponse>
{
    private readonly OrderingContext _dbContext;

    public TransactionBehaviour(OrderingContext dbContext)
    {
        _dbContext = dbContext ?? throw new ArgumentException(nameof(OrderingContext));
    }

    public async Task<TResponse> Handle(
      TRequest request, 
      RequestHandlerDelegate<TResponse> next, 
      CancellationToken cancellationToken)
    {
        if (_dbContext.HasActiveTransaction)
        {
            return await next();
        }        
        
        await using var transaction = await _dbContext.BeginTransactionAsync();
        
        response = await next();

        // ideal place to dispatch domain events        
    
        await _dbContext.CommitTransactionAsync(transaction);
     
        return response;
    }
}

Look what we achieved:

• DbContext is clean and does not have additional responsibilities
• domain events are published before a transaction is submitted
• the process is transparent for developers
• the Infrastructure layer can be easily reused regardless context
• the Infrastructure layer independent of the Application layer
• no more issues with cyclic dependencies
• no need to support NoMediator class
• we can remove MediatR nuget from the Infrastructure layer (don’t let it sprout across the entire project)
• all domain event processing logic (both dispatching and handling) now centralized in a single Application layer

Integration events

Let’s continue with another kind of event. There are two types of events:

• domain events — are used to communicate changes and trigger reactions inside the application
• integration events — are used to communicate changes and trigger reactions outside the application

I know, I was a bit spicy before 🙃, but for the Integration events, Microsoft did a great job. I have some remarks, but those are more of nitpicking than anything valuable, so let’s just investigate and discuss the flows of integration events.

Producing integration events

When publishing integration events across different microservices it is important to keep data consistency. Therefore the following issues should be solved:

• event should not be published until data is committed to the database
• event should not be lost

It can be achieved with the Outbox pattern. The idea is simple here, store integration events in the database as part of the transaction that updates the business entities. A separate process then sends the messages to the message broker.

Let’s see how Microsoft implemented it. Everything starts in our use case:

public class ShipOrderCommandHandler : IRequestHandler<ShipOrderCommand>
{
    private readonly IOrderRepository _orderRepository;

    public ShipOrderCommandHandler(IOrderRepository orderRepository)
    {
        _orderRepository = orderRepository;
    }

    public async Task Handle(ShipOrderCommand command, CancellationToken cancellationToken)
    {
        var orderToUpdate = await _orderRepository.GetAsync(command.OrderNumber);

        orderToUpdate.SetShippedStatus();

        await _orderRepository.UnitOfWork.SaveEntitiesAsync(cancellationToken);
    }
}

Nothing special here. Gets the order, updates it, and saves it. When the order is updated, the domain event will be published. Later in the code, we can find such a handler for that event. It does almost nothing, creates and stores an integration event to publish it later.

public class OrderShippedDomainEventHandler : INotificationHandler<OrderShippedDomainEvent>
{
    private readonly IOrderRepository _orderRepository;
    private readonly IBuyerRepository _buyerRepository;
    private readonly IOrderingIntegrationEventService _orderingIntegrationEventService;

    public OrderShippedDomainEventHandler(
        IOrderRepository orderRepository,
        IBuyerRepository buyerRepository,
        IOrderingIntegrationEventService orderingIntegrationEventService)
    {
        _orderRepository = orderRepository;
        _buyerRepository = buyerRepository;
        _orderingIntegrationEventService = orderingIntegrationEventService;
    }

    public async Task Handle(OrderShippedDomainEvent domainEvent, CancellationToken cancellationToken)
    {
        var order = await _orderRepository.GetAsync(domainEvent.Order.Id);
        var buyer = await _buyerRepository.FindByIdAsync(order.GetBuyerId.Value.ToString());

        var integrationEvent = new OrderStatusChangedToShippedIntegrationEvent(order.Id, order.OrderStatus.Name, buyer.Name);
        await _orderingIntegrationEventService.SaveEventAsync(integrationEvent);
    }
}

It is strange to me that they would move this logic to the domain event handler instead of keeping it in the use case handler. To me use case handler should be responsible for all application logic (orchestrate domain with infrastructure, thread coordination, log management, integration events producing, etc) while a domain events handler should be triggered when changes to other aggregates are needed.

I would just publish integration events in the command handler. This way you are aware of all side effects, the code is more structural and explicit. You won’t wake up one morning realizing you are in the callback hell.

On the other side, this way Microsoft guarantees that integration events will be created even when a developer forgets to explicitly do it. Additionally, they don’t need to create a mapper from domain to integration events as those guys did.

You can play a bit with different approaches to see what is best for you. Try combining them. Create a single domain event handler for all domain events to encapsulate the logic of creating integration events in a single place.

But there is a more interesting thing. Have you noticed that integration events have not been published yet? They are just saved to be published later.

public class OrderingIntegrationEventService : IOrderingIntegrationEventService
{
    private readonly OrderingContext _orderingContext;

    public OrderingIntegrationEventService(
        OrderingContext orderingContext)
    {
        _orderingContext = orderingContext;
    }

    public async Task SaveEventAsync(IntegrationEvent evt)
    {
        var transaction = _orderingContext.GetCurrentTransaction()
        var eventLogEntry = new IntegrationEventLogEntry(evt, transaction.TransactionId);
        
        // in the current version of the repository 
        // they have a separate DbContext for integration events
        // we will keep stuff simple and store them in the same DbContext
        // it does not affect the flow
        // just adds extra infrastructure code
        _orderingContext.IntegrationEventLogs.Add(eventLogEntry);
        
        await _orderingContext.SaveChangesAsync();  
    }
                        .  .  .
}

This is the first part of the Outbox pattern. It ensures that either both the state change to the domain entity and the corresponding event are persisted, or none of them is. This is possible thankfully to the atomic nature of the transaction.

Now, when integration events are stored, you would be expected to see some background service peeking it up to publish. So would I. However, this is where Microsoft surprised me with their clever solution. They publish integration events after a transaction is committed in the TransactionBehaviour which we already have seen before:

class TransactionBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse> where TRequest : IRequest<TResponse>
{
                .  .  .

    public async Task<TResponse> Handle(
      TRequest request, 
      RequestHandlerDelegate<TResponse> next, 
      CancellationToken cancellationToken)
    {
        if (_dbContext.HasActiveTransaction)
        {
            return await next();
        }        
        
        await using var transaction = await _dbContext.BeginTransactionAsync();
        
        response = await next();

        await DispatchDomainEventsAsync(_mediator, _dbContext);     
    
        await _dbContext.CommitTransactionAsync(transaction);
        
        // publish integration events related to this transaction
        // no latency between saving and publishing events.
        await _orderingIntegrationEventService
            .PublishEventsThroughEventBusAsync(transaction.TransactionId);
     
        return response;
    }
                  .  .  .
}

The implementation lacks a retry mechanism. The serialization/deserialization could be done better. Other small improvements. However, I am ready to forgive them that 😌:

public class OrderingIntegrationEventService : IOrderingIntegrationEventService
{
    private readonly IEventBus _eventBus;
    private readonly OrderingContext _orderingContext;

    public OrderingIntegrationEventService(
        IEventBus eventBus,
        OrderingContext orderingContext)
    {
        _eventBus = eventBus;
        _orderingContext = orderingContext;
    }

    public async Task PublishEventsThroughEventBusAsync(Guid transactionId)
    {
        var pendingLogEvents = await orderingContext
            .IntegrationEventLogs
            .Where(e => e.TransactionId == tid)
            .Where(e => e.State == EventStateEnum.NotPublished)
            .ToListAsync();

        foreach (var logEvt in pendingLogEvents)
        {
            try
            {
                _eventBus.Publish(logEvt.IntegrationEvent);
                logEvt.Status = EventStateEnum.Published;
            }
            catch (Exception ex)
            {
                logEvt.Status = EventStateEnum.PublishedFailed;
            }
        }
         
        await _orderingContext.SaveChangesAsync();
    }
                      .  .  .  
}

To sum up:

• we can not publish integration events before changes are committed to the database. Transaction could be reverted while event could not (yes, you can have compensation events, but not every action could be compensated and it just tons of additional work). Publishing integration events should be the very last line of your code when you are aware that all previous code was executed without issues
• publishing integration events could also fail. For example, when a message broker goes down. This is why we had to implement the Outbox pattern
• OrderingIntegrationEventService stores events in the database so we don’t lose them when the application goes down. However, if your application does not require such a level of reliability, you can just store them in memory for performance gain

Just one last remark before going to the next section. You probably don’t need to implement the Outbox pattern from scratch. There is a high chance that a library that implements it already exists.

Consuming integration events

To complete the image we need to see how integration events that are published by other microservices are consumed.

There is nothing supernatural, just receive an event and create a command to handle it:

public class OrderPaymentSucceededIntegrationEventHandler
  : IIntegrationEventHandler<OrderPaymentSucceededIntegrationEvent>
{
    private readonly IMediator _mediator;

    public OrderPaymentSucceededIntegrationEventHandler(IMediator mediator)
    {
        _mediator = mediator;
    }

    public async Task Handle(OrderPaymentSucceededIntegrationEvent evt)
    {
        var command = new SetPaidOrderStatusCommand(evt.OrderId);

        await _mediator.Send(command);
    }
}

But wait. Why wouldn’t they just implement everything in this event handler? What is the point of creating a command and delegating the work to it?🤔 This is another example of Microsoft being professional.

Handing integration events is not a concern of the Infrastructure layer, but the Application layer. Similar to background jobs or controllers, integration event handlers are only here to trigger use cases. And you would not write your use case’s logic inside controllers, would you?

Idempotency

Oh, dear…

Just because they showcase it I can forget all their sins. I mean it.

Idempotency is such an important and at the same time completely forgotten topic that just by mentioning it you will be immediately identified as a senior dev.

Idempotency is a property of an operation, which guarantees that performing the operation multiple times will yield the same result as if it was executed only once.

Or in plain English, if you have an API endpoint to pay for an order and unpetient users click 10 times, only 1 payment will be charged.

As always, let’s see the full flow, and holy moly that implementation sucks big time.

First of all, they have added a wrapper class for the command. It also has an ID of the request to identify whether it was already processed or not.

public class IdentifiedCommand<T, R> : IRequest<R>
    where T : IRequest<R>
{
    public T Command { get; }
    public string Id { get; }

    public IdentifiedCommand(T command, string id)
    {
        Command = command;
        Id = id;
    }
}

As you can see the ID comes from headers, so a client has to manually add it. The original command is wrapped with this new command and published to the Mediator.

[Authorize]
[ApiController]
public class OrdersController : ControllerBase
{
                            . . .
    [HttpPut("cancel")]
    public async Task<IActionResult> CancelOrderAsync(
        [FromBody] CancelOrderCommand command,
        [FromHeader(Name = "x-requestid")] string requestId)
    {
        var requestCancelOrder = new IdentifiedCommand<CancelOrderCommand, bool>(command, requestId);

        await _mediator.Send(requestCancelOrder);

        return Ok();
    }
                             . . .
}

Every idempotent command will be handled by the generic handler. It will check if a command with such ID is already processed. If so, that command won’t be processed again, resulting in idempotent behavior. Otherwise, the command needs to be processed, so it stores the ID and resubmits that inner-wrapped command to the Mediator.

public class IdentifiedCommandHandler<T, R> : IRequestHandler<IdentifiedCommand<T, R>, R>
    where T : IRequest<R>
{
    private readonly IMediator _mediator;
    private readonly IRequestManager _requestManager;

    public IdentifiedCommandHandler(
        IMediator mediator,
        IRequestManager requestManager)
    {
        _mediator = mediator;
        _requestManager = requestManager;
    }

    public async Task<R> Handle(IdentifiedCommand<T, R> message, CancellationToken cancellationToken)
    {
        var alreadyExists = await _requestManager.ExistAsync(message.Id);

        if (alreadyExists)
        {
            return CreateResultForDuplicateRequest();
        }

        await _requestManager.CreateRequestForCommandAsync<T>(message.Id);

        // resubmit command
        return await _mediator.Send(command, cancellationToken);        
    }

    // Creates the result value to return if a previous request was found
    protected virtual R CreateResultForDuplicateRequest()
    {
        return default(R);
    }
}

That infrastructure code is performed by the RequestManager.

Just to note, it stores requests in the database, because microservice can scale horizontally. However, if this is not the case for you I would suggest using in-memory cache. It will not only improve performance but also automatically clear outdated data.

public class RequestManager : IRequestManager
{
    private readonly OrderingContext _context;

    public RequestManager(OrderingContext context)
    {
        _context = context ?? throw new ArgumentNullException(nameof(context));
    }

    public async Task<bool> ExistAsync(Guid id)
    {
        var request = await _context.FindAsync<ClientRequest>(id);

        return request != null;
    }

    public async Task CreateRequestForCommandAsync<T>(Guid id)
    {
        var exists = await ExistAsync(id);
        if (exists)
        {
            throw new OrderingDomainException($"Request with {id} already exists") :
        }

        var request =  new ClientRequest
        {
            Id = id,
        };

        _context.Add(request);

        await _context.SaveChangesAsync();
    }
}

You could also notice that the generic handler has CreateResultForDuplicateRequest() method. It is there, so you can override a response for duplicated requests in a specific command:

public class CancelOrderCommandHandler : IRequestHandler<CancelOrderCommand, bool>
{
    public async Task<bool> Handle(CancelOrderCommand command, CancellationToken cancellationToken)
    {
              .  .  .
    }
}

// use for Idempotency in Command process
public class CancelOrderIdentifiedCommandHandler : IdentifiedCommandHandler<CancelOrderCommand, bool>
{
    protected override bool CreateResultForDuplicateRequest()
    {
        return true;
    }
}

However, it is still a question to me why CancelOrderCommandHandler does not implement both IRequestHandler and IdentifiedCommandHandler🤔. At least logic would be encapsulated in a single place.

There are a few things that I found problematic about this implementation:

• seems like Microsoft has not decided at which layer idempotency should be implemented. It gets ID from the network (infrastructure layer), wraps it to a command (application layer), stores it in the database (infrastructure layer)
• nothing guides a developer which command is idempotent, which isn’t
• nothing prevents a developer from calling an idempotent command multiple times without IdentifiedCommand wrapper
• to call the idempotent command in the application layer you need IHttpContextAccessor, which violates the layer’s awareness
• CreateResultForDuplicateRequest() is hard to implement for complex object results

Having said that, let’s make idempotency great again.

Instead of using composition we will use inheritance. All idempotent commands will inherit the base class. There is a constructor, so a developer will have to explicitly set a CommandId.

abstract class IdempotentCommandBase
{
    public string CommandId { get; }

    protected IdempotentCommandBase(string commandId)
    {
        CommandId = commandId;
    }
}

Then, we won’t do any shinanigans with resubmitting commands and so on. MediatR gives us a common way to intercept command workflow, so let’s get use of it. For that, we just need to implement a new pipeline behavior:

class IdempotentBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse>
    where TRequest : IdempotentCommandBase, IRequest<TResponse>
{
    private readonly IMemoryCache _memoryCache;

    public IdempotentBehaviour(IMemoryCache memoryCache)
    {
        _memoryCache = memoryCache;
    }

    public Task<TResponse> Handle(
        TRequest request, 
        RequestHandlerDelegate<TResponse> next,
        CancellationToken cancellationToken)
    {
        var cacheKey = $"{request.GetType().Name}~{request.CommandId}";

        return _memoryCache.GetOrCreateAsync(cacheKey, cacheEntry =>
        {
            cacheEntry.AbsoluteExpirationRelativeToNow = TimeSpan.FromHours(1);

            return next();
        });
    }
}

That’s it!😃

You can use IRequestManager instead of IMemoryCache but at least place the interface in the application layer.

It is also important to implement idempotency when consuming integration events. Then you will be sure the same event won’t be handled twice.

Closing Remarks

While it might appear that my intention is solely to criticize eShopOnContainers, that’s not the case. I genuinely appreciate its brilliance in the vast landscape of open-source projects. I must acknowledge that their book stands out as one of the best I’ve ever read; it has the potential to elevate a mid-level developer to a senior one.

However, it’s essential to recognize that, despite Microsoft’s professionalism, they are just mortal humans capable of making mistakes. This isn’t to claim that my code is superior or inherently flawless. Instead, I aim to highlight the possibility of implementing things differently and emphasize that even professionally developed code can be subject to constructive criticism. 😉

💬 Let me know, if should I review Jason Taylor’s Clean Architecture. Are you interested in part 2 of this article? Do not be afraid to argue with me in the comments and propose better solutions. As always, there is always somebody who will point out your mistakes. Like, you can not just say “Good stuff bro. Keep going” 😒 Leave your opinion in the comments below
✅ Don’t forget to Subscribe
👏 Clap
☕️ Support
🙃 And be awesome