GC-1225 refactor bookmark

src/server/models/bookmark.js

@@ -1,8 +1,9 @@
 module.exports = function(crowi) {
-  var debug = require('debug')('growi:models:bookmark')
-    , mongoose = require('mongoose')
-    , ObjectId = mongoose.Schema.Types.ObjectId
-    , bookmarkSchema;
+  const debug = require('debug')('growi:models:bookmark');
+  const mongoose = require('mongoose');
+  const ObjectId = mongoose.Schema.Types.ObjectId;
+
+  let bookmarkSchema = null;
 
 
   bookmarkSchema = new mongoose.Schema({
@@ -15,13 +16,9 @@ module.exports = function(crowi) {
   bookmarkSchema.statics.populatePage = function(bookmarks, requestUser) {
     const Bookmark = this;
     const User = crowi.model('User');
-    const Page = crowi.model('Page');
 
     requestUser = requestUser || null;
 
-    // mongoose promise に置き換えてみたものの、こいつは not native promise but original promise だったので
-    // これ以上は置き換えないことにする ...
-    // @see http://eddywashere.com/blog/switching-out-callbacks-with-promises-in-mongoose/
     return Bookmark.populate(bookmarks, {path: 'page'})
       .then(function(bookmarks) {
         return Bookmark.populate(bookmarks, {path: 'page.revision', model: 'Revision'});
@@ -63,15 +60,14 @@ module.exports = function(crowi) {
    * }
    */
   bookmarkSchema.statics.findByUser = function(user, option) {
-    var User = crowi.model('User');
-    var Bookmark = this;
-    var requestUser = option.requestUser || null;
+    const Bookmark = this;
+    const requestUser = option.requestUser || null;
 
     debug('Finding bookmark with requesting user:', requestUser);
 
-    var limit = option.limit || 50;
-    var offset = option.offset || 0;
-    var populatePage = option.populatePage || false;
+    const limit = option.limit || 50;
+    const offset = option.offset || 0;
+    const populatePage = option.populatePage || false;
 
     return new Promise(function(resolve, reject) {
       Bookmark
@@ -94,10 +90,10 @@ module.exports = function(crowi) {
   };
 
   bookmarkSchema.statics.add = function(page, user) {
-    var Bookmark = this;
+    const Bookmark = this;
 
     return new Promise(function(resolve, reject) {
-      var newBookmark = new Bookmark;
+      const newBookmark = new Bookmark;
 
       newBookmark.page = page;
       newBookmark.user = user;

src/server/routes/bookmark.js

@@ -1,15 +1,12 @@
 module.exports = function(crowi, app) {
   'use strict';
 
-  var debug = require('debug')('growi:routes:bookmark')
-    , Bookmark = crowi.model('Bookmark')
-    , Page = crowi.model('Page')
-    , User = crowi.model('User')
-    , Revision = crowi.model('Revision')
-    , Bookmark = crowi.model('Bookmark')
-    , ApiResponse = require('../util/apiResponse')
-    , actions = {}
-  ;
+  const debug = require('debug')('growi:routes:bookmark');
+  const Bookmark = crowi.model('Bookmark');
+  const Page = crowi.model('Page');
+  const ApiResponse = require('../util/apiResponse');
+  const ApiPaginate = require('../util/apiPaginate');
+  let actions = {};
   actions.api = {};
 
   /**
@@ -20,20 +17,35 @@ module.exports = function(crowi, app) {
    * @apiParam {String} page_id Page Id.
    */
   actions.api.get = function(req, res) {
-    var pageId = req.query.page_id;
+    let pageId = req.query.page_id;
 
     Bookmark.findByPageIdAndUserId(pageId, req.user)
-    .then(function(data) {
-      debug('bookmark found', pageId, data);
-      var result = {};
-      if (data) {
-      }
+      .then(function(data) {
+        debug('bookmark found', pageId, data);
+        let result = {};
 
-      result.bookmark = data;
-      return res.json(ApiResponse.success(result));
-    }).catch(function(err) {
-      return res.json(ApiResponse.error(err));
-    });
+        result.bookmark = data;
+        return res.json(ApiResponse.success(result));
+      })
+      .catch(function(err) {
+        return res.json(ApiResponse.error(err));
+      });
+  };
+
+  /**
+   *
+   */
+  actions.api.list = function(req, res) {
+    let paginateOptions = ApiPaginate.parseOptions(req.query);
+
+    let options = Object.assign(paginateOptions, { populatePage: true });
+    Bookmark.findByUserId(req.user._id, options)
+      .then(function(result) {
+        return res.json(ApiResponse.success(result));
+      })
+      .catch(function(err) {
+        return res.json(ApiResponse.error(err));
+      });
   };
 
   /**
@@ -43,27 +55,21 @@ module.exports = function(crowi, app) {
    *
    * @apiParam {String} page_id Page Id.
    */
-  actions.api.add = function(req, res) {
-    var pageId = req.body.page_id;
+  actions.api.add = async function(req, res) {
+    const pageId = req.body.page_id;
+
+    const page = await Page.findByIdAndViewer(pageId, req.user);
+    if (page == null) {
+      return res.json(ApiResponse.success({ bookmark: null }));
+    }
 
-    Page.findByIdAndGrantedUser(pageId, req.user)
-    .then(function(pageData) {
-      if (pageData) {
-        return Bookmark.add(pageData, req.user);
-      }
-      else {
-        return res.json(ApiResponse.success({bookmark: null}));
-      }
-    }).then(function(data) {
-      var result = {};
-      data.depopulate('page');
-      data.depopulate('user');
+    const bookmark = await Bookmark.add(page, req.user);
 
-      result.bookmark = data;
-      return res.json(ApiResponse.success(result));
-    }).catch(function(err) {
-      return res.json(ApiResponse.error(err));
-    });
+    bookmark.depopulate('page');
+    bookmark.depopulate('user');
+    const result = { bookmark };
+
+    return res.json(ApiResponse.success(result));
   };
 
   /**
@@ -74,17 +80,17 @@ module.exports = function(crowi, app) {
    * @apiParam {String} page_id Page Id.
    */
   actions.api.remove = function(req, res) {
-    var pageId = req.body.page_id;
+    let pageId = req.body.page_id;
 
     Bookmark.removeBookmark(pageId, req.user)
-    .then(function(data) {
-      debug('Bookmark removed.', data); // if the bookmark is not exists, this 'data' is null
-      return res.json(ApiResponse.success());
-    }).catch(function(err) {
-      return res.json(ApiResponse.error(err));
-    });
+      .then(function(data) {
+        debug('Bookmark removed.', data); // if the bookmark is not exists, this 'data' is null
+        return res.json(ApiResponse.success());
+      })
+      .catch(function(err) {
+        return res.json(ApiResponse.error(err));
+      });
   };
 
-
   return actions;
 };

src/server/util/apiPaginate.js

@@ -0,0 +1,44 @@
+'use strict';
+
+const LIMIT_DEFAULT = 50;
+const LIMIT_MAX = 1000;
+
+const OFFSET_DEFAULT = 0;
+
+const DEFAULT_MAX_RESULT_WINDOW = 10000;
+
+const parseIntValue = function(value, defaultValue, maxLimit) {
+  if (!value) {
+    return defaultValue;
+  }
+
+  let v = parseInt(value);
+  if (!maxLimit) {
+    return v;
+  }
+
+  return v <= maxLimit ? v : maxLimit;
+};
+
+function ApiPaginate() {}
+
+ApiPaginate.parseOptionsForElasticSearch = function(params) {
+  let limit = parseIntValue(params.limit, LIMIT_DEFAULT, LIMIT_MAX);
+  let offset = parseIntValue(params.offset, OFFSET_DEFAULT);
+
+  // See https://github.com/crowi/crowi/pull/293
+  if (limit + offset > DEFAULT_MAX_RESULT_WINDOW) {
+    throw new Error(`(limit + offset) must be less than or equal to ${DEFAULT_MAX_RESULT_WINDOW}`);
+  }
+
+  return { limit: limit, offset: offset };
+};
+
+ApiPaginate.parseOptions = function(params) {
+  let limit = parseIntValue(params.limit, LIMIT_DEFAULT, LIMIT_MAX);
+  let offset = parseIntValue(params.offset, OFFSET_DEFAULT);
+
+  return { limit: limit, offset: offset };
+};
+
+module.exports = ApiPaginate;