From 8fda0f9546802028770183edab54737eefdafd0a Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 08:47:19 -0800 Subject: [PATCH 01/14] Adding CLI option for listing plugins --- src/cli/plugin/plugin.js | 6 ++++++ src/cli/plugin/plugin_lister.js | 18 ++++++++++++++++++ src/cli/plugin/setting_parser.js | 8 ++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 src/cli/plugin/plugin_lister.js diff --git a/src/cli/plugin/plugin.js b/src/cli/plugin/plugin.js index e9898f21b4250..efb940d5c23cd 100644 --- a/src/cli/plugin/plugin.js +++ b/src/cli/plugin/plugin.js @@ -3,6 +3,7 @@ const fromRoot = utils('fromRoot'); const settingParser = require('./setting_parser'); const installer = require('./plugin_installer'); const remover = require('./plugin_remover'); +const lister = require('./plugin_lister'); const pluginLogger = require('./plugin_logger'); export default function pluginCli(program) { @@ -24,12 +25,17 @@ export default function pluginCli(program) { if (settings.action === 'remove') { remover.remove(settings, logger); } + if (settings.action === 'list') { + lister.list(settings, logger); + } + } program .command('plugin') .option('-i, --install //', 'The plugin to install') .option('-r, --remove ', 'The plugin to remove') + .option('-l, --list', 'List installed plugins') .option('-q, --quiet', 'Disable all process messaging except errors') .option('-s, --silent', 'Disable all process messaging') .option('-u, --url ', 'Specify download url') diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js new file mode 100644 index 0000000000000..9e58eada69096 --- /dev/null +++ b/src/cli/plugin/plugin_lister.js @@ -0,0 +1,18 @@ +const fs = require('fs'); + +module.exports = { + list: list +}; + +function list(settings, logger) { + fs.readdir(settings.pluginDir, function (err, files) { + + var pluginFiles = files.filter(function (file) { + return file[0] !== '.'; + }); + + pluginFiles.forEach(function (pluginFile) { + logger.log(pluginFile); + }); + }); +} diff --git a/src/cli/plugin/setting_parser.js b/src/cli/plugin/setting_parser.js index 783f0364f862d..e936b1ac7b0d5 100644 --- a/src/cli/plugin/setting_parser.js +++ b/src/cli/plugin/setting_parser.js @@ -84,8 +84,12 @@ export default function createSettingParser(options) { settings.package = parts.shift(); } - if (!settings.action || (options.install && options.remove)) { - throw new Error('Please specify either --install or --remove.'); + if (options.list) { + settings.action = 'list'; + } + + if (!settings.action || (options.install && options.remove && options.list)) { + throw new Error('Please specify either --install, --remove, or --list.'); } settings.pluginDir = options.pluginDir; From 0e14e41be2c95181f2ac56b87a04c449ae72b018 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 10:46:35 -0800 Subject: [PATCH 02/14] Using switch-case instead of ifs --- src/cli/plugin/plugin.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/cli/plugin/plugin.js b/src/cli/plugin/plugin.js index efb940d5c23cd..7c2bce16a773b 100644 --- a/src/cli/plugin/plugin.js +++ b/src/cli/plugin/plugin.js @@ -19,16 +19,17 @@ export default function pluginCli(program) { const logger = pluginLogger(settings); - if (settings.action === 'install') { - installer.install(settings, logger); + switch (settings.action) { + case 'install': + installer.install(settings, logger); + break; + case 'remove': + remover.remove(settings, logger); + break; + case 'list': + lister.list(settings, logger); + break; } - if (settings.action === 'remove') { - remover.remove(settings, logger); - } - if (settings.action === 'list') { - lister.list(settings, logger); - } - } program From ea0f2ef9ade32a9128f34bbe2dab5875d936f152 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 11:21:04 -0800 Subject: [PATCH 03/14] Getting rid of var and chaining calls instead --- src/cli/plugin/plugin_lister.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js index 9e58eada69096..25acb01a5fbce 100644 --- a/src/cli/plugin/plugin_lister.js +++ b/src/cli/plugin/plugin_lister.js @@ -7,11 +7,11 @@ module.exports = { function list(settings, logger) { fs.readdir(settings.pluginDir, function (err, files) { - var pluginFiles = files.filter(function (file) { + files + .filter(function (file) { return file[0] !== '.'; - }); - - pluginFiles.forEach(function (pluginFile) { + }) + .forEach(function (pluginFile) { logger.log(pluginFile); }); }); From d09f83b8d77acee671bde7d7a9b839f139022854 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 11:21:49 -0800 Subject: [PATCH 04/14] Using es6 export --- src/cli/plugin/plugin_lister.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js index 25acb01a5fbce..262cc2eb008d8 100644 --- a/src/cli/plugin/plugin_lister.js +++ b/src/cli/plugin/plugin_lister.js @@ -1,10 +1,6 @@ const fs = require('fs'); -module.exports = { - list: list -}; - -function list(settings, logger) { +export function list(settings, logger) { fs.readdir(settings.pluginDir, function (err, files) { files From 2e9f00f8f4f3ecb464cd9ac213fa4c660e68d969 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 11:28:12 -0800 Subject: [PATCH 05/14] Using synch flavor of readdir to bubble up exception --- src/cli/plugin/plugin_lister.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js index 262cc2eb008d8..64b1e17e74a96 100644 --- a/src/cli/plugin/plugin_lister.js +++ b/src/cli/plugin/plugin_lister.js @@ -1,14 +1,12 @@ const fs = require('fs'); export function list(settings, logger) { - fs.readdir(settings.pluginDir, function (err, files) { - - files - .filter(function (file) { - return file[0] !== '.'; - }) - .forEach(function (pluginFile) { - logger.log(pluginFile); - }); + const files = fs.readdirSync(settings.pluginDir); + files + .filter(function (file) { + return file[0] !== '.'; + }) + .forEach(function (pluginFile) { + logger.log(pluginFile); }); } From 5bfccda7be2c07feb079bd741a4da96f5ddb610c Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 12:07:18 -0800 Subject: [PATCH 06/14] Fixing logic error --- src/cli/plugin/setting_parser.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cli/plugin/setting_parser.js b/src/cli/plugin/setting_parser.js index e936b1ac7b0d5..6d34f5725db45 100644 --- a/src/cli/plugin/setting_parser.js +++ b/src/cli/plugin/setting_parser.js @@ -22,6 +22,16 @@ export default function createSettingParser(options) { return 'https://download.elastic.co/' + settings.organization + '/' + settings.package + '/' + filename; } + function areMultipleOptionsChosen(options, choices) { + let numChosen = 0; + choices.forEach(function (choice) { + if (options[choice]) { + ++numChosen; + } + }); + return (numChosen > 1); + } + function parse() { let parts; let settings = { @@ -88,7 +98,7 @@ export default function createSettingParser(options) { settings.action = 'list'; } - if (!settings.action || (options.install && options.remove && options.list)) { + if (!settings.action || areMultipleOptionsChosen(options, [ 'install', 'remove', 'list' ])) { throw new Error('Please specify either --install, --remove, or --list.'); } From 42334e366985a6bb31bba2fa5ace6f09889e7de4 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 12:59:14 -0800 Subject: [PATCH 07/14] Removing unnecessary filter --- src/cli/plugin/plugin_lister.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js index 64b1e17e74a96..946f09dfa39ae 100644 --- a/src/cli/plugin/plugin_lister.js +++ b/src/cli/plugin/plugin_lister.js @@ -3,9 +3,6 @@ const fs = require('fs'); export function list(settings, logger) { const files = fs.readdirSync(settings.pluginDir); files - .filter(function (file) { - return file[0] !== '.'; - }) .forEach(function (pluginFile) { logger.log(pluginFile); }); From cac0f0601715a75683400e7082c2035917ea1aa2 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 12:59:39 -0800 Subject: [PATCH 08/14] Another variable bites the dust --- src/cli/plugin/plugin_lister.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/plugin/plugin_lister.js b/src/cli/plugin/plugin_lister.js index 946f09dfa39ae..8351c3a96a0b0 100644 --- a/src/cli/plugin/plugin_lister.js +++ b/src/cli/plugin/plugin_lister.js @@ -1,8 +1,7 @@ const fs = require('fs'); export function list(settings, logger) { - const files = fs.readdirSync(settings.pluginDir); - files + fs.readdirSync(settings.pluginDir) .forEach(function (pluginFile) { logger.log(pluginFile); }); From ff1579440c52b130aba350194ed28af7f97d47e1 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 13:04:43 -0800 Subject: [PATCH 09/14] Simplifying using intersection from lodash --- src/cli/plugin/setting_parser.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/cli/plugin/setting_parser.js b/src/cli/plugin/setting_parser.js index 6d34f5725db45..e3654ac06a5ed 100644 --- a/src/cli/plugin/setting_parser.js +++ b/src/cli/plugin/setting_parser.js @@ -1,5 +1,6 @@ const { resolve } = require('path'); const expiry = require('expiry-js'); +import intersection from 'lodash'; export default function createSettingParser(options) { function parseMilliseconds(val) { @@ -23,13 +24,7 @@ export default function createSettingParser(options) { } function areMultipleOptionsChosen(options, choices) { - let numChosen = 0; - choices.forEach(function (choice) { - if (options[choice]) { - ++numChosen; - } - }); - return (numChosen > 1); + return intersection(Object.keys(options), choices).length > 1; } function parse() { From 220fc8b5de79c5d04f0bee84763be74d263e18eb Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Fri, 15 Jan 2016 13:13:09 -0800 Subject: [PATCH 10/14] Using import with proper semantics --- src/cli/plugin/setting_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/plugin/setting_parser.js b/src/cli/plugin/setting_parser.js index e3654ac06a5ed..b18b894b9fc43 100644 --- a/src/cli/plugin/setting_parser.js +++ b/src/cli/plugin/setting_parser.js @@ -1,6 +1,6 @@ const { resolve } = require('path'); const expiry = require('expiry-js'); -import intersection from 'lodash'; +import { intersection } from 'lodash'; export default function createSettingParser(options) { function parseMilliseconds(val) { From b9a8098b8ed4c3fca23dc01b4c56ed5f96bf9fff Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 16 Jan 2016 07:56:02 -0800 Subject: [PATCH 11/14] Fixing description of test --- src/cli/plugin/__tests__/setting_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/plugin/__tests__/setting_parser.js b/src/cli/plugin/__tests__/setting_parser.js index 384d24078b78d..8e467993dd92f 100644 --- a/src/cli/plugin/__tests__/setting_parser.js +++ b/src/cli/plugin/__tests__/setting_parser.js @@ -312,7 +312,7 @@ describe('kibana cli', function () { expect(settings).to.have.property('package', 'test-plugin'); }); - it('should not allow more than one part to the install parameter', function () { + it('should not allow more than one part to the remove parameter', function () { options.install = null; options.remove = 'kibana/test-plugin'; parser = settingParser(options); From cbc33509721d871b56229f8040b52bfffabf0e99 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 16 Jan 2016 07:56:27 -0800 Subject: [PATCH 12/14] Adding test for list option --- src/cli/plugin/__tests__/setting_parser.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/cli/plugin/__tests__/setting_parser.js b/src/cli/plugin/__tests__/setting_parser.js index 8e467993dd92f..9ccb02ad8180c 100644 --- a/src/cli/plugin/__tests__/setting_parser.js +++ b/src/cli/plugin/__tests__/setting_parser.js @@ -334,6 +334,21 @@ describe('kibana cli', function () { }); + describe('list option', function () { + + it('should set settings.action property to "list"', function () { + delete options.install; + delete options.remove; + options.list = true; + parser = settingParser(options); + + var settings = parser.parse(); + + expect(settings).to.have.property('action', 'list'); + }); + + }); + }); }); From f8ba6ca38ca9cf69a4b900cb79674c0367bdb771 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 16 Jan 2016 07:57:04 -0800 Subject: [PATCH 13/14] Updating error message tests to account for new "list" option --- src/cli/plugin/__tests__/setting_parser.js | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/cli/plugin/__tests__/setting_parser.js b/src/cli/plugin/__tests__/setting_parser.js index 9ccb02ad8180c..b405219564ac5 100644 --- a/src/cli/plugin/__tests__/setting_parser.js +++ b/src/cli/plugin/__tests__/setting_parser.js @@ -76,11 +76,11 @@ describe('kibana cli', function () { options = { install: 'dummy/dummy', pluginDir: fromRoot('installedPlugins') }; }); - it('should require the user to specify either install and remove', function () { + it('should require the user to specify either install, remove, or list', function () { options.install = null; parser = settingParser(options); - expect(parser.parse).withArgs().to.throwError(/Please specify either --install or --remove./); + expect(parser.parse).withArgs().to.throwError(/Please specify either --install, --remove, or --list./); }); it('should not allow the user to specify both install and remove', function () { @@ -88,7 +88,32 @@ describe('kibana cli', function () { options.install = 'org/package/version'; parser = settingParser(options); - expect(parser.parse).withArgs().to.throwError(/Please specify either --install or --remove./); + expect(parser.parse).withArgs().to.throwError(/Please specify either --install, --remove, or --list./); + }); + + it('should not allow the user to specify both install and list', function () { + options.list = true; + options.install = 'org/package/version'; + parser = settingParser(options); + + expect(parser.parse).withArgs().to.throwError(/Please specify either --install, --remove, or --list./); + }); + + it('should not allow the user to specify both remove and list', function () { + options.list = true; + options.remove = 'package'; + parser = settingParser(options); + + expect(parser.parse).withArgs().to.throwError(/Please specify either --install, --remove, or --list./); + }); + + it('should not allow the user to specify install, remove, and list', function () { + options.list = true; + options.install = 'org/package/version'; + options.remove = 'package'; + parser = settingParser(options); + + expect(parser.parse).withArgs().to.throwError(/Please specify either --install, --remove, or --list./); }); describe('quiet option', function () { From 13c0ccc71f052fe0eb85f4ff6f045ce8d7960aef Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Sat, 16 Jan 2016 07:57:33 -0800 Subject: [PATCH 14/14] Better removal of options set by previous tests --- src/cli/plugin/__tests__/setting_parser.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli/plugin/__tests__/setting_parser.js b/src/cli/plugin/__tests__/setting_parser.js index b405219564ac5..f02d5905e090d 100644 --- a/src/cli/plugin/__tests__/setting_parser.js +++ b/src/cli/plugin/__tests__/setting_parser.js @@ -318,7 +318,7 @@ describe('kibana cli', function () { describe('remove option', function () { it('should set settings.action property to "remove"', function () { - options.install = null; + delete options.install; options.remove = 'package'; parser = settingParser(options); @@ -328,7 +328,7 @@ describe('kibana cli', function () { }); it('should allow one part to the remove parameter', function () { - options.install = null; + delete options.install; options.remove = 'test-plugin'; parser = settingParser(options); @@ -338,7 +338,7 @@ describe('kibana cli', function () { }); it('should not allow more than one part to the remove parameter', function () { - options.install = null; + delete options.install; options.remove = 'kibana/test-plugin'; parser = settingParser(options); @@ -347,7 +347,7 @@ describe('kibana cli', function () { }); it('should populate the pluginPath', function () { - options.install = null; + delete options.install; options.remove = 'test-plugin'; parser = settingParser(options);