From 9ef375d83cfa13a1c2addeed740e024bdd84c203 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 17 Jun 2023 08:52:57 +0100 Subject: [PATCH] fix: Import max length not correctly communicated on import (#5434) --- server/collaboration/PersistenceExtension.ts | 8 +-- server/collaboration/utils/markdownToYDoc.ts | 47 -------------- server/commands/documentImporter.ts | 12 ++++ server/models/helpers/ProsemirrorHelper.tsx | 67 ++++++++++++++++++-- server/routes/api/documents/documents.ts | 3 +- server/test/setup.ts | 5 -- 6 files changed, 81 insertions(+), 61 deletions(-) delete mode 100644 server/collaboration/utils/markdownToYDoc.ts diff --git a/server/collaboration/PersistenceExtension.ts b/server/collaboration/PersistenceExtension.ts index d0f9ddd39..4342778e7 100644 --- a/server/collaboration/PersistenceExtension.ts +++ b/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, diff --git a/server/collaboration/utils/markdownToYDoc.ts b/server/collaboration/utils/markdownToYDoc.ts deleted file mode 100644 index 216bfe0ab..000000000 --- a/server/collaboration/utils/markdownToYDoc.ts +++ /dev/null @@ -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); -} diff --git a/server/commands/documentImporter.ts b/server/commands/documentImporter.ts index 4fbc18f35..e12252f34 100644 --- a/server/commands/documentImporter.ts +++ b/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, }; } diff --git a/server/models/helpers/ProsemirrorHelper.tsx b/server/models/helpers/ProsemirrorHelper.tsx index 86494a6ad..1eeae724d 100644 --- a/server/models/helpers/ProsemirrorHelper.tsx +++ b/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) { diff --git a/server/routes/api/documents/documents.ts b/server/routes/api/documents/documents.ts index 7b4a63f78..7a29a5da4 100644 --- a/server/routes/api/documents/documents.ts +++ b/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, diff --git a/server/test/setup.ts b/server/test/setup.ts index 86a5e5af4..b8ae95cbe 100644 --- a/server/test/setup.ts +++ b/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());