Kaynağa Gözat

Add endpoint to retrieve document with history ranges and use it in dsmp API (#27564)

GitOrigin-RevId: 3d2ac33cdc903a07b8ec67f7fb6f723ae9c81a26
Domagoj Kriskovic 1 yıl önce
ebeveyn
işleme
704e577c23

+ 39 - 12
services/document-updater/app/js/HttpController.js

@@ -10,12 +10,16 @@ const DeleteQueueManager = require('./DeleteQueueManager')
 const { getTotalSizeOfLines } = require('./Limits')
 const { getTotalSizeOfLines } = require('./Limits')
 const async = require('async')
 const async = require('async')
 const { StringFileData } = require('overleaf-editor-core')
 const { StringFileData } = require('overleaf-editor-core')
+const { addTrackedDeletesToContent } = require('./Utils')
+const HistoryConversions = require('./HistoryConversions')
 
 
 function getDoc(req, res, next) {
 function getDoc(req, res, next) {
   let fromVersion
   let fromVersion
   const docId = req.params.doc_id
   const docId = req.params.doc_id
   const projectId = req.params.project_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')
   const timer = new Metrics.Timer('http.getDoc')
 
 
   if (req.query.fromVersion != null) {
   if (req.query.fromVersion != null) {
@@ -33,7 +37,7 @@ function getDoc(req, res, next) {
       if (error) {
       if (error) {
         return next(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) {
       if (lines == null || version == null) {
         return next(new Errors.NotFoundError('document not found'))
         return next(new Errors.NotFoundError('document not found'))
       }
       }
@@ -42,16 +46,39 @@ function getDoc(req, res, next) {
         // TODO(24596): tc support for history-ot
         // TODO(24596): tc support for history-ot
         lines = file.getLines()
         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,
+        })
+      }
     }
     }
   )
   )
 }
 }

+ 80 - 2
services/document-updater/test/unit/js/HttpController/HttpControllerTests.js

@@ -19,6 +19,12 @@ describe('HttpController', function () {
         }),
         }),
         './Metrics': (this.Metrics = {}),
         './Metrics': (this.Metrics = {}),
         './Errors': Errors,
         './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 },
         '@overleaf/settings': { max_doc_length: 2 * 1024 * 1024 },
       },
       },
     })
     })
@@ -95,7 +101,11 @@ describe('HttpController', function () {
       it('should log the request', function () {
       it('should log the request', function () {
         this.logger.debug
         this.logger.debug
           .calledWith(
           .calledWith(
-            { docId: this.doc_id, projectId: this.project_id },
+            {
+              docId: this.doc_id,
+              projectId: this.project_id,
+              historyRanges: false,
+            },
             'getting doc via http'
             'getting doc via http'
           )
           )
           .should.equal(true)
           .should.equal(true)
@@ -147,7 +157,75 @@ describe('HttpController', function () {
       it('should log the request', function () {
       it('should log the request', function () {
         this.logger.debug
         this.logger.debug
           .calledWith(
           .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'
             'getting doc via http'
           )
           )
           .should.equal(true)
           .should.equal(true)

+ 25 - 0
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) {
 function setDocument(projectId, docId, userId, docLines, source, callback) {
   _makeRequest(
   _makeRequest(
     {
     {
@@ -661,6 +684,7 @@ module.exports = {
   blockProject,
   blockProject,
   unblockProject,
   unblockProject,
   updateProjectStructure,
   updateProjectStructure,
+  getDocumentWithHistoryRanges,
   promises: {
   promises: {
     flushProjectToMongo: promisify(flushProjectToMongo),
     flushProjectToMongo: promisify(flushProjectToMongo),
     flushMultipleProjectsToMongo: promisify(flushMultipleProjectsToMongo),
     flushMultipleProjectsToMongo: promisify(flushMultipleProjectsToMongo),
@@ -688,5 +712,6 @@ module.exports = {
     unblockProject: promisify(unblockProject),
     unblockProject: promisify(unblockProject),
     updateProjectStructure: promisify(updateProjectStructure),
     updateProjectStructure: promisify(updateProjectStructure),
     appendToDocument: promisify(appendToDocument),
     appendToDocument: promisify(appendToDocument),
+    getDocumentWithHistoryRanges: promisify(getDocumentWithHistoryRanges),
   },
   },
 }
 }