From c82913973269c424736a730a42c42bb94d3a3f58 Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 6 Feb 2017 09:33:24 -0500 Subject: [PATCH 1/3] Resolving unused settings bug when plugin is disabled --- src/server/config/__tests__/config.js | 19 ------------------- src/server/config/config.js | 10 ---------- src/server/plugins/plugin_collection.js | 6 ------ 3 files changed, 35 deletions(-) diff --git a/src/server/config/__tests__/config.js b/src/server/config/__tests__/config.js index 200140adca6c8..641bf55d683d0 100644 --- a/src/server/config/__tests__/config.js +++ b/src/server/config/__tests__/config.js @@ -238,24 +238,5 @@ describe('lib/config/config', function () { }); }); - - describe('#removeSchema(key)', function () { - it('should completely remove the key', function () { - const config = new Config(Joi.object().keys({ - a: Joi.number().default(1) - })); - - expect(config.get('a')).to.be(1); - config.removeSchema('a'); - expect(() => config.get('a')).to.throwException('Unknown config key'); - }); - - it('only removes existing keys', function () { - const config = new Config(Joi.object()); - - expect(() => config.removeSchema('b')).to.throwException('Unknown schema'); - }); - }); - }); }); diff --git a/src/server/config/config.js b/src/server/config/config.js index 1d036ca63e96a..dc7808a8736e2 100644 --- a/src/server/config/config.js +++ b/src/server/config/config.js @@ -42,16 +42,6 @@ module.exports = class Config { this.set(key, settings); } - removeSchema(key) { - if (!_.has(this[schemaExts], key)) { - throw new TypeError(`Unknown schema key: ${key}`); - } - - this[schema] = null; - unset(this[schemaExts], key); - unset(this[vals], key); - } - resetTo(obj) { this._commit(obj); } diff --git a/src/server/plugins/plugin_collection.js b/src/server/plugins/plugin_collection.js index 2d479dedcbd79..33d0fa669924e 100644 --- a/src/server/plugins/plugin_collection.js +++ b/src/server/plugins/plugin_collection.js @@ -24,11 +24,6 @@ async function addPluginConfig(pluginCollection, plugin) { config.extendSchema(configSchema, transformedPluginSettings, plugin.configPrefix); } -function removePluginConfig(pluginCollection, plugin) { - const { config } = pluginCollection.kbnServer; - config.removeSchema(plugin.configPrefix); -} - module.exports = class Plugins extends Collection { constructor(kbnServer) { @@ -60,7 +55,6 @@ module.exports = class Plugins extends Collection { } async disable(plugin) { - removePluginConfig(this, plugin); this.delete(plugin); } From 81538f444d258c5f84557c4f11238f1a504e9a38 Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 8 Feb 2017 17:16:45 -0500 Subject: [PATCH 2/3] Revert "Resolving unused settings bug when plugin is disabled" This reverts commit c82913973269c424736a730a42c42bb94d3a3f58. --- src/server/config/__tests__/config.js | 19 +++++++++++++++++++ src/server/config/config.js | 10 ++++++++++ src/server/plugins/plugin_collection.js | 6 ++++++ 3 files changed, 35 insertions(+) diff --git a/src/server/config/__tests__/config.js b/src/server/config/__tests__/config.js index 641bf55d683d0..200140adca6c8 100644 --- a/src/server/config/__tests__/config.js +++ b/src/server/config/__tests__/config.js @@ -238,5 +238,24 @@ describe('lib/config/config', function () { }); }); + + describe('#removeSchema(key)', function () { + it('should completely remove the key', function () { + const config = new Config(Joi.object().keys({ + a: Joi.number().default(1) + })); + + expect(config.get('a')).to.be(1); + config.removeSchema('a'); + expect(() => config.get('a')).to.throwException('Unknown config key'); + }); + + it('only removes existing keys', function () { + const config = new Config(Joi.object()); + + expect(() => config.removeSchema('b')).to.throwException('Unknown schema'); + }); + }); + }); }); diff --git a/src/server/config/config.js b/src/server/config/config.js index dc7808a8736e2..1d036ca63e96a 100644 --- a/src/server/config/config.js +++ b/src/server/config/config.js @@ -42,6 +42,16 @@ module.exports = class Config { this.set(key, settings); } + removeSchema(key) { + if (!_.has(this[schemaExts], key)) { + throw new TypeError(`Unknown schema key: ${key}`); + } + + this[schema] = null; + unset(this[schemaExts], key); + unset(this[vals], key); + } + resetTo(obj) { this._commit(obj); } diff --git a/src/server/plugins/plugin_collection.js b/src/server/plugins/plugin_collection.js index 33d0fa669924e..2d479dedcbd79 100644 --- a/src/server/plugins/plugin_collection.js +++ b/src/server/plugins/plugin_collection.js @@ -24,6 +24,11 @@ async function addPluginConfig(pluginCollection, plugin) { config.extendSchema(configSchema, transformedPluginSettings, plugin.configPrefix); } +function removePluginConfig(pluginCollection, plugin) { + const { config } = pluginCollection.kbnServer; + config.removeSchema(plugin.configPrefix); +} + module.exports = class Plugins extends Collection { constructor(kbnServer) { @@ -55,6 +60,7 @@ module.exports = class Plugins extends Collection { } async disable(plugin) { + removePluginConfig(this, plugin); this.delete(plugin); } From 7e66762022323b508ff4d79a7de909e107f03d11 Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 8 Feb 2017 17:28:41 -0500 Subject: [PATCH 3/3] Replacing a disabled plugin's config with a simple schema/config with enabled set to false --- src/server/plugins/plugin_collection.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/server/plugins/plugin_collection.js b/src/server/plugins/plugin_collection.js index 2d479dedcbd79..ba3f238fc2955 100644 --- a/src/server/plugins/plugin_collection.js +++ b/src/server/plugins/plugin_collection.js @@ -6,6 +6,7 @@ import toPath from 'lodash/internal/toPath'; import Collection from '../../utils/collection'; import { transformDeprecations } from '../config/transform_deprecations'; import { createTransform } from '../../deprecation'; +import Joi from 'joi'; const byIdCache = Symbol('byIdCache'); const pluginApis = Symbol('pluginApis'); @@ -24,9 +25,13 @@ async function addPluginConfig(pluginCollection, plugin) { config.extendSchema(configSchema, transformedPluginSettings, plugin.configPrefix); } -function removePluginConfig(pluginCollection, plugin) { +function disablePluginConfig(pluginCollection, plugin) { + // when disabling a plugin's config we remove the existing schema and + // replace it with a simple schema/config that only has enabled set to false const { config } = pluginCollection.kbnServer; config.removeSchema(plugin.configPrefix); + const schema = Joi.object({ enabled: Joi.bool() }); + config.extendSchema(schema, { enabled: false }, plugin.configPrefix); } module.exports = class Plugins extends Collection { @@ -60,7 +65,7 @@ module.exports = class Plugins extends Collection { } async disable(plugin) { - removePluginConfig(this, plugin); + disablePluginConfig(this, plugin); this.delete(plugin); }