From 44dedd6091be40294c6b206ee8386ce86d49a263 Mon Sep 17 00:00:00 2001 From: Tyler Smalley Date: Tue, 11 Jul 2017 12:33:58 -0700 Subject: [PATCH] Uses SavedObjectsClient for UI Settings (#12747) --- .../saved_objects/saved_objects_mixin.js | 17 +++++--- .../__tests__/lib/call_cluster_stub.js | 41 ------------------- .../lib/create_objects_client_stub.js | 28 +++++++++++++ src/ui/ui_settings/__tests__/lib/index.js | 2 +- .../__tests__/ui_settings_service.js | 20 ++++----- src/ui/ui_settings/ui_settings_service.js | 29 +++---------- .../ui_settings_service_factory.js | 5 +-- .../ui_settings_service_for_request.js | 5 +-- test/functional/apps/status_page/index.js | 4 +- 9 files changed, 59 insertions(+), 92 deletions(-) delete mode 100644 src/ui/ui_settings/__tests__/lib/call_cluster_stub.js create mode 100644 src/ui/ui_settings/__tests__/lib/create_objects_client_stub.js diff --git a/src/server/saved_objects/saved_objects_mixin.js b/src/server/saved_objects/saved_objects_mixin.js index f699cadfe78ae..f960229a6001c 100644 --- a/src/server/saved_objects/saved_objects_mixin.js +++ b/src/server/saved_objects/saved_objects_mixin.js @@ -26,6 +26,14 @@ export function savedObjectsMixin(kbnServer, server) { server.route(createGetRoute(prereqs)); server.route(createUpdateRoute(prereqs)); + server.decorate('server', 'savedObjectsClientFactory', ({ callCluster }) => { + return new SavedObjectsClient( + server.config().get('kibana.index'), + kbnServer.uiExports.mappings.getCombined(), + callCluster + ); + }); + const savedObjectsClientCache = new WeakMap(); server.decorate('request', 'getSavedObjectsClient', function () { const request = this; @@ -35,12 +43,9 @@ export function savedObjectsMixin(kbnServer, server) { } const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin'); - const callAdminCluster = (...args) => callWithRequest(request, ...args); - const savedObjectsClient = new SavedObjectsClient( - server.config().get('kibana.index'), - kbnServer.uiExports.mappings.getCombined(), - callAdminCluster - ); + const callCluster = (...args) => callWithRequest(request, ...args); + const savedObjectsClient = server.savedObjectsClientFactory({ callCluster }); + savedObjectsClientCache.set(request, savedObjectsClient); return savedObjectsClient; }); diff --git a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js b/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js deleted file mode 100644 index 568c18388912d..0000000000000 --- a/src/ui/ui_settings/__tests__/lib/call_cluster_stub.js +++ /dev/null @@ -1,41 +0,0 @@ -import sinon from 'sinon'; -import expect from 'expect.js'; - -export function createCallClusterStub(index, type, id, 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: { ...esDocSource } }; - - case 'update': - expect(params).to.have.property('body'); - expect(params.body).to.have.property('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); - sinon.assert.calledWithExactly(callCluster, 'update', { - index, - type, - id, - body: { doc } - }); - }; - - return callCluster; -} diff --git a/src/ui/ui_settings/__tests__/lib/create_objects_client_stub.js b/src/ui/ui_settings/__tests__/lib/create_objects_client_stub.js new file mode 100644 index 0000000000000..5b6c38b8a5cf5 --- /dev/null +++ b/src/ui/ui_settings/__tests__/lib/create_objects_client_stub.js @@ -0,0 +1,28 @@ +import sinon from 'sinon'; +import expect from 'expect.js'; + +export function createObjectsClientStub(type, id, esDocSource = {}) { + const savedObjectsClient = { + update: sinon.stub().returns(Promise.resolve()), + get: sinon.stub().returns({ attributes: esDocSource }) + }; + + savedObjectsClient.assertGetQuery = () => { + sinon.assert.calledOnce(savedObjectsClient.get); + + const { args } = savedObjectsClient.get.getCall(0); + expect(args[0]).to.be(type); + expect(args[1]).to.eql(id); + }; + + savedObjectsClient.assertUpdateQuery = (expectedChanges) => { + sinon.assert.calledOnce(savedObjectsClient.update); + + const { args } = savedObjectsClient.update.getCall(0); + expect(args[0]).to.be(type); + expect(args[1]).to.eql(id); + expect(args[2]).to.eql(expectedChanges); + }; + + return savedObjectsClient; +} diff --git a/src/ui/ui_settings/__tests__/lib/index.js b/src/ui/ui_settings/__tests__/lib/index.js index f3cd36fad2691..14c9b5af0c610 100644 --- a/src/ui/ui_settings/__tests__/lib/index.js +++ b/src/ui/ui_settings/__tests__/lib/index.js @@ -1 +1 @@ -export { createCallClusterStub } from './call_cluster_stub'; +export { createObjectsClientStub } from './create_objects_client_stub'; diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js index ed121e004bc69..aad976cb98d81 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_service.js +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -5,9 +5,8 @@ import Chance from 'chance'; import { UiSettingsService } from '../ui_settings_service'; -import { createCallClusterStub } from './lib'; +import { createObjectsClientStub } from './lib'; -const INDEX = '.kibana'; const TYPE = 'config'; const ID = 'kibana-version'; const chance = new Chance(); @@ -18,22 +17,21 @@ function setup(options = {}) { getDefaults, defaults = {}, esDocSource = {}, - callCluster = createCallClusterStub(INDEX, TYPE, ID, esDocSource) + savedObjectsClient = createObjectsClientStub(TYPE, ID, esDocSource) } = options; const uiSettings = new UiSettingsService({ - index: INDEX, type: TYPE, id: ID, getDefaults: getDefaults || (() => defaults), readInterceptor, - callCluster, + savedObjectsClient, }); return { uiSettings, - assertGetQuery: callCluster.assertGetQuery, - assertUpdateQuery: callCluster.assertUpdateQuery, + assertGetQuery: savedObjectsClient.assertGetQuery, + assertUpdateQuery: savedObjectsClient.assertUpdateQuery, }; } @@ -199,9 +197,9 @@ describe('ui settings', () => { it('throws 401 errors', async () => { const { uiSettings } = setup({ - async callCluster() { + savedObjectsClient: { async get() { throw new esErrors[401](); - } + } } }); try { @@ -216,9 +214,9 @@ describe('ui settings', () => { const expectedUnexpectedError = new Error('unexpected'); const { uiSettings } = setup({ - async callCluster() { + savedObjectsClient: { async get() { throw expectedUnexpectedError; - } + } } }); try { diff --git a/src/ui/ui_settings/ui_settings_service.js b/src/ui/ui_settings/ui_settings_service.js index 57d0db8e9c96b..d966d4a0e9683 100644 --- a/src/ui/ui_settings/ui_settings_service.js +++ b/src/ui/ui_settings/ui_settings_service.js @@ -25,20 +25,18 @@ function hydrateUserSettings(userSettings) { export class UiSettingsService { constructor(options) { const { - index, type, id, - callCluster, + savedObjectsClient, readInterceptor = noop, // we use a function for getDefaults() so that defaults can be different in // different scenarios, and so they can change over time getDefaults = () => ({}), } = options; - this._callCluster = callCluster; + this._savedObjectsClient = savedObjectsClient; this._getDefaults = getDefaults; this._readInterceptor = readInterceptor; - this._index = index; this._type = type; this._id = id; } @@ -95,14 +93,7 @@ export class UiSettingsService { } async _write(changes) { - await this._callCluster('update', { - index: this._index, - type: this._type, - id: this._id, - body: { - doc: changes - } - }); + await this._savedObjectsClient.update(this._type, this._id, changes); } async _read(options = {}) { @@ -123,18 +114,8 @@ export class UiSettingsService { ); 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; + const resp = await this._savedObjectsClient.get(this._type, this._id); + return resp.attributes; } catch (error) { if (isIgnorableError(error)) { return {}; diff --git a/src/ui/ui_settings/ui_settings_service_factory.js b/src/ui/ui_settings/ui_settings_service_factory.js index c0b70223a3dda..eeb26e3ce9d4f 100644 --- a/src/ui/ui_settings/ui_settings_service_factory.js +++ b/src/ui/ui_settings/ui_settings_service_factory.js @@ -19,16 +19,15 @@ export function uiSettingsServiceFactory(server, options) { const config = server.config(); const { - callCluster, + savedObjectsClient, readInterceptor, getDefaults, } = options; return new UiSettingsService({ - index: config.get('kibana.index'), type: 'config', id: config.get('pkg.version'), - callCluster, + savedObjectsClient, readInterceptor, getDefaults, }); 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 d43657b6d8f42..5ed3db8428e6e 100644 --- a/src/ui/ui_settings/ui_settings_service_for_request.js +++ b/src/ui/ui_settings/ui_settings_service_for_request.js @@ -28,13 +28,10 @@ export function getUiSettingsServiceForRequest(server, request, options = {}) { getDefaults } = options; - const adminCluster = server.plugins.elasticsearch.getCluster('admin'); const uiSettingsService = uiSettingsServiceFactory(server, { readInterceptor, getDefaults, - callCluster(...args) { - return adminCluster.callWithRequest(request, ...args); - } + savedObjectsClient: request.getSavedObjectsClient() }); BY_REQUEST_CACHE.set(request, uiSettingsService); diff --git a/test/functional/apps/status_page/index.js b/test/functional/apps/status_page/index.js index 53c78ebda6c34..be1a5ca48a857 100644 --- a/test/functional/apps/status_page/index.js +++ b/test/functional/apps/status_page/index.js @@ -7,8 +7,8 @@ export default function ({ getService, getPageObjects }) { const PageObjects = getPageObjects(['common']); describe('status page', function () { - before(function () { - return PageObjects.common.navigateToApp('status_page'); + beforeEach(async () => { + await PageObjects.common.navigateToApp('status_page'); }); it('should show the kibana plugin as ready', function () {