Skip to content

Commit

Permalink
[data.search.SearchSource] Unify FetchHandler types & add onResponse/…
Browse files Browse the repository at this point in the history
…callMsearch. (#77430)
  • Loading branch information
lukeelmers committed Sep 15, 2020
1 parent 98113ee commit e6d6751
Show file tree
Hide file tree
Showing 16 changed files with 143 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ Using `createSearchSource`<!-- -->, the instance can be re-created.
```typescript
serialize(): {
searchSourceJSON: string;
references: import("../../../../../core/public").SavedObjectReference[];
references: import("../../../../../core/types").SavedObjectReference[];
};
```
<b>Returns:</b>

`{
searchSourceJSON: string;
references: import("../../../../../core/public").SavedObjectReference[];
references: import("../../../../../core/types").SavedObjectReference[];
}`

3 changes: 1 addition & 2 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { ExpressionAstFunction } from 'src/plugins/expressions/common';
import { ExpressionsSetup } from 'src/plugins/expressions/public';
import { History } from 'history';
import { Href } from 'history';
import { HttpStart } from 'src/core/public';
import { IconType } from '@elastic/eui';
import { InjectedIntl } from '@kbn/i18n/react';
import { ISearchOptions as ISearchOptions_2 } from 'src/plugins/data/public';
Expand Down Expand Up @@ -2018,7 +2017,7 @@ export class SearchSource {
onRequestStart(handler: (searchSource: SearchSource, options?: ISearchOptions) => Promise<unknown>): void;
serialize(): {
searchSourceJSON: string;
references: import("../../../../../core/public").SavedObjectReference[];
references: import("../../../../../core/types").SavedObjectReference[];
};
setField<K extends keyof SearchSourceFields>(field: K, value: SearchSourceFields[K]): this;
setFields(newFields: SearchSourceFields): this;
Expand Down
18 changes: 13 additions & 5 deletions src/plugins/data/public/search/fetch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
* under the License.
*/

import { HttpStart } from 'src/core/public';
import { BehaviorSubject } from 'rxjs';
import { SearchResponse } from 'elasticsearch';
import { GetConfigFn } from '../../../common';
import { LegacyFetchHandlers } from '../legacy/types';

/**
* @internal
Expand All @@ -31,9 +31,17 @@ import { GetConfigFn } from '../../../common';
export type SearchRequest = Record<string, any>;

export interface FetchHandlers {
config: { get: GetConfigFn };
http: HttpStart;
loadingCount$: BehaviorSubject<number>;
getConfig: GetConfigFn;
/**
* Callback which can be used to hook into responses, modify them, or perform
* side effects like displaying UI errors on the client.
*/
onResponse: (request: SearchRequest, response: SearchResponse<any>) => SearchResponse<any>;
/**
* These handlers are only used by the legacy defaultSearchStrategy and can be removed
* once that strategy has been deprecated.
*/
legacy: LegacyFetchHandlers;
}

export interface SearchError {
Expand Down
33 changes: 16 additions & 17 deletions src/plugins/data/public/search/legacy/call_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,13 @@
* under the License.
*/

import { coreMock } from '../../../../../core/public/mocks';
import { callClient } from './call_client';
import { SearchStrategySearchParams } from './types';
import { defaultSearchStrategy } from './default_search_strategy';
import { FetchHandlers } from '../fetch';
import { handleResponse } from '../fetch/handle_response';
import { BehaviorSubject } from 'rxjs';

const mockAbortFn = jest.fn();
jest.mock('../fetch/handle_response', () => ({
handleResponse: jest.fn((request, response) => response),
}));

jest.mock('./default_search_strategy', () => {
return {
Expand All @@ -50,32 +45,36 @@ jest.mock('./default_search_strategy', () => {
});

describe('callClient', () => {
const handleResponse = jest.fn().mockImplementation((req, res) => res);
const handlers = {
getConfig: jest.fn(),
onResponse: handleResponse,
legacy: {
callMsearch: jest.fn(),
loadingCount$: new BehaviorSubject(0),
},
} as FetchHandlers;

beforeEach(() => {
(handleResponse as jest.Mock).mockClear();
handleResponse.mockClear();
});

test('Passes the additional arguments it is given to the search strategy', () => {
const searchRequests = [{ _searchStrategyId: 0 }];
const args = {
http: coreMock.createStart().http,
legacySearchService: {},
config: { get: jest.fn() },
loadingCount$: new BehaviorSubject(0),
} as FetchHandlers;

callClient(searchRequests, [], args);
callClient(searchRequests, [], handlers);

expect(defaultSearchStrategy.search).toBeCalled();
expect((defaultSearchStrategy.search as any).mock.calls[0][0]).toEqual({
searchRequests,
...args,
...handlers,
});
});

test('Returns the responses in the original order', async () => {
const searchRequests = [{ _searchStrategyId: 1 }, { _searchStrategyId: 0 }];

const responses = await Promise.all(callClient(searchRequests, [], {} as FetchHandlers));
const responses = await Promise.all(callClient(searchRequests, [], handlers));

expect(responses[0]).toEqual({ id: searchRequests[0]._searchStrategyId });
expect(responses[1]).toEqual({ id: searchRequests[1]._searchStrategyId });
Expand All @@ -84,7 +83,7 @@ describe('callClient', () => {
test('Calls handleResponse with each request and response', async () => {
const searchRequests = [{ _searchStrategyId: 0 }, { _searchStrategyId: 1 }];

const responses = callClient(searchRequests, [], {} as FetchHandlers);
const responses = callClient(searchRequests, [], handlers);
await Promise.all(responses);

expect(handleResponse).toBeCalledTimes(2);
Expand All @@ -105,7 +104,7 @@ describe('callClient', () => {
},
];

callClient(searchRequests, requestOptions, {} as FetchHandlers);
callClient(searchRequests, requestOptions, handlers);
abortController.abort();

expect(mockAbortFn).toBeCalled();
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data/public/search/legacy/call_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { SearchResponse } from 'elasticsearch';
import { ISearchOptions } from 'src/plugins/data/common';
import { FetchHandlers, handleResponse } from '../fetch';
import { FetchHandlers } from '../fetch';
import { defaultSearchStrategy } from './default_search_strategy';
import { SearchRequest } from '../index';

Expand All @@ -42,7 +42,7 @@ export function callClient(
});

searchRequests.forEach((request, i) => {
const response = searching.then((results) => handleResponse(request, results[i]));
const response = searching.then((results) => fetchHandlers.onResponse(request, results[i]));
const { abortSignal = null } = requestOptionsMap.get(request) || {};
if (abortSignal) abortSignal.addEventListener('abort', abort);
requestResponseMap.set(request, response);
Expand Down
37 changes: 37 additions & 0 deletions src/plugins/data/public/search/legacy/call_msearch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { HttpStart } from 'src/core/public';
import { LegacyFetchHandlers } from './types';

/**
* Wrapper for calling the internal msearch endpoint from the client.
* This is needed to abstract away differences in the http service
* between client & server.
*
* @internal
*/
export function getCallMsearch({ http }: { http: HttpStart }): LegacyFetchHandlers['callMsearch'] {
return async ({ body, signal }) => {
return http.post('/internal/_msearch', {
body: JSON.stringify(body),
signal,
});
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

import { HttpStart } from 'src/core/public';
import { coreMock } from '../../../../../core/public/mocks';
import { getCallMsearch } from './call_msearch';
import { defaultSearchStrategy } from './default_search_strategy';
import { SearchStrategySearchParams } from './types';
import { LegacyFetchHandlers, SearchStrategySearchParams } from './types';
import { BehaviorSubject } from 'rxjs';

const { search } = defaultSearchStrategy;
Expand All @@ -44,11 +45,12 @@ describe('defaultSearchStrategy', function () {
index: { title: 'foo' },
},
],
http,
config: {
get: jest.fn(),
},
loadingCount$: new BehaviorSubject(0) as any,
getConfig: jest.fn(),
onResponse: (req, res) => res,
legacy: {
callMsearch: getCallMsearch({ http }),
loadingCount$: new BehaviorSubject(0) as any,
} as jest.Mocked<LegacyFetchHandlers>,
};
});

Expand Down
17 changes: 9 additions & 8 deletions src/plugins/data/public/search/legacy/default_search_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ export const defaultSearchStrategy: SearchStrategyProvider = {
},
};

function msearch({ searchRequests, config, http, loadingCount$ }: SearchStrategySearchParams) {
function msearch({ searchRequests, getConfig, legacy }: SearchStrategySearchParams) {
const { callMsearch, loadingCount$ } = legacy;

const requests = searchRequests.map(({ index, body }) => {
return {
header: {
index: index.title || index,
preference: getPreference(config.get),
preference: getPreference(getConfig),
},
body,
};
Expand All @@ -55,12 +57,11 @@ function msearch({ searchRequests, config, http, loadingCount$ }: SearchStrategy
}
};

const searching = http
.post('/internal/_msearch', {
body: JSON.stringify({ searches: requests }),
signal: abortController.signal,
})
.then(({ body }) => body?.responses)
const searching = callMsearch({
body: { searches: requests },
signal: abortController.signal,
})
.then((res: any) => res?.body?.responses)
.finally(() => cleanup());

return {
Expand Down
32 changes: 11 additions & 21 deletions src/plugins/data/public/search/legacy/fetch_soon.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,21 @@ describe('fetchSoon', () => {
});

test('should execute asap if config is set to not batch searches', () => {
const config = {
get: getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: false }),
};
const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: false });
const request = {};
const options = {};

fetchSoon(request, options, { config } as FetchHandlers);
fetchSoon(request, options, { getConfig } as FetchHandlers);

expect(callClient).toBeCalled();
});

test('should delay by 50ms if config is set to batch searches', () => {
const config = {
get: getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true }),
};
const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
const request = {};
const options = {};

fetchSoon(request, options, { config } as FetchHandlers);
fetchSoon(request, options, { getConfig } as FetchHandlers);

expect(callClient).not.toBeCalled();
jest.advanceTimersByTime(0);
Expand All @@ -95,14 +91,12 @@ describe('fetchSoon', () => {
});

test('should send a batch of requests to callClient', () => {
const config = {
get: getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true }),
};
const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
const requests = [{ foo: 1 }, { foo: 2 }];
const options = [{ bar: 1 }, { bar: 2 }];

requests.forEach((request, i) => {
fetchSoon(request, options[i] as ISearchOptions, { config } as FetchHandlers);
fetchSoon(request, options[i] as ISearchOptions, { getConfig } as FetchHandlers);
});

jest.advanceTimersByTime(50);
Expand All @@ -112,13 +106,11 @@ describe('fetchSoon', () => {
});

test('should return the response to the corresponding call for multiple batched requests', async () => {
const config = {
get: getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true }),
};
const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
const requests = [{ _mockResponseId: 'foo' }, { _mockResponseId: 'bar' }];

const promises = requests.map((request) => {
return fetchSoon(request, {}, { config } as FetchHandlers);
return fetchSoon(request, {}, { getConfig } as FetchHandlers);
});
jest.advanceTimersByTime(50);
const results = await Promise.all(promises);
Expand All @@ -127,18 +119,16 @@ describe('fetchSoon', () => {
});

test('should wait for the previous batch to start before starting a new batch', () => {
const config = {
get: getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true }),
};
const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
const firstBatch = [{ foo: 1 }, { foo: 2 }];
const secondBatch = [{ bar: 1 }, { bar: 2 }];

firstBatch.forEach((request) => {
fetchSoon(request, {}, { config } as FetchHandlers);
fetchSoon(request, {}, { getConfig } as FetchHandlers);
});
jest.advanceTimersByTime(50);
secondBatch.forEach((request) => {
fetchSoon(request, {}, { config } as FetchHandlers);
fetchSoon(request, {}, { getConfig } as FetchHandlers);
});

expect(callClient).toBeCalledTimes(1);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/search/legacy/fetch_soon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function fetchSoon(
options: ISearchOptions,
fetchHandlers: FetchHandlers
) {
const msToDelay = fetchHandlers.config.get(UI_SETTINGS.COURIER_BATCH_SEARCHES) ? 50 : 0;
const msToDelay = fetchHandlers.getConfig(UI_SETTINGS.COURIER_BATCH_SEARCHES) ? 50 : 0;
return delayedFetch(request, options, fetchHandlers, msToDelay);
}

Expand Down
10 changes: 10 additions & 0 deletions src/plugins/data/public/search/legacy/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,20 @@
* under the License.
*/

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

// @internal
export interface LegacyFetchHandlers {
callMsearch: (params: {
body: SearchRequest;
signal: AbortSignal;
}) => Promise<Array<SearchResponse<any>>>;
loadingCount$: BehaviorSubject<number>;
}

export interface SearchStrategySearchParams extends FetchHandlers {
searchRequests: SearchRequest[];
}
Expand Down
Loading

0 comments on commit e6d6751

Please sign in to comment.