diff --git a/.betterer.results b/.betterer.results index 09551ef7f11..584acbcd0f1 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3681,8 +3681,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "3"] ], "public/app/core/utils/dag.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "public/app/core/utils/deferred.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/core/utils/dag.ts b/public/app/core/utils/dag.ts index 94716be2855..edfc698c91a 100644 --- a/public/app/core/utils/dag.ts +++ b/public/app/core/utils/dag.ts @@ -118,7 +118,7 @@ export class Node { } export class Graph { - nodes: any = {}; + nodes: Record = {}; constructor() {} @@ -189,6 +189,34 @@ export class Graph { return edges; } + descendants(nodes: Node[] | string[]): Set { + if (!nodes.length) { + return new Set(); + } + + const initialNodes = new Set( + isStringArray(nodes) ? nodes.map((n) => this.nodes[n]).filter((n) => n !== undefined) : nodes + ); + + return this.descendantsRecursive(initialNodes); + } + + private descendantsRecursive(nodes: Set, descendants = new Set()): Set { + for (const node of nodes) { + const newDescendants = new Set(); + for (const { inputNode } of node.inputEdges) { + if (inputNode && !descendants.has(inputNode)) { + descendants.add(inputNode); + newDescendants.add(inputNode); + } + } + + this.descendantsRecursive(newDescendants, descendants); + } + + return descendants; + } + createEdge(): Edge { return new Edge(); } @@ -212,3 +240,7 @@ export const printGraph = (g: Graph) => { console.log(`${n.name}:\n - links to: ${outputEdges}\n - links from: ${inputEdges}`); }); }; + +function isStringArray(arr: unknown[]): arr is string[] { + return arr.length > 0 && typeof arr[0] === 'string'; +} diff --git a/public/app/core/utils/set.ts b/public/app/core/utils/set.ts new file mode 100644 index 00000000000..9d8390ca1f9 --- /dev/null +++ b/public/app/core/utils/set.ts @@ -0,0 +1,8 @@ +export function mapSet(set: Set, callback: (t: T) => R): Set { + const newSet = new Set(); + for (const el of set) { + newSet.add(callback(el)); + } + + return newSet; +} diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index e41a1c612c4..c4a2fcc6d4d 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -19,6 +19,7 @@ import { } from '@grafana/data'; import { getTemplateSrv, RefreshEvent } from '@grafana/runtime'; import config from 'app/core/config'; +import { safeStringifyValue } from 'app/core/utils/explore'; import { getNextRefIdChar } from 'app/core/utils/query'; import { QueryGroupOptions } from 'app/types'; import { @@ -610,3 +611,18 @@ interface PanelOptionsCache { properties: any; fieldConfig: FieldConfigSource; } + +// For cases where we immediately want to stringify the panel model without cloning each property +export function stringifyPanelModel(panel: PanelModel) { + const model: any = {}; + + Object.entries(panel) + .filter( + ([prop, val]) => !notPersistedProperties[prop] && panel.hasOwnProperty(prop) && !isEqual(val, defaults[prop]) + ) + .forEach(([k, v]) => { + model[k] = v; + }); + + return safeStringifyValue(model); +} diff --git a/public/app/features/variables/inspect/utils.test.ts b/public/app/features/variables/inspect/utils.test.ts index 63b1155a8f9..bc508c0f885 100644 --- a/public/app/features/variables/inspect/utils.test.ts +++ b/public/app/features/variables/inspect/utils.test.ts @@ -4,14 +4,9 @@ import { variableAdapters } from '../adapters'; import { createCustomVariableAdapter } from '../custom/adapter'; import { createDataSourceVariableAdapter } from '../datasource/adapter'; import { createQueryVariableAdapter } from '../query/adapter'; +import { createGraph } from '../state/actions'; -import { - flattenPanels, - getAffectedPanelIdsForVariable, - getAllAffectedPanelIdsForVariableChange, - getDependenciesForVariable, - getPropsWithVariable, -} from './utils'; +import { flattenPanels, getAllAffectedPanelIdsForVariableChange, getPanelVars, getPropsWithVariable } from './utils'; describe('getPropsWithVariable', () => { it('when called it should return the correct graph', () => { @@ -217,43 +212,12 @@ describe('getPropsWithVariable', () => { }); }); -describe('getAffectedPanelIdsForVariable', () => { - describe('when called with a real world example with rows and repeats', () => { - it('then it should return correct panel ids', () => { - const panels = dashWithRepeatsAndRows.panels.map((panel: PanelModel) => ({ - id: panel.id, - getSaveModel: () => panel, - })); - const result = getAffectedPanelIdsForVariable('query0', panels); - expect(result).toEqual([15, 16, 17, 11, 12, 13, 2, 5, 7, 6]); - }); - }); -}); - variableAdapters.setInit(() => [ createDataSourceVariableAdapter(), createCustomVariableAdapter(), createQueryVariableAdapter(), ]); -describe('getDependenciesForVariable', () => { - describe('when called with a real world example with dependencies', () => { - it('then it should return correct dependencies', () => { - const { - templating: { list: variables }, - } = dashWithTemplateDependenciesAndPanels; - const result = getDependenciesForVariable('ds_instance', variables, new Set()); - expect([...result]).toEqual([ - 'ds', - 'query_with_ds', - 'depends_on_query_with_ds', - 'depends_on_query_with_ds_regex', - 'depends_on_all', - ]); - }); - }); -}); - describe('getAllAffectedPanelIdsForVariableChange ', () => { describe('when called with a real world example with dependencies and panels', () => { it('then it should return correct panelIds', () => { @@ -261,12 +225,11 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => { panels: panelsAsJson, templating: { list: variables }, } = dashWithTemplateDependenciesAndPanels; - const panels = panelsAsJson.map((panel: PanelModel) => ({ - id: panel.id, - getSaveModel: () => panel, - })); - const result = getAllAffectedPanelIdsForVariableChange('ds_instance', variables, panels); - expect(result).toEqual([2, 3, 4, 5]); + const panelVarPairs = getPanelVars(panelsAsJson); + const varGraph = createGraph(variables); + + const result = [...getAllAffectedPanelIdsForVariableChange(['ds_instance'], varGraph, panelVarPairs)]; + expect(result).toEqual([5, 2, 4, 3]); }); }); @@ -276,11 +239,9 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => { panels: panelsAsJson, templating: { list: variables }, } = dashWithTemplateDependenciesAndPanels; - const panels = panelsAsJson.map((panel: PanelModel) => ({ - id: panel.id, - getSaveModel: () => panel, - })); - const result = getAllAffectedPanelIdsForVariableChange('depends_on_all', variables, panels); + const panelVarPairs = getPanelVars(panelsAsJson); + const varGraph = createGraph(variables); + const result = [...getAllAffectedPanelIdsForVariableChange(['depends_on_all'], varGraph, panelVarPairs)]; expect(result).toEqual([2]); }); }); @@ -291,11 +252,9 @@ describe('getAllAffectedPanelIdsForVariableChange ', () => { panels: panelsAsJson, templating: { list: variables }, } = dashWithAllVariables; - const panels = panelsAsJson.map((panel: PanelModel) => ({ - id: panel.id, - getSaveModel: () => panel, - })); - const result = getAllAffectedPanelIdsForVariableChange('unknown', variables, panels); + const panelVarPairs = getPanelVars(panelsAsJson); + const varGraph = createGraph(variables); + const result = [...getAllAffectedPanelIdsForVariableChange(['unknown'], varGraph, panelVarPairs)]; expect(result).toEqual([2, 3]); }); }); diff --git a/public/app/features/variables/inspect/utils.ts b/public/app/features/variables/inspect/utils.ts index 36afa0e2cf4..3ff5b9993ba 100644 --- a/public/app/features/variables/inspect/utils.ts +++ b/public/app/features/variables/inspect/utils.ts @@ -1,4 +1,7 @@ import { DataLinkBuiltInVars } from '@grafana/data'; +import { Graph } from 'app/core/utils/dag'; +import { mapSet } from 'app/core/utils/set'; +import { stringifyPanelModel } from 'app/features/dashboard/state/PanelModel'; import { safeStringifyValue } from '../../../core/utils/explore'; import { DashboardModel, PanelModel } from '../../dashboard/state'; @@ -51,10 +54,10 @@ export const createDependencyEdges = (variables: VariableModel[]): GraphEdge[] = return edges; }; -function getVariableName(expression: string) { +export function getVariableName(expression: string) { const match = variableRegexExec(expression); if (!match) { - return null; + return undefined; } const variableName = match.slice(1).find((match) => match !== undefined); return variableName; @@ -253,86 +256,32 @@ function createUnknownsNetwork(variables: VariableModel[], dashboard: DashboardM This doesn't take circular dependencies in consideration. */ + export function getAllAffectedPanelIdsForVariableChange( - variableId: string, - variables: VariableModel[], - panels: PanelModel[] -): number[] { - const flattenedPanels = flattenPanels(panels); - let affectedPanelIds: number[] = getAffectedPanelIdsForVariable(variableId, flattenedPanels); - const affectedPanelIdsForAllVariables = getAffectedPanelIdsForVariable( - DataLinkBuiltInVars.includeVars, - flattenedPanels - ); - affectedPanelIds = [...new Set([...affectedPanelIdsForAllVariables, ...affectedPanelIds])]; - - const dependencies = getDependenciesForVariable(variableId, variables, new Set()); - for (const dependency of dependencies) { - const affectedPanelIdsForDependency = getAffectedPanelIdsForVariable(dependency, flattenedPanels); - affectedPanelIds = [...new Set([...affectedPanelIdsForDependency, ...affectedPanelIds])]; + variableIds: string[], + variableGraph: Graph, + panelsByVar: Record> +): Set { + const allDependencies = mapSet(variableGraph.descendants(variableIds), (n) => n.name); + allDependencies.add(DataLinkBuiltInVars.includeVars); + for (const id of variableIds) { + allDependencies.add(id); } + const affectedPanelIds = getDependentPanels([...allDependencies], panelsByVar); return affectedPanelIds; } -export function getDependenciesForVariable( - variableId: string, - variables: VariableModel[], - deps: Set -): Set { - if (!variables.length) { - return deps; - } - - for (const variable of variables) { - if (variable.name === variableId) { - continue; - } - - const depends = variableAdapters.get(variable.type).dependsOn(variable, { name: variableId }); - if (!depends) { - continue; - } - - deps.add(variable.name); - deps = getDependenciesForVariable(variable.name, variables, deps); - } - - return deps; -} - -export function getAffectedPanelIdsForVariable(variableId: string, panels: PanelModel[]): number[] { - if (!panels.length) { - return []; - } - - const affectedPanelIds: number[] = []; - const repeatRegex = new RegExp(`"repeat":"${variableId}"`); - for (const panel of panels) { - const panelAsJson = safeStringifyValue(panel.getSaveModel()); - - // check for repeats that don't use variableRegex - const repeatMatches = panelAsJson.match(repeatRegex); - if (repeatMatches?.length) { - affectedPanelIds.push(panel.id); - continue; - } - - const matches = panelAsJson.match(variableRegex); - if (!matches) { - continue; - } - - for (const match of matches) { - const variableName = getVariableName(match); - if (variableName === variableId) { - affectedPanelIds.push(panel.id); - break; - } +// Return an array of panel IDs depending on variables +export function getDependentPanels(variables: string[], panelsByVarUsage: Record>) { + const thePanels: number[] = []; + for (const varId of variables) { + if (panelsByVarUsage[varId]) { + thePanels.push(...panelsByVarUsage[varId]); } } - return affectedPanelIds; + return new Set(thePanels); } export interface UsagesToNetwork { @@ -419,3 +368,24 @@ export function flattenPanels(panels: PanelModel[]): PanelModel[] { return result; } + +// Accepts an array of panel models, and returns an array of panel IDs paired with +// the names of any template variables found +export function getPanelVars(panels: PanelModel[]) { + const panelsByVar: Record> = {}; + for (const p of panels) { + const jsonString = stringifyPanelModel(p); + const repeats = [...jsonString.matchAll(/"repeat":"([^"]+)"/g)].map((m) => m[1]); + const varRegexMatches = jsonString.match(variableRegex)?.map((m) => getVariableName(m)) ?? []; + const varNames = [...repeats, ...varRegexMatches]; + for (const varName of varNames) { + if (varName! in panelsByVar) { + panelsByVar[varName!].add(p.id); + } else { + panelsByVar[varName!] = new Set([p.id]); + } + } + } + + return panelsByVar; +} diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index 85ead05d594..ee5535f5c20 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -36,7 +36,7 @@ import { isMulti, isQuery, } from '../guard'; -import { getAllAffectedPanelIdsForVariableChange } from '../inspect/utils'; +import { getAllAffectedPanelIdsForVariableChange, getPanelVars } from '../inspect/utils'; import { cleanPickerState } from '../pickers/OptionsPicker/reducer'; import { alignCurrentWithMulti } from '../shared/multiOptions'; import { @@ -549,7 +549,7 @@ export const setOptionAsCurrent = ( }; }; -const createGraph = (variables: VariableModel[]) => { +export const createGraph = (variables: VariableModel[]) => { const g = new Graph(); variables.forEach((v) => { @@ -594,9 +594,14 @@ export const variableUpdated = ( const variables = getVariablesByKey(rootStateKey, state); const g = createGraph(variables); const panels = state.dashboard?.getModel()?.panels ?? []; + const panelVars = getPanelVars(panels); + const event: VariablesChangedEvent = isAdHoc(variableInState) ? { refreshAll: true, panelIds: [] } // for adhoc variables we don't know which panels that will be impacted - : { refreshAll: false, panelIds: getAllAffectedPanelIdsForVariableChange(variableInState.id, variables, panels) }; + : { + refreshAll: false, + panelIds: Array.from(getAllAffectedPanelIdsForVariableChange([variableInState.id], g, panelVars)), + }; const node = g.getNode(variableInState.name); let promises: Array> = []; @@ -643,7 +648,7 @@ export const onTimeRangeUpdated = }) as VariableWithOptions[]; const variableIds = variablesThatNeedRefresh.map((variable) => variable.id); - const promises = variablesThatNeedRefresh.map((variable: VariableWithOptions) => + const promises = variablesThatNeedRefresh.map((variable) => dispatch(timeRangeUpdated(toKeyedVariableIdentifier(variable))) ); @@ -678,8 +683,8 @@ export const templateVarsChangedInUrl = async (dispatch, getState) => { const update: Array> = []; const dashboard = getState().dashboard.getModel(); - const panelIds = new Set(); const variables = getVariablesByKey(key, getState()); + for (const variable of variables) { const key = `var-${variable.name}`; if (!vars.hasOwnProperty(key)) { @@ -706,24 +711,29 @@ export const templateVarsChangedInUrl = } } - // for adhoc variables we don't know which panels that will be impacted - if (!isAdHoc(variable)) { - getAllAffectedPanelIdsForVariableChange(variable.id, variables, dashboard?.panels ?? []).forEach((id) => - panelIds.add(id) - ); - } - const promise = variableAdapters.get(variable.type).setValueFromUrl(variable, value); update.push(promise); } + const filteredVars = variables.filter((v) => { + const key = `var-${v.name}`; + return vars.hasOwnProperty(key) && isVariableUrlValueDifferentFromCurrent(v, vars[key].value) && !isAdHoc(v); + }); + const varGraph = createGraph(variables); + const panelVars = getPanelVars(dashboard?.panels ?? []); + const affectedPanels = getAllAffectedPanelIdsForVariableChange( + filteredVars.map((v) => v.id), + varGraph, + panelVars + ); + if (update.length) { await Promise.all(update); events.publish( new VariablesChangedInUrl({ - refreshAll: panelIds.size === 0, - panelIds: Array.from(panelIds), + refreshAll: affectedPanels.size === 0, + panelIds: Array.from(affectedPanels), }) ); }