Variables: Prevents panel from crashing when using adhoc variable in data links (#39546)

* Variables: Prevents panel from crashing when using adhoc variable in data links

* Refactor: uses isAdhoc instead

* Chore: updates after PR feedback

public/app/features/templating/formatRegistry.ts

@@ -16,10 +16,29 @@ export interface FormatRegistryItem extends RegistryItem {
   formatter(options: FormatOptions, variable: VariableModel): string;
 }
 
+export enum FormatRegistryID {
+  lucene = 'lucene',
+  raw = 'raw',
+  regex = 'regex',
+  pipe = 'pipe',
+  distributed = 'distributed',
+  csv = 'csv',
+  html = 'html',
+  json = 'json',
+  percentEncode = 'percentencode',
+  singleQuote = 'singlequote',
+  doubleQuote = 'doublequote',
+  sqlString = 'sqlstring',
+  date = 'date',
+  glob = 'glob',
+  text = 'text',
+  queryParam = 'queryparam',
+}
+
 export const formatRegistry = new Registry<FormatRegistryItem>(() => {
   const formats: FormatRegistryItem[] = [
     {
-      id: 'lucene',
+      id: FormatRegistryID.lucene,
       name: 'Lucene',
       description: 'Values are lucene escaped and multi-valued variables generate an OR expression',
       formatter: ({ value }) => {
@@ -39,13 +58,13 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'raw',
+      id: FormatRegistryID.raw,
       name: 'raw',
       description: 'Keep value as is',
       formatter: ({ value }) => value,
     },
     {
-      id: 'regex',
+      id: FormatRegistryID.regex,
       name: 'Regex',
       description: 'Values are regex escaped and multi-valued variables generate a (<value>|<value>) expression',
       formatter: ({ value }) => {
@@ -61,7 +80,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'pipe',
+      id: FormatRegistryID.pipe,
       name: 'Pipe',
       description: 'Values are separated by | character',
       formatter: ({ value }) => {
@@ -72,7 +91,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'distributed',
+      id: FormatRegistryID.distributed,
       name: 'Distributed',
       description: 'Multiple values are formatted like variable=value',
       formatter: ({ value }, variable) => {
@@ -91,7 +110,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'csv',
+      id: FormatRegistryID.csv,
       name: 'Csv',
       description: 'Comma-separated values',
       formatter: ({ value }) => {
@@ -102,7 +121,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'html',
+      id: FormatRegistryID.html,
       name: 'HTML',
       description: 'HTML escaping of values',
       formatter: ({ value }) => {
@@ -113,7 +132,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'json',
+      id: FormatRegistryID.json,
       name: 'JSON',
       description: 'JSON stringify valu',
       formatter: ({ value }) => {
@@ -121,7 +140,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'percentencode',
+      id: FormatRegistryID.percentEncode,
       name: 'Percent encode',
       description: 'Useful for URL escaping values',
       formatter: ({ value }) => {
@@ -133,7 +152,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'singlequote',
+      id: FormatRegistryID.singleQuote,
       name: 'Single quote',
       description: 'Single quoted values',
       formatter: ({ value }) => {
@@ -146,7 +165,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'doublequote',
+      id: FormatRegistryID.doubleQuote,
       name: 'Double quote',
       description: 'Double quoted values',
       formatter: ({ value }) => {
@@ -159,7 +178,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'sqlstring',
+      id: FormatRegistryID.sqlString,
       name: 'SQL string',
       description: 'SQL string quoting and commas for use in IN statements and other scenarios',
       formatter: ({ value }) => {
@@ -172,7 +191,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'date',
+      id: FormatRegistryID.date,
       name: 'Date',
       description: 'Format date in different ways',
       formatter: ({ value, args }) => {
@@ -191,7 +210,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'glob',
+      id: FormatRegistryID.glob,
       name: 'Glob',
       description: 'Format multi-valued variables using glob syntax, example {value1,value2}',
       formatter: ({ value }) => {
@@ -202,7 +221,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'text',
+      id: FormatRegistryID.text,
       name: 'Text',
       description: 'Format variables in their text representation. Example in multi-variable scenario A + B + C.',
       formatter: (options, variable) => {
@@ -220,7 +239,7 @@ export const formatRegistry = new Registry<FormatRegistryItem>(() => {
       },
     },
     {
-      id: 'queryparam',
+      id: FormatRegistryID.queryParam,
       name: 'Query parameter',
       description:
         'Format variables as URL parameters. Example in multi-variable scenario A + B + C => var-foo=A&var-foo=B&var-foo=C.',

public/app/features/templating/template_srv.test.ts

@@ -5,6 +5,7 @@ import { VariableAdapter, variableAdapters } from '../variables/adapters';
 import { createQueryVariableAdapter } from '../variables/query/adapter';
 import { createAdHocVariableAdapter } from '../variables/adhoc/adapter';
 import { VariableModel } from '../variables/types';
+import { FormatRegistryID } from './formatRegistry';
 
 variableAdapters.setInit(() => [
   (createQueryVariableAdapter() as unknown) as VariableAdapter<VariableModel>,
@@ -660,6 +661,46 @@ describe('templateSrv', () => {
     });
   });
 
+  describe('adhoc variables', () => {
+    beforeEach(() => {
+      _templateSrv = initTemplateSrv([
+        {
+          type: 'adhoc',
+          name: 'adhoc',
+          filters: [
+            {
+              condition: '',
+              key: 'alertstate',
+              operator: '=',
+              value: 'firing',
+            },
+            {
+              condition: '',
+              key: 'alertname',
+              operator: '=',
+              value: 'ExampleAlertAlwaysFiring',
+            },
+          ],
+        },
+      ]);
+    });
+
+    it(`should not be handled by any registry items except for queryparam`, () => {
+      const registryItems = Object.values(FormatRegistryID);
+      for (const registryItem of registryItems) {
+        if (registryItem === FormatRegistryID.queryParam) {
+          continue;
+        }
+
+        const firstTarget = _templateSrv.replace(`\${adhoc:${registryItem}}`, {});
+        expect(firstTarget).toBe('');
+
+        const secondTarget = _templateSrv.replace('${adhoc}', {}, registryItem);
+        expect(secondTarget).toBe('');
+      }
+    });
+  });
+
   describe('queryparam', () => {
     beforeEach(() => {
       _templateSrv = initTemplateSrv([
@@ -675,11 +716,29 @@ describe('templateSrv', () => {
           current: { value: ['value1', 'value2'] },
           options: [{ value: 'value1' }, { value: 'value2' }],
         },
+        {
+          type: 'adhoc',
+          name: 'adhoc',
+          filters: [
+            {
+              condition: '',
+              key: 'alertstate',
+              operator: '=',
+              value: 'firing',
+            },
+            {
+              condition: '',
+              key: 'alertname',
+              operator: '=',
+              value: 'ExampleAlertAlwaysFiring',
+            },
+          ],
+        },
       ]);
     });
 
     it('query variable with single value with queryparam format should return correct queryparam', () => {
-      const target = _templateSrv.replace('${single:queryparam}', {});
+      const target = _templateSrv.replace(`\${single:queryparam}`, {});
       expect(target).toBe('var-single=value1');
     });
 
@@ -689,7 +748,7 @@ describe('templateSrv', () => {
     });
 
     it('query variable with multi value with queryparam format should return correct queryparam', () => {
-      const target = _templateSrv.replace('${multi:queryparam}', {});
+      const target = _templateSrv.replace(`\${multi:queryparam}`, {});
       expect(target).toBe('var-multi=value1&var-multi=value2');
     });
 
@@ -697,5 +756,15 @@ describe('templateSrv', () => {
       const target = _templateSrv.replace('${multi}', {}, 'queryparam');
       expect(target).toBe('var-multi=value1&var-multi=value2');
     });
+
+    it('query variable with adhoc value with queryparam format should return correct queryparam', () => {
+      const target = _templateSrv.replace(`\${adhoc:queryparam}`, {});
+      expect(target).toBe('var-adhoc=alertstate%7C%3D%7Cfiring&var-adhoc=alertname%7C%3D%7CExampleAlertAlwaysFiring');
+    });
+
+    it('query variable with multi value and queryparam format should return correct queryparam', () => {
+      const target = _templateSrv.replace('${adhoc}', {}, 'queryparam');
+      expect(target).toBe('var-adhoc=alertstate%7C%3D%7Cfiring&var-adhoc=alertname%7C%3D%7CExampleAlertAlwaysFiring');
+    });
   });
 });

public/app/features/templating/template_srv.ts

@@ -1,12 +1,13 @@
-import { isString, property, escape } from 'lodash';
+import { escape, isString, property } from 'lodash';
 import { deprecationWarning, ScopedVars, TimeRange } from '@grafana/data';
 import { getFilteredVariables, getVariables, getVariableWithName } from '../variables/state/selectors';
 import { variableRegex } from '../variables/utils';
 import { isAdHoc } from '../variables/guard';
 import { VariableModel } from '../variables/types';
 import { setTemplateSrv, TemplateSrv as BaseTemplateSrv } from '@grafana/runtime';
-import { FormatOptions, formatRegistry } from './formatRegistry';
+import { FormatOptions, formatRegistry, FormatRegistryID } from './formatRegistry';
 import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from '../variables/state/types';
+import { safeStringifyValue } from '../../core/utils/explore';
 
 interface FieldAccessorCache {
   [key: string]: (obj: any) => any;
@@ -115,6 +116,10 @@ export class TemplateSrv implements BaseTemplateSrv {
       return '';
     }
 
+    if (isAdHoc(variable) && format !== FormatRegistryID.queryParam) {
+      return '';
+    }
+
     // if it's an object transform value to string
     if (!Array.isArray(value) && typeof value === 'object') {
       value = `${value}`;
@@ -125,7 +130,7 @@ export class TemplateSrv implements BaseTemplateSrv {
     }
 
     if (!format) {
-      format = 'glob';
+      format = FormatRegistryID.glob;
     }
 
     // some formats have arguments that come after ':' character
@@ -141,7 +146,7 @@ export class TemplateSrv implements BaseTemplateSrv {
 
     if (!formatItem) {
       console.error(`Variable format ${format} not found. Using glob format as fallback.`);
-      formatItem = formatRegistry.get('glob');
+      formatItem = formatRegistry.get(FormatRegistryID.glob);
     }
 
     const options: FormatOptions = { value, args, text: text ?? value };
@@ -270,6 +275,13 @@ export class TemplateSrv implements BaseTemplateSrv {
         return match;
       }
 
+      if (isAdHoc(variable)) {
+        const value = safeStringifyValue(variable.filters);
+        const text = variable.id;
+
+        return this.formatValue(value, fmt, variable, text);
+      }
+
       const systemValue = this.grafanaVariables[variable.current.value];
       if (systemValue) {
         return this.formatValue(systemValue, fmt, variable);
@@ -282,7 +294,7 @@ export class TemplateSrv implements BaseTemplateSrv {
         value = this.getAllValue(variable);
         text = ALL_VARIABLE_TEXT;
         // skip formatting of custom all values
-        if (variable.allValue && fmt !== 'text' && fmt !== 'queryparam') {
+        if (variable.allValue && fmt !== FormatRegistryID.text && fmt !== FormatRegistryID.queryParam) {
           return this.replace(value);
         }
       }