swagger icon indicating copy to clipboard operation
swagger copied to clipboard

Duplicate operationIds when using nestjs Version feature

Open SamCullin opened this issue 2 years ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Current behavior

I just started upgrading my Nestjs API from version 7 to 8. The reason for the upgrade was to take advantage of the new Versions feature.

https://docs.nestjs.com/techniques/versioning

After adding versions to several of my endpoints and regenerating my swagger.json I found that endpoints with multiple versions attached to them have duplicate operationIds.

I have created a hacky patch-package for this where I just check if the path contains 'v2' and add a 'v2' to the operation Id.

Minimum reproduction code

https://codesandbox.io/s/nestjs-swagger-mrp-dupeoperationid-owkhm?file=/src/main.ts

Steps to reproduce

Run The app and check swagger

  1. Open New Terminal
  2. yarn start
  3. open https://owkhm-3001.sse.codesandbox.io/swagger-json

Run test that checks for duplicate operationId

  1. Open New Terminal
  2. yarn test:e2e

Expected behavior

Each of the endpoints rendered in the swagger document should have a unique operationId

Package version

5.1.5

NestJS version

8.1.1

Node.js version

v14.18.1

In which operating systems have you tested?

  • [ ] macOS
  • [ ] Windows
  • [X] Linux

Other

I found this issue when copying the swagger.json into the below site. https://editor.swagger.io/

This error also does not pass the validation that many tools do before using the swagger.json for code generation.

Example error Produced

Semantic error at paths./api/v2/
Operations must have unique operationIds.
Jump to line 3172

SamCullin avatar Dec 06 '21 08:12 SamCullin

Would you like to create a PR to address this issue?

kamilmysliwiec avatar Dec 06 '21 11:12 kamilmysliwiec

I can give it a shot this weekend.

SamCullin avatar Dec 07 '21 03:12 SamCullin

Hey just reviving this ticket. Is there a preferred approach to fixing this issue?

To clarify - the issue is caused by controller methods that are used for multiple versions. This occurs if the Controller class itself is set to support multiple versions:

@Controller({ path: 'myController', version: ['v1', 'v2'] })

Or if a method is set to handle multiple versions:

@Version(['v1,'v2'])

The reason this fails is because operationIdFactory only has the controller and method names passed in as parameters - but in our case we are using the same controller method for multiple operations. These should have unique operation IDs.

I think the simplest path forward would be to add a 3rd optional parameter to operationIdFactory that specifies the version of the operation. This would be backwards compatible, as version would be undefined if Nest's versioning is not enabled.

The default would be something like controllerKey_versionKey_methodKey, but it means you could omit the version prefix if, for example, the operation was for the default version.

I am happy to do this work, would just like feedback on the suggested approach.

MichaelMarner avatar Jun 20 '22 05:06 MichaelMarner

+1

kembala avatar Nov 30 '22 12:11 kembala

Let's track this here https://github.com/nestjs/swagger/pull/1949

kamilmysliwiec avatar Feb 06 '23 10:02 kamilmysliwiec