Procházet zdrojové kódy

Send origin metadata through docupdater and project-history when restoring files (#18721)

* add RestoreFileOrigin in overleaf-editor-core

* support source to be an object

* use sourceOrOrigin as param

* rename to originOrSource so the priority is more clear

* get timestamp from version

* fix test

* include version and min_count in getUpdatesFromHistory

* extractOriginOrSource util function

* fix RestoreManagerTests

GitOrigin-RevId: 0ace05a6ade2794c753a9d0bffb4f858ecc6899a
Domagoj Kriskovic před 2 roky
rodič
revize
7e8e2b0585

+ 2 - 0
libraries/overleaf-editor-core/index.js

@@ -22,6 +22,7 @@ const SetFileMetadataOperation = require('./lib/operation/set_file_metadata_oper
 const NoOperation = require('./lib/operation/no_operation')
 const Operation = require('./lib/operation')
 const RestoreOrigin = require('./lib/origin/restore_origin')
+const RestoreFileOrigin = require('./lib/origin/restore_file_origin')
 const Origin = require('./lib/origin')
 const OtClient = require('./lib/ot_client')
 const TextOperation = require('./lib/operation/text_operation')
@@ -66,6 +67,7 @@ exports.SetFileMetadataOperation = SetFileMetadataOperation
 exports.NoOperation = NoOperation
 exports.Operation = Operation
 exports.RestoreOrigin = RestoreOrigin
+exports.RestoreFileOrigin = RestoreFileOrigin
 exports.Origin = Origin
 exports.OtClient = OtClient
 exports.TextOperation = TextOperation

+ 4 - 0
libraries/overleaf-editor-core/lib/origin/index.js

@@ -5,6 +5,7 @@ const assert = require('check-types').assert
 // Dependencies are loaded at the bottom of the file to mitigate circular
 // dependency
 let RestoreOrigin = null
+let RestoreFileOrigin = null
 
 /**
  * An Origin records where a {@link Change} came from. The Origin class handles
@@ -31,6 +32,8 @@ class Origin {
   static fromRaw(raw) {
     if (!raw) return null
     if (raw.kind === RestoreOrigin.KIND) return RestoreOrigin.fromRaw(raw)
+    if (raw.kind === RestoreFileOrigin.KIND)
+      return RestoreFileOrigin.fromRaw(raw)
     return new Origin(raw.kind)
   }
 
@@ -54,3 +57,4 @@ class Origin {
 module.exports = Origin
 
 RestoreOrigin = require('./restore_origin')
+RestoreFileOrigin = require('./restore_file_origin')

+ 62 - 0
libraries/overleaf-editor-core/lib/origin/restore_file_origin.js

@@ -0,0 +1,62 @@
+'use strict'
+
+const assert = require('check-types').assert
+
+const Origin = require('.')
+
+class RestoreFileOrigin extends Origin {
+  /**
+   * @param {number} version that was restored
+   * @param {string} path that was restored
+   * @param {Date} timestamp from the restored version
+   */
+  constructor(version, path, timestamp) {
+    assert.integer(version, 'RestoreFileOrigin: bad version')
+    assert.string(path, 'RestoreFileOrigin: bad path')
+    assert.date(timestamp, 'RestoreFileOrigin: bad timestamp')
+
+    super(RestoreFileOrigin.KIND)
+    this.version = version
+    this.path = path
+    this.timestamp = timestamp
+  }
+
+  static fromRaw(raw) {
+    return new RestoreFileOrigin(raw.version, raw.path, new Date(raw.timestamp))
+  }
+
+  /** @inheritdoc */
+  toRaw() {
+    return {
+      kind: RestoreFileOrigin.KIND,
+      version: this.version,
+      path: this.path,
+      timestamp: this.timestamp.toISOString(),
+    }
+  }
+
+  /**
+   * @return {number}
+   */
+  getVersion() {
+    return this.version
+  }
+
+  /**
+   * @return {string}
+   */
+  getPath() {
+    return this.path
+  }
+
+  /**
+   * @return {Date}
+   */
+  getTimestamp() {
+    return this.timestamp
+  }
+}
+
+RestoreFileOrigin.KIND = 'file-restore'
+
+module.exports = RestoreFileOrigin

+ 25 - 0
libraries/overleaf-editor-core/test/change.test.js

@@ -34,4 +34,29 @@ describe('Change', function () {
       expect(blobHashes.has(File.EMPTY_FILE_HASH)).to.be.true
     })
   })
+
+  describe('RestoreFileOrigin', function () {
+    it('should convert to and from raw', function () {
+      const origin = new core.RestoreFileOrigin(1, 'path', new Date())
+      const raw = origin.toRaw()
+      const newOrigin = core.Origin.fromRaw(raw)
+      expect(newOrigin).to.eql(origin)
+    })
+
+    it('change should have a correct origin class', function () {
+      const change = Change.fromRaw({
+        operations: [],
+        timestamp: '2015-03-05T12:03:53.035Z',
+        authors: [null],
+        origin: {
+          kind: 'file-restore',
+          version: 1,
+          path: 'path',
+          timestamp: '2015-03-05T12:03:53.035Z',
+        },
+      })
+
+      expect(change.getOrigin()).to.be.an.instanceof(core.RestoreFileOrigin)
+    })
+  })
 })

+ 10 - 2
services/document-updater/app/js/DocumentManager.js

@@ -8,6 +8,7 @@ const Metrics = require('./Metrics')
 const HistoryManager = require('./HistoryManager')
 const Errors = require('./Errors')
 const RangesManager = require('./RangesManager')
+const { extractOriginOrSource } = require('./Utils')
 
 const MAX_UNFLUSHED_AGE = 300 * 1000 // 5 mins, document should be flushed to mongo this time after a change
 
@@ -111,7 +112,7 @@ const DocumentManager = {
     }
   },
 
-  async setDoc(projectId, docId, newLines, source, userId, undoing) {
+  async setDoc(projectId, docId, newLines, originOrSource, userId, undoing) {
     if (newLines == null) {
       throw new Error('No lines were provided to setDoc')
     }
@@ -141,16 +142,23 @@ const DocumentManager = {
         o.u = true
       } // Turn on undo flag for each op for track changes
     }
+
+    const { origin, source } = extractOriginOrSource(originOrSource)
+
     const update = {
       doc: docId,
       op,
       v: version,
       meta: {
         type: 'external',
-        source,
         user_id: userId,
       },
     }
+    if (origin) {
+      update.meta.origin = origin
+    } else if (source) {
+      update.meta.source = source
+    }
     // Keep track of external updates, whether they are for live documents
     // (flush) or unloaded documents (evict), and whether the update is a no-op.
     Metrics.inc('external-update', 1, {

+ 21 - 5
services/document-updater/app/js/ProjectHistoryRedisManager.js

@@ -9,7 +9,7 @@ const rclient = require('@overleaf/redis-wrapper').createClient(
 const logger = require('@overleaf/logger')
 const metrics = require('./Metrics')
 const { docIsTooLarge } = require('./Limits')
-const { addTrackedDeletesToContent } = require('./Utils')
+const { addTrackedDeletesToContent, extractOriginOrSource } = require('./Utils')
 const HistoryConversions = require('./HistoryConversions')
 const OError = require('@overleaf/o-error')
 
@@ -54,7 +54,7 @@ const ProjectHistoryRedisManager = {
     entityId,
     userId,
     projectUpdate,
-    source
+    originOrSource
   ) {
     projectUpdate = {
       pathname: projectUpdate.pathname,
@@ -67,7 +67,15 @@ const ProjectHistoryRedisManager = {
       projectHistoryId,
     }
     projectUpdate[entityType] = entityId
-    if (source != null) {
+
+    const { origin, source } = extractOriginOrSource(originOrSource)
+
+    if (origin != null) {
+      projectUpdate.meta.origin = origin
+      if (origin.kind !== 'editor') {
+        projectUpdate.meta.type = 'external'
+      }
+    } else if (source != null) {
       projectUpdate.meta.source = source
       if (source !== 'editor') {
         projectUpdate.meta.type = 'external'
@@ -90,7 +98,7 @@ const ProjectHistoryRedisManager = {
     entityId,
     userId,
     projectUpdate,
-    source
+    originOrSource
   ) {
     let docLines = projectUpdate.docLines
     let ranges
@@ -117,7 +125,15 @@ const ProjectHistoryRedisManager = {
       projectUpdate.ranges = ranges
     }
     projectUpdate[entityType] = entityId
-    if (source != null) {
+
+    const { origin, source } = extractOriginOrSource(originOrSource)
+
+    if (origin != null) {
+      projectUpdate.meta.origin = origin
+      if (origin.kind !== 'editor') {
+        projectUpdate.meta.type = 'external'
+      }
+    } else if (source != null) {
       projectUpdate.meta.source = source
       if (source !== 'editor') {
         projectUpdate.meta.type = 'external'

+ 18 - 0
services/document-updater/app/js/Utils.js

@@ -83,10 +83,28 @@ function addTrackedDeletesToContent(content, trackedChanges) {
   return result
 }
 
+/**
+ * checks if the given originOrSource should be treated as a source or origin
+ * TODO: remove this hack and remove all "source" references
+ */
+function extractOriginOrSource(originOrSource) {
+  let source = null
+  let origin = null
+
+  if (typeof originOrSource === 'string') {
+    source = originOrSource
+  } else if (originOrSource && typeof originOrSource === 'object') {
+    origin = originOrSource
+  }
+
+  return { source, origin }
+}
+
 module.exports = {
   isInsert,
   isDelete,
   isComment,
   addTrackedDeletesToContent,
   getDocLength,
+  extractOriginOrSource,
 }

+ 22 - 4
services/web/app/src/Features/History/RestoreManager.js

@@ -48,7 +48,6 @@ const RestoreManager = {
   },
 
   async revertFile(userId, projectId, version, pathname) {
-    const source = 'file-revert'
     const fsPath = await RestoreManager._writeFileVersionToDisk(
       projectId,
       version,
@@ -71,6 +70,19 @@ const RestoreManager = {
       })
       .catch(() => null)
 
+    const updates = await RestoreManager._getUpdatesFromHistory(
+      projectId,
+      version
+    )
+    const updateAtVersion = updates.find(update => update.toV === version)
+
+    const origin = {
+      kind: 'file-restore',
+      path: pathname,
+      version,
+      timestamp: new Date(updateAtVersion.meta.end_ts).toISOString(),
+    }
+
     const importInfo = await FileSystemImportManager.promises.importFile(
       fsPath,
       pathname
@@ -82,7 +94,7 @@ const RestoreManager = {
         basename,
         fsPath,
         file?.element?.linkedFileData,
-        source,
+        origin,
         userId
       )
 
@@ -101,7 +113,7 @@ const RestoreManager = {
         projectId,
         file.element._id,
         importInfo.type,
-        'revert',
+        origin,
         userId
       )
     }
@@ -179,7 +191,7 @@ const RestoreManager = {
       basename,
       importInfo.lines,
       newRanges,
-      'revert',
+      origin,
       userId
     )
   },
@@ -226,6 +238,12 @@ const RestoreManager = {
     }/project/${projectId}/ranges/version/${version}/${encodeURIComponent(pathname)}`
     return await fetchJson(url)
   },
+
+  async _getUpdatesFromHistory(projectId, version) {
+    const url = `${Settings.apis.project_history.url}/project/${projectId}/updates?before=${version}&min_count=1`
+    const res = await fetchJson(url)
+    return res.updates
+  },
 }
 
 module.exports = { ...callbackifyAll(RestoreManager), promises: RestoreManager }

+ 2 - 1
services/web/frontend/js/features/history/services/types/shared.ts

@@ -12,7 +12,7 @@ export interface Meta {
   start_ts: number
   end_ts: number
   type?: 'external' // TODO
-  source?: 'git-bridge' | 'file-revert' // TODO
+  source?: 'git-bridge' // TODO
   origin?: {
     kind:
       | 'dropbox'
@@ -21,5 +21,6 @@ export interface Meta {
       | 'github'
       | 'history-resync'
       | 'history-migration'
+      | 'file-restore'
   }
 }

+ 1 - 1
services/web/frontend/js/features/ide-react/context/editor-manager-context.tsx

@@ -285,7 +285,7 @@ export const EditorManagerProvider: FC = ({ children }) => {
         ) {
           return
         }
-        if (update.meta.source === 'file-revert') {
+        if (update.meta.origin?.kind === 'file-restore') {
           return
         }
         showGenericMessageModal(

+ 19 - 2
services/web/test/unit/src/History/RestoreManagerTests.js

@@ -286,6 +286,9 @@ describe('RestoreManager', function () {
             changes: this.tracked_changes,
             comments: this.comments,
           })
+        this.RestoreManager.promises._getUpdatesFromHistory = sinon
+          .stub()
+          .resolves([{ toV: this.version, meta: { end_ts: Date.now() } }])
         this.EditorController.promises.addDocWithRanges = sinon
           .stub()
           .resolves(
@@ -360,7 +363,12 @@ describe('RestoreManager', function () {
             this.project_id,
             'mock-file-id',
             'doc',
-            'revert',
+            {
+              kind: 'file-restore',
+              path: this.pathname,
+              version: this.version,
+              timestamp: new Date().toISOString(),
+            },
             this.user_id
           )
         })
@@ -389,7 +397,13 @@ describe('RestoreManager', function () {
             this.folder_id,
             'foo.tex',
             ['foo', 'bar', 'baz'],
-            { changes: this.tracked_changes, comments: this.remappedComments }
+            { changes: this.tracked_changes, comments: this.remappedComments },
+            {
+              kind: 'file-restore',
+              path: this.pathname,
+              version: this.version,
+              timestamp: new Date().toISOString(),
+            }
           )
         })
 
@@ -418,6 +432,9 @@ describe('RestoreManager', function () {
         this.EditorController.promises.upsertFile = sinon
           .stub()
           .resolves({ _id: 'mock-file-id', type: 'file' })
+        this.RestoreManager.promises._getUpdatesFromHistory = sinon
+          .stub()
+          .resolves([{ toV: this.version, meta: { end_ts: Date.now() } }])
       })
 
       it('should return the created entity if file exists', async function () {