Просмотр исходного кода

Merge pull request #5578 from weseek/fix/normalize-parent-add-test-and-fix

fix: Normalize parent add test and fix
Yuki Takei 4 лет назад
Родитель
Сommit
646a8a88a1

+ 4 - 1
packages/app/src/components/LegacyPrivatePagesMigrationModal.tsx

@@ -55,7 +55,10 @@ export const LegacyPrivatePagesMigrationModal = (props: Props): JSX.Element => {
           className="custom-control-input"
           id="convertRecursively"
           type="checkbox"
-          onChange={e => setIsRecursively(e.target.checked)}
+          checked={isRecursively}
+          onChange={(e) => {
+            setIsRecursively(e.target.checked);
+          }}
         />
         <label className="custom-control-label" htmlFor="convertRecursively">
           { t('private_legacy_pages.modal.convert_recursively_label') }

+ 1 - 0
packages/app/src/server/interfaces/page-operation.ts

@@ -22,4 +22,5 @@ export type IUserForResuming = {
 export type IOptionsForResuming = {
   updateMetadata?: boolean,
   createRedirectPage?: boolean,
+  prevDescendantCount?: number,
 };

+ 1 - 0
packages/app/src/server/models/page-operation.ts

@@ -72,6 +72,7 @@ const userSchemaForResuming = new Schema<IUserForResuming>({
 const optionsSchemaForResuming = new Schema<IOptionsForResuming>({
   createRedirectPage: { type: Boolean },
   updateMetadata: { type: Boolean },
+  prevDescendantCount: { type: Number },
 }, { _id: false });
 
 const schema = new Schema<PageOperationDocument, PageOperationModel>({

+ 57 - 18
packages/app/src/server/models/page.ts

@@ -45,7 +45,7 @@ type TargetAndAncestorsResult = {
 export type CreateMethod = (path: string, body: string, user, options) => Promise<PageDocument & { _id: any }>
 export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
-  createEmptyPagesByPaths(paths: string[], onlyMigratedAsExistingPages?: boolean, publicOnly?: boolean): Promise<void>
+  createEmptyPagesByPaths(paths: string[], user: any | null, onlyMigratedAsExistingPages?: boolean, andFilter?): Promise<void>
   getParentAndFillAncestors(path: string, user): Promise<PageDocument & { _id: any }>
   findByIdsAndViewer(pageIds: ObjectIdLike[], user, userGroups?, includeEmpty?: boolean): Promise<PageDocument[]>
   findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean, includeEmpty?: boolean): Promise<PageDocument[]>
@@ -141,6 +141,27 @@ class PageQueryBuilder {
     }
   }
 
+  /**
+   * Used for filtering the pages at specified paths not to include unintentional pages.
+   * @param pathsToFilter The paths to have additional filters as to be applicable
+   * @returns PageQueryBuilder
+   */
+  addConditionToFilterByApplicableAncestors(pathsToFilter: string[]) {
+    this.query = this.query
+      .and(
+        {
+          $or: [
+            { path: '/' },
+            { path: { $in: pathsToFilter }, grant: GRANT_PUBLIC, status: STATUS_PUBLISHED },
+            { path: { $in: pathsToFilter }, parent: { $ne: null }, status: STATUS_PUBLISHED },
+            { path: { $nin: pathsToFilter }, status: STATUS_PUBLISHED },
+          ],
+        },
+      );
+
+    return this;
+  }
+
   addConditionToExcludeTrashed() {
     this.query = this.query
       .and({
@@ -410,23 +431,39 @@ class PageQueryBuilder {
  * @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[], user: any | null, onlyMigratedAsExistingPages = true): Promise<void> {
-  // find existing parents
-  const builder = new PageQueryBuilder(this.find({}, { _id: 0, path: 1 }), true);
-
-  await this.addConditionToFilteringByViewerToEdit(builder, user);
-
+schema.statics.createEmptyPagesByPaths = async function(paths: string[], user: any | null, onlyMigratedAsExistingPages = true, filter?): Promise<void> {
+  const aggregationPipeline: any[] = [];
+  // 1. Filter by paths
+  aggregationPipeline.push({ $match: { path: { $in: paths } } });
+  // 2. Normalized condition
   if (onlyMigratedAsExistingPages) {
-    builder.addConditionAsMigrated();
+    aggregationPipeline.push({
+      $match: {
+        $or: [
+          { parent: { $ne: null } },
+          { path: '/' },
+        ],
+      },
+    });
   }
+  // 3. Add custom pipeline
+  if (filter != null) {
+    aggregationPipeline.push({ $match: filter });
+  }
+  // 4. Add grant conditions
+  let userGroups = null;
+  if (user != null) {
+    const UserGroupRelation = mongoose.model('UserGroupRelation') as any;
+    userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
+  }
+  const grantCondition = this.generateGrantCondition(user, userGroups);
+  aggregationPipeline.push({ $match: grantCondition });
 
-  const existingPages = await builder
-    .addConditionToListByPathsArray(paths)
-    .query
-    .lean()
-    .exec();
-  const existingPagePaths = existingPages.map(page => page.path);
+  // Run aggregation
+  const existingPages = await this.aggregate(aggregationPipeline);
 
+
+  const existingPagePaths = existingPages.map(page => page.path);
   // paths to create empty pages
   const notExistingPagePaths = paths.filter(path => !existingPagePaths.includes(path));
 
@@ -536,6 +573,10 @@ schema.statics.getParentAndFillAncestors = async function(path: string, user): P
 
   // find ancestors
   const builder2 = new PageQueryBuilder(this.find(), true);
+
+  // avoid including not normalized pages
+  builder2.addConditionToFilterByApplicableAncestors(ancestorPaths);
+
   const ancestors = await builder2
     .addConditionToListByPathsArray(ancestorPaths)
     .addConditionToSortPagesByDescPath()
@@ -974,7 +1015,7 @@ export default (crowi: Crowi): any => {
         const shouldCheckDescendants = emptyPage != null;
         const newGrantedUserIds = grant === GRANT_OWNER ? [user._id] as IObjectId[] : undefined;
 
-        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(path, grant, newGrantedUserIds, grantUserGroupId, shouldCheckDescendants);
+        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(user, path, grant, newGrantedUserIds, grantUserGroupId, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}" of grant ${grant}:`, err);
@@ -1078,9 +1119,7 @@ export default (crowi: Crowi): any => {
     if (shouldBeOnTree) {
       let isGrantNormalized = false;
       try {
-        const shouldCheckDescendants = true;
-
-        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(pageData.path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+        isGrantNormalized = await crowi.pageGrantService.isGrantNormalized(user, pageData.path, grant, grantedUserIds, grantUserGroupId, true);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${pageData.path}" of grant ${grant}:`, err);

+ 3 - 1
packages/app/src/server/routes/apiv3/pages.js

@@ -197,7 +197,9 @@ module.exports = (crowi) => {
     ],
     legacyPagesMigration: [
       body('pageIds').isArray().withMessage('pageIds is required'),
-      body('isRecursively').isBoolean().withMessage('isRecursively is required'),
+      body('isRecursively')
+        .custom(v => v === 'true' || v === true || v == null)
+        .withMessage('The body property "isRecursively" must be "true" or true. (Omit param for false)'),
     ],
   };
 

+ 33 - 16
packages/app/src/server/service/page-grant.ts

@@ -259,21 +259,40 @@ class PageGrantService {
    * @param targetPath string of the target path
    * @returns ComparableDescendants
    */
-  private async generateComparableDescendants(targetPath: string, includeNotMigratedPages: boolean): Promise<ComparableDescendants> {
+  private async generateComparableDescendants(targetPath: string, user, includeNotMigratedPages: boolean): Promise<ComparableDescendants> {
     const Page = mongoose.model('Page') as unknown as PageModel;
+    const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
 
-    /*
-     * make granted users list of descendant's
-     */
-    const pathWithTrailingSlash = addTrailingSlash(targetPath);
-    const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
+    // Build conditions
+    const $match: {$or: any} = {
+      $or: [],
+    };
+
+    const commonCondition = {
+      path: new RegExp(`^${escapeStringRegexp(addTrailingSlash(targetPath))}`, 'i'),
+      isEmpty: false,
+    };
 
-    const $match: any = {
-      path: new RegExp(`^${startsPattern}`),
-      isEmpty: { $ne: true },
+    const conditionForNormalizedPages: any = {
+      ...commonCondition,
+      parent: { $ne: null },
     };
+    $match.$or.push(conditionForNormalizedPages);
+
     if (includeNotMigratedPages) {
-      $match.parent = { $ne: null };
+      // Add grantCondition for not normalized pages
+      const userGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
+      const grantCondition = Page.generateGrantCondition(user, userGroups);
+      const conditionForNotNormalizedPages = {
+        $and: [
+          {
+            ...commonCondition,
+            parent: null,
+          },
+          grantCondition,
+        ],
+      };
+      $match.$or.push(conditionForNotNormalizedPages);
     }
 
     const result = await Page.aggregate([
@@ -327,7 +346,7 @@ class PageGrantService {
    */
   async isGrantNormalized(
       // eslint-disable-next-line max-len
-      targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false, includeNotMigratedPages = false,
+      user, targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false, includeNotMigratedPages = false,
   ): Promise<boolean> {
     if (isTopPage(targetPath)) {
       return true;
@@ -341,7 +360,7 @@ class PageGrantService {
     }
 
     const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupId, true);
-    const comparableDescendants = await this.generateComparableDescendants(targetPath, includeNotMigratedPages);
+    const comparableDescendants = await this.generateComparableDescendants(targetPath, user, includeNotMigratedPages);
 
     return this.processValidation(comparableTarget, comparableAncestor, comparableDescendants);
   }
@@ -352,13 +371,11 @@ class PageGrantService {
    * @param pageIds pageIds to be tested
    * @returns a tuple with the first element of normalizable pages and the second element of NOT normalizable pages
    */
-  async separateNormalizableAndNotNormalizablePages(pages): Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]> {
+  async separateNormalizableAndNotNormalizablePages(user, pages): Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]> {
     if (pages.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
       throw Error(`The maximum number of pageIds allowed is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
 
-    const Page = mongoose.model('Page') as unknown as PageModel;
-
     const shouldCheckDescendants = true;
     const shouldIncludeNotMigratedPages = true;
 
@@ -375,7 +392,7 @@ class PageGrantService {
         continue;
       }
 
-      if (await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
+      if (await this.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
         normalizable.push(page);
       }
       else {

+ 80 - 48
packages/app/src/server/service/page.ts

@@ -486,9 +486,7 @@ class PageService {
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       try {
-        const shouldCheckDescendants = false;
-
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(newPagePath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupId, false);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when renaming`, err);
@@ -942,9 +940,7 @@ class PageService {
     if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       try {
-        const shouldCheckDescendants = false;
-
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(newPagePath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, newPagePath, grant, grantedUserIds, grantedGroupId, false);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${newPagePath}" when duplicating`, err);
@@ -2257,23 +2253,20 @@ class PageService {
           throw Error(`Cannot operate normalizeParent to path "${page.path}" right now.`);
         }
 
-        const normalizedPage = await this.normalizeParentByPageId(pageId, user);
+        const normalizedPage = await this.normalizeParentByPage(page, user);
 
         if (normalizedPage == null) {
           logger.error(`Failed to update descendantCount of page of id: "${pageId}"`);
         }
-        else {
-          // update descendantCount of ancestors'
-          await this.updateDescendantCountOfAncestors(pageId, normalizedPage.descendantCount, false);
-        }
       }
       catch (err) {
+        logger.error('Something went wrong while normalizing parent.', err);
         // socket.emit('normalizeParentByPageIds', { error: err.message }); TODO: use socket to tell user
       }
     }
   }
 
-  private async normalizeParentByPageId(page, user) {
+  private async normalizeParentByPage(page, user) {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     const {
@@ -2281,7 +2274,7 @@ class PageService {
     } = page;
 
     // check if any page exists at target path already
-    const existingPage = await Page.findOne({ path });
+    const existingPage = await Page.findOne({ path, parent: { $ne: null } });
     if (existingPage != null && !existingPage.isEmpty) {
       throw Error('Page already exists. Please rename the page to continue.');
     }
@@ -2294,7 +2287,7 @@ class PageService {
       try {
         const shouldCheckDescendants = true;
 
-        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(user, path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${path}"`, err);
@@ -2316,11 +2309,14 @@ class PageService {
       updatedPage = await Page.findById(page._id);
     }
     else {
-      // getParentAndFillAncestors
       const parent = await Page.getParentAndFillAncestors(page.path, user);
       updatedPage = await Page.findOneAndUpdate({ _id: page._id }, { parent: parent._id }, { new: true });
     }
 
+    // Update descendantCount
+    const inc = 1;
+    await this.updateDescendantCountOfAncestors(updatedPage.parent, inc, true);
+
     return updatedPage;
   }
 
@@ -2342,7 +2338,7 @@ class PageService {
     let normalizablePages;
     let nonNormalizablePages;
     try {
-      [normalizablePages, nonNormalizablePages] = await this.crowi.pageGrantService.separateNormalizableAndNotNormalizablePages(pagesToNormalize);
+      [normalizablePages, nonNormalizablePages] = await this.crowi.pageGrantService.separateNormalizableAndNotNormalizablePages(user, pagesToNormalize);
     }
     catch (err) {
       throw err;
@@ -2387,7 +2383,13 @@ class PageService {
   }
 
   async normalizeParentRecursivelyMainOperation(page, user, pageOpId: ObjectIdLike): Promise<void> {
-    // TODO: insertOne PageOperationBlock
+    // Save prevDescendantCount for sub-operation
+    const Page = mongoose.model('Page') as unknown as PageModel;
+    const { PageQueryBuilder } = Page;
+    const builder = new PageQueryBuilder(Page.findOne(), true);
+    builder.addConditionAsMigrated();
+    const exPage = await builder.query.exec();
+    const options = { prevDescendantCount: exPage?.descendantCount ?? 0 };
 
     try {
       await this.normalizeParentRecursively([page.path], user);
@@ -2405,10 +2407,10 @@ class PageService {
       throw Error('PageOperation document not found');
     }
 
-    await this.normalizeParentRecursivelySubOperation(page, user, pageOp._id);
+    await this.normalizeParentRecursivelySubOperation(page, user, pageOp._id, options);
   }
 
-  async normalizeParentRecursivelySubOperation(page, user, pageOpId: ObjectIdLike): Promise<void> {
+  async normalizeParentRecursivelySubOperation(page, user, pageOpId: ObjectIdLike, options: {prevDescendantCount: number}): Promise<void> {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     try {
@@ -2422,9 +2424,9 @@ class PageService {
         throw Error('Page not found after updating descendantCount');
       }
 
-      const exDescendantCount = page.descendantCount;
+      const { prevDescendantCount } = options;
       const newDescendantCount = pageAfterUpdatingDescendantCount.descendantCount;
-      const inc = newDescendantCount - exDescendantCount;
+      const inc = (newDescendantCount - prevDescendantCount) + 1;
       await this.updateDescendantCountOfAncestors(page._id, inc, false);
     }
     catch (err) {
@@ -2543,8 +2545,12 @@ class PageService {
   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'));
+    const ancestorPaths = paths.flatMap(p => collectAncestorPaths(p, []));
+    // targets' descendants
+    const pathAndRegExpsToNormalize: (RegExp | string)[] = paths
+      .map(p => new RegExp(`^${escapeStringRegexp(addTrailingSlash(p))}`, 'i'));
+    // include targets' path
+    pathAndRegExpsToNormalize.push(...paths);
 
     // determine UserGroup condition
     let userGroups = null;
@@ -2555,11 +2561,13 @@ class PageService {
 
     const grantFiltersByUser: { $or: any[] } = Page.generateGrantCondition(user, userGroups);
 
-    return this._normalizeParentRecursively(regexps, ancestorPaths, grantFiltersByUser, user);
+    return this._normalizeParentRecursively(pathAndRegExpsToNormalize, ancestorPaths, grantFiltersByUser, user);
   }
 
   // TODO: use websocket to show progress
-  private async _normalizeParentRecursively(regexps: RegExp[], pathsToInclude: string[], grantFiltersByUser: { $or: any[] }, user): Promise<void> {
+  private async _normalizeParentRecursively(
+      pathOrRegExps: (RegExp | string)[], publicPathsToNormalize: string[], grantFiltersByUser: { $or: any[] }, user,
+  ): Promise<void> {
     const BATCH_SIZE = 100;
     const PAGES_LIMIT = 1000;
 
@@ -2567,7 +2575,7 @@ class PageService {
     const { PageQueryBuilder } = Page;
 
     // Build filter
-    const filter: any = {
+    const andFilter: any = {
       $and: [
         {
           parent: null,
@@ -2576,25 +2584,36 @@ class PageService {
         },
       ],
     };
-    let pathCondition: (RegExp | string)[] = [];
-    if (regexps.length > 0) {
-      pathCondition = [...regexps];
-    }
-    if (pathsToInclude.length > 0) {
-      pathCondition = [...pathCondition, ...pathsToInclude];
-    }
-    if (pathCondition.length > 0) {
-      filter.$and.push({
-        parent: null,
-        status: Page.STATUS_PUBLISHED,
-        path: { $in: pathCondition },
-      });
+    const orFilter: any = { $or: [] };
+    // specified pathOrRegExps
+    if (pathOrRegExps.length > 0) {
+      orFilter.$or.push(
+        {
+          path: { $in: pathOrRegExps },
+        },
+      );
+    }
+    // not specified but ancestors of specified pathOrRegExps
+    if (publicPathsToNormalize.length > 0) {
+      orFilter.$or.push(
+        {
+          path: { $in: publicPathsToNormalize },
+          grant: Page.GRANT_PUBLIC, // use only public pages to complete the tree
+        },
+      );
     }
 
+    // Merge filters
+    const mergedFilter = {
+      $and: [
+        { $and: [grantFiltersByUser, ...andFilter.$and] },
+        { $or: orFilter.$or },
+      ],
+    };
+
     let baseAggregation = Page
       .aggregate([
-        { $match: grantFiltersByUser },
-        { $match: filter },
+        { $match: mergedFilter },
         {
           $project: { // minimize data to fetch
             _id: 1,
@@ -2604,7 +2623,7 @@ class PageService {
       ]);
 
     // Limit pages to get
-    const total = await Page.countDocuments(filter);
+    const total = await Page.countDocuments(mergedFilter);
     if (total > PAGES_LIMIT) {
       baseAggregation = baseAggregation.limit(Math.floor(total * 0.3));
     }
@@ -2643,16 +2662,29 @@ class PageService {
         });
 
         await Page.bulkWrite(resetParentOperations);
-
         await Page.removeEmptyPages(pageIdsToNotDelete, emptyPagePathsToDelete);
 
         // 2. Create lacking parents as empty pages
-        await Page.createEmptyPagesByPaths(parentPaths, user, false);
+        const orFilters = [
+          { path: '/' },
+          { path: { $in: publicPathsToNormalize }, grant: Page.GRANT_PUBLIC, status: Page.STATUS_PUBLISHED },
+          { path: { $in: publicPathsToNormalize }, parent: { $ne: null }, status: Page.STATUS_PUBLISHED },
+          { path: { $nin: publicPathsToNormalize }, status: Page.STATUS_PUBLISHED },
+        ];
+        const filterForApplicableAncestors = { $or: orFilters };
+        await Page.createEmptyPagesByPaths(parentPaths, user, false, filterForApplicableAncestors);
 
         // 3. Find parents
-        const builder2 = new PageQueryBuilder(Page.find({}, { _id: 1, path: 1 }), true);
+        const addGrantCondition = (builder) => {
+          builder.query = builder.query.and(grantFiltersByUser);
+
+          return builder;
+        };
+        const builder2 = new PageQueryBuilder(Page.find(), true);
+        addGrantCondition(builder2);
         const parents = await builder2
           .addConditionToListByPathsArray(parentPaths)
+          .addConditionToFilterByApplicableAncestors(publicPathsToNormalize)
           .query
           .lean()
           .exec();
@@ -2668,6 +2700,7 @@ class PageService {
               {
                 path: { $regex: new RegExp(`^${parentPathEscaped}(\\/[^/]+)\\/?$`, 'i') }, // see: regexr.com/6889f (e.g. /parent/any_child or /any_level1)
               },
+              filterForApplicableAncestors,
               grantFiltersByUser,
             ],
           };
@@ -2717,9 +2750,8 @@ class PageService {
 
     await streamToPromise(migratePagesStream);
 
-    const existsFilter = { $and: [grantFiltersByUser, ...filter.$and] };
-    if (await Page.exists(existsFilter) && shouldContinue) {
-      return this._normalizeParentRecursively(regexps, pathsToInclude, grantFiltersByUser, user);
+    if (await Page.exists(mergedFilter) && shouldContinue) {
+      return this._normalizeParentRecursively(pathOrRegExps, publicPathsToNormalize, grantFiltersByUser, user);
     }
 
   }

+ 12 - 12
packages/app/test/integration/service/page-grant.test.js

@@ -220,7 +220,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -232,7 +232,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = groupParent._id;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -244,7 +244,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -256,7 +256,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = groupParent._id;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -268,7 +268,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -280,7 +280,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -292,7 +292,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });
@@ -304,7 +304,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = groupParent._id;
       const shouldCheckDescendants = false;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });
@@ -318,7 +318,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -330,7 +330,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -342,7 +342,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = groupParent._id;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(true);
     });
@@ -354,7 +354,7 @@ describe('PageGrantService', () => {
       const grantedGroupId = null;
       const shouldCheckDescendants = true;
 
-      const result = await pageGrantService.isGrantNormalized(targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      const result = await pageGrantService.isGrantNormalized(user1, targetPath, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
 
       expect(result).toBe(false);
     });

+ 351 - 12
packages/app/test/integration/service/v5.migration.test.js

@@ -1,3 +1,4 @@
+/* eslint-disable max-len */
 const mongoose = require('mongoose');
 
 const { getInstance } = require('../setup-crowi');
@@ -10,9 +11,12 @@ describe('V5 page migration', () => {
   let UserGroupRelation;
 
   let testUser1;
+  let rootUser;
 
   let rootPage;
 
+  const rootUserGroupId = new mongoose.Types.ObjectId();
+  const testUser1GroupId = new mongoose.Types.ObjectId();
   const groupIdIsolate = new mongoose.Types.ObjectId();
   const groupIdA = new mongoose.Types.ObjectId();
   const groupIdB = new mongoose.Types.ObjectId();
@@ -41,11 +45,23 @@ describe('V5 page migration', () => {
 
     await crowi.configManager.updateConfigsInTheSameNamespace('crowi', { 'app:isV5Compatible': true });
 
-    await User.insertMany([{ name: 'testUser1', username: 'testUser1', email: 'testUser1@example.com' }]);
+    await User.insertMany([
+      { name: 'rootUser', username: 'rootUser', email: 'rootUser@example.com' },
+      { name: 'testUser1', username: 'testUser1', email: 'testUser1@example.com' },
+    ]);
+    rootUser = await User.findOne({ username: 'rootUser' });
     testUser1 = await User.findOne({ username: 'testUser1' });
     rootPage = await Page.findOne({ path: '/' });
 
     await UserGroup.insertMany([
+      {
+        _id: rootUserGroupId,
+        name: 'rootUserGroup',
+      },
+      {
+        _id: testUser1GroupId,
+        name: 'testUser1Group',
+      },
       {
         _id: groupIdIsolate,
         name: 'groupIsolate',
@@ -67,6 +83,14 @@ describe('V5 page migration', () => {
     ]);
 
     await UserGroupRelation.insertMany([
+      {
+        relatedGroup: rootUserGroupId,
+        relatedUser: rootUser._id,
+      },
+      {
+        relatedGroup: testUser1GroupId,
+        relatedUser: testUser1._id,
+      },
       {
         relatedGroup: groupIdIsolate,
         relatedUser: testUser1._id,
@@ -232,11 +256,15 @@ describe('V5 page migration', () => {
     });
   };
 
-  describe('normalizeParentRecursivelyByPages()', () => {
+  const normalizeParentRecursivelyByPages = async(pages, user) => {
+    return crowi.pageService.normalizeParentRecursivelyByPages(pages, user);
+  };
 
-    const normalizeParentRecursivelyByPages = async(pages, user) => {
-      return crowi.pageService.normalizeParentRecursivelyByPages(pages, user);
-    };
+  const normalizeParentByPage = async(page, user) => {
+    return crowi.pageService.normalizeParentByPage(page, user);
+  };
+
+  describe('normalizeParentRecursivelyByPages()', () => {
 
     test('should migrate all pages specified by pageIds', async() => {
       jest.restoreAllMocks();
@@ -281,7 +309,7 @@ describe('V5 page migration', () => {
       expect(page10AM.parent).toStrictEqual(page7._id);
     });
 
-    test("should replace empty page with same path with new non-empty page and update all related children's parent", async() => {
+    test('should replace empty page with same path with new non-empty page and update all related children\'s parent', async() => {
       const page1 = await Page.findOne({ path: '/normalize_10', isEmpty: true, parent: { $ne: null } });
       const page2 = await Page.findOne({
         path: '/normalize_10/normalize_11_gA', _id: pageId8, isEmpty: true, parent: { $ne: null },
@@ -309,6 +337,320 @@ describe('V5 page migration', () => {
 
       expect(page3AM.isEmpty).toBe(false);
     });
+
+  });
+
+  describe('should normalize only selected pages recursively (especially should NOT normalize non-selected ancestors)', () => {
+    /*
+     * # Test flow
+     * - Existing pages
+     *   - All pages are NOT normalized
+     *   - A, B, C, and D are owned by "testUser1"
+     *   A. /normalize_A_owned
+     *   B. /normalize_A_owned/normalize_B_owned
+     *   C. /normalize_A_owned/normalize_B_owned/normalize_C_owned
+     *   D. /normalize_A_owned/normalize_B_owned/normalize_C_owned/normalize_D_owned
+     *   E. /normalize_A_owned/normalize_B_owned/normalize_C_owned/normalize_D_root
+     *     - Owned by "rootUser"
+     *   F. /normalize_A_owned/normalize_B_owned/normalize_C_owned/normalize_D_group
+     *     - Owned by the userGroup "groupIdIsolate"
+     *
+     * 1. Normalize A and B one by one.
+     *   - Expect
+     *     - A and B are normalized
+     *     - C and D are NOT normalized
+     *     - E and F are NOT normalized
+     * 2. Recursively normalize D.
+     *   - Expect
+     *     - A, B, and D are normalized
+     *     - C is NOT normalized
+     *       - C is substituted by an empty page whose path is "/normalize_A_owned/normalize_B_owned/normalize_C_owned"
+     *     - E and F are NOT normalized
+     * 3. Recursively normalize C.
+     *   - Expect
+     *     - A, B, C, and D are normalized
+     *     - An empty page at "/normalize_A_owned/normalize_B_owned/normalize_C_owned" does NOT exist (removed)
+     *     - E and F are NOT normalized
+     */
+
+    const owned = filter => ({ grantedUsers: [testUser1._id], ...filter });
+    const root = filter => ({ grantedUsers: [rootUser._id], ...filter });
+    const rootUserGroup = filter => ({ grantedGroup: rootUserGroupId, ...filter });
+    const testUser1Group = filter => ({ grantedGroup: testUser1GroupId, ...filter });
+
+    const normalized = { parent: { $ne: null } };
+    const notNormalized = { parent: null };
+    const empty = { isEmpty: true };
+
+    beforeAll(async() => {
+      // Prepare data
+      const id17 = new mongoose.Types.ObjectId();
+      const id21 = new mongoose.Types.ObjectId();
+      const id22 = new mongoose.Types.ObjectId();
+      const id23 = new mongoose.Types.ObjectId();
+
+      await Page.insertMany([
+        // 1
+        {
+          path: '/normalize_13_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_13_owned/normalize_14_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_root',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [rootUser._id],
+        },
+        {
+          path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_group',
+          grant: Page.GRANT_USER_GROUP,
+          grantedGroup: testUser1GroupId,
+        },
+
+        // 2
+        {
+          _id: id17,
+          path: '/normalize_17_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: rootPage._id,
+        },
+        {
+          path: '/normalize_17_owned/normalize_18_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: id17,
+        },
+        {
+          path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+        },
+        {
+          path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_root',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [rootUser._id],
+        },
+        {
+          path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_group',
+          grant: Page.GRANT_USER_GROUP,
+          grantedGroup: rootUserGroupId,
+        },
+
+        // 3
+        {
+          _id: id21,
+          path: '/normalize_21_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: rootPage._id,
+        },
+        {
+          _id: id22,
+          path: '/normalize_21_owned/normalize_22_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: id21,
+        },
+        {
+          path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: null,
+        },
+        {
+          _id: id23,
+          path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned',
+          isEmpty: true,
+          parent: id22,
+        },
+        {
+          path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_owned',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [testUser1._id],
+          parent: id23,
+        },
+        {
+          path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_root',
+          grant: Page.GRANT_OWNER,
+          grantedUsers: [rootUser._id],
+        },
+        {
+          path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_rootGroup',
+          grant: Page.GRANT_USER_GROUP,
+          grantedGroup: rootUserGroupId,
+        },
+      ]);
+    });
+
+
+    test('Should normalize pages one by one without including other pages', async() => {
+      const _owned13 = await Page.findOne(owned({ path: '/normalize_13_owned', ...notNormalized }));
+      const _owned14 = await Page.findOne(owned({ path: '/normalize_13_owned/normalize_14_owned', ...notNormalized }));
+      const _owned15 = await Page.findOne(owned({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned', ...notNormalized }));
+      const _owned16 = await Page.findOne(owned({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_owned', ...notNormalized }));
+      const _root16 = await Page.findOne(root({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_root', ...notNormalized }));
+      const _group16 = await Page.findOne(testUser1Group({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_group', ...notNormalized }));
+
+      expect(_owned13).not.toBeNull();
+      expect(_owned14).not.toBeNull();
+      expect(_owned15).not.toBeNull();
+      expect(_owned16).not.toBeNull();
+      expect(_root16).not.toBeNull();
+      expect(_group16).not.toBeNull();
+
+      // Normalize
+      await normalizeParentByPage(_owned14, testUser1);
+
+      const owned13 = await Page.findOne({ path: '/normalize_13_owned' });
+      const empty13 = await Page.findOne({ path: '/normalize_13_owned', ...empty });
+      const owned14 = await Page.findOne({ path: '/normalize_13_owned/normalize_14_owned' });
+      const owned15 = await Page.findOne({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned' });
+      const owned16 = await Page.findOne({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_owned' });
+      const root16 = await Page.findOne(root({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_root' }));
+      const group16 = await Page.findOne(testUser1Group({ path: '/normalize_13_owned/normalize_14_owned/normalize_15_owned/normalize_16_group' }));
+
+      expect(owned13).not.toBeNull();
+      expect(empty13).not.toBeNull();
+      expect(owned14).not.toBeNull();
+      expect(owned15).not.toBeNull();
+      expect(owned16).not.toBeNull();
+      expect(root16).not.toBeNull();
+      expect(group16).not.toBeNull();
+
+      // Check parent
+      expect(owned13.parent).toBeNull();
+      expect(empty13.parent).toStrictEqual(rootPage._id);
+      expect(owned14.parent).toStrictEqual(empty13._id);
+      expect(owned15.parent).toBeNull();
+      expect(owned16.parent).toBeNull();
+      expect(root16.parent).toBeNull();
+      expect(group16.parent).toBeNull();
+
+      // Check descendantCount
+      expect(owned13.descendantCount).toBe(0);
+      expect(empty13.descendantCount).toBe(1);
+      expect(owned14.descendantCount).toBe(0);
+    });
+
+    test('Should normalize pages recursively excluding the pages not selected', async() => {
+      const _owned17 = await Page.findOne(owned({ path: '/normalize_17_owned', ...normalized }));
+      const _owned18 = await Page.findOne(owned({ path: '/normalize_17_owned/normalize_18_owned', ...normalized }));
+      const _owned19 = await Page.findOne(owned({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned', ...notNormalized }));
+      const _owned20 = await Page.findOne(owned({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_owned', ...notNormalized }));
+      const _root20 = await Page.findOne(root({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_root', ...notNormalized }));
+      const _group20 = await Page.findOne(rootUserGroup({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_group', ...notNormalized }));
+
+      expect(_owned17).not.toBeNull();
+      expect(_owned18).not.toBeNull();
+      expect(_owned19).not.toBeNull();
+      expect(_owned20).not.toBeNull();
+      expect(_root20).not.toBeNull();
+      expect(_group20).not.toBeNull();
+
+      // Normalize
+      await normalizeParentRecursivelyByPages([_owned20], testUser1);
+
+      const owned17 = await Page.findOne({ path: '/normalize_17_owned' });
+      const owned18 = await Page.findOne({ path: '/normalize_17_owned/normalize_18_owned' });
+      const owned19 = await Page.findOne({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned' });
+      const empty19 = await Page.findOne({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned', ...empty });
+      const owned20 = await Page.findOne({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_owned' });
+      const root20 = await Page.findOne(root({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_root' }));
+      const group20 = await Page.findOne(rootUserGroup({ path: '/normalize_17_owned/normalize_18_owned/normalize_19_owned/normalize_20_group' }));
+
+      expect(owned17).not.toBeNull();
+      expect(owned18).not.toBeNull();
+      expect(owned19).not.toBeNull();
+      expect(empty19).not.toBeNull();
+      expect(owned20).not.toBeNull();
+      expect(root20).not.toBeNull();
+      expect(group20).not.toBeNull();
+
+      // Check parent
+      expect(owned17.parent).toStrictEqual(rootPage._id);
+      expect(owned18.parent).toStrictEqual(owned17._id);
+      expect(owned19.parent).toBeNull();
+      expect(empty19.parent).toStrictEqual(owned18._id);
+      expect(owned20.parent).toStrictEqual(empty19._id);
+      expect(root20.parent).toBeNull();
+      expect(group20.parent).toBeNull();
+
+      // Check isEmpty
+      expect(owned17.isEmpty).toBe(false);
+      expect(owned18.isEmpty).toBe(false);
+    });
+
+    test('Should normalize pages recursively excluding the pages of not user\'s & Should delete unnecessary empty pages', async() => {
+      const _owned21 = await Page.findOne(owned({ path: '/normalize_21_owned', ...normalized }));
+      const _owned22 = await Page.findOne(owned({ path: '/normalize_21_owned/normalize_22_owned', ...normalized }));
+      const _owned23 = await Page.findOne(owned({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned', ...notNormalized }));
+      const _empty23 = await Page.findOne({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned', ...normalized, ...empty });
+      const _owned24 = await Page.findOne(owned({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_owned', ...normalized }));
+      const _root24 = await Page.findOne(root({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_root', ...notNormalized }));
+      const _rootGroup24 = await Page.findOne(rootUserGroup({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_rootGroup', ...notNormalized }));
+
+      expect(_owned21).not.toBeNull();
+      expect(_owned22).not.toBeNull();
+      expect(_owned23).not.toBeNull();
+      expect(_empty23).not.toBeNull();
+      expect(_owned24).not.toBeNull();
+      expect(_root24).not.toBeNull();
+      expect(_rootGroup24).not.toBeNull();
+
+      // Normalize
+      await normalizeParentRecursivelyByPages([_owned23], testUser1);
+
+      const owned21 = await Page.findOne({ path: '/normalize_21_owned' });
+      const owned22 = await Page.findOne({ path: '/normalize_21_owned/normalize_22_owned' });
+      const owned23 = await Page.findOne({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned' });
+      const empty23 = await Page.findOne({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned', ...empty });
+      const owned24 = await Page.findOne({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_owned' });
+      const root24 = await Page.findOne(root({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_root' }));
+      const rootGroup24 = await Page.findOne(rootUserGroup({ path: '/normalize_21_owned/normalize_22_owned/normalize_23_owned/normalize_24_rootGroup' }));
+
+      expect(owned21).not.toBeNull();
+      expect(owned22).not.toBeNull();
+      expect(owned23).not.toBeNull();
+      expect(empty23).toBeNull(); // removed
+      expect(owned24).not.toBeNull();
+      expect(root24).not.toBeNull();
+      expect(rootGroup24).not.toBeNull();
+
+      // Check parent
+      expect(owned21.parent).toStrictEqual(rootPage._id);
+      expect(owned22.parent).toStrictEqual(owned21._id);
+      expect(owned23.parent).toStrictEqual(owned22._id);
+      expect(owned24.parent).toStrictEqual(owned23._id); // not empty23._id
+      expect(root24.parent).toBeNull();
+      expect(rootGroup24.parent).toBeNull(); // excluded from the pages to be normalized
+
+      // Check isEmpty
+      expect(owned21.isEmpty).toBe(false);
+      expect(owned22.isEmpty).toBe(false);
+      expect(owned23.isEmpty).toBe(false);
+    });
+
   });
 
   describe('normalizeAllPublicPages()', () => {
@@ -407,17 +749,14 @@ describe('V5 page migration', () => {
     });
   });
 
-  describe('normalizeParentByPageId()', () => {
-    const normalizeParentByPageId = async(page, user) => {
-      return crowi.pageService.normalizeParentByPageId(page, user);
-    };
+  describe('normalizeParentByPage()', () => {
     test('it should normalize not v5 page with usergroup that has parent group', async() => {
       const page1 = await Page.findOne({ _id: pageId1, path: '/normalize_1', isEmpty: true });
       const page2 = await Page.findOne({ _id: pageId2, path: '/normalize_1/normalize_2', parent: page1._id });
       const page3 = await Page.findOne({ _id: pageId3, path: '/normalize_1' }); // NOT v5
       expectAllToBeTruthy([page1, page2, page3]);
 
-      await normalizeParentByPageId(page3, testUser1);
+      await normalizeParentByPage(page3, testUser1);
 
       // AM => After Migration
       const page1AM = await Page.findOne({ _id: pageId1, path: '/normalize_1', isEmpty: true });
@@ -439,7 +778,7 @@ describe('V5 page migration', () => {
 
       let isThrown;
       try {
-        await normalizeParentByPageId(page6, testUser1);
+        await normalizeParentByPage(page6, testUser1);
       }
       catch (err) {
         isThrown = true;