Take LifetimeScopeProvider as Ctor Parameter in AutofacDependencyResolver

[GoogleCodeExporter]

Right now the AutofacDependencyResolver in Autofac.Integration.Mvc accepts only 
a lifetime scope as a constructor parameter and uses that scope as the location 
from which resolutions should be performed.

public AutofacDependencyResolver(ILifetimeScope container)
{
  if (container == null) throw new ArgumentNullException("container");
  _container = container;
  if (!_container.TryResolve(out _lifetimeScopeProvider))
    _lifetimeScopeProvider = new DefaultLifetimeScopeProvider();
}

Unfortunately, at the time of construction (as seen above) the resolver 
attempts to actually do a resolution to retrieve the lifetime scope provider 
from the container. This causes problems for extensibility, like with the 
AutofacContrib.Multitenant library. For example, in a multitenant scenario, you 
may need a full ASP.NET request running in order to determine the tenant for 
which things are getting resolved, but since the ADR is doing a resolution 
right at app startup, that context doesn't exist.

A discussion on some of the troubles one user is running into is here:
https://groups.google.com/d/topic/autofac/EYBxcA6BIl8/discussion

While normally I'd not request a change be made so a contrib library will work 
better, it would be nice to be able to provide the lifetime scope provider to 
AutofacDependencyResolver as a constructor parameter and/or a get/set property 
on the class rather than having the value resolved from the container passed 
in. It would alleviate some of the confusion and make it easier to work with.

I would be happy to make the change myself if this issue is accepted. Just let 
me know.

Original issue reported on code.google.com by travis.illig on 21 Feb 2011 at 6:30

[GoogleCodeExporter]

Great suggestion - passing it as a parameter sounds like a good choice. We 
should probably keep (but deprecate) the existing mechanism for a release or 
two.

Original comment by nicholas...@gmail.com on 23 Feb 2011 at 10:05

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

The ILifetimeScopeProvider interface was made internal just before the release 
because we were not sure exactly how it would be used and decided to limit the 
API surface area. Some further refactoring also had to be done in that area due 
to issues accessing the HTTP module in medium trust scenarios, though that all 
happened after the decision to make it internal.

I have delayed the resolution of the ILifetimeScopeProvider until it is 
actually needed to ensure that a HTTP request is available. This is the first 
time the RequestLifetimeScope property is accessed, which should be the first 
call to the GetService or GetServices method.

Aside from being used for mocking the lifetime scope by the unit tests, I 
always figured the provider could be handy for things like the multitenant 
contrib. If the provider interface was made public again would it make 
implementing multitenant support easier? I just want to make sure that the only 
reason for wanting to provide your own instance was to avoid the issue of the 
HTTP request not being available. If it would make things easier then I am 
definitely happy to expose it.

Original comment by alex.meyergleaves on 23 Feb 2011 at 1:09

[GoogleCodeExporter]

The reason I'm interested in making it public is mostly around clarity and 
transparency. The HttpRequest stuff recently was a catalyst, for sure. It 
raises things like:

* Constructor side-effects. Simply by virtue of constructing the ADR, something 
gets resolved out of the scope passed in. Not a big deal, but definitely not 
expected, and the root of the recent discussion group thread. (Sounds like this 
was changed, so that's good.)

* Clarity. Without actually looking at the source, there's no way to know that 
something's going to be resolved from the container I pass in. It's not 
documented, so if the container I pass in does something else - logging on 
every resolution or something - I'm going to see some actions going on that I 
don't expect, even if that resolution is delayed until later.

* Being explicit. Using the multitenant container as an example, when you 
resolve something from the container it actually looks up which tenant is doing 
the resolution and THEN resolves. That means if, somehow, someone registers an 
ILifetimeScopeProvider for multiple tenants, the first tenant that accesses the 
application wins - that's the ILifetimeScopeProvider that gets registered. It 
then gets more confusing - how do I register a "global" provider and force the 
resolution to happen at the root container level and not at a per-tenant level?

* Testability. If I have tests for classes that actually reference 
DependencyResolver.Current, I will want to set them up using a container full 
of test types. If the ADR is always looking for 
RequestHttpModule.GetLifetimeScope() because of the default lifetime provider, 
it means I have to come up with a different AutofacDependencyResolver rather 
than a different lifetime provider. (We're actually doing this right now, in 
fact.)

It also occurs to me that if the ILifetimeScopeProvider is resolved from the 
container, but the interface is internal, then there's really no way anyone 
could register one, so there's not a lot of point (at least right now) in 
trying to resolve it at all. It'll always be the default provider.

Finally, while we're talking about visibility, it'd be nice to have the 
ILifetimeScope passed into the ADR be a public get property, also for testing 
and for some extensibility. For example, there are places in our application 
where I need to have a special case around multitenancy and ensure I'm 
resolving from the root application container. That means I need to get the 
ADR's ILifetimeScope, try to cast it to MultitenantContainer, and if that 
works, resolve from the root/application container and not from a 
tenant-specific lifetime. But, since I can't get the ILifetimeScope... it means 
doing some mean looking reflection to get the private _container field and do 
the same thing.

Original comment by travis.illig on 23 Feb 2011 at 3:43

[GoogleCodeExporter]

@Travis – Thanks for all the information. That is really great feedback. The 
interface was useful in the unit tests for the integration, though in 
retrospect it would have been better to simply set the provider through an 
internal property to avoid the whole container resolution issue.

Regardless, I have now made the ILifetimeScopeProvider interface public and 
added constructor overloads to the AutofacDependencyResolver that takes 
instances of the interface. If no instance of the ILifetimeScopeProvider is 
provided in the constructor the default instance is created upon first use. The 
instance is no longer resolved from the container at any point. The application 
container is now also exposed by a public property. Please give these changes a 
run and let me know how they work out for you.

@Nick – I’m not sure what you mean about deprecating the existing 
mechanism. The current process should work as before, as the 
ILifetimeScopeProvider is passed in via the new constructor overloads, so 
existing code will not need to be changed. Feel free to take a look at the 
changes and let me know if you had something different in mind.

Hopefully the changes will make things easier for Travis in the multitenant 
code, and for other consumers provide a mechanism to mock the lifetime scope in 
unit tests for code where DependencyResolver.Current is accessed.

Original comment by alex.meyergleaves on 24 Feb 2011 at 1:28

[GoogleCodeExporter]

Thanks for looking into this guys - sorry to not offer much help, short on time 
at the moment. One thing I think we should consider carefully is whether 
ILifetimeScopeProvider can be unified with IContainerProvider. Having two 
incompatible mechanisms will be a problem. We might need to push one (or 
another interface altogether) to Autofac.dll in order for both libraries to use 
the same abstraction.

Original comment by nicholas...@gmail.com on 1 Mar 2011 at 9:53

[GoogleCodeExporter]

I have done some more refactoring to make it easier to implement an 
ILifetimeScopeProvider without having to worry about dealing with the HTTP 
request lifecycle.

The DefaultLifetimeScopeProvider has been made public and renamed to 
RequestLifetimeScopeProvider. Much of the implementation that was in the 
RequestLifetimeHttpModule has been moved into the RequestLifetimeScopeProvider. 
When a new instance of the RequestLifetimeScopeProvider is created is sets 
itself as the current provider on the RequestLifetimeHttpModule using the 
static SetLifetimeScopeProvider method. The ILifetimeScopeProvider interface 
now includes an EndLifetimeScope method that the RequestLifetimeHttpModule 
calls when the HTTP request ends.

The GetLifetimeScope method on the ILifetimeScopeProvider interface no longer 
requires any parameters. This should provide more freedom to the implementer on 
how they want to go about creating the ILifetimeScope. To make it much easier 
to change how the ILifetimeScope is constructed without having to worry about 
managing the HTTP request lifecycle yourself, you can now inherit from the 
RequestLifetimeScopeProvider and override the GetLifetimeScopeCore method to 
provide an ILifetimeScope instance using your own custom logic. The base 
implementation of the RequestLifetimeScopeProvider will make sure the 
ILifetimeScope is only constructed once during the HTTP request and will 
dispose the provided instance when the HTTP request ends.

These changes move the provider interface to being a lot closer to the old 
IContainerProvider. I will start thinking about how we could consolidate these 
two interfaces into a single unified interface despite the very different 
implementations. In the meantime, I would be interested to hear how the 
refactoring ends up helping with the multitenant scenario. The 
AutofacDependencyResolver still accepts an ILifetimeScopeProvider instance in 
the constructor as per my last refactoring, so mocking it for unit tests should 
remain just as easy.

Original comment by alex.meyergleaves on 1 Mar 2011 at 3:43

[GoogleCodeExporter]

Sorry, in SYD on business this week so I've not had a chance to grab the 
latest. Just browsing the source, it appears that this will resolve a lot of 
the confusion that the one user was having with the dependency resolution 
happening at ADR construction time, and the exposition of the container as a 
public property will help a LOT in multitenant cases where we need to get the 
root container and resolve a tenant-agnostic instance of something, like...

var resolver = DependencyResolver.Current as AutofacDependencyResolver;
var mtc = resolver.Container as MultitenantContainer;
var container = mtc.ApplicationContainer;
var resolved = container.Resolve<Something>();

It doesn't happen a lot, but in some cases (like integrating with Windows 
Identity Foundation where things get cached and need to be tenant-agnostic in 
the STS) it's really necessary. I'll probably add some sort of convenience 
method to multitenant (at least in my application code) that allows me to do 
all that in one line:

var resolved = DependencyResolver.Current.ResolveGlobal<Something>();

(Not glued to the name, but you get the idea.)

Big thanks for getting this in there. I'll tell my team to compile the latest 
and lose the reflection-based retrieval of the container.

Original comment by travis.illig on 1 Mar 2011 at 10:27

[GoogleCodeExporter]

This defect appears to be resolved - should it be marked closed?

Original comment by travis.illig on 10 May 2011 at 6:48

[GoogleCodeExporter]

Indeed. Thanks for the reminder Travis.

Original comment by alex.meyergleaves on 11 May 2011 at 1:49

• Changed state: Fixed