When I am developing MVC applications I see that there are some common behavior for certain types of actions (eg. list view, edit view) which means similar code. A common solution to that problem is to create a base controller with that behavior. And that’s also how I tried to solve the problem first. I created base controllers for each view type and their related actions. But it created new problems.
I ended up with a multi-level inheritance tree which made it difficult to track down things. It wasn’t possible to look at the controller and simply say what it is doing. Some of the actions were defined at the upper level that required implementations at the lower level and so on, which created a lot of jumping between classes.
And then there was a common inheritance problem – I needed some behavior from base controller but not all, so I had to implement methods I really didn’t need. Or I needed behavior from multiple base controllers.
Third, it created a big constructors in controllers with all different dependencies required by base controller classes.
I thought about using approach called controllerless actions, but after a little research I found it is too complicated with a given structure. Luckily I can create custom action results that can encapsulate all the logic and data for returning a result for given action. But then I found this post which described separating WHAT of an action result from the HOW. By that I mean that my custom action result contains all the required information to execute an action, but the execution logic is in different object – in an action result invoker I created.
By using this approach I was able to separate all that execution logic from base controllers to separate action invoker. And controller was only responsible for returning a proper action method result with all relevant information based on the input. No repetitive execution logic anymore. For that, I created a base ActionMethodResult which is so called marker base class for my custom action method results.
public abstract class ActionMethodResult : ActionResult
{
public override void ExecuteResult(ControllerContext context)
{
throw new InvalidOperationException("Action should be executed through action invoker");
}
}
A one simple action method result implementation looks like this. It contains a task to execute and a result to return when task execution was successful and other one for failure cases.
public class TaskActionResult<TEntity> : ActionMethodResult
where TEntity : IEntity<int>
{
private readonly Func<ITask<TEntity>> _task;
private readonly Func<TaskResult<TEntity>, ActionResult> _successContinuation;
private readonly Func<TaskResult<TEntity>, ActionResult> _failureContinuation;
public TaskActionResult(Func<ITask<TEntity>> task, Func<TaskResult<TEntity>, ActionResult> successContinuation, Func<TaskResult<TEntity>, ActionResult> failureContinuation)
{
_task = task;
_successContinuation = successContinuation;
_failureContinuation = failureContinuation;
}
public Func<ITask<TEntity>> Task
{
get { return _task; }
}
public Func<TaskResult<TEntity>, ActionResult> SuccessContinuation
{
get { return _successContinuation; }
}
public Func<TaskResult<TEntity>, ActionResult> FailureContinuation
{
get { return _failureContinuation; }
}
}
But it’s not the case with all action results. Some of them just take some inputs and invoker decides how to respond to that. The main point is the same – invoker generates some response based on the given action method result in the context of current request. Action invoker for TaskActionResult is like this.
public class TaskActionResultInvoker<TEntity> : ITaskActionMethodResultInvoker<TEntity>
where TEntity : IEntity<int>
{
public ActionResult Invoke(TaskActionResult<TEntity> actionResult, ControllerContext controllerContext)
{
if (!controllerContext.Controller.ViewData.ModelState.IsValid)
{
return actionResult.FailureContinuation(null);
}
var result = actionResult.Task().Run();
if (!result.ValidationResults.IsValid)
{
result.ValidationResults.AddToModelState(controllerContext.Controller.ViewData.ModelState);
return actionResult.FailureContinuation(result);
}
return actionResult.SuccessContinuation(result);
}
}
Then I created a custom IActionInvoker where I overrode the CreateActionResult method to create a proper ActionResult based on my custom action method result. Each of my action invoker held it’s own dependencies so I needed to create a façade for that which got my action invoker as a constructor argument so it could be resolved by my container. The creation of the facades and action invokers is a bit tedious, but it was a compromise I was willing ta make.
public class InjectingActionInvoker : ControllerActionInvoker, IActionMethodResultCreator
{
private readonly IServiceLocator _serviceLocator;
public InjectingActionInvoker(IServiceLocator serviceLocator)
{
_serviceLocator = serviceLocator;
}
protected override ActionResult CreateActionResult(ControllerContext controllerContext, ActionDescriptor actionDescriptor, object actionReturnValue)
{
if (IsActionMethodResult(actionReturnValue))
{
var result = CreateActionMethodResult(actionReturnValue, controllerContext);
return IsActionMethodResult(result)
? CreateActionResult(controllerContext, actionDescriptor, result)
: result;
}
return base.CreateActionResult(controllerContext, actionDescriptor, actionReturnValue);
}
public ActionResult CreateActionMethodResult(object actionReturnValue, ControllerContext controllerContext)
{
var wrappedResultType = CreateActionInvokerFacadeType(actionReturnValue);
var invokerFacade = (IActionMethodResultInvoker)_serviceLocator.Resolve(wrappedResultType);
return invokerFacade.Invoke(actionReturnValue, controllerContext);
}
private static Type CreateActionInvokerFacadeType(object actionReturnValue)
{
var actionMethodResultType = actionReturnValue.GetType();
Type openWrappedType;
Type wrappedResultType;
if (actionMethodResultType.IsGenericImplementation(typeof(TaskActionResult<>)))
{
openWrappedType = typeof(TaskActionResultInvokerInvokerFacade<>);
wrappedResultType = openWrappedType.MakeGenericType(actionMethodResultType.GetGenericArguments()[0]);
}
// code for other types of action results removed for brevity
else
{
openWrappedType = typeof(ActionMethodResultInvokerFacade<>);
wrappedResultType = openWrappedType.MakeGenericType(actionMethodResultType);
}
return wrappedResultType;
}
}
Note the recursive call to the CreateActionResult method. It is because my action method results my return other action method results for some execution paths, which in turn need to be invoked with their own type of invokers. So invokers are executed so long as result is “regular” ActionResult which can be executed using ASP.NET MVC built-in behavior.
Finally, when all hooked up, my controller action looks like this. Simple, clean and without inheritance.
[HttpPost]
public ActionResult Edit(int id, ViewModel item)
{
item.Id = id;
return new TaskActionResult<Entity>(
() => _taskFactory.ForSave(() => CreateEntityFromViewModel(item)),
success => SaveSuccessResult(success, "Task executed!"),
SaveFailureResult);
}
Now I’m able to create much more simple controllers in the maintenance point of view, they are better to follow. It also improved testability. Because in my opinion, inheritance is pain in the butt regarding testing. This approach allowed me to easily test the result of the controller action (which is basically a plain data) separately from the execution of that result. Which I can do in an isolation compared to the approach when I was using controller inheritance which also resulted in some sort of tests inheritance.
