Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim whitespace from profile attributes #56

Merged
merged 1 commit into from
Aug 20, 2019
+52 −26
Merged

Trim whitespace from profile attributes #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

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
Open in desktop
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
Open in desktop
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