Add support for IMiddlewareFactory and IMiddleware

[halter73]

The IMiddlewareFactory is a new extensibility point for middleware activation that would be registered with the application's container. The default IMiddlewareFactory should be registered as a scoped service, and (maybe) only resolve middlewareTypes that are already registered in the container.

Hosting will probably need to add the default IMiddlewareFactory to the container, but the implementation can stay in HttpAbstractions.

namespace Microsoft.AspNetCore.Http
{
    /// <summary>
    /// Provides methods to create middlware.
    /// </summary>
    public interface IMiddlewareFactory
    {
        /// <summary>
        /// Creates a middleware instance for each request.
        /// </summary>
        /// <param name="middlewareType">The concrete <see cref="Type"/> of the <see cref="IMiddleware"/>.</param>
        /// <returns>The <see cref="IMiddleware"/> instance.</returns>
        IMiddleware Create(Type middlewareType);

        /// <summary>
        /// Releases a <see cref="IMiddleware"/> instance at the end of each request.
        /// </summary>
        /// <param name="middleware">The <see cref="IMiddleware"/> instance to release.</param>
        void Release(IMiddleware middleware);
    }

    /// <summary>
    /// Defines middleware that can be added to the application's request pipeline.
    /// </summary>
    public interface IMiddleware
    {
        /// <summary>
        /// Request handling method.
        /// </summary>
        /// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
        /// <param name="next">The delegate representing the remaining middleware in the request pipeline.</param>
        /// <returns>A <see cref="Task"/> that represents the execution of this middleware.</returns>
        Task Invoke(HttpContext context, RequestDelegate next);
    }
}

The typed UseMiddleware extension methods would check if the registered type implements the new IMiddleware interface. If it does, it will automatically decide to use the IMiddlewareFactory to resolve the IMiddleware instead of using the existing middleware activation logic. This will likely be implemented by registering a Func<RequestDelegate, RequestDelegate> that resolves the IMiddlewareFactory from HttpContext.RequestServices.

The existing middleware activation logic will remain for convention-based middleware to preserve backwards compatibility.

[Tratcher]

Curious, what problem is this addressing? You want to get rid of the weird Invoke injection pattern? Is anybody actually using that?

How do you anticipate Release being used?

[benaadams]

Not sure on use case; but strong type it?

    public interface IMiddlewareFactory<TMiddleware> where TMiddleware : IMiddleware
    {
        TMiddleware Create();
        void Release(TMiddleware middleware);
    }

[davidfowl]

Curious, what problem is this addressing? You want to get rid of the weird Invoke injection pattern? Is anybody actually using that?

It's to give non conforming containers the ability to use a container to activate middleware.

How do you anticipate Release being used?

To dispose stuff if the implementation doesn't use a container and the middleware is disposable.

[Tratcher]

VS making IMiddleware Disposable?

[davidfowl]

@Tratcher the caller needs to dispose the middleware. When using a container, the container is responsible for disposing.

[Tratcher]

@davidfowl That doesn't explain why the Release method is on the factory vs the IMiddleware.

[davidfowl]

@Tratcher it does. The thing that creates the object is responsible for releasing it.

[davidfowl]

The other things this breaks is passing regular objects to the Middleware constructor (say a bool or any other object).

public static IApplicationBuilder UseMyMiddleware(this IApplicationBuilder app, bool option)
{
    return app.UseMiddleware<MyMiddleware>(option);
}

[halter73]

@davidfowl Good point. Should we throw in the UseMiddleware extension methods if the middleware type implements the new IMiddleware interface and the params list is not empty?

[davidfowl]

@halter73 that might mean our API sucks. I'll play around with it.

[davidfowl]

If we find the limitation of not being able to pass raw arguments to the middleware too limiting then we can change the signature to the following:

/// <summary>
    /// Defines middleware that can be added to the application's request pipeline.
    /// </summary>
    public interface IMiddleware
    {
        /// <summary>
        /// Request handling method.
        /// </summary>
        /// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
        /// <param name="next">The delegate representing the remaining middleware in the request pipeline.</param>
        /// <returns>A <see cref="Task"/> that represents the execution of this middleware.</returns>
        Task Invoke(HttpContext context, RequestDelegate next, object[] args);
    }

[Mystere]

So, to be clear, the existing conforming container implementation will continue to be supported (and probably most commonly used)? This is only for non-conforming containers, or simply if someone wants to use a strongly typed implementation?

I guess my question is really, can you assure us that there is no intention of ever deprecating the dynamically dispatched version?

I'm not reassured by the comment:

The existing middleware activation logic will remain for convention-based middleware to preserve backwards compatibility.

"backwards compatibility" implies that this is going to be the preferred method.

[davidfowl]

The convention based approach will not be deprecated. We're using it for all of our own middleware. The benefits of using the interface are:

• You have a strong type
• You get activation per request, so you can inject scoped services
• If you have a non conforming container, you can get middleware activation in a first class way if you replace the IMiddlewareFactory.