From e288a6ad18f5c53c280945b2eb21d93a811bbb90 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 3 Nov 2021 13:38:16 -0700 Subject: [PATCH] PluginController: Allow enabling / disabling plugins (#116) * Added enable/disable plugin to PluginController * Added guard for startPlugin when disabled * Added guard support for transitions and stopPlugin inside disablePlugin * Update packages/controllers/src/plugins/PluginController.ts Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com> * Added docs outlining XState conventions * Update packages/controllers/src/plugins/PluginController.ts Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com> --- .../src/plugins/PluginController.test.ts | 115 ++++++++++++++++++ .../src/plugins/PluginController.ts | 84 ++++++++++++- 2 files changed, 195 insertions(+), 4 deletions(-) diff --git a/packages/controllers/src/plugins/PluginController.test.ts b/packages/controllers/src/plugins/PluginController.test.ts index 89f9190ec0..c9c3a0a453 100644 --- a/packages/controllers/src/plugins/PluginController.test.ts +++ b/packages/controllers/src/plugins/PluginController.test.ts @@ -380,6 +380,7 @@ describe('PluginController Controller', () => { version: '0.0.1', sourceCode: 'console.log("foo")', name: 'foo', + enabled: true, status: PluginStatus.idle, }, }, @@ -929,4 +930,118 @@ describe('PluginController Controller', () => { }); expect(results).toStrictEqual('test1'); }); + + it('can add a plugin disable/enable it and still get a response from method "test"', async () => { + const messenger = new ControllerMessenger< + PluginControllerActions, + ErrorMessageEvent | UnresponsiveMessageEvent + >().getRestricted({ + name: 'PluginController', + allowedEvents: [ + 'ServiceMessenger:unhandledError', + 'ServiceMessenger:unresponsive', + ], + }); + const webWorkerExecutionEnvironment = + new WebWorkerExecutionEnvironmentService({ + messenger, + setupPluginProvider: jest.fn(), + workerUrl: new URL(URL.createObjectURL(new Blob([workerCode]))), + }); + const pluginController = new PluginController({ + idleTimeCheckInterval: 1000, + maxIdleTime: 2000, + terminateAllPlugins: + webWorkerExecutionEnvironment.terminateAllPlugins.bind( + webWorkerExecutionEnvironment, + ), + terminatePlugin: webWorkerExecutionEnvironment.terminatePlugin.bind( + webWorkerExecutionEnvironment, + ), + executePlugin: webWorkerExecutionEnvironment.executePlugin.bind( + webWorkerExecutionEnvironment, + ), + getRpcMessageHandler: + webWorkerExecutionEnvironment.getRpcMessageHandler.bind( + webWorkerExecutionEnvironment, + ), + removeAllPermissionsFor: jest.fn(), + getPermissions: jest.fn(), + hasPermission: jest.fn(), + requestPermissions: jest.fn(), + closeAllConnections: jest.fn(), + messenger, + }); + + const plugin = await pluginController.add({ + name: 'TestPlugin', + sourceCode: ` + wallet.registerRpcMessageHandler(async (origin, request) => { + const {method, params, id} = request; + wallet.request({method: 'setState'}) + return method + id; + }); + `, + manifest: { + web3Wallet: { + initialPermissions: {}, + }, + version: '0.0.0-development', + }, + }); + + const handler = await pluginController.getRpcMessageHandler(plugin.name); + + await pluginController.startPlugin(plugin.name); + + expect(pluginController.state.plugins[plugin.name].status).toStrictEqual( + 'running', + ); + + pluginController.disablePlugin(plugin.name); + + expect(pluginController.state.plugins[plugin.name].status).toStrictEqual( + 'stopped', + ); + + await expect(pluginController.startPlugin(plugin.name)).rejects.toThrow( + /^Plugin "TestPlugin" is disabled.$/u, + ); + + await expect( + handler('foo.com', { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 1, + }), + ).rejects.toThrow(/^Plugin "TestPlugin" is disabled.$/u); + + expect(pluginController.state.plugins[plugin.name].status).toStrictEqual( + 'stopped', + ); + + expect(pluginController.state.plugins[plugin.name].enabled).toStrictEqual( + false, + ); + + pluginController.enablePlugin(plugin.name); + + const results = await handler('foo.com', { + jsonrpc: '2.0', + method: 'test', + params: {}, + id: 1, + }); + + expect(pluginController.state.plugins[plugin.name].enabled).toStrictEqual( + true, + ); + + expect(pluginController.state.plugins[plugin.name].status).toStrictEqual( + 'running', + ); + + expect(results).toStrictEqual('test1'); + }); }); diff --git a/packages/controllers/src/plugins/PluginController.ts b/packages/controllers/src/plugins/PluginController.ts index 4c8ea630e7..0c0043a8f0 100644 --- a/packages/controllers/src/plugins/PluginController.ts +++ b/packages/controllers/src/plugins/PluginController.ts @@ -42,6 +42,7 @@ export type SerializablePlugin = { permissionName: string; version: string; status: PluginStatus; + enabled: boolean; }; export type Plugin = SerializablePlugin & { @@ -163,12 +164,27 @@ export enum PluginStatusEvent { crash = 'crash', } +/** + * Guard transitioning when the plugin is disabled. + */ +const disabledGuard = (serializedPlugin: SerializablePlugin) => { + return serializedPlugin.enabled; +}; + +/** + * The state machine configuration for a plugins `status` state. + * Using a state machine for a plugins `status` ensures that the plugin transitions to a valid next lifecycle state. + * Supports a very minimal subset of XState conventions outlined in `_transitionPluginState`. + */ const pluginStatusStateMachineConfig = { initial: PluginStatus.idle, states: { [PluginStatus.idle]: { on: { - [PluginStatusEvent.start]: PluginStatus.running, + [PluginStatusEvent.start]: { + target: PluginStatus.running, + cond: disabledGuard, + }, }, }, [PluginStatus.running]: { @@ -179,12 +195,18 @@ const pluginStatusStateMachineConfig = { }, [PluginStatus.stopped]: { on: { - [PluginStatusEvent.start]: PluginStatus.running, + [PluginStatusEvent.start]: { + target: PluginStatus.running, + cond: disabledGuard, + }, }, }, [PluginStatus.crashed]: { on: { - [PluginStatusEvent.start]: PluginStatus.running, + [PluginStatusEvent.start]: { + target: PluginStatus.running, + cond: disabledGuard, + }, }, }, }, @@ -352,11 +374,33 @@ export class PluginController extends BaseController< this.addPluginError(error); } + /** + * Transitions between states using `pluginStatusStateMachineConfig` as the template to figure out the next state. + * This transition function uses a very minimal subset of XState conventions: + * - supports initial state + * - .on supports raw event target string + * - .on supports {target, cond} object + * - the arguments for `cond` is the `SerializedPlugin` instead of Xstate convention of `(event, context) => boolean` + * @param pluginName the name of the plugin to transition + * @param event the event enum to use to transition + */ _transitionPluginState(pluginName: string, event: PluginStatusEvent) { const pluginStatus = this.state.plugins[pluginName].status; - const nextStatus = + let nextStatus = (pluginStatusStateMachineConfig.states[pluginStatus].on as any)[event] ?? pluginStatus; + if (nextStatus.cond) { + const cond = nextStatus.cond(this.state.plugins[pluginName]); + if (cond === false) { + throw new Error( + `Condition failed for state transition "${pluginName}" with event "${event}".`, + ); + } + } + + if (nextStatus.target) { + nextStatus = nextStatus.target; + } if (nextStatus === pluginStatus) { return; @@ -411,12 +455,39 @@ export class PluginController extends BaseController< throw new Error(`Plugin "${pluginName}" not found.`); } + if (this.state.plugins[pluginName].enabled === false) { + throw new Error(`Plugin "${pluginName}" is disabled.`); + } + await this._startPlugin({ pluginName, sourceCode: plugin.sourceCode, }); } + /** + * Enables the given plugin. A plugin can only be started if it is enabled. + * + * @param pluginName - The name of the plugin to enable. + */ + enablePlugin(pluginName: string): void { + this.update((state: any) => { + state.plugins[pluginName].enabled = true; + }); + } + + /** + * Disables the given plugin. A plugin can only be started if it is enabled. + * + * @param pluginName - The name of the plugin to disable. + */ + disablePlugin(pluginName: string): void { + this.stopPlugin(pluginName); + this.update((state: any) => { + state.plugins[pluginName].enabled = false; + }); + } + /** * Stops the given plugin. Throws an error if no such plugin exists * or if it is already stopped. @@ -804,6 +875,7 @@ export class PluginController extends BaseController< permissionName: PLUGIN_PREFIX + pluginName, // so we can easily correlate them sourceCode, version: manifest.version, + enabled: true, status: pluginStatusStateMachineConfig.initial, }; @@ -951,6 +1023,10 @@ export class PluginController extends BaseController< ) => { let handler = await this._getRpcMessageHandler(pluginName); + if (this.state.plugins[pluginName].enabled === false) { + throw new Error(`Plugin "${pluginName}" is disabled.`); + } + if (!handler && this.isRunning(pluginName) === false) { // cold start await this.startPlugin(pluginName);