From a5c47562eae8410c82fe2f6308f26f8e78b6a3e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Berthommier?= Date: Mon, 27 Apr 2020 18:54:48 +0200 Subject: [PATCH] feat(core): use @feathers/hooks and add async type (#1929) --- .github/issue_template.md | 2 +- packages/client/webpack.config.js | 5 +- packages/commons/package.json | 3 + packages/commons/src/hooks.ts | 143 ++++- packages/express/test/rest.test.js | 1 + packages/feathers/lib/events.js | 12 +- packages/feathers/lib/hooks/base.js | 8 +- packages/feathers/lib/hooks/index.js | 245 ++++---- packages/feathers/package.json | 1 + packages/feathers/test/application.test.js | 30 + packages/feathers/test/hooks/app.test.js | 29 + packages/feathers/test/hooks/async.test.js | 537 ++++++++++++++++++ packages/feathers/test/hooks/error.test.js | 25 +- packages/feathers/test/hooks/hooks.test.js | 71 +++ .../feathers/test/hooks/with-hooks.test.js | 129 ----- .../transport-commons/src/channels/index.ts | 2 +- 16 files changed, 954 insertions(+), 289 deletions(-) create mode 100644 packages/feathers/test/hooks/async.test.js delete mode 100755 packages/feathers/test/hooks/with-hooks.test.js diff --git a/.github/issue_template.md b/.github/issue_template.md index 57a8f3e0f1..249464ce8b 100644 --- a/.github/issue_template.md +++ b/.github/issue_template.md @@ -26,4 +26,4 @@ Tell us about the applicable parts of your setup. **React Native Version**: -**Module Loader**: \ No newline at end of file +**Module Loader**: diff --git a/packages/client/webpack.config.js b/packages/client/webpack.config.js index 96c35f70d9..c11813ad10 100644 --- a/packages/client/webpack.config.js +++ b/packages/client/webpack.config.js @@ -6,7 +6,10 @@ const UglifyJSPlugin = require('uglifyjs-webpack-plugin'); function createConfig (name, isProduction = false) { const output = name === 'index' ? 'feathers' : name; const commons = { - entry: `./src/${name}.js`, + entry: [ + 'regenerator-runtime/runtime', + `./src/${name}.js` + ], output: { library: 'feathers', libraryTarget: 'umd', diff --git a/packages/commons/package.json b/packages/commons/package.json index 379f59ad55..644ff33a3d 100644 --- a/packages/commons/package.json +++ b/packages/commons/package.json @@ -40,6 +40,9 @@ "publishConfig": { "access": "public" }, + "dependencies": { + "@feathersjs/hooks": "^0.4.0-alpha.0" + }, "devDependencies": { "@types/mocha": "^7.0.2", "@types/node": "^13.11.1", diff --git a/packages/commons/src/hooks.ts b/packages/commons/src/hooks.ts index 9852c01422..7fa9314999 100644 --- a/packages/commons/src/hooks.ts +++ b/packages/commons/src/hooks.ts @@ -1,6 +1,8 @@ +import { HookContext } from '@feathersjs/hooks'; import { createSymbol, _ } from './utils'; -const { each, pick } = _; +const { each, pick, omit } = _; +const noop = () => {}; export const ACTIVATE_HOOKS = createSymbol('__feathersActivateHooks'); @@ -87,9 +89,13 @@ export function convertHookData (obj: any) { // Duck-checks a given object to be a hook object // A valid hook object has `type` and `method` export function isHookObject (hookObject: any) { - return typeof hookObject === 'object' && - typeof hookObject.method === 'string' && - typeof hookObject.type === 'string'; + return ( + hookObject instanceof HookContext || ( + typeof hookObject === 'object' && + typeof hookObject.method === 'string' && + typeof hookObject.type === 'string' + ) + ); } // Returns all service and application hooks combined @@ -193,3 +199,132 @@ export function enableHooks (obj: any, methods: string[], types: string[]) { } }); } + +async function handleError (hook: any, context: any, onError: any) { + try { + const result = await hook.call(context.self, context); + Object.assign(context, omit(result, 'arguments')); + } catch (errorError) { + if (typeof onError === 'function') { + onError(errorError, context); + } + throw errorError; + } + + if (typeof context.error !== 'undefined') { + throw context.error; + } +} + +export function firstHook (context: any, next: any) { + context.type = 'before'; + return next(); +} + +export function lastHook (context: any, next: any) { + context.type = 'after'; + return next(); +} + +export function toBeforeHook (hook: any) { + return async (context: any, next: any) => { + const result = await hook.call(context.self, context); + Object.assign(context, omit(result, 'arguments')); + await next(); + }; +} + +export function toAfterHook (hook: any) { + return async (context: any, next: any) => { + await next(); + const result = await hook.call(context.self, context); + Object.assign(context, omit(result, 'arguments')); + }; +} + +export function toErrorHook (hook: any, onError: any, control: any) { + return async (context: any, next: any) => { + try { + await next(); + } catch (error) { + if (typeof control === 'function') { + control(context); + } + + context.error = error; + context.result = undefined; + + await handleError(hook, context, onError); + } + }; +} + +export function toFinallyHook (hook: any, onError: any, control: any) { + return async (context: any, next: any) => { + try { + await next(); + } catch (error) { + throw error; + } finally { + if (typeof control === 'function') { + control(context); + } + + await handleError(hook, context, onError); + } + }; +} + +export function beforeWrapper (hooks: any) { + return [firstHook, ...[].concat(hooks).map(toBeforeHook)]; +} + +export function afterWrapper (hooks: any) { + return [...[].concat(hooks).reverse().map(toAfterHook), lastHook]; +} + +export function finallyWrapper (hooks: any) { + let errorInFinally: any; + + const onError = (error: any, context: any) => { + errorInFinally = error; + context.error = error; + context.result = undefined; + }; + const control = () => { + if (errorInFinally) { + throw errorInFinally; + } + }; + + return [].concat(hooks).reverse().map(hook => toFinallyHook(hook, onError, control)); +} + +export function errorWrapper (hooks: any) { + let errorInError: any; + + const onError = (error: any, context: any) => { + errorInError = error; + context.error = error; + context.result = undefined; + }; + const control = (context: any) => { + if (!context.original) { + context.original = { ...context }; + } + if (errorInError) { + throw errorInError; + } + context.type = 'error'; + }; + + return [noop].concat(hooks).reverse().map(hook => toErrorHook(hook, onError, control)); +} + +export function wrap ({ async = [], before = [], after = [] }: any = {}) { + return [ + ...[].concat(async), + ...beforeWrapper(before), + ...afterWrapper(after) + ]; +} diff --git a/packages/express/test/rest.test.js b/packages/express/test/rest.test.js index 6db3e6d862..5f236e23fe 100644 --- a/packages/express/test/rest.test.js +++ b/packages/express/test/rest.test.js @@ -118,6 +118,7 @@ describe('@feathersjs/express/rest provider', () => { const convertHook = hook => { const result = Object.assign({}, hook); + delete result.self; delete result.service; delete result.app; delete result.error; diff --git a/packages/feathers/lib/events.js b/packages/feathers/lib/events.js index 96e9e75b3c..9ed3c84d09 100644 --- a/packages/feathers/lib/events.js +++ b/packages/feathers/lib/events.js @@ -4,16 +4,16 @@ const Proto = require('uberproto'); // Returns a hook that emits service events. Should always be // used as the very last hook in the chain const eventHook = exports.eventHook = function eventHook () { - return function (hook) { - const { app, service } = hook; - const eventName = hook.event === null ? hook.event : app.eventMappings[hook.method]; + return function (ctx) { + const { app, service, method, event, type, result } = ctx; + const eventName = event === null ? event : app.eventMappings[method]; const isHookEvent = service._hookEvents && service._hookEvents.indexOf(eventName) !== -1; // If this event is not being sent yet and we are not in an error hook - if (eventName && isHookEvent && hook.type !== 'error') { - const results = Array.isArray(hook.result) ? hook.result : [ hook.result ]; + if (eventName && isHookEvent && type !== 'error') { + const results = Array.isArray(result) ? result : [ result ]; - results.forEach(element => service.emit(eventName, element, hook)); + results.forEach(element => service.emit(eventName, element, ctx)); } }; }; diff --git a/packages/feathers/lib/hooks/base.js b/packages/feathers/lib/hooks/base.js index bde4fea210..15be029670 100644 --- a/packages/feathers/lib/hooks/base.js +++ b/packages/feathers/lib/hooks/base.js @@ -1,6 +1,6 @@ const { _ } = require('@feathersjs/commons'); -const assignArguments = context => { +const assignArguments = (context, next) => { const { service, method } = context; const parameters = service.methods[method]; @@ -12,10 +12,10 @@ const assignArguments = context => { context.params = {}; } - return context; + return next(); }; -const validate = context => { +const validate = (context, next) => { const { service, method, path } = context; const parameters = service.methods[method]; @@ -27,7 +27,7 @@ const validate = context => { throw new Error(`A data object must be provided to the '${path}.${method}' method`); } - return context; + return next(); }; module.exports = [ assignArguments, validate ]; diff --git a/packages/feathers/lib/hooks/index.js b/packages/feathers/lib/hooks/index.js index e1db2eb6f6..5f2ebf574e 100644 --- a/packages/feathers/lib/hooks/index.js +++ b/packages/feathers/lib/hooks/index.js @@ -1,132 +1,110 @@ -const { hooks, isPromise } = require('@feathersjs/commons'); +const { hooks: hookCommons } = require('@feathersjs/commons'); +const { + hooks: hooksDecorator, + HookContext, + getMiddleware, + withParams, + withProps +} = require('@feathersjs/hooks'); const baseHooks = require('./base'); const { - createHookObject, getHooks, - processHooks, enableHooks, - ACTIVATE_HOOKS -} = hooks; + ACTIVATE_HOOKS, + finallyWrapper, + errorWrapper, + wrap +} = hookCommons; + +function getContextUpdaters (app, service, method) { + const parameters = service.methods[method].map(v => (v === 'params' ? ['params', {}] : v)); + + return [ + withParams(...parameters), + withProps({ + app, + service, + type: 'before', + get path () { + if (!service || !app || !app.services) { + return null; + } + + return Object.keys(app.services) + .find(path => app.services[path] === service); + } + }) + ]; +} + +function getCollector (app, service, method) { + return (self, fn, args) => { + const middleware = [ + ...getMiddleware(self), + ...(fn && typeof fn.collect === 'function' ? fn.collect(fn, fn.original, args) : []) + ]; + + if (typeof self === 'object') { + return middleware; + } -const withHooks = function withHooks ({ - app, - service, - method, - original -}) { - return (_hooks = {}) => { - const hooks = app.hookTypes.reduce((result, type) => { - const value = _hooks[type] || []; + const hooks = { + async: getHooks(app, service, 'async', method), + before: getHooks(app, service, 'before', method), + after: getHooks(app, service, 'after', method, true), + error: getHooks(app, service, 'error', method, true), + finally: getHooks(app, service, 'finally', method, true) + }; - result[type] = Array.isArray(value) ? value : [ value ]; + return [ + ...finallyWrapper(hooks.finally), + ...errorWrapper(hooks.error), + ...baseHooks, + ...middleware, + ...wrap(hooks) + ]; + }; +} - return result; - }, {}); - - return function (...args) { - const returnHook = args[args.length - 1] === true - ? args.pop() : false; - - // Create the hook object that gets passed through - const hookObject = createHookObject(method, { - type: 'before', // initial hook object type - arguments: args, - service, - app - }); - - return Promise.resolve(hookObject) - - // Run `before` hooks - .then(hookObject => { - return processHooks.call(service, baseHooks.concat(hooks.before), hookObject); - }) - - // Run the original method - .then(hookObject => { - // If `hookObject.result` is set, skip the original method - if (typeof hookObject.result !== 'undefined') { - return hookObject; - } - - // Otherwise, call it with arguments created from the hook object - const promise = new Promise(resolve => { - const func = original || service[method]; - const args = service.methods[method].map((value) => hookObject[value]); - const result = func.apply(service, args); - - if (!isPromise(result)) { - throw new Error(`Service method '${hookObject.method}' for '${hookObject.path}' service must return a promise`); - } - - resolve(result); - }); - - return promise - .then(result => { - hookObject.result = result; - return hookObject; - }) - .catch(error => { - error.hook = hookObject; - throw error; - }); - }) - - // Run `after` hooks - .then(hookObject => { - const afterHookObject = Object.assign({}, hookObject, { - type: 'after' - }); - - return processHooks.call(service, hooks.after, afterHookObject); - }) - - // Run `errors` hooks - .catch(error => { - const errorHookObject = Object.assign({}, error.hook, { - type: 'error', - original: error.hook, - error, - result: undefined - }); - - return processHooks.call(service, hooks.error, errorHookObject) - .catch(error => { - const errorHookObject = Object.assign({}, error.hook, { - error, - result: undefined - }); - - return errorHookObject; - }); - }) - - // Run `finally` hooks - .then(hookObject => { - return processHooks.call(service, hooks.finally, hookObject) - .catch(error => { - const errorHookObject = Object.assign({}, error.hook, { - error, - result: undefined - }); - - return errorHookObject; - }); - }) - - // Resolve with a result or reject with an error - .then(hookObject => { - if (typeof hookObject.error !== 'undefined' && typeof hookObject.result === 'undefined') { - return Promise.reject(returnHook ? hookObject : hookObject.error); - } else { - return returnHook ? hookObject : hookObject.result; - } - }); +function withHooks (app, service, methods) { + const hookMap = methods.reduce((accu, method) => { + if (typeof service[method] !== 'function') { + return accu; + } + + accu[method] = { + middleware: [], + context: getContextUpdaters(app, service, method), + collect: getCollector(app, service, method) }; - }; -}; + + return accu; + }, {}); + + hooksDecorator(service, hookMap); +} + +function mixinMethod () { + const service = this; + const args = Array.from(arguments); + + const returnHook = args[args.length - 1] === true || args[args.length - 1] instanceof HookContext + ? args.pop() : false; + + const hookContext = returnHook instanceof HookContext ? returnHook : new HookContext(); + + return this._super.call(service, ...args, hookContext) + .then(() => returnHook ? hookContext : hookContext.result) + // Handle errors + .catch(() => { + if (typeof hookContext.error !== 'undefined' && typeof hookContext.result === 'undefined') { + return Promise.reject(returnHook ? hookContext : hookContext.error); + } else { + return returnHook ? hookContext : hookContext.result; + } + }); +} // A service mixin that adds `service.hooks()` method and functionality const hookMixin = exports.hookMixin = function hookMixin (service) { @@ -152,29 +130,16 @@ const hookMixin = exports.hookMixin = function hookMixin (service) { const app = this; const methodNames = Object.keys(service.methods); - // Assemble the mixin object that contains all "hooked" service methods + + withHooks(app, service, methodNames); + + // Usefull only for the `returnHook` backwards compatibility with `true` const mixin = methodNames.reduce((mixin, method) => { if (typeof service[method] !== 'function') { return mixin; } - mixin[method] = function () { - const service = this; - const args = Array.from(arguments); - const original = service._super.bind(service); - - return withHooks({ - app, - service, - method, - original - })({ - before: getHooks(app, service, 'before', method), - after: getHooks(app, service, 'after', method, true), - error: getHooks(app, service, 'error', method, true), - finally: getHooks(app, service, 'finally', method, true) - })(...args); - }; + mixin[method] = mixinMethod; return mixin; }, {}); @@ -190,7 +155,7 @@ module.exports = function () { // We store a reference of all supported hook types on the app // in case someone needs it Object.assign(app, { - hookTypes: ['before', 'after', 'error', 'finally'] + hookTypes: ['async', 'before', 'after', 'error', 'finally'] }); // Add functionality for hooks to be registered as app.hooks @@ -200,8 +165,6 @@ module.exports = function () { }; }; -module.exports.withHooks = withHooks; - module.exports.ACTIVATE_HOOKS = ACTIVATE_HOOKS; module.exports.activateHooks = function activateHooks (args) { diff --git a/packages/feathers/package.json b/packages/feathers/package.json index 90531e02fe..a2531e23dd 100644 --- a/packages/feathers/package.json +++ b/packages/feathers/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "@feathersjs/commons": "^4.5.2", + "@feathersjs/hooks": "^0.4.0-alpha.0", "debug": "^4.1.1", "events": "^3.1.0", "uberproto": "^2.0.6" diff --git a/packages/feathers/test/application.test.js b/packages/feathers/test/application.test.js index 36588863e1..d747c8f84a 100644 --- a/packages/feathers/test/application.test.js +++ b/packages/feathers/test/application.test.js @@ -168,6 +168,36 @@ describe('Feathers application', () => { app1.service('testing').create({ message: 'Hi' }); }); + it('async hooks', done => { + const app = feathers(); + + app.use('/dummy', { + create (data) { + return Promise.resolve(data); + } + }); + + const dummy = app.service('dummy'); + + dummy.hooks({ + async: async (ctx, next) => { + await next(); + ctx.params.fromAsyncHook = true; + }, + before: { + create (hook) { + hook.params.fromAsyncHook = false; + } + } + }); + + dummy.create({ message: 'Hi' }, {}, true) + .then(ctx => { + assert.ok(ctx.params.fromAsyncHook); + }) + .then(done, done); + }); + it('services conserve Symbols', () => { const TEST = Symbol('test'); const dummyService = { diff --git a/packages/feathers/test/hooks/app.test.js b/packages/feathers/test/hooks/app.test.js index bf9dd9a92b..2ba95ff9ed 100644 --- a/packages/feathers/test/hooks/app.test.js +++ b/packages/feathers/test/hooks/app.test.js @@ -25,6 +25,35 @@ describe('app.hooks', () => { assert.strictEqual(typeof app.hooks, 'function'); }); + describe('app.hooks({ async })', () => { + it('basic app async hook', () => { + const service = app.service('todos'); + + app.hooks({ + async async (hook, next) { + assert.strictEqual(hook.app, app); + await next(); + hook.params.ran = true; + } + }); + + return service.get('test').then(result => { + assert.deepStrictEqual(result, { + id: 'test', + params: { ran: true } + }); + + const data = { test: 'hi' }; + + return service.create(data).then(result => { + assert.deepStrictEqual(result, { + data, params: { ran: true } + }); + }); + }); + }); + }); + describe('app.hooks({ before })', () => { it('basic app before hook', () => { const service = app.service('todos'); diff --git a/packages/feathers/test/hooks/async.test.js b/packages/feathers/test/hooks/async.test.js new file mode 100644 index 0000000000..c851d022a9 --- /dev/null +++ b/packages/feathers/test/hooks/async.test.js @@ -0,0 +1,537 @@ +const assert = require('assert'); +const feathers = require('../../lib'); + +describe('`async` hooks', () => { + describe('function([hook])', () => { + it('returning a non-hook object throws error', () => { + const app = feathers().use('/dummy', { + get (id) { + return Promise.resolve({ id }); + } + }); + const service = app.service('dummy'); + + service.hooks({ + async: { + get () { + return {}; + } + } + }); + + return service.get(10).catch(e => { + assert.strictEqual(e.message, 'async hook for \'get\' method returned invalid hook object'); + }); + }); + + it('hooks in chain can be replaced', () => { + const app = feathers().use('/dummy', { + get (id) { + return Promise.resolve({ + id, description: `You have to do ${id}` + }); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get: [ + function (hook) { + return Object.assign({}, hook, { + modified: true + }); + }, + function (hook) { + assert.ok(hook.modified); + } + ] + } + }); + + return service.get('laundry'); + }); + + it('.async hooks can return a promise', () => { + const app = feathers().use('/dummy', { + get (id, params) { + assert.ok(params.ran, 'Ran through promise hook'); + + return Promise.resolve({ + id: id, + description: `You have to do ${id}` + }); + }, + + remove () { + assert.ok(false, 'Should never get here'); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get (hook) { + return new Promise(resolve => { + hook.params.ran = true; + resolve(); + }); + }, + + remove () { + return new Promise((resolve, reject) => { + reject(new Error('This did not work')); + }); + } + } + }); + + return service.get('dishes').then(() => service.remove(10)) + .catch(error => { + assert.strictEqual(error.message, 'This did not work'); + }); + }); + + it('.async hooks do not need to return anything', () => { + const app = feathers().use('/dummy', { + get (id, params) { + assert.ok(params.ran, 'Ran through promise hook'); + + return Promise.resolve({ + id: id, + description: `You have to do ${id}` + }); + }, + + remove () { + assert.ok(false, 'Should never get here'); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get (hook) { + hook.params.ran = true; + }, + + remove () { + throw new Error('This did not work'); + } + } + }); + + return service.get('dishes').then(() => service.remove(10)) + .catch(error => { + assert.strictEqual(error.message, 'This did not work'); + }); + }); + + it('.async hooks can set hook.result which will skip service method', () => { + const app = feathers().use('/dummy', { + get (id) { + assert.ok(false, 'This should never run'); + return Promise.resolve({ id }); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + async get (hook, next) { + hook.result = { + id: hook.id, + message: 'Set from hook' + }; + + await next(); + } + } + }); + + return service.get(10, {}).then(data => { + assert.deepStrictEqual(data, { + id: 10, + message: 'Set from hook' + }); + }); + }); + }); + + describe('function(hook, next)', () => { + it('gets mixed into a service and modifies data', () => { + const dummyService = { + create (data, params) { + assert.deepStrictEqual(data, { + some: 'thing', + modified: 'data' + }, 'Data modified'); + + assert.deepStrictEqual(params, { + modified: 'params' + }, 'Params modified'); + + return Promise.resolve(data); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + async: { + create (hook, next) { + assert.strictEqual(hook.type, 'before'); + + hook.data.modified = 'data'; + + Object.assign(hook.params, { + modified: 'params' + }); + + return next(); + } + } + }); + + return service.create({ some: 'thing' }).then(data => { + assert.deepStrictEqual(data, { + some: 'thing', + modified: 'data' + }, 'Data got modified'); + }); + }); + + it('contains the app object at hook.app', () => { + const someServiceConfig = { + create (data, params, callback) { + return Promise.resolve(data); + } + }; + + const app = feathers().use('/some-service', someServiceConfig); + const someService = app.service('some-service'); + + someService.hooks({ + async: { + create (hook, next) { + hook.data.appPresent = typeof hook.app !== 'undefined'; + assert.strictEqual(hook.data.appPresent, true); + return next(); + } + } + }); + + return someService.create({ some: 'thing' }).then(data => { + assert.deepStrictEqual(data, { + some: 'thing', + appPresent: true + }, 'App object was present'); + }); + }); + + it('passes errors', () => { + const dummyService = { + update () { + assert.ok(false, 'Never should be called'); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + async: { + update () { + throw new Error('You are not allowed to update'); + } + } + }); + + return service.update(1, {}).catch(error => { + assert.ok(error, 'Got an error'); + assert.strictEqual(error.message, 'You are not allowed to update', 'Got error message'); + }); + }); + + it('does not run after hook when there is an error', () => { + const dummyService = { + remove () { + return Promise.reject(new Error('Error removing item')); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + after: { + async remove (context, next) { + await next(); + + assert.ok(false, 'This should never get called'); + } + } + }); + + return service.remove(1, {}).catch(error => { + assert.ok(error, 'Got error'); + assert.strictEqual(error.message, 'Error removing item', 'Got error message from service'); + }); + }); + + it('calling back with no arguments uses the old ones', () => { + const dummyService = { + remove (id, params) { + assert.strictEqual(id, 1, 'Got id'); + assert.deepStrictEqual(params, { my: 'param' }); + + return Promise.resolve({ id }); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + async: { + remove (hook, next) { + next(); + } + } + }); + + return service.remove(1, { my: 'param' }); + }); + + it('adds .hooks() and chains multiple hooks for the same method', () => { + const dummyService = { + create (data, params) { + assert.deepStrictEqual(data, { + some: 'thing', + modified: 'second data' + }, 'Data modified'); + + assert.deepStrictEqual(params, { + modified: 'params' + }, 'Params modified'); + + return Promise.resolve(data); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + async: { + create (hook, next) { + hook.params.modified = 'params'; + + next(); + } + } + }); + + service.hooks({ + async: { + create (hook, next) { + hook.data.modified = 'second data'; + + next(); + } + } + }); + + return service.create({ some: 'thing' }); + }); + + it('chains multiple async hooks using array syntax', () => { + const dummyService = { + create (data, params) { + assert.deepStrictEqual(data, { + some: 'thing', + modified: 'second data' + }, 'Data modified'); + + assert.deepStrictEqual(params, { + modified: 'params' + }, 'Params modified'); + + return Promise.resolve(data); + } + }; + + const app = feathers().use('/dummy', dummyService); + const service = app.service('dummy'); + + service.hooks({ + async: { + create: [ + function (hook, next) { + hook.params.modified = 'params'; + + next(); + }, + function (hook, next) { + hook.data.modified = 'second data'; + + next(); + } + ] + } + }); + + return service.create({ some: 'thing' }); + }); + + it('.async hooks run in the correct order (#13)', () => { + const app = feathers().use('/dummy', { + get (id, params, callback) { + assert.deepStrictEqual(params.items, ['first', 'second', 'third']); + + return Promise.resolve({ + id: id, + items: [] + }); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get (hook, next) { + hook.params.items = ['first']; + next(); + } + } + }); + + service.hooks({ + async: { + get: [ + function (hook, next) { + hook.params.items.push('second'); + next(); + }, + function (hook, next) { + hook.params.items.push('third'); + next(); + } + ] + } + }); + + return service.get(10); + }); + + it('async all hooks (#11)', () => { + const app = feathers().use('/dummy', { + get (id, params) { + assert.ok(params.asyncAllObject); + assert.ok(params.asyncAllMethodArray); + + return Promise.resolve({ + id: id, + items: [] + }); + }, + + find (params) { + assert.ok(params.asyncAllObject); + assert.ok(params.asyncAllMethodArray); + + return Promise.resolve([]); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + all (hook, next) { + hook.params.asyncAllObject = true; + next(); + } + } + }); + + service.hooks({ + async: [ + function (hook, next) { + hook.params.asyncAllMethodArray = true; + next(); + } + ] + }); + + return service.find(); + }); + + it('async hooks have service as context and keep it in service method (#17)', () => { + const app = feathers().use('/dummy', { + number: 42, + get (id, params) { + return Promise.resolve({ + id: id, + number: this.number, + test: params.test + }); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get (hook, next) { + hook.params.test = this.number + 2; + return next(); + } + } + }); + + return service.get(10).then(data => { + assert.deepStrictEqual(data, { + id: 10, + number: 42, + test: 44 + }); + }); + }); + + it('calling next() multiple times does not do anything', () => { + const app = feathers().use('/dummy', { + get (id) { + return Promise.resolve({ id }); + } + }); + + const service = app.service('dummy'); + + service.hooks({ + async: { + get: [ + function (hook, next) { + return next(); + }, + + function (hook, next) { + next(); + return next(); + } + ] + } + }); + + return service.get(10).catch(e => { + assert.strictEqual(e.message, 'next() called multiple times'); + }); + }); + }); +}); diff --git a/packages/feathers/test/hooks/error.test.js b/packages/feathers/test/hooks/error.test.js index a9539fcf8b..b7afadf168 100644 --- a/packages/feathers/test/hooks/error.test.js +++ b/packages/feathers/test/hooks/error.test.js @@ -205,7 +205,7 @@ describe('`error` hooks', () => { } }).hooks({ error (hook) { - assert.strictEqual(hook.error.hook.type, 'before', + assert.strictEqual(hook.original.type, 'before', 'Original hook still set' ); assert.strictEqual(hook.id, 'dishes'); @@ -227,7 +227,7 @@ describe('`error` hooks', () => { }, error (hook) { - assert.strictEqual(hook.error.hook.type, 'after', + assert.strictEqual(hook.original.type, 'after', 'Original hook still set' ); assert.strictEqual(hook.id, 'dishes'); @@ -266,6 +266,27 @@ describe('`error` hooks', () => { }) .catch(error => assert.strictEqual(error.message, errorMessage)); }); + + it('error in async hook', () => { + service.hooks({ + async (hook) { + hook.modified = true; + + throw new Error(errorMessage); + }, + + error (hook) { + assert.ok(hook.modified); + assert.strictEqual(hook.original.type, 'before'); + } + }); + + return service.get('laundry') + .then(() => { + throw new Error('Should never get here'); + }) + .catch(error => assert.strictEqual(error.message, errorMessage)); + }); }); it('Error in before hook causes inter-service calls to have wrong hook context (https://github.com/feathersjs/feathers/issues/841)', () => { diff --git a/packages/feathers/test/hooks/hooks.test.js b/packages/feathers/test/hooks/hooks.test.js index efe184c633..e337e5c919 100644 --- a/packages/feathers/test/hooks/hooks.test.js +++ b/packages/feathers/test/hooks/hooks.test.js @@ -1,7 +1,78 @@ const assert = require('assert'); +const { hooks, HookContext } = require('@feathersjs/hooks'); const feathers = require('../../lib'); describe('hooks basics', () => { + it('mix @feathersjs/hooks and .hooks', async () => { + const svc = { + get (id, params) { + return Promise.resolve({ id, user: params.user }); + } + }; + + hooks(svc, { + get: [async (ctx, next) => { + ctx.chain.push('@hooks 1 before'); + await next(); + ctx.chain.push('@hooks 1 after'); + }] + }); + + const app = feathers().use('/dummy', svc); + const service = app.service('dummy'); + + service.hooks({ + before: { + get: ctx => { + ctx.chain.push('.hooks 1 before'); + } + }, + after: { + get: ctx => { + ctx.chain.push('.hooks 1 after'); + } + } + }); + + hooks(service, { + get: [async (ctx, next) => { + ctx.chain.push('@hooks 2 before'); + await next(); + ctx.chain.push('@hooks 2 after'); + }] + }); + + service.hooks({ + before: { + get: ctx => { + ctx.chain.push('.hooks 2 before'); + } + }, + after: { + get: ctx => { + ctx.chain.push('.hooks 2 after'); + } + } + }); + + const hookContext = new HookContext({ + chain: [] + }); + const resultContext = await service.get(1, {}, hookContext); + + assert.strictEqual(hookContext, resultContext); + assert.deepStrictEqual(resultContext.chain, [ + '@hooks 2 before', + '@hooks 1 before', + '.hooks 1 before', + '.hooks 2 before', + '.hooks 1 after', + '.hooks 2 after', + '@hooks 1 after', + '@hooks 2 after' + ]); + }); + it('validates arguments', () => { const app = feathers().use('/dummy', { get (id, params) { diff --git a/packages/feathers/test/hooks/with-hooks.test.js b/packages/feathers/test/hooks/with-hooks.test.js deleted file mode 100755 index a053709676..0000000000 --- a/packages/feathers/test/hooks/with-hooks.test.js +++ /dev/null @@ -1,129 +0,0 @@ - -const assert = require('assert'); -const feathers = require('../../lib'); -const { withHooks } = require('../../lib/hooks'); - -function createApp (findResult) { - return feathers() - .use('svc', { - create (data) { - return Promise.resolve(data); - }, - find () { - return Promise.resolve(findResult); - } - }); -} - -const testHook = hook => { - hook._called = 'called'; - return hook; -}; - -describe('services withHooks', () => { - it('get expected hook & object result', () => { - const data = { name: 'john' }; - const params = {}; - - const app = createApp(); - const svc = app.service('svc'); - - return withHooks({ - app, - service: svc, - method: 'create' - })({ - before: testHook - })(data, params, true) - .then(hook => { - assert.deepStrictEqual(hook.result, data, 'test result'); - assert.deepStrictEqual(hook, { - app, - params, - service: svc, - method: 'create', - path: 'svc', - data, - _called: 'called', - result: data, - type: 'after', - arguments: [ data, params ] - }, 'test hook'); - }); - }); - - it('get expected array result', () => { - const data = [{ name: 'john' }]; - - const app = createApp(); - const svc = app.service('svc'); - - return withHooks({ - app, - service: svc, - method: 'create' - })({ - before: testHook - })(data) - .then(result => { - assert.deepStrictEqual(result, data, 'test result'); - }); - }); - - it('get expected find result', () => { - const data = { total: 1, data: [{ name: 'john' }] }; - - const app = createApp(data); - const svc = app.service('svc'); - - return withHooks({ - app, - service: svc, - method: 'find' - })({ - before: testHook - })() - .then(result => { - assert.deepStrictEqual(result, data, 'test result'); - }); - }); - - it('get expected find result with no hooks at all', () => { - const data = { total: 1, data: [{ name: 'john' }] }; - - const app = createApp(data); - const svc = app.service('svc'); - - return withHooks({ - app, - service: svc, - method: 'find' - })()().then(result => { - assert.deepStrictEqual(result, data, 'test result'); - }); - }); - - it('test using keep hook', () => { - const data = [{ name: 'John', job: 'dev', address: { city: 'Montreal', postal: 'H4T 2A1' } }]; - - const app = createApp(); - const svc = app.service('svc'); - - return withHooks({ - app, - service: svc, - method: 'create' - })({ - after: context => { - (Array.isArray(context.result) ? context.result : [context.result]) - .forEach(value => { - delete value.job; - delete value.address.postal; - }); - } - })(data) - .then(result => { - assert.deepStrictEqual(result, [{ name: 'John', address: { city: 'Montreal' } }]); - }); - }); -}); diff --git a/packages/transport-commons/src/channels/index.ts b/packages/transport-commons/src/channels/index.ts index 2ef89d880c..1fcc5db9ac 100644 --- a/packages/transport-commons/src/channels/index.ts +++ b/packages/transport-commons/src/channels/index.ts @@ -93,7 +93,7 @@ export function channels () { return; } - const results = Array.isArray(result) ? compact(flattenDeep(result)) : [result]; + const results = (Array.isArray(result) ? compact(flattenDeep(result)) : [result] as Channel[]); const channel = new CombinedChannel(results); if (channel && channel.length > 0) {