Skip to content

Commit

Permalink
[7.10] [ML] Fix time range adjustment for the swim lane causing the i…
Browse files Browse the repository at this point in the history
…nfinite loop update (#86461) (#86680)

* [ML] Fix time range adjustment for the swim lane causing the infinite loop update (#86461)

* [ML] fix swim lane time selection adjustment

* [ML] fix adjustment

* [ML] fix tooManyBuckets condition

* [ML] fix typo
# Conflicts:
#	x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts
#	x-pack/plugins/ml/public/application/routing/routes/explorer.tsx

* [ML] adjust unit tests

* [ML] disable mock ts check
  • Loading branch information
darnautov authored Dec 22, 2020
1 parent 9e5f4ae commit 58d9c4a
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
*/

export { useMlKibana } from './kibana_context';
export { useTimefilter } from './use_timefilter';
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { TimefilterContract } from '../../../../../../../../src/plugins/data/public';
import { Observable } from 'rxjs';

/**
* Copy from {@link '../../../../../../../../src/plugins/data/public/query/timefilter/timefilter_service.mock'}
*/
// @ts-ignore
const timefilterMock: jest.Mocked<TimefilterContract> = {
isAutoRefreshSelectorEnabled: jest.fn(),
isTimeRangeSelectorEnabled: jest.fn(),
getRefreshIntervalUpdate$: jest.fn(),
getAutoRefreshFetch$: jest.fn(() => new Observable<unknown>()),
getFetch$: jest.fn(),
getTime: jest.fn(),
setTime: jest.fn(),
setRefreshInterval: jest.fn(),
getRefreshInterval: jest.fn(),
getActiveBounds: jest.fn(),
disableAutoRefreshSelector: jest.fn(),
disableTimeRangeSelector: jest.fn(),
enableAutoRefreshSelector: jest.fn(),
enableTimeRangeSelector: jest.fn(),
getBounds: jest.fn(),
calculateBounds: jest.fn(),
createFilter: jest.fn(),
getRefreshIntervalDefaults: jest.fn(),
getTimeDefaults: jest.fn(),
};

export const useTimefilter = jest.fn(() => {
return timefilterMock;
});
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,10 @@ function calculateChartRange(
chartRange.min = chartRange.min + maxBucketSpanMs;
}

if (chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) {
if (
(chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) &&
chartRange.max - chartRange.min < selectedLatestMs - selectedEarliestMs
) {
tooManyBuckets = true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import moment from 'moment';
import { renderHook } from '@testing-library/react-hooks';
import { useSelectedCells } from './use_selected_cells';
import { ExplorerAppState } from '../../../../common/types/ml_url_generator';
import { TimefilterContract } from '../../../../../../../src/plugins/data/public';

import { useTimefilter } from '../../contexts/kibana';

jest.mock('../../contexts/kibana');

describe('useSelectedCells', () => {
test('should not set state when the cell selection is correct', () => {
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
min: moment(1498824778 * 1000),
max: moment(1502366798 * 1000),
});

const urlState = {
mlExplorerSwimlane: {
selectedType: 'overall',
selectedLanes: ['Overall'],
selectedTimes: [1498780800, 1498867200],
showTopFieldValues: true,
viewByFieldName: 'apache2.access.remote_ip',
viewByFromPage: 1,
viewByPerPage: 10,
},
mlExplorerFilter: {},
} as ExplorerAppState;

const setUrlState = jest.fn();

const bucketInterval = 86400;

renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));

expect(setUrlState).not.toHaveBeenCalled();
});

test('should reset cell selection when it is completely out of range', () => {
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
min: moment(1501071178 * 1000),
max: moment(1502366798 * 1000),
});

const urlState = {
mlExplorerSwimlane: {
selectedType: 'overall',
selectedLanes: ['Overall'],
selectedTimes: [1498780800, 1498867200],
showTopFieldValues: true,
viewByFieldName: 'apache2.access.remote_ip',
viewByFromPage: 1,
viewByPerPage: 10,
},
mlExplorerFilter: {},
} as ExplorerAppState;

const setUrlState = jest.fn();

const bucketInterval = 86400;

const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));

expect(result.current[0]).toEqual({
lanes: ['Overall'],
showTopFieldValues: true,
times: [1498780800, 1498867200],
type: 'overall',
viewByFieldName: 'apache2.access.remote_ip',
});

expect(setUrlState).toHaveBeenCalledWith('mlExplorerSwimlane', {
viewByFieldName: 'apache2.access.remote_ip',
viewByFromPage: 1,
viewByPerPage: 10,
});
});

test('should adjust cell selection time boundaries based on the main time range', () => {
(useTimefilter() as jest.Mocked<TimefilterContract>).getBounds.mockReturnValue({
min: moment(1501071178 * 1000),
max: moment(1502366798 * 1000),
});

const urlState = {
mlExplorerSwimlane: {
selectedType: 'overall',
selectedLanes: ['Overall'],
selectedTimes: [1498780800, 1502366798],
showTopFieldValues: true,
viewByFieldName: 'apache2.access.remote_ip',
viewByFromPage: 1,
viewByPerPage: 10,
},
mlExplorerFilter: {},
} as ExplorerAppState;

const setUrlState = jest.fn();

const bucketInterval = 86400;

const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval));

expect(result.current[0]).toEqual({
lanes: ['Overall'],
showTopFieldValues: true,
times: [1498780800, 1502366798],
type: 'overall',
viewByFieldName: 'apache2.access.remote_ip',
});

expect(setUrlState).toHaveBeenCalledWith('mlExplorerSwimlane', {
selectedLanes: ['Overall'],
selectedTimes: [1500984778, 1502366798],
selectedType: 'overall',
showTopFieldValues: true,
viewByFieldName: 'apache2.access.remote_ip',
viewByFromPage: 1,
viewByPerPage: 10,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import { useCallback, useEffect, useMemo } from 'react';
import { Duration } from 'moment';
import { useUrlState } from '../../util/url_state';
import { SWIMLANE_TYPE } from '../explorer_constants';
import { AppStateSelectedCells } from '../explorer_utils';
Expand All @@ -15,10 +14,9 @@ import { useTimefilter } from '../../contexts/kibana';
export const useSelectedCells = (
appState: ExplorerAppState,
setAppState: ReturnType<typeof useUrlState>[1],
bucketInterval: Duration | undefined
bucketIntervalInSeconds: number | undefined
): [AppStateSelectedCells | undefined, (swimlaneSelectedCells: AppStateSelectedCells) => void] => {
const timeFilter = useTimefilter();

const timeBounds = timeFilter.getBounds();

// keep swimlane selection, restore selectedCells from AppState
Expand Down Expand Up @@ -75,43 +73,45 @@ export const useSelectedCells = (
* Adjust cell selection with respect to the time boundaries.
* Reset it entirely when it out of range.
*/
useEffect(() => {
if (selectedCells?.times === undefined || bucketInterval === undefined) return;

let [selectedFrom, selectedTo] = selectedCells.times;

const rangeFrom = timeBounds.min!.unix();
/**
* Because each cell on the swim lane represent the fixed bucket interval,
* the selection range could be outside of the time boundaries with
* correction within the bucket interval.
*/
const rangeTo = timeBounds.max!.unix() + bucketInterval.asSeconds();

selectedFrom = Math.max(selectedFrom, rangeFrom);

selectedTo = Math.min(selectedTo, rangeTo);

const isSelectionOutOfRange = rangeFrom > selectedTo || rangeTo < selectedFrom;

if (isSelectionOutOfRange) {
// reset selection
setSelectedCells();
return;
}

if (selectedFrom !== rangeFrom || selectedTo !== rangeTo) {
setSelectedCells({
...selectedCells,
times: [selectedFrom, selectedTo],
});
}
}, [
timeBounds.min?.valueOf(),
timeBounds.max?.valueOf(),
selectedCells,
bucketInterval?.asMilliseconds(),
]);
useEffect(
function adjustSwimLaneTimeSelection() {
if (selectedCells?.times === undefined || bucketIntervalInSeconds === undefined) return;

const [selectedFrom, selectedTo] = selectedCells.times;

/**
* Because each cell on the swim lane represent the fixed bucket interval,
* the selection range could be outside of the time boundaries with
* correction within the bucket interval.
*/
const rangeFrom = timeBounds.min!.unix() - bucketIntervalInSeconds;
const rangeTo = timeBounds.max!.unix() + bucketIntervalInSeconds;

const resultFrom = Math.max(selectedFrom, rangeFrom);
const resultTo = Math.min(selectedTo, rangeTo);

const isSelectionOutOfRange = rangeFrom > resultTo || rangeTo < resultFrom;

if (isSelectionOutOfRange) {
// reset selection
setSelectedCells();
return;
}

if (selectedFrom === resultFrom && selectedTo === resultTo) {
// selection is correct, no need to adjust the range
return;
}

if (resultFrom !== rangeFrom || resultTo !== rangeTo) {
setSelectedCells({
...selectedCells,
times: [resultFrom, resultTo],
});
}
},
[timeBounds.min?.unix(), timeBounds.max?.unix(), selectedCells, bucketIntervalInSeconds]
);

return [selectedCells, setSelectedCells];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ const ExplorerUrlStateManager: FC<ExplorerUrlStateManagerProps> = ({ jobsWithTim
const [selectedCells, setSelectedCells] = useSelectedCells(
appState,
setAppState,
explorerState?.swimlaneBucketInterval
explorerState?.swimlaneBucketInterval?.asSeconds()
);

useEffect(() => {
Expand Down

0 comments on commit 58d9c4a

Please sign in to comment.