typestack / routing-controllers

Create structured, declarative and beautifully organized class-based controllers with heavy decorators usage in Express / Koa using TypeScript and Routing Controllers Framework.
MIT License
4.41k stars 394 forks source link

Proposal: Replacing useBefore and useAfter middlewares with Filters #203

Open adnan-kamili opened 7 years ago

adnan-kamili commented 7 years ago

Global middlewares by default should be useBefore, and action specific useBefore and useAfter middlewares should be replaced with Filters. It's more intuitive. Please refer to following:

https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/filters

NoNameProvided commented 7 years ago

Hmm I am afraid this information is not enough to start a discussion about this. A few question and notes below.

action specific

What do you mean by action specific. Routing controllers has a type called Action do you refer to that or route specific middlewares?

Can you argue more about why do you think this more intuitive? From the first few section of that article, it seems a filter in ASP.net is the same as middleware is the NodeJS world.

Filters in ASP.NET Core MVC allow you to run code before or after certain stages in the request processing pipeline.

This is what exactly middlewares does. Here we call them middlewares, because they called this way in NodeJS.

I am kind of against this. Maybe I am biased but I like the before/after middleware naming & how it works is straightforward to use. Also many other libraries has the before/after middleware concept, so shifting from it would not be a win in my opinion.

I am not familiar with ASP.net, so if I mis understood you please let me know.

adnan-kamili commented 7 years ago

Asp.Net Core has middlewares which behave exactly in the same manner as koa middlewares, even the signature is same:

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/middleware

app.Use(async (context, next) =>
{
    // Do something before execution
    await next.Invoke();
    // Do something after execution
});

But there are some operations which are specific to the actions (controller methods) which are done just before or after the controller action is executed. So microsoft after experience of decades decided to add Filters even if middlewares are present.

Filters are special middlewares which can be global or used using attributes (similar to decorators).

The reason I felt the need for same is the way cors and authorization checker syntax. The whole framework should be built using middlewares, but suddenly cors and authorization are configured in a different manner.

Authorization, interception etc. are handled by Filters by overriding the lifecycle events which different filters help us to hook to and override the behaviour. Even laravel uses filters:

https://mattstauffer.co/blog/laravel-5.0-middleware-filter-style

https://stackoverflow.com/questions/42582758/asp-net-core-middleware-vs-filters

Even if you don't agree, can you provide a justification for why authorization and cors are handled so differently? Filters actually solve this problem.

Ideally you should have allowed to override the authorization middleware, by letting users to extend it.

NoNameProvided commented 7 years ago

The reason I felt the need for same is the way cors and authorization checker syntax.

Even if you don't agree, can you provide a justification for why authorization and cors are handled so differently?

True of that, it feels strange, I would vote also to make them a middleware. However I still fail to see how would introducing a new type help the user of this lib create better apps.

Filters are special middlewares which can be global or used using attributes (similar to decorators).

So is it true that filters are basically still middlewares just with different names?

Btw, for response checking you have interceptors.

From the Laravel blog post you linked:

NOTE: Filters still exist in the codebase, so you can still use them, but middleware is becoming the preferred practice and way of thinking about decorating your routes.

Do I get it right that the thing you want to solve is to use the same middleware approach for the authorization checker and cors plugin?

I would totally love to have the ability to create middlewares which accepts parameters. (This is what makes authorization checker different. It's the only middleware you can pass params to.)

cc @pleerock What do you think about changing the middleware implementation so it can accepts params like @MyCustomAuthMiddleware([Role.Distributor, Role.Admin])? So the declaration would be something like createDecoratorParams?

github-actions[bot] commented 4 years ago

Stale issue message