Adding ability to inject IOwinContext into externally created scope

[srogovtsev]

Added IAppBuilder.RegisterOwinContext extension method to allow to inject IOwinContext into externally created lifetime scope in the same manner as UseAutofacLifetimeScopeInjector does it for self-created scopes.

Also a couple of small style fixes.

[tillig]

We're trying to obsolete the Update method. It isn't marked obsolete in Autofac 3.5.0 but it is in 4. Rather than adding an extension method to do this for people, could we address it with documentation? By and large the custom creation of a lifetime scope will be done inside middleware anyway (right?) so it'd be just as easy to register it oneself during that creation.

The rest of the changes are fine.

[srogovtsev]

I've heard that you're obsoleting the Update, but what are you planning to do with WebAPI integration where I copied this from?

And unfortunately the creation of lifetime scope won't be done in the middleware: I'll be doing it in ASP.NET's BeginRequest, register within HttpContext, then in the middleware get the HttpContext from IOwinContext, and then the lifetime scope from it (so I will be doing the update anyway). It's exactly the same scenario you have with OWIN/WebAPI flow when you create the scope in OWIN and then simply inject it to WebAPI pipeline.

[tillig]

I don't have a specific solution for the areas where Update is used quite yet, but we will definitely have to change it. What I'm trying to avoid is adding new dependencies on it when we're really trying to stop doing that.

I guess this is another reason that, at least for now, we may want to address this just via documentation - if you choose to manually ignore the obsolete warning, you totally can and still get what you want. At the same time, we won't add any new references to Update so we won't have more places where people say, "Well, you guys are using Update here so why can't I do it over there?" By and large Update doesn't work how folks think it's going to work, so trying to use registration lambdas during lifetime scope creation is a better way to go.

I can imagine (without trying this out, since I'm sorta in the middle of some other stuff right now) that the solution in the WebAPI or other scenarios will be to register a lambda where it lazy-resolves HttpContext and calls GetOwinContext() on that or something. However, again, I'm not really in a position to dive deep in that right now; the day job is sort of eating my lunch. 😦

[tillig]

We do have some options, though:

If you need the other parts you already PR'd in, we can get a 4.1.0 pushed to NuGet to unblock you. No problem there; this additional stuff isn't required for the rest. Then when there's more time we can revisit this.

If you want to push the style change updates without the additional use of Update, we can do that, too.

Or, if you really feel strongly about it, I can hold the 4.1.0 push until I have time to dive more fully into this. Could be a bit, my current project is... well, it's not stellar from the work-life balance side of things.

[srogovtsev]

I'm okay with pushing only the style update (first commit) - or even the 4.1.0 as it is now - I'll be able to do Update on my side w/o any problem. And then we'll have time to think about how to do it properly. Maybe I'll stumble onto something more agreeable while doing my part.

[tillig]

Cool. I'll bring that first commit in, then, and leave the rest of this open for a while to let it simmer. I'll comment on the original issue and close it once I've got a build pushed to NuGet. Thanks for all your help here, it truly is very much appreciated. I wish there were more OSS folks out there like you!

[srogovtsev]

Well, thank you for all the praise, but I'm simply doing something to make my own life easier.

[tillig]

I haven't forgotten this, just got sucked into some other things. I'm curious if we might take some hints from .NET Core on this and make use of things like IHttpContextAccessor to get around having to change the lifetime scope. I'm not glued to that, just brainstorming. Some way to get a registration into the scope (the accessor) early and do some sort of late-bound set on the instance. That'd solve the WebApi integration thing, too.

Another option might be to add some sort of Items collection to ILifetimeScope set stuff there, register lambdas that use the collection to resolve. Hmmm. Will keep thinking.

[tillig]

Just to cross-reference - the idea to use IHttpContextAccessor style services looks like it overlaps with the PR at autofac/Autofac.WebApi#29 - the desire to enhance the Web API integration with an accessor for the HttpRequestMessage. If we did something like that for MVC, Web API, and OWIN it would be consistent and remove the Update calls.

[srogovtsev]

I'm not sure I actually like the idea of IHttpContextAccessor, especially when it comes to Owin Context, because when the whole pipeline is built around the idea of explicit message passing, and then you build an ambient context for it, just to allow access to the same message from the container - it somewhat smells.

I'd actually prefer creating explicit lifetime scopes on every nested integration (one for the outermost ASP.net request, next one for OWIN, next one for WebAPI inside OWIN), but that would require explicit tagging of the outermost request if we want to share dependencies, and that might feel as a breaking change.

[tillig]

I thought about the nested scope thing but that would certainly break instance per lifetime scope registrations where folks are trying to share modules across project types. The Items collection on a scope might sidestep both problems since the various contexts would be passed along with the scope itself.

I dunno, still thinking. I agree the ambient context concept somewhat breaks the message passing notion, but I also know how many places in classic ASP.NET aren't part of the composition root or the message passing chain (like HttpModules). It's definitely better if folks switch up (like switch modules to be middleware) but that isn't always possible.

But, again, totally not sold on it. Just trying to brainstorm and write down ideas so they're not lost. I definitely appreciate the feedback, too, so thank you.

[tillig]

I messed around with some repros on this issue and I'm curious if you couldn't get away with using the existing hooks to get the IOwinContext into the externally created lifetime scope. This is based on the thing I posted back in July.

For example, let's say you're using the web forms integration with OWIN. Your BeginRequest handler could use the HttpContext.Current.GetOwinContext() extension from Microsoft.Owin.Host.SystemWeb to register the context:

protected void Application_BeginRequest(object sender, EventArgs e)
{
  var scope = this.ContainerProvider.ApplicationContainer.BeginLifetimeScope(
    MatchingScopeLifetimeTags.RequestLifetimeScopeTag,
    builder =>
    {
      builder.Register(ctx => HttpContext.Current.GetOwinContext())
             .As<IOwinContext>()
             .InstancePerLifetimeScope();
    });

  HttpContext.Current.Items[typeof(ILifetimeScope)] = scope;
}

This works to inject an IOwinContext into a web form.

And, of course, if the request doesn't flow through BeginRequest for whatever reason you can use the OWIN app.UseAutofacLifetimeScopeInjector(Func<IOwinContext,ILifetimeScope>) overload to call your scope creation method and insert the passed in IOwinContext directly.

I did notice that BeginRequest happens before the OWIN context is generally created, which one way I can see might lead to this point where the context should be injected after the fact, but I think with a lambda registration you can get away with things. It won't try to get the OWIN environment until later.

One caveat is that if you're using web forms you may have to make some of the properties Lazy<IOwinContext> instead of just IOwinContext to allow the deferred resolution of the context from HttpContext. For OWIN middleware and such, things should just work.

In either case, I'm not sure that there's a need with the current hooks to add an extension to retroactively insert the OWIN context into a created scope. It might take some refactoring of the code that creates the scope to allow some deferred lambda calls, but it'd avoid the call to Update.

And I do get that some of this breaks the clean message passing of OWIN... but if you're integrating with System.Web, HttpContext, and app events, you're already into messy territory.

[srogovtsev]

I think this should work one way or another (and if nothing works for me, I can stick with Update in my own middleware with a little hacking).

BTW (I haven't tested it yet, but I think it should work also) we can get rid of the HttpContext.Current usage in registration to make it even more predictable:

protected void Application_BeginRequest(object sender, EventArgs e)
{
  var httpContext = Context; //provided by HttpApplication, and it's _this_ request's context
  var scope = this.ContainerProvider.ApplicationContainer.BeginLifetimeScope(
    MatchingScopeLifetimeTags.RequestLifetimeScopeTag,
    builder =>
    {
      builder.RegisterInstance<HttpContext>(httpContext);
      builder.Register(ctx => ctx.Resolve<HttpContext>.GetOwinContext())
             .As<IOwinContext>()
             .InstancePerLifetimeScope();
    });

  httpContext.Items[typeof(ILifetimeScope)] = scope;
}