Separate lifetime scope injection and component adding

[srogovtsev]

The problem

I have fairly simple OWIN pipeline:

app
    .UseAutofacMiddleware(container)
    .UseBasicAuthentication()
    .Use((c, next) =>
    {
        //authorization
        return next();
    })
    .Use<PathRewriter>
    (
        container.Resolve<EventConverter>(),
        container.Resolve<Func<Guid, BlobStream>>()
    )
    .UseSendFileFallback()
    .UseStaticFiles();

I think that you can already spot the weak spot: Use<PathRewriter>(container.Resolve<EventConverter>(), container.Resolve<Func<Guid, BlobStream>>())

The thing is, I have to use several middleware components that are not based on OwinMiddleware or are registered by other means (like UseBasicAuthentication), and, as far as I know UseAutofacMiddleware injects all found middleware right after it.

Proposed solution

Extract two parts, UseAutofacLifetimeScopeInjector and UseMiddlewareFromContainer<T> from UseAutofacMiddleware thus allowing more precise control over pipeline composition. The example from above would be rewritten as:

app
    .UseAutofacLifetimeScopeInjector(container)
    .UseBasicAuthentication()
    .Use((c, next) =>
    {
        //authorization
        return next();
    })
    .UseMiddlewareFromContainer<PathRewriter>()
    .UseSendFileFallback()
    .UseStaticFiles();

[tillig]

We're looking at updating everything to version 4.0.0 for the DNX and updated Autofac core support, so that would be a good time to include this, too. I think it's fairly reasonable and implemented in a backwards-compatible format. I dig it.

I've gotta set up gitflow on this repo so I'll probably merge to a different branch rather than taking the PR right into master. I'll keep you posted.

Thanks for the PR!

[tillig]

Question on this:

If someone calls UseMiddlwareFromContainer<T> before calling UseAutofacLifetimeScopeInjector then they may get unexpected results - the lifetime scope won't be in place.

Should we check to see if UseMiddlwareFromContainer<T> is called early and throw if it's called before UseAutofacLifetimeScopeInjector?

(BTW, I did pull this into the develop branch, so don't worry about updating the PR; I'll make the change right in develop.)

[srogovtsev]

Yes, I think we should throw if there's no lifetime injector in the pipeline.

BTW why does AutofacMiddleware just short-circuit to next handler if there's no lifetime scope defined? If it didn't, there would be no need to implement the check.

[tillig]

Yeah, I noticed that short-circuit thing too. If there's always a scope, there's probably no need to check. I'll update that, too.

The first cut of this in a usable thing is in Autofac.Owin.4.0.0-CI-220 on the MyGet (https://www.myget.org/F/autofac/) if you want to check it out.

[tillig]

OK. 4.0.0-CI-221 removes the short circuit and adds some safety checks to make sure people don't try to register injected middleware without first adding the lifetime scope injector to the pipeline. Give it a run, let me know what you think. I'm going to give it a bit to see if I hear back from folks on issue #1 before pushing a beta to NuGet. Maybe early next week?

[srogovtsev]

I'll try to give it a try asap. Can I run 4.0 without DNX?

[tillig]

Yup. The Autofac.Owin 4.x is still compatible with the 3.x series core Autofac. If you do go for core Autofac 4.0 beta, we're still totally compatible with full .NET 4.5 and non-DNX platforms.

[srogovtsev]

Why does Autofac.Owin 4 require Microsoft.Owin.Hosting? It didn't before.

[srogovtsev]

Aside from question about new dependency, the integration seems to work as expected.

[tillig]

Fixed. The reference showed up due to a switch in build servers (from MyGet to AppVeyor) + changing the way we were packaging (from MSBuild very explicit packaging to AppVeyor "automated") + the weird extraneous reference in the .csproj that was there but shouldn't have been. (It wasn't in the .nuspec, but AppVeyor packaging "helped" by adding the missing reference.)

4.0.0-CI-222 on MyGet fixes it. Thanks for catching it! I'd have totally missed it.

[srogovtsev]

Yep, it helped.

[tillig]

Pushed to NuGet as 4.0.0-beta7-222.