From ce00fcb04fd632963c2146eb263eaca896264913 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Wed, 6 Oct 2021 14:42:34 +0200 Subject: [PATCH 1/6] First iteration on clearing logs cache --- .../deprecations_count_checkpoint.tsx | 3 +- .../public/application/lib/api.ts | 9 ++++++ .../server/routes/deprecation_logging.ts | 29 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx index 955d86493d479..7714e156001e8 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx +++ b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx @@ -71,9 +71,10 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ const calloutIcon = hasLogs ? 'alert' : 'check'; const calloutTestId = hasLogs ? 'hasWarningsCallout' : 'noWarningsCallout'; - const onResetClick = () => { + const onResetClick = async () => { const now = moment().toISOString(); uiMetricService.trackUiMetric(METRIC_TYPE.CLICK, UIM_RESET_LOGS_COUNTER_CLICK); + api.deleteDeprecationLogsCache(); setCheckpoint(now); }; diff --git a/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts b/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts index 1d51510333ef4..cedef8f8e3726 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts +++ b/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts @@ -96,6 +96,15 @@ export class ApiService { }); } + public async deleteDeprecationLogsCache() { + const result = await this.sendRequest({ + path: `${API_BASE_PATH}/deprecation_logging/deprecation_cache`, + method: 'delete', + }); + + return result; + } + public async updateIndexSettings(indexName: string, settings: string[]) { const result = await this.sendRequest({ path: `${API_BASE_PATH}/${indexName}/index_settings`, diff --git a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts index 00116ae24a3fb..48445810705ff 100644 --- a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts +++ b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts @@ -124,4 +124,33 @@ export function registerDeprecationLoggingRoutes({ } ) ); + + router.delete( + { + path: `${API_BASE_PATH}/deprecation_logging/deprecation_cache`, + validate: false, + }, + versionCheckHandlerWrapper( + async ( + { + core: { + elasticsearch: { client }, + }, + }, + request, + response + ) => { + try { + await client.asCurrentUser.transport.request({ + method: 'DELETE', + path: '/_logging/deprecation_cache', + }); + + return response.ok({ body: 'ok' }); + } catch (error) { + return handleEsError({ error, response }); + } + } + ) + ); } From 01a2ec70d9c762463bbb245ec07668f8ad5581f9 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Wed, 6 Oct 2021 16:31:30 +0200 Subject: [PATCH 2/6] Show toast when an error ocurrs and add tests --- .../helpers/http_requests.ts | 11 +++++ .../fix_logs_step/fix_logs_step.test.tsx | 42 +++++++++++++++++++ .../deprecations_count_checkpoint.tsx | 19 ++++++++- .../public/application/lib/api.ts | 8 ++-- .../server/routes/deprecation_logging.test.ts | 38 +++++++++++++++++ .../server/routes/deprecation_logging.ts | 2 +- 6 files changed, 112 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts index 448eb79152894..4f0e048da0792 100644 --- a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts +++ b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/helpers/http_requests.ts @@ -69,6 +69,16 @@ const registerHttpRequestMockHelpers = (server: SinonFakeServer) => { ]); }; + const setDeleteLogsCacheResponse = (response?: string, error?: ResponseError) => { + const status = error ? error.statusCode || 400 : 200; + const body = error ? error : response; + server.respondWith('DELETE', `${API_BASE_PATH}/deprecation_logging/cache`, [ + status, + { 'Content-Type': 'application/json' }, + JSON.stringify(body), + ]); + }; + const setUpdateDeprecationLoggingResponse = ( response?: DeprecationLoggingStatus, error?: ResponseError @@ -169,6 +179,7 @@ const registerHttpRequestMockHelpers = (server: SinonFakeServer) => { setDeleteMlSnapshotResponse, setUpgradeMlSnapshotStatusResponse, setLoadDeprecationLogsCountResponse, + setDeleteLogsCacheResponse, setStartReindexingResponse, setReindexStatusResponse, setLoadMlUpgradeModeResponse, diff --git a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx index 7f58b36d29c36..10a098240f42a 100644 --- a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx +++ b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx @@ -248,6 +248,7 @@ describe('Overview - Fix deprecation logs step', () => { describe('Step 3 - Resolve log issues', () => { beforeEach(async () => { httpRequestsMockHelpers.setLoadDeprecationLoggingResponse(getLoggingResponse(true)); + httpRequestsMockHelpers.setDeleteLogsCacheResponse('ok'); }); test('With deprecation warnings', async () => { @@ -337,6 +338,47 @@ describe('Overview - Fix deprecation logs step', () => { expect(exists('noWarningsCallout')).toBe(true); }); + test('Shows a toast if deleting cache fails', async () => { + const error = { + statusCode: 500, + error: 'Internal server error', + message: 'Internal server error', + }; + + httpRequestsMockHelpers.setDeleteLogsCacheResponse(undefined, error); + httpRequestsMockHelpers.setLoadDeprecationLogsCountResponse({ count: 10 }); + + const addDanger = jest.fn(); + await act(async () => { + testBed = await setupOverviewPage({ + services: { + core: { + notifications: { + toasts: { + addDanger, + }, + }, + }, + }, + }); + }); + + const { exists, actions, component } = testBed; + + component.update(); + + httpRequestsMockHelpers.setLoadDeprecationLogsCountResponse({ count: 0 }); + + await actions.clickResetButton(); + + // The toast should always be shown if the delete logs cache fails. + expect(addDanger).toHaveBeenCalled(); + // Even though a stub for the API call getting the count was set to return 0, + // given that the delete logs cache API call will fail the callout remains in + // warning state. + expect(exists('hasWarningsCallout')).toBe(true); + }); + describe('Poll for logs count', () => { beforeEach(async () => { jest.useFakeTimers(); diff --git a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx index 7714e156001e8..7bd83d411a7c2 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx +++ b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx @@ -46,6 +46,9 @@ const i18nTexts = { defaultMessage: 'Reset counter', } ), + errorToastTitle: i18n.translate('xpack.upgradeAssistant.overview.verifyChanges.errorToastTitle', { + defaultMessage: 'Could not clear deprecation logs cache', + }), }; interface Props { @@ -60,7 +63,10 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ setHasNoDeprecationLogs, }) => { const { - services: { api }, + services: { + api, + core: { notifications }, + }, } = useAppContext(); const { data, error, isLoading, resendRequest, isInitialRequest } = api.getDeprecationLogsCount(checkpoint); @@ -72,9 +78,18 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ const calloutTestId = hasLogs ? 'hasWarningsCallout' : 'noWarningsCallout'; const onResetClick = async () => { + const { error: deleteLogsCacheError } = await api.deleteDeprecationLogsCache(); + + if (deleteLogsCacheError) { + notifications.toasts.addDanger({ + title: i18nTexts.errorToastTitle, + text: deleteLogsCacheError.message.toString(), + }); + return; + } + const now = moment().toISOString(); uiMetricService.trackUiMetric(METRIC_TYPE.CLICK, UIM_RESET_LOGS_COUNTER_CLICK); - api.deleteDeprecationLogsCache(); setCheckpoint(now); }; diff --git a/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts b/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts index cedef8f8e3726..262a62c50ea31 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts +++ b/x-pack/plugins/upgrade_assistant/public/application/lib/api.ts @@ -96,13 +96,11 @@ export class ApiService { }); } - public async deleteDeprecationLogsCache() { - const result = await this.sendRequest({ - path: `${API_BASE_PATH}/deprecation_logging/deprecation_cache`, + public deleteDeprecationLogsCache() { + return this.sendRequest({ + path: `${API_BASE_PATH}/deprecation_logging/cache`, method: 'delete', }); - - return result; } public async updateIndexSettings(indexName: string, settings: string[]) { diff --git a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.test.ts b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.test.ts index 1a4b55447f4ad..fc61832e69c12 100644 --- a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.test.ts +++ b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.test.ts @@ -176,4 +176,42 @@ describe('deprecation logging API', () => { ).rejects.toThrow('scary error!'); }); }); + + describe('DELETE /api/upgrade_assistant/deprecation_logging/cache', () => { + it('returns ok if if the cache was deleted', async () => { + ( + routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport + .request as jest.Mock + ).mockResolvedValue({ + body: 'ok', + }); + + const resp = await routeDependencies.router.getHandler({ + method: 'delete', + pathPattern: '/api/upgrade_assistant/deprecation_logging/cache', + })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory); + + expect(resp.status).toEqual(200); + expect( + routeHandlerContextMock.core.elasticsearch.client.asCurrentUser.transport.request + ).toHaveBeenCalledWith({ + method: 'DELETE', + path: '/_logging/deprecation_cache', + }); + expect(resp.payload).toEqual('ok'); + }); + + 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: 'delete', + pathPattern: '/api/upgrade_assistant/deprecation_logging/cache', + })(routeHandlerContextMock, createRequestMock(), kibanaResponseFactory) + ).rejects.toThrow('scary error!'); + }); + }); }); diff --git a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts index 48445810705ff..5d7f0f67b0ca9 100644 --- a/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts +++ b/x-pack/plugins/upgrade_assistant/server/routes/deprecation_logging.ts @@ -127,7 +127,7 @@ export function registerDeprecationLoggingRoutes({ router.delete( { - path: `${API_BASE_PATH}/deprecation_logging/deprecation_cache`, + path: `${API_BASE_PATH}/deprecation_logging/cache`, validate: false, }, versionCheckHandlerWrapper( From 2d794041b7ab2a2dbff10a237260892ee0617b2c Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Wed, 6 Oct 2021 16:40:03 +0200 Subject: [PATCH 3/6] Add better docs --- .../overview/fix_logs_step/fix_logs_step.test.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx index 10a098240f42a..7d4b6b8518dff 100644 --- a/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx +++ b/x-pack/plugins/upgrade_assistant/__jest__/client_integration/overview/fix_logs_step/fix_logs_step.test.tsx @@ -346,6 +346,7 @@ describe('Overview - Fix deprecation logs step', () => { }; httpRequestsMockHelpers.setDeleteLogsCacheResponse(undefined, error); + // Initially we want to have the callout to have a warning state httpRequestsMockHelpers.setLoadDeprecationLogsCountResponse({ count: 10 }); const addDanger = jest.fn(); @@ -373,9 +374,9 @@ describe('Overview - Fix deprecation logs step', () => { // The toast should always be shown if the delete logs cache fails. expect(addDanger).toHaveBeenCalled(); - // Even though a stub for the API call getting the count was set to return 0, - // given that the delete logs cache API call will fail the callout remains in - // warning state. + // Even though we changed the response of the getLogsCountResponse, when the + // deleteLogsCache fails the getLogsCount api should not be called and the + // status of the callout should remain the same it initially was. expect(exists('hasWarningsCallout')).toBe(true); }); From 559dac6ea231057c579595b73c7b40e713a8f9cf Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Wed, 6 Oct 2021 16:41:29 +0200 Subject: [PATCH 4/6] Change copy --- .../deprecations_count_checkpoint.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx index 7bd83d411a7c2..12ec9ed7e2317 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx +++ b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx @@ -47,7 +47,7 @@ const i18nTexts = { } ), errorToastTitle: i18n.translate('xpack.upgradeAssistant.overview.verifyChanges.errorToastTitle', { - defaultMessage: 'Could not clear deprecation logs cache', + defaultMessage: 'Could not delete deprecation logs cache', }), }; From 8b5016af15f86baa0cbb05fc1ea9618a8f119b95 Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Thu, 7 Oct 2021 08:53:33 +0200 Subject: [PATCH 5/6] Disable button while request is ongoing --- .../deprecations_count_checkpoint.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx index 7cf1ebc3ed475..e543cf66e7b45 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx +++ b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { FunctionComponent, useEffect } from 'react'; +import React, { FunctionComponent, useEffect, useState } from 'react'; import moment from 'moment-timezone'; import { FormattedDate, FormattedTime, FormattedMessage } from '@kbn/i18n/react'; @@ -60,6 +60,7 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ setCheckpoint, setHasNoDeprecationLogs, }) => { + const [isDeletingCache, setIsDeletingCache] = useState(false); const { services: { api, @@ -76,7 +77,9 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ const calloutTestId = hasLogs ? 'hasWarningsCallout' : 'noWarningsCallout'; const onResetClick = async () => { + setIsDeletingCache(true); const { error: deleteLogsCacheError } = await api.deleteDeprecationLogsCache(); + setIsDeletingCache(false); if (deleteLogsCacheError) { notifications.toasts.addDanger({ @@ -130,7 +133,12 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ data-test-subj={calloutTestId} >

{i18nTexts.calloutBody}

- + {i18nTexts.resetCounterButton} From 0c2e7cb298bd55c1943a4e98494755e3e101210c Mon Sep 17 00:00:00 2001 From: Ignacio Rivas Date: Mon, 11 Oct 2021 12:14:20 +0200 Subject: [PATCH 6/6] Address CR changes --- .../deprecations_count_checkpoint.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx index e543cf66e7b45..e444c6f35348b 100644 --- a/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx +++ b/x-pack/plugins/upgrade_assistant/public/application/components/overview/fix_logs_step/deprecations_count_checkpoint/deprecations_count_checkpoint.tsx @@ -136,7 +136,7 @@ export const DeprecationsCountCheckpoint: FunctionComponent = ({ {i18nTexts.resetCounterButton}