diff --git a/.jshintrc b/.jshintrc index a93bf5e..354e8d6 100644 --- a/.jshintrc +++ b/.jshintrc @@ -17,14 +17,14 @@ "strict": true, "unused": true, "globals": { - "require": true, + "Promise": true, + "after": true, + "afterEach": true, "before": true, "beforeEach": true, - "after": true, - "define": true, "describe": true, - "it": true, "expect": true, - "Promise": true + "it": true, + "require": true } } diff --git a/README.md b/README.md index 0419f5e..003ba55 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,8 @@ npm install fuel-rest --save * `options.auth` - will be passed into [getAccessToken][4] inside Fuel Auth * `options.uri` - can either be a full url or a path that is appended to `options.origin` used at initialization ([url.resolve][2]) * `options.retry` - boolean value representing whether or not to retry request (and request new token) on 401 invalid token response. `default: false` - * `callback` - executed after task is completed. **required** + * `callback` - executed after task is completed. + * if no callback is passed, you'll need to use the promise interface * **get | post | put | patch | delete(options, callback)** * `options` - see apiRequest options * `options.retry` - see above for description. `default: true` @@ -75,6 +76,21 @@ RestClient.get(options, function(err, response) { // response.res === full response from request client console.log(response); }); + +// or with promises +RestClient + .get(options) + .then(function(response) { + // will be delivered with 200, 400, 401, 500, etc status codes + // response.body === payload from response + // response.res === full response from request client + console.log(response); + }) + .catch(function(err) { + // error here + console.log(err); + }); +}); ``` ## Contributors diff --git a/lib/fuel-rest.js b/lib/fuel-rest.js index e6655b3..927c48b 100644 --- a/lib/fuel-rest.js +++ b/lib/fuel-rest.js @@ -29,6 +29,7 @@ var helpers = require('./helpers'); var request = require('request'); var _ = require('lodash'); var FuelAuth = require('fuel-auth'); +var Promiser = (typeof Promise === 'undefined') ? require('bluebird') : Promise; var FuelRest = function(options) { 'use strict'; @@ -61,81 +62,86 @@ var FuelRest = function(options) { FuelRest.prototype.apiRequest = function(options, callback) { 'use strict'; - // we need a callback - if(!_.isFunction(callback)) { - throw new TypeError('callback argument is required'); - } + var self = this; // we need options if(!_.isPlainObject(options)) { throw new TypeError('options argument is required'); } - this.AuthClient.getAccessToken(_.clone(options.auth), function(err, body) { - var localError, retry, authOptions; - - if(err) { - helpers.respond('error', err, callback, 'FuelAuth'); - return; - } - - // if there's no access token we have a problem - if(!body.accessToken) { - localError = new Error('No access token'); - localError.res = body; - helpers.respond('error', localError, callback, 'FuelAuth'); - return; - } - - // retry request? - retry = options.retry || false; - authOptions = _.clone(options.auth); - - // clean up - delete options.retry; - delete options.auth; - - // if we don't have a fully qualified URL let's make one - options.uri = helpers.resolveUri(this.origin, options.uri); - - // merge headers - options.headers = _.merge({}, this.defaultHeaders, options.headers); - - // adding the bearer token - options.headers.Authorization = options.headers.Authorization || 'Bearer ' + body.accessToken; - - // send request to api - request(options, function(err, res, body) { - var parsedBody; - - if(err) { - helpers.respond('error', err, callback, 'Request Module inside apiRequest'); - return; - } - - // check if we should retry req - if(helpers.check401header(res) && !!retry) { - options.auth = authOptions; - this.apiRequest(options, callback); - return; - } - - // checking to make sure it's json from api - if(!res.headers['content-type'] || res.headers['content-type'].split(';')[0].toLowerCase() !== 'application/json') { - helpers.respond('error', new Error('API did not return JSON'), callback, 'Fuel REST'); - return; - } - - // trying to parse body - try { - parsedBody = JSON.parse(body); - } catch(err) { - parsedBody = body; - } - - helpers.respond('response', { res: res, body: parsedBody }, callback); - }.bind(this)); - }.bind(this)); + return self.AuthClient + .getAccessToken(_.clone(options.auth)) + .then(function(authResponse) { + return new Promiser(function(resolve, reject) { + var authOptions; + var jsonRequested; + var localError; + var retry; + + if(!authResponse.accessToken) { + localError = new Error('No access token'); + localError.res = authResponse; + reject(localError); + return; + } + + // retry request? + retry = options.retry || false; + authOptions = _.clone(options.auth); + + // clean up + delete options.retry; + delete options.auth; + + options.uri = helpers.resolveUri(self.origin, options.uri); + options.headers = _.merge({}, self.defaultHeaders, options.headers); + options.headers.Authorization = options.headers.Authorization || 'Bearer ' + authResponse.accessToken; + + request(options, function(err, res, body) { + var parsedBody, restResponse; + + if(err) { + reject(err); + return; + } + + // check if we should retry req + if(helpers.isValid401(res) && retry) { + options.auth = authOptions; + self.apiRequest(options, callback); + return; + } + + // checking to make sure it's json from api + jsonRequested = res.headers['content-type'] && res.headers[ 'content-type'].split(';')[ 0].toLowerCase() === 'application/json'; + if(!jsonRequested) { + localError = new Error('API did not return JSON'); + helpers.cbRespond('error', localError, callback); + reject(err); + return; + } + + // trying to parse body + try { + parsedBody = JSON.parse(body); + } catch(err) { + parsedBody = body; + } + + restResponse = { + res: res + , body: parsedBody + }; + + helpers.cbRespond('response', restResponse, callback); + resolve(restResponse); + }); + }); + }) + .catch(function(err) { + helpers.cbRespond('error', err, callback); + return err; + }); }; FuelRest.prototype.get = function(options, callback) { @@ -144,7 +150,7 @@ FuelRest.prototype.get = function(options, callback) { options.method = 'GET'; options.retry = true; - this.apiRequest(options, callback); + return this.apiRequest(options, callback); }; FuelRest.prototype.post = function(options, callback) { @@ -153,7 +159,7 @@ FuelRest.prototype.post = function(options, callback) { options.method = 'POST'; options.retry = true; - this.apiRequest(options, callback); + return this.apiRequest(options, callback); }; FuelRest.prototype.put = function(options, callback) { @@ -162,7 +168,7 @@ FuelRest.prototype.put = function(options, callback) { options.method = 'PUT'; options.retry = true; - this.apiRequest(options, callback); + return this.apiRequest(options, callback); }; FuelRest.prototype.patch = function(options, callback) { @@ -171,7 +177,7 @@ FuelRest.prototype.patch = function(options, callback) { options.method = 'PATCH'; options.retry = true; - this.apiRequest(options, callback); + return this.apiRequest(options, callback); }; FuelRest.prototype.delete = function(options, callback) { @@ -180,7 +186,7 @@ FuelRest.prototype.delete = function(options, callback) { options.method = 'DELETE'; options.retry = true; - this.apiRequest(options, callback); + return this.apiRequest(options, callback); }; module.exports = FuelRest; diff --git a/lib/helpers.js b/lib/helpers.js index 894de50..db17a83 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -1,30 +1,39 @@ 'use strict'; +var _ = require('lodash'); var url = require('url'); +var invalidTypeMsg = 'invalid response type'; + module.exports = { - check401header: function(res) { - var is401 = res.statusCode === 401; - var isFailureFromBadToken = /^Bearer\s.+?invalid_token/.test(res.headers['www-authenticate']); + isValid401: function(res) { + var is401 = (res.statusCode === 401); + var isFailureFromBadToken = false; + + if(res.headers && res.headers['www-authenticate']) { + isFailureFromBadToken = /^Bearer\s.+?invalid_token/.test(res.headers['www-authenticate']); + } return is401 && isFailureFromBadToken; } - , respond: function(type, data, callback, errorFrom) { + , resolveUri: function(origin, uri) { + if(origin && uri && !/^http/.test(uri)) { + uri = url.resolve(origin, uri); + } + return uri; + } + , cbRespond: function(type, data, callback) { + if(!_.isFunction(callback)) { + return; + } + // if it's an error and we have where it occured, let's tack it on if(type === 'error') { - if(!!errorFrom) { - data.errorPropagatedFrom = errorFrom; - } - callback(data, null); } else if(type === 'response') { callback(null, data); + } else { + callback(invalidTypeMsg, null); } } - , resolveUri: function(origin, uri) { - if(!/^http/.test(uri)) { - uri = url.resolve(origin, uri); - } - return uri; - } }; diff --git a/package.json b/package.json index 47ee259..257c9b6 100644 --- a/package.json +++ b/package.json @@ -18,12 +18,12 @@ "grunt": "~0.4.5", "grunt-bump": "~0.3.0", "grunt-contrib-jshint": "~0.11.0", - "grunt-jscs": "^1.8.0", + "grunt-jscs": "~1.8.0", "mocha": "~2.2.1", - "promise": "~6.1.0", "sinon": "~1.13.0" }, "dependencies": { + "bluebird": "~2.9.25", "fuel-auth": "~1.0.0", "lodash": "~3.5.0", "request": "~2.53.0" diff --git a/test/mock-server.js b/test/mock-server.js index 291af74..a97e3e3 100644 --- a/test/mock-server.js +++ b/test/mock-server.js @@ -91,7 +91,7 @@ module.exports = function(port) { if(reqUrl === validUrls.invalidToken) { if(totalRequests === 0) { res.writeHead(401, { - 'WWW-Authenticate': 'Bearer realm="example.com", error="invalid_token", error_description="The access token expired"' + 'WWW-Authenticate': sampleResponses.invalidToken }); res.end(JSON.stringify(sampleResponses['401'])); } else { diff --git a/test/sample-responses.js b/test/sample-responses.js index 6d37045..e48fd4c 100644 --- a/test/sample-responses.js +++ b/test/sample-responses.js @@ -43,4 +43,5 @@ module.exports = { , errorcode: 1 , message: "Unauthorized" } + , invalidToken: 'Bearer realm="example.com", error="invalid_token", error_description="The access token expired"' }; diff --git a/test/specs/function-apiRequest.js b/test/specs/fn-apiRequest.js similarity index 90% rename from test/specs/function-apiRequest.js rename to test/specs/fn-apiRequest.js index 37c0848..482d572 100644 --- a/test/specs/function-apiRequest.js +++ b/test/specs/fn-apiRequest.js @@ -29,7 +29,7 @@ var FuelAuth = require('fuel-auth'); var FuelRest = require('../../lib/fuel-rest'); var mockServer = require('../mock-server'); var port = 4550; -var Promiser = (typeof Promise === 'undefined') ? require('promise') : Promise; +var Promiser = (typeof Promise === 'undefined') ? require('bluebird') : Promise; var sinon = require('sinon'); var routes = require('../config').routes; @@ -38,7 +38,9 @@ var localhost = 'http://127.0.0.1:' + port; describe('apiRequest method', function() { 'use strict'; - var server, RestClient, requestOptions; + var RestClient; + var requestOptions; + var server; var initOptions = { auth: { @@ -80,15 +82,6 @@ describe('apiRequest method', function() { } }); - it('should throw an error if no callback is present', function() { - try { - RestClient.apiRequest(null, null); - } catch(err) { - expect(err.name).to.equal('TypeError'); - expect(err.message).to.equal('callback argument is required'); - } - }); - it('should make a requset to the API', function(done) { RestClient.apiRequest(requestOptions, function(err, data) { // making sure original request was GET @@ -164,7 +157,6 @@ describe('apiRequest method', function() { RestClient.apiRequest(options, function(err, data) { // error should be passed, and data should be null expect(!!err).to.be.true; - expect(err.errorPropagatedFrom).to.equal('Request Module inside apiRequest'); expect(data).to.be.null; // finish async test @@ -176,11 +168,15 @@ describe('apiRequest method', function() { var RestClient; // stubbing response from auth client with no access token - sinon.stub(FuelAuth.prototype, 'getAccessToken', function(options, callback) { - callback(null, { - documentation: "https://code.docs.exacttarget.com/rest/errors/404" - , errorcode: 404 - , message: "Not Found" + sinon.stub(FuelAuth.prototype, 'getAccessToken', function() { + return new Promiser(function(resolve) { + // fuel auth only rejects if there was an error returned from request + // not if the API returned an error code other than 200 + resolve({ + documentation: "https://code.docs.exacttarget.com/rest/errors/404" + , errorcode: 404 + , message: "Not Found" + }); }); }); @@ -189,7 +185,6 @@ describe('apiRequest method', function() { RestClient.apiRequest(requestOptions, function(err, data) { // error should be passed, and data should be null expect(!!err).to.be.true; - expect(err.errorPropagatedFrom).to.equal('FuelAuth'); expect(err.message).to.equal('No access token'); expect(err.res).to.be.a('object'); expect(data).to.be.null; @@ -217,7 +212,6 @@ describe('apiRequest method', function() { RestClient.apiRequest(requestOptions, function(err, data) { // error should be passed, and data should be null expect(!!err).to.be.true; - expect(err.errorPropagatedFrom).to.equal('FuelAuth'); expect(err.message).to.equal('error from auth client'); expect(data).to.be.null; @@ -230,7 +224,8 @@ describe('apiRequest method', function() { }); it('should try request again if 401 stating token is invalid', function(done) { - var requestSpy, RestClient; + var requestSpy; + var RestClient; requestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); diff --git a/test/specs/function-http-methods.js b/test/specs/fn-http-methods.js similarity index 95% rename from test/specs/function-http-methods.js rename to test/specs/fn-http-methods.js index 72f15a0..f0cfe3b 100644 --- a/test/specs/function-http-methods.js +++ b/test/specs/fn-http-methods.js @@ -35,7 +35,9 @@ var routes = require('../config').routes; describe('HTTP methods', function() { 'use strict'; - var server, initOptions, requestOptions; + var initOptions; + var requestOptions; + var server; before(function() { server = mockServer(port); @@ -63,7 +65,8 @@ describe('HTTP methods', function() { describe('GET', function() { it('should deliver a GET + response', function(done) { - var apiRequestSpy, RestClient; + var apiRequestSpy; + var RestClient; apiRequestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); RestClient = new FuelRest(initOptions); @@ -87,7 +90,8 @@ describe('HTTP methods', function() { describe('POST', function() { it('should deliver a POST', function(done) { - var apiRequestSpy, RestClient; + var apiRequestSpy; + var RestClient; apiRequestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); RestClient = new FuelRest(initOptions); @@ -114,7 +118,8 @@ describe('HTTP methods', function() { describe('PUT', function() { it('should deliever an PUT/UPDATE', function(done) { - var apiRequestSpy, RestClient; + var apiRequestSpy; + var RestClient; apiRequestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); RestClient = new FuelRest(initOptions); @@ -140,7 +145,8 @@ describe('HTTP methods', function() { describe('PATCH', function() { it('should deliever an PATCH', function(done) { - var apiRequestSpy, RestClient; + var apiRequestSpy; + var RestClient; apiRequestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); RestClient = new FuelRest(initOptions); @@ -166,7 +172,8 @@ describe('HTTP methods', function() { describe('DELETE', function() { it('should deliever an DELETE', function(done) { - var apiRequestSpy, RestClient; + var apiRequestSpy; + var RestClient; apiRequestSpy = sinon.spy(FuelRest.prototype, 'apiRequest'); RestClient = new FuelRest(initOptions); diff --git a/test/specs/general-tests.js b/test/specs/general-tests.js index 816c03d..3e26f06 100644 --- a/test/specs/general-tests.js +++ b/test/specs/general-tests.js @@ -62,7 +62,8 @@ describe('General Tests', function() { }); it('should use already initialized fuel auth client', function() { - var AuthClient, RestClient; + var AuthClient; + var RestClient; AuthClient = new FuelAuth(options.auth); diff --git a/test/specs/helpers.js b/test/specs/helpers.js new file mode 100644 index 0000000..48fe0c3 --- /dev/null +++ b/test/specs/helpers.js @@ -0,0 +1,175 @@ +/** +* Copyright (c) 2014​, salesforce.com, inc. +* All rights reserved. +* +* Redistribution and use in source and binary forms, with or without modification, are permitted provided +* that the following conditions are met: +* +* Redistributions of source code must retain the above copyright notice, this list of conditions and the +* following disclaimer. +* +* Redistributions in binary form must reproduce the above copyright notice, this list of conditions and +* the following disclaimer in the documentation and/or other materials provided with the distribution. +* +* Neither the name of salesforce.com, inc. nor the names of its contributors may be used to endorse or +* promote products derived from this software without specific prior written permission. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED +* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +* PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR +* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED +* TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ + +var expect = require('chai').expect; +var sinon = require('sinon'); +var helpers = require('../../lib/helpers'); +var sampleResponses = require('../sample-responses'); +var url = require('url'); + +describe('helpers', function() { + 'use strict'; + + var invalidTypeMsg = 'invalid response type'; + + describe('isValid401', function() { + it('should return true if 401 and token failure', function() { + var res; + var result; + + res = { + statusCode: 401 + , headers: { + 'www-authenticate': sampleResponses.invalidToken + } + }; + + result = helpers.isValid401(res); + expect(result).to.be.true; + }); + + it('should return false if 401 and no token failure', function() { + var res; + var result; + + res = { + statusCode: 401 + , headers: {} + }; + + result = helpers.isValid401(res); + expect(result).to.be.false; + }); + + it('should return false if not 401', function() { + var res; + var result; + + res = { + statusCode: 200 + , headers: {} + }; + + result = helpers.isValid401(res); + expect(result).to.be.false; + }); + + it('should return false if no headers passed', function() { + var res; + var result; + + res = { + statusCode: 401 + }; + + result = helpers.isValid401(res); + expect(result).to.be.false; + }); + }); + + describe('resolveUri', function() { + var origin; + var resolveSpy; + var uri; + + beforeEach(function() { + origin = 'http://test.com'; + resolveSpy = sinon.stub(url, 'resolve'); + uri = 'path/to/test'; + }); + + afterEach(function() { + url.resolve.restore(); + }); + + it('should resolve a URI if origin and uri passed', function() { + helpers.resolveUri(origin, uri); + expect(resolveSpy.calledOnce).to.be.true; + }); + + it('should not resolve if URI has http in it', function() { + uri = origin + uri; + helpers.resolveUri(origin, uri); + expect(resolveSpy.calledOnce).to.be.false; + }); + + it('should not resolve if no origin is passed', function() { + helpers.resolveUri(null, uri); + expect(resolveSpy.calledOnce).to.be.false; + }); + + it('should not resolve if no URI is passed', function() { + helpers.resolveUri(origin, null); + expect(resolveSpy.calledOnce).to.be.false; + }); + }); + + describe('cbRespond', function() { + var options; + + beforeEach(function() { + options = { + type: 'error' + , data: { + test: true + } + , cb: function() {} + }; + }); + + it('should return if no callback', function() { + var promiseSpy = sinon.stub(options, 'cb'); + + helpers.cbRespond(options.type, options.data, null); + helpers.cbRespond(options.type, options.data, 'test'); + helpers.cbRespond(options.type, options.data, options.cb); + + expect(promiseSpy.callCount).to.equal(1); + }); + + it('should use callbacks for success if applicable', function() { + var cbSpy = sinon.spy(options, 'cb'); + + options.type = 'response'; + helpers.cbRespond(options.type, options.data, options.cb); + expect(cbSpy.calledWith(null, options.data)).to.be.true; + }); + + it('should use callbacks for errors if applicable', function() { + var cbSpy = sinon.spy(options, 'cb'); + + helpers.cbRespond(options.type, options.data, options.cb); + expect(cbSpy.calledWith(options.data, null)).to.be.true; + }); + + it('should return error if invalid type is passed', function() { + var cbSpy = sinon.spy(options, 'cb'); + + helpers.cbRespond('invalid', options.data, options.cb); + expect(cbSpy.calledWith(invalidTypeMsg, null)).to.be.true; + }); + }); +});