Skip to content

Commit

Permalink
Remove abort signal from dashboard and move to handler, and abort fet…
Browse files Browse the repository at this point in the history
…ches when necessary
  • Loading branch information
lukasolson committed Jul 9, 2019
1 parent 04e1240 commit 0c429c1
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export interface DashboardAppScope extends ng.IScope {
getEmbeddableFactory: (type: string) => EmbeddableFactory;
getDashboardState: () => DashboardStateManager;
refresh: () => void;
abortSignal?: AbortSignal;
}

const app = uiModules.get('app/dashboard', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ export class DashboardAppController {
const panelActionsRegistry = Private(ContextMenuActionsRegistryProvider);
const getUnhashableStates = Private(getUnhashableStatesProvider);
const shareContextMenuExtensions = Private(ShareContextMenuExtensionsRegistryProvider);
const abortController = new AbortController();
$scope.abortSignal = abortController.signal;

// @ts-ignore This code is going away shortly.
panelActionsStore.initializeFromRegistry(panelActionsRegistry);
Expand Down Expand Up @@ -251,17 +249,15 @@ export class DashboardAppController {
!dashboardConfig.getHideWriteControls();

$scope.updateQueryAndFetch = function({ query, dateRange }) {
if (dateRange) {
timefilter.setTime(dateRange);
}

const oldQuery = $scope.model.query;
if (_.isEqual(oldQuery, query)) {
if (_.isEqual($scope.model.query, query) && _.isEqual($scope.model.timeRange, dateRange)) {
// The user can still request a reload in the query bar, even if the
// query is the same, and in that case, we have to explicitly ask for
// a reload, since no state changes will cause it.
dashboardStateManager.requestReload();
} else {
if (dateRange) {
timefilter.setTime(dateRange);
}
$scope.model.query = query;
dashboardStateManager.applyFilters($scope.model.query, $scope.model.filters);
}
Expand Down Expand Up @@ -569,7 +565,6 @@ export class DashboardAppController {
updateSubscription.unsubscribe();
fetchSubscription.unsubscribe();
dashboardStateManager.destroy();
abortController.abort();
});

if (
Expand Down
13 changes: 5 additions & 8 deletions src/legacy/ui/public/vis/editors/default/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,15 @@ const defaultEditor = function ($rootScope, $compile) {
}
this._loader = loader;
this._handler = this._loader.embedVisualizationWithSavedObject(visualizationEl, this.savedObj, {
uiState: uiState,
listenOnChange: false,
timeRange: timeRange,
filters: filters,
appState: appState,
uiState,
timeRange,
filters,
appState,
});
});
} else {
this._handler.update({
timeRange: timeRange,
filters: filters,
});
this._handler.update({ timeRange, filters });
}

});
Expand Down
9 changes: 7 additions & 2 deletions src/legacy/ui/public/vis/request_handlers/courier.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ const CourierRequestHandlerProvider = function () {
partialRows,
metricsAtAllLevels,
inspectorAdapters,
queryFilter
queryFilter,
abortSignal
}) {

// Create a new search source that inherits the original search source
// but has the appropriate timeRange applied via a filter.
// This is a temporary solution until we properly pass down all required
Expand Down Expand Up @@ -102,6 +102,11 @@ const CourierRequestHandlerProvider = function () {
request.stats(getRequestInspectorStats(requestSearchSource));

try {
if (abortSignal) {
abortSignal.addEventListener('abort', () => {
requestSearchSource.cancelQueued();
});
}
const response = await requestSearchSource.fetch();

searchSource.lastQuery = queryHash;
Expand Down
28 changes: 11 additions & 17 deletions src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ interface EmbeddedVisualizeHandlerParams extends VisualizeLoaderParams {
queryFilter: any;
autoFetch?: boolean;
pipelineDataLoader?: boolean;
abortSignal?: AbortSignal;
}

const RENDER_COMPLETE_EVENT = 'render_complete';
Expand Down Expand Up @@ -94,6 +93,12 @@ export class EmbeddedVisualizeHandler {

const forceFetch = this.shouldForceNextFetch;
this.shouldForceNextFetch = false;

// Abort any in-progress requests
if (this.abortController) this.abortController.abort();
this.abortController = new AbortController();
this.dataLoaderParams.abortSignal = this.abortController.signal;

this.fetch(forceFetch).then(this.render);
}, 100);

Expand All @@ -105,6 +110,7 @@ export class EmbeddedVisualizeHandler {
private actions: any = {};
private events$: Rx.Observable<any>;
private autoFetch: boolean;
private abortController?: AbortController;

constructor(
private readonly element: HTMLElement,
Expand All @@ -123,7 +129,6 @@ export class EmbeddedVisualizeHandler {
autoFetch = true,
pipelineDataLoader = false,
Private,
abortSignal,
} = params;

this.dataLoaderParams = {
Expand All @@ -135,7 +140,6 @@ export class EmbeddedVisualizeHandler {
uiState,
aggs: vis.getAggConfig(),
forceFetch: false,
abortSignal,
};

this.pipelineDataLoader = pipelineDataLoader;
Expand Down Expand Up @@ -226,7 +230,7 @@ export class EmbeddedVisualizeHandler {
*/
public update(params: VisualizeUpdateParams = {}) {
// Apply data- attributes to the element if specified
const dataAttrs = params.dataAttrs;
const { dataAttrs, ...otherParams } = params;
if (dataAttrs) {
Object.keys(dataAttrs).forEach(key => {
if (dataAttrs[key] === null) {
Expand All @@ -238,19 +242,8 @@ export class EmbeddedVisualizeHandler {
});
}

let fetchRequired = false;
if (params.hasOwnProperty('timeRange')) {
fetchRequired = true;
this.dataLoaderParams.timeRange = params.timeRange;
}
if (params.hasOwnProperty('filters')) {
fetchRequired = true;
this.dataLoaderParams.filters = params.filters;
}
if (params.hasOwnProperty('query')) {
fetchRequired = true;
this.dataLoaderParams.query = params.query;
}
const fetchRequired = ['timeRange', 'filters', 'query'].some(key => params.hasOwnProperty(key));
this.dataLoaderParams = { ...this.dataLoaderParams, ...otherParams };

if (fetchRequired) {
this.fetchAndRender();
Expand All @@ -263,6 +256,7 @@ export class EmbeddedVisualizeHandler {
*/
public destroy(): void {
this.destroyed = true;
if (this.abortController) this.abortController.abort();
this.debouncedFetchAndRender.cancel();
if (this.autoFetch) {
timefilter.off('autoRefreshFetch', this.reload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function interpreterProvider(config: any) {

// if execution was aborted return error
if (handlers.abortSignal && handlers.abortSignal.aborted) {
return createError(`abort was requested`);
return new Error(`abort was requested`);
}

// Continue re-invoking chain until it's empty
Expand Down

0 comments on commit 0c429c1

Please sign in to comment.