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 155 forks source link

Collection.Register set Lifestyle #688

Closed VladSnap closed 5 years ago

VladSnap commented 5 years ago

I want to register a collection of open-generic types with a specific Transient. I do not understand how I can achieve this? By default, I set up Lifestyle.Scoped.

I have decorators (only an injection is not through the designer), the implementations of decorators are open generics. I can register them only through Collection.Register; if I register through Register, I get an error:

The supplied list of types contains one or multiple open-generic types, but this method is unable to handle open-generic types because it can only map closed-generic service types to a single implementation. You must register the open-generic types separately using the Register(Type, Type) overload. Alternatively, try using Container.Collection.Register instead, if you expect to have multiple implementations per closed-generic abstraction. Invalid types: SaveChangesDecorator<TIn, TOut>

My register code:

var typesToRegister = container.GetTypesToRegister(typeof(IDecoratorHandler<,>), assemblies,
    new TypesToRegisterOptions
    {
        IncludeGenericTypeDefinitions = true,
        IncludeComposites = false,
    });

// not work
//container.Register(typeof(IDecoratorHandler<,>), typesToRegister, Lifestyle.Transient);
// work, how to set Lifestyle.Transient?
container.Collection.Register(typeof(IDecoratorHandler<,>), typesToRegister);
dotnetjunkie commented 5 years ago

Your question is unclear to me:

It would be good if you describe an Minimal, Complete, and Verifiable example that contains:

VladSnap commented 5 years ago

Hi Steve, thank for fast answer! I think I do not fully understand the API registration or I have a conceptual error.

What is the meaning of IDecoratorHandler? Is that an abstraction specificically for decorators?

Yes, this decorator abstraction.

Do you wish to inject collections of those handlers into consumers or always just a single instance?

I explicitly get implementations in the command or query constructor. That is, I always need only a specific implementation, I do not work with the decorator list.

The main thing for me is to understand how to specify a lifestyle when registering a collection. I have a handler for the OrderCreatedCmdHandler command, several decoratorsIDecoratorHandler <TIn, TOut>are injected into it through the constructor. I also have similar handlers for the IIntegrationEventHandler <TEvent> integration events. In my case, I have two different IIntegrationEventHandler <TEvent>, which in the constructor accepts the same OrderCreatedCmdHandler. And the lifestyle is standard in my application Lifestyle.Scoped and it turns out when I callCollection.Register, all my decorators have the lifestyle Lifestyle.Scoped, but they should beTransient (The whole question is, how do I specify during registration Transient). When I call Container.Verify (), I get an error when creating the pipeline, (Important! The second call to the constructor OrderCreatedCmdHandler due to twoIIntegrationEventHandler <TEvent>that consume the sameOrderCreatedCmdHandler) . The error occurs because, in the decorator, an injection of another decorator has already been made (I have a test for this case). The trouble is that IDecoratorHandler <TIn, TOut> has the Scoped lifestyle, while OrderCreatedCmdHandler has the Transient lifestyle.

MCVE: (This code reproduces my problem)

This is an analogue of MediatR interfaces.

public interface IDecoratorHandler<TIn, TOut> : IHandler<TIn, TOut>
{
    IHandler<TIn, TOut> NextHandler { get; }

    void InjectDecorator(IHandler<TIn, TOut> handler);
}

public interface IHandler<in TIn, TOut>
{
    Task<TOut> Handle(TIn input);
}

The implementation of decorators. I decided to manually manage the injection to build the pipeline manually.

public abstract class BaseDecorator<TIn, TOut> : IDecoratorHandler<TIn, TOut>
{
    protected bool _isDecoratorInjected;
    protected IHandler<TIn, TOut> _decorated;
    public IHandler<TIn, TOut> NextHandler { get; private set; }

    public BaseDecorator() { }

    public abstract Task<TOut> Handle(TIn input);

    public void InjectDecorator(IHandler<TIn, TOut> handler)
    {
        // this throw exception
        if (_isDecoratorInjected)
            throw new InvalidOperationException("Decorator already has injected");

        _isDecoratorInjected = true;
        _decorated = handler;
    }
}

public class IntegrationSenderDecorator<TIn, TOut> : BaseDecorator<TIn, TOut>
{
    public IntegrationSenderDecorator() { }

    public override async Task<TOut> Handle(TIn input)
    {
        var res = await _decorated.Handle(input);
        // ...
        return res;
    }
}

public class SaveChangesDecorator<TIn, TOut> : BaseDecorator<TIn, TOut>
{
    public SaveChangesDecorator() { }

    public override async Task<TOut> Handle(TIn input)
    {
        var res = await _decorated.Handle(input);
        // ...
        return res;
    }
}

I create manually pipeline from decorators. The handler with logic still has to be the last, I pass it.

public class OrderCreatedCmdHandler : IHandler<string, int>
{
    public OrderCreatedCmdHandler(
        IntegrationSenderDecorator<string, int> integrationSenderDecorator,
        SaveChangesDecorator<string, int> saveChangesDecorator)
    {
        //_pipeline = DecoratorPipelineBuilder<string, int>.Create()
        //    .Add(integrationSenderDecorator)
        //    .Add(saveChangesDecorator)
        //    .Build(logicHandler); // skip

        // analog Build code:
        integrationSenderDecorator.InjectDecorator(saveChangesDecorator);
    }

    public async Task<int> Handle(string input)
    {
        return 123;
    }
}

There are two different integration event handlers that need the same event command.

public class IntegrationEventHandler1
{
    public IntegrationEventHandler1(IHandler<string, int> cmdHandler) { }
}

public class IntegrationEventHandler2
{
    public IntegrationEventHandler2(IHandler<string, int> cmdHandler) { }
    //public IntegrationEventHandler2(/*IHandler<string, int> cmdHandler*/) { } // Everything is working
}

Reproduction problems:

static void Main(string[] args)
{
    var container = new Container();
    container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();
    container.Options.DefaultLifestyle = Lifestyle.Scoped;
    //container.Options.DefaultLifestyle = Lifestyle.Transient; // Everything is working
    container.Register<IHandler<string, int>, OrderCreatedCmdHandler>(Lifestyle.Transient);
    container.Register<IntegrationEventHandler1>(Lifestyle.Transient);
    container.Register<IntegrationEventHandler2>(Lifestyle.Transient);

    var typesToRegister = container.GetTypesToRegister(typeof(IDecoratorHandler<,>), 
        new Assembly[] { typeof(Program).Assembly },
        new TypesToRegisterOptions {
            IncludeGenericTypeDefinitions = true,
            IncludeComposites = false
        });

    container.Collection.Register(typeof(IDecoratorHandler<,>), typesToRegister);
    container.Verify();
}

Thank you in advance for your reply!

Program.cs.txt

dotnetjunkie commented 5 years ago

Hi VladSnap,

I think I do not fully understand the API registration or I have a conceptual error.

The short rule is:

The main thing for me is to understand how to specify a lifestyle when registering a collection.

There are several overloads available of the Collection.Append method that allow you to specify a lifestyle. In your case, however, those overloads are not helpful, because they are all generic, while you have to specify a Type instance. I created a work item for that (#691).

UPDATE: This feature has been implemented in Simple Injector v4.6. This release is available on NuGet. You can now use the non-generic Collection.Append overloads.

The best current workaround I can think of right now is to override the Options.LifestyleSelectionBehavior:


class TransientDecoratorHandlerBehavior : ILifestyleSelectionBehavior
{
    private readonly Container container;

    public TransientDecoratorHandlerBehavior(Container container)
        => this.container = container;

    public Lifestyle SelectLifestyle(Type implementationType) =>
        implementationType.IsClosedTypeOf(typeof(IDecoratorHandler<,>))
            ? Lifestyle.Transient
            : this.container.Options.DefaultLifestyle;
}

You can hook this up as follows:

var container = new Container();
container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();
container.Options.DefaultLifestyle = Lifestyle.Scoped;
container.Options.LifestyleSelectionBehavior = new TransientDecoratorHandlerBehavior(container);

I must admit, though, that I find your current design quite confusing:

Perhaps I don't understand your application's constraints that lead to this design, but here are a few suggestions:

VladSnap commented 5 years ago

Hi, Steve! I apologize for the delay in replying. Thanks for the answer, I was able to temporarily solve the problem. I just made the default 'Transient' lifestyle, as you suggested, to simplify the configuration. I am very grateful to you for the answers, this is very cool!

But I would like to consult you about the design, you would be very helpful if it is possible.

I created a work item for that (#691)

It's great that my question helped improve the simple injector.

I must admit, though, that I find your current design quite confusing

I agree, I want to fix it.

Stop using MediatR as a reference for your own architecture.

I don't copy it, in fact I take most of it from here https://github.com/hightechgroup/force/tree/demo-app These are the developments that were shown at the Moscow DotNext conference 2018 (Instant design: https://2018.dotnext-moscow.ru/2018/msk/talks/1siysgi5tsumw6o22e6iue/). But unfortunately they are raw and there are no examples. The author of this library makes a reference to MediatR and by the way recommends SimpleInjector :)

You talk about 'decorators' Perhaps I don't understand your application's constraints that lead to this design

Yes, I did it temporarily. There are no restrictions that lead to such a design.

  1. I do not understand how I create a different pipeline? For example, in one case it is necessary to cache, in the other not to cache. Perhaps this is implemented through type restrictions?

Instead of injecting cross-cutting concerns into concrete consumers, wrap them around those consumers (decoration). This is less fragile, and more maintainable

  1. Can you tell me how to do this correctly?

  2. As I understand it, I have to make real decorators to improve the design. Will I have to manually register all decorators using RegisterDecorator, in the sequence in which I want to get the pipeline?

  3. Now I have classes of the form ... CmdHandler only collect the pipeline, and the logic itself is processed in a separate class IHandler. But it seems that I understood that this class should describe the logic of processing a yuzkeys, and decorators should wrap it. Am I thinking right?

dotnetjunkie commented 5 years ago

I'm sorry for not getting back to you on your questions. I forgot about them. Will try to answer Monday.

VladSnap commented 5 years ago

It's okay =) You have already helped me, these are side questions that are not relevant to the topic of the question. But if you answer, I will be glad and grateful to you! Your wise advice is very valuable! Thanks!

dotnetjunkie commented 5 years ago
  1. I do not understand how I create a different pipeline? For example, in one case it is necessary to cache, in the other not to cache. ? Perhaps this is implemented through type restrictions?

There are several ways to do this. Personally, I like separating queries from commands using a different abstraction. MediatR bundles them using a single IRequestHandler<T, R> abstraction, making it harder to differentiate between query handlers and command handlers, which often require a different set of cross-cutting concerns. These two articles: this and this, describe separate abstractions for command handlers and query handlers.

But even if you keep the two models (queries and commands) combined in a single IRequestHandler abstraction, Simple Injector allows you to register decorators conditionally, by supplying a predicate:

container.RegsiterDecorator(
    typeof(IHandler<,>),
    typeof(CachingHandlerDecorator<,>),
    c => { some condition });

This condition can be anything:

Another option is to declare generic type constraints on your decorators. This allows them to be applied conditionally. e.g.:

public CachingHandlerDecorator<TIn, TOut> : IHandler<TIn, TOut>
    where TIn : ICachableQuery // TIn must be a cachable query
{
    public CachingHandlerDecorator(IHandler<TIn, TOut> decoratee) { ... }
}

In this case, you can simply register this decorator as follows:

container.RegsiterDecorator(typeof(IHandler<,>), typeof(CachingHandlerDecorator<,>));

Simple Injector will automatically detect the type constraints and apply the decorator conditionally based on it generic type constraints.

  1. Can you tell me how to do this correctly?

I would, again, urge you to read the two linked articles. They describe how to define and use decorators.

  1. As I understand it, I have to make real decorators to improve the design. Will I have to manually register all decorators using RegisterDecorator, in the sequence in which I want to get the pipeline?

You could register decorators using Auto-Registration (using reflection), but that typically doesn't work well for decorators, as a specific order is required, while with Auto-Registration, you lose the ordering guarantee. Thing to note is, though, that decorators should typically be generic in nature, and not defined for a specific use case. For instance, don't do this:

// Bad idea
container.RegisterDecorator<IHandler<CreateOrder, int>, ValidateCreateOrderDecorator>();
container.RegisterDecorator<IHandler<CancelOrder, int>, ValidateCancelOrderDecorator>();
container.RegisterDecorator<IHandler<ShipOrder, int>, ValidateShipOrderDecorator>();

This is bad, because it causes a lot of maintainance on your Composition Root, and now you are starting to implement actual business logic into a specific decorator. This makes the business logic more complex than strictly required.

Instead, I propose a model where the decorator is generic, and delegates to underlying (and specific) objects. Using this validation example, for instance, think about doing something as follows:

// NOTE: Generic decorator
container.RegisterDecorator(typeof(IHandler<,>), typeof(ValidateHandlerDecorator<,>));

// Register another decorator. Decorators are always wrapped in order of registration. This means
// that this decorator will wrap the ValidateHandlerDecorator. The order in which decorators are
// executed is often of outmost importance.
container.RegisterDecorator(typeof(IHandler<,>), typeof(SecurityHandlerDecorator<,>));

The generic decorator is implemented as follows:

public class ValidateHandlerDecorator<TIn, TOut> : IHandler<TIn, TOut>
{
    private readonly IEnumerable<IValidator<TIn>> validators,;
    private readonly IHandler<TIn, TOut> decoratee;

    public ValidateHandlerDecorator(
        IEnumerable<IValidator<TIn>> validators,
        IHandler<TIn, TOut> decoratee
    {
        this.validators = validators;
        this.decoratee = decoratee;
    }

    public async Task<TOut> Handle(TIn input)
    {
        var errors = (
            from validator this.validators
            from result in validator.Validate(input)
            select result)
            .ToArray();

        if (errors.Any()) throw new ValidationException(errors);

        return await this.decoratee.Handle(input);
    }
}

In this case, the ValidateHandlerDecorator<TIn, TOut> is just a generic piece of infrastructure that delegates the actual validation to an injected collection of IValidator<TIn> implementations. Such IValidator<T> could be defined as follows:

public interface IValidator<T>
{
    IEnumerable<string>Validate(T input);
}

This allows the actual validation to be moved to these IValiadator<T> implementations, for instance:

public CreateOrderValidator : IValidator<CreateOrder>
{
    public CreateOrderValidator(IMyUnitOfWork uow) { ... }

    public IEnumerable<string> Validate(CreateOrder input)
    {
        if (input.Id == Guid.Empty) yield return "Id is required.";
    }
}

Because your validator implementations are now declared using the generic IValidator<T> abstraction, it becomes trivial to register them:

container.Collection.Register(typeof(IValidator<>), typeof(CreateOrderValidator).Assembly);

There are a lot of variations you can apply here. For instance:

Central point here, though, is that you should:

  1. Now I have classes of the form ... CmdHandler only collect the pipeline, and the logic itself is processed in a separate class IHandler. But it seems that I understood that this class should describe the logic of processing a yuzkeys, and decorators should wrap it. Am I thinking right?

Not sure I understand, but I think that the referenced articles should make things clear.

VladSnap commented 5 years ago

Hi, Steve!

Thank you very much for your answer! I will study your articles, thanks for the links. You answer the questions in great detail, this is amazing, thanks again!