Repository: nestjs/swagger (ts)

Issue:
Duplicate operationIds when using nestjs Version feature
### 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
```


Pull request:
feat(swagger): Provide URI version to operationIdFactory
Adds a new optional parameter to operationIdFactory to allow providing the API version prefix added to the operation's path.

This allows the operationIdFactory to generate unique IDs in the case of the same controller method being used for multiple API versions. Default behavior is unchanged - the operation IDs will be of the form controllerKey_methodKey, but where necessary this can be customised using the URI version prefix.

The implementation here gets the versions that the method is for, and generates the URI prefixes. It then looks for the version prefixes in the paths returned from `RoutePathFactory`, and provides that information to operationIdFactory, so that unique IDs can be generated.

I'm not _super_ familiar with Nest internals, but this looks like the only real way to implement this without making more extensive changes to `nest-router`, which would have potentially wider side effects. 

Closes #1711

## PR Checklist
Please check if your PR fulfills the following requirements:

- [X] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [X] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)


## PR Type
What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->
- [x] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:

## What is the current behavior?
When using Nest's API versioning feature, and having a controller method cover multiple versions, nestjs-swagger generates invalid OpenAPI spec, due to duplicate operation IDs.

It generates duplicate operation IDs because only the controller and method names are available to `operationIdFactory` - but in these cases the same controller and method is used multiple times.

Issue Number: #1711 


## What is the new behavior?

When URI versioning is used, an additional parameter containing the version is provided to `operationIdFactory`. This allows developers to provide unique operation IDs in the case of a method being used in multiple versions.

## Does this PR introduce a breaking change?
- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->


## Other information


Patch:
diff --git a/lib/interfaces/swagger-document-options.interface.ts b/lib/interfaces/swagger-document-options.interface.ts
index 041c6222..3c8c9630 100644
--- a/lib/interfaces/swagger-document-options.interface.ts
+++ b/lib/interfaces/swagger-document-options.interface.ts
@@ -1,3 +1,15 @@
+export type OperationIdFactory =
+  | ((
+      controllerKey: string,
+      methodKey: string,
+      pathVersionKey?: string
+    ) => string)
+  | ((
+      controllerKey: string,
+      methodKey: string,
+      pathVersionKey?: string
+    ) => string);
+
 export interface SwaggerDocumentOptions {
   /**
    * List of modules to include in the specification
@@ -24,5 +36,5 @@ export interface SwaggerDocumentOptions {
    * based on the `controllerKey` and `methodKey`
    * @default () => controllerKey_methodKey
    */
-  operationIdFactory?: (controllerKey: string, methodKey: string) => string;
+  operationIdFactory?: OperationIdFactory;
 }
diff --git a/lib/swagger-explorer.ts b/lib/swagger-explorer.ts
index 75d7862b..fd24c3c9 100644
--- a/lib/swagger-explorer.ts
+++ b/lib/swagger-explorer.ts
@@ -52,6 +52,7 @@ import {
   exploreApiTagsMetadata,
   exploreGlobalApiTagsMetadata
 } from './explorers/api-use-tags.explorer';
+import { OperationIdFactory } from './interfaces';
 import { DenormalizedDocResolvers } from './interfaces/denormalized-doc-resolvers.interface';
 import { DenormalizedDoc } from './interfaces/denormalized-doc.interface';
 import {
@@ -67,8 +68,10 @@ export class SwaggerExplorer {
   private readonly mimetypeContentWrapper = new MimetypeContentWrapper();
   private readonly metadataScanner = new MetadataScanner();
   private readonly schemas: Record<string, SchemaObject> = {};
-  private operationIdFactory = (controllerKey: string, methodKey: string) =>
-    controllerKey ? `${controllerKey}_${methodKey}` : methodKey;
+  private operationIdFactory: OperationIdFactory = (
+    controllerKey: string,
+    methodKey: string
+  ) => (controllerKey ? `${controllerKey}_${methodKey}` : methodKey);
   private routePathFactory?: RoutePathFactory;
 
   constructor(private readonly schemaObjectFactory: SchemaObjectFactory) {}
@@ -78,7 +81,7 @@ export class SwaggerExplorer {
     applicationConfig: ApplicationConfig,
     modulePath?: string | undefined,
     globalPrefix?: string | undefined,
-    operationIdFactory?: (controllerKey: string, methodKey: string) => string
+    operationIdFactory?: OperationIdFactory
   ) {
     this.routePathFactory = new RoutePathFactory(applicationConfig);
     if (operationIdFactory) {
@@ -274,6 +277,23 @@ export class SwaggerExplorer {
       metatype,
       applicationConfig.getVersioning()
     );
+
+    const versionOrVersions = methodVersion ?? controllerVersion;
+    let versions = [];
+    if (!!versionOrVersions) {
+      if (!Array.isArray(versionOrVersions)) {
+        versions = [versionOrVersions];
+      } else {
+        versions = versionOrVersions as string[];
+      }
+
+      const versionConfig = applicationConfig.getVersioning();
+      if (versionConfig?.type == VersioningType.URI) {
+        const prefix = this.routePathFactory.getVersionPrefix(versionConfig);
+        versions = versions.map((v) => `${prefix}${v}`);
+      }
+    }
+
     const allRoutePaths = this.routePathFactory.create(
       {
         methodPath,
@@ -292,19 +312,25 @@ export class SwaggerExplorer {
         DECORATORS.API_EXTENSION,
         method
       );
+      const pathVersion = versions?.filter((v) => fullPath.includes(v))[0];
       return {
         method: RequestMethod[requestMethod].toLowerCase(),
         path: fullPath === '' ? '/' : fullPath,
-        operationId: this.getOperationId(instance, method),
+        operationId: this.getOperationId(instance, method, pathVersion),
         ...apiExtension
       };
     });
   }
 
-  private getOperationId(instance: object, method: Function): string {
+  private getOperationId(
+    instance: object,
+    method: Function,
+    pathVersion?: string
+  ): string {
     return this.operationIdFactory(
       instance.constructor?.name || '',
-      method.name
+      method.name,
+      pathVersion
     );
   }
 
diff --git a/lib/swagger-scanner.ts b/lib/swagger-scanner.ts
index 5a1d6614..a3812264 100644
--- a/lib/swagger-scanner.ts
+++ b/lib/swagger-scanner.ts
@@ -4,7 +4,11 @@ import { ApplicationConfig, NestContainer } from '@nestjs/core';
 import { InstanceWrapper } from '@nestjs/core/injector/instance-wrapper';
 import { InstanceToken, Module } from '@nestjs/core/injector/module';
 import { flatten, isEmpty } from 'lodash';
-import { OpenAPIObject, SwaggerDocumentOptions } from './interfaces';
+import {
+  OpenAPIObject,
+  OperationIdFactory,
+  SwaggerDocumentOptions
+} from './interfaces';
 import { ModuleRoute } from './interfaces/module-route.interface';
 import {
   ReferenceObject,
@@ -106,7 +110,7 @@ export class SwaggerScanner {
     modulePath: string | undefined,
     globalPrefix: string | undefined,
     applicationConfig: ApplicationConfig,
-    operationIdFactory?: (controllerKey: string, methodKey: string) => string
+    operationIdFactory?: OperationIdFactory
   ): ModuleRoute[] {
     const denormalizedArray = [...routes.values()].map((ctrl) =>
       this.explorer.exploreController(


Test patch:
diff --git a/test/explorer/swagger-explorer.spec.ts b/test/explorer/swagger-explorer.spec.ts
index 9b3e0d32..941f4e1b 100644
--- a/test/explorer/swagger-explorer.spec.ts
+++ b/test/explorer/swagger-explorer.spec.ts
@@ -44,6 +44,11 @@ describe('SwaggerExplorer', () => {
     controllerKey: string,
     methodKey: string
   ) => `${controllerKey}.${methodKey}`;
+  const controllerKeyMethodKeyVersionKeyOperationIdFactory = (
+    controllerKey: string,
+    methodKey: string,
+    versionKey: string
+  ) => `${controllerKey}.${methodKey}.${versionKey}`;
 
   describe('when module only uses metadata', () => {
     class Foo {}
@@ -1260,60 +1265,148 @@ describe('SwaggerExplorer', () => {
           defaultVersion: 'THIS_SHOULD_NOT_APPEAR_ANYWHERE'
         });
       });
+      describe('and using the default operationIdFactory', () => {
+        it('should use controller version defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithVersionController(),
+              metatype: WithVersionController
+            } as InstanceWrapper<WithVersionController>,
+            config,
+            'modulePath',
+            'globalPrefix'
+          );
+
+          expect(routes[0].root.path).toEqual(
+            `/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
+          );
+          expect(routes[0].root.operationId).toEqual(
+            `WithVersionController_foo`
+          );
+        });
 
-      it('should use controller version defined', () => {
-        const routes = explorer.exploreController(
-          {
-            instance: new WithVersionController(),
-            metatype: WithVersionController
-          } as InstanceWrapper<WithVersionController>,
-          config,
-          'modulePath',
-          'globalPrefix'
-        );
-
-        expect(routes[0].root.path).toEqual(
-          `/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
-        );
-      });
-
-      it('should use route version defined', () => {
-        const routes = explorer.exploreController(
-          {
-            instance: new WithVersionController(),
-            metatype: WithVersionController
-          } as InstanceWrapper<WithVersionController>,
-          config,
-          'modulePath',
-          'globalPrefix'
-        );
+        it('should use route version defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithVersionController(),
+              metatype: WithVersionController
+            } as InstanceWrapper<WithVersionController>,
+            config,
+            'modulePath',
+            'globalPrefix'
+          );
+
+          expect(routes[1].root.path).toEqual(
+            `/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
+          );
+          expect(routes[1].root.operationId).toEqual(
+            `WithVersionController_bar`
+          );
+        });
 
-        expect(routes[1].root.path).toEqual(
-          `/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
-        );
+        it('should use multiple versions defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithMultipleVersionsController(),
+              metatype: WithMultipleVersionsController
+            } as InstanceWrapper<WithMultipleVersionsController>,
+            config,
+            'modulePath',
+            'globalPrefix'
+          );
+
+          expect(routes[0].root.path).toEqual(
+            `/globalPrefix/v${
+              CONTROLLER_MULTIPLE_VERSIONS[0] as string
+            }/modulePath/with-multiple-version`
+          );
+          expect(routes[0].root.operationId).toEqual(
+            `WithMultipleVersionsController_foo`
+          );
+          expect(routes[1].root.path).toEqual(
+            `/globalPrefix/v${
+              CONTROLLER_MULTIPLE_VERSIONS[1] as string
+            }/modulePath/with-multiple-version`
+          );
+          expect(routes[1].root.operationId).toEqual(
+            `WithMultipleVersionsController_foo`
+          );
+        });
       });
+      describe('and has an operationIdFactory that uses the method version', () => {
+        it('should use controller version defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithVersionController(),
+              metatype: WithVersionController
+            } as InstanceWrapper<WithVersionController>,
+            config,
+            'modulePath',
+            'globalPrefix',
+            controllerKeyMethodKeyVersionKeyOperationIdFactory
+          );
+
+          expect(routes[0].root.path).toEqual(
+            `/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
+          );
+          expect(routes[0].root.operationId).toEqual(
+            `WithVersionController.foo.v${CONTROLLER_VERSION}`
+          );
+        });
 
-      it('should use multiple versions defined', () => {
-        const routes = explorer.exploreController(
-          {
-            instance: new WithMultipleVersionsController(),
-            metatype: WithMultipleVersionsController
-          } as InstanceWrapper<WithMultipleVersionsController>,
-          config,
-          'modulePath',
-          'globalPrefix'
-        );
+        it('should use route version defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithVersionController(),
+              metatype: WithVersionController
+            } as InstanceWrapper<WithVersionController>,
+            config,
+            'modulePath',
+            'globalPrefix',
+            controllerKeyMethodKeyVersionKeyOperationIdFactory
+          );
+
+          expect(routes[1].root.path).toEqual(
+            `/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
+          );
+          expect(routes[1].root.operationId).toEqual(
+            `WithVersionController.bar.v${METHOD_VERSION}`
+          );
+        });
 
-        expect(routes[0].root.path).toEqual(
-          `/globalPrefix/v${
-            CONTROLLER_MULTIPLE_VERSIONS[0] as string
-          }/modulePath/with-multiple-version`
-        );
-        expect(routes[1].root.path).toEqual(
-          `/globalPrefix/v${
-            CONTROLLER_MULTIPLE_VERSIONS[1] as string
-          }/modulePath/with-multiple-version`
-        );
+        it('should use multiple versions defined', () => {
+          const routes = explorer.exploreController(
+            {
+              instance: new WithMultipleVersionsController(),
+              metatype: WithMultipleVersionsController
+            } as InstanceWrapper<WithMultipleVersionsController>,
+            config,
+            'modulePath',
+            'globalPrefix',
+            controllerKeyMethodKeyVersionKeyOperationIdFactory
+          );
+
+          expect(routes[0].root.path).toEqual(
+            `/globalPrefix/v${
+              CONTROLLER_MULTIPLE_VERSIONS[0] as string
+            }/modulePath/with-multiple-version`
+          );
+          expect(routes[0].root.operationId).toEqual(
+            `WithMultipleVersionsController.foo.v${
+              CONTROLLER_MULTIPLE_VERSIONS[0] as string
+            }`
+          );
+          expect(routes[1].root.path).toEqual(
+            `/globalPrefix/v${
+              CONTROLLER_MULTIPLE_VERSIONS[1] as string
+            }/modulePath/with-multiple-version`
+          );
+          expect(routes[1].root.operationId).toEqual(
+            `WithMultipleVersionsController.foo.v${
+              CONTROLLER_MULTIPLE_VERSIONS[1] as string
+            }`
+          );
+        });
       });
     });