Allow Autofac plugin to provide existing lifetime scope

[robertcoltheart]

The use case for this change is when running a specflow test suite that tests an ASP.NET Core application using Autofac internally. Specflow should be able to use all the existing registrations in the application container, and then be able to override specific registrations (for proxying services), register the bindings, and contexts.

Let me know your thoughts, thanks!

For example:

// Top-level Program.cs
var builder = WebApplication.CreateBuilder(args);
builder.Host.UseServiceProviderFactory(new AutofacServiceProviderFactory());
builder.Host.ConfigureContainer<ContainerBuilder>(x =>
{
    // TODO: Add types and modules
});

var app = builder.Build();

app.MapGet("/weatherforecast", () => new object());

app.Run();

// SpecFlow scaffolding handling app startup/shutdown
[Binding]
public class Hooks
{
    private static ILifetimeScope container;

    [ScenarioDependencies]
    public static ILifetimeScope GetLifetimeScope()
    {
        return container.BeginLifetimeScope();
    }

    [ScenarioDependencies]
    public static void ConfigureScenarioContainer(ContainerBuilder builder)
    {
        builder.AddSpecFlowBindings<Hooks>();
    }

    [BeforeTestRun]
    public static void OnStart()
    {
        var application = new IntegrationApplication();
        var client = application.CreateClient();

        container = application.Services.GetRequiredService<ILifetimeScope>();
    }

    private class IntegrationApplication : WebApplicationFactory<Program>
    {
        protected override IHost CreateHost(IHostBuilder builder)
        {
            builder.ConfigureContainer<ContainerBuilder>(x =>
            {
                //TODO: Register modules and types for test run
            });

            return base.CreateHost(builder);
        }
    }
}

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue).
• [x] New feature (non-breaking change which adds functionality).
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected).
• [ ] Performance improvement
• [ ] Refactoring (so no functional change)
• [ ] Other (docs, build config, etc)

Checklist:

• [ ] I've added tests for my code. (most of the time mandatory)
• [ ] I have added an entry to the changelog. (mandatory)
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.

[SabotageAndi]

Thanks for the PR! @gasparnagy could you have a look at it?

[gasparnagy]

@SabotageAndi @robertcoltheart I can check it next week.

[robertcoltheart]

Some of this is a bit janky, for instance it would be nice to register the contexts and bindings in the same ScenarioDependencies method instead of using 2 static methods, but Autofac doesn't allow you to register types outside of the BeginLifetimeScope call. This means the AutofacPlugin has to create a 2nd lifetime scope and then use that to add the contexts. Which means the bindings can't resolve the contexts.

One way around this might be to add parameters to the ScenarioDependencies attribute to automatically register the bindings for the user, similar to how it's done for Windsor plugin.

[robertcoltheart]

In the case you require, you would have an existing lifetime scope that is the lifetime scope used by the ASP.NET core app. Basically what you would like to do is to create the scenario-level scopes as a child scope of that instead of a new root one.

Yes, your understanding is correct. I want to be able to use all the existing registrations my app makes, but then have a scenario-level (short-lived) scope that will be used for the SpecFlow tests.

I did go down the [GlobalDependencies] route for some time before realizing that the [GlobalDependencies] gets called before [BeforeTestRun], which is when the ASP.NET app gets spun up. So I wonder whether the current implementation is perhaps wrong, or is intentioned to run in the order it does? Not sure. If [GlobalDependencies] ran after [BeforeTestRun] then yes I agree, it would be better to have a specialized [GlobalDependencies].

Let me know, I'm happy to make further changes.

[gasparnagy]

@robertcoltheart That is a good point! The [GlobalDependencies] was invoked before the [BeforeTestRun], because you might need to be able to access resources already in the [BeforeTestRun] hook from the global container (if you have a parameter in the hook that it is going to be resolved from the container), so we cannot (?) change that behavior.

Let me think on this a bit more and I will come back.

[robertcoltheart]

Yeah, it might need a 3rd attribute to wedge in-between [GlobalDependencies] and [ScenarioDependencies] 🤣. Will leave it with you to ruminate.

[robertcoltheart]

@gasparnagy Any further thoughts on this? What about having a [FeatureDependencies] attribute that fires between the global and scenario attributes? It would return the child lifetime scope, as per my change above. Would that help?

[gasparnagy]

@robertcoltheart yeah, that is definitely one useful direction... please give me a bit more time. I to a training until Tuesday, but on Wednesday I will have more time to think it over properly. I am somehow thinking on how this could be achieved as a global dependency, because in fact this is the kind of thing we want here... But I need to think this over fully.

[gasparnagy]

Hi! So finally, I could have a look with a clean head and I think we are almost good.

I see two options, see them below. I think option A is simpler to document and (especially with making test-thread scope also overridable) solves the original problem well, but option B has also some good points. @robertcoltheart What do you think?

Option A: Introduce overriding feature lifetime scope

This is more or less what we have in the PR, but:

• I would make a new attribute (instead of using the [ScenarioDependencies]): [FeatureDependencies]. Currently we would only support this for methods returning an ILifetimeScope (as you did in the PR), later we could support other overloads as well.
• I would move the logic that resolves this from the GetFeatureScope method to the runtimePluginEvents.CustomizeFeatureDependencies event handler (line 61), especially before the if we currently there. With this change, the creation of the feature scope would be in this event handler fully, in a way that it first tries to find a method with [FeatureDependencies] and use it if available (that's the new code) and if not, it creates a new scope if the parent scope was registered (this is the existing stuff in the event handler).

So basically for this option, the only thing you need to do is to introduce a new attribute (and use it), and move the resolution logic from the GetFeatureScope to the runtimePluginEvents.CustomizeFeatureDependencies event handler. (Of course the internal fields/methods GetLifetimeScope should be also renamed to GetFeatureLifetimeScope for more clarity.)

We could do the same BTW for the test-thread scope as well (runtimePluginEvents.CustomizeTestThreadDependencies / [TestThreadDependencies]) or only to that. If I would replace the Autofac scope to a global one, probably I would do it for the entire test-thread and not for every single feature.

Option B: We introduce a concept of scenario base scope

This is also close to what we have, but...

• We would need to introduce a new attribute [ScenarioBaseScope] and use it for the new method instead of [ScenarioDependencies]. It could even get the existing feature lifetime scope as parameter, that the method could of course ignore when it replaces the entire scope.
• With this option, the resolution code is good in the GetFeatureScope method, however I would rename that method to GetScenarioBaseScope then.

[SabotageAndi]

@gasparnagy & @robertcoltheart Please don't forget that we have the Castle.Windsor plugin and there are other plugins for other DI frameworks out there. So the solution should work also for them.

And we need to do this in the 4.0 phase or we would need then again a 4.1 after that.

[gasparnagy]

@SabotageAndi I don't think this is a problem as this is clearly an additional feature, no breaking change.

We can add similar things for the other DI plugins later, but that would speak for Solution A, because that is a concept that will exist in all DI implementations.

[robertcoltheart]

To my mind, option A that you've outlined seems to be the cleaner method, as it follows the existing patterns and pairs nicely with the various scope configurations in the object container. I'll look at refactoring this later today, and perhaps add a test thread dependency attribute, as you suggested.

I think this concept could work well with the Windsor plugin as well, though bear in mind that there will always be differences between the DI plugins. For example, Windsor returns an IWindsorContainer whereas Autofac returns an ILifetimeScope.

[robertcoltheart]

@gasparnagy Do take a look at the changes I've made. I've tested this out and it seems to behave the way I expect it to.

[robertcoltheart]

@gasparnagy @SabotageAndi Bump, have you had a chance to review the changes above?

[robertcoltheart]

Thanks @gasparnagy, I've made the requested changes now.

[robertcoltheart]

@gasparnagy Any chance of pushing this out to Nuget as a beta?

[gasparnagy]

@robertcoltheart we just finished a couple of PRs and now merging them. After that there will be a beta (within a week, I hope)