Browse Source

Merge branch 'feat/page-migration-remove-unique-constraint' into feat/update-parent-when-create-page

Taichi Masuyama 4 years ago
parent
commit
4655239df2
2 changed files with 61 additions and 37 deletions
  1. 13 15
      packages/app/src/server/routes/apiv3/pages.js
  2. 48 22
      packages/app/src/server/service/page.js

+ 13 - 15
packages/app/src/server/routes/apiv3/pages.js

@@ -684,29 +684,27 @@ module.exports = (crowi) => {
 
   });
 
-  // TODO: use socket conn to show progress
   router.post('/v5-schema-migration', accessTokenParser, loginRequired, adminRequired, csrf, validator.v5PageMigration, apiV3FormValidator, async(req, res) => {
     const { action } = req.body;
     const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
 
-    switch (action) {
-      case 'upgrade':
-        try {
+    try {
+      switch (action) {
+        case 'upgrade':
           if (!isV5Compatible) {
             const Page = crowi.model('Page');
-            // not await
-            crowi.pageService.v5RecursiveMigration(Page.GRANT_PUBLIC);
+            // this method throws and emit socketIo event when error occurs
+            crowi.pageService.v5Migration(Page.GRANT_PUBLIC); // not await
           }
-        }
-        catch (err) {
-          logger.error('Error\n', err);
-          return res.apiv3Err(new ErrorV3('Failed to migrate pages. Please try again.', 'v5_migration_failed'), 500);
-        }
-        break;
+          break;
 
-      default:
-        logger.error(`${action} action is not supported.`);
-        return res.apiv3Err(new ErrorV3('This action is not supported.', 'not_supported'), 400);
+        default:
+          logger.error(`${action} action is not supported.`);
+          return res.apiv3Err(new ErrorV3('This action is not supported.', 'not_supported'), 400);
+      }
+    }
+    catch (err) {
+      return res.apiv3Err(new ErrorV3(`Failed to migrate pages: ${err.message}`), 500);
     }
 
     return res.apiv3({ isV5Compatible });

+ 48 - 22
packages/app/src/server/service/page.js

@@ -738,7 +738,21 @@ class PageService {
     }
   }
 
-  async v5RecursiveMigration(grant, rootPath = null) {
+  async v5Migration(grant, rootPath = null) {
+    const socket = this.crowicrowi.socketIoService.getAdminSocket();
+    try {
+      await this._v5RecursiveMigration(grant, rootPath);
+    }
+    catch (err) {
+      logger.error('V5 miration failed.', err);
+      socket.emit('v5MirationFailed', { error: err.message });
+
+      throw err;
+    }
+  }
+
+  // TODO: use websocket to show progress
+  async _v5RecursiveMigration(grant, rootPath) {
     const BATCH_SIZE = 100;
     const PAGES_LIMIT = 1000;
     const Page = this.crowi.model('Page');
@@ -767,7 +781,7 @@ class PageService {
       baseAggregation = baseAggregation.limit(Math.floor(total * 0.3));
     }
 
-    const randomPagesStream = await baseAggregation.cursor({ batchSize: BATCH_SIZE }).exec();
+    const pagesStream = await baseAggregation.cursor({ batchSize: BATCH_SIZE }).exec();
 
     // use batch stream
     const batchStream = createBatchStream(BATCH_SIZE);
@@ -800,6 +814,7 @@ class PageService {
         }
         catch (err) {
           logger.error('Failed to insert empty pages.', err);
+          throw err;
         }
 
         // find parents again
@@ -837,6 +852,7 @@ class PageService {
         }
         catch (err) {
           logger.error('Failed to update page.parent.', err);
+          throw err;
         }
 
         callback();
@@ -846,11 +862,12 @@ class PageService {
       },
     });
 
-    randomPagesStream
+    pagesStream
       .pipe(batchStream)
       .pipe(migratePagesStream);
 
     await streamToPromise(migratePagesStream);
+
     if (await Page.exists({ grant, parent: null, path: { $ne: '/' } })) {
       return this.v5RecursiveMigration(grant, rootPath);
     }
@@ -863,24 +880,33 @@ class PageService {
     }
 
     const collection = mongoose.connection.collection('pages');
-    try {
-      // drop pages.path_1 indexes
-      await collection.dropIndex('path_1');
-      logger.info('Succeeded to drop unique indexes from pages.path.');
-    }
-    catch (err) {
-      // return not to set app:isV5Compatible to true
-      return logger.error('Failed to drop unique indexes from pages.path.', err);
-    }
+    const indexStatus = await Page.aggregate([{ $indexStats: {} }]);
+    const pathIndexStatus = indexStatus.filter(status => status.name === 'path_1')?.[0]?.spec?.unique;
 
-    try {
-      // create indexes without
-      await collection.createIndex({ path: 1 }, { unique: false });
-      logger.info('Succeeded to create non-unique indexes on pages.path.');
-    }
-    catch (err) {
-      // return not to set app:isV5Compatible to true
-      return logger.error('Failed to create non-unique indexes on pages.path.', err);
+    // skip index modification if path is unique
+    if (pathIndexStatus == null || pathIndexStatus === true) {
+      // drop only when true. this condition is for in case createIndex failed but dropIndex succeeded before
+      if (pathIndexStatus) {
+        try {
+          // drop pages.path_1 indexes
+          await collection.dropIndex('path_1');
+          logger.info('Succeeded to drop unique indexes from pages.path.');
+        }
+        catch (err) {
+          logger.warn('Failed to drop unique indexes from pages.path.', err);
+          throw err;
+        }
+      }
+
+      try {
+        // create indexes without
+        await collection.createIndex({ path: 1 }, { unique: false });
+        logger.info('Succeeded to create non-unique indexes on pages.path.');
+      }
+      catch (err) {
+        logger.warn('Failed to create non-unique indexes on pages.path.', err);
+        throw err;
+      }
     }
 
     try {
@@ -890,8 +916,8 @@ class PageService {
       logger.info('Successfully migrated all public pages.');
     }
     catch (err) {
-      // just to know
-      logger.error('Failed to update app:isV5Compatible to true.');
+      logger.warn('Failed to update app:isV5Compatible to true.');
+      throw err;
     }
   }