From 03d2f2c79e07777e0a81bf8d0ca17848a1c34292 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Wed, 29 Apr 2020 21:01:06 -0700 Subject: [PATCH 1/9] feat: add regional and edge support --- src/base-commands/twilio-client-command.js | 1 + src/services/config.js | 28 ++- src/services/open-api-client.js | 11 +- src/services/twilio-api/twilio-client.js | 11 +- .../twilio-client-command.test.js | 2 + test/services/config.test.js | 18 +- .../services/twilio-api/twilio-client.test.js | 194 ++++++++++++++++++ 7 files changed, 248 insertions(+), 17 deletions(-) diff --git a/src/base-commands/twilio-client-command.js b/src/base-commands/twilio-client-command.js index 95bd8c19..01de58d5 100644 --- a/src/base-commands/twilio-client-command.js +++ b/src/base-commands/twilio-client-command.js @@ -111,6 +111,7 @@ class TwilioClientCommand extends BaseCommand { buildClient(ClientClass) { return new ClientClass(this.currentProfile.apiKey, this.currentProfile.apiSecret, { accountSid: this.flags[CliFlags.ACCOUNT_SID] || this.currentProfile.accountSid, + edge: this.currentProfile.edge, region: this.currentProfile.region, httpClient: this.httpClient }); diff --git a/src/services/config.js b/src/services/config.js index 76b7e8e4..da9ba154 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -6,10 +6,11 @@ const MessageTemplates = require('./messaging/templates'); const CLI_NAME = 'twilio-cli'; class ConfigDataProfile { - constructor(id, accountSid, region) { + constructor(id, accountSid, region, edge) { this.id = id; this.accountSid = accountSid; this.region = region; + this.edge = edge; } } @@ -22,7 +23,14 @@ class ConfigData { } getProfileFromEnvironment() { - const { TWILIO_ACCOUNT_SID, TWILIO_AUTH_TOKEN, TWILIO_API_KEY, TWILIO_API_SECRET } = process.env; + const { + TWILIO_ACCOUNT_SID, + TWILIO_AUTH_TOKEN, + TWILIO_API_KEY, + TWILIO_API_SECRET, + TWILIO_REGION, + TWILIO_EDGE + } = process.env; if (!TWILIO_ACCOUNT_SID) return; if (TWILIO_API_KEY && TWILIO_API_SECRET) @@ -31,7 +39,9 @@ class ConfigData { id: '${TWILIO_API_KEY}/${TWILIO_API_SECRET}', accountSid: TWILIO_ACCOUNT_SID, apiKey: TWILIO_API_KEY, - apiSecret: TWILIO_API_SECRET + apiSecret: TWILIO_API_SECRET, + region: TWILIO_REGION, + edge: TWILIO_EDGE }; if (TWILIO_AUTH_TOKEN) @@ -40,7 +50,9 @@ class ConfigData { id: '${TWILIO_ACCOUNT_SID}/${TWILIO_AUTH_TOKEN}', accountSid: TWILIO_ACCOUNT_SID, apiKey: TWILIO_ACCOUNT_SID, - apiSecret: TWILIO_AUTH_TOKEN + apiSecret: TWILIO_AUTH_TOKEN, + region: TWILIO_REGION, + edge: TWILIO_EDGE }; } @@ -97,18 +109,20 @@ class ConfigData { } } - addProfile(id, accountSid, region) { + addProfile(id, accountSid, region, edge) { // Clean all the inputs. id = this.sanitize(id); accountSid = this.sanitize(accountSid); region = this.sanitize(region); + edge = this.sanitize(edge); const existing = this.getProfileById(id); if (existing) { existing.accountSid = accountSid; existing.region = region; + existing.edge = edge; } else { - this.profiles.push(new ConfigDataProfile(id, accountSid, region)); + this.profiles.push(new ConfigDataProfile(id, accountSid, region, edge)); } } @@ -134,7 +148,7 @@ class ConfigData { this.prompts = configObj.prompts || {}; // Note the historical 'projects' naming. configObj.profiles = configObj.projects || []; - configObj.profiles.forEach(profile => this.addProfile(profile.id, profile.accountSid, profile.region)); + configObj.profiles.forEach(profile => this.addProfile(profile.id, profile.accountSid, profile.region, profile.edge)); this.setActiveProfile(configObj.activeProject); } diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index 7cae1f39..d9a0abeb 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -44,13 +44,10 @@ class OpenApiClient { } if (opts.region) { - const parts = opts.host.split('.'); - - // From 'https://api.twilio.com/' to 'https://api.{region}.twilio.com/' - if (parts.length > 1 && parts[1] !== opts.region) { - parts.splice(1, 0, opts.region); - opts.host = parts.join('.'); - } + const domain = opts.host.split('.').slice(-2).join('.'); + const prefix = opts.host.split('.' + domain)[0]; + let product = prefix.split('.')[0]; + opts.host = [product, opts.edge, opts.region, domain].filter(part => part).join('.'); } opts.uri = opts.host + opts.uri; diff --git a/src/services/twilio-api/twilio-client.js b/src/services/twilio-api/twilio-client.js index 43ac4181..b6dca07a 100644 --- a/src/services/twilio-api/twilio-client.js +++ b/src/services/twilio-api/twilio-client.js @@ -20,6 +20,7 @@ class TwilioApiClient { this.username = username; this.password = password; this.accountSid = opts.accountSid || this.username; + this.edge = opts.edge; this.region = opts.region; this.apiClient = new OpenApiClient({ @@ -148,8 +149,9 @@ class TwilioApiClient { * @param {object} opts - The options argument * @param {string} opts.method - The http method * @param {string} opts.path - The request path + * @param {string} [opts.edge] - The request edge. Defaults to none. * @param {string} [opts.host] - The request host - * @param {string} [opts.region] - The request region + * @param {string} [opts.region] - The request region. Default to us1 if edge defined * @param {string} [opts.uri] - The request uri * @param {string} [opts.username] - The username used for auth * @param {string} [opts.password] - The password used for auth @@ -164,7 +166,6 @@ class TwilioApiClient { opts.username = opts.username || this.username; opts.password = opts.password || this.password; - opts.region = opts.region || this.region; opts.headers = opts.headers || {}; opts.data = opts.data || {}; opts.pathParams = opts.pathParams || {}; @@ -186,6 +187,12 @@ class TwilioApiClient { } } + opts.edge = this.edge || opts.edge; + if (this.region) { + opts.region = this.region; + } else if (this.edge || (opts.edge && !opts.region)) { + opts.region = 'us1'; + } return this.apiClient.request(opts); } } diff --git a/test/base-commands/twilio-client-command.test.js b/test/base-commands/twilio-client-command.test.js index b6890ef9..c7396e7e 100644 --- a/test/base-commands/twilio-client-command.test.js +++ b/test/base-commands/twilio-client-command.test.js @@ -65,6 +65,7 @@ describe('base-commands', () => { expect(ctx.testCmd.twilioClient.username).to.equal(constants.FAKE_API_KEY); expect(ctx.testCmd.twilioClient.password).to.equal(constants.FAKE_API_SECRET + 'MyFirstProfile'); expect(ctx.testCmd.twilioClient.region).to.equal(undefined); + expect(ctx.testCmd.twilioClient.edge).to.equal(undefined); }); setUpTest(['-l', 'debug', '--account-sid', 'ACbaccbaccbaccbaccbaccbaccbaccbacc'], { commandClass: AccountSidClientCommand }).it( @@ -75,6 +76,7 @@ describe('base-commands', () => { expect(ctx.testCmd.twilioClient.username).to.equal(constants.FAKE_API_KEY); expect(ctx.testCmd.twilioClient.password).to.equal(constants.FAKE_API_SECRET + 'MyFirstProfile'); expect(ctx.testCmd.twilioClient.region).to.equal(undefined); + expect(ctx.testCmd.twilioClient.edge).to.equal(undefined); } ); diff --git a/test/services/config.test.js b/test/services/config.test.js index 7b3bd623..ebc8dec3 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -76,7 +76,6 @@ describe('services', () => { expect(profile.apiKey).to.equal(constants.FAKE_ACCOUNT_SID); expect(profile.apiSecret).to.equal(FAKE_AUTH_TOKEN); }); - test.it('should return profile populated from AccountSid/ApiKey env vars', () => { const configData = new ConfigData(); configData.addProfile('envProfile', constants.FAKE_ACCOUNT_SID); @@ -91,6 +90,23 @@ describe('services', () => { expect(profile.apiKey).to.equal(constants.FAKE_API_KEY); expect(profile.apiSecret).to.equal(constants.FAKE_API_SECRET); }); + + test.it('should return profile populated with region/edge env vars', () => { + const configData = new ConfigData(); + configData.addProfile('envProfile', constants.FAKE_ACCOUNT_SID); + + process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; + process.env.TWILIO_AUTH_TOKEN = FAKE_AUTH_TOKEN; + process.env.TWILIO_REGION = 'region'; + process.env.TWILIO_EDGE = 'edge'; + + const profile = configData.getProfileById(); + expect(profile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID); + expect(profile.apiKey).to.equal(constants.FAKE_ACCOUNT_SID); + expect(profile.apiSecret).to.equal(FAKE_AUTH_TOKEN); + expect(profile.region).to.equal('region'); + expect(profile.edge).to.equal('edge'); + }); }); describe('ConfigData.activeProfile', () => { diff --git a/test/services/twilio-api/twilio-client.test.js b/test/services/twilio-api/twilio-client.test.js index c1363531..03d3ee7b 100644 --- a/test/services/twilio-api/twilio-client.test.js +++ b/test/services/twilio-api/twilio-client.test.js @@ -265,6 +265,200 @@ describe('services', () => { expect(options.PageSize).to.be.undefined; }); }); + + describe('regional and edge support', () => { + let regionTest = test + .nock('https://api.edge.us1.twilio.com', api => { + api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { + status: 'queued' + }); + }); + + regionTest + .it('uses the default region if only edge is defined', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest + .it('uses the default region if only edge is defined and region is provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest + .it('uses the default region if only edge is defined and edge and region are provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge2', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest + .it('uses the default region if edge is provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest = test.nock('https://api.region.twilio.com', api => { + api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { + status: 'queued' + }); + }); + + regionTest.it('uses the client region if defined', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the client region if defined and region is provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + region: 'region2' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the provided region', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest = + test.nock('https://api.edge.region.twilio.com', api => { + api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { + status: 'queued' + }); + }); + + regionTest.it('should set the region and edge properly', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge', region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the client region and edge when edge provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge', region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge2' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the client region and edge when region provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge', region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + region: 'region2' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the provided region and edge', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest.it('uses the client region and provided edge when edge and region provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge', + region: 'region2' + }); + expect(response).to.eql({ status: 'queued' }); + }); + }); }); }); }); From 1490ad4fd78c2b8b51c30924912afc7568ff0c93 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Thu, 30 Apr 2020 09:40:08 -0700 Subject: [PATCH 2/9] update regional use --- src/services/twilio-api/twilio-client.js | 5 +- .../services/twilio-api/twilio-client.test.js | 62 +++++++++---------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/src/services/twilio-api/twilio-client.js b/src/services/twilio-api/twilio-client.js index b6dca07a..eca80c58 100644 --- a/src/services/twilio-api/twilio-client.js +++ b/src/services/twilio-api/twilio-client.js @@ -188,9 +188,8 @@ class TwilioApiClient { } opts.edge = this.edge || opts.edge; - if (this.region) { - opts.region = this.region; - } else if (this.edge || (opts.edge && !opts.region)) { + opts.region = this.region || opts.region; + if (opts.edge && !opts.region) { opts.region = 'us1'; } return this.apiClient.request(opts); diff --git a/test/services/twilio-api/twilio-client.test.js b/test/services/twilio-api/twilio-client.test.js index 03d3ee7b..79021ed9 100644 --- a/test/services/twilio-api/twilio-client.test.js +++ b/test/services/twilio-api/twilio-client.test.js @@ -288,37 +288,6 @@ describe('services', () => { expect(response).to.eql({ status: 'queued' }); }); - regionTest - .it('uses the default region if only edge is defined and region is provided', async () => { - const client = new TwilioApiClient( - constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, edge: 'edge' } - ); - - const response = await client.create({ - domain: 'api', - path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - region: 'region' - }); - expect(response).to.eql({ status: 'queued' }); - }); - - regionTest - .it('uses the default region if only edge is defined and edge and region are provided', async () => { - const client = new TwilioApiClient( - constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, edge: 'edge' } - ); - - const response = await client.create({ - domain: 'api', - path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - edge: 'edge2', - region: 'region' - }); - expect(response).to.eql({ status: 'queued' }); - }); - regionTest .it('uses the default region if edge is provided', async () => { const client = new TwilioApiClient( @@ -458,6 +427,37 @@ describe('services', () => { }); expect(response).to.eql({ status: 'queued' }); }); + + regionTest + .it('uses the provided region if only edge is defined and region is provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + regionTest + .it('uses the provided region if only edge is defined and edge and region are provided', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, edge: 'edge' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + edge: 'edge2', + region: 'region' + }); + expect(response).to.eql({ status: 'queued' }); + }); }); }); }); From e9ca6eb18761e0c0bc69e74382aa0c86e805c6f5 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Thu, 30 Apr 2020 18:50:09 -0700 Subject: [PATCH 3/9] fix precedence --- src/base-commands/twilio-client-command.js | 2 +- src/services/config.js | 23 ++-- src/services/open-api-client.js | 41 +++++-- src/services/twilio-api/twilio-client.js | 15 +-- test/services/config.test.js | 15 ++- .../services/twilio-api/twilio-client.test.js | 112 +++++++++++------- 6 files changed, 130 insertions(+), 78 deletions(-) diff --git a/src/base-commands/twilio-client-command.js b/src/base-commands/twilio-client-command.js index 01de58d5..bcc15df2 100644 --- a/src/base-commands/twilio-client-command.js +++ b/src/base-commands/twilio-client-command.js @@ -111,7 +111,7 @@ class TwilioClientCommand extends BaseCommand { buildClient(ClientClass) { return new ClientClass(this.currentProfile.apiKey, this.currentProfile.apiSecret, { accountSid: this.flags[CliFlags.ACCOUNT_SID] || this.currentProfile.accountSid, - edge: this.currentProfile.edge, + edge: process.env.TWILIO_EDGE || this.userConfig.edge, region: this.currentProfile.region, httpClient: this.httpClient }); diff --git a/src/services/config.js b/src/services/config.js index da9ba154..f7949a52 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -6,16 +6,16 @@ const MessageTemplates = require('./messaging/templates'); const CLI_NAME = 'twilio-cli'; class ConfigDataProfile { - constructor(id, accountSid, region, edge) { + constructor(id, accountSid, region) { this.id = id; this.accountSid = accountSid; this.region = region; - this.edge = edge; } } class ConfigData { constructor() { + this.edge = process.env.TWILIO_EDGE; this.email = {}; this.prompts = {}; this.profiles = []; @@ -28,8 +28,7 @@ class ConfigData { TWILIO_AUTH_TOKEN, TWILIO_API_KEY, TWILIO_API_SECRET, - TWILIO_REGION, - TWILIO_EDGE + TWILIO_REGION } = process.env; if (!TWILIO_ACCOUNT_SID) return; @@ -40,8 +39,7 @@ class ConfigData { accountSid: TWILIO_ACCOUNT_SID, apiKey: TWILIO_API_KEY, apiSecret: TWILIO_API_SECRET, - region: TWILIO_REGION, - edge: TWILIO_EDGE + region: TWILIO_REGION }; if (TWILIO_AUTH_TOKEN) @@ -51,8 +49,7 @@ class ConfigData { accountSid: TWILIO_ACCOUNT_SID, apiKey: TWILIO_ACCOUNT_SID, apiSecret: TWILIO_AUTH_TOKEN, - region: TWILIO_REGION, - edge: TWILIO_EDGE + region: TWILIO_REGION }; } @@ -109,20 +106,18 @@ class ConfigData { } } - addProfile(id, accountSid, region, edge) { + addProfile(id, accountSid, region) { // Clean all the inputs. id = this.sanitize(id); accountSid = this.sanitize(accountSid); region = this.sanitize(region); - edge = this.sanitize(edge); const existing = this.getProfileById(id); if (existing) { existing.accountSid = accountSid; existing.region = region; - existing.edge = edge; } else { - this.profiles.push(new ConfigDataProfile(id, accountSid, region, edge)); + this.profiles.push(new ConfigDataProfile(id, accountSid, region)); } } @@ -144,11 +139,12 @@ class ConfigData { } loadFromObject(configObj) { + this.edge = this.edge || configObj.edge; this.email = configObj.email || {}; this.prompts = configObj.prompts || {}; // Note the historical 'projects' naming. configObj.profiles = configObj.projects || []; - configObj.profiles.forEach(profile => this.addProfile(profile.id, profile.accountSid, profile.region, profile.edge)); + configObj.profiles.forEach(profile => this.addProfile(profile.id, profile.accountSid, profile.region)); this.setActiveProfile(configObj.activeProject); } @@ -177,6 +173,7 @@ class Config { async save(configData) { configData = { + edge: configData.edge, email: configData.email, prompts: configData.prompts, // Note the historical 'projects' naming. diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index d9a0abeb..8fb27ff7 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -1,3 +1,4 @@ +const url = require('url'); const { logger } = require('./messaging/logging'); const { doesObjectHaveProperty } = require('./javascript-utilities'); const JsonSchemaConverter = require('./api-schema/json-converter'); @@ -34,7 +35,7 @@ class OpenApiClient { const params = this.getParams(opts, operation); if (!opts.uri) { - opts.uri = this.getUri(opts); + opts.uri = this.getUri(opts.uri, opts); } // If the URI is relative, determine the host and prepend it. @@ -42,15 +43,13 @@ class OpenApiClient { if (!opts.host) { opts.host = path.server; } - - if (opts.region) { - const domain = opts.host.split('.').slice(-2).join('.'); - const prefix = opts.host.split('.' + domain)[0]; - let product = prefix.split('.')[0]; - opts.host = [product, opts.edge, opts.region, domain].filter(part => part).join('.'); - } - + opts.host = this.getHost(opts.host, opts); opts.uri = opts.host + opts.uri; + } else if (opts.uri) { + let uri = new url.URL(opts.uri); + uri.hostname = this.getHost(uri.hostname, opts); + uri.pathname = this.getUri(uri.pathname, opts); + opts.uri = uri.href; } opts.params = (isPost ? null : params); @@ -78,7 +77,10 @@ class OpenApiClient { return params; } - getUri(opts) { + getUri(path, opts) { + if (path && path !== '/') { + return path; + } // Evaluate the request path by replacing path parameters with their value // from the request data. return opts.path.replace(/{(.+?)}/g, (fullMatch, pathNode) => { @@ -94,6 +96,25 @@ class OpenApiClient { }); } + getHost(host, opts) { + if (opts.region || opts.edge) { + const domain = host.split('.').slice(-2).join('.'); + const prefix = host.split('.' + domain)[0]; + let [product, edge, region] = prefix.split('.'); + if (edge && !region) { + region = edge; + edge = undefined; + } + opts.edge = opts.edge || edge; + opts.region = opts.region || region || (opts.edge ? 'us1' : undefined); + return [product, + opts.edge, + opts.region, + domain].filter(part => part).join('.'); + } + return host; + } + parseResponse(domain, operation, response, requestOpts) { if (response.body) { const responseSchema = this.getResponseSchema(domain, operation, response.statusCode, requestOpts.headers.Accept); diff --git a/src/services/twilio-api/twilio-client.js b/src/services/twilio-api/twilio-client.js index eca80c58..818f6e8f 100644 --- a/src/services/twilio-api/twilio-client.js +++ b/src/services/twilio-api/twilio-client.js @@ -149,8 +149,8 @@ class TwilioApiClient { * @param {object} opts - The options argument * @param {string} opts.method - The http method * @param {string} opts.path - The request path - * @param {string} [opts.edge] - The request edge. Defaults to none. * @param {string} [opts.host] - The request host + * @param {string} [opts.edge] - The request edge. Defaults to none. * @param {string} [opts.region] - The request region. Default to us1 if edge defined * @param {string} [opts.uri] - The request uri * @param {string} [opts.username] - The username used for auth @@ -181,17 +181,12 @@ class TwilioApiClient { opts.headers.Accept = 'application/json'; } - if (!opts.uri) { - if (opts.path.includes(TwilioApiFlags.ACCOUNT_SID) && !doesObjectHaveProperty(opts.pathParams, TwilioApiFlags.ACCOUNT_SID)) { - opts.pathParams[TwilioApiFlags.ACCOUNT_SID] = this.accountSid; - } + if (opts.path.includes(TwilioApiFlags.ACCOUNT_SID) && !doesObjectHaveProperty(opts.pathParams, TwilioApiFlags.ACCOUNT_SID)) { + opts.pathParams[TwilioApiFlags.ACCOUNT_SID] = this.accountSid; } - opts.edge = this.edge || opts.edge; - opts.region = this.region || opts.region; - if (opts.edge && !opts.region) { - opts.region = 'us1'; - } + opts.edge = opts.edge || this.edge; + opts.region = opts.region || this.region; return this.apiClient.request(opts); } } diff --git a/test/services/config.test.js b/test/services/config.test.js index ebc8dec3..6e3f3ce3 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -91,21 +91,19 @@ describe('services', () => { expect(profile.apiSecret).to.equal(constants.FAKE_API_SECRET); }); - test.it('should return profile populated with region/edge env vars', () => { + test.it('should return profile populated with region env var', () => { const configData = new ConfigData(); configData.addProfile('envProfile', constants.FAKE_ACCOUNT_SID); process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; process.env.TWILIO_AUTH_TOKEN = FAKE_AUTH_TOKEN; process.env.TWILIO_REGION = 'region'; - process.env.TWILIO_EDGE = 'edge'; const profile = configData.getProfileById(); expect(profile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID); expect(profile.apiKey).to.equal(constants.FAKE_ACCOUNT_SID); expect(profile.apiSecret).to.equal(FAKE_AUTH_TOKEN); expect(profile.region).to.equal('region'); - expect(profile.edge).to.equal('edge'); }); }); @@ -221,6 +219,17 @@ describe('services', () => { const saveMessage = await config.save(userConfig); expect(saveMessage).to.contain(`${nestedConfig}${path.sep}config.json`); }); + + test.it('uses env vars over config to set edge', async () => { + const config = new Config(tempConfigDir.name); + const userConfig = await config.load(); + + expect(userConfig.edge).to.be.undefined; + process.env.TWILIO_EDGE = 'edge'; + + const loadedConfig = await config.load(); + expect(loadedConfig.edge).to.equal('edge'); + }); }); }); }); diff --git a/test/services/twilio-api/twilio-client.test.js b/test/services/twilio-api/twilio-client.test.js index 79021ed9..cdbd3e67 100644 --- a/test/services/twilio-api/twilio-client.test.js +++ b/test/services/twilio-api/twilio-client.test.js @@ -267,14 +267,14 @@ describe('services', () => { }); describe('regional and edge support', () => { - let regionTest = test + const defaultRegionTest = test .nock('https://api.edge.us1.twilio.com', api => { api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { status: 'queued' }); }); - regionTest + defaultRegionTest .it('uses the default region if only edge is defined', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, @@ -288,7 +288,7 @@ describe('services', () => { expect(response).to.eql({ status: 'queued' }); }); - regionTest + defaultRegionTest .it('uses the default region if edge is provided', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, @@ -303,7 +303,7 @@ describe('services', () => { expect(response).to.eql({ status: 'queued' }); }); - regionTest = test.nock('https://api.region.twilio.com', api => { + const regionTest = test.nock('https://api.region.twilio.com', api => { api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { status: 'queued' }); @@ -317,21 +317,25 @@ describe('services', () => { const response = await client.create({ domain: 'api', - path: '/2010-04-01/Accounts/{AccountSid}/Messages.json' + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + // Should ignore the region in the uri + uri: 'https://api.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest.it('uses the client region if defined and region is provided', async () => { + regionTest.it('uses the provided region if client region defined and region is provided', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, region: 'region' } + { accountSid, httpClient, region: 'region2' } ); const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - region: 'region2' + region: 'region', + // Should ignore the region in the uri + uri: 'https://api.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); @@ -345,32 +349,21 @@ describe('services', () => { const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - region: 'region' + region: 'region', + // Should ignore the region in the uri + uri: 'https://api.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest = + const edgeRegionTest = test.nock('https://api.edge.region.twilio.com', api => { api.post(`/2010-04-01/Accounts/${accountSid}/Messages.json`).reply(201, { status: 'queued' }); }); - regionTest.it('should set the region and edge properly', async () => { - const client = new TwilioApiClient( - constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, edge: 'edge', region: 'region' } - ); - - const response = await client.create({ - domain: 'api', - path: '/2010-04-01/Accounts/{AccountSid}/Messages.json' - }); - expect(response).to.eql({ status: 'queued' }); - }); - - regionTest.it('uses the client region and edge when edge provided', async () => { + edgeRegionTest.it('should set the region and edge properly', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, { accountSid, httpClient, edge: 'edge', region: 'region' } @@ -379,56 +372,62 @@ describe('services', () => { const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - edge: 'edge2' + // Should ignore the edge and region in the uri + uri: 'https://api.uriEdge.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest.it('uses the client region and edge when region provided', async () => { + edgeRegionTest.it('uses the client region and provided edge when edge provided', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, edge: 'edge', region: 'region' } + { accountSid, httpClient, edge: 'clientEdge', region: 'region' } ); const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - region: 'region2' + edge: 'edge', + // Should ignore the edge and region in the uri + uri: 'https://api.uriEdge.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest.it('uses the provided region and edge', async () => { + edgeRegionTest.it('uses the provided region and client edge when region provided', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient } + { accountSid, httpClient, edge: 'edge', region: 'clientRegion' } ); const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - edge: 'edge', - region: 'region' + region: 'region', + // Should ignore the edge and region in the uri + uri: 'https://api.uriEdge.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest.it('uses the client region and provided edge when edge and region provided', async () => { + edgeRegionTest.it('uses the provided region and edge', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, - { accountSid, httpClient, region: 'region' } + { accountSid, httpClient } ); const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', edge: 'edge', - region: 'region2' + region: 'region', + // Should ignore the edge and region in the uri + uri: 'https://api.uriEdge.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest + edgeRegionTest .it('uses the provided region if only edge is defined and region is provided', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, @@ -438,13 +437,15 @@ describe('services', () => { const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - region: 'region' + region: 'region', + // Should ignore the edge and region in the uri + uri: 'https://api.uriEdge.uriRegion.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); - regionTest - .it('uses the provided region if only edge is defined and edge and region are provided', async () => { + edgeRegionTest + .it('uses the uri region and client edge', async () => { const client = new TwilioApiClient( constants.FAKE_API_KEY, constants.FAKE_API_SECRET, { accountSid, httpClient, edge: 'edge' } @@ -453,11 +454,40 @@ describe('services', () => { const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - edge: 'edge2', - region: 'region' + // Should ignore the edge in the uri + uri: 'https://api.uriEdge.region.twilio.com' }); expect(response).to.eql({ status: 'queued' }); }); + + edgeRegionTest.it('uses the client region and uri edge', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient, region: 'region' } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + // Should ignore the region in the uri + uri: 'https://api.edge.uriRegion.twilio.com' + }); + expect(response).to.eql({ status: 'queued' }); + }); + + edgeRegionTest.it('uses the uri region and edge', async () => { + const client = new TwilioApiClient( + constants.FAKE_API_KEY, constants.FAKE_API_SECRET, + { accountSid, httpClient } + ); + + const response = await client.create({ + domain: 'api', + path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', + uri: 'https://api.edge.region.twilio.com' + }); + expect(response).to.eql({ status: 'queued' }); + }); }); }); }); From 68c7e689267917c5240e82445dc5b099a0400204 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Fri, 1 May 2020 09:38:51 -0700 Subject: [PATCH 4/9] for sam --- src/services/open-api-client.js | 2 +- test/services/config.test.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index 8fb27ff7..14a1fbf9 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -106,7 +106,7 @@ class OpenApiClient { edge = undefined; } opts.edge = opts.edge || edge; - opts.region = opts.region || region || (opts.edge ? 'us1' : undefined); + opts.region = opts.region || region || (opts.edge && 'us1'); return [product, opts.edge, opts.region, diff --git a/test/services/config.test.js b/test/services/config.test.js index 6e3f3ce3..5194fd01 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -35,6 +35,7 @@ describe('services', () => { const profile = configData.getProfileById('DOES_NOT_EXIST'); expect(profile).to.be.undefined; }); + test.it('should return undefined if no profiles, even with env vars', () => { const configData = new ConfigData(); process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; @@ -43,6 +44,7 @@ describe('services', () => { const profile = configData.getProfileById('DOES_NOT_EXIST'); expect(profile).to.be.undefined; }); + test.it('should return first profile if it exists, and no env vars', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); @@ -52,6 +54,7 @@ describe('services', () => { expect(profile.apiKey).to.be.undefined; expect(profile.apiSecret).to.be.undefined; }); + test.it('return the active profile if there are multiple profiles', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); @@ -64,6 +67,7 @@ describe('services', () => { expect(profile.apiKey).to.be.undefined; expect(profile.apiSecret).to.be.undefined; }); + test.it('should return profile populated from AccountSid/AuthToken env vars', () => { const configData = new ConfigData(); configData.addProfile('envProfile', constants.FAKE_ACCOUNT_SID); @@ -76,6 +80,7 @@ describe('services', () => { expect(profile.apiKey).to.equal(constants.FAKE_ACCOUNT_SID); expect(profile.apiSecret).to.equal(FAKE_AUTH_TOKEN); }); + test.it('should return profile populated from AccountSid/ApiKey env vars', () => { const configData = new ConfigData(); configData.addProfile('envProfile', constants.FAKE_ACCOUNT_SID); @@ -118,6 +123,7 @@ describe('services', () => { expect(active.id).to.equal('firstProfile'); expect(active.accountSid).to.equal(constants.FAKE_ACCOUNT_SID); }); + test.it('should return active profile when active profile has been set', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); @@ -129,12 +135,14 @@ describe('services', () => { expect(active.id).to.equal('secondProfile'); expect(active.accountSid).to.equal('new_account_SID'); }); + test.it('should not allow the active profile to not exist', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); expect(configData.setActiveProfile('secondProfile')).to.be.undefined; expect(configData.getActiveProfile().id).to.equal('firstProfile'); }); + test.it('should return undefined if profile does not exist and there are no profiles configured', () => { const configData = new ConfigData(); const active = configData.getActiveProfile(); @@ -157,6 +165,7 @@ describe('services', () => { expect(configData.profiles.length).to.equal(originalLength); }); + test.it('removes profile', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); @@ -168,6 +177,7 @@ describe('services', () => { expect(configData.profiles[1].id).to.equal('thirdProfile'); expect(configData.profiles[1].accountSid).to.equal('newest_account_SID'); }); + test.it('removes active profile', () => { const configData = new ConfigData(); configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID); From 0d61218af54ecc57f919a3d25cc1cb30cb568408 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Fri, 1 May 2020 09:57:16 -0700 Subject: [PATCH 5/9] clean up --- src/services/open-api-client.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index 14a1fbf9..c248bcf9 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -45,13 +45,13 @@ class OpenApiClient { } opts.host = this.getHost(opts.host, opts); opts.uri = opts.host + opts.uri; - } else if (opts.uri) { - let uri = new url.URL(opts.uri); - uri.hostname = this.getHost(uri.hostname, opts); - uri.pathname = this.getUri(uri.pathname, opts); - opts.uri = uri.href; } + let uri = new url.URL(opts.uri); + uri.hostname = this.getHost(uri.hostname, opts); + uri.pathname = this.getUri(uri.pathname, opts); + opts.uri = uri.href; + opts.params = (isPost ? null : params); opts.data = (isPost ? params : null); @@ -105,12 +105,9 @@ class OpenApiClient { region = edge; edge = undefined; } - opts.edge = opts.edge || edge; - opts.region = opts.region || region || (opts.edge && 'us1'); - return [product, - opts.edge, - opts.region, - domain].filter(part => part).join('.'); + edge = opts.edge || edge; + region = opts.region || region || (opts.edge && 'us1'); + return [product, edge, region, domain].filter(part => part).join('.'); } return host; } From d27adf57139dd180fec46b6841f9f000812dc41a Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Fri, 1 May 2020 12:09:34 -0700 Subject: [PATCH 6/9] fix edge config --- src/services/config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/config.js b/src/services/config.js index f7949a52..da69662e 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -15,7 +15,7 @@ class ConfigDataProfile { class ConfigData { constructor() { - this.edge = process.env.TWILIO_EDGE; + this.edge = undefined; this.email = {}; this.prompts = {}; this.profiles = []; @@ -166,7 +166,7 @@ class Config { if (!fs.existsSync(this.filePath)) { return configData; } - + configData.edge = process.env.TWILIO_EDGE; configData.loadFromObject(await fs.readJSON(this.filePath)); return configData; } From 4d7b49885d3b000d31fd1c2ba54e2dc9f4e67ca1 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Tue, 5 May 2020 13:23:23 -0700 Subject: [PATCH 7/9] fix config and add tests --- src/base-commands/twilio-client-command.js | 2 +- src/services/config.js | 3 +- src/services/open-api-client.js | 2 +- .../twilio-client-command.test.js | 66 +++++++++++++++++++ test/services/config.test.js | 11 ---- 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/base-commands/twilio-client-command.js b/src/base-commands/twilio-client-command.js index bcc15df2..2eb763fa 100644 --- a/src/base-commands/twilio-client-command.js +++ b/src/base-commands/twilio-client-command.js @@ -112,7 +112,7 @@ class TwilioClientCommand extends BaseCommand { return new ClientClass(this.currentProfile.apiKey, this.currentProfile.apiSecret, { accountSid: this.flags[CliFlags.ACCOUNT_SID] || this.currentProfile.accountSid, edge: process.env.TWILIO_EDGE || this.userConfig.edge, - region: this.currentProfile.region, + region: process.env.TWILIO_REGION || this.currentProfile.region, httpClient: this.httpClient }); } diff --git a/src/services/config.js b/src/services/config.js index da69662e..3d02adeb 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -139,7 +139,7 @@ class ConfigData { } loadFromObject(configObj) { - this.edge = this.edge || configObj.edge; + this.edge = configObj.edge; this.email = configObj.email || {}; this.prompts = configObj.prompts || {}; // Note the historical 'projects' naming. @@ -166,7 +166,6 @@ class Config { if (!fs.existsSync(this.filePath)) { return configData; } - configData.edge = process.env.TWILIO_EDGE; configData.loadFromObject(await fs.readJSON(this.filePath)); return configData; } diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index c248bcf9..104f52e5 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -47,7 +47,7 @@ class OpenApiClient { opts.uri = opts.host + opts.uri; } - let uri = new url.URL(opts.uri); + const uri = new url.URL(opts.uri); uri.hostname = this.getHost(uri.hostname, opts); uri.pathname = this.getUri(uri.pathname, opts); opts.uri = uri.href; diff --git a/test/base-commands/twilio-client-command.test.js b/test/base-commands/twilio-client-command.test.js index c7396e7e..d60ab2df 100644 --- a/test/base-commands/twilio-client-command.test.js +++ b/test/base-commands/twilio-client-command.test.js @@ -2,6 +2,8 @@ const { expect, test, constants } = require('@twilio/cli-test'); const TwilioClientCommand = require('../../src/base-commands/twilio-client-command'); const { Config, ConfigData } = require('../../src/services/config'); +const ORIGINAL_ENV = process.env; + describe('base-commands', () => { describe('twilio-client-command', () => { class TestClientCommand extends TwilioClientCommand { @@ -232,5 +234,69 @@ describe('base-commands', () => { expect(ctx.stderr).to.contain('A fake API error'); }); }); + + describe('regional and edge support', () => { + const envTest = ( + args = [], + { envRegion = undefined, envEdge = undefined, configEdge = undefined } = {} + ) => { + return test + .do(ctx => { + ctx.userConfig = new ConfigData(); + if (configEdge) { + ctx.userConfig.edge = configEdge; + } + if (envRegion) { + process.env.TWILIO_REGION = envRegion; + } + if (envEdge) { + process.env.TWILIO_EDGE = envEdge; + } + ctx.userConfig.addProfile('default-profile', constants.FAKE_ACCOUNT_SID); + ctx.userConfig.addProfile('region-edge-testing', constants.FAKE_ACCOUNT_SID, 'configRegion'); + }) + .twilioCliEnv(Config) + .do(async ctx => { + ctx.testCmd = new TwilioClientCommand(args, ctx.fakeConfig); + ctx.testCmd.secureStorage = + { + async getCredentials(profileId) { + return { + apiKey: constants.FAKE_API_KEY, + apiSecret: constants.FAKE_API_SECRET + profileId + }; + } + }; + + // This is essentially what oclif does behind the scenes. + try { + await ctx.testCmd.run(); + } catch (error) { + await ctx.testCmd.catch(error); + } + process.env = ORIGINAL_ENV; + }); + }; + + envTest([], { configEdge: 'edge' }).it('should use the config edge when defined', ctx => { + expect(ctx.testCmd.twilioApiClient.edge).to.equal('edge'); + expect(ctx.testCmd.twilioApiClient.region).to.be.undefined; + }); + + envTest(['-p', 'region-edge-testing']).it('should use the config region when defined', ctx => { + expect(ctx.testCmd.twilioApiClient.region).to.equal('configRegion'); + expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; + }); + + envTest(['-p', 'region-edge-testing'], { envRegion: 'region' }).it('should use the env region over a config region', ctx => { + expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); + expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; + }); + + envTest([], { configEdge: 'configEdge', envEdge: 'edge', envRegion: 'region' }).it('should use the env edge over a config edge', ctx => { + expect(ctx.testCmd.twilioApiClient.edge).to.equal('edge'); + expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); + }); + }); }); }); diff --git a/test/services/config.test.js b/test/services/config.test.js index 5194fd01..d89f2fd3 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -229,17 +229,6 @@ describe('services', () => { const saveMessage = await config.save(userConfig); expect(saveMessage).to.contain(`${nestedConfig}${path.sep}config.json`); }); - - test.it('uses env vars over config to set edge', async () => { - const config = new Config(tempConfigDir.name); - const userConfig = await config.load(); - - expect(userConfig.edge).to.be.undefined; - process.env.TWILIO_EDGE = 'edge'; - - const loadedConfig = await config.load(); - expect(loadedConfig.edge).to.equal('edge'); - }); }); }); }); From 6808ac79253120264e7105008a52fa50603f73f8 Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Tue, 5 May 2020 13:31:00 -0700 Subject: [PATCH 8/9] stop doing extra work --- src/services/open-api-client.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index 104f52e5..d2efbb75 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -43,7 +43,6 @@ class OpenApiClient { if (!opts.host) { opts.host = path.server; } - opts.host = this.getHost(opts.host, opts); opts.uri = opts.host + opts.uri; } From dbaceec47f5fc9bfd385fd47a27a122a43b8a90f Mon Sep 17 00:00:00 2001 From: Elise Shanholtz Date: Tue, 5 May 2020 15:35:51 -0700 Subject: [PATCH 9/9] i hope this is it --- src/base-commands/twilio-client-command.js | 2 +- src/services/config.js | 5 ++--- src/services/open-api-client.js | 8 ++----- src/services/twilio-api/twilio-client.js | 6 +++-- .../twilio-client-command.test.js | 14 +++++++----- .../services/twilio-api/twilio-client.test.js | 22 +++++++++---------- 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/base-commands/twilio-client-command.js b/src/base-commands/twilio-client-command.js index 2eb763fa..bcc15df2 100644 --- a/src/base-commands/twilio-client-command.js +++ b/src/base-commands/twilio-client-command.js @@ -112,7 +112,7 @@ class TwilioClientCommand extends BaseCommand { return new ClientClass(this.currentProfile.apiKey, this.currentProfile.apiSecret, { accountSid: this.flags[CliFlags.ACCOUNT_SID] || this.currentProfile.accountSid, edge: process.env.TWILIO_EDGE || this.userConfig.edge, - region: process.env.TWILIO_REGION || this.currentProfile.region, + region: this.currentProfile.region, httpClient: this.httpClient }); } diff --git a/src/services/config.js b/src/services/config.js index 3d02adeb..3a13d13b 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -1,6 +1,5 @@ const fs = require('fs-extra'); const path = require('path'); -const shell = require('shelljs'); const MessageTemplates = require('./messaging/templates'); const CLI_NAME = 'twilio-cli'; @@ -166,6 +165,7 @@ class Config { if (!fs.existsSync(this.filePath)) { return configData; } + configData.loadFromObject(await fs.readJSON(this.filePath)); return configData; } @@ -180,8 +180,7 @@ class Config { activeProject: configData.activeProfile }; - // Migrate to 'fs.mkdirSync' with 'recursive: true' when no longer supporting Node8. - shell.mkdir('-p', this.configDir); + fs.mkdirSync(this.configDir, { recursive: true }); await fs.writeJSON(this.filePath, configData, { flag: 'w' }); return MessageTemplates.configSaved({ path: this.filePath }); diff --git a/src/services/open-api-client.js b/src/services/open-api-client.js index d2efbb75..ca00e849 100644 --- a/src/services/open-api-client.js +++ b/src/services/open-api-client.js @@ -35,7 +35,7 @@ class OpenApiClient { const params = this.getParams(opts, operation); if (!opts.uri) { - opts.uri = this.getUri(opts.uri, opts); + opts.uri = this.getUri(opts); } // If the URI is relative, determine the host and prepend it. @@ -48,7 +48,6 @@ class OpenApiClient { const uri = new url.URL(opts.uri); uri.hostname = this.getHost(uri.hostname, opts); - uri.pathname = this.getUri(uri.pathname, opts); opts.uri = uri.href; opts.params = (isPost ? null : params); @@ -76,10 +75,7 @@ class OpenApiClient { return params; } - getUri(path, opts) { - if (path && path !== '/') { - return path; - } + getUri(opts) { // Evaluate the request path by replacing path parameters with their value // from the request data. return opts.path.replace(/{(.+?)}/g, (fullMatch, pathNode) => { diff --git a/src/services/twilio-api/twilio-client.js b/src/services/twilio-api/twilio-client.js index 818f6e8f..3ebe90ed 100644 --- a/src/services/twilio-api/twilio-client.js +++ b/src/services/twilio-api/twilio-client.js @@ -181,8 +181,10 @@ class TwilioApiClient { opts.headers.Accept = 'application/json'; } - if (opts.path.includes(TwilioApiFlags.ACCOUNT_SID) && !doesObjectHaveProperty(opts.pathParams, TwilioApiFlags.ACCOUNT_SID)) { - opts.pathParams[TwilioApiFlags.ACCOUNT_SID] = this.accountSid; + if (!opts.uri) { + if (opts.path.includes(TwilioApiFlags.ACCOUNT_SID) && !doesObjectHaveProperty(opts.pathParams, TwilioApiFlags.ACCOUNT_SID)) { + opts.pathParams[TwilioApiFlags.ACCOUNT_SID] = this.accountSid; + } } opts.edge = opts.edge || this.edge; diff --git a/test/base-commands/twilio-client-command.test.js b/test/base-commands/twilio-client-command.test.js index d60ab2df..ea1aaca3 100644 --- a/test/base-commands/twilio-client-command.test.js +++ b/test/base-commands/twilio-client-command.test.js @@ -238,22 +238,24 @@ describe('base-commands', () => { describe('regional and edge support', () => { const envTest = ( args = [], - { envRegion = undefined, envEdge = undefined, configEdge = undefined } = {} + { envRegion, envEdge, configRegion = 'configRegion', configEdge } = {} ) => { return test .do(ctx => { ctx.userConfig = new ConfigData(); - if (configEdge) { - ctx.userConfig.edge = configEdge; - } + ctx.userConfig.edge = configEdge; + if (envRegion) { process.env.TWILIO_REGION = envRegion; + process.env.TWILIO_ACCOUNT_SID = constants.FAKE_ACCOUNT_SID; + process.env.TWILIO_AUTH_TOKEN = constants.FAKE_API_SECRET; } if (envEdge) { process.env.TWILIO_EDGE = envEdge; } + ctx.userConfig.addProfile('default-profile', constants.FAKE_ACCOUNT_SID); - ctx.userConfig.addProfile('region-edge-testing', constants.FAKE_ACCOUNT_SID, 'configRegion'); + ctx.userConfig.addProfile('region-edge-testing', constants.FAKE_ACCOUNT_SID, configRegion); }) .twilioCliEnv(Config) .do(async ctx => { @@ -288,7 +290,7 @@ describe('base-commands', () => { expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; }); - envTest(['-p', 'region-edge-testing'], { envRegion: 'region' }).it('should use the env region over a config region', ctx => { + envTest([], { envRegion: 'region' }).it('should use the env region over a config region', ctx => { expect(ctx.testCmd.twilioApiClient.region).to.equal('region'); expect(ctx.testCmd.twilioApiClient.edge).to.be.undefined; }); diff --git a/test/services/twilio-api/twilio-client.test.js b/test/services/twilio-api/twilio-client.test.js index cdbd3e67..a0e27ac4 100644 --- a/test/services/twilio-api/twilio-client.test.js +++ b/test/services/twilio-api/twilio-client.test.js @@ -319,7 +319,7 @@ describe('services', () => { domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', // Should ignore the region in the uri - uri: 'https://api.uriRegion.twilio.com' + uri: `https://api.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -335,7 +335,7 @@ describe('services', () => { path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', region: 'region', // Should ignore the region in the uri - uri: 'https://api.uriRegion.twilio.com' + uri: `https://api.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -351,7 +351,7 @@ describe('services', () => { path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', region: 'region', // Should ignore the region in the uri - uri: 'https://api.uriRegion.twilio.com' + uri: `https://api.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -373,7 +373,7 @@ describe('services', () => { domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', // Should ignore the edge and region in the uri - uri: 'https://api.uriEdge.uriRegion.twilio.com' + uri: `https://api.uriEdge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -389,7 +389,7 @@ describe('services', () => { path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', edge: 'edge', // Should ignore the edge and region in the uri - uri: 'https://api.uriEdge.uriRegion.twilio.com' + uri: `https://api.uriEdge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -405,7 +405,7 @@ describe('services', () => { path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', region: 'region', // Should ignore the edge and region in the uri - uri: 'https://api.uriEdge.uriRegion.twilio.com' + uri: `https://api.uriEdge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -422,7 +422,7 @@ describe('services', () => { edge: 'edge', region: 'region', // Should ignore the edge and region in the uri - uri: 'https://api.uriEdge.uriRegion.twilio.com' + uri: `https://api.uriEdge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -439,7 +439,7 @@ describe('services', () => { path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', region: 'region', // Should ignore the edge and region in the uri - uri: 'https://api.uriEdge.uriRegion.twilio.com' + uri: `https://api.uriEdge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -455,7 +455,7 @@ describe('services', () => { domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', // Should ignore the edge in the uri - uri: 'https://api.uriEdge.region.twilio.com' + uri: `https://api.uriEdge.region.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -470,7 +470,7 @@ describe('services', () => { domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', // Should ignore the region in the uri - uri: 'https://api.edge.uriRegion.twilio.com' + uri: `https://api.edge.uriRegion.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); }); @@ -484,7 +484,7 @@ describe('services', () => { const response = await client.create({ domain: 'api', path: '/2010-04-01/Accounts/{AccountSid}/Messages.json', - uri: 'https://api.edge.region.twilio.com' + uri: `https://api.edge.region.twilio.com/2010-04-01/Accounts/${accountSid}/Messages.json` }); expect(response).to.eql({ status: 'queued' }); });