fix(metadata): fix method-level parameter decorators

loopbackio · Dec 21, 2017 · c5127d4 · c5127d4
1 parent 721e8ee
commit c5127d4
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 5 deletions.
diff --git a/packages/metadata/src/decorator-factory.ts b/packages/metadata/src/decorator-factory.ts
@@ -595,13 +595,63 @@ export class MethodParameterDecoratorFactory<T> extends DecoratorFactory<
   MetadataMap<T[]>,
   MethodDecorator
 > {
+  /**
+   * Find the corresponding parameter index for the decoration
+   * @param target
+   * @param methodName
+   * @param methodDescriptor
+   */
+  private getParameterIndex(
+    target: Object,
+    methodName?: string | symbol,
+    methodDescriptor?: TypedPropertyDescriptor<any> | number,
+  ) {
+    const numOfParams = this.getNumberOfParameters(target, methodName);
+    // Fetch the cached parameter index
+    let index = Reflector.getOwnMetadata(
+      this.key + ':index',
+      target,
+      methodName,
+    );
+    // Default to the last parameter
+    if (index == null) index = numOfParams - 1;
+    if (index < 0) {
+      // Excessive decorations than the number of parameters detected
+      const method = this.getTargetName(target, methodName, methodDescriptor);
+      throw new Error(
+        `The decorator is used more than ${numOfParams} time(s) on ${method}`,
+      );
+    }
+    return index;
+  }
+
   protected mergeWithInherited(
     inheritedMetadata: MetadataMap<T[]>,
     target: Object,
     methodName?: string | symbol,
     methodDescriptor?: TypedPropertyDescriptor<any> | number,
   ) {
-    return {[methodName!]: [this.spec]};
+    inheritedMetadata = inheritedMetadata || {};
+    const index = this.getParameterIndex(target, methodName, methodDescriptor);
+
+    const inheritedParams =
+      inheritedMetadata[methodName!] || new Array(index + 1).fill(undefined);
+    if (inheritedParams.length) {
+      // First time applied to a method. This is the last parameter of the method
+      inheritedParams[index] = this.withTarget(
+        <T>this.inherit(inheritedParams[index]),
+        target,
+      );
+    }
+    // Cache the index to help us position the next parameter
+    Reflector.defineMetadata(
+      this.key + ':index',
+      index - 1,
+      target,
+      methodName,
+    );
+    inheritedMetadata[methodName!] = inheritedParams;
+    return inheritedMetadata;
   }
 
   protected mergeWithOwn(
@@ -611,9 +661,19 @@ export class MethodParameterDecoratorFactory<T> extends DecoratorFactory<
     methodDescriptor?: TypedPropertyDescriptor<any> | number,
   ) {
     ownMetadata = ownMetadata || {};
-    let params = ownMetadata[methodName!];
-    params = [this.spec].concat(params);
+    const index = this.getParameterIndex(target, methodName, methodDescriptor);
+
+    let params =
+      ownMetadata[methodName!] || new Array(index + 1).fill(undefined);
+    params[index] = this.withTarget(<T>this.inherit(params[index]), target);
     ownMetadata[methodName!] = params;
+    // Cache the index to help us position the next parameter
+    Reflector.defineMetadata(
+      this.key + ':index',
+      index - 1,
+      target,
+      methodName,
+    );
     return ownMetadata;
   }
 

diff --git a/packages/metadata/test/unit/decorator-factory.test.ts b/packages/metadata/test/unit/decorator-factory.test.ts
@@ -9,11 +9,11 @@ import {
   PropertyDecoratorFactory,
   MethodDecoratorFactory,
   ParameterDecoratorFactory,
+  MethodParameterDecoratorFactory,
   DecoratorFactory,
+  Reflector,
 } from '../..';
 
-import {Reflector} from '../../src/reflect';
-
 describe('ClassDecoratorFactory', () => {
   /**
    * Define `@classDecorator(spec)`
@@ -576,3 +576,63 @@ describe('ParameterDecoratorFactory for a static method', () => {
     );
   });
 });
+
+describe('MethodParameterDecoratorFactory', () => {
+  /**
+   * Define `@parameterDecorator(spec)`
+   * @param spec
+   */
+  function methodParameterDecorator(spec: object): MethodDecorator {
+    return MethodParameterDecoratorFactory.createDecorator('test', spec);
+  }
+
+  class BaseController {
+    @methodParameterDecorator({x: 1}) // Will be applied to b
+    myMethod(a: string, b: number) {}
+  }
+
+  class SubController extends BaseController {
+    @methodParameterDecorator({x: 2}) // For a
+    @methodParameterDecorator({y: 2}) // For b
+    myMethod(a: string, b: number) {}
+  }
+
+  it('applies metadata to a method parameter', () => {
+    const meta = Reflector.getOwnMetadata('test', BaseController.prototype);
+    expect(meta.myMethod).to.eql([undefined, {x: 1}]);
+  });
+
+  it('merges with base method metadata', () => {
+    const meta = Reflector.getOwnMetadata('test', SubController.prototype);
+    expect(meta.myMethod).to.eql([{x: 2}, {x: 1, y: 2}]);
+  });
+
+  it('does not mutate base method parameter metadata', () => {
+    const meta = Reflector.getOwnMetadata('test', BaseController.prototype);
+    expect(meta.myMethod).to.eql([undefined, {x: 1}]);
+  });
+});
+
+describe('MethodParameterDecoratorFactory with invalid decorations', () => {
+  /**
+   * Define `@parameterDecorator(spec)`
+   * @param spec
+   */
+  function methodParameterDecorator(spec: object): MethodDecorator {
+    return MethodParameterDecoratorFactory.createDecorator('test', spec);
+  }
+
+  it('reports error if the # of decorations exceeeds the # of params', () => {
+    expect(() => {
+      // tslint:disable-next-line:no-unused-variable
+      class MyController {
+        @methodParameterDecorator({x: 1}) // Causing error
+        @methodParameterDecorator({x: 2}) // For a
+        @methodParameterDecorator({x: 3}) // For b
+        myMethod(a: string, b: number) {}
+      }
+    }).to.throw(
+      /The decorator is used more than 2 time\(s\) on method MyController\.prototype\.myMethod/,
+    );
+  });
+});