AzureMonitor: Fix template variables being cleared out (#39173)

* AzureMonitor: Fix template variables being cleared out

* fix metric namespace from resetting

* tests :)

public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/datasource.ts

@@ -1,4 +1,5 @@
 import Datasource from '../datasource';
+import { mocked } from 'ts-jest/utils';
 
 type DeepPartial<T> = {
   [P in keyof T]?: DeepPartial<T[P]>;
@@ -43,5 +44,5 @@ export default function createMockDatasource() {
 
   const mockDatasource = _mockDatasource as Datasource;
 
-  return mockDatasource;
+  return mocked(mockDatasource, true);
 }

public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.test.ts

@@ -1,11 +1,26 @@
 import { renderHook } from '@testing-library/react-hooks';
 
-import { useAsyncState } from './dataHooks';
+import {
+  DataHook,
+  useAsyncState,
+  useMetricNames,
+  useResourceGroups,
+  useResourceNames,
+  useResourceTypes,
+} from './dataHooks';
+import { AzureMetricQuery, AzureMonitorOption, AzureQueryType } from '../../types';
+import createMockDatasource from '../../__mocks__/datasource';
+import { MockedObjectDeep } from 'ts-jest/dist/utils/testing';
+import Datasource from '../../datasource';
 
 interface WaitableMock extends jest.Mock<any, any> {
   waitToBeCalled(): Promise<unknown>;
 }
 
+const WAIT_OPTIONS = {
+  timeout: 1000,
+};
+
 function createWaitableMock() {
   let resolve: Function;
 
@@ -21,6 +36,8 @@ function createWaitableMock() {
   return mock;
 }
 
+const opt = (text: string, value: string) => ({ text, value });
+
 describe('AzureMonitor: useAsyncState', () => {
   const MOCKED_RANDOM_VALUE = 0.42069;
 
@@ -64,3 +81,267 @@ describe('AzureMonitor: useAsyncState', () => {
     expect(setError).toHaveBeenCalledWith(MOCKED_RANDOM_VALUE, undefined);
   });
 });
+
+interface TestScenario {
+  name: string;
+  hook: DataHook;
+
+  // For conviencence, only need to define the azureMonitor part of the query
+  emptyQueryPartial: AzureMetricQuery;
+  validQueryPartial: AzureMetricQuery;
+  invalidQueryPartial: AzureMetricQuery;
+  templateVariableQueryPartial: AzureMetricQuery;
+
+  expectedClearedQueryPartial: AzureMetricQuery;
+  expectedOptions: AzureMonitorOption[];
+}
+
+describe('AzureMonitor: metrics dataHooks', () => {
+  const bareQuery = {
+    refId: 'A',
+    queryType: AzureQueryType.AzureMonitor,
+    subscription: 'sub-abc-123',
+  };
+
+  const testTable: TestScenario[] = [
+    {
+      name: 'useResourceGroups',
+      hook: useResourceGroups,
+      emptyQueryPartial: {},
+      validQueryPartial: {
+        resourceGroup: 'web-app-development',
+      },
+      invalidQueryPartial: {
+        resourceGroup: 'wrong-resource-group`',
+      },
+      templateVariableQueryPartial: {
+        resourceGroup: '$rg',
+      },
+      expectedOptions: [
+        {
+          label: 'Web App - Production',
+          value: 'web-app-production',
+        },
+        {
+          label: 'Web App - Development',
+          value: 'web-app-development',
+        },
+      ],
+      expectedClearedQueryPartial: {
+        resourceGroup: undefined,
+      },
+    },
+
+    {
+      name: 'useResourceTypes',
+      hook: useResourceTypes,
+      emptyQueryPartial: {
+        resourceGroup: 'web-app-development',
+      },
+      validQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+      },
+      invalidQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/invalid-resource-type',
+      },
+      templateVariableQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: '$rt',
+      },
+      expectedOptions: [
+        {
+          label: 'Virtual Machine',
+          value: 'azure/vm',
+        },
+        {
+          label: 'Database',
+          value: 'azure/db',
+        },
+      ],
+      expectedClearedQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: undefined,
+      },
+    },
+
+    {
+      name: 'useResourceNames',
+      hook: useResourceNames,
+      emptyQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+      },
+      validQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+      },
+      invalidQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'resource-that-doesnt-exist',
+      },
+      templateVariableQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: '$variable',
+      },
+      expectedOptions: [
+        {
+          label: 'Web server',
+          value: 'web-server',
+        },
+        {
+          label: 'Job server',
+          value: 'job-server',
+        },
+      ],
+      expectedClearedQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: undefined,
+      },
+    },
+
+    {
+      name: 'useMetricNames',
+      hook: useMetricNames,
+      emptyQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+        metricNamespace: 'azure/vm',
+      },
+      validQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+        metricNamespace: 'azure/vm',
+      },
+      invalidQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+        metricNamespace: 'azure/vm',
+        metricName: 'invalid-metric',
+      },
+      templateVariableQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+        metricNamespace: 'azure/vm',
+        metricName: '$variable',
+      },
+      expectedOptions: [
+        {
+          label: 'Percentage CPU',
+          value: 'percentage-cpu',
+        },
+        {
+          label: 'Free memory',
+          value: 'free-memory',
+        },
+      ],
+      expectedClearedQueryPartial: {
+        resourceGroup: 'web-app-development',
+        metricDefinition: 'azure/vm',
+        resourceName: 'web-server',
+        metricNamespace: 'azure/vm',
+        metricName: undefined,
+      },
+    },
+  ];
+
+  let datasource: MockedObjectDeep<Datasource>;
+  let onChange: jest.Mock<any, any>;
+  let setError: jest.Mock<any, any>;
+
+  beforeEach(() => {
+    onChange = jest.fn();
+    setError = jest.fn();
+
+    datasource = createMockDatasource();
+    datasource.getVariables = jest.fn().mockReturnValue(['$sub', '$rg', '$rt', '$variable']);
+
+    datasource.getResourceGroups = jest
+      .fn()
+      .mockResolvedValue([
+        opt('Web App - Production', 'web-app-production'),
+        opt('Web App - Development', 'web-app-development'),
+      ]);
+
+    datasource.getMetricDefinitions = jest
+      .fn()
+      .mockResolvedValue([opt('Virtual Machine', 'azure/vm'), opt('Database', 'azure/db')]);
+
+    datasource.getResourceNames = jest
+      .fn()
+      .mockResolvedValue([opt('Web server', 'web-server'), opt('Job server', 'job-server')]);
+
+    datasource.getMetricNames = jest
+      .fn()
+      .mockResolvedValue([opt('Percentage CPU', 'percentage-cpu'), opt('Free memory', 'free-memory')]);
+  });
+
+  describe.each(testTable)('scenario %#: $name', (scenario) => {
+    it('returns values', async () => {
+      const query = {
+        ...bareQuery,
+        azureMonitor: scenario.emptyQueryPartial,
+      };
+      const { result, waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
+      await waitForNextUpdate(WAIT_OPTIONS);
+
+      expect(result.current).toEqual(scenario.expectedOptions);
+    });
+
+    it('does not call onChange when the property has not been set', async () => {
+      const query = {
+        ...bareQuery,
+        azureMonitor: scenario.emptyQueryPartial,
+      };
+      const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
+      await waitForNextUpdate(WAIT_OPTIONS);
+
+      expect(onChange).not.toHaveBeenCalled();
+    });
+
+    it('does not clear the property when it is a valid option', async () => {
+      const query = {
+        ...bareQuery,
+        azureMonitor: scenario.validQueryPartial,
+      };
+      const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
+      await waitForNextUpdate(WAIT_OPTIONS);
+
+      expect(onChange).not.toHaveBeenCalled();
+    });
+
+    it('does not clear the property when it is a template variable', async () => {
+      const query = {
+        ...bareQuery,
+        azureMonitor: scenario.templateVariableQueryPartial,
+      };
+      const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
+      await waitForNextUpdate(WAIT_OPTIONS);
+
+      expect(onChange).not.toHaveBeenCalled();
+    });
+
+    it('clears the property when it is not a valid option', async () => {
+      const query = {
+        ...bareQuery,
+        azureMonitor: scenario.invalidQueryPartial,
+      };
+      const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
+      await waitForNextUpdate(WAIT_OPTIONS);
+
+      expect(onChange).toHaveBeenCalledWith({
+        ...query,
+        azureMonitor: scenario.expectedClearedQueryPartial,
+      });
+    });
+  });
+});

public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts

@@ -25,7 +25,7 @@ export interface MetricMetadata {
 type SetErrorFn = (source: string, error: AzureMonitorErrorish | undefined) => void;
 type OnChangeFn = (newQuery: AzureMonitorQuery) => void;
 
-type DataHook = (
+export type DataHook = (
   query: AzureMonitorQuery,
   datasource: Datasource,
   onChange: OnChangeFn,
@@ -97,7 +97,7 @@ export const useResourceGroups: DataHook = (query, datasource, onChange, setErro
       const results = await datasource.getResourceGroups(subscription);
       const options = results.map(toOption);
 
-      if (resourceGroup && !hasOption(options, resourceGroup)) {
+      if (isInvalidOption(resourceGroup, options, datasource.getVariables())) {
         onChange(setResourceGroup(query, undefined));
       }
 
@@ -121,7 +121,7 @@ export const useResourceTypes: DataHook = (query, datasource, onChange, setError
       const results = await datasource.getMetricDefinitions(subscription, resourceGroup);
       const options = results.map(toOption);
 
-      if (metricDefinition && !hasOption(options, metricDefinition)) {
+      if (isInvalidOption(metricDefinition, options, datasource.getVariables())) {
         onChange(setResourceType(query, undefined));
       }
 
@@ -145,7 +145,7 @@ export const useResourceNames: DataHook = (query, datasource, onChange, setError
       const results = await datasource.getResourceNames(subscription, resourceGroup, metricDefinition);
       const options = results.map(toOption);
 
-      if (resourceName && !hasOption(options, resourceName)) {
+      if (isInvalidOption(resourceName, options, datasource.getVariables())) {
         onChange(setResourceName(query, undefined));
       }
 
@@ -170,9 +170,9 @@ export const useMetricNamespaces: DataHook = (query, datasource, onChange, setEr
       const options = results.map(toOption);
 
       // Do some cleanup of the query state if need be
-      if ((!metricNamespace && options.length) || options.length === 1) {
+      if (!metricNamespace && options.length) {
         onChange(setMetricNamespace(query, options[0].value));
-      } else if (options[0] && metricNamespace && !hasOption(options, metricNamespace)) {
+      } else if (options[0] && isInvalidOption(metricNamespace, options, datasource.getVariables())) {
         onChange(setMetricNamespace(query, options[0].value));
       }
 
@@ -205,7 +205,7 @@ export const useMetricNames: DataHook = (query, datasource, onChange, setError)
 
       const options = results.map(toOption);
 
-      if (metricName && !hasOption(options, metricName)) {
+      if (isInvalidOption(metricName, options, datasource.getVariables())) {
         onChange(setMetricName(query, undefined));
       }
 
@@ -277,3 +277,7 @@ export const useMetricMetadata = (query: AzureMonitorQuery, datasource: Datasour
 
   return metricMetadata;
 };
+
+function isInvalidOption(value: string | undefined, options: AzureMonitorOption[], templateVariables: string[]) {
+  return value && !templateVariables.includes(value) && !hasOption(options, value);
+}

public/app/plugins/datasource/grafana-azure-monitor-datasource/datasource.ts

@@ -290,6 +290,10 @@ export default class Datasource extends DataSourceApi<AzureMonitorQuery, AzureDa
   getVariables() {
     return this.templateSrv.getVariables().map((v) => `$${v.name}`);
   }
+
+  isTemplateVariable(value: string) {
+    return this.getVariables().includes(value);
+  }
 }
 
 function hasQueryForType(query: AzureMonitorQuery): boolean {