Variables: Improves inspection performance and unknown filtering (#31811) (#31813)

* Refactor: moves inspect calculation to Redux

* Refactor: adds valid filters and tests

(cherry picked from commit 63746d027b)

Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com>

public/app/features/variables/editor/VariableEditorContainer.tsx

@@ -17,6 +17,11 @@ const mapStateToProps = (state: StoreState) => ({
   variables: getEditorVariables(state),
   idInEditor: state.templating.editor.id,
   dashboard: state.dashboard.getModel(),
+  unknownsNetwork: state.templating.inspect.unknownsNetwork,
+  unknownExists: state.templating.inspect.unknownExits,
+  usagesNetwork: state.templating.inspect.usagesNetwork,
+  unknown: state.templating.inspect.unknown,
+  usages: state.templating.inspect.usages,
 });
 
 const mapDispatchToProps = {
@@ -67,6 +72,7 @@ class VariableEditorContainerUnconnected extends PureComponent<Props> {
 
   render() {
     const variableToEdit = this.props.variables.find((s) => s.id === this.props.idInEditor) ?? null;
+
     return (
       <div>
         <div className="page-action-bar">
@@ -114,8 +120,10 @@ class VariableEditorContainerUnconnected extends PureComponent<Props> {
               onChangeVariableOrder={this.onChangeVariableOrder}
               onDuplicateVariable={this.onDuplicateVariable}
               onRemoveVariable={this.onRemoveVariable}
+              usages={this.props.usages}
+              usagesNetwork={this.props.usagesNetwork}
             />
-            <VariablesUnknownTable dashboard={this.props.dashboard} variables={this.props.variables} />
+            {this.props.unknownExists ? <VariablesUnknownTable usages={this.props.unknownsNetwork} /> : null}
           </>
         )}
         {variableToEdit && <VariableEditorEditor identifier={toVariableIdentifier(variableToEdit)} />}

public/app/features/variables/editor/VariableEditorList.tsx

@@ -8,13 +8,15 @@ import EmptyListCTA from '../../../core/components/EmptyListCTA/EmptyListCTA';
 import { QueryVariableModel, VariableModel } from '../types';
 import { toVariableIdentifier, VariableIdentifier } from '../state/types';
 import { DashboardModel } from '../../dashboard/state';
-import { getVariableUsages } from '../inspect/utils';
+import { getVariableUsages, UsagesToNetwork, VariableUsageTree } from '../inspect/utils';
 import { isAdHoc } from '../guard';
 import { VariableUsagesButton } from '../inspect/VariableUsagesButton';
 
 export interface Props {
   variables: VariableModel[];
   dashboard: DashboardModel | null;
+  usages: VariableUsageTree[];
+  usagesNetwork: UsagesToNetwork[];
   onAddClick: (event: MouseEvent) => void;
   onEditClick: (identifier: VariableIdentifier) => void;
   onChangeVariableOrder: (identifier: VariableIdentifier, fromIndex: number, toIndex: number) => void;
@@ -97,7 +99,7 @@ export class VariableEditorList extends PureComponent<Props> {
                       : typeof variable.query === 'string'
                       ? variable.query
                       : '';
-                    const usages = getVariableUsages(variable.id, this.props.variables, this.props.dashboard);
+                    const usages = getVariableUsages(variable.id, this.props.usages);
                     const passed = usages > 0 || isAdHoc(variable);
                     return (
                       <tr key={`${variable.name}-${index}`}>
@@ -129,9 +131,9 @@ export class VariableEditorList extends PureComponent<Props> {
 
                         <td style={{ width: '1%' }}>
                           <VariableUsagesButton
-                            variable={variable}
+                            id={variable.id}
-                            variables={this.props.variables}
+                            isAdhoc={isAdHoc(variable)}
-                            dashboard={this.props.dashboard}
+                            usages={this.props.usagesNetwork}
                           />
                         </td>
 

public/app/features/variables/editor/actions.ts

@@ -1,5 +1,5 @@
 import { ThunkResult } from '../../../types';
-import { getNewVariabelIndex, getVariable, getVariables } from '../state/selectors';
+import { getEditorVariables, getNewVariabelIndex, getVariable, getVariables } from '../state/selectors';
 import {
   changeVariableNameFailed,
   changeVariableNameSucceeded,
@@ -15,6 +15,8 @@ import { VariableType } from '@grafana/data';
 import { addVariable, removeVariable } from '../state/sharedReducer';
 import { updateOptions } from '../state/actions';
 import { VariableModel } from '../types';
+import { initInspect } from '../inspect/reducer';
+import { createUsagesNetwork, transformUsagesToNetwork } from '../inspect/utils';
 
 export const variableEditorMount = (identifier: VariableIdentifier): ThunkResult<void> => {
   return async (dispatch) => {
@@ -102,8 +104,17 @@ export const switchToEditMode = (identifier: VariableIdentifier): ThunkResult<vo
   dispatch(setIdInEditor({ id: identifier.id }));
 };
 
-export const switchToListMode = (): ThunkResult<void> => (dispatch) => {
+export const switchToListMode = (): ThunkResult<void> => (dispatch, getState) => {
   dispatch(clearIdInEditor());
+  const state = getState();
+  const variables = getEditorVariables(state);
+  const dashboard = state.dashboard.getModel();
+  const { unknown, usages } = createUsagesNetwork(variables, dashboard);
+  const unknownsNetwork = transformUsagesToNetwork(unknown);
+  const unknownExits = Object.keys(unknown).length > 0;
+  const usagesNetwork = transformUsagesToNetwork(usages);
+
+  dispatch(initInspect({ unknown, usages, usagesNetwork, unknownsNetwork, unknownExits }));
 };
 
 export function getNextAvailableId(type: VariableType, variables: VariableModel[]): string {

public/app/features/variables/inspect/VariableUsagesButton.tsx

@@ -1,33 +1,18 @@
 import React, { FC, useMemo } from 'react';
-import { Provider } from 'react-redux';
 import { IconButton } from '@grafana/ui';
 
-import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
+import { UsagesToNetwork } from './utils';
-import { store } from '../../../store/store';
-import { isAdHoc } from '../guard';
 import { NetworkGraphModal } from './NetworkGraphModal';
-import { VariableModel } from '../types';
-import { DashboardModel } from '../../dashboard/state';
 
-interface OwnProps {
+interface Props {
-  variables: VariableModel[];
+  id: string;
-  variable: VariableModel;
+  usages: UsagesToNetwork[];
-  dashboard: DashboardModel | null;
+  isAdhoc: boolean;
 }
 
-interface ConnectedProps {}
+export const VariableUsagesButton: FC<Props> = ({ id, usages, isAdhoc }) => {
-
+  const network = useMemo(() => usages.find((n) => n.variable.id === id), [usages, id]);
-interface DispatchProps {}
+  if (usages.length === 0 || isAdhoc || !network) {
-
-type Props = OwnProps & ConnectedProps & DispatchProps;
-
-export const UnProvidedVariableUsagesGraphButton: FC<Props> = ({ variables, variable, dashboard }) => {
-  const { id } = variable;
-  const { usages } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
-  const network = useMemo(() => transformUsagesToNetwork(usages).find((n) => n.variable.id === id), [usages, id]);
-  const adhoc = useMemo(() => isAdHoc(variable), [variable]);
-
-  if (usages.length === 0 || adhoc || !network) {
     return null;
   }
 
@@ -46,9 +31,3 @@ export const UnProvidedVariableUsagesGraphButton: FC<Props> = ({ variables, vari
     </NetworkGraphModal>
   );
 };
-
-export const VariableUsagesButton: FC<Props> = (props) => (
-  <Provider store={store}>
-    <UnProvidedVariableUsagesGraphButton {...props} />
-  </Provider>
-);

public/app/features/variables/inspect/VariablesUnknownButton.tsx

@@ -1,31 +1,17 @@
 import React, { FC, useMemo } from 'react';
-import { Provider } from 'react-redux';
 import { IconButton } from '@grafana/ui';
-import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
+import { UsagesToNetwork } from './utils';
-import { store } from '../../../store/store';
-import { VariableModel } from '../types';
-import { DashboardModel } from '../../dashboard/state';
 import { NetworkGraphModal } from './NetworkGraphModal';
 
-interface OwnProps {
+interface Props {
-  variable: VariableModel;
+  id: string;
-  variables: VariableModel[];
+  usages: UsagesToNetwork[];
-  dashboard: DashboardModel | null;
 }
 
-interface ConnectedProps {}
+export const VariablesUnknownButton: FC<Props> = ({ id, usages }) => {
+  const network = useMemo(() => usages.find((n) => n.variable.id === id), [id, usages]);
 
-interface DispatchProps {}
+  if (!network) {
-
-type Props = OwnProps & ConnectedProps & DispatchProps;
-
-export const UnProvidedVariablesUnknownGraphButton: FC<Props> = ({ variable, variables, dashboard }) => {
-  const { id } = variable;
-  const { unknown } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
-  const network = useMemo(() => transformUsagesToNetwork(unknown).find((n) => n.variable.id === id), [id, unknown]);
-  const unknownExist = useMemo(() => Object.keys(unknown).length > 0, [unknown]);
-
-  if (!unknownExist || !network) {
     return null;
   }
 
@@ -44,9 +30,3 @@ export const UnProvidedVariablesUnknownGraphButton: FC<Props> = ({ variable, var
     </NetworkGraphModal>
   );
 };
-
-export const VariablesUnknownButton: FC<Props> = (props) => (
-  <Provider store={store}>
-    <UnProvidedVariablesUnknownGraphButton {...props} />
-  </Provider>
-);

public/app/features/variables/inspect/VariablesUnknownTable.tsx

@@ -1,35 +1,16 @@
-import React, { FC, useMemo } from 'react';
+import React, { FC } from 'react';
-import { Provider } from 'react-redux';
 import { css } from 'emotion';
 import { Icon, Tooltip, useStyles } from '@grafana/ui';
 import { GrafanaTheme } from '@grafana/data';
-import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
+import { UsagesToNetwork } from './utils';
-import { store } from '../../../store/store';
 import { VariablesUnknownButton } from './VariablesUnknownButton';
-import { VariableModel } from '../types';
-import { DashboardModel } from '../../dashboard/state';
 
-interface OwnProps {
+interface Props {
-  variables: VariableModel[];
+  usages: UsagesToNetwork[];
-  dashboard: DashboardModel | null;
 }
 
-interface ConnectedProps {}
+export const VariablesUnknownTable: FC<Props> = ({ usages }) => {
-
-interface DispatchProps {}
-
-type Props = OwnProps & ConnectedProps & DispatchProps;
-
-export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboard }) => {
   const style = useStyles(getStyles);
-  const { unknown } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
-  const networks = useMemo(() => transformUsagesToNetwork(unknown), [unknown]);
-  const unknownExist = useMemo(() => Object.keys(unknown).length > 0, [unknown]);
-
-  if (!unknownExist) {
-    return null;
-  }
-
   return (
     <div className={style.container}>
       <h5>
@@ -48,8 +29,8 @@ export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboar
             </tr>
           </thead>
           <tbody>
-            {networks.map((network) => {
+            {usages.map((usage) => {
-              const { variable } = network;
+              const { variable } = usage;
               const { id, name } = variable;
               return (
                 <tr key={id}>
@@ -60,7 +41,7 @@ export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboar
                   <td className={style.defaultColumn} />
                   <td className={style.defaultColumn} />
                   <td className={style.lastColumn}>
-                    <VariablesUnknownButton variable={variable} variables={variables} dashboard={dashboard} />
+                    <VariablesUnknownButton id={variable.id} usages={usages} />
                   </td>
                 </tr>
               );
@@ -97,9 +78,3 @@ const getStyles = (theme: GrafanaTheme) => ({
     text-align: right;
   `,
 });
-
-export const VariablesUnknownTable: FC<Props> = (props) => (
-  <Provider store={store}>
-    <UnProvidedVariablesUnknownTable {...props} />
-  </Provider>
-);

public/app/features/variables/inspect/reducer.test.ts

@@ -0,0 +1,29 @@
+import { reducerTester } from '../../../../test/core/redux/reducerTester';
+import { initialVariableInspectState, initInspect, variableInspectReducer } from './reducer';
+import { textboxBuilder } from '../shared/testing/builders';
+
+describe('variableInspectReducer', () => {
+  describe('when initInspect is dispatched', () => {
+    it('then state should be correct', () => {
+      const variable = textboxBuilder().withId('text').withName('text').build();
+      reducerTester()
+        .givenReducer(variableInspectReducer, { ...initialVariableInspectState })
+        .whenActionIsDispatched(
+          initInspect({
+            unknownExits: true,
+            unknownsNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
+            usagesNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
+            usages: [{ variable, tree: {} }],
+            unknown: [{ variable, tree: {} }],
+          })
+        )
+        .thenStateShouldEqual({
+          unknownExits: true,
+          unknownsNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
+          usagesNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
+          usages: [{ variable, tree: {} }],
+          unknown: [{ variable, tree: {} }],
+        });
+    });
+  });
+});

public/app/features/variables/inspect/reducer.ts

@@ -0,0 +1,46 @@
+import { UsagesToNetwork, VariableUsageTree } from './utils';
+import { createSlice, PayloadAction } from '@reduxjs/toolkit';
+
+export interface VariableInspectState {
+  unknown: VariableUsageTree[];
+  usages: VariableUsageTree[];
+  unknownsNetwork: UsagesToNetwork[];
+  usagesNetwork: UsagesToNetwork[];
+  unknownExits: boolean;
+}
+
+export const initialVariableInspectState: VariableInspectState = {
+  unknown: [],
+  usages: [],
+  unknownsNetwork: [],
+  usagesNetwork: [],
+  unknownExits: false,
+};
+
+const variableInspectReducerSlice = createSlice({
+  name: 'templating/inspect',
+  initialState: initialVariableInspectState,
+  reducers: {
+    initInspect: (
+      state,
+      action: PayloadAction<{
+        unknown: VariableUsageTree[];
+        usages: VariableUsageTree[];
+        unknownsNetwork: UsagesToNetwork[];
+        usagesNetwork: UsagesToNetwork[];
+        unknownExits: boolean;
+      }>
+    ) => {
+      const { unknown, usages, unknownExits, unknownsNetwork, usagesNetwork } = action.payload;
+      state.usages = usages;
+      state.unknown = unknown;
+      state.unknownsNetwork = unknownsNetwork;
+      state.unknownExits = unknownExits;
+      state.usagesNetwork = usagesNetwork;
+    },
+  },
+});
+
+export const variableInspectReducer = variableInspectReducerSlice.reducer;
+
+export const { initInspect } = variableInspectReducerSlice.actions;

public/app/features/variables/inspect/utils.test.ts

@@ -44,4 +44,104 @@ describe('getPropsWithVariable', () => {
       },
     });
   });
+
+  it('when called with a valid an id that is not part of valid names it should return the correct graph', () => {
+    const value = {
+      targets: [
+        {
+          id: 'A',
+          description: '$tag_host-[[tag_host]]',
+          query:
+            'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
+          alias: '$tag_host [[tag_host]] $col $host',
+        },
+      ],
+    };
+
+    const result = getPropsWithVariable(
+      'host',
+      {
+        key: 'model',
+        value,
+      },
+      {}
+    );
+
+    expect(result).toEqual({
+      targets: {
+        A: {
+          alias: '$tag_host [[tag_host]] $col $host',
+          query:
+            'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
+        },
+      },
+    });
+  });
+
+  it('when called with an id that is part of valid alias names it should return the correct graph', () => {
+    const value = {
+      targets: [
+        {
+          id: 'A',
+          description: '[[tag_host1]]',
+          description2: '$tag_host1',
+          query:
+            'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
+          alias: '[[tag_host1]] $tag_host1 $col $host',
+        },
+      ],
+    };
+
+    const tagHostResult = getPropsWithVariable(
+      'tag_host1',
+      {
+        key: 'model',
+        value,
+      },
+      {}
+    );
+
+    expect(tagHostResult).toEqual({
+      targets: {
+        A: {
+          description: '[[tag_host1]]',
+          description2: '$tag_host1',
+        },
+      },
+    });
+  });
+
+  it('when called with an id that is part of valid query names it should return the correct graph', () => {
+    const value = {
+      targets: [
+        {
+          id: 'A',
+          description: '[[timeFilter]]',
+          description2: '$timeFilter',
+          query:
+            'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
+          alias: '[[timeFilter]] $timeFilter $col $host',
+        },
+      ],
+    };
+
+    const tagHostResult = getPropsWithVariable(
+      'timeFilter',
+      {
+        key: 'model',
+        value,
+      },
+      {}
+    );
+
+    expect(tagHostResult).toEqual({
+      targets: {
+        A: {
+          description: '[[timeFilter]]',
+          description2: '$timeFilter',
+          alias: '[[timeFilter]] $timeFilter $col $host',
+        },
+      },
+    });
+  });
 });

public/app/features/variables/inspect/utils.ts

@@ -65,6 +65,16 @@ export const toVisNetworkEdges = (edges: GraphEdge[]): any[] => {
   return new vis.DataSet(edgesWithStyle);
 };
 
+function getVariableName(expression: string) {
+  variableRegex.lastIndex = 0;
+  const match = variableRegex.exec(expression);
+  if (!match) {
+    return null;
+  }
+  const variableName = match.slice(1).find((match) => match !== undefined);
+  return variableName;
+}
+
 export const getUnknownVariableStrings = (variables: VariableModel[], model: any) => {
   const unknownVariableNames: string[] = [];
   const modelAsString = safeStringifyValue(model, 2);
@@ -89,7 +99,7 @@ export const getUnknownVariableStrings = (variables: VariableModel[], model: any
       continue;
     }
 
-    const variableName = match.slice(1);
+    const variableName = getVariableName(match);
 
     if (variables.some((variable) => variable.id === variableName)) {
       // ignore defined variables
@@ -100,16 +110,32 @@ export const getUnknownVariableStrings = (variables: VariableModel[], model: any
       continue;
     }
 
-    unknownVariableNames.push(variableName);
+    if (variableName) {
+      unknownVariableNames.push(variableName);
+    }
   }
 
   return unknownVariableNames;
 };
 
+const validVariableNames: Record<string, RegExp[]> = {
+  alias: [/^m$/, /^measurement$/, /^col$/, /^tag_\w+|\d+$/],
+  query: [/^timeFilter$/],
+};
+
 export const getPropsWithVariable = (variableId: string, parent: { key: string; value: any }, result: any) => {
   const stringValues = Object.keys(parent.value).reduce((all, key) => {
     const value = parent.value[key];
-    if (value && typeof value === 'string' && containsVariable(value, variableId)) {
+    if (!value || typeof value !== 'string') {
+      return all;
+    }
+
+    const isValidName = validVariableNames[key]
+      ? validVariableNames[key].find((regex: RegExp) => regex.test(variableId))
+      : undefined;
+    const hasVariable = containsVariable(value, variableId);
+
+    if (!isValidName && hasVariable) {
       all = {
         ...all,
         [key]: value,
@@ -146,10 +172,15 @@ export const getPropsWithVariable = (variableId: string, parent: { key: string;
   return result;
 };
 
+export interface VariableUsageTree {
+  variable: VariableModel;
+  tree: any;
+}
+
 export interface VariableUsages {
   unUsed: VariableModel[];
-  unknown: Array<{ variable: VariableModel; tree: any }>;
+  unknown: VariableUsageTree[];
-  usages: Array<{ variable: VariableModel; tree: any }>;
+  usages: VariableUsageTree[];
 }
 
 export const createUsagesNetwork = (variables: VariableModel[], dashboard: DashboardModel | null): VariableUsages => {
@@ -158,8 +189,8 @@ export const createUsagesNetwork = (variables: VariableModel[], dashboard: Dashb
   }
 
   const unUsed: VariableModel[] = [];
-  let usages: Array<{ variable: VariableModel; tree: any }> = [];
+  let usages: VariableUsageTree[] = [];
-  let unknown: Array<{ variable: VariableModel; tree: any }> = [];
+  let unknown: VariableUsageTree[] = [];
   const model = dashboard.getSaveModelClone();
 
   const unknownVariables = getUnknownVariableStrings(variables, model);
@@ -220,7 +251,7 @@ export const traverseTree = (usage: UsagesToNetwork, parent: { id: string; value
   return usage;
 };
 
-export const transformUsagesToNetwork = (usages: Array<{ variable: VariableModel; tree: any }>): UsagesToNetwork[] => {
+export const transformUsagesToNetwork = (usages: VariableUsageTree[]): UsagesToNetwork[] => {
   const results: UsagesToNetwork[] = [];
 
   for (const usage of usages) {
@@ -249,12 +280,7 @@ const countLeaves = (object: any): number => {
   return (total as unknown) as number;
 };
 
-export const getVariableUsages = (
+export const getVariableUsages = (variableId: string, usages: VariableUsageTree[]): number => {
-  variableId: string,
-  variables: VariableModel[],
-  dashboard: DashboardModel | null
-): number => {
-  const { usages } = createUsagesNetwork(variables, dashboard);
   const usage = usages.find((usage) => usage.variable.id === variableId);
   if (!usage) {
     return 0;

public/app/features/variables/state/reducers.ts

@@ -4,12 +4,14 @@ import { variableEditorReducer, VariableEditorState } from '../editor/reducer';
 import { variablesReducer } from './variablesReducer';
 import { VariableModel } from '../types';
 import { transactionReducer, TransactionState } from './transactionReducer';
+import { variableInspectReducer, VariableInspectState } from '../inspect/reducer';
 
 export interface TemplatingState {
   variables: Record<string, VariableModel>;
   optionsPicker: OptionsPickerState;
   editor: VariableEditorState;
   transaction: TransactionState;
+  inspect: VariableInspectState;
 }
 
 export const templatingReducers = combineReducers({
@@ -17,6 +19,7 @@ export const templatingReducers = combineReducers({
   variables: variablesReducer,
   optionsPicker: optionsPickerReducer,
   transaction: transactionReducer,
+  inspect: variableInspectReducer,
 });
 
 export default {