Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,40 @@
/*
* 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'}
*/
const timefilterMock: jest.Mocked<TimefilterContract> = {
isAutoRefreshSelectorEnabled: jest.fn(),
isTimeRangeSelectorEnabled: jest.fn(),
isTimeTouched: jest.fn(),
getEnabledUpdated$: jest.fn(),
getTimeUpdate$: 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 @@ -694,7 +694,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,133 @@
/*
* 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 { SWIMLANE_TYPE } from '../explorer_constants';
import { AppStateSelectedCells } from '../explorer_utils';
import { ExplorerAppState } from '../../../../common/types/ml_url_generator';
Expand All @@ -14,10 +13,9 @@ import { useTimefilter } from '../../contexts/kibana';
export const useSelectedCells = (
appState: ExplorerAppState,
setAppState: (update: Partial<ExplorerAppState>) => void,
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 @@ -76,43 +74,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 @@ -201,7 +201,7 @@ const ExplorerUrlStateManager: FC<ExplorerUrlStateManagerProps> = ({ jobsWithTim
const [selectedCells, setSelectedCells] = useSelectedCells(
explorerUrlState,
setExplorerUrlState,
explorerState?.swimlaneBucketInterval
explorerState?.swimlaneBucketInterval?.asSeconds()
);

useEffect(() => {
Expand Down