From 902b4356fb9dfc8814d71dfc97b63ec302cfab0d Mon Sep 17 00:00:00 2001 From: Michael Heap Date: Thu, 30 Nov 2017 15:22:55 +0000 Subject: [PATCH 1/4] Add ability to search SMS Resolves #139 --- src/Message.js | 32 ++++++++++++ src/index.js | 2 + test/Message-test.js | 103 +++++++++++++++++++++++++++++++++++++ test/ResourceTestHelper.js | 26 ++++++++-- 4 files changed, 159 insertions(+), 4 deletions(-) diff --git a/src/Message.js b/src/Message.js index d3cd0d0a..83f31532 100644 --- a/src/Message.js +++ b/src/Message.js @@ -1,5 +1,6 @@ "use strict"; +import querystring from "querystring"; import nexmo from "./index"; class Message { @@ -64,6 +65,37 @@ class Message { shortcodeMarketing() { this._nexmo.shortcodeMarketing.apply(this._nexmo, arguments); } + + search(id, callback) { + this.options.httpClient.request( + this.getRequestConfig("GET", "/search/message", { id: id }), + callback + ); + } + + getRequestConfig(verb, path, params) { + params["api_key"] = this.options.auth.api_key; + params["api_secret"] = this.options.auth.api_secret; + + var bodyParams = ""; + params = querystring.stringify(params); + + if (verb === "GET") { + path += "?" + params; + } else { + bodyParams = params; + } + + return { + host: "rest.nexmo.com", + path: path, + method: verb, + body: bodyParams, + headers: { + "Content-Type": "application/json" + } + }; + } } export default Message; diff --git a/src/index.js b/src/index.js index 56b20736..e8f93cc3 100644 --- a/src/index.js +++ b/src/index.js @@ -73,6 +73,8 @@ exports.initialize = function(pkey, psecret, options) { api_key: pkey, api_secret: psecret }; + + options.auth = up; _options = options; }; diff --git a/test/Message-test.js b/test/Message-test.js index 4463e103..663ed0f4 100644 --- a/test/Message-test.js +++ b/test/Message-test.js @@ -1,6 +1,13 @@ import Message from "../lib/Message"; +import Credentials from "../lib/Credentials"; +import HttpClient from "../lib/HttpClient"; +import NullLogger from "../lib/ConsoleLogger.js"; import NexmoStub from "./NexmoStub"; +import ResourceTestHelper from "./ResourceTestHelper"; + +import sinon from "sinon"; +import chai, { expect } from "chai"; var smsAPIs = { sendBinaryMessage: "sendBinaryMessage", @@ -20,3 +27,99 @@ describe("Message Object", function() { NexmoStub.checkAllFunctionsAreCalled(smsAPIs, Message); }); }); + +describe("Message", function() { + beforeEach(function() { + var creds = Credentials.parse({ + apiKey: "myKey", + apiSecret: "mySecret" + }); + + this.httpClientStub = sinon.createStubInstance(HttpClient); + + var options = { + httpClient: this.httpClientStub + }; + + this.message = new Message(creds, options); + }); + + describe("#search", function() { + it("should call the correct endpoint", function(done) { + this.httpClientStub.request.yields(null, { a: "b" }); + + var expectedRequestArgs = ResourceTestHelper.requestArgsMatch(null, { + host: "rest.nexmo.com", + path: "/search/message?id=0D00000068264896", + method: "GET", + body: "", + headers: { + "Content-Type": "application/json" + } + }); + + this.message.search( + "0D00000068264896", + function(err, data) { + expect(this.httpClientStub.request).to.have.been.calledWith( + sinon.match(expectedRequestArgs) + ); + + done(); + }.bind(this) + ); + }); + + it("returns data on a successful request", function(done) { + const mockData = { + "message-id": "0D00000068264896", + "account-id": "abc123", + network: "23430", + from: "TestTest", + to: "442079460000", + body: "Hello", + price: "0.03330000", + "date-received": "2017-11-24 15:09:30", + "final-status": "DELIVRD", + "date-closed": "2017-11-24 15:09:45", + latency: 14806, + type: "MT" + }; + + this.httpClientStub.request.yields(null, mockData); + this.message.search("0D00000068264896", function(err, data) { + expect(err).to.eql(null); + expect(data).to.eql(mockData); + done(); + }); + }); + + it("returns an error when the connection fails", function(done) { + const mockError = { + body: { + "error-code": "401", + "error-code-label": "authentication failed" + }, + headers: { + "content-type": "application/json;charset=UTF-8", + date: "Thu, 30 Nov 2017 14:41:50 GMT", + server: "nginx", + "strict-transport-security": "max-age=31536000; includeSubdomains", + "x-frame-options": "deny", + "x-nexmo-trace-id": "91f401d459aa5050af280aee53288135", + "x-xss-protection": "1; mode=block;", + "content-length": "63", + connection: "close" + }, + statusCode: 401 + }; + + this.httpClientStub.request.yields(mockError, null); + this.message.search("0D00000068264896", function(err, data) { + expect(err).to.eql(mockError); + expect(data).to.eql(null); + done(); + }); + }); + }); +}); diff --git a/test/ResourceTestHelper.js b/test/ResourceTestHelper.js index 3ad41bdc..8e702074 100644 --- a/test/ResourceTestHelper.js +++ b/test/ResourceTestHelper.js @@ -1,3 +1,5 @@ +import querystring from "querystring"; + class ResourceTestHelper { static getRequestArgs(params, overrides = {}) { var callsArgs = { @@ -30,15 +32,31 @@ class ResourceTestHelper { requestOverrides ); + // We strip api_key and api_secret out of `path` so that our tests + // only look for specific parameters + var qs = actual.path.split("?"); + var qsParts = querystring.parse(qs[1]); + delete qsParts["api_key"]; + delete qsParts["api_secret"]; + if (Object.keys(qsParts).length) { + actual.path = qs[0] + "?" + querystring.stringify(qsParts); + } + var match = expected.host === actual.host && expected.path === actual.path && expected.method === actual.method && expected.body === actual.body && - expected.headers["Content-Type"] === actual.headers["Content-Type"] && - actual.headers["Authorization"].indexOf( - expected.headers["Authorization"] - ) === 0; + expected.headers["Content-Type"] === actual.headers["Content-Type"]; + + // Some requests don't use the auth header, so only check if it's set + if (actual.headers["Authorization"]) { + match = + match && + actual.headers["Authorization"].indexOf( + expected.headers["Authorization"] + ) === 0; + } return match; }; } From 3b3122d7906099e57077ec1e89e2af3b28f70a72 Mon Sep 17 00:00:00 2001 From: Michael Heap Date: Fri, 1 Dec 2017 15:39:01 +0000 Subject: [PATCH 2/4] Add `get` method to HttpClient There are two new HttpClient instances available: * this.options.api (api.nexmo.com) * this.options.rest (rest.nexmo.com) If you specify `host` in `options` when running `new Nexmo()` you can override the URL used --- src/HttpClient.js | 17 +++++++++++++++-- src/Message.js | 30 +----------------------------- src/Nexmo.js | 17 ++++++++++++++++- src/index.js | 2 -- test/Message-test.js | 2 +- 5 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/HttpClient.js b/src/HttpClient.js index 19dea744..9ac74daf 100644 --- a/src/HttpClient.js +++ b/src/HttpClient.js @@ -1,8 +1,11 @@ var https = require("https"); var http = require("http"); +var querystring = require("querystring"); class HttpClient { - constructor(options) { + constructor(options, credentials) { + this.credentials = credentials; + this.host = options.host || "rest.nexmo.com"; this.port = options.port || 443; this.https = options.https || https; this.http = options.http || http; @@ -32,7 +35,7 @@ class HttpClient { // headers['Content-Length'] = 0; } var options = { - host: endpoint.host ? endpoint.host : "rest.nexmo.com", + host: endpoint.host ? endpoint.host : this.host, port: this.port, path: endpoint.path, method: endpoint.method, @@ -163,6 +166,16 @@ class HttpClient { callback(error, response); } } + + get(path, params, callback) { + params = params || {}; + params["api_key"] = this.credentials.apiKey; + params["api_secret"] = this.credentials.apiSecret; + + path = path + "?" + querystring.stringify(params); + + this.request({ path: path }, "GET", callback); + } } export default HttpClient; diff --git a/src/Message.js b/src/Message.js index 83f31532..01e4cbe2 100644 --- a/src/Message.js +++ b/src/Message.js @@ -1,6 +1,5 @@ "use strict"; -import querystring from "querystring"; import nexmo from "./index"; class Message { @@ -67,34 +66,7 @@ class Message { } search(id, callback) { - this.options.httpClient.request( - this.getRequestConfig("GET", "/search/message", { id: id }), - callback - ); - } - - getRequestConfig(verb, path, params) { - params["api_key"] = this.options.auth.api_key; - params["api_secret"] = this.options.auth.api_secret; - - var bodyParams = ""; - params = querystring.stringify(params); - - if (verb === "GET") { - path += "?" + params; - } else { - bodyParams = params; - } - - return { - host: "rest.nexmo.com", - path: path, - method: verb, - body: bodyParams, - headers: { - "Content-Type": "application/json" - } - }; + this.options.rest.get("/search/message", { id: id }, callback); } } diff --git a/src/Nexmo.js b/src/Nexmo.js index 851ae6e4..8b43660f 100644 --- a/src/Nexmo.js +++ b/src/Nexmo.js @@ -55,7 +55,22 @@ class Nexmo { if (this.options.appendToUserAgent) { this.options.userAgent += ` ${this.options.appendToUserAgent}`; } - this.options.httpClient = new HttpClient(this.options); + + // This is legacy, everything should use rest or api going forward + this.options.httpClient = new HttpClient( + Object.assign({ host: "rest.nexmo.com" }, this.options), + this.credentials + ); + + // We have two different hosts, so we use two different HttpClients + this.options.api = new HttpClient( + Object.assign({ host: "api.nexmo.com" }, this.options), + this.credentials + ); + this.options.rest = new HttpClient( + Object.assign({ host: "rest.nexmo.com" }, this.options), + this.credentials + ); this.message = new Message(this.credentials, this.options); this.voice = new Voice(this.credentials, this.options); diff --git a/src/index.js b/src/index.js index e8f93cc3..56b20736 100644 --- a/src/index.js +++ b/src/index.js @@ -73,8 +73,6 @@ exports.initialize = function(pkey, psecret, options) { api_key: pkey, api_secret: psecret }; - - options.auth = up; _options = options; }; diff --git a/test/Message-test.js b/test/Message-test.js index 663ed0f4..390a4f74 100644 --- a/test/Message-test.js +++ b/test/Message-test.js @@ -38,7 +38,7 @@ describe("Message", function() { this.httpClientStub = sinon.createStubInstance(HttpClient); var options = { - httpClient: this.httpClientStub + rest: this.httpClientStub }; this.message = new Message(creds, options); From 11c45d09a0c4d7fd99415359cb60397d57910d67 Mon Sep 17 00:00:00 2001 From: Michael Heap Date: Fri, 1 Dec 2017 16:23:10 +0000 Subject: [PATCH 3/4] ResourceTestHelper should only look for fields that are expected --- test/Message-test.js | 21 ++++++++++---------- test/ResourceTestHelper.js | 40 ++++++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/test/Message-test.js b/test/Message-test.js index 390a4f74..f5cb5a22 100644 --- a/test/Message-test.js +++ b/test/Message-test.js @@ -35,7 +35,14 @@ describe("Message", function() { apiSecret: "mySecret" }); - this.httpClientStub = sinon.createStubInstance(HttpClient); + this.httpClientStub = new HttpClient( + { + logger: new NullLogger() + }, + creds + ); + + sinon.stub(this.httpClientStub, "request"); var options = { rest: this.httpClientStub @@ -46,16 +53,10 @@ describe("Message", function() { describe("#search", function() { it("should call the correct endpoint", function(done) { - this.httpClientStub.request.yields(null, { a: "b" }); + this.httpClientStub.request.yields(null, {}); - var expectedRequestArgs = ResourceTestHelper.requestArgsMatch(null, { - host: "rest.nexmo.com", - path: "/search/message?id=0D00000068264896", - method: "GET", - body: "", - headers: { - "Content-Type": "application/json" - } + var expectedRequestArgs = ResourceTestHelper.requestArgsMatch({ + path: "/search/message?id=0D00000068264896" }); this.message.search( diff --git a/test/ResourceTestHelper.js b/test/ResourceTestHelper.js index 8e702074..b56770c6 100644 --- a/test/ResourceTestHelper.js +++ b/test/ResourceTestHelper.js @@ -27,10 +27,12 @@ class ResourceTestHelper { static requestArgsMatch(params, requestOverrides) { return function(actual) { - var expected = ResourceTestHelper.getRequestArgs( - params, - requestOverrides - ); + var expected; + if (requestOverrides) { + expected = ResourceTestHelper.getRequestArgs(params, requestOverrides); + } else { + expected = params; + } // We strip api_key and api_secret out of `path` so that our tests // only look for specific parameters @@ -42,21 +44,35 @@ class ResourceTestHelper { actual.path = qs[0] + "?" + querystring.stringify(qsParts); } - var match = - expected.host === actual.host && - expected.path === actual.path && - expected.method === actual.method && - expected.body === actual.body && - expected.headers["Content-Type"] === actual.headers["Content-Type"]; + var match = true; + + // Check response parameters + ["host", "path", "method", "body"].forEach(function(k) { + if (expected[k]) { + match = match && expected[k] == actual[k]; + } + }); - // Some requests don't use the auth header, so only check if it's set - if (actual.headers["Authorization"]) { + // Also check for any headers that we're expecting + expected.headers = expected.headers || {}; + Object.keys(expected.headers).forEach(function(k) { + // We have a special check for authorization + if (k === "Authorization") { + return true; + } + match = match && expected.headers[k] === actual.headers[k]; + }); + + // For Authorization we only check the beginning as JWTs are + // dynamically created + if (expected.headers["Authorization"]) { match = match && actual.headers["Authorization"].indexOf( expected.headers["Authorization"] ) === 0; } + return match; }; } From 31ad18b534b90fb082400d2e74db94f11be5dff5 Mon Sep 17 00:00:00 2001 From: Michael Heap Date: Fri, 1 Dec 2017 18:18:09 +0000 Subject: [PATCH 4/4] Add support for sending a list of IDs --- src/Message.js | 7 +++++- test/Message-test.js | 52 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/Message.js b/src/Message.js index 01e4cbe2..5c61b4cf 100644 --- a/src/Message.js +++ b/src/Message.js @@ -66,7 +66,12 @@ class Message { } search(id, callback) { - this.options.rest.get("/search/message", { id: id }, callback); + if (typeof id == "string") { + return this.options.rest.get("/search/message", { id: id }, callback); + } + + // Otherwise we expect an array + return this.options.rest.get("/search/messages", { ids: id }, callback); } } diff --git a/test/Message-test.js b/test/Message-test.js index f5cb5a22..68019485 100644 --- a/test/Message-test.js +++ b/test/Message-test.js @@ -52,7 +52,7 @@ describe("Message", function() { }); describe("#search", function() { - it("should call the correct endpoint", function(done) { + it("should call the correct endpoint (single)", function(done) { this.httpClientStub.request.yields(null, {}); var expectedRequestArgs = ResourceTestHelper.requestArgsMatch({ @@ -71,7 +71,26 @@ describe("Message", function() { ); }); - it("returns data on a successful request", function(done) { + it("should call the correct endpoint (multiple)", function(done) { + this.httpClientStub.request.yields(null, {}); + + var expectedRequestArgs = ResourceTestHelper.requestArgsMatch({ + path: "/search/messages?ids=1&ids=2" + }); + + this.message.search( + [1, 2], + function(err, data) { + expect(this.httpClientStub.request).to.have.been.calledWith( + sinon.match(expectedRequestArgs) + ); + + done(); + }.bind(this) + ); + }); + + it("returns data on a successful request (single)", function(done) { const mockData = { "message-id": "0D00000068264896", "account-id": "abc123", @@ -95,6 +114,35 @@ describe("Message", function() { }); }); + it("returns data on a successful request (multiple)", function(done) { + const mockData = { + count: 1, + items: [ + { + "message-id": "0D00000068264896", + "account-id": "abc123", + network: "23430", + from: "TestTest", + to: "442079460000", + body: "Hello", + price: "0.03330000", + "date-received": "2017-11-24 15:09:30", + "final-status": "DELIVRD", + "date-closed": "2017-11-24 15:09:45", + latency: 14806, + type: "MT" + } + ] + }; + + this.httpClientStub.request.yields(null, mockData); + this.message.search(["0D00000068264896"], function(err, data) { + expect(err).to.eql(null); + expect(data).to.eql(mockData); + done(); + }); + }); + it("returns an error when the connection fails", function(done) { const mockError = { body: {