From 7ae8db6b12a1c1e8d228cf814f31ae10d3ecee8b Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 26 Aug 2026 14:43:54 +0200 Subject: [PATCH] [editor-core] extract shared pieces for history-ot in editor (#36828) * [editor-core] share the pieces the editor, real-time and history-v1 all need Project-level history-OT has the browser, real-time and history-v1 all reason about the same changes, so the rules they reason with move into the shared library rather than being reimplemented on each side. - `origin/editor_origin.js` plus `EDITOR_ORIGIN_KIND` and its schema entry: the origin an editor client's change carries, holding the editorId that scopes a change's timestamp. - `change_identity.js`: how a submission is recognised when it comes back, so a resend of a change that already landed is deduplicated rather than applied twice. - `rebase.js`: transforming a change onto concurrent ones, in the same order on both sides, which is what lets the server acknowledge a change with only the version it landed at. - `text_file_defaults.js`: the doc-vs-file extension lists, now read from here by web's settings instead of being spelled out again. - `blob_string_length.js`: the string length of a blob, used by history-v1's blob store and by web when it writes an uploaded file into history. - `file_tree_diff.js`: file-tree operations to a diff, used by project-history's FileTreeDiffGenerator. - `Blob.EMPTY_HASH`, so callers can answer for the empty blob without storing it. Co-Authored-By: Claude Opus 5 * [project-history] build the file-tree diff with the shared helper FileTreeDiffGenerator's hand-rolled walk over add/edit/move/delete operations becomes a call to `buildFileTreeDiff` from overleaf-editor-core, so the editor and project-history read a file-tree change the same way. Adds the unit tests the decaffeinated original never had. Co-Authored-By: Claude Opus 5 * [web] read the editable-file lists from overleaf-editor-core web's settings spelled out the text extensions and editable filenames that decide whether a file becomes a doc, and every other service that has to make that call carried its own copy. The lists now live in overleaf-editor-core and web builds settings.textExtensions and settings.editableFilenames from them, keeping the ADDITIONAL_TEXT_EXTENSIONS override for Server CE/Pro on top. Co-Authored-By: Claude Opus 5 * [history-v1] read a blob's string length from overleaf-editor-core Two copies of getStringLengthOfFile lived here, the one in recover_zip.js carrying a TODO asking for exactly this. Both now import the shared implementation, which the blob store keeps re-exporting for backupArchiver. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 GitOrigin-RevId: 31948332f416f56848e3b0dfb5142a4c9f93f508 --- libraries/overleaf-editor-core/index.js | 20 + libraries/overleaf-editor-core/lib/blob.js | 12 + .../lib/blob_string_length.js | 54 +++ .../lib/change_identity.js | 108 +++++ libraries/overleaf-editor-core/lib/file.js | 5 + .../lib/file_tree_diff.js | 207 ++++++++ .../lib/operation/index.js | 4 +- .../lib/origin/editor_origin.js | 55 +++ .../overleaf-editor-core/lib/origin/index.js | 15 + libraries/overleaf-editor-core/lib/rebase.js | 53 ++ libraries/overleaf-editor-core/lib/schemas.js | 8 + .../lib/text_file_defaults.js | 84 ++++ libraries/overleaf-editor-core/package.json | 3 +- .../test/unit/blob_string_length.test.js | 104 ++++ .../test/unit/change_identity.test.js | 162 +++++++ .../test/unit/editor_origin.test.js | 102 ++++ .../test/unit/file_tree_diff.test.js | 456 ++++++++++++++++++ .../test/unit/rebase.test.js | 174 +++++++ .../storage/lib/blob_store/index.js | 23 +- .../history-v1/storage/scripts/recover_zip.js | 23 +- .../app/js/FileTreeDiffGenerator.js | 201 ++++---- .../test/acceptance/js/SyncTests.js | 2 +- .../FileTreeDiffGeneratorTests.js | 446 +++++++++++++++++ services/web/config/settings.defaults.js | 55 +-- yarn.lock | 1 + 25 files changed, 2172 insertions(+), 205 deletions(-) create mode 100644 libraries/overleaf-editor-core/lib/blob_string_length.js create mode 100644 libraries/overleaf-editor-core/lib/change_identity.js create mode 100644 libraries/overleaf-editor-core/lib/file_tree_diff.js create mode 100644 libraries/overleaf-editor-core/lib/origin/editor_origin.js create mode 100644 libraries/overleaf-editor-core/lib/rebase.js create mode 100644 libraries/overleaf-editor-core/lib/text_file_defaults.js create mode 100644 libraries/overleaf-editor-core/test/unit/blob_string_length.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/change_identity.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/editor_origin.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/file_tree_diff.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/rebase.test.js create mode 100644 services/project-history/test/unit/js/FileTreeDiffGenerator/FileTreeDiffGeneratorTests.js diff --git a/libraries/overleaf-editor-core/index.js b/libraries/overleaf-editor-core/index.js index 3bbb510a5c..baf10e31d8 100644 --- a/libraries/overleaf-editor-core/index.js +++ b/libraries/overleaf-editor-core/index.js @@ -25,11 +25,23 @@ const Operation = require('./lib/operation') const RestoreOrigin = require('./lib/origin/restore_origin') const RestoreFileOrigin = require('./lib/origin/restore_file_origin') const Origin = require('./lib/origin') +const { EDITOR_ORIGIN_KIND } = require('./lib/origin') +const EditorOrigin = require('./lib/origin/editor_origin') const OtClient = require('./lib/ot_client') +const rebaseChanges = require('./lib/rebase') +const { + editorChangeIdentity, + editorChangeIdentityOf, + isSameEditorChange, +} = require('./lib/change_identity') const TextOperation = require('./lib/operation/text_operation') const EditOperation = require('./lib/operation/edit_operation') const safePathname = require('./lib/safe_pathname') const Snapshot = require('./lib/snapshot') +const { + DEFAULT_TEXT_EXTENSIONS, + DEFAULT_EDITABLE_FILENAMES, +} = require('./lib/text_file_defaults') const util = require('./lib/util') const V2DocVersions = require('./lib/v2_doc_versions') const { @@ -60,6 +72,8 @@ exports.ChangeNote = ChangeNote exports.Chunk = Chunk exports.ChunkResponse = ChunkResponse exports.Comment = Comment +exports.DEFAULT_TEXT_EXTENSIONS = DEFAULT_TEXT_EXTENSIONS +exports.DEFAULT_EDITABLE_FILENAMES = DEFAULT_EDITABLE_FILENAMES exports.DeleteCommentOperation = DeleteCommentOperation exports.File = File exports.FileMap = FileMap @@ -80,7 +94,13 @@ exports.Operation = Operation exports.RestoreOrigin = RestoreOrigin exports.RestoreFileOrigin = RestoreFileOrigin exports.Origin = Origin +exports.EditorOrigin = EditorOrigin +exports.EDITOR_ORIGIN_KIND = EDITOR_ORIGIN_KIND exports.OtClient = OtClient +exports.rebaseChanges = rebaseChanges +exports.editorChangeIdentity = editorChangeIdentity +exports.editorChangeIdentityOf = editorChangeIdentityOf +exports.isSameEditorChange = isSameEditorChange exports.TextOperation = TextOperation exports.EditOperation = EditOperation exports.safePathname = safePathname diff --git a/libraries/overleaf-editor-core/lib/blob.js b/libraries/overleaf-editor-core/lib/blob.js index 7f1b7bb042..0226cf62c4 100644 --- a/libraries/overleaf-editor-core/lib/blob.js +++ b/libraries/overleaf-editor-core/lib/blob.js @@ -38,6 +38,18 @@ class Blob { */ static MAX_EDITABLE_BYTE_LENGTH_BOUND = 3 * TextOperation.MAX_STRING_LENGTH + /** + * The hash of empty content, which is the same for every project. + * + * git's hash of an empty blob, as `blobHash` computes it: it is a property of the + * content rather than of anything stored, so it can be recognised without looking + * anything up. `File.EMPTY_FILE_HASH` is the same value under the name a file's + * content goes by. + * + * @type {string} + */ + static EMPTY_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' + static NotFoundError = NotFoundError /** diff --git a/libraries/overleaf-editor-core/lib/blob_string_length.js b/libraries/overleaf-editor-core/lib/blob_string_length.js new file mode 100644 index 0000000000..513e191f73 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/blob_string_length.js @@ -0,0 +1,54 @@ +// @ts-check +'use strict' + +/** + * The rule that decides whether the content of a blob counts as editable text, + * and if so what string length to record on the {@link Blob}. + * + * This module is Node-only: it reads files and validates UTF-8 with a native + * addon. It must never be reachable from index.js, because overleaf-editor-core + * is bundled for the browser. Consumers deep import it, which the package + * exports allow: + * + * require('overleaf-editor-core/lib/blob_string_length') + */ + +const fs = require('node:fs') +const isValidUtf8 = require('utf-8-validate') + +const Blob = require('./blob') +const TextOperation = require('./operation/text_operation') +const { containsNonBmpChars } = require('./util') + +/** + * @param {Buffer} buffer + * @return {number | null} the string length, or null when the content is not + * editable text + */ +function getStringLengthOfBuffer(buffer) { + if (!isValidUtf8(buffer)) return null + const data = buffer.toString() + if (data.length > TextOperation.MAX_STRING_LENGTH) return null + // We cannot edit files containing non-BMP or null characters. + if (containsNonBmpChars(data)) return null + if (data.indexOf('\x00') !== -1) return null + return data.length +} + +/** + * @param {number} byteLength + * @param {string} pathname + * @return {Promise} the string length, or null when the content + * is not editable text + */ +async function getStringLengthOfFile(byteLength, pathname) { + // We have to read the file into memory to get its UTF-8 length, so don't + // bother for files that are too large for us to edit anyway. + if (byteLength > Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND) { + return null + } + + return getStringLengthOfBuffer(await fs.promises.readFile(pathname)) +} + +module.exports = { getStringLengthOfBuffer, getStringLengthOfFile } diff --git a/libraries/overleaf-editor-core/lib/change_identity.js b/libraries/overleaf-editor-core/lib/change_identity.js new file mode 100644 index 0000000000..42ed6f65cf --- /dev/null +++ b/libraries/overleaf-editor-core/lib/change_identity.js @@ -0,0 +1,108 @@ +// @ts-check + +'use strict' + +const { EDITOR_ORIGIN_KIND } = require('./origin') + +/** + * @import { RawChange } from "./types" + */ + +/** + * Identifying a change an editor client submitted. + * + * history-v1 uses this to recognise a change a client is resending, so as not to + * apply the same operations twice; the client uses it to recognise its own change + * coming back on the change stream, so as not to apply it a second time locally. + * Both sides have to agree exactly — if one of them compares a field the other + * does not, they disagree about what is a duplicate, and a change is either + * applied twice or discarded. Neither reports an error. So the key lives here + * rather than being spelled out in each service. + * + * The fields are the ones a rebase leaves alone: transforming a change rewrites + * its operations, so those cannot identify it, while the origin, the author and + * the timestamp travel through history untouched. + * + * Each rules out a different false match: + * + * - the origin kind keeps other writers out, so a Dropbox sync or a resync is + * never mistaken for a client's change. + * - `editorId` separates editors that share an author. Nothing coordinates + * timestamps between two tabs of one user, or between anonymous editors, who + * share an empty author — without it, two clients colliding on a millisecond + * look identical. + * - the author makes a forged `editorId` unusable: it arrives from the client, but + * a change belonging to another account can never match. + * - the timestamp separates successive changes from one editor, which the client + * keeps distinct and stable across resends. + * + * @typedef {{editorId: string, author: (string|null), timestamp: number}} EditorChangeIdentity + */ + +/** + * The identity of a raw change, or null if it is not an identifiable editor + * change — another writer's, or one whose `editorId` was dropped when its chunk + * was written. + * + * The timestamp is compared as a point in time rather than as a string, so that + * two spellings of the same instant cannot read as different changes. + * + * @param {RawChange} [raw] + * @return {EditorChangeIdentity | null} + */ +function editorChangeIdentity(raw) { + const origin = raw?.origin + if (!origin || origin.kind !== EDITOR_ORIGIN_KIND) return null + + // The origin union's catch-all variant is `{kind: string}`, which also admits + // this kind, so checking the kind cannot narrow to the variant carrying the id. + const { editorId } = /** @type {{editorId?: string}} */ (origin) + if (!editorId) return null + + // real-time stamps exactly one author on every change it forwards, so anything + // else did not come from this path. + const authors = raw.v2Authors + if (!Array.isArray(authors) || authors.length !== 1) return null + + const timestamp = new Date(raw.timestamp).getTime() + if (Number.isNaN(timestamp)) return null + + return { editorId, author: authors[0] ?? null, timestamp } +} + +/** + * Build an identity from a client's own record of a change it submitted, for + * comparing against what comes back from history. + * + * @param {Object} params + * @param {string} params.editorId + * @param {string | null} params.author + * @param {Date} params.timestamp + * @return {EditorChangeIdentity} + */ +function editorChangeIdentityOf({ editorId, author, timestamp }) { + return { editorId, author: author ?? null, timestamp: timestamp.getTime() } +} + +/** + * Whether two identities name the same change. A null identity matches nothing, + * including another null. + * + * @param {EditorChangeIdentity | null} [a] + * @param {EditorChangeIdentity | null} [b] + * @return {boolean} + */ +function isSameEditorChange(a, b) { + if (!a || !b) return false + return ( + a.editorId === b.editorId && + a.author === b.author && + a.timestamp === b.timestamp + ) +} + +module.exports = { + editorChangeIdentity, + editorChangeIdentityOf, + isSameEditorChange, +} diff --git a/libraries/overleaf-editor-core/lib/file.js b/libraries/overleaf-editor-core/lib/file.js index 870cffa9cb..c24990c8da 100644 --- a/libraries/overleaf-editor-core/lib/file.js +++ b/libraries/overleaf-editor-core/lib/file.js @@ -47,6 +47,11 @@ class File { /** * Blob hash for an empty file. * + * Spelled out rather than read from `Blob.EMPTY_HASH`, which is the same value: a + * static field initialised from another module's class is `undefined` wherever the + * load order has that module still mid-load, and then everything comparing a hash + * against this fails without saying anything. + * * @type {String} */ static EMPTY_FILE_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' diff --git a/libraries/overleaf-editor-core/lib/file_tree_diff.js b/libraries/overleaf-editor-core/lib/file_tree_diff.js new file mode 100644 index 0000000000..2ea689bedb --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_tree_diff.js @@ -0,0 +1,207 @@ +// @ts-check +'use strict' + +const AddFileOperation = require('./operation/add_file_operation') +const EditFileOperation = require('./operation/edit_file_operation') +const MoveFileOperation = require('./operation/move_file_operation') + +/** + * @import Change from './change' + * @import File from './file' + */ + +/** + * A file as it is tracked through a window of changes. + * + * @typedef {Object} FileTreeDiffEntry + * @property {string | null} origin the pathname the file had before the window, + * or null when the file was added inside the window + * @property {string[]} chain every pathname the file has occupied, in order. + * The first element is `origin` when the file existed before the + * window, and the last element is the pathname the file ends up at + * (the pathname it was removed from, for removed files). + * @property {boolean} edited whether an edit operation touched the file + * @property {number | null} firstEditedAtChainIndex index into `chain` of the + * pathname the file was at when it was first edited; null when the + * file was never edited + * @property {File | null} file the file added by the most recent add operation, + * null when the file was not added inside the window. Callers that + * need the pre-window file look it up in a snapshot by `origin`. + * @property {number | null} deletedAtChangeIndex index into `changes` of the + * change that removed the file from the tree; null while the file is + * part of the tree + */ + +/** + * Collapse a window of changes into one entry per file, keyed by the pathname + * the file ends up at. + * + * Chains of moves collapse into a single entry, so a file renamed from `a` to + * `b` and then to `c` is reported once, at `c`, with `origin` `a`. Only the + * file tree is folded: the content of edits is not inspected, edits are + * recorded as a flag on the entry. + * + * The fold follows FileMap semantics: adding a file at, or moving a file onto, + * a pathname that is occupied replaces the file that was there. A replaced file + * is reported as removed. + * + * Files that exist before the window are only known through + * `options.initialPathnames`. When it is given, the set of pathnames is + * authoritative: moves and removals of pathnames that are neither seeded nor + * added inside the window are skipped, because there is no file to move or + * remove. When it is omitted, any pathname an operation refers to is assumed to + * have existed before the window, and an entry is created for it on demand. An + * edit is always recorded, on an entry created on demand if need be. + * + * @param {Change[]} changes in order + * @param {object} [options] + * @param {Iterable} [options.initialPathnames] pathnames present before + * the window. Entries are reported in this order, followed by the + * pathnames that the window makes use of. + * @param {(entry: FileTreeDiffEntry, operation: MoveFileOperation) => void} + * [options.onMoveCollision] called before a move replaces the file at + * its target pathname, so that callers that consider this inconsistent + * can throw + * @return {{ + * entries: Map, + * removed: FileTreeDiffEntry[], + * }} `entries` is the state of the file tree at the end of the window, keyed by + * pathname: an entry with a `deletedAtChangeIndex` is a file that was + * removed from that pathname and that nothing has taken the place of since. + * `removed` is every file that left the tree during the window, in the order + * they left it, including the files that another file has since taken the + * place of. + */ +function buildFileTreeDiff( + changes, + { initialPathnames, onMoveCollision } = {} +) { + const seeded = initialPathnames != null + /** @type {Map} */ + const entries = new Map() + /** @type {FileTreeDiffEntry[]} */ + const removed = [] + + if (initialPathnames != null) { + for (const pathname of initialPathnames) { + entries.set(pathname, existingFileEntry(pathname)) + } + } + + /** + * The file at the given pathname, if the tree has one. + * + * @param {string} pathname + * @return {FileTreeDiffEntry | undefined} + */ + function getLiveEntry(pathname) { + const entry = entries.get(pathname) + if (entry != null && entry.deletedAtChangeIndex == null) return entry + } + + /** + * Record the file at the given pathname as removed, without freeing up the + * pathname: callers that replace the file set the new entry themselves, which + * keeps the position of the pathname in `entries`. + * + * @param {string} pathname + * @param {number} changeIndex + */ + function removeLiveEntry(pathname, changeIndex) { + const entry = getLiveEntry(pathname) + if (entry == null) return + entry.deletedAtChangeIndex = changeIndex + removed.push(entry) + } + + for (const [changeIndex, change] of changes.entries()) { + for (const operation of change.getOperations()) { + if (operation instanceof AddFileOperation) { + const pathname = operation.getPathname() + if (!pathname) continue + removeLiveEntry(pathname, changeIndex) + entries.set(pathname, addedFileEntry(pathname, operation.getFile())) + } else if (operation instanceof EditFileOperation) { + const pathname = operation.getPathname() + if (!pathname) continue + let entry = entries.get(pathname) + if (entry == null) { + entry = existingFileEntry(pathname) + entries.set(pathname, entry) + } else if (entry.deletedAtChangeIndex != null) { + // The file has been removed, there is nothing to edit. + continue + } + if (!entry.edited) { + entry.edited = true + entry.firstEditedAtChainIndex = entry.chain.length - 1 + } + } else if (operation instanceof MoveFileOperation) { + const pathname = operation.getPathname() + if (!pathname) continue + const newPathname = operation.getNewPathname() + if (operation.isRemoveFile()) { + if (!seeded && !entries.has(pathname)) { + entries.set(pathname, existingFileEntry(pathname)) + } + removeLiveEntry(pathname, changeIndex) + } else if (pathname !== newPathname) { + const target = getLiveEntry(newPathname) + if (target != null && onMoveCollision != null) { + onMoveCollision(target, operation) + } + let entry = getLiveEntry(pathname) + if (entry == null) { + // Only a pathname that the window has not made use of can be + // assumed to have held a file before the window. + if (seeded || entries.has(pathname)) continue + entry = existingFileEntry(pathname) + } + removeLiveEntry(newPathname, changeIndex) + entry.chain.push(newPathname) + entries.set(newPathname, entry) + entries.delete(pathname) + } + } + } + } + + return { entries, removed } +} + +/** + * An entry for a file that was there before the window. + * + * @param {string} pathname + * @return {FileTreeDiffEntry} + */ +function existingFileEntry(pathname) { + return { + origin: pathname, + chain: [pathname], + edited: false, + firstEditedAtChainIndex: null, + file: null, + deletedAtChangeIndex: null, + } +} + +/** + * An entry for a file that an add operation created inside the window. + * + * @param {string} pathname + * @param {File} file + * @return {FileTreeDiffEntry} + */ +function addedFileEntry(pathname, file) { + return { + origin: null, + chain: [pathname], + edited: false, + firstEditedAtChainIndex: null, + file, + deletedAtChangeIndex: null, + } +} + +module.exports = { buildFileTreeDiff } diff --git a/libraries/overleaf-editor-core/lib/operation/index.js b/libraries/overleaf-editor-core/lib/operation/index.js index 47b08c37a6..c3e0456a0f 100644 --- a/libraries/overleaf-editor-core/lib/operation/index.js +++ b/libraries/overleaf-editor-core/lib/operation/index.js @@ -13,7 +13,7 @@ let EditFileOperation = null let SetFileMetadataOperation = null /** - * @import { ReadonlyBlobStore } from "../types" + * @import { ReadonlyBlobStore, RawOperation } from "../types" * @import Snapshot from "../snapshot" */ @@ -55,7 +55,7 @@ class Operation { /** * Serialize an Operation. * - * @return {Object} + * @return {RawOperation} */ toRaw() { return {} diff --git a/libraries/overleaf-editor-core/lib/origin/editor_origin.js b/libraries/overleaf-editor-core/lib/origin/editor_origin.js new file mode 100644 index 0000000000..d8bdcfbff4 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/origin/editor_origin.js @@ -0,0 +1,55 @@ +'use strict' + +const assert = require('check-types').assert + +const Origin = require('.') + +/** + * An Origin for a change an editor client submitted over the `applyHistoryOt` + * rpc. + * + * The `editorId` identifies the editor instance the change came from. It is what + * scopes the change's timestamp: a client keeps its timestamps distinct, but two + * clients of the same user do not coordinate, so the pair is what identifies a + * submission. history-v1 uses it to recognise a change it has already applied, + * rather than applying a resend twice. + * + * It is only needed while a change can still be resent, so it is dropped from + * all but the latest change of an editor when a chunk is written to storage. The + * plain {@link Origin} with this kind is what remains, and it keeps recording + * that the change came from the editor. + */ +class EditorOrigin extends Origin { + /** + * @param {string} editorId a UUID identifying the editor instance + */ + constructor(editorId) { + assert.nonEmptyString(editorId, 'EditorOrigin: bad editorId') + + super(EditorOrigin.KIND) + this.editorId = editorId + } + + static fromRaw(raw) { + return new EditorOrigin(raw.editorId) + } + + /** @inheritdoc */ + toRaw() { + return { + kind: EditorOrigin.KIND, + editorId: this.editorId, + } + } + + /** + * @return {string} + */ + getEditorId() { + return this.editorId + } +} + +EditorOrigin.KIND = Origin.EDITOR_ORIGIN_KIND + +module.exports = EditorOrigin diff --git a/libraries/overleaf-editor-core/lib/origin/index.js b/libraries/overleaf-editor-core/lib/origin/index.js index 6575157f7c..878a3d4ce2 100644 --- a/libraries/overleaf-editor-core/lib/origin/index.js +++ b/libraries/overleaf-editor-core/lib/origin/index.js @@ -7,6 +7,14 @@ const assert = require('check-types').assert let RestoreOrigin = null let RestoreFileOrigin = null let RestoreProjectOrigin = null +let EditorOrigin = null + +/** + * The origin kind of a change the editor submitted over the `applyHistoryOt` + * rpc. real-time stamps it and history-v1 matches on it when recognising a + * resend, so both need the same string. + */ +const EDITOR_ORIGIN_KIND = 'editor' /** * An Origin records where a {@link Change} came from. The Origin class handles @@ -37,6 +45,11 @@ class Origin { return RestoreFileOrigin.fromRaw(raw) if (raw.kind === RestoreProjectOrigin.KIND) return RestoreProjectOrigin.fromRaw(raw) + // Only an editor change that still carries its editorId is an EditorOrigin. + // The id is dropped from all but an editor's latest change when a chunk is + // written, so most editor changes read back from storage are plain origins. + if (raw.kind === EditorOrigin.KIND && raw.editorId) + return EditorOrigin.fromRaw(raw) return new Origin(raw.kind) } @@ -58,7 +71,9 @@ class Origin { } module.exports = Origin +module.exports.EDITOR_ORIGIN_KIND = EDITOR_ORIGIN_KIND RestoreOrigin = require('./restore_origin') RestoreFileOrigin = require('./restore_file_origin') RestoreProjectOrigin = require('./restore_project_origin') +EditorOrigin = require('./editor_origin') diff --git a/libraries/overleaf-editor-core/lib/rebase.js b/libraries/overleaf-editor-core/lib/rebase.js new file mode 100644 index 0000000000..976ce62e79 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/rebase.js @@ -0,0 +1,53 @@ +// @ts-check + +'use strict' + +const Operation = require('./operation') + +/** + * @typedef {import('./change')} Change + */ + +/** + * Rebase changes onto the changes another writer got in first. + * + * `Operation.transformMultiple` rewrites *both* operation lists in place, which + * is what makes a sequence of our changes come out right: each of ours is + * transformed against their change as it stands after our earlier ones, rather + * than against the original. + * + * Operations that transform down to a no-op are pruned, and a change left with + * none is dropped, so the result contains only changes that still carry an + * operation. An empty result means everything we had was already accounted for + * by theirs. + * + * Only the operations are touched. A change's `origin`, `authors`, `v2Authors` + * and `timestamp` travel through untouched, which is what lets a caller + * recognise one of its own changes read back from history after it has been + * rebased. + * + * @param {Change[]} ours modified in place + * @param {Change[]} theirs the intervening changes, in version order + * @return {Change[]} the subset of `ours` that still carries an operation + */ +function rebaseChanges(ours, theirs) { + for (const change of theirs) { + const theirOperations = change.getOperations() + for (const ourChange of ours) { + Operation.transformMultiple(ourChange.getOperations(), theirOperations) + } + } + + const rebased = [] + for (const change of ours) { + const operations = change + .getOperations() + .filter(operation => !operation.isNoOp()) + if (operations.length === 0) continue + change.setOperations(operations) + rebased.push(change) + } + return rebased +} + +module.exports = rebaseChanges diff --git a/libraries/overleaf-editor-core/lib/schemas.js b/libraries/overleaf-editor-core/lib/schemas.js index 6c441a8f16..cb19c7507a 100644 --- a/libraries/overleaf-editor-core/lib/schemas.js +++ b/libraries/overleaf-editor-core/lib/schemas.js @@ -258,6 +258,14 @@ const rawOrigin = z.union([ version: z.number().int(), timestamp: z.iso.datetime(), }), + // An editor change that still carries the id of the editor instance it came + // from. Once a chunk is written the id is dropped from all but that editor's + // latest change, leaving a bare {kind: 'editor'} that the catch-all below + // accepts. + z.strictObject({ + kind: z.literal('editor'), + editorId: z.uuid(), + }), z.strictObject({ kind: z.string(), }), diff --git a/libraries/overleaf-editor-core/lib/text_file_defaults.js b/libraries/overleaf-editor-core/lib/text_file_defaults.js new file mode 100644 index 0000000000..ac0501c028 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/text_file_defaults.js @@ -0,0 +1,84 @@ +// @ts-check +'use strict' + +/** + * The file names that may hold editable text, as data. + * + * These lists decide whether a file becomes a doc or a binary file, so every + * service that classifies a file has to work from the same list: one carrying a + * shorter copy turns the same bytes into a binary file, and a project changes + * the type of a file just by taking a different path into history. Consumers + * read them from here instead of carrying a copy. web layers the + * ADDITIONAL_TEXT_EXTENSIONS override on top for Server CE/Pro. + * + * Frozen because they are shared: copy them (`.concat(...)`, `.slice()`) before + * handing them to anything that merges configuration into its own objects. + * + * When adding an extension, see + * developer-manual/development/code/editable-text-file-extensions.md for the + * other places that need touching. + */ + +/** + * Extensions without a leading dot, lower case. + * + * @type {ReadonlyArray} + */ +const DEFAULT_TEXT_EXTENSIONS = Object.freeze([ + 'tex', + 'latex', + 'sty', + 'cls', + 'bst', + 'bib', + 'bibtex', + 'txt', + 'tikz', + 'mtx', + 'rtex', + 'md', + 'asy', + 'lbx', + 'bbx', + 'cbx', + 'm', + 'lco', + 'dtx', + 'ins', + 'ist', + 'def', + 'clo', + 'ldf', + 'rmd', + 'qmd', + 'lua', + 'py', + 'gv', + 'mf', + 'yml', + 'yaml', + 'lhs', + 'lean', + 'lean4', + 'hs', + 'mk', + 'xmpdata', + 'cfg', + 'rnw', + 'ltx', + 'inc', +]) + +/** + * Whole file names, lower case, editable whatever their extension says. + * + * @type {ReadonlyArray} + */ +const DEFAULT_EDITABLE_FILENAMES = Object.freeze([ + 'latexmkrc', + '.latexmkrc', + 'makefile', + 'gnumakefile', +]) + +module.exports = { DEFAULT_TEXT_EXTENSIONS, DEFAULT_EDITABLE_FILENAMES } diff --git a/libraries/overleaf-editor-core/package.json b/libraries/overleaf-editor-core/package.json index f3ba1b0be6..443494041a 100644 --- a/libraries/overleaf-editor-core/package.json +++ b/libraries/overleaf-editor-core/package.json @@ -39,6 +39,7 @@ "diff-match-patch": "overleaf/diff-match-patch#89805f9c671a77a263fc53461acd62aa7498f688", "lodash": "^4.18.1", "p-map": "^4.0.0", - "path-browserify": "^1.0.1" + "path-browserify": "^1.0.1", + "utf-8-validate": "^5.0.4" } } diff --git a/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js b/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js new file mode 100644 index 0000000000..2f4d5d14f8 --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js @@ -0,0 +1,104 @@ +// @ts-check +'use strict' + +const { expect } = require('chai') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') + +const { + getStringLengthOfBuffer, + getStringLengthOfFile, +} = require('../../lib/blob_string_length') +const Blob = require('../../lib/blob') +const TextOperation = require('../../lib/operation/text_operation') + +describe('blob string length', function () { + /** @type {string} */ + let tmpDir + + beforeEach(async function () { + tmpDir = await fs.promises.mkdtemp( + path.join(os.tmpdir(), 'blob-string-length-') + ) + }) + + afterEach(async function () { + await fs.promises.rm(tmpDir, { recursive: true, force: true }) + }) + + /** + * @param {Buffer} content + * @return {Promise} + */ + async function writeFile(content) { + const pathname = path.join(tmpDir, 'blob') + await fs.promises.writeFile(pathname, content) + return pathname + } + + describe('getStringLengthOfBuffer', function () { + it('counts the characters of plain ASCII', function () { + expect(getStringLengthOfBuffer(Buffer.from('hello'))).to.equal(5) + }) + + it('counts characters rather than bytes for multi-byte UTF-8', function () { + // 'é' is two bytes, 'ᚠ' is three. + const buffer = Buffer.from('héllᚠ') + expect(buffer.byteLength).to.equal(8) + expect(getStringLengthOfBuffer(buffer)).to.equal(5) + }) + + it('rejects bytes that are not valid UTF-8', function () { + // 0x80 is a continuation byte with nothing to continue. + expect(getStringLengthOfBuffer(Buffer.from([0x61, 0x80, 0x62]))).to.be + .null + }) + + it('rejects a NUL character', function () { + expect(getStringLengthOfBuffer(Buffer.from('a\0b'))).to.be.null + }) + + it('rejects non-BMP characters', function () { + expect(getStringLengthOfBuffer(Buffer.from('a 🙂 face'))).to.be.null + }) + + it('rejects content longer than a text operation can hold', function () { + const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH + 1) + expect(getStringLengthOfBuffer(Buffer.from(content))).to.be.null + }) + + it('accepts content at the maximum length', function () { + const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH) + expect(getStringLengthOfBuffer(Buffer.from(content))).to.equal( + TextOperation.MAX_STRING_LENGTH + ) + }) + }) + + describe('getStringLengthOfFile', function () { + it('counts the characters of an editable file', async function () { + const content = Buffer.from('héllᚠ') + const pathname = await writeFile(content) + expect( + await getStringLengthOfFile(content.byteLength, pathname) + ).to.equal(5) + }) + + it('rejects a file that is not editable text', async function () { + const content = Buffer.from([0x61, 0x00, 0x62]) + const pathname = await writeFile(content) + expect(await getStringLengthOfFile(content.byteLength, pathname)).to.be + .null + }) + + it('does not read a file too large to be editable', async function () { + // A pathname that cannot be read at all: reaching the read would throw. + const result = await getStringLengthOfFile( + Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND + 1, + path.join(tmpDir, 'does-not-exist') + ) + expect(result).to.be.null + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/change_identity.test.js b/libraries/overleaf-editor-core/test/unit/change_identity.test.js new file mode 100644 index 0000000000..0adc6578fd --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/change_identity.test.js @@ -0,0 +1,162 @@ +'use strict' + +const { expect } = require('chai') +const { + editorChangeIdentity, + editorChangeIdentityOf, + isSameEditorChange, + EDITOR_ORIGIN_KIND, +} = require('../..') + +const EDITOR_A = '3f2504e0-4f89-41d3-9a0c-0305e82c3301' +const EDITOR_B = '9c858901-8a57-4791-81fe-4c455b099bc9' +const USER_A = '65b9d7fb2a1b2c3d4e5f6a7b' +const USER_B = '65b9d7fb2a1b2c3d4e5f6a7c' +const TIMESTAMP = '2025-01-02T03:04:05.678Z' + +describe('editor change identity', function () { + function raw(overrides = {}) { + return { + operations: [], + timestamp: overrides.timestamp || TIMESTAMP, + v2Authors: 'v2Authors' in overrides ? overrides.v2Authors : [USER_A], + origin: + 'origin' in overrides + ? overrides.origin + : { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_A }, + } + } + + function same(a, b) { + return isSameEditorChange(editorChangeIdentity(a), editorChangeIdentity(b)) + } + + it('recognises the same change', function () { + expect(same(raw(), raw())).to.be.true + }) + + describe('what it refuses to match', function () { + // Each of these is a false match waiting to happen, and a false match means a + // change is treated as a duplicate and silently discarded. + + it('a different editor sharing the author and timestamp', function () { + // The case editorId exists for: two tabs of one user, or two anonymous + // editors, can produce changes on the same millisecond. + expect( + same( + raw(), + raw({ origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_B } }) + ) + ).to.be.false + }) + + it('a different author sharing the editorId', function () { + // editorId comes from the client, so a forged one must not let one account + // suppress another's change. + expect(same(raw(), raw({ v2Authors: [USER_B] }))).to.be.false + }) + + it('a different timestamp', function () { + expect(same(raw(), raw({ timestamp: '2025-01-02T03:04:05.679Z' }))).to.be + .false + }) + + it('a change from another writer', function () { + expect(same(raw(), raw({ origin: { kind: 'dropbox' } }))).to.be.false + }) + + it('a change with no origin at all', function () { + expect(same(raw(), raw({ origin: undefined }))).to.be.false + }) + + it('a change whose editorId was stripped when its chunk was written', function () { + expect(same(raw(), raw({ origin: { kind: EDITOR_ORIGIN_KIND } }))).to.be + .false + }) + + it('a change with more than one author', function () { + // real-time stamps exactly one, so anything else did not come from there. + expect(same(raw(), raw({ v2Authors: [USER_A, USER_B] }))).to.be.false + }) + + it('a change with no authors', function () { + expect(same(raw(), raw({ v2Authors: [] }))).to.be.false + }) + }) + + describe('anonymous authors', function () { + it('matches when both are anonymous from the same editor', function () { + expect(same(raw({ v2Authors: [null] }), raw({ v2Authors: [null] }))).to.be + .true + }) + + it('still separates two anonymous editors', function () { + // They share an empty author, so the editorId is the only thing keeping + // their changes apart. + expect( + same( + raw({ v2Authors: [null] }), + raw({ + v2Authors: [null], + origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_B }, + }) + ) + ).to.be.false + }) + + it('does not match an anonymous change against a signed-in one', function () { + expect(same(raw({ v2Authors: [null] }), raw())).to.be.false + }) + }) + + describe('timestamps', function () { + it('compares the instant rather than the spelling', function () { + // The two sides serialise independently, so equal instants must match even + // if the strings differ. + expect( + same( + raw({ timestamp: '2025-01-02T03:04:05.678Z' }), + raw({ + timestamp: '2025-01-02T04:04:05.678+01:00', + }) + ) + ).to.be.true + }) + + it('refuses an unparseable timestamp', function () { + expect(editorChangeIdentity(raw({ timestamp: 'not a date' }))).to.be.null + }) + }) + + describe('editorChangeIdentityOf', function () { + it('matches a raw change describing the same submission', function () { + const mine = editorChangeIdentityOf({ + editorId: EDITOR_A, + author: USER_A, + timestamp: new Date(TIMESTAMP), + }) + + expect(isSameEditorChange(mine, editorChangeIdentity(raw()))).to.be.true + }) + + it('treats a missing author as anonymous', function () { + const mine = editorChangeIdentityOf({ + editorId: EDITOR_A, + author: null, + timestamp: new Date(TIMESTAMP), + }) + + expect( + isSameEditorChange( + mine, + editorChangeIdentity(raw({ v2Authors: [null] })) + ) + ).to.be.true + }) + }) + + it('never matches a null identity, including against another null', function () { + expect(isSameEditorChange(null, null)).to.be.false + expect(isSameEditorChange(editorChangeIdentity(raw()), null)).to.be.false + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/editor_origin.test.js b/libraries/overleaf-editor-core/test/unit/editor_origin.test.js new file mode 100644 index 0000000000..7dbfb0ab5a --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/editor_origin.test.js @@ -0,0 +1,102 @@ +'use strict' + +const { expect } = require('chai') +const { Change, EditorOrigin, Origin, EDITOR_ORIGIN_KIND } = require('../..') +const schemas = require('../../lib/schemas') + +describe('EditorOrigin', function () { + const EDITOR_ID = '3f2504e0-4f89-41d3-9a0c-0305e82c3301' + + it('round trips through raw form', function () { + const origin = new EditorOrigin(EDITOR_ID) + expect(origin.toRaw()).to.deep.equal({ + kind: EDITOR_ORIGIN_KIND, + editorId: EDITOR_ID, + }) + + const parsed = Origin.fromRaw(origin.toRaw()) + expect(parsed).to.be.an.instanceof(EditorOrigin) + expect(parsed.getEditorId()).to.equal(EDITOR_ID) + expect(parsed.getKind()).to.equal(EDITOR_ORIGIN_KIND) + }) + + it('rejects a missing editorId', function () { + expect(() => new EditorOrigin()).to.throw() + expect(() => new EditorOrigin('')).to.throw() + }) + + describe('once the editorId has been stripped', function () { + // Writing a chunk drops the editorId from all but an editor's latest change, + // leaving a bare {kind: 'editor'}. Reading one of those back has to keep + // working, so it must not be treated as an EditorOrigin. + it('parses as a plain Origin rather than throwing', function () { + const parsed = Origin.fromRaw({ kind: EDITOR_ORIGIN_KIND }) + + expect(parsed).to.be.an.instanceof(Origin) + expect(parsed).to.not.be.an.instanceof(EditorOrigin) + expect(parsed.getKind()).to.equal(EDITOR_ORIGIN_KIND) + expect(parsed.toRaw()).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) + }) + + it('survives a Change round trip', function () { + const change = Change.fromRaw({ + operations: [], + timestamp: '2025-01-02T03:04:05.678Z', + origin: { kind: EDITOR_ORIGIN_KIND }, + }) + + expect(change.getOrigin().getKind()).to.equal(EDITOR_ORIGIN_KIND) + expect(change.toRaw().origin).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) + }) + }) + + it('is carried through a Change round trip while it has an id', function () { + const change = Change.fromRaw({ + operations: [], + timestamp: '2025-01-02T03:04:05.678Z', + origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_ID }, + }) + + expect(change.getOrigin()).to.be.an.instanceof(EditorOrigin) + expect(change.getOrigin().getEditorId()).to.equal(EDITOR_ID) + expect(change.toRaw().origin).to.deep.equal({ + kind: EDITOR_ORIGIN_KIND, + editorId: EDITOR_ID, + }) + }) + + describe('rawOrigin schema', function () { + it('accepts an editor origin with a uuid', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + editorId: EDITOR_ID, + }).success + ).to.be.true + }) + + it('accepts the stripped form', function () { + expect(schemas.rawOrigin.safeParse({ kind: EDITOR_ORIGIN_KIND }).success) + .to.be.true + }) + + it('rejects an editorId that is not a uuid', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + editorId: 'not-a-uuid', + }).success + ).to.be.false + }) + + it('rejects unknown fields alongside the editorId', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + editorId: EDITOR_ID, + somethingElse: 1, + }).success + ).to.be.false + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/file_tree_diff.test.js b/libraries/overleaf-editor-core/test/unit/file_tree_diff.test.js new file mode 100644 index 0000000000..d6fb7ce7ea --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/file_tree_diff.test.js @@ -0,0 +1,456 @@ +// @ts-check +'use strict' + +const { expect } = require('chai') +const sinon = require('sinon') + +const { buildFileTreeDiff } = require('../../lib/file_tree_diff') +const Change = require('../../lib/change') +const File = require('../../lib/file') +const Operation = require('../../lib/operation') +const TextOperation = require('../../lib/operation/text_operation') + +/** + * @import { FileTreeDiffEntry } from '../../lib/file_tree_diff' + */ + +const TS = new Date('2026-07-10T00:00:00.000Z') + +/** @param {Operation[]} operations */ +const change = (...operations) => new Change(operations, TS) + +const addFile = (pathname, content = 'content') => + Operation.addFile(pathname, File.fromString(content)) +const editFile = pathname => + Operation.editFile(pathname, new TextOperation().insert('x')) +const moveFile = (pathname, newPathname) => + Operation.moveFile(pathname, newPathname) +const removeFile = pathname => Operation.removeFile(pathname) + +/** + * The shape the test cases below are written in. + * + * @param {string} pathname + * @param {Object} [overrides] + */ +function summary(pathname, overrides = {}) { + return { + pathname, + origin: pathname, + chain: [pathname], + edited: false, + firstEditedAtChainIndex: null, + hasFile: false, + deletedAtChangeIndex: null, + ...overrides, + } +} + +/** + * @param {FileTreeDiffEntry} entry + */ +function summarize(entry) { + return { + pathname: entry.chain[entry.chain.length - 1], + origin: entry.origin, + chain: entry.chain, + edited: entry.edited, + firstEditedAtChainIndex: entry.firstEditedAtChainIndex, + hasFile: entry.file != null, + deletedAtChangeIndex: entry.deletedAtChangeIndex, + } +} + +/** + * @param {Map} entries + */ +function summarizeEntries(entries) { + return Array.from(entries, ([pathname, entry]) => { + expect(pathname).to.equal(entry.chain[entry.chain.length - 1]) + return summarize(entry) + }) +} + +describe('buildFileTreeDiff', function () { + const testCases = [ + { + name: 'reports the files that the window leaves untouched', + initialPathnames: ['a.tex', 'b.tex'], + changes: [], + entries: [summary('a.tex'), summary('b.tex')], + removed: [], + }, + { + name: 'collapses a chain of moves into a single entry', + initialPathnames: ['a.tex'], + changes: [ + change(moveFile('a.tex', 'b.tex')), + change(moveFile('b.tex', 'c.tex')), + ], + entries: [ + summary('c.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex', 'c.tex'], + }), + ], + removed: [], + }, + { + name: 'records the pathnames of a file that is moved and then removed', + initialPathnames: ['a.tex'], + changes: [ + change(moveFile('a.tex', 'b.tex')), + change(removeFile('b.tex')), + ], + entries: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + deletedAtChangeIndex: 1, + }), + ], + removed: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + deletedAtChangeIndex: 1, + }), + ], + }, + { + name: 'reports a file that is added and then moved at its new pathname', + initialPathnames: [], + changes: [change(addFile('a.tex')), change(moveFile('a.tex', 'b.tex'))], + entries: [ + summary('b.tex', { + origin: null, + chain: ['a.tex', 'b.tex'], + hasFile: true, + }), + ], + removed: [], + }, + { + name: 'reports a file that is added and then removed as removed', + initialPathnames: [], + changes: [change(addFile('a.tex')), change(removeFile('a.tex'))], + entries: [ + summary('a.tex', { + origin: null, + hasFile: true, + deletedAtChangeIndex: 1, + }), + ], + removed: [ + summary('a.tex', { + origin: null, + hasFile: true, + deletedAtChangeIndex: 1, + }), + ], + }, + { + name: 'reports a file that is removed and then added again as added', + initialPathnames: ['a.tex'], + changes: [change(removeFile('a.tex')), change(addFile('a.tex'))], + entries: [summary('a.tex', { origin: null, hasFile: true })], + removed: [summary('a.tex', { deletedAtChangeIndex: 0 })], + }, + { + name: 'reports a file that another file is moved onto as removed', + initialPathnames: ['a.tex', 'b.tex'], + changes: [change(moveFile('a.tex', 'b.tex'))], + entries: [ + summary('b.tex', { origin: 'a.tex', chain: ['a.tex', 'b.tex'] }), + ], + removed: [summary('b.tex', { deletedAtChangeIndex: 0 })], + }, + { + name: 'reports a file that another file is added over as removed', + initialPathnames: ['a.tex'], + changes: [change(addFile('a.tex'))], + entries: [summary('a.tex', { origin: null, hasFile: true })], + removed: [summary('a.tex', { deletedAtChangeIndex: 0 })], + }, + { + name: 'reports a file that is moved onto a pathname removed earlier in the window', + initialPathnames: ['a.tex', 'b.tex'], + changes: [ + change(removeFile('a.tex')), + change(moveFile('b.tex', 'a.tex')), + ], + entries: [ + summary('a.tex', { origin: 'b.tex', chain: ['b.tex', 'a.tex'] }), + ], + removed: [summary('a.tex', { deletedAtChangeIndex: 0 })], + }, + { + name: 'records an edit of a file that is moved afterwards', + initialPathnames: ['a.tex'], + changes: [change(editFile('a.tex')), change(moveFile('a.tex', 'b.tex'))], + entries: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + edited: true, + firstEditedAtChainIndex: 0, + }), + ], + removed: [], + }, + { + name: 'records an edit of a file that was moved before', + initialPathnames: ['a.tex'], + changes: [change(moveFile('a.tex', 'b.tex')), change(editFile('b.tex'))], + entries: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + edited: true, + firstEditedAtChainIndex: 1, + }), + ], + removed: [], + }, + { + name: 'records the first of several edits', + initialPathnames: ['a.tex'], + changes: [ + change(editFile('a.tex')), + change(moveFile('a.tex', 'b.tex')), + change(editFile('b.tex')), + ], + entries: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + edited: true, + firstEditedAtChainIndex: 0, + }), + ], + removed: [], + }, + { + name: 'creates an entry for an edit of a pathname it has not seen', + initialPathnames: [], + changes: [change(editFile('a.tex'))], + entries: [summary('a.tex', { edited: true, firstEditedAtChainIndex: 0 })], + removed: [], + }, + { + name: 'ignores an edit of a removed file', + initialPathnames: ['a.tex'], + changes: [change(removeFile('a.tex')), change(editFile('a.tex'))], + entries: [summary('a.tex', { deletedAtChangeIndex: 0 })], + removed: [summary('a.tex', { deletedAtChangeIndex: 0 })], + }, + { + name: 'resolves a swap of two files by their pathnames', + initialPathnames: ['a.tex', 'b.tex'], + changes: [ + change(moveFile('a.tex', 'tmp.tex')), + change(moveFile('b.tex', 'a.tex')), + change(moveFile('tmp.tex', 'b.tex')), + ], + entries: [ + summary('a.tex', { origin: 'b.tex', chain: ['b.tex', 'a.tex'] }), + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'tmp.tex', 'b.tex'], + }), + ], + removed: [], + }, + { + name: 'skips a move of a pathname that holds no file', + initialPathnames: ['a.tex'], + changes: [change(moveFile('missing.tex', 'b.tex'))], + entries: [summary('a.tex')], + removed: [], + }, + { + name: 'skips a removal of a pathname that holds no file', + initialPathnames: ['a.tex'], + changes: [change(removeFile('missing.tex'))], + entries: [summary('a.tex')], + removed: [], + }, + { + name: 'skips operations without a pathname', + initialPathnames: [], + changes: [ + change( + editFile(''), + moveFile('', 'a.tex'), + removeFile(''), + addFile('') + ), + ], + entries: [], + removed: [], + }, + { + name: 'ignores operations that leave the file tree unchanged', + initialPathnames: ['a.tex'], + changes: [ + change( + Operation.setFileMetadata('a.tex', { main: true }), + moveFile('a.tex', 'a.tex'), + Operation.NO_OP + ), + ], + entries: [summary('a.tex')], + removed: [], + }, + { + name: 'folds the operations of a single change in order', + initialPathnames: ['a.tex'], + changes: [ + change(moveFile('a.tex', 'b.tex'), addFile('a.tex'), editFile('b.tex')), + ], + entries: [ + summary('b.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex'], + edited: true, + firstEditedAtChainIndex: 1, + }), + summary('a.tex', { origin: null, hasFile: true }), + ], + removed: [], + }, + { + name: 'reports the index of the change that removed a file', + initialPathnames: ['a.tex', 'b.tex'], + changes: [ + change(editFile('a.tex')), + change(removeFile('b.tex')), + change(removeFile('a.tex')), + ], + entries: [ + summary('a.tex', { + edited: true, + firstEditedAtChainIndex: 0, + deletedAtChangeIndex: 2, + }), + summary('b.tex', { deletedAtChangeIndex: 1 }), + ], + removed: [ + summary('b.tex', { deletedAtChangeIndex: 1 }), + summary('a.tex', { + edited: true, + firstEditedAtChainIndex: 0, + deletedAtChangeIndex: 2, + }), + ], + }, + ] + + for (const testCase of testCases) { + it(testCase.name, function () { + const { entries, removed } = buildFileTreeDiff(testCase.changes, { + initialPathnames: testCase.initialPathnames, + }) + expect(summarizeEntries(entries)).to.deep.equal(testCase.entries) + expect(removed.map(summarize)).to.deep.equal(testCase.removed) + }) + } + + it('keeps the file of the most recent add', function () { + const file = File.fromString('final') + const { entries } = buildFileTreeDiff( + [ + change(addFile('a.tex', 'first')), + change(Operation.addFile('a.tex', file)), + change(moveFile('a.tex', 'b.tex')), + ], + { initialPathnames: [] } + ) + expect(entries.get('b.tex')?.file).to.equal(file) + }) + + describe('without initialPathnames', function () { + it('assumes that a moved file existed before the window', function () { + const { entries, removed } = buildFileTreeDiff([ + change(moveFile('a.tex', 'b.tex')), + change(moveFile('b.tex', 'c.tex')), + ]) + expect(summarizeEntries(entries)).to.deep.equal([ + summary('c.tex', { + origin: 'a.tex', + chain: ['a.tex', 'b.tex', 'c.tex'], + }), + ]) + expect(removed).to.deep.equal([]) + }) + + it('assumes that a removed file existed before the window', function () { + const { entries, removed } = buildFileTreeDiff([ + change(removeFile('a.tex')), + ]) + expect(summarizeEntries(entries)).to.deep.equal([ + summary('a.tex', { deletedAtChangeIndex: 0 }), + ]) + expect(removed.map(summarize)).to.deep.equal([ + summary('a.tex', { deletedAtChangeIndex: 0 }), + ]) + }) + + it('has no file to report as removed at the target of a move', function () { + const { entries, removed } = buildFileTreeDiff([ + change(moveFile('a.tex', 'b.tex')), + ]) + expect(summarizeEntries(entries)).to.deep.equal([ + summary('b.tex', { origin: 'a.tex', chain: ['a.tex', 'b.tex'] }), + ]) + expect(removed.map(summarize)).to.deep.equal([]) + }) + }) + + describe('onMoveCollision', function () { + it('is called with the file at the target pathname before it is replaced', function () { + const calls = [] + const operation = moveFile('a.tex', 'b.tex') + const { entries } = buildFileTreeDiff([change(operation)], { + initialPathnames: ['a.tex', 'b.tex'], + onMoveCollision: (entry, movedBy) => { + calls.push({ entry: summarize(entry), movedBy }) + }, + }) + expect(calls).to.deep.equal([ + { entry: summary('b.tex'), movedBy: operation }, + ]) + expect(Array.from(entries.keys())).to.deep.equal(['b.tex']) + }) + + it('is not called when the target pathname holds no file', function () { + const onMoveCollision = sinon.stub() + buildFileTreeDiff([change(moveFile('a.tex', 'b.tex'))], { + initialPathnames: ['a.tex'], + onMoveCollision, + }) + expect(onMoveCollision.called).to.be.false + }) + + it('is not called when the file at the target pathname was removed', function () { + const onMoveCollision = sinon.stub() + buildFileTreeDiff( + [change(removeFile('b.tex')), change(moveFile('a.tex', 'b.tex'))], + { initialPathnames: ['a.tex', 'b.tex'], onMoveCollision } + ) + expect(onMoveCollision.called).to.be.false + }) + + it('can throw before the move is folded in', function () { + const err = new Error('collision') + expect(() => + buildFileTreeDiff([change(moveFile('a.tex', 'b.tex'))], { + initialPathnames: ['a.tex', 'b.tex'], + onMoveCollision: () => { + throw err + }, + }) + ).to.throw(err) + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/rebase.test.js b/libraries/overleaf-editor-core/test/unit/rebase.test.js new file mode 100644 index 0000000000..079bfd87a8 --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/rebase.test.js @@ -0,0 +1,174 @@ +'use strict' + +const { expect } = require('chai') +const { + Change, + File, + Operation, + Origin, + TextOperation, + rebaseChanges, + EDITOR_ORIGIN_KIND, +} = require('../..') + +describe('rebaseChanges', function () { + const TIMESTAMP = new Date('2025-01-02T03:04:05.678Z') + + function change(operations, overrides = {}) { + return new Change( + operations, + overrides.timestamp || TIMESTAMP, + overrides.authors, + overrides.origin, + overrides.v2Authors + ) + } + + function addFile(pathname, content = 'content') { + return Operation.addFile(pathname, File.fromString(content)) + } + + it('leaves operations alone when nothing conflicts', function () { + const ours = [change([addFile('ours.tex')])] + + const rebased = rebaseChanges(ours, [change([addFile('theirs.tex')])]) + + expect(rebased).to.have.length(1) + expect(rebased[0].getOperations()).to.have.length(1) + expect(rebased[0].getOperations()[0].getPathname()).to.equal('ours.tex') + }) + + it('drops a change whose only operation transforms away', function () { + // Two clients add the same pathname. transformAddFileAddFile resolves this + // in favour of the change that landed first, so ours becomes a no-op. + const ours = [change([addFile('main.tex', 'ours')])] + + const rebased = rebaseChanges(ours, [ + change([addFile('main.tex', 'theirs')]), + ]) + + expect(rebased).to.deep.equal([]) + }) + + it('prunes no-op operations but keeps the change when others survive', function () { + const ours = [change([addFile('main.tex'), addFile('other.tex')])] + + const rebased = rebaseChanges(ours, [change([addFile('main.tex')])]) + + expect(rebased).to.have.length(1) + const operations = rebased[0].getOperations() + expect(operations).to.have.length(1) + expect(operations[0].getPathname()).to.equal('other.tex') + }) + + it('drops only the changes that empty out, keeping the rest in order', function () { + const ours = [ + change([addFile('a.tex')]), + change([addFile('collides.tex')]), + change([addFile('b.tex')]), + ] + + const rebased = rebaseChanges(ours, [change([addFile('collides.tex')])]) + + expect(rebased.map(c => c.getOperations()[0].getPathname())).to.deep.equal([ + 'a.tex', + 'b.tex', + ]) + }) + + it('returns an empty array when every change transforms away', function () { + const ours = [change([addFile('a.tex')]), change([addFile('b.tex')])] + + const rebased = rebaseChanges(ours, [ + change([addFile('a.tex')]), + change([addFile('b.tex')]), + ]) + + expect(rebased).to.deep.equal([]) + }) + + it('transforms against each intervening change in turn', function () { + // A rename chain: they moved a.tex -> b.tex, then b.tex -> c.tex. Our edit + // of a.tex has to end up addressing c.tex, which only happens if the second + // intervening change is applied to the already-transformed operation. + const ours = [ + change([ + Operation.editFile( + 'a.tex', + TextOperation.fromJSON({ textOperation: ['hello'] }) + ), + ]), + ] + + const rebased = rebaseChanges(ours, [ + change([Operation.moveFile('a.tex', 'b.tex')]), + change([Operation.moveFile('b.tex', 'c.tex')]), + ]) + + expect(rebased).to.have.length(1) + expect(rebased[0].getOperations()[0].getPathname()).to.equal('c.tex') + }) + + it('transforms a sequence of our changes against one of theirs', function () { + // Both of our changes address a.tex, which they renamed. Each of ours has + // to be transformed, not just the first. + const ours = [ + change([ + Operation.editFile( + 'a.tex', + TextOperation.fromJSON({ textOperation: ['one'] }) + ), + ]), + change([ + Operation.editFile( + 'a.tex', + TextOperation.fromJSON({ textOperation: [3, 'two'] }) + ), + ]), + ] + + const rebased = rebaseChanges(ours, [ + change([Operation.moveFile('a.tex', 'renamed.tex')]), + ]) + + expect(rebased).to.have.length(2) + expect(rebased.map(c => c.getOperations()[0].getPathname())).to.deep.equal([ + 'renamed.tex', + 'renamed.tex', + ]) + }) + + it('preserves the fields a resend is recognised by', function () { + // history-v1 identifies its own change read back from history by origin + // kind, author and timestamp. A rebase must not disturb any of them, or + // deduplicating a resend stops working. + const authorId = '65b9d7fb2a1b2c3d4e5f6a7b' + const ours = [ + change([addFile('main.tex'), addFile('survivor.tex')], { + origin: new Origin(EDITOR_ORIGIN_KIND), + v2Authors: [authorId], + }), + ] + + const rebased = rebaseChanges(ours, [change([addFile('main.tex')])]) + + expect(rebased).to.have.length(1) + const raw = rebased[0].toRaw() + expect(raw.origin).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) + expect(raw.v2Authors).to.deep.equal([authorId]) + expect(raw.timestamp).to.equal(TIMESTAMP.toISOString()) + }) + + it('leaves ours untouched when theirs is empty', function () { + const ours = [change([addFile('main.tex')])] + + const rebased = rebaseChanges(ours, []) + + expect(rebased).to.have.length(1) + expect(rebased[0].getOperations()[0].getPathname()).to.equal('main.tex') + }) + + it('returns an empty array when ours is empty', function () { + expect(rebaseChanges([], [change([addFile('main.tex')])])).to.deep.equal([]) + }) +}) diff --git a/services/history-v1/storage/lib/blob_store/index.js b/services/history-v1/storage/lib/blob_store/index.js index dbbf329a01..759a20584f 100644 --- a/services/history-v1/storage/lib/blob_store/index.js +++ b/services/history-v1/storage/lib/blob_store/index.js @@ -2,15 +2,15 @@ const config = require('config') const fs = require('node:fs') -const isValidUtf8 = require('utf-8-validate') const { ReadableString } = require('@overleaf/stream-utils') const core = require('overleaf-editor-core') +const { + getStringLengthOfFile, +} = require('overleaf-editor-core/lib/blob_string_length') const objectPersistor = require('@overleaf/object-persistor') const OError = require('@overleaf/o-error') const Blob = core.Blob -const TextOperation = core.TextOperation -const containsNonBmpChars = core.util.containsNonBmpChars const assert = require('../assert') const blobHash = require('../blob_hash') @@ -108,23 +108,6 @@ async function makeBlobForFile(pathname) { return new Blob(hash, byteLength) } -async function getStringLengthOfFile(byteLength, pathname) { - // We have to read the file into memory to get its UTF-8 length, so don't - // bother for files that are too large for us to edit anyway. - if (byteLength > Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND) { - return null - } - - // We need to check if the file contains nonBmp or null characters - let data = await fs.promises.readFile(pathname) - if (!isValidUtf8(data)) return null - data = data.toString() - if (data.length > TextOperation.MAX_STRING_LENGTH) return null - if (containsNonBmpChars(data)) return null - if (data.indexOf('\x00') !== -1) return null - return data.length -} - async function deleteBlobsInBucket(projectId) { const bucket = config.get('blobStore.projectBucket') const prefix = `${projectKey.format(projectId)}/` diff --git a/services/history-v1/storage/scripts/recover_zip.js b/services/history-v1/storage/scripts/recover_zip.js index 6a0dcc8dcd..56d7613f17 100644 --- a/services/history-v1/storage/scripts/recover_zip.js +++ b/services/history-v1/storage/scripts/recover_zip.js @@ -27,7 +27,6 @@ const config = require('config') // We depend on this via object-persistor. // eslint-disable-next-line import/no-extraneous-dependencies const { Storage } = require('@google-cloud/storage') -const isValidUtf8 = require('utf-8-validate') // zip-stream@7 uses ESM default export const ZipStream = require('zip-stream').default @@ -43,6 +42,9 @@ function createStorage() { } const core = require('overleaf-editor-core') +const { + getStringLengthOfFile, +} = require('overleaf-editor-core/lib/blob_string_length') const projectKey = require('@overleaf/object-persistor/src/ProjectKey.js') const streams = require('../lib/streams') @@ -112,25 +114,6 @@ async function loadChunk(historyPathname, blobStore) { return new core.Chunk(history, 0) } -// TODO: it would be nice to export / expose this from BlobStore; -// currently this is a copy of the method there. -async function getStringLengthOfFile(byteLength, pathname) { - // We have to read the file into memory to get its UTF-8 length, so don't - // bother for files that are too large for us to edit anyway. - if (byteLength > core.Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND) { - return null - } - - // We need to check if the file contains nonBmp or null characters - let data = await fs.promises.readFile(pathname) - if (!isValidUtf8(data)) return null - data = data.toString() - if (data.length > core.TextOperation.MAX_STRING_LENGTH) return null - if (core.util.containsNonBmpChars(data)) return null - if (data.indexOf('\x00') !== -1) return null - return data.length -} - class RecoveryBlobStore { constructor(historyId, tmp) { this.historyId = historyId diff --git a/services/project-history/app/js/FileTreeDiffGenerator.js b/services/project-history/app/js/FileTreeDiffGenerator.js index 16696c686c..f6fbfea495 100644 --- a/services/project-history/app/js/FileTreeDiffGenerator.js +++ b/services/project-history/app/js/FileTreeDiffGenerator.js @@ -1,129 +1,102 @@ -/* eslint-disable - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import Core from 'overleaf-editor-core' -import logger from '@overleaf/logger' +import { buildFileTreeDiff } from 'overleaf-editor-core/lib/file_tree_diff.js' import * as Errors from './Errors.js' -const { MoveFileOperation, AddFileOperation, EditFileOperation } = Core +/** + * @import { FileTreeDiffEntry } from 'overleaf-editor-core/lib/file_tree_diff' + * @import { Snapshot } from 'overleaf-editor-core' + */ -export function buildDiff(chunk, fromVersion, toVersion) { - chunk = Core.Chunk.fromRaw(chunk.chunk) +/** + * One file of the diff. Files without an operation are unchanged. + * + * @typedef {object} FileTreeDiffFile + * @property {string} pathname the pathname of the file at fromVersion, or the + * pathname it was added at + * @property {string} [newPathname] the pathname of a renamed file at toVersion + * @property {'added' | 'edited' | 'renamed' | 'removed'} [operation] + * @property {number} [deletedAtV] version a removed file was removed at + * @property {boolean | null} [editable] null when it is not known + */ + +/** + * Describe how the file tree changed between two versions of a chunk. + * + * @param {object} rawChunk chunk covering fromVersion to toVersion + * @param {number} fromVersion + * @param {number} toVersion + * @return {FileTreeDiffFile[]} one entry per file + */ +export function buildDiff(rawChunk, fromVersion, toVersion) { + const chunk = Core.Chunk.fromRaw(rawChunk.chunk) const chunkStartVersion = chunk.getStartVersion() + const changes = chunk.getChanges() - const diff = _getInitialDiffSnapshot(chunk, fromVersion) + // The chunk starts at chunkStartVersion, so bring its snapshot forward to the + // file tree at fromVersion, which the diff is reported against. + const initialSnapshot = chunk.getSnapshot() + initialSnapshot.applyAll(changes.slice(0, fromVersion - chunkStartVersion)) - const changes = chunk - .getChanges() - .slice(fromVersion - chunkStartVersion, toVersion - chunkStartVersion) - for (let i = 0; i < changes.length; i++) { - const change = changes[i] - for (const operation of Array.from(change.getOperations())) { - if (operation.pathname === null || operation.pathname === '') { - // skip operations for missing files - logger.warn({ diff, operation }, 'invalid pathname in operation') - } else if (operation instanceof EditFileOperation) { - _applyEditFileToDiff(diff, operation) - } else if (operation instanceof AddFileOperation) { - _applyAddFileToDiff(diff, operation) - } else if (operation instanceof MoveFileOperation) { - if (operation.isRemoveFile()) { - const deletedAtV = fromVersion + i - _applyDeleteFileToDiff(diff, operation, deletedAtV) - } else { - _applyMoveFileToDiff(diff, operation) - } - } + const { entries } = buildFileTreeDiff( + changes.slice( + fromVersion - chunkStartVersion, + toVersion - chunkStartVersion + ), + { + initialPathnames: initialSnapshot.getFilePathnames(), + onMoveCollision: (entry, operation) => { + throw new Errors.InconsistentChunkError( + 'trying to move to file that already exists', + { + pathname: operation.getPathname(), + newPathname: operation.getNewPathname(), + } + ) + }, } - } + ) - return Object.values(diff) + return Array.from(entries.values(), entry => + _buildDiffEntry(entry, initialSnapshot, fromVersion) + ) } -function _getInitialDiffSnapshot(chunk, fromVersion) { - // Start with a 'diff' which is snapshot of the filetree at the beginning, - // with nothing in the diff marked as changed. - // Use a bare object to protect against reserved names. - const diff = Object.create(null) - const files = _getInitialFiles(chunk, fromVersion) - for (const [pathname, file] of Object.entries(files)) { - diff[pathname] = { pathname, editable: file.isEditable() } - } - return diff -} +/** + * @param {FileTreeDiffEntry} entry + * @param {Snapshot} initialSnapshot file tree at fromVersion + * @param {number} fromVersion + * @return {FileTreeDiffFile} + */ +function _buildDiffEntry(entry, initialSnapshot, fromVersion) { + const { origin } = entry + const pathname = entry.chain[entry.chain.length - 1] + const renamed = origin != null && pathname !== origin -function _getInitialFiles(chunk, fromVersion) { - const snapshot = chunk.getSnapshot() - const changes = chunk - .getChanges() - .slice(0, fromVersion - chunk.getStartVersion()) - snapshot.applyAll(changes) - return snapshot.fileMap.files -} + /** @type {FileTreeDiffFile} */ + const diffEntry = { pathname: origin ?? pathname } + if (renamed) { + diffEntry.newPathname = pathname + } + if (entry.deletedAtChangeIndex != null) { + diffEntry.operation = 'removed' + diffEntry.deletedAtV = fromVersion + entry.deletedAtChangeIndex + } else if (origin == null) { + diffEntry.operation = 'added' + } else if (renamed) { + diffEntry.operation = 'renamed' + } else if (entry.edited) { + diffEntry.operation = 'edited' + } -function _applyAddFileToDiff(diff, operation) { - return (diff[operation.pathname] = { - pathname: operation.pathname, - operation: 'added', - editable: operation.file.isEditable(), - }) -} + // Report the editability of the file, so that the frontend knows whether it + // can display a diff for it. It is left out for files that were edited before + // anything else happened to them: an edit implies that the file is editable. + const editedFirst = entry.file == null && entry.firstEditedAtChainIndex === 0 + const file = + entry.file ?? (origin != null ? initialSnapshot.getFile(origin) : null) + if (file != null && !editedFirst) { + diffEntry.editable = file.isEditable() + } -function _applyEditFileToDiff(diff, operation) { - const change = diff[operation.pathname] - if ((change != null ? change.operation : undefined) == null) { - // avoid exception for non-existent change - return (diff[operation.pathname] = { - pathname: operation.pathname, - operation: 'edited', - }) - } -} - -function _applyMoveFileToDiff(diff, operation) { - if ( - diff[operation.newPathname] != null && - diff[operation.newPathname].operation !== 'removed' - ) { - const err = new Errors.InconsistentChunkError( - 'trying to move to file that already exists', - { diff, operation } - ) - throw err - } - const change = diff[operation.pathname] - if (change == null) { - logger.warn({ diff, operation }, 'tried to rename non-existent file') - return - } - change.newPathname = operation.newPathname - if (change.operation === 'added') { - // If this file was added this time, just leave it as an add, but - // at the new name. - change.pathname = operation.newPathname - delete change.newPathname - } else { - change.operation = 'renamed' - } - diff[operation.newPathname] = change - return delete diff[operation.pathname] -} - -function _applyDeleteFileToDiff(diff, operation, deletedAtV) { - // avoid exception for non-existent change - if (diff[operation.pathname] != null) { - diff[operation.pathname].operation = 'removed' - } - return diff[operation.pathname] != null - ? (diff[operation.pathname].deletedAtV = deletedAtV) - : undefined + return diffEntry } diff --git a/services/project-history/test/acceptance/js/SyncTests.js b/services/project-history/test/acceptance/js/SyncTests.js index 9dd1d87ee8..3761b9c488 100644 --- a/services/project-history/test/acceptance/js/SyncTests.js +++ b/services/project-history/test/acceptance/js/SyncTests.js @@ -18,7 +18,7 @@ import { fetchNothing, RequestFailedError } from '@overleaf/fetch-utils' import { _getBlobHashFromString } from '../../../app/js/HashManager.js' import { db, ObjectId } from '../../../app/js/mongodb.js' -const EMPTY_FILE_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' +const EMPTY_FILE_HASH = File.EMPTY_FILE_HASH const MockHistoryStore = () => nock('http://127.0.0.1:3100') const MockFileStore = () => nock('http://127.0.0.1:3009') diff --git a/services/project-history/test/unit/js/FileTreeDiffGenerator/FileTreeDiffGeneratorTests.js b/services/project-history/test/unit/js/FileTreeDiffGenerator/FileTreeDiffGeneratorTests.js new file mode 100644 index 0000000000..a66b0b93e0 --- /dev/null +++ b/services/project-history/test/unit/js/FileTreeDiffGenerator/FileTreeDiffGeneratorTests.js @@ -0,0 +1,446 @@ +import crypto from 'node:crypto' +import { expect } from 'chai' +import * as FileTreeDiffGenerator from '../../../../app/js/FileTreeDiffGenerator.js' +import * as Errors from '../../../../app/js/Errors.js' + +const sha = data => crypto.createHash('sha1').update(data).digest('hex') + +/** An editable file: the string length is known. */ +const textFile = (content = 'mock-content') => ({ + hash: sha(content), + stringLength: 42, +}) + +/** A binary file: it has a byte length, but no string length. */ +const binaryFile = (content = 'mock-binary') => ({ + hash: sha(content), + byteLength: 42, +}) + +/** A file with an unknown editability: neither length is known. */ +const hashOnlyFile = (content = 'mock-hash-only') => ({ hash: sha(content) }) + +const change = operations => ({ + operations, + timestamp: '2017-12-04T10:29:17.786Z', + authors: [31], +}) + +const add = (pathname, file = textFile()) => ({ pathname, file }) +const edit = pathname => ({ pathname, textOperation: ['lorem ipsum'] }) +const move = (pathname, newPathname) => ({ pathname, newPathname }) +const remove = pathname => ({ pathname, newPathname: '' }) +const setMetadata = (pathname, metadata) => ({ pathname, metadata }) + +/** + * Build the raw chunk that buildDiff() expects, with one operation per change. + */ +function chunk(files, operations, startVersion = 0) { + return { + chunk: { + history: { + snapshot: { files }, + changes: operations.map(operation => change([operation])), + }, + startVersion, + }, + } +} + +describe('FileTreeDiffGenerator', function () { + describe('buildDiff', function () { + it('reports files from the initial snapshot with their editability', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk( + { + 'a.tex': textFile(), + 'img.png': binaryFile(), + 'unknown.tex': hashOnlyFile(), + }, + [] + ), + 0, + 0 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', editable: true }, + { pathname: 'img.png', editable: false }, + { pathname: 'unknown.tex', editable: null }, + ]) + }) + + it('reports an added file with the editability of the added file', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [add('new.tex'), add('new.png', binaryFile())]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'new.tex', operation: 'added', editable: true }, + { pathname: 'new.png', operation: 'added', editable: false }, + ]) + }) + + it('replaces an untouched file with an edit, dropping the editability', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [edit('a.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([{ pathname: 'a.tex', operation: 'edited' }]) + }) + + it('reports an edit of a file that is not in the snapshot', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [edit('missing.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([ + { pathname: 'missing.tex', operation: 'edited' }, + ]) + }) + + it('reports a rename with the old pathname first', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [move('a.tex', 'b.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + newPathname: 'b.tex', + operation: 'renamed', + editable: true, + }, + ]) + }) + + it('collapses a chain of renames into a single rename', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [ + move('a.tex', 'b.tex'), + move('b.tex', 'c.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + newPathname: 'c.tex', + operation: 'renamed', + editable: true, + }, + ]) + }) + + it('reports a removal with the version it was deleted at', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile(), 'b.tex': textFile() }, [ + edit('b.tex'), + remove('a.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + operation: 'removed', + editable: true, + deletedAtV: 1, + }, + { pathname: 'b.tex', operation: 'edited' }, + ]) + }) + + it('reports a rename followed by a removal', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [ + move('a.tex', 'b.tex'), + remove('b.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + newPathname: 'b.tex', + operation: 'removed', + editable: true, + deletedAtV: 1, + }, + ]) + }) + + it('keeps an added file as an add at its new pathname when it is renamed', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [add('a.tex'), move('a.tex', 'b.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'b.tex', operation: 'added', editable: true }, + ]) + }) + + it('reports an added file that is removed again as a removal', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [add('a.tex'), remove('a.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + operation: 'removed', + editable: true, + deletedAtV: 1, + }, + ]) + }) + + describe('edits of files that are already reported with an operation', function () { + it('ignores an edit of an added file', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [add('a.tex'), edit('a.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', operation: 'added', editable: true }, + ]) + }) + + it('ignores an edit of a renamed file', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [ + move('a.tex', 'b.tex'), + edit('b.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + newPathname: 'b.tex', + operation: 'renamed', + editable: true, + }, + ]) + }) + + it('ignores an edit of a removed file', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [remove('a.tex'), edit('a.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'a.tex', + operation: 'removed', + editable: true, + deletedAtV: 0, + }, + ]) + }) + }) + + describe('edits before a rename or a removal', function () { + it('drops the editability when an edited file is renamed', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [ + edit('a.tex'), + move('a.tex', 'b.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', newPathname: 'b.tex', operation: 'renamed' }, + ]) + }) + + it('drops the editability when an edited file is removed', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [edit('a.tex'), remove('a.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', operation: 'removed', deletedAtV: 1 }, + ]) + }) + }) + + it('replaces a file that is added over an existing file', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [add('a.tex', hashOnlyFile())]), + 0, + 1 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', operation: 'added', editable: null }, + ]) + }) + + it('replaces a removed file that is added again', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [remove('a.tex'), add('a.tex')]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', operation: 'added', editable: true }, + ]) + }) + + it('replaces a removed file that another file is renamed onto', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile(), 'b.tex': textFile() }, [ + remove('a.tex'), + move('b.tex', 'a.tex'), + ]), + 0, + 2 + ) + expect(diff).to.deep.equal([ + { + pathname: 'b.tex', + newPathname: 'a.tex', + operation: 'renamed', + editable: true, + }, + ]) + }) + + it('throws when a file is renamed onto a file that still exists', function () { + expect(() => + FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile(), 'b.tex': textFile() }, [ + move('a.tex', 'b.tex'), + ]), + 0, + 1 + ) + ).to.throw(Errors.InconsistentChunkError) + }) + + it('skips a rename of a file that does not exist', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [move('missing.tex', 'renamed.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([]) + }) + + it('skips a removal of a file that does not exist', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [remove('missing.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([]) + }) + + it('skips operations without a pathname', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({}, [ + edit(''), + move('', 'renamed.tex'), + remove(''), + add('', textFile()), + ]), + 0, + 4 + ) + expect(diff).to.deep.equal([]) + }) + + it('ignores metadata operations', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [setMetadata('a.tex', { main: true })]), + 0, + 1 + ) + expect(diff).to.deep.equal([{ pathname: 'a.tex', editable: true }]) + }) + + it('reports the entries in the order of the pathnames they end up at', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk( + { + 'a.tex': textFile(), + 'b.tex': textFile(), + 'c.tex': textFile(), + }, + [ + move('b.tex', 'b2.tex'), + remove('c.tex'), + add('d.tex'), + edit('a.tex'), + ] + ), + 0, + 4 + ) + expect(diff).to.deep.equal([ + { pathname: 'a.tex', operation: 'edited' }, + { + pathname: 'c.tex', + operation: 'removed', + editable: true, + deletedAtV: 1, + }, + { + pathname: 'b.tex', + newPathname: 'b2.tex', + operation: 'renamed', + editable: true, + }, + { pathname: 'd.tex', operation: 'added', editable: true }, + ]) + }) + + it('applies the changes before fromVersion to the initial snapshot', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk( + { 'a.tex': textFile() }, + [ + add('b.tex'), + move('a.tex', 'a2.tex'), + remove('b.tex'), + edit('a2.tex'), + ], + 3 + ), + 5, + 7 + ) + expect(diff).to.deep.equal([ + { + pathname: 'b.tex', + operation: 'removed', + editable: true, + deletedAtV: 5, + }, + { pathname: 'a2.tex', operation: 'edited' }, + ]) + }) + + it('ignores changes after toVersion', function () { + const diff = FileTreeDiffGenerator.buildDiff( + chunk({ 'a.tex': textFile() }, [edit('a.tex'), remove('a.tex')]), + 0, + 1 + ) + expect(diff).to.deep.equal([{ pathname: 'a.tex', operation: 'edited' }]) + }) + }) +}) diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 3e1fa1aaab..f6519876e3 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -1,5 +1,9 @@ const Path = require('node:path') const { merge } = require('@overleaf/settings/merge') +const { + DEFAULT_TEXT_EXTENSIONS, + DEFAULT_EDITABLE_FILENAMES, +} = require('overleaf-editor-core/lib/text_file_defaults') let defaultFeatures, siteUrl @@ -28,51 +32,6 @@ const intFromEnv = function (name, defaultValue) { return parseInt(process.env[name], 10) || defaultValue } -const defaultTextExtensions = [ - 'tex', - 'latex', - 'sty', - 'cls', - 'bst', - 'bib', - 'bibtex', - 'txt', - 'tikz', - 'mtx', - 'rtex', - 'md', - 'asy', - 'lbx', - 'bbx', - 'cbx', - 'm', - 'lco', - 'dtx', - 'ins', - 'ist', - 'def', - 'clo', - 'ldf', - 'rmd', - 'qmd', - 'lua', - 'py', - 'gv', - 'mf', - 'yml', - 'yaml', - 'lhs', - 'lean', - 'lean4', - 'hs', - 'mk', - 'xmpdata', - 'cfg', - 'rnw', - 'ltx', - 'inc', -] - const parseTextExtensions = function (extensions) { if (extensions) { return extensions.split(',').map(ext => ext.trim()) @@ -899,12 +858,14 @@ module.exports = { compileBodySizeLimitMb: process.env.COMPILE_BODY_SIZE_LIMIT_MB || 7, - textExtensions: defaultTextExtensions.concat( + // The defaults come from overleaf-editor-core so that every service that + // classifies a file as a doc or as a binary file works from the same list. + textExtensions: DEFAULT_TEXT_EXTENSIONS.concat( parseTextExtensions(process.env.ADDITIONAL_TEXT_EXTENSIONS) ), // case-insensitive file names that is editable (doc) in the editor - editableFilenames: ['latexmkrc', '.latexmkrc', 'makefile', 'gnumakefile'], + editableFilenames: DEFAULT_EDITABLE_FILENAMES.slice(), fileIgnorePattern: process.env.FILE_IGNORE_PATTERN || diff --git a/yarn.lock b/yarn.lock index 82d8ffdf68..45c540dc46 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26005,6 +26005,7 @@ __metadata: path-browserify: "npm:^1.0.1" sinon: "npm:^9.2.4" typescript: "npm:^5.0.4" + utf-8-validate: "npm:^5.0.4" languageName: unknown linkType: soft