From adc47fd053eb351cc72b876dd8b61446cc9c7e81 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Tue, 31 Oct 2017 20:18:15 -0600 Subject: [PATCH] Use relative linking (#6011) * Use relative linking * Add copyFile method and tests Closes #5988 * Fix relative linking on Windows Hard links and junctions don't work with relative paths * Fix tests * Revert ghange to gitignore --- src/file.js | 91 ++++++++++++++++++++++++++++++++++++-------- src/meta/js.js | 26 ++++++------- test/file.js | 100 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 30 deletions(-) create mode 100644 test/file.js diff --git a/src/file.js b/src/file.js index ec86c624b1..c091da614f 100644 --- a/src/file.js +++ b/src/file.js @@ -12,35 +12,76 @@ var utils = require('./utils'); var file = module.exports; +/** + * Asynchronously copies `src` to `dest` + * @param {string} src - source filename to copy + * @param {string} dest - destination filename of the copy operation + * @param {function(Error): void} callback + */ +function copyFile(src, dest, callback) { + var calledBack = false; + + var read; + var write; + + function done(err) { + if (calledBack) { + return; + } + calledBack = true; + + if (err) { + if (read) { + read.destroy(); + } + if (write) { + write.destroy(); + } + } + + callback(err); + } + + read = fs.createReadStream(src); + read.on('error', done); + + write = fs.createWriteStream(dest); + write.on('error', done); + write.on('close', function () { + done(); + }); + + read.pipe(write); +} + +file.copyFile = (typeof fs.copyFile === 'function') ? fs.copyFile : copyFile; + file.saveFileToLocal = function (filename, folder, tempPath, callback) { /* * remarkable doesn't allow spaces in hyperlinks, once that's fixed, remove this. */ - filename = filename.split('.'); - filename.forEach(function (name, idx) { - filename[idx] = utils.slugify(name); - }); - filename = filename.join('.'); + filename = filename.split('.').map(function (name) { + return utils.slugify(name); + }).join('.'); var uploadPath = path.join(nconf.get('upload_path'), folder, filename); winston.verbose('Saving file ' + filename + ' to : ' + uploadPath); mkdirp(path.dirname(uploadPath), function (err) { if (err) { - callback(err); + return callback(err); } - var is = fs.createReadStream(tempPath); - var os = fs.createWriteStream(uploadPath); - is.on('end', function () { + file.copyFile(tempPath, uploadPath, function (err) { + if (err) { + return callback(err); + } + callback(null, { url: '/assets/uploads/' + folder + '/' + filename, path: uploadPath, }); }); - - os.on('error', callback); - is.pipe(os); }); }; @@ -125,15 +166,33 @@ file.delete = function (path) { } }; -file.link = function link(filePath, destPath, cb) { +file.link = function link(filePath, destPath, relative, callback) { + if (!callback) { + callback = relative; + relative = false; + } + + if (relative && process.platform !== 'win32') { + filePath = path.relative(path.dirname(destPath), filePath); + } + if (process.platform === 'win32') { - fs.link(filePath, destPath, cb); + fs.link(filePath, destPath, callback); } else { - fs.symlink(filePath, destPath, 'file', cb); + fs.symlink(filePath, destPath, 'file', callback); } }; -file.linkDirs = function linkDirs(sourceDir, destDir, callback) { +file.linkDirs = function linkDirs(sourceDir, destDir, relative, callback) { + if (!callback) { + callback = relative; + relative = false; + } + + if (relative && process.platform !== 'win32') { + sourceDir = path.relative(path.dirname(destDir), sourceDir); + } + var type = (process.platform === 'win32') ? 'junction' : 'dir'; fs.symlink(sourceDir, destDir, type, callback); }; diff --git a/src/meta/js.js b/src/meta/js.js index 8082be2412..cedb863b3b 100644 --- a/src/meta/js.js +++ b/src/meta/js.js @@ -88,6 +88,14 @@ JS.scripts = { }, }; +function linkIfLinux(srcPath, destPath, next) { + if (process.platform === 'win32') { + file.copyFile(srcPath, destPath, next); + } else { + file.link(srcPath, destPath, true, next); + } +} + var basePath = path.resolve(__dirname, '../..'); function minifyModules(modules, fork, callback) { @@ -120,7 +128,7 @@ function minifyModules(modules, fork, callback) { }, function (cb) { async.eachLimit(filtered.skip, 500, function (mod, next) { - file.link(mod.srcPath, mod.destPath, next); + linkIfLinux(mod.srcPath, mod.destPath, next); }, cb); }, ], callback); @@ -148,20 +156,10 @@ function linkModules(callback) { return next(err); } if (res.stats.isDirectory()) { - return file.linkDirs(srcPath, destPath, next); + return file.linkDirs(srcPath, destPath, true, next); } - if (process.platform === 'win32') { - fs.readFile(srcPath, function (err, file) { - if (err) { - return next(err); - } - - fs.writeFile(destPath, file, next); - }); - } else { - file.link(srcPath, destPath, next); - } + linkIfLinux(srcPath, destPath, next); }); }, callback); } @@ -267,7 +265,7 @@ JS.linkStatics = function (callback) { return next(err); } - file.linkDirs(sourceDir, destDir, next); + file.linkDirs(sourceDir, destDir, true, next); }); }, callback); }); diff --git a/test/file.js b/test/file.js new file mode 100644 index 0000000000..630076bda6 --- /dev/null +++ b/test/file.js @@ -0,0 +1,100 @@ +'use strict'; + +var assert = require('assert'); +var fs = require('fs'); +var path = require('path'); +var nconf = require('nconf'); + +var utils = require('../src/utils'); +var file = require('../src/file'); + +describe('file', function () { + var filename = utils.generateUUID() + '.png'; + var folder = 'files'; + var uploadPath = path.join(nconf.get('upload_path'), folder, filename); + var tempPath = path.join(__dirname, './files/test.png'); + + afterEach(function (done) { + fs.unlink(uploadPath, function () { + done(); + }); + }); + + describe('copyFile', function () { + it('should copy a file', function (done) { + file.copyFile(tempPath, uploadPath, function (err) { + assert.ifError(err); + + assert(file.existsSync(uploadPath)); + + var srcContent = fs.readFileSync(tempPath, 'utf8'); + var destContent = fs.readFileSync(uploadPath, 'utf8'); + + assert.strictEqual(srcContent, destContent); + done(); + }); + }); + + it('should override an existing file', function (done) { + fs.writeFileSync(uploadPath, 'hsdkjhgkjsfhkgj'); + + file.copyFile(tempPath, uploadPath, function (err) { + assert.ifError(err); + + assert(file.existsSync(uploadPath)); + + var srcContent = fs.readFileSync(tempPath, 'utf8'); + var destContent = fs.readFileSync(uploadPath, 'utf8'); + + assert.strictEqual(srcContent, destContent); + done(); + }); + }); + + it('should error if source file does not exist', function (done) { + file.copyFile(tempPath + '0000000000', uploadPath, function (err) { + assert(err); + assert.strictEqual(err.code, 'ENOENT'); + + done(); + }); + }); + + it('should error if existing file is read only', function (done) { + fs.writeFileSync(uploadPath, 'hsdkjhgkjsfhkgj'); + fs.chmodSync(uploadPath, '444'); + + file.copyFile(tempPath, uploadPath, function (err) { + assert(err); + assert(err.code === 'EPERM' || err.code === 'EACCES'); + + done(); + }); + }); + }); + + describe('saveFileToLocal', function () { + it('should work', function (done) { + file.saveFileToLocal(filename, folder, tempPath, function (err) { + assert.ifError(err); + + assert(file.existsSync(uploadPath)); + + var oldFile = fs.readFileSync(tempPath, 'utf8'); + var newFile = fs.readFileSync(uploadPath, 'utf8'); + assert.strictEqual(oldFile, newFile); + + done(); + }); + }); + + it('should error if source does not exist', function (done) { + file.saveFileToLocal(filename, folder, tempPath + '000000000', function (err) { + assert(err); + assert.strictEqual(err.code, 'ENOENT'); + + done(); + }); + }); + }); +});