Why I have overused the CQRS pattern

In this post, I want to share how I might have overused the Command Query Responsibility Segregation (CQRS) pattern when developing .NET applications that follow clean architecture.

In a typical layered application, there is a dedicated layer for data access. Often, a common approach is to use the repository pattern to encapsulate the complexity of retrieving data from a database or another data source, and then a service layer which handles business logic and calls out to the repository layer to retrieve data. For example, I worked on an AI-enabled chatbot solution which allows users to configure and build chatbots without writing code. In the database, we have tables like Chatbots, ChatbotKnowledgeBases, KnowledgeBases, Users, etc. In the application, we have corresponding repositories like ChatbotsRepository, KnowledgeBaseRepository, UserRepository, etc. The app allows users to upload documents to a knowledge base and associate chatbots with knowledge bases. The user can chat with the bots about the knowledge bases which the bots have access to. In the ChatbotRepository, we have CRUD (Create, Read, Update, Delete) methods which interact directly with the Chatbots table. In addition, we also have methods to retrieve knowledge bases of a chatbot and associate knowledge bases to a chatbot. When I was working on the application, I thought using the repository pattern had a few drawbacks. One of which is that it’s not clear which repository class to put methods that span across multiple tables. For instance, to associate a knowledge base to a chatbot, should I create a method in the ChatbotRepository or the KnowledgeBaseRepository class? I could not decide which class is a better fit at the time, as both seemed to work fine for me. However, I like a design where everything is clear and consistent, and the fact that not having a clear way to put the method annoyed me. Another issue I had with the repository pattern was that it seems to not align with the Single Responsibility Principle (SRP), which states a class or method should have a single responsibility. At a time, I thought that the ChatbotRepository class should not have to concern with other tables to stay compliant with the SRP. Because of the concerns I had, I ended up refactoring the code to use CQRS and represented each of the methods as either a command or a query with a corresponding handler. I used the MediatR library to set up the queries, commands, and handlers.

As I read and understand more about the pattern, CQRS is suitable to use when the domain is complex, or for separating read and write operations to improve performance. Using CQRS definitely helps to eliminate confusion or ambiguity of not knowing which repositories to place methods that work with data across multiple tables, since we can encapsulate each method as a command or a query with a corresponding handler. Additionally, by separating query and command operations, we have a clear and maintainable pattern to have the read and write operations hit different databases to improve performance. For instance, we can have the read operations go to a replica while the write operations hit the main database. Finally, using CQRS definitely helps to make the codebase align with the SRP, effectively making it easier to maintain and scale.

However, it is easy to overuse the CQRS pattern, and you may end up making the codebase more complex than necessary if you do so. In my case, I liked how CQRS helped to keep the codebase in alignment with the SRP so much that I used it for other use cases beyond data access. For example, in the Chatbot AI project, I used CQRS for work that just calls out to a downstream API and does not contain database operations, as shown in the snippet below.

  public async Task<ChatResponseV2Dto?> Handle(SendChatCommandV2 request, CancellationToken cancellationToken)
  {
      try
      {
          _logger.LogDebug("Sending chat request to bot ID {ChatBotId} for conversation ID {ChatConversationId}", request.ChatBotId, request.ConversationId);
          
          // Call API to send chat request
          // Code omitted for brevity 
          
          _logger.LogDebug("Successfully processed chat request.");
          return _entityConversionService.Convert<AiSvcV2ChatResponseDto, ChatResponseV2Dto>(chatResponseContainer);
      }
      catch (Exception ex)
      {
          _logger.LogError(ex, "Exception occurred while sending chat request.");
          return new ChatResponseV2Dto()
          {
              CompleteMessage = "Sorry, an error occurred. Please try again later.",
          };
      }
  }1

In retrospect, I believe using CQRS for this scenario—when the app just calls the downstream API—was a little bit overkill.

A drawback of using CQRS and the MediatR library for this use case is that the command interface needs to be in the infrastructure layer, or the Core layer needs to have a dependency on the MediatR library, which breaks the Clean Architecture pattern. Either way, when I overuse the CQRS pattern and represent all kinds of work as queries/commands and handlers, I often end up with all or most of the business logic in the infrastructure layer, and my Core layer typically only has interfaces. However, ideally, you want the Core layer to contain the business logic, and the infrastructure layer to encapsulate just the parts that need to interact with external services or data sources.

Another scenario where using CQRS can be overkill or not appropriate is when you want to perform multiple operations as part of a transaction. In the code snippet below, I used multiple commands and handlers to implement a workflow that includes translating documents and updating multiple tables in the database.

  public async Task<Uri> Translate(IBrowserFile document, string fromLanguage, string toLanguage, int userId)
  {
// invoke command to persist request into database. 
   var persistRequestCommand = new PersistDocumentTranslationRequestCommand()
   {
       DateOfRequest = (DateTime)request.DateOfRequest!,
       CaseTypeId = (int)request.CaseType!,
       RequestingParty = request.RequestingParty!,
       TypeOfDocument = request.TypeOfDocument!,
       DueDate = (DateTime)request.DueDate!,
       NumberOfPagesId = (int)request.NumberOfPages!,
       JusticeCenterId = (int)request.JusticeCenter!,
       TargetAudience = request.TargetAudience!,
       CreatedBy = userId
   };
   var persistRequestEntityId = await _mediator.Send(persistRequestCommand);

// boilerplate code for translating document using Azure Translator services. 
// code omitted for brevity 
// ... 
// invoke command to persist the translation result to DB. 
      if (toReturn != null)
      {
          var command = new PersistDocumentTranslationCommand()
          {
              SourceDocumentName = sourceFileName,
              SourceDocumentType = inputFileExtension,
              SourceDocumentUrl = finalInputPath,
              SourceDocumentContainerName = containerName,
              SourceDocumentFolderPath = blobFolderInputPath,
              SourceDocumentBlobName = sourceFileName,
              SourceDocumentBlobPath = containerName + blobFolderInputPath + "/" + sourceFileName,
              SourceLanguageCode = fromLanguage,
              TargetDocumentName = targetFileName,
              TargetDocumentType = inputFileExtension,
              TargetDocumentUrl = finalOutputPath,
              TargetDocumentContainerName = containerName,
              TargetDocumentFolderPath = blobFolderOutputPath,
              TargetDocumentBlobName = targetFileName,
              TargetDocumentBlobPath = containerName + blobFolderOutputPath + "/" + targetFileName,
              TargetLanguageCode = toLanguage,
              UserId = userId
          };
          await _mediator.Send(command);

          return toReturn;
      } else
      {
          _logger.LogError("Error translating document.  No further details available.");
          throw new Exception("Error translating document.");
      }   
  }

In the above snippet, the Translate method first stores the request into the database, does the translation, and then stores the result into the database. If the translation fails and throws an exception after the request is persisted, then the translation result might not be saved into the database, and I might end up with missing data. Ideally, if the translation fails, I do not want to persist anything to the database. The code issues commands via CQRS for the database operations. The problem is that the logic in the handlers persists the data when invoked, outside of a transaction. I would need to modify the handlers or add additional logic to roll back the changes if one of the operations fails. Clearly, using CQRS in this case is not the best approach. Instead, a better approach is to implement the Unit of Work pattern, grouping the operations as part of a single unit or transaction, with the ability to cancel or roll back if one of the operations fails. In fact, I have since refactored the code to follow the Unit of Work pattern, as shown in the snippet below.

  public async Task<DocumentTranslationResultDto> TranslateDocumentAsync(DocumentTranslationRequestDto requestDto, CancellationToken? cancellationToken)
  {
      ValidateTranslationRequest(requestDto);
      
      var documentTranslationRequest = entityConversionService.Convert<DocumentTranslationRequestDto, DocumentTranslationRequestT>(requestDto);

      try
      {
          var translationRequest = await documentTranslationUnitOfWork.Add(documentTranslationRequest);

          var translator = documentTranslatorFactory.GetTranslator(requestDto.TranslationEngine);
          var translationResultDto = await translator.Translate(requestDto);

          ValidateTranslationResult(translationResultDto);

          await PersistDocumentTranslation(
              translationResultDto!.InputMetadata!,
              translationResultDto.OutputMetadata!,
              translationRequest);

          // Ensure that changes are saved to the database
          await documentTranslationUnitOfWork.SaveChangesAsync(cancellationToken ?? CancellationToken.None);

          return translationResultDto;
      }
      catch (Exception e)
      {
          if (e is TranslationFailedException)
          {
              throw; // Re-throw known translation exceptions
          }
          throw new TranslationFailedException("An error occurred during document translation.", e);
      }
      finally
      {
          await documentTranslationUnitOfWork.DisposeAsync();
      }
  }

A nice effect of the refactoring is that the high-level business logic just relies on plain interfaces, without the MediatR library, and so I am able to place the class in the Core layer, which is where I feel it should belong. The implementation of the Unit of Work is in Infrastructure since it mostly deals with data in the database, which is appropriate.

In retrospect, the CQRS pattern is one of the great patterns to understand and utilize. However, it’s not always appropriate or the best choice. If you don’t fully understand the pattern, you may end up making the codebase more complex than necessary. As for the warning sign—if you find that most of the code in the Core layer is just interfaces with little business logic, or if you need to depend on the MediatR library in Core—it may be a sign that you are overapplying CQRS.