refactor: move configs from revision to note

Signed-off-by: yamashush <38120991+yamashush@users.noreply.github.com>

backend/src/config/mock/note.config.mock.ts

@@ -21,6 +21,7 @@ export function createDefaultMockNoteConfig(): NoteConfig {
       },
     },
     guestAccess: GuestAccess.CREATE,
+    revisionRetentionDays: 0,
   };
 }
 

backend/src/config/mock/revision.config.mock.ts

@@ -1,23 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-import { ConfigFactoryKeyHost, registerAs } from '@nestjs/config';
-import { ConfigFactory } from '@nestjs/config/dist/interfaces';
-
-import { RevisionConfig } from '../revision.config';
-
-export function createDefaultMockRevisionConfig(): RevisionConfig {
-  return {
-    retentionDays: 0,
-  };
-}
-
-export function registerRevisionConfig(
-  revisionConfig: RevisionConfig,
-): ConfigFactory<RevisionConfig> & ConfigFactoryKeyHost<RevisionConfig> {
-  return registerAs('revisionConfig', (): RevisionConfig => revisionConfig);
-}
-
-export default registerRevisionConfig(createDefaultMockRevisionConfig());

backend/src/config/note.config.spec.ts

@@ -19,6 +19,7 @@ describe('noteConfig', () => {
   const invalidMaxDocumentLength = 'not-a-max-document-length';
   const guestAccess = GuestAccess.CREATE;
   const wrongDefaultPermission = 'wrong';
+  const retentionDays = 30;
 
   describe('correctly parses config', () => {
     it('when given correct and complete environment variables', () => {
@@ -30,6 +31,7 @@ describe('noteConfig', () => {
           HD_PERMISSION_DEFAULT_EVERYONE: DefaultAccessLevel.READ,
           HD_PERMISSION_DEFAULT_LOGGED_IN: DefaultAccessLevel.READ,
           HD_GUEST_ACCESS: guestAccess,
+          HD_REVISION_RETENTION_DAYS: retentionDays.toString(),
           /* eslint-enable @typescript-eslint/naming-convention */
         },
         {
@@ -47,6 +49,7 @@ describe('noteConfig', () => {
         DefaultAccessLevel.READ,
       );
       expect(config.guestAccess).toEqual(guestAccess);
+      expect(config.revisionRetentionDays).toEqual(retentionDays);
       restore();
     });
 
@@ -221,6 +224,36 @@ describe('noteConfig', () => {
       expect(config.guestAccess).toEqual(GuestAccess.WRITE);
       restore();
     });
+
+    it('when no HD_REVISION_RETENTION_DAYS is set', () => {
+      const restore = mockedEnv(
+        {
+          /* eslint-disable @typescript-eslint/naming-convention */
+          HD_FORBIDDEN_NOTE_IDS: forbiddenNoteIds.join(' , '),
+          HD_MAX_DOCUMENT_LENGTH: maxDocumentLength.toString(),
+          HD_PERMISSION_DEFAULT_EVERYONE: DefaultAccessLevel.READ,
+          HD_PERMISSION_DEFAULT_LOGGED_IN: DefaultAccessLevel.READ,
+          HD_GUEST_ACCESS: guestAccess,
+          /* eslint-enable @typescript-eslint/naming-convention */
+        },
+        {
+          clear: true,
+        },
+      );
+      const config = noteConfig();
+      expect(config.forbiddenNoteIds).toHaveLength(forbiddenNoteIds.length);
+      expect(config.forbiddenNoteIds).toEqual(forbiddenNoteIds);
+      expect(config.maxDocumentLength).toEqual(maxDocumentLength);
+      expect(config.permissions.default.everyone).toEqual(
+        DefaultAccessLevel.READ,
+      );
+      expect(config.permissions.default.loggedIn).toEqual(
+        DefaultAccessLevel.READ,
+      );
+      expect(config.guestAccess).toEqual(guestAccess);
+      expect(config.revisionRetentionDays).toEqual(0);
+      restore();
+    });
   });
 
   describe('throws error', () => {
@@ -454,5 +487,27 @@ describe('noteConfig', () => {
       );
       restore();
     });
+
+    it('when given a negative retention days', async () => {
+      const restore = mockedEnv(
+        {
+          /* eslint-disable @typescript-eslint/naming-convention */
+          HD_FORBIDDEN_NOTE_IDS: invalidforbiddenNoteIds.join(' , '),
+          HD_MAX_DOCUMENT_LENGTH: maxDocumentLength.toString(),
+          HD_PERMISSION_DEFAULT_EVERYONE: DefaultAccessLevel.READ,
+          HD_PERMISSION_DEFAULT_LOGGED_IN: DefaultAccessLevel.READ,
+          HD_GUEST_ACCESS: guestAccess,
+          HD_REVISION_RETENTION_DAYS: (-1).toString(),
+          /* eslint-enable @typescript-eslint/naming-convention */
+        },
+        {
+          clear: true,
+        },
+      );
+      expect(() => noteConfig()).toThrow(
+        '"forbiddenNoteIds[0]" is not allowed to be empty',
+      );
+      restore();
+    });
   });
 });

backend/src/config/note.config.ts

@@ -23,6 +23,7 @@ export interface NoteConfig {
       loggedIn: DefaultAccessLevel;
     };
   };
+  revisionRetentionDays: number;
 }
 
 const schema = Joi.object<NoteConfig>({
@@ -56,6 +57,12 @@ const schema = Joi.object<NoteConfig>({
         .label('HD_PERMISSION_DEFAULT_LOGGED_IN'),
     },
   },
+  revisionRetentionDays: Joi.number()
+    .integer()
+    .default(0)
+    .min(0)
+    .optional()
+    .label('HD_REVISION_RETENTION_DAYS'),
 });
 
 function checkEveryoneConfigIsConsistent(config: NoteConfig): void {
@@ -97,6 +104,9 @@ export default registerAs('noteConfig', () => {
           loggedIn: process.env.HD_PERMISSION_DEFAULT_LOGGED_IN,
         },
       },
+      revisionRetentionDays: parseOptionalNumber(
+        process.env.HD_REVISION_RETENTION_DAYS,
+      ),
     } as NoteConfig,
     {
       abortEarly: false,

backend/src/config/revision.config.spec.ts

@@ -1,61 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-import mockedEnv from 'mocked-env';
-
-import revisionConfig from './revision.config';
-
-describe('revisionConfig', () => {
-  const retentionDays = 30;
-
-  describe('correctly parses config', () => {
-    it('when given correct and complete environment variables', () => {
-      const restore = mockedEnv(
-        {
-          /* eslint-disable @typescript-eslint/naming-convention */
-          HD_REVISION_RETENTION_DAYS: retentionDays.toString(),
-          /* eslint-enable @typescript-eslint/naming-convention */
-        },
-        {
-          clear: true,
-        },
-      );
-      const config = revisionConfig();
-      expect(config.retentionDays).toEqual(retentionDays);
-      restore();
-    });
-
-    it('when no HD_REVISION_RETENTION_DAYS is set', () => {
-      const restore = mockedEnv(
-        {},
-        {
-          clear: true,
-        },
-      );
-      const config = revisionConfig();
-      expect(config.retentionDays).toEqual(0);
-      restore();
-    });
-  });
-
-  describe('throws error', () => {
-    it('when given a negative retention days', async () => {
-      const restore = mockedEnv(
-        {
-          /* eslint-disable @typescript-eslint/naming-convention */
-          HD_REVISION_RETENTION_DAYS: (-1).toString(),
-          /* eslint-enable @typescript-eslint/naming-convention */
-        },
-        {
-          clear: true,
-        },
-      );
-      expect(() => revisionConfig()).toThrow(
-        '"HD_REVISION_RETENTION_DAYS" must be greater than or equal to 0',
-      );
-      restore();
-    });
-  });
-});

backend/src/config/revision.config.ts

@@ -1,45 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-import { registerAs } from '@nestjs/config';
-import * as Joi from 'joi';
-
-import { buildErrorMessage, parseOptionalNumber } from './utils';
-
-export interface RevisionConfig {
-  retentionDays: number;
-}
-
-const schema = Joi.object({
-  retentionDays: Joi.number()
-    .integer()
-    .default(0)
-    .min(0)
-    .optional()
-    .label('HD_REVISION_RETENTION_DAYS'),
-});
-
-export default registerAs('revisionConfig', () => {
-  const revisionConfig = schema.validate(
-    {
-      retentionDays: parseOptionalNumber(
-        process.env.HD_REVISION_RETENTION_DAYS,
-      ),
-    },
-    {
-      abortEarly: false,
-      presence: 'required',
-    },
-  );
-  if (revisionConfig.error) {
-    const errorMessages = revisionConfig.error.details.map(
-      (detail) => detail.message,
-    );
-    console.log(errorMessages);
-
-    throw new Error(buildErrorMessage(errorMessages));
-  }
-  return revisionConfig.value as RevisionConfig;
-});

backend/src/revisions/revisions.service.spec.ts

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 import { ConfigModule } from '@nestjs/config';
+import { createPatch } from 'diff';
 import { EventEmitterModule } from '@nestjs/event-emitter';
 import { Test, TestingModule } from '@nestjs/testing';
 import { getRepositoryToken } from '@nestjs/typeorm';
@@ -17,10 +18,10 @@ import authConfigMock from '../config/mock/auth.config.mock';
 import databaseConfigMock from '../config/mock/database.config.mock';
 import noteConfigMock from '../config/mock/note.config.mock';
 import {
-  createDefaultMockRevisionConfig,
-  registerRevisionConfig,
-} from '../config/mock/revision.config.mock';
-import { RevisionConfig } from '../config/revision.config';
+  createDefaultMockNoteConfig,
+  registerNoteConfig,
+} from '../config/mock/note.config.mock';
+import { NoteConfig } from '../config/note.config';
 import { NotInDBError } from '../errors/errors';
 import { eventModuleConfig } from '../events';
 import { Group } from '../groups/group.entity';
@@ -43,7 +44,7 @@ describe('RevisionsService', () => {
   let service: RevisionsService;
   let revisionRepo: Repository<Revision>;
   let noteRepo: Repository<Note>;
-  const revisionConfig: RevisionConfig = createDefaultMockRevisionConfig();
+  const noteConfig: NoteConfig = createDefaultMockNoteConfig();
 
   beforeEach(async () => {
     noteRepo = new Repository<Note>(
@@ -80,7 +81,7 @@ describe('RevisionsService', () => {
             databaseConfigMock,
             authConfigMock,
             noteConfigMock,
-            registerRevisionConfig(revisionConfig),
+            registerNoteConfig(noteConfig),
           ],
         }),
         EventEmitterModule.forRoot(eventModuleConfig),
@@ -459,9 +460,9 @@ describe('RevisionsService', () => {
     const retentionDays = 30;
 
     beforeEach(() => {
-      revisionConfig.retentionDays = retentionDays;
+      noteConfig.revisionRetentionDays = retentionDays;
 
-      note = Mock.of<Note>({ id: 1 });
+      note = Mock.of<Note>({ publicId: 'test-note', id: 1 });
       notes = [note];
     });
 
@@ -469,12 +470,12 @@ describe('RevisionsService', () => {
       jest.clearAllMocks();
     });
 
-    it('remove all except latest revision', async () => {
+    it('remove all revisions except latest revision', async () => {
       const date1 = new Date();
       const date2 = new Date();
       const date3 = new Date();
-      date1.setDate(date1.getDate() - retentionDays);
-      date2.setDate(date2.getDate() - retentionDays + 1);
+      date1.setDate(date1.getDate() - retentionDays - 2);
+      date2.setDate(date2.getDate() - retentionDays - 1);
 
       const revision1 = Mock.of<Revision>({
         id: 1,
@@ -485,12 +486,81 @@ describe('RevisionsService', () => {
         id: 2,
         createdAt: date2,
         note: Promise.resolve(note),
+        content: 'old content\n',
+      });
+      const revision3 = Mock.of<Revision>({
+        id: 3,
+        createdAt: date3,
+        note: Promise.resolve(note),
+        content: '---\ntitle: new title\ndescription: new description\ntags: [ "tag1" ]\n---\nnew content\n',
+      });
+      revision3.patch = createPatch(
+        note.publicId,
+        revision2.content,
+        revision3.content,
+      )
+
+      revisions = [revision1, revision2, revision3];
+      oldRevisions = [revision1, revision2];
+
+      jest.spyOn(noteRepo, 'find').mockResolvedValueOnce(notes);
+      jest.spyOn(revisionRepo, 'find').mockResolvedValueOnce(revisions);
+      jest
+        .spyOn(revisionRepo, 'remove')
+        .mockImplementationOnce(async (entry, _) => {
+          expect(entry).toEqual(oldRevisions);
+          return entry;
+        });
+      jest
+        .spyOn(revisionRepo, 'save')
+        .mockResolvedValue(revision3);
+
+      await service.removeOldRevisions();
+      expect(revision3.patch).toMatchInlineSnapshot(`
+        "Index: test-note
+        ===================================================================
+        --- test-note
+        +++ test-note
+        @@ -0,0 +1,6 @@
+        +---
+        +title: new title
+        +description: new description
+        +tags: [ "tag1" ]
+        +---
+        +new content
+        "
+      `);
+    });
+
+    it('remove a part of old revisions', async () => {
+      const date1 = new Date();
+      const date2 = new Date();
+      const date3 = new Date();
+      date1.setDate(date1.getDate() - retentionDays);
+      date2.setDate(date2.getDate() - retentionDays + 1);
+
+      const revision1 = Mock.of<Revision>({
+        id: 1,
+        createdAt: date1,
+        note: Promise.resolve(note),
+        content: 'old content\n',
+      });
+      const revision2 = Mock.of<Revision>({
+        id: 2,
+        createdAt: date2,
+        note: Promise.resolve(note),
+        content: '---\ntitle: new title\ndescription: new description\ntags: [ "tag1" ]\n---\nnew content\n',
       });
       const revision3 = Mock.of<Revision>({
         id: 3,
         createdAt: date3,
         note: Promise.resolve(note),
       });
+      revision2.patch = createPatch(
+        note.publicId,
+        revision1.content,
+        revision2.content,
+      )
 
       revisions = [revision1, revision2, revision3];
       oldRevisions = [revision1];
@@ -503,8 +573,26 @@ describe('RevisionsService', () => {
           expect(entry).toEqual(oldRevisions);
           return entry;
         });
+      jest
+        .spyOn(revisionRepo, 'save')
+        .mockResolvedValue(revision2);
 
       await service.removeOldRevisions();
+      expect(revision2.patch).toMatchInlineSnapshot(`
+        "Index: test-note
+        ===================================================================
+        --- test-note
+        +++ test-note
+        @@ -1,1 +1,6 @@
+        -old content
+        +---
+        +title: new title
+        +description: new description
+        +tags: [ "tag1" ]
+        +---
+        +new content
+        "
+      `);
     });
 
     it('do nothing when only one revision', async () => {
@@ -528,7 +616,7 @@ describe('RevisionsService', () => {
     });
 
     it('do nothing when retention days config is zero', async () => {
-      revisionConfig.retentionDays = 0;
+      noteConfig.revisionRetentionDays = 0;
       const spyOnRemove = jest.spyOn(revisionRepo, 'remove');
 
       await service.removeOldRevisions();

backend/src/revisions/revisions.service.ts

@@ -9,9 +9,9 @@ import { InjectRepository } from '@nestjs/typeorm';
 import { createPatch } from 'diff';
 import { Repository } from 'typeorm';
 
-import revisionConfiguration, {
-  RevisionConfig,
-} from '../config/revision.config';
+import noteConfiguration, {
+  NoteConfig,
+} from '../config/note.config';
 import { NotInDBError } from '../errors/errors';
 import { ConsoleLoggerService } from '../logger/console-logger.service';
 import { Note } from '../notes/note.entity';
@@ -35,7 +35,7 @@ export class RevisionsService {
     private revisionRepository: Repository<Revision>,
     @InjectRepository(Note)
     private noteRepository: Repository<Note>,
-    @Inject(revisionConfiguration.KEY) private revisionConfig: RevisionConfig,
+    @Inject(noteConfiguration.KEY) private noteConfig: NoteConfig,
     private editService: EditService,
   ) {
     this.logger.setContext(RevisionsService.name);
@@ -241,7 +241,7 @@ export class RevisionsService {
 
   async removeOldRevisions(): Promise<void> {
     const currentTime = new Date().getTime();
-    const revisionRetentionDays: number = this.revisionConfig.retentionDays;
+    const revisionRetentionDays: number = this.noteConfig.revisionRetentionDays;
     if (revisionRetentionDays <= 0) {
       return;
     }
@@ -252,18 +252,40 @@ export class RevisionsService {
         where: {
           note: { id: note.id },
         },
+        order: {
+          createdAt: "ASC",
+        },
       });
+
       const oldRevisions = revisions
-        .sort((a, b) => a.createdAt.getTime() - b.createdAt.getTime())
         .slice(0, -1) // always keep the latest revision
         .filter(
           (revision) =>
             new Date(revision.createdAt).getTime() <=
             currentTime - revisionRetentionDays * 24 * 60 * 60 * 1000,
         );
+      const remainedRevisions = revisions.filter((val) => !oldRevisions.includes(val))
 
       if (!oldRevisions.length) {
         continue;
+
+      } else if (oldRevisions.length == revisions.length - 1 ){
+        const beUpdatedRevision = revisions.slice(-1)[0]
+        beUpdatedRevision.patch = createPatch(
+          note.publicId,
+          '', // donnt exist older revision
+          beUpdatedRevision.content,
+        );
+        await this.revisionRepository.save(beUpdatedRevision);
+
+      } else {
+        const beUpdatedRevision = remainedRevisions.slice(0)[0]
+        beUpdatedRevision.patch = createPatch(
+          note.publicId,
+          oldRevisions.slice(-1)[0].content,
+          beUpdatedRevision.content,
+        );
+        await this.revisionRepository.save(beUpdatedRevision);
       }
 
       await this.revisionRepository.remove(oldRevisions);

docs/content/references/config/notes.md

@@ -7,3 +7,4 @@
 | `HD_GUEST_ACCESS`                 | `write` | `deny`, `read`, `write`, `create` | Defines the maximum access level for guest users to the instance. If guest access is set lower than the "everyone" permission of a note then the note permission will be overridden. |
 | `HD_PERMISSION_DEFAULT_LOGGED_IN` | `write` | `none`, `read`, `write`           | The default permission for the "logged-in" group that is set on new notes.                                                                                                           |
 | `HD_PERMISSION_DEFAULT_EVERYONE`  | `read`  | `none`, `read`, `write`           | The default permission for the "everyone" group (logged-in & guest users), that is set on new notes created by logged-in users. Notes created by guests always set this to "write".  |
+| `HD_REVISION_RETENTION_DAYS`      | 0       |                                   | The number of days a revision should be kept. If the config option is not set or set to 0, all revisions will be kept forever. |

docs/content/references/config/revision.md

@@ -1,8 +0,0 @@
-# Revisions
-
-| environment variable         | default | example | description                                                                                                                    |
-| ---------------------------- | ------- | ------- | ------------------------------------------------------------------------------------------------------------------------------ |
-| `HD_REVISION_RETENTION_DAYS` | 0       |         | The number of days a revision should be kept. If the config option is not set or set to 0, all revisions will be kept forever. |
-<!--
-| `HD_IMAGE_PROXY` | -       | `https://image-proxy.example.com`   | **ToDo:** Add description  |
--->