Skip to content
27 changes: 26 additions & 1 deletion src/core/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BackendSrv, TemplateSrv } from "@grafana/runtime";
import { validateNumericInput, enumToOptions, filterXSSField, filterXSSLINQExpression, replaceVariables, queryInBatches, queryUsingSkip, queryUntilComplete, getVariableOptions, get, post, addOptionsToLookup, multipleValuesQuery, timeFieldsQuery } from "./utils";
import { validateNumericInput, enumToOptions, filterXSSField, filterXSSLINQExpression, replaceVariables, queryInBatches, queryUsingSkip, queryUntilComplete, getVariableOptions, get, post, addOptionsToLookup, multipleValuesQuery, timeFieldsQuery, getFilterConcatOperator } from "./utils";
import { BatchQueryConfig, QBField, QueryBuilderOption } from "./types";
import { of, throwError } from 'rxjs';

Expand Down Expand Up @@ -733,3 +733,28 @@ describe('multipleValuesQuery', () => {
expect(result).toBe('(field like "value1")');
});
});

describe('getLogicalOperator', () => {
[
{
name: 'equals',
operator: '=',
},
{
name: 'is not blank',
operator: 'isnotblank',
},
].forEach(testCase => {
it(`should return OR for ${testCase.name} operator`, () => {
const result = getFilterConcatOperator(testCase.operator);

expect(result).toBe('||');
});
});

it('should return AND as the default logical operator', () => {
const result = getFilterConcatOperator('>');

expect(result).toBe('&&');
});
});
24 changes: 13 additions & 11 deletions src/core/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,26 @@ export function multipleValuesQuery(field: string): ExpressionTransformFunction
return (value: string, operation: string, _options?: any) => {
const isMultiSelect = isMultiValueExpression(value);
const valuesArray = getMultipleValuesArray(value);
const logicalOperator = getLogicalOperator(operation);
const logicalOperator = getFilterConcatOperator(operation);

return isMultiSelect
? `(${valuesArray.map(val => buildExpression(field, val, operation)).join(` ${logicalOperator} `)})`
: buildExpression(field, value, operation);
};
}

/**
* Gets the logical operator for a given query operation when building multi-value expressions
* or combining multiple conditions.
* @param operation The operation to be checked.
* @returns The logical operator as a string.
*/
export function getFilterConcatOperator(operation: string): string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, the purpose of this funtion is to return || for postive operations and && for neagative operation.

This is needed when we split one query into multiple queries.

For example,

To split source = abc into minionId = abc || system = abc.
To split source != abc into minionId = abc && system = abc.
To split source = {abc, efg} into minionId = abc || system = abc || minionId = efg || system = efg.
To split partNumber = {1, 2, 3} into partNumber = 1 || partNumber = 2 || partNumber = .

So, I would suggest to name this as getLogicalOperatorToCombineSplitQuery to be clear. I'm happy to consider better name suggestions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the test cases to reflect this purpose.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operators such as IS_NOT_BLANK is not really applicable for the multipleValuesQuery. Because, if those operators are used that won't be a isMultiValueExpression at all.

So, let's not couple the responsibilities. Have two methods.

  • getLogicalOperatorForMultipleValuesQuery
  • getLogicalOperatorForMultiplePropertiesQuery

I'm happy to consider better name suggestions.

return operation === QueryBuilderOperations.EQUALS.name || operation === QueryBuilderOperations.IS_NOT_BLANK.name
? '||'
: '&&';
}

/**
* Builds a query expression for a specific field, value, and operation.
* @param field - The name of the field to be queried.
Expand Down Expand Up @@ -390,16 +402,6 @@ function getMultipleValuesArray(value: string): string[] {
return value.replace(/({|})/g, '').split(',');
}

/**
* Gets the logical operator for a given query operation when building multi-value expressions
* or combining multiple conditions.
* @param operation The operation to be checked.
* @returns The logical operator as a string.
*/
function getLogicalOperator(operation: string): string {
return operation === QueryBuilderOperations.EQUALS.name ? '||' : '&&';
}

async function delay(timeout: number): Promise<void> {
return new Promise(resolve => setTimeout(resolve, timeout));
}
Expand Down
77 changes: 77 additions & 0 deletions src/datasources/alarms/AlarmsDataSourceCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,83 @@ describe('AlarmsDataSourceCore', () => {
expect(datastore.templateSrv.replace).toHaveBeenCalledWith('channel != "${query0}"', {});
expect(transformQuery).toBe('(channel != "channel1" && channel != "channel2")');
});

describe('transformSourceFilter', () => {
[
{
name: 'source equals',
input: 'source = "test-source"',
expected: '(properties.system = "test-source" || properties.minionId = "test-source")',
},
{
name: 'source does not equal',
input: 'source != "test-source"',
expected: '(properties.system != "test-source" && properties.minionId != "test-source")',
},
{
name: 'source is blank',
input: 'string.IsNullOrEmpty(source)',
expected: '(string.IsNullOrEmpty(properties.system) && string.IsNullOrEmpty(properties.minionId))',
},
{
name: 'source is not blank',
input: '!string.IsNullOrEmpty(source)',
expected: '(!string.IsNullOrEmpty(properties.system) || !string.IsNullOrEmpty(properties.minionId))',
},
].forEach(({ name, input, expected }) => {
it(`should transform ${name} filter`, () => {
const result = datastore.transformAlarmsQueryWrapper({}, input);

expect(result).toBe(expected);
});
});

[
{
name: 'source equals',
input: 'source = "${query0}"',
replacedInput: 'source = "{source1,source2}"',
expected:
'((properties.system = "source1" || properties.system = "source2") || (properties.minionId = "source1" || properties.minionId = "source2"))',
},
{
name: 'source does not equal',
input: 'source != "${query0}"',
replacedInput: 'source != "{source1,source2}"',
expected:
'((properties.system != "source1" && properties.system != "source2") && (properties.minionId != "source1" && properties.minionId != "source2"))',
},
].forEach(({ name, input, replacedInput, expected }) => {
it(`should transform ${name} for mutiple value variable filter`, () => {
jest.spyOn(datastore.templateSrv, 'replace').mockReturnValue(replacedInput);

const transformQuery = datastore.transformAlarmsQueryWrapper({}, input);

expect(datastore.templateSrv.replace).toHaveBeenCalledWith(input, {});
expect(transformQuery).toBe(expected);
});
});

it('should replace single value variable in the source filter', () => {
const mockQueryBy = 'source != "${query0}"';
jest.spyOn(datastore.templateSrv, 'replace').mockReturnValue('source != "test-source"');

const transformQuery = datastore.transformAlarmsQueryWrapper({}, mockQueryBy);

expect(datastore.templateSrv.replace).toHaveBeenCalledWith('source != "${query0}"', {});
expect(transformQuery).toBe('(properties.system != "test-source" && properties.minionId != "test-source")');
});

it('should handle transformation for multiple source filters in a query', () => {
const mockFilter = 'source = "source1" || string.IsNullOrEmpty(source)';

const result = datastore.transformAlarmsQueryWrapper({}, mockFilter);

expect(result).toBe(
'(properties.system = "source1" || properties.minionId = "source1") || (string.IsNullOrEmpty(properties.system) && string.IsNullOrEmpty(properties.minionId))'
);
});
});
});
});

Expand Down
26 changes: 19 additions & 7 deletions src/datasources/alarms/AlarmsDataSourceCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import { ExpressionTransformFunction, transformComputedFieldsQuery } from "core/
import { ALARMS_TIME_FIELDS, AlarmsQueryBuilderFields } from "./constants/AlarmsQueryBuilder.constants";
import { QueryBuilderOption, Workspace } from "core/types";
import { WorkspaceUtils } from "shared/workspace.utils";
import { getVariableOptions, multipleValuesQuery, timeFieldsQuery } from "core/utils";
import { getFilterConcatOperator, getVariableOptions, multipleValuesQuery, timeFieldsQuery } from "core/utils";
import { BackendSrv, getBackendSrv, getTemplateSrv, TemplateSrv } from "@grafana/runtime";
import { MINION_ID_CUSTOM_PROPERTY, SYSTEM_CUSTOM_PROPERTY } from "./constants/SourceProperties.constants";

export abstract class AlarmsDataSourceCore extends DataSourceBase<AlarmsQuery> {
private readonly queryAlarmsUrl = `${this.instanceSettings.url}${QUERY_ALARMS_RELATIVE_PATH}`;
Expand Down Expand Up @@ -65,19 +66,30 @@ export abstract class AlarmsDataSourceCore extends DataSourceBase<AlarmsQuery> {
Object.values(AlarmsQueryBuilderFields).map(field => {
const dataField = field.dataField as string;

return [
dataField,
this.isTimeField(dataField)
? timeFieldsQuery(dataField)
: multipleValuesQuery(dataField),
];
if (dataField === AlarmsQueryBuilderFields.SOURCE.dataField) {
return [dataField, this.getSourceTransformation()];
} else if (this.isTimeField(dataField)) {
return [dataField, timeFieldsQuery(dataField)];
} else {
return [dataField, multipleValuesQuery(dataField)];
}
})
);
Comment on lines 66 to 77
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use switch case with callback variable.

Suggested change
Object.values(AlarmsQueryBuilderFields).map(field => {
const dataField = field.dataField as string;
return [
dataField,
this.isTimeField(dataField)
? timeFieldsQuery(dataField)
: multipleValuesQuery(dataField),
];
if (dataField === AlarmsQueryBuilderFields.SOURCE.dataField) {
return [dataField, this.getSourceTransformation()];
} else if (this.isTimeField(dataField)) {
return [dataField, timeFieldsQuery(dataField)];
} else {
return [dataField, multipleValuesQuery(dataField)];
}
})
);
Object.values(AlarmsQueryBuilderFields).map(field => {
const dataField = field.dataField as string;
let callback;
switch(dataField) {
case AlarmsQueryBuilderFields.SOURCE.dataField:
callback = this.getSourceTransformation();
break;
}
return [dataField, callback];
})


private isTimeField(field: string): boolean {
return ALARMS_TIME_FIELDS.includes(field);
}

private getSourceTransformation(): ExpressionTransformFunction {
return (value: string, operation: string) => {
const systemExpression = multipleValuesQuery(`properties.${SYSTEM_CUSTOM_PROPERTY}`)(value, operation);
const minionExpression = multipleValuesQuery(`properties.${MINION_ID_CUSTOM_PROPERTY}`)(value, operation);
const logicalOperator = getFilterConcatOperator(operation);

return `(${systemExpression} ${logicalOperator} ${minionExpression})`;
};
}

private getStatusCodeErrorMessage(errorDetails: { statusCode: string; message: string }): string {
let errorMessage: string;
switch (errorDetails.statusCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ describe('AlarmsQueryBuilder', () => {
expect(conditionsContainer.item(0)?.textContent).toContain('Workspace Name');
});

it('should select source in query builder', () => {
const { conditionsContainer } = renderElement('source = "test-source"', [], [workspace]);

expect(conditionsContainer?.length).toBe(1);
const conditionText = conditionsContainer.item(0)?.textContent;
expect(conditionText).toContain('Source');
expect(conditionText).toContain('equals');
expect(conditionText).toContain('test-source');
});

it('should select value for alarm ID in query builder', () => {
const { conditionsContainer } = renderElement('alarmId = "test-alarm-123"');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,19 @@ export const AlarmsQueryBuilderFields: Record<string, QBField> = {
dataField: 'resourceType',
filterOperations: BASIC_STRING_FILTER_OPERATIONS,
},
SOURCE: {
label: 'Source',
dataField: 'source',
filterOperations: [
QueryBuilderOperations.EQUALS.name,
QueryBuilderOperations.DOES_NOT_EQUAL.name,
QueryBuilderOperations.IS_BLANK.name,
QueryBuilderOperations.IS_NOT_BLANK.name,
/* #AB#3422087 - Switch to BASIC_STRING_FILTER_OPERATIONS
once transformation support for "contains" and "does not contain"
is implemented */
],
},
WORKSPACE: {
label: 'Workspace',
dataField: 'workspace',
Expand All @@ -171,4 +184,5 @@ export const AlarmsQueryBuilderStaticFields: QBField[] = [
AlarmsQueryBuilderFields.KEYWORD,
AlarmsQueryBuilderFields.PROPERTIES,
AlarmsQueryBuilderFields.RESOURCE_TYPE,
AlarmsQueryBuilderFields.SOURCE,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const SYSTEM_CUSTOM_PROPERTY = 'system';
export const MINION_ID_CUSTOM_PROPERTY = 'minionId';