Decorators unable to use built-in relationships (Lazy, Func) for the decorated service

[fuine6]

Sorry, my english is poor.
So just show my code.

public interface ICommand<out TResult> { }

interface ICommandHandler<in TCommand, TResult> where TCommand : ICommand<TResult>
{
	Task<TResult> Handle(TCommand command, CancellationToken cancellationToken);
}

class CreateInvoiceCommandHandler : ICommandHandler<CreateInvoiceCommand, CreateInvoiceDTO>
{
	private readonly IInvoiceRepository _invoiceRepository;

	public CreateInvoiceCommandHandler(IInvoiceRepository invoiceRepository)
	{
		_invoiceRepository = invoiceRepository;
	}

	public async Task<CreateInvoiceDTO> Handle(CreateInvoiceCommand command, CancellationToken cancellationToken)
	{
		return new CreateInvoiceDTO();
	}
}

class LazyCommandHandlerDecorator<TCommand, TResult> : ICommandHandler<TCommand, TResult> where TCommand : ICommand<TResult>
{
	private readonly Lazy<ICommandHandler<TCommand, TResult>> _commandHandler;

	public LazyCommandHandlerDecorator(Lazy<ICommandHandler<TCommand, TResult>> commandHandler)
	{
		_commandHandler = commandHandler;
	}

	public async Task<TResult> Handle(TCommand command, CancellationToken cancellationToken)
	{
		return await _commandHandler.Value.Handle(command, cancellationToken);
	}
}

And I use this to register:

public class ApplicationModule : Autofac.Module
{
	protected override void Load(ContainerBuilder builder)
	{
		builder.RegisterAssemblyTypes(Assembly.GetExecutingAssembly())
			.Where(x => !x.IsGenericType)
			.AsClosedTypesOf(typeof(ICommandHandler<,>));

		builder.RegisterGenericDecorator(typeof(LazyCommandHandlerDecorator<,>), typeof(ICommandHandler<,>));
	}
}

When LazyCommandHandlerDecorator is create,
the parameter in constructor is LazyCommandHandlerDecorator,
i wish it is CreateInvoiceCommandHandler,
what mistake did i make?

[tillig]

builder.RegisterGenericDecorator(typeof(LazyCommandHandlerDecorator<,>), typeof(ICommandHandler<,>));

You're decorating all of your command handlers - wrapping them. If you don't want it decorated, don't register the decorator.

[tillig]

Correction - I see what you're saying now.

Don't register the decorator in the RegisterAssemblyTypes call. You'll have to register all BUT that one.

[tillig]

For future questions, please ask on StackOverflow as noted in the issue template. You will get a faster and more accurate response there.

[fuine6]

I change my register code,
but it is same problem happened.

public class ApplicationModule : Autofac.Module
{
	protected override void Load(ContainerBuilder builder)
	{
		builder.RegisterType(typeof(CreateInvoiceCommandHandler)).As(typeof(ICommandHandler<CreateInvoiceCommand, CreateInvoiceDTO>));

		builder.RegisterDecorator(typeof(LazyCommandHandlerDecorator<CreateInvoiceCommand, CreateInvoiceDTO>), typeof(ICommandHandler<CreateInvoiceCommand, CreateInvoiceDTO>));
	}
}

If i remove the Lazy<> in LazyCommandHandlerDecorator. like this

class LazyCommandHandlerDecorator<TCommand, TResult> : ICommandHandler<TCommand, TResult> where TCommand : ICommand<TResult>
{
	private readonly ICommandHandler<TCommand, TResult> _commandHandler;

	public LazyCommandHandlerDecorator(ICommandHandler<TCommand, TResult> commandHandler)
	{
		_commandHandler = commandHandler;
	}

	public async Task<TResult> Handle(TCommand command, CancellationToken cancellationToken)
	{
		return await _commandHandler.Handle(command, cancellationToken);
	}
}

The Decorator work fine,
but loss Lazy feature which i need.

[tillig]

Have you tried with the latest 4.9.2 release of Autofac?

[fuine6]

I update 4.9.1 to 4.9.2 from nuget,
and the problem is same.

[tillig]

Turns out there is a bug here. I've added a minimal repro test to the suite.

[Fact(Skip = "Issue #967")]
public void DecoratorParameterSupportsLazy()
{
    var builder = new ContainerBuilder();
    builder.RegisterType<ImplementorA>().As<IDecoratedService>();
    builder.RegisterDecorator<DecoratorWithLazy, IDecoratedService>();
    var container = builder.Build();

    var instance = container.Resolve<IDecoratedService>();

    Assert.IsType<DecoratorWithLazy>(instance);
    Assert.IsType<ImplementorA>(instance.Decorated);
}

private interface IDecoratedService : IService
{
    IDecoratedService Decorated { get; }
}

private interface IService
{
}

private class DecoratorWithLazy : IDecoratedService
{
    public DecoratorWithLazy(Lazy<IDecoratedService> decorated)
    {
        this.Decorated = decorated.Value;
    }

    public IDecoratedService Decorated { get; }
}

I'm not sure how to fix it off the top of my head, but @alexmg just fixed #965 which is also decorator related, unclear if maybe something might pop out at him as being how to fix this one. I'm personally a little swamped so I can't dive in at the moment.

[tillig]

Turns out this happens with Lazy<T> and Func<T> if you use those in the parameter for the decorator. I've updated the tests and the title of the issue to include that.

[alexmg]

The problem here is that the when resolving IDecoratedService the first thing that occurs is an instance of ImplementorA is created. That is expected because it is the default registration for IDecoratedService. Once the instance of ImplementorA has been created the decorator DecoratorWithFunc is created with the expectation that the inner ImplementorA instance will be provided to its constructor. Instead of the original instance being provided a Func<IDecoratedService> is requested instead and the original implementation is now orphaned. The default implementation for IDecoratedService remains ImplementorA so when the Func is invoked in the constructor of the DecoratorWithFunc class it attempts to create another instance of ImplementorA. That new instance is identified as requiring decoration and the resolve operation goes into a loop because of the cycle.

What is being asked for here is that the decoration process be reversed, with the decorator class getting created before the component being decorated, and the decorated instance instead being provided to it in a late bound/lazy manner. I think the real issue is that the cycle is not being detected by the existing mechanism and that we should be checking that the decorator class actually contains a parameter that will accept the instantiated instance directly, and not via Lazy or Func which implies a reversed relationship.

The original issue with the Lazy is similar but the cycle does not occur in the constructor. It is delayed until the Lazy is accessed but a similar scenario unfolds with the implementation being created and then decorated again.

The entire decorator mechanism is based around resolving the implementation type first and then finding and applying any relevant decorators on top. I think supporting that and this alternative reverse decoration process is just too much. Extend that reverse decoration concept to a longer decorator chain and my head to starts to hurt thinking about decorators in the middle of the chain also being lazy or factories to other decorators. This just doesn't feel like something we can support so the appropriate error handling will need to be put in place to prevent it from happening.

[alexmg]

I'm not going to make any changes at this point and will think about the scenario for a while. I can see the desire to structure things like this but cannot see how the implementation would work yet.

[RaymondHuy]

Hi @alexmg @tillig what is your final decision for this issue ? Add the error handling for this case or try to support it ? I can spend my time solving this issue based on your decision :)

[johneking]

I've been looking at this one a bit because of the connection to a few other items, and have a few thoughts. Apologies if this is ground that's been covered or it gets off track, I'm happy to be corrected!
A couple of initial points:

1. is Allow decorators to take relationship types #1000 a duplicate?

2. in @tillig's example, I think it adds a bit of clarity to not take the .Value of the Lazy<T> in the constructor of DecoratorWithLazy. In the original example, a reference to the Lazy<T> itself is stored and then accessed later when methods are called. Resolving .Value as soon as the parent class is created somewhat hides the laziness of it, IMO.

Generally, I'm a bit confused by the semantics here and not sure if that actually points to a deeper problem. To expand on @alexmg's comment, if a decorator for a service T doesn't take T as a direct dependency, is it really a decorator for T? The problem Alex outlined kind of makes sense in that context - should you really expect a decorator to implicitly adapt between types, too? Or are any dependencies involving T but not directly T prone to cause circular dependencies and not make a whole lot of sense in terms of the original registrations? An extended syntax might be clearer, like:

builder.RegisterAdaptedDecorator<DecoratorWithLazy, Lazy<IDecoratedService>, IDecoratedService>();

where Lazy<IDecoratedService> is used to indicate the root service which needs to be resolved. Thinking about it that way though, it might not make sense in this form at all - for every component registered to a service T, we want a corresponding Lazy<T>, which we then expose back as a T, but because in the current setup the adaption from T -> Lazy<T> happens elsewhere, we can't directly replace the original instances of T. What happens if we have additional registrations for Lazy<T> which aren't even based on an underlying T? Seems like there are a few problems there.

I think a "real" decorator would need to do both parts of the process above, internally duplicating what LazyRegistrationSource does, perhaps explicitly with a reference to a ComponentContext and then adapt back to T. That would be pretty messy internally but would mean:

• some of Alex's concerns about late-binding would go away (since we always have a T of some sort), although the process would still need to happen "in-reverse", because in the current setup there still wouldn't be a use for the undecorated T which gets resolved first at the moment
• I think it implies that there could only be one deferred resolution which happens, and it would have to be first in the chain, because it would reset the decoration stack whenever Func<> or Lazy<> retrieves a T from a component context. But maybe that makes sense conceptually? Even if not, it might be something we can live with
• the decorator filtering might need a review since we wouldn't have an underlying base instance of T to use

The suggestion in #1000 is to potentially construct a single decorator as a registration source. But presumably that's why the newer decoration structure exists, because:

a) it's tricky to resolve different components for T at different points inside the decoration process
b) it's tricky to only return components which have been decorated, filtering the originals

However, using the old adapter syntax (with a few modifications to open it up a bit more for keys), the following works, making use of the existing built-in adaption from T -> Lazy<T>:

public void AdapterPairsCanMimicDecoration()
{
	var builder = new ContainerBuilder();
	builder.RegisterType<ImplementorA>().Keyed<IDecoratedService>("inner");
	builder.RegisterType<ImplementorB>().Keyed<IDecoratedService>("inner");

	builder.RegisterAdapter<Lazy<IDecoratedService>, IDecoratedService>((c, p, lazy) => new DecoratorWithLazy(lazy), "inner", "outer");

	var container = builder.Build();
	var instances = container.ResolveKeyed<IEnumerable<IDecoratedService>>("outer").ToList();
	Assert.Equal(2, instances.Count);
	Assert.IsType<DecoratorWithLazy>(instances.First());
	Assert.IsType<DecoratorWithLazy>(instances.Last());
	Assert.IsType<ImplementorA>(instances.First().Decorated);
	Assert.IsType<ImplementorB>(instances.Last().Decorated);
}

Could something like that be a solution in the short-term? It's not all that neat, particularly because it changes the usage to require ResolveKeyed, but is at least functional.

A final point to summarise after thinking about this for a while: overall the desired functionality seems to be "the ability to defer the resolution of a service until its members are used, but without changing its interface". That does seem like a pretty special case - some questions:

• Is it enough to warrant special-casing this as part of Autofac, given that it might well still be more intuitive in applications to use Lazy or similar for this scenario? Is using the same interface for this particular type of decoration actually misleading or harmful? Going back to the original post, what's the reason for needing all command handlers to be Lazy - or is there actually just one internal dependency which should be wrapped explicitly in a Lazy<>, like

private readonly Lazy<IInvoiceRepository> _invoiceRepository;

• If we did implement this, should it be a unique decorator which can only be applied "first", which uses DynamicObject or similar internally? For example, syntax-wise something like .LazyInstatiation().
• Can we actually say "decorators for a service should only ever accept a single (direct) dependency on that service anywhere in the dependency hierarchy", or is that not right? I can't think of counter-examples.

I have some other thoughts connected to how adapters, decorators and potentially composites relate to each other too but may write that up elsewhere since it's a bit more general and cuts across a few other issues.

[fuine6]

• Is it enough to warrant special-casing this as part of Autofac, given that it might well still be more intuitive in applications to use Lazy or similar for this scenario? Is using the same interface for this particular type of decoration actually misleading or harmful? Going back to the original post, what's the reason for needing all command handlers to be Lazy - or is there actually just one internal dependency which should be wrapped explicitly in a Lazy<>, like

private readonly Lazy<IInvoiceRepository> _invoiceRepository;

• If we did implement this, should it be a unique decorator which can only be applied "first", which uses DynamicObject or similar internally? For example, syntax-wise something like .LazyInstatiation().
• Can we actually say "decorators for a service should only ever accept a single (direct) dependency on that service anywhere in the dependency hierarchy", or is that not right? I can't think of counter-examples.

Suppose there will be expensive costs on instance CommandHandler, and I want to do some validation in decorator, if the command has an error, I don't need to instance the CommandHandler.

class LazyCommandHandlerDecorator<TCommand, TResult> : ICommandHandler<TCommand, TResult> where TCommand : ICommand<TResult>
{
	private readonly Lazy<ICommandHandler<TCommand, TResult>> _commandHandler;

	public LazyCommandHandlerDecorator(Lazy<ICommandHandler<TCommand, TResult>> commandHandler)
	{
		_commandHandler = commandHandler;
	}

	public async Task<TResult> Handle(TCommand command, CancellationToken cancellationToken)
	{
		if(command.HasErrors())
		{
			throw new Exception("...");
		}

		return await _commandHandler.Value.Handle(command, cancellationToken);
	}
}

This is my idea at the time, but now I am not using.

[johneking]

Yep I see, seems like a reasonable requirement - although I would still question why all the command handlers need to act like they are Lazy instead of whichever internal object is expensive to create. For example, even if every command handler loaded data from a remote service before use, would it be clearer to make the set of data explicitly Lazily-loaded? Not saying it negates the need for a feature like this necessarily, just trying to fully understand the use-case.

Also, just to back-track a bit on my statement above about only taking direct dependencies - seems like that's probably wrong. I could see an approach where the decoration process itself is what creates a new Func or Lazy which points to the rest of the decoration process (for a specific registration) and eventual resolution from a component context, and that's what gets used to resolve constructor dependencies on those wrappers, as opposed to any registrations from elsewhere (such as the built-in registration source). Those might still be special cases though and am still thinking that potentially you can only take dependencies via relationships if the decoration process explicitly handles them itself.

[fuine6]

although I would still question why all the command handlers need to act like they are Lazy instead of whichever internal object is expensive to create.

Indeed, not all are expensive, but I hope that object don't care if it's lazy, object are dependent on interface, it may not understand whether building objects is expensive.

[johneking]

I've followed through some of the proposed approach above and got something for this which works, but there are a few things to note about the whole setup and a couple of queries.

Generally the approach is to switch the decoration process from iterative to recursive and then special-case dependencies on Func and Lazy. The recursive aspect allows us to build part of the decorator chain even when we defer the resolution of the underlying component instance, but in simple scenarios the resolution order just degenerates to exactly the same as the existing iterative process anyway. (side note: I actually intended to use something like this to enable the composite pattern for #970 via IEnumerables but it turns out it gets very complicated for a few reasons, not least when we have deferred resolution at the same time)

A side-effect I think might be unavoidable here (I think @alexmg also hinted at this): decorator conditions and the decorator context in general stop making sense altogether at certain points in the decorator hierarchy if you have deferred resolution. Say we have a normal chain of dependencies like:

DecoratorA ->
	DecoratorB ->
		DecoratorC ->
			ImplementationD -> [internal dependencies]

In either the existing or my new version, we just resolve with the order D, C, B, A, passing a context along in that order which records that same sequence of decorators being applied. Adding in a Func<> dependency:

 DecoratorA ->
 	DecoratorB ->
 		Func<DecoratorC> ->
 			ImplementationD -> [internal dependencies]

As in the previous post, IMO the Func<> there should not only be related to the specific registration we're resolving, but also ideally captures the remaining part of the decoration process so that the decorators themselves can be lazily resolved. That's possible but now the sequence ends up being:

B, A - this is available immediately
D, C - resolved when called within B

What does the decorator context look like when we're attempting to resolve B? There's no info about the underlying instance or other decorators - even knowing about the list of decorators which may apply and a core component registration, we don't know which decorators will eventually be applied. So in this setup I think we just need to be aware that conditions and contexts necessarily stop working across parts of the sequence which are deferred. Startables won't start etc., but that's probably the desired behaviour.

The other main issue is the special-casing aspect. As above, I've become convinced that any Func<> or Lazy<> needs to be created by the decoration process, but that means that we may need to scan for constructor dependencies, and that falls over with lambdas. Not sure if there's a clever way around it but I'd be tempted to say it's a stretch too far to support Lazy<> or similar when using the delegate syntax, or trying to resolve parameters for those. The other option might be to have an explicit command when registering, e.g. RegisterLazyDecorator and then hoping that the required dependencies line up.

@alexmg / @tillig are you able to review the working here and the assertions I've made along the way as part of this solution? Specifically:

• we can't rely on the existing Lazy and Func relationships when used for dependencies in decorators
• decorator contexts and conditions stop working at certain points in any scenarios with deferred resolution
• we're okay to assert that Lazy and Func are only supported when resolving decorators registered as types (? - or maybe specific registration commands are better?), and further that each decorator will only accept one dependency on the service being decorated.

A few provisos there but I have the draft code here with all tests passing: https://github.com/johneking/Autofac/tree/Decorators

It needs tidying, more validation and some extra tests but I can create a PR if the logic generally makes sense.

[alexmg]

@johneking I have done some refactoring around the decorator feature to address a number of other issues that seem like more pressing things to be addressed. It would be interesting to know if this makes things easier or harder for you in your branch.

There have been a lot of issues around the new decorator support compared to the original implementation which was very explicit in its nature. If some of these less common scenarios can be addressed using the more explicit key based decorator chains then that may be a perfectly reasonable solution for such cases.

It seems very easy to fix one thing and break another as things currently stand. I am concerned that making the implementation even more complicated will make fixing other issues that arise in the future a lot more difficult. There have been a number of occasions in the past were we have been too accommodating of fringe scenarios and ended up paying a price for it down the road.

The decorator context is a new feature specific to the new decorator support and I would like to keep that functional. It is very much based on the idea that the decorator chain is materialized as it is being applied. I also agree that the constructor of an object should not be doing anything expensive and if it is then some refactoring is likely the right answer to the problem.

[johneking]

Cool, I'll get the refactored version and make a PR out of it for completeness - whether that'll be worth merging or not I'm not sure. Agree that this function superficially doesn't seem like much of an extension, but on closer inspection is both an edge-case in terms of functionality (compared to alternatives) and does add a decent amount of complexity even just on a conceptual level. I'd suggest that at least it would be worth adding something to the documentation about limitations on classes which can be used for decoration, specifically in regard to constructor requirements and adaption, since it looks like this has come up a couple of times recently.

That said, the solution I'll submit is mostly just a generalisation of the existing code and when it doesn't involve deferred resolution, it results in a process which is mechanically the same. It's only when using Func<> dependencies (which I'll actually make explicit) that the context breaks down, although it will still return >1 nested contexts which are filled-in at different times. Generally, it might add some flexibility for connected issues in future, so may be worth a look.

[tillig]

Something I worry about that comes up a lot is special-casing around particular relationship types - the notion of "if we're not resolving (Func | Lazy | Owned | IEnumerable | IIndex [pick one or more]) then do XYZ, otherwise..." As the framework evolves and we need to look at new relationship types (I'm waiting for something like ValueEnumerable<T> the same as there's now a ValueTask<T>) having any code from one feature that ties to another feature with special casing seems like it's ripe for problems.

Not to mention the recent "I registered my own instance of Func<T> and that needs to take precedence over the built-in Func<T>" issue I remember reading.

It's like there needs to be a flag on the registration that says "this fulfills a special built-in relationship" which may be covered by the IsAdapterForIndividualComponents thing @alexmg just added to the context but I'm not sure. Similar issues for special casing have come up in things like the dynamic proxy support and aggregate service support, so whatever comes out here should ostensibly be something that could be used by extensions.

I'm also sort of weirded out by what the expected behavior of a Func<T> might be in the middle of a decorator chain. There's no guarantee the decorator isn't going to call that thing twice, which makes me wonder what the behavior should be. And if there's a chain of decorators all of which use Func<T> and if all of them potentially call that method more than once because people do some crazy stuff sometimes.... maybe I'm overthinking it.

[johneking]

IMO the requirement for special-casing here is somewhat inevitable due to the nature of the decoration process, specifically that it involves that chain of ordered steps - that doesn't come up elsewhere. The particular instance we want in the middle of a chain of decorators isn't the same as if we resolved directly from a container, and it's not easy to provide component-specific adapters to handle some of the relationships listed even when adaption can be included in the sequence. It looks like with that new flag we could adapt from T -> string for example, which is great. We could also construct a Func<T> from a T, but it wouldn't be the one we want - that one eventually has to point back to a component context AND account for additional decorators, which not even the component context knows about. That trickiness seems pretty fundamental and I imagine extends to at least some of the other relationship types.

The special cases for Func and Lazy do work in my updated code here though, if you can see my decorator branch . Not sure if something similar could be applied for other relationship types - I suspect it wouldn't be neat or easy.

So that maybe puts a dampener on this whole feature conceptually. However, another way to look at supporting something like it would be to jump back a step to having a single setting per registration which defers the creation of the underlying component until it's used. Thinking about that, it really might not be too hard - a dynamic proxy which is basically just a wrapper around a Lazy which uses a factory delegate resolved from a scope. That factory could possibly even include the whole decorator chain on top, so that the decorator context would always make sense. Maybe it would even be generic enough to apply a proxy to any registration...

On Func<T> behaviour in my sample implementation: calling a Func twice in its current state would just recreate any decorators (contained within the Func at least) and re-resolve the underlying component. The decorators at the top just stay unchanged. Not sure if that's an issue. I think big stacks of nested Funcs would continue working too - each nested Func would just get recreated by its parent. It's still pretty weird but does actually seem functional. I've added a passing test (note - the accessor for Decorated points to the Func here):

[Fact]
public void DecoratorParameterSupportsNestedFunc()
{
	var builder = new ContainerBuilder();
	builder.RegisterType<ImplementorA>().As<IDecoratedService>();
	builder.RegisterDecorator<DecoratorWithFunc, IDecoratedService>(null, DecoratorAdaptionType.Func);
	builder.RegisterDecorator<DecoratorWithFunc, IDecoratedService>(null, DecoratorAdaptionType.Func);

	var container = builder.Build();

	var instance = container.Resolve<IDecoratedService>();
	var firstGeneratedDecorator = instance.Decorated;
	var secondGeneratedDecorator = instance.Decorated;
	Assert.NotEqual(firstGeneratedDecorator, secondGeneratedDecorator);

	Assert.IsType<DecoratorWithFunc>(instance);
	Assert.IsType<DecoratorWithFunc>(instance.Decorated);
	Assert.IsType<ImplementorA>(instance.Decorated.Decorated);
}

I've tidied up the code but not created a PR yet after all, I won't go too far into covering all the usage permutations if conceptually this isn't a path we want to go down. It might be worth a quick review in any case, it's both a solution to this issue and also an example of the kind of pipelining outlined in #970.

In regard to precedence of registrations: that does actually seem like a pretty global improvement which is somewhat independent. Is it worth spinning off a separate issue for that?

[sglienke]

I think there is a misunderstanding here: a thing that acts as a T but gets a Lazy injected is not a decorator but a proxy. A decorator by definition always only gets a T injected that it decorates.

[johneking]

The OP was looking for something which functionally acts like decoration, essentially that for any definition of T, return a new T - but instantiate it lazily. It's a bit of a niche use-case but does seem to be a thing, even if there are some questions over the semantics.

IMO it even makes some sense as a general function - specifying decorators which internally use an adapter. At any point in the decorator hierarchy you can define a "decorator" which takes a definition of T, adapts to something else, then uses that as the dependency in a class which exposes a new T. That's where the existing mechanism falls down though - the pair of adaptations from and back to the original type need to be specified as a single unit, and that's not possible with the built-in adapters. So in that sense I agree that just trying to use the existing syntax to achieve decoration using an adapter isn't really decoration, and that should potentially be made clearer in the documentation.

But the general use-case might still be valid and could be achieved by extending the decorator syntax to specify a pair of adapters, or one built-in plus one custom adapter (as per the original scenario). Func<> and Lazy<> still require an even-more-special adapter though and that needs a few other changes, but it does seem possible as per my example.

[sglienke]

I agree that the usecase is valid - however the direction or order of dependencies is opposite here as it is with a decorator. With a decorator you first have T and then you wrap the decorator around it. With this case you don't have a T at that point which caused the issue in the first place. If nothing is being called you might even never have a T.

It sounds more like a new relationship where the fact that it is lazy initialized is not leaking to the consumer as it does with injecting Lazy<T>. That is why in my oppinion it should be called proxy and not decorator.

[tillig]

This does not appear to have been fixed, as yet, in the v6 codeline; I tried running our [currently skipped] tests for this situation against the v6 codeline and it yields a StackOverflowException. That said, it may be [more] possible to fix the issue in the v6 codeline than it was previously. /cc @alistairjevans for FYI.

[alistairjevans]

I'll take a look at this; my instinct is that it won't be a quick fix for the overall problem; would require substantial changes to the decorator middleware to inject lazy/func instead of the already-resolved instance.

That being said, it probably shouldn't result in a StackOverflowException; that feels messy. Would like to see why that's not getting caught in our normal circular dependency detection.

[alistairjevans]

Urgh, okay, so the tests hit a StackOverflow because they access the Lazy/Func inside the constructor of the decorator, which in turn resolves a new instance of itself, which in turn calls the decorator, etc etc. Because each resolve by the Lazy/Func happens against the lifetime scope as a fresh operation, they have their own separate circular dependency tracking, and don't object to the infinite recursion. Not a lot we can do about that, but if you are accessing the Lazy/Func in the constructor, you should just inject the service normally.

I'm inclined to agree with @sglienke pretty strongly here; this relationship that is being described, where something takes a Lazy<T> or Func<T> as a constructor parameter, instead of a concrete instance, is not a decorator, which has a clear contract to decorate a concrete instance of a service. It's something else, like a Proxy.

In addition , as @alexmg indicates, what is being described here is a complete reversal of the decorator process, where we would first check what the decorator wants before proceeding down the pipeline. Besides the level of change described here, it's probably mutually exclusive with the idea of decoration predicates, which needs to know the instance of an object before it decides to decorate it. Chicken and egg.

We can slightly abuse the new composites functionality in v6 to get an approximation of the desired relationship; but this requires the proxy to 'pick' the instance it wants, and sort of falls down if you want to get a collection of all services while still proxying them.

[Fact]
public void ProxyAClass()
{
    var containerBuilder = new ContainerBuilder();

    containerBuilder.RegisterType<Implementation>().As<IMyService>();

    // Composite (that will choose to only expose 1 instance).
    containerBuilder.RegisterComposite<Proxy, IMyService>();

    var container = containerBuilder.Build();

    var composite = container.Resolve<IMyService>();

    composite.DoSomething();
}

public interface IMyService
{
    void DoSomething();
}

public class Proxy : IMyService
{
    private readonly Lazy<IMyService> _lazyRef;

    public Proxy(IEnumerable<Lazy<IMyService>> allLazys)
    {
        // Pick the last one.
        _lazyRef = allLazys.Last();
    }

    public void DoSomething()
    {
        _lazyRef.Value.DoSomething();
    }
}

public class Implementation : IMyService
{
    public void DoSomething()
    {
    }
}

In summary, to support this we would need to define a new relationship type, like a Proxy, which works similar to composites in some way, but it definitely isn't an extension of the decorator feature.

Personally, I'm in favour of closing this issue, and opening a new feature request issue to define what a 'Proxy' looks like (or whichever name we think is appropriate).

[tillig]

I'll buy the argument that this isn't a decorator and could be classified as a proxy or something else. I think a new feature request with some discussion to flesh out the concept would be fine, and it shouldn't hold up v6 getting out the door. I'll get something written up shortly.