Merge pull request #20888 from overleaf/revert-20834-em-validate-tracked-deletes

Revert "Sanity check for tracked changes in document-updater"

GitOrigin-RevId: 1b7592b1f2d60374ce9ba54d9db5e34c9636ea9a
This commit is contained in:
Eric Mc Sween 2024-10-07 15:06:57 -04:00 committed by Copybot
parent 4919172d52
commit 731ba80b47
4 changed files with 3 additions and 89 deletions

View file

@ -264,12 +264,7 @@ const DocumentManager = {
throw new Errors.NotFoundError(`document not found: ${docId}`)
}
const newRanges = RangesManager.acceptChanges(
projectId,
docId,
changeIds,
ranges
)
const newRanges = RangesManager.acceptChanges(changeIds, ranges)
await RedisManager.promises.updateDocument(
projectId,

View file

@ -80,8 +80,6 @@ const RangesManager = {
}
}
sanityCheckTrackedChanges(projectId, docId, rangesTracker.changes)
if (
rangesTracker.changes?.length > RangesManager.MAX_CHANGES ||
rangesTracker.comments?.length > RangesManager.MAX_COMMENTS
@ -129,12 +127,11 @@ const RangesManager = {
return { newRanges, rangesWereCollapsed, historyUpdates }
},
acceptChanges(projectId, docId, changeIds, ranges) {
acceptChanges(changeIds, ranges) {
const { changes, comments } = ranges
logger.debug(`accepting ${changeIds.length} changes in ranges`)
const rangesTracker = new RangesTracker(changes, comments)
rangesTracker.removeChangeIds(changeIds)
sanityCheckTrackedChanges(projectId, docId, rangesTracker.changes)
const newRanges = RangesManager._getRanges(rangesTracker)
return newRanges
},
@ -560,71 +557,4 @@ function getCroppedCommentOps(op, comments) {
return historyCommentOps
}
/**
* Check some tracked changes assumptions:
*
* - Tracked changes can't be empty
* - Tracked inserts can't overlap with another tracked change
* - There can't be two tracked deletes at the same position
* - Ranges should be ordered by position, deletes before inserts
*
* If any assumption isn't upheld, log a warning.
*
* @param {string} projectId
* @param {string} docId
* @param {TrackedChange[]} changes
*/
function sanityCheckTrackedChanges(projectId, docId, changes) {
let lastDeletePos = -1 // allow a tracked delete at position 0
let lastInsertEnd = 0
let ok = true
let badChangeIndex
for (let i = 0; i < changes.length; i++) {
const change = changes[i]
const op = change.op
if ('i' in op) {
if (op.i.length === 0 || op.p < lastDeletePos || op.p < lastInsertEnd) {
ok = false
badChangeIndex = i
break
}
lastInsertEnd = op.p + op.i.length
} else if ('d' in op) {
if (op.d.length === 0 || op.p <= lastDeletePos || op.p < lastInsertEnd) {
ok = false
badChangeIndex = i
break
}
lastDeletePos = op.p
}
}
if (ok) {
return
}
const changeRanges = []
for (const change of changes) {
if ('i' in change.op) {
changeRanges.push({
id: change.id,
p: change.op.p,
i: change.op.i.length,
})
} else if ('d' in change.op) {
changeRanges.push({
id: change.id,
p: change.op.p,
d: change.op.d.length,
})
}
}
logger.warn(
{ projectId, docId, changes: changeRanges, badChangeIndex },
'Malformed tracked changes detected'
)
}
module.exports = RangesManager

View file

@ -691,8 +691,6 @@ describe('DocumentManager', function () {
it('should apply the accept change to the ranges', function () {
this.RangesManager.acceptChanges.should.have.been.calledWith(
this.project_id,
this.doc_id,
[this.change_id],
this.ranges
)
@ -724,12 +722,7 @@ describe('DocumentManager', function () {
it('should apply the accept change to the ranges', function () {
this.RangesManager.acceptChanges
.calledWith(
this.project_id,
this.doc_id,
this.change_ids,
this.ranges
)
.calledWith(this.change_ids, this.ranges)
.should.equal(true)
})
})

View file

@ -668,8 +668,6 @@ describe('RangesManager', function () {
beforeEach(function () {
this.change_ids = [this.ranges.changes[1].id]
this.result = this.RangesManager.acceptChanges(
this.project_id,
this.doc_id,
this.change_ids,
this.ranges
)
@ -716,8 +714,6 @@ describe('RangesManager', function () {
this.ranges.changes[4].id,
]
this.result = this.RangesManager.acceptChanges(
this.project_id,
this.doc_id,
this.change_ids,
this.ranges
)