0

I am trying to optimize my code for the injection of a list of classes, that implement an interface of IEventHandler<TEvent>.

I have the following structure:

public interface IEventHandlerMarker    {   }

public interface IEventHandler<in TEvent> : IEventHandlerMarker where TEvent : IEvent
{
    Task Handle(TEvent eventItem);
}

public interface IEvent
{
    public DateTime Timestamp { get; set; }
}

I register the marker interface IEventHandlerMarker in DI and when accessing the handlers, I currently do the following:

public EventPublisherService(IEnumerable<IEventHandlerMarker> eventHandlers)
{
    // Check and and all event handlers
    foreach (IEventHandlerMarker item in eventHandlers)
    {
        AddEventHandler(item);
    }
}

In my AddEventHandler method, I filter those to IEventHandler<> like this:

Type handlerType = eventHandlerMarker.GetType().GetInterfaces()
    .FirstOrDefault(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEventHandler<>));

So far everything works, but I'd like to get rid of the marker interface and the filter logic. So I changed the registration of handlers to the following method and this seems to work as expected:

public static IServiceCollection AddEventHandlers(this IServiceCollection serviceDescriptors, params Assembly[] handlerAssemblies)
{
    Type genericHandlerType = typeof(IEventHandler<>);
    foreach (var implementationType in genericHandlerType.GetTypesWithGenericInterfacesInAssemblies(handlerAssemblies))
    {
        Type interfaceType = implementationType.GetGenericInterfaceType(genericHandlerType);
        serviceDescriptors.AddSingleton(interfaceType, implementationType);
    }
    return serviceDescriptors;
}

public static List<Type> GetTypesWithGenericInterfacesInAssemblies(this Type source, params Assembly[] assemblies)
{
    return assemblies
        .SelectMany(currentAssembly => currentAssembly.GetTypes()
        .Where(type => type.GetInterfaces().Any(
                interfaceItem => interfaceItem.IsGenericType
                && interfaceItem.GetGenericTypeDefinition().Equals(source))))
        .ToList();
}

I changed the constructor of EventPublisherService to the following:

public EventPublisherService(IServiceProvider serviceProvider)
{
    Type ienumerableOfIEventHandlerType = typeof(IEnumerable<>).MakeGenericType(typeof(IEventHandler<>));
    object result = serviceProvider.GetService(ienumerableOfIEventHandlerType);
}

But result always turns out to be null. I googled and checked some articles on Stackoverflow and came across the following article: https://stackoverflow.com/a/51504151/1099519

I am not sure if this is the same case, as I am not using a factory.

Versions used: .NET Core 3.1 and Autofac 4.9.4 for the Dependency Injection management.

DominikAmon
  • 892
  • 1
  • 14
  • 26
  • 1
    Unless I'm missing something, Autofac does everything you need by itself. Check this question: https://stackoverflow.com/questions/16757945/how-to-register-many-for-open-generic-in-autofac – Francesc Castells Mar 03 '20 at 08:09
  • I don't have a problem, when registering the classes, this seems to work fine - My problem is, that I can't access them from the ServiceProvider. – DominikAmon Mar 03 '20 at 08:13
  • In [chapter 15](https://livebook.manning.com/book/dependency-injection-principles-practices-patterns/chapter-15) of [DIPP&P](https://mng.bz/BYNl) have an extensive discussion about registering generic types in MS.DI. Take a look, for instance, as [this](https://livebook.manning.com/book/dependency-injection-principles-practices-patterns/chapter-15/235) section. Conclusion from that chapter was, however, that MS.DI is severely limited, making these kinds of scenarios extraordinary hard to achieve. – Steven Mar 03 '20 at 09:28
  • i agree with @Steven, MS DI is certainly not up to the level that Autofac is. It is however more light than Autofac and will fit a lot of needs that do not need a heavyweight such as Autofac – Jonathan Busuttil Mar 03 '20 at 10:08
  • @DominikAmon it's not clear to me what you are trying to achieve. Do you need to resolve all IEventHandlers regardless of the TEvent or only the ones for the TEvent that you are publishing? These have two different solutions. Also, you say you don't have a problem with registering the classes, but it seems very verbose for something that Autofac does in one line of code. Finally, have you considered just caching the matching TEvents with IEventHandlers in a dictionary? doing reflection on every event dispatch will be very inefficient. – Francesc Castells Mar 03 '20 at 12:40
  • @JonathanBusuttil I have Autofac anyways, because I do need the decorator functionality. So If there is a solution with Autofac, I open for this as well of course – DominikAmon Mar 03 '20 at 13:13
  • @FrancescCastells My goal is find all handlers that implement IEventHandler and call the "Handle" method regardless of their TEvent type. I am caching all the handlers on start up in a dictonary. The call is currently done by reflection "Invoke". So it basically it is optimization I'd like to do. – DominikAmon Mar 03 '20 at 13:15

2 Answers2

2

Register all handlers automatically as shown in this question/answer:

builder.RegisterAssemblyTypes(AppDomain.CurrentDomain.GetAssemblies())
   .AsClosedTypesOf(typeof (IEventHandler<>)).AsImplementedInterfaces();

When you have TEvent and want to find all handlers, get them by constructing the concrete interface type as follows:

Type generic = typeof(IEnumerable<IEventHandler<>>);
Type[] typeArgs = { typeof(TEvent) }; // you might get the Type of TEvent in a different way
Type constructed = generic.MakeGenericType(typeArgs);

You should cache this in an dictionary to avoid doing reflection at every event dispatch.

Once you have the constructed concrete interface type, you can ask Autofac for all implementations of that interface:

var handlers = container.Resolve(constructed);

Now, the problem is that with the handler instances you can only call the Handle method using Invoke (reflection). This is a performance issue, but it's unrelated to how you register and resolve the handlers. It's related to the fact that you need to call a concrete method from an object. To avoid using reflection you need compiled code that calls the concrete method (note that using Generics also creates concrete compiled code for each Type).

I can think of two ways of getting compiled code to do this calls:

  1. Manually writing a delegate which casts your object handler instance into a concrete type for every TEvent type that you have. Then store all these delegates in a dictionary so you can find them at runtime based on TEvent type and call it passing the handler instance and the event instance. With this approach, for every new TEvent that you create, you need to create a matching delegate.

  2. Doing a similar thing as before but by emitting the code at startup. So, same thing, but the whole creation of the delegates is automatic.


Update

Based on the repository shared by the OP, I created a working version. The main code, to resolve the handlers and call them is in the EventPublisherService class

public class EventPublisherService : IEventPublisherService
{
    private readonly ILifetimeScope _lifetimeScope;

    public EventPublisherService(ILifetimeScope lifetimeScope)
    {
        _lifetimeScope = lifetimeScope;
    }

    public async Task Emit(IEvent eventItem)
    {
        Type[] typeArgs = { eventItem.GetType() };
        Type handlerType = typeof(IEventHandler<>).MakeGenericType(typeArgs);
        Type handlerCollectionType = typeof(IEnumerable<>).MakeGenericType(handlerType);

        var handlers = (IEnumerable<object>)_lifetimeScope.Resolve(handlerCollectionType);

        var handleMethod = handlerType.GetMethod("Handle");

        foreach (object handler in handlers)
        {
            await ((Task)handleMethod.Invoke(handler, new object[] { eventItem }));
        }
    }
}

Note that as specified in the original answer, this solution does not include the very necessary performance optimisations. The minimum that should be done is to cache all the types and MethodInfos so they don't need to be constructed every time. The second optimisation would be, as explained, to avoid using Invoke, but it's more complicated to achieve and IMO it requires a separate question.

Francesc Castells
  • 2,692
  • 21
  • 25
  • My co worker created a project on Github to reproduce: https://github.com/Lutti1988/effective-di Didn't get it working? – DominikAmon Mar 09 '20 at 08:53
  • Did you push the whole solution? If I search for "MakeGenericType" in the repository it doesn't find it. – Francesc Castells Mar 09 '20 at 09:06
  • We tried several ways, but maybe we did missunderstand something. Here is the latest Commit: https://github.com/Lutti1988/effective-di/commit/4f505f707ec9137142417c0f464bdadd025fe554 – DominikAmon Mar 10 '20 at 13:46
  • Ok, I see the code needs to be adjusted to make it work. I'll try to make a sample for you. – Francesc Castells Mar 10 '20 at 15:09
  • Thank you for the update! I now see what you ment, I didn't think about getting the type of the event inside the Emit method with a concrete implementation of IEvent, as I was only focusing on the constructor. It works like a charm :) – DominikAmon Mar 11 '20 at 07:08
  • No problem. In my original answer there was also a mistake constructing the type, so running it helped me to fix that. – Francesc Castells Mar 11 '20 at 08:09
0

With autofac you can inject an IEnumerable<IEventHandler<TEvent>> and Autofac should resolve a List of all implementations of it.

https://autofaccn.readthedocs.io/en/latest/resolve/relationships.html