Skip to content

Commit

Permalink
[Discover] fix auto-refresh (elastic#80635)
Browse files Browse the repository at this point in the history
* fix refresh interval in discover
* Also implicitly fixes a subtle bug with excessive re-fetch after deleting already disabled filter

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Dosant and kibanamachine committed Oct 16, 2020
1 parent ee7ae63 commit 7f5c656
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { QueryStringManager } from './query_string_manager';
import { Storage } from '../../../../kibana_utils/public/storage';
import { StubBrowserStorage } from 'test_utils/stub_browser_storage';
import { coreMock } from '../../../../../core/public/mocks';
import { Query } from '../../../common/query';

describe('QueryStringManager', () => {
let service: QueryStringManager;

beforeEach(() => {
service = new QueryStringManager(
new Storage(new StubBrowserStorage()),
coreMock.createSetup().uiSettings
);
});

test('getUpdates$ is a cold emits only after query changes', () => {
const obs$ = service.getUpdates$();
const emittedValues: Query[] = [];
obs$.subscribe((v) => {
emittedValues.push(v);
});
expect(emittedValues).toHaveLength(0);

const newQuery = { query: 'new query', language: 'kquery' };
service.setQuery(newQuery);
expect(emittedValues).toHaveLength(1);
expect(emittedValues[0]).toEqual(newQuery);

service.setQuery({ ...newQuery });
expect(emittedValues).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* under the License.
*/

import _ from 'lodash';
import { BehaviorSubject } from 'rxjs';
import { skip } from 'rxjs/operators';
import { CoreStart } from 'kibana/public';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { Query, UI_SETTINGS } from '../../../common';
Expand Down Expand Up @@ -61,7 +61,7 @@ export class QueryStringManager {
}

public getUpdates$ = () => {
return this.query$.asObservable();
return this.query$.asObservable().pipe(skip(1));
};

public getQuery = (): Query => {
Expand Down
18 changes: 12 additions & 6 deletions src/plugins/discover/public/application/angular/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ app.directive('discoverApp', function () {
function discoverController($element, $route, $scope, $timeout, $window, Promise, uiCapabilities) {
const { isDefault: isDefaultType } = indexPatternsUtils;
const subscriptions = new Subscription();
const $fetchObservable = new Subject();
const refetch$ = new Subject();
let inspectorRequest;
const savedSearch = $route.current.locals.savedObjects.savedSearch;
$scope.searchSource = savedSearch.searchSource;
Expand Down Expand Up @@ -267,7 +267,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
);

if (changes.length) {
$fetchObservable.next();
refetch$.next();
}
});
}
Expand Down Expand Up @@ -634,12 +634,18 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise

const init = _.once(() => {
$scope.updateDataSource().then(async () => {
const searchBarChanges = merge(data.query.state$, $fetchObservable).pipe(debounceTime(100));
const fetch$ = merge(
refetch$,
filterManager.getFetches$(),
timefilter.getFetch$(),
timefilter.getAutoRefreshFetch$(),
data.query.queryString.getUpdates$()
).pipe(debounceTime(100));

subscriptions.add(
subscribeWithScope(
$scope,
searchBarChanges,
fetch$,
{
next: $scope.fetch,
},
Expand Down Expand Up @@ -718,7 +724,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise

init.complete = true;
if (shouldSearchOnPageLoad()) {
$fetchObservable.next();
refetch$.next();
}
});
});
Expand Down Expand Up @@ -816,7 +822,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise

$scope.handleRefresh = function (_payload, isUpdate) {
if (isUpdate === false) {
$fetchObservable.next();
refetch$.next();
}
};

Expand Down
33 changes: 33 additions & 0 deletions test/functional/apps/discover/_discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default function ({ getService, getPageObjects }) {
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');
const queryBar = getService('queryBar');
const inspector = getService('inspector');
const PageObjects = getPageObjects(['common', 'discover', 'header', 'timePicker']);
const defaultSettings = {
defaultIndex: 'logstash-*',
Expand Down Expand Up @@ -317,5 +318,37 @@ export default function ({ getService, getPageObjects }) {
expect(currentUrlWithoutScore).not.to.contain('_score');
});
});

describe('refresh interval', function () {
it('should refetch when autofresh is enabled', async () => {
const intervalS = 5;
await PageObjects.timePicker.startAutoRefresh(intervalS);

// check inspector panel request stats for timestamp
await inspector.open();

const getRequestTimestamp = async () => {
const requestStats = await inspector.getTableData();
const requestTimestamp = requestStats.filter((r) =>
r[0].includes('Request timestamp')
)[0][1];
return requestTimestamp;
};

const requestTimestampBefore = await getRequestTimestamp();
await retry.waitFor('refetch because of refresh interval', async () => {
const requestTimestampAfter = await getRequestTimestamp();
log.debug(
`Timestamp before: ${requestTimestampBefore}, Timestamp after: ${requestTimestampAfter}`
);
return requestTimestampBefore !== requestTimestampAfter;
});
});

after(async () => {
await inspector.close();
await PageObjects.timePicker.pauseAutoRefresh();
});
});
});
}
13 changes: 3 additions & 10 deletions test/functional/apps/visualize/_line_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,8 @@ export default function ({ getService, getPageObjects }) {
});

it('should request new data when autofresh is enabled', async () => {
// enable autorefresh
const interval = 3;
await PageObjects.timePicker.openQuickSelectTimeMenu();
await PageObjects.timePicker.inputValue(
'superDatePickerRefreshIntervalInput',
interval.toString()
);
await testSubjects.click('superDatePickerToggleRefreshButton');
await PageObjects.timePicker.closeQuickSelectTimeMenu();
const intervalS = 3;
await PageObjects.timePicker.startAutoRefresh(intervalS);

// check inspector panel request stats for timestamp
await inspector.open();
Expand All @@ -155,7 +148,7 @@ export default function ({ getService, getPageObjects }) {
)[0][1];

// pause to allow time for autorefresh to fire another request
await PageObjects.common.sleep(interval * 1000 * 1.5);
await PageObjects.common.sleep(intervalS * 1000 * 1.5);

// get the latest timestamp from request stats
const requestStatsAfter = await inspector.getTableData();
Expand Down
11 changes: 11 additions & 0 deletions test/functional/page_objects/time_picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,17 @@ export function TimePickerProvider({ getService, getPageObjects }: FtrProviderCo
return moment.duration(endMoment.diff(startMoment)).asHours();
}

public async startAutoRefresh(intervalS = 3) {
await this.openQuickSelectTimeMenu();
await this.inputValue('superDatePickerRefreshIntervalInput', intervalS.toString());
const refreshConfig = await this.getRefreshConfig(true);
if (refreshConfig.isPaused) {
log.debug('start auto refresh');
await testSubjects.click('superDatePickerToggleRefreshButton');
}
await this.closeQuickSelectTimeMenu();
}

public async pauseAutoRefresh() {
log.debug('pauseAutoRefresh');
const refreshConfig = await this.getRefreshConfig(true);
Expand Down

0 comments on commit 7f5c656

Please sign in to comment.