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

Allow access to constructed container when deciding whether or not a decorator should be applied #1338

Closed
julealgon opened this issue Aug 10, 2022 · 2 comments · Fixed by #1352

Comments

@julealgon
Copy link

Problem Statement

There is support today for conditionally applying decorators by providing a lambda on the RegisterDecorator call. However, only information about the instance being decorated is provided there, so it is impossible to request something from the container that could be relevant to making the decision.

I have a scenario where I need to inspect an IOptionsSnapshot<T> object with a specific setting to decide whether or not a given decorator should be applied. This must be done everytime the decorator is attempted to be resolved, so that the setting can work "in real time" (i.e. without having to restart the application).

Desired Solution

The overload to Decorate that takes a decision predicate should be expanded to also provide the IContainer itself so that the caller can resolve services from the container at the time of the decision is made.

public static void RegisterDecorator<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TDecorator, TService>(this ContainerBuilder builder, Func<IDecoratorContext, bool>? condition = null)

This should have a variation such as:

    public static void RegisterDecorator<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TDecorator, TService>(
        this ContainerBuilder builder, 
        Func<IDecoratorContext, IContainer, bool>? condition = null)
        where TDecorator : notnull, TService

Alternatives You've Considered

As a poor-man's alternative, I used the overloads that don't take a concrete decorator (which expose IContainer) and then manually constructed my decorator there based on a service registered in the container, or used the instance value when no decoration was needed.

This works, but is more convoluted/less flexible.

builder.RegisterDecorator<IMyService>(
  (context, parameters, instance) => 
  {
    if (context.Resolve<IOptionsSnapshot<MyOptions>>().Value.EnableStuffing)
    {
        return new StuffedService(instance);
    }
    else
    {
        return instance;
    }
  });

This also becomes unwieldy when the concrete decorator takes more dependencies, as you have to also manually resolve each one.

@alistairjevans
Copy link
Member

OK, I can see the benefit of being able to resolve services inside the condition callback.

Might I propose though that instead of adding IContainer or a similar scope to the callback signature, we make IDecoratorContext derive from IComponentContext?

That means the decorator context would provide all the resolve functionality of IContainer/ILifetimeScope without modifying the signature, so you could do:

builder.RegisterDecorator<MyDecorator, IMyService>(
    context => context.Resolve<IOptionsSnapshot<MyOptions>>().Value.EnableStuffing)

Does that seem ok, and would you be willing to raise a PR to that effect? I actually don't think the changes are particularly complicated; when we create the DecoratorContext in DecoratorMiddleware we can pass in the ResolveRequestContext, and route all requests done via DecoratorContext through that provided request context.

@julealgon
Copy link
Author

Might I propose though that instead of adding IContainer or a similar scope to the callback signature, we make IDecoratorContext derive from IComponentContext?

That means the decorator context would provide all the resolve functionality of IContainer/ILifetimeScope without modifying the signature, so you could do:

builder.RegisterDecorator<MyDecorator, IMyService>(
    context => context.Resolve<IOptionsSnapshot<MyOptions>>().Value.EnableStuffing)

Does that seem ok...

That's perfectly fine in my book.

..., and would you be willing to raise a PR to that effect?

It is unlikely I'll be able to, but not impossible. I'm actually only dealing with Autofac here in a legacy integration, which is where I stumbled upon the situation I described. If I do, I'll let everyone know by replying here before starting though.

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

Successfully merging a pull request may close this issue.

2 participants