Kaynağa Gözat

[project-history] forward all the optional file attributes during resync (#23260)

Notably, 'createdBlob' was missing. And others like metadata should not
be added if not set.

GitOrigin-RevId: c4a6746c4824d637f7c54b4c68a7025b60c15ff2
Jakob Ackermann 1 yıl önce
ebeveyn
işleme
72b4083318

+ 8 - 6
services/project-history/app/js/SyncManager.js

@@ -530,9 +530,10 @@ class SyncUpdateExpander {
         this.files[update.pathname] = File.fromString('')
       } else {
         update.file = entity.file
-        update.url = entity.url
-        update.hash = entity._hash
-        update.metadata = entity.metadata
+        if (entity.url) update.url = entity.url
+        if (entity._hash) update.hash = entity._hash
+        if (entity.createdBlob) update.createdBlob = entity.createdBlob
+        if (entity.metadata) update.metadata = entity.metadata
       }
 
       this.expandedUpdates.push(update)
@@ -619,10 +620,11 @@ class SyncUpdateExpander {
           ts: update.meta.ts,
         },
         file: entity.file,
-        url: entity.url,
-        hash: entity._hash,
-        metadata: entity.metadata,
       }
+      if (entity.url) addUpdate.url = entity.url
+      if (entity._hash) addUpdate.hash = entity._hash
+      if (entity.createdBlob) addUpdate.createdBlob = entity.createdBlob
+      if (entity.metadata) addUpdate.metadata = entity.metadata
       this.expandedUpdates.push(addUpdate)
       Metrics.inc('project_history_resync_operation', 1, {
         status: 'update binary file contents',

+ 2 - 1
services/project-history/app/js/types.ts

@@ -212,7 +212,8 @@ export type File = {
   file: string
   url?: string
   path: string
-  _hash: string
+  _hash?: string
+  createdBlob?: boolean
   metadata?: LinkedFileData
 }
 

+ 222 - 0
services/project-history/test/acceptance/js/SyncTests.js

@@ -622,6 +622,110 @@ describe('Syncing with web and doc-updater', function () {
             }
           )
         })
+        it('should add file w/o url', function (done) {
+          MockHistoryStore()
+            .get(`/api/projects/${historyId}/latest/history`)
+            .reply(200, {
+              chunk: {
+                history: {
+                  snapshot: {
+                    files: {
+                      persistedFile: { hash: EMPTY_FILE_HASH, byteLength: 0 },
+                    },
+                  },
+                  changes: [],
+                },
+                startVersion: 0,
+              },
+            })
+
+          const fileContents = Buffer.from([1, 2, 3])
+          const fileHash = 'aed2973e4b8a7ff1b30ff5c4751e5a2b38989e74'
+
+          MockFileStore()
+            .get(`/project/${this.project_id}/file/${this.file_id}`)
+            .reply(200, fileContents)
+          const headBlob = MockHistoryStore()
+            .head(`/api/projects/${historyId}/blobs/${fileHash}`)
+            .reply(200)
+          const createBlob = MockHistoryStore()
+            .put(`/api/projects/${historyId}/blobs/${fileHash}`, fileContents)
+            .reply(201)
+
+          const addFile = MockHistoryStore()
+            .post(`/api/projects/${historyId}/legacy_changes`, body => {
+              expect(body).to.deep.equal([
+                {
+                  v2Authors: [],
+                  authors: [],
+                  timestamp: this.timestamp.toJSON(),
+                  operations: [
+                    {
+                      pathname: 'test.png',
+                      file: {
+                        hash: fileHash,
+                      },
+                    },
+                  ],
+                  origin: { kind: 'test-origin' },
+                },
+              ])
+              return true
+            })
+            .query({ end_version: 0 })
+            .reply(204)
+
+          async.series(
+            [
+              cb => {
+                ProjectHistoryClient.resyncHistory(this.project_id, cb)
+              },
+              cb => {
+                const update = {
+                  projectHistoryId: historyId,
+                  resyncProjectStructure: {
+                    docs: [],
+                    files: [
+                      {
+                        file: this.file_id,
+                        path: '/test.png',
+                        _hash: fileHash,
+                        createdBlob: true,
+                      },
+                      { path: '/persistedFile' },
+                    ],
+                  },
+                  meta: {
+                    ts: this.timestamp,
+                  },
+                }
+                ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb)
+              },
+              cb => {
+                ProjectHistoryClient.flushProject(this.project_id, cb)
+              },
+            ],
+            error => {
+              if (error) {
+                throw error
+              }
+              assert(!loggerWarn.called, 'no warning logged on 404')
+              assert(
+                headBlob.isDone(),
+                'HEAD /api/projects/:historyId/blobs/:hash should have been called'
+              )
+              assert(
+                !createBlob.isDone(),
+                '/api/projects/:historyId/blobs/:hash should have been skipped'
+              )
+              assert(
+                addFile.isDone(),
+                `/api/projects/${historyId}/changes should have been called`
+              )
+              done()
+            }
+          )
+        })
         describe('with filestore disabled', function () {
           before(function () {
             Settings.apis.filestore.enabled = false
@@ -760,6 +864,124 @@ describe('Syncing with web and doc-updater', function () {
         })
       })
 
+      describe('when a file hash mismatches', function () {
+        it('should remove and re-add file w/o url', function (done) {
+          MockHistoryStore()
+            .get(`/api/projects/${historyId}/latest/history`)
+            .reply(200, {
+              chunk: {
+                history: {
+                  snapshot: {
+                    files: {
+                      'test.png': { hash: EMPTY_FILE_HASH, byteLength: 0 },
+                    },
+                  },
+                  changes: [],
+                },
+                startVersion: 0,
+              },
+            })
+
+          const fileContents = Buffer.from([1, 2, 3])
+          const fileHash = 'aed2973e4b8a7ff1b30ff5c4751e5a2b38989e74'
+
+          MockFileStore()
+            .get(`/project/${this.project_id}/file/${this.file_id}`)
+            .reply(200, fileContents)
+          const headBlob = MockHistoryStore()
+            .head(`/api/projects/${historyId}/blobs/${fileHash}`)
+            .reply(200)
+          const createBlob = MockHistoryStore()
+            .put(`/api/projects/${historyId}/blobs/${fileHash}`, fileContents)
+            .reply(201)
+
+          const addFile = MockHistoryStore()
+            .post(`/api/projects/${historyId}/legacy_changes`, body => {
+              expect(body).to.deep.equal([
+                {
+                  v2Authors: [],
+                  authors: [],
+                  timestamp: this.timestamp.toJSON(),
+                  operations: [
+                    {
+                      pathname: 'test.png',
+                      newPathname: '',
+                    },
+                  ],
+                  origin: { kind: 'test-origin' },
+                },
+                {
+                  v2Authors: [],
+                  authors: [],
+                  timestamp: this.timestamp.toJSON(),
+                  operations: [
+                    {
+                      pathname: 'test.png',
+                      file: {
+                        hash: fileHash,
+                      },
+                    },
+                  ],
+                  origin: { kind: 'test-origin' },
+                },
+              ])
+              return true
+            })
+            .query({ end_version: 0 })
+            .reply(204)
+
+          async.series(
+            [
+              cb => {
+                ProjectHistoryClient.resyncHistory(this.project_id, cb)
+              },
+              cb => {
+                const update = {
+                  projectHistoryId: historyId,
+                  resyncProjectStructure: {
+                    docs: [],
+                    files: [
+                      {
+                        file: this.file_id,
+                        path: '/test.png',
+                        _hash: fileHash,
+                        createdBlob: true,
+                      },
+                    ],
+                  },
+                  meta: {
+                    ts: this.timestamp,
+                  },
+                }
+                ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb)
+              },
+              cb => {
+                ProjectHistoryClient.flushProject(this.project_id, cb)
+              },
+            ],
+            error => {
+              if (error) {
+                throw error
+              }
+              assert(!loggerWarn.called, 'no warning logged on 404')
+              assert(
+                headBlob.isDone(),
+                'HEAD /api/projects/:historyId/blobs/:hash should have been called'
+              )
+              assert(
+                !createBlob.isDone(),
+                '/api/projects/:historyId/blobs/:hash should have been skipped'
+              )
+              assert(
+                addFile.isDone(),
+                `/api/projects/${historyId}/changes should have been called`
+              )
+              done()
+            }
+          )
+        })
+      })
+
       describe("when a file exists which shouldn't", function () {
         it('should send remove file updates to the history store', function (done) {
           MockHistoryStore()

+ 83 - 3
services/project-history/test/unit/js/SyncManager/SyncManagerTests.js

@@ -631,7 +631,43 @@ describe('SyncManager', function () {
             file: newFile.file,
             url: newFile.url,
             hash: 'hash-42',
-            metadata: undefined,
+            meta: {
+              resync: true,
+              ts: TIMESTAMP,
+              origin: { kind: 'history-resync' },
+            },
+          },
+        ])
+        expect(this.extendLock).to.have.been.called
+      })
+
+      it('queues file additions for missing regular files w/o url', async function () {
+        const newFile = {
+          path: '2.png',
+          file: {},
+          _hash: 'hash-42',
+          createdBlob: true,
+        }
+        const updates = [
+          resyncProjectStructureUpdate(
+            [this.persistedDoc],
+            [this.persistedFile, newFile]
+          ),
+        ]
+        const expandedUpdates =
+          await this.SyncManager.promises.expandSyncUpdates(
+            this.projectId,
+            this.historyId,
+            updates,
+            this.extendLock
+          )
+
+        expect(expandedUpdates).to.deep.equal([
+          {
+            pathname: newFile.path,
+            file: newFile.file,
+            hash: 'hash-42',
+            createdBlob: true,
             meta: {
               resync: true,
               ts: TIMESTAMP,
@@ -757,7 +793,6 @@ describe('SyncManager', function () {
             file: fileWichWasADoc.file,
             url: fileWichWasADoc.url,
             hash: 'other-hash',
-            metadata: undefined,
             meta: {
               resync: true,
               ts: TIMESTAMP,
@@ -1035,7 +1070,52 @@ describe('SyncManager', function () {
             file: persistedFileWithNewContent.file,
             url: persistedFileWithNewContent.url,
             hash: 'anotherhashvalue',
-            metadata: undefined,
+            meta: {
+              resync: true,
+              ts: TIMESTAMP,
+              origin: { kind: 'history-resync' },
+            },
+          },
+        ])
+        expect(this.extendLock).to.have.been.called
+      })
+
+      it('removes and re-adds binary files w/o url if they do not have same hash', async function () {
+        const persistedFileWithNewContent = {
+          _hash: 'anotherhashvalue',
+          hello: 'world',
+          path: '1.png',
+          createdBlob: true,
+        }
+        const updates = [
+          resyncProjectStructureUpdate(
+            [this.persistedDoc],
+            [persistedFileWithNewContent]
+          ),
+        ]
+        const expandedUpdates =
+          await this.SyncManager.promises.expandSyncUpdates(
+            this.projectId,
+            this.historyId,
+            updates,
+            this.extendLock
+          )
+
+        expect(expandedUpdates).to.deep.equal([
+          {
+            pathname: persistedFileWithNewContent.path,
+            new_pathname: '',
+            meta: {
+              resync: true,
+              ts: TIMESTAMP,
+              origin: { kind: 'history-resync' },
+            },
+          },
+          {
+            pathname: persistedFileWithNewContent.path,
+            file: persistedFileWithNewContent.file,
+            hash: 'anotherhashvalue',
+            createdBlob: true,
             meta: {
               resync: true,
               ts: TIMESTAMP,