From 613f873dd65f5780f63c2f8bdbb00a83111ce5ec Mon Sep 17 00:00:00 2001 From: ppisljar Date: Thu, 15 Oct 2020 05:20:16 -0700 Subject: [PATCH] cleaning up expression service types --- .../execution/execution.abortion.test.ts | 3 +- .../common/execution/execution.test.ts | 14 ++-- .../expressions/common/execution/execution.ts | 74 ++++++++----------- .../execution/execution_contract.test.ts | 2 +- .../common/execution/execution_contract.ts | 11 +-- .../expressions/common/execution/types.ts | 2 + .../expressions/common/executor/executor.ts | 42 +++++------ .../expression_functions/specs/kibana.ts | 8 +- .../specs/tests/kibana.test.ts | 4 +- .../specs/tests/theme.test.ts | 1 + .../specs/tests/var.test.ts | 1 + .../specs/tests/var_set.test.ts | 1 + src/plugins/expressions/common/mocks.ts | 1 + .../common/service/expressions_services.ts | 45 +++++++---- src/plugins/expressions/public/loader.ts | 22 +++--- src/plugins/expressions/public/types/index.ts | 1 + .../public/app/components/main.tsx | 2 +- 17 files changed, 112 insertions(+), 122 deletions(-) diff --git a/src/plugins/expressions/common/execution/execution.abortion.test.ts b/src/plugins/expressions/common/execution/execution.abortion.test.ts index ecbf94eceae6429..9fa7d92875b9a8d 100644 --- a/src/plugins/expressions/common/execution/execution.abortion.test.ts +++ b/src/plugins/expressions/common/execution/execution.abortion.test.ts @@ -36,8 +36,7 @@ const createExecution = ( const execution = new Execution({ executor, ast: parseExpression(expression), - context, - debug, + params: { ...context, debug }, }); return execution; }; diff --git a/src/plugins/expressions/common/execution/execution.test.ts b/src/plugins/expressions/common/execution/execution.test.ts index ff331d7c5ddaffc..da3f44ea08f5ba4 100644 --- a/src/plugins/expressions/common/execution/execution.test.ts +++ b/src/plugins/expressions/common/execution/execution.test.ts @@ -38,8 +38,10 @@ const createExecution = ( const execution = new Execution({ executor, ast: parseExpression(expression), - context, - debug, + params: { + ...context, + debug, + }, }); return execution; }; @@ -143,6 +145,7 @@ describe('Execution', () => { const execution = new Execution({ executor, expression, + params: {}, }); expect(execution.expression).toBe(expression); }); @@ -153,6 +156,7 @@ describe('Execution', () => { const execution = new Execution({ ast: parseExpression(expression), executor, + params: {}, }); expect(execution.expression).toBe(expression); }); @@ -619,7 +623,7 @@ describe('Execution', () => { const execution = new Execution({ executor, ast: parseExpression('add val=1 | throws | add val=3'), - debug: true, + params: { debug: true }, }); execution.start(0); await execution.result; @@ -637,7 +641,7 @@ describe('Execution', () => { const execution = new Execution({ executor, ast: parseExpression('add val=1 | throws | add val=3'), - debug: true, + params: { debug: true }, }); execution.start(0); await execution.result; @@ -658,7 +662,7 @@ describe('Execution', () => { const execution = new Execution({ executor, ast: parseExpression('add val=1 | throws | add val=3'), - debug: true, + params: { debug: true }, }); execution.start(0); await execution.result; diff --git a/src/plugins/expressions/common/execution/execution.ts b/src/plugins/expressions/common/execution/execution.ts index 69140453f486d77..52a16bb6b714c12 100644 --- a/src/plugins/expressions/common/execution/execution.ts +++ b/src/plugins/expressions/common/execution/execution.ts @@ -19,7 +19,7 @@ import { i18n } from '@kbn/i18n'; import { keys, last, mapValues, reduce, zipObject } from 'lodash'; -import { Executor, ExpressionExecOptions } from '../executor'; +import { Executor } from '../executor'; import { createExecutionContainer, ExecutionContainer } from './container'; import { createError } from '../util'; import { Defer, now } from '../../../kibana_utils/common'; @@ -39,6 +39,7 @@ import { getType, ExpressionValue } from '../expression_types'; import { ArgumentType, ExpressionFunction } from '../expression_functions'; import { getByAlias } from '../util/get_by_alias'; import { ExecutionContract } from './execution_contract'; +import { ExpressionExecutionParams } from '../service'; const createAbortErrorValue = () => createError({ @@ -46,20 +47,11 @@ const createAbortErrorValue = () => name: 'AbortError', }); -export interface ExecutionParams< - ExtraContext extends Record = Record -> { +export interface ExecutionParams { executor: Executor; ast?: ExpressionAstExpression; expression?: string; - context?: ExtraContext; - - /** - * Whether to execute expression in *debug mode*. In *debug mode* inputs and - * outputs as well as all resolved arguments and time it took to execute each - * function are saved and are available for introspection. - */ - debug?: boolean; + params: ExpressionExecutionParams; } const createDefaultInspectorAdapters = (): DefaultInspectorAdapters => ({ @@ -68,11 +60,10 @@ const createDefaultInspectorAdapters = (): DefaultInspectorAdapters => ({ }); export class Execution< - ExtraContext extends Record = Record, Input = unknown, Output = unknown, - InspectorAdapters extends Adapters = ExtraContext['inspectorAdapters'] extends object - ? ExtraContext['inspectorAdapters'] + InspectorAdapters extends Adapters = ExpressionExecutionParams['inspectorAdapters'] extends object + ? ExpressionExecutionParams['inspectorAdapters'] : DefaultInspectorAdapters > { /** @@ -92,7 +83,7 @@ export class Execution< * Execution context - object that allows to do side-effects. Context is passed * to every function. */ - public readonly context: ExecutionContext & ExtraContext; + public readonly context: ExecutionContext; /** * AbortController to cancel this Execution. @@ -126,11 +117,10 @@ export class Execution< * can return to other plugins for their consumption. */ public readonly contract: ExecutionContract< - ExtraContext, Input, Output, InspectorAdapters - > = new ExecutionContract(this); + > = new ExecutionContract(this); public readonly expression: string; @@ -142,17 +132,17 @@ export class Execution< return this.context.inspectorAdapters; } - constructor(public readonly params: ExecutionParams) { - const { executor } = params; + constructor(public readonly execution: ExecutionParams) { + const { executor } = execution; - if (!params.ast && !params.expression) { + if (!execution.ast && !execution.expression) { throw new TypeError('Execution params should contain at least .ast or .expression key.'); - } else if (params.ast && params.expression) { + } else if (execution.ast && execution.expression) { throw new TypeError('Execution params cannot contain both .ast and .expression key.'); } - this.expression = params.expression || formatExpression(params.ast!); - const ast = params.ast || parseExpression(this.expression); + this.expression = execution.expression || formatExpression(execution.ast!); + const ast = execution.ast || parseExpression(this.expression); this.state = createExecutionContainer({ ...executor.state.get(), @@ -161,14 +151,15 @@ export class Execution< }); this.context = { - getInitialInput: () => this.input, - variables: {}, + getInitialInput: () => this.execution.params.searchContext || {}, + getSessionId: () => execution.params.sessionId, + variables: execution.params.variables || {}, types: executor.getTypes(), abortSignal: this.abortController.signal, - ...(params.context || ({} as ExtraContext)), - inspectorAdapters: (params.context && params.context.inspectorAdapters - ? params.context.inspectorAdapters - : createDefaultInspectorAdapters()) as InspectorAdapters, + inspectorAdapters: + execution.params.inspectorAdapters || + ((createDefaultInspectorAdapters() as any) as InspectorAdapters), + ...(execution.params as any).extraContext, }; } @@ -249,10 +240,10 @@ export class Execution< // actually have a `then` function which would be treated as a `Promise`. const { resolvedArgs } = await this.race(this.resolveArgs(fn, input, fnArgs)); args = resolvedArgs; - timeStart = this.params.debug ? now() : 0; + timeStart = this.execution.params.debug ? now() : 0; const output = await this.race(this.invokeFunction(fn, input, resolvedArgs)); - if (this.params.debug) { + if (this.execution.params.debug) { const timeEnd: number = now(); (link as ExpressionAstFunction).debug = { success: true, @@ -267,11 +258,11 @@ export class Execution< if (getType(output) === 'error') return output; input = output; } catch (rawError) { - const timeEnd: number = this.params.debug ? now() : 0; + const timeEnd: number = this.execution.params.debug ? now() : 0; const error = createError(rawError) as ExpressionValueError; error.error.message = `[${fnName}] > ${error.error.message}`; - if (this.params.debug) { + if (this.execution.params.debug) { (link as ExpressionAstFunction).debug = { success: false, fn: fn.name, @@ -404,9 +395,7 @@ export class Execution< const resolveArgFns = mapValues(argAstsWithDefaults, (asts, argName) => { return asts.map((item: ExpressionAstExpression) => { return async (subInput = input) => { - const output = await this.interpret(item, subInput, { - debug: this.params.debug, - }); + const output = await this.interpret(item, subInput); if (isExpressionValueError(output)) throw output.error; const casted = this.cast(output, argDefs[argName as any].types); return casted; @@ -438,17 +427,12 @@ export class Execution< return { resolvedArgs }; } - public async interpret( - ast: ExpressionAstNode, - input: T, - options?: ExpressionExecOptions - ): Promise { + public async interpret(ast: ExpressionAstNode, input: T): Promise { switch (getType(ast)) { case 'expression': - const execution = this.params.executor.createExecution( + const execution = this.execution.executor.createExecution( ast as ExpressionAstExpression, - this.context, - options + this.execution.params ); execution.start(input); return await execution.result; diff --git a/src/plugins/expressions/common/execution/execution_contract.test.ts b/src/plugins/expressions/common/execution/execution_contract.test.ts index c33f8a1a0f36e0d..856b22470d782b1 100644 --- a/src/plugins/expressions/common/execution/execution_contract.test.ts +++ b/src/plugins/expressions/common/execution/execution_contract.test.ts @@ -30,7 +30,7 @@ const createExecution = ( const execution = new Execution({ executor, ast: parseExpression(expression), - context, + params: { ...context }, }); return execution; }; diff --git a/src/plugins/expressions/common/execution/execution_contract.ts b/src/plugins/expressions/common/execution/execution_contract.ts index 79bb4c58ab48d56..f05f1ded82799d6 100644 --- a/src/plugins/expressions/common/execution/execution_contract.ts +++ b/src/plugins/expressions/common/execution/execution_contract.ts @@ -25,21 +25,14 @@ import { ExpressionAstExpression } from '../ast'; * `ExecutionContract` is a wrapper around `Execution` class. It provides the * same functionality but does not expose Expressions plugin internals. */ -export class ExecutionContract< - ExtraContext extends Record = Record, - Input = unknown, - Output = unknown, - InspectorAdapters = unknown -> { +export class ExecutionContract { public get isPending(): boolean { const state = this.execution.state.get().state; const finished = state === 'error' || state === 'result'; return !finished; } - constructor( - protected readonly execution: Execution - ) {} + constructor(protected readonly execution: Execution) {} /** * Cancel the execution of the expression. This will set abort signal diff --git a/src/plugins/expressions/common/execution/types.ts b/src/plugins/expressions/common/execution/types.ts index 7c26e586fb790a5..a5b10c0c170b10d 100644 --- a/src/plugins/expressions/common/execution/types.ts +++ b/src/plugins/expressions/common/execution/types.ts @@ -57,6 +57,8 @@ export interface ExecutionContext string | undefined; + /** * Allows to fetch saved objects from ElasticSearch. In browser `getSavedObject` * function is provided automatically by the Expressions plugin. On the server diff --git a/src/plugins/expressions/common/executor/executor.ts b/src/plugins/expressions/common/executor/executor.ts index fd7f5808f034007..85b5589b593af03 100644 --- a/src/plugins/expressions/common/executor/executor.ts +++ b/src/plugins/expressions/common/executor/executor.ts @@ -33,6 +33,7 @@ import { functionSpecs } from '../expression_functions/specs'; import { getByAlias } from '../util'; import { SavedObjectReference } from '../../../../core/types'; import { PersistableState } from '../../../kibana_utils/common'; +import { ExpressionExecutionParams } from '../service'; export interface ExpressionExecOptions { /** @@ -166,43 +167,34 @@ export class Executor = Record = Record - >( + public async run( ast: string | ExpressionAstExpression, input: Input, - context?: ExtraContext, - options?: ExpressionExecOptions + params: ExpressionExecutionParams = {} ) { - const execution = this.createExecution(ast, context, options); + const execution = this.createExecution(ast, params); execution.start(input); return (await execution.result) as Output; } - public createExecution< - ExtraContext extends Record = Record, - Input = unknown, - Output = unknown - >( + public createExecution( ast: string | ExpressionAstExpression, - context: ExtraContext = {} as ExtraContext, - { debug }: ExpressionExecOptions = {} as ExpressionExecOptions - ): Execution { - const params: ExecutionParams = { + params: ExpressionExecutionParams = {} + ): Execution { + const executionParams: ExecutionParams = { executor: this, - context: { - ...this.context, - ...context, - } as Context & ExtraContext, - debug, + params: { + ...params, + // for canvas we are passing this in, + // canvas should be refactored to not pass any extra context in + extraContext: this.context, + } as any, }; - if (typeof ast === 'string') params.expression = ast; - else params.ast = ast; + if (typeof ast === 'string') executionParams.expression = ast; + else executionParams.ast = ast; - const execution = new Execution(params); + const execution = new Execution(executionParams); return execution; } diff --git a/src/plugins/expressions/common/expression_functions/specs/kibana.ts b/src/plugins/expressions/common/expression_functions/specs/kibana.ts index 2144a8aba2d19d9..ecc16566b884535 100644 --- a/src/plugins/expressions/common/expression_functions/specs/kibana.ts +++ b/src/plugins/expressions/common/expression_functions/specs/kibana.ts @@ -44,15 +44,15 @@ export const kibana: ExpressionFunctionKibana = { args: {}, - fn(input, _, { search = {} }) { + fn(input, _, { getInitialInput }) { const output: ExpressionValueSearchContext = { // TODO: This spread is left here for legacy reasons, possibly Lens uses it. // TODO: But it shouldn't be need. ...input, type: 'kibana_context', - query: [...toArray(search.query), ...toArray((input || {}).query)], - filters: [...(search.filters || []), ...((input || {}).filters || [])], - timeRange: search.timeRange || (input ? input.timeRange : undefined), + query: [...toArray((getInitialInput() as any).query), ...toArray((input || {}).query)], + filters: [...((getInitialInput() as any).filters || []), ...((input || {}).filters || [])], + timeRange: (getInitialInput() as any).timeRange || (input ? input.timeRange : undefined), }; return output; diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/kibana.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/kibana.test.ts index e5bd53f63c91d5b..4a496ddccabbe2b 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/kibana.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/kibana.test.ts @@ -46,8 +46,8 @@ describe('interpreter/functions#kibana', () => { timeRange: { from: '2', to: '3' }, }; context = { - search, - getInitialInput: () => input, + getInitialInput: () => search, + getSessionId: () => undefined, types: {}, variables: {}, abortSignal: {} as any, diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/theme.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/theme.test.ts index 263409f0caca289..d0c4b14e59f7d2e 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/theme.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/theme.test.ts @@ -38,6 +38,7 @@ describe('expression_functions', () => { context = { getInitialInput: () => {}, + getSessionId: () => undefined, types: {}, variables: { theme: themeProps }, abortSignal: {} as any, diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/var.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/var.test.ts index ccf49ec918d3dc4..652d92c8f0d1723 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/var.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/var.test.ts @@ -32,6 +32,7 @@ describe('expression_functions', () => { input = { timeRange: { from: '0', to: '1' } }; context = { getInitialInput: () => input, + getSessionId: () => undefined, types: {}, variables: { test: 1 }, abortSignal: {} as any, diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/var_set.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/var_set.test.ts index b1ae44e6f899eb2..302122fc407a0b0 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/var_set.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/var_set.test.ts @@ -33,6 +33,7 @@ describe('expression_functions', () => { input = { timeRange: { from: '0', to: '1' } }; context = { getInitialInput: () => input, + getSessionId: () => undefined, types: {}, variables: { test: 1 }, abortSignal: {} as any, diff --git a/src/plugins/expressions/common/mocks.ts b/src/plugins/expressions/common/mocks.ts index 502d88ac955ae67..f5282f02d11471e 100644 --- a/src/plugins/expressions/common/mocks.ts +++ b/src/plugins/expressions/common/mocks.ts @@ -24,6 +24,7 @@ export const createMockExecutionContext = ): ExecutionContext & ExtraContext => { const executionContext: ExecutionContext = { getInitialInput: jest.fn(), + getSessionId: jest.fn(), variables: {}, types: {}, abortSignal: { diff --git a/src/plugins/expressions/common/service/expressions_services.ts b/src/plugins/expressions/common/service/expressions_services.ts index 3d0fb968e8a3a2e..a2846a2ac4c68b8 100644 --- a/src/plugins/expressions/common/service/expressions_services.ts +++ b/src/plugins/expressions/common/service/expressions_services.ts @@ -17,7 +17,7 @@ * under the License. */ -import { Executor, ExpressionExecOptions } from '../executor'; +import { Executor } from '../executor'; import { AnyExpressionRenderDefinition, ExpressionRendererRegistry } from '../expression_renderers'; import { ExpressionAstExpression } from '../ast'; import { ExecutionContract } from '../execution/execution_contract'; @@ -25,6 +25,8 @@ import { AnyExpressionTypeDefinition } from '../expression_types'; import { AnyExpressionFunctionDefinition } from '../expression_functions'; import { SavedObjectReference } from '../../../../core/types'; import { PersistableState } from '../../../kibana_utils/common'; +import { Adapters } from '../../../inspector/common/adapters'; +import { ExecutionContextSearch } from '../execution'; /** * The public contract that `ExpressionsService` provides to other plugins @@ -45,6 +47,23 @@ export type ExpressionsServiceSetup = Pick< | 'fork' >; +export interface ExpressionExecutionParams { + searchContext?: ExecutionContextSearch; + + variables?: Record; + + /** + * Whether to execute expression in *debug mode*. In *debug mode* inputs and + * outputs as well as all resolved arguments and time it took to execute each + * function are saved and are available for introspection. + */ + debug?: boolean; + + sessionId?: string; + + inspectorAdapters?: Adapters; +} + /** * The public contract that `ExpressionsService` provides to other plugins * in Kibana Platform in *start* life-cycle. @@ -98,11 +117,10 @@ export interface ExpressionsServiceStart { * expressions.run('...', null, { elasticsearchClient }); * ``` */ - run: = Record>( + run: ( ast: string | ExpressionAstExpression, input: Input, - context?: ExtraContext, - options?: ExpressionExecOptions + params?: ExpressionExecutionParams ) => Promise; /** @@ -110,17 +128,12 @@ export interface ExpressionsServiceStart { * instance that tracks the progress of the execution and can be used to * interact with the execution. */ - execute: < - Input = unknown, - Output = unknown, - ExtraContext extends Record = Record - >( + execute: ( ast: string | ExpressionAstExpression, // This any is for legacy reasons. input: Input, - context?: ExtraContext, - options?: ExpressionExecOptions - ) => ExecutionContract; + params?: ExpressionExecutionParams + ) => ExecutionContract; /** * Create a new instance of `ExpressionsService`. The new instance inherits @@ -214,8 +227,8 @@ export class ExpressionsService implements PersistableState AnyExpressionRenderDefinition) ): void => this.renderers.register(definition); - public readonly run: ExpressionsServiceStart['run'] = (ast, input, context, options) => - this.executor.run(ast, input, context, options); + public readonly run: ExpressionsServiceStart['run'] = (ast, input, params) => + this.executor.run(ast, input, params); public readonly getFunction: ExpressionsServiceStart['getFunction'] = (name) => this.executor.getFunction(name); @@ -246,8 +259,8 @@ export class ExpressionsService implements PersistableState => this.executor.getTypes(); - public readonly execute: ExpressionsServiceStart['execute'] = ((ast, input, context, options) => { - const execution = this.executor.createExecution(ast, context, options); + public readonly execute: ExpressionsServiceStart['execute'] = ((ast, input, params) => { + const execution = this.executor.createExecution(ast, params); execution.start(input); return execution.contract; }) as ExpressionsServiceStart['execute']; diff --git a/src/plugins/expressions/public/loader.ts b/src/plugins/expressions/public/loader.ts index aef4b73f86e34ab..a2c97f11de1d13c 100644 --- a/src/plugins/expressions/public/loader.ts +++ b/src/plugins/expressions/public/loader.ts @@ -145,18 +145,13 @@ export class ExpressionLoader { this.execution.cancel(); } this.setParams(params); - this.execution = getExpressionsService().execute( - expression, - params.context, - { - search: params.searchContext, - variables: params.variables || {}, - inspectorAdapters: params.inspectorAdapters, - }, - { - debug: params.debug, - } - ); + this.execution = getExpressionsService().execute(expression, params.context, { + searchContext: params.searchContext, + variables: params.variables || {}, + inspectorAdapters: params.inspectorAdapters, + sessionId: params.sessionId, + debug: params.debug, + }); const prevDataHandler = this.execution; const data = await prevDataHandler.getData(); @@ -188,6 +183,9 @@ export class ExpressionLoader { if (params.variables && this.params) { this.params.variables = params.variables; } + if (params.sessionId && this.params) { + this.params.sessionId = params.sessionId; + } this.params.debug = Boolean(params.debug); this.params.inspectorAdapters = (params.inspectorAdapters || diff --git a/src/plugins/expressions/public/types/index.ts b/src/plugins/expressions/public/types/index.ts index 054c5ac3dc467c8..74cf273ab20efd8 100644 --- a/src/plugins/expressions/public/types/index.ts +++ b/src/plugins/expressions/public/types/index.ts @@ -53,6 +53,7 @@ export interface IExpressionLoaderParams { uiState?: unknown; inspectorAdapters?: Adapters; onRenderError?: RenderErrorHandlerFnType; + sessionId?: string; } export interface ExpressionRenderError extends Error { diff --git a/test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/app/components/main.tsx b/test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/app/components/main.tsx index b4f9634b23d2996..8c62fa246dd5916 100644 --- a/test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/app/components/main.tsx +++ b/test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/app/components/main.tsx @@ -63,7 +63,7 @@ class Main extends React.Component<{}, State> { return getExpressions() .execute(expression, context || { type: 'null' }, { inspectorAdapters: adapters, - search: initialContext as any, + searchContext: initialContext as any, }) .getData(); };