From fa892b336d0c0f2eddd3bf9df4b944094a640dad Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Thu, 14 Aug 2025 10:35:50 +0200 Subject: [PATCH] Add endpoint to retrieve document with history ranges and use it in dsmp API (#27564) GitOrigin-RevId: 3d2ac33cdc903a07b8ec67f7fb6f723ae9c81a26 --- .../document-updater/app/js/HttpController.js | 51 +++++++++--- .../js/HttpController/HttpControllerTests.js | 82 ++++++++++++++++++- .../DocumentUpdater/DocumentUpdaterHandler.js | 25 ++++++ 3 files changed, 144 insertions(+), 14 deletions(-) diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index 04dfdaf543..7e16a0b0dd 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -10,12 +10,16 @@ const DeleteQueueManager = require('./DeleteQueueManager') const { getTotalSizeOfLines } = require('./Limits') const async = require('async') const { StringFileData } = require('overleaf-editor-core') +const { addTrackedDeletesToContent } = require('./Utils') +const HistoryConversions = require('./HistoryConversions') function getDoc(req, res, next) { let fromVersion const docId = req.params.doc_id const projectId = req.params.project_id - logger.debug({ projectId, docId }, 'getting doc via http') + const historyRanges = req.query.historyRanges === 'true' + + logger.debug({ projectId, docId, historyRanges }, 'getting doc via http') const timer = new Metrics.Timer('http.getDoc') if (req.query.fromVersion != null) { @@ -33,7 +37,7 @@ function getDoc(req, res, next) { if (error) { return next(error) } - logger.debug({ projectId, docId }, 'got doc via http') + logger.debug({ projectId, docId, historyRanges }, 'got doc via http') if (lines == null || version == null) { return next(new Errors.NotFoundError('document not found')) } @@ -42,16 +46,39 @@ function getDoc(req, res, next) { // TODO(24596): tc support for history-ot lines = file.getLines() } - res.json({ - id: docId, - lines, - version, - ops, - ranges, - pathname, - ttlInS: RedisManager.DOC_OPS_TTL, - type, - }) + + if (historyRanges) { + const docContentWithTrackedDeletes = addTrackedDeletesToContent( + lines.join('\n'), + ranges?.changes ?? [] + ) + const docLinesWithTrackedDeletes = + docContentWithTrackedDeletes.split('\n') + const rangesWithTrackedDeletes = + HistoryConversions.toHistoryRanges(ranges) + + res.json({ + id: docId, + lines: docLinesWithTrackedDeletes, + version, + ops, + ranges: rangesWithTrackedDeletes, + pathname, + ttlInS: RedisManager.DOC_OPS_TTL, + type, + }) + } else { + res.json({ + id: docId, + lines, + version, + ops, + ranges, + pathname, + ttlInS: RedisManager.DOC_OPS_TTL, + type, + }) + } } ) } diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index 333da10d15..9f99659d9e 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js @@ -19,6 +19,12 @@ describe('HttpController', function () { }), './Metrics': (this.Metrics = {}), './Errors': Errors, + './Utils': (this.Utils = { + addTrackedDeletesToContent: sinon.stub().returnsArg(0), + }), + './HistoryConversions': (this.HistoryConversions = { + toHistoryRanges: sinon.stub().returnsArg(0), + }), '@overleaf/settings': { max_doc_length: 2 * 1024 * 1024 }, }, }) @@ -95,7 +101,11 @@ describe('HttpController', function () { it('should log the request', function () { this.logger.debug .calledWith( - { docId: this.doc_id, projectId: this.project_id }, + { + docId: this.doc_id, + projectId: this.project_id, + historyRanges: false, + }, 'getting doc via http' ) .should.equal(true) @@ -147,7 +157,75 @@ describe('HttpController', function () { it('should log the request', function () { this.logger.debug .calledWith( - { docId: this.doc_id, projectId: this.project_id }, + { + docId: this.doc_id, + projectId: this.project_id, + historyRanges: false, + }, + 'getting doc via http' + ) + .should.equal(true) + }) + + it('should time the request', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) + }) + }) + + describe('when historyRanges query param is true', function () { + beforeEach(function () { + this.DocumentManager.getDocAndRecentOpsWithLock = sinon + .stub() + .callsArgWith( + 3, + null, + this.lines, + this.version, + [], + this.ranges, + this.pathname, + this.projectHistoryId, + 'sharejs-text-ot' + ) + this.req.query = { historyRanges: 'true' } + this.HttpController.getDoc(this.req, this.res, this.next) + }) + + it('should get the doc', function () { + this.DocumentManager.getDocAndRecentOpsWithLock + .calledWith(this.project_id, this.doc_id, -1) + .should.equal(true) + }) + + it('should return the doc as JSON with history ranges processing', function () { + this.res.json.should.have.been.calledWith({ + id: this.doc_id, + lines: this.lines, + version: this.version, + ops: [], + ranges: this.ranges, + pathname: this.pathname, + ttlInS: 42, + type: 'sharejs-text-ot', + }) + }) + + it('should call addTrackedDeletesToContent for history ranges processing', function () { + this.Utils.addTrackedDeletesToContent.called.should.equal(true) + }) + + it('should call toHistoryRanges for range conversion', function () { + this.HistoryConversions.toHistoryRanges.called.should.equal(true) + }) + + it('should log the request with historyRanges: true', function () { + this.logger.debug + .calledWith( + { + docId: this.doc_id, + projectId: this.project_id, + historyRanges: true, + }, 'getting doc via http' ) .should.equal(true) diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 670ec598c5..fd70f47ed1 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -137,6 +137,29 @@ function getDocument(projectId, docId, fromVersion, callback) { ) } +/** + * Get a document with its history ranges + * @param {string} projectId + * @param {string} docId + * @param {Callback} callback + */ +function getDocumentWithHistoryRanges(projectId, docId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}?historyRanges=true`, + json: true, + }, + projectId, + 'get-document-with-history-ranges', + function (error, doc) { + if (error) { + return callback(error) + } + callback(null, doc) + } + ) +} + function setDocument(projectId, docId, userId, docLines, source, callback) { _makeRequest( { @@ -661,6 +684,7 @@ module.exports = { blockProject, unblockProject, updateProjectStructure, + getDocumentWithHistoryRanges, promises: { flushProjectToMongo: promisify(flushProjectToMongo), flushMultipleProjectsToMongo: promisify(flushMultipleProjectsToMongo), @@ -688,5 +712,6 @@ module.exports = { unblockProject: promisify(unblockProject), updateProjectStructure: promisify(updateProjectStructure), appendToDocument: promisify(appendToDocument), + getDocumentWithHistoryRanges: promisify(getDocumentWithHistoryRanges), }, }