From f2b461e37fb07ec62cb99e5aee53a29125615f91 Mon Sep 17 00:00:00 2001 From: Yan Zeng Date: Wed, 14 Dec 2022 11:18:31 -0800 Subject: [PATCH 1/4] Update console to use core.http instead of jQuery.ajax Signed-off-by: Yan Zeng --- .../application/contexts/services_context.tsx | 3 +- .../send_request_to_opensearch.ts | 155 ++++++++++-------- .../use_send_current_request_to_opensearch.ts | 6 +- .../console/public/application/index.tsx | 1 + .../public/lib/opensearch/opensearch.ts | 45 ++--- 5 files changed, 107 insertions(+), 103 deletions(-) diff --git a/src/plugins/console/public/application/contexts/services_context.tsx b/src/plugins/console/public/application/contexts/services_context.tsx index 9e926eef9730..fc9ab157f783 100644 --- a/src/plugins/console/public/application/contexts/services_context.tsx +++ b/src/plugins/console/public/application/contexts/services_context.tsx @@ -29,7 +29,7 @@ */ import React, { createContext, useContext, useEffect } from 'react'; -import { NotificationsSetup } from 'opensearch-dashboards/public'; +import { HttpSetup, NotificationsSetup } from 'opensearch-dashboards/public'; import { History, Settings, Storage } from '../../services'; import { ObjectStorageClient } from '../../../common/types'; import { MetricsTracker } from '../../types'; @@ -43,6 +43,7 @@ interface ContextServices { objectStorageClient: ObjectStorageClient; trackUiMetric: MetricsTracker; opensearchHostService: OpenSearchHostService; + http: HttpSetup; } export interface ContextValue { diff --git a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts index a74ab610a67c..d3785b51694a 100644 --- a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts +++ b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts @@ -28,6 +28,7 @@ * under the License. */ +import { HttpFetchError, HttpSetup } from 'opensearch-dashboards/public'; import { extractDeprecationMessages } from '../../../lib/utils'; import { XJson } from '../../../../../opensearch_ui_shared/public'; const { collapseLiteralStrings } = XJson; @@ -36,6 +37,7 @@ import * as opensearch from '../../../lib/opensearch/opensearch'; import { BaseResponseType } from '../../../types'; export interface OpenSearchRequestArgs { + http: HttpSetup; requests: any; } @@ -76,7 +78,7 @@ export function sendRequestToOpenSearch( const isMultiRequest = requests.length > 1; - const sendNextRequest = () => { + const sendNextRequest = async () => { if (reqId !== CURRENT_REQ_ID) { resolve(results); return; @@ -94,79 +96,94 @@ export function sendRequestToOpenSearch( } // append a new line for bulk requests. const startTime = Date.now(); - opensearch - .send(opensearchMethod, opensearchPath, opensearchData) - .always((dataOrjqXHR: any, textStatus: string, jqXhrORerrorThrown: any) => { - if (reqId !== CURRENT_REQ_ID) { - return; - } - - const xhr = dataOrjqXHR.promise ? dataOrjqXHR : jqXhrORerrorThrown; - - const isSuccess = - typeof xhr.status === 'number' && - // Things like DELETE index where the index is not there are OK. - ((xhr.status >= 200 && xhr.status < 300) || xhr.status === 404); - - if (isSuccess) { - let value = xhr.responseText; - - const warnings = xhr.getResponseHeader('warning'); - if (warnings) { - const deprecationMessages = extractDeprecationMessages(warnings); - value = deprecationMessages.join('\n') + '\n' + value; - } - - if (isMultiRequest) { - value = '# ' + req.method + ' ' + req.url + '\n' + value; - } - - results.push({ - response: { - timeMs: Date.now() - startTime, - statusCode: xhr.status, - statusText: xhr.statusText, - contentType: xhr.getResponseHeader('Content-Type'), - value, - }, - request: { - data: opensearchData, - method: opensearchMethod, - path: opensearchPath, - }, - }); - - // single request terminate via sendNextRequest as well - sendNextRequest(); + try { + const httpResponse = await opensearch.send( + args.http, + opensearchMethod, + opensearchPath, + opensearchData + ); + if (reqId !== CURRENT_REQ_ID) { + return; + } + const statusCode = httpResponse.response?.status; + const isSuccess = statusCode && (statusCode >= 200 || statusCode === 404); + if (isSuccess) { + const contentType = httpResponse.response.headers.get('Content-Type') as BaseResponseType; + let value = ''; + if (contentType.includes('application/json')) { + value = JSON.stringify(httpResponse.body, null, 2); } else { - let value; - let contentType: string; - if (xhr.responseText) { - value = xhr.responseText; // OpenSearch error should be shown - contentType = xhr.getResponseHeader('Content-Type'); + value = httpResponse.body; + } + const warnings = httpResponse.response.headers.get('warning'); + if (warnings) { + const deprecationMessages = extractDeprecationMessages(warnings); + value = deprecationMessages.join('\n') + '\n' + value; + } + if (isMultiRequest) { + value = '# ' + req.method + ' ' + req.url + '\n' + value; + } + results.push({ + response: { + timeMs: Date.now() - startTime, + statusCode, + statusText: httpResponse.response.statusText, + contentType, + value, + }, + request: { + data: opensearchData, + method: opensearchMethod, + path: opensearchPath, + }, + }); + + // single request terminate via sendNextRequest as well + await sendNextRequest(); + } + } catch (error) { + const httpError = error as HttpFetchError; + const httpResponse = httpError.response; + let value; + let contentType: string; + if (httpResponse) { + if (httpError.body) { + contentType = httpResponse.headers.get('Content-Type') as string; + if (contentType?.includes('application/json')) { + value = JSON.stringify(httpError.body, null, 2); } else { - value = 'Request failed to get to the server (status code: ' + xhr.status + ')'; - contentType = 'text/plain'; - } - if (isMultiRequest) { - value = '# ' + req.method + ' ' + req.url + '\n' + value; + value = httpError.body; } - reject({ - response: { - value, - contentType, - timeMs: Date.now() - startTime, - statusCode: xhr.status, - statusText: xhr.statusText, - }, - request: { - data: opensearchData, - method: opensearchMethod, - path: opensearchPath, - }, - }); + } else { + value = + 'Request failed to get to the server (status code: ' + httpResponse.status + ')'; + contentType = 'text/plain'; } + } else { + value = + "\n\nFailed to connect to Console's backend.\nPlease check the OpenSearch Dashboards server is up and running"; + contentType = 'text/plain'; + } + + if (isMultiRequest) { + value = '# ' + req.method + ' ' + req.url + '\n' + value; + } + reject({ + response: { + value, + contentType, + timeMs: Date.now() - startTime, + statusCode: httpResponse?.status, + statusText: httpResponse?.statusText, + }, + request: { + data: opensearchData, + method: opensearchMethod, + path: opensearchPath, + }, }); + } }; sendNextRequest(); diff --git a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/use_send_current_request_to_opensearch.ts b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/use_send_current_request_to_opensearch.ts index e5c9e7f2d2d1..30714f56b852 100644 --- a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/use_send_current_request_to_opensearch.ts +++ b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/use_send_current_request_to_opensearch.ts @@ -40,7 +40,7 @@ import { retrieveAutoCompleteInfo } from '../../../lib/mappings/mappings'; export const useSendCurrentRequestToOpenSearch = () => { const { - services: { history, settings, notifications, trackUiMetric }, + services: { history, settings, notifications, trackUiMetric, http }, } = useServicesContext(); const dispatch = useRequestActionContext(); @@ -64,7 +64,7 @@ export const useSendCurrentRequestToOpenSearch = () => { // Fire and forget setTimeout(() => track(requests, editor, trackUiMetric), 0); - const results = await sendRequestToOpenSearch({ requests }); + const results = await sendRequestToOpenSearch({ http, requests }); results.forEach(({ request: { path, method, data } }) => { try { @@ -112,5 +112,5 @@ export const useSendCurrentRequestToOpenSearch = () => { }); } } - }, [dispatch, settings, history, notifications, trackUiMetric]); + }, [dispatch, settings, history, notifications, trackUiMetric, http]); }; diff --git a/src/plugins/console/public/application/index.tsx b/src/plugins/console/public/application/index.tsx index 51136c7805d9..a7d757482e96 100644 --- a/src/plugins/console/public/application/index.tsx +++ b/src/plugins/console/public/application/index.tsx @@ -82,6 +82,7 @@ export function renderApp({ notifications, trackUiMetric, objectStorageClient, + http, }, }} > diff --git a/src/plugins/console/public/lib/opensearch/opensearch.ts b/src/plugins/console/public/lib/opensearch/opensearch.ts index 3360d0108c12..ab6b79469e88 100644 --- a/src/plugins/console/public/lib/opensearch/opensearch.ts +++ b/src/plugins/console/public/lib/opensearch/opensearch.ts @@ -28,8 +28,7 @@ * under the License. */ -import $ from 'jquery'; -import { stringify } from 'query-string'; +import { HttpResponse, HttpSetup } from 'opensearch-dashboards/public'; const opensearchVersion: string[] = []; @@ -42,35 +41,21 @@ export function getContentType(body: any) { return 'application/json'; } -export function send(method: string, path: string, data: any) { - const wrappedDfd = $.Deferred(); - - const options: JQuery.AjaxSettings = { - url: '../api/console/proxy?' + stringify({ path, method }, { sort: false }), - headers: { - 'osd-xsrf': 'opensearchDashboards', - }, - data, - contentType: getContentType(data), - cache: false, - crossDomain: true, - type: 'POST', - dataType: 'text', // disable automatic guessing - }; - - $.ajax(options).then( - (responseData: any, textStatus: string, jqXHR: any) => { - wrappedDfd.resolveWith({}, [responseData, textStatus, jqXHR]); +export async function send( + http: HttpSetup, + method: string, + path: string, + data: any +): Promise { + return await http.post('/api/console/proxy', { + query: { + path, + method, }, - ((jqXHR: any, textStatus: string, errorThrown: Error) => { - if (jqXHR.status === 0) { - jqXHR.responseText = - "\n\nFailed to connect to Console's backend.\nPlease check the OpenSearch Dashboards server is up and running"; - } - wrappedDfd.rejectWith({}, [jqXHR, textStatus, errorThrown]); - }) as any - ); - return wrappedDfd; + body: data, + prependBasePath: true, + asResponse: true, + }); } export function constructOpenSearchUrl(baseUri: string, path: string) { From 72f0da26ee9c3e860db4cd77a38b747d19e16acd Mon Sep 17 00:00:00 2001 From: Yan Zeng Date: Wed, 14 Dec 2022 11:50:58 -0800 Subject: [PATCH 2/4] Update functional test, add change log Signed-off-by: Yan Zeng --- CHANGELOG.md | 1 + test/functional/apps/console/_console.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ff128235ef1..c4038d043546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [MD] Refactor data source error handling ([#2661](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2661)) - Refactor and improve Discover field summaries ([#2391](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2391)) - [Vis Builder] Removed Hard Coded Strings and Used i18n to transalte([#2867](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2867)) +- [Console] Replace jQuery.ajax with core.http when calling OSD APIs in console ([#3080]https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3080)) ### 🔩 Tests diff --git a/test/functional/apps/console/_console.ts b/test/functional/apps/console/_console.ts index f5cd93c35280..302197bb4550 100644 --- a/test/functional/apps/console/_console.ts +++ b/test/functional/apps/console/_console.ts @@ -71,7 +71,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); it('default request response should include `"timed_out" : false`', async () => { - const expectedResponseContains = '"timed_out" : false,'; + const expectedResponseContains = '"timed_out": false,'; await PageObjects.console.clickPlay(); await retry.try(async () => { const actualResponse = await PageObjects.console.getResponse(); From 42e3c05aea7edee4a7a323e6c07b8b7a79ff18cd Mon Sep 17 00:00:00 2001 From: Yan Zeng Date: Mon, 19 Dec 2022 01:55:16 -0800 Subject: [PATCH 3/4] Fix status code check, add more test cases Signed-off-by: Yan Zeng --- .../send_request_to_opensearch.test.ts | 182 ++++++++++++++++++ .../send_request_to_opensearch.ts | 3 +- 2 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.test.ts diff --git a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.test.ts b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.test.ts new file mode 100644 index 000000000000..eaa171132785 --- /dev/null +++ b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.test.ts @@ -0,0 +1,182 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Any modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +import { + HttpFetchError, + HttpFetchOptionsWithPath, + HttpResponse, + HttpSetup, +} from '../../../../../../core/public'; +import { OpenSearchRequestArgs, sendRequestToOpenSearch } from './send_request_to_opensearch'; +import * as opensearch from '../../../lib/opensearch/opensearch'; + +const createMockResponse = ( + statusCode: number, + statusText: string, + headers: Array<[string, string]> +): Response => { + return { + // headers: {} as Headers, + headers: new Headers(headers), + ok: true, + redirected: false, + status: statusCode, + statusText, + type: 'basic', + url: '', + clone: jest.fn(), + body: (jest.fn() as unknown) as ReadableStream, + bodyUsed: true, + arrayBuffer: jest.fn(), + blob: jest.fn(), + text: jest.fn(), + formData: jest.fn(), + json: jest.fn(), + }; +}; + +const createMockHttpResponse = ( + statusCode: number, + statusText: string, + headers: Array<[string, string]>, + body: any +): HttpResponse => { + return { + fetchOptions: (jest.fn() as unknown) as Readonly, + request: (jest.fn() as unknown) as Readonly, + response: createMockResponse(statusCode, statusText, headers), + body, + }; +}; +const dummyArgs: OpenSearchRequestArgs = { + http: ({ + post: jest.fn(), + } as unknown) as HttpSetup, + requests: [ + { + method: 'GET', + url: '/dummy/api', + data: ['{}'], + }, + ], +}; + +describe('test sendRequestToOpenSearch', () => { + it('test request success, json', () => { + const mockHttpResponse = createMockHttpResponse( + 200, + 'ok', + [['Content-Type', 'application/json, utf-8']], + { + ok: true, + } + ); + + jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + sendRequestToOpenSearch(dummyArgs).then((result) => { + expect((result as any)[0].response.value).toBe('{\n "ok": true\n}'); + }); + }); + + it('test request success, text', () => { + const mockHttpResponse = createMockHttpResponse( + 200, + 'ok', + [['Content-Type', 'text/plain']], + 'response text' + ); + + jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + sendRequestToOpenSearch(dummyArgs).then((result) => { + expect((result as any)[0].response.value).toBe('response text'); + }); + }); + + it('test request success, with warning', () => { + const mockHttpResponse = createMockHttpResponse( + 200, + 'ok', + [ + ['Content-Type', 'text/plain'], + ['warning', 'dummy warning'], + ], + 'response text' + ); + + jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + sendRequestToOpenSearch(dummyArgs).then((result) => { + expect((result as any)[0].response.value).toBe( + '#! Deprecation: dummy warning\nresponse text' + ); + }); + }); + + it('test request 404', () => { + const mockHttpResponse = createMockHttpResponse( + 404, + 'not found', + [['Content-Type', 'text/plain']], + 'response text' + ); + + jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + sendRequestToOpenSearch(dummyArgs).then((result) => { + expect((result as any)[0].response.value).toBe('response text'); + }); + }); + + it('test request 500, json', () => { + const mockHttpError: HttpFetchError = new HttpFetchError( + 'error message', + 'name', + (jest.fn as unknown) as Request, + createMockResponse(500, 'Server Error', [['Content-Type', 'application/json, utf-8']]), + { errorMsg: 'message' } + ); + + jest.spyOn(opensearch, 'send').mockRejectedValue(mockHttpError); + sendRequestToOpenSearch(dummyArgs).catch((error) => { + expect(error.response.value).toBe('{\n "errorMsg": "message"\n}'); + }); + }); + + it('test request 500, text', () => { + const mockHttpError: HttpFetchError = new HttpFetchError( + 'error message', + 'name', + (jest.fn as unknown) as Request, + createMockResponse(500, 'Server Error', [['Content-Type', 'text/plain']]), + 'error message' + ); + + jest.spyOn(opensearch, 'send').mockRejectedValue(mockHttpError); + sendRequestToOpenSearch(dummyArgs).catch((error) => { + expect(error.response.value).toBe('error message'); + }); + }); + + it('test no connection', () => { + const mockHttpError: HttpFetchError = new HttpFetchError( + 'error message', + 'name', + (jest.fn as unknown) as Request, + undefined, + 'error message' + ); + + jest.spyOn(opensearch, 'send').mockRejectedValue(mockHttpError); + sendRequestToOpenSearch(dummyArgs).catch((error) => { + expect(error.response.value).toBe( + "\n\nFailed to connect to Console's backend.\nPlease check the OpenSearch Dashboards server is up and running" + ); + }); + }); +}); diff --git a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts index d3785b51694a..3128c4ec339f 100644 --- a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts +++ b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts @@ -107,7 +107,8 @@ export function sendRequestToOpenSearch( return; } const statusCode = httpResponse.response?.status; - const isSuccess = statusCode && (statusCode >= 200 || statusCode === 404); + const isSuccess = + statusCode && ((statusCode >= 200 && statusCode < 300) || statusCode === 404); if (isSuccess) { const contentType = httpResponse.response.headers.get('Content-Type') as BaseResponseType; let value = ''; From 393283f73c7b6c2b042fa4953f7d71f1951dd8b9 Mon Sep 17 00:00:00 2001 From: Yan Zeng Date: Tue, 20 Dec 2022 12:00:02 -0800 Subject: [PATCH 4/4] address comments Signed-off-by: Yan Zeng --- .../send_request_to_opensearch.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts index 3128c4ec339f..ad7ba440b6d4 100644 --- a/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts +++ b/src/plugins/console/public/application/hooks/use_send_current_request_to_opensearch/send_request_to_opensearch.ts @@ -108,6 +108,7 @@ export function sendRequestToOpenSearch( } const statusCode = httpResponse.response?.status; const isSuccess = + // Things like DELETE index where the index is not there are OK. statusCode && ((statusCode >= 200 && statusCode < 300) || statusCode === 404); if (isSuccess) { const contentType = httpResponse.response.headers.get('Content-Type') as BaseResponseType;