typestack/routing-controllers

Question: @UseBefore on a middleware ?

AdmanDev opened this issue · 10 comments

Hello,
I have to execute a middleware before another.
I tried to use @UseBefore on my middelware but not working.

Like this :

@Service()
class MiddlewareOne implements ExpressMiddlewareInterface {}

@Service()
@UseBefore(MiddlewareOne )
class MiddlewareTwo implements ExpressMiddlewareInterface {}

@Service()
@UseBefore(MiddlewareTwo)
class MyController {}

Only MiddlewareTwo is executed !

Is there any solution ? Or maybe add this feature ?

PS: Sorry for my english, I'm french :)

Thank you for your help

Middlewares dont handle additional actions at the moment and frankly I dont really see the point. You can just inject the other middleware through contructor and call use on it while providing the next function.

If you can provide some use case we can look into it.

Middlewares dont handle additional actions at the moment and frankly I dont really see the point. You can just inject the other middleware through contructor and call use on it while providing the next function.

If you can provide some use case we can look into it.

@attilaorosz Thank you for your answer !

I injected the other middleware through contructor and called use on it but when an error occured in the injected middleware, the error middleware is not executed.

Using @UseBefore is faster than injecting middleware and using it as NextFunction.
Also, it allows to write less code and have cleaner code, in my opinion

Could you please provide an example on how you tried to inject and call use on the other middleware? The error handler should be called.

Regarding the general idea, I get it thats its faster but it opens a whole new can of worms because then we should do the same for UseAfter and interceptors to keep it consistent. Also its not really aligning with the flow of express and koa where the idea is that a middleware is pure and does not have their own “guards”.

But im open for discussion
@NoNameProvided any thoughts?

Is there a reason simply specifying @UseBefore multiple times on the route doesn't work for you? You can do the following:

@Service()
class MiddlewareOne implements ExpressMiddlewareInterface {
  use(request: any, response: any, next: (err?: any) => any) {
    console.log('Handling in middleware one.')

    return next();
  }
  
}

@Service()
class MiddlewareTwo implements ExpressMiddlewareInterface {
  use(request: any, response: any, next: (err?: any) => any) {
    console.log('Handling in middleware two.');

    return next();
  }
}

@Service()
@JsonController('/v1/health-check')
export class HealthCheckroute {
  @Get('')
  @UseBefore(MiddlewareOne)
  @UseBefore(MiddlewareTwo)
  public async azureHealthCheck(): Promise<ApiResponse<any>> {
    return { payload: { status: 'ok' } };
  }
}

In the above example, both middlewares will be called (the order they are defined on the route matters).

@NoNameProvided Yes this code work for me, but the middleware two is called before middleware one.
(I don't change the order)

I wrote this snippet top of my head, so maybe you need to use reverse order then:

@UseBefore(MiddlewareTwo)
@UseBefore(MiddlewareOne)

(which is unintuitive but should work)

@NoNameProvided Yes, it work after reversing the order . :)

I retry to call "use" method of MiddlewareOne in my MiddlewareTwo and I added "await" in my call. It work now !

According to the code, you should also be able to group the middleware within a single UseBefore annotation.

@UseBefore([MiddlewareOne, MiddlewareTwo])

The current implementation will respect the order in the array. Unfortunately, I couldn't find any tests to link here to help understand @UseBefore better.

Reference: https://github.com/typestack/routing-controllers/blob/develop/src/decorator/UseBefore.ts

export function UseBefore(
  ...middlewares: Array<Function | ((request: any, response: any, next: Function) => any)>
): Function {
  return function (objectOrFunction: Object | Function, methodName?: string) {
    middlewares.forEach(middleware => {
      getMetadataArgsStorage().uses.push({
        target: methodName ? objectOrFunction.constructor : (objectOrFunction as Function),
        method: methodName,
        middleware: middleware,
        afterAction: false,
      });
    });
  };
}

Closing due to inactivity

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.