
TLDR
Move your ASP.NET Core MVC action method logic into lazily loaded commands using the command pattern.
When writing your Controllers in ASP.NET Core, you can end up with a very long class if you're not careful. You may have written several action methods with a few lines of code in each, you may be injecting a few services into your controller and you may have commented your action methods to support Swagger. The point is it's very easy to do, here is an example:
[Route("[controller]")]
public class RocketController : Controller
{
private readonly IPlanetRepository planetRepository;
private readonly IRocketRepository rocketRepository;
public RocketController(
IPlanetRepository planetRepository,
IRocketRepository rocketRepository)
{
this.planetRepository = planetRepository;
this.rocketRepository = rocketRepository;
}
[HttpGet("{rocketId}")]
public async Task<IActionResult> GetRocket(int rocketId)
{
var rocket = await this.rocketRepository.GetRocket(rocketId);
if (rocket == null)
{
return this.NotFound();
}
return this.Ok(rocket);
}
[HttpGet("{rocketId}/launch/{planetId}")]
public async Task<IActionResult> LaunchRocket(int rocketId, int planetId)
{
var rocket = await this.rocketRepository.GetRocket(rocketId);
if (rocket == null)
{
return this.NotFound();
}
var planet = await this.planetRepository.GetPlanet(planetId);
if (planet == null)
{
return this.NotFound();
}
this.rocketRepository.VisitPlanet(rocket, planet);
return this.Ok(rocket);
}
}
The Command Pattern
This is where the command pattern can come in handy. The command pattern moves logic from each action method and injected dependencies into their own class like so:
[Route("[controller]")]
public class RocketController : Controller
{
private readonly Lazy<IGetRocketCommand> getRocketCommand;
private readonly Lazy<ILaunchRocketCommand> launchRocketCommand;
public RocketController(
Lazy<IGetRocketCommand> getRocketCommand,
Lazy<ILaunchRocketCommand> launchRocketCommand)
{
this.getRocketCommand = getRocketCommand;
this.launchRocketCommand = launchRocketCommand;
}
[HttpGet("{rocketId}")]
public Task<IActionResult> GetRocket(int rocketId) =>
this.getRocketCommand.Value.ExecuteAsync(rocketId);
[HttpGet("{rocketId}/launch/{planetId}")]
public Task<IActionResult> LaunchRocket(int rocketId, int planetId) =>
this.launchRocketCommand.Value.ExecuteAsync(rocketId, planetId);
}
public interface IGetRocketCommand : IAsyncCommand<int>
{
}
public class GetRocketCommand : IGetRocketCommand
{
private readonly IRocketRepository rocketRepository;
public GetRocketCommand(IRocketRepository rocketRepository) =>
this.rocketRepository = rocketRepository;
public async Task<IActionResult> ExecuteAsync(int rocketId)
{
var rocket = await this.rocketRepository.GetRocket(rocketId);
if (rocket == null)
{
return new NotFoundResult();
}
return new OkObjectResult(rocket);
}
}
All the logic and dependencies in the controllers gets moved to the command which now has a single responsibility. The controller now has a different set of dependencies, it now lazily injects one command per action method.
You may have noticed the IAsyncCommand interface. I keep four of these handy to inherit from. They all outline an ExecuteAsync method to execute the command and return an IActionResult but they have a differing number of parameters. I personally feel if you are needing more than three parameters you should be using a class to represent your parameters, so I've put the limit on three parameters.
public interface IAsyncCommand
{
Task<IActionResult> ExecuteAsync();
}
public interface IAsyncCommand<T>
{
Task<IActionResult> ExecuteAsync(T parameter);
}
public interface IAsyncCommand<T1, T2>
{
Task<IActionResult> ExecuteAsync(T1 parameter1, T2 parameter2);
}
public interface IAsyncCommand<T1, T2, T3>
{
Task<IActionResult> ExecuteAsync(T1 parameter1, T2 parameter2, T3 parameter3);
}
Why so Lazy?
Why do we use Lazy<T>? Well the answer is that if we have multiple action methods on our controller, we don't want to instantiate the dependencies for every action method if we are only planning on using one action method. Registering our Lazy commands requires a bit of extra work in out Startup.cs. We can register lazy dependencies like so:
public void ConfigureServices(IServiceCollection services)
{
// ...Omitted
services
.AddScoped<IGetRocketCommand, GetRocketCommand>()
.AddScoped(x => new Lazy<IGetRocketCommand>(
() => x.GetRequiredService<IGetRocketCommand>()));
}
HttpContext and ActionContext
Now you might be thinking, how do I access the HttpContext or ActionContext if I want to set a HTTP header for example? Well, you can use the IHttpContextAccessor or IActionContextAccessor interfaces for this purpose. You can register them in your Startup class like so:
public void ConfigureServices(IServiceCollection services)
{
// ...Omitted
services
.AddSingleton<IHttpContextAccessor, HttpContextAccessor>()
.AddSingleton<IActionContextAccessor, ActionContextAccessor>();
}
Notice that they can be registered as singletons. You can then use them to get hold of the HttpContext or ActionContext objects for the current HTTP request. Here is a really simple example.
public class SetHttpHeaderCommand : ISetHttpHeaderCommand
{
private readonly IHttpContextAccessor httpContextAccessor;
public GetRocketCommand(IHttpContextAccessor httpContextAccessor) =>
this.httpContextAccessor = httpContextAccessor;
public async Task<IActionResult> ExecuteAsync()
{
this.httpContextAccessor.HttpContext.Response.Headers.Add("X-Rocket", "Saturn V");
return new OkResult();
}
}
Unit Testing
Another upside to the command pattern is that testing each command becomes super simple. You don't need to setup a controller with lots of dependencies that you don't care about. You only need to write test code for that single feature.
Conclusions
For a full working example, take a look at the .NET Boxed API project template which makes full use of the Lazy Command Pattern.
Comment
18 Comments
Command pattern is indeed awesome! Thank you for promoting it. As you may know, it was a staple in WPF development.
Quick commentary, however. ;) What used to be a simple:
... in WPF is now:
... in .NET Core. So, 38 characters "old" way vs. 62 characters "new" way. So much for progress. ;) As you are using an expression body for the constructor I am sure you can appreciate the sentiment that less code is better.
https://visualstudio.uservoice.com/forums/121579-visual-studio-ide/suggestions/9126493-improve-asynchronous-programming-model
Additionally, this design muddies the water with command and query separation, essentially forcing you to enable every "query" method (anything that returns a result) into a command method, which implicitly changes state and has side effects (which query methods are not supposed to have).
Food for thought. :)
Boo, the blog formatted out the brackets. Let's see if JSON-based formatting works (sigh):
It's actually 37 "old" vs 62 "new."
I totally understand the SRP and the IOC Concept behind this, but whats then the purpose of the Controller? Besides declaring The Routes? I mean in a more reallife Example?
I think of a controller as a kind of specialized interface for HTTP or maybe a better way to describe it would be a contract. It has all your routes, caching and authorization attributes etc. It defines the public contract for how you want to do HTTP.
In an imaginary world where each action method was turned into an action command class, it'd be quite difficult to get a quick overview of all of your HTTP endpoints, although such an architecture might be cool.
Nice idea, better sample with asp identity would be nice
Agreed. ASP.NET Identity is a mess and a prime example of code that could use the command pattern. Not enough time in my day to show that though, so that's an exercise for the reader.
I use Jimmy Bogard's MediatR project to accomplish a similar purpose where controllers simply execute commands and queries. I love that pattern!
I have used this pattern as well to keep the API controller lean. I decided to move the "protocol" out of the command and into the controller action. So the command returns an object and the controller action returns the wire result
NotFoundResult/OkObjectResult.The question that keeps spinning in my head: why not create
ApiControllerswith a single method? So instead ofRocketControllerhaveRocketGetController,RocketLaunchController. That way the controller turns into a command/action.I do not totally agree. If you organize the commands in folder or namespaces and use names related to HTTP methods, it could be better and simpler. It is almost like a class for each action.
We could create controllers that implement command interface and we could avoid all these ugly Lazy fields.
Controllers with a single action method are interesting and certainly a possible direction. I personally feel that an action method is doing two things:
These two things go against the single responsibility principle. When you are working on building a reponse, you don't care about the request interface (routes, auth etc.), thats all noise. A controller per action method would also make it more difficult to see all your related routes in one place. Using the command pattern, the controller just becomes a specialized interface for HTTP.
See my answer to Theo Albers.
I agree with Theo that I cannot see how this approach offers any benefit over 'one action controllers' and in fact it just adds complexity. You mentioned that the controller defies the interface for the request but this is not strictly true. The route is only part of the interface. The request and response types are also a significant part of the contract and these are defined in the command. So now we have the command being responsible for some of 1) and all of 2). Just my two cents.
Looks interesting however very verbose and overcomplicated.
Solutions with MediatR look much more concise and elegant.
And in my opinion your current implementation look like moving controller specific stuff to business layer.
MediatR is interesting but it's another dependency, and much slower than this implementation because it uses
Activator.CreateInstance(). This implementation is much simpler (a couple of interfaces used by the DI container, has zero dependencies and is as fast as it gets.I debated with myself whether the controller should return a business object or an
IActionResult. I actually started with the former but settled on the latter. It makes things much simpler, it's all C# POCO's which makes unit testing a breeze.Hello,
How did you do it? Can you please post sample code?
While abstracting away common patterns is of course good, this overlooks the basic facilities the language already provides to accomplish this.
The
IAsyncCommandinterfaces you have defined introduce unnecessary complexity and represent a barrier to simple composition. In particular, the whole pattern reeks of Java and the complexity and ugliness inherent in its abhorrent SAM types.This is C#, we don't need nonsensical, SAM types, we have delegates.
Instead of your lazy stuff use FromServicesAttribute for each action/command
@TarasKuz Agreed, soon after I wrote this, I switched to using
[FromServices].