fix: Import max length not correctly communicated on import (#5434)

server/collaboration/PersistenceExtension.ts

@@ -9,8 +9,8 @@ import { sequelize } from "@server/database/sequelize";
 import Logger from "@server/logging/Logger";
 import { trace } from "@server/logging/tracing";
 import Document from "@server/models/Document";
+import ProsemirrorHelper from "@server/models/helpers/ProsemirrorHelper";
 import documentCollaborativeUpdater from "../commands/documentCollaborativeUpdater";
-import markdownToYDoc from "./utils/markdownToYDoc";
 
 @trace()
 export default class PersistenceExtension implements Extension {
@@ -51,11 +51,11 @@ export default class PersistenceExtension implements Extension {
         "database",
         `Document ${documentId} is not in state, creating from markdown`
       );
-      const ydoc = markdownToYDoc(document.text, fieldName);
-      const state = Y.encodeStateAsUpdate(ydoc);
+      const ydoc = ProsemirrorHelper.toYDoc(document.text, fieldName);
+      const state = ProsemirrorHelper.toState(ydoc);
       await document.update(
         {
-          state: Buffer.from(state),
+          state,
         },
         {
           silent: true,

server/collaboration/utils/markdownToYDoc.ts

@@ -1,47 +0,0 @@
-import { prosemirrorToYDoc } from "@getoutline/y-prosemirror";
-import { Node, Fragment } from "prosemirror-model";
-import * as Y from "yjs";
-import embeds from "@shared/editor/embeds";
-import { parser, schema } from "@server/editor";
-
-export default function markdownToYDoc(
-  markdown: string,
-  fieldName = "default"
-): Y.Doc {
-  let node = parser.parse(markdown);
-
-  // in the editor embeds were created at runtime by converting links
-  // into embeds where they match. Because we're converting to a CRDT structure
-  // on the server we need to mimic this behavior.
-  function urlsToEmbeds(node: Node): Node | null {
-    if (node.type.name === "paragraph") {
-      // @ts-expect-error ts-migrate(2339) FIXME: Property 'content' does not exist on type 'Fragmen... Remove this comment to see the full error message
-      for (const textNode of node.content.content) {
-        for (const embed of embeds) {
-          if (textNode.text && embed.matcher(textNode.text)) {
-            return schema.nodes.embed.createAndFill({
-              href: textNode.text,
-            });
-          }
-        }
-      }
-    }
-
-    if (node.content) {
-      const contentAsArray =
-        // @ts-expect-error content
-        node.content instanceof Fragment ? node.content.content : node.content;
-      // @ts-expect-error content
-      node.content = Fragment.fromArray(contentAsArray.map(urlsToEmbeds));
-    }
-
-    return node;
-  }
-
-  if (node) {
-    node = urlsToEmbeds(node);
-  }
-
-  // @ts-expect-error null node
-  return prosemirrorToYDoc(node, fieldName);
-}

server/commands/documentImporter.ts

@@ -9,6 +9,7 @@ import parseTitle from "@shared/utils/parseTitle";
 import { DocumentValidation } from "@shared/validations";
 import { traceFunction } from "@server/logging/tracing";
 import { User } from "@server/models";
+import ProsemirrorHelper from "@server/models/helpers/ProsemirrorHelper";
 import dataURItoBuffer from "@server/utils/dataURItoBuffer";
 import parseImages from "@server/utils/parseImages";
 import turndownService from "@server/utils/turndown";
@@ -149,6 +150,7 @@ async function documentImporter({
 }): Promise<{
   text: string;
   title: string;
+  state: Buffer;
 }> {
   const fileInfo = importMapping.filter((item) => {
     if (item.type === mimeType) {
@@ -225,8 +227,18 @@ async function documentImporter({
   // It's better to truncate particularly long titles than fail the import
   title = truncate(title, { length: DocumentValidation.maxTitleLength });
 
+  const ydoc = ProsemirrorHelper.toYDoc(text);
+  const state = ProsemirrorHelper.toState(ydoc);
+
+  if (state.length > DocumentValidation.maxStateLength) {
+    throw InvalidRequestError(
+      `The document is too large to import, please reduce the length and try again`
+    );
+  }
+
   return {
     text,
+    state,
     title,
   };
 }

server/models/helpers/ProsemirrorHelper.tsx

@@ -1,13 +1,16 @@
+import { prosemirrorToYDoc } from "@getoutline/y-prosemirror";
 import { JSDOM } from "jsdom";
-import { Node, DOMSerializer } from "prosemirror-model";
+import { Node, DOMSerializer, Fragment } from "prosemirror-model";
 import * as React from "react";
 import { renderToString } from "react-dom/server";
 import styled, { ServerStyleSheet, ThemeProvider } from "styled-components";
+import * as Y from "yjs";
 import EditorContainer from "@shared/editor/components/Styles";
+import embeds from "@shared/editor/embeds";
 import GlobalStyles from "@shared/styles/globals";
 import light from "@shared/styles/theme";
 import { isRTL } from "@shared/utils/rtl";
-import { schema } from "@server/editor";
+import { schema, parser } from "@server/editor";
 import Logger from "@server/logging/Logger";
 import { trace } from "@server/logging/tracing";
 
@@ -31,9 +34,65 @@ type MentionAttrs = {
 @trace()
 export default class ProsemirrorHelper {
   /**
-   * Returns the data as a Prosemirror Node.
+   * Returns the input text as a Y.Doc.
    *
-   * @param node The node to parse
+   * @param markdown The text to parse
+   * @returns The content as a Y.Doc.
+   */
+  static toYDoc(markdown: string, fieldName = "default"): Y.Doc {
+    let node = parser.parse(markdown);
+
+    // in the editor embeds are created at runtime by converting links into
+    // embeds where they match.Because we're converting to a CRDT structure on
+    //  the server we need to mimic this behavior.
+    function urlsToEmbeds(node: Node): Node | null {
+      if (node.type.name === "paragraph") {
+        // @ts-expect-error content
+        for (const textNode of node.content.content) {
+          for (const embed of embeds) {
+            if (textNode.text && embed.matcher(textNode.text)) {
+              return schema.nodes.embed.createAndFill({
+                href: textNode.text,
+              });
+            }
+          }
+        }
+      }
+
+      if (node.content) {
+        const contentAsArray =
+          node.content instanceof Fragment
+            ? // @ts-expect-error content
+              node.content.content
+            : node.content;
+        // @ts-expect-error content
+        node.content = Fragment.fromArray(contentAsArray.map(urlsToEmbeds));
+      }
+
+      return node;
+    }
+
+    if (node) {
+      node = urlsToEmbeds(node);
+    }
+
+    return node ? prosemirrorToYDoc(node, fieldName) : new Y.Doc();
+  }
+
+  /**
+   * Returns the input Y.Doc encoded as a YJS state update.
+   *
+   * @param ydoc The Y.Doc to encode
+   * @returns The content as a YJS state update
+   */
+  static toState(ydoc: Y.Doc) {
+    return Buffer.from(Y.encodeStateAsUpdate(ydoc));
+  }
+
+  /**
+   * Converts a plain object into a Prosemirror Node.
+   *
+   * @param data The object to parse
    * @returns The content as a Prosemirror Node
    */
   static toProsemirror(data: Record<string, any>) {

server/routes/api/documents/documents.ts

@@ -1219,7 +1219,7 @@ router.post(
 
     const content = await fs.readFile(file.filepath);
     const document = await sequelize.transaction(async (transaction) => {
-      const { text, title } = await documentImporter({
+      const { text, state, title } = await documentImporter({
         user,
         fileName: file.originalFilename ?? file.newFilename,
         mimeType: file.mimetype ?? "",
@@ -1232,6 +1232,7 @@ router.post(
         source: "import",
         title,
         text,
+        state,
         publish,
         collectionId,
         parentDocumentId,

server/test/setup.ts

@@ -9,9 +9,6 @@ jest.mock("bull");
 // This is needed for the relative manual mock to be picked up
 jest.mock("../queues");
 
-// Avoid "Yjs was already imported" errors in the test environment
-jest.mock("yjs");
-
 // We never want to make real S3 requests in test environment
 jest.mock("aws-sdk", () => {
   const mS3 = {
@@ -26,6 +23,4 @@ jest.mock("aws-sdk", () => {
   };
 });
 
-jest.mock("@getoutline/y-prosemirror", () => ({}));
-
 afterAll(() => Redis.defaultClient.disconnect());