[RFC] Allow 'leasing' of Expression constructs

src/plugins/expressions/common/executor/container.ts

@@ -27,13 +27,89 @@ export const defaultState: ExecutorState<any> = {
 
 export interface ExecutorPureTransitions {
   addFunction: (state: ExecutorState) => (fn: ExpressionFunction) => ExecutorState;
+  addFunctions: (state: ExecutorState) => (fns: ExpressionFunction[]) => ExecutorState;
+  removeFunction: (state: ExecutorState) => (fnName: ExpressionFunction['name']) => ExecutorState;
+  removeFunctions: (
+    state: ExecutorState
+  ) => (fnNames: Array<ExpressionFunction['name']>) => ExecutorState;
   addType: (state: ExecutorState) => (type: ExpressionType) => ExecutorState;
+  addTypes: (state: ExecutorState) => (types: ExpressionType[]) => ExecutorState;
+  removeType: (state: ExecutorState) => (typeName: ExpressionType['name']) => ExecutorState;
+  removeTypes: (state: ExecutorState) => (typeName: Array<ExpressionType['name']>) => ExecutorState;
   extendContext: (state: ExecutorState) => (extraContext: Record<string, unknown>) => ExecutorState;
 }
 
+const addFunctions: ExecutorPureTransitions['addFunctions'] = (state) => (fns) => {
+  const functions = {} as Record<string, ExpressionFunction>;
+
+  fns.forEach((fn) => {
+    functions[fn.name] = fn;
+  });
+
+  return {
+    ...state,
+    functions: {
+      ...state.functions,
+      ...functions,
+    },
+  };
+};
+
+const removeFunctions: ExecutorPureTransitions['removeFunctions'] = (state) => (names) => {
+  const functions = {} as Record<string, ExpressionFunction>;
+
+  for (const name in state.functions) {
+    if (!names.includes(name)) {
+      functions[name] = state.functions[name];
+    }
+  }
+
+  return {
+    ...state,
+    functions,
+  };
+};
+
+const addTypes: ExecutorPureTransitions['addTypes'] = (state) => (typesToAdd) => {
+  const types = {} as Record<string, ExpressionType>;
+
+  typesToAdd.forEach((type) => {
+    types[type.name] = type;
+  });
+
+  return {
+    ...state,
+    types: {
+      ...state.types,
+      ...types,
+    },
+  };
+};
+
+const removeTypes: ExecutorPureTransitions['removeTypes'] = (state) => (typesToRemove) => {
+  const types = {} as Record<string, ExpressionType>;
+
+  for (const name in state.types) {
+    if (!typesToRemove.includes(name)) {
+      types[name] = state.types[name];
+    }
+  }
+
+  return {
+    ...state,
+    types,
+  };
+};
+
 export const pureTransitions: ExecutorPureTransitions = {
-  addFunction: (state) => (fn) => ({ ...state, functions: { ...state.functions, [fn.name]: fn } }),
-  addType: (state) => (type) => ({ ...state, types: { ...state.types, [type.name]: type } }),
+  addFunction: (state) => (fn) => addFunctions(state)([fn]),
+  addFunctions,
+  removeFunction: (state) => (fnName) => removeFunctions(state)([fnName]),
+  removeFunctions,
+  addType: (state) => (type) => addTypes(state)([type]),
+  addTypes,
+  removeType: (state) => (typeName) => removeTypes(state)([typeName]),
+  removeTypes,
   extendContext: (state) => (extraContext) => ({
     ...state,
     context: { ...state.context, ...extraContext },

src/plugins/expressions/common/executor/executor.test.ts

@@ -44,6 +44,18 @@ describe('Executor', () => {
         expressionTypes.typeSpecs.map((spec) => spec.name).sort()
       );
     });
+
+    test('can lease all types', () => {
+      const executor = new Executor();
+      const release = executor.leaseTypes(expressionTypes.typeSpecs);
+      let types = executor.getTypes();
+      expect(Object.keys(types).sort()).toEqual(
+        expressionTypes.typeSpecs.map((spec) => spec.name).sort()
+      );
+      release();
+      types = executor.getTypes();
+      expect(Object.keys(types).length).toBe(0);
+    });
   });
 
   describe('function registry', () => {
@@ -80,6 +92,28 @@ describe('Executor', () => {
 
       expect(Object.keys(functions).sort()).toEqual(functionSpecs.map((spec) => spec.name).sort());
     });
+
+    test('can lease functions', () => {
+      const executor = new Executor();
+      const functionSpecs = [
+        expressionFunctions.clog,
+        expressionFunctions.font,
+        expressionFunctions.variableSet,
+        expressionFunctions.variable,
+        expressionFunctions.theme,
+        expressionFunctions.cumulativeSum,
+        expressionFunctions.derivative,
+        expressionFunctions.movingAverage,
+        expressionFunctions.mapColumn,
+        expressionFunctions.math,
+      ];
+      const release = executor.leaseFunctions(functionSpecs);
+      let functions = executor.getFunctions();
+      expect(Object.keys(functions).sort()).toEqual(functionSpecs.map((spec) => spec.name).sort());
+      release();
+      functions = executor.getFunctions();
+      expect(Object.keys(functions).length).toBe(0);
+    });
   });
 
   describe('context', () => {

src/plugins/expressions/common/executor/executor.ts

@@ -77,14 +77,16 @@ export class FunctionsRegistry implements IRegistry<ExpressionFunction> {
   }
 }
 
+type FunctionDefinition = AnyExpressionFunctionDefinition | (() => AnyExpressionFunctionDefinition);
+type TypeDefinition = AnyExpressionTypeDefinition | (() => AnyExpressionTypeDefinition);
+
 export class Executor<Context extends Record<string, unknown> = Record<string, unknown>>
   implements PersistableStateService<ExpressionAstExpression> {
   static createWithDefaults<Ctx extends Record<string, unknown> = Record<string, unknown>>(
     state?: ExecutorState<Ctx>
   ): Executor<Ctx> {
     const executor = new Executor<Ctx>(state);
-    for (const type of typeSpecs) executor.registerType(type);
-
+    executor.registerTypes(typeSpecs);
     return executor;
   }
 
@@ -106,13 +108,27 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
     this.types = new TypesRegistry(this);
   }
 
-  public registerFunction(
-    functionDefinition: AnyExpressionFunctionDefinition | (() => AnyExpressionFunctionDefinition)
-  ) {
-    const fn = new ExpressionFunction(
-      typeof functionDefinition === 'object' ? functionDefinition : functionDefinition()
+  public registerFunction(functionDefinition: FunctionDefinition) {
+    this.registerFunctions([functionDefinition]);
+  }
+
+  public registerFunctions(functionDefinitions: FunctionDefinition[]) {
+    const fns = functionDefinitions.map(
+      (fn) => new ExpressionFunction(typeof fn === 'object' ? fn : fn())
     );
-    this.state.transitions.addFunction(fn);
+
+    this.state.transitions.addFunctions(fns);
+  }
+
+  public leaseFunctions(functionDefinitions: FunctionDefinition[]) {
+    const fns = functionDefinitions.map(
+      (fn) => new ExpressionFunction(typeof fn === 'object' ? fn : fn())
+    );
+
+    this.state.transitions.addFunctions(fns);
+    const names = fns.map((fn) => fn.name);
+
+    return () => this.state.transitions.removeFunctions(names);
   }
 
   public getFunction(name: string): ExpressionFunction | undefined {
@@ -123,13 +139,29 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
     return { ...this.state.get().functions };
   }
 
-  public registerType(
-    typeDefinition: AnyExpressionTypeDefinition | (() => AnyExpressionTypeDefinition)
-  ) {
-    const type = new ExpressionType(
-      typeof typeDefinition === 'object' ? typeDefinition : typeDefinition()
+  public registerType(typeDefinition: TypeDefinition) {
+    this.registerTypes([typeDefinition]);
+  }
+
+  public registerTypes(typeDefinitions: TypeDefinition[]) {
+    const types = typeDefinitions.map(
+      (typeDefinition) =>
+        new ExpressionType(typeof typeDefinition === 'object' ? typeDefinition : typeDefinition())
     );
-    this.state.transitions.addType(type);
+
+    this.state.transitions.addTypes(types);
+  }
+
+  public leaseTypes(typeDefinitions: TypeDefinition[]) {
+    const types = typeDefinitions.map(
+      (typeDefinition) =>
+        new ExpressionType(typeof typeDefinition === 'object' ? typeDefinition : typeDefinition())
+    );
+
+    this.state.transitions.addTypes(types);
+    const names = types.map((type) => type.name);
+
+    return () => this.state.transitions.removeTypes(names);
   }
 
   public getType(name: string): ExpressionType | undefined {

src/plugins/expressions/common/expression_renderers/expression_renderer_registry.ts

@@ -22,6 +22,10 @@ export class ExpressionRendererRegistry implements IRegistry<ExpressionRenderer>
     this.renderers.set(renderer.name, renderer);
   }
 
+  remove(name: string) {
+    this.renderers.delete(name);
+  }
+
   public get(id: string): ExpressionRenderer | null {
     return this.renderers.get(id) || null;
   }

src/plugins/expressions/common/service/expressions_services.ts

@@ -14,8 +14,7 @@ import { Executor } from '../executor';
 import { AnyExpressionRenderDefinition, ExpressionRendererRegistry } from '../expression_renderers';
 import { ExpressionAstExpression } from '../ast';
 import { ExecutionContract, ExecutionResult } from '../execution';
-import { AnyExpressionTypeDefinition, ExpressionValueError } from '../expression_types';
-import { AnyExpressionFunctionDefinition } from '../expression_functions';
+import { ExpressionValueError } from '../expression_types';
 import { SavedObjectReference } from '../../../../core/types';
 import { PersistableStateService, SerializableState } from '../../../kibana_utils/common';
 import { Adapters } from '../../../inspector/common/adapters';
@@ -43,13 +42,19 @@ export type ExpressionsServiceSetup = Pick<
   ExpressionsService,
   | 'getFunction'
   | 'getFunctions'
+  | 'leaseFunctions'
   | 'getRenderer'
   | 'getRenderers'
   | 'getType'
   | 'getTypes'
+  | 'leaseTypes'
   | 'registerFunction'
+  | 'registerFunctions'
   | 'registerRenderer'
+  | 'registerRenderers'
+  | 'leaseRenderers'
   | 'registerType'
+  | 'registerTypes'
   | 'run'
   | 'fork'
 >;
@@ -231,17 +236,44 @@ export class ExpressionsService implements PersistableStateService<ExpressionAst
    * 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 registerFunction: Executor['registerFunction'] = (functionDefinition) =>
+    this.executor.registerFunction(functionDefinition);
 
-  public readonly registerType = (
-    typeDefinition: AnyExpressionTypeDefinition | (() => AnyExpressionTypeDefinition)
-  ): void => this.executor.registerType(typeDefinition);
+  public readonly registerFunctions: Executor['registerFunctions'] = (functionDefinitions) =>
+    this.executor.registerFunctions(functionDefinitions);
 
-  public readonly registerRenderer = (
-    definition: AnyExpressionRenderDefinition | (() => AnyExpressionRenderDefinition)
-  ): void => this.renderers.register(definition);
+  public readonly leaseFunctions: Executor['leaseFunctions'] = (fns) =>
+    this.executor.leaseFunctions(fns);
+
+  public readonly registerType: Executor['registerType'] = (typeDefinition) =>
+    this.executor.registerType(typeDefinition);
+
+  public readonly registerTypes: Executor['registerTypes'] = (typeDefinitions) =>
+    this.executor.registerTypes(typeDefinitions);
+
+  public readonly leaseTypes: Executor['leaseTypes'] = (types) => this.executor.leaseTypes(types);
+
+  public readonly registerRenderer: ExpressionRendererRegistry['register'] = (definition): void =>
+    this.renderers.register(definition);
+
+  public readonly registerRenderers = (
+    definitions: Array<AnyExpressionRenderDefinition | (() => AnyExpressionRenderDefinition)>
+  ) => {
+    definitions.forEach(this.registerRenderer);
+  };
+
+  public readonly leaseRenderers = (
+    definitions: Array<AnyExpressionRenderDefinition | (() => AnyExpressionRenderDefinition)>
+  ) => {
+    const names = definitions.map((definition) => {
+      this.renderers.register(definition);
+      return definition.name;
+    });
+
+    return () => {
+      names.forEach((name) => this.renderers.remove(name));
+    };
+  };
 
   public readonly run: ExpressionsServiceStart['run'] = (ast, input, params) =>
     this.executor.run(ast, input, params);

x-pack/plugins/canvas/canvas_plugin_src/plugin.ts

@@ -7,16 +7,20 @@
 
 import { CoreSetup, CoreStart, Plugin } from 'src/core/public';
 import { ChartsPluginStart } from 'src/plugins/charts/public';
+import { ExpressionsSetup } from 'src/plugins/expressions/public';
+
 import { CanvasSetup } from '../public';
 import { EmbeddableStart } from '../../../../src/plugins/embeddable/public';
 import { UiActionsStart } from '../../../../src/plugins/ui_actions/public';
 import { Start as InspectorStart } from '../../../../src/plugins/inspector/public';
+import { AnyExpressionRenderDefinition } from '../types';
 
 import { functions } from './functions/browser';
 import { typeFunctions } from './expression_types';
 import { renderFunctions, renderFunctionFactories } from './renderers';
 interface SetupDeps {
   canvas: CanvasSetup;
+  expressions: ExpressionsSetup;
 }
 
 export interface StartDeps {
@@ -32,15 +36,24 @@ export type StartInitializer<T> = (core: CoreStart, plugins: StartDeps) => T;
 /** @internal */
 export class CanvasSrcPlugin implements Plugin<void, void, SetupDeps, StartDeps> {
   public setup(core: CoreSetup<StartDeps>, plugins: SetupDeps) {
-    plugins.canvas.addFunctions(functions);
-    plugins.canvas.addTypes(typeFunctions);
+    const { expressions } = plugins;
+    const releaseFunctions = expressions.leaseFunctions(functions);
+    const releaseTypes = expressions.leaseTypes(typeFunctions);
+
+    // There is an issue of the canvas render definition not matching the expression render definition
+    // due to our handlers needing additional methods.  For now, we are going to cast to get to the proper
+    // type, but we should work with AppArch to figure out how the Handlers can be genericized
+    const releaseRenderers = expressions.leaseRenderers(
+      (renderFunctions as unknown) as AnyExpressionRenderDefinition[]
+    );
 
-    plugins.canvas.addRenderers(renderFunctions);
+    let releaseFactories = () => {};
 
     core.getStartServices().then(([coreStart, depsStart]) => {
-      plugins.canvas.addRenderers(
-        renderFunctionFactories.map((factory: any) => factory(coreStart, depsStart))
+      const renderers = renderFunctionFactories.map((factory: any) =>
+        factory(coreStart, depsStart)
       );
+      releaseFactories = expressions.leaseRenderers(renderers);
     });
 
     plugins.canvas.addDatasourceUIs(async () => {
@@ -81,6 +94,13 @@ export class CanvasSrcPlugin implements Plugin<void, void, SetupDeps, StartDeps>
       const { transformSpecs } = await import('./canvas_addons');
       return transformSpecs;
     });
+
+    return () => {
+      releaseFunctions();
+      releaseTypes();
+      releaseRenderers();
+      releaseFactories();
+    };
   }
 
   public start(core: CoreStart, plugins: StartDeps) {}

x-pack/plugins/canvas/public/plugin.tsx

@@ -75,7 +75,8 @@ export class CanvasPlugin
   private appUpdater = new BehaviorSubject<AppUpdater>(() => ({}));
 
   public setup(coreSetup: CoreSetup<CanvasStartDeps>, setupPlugins: CanvasSetupDeps) {
-    const { api: canvasApi, registries } = getPluginApi(setupPlugins.expressions);
+    const { expressions } = setupPlugins;
+    const { api: canvasApi, registries } = getPluginApi();
 
     // Set the nav link to the last saved url if we have one in storage
     const lastPath = getSessionStorage().get(
@@ -97,7 +98,7 @@ export class CanvasPlugin
       mount: async (params: AppMountParameters) => {
         const { CanvasSrcPlugin } = await import('../canvas_plugin_src/plugin');
         const srcPlugin = new CanvasSrcPlugin();
-        srcPlugin.setup(coreSetup, { canvas: canvasApi });
+        const teardown = srcPlugin.setup(coreSetup, { canvas: canvasApi, expressions });
 
         // Get start services
         const [coreStart, startPlugins] = await coreSetup.getStartServices();
@@ -123,6 +124,7 @@ export class CanvasPlugin
 
         return () => {
           unmount();
+          teardown();
           teardownCanvas(coreStart);
         };
       },

x-pack/plugins/canvas/public/plugin_api.ts

@@ -5,30 +5,19 @@
  * 2.0.
  */
 
-import {
-  AnyExpressionFunctionDefinition,
-  AnyExpressionTypeDefinition,
-  AnyExpressionRenderDefinition,
-  AnyRendererFactory,
-} from '../types';
 import { ElementFactory } from '../types';
-import { ExpressionsSetup } from '../../../../src/plugins/expressions/public';
 
 type SpecPromiseFn<T extends any> = () => Promise<T[]>;
 type AddToRegistry<T extends any> = (add: T[] | SpecPromiseFn<T>) => void;
-type AddSpecsToRegistry<T extends any> = (add: T[]) => void;
 
 export interface CanvasApi {
   addArgumentUIs: AddToRegistry<any>;
   addDatasourceUIs: AddToRegistry<any>;
   addElements: AddToRegistry<ElementFactory>;
-  addFunctions: AddSpecsToRegistry<() => AnyExpressionFunctionDefinition>;
   addModelUIs: AddToRegistry<any>;
-  addRenderers: AddSpecsToRegistry<AnyRendererFactory>;
   addTagUIs: AddToRegistry<any>;
   addTransformUIs: AddToRegistry<any>;
   addTransitions: AddToRegistry<any>;
-  addTypes: AddSpecsToRegistry<() => AnyExpressionTypeDefinition>;
   addViewUIs: AddToRegistry<any>;
 }
 
@@ -43,9 +32,7 @@ export interface SetupRegistries extends Record<string, any[]> {
   transitions: any[];
 }
 
-export function getPluginApi(
-  expressionsPluginSetup: ExpressionsSetup
-): { api: CanvasApi; registries: SetupRegistries } {
+export function getPluginApi(): { api: CanvasApi; registries: SetupRegistries } {
   const registries: SetupRegistries = {
     elements: [],
     transformUIs: [],
@@ -68,26 +55,6 @@ export function getPluginApi(
   };
 
   const api: CanvasApi = {
-    // Functions, types and renderers are registered directly to expression plugin
-    addFunctions: (fns) => {
-      fns.forEach((fn) => {
-        expressionsPluginSetup.registerFunction(fn);
-      });
-    },
-    addTypes: (types) => {
-      types.forEach((type) => {
-        expressionsPluginSetup.registerType(type as any);
-      });
-    },
-    addRenderers: (renderers) => {
-      renderers.forEach((r) => {
-        // There is an issue of the canvas render definition not matching the expression render definition
-        // due to our handlers needing additional methods.  For now, we are going to cast to get to the proper
-        // type, but we should work with AppArch to figure out how the Handlers can be genericized
-        expressionsPluginSetup.registerRenderer((r as unknown) as AnyExpressionRenderDefinition);
-      });
-    },
-
     // All these others are local to canvas, and they will only register on start
     addElements: addToRegistry(registries.elements),
     addTransformUIs: addToRegistry(registries.transformUIs),