Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update code and docs for generic hosting upcoming changes #36

Open
tillig opened this issue Nov 13, 2018 · 15 comments

Comments

Projects
None yet
3 participants
@tillig
Copy link
Contributor

commented Nov 13, 2018

Pull request aspnet/Hosting#1580 includes changes to how hosting works to support a generic host builder on which web host builder is layered. Some of these changes are breaking and we'll need to adjust docs, examples, and code to support it.

  • Adding an IServiceProviderFactory as a service no longer works. Call IHostBuilder.UseServiceProviderFactory instead. This changes the samples and renders the ServiceCollectionExtensions.AddAutofac method obsolete.
  • You can no longer return an IServiceProvider from Startup.ConfigureServices. You must use the IServiceProviderFactory and ConfigureContainer. This will affect the use case where folks intentionally want to keep a reference to the built container or use a child lifetime scope as the pseudo-root of the app. I've mentioned that in a comment on the PR. You may need a custom service provider factory for this, but you can capture the factory instance during Configure.

In general it appears that while the new pattern moves toward a more generic hosting mechanism, it moves away from the notion of having composite application types (e.g., a self-hosted ASP.NET Core app and a background service all in one logical process).

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

It does appear in ASP.NET Core 3 the move to the generic host builder will not support this and you'll get an exception if you try to return your own IServiceProvider.

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

Based on #46, it would be nice to see if we can somehow allow the IServiceProviderFactory to use a child lifetime scope for the provider it creates rather than assuming the root container. Perhaps some AddAutofac overloads would allow that, though I'm not sure at this precise moment what that would look like.

(I may be repeating myself, but I did raise this exact concern in the PR to the ASP.NET Core hosting model that removed the ability to do child lifetime scopes and got no response from that team.)

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

Looking at the IServiceProviderFactory<TContainerBuilder> interface, it assumes that...

  • CreateBuilder will return a ContainerBuilder that has the set of registered services already populated in it - the stuff that came from Startup.ConfigureServices.
  • CreateServiceProvider will take a ContainerBuilder but not necessarily guaranteed to be the same one issued from CreateBuilder and build a service provider.

In practice, CreateBuilder and CreateServiceProvider should both get executed from the same provider factory instance since it's registered as a singleton. This is how the default Microsoft provider works as well. It's likely that the CreateServiceProvider method will be called with the same ContainerBuilder as provided by CreateBuilder... but that's not documented, tested, or otherwise guaranteed by the ASP.NET Core framework or the hosting mechanism that I can find. It would be bad if we made that assumption and counted on it since, long term, that assumption could break and we'd be on the hook to try and figure out ways around it.

I'm also not sure how to return the ContainerBuilder that's the parameter in a lambda expression that has to run when you build the lifetime scope. Like, when you create a child lifetime scope, it's...

var scope = container.BeginLifetimeScope(scopeBuilder => scopeBuilder.RegisterType<T>());

Like that. It's a lambda expression. There isn't actually a ContainerBuilder object. So I don't know what CreateBuilder would return, because logically it needs to return that scope-level builder. There's also not currently a way to take a ContainerBuilder and somehow... transfer the registrations from one to another. It doesn't work like that. We couldn't just return a ContainerBuilder and then in the CreateServiceProvider step just "apply" those registrations to the scope.

If we could, we'd still need to figure out how CreateServiceProvider changes. It can't just call ContainerBuilder.Build() anymore, it'd have to decide whether it creates a lifetime scope (and, possibly, tags it) or whether it creates the container... and then wraps the outcome in an AutofacServiceProvider.

I see there's a notion of a base startup class that has a way to manually return an IServiceProvider but I'm not sure if that's still a valid thing or not. In general the pattern isn't to derive from that anyway at the moment, so we may need to try that and see if it's an option for workaround... but it won't help the folks that can't change their startup classes to derive from that.

I also noticed in the PR and issue I linked earlier in the ASP.NET Core repo that one of the big things they are trying to force is the use of a single, global container. No double-containers, no partitions. One app, one container. You can't even inject a logger into Startup anymore - you get IConfiguration and IHostingEnvironment and that's it. They're intentionally making it hard for you to control these things.

After diving in here, and given the notes in the ASP.NET Core repo... while we may be able to do some pretty intricate workarounds to make the "ASP.NET Core app gets a child scope in a container instead of a whole container" thing... we may also be really trying to force a square peg into a round hole. I'm not sure how supportable it will be or if the changes we'd make to allow that to work would continue working in the next version when things get locked down even more.

If the StartupBase thing doesn't effectively work around it by allowing you to continue returning your own IServiceProvider out of ConfigureServices then individual projects/apps may want to create their own custom IServiceProviderFactory<TContainerBuilder> implementations that work enough to get this happening in that project but aren't something the Autofac team has to try to keep working across ASP.NET Core releases. It's already kind of hard to make sure they don't add things to their conforming container spec that don't work with... well, honestly, most IoC containers. Keeping the 90% case working here and calling it good may be the most maintainable option.

And, of course, if someone does solve this in a maintainable way, we can take a PR or that person could publish their factory as their own NuGet package.

@JuergenGutsch

This comment has been minimized.

Copy link

commented Jun 26, 2019

Just to get a conclusion out of that issue: There is currently no easy way to get Autofac running in ASP.NET Core 3.0. Is that right?

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Don't know, haven't done work on it yet. Interesting using the service provider factory should just work, though I can't tell you the gotchas or workarounds. The problem I see mainly is that we likely can't continue to support hosting where the "root lifetime scope" for the hosted app isn't a container. Some folks try to share a container and host their app with the root lifetime being a child scope. I don't think we'll be able to keep that.

@JuergenGutsch

This comment has been minimized.

Copy link

commented Jun 27, 2019

Ok. I'm going to try to use the service provider factory. (This issue kinda blocks me finishing a book about ASP.NET Core 3.0)
Did you try to talk to @davidfowl about that. He helped me with an issue I had with the LightCore IoC Container.

@JuergenGutsch

This comment has been minimized.

Copy link

commented Jun 27, 2019

As you wrote, using the service provider factory just works.

public static class IHostBuilderExtension
{
    public static IHostBuilder UseAutofacServiceProviderFactory(this IHostBuilder hostbuilder)
    {
        hostbuilder.UseServiceProviderFactory<ContainerBuilder>(new AutofacServiceProviderFactory());
        return hostbuilder;
    }
}

Usage:

public static IHostBuilder CreateHostBuilder(string[] args) =>
    Host.CreateDefaultBuilder(args)
        .UseAutofacServiceProviderFactory()
        .ConfigureWebHostDefaults(webBuilder =>
        {
            webBuilder                       
                .UseStartup<Startup>();
        });

I used an UseAutofacServiceProviderFactory implemented by my own previously to check whether this is really used by ASP.NET Core 3.0:

public class AutofacServiceProviderFactory : IServiceProviderFactory<ContainerBuilder>
{
    public ContainerBuilder CreateBuilder(IServiceCollection services)
    {
        var containerBuilder = new ContainerBuilder();

        // read service collection to Autofac
        containerBuilder.Populate(services);

        return containerBuilder;
    }

    public IServiceProvider CreateServiceProvider(ContainerBuilder containerBuilder)
    {
        var applicationContainer = containerBuilder.Build();

        // creating the IServiceProvider out of the Autofac container
        return new AutofacServiceProvider(applicationContainer);
    }
}
@davidfowl

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Autofac already has an AutofacServiceProviderFactory implementation you can use, no need to write your own.

The problem I see mainly is that we likely can't continue to support hosting where the "root lifetime scope" for the hosted app isn't a container. Some folks try to share a container and host their app with the root lifetime being a child scope. I don't think we'll be able to keep that.

Can't that be a feature of the factory?

@JuergenGutsch

This comment has been minimized.

Copy link

commented Jun 27, 2019

Autofac already has an AutofacServiceProviderFactory implementation you can use, no need to write your own.
Thanks @davidfowl :-) Yes I know. I did just to simply debug into things. Wasn't that much effort ;-)

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

There are a few challenges with trying to wrap things into the service provider factory that I'm not sure how to address. For the 80% case, the answer is:

  • Remove the ServiceCollectionExtensions.AddAutofac extension - having the service provider factory inside the service collection doesn't work anymore. (Pretty sure.)
  • Create a new HostBuilderExtensions.UseAutofac extension that attaches the Autofac service provider factory to the builder.
  • Profit.

However, as I mentioned in my earlier fairly-long-and-rambling post, I don't know what that looks like if you already have a container and the service provider factory is supposed to contribute to a child lifetime scope.

Let's say I have that container.

var builder = new ContainerBuilder();
var container = builder.Build();

When you create a lifetime scope, it's like this:

builder.BeginLifetimeScope("optional-tag", scopeBuilder => scopeBuilder.RegisterThingsHere());

The host builder startup process does this:

  • Call Startup.ConfigureServices to get an IServiceCollection
  • Call AutofacServiceProviderFactory.CreateBuilder to put the IServiceCollection into a ContainerBuilder and return that ContainerBuilder.
  • Pass the ContainerBuilder to Startup.ConfigureContainer to register more stuff.
  • Pass the ContainerBuilder to AutofacServiceProviderFactory.CreateServiceProvider to get an IServiceProvider with everything ready to go.

The problem is, BeginLifetimeScope executes that lambda immediately. It's not a deferred thing. We also don't have the notion of "run a bunch of registrations on one ContainerBuilder and apply those registrations against a second ContainerBuilder."

Here's the current AutofacServiceProviderFactory:

public class AutofacServiceProviderFactory : IServiceProviderFactory<ContainerBuilder>
{
  private readonly Action<ContainerBuilder> _configurationAction;

  public AutofacServiceProviderFactory(Action<ContainerBuilder> configurationAction = null)
  {
    _configurationAction = configurationAction ?? (builder => { });
  }

  public ContainerBuilder CreateBuilder(IServiceCollection services)
  {
    var builder = new ContainerBuilder();
    builder.Populate(services);
    _configurationAction(builder);
    return builder;
  }

  public IServiceProvider CreateServiceProvider(ContainerBuilder containerBuilder)
  {
    if (containerBuilder == null) throw new ArgumentNullException(nameof(containerBuilder));
    var container = containerBuilder.Build();
    return new AutofacServiceProvider(container);
  }
}

Let's say we were trying to support child lifetime scopes as a root for the app. How do I split the BeginLifetimeScope lambda across CreateBuilder and CreateServiceProvider? With the stuff we have at the moment, the answer to that isn't coming to me.

I can imagine some sort of really not-pretty indirection, like maybe this:

public class AutofacServiceProviderFactory : IServiceProviderFactory<IList<Action<ContainerBuilder>>>
{
  private readonly Action<ContainerBuilder> _configurationAction;
  private readonly ILifetimeScope _parent;

  public AutofacServiceProviderFactory(ILifetimeScope parent, Action<ContainerBuilder> configurationAction = null)
  {
    _parent = parent;
    _configurationAction = configurationAction ?? (builder => { });
  }

  public IList<Action<ContainerBuilder>> CreateBuilder(IServiceCollection services)
  {
    var actions = new List<Action<ContainerBuilder>>();
    actions.Add(b => b.Populate(services));
    actions.Add(b => _configurationAction(b));
    return actions;
  }

  public IServiceProvider CreateServiceProvider(IList<Action<ContainerBuilder>> containerBuilderActions)
  {
    if (containerBuilderActions == null) throw new ArgumentNullException(nameof(containerBuilderActions));
    var scope = _parent.BeginLifetimeScope(scopeBuilder =>
    {
      foreach(var action in containerBuilderActions)
      {
        action(scopeBuilder);
      }
    }
    return new AutofacServiceProvider(scope);
  }
}

The usage on that would sort of be sucky in the Startup but maybe that's the price you pay for wanting to be in a child scope.

public void ConfigureContainer(IList<Action<ContainerBuilder>> actions)
{
  actions.Add(b =>
  {
    b.RegisterType<Foo>().As<IFoo>());
    b.RegisterType<Bar>().As<IBar>());
  });
}

Likely we'd make that a different provider factory so the 80% case would be easier.

We have a similar challenge with multitenant support in the new world. Building a multitenant container is like:

var builder = new ContainerBuilder();
var container = builder.Build();
var multitenantContainer = new MultitenantContainer(container, new TenantIdentificationStrategy());
multitenantContainer.RegisterTenant("tenant-1", b =>
{
  b.Register<TenantDependency>();
});

In the situation where you could return the IServiceProvider from ConfigureServices this was an easier thing to handle, or at least more straightforward to understand. Forcing it through a factory, I'm not sure how to deal with it. Maybe there's a multitenant factory that takes a delegate to do the multitenant configuration after the fact? I don't get a reference in the factory to the startup class, so I can't make that interaction automatic or pretty. That also means I can't use an instance method to get things like configuration or hosting environment that I'd potentially otherwise have in a startup class.

Point being, unless I'm missing something... the 80% case is easy to solve, but these other use cases don't feel so straightforward in the new world.

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

I'm also not sure how to address the issue where folks want to keep their own reference to the container. If the container is built inside the factory and the factory returns an IServiceProvider then there's no room to return the container itself, too. Options I can think of off the top of my head:

  • Allow a lambda to be passed to the factory that takes a container (or lifetime scope). The factory can call that lambda after the container is built. In that lambda, you can set a static variable to the container.
  • Folks can register a container build callback that will set the global variable to the container reference. This isn't a thing on lifetime scopes, though, IIRC, so the folks wanting child lifetime scopes as the "root" for the app may be stuck. I'd have to test it.
  • Run some logic that resolves ILifetimeScope from the IServiceProvider - this will give the lifetime scope in the service provider. Store that. This is probably the best option, but won't work in multitenant mode because all resolutions from the multitenant container factor in the tenant. To properly support multitenant request services, we absolutely need the reference to the root container.
@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

More brainstorming: Perhaps an IServiceProvider extension that tries to cast the provider to AutofacServiceProvider and get container (or whatever) references that way. We'd have to add a property to the AutofacServiceProvider to get the root lifetime scope, but it could be something like:

public ILifetimeScope GetAutofacRoot(this IServiceProvider provider)
{
  return (provider as AutofacServiceProvider)?.LifetimeScope;
}

That would solve the issue of folks wanting the reference to the container and would keep it out of the factory.

@davidfowl

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

More brainstorming: Perhaps an IServiceProvider extension that tries to cast the provider to AutofacServiceProvider and get container (or whatever) references that way. We'd have to add a property to the AutofacServiceProvider to get the root lifetime scope, but it could be something like:

I was going to suggest that. It's a much better approach as it makes the autofac ILifetimeScope available everywhere.

As for the other thing, I'm a fan of the indirection but maybe not using IList<Action<ContainerBuilder>>, maybe it could be some wrapper type that doesn't look as noisy for these more advanced scenarios?

@tillig

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

Yeah, we could have like public class ScopeBuilder : List<Action<ContainerBuilder>> { } or something to make it a little nicer.

Not totally sure about the multitenant container thing, possibly solvable with a similar strategy. Most of this is going to boil down to getting the time [and, honestly, motivation... I've a non-zero level of OSS maintainer burnout set in] to sit down, work up a few options, try them out, see how they work, and get it done.

@davidfowl

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

You could also consider having a single builder that can handle all scenarios but I haven't looked at the API deeply enough to know how trivial it would be.

@tillig tillig added the help wanted label Jun 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.