diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index 81b9d40b130..e73555705bf 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -149,6 +149,22 @@ describe('shared actions', () => { }); describe('when processVariables is dispatched', () => { + it('then circular dependencies fail gracefully', async () => { + const key = 'key'; + const var1 = queryBuilder().withName('var1').withQuery('$var2').build(); + const var2 = queryBuilder().withName('var2').withQuery('$var1').build(); + const dashboard: any = { templating: { list: [var1, var2] } }; + const preloadedState = getPreloadedState(key, {}); + + await expect(async () => { + await reduxTester({ preloadedState }) + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(toKeyedAction(key, variablesInitTransaction({ uid: key }))) + .whenActionIsDispatched(initDashboardTemplating(key, dashboard)) + .whenAsyncActionIsDispatched(processVariables(key), true); + }).rejects.toThrow(/circular dependency in dashboard variables detected/i); + }); + it('then correct actions are dispatched', async () => { const key = 'key'; const query = queryBuilder().build(); diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index b1d57723a8b..6c064431173 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -271,20 +271,12 @@ export const processVariableDependencies = async (variable: VariableModel, state throw new Error(`rootStateKey not found for variable with id:${variable.id}`); } - const dependencies: VariableModel[] = []; - - for (const otherVariable of getVariablesByKey(variable.rootStateKey, state)) { - if (variable === otherVariable) { - continue; - } - - if (variableAdapters.getIfExists(variable.type)) { - if (variableAdapters.get(variable.type).dependsOn(variable, otherVariable)) { - dependencies.push(otherVariable); - } - } + if (isDependencyGraphCircular(variable, state)) { + throw new Error('Circular dependency in dashboard variables detected. Dashboard may not work as expected.'); } + const dependencies = getDirectDependencies(variable, state); + if (!isWaitingForDependencies(variable.rootStateKey, dependencies, state)) { return; } @@ -303,6 +295,44 @@ export const processVariableDependencies = async (variable: VariableModel, state }); }; +const isDependencyGraphCircular = ( + variable: VariableModel, + state: StoreState, + encounteredDependencyIds: Set = new Set() +): boolean => { + if (encounteredDependencyIds.has(variable.id)) { + return true; + } + + encounteredDependencyIds = new Set([...encounteredDependencyIds, variable.id]); + + return getDirectDependencies(variable, state).some((dependency) => { + return isDependencyGraphCircular(dependency, state, encounteredDependencyIds); + }); +}; + +const getDirectDependencies = (variable: VariableModel, state: StoreState) => { + if (!variable.rootStateKey) { + return []; + } + + const directDependencies: VariableModel[] = []; + + for (const otherVariable of getVariablesByKey(variable.rootStateKey, state)) { + if (variable === otherVariable) { + continue; + } + + if (variableAdapters.getIfExists(variable.type)) { + if (variableAdapters.get(variable.type).dependsOn(variable, otherVariable)) { + directDependencies.push(otherVariable); + } + } + } + + return directDependencies; +}; + const isWaitingForDependencies = (key: string, dependencies: VariableModel[], state: StoreState): boolean => { if (dependencies.length === 0) { return false;