From faeaeb61a0e80a458eec9255cbe8ff9a22e5a541 Mon Sep 17 00:00:00 2001 From: Amol Umbark Date: Mon, 26 Dec 2022 15:10:01 +0530 Subject: [PATCH] fix: added validations on query builder (#1906) Co-authored-by: mindhash Co-authored-by: Pranay Prateek Co-authored-by: Ankit Nayan --- .../QueryBuilder/QueryBuilder.tsx | 105 ++++++++---------- .../SearchFields/QueryBuilder/utils.ts | 14 --- .../SearchFields/Suggestions.tsx | 25 +++-- .../LogsSearchFilter/SearchFields/index.tsx | 83 +++++++++++++- .../LogsSearchFilter/SearchFields/utils.ts | 60 +++++++++- frontend/src/lib/logql/reverseParser.ts | 16 ++- frontend/src/lib/logql/tokens.ts | 58 ++++++++++ 7 files changed, 273 insertions(+), 88 deletions(-) diff --git a/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/QueryBuilder.tsx b/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/QueryBuilder.tsx index 1aab2e8e75..5b06c3ca82 100644 --- a/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/QueryBuilder.tsx +++ b/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, { useEffect, useMemo, useRef, useState } from 'react'; +import React, { useMemo } 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((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({ /> ) : ( 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} /> )} @@ -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 { - const { - searchFilter: { parsedQuery }, - } = useSelector((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]); - +}: QueryBuilderProps): JSX.Element { const handleUpdate = (query: Query, queryIndex: number): void => { - const updatedParsedQuery = generatedQueryStructure; - updatedParsedQuery[queryIndex] = parseQuery(query) as never; - - const flatParsedQuery = flatten(updatedParsedQuery).filter((q) => q.value); - keyPrefixRef.current = hashCode(JSON.stringify(flatParsedQuery)); - updateParsedQuery(flatParsedQuery); + const updated = [...fieldsQuery]; + updated[queryIndex] = query as never; // parseQuery(query) as never; + setFieldsQuery(updated); }; const handleDelete = (queryIndex: number): void => { - const updatedParsedQuery = generatedQueryStructure; - updatedParsedQuery.splice(queryIndex - 1, 2); + const updated = [...fieldsQuery]; + if (queryIndex !== 0) updated.splice(queryIndex - 1, 2); + else updated.splice(queryIndex, 2); - const flatParsedQuery = flatten(updatedParsedQuery).filter((q) => q.value); - keyPrefixRef.current = v4(); - updateParsedQuery(flatParsedQuery); + setFieldsQuery(updated); }; - const QueryUI = (): JSX.Element | JSX.Element[] => - generatedQueryStructure.map((query, idx) => { - if (Array.isArray(query)) - return ( + const QueryUI = ( + fieldsQuery: QueryFields[][], + ): JSX.Element | JSX.Element[] => { + const result: JSX.Element[] = []; + fieldsQuery.forEach((query, idx) => { + if (Array.isArray(query) && query.length > 1) { + result.push( + />, ); - - return ( -
- -
- ); + } else { + result.push( +
+ +
, + ); + } }); + return result; + }; return ( <> - + LOG QUERY BUILDER - {QueryUI()} + {QueryUI(fieldsQuery)} ); } diff --git a/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/utils.ts b/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/utils.ts index 2641d8af35..2025f5d8f2 100644 --- a/frontend/src/container/LogsSearchFilter/SearchFields/QueryBuilder/utils.ts +++ b/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), - )}`; -}; diff --git a/frontend/src/container/LogsSearchFilter/SearchFields/Suggestions.tsx b/frontend/src/container/LogsSearchFilter/SearchFields/Suggestions.tsx index 838d790954..56255d1ed1 100644 --- a/frontend/src/container/LogsSearchFilter/SearchFields/Suggestions.tsx +++ b/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 { - const dispatch = useDispatch(); - +function SuggestedItem({ + name, + type, + applySuggestion, +}: SuggestedItemProps): JSX.Element { const addSuggestedField = (): void => { - dispatch({ - type: ADD_SEARCH_FIELD_QUERY_STRING, - payload: name, - }); + applySuggestion(name); }; return ( + + ); } diff --git a/frontend/src/container/LogsSearchFilter/SearchFields/utils.ts b/frontend/src/container/LogsSearchFilter/SearchFields/utils.ts index ae091dc061..fec1c45bee 100644 --- a/frontend/src/container/LogsSearchFilter/SearchFields/utils.ts +++ b/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), + )}`; +}; diff --git a/frontend/src/lib/logql/reverseParser.ts b/frontend/src/lib/logql/reverseParser.ts index 7a80138ad4..c83da13829 100644 --- a/frontend/src/lib/logql/reverseParser.ts +++ b/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 { - queryString += query.value; + 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); diff --git a/frontend/src/lib/logql/tokens.ts b/frontend/src/lib/logql/tokens.ts index 1001f29f6a..e4eb64fe3c 100644 --- a/frontend/src/lib/logql/tokens.ts +++ b/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; + } +};