diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx index 19d91c1006cf05..936f1e477057d0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx @@ -35,7 +35,7 @@ const operationDefinitionMap: Record = { }), } as unknown) as GenericOperationDefinition, terms: { input: 'field' } as GenericOperationDefinition, - sum: { input: 'field' } as GenericOperationDefinition, + sum: { input: 'field', filterable: true } as GenericOperationDefinition, last_value: { input: 'field' } as GenericOperationDefinition, max: { input: 'field' } as GenericOperationDefinition, count: ({ @@ -928,6 +928,63 @@ invalid: " ).toEqual(['The operation average does not accept any parameter']); }); + it('returns an error if first argument type is passed multiple times', () => { + const formulas = [ + 'average(bytes, bytes)', + "sum(bytes, kql='category.keyword: *', bytes)", + 'moving_average(average(bytes), average(bytes))', + "moving_average(average(bytes), kql='category.keyword: *', average(bytes))", + 'moving_average(average(bytes, bytes), count())', + 'moving_average(moving_average(average(bytes, bytes), count(), count()))', + ]; + for (const formula of formulas) { + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(formula), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual( + expect.arrayContaining([ + expect.stringMatching( + /The operation (moving_average|average|sum) in the Formula requires a single (field|metric), found:/ + ), + ]) + ); + } + }); + + it('returns an error if a function received an argument of the wrong argument type in any position', () => { + const formulas = [ + 'average(bytes, count())', + "sum(bytes, kql='category.keyword: *', count(), count())", + 'average(bytes, bytes + 1)', + 'average(count(), bytes)', + 'moving_average(average(bytes), bytes)', + 'moving_average(bytes, bytes)', + 'moving_average(average(bytes), window=7, bytes)', + 'moving_average(window=7, bytes)', + "moving_average(kql='category.keyword: *', bytes)", + ]; + for (const formula of formulas) { + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(formula), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual( + expect.arrayContaining([ + expect.stringMatching( + /The operation (moving_average|average|sum) in the Formula does not support (metric|field) parameters, found:/ + ), + ]) + ); + } + }); + it('returns an error if the parameter passed to an operation is of the wrong type', () => { expect( formulaOperation.getErrorMessage!( @@ -1087,6 +1144,14 @@ invalid: " ) ).toEqual([`The first argument for ${fn} should be a field name. Found no field`]); } + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(`sum(kql='category.keyword: *')`), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual([`The first argument for sum should be a field name. Found category.keyword: *`]); }); it("returns a clear error when there's a missing function for a fullReference operation", () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 9806cdaad637ed..3770eae8cc9c4a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -431,7 +431,7 @@ ${'`square(last_value(length))`'} }, }; -export function isMathNode(node: TinymathAST) { +export function isMathNode(node: TinymathAST | string) { return isObject(node) && node.type === 'function' && tinymathFunctions[node.name]; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index 5b7a9beaa4e324..d65ef5ada8b379 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -7,7 +7,7 @@ import { isObject, partition } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { parse, TinymathLocation } from '@kbn/tinymath'; +import { parse, TinymathLocation, TinymathVariable } from '@kbn/tinymath'; import type { TinymathAST, TinymathFunction, TinymathNamedArgument } from '@kbn/tinymath'; import { esKuery, esQuery } from '../../../../../../../../src/plugins/data/public'; import { @@ -63,6 +63,19 @@ interface ValidationErrors { message: string; type: {}; }; + tooManyFirstArguments: { + message: string; + type: { + operation: string; + type: string; + text: string; + supported?: number; + }; + }; + wrongArgument: { + message: string; + type: { operation: string; text: string; type: string }; + }; } type ErrorTypes = keyof ValidationErrors; @@ -276,6 +289,25 @@ function getMessageFromId({ defaultMessage: 'Use only one of kql= or lucene=, not both', }); break; + case 'tooManyFirstArguments': + message = i18n.translate('xpack.lens.indexPattern.formulaOperationTooManyFirstArguments', { + defaultMessage: + 'The operation {operation} in the Formula requires a {supported, plural, one {single} other {supported}} {type}, found: {text}', + values: { + operation: out.operation, + text: out.text, + type: out.type, + supported: out.supported || 1, + }, + }); + break; + case 'wrongArgument': + message = i18n.translate('xpack.lens.indexPattern.formulaOperationwrongArgument', { + defaultMessage: + 'The operation {operation} in the Formula does not support {type} parameters, found: {text}', + values: { operation: out.operation, text: out.text, type: out.type }, + }); + break; // case 'mathRequiresFunction': // message = i18n.translate('xpack.lens.indexPattern.formulaMathRequiresFunctionLabel', { // defaultMessage; 'The function {name} requires an Elasticsearch function', @@ -531,14 +563,16 @@ function runFullASTValidation( } else { if (nodeOperation.input === 'field') { if (shouldHaveFieldArgument(node)) { - if (!isFirstArgumentValidType(firstArg, 'variable')) { + if (!isArgumentValidType(firstArg, 'variable')) { if (isMathNode(firstArg)) { errors.push( getMessageFromId({ messageId: 'wrongFirstArgument', values: { operation: node.name, - type: 'field', + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), argument: `math operation`, }, locations: node.location ? [node.location] : [], @@ -550,7 +584,9 @@ function runFullASTValidation( messageId: 'wrongFirstArgument', values: { operation: node.name, - type: 'field', + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), argument: getValueOrName(firstArg) || i18n.translate('xpack.lens.indexPattern.formulaNoFieldForOperation', { @@ -561,6 +597,25 @@ function runFullASTValidation( }) ); } + } else { + // If the first argument is valid proceed with the other arguments validation + const fieldErrors = validateFieldArguments(node, variables, { + isFieldOperation: true, + firstArg, + }); + if (fieldErrors.length) { + errors.push(...fieldErrors); + } + } + const functionErrors = validateFunctionArguments(node, functions, 0, { + isFieldOperation: true, + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + firstArgValidation: false, + }); + if (functionErrors.length) { + errors.push(...functionErrors); } } else { // Named arguments only @@ -602,16 +657,20 @@ function runFullASTValidation( if (nodeOperation.input === 'fullReference') { // What about fn(7 + 1)? We may want to allow that // In general this should be handled down the Esaggs route rather than here - if ( - !isFirstArgumentValidType(firstArg, 'function') || - (isMathNode(firstArg) && validateMathNodes(firstArg, missingVariablesSet).length) - ) { + const isFirstArgumentNotValid = Boolean( + !isArgumentValidType(firstArg, 'function') || + (isMathNode(firstArg) && validateMathNodes(firstArg, missingVariablesSet).length) + ); + // First field has a special handling + if (isFirstArgumentNotValid) { errors.push( getMessageFromId({ messageId: 'wrongFirstArgument', values: { operation: node.name, - type: 'operation', + type: i18n.translate('xpack.lens.indexPattern.formulaOperationValue', { + defaultMessage: 'operation', + }), argument: getValueOrName(firstArg) || i18n.translate('xpack.lens.indexPattern.formulaNoOperation', { @@ -622,6 +681,21 @@ function runFullASTValidation( }) ); } + // Check for multiple function passed + const requiredFunctions = nodeOperation.requiredReferences + ? nodeOperation.requiredReferences.length + : 1; + const functionErrors = validateFunctionArguments(node, functions, requiredFunctions, { + isFieldOperation: false, + firstArgValidation: isFirstArgumentNotValid, + type: i18n.translate('xpack.lens.indexPattern.formulaMetricValue', { + defaultMessage: 'metric', + }), + }); + if (functionErrors.length) { + errors.push(...functionErrors); + } + if (!canHaveParams(nodeOperation) && namedArguments.length) { errors.push( getMessageFromId({ @@ -633,6 +707,14 @@ function runFullASTValidation( }) ); } else { + // check for fields passed at any position + const fieldErrors = validateFieldArguments(node, variables, { + isFieldOperation: false, + firstArg, + }); + if (fieldErrors.length) { + errors.push(...fieldErrors); + } const argumentsErrors = validateNameArguments( node, nodeOperation, @@ -736,7 +818,7 @@ export function hasFunctionFieldArgument(type: string) { return !['count'].includes(type); } -export function isFirstArgumentValidType(arg: TinymathAST, type: TinymathNodeTypes['type']) { +export function isArgumentValidType(arg: TinymathAST | string, type: TinymathNodeTypes['type']) { return isObject(arg) && arg.type === type; } @@ -812,3 +894,109 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set, + { isFieldOperation, firstArg }: { isFieldOperation: boolean; firstArg: TinymathAST } +) { + const fields = variables.filter( + (arg) => isArgumentValidType(arg, 'variable') && !isMathNode(arg) + ); + const errors = []; + if (isFieldOperation && (fields.length > 1 || (fields.length === 1 && fields[0] !== firstArg))) { + errors.push( + getMessageFromId({ + messageId: 'tooManyFirstArguments', + values: { + operation: node.name, + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + supported: 1, + text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), + }, + locations: node.location ? [node.location] : [], + }) + ); + } + if (!isFieldOperation && fields.length) { + errors.push( + getMessageFromId({ + messageId: 'wrongArgument', + values: { + operation: node.name, + text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + }, + locations: node.location ? [node.location] : [], + }) + ); + } + return errors; +} + +function validateFunctionArguments( + node: TinymathFunction, + functions: TinymathFunction[], + requiredFunctions: number = 0, + { + isFieldOperation, + firstArgValidation, + type, + }: { isFieldOperation: boolean; firstArgValidation: boolean; type: string } +) { + const errors = []; + // For math operation let the native operation run its own validation + const [esOperations, mathOperations] = partition(functions, (arg) => !isMathNode(arg)); + if (esOperations.length > requiredFunctions) { + if (isFieldOperation) { + errors.push( + getMessageFromId({ + messageId: 'wrongArgument', + values: { + operation: node.name, + text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + type: i18n.translate('xpack.lens.indexPattern.formulaMetricValue', { + defaultMessage: 'metric', + }), + }, + locations: node.location ? [node.location] : [], + }) + ); + } else { + errors.push( + getMessageFromId({ + messageId: 'tooManyFirstArguments', + values: { + operation: node.name, + type, + supported: requiredFunctions, + text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + }, + locations: node.location ? [node.location] : [], + }) + ); + } + } + // full reference operation have another way to handle math operations + if ( + isFieldOperation && + ((!firstArgValidation && mathOperations.length) || mathOperations.length > 1) + ) { + errors.push( + getMessageFromId({ + messageId: 'wrongArgument', + values: { + operation: node.name, + type, + text: (mathOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + }, + locations: node.location ? [node.location] : [], + }) + ); + } + return errors; +}