Skip to content

Commit 0baaf30

Browse files
darnautovdarnautov
authored
[ML] Fix time range adjustment for the swim lane causing the infinite loop update (#86461) (#86679)
* [ML] fix swim lane time selection adjustment * [ML] fix adjustment * [ML] fix tooManyBuckets condition * [ML] fix typo
1 parent a1fd24d commit 0baaf30

File tree

6 files changed

+219
-42
lines changed

6 files changed

+219
-42
lines changed

‎x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@
55
*/
66

77
export { useMlKibana } from './kibana_context';
8+
export { useTimefilter } from './use_timefilter';

‎x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/use_timefilter.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { TimefilterContract } from '../../../../../../../../src/plugins/data/public';
8+
import { Observable } from 'rxjs';
9+
10+
/**
11+
* Copy from {@link '../../../../../../../../src/plugins/data/public/query/timefilter/timefilter_service.mock'}
12+
*/
13+
const timefilterMock: jest.Mocked<TimefilterContract> = {
14+
isAutoRefreshSelectorEnabled: jest.fn(),
15+
isTimeRangeSelectorEnabled: jest.fn(),
16+
isTimeTouched: jest.fn(),
17+
getEnabledUpdated$: jest.fn(),
18+
getTimeUpdate$: jest.fn(),
19+
getRefreshIntervalUpdate$: jest.fn(),
20+
getAutoRefreshFetch$: jest.fn(() => new Observable<unknown>()),
21+
getFetch$: jest.fn(),
22+
getTime: jest.fn(),
23+
setTime: jest.fn(),
24+
setRefreshInterval: jest.fn(),
25+
getRefreshInterval: jest.fn(),
26+
getActiveBounds: jest.fn(),
27+
disableAutoRefreshSelector: jest.fn(),
28+
disableTimeRangeSelector: jest.fn(),
29+
enableAutoRefreshSelector: jest.fn(),
30+
enableTimeRangeSelector: jest.fn(),
31+
getBounds: jest.fn(),
32+
calculateBounds: jest.fn(),
33+
createFilter: jest.fn(),
34+
getRefreshIntervalDefaults: jest.fn(),
35+
getTimeDefaults: jest.fn(),
36+
};
37+
38+
export const useTimefilter = jest.fn(() => {
39+
return timefilterMock;
40+
});

‎x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ function calculateChartRange(
694694
chartRange.min = chartRange.min + maxBucketSpanMs;
695695
}
696696

697-
if (chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) {
697+
if (
698+
(chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) &&
699+
chartRange.max - chartRange.min < selectedLatestMs - selectedEarliestMs
700+
) {
698701
tooManyBuckets = true;
699702
}
700703

‎x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import moment from 'moment';
8+
import { renderHook } from '@testing-library/react-hooks';
9+
import { useSelectedCells } from './use_selected_cells';
10+
import { ExplorerAppState } from '../../../../common/types/ml_url_generator';
11+
import { TimefilterContract } from '../../../../../../../src/plugins/data/public';
12+
13+
import { useTimefilter } from '../../contexts/kibana';
14+
15+
jest.mock('../../contexts/kibana');
16+
17+
describe('useSelectedCells', () => {
18+
test('should not set state when the cell selection is correct', () => {
19+
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
20+
min: moment(1498824778 * 1000),
21+
max: moment(1502366798 * 1000),
22+
});
23+
24+
const urlState = {
25+
mlExplorerSwimlane: {
26+
selectedType: 'overall',
27+
selectedLanes: ['Overall'],
28+
selectedTimes: [1498780800, 1498867200],
29+
showTopFieldValues: true,
30+
viewByFieldName: 'apache2.access.remote_ip',
31+
viewByFromPage: 1,
32+
viewByPerPage: 10,
33+
},
34+
mlExplorerFilter: {},
35+
} as ExplorerAppState;
36+
37+
const setUrlState = jest.fn();
38+
39+
const bucketInterval = 86400;
40+
41+
renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));
42+
43+
expect(setUrlState).not.toHaveBeenCalled();
44+
});
45+
46+
test('should reset cell selection when it is completely out of range', () => {
47+
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
48+
min: moment(1501071178 * 1000),
49+
max: moment(1502366798 * 1000),
50+
});
51+
52+
const urlState = {
53+
mlExplorerSwimlane: {
54+
selectedType: 'overall',
55+
selectedLanes: ['Overall'],
56+
selectedTimes: [1498780800, 1498867200],
57+
showTopFieldValues: true,
58+
viewByFieldName: 'apache2.access.remote_ip',
59+
viewByFromPage: 1,
60+
viewByPerPage: 10,
61+
},
62+
mlExplorerFilter: {},
63+
} as ExplorerAppState;
64+
65+
const setUrlState = jest.fn();
66+
67+
const bucketInterval = 86400;
68+
69+
const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));
70+
71+
expect(result.current[0]).toEqual({
72+
lanes: ['Overall'],
73+
showTopFieldValues: true,
74+
times: [1498780800, 1498867200],
75+
type: 'overall',
76+
viewByFieldName: 'apache2.access.remote_ip',
77+
});
78+
79+
expect(setUrlState).toHaveBeenCalledWith({
80+
mlExplorerSwimlane: {
81+
viewByFieldName: 'apache2.access.remote_ip',
82+
viewByFromPage: 1,
83+
viewByPerPage: 10,
84+
},
85+
});
86+
});
87+
88+
test('should adjust cell selection time boundaries based on the main time range', () => {
89+
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
90+
min: moment(1501071178 * 1000),
91+
max: moment(1502366798 * 1000),
92+
});
93+
94+
const urlState = {
95+
mlExplorerSwimlane: {
96+
selectedType: 'overall',
97+
selectedLanes: ['Overall'],
98+
selectedTimes: [1498780800, 1502366798],
99+
showTopFieldValues: true,
100+
viewByFieldName: 'apache2.access.remote_ip',
101+
viewByFromPage: 1,
102+
viewByPerPage: 10,
103+
},
104+
mlExplorerFilter: {},
105+
} as ExplorerAppState;
106+
107+
const setUrlState = jest.fn();
108+
109+
const bucketInterval = 86400;
110+
111+
const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));
112+
113+
expect(result.current[0]).toEqual({
114+
lanes: ['Overall'],
115+
showTopFieldValues: true,
116+
times: [1498780800, 1502366798],
117+
type: 'overall',
118+
viewByFieldName: 'apache2.access.remote_ip',
119+
});
120+
121+
expect(setUrlState).toHaveBeenCalledWith({
122+
mlExplorerSwimlane: {
123+
selectedLanes: ['Overall'],
124+
selectedTimes: [1500984778, 1502366798],
125+
selectedType: 'overall',
126+
showTopFieldValues: true,
127+
viewByFieldName: 'apache2.access.remote_ip',
128+
viewByFromPage: 1,
129+
viewByPerPage: 10,
130+
},
131+
});
132+
});
133+
});

‎x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66

77
import { useCallback, useEffect, useMemo } from 'react';
8-
import { Duration } from 'moment';
98
import { SWIMLANE_TYPE } from '../explorer_constants';
109
import { AppStateSelectedCells } from '../explorer_utils';
1110
import { ExplorerAppState } from '../../../../common/types/ml_url_generator';
@@ -14,10 +13,9 @@ import { useTimefilter } from '../../contexts/kibana';
1413
export const useSelectedCells = (
1514
appState: ExplorerAppState,
1615
setAppState: (update: Partial<ExplorerAppState>) => void,
17-
bucketInterval: Duration | undefined
16+
bucketIntervalInSeconds: number | undefined
1817
): [AppStateSelectedCells | undefined, (swimlaneSelectedCells: AppStateSelectedCells) => void] => {
1918
const timeFilter = useTimefilter();
20-
2119
const timeBounds = timeFilter.getBounds();
2220

2321
// keep swimlane selection, restore selectedCells from AppState
@@ -76,43 +74,45 @@ export const useSelectedCells = (
7674
* Adjust cell selection with respect to the time boundaries.
7775
* Reset it entirely when it out of range.
7876
*/
79-
useEffect(() => {
80-
if (selectedCells?.times === undefined || bucketInterval === undefined) return;
81-
82-
let [selectedFrom, selectedTo] = selectedCells.times;
83-
84-
const rangeFrom = timeBounds.min!.unix();
85-
/**
86-
* Because each cell on the swim lane represent the fixed bucket interval,
87-
* the selection range could be outside of the time boundaries with
88-
* correction within the bucket interval.
89-
*/
90-
const rangeTo = timeBounds.max!.unix() + bucketInterval.asSeconds();
91-
92-
selectedFrom = Math.max(selectedFrom, rangeFrom);
93-
94-
selectedTo = Math.min(selectedTo, rangeTo);
95-
96-
const isSelectionOutOfRange = rangeFrom > selectedTo || rangeTo < selectedFrom;
97-
98-
if (isSelectionOutOfRange) {
99-
// reset selection
100-
setSelectedCells();
101-
return;
102-
}
103-
104-
if (selectedFrom !== rangeFrom || selectedTo !== rangeTo) {
105-
setSelectedCells({
106-
...selectedCells,
107-
times: [selectedFrom, selectedTo],
108-
});
109-
}
110-
}, [
111-
timeBounds.min?.valueOf(),
112-
timeBounds.max?.valueOf(),
113-
selectedCells,
114-
bucketInterval?.asMilliseconds(),
115-
]);
77+
useEffect(
78+
function adjustSwimLaneTimeSelection() {
79+
if (selectedCells?.times === undefined || bucketIntervalInSeconds === undefined) return;
80+
81+
const [selectedFrom, selectedTo] = selectedCells.times;
82+
83+
/**
84+
* Because each cell on the swim lane represent the fixed bucket interval,
85+
* the selection range could be outside of the time boundaries with
86+
* correction within the bucket interval.
87+
*/
88+
const rangeFrom = timeBounds.min!.unix() - bucketIntervalInSeconds;
89+
const rangeTo = timeBounds.max!.unix() + bucketIntervalInSeconds;
90+
91+
const resultFrom = Math.max(selectedFrom, rangeFrom);
92+
const resultTo = Math.min(selectedTo, rangeTo);
93+
94+
const isSelectionOutOfRange = rangeFrom > resultTo || rangeTo < resultFrom;
95+
96+
if (isSelectionOutOfRange) {
97+
// reset selection
98+
setSelectedCells();
99+
return;
100+
}
101+
102+
if (selectedFrom === resultFrom && selectedTo === resultTo) {
103+
// selection is correct, no need to adjust the range
104+
return;
105+
}
106+
107+
if (resultFrom !== rangeFrom || resultTo !== rangeTo) {
108+
setSelectedCells({
109+
...selectedCells,
110+
times: [resultFrom, resultTo],
111+
});
112+
}
113+
},
114+
[timeBounds.min?.unix(), timeBounds.max?.unix(), selectedCells, bucketIntervalInSeconds]
115+
);
116116

117117
return [selectedCells, setSelectedCells];
118118
};

‎x-pack/plugins/ml/public/application/routing/routes/explorer.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ const ExplorerUrlStateManager: FC<ExplorerUrlStateManagerProps> = ({ jobsWithTim
201201
const [selectedCells, setSelectedCells] = useSelectedCells(
202202
explorerUrlState,
203203
setExplorerUrlState,
204-
explorerState?.swimlaneBucketInterval
204+
explorerState?.swimlaneBucketInterval?.asSeconds()
205205
);
206206

207207
useEffect(() => {

0 commit comments

Comments
 (0)