Make IDecoratorContext extend IComponentContext.

[alistairjevans]

Fairly simple change this one; added a test to prove resolve from decorator conditional is possible.

Also added a test to prove that if a decorator resolves itself from inside the conditional, it throws a circular dependency exception (rather than stack-overflowing or something).

Closes #1338.

[codecov]

Codecov Report

Base: 77.93% // Head: 77.98% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (ed66591) compared to base (3465490).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1352      +/-   ##
===========================================
+ Coverage    77.93%   77.98%   +0.05%     
===========================================
  Files          195      195              
  Lines         5583     5587       +4     
  Branches      1119     1119              
===========================================
+ Hits          4351     4357       +6     
+ Misses         717      716       -1     
+ Partials       515      514       -1     

Impacted Files	Coverage Δ
...rc/Autofac/Features/Decorators/DecoratorContext.cs	100.00% <100.00%> (ø)
...Autofac/Features/Decorators/DecoratorMiddleware.cs	90.47% <100.00%> (ø)
src/Autofac/Util/SequenceGenerator.cs	100.00% <0.00%> (+28.57%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tillig]

The only question I have on this one is whether it's actually interesting to have IDecoratorContext implement IComponentContext or if it's enough to just have a new property that exposes the component context.

That is, instead of
IDecoratorContext : IComponentContext
it's
public IComponentContext ComponentContext { get; private set; }

I don't really feel strongly one way or the other, just thought it reasonable to ask the question. Having it derive does provide a shorter syntax for resolution, but it also sort of breaks down the "composability" notion - instead of composing two things together, the two things are sort of merged. Reminds me a little of how ASP.NET has things like ControllerContext where there are convenience properties to access HttpContext things, but ControllerContext is not, itself, an HttpContext.

[alistairjevans]

I do understand your point; I was trying to match the form of IComponentContext being passed directly to the delegate activator. It feels like users may "expect" (citation needed) that a "context" object passed to a mid-resolve method have direct Resolve methods. To be honest, the reason I agreed with the original issue fairly quickly is that it felt "natural" that IDecoratorContext should be able to resolve, and I was sort of surprised that it couldn't.

I think there are two differences between this and HttpContext composability.

First is perhaps about the complexity/size of the HttpContext contract? In Autofac, the IComponentContext contract is actually pretty tiny; just one property and one method. HttpContext is 12 total? I don't have a firm line for "this is too big a contract to add to a type", but I guess I know it when I see it?

Second, HttpContext is an abstract class, so deriving from it limits classes from deriving from a different, more appropriate, base type. Not the case with interfaces. I think I'd lean towards composability if our IComponentContext was actually an abstract class.

[tillig]

Sold. Seems like a reasonable way to go.

[tillig]

🐻‍❄️

[tillig]

I'll let you delete the feature branch so it doesn't disappear out from under you.