Просмотр исходного кода

Merge pull request #14135 from overleaf/jpa-web-tpds-no-filewriter

[web] remove FileWriter dependency in tpds update

GitOrigin-RevId: 7b7b31d919e61631fdeaf4909637ab2181848060
Jakob Ackermann 3 лет назад
Родитель
Сommit
595f59201c

+ 24 - 6
services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js

@@ -1,17 +1,17 @@
 const { callbackify } = require('util')
 const _ = require('underscore')
 const fsPromises = require('fs/promises')
+const fs = require('fs')
 const logger = require('@overleaf/logger')
 const EditorController = require('../Editor/EditorController')
 const FileTypeManager = require('../Uploads/FileTypeManager')
-const FileWriter = require('../../infrastructure/FileWriter')
 const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
+const crypto = require('crypto')
+const Settings = require('@overleaf/settings')
+const { pipeline } = require('stream/promises')
 
 async function mergeUpdate(userId, projectId, path, updateRequest, source) {
-  const fsPath = await FileWriter.promises.writeStreamToDisk(
-    projectId,
-    updateRequest
-  )
+  const fsPath = await writeUpdateToDisk(projectId, updateRequest)
   try {
     const metadata = await _mergeUpdate(userId, projectId, path, fsPath, source)
     return metadata
@@ -19,9 +19,27 @@ async function mergeUpdate(userId, projectId, path, updateRequest, source) {
     try {
       await fsPromises.unlink(fsPath)
     } catch (err) {
-      logger.err({ projectId, fsPath }, 'error deleting file')
+      logger.err({ err, projectId, fsPath }, 'error deleting file')
+    }
+  }
+}
+
+async function writeUpdateToDisk(projectId, updateStream) {
+  const fsPath = `${
+    Settings.path.dumpFolder
+  }/${projectId}_${crypto.randomUUID()}`
+  const writeStream = fs.createWriteStream(fsPath)
+  try {
+    await pipeline(updateStream, writeStream)
+  } catch (err) {
+    try {
+      await fsPromises.unlink(fsPath)
+    } catch (err) {
+      logger.error({ err, projectId, fsPath }, 'error deleting file')
     }
+    throw err
   }
+  return fsPath
 }
 
 async function _findExistingFileType(projectId, path) {

+ 17 - 7
services/web/test/unit/src/ThirdPartyDataStore/UpdateMergerTests.js

@@ -11,6 +11,8 @@ describe('UpdateMerger :', function () {
   beforeEach(function () {
     this.projectId = 'project_id_here'
     this.userId = 'mock-user-id'
+    this.randomUUID = 'random-uuid'
+    this.dumpPath = '/dump'
 
     this.docPath = this.newDocPath = '/folder/doc.tex'
     this.filePath = this.newFilePath = '/folder/file.png'
@@ -23,7 +25,7 @@ describe('UpdateMerger :', function () {
     this.existingDocs = [{ path: '/main.tex' }, { path: '/folder/other.tex' }]
     this.existingFiles = [{ path: '/figure.pdf' }, { path: '/folder/fig1.pdf' }]
 
-    this.fsPath = '/tmp/file/path'
+    this.fsPath = `${this.dumpPath}/${this.projectId}_${this.randomUUID}`
     this.fileContents = `\\documentclass{article}
 \\usepackage[utf8]{inputenc}
 
@@ -33,10 +35,16 @@ describe('UpdateMerger :', function () {
     this.docLines = this.fileContents.split('\n')
     this.source = 'dropbox'
     this.updateRequest = new Writable()
+    this.writeStream = new Writable()
 
     this.fsPromises = {
       unlink: sinon.stub().resolves(),
       readFile: sinon.stub().withArgs(this.fsPath).resolves(this.fileContents),
+      mkdir: sinon.stub().resolves(),
+    }
+
+    this.fs = {
+      createWriteStream: sinon.stub().returns(this.writeStream),
     }
 
     this.doc = {
@@ -71,10 +79,8 @@ describe('UpdateMerger :', function () {
       },
     }
 
-    this.FileWriter = {
-      promises: {
-        writeStreamToDisk: sinon.stub().resolves(this.fsPath),
-      },
+    this.crypto = {
+      randomUUID: sinon.stub().returns(this.randomUUID),
     }
 
     this.ProjectEntityHandler = {
@@ -86,16 +92,20 @@ describe('UpdateMerger :', function () {
       },
     }
 
-    this.Settings = { path: { dumpPath: 'dump_here' } }
+    this.Settings = { path: { dumpFolder: this.dumpPath } }
+
+    this.stream = { pipeline: sinon.stub().resolves() }
 
     this.UpdateMerger = SandboxedModule.require(MODULE_PATH, {
       requires: {
         'fs/promises': this.fsPromises,
+        fs: this.fs,
         '../Editor/EditorController': this.EditorController,
         '../Uploads/FileTypeManager': this.FileTypeManager,
-        '../../infrastructure/FileWriter': this.FileWriter,
         '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
         '@overleaf/settings': this.Settings,
+        'stream/promises': this.stream,
+        crypto: this.crypto,
       },
     })
   })