SavedObjects service and client provider (#19349) (#19673)

* Renaming client to service

* Implementing the SavedObjects service and client provider

* Making saved objects service lazy instantiated so the test server works

* Fixing import

* Fixing another import

* Fixing reporting's usage of the savedObjectsClientFactory

* No longer passing the callCluster function to the kbnTestServer

* Passing empty request...

* Fixing reporting

* Exposing server.savedObjects without a getter

This required me to remove the kbnTestServer.createServer() method and
change all usages to createServerWithCorePlugins since we're reading the
kibana.index, which is part of the core plugins, when configuring the
saved objects service which is attached to the server.

* Don't need to set kibana.index

* Changing more usages of kbnTestServer.createServer

* Revert "Changing more usages of kbnTestServer.createServer"

This reverts commit 464f73abb2.

* Partially reverting 17d36b3e9a

* Fixing headers

* Improving tests

* Addressing some peer review feedback

* Addressing more peer review feedback

* Adding server.log warning when the saved objects can't initialize

* Getting rid of the savedObjectsClientFactory

* Branches are for trees

* Fixing comment to be proper english

* Fixing fat-fingering method call

* Fixing other usages of getScopedSavedObjectsClient
This commit is contained in:
Brandon Kobel 2018-06-05 07:24:44 -04:00 committed by GitHub
parent a3f9322a39
commit 6595984cf9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
36 changed files with 372 additions and 126 deletions

View file

@ -19,7 +19,7 @@
import sinon from 'sinon';
import { shortUrlLookupProvider } from './short_url_lookup';
import { SavedObjectsClient } from '../saved_objects/client';
import { SavedObjectsClient } from '../saved_objects';
describe('shortUrlLookupProvider', () => {
const ID = 'bf00ad16941fc51420f91a93428b27a0';

View file

@ -18,4 +18,4 @@
*/
export { savedObjectsMixin } from './saved_objects_mixin';
export { SavedObjectsClient } from './client';
export { SavedObjectsClient } from './service';

View file

@ -17,7 +17,7 @@
* under the License.
*/
import { SavedObjectsClient } from './client';
import { createSavedObjectsService } from './service';
import {
createBulkGetRoute,
@ -29,6 +29,13 @@ import {
} from './routes';
export function savedObjectsMixin(kbnServer, server) {
// we use kibana.index which is technically defined in the kibana plugin, so if
// we don't have the plugin (mainly tests) we can't initialize the saved objects
if (!kbnServer.pluginSpecs.some(p => p.getId() === 'kibana')) {
server.log(['warning', 'saved-objects'], `Saved Objects uninitialized because the Kibana plugin is disabled.`);
return;
}
const prereqs = {
getSavedObjectsClient: {
assign: 'savedObjectsClient',
@ -45,50 +52,7 @@ export function savedObjectsMixin(kbnServer, server) {
server.route(createGetRoute(prereqs));
server.route(createUpdateRoute(prereqs));
async function onBeforeWrite() {
const adminCluster = server.plugins.elasticsearch.getCluster('admin');
try {
const index = server.config().get('kibana.index');
await adminCluster.callWithInternalUser('indices.putTemplate', {
name: `kibana_index_template:${index}`,
body: {
template: index,
settings: {
number_of_shards: 1,
auto_expand_replicas: '0-1',
},
mappings: server.getKibanaIndexMappingsDsl(),
},
});
} catch (error) {
server.log(['debug', 'savedObjects'], {
tmpl: 'Attempt to write indexTemplate for SavedObjects index failed: <%= err.message %>',
es: {
resp: error.body,
status: error.status,
},
err: {
message: error.message,
stack: error.stack,
},
});
// We reject with `es.ServiceUnavailable` because writing an index
// template is a very simple operation so if we get an error here
// then something must be very broken
throw new adminCluster.errors.ServiceUnavailable();
}
}
server.decorate('server', 'savedObjectsClientFactory', ({ callCluster }) => {
return new SavedObjectsClient({
index: server.config().get('kibana.index'),
mappings: server.getKibanaIndexMappingsDsl(),
callCluster,
onBeforeWrite,
});
});
server.decorate('server', 'savedObjects', createSavedObjectsService(server));
const savedObjectsClientCache = new WeakMap();
server.decorate('request', 'getSavedObjectsClient', function () {
@ -98,9 +62,7 @@ export function savedObjectsMixin(kbnServer, server) {
return savedObjectsClientCache.get(request);
}
const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin');
const callCluster = (...args) => callWithRequest(request, ...args);
const savedObjectsClient = server.savedObjectsClientFactory({ callCluster });
const savedObjectsClient = server.savedObjects.getScopedSavedObjectsClient(request);
savedObjectsClientCache.set(request, savedObjectsClient);
return savedObjectsClient;

View file

@ -0,0 +1,95 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { SavedObjectsRepository, ScopedSavedObjectsClientProvider } from './lib';
import { SavedObjectsClient } from './saved_objects_client';
export function createSavedObjectsService(server) {
const onBeforeWrite = async () => {
const adminCluster = server.plugins.elasticsearch.getCluster('admin');
try {
const index = server.config().get('kibana.index');
await adminCluster.callWithInternalUser('indices.putTemplate', {
name: `kibana_index_template:${index}`,
body: {
template: index,
settings: {
number_of_shards: 1,
auto_expand_replicas: '0-1',
},
mappings: server.getKibanaIndexMappingsDsl(),
},
});
} catch (error) {
server.log(['debug', 'savedObjects'], {
tmpl:
'Attempt to write indexTemplate for SavedObjects index failed: <%= err.message %>',
es: {
resp: error.body,
status: error.status,
},
err: {
message: error.message,
stack: error.stack,
},
});
// We reject with `es.ServiceUnavailable` because writing an index
// template is a very simple operation so if we get an error here
// then something must be very broken
throw new adminCluster.errors.ServiceUnavailable();
}
};
const scopedClientProvider = new ScopedSavedObjectsClientProvider({
index: server.config().get('kibana.index'),
mappings: server.getKibanaIndexMappingsDsl(),
onBeforeWrite,
defaultClientFactory({
request,
index,
mappings,
onBeforeWrite
}) {
const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin');
const callCluster = (...args) => callWithRequest(request, ...args);
const repository = new SavedObjectsRepository({
index,
mappings,
onBeforeWrite,
callCluster
});
return new SavedObjectsClient(repository);
}
});
return {
SavedObjectsClient,
SavedObjectsRepository,
getScopedSavedObjectsClient: (...args) =>
scopedClientProvider.getClient(...args),
setScopedSavedObjectsClientFactory: (...args) =>
scopedClientProvider.setClientFactory(...args),
addScopedSavedObjectsClientWrapperFactory: (...args) =>
scopedClientProvider.addClientWrapperFactory(...args),
};
}

View file

@ -18,3 +18,4 @@
*/
export { SavedObjectsClient } from './saved_objects_client';
export { createSavedObjectsService } from './create_saved_objects_service';

View file

@ -0,0 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`throws error when more than one scoped saved objects client factory is set 1`] = `"custom client factory is already set, unable to replace the current one"`;

View file

@ -18,6 +18,7 @@
*/
export { SavedObjectsRepository } from './repository';
export { ScopedSavedObjectsClientProvider } from './scoped_client_provider';
import * as errors from './errors';
export { errors };

View file

@ -0,0 +1,74 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/**
* Provider for the Scoped Saved Object Client.
*/
export class ScopedSavedObjectsClientProvider {
_wrapperFactories = [];
constructor({
index,
mappings,
onBeforeWrite,
defaultClientFactory
}) {
this._index = index;
this._mappings = mappings;
this._onBeforeWrite = onBeforeWrite;
this._originalClientFactory = this._clientFactory = defaultClientFactory;
}
// the client wrapper factories are put at the front of the array, so that
// when we use `reduce` below they're invoked in LIFO order. This is so that
// if multiple plugins register their client wrapper factories, then we can use
// the plugin dependencies/optionalDependencies to implicitly control the order
// in which these are used. For example, if we have a plugin a that declares a
// dependency on plugin b, that means that plugin b's client wrapper would want
// to be able to run first when the SavedObjectClient methods are invoked to
// provide additional context to plugin a's client wrapper.
addClientWrapperFactory(wrapperFactory) {
this._wrapperFactories.unshift(wrapperFactory);
}
setClientFactory(customClientFactory) {
if (this._clientFactory !== this._originalClientFactory) {
throw new Error(`custom client factory is already set, unable to replace the current one`);
}
this._clientFactory = customClientFactory;
}
getClient(request) {
const client = this._clientFactory({
request,
index: this._index,
mappings: this._mappings,
onBeforeWrite: this._onBeforeWrite,
});
return this._wrapperFactories.reduce((clientToWrap, wrapperFactory) => {
return wrapperFactory({
request,
client: clientToWrap,
});
}, client);
}
}

View file

@ -0,0 +1,130 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { ScopedSavedObjectsClientProvider } from './scoped_client_provider';
test(`uses default client factory when one isn't set`, () => {
const returnValue = Symbol();
const defaultClientFactoryMock = jest.fn().mockReturnValue(returnValue);
const index = Symbol();
const mappings = Symbol();
const onBeforeWrite = () => {};
const request = Symbol();
const clientProvider = new ScopedSavedObjectsClientProvider({
index,
mappings,
onBeforeWrite,
defaultClientFactory: defaultClientFactoryMock
});
const result = clientProvider.getClient(request);
expect(result).toBe(returnValue);
expect(defaultClientFactoryMock).toHaveBeenCalledTimes(1);
expect(defaultClientFactoryMock).toHaveBeenCalledWith({
request,
index,
mappings,
onBeforeWrite,
});
});
test(`uses custom client factory when one is set`, () => {
const defaultClientFactoryMock = jest.fn();
const index = Symbol();
const mappings = Symbol();
const onBeforeWrite = () => {};
const request = Symbol();
const returnValue = Symbol();
const customClientFactoryMock = jest.fn().mockReturnValue(returnValue);
const clientProvider = new ScopedSavedObjectsClientProvider({
index,
mappings,
onBeforeWrite,
defaultClientFactory: defaultClientFactoryMock
});
clientProvider.setClientFactory(customClientFactoryMock);
const result = clientProvider.getClient(request);
expect(result).toBe(returnValue);
expect(defaultClientFactoryMock).not.toHaveBeenCalled();
expect(customClientFactoryMock).toHaveBeenCalledTimes(1);
expect(customClientFactoryMock).toHaveBeenCalledWith({
request,
index,
mappings,
onBeforeWrite,
});
});
test(`throws error when more than one scoped saved objects client factory is set`, () => {
const clientProvider = new ScopedSavedObjectsClientProvider({});
clientProvider.setClientFactory(() => {});
expect(() => {
clientProvider.setClientFactory(() => {});
}).toThrowErrorMatchingSnapshot();
});
test(`invokes and uses instance from single added wrapper factory`, () => {
const defaultClient = Symbol();
const defaultClientFactoryMock = jest.fn().mockReturnValue(defaultClient);
const clientProvider = new ScopedSavedObjectsClientProvider({
defaultClientFactory: defaultClientFactoryMock
});
const wrappedClient = Symbol();
const clientWrapperFactoryMock = jest.fn().mockReturnValue(wrappedClient);
const request = Symbol();
clientProvider.addClientWrapperFactory(clientWrapperFactoryMock);
const actualClient = clientProvider.getClient(request);
expect(actualClient).toBe(wrappedClient);
expect(clientWrapperFactoryMock).toHaveBeenCalledWith({
request,
client: defaultClient
});
});
test(`invokes and uses wrappers in LIFO order`, () => {
const defaultClient = Symbol();
const defaultClientFactoryMock = jest.fn().mockReturnValue(defaultClient);
const clientProvider = new ScopedSavedObjectsClientProvider({
defaultClientFactory: defaultClientFactoryMock
});
const firstWrappedClient = Symbol();
const firstClientWrapperFactoryMock = jest.fn().mockReturnValue(firstWrappedClient);
const secondWrapperClient = Symbol();
const secondClientWrapperFactoryMock = jest.fn().mockReturnValue(secondWrapperClient);
const request = Symbol();
clientProvider.addClientWrapperFactory(firstClientWrapperFactoryMock);
clientProvider.addClientWrapperFactory(secondClientWrapperFactoryMock);
const actualClient = clientProvider.getClient(request);
expect(actualClient).toBe(firstWrappedClient);
expect(firstClientWrapperFactoryMock).toHaveBeenCalledWith({
request,
client: secondWrapperClient
});
expect(secondClientWrapperFactoryMock).toHaveBeenCalledWith({
request,
client: defaultClient
});
});

View file

@ -18,13 +18,12 @@
*/
import {
SavedObjectsRepository,
errors,
} from './lib';
export class SavedObjectsClient {
constructor(options) {
this._repository = new SavedObjectsRepository(options);
constructor(repository) {
this._repository = repository;
}
/**

View file

@ -18,30 +18,13 @@
*/
import { SavedObjectsClient } from './saved_objects_client';
import { SavedObjectsRepository } from './lib/repository';
jest.mock('./lib/repository');
beforeEach(() => {
SavedObjectsRepository.mockClear();
});
const setupMockRepository = (mock) => {
SavedObjectsRepository.mockImplementation(() => mock);
return mock;
};
test(`#constructor`, () => {
const options = {};
new SavedObjectsClient(options);
expect(SavedObjectsRepository).toHaveBeenCalledWith(options);
});
test(`#create`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
create: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const type = 'foo';
const attributes = {};
@ -54,10 +37,10 @@ test(`#create`, async () => {
test(`#bulkCreate`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
bulkCreate: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const objects = [];
const options = {};
@ -69,10 +52,10 @@ test(`#bulkCreate`, async () => {
test(`#delete`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
delete: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const type = 'foo';
const id = 1;
@ -84,10 +67,10 @@ test(`#delete`, async () => {
test(`#find`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
find: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const options = {};
const result = await client.find(options);
@ -98,10 +81,10 @@ test(`#find`, async () => {
test(`#bulkGet`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
bulkGet: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const objects = {};
const result = await client.bulkGet(objects);
@ -112,10 +95,10 @@ test(`#bulkGet`, async () => {
test(`#get`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
get: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const type = 'foo';
const id = 1;
@ -127,10 +110,10 @@ test(`#get`, async () => {
test(`#update`, async () => {
const returnValue = Symbol();
const mockRepository = setupMockRepository({
const mockRepository = {
update: jest.fn().mockReturnValue(Promise.resolve(returnValue)),
});
const client = new SavedObjectsClient();
};
const client = new SavedObjectsClient(mockRepository);
const type = 'foo';
const id = 1;

View file

@ -49,7 +49,7 @@ const DEFAULT_SETTINGS_WITH_CORE_PLUGINS = {
url: esTestConfig.getUrl(),
username: kibanaServerTestUser.username,
password: kibanaServerTestUser.password
}
},
};
/**

View file

@ -19,7 +19,7 @@
import sinon from 'sinon';
import expect from 'expect.js';
import { SavedObjectsClient } from '../../../../server/saved_objects/client';
import { SavedObjectsClient } from '../../../../server/saved_objects';
export const savedObjectsClientErrors = SavedObjectsClient.errors;

View file

@ -56,9 +56,8 @@ describe('createOrUpgradeSavedConfig()', () => {
await kbnServer.server.plugins.elasticsearch.waitUntilReady();
savedObjectsClient = kbnServer.server.savedObjectsClientFactory({
callCluster: es.getCallCluster(),
});
const savedObjects = kbnServer.server.savedObjects;
savedObjectsClient = savedObjects.getScopedSavedObjectsClient({});
await savedObjectsClient.bulkCreate([
{

View file

@ -51,9 +51,8 @@ export function getServices() {
const callCluster = es.getCallCluster();
const savedObjectsClient = kbnServer.server.savedObjectsClientFactory({
callCluster,
});
const savedObjects = kbnServer.server.savedObjects;
const savedObjectsClient = savedObjects.getScopedSavedObjectsClient();
const uiSettings = kbnServer.server.uiSettingsServiceFactory({
savedObjectsClient,

View file

@ -10,7 +10,6 @@ import sinon from 'sinon';
import nodeCrypto from '@elastic/node-crypto';
import { CancellationToken } from '../../../../server/lib/esqueue/helpers/cancellation_token';
import { SavedObjectsClient } from '../../../../../../../src/server/saved_objects/client/saved_objects_client.js';
import { FieldFormat } from '../../../../../../../src/ui/field_formats/field_format.js';
import { FieldFormatsService } from '../../../../../../../src/ui/field_formats/field_formats_service.js';
import { createStringFormat } from '../../../../../../../src/core_plugins/kibana/common/field_formats/types/string.js';
@ -97,18 +96,12 @@ describe('CSV Execute Job', function () {
get: configGetStub
};
},
savedObjectsClientFactory: (opts) => {
return new SavedObjectsClient({
index: '.kibana',
mappings: { rootType: { properties: {} } },
callCluster: opts.callCluster
});
},
uiSettingsServiceFactory: () => {
return {
get: uiSettingsGetStub
};
savedObjects: {
getScopedSavedObjectsClient: sinon.stub()
},
uiSettingsServiceFactory: sinon.stub().returns({
get: uiSettingsGetStub
}),
log: function () {}
};
mockServer.config().get.withArgs('xpack.reporting.encryptionKey').returns(encryptionKey);
@ -116,12 +109,23 @@ describe('CSV Execute Job', function () {
mockServer.config().get.withArgs('xpack.reporting.csv.scroll').returns({});
});
describe('uiSettings', function () {
it('always calls callWithRequest with decrypted headers', async function () {
describe('savedObjects', function () {
it('calls getScopedSavedObjectsClient with request containing decrypted headers', async function () {
const executeJob = executeJobFactory(mockServer);
await executeJob({ headers: encryptedHeaders, fields: [], searchRequest: { index: null, body: null } }, cancellationToken);
const requestMatch = sinon.match.has('headers', headers).and(sinon.match.has('path', sinon.match.string));
callWithRequestStub.alwaysCalledWith(requestMatch, sinon.match.any, sinon.match.any);
expect(mockServer.savedObjects.getScopedSavedObjectsClient.calledOnce).to.be(true);
expect(mockServer.savedObjects.getScopedSavedObjectsClient.firstCall.args[0].headers).to.be.eql(headers);
});
});
describe('uiSettings', function () {
it('passed scoped SavedObjectsClient to uiSettingsServiceFactory', async function () {
const returnValue = Symbol();
mockServer.savedObjects.getScopedSavedObjectsClient.returns(returnValue);
const executeJob = executeJobFactory(mockServer);
await executeJob({ headers: encryptedHeaders, fields: [], searchRequest: { index: null, body: null } }, cancellationToken);
expect(mockServer.uiSettingsServiceFactory.calledOnce).to.be(true);
expect(mockServer.uiSettingsServiceFactory.firstCall.args[0].savedObjectsClient).to.be(returnValue);
});
});

View file

@ -36,9 +36,8 @@ function executeJobFn(server) {
const callEndpoint = (endpoint, clientParams = {}, options = {}) => {
return callWithRequest(fakeRequest, endpoint, clientParams, options);
};
const savedObjectsClient = server.savedObjectsClientFactory({
callCluster: callEndpoint
});
const savedObjects = server.savedObjects;
const savedObjectsClient = savedObjects.getScopedSavedObjectsClient(fakeRequest);
const uiSettings = server.uiSettingsServiceFactory({
savedObjectsClient
});

View file

@ -21,7 +21,6 @@ const KBN_SCREENSHOT_HEADER_BLACKLIST = [
];
function executeJobFn(server) {
const { callWithRequest } = server.plugins.elasticsearch.getCluster('data');
const generatePdfObservable = generatePdfObservableFactory(server);
const crypto = cryptoFactory(server);
const compatibilityShim = compatibilityShimFactory(server);
@ -41,12 +40,8 @@ function executeJobFn(server) {
headers: filteredHeaders,
};
const callEndpoint = (endpoint, clientParams = {}, options = {}) => {
return callWithRequest(fakeRequest, endpoint, clientParams, options);
};
const savedObjectsClient = server.savedObjectsClientFactory({
callCluster: callEndpoint
});
const savedObjects = server.savedObjects;
const savedObjectsClient = savedObjects.getScopedSavedObjectsClient(fakeRequest);
const uiSettings = server.uiSettingsServiceFactory({
savedObjectsClient
});

View file

@ -35,7 +35,9 @@ beforeEach(() => {
})
}
},
savedObjectsClientFactory: jest.fn(),
savedObjects: {
getScopedSavedObjectsClient: jest.fn(),
},
uiSettingsServiceFactory: jest.fn().mockReturnValue({ get: jest.fn() }),
};