Merge pull request #8735 from micalevisk/fix-duplicated-versions

feat(core,common): fix ignore duplicated versions and support an array containing VERSION_NEUTRAL
nestjs · Dec 17, 2021 · f704a43 · f704a43
2 parents ffc0389 + 094704f
commit f704a43
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 23 deletions.
diff --git a/packages/common/decorators/core/controller.decorator.ts b/packages/common/decorators/core/controller.decorator.ts
@@ -164,7 +164,9 @@ export function Controller(
         prefixOrOptions.path || defaultPath,
         prefixOrOptions.host,
         { scope: prefixOrOptions.scope },
-        prefixOrOptions.version,
+        Array.isArray(prefixOrOptions.version)
+          ? Array.from(new Set(prefixOrOptions.version))
+          : prefixOrOptions.version,
       ];
 
   return (target: object) => {

diff --git a/packages/common/decorators/core/version.decorator.ts b/packages/common/decorators/core/version.decorator.ts
@@ -7,6 +7,11 @@ import { VersionValue } from '../../interfaces/version-options.interface';
  * @publicApi
  */
 export function Version(version: VersionValue): MethodDecorator {
+  if (Array.isArray(version)) {
+    // Drop duplicated versions
+    version = Array.from(new Set(version));
+  }
+
   return (
     target: any,
     key: string | symbol,

diff --git a/packages/common/interfaces/version-options.interface.ts b/packages/common/interfaces/version-options.interface.ts
@@ -7,7 +7,10 @@ import { VersioningType } from '../enums/version-type.enum';
  */
 export const VERSION_NEUTRAL = Symbol('VERSION_NEUTRAL');
 
-export type VersionValue = string | string[] | typeof VERSION_NEUTRAL;
+export type VersionValue =
+  | string
+  | typeof VERSION_NEUTRAL
+  | Array<string | typeof VERSION_NEUTRAL>;
 
 /**
  * @publicApi

diff --git a/packages/common/test/decorators/controller.decorator.spec.ts b/packages/common/test/decorators/controller.decorator.spec.ts
@@ -7,6 +7,8 @@ describe('@Controller', () => {
   const reflectedHost = 'api.example.com';
   const reflectedHostArray = ['api1.example.com', 'api2.example.com'];
   const reflectedVersion = '1';
+  const reflectedVersionWithDuplicates = ['1', '2', '2', '1', '2', '1'];
+  const reflectedVersionWithoutDuplicates = ['1', '2'];
 
   @Controller()
   class EmptyDecorator {}
@@ -33,6 +35,9 @@ describe('@Controller', () => {
   @Controller({ version: reflectedVersion })
   class VersionOnlyDecorator {}
 
+  @Controller({ version: reflectedVersionWithDuplicates })
+  class VersionOnlyArrayDecorator {}
+
   it(`should enhance component with "${CONTROLLER_WATERMARK}" metadata`, () => {
     const controllerWatermark = Reflect.getMetadata(
       CONTROLLER_WATERMARK,
@@ -73,6 +78,11 @@ describe('@Controller', () => {
       VersionOnlyDecorator,
     );
     expect(version2).to.be.eql(reflectedVersion);
+    const version3 = Reflect.getMetadata(
+      VERSION_METADATA,
+      VersionOnlyArrayDecorator,
+    );
+    expect(version3).to.be.eql(reflectedVersionWithoutDuplicates);
   });
 
   it('should set default path when no object passed as param', () => {

diff --git a/packages/common/test/decorators/version.decorator.spec.ts b/packages/common/test/decorators/version.decorator.spec.ts
@@ -4,14 +4,27 @@ import { Version } from '../../decorators/core/version.decorator';
 
 describe('@Version', () => {
   const version = '1';
+  const versions = ['1', '2', '2', '1', '2', '1'];
+  const versionsWithoutDuplicates = ['1', '2'];
 
   class Test {
     @Version(version)
-    public static test() {}
+    public static oneVersion() {}
+
+    @Version(versions)
+    public static multipleVersions() {}
   }
 
   it('should enhance method with expected version string', () => {
-    const metadata = Reflect.getMetadata(VERSION_METADATA, Test.test);
+    const metadata = Reflect.getMetadata(VERSION_METADATA, Test.oneVersion);
     expect(metadata).to.be.eql(version);
   });
+
+  it('should enhance method with expected version array', () => {
+    const metadata = Reflect.getMetadata(
+      VERSION_METADATA,
+      Test.multipleVersions,
+    );
+    expect(metadata).to.be.eql(versionsWithoutDuplicates);
+  });
 });
diff --git a/packages/core/application-config.ts b/packages/core/application-config.ts
@@ -136,6 +136,11 @@ export class ApplicationConfig {
   }
 
   public enableVersioning(options: VersioningOptions): void {
+    if (Array.isArray(options.defaultVersion)) {
+      // Drop duplicated versions
+      options.defaultVersion = Array.from(new Set(options.defaultVersion));
+    }
+
     this.versioningOptions = options;
   }
 

diff --git a/packages/core/helpers/messages.ts b/packages/core/helpers/messages.ts
@@ -17,10 +17,12 @@ export const VERSIONED_ROUTE_MAPPED_MESSAGE = (
   method: string | number,
   version: VersionValue,
 ) => {
-  if (version === VERSION_NEUTRAL) {
-    version = 'Neutral';
-  }
-  return `Mapped {${path}, ${RequestMethod[method]}} (version: ${version}) route`;
+  const controllerVersions = Array.isArray(version) ? version : [version];
+  const versions = controllerVersions
+    .map(version => (version === VERSION_NEUTRAL ? 'Neutral' : version))
+    .join(',');
+
+  return `Mapped {${path}, ${RequestMethod[method]}} (version: ${versions}) route`;
 };
 
 export const CONTROLLER_MAPPING_MESSAGE = (name: string, path: string) =>
@@ -31,10 +33,12 @@ export const VERSIONED_CONTROLLER_MAPPING_MESSAGE = (
   path: string,
   version: VersionValue,
 ) => {
-  if (version === VERSION_NEUTRAL) {
-    version = 'Neutral';
-  }
-  return `${name} {${path}} (version: ${version}):`;
+  const controllerVersions = Array.isArray(version) ? version : [version];
+  const versions = controllerVersions
+    .map(version => (version === VERSION_NEUTRAL ? 'Neutral' : version))
+    .join(',');
+
+  return `${name} {${path}} (version: ${versions}):`;
 };
 
 export const INVALID_EXECUTION_CONTEXT = (

diff --git a/packages/core/router/route-path-factory.ts b/packages/core/router/route-path-factory.ts
@@ -23,18 +23,30 @@ export class RoutePathFactory {
   ): string[] {
     let paths = [''];
 
-    const version = this.getVersion(metadata);
-    if (version && metadata.versioningOptions?.type === VersioningType.URI) {
+    const versionOrVersions = this.getVersion(metadata);
+    if (
+      versionOrVersions &&
+      metadata.versioningOptions?.type === VersioningType.URI
+    ) {
       const versionPrefix = this.getVersionPrefix(metadata.versioningOptions);
 
-      // Version Neutral - Do not include version in URL
-      if (version !== VERSION_NEUTRAL) {
-        if (Array.isArray(version)) {
-          paths = flatten(
-            paths.map(path => version.map(v => path + `/${versionPrefix}${v}`)),
+      if (Array.isArray(versionOrVersions)) {
+        paths = flatten(
+          paths.map(path =>
+            versionOrVersions.map(version =>
+              // Version Neutral - Do not include version in URL
+              version === VERSION_NEUTRAL
+                ? path
+                : `${path}/${versionPrefix}${version}`,
+            ),
+          ),
+        );
+      } else {
+        // Version Neutral - Do not include version in URL
+        if (versionOrVersions !== VERSION_NEUTRAL) {
+          paths = paths.map(
+            path => `${path}/${versionPrefix}${versionOrVersions}`,
           );
-        } else {
-          paths = paths.map(path => path + `/${versionPrefix}${version}`);
         }
       }
     }

diff --git a/packages/core/test/application-config.spec.ts b/packages/core/test/application-config.spec.ts
@@ -1,5 +1,8 @@
 import { RequestMethod } from '@nestjs/common';
-import { GlobalPrefixOptions } from '@nestjs/common/interfaces';
+import {
+  GlobalPrefixOptions,
+  VersioningOptions,
+} from '@nestjs/common/interfaces';
 import { expect } from 'chai';
 import { ApplicationConfig } from '../application-config';
 import { ExcludeRouteMetadata } from '../router/interfaces/exclude-route-metadata.interface';
@@ -130,6 +133,13 @@ describe('ApplicationConfig', () => {
       expect(appConfig.getVersioning()).to.be.eql(options);
     });
 
+    it('should ignore duplicated versions on defaultVersion array', () => {
+      const options = { type: 'test', defaultVersion: ['1', '2', '2', '1'] };
+      appConfig.enableVersioning(options as any);
+
+      expect(appConfig.getVersioning().defaultVersion).to.be.eql(['1', '2']);
+    });
+
     it('should have undefined as the versioning by default', () => {
       expect(appConfig.getVersioning()).to.be.eql(undefined);
     });

diff --git a/packages/core/test/router/route-path-factory.spec.ts b/packages/core/test/router/route-path-factory.spec.ts
@@ -1,4 +1,4 @@
-import { RequestMethod, VersioningType } from '@nestjs/common';
+import { RequestMethod, VersioningType, VERSION_NEUTRAL } from '@nestjs/common';
 import { expect } from 'chai';
 import * as pathToRegexp from 'path-to-regexp';
 import * as sinon from 'sinon';
@@ -180,13 +180,50 @@ describe('RoutePathFactory', () => {
         }),
       ).to.deep.equal(['/api/v1.1.1', '/api/v1.2.3']);
 
+      expect(
+        routePathFactory.create({
+          ctrlPath: '',
+          methodPath: '',
+          globalPrefix: '',
+          controllerVersion: VERSION_NEUTRAL,
+          versioningOptions: {
+            type: VersioningType.URI,
+            defaultVersion: VERSION_NEUTRAL,
+          },
+        }),
+      ).to.deep.equal(['/']);
+
+      expect(
+        routePathFactory.create({
+          ctrlPath: '',
+          methodPath: '',
+          globalPrefix: '',
+          controllerVersion: ['1', VERSION_NEUTRAL],
+          versioningOptions: {
+            type: VersioningType.URI,
+            defaultVersion: ['1', VERSION_NEUTRAL],
+          },
+        }),
+      ).to.deep.equal(['/v1', '/']);
+
       expect(
         routePathFactory.create({
           ctrlPath: '',
           methodPath: '',
           globalPrefix: '',
         }),
       ).to.deep.equal(['/']);
+
+      sinon.stub(routePathFactory, 'isExcludedFromGlobalPrefix').returns(true);
+      expect(
+        routePathFactory.create({
+          ctrlPath: '/ctrlPath/',
+          methodPath: '/',
+          modulePath: '/',
+          globalPrefix: '/api',
+        }),
+      ).to.deep.equal(['/ctrlPath']);
+      sinon.restore();
     });
   });