simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 153 forks source link

Add factory delegates to collection registration #866

Closed BahKoo closed 3 years ago

BahKoo commented 4 years ago

I'm looking for a way to register a collection using a delegate. To be clear, I'm not looking to use a factory to append a single instance to the collection; I'm looking for a factory that produces the collection.

My use case for this is that I'm using a 3rd party library named MediatR which gets the collection of INotificationHandler<TNotification> from the container. At design-time, I am not able to provide the list of assemblies that will contain all implementation of INotificationHandler<TNotification> because some of the implementations will come from dynamically loaded assemblies. I can't make a composite object that would produce the list of INotificationHandler because this is not what is expected in the 3rd party library.

Is there a way to do this or a reason why it would be a bad idea to make this possible in Simple Injector?

dotnetjunkie commented 4 years ago

I'm looking for a factory that produces the collection.

That would be something like this:

var collection = new Collection<IService>();
container.RegisterInstance<IEnumerable<IService>>(collection);

collection of INotificationHandler<TNotification>

But that changes everything, because INotificationHandler<T> is generic. You probably don't want to register each closed version of this interface separately.

some of the implementations will come from dynamically loaded assemblies

I assume that these dynamically loaded assemblies can all be loaded at startup, otherwise you would step into a world of complexity (as DI Containers like to know everything up front). When you can load all assemblies at startup, you can just do this:

var assemblies = LoadAllDynamicAssemblies().Concat(compileTimeAssemblies);

container.Collection.Register(typeof(INotificationHandler<>), assemblies);
BahKoo commented 4 years ago

I understand that, generally, DI containers like to know everything up front. I see how loading all the assemblies upfront could solve my problem but unfortunately this is not a solution that makes sense in my application. I see factory delegates as an opportunity to defer the need to locate concrete implementations upfront. Am I wrong in that assumption?

For example, the ContainerCollectionRegistrator could possibly have a function like this:

public void Register<TService>(Func<IEnumerable<TService>> collectionCreator)

Locating the assemblies could be deferred to the time at which the collection in retrieved.

dotnetjunkie commented 4 years ago

If you can't load the assembly at startup, when would you be able to load it, and how would you load it?

I'm not saying that what you want isn't possible, and I like to help you in getting there, but it's a lot more work to get this running. If you can answer my previous question, I'll probably be able to show you some POC code.

BahKoo commented 4 years ago

I appreciate the help.

This application is a boxed application that we deliver to multiple customers. Each customer hosts the application locally.

With this application, we deliver a library of assemblies that add to the base implementation of the application. Each of these assemblies is tied to a particular integration partner. For example, we deliver assemblies like this (integration.AAA.dll, integration.BBB.dll, etc...).

After a user logs in to our application, we are able to determine which integration they are currently using. We can then load the assembly for that integration. From that point forward, for all logic that is scoped to that user, I want to ensure all the implementations of INotificationHandler<T> in the base application plus the user's current integration can be injected as a collection.

From my understanding, there would maybe be 3 ways to do this. 1) Load all integration assemblies upfront. The downsides to this would be that I would need to load ALL integration assemblies even if a customer is only using a single one. Also, I would need a strategy to filter out the concrete implementations from the integrations the user isn't currently using. 2) Create a composite class that is able to load the appropriate concrete implementations based on the logged-in users' current integration. Unfortunately, this wouldn't work with Mediatr as I don't own the code that gets the instances of INotificationHandler<T> directly. 3) Add the function to SimpleInjector as I suggested. I don't know if this is a common enough scenario to justify that feature.

dotnetjunkie commented 4 years ago

Making the registrations for your notification handlers dynamically isn't the problem. But do these handlers have their own dependencies that live in those same integration assemblies? Or are all dependencies guaranteed to have already been registered in the Container?

BahKoo commented 4 years ago

All dependencies will have already been registered.

dotnetjunkie commented 4 years ago

Few more questions:

  1. You described that the user logs in to the application. Are multiple users (that might require different integration DLLs) using the same app domain? For instance, is this code running inside a web application? Or is only one user logging in per app domain or only one homogeneous group of users that all require the same integration DLLs?
  2. How are you currently determining which (set of) integration DLL(s) the logged in user requires?
  3. You noted that you "can't make a composite object that would produce the list of INotificationHandler because this is not what is expected in the 3rd party library." What is MediatR's expectation and why can't that be changed?
BahKoo commented 4 years ago
  1. Yes. Multiple users that might require different integration DLLs are using the same app domain because the code is running inside a web application.
  2. When a user logs in, they choose which "database" to login to. Each database is associated with a single integration. For the duration of their session, that user requires the single integration DLL that is associated with the database they are currently logged in to.
  3. The code in MediatR that uses SimpleInjector (indirectly) is on line 22 of the internal NotificationHandlerWrapperImpl class which can be seen here: https://github.com/jbogard/MediatR/blob/master/src/MediatR/Internal/NotificationHandlerWrapper.cs
    21 var handlers = serviceFactory
    22    .GetInstances<INotificationHandler<TNotification>>()

    Note that this class is marked internal and its usage in the Mediator.cs class is not overridable.

I have not raised an issue in the MediatR repository because I'm not sure whether a good solution involves using SimpleInjector differently, changing how MeditaR works are taking a different approach to solving this problem.

dotnetjunkie commented 4 years ago

There are multiple ways to approach this. Let's start with option 1:

Option 1

If you want to go with a full-fledged lazy-loaded solution, what you want to do is register your own IEnumerable<T> implementation that acts as a factory. This factory would look something like this:

public sealed class NotificationHandlerCollection<T> : IEnumerable<INotificationHandler<T>>
    where T : INotification
{
    private readonly Container container;
    private readonly IUserContext userContext;

    private readonly ConcurrentDictionary<string, InstanceProducer<INotificationHandler<T>>[]> handlers =
        new ConcurrentDictionary<string, InstanceProducer<INotificationHandler<T>>[]>();

    public NotificationHandlerCollection(Container container, IUserContext userContext)
    {
        this.container = container;
        this.userContext = userContext;
    }

    public IEnumerator<INotificationHandler<T>> GetEnumerator()
    {
        var coreProducers = this.handlers.GetOrAdd("core", this.CreateDatabaseHandlers);
        var dbProducers = this.handlers.GetOrAdd(this.userContext.Database, this.CreateDatabaseHandlers);

        foreach (var producer in coreProducers.Concat(dbProducers))
        {
            yield return producer.GetInstance();
        }
    }

    IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();

    private InstanceProducer<INotificationHandler<T>>[] CreateDatabaseHandlers(string database)
    {
        Assembly[] assemblies = this.GetAssembliesFor(database);

        return (
            from type in this.container.GetTypesToRegister(typeof(INotificationHandler<T>), assemblies)
            select Lifestyle.Transient.CreateProducer<INotificationHandler<T>>(type, this.container))
            .ToArray();
    }

    private Assembly[] GetAssembliesFor(string database)
    {
        if (database == "core") return new[] { this.GetType().Assembly };

        // TODO: Loading of database assemblies
        throw new NotImplementedException();
    }
}

And you register it as follows:

container.RegisterSingleton(typeof(IEnumerable<>), typeof(NotificationHandlerCollection<>));

This might require some explanation. So here goes:

Typically, collections are registered in Simple Injector using Container.Collection.Register. This notifies Simple Injector that requests for IEnumerable<Something>, IList<Something>, ICollection, etc should be forwarded to that registration. But nothing stops you from registering anIEnumerable` implementation yourself, which is what the previous implementation does.

Because MediatR eventually calls Container.GetInstance(typeof(IEnumerable<INotificationHandler<T>>)), the explicitly registered NotificationHandlerCollection<T> will get returned as it matches the request. After the collection is requested, MediatR starts iterating the collection.

The NotificationHandlerCollection<T> doesn't do anything upon creation. It starts to do its unless it gets iterated. Upon iteration it will start loading assemblies based on a database, loads the handler types from those assemblies, creates InstanceProducers for them (Simple Injector's factories), and store those producers in a thread-safe collection.

Those instance producers are iterated and their corresponding handlers are requested and returned from the GetEnumerator() method.

This class depends on the IUserContext service. That service allows retrieving the current user's database. Based on that database you will be able to load the proper assemblies.

The only thing you have to do is to implement the GetAssembliesFor method correctly.

This was option 1. I'll post other options below, although it might take some time for me to do so.

Cheers

BahKoo commented 4 years ago

Option 1 is a very creative solution. I have implemented it and my problem is solved. I look forward to seeing the other options.

dotnetjunkie commented 3 years ago

Option 2

A second option is to replace MediatR's default IMediator implementation with a custom one. This might look like a lot of work, but actually isn't the case, because you only have to replace the "publish" behavior and can reuse the existing "send" behavior. This works best if you don't use the IMediator interface in your application at all, but solely depend on ISender and IPublisher, because this allows you to solely replace the IPublisher implementation. In the code below, however, I assume IMediator, and create a custom implementation for that.

This next implementation shows the skeleton which you can use to replace the logic you want:

public class SimpleInjectorMediator : IMediator
{
    private readonly Container container;
    private readonly Mediator original;

    public SimpleInjectorMediator(
        Container container,
        Mediator original)
    {
        this.container = container;
        this.original = original;
    }

    // Send calls just forward to the built-in Mediator.
    public Task<TResponse> Send<TResponse>(
        IRequest<TResponse> request, CancellationToken token) =>
        this.original.Send(request, token);

    public Task<object> Send(object request, CancellationToken token) =>
        this.original.Send(request, token);

    public Task Publish<TNotification>(TNotification notification, CancellationToken token)
        where TNotification : INotification =>
            this.PublishCore(notification, token);

    // By using dynamic, we can map this non-generic overload to the generic PublishCore<T>.
    // This saves a lot of ugly reflection and might even be faster.
    public Task Publish(object notification, CancellationToken token) =>
        this.PublishCore((dynamic)notification, token);

    // NOTE: This method must be public, because Publish(object) calls it using dynamic.
    public async Task PublishCore<T>(T notification, CancellationToken token)
        where T : INotification
    {
        // TODO: replace this line with lazy loading
        var allHandlers = this.container.GetAllInstances<INotificationHandler<T>>();

        foreach (var handler in allHandlers)
        {
            await handler.Handle(notification, token).ConfigureAwait(false);
        }
    }
}

// Registration
container.RegisterInstance<IMediator>(
    new SimpleInjectorMediator(container, new Mediator(container.GetInstance)));

PublishCore is the place where you would implement the lazy loading and dictionary approach that was implemented in option 1.

dotnetjunkie commented 3 years ago

Option 3

Instead of using MediatR with its abstractions and defined implementations, consider defining those interfaces yourself. This generally has my preference, because it keeps the application in control over how interfaces would look like and prevents the complete application from requiring a dependency on an external library.

I've written about this in the past:

If you use Simple Injector and replace the pipeline behaviors with simple decorators, hooking everything up with Simple Injector would be dead-simple.

Of course you still want the lazy loading, but this can now be implemented in an adapter of a custom IPublisher implementation, or you could even place this logic inside a Composite implementation for INotifacationHandler<T>. This would allow you to simply inject an INotificationHandler<T> into a consumer, which would cause the composite to be called, which would forward the event to the underlying handlers.

BahKoo commented 3 years ago

Thank you for your very detailed response. I will need to spend more time considering options 2 & 3 and read through all your past comments on the matter. What led me to using MediatR was Microsoft's guide for Tackle Business Complexity in a Microservice with DDD and CQRS Patterns. I found Microsoft's guide very helpful for getting started with DDD principals and following it has led me to practical solutions to many of my problems. I have noticed how lightweight the MediatR library is and have already introduced child interfaces to differentiate my queries from my commands. I can see how its necessity might fade away as I look to extend or replace more of its parts.

Thanks again.