Implementing IMiddlewareFactory with Simple Injector for ASP.NET Core

[guardrex]

Hi Steven (@dotnetjunkie),

I'm working on the advanced IMiddleware/IMiddlewareFactory topic for the ASP.NET Core docs, and I've hit a snag with scope lifetimes that I can't work out. I probably have created a completely bonkers wiring-up job and/or factory! lol

The introductory topic that compares conventional middleware activation with factory-based activation is here 👉 https://docs.microsoft.com/aspnet/core/fundamentals/middleware/extensibility

I'm working on a new topic that uses the factory with Simple Injector. Everything seemed to be going along fairly well until I hit ye 'olde scoping problems, such as ...

The AppDbContext is registered as 'Async Scoped' lifestyle, but the instance is requested outside the context of an active (Async Scoped) scope.

Exception has occurred: CLR/SimpleInjector.ActivationException
An exception of type 'SimpleInjector.ActivationException' occurred in SimpleInjector.dll but was not handled in user code: 'The AppDbContext is registered as 'Async Scoped' lifestyle, but the instance is requested outside the context of an active (Async Scoped) scope.'
   at SimpleInjector.Scope.GetScopelessInstance[TImplementation](ScopedRegistration`1 registration)
   at SimpleInjector.Advanced.Internal.LazyScopedRegistration`1.GetInstance(Scope scope)
   at SimpleInjector.InstanceProducer.GetInstance()
   at SimpleInjector.Container.GetInstance(Type serviceType)
   at MiddlewareExtensibilitySample.Middleware.SimpleInjectorMiddlewareFactory.Create(Type middlewareType) in C:\Users\guard\Documents\GitHub\MiddlewareExtensibilitySample2\Middleware\SimpleInjectorMiddlewareFactory.cs:line 22
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass5_1.<<UseMiddlewareInterface>b__1>d.MoveNext()

The middleware is unremarkable ... just writes a query string value to an in-memory dB:

public class SimpleInjectorActivatedMiddleware : IMiddleware
{
    private readonly AppDbContext _db;

    public SimpleInjectorActivatedMiddleware(AppDbContext db)
    {
        _db = db;
    }

    public async Task InvokeAsync(HttpContext context, RequestDelegate next)
    {
        var keyValue = context.Request.Query["key"];

        if (!string.IsNullOrWhiteSpace(keyValue))
        {
            _db.Add(new Request()
                {
                    DT = DateTime.UtcNow, 
                    MiddlewareActivation = "SimpleInjectorActivatedMiddleware", 
                    Value = keyValue
                });

            await _db.SaveChangesAsync();
        }

        await next(context);
    }
}

I'd like the factory to get that going. I can't confirm this works because of the scoping issue, but here's what I was thinking ...

public class SimpleInjectorMiddlewareFactory : IMiddlewareFactory
{
    private readonly Container _container;

    public SimpleInjectorMiddlewareFactory(Container container)
    {
        _container = container;
    }

    public IMiddleware Created { get; private set; }
    public IMiddleware Released { get; private set; }

    public IMiddleware Create(Type middlewareType)
    {
        Created = _container.GetInstance(middlewareType) as IMiddleware;

        return Created;
    }

    public void Release(IMiddleware middleware)
    {
        Released = middleware;
    }
}

... an unremarkable pipeline for this sample (trying to keep the Startup class lean, so I moved InitializeContainer bits up to ConfigureServices) ...

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
    if (env.IsDevelopment())
    {
        app.UseDeveloperExceptionPage();
    }
    else
    {
        app.UseExceptionHandler("/Error");
    }

    app.UseSimpleInjectorActivatedMiddleware();

    app.UseStaticFiles();
    app.UseMvc();
}

... and the wiring up bits in ConfigureServices look like this (again, dropping the IntegrateSimpleInjector format to keep things lean) ...

public void ConfigureServices(IServiceCollection services)
{
    services.AddTransient<SimpleInjectorActivatedMiddleware>();
    services.AddTransient<IMiddlewareFactory>(_ =>
    {
        return new SimpleInjectorMiddlewareFactory(container);
    });

    services.AddMvc();

    container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

    container.Register<AppDbContext>(() => 
    {
        var optionsBuilder = new DbContextOptionsBuilder<DbContext>();
        optionsBuilder.UseInMemoryDatabase("InMemoryDb");
        return new AppDbContext(optionsBuilder.Options);
    }, Lifestyle.Scoped);

    container.Register<SimpleInjectorMiddlewareFactory>();

    container.Register<SimpleInjectorActivatedMiddleware>();

    container.Verify();
}

The full project is over here 👉 https://github.com/guardrex/MiddlewareExtensibilitySample2

... and the scopes you see in the GH project and here ☝️ I hacked around with trying to resolve this but no luck. It actually seems like lifestyles aren't my problem and that I haven't wired Simple Injector up properly or I built a broken factory.

[dotnetjunkie]

Are you calling services.UseSimpleInjectorAspNetRequestScoping(container) from your ConfigureServices method as explained in the documentation?

Apart from that, is there any particular reason to use this specific method of applying middleware (going through the framework provided factory abstraction) instead of using the approach described in the docs?

[guardrex]

is there any particular reason to use this specific method of applying middleware

See [@]davidfowl's comment 👉 aspnet/HttpAbstractions#754 (comment)

services.UseSimpleInjectorAspNetRequestScoping(container)

Yes, thanks, that seems to have helped. I saw it in the docs, but there's no explicit explanation of what it does. I thought it might just be part of controller or view component activation.

This project still doesn't quite work, but it's getting closer ...

Now, the problem is that the Razor Page is choking with "can't find the AppDbContext" because it's trying to resolve the dB context from the default container and not the SI container.

Registering the page class, IndexModel, with the SI container isn't enough to get SI to take care of injecting its dB context. I'll need to work a bit more on how to use SI with a Razor Pages page (unless you tell me that it's not capable of that yet). I mean I see controller activation logic in your MVC Core example, so I'm hoping that bringing that into the app will resolve this and allow the Razor Page to obtain that AppDbContext from SI.

I have been able to confirm that the middleware is working and adding entries for each request. I think the last problem is getting the page to use that dB context. It all should work after that.

[guardrex]

Update

Wiring up for IControllerActivator and IViewComponentActivator wasn't enough to use SI with a Razor Page. It's still throwing ...

System.InvalidOperationException: Unable to resolve service for type 'MiddlewareExtensibilitySample.Data.AppDbContext' while attempting to activate 'MiddlewareExtensibilitySample.Pages.IndexModel'.
   at Microsoft.Extensions.Internal.ActivatorUtilities.GetService(IServiceProvider sp, Type type, Type requiredBy, Boolean isDefaultParameterRequired)
   at lambda_method(Closure , IServiceProvider , Object[] )
   at Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.DefaultPageModelActivatorProvider.<>c__DisplayClass1_0.<CreateActivator>b__0(PageContext context)
   at Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.DefaultPageModelFactoryProvider.<>c__DisplayClass3_0.<CreateModelFactory>b__0(PageContext pageContext)
   at Microsoft.AspNetCore.Mvc.RazorPages.Internal.PageActionInvoker.CreateInstance()
   at Microsoft.AspNetCore.Mvc.RazorPages.Internal.PageActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.RazorPages.Internal.PageActionInvoker.<InvokeInnerFilterAsync>d__22.MoveNext()

I certainly can convert this app over to MVC Core. Let me know if there's some other way to get SI to take over the DI for a Razor Page. I'd prefer to keep this a Razor Pages sample if possible.

[dotnetjunkie]

Now, the problem is that the Razor Page is choking with "can't find the AppDbContext" because it's trying to resolve the dB context from the default container and not the SI container.

Prevent injecting any dependencies into razor pages in the first place. See this discussion for more details:
#362.

I certainly can convert this app over to MVC Core.

The @inject directive is new in MVC Core, so if you are converting an existing ASP.NET MVC app to Core, you shouldn't run into that issue, since MVC 'classic' lacks such feature.

[guardrex]

Ah! I see. Thanks. As an aside, I hope [@]DamianEdwards received that feedback. Anywho ... the workaround in #362 makes perfect sense. Thanks again! This sample is almost there!

[guardrex]

🎉 🎈 🍻 Booyeah!

Thank you so much for those tips. That permits me to shoot this updated sample back into the PR. I was meeting some requests from [@]davidfowl to make sure that SI was doing all of the DI work and handling the factory activation to execute the middleware.

I'll ping you over on the docs PR shortly, and I hope you'll take a look and provide feedback.

Thanks again for your help!

[dotnetjunkie]

I would advise against using Simple Injector as example for demonstrating IMiddlewareFactory in the ASP.NET Core documentation. I think the model described in the Simple Injector documentation is simpler and more practical. I don't see any benefits in implementing a custom IMiddlewareFactory when using Simple Injector. Try to find a different DI Container where this integration point actually makes sense.

For Simple Injector, a much simpler model is to have the following extension method:

public static class SimpleInjectorMiddlewareExtensions
{
    public static void UseMiddleware<TMiddleware>(
        this IApplicationBuilder app, Container container)
        where TMiddleware : IMiddleware
    {
        container.Register<TMiddleware>(Lifestyle.Transient);
        app.Use((c, next) => container.GetInstance<TMiddleware>().InvokeAsync(c, _ => next()));
    }
}

This extension method can be used as follows to add middleware to the pipeline:

public void Configure(
    IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
    app.UseMiddleware<MyCustomMiddleware>(this.container);
}

We might even add this extension method to the ASP.NET Core integration package.

[davidfowl]

@dotnetjunkie Why do you think that approach is cleaner? Is it because the IMiddlewareFactory is too global?

[dotnetjunkie]

@davidfowl explain what the problem is that the factory solves.

[davidfowl]

Today the issue with conventional middleware is that it takes arguments that aren't services. The RequestDelegate argument passed to the constructor makes it not very DI friendly. The middleware class is also a singleton today, which means scoped services can't be passed into the constructor, instead we support resolving scoped dependencies from the InvokeAsync method.

In order to fix this, we added IMiddleware and IMiddlewareFactory which changes the pattern to create the middle per request instead of per app. This means that scoped services can be specified in constructor instead of the Invoke/InvokeAsync method. The IMiddlewareFactory is the composition root for the IMiddleware activation. Similar to the IControllerFactory in MVC which is responsible for creating and destroying controllers, it's the thing responsible for creating and destroying IMiddleware.

[dotnetjunkie]

Today the issue with conventional middleware is that it takes arguments that aren't services. The RequestDelegate argument passed to the constructor makes it not very DI friendly. The middleware class is also a singleton today, which means scoped services can't be passed into the constructor

I wholeheartedly agree that this is a problem. The underlying problem here is that the current model forces the injection of runtime data (the next delegate) into the constructor. That's why the Simple Injector documentation described from the start how the next delete should be passed on through the Invoke method instead.

So it's great to hear you are addressing this, and the the IMiddleware abstraction is a great solution and I applaud you for having this.

The IMiddlewareFactory might be a good abstraction as well, since it allows anyone to intercept that particular stage of the pipeline. As far as I can see, that IMiddlewareFactory abstraction however does not solve a problem for non-conformers nor Pure DI practitioners and I my advice would be to not promote that abstraction as existing for this exact purpose, as you seem to do right now.

The addition of the IMiddlewareFactory, doesn't seem to solve any problem, since the IMiddleware abstraction itself already fixed the problem. The IMiddleware abstraction pushes developers into separating their runtime data from their design time dependencies. As my previous example demonstrated, middleware is easily resolved from a container without the existence of the IMiddlewareFactory.

The documentation example for IMiddlewareFactory should demonstrate how to solve a different use case, one that is actually useful for this abstraction. If you decide to keep the current use case described, at the very least please refrain from using Simple Injector as example for this particular feature in your documentation, because it pushes Simple Injector users into a direction that is not best for Simple Injector users.

[davidfowl]

No problem!

[guardrex]

@dotnetjunkie As of this moment, I'm just following orders. I DO stand ready to create a new sample using another container if they order such a move. In the meantime, I hope I reduce your concerns a tad with some very strong language in the note that will be added to the topic on dotnet/AspNetCore.Docs#5382.