diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 3acab6ed56d..74bc37c1d3c 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -53,6 +53,8 @@ enum CancellationType { dataSourceRequest, } +const CANCEL_ALL_REQUESTS_REQUEST_ID = 'cancel_all_requests_request_id'; + export interface BackendSrvDependencies { fromFetch: (input: string | Request, init?: RequestInit) => Observable; appEvents: Emitter; @@ -182,6 +184,10 @@ export class BackendSrv implements BackendService { this.inFlightRequests.next(requestId); } + cancelAllInFlightRequests() { + this.inFlightRequests.next(CANCEL_ALL_REQUESTS_REQUEST_ID); + } + async datasourceRequest(options: BackendSrvRequest): Promise { // A requestId is provided by the datasource as a unique identifier for a // particular query. Every observable below has a takeUntil that subscribes to this.inFlightRequests and @@ -528,12 +534,18 @@ export class BackendSrv implements BackendService { this.inFlightRequests.pipe( filter(requestId => { let cancelRequest = false; + if (options && options.requestId && options.requestId === requestId) { // when a new requestId is started it will be published to inFlightRequests // if a previous long running request that hasn't finished yet has the same requestId // we need to cancel that request cancelRequest = true; } + + if (requestId === CANCEL_ALL_REQUESTS_REQUEST_ID) { + cancelRequest = true; + } + return cancelRequest; }) ) diff --git a/public/app/core/specs/backend_srv.test.ts b/public/app/core/specs/backend_srv.test.ts index 9313a2d592d..764102ad3a7 100644 --- a/public/app/core/specs/backend_srv.test.ts +++ b/public/app/core/specs/backend_srv.test.ts @@ -7,6 +7,7 @@ import { BackendSrv, getBackendSrv } from '../services/backend_srv'; import { Emitter } from '../utils/emitter'; import { ContextSrv, User } from '../services/context_srv'; import { CoreEvents } from '../../types'; +import { describe, expect } from '../../../test/lib/common'; const getTestContext = (overides?: object) => { const defaults = { @@ -571,4 +572,87 @@ describe('backendSrv', () => { }); }); }); + + describe('cancelAllInFlightRequests', () => { + describe('when called with 2 separate requests and then cancelAllInFlightRequests is called', () => { + enum RequestType { + request, + dataSourceRequest, + } + + const url = '/api/dashboard/'; + const options = { + url, + method: 'GET', + }; + + const dataSourceRequestResult = { + data: ([] as unknown[]) as any[], + status: -1, + statusText: 'Request was aborted', + config: options, + }; + + const getRequestObservable = (message: string, unsubscribe: any) => + new Observable(subscriber => { + subscriber.next({ + ok: true, + status: 200, + statusText: 'Ok', + text: () => Promise.resolve(JSON.stringify({ message })), + headers: { + map: { + 'content-type': 'application/json', + }, + }, + redirected: false, + type: 'basic', + url, + }); + return unsubscribe; + }).pipe(delay(10000)); + + it.each` + firstRequestType | secondRequestType | firstRequestResult | secondRequestResult + ${RequestType.request} | ${RequestType.request} | ${[]} | ${[]} + ${RequestType.dataSourceRequest} | ${RequestType.dataSourceRequest} | ${dataSourceRequestResult} | ${dataSourceRequestResult} + ${RequestType.request} | ${RequestType.dataSourceRequest} | ${[]} | ${dataSourceRequestResult} + ${RequestType.dataSourceRequest} | ${RequestType.request} | ${dataSourceRequestResult} | ${[]} + `( + 'then it both requests should be cancelled and unsubscribed', + async ({ firstRequestType, secondRequestType, firstRequestResult, secondRequestResult }) => { + const unsubscribe = jest.fn(); + const { backendSrv, fromFetchMock } = getTestContext({ url }); + const firstObservable = getRequestObservable('First', unsubscribe); + const secondObservable = getRequestObservable('Second', unsubscribe); + + fromFetchMock.mockImplementationOnce(() => firstObservable); + fromFetchMock.mockImplementation(() => secondObservable); + + const options = { + url, + method: 'GET', + }; + + const firstRequest = + firstRequestType === RequestType.request + ? backendSrv.request(options) + : backendSrv.datasourceRequest(options); + + const secondRequest = + secondRequestType === RequestType.request + ? backendSrv.request(options) + : backendSrv.datasourceRequest(options); + + backendSrv.cancelAllInFlightRequests(); + + const result = await Promise.all([firstRequest, secondRequest]); + + expect(result[0]).toEqual(firstRequestResult); + expect(result[1]).toEqual(secondRequestResult); + expect(unsubscribe).toHaveBeenCalledTimes(2); + } + ); + }); + }); }); diff --git a/public/app/features/dashboard/containers/DashboardPage.test.tsx b/public/app/features/dashboard/containers/DashboardPage.test.tsx index 8c977cd7509..0644de12a63 100644 --- a/public/app/features/dashboard/containers/DashboardPage.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.test.tsx @@ -2,19 +2,15 @@ import React from 'react'; import { shallow, ShallowWrapper } from 'enzyme'; import { DashboardPage, mapStateToProps, Props, State } from './DashboardPage'; import { DashboardModel } from '../state'; -import { cleanUpDashboard } from '../state/reducers'; -import { - mockToolkitActionCreator, - mockToolkitActionCreatorWithoutPayload, - ToolkitActionCreatorWithoutPayloadMockType, -} from 'test/core/redux/mocks'; +import { mockToolkitActionCreator } from 'test/core/redux/mocks'; import { DashboardInitPhase, DashboardRouteInfo } from 'app/types'; import { notifyApp, updateLocation } from 'app/core/actions'; +import { cleanUpDashboardAndVariables } from '../state/actions'; jest.mock('app/features/dashboard/components/DashboardSettings/SettingsCtrl', () => ({})); interface ScenarioContext { - cleanUpDashboardMock: ToolkitActionCreatorWithoutPayloadMockType; + cleanUpDashboardAndVariablesMock: typeof cleanUpDashboardAndVariables; dashboard?: DashboardModel | null; setDashboardProp: (overrides?: any, metaOverrides?: any) => void; wrapper?: ShallowWrapper; @@ -47,7 +43,7 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo let setupFn: () => void; const ctx: ScenarioContext = { - cleanUpDashboardMock: mockToolkitActionCreatorWithoutPayload(cleanUpDashboard), + cleanUpDashboardAndVariablesMock: jest.fn(), setup: fn => { setupFn = fn; }, @@ -67,7 +63,8 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo initDashboard: jest.fn(), updateLocation: mockToolkitActionCreator(updateLocation), notifyApp: mockToolkitActionCreator(notifyApp), - cleanUpDashboard: ctx.cleanUpDashboardMock, + cleanUpDashboardAndVariables: ctx.cleanUpDashboardAndVariablesMock, + cancelVariables: jest.fn(), dashboard: null, }; @@ -233,7 +230,7 @@ describe('DashboardPage', () => { }); it('Should call clean up action', () => { - expect(ctx.cleanUpDashboardMock).toHaveBeenCalledTimes(1); + expect(ctx.cleanUpDashboardAndVariablesMock).toHaveBeenCalledTimes(1); }); }); diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index 03bf51af8ae..7e99a7cc5b0 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -8,16 +8,14 @@ import classNames from 'classnames'; import { createErrorNotification } from 'app/core/copy/appNotification'; import { getMessageFromError } from 'app/core/utils/errors'; import { Branding } from 'app/core/components/Branding/Branding'; - // Components import { DashboardGrid } from '../dashgrid/DashboardGrid'; import { DashNav } from '../components/DashNav'; import { DashboardSettings } from '../components/DashboardSettings'; import { PanelEditor } from '../components/PanelEditor/PanelEditor'; -import { Alert, CustomScrollbar, Icon } from '@grafana/ui'; +import { Alert, Button, CustomScrollbar, HorizontalGroup, Icon, VerticalGroup } from '@grafana/ui'; // Redux import { initDashboard } from '../state/initDashboard'; -import { cleanUpDashboard } from '../state/reducers'; import { notifyApp, updateLocation } from 'app/core/actions'; // Types import { @@ -32,6 +30,8 @@ import { DashboardModel, PanelModel } from 'app/features/dashboard/state'; import { InspectTab, PanelInspector } from '../components/Inspector/PanelInspector'; import { getConfig } from '../../../core/config'; import { SubMenu } from '../components/SubMenu/SubMenu'; +import { cleanUpDashboardAndVariables } from '../state/actions'; +import { cancelVariables } from '../../variables/state/actions'; export interface Props { urlUid?: string; @@ -51,11 +51,12 @@ export interface Props { dashboard: DashboardModel | null; initError?: DashboardInitError; initDashboard: typeof initDashboard; - cleanUpDashboard: typeof cleanUpDashboard; + cleanUpDashboardAndVariables: typeof cleanUpDashboardAndVariables; notifyApp: typeof notifyApp; updateLocation: typeof updateLocation; inspectTab?: InspectTab; isPanelEditorOpen?: boolean; + cancelVariables: typeof cancelVariables; } export interface State { @@ -90,10 +91,8 @@ export class DashboardPage extends PureComponent { } componentWillUnmount() { - if (this.props.dashboard) { - this.props.cleanUpDashboard(); - this.setPanelFullscreenClass(false); - } + this.props.cleanUpDashboardAndVariables(); + this.setPanelFullscreenClass(false); } componentDidUpdate(prevProps: Props) { @@ -210,11 +209,24 @@ export class DashboardPage extends PureComponent { this.setState({ updateScrollTop: 0 }); }; + cancelVariables = () => { + this.props.updateLocation({ path: '/' }); + }; + renderSlowInitState() { return (
- {this.props.initPhase} + + + {this.props.initPhase} + {' '} + + + +
); @@ -336,9 +348,10 @@ export const mapStateToProps = (state: StoreState) => ({ const mapDispatchToProps = { initDashboard, - cleanUpDashboard, + cleanUpDashboardAndVariables, notifyApp, updateLocation, + cancelVariables, }; export default hot(module)(connect(mapStateToProps, mapDispatchToProps)(DashboardPage)); diff --git a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap index 12c84b96049..0f55367eea4 100644 --- a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap +++ b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap @@ -320,12 +320,36 @@ exports[`DashboardPage Dashboard is fetching slowly Should render slow init stat
- - - Fetching + + + + + Fetching + + + + + +
`; diff --git a/public/app/features/dashboard/state/actions.ts b/public/app/features/dashboard/state/actions.ts index e53d7e034e1..6395449ac41 100644 --- a/public/app/features/dashboard/state/actions.ts +++ b/public/app/features/dashboard/state/actions.ts @@ -3,12 +3,18 @@ import { getBackendSrv } from '@grafana/runtime'; import { createSuccessNotification } from 'app/core/copy/appNotification'; // Actions import { loadPluginDashboards } from '../../plugins/state/actions'; -import { loadDashboardPermissions, panelModelAndPluginReady, setPanelAngularComponent } from './reducers'; +import { + cleanUpDashboard, + loadDashboardPermissions, + panelModelAndPluginReady, + setPanelAngularComponent, +} from './reducers'; import { notifyApp } from 'app/core/actions'; import { loadPanelPlugin } from 'app/features/plugins/state/actions'; // Types import { DashboardAcl, DashboardAclUpdateDTO, NewDashboardAclItem, PermissionLevel, ThunkResult } from 'app/types'; import { PanelModel } from './PanelModel'; +import { cancelVariables } from '../../variables/state/actions'; export function getDashboardPermissions(id: number): ThunkResult { return async dispatch => { @@ -153,3 +159,8 @@ export function changePanelPlugin(panel: PanelModel, pluginId: string): ThunkRes dispatch(panelModelAndPluginReady({ panelId: panel.id, plugin })); }; } + +export const cleanUpDashboardAndVariables = (): ThunkResult => dispatch => { + dispatch(cleanUpDashboard()); + dispatch(cancelVariables()); +}; diff --git a/public/app/features/dashboard/state/initDashboard.test.ts b/public/app/features/dashboard/state/initDashboard.test.ts index 6cce0b21662..9258f41ca9d 100644 --- a/public/app/features/dashboard/state/initDashboard.test.ts +++ b/public/app/features/dashboard/state/initDashboard.test.ts @@ -1,7 +1,7 @@ import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { initDashboard, InitDashboardArgs } from './initDashboard'; -import { DashboardRouteInfo } from 'app/types'; +import { DashboardRouteInfo, DashboardInitPhase } from 'app/types'; import { getBackendSrv } from 'app/core/services/backend_srv'; import { dashboardInitCompleted, dashboardInitFetching, dashboardInitServices } from './reducers'; import { updateLocation } from '../../../core/actions'; @@ -10,8 +10,8 @@ import { Echo } from '../../../core/services/echo/Echo'; import { getConfig } from 'app/core/config'; import { variableAdapters } from 'app/features/variables/adapters'; import { createConstantVariableAdapter } from 'app/features/variables/constant/adapter'; -import { addVariable } from 'app/features/variables/state/sharedReducer'; import { constantBuilder } from 'app/features/variables/shared/testing/builders'; +import { TransactionStatus, variablesInitTransaction } from '../../variables/state/transactionReducer'; jest.mock('app/core/services/backend_srv'); jest.mock('app/features/dashboard/services/TimeSrv', () => { @@ -116,6 +116,7 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) { const ctx: ScenarioContext = { args: { + urlUid: 'DGmvKKxZz', $injector: injectorMock, $scope: {}, fixUrl: false, @@ -134,7 +135,9 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) { location: { query: {}, }, - dashboard: {}, + dashboard: { + initPhase: DashboardInitPhase.Services, + }, user: {}, explore: { left: { @@ -144,6 +147,7 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) { }, templating: { variables: {}, + transaction: { uid: 'DGmvKKxZz', status: TransactionStatus.Completed }, }, }, setup: (fn: () => void) => { @@ -186,8 +190,8 @@ describeInitScenario('Initializing new dashboard', ctx => { }); it('Should send action dashboardInitCompleted', () => { - expect(ctx.actions[3].type).toBe(dashboardInitCompleted.type); - expect(ctx.actions[3].payload.title).toBe('New dashboard'); + expect(ctx.actions[5].type).toBe(dashboardInitCompleted.type); + expect(ctx.actions[5].payload.title).toBe('New dashboard'); }); it('Should initialize services', () => { @@ -209,11 +213,10 @@ describeInitScenario('Initializing new dashboard', ctx => { describeInitScenario('Initializing home dashboard', ctx => { ctx.setup(() => { ctx.args.routeInfo = DashboardRouteInfo.Home; - ctx.backendSrv.get.mockReturnValue( - Promise.resolve({ - redirectUri: '/u/123/my-home', - }) - ); + ctx.backendSrv.get.mockResolvedValue({ + meta: {}, + redirectUri: '/u/123/my-home', + }); }); it('Should redirect to custom home dashboard', () => { @@ -257,7 +260,7 @@ describeInitScenario('Initializing existing dashboard', ctx => { }); it('Should send action dashboardInitCompleted', () => { - const index = getConfig().featureToggles.newVariables ? 4 : 3; + const index = getConfig().featureToggles.newVariables ? 6 : 5; expect(ctx.actions[index].type).toBe(dashboardInitCompleted.type); expect(ctx.actions[index].payload.title).toBe('My cool dashboard'); }); @@ -281,6 +284,38 @@ describeInitScenario('Initializing existing dashboard', ctx => { if (!getConfig().featureToggles.newVariables) { return expect.assertions(0); } - expect(ctx.actions[3].type).toBe(addVariable.type); + expect(ctx.actions[3].type).toBe(variablesInitTransaction.type); + }); +}); + +describeInitScenario('Initializing previously canceled dashboard initialization', ctx => { + ctx.setup(() => { + ctx.storeState.dashboard.initPhase = DashboardInitPhase.Fetching; + }); + + it('Should send action dashboardInitFetching', () => { + expect(ctx.actions[0].type).toBe(dashboardInitFetching.type); + }); + + it('Should send action dashboardInitServices ', () => { + expect(ctx.actions[1].type).toBe(dashboardInitServices.type); + }); + + it('Should not send action dashboardInitCompleted', () => { + const dashboardInitCompletedAction = ctx.actions.find(a => { + return a.type === dashboardInitCompleted.type; + }); + expect(dashboardInitCompletedAction).toBe(undefined); + }); + + it('Should initialize timeSrv and annotationsSrv', () => { + expect(ctx.timeSrv.init).toBeCalled(); + expect(ctx.annotationsSrv.init).toBeCalled(); + }); + + it('Should not initialize other services', () => { + expect(ctx.unsavedChangesSrv.init).not.toBeCalled(); + expect(ctx.keybindingSrv.setupDashboardBindings).not.toBeCalled(); + expect(ctx.dashboardSrv.setCurrent).not.toBeCalled(); }); }); diff --git a/public/app/features/dashboard/state/initDashboard.ts b/public/app/features/dashboard/state/initDashboard.ts index 9862d32c579..93b499bc277 100644 --- a/public/app/features/dashboard/state/initDashboard.ts +++ b/public/app/features/dashboard/state/initDashboard.ts @@ -18,11 +18,17 @@ import { dashboardInitSlow, } from './reducers'; // Types -import { DashboardDTO, DashboardRouteInfo, StoreState, ThunkDispatch, ThunkResult } from 'app/types'; +import { + DashboardDTO, + DashboardRouteInfo, + StoreState, + ThunkDispatch, + ThunkResult, + DashboardInitPhase, +} from 'app/types'; import { DashboardModel } from './DashboardModel'; import { DataQuery, locationUtil } from '@grafana/data'; -import { getConfig } from '../../../core/config'; -import { initDashboardTemplating, processVariables, completeDashboardTemplating } from '../../variables/state/actions'; +import { initVariablesTransaction } from '../../variables/state/actions'; import { emitDashboardViewEvent } from './analyticsProcessor'; export interface InitDashboardArgs { @@ -63,6 +69,11 @@ async function fetchDashboard( // load home dash const dashDTO: DashboardDTO = await backendSrv.get('/api/dashboards/home'); + // if above all is cancelled it will return an array + if (!dashDTO.meta) { + return null; + } + // if user specified a custom home dashboard redirect to that if (dashDTO.redirectUri) { const newUrl = locationUtil.stripBaseFromUrl(dashDTO.redirectUri); @@ -177,20 +188,19 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult { dashboard.meta.fromExplore = !!(panelId && queries); } - // template values service needs to initialize completely before - // the rest of the dashboard can load - try { - if (!getConfig().featureToggles.newVariables) { - await variableSrv.init(dashboard); - } - if (getConfig().featureToggles.newVariables) { - dispatch(initDashboardTemplating(dashboard.templating.list)); - await dispatch(processVariables()); - dispatch(completeDashboardTemplating(dashboard)); - } - } catch (err) { - dispatch(notifyApp(createErrorNotification('Templating init failed', err))); - console.log(err); + // template values service needs to initialize completely before the rest of the dashboard can load + await dispatch(initVariablesTransaction(args.urlUid, dashboard, variableSrv)); + + if (getState().templating.transaction.uid !== args.urlUid) { + // if a previous dashboard has slow running variable queries the batch uid will be the new one + // but the args.urlUid will be the same as before initVariablesTransaction was called so then we can't continue initializing + // the previous dashboard. + return; + } + + // If dashboard is in a different init phase it means it cancelled during service init + if (getState().dashboard.initPhase !== DashboardInitPhase.Services) { + return; } try { diff --git a/public/app/features/dashboard/state/reducers.ts b/public/app/features/dashboard/state/reducers.ts index 8c2aed70873..0460b9c161a 100644 --- a/public/app/features/dashboard/state/reducers.ts +++ b/public/app/features/dashboard/state/reducers.ts @@ -1,9 +1,9 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { - DashboardInitPhase, - DashboardState, DashboardAclDTO, DashboardInitError, + DashboardInitPhase, + DashboardState, PanelState, QueriesToUpdateOnDashboardLoad, } from 'app/types'; diff --git a/public/app/features/variables/query/actions.ts b/public/app/features/variables/query/actions.ts index 89121c64834..82459fbd878 100644 --- a/public/app/features/variables/query/actions.ts +++ b/public/app/features/variables/query/actions.ts @@ -22,6 +22,7 @@ export const updateQueryVariableOptions = ( return async (dispatch, getState) => { const variableInState = getVariable(identifier.id!, getState()); try { + const beforeUid = getState().templating.transaction.uid; if (getState().templating.editor.id === variableInState.id) { dispatch(removeVariableEditorError({ errorProp: 'update' })); } @@ -36,6 +37,13 @@ export const updateQueryVariableOptions = ( } const results = await dataSource.metricFindQuery(variableInState.query, queryOptions); + + const afterUid = getState().templating.transaction.uid; + if (beforeUid !== afterUid) { + // we started another batch before this metricFindQuery finished let's abort + return; + } + const templatedRegex = getTemplatedRegex(variableInState); await dispatch(updateVariableOptions(toVariablePayload(variableInState, { results, templatedRegex }))); diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index 7e1858b3788..3390ed2f063 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -1,7 +1,7 @@ import { AnyAction } from 'redux'; import { UrlQueryMap } from '@grafana/data'; -import { getTemplatingAndLocationRootReducer, getTemplatingRootReducer } from './helpers'; +import { getRootReducer, getTemplatingAndLocationRootReducer, getTemplatingRootReducer } from './helpers'; import { variableAdapters } from '../adapters'; import { createQueryVariableAdapter } from '../query/adapter'; import { createCustomVariableAdapter } from '../custom/adapter'; @@ -10,8 +10,11 @@ import { createConstantVariableAdapter } from '../constant/adapter'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { TemplatingState } from 'app/features/variables/state/reducers'; import { + cancelVariables, changeVariableMultiValue, + cleanUpVariables, initDashboardTemplating, + initVariablesTransaction, processVariables, setOptionFromUrl, validateVariableSelectionState, @@ -34,7 +37,22 @@ import { textboxBuilder, } from '../shared/testing/builders'; import { changeVariableName } from '../editor/actions'; -import { changeVariableNameFailed, changeVariableNameSucceeded, setIdInEditor } from '../editor/reducer'; +import { + changeVariableNameFailed, + changeVariableNameSucceeded, + initialVariableEditorState, + setIdInEditor, +} from '../editor/reducer'; +import { DashboardState, LocationState } from '../../../types'; +import { + TransactionStatus, + variablesClearTransaction, + variablesCompleteTransaction, + variablesInitTransaction, +} from './transactionReducer'; +import { initialState } from '../pickers/OptionsPicker/reducer'; +import { cleanVariables } from './variablesReducer'; +import { expect } from '../../../../test/lib/common'; variableAdapters.setInit(() => [ createQueryVariableAdapter(), @@ -527,4 +545,96 @@ describe('shared actions', () => { }); }); }); + + describe('initVariablesTransaction', () => { + type ReducersUsedInContext = { + templating: TemplatingState; + dashboard: DashboardState; + location: LocationState; + }; + const constant = constantBuilder() + .withId('constant') + .withName('constant') + .build(); + const templating: any = { list: [constant] }; + const uid = 'uid'; + const dashboard: any = { title: 'Some dash', uid, templating }; + const variableSrv: any = {}; + + describe('when called and the previous dashboard has completed', () => { + it('then correct actions are dispatched', async () => { + const tester = await reduxTester() + .givenRootReducer(getRootReducer()) + .whenAsyncActionIsDispatched(initVariablesTransaction(uid, dashboard, variableSrv)); + + tester.thenDispatchedActionsShouldEqual( + variablesInitTransaction({ uid }), + addVariable(toVariablePayload(constant, { global: false, index: 0, model: constant })), + addInitLock(toVariablePayload(constant)), + resolveInitLock(toVariablePayload(constant)), + removeInitLock(toVariablePayload(constant)), + variablesCompleteTransaction({ uid }) + ); + }); + }); + + describe('when called and the previous dashboard is still processing variables', () => { + it('then correct actions are dispatched', async () => { + const transactionState = { uid: 'previous-uid', status: TransactionStatus.Fetching }; + + const tester = await reduxTester({ + preloadedState: ({ + templating: { + transaction: transactionState, + variables: {}, + optionsPicker: { ...initialState }, + editor: { ...initialVariableEditorState }, + }, + } as unknown) as ReducersUsedInContext, + }) + .givenRootReducer(getRootReducer()) + .whenAsyncActionIsDispatched(initVariablesTransaction(uid, dashboard, variableSrv)); + + tester.thenDispatchedActionsShouldEqual( + cleanVariables(), + variablesClearTransaction(), + variablesInitTransaction({ uid }), + addVariable(toVariablePayload(constant, { global: false, index: 0, model: constant })), + addInitLock(toVariablePayload(constant)), + resolveInitLock(toVariablePayload(constant)), + removeInitLock(toVariablePayload(constant)), + variablesCompleteTransaction({ uid }) + ); + }); + }); + }); + + describe('cleanUpVariables', () => { + describe('when called', () => { + it('then correct actions are dispatched', async () => { + reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(cleanUpVariables()) + .thenDispatchedActionsShouldEqual(cleanVariables(), variablesClearTransaction()); + }); + }); + }); + + describe('cancelVariables', () => { + const cancelAllInFlightRequestsMock = jest.fn(); + const backendSrvMock: any = { + cancelAllInFlightRequests: cancelAllInFlightRequestsMock, + }; + + describe('when called', () => { + it('then cancelAllInFlightRequests should be called and correct actions are dispatched', async () => { + reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(cancelVariables({ getBackendSrv: () => backendSrvMock })) + .thenDispatchedActionsShouldEqual(cleanVariables(), variablesClearTransaction()); + + expect(cancelAllInFlightRequestsMock).toHaveBeenCalledTimes(1); + }); + }); + }); }); diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index 5ef0915491e..04f3f87e38a 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -14,7 +14,7 @@ import { StoreState, ThunkResult } from '../../../types'; import { getVariable, getVariables } from './selectors'; import { variableAdapters } from '../adapters'; import { Graph } from '../../../core/utils/dag'; -import { updateLocation } from 'app/core/actions'; +import { notifyApp, updateLocation } from 'app/core/actions'; import { addInitLock, addVariable, @@ -31,6 +31,17 @@ import { alignCurrentWithMulti } from '../shared/multiOptions'; import { isMulti } from '../guard'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { DashboardModel } from 'app/features/dashboard/state'; +import { getConfig } from '../../../core/config'; +import { createErrorNotification } from '../../../core/copy/appNotification'; +import { VariableSrv } from '../../templating/variable_srv'; +import { + TransactionStatus, + variablesClearTransaction, + variablesCompleteTransaction, + variablesInitTransaction, +} from './transactionReducer'; +import { getBackendSrv } from '../../../core/services/backend_srv'; +import { cleanVariables } from './variablesReducer'; // process flow queryVariable // thunk => processVariables @@ -457,3 +468,49 @@ const getQueryWithVariables = (getState: () => StoreState): UrlQueryMap => { return queryParamsNew; }; + +export const initVariablesTransaction = ( + dashboardUid: string, + dashboard: DashboardModel, + variableSrv: VariableSrv +): ThunkResult => async (dispatch, getState) => { + try { + const transactionState = getState().templating.transaction; + if (transactionState.status === TransactionStatus.Fetching) { + // previous dashboard is still fetching variables, cancel all requests + dispatch(cancelVariables()); + } + + dispatch(variablesInitTransaction({ uid: dashboardUid })); + + const newVariables = getConfig().featureToggles.newVariables; + + if (!newVariables) { + await variableSrv.init(dashboard); + } + + if (newVariables) { + dispatch(initDashboardTemplating(dashboard.templating.list)); + await dispatch(processVariables()); + dispatch(completeDashboardTemplating(dashboard)); + } + + dispatch(variablesCompleteTransaction({ uid: dashboardUid })); + } catch (err) { + dispatch(notifyApp(createErrorNotification('Templating init failed', err))); + console.log(err); + } +}; + +export const cleanUpVariables = (): ThunkResult => dispatch => { + dispatch(cleanVariables()); + dispatch(variablesClearTransaction()); +}; + +type CancelVariablesDependencies = { getBackendSrv: typeof getBackendSrv }; +export const cancelVariables = ( + dependencies: CancelVariablesDependencies = { getBackendSrv: getBackendSrv } +): ThunkResult => dispatch => { + dependencies.getBackendSrv().cancelAllInFlightRequests(); + dispatch(cleanUpVariables()); +}; diff --git a/public/app/features/variables/state/helpers.ts b/public/app/features/variables/state/helpers.ts index 8d705c04fce..c193f5ae3e4 100644 --- a/public/app/features/variables/state/helpers.ts +++ b/public/app/features/variables/state/helpers.ts @@ -2,12 +2,11 @@ import { combineReducers } from '@reduxjs/toolkit'; import { NEW_VARIABLE_ID } from './types'; import { VariableHide, VariableModel } from '../../templating/types'; -import { variablesReducer, VariablesState } from './variablesReducer'; -import { optionsPickerReducer } from '../pickers/OptionsPicker/reducer'; -import { variableEditorReducer } from '../editor/reducer'; +import { VariablesState } from './variablesReducer'; import { locationReducer } from '../../../core/reducers/location'; import { VariableAdapter } from '../adapters'; import { dashboardReducer } from 'app/features/dashboard/state/reducers'; +import { templatingReducers } from './reducers'; export const getVariableState = ( noOfVariables: number, @@ -65,28 +64,16 @@ export const getRootReducer = () => combineReducers({ location: locationReducer, dashboard: dashboardReducer, - templating: combineReducers({ - optionsPicker: optionsPickerReducer, - editor: variableEditorReducer, - variables: variablesReducer, - }), + templating: templatingReducers, }); export const getTemplatingRootReducer = () => combineReducers({ - templating: combineReducers({ - optionsPicker: optionsPickerReducer, - editor: variableEditorReducer, - variables: variablesReducer, - }), + templating: templatingReducers, }); export const getTemplatingAndLocationRootReducer = () => combineReducers({ - templating: combineReducers({ - optionsPicker: optionsPickerReducer, - editor: variableEditorReducer, - variables: variablesReducer, - }), + templating: templatingReducers, location: locationReducer, }); diff --git a/public/app/features/variables/state/reducers.test.ts b/public/app/features/variables/state/reducers.test.ts index b76320f44ef..03ac4876b4d 100644 --- a/public/app/features/variables/state/reducers.test.ts +++ b/public/app/features/variables/state/reducers.test.ts @@ -1,9 +1,8 @@ import { reducerTester } from '../../../../test/core/redux/reducerTester'; -import { cleanUpDashboard } from 'app/features/dashboard/state/reducers'; import { QueryVariableModel, VariableHide } from '../../templating/types'; import { VariableAdapter, variableAdapters } from '../adapters'; import { createAction } from '@reduxjs/toolkit'; -import { variablesReducer, VariablesState } from './variablesReducer'; +import { cleanVariables, variablesReducer, VariablesState } from './variablesReducer'; import { toVariablePayload, VariablePayload } from './types'; import { VariableType } from '@grafana/data'; @@ -71,7 +70,7 @@ describe('variablesReducer', () => { reducerTester() .givenReducer(variablesReducer, initialState) - .whenActionIsDispatched(cleanUpDashboard()) + .whenActionIsDispatched(cleanVariables()) .thenStateShouldEqual({ '1': { id: '1', diff --git a/public/app/features/variables/state/reducers.ts b/public/app/features/variables/state/reducers.ts index 86107d0c8b0..8f3784c5e1d 100644 --- a/public/app/features/variables/state/reducers.ts +++ b/public/app/features/variables/state/reducers.ts @@ -3,17 +3,22 @@ import { optionsPickerReducer, OptionsPickerState } from '../pickers/OptionsPick import { variableEditorReducer, VariableEditorState } from '../editor/reducer'; import { variablesReducer } from './variablesReducer'; import { VariableModel } from '../../templating/types'; +import { transactionReducer, TransactionState } from './transactionReducer'; export interface TemplatingState { variables: Record; optionsPicker: OptionsPickerState; editor: VariableEditorState; + transaction: TransactionState; } +export const templatingReducers = combineReducers({ + editor: variableEditorReducer, + variables: variablesReducer, + optionsPicker: optionsPickerReducer, + transaction: transactionReducer, +}); + export default { - templating: combineReducers({ - editor: variableEditorReducer, - variables: variablesReducer, - optionsPicker: optionsPickerReducer, - }), + templating: templatingReducers, }; diff --git a/public/app/features/variables/state/sharedReducer.ts b/public/app/features/variables/state/sharedReducer.ts index 8c0a3aef8f7..d03866c9e6f 100644 --- a/public/app/features/variables/state/sharedReducer.ts +++ b/public/app/features/variables/state/sharedReducer.ts @@ -35,10 +35,22 @@ const sharedReducerSlice = createSlice({ }, resolveInitLock: (state: VariablesState, action: PayloadAction) => { const instanceState = getInstanceState(state, action.payload.id!); + + if (!instanceState) { + // we might have cancelled a batch so then this state has been removed + return; + } + instanceState.initLock?.resolve(); }, removeInitLock: (state: VariablesState, action: PayloadAction) => { const instanceState = getInstanceState(state, action.payload.id!); + + if (!instanceState) { + // we might have cancelled a batch so then this state has been removed + return; + } + instanceState.initLock = null; }, removeVariable: (state: VariablesState, action: PayloadAction>) => { diff --git a/public/app/features/variables/state/transactionReducer.test.ts b/public/app/features/variables/state/transactionReducer.test.ts new file mode 100644 index 00000000000..760bd3882a5 --- /dev/null +++ b/public/app/features/variables/state/transactionReducer.test.ts @@ -0,0 +1,61 @@ +import { reducerTester } from '../../../../test/core/redux/reducerTester'; +import { + initialTransactionState, + transactionReducer, + TransactionStatus, + variablesClearTransaction, + variablesCompleteTransaction, + variablesInitTransaction, +} from './transactionReducer'; + +describe('transactionReducer', () => { + describe('when variablesInitTransaction is dispatched', () => { + it('then state should be correct', () => { + reducerTester() + .givenReducer(transactionReducer, { ...initialTransactionState }) + .whenActionIsDispatched(variablesInitTransaction({ uid: 'a uid' })) + .thenStateShouldEqual({ ...initialTransactionState, uid: 'a uid', status: TransactionStatus.Fetching }); + }); + }); + + describe('when variablesCompleteTransaction is dispatched', () => { + describe('and transaction uid is the same', () => { + it('then state should be correct', () => { + reducerTester() + .givenReducer(transactionReducer, { + ...initialTransactionState, + uid: 'before', + status: TransactionStatus.Fetching, + }) + .whenActionIsDispatched(variablesCompleteTransaction({ uid: 'before' })) + .thenStateShouldEqual({ ...initialTransactionState, uid: 'before', status: TransactionStatus.Completed }); + }); + }); + + describe('and transaction uid is not the same', () => { + it('then state should be correct', () => { + reducerTester() + .givenReducer(transactionReducer, { + ...initialTransactionState, + uid: 'before', + status: TransactionStatus.Fetching, + }) + .whenActionIsDispatched(variablesCompleteTransaction({ uid: 'after' })) + .thenStateShouldEqual({ ...initialTransactionState, uid: 'before', status: TransactionStatus.Fetching }); + }); + }); + }); + + describe('when variablesClearTransaction is dispatched', () => { + it('then state should be correct', () => { + reducerTester() + .givenReducer(transactionReducer, { + ...initialTransactionState, + uid: 'before', + status: TransactionStatus.Completed, + }) + .whenActionIsDispatched(variablesClearTransaction()) + .thenStateShouldEqual({ ...initialTransactionState }); + }); + }); +}); diff --git a/public/app/features/variables/state/transactionReducer.ts b/public/app/features/variables/state/transactionReducer.ts new file mode 100644 index 00000000000..7cf116a9d23 --- /dev/null +++ b/public/app/features/variables/state/transactionReducer.ts @@ -0,0 +1,45 @@ +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; + +export enum TransactionStatus { + NotStarted = 'Not started', + Fetching = 'Fetching', + Completed = 'Completed', +} + +export interface TransactionState { + uid: string | undefined | null; + status: TransactionStatus; +} + +export const initialTransactionState: TransactionState = { uid: null, status: TransactionStatus.NotStarted }; + +const transactionSlice = createSlice({ + name: 'templating/transaction', + initialState: initialTransactionState, + reducers: { + variablesInitTransaction: (state, action: PayloadAction<{ uid: string | undefined | null }>) => { + state.uid = action.payload.uid; + state.status = TransactionStatus.Fetching; + }, + variablesCompleteTransaction: (state, action: PayloadAction<{ uid: string | undefined | null }>) => { + if (state.uid !== action.payload.uid) { + // this might be an action from a cancelled batch + return; + } + + state.status = TransactionStatus.Completed; + }, + variablesClearTransaction: (state, action: PayloadAction) => { + state.uid = null; + state.status = TransactionStatus.NotStarted; + }, + }, +}); + +export const { + variablesInitTransaction, + variablesClearTransaction, + variablesCompleteTransaction, +} = transactionSlice.actions; + +export const transactionReducer = transactionSlice.reducer; diff --git a/public/app/features/variables/state/variablesReducer.ts b/public/app/features/variables/state/variablesReducer.ts index 317da662458..001e92ca5ae 100644 --- a/public/app/features/variables/state/variablesReducer.ts +++ b/public/app/features/variables/state/variablesReducer.ts @@ -1,5 +1,4 @@ -import { PayloadAction } from '@reduxjs/toolkit'; -import { cleanUpDashboard } from '../../dashboard/state/reducers'; +import { createAction, PayloadAction } from '@reduxjs/toolkit'; import { variableAdapters } from '../adapters'; import { sharedReducer } from './sharedReducer'; import { VariableModel } from '../../templating/types'; @@ -9,11 +8,13 @@ export interface VariablesState extends Record {} export const initialVariablesState: VariablesState = {}; +export const cleanVariables = createAction('templating/cleanVariables'); + export const variablesReducer = ( state: VariablesState = initialVariablesState, action: PayloadAction ): VariablesState => { - if (cleanUpDashboard.match(action)) { + if (cleanVariables.match(action)) { const globalVariables = Object.values(state).filter(v => v.global); if (!globalVariables) { return initialVariablesState;