From 134e82c666525070bdd8e498fe520550c447b0e7 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 28 Oct 2020 04:46:11 -0700 Subject: [PATCH] review updates --- .../common/executor/executor.test.ts | 31 +++++++++++++++++++ .../expressions/common/executor/executor.ts | 22 ++++++++----- .../expression_functions/specs/kibana.ts | 22 ------------- .../common/service/expressions_services.ts | 12 +++---- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/plugins/expressions/common/executor/executor.test.ts b/src/plugins/expressions/common/executor/executor.test.ts index a658d3457407c..308d6f7e71814 100644 --- a/src/plugins/expressions/common/executor/executor.test.ts +++ b/src/plugins/expressions/common/executor/executor.test.ts @@ -22,6 +22,7 @@ import * as expressionTypes from '../expression_types'; import * as expressionFunctions from '../expression_functions'; import { Execution } from '../execution'; import { ExpressionAstFunction, parseExpression } from '../ast'; +import { MigrateFunction } from '../../../kibana_utils/common/persistable_state'; describe('Executor', () => { test('can instantiate', () => { @@ -158,6 +159,7 @@ describe('Executor', () => { const injectFn = jest.fn().mockImplementation((args, references) => args); const extractFn = jest.fn().mockReturnValue({ args: {}, references: [] }); + const migrateFn = jest.fn().mockImplementation((args) => args); const fooFn = { name: 'foo', @@ -174,6 +176,14 @@ describe('Executor', () => { inject: (state: ExpressionAstFunction['arguments']) => { return injectFn(state); }, + migrations: { + '7.10.0': (((state: ExpressionAstFunction, version: string): ExpressionAstFunction => { + return migrateFn(state, version); + }) as any) as MigrateFunction, + '7.10.1': (((state: ExpressionAstFunction, version: string): ExpressionAstFunction => { + return migrateFn(state, version); + }) as any) as MigrateFunction, + }, fn: jest.fn(), }; executor.registerFunction(fooFn); @@ -194,5 +204,26 @@ describe('Executor', () => { expect(extractFn).toBeCalledTimes(5); }); }); + + describe('.migrate', () => { + test('calls migrate function for every expression function in expression', () => { + executor.migrate( + parseExpression('foo bar="baz" | foo bar={foo bar="baz" | foo bar={foo bar="baz"}}'), + '7.10.0' + ); + expect(migrateFn).toBeCalledTimes(5); + }); + }); + + describe('.migrateToLatest', () => { + test('calls extract function for every expression function in expression', () => { + migrateFn.mockClear(); + executor.migrateToLatest( + parseExpression('foo bar="baz" | foo bar={foo bar="baz" | foo bar={foo bar="baz"}}'), + '7.10.0' + ); + expect(migrateFn).toBeCalledTimes(10); + }); + }); }); }); diff --git a/src/plugins/expressions/common/executor/executor.ts b/src/plugins/expressions/common/executor/executor.ts index 373f6dbecff62..19fc4cf5a14a2 100644 --- a/src/plugins/expressions/common/executor/executor.ts +++ b/src/plugins/expressions/common/executor/executor.ts @@ -90,8 +90,11 @@ export class FunctionsRegistry implements IRegistry { const semverGte = (semver1: string, semver2: string) => { const regex = /^([0-9]+)\.([0-9]+)\.([0-9]+)$/; - const [major1, minor1, patch1] = semver1.match(regex) as RegExpMatchArray; - const [major2, minor2, patch2] = semver2.match(regex) as RegExpMatchArray; + const matches1 = regex.exec(semver1) as RegExpMatchArray; + const matches2 = regex.exec(semver2) as RegExpMatchArray; + + const [, major1, minor1, patch1] = matches1; + const [, major2, minor2, patch2] = matches2; return ( major1 > major2 || @@ -260,17 +263,22 @@ export class Executor = Record { - link = fn.migrations[version](link) as ExpressionAstFunction; + if (!fn.migrations[version]) return link; + const updatedAst = fn.migrations[version](link) as ExpressionAstFunction; + link.arguments = updatedAst.arguments; + link.type = updatedAst.type; }); } - migrateToLatest(ast: unknown, version: string) { + public migrateToLatest(ast: unknown, version: string) { return this.walkAst(cloneDeep(ast) as ExpressionAstExpression, (fn, link) => { - for (const key in Object.keys(fn.migrations)) { + for (const key of Object.keys(fn.migrations)) { if (semverGte(key, version)) { - link = fn.migrations[key](link) as ExpressionAstFunction; + const updatedAst = fn.migrations[key](link) as ExpressionAstFunction; + link.arguments = updatedAst.arguments; + link.type = updatedAst.type; } } }); diff --git a/src/plugins/expressions/common/expression_functions/specs/kibana.ts b/src/plugins/expressions/common/expression_functions/specs/kibana.ts index 592e41b3c0c84..3ec4c23eab28d 100644 --- a/src/plugins/expressions/common/expression_functions/specs/kibana.ts +++ b/src/plugins/expressions/common/expression_functions/specs/kibana.ts @@ -20,7 +20,6 @@ import { i18n } from '@kbn/i18n'; import { ExpressionFunctionDefinition } from '../types'; import { ExpressionValueSearchContext } from '../../expression_types'; -import { MigrateFunction } from '../../../../kibana_utils/common/persistable_state'; const toArray = (query: undefined | T | T[]): T[] => !query ? [] : Array.isArray(query) ? query : [query]; @@ -33,23 +32,6 @@ export type ExpressionFunctionKibana = ExpressionFunctionDefinition< ExpressionValueSearchContext >; -type MYSTATE_7_10_0 = { - name: 'kibana'; - args: {}; -}; - -type MYSTATE_7_10_1 = { - name: 'kibana_input'; - args: {}; -}; - -const MIGRATE7_10_1: MigrateFunction = (state) => { - return { - name: 'kibana_input', - args: state.args, - }; -}; - export const kibana: ExpressionFunctionKibana = { name: 'kibana', type: 'kibana_context', @@ -62,10 +44,6 @@ export const kibana: ExpressionFunctionKibana = { args: {}, - migrations: { - '7.10.1': (MIGRATE7_10_1 as any) as MigrateFunction, - }, - fn(input, _, { getSearchContext }) { const output: ExpressionValueSearchContext = { // TODO: This spread is left here for legacy reasons, possibly Lens uses it. diff --git a/src/plugins/expressions/common/service/expressions_services.ts b/src/plugins/expressions/common/service/expressions_services.ts index eb0cf0936d5b4..0f898563c3d0e 100644 --- a/src/plugins/expressions/common/service/expressions_services.ts +++ b/src/plugins/expressions/common/service/expressions_services.ts @@ -304,20 +304,20 @@ export class ExpressionsService implements PersistableState { return this.executor.migrate(state, version); }; /** - * Injects saved object references into expression AST + * Migrates expression to the latest version * @param state expression AST to update - * @param references array of saved object references - * @returns new expression AST with references injected + * @param version the version of kibana in which expression was created + * @returns migrated expression AST */ public readonly migrateToLatest = (state: unknown, version: string) => { return this.executor.migrateToLatest(state, version);