From 8acd6f8706d93600d11801b4d62f052ecfbc6818 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Thu, 25 Nov 2021 11:54:25 +0100 Subject: [PATCH 1/9] Use PointInTimeFinder to paginate through indexPatterns in a more performant way --- .../watcher/public/application/lib/api.ts | 8 ++-- .../threshold_watch_edit.tsx | 5 +- .../register_get_index_patterns_route.ts | 47 +++++++++++++++++++ .../api/indices/register_indices_routes.ts | 2 + 4 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts diff --git a/x-pack/plugins/watcher/public/application/lib/api.ts b/x-pack/plugins/watcher/public/application/lib/api.ts index 61ea124695cb..0971371f7949 100644 --- a/x-pack/plugins/watcher/public/application/lib/api.ts +++ b/x-pack/plugins/watcher/public/application/lib/api.ts @@ -151,12 +151,10 @@ export const executeWatch = async (executeWatchDetails: ExecutedWatchDetails, wa }; export const loadIndexPatterns = async () => { - const { savedObjects } = await getSavedObjectsClient().find({ - type: 'index-pattern', - fields: ['title'], - perPage: 10000, + return sendRequest({ + path: `${basePath}/indices/index_patterns`, + method: 'get', }); - return savedObjects; }; const getWatchVisualizationDataDeserializer = (data: { visualizeData: any }) => data?.visualizeData; diff --git a/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/threshold_watch_edit.tsx b/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/threshold_watch_edit.tsx index 667fe6b6fae9..949c161cec3a 100644 --- a/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/threshold_watch_edit.tsx +++ b/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/threshold_watch_edit.tsx @@ -182,9 +182,8 @@ export const ThresholdWatchEdit = ({ pageTitle }: { pageTitle: string }) => { useEffect(() => { const getIndexPatterns = async () => { - const indexPatternObjects = await loadIndexPatterns(); - const titles = indexPatternObjects.map((indexPattern: any) => indexPattern.attributes.title); - setIndexPatterns(titles); + const { data: indexPatternTitles } = await loadIndexPatterns(); + setIndexPatterns(indexPatternTitles); }; const loadData = async () => { diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts new file mode 100644 index 000000000000..eab3e75936b4 --- /dev/null +++ b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts @@ -0,0 +1,47 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { SavedObject, SavedObjectsFindResult } from 'src/core/server'; +import { RouteDependencies } from '../../../types'; + +export function registerGetIndexPatternsRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { + router.get( + { + path: '/api/watcher/indices/index_patterns', + validate: false, + }, + license.guardApiRoute(async ({ core: { savedObjects } }, request, response) => { + try { + const finder = savedObjects.client.createPointInTimeFinder({ + type: 'index-pattern', + fields: ['title'], + perPage: 1000, + }); + + const responses: SavedObject[] = []; + + for await (const result of finder.find()) { + responses.push( + ...result.saved_objects.map( + (indexPattern: SavedObjectsFindResult) => indexPattern.attributes.title + ) + ); + } + + await finder.close(); + + return response.ok({ body: responses }); + } catch (e) { + return handleEsError({ error: e, response }); + } + }) + ); +} diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_indices_routes.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_indices_routes.ts index 041457aadf8e..6e7003f1cafe 100644 --- a/x-pack/plugins/watcher/server/routes/api/indices/register_indices_routes.ts +++ b/x-pack/plugins/watcher/server/routes/api/indices/register_indices_routes.ts @@ -6,8 +6,10 @@ */ import { registerGetRoute } from './register_get_route'; +import { registerGetIndexPatternsRoute } from './register_get_index_patterns_route'; import { RouteDependencies } from '../../../types'; export function registerIndicesRoutes(deps: RouteDependencies) { registerGetRoute(deps); + registerGetIndexPatternsRoute(deps); } From db61939164104eb56bf633bd5210859a88033e43 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Thu, 25 Nov 2021 11:57:04 +0100 Subject: [PATCH 2/9] commit using @elastic.co From f380922c972009471534237e34b435b917ef48e2 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Thu, 25 Nov 2021 13:32:45 +0100 Subject: [PATCH 3/9] Start working on tests --- .../register_get_index_patterns_route.test.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts new file mode 100644 index 000000000000..4b880473ca5e --- /dev/null +++ b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { kibanaResponseFactory } from 'src/core/server'; +import { createMockRouter, MockRouter, routeHandlerContextMock } from './__mocks__/routes.mock'; +import { createRequestMock } from './__mocks__/request.mock'; +import { handleEsError } from '../../../shared_imports'; + +jest.mock('../lib/es_version_precheck', () => ({ + versionCheckHandlerWrapper: (a: any) => a, +})); + +import { registerGetIndexPatternsRoute } from './register_get_index_patterns_route'; + +/** + * Since these route callbacks are so thin, these serve simply as integration tests + * to ensure they're wired up to the lib functions correctly. + */ +describe('GET index patterns API', () => { + let mockRouter: MockRouter; + let routeDependencies: any; + + beforeEach(() => { + mockRouter = createMockRouter(); + routeDependencies = { + router: mockRouter, + lib: { handleEsError }, + }; + registerSystemIndicesMigrationRoutes(routeDependencies); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('GET /api/watcher/indices/index_patterns', () => { + it('returns system indices migration status', async () => { + ( + routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport + .request as jest.Mock + ).mockResolvedValue({ + body: mockedResponse, + }); + + const resp = await routeDependencies.router.getHandler({ + method: 'get', + pathPattern: '/api/upgrade_assistant/system_indices_migration', + })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory); + + expect(resp.status).toEqual(200); + expect( + routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport.request + ).toHaveBeenCalledWith({ + method: 'GET', + path: '/_migration/system_features', + }); + expect(resp.payload).toEqual({ + ...mockedResponse, + features: mockedResponse.features.filter( + (feature) => feature.migration_status !== 'NO_MIGRATION_NEEDED' + ), + }); + }); + + it('returns an error if it throws', async () => { + ( + routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport + .request as jest.Mock + ).mockRejectedValue(new Error('scary error!')); + await expect( + routeDependencies.router.getHandler({ + method: 'get', + pathPattern: '/api/upgrade_assistant/system_indices_migration', + })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory) + ).rejects.toThrow('scary error!'); + }); + }); +}); From 80d7c01a39ebf6e98c4a218083b828fea9150e9c Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Mon, 29 Nov 2021 11:57:56 +0100 Subject: [PATCH 4/9] Add tests --- .../register_get_index_patterns_route.test.ts | 82 ------------------- .../register_get_index_patterns_route.ts | 4 +- x-pack/test/api_integration/apis/index.ts | 1 + .../api_integration/apis/watcher/index.ts | 14 ++++ .../api_integration/apis/watcher/watcher.ts | 26 ++++++ 5 files changed, 43 insertions(+), 84 deletions(-) delete mode 100644 x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts create mode 100644 x-pack/test/api_integration/apis/watcher/index.ts create mode 100644 x-pack/test/api_integration/apis/watcher/watcher.ts diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts deleted file mode 100644 index 4b880473ca5e..000000000000 --- a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { kibanaResponseFactory } from 'src/core/server'; -import { createMockRouter, MockRouter, routeHandlerContextMock } from './__mocks__/routes.mock'; -import { createRequestMock } from './__mocks__/request.mock'; -import { handleEsError } from '../../../shared_imports'; - -jest.mock('../lib/es_version_precheck', () => ({ - versionCheckHandlerWrapper: (a: any) => a, -})); - -import { registerGetIndexPatternsRoute } from './register_get_index_patterns_route'; - -/** - * Since these route callbacks are so thin, these serve simply as integration tests - * to ensure they're wired up to the lib functions correctly. - */ -describe('GET index patterns API', () => { - let mockRouter: MockRouter; - let routeDependencies: any; - - beforeEach(() => { - mockRouter = createMockRouter(); - routeDependencies = { - router: mockRouter, - lib: { handleEsError }, - }; - registerSystemIndicesMigrationRoutes(routeDependencies); - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - describe('GET /api/watcher/indices/index_patterns', () => { - it('returns system indices migration status', async () => { - ( - routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport - .request as jest.Mock - ).mockResolvedValue({ - body: mockedResponse, - }); - - const resp = await routeDependencies.router.getHandler({ - method: 'get', - pathPattern: '/api/upgrade_assistant/system_indices_migration', - })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory); - - expect(resp.status).toEqual(200); - expect( - routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport.request - ).toHaveBeenCalledWith({ - method: 'GET', - path: '/_migration/system_features', - }); - expect(resp.payload).toEqual({ - ...mockedResponse, - features: mockedResponse.features.filter( - (feature) => feature.migration_status !== 'NO_MIGRATION_NEEDED' - ), - }); - }); - - it('returns an error if it throws', async () => { - ( - routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport - .request as jest.Mock - ).mockRejectedValue(new Error('scary error!')); - await expect( - routeDependencies.router.getHandler({ - method: 'get', - pathPattern: '/api/upgrade_assistant/system_indices_migration', - })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory) - ).rejects.toThrow('scary error!'); - }); - }); -}); diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts index eab3e75936b4..be8308d0120e 100644 --- a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts +++ b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts @@ -39,8 +39,8 @@ export function registerGetIndexPatternsRoute({ await finder.close(); return response.ok({ body: responses }); - } catch (e) { - return handleEsError({ error: e, response }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/test/api_integration/apis/index.ts b/x-pack/test/api_integration/apis/index.ts index 56b2042dc485..b37d88a5dc42 100644 --- a/x-pack/test/api_integration/apis/index.ts +++ b/x-pack/test/api_integration/apis/index.ts @@ -34,5 +34,6 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./painless_lab')); loadTestFile(require.resolve('./file_upload')); loadTestFile(require.resolve('./ml')); + loadTestFile(require.resolve('./watcher')); }); } diff --git a/x-pack/test/api_integration/apis/watcher/index.ts b/x-pack/test/api_integration/apis/watcher/index.ts new file mode 100644 index 000000000000..964b7fa0099a --- /dev/null +++ b/x-pack/test/api_integration/apis/watcher/index.ts @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ loadTestFile }: FtrProviderContext) { + describe('Watcher', () => { + loadTestFile(require.resolve('./watcher')); + }); +} diff --git a/x-pack/test/api_integration/apis/watcher/watcher.ts b/x-pack/test/api_integration/apis/watcher/watcher.ts new file mode 100644 index 000000000000..ef2981f505fd --- /dev/null +++ b/x-pack/test/api_integration/apis/watcher/watcher.ts @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; + +export default function ({ getService }) { + const supertest = getService('supertest'); + + describe('watcher', () => { + describe('POST /api/watcher/indices/index_patterns', () => { + it('returns list of index patterns', async () => { + const response = await supertest + .get('/api/watcher/indices/index_patterns') + .set('kbn-xsrf', 'kibana') + .expect(200); + + const expectedResponse = ['metrics-*', 'logs-*']; + expect(response.body).to.eql(expectedResponse); + }); + }); + }); +} From 99ec99539a815543785a69e70b0ba9e3e7f498a4 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Mon, 29 Nov 2021 13:07:01 +0100 Subject: [PATCH 5/9] Add missing types --- x-pack/test/api_integration/apis/watcher/watcher.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/test/api_integration/apis/watcher/watcher.ts b/x-pack/test/api_integration/apis/watcher/watcher.ts index ef2981f505fd..c1686226e112 100644 --- a/x-pack/test/api_integration/apis/watcher/watcher.ts +++ b/x-pack/test/api_integration/apis/watcher/watcher.ts @@ -7,7 +7,9 @@ import expect from '@kbn/expect'; -export default function ({ getService }) { +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); describe('watcher', () => { From c33b64f3034e0cf4a99196f0d60369e9759f934a Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Tue, 30 Nov 2021 10:41:05 +0100 Subject: [PATCH 6/9] Fix tests --- .../api_integration/apis/watcher/watcher.ts | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/x-pack/test/api_integration/apis/watcher/watcher.ts b/x-pack/test/api_integration/apis/watcher/watcher.ts index c1686226e112..734b6c8c6212 100644 --- a/x-pack/test/api_integration/apis/watcher/watcher.ts +++ b/x-pack/test/api_integration/apis/watcher/watcher.ts @@ -10,9 +10,29 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getService }: FtrProviderContext) { + const log = getService('log'); const supertest = getService('supertest'); + const transform = getService('transform'); describe('watcher', () => { + before(async () => { + try { + await transform.testResources.createIndexPatternIfNeeded('ft_ecommerce', 'order_date'); + } catch (error) { + log.debug('[Setup error] Error creating index pattern'); + throw error; + } + }); + + after(async () => { + try { + await transform.testResources.deleteIndexPatternByTitle('ft_ecommerce'); + } catch (error) { + log.debug('[Cleanup error] Error deleting index pattern'); + throw error; + } + }); + describe('POST /api/watcher/indices/index_patterns', () => { it('returns list of index patterns', async () => { const response = await supertest @@ -20,8 +40,7 @@ export default function ({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'kibana') .expect(200); - const expectedResponse = ['metrics-*', 'logs-*']; - expect(response.body).to.eql(expectedResponse); + expect(response.body).to.contain('ft_ecommerce'); }); }); }); From c15530a124ad33faa025831ebd025203be92f2ee Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Tue, 30 Nov 2021 10:51:21 +0100 Subject: [PATCH 7/9] Update tests responses --- .../client_integration/watch_create_threshold.test.tsx | 7 +------ .../watcher/__jest__/client_integration/watch_edit.test.ts | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx b/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx index 481f59093d7d..c76dc755d858 100644 --- a/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx +++ b/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx @@ -51,12 +51,7 @@ jest.mock('../../public/application/lib/api', () => { return { ...original, loadIndexPatterns: async () => { - const INDEX_PATTERNS = [ - { attributes: { title: 'index1' } }, - { attributes: { title: 'index2' } }, - { attributes: { title: 'index3' } }, - ]; - return await INDEX_PATTERNS; + return ['index1', 'index2', 'index3']; }, getHttpClient: () => mockHttpClient, }; diff --git a/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts b/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts index 1188cc8469a5..d0080ea5ef59 100644 --- a/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts +++ b/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts @@ -24,12 +24,7 @@ jest.mock('../../public/application/lib/api', () => { return { ...original, loadIndexPatterns: async () => { - const INDEX_PATTERNS = [ - { attributes: { title: 'index1' } }, - { attributes: { title: 'index2' } }, - { attributes: { title: 'index3' } }, - ]; - return await INDEX_PATTERNS; + return ['index1', 'index2', 'index3']; }, getHttpClient: () => mockHttpClient, }; From 85512ed18772f04085e980cc0964e6957e82b632 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Wed, 1 Dec 2021 11:45:44 +0100 Subject: [PATCH 8/9] Remove unnecessary mocks --- .../client_integration/watch_create_threshold.test.tsx | 3 --- .../watcher/__jest__/client_integration/watch_edit.test.ts | 3 --- 2 files changed, 6 deletions(-) diff --git a/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx b/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx index c76dc755d858..52c3a69938d7 100644 --- a/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx +++ b/x-pack/plugins/watcher/__jest__/client_integration/watch_create_threshold.test.tsx @@ -50,9 +50,6 @@ jest.mock('../../public/application/lib/api', () => { return { ...original, - loadIndexPatterns: async () => { - return ['index1', 'index2', 'index3']; - }, getHttpClient: () => mockHttpClient, }; }); diff --git a/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts b/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts index d0080ea5ef59..b5fb2aa9d915 100644 --- a/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts +++ b/x-pack/plugins/watcher/__jest__/client_integration/watch_edit.test.ts @@ -23,9 +23,6 @@ jest.mock('../../public/application/lib/api', () => { return { ...original, - loadIndexPatterns: async () => { - return ['index1', 'index2', 'index3']; - }, getHttpClient: () => mockHttpClient, }; }); From f152e4b98e1e3329d18a8909fc48be77bf534967 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Fri, 3 Dec 2021 10:15:20 +0100 Subject: [PATCH 9/9] Fix type --- .../routes/api/indices/register_get_index_patterns_route.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts index be8308d0120e..237f6dac6a25 100644 --- a/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts +++ b/x-pack/plugins/watcher/server/routes/api/indices/register_get_index_patterns_route.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { SavedObject, SavedObjectsFindResult } from 'src/core/server'; +import { SavedObjectsFindResult } from 'src/core/server'; import { RouteDependencies } from '../../../types'; export function registerGetIndexPatternsRoute({ @@ -26,7 +26,7 @@ export function registerGetIndexPatternsRoute({ perPage: 1000, }); - const responses: SavedObject[] = []; + const responses: string[] = []; for await (const result of finder.find()) { responses.push(