Skip to content

Commit e26e842

Browse files
ThomThomsonThomThomson
authored
[Controls] Fix initialization race condition (#237726)
Fixes a race condition in the controls initialization step.
1 parent ffe1390 commit e26e842

File tree

8 files changed

+167
-37
lines changed

8 files changed

+167
-37
lines changed

‎src/platform/plugins/shared/controls/public/control_group/selections_manager.test.ts‎

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,28 +28,29 @@ describe('selections manager', () => {
2828
timeslice$?: BehaviorSubject<[number, number] | undefined>;
2929
};
3030
}>({});
31+
const setupControlChildren = () => {
32+
control1Api.filters$.next(undefined);
33+
control2Api.filters$.next([
34+
{
35+
meta: {
36+
alias: 'control2 original filter',
37+
},
38+
},
39+
]);
40+
control3Api.timeslice$.next([
41+
Date.parse('2024-06-09T06:00:00.000Z'),
42+
Date.parse('2024-06-09T12:00:00.000Z'),
43+
]);
44+
controlGroupApi.children$.next({
45+
control1: control1Api,
46+
control2: control2Api,
47+
control3: control3Api,
48+
});
49+
};
3150
const controlGroupApi = {
3251
autoApplySelections$: new BehaviorSubject(false),
3352
children$,
34-
untilInitialized: async () => {
35-
control1Api.filters$.next(undefined);
36-
control2Api.filters$.next([
37-
{
38-
meta: {
39-
alias: 'control2 original filter',
40-
},
41-
},
42-
]);
43-
control3Api.timeslice$.next([
44-
Date.parse('2024-06-09T06:00:00.000Z'),
45-
Date.parse('2024-06-09T12:00:00.000Z'),
46-
]);
47-
controlGroupApi.children$.next({
48-
control1: control1Api,
49-
control2: control2Api,
50-
control3: control3Api,
51-
});
52-
},
53+
untilInitialized: async () => setupControlChildren(),
5354
};
5455

5556
const onFireMock = jest.fn();
@@ -58,6 +59,51 @@ describe('selections manager', () => {
5859
controlGroupApi.children$.next({});
5960
});
6061

62+
describe('initialization', () => {
63+
it('should wait for all child apis to be available before publishing initial filters', async () => {
64+
let resolveChildren: (() => void) | undefined;
65+
const childrenResolvePromise = new Promise<void>((r) => (resolveChildren = r));
66+
67+
const slowControlGroupApi = {
68+
...controlGroupApi,
69+
untilInitialized: async () => {
70+
await childrenResolvePromise;
71+
return setupControlChildren();
72+
},
73+
};
74+
const selectionsManager = initSelectionsManager(
75+
slowControlGroupApi as unknown as Pick<
76+
ControlGroupApi,
77+
'autoApplySelections$' | 'children$' | 'untilInitialized'
78+
>
79+
);
80+
81+
// instrumentation to tell when filters are published.
82+
let filtersPublished = false;
83+
(async () => {
84+
await selectionsManager.api.untilFiltersPublished();
85+
filtersPublished = true;
86+
})();
87+
88+
// filters should not have been published yet even after waiting 5 ms.
89+
await new Promise((resolve) => setTimeout(resolve, 5));
90+
expect(selectionsManager.api.filters$.value).toEqual([]); // default empty state for filters
91+
expect(filtersPublished).toBe(false);
92+
93+
// resolve children and ensure initial filters are reported
94+
resolveChildren?.();
95+
await new Promise((resolve) => setTimeout(resolve, 1));
96+
expect(filtersPublished).toBe(true);
97+
expect(selectionsManager.api.filters$.value).toEqual([
98+
{
99+
meta: {
100+
alias: 'control2 original filter',
101+
},
102+
},
103+
]);
104+
});
105+
});
106+
61107
describe('auto apply selections disabled', () => {
62108
beforeEach(() => {
63109
controlGroupApi.autoApplySelections$.next(false);

‎src/platform/plugins/shared/controls/public/control_group/selections_manager.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import type { Subscription } from 'rxjs';
11-
import { BehaviorSubject, combineLatest } from 'rxjs';
11+
import { BehaviorSubject, combineLatest, first } from 'rxjs';
1212
import deepEqual from 'fast-deep-equal';
1313
import type { Filter } from '@kbn/es-query';
1414
import { combineCompatibleChildrenApis } from '@kbn/presentation-containers';
@@ -25,6 +25,15 @@ export function initSelectionsManager(
2525
const unpublishedTimeslice$ = new BehaviorSubject<[number, number] | undefined>(undefined);
2626
const hasUnappliedSelections$ = new BehaviorSubject(false);
2727

28+
const filtersPublished = new BehaviorSubject<boolean>(false);
29+
const untilFiltersPublished = () =>
30+
new Promise<void>((resolve) => {
31+
filtersPublished.pipe(first((isComplete) => isComplete)).subscribe(() => {
32+
resolve();
33+
filtersPublished.complete();
34+
});
35+
});
36+
2837
const subscriptions: Subscription[] = [];
2938
controlGroupApi.untilInitialized().then(() => {
3039
const initialFilters: Filter[] = [];
@@ -93,6 +102,7 @@ export function initSelectionsManager(
93102
}
94103
})
95104
);
105+
filtersPublished.next(true);
96106
});
97107

98108
function applySelections() {
@@ -108,6 +118,7 @@ export function initSelectionsManager(
108118
api: {
109119
filters$,
110120
timeslice$,
121+
untilFiltersPublished,
111122
},
112123
applySelections,
113124
cleanup: () => {

‎src/platform/plugins/shared/controls/public/control_group/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,16 @@ export type ControlGroupApi = PresentationContainer &
7171
controlStateTransform?: ControlStateTransform;
7272
onSave?: () => void;
7373
}) => void;
74+
/**
75+
* @returns a promise which is resolved when all controls children have finished initializing.
76+
*/
7477
untilInitialized: () => Promise<void>;
7578

79+
/**
80+
* @returns a promise which is resolved when all initial selections have been initialized and published.
81+
*/
82+
untilFiltersPublished: () => Promise<void>;
83+
7684
/** Public getters */
7785
getEditorConfig: () => ControlGroupEditorConfig | undefined;
7886

‎src/platform/plugins/shared/controls/public/controls/data_controls/data_control_manager.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import type { Observable } from 'rxjs';
11-
import { BehaviorSubject, combineLatest, debounceTime, first, skip, switchMap, tap } from 'rxjs';
11+
import { BehaviorSubject, combineLatest, first, skip, switchMap, tap } from 'rxjs';
1212

1313
import type { DataView, DataViewField } from '@kbn/data-views-plugin/common';
1414
import { DATA_VIEW_SAVED_OBJECT_TYPE } from '@kbn/data-views-plugin/common';
@@ -175,7 +175,7 @@ export const initializeDataControlManager = <EditorState extends object = {}>(
175175
});
176176
};
177177

178-
const filtersReadySubscription = filters$.pipe(skip(1), debounceTime(0)).subscribe(() => {
178+
const filtersReadySubscription = filters$.pipe(skip(1)).subscribe(() => {
179179
// Set filtersReady$.next(true); in filters$ subscription instead of setOutputFilter
180180
// to avoid signaling filters ready until after filters have been emitted
181181
// to avoid timing issues

‎src/platform/plugins/shared/controls/public/controls/data_controls/options_list_control/get_options_list_control_factory.test.tsx‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import React from 'react';
11-
10+
import { EuiThemeProvider } from '@elastic/eui';
1211
import type { DataView } from '@kbn/data-views-plugin/common';
1312
import { createStubDataView } from '@kbn/data-views-plugin/common/data_view.stub';
1413
import { render as rtlRender, waitFor } from '@testing-library/react';
1514
import userEvent from '@testing-library/user-event';
16-
import { EuiThemeProvider } from '@elastic/eui';
17-
15+
import React from 'react';
1816
import { coreServices, dataViewsService } from '../../../services/kibana_services';
1917
import { getMockedControlGroupApi, getMockedFinalizeApi } from '../../mocks/control_mocks';
2018
import { getOptionsListControlFactory } from './get_options_list_control_factory';
@@ -29,7 +27,7 @@ describe('Options List Control Api', () => {
2927
const factory = getOptionsListControlFactory();
3028
const finalizeApi = getMockedFinalizeApi(uuid, factory, controlGroupApi);
3129

32-
dataViewsService.get = jest.fn().mockImplementation(async (id: string): Promise<DataView> => {
30+
const getDataView = async (id: string): Promise<DataView> => {
3331
if (id !== 'myDataViewId') {
3432
throw new Error(`Simulated error: no data view found for id ${id}`);
3533
}
@@ -59,9 +57,75 @@ describe('Options List Control Api', () => {
5957
};
6058
});
6159
return stubDataView;
60+
};
61+
62+
describe('initialization', () => {
63+
let dataviewDelayPromise: Promise<void> | undefined;
64+
65+
beforeAll(() => {
66+
dataViewsService.get = jest.fn().mockImplementation(async (id: string) => {
67+
if (dataviewDelayPromise) await dataviewDelayPromise;
68+
return getDataView(id);
69+
});
70+
});
71+
72+
it('returns api immediately when no initial selections are configured', async () => {
73+
let resolveDataView: (() => void) | undefined;
74+
let apiReturned = false;
75+
dataviewDelayPromise = new Promise((res) => (resolveDataView = res));
76+
(async () => {
77+
await factory.buildControl({
78+
initialState: {
79+
dataViewId: 'myDataViewId',
80+
fieldName: 'myFieldName',
81+
},
82+
finalizeApi,
83+
uuid,
84+
controlGroupApi,
85+
});
86+
apiReturned = true;
87+
})();
88+
await new Promise((r) => setTimeout(r, 1));
89+
expect(apiReturned).toBe(true);
90+
resolveDataView?.();
91+
dataviewDelayPromise = undefined;
92+
});
93+
94+
it('waits until data view is available before returning api when initial selections are configured', async () => {
95+
let resolveDataView: (() => void) | undefined;
96+
let apiReturned = false;
97+
dataviewDelayPromise = new Promise((res) => (resolveDataView = res));
98+
(async () => {
99+
await factory.buildControl({
100+
initialState: {
101+
dataViewId: 'myDataViewId',
102+
fieldName: 'myFieldName',
103+
selectedOptions: ['cool', 'test'],
104+
},
105+
finalizeApi,
106+
uuid,
107+
controlGroupApi,
108+
});
109+
apiReturned = true;
110+
})();
111+
112+
// even after 10ms the API should not have returned yet because the data view was not available
113+
await new Promise((r) => setTimeout(r, 10));
114+
expect(apiReturned).toBe(false);
115+
116+
// resolve the data view and ensure the api returns
117+
resolveDataView?.();
118+
await new Promise((r) => setTimeout(r, 10));
119+
expect(apiReturned).toBe(true);
120+
dataviewDelayPromise = undefined;
121+
});
62122
});
63123

64124
describe('filters$', () => {
125+
beforeAll(() => {
126+
dataViewsService.get = jest.fn().mockImplementation(getDataView);
127+
});
128+
65129
test('should not set filters$ when selectedOptions is not provided', async () => {
66130
const { api } = await factory.buildControl({
67131
initialState: {
@@ -172,6 +236,7 @@ describe('Options List Control Api', () => {
172236

173237
describe('make selection', () => {
174238
beforeAll(() => {
239+
dataViewsService.get = jest.fn().mockImplementation(getDataView);
175240
coreServices.http.fetch = jest.fn().mockResolvedValue({
176241
suggestions: [
177242
{ value: 'woof', docCount: 10 },

‎src/platform/plugins/shared/controls/public/controls/data_controls/options_list_control/get_options_list_control_factory.tsx‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,14 @@ export const getOptionsListControlFactory = (): DataControlFactory<
235235
const field = dataView && fieldName ? dataView.getFieldByName(fieldName) : undefined;
236236

237237
let newFilter: Filter | undefined;
238-
if (dataView && field) {
239-
if (existsSelected) {
240-
newFilter = buildExistsFilter(field, dataView);
241-
} else if (selectedOptions && selectedOptions.length > 0) {
242-
newFilter =
243-
selectedOptions.length === 1
244-
? buildPhraseFilter(field, selectedOptions[0], dataView)
245-
: buildPhrasesFilter(field, selectedOptions, dataView);
246-
}
238+
if (!dataView || !field) return;
239+
if (existsSelected) {
240+
newFilter = buildExistsFilter(field, dataView);
241+
} else if (selectedOptions && selectedOptions.length > 0) {
242+
newFilter =
243+
selectedOptions.length === 1
244+
? buildPhraseFilter(field, selectedOptions[0], dataView)
245+
: buildPhrasesFilter(field, selectedOptions, dataView);
247246
}
248247
if (newFilter) {
249248
newFilter.meta.key = field?.name;

‎src/platform/plugins/shared/dashboard/public/dashboard_api/control_group_manager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function initializeControlGroupManager(
2626
.pipe(
2727
skipWhile((controlGroupApi) => !controlGroupApi),
2828
switchMap(async (controlGroupApi) => {
29-
await controlGroupApi?.untilInitialized();
29+
await controlGroupApi?.untilFiltersPublished();
3030
}),
3131
first()
3232
)

‎src/platform/plugins/shared/dashboard/public/mocks.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export function setupIntersectionObserverMock({
6969

7070
export const mockControlGroupApi = {
7171
untilInitialized: async () => {},
72+
untilFiltersPublished: async () => {},
7273
filters$: new BehaviorSubject(undefined),
7374
query$: new BehaviorSubject(undefined),
7475
timeslice$: new BehaviorSubject(undefined),

0 commit comments

Comments
 (0)