Merge pull request #22320 from overleaf/revert-22213-ar-avoid-duplicate-blob-writes

Revert "Avoid duplicate blob writes"

GitOrigin-RevId: 9f86bcea654cd3fa5f66fbdf42080e7f6e2861a7

services/document-updater/app/js/ProjectHistoryRedisManager.js

@@ -122,7 +122,6 @@ const ProjectHistoryRedisManager = {
       hash: projectUpdate.hash,
       metadata: projectUpdate.metadata,
       projectHistoryId,
-      createdBlob: projectUpdate.createdBlob ?? false,
     }
     if (ranges) {
       projectUpdate.ranges = ranges

services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js

@@ -162,7 +162,6 @@ describe('ProjectHistoryRedisManager', function () {
         },
         version: this.version,
         projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
         doc: this.doc_id,
       }
 
@@ -208,7 +207,6 @@ describe('ProjectHistoryRedisManager', function () {
         hash: '1337',
         metadata,
         projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
         file: fileId,
       }
 
@@ -293,7 +291,6 @@ describe('ProjectHistoryRedisManager', function () {
         },
         version: this.version,
         projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
         ranges: historyCompatibleRanges,
         doc: this.doc_id,
       }
@@ -346,7 +343,6 @@ describe('ProjectHistoryRedisManager', function () {
         },
         version: this.version,
         projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
         doc: this.doc_id,
       }
 
@@ -398,68 +394,6 @@ describe('ProjectHistoryRedisManager', function () {
         },
         version: this.version,
         projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
-        doc: this.doc_id,
-      }
-
-      this.ProjectHistoryRedisManager.promises.queueOps
-        .calledWithExactly(this.project_id, JSON.stringify(update))
-        .should.equal(true)
-    })
-
-    it('should pass "false" as the createdBlob field if not provided', async function () {
-      await this.ProjectHistoryRedisManager.promises.queueAddEntity(
-        this.project_id,
-        this.projectHistoryId,
-        'doc',
-        this.doc_id,
-        this.user_id,
-        this.rawUpdate,
-        this.source
-      )
-
-      const update = {
-        pathname: this.pathname,
-        docLines: this.docLines,
-        meta: {
-          user_id: this.user_id,
-          ts: new Date(),
-          source: this.source,
-        },
-        version: this.version,
-        projectHistoryId: this.projectHistoryId,
-        createdBlob: false,
-        doc: this.doc_id,
-      }
-
-      this.ProjectHistoryRedisManager.promises.queueOps
-        .calledWithExactly(this.project_id, JSON.stringify(update))
-        .should.equal(true)
-    })
-
-    it('should pass through the value of the createdBlob field', async function () {
-      this.rawUpdate.createdBlob = true
-      await this.ProjectHistoryRedisManager.promises.queueAddEntity(
-        this.project_id,
-        this.projectHistoryId,
-        'doc',
-        this.doc_id,
-        this.user_id,
-        this.rawUpdate,
-        this.source
-      )
-
-      const update = {
-        pathname: this.pathname,
-        docLines: this.docLines,
-        meta: {
-          user_id: this.user_id,
-          ts: new Date(),
-          source: this.source,
-        },
-        version: this.version,
-        projectHistoryId: this.projectHistoryId,
-        createdBlob: true,
         doc: this.doc_id,
       }
 

services/project-history/app/js/HistoryStoreManager.js

@@ -300,18 +300,8 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
     if (urlMatch[1] !== projectId) {
       return callback(new OError('invalid project for blob creation'))
     }
-
     const fileId = urlMatch[2]
     const filestoreURL = `${Settings.apis.filestore.url}/project/${projectId}/file/${fileId}`
-
-    if (update.createdBlob) {
-      logger.debug(
-        { projectId, fileId },
-        'Skipping blob creation as it has already been created'
-      )
-      return callback(null, { file: update.hash })
-    }
-
     fetchStream(filestoreURL, {
       signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT),
     })

services/project-history/app/js/types.ts

@@ -76,7 +76,6 @@ export type AddFileUpdate = ProjectUpdateBase & {
   file: string
   url: string
   hash: string
-  createdBlob?: boolean
   metadata?: LinkedFileData
 }
 

services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js

@@ -491,44 +491,6 @@ describe('HistoryStoreManager', function () {
         expect(this.actualHash).to.equal(this.hash)
       })
     })
-    describe('when the createdBlob flag is set on the update', function () {
-      beforeEach(function (done) {
-        this.file_id = '012345678901234567890123'
-        this.update = {
-          file: true,
-          createdBlob: true,
-          url: `http://filestore.other.cloud.provider/project/${this.projectId}/file/${this.file_id}`,
-          hash: this.hash,
-        }
-        this.HistoryStoreManager.createBlobForUpdate(
-          this.projectId,
-          this.historyId,
-          this.update,
-          (err, { file: hash }) => {
-            if (err) {
-              return done(err)
-            }
-            this.actualHash = hash
-            done()
-          }
-        )
-      })
-
-      it('should call the callback with the existing hash', function () {
-        expect(this.actualHash).to.equal(this.hash)
-      })
-
-      it('should not request the file from the filestore', function () {
-        expect(this.FetchUtils.fetchStream).to.not.have.been.called
-      })
-
-      it('should log a debug level message', function () {
-        expect(this.logger.debug).to.have.been.calledWith(
-          { projectId: this.projectId, fileId: this.file_id },
-          'Skipping blob creation as it has already been created'
-        )
-      })
-    })
   })
 
   describe('getProjectBlob', function () {

services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js

@@ -490,7 +490,6 @@ function _getUpdates(
         url: newEntity.url,
         hash: newEntity.file != null ? newEntity.file.hash : undefined,
         metadata: buildFileMetadataForHistory(newEntity.file),
-        createdBlob: newEntity.createdBlob ?? false,
       })
     } else if (newEntity.path !== oldEntity.path) {
       // entity renamed

services/web/app/src/Features/FileStore/FileStoreHandler.js

@@ -100,7 +100,7 @@ const FileStoreHandler = {
                   })
                   return callback(err)
                 }
-                callback(err, result.url, result.fileRef, true)
+                callback(err, result.url, result.fileRef)
               }
             )
           }
@@ -303,6 +303,6 @@ module.exports = FileStoreHandler
 module.exports.promises = promisifyAll(FileStoreHandler, {
   multiResult: {
     uploadFileFromDisk: ['url', 'fileRef'],
-    uploadFileFromDiskWithHistoryId: ['url', 'fileRef', 'createdBlob'],
+    uploadFileFromDiskWithHistoryId: ['url', 'fileRef'],
   },
 })

services/web/app/src/Features/Project/ProjectDuplicator.js

@@ -211,7 +211,6 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
       if (sourceFile.hash != null) {
         file.hash = sourceFile.hash
       }
-      let createdBlob = false
       if (file.hash != null) {
         try {
           await HistoryManager.promises.copyBlob(
@@ -219,7 +218,6 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
             targetHistoryId,
             file.hash
           )
-          createdBlob = true
         } catch (err) {
           logger.error(
             {
@@ -239,7 +237,7 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
         targetProject._id,
         file._id
       )
-      return { createdBlob, file, path, url }
+      return { file, path, url }
     }
   )
   return targetEntries

services/web/app/src/Features/Uploads/ProjectUploadManager.js

@@ -194,14 +194,14 @@ async function _createFile(project, projectPath, fsPath) {
     throw new OError('missing history id')
   }
   const fileName = Path.basename(projectPath)
-  const { createdBlob, fileRef, url } =
+  const { fileRef, url } =
     await FileStoreHandler.promises.uploadFileFromDiskWithHistoryId(
       projectId,
       historyId,
       { name: fileName },
       fsPath
     )
-  return { createdBlob, file: fileRef, path: projectPath, url }
+  return { file: fileRef, path: projectPath, url }
 }
 
 async function _notifyDocumentUpdater(project, userId, changes) {

services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js

@@ -1132,7 +1132,6 @@ describe('DocumentUpdaterHandler', function () {
               hash: undefined,
               ranges: undefined,
               metadata: undefined,
-              createdBlob: false,
             },
           ]
 
@@ -1185,7 +1184,6 @@ describe('DocumentUpdaterHandler', function () {
               historyRangesSupport: false,
               hash: '12345',
               ranges: undefined,
-              createdBlob: false,
               metadata: undefined,
             },
           ]
@@ -1298,7 +1296,6 @@ describe('DocumentUpdaterHandler', function () {
               hash: undefined,
               ranges: undefined,
               metadata: undefined,
-              createdBlob: false,
             },
           ]
 
@@ -1401,7 +1398,6 @@ describe('DocumentUpdaterHandler', function () {
               hash: undefined,
               ranges: this.ranges,
               metadata: undefined,
-              createdBlob: false,
             },
           ]
 
@@ -1448,7 +1444,6 @@ describe('DocumentUpdaterHandler', function () {
               hash: undefined,
               ranges: this.ranges,
               metadata: undefined,
-              createdBlob: false,
             },
           ]
 

services/web/test/unit/src/Project/ProjectDuplicatorTests.js

@@ -105,19 +105,16 @@ describe('ProjectDuplicator', function () {
     ]
     this.fileEntries = [
       {
-        createdBlob: false,
         path: this.file0Path,
         file: this.newFile0,
         url: this.filestoreUrl,
       },
       {
-        createdBlob: false,
         path: this.file1Path,
         file: this.newFile1,
         url: this.filestoreUrl,
       },
       {
-        createdBlob: true,
         path: this.file2Path,
         file: this.newFile2,
         url: this.filestoreUrl,

services/web/test/unit/src/Uploads/ProjectUploadManagerTests.js

@@ -58,12 +58,7 @@ describe('ProjectUploadManager', function () {
       },
     ]
     this.fileEntries = [
-      {
-        file: this.file,
-        path: `/${this.file.name}`,
-        url: this.fileStoreUrl,
-        createdBlob: true,
-      },
+      { file: this.file, path: `/${this.file.name}`, url: this.fileStoreUrl },
     ]
 
     this.fs = {
@@ -99,11 +94,9 @@ describe('ProjectUploadManager', function () {
     }
     this.FileStoreHandler = {
       promises: {
-        uploadFileFromDiskWithHistoryId: sinon.stub().resolves({
-          fileRef: this.file,
-          url: this.fileStoreUrl,
-          createdBlob: true,
-        }),
+        uploadFileFromDiskWithHistoryId: sinon
+          .stub()
+          .resolves({ fileRef: this.file, url: this.fileStoreUrl }),
       },
     }
     this.FileSystemImportManager = {