From c06d6047855c8a92c10e41b3c944e8335475dc6f Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Thu, 23 Sep 2021 08:44:16 +0200 Subject: [PATCH] [Expressions] Fix setup and start contracts (#110841) * Refactor executor forking to implement state inheritance * Fix setup and start contracts typings * Add support of named forks * Add expressions service life-cycle assertions --- .../expressions/common/execution/execution.ts | 2 +- .../expressions/common/executor/executor.ts | 117 +++---- .../service/expressions_services.test.ts | 52 +++- .../common/service/expressions_services.ts | 288 +++++++++++------- src/plugins/expressions/public/loader.test.ts | 2 + src/plugins/expressions/public/mocks.tsx | 10 +- src/plugins/expressions/public/plugin.test.ts | 10 - src/plugins/expressions/server/mocks.ts | 39 +-- src/plugins/expressions/server/plugin.test.ts | 10 - x-pack/plugins/canvas/public/application.tsx | 5 +- x-pack/plugins/canvas/public/plugin.tsx | 3 +- .../canvas/public/services/expressions.ts | 4 +- .../public/services/kibana/expressions.ts | 2 +- .../saved_objects/workpad_references.ts | 7 +- 14 files changed, 320 insertions(+), 231 deletions(-) diff --git a/src/plugins/expressions/common/execution/execution.ts b/src/plugins/expressions/common/execution/execution.ts index 0c4185c82dc3c..0bb12951202a5 100644 --- a/src/plugins/expressions/common/execution/execution.ts +++ b/src/plugins/expressions/common/execution/execution.ts @@ -180,7 +180,7 @@ export class Execution< const ast = execution.ast || parseExpression(this.expression); this.state = createExecutionContainer({ - ...executor.state.get(), + ...executor.state, state: 'not-started', ast, }); diff --git a/src/plugins/expressions/common/executor/executor.ts b/src/plugins/expressions/common/executor/executor.ts index 55d3a7b897864..ce411ea94eafe 100644 --- a/src/plugins/expressions/common/executor/executor.ts +++ b/src/plugins/expressions/common/executor/executor.ts @@ -49,7 +49,7 @@ export class TypesRegistry implements IRegistry { } public get(id: string): ExpressionType | null { - return this.executor.state.selectors.getType(id); + return this.executor.getType(id) ?? null; } public toJS(): Record { @@ -71,7 +71,7 @@ export class FunctionsRegistry implements IRegistry { } public get(id: string): ExpressionFunction | null { - return this.executor.state.selectors.getFunction(id); + return this.executor.getFunction(id) ?? null; } public toJS(): Record { @@ -95,22 +95,44 @@ export class Executor = Record; + public readonly container: ExecutorContainer; /** * @deprecated */ - public readonly functions: FunctionsRegistry; + public readonly functions = new FunctionsRegistry(this); /** * @deprecated */ - public readonly types: TypesRegistry; + public readonly types = new TypesRegistry(this); + + protected parent?: Executor; constructor(state?: ExecutorState) { - this.state = createExecutorContainer(state); - this.functions = new FunctionsRegistry(this); - this.types = new TypesRegistry(this); + this.container = createExecutorContainer(state); + } + + public get state(): ExecutorState { + const parent = this.parent?.state; + const state = this.container.get(); + + return { + ...(parent ?? {}), + ...state, + types: { + ...(parent?.types ?? {}), + ...state.types, + }, + functions: { + ...(parent?.functions ?? {}), + ...state.functions, + }, + context: { + ...(parent?.context ?? {}), + ...state.context, + }, + }; } public registerFunction( @@ -119,15 +141,18 @@ export class Executor = Record { - return { ...this.state.get().functions }; + return { + ...(this.parent?.getFunctions() ?? {}), + ...this.container.get().functions, + }; } public registerType( @@ -136,23 +161,30 @@ export class Executor = Record { - return { ...this.state.get().types }; + return { + ...(this.parent?.getTypes() ?? {}), + ...this.container.get().types, + }; } public extendContext(extraContext: Record) { - this.state.transitions.extendContext(extraContext); + this.container.transitions.extendContext(extraContext); } public get context(): Record { - return this.state.selectors.getContext(); + return { + ...(this.parent?.context ?? {}), + ...this.container.selectors.getContext(), + }; } /** @@ -199,18 +231,15 @@ export class Executor = Record { - return asts.map((arg) => { - if (arg && typeof arg === 'object') { - return this.walkAst(arg, action); - } - return arg; - }); - }); + link.arguments = mapValues(fnArgs, (asts) => + asts.map((arg) => + arg != null && typeof arg === 'object' ? this.walkAst(arg, action) : arg + ) + ); action(fn, link); } @@ -275,39 +304,19 @@ export class Executor = Record { - if (!fn.migrations[version]) return link; - const updatedAst = fn.migrations[version](link) as ExpressionAstFunction; - link.arguments = updatedAst.arguments; - link.type = updatedAst.type; + if (!fn.migrations[version]) { + return; + } + + ({ arguments: link.arguments, type: link.type } = fn.migrations[version]( + link + ) as ExpressionAstFunction); }); } public fork(): Executor { - const initialState = this.state.get(); - const fork = new Executor(initialState); - - /** - * Synchronize registry state - make any new types, functions and context - * also available in the forked instance of `Executor`. - */ - this.state.state$.subscribe(({ types, functions, context }) => { - const state = fork.state.get(); - fork.state.set({ - ...state, - types: { - ...types, - ...state.types, - }, - functions: { - ...functions, - ...state.functions, - }, - context: { - ...context, - ...state.context, - }, - }); - }); + const fork = new Executor(); + fork.parent = this; return fork; } diff --git a/src/plugins/expressions/common/service/expressions_services.test.ts b/src/plugins/expressions/common/service/expressions_services.test.ts index db73d300e1273..620917dc64d4d 100644 --- a/src/plugins/expressions/common/service/expressions_services.test.ts +++ b/src/plugins/expressions/common/service/expressions_services.test.ts @@ -17,11 +17,16 @@ describe('ExpressionsService', () => { const expressions = new ExpressionsService(); expect(expressions.setup()).toMatchObject({ + getFunction: expect.any(Function), getFunctions: expect.any(Function), + getRenderer: expect.any(Function), + getRenderers: expect.any(Function), + getType: expect.any(Function), + getTypes: expect.any(Function), registerFunction: expect.any(Function), registerType: expect.any(Function), registerRenderer: expect.any(Function), - run: expect.any(Function), + fork: expect.any(Function), }); }); @@ -30,7 +35,16 @@ describe('ExpressionsService', () => { expressions.setup(); expect(expressions.start()).toMatchObject({ + getFunction: expect.any(Function), getFunctions: expect.any(Function), + getRenderer: expect.any(Function), + getRenderers: expect.any(Function), + getType: expect.any(Function), + getTypes: expect.any(Function), + registerFunction: expect.any(Function), + registerType: expect.any(Function), + registerRenderer: expect.any(Function), + execute: expect.any(Function), run: expect.any(Function), }); }); @@ -54,21 +68,21 @@ describe('ExpressionsService', () => { const service = new ExpressionsService(); const fork = service.fork(); - expect(fork.executor.state.get().types).toEqual(service.executor.state.get().types); + expect(fork.getTypes()).toEqual(service.getTypes()); }); test('fork keeps all functions of the origin service', () => { const service = new ExpressionsService(); const fork = service.fork(); - expect(fork.executor.state.get().functions).toEqual(service.executor.state.get().functions); + expect(fork.getFunctions()).toEqual(service.getFunctions()); }); test('fork keeps context of the origin service', () => { const service = new ExpressionsService(); const fork = service.fork(); - expect(fork.executor.state.get().context).toEqual(service.executor.state.get().context); + expect(fork.executor.state.context).toEqual(service.executor.state.context); }); test('newly registered functions in origin are also available in fork', () => { @@ -82,7 +96,7 @@ describe('ExpressionsService', () => { fn: () => {}, }); - expect(fork.executor.state.get().functions).toEqual(service.executor.state.get().functions); + expect(fork.getFunctions()).toEqual(service.getFunctions()); }); test('newly registered functions in fork are NOT available in origin', () => { @@ -96,14 +110,15 @@ describe('ExpressionsService', () => { fn: () => {}, }); - expect(Object.values(fork.executor.state.get().functions)).toHaveLength( - Object.values(service.executor.state.get().functions).length + 1 + expect(Object.values(fork.getFunctions())).toHaveLength( + Object.values(service.getFunctions()).length + 1 ); }); test('fork can execute an expression with newly registered function', async () => { const service = new ExpressionsService(); const fork = service.fork(); + fork.start(); service.registerFunction({ name: '__test__', @@ -118,5 +133,28 @@ describe('ExpressionsService', () => { expect(result).toBe('123'); }); + + test('throw on fork if the service is already started', async () => { + const service = new ExpressionsService(); + service.start(); + + expect(() => service.fork()).toThrow(); + }); + }); + + describe('.execute()', () => { + test('throw if the service is not started', () => { + const expressions = new ExpressionsService(); + + expect(() => expressions.execute('foo', null)).toThrow(); + }); + }); + + describe('.run()', () => { + test('throw if the service is not started', () => { + const expressions = new ExpressionsService(); + + expect(() => expressions.run('foo', null)).toThrow(); + }); }); }); diff --git a/src/plugins/expressions/common/service/expressions_services.ts b/src/plugins/expressions/common/service/expressions_services.ts index 2be4f5207bb82..f21eaa34d7868 100644 --- a/src/plugins/expressions/common/service/expressions_services.ts +++ b/src/plugins/expressions/common/service/expressions_services.ts @@ -41,22 +41,86 @@ import { * The public contract that `ExpressionsService` provides to other plugins * in Kibana Platform in *setup* life-cycle. */ -export type ExpressionsServiceSetup = Pick< - ExpressionsService, - | 'getFunction' - | 'getFunctions' - | 'getRenderer' - | 'getRenderers' - | 'getType' - | 'getTypes' - | 'registerFunction' - | 'registerRenderer' - | 'registerType' - | 'run' - | 'fork' - | 'extract' - | 'inject' ->; +export interface ExpressionsServiceSetup { + /** + * Get a registered `ExpressionFunction` by its name, which was registered + * using the `registerFunction` method. The returned `ExpressionFunction` + * instance is an internal representation of the function in Expressions + * service - do not mutate that object. + * @deprecated Use start contract instead. + */ + getFunction(name: string): ReturnType; + + /** + * Returns POJO map of all registered expression functions, where keys are + * names of the functions and values are `ExpressionFunction` instances. + * @deprecated Use start contract instead. + */ + getFunctions(): ReturnType; + + /** + * Returns POJO map of all registered expression types, where keys are + * names of the types and values are `ExpressionType` instances. + * @deprecated Use start contract instead. + */ + getTypes(): ReturnType; + + /** + * Create a new instance of `ExpressionsService`. The new instance inherits + * all state of the original `ExpressionsService`, including all expression + * types, expression functions and context. Also, all new types and functions + * registered in the original services AFTER the forking event will be + * available in the forked instance. However, all new types and functions + * registered in the forked instances will NOT be available to the original + * service. + * @param name A fork name that can be used to get fork instance later. + */ + fork(name?: string): ExpressionsService; + + /** + * Register an expression function, which will be possible to execute as + * part of the expression pipeline. + * + * Below we register a function which simply sleeps for given number of + * milliseconds to delay the execution and outputs its input as-is. + * + * ```ts + * expressions.registerFunction({ + * name: 'sleep', + * args: { + * time: { + * aliases: ['_'], + * help: 'Time in milliseconds for how long to sleep', + * types: ['number'], + * }, + * }, + * help: '', + * fn: async (input, args, context) => { + * await new Promise(r => setTimeout(r, args.time)); + * return input; + * }, + * } + * ``` + * + * The actual function is defined in the `fn` key. The function can be *async*. + * It receives three arguments: (1) `input` is the output of the previous function + * or the initial input of the expression if the function is first in chain; + * (2) `args` are function arguments as defined in expression string, that can + * be edited by user (e.g in case of Canvas); (3) `context` is a shared object + * passed to all functions that can be used for side-effects. + */ + registerFunction( + functionDefinition: AnyExpressionFunctionDefinition | (() => AnyExpressionFunctionDefinition) + ): void; + + registerType( + typeDefinition: AnyExpressionTypeDefinition | (() => AnyExpressionTypeDefinition) + ): void; + + registerRenderer( + definition: AnyExpressionRenderDefinition | (() => AnyExpressionRenderDefinition) + ): void; +} export interface ExpressionExecutionParams { searchContext?: SerializableRecord; @@ -97,7 +161,13 @@ export interface ExpressionsServiceStart { * instance is an internal representation of the function in Expressions * service - do not mutate that object. */ - getFunction: (name: string) => ReturnType; + getFunction(name: string): ReturnType; + + /** + * Returns POJO map of all registered expression functions, where keys are + * names of the functions and values are `ExpressionFunction` instances. + */ + getFunctions(): ReturnType; /** * Get a registered `ExpressionRenderer` by its name, which was registered @@ -105,7 +175,13 @@ export interface ExpressionsServiceStart { * instance is an internal representation of the renderer in Expressions * service - do not mutate that object. */ - getRenderer: (name: string) => ReturnType; + getRenderer(name: string): ReturnType; + + /** + * Returns POJO map of all registered expression renderers, where keys are + * names of the renderers and values are `ExpressionRenderer` instances. + */ + getRenderers(): ReturnType; /** * Get a registered `ExpressionType` by its name, which was registered @@ -113,7 +189,13 @@ export interface ExpressionsServiceStart { * instance is an internal representation of the type in Expressions * service - do not mutate that object. */ - getType: (name: string) => ReturnType; + getType(name: string): ReturnType; + + /** + * Returns POJO map of all registered expression types, where keys are + * names of the types and values are `ExpressionType` instances. + */ + getTypes(): ReturnType; /** * Executes expression string or a parsed expression AST and immediately @@ -139,34 +221,23 @@ export interface ExpressionsServiceStart { * expressions.run('...', null, { elasticsearchClient }); * ``` */ - run: ( + run( ast: string | ExpressionAstExpression, input: Input, params?: ExpressionExecutionParams - ) => Observable>; + ): Observable>; /** * Starts expression execution and immediately returns `ExecutionContract` * instance that tracks the progress of the execution and can be used to * interact with the execution. */ - execute: ( + execute( ast: string | ExpressionAstExpression, // This any is for legacy reasons. input: Input, params?: ExpressionExecutionParams - ) => ExecutionContract; - - /** - * Create a new instance of `ExpressionsService`. The new instance inherits - * all state of the original `ExpressionsService`, including all expression - * types, expression functions and context. Also, all new types and functions - * registered in the original services AFTER the forking event will be - * available in the forked instance. However, all new types and functions - * registered in the forked instances will NOT be available to the original - * service. - */ - fork: () => ExpressionsService; + ): ExecutionContract; } export interface ExpressionServiceParams { @@ -193,7 +264,19 @@ export interface ExpressionServiceParams { * * so that JSDoc appears in developers IDE when they use those `plugins.expressions.registerFunction(`. */ -export class ExpressionsService implements PersistableStateService { +export class ExpressionsService + implements + PersistableStateService, + ExpressionsServiceSetup, + ExpressionsServiceStart +{ + /** + * @note Workaround since the expressions service is frozen. + */ + private static started = new WeakSet(); + private children = new Map(); + private parent?: ExpressionsService; + public readonly executor: Executor; public readonly renderers: ExpressionRendererRegistry; @@ -205,94 +288,85 @@ export class ExpressionsService implements PersistableStateService { - * await new Promise(r => setTimeout(r, args.time)); - * return input; - * }, - * } - * ``` - * - * The actual function is defined in the `fn` key. The function can be *async*. - * It receives three arguments: (1) `input` is the output of the previous function - * or the initial input of the expression if the function is first in chain; - * (2) `args` are function arguments as defined in expression string, that can - * be edited by user (e.g in case of Canvas); (3) `context` is a shared object - * passed to all functions that can be used for side-effects. - */ - public readonly registerFunction = ( - functionDefinition: AnyExpressionFunctionDefinition | (() => AnyExpressionFunctionDefinition) - ): void => this.executor.registerFunction(functionDefinition); - - public readonly registerType = ( - typeDefinition: AnyExpressionTypeDefinition | (() => AnyExpressionTypeDefinition) - ): void => this.executor.registerType(typeDefinition); + private isStarted(): boolean { + return !!(ExpressionsService.started.has(this) || this.parent?.isStarted()); + } - public readonly registerRenderer = ( - definition: AnyExpressionRenderDefinition | (() => AnyExpressionRenderDefinition) - ): void => this.renderers.register(definition); + private assertSetup() { + if (this.isStarted()) { + throw new Error('The expression service is already started and can no longer be configured.'); + } + } - public readonly run: ExpressionsServiceStart['run'] = (ast, input, params) => - this.executor.run(ast, input, params); + private assertStart() { + if (!this.isStarted()) { + throw new Error('The expressions service has not started yet.'); + } + } public readonly getFunction: ExpressionsServiceStart['getFunction'] = (name) => this.executor.getFunction(name); - /** - * Returns POJO map of all registered expression functions, where keys are - * names of the functions and values are `ExpressionFunction` instances. - */ - public readonly getFunctions = (): ReturnType => + public readonly getFunctions: ExpressionsServiceStart['getFunctions'] = () => this.executor.getFunctions(); - public readonly getRenderer: ExpressionsServiceStart['getRenderer'] = (name) => - this.renderers.get(name); + public readonly getRenderer: ExpressionsServiceStart['getRenderer'] = (name) => { + this.assertStart(); - /** - * Returns POJO map of all registered expression renderers, where keys are - * names of the renderers and values are `ExpressionRenderer` instances. - */ - public readonly getRenderers = (): ReturnType => - this.renderers.toJS(); + return this.renderers.get(name); + }; - public readonly getType: ExpressionsServiceStart['getType'] = (name) => - this.executor.getType(name); + public readonly getRenderers: ExpressionsServiceStart['getRenderers'] = () => { + this.assertStart(); - /** - * Returns POJO map of all registered expression types, where keys are - * names of the types and values are `ExpressionType` instances. - */ - public readonly getTypes = (): ReturnType => this.executor.getTypes(); + return this.renderers.toJS(); + }; + + public readonly getType: ExpressionsServiceStart['getType'] = (name) => { + this.assertStart(); + + return this.executor.getType(name); + }; + + public readonly getTypes: ExpressionsServiceStart['getTypes'] = () => this.executor.getTypes(); + + public readonly registerFunction: ExpressionsServiceSetup['registerFunction'] = ( + functionDefinition + ) => this.executor.registerFunction(functionDefinition); + + public readonly registerType: ExpressionsServiceSetup['registerType'] = (typeDefinition) => + this.executor.registerType(typeDefinition); + + public readonly registerRenderer: ExpressionsServiceSetup['registerRenderer'] = (definition) => + this.renderers.register(definition); + + public readonly fork: ExpressionsServiceSetup['fork'] = (name) => { + this.assertSetup(); + + const executor = this.executor.fork(); + const renderers = this.renderers; + const fork = new (this.constructor as typeof ExpressionsService)({ executor, renderers }); + fork.parent = this; + + if (name) { + this.children.set(name, fork); + } + + return fork; + }; public readonly execute: ExpressionsServiceStart['execute'] = ((ast, input, params) => { + this.assertStart(); const execution = this.executor.createExecution(ast, params); execution.start(input); + return execution.contract; }) as ExpressionsServiceStart['execute']; - public readonly fork = () => { - const executor = this.executor.fork(); - const renderers = this.renderers; - const fork = new (this.constructor as typeof ExpressionsService)({ executor, renderers }); + public readonly run: ExpressionsServiceStart['run'] = (ast, input, params) => { + this.assertStart(); - return fork; + return this.executor.run(ast, input, params); }; /** @@ -371,8 +445,12 @@ export class ExpressionsService implements PersistableStateService { service.registerFunction(func); } + service.start(); + const moduleMock = { __execution: undefined, __getLastExecution: () => moduleMock.__execution, diff --git a/src/plugins/expressions/public/mocks.tsx b/src/plugins/expressions/public/mocks.tsx index 3a5450fc02837..f2f6a6807f339 100644 --- a/src/plugins/expressions/public/mocks.tsx +++ b/src/plugins/expressions/public/mocks.tsx @@ -16,19 +16,13 @@ export type Start = jest.Mocked; const createSetupContract = (): Setup => { const setupContract: Setup = { - extract: jest.fn(), fork: jest.fn(), getFunction: jest.fn(), getFunctions: jest.fn(), - getRenderer: jest.fn(), - getRenderers: jest.fn(), - getType: jest.fn(), getTypes: jest.fn(), - inject: jest.fn(), registerFunction: jest.fn(), registerRenderer: jest.fn(), registerType: jest.fn(), - run: jest.fn(), }; return setupContract; }; @@ -38,10 +32,12 @@ const createStartContract = (): Start => { execute: jest.fn(), ExpressionLoader: jest.fn(), ExpressionRenderHandler: jest.fn(), - fork: jest.fn(), getFunction: jest.fn(), + getFunctions: jest.fn(), getRenderer: jest.fn(), + getRenderers: jest.fn(), getType: jest.fn(), + getTypes: jest.fn(), loader: jest.fn(), ReactExpressionRenderer: jest.fn((props) => <>), render: jest.fn(), diff --git a/src/plugins/expressions/public/plugin.test.ts b/src/plugins/expressions/public/plugin.test.ts index 1963eb1f1b3f7..61ff0d8b54033 100644 --- a/src/plugins/expressions/public/plugin.test.ts +++ b/src/plugins/expressions/public/plugin.test.ts @@ -32,16 +32,6 @@ describe('ExpressionsPublicPlugin', () => { expect(setup.getFunctions().add.name).toBe('add'); }); }); - - describe('.run()', () => { - test('can execute simple expression', async () => { - const { setup } = await expressionsPluginMock.createPlugin(); - const { result } = await setup - .run('var_set name="foo" value="bar" | var name="foo"', null) - .toPromise(); - expect(result).toBe('bar'); - }); - }); }); describe('start contract', () => { diff --git a/src/plugins/expressions/server/mocks.ts b/src/plugins/expressions/server/mocks.ts index f4379145f6a6c..bf36ab3c5daa9 100644 --- a/src/plugins/expressions/server/mocks.ts +++ b/src/plugins/expressions/server/mocks.ts @@ -13,37 +13,24 @@ import { coreMock } from '../../../core/server/mocks'; export type Setup = jest.Mocked; export type Start = jest.Mocked; -const createSetupContract = (): Setup => { - const setupContract: Setup = { - extract: jest.fn(), - fork: jest.fn(), - getFunction: jest.fn(), - getFunctions: jest.fn(), - getRenderer: jest.fn(), - getRenderers: jest.fn(), - getType: jest.fn(), - getTypes: jest.fn(), - inject: jest.fn(), - registerFunction: jest.fn(), - registerRenderer: jest.fn(), - registerType: jest.fn(), - run: jest.fn(), - }; - return setupContract; -}; - -const createStartContract = (): Start => { - const startContract: Start = { +const createSetupContract = (): Setup => ({ + fork: jest.fn(), + getFunction: jest.fn(), + getFunctions: jest.fn(), + getTypes: jest.fn(), + registerFunction: jest.fn(), + registerRenderer: jest.fn(), + registerType: jest.fn(), +}); + +const createStartContract = (): Start => + ({ execute: jest.fn(), - fork: jest.fn(), getFunction: jest.fn(), getRenderer: jest.fn(), getType: jest.fn(), run: jest.fn(), - }; - - return startContract; -}; + } as unknown as Start); const createPlugin = async () => { const pluginInitializerContext = coreMock.createPluginInitializerContext(); diff --git a/src/plugins/expressions/server/plugin.test.ts b/src/plugins/expressions/server/plugin.test.ts index c41cda36e7623..52ecf1ff9979e 100644 --- a/src/plugins/expressions/server/plugin.test.ts +++ b/src/plugins/expressions/server/plugin.test.ts @@ -24,15 +24,5 @@ describe('ExpressionsServerPlugin', () => { expect(setup.getFunctions().add.name).toBe('add'); }); }); - - describe('.run()', () => { - test('can execute simple expression', async () => { - const { setup } = await expressionsPluginMock.createPlugin(); - const { result } = await setup - .run('var_set name="foo" value="bar" | var name="foo"', null) - .toPromise(); - expect(result).toBe('bar'); - }); - }); }); }); diff --git a/x-pack/plugins/canvas/public/application.tsx b/x-pack/plugins/canvas/public/application.tsx index 30b2d78a6b1fe..f2fe944bfd45d 100644 --- a/x-pack/plugins/canvas/public/application.tsx +++ b/x-pack/plugins/canvas/public/application.tsx @@ -98,11 +98,10 @@ export const initializeCanvas = async ( setupPlugins: CanvasSetupDeps, startPlugins: CanvasStartDeps, registries: SetupRegistries, - appUpdater: BehaviorSubject, - pluginServices: PluginServices + appUpdater: BehaviorSubject ) => { await startLegacyServices(coreSetup, coreStart, setupPlugins, startPlugins, appUpdater); - const { expressions } = pluginServices.getServices(); + const { expressions } = setupPlugins; // Adding these functions here instead of in plugin.ts. // Some of these functions have deep dependencies into Canvas, which was bulking up the size diff --git a/x-pack/plugins/canvas/public/plugin.tsx b/x-pack/plugins/canvas/public/plugin.tsx index 555cedb6b16a1..bd5d884f1485c 100644 --- a/x-pack/plugins/canvas/public/plugin.tsx +++ b/x-pack/plugins/canvas/public/plugin.tsx @@ -132,8 +132,7 @@ export class CanvasPlugin setupPlugins, startPlugins, registries, - this.appUpdater, - pluginServices + this.appUpdater ); const unmount = renderApp({ coreStart, startPlugins, params, canvasStore, pluginServices }); diff --git a/x-pack/plugins/canvas/public/services/expressions.ts b/x-pack/plugins/canvas/public/services/expressions.ts index a1af0fba50a5c..01bb0adb17711 100644 --- a/x-pack/plugins/canvas/public/services/expressions.ts +++ b/x-pack/plugins/canvas/public/services/expressions.ts @@ -5,6 +5,6 @@ * 2.0. */ -import { ExpressionsService } from '../../../../../src/plugins/expressions/public'; +import { ExpressionsServiceStart } from '../../../../../src/plugins/expressions/public'; -export type CanvasExpressionsService = ExpressionsService; +export type CanvasExpressionsService = ExpressionsServiceStart; diff --git a/x-pack/plugins/canvas/public/services/kibana/expressions.ts b/x-pack/plugins/canvas/public/services/kibana/expressions.ts index 4e3bb52a5d449..780de5309d97e 100644 --- a/x-pack/plugins/canvas/public/services/kibana/expressions.ts +++ b/x-pack/plugins/canvas/public/services/kibana/expressions.ts @@ -16,4 +16,4 @@ export type CanvasExpressionsServiceFactory = KibanaPluginServiceFactory< >; export const expressionsServiceFactory: CanvasExpressionsServiceFactory = ({ startPlugins }) => - startPlugins.expressions.fork(); + startPlugins.expressions; diff --git a/x-pack/plugins/canvas/server/saved_objects/workpad_references.ts b/x-pack/plugins/canvas/server/saved_objects/workpad_references.ts index b0d20add2f79a..e9eefa1bdb3f4 100644 --- a/x-pack/plugins/canvas/server/saved_objects/workpad_references.ts +++ b/x-pack/plugins/canvas/server/saved_objects/workpad_references.ts @@ -6,14 +6,15 @@ */ import { fromExpression, toExpression } from '@kbn/interpreter/common'; +import { PersistableStateService } from '../../../../../src/plugins/kibana_utils/common'; import { SavedObjectReference } from '../../../../../src/core/server'; import { WorkpadAttributes } from '../routes/workpad/workpad_attributes'; -import { ExpressionsServerSetup } from '../../../../../src/plugins/expressions/server'; +import type { ExpressionAstExpression } from '../../../../../src/plugins/expressions'; export const extractReferences = ( workpad: WorkpadAttributes, - expressions: ExpressionsServerSetup + expressions: PersistableStateService ): { workpad: WorkpadAttributes; references: SavedObjectReference[] } => { // We need to find every element in the workpad and extract references const references: SavedObjectReference[] = []; @@ -42,7 +43,7 @@ export const extractReferences = ( export const injectReferences = ( workpad: WorkpadAttributes, references: SavedObjectReference[], - expressions: ExpressionsServerSetup + expressions: PersistableStateService ) => { const pages = workpad.pages.map((page) => { const elements = page.elements.map((element) => {