From 0ec83038783ba73d410fda74bd34c642906bb1a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 1 Oct 2019 10:22:41 +0200 Subject: [PATCH] Panels: Skip re-rendering panel/visualisation in loading state (#19518) * Loading states and partial rendering, set loading state in mixed data source, and do not render loading states for react panels * Updated mixed data source tests --- .../dashboard/dashgrid/PanelChrome.tsx | 45 +++++++++------- .../datasource/mixed/MixedDataSource.test.ts | 9 +++- .../datasource/mixed/MixedDataSource.ts | 52 ++++++++++++------- public/test/mocks/datasource_srv.ts | 6 ++- 4 files changed, 73 insertions(+), 39 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 0d7dadaa928..183240e7a56 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -115,27 +115,34 @@ export class PanelChrome extends PureComponent { return; } - let { errorMessage, isFirstLoad } = this.state; + let { isFirstLoad } = this.state; + let errorMessage: string | null = null; - if (data.state === LoadingState.Error) { - const { error } = data; - if (error) { - if (errorMessage !== error.message) { - errorMessage = error.message; + switch (data.state) { + case LoadingState.Loading: + // Skip updating state data if it is already in loading state + // This is to avoid rendering partial loading responses + if (this.state.data.state === LoadingState.Loading) { + return; } - } - } else { - errorMessage = null; - } - - if (data.state === LoadingState.Done) { - // If we are doing a snapshot save data in panel model - if (this.props.dashboard.snapshot) { - this.props.panel.snapshotData = data.series.map(frame => toDataFrameDTO(frame)); - } - if (isFirstLoad) { - isFirstLoad = false; - } + break; + case LoadingState.Error: + const { error } = data; + if (error) { + if (errorMessage !== error.message) { + errorMessage = error.message; + } + } + break; + case LoadingState.Done: + // If we are doing a snapshot save data in panel model + if (this.props.dashboard.snapshot) { + this.props.panel.snapshotData = data.series.map(frame => toDataFrameDTO(frame)); + } + if (isFirstLoad) { + isFirstLoad = false; + } + break; } this.setState({ isFirstLoad, errorMessage, data }); diff --git a/public/app/plugins/datasource/mixed/MixedDataSource.test.ts b/public/app/plugins/datasource/mixed/MixedDataSource.test.ts index c20aab66f37..42fea21f0cf 100644 --- a/public/app/plugins/datasource/mixed/MixedDataSource.test.ts +++ b/public/app/plugins/datasource/mixed/MixedDataSource.test.ts @@ -2,6 +2,7 @@ import { DatasourceSrvMock, MockDataSourceApi } from 'test/mocks/datasource_srv' import { getDataSourceSrv } from '@grafana/runtime'; import { getQueryOptions } from 'test/helpers/getQueryOptions'; import { DataSourceInstanceSettings } from '@grafana/ui'; +import { LoadingState } from '@grafana/data'; import { MixedDatasource } from './module'; import { from } from 'rxjs'; @@ -29,17 +30,23 @@ describe('MixedDatasource', () => { }); const results: any[] = []; - beforeEach(async () => { + beforeEach(async done => { const ds = await getDataSourceSrv().get('-- Mixed --'); + from(ds.query(requestMixed)).subscribe(result => { results.push(result); + if (result.state === LoadingState.Done) { + done(); + } }); }); it('direct query should return results', async () => { expect(results.length).toBe(3); expect(results[0].data).toEqual(['AAAA']); + expect(results[0].state).toEqual(LoadingState.Loading); expect(results[1].data).toEqual(['BBBB']); expect(results[2].data).toEqual(['CCCC']); + expect(results[2].state).toEqual(LoadingState.Done); }); }); diff --git a/public/app/plugins/datasource/mixed/MixedDataSource.ts b/public/app/plugins/datasource/mixed/MixedDataSource.ts index 0962bc1c498..fe3602a3247 100644 --- a/public/app/plugins/datasource/mixed/MixedDataSource.ts +++ b/public/app/plugins/datasource/mixed/MixedDataSource.ts @@ -1,10 +1,12 @@ import cloneDeep from 'lodash/cloneDeep'; import groupBy from 'lodash/groupBy'; import { from, of, Observable, merge } from 'rxjs'; +import { tap } from 'rxjs/operators'; +import { LoadingState } from '@grafana/data'; import { DataSourceApi, DataQuery, DataQueryRequest, DataQueryResponse, DataSourceInstanceSettings } from '@grafana/ui'; import { getDataSourceSrv } from '@grafana/runtime'; -import { mergeMap, map, filter } from 'rxjs/operators'; +import { mergeMap, map } from 'rxjs/operators'; export const MIXED_DATASOURCE_NAME = '-- Mixed --'; @@ -25,13 +27,14 @@ export class MixedDatasource extends DataSourceApi { const sets: { [key: string]: DataQuery[] } = groupBy(queries, 'datasource'); const observables: Array> = []; + let runningSubRequests = 0; for (const key in sets) { const targets = sets[key]; const dsName = targets[0].datasource; const observable = from(getDataSourceSrv().get(dsName)).pipe( - map((dataSourceApi: DataSourceApi) => { + mergeMap((dataSourceApi: DataSourceApi) => { const datasourceRequest = cloneDeep(request); // Remove any unused hidden queries @@ -42,28 +45,41 @@ export class MixedDatasource extends DataSourceApi { datasourceRequest.targets = newTargets; datasourceRequest.requestId = `${dsName}${datasourceRequest.requestId || ''}`; - return { - dataSourceApi, - datasourceRequest, - }; - }) - ); - const noTargets = observable.pipe( - filter(({ datasourceRequest }) => datasourceRequest.targets.length === 0), - mergeMap(() => { - return of({ data: [] } as DataQueryResponse); - }) - ); + // all queries hidden return empty result for for this requestId + if (datasourceRequest.targets.length === 0) { + return of({ data: [], key: datasourceRequest.requestId }); + } + + runningSubRequests++; + let hasCountedAsDone = false; - const hasTargets = observable.pipe( - filter(({ datasourceRequest }) => datasourceRequest.targets.length > 0), - mergeMap(({ dataSourceApi, datasourceRequest }) => { return from(dataSourceApi.query(datasourceRequest)).pipe( + tap( + (response: DataQueryResponse) => { + if ( + hasCountedAsDone || + response.state === LoadingState.Streaming || + response.state === LoadingState.Loading + ) { + return; + } + runningSubRequests--; + hasCountedAsDone = true; + }, + () => { + if (hasCountedAsDone) { + return; + } + hasCountedAsDone = true; + runningSubRequests--; + } + ), map((response: DataQueryResponse) => { return { ...response, data: response.data || [], + state: runningSubRequests === 0 ? LoadingState.Done : LoadingState.Loading, key: `${dsName}${response.key || ''}`, } as DataQueryResponse; }) @@ -71,7 +87,7 @@ export class MixedDatasource extends DataSourceApi { }) ); - observables.push(merge(noTargets, hasTargets)); + observables.push(observable); } return merge(...observables); diff --git a/public/test/mocks/datasource_srv.ts b/public/test/mocks/datasource_srv.ts index 3f71bfda4d2..5677ef68bd4 100644 --- a/public/test/mocks/datasource_srv.ts +++ b/public/test/mocks/datasource_srv.ts @@ -40,7 +40,11 @@ export class MockDataSourceApi extends DataSourceApi { if (this.queryResolver) { return this.queryResolver; } - return Promise.resolve(this.result); + return new Promise(resolver => { + setTimeout(() => { + resolver(this.result); + }); + }); } testDatasource() {