From e4d7c01a3842e504869a00e0e3260b487d2483bb Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 7 Sep 2024 00:20:57 +0200 Subject: [PATCH] fix(core): parameter pointcut being mixed with its methodpointcut counterpart --- packages/common/src/abstract/abstract.type.ts | 4 +- .../context/registry/by-ref-selector.ts | 10 +- .../annotation/factory/annotation.factory.ts | 14 +- packages/common/src/public_api.ts | 1 + packages/common/src/reflect/reflect.error.ts | 1 + packages/common/utils/src/utils.ts | 2 +- .../core/src/advice/mutable-advice.context.ts | 2 +- .../after-return/after-return.context.ts | 1 + .../core/src/aspect/aspect-metadata.type.ts | 6 +- packages/core/src/aspect/aspect.annotation.ts | 4 +- packages/core/src/aspect/aspect.registry.ts | 31 +- packages/core/src/aspect/aspect.type.ts | 29 +- packages/core/src/errors/aspect.error.ts | 4 +- packages/core/src/errors/weaving.error.ts | 4 +- .../jit-abstract-method-canvas.strategy.ts | 130 ++++++ .../src/jit/canvas/jit-canvas.strategy.ts | 37 +- .../core/src/jit/canvas/jit-canvas.type.ts | 22 +- .../jit/canvas/jit-class-canvas.strategy.ts | 14 +- .../jit/canvas/jit-method-canvas.strategy.ts | 196 +++------ .../canvas/jit-parameter-canvas.strategy.ts | 12 +- .../canvas/jit-property-canvas.strategy.ts | 28 +- .../src/jit/jit-weaver-decorator.utils.ts | 2 +- packages/core/src/jit/jit-weaver.spec.ts | 2 +- packages/core/src/jit/jit-weaver.ts | 49 ++- packages/core/src/public_api.ts | 3 +- ...n-target.ts => annotation-mixin-target.ts} | 0 ...pec.ts => annotation-mixin.aspect.spec.ts} | 10 +- ...e.aspect.ts => annotation-mixin.aspect.ts} | 4 +- .../src/weaver/canvas/canvas-strategy.type.ts | 31 +- packages/core/src/weaver/weaver.ts | 15 +- .../src/00.guide/010.examples/001.recipes.md | 2 +- .../00.guide/010.examples/010.deprecated.md | 5 +- .../src/annotations/body.annotation.spec.ts | 11 +- .../src/annotations/fetch/path.spec.ts | 18 +- .../annotations/fetch/response-body.spec.ts | 281 ++++++++++++- .../src/annotations/header.annotation.ts | 5 + .../src/annotations/header.spec.ts | 166 ++++++++ .../src/annotations/headers.annotation.ts | 5 + .../src/annotations/path-variable.spec.ts | 31 +- .../src/annotations/type.annotation.ts | 4 +- .../abstract-aop-http-client.aspect.ts | 194 +++++++++ .../src/aspects/httyped-client.aspect.ts | 378 +++++++++--------- .../src/client-factory/client-config.type.ts | 2 +- .../src/client-factory/client.factory.ts | 47 +-- .../default-json-response-handler.ts | 111 +++++ .../src/types/body-metadata.type.ts | 6 + .../httyped-client/src/types/mapper.error.ts | 14 + .../httyped-client/src/types/mapper.type.ts | 81 ++-- .../src/types/response-handler.type.ts | 9 +- .../src/types/type-hint.type.ts | 3 + packages/httyped-client/test/main.ts | 43 +- packages/httyped-client/tsconfig.spec.json | 5 +- .../common/src/nestjs-common-bridge.aspect.ts | 17 +- tsconfig.spec.json | 2 +- 54 files changed, 1458 insertions(+), 650 deletions(-) create mode 100644 packages/common/src/reflect/reflect.error.ts create mode 100644 packages/core/src/jit/canvas/jit-abstract-method-canvas.strategy.ts rename packages/core/src/utils/{bindable-annotation-target.ts => annotation-mixin-target.ts} (100%) rename packages/core/src/utils/{decorator-bridge.aspect.spec.ts => annotation-mixin.aspect.spec.ts} (95%) rename packages/core/src/utils/{decorator-bridge.aspect.ts => annotation-mixin.aspect.ts} (96%) create mode 100644 packages/httyped-client/src/annotations/header.annotation.ts create mode 100644 packages/httyped-client/src/annotations/header.spec.ts create mode 100644 packages/httyped-client/src/annotations/headers.annotation.ts create mode 100644 packages/httyped-client/src/aspects/abstract-aop-http-client.aspect.ts create mode 100644 packages/httyped-client/src/client-factory/default-json-response-handler.ts create mode 100644 packages/httyped-client/src/types/body-metadata.type.ts create mode 100644 packages/httyped-client/src/types/mapper.error.ts create mode 100644 packages/httyped-client/src/types/type-hint.type.ts diff --git a/packages/common/src/abstract/abstract.type.ts b/packages/common/src/abstract/abstract.type.ts index 1f4b57b6..3dd4a3f5 100644 --- a/packages/common/src/abstract/abstract.type.ts +++ b/packages/common/src/abstract/abstract.type.ts @@ -20,7 +20,7 @@ export const _defuseAbstract = (receipe: () => {}) => { } const multipleAbstractCalls = - (_isAbstractToken(val) && + (isAbstractToken(val) && (val as _AbstractTokenImpl).counter - abstractCounter !== 0) || (_abstractThrows && abstractCounter - _abstractCounter !== 1); if (multipleAbstractCalls) { @@ -54,4 +54,4 @@ export const abstract = (template?: T): T => { return new _AbstractTokenImpl(++abstractCounter, template) as any; }; -export const _isAbstractToken = (val: any) => val instanceof _AbstractTokenImpl; +export const isAbstractToken = (val: any) => val instanceof _AbstractTokenImpl; diff --git a/packages/common/src/annotation/context/registry/by-ref-selector.ts b/packages/common/src/annotation/context/registry/by-ref-selector.ts index 756511fc..5352ff19 100644 --- a/packages/common/src/annotation/context/registry/by-ref-selector.ts +++ b/packages/common/src/annotation/context/registry/by-ref-selector.ts @@ -69,10 +69,16 @@ export class AnnotationsByRefSelector< annotation: Annotation, ): AnnotationsSelector; annotations( - ...annotations: (AnnotationRef | Annotation)[] + ...annotations: ( + | AnnotationRef + | Annotation + )[] ): AnnotationsSelector; annotations( - ...annotations: (Annotation | AnnotationRef)[] + ...annotations: ( + | Annotation + | AnnotationRef + )[] ): AnnotationsSelector { let annotationRefs = this.annotationsRefs; if (annotations.length) { diff --git a/packages/common/src/annotation/factory/annotation.factory.ts b/packages/common/src/annotation/factory/annotation.factory.ts index 944d98bb..6e754473 100755 --- a/packages/common/src/annotation/factory/annotation.factory.ts +++ b/packages/common/src/annotation/factory/annotation.factory.ts @@ -12,6 +12,7 @@ import { DecoratorProviderRegistry } from './decorator-provider.registry'; /* eslint-disable @typescript-eslint/no-empty-function */ import { _copyPropsAndMeta, assert } from '@aspectjs/common/utils'; +import { ReflectError } from '../../public_api'; import { reflectContext } from '../../reflect/reflect.context.global'; import type { AnnotationStub } from '../annotation.types'; import { AnnotationRegistry } from '../registry/annotation.registry'; @@ -227,11 +228,14 @@ export class AnnotationFactory { } return decoree; } catch (e) { - console.error( - `Error applying annotation hook ${name}: ${ - (e as Error).message - }`, - ); + if (!(e instanceof ReflectError)) { + console.error( + `Error applying annotation hook ${name}: ${ + (e as Error).message + }`, + ); + } + throw e; } }, diff --git a/packages/common/src/public_api.ts b/packages/common/src/public_api.ts index 3b302e43..deabe0d0 100644 --- a/packages/common/src/public_api.ts +++ b/packages/common/src/public_api.ts @@ -21,3 +21,4 @@ export * from './reflect/module/reflect-module.type'; export * from './reflect/reflect-provider.type'; export * from './reflect/reflect.context'; export * from './reflect/reflect.context.global'; +export * from './reflect/reflect.error'; diff --git a/packages/common/src/reflect/reflect.error.ts b/packages/common/src/reflect/reflect.error.ts new file mode 100644 index 00000000..c39608d9 --- /dev/null +++ b/packages/common/src/reflect/reflect.error.ts @@ -0,0 +1 @@ +export class ReflectError extends Error {} diff --git a/packages/common/utils/src/utils.ts b/packages/common/utils/src/utils.ts index f02ef74f..86def042 100644 --- a/packages/common/utils/src/utils.ts +++ b/packages/common/utils/src/utils.ts @@ -32,7 +32,7 @@ export function isFunction( } export function isObject(value: unknown): value is object { - return typeof value === 'object' && !(value instanceof Array); + return typeof value === 'object' && !Array.isArray(value); } export function isNumber(value: unknown): value is number { diff --git a/packages/core/src/advice/mutable-advice.context.ts b/packages/core/src/advice/mutable-advice.context.ts index bd83c9f6..1c208b6e 100644 --- a/packages/core/src/advice/mutable-advice.context.ts +++ b/packages/core/src/advice/mutable-advice.context.ts @@ -18,7 +18,7 @@ import type { AfterThrowContext } from '../advices/after-throw/after-throw.conte import type { AroundContext } from '../advices/around/around.context'; import type { BeforeContext } from '../advices/before/before.context'; import type { CompileContext } from '../advices/compile/compile.context'; -import { _BindableAnnotationTarget } from '../utils/bindable-annotation-target'; +import { _BindableAnnotationTarget } from '../utils/annotation-mixin-target'; import { AdviceContext } from './advice.context'; export class MutableAdviceContext< diff --git a/packages/core/src/advices/after-return/after-return.context.ts b/packages/core/src/advices/after-return/after-return.context.ts index a961fc1f..81d25f50 100644 --- a/packages/core/src/advices/after-return/after-return.context.ts +++ b/packages/core/src/advices/after-return/after-return.context.ts @@ -19,6 +19,7 @@ export interface AfterReturnContext< readonly annotations: AnnotationsByRefSelector< ToAnnotationType >['annotations']; + /** The 'this' instance bound to the current execution context **/ readonly instance: X; /** the arguments originally passed to the joinpoint **/ diff --git a/packages/core/src/aspect/aspect-metadata.type.ts b/packages/core/src/aspect/aspect-metadata.type.ts index 9effd62e..d3084223 100644 --- a/packages/core/src/aspect/aspect-metadata.type.ts +++ b/packages/core/src/aspect/aspect-metadata.type.ts @@ -1,4 +1,6 @@ +export interface AspectOptions { + readonly id?: string; +} export interface AspectMetadata { - id?: string; - // TODO: add unique?: boolean flag + readonly id: string; } diff --git a/packages/core/src/aspect/aspect.annotation.ts b/packages/core/src/aspect/aspect.annotation.ts index 9c3c4b5d..3da07fc8 100644 --- a/packages/core/src/aspect/aspect.annotation.ts +++ b/packages/core/src/aspect/aspect.annotation.ts @@ -4,7 +4,7 @@ import type { Weaver } from './../weaver/weaver'; import { _CORE_ANNOTATION_FACTORY } from '../utils'; -import type { AspectMetadata } from './aspect-metadata.type'; +import type { AspectOptions } from './aspect-metadata.type'; /* eslint-disable @typescript-eslint/no-unused-vars */ /** @@ -24,5 +24,5 @@ export const Aspect = _CORE_ANNOTATION_FACTORY.create( AnnotationType.CLASS, // eslint-disable-next-line @typescript-eslint/ban-ts-comment,@typescript-eslint/no-unused-vars // @ts-ignore - function Aspect(id: string | AspectMetadata = {}) {}, + function Aspect(id: string | AspectOptions = {}) {}, ); diff --git a/packages/core/src/aspect/aspect.registry.ts b/packages/core/src/aspect/aspect.registry.ts index a86cf9ba..457cadd5 100644 --- a/packages/core/src/aspect/aspect.registry.ts +++ b/packages/core/src/aspect/aspect.registry.ts @@ -12,12 +12,7 @@ import { AdviceRegistry } from '../advice/registry/advice.registry'; import { WeavingError } from '../errors/weaving.error'; import { WeaverContext } from '../weaver/context/weaver.context'; -import { - ASPECT_ID_SYMBOL, - AspectType, - getAspectMetadata, - isAspect, -} from './aspect.type'; +import { AspectType, getAspectMetadata, isAspect } from './aspect.type'; let _globalRegId = 0; export class AspectRegistry { @@ -76,19 +71,19 @@ export class AspectRegistry { } this.aspectsMap.set(id, aspect); - aspect[ASPECT_ID_SYMBOL] = id; - this.adviceReg.register(aspect); } - getAspects( - aspect?: string | ConstructorType, - ): (T & AspectType)[] { - return ( - aspect - ? this.aspectsMap.get(getAspectId(aspect)) ?? [] - : [...this.aspectsMap.values()].flatMap((a) => a) - ) as (T & AspectType)[]; + getAspect( + aspect: string | ConstructorType, + ): (T & AspectType) | undefined { + return this.aspectsMap.get(getAspectId(aspect)) as + | (T & AspectType) + | undefined; + } + + getAspects(): AspectType[] { + return [...this.aspectsMap.values()].flatMap((a) => a); } private __isAspectRegistered(aspect: AspectType) { @@ -100,7 +95,5 @@ export class AspectRegistry { } function getAspectId(aspect: AspectType | string): string { - return typeof aspect === 'string' - ? aspect - : getPrototype(aspect).constructor.name; + return typeof aspect === 'string' ? aspect : getAspectMetadata(aspect).id; } diff --git a/packages/core/src/aspect/aspect.type.ts b/packages/core/src/aspect/aspect.type.ts index 99a11fb2..167521ad 100644 --- a/packages/core/src/aspect/aspect.type.ts +++ b/packages/core/src/aspect/aspect.type.ts @@ -6,20 +6,14 @@ import { AnnotationType, reflectContext, } from '@aspectjs/common'; -import { AspectMetadata } from './aspect-metadata.type'; +import { AspectMetadata, AspectOptions } from './aspect-metadata.type'; import { Aspect } from './aspect.annotation'; -export const ASPECT_ID_SYMBOL = Symbol.for('aspectjs:aspectId'); - -export type AspectType = object & { - [ASPECT_ID_SYMBOL]?: string; -}; +export type AspectType = object & {}; let _globalAspectId = 0; -export function getAspectMetadata( - aspect: AspectType, -): Required { +export function getAspectMetadata(aspect: AspectType): AspectMetadata { const target = reflectContext() .get(AnnotationTargetFactory) .of(aspect); @@ -46,14 +40,15 @@ export function isAspect(aspect: AspectType) { function coerceAspectOptions( aspectTarget: AnnotationTarget, idOrOptions: unknown, -): Required { - const options: AspectMetadata = +): AspectMetadata { + const options: AspectOptions = typeof idOrOptions === 'object' ? { ...idOrOptions } : {}; - options.id = - typeof idOrOptions === 'string' - ? idOrOptions - : options.id ?? - `${aspectTarget.proto.constructor.name}#${_globalAspectId++}`; - return options as Required; + return { + id: + typeof idOrOptions === 'string' + ? idOrOptions + : options.id ?? + `${aspectTarget.proto.constructor.name}#${_globalAspectId++}`, + } satisfies AspectMetadata; } diff --git a/packages/core/src/errors/aspect.error.ts b/packages/core/src/errors/aspect.error.ts index a6f1f0c9..c43fe38f 100644 --- a/packages/core/src/errors/aspect.error.ts +++ b/packages/core/src/errors/aspect.error.ts @@ -1,8 +1,8 @@ -import { AspectType, ASPECT_ID_SYMBOL } from '../aspect/aspect.type'; +import { AspectType, getAspectMetadata } from '../aspect/aspect.type'; import { WeavingError } from './weaving.error'; export class AspectError extends WeavingError { constructor(aspect: AspectType, msg: string) { - super(`[${aspect[ASPECT_ID_SYMBOL]}]: ${msg}`); + super(`[${getAspectMetadata(aspect).id}]: ${msg}`); } } diff --git a/packages/core/src/errors/weaving.error.ts b/packages/core/src/errors/weaving.error.ts index acb4fb86..23cfe7b2 100644 --- a/packages/core/src/errors/weaving.error.ts +++ b/packages/core/src/errors/weaving.error.ts @@ -1,4 +1,6 @@ +import { ReflectError } from '@aspectjs/common'; + /** * Error thrown during the weaving process meaning the weaver has illegal state. */ -export class WeavingError extends Error {} +export class WeavingError extends ReflectError {} diff --git a/packages/core/src/jit/canvas/jit-abstract-method-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-abstract-method-canvas.strategy.ts new file mode 100644 index 00000000..748b0f40 --- /dev/null +++ b/packages/core/src/jit/canvas/jit-abstract-method-canvas.strategy.ts @@ -0,0 +1,130 @@ +import { PointcutType } from '../../pointcut/pointcut-target.type'; +import { JitWeaverCanvasStrategy } from './jit-canvas.strategy'; + +import { + MethodPropertyDescriptor, + _copyPropsAndMeta, + _defuseAbstract, + assert, +} from '@aspectjs/common/utils'; +import { AdviceType } from '../../advice/advice-type.type'; +import { JoinPoint } from '../../advice/joinpoint'; +import { MutableAdviceContext } from '../../advice/mutable-advice.context'; +import { AdviceEntry } from '../../advice/registry/advice-entry.model'; +import { AdviceError } from '../../errors/advice.error'; +import { CompiledSymbol } from '../../weaver/canvas/canvas-strategy.type'; +import { renameFunction } from './canvas.utils'; + +/** + * Canvas to advise method and parameters + */ +export abstract class AbstractJitMethodCanvasStrategy< + T extends PointcutType.METHOD | PointcutType.PARAMETER, + X = unknown, +> extends JitWeaverCanvasStrategy { + protected abstract getAdviceEntries

( + pointcutType: P, + ): AdviceEntry[]; + + compile(ctxt: MutableAdviceContext): CompiledSymbol { + // if method already compiled, it might also be linked. + // Use the last known compiled symbol as a reference to avoid linking twice. + let methodDescriptor = ctxt.target.getMetadata( + '@ajs:compiledSymbol', + () => ctxt.target.descriptor as CompiledSymbol, + )!; + assert(!!methodDescriptor); + + // if no method compile advices, return method as is + const adviceEntries = this.getAdviceEntries(AdviceType.COMPILE); + if (!adviceEntries.length) { + return methodDescriptor; + } + + assert(!!ctxt.target.propertyKey); + // if (!adviceEntries.length) { + // return Reflect.getOwnPropertyDescriptor( + // ctxt.target.proto, + // ctxt.target.propertyKey, + // ) as CompiledSymbol; + // } + + adviceEntries + // prevent calling them twice. + .filter((e) => !ctxt.target.getMetadata(`compiled_${e.id}`, () => false)) + .forEach((entry) => { + assert(typeof entry.advice === 'function'); + Object.defineProperty( + ctxt.target.proto, + ctxt.target.propertyKey, + methodDescriptor, + ); + + let newMethodDescriptor = entry.advice.call( + entry.aspect, + ctxt.asCompileContext(), + ) as CompiledSymbol; + + if (newMethodDescriptor) { + if (typeof newMethodDescriptor === 'function') { + const surrogate = { + fn: newMethodDescriptor, + }; + newMethodDescriptor = Object.getOwnPropertyDescriptor( + surrogate, + 'fn', + )! as CompiledSymbol; + } + if (typeof newMethodDescriptor.value !== 'function') { + throw new AdviceError( + entry.aspect, + entry.advice, + ctxt.target, + 'should return void, a function, or a Method property descriptor', + ); + } + + _copyPropsAndMeta(newMethodDescriptor.value, methodDescriptor.value); // copy static props + methodDescriptor = newMethodDescriptor; + } + + ctxt.target.defineMetadata(`compiled_${entry.id}`, true); + }); + ctxt.target.defineMetadata('@ajs:compiledSymbol', methodDescriptor); + return methodDescriptor; + } + + override callJoinpoint( + ctxt: MutableAdviceContext, + originalSymbol: MethodPropertyDescriptor, + ): unknown { + return (ctxt.value = _defuseAbstract(() => + originalSymbol.value.call(ctxt.instance, ...ctxt.args!), + )); + } + + link( + ctxt: MutableAdviceContext, + compiledSymbol: MethodPropertyDescriptor, + joinpoint: (...args: any[]) => unknown, + ): CompiledSymbol { + compiledSymbol = wrapMethodDescriptor(ctxt, compiledSymbol, joinpoint); + + return compiledSymbol as CompiledSymbol; + } +} + +function wrapMethodDescriptor( + ctxt: MutableAdviceContext, + descriptor: MethodPropertyDescriptor, + joinpoint: JoinPoint, +): CompiledSymbol { + return { + ...descriptor, + value: renameFunction( + joinpoint, + ctxt.target.descriptor.value, + `function ${String(ctxt.target.propertyKey)}$$advised`, + ), + }; +} diff --git a/packages/core/src/jit/canvas/jit-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-canvas.strategy.ts index 4faa97ca..69a2f30d 100644 --- a/packages/core/src/jit/canvas/jit-canvas.strategy.ts +++ b/packages/core/src/jit/canvas/jit-canvas.strategy.ts @@ -24,40 +24,37 @@ export abstract class JitWeaverCanvasStrategy< { constructor( protected readonly weaverContext: WeaverContext, + protected readonly advices: AdvicesSelection, public readonly pointcutTypes: T[], ) {} abstract compile( ctxt: MutableAdviceContext, - selection: AdvicesSelection, ): CompiledSymbol | undefined; - before(ctxt: MutableAdviceContext, selection: AdvicesSelection): void { + before(ctxt: MutableAdviceContext): void { this._applyNotReturn( ctxt, () => ctxt.asBeforeContext(), - selection.find(this.pointcutTypes, [AdviceType.BEFORE]), + this.advices.find(this.pointcutTypes, [AdviceType.BEFORE]), ); } - after(ctxt: MutableAdviceContext, selection: AdvicesSelection): void { + after(ctxt: MutableAdviceContext): void { this._applyNotReturn( ctxt, () => ctxt.asAfterContext(), - selection.find(this.pointcutTypes, [AdviceType.AFTER]), + this.advices.find(this.pointcutTypes, [AdviceType.AFTER]), ); } - afterReturn( - ctxt: MutableAdviceContext, - selection: AdvicesSelection, - ): T { + afterReturn(ctxt: MutableAdviceContext): unknown { const advices = [ - ...selection.find(this.pointcutTypes, [AdviceType.AFTER_RETURN]), + ...this.advices.find(this.pointcutTypes, [AdviceType.AFTER_RETURN]), ]; if (!advices.length) { - return ctxt.value as T; + return ctxt.value; } advices.forEach((advice) => { @@ -69,16 +66,12 @@ export abstract class JitWeaverCanvasStrategy< ]); }); - return ctxt.value as T; + return ctxt.value; } - afterThrow( - ctxt: MutableAdviceContext, - advicesSelection: AdvicesSelection, - allowReturn = true, - ): any { + afterThrow(ctxt: MutableAdviceContext, allowReturn = true): any { const adviceEntries = [ - ...advicesSelection.find(this.pointcutTypes, [AdviceType.AFTER_THROW]), + ...this.advices.find(this.pointcutTypes, [AdviceType.AFTER_THROW]), ]; if (!adviceEntries.length) { @@ -112,13 +105,9 @@ export abstract class JitWeaverCanvasStrategy< return ctxt.value; } - around( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - allowReturn = true, - ): JoinPoint { + around(ctxt: MutableAdviceContext, allowReturn = true): JoinPoint { const advices = [ - ...advicesEntries.find(this.pointcutTypes, [AdviceType.AROUND]), + ...this.advices.find(this.pointcutTypes, [AdviceType.AROUND]), ]; if (!advices.length) { return ctxt.joinpoint!; diff --git a/packages/core/src/jit/canvas/jit-canvas.type.ts b/packages/core/src/jit/canvas/jit-canvas.type.ts index bf64dd79..b071d3ff 100644 --- a/packages/core/src/jit/canvas/jit-canvas.type.ts +++ b/packages/core/src/jit/canvas/jit-canvas.type.ts @@ -1,11 +1,10 @@ -import { _isAbstractToken, assert } from '@aspectjs/common/utils'; +import { assert, isAbstractToken } from '@aspectjs/common/utils'; import { WeavingError } from '../../errors/weaving.error'; import type { PointcutType } from '../../pointcut/pointcut-target.type'; import { MutableAdviceContext } from '../../advice/mutable-advice.context'; -import type { AdvicesSelection } from '../../advice/registry/advices-selection.model'; import type { CompiledSymbol } from '../../weaver/canvas/canvas-strategy.type'; import type { JitWeaverCanvasStrategy } from './jit-canvas.strategy'; @@ -22,10 +21,7 @@ export class JitWeaverCanvas< > { constructor(private readonly strategy: JitWeaverCanvasStrategy) {} - compile>( - ctxt: C, - selection: AdvicesSelection, - ): CompiledCanvas { + compile>(ctxt: C): CompiledCanvas { // if no advices, do not compile. // in fact, this condition makes it impossible to enable aspects lately, we enhance the target anyway // if (selection.find().next().done) { @@ -35,7 +31,7 @@ export class JitWeaverCanvas< // }; // } - const compiledSymbol = this.strategy.compile(ctxt, selection); + const compiledSymbol = this.strategy.compile(ctxt); if (!compiledSymbol) { assert(false); } @@ -58,11 +54,11 @@ export class JitWeaverCanvas< ctxt.args = args; try { - this.strategy.before(ctxt, selection); + this.strategy.before(ctxt); this.strategy.callJoinpoint(ctxt, compiledSymbol); - return this.strategy.afterReturn(ctxt, selection); + return this.strategy.afterReturn(ctxt); } catch (error) { // consider WeavingErrors as not recoverable by an aspect if (error instanceof WeavingError) { @@ -70,15 +66,15 @@ export class JitWeaverCanvas< } ctxt.error = error; - return this.strategy.afterThrow(ctxt, selection); + return this.strategy.afterThrow(ctxt); } finally { - this.strategy.after(ctxt, selection); + this.strategy.after(ctxt); } }; - const returnValue = this.strategy.around(ctxt, selection)(...args); + const returnValue = this.strategy.around(ctxt)(...args); - if (_isAbstractToken(returnValue)) { + if (isAbstractToken(returnValue)) { throw new WeavingError( `${ctxt.target} returned "abstract()" token. "abstract()" is meant to be superseded by a @AfterReturn advice or an @Around advice.`, ); diff --git a/packages/core/src/jit/canvas/jit-class-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-class-canvas.strategy.ts index 2a20363a..1c962c02 100644 --- a/packages/core/src/jit/canvas/jit-class-canvas.strategy.ts +++ b/packages/core/src/jit/canvas/jit-class-canvas.strategy.ts @@ -22,13 +22,12 @@ import { renameFunction } from './canvas.utils'; export class JitClassCanvasStrategy< X = unknown, > extends JitWeaverCanvasStrategy { - constructor(weaverContext: WeaverContext) { - super(weaverContext, [PointcutType.CLASS]); + constructor(weaverContext: WeaverContext, advices: AdvicesSelection) { + super(weaverContext, advices, [PointcutType.CLASS]); } compile( ctxt: MutableAdviceContext, - selection: AdvicesSelection, ): ConstructorType { // if class already compiled, it might also be linked. // Use the last known compiled symbol as a reference to avoid linking twice. @@ -38,7 +37,7 @@ export class JitClassCanvasStrategy< ) as ConstructorType; const adviceEntries = [ - ...selection.find([PointcutType.CLASS], [AdviceType.COMPILE]), + ...this.advices.find([PointcutType.CLASS], [AdviceType.COMPILE]), ]; // if no class compile advices, return ctor as is if (!adviceEntries.length) { @@ -79,11 +78,8 @@ export class JitClassCanvasStrategy< return constructor; } - override before( - ctxt: MutableAdviceContext, - selection: AdvicesSelection, - ): void { - super.before(withNullInstance(ctxt), selection); + override before(ctxt: MutableAdviceContext): void { + super.before(withNullInstance(ctxt)); } override callJoinpoint( diff --git a/packages/core/src/jit/canvas/jit-method-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-method-canvas.strategy.ts index ec97b35f..068f2b56 100644 --- a/packages/core/src/jit/canvas/jit-method-canvas.strategy.ts +++ b/packages/core/src/jit/canvas/jit-method-canvas.strategy.ts @@ -1,156 +1,95 @@ import { PointcutType } from '../../pointcut/pointcut-target.type'; -import { JitWeaverCanvasStrategy } from './jit-canvas.strategy'; -import { - MethodPropertyDescriptor, - _copyPropsAndMeta, - _defuseAbstract, - assert, -} from '@aspectjs/common/utils'; +import { MethodPropertyDescriptor } from '@aspectjs/common/utils'; import { AdviceType } from '../../advice/advice-type.type'; import { JoinPoint } from '../../advice/joinpoint'; import { MutableAdviceContext } from '../../advice/mutable-advice.context'; import { AdviceEntry } from '../../advice/registry/advice-entry.model'; import { AdvicesSelection } from '../../advice/registry/advices-selection.model'; -import { AdviceError } from '../../errors/advice.error'; -import { CompiledSymbol } from '../../weaver/canvas/canvas-strategy.type'; import type { WeaverContext } from '../../weaver/context/weaver.context'; -import { renameFunction } from './canvas.utils'; +import { AbstractJitMethodCanvasStrategy } from './jit-abstract-method-canvas.strategy'; +import { JitParameterCanvasStrategy } from './jit-parameter-canvas.strategy'; -/** - * Canvas to advise method and parameters - */ -export abstract class AbstractJitMethodCanvasStrategy< - T extends PointcutType.METHOD | PointcutType.PARAMETER, +export class JitMethodCanvasStrategy< X = unknown, -> extends JitWeaverCanvasStrategy { - protected abstract getAdviceEntries

( - selection: AdvicesSelection, - pointcutType: P, - ): AdviceEntry[]; - - compile( - ctxt: MutableAdviceContext, - selection: AdvicesSelection, - ): CompiledSymbol { - // if method already compiled, it might also be linked. - // Use the last known compiled symbol as a reference to avoid linking twice. - let methodDescriptor = ctxt.target.getMetadata( - '@ajs:compiledSymbol', - () => ctxt.target.descriptor as CompiledSymbol, - )!; - assert(!!methodDescriptor); - - // if no method compile advices, return method as is - const adviceEntries = this.getAdviceEntries(selection, AdviceType.COMPILE); - if (!adviceEntries.length) { - return methodDescriptor; - } - - assert(!!ctxt.target.propertyKey); - // if (!adviceEntries.length) { - // return Reflect.getOwnPropertyDescriptor( - // ctxt.target.proto, - // ctxt.target.propertyKey, - // ) as CompiledSymbol; - // } - - adviceEntries - // prevent calling them twice. - .filter((e) => !ctxt.target.getMetadata(`compiled_${e.id}`, () => false)) - .forEach((entry) => { - assert(typeof entry.advice === 'function'); - Object.defineProperty( - ctxt.target.proto, - ctxt.target.propertyKey, - methodDescriptor, - ); - - let newMethodDescriptor = entry.advice.call( - entry.aspect, - ctxt.asCompileContext(), - ) as CompiledSymbol; +> extends AbstractJitMethodCanvasStrategy { + // appart from @Compile advices, calling a method should search for method advices & parameter advices as well. + // as @Compile advice for method overrides compiled parameter's canvas, remember to delecase to parameter canvas here. + private readonly parameterCanvasStrategy?: JitParameterCanvasStrategy; + constructor( + weaverContext: WeaverContext, + advices: AdvicesSelection, + parameterAdvices?: AdvicesSelection, + ) { + super(weaverContext, advices, [PointcutType.METHOD]); + this.parameterCanvasStrategy = parameterAdvices + ? new JitParameterCanvasStrategy(weaverContext, parameterAdvices) + : undefined; + } - if (newMethodDescriptor) { - if (typeof newMethodDescriptor === 'function') { - const surrogate = { - fn: newMethodDescriptor, - }; - newMethodDescriptor = Object.getOwnPropertyDescriptor( - surrogate, - 'fn', - )! as CompiledSymbol; - } - if (typeof newMethodDescriptor.value !== 'function') { - throw new AdviceError( - entry.aspect, - entry.advice, - ctxt.target, - 'should return void, a function, or a Method property descriptor', - ); - } + override before( + ctxt: MutableAdviceContext, + ): void { + this.parameterCanvasStrategy?.before(ctxt); + super.before(ctxt); + } - _copyPropsAndMeta(newMethodDescriptor.value, methodDescriptor.value); // copy static props - methodDescriptor = newMethodDescriptor; - } + override around( + ctxt: MutableAdviceContext, - ctxt.target.defineMetadata(`compiled_${entry.id}`, true); - }); - ctxt.target.defineMetadata('@ajs:compiledSymbol', methodDescriptor); - return methodDescriptor; + allowReturn?: boolean, + ): JoinPoint { + ctxt.joinpoint = this.parameterCanvasStrategy + ? this.parameterCanvasStrategy.around(ctxt, allowReturn) + : ctxt.joinpoint; + return super.around(ctxt, allowReturn); } - override callJoinpoint( - ctxt: MutableAdviceContext, - originalSymbol: MethodPropertyDescriptor, + override afterReturn( + ctxt: MutableAdviceContext, ): unknown { - return (ctxt.value = _defuseAbstract(() => - originalSymbol.value.call(ctxt.instance, ...ctxt.args!), - )); + ctxt.value = this.parameterCanvasStrategy + ? this.parameterCanvasStrategy.afterReturn(ctxt) + : ctxt.value; + return super.afterReturn(ctxt); } - link( - ctxt: MutableAdviceContext, - compiledSymbol: MethodPropertyDescriptor, - joinpoint: (...args: any[]) => unknown, - ): CompiledSymbol { - compiledSymbol = wrapMethodDescriptor(ctxt, compiledSymbol, joinpoint); + override afterThrow( + ctxt: MutableAdviceContext, + ): unknown { + try { + if (this.parameterCanvasStrategy) { + ctxt.value = this.parameterCanvasStrategy.afterThrow(ctxt); + return ctxt.value; + } + } catch (newError) { + ctxt.error = newError; + } - return compiledSymbol as CompiledSymbol; + return super.afterThrow(ctxt); } -} -export class JitMethodCanvasStrategy< - X = unknown, -> extends AbstractJitMethodCanvasStrategy< - PointcutType.METHOD | PointcutType.PARAMETER, - X -> { - constructor(weaverContext: WeaverContext) { - super(weaverContext, [PointcutType.METHOD, PointcutType.PARAMETER]); + override after( + ctxt: MutableAdviceContext, + ): void { + this.parameterCanvasStrategy?.after(ctxt); + return super.after(ctxt); } protected override getAdviceEntries

( - selection: AdvicesSelection, pointcutType: P, - ): AdviceEntry[] { - return [ - ...selection.find( - [PointcutType.METHOD, PointcutType.PARAMETER], - [pointcutType], - ), - ]; + ): AdviceEntry[] { + return [...this.advices.find([PointcutType.METHOD], [pointcutType])]; } override compile( - ctxt: MutableAdviceContext, - selection: AdvicesSelection, + ctxt: MutableAdviceContext, ): MethodPropertyDescriptor { - return super.compile(ctxt, selection) as MethodPropertyDescriptor; + return super.compile(ctxt) as MethodPropertyDescriptor; } override link( - ctxt: MutableAdviceContext, + ctxt: MutableAdviceContext, compiledSymbol: MethodPropertyDescriptor, joinpoint: (...args: any[]) => unknown, ): MethodPropertyDescriptor { @@ -161,18 +100,3 @@ export class JitMethodCanvasStrategy< ) as MethodPropertyDescriptor; } } - -function wrapMethodDescriptor( - ctxt: MutableAdviceContext, - descriptor: MethodPropertyDescriptor, - joinpoint: JoinPoint, -): CompiledSymbol { - return { - ...descriptor, - value: renameFunction( - joinpoint, - ctxt.target.descriptor.value, - `function ${String(ctxt.target.propertyKey)}$$advised`, - ), - }; -} diff --git a/packages/core/src/jit/canvas/jit-parameter-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-parameter-canvas.strategy.ts index 2d42438d..7cb40a99 100644 --- a/packages/core/src/jit/canvas/jit-parameter-canvas.strategy.ts +++ b/packages/core/src/jit/canvas/jit-parameter-canvas.strategy.ts @@ -6,7 +6,7 @@ import { MutableAdviceContext } from '../../advice/mutable-advice.context'; import { AdviceEntry } from '../../advice/registry/advice-entry.model'; import { AdvicesSelection } from '../../advice/registry/advices-selection.model'; import type { WeaverContext } from '../../weaver/context/weaver.context'; -import { AbstractJitMethodCanvasStrategy } from './jit-method-canvas.strategy'; +import { AbstractJitMethodCanvasStrategy } from './jit-abstract-method-canvas.strategy'; const _defineProperty = Object.defineProperty; @@ -16,24 +16,22 @@ const _defineProperty = Object.defineProperty; export class JitParameterCanvasStrategy< X = unknown, > extends AbstractJitMethodCanvasStrategy { - constructor(weaverContext: WeaverContext) { - super(weaverContext, [PointcutType.PARAMETER]); + constructor(weaverContext: WeaverContext, advices: AdvicesSelection) { + super(weaverContext, advices, [PointcutType.PARAMETER]); } override compile( ctxt: MutableAdviceContext, - selection: AdvicesSelection, ): MethodPropertyDescriptor { - const compiledDescriptor = super.compile(ctxt, selection); + const compiledDescriptor = super.compile(ctxt); return compiledDescriptor; } protected override getAdviceEntries

( - selection: AdvicesSelection, pointcutType: P, ): AdviceEntry[] { - return [...selection.find([PointcutType.PARAMETER], [pointcutType])]; + return [...this.advices.find([PointcutType.PARAMETER], [pointcutType])]; } override link( diff --git a/packages/core/src/jit/canvas/jit-property-canvas.strategy.ts b/packages/core/src/jit/canvas/jit-property-canvas.strategy.ts index 58c5d1c8..09fd9807 100644 --- a/packages/core/src/jit/canvas/jit-property-canvas.strategy.ts +++ b/packages/core/src/jit/canvas/jit-property-canvas.strategy.ts @@ -27,8 +27,8 @@ export class JitPropertyCanvasStrategy< X >; - constructor(weaverContext: WeaverContext) { - super(weaverContext, [PointcutType.GET_PROPERTY]); + constructor(weaverContext: WeaverContext, advices: AdvicesSelection) { + super(weaverContext, advices, [PointcutType.GET_PROPERTY]); this.propertySetterStrategy = this.createPropertySetterStrategy(); } @@ -37,11 +37,10 @@ export class JitPropertyCanvasStrategy< PointcutType.GET_PROPERTY | PointcutType.SET_PROPERTY, X >, - selection: AdvicesSelection, ): PropertyDescriptor | undefined { this.propertySetterCanvas = new JitWeaverCanvas( this.propertySetterStrategy, - ).compile(new MutableAdviceContext(ctxt), selection); + ).compile(new MutableAdviceContext(ctxt)); return this.propertySetterCanvas.compiledSymbol!; } @@ -99,7 +98,7 @@ export class JitPropertyCanvasStrategy< } private createPropertySetterStrategy() { - return new JitPropertySetCanvasStrategy(this.weaverContext); + return new JitPropertySetCanvasStrategy(this.weaverContext, this.advices); } } @@ -107,8 +106,8 @@ class JitPropertySetCanvasStrategy extends JitWeaverCanvasStrategy< PointcutType.GET_PROPERTY | PointcutType.SET_PROPERTY, X > { - constructor(weaverContext: WeaverContext) { - super(weaverContext, [PointcutType.SET_PROPERTY]); + constructor(weaverContext: WeaverContext, advices: AdvicesSelection) { + super(weaverContext, advices, [PointcutType.SET_PROPERTY]); } override callJoinpoint( @@ -150,7 +149,6 @@ class JitPropertySetCanvasStrategy extends JitWeaverCanvasStrategy< PointcutType.GET_PROPERTY | PointcutType.SET_PROPERTY, X >, - selection: AdvicesSelection, ): CompiledSymbol { // if property already compiled, it might also be linked. // Use the last known compiled symbol as a reference to avoid linking twice. @@ -164,8 +162,8 @@ class JitPropertySetCanvasStrategy extends JitWeaverCanvasStrategy< const target = ctxt.target; const adviceEntries = [ - ...selection.find([PointcutType.GET_PROPERTY], [AdviceType.COMPILE]), - ...selection.find([PointcutType.SET_PROPERTY], [AdviceType.COMPILE]), + ...this.advices.find([PointcutType.GET_PROPERTY], [AdviceType.COMPILE]), + ...this.advices.find([PointcutType.SET_PROPERTY], [AdviceType.COMPILE]), ]; adviceEntries @@ -213,15 +211,11 @@ class JitPropertySetCanvasStrategy extends JitWeaverCanvasStrategy< override afterThrow( ctxt: MutableAdviceContext, - advicesSelection: AdvicesSelection, ) { - return super.afterThrow(ctxt, advicesSelection, false); + return super.afterThrow(ctxt, false); } - override around( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ) { - return super.around(ctxt, advicesEntries, false); + override around(ctxt: MutableAdviceContext) { + return super.around(ctxt, false); } private normalizeDescriptor(descriptor: PropertyDescriptor): any { diff --git a/packages/core/src/jit/jit-weaver-decorator.utils.ts b/packages/core/src/jit/jit-weaver-decorator.utils.ts index e7361e93..8cb1038d 100644 --- a/packages/core/src/jit/jit-weaver-decorator.utils.ts +++ b/packages/core/src/jit/jit-weaver-decorator.utils.ts @@ -1,6 +1,6 @@ import { AnnotationTargetFactory } from '@aspectjs/common'; import { assert } from '@aspectjs/common/utils'; -import { _BindableAnnotationTarget } from '../utils/bindable-annotation-target'; +import { _BindableAnnotationTarget } from '../utils/annotation-mixin-target'; import { JitWeaver } from './jit-weaver'; export const createJitWeaverDecorator = ( diff --git a/packages/core/src/jit/jit-weaver.spec.ts b/packages/core/src/jit/jit-weaver.spec.ts index 57fe164d..94eabdcd 100644 --- a/packages/core/src/jit/jit-weaver.spec.ts +++ b/packages/core/src/jit/jit-weaver.spec.ts @@ -12,7 +12,7 @@ import { Aspect } from '../aspect/aspect.annotation'; import { WeavingError } from '../errors/weaving.error'; import { on } from '../pointcut/pointcut-expression.factory'; import { Before, Compile } from '../public_api'; -import { _BindableAnnotationTarget } from '../utils/bindable-annotation-target'; +import { _BindableAnnotationTarget } from '../utils/annotation-mixin-target'; import { WeaverModule } from '../weaver/weaver.module'; import { JitWeaver } from './jit-weaver'; diff --git a/packages/core/src/jit/jit-weaver.ts b/packages/core/src/jit/jit-weaver.ts index ec2fa7c3..d6040c99 100644 --- a/packages/core/src/jit/jit-weaver.ts +++ b/packages/core/src/jit/jit-weaver.ts @@ -64,10 +64,14 @@ export class JitWeaver implements Weaver { return this; } - getAspects( - aspect?: string | ConstructorType, - ): (T & AspectType)[] { - return this.aspectRegistry.getAspects(aspect); + getAspect( + aspect: string | ConstructorType, + ): (T & AspectType) | undefined { + return this.aspectRegistry.getAspect(aspect); + } + + getAspects(): AspectType[] { + return this.aspectRegistry.getAspects(); } enhance( @@ -113,9 +117,9 @@ export class JitWeaver implements Weaver { }); return new JitWeaverCanvas( - new JitClassCanvasStrategy(this.weaverContext), + new JitClassCanvasStrategy(this.weaverContext, advicesSelection), ) - .compile(ctxt, advicesSelection) + .compile(ctxt) .link(); } @@ -148,8 +152,8 @@ export class JitWeaver implements Weaver { return new JitWeaverCanvas< PointcutType.GET_PROPERTY | PointcutType.SET_PROPERTY, X - >(new JitPropertyCanvasStrategy(this.weaverContext)) - .compile(ctxt, advicesSelection) + >(new JitPropertyCanvasStrategy(this.weaverContext, advicesSelection)) + .compile(ctxt) .link(); } @@ -161,7 +165,7 @@ export class JitWeaver implements Weaver { .select() .on({ target, - types: [AnnotationType.METHOD, AnnotationType.PARAMETER], + types: [AnnotationType.METHOD], }) .find(); if (!annotationsForType.length) { @@ -175,10 +179,29 @@ export class JitWeaver implements Weaver { annotations: annotationsForType.map((a) => a.ref), }); + const annotationsForParameters = this.annotationContextRegistry + .select() + .on({ + target, + types: [AnnotationType.PARAMETER], + }) + .find(); + + // find all method advices for enabled aspects + const parameterAdvicesSelection = annotationsForParameters.length + ? this.adviceRegistry.select({ + annotations: annotationsForParameters.map((a) => a.ref), + }) + : undefined; + return new JitWeaverCanvas( - new JitMethodCanvasStrategy(this.weaverContext), + new JitMethodCanvasStrategy( + this.weaverContext, + advicesSelection, + parameterAdvicesSelection, + ), ) - .compile(ctxt, advicesSelection) + .compile(ctxt) .link(); } @@ -206,9 +229,9 @@ export class JitWeaver implements Weaver { }); return new JitWeaverCanvas( - new JitParameterCanvasStrategy(this.weaverContext), + new JitParameterCanvasStrategy(this.weaverContext, advicesSelection), ) - .compile(ctxt, advicesSelection) + .compile(ctxt) .link(); } diff --git a/packages/core/src/public_api.ts b/packages/core/src/public_api.ts index 9c97fa8e..d83b4288 100644 --- a/packages/core/src/public_api.ts +++ b/packages/core/src/public_api.ts @@ -21,6 +21,7 @@ export * from './advices/compile/compile.annotation'; export * from './annotations/order.annotation'; export * from './aspect/aspect-metadata.type'; export * from './aspect/aspect.annotation'; +export * from './aspect/aspect.type'; export * from './errors/advice.error'; export * from './errors/aspect.error'; export * from './errors/weaving.error'; @@ -28,7 +29,7 @@ export * from './pointcut/pointcut'; export * from './pointcut/pointcut-expression.factory'; export * from './pointcut/pointcut-expression.type'; export * from './pointcut/pointcut-target.type'; -export * from './utils/decorator-bridge.aspect'; +export * from './utils/annotation-mixin.aspect'; export * from './weaver/context/weaver.context.global'; export * from './weaver/weaver'; export * from './weaver/weaver.module'; diff --git a/packages/core/src/utils/bindable-annotation-target.ts b/packages/core/src/utils/annotation-mixin-target.ts similarity index 100% rename from packages/core/src/utils/bindable-annotation-target.ts rename to packages/core/src/utils/annotation-mixin-target.ts diff --git a/packages/core/src/utils/decorator-bridge.aspect.spec.ts b/packages/core/src/utils/annotation-mixin.aspect.spec.ts similarity index 95% rename from packages/core/src/utils/decorator-bridge.aspect.spec.ts rename to packages/core/src/utils/annotation-mixin.aspect.spec.ts index 3e5738ca..22071619 100644 --- a/packages/core/src/utils/decorator-bridge.aspect.spec.ts +++ b/packages/core/src/utils/annotation-mixin.aspect.spec.ts @@ -1,12 +1,12 @@ import { AnnotationFactory, AnnotationType } from '@aspectjs/common'; import { configureTesting } from '@aspectjs/common/testing'; import { getWeaver, WeaverModule } from '../public_api'; -import { DecoratorBridgeAspect } from './decorator-bridge.aspect'; +import { AnnotationMixinAspect } from './annotation-mixin.aspect'; -describe('DecoratorBridgeAspect', () => { +describe(`${AnnotationMixinAspect.name}`, () => { describe('configured to bridge decorator XDecorator() to annotation X()', () => { const af = new AnnotationFactory('test'); - let decoratorBridgeAspect: DecoratorBridgeAspect; + let annotationMixinAspect: AnnotationMixinAspect; const XClass = af.create( AnnotationType.CLASS, @@ -76,7 +76,7 @@ describe('DecoratorBridgeAspect', () => { } beforeEach(() => { - decoratorBridgeAspect = new DecoratorBridgeAspect() + annotationMixinAspect = new AnnotationMixinAspect() .bridge(XClass, (a: string, b: string) => XClassDecorator( a.toLocaleUpperCase() as Uppercase, @@ -112,7 +112,7 @@ describe('DecoratorBridgeAspect', () => { configureTesting(WeaverModule); - getWeaver().enable(decoratorBridgeAspect); + getWeaver().enable(annotationMixinAspect); }); describe('using the X annotation on a class', () => { diff --git a/packages/core/src/utils/decorator-bridge.aspect.ts b/packages/core/src/utils/annotation-mixin.aspect.ts similarity index 96% rename from packages/core/src/utils/decorator-bridge.aspect.ts rename to packages/core/src/utils/annotation-mixin.aspect.ts index da34df07..73011036 100644 --- a/packages/core/src/utils/decorator-bridge.aspect.ts +++ b/packages/core/src/utils/annotation-mixin.aspect.ts @@ -5,8 +5,8 @@ import { Compile } from '../advices/compile/compile.annotation'; import { Aspect } from '../aspect/aspect.annotation'; import { on } from '../pointcut/pointcut-expression.factory'; -@Aspect() // no id, so multiple DecoratorBridgeAspect will not replace each other -export class DecoratorBridgeAspect { +@Aspect() // no id, so multiple AnnotationMixinAspect will not replace each other +export class AnnotationMixinAspect { private bridgeCounter = 0; constructor() {} diff --git a/packages/core/src/weaver/canvas/canvas-strategy.type.ts b/packages/core/src/weaver/canvas/canvas-strategy.type.ts index e5115a2e..e09c57c5 100644 --- a/packages/core/src/weaver/canvas/canvas-strategy.type.ts +++ b/packages/core/src/weaver/canvas/canvas-strategy.type.ts @@ -3,7 +3,6 @@ import type { MethodPropertyDescriptor, } from '@aspectjs/common/utils'; import { MutableAdviceContext } from '../../advice/mutable-advice.context'; -import { AdvicesSelection } from '../../advice/registry/advices-selection.model'; import type { JoinPoint } from '../../advice/joinpoint'; import type { PointcutType } from '../../pointcut/pointcut-target.type'; @@ -23,40 +22,22 @@ export interface WeaverCanvasStrategy< T extends PointcutType = PointcutType, X = unknown, > { - compile( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ): CompiledSymbol | undefined; + compile(ctxt: MutableAdviceContext): CompiledSymbol | undefined; - before( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ): void; + before(ctxt: MutableAdviceContext): void; callJoinpoint( ctxt: MutableAdviceContext, originalSymbol: CompiledSymbol, ): unknown; - afterReturn( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ): T; + afterReturn(ctxt: MutableAdviceContext): unknown; - afterThrow( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ): T; + afterThrow(ctxt: MutableAdviceContext): unknown; - after( - ctxt: MutableAdviceContext, - afteradvicesEntries: AdvicesSelection, - ): void; + after(ctxt: MutableAdviceContext): void; - around( - ctxt: MutableAdviceContext, - advicesEntries: AdvicesSelection, - ): JoinPoint; + around(ctxt: MutableAdviceContext): JoinPoint; link?( ctxt: MutableAdviceContext, diff --git a/packages/core/src/weaver/weaver.ts b/packages/core/src/weaver/weaver.ts index 0962c51a..9bbb4568 100644 --- a/packages/core/src/weaver/weaver.ts +++ b/packages/core/src/weaver/weaver.ts @@ -1,3 +1,4 @@ +import { ConstructorType } from '@aspectjs/common/utils'; import type { AspectType } from '../aspect/aspect.type'; /** @@ -12,9 +13,17 @@ export interface Weaver { enable(...aspects: AspectType[]): this; /** - * Find aspects among enabled aspects given an aspect id or constructor. + * Find an aspect among enabled aspects given an aspect id or constructor. * @param aspect - The aspect id or constructor to find. - * @returns The aspects maching the given id, or an empty array if no aspects found. Returns all aspects if no parameter given. + * @returns The aspects maching the given id, or undefined */ - getAspects(aspect?: string | (new () => T)): T[]; + getAspect( + aspect: string | ConstructorType, + ): (T & AspectType) | undefined; + + /** + * Find all aspects among enabled aspects given. + * @returns all aspects registered + */ + getAspects(): AspectType[]; } diff --git a/packages/docs/src/00.guide/010.examples/001.recipes.md b/packages/docs/src/00.guide/010.examples/001.recipes.md index 207e5e8f..e7dd1c67 100644 --- a/packages/docs/src/00.guide/010.examples/001.recipes.md +++ b/packages/docs/src/00.guide/010.examples/001.recipes.md @@ -215,7 +215,7 @@ export class DeprecatedAspect { @Before(on.any.withAnnotations(Deprecated)) logDeprecated(context: BeforeContext) { // find the @Deprecated annotation on the current target - const deprecated = context.annotations.filter(Deprecated).find()[0]; + const deprecated = context.annotations(Deprecated).find()[0]; // get the arguments of the annotation const options: string | undefined = deprecated.args[0]; diff --git a/packages/docs/src/00.guide/010.examples/010.deprecated.md b/packages/docs/src/00.guide/010.examples/010.deprecated.md index 498a40cd..e885e479 100644 --- a/packages/docs/src/00.guide/010.examples/010.deprecated.md +++ b/packages/docs/src/00.guide/010.examples/010.deprecated.md @@ -34,7 +34,7 @@ import { AnnotationTarget } from '@aspectjs/common'; import { Aspect, Before, BeforeContext, on } from '@aspectjs/core'; import { Deprecated } from './deprecated.annotation'; -@Aspect() +@Aspect('deprecated') export class DeprecatedAspect { // log deprecated message only one per target private readonly loggedTargets = new Set(); @@ -46,6 +46,7 @@ export class DeprecatedAspect { private version?: number | string, ) {} + // or: @Before( on.any.withAnnotations(Deprecated)) @Before( on.methods.withAnnotations(Deprecated), on.classes.withAnnotations(Deprecated), @@ -55,7 +56,7 @@ export class DeprecatedAspect { ) logDeprecated(context: BeforeContext) { // get the @Deprecated annotation - const deprecated = context.annotations.filter(Deprecated).find()[0]; + const deprecated = context.annotations(Deprecated).find()[0]; // get the arguments of the annotation const options = deprecated.args[0]; diff --git a/packages/httyped-client/src/annotations/body.annotation.spec.ts b/packages/httyped-client/src/annotations/body.annotation.spec.ts index d908238b..3abc6807 100644 --- a/packages/httyped-client/src/annotations/body.annotation.spec.ts +++ b/packages/httyped-client/src/annotations/body.annotation.spec.ts @@ -49,7 +49,7 @@ describe('@Body() on a method parameter', () => { } api = httypedClientFactory.create(HttpClientApi); - httypedClientAspect = getWeaver().getAspects(HttypedClientAspect)[0]!; + httypedClientAspect = getWeaver().getAspect(HttypedClientAspect)!; }); describe('when the method is not annotated with a fetch annotation', () => { @@ -83,9 +83,7 @@ describe('@Body() on a method parameter', () => { describe('that does not accept the given body', () => { beforeEach(() => { requestBodyMappers.add({ - accepts(obj, mapperContext) { - return typeof obj === 'boolean'; - }, + typeHint: 'Void', map(body, mapperContext) { return false; }, @@ -105,9 +103,7 @@ describe('@Body() on a method parameter', () => { beforeEach(() => { requestBodyMappers.add({ - accepts(obj, mapperContext) { - return typeof obj === 'string'; - }, + typeHint: String, map(body: string, mapperContext) { map(body, mapperContext); return body.toUpperCase(); @@ -127,7 +123,6 @@ describe('@Body() on a method parameter', () => { api.method('rest-body'); expect(map).toBeCalled(); const mapperContext = map.mock.calls[0][1] as MapperContext; - expect(mapperContext.typeHint).toEqual(String); expect(mapperContext.mappers).toEqual(expect.any(MappersRegistry)); }); }); diff --git a/packages/httyped-client/src/annotations/fetch/path.spec.ts b/packages/httyped-client/src/annotations/fetch/path.spec.ts index 0e873436..27d66c71 100644 --- a/packages/httyped-client/src/annotations/fetch/path.spec.ts +++ b/packages/httyped-client/src/annotations/fetch/path.spec.ts @@ -93,9 +93,12 @@ describe.each(ALL_FETCH_ANNOTATIONS)( it(`calls fetch("", { method: ${method}})`, async () => { await api.method(); expect(fetchAdapter).toHaveBeenCalled(); - expect(fetchAdapter).toHaveBeenCalledWith(TEST_BASE_URL, { - method, - }); + expect(fetchAdapter).toHaveBeenCalledWith( + TEST_BASE_URL, + expect.objectContaining({ + method, + }), + ); }); }); @@ -104,9 +107,12 @@ describe.each(ALL_FETCH_ANNOTATIONS)( it(`calls fetch("", { method: ${method}})`, async () => { await api.method(); expect(fetchAdapter).toHaveBeenCalled(); - expect(fetchAdapter).toHaveBeenCalledWith(`${TEST_BASE_URL}/path`, { - method, - }); + expect(fetchAdapter).toHaveBeenCalledWith( + `${TEST_BASE_URL}/path`, + expect.objectContaining({ + method, + }), + ); }); }); }); diff --git a/packages/httyped-client/src/annotations/fetch/response-body.spec.ts b/packages/httyped-client/src/annotations/fetch/response-body.spec.ts index 4bdc8c1d..fad92690 100644 --- a/packages/httyped-client/src/annotations/fetch/response-body.spec.ts +++ b/packages/httyped-client/src/annotations/fetch/response-body.spec.ts @@ -2,32 +2,307 @@ import 'reflect-metadata'; import 'whatwg-fetch'; import { configureTesting } from '@aspectjs/common/testing'; +import { abstract } from '@aspectjs/common/utils'; import { getWeaver, WeaverModule } from '@aspectjs/core'; import nodeFetch from 'node-fetch'; import { HttypedClientAspect } from '../../aspects/httyped-client.aspect'; import { HttypedClientFactory } from '../../client-factory/client.factory'; import { ALL_FETCH_ANNOTATIONS } from '../../test-helpers/all-fetch-annotations.helper'; +import { MapperError } from '../../types/mapper.error'; +import { Mapper } from '../../types/mapper.type'; +import { HttypedClient } from '../http-client.annotation'; +import { TypeHint } from '../type.annotation'; const TEST_BASE_URL = 'http://testBaseUrl'; +interface IApi { + method(): Promise; +} +const USER: User = { + type: 'user', + name: 'Joe', + lastName: 'Dalton', +}; + +class User { + type: 'user' = 'user'; + name!: string; + lastName!: string; +} + describe.each(ALL_FETCH_ANNOTATIONS)( - 'return value of a method annotated with $annotationName()', + 'Given a method annotated with $annotationName() that returns a value', ({ annotation, method }) => { let fetchAdapter: typeof nodeFetch & jest.SpyInstance; let httypedClientFactory: HttypedClientFactory; let httypedClientAspect: HttypedClientAspect; + let api: IApi; + + let mapper1 = { + typeHint: 'Undefined' as Mapper['typeHint'], + map: jest.fn(), + } satisfies Mapper; + + let mapper2 = { + typeHint: 'Undefined' as Mapper['typeHint'], + map: jest.fn(), + } satisfies Mapper; beforeEach(() => { fetchAdapter = jest.fn((..._args: any[]) => { - return Promise.resolve(undefined as any); + return Promise.resolve(new Response(JSON.stringify(USER))) as any; }); configureTesting(WeaverModule); httypedClientAspect = new HttypedClientAspect(); getWeaver().enable(httypedClientAspect); + httypedClientFactory = new HttypedClientFactory({ - fetchAdapter: fetchAdapter, + fetchAdapter: (...args: any[]) => (fetchAdapter as any)(...args), baseUrl: TEST_BASE_URL, }); + + mapper1 = { + typeHint: 'Undefined', + map: jest.fn(), + } satisfies Mapper; + + mapper2 = { + typeHint: 'Undefined', + map: jest.fn(), + } satisfies Mapper; + }); + + describe('when no mappers matches', () => { + beforeEach(() => { + @HttypedClient() + abstract class Api implements IApi { + @annotation() + async method() { + return abstract(new User()); + } + } + api = httypedClientFactory.create(Api); + httypedClientFactory.addResponseBodyMappers(mapper1); + }); + it('returns a JSON object as is', async () => { + expect(await api.method()).toEqual(USER); + }); + + it('does not invoke the mappers', async () => { + await api.method(); + expect(mapper1.map).not.toHaveBeenCalled(); + }); + }); + + describe(`when the method returns a templated ${abstract.name}()`, () => { + beforeEach(() => { + @HttypedClient() + abstract class Api implements IApi { + @annotation() + async method() { + return abstract(new User()); + } + } + api = httypedClientFactory.create(Api); + }); + describe('and a mapper has been registered for this type constructor', () => { + beforeEach(() => { + mapper2.typeHint = [User, 'Void']; + mapper2.map = jest.fn((obj) => obj); + + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + }); + it('calls the mapper that matches the typeHint', async () => { + await api.method(); + + expect(mapper1.map).not.toHaveBeenCalled(); + expect(mapper2.map).toHaveBeenCalled(); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + }); + }); + + describe('and a mapper has been registered for this type name', () => { + beforeEach(() => { + mapper2.typeHint = ['User', 'Void']; + mapper2.map = jest.fn((obj) => obj); + + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + }); + it('calls the mapper that matches the typeHint', async () => { + await api.method(); + + expect(mapper1.map).not.toHaveBeenCalled(); + expect(mapper2.map).toHaveBeenCalled(); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + }); + }); + describe('and a mapper has been registered for a parent constructor', () => { + beforeEach(() => { + mapper2.typeHint = [User, 'Void']; + mapper2.map = jest.fn((obj) => obj); + + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + + class Customer extends User {} + @HttypedClient() + abstract class Api implements IApi { + @annotation() + async method() { + return abstract(new Customer()); + } + } + api = httypedClientFactory.create(Api); + }); + + it('calls the mapper that matches the parent type', async () => { + await api.method(); + + expect(mapper1.map).not.toHaveBeenCalled(); + expect(mapper2.map).toHaveBeenCalled(); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + }); + }); + + describe('and the returned abstract is an array template', () => { + beforeEach(() => { + mapper2.typeHint = [User, 'Void']; + mapper2.map = jest.fn((obj) => + Object.setPrototypeOf({ ...obj }, User.prototype), + ); + + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + + @HttypedClient() + abstract class Api implements IApi { + @annotation() + async method() { + return abstract([new User(), { street: 'street' }, new User()]); + } + } + api = httypedClientFactory.create(Api); + }); + describe('But the http call does not return an array', () => { + it('throws an error', async () => { + try { + await api.method(); + throw new Error('method call should have thrown an error'); + } catch (e) { + expect(e).toEqual(expect.any(MapperError)); + expect((e as Error).message).toEqual( + 'Mapper expected an array, but got Object', + ); + } + }); + }); + it('returns an array of values, calling the corresponding mappers', async () => { + fetchAdapter = jest.fn((..._args: any[]) => { + return Promise.resolve( + new Response(JSON.stringify([USER, { street: 'street' }, USER])), + ) as any; + }); + + const res = await api.method(); + + expect(mapper1.map).not.toHaveBeenCalled(); + expect(mapper2.map).toHaveBeenCalledTimes(2); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + expect(res[0]).toEqual(expect.any(User)); + expect(res[1]).not.toEqual(expect.any(User)); + expect(res[2]).toEqual(expect.any(User)); + }); + }); + }); + describe(`and the method has a ${TypeHint}() annotation`, () => { + describe(`that specifies the type as a constructor`, () => { + beforeEach(() => { + mapper2.typeHint = User; + mapper2.map = jest.fn((obj) => + Object.setPrototypeOf({ ...obj }, User.prototype), + ); + + @HttypedClient() + abstract class Api implements IApi { + @annotation() + @TypeHint(User) + async method() { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + }); + it(`calls the mapper that matches the typeHint`, async () => { + const res = await api.method(); + + expect(mapper2.map).toHaveBeenCalled(); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + expect(res).toEqual(expect.any(User)); + }); + }); + + describe(`that specifies the type as a type name`, () => { + beforeEach(() => { + mapper2.typeHint = User; + mapper2.map = jest.fn((obj) => + Object.setPrototypeOf({ ...obj }, User.prototype), + ); + + @HttypedClient() + abstract class Api implements IApi { + @annotation() + @TypeHint('User') + async method() { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + }); + it(`calls the mapper that matches the typeHint`, async () => { + const res = await api.method(); + + expect(mapper2.map).toHaveBeenCalled(); + const args = mapper2.map.mock.calls[0]!; + expect(args[0]).toEqual(USER); + + expect(res).toEqual(expect.any(User)); + }); + }); + }); + + describe('when several mappers match', () => { + beforeEach(() => { + mapper1.typeHint = User; + mapper2.typeHint = User; + + @HttypedClient() + abstract class Api implements IApi { + @annotation() + @TypeHint(User) + async method() { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + httypedClientFactory.addResponseBodyMappers(mapper1, mapper2); + }); + + it('calls the last mapper that matches', async () => { + await api.method(); + + expect(mapper2.map).toHaveBeenCalled(); + }); + + it('calls only the last mapper that matches', async () => { + await api.method(); + + expect(mapper1.map).not.toHaveBeenCalled(); + }); }); }, ); diff --git a/packages/httyped-client/src/annotations/header.annotation.ts b/packages/httyped-client/src/annotations/header.annotation.ts new file mode 100644 index 00000000..c4a173fc --- /dev/null +++ b/packages/httyped-client/src/annotations/header.annotation.ts @@ -0,0 +1,5 @@ +import { ASPECTJS_HTTP_ANNOTATION_FACTORY } from './annotation-factory'; +export const Header = ASPECTJS_HTTP_ANNOTATION_FACTORY.create( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + function Header(headerName: string, value: string | string[]) {}, +); diff --git a/packages/httyped-client/src/annotations/header.spec.ts b/packages/httyped-client/src/annotations/header.spec.ts new file mode 100644 index 00000000..f1dc3eb1 --- /dev/null +++ b/packages/httyped-client/src/annotations/header.spec.ts @@ -0,0 +1,166 @@ +import 'reflect-metadata'; +import 'whatwg-fetch'; + +import { configureTesting } from '@aspectjs/common/testing'; +import { abstract } from '@aspectjs/common/utils'; +import { WeaverModule, getWeaver } from '@aspectjs/core'; +import nodeFetch from 'node-fetch'; +import { HttypedClientAspect } from '../aspects/httyped-client.aspect'; +import { HttypedClientFactory } from '../client-factory/client.factory'; +import { ALL_FETCH_ANNOTATIONS } from '../test-helpers/all-fetch-annotations.helper'; +import { Header } from './header.annotation'; +import { HttypedClient } from './http-client.annotation'; + +const TEST_BASE_URL = 'http://testBaseUrl'; + +interface IApi { + method(...args: any[]): Promise; +} + +describe.each(ALL_FETCH_ANNOTATIONS)( + `${Header}(, ) annotation`, + ({ annotation, method }) => { + let fetchAdapter: typeof nodeFetch & jest.SpyInstance; + let httypedClientFactory: HttypedClientFactory; + let httypedClientAspect: HttypedClientAspect; + let api: IApi; + + beforeEach(() => { + fetchAdapter = jest.fn((..._args: any[]) => { + return Promise.resolve( + new Response('{ "val": "dummy response"}'), + ) as any; + }); + configureTesting(WeaverModule); + httypedClientAspect = new HttypedClientAspect(); + getWeaver().enable(httypedClientAspect); + + httypedClientFactory = new HttypedClientFactory({ + fetchAdapter: fetchAdapter, + baseUrl: TEST_BASE_URL, + }); + }); + + describe(`on a class annotated with ${HttypedClient}`, () => { + beforeEach(() => { + @HttypedClient() + @Header('header1', 'value1') + @Header('header2', 'value2') + abstract class Api implements IApi { + @annotation() + async method(args: any[]): Promise { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + }); + + describe(`calling a method annotated with ${method}`, () => { + // it('everything is fine', async () => {}); + it('adds the given header to the request', async () => { + await api.method(); + expect(fetchAdapter).toHaveBeenCalled(); + const requestInit: RequestInit = fetchAdapter.mock.calls[0][1]; + expect(requestInit.headers).toEqual( + expect.objectContaining({ + header1: 'value1', + header2: 'value2', + }), + ); + }); + + describe(`when parent class is annotated with ${Header}`, () => { + it(`merges with the parent headers`, async () => { + @HttypedClient() + @Header('header1', 'parentValue1') + @Header('parentHeader', 'parentValue') + abstract class ParentApi implements IApi { + @annotation() + async method(...args: any[]): Promise { + return abstract(); + } + } + + @HttypedClient() + @Header('header1', 'value1') + @Header('header2', 'value2') + abstract class Api extends ParentApi { + @annotation() + override async method(...args: any[]): Promise { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + + await api.method(); + expect(fetchAdapter).toHaveBeenCalled(); + const requestInit: RequestInit = fetchAdapter.mock.calls[0][1]; + expect(requestInit.headers).toEqual( + expect.objectContaining({ + parentHeader: 'parentValue', + header1: 'value1', + header2: 'value2', + }), + ); + }); + }); + }); + describe(`on a method annotated with ${annotation}`, () => { + beforeEach(() => { + @HttypedClient() + @Header('header1', 'classValue1') + @Header('header2', 'classValue2') + abstract class Api implements IApi { + @annotation() + @Header('header2', 'methodValue2') + async method(...args: any[]): Promise { + return abstract(); + } + } + api = httypedClientFactory.create(Api); + }); + + it('merges with the headers of the class', async () => { + await api.method(); + expect(fetchAdapter).toHaveBeenCalled(); + const requestInit: RequestInit = fetchAdapter.mock.calls[0][1]; + expect(requestInit.headers).toEqual( + expect.objectContaining({ + header1: 'classValue1', + header2: 'methodValue2', + }), + ); + }); + }); + xdescribe('on a property', () => { + it('throws an error', () => { + expect(() => { + @HttypedClient() + abstract class Api implements IApi { + @Header('header2', 'methodValue2') + prop!: string; + async method(...args: any[]): Promise {} + } + }).toThrow( + `[ajs.httyped-client]: Annotations are not allowed: ${Header.ref} on property Api.prop`, + ); + }); + }); + xdescribe('on a parameter', () => { + it('throws an error', () => { + expect(() => { + @HttypedClient() + abstract class Api implements IApi { + async method( + @Header('header2', 'methodValue2') + x: string, + ): Promise {} + } + }).toThrow( + `[ajs.httyped-client]: Annotations are not allowed: ${Header.ref} on argument Api.method[0]`, + ); + }); + }); + }); + }, +); diff --git a/packages/httyped-client/src/annotations/headers.annotation.ts b/packages/httyped-client/src/annotations/headers.annotation.ts new file mode 100644 index 00000000..be84584d --- /dev/null +++ b/packages/httyped-client/src/annotations/headers.annotation.ts @@ -0,0 +1,5 @@ +import { ASPECTJS_HTTP_ANNOTATION_FACTORY } from './annotation-factory'; +export const Headers = ASPECTJS_HTTP_ANNOTATION_FACTORY.create( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + function Headers(headers: HeadersInit) {}, +); diff --git a/packages/httyped-client/src/annotations/path-variable.spec.ts b/packages/httyped-client/src/annotations/path-variable.spec.ts index 9385ea47..845b55ea 100644 --- a/packages/httyped-client/src/annotations/path-variable.spec.ts +++ b/packages/httyped-client/src/annotations/path-variable.spec.ts @@ -68,13 +68,29 @@ describe.each(ALL_FETCH_ANNOTATIONS)( }); it('throws an error', async () => { expect(() => api.method('a', 'b')).toThrowError( - `[${HttypedClientAspect.name}]: ${PathVariable}(:unmatched) parameter of method HttpClientApi.method does not match url ${TEST_BASE_URL}/:variable`, + `[ajs.httyped-client]: ${PathVariable}(:unmatched) parameter of method HttpClientApi.method does not match url ${TEST_BASE_URL}/:variable`, ); }); }); - xdescribe(`when the method has the same ${PathVariable} more than once`, () => { - it('throws an error', () => {}); + describe(`when the method has the same ${PathVariable} more than once`, () => { + beforeEach(() => { + @HttypedClient() + class HttpClientApi { + @annotation('/:variable') + method( + @PathVariable('variable') var1: string, + @PathVariable('variable') var2: string, + ) {} + } + + api = httypedClientFactory.create(HttpClientApi); + }); + it('throws an error', async () => { + expect(() => api.method('a', 'b')).toThrowError( + `[ajs.httyped-client]: ${PathVariable}(variable) is specified twice for method HttpClientApi.method`, + ); + }); }); describe(`when the method has an argument annotated with ${PathVariable}`, () => { @@ -94,9 +110,12 @@ describe.each(ALL_FETCH_ANNOTATIONS)( it(`calls fetch("", { method: ${method}})`, async () => { await api.method('a', 'b'); expect(fetchAdapter).toHaveBeenCalled(); - expect(fetchAdapter).toHaveBeenCalledWith(`${TEST_BASE_URL}/a/b`, { - method, - }); + expect(fetchAdapter).toHaveBeenCalledWith( + `${TEST_BASE_URL}/a/b`, + expect.objectContaining({ + method, + }), + ); }); }); }, diff --git a/packages/httyped-client/src/annotations/type.annotation.ts b/packages/httyped-client/src/annotations/type.annotation.ts index 9328d4ff..6fdfa07a 100644 --- a/packages/httyped-client/src/annotations/type.annotation.ts +++ b/packages/httyped-client/src/annotations/type.annotation.ts @@ -1,6 +1,6 @@ import { AnnotationFactory } from '@aspectjs/common'; import { ConstructorType } from '@aspectjs/common/utils'; -export const Type = new AnnotationFactory('aspectjs.utils').create( - function Type(type: ConstructorType) {}, +export const TypeHint = new AnnotationFactory('aspectjs.utils').create( + function TypeHint(type: ConstructorType | string) {}, ); diff --git a/packages/httyped-client/src/aspects/abstract-aop-http-client.aspect.ts b/packages/httyped-client/src/aspects/abstract-aop-http-client.aspect.ts new file mode 100644 index 00000000..13ef0fd5 --- /dev/null +++ b/packages/httyped-client/src/aspects/abstract-aop-http-client.aspect.ts @@ -0,0 +1,194 @@ +import { + AdviceContext, + AfterReturnContext, + AspectError, + PointcutType, +} from '@aspectjs/core'; +import { PathVariable } from '../annotations/path-variable.annotation'; +import { HttypedClientConfig } from '../client-factory/client-config.type'; +import { + MissingPathVariableError, + PathVariableNotMatchedError, +} from '../client-factory/path-variables-handler.type'; +import { BodyMetadata } from '../types/body-metadata.type'; +import { HttpClassMetadata } from '../types/http-class-metadata.type'; +import { HttpEndpointMetadata } from '../types/http-endpoint-metadata.type'; +import { MapperContext } from '../types/mapper.type'; +import type { Request } from '../types/request-handler.type'; +import '../url-canparse.polyfill'; + +export abstract class AbstractAopHttpClientAspect { + constructor(private readonly aspectId: string) {} + + // Advice methods + // ===================== + + // protected methods + ////////////////////: + + protected callHttpAdapter( + endpointConfig: Required, + url: string, + requestInit: RequestInit, + ) { + return endpointConfig.fetchAdapter(url, requestInit as any); + } + + protected replacePathVariables( + url: string, + endpointConfig: Required, + ctxt: AdviceContext, + ): string { + const variableHandler = endpointConfig.pathVariablesHandler; + const variables = this.findPathVariables(ctxt); + + try { + return variableHandler.replace(url, variables); + } catch (e) { + if (e instanceof MissingPathVariableError) { + throw new AspectError( + this, + `${PathVariable}(${e.variable}) parameter is missing for ${ctxt.target.label}`, + ); + } else if (e instanceof PathVariableNotMatchedError) { + throw new AspectError( + this, + `${PathVariable}(${e.variable}) parameter of ${ctxt.target.label} does not match url ${url}`, + ); + } else { + throw e; + } + } + } + protected abstract findPathVariables( + ctxt: AdviceContext, + ): Record; + + protected applyRequestHandlers( + config: Required, + requestInit: Request, + ) { + for (const h of config.requestHandlers) { + requestInit = h(requestInit) ?? requestInit; + } + return requestInit; + } + + protected async applyResponseHandlers( + config: Required, + response: Response, + ctxt: AfterReturnContext, + ): Promise { + let mappedResponse: any; + for (const handler of config.responseHandlers) { + mappedResponse = await handler(response, config, ctxt); + } + + return mappedResponse; + } + + protected setClientConfig( + config: Required, + clientInstance: InstanceType, + ) { + Reflect.defineMetadata( + `${this.aspectId}:client-config`, + config, + clientInstance, + ); + } + + protected getClientConfig( + clientInstance: InstanceType, + ): Required { + return Reflect.getMetadata( + `${this.aspectId}:client-config`, + clientInstance, + ); + } + + protected isManagedInstance(clientInstance: InstanceType) { + return !!this.getClientConfig(clientInstance); + } + /** + * Extracts the endpoint metadata from the fetch annotation + */ + protected abstract getEndpointMetadata( + ctxt: AdviceContext, + ): HttpEndpointMetadata; + + /** + * Extracts the api metadata from the HttpClient annotation + */ + + protected abstract getClassMetadata( + ctxt: AdviceContext, + ): HttpClassMetadata; + + protected mergeConfig( + config: Required, + classConfig: HttpClassMetadata, + endpointMetadata: HttpEndpointMetadata, + ): Required { + const baseUrl = URL.canParse(classConfig.baseUrl ?? '') + ? classConfig.baseUrl! + : this.joinUrls(config.baseUrl, classConfig.baseUrl); + return { + ...config, + ...classConfig, + ...endpointMetadata, + baseUrl, + }; + } + + /** + * Extracts the method body metadata from annotations + * @param config The config of the HttpClientAspect + * @param ctxt the advice context + * @returns the method body metadata + */ + protected abstract findRequestBodyMetadata( + ctxt: AdviceContext, + ): BodyMetadata | undefined; + + /** + * Extracts the method body from metadata, then use the configured mappers to map the body object indo a BodyInit + * @param config The config of the HttpClientAspect + * @param ctxt the advice context + * @returns the method body + */ + protected serializeRequestBody( + config: Required, + ctxt: AdviceContext, + ): BodyInit | undefined { + const bodyMeta = this.findRequestBodyMetadata(ctxt); + if (!bodyMeta) { + return; + } + + const typeHint = bodyMeta.typeHint; + let body = bodyMeta.value; + + const mappers = config.requestBodyMappers; + const context: MapperContext = { + mappers, + data: {}, + }; + + const mapper = mappers.findMapper(typeHint); + body = mapper ? mapper.map(body, context) : body; + return typeof body === 'string' ? body : JSON.stringify(body); + } + + protected abstract findTypeHintAnnotation( + ctxt: AdviceContext, + ): Function | string | undefined; + + protected joinUrls(...paths: (string | undefined)[]): string { + return paths + .map((u) => u ?? '') + .map((u) => u.replace(/^\//, '')) + .filter((url) => url !== '') + .join('/'); + } +} diff --git a/packages/httyped-client/src/aspects/httyped-client.aspect.ts b/packages/httyped-client/src/aspects/httyped-client.aspect.ts index 206019c6..baec8dfe 100644 --- a/packages/httyped-client/src/aspects/httyped-client.aspect.ts +++ b/packages/httyped-client/src/aspects/httyped-client.aspect.ts @@ -12,7 +12,6 @@ import { Aspect, AspectError, Before, - BeforeContext, PointcutType, on, } from '@aspectjs/core'; @@ -21,25 +20,30 @@ import { FETCH_ANNOTATIONS, FetchAnnotationContext, } from '../annotations/fetch/fetch-annotations'; +import { Header } from '../annotations/header.annotation'; +import { Headers } from '../annotations/headers.annotation'; import { HttypedClient } from '../annotations/http-client.annotation'; import { PathVariable } from '../annotations/path-variable.annotation'; -import { Type } from '../annotations/type.annotation'; +import { TypeHint } from '../annotations/type.annotation'; import { HttypedClientConfig } from '../client-factory/client-config.type'; -import { - MissingPathVariableError, - PathVariableNotMatchedError, -} from '../client-factory/path-variables-handler.type'; +import { RequestParam } from '../public_api'; +import { BodyMetadata } from '../types/body-metadata.type'; import { HttpClassMetadata } from '../types/http-class-metadata.type'; import { HttpEndpointMetadata } from '../types/http-endpoint-metadata.type'; -import { MapperContext } from '../types/mapper.type'; import type { Request } from '../types/request-handler.type'; import '../url-canparse.polyfill'; +import { AbstractAopHttpClientAspect } from './abstract-aop-http-client.aspect'; -@Aspect('@aspectjs/http:client') -export class HttypedClientAspect { - constructor() {} +const ASPECT_ID = 'ajs.httyped-client'; +@Aspect(ASPECT_ID) +export class HttypedClientAspect extends AbstractAopHttpClientAspect { + constructor() { + super(ASPECT_ID); + } - defineClientConfig(clientInstance: T, config: HttypedClientConfig): T { + // Public methods + // ===================== + addClient(clientInstance: T, config: Required): T { const ctor = Object.getPrototypeOf(clientInstance).constructor; const httypedAnnotation = getAnnotations(HttypedClient) .onClass(ctor) @@ -52,35 +56,76 @@ export class HttypedClientAspect { ); } - Reflect.defineMetadata('@ajs/http:client-config', config, clientInstance!); + this.setClientConfig(config, clientInstance); return clientInstance; } - @Before(on.parameters.withAnnotations(Body)) - protected assertBodyImpliesFetch( - ctxt: BeforeContext, + // Advice methods + // ===================== + + @Before( + on.parameters.withAnnotations(Body), + on.parameters.withAnnotations(RequestParam), + on.parameters.withAnnotations(RequestParam), + on.parameters.withAnnotations(PathVariable), + ) + protected assertIsFetchMethod(ctxt: AdviceContext) { + ctxt.target.getMetadata(`${ASPECT_ID}:assertBodyImpliesFetch`, () => { + if (!this.findHttpMethodAnnotation(ctxt)) { + throw new AspectError( + this, + `${ctxt.target.label} is missing a fetch annotation`, + ); + } + return true; + }); + } + + @Before( + on.classes.withAnnotations(Header), + on.methods.withAnnotations(Header), + on.parameters.withAnnotations(Body), + ...FETCH_ANNOTATIONS.map((a) => on.methods.withAnnotations(a)), + ) + protected assertIsFetchClient( + ctxt: AdviceContext, ) { - if (ctxt.target.getMetadata('@ajs/http:assertBodyImpliesFetch')) { - return; - } - if (!this.findFetchAnnotation(ctxt)) { - throw new AspectError( - this, - `${ctxt.target.label} is missing a fetch annotation`, - ); - } - ctxt.target.defineMetadata('@ajs/http:assertBodyImpliesFetch', true); + ctxt.target.getMetadata(`${ASPECT_ID}:assertHeaderImpliesClient`, () => { + if (!this.findHttpClientAnnotation(ctxt)) { + throw new AspectError( + this, + `${ctxt.target.declaringClass.label} is missing a ${HttypedClient} annotation`, + ); + } + return true; + }); + } + + @Before( + on.properties.withAnnotations(Header), + on.parameters.withAnnotations(Header), + ) + protected prohibitWrongTarget( + ctxt: AdviceContext, + ) { + throw new AspectError( + this, + `Annotations are not allowed: ${ctxt + .annotations() + .find() + .map((a) => `${a}`)}`, + ); } @AfterReturn(...FETCH_ANNOTATIONS.map((a) => on.methods.withAnnotations(a))) - protected fetch(ctxt: AfterReturnContext) { - let config = this.getClientConfig(ctxt); - if (!config) { + protected fetch(ctxt: AfterReturnContext) { + if (!this.isManagedInstance(ctxt.instance!)) { // not received a client config = not created through the HttypedClientFactory. return; } + let config = this.getClientConfig(ctxt.instance!); - const endpointMetadata = this.getEndpointMedatada(ctxt); + const endpointMetadata = this.getEndpointMetadata(ctxt); const classMetadata = this.getClassMetadata(ctxt); const endpointConfig = this.mergeConfig( @@ -89,9 +134,11 @@ export class HttypedClientAspect { endpointMetadata, ); - let url = joinUrls(endpointConfig.baseUrl, endpointMetadata.url); - - url = this.replacePathVariables(url, endpointConfig, ctxt); + let url = this.replacePathVariables( + this.joinUrls(endpointConfig.baseUrl, endpointMetadata.url), + endpointConfig, + ctxt, + ); let requestInit: Request = { ...endpointConfig.requestInit, @@ -103,34 +150,35 @@ export class HttypedClientAspect { requestInit.body = body; } + requestInit.headers = this.getHeadersMetadata(ctxt); + requestInit = this.applyRequestHandlers(config, requestInit); url = requestInit.url; delete (requestInit as Partial & RequestInit).url; return this.callHttpAdapter(endpointConfig, url, requestInit).then( - async (r) => this.applyResponseHandlers(config, r as Response), + async (r) => this.applyResponseHandlers(config, r as Response, ctxt), ); } - callHttpAdapter( - endpointConfig: Required, - url: string, - requestInit: RequestInit, - ) { - return endpointConfig.fetchAdapter(url, requestInit as any); - } - protected replacePathVariables( - url: string, - endpointConfig: Required, - ctxt: AfterReturnContext, - ): string { - const variableHandler = endpointConfig.pathVariablesHandler; - - const variables = ctxt + + // protected methods + ////////////////////: + + protected override findPathVariables( + ctxt: AdviceContext, + ): Record { + return ctxt .annotations(PathVariable) .find({ searchParents: true }) .reduce( (variables, annotation) => { + if (Object.getOwnPropertyDescriptor(variables, annotation.args[0])) { + throw new AspectError( + this, + `${PathVariable}(${annotation.args[0]}) is specified twice for ${ctxt.target.label}`, + ); + } return { ...variables, [annotation.args[0]]: annotation.target.eval(), @@ -138,80 +186,88 @@ export class HttypedClientAspect { }, {} as Record, ); - - try { - return variableHandler.replace(url, variables); - } catch (e) { - if (e instanceof MissingPathVariableError) { - throw new AspectError( - this, - `${PathVariable}(${e.variable}) parameter is missing for ${ctxt.target.label}`, - ); - } else if (e instanceof PathVariableNotMatchedError) { - throw new AspectError( - this, - `${PathVariable}(${e.variable}) parameter of ${ctxt.target.label} does not match url ${url}`, - ); - } else { - throw e; - } - } - } - protected applyRequestHandlers( - config: Required, - requestInit: Request, - ) { - for (const h of config.requestHandlers) { - requestInit = h(requestInit) ?? requestInit; - } - return requestInit; - } - - protected async applyResponseHandlers( - config: Required, - response: Response, - ): Promise { - let mappedResponse: any; - for (const handler of config.responseHandler) { - mappedResponse = await handler(response); - } - - return mappedResponse; - } - - protected getClientConfig( - ctxt: AfterReturnContext, - ): Required { - return Reflect.getMetadata('@ajs/http:client-config', ctxt.instance!); } /** * Extracts the endpoint metadata from the fetch annotation */ - protected getEndpointMedatada( - ctxt: AfterReturnContext, + protected override getEndpointMetadata( + ctxt: AdviceContext, ): HttpEndpointMetadata { - let metadata = - ctxt.target.getMetadata('@ajs/http:endpoint'); + return ctxt.target.getMetadata( + `${ASPECT_ID}:endpoint`, + () => { + const fetchAnnotation = this.findHttpMethodAnnotation(ctxt)!; - if (metadata) { - return metadata; - } - const fetchAnnotation = this.findFetchAnnotation(ctxt)!; + let url = fetchAnnotation.args[0] ?? ''; - let url = fetchAnnotation.args[0] ?? ''; + const metadata = { + url, + method: + fetchAnnotation.ref.name.toLowerCase() as HttpEndpointMetadata['method'], + requestInit: fetchAnnotation.args[1], + }; - metadata = { - url, - method: - fetchAnnotation.ref.name.toLowerCase() as HttpEndpointMetadata['method'], - requestInit: fetchAnnotation.args[1], - }; + ctxt.target.defineMetadata(`${ASPECT_ID}:endpoint`, metadata); + + assert(!!metadata); + return metadata!; + }, + ); + } - ctxt.target.defineMetadata('@ajs/http:endpoint', metadata); + protected getHeadersMetadata( + ctxt: AfterReturnContext, + ): HeadersInit { + return ctxt.target.getMetadata(`${ASPECT_ID}:headers`, () => { + // reverse annotations order in order to let child annotations override parent annotations + const headerAnnotations = this.findHeaderAnnotations(ctxt).reverse(); + const headersAnnotations = this.findHeadersAnnotations(ctxt).reverse(); + + const headers = [ + headersAnnotations.map((a) => ({ + type: a.target.type, + headers: a.args[0], + })), + headerAnnotations.map((a) => ({ + type: a.target.type, + headers: { [a.args[0]]: a.args[1] }, + })), + ] + .flat() + // sort class annotations first + .sort((a1, a2) => a1.type - a2.type) + .reduce((res, { headers }) => { + return { ...res, ...headers } as HeadersInit; + }, {} as HeadersInit); + + return headers; + }); + } - assert(!!metadata); - return metadata!; + protected findHeaderAnnotations( + ctxt: AfterReturnContext, + ) { + return getAnnotations(Header) + .on({ + target: ctxt.target, + types: [AnnotationType.CLASS, AnnotationType.METHOD], + }) + .find({ + searchParents: true, + }); + } + protected findHeadersAnnotations( + ctxt: AfterReturnContext, + ) { + return getAnnotations(Headers) + .on({ + target: ctxt.target, + types: [AnnotationType.CLASS, AnnotationType.METHOD], + }) + .find({ + searchParents: true, + }); } /** @@ -219,12 +275,11 @@ export class HttypedClientAspect { */ protected getClassMetadata( - ctxt: AfterReturnContext, + ctxt: AdviceContext, ): HttpClassMetadata { - let metadata = - ctxt.target.declaringClass.getMetadata( - '@ajs/http:class', - ); + let metadata = ctxt.target.declaringClass.getMetadata( + `${ASPECT_ID}:class`, + ); if (metadata) { return metadata!; @@ -238,27 +293,38 @@ export class HttypedClientAspect { baseUrl: arg, } : arg ?? {}; - ctxt.target.defineMetadata('@ajs/http:class', metadata); - + `${ASPECT_ID}:class`; return metadata!; } - protected mergeConfig( - config: Required, - classConfig: HttpClassMetadata, - endpointMetadata: HttpEndpointMetadata, - ): Required { - const baseUrl = URL.canParse(classConfig.baseUrl ?? '') - ? classConfig.baseUrl! - : joinUrls(config.baseUrl, classConfig.baseUrl); + protected findRequestBodyMetadata( + ctxt: AdviceContext, + ): BodyMetadata | undefined { + const bodyAnnotation = this.findBodyAnnotation(ctxt); + if (bodyAnnotation === undefined) { + return; + } + + // const typeHint = + // ctxt.annotations(Type).find({ searchParents: true })[0]?.args[0] ?? + // ( + // bodyAnnotation.target.parent.getMetadata( + // 'design:paramtypes', + // ) as unknown[] + // )[bodyAnnotation.target.parameterIndex]; + return { - ...config, - ...classConfig, - ...endpointMetadata, - baseUrl, + value: bodyAnnotation.target.eval(), + typeHint: Object.getPrototypeOf(bodyAnnotation.target.eval()).constructor, }; } + protected findTypeHintAnnotation( + ctxt: AdviceContext, + ): Function | string | undefined { + return ctxt.annotations(TypeHint).find({ searchParents: true })[0]?.args[0]; + } + /** * Extracts the method body from metadata * @param config The config of the HttpClientAspect @@ -285,49 +351,9 @@ export class HttypedClientAspect { return bodyAnnotations[0]; } - /** - * Extracts the method body from metadata, then use the configured mappers to map the body object indo a BodyInit - * @param config The config of the HttpClientAspect - * @param ctxt the advice context - * @returns the method body - */ - protected serializeRequestBody( - config: Required, + protected findHttpMethodAnnotation( ctxt: AdviceContext, - ): BodyInit | undefined { - const bodyAnnotation = this.findBodyAnnotation(ctxt); - if (bodyAnnotation === undefined) { - return; - } - - let body = bodyAnnotation.target.eval(); - - if (typeof body === undefined) { - return; - } - const typeHint = - ctxt.annotations(Type).find({ searchParents: true })[0]?.args[0] ?? - ( - bodyAnnotation.target.parent.getMetadata( - 'design:paramtypes', - ) as unknown[] - )[bodyAnnotation.target.parameterIndex]; - - const mappers = config.requestBodyMappers; - const context: MapperContext = { - typeHint, - mappers, - }; - for (let mapper of mappers) { - if (mapper.accepts(body, context)) { - body = mapper.map(body, context); - break; - } - } - - return typeof body === 'string' ? body : JSON.stringify(body); - } - protected findFetchAnnotation(ctxt: AdviceContext): FetchAnnotationContext { + ): FetchAnnotationContext { const fetchAnnotations = ctxt .annotations(...FETCH_ANNOTATIONS) .find({ searchParents: true }); @@ -377,11 +403,3 @@ export class HttypedClientAspect { return httpClientAnnotation; } } - -function joinUrls(...paths: (string | undefined)[]): string { - return paths - .map((u) => u ?? '') - .map((u) => u.replace(/^\//, '')) - .filter((url) => url !== '') - .join('/'); -} diff --git a/packages/httyped-client/src/client-factory/client-config.type.ts b/packages/httyped-client/src/client-factory/client-config.type.ts index dac26ece..566d790c 100644 --- a/packages/httyped-client/src/client-factory/client-config.type.ts +++ b/packages/httyped-client/src/client-factory/client-config.type.ts @@ -9,7 +9,7 @@ export interface HttypedClientConfig { requestInit?: RequestInit; fetchAdapter?: FetchAdapter; requestHandlers?: RequestHandler[]; - responseHandler?: ResponseHandler[]; + responseHandlers?: ResponseHandler[]; responseBodyMappers?: MappersRegistry; requestBodyMappers?: MappersRegistry; pathVariablesHandler?: PathVariablesHandler; diff --git a/packages/httyped-client/src/client-factory/client.factory.ts b/packages/httyped-client/src/client-factory/client.factory.ts index 937092a5..2fd8a3f0 100644 --- a/packages/httyped-client/src/client-factory/client.factory.ts +++ b/packages/httyped-client/src/client-factory/client.factory.ts @@ -5,12 +5,11 @@ import { Mapper, MappersRegistry } from '../types/mapper.type'; import { RequestHandler } from '../types/request-handler.type'; import { ResponseHandler } from '../types/response-handler.type'; import { HttypedClientConfig } from './client-config.type'; +import { MAP_JSON_RESPONSE_HANDLER } from './default-json-response-handler'; import { DefaultPathVariablesHandler } from './path-variables-handler.type'; -const JSON_RESPONSE_HANDLER: ResponseHandler = (r) => r.json(); - const DEFAULT_CONFIG: Required = { - responseHandler: [JSON_RESPONSE_HANDLER], + responseHandlers: [MAP_JSON_RESPONSE_HANDLER], requestHandlers: [], baseUrl: '', requestInit: {}, @@ -21,14 +20,13 @@ const DEFAULT_CONFIG: Required = { }; const configureAspect = () => { - let clientAspect = getWeaver().getAspects(HttypedClientAspect)[0]; + let clientAspect = getWeaver().getAspect(HttypedClientAspect); if (clientAspect) { return clientAspect; } clientAspect = new HttypedClientAspect(); getWeaver().enable(clientAspect); - return clientAspect; }; @@ -43,35 +41,32 @@ export class HttypedClientFactory { } addResonseHandler(...handlers: ResponseHandler[]): HttypedClientFactory { - return new HttypedClientFactory({ - ...this.config, - responseHandler: [...this.config.responseHandler, ...handlers], - }); + this.config.responseHandlers = [ + ...this.config.responseHandlers, + ...handlers, + ]; + + return this; } addRequestHandler(...handlers: RequestHandler[]): HttypedClientFactory { - return new HttypedClientFactory({ - ...this.config, - requestHandlers: [...this.config.requestHandlers, ...handlers], - }); + this.config.requestHandlers = [...this.config.requestHandlers, ...handlers]; + + return this; } addRequestBodyMappers(...mappers: Mapper[]): HttypedClientFactory { - return new HttypedClientFactory({ - ...this.config, - requestBodyMappers: new MappersRegistry( - this.config.requestBodyMappers, - ).add(...mappers), - }); + this.config.requestBodyMappers = new MappersRegistry( + this.config.requestBodyMappers, + ).add(...mappers); + return this; } addResponseBodyMappers(...mappers: Mapper[]): HttypedClientFactory { - return new HttypedClientFactory({ - ...this.config, - responseBodyMappers: new MappersRegistry( - this.config.responseBodyMappers, - ).add(...mappers), - }); + this.config.responseBodyMappers = new MappersRegistry( + this.config.responseBodyMappers, + ).add(...mappers); + return this; } create( @@ -83,7 +78,7 @@ export class HttypedClientFactory { args ?? ([] as any), ); - return configureAspect().defineClientConfig(client, this.config); + return configureAspect().addClient(client, this.config); } } diff --git a/packages/httyped-client/src/client-factory/default-json-response-handler.ts b/packages/httyped-client/src/client-factory/default-json-response-handler.ts new file mode 100644 index 00000000..f0c9c0db --- /dev/null +++ b/packages/httyped-client/src/client-factory/default-json-response-handler.ts @@ -0,0 +1,111 @@ +import { + AbstractToken, + ConstructorType, + isAbstractToken, +} from '@aspectjs/common/utils'; +import { AfterReturnContext, PointcutType } from '@aspectjs/core'; +import { TypeHint } from '../annotations/type.annotation'; +import { MapperError } from '../types/mapper.error'; +import { MapperContext } from '../types/mapper.type'; +import { ResponseHandler } from '../types/response-handler.type'; +import { TypeHintType } from '../types/type-hint.type'; + +export const MAP_JSON_RESPONSE_HANDLER: ResponseHandler = async ( + response, + config, + ctxt, +) => { + let body = await response.json(); + + if (body) { + let typeHint = await _findTypeHint(ctxt); + + const context: MapperContext = { + mappers: config.responseBodyMappers, + data: {}, + }; + + if (Array.isArray(body)) { + return _arrayToMappedType(body, context, typeHint); + } else { + if (Array.isArray(typeHint)) { + throw new MapperError( + body, + typeHint, + `Mapper expected an array, but got ${ + Object.getPrototypeOf(body).constructor.name + }`, + ); + } + const mapper = config.responseBodyMappers.findMapper(typeHint); + return mapper ? mapper.map(body, context) : body; + } + } + + return body; +}; +async function _findTypeHint( + ctxt: AfterReturnContext, +): Promise { + return ctxt.target.getMetadata>( + 'ajs.httyped-client:response-typeHint', + async () => { + const typeAnnotation = ctxt + .annotations(TypeHint) + .find({ searchParents: true })[0]; + + if (typeAnnotation) { + return typeAnnotation.args[0]; + } + + const returnValue = await ctxt.value; + const abstractTokenTemplate = isAbstractToken(returnValue) + ? (returnValue as AbstractToken).template + : undefined; + + if (abstractTokenTemplate !== undefined) { + return Array.isArray(abstractTokenTemplate) + ? _arrayToTypeArray(abstractTokenTemplate) + : Object.getPrototypeOf(abstractTokenTemplate).constructor; + } + }, + ); +} + +function _arrayToTypeArray(array: T[]): ConstructorType[] { + return array.map((t) => { + return Array.isArray(t) + ? _arrayToTypeArray(t) + : Object.getPrototypeOf(t).constructor; + }); +} + +function _arrayToMappedType( + array: T[], + context: MapperContext, + typeHint: TypeHintType | TypeHintType[], +): U[] { + if (Array.isArray(typeHint) && typeHint.length === 1) { + // take single typehint as a template for all array items + return _arrayToMappedType( + array, + context, + (typeHint = typeHint[0] as TypeHintType), + ); + } else if (!Array.isArray(typeHint)) { + const mapper = context.mappers.findMapper(typeHint); + const res = mapper ? array.map((u) => mapper.map(u, context)) : array; + return res as U[]; + } else if (typeHint.length === array.length) { + return typeHint.map((t, i) => { + const mapper = context.mappers.findMapper(t); + return (mapper ? mapper.map(array[i], context) : array[i]) as U; + }); + } else { + throw new TypeError( + `Mapper expected a tuple (${typeHint.map((t) => + t instanceof Function ? t.name : t, + )}), but got Array.length = ${array.length}`, + ); + } +} diff --git a/packages/httyped-client/src/types/body-metadata.type.ts b/packages/httyped-client/src/types/body-metadata.type.ts new file mode 100644 index 00000000..246d6c54 --- /dev/null +++ b/packages/httyped-client/src/types/body-metadata.type.ts @@ -0,0 +1,6 @@ +import { TypeHintType } from './type-hint.type'; + +export interface BodyMetadata { + value: T; + typeHint: TypeHintType; +} diff --git a/packages/httyped-client/src/types/mapper.error.ts b/packages/httyped-client/src/types/mapper.error.ts new file mode 100644 index 00000000..79b4e98d --- /dev/null +++ b/packages/httyped-client/src/types/mapper.error.ts @@ -0,0 +1,14 @@ +import { TypeHintType } from './type-hint.type'; + +/** + * Error that is thrown when something unexpected happens while mapping a Request Body or a Response Body. + */ +export class MapperError extends Error { + constructor( + readonly value: any, + readonly typeHint: TypeHintType | TypeHintType[] | undefined, + message: string, + ) { + super(message); + } +} diff --git a/packages/httyped-client/src/types/mapper.type.ts b/packages/httyped-client/src/types/mapper.type.ts index 67dfe4b8..896b4ff3 100644 --- a/packages/httyped-client/src/types/mapper.type.ts +++ b/packages/httyped-client/src/types/mapper.type.ts @@ -1,45 +1,33 @@ -import { assert } from '@aspectjs/common/utils'; +import { ConstructorType } from '@aspectjs/common/utils'; +import { TypeHintType } from './type-hint.type'; export interface MapperContext { - typeHint: Function; - mappers: MappersRegistry; + readonly mappers: MappersRegistry; + readonly data: Record; } export interface Mapper { - accepts(obj: T, context: MapperContext): boolean; + typeHint: TypeHintType | TypeHintType[]; map(obj: T, context: MapperContext): U; } export class MappersRegistry { - private readonly knownMappers: Map = new Map(); - private readonly mappers: Mapper[] = []; + private readonly mappers: Map = new Map(); constructor(mappersReg?: MappersRegistry) { if (mappersReg) { - this.knownMappers = new Map(mappersReg.knownMappers); - this.mappers = [...mappersReg.mappers]; - } - } - accepts(obj: T, context: MapperContext): boolean { - return !!this.findMapper(obj, context); - } - map(obj: T, context: MapperContext): U { - const mapper = this.findMapper(obj, context); - if (mapper) { - return mapper.map(obj, context) as U; - } else { - throw new Error( - `No mapper found for object of type ${ - Object.getPrototypeOf(obj).constructor.name - }`, - ); + this.mappers = new Map(mappersReg.mappers); } } add(...mappers: Mapper[]) { - this.mappers.unshift(...mappers); - - // have to refresk all known mappers - this.knownMappers.clear(); + mappers.forEach((mapper) => { + [mapper.typeHint].flat().forEach((typeHint) => { + this.mappers.set(typeHint, mapper); + if (typeof typeHint === 'function') { + this.mappers.set(typeHint.name, mapper); + } + }); + }); return this; } @@ -48,23 +36,28 @@ export class MappersRegistry { return this.mappers[Symbol.iterator](); } - private findMapper(obj: T, context: MapperContext): Mapper | undefined { - const ctor = Object.getPrototypeOf(obj).constructor; - let mapper = this.knownMappers.get(ctor); - if (mapper) { - assert( - mapper.accepts(obj, context), - `Mapper ${mapper.constructor.name} does not accept object of type ${ - Object.getPrototypeOf(obj).constructor.name - }`, - ); - } else { - for (const m of this.mappers) { - if (m.accepts(obj, context)) { - this.knownMappers.set(ctor, m); - mapper = m; - break; - } + findMapper(typeHint: TypeHintType): Mapper | undefined { + let mapper = this.mappers.get(typeHint); + if (!mapper && typeof (typeHint as Function) === 'function') { + // try to lookup mappers by type name + mapper = this._findMapperByCtor(typeHint as ConstructorType); + } + + return mapper; + } + + private _findMapperByCtor( + ctor: ConstructorType, + ): Mapper | undefined { + let mapper = + this.mappers.get(ctor) ?? (this.mappers.get(ctor.name) as Mapper); + + if (!mapper) { + const parent = ctor.prototype + ? Object.getPrototypeOf(ctor.prototype)?.constructor + : null; + if (parent && parent !== Object) { + return this._findMapperByCtor(parent); } } diff --git a/packages/httyped-client/src/types/response-handler.type.ts b/packages/httyped-client/src/types/response-handler.type.ts index 13dac48c..69893791 100644 --- a/packages/httyped-client/src/types/response-handler.type.ts +++ b/packages/httyped-client/src/types/response-handler.type.ts @@ -1 +1,8 @@ -export type ResponseHandler = (response: Response) => Promise; +import { AfterReturnContext, PointcutType } from '@aspectjs/core'; +import { HttypedClientConfig } from '../client-factory/client-config.type'; + +export type ResponseHandler = ( + response: Response, + config: Readonly>, + ctxt: AfterReturnContext, +) => Promise; diff --git a/packages/httyped-client/src/types/type-hint.type.ts b/packages/httyped-client/src/types/type-hint.type.ts new file mode 100644 index 00000000..f4982c33 --- /dev/null +++ b/packages/httyped-client/src/types/type-hint.type.ts @@ -0,0 +1,3 @@ +import { ConstructorType } from '@aspectjs/common/utils'; + +export type TypeHintType = string | ConstructorType; diff --git a/packages/httyped-client/test/main.ts b/packages/httyped-client/test/main.ts index ba3b6f3f..78e79588 100755 --- a/packages/httyped-client/test/main.ts +++ b/packages/httyped-client/test/main.ts @@ -1,5 +1,5 @@ -import { Annotation, AnnotationFactory } from '@aspectjs/common'; -import { Aspect, BeforeContext, Compile, getWeaver, on } from '@aspectjs/core'; +import { AnnotationFactory } from '@aspectjs/common'; +import { AnnotationMixinAspect, getWeaver } from '@aspectjs/core'; import 'reflect-metadata'; import 'whatwg-fetch'; @@ -16,44 +16,7 @@ const GetAnnotation = new AnnotationFactory('test').create(function Get( ...args: any[] ) {}); -@Aspect('test') -class DecoratorBridgeAspect { - private bridgeCounter = 0; - constructor() {} - - bridge( - annotation: Annotation, - decorator: (...args: any[]) => MethodDecorator, - ) { - const proto = Object.getPrototypeOf(this); - const bridgeAdviceName = `bridge_${annotation.ref}_${decorator.name}_${this - .bridgeCounter++}`; - - const bridgeAdvice: PropertyDescriptor = { - value: (ctxt: BeforeContext) => { - console.log(`Invoking decorator ${decorator.name}`); - - return (decorator(ctxt.annotations(annotation).find()[0]!.args) as any)( - ...ctxt.target.asDecoratorArgs(), - ); - }, - writable: true, - configurable: true, - }; - - Object.defineProperty(proto, bridgeAdviceName, bridgeAdvice); - - Compile(on.any.withAnnotations(annotation))( - proto, - bridgeAdviceName, - bridgeAdvice, - ); - - return this; - } -} - -getWeaver().enable(new DecoratorBridgeAspect().bridge(GetAnnotation, Get)); +getWeaver().enable(new AnnotationMixinAspect().bridge(GetAnnotation, Get)); class X { @GetAnnotation('annotationArg1') diff --git a/packages/httyped-client/tsconfig.spec.json b/packages/httyped-client/tsconfig.spec.json index 6cd4f9fc..5546ad85 100644 --- a/packages/httyped-client/tsconfig.spec.json +++ b/packages/httyped-client/tsconfig.spec.json @@ -1,8 +1,5 @@ { "$schema": "http://json.schemastore.org/tsconfig", "extends": "../../tsconfig.spec.json", - "include": ["src/**/*.spec.ts", "src/**/*.d.ts"], - "compilerOptions": { - "types": ["jest", "node", "reflect-metadata"] - } + "include": ["src/**/*.spec.ts", "src/**/*.d.ts"] } diff --git a/packages/nestjs/common/src/nestjs-common-bridge.aspect.ts b/packages/nestjs/common/src/nestjs-common-bridge.aspect.ts index ec4cdcfe..92b81050 100644 --- a/packages/nestjs/common/src/nestjs-common-bridge.aspect.ts +++ b/packages/nestjs/common/src/nestjs-common-bridge.aspect.ts @@ -1,5 +1,4 @@ -import { AnnotationRef } from '@aspectjs/common'; -import { Aspect, DecoratorBridgeAspect } from '@aspectjs/core'; +import { AnnotationMixinAspect, Aspect } from '@aspectjs/core'; import { Body as NBody, Controller as NController, @@ -22,21 +21,11 @@ import { Options } from './annotations/options.annotation'; import { Patch } from './annotations/patch.annotation'; import { Post } from './annotations/post.annotation'; import { Put } from './annotations/put.annotation'; -@Aspect('@aspectjs/nestjs:common-bridge') -export class NestJSCommonBridgeAspect extends DecoratorBridgeAspect { +@Aspect('ajs.nestjs:common-bridge') +export class NestJSCommonMixinAspect extends AnnotationMixinAspect { constructor() { super(); - // bridge @aspectjs/http-client annotations - this.bridge(AnnotationRef.of('aspectjs.http', 'Get'), NGet) - .bridge(AnnotationRef.of('aspectjs.http', 'Post'), NPost) - .bridge(AnnotationRef.of('aspectjs.http', 'Put'), NPut) - .bridge(AnnotationRef.of('aspectjs.http', 'Patch'), NPatch) - .bridge(AnnotationRef.of('aspectjs.http', 'Delete'), NDelete) - .bridge(AnnotationRef.of('aspectjs.http', 'Options'), NOptions) - .bridge(AnnotationRef.of('aspectjs.http', 'Head'), NHead) - .bridge(AnnotationRef.of('aspectjs.http', 'Body'), NBody); - // bridge @aspectjs/nestjs/common annotations this.bridge(Get, NGet) .bridge(Post, NPost) diff --git a/tsconfig.spec.json b/tsconfig.spec.json index 0ed8929c..e3ab2d80 100644 --- a/tsconfig.spec.json +++ b/tsconfig.spec.json @@ -6,7 +6,7 @@ "emitDecoratorMetadata": true, "outDir": "./out-tsc/spec", "lib": ["DOM", "ES2021"], - "types": ["jest", "node"], + "types": ["jest", "node", "reflect-metadata"], "noUnusedLocals": false, "noUnusedParameters": false, "paths": {