From 1ea1cad8f517599e9f3dca62beba847d45e626b8 Mon Sep 17 00:00:00 2001 From: Pablo Palacios Date: Tue, 20 Aug 2024 19:34:32 +0200 Subject: [PATCH] feat: add support for async based resource operation handlers (#523) --- README.md | 32 ++-- libs/fetcher.js | 174 ++++++++++++--------- tests/functional/resources/alwaysSlow.js | 16 +- tests/functional/resources/error.js | 30 ++-- tests/functional/resources/headers.js | 20 ++- tests/functional/resources/item.js | 29 ++-- tests/functional/resources/slowThenFast.js | 13 +- tests/functional/resources/wait.js | 3 + tests/mock/MockErrorService.js | 103 ++++-------- tests/mock/MockService.js | 113 +++++-------- 10 files changed, 248 insertions(+), 285 deletions(-) create mode 100644 tests/functional/resources/wait.js diff --git a/README.md b/README.md index 6b410781..330ea9ad 100644 --- a/README.md +++ b/README.md @@ -75,13 +75,13 @@ export default { // resource is required resource: 'data_service', // at least one of the CRUD methods is required - read: function (req, resource, params, config, callback) { - //... + read: async function ({ req, resource, params, config }) { + return { data: 'foo' }; }, // other methods - // create: function(req, resource, params, body, config, callback) {}, - // update: function(req, resource, params, body, config, callback) {}, - // delete: function(req, resource, params, config, callback) {} + // create: async function({ req, resource, params, body, config }) {}, + // update: async function({ req, resource, params, body, config }) {}, + // delete: async function({ req, resource, params, config }) {} }; ``` @@ -173,16 +173,16 @@ property of the resolved value. // dataService.js export default { resource: 'data_service', - read: function (req, resource, params, config, callback) { - // business logic - const data = 'response'; - const meta = { - headers: { - 'cache-control': 'public, max-age=3600', + read: async function ({ req, resource, params, config }) { + return { + data: 'response', // business logic + meta: { + headers: { + 'cache-control': 'public, max-age=3600', + }, + statusCode: 200, // You can even provide a custom statusCode for the fetch response }, - statusCode: 200, // You can even provide a custom statusCode for the fetch response }; - callback(null, data, meta); }, }; ``` @@ -225,17 +225,17 @@ fetcher.updateOptions({ ## Error Handling -When an error occurs in your Fetchr CRUD method, you should return an error object to the callback. The error object should contain a `statusCode` (default 500) and `output` property that contains a JSON serializable object which will be sent to the client. +When an error occurs in your Fetchr CRUD method, you should throw an error object. The error object should contain a `statusCode` (default 500) and `output` property that contains a JSON serializable object which will be sent to the client. ```js export default { resource: 'FooService', - read: function create(req, resource, params, configs, callback) { + read: async function create(req, resource, params, configs) { const err = new Error('it failed'); err.statusCode = 404; err.output = { message: 'Not found', more: 'meta data' }; err.meta = { foo: 'bar' }; - return callback(err); + throw err; }, }; ``` diff --git a/libs/fetcher.js b/libs/fetcher.js index 6ba07288..cb8b37d8 100644 --- a/libs/fetcher.js +++ b/libs/fetcher.js @@ -9,6 +9,8 @@ const OP_READ = 'read'; const OP_CREATE = 'create'; const OP_UPDATE = 'update'; const OP_DELETE = 'delete'; +const OPERATIONS = [OP_READ, OP_CREATE, OP_UPDATE, OP_DELETE]; + const RESOURCE_SANTIZER_REGEXP = /[^\w.]+/g; class FetchrError extends Error { @@ -18,6 +20,21 @@ class FetchrError extends Error { } } +function _checkResourceHandlers(service) { + for (const operation of OPERATIONS) { + const handler = service[operation]; + if (!handler) { + continue; + } + + if (handler.length > 1) { + console.warn( + `${service.resource} ${operation} handler is callback based. Callback based resource handlers are deprecated and will be removed in the next version.`, + ); + } + } +} + function parseValue(value) { // take care of value of type: array, object try { @@ -200,26 +217,83 @@ class Request { * @param {Object} errData The error response for failed request * @param {Object} result The response data for successful request */ - _captureMetaAndStats(errData, result) { - const meta = (errData && errData.meta) || (result && result.meta); + _captureMetaAndStats(err, meta) { if (meta) { this.serviceMeta.push(meta); } if (typeof this._statsCollector === 'function') { - const err = errData && errData.err; this._statsCollector({ resource: this.resource, operation: this.operation, params: this._params, statusCode: err ? err.statusCode - : (result && result.meta && result.meta.statusCode) || 200, + : (meta && meta.statusCode) || 200, err, time: Date.now() - this._startTime, }); } } + /** + * Execute this fetcher request + * @returns {Promise} + */ + async _executeRequest() { + if (!Fetcher.isRegistered(this.resource)) { + const err = new FetchrError( + `Service "${sanitizeResourceName(this.resource)}" could not be found`, + ); + return { err }; + } + + const service = Fetcher.getService(this.resource); + const handler = service[this.operation]; + + if (!handler) { + const err = new FetchrError( + `operation: ${this.operation} is undefined on service: ${this.resource}`, + ); + return { err }; + } + + // async based handler + if (handler.length <= 1) { + return handler + .call(service, { + body: this._body, + config: this._clientConfig, + params: this._params, + req: this.req, + resource: this.resource, + }) + .catch((err) => ({ err })); + } + + // callback based handler + return new Promise((resolve) => { + const args = [ + this.req, + this.resource, + this._params, + this._clientConfig, + function executeRequestCallback(err, data, meta) { + resolve({ err, data, meta }); + }, + ]; + + if (this.operation === OP_CREATE || this.operation === OP_UPDATE) { + args.splice(3, 0, this._body); + } + + try { + handler.apply(service, args); + } catch (err) { + resolve({ err }); + } + }); + } + /** * Execute this fetcher request and call callback. * @param {fetcherCallback} callback callback invoked when service @@ -234,31 +308,19 @@ class Request { this._startTime = Date.now(); - const promise = new Promise((resolve, reject) => { - setImmediate(executeRequest, this, resolve, reject); - }).then( - (result) => { - this._captureMetaAndStats(null, result); - return result; - }, - (errData) => { - this._captureMetaAndStats(errData); - throw errData.err; - }, - ); - - if (callback) { - promise.then( - (result) => { - setImmediate(callback, null, result.data, result.meta); - }, - (err) => { - setImmediate(callback, err); - }, - ); - } else { - return promise; - } + return this._executeRequest().then(({ err, data, meta }) => { + this._captureMetaAndStats(err, meta); + if (callback) { + callback(err, data, meta); + return; + } + + if (err) { + throw err; + } + + return { data, meta }; + }); } then(resolve, reject) { @@ -280,45 +342,6 @@ class Request { } } -/** - * Execute and resolve/reject this fetcher request - * @param {Object} request Request instance object - * @param {Function} resolve function to call when request fulfilled - * @param {Function} reject function to call when request rejected - */ -function executeRequest(request, resolve, reject) { - const args = [ - request.req, - request.resource, - request._params, - request._clientConfig, - function executeRequestCallback(err, data, meta) { - if (err) { - reject({ err, meta }); - } else { - resolve({ data, meta }); - } - }, - ]; - - const op = request.operation; - if (op === OP_CREATE || op === OP_UPDATE) { - args.splice(3, 0, request._body); - } - - try { - const service = Fetcher.getService(request.resource); - if (!service[op]) { - throw new FetchrError( - `operation: ${op} is undefined on service: ${request.resource}`, - ); - } - service[op].apply(service, args); - } catch (err) { - reject({ err }); - } -} - class Fetcher { /** * Fetcher class for the server. @@ -386,6 +409,7 @@ class Fetcher { '"resource" property is missing in service definition.', ); } + _checkResourceHandlers(service); Fetcher.services[resource] = service; return; @@ -485,18 +509,14 @@ following services definitions: ${services}.`); return next(badOperationError(resource, operation)); } - const serviceMeta = []; - new Request(operation, resource, { req, - serviceMeta, statsCollector, paramsProcessor, }) .params(params) .body(body) - .end((err, data) => { - const meta = serviceMeta[0] || {}; + .end((err, data, meta = {}) => { if (meta.headers) { res.set(meta.headers); } @@ -694,3 +714,11 @@ module.exports = Fetcher; * @param {Object} [meta] request meta-data * @param {number} [meta.statusCode=200] http status code to return */ + +/** + * @typedef {Object} FetchrResponse + * @property {?object} data - Any data returned by the fetchr resource. + * @property {?object} meta - Any meta data returned by the fetchr resource. + * @property {?Error} err - an error that occurred before, during or + * after request was sent. + */ diff --git a/tests/functional/resources/alwaysSlow.js b/tests/functional/resources/alwaysSlow.js index f04cd7ed..fcc07612 100644 --- a/tests/functional/resources/alwaysSlow.js +++ b/tests/functional/resources/alwaysSlow.js @@ -1,17 +1,17 @@ +const wait = require('./wait'); + // This resource allows us to exercise timeout and abort capacities of // the fetchr client. const alwaysSlowService = { resource: 'slow', - read(req, resource, params, config, callback) { - setTimeout(() => { - callback(null, { ok: true }); - }, 5000); + async read() { + await wait(5000); + return { data: { ok: true } }; }, - create(req, resource, params, body, config, callback) { - setTimeout(() => { - callback(null, { ok: true }); - }, 5000); + async create() { + await wait(5000); + return { data: { ok: true } }; }, }; diff --git a/tests/functional/resources/error.js b/tests/functional/resources/error.js index aa9d8005..61fb5b19 100644 --- a/tests/functional/resources/error.js +++ b/tests/functional/resources/error.js @@ -1,17 +1,17 @@ +const wait = require('./wait'); + const retryToggle = { error: true }; const errorsService = { resource: 'error', - read(req, resource, params, config, callback) { + async read({ params }) { if (params.error === 'unexpected') { throw new Error('unexpected'); } if (params.error === 'timeout') { - setTimeout(() => { - callback(null, { ok: true }); - }, 100); - return; + await wait(100); + return { data: { ok: true } }; } if (params.error === 'retry') { @@ -19,19 +19,25 @@ const errorsService = { retryToggle.error = false; const err = new Error('retry'); err.statusCode = 408; - callback(err); - } else { - callback(null, { retry: 'ok' }); + throw err; } - return; + + return { data: { retry: 'ok' } }; } const err = new Error('error'); err.statusCode = 400; - callback(err, null, { foo: 'bar' }); + + return { + err, + meta: { + foo: 'bar', + }, + }; }, - create(req, resource, params, body, config, callback) { - this.read(req, resource, params, config, callback); + + async create({ params }) { + return this.read({ params }); }, }; diff --git a/tests/functional/resources/headers.js b/tests/functional/resources/headers.js index 1302d9f0..b58cca3e 100644 --- a/tests/functional/resources/headers.js +++ b/tests/functional/resources/headers.js @@ -1,18 +1,22 @@ const headersService = { resource: 'header', - read(req, resource, params, config, callback) { + async read({ req }) { if (req.headers['x-fetchr-request'] !== '42') { const err = new Error('missing x-fetchr header'); err.statusCode = 400; - callback(err); - return; + throw err; } - callback( - null, - { headers: 'ok' }, - { headers: { 'x-fetchr-response': '42' } }, - ); + return { + data: { + headers: 'ok', + }, + meta: { + headers: { + 'x-fetchr-response': '42', + }, + }, + }; }, }; diff --git a/tests/functional/resources/item.js b/tests/functional/resources/item.js index 41b36101..47b1847d 100644 --- a/tests/functional/resources/item.js +++ b/tests/functional/resources/item.js @@ -3,51 +3,54 @@ const itemsData = {}; const itemsService = { resource: 'item', - create(req, resource, params, body, config, callback) { + async create({ params, body }) { const item = { id: params.id, value: body.value, }; itemsData[item.id] = item; - callback(null, item, { statusCode: 201 }); + return { data: item, meta: { statusCode: 201 } }; }, - read(req, resource, params, config, callback) { + async read({ params }) { if (params.id) { const item = itemsData[params.id]; if (!item) { const err = new Error('not found'); err.statusCode = 404; - callback(err, null, { foo: 42 }); + return { err, meta: { foo: 42 } }; } else { - callback(null, item, { statusCode: 200 }); + return { data: item, meta: { statusCode: 200 } }; } } else { - callback(null, Object.values(itemsData), { statusCode: 200 }); + return { + data: Object.values(itemsData), + meta: { statusCode: 200 }, + }; } }, - update(req, resource, params, body, config, callback) { + async update({ params, body }) { const item = itemsData[params.id]; if (!item) { const err = new Error('not found'); err.statusCode = 404; - callback(err); + throw err; } else { const updatedItem = { ...item, ...body }; itemsData[params.id] = updatedItem; - callback(null, updatedItem, { statusCode: 201 }); + return { data: updatedItem, meta: { statusCode: 201 } }; } }, - delete(req, resource, params, config, callback) { + async delete({ params }) { try { delete itemsData[params.id]; - callback(null, null, { statusCode: 200 }); - } catch (_) { + return { data: null, meta: { statusCode: 200 } }; + } catch { const err = new Error('not found'); err.statusCode = 404; - callback(err); + throw err; } }, }; diff --git a/tests/functional/resources/slowThenFast.js b/tests/functional/resources/slowThenFast.js index e58e2a27..6b716c01 100644 --- a/tests/functional/resources/slowThenFast.js +++ b/tests/functional/resources/slowThenFast.js @@ -1,3 +1,5 @@ +const wait = require('./wait'); + // This resource gives 2 slow responses and then a fast one. This is // so, so we can test that fetchr client is able to retry timed out // requests. @@ -8,17 +10,18 @@ const state = { const slowThenFastService = { resource: 'slow-then-fast', - read(req, resource, params, config, callback) { + async read({ params }) { if (params.reset) { state.count = 0; - callback(); + return {}; } const timeout = state.count === 2 ? 0 : 5000; state.count++; - setTimeout(() => { - callback(null, { attempts: state.count }); - }, timeout); + + await wait(timeout); + + return { data: { attempts: state.count } }; }, }; diff --git a/tests/functional/resources/wait.js b/tests/functional/resources/wait.js new file mode 100644 index 00000000..e8b1dec6 --- /dev/null +++ b/tests/functional/resources/wait.js @@ -0,0 +1,3 @@ +module.exports = function wait(time) { + return new Promise((resolve) => setTimeout(() => resolve(), time)); +}; diff --git a/tests/mock/MockErrorService.js b/tests/mock/MockErrorService.js index 0eee4016..10465ced 100644 --- a/tests/mock/MockErrorService.js +++ b/tests/mock/MockErrorService.js @@ -5,22 +5,10 @@ var MockErrorService = { resource: 'mock_error_service', - // ------------------------------------------------------------------ - // CRUD Methods - // ------------------------------------------------------------------ + read: async function ({ req, params }) { + const meta = this.meta || params.meta; + this.meta = null; - /** - * read operation (read as in CRUD). - * @method read - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - read: function (req, resource, params, config, callback) { if ( req.query && req.query.cors && @@ -39,87 +27,56 @@ var MockErrorService = { params[key] = value; } } - callback( - { + return { + err: { statusCode: parseInt(params.statusCode), output: params.output, message: params.message, read: 'error', }, - null, - this.meta || params.meta, - ); - this.meta = null; + data: null, + meta, + }; }, - /** - * create operation (create as in CRUD). - * @method create - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} body The JSON object that contains the resource data that is being created - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - create: function (req, resource, params, body, config, callback) { - callback( - { + + create: async function ({ params }) { + const meta = this.meta || params.meta; + this.meta = null; + + return { + err: { statusCode: parseInt(params.statusCode), message: params.message, output: params.output, create: 'error', }, - null, - this.meta || params.meta, - ); - this.meta = null; + data: null, + meta, + }; }, - /** - * update operation (update as in CRUD). - * @method update - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} body The JSON object that contains the resource data that is being updated - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - update: function (req, resource, params, body, config, callback) { - callback( - { + + update: async function ({ params }) { + return { + err: { statusCode: parseInt(params.statusCode), message: params.message, output: params.output, update: 'error', }, - null, - ); + data: null, + }; }, - /** - * delete operation (delete as in CRUD). - * @method delete - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - delete: function (req, resource, params, config, callback) { - callback( - { + + delete: async function ({ params }) { + return { + err: { statusCode: parseInt(params.statusCode), message: params.message, output: params.output, delete: 'error', }, - null, - ); + data: null, + }; }, }; diff --git a/tests/mock/MockService.js b/tests/mock/MockService.js index c3d6a53b..ed9ff020 100644 --- a/tests/mock/MockService.js +++ b/tests/mock/MockService.js @@ -5,22 +5,10 @@ var MockService = { resource: 'mock_service', - // ------------------------------------------------------------------ - // CRUD Methods - // ------------------------------------------------------------------ + read: async function ({ req, resource, params }) { + const meta = this.meta || params.meta; + this.meta = null; - /** - * read operation (read as in CRUD). - * @method read - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - read: function (req, resource, params, config, callback) { if ( req.query && req.query.cors && @@ -39,9 +27,9 @@ var MockService = { params[key] = value; } } - callback( - null, - { + + return { + data: { operation: { name: 'read', success: true, @@ -51,26 +39,16 @@ var MockService = { params: params, }, }, - this.meta || params.meta, - ); - this.meta = null; + meta, + }; }, - /** - * create operation (create as in CRUD). - * @method create - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} body The JSON object that contains the resource data that is being created - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - create: function (req, resource, params, body, config, callback) { - callback( - null, - { + + create: async function ({ resource, params }) { + const meta = this.meta || params.meta; + this.meta = null; + + return { + data: { operation: { name: 'create', success: true, @@ -80,26 +58,17 @@ var MockService = { params: params, }, }, - this.meta || params.meta, - ); - this.meta = null; + err: null, + meta, + }; }, - /** - * update operation (update as in CRUD). - * @method update - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} body The JSON object that contains the resource data that is being updated - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - update: function (req, resource, params, body, config, callback) { - callback( - null, - { + + update: async function ({ resource, params }) { + const meta = this.meta || params.meta; + this.meta = null; + + return { + data: { operation: { name: 'update', success: true, @@ -109,25 +78,16 @@ var MockService = { params: params, }, }, - this.meta || params.meta, - ); - this.meta = null; + meta, + }; }, - /** - * delete operation (delete as in CRUD). - * @method delete - * @param {Object} req The request object from connect/express - * @param {String} resource The resource name - * @param {Object} params The parameters identify the resource, and along with information - * carried in query and matrix parameters in typical REST API - * @param {Object} [config={}] The config object. It can contain "config" for per-request config data. - * @param {Fetcher~fetcherCallback} callback callback invoked when fetcher is complete. - * @static - */ - delete: function (req, resource, params, config, callback) { - callback( - null, - { + + delete: async function ({ resource, params }) { + const meta = this.meta || params.meta; + this.meta = null; + + return { + data: { operation: { name: 'delete', success: true, @@ -137,9 +97,8 @@ var MockService = { params: params, }, }, - this.meta || params.meta, - ); - this.meta = null; + meta, + }; }, };