Skip to content

Commit

Permalink
[Search] Cleanup SearchRequest and SearchResponse types (#75471)
Browse files Browse the repository at this point in the history
* improve test stability

* Replace SearchRequest = any with Record<string, any>

* Remove SearchResponse = any from data plugin

* docs

* logs

* Revert "Replace SearchRequest = any with Record<string, any>"

This reverts commit 9914ab5.

* code review

* list control

* null check

* null null null

* Jest fix
  • Loading branch information
lizozom authored Aug 23, 2020
1 parent 6dbc4be commit bdb73b5
Show file tree
Hide file tree
Showing 28 changed files with 125 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@
| [RangeFilterMeta](./kibana-plugin-plugins-data-public.rangefiltermeta.md) | |
| [SavedQueryTimeFilter](./kibana-plugin-plugins-data-public.savedquerytimefilter.md) | |
| [SearchBarProps](./kibana-plugin-plugins-data-public.searchbarprops.md) | |
| [SearchRequest](./kibana-plugin-plugins-data-public.searchrequest.md) | |
| [SearchResponse](./kibana-plugin-plugins-data-public.searchresponse.md) | |
| [StatefulSearchBarProps](./kibana-plugin-plugins-data-public.statefulsearchbarprops.md) | |
| [TabbedAggRow](./kibana-plugin-plugins-data-public.tabbedaggrow.md) | \* |
| [TimefilterContract](./kibana-plugin-plugins-data-public.timefiltercontract.md) | |
Expand Down

This file was deleted.

This file was deleted.

4 changes: 2 additions & 2 deletions src/plugins/data/common/search/es_search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import { IKibanaSearchRequest, IKibanaSearchResponse } from '../types';

export const ES_SEARCH_STRATEGY = 'es';

export type ISearchRequestParams = {
export type ISearchRequestParams<T = Record<string, any>> = {
trackTotalHits?: boolean;
} & Search;
} & Search<T>;

export interface IEsSearchRequest extends IKibanaSearchRequest {
params?: ISearchRequestParams;
Expand Down
1 change: 0 additions & 1 deletion src/plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ export {
SearchInterceptor,
SearchInterceptorDeps,
SearchRequest,
SearchResponse,
SearchSourceFields,
SortDirection,
// expression functions and types
Expand Down
46 changes: 20 additions & 26 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ import { SavedObject } from 'src/core/server';
import { SavedObject as SavedObject_3 } from 'src/core/public';
import { SavedObjectsClientContract } from 'src/core/public';
import { Search } from '@elastic/elasticsearch/api/requestParams';
import { SearchResponse as SearchResponse_2 } from 'elasticsearch';
import { SearchResponse } from 'elasticsearch';
import { SerializedFieldFormat as SerializedFieldFormat_2 } from 'src/plugins/expressions/common';
import { Subscription } from 'rxjs';
import { Toast } from 'kibana/public';
Expand Down Expand Up @@ -727,6 +727,7 @@ export function getEsPreference(uiSettings: IUiSettingsClient_2, sessionId?: str
export const getKbnTypeNames: () => string[];

// Warning: (ae-forgotten-export) The symbol "ISearchRequestParams" needs to be exported by the entry point index.d.ts
// Warning: (ae-incompatible-release-tags) The symbol "getSearchParamsFromRequest" is marked as @public, but its signature references "SearchRequest" which is marked as @internal
//
// @public (undocumented)
export function getSearchParamsFromRequest(searchRequest: SearchRequest, dependencies: {
Expand Down Expand Up @@ -795,7 +796,7 @@ export interface IEsSearchResponse<Source = any> extends IKibanaSearchResponse {
isPartial?: boolean;
isRunning?: boolean;
// (undocumented)
rawResponse: SearchResponse_2<Source>;
rawResponse: SearchResponse<Source>;
}

// Warning: (ae-missing-release-tag) "IFieldFormat" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
Expand Down Expand Up @@ -1783,15 +1784,8 @@ export interface SearchInterceptorDeps {
usageCollector?: SearchUsageCollector;
}

// Warning: (ae-missing-release-tag) "SearchRequest" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export type SearchRequest = any;

// Warning: (ae-missing-release-tag) "SearchResponse" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export type SearchResponse = any;
// @internal
export type SearchRequest = Record<string, any>;

// Warning: (ae-missing-release-tag) "SearchSourceFields" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
Expand Down Expand Up @@ -1991,21 +1985,21 @@ export const UI_SETTINGS: {
// src/plugins/data/public/index.ts:234:27 - (ae-forgotten-export) The symbol "getFromSavedObject" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:234:27 - (ae-forgotten-export) The symbol "flattenHitWrapper" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:234:27 - (ae-forgotten-export) The symbol "formatHitProvider" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:372:20 - (ae-forgotten-export) The symbol "getRequestInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:372:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:372:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:372:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:374:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:375:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:384:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:385:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:386:1 - (ae-forgotten-export) The symbol "Ipv4Address" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:387:1 - (ae-forgotten-export) The symbol "isDateHistogramBucketAggConfig" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:391:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:392:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:395:1 - (ae-forgotten-export) The symbol "parseInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:396:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:399:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:371:20 - (ae-forgotten-export) The symbol "getRequestInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:371:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:371:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:371:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:373:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:374:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:383:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:384:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:385:1 - (ae-forgotten-export) The symbol "Ipv4Address" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:386:1 - (ae-forgotten-export) The symbol "isDateHistogramBucketAggConfig" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:390:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:391:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:394:1 - (ae-forgotten-export) The symbol "parseInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:395:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:398:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/query/state_sync/connect_to_query_state.ts:45:5 - (ae-forgotten-export) The symbol "FilterStateStore" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/types.ts:62:5 - (ae-forgotten-export) The symbol "createFiltersFromValueClickAction" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/types.ts:63:5 - (ae-forgotten-export) The symbol "createFiltersFromRangeSelectAction" needs to be exported by the entry point index.d.ts
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/data/public/search/fetch/handle_response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { handleResponse } from './handle_response';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { notificationServiceMock } from '../../../../../core/public/notifications/notifications_service.mock';
import { setNotifications } from '../../services';
import { SearchResponse } from 'elasticsearch';

jest.mock('@kbn/i18n', () => {
return {
Expand All @@ -44,7 +45,7 @@ describe('handleResponse', () => {
const request = { body: {} };
const response = {
timed_out: true,
};
} as SearchResponse<any>;
const result = handleResponse(request, response);
expect(result).toBe(response);
expect(notifications.toasts.addWarning).toBeCalled();
Expand All @@ -57,9 +58,12 @@ describe('handleResponse', () => {
const request = { body: {} };
const response = {
_shards: {
failed: true,
failed: 1,
total: 2,
successful: 1,
skipped: 1,
},
};
} as SearchResponse<any>;
const result = handleResponse(request, response);
expect(result).toBe(response);
expect(notifications.toasts.addWarning).toBeCalled();
Expand All @@ -70,7 +74,7 @@ describe('handleResponse', () => {

test('returns the response', () => {
const request = {};
const response = {};
const response = {} as SearchResponse<any>;
const result = handleResponse(request, response);
expect(result).toBe(response);
});
Expand Down
13 changes: 5 additions & 8 deletions src/plugins/data/public/search/fetch/handle_response.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@
import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiSpacer } from '@elastic/eui';
import { ShardFailureOpenModalButton, ShardFailureRequest, ShardFailureResponse } from '../../ui';
import { SearchResponse } from 'elasticsearch';
import { ShardFailureOpenModalButton } from '../../ui';
import { toMountPoint } from '../../../../kibana_react/public';
import { getNotifications } from '../../services';
import { SearchRequest, SearchResponse } from '..';
import { SearchRequest } from '..';

export function handleResponse(request: SearchRequest, response: SearchResponse) {
export function handleResponse(request: SearchRequest, response: SearchResponse<any>) {
if (response.timed_out) {
getNotifications().toasts.addWarning({
title: i18n.translate('data.search.searchSource.fetch.requestTimedOutNotificationMessage', {
Expand Down Expand Up @@ -53,11 +54,7 @@ export function handleResponse(request: SearchRequest, response: SearchResponse)
<>
{description}
<EuiSpacer size="s" />
<ShardFailureOpenModalButton
request={request.body as ShardFailureRequest}
response={response as ShardFailureResponse}
title={title}
/>
<ShardFailureOpenModalButton request={request.body} response={response} title={title} />
</>
);

Expand Down
7 changes: 4 additions & 3 deletions src/plugins/data/public/search/fetch/request_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@
* under the License.
*/

import { SearchResponse } from 'elasticsearch';
import { KbnError } from '../../../../kibana_utils/common';
import { SearchError, SearchResponse } from './types';
import { SearchError } from './types';

/**
* Request Failure - When an entire multi request fails
* @param {Error} err - the Error that came back
* @param {Object} resp - optional HTTP response
*/
export class RequestFailure extends KbnError {
public resp: SearchResponse;
constructor(err: SearchError | null = null, resp?: SearchResponse) {
public resp?: SearchResponse<any>;
constructor(err: SearchError | null = null, resp?: SearchResponse<any>) {
super(`Request to Elasticsearch failed: ${JSON.stringify(resp || err?.message)}`);

this.resp = resp;
Expand Down
10 changes: 8 additions & 2 deletions src/plugins/data/public/search/fetch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@
import { GetConfigFn } from '../../../common';
import { ISearchStartLegacy } from '../types';

export type SearchRequest = any;
export type SearchResponse = any;
/**
* @internal
*
* This type is used when flattenning a SearchSource and passing it down to legacy search.
* Once legacy search is removed, this type should become internal to `SearchSource`,
* where `ISearchRequestParams` is used externally instead.
*/
export type SearchRequest = Record<string, any>;

export interface FetchOptions {
abortSignal?: AbortSignal;
Expand Down
8 changes: 1 addition & 7 deletions src/plugins/data/public/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ export { getEsPreference } from './es_search';

export { IKibanaSearchResponse, IKibanaSearchRequest } from '../../common/search';

export {
SearchError,
FetchOptions,
SearchRequest,
SearchResponse,
getSearchParamsFromRequest,
} from './fetch';
export { SearchError, FetchOptions, getSearchParamsFromRequest, SearchRequest } from './fetch';

export {
ISearchSource,
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/data/public/search/legacy/call_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

import { SearchResponse } from 'elasticsearch';
import { FetchOptions, FetchHandlers, handleResponse } from '../fetch';
import { defaultSearchStrategy } from './default_search_strategy';
import { SearchRequest } from '../index';
Expand All @@ -32,7 +33,7 @@ export function callClient(
FetchOptions
]> = searchRequests.map((request, i) => [request, requestsOptions[i]]);
const requestOptionsMap = new Map<SearchRequest, FetchOptions>(requestOptionEntries);
const requestResponseMap = new Map();
const requestResponseMap = new Map<SearchRequest, Promise<SearchResponse<any>>>();

const { searching, abort } = defaultSearchStrategy.search({
searchRequests,
Expand All @@ -45,5 +46,5 @@ export function callClient(
if (abortSignal) abortSignal.addEventListener('abort', abort);
requestResponseMap.set(request, response);
});
return searchRequests.map((request) => requestResponseMap.get(request));
return searchRequests.map((request) => requestResponseMap.get(request)!);
}
5 changes: 3 additions & 2 deletions src/plugins/data/public/search/legacy/es_client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
* under the License.
*/

import { SearchRequest, SearchResponse } from '../../fetch';
import { SearchResponse } from 'elasticsearch';
import { SearchRequest } from '../../fetch';

export interface LegacyApiCaller {
search: (searchRequest: SearchRequest) => LegacyApiCallerResponse;
msearch: (searchRequest: SearchRequest) => LegacyApiCallerResponse;
}

interface LegacyApiCallerResponse extends Promise<SearchResponse> {
interface LegacyApiCallerResponse extends Promise<SearchResponse<any>> {
abort: () => void;
}
20 changes: 15 additions & 5 deletions src/plugins/data/public/search/legacy/fetch_soon.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,27 @@
import { fetchSoon } from './fetch_soon';
import { callClient } from './call_client';
import { FetchHandlers, FetchOptions } from '../fetch/types';
import { SearchRequest, SearchResponse } from '../index';
import { SearchRequest } from '../index';
import { SearchResponse } from 'elasticsearch';
import { GetConfigFn, UI_SETTINGS } from '../../../common';

function getConfigStub(config: any = {}): GetConfigFn {
return (key) => config[key];
}

const mockResponses: Record<string, SearchResponse> = {
foo: {},
bar: {},
baz: {},
const mockResponses: Record<string, SearchResponse<any>> = {
foo: {
took: 1,
timed_out: false,
} as SearchResponse<any>,
bar: {
took: 2,
timed_out: false,
} as SearchResponse<any>,
baz: {
took: 3,
timed_out: false,
} as SearchResponse<any>,
};

jest.useFakeTimers();
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/data/public/search/legacy/fetch_soon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
* under the License.
*/

import { SearchResponse } from 'elasticsearch';
import { callClient } from './call_client';
import { FetchHandlers, FetchOptions } from '../fetch/types';
import { SearchRequest, SearchResponse } from '../index';
import { SearchRequest } from '../index';
import { UI_SETTINGS } from '../../../common';

/**
Expand Down Expand Up @@ -53,7 +54,7 @@ let requestsToFetch: SearchRequest[] = [];
let requestOptions: FetchOptions[] = [];

// The in-progress fetch (if there is one)
let fetchInProgress: Promise<SearchResponse> | null = null;
let fetchInProgress: any = null;

/**
* Delay fetching for a given amount of time, while batching up the requests to be fetched.
Expand All @@ -67,15 +68,18 @@ async function delayedFetch(
options: FetchOptions,
fetchHandlers: FetchHandlers,
ms: number
) {
): Promise<SearchResponse<any>> {
if (ms === 0) {
return callClient([request], [options], fetchHandlers)[0];
}

const i = requestsToFetch.length;
requestsToFetch = [...requestsToFetch, request];
requestOptions = [...requestOptions, options];
const responses: SearchResponse[] = await (fetchInProgress =

// Note: the typescript here only worked because `SearchResponse` was `any`
// Since this code is legacy, I'm leaving the any here.
const responses: any[] = await (fetchInProgress =
fetchInProgress ||
delay(() => {
const response = callClient(requestsToFetch, requestOptions, fetchHandlers);
Expand Down
7 changes: 4 additions & 3 deletions src/plugins/data/public/search/legacy/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { SearchResponse } from 'elasticsearch';
import { FetchHandlers } from '../fetch';
import { SearchRequest, SearchResponse } from '..';
import { SearchRequest } from '..';

export interface SearchStrategySearchParams extends FetchHandlers {
searchRequests: SearchRequest[];
Expand All @@ -30,7 +31,7 @@ export interface SearchStrategyProvider {
search: (params: SearchStrategySearchParams) => SearchStrategyResponse;
}

export interface SearchStrategyResponse {
searching: Promise<SearchResponse[]>;
export interface SearchStrategyResponse<T = any> {
searching: Promise<Array<SearchResponse<T>>>;
abort: () => void;
}
Loading

0 comments on commit bdb73b5

Please sign in to comment.