Merge branch 'imprv/reduce-the-load-of-the-delete-process' into imprv/refactor-revertDelete

src/client/js/components/EmptyTrashModal.jsx

@@ -8,12 +8,13 @@ import {
 import { withTranslation } from 'react-i18next';
 import { withUnstatedContainers } from './UnstatedUtils';
 
+import SocketIoContainer from '../services/SocketIoContainer';
 import AppContainer from '../services/AppContainer';
 import ApiErrorMessageList from './PageManagement/ApiErrorMessageList';
 
 const EmptyTrashModal = (props) => {
   const {
-    t, isOpen, onClose, appContainer,
+    t, isOpen, onClose, appContainer, socketIoContainer,
   } = props;
 
   const [errs, setErrs] = useState(null);
@@ -22,7 +23,7 @@ const EmptyTrashModal = (props) => {
     setErrs(null);
 
     try {
-      await appContainer.apiv3Delete('/pages/empty-trash');
+      await appContainer.apiv3Delete('/pages/empty-trash', { socketclientId: socketIoContainer.getSocketClientId() });
       window.location.reload();
     }
     catch (err) {
@@ -55,12 +56,13 @@ const EmptyTrashModal = (props) => {
 /**
  * Wrapper component for using unstated
  */
-const EmptyTrashModalWrapper = withUnstatedContainers(EmptyTrashModal, [AppContainer]);
+const EmptyTrashModalWrapper = withUnstatedContainers(EmptyTrashModal, [AppContainer, SocketIoContainer]);
 
 
 EmptyTrashModal.propTypes = {
   t: PropTypes.func.isRequired, //  i18next
   appContainer: PropTypes.instanceOf(AppContainer).isRequired,
+  socketIoContainer: PropTypes.instanceOf(SocketIoContainer),
 
   isOpen: PropTypes.bool.isRequired,
   onClose: PropTypes.func.isRequired,

src/server/models/page.js

@@ -1141,6 +1141,22 @@ module.exports = function(crowi) {
     }));
   };
 
+  pageSchema.statics.revertDeletedPageRecursively = async function(targetPage, user, options = {}) {
+    const findOpts = { includeTrashed: true };
+    const pages = await this.findManageableListWithDescendants(targetPage, user, findOpts);
+
+    let updatedPage = null;
+    await Promise.all(pages.map((page) => {
+      const isParent = (page.path === targetPage.path);
+      const p = crowi.pageService.revertDeletedPages(page, user, options);
+      if (isParent) {
+        updatedPage = p;
+      }
+      return p;
+    }));
+
+    return updatedPage;
+  };
 
   pageSchema.statics.removeByPath = function(path) {
     if (path == null) {
@@ -1201,8 +1217,7 @@ module.exports = function(crowi) {
       await Page.create(path, body, user, { redirectTo: newPagePath });
     }
 
-    // The reason using array is to match the types in elasticsearch.js ( deletePages )
-    pageEvent.emit('delete', [pageData], user, socketClientId);
+    pageEvent.emit('delete', pageData, user, socketClientId);
     pageEvent.emit('create', updatedPageData, user, socketClientId);
 
     return updatedPageData;

src/server/routes/apiv3/pages.js

@@ -436,8 +436,11 @@ module.exports = (crowi) => {
    *            description: Succeeded to remove all trash pages
    */
   router.delete('/empty-trash', loginRequired, adminRequired, csrf, async(req, res) => {
+    const socketClientId = req.query.socketclientId;
+    const options = { socketClientId };
+
     try {
-      const pages = await crowi.pageService.completelyDeletePageRecursively({ path: '/trash' }, req.user);
+      const pages = await crowi.pageService.completelyDeletePageRecursively({ path: '/trash' }, req.user, options);
       return res.apiv3({ pages });
     }
     catch (err) {

src/server/routes/page.js

@@ -1197,7 +1197,7 @@ module.exports = function(crowi, app) {
           await crowi.pageService.completelyDeletePageRecursively(page, req.user, options);
         }
         else {
-          await crowi.pageService.completelyDeletePage(page, req.user, options);
+          await crowi.pageService.completelyDeleteSinglePage(page, req.user, options);
         }
       }
       else {
@@ -1258,7 +1258,7 @@ module.exports = function(crowi, app) {
         page = await Page.revertDeletedPageRecursively(page, req.user, { socketClientId });
       }
       else {
-        page = await crowi.pageService.revertDeletedPage(page, req.user, { socketClientId });
+        page = await crowi.pageService.revertSingleDeletedPage(page, req.user, { socketClientId });
       }
     }
     catch (err) {

src/server/service/attachment.js

@@ -46,8 +46,10 @@ class AttachmentService {
   async removeAttachment(attachments) {
     const { fileUploadService } = this.crowi;
 
-    await attachments.forEach(attachment => fileUploadService.deleteFile(attachment));
-    await attachments.forEach(attachment => attachment.remove());
+    return attachments.forEach((attachment) => {
+      fileUploadService.deleteFile(attachment);
+      attachment.remove();
+    });
   }
 
 }

src/server/service/page.js

@@ -21,7 +21,7 @@ class PageService {
     const ShareLink = this.crowi.model('ShareLink');
     const Revision = this.crowi.model('Revision');
 
-    await Promise.all([
+    return Promise.all([
       Bookmark.find({ page: { $in: pageIds } }).remove({}),
       Comment.find({ page: { $in: pageIds } }).remove({}),
       PageTagRelation.find({ relatedPage: { $in: pageIds } }).remove({}),
@@ -38,7 +38,7 @@ class PageService {
     const { attachmentService } = this.crowi;
     const attachments = await Attachment.find({ page: { $in: pageIds } });
 
-    attachmentService.removeAttachment(attachments);
+    return attachmentService.removeAttachment(attachments);
   }
 
   async duplicate(page, newPagePath, user) {
@@ -93,8 +93,8 @@ class PageService {
     return newParentpage;
   }
 
-
-  async completelyDeletePage(pagesData, user, options = {}) {
+  // delete multiple pages
+  async completelyDeletePages(pagesData, user, options = {}) {
     this.validateCrowi();
     let pageEvent;
     // init event
@@ -104,25 +104,33 @@ class PageService {
       pageEvent.on('update', pageEvent.onUpdate);
     }
 
-    // In case of recursively
-    if (pagesData.length != null) {
-      const ids = pagesData.map(page => (page._id));
-      const paths = pagesData.map(page => (page.path));
-      const socketClientId = options.socketClientId || null;
+    const ids = pagesData.map(page => (page._id));
+    const paths = pagesData.map(page => (page.path));
+    const socketClientId = options.socketClientId || null;
 
-      logger.debug('Deleting completely', paths);
+    logger.debug('Deleting completely', paths);
 
-      await this.deleteCompletely(ids, paths);
+    await this.deleteCompletely(ids, paths);
 
-      if (socketClientId != null) {
-        pageEvent.emit('delete', pagesData, user, socketClientId); // update as renamed page
-      }
-      return;
+    if (socketClientId != null) {
+      pageEvent.emit('deleteCompletely', pagesData, user, socketClientId); // update as renamed page
+    }
+    return;
+  }
+
+  // delete single page completely
+  async completelyDeleteSinglePage(pageData, user, options = {}) {
+    this.validateCrowi();
+    let pageEvent;
+    // init event
+    if (this.crowi != null) {
+      pageEvent = this.crowi.event('page');
+      pageEvent.on('create', pageEvent.onCreate);
+      pageEvent.on('update', pageEvent.onUpdate);
     }
 
-    //  Simply delete completely a page
-    const ids = [pagesData].map(page => (page._id));
-    const paths = [pagesData].map(page => (page.path));
+    const ids = [pageData._id];
+    const paths = [pageData.path];
     const socketClientId = options.socketClientId || null;
 
     logger.debug('Deleting completely', paths);
@@ -130,8 +138,9 @@ class PageService {
     await this.deleteCompletely(ids, paths);
 
     if (socketClientId != null) {
-      pageEvent.emit('delete', [pagesData], user, socketClientId); // update as renamed page
+      pageEvent.emit('delete', pageData, user, socketClientId); // update as renamed page
     }
+    return;
   }
 
   /**
@@ -145,28 +154,33 @@ class PageService {
     const pages = await Page.findManageableListWithDescendants(targetPage, user, findOpts);
 
     // TODO streaming bellow action
-    await this.completelyDeletePage(pages, user, options);
+    return this.completelyDeletePages(pages, user, options);
   }
 
-  async revertDeletedPageRecursively(targetPage, user, options = {}) {
-    const findOpts = { includeTrashed: true };
+  // revert pages recursively
+  async revertDeletedPages(page, user, options = {}) {
     const Page = this.crowi.model('Page');
-    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.revertDeletedPage(page, user, options);
-      if (isParent) {
-        updatedPage = p;
+    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.');
       }
-      return p;
-    }));
+      // originPage is object.
+      await this.completelyDeletePages([originPage], options);
+    }
 
+    page.status = STATUS_PUBLISHED;
+    page.lastUpdateUser = user;
+    debug('Revert deleted the page', page, newPath);
+    const updatedPage = await Page.rename(page, newPath, user, {});
     return updatedPage;
   }
 
-  async revertDeletedPage(page, user, options = {}) {
+  async revertSingleDeletedPage(page, user, options = {}) {
     const Page = this.crowi.model('Page');
     const newPath = Page.getRevertDeletedPageName(page.path);
     const originPage = await Page.findByPath(newPath);
@@ -177,7 +191,7 @@ class PageService {
       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.');
       }
-      await this.completelyDeletePage(originPage, options);
+      await this.completelyDeleteSinglePage(originPage, options);
     }
 
     page.status = STATUS_PUBLISHED;
@@ -198,10 +212,7 @@ class PageService {
         }));
         break;
       case 'delete':
-        await Promise.all(pages.map((page) => {
-          return this.completelyDeletePage(page);
-        }));
-        break;
+        return this.completelyDeletePages(pages);
       case 'transfer':
         await Promise.all(pages.map((page) => {
           return Page.transferPageToGroup(page, transferToUserGroupId);

src/server/service/search-delegator/elasticsearch.js

@@ -503,7 +503,6 @@ class ElasticsearchDelegator {
   deletePages(pages) {
     const self = this;
     const body = [];
-
     pages.forEach(page => self.prepareBodyForDelete(body, page));
 
     logger.debug('deletePages(): Sending Request to ES', body);
@@ -960,7 +959,7 @@ class ElasticsearchDelegator {
     return this.updateOrInsertPageById(page._id);
   }
 
-  async syncPageDeleted(pages, user) {
+  async syncPagesDeletedCompletely(pages, user) {
     for (let i = 0; i < pages.length; i++) {
       logger.debug('SearchClient.syncPageDeleted', pages[i].path);
     }
@@ -973,6 +972,17 @@ class ElasticsearchDelegator {
     }
   }
 
+  async syncPageDeleted(page, user) {
+    logger.debug('SearchClient.syncPageDeleted', page.path);
+
+    try {
+      return await this.deletePages([page]);
+    }
+    catch (err) {
+      logger.error('deletePages:ES Error', err);
+    }
+  }
+
   async syncBookmarkChanged(pageId) {
     logger.debug('SearchClient.syncBookmarkChanged', pageId);
 

src/server/service/search.js

@@ -58,6 +58,7 @@ class SearchService {
     const pageEvent = this.crowi.event('page');
     pageEvent.on('create', this.delegator.syncPageUpdated.bind(this.delegator));
     pageEvent.on('update', this.delegator.syncPageUpdated.bind(this.delegator));
+    pageEvent.on('deleteCompletely', this.delegator.syncPagesDeletedCompletely.bind(this.delegator));
     pageEvent.on('delete', this.delegator.syncPageDeleted.bind(this.delegator));
 
     const bookmarkEvent = this.crowi.event('bookmark');