Merge branch 'imprv/typescriptize-page-service' into imprv/rename-testcode-and-interface

packages/app/src/server/models/page.ts

@@ -370,8 +370,8 @@ export default (crowi: Crowi): any => {
     }
 
     const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
+    // v4 compatible process
     if (!isV5Compatible) {
-      // v4 compatible process
       return this.createV4(path, body, user, options);
     }
 
@@ -380,28 +380,27 @@ export default (crowi: Crowi): any => {
     const {
       format = 'markdown', redirectTo, grantUserGroupId,
     } = options;
+    let grant = options.grant;
 
     // sanitize path
     path = crowi.xss.process(path); // eslint-disable-line no-param-reassign
-
-    let grant = options.grant;
-    // force public
-    if (isTopPage(path)) {
-      grant = GRANT_PUBLIC;
-    }
-
+    // throw if exists
     const isExist = (await this.count({ path, isEmpty: false })) > 0; // not validate empty page
     if (isExist) {
       throw new Error('Cannot create new page to existed path');
     }
+    // force public
+    if (isTopPage(path)) {
+      grant = GRANT_PUBLIC;
+    }
 
-    // find existing empty page at target path
+    // find an existing empty page
     const emptyPage = await Page.findOne({ path, isEmpty: true });
 
+    /*
+     * UserGroup & Owner validation
+     */
     if (grant !== GRANT_RESTRICTED) {
-      /*
-       * UserGroup & Owner validation
-       */
       let isGrantNormalized = false;
       try {
         // It must check descendants as well if emptyTarget is not null
@@ -443,11 +442,22 @@ export default (crowi: Crowi): any => {
     page.lastUpdateUser = user;
     page.redirectTo = redirectTo;
     page.status = STATUS_PUBLISHED;
-    page.parent = options.grant === GRANT_RESTRICTED ? null : parentId;
+
+    // set parent to null when GRANT_RESTRICTED
+    if (grant === GRANT_RESTRICTED) {
+      page.parent = null;
+    }
+    else {
+      page.parent = parentId;
+    }
 
     page.applyScope(user, grant, grantUserGroupId);
 
     let savedPage = await page.save();
+
+    /*
+     * After save
+     */
     const newRevision = Revision.prepareRevision(savedPage, body, null, user, { format });
     const revision = await pushRevision(savedPage, newRevision, user);
     savedPage = await this.findByPath(revision.path);

packages/app/src/server/service/page-grant.ts

@@ -1,11 +1,13 @@
 import mongoose from 'mongoose';
-import { pagePathUtils } from '@growi/core';
+import { pagePathUtils, pathUtils } from '@growi/core';
+import escapeStringRegexp from 'escape-string-regexp';
 
 import UserGroup from '~/server/models/user-group';
 import { PageModel } from '~/server/models/page';
 import { PageQueryBuilder } from '../models/obsolete-page';
 import { isIncludesObjectId, removeDuplicates, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
+const { addTrailingSlash } = pathUtils;
 const { isTopPage } = pagePathUtils;
 
 type ObjectId = mongoose.Types.ObjectId;
@@ -254,32 +256,50 @@ class PageGrantService {
     /*
      * make granted users list of descendant's
      */
-    // find all descendants excluding empty pages
-    const builderForDescendants = new PageQueryBuilder(Page.find({}, {
-      _id: 0, grant: 1, grantedUsers: 1, grantedGroup: 1,
-    }), false);
-    const descendants = await builderForDescendants
-      .addConditionToListOnlyDescendants(targetPath)
-      .query
-      .exec();
+    const pathWithTrailingSlash = addTrailingSlash(targetPath);
+    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+
+    const result = await Page.aggregate([
+      { // match to descendants excluding empty pages
+        $match: {
+          path: new RegExp(`^${startsPattern}`),
+          isEmpty: { $ne: true },
+        },
+      },
+      {
+        $project: {
+          _id: 0,
+          grant: 1,
+          grantedUsers: 1,
+          grantedGroup: 1,
+        },
+      },
+      { // remove duplicates from pipeline
+        $group: {
+          _id: '$grant',
+          grantedGroupSet: { $addToSet: '$grantedGroup' },
+          grantedUsersSet: { $addToSet: '$grantedUsers' },
+        },
+      },
+      { // flatten granted user set
+        $unwind: {
+          path: '$grantedUsersSet',
+        },
+      },
+    ]);
+
+    // GRANT_PUBLIC group
+    const isPublicExist = result.some(r => r._id === Page.GRANT_PUBLIC);
+    // GRANT_OWNER group
+    const grantOwnerResult = result.filter(r => r._id === Page.GRANT_OWNER)[0]; // users of GRANT_OWNER
+    const grantedUserIds: ObjectId[] = grantOwnerResult?.grantedUsersSet ?? [];
+    // GRANT_USER_GROUP group
+    const grantUserGroupResult = result.filter(r => r._id === Page.GRANT_USER_GROUP)[0]; // users of GRANT_OWNER
+    const grantedGroupIds = grantUserGroupResult?.grantedGroupSet ?? [];
 
-    const isPublicExist = descendants.some(d => d.grant === Page.GRANT_PUBLIC);
-
-    let grantedUsersOfGrantOwner: ObjectId[] = []; // users of GRANT_OWNER
-    const grantedGroups: ObjectId[] = [];
-    descendants.forEach((d) => {
-      if (d.grantedUsers != null) {
-        grantedUsersOfGrantOwner = grantedUsersOfGrantOwner.concat(d.grantedUsers);
-      }
-      if (d.grantedGroup != null) {
-        grantedGroups.push(d.grantedGroup);
-      }
-    });
-
-    const grantedGroupIds = removeDuplicates(grantedGroups);
     return {
       isPublicExist,
-      grantedUserIds: grantedUsersOfGrantOwner,
+      grantedUserIds,
       grantedGroupIds,
     };
   }

packages/app/src/server/service/page.js → packages/app/src/server/service/page.ts

@@ -1,29 +1,32 @@
 import { pagePathUtils } from '@growi/core';
-
+import mongoose from 'mongoose';
+import escapeStringRegexp from 'escape-string-regexp';
+import streamToPromise from 'stream-to-promise';
+import pathlib from 'path';
+import { Writable } from 'stream';
+
+import { serializePageSecurely } from '../models/serializers/page-serializer';
+import { createBatchStream } from '~/server/util/batch-stream';
 import loggerFactory from '~/utils/logger';
-import { generateGrantCondition } from '~/server/models/page';
-
+import { generateGrantCondition, PageModel } from '~/server/models/page';
 import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
-
 import ActivityDefine from '../util/activityDefine';
 
-const mongoose = require('mongoose');
-const escapeStringRegexp = require('escape-string-regexp');
-const streamToPromise = require('stream-to-promise');
-const pathlib = require('path');
-
-const logger = loggerFactory('growi:services:page');
 const debug = require('debug')('growi:services:page');
-const { Writable } = require('stream');
-const { createBatchStream } = require('~/server/util/batch-stream');
 
+const logger = loggerFactory('growi:services:page');
 const { isCreatablePage, isDeletablePage, isTrashPage } = pagePathUtils;
-const { serializePageSecurely } = require('../models/serializers/page-serializer');
 
 const BULK_REINDEX_SIZE = 100;
 
 class PageService {
 
+  crowi: any;
+
+  pageEvent: any;
+
+  tagEvent: any;
+
   constructor(crowi) {
     this.crowi = crowi;
     this.pageEvent = crowi.event('page');
@@ -117,7 +120,7 @@ class PageService {
       page = await Page.findByPathAndViewer(path, user);
     }
 
-    const result = {};
+    const result: any = {};
 
     if (page == null) {
       const isExist = await Page.count({ $or: [{ _id: pageId }, { path }] }) > 0;
@@ -154,7 +157,7 @@ class PageService {
    * @param {object} redirectToPagePathMapping
    * @param {array} pagePaths
    */
-  prepareShoudDeletePagesByRedirectTo(redirectTo, redirectToPagePathMapping, pagePaths = []) {
+  prepareShoudDeletePagesByRedirectTo(redirectTo, redirectToPagePathMapping, pagePaths: any[] = []) {
     const pagePath = redirectToPagePathMapping[redirectTo];
 
     if (pagePath == null) {
@@ -202,7 +205,7 @@ class PageService {
       await this.renameDescendantsWithStream(page, newPagePath, user, options);
     }
 
-    const update = {};
+    const update: any = {};
     // update Page
     update.path = newPagePath;
     if (updateMetadata) {
@@ -257,7 +260,7 @@ class PageService {
           _id: revisionId, path: page.path, body: `redirect ${newPagePath}`, author: user._id, format: 'markdown',
         });
       }
-      revisionUnorderedBulkOp.find({ path: page.path }).update({ $set: { path: newPagePath } }, { multi: true });
+      revisionUnorderedBulkOp.find({ path: page.path }).update({ $set: { path: newPagePath } });
     });
 
     try {
@@ -271,7 +274,7 @@ class PageService {
     }
     catch (err) {
       if (err.code !== 11000) {
-        throw new Error('Failed to rename pages: ', err);
+        throw new Error(`Failed to rename pages: ${err}`);
       }
     }
 
@@ -341,7 +344,7 @@ class PageService {
       redirectToPagePathMapping[page.redirectTo] = page.path;
     });
 
-    const redirectedFromPagePaths = [];
+    const redirectedFromPagePaths: any[] = [];
     pagePaths.forEach((pagePath) => {
       redirectedFromPagePaths.push(...this.prepareShoudDeletePagesByRedirectTo(pagePath, redirectToPagePathMapping));
     });
@@ -359,12 +362,12 @@ class PageService {
 
   async duplicate(page, newPagePath, user, isRecursively) {
     const Page = this.crowi.model('Page');
-    const PageTagRelation = mongoose.model('PageTagRelation');
+    const PageTagRelation = mongoose.model('PageTagRelation') as any; // TODO: Typescriptize model
     // populate
     await page.populate({ path: 'revision', model: 'Revision', select: 'body' });
 
     // create option
-    const options = { page };
+    const options: any = { page };
     options.grant = page.grant;
     options.grantUserGroupId = page.grantedGroup;
     options.grantedUserIds = page.grantedUsers;
@@ -403,7 +406,7 @@ class PageService {
 
     // convert pageId from string to ObjectId
     const pageIds = Object.keys(pageIdMapping);
-    const stage = { $or: pageIds.map((pageId) => { return { relatedPage: mongoose.Types.ObjectId(pageId) } }) };
+    const stage = { $or: pageIds.map((pageId) => { return { relatedPage: new mongoose.Types.ObjectId(pageId) } }) };
 
     const pagesAssociatedWithTag = await PageTagRelation.aggregate([
       {
@@ -417,7 +420,7 @@ class PageService {
       },
     ]);
 
-    const newPageTagRelation = [];
+    const newPageTagRelation: any[] = [];
     pagesAssociatedWithTag.forEach(({ _id, relatedPages }) => {
       // relatedPages
       relatedPages.forEach((pageId) => {
@@ -446,8 +449,8 @@ class PageService {
 
     // key: oldPageId, value: newPageId
     const pageIdMapping = {};
-    const newPages = [];
-    const newRevisions = [];
+    const newPages: any[] = [];
+    const newRevisions: any[] = [];
 
     pages.forEach((page) => {
       const newPageId = new mongoose.Types.ObjectId();
@@ -564,7 +567,7 @@ class PageService {
     const deletePageBulkOp = pageCollection.initializeUnorderedBulkOp();
     const updateRevisionListOp = revisionCollection.initializeUnorderedBulkOp();
     const createRediectRevisionBulkOp = revisionCollection.initializeUnorderedBulkOp();
-    const newPagesForRedirect = [];
+    const newPagesForRedirect: any[] = [];
 
     pages.forEach((page) => {
       const newPath = Page.getDeletedPageName(page.path);
@@ -601,7 +604,7 @@ class PageService {
     }
     catch (err) {
       if (err.code !== 11000) {
-        throw new Error('Failed to revert pages: ', err);
+        throw new Error(`Failed to revert pages: ${err}`);
       }
     }
     finally {
@@ -747,7 +750,7 @@ class PageService {
           path: toPath, status: Page.STATUS_PUBLISHED, lastUpdateUser: user._id, deleteUser: null, deletedAt: null,
         },
       });
-      revertRevisionBulkOp.find({ path: page.path }).update({ $set: { path: toPath } }, { multi: true });
+      revertRevisionBulkOp.find({ path: page.path }).update({ $set: { path: toPath } });
     });
 
     try {
@@ -757,7 +760,7 @@ class PageService {
     }
     catch (err) {
       if (err.code !== 11000) {
-        throw new Error('Failed to revert pages: ', err);
+        throw new Error(`Failed to revert pages: ${err}`);
       }
     }
   }
@@ -839,7 +842,6 @@ class PageService {
     const Page = this.crowi.model('Page');
     const pages = await Page.find({ grantedGroup: { $in: groupsToDelete } });
 
-    let operationsToPublicize;
     switch (action) {
       case 'public':
         await Page.publicizePages(pages);
@@ -861,7 +863,7 @@ class PageService {
     // aggregation options
     const viewerCondition = await generateGrantCondition(user, null);
     const filterByIds = {
-      _id: { $in: pageIds.map(id => mongoose.Types.ObjectId(id)) },
+      _id: { $in: pageIds.map(id => new mongoose.Types.ObjectId(id)) },
     };
 
     let pages;
@@ -1042,7 +1044,7 @@ class PageService {
    * returns an array of js RegExp instance instead of RE2 instance for mongo filter
    */
   async _generateRegExpsByPageIds(pageIds) {
-    const Page = mongoose.model('Page');
+    const Page = mongoose.model('Page') as PageModel;
 
     let result;
     try {
@@ -1080,7 +1082,7 @@ class PageService {
     const { PageQueryBuilder } = Page;
 
     // generate filter
-    let filter = {
+    let filter: any = {
       parent: null,
       path: { $ne: '/' },
     };
@@ -1157,7 +1159,7 @@ class PageService {
             parentPath = parentPath.replace(bracket, `\\${bracket}`);
           });
 
-          const filter = {
+          const filter: any = {
             // regexr.com/6889f
             // ex. /parent/any_child OR /any_level1
             path: { $regex: new RegExp(`^${parentPath}(\\/[^/]+)\\/?$`, 'g') },

packages/app/src/test/integration/service/page.test.js

@@ -307,29 +307,29 @@ describe('PageService', () => {
     /*
      * TODO: rewrite test when modify rename function
      */
-    // test('rename page with different tree with isRecursively [shallower]', async() => {
-    //   // setup
-    //   expect(await Page.findOne({ path: '/level1' })).toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/level2' })).not.toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/level2/child' })).not.toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/level2/level2' })).not.toBeNull();
-    //   expect(await Page.findOne({ path: '/level1-2021H1' })).not.toBeNull();
-
-    //   // when
-    //   //   rename /level1/level2 --> /level1
-    //   await crowi.pageService.renamePage(parentForRename7, '/level1', testUser1, {}, true);
-
-    //   // then
-    //   expect(await Page.findOne({ path: '/level1' })).not.toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/child' })).not.toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/level2' })).toBeNull();
-    //   expect(await Page.findOne({ path: '/level1/level2/child' })).toBeNull();
-    //   // The changed path is duplicated with the existing path (/level1/level2), so it will not be changed
-    //   expect(await Page.findOne({ path: '/level1/level2/level2' })).not.toBeNull();
-
-    //   // Check that pages that are not to be renamed have not been renamed
-    //   expect(await Page.findOne({ path: '/level1-2021H1' })).not.toBeNull();
-    // });
+    test('rename page with different tree with isRecursively [shallower]', async() => {
+      // setup
+      expect(await Page.findOne({ path: '/level1' })).toBeNull();
+      expect(await Page.findOne({ path: '/level1/level2' })).not.toBeNull();
+      expect(await Page.findOne({ path: '/level1/level2/child' })).not.toBeNull();
+      expect(await Page.findOne({ path: '/level1/level2/level2' })).not.toBeNull();
+      expect(await Page.findOne({ path: '/level1-2021H1' })).not.toBeNull();
+
+      // when
+      //   rename /level1/level2 --> /level1
+      await crowi.pageService.renamePage(parentForRename7, '/level1', testUser1, {}, true);
+
+      // then
+      expect(await Page.findOne({ path: '/level1' })).not.toBeNull();
+      expect(await Page.findOne({ path: '/level1/child' })).not.toBeNull();
+      expect(await Page.findOne({ path: '/level1/level2' })).toBeNull();
+      expect(await Page.findOne({ path: '/level1/level2/child' })).toBeNull();
+      // The changed path is duplicated with the existing path (/level1/level2), so it will not be changed
+      expect(await Page.findOne({ path: '/level1/level2/level2' })).not.toBeNull();
+
+      // Check that pages that are not to be renamed have not been renamed
+      expect(await Page.findOne({ path: '/level1-2021H1' })).not.toBeNull();
+    });
   });
 
   describe('rename page', () => {