From 44e0742aa33b37bddafb8cd10f25f1ba999de1ac Mon Sep 17 00:00:00 2001
From: Brian Gough <brian.gough@overleaf.com>
Date: Fri, 14 May 2021 15:49:20 +0100
Subject: [PATCH 1/2] use fse.copy for performance

it uses the native fs.copyFile method
---
 services/clsi/app/js/UrlCache.js            | 26 +++++----------------
 services/clsi/test/unit/js/UrlCacheTests.js |  5 ++--
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/services/clsi/app/js/UrlCache.js b/services/clsi/app/js/UrlCache.js
index e8ee10dc67..90c6486f98 100644
--- a/services/clsi/app/js/UrlCache.js
+++ b/services/clsi/app/js/UrlCache.js
@@ -19,6 +19,7 @@ const UrlFetcher = require('./UrlFetcher')
 const Settings = require('settings-sharelatex')
 const crypto = require('crypto')
 const fs = require('fs')
+const fse = require('fs-extra')
 const logger = require('logger-sharelatex')
 const async = require('async')
 
@@ -35,8 +36,12 @@ module.exports = UrlCache = {
         if (error != null) {
           return callback(error)
         }
-        return UrlCache._copyFile(pathToCachedUrl, destPath, function (error) {
+        return fse.copy(pathToCachedUrl, destPath, function (error) {
           if (error != null) {
+            logger.error(
+              { err: error, from: pathToCachedUrl, to: destPath },
+              'error copying file from cache'
+            )
             return UrlCache._clearUrlDetails(project_id, url, () =>
               callback(error)
             )
@@ -163,25 +168,6 @@ module.exports = UrlCache = {
     )}`
   },
 
-  _copyFile(from, to, _callback) {
-    if (_callback == null) {
-      _callback = function (error) {}
-    }
-    const callbackOnce = function (error) {
-      if (error != null) {
-        logger.error({ err: error, from, to }, 'error copying file from cache')
-      }
-      _callback(error)
-      return (_callback = function () {})
-    }
-    const writeStream = fs.createWriteStream(to)
-    const readStream = fs.createReadStream(from)
-    writeStream.on('error', callbackOnce)
-    readStream.on('error', callbackOnce)
-    writeStream.on('close', callbackOnce)
-    return writeStream.on('open', () => readStream.pipe(writeStream))
-  },
-
   _clearUrlFromCache(project_id, url, callback) {
     if (callback == null) {
       callback = function (error) {}
diff --git a/services/clsi/test/unit/js/UrlCacheTests.js b/services/clsi/test/unit/js/UrlCacheTests.js
index 2b991245a9..7276b1325d 100644
--- a/services/clsi/test/unit/js/UrlCacheTests.js
+++ b/services/clsi/test/unit/js/UrlCacheTests.js
@@ -27,7 +27,8 @@ describe('UrlCache', function () {
         'settings-sharelatex': (this.Settings = {
           path: { clsiCacheDir: '/cache/dir' }
         }),
-        fs: (this.fs = {})
+        fs: (this.fs = {}),
+        'fs-extra': (this.fse = { copy: sinon.stub().yields() })
       }
     }))
   })
@@ -268,7 +269,7 @@ describe('UrlCache', function () {
     })
 
     it('should copy the file to the new location', function () {
-      return this.UrlCache._copyFile
+      return this.fse.copy
         .calledWith(this.cachePath, this.destPath)
         .should.equal(true)
     })

From 371de76a4a2c65bdc34b48b009cb2d0fd7e7347e Mon Sep 17 00:00:00 2001
From: Brian Gough <brian.gough@overleaf.com>
Date: Mon, 17 May 2021 10:54:11 +0100
Subject: [PATCH 2/2] use fs.copyFile instead of fse.copy in UrlCache module

---
 services/clsi/app/js/UrlCache.js            | 3 +--
 services/clsi/test/unit/js/UrlCacheTests.js | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/services/clsi/app/js/UrlCache.js b/services/clsi/app/js/UrlCache.js
index 90c6486f98..23afafaaa8 100644
--- a/services/clsi/app/js/UrlCache.js
+++ b/services/clsi/app/js/UrlCache.js
@@ -19,7 +19,6 @@ const UrlFetcher = require('./UrlFetcher')
 const Settings = require('settings-sharelatex')
 const crypto = require('crypto')
 const fs = require('fs')
-const fse = require('fs-extra')
 const logger = require('logger-sharelatex')
 const async = require('async')
 
@@ -36,7 +35,7 @@ module.exports = UrlCache = {
         if (error != null) {
           return callback(error)
         }
-        return fse.copy(pathToCachedUrl, destPath, function (error) {
+        return fs.copyFile(pathToCachedUrl, destPath, function (error) {
           if (error != null) {
             logger.error(
               { err: error, from: pathToCachedUrl, to: destPath },
diff --git a/services/clsi/test/unit/js/UrlCacheTests.js b/services/clsi/test/unit/js/UrlCacheTests.js
index 7276b1325d..40652c5899 100644
--- a/services/clsi/test/unit/js/UrlCacheTests.js
+++ b/services/clsi/test/unit/js/UrlCacheTests.js
@@ -27,8 +27,7 @@ describe('UrlCache', function () {
         'settings-sharelatex': (this.Settings = {
           path: { clsiCacheDir: '/cache/dir' }
         }),
-        fs: (this.fs = {}),
-        'fs-extra': (this.fse = { copy: sinon.stub().yields() })
+        fs: (this.fs = { copyFile: sinon.stub().yields() })
       }
     }))
   })
@@ -249,7 +248,6 @@ describe('UrlCache', function () {
     beforeEach(function () {
       this.cachePath = 'path/to/cached/url'
       this.destPath = 'path/to/destination'
-      this.UrlCache._copyFile = sinon.stub().callsArg(2)
       this.UrlCache._ensureUrlIsInCache = sinon
         .stub()
         .callsArgWith(3, null, this.cachePath)
@@ -269,7 +267,7 @@ describe('UrlCache', function () {
     })
 
     it('should copy the file to the new location', function () {
-      return this.fse.copy
+      return this.fs.copyFile
         .calledWith(this.cachePath, this.destPath)
         .should.equal(true)
     })