From 817d2612a43b8c38671db5c5b6fb9c97749bfbf2 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 11:07:41 -0700 Subject: [PATCH 01/18] [server/uiSettings] make uiSettings service request based --- .../routes/api/settings/register_delete.js | 7 +- .../routes/api/settings/register_get.js | 6 +- .../routes/api/settings/register_set.js | 7 +- .../routes/api/settings/register_set_many.js | 7 +- .../timelion/server/routes/run.js | 2 +- .../timelion/server/routes/validate_es.js | 2 +- src/server/http/index.js | 4 +- src/ui/index.js | 4 +- src/ui/ui_settings/ui_settings.js | 122 +++++++++--------- src/ui/ui_settings/ui_settings_factory.js | 25 ++++ src/ui/ui_settings/ui_settings_for_request.js | 20 +++ src/ui/ui_settings/ui_settings_mixin.js | 34 ++++- 12 files changed, 159 insertions(+), 81 deletions(-) create mode 100644 src/ui/ui_settings/ui_settings_factory.js create mode 100644 src/ui/ui_settings/ui_settings_for_request.js diff --git a/src/core_plugins/kibana/server/routes/api/settings/register_delete.js b/src/core_plugins/kibana/server/routes/api/settings/register_delete.js index ea68620298d3d..1b78e83b5eac6 100644 --- a/src/core_plugins/kibana/server/routes/api/settings/register_delete.js +++ b/src/core_plugins/kibana/server/routes/api/settings/register_delete.js @@ -6,11 +6,12 @@ export default function registerDelete(server) { method: 'DELETE', handler: function (req, reply) { const { key } = req.params; - const uiSettings = server.uiSettings(); + const uiSettings = req.getUiSettingsService(); + uiSettings - .remove(req, key) + .remove(key) .then(() => uiSettings - .getUserProvided(req) + .getUserProvided() .then(settings => reply({ settings }).type('application/json')) ) .catch(err => reply(Boom.wrap(err, err.statusCode))); diff --git a/src/core_plugins/kibana/server/routes/api/settings/register_get.js b/src/core_plugins/kibana/server/routes/api/settings/register_get.js index 85fc8c9ca5f27..123f869ef0672 100644 --- a/src/core_plugins/kibana/server/routes/api/settings/register_get.js +++ b/src/core_plugins/kibana/server/routes/api/settings/register_get.js @@ -5,9 +5,9 @@ export default function registerGet(server) { path: '/api/kibana/settings', method: 'GET', handler: function (req, reply) { - server - .uiSettings() - .getUserProvided(req) + req + .getUiSettingsService() + .getUserProvided() .then(settings => reply({ settings }).type('application/json')) .catch(err => reply(Boom.wrap(err, err.statusCode))); } diff --git a/src/core_plugins/kibana/server/routes/api/settings/register_set.js b/src/core_plugins/kibana/server/routes/api/settings/register_set.js index 8d572bbd1ab25..f82cafef43324 100644 --- a/src/core_plugins/kibana/server/routes/api/settings/register_set.js +++ b/src/core_plugins/kibana/server/routes/api/settings/register_set.js @@ -7,11 +7,12 @@ export default function registerSet(server) { handler: function (req, reply) { const { key } = req.params; const { value } = req.payload; - const uiSettings = server.uiSettings(); + const uiSettings = req.getUiSettingsService(); + uiSettings - .set(req, key, value) + .set(key, value) .then(() => uiSettings - .getUserProvided(req) + .getUserProvided() .then(settings => reply({ settings }).type('application/json')) ) .catch(err => reply(Boom.wrap(err, err.statusCode))); diff --git a/src/core_plugins/kibana/server/routes/api/settings/register_set_many.js b/src/core_plugins/kibana/server/routes/api/settings/register_set_many.js index 57ce8357fb1d2..71e873bd80d79 100644 --- a/src/core_plugins/kibana/server/routes/api/settings/register_set_many.js +++ b/src/core_plugins/kibana/server/routes/api/settings/register_set_many.js @@ -6,11 +6,12 @@ export default function registerSet(server) { method: 'POST', handler: function (req, reply) { const { changes } = req.payload; - const uiSettings = server.uiSettings(); + const uiSettings = req.getUiSettingsService(); + uiSettings - .setMany(req, changes) + .setMany(changes) .then(() => uiSettings - .getUserProvided(req) + .getUserProvided() .then(settings => reply({ settings }).type('application/json')) ) .catch(err => reply(Boom.wrap(err, err.statusCode))); diff --git a/src/core_plugins/timelion/server/routes/run.js b/src/core_plugins/timelion/server/routes/run.js index 6a5cffcd1754b..136d5e08673dd 100644 --- a/src/core_plugins/timelion/server/routes/run.js +++ b/src/core_plugins/timelion/server/routes/run.js @@ -14,7 +14,7 @@ export default function (server) { path: '/api/timelion/run', handler: async (request, reply) => { try { - const uiSettings = await server.uiSettings().getAll(request); + const uiSettings = await request.getUiSettingsService().getAll(); const tlConfig = require('../handlers/lib/tl_config.js')({ server, diff --git a/src/core_plugins/timelion/server/routes/validate_es.js b/src/core_plugins/timelion/server/routes/validate_es.js index 0bac26f414af3..7802c09a6196e 100644 --- a/src/core_plugins/timelion/server/routes/validate_es.js +++ b/src/core_plugins/timelion/server/routes/validate_es.js @@ -3,7 +3,7 @@ export default function (server) { method: 'GET', path: '/api/timelion/validate/es', handler: function (request, reply) { - return server.uiSettings().getAll(request).then((uiSettings) => { + return request.getUiSettingsService().getAll().then((uiSettings) => { const { callWithRequest } = server.plugins.elasticsearch.getCluster('data'); const timefield = uiSettings['timelion:es.timefield']; diff --git a/src/server/http/index.js b/src/server/http/index.js index 6380c0beef33d..ef1fec8264f08 100644 --- a/src/server/http/index.js +++ b/src/server/http/index.js @@ -120,8 +120,8 @@ export default async function (kbnServer, server, config) { const url = await shortUrlLookup.getUrl(request.params.urlId, request); shortUrlAssertValid(url); - const uiSettings = server.uiSettings(); - const stateStoreInSessionStorage = await uiSettings.get(request, 'state:storeInSessionStorage'); + const uiSettings = request.getUiSettingsService(); + const stateStoreInSessionStorage = await uiSettings.get('state:storeInSessionStorage'); if (!stateStoreInSessionStorage) { reply().redirect(config.get('server.basePath') + url); return; diff --git a/src/ui/index.js b/src/ui/index.js index cec250e9d1b8e..34545e63be1ed 100644 --- a/src/ui/index.js +++ b/src/ui/index.js @@ -67,7 +67,7 @@ export default async (kbnServer, server, config) => { }); async function getKibanaPayload({ app, request, includeUserProvidedConfig, injectedVarsOverrides }) { - const uiSettings = server.uiSettings(); + const uiSettings = request.getUiSettingsService(); const translations = await uiI18n.getTranslationsForRequest(request); return { @@ -83,7 +83,7 @@ export default async (kbnServer, server, config) => { translations: translations, uiSettings: await props({ defaults: uiSettings.getDefaults(), - user: includeUserProvidedConfig && uiSettings.getUserProvided(request) + user: includeUserProvidedConfig && uiSettings.getUserProvided() }), vars: await reduceAsync( uiExports.injectedVarsReplacers, diff --git a/src/ui/ui_settings/ui_settings.js b/src/ui/ui_settings/ui_settings.js index e645cee68edc4..faff64ad32c4f 100644 --- a/src/ui/ui_settings/ui_settings.js +++ b/src/ui/ui_settings/ui_settings.js @@ -1,31 +1,31 @@ -import { defaultsDeep } from 'lodash'; +import { defaultsDeep, noop } from 'lodash'; import Bluebird from 'bluebird'; +import { errors as esErrors } from 'elasticsearch'; import { getDefaultSettings } from './defaults'; -function hydrateUserSettings(user) { - return Object.keys(user) - .map(key => ({ key, userValue: user[key] })) +function hydrateUserSettings(userSettings) { + return Object.keys(userSettings) + .map(key => ({ key, userValue: userSettings[key] })) .filter(({ userValue }) => userValue !== null) .reduce((acc, { key, userValue }) => ({ ...acc, [key]: { userValue } }), {}); } -function assertRequest(req) { - if ( - !req || - typeof req !== 'object' || - typeof req.path !== 'string' || - !req.headers || - typeof req.headers !== 'object' - ) { - throw new TypeError('all uiSettings methods must be passed a hapi.Request object'); - } -} - -export class UiSettings { - constructor(server, status) { - this._server = server; - this._status = status; +export class UiSettingsService { + constructor(options = {}) { + const { + index, + type, + id, + callCluster, + readInterceptor = noop, + } = options; + + this._callCluster = callCluster; + this._readInterceptor = readInterceptor; + this._index = index; + this._type = type; + this._id = id; } getDefaults() { @@ -33,15 +33,13 @@ export class UiSettings { } // returns a Promise for the value of the requested setting - async get(req, key) { - assertRequest(req); - return this.getAll(req) + get(key) { + return this.getAll() .then(all => all[key]); } - async getAll(req) { - assertRequest(req); - return this.getRaw(req) + getAll() { + return this.getRaw() .then(raw => Object.keys(raw) .reduce((all, key) => { const item = raw[key]; @@ -52,69 +50,73 @@ export class UiSettings { ); } - async getRaw(req) { - assertRequest(req); - return this.getUserProvided(req) + getRaw() { + return this.getUserProvided() .then(user => defaultsDeep(user, this.getDefaults())); } - async getUserProvided(req, { ignore401Errors = false } = {}) { - assertRequest(req); - const { callWithRequest, errors } = this._server.plugins.elasticsearch.getCluster('admin'); + async getUserProvided(options) { + return hydrateUserSettings(this._read(options)); + } - // If the ui settings status isn't green, we shouldn't be attempting to get - // user settings, since we can't be sure that all the necessary conditions - // (e.g. elasticsearch being available) are met. - if (this._status.state !== 'green') { - return hydrateUserSettings({}); + async _read(options = {}) { + const interceptValue = await this._readInterceptor(options); + if (interceptValue != null) { + return interceptValue; } + const { + ignore401Errors = false + } = options; + const params = this._getClientSettings(); - const allowedErrors = [errors[404], errors[403], errors.NoConnections]; - if (ignore401Errors) allowedErrors.push(errors[401]); + const allowedErrors = [ + esErrors[404], + esErrors[403], + esErrors.NoConnections + ]; + + if (ignore401Errors) { + allowedErrors.push(esErrors[401]); + } return Bluebird - .resolve(callWithRequest(req, 'get', params, { wrap401Errors: !ignore401Errors })) + .resolve(this._callCluster('get', params, { wrap401Errors: !ignore401Errors })) .catch(...allowedErrors, () => ({})) - .then(resp => resp._source || {}) - .then(source => hydrateUserSettings(source)); + .then(resp => resp._source || {}); } - async setMany(req, changes) { - assertRequest(req); - const { callWithRequest } = this._server.plugins.elasticsearch.getCluster('admin'); + setMany(changes) { const clientParams = { ...this._getClientSettings(), body: { doc: changes } }; - return callWithRequest(req, 'update', clientParams) + + return this._callCluster('update', clientParams) .then(() => ({})); } - async set(req, key, value) { - assertRequest(req); - return this.setMany(req, { [key]: value }); + set(key, value) { + return this.setMany({ [key]: value }); } - async remove(req, key) { - assertRequest(req); - return this.set(req, key, null); + remove(key) { + return this.set(key, null); } - async removeMany(req, keys) { - assertRequest(req); + removeMany(keys) { const changes = {}; keys.forEach(key => { changes[key] = null; }); - return this.setMany(req, changes); + return this.setMany(changes); } _getClientSettings() { - const config = this._server.config(); - const index = config.get('kibana.index'); - const id = config.get('pkg.version'); - const type = 'config'; - return { index, type, id }; + return { + index: this._index, + type: this._type, + id: this._id + }; } } diff --git a/src/ui/ui_settings/ui_settings_factory.js b/src/ui/ui_settings/ui_settings_factory.js new file mode 100644 index 0000000000000..27de9d1bcbb1f --- /dev/null +++ b/src/ui/ui_settings/ui_settings_factory.js @@ -0,0 +1,25 @@ +import { UiSettingsService } from './ui_settings'; + +/** + * Create an instance of UiSettingsService that will use the + * passed `callCluster` function to communicate with elasticsearch + * + * @param {Function} callCluster should accept a method name as a string and a params object + * @return {UiSettingsService} + */ +export function uiSettingsServiceFactory(server, options = {}) { + const config = server.config(); + + const { + callCluster, + readInterceptor + } = options; + + return new UiSettingsService({ + index: config.get('kibana.index'), + type: 'config', + id: config.get('pkg.version'), + callCluster, + readInterceptor, + }); +} diff --git a/src/ui/ui_settings/ui_settings_for_request.js b/src/ui/ui_settings/ui_settings_for_request.js new file mode 100644 index 0000000000000..6b8008f3c8f05 --- /dev/null +++ b/src/ui/ui_settings/ui_settings_for_request.js @@ -0,0 +1,20 @@ +import { uiSettingsServiceFactory } from './ui_settings_factory'; + +const BY_REQUEST_CACHE = new WeakMap(); + +export function getUiSettingsServiceForRequest(server, request, readInterceptor) { + if (BY_REQUEST_CACHE.has(request)) { + return BY_REQUEST_CACHE.get(request); + } + + const adminCluster = server.plugins.elasticsearch.getCluster('admin'); + const uiSettingsServices = uiSettingsServiceFactory(server, { + readInterceptor, + callCluster(...args) { + return adminCluster.callWithRequest(request, ...args); + } + }); + + BY_REQUEST_CACHE.set(request, uiSettingsServices); + return uiSettingsServices; +} diff --git a/src/ui/ui_settings/ui_settings_mixin.js b/src/ui/ui_settings/ui_settings_mixin.js index c8d37abbafc32..adde9238f3ba8 100644 --- a/src/ui/ui_settings/ui_settings_mixin.js +++ b/src/ui/ui_settings/ui_settings_mixin.js @@ -1,4 +1,5 @@ -import { UiSettings } from './ui_settings'; +import { uiSettingsServiceFactory } from './ui_settings_factory'; +import { getUiSettingsServiceForRequest } from './ui_settings_for_request'; import { mirrorStatus } from './mirror_status'; export function uiSettingsMixin(kbnServer, server, config) { @@ -9,9 +10,36 @@ export function uiSettingsMixin(kbnServer, server, config) { return; } - const uiSettings = new UiSettings(server, status); - server.decorate('server', 'uiSettings', () => uiSettings); + // Passed to the UiSettingsService. + // UiSettingsService calls the function before trying to read data from + // elasticsearch, giving us a chance to prevent it from happening. + // + // If the ui settings status isn't green we shouldn't be attempting to get + // user settings, since we can't be sure that all the necessary conditions + // (e.g. elasticsearch being available) are met. + const readUiSettingsInterceptor = () => { + if (status.state !== 'green') { + return {}; + } + }; + + // don't return, just let it happen when the plugins are ready kbnServer.ready().then(() => { mirrorStatus(status, kbnServer.status.getForPluginId('elasticsearch')); }); + + server.decorate('server', 'uiSettingsServiceFactory', function (options) { + return uiSettingsServiceFactory(server, options); + }); + + server.decorate('request', 'getUiSettingsService', function () { + return getUiSettingsServiceForRequest(server, this, readUiSettingsInterceptor); + }); + + server.decorate('server', 'uiSettings', () => { + throw new Error(` + server.uiSettings has been removed, use \`request.getUiSettingsService()\` + or \`server.uiSettingsServiceFactory(options)\` instead. + `); + }); } From 04a543d076e1402381f6be3f815d8c7efa312ac5 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 11:26:09 -0700 Subject: [PATCH 02/18] [server/uiSettings] disambiguate UiSettings/Service --- src/ui/ui_settings/ui_settings_mixin.js | 4 ++-- src/ui/ui_settings/{ui_settings.js => ui_settings_service.js} | 0 ...{ui_settings_factory.js => ui_settings_service_factory.js} | 2 +- ...ings_for_request.js => ui_settings_service_for_request.js} | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename src/ui/ui_settings/{ui_settings.js => ui_settings_service.js} (100%) rename src/ui/ui_settings/{ui_settings_factory.js => ui_settings_service_factory.js} (91%) rename src/ui/ui_settings/{ui_settings_for_request.js => ui_settings_service_for_request.js} (88%) diff --git a/src/ui/ui_settings/ui_settings_mixin.js b/src/ui/ui_settings/ui_settings_mixin.js index adde9238f3ba8..7db9d9f67c20b 100644 --- a/src/ui/ui_settings/ui_settings_mixin.js +++ b/src/ui/ui_settings/ui_settings_mixin.js @@ -1,5 +1,5 @@ -import { uiSettingsServiceFactory } from './ui_settings_factory'; -import { getUiSettingsServiceForRequest } from './ui_settings_for_request'; +import { uiSettingsServiceFactory } from './ui_settings_service_factory'; +import { getUiSettingsServiceForRequest } from './ui_settings_service_for_request'; import { mirrorStatus } from './mirror_status'; export function uiSettingsMixin(kbnServer, server, config) { diff --git a/src/ui/ui_settings/ui_settings.js b/src/ui/ui_settings/ui_settings_service.js similarity index 100% rename from src/ui/ui_settings/ui_settings.js rename to src/ui/ui_settings/ui_settings_service.js diff --git a/src/ui/ui_settings/ui_settings_factory.js b/src/ui/ui_settings/ui_settings_service_factory.js similarity index 91% rename from src/ui/ui_settings/ui_settings_factory.js rename to src/ui/ui_settings/ui_settings_service_factory.js index 27de9d1bcbb1f..12e220ebbebbd 100644 --- a/src/ui/ui_settings/ui_settings_factory.js +++ b/src/ui/ui_settings/ui_settings_service_factory.js @@ -1,4 +1,4 @@ -import { UiSettingsService } from './ui_settings'; +import { UiSettingsService } from './ui_settings_service'; /** * Create an instance of UiSettingsService that will use the diff --git a/src/ui/ui_settings/ui_settings_for_request.js b/src/ui/ui_settings/ui_settings_service_for_request.js similarity index 88% rename from src/ui/ui_settings/ui_settings_for_request.js rename to src/ui/ui_settings/ui_settings_service_for_request.js index 6b8008f3c8f05..4c468cf2a1a2b 100644 --- a/src/ui/ui_settings/ui_settings_for_request.js +++ b/src/ui/ui_settings/ui_settings_service_for_request.js @@ -1,4 +1,4 @@ -import { uiSettingsServiceFactory } from './ui_settings_factory'; +import { uiSettingsServiceFactory } from './ui_settings_service_factory'; const BY_REQUEST_CACHE = new WeakMap(); From a0b7f1f79bfa151a56d387f2f8a04d3e23f855aa Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 11:35:05 -0700 Subject: [PATCH 03/18] [server/uiSettings] link to PR in removal error --- src/ui/ui_settings/ui_settings_mixin.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ui/ui_settings/ui_settings_mixin.js b/src/ui/ui_settings/ui_settings_mixin.js index 7db9d9f67c20b..92651ef59e7e5 100644 --- a/src/ui/ui_settings/ui_settings_mixin.js +++ b/src/ui/ui_settings/ui_settings_mixin.js @@ -38,8 +38,7 @@ export function uiSettingsMixin(kbnServer, server, config) { server.decorate('server', 'uiSettings', () => { throw new Error(` - server.uiSettings has been removed, use \`request.getUiSettingsService()\` - or \`server.uiSettingsServiceFactory(options)\` instead. + server.uiSettings has been removed, see https://github.com/elastic/kibana/pull/12243. `); }); } From f3221b83dad0d54f9edf0a434f7bfcc2470d9165 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 12:43:38 -0700 Subject: [PATCH 04/18] [server/uiSettings] await _read before hydrating --- src/ui/ui_settings/ui_settings_service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/ui_settings/ui_settings_service.js b/src/ui/ui_settings/ui_settings_service.js index faff64ad32c4f..04fac9da78ea8 100644 --- a/src/ui/ui_settings/ui_settings_service.js +++ b/src/ui/ui_settings/ui_settings_service.js @@ -56,7 +56,7 @@ export class UiSettingsService { } async getUserProvided(options) { - return hydrateUserSettings(this._read(options)); + return hydrateUserSettings(await this._read(options)); } async _read(options = {}) { From c583035b9dea70ad82f230acbe4f6fb38fe6c927 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 12:44:27 -0700 Subject: [PATCH 05/18] [server/uiSettings] focus tests, remove server integration --- src/ui/ui_settings/__tests__/index.js | 497 ------------------ .../__tests__/lib/call_cluster_stub.js | 48 ++ src/ui/ui_settings/__tests__/lib/index.js | 2 + src/ui/ui_settings/__tests__/lib/promises.js | 17 + .../__tests__/ui_settings_service.js | 363 +++++++++++++ 5 files changed, 430 insertions(+), 497 deletions(-) delete mode 100644 src/ui/ui_settings/__tests__/index.js create mode 100644 src/ui/ui_settings/__tests__/lib/call_cluster_stub.js create mode 100644 src/ui/ui_settings/__tests__/lib/index.js create mode 100644 src/ui/ui_settings/__tests__/lib/promises.js create mode 100644 src/ui/ui_settings/__tests__/ui_settings_service.js diff --git a/src/ui/ui_settings/__tests__/index.js b/src/ui/ui_settings/__tests__/index.js deleted file mode 100644 index 40c681f2b9765..0000000000000 --- a/src/ui/ui_settings/__tests__/index.js +++ /dev/null @@ -1,497 +0,0 @@ -import { isEqual } from 'lodash'; -import sinon from 'sinon'; -import expect from 'expect.js'; -import { uiSettingsMixin } from '../ui_settings_mixin'; -import { getDefaultSettings } from '../defaults'; -import { errors as esErrors } from 'elasticsearch'; - -async function expectRejection(promise, errorMessageContain) { - if (!promise || typeof promise.then !== 'function') { - throw new Error('Expected function to return a promise'); - } - - try { - await promise; - } catch (err) { - expect(err.message).to.contain(errorMessageContain); - } -} - -describe('ui settings', function () { - describe('overview', function () { - it('has expected api surface', function () { - const { uiSettings } = instantiate(); - expect(typeof uiSettings.get).to.equal('function'); - expect(typeof uiSettings.getAll).to.equal('function'); - expect(typeof uiSettings.getDefaults).to.equal('function'); - expect(typeof uiSettings.getRaw).to.equal('function'); - expect(typeof uiSettings.getUserProvided).to.equal('function'); - expect(typeof uiSettings.remove).to.equal('function'); - expect(typeof uiSettings.removeMany).to.equal('function'); - expect(typeof uiSettings.set).to.equal('function'); - expect(typeof uiSettings.setMany).to.equal('function'); - }); - - it('throws if the first error is not a request', async () => { - const { uiSettings } = instantiate(); - await expectRejection(uiSettings.get(null), 'hapi.Request'); - await expectRejection(uiSettings.get(false), 'hapi.Request'); - await expectRejection(uiSettings.get('key'), 'hapi.Request'); - await expectRejection(uiSettings.get(/regex/), 'hapi.Request'); - await expectRejection(uiSettings.get(new Date()), 'hapi.Request'); - await expectRejection(uiSettings.get({}), 'hapi.Request'); - await expectRejection(uiSettings.get({ path:'' }), 'hapi.Request'); - await expectRejection(uiSettings.get({ path:'', headers:null }), 'hapi.Request'); - await expectRejection(uiSettings.get({ headers:{} }), 'hapi.Request'); - }); - }); - - describe('#setMany()', function () { - it('returns a promise', () => { - const { uiSettings, req } = instantiate(); - const result = uiSettings.setMany(req, { a: 'b' }); - expect(result).to.be.a(Promise); - }); - - it('updates a single value in one operation', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.setMany(req, { one: 'value' }); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: 'value' - }); - }); - - it('updates several values in one operation', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.setMany(req, { one: 'value', another: 'val' }); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: 'value', another: 'val' - }); - }); - }); - - describe('#set()', function () { - it('returns a promise', () => { - const { uiSettings, req } = instantiate(); - const result = uiSettings.set(req, 'a', 'b'); - expect(result).to.be.a(Promise); - }); - - it('updates single values by (key, value)', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.set(req, 'one', 'value'); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: 'value' - }); - }); - }); - - describe('#remove()', function () { - it('returns a promise', () => { - const { uiSettings, req } = instantiate(); - const result = uiSettings.remove(req, 'one'); - expect(result).to.be.a(Promise); - }); - - it('removes single values by key', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.remove(req, 'one'); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: null - }); - }); - }); - - describe('#removeMany()', function () { - it('returns a promise', () => { - const { uiSettings, req } = instantiate(); - const result = uiSettings.removeMany(req, ['one']); - expect(result).to.be.a(Promise); - }); - - it('removes a single value', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.removeMany(req, ['one']); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: null - }); - }); - - it('updates several values in one operation', function () { - const { server, uiSettings, configGet, req } = instantiate(); - uiSettings.removeMany(req, ['one', 'two', 'three']); - expectElasticsearchUpdateQuery(server, req, configGet, { - one: null, two: null, three: null - }); - }); - }); - - describe('#getDefaults()', function () { - it('is promised the default values', async function () { - const { - uiSettings - } = instantiate(); - const defaults = await uiSettings.getDefaults(); - expect(isEqual(defaults, getDefaultSettings())).to.equal(true); - }); - - - describe('defaults for formatters', async function () { - - const defaults = getDefaultSettings(); - const mapping = JSON.parse(defaults['format:defaultTypeMap'].value); - const expected = { - ip: { id: 'ip', params: {} }, - date: { id: 'date', params: {} }, - number: { id: 'number', params: {} }, - boolean: { id: 'boolean', params: {} }, - _source: { id: '_source', params: {} }, - _default_: { id: 'string', params: {} } - }; - - Object.keys(mapping).forEach(function (dataType) { - it(`should configure ${dataType}`, function () { - expect(expected.hasOwnProperty(dataType)).to.equal(true); - expect(mapping[dataType].id).to.equal(expected[dataType].id); - expect(JSON.stringify(mapping[dataType].params)).to.equal(JSON.stringify(expected[dataType].params)); - }); - }); - }); - }); - - describe('#getUserProvided()', function () { - it('pulls user configuration from ES', async function () { - const getResult = { user: 'customized' }; - const { server, uiSettings, configGet, req } = instantiate({ getResult }); - await uiSettings.getUserProvided(req); - expectElasticsearchGetQuery(server, req, configGet); - }); - - it('returns user configuration', async function () { - const getResult = { user: 'customized' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getUserProvided(req); - expect(isEqual(result, { - user: { userValue: 'customized' } - })).to.equal(true); - }); - - it('ignores null user configuration (because default values)', async function () { - const getResult = { user: 'customized', usingDefault: null, something: 'else' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getUserProvided(req); - expect(isEqual(result, { - user: { userValue: 'customized' }, something: { userValue: 'else' } - })).to.equal(true); - }); - - it('returns an empty object when status is not green', async function () { - const { uiSettings, req } = instantiate({ - settingsStatusOverrides: { state: 'yellow' } - }); - - expect(await uiSettings.getUserProvided(req)).to.eql({}); - }); - - it('returns an empty object on 404 responses', async function () { - const { uiSettings, req } = instantiate({ - async callWithRequest() { - throw new esErrors[404](); - } - }); - - expect(await uiSettings.getUserProvided(req)).to.eql({}); - }); - - it('returns an empty object on 403 responses', async function () { - const { uiSettings, req } = instantiate({ - async callWithRequest() { - throw new esErrors[403](); - } - }); - - expect(await uiSettings.getUserProvided(req)).to.eql({}); - }); - - it('returns an empty object on NoConnections responses', async function () { - const { uiSettings, req } = instantiate({ - async callWithRequest() { - throw new esErrors.NoConnections(); - } - }); - - expect(await uiSettings.getUserProvided(req)).to.eql({}); - }); - - it('throws 401 errors', async function () { - const { uiSettings, req } = instantiate({ - async callWithRequest() { - throw new esErrors[401](); - } - }); - - try { - await uiSettings.getUserProvided(req); - throw new Error('expect getUserProvided() to throw'); - } catch (err) { - expect(err).to.be.a(esErrors[401]); - } - }); - - it('throw when callWithRequest fails in some unexpected way', async function () { - const expectedUnexpectedError = new Error('unexpected'); - - const { uiSettings, req } = instantiate({ - async callWithRequest() { - throw expectedUnexpectedError; - } - }); - - try { - await uiSettings.getUserProvided(req); - throw new Error('expect getUserProvided() to throw'); - } catch (err) { - expect(err).to.be(expectedUnexpectedError); - } - }); - }); - - describe('#getRaw()', function () { - it('pulls user configuration from ES', async function () { - const getResult = {}; - const { server, uiSettings, configGet, req } = instantiate({ getResult }); - await uiSettings.getRaw(req); - expectElasticsearchGetQuery(server, req, configGet); - }); - - it(`without user configuration it's equal to the defaults`, async function () { - const getResult = {}; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getRaw(req); - expect(isEqual(result, getDefaultSettings())).to.equal(true); - }); - - it(`user configuration gets merged with defaults`, async function () { - const getResult = { foo: 'bar' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getRaw(req); - const merged = getDefaultSettings(); - merged.foo = { userValue: 'bar' }; - expect(isEqual(result, merged)).to.equal(true); - }); - - it(`user configuration gets merged into defaults`, async function () { - const getResult = { dateFormat: 'YYYY-MM-DD' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getRaw(req); - const merged = getDefaultSettings(); - merged.dateFormat.userValue = 'YYYY-MM-DD'; - expect(isEqual(result, merged)).to.equal(true); - }); - }); - - describe('#getAll()', function () { - it('pulls user configuration from ES', async function () { - const getResult = {}; - const { server, uiSettings, configGet, req } = instantiate({ getResult }); - await uiSettings.getAll(req); - expectElasticsearchGetQuery(server, req, configGet); - }); - - it(`returns key value pairs`, async function () { - const getResult = {}; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getAll(req); - const defaults = getDefaultSettings(); - const expectation = {}; - Object.keys(defaults).forEach(key => { - expectation[key] = defaults[key].value; - }); - expect(isEqual(result, expectation)).to.equal(true); - }); - - it(`returns key value pairs including user configuration`, async function () { - const getResult = { something: 'user-provided' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getAll(req); - const defaults = getDefaultSettings(); - const expectation = {}; - Object.keys(defaults).forEach(key => { - expectation[key] = defaults[key].value; - }); - expectation.something = 'user-provided'; - expect(isEqual(result, expectation)).to.equal(true); - }); - - it(`returns key value pairs including user configuration for existing settings`, async function () { - const getResult = { dateFormat: 'YYYY-MM-DD' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.getAll(req); - const defaults = getDefaultSettings(); - const expectation = {}; - Object.keys(defaults).forEach(key => { - expectation[key] = defaults[key].value; - }); - expectation.dateFormat = 'YYYY-MM-DD'; - expect(isEqual(result, expectation)).to.equal(true); - }); - }); - - describe('#get()', function () { - it('pulls user configuration from ES', async function () { - const getResult = {}; - const { server, uiSettings, configGet, req } = instantiate({ getResult }); - await uiSettings.get(req); - expectElasticsearchGetQuery(server, req, configGet); - }); - - it(`returns the promised value for a key`, async function () { - const getResult = {}; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.get(req, 'dateFormat'); - const defaults = getDefaultSettings(); - expect(result).to.equal(defaults.dateFormat.value); - }); - - it(`returns the user-configured value for a custom key`, async function () { - const getResult = { custom: 'value' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.get(req, 'custom'); - expect(result).to.equal('value'); - }); - - it(`returns the user-configured value for a modified key`, async function () { - const getResult = { dateFormat: 'YYYY-MM-DD' }; - const { - uiSettings, - req - } = instantiate({ getResult }); - const result = await uiSettings.get(req, 'dateFormat'); - expect(result).to.equal('YYYY-MM-DD'); - }); - }); -}); - -function expectElasticsearchGetQuery(server, req, configGet) { - const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin'); - sinon.assert.calledOnce(callWithRequest); - const [reqPassed, method, params] = callWithRequest.args[0]; - expect(reqPassed).to.be(req); - expect(method).to.be('get'); - expect(params).to.eql({ - index: configGet('kibana.index'), - id: configGet('pkg.version'), - type: 'config' - }); -} - -function expectElasticsearchUpdateQuery(server, req, configGet, doc) { - const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin'); - sinon.assert.calledOnce(callWithRequest); - const [reqPassed, method, params] = callWithRequest.args[0]; - expect(reqPassed).to.be(req); - expect(method).to.be('update'); - expect(params).to.eql({ - index: configGet('kibana.index'), - id: configGet('pkg.version'), - type: 'config', - body: { doc } - }); -} - -function instantiate({ getResult, callWithRequest, settingsStatusOverrides } = {}) { - const esStatus = { - state: 'green', - on: sinon.spy() - }; - const settingsStatus = { - state: 'green', - red: sinon.spy(), - yellow: sinon.spy(), - green: sinon.spy(), - ...settingsStatusOverrides - }; - const kbnServer = { - status: { - create: sinon.stub().withArgs('ui settings').returns(settingsStatus), - getForPluginId: sinon.stub().withArgs('elasticsearch').returns(esStatus) - }, - ready: sinon.stub().returns(Promise.resolve()) - }; - - const req = { __stubHapiRequest: true, path: '', headers: {} }; - - const adminCluster = { - errors: esErrors, - callWithInternalUser: sinon.stub(), - callWithRequest: sinon.spy((withReq, method, params) => { - if (callWithRequest) { - return callWithRequest(withReq, method, params); - } - - expect(withReq).to.be(req); - switch (method) { - case 'get': - return Promise.resolve({ _source: getResult }); - case 'update': - return Promise.resolve(); - default: - throw new Error(`callWithRequest() is using unexpected method "${method}"`); - } - }) - }; - - adminCluster.callWithInternalUser.withArgs('get', sinon.match.any).returns(Promise.resolve({ _source: getResult })); - adminCluster.callWithInternalUser.withArgs('update', sinon.match.any).returns(Promise.resolve()); - - const configGet = sinon.stub(); - configGet.withArgs('kibana.index').returns('.kibana'); - configGet.withArgs('pkg.version').returns('1.2.3-test'); - configGet.withArgs('uiSettings.enabled').returns(true); - const config = { - get: configGet - }; - - const server = { - config: () => config, - decorate: (_, key, value) => server[key] = value, - plugins: { - elasticsearch: { - getCluster: sinon.stub().withArgs('admin').returns(adminCluster) - } - } - }; - uiSettingsMixin(kbnServer, server, config); - const uiSettings = server.uiSettings(); - return { server, uiSettings, configGet, req }; -} diff --git a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js new file mode 100644 index 0000000000000..de2639632f59a --- /dev/null +++ b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js @@ -0,0 +1,48 @@ +import sinon from 'sinon'; +import expect from 'expect.js'; +import { merge } from 'lodash'; + +export function createCallClusterStub(index, type, id, esDocSource) { + const currentEsDocValue = merge({}, esDocSource); + + const callCluster = sinon.spy(async (method, params) => { + expect(params) + .to.have.property('index', index) + .and.to.have.property('type', type) + .and.to.have.property('id', id); + + switch (method) { + case 'get': + return { _source: merge({}, currentEsDocValue) }; + + case 'update': + expect(params).to.have.property('body'); + expect(params.body).to.have.property('doc'); + merge(currentEsDocValue, params.body.doc); + return {}; + + default: + throw new Error(`unexpected es method ${method}`); + } + }); + + callCluster.assertGetQuery = () => { + sinon.assert.calledOnce(callCluster); + sinon.assert.calledWith(callCluster, 'get'); + }; + + callCluster.assertUpdateQuery = doc => { + sinon.assert.calledOnce(callCluster); + expect(callCluster.firstCall.args).to.eql([ + 'update', + { + index, + type, + id, + body: { doc }, + } + ]); + }; + + return callCluster; +} diff --git a/src/ui/ui_settings/__tests__/lib/index.js b/src/ui/ui_settings/__tests__/lib/index.js new file mode 100644 index 0000000000000..a8381d6a1502f --- /dev/null +++ b/src/ui/ui_settings/__tests__/lib/index.js @@ -0,0 +1,2 @@ +export { createCallClusterStub } from './call_cluster_stub'; +export { assertPromise, assertRejection } from './promises'; diff --git a/src/ui/ui_settings/__tests__/lib/promises.js b/src/ui/ui_settings/__tests__/lib/promises.js new file mode 100644 index 0000000000000..dca524429a4ee --- /dev/null +++ b/src/ui/ui_settings/__tests__/lib/promises.js @@ -0,0 +1,17 @@ +import expect from 'expect.js'; + +export function assertPromise(promise) { + if (!promise || typeof promise.then !== 'function') { + throw new Error('Expected function to return a promise'); + } +} + +export async function assertRejection(promise, errorMessageContain) { + assertPromise(promise); + + try { + await promise; + } catch (err) { + expect(err.message).to.contain(errorMessageContain); + } +} diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js new file mode 100644 index 0000000000000..1763a82ca76d4 --- /dev/null +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -0,0 +1,363 @@ +import { isEqual } from 'lodash'; +import expect from 'expect.js'; +import { errors as esErrors } from 'elasticsearch'; + +import { getDefaultSettings } from '../defaults'; +import { UiSettingsService } from '../ui_settings_service'; + +import { + assertPromise, + assertRejection, + createCallClusterStub, +} from './lib'; + +const INDEX = '.kibana'; +const TYPE = 'config'; +const ID = 'kibana-version'; + +function setup(options = {}) { + const { + esDocSource = {}, + callCluster = createCallClusterStub(INDEX, TYPE, ID, esDocSource) + } = options; + + const uiSettings = new UiSettingsService({ + index: INDEX, + type: TYPE, + id: ID, + callCluster, + }); + + return { + uiSettings, + assertGetQuery: callCluster.assertGetQuery, + assertUpdateQuery: callCluster.assertUpdateQuery, + }; +} + +describe('ui settings', function () { + describe('overview', function () { + it('has expected api surface', function () { + const { uiSettings } = setup(); + expect(typeof uiSettings.get).to.equal('function'); + expect(typeof uiSettings.getAll).to.equal('function'); + expect(typeof uiSettings.getDefaults).to.equal('function'); + expect(typeof uiSettings.getRaw).to.equal('function'); + expect(typeof uiSettings.getUserProvided).to.equal('function'); + expect(typeof uiSettings.remove).to.equal('function'); + expect(typeof uiSettings.removeMany).to.equal('function'); + expect(typeof uiSettings.set).to.equal('function'); + expect(typeof uiSettings.setMany).to.equal('function'); + }); + + it('throws if the first error is not a request', async () => { + const { uiSettings } = setup(); + await assertRejection(uiSettings.get(null), 'hapi.Request'); + await assertRejection(uiSettings.get(false), 'hapi.Request'); + await assertRejection(uiSettings.get('key'), 'hapi.Request'); + await assertRejection(uiSettings.get(/regex/), 'hapi.Request'); + await assertRejection(uiSettings.get(new Date()), 'hapi.Request'); + await assertRejection(uiSettings.get({}), 'hapi.Request'); + await assertRejection(uiSettings.get({ path:'' }), 'hapi.Request'); + await assertRejection(uiSettings.get({ path:'', headers:null }), 'hapi.Request'); + await assertRejection(uiSettings.get({ headers:{} }), 'hapi.Request'); + }); + }); + + describe('#setMany()', function () { + it('returns a promise', () => { + const { uiSettings } = setup(); + assertPromise(uiSettings.setMany({ a: 'b' })); + }); + + it('updates a single value in one operation', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.setMany({ one: 'value' }); + assertUpdateQuery({ one: 'value' }); + }); + + it('updates several values in one operation', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.setMany({ one: 'value', another: 'val' }); + assertUpdateQuery({ one: 'value', another: 'val' }); + }); + }); + + describe('#set()', function () { + it('returns a promise', () => { + const { uiSettings } = setup(); + assertPromise(uiSettings.set('a', 'b')); + }); + + it('updates single values by (key, value)', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.set('one', 'value'); + assertUpdateQuery({ one: 'value' }); + }); + }); + + describe('#remove()', function () { + it('returns a promise', () => { + const { uiSettings } = setup(); + assertPromise(uiSettings.remove('one')); + }); + + it('removes single values by key', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.remove('one'); + assertUpdateQuery({ one: null }); + }); + }); + + describe('#removeMany()', function () { + it('returns a promise', () => { + const { uiSettings } = setup(); + assertPromise(uiSettings.removeMany(['one'])); + }); + + it('removes a single value', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.removeMany(['one']); + assertUpdateQuery({ one: null }); + }); + + it('updates several values in one operation', async function () { + const { uiSettings, assertUpdateQuery } = setup(); + await uiSettings.removeMany(['one', 'two', 'three']); + assertUpdateQuery({ one: null, two: null, three: null }); + }); + }); + + describe('#getDefaults()', function () { + it('is promised the default values', async function () { + const { + uiSettings + } = setup(); + const defaults = await uiSettings.getDefaults(); + expect(isEqual(defaults, getDefaultSettings())).to.equal(true); + }); + + + describe('defaults for formatters', async function () { + + const defaults = getDefaultSettings(); + const mapping = JSON.parse(defaults['format:defaultTypeMap'].value); + const expected = { + ip: { id: 'ip', params: {} }, + date: { id: 'date', params: {} }, + number: { id: 'number', params: {} }, + boolean: { id: 'boolean', params: {} }, + _source: { id: '_source', params: {} }, + _default_: { id: 'string', params: {} } + }; + + Object.keys(mapping).forEach(function (dataType) { + it(`should configure ${dataType}`, function () { + expect(expected.hasOwnProperty(dataType)).to.equal(true); + expect(mapping[dataType].id).to.equal(expected[dataType].id); + expect(JSON.stringify(mapping[dataType].params)).to.equal(JSON.stringify(expected[dataType].params)); + }); + }); + }); + }); + + describe('#getUserProvided()', function () { + it('pulls user configuration from ES', async function () { + const { uiSettings, assertGetQuery } = setup(); + await uiSettings.getUserProvided(); + assertGetQuery(); + }); + + it('returns user configuration', async function () { + const esDocSource = { user: 'customized' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getUserProvided(); + expect(isEqual(result, { + user: { userValue: 'customized' } + })).to.equal(true); + }); + + it('ignores null user configuration (because default values)', async function () { + const esDocSource = { user: 'customized', usingDefault: null, something: 'else' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getUserProvided(); + expect(isEqual(result, { + user: { userValue: 'customized' }, something: { userValue: 'else' } + })).to.equal(true); + }); + + it('returns an empty object on 404 responses', async function () { + const { uiSettings } = setup({ + async callCluster() { + throw new esErrors[404](); + } + }); + + expect(await uiSettings.getUserProvided()).to.eql({}); + }); + + it('returns an empty object on 403 responses', async function () { + const { uiSettings } = setup({ + async callCluster() { + throw new esErrors[403](); + } + }); + + expect(await uiSettings.getUserProvided()).to.eql({}); + }); + + it('returns an empty object on NoConnections responses', async function () { + const { uiSettings } = setup({ + async callCluster() { + throw new esErrors.NoConnections(); + } + }); + + expect(await uiSettings.getUserProvided()).to.eql({}); + }); + + it('throws 401 errors', async function () { + const { uiSettings } = setup({ + async callCluster() { + throw new esErrors[401](); + } + }); + + try { + await uiSettings.getUserProvided(); + throw new Error('expect getUserProvided() to throw'); + } catch (err) { + expect(err).to.be.a(esErrors[401]); + } + }); + + it('throw when callCluster fails in some unexpected way', async function () { + const expectedUnexpectedError = new Error('unexpected'); + + const { uiSettings } = setup({ + async callCluster() { + throw expectedUnexpectedError; + } + }); + + try { + await uiSettings.getUserProvided(); + throw new Error('expect getUserProvided() to throw'); + } catch (err) { + expect(err).to.be(expectedUnexpectedError); + } + }); + }); + + describe('#getRaw()', function () { + it('pulls user configuration from ES', async function () { + const esDocSource = {}; + const { uiSettings, assertGetQuery } = setup({ esDocSource }); + await uiSettings.getRaw(); + assertGetQuery(); + }); + + it(`without user configuration it's equal to the defaults`, async function () { + const esDocSource = {}; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getRaw(); + expect(isEqual(result, getDefaultSettings())).to.equal(true); + }); + + it(`user configuration gets merged with defaults`, async function () { + const esDocSource = { foo: 'bar' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getRaw(); + const merged = getDefaultSettings(); + merged.foo = { userValue: 'bar' }; + expect(isEqual(result, merged)).to.equal(true); + }); + + it(`user configuration gets merged into defaults`, async function () { + const esDocSource = { dateFormat: 'YYYY-MM-DD' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getRaw(); + const merged = getDefaultSettings(); + merged.dateFormat.userValue = 'YYYY-MM-DD'; + expect(isEqual(result, merged)).to.equal(true); + }); + }); + + describe('#getAll()', function () { + it('pulls user configuration from ES', async function () { + const esDocSource = {}; + const { uiSettings, assertGetQuery } = setup({ esDocSource }); + await uiSettings.getAll(); + assertGetQuery(); + }); + + it(`returns key value pairs`, async function () { + const esDocSource = {}; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getAll(); + const defaults = getDefaultSettings(); + const expectation = {}; + Object.keys(defaults).forEach(key => { + expectation[key] = defaults[key].value; + }); + expect(isEqual(result, expectation)).to.equal(true); + }); + + it(`returns key value pairs including user configuration`, async function () { + const esDocSource = { something: 'user-provided' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getAll(); + const defaults = getDefaultSettings(); + const expectation = {}; + Object.keys(defaults).forEach(key => { + expectation[key] = defaults[key].value; + }); + expectation.something = 'user-provided'; + expect(isEqual(result, expectation)).to.equal(true); + }); + + it(`returns key value pairs including user configuration for existing settings`, async function () { + const esDocSource = { dateFormat: 'YYYY-MM-DD' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.getAll(); + const defaults = getDefaultSettings(); + const expectation = {}; + Object.keys(defaults).forEach(key => { + expectation[key] = defaults[key].value; + }); + expectation.dateFormat = 'YYYY-MM-DD'; + expect(isEqual(result, expectation)).to.equal(true); + }); + }); + + describe('#get()', function () { + it('pulls user configuration from ES', async function () { + const esDocSource = {}; + const { uiSettings, assertGetQuery } = setup({ esDocSource }); + await uiSettings.get(); + assertGetQuery(); + }); + + it(`returns the promised value for a key`, async function () { + const esDocSource = {}; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.get('dateFormat'); + const defaults = getDefaultSettings(); + expect(result).to.equal(defaults.dateFormat.value); + }); + + it(`returns the user-configured value for a custom key`, async function () { + const esDocSource = { custom: 'value' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.get('custom'); + expect(result).to.equal('value'); + }); + + it(`returns the user-configured value for a modified key`, async function () { + const esDocSource = { dateFormat: 'YYYY-MM-DD' }; + const { uiSettings } = setup({ esDocSource }); + const result = await uiSettings.get('dateFormat'); + expect(result).to.equal('YYYY-MM-DD'); + }); + }); +}); From 04a1658862ca7417a7a42e391d6013b388b936b7 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 12:46:37 -0700 Subject: [PATCH 06/18] [server/uiSettings] add tests for readInterceptor() arg --- .../__tests__/ui_settings_service.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js index 1763a82ca76d4..3c6ef81d6f287 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_service.js +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -17,6 +17,7 @@ const ID = 'kibana-version'; function setup(options = {}) { const { + readInterceptor, esDocSource = {}, callCluster = createCallClusterStub(INDEX, TYPE, ID, esDocSource) } = options; @@ -25,6 +26,7 @@ function setup(options = {}) { index: INDEX, type: TYPE, id: ID, + readInterceptor, callCluster, }); @@ -360,4 +362,49 @@ describe('ui settings', function () { expect(result).to.equal('YYYY-MM-DD'); }); }); + + describe('readInterceptor() argument', () => { + describe('#getUserProvided()', () => { + it('returns a promise when interceptValue doesn\'t', () => { + const { uiSettings } = setup({ readInterceptor: () => ({}) }); + assertPromise(uiSettings.getUserProvided()); + }); + + it('returns intercept values', async () => { + const { uiSettings } = setup({ + readInterceptor: () => ({ + foo: 'bar' + }) + }); + + expect(await uiSettings.getUserProvided()).to.eql({ + foo: { + userValue: 'bar' + } + }); + }); + }); + + describe('#getAll()', () => { + it('merges intercept value with defaults', async () => { + const { uiSettings } = setup({ + readInterceptor: () => ({ + foo: 'not foo' + }), + }); + + const defaults = getDefaultSettings(); + const defaultValues = Object.keys(defaults).reduce((acc, key) => ({ + ...acc, + [key]: defaults[key].value, + }), {}); + + expect(await uiSettings.getAll()).to.eql({ + ...defaultValues, + foo: 'not foo', + }); + }); + }); + }); + }); From bd38111e7e9875792f541d59bf45c62930ae22ce Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 15:30:31 -0700 Subject: [PATCH 07/18] [server/uiSettings] add server integration tests --- .../ui_settings_mixin_integration.js | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js diff --git a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js new file mode 100644 index 0000000000000..fba39fb31afe6 --- /dev/null +++ b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js @@ -0,0 +1,194 @@ +import sinon from 'sinon'; +import expect from 'expect.js'; +import Chance from 'chance'; + +import ServerStatus from '../../../server/status/server_status'; +import Config from '../../../server/config/config'; + +import * as uiSettingsServiceFactoryNS from '../ui_settings_service_factory'; +import { uiSettingsServiceFactory } from '../ui_settings_service_factory'; +import * as getUiSettingsServiceForRequestNS from '../ui_settings_service_for_request'; +import { getUiSettingsServiceForRequest } from '../ui_settings_service_for_request'; +import { uiSettingsMixin } from '../ui_settings_mixin'; + +const chance = new Chance(); + +describe('uiSettingsMixin()', () => { + const sandbox = sinon.sandbox.create(); + + function setup(options = {}) { + const { + enabled = true + } = options; + + const config = Config.withDefaultSchema({ + uiSettings: { enabled } + }); + + // maps of decorations passed to `server.decorate()` + const decorations = { + server: {}, + request: {} + }; + + // mock hapi server + const server = { + log: sinon.stub(), + config: () => config, + decorate: sinon.spy((type, name, value) => { + decorations[type][name] = value; + }), + }; + + // "promise" returned from kbnServer.ready() + const readyPromise = { + then: sinon.stub(), + }; + + const kbnServer = { + server, + config, + status: new ServerStatus(server), + ready: sinon.stub().returns(readyPromise), + }; + + uiSettingsMixin(kbnServer, server, config); + + return { + kbnServer, + server, + decorations, + readyPromise, + status: kbnServer.status.get('ui settings'), + }; + } + + afterEach(() => sandbox.restore()); + + describe('status', () => { + it('creates a "ui settings" status', () => { + const { status } = setup(); + expect(status).to.have.property('state', 'uninitialized'); + }); + + describe('disabled', () => { + it('disables if uiSettings.enabled config is false', () => { + const { status } = setup({ enabled: false }); + expect(status).to.have.property('state', 'disabled'); + }); + + it('does not register a handler for kbnServer.ready()', () => { + const { readyPromise } = setup({ enabled: false }); + sinon.assert.notCalled(readyPromise.then); + }); + }); + + describe('enabled', () => { + it('registers a handler for kbnServer.ready()', () => { + const { readyPromise } = setup(); + sinon.assert.calledOnce(readyPromise.then); + }); + + it('mirrors the elasticsearch plugin status once kibanaServer.ready() resolves', () => { + const { kbnServer, readyPromise, status } = setup(); + const esStatus = kbnServer.status.createForPlugin({ + id: 'elasticsearch', + version: 'kibana', + }); + + esStatus.green(); + expect(status).to.have.property('state', 'uninitialized'); + const readyPromiseHandler = readyPromise.then.firstCall.args[0]; + readyPromiseHandler(); + expect(status).to.have.property('state', 'green'); + + + const states = chance.shuffle(['red', 'green', 'yellow']); + states.forEach(state => { + esStatus[state](); + expect(esStatus).to.have.property('state', state); + expect(status).to.have.property('state', state); + }); + }); + }); + }); + + describe('server.uiSettingsServiceFactory()', () => { + it('decorates server with "uiSettingsServiceFactory"', () => { + const { decorations } = setup(); + expect(decorations.server).to.have.property('uiSettingsServiceFactory').a('function'); + + sandbox.stub(uiSettingsServiceFactoryNS, 'uiSettingsServiceFactory'); + sinon.assert.notCalled(uiSettingsServiceFactory); + decorations.server.uiSettingsServiceFactory(); + sinon.assert.calledOnce(uiSettingsServiceFactory); + }); + + it('passes `server` and `options` argument to factory', () => { + const { decorations, server } = setup(); + expect(decorations.server).to.have.property('uiSettingsServiceFactory').a('function'); + + sandbox.stub(uiSettingsServiceFactoryNS, 'uiSettingsServiceFactory'); + sinon.assert.notCalled(uiSettingsServiceFactory); + const football = {}; + decorations.server.uiSettingsServiceFactory(football); + sinon.assert.calledWith(uiSettingsServiceFactory, server, football); + }); + }); + + describe('request.getUiSettingsService()', () => { + it('exposes "getUiSettingsService" on requests', () => { + const { decorations } = setup(); + expect(decorations.request).to.have.property('getUiSettingsService').a('function'); + + sandbox.stub(getUiSettingsServiceForRequestNS, 'getUiSettingsServiceForRequest'); + sinon.assert.notCalled(getUiSettingsServiceForRequest); + decorations.request.getUiSettingsService(); + sinon.assert.calledOnce(getUiSettingsServiceForRequest); + }); + + it('passes request to getUiSettingsServiceForRequest', () => { + const { server, decorations } = setup(); + expect(decorations.request).to.have.property('getUiSettingsService').a('function'); + + sandbox.stub(getUiSettingsServiceForRequestNS, 'getUiSettingsServiceForRequest'); + sinon.assert.notCalled(getUiSettingsServiceForRequest); + const request = {}; + decorations.request.getUiSettingsService.call(request); + sinon.assert.calledWith(getUiSettingsServiceForRequest, server, request); + }); + + it('defines read interceptor that intercepts when status is not green', () => { + const { status, decorations } = setup(); + expect(decorations.request).to.have.property('getUiSettingsService').a('function'); + + sandbox.stub(getUiSettingsServiceForRequestNS, 'getUiSettingsServiceForRequest'); + decorations.request.getUiSettingsService(); + + const readInterceptor = getUiSettingsServiceForRequest.firstCall.args[2]; + expect(readInterceptor).to.be.a('function'); + + status.green(); + expect(readInterceptor()).to.be(undefined); + + status.yellow(); + expect(readInterceptor()).to.eql({}); + + status.red(); + expect(readInterceptor()).to.eql({}); + + status.green(); + expect(readInterceptor()).to.eql(undefined); + }); + }); + + describe('server.uiSettings()', () => { + it('throws an error, links to pr', () => { + const { decorations } = setup(); + expect(decorations.server).to.have.property('uiSettings').a('function'); + expect(() => { + decorations.server.uiSettings(); + }).to.throwError('http://github.com'); + }); + }); +}); From b74e71e4ecff9122edb008c796f6ca4fe413f340 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 15:33:22 -0700 Subject: [PATCH 08/18] [server/uiExports] fix replaceInjectedVars tests --- src/ui/__tests__/ui_exports_replace_injected_vars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/__tests__/ui_exports_replace_injected_vars.js b/src/ui/__tests__/ui_exports_replace_injected_vars.js index 6ebd06df03237..662236464c90f 100644 --- a/src/ui/__tests__/ui_exports_replace_injected_vars.js +++ b/src/ui/__tests__/ui_exports_replace_injected_vars.js @@ -40,7 +40,7 @@ describe('UiExports', function () { await kbnServer.ready(); kbnServer.status.get('ui settings').state = 'green'; - kbnServer.server.decorate('server', 'uiSettings', () => { + kbnServer.server.decorate('request', 'getUiSettingsService', () => { return { getDefaults: noop, getUserProvided: noop }; }); }); From c8a410e2d3fb8c91d220c28ae578cd2fd71c9eec Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 8 Jun 2017 16:02:56 -0700 Subject: [PATCH 09/18] [server/uiSettings] convert all methods to use async/await --- src/ui/ui_settings/ui_settings_service.js | 132 ++++++++++++---------- 1 file changed, 70 insertions(+), 62 deletions(-) diff --git a/src/ui/ui_settings/ui_settings_service.js b/src/ui/ui_settings/ui_settings_service.js index 04fac9da78ea8..4f1b48708267d 100644 --- a/src/ui/ui_settings/ui_settings_service.js +++ b/src/ui/ui_settings/ui_settings_service.js @@ -1,5 +1,4 @@ import { defaultsDeep, noop } from 'lodash'; -import Bluebird from 'bluebird'; import { errors as esErrors } from 'elasticsearch'; import { getDefaultSettings } from './defaults'; @@ -33,90 +32,99 @@ export class UiSettingsService { } // returns a Promise for the value of the requested setting - get(key) { - return this.getAll() - .then(all => all[key]); + async get(key) { + const all = await this.getAll(); + return all[key]; } - getAll() { - return this.getRaw() - .then(raw => Object.keys(raw) - .reduce((all, key) => { - const item = raw[key]; - const hasUserValue = 'userValue' in item; - all[key] = hasUserValue ? item.userValue : item.value; - return all; - }, {}) - ); + async getAll() { + const raw = await this.getRaw(); + + return Object.keys(raw) + .reduce((all, key) => { + const item = raw[key]; + const hasUserValue = 'userValue' in item; + all[key] = hasUserValue ? item.userValue : item.value; + return all; + }, {}); } - getRaw() { - return this.getUserProvided() - .then(user => defaultsDeep(user, this.getDefaults())); + async getRaw() { + const userProvided = await this.getUserProvided(); + return defaultsDeep(userProvided, this.getDefaults()); } async getUserProvided(options) { return hydrateUserSettings(await this._read(options)); } - async _read(options = {}) { - const interceptValue = await this._readInterceptor(options); - if (interceptValue != null) { - return interceptValue; - } - - const { - ignore401Errors = false - } = options; - - const params = this._getClientSettings(); - const allowedErrors = [ - esErrors[404], - esErrors[403], - esErrors.NoConnections - ]; - - if (ignore401Errors) { - allowedErrors.push(esErrors[401]); - } - - return Bluebird - .resolve(this._callCluster('get', params, { wrap401Errors: !ignore401Errors })) - .catch(...allowedErrors, () => ({})) - .then(resp => resp._source || {}); + async setMany(changes) { + await this._write(changes); } - setMany(changes) { - const clientParams = { - ...this._getClientSettings(), - body: { doc: changes } - }; - - return this._callCluster('update', clientParams) - .then(() => ({})); + async set(key, value) { + await this.setMany({ [key]: value }); } - set(key, value) { - return this.setMany({ [key]: value }); + async remove(key) { + await this.set(key, null); } - remove(key) { - return this.set(key, null); - } - - removeMany(keys) { + async removeMany(keys) { const changes = {}; keys.forEach(key => { changes[key] = null; }); - return this.setMany(changes); + await this.setMany(changes); } - _getClientSettings() { - return { + async _write(changes) { + await this._callCluster('update', { index: this._index, type: this._type, - id: this._id - }; + id: this._id, + body: { + doc: changes + } + }); + } + + async _read(options = {}) { + const interceptValue = await this._readInterceptor(options); + if (interceptValue != null) { + return interceptValue; + } + + const { + ignore401Errors = false + } = options; + + const isIgnorableError = error => ( + error instanceof esErrors[404] || + error instanceof esErrors[403] || + error instanceof esErrors.NoConnections || + (ignore401Errors && error instanceof esErrors[401]) + ); + + try { + const clientParams = { + index: this._index, + type: this._type, + id: this._id, + }; + + const callOptions = { + wrap401Errors: !ignore401Errors + }; + + const resp = await this._callCluster('get', clientParams, callOptions); + return resp._source; + } catch (error) { + if (isIgnorableError(error)) { + return {}; + } + + throw error; + } } } From 19279f3dedcdc16a7a428bd99d23a94cd99f52b7 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 12:57:39 -0700 Subject: [PATCH 10/18] [uiSettings/serviceFactory] fix doc block --- src/ui/ui_settings/ui_settings_service_factory.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ui/ui_settings/ui_settings_service_factory.js b/src/ui/ui_settings/ui_settings_service_factory.js index 12e220ebbebbd..c300bc9bc4bf9 100644 --- a/src/ui/ui_settings/ui_settings_service_factory.js +++ b/src/ui/ui_settings/ui_settings_service_factory.js @@ -4,10 +4,16 @@ import { UiSettingsService } from './ui_settings_service'; * Create an instance of UiSettingsService that will use the * passed `callCluster` function to communicate with elasticsearch * - * @param {Function} callCluster should accept a method name as a string and a params object + * @param {Hapi.Server} server + * @param {Object} options + * @property {AsyncFunction} options.callCluster function that accepts a method name and + * param object which causes a request via some elasticsearch client + * @property {AsyncFunction} [options.readInterceptor] async function that is called when the + * UiSettingsService does a read() an has an oportunity to intercept the + * request and return an alternate `_source` value to use. * @return {UiSettingsService} */ -export function uiSettingsServiceFactory(server, options = {}) { +export function uiSettingsServiceFactory(server, options) { const config = server.config(); const { From 099964c793e82752fdabdfe3e513006f239de4e0 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:01:26 -0700 Subject: [PATCH 11/18] [uiSettings/service] fix doc block --- src/ui/ui_settings/ui_settings_service.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ui/ui_settings/ui_settings_service.js b/src/ui/ui_settings/ui_settings_service.js index 4f1b48708267d..f929d79d91437 100644 --- a/src/ui/ui_settings/ui_settings_service.js +++ b/src/ui/ui_settings/ui_settings_service.js @@ -10,8 +10,22 @@ function hydrateUserSettings(userSettings) { .reduce((acc, { key, userValue }) => ({ ...acc, [key]: { userValue } }), {}); } +/** + * Service that provides access to the UiSettings stored in elasticsearch. + * + * @class UiSettingsService + * @param {Object} options + * @property {string} options.index Elasticsearch index name where settings are stored + * @property {string} options.type type of ui settings Elasticsearch doc + * @property {string} options.id id of ui settings Elasticsearch doc + * @property {AsyncFunction} options.callCluster function that accepts a method name and + * param object which causes a request via some elasticsearch client + * @property {AsyncFunction} [options.readInterceptor] async function that is called when the + * UiSettingsService does a read() an has an oportunity to intercept the + * request and return an alternate `_source` value to use. + */ export class UiSettingsService { - constructor(options = {}) { + constructor(options) { const { index, type, From 1ae25cda29c5d578e76e9b107a2a60f705b44191 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:07:47 -0700 Subject: [PATCH 12/18] [uiSettings/tests/callClusterStub] stop tracking state needlessly --- src/ui/ui_settings/__tests__/lib/call_cluster_stub.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js index de2639632f59a..94952fc643715 100644 --- a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js +++ b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js @@ -1,10 +1,7 @@ import sinon from 'sinon'; import expect from 'expect.js'; -import { merge } from 'lodash'; export function createCallClusterStub(index, type, id, esDocSource) { - const currentEsDocValue = merge({}, esDocSource); - const callCluster = sinon.spy(async (method, params) => { expect(params) .to.have.property('index', index) @@ -13,12 +10,11 @@ export function createCallClusterStub(index, type, id, esDocSource) { switch (method) { case 'get': - return { _source: merge({}, currentEsDocValue) }; + return { _source: { ...esDocSource } }; case 'update': expect(params).to.have.property('body'); expect(params.body).to.have.property('doc'); - merge(currentEsDocValue, params.body.doc); return {}; default: From 3c141b48fcfb6b6bd36555bcfc3e5d5e5cfb110a Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:14:36 -0700 Subject: [PATCH 13/18] [uiSettings/tests] remove invalid tests and pointless promise helpers --- src/ui/ui_settings/__tests__/lib/index.js | 1 - src/ui/ui_settings/__tests__/lib/promises.js | 17 ------- .../__tests__/ui_settings_service.js | 47 ++++++------------- 3 files changed, 15 insertions(+), 50 deletions(-) delete mode 100644 src/ui/ui_settings/__tests__/lib/promises.js diff --git a/src/ui/ui_settings/__tests__/lib/index.js b/src/ui/ui_settings/__tests__/lib/index.js index a8381d6a1502f..f3cd36fad2691 100644 --- a/src/ui/ui_settings/__tests__/lib/index.js +++ b/src/ui/ui_settings/__tests__/lib/index.js @@ -1,2 +1 @@ export { createCallClusterStub } from './call_cluster_stub'; -export { assertPromise, assertRejection } from './promises'; diff --git a/src/ui/ui_settings/__tests__/lib/promises.js b/src/ui/ui_settings/__tests__/lib/promises.js deleted file mode 100644 index dca524429a4ee..0000000000000 --- a/src/ui/ui_settings/__tests__/lib/promises.js +++ /dev/null @@ -1,17 +0,0 @@ -import expect from 'expect.js'; - -export function assertPromise(promise) { - if (!promise || typeof promise.then !== 'function') { - throw new Error('Expected function to return a promise'); - } -} - -export async function assertRejection(promise, errorMessageContain) { - assertPromise(promise); - - try { - await promise; - } catch (err) { - expect(err.message).to.contain(errorMessageContain); - } -} diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js index 3c6ef81d6f287..3c88bec45462a 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_service.js +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -5,11 +5,7 @@ import { errors as esErrors } from 'elasticsearch'; import { getDefaultSettings } from '../defaults'; import { UiSettingsService } from '../ui_settings_service'; -import { - assertPromise, - assertRejection, - createCallClusterStub, -} from './lib'; +import { createCallClusterStub } from './lib'; const INDEX = '.kibana'; const TYPE = 'config'; @@ -41,35 +37,22 @@ describe('ui settings', function () { describe('overview', function () { it('has expected api surface', function () { const { uiSettings } = setup(); - expect(typeof uiSettings.get).to.equal('function'); - expect(typeof uiSettings.getAll).to.equal('function'); - expect(typeof uiSettings.getDefaults).to.equal('function'); - expect(typeof uiSettings.getRaw).to.equal('function'); - expect(typeof uiSettings.getUserProvided).to.equal('function'); - expect(typeof uiSettings.remove).to.equal('function'); - expect(typeof uiSettings.removeMany).to.equal('function'); - expect(typeof uiSettings.set).to.equal('function'); - expect(typeof uiSettings.setMany).to.equal('function'); - }); - - it('throws if the first error is not a request', async () => { - const { uiSettings } = setup(); - await assertRejection(uiSettings.get(null), 'hapi.Request'); - await assertRejection(uiSettings.get(false), 'hapi.Request'); - await assertRejection(uiSettings.get('key'), 'hapi.Request'); - await assertRejection(uiSettings.get(/regex/), 'hapi.Request'); - await assertRejection(uiSettings.get(new Date()), 'hapi.Request'); - await assertRejection(uiSettings.get({}), 'hapi.Request'); - await assertRejection(uiSettings.get({ path:'' }), 'hapi.Request'); - await assertRejection(uiSettings.get({ path:'', headers:null }), 'hapi.Request'); - await assertRejection(uiSettings.get({ headers:{} }), 'hapi.Request'); + expect(uiSettings).to.have.property('get').a('function'); + expect(uiSettings).to.have.property('getAll').a('function'); + expect(uiSettings).to.have.property('getDefaults').a('function'); + expect(uiSettings).to.have.property('getRaw').a('function'); + expect(uiSettings).to.have.property('getUserProvided').a('function'); + expect(uiSettings).to.have.property('remove').a('function'); + expect(uiSettings).to.have.property('removeMany').a('function'); + expect(uiSettings).to.have.property('set').a('function'); + expect(uiSettings).to.have.property('setMany').a('function'); }); }); describe('#setMany()', function () { it('returns a promise', () => { const { uiSettings } = setup(); - assertPromise(uiSettings.setMany({ a: 'b' })); + expect(uiSettings.setMany({ a: 'b' })).to.be.a(Promise); }); it('updates a single value in one operation', async function () { @@ -88,7 +71,7 @@ describe('ui settings', function () { describe('#set()', function () { it('returns a promise', () => { const { uiSettings } = setup(); - assertPromise(uiSettings.set('a', 'b')); + expect(uiSettings.set('a', 'b')).to.be.a(Promise); }); it('updates single values by (key, value)', async function () { @@ -101,7 +84,7 @@ describe('ui settings', function () { describe('#remove()', function () { it('returns a promise', () => { const { uiSettings } = setup(); - assertPromise(uiSettings.remove('one')); + expect(uiSettings.remove('one')).to.be.a(Promise); }); it('removes single values by key', async function () { @@ -114,7 +97,7 @@ describe('ui settings', function () { describe('#removeMany()', function () { it('returns a promise', () => { const { uiSettings } = setup(); - assertPromise(uiSettings.removeMany(['one'])); + expect(uiSettings.removeMany(['one'])).to.be.a(Promise); }); it('removes a single value', async function () { @@ -367,7 +350,7 @@ describe('ui settings', function () { describe('#getUserProvided()', () => { it('returns a promise when interceptValue doesn\'t', () => { const { uiSettings } = setup({ readInterceptor: () => ({}) }); - assertPromise(uiSettings.getUserProvided()); + expect(uiSettings.getUserProvided()).to.be.a(Promise); }); it('returns intercept values', async () => { From c3e330af3512f245e75793c4886b9dbbb9059e99 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:14:59 -0700 Subject: [PATCH 14/18] [uiSettings/forRequest] fix typo --- src/ui/ui_settings/ui_settings_service_for_request.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ui/ui_settings/ui_settings_service_for_request.js b/src/ui/ui_settings/ui_settings_service_for_request.js index 4c468cf2a1a2b..650bc98a53781 100644 --- a/src/ui/ui_settings/ui_settings_service_for_request.js +++ b/src/ui/ui_settings/ui_settings_service_for_request.js @@ -8,13 +8,13 @@ export function getUiSettingsServiceForRequest(server, request, readInterceptor) } const adminCluster = server.plugins.elasticsearch.getCluster('admin'); - const uiSettingsServices = uiSettingsServiceFactory(server, { + const uiSettingsService = uiSettingsServiceFactory(server, { readInterceptor, callCluster(...args) { return adminCluster.callWithRequest(request, ...args); } }); - BY_REQUEST_CACHE.set(request, uiSettingsServices); - return uiSettingsServices; + BY_REQUEST_CACHE.set(request, uiSettingsService); + return uiSettingsService; } From 452393ab3ff83efd4ab083082ad873365a61cc41 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:17:57 -0700 Subject: [PATCH 15/18] [uiSettings/tests] remove mixture of arrow and function expressions --- .../__tests__/ui_settings_service.js | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js index 3c88bec45462a..c7706414b0a0c 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_service.js +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -33,9 +33,9 @@ function setup(options = {}) { }; } -describe('ui settings', function () { - describe('overview', function () { - it('has expected api surface', function () { +describe('ui settings', () => { + describe('overview', () => { + it('has expected api surface', () => { const { uiSettings } = setup(); expect(uiSettings).to.have.property('get').a('function'); expect(uiSettings).to.have.property('getAll').a('function'); @@ -49,72 +49,72 @@ describe('ui settings', function () { }); }); - describe('#setMany()', function () { + describe('#setMany()', () => { it('returns a promise', () => { const { uiSettings } = setup(); expect(uiSettings.setMany({ a: 'b' })).to.be.a(Promise); }); - it('updates a single value in one operation', async function () { + it('updates a single value in one operation', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.setMany({ one: 'value' }); assertUpdateQuery({ one: 'value' }); }); - it('updates several values in one operation', async function () { + it('updates several values in one operation', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.setMany({ one: 'value', another: 'val' }); assertUpdateQuery({ one: 'value', another: 'val' }); }); }); - describe('#set()', function () { + describe('#set()', () => { it('returns a promise', () => { const { uiSettings } = setup(); expect(uiSettings.set('a', 'b')).to.be.a(Promise); }); - it('updates single values by (key, value)', async function () { + it('updates single values by (key, value)', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.set('one', 'value'); assertUpdateQuery({ one: 'value' }); }); }); - describe('#remove()', function () { + describe('#remove()', () => { it('returns a promise', () => { const { uiSettings } = setup(); expect(uiSettings.remove('one')).to.be.a(Promise); }); - it('removes single values by key', async function () { + it('removes single values by key', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.remove('one'); assertUpdateQuery({ one: null }); }); }); - describe('#removeMany()', function () { + describe('#removeMany()', () => { it('returns a promise', () => { const { uiSettings } = setup(); expect(uiSettings.removeMany(['one'])).to.be.a(Promise); }); - it('removes a single value', async function () { + it('removes a single value', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.removeMany(['one']); assertUpdateQuery({ one: null }); }); - it('updates several values in one operation', async function () { + it('updates several values in one operation', async () => { const { uiSettings, assertUpdateQuery } = setup(); await uiSettings.removeMany(['one', 'two', 'three']); assertUpdateQuery({ one: null, two: null, three: null }); }); }); - describe('#getDefaults()', function () { - it('is promised the default values', async function () { + describe('#getDefaults()', () => { + it('is promised the default values', async () => { const { uiSettings } = setup(); @@ -123,7 +123,7 @@ describe('ui settings', function () { }); - describe('defaults for formatters', async function () { + describe('defaults for formatters', async () => { const defaults = getDefaultSettings(); const mapping = JSON.parse(defaults['format:defaultTypeMap'].value); @@ -136,8 +136,8 @@ describe('ui settings', function () { _default_: { id: 'string', params: {} } }; - Object.keys(mapping).forEach(function (dataType) { - it(`should configure ${dataType}`, function () { + Object.keys(mapping).forEach((dataType) => { + it(`should configure ${dataType}`, () => { expect(expected.hasOwnProperty(dataType)).to.equal(true); expect(mapping[dataType].id).to.equal(expected[dataType].id); expect(JSON.stringify(mapping[dataType].params)).to.equal(JSON.stringify(expected[dataType].params)); @@ -146,14 +146,14 @@ describe('ui settings', function () { }); }); - describe('#getUserProvided()', function () { - it('pulls user configuration from ES', async function () { + describe('#getUserProvided()', () => { + it('pulls user configuration from ES', async () => { const { uiSettings, assertGetQuery } = setup(); await uiSettings.getUserProvided(); assertGetQuery(); }); - it('returns user configuration', async function () { + it('returns user configuration', async () => { const esDocSource = { user: 'customized' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); @@ -162,7 +162,7 @@ describe('ui settings', function () { })).to.equal(true); }); - it('ignores null user configuration (because default values)', async function () { + it('ignores null user configuration (because default values)', async () => { const esDocSource = { user: 'customized', usingDefault: null, something: 'else' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); @@ -171,7 +171,7 @@ describe('ui settings', function () { })).to.equal(true); }); - it('returns an empty object on 404 responses', async function () { + it('returns an empty object on 404 responses', async () => { const { uiSettings } = setup({ async callCluster() { throw new esErrors[404](); @@ -181,7 +181,7 @@ describe('ui settings', function () { expect(await uiSettings.getUserProvided()).to.eql({}); }); - it('returns an empty object on 403 responses', async function () { + it('returns an empty object on 403 responses', async () => { const { uiSettings } = setup({ async callCluster() { throw new esErrors[403](); @@ -191,7 +191,7 @@ describe('ui settings', function () { expect(await uiSettings.getUserProvided()).to.eql({}); }); - it('returns an empty object on NoConnections responses', async function () { + it('returns an empty object on NoConnections responses', async () => { const { uiSettings } = setup({ async callCluster() { throw new esErrors.NoConnections(); @@ -201,7 +201,7 @@ describe('ui settings', function () { expect(await uiSettings.getUserProvided()).to.eql({}); }); - it('throws 401 errors', async function () { + it('throws 401 errors', async () => { const { uiSettings } = setup({ async callCluster() { throw new esErrors[401](); @@ -216,7 +216,7 @@ describe('ui settings', function () { } }); - it('throw when callCluster fails in some unexpected way', async function () { + it('throw when callCluster fails in some unexpected way', async () => { const expectedUnexpectedError = new Error('unexpected'); const { uiSettings } = setup({ @@ -234,22 +234,22 @@ describe('ui settings', function () { }); }); - describe('#getRaw()', function () { - it('pulls user configuration from ES', async function () { + describe('#getRaw()', () => { + it('pulls user configuration from ES', async () => { const esDocSource = {}; const { uiSettings, assertGetQuery } = setup({ esDocSource }); await uiSettings.getRaw(); assertGetQuery(); }); - it(`without user configuration it's equal to the defaults`, async function () { + it(`without user configuration it's equal to the defaults`, async () => { const esDocSource = {}; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getRaw(); expect(isEqual(result, getDefaultSettings())).to.equal(true); }); - it(`user configuration gets merged with defaults`, async function () { + it(`user configuration gets merged with defaults`, async () => { const esDocSource = { foo: 'bar' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getRaw(); @@ -258,7 +258,7 @@ describe('ui settings', function () { expect(isEqual(result, merged)).to.equal(true); }); - it(`user configuration gets merged into defaults`, async function () { + it(`user configuration gets merged into defaults`, async () => { const esDocSource = { dateFormat: 'YYYY-MM-DD' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getRaw(); @@ -268,46 +268,46 @@ describe('ui settings', function () { }); }); - describe('#getAll()', function () { - it('pulls user configuration from ES', async function () { + describe('#getAll()', () => { + it('pulls user configuration from ES', async () => { const esDocSource = {}; const { uiSettings, assertGetQuery } = setup({ esDocSource }); await uiSettings.getAll(); assertGetQuery(); }); - it(`returns key value pairs`, async function () { + it(`returns key value pairs`, async () => { const esDocSource = {}; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getAll(); const defaults = getDefaultSettings(); const expectation = {}; - Object.keys(defaults).forEach(key => { + Object.keys(defaults).forEach((key) => { expectation[key] = defaults[key].value; }); expect(isEqual(result, expectation)).to.equal(true); }); - it(`returns key value pairs including user configuration`, async function () { + it(`returns key value pairs including user configuration`, async () => { const esDocSource = { something: 'user-provided' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getAll(); const defaults = getDefaultSettings(); const expectation = {}; - Object.keys(defaults).forEach(key => { + Object.keys(defaults).forEach((key) => { expectation[key] = defaults[key].value; }); expectation.something = 'user-provided'; expect(isEqual(result, expectation)).to.equal(true); }); - it(`returns key value pairs including user configuration for existing settings`, async function () { + it(`returns key value pairs including user configuration for existing settings`, async () => { const esDocSource = { dateFormat: 'YYYY-MM-DD' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getAll(); const defaults = getDefaultSettings(); const expectation = {}; - Object.keys(defaults).forEach(key => { + Object.keys(defaults).forEach((key) => { expectation[key] = defaults[key].value; }); expectation.dateFormat = 'YYYY-MM-DD'; @@ -315,15 +315,15 @@ describe('ui settings', function () { }); }); - describe('#get()', function () { - it('pulls user configuration from ES', async function () { + describe('#get()', () => { + it('pulls user configuration from ES', async () => { const esDocSource = {}; const { uiSettings, assertGetQuery } = setup({ esDocSource }); await uiSettings.get(); assertGetQuery(); }); - it(`returns the promised value for a key`, async function () { + it(`returns the promised value for a key`, async () => { const esDocSource = {}; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.get('dateFormat'); @@ -331,14 +331,14 @@ describe('ui settings', function () { expect(result).to.equal(defaults.dateFormat.value); }); - it(`returns the user-configured value for a custom key`, async function () { + it(`returns the user-configured value for a custom key`, async () => { const esDocSource = { custom: 'value' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.get('custom'); expect(result).to.equal('value'); }); - it(`returns the user-configured value for a modified key`, async function () { + it(`returns the user-configured value for a modified key`, async () => { const esDocSource = { dateFormat: 'YYYY-MM-DD' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.get('dateFormat'); From 993ed24798fe06c9db58bf09d595fe857a097a5a Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:21:31 -0700 Subject: [PATCH 16/18] [uiSettings/tests/callClusterStub] leverage sinon.calledWithExactly --- .../__tests__/lib/call_cluster_stub.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js index 94952fc643715..568c18388912d 100644 --- a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js +++ b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js @@ -29,15 +29,12 @@ export function createCallClusterStub(index, type, id, esDocSource) { callCluster.assertUpdateQuery = doc => { sinon.assert.calledOnce(callCluster); - expect(callCluster.firstCall.args).to.eql([ - 'update', - { - index, - type, - id, - body: { doc }, - } - ]); + sinon.assert.calledWithExactly(callCluster, 'update', { + index, + type, + id, + body: { doc } + }); }; return callCluster; From 381fd498cdbe3614f5fc42fe8d64b3927dcf27b5 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:22:03 -0700 Subject: [PATCH 17/18] [uiSettings/mixin/tests] add exception for eslint import/no-duplicates --- src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js index fba39fb31afe6..303d6c9a84c8b 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js +++ b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js @@ -5,10 +5,13 @@ import Chance from 'chance'; import ServerStatus from '../../../server/status/server_status'; import Config from '../../../server/config/config'; +/* eslint-disable import/no-duplicates */ import * as uiSettingsServiceFactoryNS from '../ui_settings_service_factory'; import { uiSettingsServiceFactory } from '../ui_settings_service_factory'; import * as getUiSettingsServiceForRequestNS from '../ui_settings_service_for_request'; import { getUiSettingsServiceForRequest } from '../ui_settings_service_for_request'; +/* eslint-enable import/no-duplicates */ + import { uiSettingsMixin } from '../ui_settings_mixin'; const chance = new Chance(); From c58dbfa618a964be9cd940dc51d4189d1865bdd7 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 12 Jun 2017 13:23:08 -0700 Subject: [PATCH 18/18] [uiSettings/mixin/tests] wrap single args in parens --- src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js index 303d6c9a84c8b..11159061ef74f 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js +++ b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js @@ -107,7 +107,7 @@ describe('uiSettingsMixin()', () => { const states = chance.shuffle(['red', 'green', 'yellow']); - states.forEach(state => { + states.forEach((state) => { esStatus[state](); expect(esStatus).to.have.property('state', state); expect(status).to.have.property('state', state);