Autofac: Partially decorating service implementing multiple interfaces throws

Hooooo boy.

I figured out why this isn’t working but I’m not sure how to fix it without some non-trivial internal changes. Granted, some of those changes may be well worthwhile to do, but it could involve breaking interfaces.

The short version: By the time the decorator is getting chosen, we know which registration is being resolved, but not which service on that registration. That means a single registration exposing multiple services will try to decorate all of them.

Longer explanation with details (in which I won’t blame you if you fall asleep 😴 ):

When you resolve a component from an IComponentContext (like a container or a lifetime scope) two things happen:

1. The service (e.g., the interface type you want to resolve) gets looked up in the component registry. That should get a registration which is basically the mapping of underlying concrete type (MyServicImpl) to the set of interfaces you registered (IService, IOtherService).
2. The component context is told to resolve that registration. The information, at that point, about the exact service you’re resolving is dropped.

Later, deep in that chain of resolution, is basically where Autofac constructs (activates) all the concrete objects and handles decoration.

Again, by the time we get way, way down into the stack there, we’ve long since lost the information about exactly which service (interface) the caller was trying to resolve so the approach is just to decorate whatever comes out of the whole registration. If that registration has multiple services exposed but only one of them is decorated, you’ll get the exception because the decorator is trying to decorate anything coming out there, not just the one service.

Hmmm.

I think the solution here is to actually create a sort of ResolveContext object that gets passed around during resolution that has all the information we need, sort of like how HttpContext has all the request/response info during a web request.

The challenge with switching to that is it’ll affect a lot of public interfaces. For example, IComponentContext would change from

public interface IComponentContext
{
  IComponentRegistry ComponentRegistry { get; }
  object ResolveComponent(IComponentRegistration registration, IEnumerable<Parameter> parameters);
}

to

public interface IComponentContext
{
  IComponentRegistry ComponentRegistry { get; }
  object ResolveComponent(ResolveContext context);
}

That’s a pretty big breaking change. It’d affect resolution extension methods and likely some integration packages that we both do and don’t own. IComponentContext is one of the most prominent interfaces we have. (ILifetimeScope and IContainer both inherit IComponentContext and any resolution operations run through that ResolveComponent method… so this would break ILifetimeScope and IContainer, too.)

It may also affect memory usage and perf, but we’d have to benchmark it to see.

I think that’d be a positive change because it would allow us to add data to the resolve pipeline that we didn’t previously have; and adding/changing properties in that context object would be possible without breaking interfaces. I think there are quite a few things we could make better if we had all the information at all levels of the operation.

But… it’s not a five minute fix, so I won’t be able to drop a quick 0.0.1 bug fix on this one. It’s bigger than that.

Again, the workaround appears to be separating the registrations:

builder.RegisterType<Service>().As<IDecorable>();
builder.RegisterType<Service>().As<IService>();
builder.RegisterDecorator<Decorator, IDecorable>();

Which sucks, and isn’t as pretty, and it makes some things harder, and I’m truly sorry about that. I think this would be a great thing to target for a 5.0 release where we can do some breaking changes.

For the singleton case, I tend to disagree with tillig’s reasoning. I feel it would be natural as such :

var builder = new ContainerBuilder();
builder.RegisterType<Component>()
       .As<IService1>()
       .As<IService2>()
       .SingleInstance();
builder.RegisterDecorator<Decorator, IService1>();
var container = builder.Build();
var a = container.Resolve<IService1>();
var b = container.Resolve<IService2>();

// This should still pass! It's a singleton!
Assert.Same(a.**Inner**, b);

// What type T in here would make this true?
Assert.IsInstanceOf<**Decorator**>(a);
Assert.IsInstanceOf<**Component**>(b);

In essence, resolving IService1 would generate Decorator decorating Component resolving IService2 would generate Component

Both Component would be the same single instance, but Decorator would only wrap Component when resolving IService1.

This has been fixed in 5.1.0 which is now on NuGet.

You are right about registration order playing a role in all this @jonathansapp.

If the service order is swapped around it changes the behaviour. The example below no longer throws an exception because the decorator is not applied. Resolving IService will return Service, and resolving IDecorable will also return Service.

var builder = new ContainerBuilder();
builder.RegisterType<Service>().As<IService>().As<IDecorable>();
builder.RegisterDecorator<Decorator, IDecorable>();
var container = builder.Build();

var foo = container.Resolve<IService>();
var bar = container.Resolve<IDecorable>();

It seems that not all services are considered when it comes to looking for decorators and that is indeed a bug. I found the problem and with the fix in place the order does not matter. You will receive the InvalidCastException when resolving IService regardless of the order. I’ll get that issue sorted out first so that it is easier to reason about what is going on.

I slept on this and realized something at like 3AM when all the important thoughts hit: This may not be fixable at all.

It has to do with component lifetimes.

Just for future readers of this chain…

• A component is the underlying implementation of the thing. In a RegisterType<T>, this is the T.
• A service is the interface the component is exposing. When you register As<X>, this is the X.
• A component can expose one or more services.
• The lifetime is associated with the component, not the service.

In all the examples we’ve talked about so far, every component registration has been instance-per-dependency: Ask for an instance, get a new one.

var builder = new ContainerBuilder();
builder.RegisterType<Component>();
var container = builder.Build();
var a = container.Resolve<Component>();
var b = container.Resolve<Component>();
Assert.NotSame(a, b);

In that case, it makes total sense that something like this should work:

var builder = new ContainerBuilder();
builder.RegisterType<Component>()
       .As<IService1>()
       .As<IService2>();
builder.RegisterDecorator<Decorator, IService1>();
var container = builder.Build();
var a = container.Resolve<IService1>();
var b = container.Resolve<IService2>();
Assert.IsInstanceOf<Decorator>(a);
Assert.IsInstanceOf<Component>(b);

That is, when you ask for it as an IService1 it gets decorated because you registered a decorator for IService1, but when you ask for it as an IService2 it’s not decorated because you didn’t register a decorator for that.

However, let’s say Component is a singleton.

var builder = new ContainerBuilder();
builder.RegisterType<Component>()
       .SingleInstance();
var container = builder.Build();
var a = container.Resolve<Component>();
var b = container.Resolve<Component>();
Assert.Same(a, b);

Now you want to do the decoration again.

var builder = new ContainerBuilder();
builder.RegisterType<Component>()
       .As<IService1>()
       .As<IService2>()
       .SingleInstance();
builder.RegisterDecorator<Decorator, IService1>();
var container = builder.Build();
var a = container.Resolve<IService1>();
var b = container.Resolve<IService2>();

// This should still pass! It's a singleton!
Assert.Same(a, b);

// What type T in here would make this true?
Assert.IsInstanceOf<T>(a);
Assert.IsInstanceOf<T>(b);

For the singleton case, the same component instance needs to fulfill all of the services. Which means if you want to decorate a singleton… the decorator also needs to fulfill all the services.

In the above unit test illustration, the T either has to be Component or Decorator since it’s the same object instance… however, a was resolved as IService1 and b is IService2, so whatever T is, it also needs to be both IService1 and IService2.

This same logic holds for instance-per-lifetime-scope (instance-per-request), ExternallyOwned, and other situations where an instance of a component may need to be shared across multiple resolve calls.

I can imagine a really, really complex system of dynamic proxies and/or interceptors where we basically generate a hybrid wrapper for objects that uses the decorator for methods in the decorated service but passes through to the undecorated methods for things that aren’t decorated, but that could get really complicated really quick, especially when you start layering decorators together.

To be honest, I’m not really interested in building or maintaining that level of complexity and potential unreliability for this case.

Basically, the only option I see here is to make the behavior and reasoning behind it more clear and make it easier to troubleshoot.

• Clarify the exception message if possible to explain why it came up so it’s not just an invalid cast.
• Document this on the decorator page to ensure people know about the gotcha and how to work around it (separating registrations).

I still think the ResolveContext idea is solid, but I don’t think it’ll help in this case. We’d still have the issue where a component with a shared lifetime can’t be both decorated and not decorated at the same time.

Sort of… Schrodinger’s Decorator, if you will.

Yeah, that sounds like the same thing - the decorator is trying to decorate the whole registration rather than just the appropriate service.

Hmmm, this is an interesting case. I think your expectation sounds right, that you should be able to decorate a particular interface but not have the system assume every interface on the registration is decorated.

As a quick workaround, you could probably separate the registration:

builder.RegisterType<Service>().As<IDecorable>();
builder.RegisterType<Service>().As<IService>();
builder.RegisterDecorator<Decorator, IDecorable>();

But that doesn’t solve the problem, it just works around it. We’ll have to see what we can do about this.