Api route is recognized as a param.

[Root-Control]

I'm submitting a...

[ ] Regression 
[X] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Is not an express route problem.

I created different controllers with user prefix, they are in conflict.

Route for upload image --> api/users/upload Route for update/delete/read: --> api/users/:id

when "api /users/:id" is applied, a middleware is called, When entering "upload api" the middleware is called (upload is recognized as a parameter), i excluded /api/upload in middleware configuration but also its not working.

Expected behavior

Differentiate the parameter on the route

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Working with a single route prefix

Environment

Nest version: 5.1.0


For Tooling issues:
- Node version: 8.9  
- Platform:  Windows 10

Others:

[kamilmysliwiec]

Please, provide a repository that reproduces your issue.

[Root-Control]

https://github.com/Root-Control/nest-master/blob/master/src/modules/user/user.controller.ts

[kamilmysliwiec]

This is exactly how the express is designed. It's not an issue. See here: https://github.com/expressjs/express/issues/2235

So basically, the ordering matters. Ensure that endpoint with wildcard (e.g. :id) is registered after static route.

// GOOD ORDER
@Get('me') 
getProfile() {
  return 'Me';
}

@Get(':id')
findOne(@Param('id') id) {
  return id;
}

// WRONG ORDER
@Get(':id')
findOne(@Param('id') id) {
  return id;
}

@Get('me')
getProfile() {
  return 'Me';
}

[Root-Control]

Is an issue, because i tested in order but doesn't working, also in my repo i'm running a middleware function for api/users/:id routes and api/users/me is still recognized as a param, pls see my repo, this is a undeniably issue, i had the same error in express, but i knew that was about order, the order is not working in nest, when you are calling a middleware api/users/:id. Both routes are fired.

[marcus-sa]

Its still not an issue, that's just how the middleware design works when you apply it like that 😄 Instead of applying the middleware for the entire route prefix, you should do it for that single route that has the param only, or simply exclude the middleware on a specific route.

export class UserModule implements NestModule {
  public configure(consumer: MiddlewareConsumer) {
    consumer
      .apply(bodyValidatorMiddleware)
      .forRoutes('users/signup');

    consumer.apply(UserIdMiddleware)
      .exclude({ path: 'users/me', method: RequestMethod.ALL }) // <--
      .forRoutes('users/:id');
  }
}

[Root-Control]

Yes, you right, this is why i'm posting as an issue, i also tested the middleware and i have two points for you. 1.- The middleware exclude behavior has changed in the latest nestjs releases, i.e. exclude have another syntax right now.

2.- Exclude single routes like "users/me" is not working properly, this is my following code.

My user controller

import { Controller, Post,  Get, Put, Delete, Param, Req,  UseGuards,  UploadedFile,  UseInterceptors, FileInterceptor } from '@nestjs/common';

import { UserService } from './user.service';
import { AuthService } from '../auth/auth.service';
import { IToken } from '../auth/interfaces/token.interface';

// Guards
import { RolesGuard } from '../../guards/roles.guard';
import { Roles } from '../../decorators/roles.decorator';

import { MulterConfig } from '../../config/multer';

@Controller('users')
@UseGuards(RolesGuard)
export class UserController {
  constructor(private readonly userService: UserService) {}

  //  Get my user information
  @Get('me')
  @Roles('user', 'admin')
  async me(@Req() req) {
       console.log('im me');
  }

  @Put('upload')
  async uploadFile() {
      console.log('im upload');
  }

  @Delete(':id')
  async deleteUser(@Req() req) {
      console.log('im delete');
  }
}

The module middleware

export class UserModule implements NestModule{
  public configure(consumer: MiddlewareConsumer) {
    consumer.apply(UserIdMiddleware)
      .exclude(
        { path: 'users/me', method: RequestMethod.GET },
        { path: 'users/upload', method: RequestMethod.PUT })
      .forRoutes({ path: 'users/:id', method: RequestMethod.ALL });
      //  users/:id calling middleware for findById users before run another methods like "delete/update/read"
  }
}

[kamilmysliwiec]

The exclude feature doesn't work in such way, see here #853. Also, once again, it's not an issue, it's about the order. Middleware is applied before routes are mounted. I'd suggest adding a simple if statement in the middleware that compare the request URL

[khashayar]

I ran into this with following controllers.

// 1st controller
@Controller('profiles')
export class NextController {
  @Get('next')
  async execute(@User() user: UserEntity): Promise<AuditAwareInstanceType<Profile> | {}> {

  }
}

// 2nd controller
@Controller('profiles')
export class DetailController {
  @Get(':id')
  async execute(@Param('id') id: string): Promise<Profile> {

  }
}

NextController is defined first in the module definition before the DetailController. Although the response of the NextController gets exposed to the client, the issue is that both controllers get called. It's very annoying since the DetailController always throws an error in this case as next can not be casted to ObjectId:

Any advice?

[marcus-sa]

@khashayar why do you split the two controllers instead of doing all the logic in one? Instead if Nest were to use a storage to store it's metadata with method name (like I've done it in my framework), then this wouldn't occur.

That is the proposal in ECMAScript and in what order metadata gets reflected, so this is NOT an issue with Nest specifically, but rather a design flaw by using reflect-metadata over a basic metadata registry.

[szkumorowski]

I belive in case of duplicated routers - it should not override them, but clearly throw an error with ambigous routes in controllers. Based on experience with Spring Framework - it works this way.

If there is possiblity to create 2 routes in order:

• GET /me
• GET /:id and second one does not override the first - then it's fine.

In case of reversed order and if it overrides it - then it should throw an error.

[lock[bot]]

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