refactor user.image

src/server/models/attachment.js

@@ -74,7 +74,7 @@ module.exports = function(crowi) {
       Attachment.find({ page: pageId})
       .then((attachments) => {
         for (let attachment of attachments) {
-          attachment.removeWithSubstance().then((res) => {
+          Attachment.removeWithSubstance(attachment).then((res) => {
             // do nothing
           }).catch((err) => {
             debug('Attachment remove error', err);
@@ -89,8 +89,11 @@ module.exports = function(crowi) {
 
   };
 
-  attachmentSchema.methods.removeWithSubstance = async function() {
-    await fileUploader.deleteFile(this);
+  attachmentSchema.statics.removeWithSubstance = async function(attachment) {
+    // retrieve data from DB
+    // because this instance fields are only partially populated
+    const att = await this.findById(attachment._id);
+    await fileUploader.deleteFile(att);
     return await this.remove();
   };
 

src/server/models/page.js

@@ -96,14 +96,14 @@ const addSlashOfEnd = (path) => {
  * @param {any} page Query or Document
  * @param {string} userPublicFields string to set to select
  */
-const populateDataToShowRevision = (page, userPublicFields) => {
+const populateDataToShowRevision = (page, userPublicFields, imagePopulation) => {
   return page
     .populate([
-      { path: 'lastUpdateUser', model: 'User', select: userPublicFields, populate: { path: 'imageAttachment', select: 'filePathProxied' } },
-      { path: 'creator', model: 'User', select: userPublicFields, populate: { path: 'imageAttachment', select: 'filePathProxied' } },
+      { path: 'lastUpdateUser', model: 'User', select: userPublicFields, populate: imagePopulation },
+      { path: 'creator', model: 'User', select: userPublicFields, populate: imagePopulation },
       { path: 'grantedGroup', model: 'UserGroup' },
       { path: 'revision', model: 'Revision', populate: {
-        path: 'author', model: 'User', select: userPublicFields, populate: { path: 'imageAttachment', select: 'filePathProxied' }
+        path: 'author', model: 'User', select: userPublicFields, populate: imagePopulation
       }}
     ]);
 };
@@ -239,17 +239,18 @@ class PageQueryBuilder {
     return this;
   }
 
-  populateDataToList(userPublicFields) {
+  populateDataToList(userPublicFields, imagePopulation) {
     this.query = this.query
       .populate({
         path: 'lastUpdateUser',
         select: userPublicFields,
-        populate: { path: 'imageAttachment', select: 'filePathProxied' }
+        populate: imagePopulation
       });
+    return this;
   }
 
-  populateDataToShowRevision(userPublicFields) {
-    this.query = populateDataToShowRevision(this.query, userPublicFields);
+  populateDataToShowRevision(userPublicFields, imagePopulation) {
+    this.query = populateDataToShowRevision(this.query, userPublicFields, imagePopulation);
     return this;
   }
 
@@ -430,7 +431,7 @@ module.exports = function(crowi) {
     validateCrowi();
 
     const User = crowi.model('User');
-    return populateDataToShowRevision(this, User.USER_PUBLIC_FIELDS)
+    return populateDataToShowRevision(this, User.USER_PUBLIC_FIELDS, User.IMAGE_POPULATION)
       .execPopulate();
   };
 
@@ -730,7 +731,7 @@ module.exports = function(crowi) {
     const totalCount = await builder.query.exec('count');
 
     // find
-    builder.populateDataToList(User.USER_PUBLIC_FIELDS);
+    builder.populateDataToList(User.USER_PUBLIC_FIELDS, User.IMAGE_POPULATION);
     const pages = await builder.query.exec('find');
 
     const result = { pages, totalCount, offset: opt.offset, limit: opt.limit };
@@ -771,7 +772,7 @@ module.exports = function(crowi) {
 
     // find
     builder.addConditionToPagenate(opt.offset, opt.limit, sortOpt);
-    builder.populateDataToList(User.USER_PUBLIC_FIELDS);
+    builder.populateDataToList(User.USER_PUBLIC_FIELDS, User.IMAGE_POPULATION);
     const pages = await builder.query.exec('find');
 
     const result = { pages, totalCount, offset: opt.offset, limit: opt.limit };

src/server/models/user.js

@@ -15,6 +15,7 @@ module.exports = function(crowi) {
   const STATUS_DELETED    = 4;
   const STATUS_INVITED    = 5;
   const USER_PUBLIC_FIELDS = '_id image isEmailPublished isGravatarEnabled googleId name username email introduction status lang createdAt admin';
+  const IMAGE_POPULATION = { path: 'imageAttachment', select: 'filePathProxied' };
 
   const LANG_EN    = 'en';
   const LANG_EN_US = 'en-US';
@@ -135,6 +136,10 @@ module.exports = function(crowi) {
     return lang;
   }
 
+  userSchema.methods.populateImage = async function() {
+    return await this.populate(IMAGE_POPULATION);
+  };
+
   userSchema.methods.isPasswordSet = function() {
     if (this.password) {
       return true;
@@ -172,7 +177,6 @@ module.exports = function(crowi) {
     });
   };
 
-
   userSchema.methods.updateIsEmailPublished = function(isEmailPublished, callback) {
     this.isEmailPublished = isEmailPublished;
     this.save(function(err, userData) {
@@ -205,18 +209,19 @@ module.exports = function(crowi) {
   };
 
   userSchema.methods.updateImage = async function(attachment) {
-    await this.deleteImage();
-
     this.imageAttachment = attachment;
     return this.save();
   };
 
   userSchema.methods.deleteImage = async function() {
+    validateCrowi();
+    const Attachment = crowi.model('Attachment');
+
     // the 'image' field became DEPRECATED in v3.3.8
     this.image = undefined;
 
     if (this.imageAttachment != null) {
-      this.imageAttachment.removeWithSubstance();
+      Attachment.removeWithSubstance(this.imageAttachment);
     }
 
     this.imageAttachment = undefined;
@@ -820,6 +825,7 @@ module.exports = function(crowi) {
   userSchema.statics.STATUS_DELETED     = STATUS_DELETED;
   userSchema.statics.STATUS_INVITED     = STATUS_INVITED;
   userSchema.statics.USER_PUBLIC_FIELDS = USER_PUBLIC_FIELDS;
+  userSchema.statics.IMAGE_POPULATION   = IMAGE_POPULATION;
   userSchema.statics.PAGE_ITEMS         = PAGE_ITEMS;
 
   userSchema.statics.LANG_EN            = LANG_EN;

src/server/routes/attachment.js

@@ -260,6 +260,7 @@ module.exports = function(crowi, app) {
 
     let attachment;
     try {
+      req.user.deleteImage();
       attachment = await createAttachment(file, req.user);
       await req.user.updateImage(attachment);
     }

src/server/routes/comment.js

@@ -46,7 +46,7 @@ module.exports = function(crowi, app) {
     }
 
     const comments = await fetcher.populate(
-      { path: 'creator', select: User.USER_PUBLIC_FIELDS, populate: { path: 'imageAttachment', select: 'filePathProxied' } }
+      { path: 'creator', select: User.USER_PUBLIC_FIELDS, populate: User.IMAGE_POPULATION }
     );
 
     res.json(ApiResponse.success({comments}));

src/server/routes/page.js

@@ -116,7 +116,9 @@ module.exports = function(crowi, app) {
   }
 
   async function addRenderVarsForUserPage(renderVars, page, requestUser) {
-    const userData = await User.findUserByUsername(User.getUsernameByPath(page.path));
+    const userData = await User.findUserByUsername(User.getUsernameByPath(page.path))
+      .populate(User.IMAGE_POPULATION);
+
     if (userData != null) {
       renderVars.pageUser = userData;
       renderVars.bookmarkList = await Bookmark.findByUser(userData, {limit: 10, populatePage: true, requestUser: requestUser});

src/server/routes/user.js

@@ -57,9 +57,7 @@ module.exports = function(crowi, app) {
 
     const data = {};
     try {
-      const users = await userFetcher.populate({
-        path: 'imageAttachment', select: 'filePathProxied'
-      });
+      const users = await userFetcher.populate(User.IMAGE_POPULATION);
       data.users = users.map(user => {
         // omit email
         if (true !== user.isEmailPublished) { // compare to 'true' because Crowi original data doesn't have 'isEmailPublished'

src/server/service/passport.js

@@ -511,10 +511,17 @@ class PassportService {
     passport.serializeUser(function(user, done) {
       done(null, user.id);
     });
-    passport.deserializeUser(function(id, done) {
-      User.findById(id, function(err, user) {
-        done(err, user);
-      });
+    passport.deserializeUser(async function(id, done) {
+      try {
+        const user = await User.findById(id).populate(User.IMAGE_POPULATION);
+        if (user == null) {
+          throw new Error('user not found');
+        }
+        done(null, user);
+      }
+      catch (err) {
+        done(err);
+      }
     });
 
     this.isSerializerSetup = true;

src/server/util/middlewares.js

@@ -16,27 +16,23 @@ exports.csrfKeyGenerator = function(crowi, app) {
 };
 
 exports.loginChecker = function(crowi, app) {
-  return function(req, res, next) {
-    var User = crowi.model('User');
+  const User = crowi.model('User');
+  return async function(req, res, next) {
+    let user = null;
+
+    try {
+      // session に user object が入ってる
+      if (req.session.user && '_id' in req.session.user) {
+        user = await User.findById(req.session.user._id).populate(User.IMAGE_POPULATION);
+      }
 
-    // session に user object が入ってる
-    if (req.session.user && '_id' in req.session.user) {
-      User.findById(req.session.user._id, function(err, userData) {
-        if (err) {
-          next();
-        }
-        else {
-          req.user = req.session.user = userData;
-          res.locals.user = req.user;
-          next();
-        }
-      });
-    }
-    else {
-      req.user = req.session.user = null;
+      req.user = req.session.user = user;
       res.locals.user = req.user;
       next();
     }
+    catch (err) {
+      next(err);
+    }
   };
 };
 

src/server/views/me/index.html

@@ -157,7 +157,7 @@
             <img src="{{ user|uploadedpicture }}" class="picture picture-lg img-circle" id="settingUserPicture"><br>
             </p>
             <p>
-            {% if user.image %}
+            {% if user.imageAttachment %}
             <form action="/me/picture/delete" method="post" class="form-horizontal" role="form" onsubmit="return window.confirm('{{ t('Delete this image?') }}');">
               <button type="submit" class="btn btn-danger">{{ t('Delete Image') }}</button>
             </form>