Merge pull request #18142 from overleaf/em-docupdater-resync-ranges

Adjust ranges for tracked deletes when resyncing projects

GitOrigin-RevId: 5f8e6279cdc31e76a2f93cf2129eaca8cac3cb78

services/document-updater/app/js/HistoryConversions.js

@@ -0,0 +1,183 @@
+// @ts-check
+
+const _ = require('lodash')
+const { isDelete } = require('./Utils')
+
+/**
+ * @typedef {import('./types').Comment} Comment
+ * @typedef {import('./types').HistoryComment} HistoryComment
+ * @typedef {import('./types').HistoryRanges} HistoryRanges
+ * @typedef {import('./types').HistoryTrackedChange} HistoryTrackedChange
+ * @typedef {import('./types').Ranges} Ranges
+ * @typedef {import('./types').TrackedChange} TrackedChange
+ */
+
+/**
+ * Convert editor ranges to history ranges
+ *
+ * @param {Ranges} ranges
+ * @return {HistoryRanges}
+ */
+function toHistoryRanges(ranges) {
+  const changes = ranges.changes ?? []
+  const comments = (ranges.comments ?? []).slice()
+
+  // Changes are assumed to be sorted, but not comments
+  comments.sort((a, b) => a.op.p - b.op.p)
+
+  /**
+   * This will allow us to go through comments at a different pace as we loop
+   * through tracked changes
+   */
+  const commentsIterator = new CommentsIterator(comments)
+
+  /**
+   * Current offset between editor pos and history pos
+   */
+  let offset = 0
+
+  /**
+   * History comments that might overlap with the tracked change considered
+   *
+   * @type {HistoryComment[]}
+   */
+  let pendingComments = []
+
+  /**
+   * The final history comments generated
+   *
+   * @type {HistoryComment[]}
+   */
+  const historyComments = []
+
+  /**
+   * The final history tracked changes generated
+   *
+   * @type {HistoryTrackedChange[]}
+   */
+  const historyChanges = []
+
+  for (const change of changes) {
+    historyChanges.push(toHistoryChange(change, offset))
+
+    // After this point, we're only interested in tracked deletes
+    if (!isDelete(change.op)) {
+      continue
+    }
+
+    // Fill pendingComments with new comments that start before this tracked
+    // delete and might overlap
+    for (const comment of commentsIterator.nextComments(change.op.p)) {
+      pendingComments.push(toHistoryComment(comment, offset))
+    }
+
+    // Save comments that are fully before this tracked delete
+    const newPendingComments = []
+    for (const historyComment of pendingComments) {
+      const commentEnd = historyComment.op.p + historyComment.op.c.length
+      if (commentEnd <= change.op.p) {
+        historyComments.push(historyComment)
+      } else {
+        newPendingComments.push(historyComment)
+      }
+    }
+    pendingComments = newPendingComments
+
+    // The rest of pending comments overlap with this tracked change. Adjust
+    // their history length.
+    for (const historyComment of pendingComments) {
+      historyComment.op.hlen =
+        (historyComment.op.hlen ?? historyComment.op.c.length) +
+        change.op.d.length
+    }
+
+    // Adjust the offset
+    offset += change.op.d.length
+  }
+  // Save the last pending comments
+  for (const historyComment of pendingComments) {
+    historyComments.push(historyComment)
+  }
+
+  // Save any comments that came after the last tracked change
+  for (const comment of commentsIterator.nextComments()) {
+    historyComments.push(toHistoryComment(comment, offset))
+  }
+
+  const historyRanges = {}
+  if (historyComments.length > 0) {
+    historyRanges.comments = historyComments
+  }
+  if (historyChanges.length > 0) {
+    historyRanges.changes = historyChanges
+  }
+  return historyRanges
+}
+
+class CommentsIterator {
+  /**
+   * Build a CommentsIterator
+   *
+   * @param {Comment[]} comments
+   */
+  constructor(comments) {
+    this.comments = comments
+    this.currentIndex = 0
+  }
+
+  /**
+   * Generator that returns the next comments to consider
+   *
+   * @param {number} beforePos - only return comments that start before this position
+   * @return {Iterable<Comment>}
+   */
+  *nextComments(beforePos = Infinity) {
+    while (this.currentIndex < this.comments.length) {
+      const comment = this.comments[this.currentIndex]
+      if (comment.op.p < beforePos) {
+        yield comment
+        this.currentIndex += 1
+      } else {
+        return
+      }
+    }
+  }
+}
+
+/**
+ * Convert an editor tracked change into a history tracked change
+ *
+ * @param {TrackedChange} change
+ * @param {number} offset - how much the history change is ahead of the
+ *                 editor change
+ * @return {HistoryTrackedChange}
+ */
+function toHistoryChange(change, offset) {
+  /** @type {HistoryTrackedChange} */
+  const historyChange = _.cloneDeep(change)
+  if (offset > 0) {
+    historyChange.op.hpos = change.op.p + offset
+  }
+  return historyChange
+}
+
+/**
+ * Convert an editor comment into a history comment
+ *
+ * @param {Comment} comment
+ * @param {number} offset - how much the history comment is ahead of the
+ *                 editor comment
+ * @return {HistoryComment}
+ */
+function toHistoryComment(comment, offset) {
+  /** @type {HistoryComment} */
+  const historyComment = _.cloneDeep(comment)
+  if (offset > 0) {
+    historyComment.op.hpos = comment.op.p + offset
+  }
+  return historyComment
+}
+
+module.exports = {
+  toHistoryRanges,
+}

services/document-updater/app/js/ProjectHistoryRedisManager.js

@@ -10,6 +10,7 @@ const logger = require('@overleaf/logger')
 const metrics = require('./Metrics')
 const { docIsTooLarge } = require('./Limits')
 const { addTrackedDeletesToContent } = require('./Utils')
+const HistoryConversions = require('./HistoryConversions')
 const OError = require('@overleaf/o-error')
 
 /**
@@ -170,7 +171,8 @@ const ProjectHistoryRedisManager = {
     }
 
     if (historyRangesSupport) {
-      projectUpdate.resyncDocContent.ranges = ranges
+      projectUpdate.resyncDocContent.ranges =
+        HistoryConversions.toHistoryRanges(ranges)
     }
 
     const jsonUpdate = JSON.stringify(projectUpdate)

services/document-updater/app/js/types.ts

@@ -1,5 +1,8 @@
 import { TrackingPropsRawData } from 'overleaf-editor-core/types/lib/types'
 
+/**
+ * An update coming from the editor
+ */
 export type Update = {
   doc: string
   op: Op[]
@@ -37,6 +40,9 @@ export type CommentOp = {
   u?: boolean
 }
 
+/**
+ * Ranges record on a document
+ */
 export type Ranges = {
   comments?: Comment[]
   changes?: TrackedChange[]
@@ -53,14 +59,35 @@ export type Comment = {
 
 export type TrackedChange = {
   id: string
-  op: Op
+  op: InsertOp | DeleteOp
   metadata: {
     user_id: string
     ts: string
   }
 }
 
-export type HistoryOp = HistoryInsertOp | HistoryDeleteOp | HistoryCommentOp | HistoryRetainOp
+/**
+ * Updates sent to project-history
+ */
+export type HistoryUpdate = {
+  op: HistoryOp[]
+  doc: string
+  v?: number
+  meta?: {
+    pathname?: string
+    doc_length?: number
+    history_doc_length?: number
+    tc?: boolean
+    user_id?: string
+  }
+  projectHistoryId?: string
+}
+
+export type HistoryOp =
+  | HistoryInsertOp
+  | HistoryDeleteOp
+  | HistoryCommentOp
+  | HistoryRetainOp
 
 export type HistoryInsertOp = InsertOp & {
   commentIds?: string[]
@@ -89,16 +116,13 @@ export type HistoryCommentOp = CommentOp & {
   hlen?: number
 }
 
-export type HistoryUpdate = {
-  op: HistoryOp[]
-  doc: string
-  v?: number
-  meta?: {
-    pathname?: string
-    doc_length?: number
-    history_doc_length?: number
-    tc?: boolean
-    user_id?: string
-  }
-  projectHistoryId?: string
+export type HistoryRanges = {
+  comments?: HistoryComment[]
+  changes?: HistoryTrackedChange[]
+}
+
+export type HistoryComment = Comment & { op: HistoryCommentOp }
+
+export type HistoryTrackedChange = TrackedChange & {
+  op: HistoryInsertOp | HistoryDeleteOp
 }

services/document-updater/test/unit/js/HistoryConversionsTests.js

@@ -0,0 +1,117 @@
+const _ = require('lodash')
+const { expect } = require('chai')
+const HistoryConversions = require('../../../app/js/HistoryConversions')
+
+describe('HistoryConversions', function () {
+  describe('toHistoryRanges', function () {
+    it('handles empty ranges', function () {
+      expect(HistoryConversions.toHistoryRanges({})).to.deep.equal({})
+    })
+
+    it("doesn't modify comments when there are no tracked changes", function () {
+      const ranges = {
+        comments: [makeComment('comment1', 5, 12)],
+      }
+      const historyRanges = HistoryConversions.toHistoryRanges(ranges)
+      expect(historyRanges).to.deep.equal(ranges)
+    })
+
+    it('adjusts comments and tracked changes to account for tracked deletes', function () {
+      const comments = [
+        makeComment('comment0', 0, 1),
+        makeComment('comment1', 10, 12),
+        makeComment('comment2', 20, 10),
+        makeComment('comment3', 15, 3),
+      ]
+      const changes = [
+        makeTrackedDelete('change0', 2, 5),
+        makeTrackedInsert('change1', 4, 5),
+        makeTrackedDelete('change2', 10, 10),
+        makeTrackedDelete('change3', 21, 6),
+        makeTrackedDelete('change4', 50, 7),
+      ]
+      const ranges = { comments, changes }
+
+      const historyRanges = HistoryConversions.toHistoryRanges(ranges)
+      expect(historyRanges.comments).to.have.deep.members([
+        comments[0],
+        // shifted by change0 and change2, extended by change3
+        enrichOp(comments[1], {
+          hpos: 25, // 10 + 5 + 10
+          hlen: 18, // 12 + 6
+        }),
+        // shifted by change0 and change2, extended by change3
+        enrichOp(comments[2], {
+          hpos: 35, // 20 + 5 + 10
+          hlen: 16, // 10 + 6
+        }),
+        // shifted by change0 and change2
+        enrichOp(comments[3], {
+          hpos: 30, // 15 + 5 + 10
+        }),
+      ])
+      expect(historyRanges.changes).to.deep.equal([
+        changes[0],
+        enrichOp(changes[1], {
+          hpos: 9, // 4 + 5
+        }),
+        enrichOp(changes[2], {
+          hpos: 15, // 10 + 5
+        }),
+        enrichOp(changes[3], {
+          hpos: 36, // 21 + 5 + 10
+        }),
+        enrichOp(changes[4], {
+          hpos: 71, // 50 + 5 + 10 + 6
+        }),
+      ])
+    })
+  })
+})
+
+function makeComment(id, pos, length) {
+  return {
+    id,
+    op: {
+      c: 'c'.repeat(length),
+      p: pos,
+      t: id,
+    },
+    metadata: makeMetadata(),
+  }
+}
+
+function makeTrackedInsert(id, pos, length) {
+  return {
+    id,
+    op: {
+      i: 'i'.repeat(length),
+      p: pos,
+    },
+    metadata: makeMetadata(),
+  }
+}
+
+function makeTrackedDelete(id, pos, length) {
+  return {
+    id,
+    op: {
+      d: 'd'.repeat(length),
+      p: pos,
+    },
+    metadata: makeMetadata(),
+  }
+}
+
+function makeMetadata() {
+  return {
+    user_id: 'user-id',
+    ts: new Date().toISOString(),
+  }
+}
+
+function enrichOp(commentOrChange, extraFields) {
+  const result = _.cloneDeep(commentOrChange)
+  Object.assign(result.op, extraFields)
+  return result
+}