Interface interception fails when used with open generic assembly scanning

[tillig]

This comes from a StackOverflow question

Using assembly scanning in conjunction with interface interception yields an exception:

The component ... cannot use interface interception as it provides services that are not publicly visible interfaces. Check your registration of the component to ensure you're not enabling interception and registering it as an internal/private interface type.

Code to reproduce the issue as a console app:

namespace InterceptorRepro
{
    public interface ICommand<TResult>
    {
    }

    public class StringCommand : ICommand<String>
    {
    }

    public interface ICommandHandler<TCommand, TResult>
        where TCommand : ICommand<TResult>
    {
        TResult Execute(ICommand<TResult> command);
    }

    public class StringCommandHandler : ICommandHandler<StringCommand, String>
    {
        public string Execute(ICommand<string> command)
        {
            return "generic-method";
        }
    }

    public class LoggingInterceptor : IInterceptor
    {
        TextWriter _output;

        public LoggingInterceptor(TextWriter output)
        {
            _output = output;
        }

        public void Intercept(IInvocation invocation)
        {
            _output.Write("Calling method {0} from LoggingInterceptor", invocation.Method.Name);
            invocation.Proceed();
            _output.WriteLine("LoggingInterceptor complete.");
        }
    }

    public class Program
    {
        public static void Main()
        {
            var builder = new ContainerBuilder();
            builder.Register(c => Console.Out).As<TextWriter>();
            builder.RegisterType<LoggingInterceptor>();
            builder
                .RegisterAssemblyTypes(typeof(Program).Assembly)
                .AsClosedTypesOf(typeof(ICommandHandler<,>))
                .EnableInterfaceInterceptors()
                .InterceptedBy(typeof(LoggingInterceptor));

            var container = builder.Build();

            try
            {
                using (var scope = container.BeginLifetimeScope())
                {
                    // Exception thrown on this resolve:
                    var handler = scope.Resolve<ICommandHandler<StringCommand, string>>();
                    Console.WriteLine("Handler type is: {0}", handler.GetType());
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
            }
        }
    }
}

If you remove the EnableInterfaceInterceptors() and InterceptedBy() calls from the registration then the resolution proceeds as expected.

[claerhouth]

Any updates here? This poses a problem for a project I am working on.
Because I need this interception to occur on every method of implementing classes for a wrapper interface, a decorator pattern implementation seems hard to maintain.

[blairconrad]

@claerhouth, I am not affiliated with the Autofac.Extras.DynamicProxy package, nor with Autofac as a whole. So I probably shouldn't be saying anything, but after a conversation with @tillig that arose out of his blog post asking for maintainers, my understanding is that most of the Autofac extension projects are severely understaffed—the core Autofac maintainers are just stretched too thin. If you don't see any indication that an issue is being worked on (like this one), it probably is not. And there's no guarantee that the Autofac maintainers will find the time to do so in the near future.

If this issue causes such problems for you, I'd suggest investigating it yourself. It appears that all the code necessary to reproduce it is available in this issue. Maybe you can find a solution, which would move your project forward while simultaneously helping out this project and its maintainers.

[tillig]

@claerhouth Sorry, but @blairconrad is right. There's only two maintainers and we're pretty overwhelmed. If you don't see an update posted, there's no update. If there's something you need an immediate fix for, a pull request with a solution will be your best way to go.

[claerhouth]

Going to take a look at it. If I am able to find a solution I'll certainly try and get it accepted...

[blairconrad]

It's been a while with no action, so I took a look. Remember, I don't use autofac, so I'm going to use the wrong terms for everything and completely misunderstand stuff. Fun!

Here's what I see

1. we're trying to wrap the resolved component with a LoggingInterceptor
2. EnableInterfaceInterceptors indicates that we want to accomplish this wrapping by having DynamicProxy create a proxy that implements an interface (rather than creating a subclass of the type of the resolved component) and delegates to the wrapped component
3. if DynamicProxy is proxying an interface, the resolution's effective return type must be the proxied interface (this is a simplification, as the proxy may implement more than interface, but let's go with that), not the concrete component type. In particular, note that the created proxy will not be subclass of the component's type
4. so this wrapping operation only makes sense when the service we're resolving is an interface (as ICommandHandler<StringCommand, string> is)
5. thus the handler registered from within EnableInterfaceInterceptors wants to make sure that the service being resolved is actually an interface (and accessible as well, but that's not our problem today), but as far as I can tell , ActivatingEventArgs doesn't provide information about the service that's being resolved, so it does the best it can by looking at the services that are registered, and it finds 2: ICommandHandler<StringCommand, string> and StringCommandHandler. From this EnsureInterfaceInterceptionApplies cannot be sure that we're not resolving a concrete type (StringCommandHandler) so it bails!

I don't see a quick fix, but I have some thoughts:

1. if I'm wrong, and there's a way to tell what service's being resolved, EnsureInterfaceInterceptionApplies should make sure that that is an interface, instead of looking at the service registrations. Otherwise,
2. I'd look for a way to limit the services that get Ased by AsClosedTypesOf. I don't know of anything that can be done without modifying this method, but maybe an alternative that only registered the found component(s) as the interface types

@tillig, @claerhouth, am I wrong? Is option one possible? Otherwise, how do we feel about the second?

[blairconrad]

Oh, @claerhouth, if your concrete components are well-behaved, you might do all right using EnableClassInterceptors.

[tillig]

@alexmg was looking at this and mentioned to me that the problem largely stems from needing to know the resolved type (i.e., thought #1 mentioned by @blairconrad). That hasn't really been passed along as yet, though some of the work done recently in core Autofac with decorator syntax that got close to having a more rich resolve context passed around that exposed the required information. I haven't got specific details on it at hand, but it may be that some modifications to the Autofac core internals could help address this in a better way than simply trying to address it by working around things at this level.

[blairconrad]

I was about to say that I think there'd still need to be a little work done in this package to resolve the bug, if we could know what we are trying to resolve. But the more I think about it, I think the EnableClassInterceptors and EnableInterfaceInterceptors should really just go away.
If you're resolving a class, I think you need the EnableClassInterceptors behaviour, and if you're resolving an interface, you need the EnableInterfaceInterceptors.

If the core changed in the way you suggest, @tillig, I'd recommend adding a 3rd method (EnableInterceptors?) to do the intercepting that would essentially delegate to either of the two behaviours depending on the service type. Then I'd deprecate the original entry points.

If it comes to that, I'd be happy to do the work, assuming nobody else wanted to.

[tillig]

I admit I got lost in tracing some of the work that got done with the decorator enhancements so I can't speak too closely to that. I think there's information that would be helpful during a resolve operation that could be passed around to aid in things like this, but also in things like circular dependency detection. As mentioned, I can't really get specific about what needs to happen; I've spent a lot of my allocatable free time lately on Moq integration, MvvmCross integration, and answering general questions through the various channels we monitor. It's hard to get momentum on the larger internal updates that need to happen.

[alexmg]

I went back to some earlier decorator work that I stashed and managed to get the Service being resolved passed through to both the activating and activated events. It's most definitely a breaking change as the ResolveComponent method on IComponentContext needs to be changed.

From:

object ResolveComponent(IComponentRegistration registration, IEnumerable<Parameter> parameters);

To:

object ResolveComponent(Service service, IComponentRegistration registration, IEnumerable<Parameter> parameters);

Need to consider if this is something that will have to wait for the 5.0 release. From a technical standpoint it's most certainly a breaking change even though it's not a method that would normally be called directly by users.

[rbev]

I had a similar issue in and as @blairconrad alluded to the problem is in AsClosedTypesOf
I'd put up a PR to fix this issue, but it really just needs a new extension added into Autofac and the documentation updated appropriatelv

My solution was to just implement an interface-only version of AsClosedTypesOf, if there is interest and an appropriate feature request raised against Autofac then i'll happily put together a PR as i dont' believe this extension method belongs in this plugin.

Here's the implementation....

        /// <summary>
        /// Specifies that a type from a scanned assembly is registered if it implements an interface
        /// that closes the provided open generic interface type.
        /// </summary>
        /// <typeparam name="TLimit">Registration limit type.</typeparam>
        /// <typeparam name="TScanningActivatorData">Activator data type.</typeparam>
        /// <typeparam name="TRegistrationStyle">Registration style.</typeparam>
        /// <param name="registration">Registration to set service mapping on.</param>
        /// <param name="openGenericServiceType">The open generic interface or base class type for which implementations will be found.</param>
        /// <returns>Registration builder allowing the registration to be configured.</returns>
        public static IRegistrationBuilder<TLimit, TScanningActivatorData, TRegistrationStyle> AsClosedInterfacesOf<TLimit, TScanningActivatorData, TRegistrationStyle>(this IRegistrationBuilder<TLimit, TScanningActivatorData, TRegistrationStyle> registration, Type openGenericServiceType) where TScanningActivatorData : ScanningActivatorData
        {
            if ((object)openGenericServiceType == null)
                throw new ArgumentNullException(nameof(openGenericServiceType));
            if (!openGenericServiceType.IsInterface)
                throw new ArgumentException("Generic type must be an interface", nameof(openGenericServiceType));

            return registration
                .Where(candidateType => candidateType.IsClosedTypeOf(openGenericServiceType))
                .As(candidateType =>
                    candidateType.GetInterfaces()
                        .Where(i => i.IsClosedTypeOf(openGenericServiceType))
                        .Select(t => (Service)new TypedService(t)));
        }

[tillig]

I think that's a decent workaround but it'd be nice if we could solve this by adjusting the actual internals. For example, we know internally when Autofac is resolving something what the target type (interface, class, whatever) should be; it just, from what I can tell, isn't made accessible via any sort of event arguments. If it was available in the ActivatingEventArgs, for example, then the interceptor bits could look at that rather than having to guess. This is what @alexmg was mentioning, and I think it'd be worth looking into.

[RaymondHuy]

Hi @tillig, @alexmg I have just had a look on this issue and I see that there will be some breaking changes:

1. Add Service property to the following classes: ActivatedEventArgs, ActivatingEventArgs, PreparingEventArgs
2. Add Service parameter to the following methods in IComponentRegistration interface: RaisePreparing, RaiseActivating, RaiseActivated

It may also effect the other extensions. Do you think it's worth to fix ? I can spend time on this breaking changes if it is the point ;)

[tillig]

I don't think it'll be too bad if we have to add some stuff to those events. It's doubtful anyone has implemented their own IComponentRegistration and I don't think adding properties to events is breaking... though adding to the constructor is.

I know we have a lot of information coming through in the ResolveRequest now; perhaps that can be helpful?

Also, for a singleton... if I'm not mistaken, you only get OnActivated once. You wouldn't want it to do something for one particular service especially if it exposes more than one.

builder.RegisterType<Component>()
       .As<IService1>()
       .As<IService2>()
       .SingleInstance();

Ideally, though, this wouldn't break all the extensions. We just went through a whole major release breaking change thing; if the solution is that we need another breaking change, I don't know that we're ready to fire off an Autofac 6.0 and do all that again right now.