Merge pull request #9002 from weseek/fix/broken-revisions-collection

feat: Automatically repair corrupted data, at least for the latest revision

apps/app/src/server/routes/apiv3/page/index.ts

@@ -17,12 +17,13 @@ import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity
 import { apiV3FormValidator } from '~/server/middlewares/apiv3-form-validator';
 import { excludeReadOnlyUser } from '~/server/middlewares/exclude-read-only-user';
 import { GlobalNotificationSettingEvent } from '~/server/models/GlobalNotificationSetting';
-import type { PageModel } from '~/server/models/page';
+import type { PageDocument, PageModel } from '~/server/models/page';
 import { Revision } from '~/server/models/revision';
 import Subscription from '~/server/models/subscription';
 import { configManager } from '~/server/service/config-manager';
 import type { IPageGrantService } from '~/server/service/page-grant';
 import { preNotifyService } from '~/server/service/pre-notify';
+import { normalizeLatestRevisionIfBroken } from '~/server/service/revision/normalize-latest-revision-if-broken';
 import loggerFactory from '~/utils/logger';
 
 import type { ApiV3Response } from '../interfaces/apiv3-response';
@@ -741,14 +742,17 @@ module.exports = (crowi) => {
   *            description: Return page's markdown
   */
   router.get('/export/:pageId', loginRequiredStrictly, validator.export, async(req, res) => {
-    const { pageId } = req.params;
+    const pageId: string = req.params.pageId;
     const { format, revisionId = null } = req.query;
     let revision;
     let pagePath;
 
+    const Page = mongoose.model<PageDocument, PageModel>('Page');
+
+    let page: PageDocument;
+
     try {
-      const Page = crowi.model('Page');
-      const page = await Page.findByIdAndViewer(pageId, req.user);
+      page = await Page.findByIdAndViewer(pageId, req.user);
 
       if (page == null) {
         const isPageExist = await Page.count({ _id: pageId }) > 0;
@@ -758,8 +762,22 @@ module.exports = (crowi) => {
         }
         return res.apiv3Err(new ErrorV3(`Page ${pageId} is not exist.`), 404);
       }
+    }
+    catch (err) {
+      logger.error('Failed to get page data', err);
+      return res.apiv3Err(err, 500);
+    }
+
+    // Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+    try {
+      await normalizeLatestRevisionIfBroken(pageId);
+    }
+    catch (err) {
+      logger.error('Error occurred in normalizing the latest revision');
+    }
 
-      const revisionIdForFind = revisionId || page.revision;
+    try {
+      const revisionIdForFind = revisionId ?? page.revision;
 
       revision = await Revision.findById(revisionIdForFind);
       pagePath = page.path;
@@ -770,7 +788,7 @@ module.exports = (crowi) => {
       }
     }
     catch (err) {
-      logger.error('Failed to get page data', err);
+      logger.error('Failed to get revision data', err);
       return res.apiv3Err(err, 500);
     }
 

apps/app/src/server/routes/apiv3/page/update-page.ts

@@ -18,6 +18,7 @@ import {
 } from '~/server/models';
 import type { PageDocument, PageModel } from '~/server/models/page';
 import { preNotifyService } from '~/server/service/pre-notify';
+import { normalizeLatestRevisionIfBroken } from '~/server/service/revision/normalize-latest-revision-if-broken';
 import { getYjsService } from '~/server/service/yjs';
 import { generalXssFilter } from '~/services/general-xss-filter';
 import loggerFactory from '~/utils/logger';
@@ -132,6 +133,16 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
       // check revision
       const currentPage = await Page.findByIdAndViewer(pageId, req.user);
 
+      if (currentPage != null) {
+        // Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+        try {
+          await normalizeLatestRevisionIfBroken(pageId);
+        }
+        catch (err) {
+          logger.error('Error occurred in normalizing the latest revision');
+        }
+      }
+
       if (currentPage != null && !await currentPage.isUpdatable(sanitizeRevisionId, origin)) {
         const latestRevision = await Revision.findById(currentPage.revision).populate('author');
         const returnLatestRevision = {

apps/app/src/server/routes/apiv3/revisions.js

@@ -1,6 +1,7 @@
 import { ErrorV3 } from '@growi/core/dist/models';
 
 import { Revision } from '~/server/models/revision';
+import { normalizeLatestRevisionIfBroken } from '~/server/service/revision/normalize-latest-revision-if-broken';
 import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
@@ -121,6 +122,14 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3('Current user is not accessible to this page.', 'forbidden-page'), 403);
     }
 
+    // Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+    try {
+      await normalizeLatestRevisionIfBroken(pageId);
+    }
+    catch (err) {
+      logger.error('Error occurred in normalizing the latest revision');
+    }
+
     try {
       const page = await Page.findOne({ _id: pageId });
       const queryOpts = {

apps/app/src/server/service/revision/normalize-latest-revision-if-broken.integ.ts

@@ -0,0 +1,124 @@
+import { getIdForRef } from '@growi/core';
+import type { HydratedDocument } from 'mongoose';
+import mongoose, { Types } from 'mongoose';
+
+import type { PageDocument, PageModel } from '~/server/models/page';
+import PageModelFactory from '~/server/models/page';
+import { Revision } from '~/server/models/revision';
+
+import { normalizeLatestRevisionIfBroken } from './normalize-latest-revision-if-broken';
+
+describe('normalizeLatestRevisionIfBroken', () => {
+
+  beforeAll(async() => {
+    await PageModelFactory(null);
+  });
+
+
+  test('should update the latest revision', async() => {
+    const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+
+    // == Arrange
+    const page = await Page.create({ path: '/foo' });
+    const revision = await Revision.create({ pageId: page._id, body: '' });
+    // connect the page and the revision
+    page.revision = revision._id;
+    await page.save();
+    // break the revision
+    await Revision.updateOne({ _id: revision._id }, { pageId: new Types.ObjectId() });
+
+    // spy
+    const updateOneSpy = vi.spyOn(Revision, 'updateOne');
+
+    // == Act
+    await normalizeLatestRevisionIfBroken(page._id);
+
+    // == Assert
+    // assert spy
+    expect(updateOneSpy).toHaveBeenCalled();
+
+    // assert revision
+    const revisionById = await Revision.findById(revision._id);
+    const revisionByPageId = await Revision.findOne({ pageId: page._id });
+    expect(revisionById).not.toBeNull();
+    expect(revisionByPageId).not.toBeNull();
+    assert(revisionById != null);
+    assert(revisionByPageId != null);
+    expect(revisionById._id).toEqual(revisionByPageId._id);
+    expect(getIdForRef(revisionById.pageId)).toEqual(page._id.toString());
+  });
+
+
+  describe('should returns without any operation', () => {
+    test('when the page has revisions at least one', async() => {
+      const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+
+      // Arrange
+      const page = await Page.create({ path: '/foo' });
+      await Revision.create({ pageId: page._id, body: '' });
+      // spy
+      const updateOneSpy = vi.spyOn(Revision, 'updateOne');
+
+      // Act
+      await normalizeLatestRevisionIfBroken(page._id);
+
+      // Assert
+      expect(updateOneSpy).not.toHaveBeenCalled();
+    });
+
+    test('when the page is not found', async() => {
+      // Arrange
+      const pageIdOfRevision = new Types.ObjectId();
+      // create an orphan revision
+      await Revision.create({ pageId: pageIdOfRevision, body: '' });
+
+      // spy
+      const updateOneSpy = vi.spyOn(Revision, 'updateOne');
+
+      // Act
+      await normalizeLatestRevisionIfBroken(pageIdOfRevision);
+
+      // Assert
+      expect(updateOneSpy).not.toHaveBeenCalled();
+    });
+
+    test('when the page.revision is null', async() => {
+      const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+
+      // Arrange
+      const page = await Page.create({ path: '/foo' });
+      // create an orphan revision
+      await Revision.create({ pageId: page._id, body: '' });
+
+      // spy
+      const updateOneSpy = vi.spyOn(Revision, 'updateOne');
+
+      // Act
+      await normalizeLatestRevisionIfBroken(page._id);
+
+      // Assert
+      expect(updateOneSpy).not.toHaveBeenCalled();
+    });
+
+    test('when the page.revision does not exist', async() => {
+      const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+
+      // Arrange
+      const revisionNonExistent = new Types.ObjectId();
+      const page = await Page.create({ path: '/foo', revision: revisionNonExistent });
+      // create an orphan revision
+      await Revision.create({ pageId: page._id, body: '' });
+
+      // spy
+      const updateOneSpy = vi.spyOn(Revision, 'updateOne');
+
+      // Act
+      await normalizeLatestRevisionIfBroken(page._id);
+
+      // Assert
+      expect(updateOneSpy).not.toHaveBeenCalled();
+    });
+
+  });
+
+});

apps/app/src/server/service/revision/normalize-latest-revision-if-broken.ts

@@ -0,0 +1,38 @@
+import type { HydratedDocument, Types } from 'mongoose';
+import mongoose from 'mongoose';
+
+import type { PageDocument, PageModel } from '~/server/models/page';
+import { Revision } from '~/server/models/revision';
+import loggerFactory from '~/utils/logger';
+
+
+const logger = loggerFactory('growi:service:revision:normalize-latest-revision');
+
+/**
+ * Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+ *
+ * @ref https://github.com/weseek/growi/pull/8998
+ */
+export const normalizeLatestRevisionIfBroken = async(pageId: string | Types.ObjectId): Promise<void> => {
+
+  if (await Revision.exists({ pageId: { $eq: pageId } })) {
+    return;
+  }
+
+  logger.info(`The page ('${pageId}') does not have any revisions. Normalization of the latest revision will be started.`);
+
+  const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+  const page = await Page.findOne({ _id: { $eq: pageId } }, { revision: 1 }).exec();
+
+  if (page == null) {
+    logger.warn(`Normalization has been canceled since the page ('${pageId}') could not be found.`);
+    return;
+  }
+  if (page.revision == null || !(await Revision.exists({ _id: page.revision }))) {
+    logger.warn(`Normalization has been canceled since the Page.revision of the page ('${pageId}') could not be found.`);
+    return;
+  }
+
+  // update Revision.pageId
+  await Revision.updateOne({ _id: page.revision }, { $set: { pageId } }).exec();
+};

apps/app/src/server/service/yjs/sync-ydoc.ts

@@ -4,6 +4,7 @@ import type { Document } from 'y-socket.io/dist/server';
 import loggerFactory from '~/utils/logger';
 
 import { Revision } from '../../models/revision';
+import { normalizeLatestRevisionIfBroken } from '../revision/normalize-latest-revision-if-broken';
 
 import type { MongodbPersistence } from './extended/mongodb-persistence';
 
@@ -26,6 +27,9 @@ type Context = {
 export const syncYDoc = async(mdb: MongodbPersistence, doc: Document, context: true | Context): Promise<void> => {
   const pageId = doc.name;
 
+  // Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+  await normalizeLatestRevisionIfBroken(pageId);
+
   const revision = await Revision
     .findOne(
       // filter

apps/app/src/server/service/yjs/yjs.integ.ts

@@ -17,6 +17,9 @@ vi.mock('y-socket.io/dist/server', () => {
   return { YSocketIO };
 });
 
+vi.mock('../revision/normalize-latest-revision-if-broken', () => ({
+  normalizeLatestRevisionIfBroken: vi.fn(),
+}));
 
 const ObjectId = Types.ObjectId;
 

apps/app/src/server/service/yjs/yjs.ts

@@ -15,6 +15,7 @@ import loggerFactory from '~/utils/logger';
 
 import type { PageModel } from '../../models/page';
 import { Revision } from '../../models/revision';
+import { normalizeLatestRevisionIfBroken } from '../revision/normalize-latest-revision-if-broken';
 
 import { createIndexes } from './create-indexes';
 import { createMongoDBPersistence } from './create-mongodb-persistence';
@@ -141,6 +142,9 @@ class YjsService implements IYjsService {
       logger.debug(`getYDocStatus('${pageId}') detected '${status}'`, args ?? {});
     };
 
+    // Normalize the latest revision which was borken by the migration script '20211227060705-revision-path-to-page-id-schema-migration--fixed-7549.js'
+    await normalizeLatestRevisionIfBroken(pageId);
+
     // get the latest revision createdAt
     const result = await Revision
       .findOne(