Switch from allowed/blocked list to validate function for plugins

jupyterlab · May 20, 2024 · e7aba13 · e7aba13
1 parent f660417
commit e7aba13
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 93 deletions.
diff --git a/packages/application/src/plugins.ts b/packages/application/src/plugins.ts
@@ -121,21 +121,9 @@ export interface IPlugin<T, U> {
  */
 export class PluginRegistry<T = any> {
   constructor(options: PluginRegistry.IOptions = {}) {
-    if ((options.allowedPlugins?.size ?? 0) > 0) {
-      console.info('Only allowed plugins will be registered.');
-      this._isAllowed = (plugin: Private.IPluginData<T>) =>
-        options.allowedPlugins!.has(plugin.id);
-      if ((options.blockedPlugins?.size ?? 0) > 0) {
-        console.warn(
-          'Allowed and blocked plugins are defined simultaneously. The allowed list will take precedence.'
-        );
-      }
-    } else {
-      if ((options.blockedPlugins?.size ?? 0) > 0) {
-        console.info('Some plugins are not allowed to be registered');
-        this._isAllowed = (plugin: Private.IPluginData<T>) =>
-          !options.blockedPlugins!.has(plugin.id);
-      }
+    if (options.validatePlugin) {
+      console.info('Plugins may be rejected by the custom validation plugin method.');
+      this._validatePlugin = options.validatePlugin;
     }
   }
 
@@ -231,13 +219,13 @@ export class PluginRegistry<T = any> {
       throw new TypeError(`Plugin '${plugin.id}' is already registered.`);
     }
 
+    if (!this._validatePlugin(plugin)) {
+      throw new Error(`Plugin '${plugin.id}' is not valid.`);
+    }
+
     // Create the normalized plugin data.
     const data = Private.createPluginData(plugin);
 
-    if (!this._isAllowed(data)) {
-      throw new Error(`Plugin '${plugin.id}' is not allowed.`);
-    }
-
     // Ensure the plugin does not cause a cyclic dependency.
     Private.ensureNoCycle(data, this._plugins, this._services);
 
@@ -504,7 +492,7 @@ export class PluginRegistry<T = any> {
   }
 
   private _application: any = null;
-  private _isAllowed: (plugin: Private.IPluginData<T>) => boolean = () => true;
+  private _validatePlugin: (plugin: IPlugin<any, any>) => boolean = () => true;
   private _plugins = new Map<string, Private.IPluginData<T>>();
   private _services = new Map<Token<any>, string>();
 }
@@ -518,19 +506,18 @@ export namespace PluginRegistry {
    */
   export interface IOptions {
     /**
-     * List of allowed plugins
+     * Validate that a plugin is allowed to be registered.
      *
-     * If defined, only allowed plugins will be able to be registered.
+     * Default is `() => true`.
      *
-     * This parameter takes precedence over {@link blockedPlugins}.
-     */
-    allowedPlugins?: Set<string>;
-    /**
-     * List of blocked plugins
-     *
-     * If defined, blocked plugins will not be able to be registered.
+     * @param plugin The plugin to validate
+     * @returns Whether the plugin can be registered or not.
+     * 
+     * #### Notes
+     * We recommend you print a console message with the reason
+     * a plugin is invalid.
      */
-    blockedPlugins?: Set<string>;
+    validatePlugin?: (plugin: IPlugin<any, any>) => boolean;
   }
 
   /**

diff --git a/packages/application/tests/src/index.spec.ts b/packages/application/tests/src/index.spec.ts
@@ -9,7 +9,7 @@
 |----------------------------------------------------------------------------*/
 import { expect } from 'chai';
 
-import { Application, PluginRegistry } from '@lumino/application';
+import { Application, PluginRegistry, type IPlugin } from '@lumino/application';
 import { ContextMenu, Widget } from '@lumino/widgets';
 import { CommandRegistry } from '@lumino/commands';
 import { Token } from '@lumino/coreutils';
@@ -634,26 +634,9 @@ describe('@lumino/application', () => {
         expect(plugins).to.be.instanceOf(PluginRegistry);
       });
 
-      it('should accept allowed plugins list', () => {
+      it('should accept validation function', () => {
         const plugins = new PluginRegistry({
-          allowedPlugins: new Set(['plugin1', 'plugin2'])
-        });
-
-        expect(plugins).to.be.instanceOf(PluginRegistry);
-      });
-
-      it('should accept blocked plugins list', () => {
-        const plugins = new PluginRegistry({
-          blockedPlugins: new Set(['plugin1', 'plugin2'])
-        });
-
-        expect(plugins).to.be.instanceOf(PluginRegistry);
-      });
-
-      it('should accept allowed and blocked plugins lists', () => {
-        const plugins = new PluginRegistry({
-          allowedPlugins: new Set(['plugin1', 'plugin2']),
-          blockedPlugins: new Set(['plugin3'])
+          validatePlugin: (plugin: IPlugin<any, any>) => !['plugin1', 'plugin2'].includes(plugin.id)
         });
 
         expect(plugins).to.be.instanceOf(PluginRegistry);
@@ -929,9 +912,9 @@ describe('@lumino/application', () => {
         expect(plugins.hasPlugin(id)).to.be.true;
       });
 
-      it('should refuse to register not allowed plugins', async () => {
+      it('should refuse to register invalid plugins', async () => {
         const plugins = new PluginRegistry({
-          allowedPlugins: new Set(['id1'])
+          validatePlugin: (plugin: IPlugin<any, any>) => ['id1'].includes(plugin.id)
         });
         expect(function () {
           plugins.registerPlugin({
@@ -948,47 +931,6 @@ describe('@lumino/application', () => {
           }
         });
       });
-
-      it('should refuse to register blocked plugins', async () => {
-        const plugins = new PluginRegistry({
-          blockedPlugins: new Set(['id1'])
-        });
-        expect(function () {
-          plugins.registerPlugin({
-            id: 'id1',
-            activate: () => {
-              /* no-op */
-            }
-          });
-        }).to.throw();
-        plugins.registerPlugin({
-          id: 'id2',
-          activate: () => {
-            /* no-op */
-          }
-        });
-      });
-
-      it('should use allowed list over blocked list of plugins', async () => {
-        const plugins = new PluginRegistry({
-          allowedPlugins: new Set(['id1', 'id2']),
-          blockedPlugins: new Set(['id2'])
-        });
-        expect(function () {
-          plugins.registerPlugin({
-            id: 'id',
-            activate: () => {
-              /* no-op */
-            }
-          });
-        }).to.throw();
-        plugins.registerPlugin({
-          id: 'id2',
-          activate: () => {
-            /* no-op */
-          }
-        });
-      });
     });
 
     describe('#deregisterPlugin', () => {