Dependency Injection composition root and decorato

Question

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.

Answer 1

Preamble

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.

Unity

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.

Answer 2

Building upon Mark's second answer I'd look to registering the CustomerService with a InjectionFactory and only register it with the service type without it's interface like:

containter.RegisterType<CustomerService>(new InjectionFactory(
    container => new CustomerService(containter.Resolve<IGenericRepository<Customer>>())));

This would then allow, as in Mark's answer, for you to register the logging object like:

containter.RegisterType<ICutomerService, CutomerServiceLoggingDecorator>(new InjectionConstructor(
    new ResolvedParameter<CustomerService>()));

This is basically the same technique that I use whenever I require something to be lazily loaded as I don't want my objects to depend upon Lazy<IService> and by wrapping them in proxy allows me to only inject IService but have it resolved lazily through the proxy.

This will also allow you to pick and choose where either the logging object or the normal object is injected instead of requiring magic strings by simply resolving a CustomerService for your object instead of the ICustomerService.

For a logging CustomerService:

container.Resolve<ICustomerService>()

Or for a non-logging CustomerService:

container.Resolve<CustomerService>()

Answer 3

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?

As was already pointed out the best way to do this is with the following construct.

container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>(
    new InjectionConstructor(new ResolvedParameter<CustomerService>()));

This allows you to specify how the parameters are resolved by type. You could also do it by name but by type is a cleaner implementation and allows for better checking during compile time as a change or mistype in a string will not be caught. Note that the only minute difference between this code part and the code offered by Mark Seemann is a correction in the spelling of InjectionConstructor. I will not elaborate on this part any more as there is nothing else to add that Mark Seemann has not already explained.

---

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?

You can do this using the way specified above using the Fluent notation OR using named dependency with a dependency override.

Fluent

This registers the controller with the container and specifies an overrload for that type in the constructor. I prefer this approach over the second but it just depends on where you want to specify the type.

container.RegisterType<MyController2>(
    new InjectionConstructor(new ResolvedParameter<CustomerService>()));

Named dependency

You do this the exact same way, you register both of them like so.

container.RegisterType<ICustomerService, CustomerService>("plainService");

container.RegisterType<ICustomerService, CustomerServiceLoggingDecorator>(
    new InjectionConstructor(new ResolvedParameter<CustomerService>()));

The difference here is that you use a named dependency instead for the other types that can be resolved using the same interface. This is because the interface needs to be resolved to exactly one concrete type every time a resolve is done by Unity so you can not have multiple unnamed registered types that are registered to the same interface. Now you can specify an override in your controller constructor using an attribute. My example is for a controller named MyController2 and I added the Dependency attribute with the name also specified above in the registration. So for this constructor a CustomerService type will be injected instead of the default CustomerServiceLoggingDecorator type. MyController1 will still use the default unnamed registration for ICustomerService which is type CustomerServiceLoggingDecorator.

public MyController2([Dependency("plainService")]ICustomerService service) 

public MyController1(ICustomerService service) 

There are also ways to do this when you manually resolve the type on the container itself, see Resolving Objects by Using Overrides. The problem here is that you need access to the container itself to do this which is not recommended. As an alternative you could create a wrapper around the container that you then inject into the Controller (or other type) and then retrieve a type that way with overrides. Again, this gets a bit messy and I would avoid it if possible.