krakenjs / swaggerize-express

Design-driven apis with swagger 2.0 and express.
Other
354 stars 81 forks source link

Bug in routing #110

Open slinkydeveloper opened 7 years ago

slinkydeveloper commented 7 years ago

I've got the swagger spec like this:

paths:
  /users/{query}: (get, put, delete)
  /users/findUser: (get)
  /users/newUser: (post)

The paths are defined in this order. After the implementation of handlers, if i try to call the path /users/findUser it will execute the handler for path /users/{query}. I know that usually in an express application if i define a path with path parameter before another path without path parameter express will execute the handler of path with path parameter, but in this case swagger specs doesn't have to be depending to implementation.

Thank you

tlivings commented 7 years ago

This module uses the express router. I've not seen a router that would avoid this issue.

Are you saying that calling post on /usres/findUser calls the query route? Because it shouldn't due to the methods you've declared above.

ksmithut commented 7 years ago

I think the issue is the order in which they registered in react router, so if in the spec you define /users/{query} before /users/findUser, the path will match that one first.

I know those routes are probably just an example, but if you follow more closely to REST standards, you shouldn't have this issue:

paths:
  /users: (post: createUser, get: queryUsers)
  /users/{id}: (get: findOne, put: update, delete: removeUser)

@slinkydeveloper Have you tried reordering your paths to make the /users/{query} appear after the others?

slinkydeveloper commented 7 years ago

Resolved reordering the paths. But i think that when paths are added in swaggerize-express, paths without regex must be added before paths with regex, to avoid this issue. As I said, IMHO api specification must be indipendent from implementation

slinkydeveloper commented 7 years ago

@tlivings sorry typing error i updated the issue description