Skip to content

Commit

Permalink
Timefilter - replace SimpleEmitter with observables (#43748) (#44089)
Browse files Browse the repository at this point in the history
* Replaced 'timeUpdate' and 'enabledUpdated' timefilter events with observables.

* Change enabledUpdated$ to a BehaviorSubject

* refreshIntervalUpdate + fixes in monitoring

* autoRefreshFetch

* getFetch + delete listenAndDigestAsync

* Removed SimpleEmitter parent

* Updated timefilter tests

* Post merge code updates in ML + type fixes

* visual editor unsubscribe

* removed unused import

* timefilter mock

* Import only from top level of timefilter

* Fixed typo in discover

* unsubscribe in monitoring

* Deleted two  tests relying on timefilter implementing EventEmitter

* Renamed subscribtion var name

* import path for fixing jest test ?

* Removed unused row
  • Loading branch information
Liza Katz authored Aug 28, 2019
1 parent 8025ea0 commit 7097cfb
Show file tree
Hide file tree
Showing 45 changed files with 277 additions and 203 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { doesKueryExpressionHaveLuceneSyntaxError } from '@kbn/es-query';
import classNames from 'classnames';
import React, { Component } from 'react';
import { Storage } from 'ui/storage';
import { timeHistory } from 'ui/timefilter/time_history';
import { timeHistory } from 'ui/timefilter';

import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiLink, EuiSuperDatePicker } from '@elastic/eui';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { Filter } from '@kbn/es-query';
import { RefreshInterval, TimeRange } from 'ui/timefilter/timefilter';
import { RefreshInterval, TimeRange } from 'ui/timefilter';
import { Query } from '../../query/query_bar';

export * from './components';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ import {

import { KbnUrl } from 'ui/url/kbn_url';
import { Filter } from '@kbn/es-query';
import { TimeRange } from 'ui/timefilter/time_history';
import { TimeRange } from 'ui/timefilter';
import { IndexPattern } from 'ui/index_patterns';
import { IPrivate } from 'ui/private';
import { StaticIndexPattern, Query, SavedQuery } from 'plugins/data';
import moment from 'moment';
import { Subscription } from 'rxjs';

import { ViewMode } from '../../../embeddable_api/public/np_ready/public';
import { SavedObjectDashboard } from './saved_dashboard/saved_dashboard';
Expand Down Expand Up @@ -81,7 +82,6 @@ export interface DashboardAppScope extends ng.IScope {
refreshInterval: any;
}) => void;
onFiltersUpdated: (filters: Filter[]) => void;
$listenAndDigestAsync: any;
onCancelApplyFilters: () => void;
onApplyFilters: (filters: Filter[]) => void;
onQuerySaved: (savedQuery: SavedQuery) => void;
Expand All @@ -93,7 +93,7 @@ export interface DashboardAppScope extends ng.IScope {
showSaveQuery: boolean;
kbnTopNav: any;
enterEditMode: () => void;
$listen: any;
timefilterSubscriptions$: Subscription;
}

const app = uiModules.get('app/dashboard', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import angular from 'angular';
import { uniq } from 'lodash';

import chrome from 'ui/chrome';
import { subscribeWithScope } from 'ui/utils/subscribe_with_scope';
import { toastNotifications } from 'ui/notify';

// @ts-ignore
Expand Down Expand Up @@ -515,23 +516,36 @@ export class DashboardAppController {
}
);

$scope.$listenAndDigestAsync(timefilter, 'fetch', () => {
// The only reason this is here is so that search embeddables work on a dashboard with
// a refresh interval turned on. This kicks off the search poller. It should be
// refactored so no embeddables need to listen to the timefilter directly but instead
// the container tells it when to reload.
courier.fetch();
});
$scope.timefilterSubscriptions$ = new Subscription();
// The only reason this is here is so that search embeddables work on a dashboard with
// a refresh interval turned on. This kicks off the search poller. It should be
// refactored so no embeddables need to listen to the timefilter directly but instead
// the container tells it when to reload.
$scope.timefilterSubscriptions$.add(
subscribeWithScope($scope, timefilter.getFetch$(), {
next: () => {
courier.fetch();
},
})
);

$scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', () => {
updateState();
refreshDashboardContainer();
});
$scope.timefilterSubscriptions$.add(
subscribeWithScope($scope, timefilter.getRefreshIntervalUpdate$(), {
next: () => {
updateState();
refreshDashboardContainer();
},
})
);

$scope.$listenAndDigestAsync(timefilter, 'timeUpdate', () => {
updateState();
refreshDashboardContainer();
});
$scope.timefilterSubscriptions$.add(
subscribeWithScope($scope, timefilter.getTimeUpdate$(), {
next: () => {
updateState();
refreshDashboardContainer();
},
})
);

function updateViewMode(newMode: ViewMode) {
$scope.topNavMenu = getTopNavConfig(
Expand Down Expand Up @@ -799,6 +813,8 @@ export class DashboardAppController {

$scope.$on('$destroy', () => {
updateSubscription.unsubscribe();
$scope.timefilterSubscriptions$.unsubscribe();

dashboardStateManager.destroy();
if (inputSubscription) {
inputSubscription.unsubscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ describe('DashboardState', function() {
getRefreshInterval: jest.fn(),
disableTimeRangeSelector: jest.fn(),
enableAutoRefreshSelector: jest.fn(),
off: jest.fn(),
on: jest.fn(),
getActiveBounds: () => {},
enableTimeRangeSelector: () => {},
isAutoRefreshSelectorEnabled: true,
isTimeRangeSelectorEnabled: true,
getAutoRefreshFetch$: jest.fn(),
getEnabledUpdated$: jest.fn(),
getRefreshIntervalUpdate$: jest.fn(),
getFetch$: jest.fn(),
getTimeUpdate$: jest.fn(),
};

function initDashboardState() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@

import _ from 'lodash';
import { AppState } from 'ui/state_management/app_state';
import { Timefilter } from 'ui/timefilter';
import { RefreshInterval } from 'ui/timefilter/timefilter';
import { Timefilter, RefreshInterval } from 'ui/timefilter';
import { FilterUtils } from './filter_utils';
import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { SearchSource } from 'ui/courier';
import { SavedObject } from 'ui/saved_objects/saved_object';
import moment from 'moment';
import { RefreshInterval } from 'ui/timefilter/timefilter';
import { RefreshInterval } from 'ui/timefilter';
import { Query } from 'src/legacy/core_plugins/data/public';
import { Filter } from '@kbn/es-query';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import _ from 'lodash';
import { i18n } from '@kbn/i18n';
import React from 'react';
import angular from 'angular';
import { Subscription } from 'rxjs';
import moment from 'moment';
import chrome from 'ui/chrome';
import dateMath from '@elastic/datemath';
Expand Down Expand Up @@ -208,8 +209,7 @@ function discoverController(
requests: new RequestAdapter()
};

let filterUpdateSubscription;
let filterFetchSubscription;
const subscriptions = new Subscription();

timefilter.disableTimeRangeSelector();
timefilter.disableAutoRefreshSelector();
Expand All @@ -235,8 +235,7 @@ function discoverController(
const savedSearch = $route.current.locals.savedSearch;
$scope.$on('$destroy', () => {
savedSearch.destroy();
if (filterFetchSubscription) filterFetchSubscription.unsubscribe();
if (filterUpdateSubscription) filterUpdateSubscription.unsubscribe();
subscriptions.unsubscribe();
});

const $appStatus = $scope.appStatus = this.appStatus = {
Expand Down Expand Up @@ -563,10 +562,19 @@ function discoverController(

$scope.updateDataSource()
.then(function () {
$scope.$listen(timefilter, 'autoRefreshFetch', $scope.fetch);
$scope.$listen(timefilter, 'refreshIntervalUpdate', $scope.updateRefreshInterval);
$scope.$listen(timefilter, 'timeUpdate', $scope.updateTime);
$scope.$listen(timefilter, 'fetch', $scope.fetch);
subscriptions.add(subscribeWithScope($scope, timefilter.getAutoRefreshFetch$(), {
next: $scope.fetch
}));

subscriptions.add(subscribeWithScope($scope, timefilter.getRefreshIntervalUpdate$(), {
next: $scope.updateRefreshInterval
}));
subscriptions.add(subscribeWithScope($scope, timefilter.getTimeUpdate$(), {
next: $scope.updateTime
}));
subscriptions.add(subscribeWithScope($scope, timefilter.getFetch$(), {
next: $scope.fetch
}));

$scope.$watchCollection('state.sort', function (sort) {
if (!sort) return;
Expand All @@ -579,19 +587,19 @@ function discoverController(
});

// update data source when filters update
filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), {
subscriptions.add(subscribeWithScope($scope, queryFilter.getUpdates$(), {
next: () => {
$scope.filters = queryFilter.getFilters();
$scope.updateDataSource().then(function () {
$state.save();
});
}
});
}));

// fetch data when filters fire fetch event
filterFetchSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), {
subscriptions.add(subscribeWithScope($scope, queryFilter.getUpdates$(), {
next: $scope.fetch
});
}));

// update data source when hitting forward/back and the query changes
$scope.$listen($state, 'fetch_with_changes', function (diff) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ import { SearchSource } from 'ui/courier';
import { StaticIndexPattern } from 'ui/index_patterns';
import { RequestAdapter } from 'ui/inspector/adapters';
import { Adapters } from 'ui/inspector/types';
import { getTime } from 'ui/timefilter/get_time';
import { Subscription } from 'rxjs';
import * as Rx from 'rxjs';
import { Filter, FilterStateStore } from '@kbn/es-query';
import { TimeRange } from 'ui/timefilter/time_history';
import { getTime, TimeRange } from 'ui/timefilter';
import { Query, onlyDisabledFiltersChanged } from '../../../../data/public';
import {
APPLY_FILTER_TRIGGER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { capabilities } from 'ui/capabilities';
import { i18n } from '@kbn/i18n';
import chrome from 'ui/chrome';
import { IPrivate } from 'ui/private';
import { TimeRange } from 'ui/timefilter/time_history';
import { TimeRange } from 'ui/timefilter';
import { FilterBarQueryFilterProvider } from 'ui/filter_manager/query_filter';
import {
EmbeddableFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { StaticIndexPattern } from 'ui/index_patterns';
import { TimeRange } from 'ui/timefilter/time_history';
import { TimeRange } from 'ui/timefilter';
import { Query } from 'src/legacy/core_plugins/data/public';
import { Filter } from '@kbn/es-query';
import { SavedSearch } from '../types';
Expand Down
26 changes: 15 additions & 11 deletions src/legacy/core_plugins/kibana/public/visualize/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

import _ from 'lodash';
import { Subscription } from 'rxjs';
import { i18n } from '@kbn/i18n';
import '../saved_visualizations/saved_visualizations';
import './visualization_editor';
Expand Down Expand Up @@ -403,12 +404,16 @@ function VisEditor(
}
};

const updateRefreshInterval = () => {
$scope.refreshInterval = timefilter.getRefreshInterval();
};
const subscriptions = new Subscription();

$scope.$listenAndDigestAsync(timefilter, 'timeUpdate', updateTimeRange);
$scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', updateRefreshInterval);
subscriptions.add(subscribeWithScope($scope, timefilter.getRefreshIntervalUpdate$(), {
next: () => {
$scope.refreshInterval = timefilter.getRefreshInterval();
}
}));
subscriptions.add(subscribeWithScope($scope, timefilter.getTimeUpdate$(), {
next: updateTimeRange
}));

// update the searchSource when query updates
$scope.fetch = function () {
Expand All @@ -419,24 +424,23 @@ function VisEditor(
};

// update the searchSource when filters update
const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), {
subscriptions.add(subscribeWithScope($scope, queryFilter.getUpdates$(), {
next: () => {
$scope.filters = queryFilter.getFilters();
$scope.globalFilters = queryFilter.getGlobalFilters();
}
});
const filterFetchSubscription = subscribeWithScope($scope, queryFilter.getFetches$(), {
}));
subscriptions.add(subscribeWithScope($scope, queryFilter.getFetches$(), {
next: $scope.fetch
});
}));

$scope.$on('$destroy', function () {
if ($scope._handler) {
$scope._handler.destroy();
}
savedVis.destroy();
stateMonitor.destroy();
filterUpdateSubscription.unsubscribe();
filterFetchSubscription.unsubscribe();
subscriptions.unsubscribe();
});

if (!$scope.chrome.getVisible()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
} from 'ui/visualize/loader/types';
import { Subscription } from 'rxjs';
import * as Rx from 'rxjs';
import { TimeRange } from 'ui/timefilter/time_history';
import { TimeRange } from 'ui/timefilter';
import { Filter } from '@kbn/es-query';
import {
EmbeddableInput,
Expand Down
2 changes: 1 addition & 1 deletion src/legacy/core_plugins/timelion/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ app.controller('timelion', function (
};

$scope.$listen($scope.state, 'fetch_with_changes', $scope.search);
$scope.$listen(timefilter, 'fetch', $scope.search);
timefilter.getFetch$().subscribe($scope.search);

$scope.opts = {
saveExpression: saveExpression,
Expand Down
4 changes: 3 additions & 1 deletion src/legacy/ui/public/courier/courier.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => {
}
};

$rootScope.$listen(timefilter, 'refreshIntervalUpdate', updateRefreshInterval);
const refreshIntervalSubscription = timefilter.getRefreshIntervalUpdate$().subscribe(updateRefreshInterval);

const closeOnFatal = _.once(() => {
// If there was a fatal error, then stop future searches. We want to use pause instead of
Expand All @@ -68,6 +68,8 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => {
if (searchRequestQueue.getCount()) {
throw new Error('Aborting all pending requests failed.');
}

refreshIntervalSubscription.unsubscribe();
});

addFatalErrorCallback(closeOnFatal);
Expand Down
2 changes: 1 addition & 1 deletion src/legacy/ui/public/courier/search_poll/search_poll.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export function SearchPollProvider(Private, Promise) {
// We use resolve() here instead of try() because the latter won't trigger a $digest
// when the promise resolves.
this._searchPromise = Promise.resolve().then(() => {
timefilter.emit('autoRefreshFetch');
timefilter.notifyShouldFetch();
const requests = searchRequestQueue.getInactive();

// The promise returned from fetchSearchRequests() only resolves when the requests complete.
Expand Down
17 changes: 0 additions & 17 deletions src/legacy/ui/public/directives/listen/listen.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,4 @@ uiModules.get('kibana')
emitter.off(eventName, handler);
});
};

/**
* Helper that registers an event listener, and removes that listener when
* the $scope is destroyed. Handler is executed inside $evalAsync, ensuring digest cycle is run after the handler
*
* @param {SimpleEmitter} emitter - the event emitter to listen to
* @param {string} eventName - the event name
* @param {Function} handler - the event handler
* @return {undefined}
*/
$rootScope.constructor.prototype.$listenAndDigestAsync = function (emitter, eventName, handler) {
const evalAsyncWrappedHandler = (...args) => {
this.$evalAsync(() => handler(args));
};
this.$listen(emitter, eventName, evalAsyncWrappedHandler);
};

});
Loading

0 comments on commit 7097cfb

Please sign in to comment.