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

fix: Dev tools console autocomplete issue #5567 #5568

Merged
merged 5 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<any> => {
return {
fetchOptions: (jest.fn() as unknown) as Readonly<HttpFetchOptionsWithPath>,
request: (jest.fn() as unknown) as Readonly<Request>,
response: createMockResponse(statusCode, statusText, headers),
body,
};
};
const dummyArgs: OpenSearchRequestArgs = {
http: ({
post: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -291,3 +294,99 @@ describe('Mappings', () => {
expect(mappings.getTypes()).toEqual(['properties']);
});
});

describe('Auto Complete Info', () => {
let response = {};

const mockHttpResponse = createMockHttpResponse(
200,
'ok',
[['Content-Type', 'application/json, utf-8']],
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 () => {
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();

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);

const {
services: { http, settings: settingsService },
} = serviceContextMock.create();

mappings.retrieveAutoCompleteInfo(
http,
settingsService,
{
fields: true,
indices: false,
templates: false,
},
dataSourceId
);

// Fast-forward until all timers have been executed
jest.runAllTimers();

// Resolve the promise chain
await Promise.resolve();

expect(sendSpy).toHaveBeenCalledTimes(1);

// Ensure send is called with different arguments
expect(sendSpy).toHaveBeenCalledWith(http, 'GET', '_mapping', null, dataSourceId);
});
});
34 changes: 14 additions & 20 deletions src/plugins/console/public/lib/mappings/mappings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,15 @@ interface IndexAliases {

type IndicesOrAliases = string | string[] | null;

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.
const POLL_INTERVAL = 60000;
export const POLL_INTERVAL = 60000;
let pollTimeoutId: NodeJS.Timeout | null;

let perIndexTypes: { [index: string]: { [type: string]: Field[] } } = {};
Expand Down Expand Up @@ -316,27 +322,16 @@ export function clear() {

function retrieveSettings(
http: HttpSetup,
settingsKey: string,
settingsKey: keyof typeof SETTING_KEY_TO_PATH_MAP,
settingsToRetrieve: any,
dataSourceId: string
): Promise<HttpResponse<any>> | Promise<void> | Promise<{}> {
const settingKeyToPathMap: { [settingsKey: string]: string } = {
fields: '_mapping',
indices: '_aliases',
templates: '_template',
};

): Promise<HttpResponse<any>> | Promise<void> {
// 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 (settingsToRetrieve[settingsKey] === false) {
// If the user doesn't want autocomplete suggestions, then clear any that exist
return Promise.resolve({});
Copy link
Member

Choose a reason for hiding this comment

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

Why did we want to change this response shape?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check settingsToRetrieve[settingsKey] === false is unnecessary, so I have removed it. Additionally, in the else condition, the response body doesn't matter, whether it's undefined or {}, as long as it's not an instance of HttpResponse.

} 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();
}
}

Expand Down Expand Up @@ -385,7 +380,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;
Expand All @@ -398,7 +393,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;
Expand All @@ -418,7 +413,6 @@ export function retrieveAutoCompleteInfo(
dataSourceId: string
) {
clearSubscriptions();

Promise.allSettled([
retrieveMappings(http, settingsToRetrieve, dataSourceId),
retrieveAliases(http, settingsToRetrieve, dataSourceId),
Expand Down
44 changes: 44 additions & 0 deletions src/plugins/console/public/lib/opensearch/http_response.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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: 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<any> => {
return {
fetchOptions: (jest.fn() as unknown) as Readonly<HttpFetchOptionsWithPath>,
request: (jest.fn() as unknown) as Readonly<Request>,
response: createMockResponse(statusCode, statusText, headers),
body,
};
};
Loading