I'm getting StackoverflowException
's in my implementation of the decorator pattern when using dependency injection. I think it is because I'm "missing" something from my understanding of DI/IoC.
For example, I currently have CustomerService
and CustomerServiceLoggingDecorator
. Both classes implement ICustomerService
, and all the decorator class does is use an injected ICustomerService
but adds some simple NLog logging so that I can use logging without affecting the code in CustomerService
while also not breaking the single responsibility principle.
However the problem here is that because CustomerServiceLoggingDecorator
implements ICustomerService
, and it also needs an implementation of ICustomerService
injected into it to work, Unity will keep trying to resolve it back to itself which causes an infinite loop until it overflows the stack.
These are my services:
public interface ICustomerService
{
IEnumerable<Customer> GetAllCustomers();
}
public class CustomerService : ICustomerService
{
private readonly IGenericRepository<Customer> _customerRepository;
public CustomerService(IGenericRepository<Customer> customerRepository)
{
if (customerRepository == null)
{
throw new ArgumentNullException(nameof(customerRepository));
}
_customerRepository = customerRepository;
}
public IEnumerable<Customer> GetAllCustomers()
{
return _customerRepository.SelectAll();
}
}
public class CustomerServiceLoggingDecorator : ICustomerService
{
private readonly ICustomerService _customerService;
private readonly ILogger _log = LogManager.GetCurrentClassLogger();
public CustomerServiceLoggingDecorator(ICustomerService customerService)
{
_customerService = customerService;
}
public IEnumerable<Customer> GetAllCustomers()
{
var stopwatch = Stopwatch.StartNew();
var result = _customerService.GetAllCustomers();
stopwatch.Stop();
_log.Trace("Querying for all customers took: {0}ms", stopwatch.Elapsed.TotalMilliseconds);
return result;
}
}
I currently have the registrations setup like this (This stub method was created by Unity.Mvc
):
public static void RegisterTypes(IUnityContainer container)
{
// NOTE: To load from web.config uncomment the line below. Make sure to add a Microsoft.Practices.Unity.Configuration to the using statements.
// container.LoadConfiguration();
// TODO: Register your types here
// container.RegisterType<IProductRepository, ProductRepository>();
// Register the database context
container.RegisterType<DbContext, CustomerDbContext>();
// Register the repositories
container.RegisterType<IGenericRepository<Customer>, GenericRepository<Customer>>();
// Register the services
// Register logging decorators
// This way "works"*
container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>(
new InjectionConstructor(
new CustomerService(
new GenericRepository<Customer>(
new CustomerDbContext()))));
// This way seems more natural for DI but overflows the stack
container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>();
}
So now I'm not sure of the "proper" way of actually creating a decorator with dependency injection. I based my decorator on Mark Seemann's answer here. In his example, he is newing up several objects that get passed into the class. This is how my it "works"* snippet works. However, I think I have missed a fundamental step.
Why manually create new objects like this? Doesn't this negate the point of having the container doing the resolving for me? Or should I instead do contain.Resolve()
(service locator) within this one method, to get all the dependencies injected still?
I'm slightly familiar with the "composition root" concept, which is where you are supposed to wire up these dependencies in one and only one place that then cascades down to the lower levels of the application. So is the Unity.Mvc
generated RegisterTypes()
the composition root of an ASP.NET MVC application? If so is it actually correct to be directly newing up objects here?
I was under the impression that generally with Unity you need to create the composition root yourself, however, Unity.Mvc
is an exception to this in that it creates it's own composition root because it seems to be able to inject dependencies into controllers that have an interface such as ICustomerService
in the constructor without me writing code to make it do that.
Question: I believe I'm missing a key piece of information, which is leading me to StackoverflowExceptions
due to circular dependencies. How do I correctly implement my decorator class while still following dependency injection/inversion of control principles and conventions?
Second question: What about if I decided I only wanted to apply the logging decorator in certain circumstances? So if I had MyController1
that I wished to have a CustomerServiceLoggingDecorator
dependency, but MyController2
only needs a normal CustomerService
, how do I create two separate registrations? Because if I do:
container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>();
container.RegisterType<ICustomerService, CustomerService>();
Then one will be overwritten meaning that both controllers will either both have a decorator injected or a normal service injected. How do I allow for both?
Edit: This is not a duplicate question because I am having problems with circular dependencies and a lack of understanding of the correct DI approach for this. My question applies to a whole concept not just the decorator pattern like the linked question.
Whenever you are having trouble with a DI Container (Unity or otherwise), ask yourself this: is using a DI Container worth the effort?
In most cases, the answer ought to be no. Use Pure DI instead. All your answers are trivial to answer with Pure DI.
If you must use Unity, perhaps the following will be of help. I haven't used Unity since 2011, so things may have changed since then, but looking up the issue in section 14.3.3 in my book, something like this might do the trick:
container.RegisterType<ICustomerService, CustomerService>("custSvc");
container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>(
new InjectionConstructor(
new ResolvedParameter<ICustomerService>("custSvc")));
Alternatively, you may also be able to do this:
container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>(
new InjectionConstructor(
new ResolvedParameter<CustomerService>()));
This alternative is easier to maintain because it does not rely on named services, but has the (potential) disadvantage that you can't resolve CustomerService
through the ICustomerService
interface. You probably shouldn't be doing that anyway, so it ought not to be an issue, so this is probably a better alternative.