Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Why don't we have an IMiddleware interface ? #729

Closed
glacasa opened this issue Oct 20, 2016 · 2 comments
Closed

Why don't we have an IMiddleware interface ? #729

glacasa opened this issue Oct 20, 2016 · 2 comments
Milestone

Comments

@glacasa
Copy link

glacasa commented Oct 20, 2016

I was reading the documentation about the middleware, and I saw something really strange : to create a Middleware, we can write a class with an Invoke method

public async Task Invoke(HttpContext context)

But we don't have an interface to define this.
https://docs.asp.net/en/latest/fundamentals/middleware.html#writing-middleware

The interface would be just something like this :

public interface IMiddleware
{
    Task Invoke(HttpContext context);
}

The problem is that we can call the function UseMiddleware with any class, and if the class doesn't contains the correct method we get the error at runtime instead of compilation. And I think the reflection code will be less performant than calling an interface method (didn't benchmark, I will trust you if you say no)

After browsing the source code I understood that we can use dependency injection in the Invoke method (like in the constructor), and it may be the reason we don't have a IMiddleware.

But this is not documented, and I don't think it will be used by many people. I even looked at several default aspnet middlewares, and I didn't see Dependency injection used there (it is always used in the constructor)

Is it really a good idea to loose the benefits of having an Interface in favor of an undocumented barely used functionnality ? Are there other benefits that I didn't see ?

@Tratcher
Copy link
Member

The core abstraction is RequestDelegate. https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.Http.Abstractions/RequestDelegate.cs#L13

Classes with an Invoke method are only one pattern on top of the core abstraction. Inline lamdas are another common pattern:
https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.Http.Abstractions/Extensions/UseWhenExtensions.cs#L46

The DI scenario does discourage the use of interfaces, and the reflection perf hasn't been a concern since the reflection is resolved once at startup rather than per request.

@muratg muratg added this to the Discussions milestone Oct 20, 2016
@JunTaoLuo
Copy link
Contributor

Duplicate of #754

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants