fix: added validations on query builder (#1906)

Co-authored-by: mindhash <mindhash@mindhashs-MacBook-Pro.local>
Co-authored-by: Pranay Prateek <pranay@signoz.io>
Co-authored-by: Ankit Nayan <ankit@signoz.io>

frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/QueryBuilder.tsx

@@ -12,19 +12,15 @@ import {
 	QueryOperatorsMultiVal,
 	QueryOperatorsSingleVal,
 } from 'lib/logql/tokens';
-import { flatten } from 'lodash-es';
+import React, { useMemo } from 'react';
-import React, { useEffect, useMemo, useRef, useState } from 'react';
 import { useSelector } from 'react-redux';
 import { AppState } from 'store/reducers';
 import { ILogsReducer } from 'types/reducer/logs';
-import { v4 } from 'uuid';
 
-import { SearchFieldsProps } from '..';
 import FieldKey from '../FieldKey';
 import { QueryFieldContainer } from '../styles';
-import { createParsedQueryStructure } from '../utils';
+import { QueryFields } from '../utils';
 import { Container, QueryWrapper } from './styles';
-import { hashCode, parseQuery } from './utils';
 
 const { Option } = Select;
 
@@ -68,7 +64,6 @@ function QueryField({
 	const {
 		fields: { selected },
 	} = useSelector<AppState, ILogsReducer>((store) => store.logs);
-
 	const getFieldType = (inputKey: string): string => {
 		// eslint-disable-next-line no-restricted-syntax
 		for (const selectedField of selected) {
@@ -147,9 +142,12 @@ function QueryField({
 					/>
 				) : (
 					<Input
-						onChange={(e): void => handleChange(2, e.target.value)}
+						onChange={(e): void => {
+							handleChange(2, e.target.value);
+						}}
 						style={{ width: '100%' }}
 						defaultValue={query[2] && query[2].value}
+						value={query[2] && query[2].value}
 					/>
 				)}
 			</div>
@@ -165,85 +163,78 @@ function QueryField({
 }
 
 interface QueryConditionFieldProps {
-	query: { value: string | string[]; type: string }[];
+	query: QueryFields;
 	queryIndex: number;
 	onUpdate: (arg0: unknown, arg1: number) => void;
 }
 
 export type Query = { value: string | string[]; type: string }[];
 
+export interface QueryBuilderProps {
+	keyPrefix: string;
+	onDropDownToggleHandler: (value: boolean) => VoidFunction;
+	fieldsQuery: QueryFields[][];
+	setFieldsQuery: (q: QueryFields[][]) => void;
+}
+
 function QueryBuilder({
-	updateParsedQuery,
+	keyPrefix,
+	fieldsQuery,
+	setFieldsQuery,
 	onDropDownToggleHandler,
-}: SearchFieldsProps): JSX.Element {
+}: QueryBuilderProps): JSX.Element {
-	const {
-		searchFilter: { parsedQuery },
-	} = useSelector<AppState, ILogsReducer>((store) => store.logs);
-
-	const keyPrefixRef = useRef(hashCode(JSON.stringify(parsedQuery)));
-	const [keyPrefix, setKeyPrefix] = useState(keyPrefixRef.current);
-	const generatedQueryStructure = createParsedQueryStructure(
-		parsedQuery as never[],
-	);
-
-	useEffect(() => {
-		const incomingHashCode = hashCode(JSON.stringify(parsedQuery));
-		if (incomingHashCode !== keyPrefixRef.current) {
-			keyPrefixRef.current = incomingHashCode;
-			setKeyPrefix(incomingHashCode);
-		}
-	}, [parsedQuery]);
-
 	const handleUpdate = (query: Query, queryIndex: number): void => {
-		const updatedParsedQuery = generatedQueryStructure;
+		const updated = [...fieldsQuery];
-		updatedParsedQuery[queryIndex] = parseQuery(query) as never;
+		updated[queryIndex] = query as never; // parseQuery(query) as never;
-
+		setFieldsQuery(updated);
-		const flatParsedQuery = flatten(updatedParsedQuery).filter((q) => q.value);
-		keyPrefixRef.current = hashCode(JSON.stringify(flatParsedQuery));
-		updateParsedQuery(flatParsedQuery);
 	};
 
 	const handleDelete = (queryIndex: number): void => {
-		const updatedParsedQuery = generatedQueryStructure;
+		const updated = [...fieldsQuery];
-		updatedParsedQuery.splice(queryIndex - 1, 2);
+		if (queryIndex !== 0) updated.splice(queryIndex - 1, 2);
+		else updated.splice(queryIndex, 2);
 
-		const flatParsedQuery = flatten(updatedParsedQuery).filter((q) => q.value);
+		setFieldsQuery(updated);
-		keyPrefixRef.current = v4();
-		updateParsedQuery(flatParsedQuery);
 	};
 
-	const QueryUI = (): JSX.Element | JSX.Element[] =>
+	const QueryUI = (
-		generatedQueryStructure.map((query, idx) => {
+		fieldsQuery: QueryFields[][],
-			if (Array.isArray(query))
+	): JSX.Element | JSX.Element[] => {
-				return (
+		const result: JSX.Element[] = [];
+		fieldsQuery.forEach((query, idx) => {
+			if (Array.isArray(query) && query.length > 1) {
+				result.push(
 					<QueryField
 						key={keyPrefix + idx}
 						query={query as never}
 						queryIndex={idx}
 						onUpdate={handleUpdate as never}
 						onDelete={handleDelete}
-					/>
+					/>,
 				);
-
+			} else {
-			return (
+				result.push(
 					<div key={keyPrefix + idx}>
 						<QueryConditionField
-						query={query}
+							query={Array.isArray(query) ? query[0] : query}
 							queryIndex={idx}
 							onUpdate={handleUpdate as never}
 						/>
-				</div>
+					</div>,
 				);
+			}
 		});
+		return result;
+	};
 
 	return (
 		<>
-			<Container isMargin={generatedQueryStructure.length === 0}>
+			<Container isMargin={fieldsQuery.length === 0}>
 				<CategoryHeading>LOG QUERY BUILDER</CategoryHeading>
 				<CloseSquareOutlined onClick={onDropDownToggleHandler(false)} />
 			</Container>
 
-			<QueryWrapper>{QueryUI()}</QueryWrapper>
+			<QueryWrapper key={keyPrefix}>{QueryUI(fieldsQuery)}</QueryWrapper>
 		</>
 	);
 }

frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/utils.ts

@@ -21,17 +21,3 @@ export const parseQuery = (queries: Query): Query => {
 	}
 	return queries;
 };
-
-export const hashCode = (s: string): string => {
-	if (!s) {
-		return '0';
-	}
-	return `${Math.abs(
-		s.split('').reduce((a, b) => {
-			// eslint-disable-next-line no-bitwise, no-param-reassign
-			a = (a << 5) - a + b.charCodeAt(0);
-			// eslint-disable-next-line no-bitwise
-			return a & a;
-		}, 0),
-	)}`;
-};

frontend/src/container/LogsSearchFilter/SearchFields/Suggestions.tsx

@@ -2,9 +2,9 @@ import { Button } from 'antd';
 import CategoryHeading from 'components/Logs/CategoryHeading';
 import map from 'lodash-es/map';
 import React from 'react';
-import { useDispatch, useSelector } from 'react-redux';
+import { useSelector } from 'react-redux';
 import { AppState } from 'store/reducers';
-import { ADD_SEARCH_FIELD_QUERY_STRING } from 'types/actions/logs';
+// import { ADD_SEARCH_FIELD_QUERY_STRING } from 'types/actions/logs';
 import { ILogsReducer } from 'types/reducer/logs';
 
 import FieldKey from './FieldKey';
@@ -12,15 +12,15 @@ import FieldKey from './FieldKey';
 interface SuggestedItemProps {
 	name: string;
 	type: string;
+	applySuggestion: (name: string) => void;
 }
-function SuggestedItem({ name, type }: SuggestedItemProps): JSX.Element {
+function SuggestedItem({
-	const dispatch = useDispatch();
+	name,
-
+	type,
+	applySuggestion,
+}: SuggestedItemProps): JSX.Element {
 	const addSuggestedField = (): void => {
-		dispatch({
+		applySuggestion(name);
-			type: ADD_SEARCH_FIELD_QUERY_STRING,
-			payload: name,
-		});
 	};
 	return (
 		<Button
@@ -33,7 +33,11 @@ function SuggestedItem({ name, type }: SuggestedItemProps): JSX.Element {
 	);
 }
 
-function Suggestions(): JSX.Element {
+interface SuggestionsProps {
+	applySuggestion: (name: string) => void;
+}
+
+function Suggestions({ applySuggestion }: SuggestionsProps): JSX.Element {
 	const {
 		fields: { selected },
 	} = useSelector<AppState, ILogsReducer>((store) => store.logs);
@@ -47,6 +51,7 @@ function Suggestions(): JSX.Element {
 						key={JSON.stringify(field)}
 						name={field.name}
 						type={field.type}
+						applySuggestion={applySuggestion}
 					/>
 				))}
 			</div>

frontend/src/container/LogsSearchFilter/SearchFields/index.tsx

@@ -1,8 +1,20 @@
-import React from 'react';
+import { Button, notification, Row } from 'antd';
+import { flatten } from 'lodash-es';
+import React, { useCallback, useEffect, useRef, useState } from 'react';
+import { useSelector } from 'react-redux';
+import { AppState } from 'store/reducers';
+import { ILogsReducer } from 'types/reducer/logs';
 
 import QueryBuilder from './QueryBuilder/QueryBuilder';
 import Suggestions from './Suggestions';
-import { QueryFields } from './utils';
+import {
+	createParsedQueryStructure,
+	fieldsQueryIsvalid,
+	hashCode,
+	initQueryKOVPair,
+	prepareConditionOperator,
+	QueryFields,
+} from './utils';
 
 export interface SearchFieldsProps {
 	updateParsedQuery: (query: QueryFields[]) => void;
@@ -13,13 +25,76 @@ function SearchFields({
 	updateParsedQuery,
 	onDropDownToggleHandler,
 }: SearchFieldsProps): JSX.Element {
+	const {
+		searchFilter: { parsedQuery },
+	} = useSelector<AppState, ILogsReducer>((store) => store.logs);
+
+	const [fieldsQuery, setFieldsQuery] = useState(
+		createParsedQueryStructure([...parsedQuery] as never[]),
+	);
+
+	const keyPrefixRef = useRef(hashCode(JSON.stringify(fieldsQuery)));
+
+	useEffect(() => {
+		setFieldsQuery(createParsedQueryStructure([...parsedQuery] as never[]));
+	}, [parsedQuery]);
+
+	const addSuggestedField = useCallback(
+		(name: string): void => {
+			if (!name) {
+				return;
+			}
+
+			const query = [...fieldsQuery];
+
+			if (fieldsQuery.length > 0) {
+				query.push([prepareConditionOperator()]);
+			}
+
+			const newField: QueryFields[] = [];
+			initQueryKOVPair(name).forEach((q) => newField.push(q));
+
+			query.push(newField);
+			keyPrefixRef.current = hashCode(JSON.stringify(query));
+			setFieldsQuery(query);
+		},
+		[fieldsQuery, setFieldsQuery],
+	);
+
+	const applyUpdate = useCallback(
+		(e): void => {
+			e.preventDefault();
+			const flatParsedQuery = flatten(fieldsQuery);
+
+			if (!fieldsQueryIsvalid(flatParsedQuery)) {
+				notification.error({
+					message: 'Please enter a valid criteria for each of the selected fields',
+				});
+				return;
+			}
+
+			keyPrefixRef.current = hashCode(JSON.stringify(flatParsedQuery));
+			updateParsedQuery(flatParsedQuery);
+			onDropDownToggleHandler(false)();
+		},
+		[onDropDownToggleHandler, fieldsQuery, updateParsedQuery],
+	);
+
 	return (
 		<>
 			<QueryBuilder
+				key={keyPrefixRef.current}
+				keyPrefix={keyPrefixRef.current}
 				onDropDownToggleHandler={onDropDownToggleHandler}
-				updateParsedQuery={updateParsedQuery}
+				fieldsQuery={fieldsQuery}
+				setFieldsQuery={setFieldsQuery}
 			/>
-			<Suggestions />
+			<Row style={{ justifyContent: 'flex-end', paddingRight: '2.4rem' }}>
+				<Button type="primary" onClick={applyUpdate}>
+					Apply
+				</Button>
+			</Row>
+			<Suggestions applySuggestion={addSuggestedField} />
 		</>
 	);
 }

frontend/src/container/LogsSearchFilter/SearchFields/utils.ts

@@ -2,11 +2,30 @@
 // @ts-ignore
 // @ts-nocheck
 
-import { QueryTypes, QueryOperatorsSingleVal } from 'lib/logql/tokens';
+import { QueryTypes, ConditionalOperators, ValidTypeSequence, ValidTypeValue } from 'lib/logql/tokens';
 
 export interface QueryFields {
 	type: keyof typeof QueryTypes;
-	value: string;
+	value: string | string[];
+}
+
+
+export function fieldsQueryIsvalid(queryFields: QueryFields[]): boolean {
+	let lastOp: string;
+	let result = true;
+	queryFields.forEach((q, idx)=> {
+		
+		if (!q.value || q.value === null || q.value === '') result = false;
+		
+		if (Array.isArray(q.value) && q.value.length === 0 ) result = false;
+
+		const nextOp = idx < queryFields.length  ? queryFields[idx+1]: undefined;
+		if (!ValidTypeSequence(lastOp?.type, q?.type, nextOp?.type)) result = false
+
+		if (!ValidTypeValue(lastOp?.value, q.value)) result = false;
+		lastOp = q;
+	});
+	return result
 }
 
 export const queryKOVPair = (): QueryFields[] => [
@@ -23,6 +42,29 @@ export const queryKOVPair = (): QueryFields[] => [
 		value: null,
 	},
 ];
+
+export const initQueryKOVPair = (name?: string = null, op?: string = null  , value?: string | string[] = null ): QueryFields[] => [
+	{
+		type: QueryTypes.QUERY_KEY,
+		value: name,
+	},
+	{
+		type: QueryTypes.QUERY_OPERATOR,
+		value: op,
+	},
+	{
+		type: QueryTypes.QUERY_VALUE,
+		value: value,
+	},
+];
+
+export const prepareConditionOperator = (op?: string = ConditionalOperators.AND): QueryFields => {
+	return {
+		type: QueryTypes.CONDITIONAL_OPERATOR,
+		value: op,
+	}
+}
+
 export const createParsedQueryStructure = (parsedQuery = []) => {
 	if (!parsedQuery.length) {
 		return parsedQuery;
@@ -64,3 +106,17 @@ export const createParsedQueryStructure = (parsedQuery = []) => {
 	});
 	return structuredArray;
 };
+
+export const hashCode = (s: string): string => {
+	if (!s) {
+		return '0';
+	}
+	return `${Math.abs(
+		s.split('').reduce((a, b) => {
+			// eslint-disable-next-line no-bitwise, no-param-reassign
+			a = (a << 5) - a + b.charCodeAt(0);
+			// eslint-disable-next-line no-bitwise
+			return a & a;
+		}, 0),
+	)}`;
+};

frontend/src/lib/logql/reverseParser.ts

@@ -2,20 +2,34 @@
 // @ts-ignore
 // @ts-nocheck
 
+import { QueryTypes, StringTypeQueryOperators } from "./tokens";
+
 export const reverseParser = (
 	parserQueryArr: { type: string; value: any }[] = [],
 ) => {
 	let queryString = '';
+	let lastToken: { type: string; value: any }; 
 	parserQueryArr.forEach((query) => {
 		if (queryString) {
 			queryString += ' ';
 		}
 
 		if (Array.isArray(query.value) && query.value.length > 0) {
+			// if the values are array type, here we spread them in 
+			// ('a', 'b') format 
 			queryString += `(${query.value.map((val) => `'${val}'`).join(',')})`;
+		} else {
+			if (query.type === QueryTypes.QUERY_VALUE 
+				&& lastToken.type === QueryTypes.QUERY_OPERATOR 
+				&& Object.values(StringTypeQueryOperators).includes(lastToken.value) ) {
+					// for operators that need string type value, here we append single 
+					// quotes. if the content has single quote they would be removed 
+					queryString += `'${query.value?.replace(/'/g, '')}'`;
 			} else {
 				queryString += query.value;
 			}
+		}
+		lastToken = query;
 	});
 
 	// console.log(queryString);

frontend/src/lib/logql/tokens.ts

@@ -7,6 +7,21 @@ export const QueryOperatorsSingleVal = {
 	NCONTAINS: 'NCONTAINS',
 };
 
+// list of operators that support only number values
+export const NumTypeQueryOperators = {
+	GTE: 'GTE',
+	GT: 'GT',
+	LTE: 'LTE',
+	LT: 'LT',
+};
+
+// list of operators that support only string values
+export const StringTypeQueryOperators = {
+	CONTAINS: 'CONTAINS',
+	NCONTAINS: 'NCONTAINS',
+};
+
+// list of operators that support array values
 export const QueryOperatorsMultiVal = {
 	IN: 'IN',
 	NIN: 'NIN',
@@ -23,3 +38,46 @@ export const QueryTypes = {
 	QUERY_VALUE: 'QUERY_VALUE',
 	CONDITIONAL_OPERATOR: 'CONDITIONAL_OPERATOR',
 };
+
+export const ValidTypeValue = (
+	op: string,
+	value: string | string[],
+): boolean => {
+	if (!op) return true;
+	if (Object.values(NumTypeQueryOperators).includes(op)) {
+		if (Array.isArray(value)) return false;
+		return !Number.isNaN(Number(value));
+	}
+	return true;
+};
+
+// ValidTypeSequence takes prior, current and next op to confirm
+// the proper sequence. For example, if QUERY_VALUE needs to be
+// in between QUERY_OPERATOR and (empty or CONDITIONAL_OPERATOR).
+export const ValidTypeSequence = (
+	prior: string | undefined,
+	current: string | undefined,
+	next: string | undefined,
+): boolean => {
+	switch (current) {
+		case QueryTypes.QUERY_KEY:
+			// query key can have an empty prior
+			if (!prior) return true;
+			return [QueryTypes.CONDITIONAL_OPERATOR].includes(prior);
+		case QueryTypes.QUERY_OPERATOR:
+			// empty prior is not allowed
+			if (!prior || ![QueryTypes.QUERY_KEY].includes(prior)) return false;
+			if (!next || ![QueryTypes.QUERY_VALUE].includes(next)) return false;
+			return true;
+		case QueryTypes.QUERY_VALUE:
+			// empty prior is not allowed
+			if (!prior) return false;
+			return [QueryTypes.QUERY_OPERATOR].includes(prior);
+		case QueryTypes.CONDITIONAL_OPERATOR:
+			// empty prior is not allowed
+			if (!next) return false;
+			return [QueryTypes.QUERY_KEY].includes(next);
+		default:
+			return false;
+	}
+};