Merge pull request #20421 from overleaf/em-history-limit-tracked-deletes

Do not count tracked deletes towards size limit in history

GitOrigin-RevId: 0185e6df80d8f3433aa489a1c90f5a6499af5ef4
This commit is contained in:
Eric Mc Sween 2024-09-17 15:05:30 -04:00 committed by Copybot
parent 5b73f08703
commit 7cd16a84e0
6 changed files with 101 additions and 26 deletions

View file

@ -3,8 +3,6 @@
const assert = require('check-types').assert const assert = require('check-types').assert
const OError = require('@overleaf/o-error') const OError = require('@overleaf/o-error')
const TextOperation = require('./operation/text_operation')
class NotFoundError extends OError { class NotFoundError extends OError {
constructor(hash) { constructor(hash) {
super(`blob ${hash} not found`, { hash }) super(`blob ${hash} not found`, { hash })
@ -26,17 +24,14 @@ class Blob {
* in to determine that; so it is useful to have an upper bound on the byte * in to determine that; so it is useful to have an upper bound on the byte
* length of a file that might be editable. * length of a file that might be editable.
* *
* The reason for the factor of 3 is as follows. We cannot currently edit files * This used to be 3 times the max editable file length to account for 3-byte
* that contain characters outside of the basic multilingual plane, so we're * UTF-8 codepoints. However, editable file blobs now include tracked deletes
* limited to characters that can be represented in a single, two-byte UCS-2 * and the system used to allow unlimited tracked deletes on a single file.
* code unit. Encoding the largest such value, 0xFFFF (which is not actually * A practical limit is the 16 MB Mongo size limit. It wouldn't have been
* a valid character), takes three bytes in UTF-8: 0xEF 0xBF 0xBF. A file * possible to store more than 16 MB of tracked deletes. We therefore fall
* composed entirely of three-byte UTF-8 codepoints is the worst case; in * back to this limit.
* practice, this is a very conservative upper bound.
*
* @type {number}
*/ */
static MAX_EDITABLE_BYTE_LENGTH_BOUND = 3 * TextOperation.MAX_STRING_LENGTH static MAX_EDITABLE_BYTE_LENGTH_BOUND = 16 * 1024 * 1024
static NotFoundError = NotFoundError static NotFoundError = NotFoundError

View file

@ -41,6 +41,17 @@ class TrackedChangeList {
return this._trackedChanges.length return this._trackedChanges.length
} }
/**
* Iterate over tracked changes
*
* @returns {Iterator<TrackedChange>}
*/
*[Symbol.iterator]() {
for (const change of this._trackedChanges) {
yield change
}
}
/** /**
* @returns {readonly TrackedChange[]} * @returns {readonly TrackedChange[]}
*/ */

View file

@ -46,9 +46,18 @@ class TextOperation extends EditOperation {
/** /**
* Length of the longest file that we'll attempt to edit, in characters. * Length of the longest file that we'll attempt to edit, in characters.
* *
* @type {number} * This number used to be 2M characters, but it didn't include tracked
* deletes. Now that it includes tracked deletes, we raise the limit to the
* Mongo document size limit. It would have been impossible to store more
* tracked deletes in a single document.
*/ */
static MAX_STRING_LENGTH = 2 * Math.pow(1024, 2) static MAX_STRING_LENGTH = 16 * Math.pow(1024, 2)
/**
* Max editable file length, excluding tracked deletes.
*/
static MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES = 2 * Math.pow(1024, 2)
static UnprocessableError = UnprocessableError static UnprocessableError = UnprocessableError
static ApplyError = ApplyError static ApplyError = ApplyError
static InvalidInsertionError = InvalidInsertionError static InvalidInsertionError = InvalidInsertionError
@ -325,9 +334,30 @@ class TextOperation extends EditOperation {
} }
if (result.length > TextOperation.MAX_STRING_LENGTH) { if (result.length > TextOperation.MAX_STRING_LENGTH) {
// We're over the hard limit, with or without tracked deletes
throw new TextOperation.TooLongError(operation, result.length) throw new TextOperation.TooLongError(operation, result.length)
} }
if (
result.length > TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES
) {
// We might be over the limit, but we need to check how much of the
// length is tracked deletes.
let trackedDeletesLength = 0
for (const change of file.trackedChanges) {
if (change.tracking.type === 'delete') {
trackedDeletesLength += change.range.length
}
}
if (
result.length - trackedDeletesLength >
TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES
) {
throw new TextOperation.TooLongError(operation, result.length)
}
}
file.content = result file.content = result
} }

View file

@ -7,16 +7,15 @@ const TextOperation = ot.TextOperation
describe('HollowStringFileData', function () { describe('HollowStringFileData', function () {
it('validates string length when edited', function () { it('validates string length when edited', function () {
const maxLength = TextOperation.MAX_STRING_LENGTH const length = 200
const fileData = new HollowStringFileData(maxLength) const fileData = new HollowStringFileData(length)
expect(fileData.getStringLength()).to.equal(maxLength)
expect(() => { expect(() => {
fileData.edit(new TextOperation().retain(maxLength).insert('x')) fileData.edit(new TextOperation().retain(length + 10).insert('x'))
}).to.throw(TextOperation.TooLongError) }).to.throw(TextOperation.ApplyError)
expect(fileData.getStringLength()).to.equal(maxLength) expect(fileData.getStringLength()).to.equal(length)
fileData.edit(new TextOperation().retain(maxLength - 1).remove(1)) fileData.edit(new TextOperation().retain(length).insert('x'))
expect(fileData.getStringLength()).to.equal(maxLength - 1) expect(fileData.getStringLength()).to.equal(length + 1)
}) })
}) })

View file

@ -178,7 +178,7 @@ describe('LazyStringFileData', function () {
expect(fileData.getStringLength()).to.equal(0) expect(fileData.getStringLength()).to.equal(0)
expect(fileData.getOperations()).to.have.length(0) expect(fileData.getOperations()).to.have.length(0)
const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) const longString = _.repeat('a', 1000)
fileData.edit(new TextOperation().insert(longString)) fileData.edit(new TextOperation().insert(longString))
expect(fileData.getHash()).not.to.exist expect(fileData.getHash()).not.to.exist
expect(fileData.getByteLength()).to.equal(longString.length) // approximate expect(fileData.getByteLength()).to.equal(longString.length) // approximate
@ -186,8 +186,10 @@ describe('LazyStringFileData', function () {
expect(fileData.getOperations()).to.have.length(1) expect(fileData.getOperations()).to.have.length(1)
expect(() => { expect(() => {
fileData.edit(new TextOperation().retain(longString.length).insert('x')) fileData.edit(
}).to.throw(TextOperation.TooLongError) new TextOperation().retain(longString.length - 123).insert('x')
)
}).to.throw(TextOperation.ApplyError)
expect(fileData.getHash()).not.to.exist expect(fileData.getHash()).not.to.exist
expect(fileData.getByteLength()).to.equal(longString.length) // approximate expect(fileData.getByteLength()).to.equal(longString.length) // approximate
expect(fileData.getStringLength()).to.equal(longString.length) expect(fileData.getStringLength()).to.equal(longString.length)

View file

@ -5,7 +5,10 @@ const { expect } = require('chai')
const _ = require('lodash') const _ = require('lodash')
const ot = require('..') const ot = require('..')
const Range = require('../lib/range')
const StringFileData = require('../lib/file_data/string_file_data') const StringFileData = require('../lib/file_data/string_file_data')
const TrackedChange = require('../lib/file_data/tracked_change')
const TrackingProps = require('../lib/file_data/tracking_props')
const TextOperation = ot.TextOperation const TextOperation = ot.TextOperation
describe('StringFileData', function () { describe('StringFileData', function () {
@ -20,7 +23,10 @@ describe('StringFileData', function () {
}) })
it('validates string length when edited', function () { it('validates string length when edited', function () {
const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) const longString = _.repeat(
'a',
TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES
)
const fileData = new StringFileData(longString) const fileData = new StringFileData(longString)
expect(fileData.getByteLength()).to.equal(longString.length) expect(fileData.getByteLength()).to.equal(longString.length)
expect(fileData.getStringLength()).to.equal(longString.length) expect(fileData.getStringLength()).to.equal(longString.length)
@ -36,6 +42,38 @@ describe('StringFileData', function () {
expect(fileData.getStringLength()).to.equal(longString.length - 1) expect(fileData.getStringLength()).to.equal(longString.length - 1)
}) })
it('ignores tracked deletes when checking the max string length', function () {
const longString = _.repeat(
'a',
TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES
)
const fileData = new StringFileData(longString)
fileData.trackedChanges.add(
new TrackedChange(
new Range(123, 100),
new TrackingProps('delete', 'some-user', new Date())
)
)
fileData.trackedChanges.add(
new TrackedChange(
new Range(456, 50),
new TrackingProps('insert', 'some-user', new Date())
)
)
// Add text the same length as the tracked delete
fileData.edit(
new TextOperation().retain(longString.length).insert('x'.repeat(100))
)
// Add more text
expect(() => {
fileData.edit(
new TextOperation().retain(longString.length + 100).insert('x')
)
}).to.throw(TextOperation.TooLongError)
})
it('getComments() should return an empty array', function () { it('getComments() should return an empty array', function () {
const fileData = new StringFileData('test') const fileData = new StringFileData('test')
expect(fileData.getComments().toRaw()).to.eql([]) expect(fileData.getComments().toRaw()).to.eql([])