Browse Source

Merge pull request #6120 from weseek/feat/recount-descendant-count-after-fixing-path

feat: Recount descendantCount of pages after fixing path by resumeRenameOperation
Yohei Shiina 3 years ago
parent
commit
d18bc016e7

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

@@ -571,7 +571,8 @@ module.exports = (crowi) => {
     }
 
     try {
-      await crowi.pageService.resumeRenameSubOperation(page);
+      const pageOp = await crowi.pageOperationService.getRenameSubOperationByPageId(page._id);
+      await crowi.pageService.resumeRenameSubOperation(page, pageOp);
     }
     catch (err) {
       logger.error(err);

+ 25 - 11
packages/app/src/server/service/page-operation.ts

@@ -8,7 +8,9 @@ import { ObjectIdLike } from '../interfaces/mongoose-utils';
 
 const logger = loggerFactory('growi:services:page-operation');
 
-const { isEitherOfPathAreaOverlap, isPathAreaOverlap, isTrashPage } = pagePathUtils;
+const {
+  isEitherOfPathAreaOverlap, isPathAreaOverlap, isTrashPage, collectAncestorPaths,
+} = pagePathUtils;
 const AUTO_UPDATE_INTERVAL_SEC = 5;
 
 const {
@@ -34,8 +36,10 @@ class PageOperationService {
    */
   async afterExpressServerReady(): Promise<void> {
     try {
+      const pageOps = await PageOperation.find({ actionType: PageActionType.Rename, actionStage: PageActionStage.Sub })
+        .sort({ createdAt: 'asc' });
       // execute rename operation
-      await this.executeAllRenameOperationBySystem();
+      await this.executeAllRenameOperationBySystem(pageOps);
     }
     catch (err) {
       logger.error(err);
@@ -45,17 +49,12 @@ class PageOperationService {
   /**
    * Execute renameSubOperation on every page operation for rename ordered by createdAt ASC
    */
-  private async executeAllRenameOperationBySystem(): Promise<void> {
-    const Page = this.crowi.model('Page');
-
-    const pageOps = await PageOperation.find({ actionType: PageActionType.Rename, actionStage: PageActionStage.Sub })
-      .sort({ createdAt: 'asc' });
+  private async executeAllRenameOperationBySystem(pageOps: PageOperationDocument[]): Promise<void> {
     if (pageOps.length === 0) return;
 
+    const Page = this.crowi.model('Page');
+
     for await (const pageOp of pageOps) {
-      const {
-        page, toPath, options, user,
-      } = pageOp;
 
       const renamedPage = await Page.findById(pageOp.page._id);
       if (renamedPage == null) {
@@ -64,7 +63,7 @@ class PageOperationService {
       }
 
       // rename
-      await this.crowi.pageService.renameSubOperation(page, toPath, user, options, renamedPage, pageOp._id);
+      await this.crowi.pageService.resumeRenameSubOperation(renamedPage, pageOp);
     }
   }
 
@@ -169,6 +168,21 @@ class PageOperationService {
     clearInterval(timerObj);
   }
 
+  /**
+   * Get ancestor's paths using fromPath and toPath. Merge same paths if any.
+   */
+  getAncestorsPathsByFromAndToPath(fromPath: string, toPath: string): string[] {
+    const fromAncestorsPaths = collectAncestorPaths(fromPath);
+    const toAncestorsPaths = collectAncestorPaths(toPath);
+    // merge duplicate paths and return paths of ancestors
+    return Array.from(new Set(toAncestorsPaths.concat(fromAncestorsPaths)));
+  }
+
+  async getRenameSubOperationByPageId(pageId: ObjectIdLike): Promise<PageOperationDocument | null> {
+    const filter = { actionType: PageActionType.Rename, actionStage: PageActionStage.Sub, 'page._id': pageId };
+    return PageOperation.findOne(filter);
+  }
+
 }
 
 export default PageOperationService;

+ 41 - 15
packages/app/src/server/service/page.ts

@@ -29,7 +29,7 @@ import { prepareDeleteConfigValuesForCalc } from '~/utils/page-delete-config';
 
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
 import { PathAlreadyExistsError } from '../models/errors';
-import PageOperation, { PageActionStage, PageActionType } from '../models/page-operation';
+import PageOperation, { PageActionStage, PageActionType, PageOperationDocument } from '../models/page-operation';
 import { PageRedirectModel } from '../models/page-redirect';
 import { serializePageSecurely } from '../models/serializers/page-serializer';
 import Subscription from '../models/subscription';
@@ -618,11 +618,7 @@ class PageService {
     await PageOperation.findByIdAndDelete(pageOpId);
   }
 
-  async resumeRenameSubOperation(renamedPage: PageDocument): Promise<void> {
-
-    // findOne PageOperation
-    const filter = { actionType: PageActionType.Rename, actionStage: PageActionStage.Sub, 'page._id': renamedPage._id };
-    const pageOp = await PageOperation.findOne(filter);
+  async resumeRenameSubOperation(renamedPage: PageDocument, pageOp: PageOperationDocument): Promise<void> {
     if (pageOp == null) {
       throw Error('There is nothing to be processed right now');
     }
@@ -630,18 +626,26 @@ class PageService {
     if (!isProcessable) {
       throw Error('This page operation is currently being processed');
     }
+    if (pageOp.toPath == null) {
+      throw Error(`Property toPath is missing which is needed to resume rename operation(${pageOp._id})`);
+    }
 
     const {
-      page, toPath, options, user,
+      page, fromPath, toPath, options, user,
     } = pageOp;
 
-    // check property
-    if (toPath == null) {
-      throw Error(`Property toPath is missing which is needed to resume page operation(${pageOp._id})`);
-    }
-
-    this.renameSubOperation(page, toPath, user, options, renamedPage, pageOp._id);
+    this.fixPathsAndDescendantCountOfAncestors(page, user, options, renamedPage, pageOp._id, fromPath, toPath);
+  }
 
+  /**
+   * Renaming paths and fixing descendantCount of ancestors. It shoud be run synchronously.
+   * `renameSubOperation` to restart rename operation
+   * `updateDescendantCountOfPagesWithPaths` to fix descendantCount of ancestors
+   */
+  private async fixPathsAndDescendantCountOfAncestors(page, user, options, renamedPage, pageOpId, fromPath, toPath): Promise<void> {
+    await this.renameSubOperation(page, toPath, user, options, renamedPage, pageOpId);
+    const ancestorsPaths = this.crowi.pageOperationService.getAncestorsPathsByFromAndToPath(fromPath, toPath);
+    await this.updateDescendantCountOfPagesWithPaths(ancestorsPaths);
   }
 
   private isRenamingToUnderTarget(fromPath: string, toPath: string): boolean {
@@ -3066,8 +3070,30 @@ class PageService {
     builder.addConditionToSortPagesByDescPath();
 
     const aggregatedPages = await builder.query.lean().cursor({ batchSize: BATCH_SIZE });
+    await this.recountAndUpdateDescendantCountOfPages(aggregatedPages, BATCH_SIZE);
+  }
 
+  /**
+   * update descendantCount of the pages sequentially from longer path to shorter path
+   */
+  async updateDescendantCountOfPagesWithPaths(paths: string[]): Promise<void> {
+    const BATCH_SIZE = 200;
+    const Page = this.crowi.model('Page');
+    const { PageQueryBuilder } = Page;
+
+    const builder = new PageQueryBuilder(Page.find(), true);
+    builder.addConditionToListByPathsArray(paths); // find by paths
+    builder.addConditionToSortPagesByDescPath(); // sort in DESC
+
+    const aggregatedPages = await builder.query.lean().cursor({ batchSize: BATCH_SIZE });
+    await this.recountAndUpdateDescendantCountOfPages(aggregatedPages, BATCH_SIZE);
+  }
 
+  /**
+   * Recount descendantCount of pages one by one
+   */
+  async recountAndUpdateDescendantCountOfPages(pageCursor: QueryCursor<any>, batchSize:number): Promise<void> {
+    const Page = this.crowi.model('Page');
     const recountWriteStream = new Writable({
       objectMode: true,
       async write(pageDocuments, encoding, callback) {
@@ -3081,8 +3107,8 @@ class PageService {
         callback();
       },
     });
-    aggregatedPages
-      .pipe(createBatchStream(BATCH_SIZE))
+    pageCursor
+      .pipe(createBatchStream(batchSize))
       .pipe(recountWriteStream);
 
     await streamToPromise(recountWriteStream);