Merge pull request #3310 from weseek/imprv/4685-4901-refactor-revert

Imprv/4685 4901 refactor revert

src/server/models/page.js

@@ -1134,10 +1134,11 @@ module.exports = function(crowi) {
       throw new Error('This method does NOT supports deleting trashed pages.');
     }
 
-    const socketClientId = options.socketClientId || null;
-    if (this.isDeletableName(targetPage.path)) {
-      targetPage.status = STATUS_DELETED;
+    if (!this.isDeletableName(targetPage.path)) {
+      throw new Error('Page is not deletable');
     }
+    const socketClientId = options.socketClientId || null;
+    targetPage.status = STATUS_DELETED;
     return await this.renameRecursively(targetPage, newPath, user, { socketClientId, createRedirectPage: true });
   };
 
@@ -1201,7 +1202,7 @@ module.exports = function(crowi) {
       await Page.create(path, body, user, { redirectTo: newPagePath });
     }
 
-    pageEvent.emit('delete', [pageData], user, socketClientId);
+    pageEvent.emit('delete', pageData, user, socketClientId);
     pageEvent.emit('create', updatedPageData, user, socketClientId);
 
     return updatedPageData;

src/server/routes/page.js

@@ -1248,13 +1248,7 @@ module.exports = function(crowi, app) {
       if (page == null) {
         throw new Error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden');
       }
-
-      if (isRecursively) {
-        page = await crowi.pageService.revertDeletedPageRecursively(page, req.user, { socketClientId });
-      }
-      else {
-        page = await crowi.pageService.revertSingleDeletedPage(page, req.user, { socketClientId });
-      }
+      page = await crowi.pageService.revertDeletedPage(page, req.user, { socketClientId }, isRecursively);
     }
     catch (err) {
       logger.error('Error occured while get setting', err);

src/server/service/page.js

@@ -297,48 +297,53 @@ class PageService {
       .pipe(writeStream);
   }
 
-  async revertDeletedPageRecursively(targetPage, user, options = {}) {
+  async revertDeletedPages(pages, user) {
     const Page = this.crowi.model('Page');
-    const findOpts = { includeTrashed: true };
-    const pages = await Page.findManageableListWithDescendants(targetPage, user, findOpts);
-
-    let updatedPage = null;
-    await Promise.all(pages.map((page) => {
-      const isParent = (page.path === targetPage.path);
-      const p = this.revertDeletedPages(page, user, options);
-      if (isParent) {
-        updatedPage = p;
-      }
-      return p;
-    }));
+    const pageCollection = mongoose.connection.collection('pages');
+    const revisionCollection = mongoose.connection.collection('revisions');
+
+    const removePageBulkOp = pageCollection.initializeUnorderedBulkOp();
+    const revertPageBulkOp = pageCollection.initializeUnorderedBulkOp();
+    const revertRevisionBulkOp = revisionCollection.initializeUnorderedBulkOp();
+
+    // e.g. key: '/test'
+    const pathToPageMapping = {};
+    const toPaths = pages.map(page => Page.getRevertDeletedPageName(page.path));
+    const toPages = await Page.find({ path: { $in: toPaths } });
+    toPages.forEach((toPage) => {
+      pathToPageMapping[toPage.path] = toPage;
+    });
 
-    return updatedPage;
-  }
+    pages.forEach((page) => {
 
-  // revert pages recursively
-  async revertDeletedPages(page, user, options = {}) {
-    const Page = this.crowi.model('Page');
-    const newPath = Page.getRevertDeletedPageName(page.path);
-    const originPage = await Page.findByPath(newPath);
-    if (originPage != null) {
-    // When the page is deleted, it will always be created with "redirectTo" in the path of the original page.
-    // So, it's ok to delete the page
-    // However, If a page exists that is not "redirectTo", something is wrong. (Data correction is needed).
-      if (originPage.redirectTo !== page.path) {
-        throw new Error('The new page of to revert is exists and the redirect path of the page is not the deleted page.');
+      // e.g. page.path = /trash/test, toPath = /test
+      const toPath = Page.getRevertDeletedPageName(page.path);
+
+      if (pathToPageMapping[toPath] != null) {
+      // When the page is deleted, it will always be created with "redirectTo" in the path of the original page.
+      // So, it's ok to delete the page
+      // However, If a page exists that is not "redirectTo", something is wrong. (Data correction is needed).
+        if (pathToPageMapping[toPath].redirectTo === page.path) {
+          removePageBulkOp.find({ path: toPath }).remove();
+        }
       }
-      // originPage is object.
-      await this.deleteMultiplePagesCompletely([originPage], options);
-    }
+      revertPageBulkOp.find({ _id: page._id }).update({ $set: { path: toPath, status: STATUS_PUBLISHED, lastUpdateUser: user._id } });
+      revertRevisionBulkOp.find({ path: page.path }).update({ $set: { path: toPath } }, { multi: true });
+    });
 
-    page.status = STATUS_PUBLISHED;
-    page.lastUpdateUser = user;
-    debug('Revert deleted the page', page, newPath);
-    const updatedPage = await Page.rename(page, newPath, user, {});
-    return updatedPage;
+    try {
+      await removePageBulkOp.execute();
+      await revertPageBulkOp.execute();
+      await revertRevisionBulkOp.execute();
+    }
+    catch (err) {
+      if (err.code !== 11000) {
+        throw new Error('Failed to revert pages: ', err);
+      }
+    }
   }
 
-  async revertSingleDeletedPage(page, user, options = {}) {
+  async revertDeletedPage(page, user, options = {}, isRecursively = false) {
     const Page = this.crowi.model('Page');
     const newPath = Page.getRevertDeletedPageName(page.path);
     const originPage = await Page.findByPath(newPath);
@@ -352,6 +357,10 @@ class PageService {
       await this.deleteCompletely(originPage, options);
     }
 
+    if (isRecursively) {
+      this.revertDeletedDescendantsWithStream(page, user, options);
+    }
+
     page.status = STATUS_PUBLISHED;
     page.lastUpdateUser = user;
     debug('Revert deleted the page', page, newPath);
@@ -359,6 +368,50 @@ class PageService {
     return updatedPage;
   }
 
+  /**
+   * Create revert stream
+   */
+  async revertDeletedDescendantsWithStream(targetPage, user, options = {}) {
+    const Page = this.crowi.model('Page');
+    const { PageQueryBuilder } = Page;
+
+    const readStream = new PageQueryBuilder(Page.find())
+      .addConditionToExcludeRedirect()
+      .addConditionToListOnlyDescendants(targetPage.path)
+      .addConditionToFilteringByViewer(user)
+      .query
+      .lean()
+      .cursor();
+
+    const revertDeletedPages = this.revertDeletedPages.bind(this);
+    let count = 0;
+    const writeStream = new Writable({
+      objectMode: true,
+      async write(batch, encoding, callback) {
+        try {
+          count += batch.length;
+          revertDeletedPages(batch, user);
+          logger.debug(`Reverting pages progressing: (count=${count})`);
+        }
+        catch (err) {
+          logger.error('revertPages error on add anyway: ', err);
+        }
+
+        callback();
+      },
+      final(callback) {
+        logger.debug(`Reverting pages has completed: (totalCount=${count})`);
+
+        callback();
+      },
+    });
+
+    readStream
+      .pipe(createBatchStream(BULK_REINDEX_SIZE))
+      .pipe(writeStream);
+  }
+
+
   async handlePrivatePagesForDeletedGroup(deletedGroup, action, transferToUserGroupId) {
     const Page = this.crowi.model('Page');
     const pages = await Page.find({ grantedGroup: deletedGroup });