perf: Removes JSDom usage on main server -> worker (#13128)

* refactor

* fix: Test setup for awaited jobs

* feedback

server/queues/tasks/DocumentImportTask.ts

@@ -3,19 +3,33 @@ import type { SourceMetadata } from "@shared/types";
 import documentCreator from "@server/commands/documentCreator";
 import documentImporter from "@server/commands/documentImporter";
 import { createContext } from "@server/context";
-import { User } from "@server/models";
+import { InvalidRequestError } from "@server/errors";
+import { Document, User } from "@server/models";
 import FileStorage from "@server/storage/files";
 import { sequelize } from "@server/storage/database";
+import type { AuthenticationType } from "@server/types";
 import { BaseTask, TaskPriority } from "./base/BaseTask";
 
 type Props = {
   userId: string;
+  /** The content to import, for callers that hold it in memory. */
+  content?: string;
+  /** The file storage key the content is staged under, removed once imported. */
+  key?: string;
+  /** Name and mime type of the content, recorded on the created document. */
   sourceMetadata: Pick<Required<SourceMetadata>, "fileName" | "mimeType">;
+  /** Attributes for the created document, taking precedence over those derived from the content. */
+  attributes?: {
+    title?: string;
+    icon?: string;
+    color?: string;
+    fullWidth?: boolean;
+  };
   publish?: boolean;
   collectionId?: string | null;
   parentDocumentId?: string | null;
-  ip: string;
-  key: string;
+  authType?: AuthenticationType | null;
+  ip?: string;
 };
 
 export type DocumentImportTaskResponse =
@@ -27,9 +41,34 @@ export type DocumentImportTaskResponse =
     };
 
 export default class DocumentImportTask extends BaseTask<Props> {
+  /**
+   * Imports a document on a worker, keeping conversion off the process that
+   * received the request, and blocks until the worker has finished.
+   *
+   * @param props the content to import and attributes for the created document.
+   * @returns the created document.
+   * @throws InvalidRequestError if the content could not be converted.
+   */
+  public static async scheduleAndWait(props: Props): Promise<Document> {
+    const job = await new DocumentImportTask().schedule(props);
+    const response: DocumentImportTaskResponse = await job.finished();
+
+    if ("error" in response) {
+      throw InvalidRequestError(response.error);
+    }
+
+    return Document.findByPk(response.documentId, {
+      userId: props.userId,
+      rejectOnEmpty: true,
+    });
+  }
+
   public async perform({
     key,
+    content,
     sourceMetadata,
+    attributes,
+    authType,
     ip,
     publish,
     collectionId,
@@ -37,19 +76,29 @@ export default class DocumentImportTask extends BaseTask<Props> {
     userId,
   }: Props): Promise<DocumentImportTaskResponse> {
     try {
-      const content = await FileStorage.getFileBuffer(key);
+      let body: Buffer | string;
+      if (key) {
+        body = await FileStorage.getFileBuffer(key);
+      } else if (content !== undefined) {
+        // Empty content is valid and imports an empty document, so only a
+        // missing one is rejected.
+        body = content;
+      } else {
+        throw InvalidRequestError("one of key or content is required");
+      }
+
       const user = await User.findByPk(userId, {
         rejectOnEmpty: true,
       });
 
       // Run document conversion and image downloading outside a transaction
-      const ctx = createContext({ user, ip });
+      const ctx = createContext({ user, authType, ip });
 
       const { text, state, title, icon } = await documentImporter({
         user,
         fileName: sourceMetadata.fileName,
         mimeType: sourceMetadata.mimeType,
-        content,
+        content: body,
         ctx,
       });
 
@@ -57,13 +106,16 @@ export default class DocumentImportTask extends BaseTask<Props> {
         documentCreator(
           createContext({
             user,
+            authType,
             ip,
             transaction,
           }),
           {
             sourceMetadata,
-            title,
-            icon,
+            title: attributes?.title ?? title,
+            icon: attributes?.icon ?? icon,
+            color: attributes?.color,
+            fullWidth: attributes?.fullWidth,
             text,
             state,
             publish,
@@ -76,7 +128,9 @@ export default class DocumentImportTask extends BaseTask<Props> {
     } catch (err) {
       return { error: errToString(err) };
     } finally {
-      await FileStorage.deleteFile(key);
+      if (key) {
+        await FileStorage.deleteFile(key);
+      }
     }
   }
 

server/routes/api/documents/documents.ts

@@ -83,7 +83,6 @@ import {
   presentGroup,
   presentFileOperation,
 } from "@server/presenters";
-import type { DocumentImportTaskResponse } from "@server/queues/tasks/DocumentImportTask";
 import DocumentImportTask from "@server/queues/tasks/DocumentImportTask";
 import EmptyTrashTask from "@server/queues/tasks/EmptyTrashTask";
 import FileStorage from "@server/storage/files";
@@ -1604,7 +1603,7 @@ router.post(
       });
     }
 
-    const job = await new DocumentImportTask().schedule({
+    const document = await DocumentImportTask.scheduleAndWait({
       key,
       sourceMetadata: {
         fileName,
@@ -1614,17 +1613,9 @@ router.post(
       collectionId: collectionId ?? parentDocument?.collectionId,
       parentDocumentId,
       publish,
+      authType: ctx.state.auth.type,
       ip: ctx.request.ip,
     });
-    const response: DocumentImportTaskResponse = await job.finished();
-    if ("error" in response) {
-      throw InvalidRequestError(response.error);
-    }
-
-    const document = await Document.findByPk(response.documentId, {
-      userId: user.id,
-      rejectOnEmpty: true,
-    });
 
     ctx.body = {
       data: await presentDocument(ctx, document),

server/test/setup.ts

@@ -1,6 +1,8 @@
 import "reflect-metadata";
 import { EventEmitter } from "node:events";
+import type { Job } from "bull";
 import { afterAll, afterEach, beforeAll, beforeEach, vi } from "vitest";
+import type { BaseTask } from "@server/queues/tasks/base/BaseTask";
 import sharedEnv from "@shared/env";
 import env from "@server/env";
 import { server } from "./msw";
@@ -61,6 +63,21 @@ void pluginModules;
 // no-op during tests.
 (PluginManager as unknown as { loaded: boolean }).loaded = true;
 
+// No worker consumes the task queue in tests, so a scheduled task is performed
+// lazily by the job handle instead. Callers that block on the result get it;
+// scheduling alone stays a no-op, as it is when the queue has no processor.
+const { BaseTask: BaseTaskClass } =
+  await import("@server/queues/tasks/base/BaseTask");
+
 beforeEach(() => {
   env.URL = sharedEnv.URL = "https://app.outline.dev";
+
+  vi.spyOn(BaseTaskClass.prototype, "schedule").mockImplementation(function (
+    this: BaseTask<object>,
+    props: object
+  ) {
+    return Promise.resolve({
+      finished: () => this.perform(props),
+    } as Job);
+  });
 });

server/tools/documents.ts

@@ -5,12 +5,12 @@ import documentCreator, {
   authorizeDocumentCreate,
   authorizeDocumentPublish,
 } from "@server/commands/documentCreator";
-import documentImporter from "@server/commands/documentImporter";
 import documentMover from "@server/commands/documentMover";
 import documentRestorer from "@server/commands/documentRestorer";
 import documentUpdater from "@server/commands/documentUpdater";
 import { Collection, Document, SearchQuery, Template } from "@server/models";
 import { SearchQuerySource } from "@server/models/SearchQuery";
+import DocumentImportTask from "@server/queues/tasks/DocumentImportTask";
 import { sequelize } from "@server/storage/database";
 import { authorize, can } from "@server/policies";
 import {
@@ -351,6 +351,9 @@ export function documentTools(server: McpServer, scopes: string[]) {
             .describe(
               'The format of the text content. Defaults to "markdown"; use "html" for rich HTML input.'
             ),
+          sourceFileName: optionalString().describe(
+            'The name of the file the content was read from, e.g. "notes.md". Recorded on the document and shown to users as the file it was imported from, so only set it when the content genuinely came from a file. Also used as the title when the content has no heading and no title is given.'
+          ),
           collectionId: optionalString().describe(
             "The collection to place the document in."
           ),
@@ -399,29 +402,46 @@ export function documentTools(server: McpServer, scopes: string[]) {
             authorize(user, "read", template);
           }
 
-          const imported =
+          // Parsing HTML loads a full DOM, which would block the event loop for
+          // every other request, so hand it to a worker instead.
+          const document =
             input.format === "html"
-              ? await documentImporter({
-                  user,
-                  fileName: "document.html",
-                  mimeType: "text/html",
+              ? await DocumentImportTask.scheduleAndWait({
                   content: input.text ?? "",
-                  ctx,
+                  sourceMetadata: {
+                    fileName: input.sourceFileName ?? "document.html",
+                    mimeType: "text/html",
+                  },
+                  attributes: {
+                    title: input.title,
+                    icon: input.icon,
+                    color: input.color,
+                    fullWidth: input.fullWidth,
+                  },
+                  userId: user.id,
+                  publish: input.publish !== false,
+                  collectionId: collection?.id,
+                  parentDocumentId,
+                  authType: ctx.state.auth.type,
+                  ip: ctx.context.ip,
                 })
-              : undefined;
-
-          const document = await documentCreator(ctx, {
-            title: input.title ?? imported?.title,
-            text: imported?.text ?? input.text,
-            state: imported?.state,
-            icon: input.icon ?? imported?.icon,
-            color: input.color,
-            parentDocumentId: parentDocumentId,
-            publish: input.publish !== false,
-            collectionId: collection?.id,
-            template: imported ? undefined : template,
-            fullWidth: input.fullWidth,
-          });
+              : await documentCreator(ctx, {
+                  title: input.title,
+                  text: input.text,
+                  icon: input.icon,
+                  color: input.color,
+                  parentDocumentId,
+                  publish: input.publish !== false,
+                  collectionId: collection?.id,
+                  template,
+                  fullWidth: input.fullWidth,
+                  sourceMetadata: input.sourceFileName
+                    ? {
+                        fileName: input.sourceFileName,
+                        mimeType: "text/markdown",
+                      }
+                    : undefined,
+                });
 
           const [{ text, ...attributes }, breadcrumb] = await Promise.all([
             presentDocument(document, {