Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -31,6 +31,7 @@ export function authenticate(strategyName: string, options?: object) {
strategy: strategyName,
options: options || {},
},
{decoratorName: '@authenticate'},
);
}

Expand Down
2 changes: 2 additions & 0 deletions packages/authorization/src/decorators/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,15 @@ export function authorize(spec: AuthorizationMetadata) {
return AuthorizeMethodDecoratorFactory.createDecorator(
AUTHORIZATION_METHOD_KEY,
spec,
{decoratorName: '@authorize'},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raymondfeng why do we need provide the decorator name in the options? it could be inferred from the AUTHORIZATION_METHOD_KEY right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the decorator name @authorize cannot be inferred from AUTHORIZATION_METHOD_KEY, which can be a free-form string.

)(target, method, methodDescriptor!);
}
if (typeof target === 'function' && !method && !methodDescriptor) {
// Class
return AuthorizeClassDecoratorFactory.createDecorator(
AUTHORIZATION_CLASS_KEY,
spec,
{decoratorName: '@authorize'},
)(target);
}
// Not on a class or method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
24 changes: 24 additions & 0 deletions packages/context/src/__tests__/unit/inject.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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')
Expand Down
1 change: 1 addition & 0 deletions packages/context/src/binding-decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export function bind(...specs: BindingSpec[]): ClassDecorator {
const decorator = BindDecoratorFactory.createDecorator(
BINDING_METADATA_KEY,
spec,
{decoratorName: '@bind'},
);
decorator(target);
};
Expand Down
4 changes: 2 additions & 2 deletions packages/context/src/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions packages/context/src/interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,13 +263,15 @@ export function intercept(...interceptorOrKeys: InterceptorOrKey[]) {
return InterceptMethodDecoratorFactory.createDecorator(
INTERCEPT_METHOD_KEY,
interceptorOrKeys,
{decoratorName: '@intercept'},
)(target, method, methodDescriptor!);
}
if (typeof target === 'function' && !method && !methodDescriptor) {
// Class
return InterceptClassDecoratorFactory.createDecorator(
INTERCEPT_CLASS_KEY,
interceptorOrKeys,
{decoratorName: '@intercept'},
)(target);
}
// Not on a class or method
Expand Down
3 changes: 3 additions & 0 deletions packages/metadata/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function myClassDecorator(spec: MyClassMetadata): ClassDecorator {
return ClassDecoratorFactory.createDecorator<MyClassMetadata>(
'metadata-key-for-my-class-decorator',
spec,
{decoratorName: '@myClassDecorator'},
);
}
```
Expand Down Expand Up @@ -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

Expand Down
23 changes: 20 additions & 3 deletions packages/metadata/src/__tests__/unit/decorator-factory.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ describe('ClassDecoratorFactory', () => {
return ClassDecoratorFactory.createDecorator('test', spec);
}

function testDecorator(spec: object): ClassDecorator {
return ClassDecoratorFactory.createDecorator('test', spec, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here, test and @test seems duplicate, and what would happen if they are different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first argument is the key for the metadata, it can be test or test.key, or other unique strings. The second one is for the decorator function name.

decoratorName: '@test',
});
}

const xSpec = {x: 1};
@classDecorator(xSpec)
class BaseController {}
Expand Down Expand Up @@ -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/,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to provide a meaningful default name when no decoratorName metadata was supplied.

);
});

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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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\(\)/,
);
});
});
73 changes: 52 additions & 21 deletions packages/metadata/src/decorator-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -58,6 +64,8 @@ export class DecoratorFactory<
M extends T | MetadataMap<T> | MetadataMap<T[]>, // Type of the metadata
D extends DecoratorType // Type of the decorator
> {
protected decoratorName: string;

/**
* A constant to reference the target of a decoration
*/
Expand All @@ -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);
}
Expand Down Expand Up @@ -219,7 +232,9 @@ export class DecoratorFactory<
member?: string | symbol,
descriptorOrIndex?: TypedPropertyDescriptor<any> | number,
): M {
throw new Error('mergeWithInherited() is not implemented');
throw new Error(
`mergeWithInherited() is not implemented for ${this.decoratorName}`,
);
}

/**
Expand All @@ -241,15 +256,39 @@ export class DecoratorFactory<
member?: string | symbol,
descriptorOrIndex?: TypedPropertyDescriptor<any> | 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<any> | number,
) {
const targetName = DecoratorFactory.getTargetName(
target,
member,
descriptorOrIndex,
);
return new Error(
`${this.decoratorName} cannot be applied more than once on ${targetName}`,
);
}

/**
* Create a decorator function of the given type. Each sub class MUST
* implement this method.
*/
create(): D {
throw new Error('create() is not implemented');
throw new Error(`create() is not implemented for ${this.decoratorName}`);
}

/**
Expand Down Expand Up @@ -368,10 +407,7 @@ export class ClassDecoratorFactory<T> extends DecoratorFactory<
descriptorOrIndex?: TypedPropertyDescriptor<any> | 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);
}
Expand Down Expand Up @@ -426,9 +462,10 @@ export class PropertyDecoratorFactory<T> 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);
Expand Down Expand Up @@ -491,10 +528,7 @@ export class MethodDecoratorFactory<T> 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);
Expand Down Expand Up @@ -584,10 +618,7 @@ export class ParameterDecoratorFactory<T> 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(
Expand Down Expand Up @@ -674,7 +705,7 @@ export class MethodParameterDecoratorFactory<T> 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;
Expand Down
3 changes: 2 additions & 1 deletion packages/openapi-v3/src/decorators/api.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -30,5 +30,6 @@ export function api(spec: ControllerSpec) {
return ClassDecoratorFactory.createDecorator<ControllerSpec>(
OAI3Keys.CLASS_KEY,
spec,
{decoratorName: '@api'},
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,6 @@ export function operation(verb: string, path: string, spec?: OperationObject) {
path,
spec,
},
{decoratorName: '@operation'},
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export function param(paramSpec: ParameterObject) {
ParameterDecoratorFactory.createDecorator<ParameterObject>(
OAI3Keys.PARAMETERS_KEY,
paramSpec,
{decoratorName: '@param'},
)(target, member, index);
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export function requestBody(requestBodySpec?: Partial<RequestBodyObject>) {
ParameterDecoratorFactory.createDecorator<RequestBodyObject>(
OAI3Keys.REQUEST_BODY_KEY,
requestBodySpec as RequestBodyObject,
{decoratorName: '@requestBody'},
)(target, member, index);
};
}
Expand Down
Loading