From edab7a1d24b2f25f59af01aad1275ea74dee3879 Mon Sep 17 00:00:00 2001 From: David Luecke Date: Sat, 4 Jan 2020 17:40:58 -0800 Subject: [PATCH] feat: Finalize functionality for initial release of @feathersjs/hooks package (#1) --- packages/hooks/src/base.ts | 72 +++++++++++++++++ packages/hooks/src/decorator.ts | 24 ++++-- packages/hooks/src/function.ts | 110 ++++++++------------------ packages/hooks/src/index.ts | 28 +++++-- packages/hooks/src/object.ts | 17 ++-- packages/hooks/test/decorator.test.ts | 65 +++++++++++---- packages/hooks/test/function.test.ts | 91 ++++++++++----------- packages/hooks/test/object.test.ts | 36 +++++---- 8 files changed, 272 insertions(+), 171 deletions(-) create mode 100644 packages/hooks/src/base.ts diff --git a/packages/hooks/src/base.ts b/packages/hooks/src/base.ts new file mode 100644 index 0000000..a9e2951 --- /dev/null +++ b/packages/hooks/src/base.ts @@ -0,0 +1,72 @@ +import { Middleware } from './compose'; + +export const HOOKS: string = Symbol('@feathersjs/hooks') as any; + +/** + * + * @param target The target object or function + * @param middleware + */ +export function registerMiddleware (target: T, middleware: Middleware[]) { + const current: Middleware[] = (target as any)[HOOKS] || []; + + (target as any)[HOOKS] = current.concat(middleware); + + return target; +} + +export function getMiddleware (target: any): Middleware[] { + return (target && target[HOOKS]) || []; +} + +/** + * The base hook context. + */ +export class HookContext { + result?: T; + arguments: any[]; + [key: string]: any; + + constructor (data: { [key: string]: any } = {}) { + Object.assign(this, data); + } +} + +/** + * A function that updates the hook context with the `this` reference and + * arguments of the function call. + */ +export type ContextUpdater = (self: any, args: any[], context: HookContext) => HookContext; + +/** + * Returns a ContextUpdater function that turns function arguments like + * `function (data, name)` into named properties (`context.data`, `context.name`) + * on the hook context + * + * @param params The list of parameter names + */ +export function withParams (...params: string[]) { + return (self: any, args: any[], context: HookContext) => { + params.forEach((name, index) => { + context[name] = args[index]; + }); + + if (params.length > 0) { + Object.defineProperty(context, 'arguments', { + get (this: HookContext) { + const result = params.map(name => this[name]); + + return Object.freeze(result); + } + }); + } else { + context.arguments = args; + } + + if (self) { + context.self = self; + } + + return context; + }; +} diff --git a/packages/hooks/src/decorator.ts b/packages/hooks/src/decorator.ts index 5d1275a..2d2b710 100644 --- a/packages/hooks/src/decorator.ts +++ b/packages/hooks/src/decorator.ts @@ -1,23 +1,35 @@ import { Middleware } from './compose'; -import { functionHooks, ContextCreator, initContext } from './function'; +import { functionHooks } from './function'; +import { ContextUpdater, withParams, HookContext, registerMiddleware } from './base'; + +export const hookDecorator = (hooks: Array>, _updateContext?: ContextUpdater) => { + return (_target: any, method: string, descriptor: TypedPropertyDescriptor): TypedPropertyDescriptor => { + if (!descriptor) { + if (_updateContext) { + throw new Error('Context can not be updated at the class decorator level. Remove updateContext parameter.'); + } + + registerMiddleware(_target.prototype, hooks); + + return _target; + } -export const hookDecorator = (hooks: Array>, createContext: ContextCreator = initContext()) => { - return (_target: object, method: string, descriptor: TypedPropertyDescriptor): TypedPropertyDescriptor => { const fn = descriptor.value; if (typeof fn !== 'function') { throw new Error(`Can not apply hooks. '${method}' is not a function`); } - const methodContext = (...args: any[]) => { - const ctx = createContext(...args); + const originalUpdateContext = _updateContext || withParams(); + const updateContext = (self: any, args: any[], context: HookContext) => { + const ctx = originalUpdateContext(self, args, context); ctx.method = method; return ctx; }; - descriptor.value = functionHooks(fn, hooks, methodContext); + descriptor.value = functionHooks(fn, hooks, updateContext); return descriptor; }; diff --git a/packages/hooks/src/function.ts b/packages/hooks/src/function.ts index 8983bcb..2c030bb 100644 --- a/packages/hooks/src/function.ts +++ b/packages/hooks/src/function.ts @@ -1,80 +1,48 @@ import { compose, Middleware } from './compose'; - -// Typing hacks converting symbols as strings so they can be used as keys -export const HOOKS: string = Symbol('@feathersjs/hooks') as any; -export const ORIGINAL: string = Symbol('@feathersjs/hooks/original') as any; -export const CONTEXT: string = Symbol('@feathersjs/hooks/context') as any; - -export class HookContext { - result?: T; - arguments: any[]; - [key: string]: any; - - toJSON () { - return Object.keys(this).reduce((result, key) => { - return { - ...result, - [key]: this[key] - }; - }, {} as { [key: string]: any }); - } -} - -export type ContextCreator = (...args: any[]) => HookContext; - -export const initContext = (...params: string[]): ContextCreator => () => { - const ctx = new HookContext(); - - if (params.length > 0) { - Object.defineProperty(ctx, 'arguments', { - get (this: HookContext) { - const result = params.map(name => this[name]); - - return typeof Object.freeze === 'function' ? Object.freeze(result) : result; - }, - - set (this: HookContext, value: any[]) { - params.forEach((name, index) => { - this[name] = value[index]; - }); - } - }); - } - - return ctx; -}; - -export const createContext = (fn: any, data: { [key: string]: any } = {}): HookContext => { - const getContext: ContextCreator = fn[CONTEXT]; - - if (typeof getContext !== 'function') { - throw new Error('Can not get context, function is not hook enabled'); - } - - const context = getContext(); - - return Object.assign(context, data); -}; - -export const functionHooks = (method: any, _hooks: Array>, defaultContext: ContextCreator = initContext()) => { - if (typeof method !== 'function') { +import { + HookContext, ContextUpdater, withParams, + registerMiddleware, getMiddleware +} from './base'; + +/** + * Returns a new function that is wrapped in the given hooks. + * Allows to pass a context updater function, usually used + * with `withParams` to initialize named parameters. If not passed + * just set `context.arguments` to the function call arguments + * and `context.self` to the function call `this` reference. + * + * @param fn The function to wrap + * @param hooks The list of hooks (middleware) + * @param updateContext A ContextUpdate method + */ +export const functionHooks = ( + fn: any, + hooks: Array>, + updateContext: ContextUpdater = withParams() +) => { + if (typeof fn !== 'function') { throw new Error('Can not apply hooks to non-function'); } - const hooks = (method[HOOKS] || []).concat(_hooks); - const original = method[ORIGINAL] || method; - const fn = function (this: any, ...args: any[]) { + const result = registerMiddleware(function (this: any, ...args: any[]) { + // If we got passed an existing HookContext instance, we want to return it as well const returnContext = args[args.length - 1] instanceof HookContext; - const context: HookContext = returnContext ? args.pop() : defaultContext.call(this, args); + // Initialize the context. Either the default context or the one that was passed + const baseContext: HookContext = returnContext ? args.pop() : new HookContext(); + // Initialize the context with the self reference and arguments + const context = updateContext(this, args, baseContext); + // Assemble the hook chain const hookChain: Middleware[] = [ // Return `ctx.result` or the context (ctx, next) => next().then(() => returnContext ? ctx : ctx.result), + // The hooks attached to the `this` object + ...getMiddleware(this), // The hook chain attached to this function - ...(fn as any)[HOOKS], + ...getMiddleware(result), // Runs the actual original method if `ctx.result` is not set (ctx, next) => { if (ctx.result === undefined) { - return Promise.resolve(original.apply(this, ctx.arguments)).then(result => { + return Promise.resolve(fn.apply(this, ctx.arguments)).then(result => { ctx.result = result; return next(); @@ -85,16 +53,8 @@ export const functionHooks = (method: any, _hooks: Array> } ]; - context.arguments = args; - return compose(hookChain).call(this, context); - }; - - Object.assign(fn, { - [CONTEXT]: defaultContext, - [HOOKS]: hooks, - [ORIGINAL]: original - }); + }, hooks); - return fn; + return Object.assign(result, { original: fn }); }; diff --git a/packages/hooks/src/index.ts b/packages/hooks/src/index.ts index b6d0a78..41ce063 100644 --- a/packages/hooks/src/index.ts +++ b/packages/hooks/src/index.ts @@ -1,13 +1,27 @@ -import { functionHooks, ContextCreator } from './function'; -import { objectHooks, MiddlewareMap, ContextCreatorMap } from './object'; +import { functionHooks } from './function'; import { Middleware } from './compose'; +import { ContextUpdater } from './base'; +import { objectHooks, MiddlewareMap, ContextUpdaterMap } from './object'; +import { hookDecorator } from './decorator'; export * from './function'; export * from './compose'; -export * from './object'; +export * from './base'; -export function hooks (method: F, _hooks: Array>, defaultContext?: ContextCreator): F; -export function hooks (obj: O, hookMap: MiddlewareMap, contextMap?: ContextCreatorMap): O; +// hooks(fn, hooks, updateContext?) +export function hooks ( + fn: F, + hooks: Array>, + updateContext?: ContextUpdater +): F&((...rest: any[]) => Promise); +// hooks(object, methodHookMap, methodUpdateContextMap?) +export function hooks (obj: T, hookMap: MiddlewareMap, contextMap?: ContextUpdaterMap): T; +// @hooks(hooks) +export function hooks ( + hooks: Array>, + updateContext?: ContextUpdater +): any; +// Fallthrough to actual implementation export function hooks (...args: any[]) { const [ target, _hooks, ...rest ] = args; @@ -15,5 +29,9 @@ export function hooks (...args: any[]) { return functionHooks(target, _hooks, ...rest); } + if (Array.isArray(target)) { + return hookDecorator(target, _hooks); + } + return objectHooks(target, _hooks, ...rest); } diff --git a/packages/hooks/src/object.ts b/packages/hooks/src/object.ts index 71d17a0..0a2a285 100644 --- a/packages/hooks/src/object.ts +++ b/packages/hooks/src/object.ts @@ -1,23 +1,24 @@ import { Middleware } from './compose'; -import { ContextCreator, functionHooks, initContext } from './function'; +import { functionHooks } from './function'; +import { ContextUpdater, HookContext, withParams } from './base'; export interface MiddlewareMap { [key: string]: Middleware[]; } -export interface ContextCreatorMap { - [key: string]: ContextCreator; +export interface ContextUpdaterMap { + [key: string]: ContextUpdater; } -export const objectHooks = (_obj: any, hookMap: MiddlewareMap, contextMap?: ContextCreatorMap) => { +export const objectHooks = (_obj: any, hookMap: MiddlewareMap, contextMap?: ContextUpdaterMap) => { const obj = typeof _obj === 'function' ? _obj.prototype : _obj; return Object.keys(hookMap).reduce((result, method) => { const value = obj[method]; const hooks = hookMap[method]; - const createContext = (contextMap && contextMap[method]) || initContext(); - const methodContext = (...args: any[]) => { - const ctx = createContext(...args); + const originalUpdateContext = (contextMap && contextMap[method]) || withParams(); + const updateContext = (self: any, args: any[], context: HookContext) => { + const ctx = originalUpdateContext(self, args, context); ctx.method = method; @@ -28,7 +29,7 @@ export const objectHooks = (_obj: any, hookMap: MiddlewareMap, contextMap?: Cont throw new Error(`Can not apply hooks. '${method}' is not a function`); } - const fn = functionHooks(value, hooks, methodContext); + const fn = functionHooks(value, hooks, updateContext); result[method] = fn; diff --git a/packages/hooks/test/decorator.test.ts b/packages/hooks/test/decorator.test.ts index 868b417..9a27a59 100644 --- a/packages/hooks/test/decorator.test.ts +++ b/packages/hooks/test/decorator.test.ts @@ -1,30 +1,63 @@ import { strict as assert } from 'assert'; -import { hookDecorator } from '../src/decorator'; -import { HookContext } from '../src/function'; -import { NextFunction } from '../src/compose'; - -describe('objectHooks', () => { - it('hooking object on class adds to the prototype', async () => { - const verify = async (ctx: HookContext, next: NextFunction) => { - assert.deepStrictEqual(ctx.toJSON(), { - method: 'sayHi', - arguments: [ 'David' ] - }); +import { hooks, HookContext, withParams, NextFunction } from '../src'; + +describe('hookDecorator', () => { + it('hook decorator on method and classes with inheritance', async () => { + const expectedName = 'David NameFromTopLevel NameFromDummyClass'; + + @hooks([async (ctx, next) => { + ctx.name += ' NameFromTopLevel'; + + await next(); + + ctx.result += ' ResultFromTopLevel'; + }]) + class TopLevel {} + + @hooks([async (ctx, next) => { + ctx.name += ' NameFromDummyClass'; await next(); - ctx.result += '?'; - }; + ctx.result += ' ResultFromDummyClass'; + }]) + class DummyClass extends TopLevel { + @hooks([async (ctx: HookContext, next: NextFunction) => { + assert.deepStrictEqual(ctx, new HookContext({ + method: 'sayHi', + self: instance, + name: expectedName + })); - class DummyClass { - @hookDecorator([ verify ]) + await next(); + + ctx.result += ' ResultFromMethodDecorator'; + }], withParams('name')) async sayHi (name: string) { return `Hi ${name}`; } + + @hooks([async (_ctx: HookContext, next: NextFunction) => next()]) + async sayWorld () { + return 'World'; + } } const instance = new DummyClass(); - assert.strictEqual(await instance.sayHi('David'), 'Hi David?'); + assert.strictEqual(await instance.sayHi('David'), + `Hi ${expectedName} ResultFromMethodDecorator ResultFromDummyClass ResultFromTopLevel` + ); + }); + + it('error cases', () => { + assert.throws(() => hooks([], withParams('jkfds'))({}), { + message: 'Context can not be updated at the class decorator level. Remove updateContext parameter.' + }); + assert.throws(() => hooks([], withParams('jkfds'))({}, 'test', { + value: 'not a function' + }), { + message: `Can not apply hooks. 'test' is not a function` + }); }); }); diff --git a/packages/hooks/test/function.test.ts b/packages/hooks/test/function.test.ts index 0147b8f..e34b880 100644 --- a/packages/hooks/test/function.test.ts +++ b/packages/hooks/test/function.test.ts @@ -1,8 +1,7 @@ import { strict as assert } from 'assert'; import { - hooks, ORIGINAL, HOOKS, CONTEXT, - HookContext, initContext, createContext, - functionHooks, NextFunction + hooks, HookContext, functionHooks, + NextFunction, getMiddleware, withParams, registerMiddleware } from '../src/'; describe('functionHooks', () => { @@ -19,13 +18,11 @@ describe('functionHooks', () => { } }); - it('returns a new function, sets ORIGINAL, HOOKS and CONTEXT', () => { + it('returns a new function, registers hooks', () => { const fn = hooks(hello, []) as any; assert.notDeepEqual(fn, hello); - assert.deepStrictEqual(fn[HOOKS], []); - assert.deepEqual(fn[ORIGINAL], hello); - assert.ok(fn[CONTEXT]); + assert.deepEqual(getMiddleware(fn), []); }); it('can override arguments, has context', async () => { @@ -72,9 +69,10 @@ describe('functionHooks', () => { assert.strictEqual(res, 'Hello There You!'); }); - it('maintains the function context', async () => { - const hook = async function (this: any, _ctx: HookContext, next: NextFunction) { + it('maintains the function context and sets context.self', async () => { + const hook = async function (this: any, context: HookContext, next: NextFunction) { assert.strictEqual(obj, this); + assert.strictEqual(context.self, obj); await next(); }; const obj = { @@ -89,7 +87,36 @@ describe('functionHooks', () => { assert.strictEqual(res, 'Hi Dave'); }); - it('adds additional hooks to an existing function, uses original', async () => { + it('uses hooks from context object and its prototypes', async () => { + const o1 = { message: 'Hi' }; + const o2 = Object.create(o1); + + registerMiddleware(o1, [async (ctx, next) => { + ctx.arguments[0] += ' o1'; + + await next(); + }]); + + registerMiddleware(o2, [async (ctx, next) => { + ctx.arguments[0] += ' o2'; + + await next(); + }]); + + o2.sayHi = hooks(async function (this: any, name: string) { + return `${this.message} ${name}`; + }, [async (ctx, next) => { + ctx.arguments[0] += ' fn'; + + await next(); + }]); + + const res = await o2.sayHi('Dave'); + + assert.strictEqual(res, 'Hi Dave o1 o2 fn'); + }); + + it('wraps an existing hooked function properly', async () => { const first = hooks(hello, [ async (ctx, next) => { await next(); @@ -105,13 +132,9 @@ describe('functionHooks', () => { } ]); - assert.deepEqual((first as any)[ORIGINAL], hello); - assert.deepEqual((second as any)[ORIGINAL], hello); - assert.strictEqual((second as any)[HOOKS].length, 2); - const result = await second('Dave'); - assert.strictEqual(result, 'Hello Dave Second First'); + assert.strictEqual(result, 'Hello Dave First Second'); }); it('creates context with params and converts to arguments', async () => { @@ -123,7 +146,7 @@ describe('functionHooks', () => { await next(); } - ], initContext('name')); + ], withParams('name')); assert.equal(await fn('Dave'), 'Hello Changed'); }); @@ -135,35 +158,14 @@ describe('functionHooks', () => { await next(); }; - const fn = hooks(hello, [ modifyArgs ], initContext('name')); + const fn = hooks(hello, [ modifyArgs ], withParams('name')); await assert.rejects(() => fn('There'), { message: `Cannot assign to read only property '0' of object '[object Array]'` }); }); - it('uses a custom initContext', async () => { - const checkContext = async (ctx: HookContext, next: NextFunction) => { - assert.deepEqual(ctx.toJSON(), { - arguments: [ 'There' ], - test: 'me' - }); - await next(); - }; - - const fn = hooks(hello, [ checkContext ], () => { - const ctx = new HookContext(); - - ctx.test = 'me'; - - return ctx; - }); - const res = await fn('There'); - - assert.strictEqual(res, 'Hello There'); - }); - - it('can create and return an existing context', async () => { + it('can take and return an existing HookContext', async () => { const message = 'Custom message'; const fn = hooks(hello, [ async (ctx, next) => { @@ -173,17 +175,16 @@ describe('functionHooks', () => { ctx.name = 'Changed'; await next(); } - ], initContext('name')); + ], withParams('name')); - const customContext = createContext(fn, { message }); - // @ts-ignore - const resultContext = await fn('Dave', customContext); + const customContext = new HookContext({ message }); + const resultContext: HookContext = await fn('Dave', customContext); assert.equal(resultContext, customContext); - assert.deepEqual(resultContext.toJSON(), { + assert.deepEqual(resultContext, new HookContext({ message: 'Custom message', name: 'Changed', result: 'Hello Changed' - }); + })); }); }); diff --git a/packages/hooks/test/object.test.ts b/packages/hooks/test/object.test.ts index f483746..8004f70 100644 --- a/packages/hooks/test/object.test.ts +++ b/packages/hooks/test/object.test.ts @@ -1,5 +1,5 @@ import { strict as assert } from 'assert'; -import { hooks, HookContext, initContext, NextFunction } from '../src'; +import { hooks, HookContext, NextFunction, withParams } from '../src'; describe('objectHooks', () => { let obj: any; @@ -32,10 +32,11 @@ describe('objectHooks', () => { it('hooks object with hook methods, sets method name', async () => { const hookedObj = hooks(obj, { sayHi: [async (ctx: HookContext, next: NextFunction) => { - assert.deepStrictEqual(ctx.toJSON(), { + assert.deepEqual(ctx, new HookContext({ + self: obj, method: 'sayHi', arguments: [ 'David' ] - }); + })); await next(); @@ -56,10 +57,11 @@ describe('objectHooks', () => { it('hooks object and allows to customize context for method', async () => { const hookedObj = hooks(obj, { sayHi: [async (ctx: HookContext, next: NextFunction) => { - assert.deepStrictEqual(ctx.toJSON(), { + assert.deepStrictEqual(ctx, new HookContext({ method: 'sayHi', - name: 'David' - }); + name: 'David', + self: obj + })); ctx.name = 'Dave'; @@ -73,7 +75,7 @@ describe('objectHooks', () => { await next(); }] }, { - sayHi: initContext('name') + sayHi: withParams('name') }); assert.strictEqual(obj, hookedObj); @@ -81,7 +83,7 @@ describe('objectHooks', () => { assert.strictEqual(await hookedObj.addOne(1), 3); }); - it('hooking multiple times combines hooks for methods', async () => { + it('hooking multiple times works properly', async () => { hooks(obj, { sayHi: [async (ctx: HookContext, next: NextFunction) => { await next(); @@ -98,7 +100,7 @@ describe('objectHooks', () => { }] }); - assert.strictEqual(await obj.sayHi('David'), 'Hi David!?'); + assert.strictEqual(await obj.sayHi('David'), 'Hi David?!'); }); it('throws an error when hooking invalid method', async () => { @@ -117,10 +119,11 @@ describe('objectHooks', () => { it('hooking object on class adds to the prototype', async () => { hooks(DummyClass, { sayHi: [async (ctx: HookContext, next: NextFunction) => { - assert.deepStrictEqual(ctx.toJSON(), { + assert.deepStrictEqual(ctx, new HookContext({ + self: instance, method: 'sayHi', arguments: [ 'David' ] - }); + })); await next(); @@ -139,13 +142,14 @@ describe('objectHooks', () => { assert.strictEqual(await instance.addOne(1), 3); }); - it('chains hooks with extended classes', async () => { + it('works with inheritance', async () => { hooks(DummyClass, { sayHi: [async (ctx: HookContext, next: NextFunction) => { - assert.deepStrictEqual(ctx.toJSON(), { + assert.deepStrictEqual(ctx, new HookContext({ method: 'sayHi', - arguments: [ 'David' ] - }); + arguments: [ 'David' ], + self: instance + })); await next(); @@ -165,6 +169,6 @@ describe('objectHooks', () => { const instance = new OtherDummy(); - assert.strictEqual(await instance.sayHi('David'), 'Hi David!?'); + assert.strictEqual(await instance.sayHi('David'), 'Hi David?!'); }); });