allow custom middlewares for controllers and methods

[cheng81]

Middleware decorators

This PR introduces a bunch of new decorators (and small changes to the route templates) that allows the usage of express, koa, and hapi middlewares at the route and individual method level.
Different middlewares can be specified depending on the server technology used (not completely sure this is actually useful, but it helps type-wise).

Example:

import { Route, Get, Middlewares, ExpressMiddlewares } from '@tsoa/runtime';
@Middlewares({
  express: [
    middleware1,
    middleware2,
  ],
  koa: [
    koaMiddleware1,
  ],
})
@Route('Foo')
export class MiddlewareTestController {
  @ExpressMiddlewares(
    middleware3,
    middleware4,
  )
  @Get('/foo')
  public async foo(): Promise<void> {
    return;
  }
}

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Have you written unit tests?
• Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)? no, but all the other Controllers do not have middlewares, and they continue to work
• This PR is associated with an existing issue?

Closing issues

closes #948
closes #624
closes #62
closes #47

If this is a new feature submission:

• Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

I have some quirks with the current solution:

1. It's not elegant - middlewares are "stored" in controllers constructor and method functions, and while this works (and has been used somewhat in tsoa already - I'm looking at you request.user defined in the authentication middleware), it also feels like an hack rather than a proper solution. If people have a better idea in mind, please do share!
2. The various middleware types are super vague (e.g. ExpressMiddleware is defined as (req: any, res: any, next: any) => Promise<any>;) since the various server libraries are not dependencies of tsoa. While I don't think it'll cause problems in practice, I would love if there was a way to make them more precise (perhaps requiring just the types libraries?).
3. I also have a bit of a problem with ergonomics: in theory I could define a single Middleware decorator that accepts ...middlewares: any[]) and be done with that: in practice I'm pretty sure that all controllers are created for a single server technology, so having @Middlewares({express: [...], koa: [...]}) is really there just for the benefit of the tests.
4. finally, perhaps middlewares should really be a configuration of other decorators: Route and the various http methods (Get, Post, and so on).

Test plan

I added a MiddlewareTest controller with fake middlewares and check in the tests that they are indeed executed.

[github-actions]

Hello there cheng81 👋

Thank you and congrats 🎉 for opening your first PR on this project.✨

We will review the following PR soon! 👀

[WoH]

I'd prefer using the reflect API here.
It seems like you're reimplementing some of the concepts that API offers manually in the template etc. Is there a reason why your approach is preferable that I'm missing?

[cheng81]

Hi, I'll try to take a look at the reflect API and let you know.
I'm not sure what I am reimplementing, if there's already a way to set custom middlewares for the various servers please let me know.
I should add that I'd really prefer to not use a custom routes template, and this feature seems to have been requested a number of times, so it seems to me that it's something the tsoa users would like.

[WoH]

What this PR is doing is adding properties onto the target which constitutes metadat.
However, there's an API that works well and avoids all of the properties: Reflect.defineMetadata and Reflect.getMetadata.

https://rbuckton.github.io/reflect-metadata/

The changes should not be too big:

const TSOA_EXPRESS_MIDDLEWARE = Symbol("TSOA_EXPRESS_MIDDLEWARE")

// ....

export function Middlewares(middlewares: Middlewares) {
  return (
      target: object,
      key: string | symbol,
      descriptor: TypedPropertyDescriptor<any>,
    ) => {
      Reflect.defineMetadata(TSOA_EXPRESS_MIDDLEWARE, middlewares.express, descriptor.value);
      return descriptor;
    };
}

[cheng81]

@WoH now I see what you mean, I didn't know about reflect-metadata, I updated the code to make use of it.

[WoH]

Mostly ready, I have some minor inconveniences when it comes to maintenance and UX for the end users. If you could try to tighten the typing a bit more, that'd be very appreciated.

[WoH]

Would prefer if we could try to type this properly as they are visible

[cheng81]

I'll try to do it, though that means I'll have to add other dependencies (the typing of express, koa and hapi) to the project.
While this means we'll need to keep up-to-date with those dependencies too, at least we'll know when/if they introduce non-retro compatible changes in their middleware definitions (not that I expect those to happen, but in theory they could do something very asinine like, e.g., putting next as 1st parameter instead of the 3rd for the express middleware)

[WoH]

That's probably worth the
a) DX
b) as you mentioned, the chance to detect potential issues earlier

[dderevjanik]

TBH, I wouldn't mind to have middlewares not typed (using any). I think it's better solution than having dependencies on express, koa and hapi types and maintaining those dependencies...

[dderevjanik]

@cheng81 Sorry for asking, but what's reason behind using framework specific decorator-middlewares?
Do you need that kind of information in api-generation or in runtime, or its only because of type inference?

AFAIK, for type inference you can use generics in your @Middleware decorator. e.g. for express:

import { RequestHandler } from 'express'; // Importing type from express instead of TSOA
import { Route, Get, Middlewares } from '@tsoa/runtime';

// Using generic to determine correct type of middleware
@Middlewares<RequestHandler>(middleware1)
@Route('Foo')
export class MiddlewareTestController {

  @Middlewares<RequestHandler>(middleware2, middleware3)
  @Get('/foo')
  public async foo(): Promise<void> {
    return;
  }
}

[cheng81]

@cheng81 Sorry for asking, but what's reason behind using framework specific decorator-middlewares? Do you need that kind of information in api-generation or in runtime, or its only because of type inference?

AFAIK, for type inference you can use generics in your @Middleware decorator. e.g. for express:

import { RequestHandler } from 'express'; // Importing type from express instead of TSOA
import { Route, Get, Middlewares } from '@tsoa/runtime';

// Using generic to determine correct type of middleware
@Middlewares<RequestHandler>(middleware1)
@Route('Foo')
export class MiddlewareTestController {

  @Middlewares<RequestHandler>(middleware2, middleware3)
  @Get('/foo')
  public async foo(): Promise<void> {
    return;
  }
}

@dderevjanik no worries, you shouldn't be sorry for asking!
So the main reason is basically security (so yes, type checking more than type inference), although I do see that in practice that would be probably a non-existent problem: in theory, by using a single Middlewares decorator, it would be possible to add, say, an express middleware and generate the routes for, say, koa. Of course this would be quickly caught by the devs.

Even using a single middleware with generics would be not ideal (in my head, at least): nothing prevents someone from, say, using @Middlewares<string> instead of the right type - again, this is stupid and no one in their sane mind would do that, but the fact that the system allows that kinda bothers me.

Perhaps there's a way to define Middleware<T: ExpressMiddleware | KoaMiddleware | HapiMiddleware> but we go back to square one: I need those types defined in tsoa.

If we follow the same patter tsoa has taken so far, then I'd just define them as any (since the compiled routes files has a tendency of using any everywhere), but honestly I'm not too much happy with it.

Also as I mentioned in the previous comment, in some way, the version of the framework used is already fixed: the code used in the route templates relies on the version it has been used, and should a major version change some of the code the routes.ts file uses, it would lead to nasty surprises.

Finally, of course I can adjust and implement a solution everybody can agree on, I just wanted to present all the reasoning behind what I came up with.

[dderevjanik]

@cheng81 I see and completely understand what are you trying to achieve. I really like idea of having custom middlewares in TSOA (thank you for opening PR 🚀 ). My opinions are highly subjective.

Personally, for me, its a little bit confusing to have defined property express inside all middleware decorators like
@Middlewares({ express: [ ] }) or having decorators only for specific framework like @ExpressMiddlewares([]). It seems that there's a lot of complexity behind so many specific middlewares and generic one.

Some pros/cons with current approach:

• 👍 Correctly typed middlewares for different frameworks
• 👎 Confusing to have @Middlewares({ express: [] }) and @ExpressMiddlewares([]). Why not only one?
• 👎 Having type dependencies (internal/external) on Express, Koa and Hapi in TSOA
• 👎 A lot more complexity in TSOA code (than having only one generic middleware)

Don't get me wrong, but I would like to suggest to split this PR into 2 iteration, or separated PRs.

• First PR: Having generic @Middlewares<F extends Function>([]) decorator seems like first iteration, easy to implement, easy to understand and super easy to use. Not type-safe (depends on user - I hope that user is not completely insane).
• Second PR: Having type-safe @Middlewares<, which could provide type safety besides easy to use approach. We can have further research or discussion.

[cheng81]

@dderevjanik @WoH this might be overkilling, but what about having different packages (e.g. @tsoa/middleware-express, etc) that all exports a single Middleware decorator with the right typing? As I said, probably overkilling, but just an idea.

I'll possibly implement for now what @dderevjanik suggested (though it'll need to be something like @Middlewares<T extends Function | Object> since hapi middlewares are.. well, a bit peculiar).

[WoH]

I'd rather see if we can use declaration merging in that case (not having all 3 types)

[cheng81]

I'd rather see if we can use declaration merging in that case (not having all 3 types)

@WoH I tried to look at declaration merging, but I'm failing to see how it would help here, could you perhaps elaborate a bit?

[WoH]

I'd rather see if we can use declaration merging in that case (not having all 3 types)

@WoH I tried to look at declaration merging, but I'm failing to see how it would help here, could you perhaps elaborate a bit?

This may not work as is, because of the non-ambient vs. ambient context in tests, but the idea is roughly to produce:

export declare function ExpressMiddlewares(...middlewares: any[]): ClassDecorator & MethodDecorator;

and, as a user, I do:

declare module '@tsoa/runtime' {
  export function ExpressMiddlewares(...middlewares: Array<import('express').RequestHandler>): ClassDecorator & MethodDecorator;
}

[cheng81]

@WoH yeah, I tried and no matter what, it kept complaining about ambient vs non-ambient errors.
In principle we could also generate an Middlewares function in the routes.ts, though that has some "chicken and egg" problem.
Anyhow, I changed the PR and removed the types dependencies, and simplified the whole thing by having a single Middlewares<T> function, let me know what you and @dderevjanik think!

[dderevjanik]

@cheng81 It looks amazing! Thank you 👍

[michaelvalasanyan]

I'm not able to get this in latest npm package ... is there a timeline for releasing it ?

[WoH]

https://github.com/lukeautry/tsoa/releases/tag/v4.0.0-alpha.0

[saqibarfeen]

I installed it via yarn add tsoa@next , but my middleware is not getting called, am I missing something?

@Route("ping")
@Tags("Ping")
export class PingController extends Controller {
    @Get()
    @Middlewares(async (req, res, next) => {
        console.log("before mw"); // this doesn't get called before the getPing controller
        next();
    })
    public async getPing(@Request() request: express.Request): Promise<any> {
        const resp: Response = { errors: [] };
        this.setStatus(200);
        return {};
    }

[RevanthGovindan]

@saqibarfeen same issue for me as well, middleware doesn't called.

[WoH]

@RevanthGovindan @saqibarfeen Please make sure you have Reflect API in scope and experimentalDecorators on.
If you can post a reproduction case, please open up an issue.

[RevanthGovindan]

@WoH
"experimentalDecorators": true in tsconfig.json and import 'reflect-metadata'; in index file, still middleware doesn't called
#1173

[LouisCharles70]

Hi @cheng81, do you have a working example with the feature you just added ? Can't find a way to make it work somehow...

[AnthonySLWhite]

Do we have a fix on this issue yet? ⬆️

[WoH]

Any repro yet?