WIP: refactor attachment model, route

src/server/models/attachment.js

@@ -1,22 +1,23 @@
 module.exports = function(crowi) {
-  var debug = require('debug')('growi:models:attachment')
-    , mongoose = require('mongoose')
-    , ObjectId = mongoose.Schema.Types.ObjectId
-    , fileUploader = require('../service/file-uploader')(crowi)
-    , attachmentSchema
-  ;
+  const debug = require('debug')('growi:models:attachment');
+  const mongoose = require('mongoose');
+  const ObjectId = mongoose.Schema.Types.ObjectId;
+
+  const fileUploader = require('../service/file-uploader')(crowi);
+
+  let attachmentSchema;
 
   function generateFileHash(fileName) {
-    var hasher = require('crypto').createHash('md5');
-    hasher.update(fileName);
+    const hash = require('crypto').createHash('md5');
+    hash.update(`${fileName}_${Date.now()}`);
 
-    return hasher.digest('hex');
+    return hash.digest('hex');
   }
 
   attachmentSchema = new mongoose.Schema({
     page: { type: ObjectId, ref: 'Page', index: true },
     creator: { type: ObjectId, ref: 'User', index: true  },
-    filePath: { type: String, required: true },
+    filePath: { type: String },   // DEPRECATED: remains for backward compatibility for v3.3.3 or below
     fileName: { type: String, required: true },
     originalName: { type: String },
     fileFormat: { type: String, required: true },
@@ -29,13 +30,7 @@ module.exports = function(crowi) {
   });
 
   attachmentSchema.virtual('fileUrl').get(function() {
-    // NOTE: use original generated Url directly (not proxy) -- 2017.05.08 Yuki Takei
-    // reason:
-    //   1. this is buggy (doesn't work on Win)
-    //   2. ensure backward compatibility of data
-
-    // return `/files/${this._id}`;
-    return fileUploader.generateUrl(this.filePath);
+    return `/files/${this._id}`;
   });
 
   attachmentSchema.statics.findById = function(id) {
@@ -156,13 +151,6 @@ module.exports = function(crowi) {
 
   };
 
-  attachmentSchema.statics.findDeliveryFile = function(attachment, forceUpdate) {
-    // TODO force update
-    // var forceUpdate = forceUpdate || false;
-
-    return fileUploader.findDeliveryFile(attachment._id, attachment.filePath);
-  };
-
   attachmentSchema.statics.removeAttachment = function(attachment) {
     const Attachment = this;
     const filePath = attachment.filePath;

src/server/routes/attachment.js

@@ -1,17 +1,27 @@
+const debug = require('debug')('growi:routss:attachment');
+const logger = require('@alias/logger')('growi:routes:attachment');
+
+const path = require('path');
+const fs = require('fs');
+const urljoin = require('url-join');
+
+const ApiResponse = require('../util/apiResponse');
+
 module.exports = function(crowi, app) {
-  'use strict';
-
-  var debug = require('debug')('growi:routss:attachment')
-    , logger = require('@alias/logger')('growi:routes:attachment')
-    , Attachment = crowi.model('Attachment')
-    , User = crowi.model('User')
-    , Page = crowi.model('Page')
-    , path = require('path')
-    , fs = require('fs')
-    , fileUploader = require('../service/file-uploader')(crowi, app)
-    , ApiResponse = require('../util/apiResponse')
-    , actions = {}
-    , api = {};
+  const Attachment = crowi.model('Attachment');
+  const User = crowi.model('User');
+  const Page = crowi.model('Page');
+  const fileUploader = require('../service/file-uploader')(crowi, app);
+
+  const actions = {};
+  const api = {};
+
+  async function findDeliveryFile(attachment) {
+    const pageId = attachment.page._id || attachment.page;
+    const filePath = urljoin('/attachment', pageId.toString(), attachment.fileName);
+
+    return fileUploader.findDeliveryFile(attachment._id, filePath);
+  }
 
   actions.api = api;
 
@@ -21,7 +31,7 @@ module.exports = function(crowi, app) {
     Attachment.findById(id)
       .then(function(data) {
 
-        Attachment.findDeliveryFile(data)
+        findDeliveryFile(data)
           .then(fileName => {
 
             // local
@@ -48,13 +58,48 @@ module.exports = function(crowi, app) {
   };
 
   /**
-   * @api {get} /attachments.get get attachments from mongoDB
+   * @api {get} /attachments.get get attachments
    * @apiName get
    * @apiGroup Attachment
    *
-   * @apiParam {String} pageId, fileName
+   * @apiParam {String} id
    */
   api.get = async function(req, res) {
+    const id = req.params.id;
+
+    const attachment = await Attachment.findById(id);
+
+    if (attachment == null) {
+      return res.json(ApiResponse.error('attachment not found'));
+    }
+
+    // TODO consider page restrection
+
+    try {
+      const file = await findDeliveryFile(attachment);
+
+      // redirect if string
+      if (typeof file === 'string') {
+        return res.redirect(file);
+      }
+
+      res.set('Content-Type', file.contentType);
+      return res.send(ApiResponse.success(file.data));
+    }
+    catch (e) {
+      // TODO handle errors
+      return res.json(ApiResponse.error(e.message));
+    }
+  };
+
+  /**
+   * @api {get} /attachments.obsoletedGetForMongoDB get attachments from mongoDB
+   * @apiName get
+   * @apiGroup Attachment
+   *
+   * @apiParam {String} pageId, fileName
+   */
+  api.obsoletedGetForMongoDB = async function(req, res) {
     if (process.env.FILE_UPLOAD !== 'mongodb') {
       return res.status(400);
     }

src/server/routes/index.js

@@ -175,8 +175,9 @@ module.exports = function(crowi, app) {
 
   app.get( '/:id([0-9a-z]{24})'       , loginRequired(crowi, app, false) , page.redirector);
   app.get( '/_r/:id([0-9a-z]{24})'    , loginRequired(crowi, app, false) , page.redirector); // alias
-  app.get( '/download/:id([0-9a-z]{24})' , loginRequired(crowi, app, false) , attachment.api.download);
-  app.get( '/attachment/:pageId/:fileName'  , loginRequired(crowi, app, false), attachment.api.get);
+  app.get( '/attachment/:pageId/:fileName'  , loginRequired(crowi, app, false), attachment.api.obsoletedGetForMongoDB); // DEPRECATED: remains for backward compatibility for v3.3.3 or below
+  app.get( '/attachment/:id([0-9a-z]{24})'  , loginRequired(crowi, app, false), attachment.api.get);
+  app.get( '/download/:id([0-9a-z]{24})'    , loginRequired(crowi, app, false), attachment.api.download);
 
   app.get( '/_search'                 , loginRequired(crowi, app, false) , search.searchPage);
   app.get( '/_api/search'             , accessTokenParser , loginRequired(crowi, app, false) , search.api.search);

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

@@ -47,14 +47,13 @@ module.exports = function(crowi) {
     });
   };
 
-  lib.generateUrl = function(filePath) {
+  /**
+   * return local file path string
+   */
+  lib.findDeliveryFile = async function(attachmentId, filePath) {
     return path.posix.join('/uploads', filePath);
   };
 
-  lib.findDeliveryFile = function(fileId, filePath) {
-    return Promise.resolve(lib.generateUrl(filePath));
-  };
-
   /**
    * chech storage for fileUpload reaches MONGO_GRIDFS_TOTAL_LIMIT (for gridfs)
    */