list missing fields of query

The error message for an incomplete query
was unspecific up until now. It now includes
a list of all missing fields.

Author: TheSpacyCat

src/backend/rest.ts

@@ -18,6 +18,7 @@ import * as process from 'process';
 import { CmkQuery } from '../types';
 import { createCmkContext, replaceVariables, toLiveStatusQuery, updateQuery } from '../utils';
 import { Backend, DatasourceOptions } from './types';
+import { validateRequestSpec } from './validate';
 
 type RestApiError = {
   detail: string;
@@ -147,13 +148,16 @@ export default class RestApiBackend implements Backend {
   }
 
   async query(request: DataQueryRequest<CmkQuery>): Promise<DataQueryResponse> {
+    request.targets.forEach((query) => {
+      validateRequestSpec(query.requestSpec, this.datasource.getEdition());
+    });
+
     const promises = request.targets
       .filter((target) => !target.hide)
       .map((target) => {
         return this.getSingleGraph(request.range, target);
       });
-    const result = await Promise.all(promises).then((data) => ({ data }));
-    return result;
+    return await Promise.all(promises).then((data) => ({ data }));
   }
 
   async testDatasource(): Promise<unknown> {

src/backend/validate.ts

@@ -0,0 +1,38 @@
+import { RequestSpec } from '../RequestSpec';
+import { Edition } from '../types';
+import { labelForRequestSpecKey } from '../ui/utils';
+
+// 'graph_type' and 'aggregation' should always have a default value
+type PotentiallyRequiredKeys = 'site' | 'service' | 'host_name' | 'graph';
+
+const missingRequiredFields = (rq: Partial<RequestSpec>, edition: Edition): string[] => {
+  const result: PotentiallyRequiredKeys[] = [];
+  if (rq.graph === undefined || rq.graph === '') {
+    result.push('graph');
+  }
+
+  if (edition === 'RAW') {
+    if (rq.site === undefined) {
+      result.push('site');
+    }
+    if (rq.service === undefined || rq.service === '') {
+      result.push('service');
+    }
+    if (rq.host_name === undefined || rq.service === '') {
+      result.push('host_name');
+    }
+  }
+  return result.sort().map((value) => labelForRequestSpecKey(value, rq));
+};
+
+export const validateRequestSpec = (rq: Partial<RequestSpec> | undefined, edition: Edition): void => {
+  if (rq === undefined) {
+    rq = {};
+  }
+
+  const missingFields = missingRequiredFields(rq, edition);
+  if (missingFields.length === 0) {
+    return;
+  }
+  throw new Error(`Please specify a value for the following fields: ${missingFields.join(', ')}`);
+};

src/backend/web.ts

@@ -14,6 +14,7 @@ import {
   createWebApiRequestSpecification,
 } from './../webapi';
 import { Backend, DatasourceOptions } from './types';
+import { validateRequestSpec } from './validate';
 
 export default class WebApiBackend implements Backend {
   datasource: DatasourceOptions;
@@ -75,6 +76,10 @@ export default class WebApiBackend implements Backend {
   }
 
   async query(options: DataQueryRequest<CmkQuery>): Promise<DataQueryResponse> {
+    options.targets.forEach((query) => {
+      validateRequestSpec(query.requestSpec, this.datasource.getEdition());
+    });
+
     const { range } = options;
     const from = range.from.unix();
     const to = range.to.unix();
@@ -140,7 +145,7 @@ export default class WebApiBackend implements Backend {
     }
   }
 
-  getGraphQuery = async (range: number[], query: CmkQuery): Promise<MutableDataFrame<unknown>> => {
+  async getGraphQuery(range: number[], query: CmkQuery): Promise<MutableDataFrame<unknown>> {
     updateQuery(query);
     const graph = get(query, 'requestSpec.graph');
     if (isUndefined(graph) || graph === '') {
@@ -183,5 +188,5 @@ export default class WebApiBackend implements Backend {
     );
 
     return frame;
-  };
+  }
 }

tests/unit/DataSource.test.ts

@@ -1,5 +1,17 @@
+import { MutableDataFrame } from '@grafana/data';
+import { cloneDeep } from 'lodash';
+
+import { DataSource } from '../../src/DataSource';
+import { RequestSpec } from '../../src/RequestSpec';
+import RestApiBackend from '../../src/backend/rest';
+import WebApiBackend from '../../src/backend/web';
+import { Edition } from '../../src/types';
+import { labelForRequestSpecKey } from '../../src/ui/utils';
+import * as utils from '../../src/utils';
 import { buildRequestBody, buildUrlWithParams } from '../../src/webapi';
 
+jest.mock('../../src/utils');
+
 describe('URL conversions', () => {
   it('Params', () => {
     expect(buildUrlWithParams('hi', { A: '5', TE: 'TTI' })).toBe('hi?A=5&TE=TTI');
@@ -8,3 +20,113 @@ describe('URL conversions', () => {
     expect(buildRequestBody({ spec: ['comb', { site: 'heute' }] })).toBe('request={"spec":["comb",{"site":"heute"}]}');
   });
 });
+
+// from https://stackoverflow.com/questions/42773836/how-to-find-all-subsets-of-a-set-in-javascript-powerset-of-array
+const allSubsets = (values: string[]): string[][] =>
+  values.reduce((subsets, value) => subsets.concat(subsets.map((set) => [value, ...set])), [[]]);
+
+const fullExampleRequestSpec: RequestSpec = {
+  graph_type: 'predefined_graph',
+  aggregation: 'off',
+  site: 'monitoring',
+  host_name: 'example.com',
+  host_name_regex: {
+    value: '*.org',
+    negated: false,
+  },
+  host_in_group: {
+    value: 'printers',
+    negated: true,
+  },
+  host_labels: ['os:linux'],
+  host_tags: [{}, {}, {}],
+  service: 'CPU load',
+  service_regex: {
+    value: 'CPU*',
+    negated: true,
+  },
+  service_in_group: {
+    value: 'web_servers',
+    negated: false,
+  },
+  graph: 'Load Average',
+};
+
+const getRequiredFields = (edition: Edition): Array<keyof RequestSpec> => {
+  const result: Array<keyof RequestSpec> = ['graph'];
+  if (edition === 'RAW') {
+    result.push('site', 'host_name', 'service');
+  }
+  return result.sort();
+};
+
+describe.each([{ edition: 'RAW' }, { edition: 'CEE' }])(
+  'Query Validation in Edition $edition',
+  (editionConfig: { edition: Edition }) => {
+    describe.each([{ backend: 'rest' }, { backend: 'web' }])(
+      'with backend $backend',
+      (backendConfig: { backend: string }) => {
+        let subject: DataSource;
+        const requiredFields = getRequiredFields(editionConfig.edition);
+        const cases = allSubsets(requiredFields)
+          .filter((arr) => arr.length > 0)
+          .map((arr) => arr.sort())
+          .map((value) => ({ fields: value }));
+
+        jest
+          .spyOn(RestApiBackend.prototype, 'getSingleGraph')
+          .mockImplementation(() => Promise.resolve('It did succeed, sadly.'));
+
+        jest
+          .spyOn(WebApiBackend.prototype, 'getGraphQuery')
+          .mockImplementation(() => Promise.resolve(new MutableDataFrame()));
+
+        utils.replaceVariables.mockImplementation((rq) => rq);
+
+        beforeEach(() => {
+          subject = new DataSource({ jsonData: { backend: backendConfig, edition: editionConfig.edition } } as any);
+        });
+
+        it.each(cases)(
+          'throws an informative error if the required fields $fields are missing in a query',
+          async ({ fields }) => {
+            const requestSpec = cloneDeep(fullExampleRequestSpec);
+            for (const key of fields) {
+              delete requestSpec[key];
+            }
+            const dataQueryRequest = {
+              targets: [
+                {
+                  requestSpec,
+                },
+              ],
+              range: [1, 2],
+            } as any;
+
+            const errorMessageRegex = fields
+              .map((value) => labelForRequestSpecKey(value as keyof RequestSpec, requestSpec))
+              .join(', ');
+
+            // make sure this test doesn't fail pass on a resolved promise
+            expect.assertions(1);
+            await expect(subject.query(dataQueryRequest)).rejects.toThrow(errorMessageRegex);
+          }
+        );
+
+        it('validates even if site is an empty string', async () => {
+          const requestSpec = cloneDeep(fullExampleRequestSpec);
+          requestSpec['site'] = '';
+          const dataQueryRequest = {
+            targets: [
+              {
+                requestSpec,
+              },
+            ],
+            range: [1, 2],
+          } as any;
+          await expect(subject.query(dataQueryRequest)).resolves.not.toThrow();
+        });
+      }
+    );
+  }
+);