Skip to content

Commit 4c3d67c

Browse files
Merge branch 'master' into fix/load-all-ui-shared-deps-css
2 parents d776d51 + 2501919 commit 4c3d67c

26 files changed

Lines changed: 440 additions & 93 deletions
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import Path from 'path';
21+
22+
import jestDiff from 'jest-diff';
23+
import { REPO_ROOT, createAbsolutePathSerializer } from '@kbn/dev-utils';
24+
25+
import { reformatJestDiff, getOptimizerCacheKey, diffCacheKey } from './cache_keys';
26+
import { OptimizerConfig } from './optimizer_config';
27+
28+
jest.mock('./get_changes.ts', () => ({
29+
getChanges: async () =>
30+
new Map([
31+
['/foo/bar/a', 'modified'],
32+
['/foo/bar/b', 'modified'],
33+
['/foo/bar/c', 'deleted'],
34+
]),
35+
}));
36+
37+
jest.mock('./get_mtimes.ts', () => ({
38+
getMtimes: async (paths: string[]) => new Map(paths.map(path => [path, 12345])),
39+
}));
40+
41+
jest.mock('execa');
42+
43+
jest.mock('fs', () => {
44+
const realFs = jest.requireActual('fs');
45+
return {
46+
...realFs,
47+
readFile: jest.fn(realFs.readFile),
48+
};
49+
});
50+
51+
expect.addSnapshotSerializer(createAbsolutePathSerializer());
52+
53+
jest.requireMock('execa').mockImplementation(async (cmd: string, args: string[], opts: object) => {
54+
expect(cmd).toBe('git');
55+
expect(args).toEqual([
56+
'log',
57+
'-n',
58+
'1',
59+
'--pretty=format:%H',
60+
'--',
61+
expect.stringContaining('kbn-optimizer'),
62+
]);
63+
expect(opts).toEqual({
64+
cwd: REPO_ROOT,
65+
});
66+
67+
return {
68+
stdout: '<last commit sha>',
69+
};
70+
});
71+
72+
describe('getOptimizerCacheKey()', () => {
73+
it('uses latest commit, bootstrap cache, and changed files to create unique value', async () => {
74+
jest
75+
.requireMock('fs')
76+
.readFile.mockImplementation(
77+
(path: string, enc: string, cb: (err: null, file: string) => void) => {
78+
expect(path).toBe(
79+
Path.resolve(REPO_ROOT, 'packages/kbn-optimizer/target/.bootstrap-cache')
80+
);
81+
expect(enc).toBe('utf8');
82+
cb(null, '<bootstrap cache>');
83+
}
84+
);
85+
86+
const config = OptimizerConfig.create({
87+
repoRoot: REPO_ROOT,
88+
});
89+
90+
await expect(getOptimizerCacheKey(config)).resolves.toMatchInlineSnapshot(`
91+
Object {
92+
"bootstrap": "<bootstrap cache>",
93+
"deletedPaths": Array [
94+
"/foo/bar/c",
95+
],
96+
"lastCommit": "<last commit sha>",
97+
"modifiedTimes": Object {
98+
"/foo/bar/a": 12345,
99+
"/foo/bar/b": 12345,
100+
},
101+
"workerConfig": Object {
102+
"browserslistEnv": "dev",
103+
"cache": true,
104+
"dist": false,
105+
"optimizerCacheKey": "♻",
106+
"profileWebpack": false,
107+
"repoRoot": <absolute path>,
108+
"watch": false,
109+
},
110+
}
111+
`);
112+
});
113+
});
114+
115+
describe('diffCacheKey()', () => {
116+
it('returns undefined if values are equal', () => {
117+
expect(diffCacheKey('1', '1')).toBe(undefined);
118+
expect(diffCacheKey(1, 1)).toBe(undefined);
119+
expect(diffCacheKey(['1', '2', { a: 'b' }], ['1', '2', { a: 'b' }])).toBe(undefined);
120+
expect(
121+
diffCacheKey(
122+
{
123+
a: '1',
124+
b: '2',
125+
},
126+
{
127+
b: '2',
128+
a: '1',
129+
}
130+
)
131+
).toBe(undefined);
132+
});
133+
134+
it('returns a diff if the values are different', () => {
135+
expect(diffCacheKey(['1', '2', { a: 'b' }], ['1', '2', { b: 'a' }])).toMatchInlineSnapshot(`
136+
"- Expected
137+
+ Received
138+
139+
 Array [
140+
 \\"1\\",
141+
 \\"2\\",
142+
 Object {
143+
- \\"a\\": \\"b\\",
144+
+ \\"b\\": \\"a\\",
145+
 },
146+
 ]"
147+
`);
148+
expect(
149+
diffCacheKey(
150+
{
151+
a: '1',
152+
b: '1',
153+
},
154+
{
155+
b: '2',
156+
a: '2',
157+
}
158+
)
159+
).toMatchInlineSnapshot(`
160+
"- Expected
161+
+ Received
162+
163+
 Object {
164+
- \\"a\\": \\"1\\",
165+
- \\"b\\": \\"1\\",
166+
+ \\"a\\": \\"2\\",
167+
+ \\"b\\": \\"2\\",
168+
 }"
169+
`);
170+
});
171+
});
172+
173+
describe('reformatJestDiff()', () => {
174+
it('reformats large jestDiff output to focus on the changed lines', () => {
175+
const diff = jestDiff(
176+
{
177+
a: ['1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1', '1', '1', '1', '1', '1'],
178+
},
179+
{
180+
b: ['1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1'],
181+
}
182+
);
183+
184+
expect(reformatJestDiff(diff)).toMatchInlineSnapshot(`
185+
"- Expected
186+
+ Received
187+
188+
 Object {
189+
- \\"a\\": Array [
190+
+ \\"b\\": Array [
191+
 \\"1\\",
192+
 \\"1\\",
193+
 ...
194+
 \\"1\\",
195+
 \\"1\\",
196+
- \\"2\\",
197+
 \\"1\\",
198+
 \\"1\\",
199+
 ...
200+
 \\"1\\",
201+
 \\"1\\",
202+
+ \\"2\\",
203+
 \\"1\\",
204+
 \\"1\\",
205+
 ..."
206+
`);
207+
});
208+
});

src/plugins/data/public/search/sync_search_strategy.test.ts

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,9 @@ describe('Sync search strategy', () => {
3535
core: mockCoreStart,
3636
getSearchStrategy: jest.fn(),
3737
});
38-
syncSearch.search(
39-
{
40-
serverStrategy: SYNC_SEARCH_STRATEGY,
41-
},
42-
{}
43-
);
38+
const request = { serverStrategy: SYNC_SEARCH_STRATEGY };
39+
syncSearch.search(request, {});
40+
4441
expect(mockCoreStart.http.fetch.mock.calls[0][0]).toEqual({
4542
path: `/internal/search/${SYNC_SEARCH_STRATEGY}`,
4643
body: JSON.stringify({
@@ -50,4 +47,47 @@ describe('Sync search strategy', () => {
5047
signal: undefined,
5148
});
5249
});
50+
51+
it('increments and decrements loading count on success', async () => {
52+
const expectedLoadingCountValues = [0, 1, 0];
53+
const receivedLoadingCountValues: number[] = [];
54+
55+
mockCoreStart.http.fetch.mockResolvedValueOnce('response');
56+
57+
const syncSearch = syncSearchStrategyProvider({
58+
core: mockCoreStart,
59+
getSearchStrategy: jest.fn(),
60+
});
61+
const request = { serverStrategy: SYNC_SEARCH_STRATEGY };
62+
63+
const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0];
64+
loadingCount$.subscribe(value => receivedLoadingCountValues.push(value));
65+
66+
await syncSearch.search(request, {}).toPromise();
67+
68+
expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues);
69+
});
70+
71+
it('increments and decrements loading count on failure', async () => {
72+
expect.assertions(1);
73+
const expectedLoadingCountValues = [0, 1, 0];
74+
const receivedLoadingCountValues: number[] = [];
75+
76+
mockCoreStart.http.fetch.mockRejectedValueOnce('error');
77+
78+
const syncSearch = syncSearchStrategyProvider({
79+
core: mockCoreStart,
80+
getSearchStrategy: jest.fn(),
81+
});
82+
const request = { serverStrategy: SYNC_SEARCH_STRATEGY };
83+
84+
const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0];
85+
loadingCount$.subscribe(value => receivedLoadingCountValues.push(value));
86+
87+
try {
88+
await syncSearch.search(request, {}).toPromise();
89+
} catch (e) {
90+
expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues);
91+
}
92+
});
5393
});

src/plugins/data/public/search/sync_search_strategy.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
*/
1919

2020
import { BehaviorSubject, from } from 'rxjs';
21-
import { IKibanaSearchRequest, IKibanaSearchResponse } from '../../common/search';
21+
import { finalize } from 'rxjs/operators';
22+
import { IKibanaSearchRequest } from '../../common/search';
2223
import { ISearch, ISearchOptions } from './i_search';
2324
import { TSearchStrategyProvider, ISearchStrategy, ISearchContext } from './types';
2425

@@ -40,16 +41,14 @@ export const syncSearchStrategyProvider: TSearchStrategyProvider<typeof SYNC_SEA
4041
) => {
4142
loadingCount$.next(loadingCount$.getValue() + 1);
4243

43-
const response: Promise<IKibanaSearchResponse> = context.core.http.fetch({
44-
path: `/internal/search/${request.serverStrategy}`,
45-
method: 'POST',
46-
body: JSON.stringify(request),
47-
signal: options.signal,
48-
});
49-
50-
response.then(() => loadingCount$.next(loadingCount$.getValue() - 1));
51-
52-
return from(response);
44+
return from(
45+
context.core.http.fetch({
46+
path: `/internal/search/${request.serverStrategy}`,
47+
method: 'POST',
48+
body: JSON.stringify(request),
49+
signal: options.signal,
50+
})
51+
).pipe(finalize(() => loadingCount$.next(loadingCount$.getValue() - 1)));
5352
};
5453

5554
return { search };

src/plugins/data/server/autocomplete/value_suggestions_route.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { IRouter } from 'kibana/server';
2323

2424
import { IFieldType, Filter } from '../index';
2525
import { findIndexPatternById, getFieldByName } from '../index_patterns';
26+
import { getRequestAbortedSignal } from '../lib';
2627

2728
export function registerValueSuggestionsRoute(router: IRouter) {
2829
router.post(
@@ -50,6 +51,7 @@ export function registerValueSuggestionsRoute(router: IRouter) {
5051
const { field: fieldName, query, boolFilter } = request.body;
5152
const { index } = request.params;
5253
const { dataClient } = context.core.elasticsearch;
54+
const signal = getRequestAbortedSignal(request.events.aborted$);
5355

5456
const autocompleteSearchOptions = {
5557
timeout: await uiSettings.get<number>('kibana.autocompleteTimeout'),
@@ -62,7 +64,7 @@ export function registerValueSuggestionsRoute(router: IRouter) {
6264
const body = await getBody(autocompleteSearchOptions, field || fieldName, query, boolFilter);
6365

6466
try {
65-
const result = await dataClient.callAsCurrentUser('search', { index, body });
67+
const result = await dataClient.callAsCurrentUser('search', { index, body }, { signal });
6668

6769
const buckets: any[] =
6870
get(result, 'aggregations.suggestions.buckets') ||
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { Subject } from 'rxjs';
21+
import { getRequestAbortedSignal } from './get_request_aborted_signal';
22+
23+
describe('abortableRequestHandler', () => {
24+
jest.useFakeTimers();
25+
26+
it('should call abort if disconnected', () => {
27+
const abortedSubject = new Subject<void>();
28+
const aborted$ = abortedSubject.asObservable();
29+
const onAborted = jest.fn();
30+
31+
const signal = getRequestAbortedSignal(aborted$);
32+
signal.addEventListener('abort', onAborted);
33+
34+
// Shouldn't be aborted or call onAborted prior to disconnecting
35+
expect(signal.aborted).toBe(false);
36+
expect(onAborted).not.toBeCalled();
37+
38+
abortedSubject.next();
39+
jest.runAllTimers();
40+
41+
// Should be aborted and call onAborted after disconnecting
42+
expect(signal.aborted).toBe(true);
43+
expect(onAborted).toBeCalled();
44+
});
45+
});

0 commit comments

Comments
 (0)