Skip to content

Commit

Permalink
Trim whitespace from profile attributes (#56)
Browse files Browse the repository at this point in the history 
Fixes twilio/twilio-cli#106
  • Loading branch information
childish-sambino authored Aug 20, 2019
1 parent ad0d9ff commit 5177d0d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 26 deletions.
2 changes: 1 addition & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 29 additions & 10 deletions src/services/config.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/* eslint no-warning-comments: "off" */
// TODO: Remove the above eslint directive when this file
// is free of TODO's.

const fs = require('fs-extra');
const path = require('path');
const shell = require('shelljs');
Expand Down Expand Up @@ -54,16 +50,31 @@ class ConfigData {
if (!profileId) {
profile = this.getProfileFromEnvironment();
}

if (!profile) {
if (profileId) {
// Clean the profile ID.
profileId = this.sanitize(profileId);
profile = this.profiles.find(profile => profile.id === profileId);
} else {
profile = this.getActiveProfile();
}
}

return profile;
}

setActiveProfile(profileId) {
if (profileId) {
const profile = this.getProfileById(profileId);

if (profile) {
this.activeProfile = profile.id;
return profile;
}
}
}

getActiveProfile() {
let profile;
if (this.profiles.length > 0) {
Expand All @@ -87,6 +98,11 @@ class ConfigData {
}

addProfile(id, accountSid, region) {
// Clean all the inputs.
id = this.sanitize(id);
accountSid = this.sanitize(accountSid);
region = this.sanitize(region);

const existing = this.getProfileById(id);
if (existing) {
existing.accountSid = accountSid;
Expand Down Expand Up @@ -116,12 +132,15 @@ class ConfigData {
loadFromObject(configObj) {
this.email = configObj.email || {};
this.prompts = configObj.prompts || {};
// TODO: Add versioning so we can drop the legacy "projects" naming.
this.activeProfile = configObj.activeProject;
// 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));
this.setActiveProfile(configObj.activeProject);
}

sanitize(string) {
// Trim whitespace if given a non-null string.
return string ? string.trim() : string;
}
}

Expand All @@ -146,7 +165,7 @@ class Config {
configData = {
email: configData.email,
prompts: configData.prompts,
// TODO: Add versioning so we can drop the legacy "projects" naming.
// Note the historical 'projects' naming.
projects: configData.profiles,
activeProject: configData.activeProfile
};
Expand Down
37 changes: 22 additions & 15 deletions test/services/config.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,44 +25,44 @@ describe('services', () => {

expect(configData.profiles[0].id).to.equal('activeProfile');
expect(configData.profiles[0].accountSid).to.equal('new-account-sid');
expect(configData.profiles[0].region).to.equal(undefined);
expect(configData.profiles[0].region).to.be.undefined;
});
});

describe('ConfigData.getProfileById', () => {
test.it('should return undefined if no profiles', () => {
const configData = new ConfigData();
const profile = configData.getProfileById('DOES_NOT_EXIST');
expect(profile).to.equal(undefined);
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;
process.env.TWILIO_AUTH_TOKEN = FAKE_AUTH_TOKEN;

const profile = configData.getProfileById('DOES_NOT_EXIST');
expect(profile).to.equal(undefined);
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);

const profile = configData.getProfileById();
expect(profile.accountSid).to.equal(constants.FAKE_ACCOUNT_SID);
expect(profile.apiKey).to.equal(undefined);
expect(profile.apiSecret).to.equal(undefined);
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);
configData.addProfile('secondProfile', 'new_account_SID');
configData.addProfile('thirdProfile', 'newest_account_SID');
configData.setActiveProfile('secondProfile');

configData.activeProfile = 'secondProfile';
const profile = configData.getProfileById();
expect(profile.accountSid).to.equal('new_account_SID');
expect(profile.apiKey).to.equal(undefined);
expect(profile.apiSecret).to.equal(undefined);
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();
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('services', () => {
});
});

describe('ConfigData.getActiveProfile', () => {
describe('ConfigData.activeProfile', () => {
test.it('should return first profile when no active profile is set', () => {
const configData = new ConfigData();
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
Expand All @@ -109,16 +109,22 @@ describe('services', () => {
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
configData.addProfile('secondProfile', 'new_account_SID');
configData.addProfile('thirdProfile', 'newest_account_SID');
configData.activeProfile = 'secondProfile';
configData.setActiveProfile('secondProfile');
const active = configData.getActiveProfile();

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();
expect(active).to.equal(undefined);
expect(active).to.be.undefined;
});
});

Expand Down Expand Up @@ -153,8 +159,7 @@ describe('services', () => {
configData.addProfile('firstProfile', constants.FAKE_ACCOUNT_SID);
configData.addProfile('secondProfile', 'new_account_SID');
configData.addProfile('thirdProfile', 'newest_account_SID');
const profile = configData.getProfileById('firstProfile');
configData.activeProfile = 'firstProfile';
const profile = configData.setActiveProfile('firstProfile');
configData.removeProfile(profile);

expect(configData.profiles[1].id).to.equal('thirdProfile');
Expand All @@ -176,17 +181,19 @@ describe('services', () => {
describe('Config', () => {
const tempConfigDir = tmp.dirSync({ unsafeCleanup: true });

test.it('saves and loads user configuration', async () => {
test.it('saves and loads user configuration with space trimmed', async () => {
const config = new Config(tempConfigDir.name);
const userConfig = await config.load();
userConfig.addProfile('myProfile', constants.FAKE_ACCOUNT_SID, 'stage');
userConfig.addProfile(' profile \t', 'sid \n ', ' stage');
userConfig.setActiveProfile('\tprofile\t');
userConfig.ackPrompt('impromptu');

const saveMessage = await config.save(userConfig);
expect(saveMessage).to.contain(`${tempConfigDir.name}${path.sep}config.json`);

const loadedConfig = await config.load();
expect(loadedConfig).to.deep.equal(userConfig);
expect(loadedConfig.getActiveProfile().id).to.equal('profile');
});

test.it('works with config dirs that did not exist', async () => {
Expand Down

0 comments on commit 5177d0d

Please sign in to comment.