From faf767ee3d97a056e1643daba80f905a5f9faa9a Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Fri, 19 Mar 2021 17:23:15 +1100 Subject: [PATCH 1/7] Allow plugins to use the userConfig object to set arbitrary data about themselves --- src/services/config.js | 13 +++++++++++++ test/services/config.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/services/config.js b/src/services/config.js index 3c1a1071..0b587d9e 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -22,6 +22,7 @@ class ConfigData { this.prompts = {}; this.profiles = []; this.activeProfile = null; + this.plugins = {}; } getProfileFromEnvironment() { @@ -142,6 +143,14 @@ class ConfigData { prompt.acked = true; } + setPluginData(pluginName, data) { + this.plugins[pluginName] = data; + } + + removePluginData(pluginName) { + delete this.plugins[pluginName]; + } + loadFromObject(configObj) { this.edge = configObj.edge; this.email = configObj.email || {}; @@ -150,6 +159,9 @@ class ConfigData { configObj.profiles = configObj.projects || []; configObj.profiles.forEach((profile) => this.addProfile(profile.id, profile.accountSid, profile.region)); this.setActiveProfile(configObj.activeProject); + Object.keys(configObj.plugins).forEach((pluginName) => + this.setPluginData(pluginName, configObj.plugins[pluginName]), + ); } sanitize(string) { @@ -183,6 +195,7 @@ class Config { // Note the historical 'projects' naming. projects: configData.profiles, activeProject: configData.activeProfile, + plugins: configData.plugins, }; fs.mkdirSync(this.configDir, { recursive: true }); diff --git a/test/services/config.test.js b/test/services/config.test.js index e38bc9d5..c5a499e1 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -203,6 +203,37 @@ describe('services', () => { }); }); + describe('ConfigData.setPluginData', () => { + test.it('should add data for a new plugin', () => { + const configData = new ConfigData(); + expect(configData.plugins['fancy-plugin']).to.be.undefined; + const testData = { arbitrary: 'data' }; + configData.setPluginData('fancy-plugin', testData); + + expect(configData.plugins['fancy-plugin']).to.deep.equal(testData); + }); + + test.it('should update data for an existing plugin', () => { + const configData = new ConfigData(); + const testData = { arbitrary: 'data' }; + const newData = { new: 'information' }; + configData.setPluginData('fancy-plugin', testData); + configData.setPluginData('fancy-plugin', newData); + + expect(configData.plugins['fancy-plugin']).to.deep.equal(newData); + expect(configData.plugins['fancy-plugin'].arbitrary).to.be.undefined; + }); + + test.it('should remove data for plugin', () => { + const configData = new ConfigData(); + const testData = { arbitrary: 'data' }; + configData.setPluginData('fancy-plugin', testData); + configData.removePluginData('fancy-plugin'); + + expect(configData.plugins['fancy-plugin']).to.be.undefined; + }); + }); + describe('Config', () => { const tempConfigDir = tmp.dirSync({ unsafeCleanup: true }); @@ -212,6 +243,7 @@ describe('services', () => { userConfig.addProfile(' profile \t', 'sid \n ', ' stage'); userConfig.setActiveProfile('\tprofile\t'); userConfig.ackPrompt('impromptu'); + userConfig.setPluginData('fancy-plugin', 'plugin-data'); const saveMessage = await config.save(userConfig); expect(saveMessage).to.contain(`${tempConfigDir.name}${path.sep}config.json`); From 00ac9b4d6b61f85bd925bf931802f6a7b4cb02cf Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Fri, 19 Mar 2021 17:31:47 +1100 Subject: [PATCH 2/7] Adds getter method for plugin data --- src/services/config.js | 4 ++++ test/services/config.test.js | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/services/config.js b/src/services/config.js index 0b587d9e..ce3bd159 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -147,6 +147,10 @@ class ConfigData { this.plugins[pluginName] = data; } + getPluginData(pluginName) { + return this.plugins[pluginName]; + } + removePluginData(pluginName) { delete this.plugins[pluginName]; } diff --git a/test/services/config.test.js b/test/services/config.test.js index c5a499e1..a7231b2a 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -206,11 +206,11 @@ describe('services', () => { describe('ConfigData.setPluginData', () => { test.it('should add data for a new plugin', () => { const configData = new ConfigData(); - expect(configData.plugins['fancy-plugin']).to.be.undefined; + expect(configData.getPluginData('fancy-plugin')).to.be.undefined; const testData = { arbitrary: 'data' }; configData.setPluginData('fancy-plugin', testData); - expect(configData.plugins['fancy-plugin']).to.deep.equal(testData); + expect(configData.getPluginData('fancy-plugin')).to.deep.equal(testData); }); test.it('should update data for an existing plugin', () => { @@ -220,8 +220,8 @@ describe('services', () => { configData.setPluginData('fancy-plugin', testData); configData.setPluginData('fancy-plugin', newData); - expect(configData.plugins['fancy-plugin']).to.deep.equal(newData); - expect(configData.plugins['fancy-plugin'].arbitrary).to.be.undefined; + expect(configData.getPluginData('fancy-plugin')).to.deep.equal(newData); + expect(configData.getPluginData('fancy-plugin').arbitrary).to.be.undefined; }); test.it('should remove data for plugin', () => { @@ -230,7 +230,7 @@ describe('services', () => { configData.setPluginData('fancy-plugin', testData); configData.removePluginData('fancy-plugin'); - expect(configData.plugins['fancy-plugin']).to.be.undefined; + expect(configData.getPluginData('fancy-plugin')).to.be.undefined; }); }); From 6b7cc99a0146d3272d65c7718397b7c61553d6d8 Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Fri, 26 Mar 2021 13:30:33 +1100 Subject: [PATCH 3/7] Revert "Adds getter method for plugin data" This reverts commit 00ac9b4d6b61f85bd925bf931802f6a7b4cb02cf. --- src/services/config.js | 4 ---- test/services/config.test.js | 10 +++++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/services/config.js b/src/services/config.js index ce3bd159..0b587d9e 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -147,10 +147,6 @@ class ConfigData { this.plugins[pluginName] = data; } - getPluginData(pluginName) { - return this.plugins[pluginName]; - } - removePluginData(pluginName) { delete this.plugins[pluginName]; } diff --git a/test/services/config.test.js b/test/services/config.test.js index a7231b2a..c5a499e1 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -206,11 +206,11 @@ describe('services', () => { describe('ConfigData.setPluginData', () => { test.it('should add data for a new plugin', () => { const configData = new ConfigData(); - expect(configData.getPluginData('fancy-plugin')).to.be.undefined; + expect(configData.plugins['fancy-plugin']).to.be.undefined; const testData = { arbitrary: 'data' }; configData.setPluginData('fancy-plugin', testData); - expect(configData.getPluginData('fancy-plugin')).to.deep.equal(testData); + expect(configData.plugins['fancy-plugin']).to.deep.equal(testData); }); test.it('should update data for an existing plugin', () => { @@ -220,8 +220,8 @@ describe('services', () => { configData.setPluginData('fancy-plugin', testData); configData.setPluginData('fancy-plugin', newData); - expect(configData.getPluginData('fancy-plugin')).to.deep.equal(newData); - expect(configData.getPluginData('fancy-plugin').arbitrary).to.be.undefined; + expect(configData.plugins['fancy-plugin']).to.deep.equal(newData); + expect(configData.plugins['fancy-plugin'].arbitrary).to.be.undefined; }); test.it('should remove data for plugin', () => { @@ -230,7 +230,7 @@ describe('services', () => { configData.setPluginData('fancy-plugin', testData); configData.removePluginData('fancy-plugin'); - expect(configData.getPluginData('fancy-plugin')).to.be.undefined; + expect(configData.plugins['fancy-plugin']).to.be.undefined; }); }); From c28678fad0c74116693c7ffe3dd371c0ae3b871c Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Fri, 26 Mar 2021 13:30:42 +1100 Subject: [PATCH 4/7] Revert "Allow plugins to use the userConfig object to set arbitrary data about themselves" This reverts commit faf767ee3d97a056e1643daba80f905a5f9faa9a. --- src/services/config.js | 13 ------------- test/services/config.test.js | 32 -------------------------------- 2 files changed, 45 deletions(-) diff --git a/src/services/config.js b/src/services/config.js index 0b587d9e..3c1a1071 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -22,7 +22,6 @@ class ConfigData { this.prompts = {}; this.profiles = []; this.activeProfile = null; - this.plugins = {}; } getProfileFromEnvironment() { @@ -143,14 +142,6 @@ class ConfigData { prompt.acked = true; } - setPluginData(pluginName, data) { - this.plugins[pluginName] = data; - } - - removePluginData(pluginName) { - delete this.plugins[pluginName]; - } - loadFromObject(configObj) { this.edge = configObj.edge; this.email = configObj.email || {}; @@ -159,9 +150,6 @@ class ConfigData { configObj.profiles = configObj.projects || []; configObj.profiles.forEach((profile) => this.addProfile(profile.id, profile.accountSid, profile.region)); this.setActiveProfile(configObj.activeProject); - Object.keys(configObj.plugins).forEach((pluginName) => - this.setPluginData(pluginName, configObj.plugins[pluginName]), - ); } sanitize(string) { @@ -195,7 +183,6 @@ class Config { // Note the historical 'projects' naming. projects: configData.profiles, activeProject: configData.activeProfile, - plugins: configData.plugins, }; fs.mkdirSync(this.configDir, { recursive: true }); diff --git a/test/services/config.test.js b/test/services/config.test.js index c5a499e1..e38bc9d5 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -203,37 +203,6 @@ describe('services', () => { }); }); - describe('ConfigData.setPluginData', () => { - test.it('should add data for a new plugin', () => { - const configData = new ConfigData(); - expect(configData.plugins['fancy-plugin']).to.be.undefined; - const testData = { arbitrary: 'data' }; - configData.setPluginData('fancy-plugin', testData); - - expect(configData.plugins['fancy-plugin']).to.deep.equal(testData); - }); - - test.it('should update data for an existing plugin', () => { - const configData = new ConfigData(); - const testData = { arbitrary: 'data' }; - const newData = { new: 'information' }; - configData.setPluginData('fancy-plugin', testData); - configData.setPluginData('fancy-plugin', newData); - - expect(configData.plugins['fancy-plugin']).to.deep.equal(newData); - expect(configData.plugins['fancy-plugin'].arbitrary).to.be.undefined; - }); - - test.it('should remove data for plugin', () => { - const configData = new ConfigData(); - const testData = { arbitrary: 'data' }; - configData.setPluginData('fancy-plugin', testData); - configData.removePluginData('fancy-plugin'); - - expect(configData.plugins['fancy-plugin']).to.be.undefined; - }); - }); - describe('Config', () => { const tempConfigDir = tmp.dirSync({ unsafeCleanup: true }); @@ -243,7 +212,6 @@ describe('services', () => { userConfig.addProfile(' profile \t', 'sid \n ', ' stage'); userConfig.setActiveProfile('\tprofile\t'); userConfig.ackPrompt('impromptu'); - userConfig.setPluginData('fancy-plugin', 'plugin-data'); const saveMessage = await config.save(userConfig); expect(saveMessage).to.contain(`${tempConfigDir.name}${path.sep}config.json`); From e10f45a8a9bb065dbb73d7bb74b032b125d3f80e Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Fri, 26 Mar 2021 17:00:07 +1100 Subject: [PATCH 5/7] Adds plugin config in specific plugin directory --- src/base-commands/base-command.js | 18 +++++++++++- src/services/config.js | 25 +++++++++++++++++ test/services/config.test.js | 46 ++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/base-commands/base-command.js b/src/base-commands/base-command.js index 4f664f1c..79fbaf93 100644 --- a/src/base-commands/base-command.js +++ b/src/base-commands/base-command.js @@ -3,7 +3,7 @@ const { CLIError } = require('@oclif/errors'); const pkg = require('../../package.json'); const MessageTemplates = require('../services/messaging/templates'); -const { Config, ConfigData } = require('../services/config'); +const { Config, ConfigData, PluginConfig } = require('../services/config'); const { TwilioCliError } = require('../services/error'); const { logger, LoggingLevel } = require('../services/messaging/logging'); const { OutputFormats } = require('../services/output-formats'); @@ -170,6 +170,22 @@ class BaseCommand extends Command { async install(name) { return requireInstall(name, this); } + + get pluginConfig() { + if (!this._pluginConfig) { + const pluginName = getCommandPlugin(this); + this._pluginConfig = new PluginConfig(this.config.configDir, pluginName); + } + return this._pluginConfig.getConfig(); + } + + set pluginConfig(config) { + if (!this._pluginConfig) { + const pluginName = getCommandPlugin(this); + this._pluginConfig = new PluginConfig(this.config.configDir, pluginName); + } + this._pluginConfig.setConfig(config); + } } BaseCommand.flags = { diff --git a/src/services/config.js b/src/services/config.js index 3c1a1071..0a259afa 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -192,8 +192,33 @@ class Config { } } +class PluginConfig { + constructor(configDir, pluginName) { + this.filePath = path.join(configDir, 'plugins', pluginName, 'config.json'); + } + + getConfig() { + try { + const config = fs.readFileSync(this.filePath, { encoding: 'utf-8' }); + return JSON.parse(config); + } catch (error) { + return {}; + } + } + + setConfig(config) { + try { + fs.writeFileSync(this.filePath, JSON.stringify(config)); + } catch (error) { + fs.mkdirSync(path.dirname(this.filePath), { recursive: true }); + fs.writeFileSync(this.filePath, JSON.stringify(config)); + } + } +} + module.exports = { CLI_NAME, Config, ConfigData, + PluginConfig, }; diff --git a/test/services/config.test.js b/test/services/config.test.js index e38bc9d5..43106372 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -1,9 +1,10 @@ const path = require('path'); +const fs = require('fs'); const tmp = require('tmp'); const { expect, test, constants } = require('@twilio/cli-test'); -const { Config, ConfigData } = require('../../src/services/config'); +const { Config, ConfigData, PluginConfig } = require('../../src/services/config'); const FAKE_AUTH_TOKEN = '1234567890abcdefghijklmnopqrstuvwxyz'; @@ -231,5 +232,48 @@ describe('services', () => { expect(saveMessage).to.contain(`${nestedConfig}${path.sep}config.json`); }); }); + + describe('PluginConfig', () => { + let tempConfigDir; + beforeEach(() => { + tempConfigDir = tmp.dirSync({ unsafeCleanup: true }); + }); + afterEach(() => { + tempConfigDir.removeCallback(); + }); + + test.it("loads an empty object when the plugin directory doesn't exist", () => { + const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); + expect(pluginConfig.getConfig()).to.deep.equal({}); + }); + + test.it('loads an existing plugin config', () => { + fs.mkdirSync(path.join(tempConfigDir.name, 'plugins', 'test-plugin'), { recursive: true }); + fs.writeFileSync( + path.join(tempConfigDir.name, 'plugins', 'test-plugin', 'config.json'), + JSON.stringify({ hello: 'world' }), + ); + const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); + expect(pluginConfig.getConfig()).to.deep.equal({ hello: 'world' }); + }); + + test.it("saves config to the plugin directory when it doesn't exist", () => { + const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); + pluginConfig.setConfig({ foo: 'bar' }); + expect(pluginConfig.getConfig()).to.deep.equal({ foo: 'bar' }); + }); + + test.it('overwrites config when it already exists', () => { + fs.mkdirSync(path.join(tempConfigDir.name, 'plugins', 'test-plugin'), { recursive: true }); + fs.writeFileSync( + path.join(tempConfigDir.name, 'plugins', 'test-plugin', 'config.json'), + JSON.stringify({ hello: 'world' }), + ); + + const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); + pluginConfig.setConfig({ foo: 'bar' }); + expect(pluginConfig.getConfig()).to.deep.equal({ foo: 'bar' }); + }); + }); }); }); From 23f2535a0c7442fa55aa711f327113c74cdc19c6 Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Sun, 11 Apr 2021 13:21:34 +1000 Subject: [PATCH 6/7] Uses read/writeJSON instead of read/writeFile. Removes fs from tests. --- src/base-commands/base-command.js | 14 ++++++------- src/services/config.js | 13 ++++++------ test/services/config.test.js | 33 ++++++++++--------------------- 3 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/base-commands/base-command.js b/src/base-commands/base-command.js index 79fbaf93..7ad3a6b5 100644 --- a/src/base-commands/base-command.js +++ b/src/base-commands/base-command.js @@ -176,15 +176,15 @@ class BaseCommand extends Command { const pluginName = getCommandPlugin(this); this._pluginConfig = new PluginConfig(this.config.configDir, pluginName); } - return this._pluginConfig.getConfig(); + return this._pluginConfig; } - set pluginConfig(config) { - if (!this._pluginConfig) { - const pluginName = getCommandPlugin(this); - this._pluginConfig = new PluginConfig(this.config.configDir, pluginName); - } - this._pluginConfig.setConfig(config); + async getPluginConfig() { + return this.pluginConfig.getConfig(); + } + + async setPluginConfig(config) { + return this.pluginConfig.setConfig(config); } } diff --git a/src/services/config.js b/src/services/config.js index 0a259afa..7c5763dc 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -197,21 +197,20 @@ class PluginConfig { this.filePath = path.join(configDir, 'plugins', pluginName, 'config.json'); } - getConfig() { + async getConfig() { try { - const config = fs.readFileSync(this.filePath, { encoding: 'utf-8' }); - return JSON.parse(config); + return await fs.readJSON(this.filePath, { encoding: 'utf-8' }); } catch (error) { return {}; } } - setConfig(config) { + async setConfig(config) { try { - fs.writeFileSync(this.filePath, JSON.stringify(config)); + await fs.writeJSON(this.filePath, config); } catch (error) { - fs.mkdirSync(path.dirname(this.filePath), { recursive: true }); - fs.writeFileSync(this.filePath, JSON.stringify(config)); + await fs.mkdir(path.dirname(this.filePath), { recursive: true }); + await fs.writeJSON(this.filePath, config); } } } diff --git a/test/services/config.test.js b/test/services/config.test.js index 43106372..48e95cda 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -242,37 +242,24 @@ describe('services', () => { tempConfigDir.removeCallback(); }); - test.it("loads an empty object when the plugin directory doesn't exist", () => { + test.it("loads an empty object when the plugin directory doesn't exist", async () => { const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); - expect(pluginConfig.getConfig()).to.deep.equal({}); + expect(await pluginConfig.getConfig()).to.deep.equal({}); }); - test.it('loads an existing plugin config', () => { - fs.mkdirSync(path.join(tempConfigDir.name, 'plugins', 'test-plugin'), { recursive: true }); - fs.writeFileSync( - path.join(tempConfigDir.name, 'plugins', 'test-plugin', 'config.json'), - JSON.stringify({ hello: 'world' }), - ); + test.it("saves config to the plugin directory when it doesn't exist", async () => { const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); - expect(pluginConfig.getConfig()).to.deep.equal({ hello: 'world' }); + await pluginConfig.setConfig({ foo: 'bar' }); + expect(await pluginConfig.getConfig()).to.deep.equal({ foo: 'bar' }); }); - test.it("saves config to the plugin directory when it doesn't exist", () => { + test.it('overwrites config when it already exists', async () => { const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); - pluginConfig.setConfig({ foo: 'bar' }); - expect(pluginConfig.getConfig()).to.deep.equal({ foo: 'bar' }); - }); - - test.it('overwrites config when it already exists', () => { - fs.mkdirSync(path.join(tempConfigDir.name, 'plugins', 'test-plugin'), { recursive: true }); - fs.writeFileSync( - path.join(tempConfigDir.name, 'plugins', 'test-plugin', 'config.json'), - JSON.stringify({ hello: 'world' }), - ); + await pluginConfig.setConfig({ hello: 'world' }); - const pluginConfig = new PluginConfig(tempConfigDir.name, 'test-plugin'); - pluginConfig.setConfig({ foo: 'bar' }); - expect(pluginConfig.getConfig()).to.deep.equal({ foo: 'bar' }); + const pluginConfig2 = new PluginConfig(tempConfigDir.name, 'test-plugin'); + await pluginConfig2.setConfig({ foo: 'bar' }); + expect(await pluginConfig2.getConfig()).to.deep.equal({ foo: 'bar' }); }); }); }); From b04fd8c9e3978ee89fef0cb4393d10d459ab5d41 Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Tue, 13 Apr 2021 09:56:26 +1000 Subject: [PATCH 7/7] Removes fs require from config test --- test/services/config.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/services/config.test.js b/test/services/config.test.js index 48e95cda..af77b689 100644 --- a/test/services/config.test.js +++ b/test/services/config.test.js @@ -1,5 +1,4 @@ const path = require('path'); -const fs = require('fs'); const tmp = require('tmp'); const { expect, test, constants } = require('@twilio/cli-test');