Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection.Register set Lifestyle #688

Closed
VladSnap opened this issue Apr 23, 2019 · 8 comments
Closed

Collection.Register set Lifestyle #688

VladSnap opened this issue Apr 23, 2019 · 8 comments
Labels

Comments

@VladSnap
Copy link

VladSnap commented Apr 23, 2019

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
Copy link
Collaborator

dotnetjunkie commented Apr 24, 2019

Your question is unclear to me:

  • What is the meaning of IDecoratorHandler? Is that an abstraction specificically for decorators?
  • Do you wish to inject collections of those handlers into consumers or always just a single instance?

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

  • the definition of the IDecoratorHandler<TIn, TOut> abstraction
  • a two or three handler implementations (but possibly with an empty Handle method)
  • the decorator(s) in play
  • some typical usage of such handler within a consumer

@VladSnap
Copy link
Author

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 call Collection.Register, all my decorators have the lifestyle Lifestyle.Scoped, but they should be Transient (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 same OrderCreatedCmdHandler) .
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
Copy link
Collaborator

dotnetjunkie commented Apr 25, 2019

Hi VladSnap,

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

The short rule is:

  • You use Container.Register in case you only want to inject a single instance into a consumer (independently of how many implementations of the generic type you have)
  • You use Container.Collection.Register in case you want to inject a collection of instances into a consumer (e.g. IEnumerable<IEventHandler<OrderCreated>> or IReadOnlyCollection<IEventHandler<OrderCreated>>)

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:

  • You talk about 'decorators', but are actually using the chain of responsibility pattern. This is something different than a decorator
  • Those 'decorators' seem cross-cutting concerns, but you are injecting them into the constructors of your application components. This causes tight coupling and seems like a very fragile model

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

  • Instead of using chain of responsibility, used decorators instead. They are simpler to comprehend, easier to wire up, and can be created stateless
  • Instead of injecting cross-cutting concerns into concrete consumers, wrap them around those consumers (decoration). This is less fragile, and more maintainable
  • Keep all registrations Transient by default. This is easy when your components are stateless, and it simplifies your configuration considerably. Only leaf objects of the graph (e.g. DbContext) will have a lifestyle other than Transient.
  • Stop using MediatR as a reference for your own architecture. In later versions of MediatR, Jimmy moved away from using decorators to the more unfortunate chain of responsibility pattern, for the simple reason that not all DI Containers had great support for decorators. The current MediatR design tries to be a least common denominator model that works on all DI Containers. You, instead, should pick the design that is most suited for your application and, with that, pick the DI Container that suits your needs the most. My experience is that using decorators to implement cross-cutting concerns on top of generic abstractions is the cleanest and most maintainable design you can have. Simple Injector works extremely well with such a design.

@VladSnap
Copy link
Author

VladSnap commented May 2, 2019

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
Copy link
Collaborator

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

@VladSnap
Copy link
Author

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
Copy link
Collaborator

  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:

  • You can take a look at the generic type used by the implementation
  • You can only cache when the implementation is marked with an attribute
  • When the TIn message is marked with an attribute
  • Based on the name of the message (e.g. when the message ends with Query).

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:

  • You could hide the complexity of looping over the validators from the decorator behind a different abstraction. This will, especially, be useful when you separate your command handlers from your query handlers.
  • You could make your IValidator<T> asynchronous
  • In the example I registered a collection of IValidator<T>. Perhaps you always only have a single IValidator<T> implementation for a given T. In that case you inject IValidator<T> and register it using container.Register(typeof(IValidator<>), typeof(CreateOrderValidator).Assembly);.
  • Perhaps returning strings from IValidator<T> is not enough; you might want to consider returning ValidationResult objects of some sort.

Central point here, though, is that you should:

  • Keep decorators generic in nature. This makes comprehensible and simpler to wire
  • Define a separate abstraction (e.g. IValidator<T>) for important artifacts (such as validation logic). This makes your design more explicit, code simpler, easier to enforce coding style, and simpler to wire.
  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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants