Improve ambient context-less support in decorators #364

Closed
dotnetjunkie opened this Issue Dec 28, 2016 · 5 comments

Projects

None yet

2 participants

@dotnetjunkie
Collaborator

Since v3.2 (or specifically #226) Simple Injector has support for working with scopes that flow through the object graph instead of being held in a AsyncLocal or ThreadLocal storage.

This support however breaks down when working with decorators that use decoratee factories, as shown in the following example:

sealed class CommandHandlerProxy<T> : ICommandHandler<T>
{
    private readonly Func<ICommandHandler<T>> commandHandlerFactory;

    public void Handle(T cmd) {
        ICommandHandler<T> handler = commandHandlerFactory();
        handler.Handle(cmd);
    }
}

Since the Func<ICommandHandler<T>> will be generated by Simple Injector as a singleton, it becomes impossible for any previously defined Scope to flow through the object graph to the dependencies of this decotator (including the decoratee).

The typical solution would be to inject the Scope into the class and resolve from the Scope, but that obviously won't work when resolving the decoratee, because that would lead to a circular reference and would cause a stack overflow.

This could be solved by allowing the Func<ICommandHandler<T>> itself to get constructed using the Scope instance, which means that the Func<T> itself should be either transient or scoped itself, since a func injected into another decorator instance would need a different Scope. This however would complicate the design a lot, since currently the Func<T> is always a singleton. It would mean that the decorator sub system should start to analyze the remaining object graph to find out whether Scope is being injected anywhere, and if so, the Func<T> dependency should hold the Scope and become itself a Scoped registration at most.

This will be really complicated and could even be confusing to users. The only other solution I can think of that would possibly work is to allow the injection of an Func<Scope, ICommandHandler<T>> into decorators:

sealed class ScopedCommandHandlerProxy<T> : ICommandHandler<T>
{
    private readonly Scope scope;
    private readonly Func<Scope, ICommandHandler<T>> commandHandlerFactory;

    public void Handle(T cmd) {
        ICommandHandler<T> handler = commandHandlerFactory(scope);
        handler.Handle(cmd);
    }
}
@dotnetjunkie dotnetjunkie added this to the Backlog milestone Dec 28, 2016
@TheBigRic

Making the decorate factories decorator (how do we call this thing...) anything shorter than singleton is a no go for me.

As you know I like my complete object graph to be singleton. Most of the time this doesn't work when working with EF's DbContext. So I currently work with a design where almost everything is singleton and somewhere deep in the stack I switch from singleton to scoped using these factories.

Making this factories non-singleton would cause a captive dependency and thus will result in a verification exception.

I like the idea of Func<Scope, ICommandHandler<T>>. It is however a very technical, probably very hard to document, solution.

Another solution would be to let Simple Injector provide a CurrentScopeProvider, which is ofcourse a singleton. Advantage would be that it will be pretty simple to do some diagnostics on this.

Something like

if constructor.Contains(Scope) and constructor.Type.IsSingleton
{
    throw new InvalidOperationExceptioin("Use SimpleInjector.CurrentScopeProvider instead 
       when working with components with a lifetime other than 'Singleton'");
}
 
@dotnetjunkie
Collaborator

Another solution would be to let Simple Injector provide a CurrentScopeProvider, which is ofcourse a singleton.

This is impossible, because that would reintroduce an ambient context in the background, which is the thing this feature tries to 'solve'.

I like the idea of Func<Scope, ICommandHandler<T>>. It is however a very technical, probably very hard to document, solution.

That's true. This whole ambient context-less idea is very complex and this is exactly what Autofac users are struggling with on a day-to-day basis.

@TheBigRic

That's true. This whole ambient context-less idea is very complex and this is exactly what Autofac users are struggling with on a day-to-day basis.

The existing of this ambient-less scope solution isn't documented I think? So do we know if users are actually using this? In other words, do we really need this feature, especially with all movements surrounding .Net Core, .Net standard, UWP apps etc. Which platforms lack the existence of an ambient context nowadays?

@dotnetjunkie
Collaborator

The existing of this ambient-less scope solution isn't documented I think?

There is no documentation yet.

So do we know if users are actually using this?

My best bet is that nobody is actively using this.

Which platforms lack the existence of an ambient context nowadays?

All new platforms support the existence of an ambient context. Since most new platforms are async top to bottom, they require the existance of AsyncLocal<T> or CallContext. AsyncLocal<T> is provided by every platform compatible with .NET Standard >= 1.3. This means .NET 4.6, .NET Core and UWP allow this.

This leaves some old platforms, which might not be that interesting to support.

@dotnetjunkie
Collaborator

Won't fix because #365

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment