From 7d2a2b78eeab603361ca328166611e28c06ad6b7 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Wed, 29 Jan 2025 08:19:14 -0500 Subject: [PATCH] Merge pull request #23212 from overleaf/em-docupdater-send-doc-hash Send doc hash with history updates from docupdater GitOrigin-RevId: 119475d4198c6603cecd4fd579a64ff4448261ce --- .../document-updater/app/js/UpdateManager.js | 15 ++++++++--- services/document-updater/app/js/Utils.js | 23 ++++++++++++++++ services/document-updater/app/js/types.ts | 1 + .../js/UpdateManager/UpdateManagerTests.js | 18 ++++++++++--- .../test/unit/js/UtilsTests.js | 27 +++++++++++++++++++ 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/services/document-updater/app/js/UpdateManager.js b/services/document-updater/app/js/UpdateManager.js index b23522a2cb..1f58a751f7 100644 --- a/services/document-updater/app/js/UpdateManager.js +++ b/services/document-updater/app/js/UpdateManager.js @@ -14,7 +14,7 @@ const DocumentManager = require('./DocumentManager') const RangesManager = require('./RangesManager') const SnapshotManager = require('./SnapshotManager') const Profiler = require('./Profiler') -const { isInsert, isDelete, getDocLength } = require('./Utils') +const { isInsert, isDelete, getDocLength, computeDocHash } = require('./Utils') /** * @import { DeleteOp, InsertOp, Op, Ranges, Update, HistoryUpdate } from "./types" @@ -162,6 +162,7 @@ const UpdateManager = { projectHistoryId, lines, ranges, + updatedDocLines, historyRangesSupport ) @@ -290,8 +291,9 @@ const UpdateManager = { * @param {HistoryUpdate[]} updates * @param {string} pathname * @param {string} projectHistoryId - * @param {string[]} lines - * @param {Ranges} ranges + * @param {string[]} lines - document lines before updates were applied + * @param {Ranges} ranges - ranges before updates were applied + * @param {string[]} newLines - document lines after updates were applied * @param {boolean} historyRangesSupport */ _adjustHistoryUpdatesMetadata( @@ -300,6 +302,7 @@ const UpdateManager = { projectHistoryId, lines, ranges, + newLines, historyRangesSupport ) { let docLength = getDocLength(lines) @@ -363,6 +366,12 @@ const UpdateManager = { delete update.meta.tc } } + + if (historyRangesSupport && updates.length > 0) { + const lastUpdate = updates[updates.length - 1] + lastUpdate.meta ??= {} + lastUpdate.meta.doc_hash = computeDocHash(newLines) + } }, } diff --git a/services/document-updater/app/js/Utils.js b/services/document-updater/app/js/Utils.js index 4e9e60ba06..a632cf32eb 100644 --- a/services/document-updater/app/js/Utils.js +++ b/services/document-updater/app/js/Utils.js @@ -1,4 +1,5 @@ // @ts-check +const { createHash } = require('node:crypto') const _ = require('lodash') /** @@ -79,6 +80,27 @@ function addTrackedDeletesToContent(content, trackedChanges) { return result } +/** + * Compute the content hash for a doc + * + * This hash is sent to the history to validate updates. + * + * @param {string[]} lines + * @return {string} the doc hash + */ +function computeDocHash(lines) { + const hash = createHash('sha1') + if (lines.length > 0) { + for (const line of lines.slice(0, lines.length - 1)) { + hash.update(line) + hash.update('\n') + } + // The last line doesn't end with a newline + hash.update(lines[lines.length - 1]) + } + return hash.digest('hex') +} + /** * checks if the given originOrSource should be treated as a source or origin * TODO: remove this hack and remove all "source" references @@ -102,5 +124,6 @@ module.exports = { isComment, addTrackedDeletesToContent, getDocLength, + computeDocHash, extractOriginOrSource, } diff --git a/services/document-updater/app/js/types.ts b/services/document-updater/app/js/types.ts index d635ab31ca..b3085adb1b 100644 --- a/services/document-updater/app/js/types.ts +++ b/services/document-updater/app/js/types.ts @@ -84,6 +84,7 @@ export type HistoryUpdate = { pathname?: string doc_length?: number history_doc_length?: number + doc_hash?: string tc?: boolean user_id?: string } diff --git a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js b/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js index 16ee0b12e1..dba66456b7 100644 --- a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js +++ b/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js @@ -1,5 +1,4 @@ -// @ts-check - +const { createHash } = require('node:crypto') const sinon = require('sinon') const { expect } = require('chai') const SandboxedModule = require('sandboxed-module') @@ -399,7 +398,9 @@ describe('UpdateManager', function () { this.historyUpdates, this.pathname, this.projectHistoryId, - this.lines + this.lines, + this.ranges, + this.updatedDocLines ) }) @@ -526,6 +527,7 @@ describe('UpdateManager', function () { describe('_adjustHistoryUpdatesMetadata', function () { beforeEach(function () { this.lines = ['some', 'test', 'data'] + this.updatedDocLines = ['after', 'updates'] this.historyUpdates = [ { v: 42, @@ -570,6 +572,7 @@ describe('UpdateManager', function () { this.pathname, this.projectHistoryId, this.lines, + this.updatedDocLines, this.ranges, false ) @@ -632,6 +635,7 @@ describe('UpdateManager', function () { this.projectHistoryId, this.lines, this.ranges, + this.updatedDocLines, true ) this.historyUpdates.should.deep.equal([ @@ -685,6 +689,7 @@ describe('UpdateManager', function () { meta: { pathname: this.pathname, doc_length: 21, // 23 - 'so' + doc_hash: stringHash(this.updatedDocLines.join('\n')), history_doc_length: 28, // 30 - 'so' }, }, @@ -699,6 +704,7 @@ describe('UpdateManager', function () { this.projectHistoryId, [], {}, + ['foobar'], false ) this.historyUpdates.should.deep.equal([ @@ -822,3 +828,9 @@ describe('UpdateManager', function () { }) }) }) + +function stringHash(s) { + const hash = createHash('sha1') + hash.update(s) + return hash.digest('hex') +} diff --git a/services/document-updater/test/unit/js/UtilsTests.js b/services/document-updater/test/unit/js/UtilsTests.js index 553b90159a..5d0f03ca64 100644 --- a/services/document-updater/test/unit/js/UtilsTests.js +++ b/services/document-updater/test/unit/js/UtilsTests.js @@ -1,5 +1,6 @@ // @ts-check +const { createHash } = require('node:crypto') const { expect } = require('chai') const Utils = require('../../../app/js/Utils') @@ -24,4 +25,30 @@ describe('Utils', function () { expect(result).to.equal('the quick brown fox jumps over the lazy dog') }) }) + + describe('computeDocHash', function () { + it('computes the hash for an empty doc', function () { + const actual = Utils.computeDocHash([]) + const expected = stringHash('') + expect(actual).to.equal(expected) + }) + + it('computes the hash for a single-line doc', function () { + const actual = Utils.computeDocHash(['hello']) + const expected = stringHash('hello') + expect(actual).to.equal(expected) + }) + + it('computes the hash for a multiline doc', function () { + const actual = Utils.computeDocHash(['hello', 'there', 'world']) + const expected = stringHash('hello\nthere\nworld') + expect(actual).to.equal(expected) + }) + }) }) + +function stringHash(s) { + const hash = createHash('sha1') + hash.update(s) + return hash.digest('hex') +}