Merge pull request #5458 from weseek/fix/normalize-parent

fix: Normalize parent

packages/app/src/server/models/obsolete-page.js

@@ -1,6 +1,8 @@
-import { templateChecker, pagePathUtils } from '@growi/core';
+import { templateChecker, pagePathUtils, pathUtils } from '@growi/core';
+
 import loggerFactory from '~/utils/logger';
 
+
 // disable no-return-await for model functions
 /* eslint-disable no-return-await */
 
@@ -12,7 +14,6 @@ const urljoin = require('url-join');
 const mongoose = require('mongoose');
 const differenceInYears = require('date-fns/differenceInYears');
 
-const { pathUtils } = require('@growi/core');
 const escapeStringRegexp = require('escape-string-regexp');
 
 const { isTopPage, isTrashPage } = pagePathUtils;
@@ -44,7 +45,7 @@ const pageSchema = {
  * @param {string} pagePath
  * @return {string[]} ancestors paths
  */
-const extractToAncestorsPaths = (pagePath) => {
+export const extractToAncestorsPaths = (pagePath) => {
   const ancestorsPaths = [];
 
   let parentPath;
@@ -62,7 +63,7 @@ const extractToAncestorsPaths = (pagePath) => {
  * @param {string} userPublicFields string to set to select
  */
 /* eslint-disable object-curly-newline, object-property-newline */
-const populateDataToShowRevision = (page, userPublicFields) => {
+export const populateDataToShowRevision = (page, userPublicFields) => {
   return page
     .populate([
       { path: 'lastUpdateUser', model: 'User', select: userPublicFields },
@@ -77,307 +78,6 @@ const populateDataToShowRevision = (page, userPublicFields) => {
 /* eslint-enable object-curly-newline, object-property-newline */
 
 
-export class PageQueryBuilder {
-
-  constructor(query, includeEmpty = false) {
-    this.query = query;
-    if (!includeEmpty) {
-      this.query = this.query
-        .and({
-          $or: [
-            { isEmpty: false },
-            { isEmpty: null }, // for v4 compatibility
-          ],
-        });
-    }
-  }
-
-  addConditionToExcludeTrashed() {
-    this.query = this.query
-      .and({
-        $or: [
-          { status: null },
-          { status: STATUS_PUBLISHED },
-        ],
-      });
-
-    return this;
-  }
-
-  /**
-   * generate the query to find the pages '{path}/*' and '{path}' self.
-   * If top page, return without doing anything.
-   */
-  addConditionToListWithDescendants(path, option) {
-    // No request is set for the top page
-    if (isTopPage(path)) {
-      return this;
-    }
-
-    const pathNormalized = pathUtils.normalizePath(path);
-    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
-
-    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
-
-    this.query = this.query
-      .and({
-        $or: [
-          { path: pathNormalized },
-          { path: new RegExp(`^${startsPattern}`) },
-        ],
-      });
-
-    return this;
-  }
-
-  /**
-   * generate the query to find the pages '{path}/*' (exclude '{path}' self).
-   * If top page, return without doing anything.
-   */
-  addConditionToListOnlyDescendants(path, option) {
-    // No request is set for the top page
-    if (isTopPage(path)) {
-      return this;
-    }
-
-    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
-
-    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
-
-    this.query = this.query
-      .and({ path: new RegExp(`^${startsPattern}`) });
-
-    return this;
-
-  }
-
-  addConditionToListOnlyAncestors(path) {
-    const pathNormalized = pathUtils.normalizePath(path);
-    const ancestorsPaths = extractToAncestorsPaths(pathNormalized);
-
-    this.query = this.query
-      .and({
-        path: {
-          $in: ancestorsPaths,
-        },
-      });
-
-    return this;
-
-  }
-
-  /**
-   * generate the query to find pages that start with `path`
-   *
-   * In normal case, returns '{path}/*' and '{path}' self.
-   * If top page, return without doing anything.
-   *
-   * *option*
-   *   Left for backward compatibility
-   */
-  addConditionToListByStartWith(path, option) {
-    // No request is set for the top page
-    if (isTopPage(path)) {
-      return this;
-    }
-
-    const startsPattern = escapeStringRegexp(path);
-
-    this.query = this.query
-      .and({ path: new RegExp(`^${startsPattern}`) });
-
-    return this;
-  }
-
-  async addConditionForParentNormalization(user) {
-    // determine UserGroup condition
-    let userGroups = null;
-    if (user != null) {
-      const UserGroupRelation = mongoose.model('UserGroupRelation');
-      userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
-    }
-
-    const grantConditions = [
-      { grant: null },
-      { grant: GRANT_PUBLIC },
-    ];
-
-    if (user != null) {
-      grantConditions.push(
-        { grant: GRANT_OWNER, grantedUsers: user._id },
-      );
-    }
-
-    if (userGroups != null && userGroups.length > 0) {
-      grantConditions.push(
-        { grant: GRANT_USER_GROUP, grantedGroup: { $in: userGroups } },
-      );
-    }
-
-    this.query = this.query
-      .and({
-        $or: grantConditions,
-      });
-
-    return this;
-  }
-
-  async addConditionAsMigratablePages(user) {
-    this.query = this.query
-      .and({
-        $or: [
-          { grant: { $ne: GRANT_RESTRICTED } },
-          { grant: { $ne: GRANT_SPECIFIED } },
-        ],
-      });
-    this.addConditionAsNotMigrated();
-    this.addConditionAsNonRootPage();
-    this.addConditionToExcludeTrashed();
-    await this.addConditionForParentNormalization(user);
-
-    return this;
-  }
-
-  addConditionToFilteringByViewer(user, userGroups, showAnyoneKnowsLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false) {
-    const grantConditions = [
-      { grant: null },
-      { grant: GRANT_PUBLIC },
-    ];
-
-    if (showAnyoneKnowsLink) {
-      grantConditions.push({ grant: GRANT_RESTRICTED });
-    }
-
-    if (showPagesRestrictedByOwner) {
-      grantConditions.push(
-        { grant: GRANT_SPECIFIED },
-        { grant: GRANT_OWNER },
-      );
-    }
-    else if (user != null) {
-      grantConditions.push(
-        { grant: GRANT_SPECIFIED, grantedUsers: user._id },
-        { grant: GRANT_OWNER, grantedUsers: user._id },
-      );
-    }
-
-    if (showPagesRestrictedByGroup) {
-      grantConditions.push(
-        { grant: GRANT_USER_GROUP },
-      );
-    }
-    else if (userGroups != null && userGroups.length > 0) {
-      grantConditions.push(
-        { grant: GRANT_USER_GROUP, grantedGroup: { $in: userGroups } },
-      );
-    }
-
-    this.query = this.query
-      .and({
-        $or: grantConditions,
-      });
-
-    return this;
-  }
-
-  addConditionToPagenate(offset, limit, sortOpt) {
-    this.query = this.query
-      .sort(sortOpt).skip(offset).limit(limit); // eslint-disable-line newline-per-chained-call
-
-    return this;
-  }
-
-  addConditionAsNonRootPage() {
-    this.query = this.query.and({ path: { $ne: '/' } });
-
-    return this;
-  }
-
-  addConditionAsNotMigrated() {
-    this.query = this.query
-      .and({ parent: null });
-
-    return this;
-  }
-
-  addConditionAsMigrated() {
-    this.query = this.query
-      .and(
-        {
-          $or: [
-            { parent: { $ne: null } },
-            { path: '/' },
-          ],
-        },
-      );
-
-    return this;
-  }
-
-  /*
-   * Add this condition when get any ancestor pages including the target's parent
-   */
-  addConditionToSortPagesByDescPath() {
-    this.query = this.query.sort('-path');
-
-    return this;
-  }
-
-  addConditionToSortPagesByAscPath() {
-    this.query = this.query.sort('path');
-
-    return this;
-  }
-
-  addConditionToMinimizeDataForRendering() {
-    this.query = this.query.select('_id path isEmpty grant revision descendantCount');
-
-    return this;
-  }
-
-  addConditionToListByPathsArray(paths) {
-    this.query = this.query
-      .and({
-        path: {
-          $in: paths,
-        },
-      });
-
-    return this;
-  }
-
-  addConditionToListByPageIdsArray(pageIds) {
-    this.query = this.query
-      .and({
-        _id: {
-          $in: pageIds,
-        },
-      });
-
-    return this;
-  }
-
-  populateDataToList(userPublicFields) {
-    this.query = this.query
-      .populate({
-        path: 'lastUpdateUser',
-        select: userPublicFields,
-      });
-    return this;
-  }
-
-  populateDataToShowRevision(userPublicFields) {
-    this.query = populateDataToShowRevision(this.query, userPublicFields);
-    return this;
-  }
-
-  addConditionToFilteringByParentId(parentId) {
-    this.query = this.query.and({ parent: parentId });
-    return this;
-  }
-
-}
-
 export const getPageSchema = (crowi) => {
   let pageEvent;
 
@@ -638,7 +338,7 @@ export const getPageSchema = (crowi) => {
       userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
     }
 
-    const queryBuilder = new PageQueryBuilder(baseQuery);
+    const queryBuilder = new this.PageQueryBuilder(baseQuery);
     queryBuilder.addConditionToFilteringByViewer(user, userGroups, true);
 
     const count = await queryBuilder.query.exec();
@@ -660,7 +360,7 @@ export const getPageSchema = (crowi) => {
       relatedUserGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
     }
 
-    const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
+    const queryBuilder = new this.PageQueryBuilder(baseQuery, includeEmpty);
     queryBuilder.addConditionToFilteringByViewer(user, relatedUserGroups, true);
 
     return queryBuilder.query.exec();
@@ -668,7 +368,7 @@ export const getPageSchema = (crowi) => {
 
   pageSchema.statics.findByIdAndViewerToEdit = async function(id, user, includeEmpty = false) {
     const baseQuery = this.findOne({ _id: id });
-    const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
+    const queryBuilder = new this.PageQueryBuilder(baseQuery, includeEmpty);
 
     // add grant conditions
     await addConditionToFilteringByViewerToEdit(queryBuilder, user);
@@ -682,7 +382,7 @@ export const getPageSchema = (crowi) => {
       return null;
     }
 
-    const builder = new PageQueryBuilder(this.findOne({ path }), includeEmpty);
+    const builder = new this.PageQueryBuilder(this.findOne({ path }), includeEmpty);
 
     return builder.query.exec();
   };
@@ -713,7 +413,7 @@ export const getPageSchema = (crowi) => {
       relatedUserGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
     }
 
-    const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
+    const queryBuilder = new this.PageQueryBuilder(baseQuery, includeEmpty);
     queryBuilder.addConditionToFilteringByViewer(user, relatedUserGroups);
 
     return queryBuilder.query.exec();
@@ -723,7 +423,7 @@ export const getPageSchema = (crowi) => {
    * find pages that is match with `path` and its descendants
    */
   pageSchema.statics.findListWithDescendants = async function(path, user, option = {}, includeEmpty = false) {
-    const builder = new PageQueryBuilder(this.find(), includeEmpty);
+    const builder = new this.PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListWithDescendants(path, option);
 
     return findListFromBuilderAndViewer(builder, user, false, option);
@@ -737,7 +437,7 @@ export const getPageSchema = (crowi) => {
       return null;
     }
 
-    const builder = new PageQueryBuilder(this.find(), includeEmpty);
+    const builder = new this.PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListWithDescendants(page.path, option);
 
     // add grant conditions
@@ -758,7 +458,7 @@ export const getPageSchema = (crowi) => {
    * find pages that start with `path`
    */
   pageSchema.statics.findListByStartWith = async function(path, user, option, includeEmpty = false) {
-    const builder = new PageQueryBuilder(this.find(), includeEmpty);
+    const builder = new this.PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListByStartWith(path, option);
 
     return findListFromBuilderAndViewer(builder, user, false, option);
@@ -773,7 +473,7 @@ export const getPageSchema = (crowi) => {
    */
   pageSchema.statics.findListByCreator = async function(targetUser, currentUser, option) {
     const opt = Object.assign({ sort: 'createdAt', desc: -1 }, option);
-    const builder = new PageQueryBuilder(this.find({ creator: targetUser._id }));
+    const builder = new this.PageQueryBuilder(this.find({ creator: targetUser._id }));
 
     let showAnyoneKnowsLink = null;
     if (targetUser != null && currentUser != null) {
@@ -787,7 +487,7 @@ export const getPageSchema = (crowi) => {
     const User = crowi.model('User');
 
     const opt = Object.assign({}, option);
-    const builder = new PageQueryBuilder(this.find({ _id: { $in: ids } }), shouldIncludeEmpty);
+    const builder = new this.PageQueryBuilder(this.find({ _id: { $in: ids } }), shouldIncludeEmpty);
 
     builder.addConditionToPagenate(opt.offset, opt.limit);
 
@@ -1089,7 +789,7 @@ export const getPageSchema = (crowi) => {
   };
 
   pageSchema.statics.applyScopesToDescendantsAsyncronously = async function(parentPage, user) {
-    const builder = new PageQueryBuilder(this.find());
+    const builder = new this.PageQueryBuilder(this.find());
     builder.addConditionToListWithDescendants(parentPage.path);
 
     // add grant conditions
@@ -1117,7 +817,7 @@ export const getPageSchema = (crowi) => {
   };
 
   pageSchema.statics.findListByPathsArray = async function(paths, includeEmpty = false) {
-    const queryBuilder = new PageQueryBuilder(this.find(), includeEmpty);
+    const queryBuilder = new this.PageQueryBuilder(this.find(), includeEmpty);
     queryBuilder.addConditionToListByPathsArray(paths);
 
     return await queryBuilder.query.exec();
@@ -1215,7 +915,5 @@ export const getPageSchema = (crowi) => {
   pageSchema.statics.GRANT_USER_GROUP = GRANT_USER_GROUP;
   pageSchema.statics.PAGE_GRANT_ERROR = PAGE_GRANT_ERROR;
 
-  pageSchema.statics.PageQueryBuilder = PageQueryBuilder;
-
   return pageSchema;
 };

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

@@ -5,13 +5,14 @@ import mongoose, {
 } from 'mongoose';
 import mongoosePaginate from 'mongoose-paginate-v2';
 import uniqueValidator from 'mongoose-unique-validator';
+import escapeStringRegexp from 'escape-string-regexp';
 import nodePath from 'path';
-import { getOrCreateModel, pagePathUtils } from '@growi/core';
+import { getOrCreateModel, pagePathUtils, pathUtils } from '@growi/core';
 
 import loggerFactory from '../../utils/logger';
 import Crowi from '../crowi';
 import { IPage } from '../../interfaces/page';
-import { getPageSchema, PageQueryBuilder } from './obsolete-page';
+import { getPageSchema, extractToAncestorsPaths, populateDataToShowRevision } from './obsolete-page';
 import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 import { PageRedirectModel } from './page-redirect';
 
@@ -44,13 +45,17 @@ export type CreateMethod = (path: string, body: string, user, options) => Promis
 export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[], onlyMigratedAsExistingPages?: boolean, publicOnly?: boolean): Promise<void>
-  getParentAndFillAncestors(path: string): Promise<PageDocument & { _id: any }>
+  getParentAndFillAncestors(path: string, user): Promise<PageDocument & { _id: any }>
   findByIdsAndViewer(pageIds: string[], user, userGroups?, includeEmpty?: boolean): Promise<PageDocument[]>
   findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean, includeEmpty?: boolean): Promise<PageDocument[]>
   findTargetAndAncestorsByPathOrId(pathOrId: string): Promise<TargetAndAncestorsResult>
   findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups?): Promise<PageDocument[]>
   findAncestorsChildrenByPathAndViewer(path: string, user, userGroups?): Promise<Record<string, PageDocument[]>>
 
+  generateGrantCondition(
+    user, userGroups, showAnyoneKnowsLink?: boolean, showPagesRestrictedByOwner?: boolean, showPagesRestrictedByGroup?: boolean,
+  ): { $or: any[] }
+
   PageQueryBuilder: typeof PageQueryBuilder
 
   GRANT_PUBLIC
@@ -118,15 +123,291 @@ const generateChildrenRegExp = (path: string): RegExp => {
   return new RegExp(`^${path}(\\/[^/]+)\\/?$`);
 };
 
-/*
+class PageQueryBuilder {
+
+  query: any;
+
+  constructor(query, includeEmpty = false) {
+    this.query = query;
+    if (!includeEmpty) {
+      this.query = this.query
+        .and({
+          $or: [
+            { isEmpty: false },
+            { isEmpty: null }, // for v4 compatibility
+          ],
+        });
+    }
+  }
+
+  addConditionToExcludeTrashed() {
+    this.query = this.query
+      .and({
+        $or: [
+          { status: null },
+          { status: STATUS_PUBLISHED },
+        ],
+      });
+
+    return this;
+  }
+
+  /**
+   * generate the query to find the pages '{path}/*' and '{path}' self.
+   * If top page, return without doing anything.
+   */
+  addConditionToListWithDescendants(path: string, option?) {
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const pathNormalized = pathUtils.normalizePath(path);
+    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
+
+    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+
+    this.query = this.query
+      .and({
+        $or: [
+          { path: pathNormalized },
+          { path: new RegExp(`^${startsPattern}`) },
+        ],
+      });
+
+    return this;
+  }
+
+  /**
+   * generate the query to find the pages '{path}/*' (exclude '{path}' self).
+   * If top page, return without doing anything.
+   */
+  addConditionToListOnlyDescendants(path, option) {
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const pathWithTrailingSlash = pathUtils.addTrailingSlash(path);
+
+    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^${startsPattern}`) });
+
+    return this;
+
+  }
+
+  addConditionToListOnlyAncestors(path) {
+    const pathNormalized = pathUtils.normalizePath(path);
+    const ancestorsPaths = extractToAncestorsPaths(pathNormalized);
+
+    this.query = this.query
+      .and({
+        path: {
+          $in: ancestorsPaths,
+        },
+      });
+
+    return this;
+
+  }
+
+  /**
+   * generate the query to find pages that start with `path`
+   *
+   * In normal case, returns '{path}/*' and '{path}' self.
+   * If top page, return without doing anything.
+   *
+   * *option*
+   *   Left for backward compatibility
+   */
+  addConditionToListByStartWith(path, option?) {
+    // No request is set for the top page
+    if (isTopPage(path)) {
+      return this;
+    }
+
+    const startsPattern = escapeStringRegexp(path);
+
+    this.query = this.query
+      .and({ path: new RegExp(`^${startsPattern}`) });
+
+    return this;
+  }
+
+  async addConditionForParentNormalization(user) {
+    // determine UserGroup condition
+    let userGroups;
+    if (user != null) {
+      const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
+      userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
+    }
+
+    const grantConditions: any[] = [
+      { grant: null },
+      { grant: GRANT_PUBLIC },
+    ];
+
+    if (user != null) {
+      grantConditions.push(
+        { grant: GRANT_OWNER, grantedUsers: user._id },
+      );
+    }
+
+    if (userGroups != null && userGroups.length > 0) {
+      grantConditions.push(
+        { grant: GRANT_USER_GROUP, grantedGroup: { $in: userGroups } },
+      );
+    }
+
+    this.query = this.query
+      .and({
+        $or: grantConditions,
+      });
+
+    return this;
+  }
+
+  async addConditionAsMigratablePages(user) {
+    this.query = this.query
+      .and({
+        $or: [
+          { grant: { $ne: GRANT_RESTRICTED } },
+          { grant: { $ne: GRANT_SPECIFIED } },
+        ],
+      });
+    this.addConditionAsNotMigrated();
+    this.addConditionAsNonRootPage();
+    this.addConditionToExcludeTrashed();
+    await this.addConditionForParentNormalization(user);
+
+    return this;
+  }
+
+  addConditionToFilteringByViewer(user, userGroups, showAnyoneKnowsLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false) {
+    const condition = generateGrantCondition(user, userGroups, showAnyoneKnowsLink, showPagesRestrictedByOwner, showPagesRestrictedByGroup);
+
+    this.query = this.query
+      .and(condition);
+
+    return this;
+  }
+
+  addConditionToPagenate(offset, limit, sortOpt?) {
+    this.query = this.query
+      .sort(sortOpt).skip(offset).limit(limit); // eslint-disable-line newline-per-chained-call
+
+    return this;
+  }
+
+  addConditionAsNonRootPage() {
+    this.query = this.query.and({ path: { $ne: '/' } });
+
+    return this;
+  }
+
+  addConditionAsNotMigrated() {
+    this.query = this.query
+      .and({ parent: null });
+
+    return this;
+  }
+
+  addConditionAsMigrated() {
+    this.query = this.query
+      .and(
+        {
+          $or: [
+            { parent: { $ne: null } },
+            { path: '/' },
+          ],
+        },
+      );
+
+    return this;
+  }
+
+  /*
+   * Add this condition when get any ancestor pages including the target's parent
+   */
+  addConditionToSortPagesByDescPath() {
+    this.query = this.query.sort('-path');
+
+    return this;
+  }
+
+  addConditionToSortPagesByAscPath() {
+    this.query = this.query.sort('path');
+
+    return this;
+  }
+
+  addConditionToMinimizeDataForRendering() {
+    this.query = this.query.select('_id path isEmpty grant revision descendantCount');
+
+    return this;
+  }
+
+  addConditionToListByPathsArray(paths) {
+    this.query = this.query
+      .and({
+        path: {
+          $in: paths,
+        },
+      });
+
+    return this;
+  }
+
+  addConditionToListByPageIdsArray(pageIds) {
+    this.query = this.query
+      .and({
+        _id: {
+          $in: pageIds,
+        },
+      });
+
+    return this;
+  }
+
+  populateDataToList(userPublicFields) {
+    this.query = this.query
+      .populate({
+        path: 'lastUpdateUser',
+        select: userPublicFields,
+      });
+    return this;
+  }
+
+  populateDataToShowRevision(userPublicFields) {
+    this.query = populateDataToShowRevision(this.query, userPublicFields);
+    return this;
+  }
+
+  addConditionToFilteringByParentId(parentId) {
+    this.query = this.query.and({ parent: parentId });
+    return this;
+  }
+
+}
+
+/**
  * Create empty pages if the page in paths didn't exist
+ * @param onlyMigratedAsExistingPages Determine whether to include non-migrated pages as existing pages. If a page exists,
+ * an empty page will not be created at that page's path.
  */
-schema.statics.createEmptyPagesByPaths = async function(paths: string[], onlyMigratedAsExistingPages = true, publicOnly = false): Promise<void> {
+schema.statics.createEmptyPagesByPaths = async function(paths: string[], user: any | null, onlyMigratedAsExistingPages = true): Promise<void> {
   // find existing parents
-  const builder = new PageQueryBuilder(this.find(publicOnly ? { grant: GRANT_PUBLIC } : {}, { _id: 0, path: 1 }), true);
+  const builder = new PageQueryBuilder(this.find({}, { _id: 0, path: 1 }), true);
+
+  await this.addConditionToFilteringByViewerToEdit(builder, user);
+
   if (onlyMigratedAsExistingPages) {
     builder.addConditionAsMigrated();
   }
+
   const existingPages = await builder
     .addConditionToListByPathsArray(paths)
     .query
@@ -220,7 +501,7 @@ schema.statics.replaceTargetWithPage = async function(exPage, pageToReplaceWith?
  * @param path string
  * @returns Promise<PageDocument>
  */
-schema.statics.getParentAndFillAncestors = async function(path: string): Promise<PageDocument> {
+schema.statics.getParentAndFillAncestors = async function(path: string, user): Promise<PageDocument> {
   const parentPath = nodePath.dirname(path);
 
   const builder1 = new PageQueryBuilder(this.find({ path: parentPath }), true);
@@ -239,7 +520,7 @@ schema.statics.getParentAndFillAncestors = async function(path: string): Promise
   const ancestorPaths = collectAncestorPaths(path); // paths of parents need to be created
 
   // just create ancestors with empty pages
-  await this.createEmptyPagesByPaths(ancestorPaths);
+  await this.createEmptyPagesByPaths(ancestorPaths, user);
 
   // find ancestors
   const builder2 = new PageQueryBuilder(this.find(), true);
@@ -571,6 +852,63 @@ schema.statics.takeOffFromTree = async function(pageId: ObjectIdLike) {
   return this.findByIdAndUpdate(pageId, { $set: { parent: null } });
 };
 
+schema.statics.removeEmptyPages = async function(pageIdsToNotRemove: ObjectIdLike[], paths: string[]): Promise<void> {
+  await this.deleteMany({
+    _id: {
+      $nin: pageIdsToNotRemove,
+    },
+    path: {
+      $in: paths,
+    },
+    isEmpty: true,
+  });
+};
+
+schema.statics.PageQueryBuilder = PageQueryBuilder as any; // mongoose does not support constructor type as statics attrs type
+
+export function generateGrantCondition(
+    user, userGroups, showAnyoneKnowsLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false,
+): { $or: any[] } {
+  const grantConditions: AnyObject[] = [
+    { grant: null },
+    { grant: GRANT_PUBLIC },
+  ];
+
+  if (showAnyoneKnowsLink) {
+    grantConditions.push({ grant: GRANT_RESTRICTED });
+  }
+
+  if (showPagesRestrictedByOwner) {
+    grantConditions.push(
+      { grant: GRANT_SPECIFIED },
+      { grant: GRANT_OWNER },
+    );
+  }
+  else if (user != null) {
+    grantConditions.push(
+      { grant: GRANT_SPECIFIED, grantedUsers: user._id },
+      { grant: GRANT_OWNER, grantedUsers: user._id },
+    );
+  }
+
+  if (showPagesRestrictedByGroup) {
+    grantConditions.push(
+      { grant: GRANT_USER_GROUP },
+    );
+  }
+  else if (userGroups != null && userGroups.length > 0) {
+    grantConditions.push(
+      { grant: GRANT_USER_GROUP, grantedGroup: { $in: userGroups } },
+    );
+  }
+
+  return {
+    $or: grantConditions,
+  };
+}
+
+schema.statics.generateGrantCondition = generateGrantCondition;
+
 export type PageCreateOptions = {
   format?: string
   grantUserGroupId?: ObjectIdLike
@@ -661,7 +999,7 @@ export default (crowi: Crowi): any => {
     }
 
     let parentId: IObjectId | string | null = null;
-    const parent = await Page.getParentAndFillAncestors(path);
+    const parent = await Page.getParentAndFillAncestors(path, user);
     if (!isTopPage(path)) {
       parentId = parent._id;
     }
@@ -775,60 +1113,3 @@ export default (crowi: Crowi): any => {
 
   return getOrCreateModel<PageDocument, PageModel>('Page', schema as any); // TODO: improve type
 };
-
-/*
- * Aggregation utilities
- */
-// TODO: use the original type when upgraded https://github.com/Automattic/mongoose/blob/master/index.d.ts#L3090
-type PipelineStageMatch = {
-  $match: AnyObject
-};
-
-export const generateGrantCondition = async(
-    user, _userGroups, showAnyoneKnowsLink = false, showPagesRestrictedByOwner = false, showPagesRestrictedByGroup = false,
-): Promise<PipelineStageMatch> => {
-  let userGroups = _userGroups;
-  if (user != null && userGroups == null) {
-    const UserGroupRelation: any = mongoose.model('UserGroupRelation');
-    userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
-  }
-
-  const grantConditions: AnyObject[] = [
-    { grant: null },
-    { grant: GRANT_PUBLIC },
-  ];
-
-  if (showAnyoneKnowsLink) {
-    grantConditions.push({ grant: GRANT_RESTRICTED });
-  }
-
-  if (showPagesRestrictedByOwner) {
-    grantConditions.push(
-      { grant: GRANT_SPECIFIED },
-      { grant: GRANT_OWNER },
-    );
-  }
-  else if (user != null) {
-    grantConditions.push(
-      { grant: GRANT_SPECIFIED, grantedUsers: user._id },
-      { grant: GRANT_OWNER, grantedUsers: user._id },
-    );
-  }
-
-  if (showPagesRestrictedByGroup) {
-    grantConditions.push(
-      { grant: GRANT_USER_GROUP },
-    );
-  }
-  else if (userGroups != null && userGroups.length > 0) {
-    grantConditions.push(
-      { grant: GRANT_USER_GROUP, grantedGroup: { $in: userGroups } },
-    );
-  }
-
-  return {
-    $match: {
-      $or: grantConditions,
-    },
-  };
-};

packages/app/src/server/routes/page.js

@@ -4,7 +4,6 @@ import { body } from 'express-validator';
 import mongoose from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
-import { PageQueryBuilder } from '../models/obsolete-page';
 import UpdatePost from '../models/update-post';
 
 const { isCreatablePage, isTopPage, isUsersHomePage } = pagePathUtils;
@@ -146,6 +145,8 @@ module.exports = function(crowi, app) {
   const ShareLink = crowi.model('ShareLink');
   const PageRedirect = mongoose.model('PageRedirect');
 
+  const { PageQueryBuilder } = Page;
+
   const ApiResponse = require('../util/apiResponse');
   const getToday = require('../util/getToday');
 

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

@@ -4,7 +4,6 @@ import escapeStringRegexp from 'escape-string-regexp';
 
 import UserGroup from '~/server/models/user-group';
 import { PageDocument, PageModel } from '~/server/models/page';
-import { PageQueryBuilder } from '../models/obsolete-page';
 import { isIncludesObjectId, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
 const { addTrailingSlash } = pathUtils;
@@ -216,6 +215,7 @@ class PageGrantService {
    */
   private async generateComparableAncestor(targetPath: string, includeNotMigratedPages: boolean): Promise<ComparableAncestor> {
     const Page = mongoose.model('Page') as unknown as PageModel;
+    const { PageQueryBuilder } = Page;
     const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
 
     let applicableUserIds: ObjectIdLike[] | undefined;

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

@@ -8,7 +8,7 @@ import { Readable, Writable } from 'stream';
 import { createBatchStream } from '~/server/util/batch-stream';
 import loggerFactory from '~/utils/logger';
 import {
-  CreateMethod, generateGrantCondition, PageCreateOptions, PageModel, PageDocument,
+  CreateMethod, PageCreateOptions, PageModel, PageDocument,
 } from '~/server/models/page';
 import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
 import {
@@ -460,7 +460,7 @@ class PageService {
       newParent = await this.getParentAndforceCreateEmptyTree(page, newPagePath);
     }
     else {
-      newParent = await Page.getParentAndFillAncestors(newPagePath);
+      newParent = await Page.getParentAndFillAncestors(newPagePath, user);
     }
 
     // 3. Put back target page to tree (also update the other attrs)
@@ -910,7 +910,7 @@ class PageService {
     };
     let duplicatedTarget;
     if (page.isEmpty) {
-      const parent = await Page.getParentAndFillAncestors(newPagePath);
+      const parent = await Page.getParentAndFillAncestors(newPagePath, user);
       duplicatedTarget = await Page.createEmptyPage(newPagePath, parent);
     }
     else {
@@ -963,7 +963,7 @@ class PageService {
     const shouldNormalize = this.shouldNormalizeParent(page);
     if (shouldNormalize) {
       try {
-        await this.normalizeParentAndDescendantCountOfDescendants(newPagePath);
+        await this.normalizeParentAndDescendantCountOfDescendants(newPagePath, user);
         logger.info(`Successfully normalized duplicated descendant pages under "${newPagePath}"`);
       }
       catch (err) {
@@ -1839,7 +1839,7 @@ class PageService {
     }
 
     // 2. Revert target
-    const parent = await Page.getParentAndFillAncestors(newPath);
+    const parent = await Page.getParentAndFillAncestors(newPath, user);
     const updatedPage = await Page.findByIdAndUpdate(page._id, {
       $set: {
         path: newPath, status: Page.STATUS_PUBLISHED, lastUpdateUser: user._id, deleteUser: null, deletedAt: null, parent: parent._id, descendantCount: 0,
@@ -1886,7 +1886,7 @@ class PageService {
     const shouldNormalize = this.shouldNormalizeParent(page);
     if (shouldNormalize) {
       try {
-        await this.normalizeParentAndDescendantCountOfDescendants(newPath);
+        await this.normalizeParentAndDescendantCountOfDescendants(newPath, user);
         logger.info(`Successfully normalized reverted descendant pages under "${newPath}"`);
       }
       catch (err) {
@@ -2082,11 +2082,16 @@ class PageService {
   }
 
   async shortBodiesMapByPageIds(pageIds: ObjectId[] = [], user): Promise<Record<string, string | null>> {
-    const Page = mongoose.model('Page');
+    const Page = mongoose.model('Page') as unknown as PageModel;
     const MAX_LENGTH = 350;
 
     // aggregation options
-    const viewerCondition = await generateGrantCondition(user, null);
+    let userGroups;
+    if (user != null && userGroups == null) {
+      const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // Typescriptize model
+      userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
+    }
+    const viewerCondition = Page.generateGrantCondition(user, userGroups);
     const filterByIds = {
       _id: { $in: pageIds },
     };
@@ -2100,7 +2105,9 @@ class PageService {
             $match: filterByIds,
           },
           // filter by viewer
-          viewerCondition,
+          {
+            $match: viewerCondition,
+          },
           // lookup: https://docs.mongodb.com/v4.4/reference/operator/aggregation/lookup/
           {
             $lookup: {
@@ -2253,7 +2260,7 @@ class PageService {
     }
     else {
       // getParentAndFillAncestors
-      const parent = await Page.getParentAndFillAncestors(page.path);
+      const parent = await Page.getParentAndFillAncestors(page.path, user);
       updatedPage = await Page.findOneAndUpdate({ _id: page._id }, { parent: parent._id }, { new: true });
     }
 
@@ -2326,7 +2333,7 @@ class PageService {
     // TODO: insertOne PageOperationBlock
 
     try {
-      await this.normalizeParentRecursively([page.path]);
+      await this.normalizeParentRecursively([page.path], user);
     }
     catch (err) {
       logger.error('V5 initial miration failed.', err);
@@ -2425,7 +2432,7 @@ class PageService {
 
     // then migrate
     try {
-      await this.normalizeParentRecursively(['/'], true);
+      await this.normalizeParentRecursively(['/'], null);
     }
     catch (err) {
       logger.error('V5 initial miration failed.', err);
@@ -2460,40 +2467,46 @@ class PageService {
     }
   }
 
-  private async normalizeParentAndDescendantCountOfDescendants(path: string): Promise<void> {
-    await this.normalizeParentRecursively([path]);
+  private async normalizeParentAndDescendantCountOfDescendants(path: string, user): Promise<void> {
+    await this.normalizeParentRecursively([path], user);
 
     // update descendantCount of descendant pages
     await this.updateDescendantCountOfSelfAndDescendants(path);
   }
 
-  async normalizeParentRecursively(paths: string[], publicOnly = false): Promise<void> {
+  /**
+   * Normalize parent attribute by passing paths and user.
+   * @param paths Pages under this paths value will be updated.
+   * @param user To be used to filter pages to update. If null, only public pages will be updated.
+   * @returns Promise<void>
+   */
+  async normalizeParentRecursively(paths: string[], user: any | null): Promise<void> {
+    const Page = mongoose.model('Page') as unknown as PageModel;
+
     const ancestorPaths = paths.flatMap(p => collectAncestorPaths(p, [p]));
     const regexps = paths.map(p => new RegExp(`^${escapeStringRegexp(addTrailingSlash(p))}`, 'i'));
 
-    return this._normalizeParentRecursively(regexps, ancestorPaths, publicOnly);
+    // determine UserGroup condition
+    let userGroups = null;
+    if (user != null) {
+      const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
+      userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
+    }
+
+    const grantFiltersByUser: { $or: any[] } = Page.generateGrantCondition(user, userGroups);
+
+    return this._normalizeParentRecursively(regexps, ancestorPaths, grantFiltersByUser, user);
   }
 
   // TODO: use websocket to show progress
-  private async _normalizeParentRecursively(regexps: RegExp[], pathsToInclude: string[], publicOnly: boolean): Promise<void> {
+  private async _normalizeParentRecursively(regexps: RegExp[], pathsToInclude: string[], grantFiltersByUser: { $or: any[] }, user): Promise<void> {
     const BATCH_SIZE = 100;
     const PAGES_LIMIT = 1000;
+
     const Page = mongoose.model('Page') as unknown as PageModel;
     const { PageQueryBuilder } = Page;
 
-    // GRANT_RESTRICTED and GRANT_SPECIFIED will never have parent
-    const grantFilter: any = {
-      $and: [
-        { grant: { $ne: Page.GRANT_RESTRICTED } },
-        { grant: { $ne: Page.GRANT_SPECIFIED } },
-      ],
-    };
-
-    if (publicOnly) { // add grant condition if not null
-      grantFilter.$and = [...grantFilter.$and, { grant: Page.GRANT_PUBLIC }];
-    }
-
-    // generate filter
+    // Build filter
     const filter: any = {
       $and: [
         {
@@ -2518,11 +2531,9 @@ class PageService {
       });
     }
 
-    const total = await Page.countDocuments(filter);
-
     let baseAggregation = Page
       .aggregate([
-        { $match: grantFilter },
+        { $match: grantFiltersByUser },
         { $match: filter },
         {
           $project: { // minimize data to fetch
@@ -2533,6 +2544,7 @@ class PageService {
       ]);
 
     // limit pages to get
+    const total = await Page.countDocuments(filter);
     if (total > PAGES_LIMIT) {
       baseAggregation = baseAggregation.limit(Math.floor(total * 0.3));
     }
@@ -2545,18 +2557,15 @@ class PageService {
     let countPages = 0;
     let shouldContinue = true;
 
-    // migrate all siblings for each page
     const migratePagesStream = new Writable({
       objectMode: true,
       async write(pages, encoding, callback) {
-        // make list to create empty pages
-        const parentPathsSet = new Set<string>(pages.map(page => pathlib.dirname(page.path)));
-        const parentPaths = Array.from(parentPathsSet);
+        const parentPaths = Array.from(new Set<string>(pages.map(p => pathlib.dirname(p.path))));
 
-        // fill parents with empty pages
-        await Page.createEmptyPagesByPaths(parentPaths, false, publicOnly);
+        // Fill parents with empty pages
+        await Page.createEmptyPagesByPaths(parentPaths, user, false);
 
-        // find parents again
+        // Find parents
         const builder = new PageQueryBuilder(Page.find({}, { _id: 1, path: 1 }), true);
         const parents = await builder
           .addConditionToListByPathsArray(parentPaths)
@@ -2564,22 +2573,20 @@ class PageService {
           .lean()
           .exec();
 
-        // bulkWrite to update parent
+        // Normalize all siblings for each page
         const updateManyOperations = parents.map((parent) => {
           const parentId = parent._id;
 
-          // modify to adjust for RegExp
-          let parentPath = parent.path === '/' ? '' : parent.path;
-          parentPath = escapeStringRegexp(parentPath);
-
+          // Build filter
+          const parentPathEscaped = escapeStringRegexp(parent.path === '/' ? '' : parent.path); // adjust the path for RegExp
           const filter: any = {
-            // regexr.com/6889f
-            // ex. /parent/any_child OR /any_level1
-            path: { $regex: new RegExp(`^${parentPath}(\\/[^/]+)\\/?$`, 'i') },
+            $and: [
+              {
+                path: { $regex: new RegExp(`^${parentPathEscaped}(\\/[^/]+)\\/?$`, 'i') }, // see: regexr.com/6889f (e.g. /parent/any_child or /any_level1)
+              },
+              grantFiltersByUser,
+            ],
           };
-          if (publicOnly) {
-            filter.grant = Page.GRANT_PUBLIC;
-          }
 
           return {
             updateMany: {
@@ -2592,16 +2599,17 @@ class PageService {
         });
         try {
           const res = await Page.bulkWrite(updateManyOperations);
+
           countPages += res.result.nModified;
           logger.info(`Page migration processing: (count=${countPages})`);
 
-          // throw
+          // Throw if any error is found
           if (res.result.writeErrors.length > 0) {
             logger.error('Failed to migrate some pages', res.result.writeErrors);
             throw Error('Failed to migrate some pages');
           }
 
-          // finish migration
+          // Finish migration if no modification occurred
           if (res.result.nModified === 0 && res.result.nMatched === 0) {
             shouldContinue = false;
             logger.error('Migration is unable to continue', 'parentPaths:', parentPaths, 'bulkWriteResult:', res);
@@ -2612,6 +2620,9 @@ class PageService {
           throw err;
         }
 
+        // Remove unnecessary empty pages
+        await Page.removeEmptyPages(pages.map(p => p._id), pages.map(p => p.path));
+
         callback();
       },
       final(callback) {
@@ -2625,9 +2636,9 @@ class PageService {
 
     await streamToPromise(migratePagesStream);
 
-    const existsFilter = { $and: [...grantFilter.$and, ...filter.$and] };
+    const existsFilter = { $and: [grantFiltersByUser, ...filter.$and] };
     if (await Page.exists(existsFilter) && shouldContinue) {
-      return this._normalizeParentRecursively(regexps, pathsToInclude, publicOnly);
+      return this._normalizeParentRecursively(regexps, pathsToInclude, grantFiltersByUser, user);
     }
 
   }

packages/app/test/integration/service/v5.migration.test.js

@@ -28,7 +28,7 @@ describe('V5 page migration', () => {
       jest.restoreAllMocks();
 
       // initialize pages for test
-      let pages = await Page.insertMany([
+      await Page.insertMany([
         {
           path: '/private1',
           grant: Page.GRANT_OWNER,
@@ -59,11 +59,6 @@ describe('V5 page migration', () => {
         },
       ]);
 
-      if (!await Page.exists({ path: '/' })) {
-        const additionalPages = await Page.insertMany([{ path: '/', grant: Page.GRANT_PUBLIC }]);
-        pages = [...additionalPages, ...pages];
-      }
-
       const pagesToRun = await Page.find({ path: { $in: ['/private1', '/dummyParent/private1'] } });
 
       // migrate