diff --git a/libs/fetcher.client.js b/libs/fetcher.client.js index 9dee1dfa..b680e9f5 100644 --- a/libs/fetcher.client.js +++ b/libs/fetcher.client.js @@ -10,65 +10,14 @@ * @module Fetcher */ var REST = require('./util/http.client'); -var DEFAULT_GUID = 'g0'; +var defaultConstructGetUri = require('./util/defaultConstructGetUri'); +var forEach = require('./util/forEach'); +var pickContext = require('./util/pickContext'); + var DEFAULT_PATH = '/api'; var DEFAULT_TIMEOUT = 3000; var MAX_URI_LEN = 2048; var OP_READ = 'read'; -var defaultConstructGetUri = require('./util/defaultConstructGetUri'); -var forEach = require('./util/forEach'); - -function isFunction(value) { - return typeof value === 'function'; -} - -function parseResponse(response) { - if (response && response.responseText) { - try { - return JSON.parse(response.responseText); - } catch (e) { - return null; - } - } - return null; -} - -/** - * Pick keys from the context object - * @method pickContext - * @param {Object} context - context object - * @param {Function|Array|String} picker - key, array of keys or - * function that return keys to be extracted from context. - * @param {String} method - method name, GET or POST - */ -function pickContext(context, picker, method) { - if (!picker || !picker[method]) { - return context; - } - - var p = picker[method]; - var result = {}; - - if (typeof p === 'string') { - result[p] = context[p]; - } else if (Array.isArray(p)) { - p.forEach(function (key) { - result[key] = context[key]; - }); - } else if (typeof p === 'function') { - forEach(context, function (value, key) { - if (p(value, key, context)) { - result[key] = context[key]; - } - }); - } else { - throw new TypeError( - 'picker must be an string, an array, or a function.' - ); - } - - return result; -} /** * A RequestClient instance represents a single fetcher request. @@ -224,117 +173,71 @@ Request.prototype.end = function (callback) { * @param {Function} reject function to call when request rejected */ function executeRequest(request, resolve, reject) { - var clientConfig = request._clientConfig; - var use_post; - var allow_retry_post; - var uri = clientConfig.uri; - var requests; - var data; - - if (!uri) { - uri = clientConfig.cors + var callback = function (err, response) { + if (err) { + return reject(err); + } + resolve(response); + }; + + var config = Object.assign( + { + unsafeAllowRetry: request.operation === OP_READ, + xhrTimeout: request.options.xhrTimeout, + }, + request._clientConfig + ); + var headers = config.headers || request.options.headers || {}; + + var url; + var baseUrl = config.uri; + if (!baseUrl) { + baseUrl = config.cors ? request.options.corsPath : request.options.xhrPath; } - use_post = request.operation !== OP_READ || clientConfig.post_for_read; - // We use GET request by default for READ operation, but you can override that behavior - // by specifying {post_for_read: true} in your request's clientConfig - if (!use_post) { - var getUriFn = isFunction(clientConfig.constructGetUri) - ? clientConfig.constructGetUri - : defaultConstructGetUri; - var get_uri = getUriFn.call( - request, - uri, - request.resource, - request._params, - clientConfig, - pickContext( - request.options.context, - request.options.contextPicker, - 'GET' - ) + if (request.operation === OP_READ) { + var buildGetUrl = + typeof config.constructGetUri === 'function' + ? config.constructGetUri + : defaultConstructGetUri; + + var context = pickContext( + request.options.context, + request.options.contextPicker, + 'GET' ); - /* istanbul ignore next */ - if (!get_uri) { - // If a custom getUriFn returns falsy value, we should run defaultConstructGetUri - // TODO: Add test for this fallback - get_uri = defaultConstructGetUri.call( - request, - uri, - request.resource, - request._params, - clientConfig, - request.options.context - ); - } - if (get_uri.length <= MAX_URI_LEN) { - uri = get_uri; - } else { - use_post = true; + var args = [ + baseUrl, + request.resource, + request._params, + request._clientConfig, + context, + ]; + + // If a custom getUriFn returns falsy value, we should run defaultConstructGetUri + // TODO: Add test for this fallback + url = + buildGetUrl.apply(request, args) || + defaultConstructGetUri.apply(request, args); + + if (url.length <= MAX_URI_LEN) { + return REST.get(url, headers, config, callback); } } - var customHeaders = clientConfig.headers || request.options.headers || {}; - if (!use_post) { - return REST.get( - uri, - customHeaders, - Object.assign( - { xhrTimeout: request.options.xhrTimeout }, - clientConfig - ), - function getDone(err, response) { - if (err) { - return reject(err); - } - resolve(parseResponse(response)); - } - ); - } - - // individual request is also normalized into a request hash to pass to api - requests = {}; - requests[DEFAULT_GUID] = { - resource: request.resource, + url = request._constructPostUri(baseUrl); + var data = { + body: request._body, + context: request.options.context, operation: request.operation, params: request._params, + resource: request.resource, }; - if (request._body) { - requests[DEFAULT_GUID].body = request._body; - } - data = { - requests: requests, - context: request.options.context, - }; // TODO: remove. leave here for now for backward compatibility - uri = request._constructPostUri(uri); - allow_retry_post = request.operation === OP_READ; - return REST.post( - uri, - customHeaders, - data, - Object.assign( - { - unsafeAllowRetry: allow_retry_post, - xhrTimeout: request.options.xhrTimeout, - }, - clientConfig - ), - function postDone(err, response) { - if (err) { - return reject(err); - } - var result = parseResponse(response); - if (result) { - result = result[DEFAULT_GUID] || {}; - } else { - result = {}; - } - resolve(result); - } - ); + + return REST.post(url, headers, data, config, callback); } /** diff --git a/libs/fetcher.js b/libs/fetcher.js index 82ad1aba..7085abd1 100644 --- a/libs/fetcher.js +++ b/libs/fetcher.js @@ -9,7 +9,6 @@ const OP_READ = 'read'; const OP_CREATE = 'create'; const OP_UPDATE = 'update'; const OP_DELETE = 'delete'; -const GET = 'GET'; const RESOURCE_SANTIZER_REGEXP = /[^\w.]+/g; function parseValue(value) { @@ -36,12 +35,55 @@ function parseParamValues(params) { }, {}); } +function parseRequest(req) { + if (req.method === 'GET') { + const path = req.path.substr(1).split(';'); + const resource = path.shift(); + const operation = 'read'; + const params = parseParamValues(qs.parse(path.join('&'))); + return { resource, operation, params }; + } + + const { resource, operation, body = {}, params } = req.body || {}; + return { resource, operation, body, params }; +} + function sanitizeResourceName(resource) { return resource ? resource.replace(RESOURCE_SANTIZER_REGEXP, '*') : resource; } +function emptyResourceError() { + const error = fumble.http.badRequest('No resource specified', { + debug: 'No resource', + }); + error.source = 'fetchr'; + return error; +} + +function badResourceError(resource) { + const resourceName = sanitizeResourceName(resource); + const errorMsg = `Resource "${resourceName}" is not registered`; + const error = fumble.http.badRequest(errorMsg, { + debug: `Bad resource ${resourceName}`, + }); + error.source = 'fetchr'; + return error; +} + +function badOperationError(resource, operation) { + const resourceName = sanitizeResourceName(resource); + const error = fumble.http.badRequest( + `Unsupported "${resourceName}.${operation}" operation`, + { + debug: 'Only "create", "read", "update" or "delete" operations are allowed', + } + ); + error.source = 'fetchr'; + return error; +} + /** * Takes an error and resolves output and statusCode to respond to client with * @@ -376,151 +418,68 @@ class Fetcher { * @returns {Function} middleware */ static middleware(options = {}) { - const responseFormatter = - options.responseFormatter || ((req, res, data) => data); - if ( Fetcher._deprecatedServicesDefinitions.length && 'production' !== process.env.NODE_ENV ) { - const deprecatedServices = Fetcher._deprecatedServicesDefinitions + const services = Fetcher._deprecatedServicesDefinitions .sort() .join(', '); console.warn(`You have registered services using a deprecated property. Please, rename the property "name" to "resource" in the -following services definitions: ${deprecatedServices}.`); +following services definitions: ${services}.`); } - return (req, res, next) => { - const serviceMeta = []; + const { paramsProcessor, statsCollector, responseFormatter } = options; + const formatResponse = responseFormatter || ((req, res, data) => data); - if (req.method === GET) { - const path = req.path.substr(1).split(';'); - const resource = path.shift(); - - if (!resource) { - const error = fumble.http.badRequest( - 'No resource specified', - { - debug: 'Bad resource', - } - ); - error.source = 'fetchr'; - return next(error); - } + return (req, res, next) => { + const { body, operation, params, resource } = parseRequest(req); - if (!Fetcher.isRegistered(resource)) { - const resourceName = sanitizeResourceName(resource); - const errorMsg = `Resource "${resourceName}" is not registered`; - const error = fumble.http.badRequest(errorMsg, { - debug: `Bad resource ${resourceName}`, - }); - error.source = 'fetchr'; - return next(error); - } + if (!resource) { + return next(emptyResourceError()); + } - new Request(OP_READ, resource, { - req, - serviceMeta, - statsCollector: options.statsCollector, - paramsProcessor: options.paramsProcessor, - }) - .params(parseParamValues(qs.parse(path.join('&')))) - .end((err, data) => { - const meta = serviceMeta[0] || {}; - - if (meta.headers) { - res.set(meta.headers); - } - - if (err) { - const { statusCode, output } = - getErrorResponse(err); - res.status(statusCode).json( - responseFormatter(req, res, { output, meta }) - ); - } else { - res.status(meta.statusCode || 200).json( - responseFormatter(req, res, { data, meta }) - ); - } - }); - } else { - const requests = req.body && req.body.requests; - - if (!requests || Object.keys(requests).length === 0) { - const error = fumble.http.badRequest( - 'No resource specified', - { - debug: 'No resources', - } - ); - error.source = 'fetchr'; - return next(error); - } + if (!Fetcher.isRegistered(resource)) { + return next(badResourceError(resource)); + } - const DEFAULT_GUID = 'g0'; - const request = requests[DEFAULT_GUID]; - - if (!Fetcher.isRegistered(request.resource)) { - const resourceName = sanitizeResourceName(request.resource); - const errorMsg = `Resource "${resourceName}" is not registered`; - const error = fumble.http.badRequest(errorMsg, { - debug: `Bad resource ${resourceName}`, - }); - error.source = 'fetchr'; - return next(error); - } + if ( + operation !== OP_CREATE && + operation !== OP_UPDATE && + operation !== OP_DELETE && + operation !== OP_READ + ) { + return next(badOperationError(resource, operation)); + } - const operation = request.operation; - if ( - operation !== OP_CREATE && - operation !== OP_UPDATE && - operation !== OP_DELETE && - operation !== OP_READ - ) { - const resourceName = sanitizeResourceName(request.resource); - const error = fumble.http.badRequest( - `Unsupported "${resourceName}.${operation}" operation`, - { - debug: 'Only "create", "read", "update" or "delete" operations are allowed', - } - ); - error.source = 'fetchr'; - return next(error); - } + const serviceMeta = []; - new Request(operation, request.resource, { - req, - serviceMeta, - statsCollector: options.statsCollector, - paramsProcessor: options.paramsProcessor, - }) - .params(request.params) - .body(request.body || {}) - .end((err, data) => { - const meta = serviceMeta[0] || {}; - if (meta.headers) { - res.set(meta.headers); - } - if (err) { - const { statusCode, output } = - getErrorResponse(err); - res.status(statusCode).json( - responseFormatter(req, res, { output, meta }) - ); - } else { - res.status(meta.statusCode || 200).json({ - [DEFAULT_GUID]: responseFormatter(req, res, { - data, - meta, - }), - }); - } - }); - } - // TODO: Batching and multi requests + new Request(operation, resource, { + req, + serviceMeta, + statsCollector, + paramsProcessor, + }) + .params(params) + .body(body) + .end((err, data) => { + const meta = serviceMeta[0] || {}; + if (meta.headers) { + res.set(meta.headers); + } + if (err) { + const { statusCode, output } = getErrorResponse(err); + res.status(statusCode).json( + formatResponse(req, res, { output, meta }) + ); + } else { + res.status(meta.statusCode || 200).json( + formatResponse(req, res, { data, meta }) + ); + } + }); }; } diff --git a/libs/util/http.client.js b/libs/util/http.client.js index 5f117b70..3ad6827c 100644 --- a/libs/util/http.client.js +++ b/libs/util/http.client.js @@ -26,6 +26,17 @@ var DEFAULT_CONFIG = { var INITIAL_ATTEMPT = 0; +function parseResponse(response) { + if (response && response.responseText) { + try { + return JSON.parse(response.responseText); + } catch (e) { + return null; + } + } + return null; +} + function normalizeHeaders(rawHeaders, method, isCors) { var headers = Object.assign({}, rawHeaders); @@ -113,7 +124,7 @@ function doRequest(method, url, headers, data, config, attempt, callback) { withCredentials: config.withCredentials, on: { success: function (err, response) { - callback(null, response); + callback(null, parseResponse(response)); }, failure: function (err, response) { if (!shouldRetry(method, config, response.status, attempt)) { diff --git a/libs/util/pickContext.js b/libs/util/pickContext.js new file mode 100644 index 00000000..b77a1da2 --- /dev/null +++ b/libs/util/pickContext.js @@ -0,0 +1,40 @@ +var forEach = require('./forEach'); + +/** + * Pick keys from the context object + * @method pickContext + * @param {Object} context - context object + * @param {Function|Array|String} picker - key, array of keys or + * function that return keys to be extracted from context. + * @param {String} method - method name, GET or POST + */ +function pickContext(context, picker, method) { + if (!picker || !picker[method]) { + return context; + } + + var p = picker[method]; + var result = {}; + + if (typeof p === 'string') { + result[p] = context[p]; + } else if (Array.isArray(p)) { + p.forEach(function (key) { + result[key] = context[key]; + }); + } else if (typeof p === 'function') { + forEach(context, function (value, key) { + if (p(value, key, context)) { + result[key] = context[key]; + } + }); + } else { + throw new TypeError( + 'picker must be an string, an array, or a function.' + ); + } + + return result; +} + +module.exports = pickContext; diff --git a/tests/unit/libs/fetcher.js b/tests/unit/libs/fetcher.js index 1a99a4f0..598ae55f 100644 --- a/tests/unit/libs/fetcher.js +++ b/tests/unit/libs/fetcher.js @@ -126,18 +126,14 @@ describe('Server Fetcher', function () { method: 'POST', path: '/' + mockService.resource, body: { - requests: { - g0: { - resource: mockService.resource, - operation: operation, - params: { - uuids: [ - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - ], - id: 'asdf', - }, - }, + resource: mockService.resource, + operation: operation, + params: { + uuids: [ + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + ], + id: 'asdf', }, context: { site: '', @@ -149,22 +145,18 @@ describe('Server Fetcher', function () { json: function (response) { expect(response).to.exist; expect(response).to.not.be.empty; - var data = response.g0.data; + var data = response.data; expect(data).to.contain.keys('operation', 'args'); expect(data.operation.name).to.equal(operation); expect(data.operation.success).to.be.true; expect(data.args).to.contain.keys('params'); - expect(data.args.params).to.equal( - req.body.requests.g0.params - ); + expect(data.args.params).to.equal(req.body.params); expect(statusCodeSet).to.be.true; expect(stats.resource).to.eql(mockService.resource); expect(stats.operation).to.eql(operation); expect(stats.statusCode).to.eql(200); expect(stats.time).to.be.at.least(0); - expect(stats.params).to.eql( - req.body.requests.g0.params - ); + expect(stats.params).to.eql(req.body.params); done(); }, status: function (code) { @@ -199,18 +191,14 @@ describe('Server Fetcher', function () { method: 'POST', path: '/' + mockService.resource, body: { - requests: { - g0: { - resource: mockService.resource, - operation: operation, - params: { - uuids: [ - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - ], - id: 'asdf', - }, - }, + resource: mockService.resource, + operation: operation, + params: { + uuids: [ + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + ], + id: 'asdf', }, context: { site: '', @@ -222,23 +210,19 @@ describe('Server Fetcher', function () { json: function (response) { expect(response).to.exist; expect(response).to.not.be.empty; - var data = response.g0.data; + var data = response.data; expect(data).to.contain.keys('operation', 'args'); expect(data.operation.name).to.equal(operation); expect(data.operation.success).to.be.true; expect(data.args).to.contain.keys('params'); - expect(data.args.params).to.equal( - req.body.requests.g0.params - ); + expect(data.args.params).to.equal(req.body.params); expect(headersSet).to.eql(responseHeaders); expect(statusCodeSet).to.be.true; expect(stats.resource).to.eql(mockService.resource); expect(stats.operation).to.eql(operation); expect(stats.statusCode).to.eql(201); expect(stats.time).to.be.at.least(0); - expect(stats.params).to.eql( - req.body.requests.g0.params - ); + expect(stats.params).to.eql(req.body.params); expect(stats.err).to.eql(null); done(); }, @@ -286,13 +270,9 @@ describe('Server Fetcher', function () { method: 'POST', path: '/' + mockErrorService.resource, body: { - requests: { - g0: { - resource: mockErrorService.resource, - operation: operation, - params: params, - }, - }, + resource: mockErrorService.resource, + operation: operation, + params: params, context: { site: '', device: '', @@ -883,7 +863,7 @@ describe('Server Fetcher', function () { var request = { method: 'GET', path: '/' }; var error = { message: 'No resource specified', - debug: 'Bad resource', + debug: 'No resource', }; makeInvalidReqTest(request, error, done); }); @@ -910,11 +890,7 @@ describe('Server Fetcher', function () { var request = { method: 'POST', body: { - requests: { - g0: { - resource: 'invalidService', - }, - }, + resource: 'invalidService', }, }; var error = { @@ -928,11 +904,7 @@ describe('Server Fetcher', function () { var request = { method: 'POST', body: { - requests: { - g0: { - resource: 'invalid&Service', - }, - }, + resource: 'invalid&Service', }, }; var error = { @@ -946,12 +918,8 @@ describe('Server Fetcher', function () { var request = { method: 'POST', body: { - requests: { - g0: { - resource: mockErrorService.resource, - operation: 'constructor', - }, - }, + resource: mockErrorService.resource, + operation: 'constructor', }, }; var error = { @@ -966,7 +934,7 @@ describe('Server Fetcher', function () { var request = { method: 'POST', body: { requests: {} } }; var error = { message: 'No resource specified', - debug: 'No resources', + debug: 'No resource', }; makeInvalidReqTest(request, error, done); }); @@ -975,7 +943,7 @@ describe('Server Fetcher', function () { var request = { method: 'POST' }; var error = { message: 'No resource specified', - debug: 'No resources', + debug: 'No resource', }; makeInvalidReqTest(request, error, done); }); @@ -1061,18 +1029,14 @@ describe('Server Fetcher', function () { method: 'POST', path: '/' + mockService.resource, body: { - requests: { - g0: { - resource: mockService.resource, - operation: operation, - params: { - uuids: [ - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - ], - id: 'asdf', - }, - }, + resource: mockService.resource, + operation: operation, + params: { + uuids: [ + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + ], + id: 'asdf', }, context: { site: '', @@ -1084,12 +1048,12 @@ describe('Server Fetcher', function () { json: function (response) { expect(response).to.exist; expect(response).to.not.be.empty; - expect(response.g0).to.contain.keys( + expect(response).to.contain.keys( 'data', 'meta', 'modified' ); - var data = response.g0.data; + var data = response.data; expect(data).to.contain.keys( 'operation', 'args' @@ -1098,7 +1062,7 @@ describe('Server Fetcher', function () { expect(data.operation.success).to.be.true; expect(data.args).to.contain.keys('params'); expect(data.args.params).to.equal( - req.body.requests.g0.params + req.body.params ); expect(statusCodeSet).to.be.true; done(); @@ -1215,18 +1179,14 @@ describe('Server Fetcher', function () { ';' + qs.stringify(params, ';'), body: { - requests: { - g0: { - resource: mockService.resource, - operation: operation, - params: { - uuids: [ - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', - ], - id: 'asdf', - }, - }, + resource: mockService.resource, + operation: operation, + params: { + uuids: [ + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + 'cd7240d6-aeed-3fed-b63c-d7e99e21ca17', + ], + id: 'asdf', }, context: { site: '', @@ -1238,15 +1198,13 @@ describe('Server Fetcher', function () { json: function (response) { expect(response).to.exist; expect(response).to.not.be.empty; - expect(response.g0).to.contain.keys('data', 'meta'); - var data = response.g0.data; + expect(response).to.contain.keys('data', 'meta'); + var data = response.data; expect(data).to.contain.keys('operation', 'args'); expect(data.operation.name).to.equal(operation); expect(data.operation.success).to.be.true; expect(data.args).to.contain.keys('params'); - expect(data.args.params).to.include( - req.body.requests.g0.params - ); + expect(data.args.params).to.include(req.body.params); expect(data.args.params.newParam).to.include('YES!'); expect(statusCodeSet).to.be.true; done(); diff --git a/tests/unit/libs/util/http.client.js b/tests/unit/libs/util/http.client.js index d04a869b..e6460037 100644 --- a/tests/unit/libs/util/http.client.js +++ b/tests/unit/libs/util/http.client.js @@ -26,7 +26,7 @@ describe('Client HTTP', function () { describe('#Successful requests', function () { beforeEach(function () { responseStatus = 200; - mockBody = 'BODY'; + mockBody = { data: 'BODY' }; }); it('GET', function (done) { @@ -45,8 +45,7 @@ describe('Client HTTP', function () { expect(options.headers.get('X-Foo')).to.equal('foo'); expect(options.method).to.equal('GET'); expect(err).to.equal(null); - expect(response.statusCode).to.equal(200); - expect(response.responseText).to.equal('BODY'); + expect(response).to.deep.equal(mockBody); done(); }); }); @@ -81,7 +80,7 @@ describe('Client HTTP', function () { describe('#Successful CORS requests', function () { beforeEach(function () { responseStatus = 200; - mockBody = 'BODY'; + mockBody = { data: 'BODY' }; sinon.spy(global, 'Request'); }); @@ -109,8 +108,7 @@ describe('Client HTTP', function () { expect(options.headers.get('X-Foo')).to.equal('foo'); expect(options.method).to.equal('GET'); expect(err).to.equal(null); - expect(response.statusCode).to.equal(200); - expect(response.responseText).to.equal('BODY'); + expect(response).to.deep.equal(mockBody); sinon.assert.calledWith( Request,