From 11ef6ce7202fd5122739209e8d1348148aa67e5f Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 7 Dec 2023 00:33:59 +0530 Subject: [PATCH 1/5] fix: Dev tools console autocomplete issue #5567 Signed-off-by: Kishor Rathva --- src/plugins/console/public/lib/mappings/mappings.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/console/public/lib/mappings/mappings.ts b/src/plugins/console/public/lib/mappings/mappings.ts index 2bd425ade95e..4bfefc79af83 100644 --- a/src/plugins/console/public/lib/mappings/mappings.ts +++ b/src/plugins/console/public/lib/mappings/mappings.ts @@ -385,7 +385,7 @@ const retrieveMappings = async (http: HttpSetup, settingsToRetrieve: any, dataSo }; const retrieveAliases = async (http: HttpSetup, settingsToRetrieve: any, dataSourceId: string) => { - const response = await retrieveSettings(http, 'fields', settingsToRetrieve, dataSourceId); + const response = await retrieveSettings(http, 'indices', settingsToRetrieve, dataSourceId); if (isHttpResponse(response) && response.body) { const aliases = response.body as IndexAliases; @@ -398,7 +398,7 @@ const retrieveTemplates = async ( settingsToRetrieve: any, dataSourceId: string ) => { - const response = await retrieveSettings(http, 'fields', settingsToRetrieve, dataSourceId); + const response = await retrieveSettings(http, 'templates', settingsToRetrieve, dataSourceId); if (isHttpResponse(response) && response.body) { const resTemplates = response.body; From 8ea5dda758de323398c4d9cbf28c113a6f6205b8 Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 7 Dec 2023 00:34:10 +0530 Subject: [PATCH 2/5] Added CHANGELOG Signed-off-by: Kishor Rathva --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 102205566e80..87aa8c065eac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -514,6 +514,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [VisBuilder] Fix Firefox legend selection issue ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [VisBuilder] Fix type errors ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [VisBuilder] Fix indexpattern selection in filter bar ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751)) +- [Console] Fix dev tool console autocomplete not loading issue for aliases ([#5568](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5568)) ### 🚞 Infrastructure From 8f3396ed7e0cdd2e938d4b7e3012ec8ceda035f1 Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 7 Dec 2023 00:34:13 +0530 Subject: [PATCH 3/5] Added test for retrieveAutoCompleteInfo Signed-off-by: Kishor Rathva --- .../send_request_to_opensearch.test.ts | 49 ++--------- .../lib/mappings/__tests__/mapping.test.ts | 81 +++++++++++++++++++ .../console/public/lib/mappings/mappings.ts | 24 +++--- .../lib/opensearch/http_response.mock.ts | 45 +++++++++++ 4 files changed, 143 insertions(+), 56 deletions(-) create mode 100644 src/plugins/console/public/lib/opensearch/http_response.mock.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 index fbeadf4cc7de..47a73e4af98a 100644 --- 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 @@ -9,53 +9,14 @@ * GitHub history for details. */ -import { - HttpFetchError, - HttpFetchOptionsWithPath, - HttpResponse, - HttpSetup, -} from '../../../../../../core/public'; +import { HttpFetchError, HttpSetup } from '../../../../../../core/public'; import { OpenSearchRequestArgs, sendRequestToOpenSearch } from './send_request_to_opensearch'; import * as opensearch from '../../../lib/opensearch/opensearch'; +import { + createMockHttpResponse, + createMockResponse, +} from '../../../lib/opensearch/http_response.mock'; -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(), diff --git a/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts b/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts index b5b1975a6a5c..fc977c32c7e5 100644 --- a/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts +++ b/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts @@ -30,7 +30,10 @@ import { Field } from '../mappings'; import '../../../application/models/sense_editor/sense_editor.test.mocks'; +import * as opensearch from '../../opensearch/opensearch'; import * as mappings from '../mappings'; +import { createMockHttpResponse } from '../../opensearch/http_response.mock'; +import { serviceContextMock } from '../../../application/contexts/services_context.mock'; describe('Mappings', () => { beforeEach(() => { @@ -291,3 +294,81 @@ describe('Mappings', () => { expect(mappings.getTypes()).toEqual(['properties']); }); }); +const tick = () => new Promise((res) => setImmediate(res)); + +export const advanceTimersByTime = async (time: number) => + jest.advanceTimersByTime(time) && (await tick()); + +export const runOnlyPendingTimers = async () => jest.runOnlyPendingTimers() && (await tick()); + +export const runAllTimers = async () => jest.runAllTimers() && (await tick()); + +describe('Auto Complete Info', () => { + let response = {}; + + beforeEach(() => { + mappings.clear(); + response = { + body: { + 'sample-ecommerce': { + mappings: { + properties: { + timestamp: { + type: 'date', + }, + }, + }, + }, + }, + }; + jest.resetAllMocks(); + jest.useFakeTimers(); // Enable automatic mocking of timers + }); + + afterEach(() => { + jest.clearAllMocks(); + jest.useRealTimers(); + }); + + test('Retrieve AutoComplete Info for Mappings, Aliases and Templates', async () => { + jest.useFakeTimers(); // Ensure fake timers are used + + const mockHttpResponse = createMockHttpResponse( + 200, + 'ok', + [['Content-Type', 'application/json, utf-8']], + response + ); + + const dataSourceId = 'mock-data-source-id'; + const sendSpy = jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + + const { + services: { http, settings: settingsService }, + } = serviceContextMock.create(); + + mappings.retrieveAutoCompleteInfo( + http, + settingsService, + { + fields: true, + indices: true, + templates: true, + }, + dataSourceId + ); + + // Fast-forward until all timers have been executed + jest.runAllTimers(); + + // Resolve the promise chain + await Promise.resolve(); + + expect(sendSpy).toHaveBeenCalledTimes(3); + + // Ensure send is called with different arguments + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_mapping', null, dataSourceId); + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_aliases', null, dataSourceId); + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_template', null, dataSourceId); + }); +}); diff --git a/src/plugins/console/public/lib/mappings/mappings.ts b/src/plugins/console/public/lib/mappings/mappings.ts index 4bfefc79af83..01c2c7d5b721 100644 --- a/src/plugins/console/public/lib/mappings/mappings.ts +++ b/src/plugins/console/public/lib/mappings/mappings.ts @@ -76,9 +76,15 @@ interface IndexAliases { type IndicesOrAliases = string | string[] | null; +interface SettingKeyToPathMap { + fields: '_mapping'; + indices: '_aliases'; + templates: '_template'; +} + // NOTE: If this value ever changes to be a few seconds or less, it might introduce flakiness // due to timing issues in our app.js tests. -const POLL_INTERVAL = 60000; +export const POLL_INTERVAL = 60000; let pollTimeoutId: NodeJS.Timeout | null; let perIndexTypes: { [index: string]: { [type: string]: Field[] } } = {}; @@ -316,11 +322,11 @@ export function clear() { function retrieveSettings( http: HttpSetup, - settingsKey: string, + settingsKey: keyof SettingKeyToPathMap, settingsToRetrieve: any, dataSourceId: string ): Promise> | Promise | Promise<{}> { - const settingKeyToPathMap: { [settingsKey: string]: string } = { + const settingKeyToPathMap: SettingKeyToPathMap = { fields: '_mapping', indices: '_aliases', templates: '_template', @@ -330,13 +336,8 @@ function retrieveSettings( if (settingsToRetrieve[settingsKey] === true) { return opensearch.send(http, 'GET', settingKeyToPathMap[settingsKey], null, dataSourceId); } else { - if (settingsToRetrieve[settingsKey] === false) { - // If the user doesn't want autocomplete suggestions, then clear any that exist - return Promise.resolve({}); - } else { - // If the user doesn't want autocomplete suggestions, then clear any that exist - return Promise.resolve(); - } + // If the user doesn't want autocomplete suggestions, then clear any that exist + return Promise.resolve(); } } @@ -418,12 +419,11 @@ export function retrieveAutoCompleteInfo( dataSourceId: string ) { clearSubscriptions(); - Promise.allSettled([ retrieveMappings(http, settingsToRetrieve, dataSourceId), retrieveAliases(http, settingsToRetrieve, dataSourceId), retrieveTemplates(http, settingsToRetrieve, dataSourceId), - ]).then(() => { + ]).then((res) => { // Schedule next request. pollTimeoutId = setTimeout(() => { // This looks strange/inefficient, but it ensures correct behavior because we don't want to send diff --git a/src/plugins/console/public/lib/opensearch/http_response.mock.ts b/src/plugins/console/public/lib/opensearch/http_response.mock.ts new file mode 100644 index 000000000000..b68ee9fb2d3e --- /dev/null +++ b/src/plugins/console/public/lib/opensearch/http_response.mock.ts @@ -0,0 +1,45 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { HttpFetchOptionsWithPath, HttpResponse } from '../../../../../core/public'; + +export 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(), + }; +}; + +export 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, + }; +}; From 310614fdbaa54062b2fae1dc2c6f10f6b8c5dceb Mon Sep 17 00:00:00 2001 From: Kishor Rathva Date: Thu, 7 Dec 2023 00:34:17 +0530 Subject: [PATCH 4/5] Refactored tests Signed-off-by: Kishor Rathva --- .../lib/mappings/__tests__/mapping.test.ts | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts b/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts index fc977c32c7e5..8e023df6c1ce 100644 --- a/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts +++ b/src/plugins/console/public/lib/mappings/__tests__/mapping.test.ts @@ -294,18 +294,17 @@ describe('Mappings', () => { expect(mappings.getTypes()).toEqual(['properties']); }); }); -const tick = () => new Promise((res) => setImmediate(res)); - -export const advanceTimersByTime = async (time: number) => - jest.advanceTimersByTime(time) && (await tick()); - -export const runOnlyPendingTimers = async () => jest.runOnlyPendingTimers() && (await tick()); - -export const runAllTimers = async () => jest.runAllTimers() && (await tick()); describe('Auto Complete Info', () => { let response = {}; + const mockHttpResponse = createMockHttpResponse( + 200, + 'ok', + [['Content-Type', 'application/json, utf-8']], + response + ); + beforeEach(() => { mappings.clear(); response = { @@ -331,15 +330,36 @@ describe('Auto Complete Info', () => { }); test('Retrieve AutoComplete Info for Mappings, Aliases and Templates', async () => { - jest.useFakeTimers(); // Ensure fake timers are used + const dataSourceId = 'mock-data-source-id'; + const sendSpy = jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); + + const { + services: { http, settings: settingsService }, + } = serviceContextMock.create(); - const mockHttpResponse = createMockHttpResponse( - 200, - 'ok', - [['Content-Type', 'application/json, utf-8']], - response + mappings.retrieveAutoCompleteInfo( + http, + settingsService, + { + fields: true, + indices: true, + templates: true, + }, + dataSourceId ); + // Fast-forward until all timers have been executed + jest.runAllTimers(); + + expect(sendSpy).toHaveBeenCalledTimes(3); + + // Ensure send is called with different arguments + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_mapping', null, dataSourceId); + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_aliases', null, dataSourceId); + expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_template', null, dataSourceId); + }); + + test('Retrieve AutoComplete Info for Specified Fields from the Settings', async () => { const dataSourceId = 'mock-data-source-id'; const sendSpy = jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse); @@ -352,8 +372,8 @@ describe('Auto Complete Info', () => { settingsService, { fields: true, - indices: true, - templates: true, + indices: false, + templates: false, }, dataSourceId ); @@ -364,11 +384,9 @@ describe('Auto Complete Info', () => { // Resolve the promise chain await Promise.resolve(); - expect(sendSpy).toHaveBeenCalledTimes(3); + expect(sendSpy).toHaveBeenCalledTimes(1); // Ensure send is called with different arguments expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_mapping', null, dataSourceId); - expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_aliases', null, dataSourceId); - expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_template', null, dataSourceId); }); }); From 2500f835e0be7ef40f56c2fe421248a282cfc51e Mon Sep 17 00:00:00 2001 From: kishor82 Date: Fri, 8 Dec 2023 12:27:48 +0530 Subject: [PATCH 5/5] Added suggested changes. Signed-off-by: Kishor Rathva --- .../console/public/lib/mappings/mappings.ts | 24 +++++++------------ .../lib/opensearch/http_response.mock.ts | 1 - 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/plugins/console/public/lib/mappings/mappings.ts b/src/plugins/console/public/lib/mappings/mappings.ts index 01c2c7d5b721..4cc188a9875f 100644 --- a/src/plugins/console/public/lib/mappings/mappings.ts +++ b/src/plugins/console/public/lib/mappings/mappings.ts @@ -76,11 +76,11 @@ interface IndexAliases { type IndicesOrAliases = string | string[] | null; -interface SettingKeyToPathMap { - fields: '_mapping'; - indices: '_aliases'; - templates: '_template'; -} +const SETTING_KEY_TO_PATH_MAP = { + fields: '_mapping', + indices: '_aliases', + templates: '_template', +}; // NOTE: If this value ever changes to be a few seconds or less, it might introduce flakiness // due to timing issues in our app.js tests. @@ -322,19 +322,13 @@ export function clear() { function retrieveSettings( http: HttpSetup, - settingsKey: keyof SettingKeyToPathMap, + settingsKey: keyof typeof SETTING_KEY_TO_PATH_MAP, settingsToRetrieve: any, dataSourceId: string -): Promise> | Promise | Promise<{}> { - const settingKeyToPathMap: SettingKeyToPathMap = { - fields: '_mapping', - indices: '_aliases', - templates: '_template', - }; - +): Promise> | Promise { // Fetch autocomplete info if setting is set to true, and if user has made changes. if (settingsToRetrieve[settingsKey] === true) { - return opensearch.send(http, 'GET', settingKeyToPathMap[settingsKey], null, dataSourceId); + return opensearch.send(http, 'GET', SETTING_KEY_TO_PATH_MAP[settingsKey], null, dataSourceId); } else { // If the user doesn't want autocomplete suggestions, then clear any that exist return Promise.resolve(); @@ -423,7 +417,7 @@ export function retrieveAutoCompleteInfo( retrieveMappings(http, settingsToRetrieve, dataSourceId), retrieveAliases(http, settingsToRetrieve, dataSourceId), retrieveTemplates(http, settingsToRetrieve, dataSourceId), - ]).then((res) => { + ]).then(() => { // Schedule next request. pollTimeoutId = setTimeout(() => { // This looks strange/inefficient, but it ensures correct behavior because we don't want to send diff --git a/src/plugins/console/public/lib/opensearch/http_response.mock.ts b/src/plugins/console/public/lib/opensearch/http_response.mock.ts index b68ee9fb2d3e..3ee40be979ab 100644 --- a/src/plugins/console/public/lib/opensearch/http_response.mock.ts +++ b/src/plugins/console/public/lib/opensearch/http_response.mock.ts @@ -11,7 +11,6 @@ export const createMockResponse = ( headers: Array<[string, string]> ): Response => { return { - // headers: {} as Headers, headers: new Headers(headers), ok: true, redirected: false,