PluginManager refactor (#6677)

* fix: refactor plugin manager

* fix: make id optional

* fix: allow add to accept single object

* fix: getHooks

* fix: tsc

* fix: remove id

plugins/azure/server/index.ts

@@ -1,9 +1,14 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import router from "./auth/azure";
 import env from "./env";
 
-PluginManager.register(PluginType.AuthProvider, router, {
+const enabled = !!env.AZURE_CLIENT_ID && !!env.AZURE_CLIENT_SECRET;
+
+if (enabled) {
+  PluginManager.add({
     ...config,
-  enabled: !!env.AZURE_CLIENT_ID && !!env.AZURE_CLIENT_SECRET,
-});
+    type: Hook.AuthProvider,
+    value: { router, id: config.id },
+  });
+}

plugins/email/plugin.json

@@ -1,5 +1,6 @@
 {
   "id": "email",
   "name": "Email",
+  "priority": 200,
   "description": "Adds an email magic link authentication provider."
 }

plugins/email/server/index.ts

@@ -1,9 +1,14 @@
 import env from "@server/env";
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import router from "./auth/email";
 
-PluginManager.register(PluginType.AuthProvider, router, {
+const enabled = (!!env.SMTP_HOST && !!env.SMTP_USERNAME) || env.isDevelopment;
+
+if (enabled) {
+  PluginManager.add({
     ...config,
-  enabled: (!!env.SMTP_HOST && !!env.SMTP_USERNAME) || env.isDevelopment,
-});
+    type: Hook.AuthProvider,
+    value: { router, id: config.id },
+  });
+}

plugins/google/server/index.ts

@@ -1,9 +1,14 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { PluginManager, Hook } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import router from "./auth/google";
 import env from "./env";
 
-PluginManager.register(PluginType.AuthProvider, router, {
+const enabled = !!env.GOOGLE_CLIENT_ID && !!env.GOOGLE_CLIENT_SECRET;
+
+if (enabled) {
+  PluginManager.add({
     ...config,
-  enabled: !!env.GOOGLE_CLIENT_ID && !!env.GOOGLE_CLIENT_SECRET,
-});
+    type: Hook.AuthProvider,
+    value: { router, id: config.id },
+  });
+}

plugins/iframely/server/index.ts

@@ -1,15 +1,21 @@
 import {
   PluginManager,
   PluginPriority,
-  PluginType,
+  Hook,
 } from "@server/utils/PluginManager";
 import env from "./env";
 import Iframely from "./iframely";
 
-PluginManager.register(PluginType.UnfurlProvider, Iframely.get, {
-  id: "iframely",
-  enabled: !!env.IFRAMELY_API_KEY && !!env.IFRAMELY_URL,
+const enabled = !!env.IFRAMELY_API_KEY && !!env.IFRAMELY_URL;
+
+if (enabled) {
+  PluginManager.add([
+    {
+      type: Hook.UnfurlProvider,
+      value: Iframely.get,
 
       // Make sure this is last in the stack to be evaluated after all other unfurl providers
       priority: PluginPriority.VeryLow,
-});
+    },
+  ]);
+}

plugins/oidc/server/index.ts

@@ -1,16 +1,21 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { PluginManager, Hook } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import router from "./auth/oidc";
 import env from "./env";
 
-PluginManager.register(PluginType.AuthProvider, router, {
-  ...config,
-  name: env.OIDC_DISPLAY_NAME || config.name,
-  enabled: !!(
+const enabled = !!(
   env.OIDC_CLIENT_ID &&
   env.OIDC_CLIENT_SECRET &&
   env.OIDC_AUTH_URI &&
   env.OIDC_TOKEN_URI &&
   env.OIDC_USERINFO_URI
-  ),
-});
+);
+
+if (enabled) {
+  PluginManager.add({
+    ...config,
+    type: Hook.AuthProvider,
+    value: { router, id: config.id },
+    name: env.OIDC_DISPLAY_NAME || config.name,
+  });
+}

plugins/slack/server/index.ts

@@ -1,4 +1,4 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { PluginManager, Hook } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import hooks from "./api/hooks";
 import router from "./auth/slack";
@@ -7,14 +7,21 @@ import SlackProcessor from "./processors/SlackProcessor";
 
 const enabled = !!env.SLACK_CLIENT_ID && !!env.SLACK_CLIENT_SECRET;
 
-PluginManager.register(PluginType.AuthProvider, router, {
+if (enabled) {
+  PluginManager.add([
+    {
       ...config,
-  enabled,
-});
-
-PluginManager.register(PluginType.API, hooks, {
+      type: Hook.AuthProvider,
+      value: { router, id: config.id },
+    },
+    {
       ...config,
-  enabled,
-});
-
-PluginManager.registerProcessor(SlackProcessor, { enabled });
+      type: Hook.API,
+      value: hooks,
+    },
+    {
+      type: Hook.Processor,
+      value: SlackProcessor,
+    },
+  ]);
+}

plugins/storage/server/index.ts

@@ -1,7 +1,11 @@
 import { existsSync, mkdirSync } from "fs";
 import env from "@server/env";
 import Logger from "@server/logging/Logger";
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import {
+  PluginManager,
+  PluginPriority,
+  Hook,
+} from "@server/utils/PluginManager";
 import router from "./api/files";
 
 if (env.FILE_STORAGE === "local") {
@@ -19,13 +23,20 @@ if (env.FILE_STORAGE === "local") {
   }
 }
 
-PluginManager.register(PluginType.API, router, {
-  id: "files",
-  name: "Local file storage",
-  description: "Plugin for storing files on the local file system",
-  enabled: !!(
+const enabled = !!(
   env.FILE_STORAGE_UPLOAD_MAX_SIZE &&
   env.FILE_STORAGE_LOCAL_ROOT_DIR &&
   env.FILE_STORAGE === "local"
-  ),
-});
+);
+
+if (enabled) {
+  PluginManager.add([
+    {
+      name: "Local file storage",
+      description: "Plugin for storing files on the local file system",
+      type: Hook.API,
+      value: router,
+      priority: PluginPriority.Normal,
+    },
+  ]);
+}

plugins/webhooks/plugin.json

@@ -1,5 +1,6 @@
 {
   "id": "webhooks",
   "name": "Webhooks",
+  "priority": 200,
   "description": "Adds HTTP webhooks for various events."
 }

plugins/webhooks/server/index.ts

@@ -1,11 +1,26 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { PluginManager, Hook } from "@server/utils/PluginManager";
 import config from "../plugin.json";
 import webhookSubscriptions from "./api/webhookSubscriptions";
 import WebhookProcessor from "./processors/WebhookProcessor";
 import CleanupWebhookDeliveriesTask from "./tasks/CleanupWebhookDeliveriesTask";
 import DeliverWebhookTask from "./tasks/DeliverWebhookTask";
 
-PluginManager.register(PluginType.API, webhookSubscriptions, config)
-  .registerProcessor(WebhookProcessor)
-  .registerTask(DeliverWebhookTask)
-  .registerTask(CleanupWebhookDeliveriesTask);
+PluginManager.add([
+  {
+    ...config,
+    type: Hook.API,
+    value: webhookSubscriptions,
+  },
+  {
+    type: Hook.Processor,
+    value: WebhookProcessor,
+  },
+  {
+    type: Hook.Task,
+    value: DeliverWebhookTask,
+  },
+  {
+    type: Hook.Task,
+    value: CleanupWebhookDeliveriesTask,
+  },
+]);

server/emails/templates/index.ts

@@ -1,4 +1,4 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import { requireDirectory } from "@server/utils/fs";
 import BaseEmail from "./BaseEmail";
 
@@ -14,8 +14,8 @@ requireDirectory<{ default: BaseEmail<any> }>(__dirname).forEach(
   }
 );
 
-PluginManager.getEnabledPlugins(PluginType.EmailTemplate).forEach((plugin) => {
-  emails[plugin.id] = plugin.value;
+PluginManager.getHooks(Hook.EmailTemplate).forEach((hook) => {
+  emails[hook.value.name] = hook.value;
 });
 
 export default emails;

server/models/helpers/AuthenticationHelper.ts

@@ -2,7 +2,7 @@
 import find from "lodash/find";
 import env from "@server/env";
 import Team from "@server/models/Team";
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 
 export default class AuthenticationHelper {
   /**
@@ -12,7 +12,7 @@ export default class AuthenticationHelper {
    * @returns A list of authentication providers
    */
   public static get providers() {
-    return PluginManager.getEnabledPlugins(PluginType.AuthProvider);
+    return PluginManager.getHooks(Hook.AuthProvider);
   }
 
   /**
@@ -26,11 +26,11 @@ export default class AuthenticationHelper {
     const isCloudHosted = env.isCloudHosted;
 
     return AuthenticationHelper.providers
-      .sort((plugin) => (plugin.id === "email" ? 1 : -1))
-      .filter((plugin) => {
+      .sort((hook) => (hook.value.id === "email" ? 1 : -1))
+      .filter((hook) => {
         // Email sign-in is an exception as it does not have an authentication
         // provider using passport, instead it exists as a boolean option.
-        if (plugin.id === "email") {
+        if (hook.value.id === "email") {
           return team?.emailSigninEnabled;
         }
 
@@ -40,7 +40,7 @@ export default class AuthenticationHelper {
         }
 
         const authProvider = find(team.authenticationProviders, {
-          name: plugin.id,
+          name: hook.value.id,
         });
 
         // If cloud hosted then the auth provider must be enabled for the team,

server/presenters/providerConfig.ts

@@ -1,12 +1,12 @@
 import { signin } from "@shared/utils/routeHelpers";
-import { Plugin, PluginType } from "@server/utils/PluginManager";
+import { Plugin, Hook } from "@server/utils/PluginManager";
 
 export default function presentProviderConfig(
-  config: Plugin<PluginType.AuthProvider>
+  config: Plugin<Hook.AuthProvider>
 ) {
   return {
-    id: config.id,
+    id: config.value.id,
     name: config.name,
-    authUrl: signin(config.id),
+    authUrl: signin(config.value.id),
   };
 }

server/queues/processors/index.ts

@@ -1,4 +1,4 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import { requireDirectory } from "@server/utils/fs";
 import BaseProcessor from "./BaseProcessor";
 
@@ -13,8 +13,8 @@ requireDirectory<{ default: BaseProcessor }>(__dirname).forEach(
   }
 );
 
-PluginManager.getEnabledPlugins(PluginType.Processor).forEach((plugin) => {
-  processors[plugin.id] = plugin.value;
+PluginManager.getHooks(Hook.Processor).forEach((hook) => {
+  processors[hook.value.name] = hook.value;
 });
 
 export default processors;

server/queues/tasks/index.ts

@@ -1,4 +1,4 @@
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import { requireDirectory } from "@server/utils/fs";
 import BaseTask from "./BaseTask";
 
@@ -13,8 +13,8 @@ requireDirectory<{ default: BaseTask<any> }>(__dirname).forEach(
   }
 );
 
-PluginManager.getEnabledPlugins(PluginType.Task).forEach((plugin) => {
-  tasks[plugin.id] = plugin.value;
+PluginManager.getHooks(Hook.Task).forEach((hook) => {
+  tasks[hook.value.name] = hook.value;
 });
 
 export default tasks;

server/routes/api/authenticationProviders/authenticationProviders.ts

@@ -89,13 +89,15 @@ router.post(
     )) as AuthenticationProvider[];
 
     const data = AuthenticationHelper.providers
-      .filter((p) => p.id !== "email")
+      .filter((p) => p.value.id !== "email")
       .map((p) => {
-        const row = teamAuthenticationProviders.find((t) => t.name === p.id);
+        const row = teamAuthenticationProviders.find(
+          (t) => t.name === p.value.id
+        );
 
         return {
-          id: p.id,
-          name: p.id,
+          id: p.value.id,
+          name: p.value.id,
           displayName: p.name,
           isEnabled: false,
           isConnected: false,

server/routes/api/index.ts

@@ -6,7 +6,7 @@ import env from "@server/env";
 import { NotFoundError } from "@server/errors";
 import coalesceBody from "@server/middlewares/coaleseBody";
 import { AppState, AppContext } from "@server/types";
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import apiKeys from "./apiKeys";
 import attachments from "./attachments";
 import auth from "./auth";
@@ -59,8 +59,8 @@ api.use(apiResponse());
 api.use(editor());
 
 // Register plugin API routes before others to allow for overrides
-PluginManager.getEnabledPlugins(PluginType.API).forEach((plugin) =>
-  router.use("/", plugin.value.routes())
+PluginManager.getHooks(Hook.API).forEach((hook) =>
+  router.use("/", hook.value.routes())
 );
 
 // routes

server/routes/api/urls/urls.ts

@@ -13,12 +13,12 @@ import { authorize } from "@server/policies";
 import { presentDocument, presentMention } from "@server/presenters/unfurls";
 import presentUnfurl from "@server/presenters/unfurls/unfurl";
 import { APIContext } from "@server/types";
-import { PluginManager, PluginType } from "@server/utils/PluginManager";
+import { Hook, PluginManager } from "@server/utils/PluginManager";
 import { RateLimiterStrategy } from "@server/utils/RateLimiter";
 import * as T from "./schema";
 
 const router = new Router();
-const plugins = PluginManager.getEnabledPlugins(PluginType.UnfurlProvider);
+const plugins = PluginManager.getHooks(Hook.UnfurlProvider);
 
 router.post(
   "urls.unfurl",

server/routes/auth/index.ts

@@ -17,7 +17,7 @@ router.use(passport.initialize());
 
 // dynamically load available authentication provider routes
 AuthenticationHelper.providers.forEach((provider) => {
-  router.use("/", provider.value.routes());
+  router.use("/", provider.value.router.routes());
 });
 
 router.get("/redirect", auth(), async (ctx: APIContext) => {

server/utils/PluginManager.ts

@@ -1,8 +1,8 @@
 import path from "path";
 import { glob } from "glob";
 import type Router from "koa-router";
+import isArray from "lodash/isArray";
 import sortBy from "lodash/sortBy";
-import { v4 as uuid } from "uuid";
 import { UnfurlSignature } from "@shared/types";
 import type BaseEmail from "@server/emails/templates/BaseEmail";
 import env from "@server/env";
@@ -21,7 +21,7 @@ export enum PluginPriority {
 /**
  * The different types of server plugins that can be registered.
  */
-export enum PluginType {
+export enum Hook {
   API = "api",
   AuthProvider = "authProvider",
   EmailTemplate = "emailTemplate",
@@ -35,100 +35,56 @@ export enum PluginType {
  * Router. Registering an API plugin causes the router to be mounted.
  */
 type PluginValueMap = {
-  [PluginType.API]: Router;
-  [PluginType.AuthProvider]: Router;
-  [PluginType.EmailTemplate]: typeof BaseEmail;
-  [PluginType.Processor]: typeof BaseProcessor;
-  [PluginType.Task]: typeof BaseTask<any>;
-  [PluginType.UnfurlProvider]: UnfurlSignature;
+  [Hook.API]: Router;
+  [Hook.AuthProvider]: { router: Router; id: string };
+  [Hook.EmailTemplate]: typeof BaseEmail;
+  [Hook.Processor]: typeof BaseProcessor;
+  [Hook.Task]: typeof BaseTask<any>;
+  [Hook.UnfurlProvider]: UnfurlSignature;
 };
 
-export type Plugin<T extends PluginType> = {
-  /** A unique ID for the plugin */
-  id: string;
+export type Plugin<T extends Hook> = {
+  /** Plugin type */
+  type: T;
   /** The plugin's display name */
   name?: string;
   /** A brief description of the plugin */
   description?: string;
   /** The plugin content */
   value: PluginValueMap[T];
-  /** An optional priority, will affect order in menus and execution. Lower is earlier. */
+  /** Priority will affect order in menus and execution. Lower is earlier. */
   priority?: number;
-  /** Whether the plugin is enabled (default: true) */
-  enabled?: boolean;
 };
 
 export class PluginManager {
-  private static plugins = new Map<PluginType, Plugin<PluginType>[]>();
-
+  private static plugins = new Map<Hook, Plugin<Hook>[]>();
   /**
-   * Register a plugin of a given type.
-   *
-   * @param type The plugin type
-   * @param value The plugin value
-   * @param options Additional options, including whether the plugin is enabled and it's priority.
-   * @returns The PluginManager instance, for chaining.
+   * Add plugins
+   * @param plugins
    */
-  public static register<T extends PluginType>(
-    type: T,
-    value: PluginValueMap[T],
-    options: Omit<Plugin<T>, "value"> = {
-      id: uuid(),
-    }
-  ) {
-    if (!this.plugins.has(type)) {
-      this.plugins.set(type, []);
+  public static add(plugins: Array<Plugin<Hook>> | Plugin<Hook>) {
+    if (isArray(plugins)) {
+      return plugins.forEach((plugin) => this.register(plugin));
     }
 
-    const plugin = {
-      value,
-      priority: PluginPriority.Normal,
-      ...options,
-    };
+    this.register(plugins);
+  }
+
+  private static register<T extends Hook>(plugin: Plugin<T>) {
+    if (!this.plugins.has(plugin.type)) {
+      this.plugins.set(plugin.type, []);
+    }
+
+    this.plugins
+      .get(plugin.type)!
+      .push({ ...plugin, priority: plugin.priority ?? PluginPriority.Normal });
 
     Logger.debug(
       "plugins",
-      `Plugin ${options.enabled === false ? "disabled" : "enabled"} "${
-        options.id
-      }" ${options.description ? `(${options.description})` : ""}`
+      `Plugin(type=${plugin.type}) registered ${
+        "name" in plugin.value ? plugin.value.name : ""
+      } ${plugin.description ? `(${plugin.description})` : ""}`
     );
-
-    this.plugins.get(type)!.push(plugin);
-
-    // allow chaining
-    return this;
-  }
-
-  /**
-   * Syntactic sugar for registering a background Task.
-   *
-   * @param value The task class
-   * @param options Additional options
-   */
-  public static registerTask(
-    value: PluginValueMap[PluginType.Task],
-    options?: Omit<Plugin<PluginType.Task>, "id" | "value">
-  ) {
-    return this.register(PluginType.Task, value, {
-      id: value.name,
-      ...options,
-    });
-  }
-
-  /**
-   * Syntactic sugar for registering a background Processor.
-   *
-   * @param value The processor class
-   * @param options Additional options
-   */
-  public static registerProcessor(
-    value: PluginValueMap[PluginType.Processor],
-    options?: Omit<Plugin<PluginType.Processor>, "id" | "value">
-  ) {
-    return this.register(PluginType.Processor, value, {
-      id: value.name,
-      ...options,
-    });
   }
 
   /**
@@ -137,21 +93,11 @@ export class PluginManager {
    * @param type The type of plugin to filter by
    * @returns A list of plugins
    */
-  public static getPlugins<T extends PluginType>(type: T) {
+  public static getHooks<T extends Hook>(type: T) {
     this.loadPlugins();
     return sortBy(this.plugins.get(type) || [], "priority") as Plugin<T>[];
   }
 
-  /**
-   * Returns all the enabled plugins of a given type in order of priority.
-   *
-   * @param type The type of plugin to filter by
-   * @returns A list of plugins
-   */
-  public static getEnabledPlugins<T extends PluginType>(type: T) {
-    return this.getPlugins(type).filter((plugin) => plugin.enabled !== false);
-  }
-
   /**
    * Load plugin server components (anything in the `/server/` directory of a plugin will be loaded)
    */