From 82059aeb01fb6ea04b16e8143178ca1f0eb71fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 29 Aug 2019 12:10:30 +0200 Subject: [PATCH 1/3] feat(metadata): allow custom decorator name for error messages --- packages/metadata/README.md | 3 + .../__tests__/unit/decorator-factory.unit.ts | 23 +++++- packages/metadata/src/decorator-factory.ts | 73 +++++++++++++------ 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/packages/metadata/README.md b/packages/metadata/README.md index f4ed07c63dd8..e1537be5b6dd 100644 --- a/packages/metadata/README.md +++ b/packages/metadata/README.md @@ -28,6 +28,7 @@ function myClassDecorator(spec: MyClassMetadata): ClassDecorator { return ClassDecoratorFactory.createDecorator( 'metadata-key-for-my-class-decorator', spec, + {decoratorName: '@myClassDecorator'}, ); } ``` @@ -209,6 +210,8 @@ functions. There are two flags for the options: Sometimes we use shared spec for the decoration, but the decorator function might need to mutate the object. Cloning the input spec makes it safe to use the same spec (`template`) to decorate different members. Default to `true`. +- decoratorName: Name for the decorator such as `@inject` for error and + debugging messages. ### Customize inheritance of metadata diff --git a/packages/metadata/src/__tests__/unit/decorator-factory.unit.ts b/packages/metadata/src/__tests__/unit/decorator-factory.unit.ts index 03a72ab7a059..d942761399eb 100644 --- a/packages/metadata/src/__tests__/unit/decorator-factory.unit.ts +++ b/packages/metadata/src/__tests__/unit/decorator-factory.unit.ts @@ -80,6 +80,12 @@ describe('ClassDecoratorFactory', () => { return ClassDecoratorFactory.createDecorator('test', spec); } + function testDecorator(spec: object): ClassDecorator { + return ClassDecoratorFactory.createDecorator('test', spec, { + decoratorName: '@test', + }); + } + const xSpec = {x: 1}; @classDecorator(xSpec) class BaseController {} @@ -116,9 +122,18 @@ describe('ClassDecoratorFactory', () => { // eslint-disable-next-line @typescript-eslint/no-unused-vars class MyController {} }).to.throw( - /Decorator cannot be applied more than once on class MyController/, + /ClassDecorator cannot be applied more than once on class MyController/, ); }); + + it('throws with decoratorName if applied more than once on the target', () => { + expect(() => { + @testDecorator({x: 1}) + @testDecorator({y: 2}) + // eslint-disable-next-line @typescript-eslint/no-unused-vars + class MyController {} + }).to.throw(/@test cannot be applied more than once on class MyController/); + }); }); describe('ClassDecoratorFactory for primitive types', () => { @@ -690,7 +705,9 @@ describe('MethodParameterDecoratorFactory with invalid decorations', () => { * @param spec */ function methodParameterDecorator(spec: object): MethodDecorator { - return MethodParameterDecoratorFactory.createDecorator('test', spec); + return MethodParameterDecoratorFactory.createDecorator('test', spec, { + decoratorName: '@param', + }); } it('reports error if the # of decorations exceeeds the # of params', () => { @@ -703,7 +720,7 @@ describe('MethodParameterDecoratorFactory with invalid decorations', () => { myMethod(a: string, b: number) {} } }).to.throw( - /The decorator is used more than 2 time\(s\) on MyController\.prototype\.myMethod\(\)/, + /@param is used more than 2 time\(s\) on MyController\.prototype\.myMethod\(\)/, ); }); }); diff --git a/packages/metadata/src/decorator-factory.ts b/packages/metadata/src/decorator-factory.ts index d64377e22b5f..dc0f9e9d64b5 100644 --- a/packages/metadata/src/decorator-factory.ts +++ b/packages/metadata/src/decorator-factory.ts @@ -28,6 +28,12 @@ export interface DecoratorOptions { * Default to `true`. */ cloneInputSpec?: boolean; + + /** + * Name of the decorator for debugging purpose, such as `@inject` + */ + decoratorName?: string; + [name: string]: any; } @@ -58,6 +64,8 @@ export class DecoratorFactory< M extends T | MetadataMap | MetadataMap, // Type of the metadata D extends DecoratorType // Type of the decorator > { + protected decoratorName: string; + /** * A constant to reference the target of a decoration */ @@ -73,12 +81,17 @@ export class DecoratorFactory< constructor( protected key: string, protected spec: T, - protected options?: DecoratorOptions, + protected options: DecoratorOptions = {}, ) { this.options = Object.assign( - {allowInheritance: true, cloneInputSpec: true}, + { + allowInheritance: true, + cloneInputSpec: true, + }, options, ); + const defaultDecoratorName = this.constructor.name.replace(/Factory$/, ''); + this.decoratorName = this.options.decoratorName || defaultDecoratorName; if (this.options.cloneInputSpec) { this.spec = DecoratorFactory.cloneDeep(spec); } @@ -219,7 +232,9 @@ export class DecoratorFactory< member?: string | symbol, descriptorOrIndex?: TypedPropertyDescriptor | number, ): M { - throw new Error('mergeWithInherited() is not implemented'); + throw new Error( + `mergeWithInherited() is not implemented for ${this.decoratorName}`, + ); } /** @@ -241,7 +256,31 @@ export class DecoratorFactory< member?: string | symbol, descriptorOrIndex?: TypedPropertyDescriptor | number, ): M { - throw new Error('mergeWithOwn() is not implemented'); + throw new Error( + `mergeWithOwn() is not implemented for ${this.decoratorName}`, + ); + } + + /** + * Create an error to report if the decorator is applied to the target more + * than once + * @param target - Decoration target + * @param member - Optional property or method + * @param descriptorOrIndex - Optional parameter index or method descriptor + */ + protected duplicateDecorationError( + target: Object, + member?: string | symbol, + descriptorOrIndex?: TypedPropertyDescriptor | number, + ) { + const targetName = DecoratorFactory.getTargetName( + target, + member, + descriptorOrIndex, + ); + return new Error( + `${this.decoratorName} cannot be applied more than once on ${targetName}`, + ); } /** @@ -249,7 +288,7 @@ export class DecoratorFactory< * implement this method. */ create(): D { - throw new Error('create() is not implemented'); + throw new Error(`create() is not implemented for ${this.decoratorName}`); } /** @@ -368,10 +407,7 @@ export class ClassDecoratorFactory extends DecoratorFactory< descriptorOrIndex?: TypedPropertyDescriptor | number, ) { if (ownMetadata != null) { - throw new Error( - 'Decorator cannot be applied more than once on ' + - DecoratorFactory.getTargetName(target), - ); + throw this.duplicateDecorationError(target, member, descriptorOrIndex); } return this.withTarget(this.spec, target); } @@ -426,9 +462,10 @@ export class PropertyDecoratorFactory extends DecoratorFactory< ) { ownMetadata = ownMetadata || {}; if (ownMetadata[propertyName!] != null) { - const targetName = DecoratorFactory.getTargetName(target, propertyName); - throw new Error( - 'Decorator cannot be applied more than once on ' + targetName, + throw this.duplicateDecorationError( + target, + propertyName, + descriptorOrParameterIndex, ); } ownMetadata[propertyName!] = this.withTarget(this.spec, target); @@ -491,10 +528,7 @@ export class MethodDecoratorFactory extends DecoratorFactory< ownMetadata = ownMetadata || {}; const methodMeta = ownMetadata[methodName!]; if (this.getTarget(methodMeta) === target) { - throw new Error( - 'Decorator cannot be applied more than once on ' + - DecoratorFactory.getTargetName(target, methodName, methodDescriptor), - ); + throw this.duplicateDecorationError(target, methodName, methodDescriptor); } // Set the method metadata ownMetadata[methodName!] = this.withTarget(this.spec, target); @@ -584,10 +618,7 @@ export class ParameterDecoratorFactory extends DecoratorFactory< const methodMeta = this.getOrInitMetadata(ownMetadata, target, methodName); const index = parameterIndex as number; if (this.getTarget(methodMeta[index]) === target) { - throw new Error( - 'Decorator cannot be applied more than once on ' + - DecoratorFactory.getTargetName(target, methodName, parameterIndex), - ); + throw this.duplicateDecorationError(target, methodName, parameterIndex); } // Set the parameter metadata methodMeta[index] = this.withTarget( @@ -674,7 +705,7 @@ export class MethodParameterDecoratorFactory extends DecoratorFactory< methodDescriptor, ); throw new Error( - `The decorator is used more than ${numOfParams} time(s) on ${method}`, + `${this.decoratorName} is used more than ${numOfParams} time(s) on ${method}`, ); } return index; From ed56078ffde652411e588bde02ff36fbff786ab1 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 30 Aug 2019 08:35:44 -0700 Subject: [PATCH 2/3] feat(context): add decorator name for @inject.*, @config.*, @intercept errors --- .../acceptance/binding-config.acceptance.ts | 13 ++++++++++ .../context/src/__tests__/unit/inject.unit.ts | 24 +++++++++++++++++++ packages/context/src/binding-decorator.ts | 1 + packages/context/src/inject.ts | 4 ++-- packages/context/src/interceptor.ts | 2 ++ 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/context/src/__tests__/acceptance/binding-config.acceptance.ts b/packages/context/src/__tests__/acceptance/binding-config.acceptance.ts index 88bbdebfd865..b83da078a016 100644 --- a/packages/context/src/__tests__/acceptance/binding-config.acceptance.ts +++ b/packages/context/src/__tests__/acceptance/binding-config.acceptance.ts @@ -59,6 +59,19 @@ describe('Context bindings - injecting configuration for bound artifacts', () => expect(server1.configObj).to.eql({port: 3000}); }); + it('reports error if @config.* is applied more than once', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + class TestClass { + constructor() {} + + @config('foo') @config('bar') foo: string; + } + }).to.throw( + '@config cannot be applied more than once on TestClass.prototype.foo', + ); + }); + it('allows propertyPath for injection', async () => { class RestServerWithPort { constructor(@config('port') public port: number) {} diff --git a/packages/context/src/__tests__/unit/inject.unit.ts b/packages/context/src/__tests__/unit/inject.unit.ts index 9ce2f8f1f557..59d9272fed3e 100644 --- a/packages/context/src/__tests__/unit/inject.unit.ts +++ b/packages/context/src/__tests__/unit/inject.unit.ts @@ -131,6 +131,17 @@ describe('function argument injection', () => { const meta = describeInjectedArguments(Test); expect(meta.map(m => m.bindingSelector)).to.deepEqual(['controller']); }); + + it('reports error if @inject is applied more than once', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + class TestClass { + constructor(@inject('foo') @inject('bar') foo: string) {} + } + }).to.throw( + '@inject cannot be applied more than once on TestClass.constructor[0]', + ); + }); }); describe('property injection', () => { @@ -182,6 +193,19 @@ describe('property injection', () => { }).to.throw(/@inject cannot be used on a method/); }); + it('reports error if @inject.* is applied more than once', () => { + expect(() => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + class TestClass { + constructor() {} + + @inject.getter('foo') @inject('bar') foo: string; + } + }).to.throw( + '@inject.getter cannot be applied more than once on TestClass.prototype.foo', + ); + }); + it('supports inheritance without overriding property', () => { class TestClass { @inject('foo') diff --git a/packages/context/src/binding-decorator.ts b/packages/context/src/binding-decorator.ts index 3e71f608171a..73b8537a0c37 100644 --- a/packages/context/src/binding-decorator.ts +++ b/packages/context/src/binding-decorator.ts @@ -83,6 +83,7 @@ export function bind(...specs: BindingSpec[]): ClassDecorator { const decorator = BindDecoratorFactory.createDecorator( BINDING_METADATA_KEY, spec, + {decoratorName: '@bind'}, ); decorator(target); }; diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts index d26e8b676103..d6d06a95d3b1 100644 --- a/packages/context/src/inject.ts +++ b/packages/context/src/inject.ts @@ -146,7 +146,7 @@ export function inject( }, // Do not deep clone the spec as only metadata is mutable and it's // shallowly cloned - {cloneInputSpec: false}, + {cloneInputSpec: false, decoratorName: injectionMetadata.decorator}, ); paramDecorator(target, member!, methodDescriptorOrParameterIndex); } else if (member) { @@ -182,7 +182,7 @@ export function inject( }, // Do not deep clone the spec as only metadata is mutable and it's // shallowly cloned - {cloneInputSpec: false}, + {cloneInputSpec: false, decoratorName: injectionMetadata.decorator}, ); propDecorator(target, member!); } else { diff --git a/packages/context/src/interceptor.ts b/packages/context/src/interceptor.ts index 8050bc073664..2b4d033cfd73 100644 --- a/packages/context/src/interceptor.ts +++ b/packages/context/src/interceptor.ts @@ -263,6 +263,7 @@ export function intercept(...interceptorOrKeys: InterceptorOrKey[]) { return InterceptMethodDecoratorFactory.createDecorator( INTERCEPT_METHOD_KEY, interceptorOrKeys, + {decoratorName: '@intercept'}, )(target, method, methodDescriptor!); } if (typeof target === 'function' && !method && !methodDescriptor) { @@ -270,6 +271,7 @@ export function intercept(...interceptorOrKeys: InterceptorOrKey[]) { return InterceptClassDecoratorFactory.createDecorator( INTERCEPT_CLASS_KEY, interceptorOrKeys, + {decoratorName: '@intercept'}, )(target); } // Not on a class or method From 9f61b6e546367fbed0437ef40e47cbf93a23b06b Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 30 Aug 2019 08:47:49 -0700 Subject: [PATCH 3/3] chore: add name for decorators --- .../src/decorators/authenticate.decorator.ts | 3 ++- .../authorization/src/decorators/authorize.ts | 2 ++ .../src/decorators/api.decorator.ts | 3 ++- .../src/decorators/operation.decorator.ts | 1 + .../src/decorators/parameter.decorator.ts | 1 + .../src/decorators/request-body.decorator.ts | 1 + .../src/decorators/model.decorator.ts | 2 ++ .../src/relations/relation.decorator.ts | 20 ++++++++++++++----- 8 files changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/authentication/src/decorators/authenticate.decorator.ts b/packages/authentication/src/decorators/authenticate.decorator.ts index c2f6524b5c9b..b4d4c7c6c5f0 100644 --- a/packages/authentication/src/decorators/authenticate.decorator.ts +++ b/packages/authentication/src/decorators/authenticate.decorator.ts @@ -4,8 +4,8 @@ // License text available at https://opensource.org/licenses/MIT import { - MetadataInspector, Constructor, + MetadataInspector, MethodDecoratorFactory, } from '@loopback/context'; import {AUTHENTICATION_METADATA_KEY} from '../keys'; @@ -31,6 +31,7 @@ export function authenticate(strategyName: string, options?: object) { strategy: strategyName, options: options || {}, }, + {decoratorName: '@authenticate'}, ); } diff --git a/packages/authorization/src/decorators/authorize.ts b/packages/authorization/src/decorators/authorize.ts index 9254ce089f8c..ba3dd971d24d 100644 --- a/packages/authorization/src/decorators/authorize.ts +++ b/packages/authorization/src/decorators/authorize.ts @@ -109,6 +109,7 @@ export function authorize(spec: AuthorizationMetadata) { return AuthorizeMethodDecoratorFactory.createDecorator( AUTHORIZATION_METHOD_KEY, spec, + {decoratorName: '@authorize'}, )(target, method, methodDescriptor!); } if (typeof target === 'function' && !method && !methodDescriptor) { @@ -116,6 +117,7 @@ export function authorize(spec: AuthorizationMetadata) { return AuthorizeClassDecoratorFactory.createDecorator( AUTHORIZATION_CLASS_KEY, spec, + {decoratorName: '@authorize'}, )(target); } // Not on a class or method diff --git a/packages/openapi-v3/src/decorators/api.decorator.ts b/packages/openapi-v3/src/decorators/api.decorator.ts index c9fff0c7f9b5..b4f349fedb9d 100644 --- a/packages/openapi-v3/src/decorators/api.decorator.ts +++ b/packages/openapi-v3/src/decorators/api.decorator.ts @@ -3,8 +3,8 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {ControllerSpec} from '../controller-spec'; import {ClassDecoratorFactory} from '@loopback/context'; +import {ControllerSpec} from '../controller-spec'; import {OAI3Keys} from '../keys'; /** @@ -30,5 +30,6 @@ export function api(spec: ControllerSpec) { return ClassDecoratorFactory.createDecorator( OAI3Keys.CLASS_KEY, spec, + {decoratorName: '@api'}, ); } diff --git a/packages/openapi-v3/src/decorators/operation.decorator.ts b/packages/openapi-v3/src/decorators/operation.decorator.ts index ca62de42a629..b7b0d2f3bf48 100644 --- a/packages/openapi-v3/src/decorators/operation.decorator.ts +++ b/packages/openapi-v3/src/decorators/operation.decorator.ts @@ -84,5 +84,6 @@ export function operation(verb: string, path: string, spec?: OperationObject) { path, spec, }, + {decoratorName: '@operation'}, ); } diff --git a/packages/openapi-v3/src/decorators/parameter.decorator.ts b/packages/openapi-v3/src/decorators/parameter.decorator.ts index bba43d88a378..be7bbfce2db8 100644 --- a/packages/openapi-v3/src/decorators/parameter.decorator.ts +++ b/packages/openapi-v3/src/decorators/parameter.decorator.ts @@ -71,6 +71,7 @@ export function param(paramSpec: ParameterObject) { ParameterDecoratorFactory.createDecorator( OAI3Keys.PARAMETERS_KEY, paramSpec, + {decoratorName: '@param'}, )(target, member, index); }; } diff --git a/packages/openapi-v3/src/decorators/request-body.decorator.ts b/packages/openapi-v3/src/decorators/request-body.decorator.ts index 4bae7cbfdfa5..1b6c033ac90b 100644 --- a/packages/openapi-v3/src/decorators/request-body.decorator.ts +++ b/packages/openapi-v3/src/decorators/request-body.decorator.ts @@ -119,6 +119,7 @@ export function requestBody(requestBodySpec?: Partial) { ParameterDecoratorFactory.createDecorator( OAI3Keys.REQUEST_BODY_KEY, requestBodySpec as RequestBodyObject, + {decoratorName: '@requestBody'}, )(target, member, index); }; } diff --git a/packages/repository/src/decorators/model.decorator.ts b/packages/repository/src/decorators/model.decorator.ts index 9c4bbe82e68a..33dca6aa7423 100644 --- a/packages/repository/src/decorators/model.decorator.ts +++ b/packages/repository/src/decorators/model.decorator.ts @@ -48,6 +48,7 @@ export function model(definition?: Partial) { const decorator = ClassDecoratorFactory.createDecorator( MODEL_KEY, definition, + {decoratorName: '@model'}, ); decorator(target); @@ -111,6 +112,7 @@ export function property(definition?: Partial) { return PropertyDecoratorFactory.createDecorator( MODEL_PROPERTIES_KEY, Object.assign({}, definition), + {decoratorName: '@property'}, ); } diff --git a/packages/repository/src/relations/relation.decorator.ts b/packages/repository/src/relations/relation.decorator.ts index 8031c737c99c..e471d7eeb526 100644 --- a/packages/repository/src/relations/relation.decorator.ts +++ b/packages/repository/src/relations/relation.decorator.ts @@ -17,7 +17,9 @@ export const RELATIONS_KEY = 'loopback:relations'; */ export function relation(definition?: Object) { // Apply relation definition to the model class - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, definition); + return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, definition, { + decoratorName: '@relation', + }); } /** @@ -46,7 +48,9 @@ export function getModelRelations( */ export function embedsOne(definition?: Object) { const rel = Object.assign({type: RelationType.embedsOne}, definition); - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel); + return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel, { + decoratorName: '@embedsOne', + }); } /** @@ -56,7 +60,9 @@ export function embedsOne(definition?: Object) { */ export function embedsMany(definition?: Object) { const rel = Object.assign({type: RelationType.embedsMany}, definition); - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel); + return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel, { + decoratorName: '@embedsMany', + }); } /** @@ -66,7 +72,9 @@ export function embedsMany(definition?: Object) { */ export function referencesOne(definition?: Object) { const rel = Object.assign({type: RelationType.referencesOne}, definition); - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel); + return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel, { + decoratorName: '@referencesOne', + }); } /** @@ -76,5 +84,7 @@ export function referencesOne(definition?: Object) { */ export function referencesMany(definition?: Object) { const rel = Object.assign({type: RelationType.referencesMany}, definition); - return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel); + return PropertyDecoratorFactory.createDecorator(RELATIONS_KEY, rel, { + decoratorName: '@referencesMany', + }); }