refactor uploadFile

src/server/models/attachment.js

@@ -1,8 +1,11 @@
-module.exports = function(crowi) {
-  const debug = require('debug')('growi:models:attachment');
-  const mongoose = require('mongoose');
-  const ObjectId = mongoose.Schema.Types.ObjectId;
+const debug = require('debug')('growi:models:attachment');
+const logger = require('@alias/logger')('growi:models:attachment');
+const path = require('path');
+
+const mongoose = require('mongoose');
+const ObjectId = mongoose.Schema.Types.ObjectId;
 
+module.exports = function(crowi) {
   const fileUploader = require('../service/file-uploader')(crowi);
 
   let attachmentSchema;
@@ -37,10 +40,11 @@ module.exports = function(crowi) {
   attachmentSchema.statics.create = async function(pageId, user, fileStream, originalName, fileFormat, fileSize) {
     const Attachment = this;
 
-    const fileName = generateFileHash(originalName);
-
-    // upload file
-    await fileUploader.uploadFile(fileStream, fileName, fileFormat);
+    const extname = path.extname(originalName);
+    let fileName = generateFileHash(originalName);
+    if (extname.length > 1) {   // ignore if empty or '.' only
+      fileName = `${fileName}${extname}`;
+    }
 
     let attachment = new Attachment();
     attachment.page = pageId;
@@ -51,7 +55,12 @@ module.exports = function(crowi) {
     attachment.fileSize = fileSize;
     attachment.createdAt = Date.now();
 
-    return await attachment.save();
+    // upload file
+    await fileUploader.uploadFile(fileStream, attachment);
+    // save attachment
+    attachment = await attachment.save();
+
+    return attachment;
   };
 
   attachmentSchema.statics.removeAttachmentsByPageId = function(pageId) {

src/server/service/file-uploader/aws.js

@@ -1,4 +1,3 @@
-const debug = require('debug')('growi:service:fileUploaderAws');
 const logger = require('@alias/logger')('growi:service:fileUploaderAws');
 
 const axios = require('axios');
@@ -38,12 +37,14 @@ module.exports = function(crowi) {
   }
 
   function getFilePathOnStorage(attachment) {
-    if (attachment.filePath != null) {  // remains for backward compatibility for v3.3.4 or below
+    if (attachment.filePath != null) {  // backward compatibility for v3.3.5 or below
       return attachment.filePath;
     }
 
-    const pageId = attachment.page._id || attachment.page;
-    const filePath = urljoin('/attachment', pageId.toString(), attachment.fileName);
+    const dirName = (attachment.page != null)
+      ? 'attachment'
+      : 'user';
+    const filePath = urljoin(dirName, attachment.fileName);
 
     return filePath;
   }
@@ -60,7 +61,7 @@ module.exports = function(crowi) {
     return new Promise((resolve, reject) => {
       s3.deleteObject(params, (err, data) => {
         if (err) {
-          debug('Failed to delete object from s3', err);
+          logger.debug('Failed to delete object from s3', err);
           return reject(err);
         }
 
@@ -72,25 +73,22 @@ module.exports = function(crowi) {
     });
   };
 
-  lib.uploadFile = function(filePath, contentType, fileStream, options) {
+  lib.uploadFile = function(fileStream, attachment) {
+    logger.debug(`File uploading: fileName=${attachment.fileName}`);
+
     const s3 = S3Factory();
     const awsConfig = getAwsConfig();
 
-    var params = {Bucket: awsConfig.bucket};
-    params.ContentType = contentType;
-    params.Key = filePath;
-    params.Body = fileStream;
-    params.ACL = 'public-read';
-
-    return new Promise(function(resolve, reject) {
-      s3.putObject(params, function(err, data) {
-        if (err) {
-          return reject(err);
-        }
+    const filePath = getFilePathOnStorage(attachment);
+    const params = {
+      Bucket: awsConfig.bucket,
+      ContentType: attachment.fileFormat,
+      Key: filePath,
+      Body: fileStream,
+      ACL: 'public-read',
+    };
 
-        return resolve(data);
-      });
-    });
+    return s3.upload(params).promise();
   };
 
   /**

src/server/service/file-uploader/gridfs.js

@@ -1,4 +1,3 @@
-const debug = require('debug')('growi:service:fileUploaderGridfs');
 const logger = require('@alias/logger')('growi:service:fileUploaderGridfs');
 const mongoose = require('mongoose');
 const util = require('util');
@@ -24,7 +23,7 @@ module.exports = function(crowi) {
 
   // delete a file
   lib.deleteFile = async function(fileId, filePath) {
-    debug('File deletion: ' + fileId);
+    logger.debug('File deletion: ' + fileId);
     const file = await getFile(filePath);
     const id = file.id;
     AttachmentFile.unlinkById(id, function(error, unlinkedAttachment) {
@@ -62,10 +61,15 @@ module.exports = function(crowi) {
     return (+process.env.MONGO_GRIDFS_TOTAL_LIMIT > usingFilesSize + +uploadFileSize);
   };
 
-  lib.uploadFile = async function(fileStream, fileName, contentType) {
-    logger.debug(`File uploading: fileName=${fileName}`);
+  lib.uploadFile = async function(fileStream, attachment) {
+    logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-    AttachmentFile.promisifiedWrite({ filename: fileName, contentType }, fileStream);
+    return AttachmentFile.promisifiedWrite(
+      {
+        filename: attachment.fileName,
+        contentType: attachment.fileFormat
+      },
+      fileStream);
   };
 
   /**

src/server/service/file-uploader/local.js

@@ -1,8 +1,9 @@
-const debug = require('debug')('growi:service:fileUploaderLocal');
+const logger = require('@alias/logger')('growi:service:fileUploaderLocal');
 
 const fs = require('fs');
 const path = require('path');
 const mkdir = require('mkdirp');
+const streamToPromise = require('stream-to-promise');
 
 module.exports = function(crowi) {
   'use strict';
@@ -11,18 +12,20 @@ module.exports = function(crowi) {
   const basePath = path.posix.join(crowi.publicDir, 'uploads');
 
   function getFilePathOnStorage(attachment) {
-    if (attachment.filePath != null) {  // remains for backward compatibility for v3.3.5 or below
-      return path.posix.join(basePath, attachment.filePath);
+    if (attachment.filePath != null) {  // backward compatibility for v3.3.5 or below
+      return attachment.filePath;
     }
 
-    const pageId = attachment.page._id || attachment.page;
-    const filePath = path.posix.join(basePath, pageId.toString(), attachment.fileName);
+    const dirName = (attachment.page != null)
+      ? 'attachment'
+      : 'user';
+    const filePath = path.posix.join(basePath, dirName, attachment.fileName);
 
     return filePath;
   }
 
   lib.deleteFile = function(fileId, filePath) {
-    debug('File deletion: ' + filePath);
+    logger.debug('File deletion: ' + filePath);
     return new Promise(function(resolve, reject) {
       fs.unlink(path.posix.join(basePath, filePath), function(err) {
         if (err) {
@@ -34,28 +37,17 @@ module.exports = function(crowi) {
     });
   };
 
-  lib.uploadFile = function(filePath, contentType, fileStream, options) {
-    debug('File uploading: ' + filePath);
-    return new Promise(function(resolve, reject) {
-      var localFilePath = path.posix.join(basePath, filePath)
-        , dirpath = path.posix.dirname(localFilePath);
-
-      mkdir(dirpath, function(err) {
-        if (err) {
-          return reject(err);
-        }
+  lib.uploadFile = async function(fileStream, attachment) {
+    logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-        var writer = fs.createWriteStream(localFilePath);
+    const filePath = getFilePathOnStorage(attachment);
+    const dirpath = path.posix.dirname(filePath);
 
-        writer.on('error', function(err) {
-          reject(err);
-        }).on('finish', function() {
-          resolve();
-        });
+    // mkdir -p
+    mkdir.sync(dirpath);
 
-        fileStream.pipe(writer);
-      });
-    });
+    const stream = fileStream.pipe(fs.createWriteStream(filePath));
+    return streamToPromise(stream);
   };
 
   /**