From d896540a9daba324d5ac13ca0d82586f6084178d Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 1 Oct 2019 15:03:44 +0200 Subject: [PATCH 1/7] Expose Saved Objects client in request context --- ...gin-server.callapioptions.wrap401errors.md | 2 +- .../server/elasticsearch/cluster_client.ts | 2 +- .../integration_tests/core_services.test.ts | 6 -- src/core/server/legacy/legacy_service.test.ts | 11 +++- .../kibana/__mocks__/kibana_migrator.ts | 27 +++++++++ .../migrations/kibana/kibana_migrator.mock.ts | 18 +++++- .../saved_objects_service.mock.ts | 4 +- .../saved_objects_service.test.ts | 33 ++++++---- .../saved_objects/saved_objects_service.ts | 60 ++++++++++++++++--- .../lib/scoped_client_provider.mock.ts | 30 ++++++++++ .../service/lib/scoped_client_provider.ts | 2 +- src/core/server/server.api.md | 2 +- src/core/server/server.ts | 18 ++++-- .../saved_objects/saved_objects_mixin.js | 13 +--- .../saved_objects/saved_objects_mixin.test.js | 26 +++----- 15 files changed, 186 insertions(+), 68 deletions(-) create mode 100644 src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts create mode 100644 src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts diff --git a/docs/development/core/server/kibana-plugin-server.callapioptions.wrap401errors.md b/docs/development/core/server/kibana-plugin-server.callapioptions.wrap401errors.md index 4436a07d0b28b..296d769533950 100644 --- a/docs/development/core/server/kibana-plugin-server.callapioptions.wrap401errors.md +++ b/docs/development/core/server/kibana-plugin-server.callapioptions.wrap401errors.md @@ -9,5 +9,5 @@ Indicates whether `401 Unauthorized` errors returned from the Elasticsearch API Signature: ```typescript -wrap401Errors: boolean; +wrap401Errors?: boolean; ``` diff --git a/src/core/server/elasticsearch/cluster_client.ts b/src/core/server/elasticsearch/cluster_client.ts index aa40010bf3b1a..661823cc577ba 100644 --- a/src/core/server/elasticsearch/cluster_client.ts +++ b/src/core/server/elasticsearch/cluster_client.ts @@ -49,7 +49,7 @@ export interface CallAPIOptions { * header that could have been returned by the API itself. If API didn't specify that * then `Basic realm="Authorization Required"` is used as `WWW-Authenticate`. */ - wrap401Errors: boolean; + wrap401Errors?: boolean; /** * A signal object that allows you to abort the request via an AbortController object. */ diff --git a/src/core/server/http/integration_tests/core_services.test.ts b/src/core/server/http/integration_tests/core_services.test.ts index 8a096d4d44371..a914cb2d0068e 100644 --- a/src/core/server/http/integration_tests/core_services.test.ts +++ b/src/core/server/http/integration_tests/core_services.test.ts @@ -266,9 +266,6 @@ describe('http service', () => { await kbnTestServer.request.get(root, '/new-platform/').expect(200); - // called twice by elasticsearch service in http route handler context provider - expect(clusterClientMock).toBeCalledTimes(2); - // admin client contains authHeaders for BWC with legacy platform. const [adminClient, dataClient] = clusterClientMock.mock.calls; const [, , adminClientHeaders] = adminClient; @@ -292,9 +289,6 @@ describe('http service', () => { .set('Authorization', authorizationHeader) .expect(200); - // called twice by elasticsearch service in http route handler context provider - expect(clusterClientMock).toBeCalledTimes(2); - const [adminClient, dataClient] = clusterClientMock.mock.calls; const [, , adminClientHeaders] = adminClient; expect(adminClientHeaders).toEqual({ authorization: authorizationHeader }); diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index cf72bb72079e9..aa5303d905051 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -44,8 +44,12 @@ import { HttpServiceStart, BasePathProxyServer } from '../http'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service'; -import { SavedObjectsServiceStart } from 'src/core/server/saved_objects/saved_objects_service'; +import { + SavedObjectsServiceStart, + SavedObjectsServiceSetup, +} from 'src/core/server/saved_objects/saved_objects_service'; import { KibanaMigrator } from '../saved_objects/migrations'; +import { ScopedSavedObjectsClientProvider } from '../saved_objects'; const MockKbnServer: jest.Mock = KbnServer as any; @@ -58,6 +62,7 @@ let setupDeps: { elasticsearch: ElasticsearchServiceSetup; http: any; plugins: PluginsServiceSetup; + savedObjects: SavedObjectsServiceSetup; }; plugins: Record; }; @@ -97,6 +102,9 @@ beforeEach(() => { internal: new Map([['plugin-id', {} as DiscoveredPluginInternal]]), }, }, + savedObjects: { + clientProvider: {} as ScopedSavedObjectsClientProvider, + }, }, plugins: { 'plugin-id': 'plugin-value' }, }; @@ -108,6 +116,7 @@ beforeEach(() => { }, savedObjects: { migrator: {} as KibanaMigrator, + clientProvider: {} as ScopedSavedObjectsClientProvider, }, plugins: { contracts: new Map() }, }, diff --git a/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts b/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts new file mode 100644 index 0000000000000..c0727bb0363b1 --- /dev/null +++ b/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts @@ -0,0 +1,27 @@ +/* + * 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 { mockKibanaMigrator } from '../kibana_migrator.mock'; + +export const mockKibanaMigratorInstance = mockKibanaMigrator.create(); + +const mockConstructor = jest.fn().mockImplementation(() => mockKibanaMigratorInstance); + +// eslint-disable-next-line import/no-default-export +export const KibanaMigrator = mockConstructor; diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts index ca732f4f15028..c1021018538d5 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.mock.ts @@ -17,16 +17,30 @@ * under the License. */ -import { KibanaMigrator, mergeProperties } from './kibana_migrator'; +import { KibanaMigrator } from './kibana_migrator'; import { buildActiveMappings } from '../core'; import { SavedObjectsMapping } from '../../mappings'; +const { mergeProperties } = jest.requireActual('./kibana_migrator'); + +const defaultSavedObjectMappings = [ + { + pluginId: 'testplugin', + properties: { + testtype: { + properties: { + name: { type: 'keyword' }, + }, + }, + }, + }, +]; const createMigrator = ( { savedObjectMappings, }: { savedObjectMappings: SavedObjectsMapping[]; - } = { savedObjectMappings: [] } + } = { savedObjectMappings: defaultSavedObjectMappings } ) => { const mockMigrator: jest.Mocked> = { runMigrations: jest.fn(), diff --git a/src/core/server/saved_objects/saved_objects_service.mock.ts b/src/core/server/saved_objects/saved_objects_service.mock.ts index 5561031d820ec..27c27c24af179 100644 --- a/src/core/server/saved_objects/saved_objects_service.mock.ts +++ b/src/core/server/saved_objects/saved_objects_service.mock.ts @@ -19,11 +19,13 @@ import { SavedObjectsService, SavedObjectsServiceStart } from './saved_objects_service'; import { mockKibanaMigrator } from './migrations/kibana/kibana_migrator.mock'; +import { ScopedSavedObjectsClientProviderMock } from './service/lib/scoped_client_provider.mock'; type SavedObjectsServiceContract = PublicMethodsOf; const createStartContractMock = () => { const startContract: jest.Mocked = { + clientProvider: ScopedSavedObjectsClientProviderMock.create(), migrator: mockKibanaMigrator.create(), }; @@ -37,7 +39,7 @@ const createsavedObjectsServiceMock = () => { stop: jest.fn(), }; - mocked.setup.mockResolvedValue({}); + mocked.setup.mockResolvedValue({ clientProvider: ScopedSavedObjectsClientProviderMock.create() }); mocked.start.mockResolvedValue(createStartContractMock()); mocked.stop.mockResolvedValue(); return mocked; diff --git a/src/core/server/saved_objects/saved_objects_service.test.ts b/src/core/server/saved_objects/saved_objects_service.test.ts index c13be579c04bb..13c5fcc35ce18 100644 --- a/src/core/server/saved_objects/saved_objects_service.test.ts +++ b/src/core/server/saved_objects/saved_objects_service.test.ts @@ -21,11 +21,12 @@ jest.mock('./migrations/kibana/kibana_migrator'); import { SavedObjectsService, SavedObjectsSetupDeps } from './saved_objects_service'; import { mockCoreContext } from '../core_context.mock'; -import { KibanaMigrator } from './migrations/kibana/kibana_migrator'; +import { KibanaMigrator, mockKibanaMigratorInstance } from './migrations/kibana/kibana_migrator'; import { of } from 'rxjs'; import elasticsearch from 'elasticsearch'; import { Env } from '../config'; import { configServiceMock } from '../mocks'; +import { ScopedSavedObjectsClientProvider } from '.'; afterEach(() => { jest.clearAllMocks(); @@ -49,7 +50,7 @@ describe('SavedObjectsService', () => { const soService = new SavedObjectsService(coreContext); const coreSetup = ({ elasticsearch: { adminClient$: of(clusterClient) }, - legacy: { uiExports: {}, pluginExtendedConfig: {} }, + legacy: { uiExports: { savedObjectMappings: [] }, pluginExtendedConfig: {} }, } as unknown) as SavedObjectsSetupDeps; await soService.setup(coreSetup); @@ -58,6 +59,18 @@ describe('SavedObjectsService', () => { 'success' ); }); + + it('resolves with clientProvider', async () => { + const coreContext = mockCoreContext.create(); + const soService = new SavedObjectsService(coreContext); + const coreSetup = ({ + elasticsearch: { adminClient$: of({ callAsInternalUser: jest.fn() }) }, + legacy: { uiExports: {}, pluginExtendedConfig: {} }, + } as unknown) as SavedObjectsSetupDeps; + + const savedObjectsSetup = await soService.setup(coreSetup); + expect(savedObjectsSetup.clientProvider).toBeInstanceOf(ScopedSavedObjectsClientProvider); + }); }); describe('#start()', () => { @@ -72,9 +85,8 @@ describe('SavedObjectsService', () => { } as unknown) as SavedObjectsSetupDeps; await soService.setup(coreSetup); - const migrator = (KibanaMigrator as jest.Mock).mock.instances[0]; await soService.start({}); - expect(migrator.runMigrations).toHaveBeenCalledWith(true); + expect(mockKibanaMigratorInstance.runMigrations).toHaveBeenCalledWith(true); }); it('skips KibanaMigrator migrations when migrations.skip=true', async () => { @@ -87,9 +99,8 @@ describe('SavedObjectsService', () => { } as unknown) as SavedObjectsSetupDeps; await soService.setup(coreSetup); - const migrator = (KibanaMigrator as jest.Mock).mock.instances[0]; await soService.start({}); - expect(migrator.runMigrations).toHaveBeenCalledWith(true); + expect(mockKibanaMigratorInstance.runMigrations).toHaveBeenCalledWith(true); }); it('resolves with KibanaMigrator after waiting for migrations to complete', async () => { @@ -102,12 +113,12 @@ describe('SavedObjectsService', () => { } as unknown) as SavedObjectsSetupDeps; await soService.setup(coreSetup); - const migrator = (KibanaMigrator as jest.Mock).mock.instances[0]; - expect(migrator.runMigrations).toHaveBeenCalledTimes(0); + expect(mockKibanaMigratorInstance.runMigrations).toHaveBeenCalledTimes(0); + const startContract = await soService.start({}); - expect(startContract.migrator).toBeInstanceOf(KibanaMigrator); - expect(migrator.runMigrations).toHaveBeenCalledWith(false); - expect(migrator.runMigrations).toHaveBeenCalledTimes(1); + expect(startContract.migrator).toBe(mockKibanaMigratorInstance); + expect(mockKibanaMigratorInstance.runMigrations).toHaveBeenCalledWith(false); + expect(mockKibanaMigratorInstance.runMigrations).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index ebcea8dc3b275..08cbbeca0c5ca 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -19,6 +19,14 @@ import { CoreService } from 'src/core/types'; import { first } from 'rxjs/operators'; +import { + SavedObjectsClient, + SavedObjectsSchema, + SavedObjectsRepository, + SavedObjectsSerializer, + ScopedSavedObjectsClientProvider, +} from './'; +import { getRootPropertiesObjects } from './mappings'; import { KibanaMigrator, IKibanaMigrator } from './migrations'; import { CoreContext } from '../core_context'; import { LegacyServiceSetup } from '../legacy/legacy_service'; @@ -26,19 +34,22 @@ import { ElasticsearchServiceSetup } from '../elasticsearch'; import { KibanaConfigType } from '../kibana_config'; import { retryCallCluster } from '../elasticsearch/retry_call_cluster'; import { SavedObjectsConfigType } from './saved_objects_config'; +import { KibanaRequest } from '../http'; import { Logger } from '..'; /** * @public */ -// eslint-disable-next-line @typescript-eslint/no-empty-interface -export interface SavedObjectsServiceSetup {} +export interface SavedObjectsServiceSetup { + clientProvider: Pick; +} /** * @public */ export interface SavedObjectsServiceStart { migrator: IKibanaMigrator; + clientProvider: Pick; } /** @internal */ @@ -54,13 +65,16 @@ export interface SavedObjectsStartDeps {} export class SavedObjectsService implements CoreService { private migrator: KibanaMigrator | undefined; - logger: Logger; + private logger: Logger; + private clientProvider: + | ScopedSavedObjectsClientProvider> + | undefined; constructor(private readonly coreContext: CoreContext) { this.logger = coreContext.logger.get('savedobjects-service'); } - public async setup(coreSetup: SavedObjectsSetupDeps) { + public async setup(coreSetup: SavedObjectsSetupDeps): Promise { this.logger.debug('Setting up SavedObjects service'); const { @@ -82,7 +96,7 @@ export class SavedObjectsService .pipe(first()) .toPromise(); - this.migrator = new KibanaMigrator({ + const migrator = (this.migrator = new KibanaMigrator({ savedObjectSchemas, savedObjectMappings, savedObjectMigrations, @@ -93,12 +107,41 @@ export class SavedObjectsService savedObjectsConfig, kibanaConfig, callCluster: retryCallCluster(adminClient.callAsInternalUser), + })); + + const mappings = this.migrator.getActiveMappings(); + const allTypes = Object.keys(getRootPropertiesObjects(mappings)); + const schema = new SavedObjectsSchema(savedObjectSchemas); + const serializer = new SavedObjectsSerializer(schema); + const visibleTypes = allTypes.filter(type => !schema.isHiddenType(type)); + + this.clientProvider = new ScopedSavedObjectsClientProvider({ + defaultClientFactory({ request }) { + const repository = new SavedObjectsRepository({ + index: kibanaConfig.index, + config: coreSetup.legacy.pluginExtendedConfig, + migrator, + mappings, + schema, + serializer, + allowedTypes: visibleTypes, + callCluster: retryCallCluster(adminClient.asScoped(request).callAsCurrentUser), + }); + + return new SavedObjectsClient(repository); + }, }); - return ({} as any) as Promise; + return { + clientProvider: this.clientProvider, + }; } public async start(core: SavedObjectsStartDeps): Promise { + if (!this.clientProvider) { + throw new Error('#setup() needs to be run first'); + } + this.logger.debug('Starting SavedObjects service'); /** @@ -119,7 +162,10 @@ export class SavedObjectsService const skipMigrations = cliArgs.optimize || savedObjectsConfig.skip; await this.migrator!.runMigrations(skipMigrations); - return { migrator: this.migrator! }; + return { + migrator: this.migrator!, + clientProvider: this.clientProvider, + }; } public async stop() {} diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts new file mode 100644 index 0000000000000..64399d532a25d --- /dev/null +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts @@ -0,0 +1,30 @@ +/* + * 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'; + +const create = (): jest.Mocked> => ({ + addClientWrapperFactory: jest.fn(), + getClient: jest.fn(), + setClientFactory: jest.fn(), +}); + +export const ScopedSavedObjectsClientProviderMock = { + create, +}; diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.ts b/src/core/server/saved_objects/service/lib/scoped_client_provider.ts index 0e93f9c443f14..93893fe162dec 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.ts +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.ts @@ -55,7 +55,7 @@ export interface SavedObjectsClientProviderOptions { } /** - * Provider for the Scoped Saved Object Client. + * Provider for the Scoped Saved Objects Client. */ export class ScopedSavedObjectsClientProvider { private readonly _wrapperFactories = new PriorityCollection<{ diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 4ae1c0c267ea9..fc0d6e494a3ff 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -75,7 +75,7 @@ export function bootstrap({ configs, cliArgs, applyConfigOverrides, features, }: // @public export interface CallAPIOptions { signal?: AbortSignal; - wrap401Errors: boolean; + wrap401Errors?: boolean; } // @public diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 2b63d6ac3be1c..c6075533ef5fd 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -21,7 +21,7 @@ import { take } from 'rxjs/operators'; import { Type } from '@kbn/config-schema'; import { ConfigService, Env, Config, ConfigPath } from './config'; -import { ElasticsearchService } from './elasticsearch'; +import { ElasticsearchService, ElasticsearchServiceSetup } from './elasticsearch'; import { HttpService, HttpServiceSetup } from './http'; import { LegacyService } from './legacy'; import { Logger, LoggerFactory } from './logging'; @@ -36,7 +36,7 @@ import { config as kibanaConfig } from './kibana_config'; import { config as savedObjectsConfig } from './saved_objects'; import { mapToObject } from '../utils/'; import { ContextService } from './context'; -import { InternalCoreSetup } from './index'; +import { SavedObjectsServiceSetup } from './saved_objects/saved_objects_service'; const coreId = Symbol('core'); @@ -90,7 +90,6 @@ export class Server { http: httpSetup, }; - this.registerCoreContext(coreSetup); const pluginsSetup = await this.plugins.setup(coreSetup); const legacySetup = await this.legacy.setup({ @@ -98,11 +97,13 @@ export class Server { plugins: mapToObject(pluginsSetup.contracts), }); - await this.savedObjects.setup({ + const savedObjectsSetup = await this.savedObjects.setup({ elasticsearch: elasticsearchServiceSetup, legacy: legacySetup, }); + this.registerCoreContext({ ...coreSetup, savedObjects: savedObjectsSetup }); + return coreSetup; } @@ -143,11 +144,18 @@ export class Server { ); } - private registerCoreContext(coreSetup: InternalCoreSetup) { + private registerCoreContext(coreSetup: { + http: HttpServiceSetup; + elasticsearch: ElasticsearchServiceSetup; + savedObjects: SavedObjectsServiceSetup; + }) { coreSetup.http.registerRouteHandlerContext(coreId, 'core', async (context, req) => { const adminClient = await coreSetup.elasticsearch.adminClient$.pipe(take(1)).toPromise(); const dataClient = await coreSetup.elasticsearch.dataClient$.pipe(take(1)).toPromise(); return { + savedObjects: { + client: coreSetup.savedObjects.clientProvider.getClient(req), + }, elasticsearch: { adminClient: adminClient.asScoped(req), dataClient: dataClient.asScoped(req), diff --git a/src/legacy/server/saved_objects/saved_objects_mixin.js b/src/legacy/server/saved_objects/saved_objects_mixin.js index edaa285006422..1a4ed33dbcebc 100644 --- a/src/legacy/server/saved_objects/saved_objects_mixin.js +++ b/src/legacy/server/saved_objects/saved_objects_mixin.js @@ -25,7 +25,6 @@ import { SavedObjectsSerializer } from '../../../core/server/saved_objects/seria import { SavedObjectsClient, SavedObjectsRepository, - ScopedSavedObjectsClientProvider, getSortedObjectsForExport, importSavedObjects, resolveImportErrors, @@ -127,17 +126,7 @@ export function savedObjectsMixin(kbnServer, server) { }); }; - const provider = new ScopedSavedObjectsClientProvider({ - index: server.config().get('kibana.index'), - mappings, - defaultClientFactory({ request }) { - const { callWithRequest } = server.plugins.elasticsearch.getCluster('admin'); - const callCluster = (...args) => callWithRequest(request, ...args); - const repository = createRepository(callCluster); - - return new SavedObjectsClient(repository); - }, - }); + const provider = kbnServer.newPlatform.start.core.savedObjects.clientProvider; const service = { types: visibleTypes, diff --git a/src/legacy/server/saved_objects/saved_objects_mixin.test.js b/src/legacy/server/saved_objects/saved_objects_mixin.test.js index cdbc642485706..0f13f670255b5 100644 --- a/src/legacy/server/saved_objects/saved_objects_mixin.test.js +++ b/src/legacy/server/saved_objects/saved_objects_mixin.test.js @@ -20,6 +20,8 @@ import { savedObjectsMixin } from './saved_objects_mixin'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { mockKibanaMigrator } from '../../../core/server/saved_objects/migrations/kibana/kibana_migrator.mock'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { ScopedSavedObjectsClientProviderMock } from '../../../core/server/saved_objects/service/lib/scoped_client_provider.mock'; const savedObjectMappings = [ { @@ -75,6 +77,7 @@ describe('Saved Objects Mixin', () => { }); beforeEach(() => { + const clientProvider = ScopedSavedObjectsClientProviderMock.create(); mockServer = { log: jest.fn(), route: jest.fn(), @@ -98,7 +101,8 @@ describe('Saved Objects Mixin', () => { }; mockKbnServer = { newPlatform: { - start: { core: { savedObjects: { migrator } } }, + start: { core: { savedObjects: { migrator, clientProvider } } }, + setup: { core: { savedObjects: { clientProvider } } }, }, server: mockServer, ready: () => {}, @@ -273,9 +277,8 @@ describe('Saved Objects Mixin', () => { }); describe('get client', () => { - it('should return a valid client object', () => { - const client = service.getScopedSavedObjectsClient(); - expect(client).toBeDefined(); + it('should have a method to get the client', () => { + expect(service).toHaveProperty('getScopedSavedObjectsClient'); }); it('should have a method to set the client factory', () => { @@ -297,21 +300,6 @@ describe('Saved Objects Mixin', () => { service.addScopedSavedObjectsClientWrapperFactory({}); }).not.toThrowError(); }); - - it('should call underlining callCluster', async () => { - mockCallCluster.mockImplementation(method => { - if (method === 'indices.get') { - return { status: 404 }; - } else if (method === 'indices.getAlias') { - return { status: 404 }; - } else if (method === 'cat.templates') { - return []; - } - }); - const client = await service.getScopedSavedObjectsClient(); - await client.create('testtype'); - expect(mockCallCluster).toHaveBeenCalled(); - }); }); describe('#getSavedObjectsClient', () => { From 08e8a405deff55dff474d87dbcc530d0c50e5b6b Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 2 Oct 2019 21:40:52 +0200 Subject: [PATCH 2/7] API Integration test for savedobjects in req context --- src/core/server/index.ts | 4 +++ .../saved_objects_service.test.ts | 1 + src/core/server/server.ts | 31 +++++++++++-------- src/plugins/testbed/server/index.ts | 19 +++++++++--- test/api_integration/apis/core/index.js | 14 ++++++--- test/api_integration/apis/index.js | 1 + 6 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index d3fe64ddc1e0d..1fe210f347da5 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -56,6 +56,7 @@ import { import { PluginsServiceSetup, PluginsServiceStart, PluginOpaqueId } from './plugins'; import { ContextSetup } from './context'; import { SavedObjectsServiceStart } from './saved_objects'; +import { SavedObjectsClientContract } from './saved_objects/types'; export { bootstrap } from './bootstrap'; export { ConfigPath, ConfigService } from './config'; @@ -179,6 +180,9 @@ export { LegacyServiceSetupDeps, LegacyServiceStartDeps } from './legacy'; */ export interface RequestHandlerContext { core: { + savedObjects: { + client: SavedObjectsClientContract; + }; elasticsearch: { dataClient: ScopedClusterClient; adminClient: ScopedClusterClient; diff --git a/src/core/server/saved_objects/saved_objects_service.test.ts b/src/core/server/saved_objects/saved_objects_service.test.ts index 13c5fcc35ce18..105d4905c27e1 100644 --- a/src/core/server/saved_objects/saved_objects_service.test.ts +++ b/src/core/server/saved_objects/saved_objects_service.test.ts @@ -21,6 +21,7 @@ jest.mock('./migrations/kibana/kibana_migrator'); import { SavedObjectsService, SavedObjectsSetupDeps } from './saved_objects_service'; import { mockCoreContext } from '../core_context.mock'; +// @ts-ignore Typescript doesn't know about the jest mock import { KibanaMigrator, mockKibanaMigratorInstance } from './migrations/kibana/kibana_migrator'; import { of } from 'rxjs'; import elasticsearch from 'elasticsearch'; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index c6075533ef5fd..5b828bdf2fda8 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -37,6 +37,7 @@ import { config as savedObjectsConfig } from './saved_objects'; import { mapToObject } from '../utils/'; import { ContextService } from './context'; import { SavedObjectsServiceSetup } from './saved_objects/saved_objects_service'; +import { RequestHandlerContext } from '.'; const coreId = Symbol('core'); @@ -149,19 +150,23 @@ export class Server { elasticsearch: ElasticsearchServiceSetup; savedObjects: SavedObjectsServiceSetup; }) { - coreSetup.http.registerRouteHandlerContext(coreId, 'core', async (context, req) => { - const adminClient = await coreSetup.elasticsearch.adminClient$.pipe(take(1)).toPromise(); - const dataClient = await coreSetup.elasticsearch.dataClient$.pipe(take(1)).toPromise(); - return { - savedObjects: { - client: coreSetup.savedObjects.clientProvider.getClient(req), - }, - elasticsearch: { - adminClient: adminClient.asScoped(req), - dataClient: dataClient.asScoped(req), - }, - }; - }); + coreSetup.http.registerRouteHandlerContext( + coreId, + 'core', + async (context, req): Promise => { + const adminClient = await coreSetup.elasticsearch.adminClient$.pipe(take(1)).toPromise(); + const dataClient = await coreSetup.elasticsearch.dataClient$.pipe(take(1)).toPromise(); + return { + savedObjects: { + client: coreSetup.savedObjects.clientProvider.getClient(req), + }, + elasticsearch: { + adminClient: adminClient.asScoped(req), + dataClient: dataClient.asScoped(req), + }, + }; + } + ); } public async setupConfigSchemas() { diff --git a/src/plugins/testbed/server/index.ts b/src/plugins/testbed/server/index.ts index f9f6b3cf97e91..4dd22d3dce1ef 100644 --- a/src/plugins/testbed/server/index.ts +++ b/src/plugins/testbed/server/index.ts @@ -43,10 +43,21 @@ class Plugin { ); const router = core.http.createRouter(); - router.get({ path: '/ping', validate: false }, async (context, req, res) => { - const response = await context.core.elasticsearch.adminClient.callAsInternalUser('ping'); - return res.ok({ body: `Pong: ${response}` }); - }); + router.get( + { path: '/requestcontext/elasticsearch', validate: false }, + async (context, req, res) => { + const response = await context.core.elasticsearch.adminClient.callAsInternalUser('ping'); + return res.ok({ body: `Elasticsearch: ${response}` }); + } + ); + + router.get( + { path: '/requestcontext/savedobjectsclient', validate: false }, + async (context, req, res) => { + const response = await context.core.savedObjects.client.find({ type: 'TYPE' }); + return res.ok({ body: `SavedObjects client: ${JSON.stringify(response)}` }); + } + ); return { data$: this.initializerContext.config.create().pipe( diff --git a/test/api_integration/apis/core/index.js b/test/api_integration/apis/core/index.js index d12a4a9d37b39..d617b2ad07351 100644 --- a/test/api_integration/apis/core/index.js +++ b/test/api_integration/apis/core/index.js @@ -20,11 +20,17 @@ export default function ({ getService }) { const supertest = getService('supertest'); - describe('core', () => { - it('provides access to request context', async () => ( + describe('core request context', () => { + it('provides access to elasticsearch', async () => ( await supertest - .get('/testbed/ping') - .expect(200, 'Pong: true') + .get('/requestcontext/elasticsearch') + .expect(200, 'Elasticsearch: true') + )); + + it('provides access to SavedObjects client', async () => ( + await supertest + .get('/requestcontext/savedobjectsclient') + .expect(200, 'SavedObjects client: {"page":1,"per_page":20,"total":0,"saved_objects":[]}') )); }); } diff --git a/test/api_integration/apis/index.js b/test/api_integration/apis/index.js index 09d1896da5c80..9f2672959390c 100644 --- a/test/api_integration/apis/index.js +++ b/test/api_integration/apis/index.js @@ -19,6 +19,7 @@ export default function ({ loadTestFile }) { describe('apis', () => { + loadTestFile(require.resolve('./core')); loadTestFile(require.resolve('./elasticsearch')); loadTestFile(require.resolve('./general')); loadTestFile(require.resolve('./home')); From 4751b314436e54630f188d7c18a8b332863e7337 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 3 Oct 2019 11:16:44 +0200 Subject: [PATCH 3/7] SavedObjectsClient docs --- .../core/server/kibana-plugin-server.md | 5 ++- ...lugin-server.requesthandlercontext.core.md | 3 ++ ...ana-plugin-server.requesthandlercontext.md | 4 +- ...server.savedobjectsclient._constructor_.md | 20 ++++++++++ ...in-server.savedobjectsclient.bulkcreate.md | 25 ++++++++++++ ...lugin-server.savedobjectsclient.bulkget.md | 29 ++++++++++++++ ...plugin-server.savedobjectsclient.create.md | 26 +++++++++++++ ...plugin-server.savedobjectsclient.delete.md | 26 +++++++++++++ ...plugin-server.savedobjectsclient.errors.md | 11 ++++++ ...a-plugin-server.savedobjectsclient.find.md | 24 ++++++++++++ ...na-plugin-server.savedobjectsclient.get.md | 26 +++++++++++++ ...kibana-plugin-server.savedobjectsclient.md | 38 +++++++++++++++++++ ...plugin-server.savedobjectsclient.update.md | 27 +++++++++++++ ...lugin-server.savedobjectsclientcontract.md | 2 +- src/core/server/index.ts | 9 +++++ .../service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 1 + src/core/server/server.api.md | 7 ++-- 18 files changed, 277 insertions(+), 8 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient._constructor_.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkcreate.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkget.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.create.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.delete.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.errors.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.find.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.get.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.md create mode 100644 docs/development/core/server/kibana-plugin-server.savedobjectsclient.update.md diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index d943228bbea06..96c18ae454370 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -20,6 +20,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [ClusterClient](./kibana-plugin-server.clusterclient.md) | Represents an Elasticsearch cluster API client and allows to call API on behalf of the internal Kibana user and the actual user that is derived from the request headers (via asScoped(...)). | | [ElasticsearchErrorHelpers](./kibana-plugin-server.elasticsearcherrorhelpers.md) | Helpers for working with errors returned from the Elasticsearch service.Since the internal data of errors are subject to change, consumers of the Elasticsearch service should always use these helpers to classify errors instead of checking error internals such as body.error.header[WWW-Authenticate] | | [KibanaRequest](./kibana-plugin-server.kibanarequest.md) | Kibana specific abstraction for an incoming request. | +| [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) | | | [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) | | | [ScopedClusterClient](./kibana-plugin-server.scopedclusterclient.md) | Serves the same purpose as "normal" ClusterClient but exposes additional callAsCurrentUser method that doesn't use credentials of the Kibana internal user (as callAsInternalUser does) to request Elasticsearch API, but rather passes HTTP headers extracted from the current user request to the API | @@ -65,7 +66,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [PluginManifest](./kibana-plugin-server.pluginmanifest.md) | Describes the set of required and optional properties plugin can define in its mandatory JSON manifest file. | | [PluginsServiceSetup](./kibana-plugin-server.pluginsservicesetup.md) | | | [PluginsServiceStart](./kibana-plugin-server.pluginsservicestart.md) | | -| [RequestHandlerContext](./kibana-plugin-server.requesthandlercontext.md) | Plugin specific context passed to a route handler. | +| [RequestHandlerContext](./kibana-plugin-server.requesthandlercontext.md) | Plugin specific context passed to a route handler.Provides the following clients: - [savedObjects.client](./kibana-plugin-server.savedobjectsclient.md) - Saved Objects client which uses the credentials of the incoming request - [elasticsearch.dataClient](./kibana-plugin-server.scopedclusterclient.md) - Elasticsearch data client which uses the credentials of the incoming request - [elasticsearch.adminClient](./kibana-plugin-server.scopedclusterclient.md) - Elasticsearch admin client which uses the credentials of the incoming request | | [RouteConfig](./kibana-plugin-server.routeconfig.md) | Route specific configuration. | | [RouteConfigOptions](./kibana-plugin-server.routeconfigoptions.md) | Additional route options. | | [SavedObject](./kibana-plugin-server.savedobject.md) | | @@ -143,6 +144,6 @@ The plugin integrates with the core system via lifecycle events: `setup` | [ResponseHeaders](./kibana-plugin-server.responseheaders.md) | Http response headers to set. | | [RouteMethod](./kibana-plugin-server.routemethod.md) | The set of common HTTP methods supported by Kibana routing. | | [SavedObjectAttribute](./kibana-plugin-server.savedobjectattribute.md) | | -| [SavedObjectsClientContract](./kibana-plugin-server.savedobjectsclientcontract.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state.\#\# SavedObjectsClient errorsSince the SavedObjectsClient has its hands in everything we are a little paranoid about the way we present errors back to to application code. Ideally, all errors will be either:1. Caused by bad implementation (ie. undefined is not a function) and as such unpredictable 2. An error that has been classified and decorated appropriately by the decorators in [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md)Type 1 errors are inevitable, but since all expected/handle-able errors should be Type 2 the isXYZError() helpers exposed at SavedObjectsErrorHelpers should be used to understand and manage error responses from the SavedObjectsClient.Type 2 errors are decorated versions of the source error, so if the elasticsearch client threw an error it will be decorated based on its type. That means that rather than looking for error.body.error.type or doing substring checks on error.body.error.reason, just use the helpers to understand the meaning of the error:\`\`\`js if (SavedObjectsErrorHelpers.isNotFoundError(error)) { // handle 404 }if (SavedObjectsErrorHelpers.isNotAuthorizedError(error)) { // 401 handling should be automatic, but in case you wanted to know }// always rethrow the error unless you handle it throw error; \`\`\`\#\#\# 404s from missing indexFrom the perspective of application code and APIs the SavedObjectsClient is a black box that persists objects. One of the internal details that users have no control over is that we use an elasticsearch index for persistance and that index might be missing.At the time of writing we are in the process of transitioning away from the operating assumption that the SavedObjects index is always available. Part of this transition is handling errors resulting from an index missing. These used to trigger a 500 error in most cases, and in others cause 404s with different error messages.From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.\#\#\# 503s from missing indexUnlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's action.auto_create_index setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated.See [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) | +| [SavedObjectsClientContract](./kibana-plugin-server.savedobjectsclientcontract.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state.\#\# SavedObjectsClient errorsSince the SavedObjectsClient has its hands in everything we are a little paranoid about the way we present errors back to to application code. Ideally, all errors will be either:1. Caused by bad implementation (ie. undefined is not a function) and as such unpredictable 2. An error that has been classified and decorated appropriately by the decorators in [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md)Type 1 errors are inevitable, but since all expected/handle-able errors should be Type 2 the isXYZError() helpers exposed at SavedObjectsErrorHelpers should be used to understand and manage error responses from the SavedObjectsClient.Type 2 errors are decorated versions of the source error, so if the elasticsearch client threw an error it will be decorated based on its type. That means that rather than looking for error.body.error.type or doing substring checks on error.body.error.reason, just use the helpers to understand the meaning of the error:\`\`\`js if (SavedObjectsErrorHelpers.isNotFoundError(error)) { // handle 404 }if (SavedObjectsErrorHelpers.isNotAuthorizedError(error)) { // 401 handling should be automatic, but in case you wanted to know }// always rethrow the error unless you handle it throw error; \`\`\`\#\#\# 404s from missing indexFrom the perspective of application code and APIs the SavedObjectsClient is a black box that persists objects. One of the internal details that users have no control over is that we use an elasticsearch index for persistance and that index might be missing.At the time of writing we are in the process of transitioning away from the operating assumption that the SavedObjects index is always available. Part of this transition is handling errors resulting from an index missing. These used to trigger a 500 error in most cases, and in others cause 404s with different error messages.From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.\#\#\# 503s from missing indexUnlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's action.auto_create_index setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated.See [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) | | [SavedObjectsClientWrapperFactory](./kibana-plugin-server.savedobjectsclientwrapperfactory.md) | Describes the factory used to create instances of Saved Objects Client Wrappers. | diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md index 3274db0cf508d..eab3dd01711f0 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.core.md @@ -8,6 +8,9 @@ ```typescript core: { + savedObjects: { + client: SavedObjectsClientContract; + }; elasticsearch: { dataClient: ScopedClusterClient; adminClient: ScopedClusterClient; diff --git a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md index d7498f04733dd..dc00c987aedbb 100644 --- a/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md +++ b/docs/development/core/server/kibana-plugin-server.requesthandlercontext.md @@ -6,6 +6,8 @@ Plugin specific context passed to a route handler. +Provides the following clients: - [savedObjects.client](./kibana-plugin-server.savedobjectsclient.md) - Saved Objects client which uses the credentials of the incoming request - [elasticsearch.dataClient](./kibana-plugin-server.scopedclusterclient.md) - Elasticsearch data client which uses the credentials of the incoming request - [elasticsearch.adminClient](./kibana-plugin-server.scopedclusterclient.md) - Elasticsearch admin client which uses the credentials of the incoming request + Signature: ```typescript @@ -16,5 +18,5 @@ export interface RequestHandlerContext | Property | Type | Description | | --- | --- | --- | -| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
elasticsearch: {
dataClient: ScopedClusterClient;
adminClient: ScopedClusterClient;
};
} | | +| [core](./kibana-plugin-server.requesthandlercontext.core.md) | {
savedObjects: {
client: SavedObjectsClientContract;
};
elasticsearch: {
dataClient: ScopedClusterClient;
adminClient: ScopedClusterClient;
};
} | | diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient._constructor_.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient._constructor_.md new file mode 100644 index 0000000000000..0bcca3ec57b54 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient._constructor_.md @@ -0,0 +1,20 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [(constructor)](./kibana-plugin-server.savedobjectsclient._constructor_.md) + +## SavedObjectsClient.(constructor) + +Constructs a new instance of the `SavedObjectsClient` class + +Signature: + +```typescript +constructor(repository: SavedObjectsRepository); +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| repository | SavedObjectsRepository | | + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkcreate.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkcreate.md new file mode 100644 index 0000000000000..1081a91f92762 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkcreate.md @@ -0,0 +1,25 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [bulkCreate](./kibana-plugin-server.savedobjectsclient.bulkcreate.md) + +## SavedObjectsClient.bulkCreate() method + +Persists multiple documents batched together as a single request + +Signature: + +```typescript +bulkCreate(objects: Array>, options?: SavedObjectsCreateOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| objects | Array<SavedObjectsBulkCreateObject<T>> | | +| options | SavedObjectsCreateOptions | | + +Returns: + +`Promise>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkget.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkget.md new file mode 100644 index 0000000000000..6fbeadd4ce67c --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.bulkget.md @@ -0,0 +1,29 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [bulkGet](./kibana-plugin-server.savedobjectsclient.bulkget.md) + +## SavedObjectsClient.bulkGet() method + +Returns an array of objects by id + +Signature: + +```typescript +bulkGet(objects?: SavedObjectsBulkGetObject[], options?: SavedObjectsBaseOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| objects | SavedObjectsBulkGetObject[] | an array of ids, or an array of objects containing id, type and optionally fields | +| options | SavedObjectsBaseOptions | | + +Returns: + +`Promise>` + +## Example + +bulkGet(\[ { id: 'one', type: 'config' }, { id: 'foo', type: 'index-pattern' } \]) + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.create.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.create.md new file mode 100644 index 0000000000000..68b97ccdf2aef --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.create.md @@ -0,0 +1,26 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [create](./kibana-plugin-server.savedobjectsclient.create.md) + +## SavedObjectsClient.create() method + +Persists a SavedObject + +Signature: + +```typescript +create(type: string, attributes: T, options?: SavedObjectsCreateOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| type | string | | +| attributes | T | | +| options | SavedObjectsCreateOptions | | + +Returns: + +`Promise>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.delete.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.delete.md new file mode 100644 index 0000000000000..657f56d591e77 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.delete.md @@ -0,0 +1,26 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [delete](./kibana-plugin-server.savedobjectsclient.delete.md) + +## SavedObjectsClient.delete() method + +Deletes a SavedObject + +Signature: + +```typescript +delete(type: string, id: string, options?: SavedObjectsBaseOptions): Promise<{}>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| type | string | | +| id | string | | +| options | SavedObjectsBaseOptions | | + +Returns: + +`Promise<{}>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.errors.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.errors.md new file mode 100644 index 0000000000000..f08440485c63c --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.errors.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [errors](./kibana-plugin-server.savedobjectsclient.errors.md) + +## SavedObjectsClient.errors property + +Signature: + +```typescript +static errors: typeof SavedObjectsErrorHelpers; +``` diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.find.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.find.md new file mode 100644 index 0000000000000..a590cc4c4b663 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.find.md @@ -0,0 +1,24 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [find](./kibana-plugin-server.savedobjectsclient.find.md) + +## SavedObjectsClient.find() method + +Find all SavedObjects matching the search query + +Signature: + +```typescript +find(options: SavedObjectsFindOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| options | SavedObjectsFindOptions | | + +Returns: + +`Promise>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.get.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.get.md new file mode 100644 index 0000000000000..bde16a134f5b5 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.get.md @@ -0,0 +1,26 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [get](./kibana-plugin-server.savedobjectsclient.get.md) + +## SavedObjectsClient.get() method + +Retrieves a single object + +Signature: + +```typescript +get(type: string, id: string, options?: SavedObjectsBaseOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| type | string | The type of SavedObject to retrieve | +| id | string | The ID of the SavedObject to retrieve | +| options | SavedObjectsBaseOptions | | + +Returns: + +`Promise>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.md new file mode 100644 index 0000000000000..0081c729fe10a --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.md @@ -0,0 +1,38 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) + +## SavedObjectsClient class + + +Signature: + +```typescript +export declare class SavedObjectsClient +``` + +## Constructors + +| Constructor | Modifiers | Description | +| --- | --- | --- | +| [(constructor)(repository)](./kibana-plugin-server.savedobjectsclient._constructor_.md) | | Constructs a new instance of the SavedObjectsClient class | + +## Properties + +| Property | Modifiers | Type | Description | +| --- | --- | --- | --- | +| [errors](./kibana-plugin-server.savedobjectsclient.errors.md) | | typeof SavedObjectsErrorHelpers | | +| [errors](./kibana-plugin-server.savedobjectsclient.errors.md) | static | typeof SavedObjectsErrorHelpers | | + +## Methods + +| Method | Modifiers | Description | +| --- | --- | --- | +| [bulkCreate(objects, options)](./kibana-plugin-server.savedobjectsclient.bulkcreate.md) | | Persists multiple documents batched together as a single request | +| [bulkGet(objects, options)](./kibana-plugin-server.savedobjectsclient.bulkget.md) | | Returns an array of objects by id | +| [create(type, attributes, options)](./kibana-plugin-server.savedobjectsclient.create.md) | | Persists a SavedObject | +| [delete(type, id, options)](./kibana-plugin-server.savedobjectsclient.delete.md) | | Deletes a SavedObject | +| [find(options)](./kibana-plugin-server.savedobjectsclient.find.md) | | Find all SavedObjects matching the search query | +| [get(type, id, options)](./kibana-plugin-server.savedobjectsclient.get.md) | | Retrieves a single object | +| [update(type, id, attributes, options)](./kibana-plugin-server.savedobjectsclient.update.md) | | Updates an SavedObject | + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclient.update.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.update.md new file mode 100644 index 0000000000000..16454c98bb55b --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclient.update.md @@ -0,0 +1,27 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) > [update](./kibana-plugin-server.savedobjectsclient.update.md) + +## SavedObjectsClient.update() method + +Updates an SavedObject + +Signature: + +```typescript +update(type: string, id: string, attributes: Partial, options?: SavedObjectsUpdateOptions): Promise>; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| type | string | | +| id | string | | +| attributes | Partial<T> | | +| options | SavedObjectsUpdateOptions | | + +Returns: + +`Promise>` + diff --git a/docs/development/core/server/kibana-plugin-server.savedobjectsclientcontract.md b/docs/development/core/server/kibana-plugin-server.savedobjectsclientcontract.md index ae948b090146b..bc5b11dd21b78 100644 --- a/docs/development/core/server/kibana-plugin-server.savedobjectsclientcontract.md +++ b/docs/development/core/server/kibana-plugin-server.savedobjectsclientcontract.md @@ -34,7 +34,7 @@ From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The obje Unlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's `action.auto_create_index` setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated. -See [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) +See [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) Signature: diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 1fe210f347da5..989af32504eaa 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -176,6 +176,15 @@ export { LegacyServiceSetupDeps, LegacyServiceStartDeps } from './legacy'; /** * Plugin specific context passed to a route handler. + * + * Provides the following clients: + * - {@link SavedObjectsClient | savedObjects.client} - Saved Objects client + * which uses the credentials of the incoming request + * - {@link ScopedClusterClient | elasticsearch.dataClient} - Elasticsearch + * data client which uses the credentials of the incoming request + * - {@link ScopedClusterClient | elasticsearch.adminClient} - Elasticsearch + * admin client which uses the credentials of the incoming request + * * @public */ export interface RequestHandlerContext { diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 039579c5a2d14..fdda23500b4cc 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -120,7 +120,7 @@ export interface SavedObjectsUpdateResponse saved_objects: Array>; } -// @internal (undocumented) +// @public (undocumented) export class SavedObjectsClient { // Warning: (ae-forgotten-export) The symbol "SavedObjectsRepository" needs to be exported by the entry point index.d.ts constructor(repository: SavedObjectsRepository); @@ -735,8 +738,6 @@ export class SavedObjectsClient { update(type: string, id: string, attributes: Partial, options?: SavedObjectsUpdateOptions): Promise>; } -// Warning: (ae-incompatible-release-tags) The symbol "SavedObjectsClientContract" is marked as @public, but its signature references "SavedObjectsClient" which is marked as @internal -// // @public export type SavedObjectsClientContract = Pick; From b432a53275a1410910e5231192138c6cb8822415 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 9 Oct 2019 12:40:45 +0200 Subject: [PATCH 4/7] SavedObjectsClient#find remove dependency on indexPatterns And use the saved objects mappings instead --- packages/kbn-es-query/src/kuery/ast/ast.d.ts | 2 +- .../server/saved_objects/service/index.ts | 1 - .../service/lib/cache_index_patterns.test.ts | 108 -------------- .../service/lib/cache_index_patterns.ts | 82 ----------- .../service/lib/filter_utils.test.ts | 134 ++++++------------ .../saved_objects/service/lib/filter_utils.ts | 76 +++++----- .../server/saved_objects/service/lib/index.ts | 2 +- .../service/lib/repository.test.js | 7 - .../saved_objects/service/lib/repository.ts | 21 +-- .../service/lib/search_dsl/query_params.ts | 5 +- .../service/lib/search_dsl/search_dsl.ts | 4 - .../saved_objects/saved_objects_mixin.js | 8 -- 12 files changed, 82 insertions(+), 368 deletions(-) delete mode 100644 src/core/server/saved_objects/service/lib/cache_index_patterns.test.ts delete mode 100644 src/core/server/saved_objects/service/lib/cache_index_patterns.ts diff --git a/packages/kbn-es-query/src/kuery/ast/ast.d.ts b/packages/kbn-es-query/src/kuery/ast/ast.d.ts index 448ef0e9cca75..06f4940e8ed3b 100644 --- a/packages/kbn-es-query/src/kuery/ast/ast.d.ts +++ b/packages/kbn-es-query/src/kuery/ast/ast.d.ts @@ -37,6 +37,6 @@ export function fromKueryExpression( parseOptions?: KueryParseOptions ): KueryNode; -export function toElasticsearchQuery(node: KueryNode, indexPattern: any): JsonObject; +export function toElasticsearchQuery(node: KueryNode, indexPattern?: any): JsonObject; export function doesKueryExpressionHaveLuceneSyntaxError(expression: string): boolean; diff --git a/src/core/server/saved_objects/service/index.ts b/src/core/server/saved_objects/service/index.ts index dbf35ff4e134d..685ce51bc7d29 100644 --- a/src/core/server/saved_objects/service/index.ts +++ b/src/core/server/saved_objects/service/index.ts @@ -56,7 +56,6 @@ export { SavedObjectsClientWrapperFactory, SavedObjectsClientWrapperOptions, SavedObjectsErrorHelpers, - SavedObjectsCacheIndexPatterns, } from './lib'; export * from './saved_objects_client'; diff --git a/src/core/server/saved_objects/service/lib/cache_index_patterns.test.ts b/src/core/server/saved_objects/service/lib/cache_index_patterns.test.ts deleted file mode 100644 index e3aeca42d1cf0..0000000000000 --- a/src/core/server/saved_objects/service/lib/cache_index_patterns.test.ts +++ /dev/null @@ -1,108 +0,0 @@ -/* - * 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 { SavedObjectsCacheIndexPatterns } from './cache_index_patterns'; - -const mockGetFieldsForWildcard = jest.fn(); -const mockIndexPatternsService: jest.Mock = jest.fn().mockImplementation(() => ({ - getFieldsForWildcard: mockGetFieldsForWildcard, - getFieldsForTimePattern: jest.fn(), -})); - -describe('SavedObjectsRepository', () => { - let cacheIndexPatterns: SavedObjectsCacheIndexPatterns; - - const fields = [ - { - aggregatable: true, - name: 'config.type', - searchable: true, - type: 'string', - }, - { - aggregatable: true, - name: 'foo.type', - searchable: true, - type: 'string', - }, - { - aggregatable: true, - name: 'bar.type', - searchable: true, - type: 'string', - }, - { - aggregatable: true, - name: 'baz.type', - searchable: true, - type: 'string', - }, - { - aggregatable: true, - name: 'dashboard.otherField', - searchable: true, - type: 'string', - }, - { - aggregatable: true, - name: 'hiddenType.someField', - searchable: true, - type: 'string', - }, - ]; - - beforeEach(() => { - cacheIndexPatterns = new SavedObjectsCacheIndexPatterns(); - jest.clearAllMocks(); - }); - - it('setIndexPatterns should return an error object when indexPatternsService is undefined', async () => { - try { - await cacheIndexPatterns.setIndexPatterns('test-index'); - } catch (error) { - expect(error.message).toMatch('indexPatternsService is not defined'); - } - }); - - it('setIndexPatterns should return an error object if getFieldsForWildcard is not defined', async () => { - mockGetFieldsForWildcard.mockImplementation(() => { - throw new Error('something happen'); - }); - try { - cacheIndexPatterns.setIndexPatternsService(new mockIndexPatternsService()); - await cacheIndexPatterns.setIndexPatterns('test-index'); - } catch (error) { - expect(error.message).toMatch('Index Pattern Error - something happen'); - } - }); - - it('setIndexPatterns should return empty array when getFieldsForWildcard is returning null or undefined', async () => { - mockGetFieldsForWildcard.mockImplementation(() => null); - cacheIndexPatterns.setIndexPatternsService(new mockIndexPatternsService()); - await cacheIndexPatterns.setIndexPatterns('test-index'); - expect(cacheIndexPatterns.getIndexPatterns()).toEqual(undefined); - }); - - it('setIndexPatterns should return index pattern when getFieldsForWildcard is returning fields', async () => { - mockGetFieldsForWildcard.mockImplementation(() => fields); - cacheIndexPatterns.setIndexPatternsService(new mockIndexPatternsService()); - await cacheIndexPatterns.setIndexPatterns('test-index'); - expect(cacheIndexPatterns.getIndexPatterns()).toEqual({ fields, title: 'test-index' }); - }); -}); diff --git a/src/core/server/saved_objects/service/lib/cache_index_patterns.ts b/src/core/server/saved_objects/service/lib/cache_index_patterns.ts deleted file mode 100644 index e96cf996f504c..0000000000000 --- a/src/core/server/saved_objects/service/lib/cache_index_patterns.ts +++ /dev/null @@ -1,82 +0,0 @@ -/* - * 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 { FieldDescriptor } from 'src/legacy/server/index_patterns/service/index_patterns_service'; -import { IndexPatternsService } from 'src/legacy/server/index_patterns'; - -export interface SavedObjectsIndexPatternField { - name: string; - type: string; - aggregatable: boolean; - searchable: boolean; -} - -export interface SavedObjectsIndexPattern { - fields: SavedObjectsIndexPatternField[]; - title: string; -} - -export class SavedObjectsCacheIndexPatterns { - private _indexPatterns: SavedObjectsIndexPattern | undefined = undefined; - private _indexPatternsService: IndexPatternsService | undefined = undefined; - - public setIndexPatternsService(indexPatternsService: IndexPatternsService) { - this._indexPatternsService = indexPatternsService; - } - - public getIndexPatternsService() { - return this._indexPatternsService; - } - - public getIndexPatterns(): SavedObjectsIndexPattern | undefined { - return this._indexPatterns; - } - - public async setIndexPatterns(index: string) { - await this._getIndexPattern(index); - } - - private async _getIndexPattern(index: string) { - try { - if (this._indexPatternsService == null) { - throw new TypeError('indexPatternsService is not defined'); - } - const fieldsDescriptor: FieldDescriptor[] = await this._indexPatternsService.getFieldsForWildcard( - { - pattern: index, - } - ); - - this._indexPatterns = - fieldsDescriptor && Array.isArray(fieldsDescriptor) && fieldsDescriptor.length > 0 - ? { - fields: fieldsDescriptor.map(field => ({ - aggregatable: field.aggregatable, - name: field.name, - searchable: field.searchable, - type: field.type, - })), - title: index, - } - : undefined; - } catch (err) { - throw new Error(`Index Pattern Error - ${err.message}`); - } - } -} diff --git a/src/core/server/saved_objects/service/lib/filter_utils.test.ts b/src/core/server/saved_objects/service/lib/filter_utils.test.ts index 73a0804512ed1..80a6a96aeaf2b 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.test.ts @@ -19,66 +19,51 @@ import { fromKueryExpression } from '@kbn/es-query'; -import { - validateFilterKueryNode, - getSavedObjectTypeIndexPatterns, - validateConvertFilterToKueryNode, -} from './filter_utils'; -import { SavedObjectsIndexPattern } from './cache_index_patterns'; +import { validateFilterKueryNode, validateConvertFilterToKueryNode } from './filter_utils'; -const mockIndexPatterns: SavedObjectsIndexPattern = { - fields: [ - { - name: 'updatedAt', +const mockMappings = { + properties: { + updatedAt: { type: 'date', - aggregatable: true, - searchable: true, }, - { - name: 'foo.title', - type: 'text', - aggregatable: true, - searchable: true, - }, - { - name: 'foo.description', - type: 'text', - aggregatable: true, - searchable: true, - }, - { - name: 'foo.bytes', - type: 'number', - aggregatable: true, - searchable: true, - }, - { - name: 'bar.foo', - type: 'text', - aggregatable: true, - searchable: true, + foo: { + properties: { + title: { + type: 'text', + }, + description: { + type: 'text', + }, + bytes: { + type: 'number', + }, + }, }, - { - name: 'bar.description', - type: 'text', - aggregatable: true, - searchable: true, + bar: { + properties: { + foo: { + type: 'text', + }, + description: { + type: 'text', + }, + }, }, - { - name: 'hiddentype.description', - type: 'text', - aggregatable: true, - searchable: true, + hiddenType: { + properties: { + description: { + type: 'text', + }, + }, }, - ], - title: 'mock', + }, }; describe('Filter Utils', () => { describe('#validateConvertFilterToKueryNode', () => { test('Validate a simple filter', () => { expect( - validateConvertFilterToKueryNode(['foo'], 'foo.attributes.title: "best"', mockIndexPatterns) + validateConvertFilterToKueryNode(['foo'], 'foo.attributes.title: "best"', mockMappings) ).toEqual(fromKueryExpression('foo.title: "best"')); }); test('Assemble filter kuery node saved object attributes with one saved object type', () => { @@ -86,7 +71,7 @@ describe('Filter Utils', () => { validateConvertFilterToKueryNode( ['foo'], 'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)', - mockIndexPatterns + mockMappings ) ).toEqual( fromKueryExpression( @@ -100,7 +85,7 @@ describe('Filter Utils', () => { validateConvertFilterToKueryNode( ['foo', 'bar'], 'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)', - mockIndexPatterns + mockMappings ) ).toEqual( fromKueryExpression( @@ -114,7 +99,7 @@ describe('Filter Utils', () => { validateConvertFilterToKueryNode( ['foo', 'bar'], '(bar.updatedAt: 5678654567 OR foo.updatedAt: 5678654567) and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or bar.attributes.description :*)', - mockIndexPatterns + mockMappings ) ).toEqual( fromKueryExpression( @@ -128,7 +113,7 @@ describe('Filter Utils', () => { validateConvertFilterToKueryNode( ['foo', 'bar'], 'updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)', - mockIndexPatterns + mockMappings ); }).toThrowErrorMatchingInlineSnapshot( `"This key 'updatedAt' need to be wrapped by a saved object type like foo,bar: Bad Request"` @@ -137,7 +122,7 @@ describe('Filter Utils', () => { test('Lets make sure that we are throwing an exception if we are using hiddentype with types', () => { expect(() => { - validateConvertFilterToKueryNode([], 'hiddentype.title: "title"', mockIndexPatterns); + validateConvertFilterToKueryNode([], 'hiddentype.title: "title"', mockMappings); }).toThrowErrorMatchingInlineSnapshot(`"This type hiddentype is not allowed: Bad Request"`); }); }); @@ -149,7 +134,7 @@ describe('Filter Utils', () => { 'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)' ), ['foo'], - getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns) + mockMappings ); expect(validationObject).toEqual([ @@ -204,7 +189,7 @@ describe('Filter Utils', () => { 'updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)' ), ['foo'], - getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns) + mockMappings ); expect(validationObject).toEqual([ @@ -259,7 +244,7 @@ describe('Filter Utils', () => { 'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.description :*)' ), ['foo'], - getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns) + mockMappings ); expect(validationObject).toEqual([ @@ -316,7 +301,7 @@ describe('Filter Utils', () => { 'bar.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)' ), ['foo'], - getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns) + mockMappings ); expect(validationObject).toEqual([ @@ -371,7 +356,7 @@ describe('Filter Utils', () => { 'foo.updatedAt33: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.header: "best" and (foo.attributes.description: t* or foo.attributes.description :*)' ), ['foo'], - getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns) + mockMappings ); expect(validationObject).toEqual([ @@ -421,37 +406,4 @@ describe('Filter Utils', () => { ]); }); }); - - describe('#getSavedObjectTypeIndexPatterns', () => { - test('Get index patterns related to your type', () => { - const indexPatternsFilterByType = getSavedObjectTypeIndexPatterns(['foo'], mockIndexPatterns); - - expect(indexPatternsFilterByType).toEqual([ - { - name: 'updatedAt', - type: 'date', - aggregatable: true, - searchable: true, - }, - { - name: 'foo.title', - type: 'text', - aggregatable: true, - searchable: true, - }, - { - name: 'foo.description', - type: 'text', - aggregatable: true, - searchable: true, - }, - { - name: 'foo.bytes', - type: 'number', - aggregatable: true, - searchable: true, - }, - ]); - }); - }); }); diff --git a/src/core/server/saved_objects/service/lib/filter_utils.ts b/src/core/server/saved_objects/service/lib/filter_utils.ts index 2397971e66966..64abf268cacd6 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.ts @@ -19,23 +19,21 @@ import { fromKueryExpression, KueryNode, nodeTypes } from '@kbn/es-query'; import { get, set } from 'lodash'; - -import { SavedObjectsIndexPattern, SavedObjectsIndexPatternField } from './cache_index_patterns'; import { SavedObjectsErrorHelpers } from './errors'; +import { IndexMapping } from '../../mappings'; export const validateConvertFilterToKueryNode = ( - types: string[], + allowedTypes: string[], filter: string, - indexPattern: SavedObjectsIndexPattern | undefined + indexMapping: IndexMapping ): KueryNode => { - if (filter && filter.length > 0 && indexPattern) { + if (filter && filter.length > 0 && indexMapping) { const filterKueryNode = fromKueryExpression(filter); - const typeIndexPatterns = getSavedObjectTypeIndexPatterns(types, indexPattern); const validationFilterKuery = validateFilterKueryNode( filterKueryNode, - types, - typeIndexPatterns, + allowedTypes, + indexMapping, filterKueryNode.type === 'function' && ['is', 'range'].includes(filterKueryNode.function) ); @@ -60,7 +58,7 @@ export const validateConvertFilterToKueryNode = ( path.length === 0 ? filterKueryNode : get(filterKueryNode, path); if (item.isSavedObjectAttr) { existingKueryNode.arguments[0].value = existingKueryNode.arguments[0].value.split('.')[1]; - const itemType = types.filter(t => t === item.type); + const itemType = allowedTypes.filter(t => t === item.type); if (itemType.length === 1) { set( filterKueryNode, @@ -84,18 +82,6 @@ export const validateConvertFilterToKueryNode = ( return null; }; -export const getSavedObjectTypeIndexPatterns = ( - types: string[], - indexPattern: SavedObjectsIndexPattern | undefined -): SavedObjectsIndexPatternField[] => { - return indexPattern != null - ? indexPattern.fields.filter( - ip => - !ip.name.includes('.') || (ip.name.includes('.') && types.includes(ip.name.split('.')[0])) - ) - : []; -}; - interface ValidateFilterKueryNode { astPath: string; error: string; @@ -107,7 +93,7 @@ interface ValidateFilterKueryNode { export const validateFilterKueryNode = ( astFilter: KueryNode, types: string[], - typeIndexPatterns: SavedObjectsIndexPatternField[], + indexMapping: IndexMapping, storeValue: boolean = false, path: string = 'arguments' ): ValidateFilterKueryNode[] => { @@ -119,7 +105,7 @@ export const validateFilterKueryNode = ( ...validateFilterKueryNode( ast, types, - typeIndexPatterns, + indexMapping, ast.type === 'function' && ['is', 'range'].includes(ast.function), `${myPath}.arguments` ), @@ -131,8 +117,8 @@ export const validateFilterKueryNode = ( ...kueryNode, { astPath: splitPath.slice(0, splitPath.length - 1).join('.'), - error: hasFilterKeyError(ast.value, types, typeIndexPatterns), - isSavedObjectAttr: isSavedObjectAttr(ast.value, typeIndexPatterns), + error: hasFilterKeyError(ast.value, types, indexMapping), + isSavedObjectAttr: isSavedObjectAttr(ast.value, indexMapping), key: ast.value, type: getType(ast.value), }, @@ -144,47 +130,55 @@ export const validateFilterKueryNode = ( const getType = (key: string) => (key.includes('.') ? key.split('.')[0] : null); -export const isSavedObjectAttr = ( - key: string, - typeIndexPatterns: SavedObjectsIndexPatternField[] -) => { - const splitKey = key.split('.'); - if (splitKey.length === 1 && typeIndexPatterns.some(tip => tip.name === splitKey[0])) { +/** + * Is this filter key referring to a a top-level SavedObject attribute such as + * `updated_at` or `references`. + * + * @param key + * @param indexMapping + */ +export const isSavedObjectAttr = (key: string, indexMapping: IndexMapping) => { + const keySplit = key.split('.'); + if (keySplit.length === 1 && fieldDefined(indexMapping, keySplit[0])) { return true; - } else if (splitKey.length > 1 && typeIndexPatterns.some(tip => tip.name === splitKey[1])) { + } else if (keySplit.length === 2 && fieldDefined(indexMapping, keySplit[1])) { return true; + } else { + return false; } - return false; }; export const hasFilterKeyError = ( key: string, types: string[], - typeIndexPatterns: SavedObjectsIndexPatternField[] + indexMapping: IndexMapping ): string | null => { if (!key.includes('.')) { return `This key '${key}' need to be wrapped by a saved object type like ${types.join()}`; } else if (key.includes('.')) { const keySplit = key.split('.'); + if (keySplit.length <= 1 || !types.includes(keySplit[0])) { return `This type ${keySplit[0]} is not allowed`; } if ( - (keySplit.length === 2 && typeIndexPatterns.some(tip => tip.name === key)) || - (keySplit.length > 2 && types.includes(keySplit[0]) && keySplit[1] !== 'attributes') + (keySplit.length === 2 && fieldDefined(indexMapping, key)) || + (keySplit.length > 2 && keySplit[1] !== 'attributes') ) { return `This key '${key}' does NOT match the filter proposition SavedObjectType.attributes.key`; } if ( - (keySplit.length === 2 && !typeIndexPatterns.some(tip => tip.name === keySplit[1])) || + (keySplit.length === 2 && !fieldDefined(indexMapping, keySplit[1])) || (keySplit.length > 2 && - !typeIndexPatterns.some( - tip => - tip.name === [...keySplit.slice(0, 1), ...keySplit.slice(2, keySplit.length)].join('.') - )) + !fieldDefined(indexMapping, keySplit[0] + '.' + keySplit.slice(2, keySplit.length))) ) { return `This key '${key}' does NOT exist in ${types.join()} saved object index patterns`; } } return null; }; + +const fieldDefined = (indexMappings: IndexMapping, key: string) => { + const mappingKey = 'properties.' + key.split('.').join('.properties.'); + return get(indexMappings, mappingKey) != null; +}; diff --git a/src/core/server/saved_objects/service/lib/index.ts b/src/core/server/saved_objects/service/lib/index.ts index be78fdde76210..e464c2ddc4fbf 100644 --- a/src/core/server/saved_objects/service/lib/index.ts +++ b/src/core/server/saved_objects/service/lib/index.ts @@ -27,4 +27,4 @@ export { export { SavedObjectsErrorHelpers } from './errors'; -export { SavedObjectsCacheIndexPatterns } from './cache_index_patterns'; +export { IndexFieldCapabilitiesCache } from './cache_index_patterns'; diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index bc646c8c1d2e1..863f6dd580774 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1159,13 +1159,6 @@ describe('SavedObjectsRepository', () => { } }); - it('requires index pattern to be defined if filter is defined', async () => { - callAdminCluster.mockReturnValue(noNamespaceSearchResults); - expect(savedObjectsRepository.find({ type: 'foo', filter: 'foo.type: hello' })) - .rejects - .toThrowErrorMatchingInlineSnapshot('"options.filter is missing index pattern to work correctly: Bad Request"'); - }); - it('passes mappings, schema, search, defaultSearchOperator, searchFields, type, sortField, sortOrder and hasReference to getSearchDsl', async () => { callAdminCluster.mockReturnValue(namespacedSearchResults); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index aadb82486ccce..b91a91e3ddb1f 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -25,7 +25,6 @@ import { getSearchDsl } from './search_dsl'; import { includedFields } from './included_fields'; import { decorateEsError } from './decorate_es_error'; import { SavedObjectsErrorHelpers } from './errors'; -import { SavedObjectsCacheIndexPatterns } from './cache_index_patterns'; import { decodeRequestVersion, encodeVersion, encodeHitVersion } from '../../version'; import { SavedObjectsSchema } from '../../schema'; import { KibanaMigrator } from '../../migrations'; @@ -77,7 +76,6 @@ export interface SavedObjectsRepositoryOptions { serializer: SavedObjectsSerializer; migrator: KibanaMigrator; allowedTypes: string[]; - cacheIndexPatterns: SavedObjectsCacheIndexPatterns; onBeforeWrite?: (...args: Parameters) => Promise; } @@ -95,13 +93,11 @@ export class SavedObjectsRepository { private _onBeforeWrite: (...args: Parameters) => Promise; private _unwrappedCallCluster: CallCluster; private _serializer: SavedObjectsSerializer; - private _cacheIndexPatterns: SavedObjectsCacheIndexPatterns; constructor(options: SavedObjectsRepositoryOptions) { const { index, config, - cacheIndexPatterns, mappings, callCluster, schema, @@ -123,7 +119,6 @@ export class SavedObjectsRepository { this._config = config; this._mappings = mappings; this._schema = schema; - this._cacheIndexPatterns = cacheIndexPatterns; if (allowedTypes.length === 0) { throw new Error('Empty or missing types for saved object repository!'); } @@ -133,9 +128,6 @@ export class SavedObjectsRepository { this._unwrappedCallCluster = async (...args: Parameters) => { await migrator.runMigrations(); - if (this._cacheIndexPatterns.getIndexPatterns() == null) { - await this._cacheIndexPatterns.setIndexPatterns(index); - } return callCluster(...args); }; this._schema = schema; @@ -441,19 +433,9 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createBadRequestError('options.fields must be an array'); } - if (filter && filter !== '' && this._cacheIndexPatterns.getIndexPatterns() == null) { - throw SavedObjectsErrorHelpers.createBadRequestError( - 'options.filter is missing index pattern to work correctly' - ); - } - const kueryNode = filter && filter !== '' - ? validateConvertFilterToKueryNode( - allowedTypes, - filter, - this._cacheIndexPatterns.getIndexPatterns() - ) + ? validateConvertFilterToKueryNode(allowedTypes, filter, this._mappings) : null; const esOptions = { @@ -474,7 +456,6 @@ export class SavedObjectsRepository { sortOrder, namespace, hasReference, - indexPattern: kueryNode != null ? this._cacheIndexPatterns.getIndexPatterns() : undefined, kueryNode, }), }, diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 125b0c40af9e4..bee35b899d83c 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -20,7 +20,6 @@ import { toElasticsearchQuery, KueryNode } from '@kbn/es-query'; import { getRootPropertiesObjects, IndexMapping } from '../../../mappings'; import { SavedObjectsSchema } from '../../../schema'; -import { SavedObjectsIndexPattern } from '../cache_index_patterns'; /** * Gets the types based on the type. Uses mappings to support @@ -93,7 +92,6 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; - indexPattern?: SavedObjectsIndexPattern; } /** @@ -109,12 +107,11 @@ export function getQueryParams({ defaultSearchOperator, hasReference, kueryNode, - indexPattern, }: QueryParams) { const types = getTypes(mappings, type); const bool: any = { filter: [ - ...(kueryNode != null ? [toElasticsearchQuery(kueryNode, indexPattern)] : []), + ...(kueryNode != null ? [toElasticsearchQuery(kueryNode)] : []), { bool: { must: hasReference diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index 68f6060702505..868ca51a76eab 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -24,7 +24,6 @@ import { IndexMapping } from '../../../mappings'; import { SavedObjectsSchema } from '../../../schema'; import { getQueryParams } from './query_params'; import { getSortingParams } from './sorting_params'; -import { SavedObjectsIndexPattern } from '../cache_index_patterns'; interface GetSearchDslOptions { type: string | string[]; @@ -39,7 +38,6 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; - indexPattern?: SavedObjectsIndexPattern; } export function getSearchDsl( @@ -57,7 +55,6 @@ export function getSearchDsl( namespace, hasReference, kueryNode, - indexPattern, } = options; if (!type) { @@ -79,7 +76,6 @@ export function getSearchDsl( defaultSearchOperator, hasReference, kueryNode, - indexPattern, }), ...getSortingParams(mappings, type, sortField, sortOrder), }; diff --git a/src/legacy/server/saved_objects/saved_objects_mixin.js b/src/legacy/server/saved_objects/saved_objects_mixin.js index 04460f8aa2cdc..7ac44d20f2beb 100644 --- a/src/legacy/server/saved_objects/saved_objects_mixin.js +++ b/src/legacy/server/saved_objects/saved_objects_mixin.js @@ -25,7 +25,6 @@ import { SavedObjectsSerializer } from '../../../core/server/saved_objects/seria import { SavedObjectsClient, SavedObjectsRepository, - SavedObjectsCacheIndexPatterns, getSortedObjectsForExport, importSavedObjects, resolveImportErrors, @@ -63,7 +62,6 @@ export function savedObjectsMixin(kbnServer, server) { const schema = new SavedObjectsSchema(kbnServer.uiExports.savedObjectSchemas); const visibleTypes = allTypes.filter(type => !schema.isHiddenType(type)); const importableAndExportableTypes = getImportableAndExportableTypes({ kbnServer, visibleTypes }); - const cacheIndexPatterns = new SavedObjectsCacheIndexPatterns(); server.decorate('server', 'kibanaMigrator', migrator); server.decorate( @@ -115,17 +113,11 @@ export function savedObjectsMixin(kbnServer, server) { const combinedTypes = visibleTypes.concat(extraTypes); const allowedTypes = [...new Set(combinedTypes)]; - if (cacheIndexPatterns.getIndexPatternsService() == null) { - cacheIndexPatterns.setIndexPatternsService( - server.indexPatternsServiceFactory({ callCluster }) - ); - } const config = server.config(); return new SavedObjectsRepository({ index: config.get('kibana.index'), config, - cacheIndexPatterns, migrator, mappings, schema, From 296a0ee2a90340125713006b0f296ca0bd344b93 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 14 Oct 2019 14:34:17 +0200 Subject: [PATCH 5/7] Review comments --- src/core/server/legacy/legacy_service.test.ts | 6 +++--- .../saved_objects/saved_objects_service.mock.ts | 6 +++--- .../saved_objects/saved_objects_service.test.ts | 4 ++-- .../saved_objects/saved_objects_service.ts | 13 ++++++------- src/core/server/saved_objects/service/index.ts | 9 +++++---- .../server/saved_objects/service/lib/index.ts | 3 ++- .../service/lib/scoped_client_provider.mock.ts | 6 +++--- .../service/lib/scoped_client_provider.ts | 17 ++++++++++++++++- src/core/server/server.api.md | 6 +++--- src/core/server/server.ts | 2 ++ 10 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index 2616fb68c2963..24aa9d1f0c3cc 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -49,7 +49,7 @@ import { SavedObjectsServiceSetup, } from 'src/core/server/saved_objects/saved_objects_service'; import { KibanaMigrator } from '../saved_objects/migrations'; -import { ScopedSavedObjectsClientProvider } from '../saved_objects'; +import { ISavedObjectsClientProvider } from '../saved_objects'; import { httpServiceMock } from '../http/http_service.mock'; const MockKbnServer: jest.Mock = KbnServer as any; @@ -105,7 +105,7 @@ beforeEach(() => { }, }, savedObjects: { - clientProvider: {} as ScopedSavedObjectsClientProvider, + clientProvider: {} as ISavedObjectsClientProvider, }, }, plugins: { 'plugin-id': 'plugin-value' }, @@ -118,7 +118,7 @@ beforeEach(() => { }, savedObjects: { migrator: {} as KibanaMigrator, - clientProvider: {} as ScopedSavedObjectsClientProvider, + clientProvider: {} as ISavedObjectsClientProvider, }, plugins: { contracts: new Map() }, }, diff --git a/src/core/server/saved_objects/saved_objects_service.mock.ts b/src/core/server/saved_objects/saved_objects_service.mock.ts index 27c27c24af179..6d7902e7e761d 100644 --- a/src/core/server/saved_objects/saved_objects_service.mock.ts +++ b/src/core/server/saved_objects/saved_objects_service.mock.ts @@ -19,13 +19,13 @@ import { SavedObjectsService, SavedObjectsServiceStart } from './saved_objects_service'; import { mockKibanaMigrator } from './migrations/kibana/kibana_migrator.mock'; -import { ScopedSavedObjectsClientProviderMock } from './service/lib/scoped_client_provider.mock'; +import { scopedSavedObjectsClientProviderMock } from './service/lib/scoped_client_provider.mock'; type SavedObjectsServiceContract = PublicMethodsOf; const createStartContractMock = () => { const startContract: jest.Mocked = { - clientProvider: ScopedSavedObjectsClientProviderMock.create(), + clientProvider: scopedSavedObjectsClientProviderMock.create(), migrator: mockKibanaMigrator.create(), }; @@ -39,7 +39,7 @@ const createsavedObjectsServiceMock = () => { stop: jest.fn(), }; - mocked.setup.mockResolvedValue({ clientProvider: ScopedSavedObjectsClientProviderMock.create() }); + mocked.setup.mockResolvedValue({ clientProvider: scopedSavedObjectsClientProviderMock.create() }); mocked.start.mockResolvedValue(createStartContractMock()); mocked.stop.mockResolvedValue(); return mocked; diff --git a/src/core/server/saved_objects/saved_objects_service.test.ts b/src/core/server/saved_objects/saved_objects_service.test.ts index 2228ce2cab90a..07bb4342c754a 100644 --- a/src/core/server/saved_objects/saved_objects_service.test.ts +++ b/src/core/server/saved_objects/saved_objects_service.test.ts @@ -27,7 +27,7 @@ import { of } from 'rxjs'; import * as legacyElasticsearch from 'elasticsearch'; import { Env } from '../config'; import { configServiceMock } from '../mocks'; -import { ScopedSavedObjectsClientProvider } from '.'; +import { SavedObjectsClientProvider } from '.'; afterEach(() => { jest.clearAllMocks(); @@ -70,7 +70,7 @@ describe('SavedObjectsService', () => { } as unknown) as SavedObjectsSetupDeps; const savedObjectsSetup = await soService.setup(coreSetup); - expect(savedObjectsSetup.clientProvider).toBeInstanceOf(ScopedSavedObjectsClientProvider); + expect(savedObjectsSetup.clientProvider).toBeInstanceOf(SavedObjectsClientProvider); }); }); diff --git a/src/core/server/saved_objects/saved_objects_service.ts b/src/core/server/saved_objects/saved_objects_service.ts index 08cbbeca0c5ca..5ccb02414d043 100644 --- a/src/core/server/saved_objects/saved_objects_service.ts +++ b/src/core/server/saved_objects/saved_objects_service.ts @@ -24,7 +24,8 @@ import { SavedObjectsSchema, SavedObjectsRepository, SavedObjectsSerializer, - ScopedSavedObjectsClientProvider, + SavedObjectsClientProvider, + ISavedObjectsClientProvider, } from './'; import { getRootPropertiesObjects } from './mappings'; import { KibanaMigrator, IKibanaMigrator } from './migrations'; @@ -41,7 +42,7 @@ import { Logger } from '..'; * @public */ export interface SavedObjectsServiceSetup { - clientProvider: Pick; + clientProvider: ISavedObjectsClientProvider; } /** @@ -49,7 +50,7 @@ export interface SavedObjectsServiceSetup { */ export interface SavedObjectsServiceStart { migrator: IKibanaMigrator; - clientProvider: Pick; + clientProvider: ISavedObjectsClientProvider; } /** @internal */ @@ -66,9 +67,7 @@ export class SavedObjectsService implements CoreService { private migrator: KibanaMigrator | undefined; private logger: Logger; - private clientProvider: - | ScopedSavedObjectsClientProvider> - | undefined; + private clientProvider: ISavedObjectsClientProvider | undefined; constructor(private readonly coreContext: CoreContext) { this.logger = coreContext.logger.get('savedobjects-service'); @@ -115,7 +114,7 @@ export class SavedObjectsService const serializer = new SavedObjectsSerializer(schema); const visibleTypes = allTypes.filter(type => !schema.isHiddenType(type)); - this.clientProvider = new ScopedSavedObjectsClientProvider({ + this.clientProvider = new SavedObjectsClientProvider({ defaultClientFactory({ request }) { const repository = new SavedObjectsRepository({ index: kibanaConfig.index, diff --git a/src/core/server/saved_objects/service/index.ts b/src/core/server/saved_objects/service/index.ts index 685ce51bc7d29..15f46711fc94b 100644 --- a/src/core/server/saved_objects/service/index.ts +++ b/src/core/server/saved_objects/service/index.ts @@ -18,7 +18,7 @@ */ import { Readable } from 'stream'; -import { ScopedSavedObjectsClientProvider } from './lib'; +import { SavedObjectsClientProvider } from './lib'; import { SavedObjectsClient } from './saved_objects_client'; import { SavedObjectsExportOptions } from '../export'; import { SavedObjectsImportOptions, SavedObjectsImportResponse } from '../import'; @@ -31,10 +31,10 @@ import { SavedObjectsResolveImportErrorsOptions } from '../import/types'; */ export interface SavedObjectsLegacyService { // ATTENTION: these types are incomplete - addScopedSavedObjectsClientWrapperFactory: ScopedSavedObjectsClientProvider< + addScopedSavedObjectsClientWrapperFactory: SavedObjectsClientProvider< Request >['addClientWrapperFactory']; - getScopedSavedObjectsClient: ScopedSavedObjectsClientProvider['getClient']; + getScopedSavedObjectsClient: SavedObjectsClientProvider['getClient']; SavedObjectsClient: typeof SavedObjectsClient; types: string[]; schema: SavedObjectsSchema; @@ -51,7 +51,8 @@ export interface SavedObjectsLegacyService { export { SavedObjectsRepository, - ScopedSavedObjectsClientProvider, + SavedObjectsClientProvider, + ISavedObjectsClientProvider, SavedObjectsClientProviderOptions, SavedObjectsClientWrapperFactory, SavedObjectsClientWrapperOptions, diff --git a/src/core/server/saved_objects/service/lib/index.ts b/src/core/server/saved_objects/service/lib/index.ts index d987737c2ffa0..4bc159e17ec0f 100644 --- a/src/core/server/saved_objects/service/lib/index.ts +++ b/src/core/server/saved_objects/service/lib/index.ts @@ -21,7 +21,8 @@ export { SavedObjectsRepository, SavedObjectsRepositoryOptions } from './reposit export { SavedObjectsClientWrapperFactory, SavedObjectsClientWrapperOptions, - ScopedSavedObjectsClientProvider, + ISavedObjectsClientProvider, + SavedObjectsClientProvider, SavedObjectsClientProviderOptions, } from './scoped_client_provider'; diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts index 64399d532a25d..c284fd1fd1ac7 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts @@ -17,14 +17,14 @@ * under the License. */ -import { ScopedSavedObjectsClientProvider } from './scoped_client_provider'; +import { SavedObjectsClientProvider } from './scoped_client_provider'; -const create = (): jest.Mocked> => ({ +const create = (): jest.Mocked> => ({ addClientWrapperFactory: jest.fn(), getClient: jest.fn(), setClientFactory: jest.fn(), }); -export const ScopedSavedObjectsClientProviderMock = { +export const scopedSavedObjectsClientProviderMock = { create, }; diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.ts b/src/core/server/saved_objects/service/lib/scoped_client_provider.ts index 93893fe162dec..ad1ceb60cdb86 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.ts +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.ts @@ -54,10 +54,25 @@ export interface SavedObjectsClientProviderOptions { excludedWrappers?: string[]; } +/** + * @public + * See {@link SavedObjectsClientProvider} + */ +export type ISavedObjectsClientProvider = Pick< + SavedObjectsClientProvider, + keyof SavedObjectsClientProvider +>; + /** * Provider for the Scoped Saved Objects Client. + * + * @internalRemarks Because `getClient` is synchronous the Client Provider does + * not support creating factories that react to new ES clients emitted from + * elasticsearch.adminClient$. The Client Provider therefore doesn't support + * configuration changes to the Elasticsearch client. TODO: revisit once we've + * closed https://github.com/elastic/kibana/pull/45796 */ -export class ScopedSavedObjectsClientProvider { +export class SavedObjectsClientProvider { private readonly _wrapperFactories = new PriorityCollection<{ id: string; factory: SavedObjectsClientWrapperFactory; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 75c04bfeb4c39..4dc237af11097 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -1409,14 +1409,14 @@ export interface SavedObjectsImportUnsupportedTypeError { // @internal @deprecated (undocumented) export interface SavedObjectsLegacyService { - // Warning: (ae-forgotten-export) The symbol "ScopedSavedObjectsClientProvider" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "SavedObjectsClientProvider" needs to be exported by the entry point index.d.ts // // (undocumented) - addScopedSavedObjectsClientWrapperFactory: ScopedSavedObjectsClientProvider['addClientWrapperFactory']; + addScopedSavedObjectsClientWrapperFactory: SavedObjectsClientProvider['addClientWrapperFactory']; // (undocumented) getSavedObjectsRepository(...rest: any[]): any; // (undocumented) - getScopedSavedObjectsClient: ScopedSavedObjectsClientProvider['getClient']; + getScopedSavedObjectsClient: SavedObjectsClientProvider['getClient']; // (undocumented) importExport: { objectLimit: number; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index f4fca18cfe7ff..b26454d822d25 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -162,6 +162,8 @@ export class Server { const dataClient = await coreSetup.elasticsearch.dataClient$.pipe(take(1)).toPromise(); return { savedObjects: { + // Note: the client provider doesn't support new ES clients + // emitted from adminClient$ client: coreSetup.savedObjects.clientProvider.getClient(req), }, elasticsearch: { From 1bd910793b18af45050b44385abd9122823628fe Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 14 Oct 2019 17:57:57 +0200 Subject: [PATCH 6/7] Review comments, fixes and tests --- src/core/server/legacy/legacy_service.ts | 2 +- .../kibana/__mocks__/kibana_migrator.ts | 1 - .../saved_objects_service.mock.ts | 6 +- .../service/lib/repository.test.js | 137 +++++++++++------- .../saved_objects/service/lib/repository.ts | 17 ++- .../lib/scoped_client_provider.mock.ts | 6 +- .../lib/scoped_client_provider.test.js | 16 +- .../saved_objects/saved_objects_mixin.js | 2 +- .../saved_objects/saved_objects_mixin.test.js | 4 +- .../apis/saved_objects/find.js | 16 ++ 10 files changed, 133 insertions(+), 74 deletions(-) diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 7e1bc32bee85e..e4456fba190ff 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -271,7 +271,7 @@ export class LegacyService implements CoreService { kibanaMigrator: startDeps.core.savedObjects.migrator, uiPlugins: setupDeps.core.plugins.uiPlugins, elasticsearch: setupDeps.core.elasticsearch, - scopedSavedObjectsClientProvider: startDeps.core.savedObjects.clientProvider, + savedObjectsClientProvider: startDeps.core.savedObjects.clientProvider, }, logger: this.coreContext.logger, }, diff --git a/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts b/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts index c0727bb0363b1..d9f745810412c 100644 --- a/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts +++ b/src/core/server/saved_objects/migrations/kibana/__mocks__/kibana_migrator.ts @@ -23,5 +23,4 @@ export const mockKibanaMigratorInstance = mockKibanaMigrator.create(); const mockConstructor = jest.fn().mockImplementation(() => mockKibanaMigratorInstance); -// eslint-disable-next-line import/no-default-export export const KibanaMigrator = mockConstructor; diff --git a/src/core/server/saved_objects/saved_objects_service.mock.ts b/src/core/server/saved_objects/saved_objects_service.mock.ts index 6d7902e7e761d..0a021ee97e26a 100644 --- a/src/core/server/saved_objects/saved_objects_service.mock.ts +++ b/src/core/server/saved_objects/saved_objects_service.mock.ts @@ -19,13 +19,13 @@ import { SavedObjectsService, SavedObjectsServiceStart } from './saved_objects_service'; import { mockKibanaMigrator } from './migrations/kibana/kibana_migrator.mock'; -import { scopedSavedObjectsClientProviderMock } from './service/lib/scoped_client_provider.mock'; +import { savedObjectsClientProviderMock } from './service/lib/scoped_client_provider.mock'; type SavedObjectsServiceContract = PublicMethodsOf; const createStartContractMock = () => { const startContract: jest.Mocked = { - clientProvider: scopedSavedObjectsClientProviderMock.create(), + clientProvider: savedObjectsClientProviderMock.create(), migrator: mockKibanaMigrator.create(), }; @@ -39,7 +39,7 @@ const createsavedObjectsServiceMock = () => { stop: jest.fn(), }; - mocked.setup.mockResolvedValue({ clientProvider: scopedSavedObjectsClientProviderMock.create() }); + mocked.setup.mockResolvedValue({ clientProvider: savedObjectsClientProviderMock.create() }); mocked.start.mockResolvedValue(createStartContractMock()); mocked.stop.mockResolvedValue(); return mocked; diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index c7c84c88a37bc..7e89b8f3c0820 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -273,10 +273,6 @@ describe('SavedObjectsRepository', () => { savedObjectsRepository = new SavedObjectsRepository({ index: '.kibana-test', - cacheIndexPatterns: { - setIndexPatterns: jest.fn(), - getIndexPatterns: () => undefined, - }, mappings, callCluster: callAdminCluster, migrator, @@ -290,8 +286,6 @@ describe('SavedObjectsRepository', () => { getSearchDslNS.getSearchDsl.mockReset(); }); - afterEach(() => { }); - describe('#create', () => { beforeEach(() => { callAdminCluster.mockImplementation((method, params) => ({ @@ -302,9 +296,7 @@ describe('SavedObjectsRepository', () => { }); it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); await expect( savedObjectsRepository.create( @@ -557,9 +549,7 @@ describe('SavedObjectsRepository', () => { describe('#bulkCreate', () => { it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); callAdminCluster.mockReturnValue({ items: [ { create: { type: 'config', id: 'config:one', _primary_term: 1, _seq_no: 1 } }, @@ -998,14 +988,12 @@ describe('SavedObjectsRepository', () => { expect(onBeforeWrite).toHaveBeenCalledTimes(1); }); - it('should return objects in the same order regardless of type', () => { }); + it('should return objects in the same order regardless of type', () => {}); }); describe('#delete', () => { it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); callAdminCluster.mockReturnValue({ result: 'deleted' }); await expect( savedObjectsRepository.delete('index-pattern', 'logstash-*', { @@ -1119,9 +1107,7 @@ describe('SavedObjectsRepository', () => { describe('#find', () => { it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); callAdminCluster.mockReturnValue(noNamespaceSearchResults); await expect(savedObjectsRepository.find({ type: 'foo' })).resolves.toBeDefined(); @@ -1183,6 +1169,75 @@ describe('SavedObjectsRepository', () => { expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, schema, relevantOpts); }); + it('accepts KQL filter and passes keuryNode to getSearchDsl', async () => { + callAdminCluster.mockReturnValue(namespacedSearchResults); + const findOpts = { + namespace: 'foo-namespace', + search: 'foo*', + searchFields: ['foo'], + type: ['dashboard'], + sortField: 'name', + sortOrder: 'desc', + defaultSearchOperator: 'AND', + hasReference: { + type: 'foo', + id: '1', + }, + indexPattern: undefined, + filter: 'dashboard.attributes.otherField: *', + }; + + await savedObjectsRepository.find(findOpts); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + const { kueryNode } = getSearchDslNS.getSearchDsl.mock.calls[0][2]; + expect(kueryNode).toMatchInlineSnapshot(` + Object { + "arguments": Array [ + Object { + "type": "literal", + "value": "dashboard.otherField", + }, + Object { + "type": "wildcard", + "value": "@kuery-wildcard@", + }, + Object { + "type": "literal", + "value": false, + }, + ], + "function": "is", + "type": "function", + } + `); + }); + + it('KQL filter syntax errors rejects with bad request', async () => { + callAdminCluster.mockReturnValue(namespacedSearchResults); + const findOpts = { + namespace: 'foo-namespace', + search: 'foo*', + searchFields: ['foo'], + type: ['dashboard'], + sortField: 'name', + sortOrder: 'desc', + defaultSearchOperator: 'AND', + hasReference: { + type: 'foo', + id: '1', + }, + indexPattern: undefined, + filter: 'dashboard.attributes.otherField:<', + }; + + await expect(savedObjectsRepository.find(findOpts)).rejects.toMatchInlineSnapshot(` + [Error: KQLSyntaxError: Expected "(", value, whitespace but "<" found. + dashboard.attributes.otherField:< + --------------------------------^: Bad Request] + `); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(0); + }); + it('merges output of getSearchDsl into es request body', async () => { callAdminCluster.mockReturnValue(noNamespaceSearchResults); getSearchDslNS.getSearchDsl.mockReturnValue({ query: 1, aggregations: 2 }); @@ -1322,9 +1377,7 @@ describe('SavedObjectsRepository', () => { }; it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); callAdminCluster.mockResolvedValue(noNamespaceResult); await expect( @@ -1415,9 +1468,7 @@ describe('SavedObjectsRepository', () => { describe('#bulkGet', () => { it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); callAdminCluster.mockReturnValue({ docs: [] }); await expect( @@ -1669,9 +1720,7 @@ describe('SavedObjectsRepository', () => { }); it('waits until migrations are complete before proceeding', async () => { - migrator.runMigrations = jest.fn(async () => - expect(callAdminCluster).not.toHaveBeenCalled() - ); + migrator.runMigrations = jest.fn(async () => expect(callAdminCluster).not.toHaveBeenCalled()); await expect( savedObjectsRepository.update('index-pattern', 'logstash-*', attributes, { @@ -1738,11 +1787,7 @@ describe('SavedObjectsRepository', () => { }); it('does not pass references if omitted', async () => { - await savedObjectsRepository.update( - type, - id, - { title: 'Testing' } - ); + await savedObjectsRepository.update(type, id, { title: 'Testing' }); expect(callAdminCluster).toHaveBeenCalledTimes(1); expect(callAdminCluster).not.toHaveBeenCalledWith( @@ -1751,19 +1796,14 @@ describe('SavedObjectsRepository', () => { body: { doc: expect.objectContaining({ references: [], - }) - } + }), + }, }) ); }); it('passes references if they are provided', async () => { - await savedObjectsRepository.update( - type, - id, - { title: 'Testing' }, - { references: ['foo'] } - ); + await savedObjectsRepository.update(type, id, { title: 'Testing' }, { references: ['foo'] }); expect(callAdminCluster).toHaveBeenCalledTimes(1); expect(callAdminCluster).toHaveBeenCalledWith( @@ -1772,19 +1812,14 @@ describe('SavedObjectsRepository', () => { body: { doc: expect.objectContaining({ references: ['foo'], - }) - } + }), + }, }) ); }); it('passes empty references array if empty references array is provided', async () => { - await savedObjectsRepository.update( - type, - id, - { title: 'Testing' }, - { references: [] } - ); + await savedObjectsRepository.update(type, id, { title: 'Testing' }, { references: [] }); expect(callAdminCluster).toHaveBeenCalledTimes(1); expect(callAdminCluster).toHaveBeenCalledWith( @@ -1793,8 +1828,8 @@ describe('SavedObjectsRepository', () => { body: { doc: expect.objectContaining({ references: [], - }) - } + }), + }, }) ); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 4bce2ce5b5b31..b4723f35b1efc 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -433,10 +433,19 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createBadRequestError('options.fields must be an array'); } - const kueryNode = - filter && filter !== '' - ? validateConvertFilterToKueryNode(allowedTypes, filter, this._mappings) - : null; + let kueryNode; + try { + kueryNode = + filter && filter !== '' + ? validateConvertFilterToKueryNode(allowedTypes, filter, this._mappings) + : null; + } catch (e) { + if (e.name === 'KQLSyntaxError') { + throw SavedObjectsErrorHelpers.createBadRequestError('KQLSyntaxError: ' + e.message); + } else { + throw e; + } + } const esOptions = { index: this.getIndicesForTypes(allowedTypes), diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts index c284fd1fd1ac7..5de234f4f93be 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.mock.ts @@ -17,14 +17,14 @@ * under the License. */ -import { SavedObjectsClientProvider } from './scoped_client_provider'; +import { ISavedObjectsClientProvider } from './scoped_client_provider'; -const create = (): jest.Mocked> => ({ +const create = (): jest.Mocked => ({ addClientWrapperFactory: jest.fn(), getClient: jest.fn(), setClientFactory: jest.fn(), }); -export const scopedSavedObjectsClientProviderMock = { +export const savedObjectsClientProviderMock = { create, }; diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js b/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js index bce76bc7841e6..eb210b6843de0 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js @@ -17,14 +17,14 @@ * under the License. */ -import { ScopedSavedObjectsClientProvider } from './scoped_client_provider'; +import { SavedObjectsClientProvider } from './scoped_client_provider'; test(`uses default client factory when one isn't set`, () => { const returnValue = Symbol(); const defaultClientFactoryMock = jest.fn().mockReturnValue(returnValue); const request = Symbol(); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); const result = clientProvider.getClient(request); @@ -42,7 +42,7 @@ test(`uses custom client factory when one is set`, () => { const returnValue = Symbol(); const customClientFactoryMock = jest.fn().mockReturnValue(returnValue); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); clientProvider.setClientFactory(customClientFactoryMock); @@ -57,7 +57,7 @@ test(`uses custom client factory when one is set`, () => { }); test(`throws error when more than one scoped saved objects client factory is set`, () => { - const clientProvider = new ScopedSavedObjectsClientProvider({}); + const clientProvider = new SavedObjectsClientProvider({}); clientProvider.setClientFactory(() => {}); expect(() => { clientProvider.setClientFactory(() => {}); @@ -66,7 +66,7 @@ test(`throws error when more than one scoped saved objects client factory is set test(`throws error when registering a wrapper with a duplicate id`, () => { const defaultClientFactoryMock = jest.fn(); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); const firstClientWrapperFactoryMock = jest.fn(); @@ -81,7 +81,7 @@ test(`throws error when registering a wrapper with a duplicate id`, () => { test(`invokes and uses wrappers in specified order`, () => { const defaultClient = Symbol(); const defaultClientFactoryMock = jest.fn().mockReturnValue(defaultClient); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); const firstWrappedClient = Symbol('first client'); @@ -108,7 +108,7 @@ test(`invokes and uses wrappers in specified order`, () => { test(`does not invoke or use excluded wrappers`, () => { const defaultClient = Symbol(); const defaultClientFactoryMock = jest.fn().mockReturnValue(defaultClient); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); const firstWrappedClient = Symbol('first client'); @@ -135,7 +135,7 @@ test(`does not invoke or use excluded wrappers`, () => { test(`allows all wrappers to be excluded`, () => { const defaultClient = Symbol(); const defaultClientFactoryMock = jest.fn().mockReturnValue(defaultClient); - const clientProvider = new ScopedSavedObjectsClientProvider({ + const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, }); const firstWrappedClient = Symbol('first client'); diff --git a/src/legacy/server/saved_objects/saved_objects_mixin.js b/src/legacy/server/saved_objects/saved_objects_mixin.js index a01e347111e49..0583dea333c7d 100644 --- a/src/legacy/server/saved_objects/saved_objects_mixin.js +++ b/src/legacy/server/saved_objects/saved_objects_mixin.js @@ -126,7 +126,7 @@ export async function savedObjectsMixin(kbnServer, server) { }); }; - const provider = kbnServer.newPlatform.__internals.scopedSavedObjectsClientProvider; + const provider = kbnServer.newPlatform.__internals.savedObjectsClientProvider; const service = { types: visibleTypes, diff --git a/src/legacy/server/saved_objects/saved_objects_mixin.test.js b/src/legacy/server/saved_objects/saved_objects_mixin.test.js index 248eed1db2301..45ac31ab7fc61 100644 --- a/src/legacy/server/saved_objects/saved_objects_mixin.test.js +++ b/src/legacy/server/saved_objects/saved_objects_mixin.test.js @@ -21,7 +21,7 @@ import { savedObjectsMixin } from './saved_objects_mixin'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { mockKibanaMigrator } from '../../../core/server/saved_objects/migrations/kibana/kibana_migrator.mock'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { ScopedSavedObjectsClientProviderMock } from '../../../core/server/saved_objects/service/lib/scoped_client_provider.mock'; +import { savedObjectsClientProviderMock } from '../../../core/server/saved_objects/service/lib/scoped_client_provider.mock'; const savedObjectMappings = [ { @@ -77,7 +77,7 @@ describe('Saved Objects Mixin', () => { }); beforeEach(() => { - const clientProvider = ScopedSavedObjectsClientProviderMock.create(); + const clientProvider = savedObjectsClientProviderMock.create(); mockServer = { log: jest.fn(), route: jest.fn(), diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index a41df24ea7a41..364dbd842b189 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -165,6 +165,22 @@ export default function ({ getService }) { }); }) )); + + it('KQL syntax error should return 400 with Bad Request', async () => ( + await supertest + .get('/api/saved_objects/_find?type=visualization&filter=dashboard.attributes.title:foo { + console.log('body', JSON.stringify(resp.body)); + expect(resp.body).to.eql({ + error: 'Bad Request', + message: 'KQLSyntaxError: Expected AND, OR, end of input, ' + + 'whitespace but \"<\" found.\ndashboard.attributes.title:foo' + + ' Date: Tue, 15 Oct 2019 15:11:57 +0200 Subject: [PATCH 7/7] Use correct type for KQL syntax check --- test/api_integration/apis/saved_objects/find.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index 364dbd842b189..a1c84a766a116 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -168,7 +168,7 @@ export default function ({ getService }) { it('KQL syntax error should return 400 with Bad Request', async () => ( await supertest - .get('/api/saved_objects/_find?type=visualization&filter=dashboard.attributes.title:foo { console.log('body', JSON.stringify(resp.body));