Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update console to use core.http instead of jQuery.ajax #3080

Merged
merged 4 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -43,6 +43,7 @@ interface ContextServices {
objectStorageClient: ObjectStorageClient;
trackUiMetric: MetricsTracker;
opensearchHostService: OpenSearchHostService;
http: HttpSetup;
}

export interface ContextValue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,6 +37,7 @@ import * as opensearch from '../../../lib/opensearch/opensearch';
import { BaseResponseType } from '../../../types';

export interface OpenSearchRequestArgs {
http: HttpSetup;
requests: any;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't statusCode >= 200 imply that we'll consider all 3xx, 4xx, 5xx status codes to be successful, too? The previous implementation was >= 200 && <300

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, it should be >= 200 && <300, will fix it and add a few more test cases in next revision

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);
kristenTian marked this conversation as resolved.
Show resolved Hide resolved
} 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 {
Expand Down Expand Up @@ -112,5 +112,5 @@ export const useSendCurrentRequestToOpenSearch = () => {
});
}
}
}, [dispatch, settings, history, notifications, trackUiMetric]);
}, [dispatch, settings, history, notifications, trackUiMetric, http]);
};
1 change: 1 addition & 0 deletions src/plugins/console/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export function renderApp({
notifications,
trackUiMetric,
objectStorageClient,
http,
},
}}
>
Expand Down
45 changes: 15 additions & 30 deletions src/plugins/console/public/lib/opensearch/opensearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];

Expand All @@ -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<HttpResponse> {
return await http.post<HttpResponse>('/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) {
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/console/_console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down