From 4ab98e7d5dd4aa45e6b8cc9654372132faa024f9 Mon Sep 17 00:00:00 2001 From: jbellenger Date: Thu, 11 Feb 2016 11:44:06 -0800 Subject: [PATCH 1/4] add support for pagination --- lib/client.js | 75 ++++++++++++++++++++++++++++++++++++-------------- package.json | 3 +- test/client.js | 63 ++++++++++++++++++++++++++---------------- 3 files changed, 96 insertions(+), 45 deletions(-) diff --git a/lib/client.js b/lib/client.js index 7803355..cc1e294 100644 --- a/lib/client.js +++ b/lib/client.js @@ -5,6 +5,7 @@ var winston = require('winston'); var lodash = require('lodash'); var request = require('request'); var Promise = require('promise'); +var parseLinkHeader = require('parse-link-header'); var DSN = require('./dsn').DSN; var Projects = require('./projects'); @@ -92,25 +93,18 @@ Client.prototype.teams = null; */ Client.prototype.events = null; -/** - * Make a request to the Sentry API. - * - * @method request - * @param {String} path The request path. - * @param {Object} options Request options. These are the same as the request module's options. - * @param {Function} callback Function to execute when the request completes. - * @return {Promise} - */ -Client.prototype.request = function(path, options, callback) { - var uri = util.format('%s/api/%d/%s', this.dsn.uri, this.config.version, path); +Client.prototype._merge = function(page, pages) { + if (lodash.isArray(pages)) { + return pages.concat(page); + } else { + return lodash.merge(pages, page); + } +}; +Client.prototype._request = function(uri, options) { this.logger.info(options.method, uri); - if (lodash.isUndefined(callback)) { - callback = function() {}; - } - - return new Promise(function (resolve, reject) { + return new Promise(function(resolve, reject) { request(lodash.extend({ uri: uri, json: true, @@ -120,13 +114,11 @@ Client.prototype.request = function(path, options, callback) { }, options), function(error, response, body) { if (error) { this.logger.error(error); - callback(error, null); reject(error); } else if (response.statusCode >= 200 && response.statusCode < 300) { this.logger.info(response.statusCode, response.statusMessage); - callback(null, body); - resolve(body); + resolve([response, body]); } else { this.logger.warn(response.statusCode, response.statusMessage); @@ -139,13 +131,56 @@ Client.prototype.request = function(path, options, callback) { else { error = new Error(response.statusCode); } - callback(error); reject(error) } }.bind(this)); }.bind(this)); }; +/** + * Make a request to the Sentry API. + * + * @method request + * @param {String} path The request path. + * @param {Object} options Request options. These are the same as the request module's options. + * @param {Function} callback Function to execute when the request completes. + * @return {Promise} + */ +Client.prototype.request = function(path, options, callback) { + var uri = util.format('%s/api/%d/%s', this.dsn.uri, this.config.version, path); + + if (lodash.isUndefined(callback)) { + callback = function() {}; + } + + const promise = this._request(uri, options) + .then(function(result) { + var response = result[0]; + var body = result[1]; + if (response.headers.link) { + var next = parseLinkHeader(response.headers.link).next; + if (next && next.results && next.url) { + return this._request(next.url, options) + .then(function (tailResult) { + return this._merge(tailResult[1], body); + }.bind(this)); + } + } + + return body; + }.bind(this)); + + return promise + .catch(function(error) { + callback(error, null); + throw error; + }) + .then(function(result) { + callback(null, result); + return result; + }); +}; + /** * Convenience method for making GET requests. * diff --git a/package.json b/package.json index 63ef59f..49e7b13 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sentry-api", - "version": "0.0.8", + "version": "0.1.0", "description": "A client for the Sentry API", "main": "index.js", "scripts": { @@ -13,6 +13,7 @@ "license": "ISC", "dependencies": { "lodash": "^3.10.1", + "parse-link-header": "^0.4.1", "promise": "^7.0.4", "request": "^2.65.0", "winston": "^2.1.0" diff --git a/test/client.js b/test/client.js index bae9013..aaeb05a 100644 --- a/test/client.js +++ b/test/client.js @@ -6,7 +6,7 @@ var Client = require('../lib/client').Client; exports.testConstructor = function(test) { test.expect(2); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); test.equal(client.config.version, 0, 'Should default version to 0.'); test.equal(client.config.logging, false, 'Should default logging to false.'); @@ -14,22 +14,37 @@ exports.testConstructor = function(test) { }; exports.testRequest = function(test) { - test.expect(2); + test.expect(5); + + var reqheaders = { + 'authorization': 'Basic ' + new Buffer('PUBLIC:').toString('base64'), + 'accept': 'application/json' + }; - var request = nock('https://host.com', { - reqheaders: { - 'authorization': 'Basic ' + new Buffer('PUBLIC:').toString('base64'), - 'accept': 'application/json' - } - }) + var request1 = nock('https://test.com', {reqheaders: reqheaders}) .get('/api/0/path') - .reply(200, {foo: 'bar'}); + .reply(200, {foo: 'FOO'}, { + 'Link': '; rel="previous"; results="false", ; rel="next"; results="true"', + }); + + var request2 = nock('https://test.com', {reqheaders: reqheaders}) + .get('/api/0/path/?&cursor=2') + .reply(200, {bar: 'BAR'}, { + 'Link': '; rel="previous"; results="true", ; rel="next"; results="false"', + }); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var request3 = nock('https://test.com', {reqheaders: reqheaders}) + .get('/api/0/path/?&cursor=3') + .reply(404); + + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.request('path', {}, function(error, response) { - test.ok(request.isDone(), 'Should make the correct request.'); - test.equal(response.foo, 'bar', 'Should return the response as an object.'); + test.ok(request1.isDone(), 'Should make the correct request.'); + test.ok(request2.isDone(), 'Should request all pages.'); + test.ok(!request3.isDone(), 'Should not request pages without results.'); + test.equal(response.foo, 'FOO', 'Should return the response as an object.'); + test.equal(response.bar, 'BAR', 'Should merge pages.'); test.done(); }); }; @@ -37,11 +52,11 @@ exports.testRequest = function(test) { exports.testError = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .get('/api/0/path') .reply(400); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.request('path', {}, function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); @@ -53,11 +68,11 @@ exports.testError = function(test) { exports.testJsonError = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .get('/api/0/path') .reply(400, {detail: 'bar'}); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.request('path', {}, function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); @@ -69,12 +84,12 @@ exports.testJsonError = function(test) { exports.testGet = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .get('/api/0/path') .query({foo: 'bar'}) .reply(200, {foo: 'bar'}); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.get('path', {foo: 'bar'}, function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); @@ -86,11 +101,11 @@ exports.testGet = function(test) { exports.testPost = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .post('/api/0/path', {foo: 'bar'}) .reply(200, {foo: 'bar'}); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.post('path', {foo: 'bar'}, function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); @@ -102,11 +117,11 @@ exports.testPost = function(test) { exports.testPut = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .put('/api/0/path', {foo: 'bar'}) .reply(200, {foo: 'bar'}); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.put('path', {foo: 'bar'}, function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); @@ -118,11 +133,11 @@ exports.testPut = function(test) { exports.testDelete = function(test) { test.expect(2); - var request = nock('https://host.com') + var request = nock('https://test.com') .delete('/api/0/path') .reply(200, {foo: 'bar'}); - var client = new Client('https://PUBLIC:SECRET@host.com/123'); + var client = new Client('https://PUBLIC:SECRET@test.com/123'); client.delete('path', function(error, response) { test.ok(request.isDone(), 'Should make the correct request.'); From 322313fd7281a6b7225f8b8dfad4272ea1f91a7a Mon Sep 17 00:00:00 2001 From: jbellenger Date: Tue, 16 Feb 2016 15:26:14 -0800 Subject: [PATCH 2/4] make sure we always really load all pages --- lib/client.js | 34 +++++++++++++++++++--------------- test/client.js | 16 ++++++++++++---- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/client.js b/lib/client.js index cc1e294..ac804ad 100644 --- a/lib/client.js +++ b/lib/client.js @@ -153,24 +153,28 @@ Client.prototype.request = function(path, options, callback) { callback = function() {}; } - const promise = this._request(uri, options) - .then(function(result) { - var response = result[0]; - var body = result[1]; - if (response.headers.link) { - var next = parseLinkHeader(response.headers.link).next; - if (next && next.results && next.url) { - return this._request(next.url, options) - .then(function (tailResult) { - return this._merge(tailResult[1], body); - }.bind(this)); + var self = this; + function load(uri) { + return self._request(uri, options) + .then(function(result) { + var response = result[0]; + var body = result[1]; + + if (response.headers.link) { + var next = parseLinkHeader(response.headers.link).next; + if (next && next.results === 'true' && next.url) { + return load(next.url, options) + .then(function (tailResult) { + return self._merge(tailResult, body); + }); + } } - } - return body; - }.bind(this)); + return body; + }); + } - return promise + return load(uri) .catch(function(error) { callback(error, null); throw error; diff --git a/test/client.js b/test/client.js index aaeb05a..f17db8d 100644 --- a/test/client.js +++ b/test/client.js @@ -14,7 +14,7 @@ exports.testConstructor = function(test) { }; exports.testRequest = function(test) { - test.expect(5); + test.expect(7); var reqheaders = { 'authorization': 'Basic ' + new Buffer('PUBLIC:').toString('base64'), @@ -30,10 +30,16 @@ exports.testRequest = function(test) { var request2 = nock('https://test.com', {reqheaders: reqheaders}) .get('/api/0/path/?&cursor=2') .reply(200, {bar: 'BAR'}, { - 'Link': '; rel="previous"; results="true", ; rel="next"; results="false"', + 'Link': '; rel="previous"; results="true", ; rel="next"; results="true"', }); var request3 = nock('https://test.com', {reqheaders: reqheaders}) + .get('/api/0/path/?&cursor=3') + .reply(200, {baz: 'BAZ'}, { + 'Link': '; rel="previous"; results="true", ; rel="next"; results="false"', + }); + + var request4 = nock('https://test.com', {reqheaders: reqheaders}) .get('/api/0/path/?&cursor=3') .reply(404); @@ -42,9 +48,11 @@ exports.testRequest = function(test) { client.request('path', {}, function(error, response) { test.ok(request1.isDone(), 'Should make the correct request.'); test.ok(request2.isDone(), 'Should request all pages.'); - test.ok(!request3.isDone(), 'Should not request pages without results.'); + test.ok(request3.isDone(), 'Should request all pages.'); + test.ok(!request4.isDone(), 'Should not request pages without results.'); test.equal(response.foo, 'FOO', 'Should return the response as an object.'); - test.equal(response.bar, 'BAR', 'Should merge pages.'); + test.equal(response.bar, 'BAR', 'Should merge second page.'); + test.equal(response.baz, 'BAZ', 'Should merge third page.'); test.done(); }); }; From f3cec28edb54aa469e35b14d91b3c229f25a571c Mon Sep 17 00:00:00 2001 From: jbellenger Date: Mon, 24 Apr 2017 15:32:27 -0700 Subject: [PATCH 3/4] add passthru for request options --- lib/client.js | 3 ++- package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index ac804ad..4436ed7 100644 --- a/lib/client.js +++ b/lib/client.js @@ -40,6 +40,7 @@ var Client = function(dsn, config) { this.config = lodash.defaults(config || {}, { version: 0, logging: false, + requestOptions: {} }); this.logger = new(winston.Logger)({ @@ -111,7 +112,7 @@ Client.prototype._request = function(uri, options) { auth: { user: this.dsn.publicKey, }, - }, options), function(error, response, body) { + }, options, this.config.requestOptions), function(error, response, body) { if (error) { this.logger.error(error); reject(error); diff --git a/package.json b/package.json index 49e7b13..e930b25 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sentry-api", - "version": "0.1.0", + "version": "0.1.1-jmb1", "description": "A client for the Sentry API", "main": "index.js", "scripts": { From fb293146f595fd6ef8a18caa9a431066a00ff447 Mon Sep 17 00:00:00 2001 From: jbellenger Date: Mon, 24 Apr 2017 15:37:39 -0700 Subject: [PATCH 4/4] . --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e930b25..d726280 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sentry-api", - "version": "0.1.1-jmb1", + "version": "0.1.1", "description": "A client for the Sentry API", "main": "index.js", "scripts": {