Merge pull request #2610 from weseek/imprv/3416-optimize-page-history

Imprv/3416 optimize page history

src/server/models/revision.js

@@ -9,7 +9,7 @@ module.exports = function(crowi) {
 
   const ObjectId = mongoose.Schema.Types.ObjectId;
   const revisionSchema = new mongoose.Schema({
-    path: { type: String, required: true },
+    path: { type: String, required: true, index: true },
     body: {
       type: String,
       required: true,

src/server/routes/index.js

@@ -168,7 +168,6 @@ module.exports = function(crowi, app) {
 
   app.get('/_api/revisions.get'       , certifySharedPage , accessTokenParser , loginRequired , revision.api.get);
   app.get('/_api/revisions.ids'       , certifySharedPage , accessTokenParser , loginRequired , revision.api.ids);
-  app.get('/_api/revisions.list'      , accessTokenParser , loginRequired , revision.api.list);
 
   app.get('/trash$'                   , loginRequired , page.trashPageShowWrapper);
   app.get('/trash/$'                  , loginRequired , page.trashPageListShowWrapper);

src/server/routes/revision.js

@@ -45,7 +45,6 @@
  */
 
 module.exports = function(crowi, app) {
-  const debug = require('debug')('growi:routes:revision');
   const logger = require('@alias/logger')('growi:routes:revision');
   const Page = crowi.model('Page');
   const Revision = crowi.model('Revision');
@@ -109,8 +108,7 @@ module.exports = function(crowi, app) {
     }
 
     // check whether accessible
-    const isAccessible = await Page.isAccessiblePageByViewer(pageId, req.user);
-    if (!isSharedPage && !isAccessible) {
+    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
       return res.json(ApiResponse.error('Current user is not accessible to this page.'));
     }
 
@@ -164,101 +162,27 @@ module.exports = function(crowi, app) {
    *
    * @apiParam {String} page_id      Page Id.
    */
-  actions.api.ids = function(req, res) {
-    const pageId = req.query.page_id || null;
+  actions.api.ids = async function(req, res) {
+    const pageId = req.query.page_id;
+    const { isSharedPage } = req;
 
-    if (pageId && crowi.isPageId(pageId)) {
-      Page.findByIdAndViewer(pageId, req.user)
-        .then((pageData) => {
-          debug('Page found', pageData._id, pageData.path);
-          return Revision.findRevisionList(pageData.path);
-        })
-        .then((revisions) => {
-          return res.json(ApiResponse.success({ revisions }));
-        })
-        .catch((err) => {
-          return res.json(ApiResponse.error(err));
-        });
-    }
-    else {
-      return res.json(ApiResponse.error('Parameter error.'));
+    if (pageId == null) {
+      return res.json(ApiResponse.error('Parameter page_id is required.'));
     }
-  };
 
-  /**
-   * @swagger
-   *
-   *    /revisions.list:
-   *      get:
-   *        tags: [Revisions, CrowiCompatibles]
-   *        operationId: revisions.list
-   *        summary: /revisions.list
-   *        description: Get revisions
-   *        parameters:
-   *          - in: query
-   *            name: page_id
-   *            schema:
-   *              $ref: '#/components/schemas/Page/properties/_id'
-   *          - in: query
-   *            name: revision_ids
-   *            schema:
-   *              type: string
-   *              description: revision ids
-   *              example: 5e0734e472560e001761fa68,5e079a0a0afa6700170a75fb
-   *        responses:
-   *          200:
-   *            description: Succeeded to get revisions.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  properties:
-   *                    ok:
-   *                      $ref: '#/components/schemas/V1Response/properties/ok'
-   *                    revisions:
-   *                      type: array
-   *                      items:
-   *                        $ref: '#/components/schemas/Revision'
-   *          403:
-   *            $ref: '#/components/responses/403'
-   *          500:
-   *            $ref: '#/components/responses/500'
-   */
-  /**
-   * @api {get} /revisions.list Get revisions
-   * @apiName ListRevision
-   * @apiGroup Revision
-   *
-   * @apiParam {String} revision_ids Revision Ids.
-   * @apiParam {String} page_id      Page Id.
-   */
-  actions.api.list = function(req, res) {
-    const revisionIds = (req.query.revision_ids || '').split(',');
-    const pageId = req.query.page_id || null;
-
-    if (pageId) {
-      Page.findByIdAndViewer(pageId, req.user)
-        .then((pageData) => {
-          debug('Page found', pageData._id, pageData.path);
-          return Revision.findRevisionList(pageData.path, {});
-        })
-        .then((revisions) => {
-          return res.json(ApiResponse.success(revisions));
-        })
-        .catch((err) => {
-          return res.json(ApiResponse.error(err));
-        });
+    // check whether accessible
+    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
+      return res.json(ApiResponse.error('Current user is not accessible to this page.'));
     }
-    else if (revisionIds.length > 0) {
-      Revision.findRevisions(revisionIds)
-        .then((revisions) => {
-          return res.json(ApiResponse.success(revisions));
-        })
-        .catch((err) => {
-          return res.json(ApiResponse.error(err));
-        });
+
+    try {
+      const page = await Page.findOne({ _id: pageId });
+      const revisions = await Revision.findRevisionIdList(page.path);
+      return res.json(ApiResponse.success({ revisions }));
     }
-    else {
-      return res.json(ApiResponse.error('Parameter error.'));
+    catch (err) {
+      logger.error('Error revisios.ids', err);
+      return res.json(ApiResponse.error(err));
     }
   };