diff --git a/docs/content/config/index.md b/docs/content/config/index.md index 51d19cf49..1942bb157 100644 --- a/docs/content/config/index.md +++ b/docs/content/config/index.md @@ -19,14 +19,15 @@ We also provide an `.env.example` file containing a minimal configuration in the ## General -| environment variable | default | example | description | -|--------------------------|-----------|------------------------------|---------------------------------------------------------------------------------------------------| -| `HD_DOMAIN` | - | `https://md.example.com` | The URL the HedgeDoc instance runs on. | -| `PORT` | 3000 | | The port the HedgeDoc instance runs on. | -| `HD_RENDERER_ORIGIN` | HD_DOMAIN | | The URL the renderer runs on. If omitted this will be same as `HD_DOMAIN`. | -| `HD_LOGLEVEL` | warn | | The loglevel that should be used. Options are `error`, `warn`, `info`, `debug` or `trace`. | -| `HD_FORBIDDEN_NOTE_IDS` | - | `notAllowed, alsoNotAllowed` | A list of note ids (separated by `,`), that are not allowed to be created or requested by anyone. | -| `HD_MAX_DOCUMENT_LENGTH` | 100000 | | The maximum length of any one document. Changes to this will impact performance for your users. | +| environment variable | default | example | description | +|--------------------------|-----------|------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------| +| `HD_DOMAIN` | - | `https://md.example.com` | The URL the HedgeDoc instance runs on. | +| `PORT` | 3000 | | The port the HedgeDoc instance runs on. | +| `HD_RENDERER_ORIGIN` | HD_DOMAIN | | The URL the renderer runs on. If omitted this will be same as `HD_DOMAIN`. | +| `HD_LOGLEVEL` | warn | | The loglevel that should be used. Options are `error`, `warn`, `info`, `debug` or `trace`. | +| `HD_FORBIDDEN_NOTE_IDS` | - | `notAllowed, alsoNotAllowed` | A list of note ids (separated by `,`), that are not allowed to be created or requested by anyone. | +| `HD_MAX_DOCUMENT_LENGTH` | 100000 | | The maximum length of any one document. Changes to this will impact performance for your users. | +| `HD_PERSIST_INTERVAL` | 10 | `0`, `5`, `10`, `20` | The time interval in **minutes** for the periodic note revision creation during realtime editing. `0` deactivates the periodic note revision creation. | ### Why should I want to run my renderer on a different (sub-)domain? diff --git a/package.json b/package.json index d9a64a281..b717c7230 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "cli-color": "2.0.3", "connect-typeorm": "2.0.0", "cookie": "0.5.0", + "diff": "5.1.0", "express-session": "1.17.3", "file-type": "16.5.4", "joi": "17.6.0", @@ -82,6 +83,7 @@ "@types/cli-color": "2.0.2", "@types/cookie": "0.5.1", "@types/cookie-signature": "1.0.4", + "@types/diff": "5.0.2", "@types/express": "4.17.13", "@types/express-session": "1.17.5", "@types/jest": "28.1.6", diff --git a/src/config/app.config.spec.ts b/src/config/app.config.spec.ts index c2ba39ece..49dbccc15 100644 --- a/src/config/app.config.spec.ts +++ b/src/config/app.config.spec.ts @@ -19,6 +19,7 @@ describe('appConfig', () => { const invalidPort = 'not-a-port'; const loglevel = Loglevel.TRACE; const invalidLoglevel = 'not-a-loglevel'; + const invalidPersistInterval = -1; describe('correctly parses config', () => { it('when given correct and complete environment variables', async () => { @@ -29,6 +30,7 @@ describe('appConfig', () => { HD_RENDERER_ORIGIN: rendererOrigin, PORT: port.toString(), HD_LOGLEVEL: loglevel, + HD_PERSIST_INTERVAL: '100', /* eslint-enable @typescript-eslint/naming-convention */ }, { @@ -40,6 +42,7 @@ describe('appConfig', () => { expect(config.rendererOrigin).toEqual(rendererOrigin); expect(config.port).toEqual(port); expect(config.loglevel).toEqual(loglevel); + expect(config.persistInterval).toEqual(100); restore(); }); @@ -50,6 +53,7 @@ describe('appConfig', () => { HD_DOMAIN: domain, PORT: port.toString(), HD_LOGLEVEL: loglevel, + HD_PERSIST_INTERVAL: '100', /* eslint-enable @typescript-eslint/naming-convention */ }, { @@ -61,6 +65,7 @@ describe('appConfig', () => { expect(config.rendererOrigin).toEqual(domain); expect(config.port).toEqual(port); expect(config.loglevel).toEqual(loglevel); + expect(config.persistInterval).toEqual(100); restore(); }); @@ -71,6 +76,7 @@ describe('appConfig', () => { HD_DOMAIN: domain, HD_RENDERER_ORIGIN: rendererOrigin, HD_LOGLEVEL: loglevel, + HD_PERSIST_INTERVAL: '100', /* eslint-enable @typescript-eslint/naming-convention */ }, { @@ -82,6 +88,7 @@ describe('appConfig', () => { expect(config.rendererOrigin).toEqual(rendererOrigin); expect(config.port).toEqual(3000); expect(config.loglevel).toEqual(loglevel); + expect(config.persistInterval).toEqual(100); restore(); }); @@ -92,6 +99,7 @@ describe('appConfig', () => { HD_DOMAIN: domain, HD_RENDERER_ORIGIN: rendererOrigin, PORT: port.toString(), + HD_PERSIST_INTERVAL: '100', /* eslint-enable @typescript-eslint/naming-convention */ }, { @@ -103,6 +111,54 @@ describe('appConfig', () => { expect(config.rendererOrigin).toEqual(rendererOrigin); expect(config.port).toEqual(port); expect(config.loglevel).toEqual(Loglevel.WARN); + expect(config.persistInterval).toEqual(100); + restore(); + }); + + it('when no HD_PERSIST_INTERVAL is set', () => { + const restore = mockedEnv( + { + /* eslint-disable @typescript-eslint/naming-convention */ + HD_DOMAIN: domain, + HD_RENDERER_ORIGIN: rendererOrigin, + HD_LOGLEVEL: loglevel, + PORT: port.toString(), + /* eslint-enable @typescript-eslint/naming-convention */ + }, + { + clear: true, + }, + ); + const config = appConfig(); + expect(config.domain).toEqual(domain); + expect(config.rendererOrigin).toEqual(rendererOrigin); + expect(config.port).toEqual(port); + expect(config.loglevel).toEqual(Loglevel.TRACE); + expect(config.persistInterval).toEqual(10); + restore(); + }); + + it('when HD_PERSIST_INTERVAL is zero', () => { + const restore = mockedEnv( + { + /* eslint-disable @typescript-eslint/naming-convention */ + HD_DOMAIN: domain, + HD_RENDERER_ORIGIN: rendererOrigin, + HD_LOGLEVEL: loglevel, + PORT: port.toString(), + HD_PERSIST_INTERVAL: '0', + /* eslint-enable @typescript-eslint/naming-convention */ + }, + { + clear: true, + }, + ); + const config = appConfig(); + expect(config.domain).toEqual(domain); + expect(config.rendererOrigin).toEqual(rendererOrigin); + expect(config.port).toEqual(port); + expect(config.loglevel).toEqual(Loglevel.TRACE); + expect(config.persistInterval).toEqual(0); restore(); }); }); @@ -210,5 +266,23 @@ describe('appConfig', () => { expect(() => appConfig()).toThrow('HD_LOGLEVEL'); restore(); }); + + it('when given a negative HD_PERSIST_INTERVAL', () => { + const restore = mockedEnv( + { + /* eslint-disable @typescript-eslint/naming-convention */ + HD_DOMAIN: domain, + PORT: port.toString(), + HD_LOGLEVEL: invalidLoglevel, + HD_PERSIST_INTERVAL: invalidPersistInterval.toString(), + /* eslint-enable @typescript-eslint/naming-convention */ + }, + { + clear: true, + }, + ); + expect(() => appConfig()).toThrow('HD_PERSIST_INTERVAL'); + restore(); + }); }); }); diff --git a/src/config/app.config.ts b/src/config/app.config.ts index 80548078a..912e38f17 100644 --- a/src/config/app.config.ts +++ b/src/config/app.config.ts @@ -14,6 +14,7 @@ export interface AppConfig { rendererOrigin: string; port: number; loglevel: Loglevel; + persistInterval: number; } const schema = Joi.object({ @@ -41,6 +42,12 @@ const schema = Joi.object({ .default(Loglevel.WARN) .optional() .label('HD_LOGLEVEL'), + persistInterval: Joi.number() + .integer() + .min(0) + .default(10) + .optional() + .label('HD_PERSIST_INTERVAL'), }); export default registerAs('appConfig', () => { @@ -50,6 +57,7 @@ export default registerAs('appConfig', () => { rendererOrigin: process.env.HD_RENDERER_ORIGIN, port: parseOptionalNumber(process.env.PORT), loglevel: process.env.HD_LOGLEVEL, + persistInterval: process.env.HD_PERSIST_INTERVAL, }, { abortEarly: false, diff --git a/src/config/mock/app.config.mock.ts b/src/config/mock/app.config.mock.ts index 059887e80..e79938911 100644 --- a/src/config/mock/app.config.mock.ts +++ b/src/config/mock/app.config.mock.ts @@ -15,5 +15,6 @@ export default registerAs( rendererOrigin: 'md-renderer.example.com', port: 3000, loglevel: Loglevel.ERROR, + persistInterval: 10, }), ); diff --git a/src/frontend-config/frontend-config.service.spec.ts b/src/frontend-config/frontend-config.service.spec.ts index 2fce73558..a7826f3b6 100644 --- a/src/frontend-config/frontend-config.service.spec.ts +++ b/src/frontend-config/frontend-config.service.spec.ts @@ -168,6 +168,7 @@ describe('FrontendConfigService', () => { rendererOrigin: domain, port: 3000, loglevel: Loglevel.ERROR, + persistInterval: 10, }; const authConfig: AuthConfig = { ...emptyAuthConfig, @@ -322,6 +323,7 @@ describe('FrontendConfigService', () => { rendererOrigin: renderOrigin ?? domain, port: 3000, loglevel: Loglevel.ERROR, + persistInterval: 10, }; const authConfig: AuthConfig = { ...emptyAuthConfig, diff --git a/src/main.ts b/src/main.ts index 7f25ef785..61c2e3121 100644 --- a/src/main.ts +++ b/src/main.ts @@ -42,6 +42,13 @@ async function bootstrap(): Promise { // Setup code must be added there! await setupApp(app, appConfig, authConfig, mediaConfig, logger); + /** + * enableShutdownHooks consumes memory by starting listeners. In cases where you are running multiple Nest apps in a + * single Node process (e.g., when running parallel tests with Jest), Node may complain about excessive listener processes. + * For this reason, enableShutdownHooks is not enabled in the tests. + */ + app.enableShutdownHooks(); + // Start the server await app.listen(appConfig.port); logger.log(`Listening on port ${appConfig.port}`, 'AppBootstrap'); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index b01e24a5a..c8b75366c 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -16,6 +16,7 @@ import { import { GroupsService } from '../groups/groups.service'; import { HistoryEntry } from '../history/history-entry.entity'; import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { RealtimeNoteStore } from '../realtime/realtime-note/realtime-note-store.service'; import { RealtimeNoteService } from '../realtime/realtime-note/realtime-note.service'; import { Revision } from '../revisions/revision.entity'; import { RevisionsService } from '../revisions/revisions.service'; @@ -45,6 +46,7 @@ export class NotesService { private noteConfig: NoteConfig, @Inject(forwardRef(() => AliasService)) private aliasService: AliasService, private realtimeNoteService: RealtimeNoteService, + private realtimeNoteStore: RealtimeNoteStore, ) { this.logger.setContext(NotesService.name); } @@ -119,10 +121,7 @@ export class NotesService { */ async getNoteContent(note: Note): Promise { return ( - this.realtimeNoteService - .getRealtimeNote(note.id) - ?.getYDoc() - .getCurrentContent() ?? + this.realtimeNoteStore.find(note.id)?.getYDoc().getCurrentContent() ?? (await this.revisionsService.getLatestRevision(note)).content ); } diff --git a/src/realtime/realtime-note/realtime-note-store.service.ts b/src/realtime/realtime-note/realtime-note-store.service.ts new file mode 100644 index 000000000..2b69b326a --- /dev/null +++ b/src/realtime/realtime-note/realtime-note-store.service.ts @@ -0,0 +1,50 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Injectable } from '@nestjs/common'; + +import { Note } from '../../notes/note.entity'; +import { RealtimeNote } from './realtime-note'; + +@Injectable() +export class RealtimeNoteStore { + private noteIdToRealtimeNote = new Map(); + + /** + * Creates a new {@link RealtimeNote} for the given {@link Note} and memorizes it. + * + * @param note The note for which the realtime note should be created + * @param initialContent The initial content for the realtime note + * @throws Error if there is already an realtime note for the given note. + * @return The created realtime note + */ + public create(note: Note, initialContent: string): RealtimeNote { + if (this.noteIdToRealtimeNote.has(note.id)) { + throw new Error(`Realtime note for note ${note.id} already exists.`); + } + const realtimeNote = new RealtimeNote(note, initialContent); + realtimeNote.on('destroy', () => { + this.noteIdToRealtimeNote.delete(note.id); + }); + this.noteIdToRealtimeNote.set(note.id, realtimeNote); + return realtimeNote; + } + + /** + * Retrieves a {@link RealtimeNote} that is linked to the given {@link Note} id. + * @param noteId The id of the {@link Note} + * @return A {@link RealtimeNote} or {@code undefined} if no instance is existing. + */ + public find(noteId: string): RealtimeNote | undefined { + return this.noteIdToRealtimeNote.get(noteId); + } + + /** + * Returns all registered {@link RealtimeNote realtime notes}. + */ + public getAllRealtimeNotes(): RealtimeNote[] { + return [...this.noteIdToRealtimeNote.values()]; + } +} diff --git a/src/realtime/realtime-note/realtime-note-store.spec.ts b/src/realtime/realtime-note/realtime-note-store.spec.ts new file mode 100644 index 000000000..e7caaed20 --- /dev/null +++ b/src/realtime/realtime-note/realtime-note-store.spec.ts @@ -0,0 +1,70 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Mock } from 'ts-mockery'; + +import { Note } from '../../notes/note.entity'; +import * as realtimeNoteModule from './realtime-note'; +import { RealtimeNote } from './realtime-note'; +import { RealtimeNoteStore } from './realtime-note-store.service'; +import { mockRealtimeNote } from './test-utils/mock-realtime-note'; +import { WebsocketAwareness } from './websocket-awareness'; +import { WebsocketDoc } from './websocket-doc'; + +describe('RealtimeNoteStore', () => { + let realtimeNoteStore: RealtimeNoteStore; + let mockedNote: Note; + let mockedRealtimeNote: RealtimeNote; + let realtimeNoteConstructorSpy: jest.SpyInstance; + const mockedContent = 'mockedContent'; + const mockedNoteId = 'mockedNoteId'; + + beforeEach(async () => { + jest.resetAllMocks(); + jest.resetModules(); + + realtimeNoteStore = new RealtimeNoteStore(); + + mockedNote = Mock.of({ id: mockedNoteId }); + mockedRealtimeNote = mockRealtimeNote( + mockedNote, + Mock.of(), + Mock.of(), + ); + realtimeNoteConstructorSpy = jest + .spyOn(realtimeNoteModule, 'RealtimeNote') + .mockReturnValue(mockedRealtimeNote); + }); + + it("can create a new realtime note if it doesn't exist yet", () => { + expect(realtimeNoteStore.create(mockedNote, mockedContent)).toBe( + mockedRealtimeNote, + ); + expect(realtimeNoteConstructorSpy).toBeCalledWith( + mockedNote, + mockedContent, + ); + expect(realtimeNoteStore.find(mockedNoteId)).toBe(mockedRealtimeNote); + expect(realtimeNoteStore.getAllRealtimeNotes()).toStrictEqual([ + mockedRealtimeNote, + ]); + }); + + it('throws if a realtime note has already been created for the given note', () => { + expect(realtimeNoteStore.create(mockedNote, mockedContent)).toBe( + mockedRealtimeNote, + ); + expect(() => realtimeNoteStore.create(mockedNote, mockedContent)).toThrow(); + }); + + it('deletes a note if it gets destroyed', () => { + expect(realtimeNoteStore.create(mockedNote, mockedContent)).toBe( + mockedRealtimeNote, + ); + mockedRealtimeNote.emit('destroy'); + expect(realtimeNoteStore.find(mockedNoteId)).toBe(undefined); + expect(realtimeNoteStore.getAllRealtimeNotes()).toStrictEqual([]); + }); +}); diff --git a/src/realtime/realtime-note/realtime-note.module.ts b/src/realtime/realtime-note/realtime-note.module.ts index e34494570..903cf9d95 100644 --- a/src/realtime/realtime-note/realtime-note.module.ts +++ b/src/realtime/realtime-note/realtime-note.module.ts @@ -4,12 +4,14 @@ * SPDX-License-Identifier: AGPL-3.0-only */ import { Module } from '@nestjs/common'; +import { ScheduleModule } from '@nestjs/schedule'; import { LoggerModule } from '../../logger/logger.module'; import { PermissionsModule } from '../../permissions/permissions.module'; import { RevisionsModule } from '../../revisions/revisions.module'; import { SessionModule } from '../../session/session.module'; import { UsersModule } from '../../users/users.module'; +import { RealtimeNoteStore } from './realtime-note-store.service'; import { RealtimeNoteService } from './realtime-note.service'; @Module({ @@ -19,8 +21,9 @@ import { RealtimeNoteService } from './realtime-note.service'; PermissionsModule, SessionModule, RevisionsModule, + ScheduleModule.forRoot(), ], - exports: [RealtimeNoteService], - providers: [RealtimeNoteService], + exports: [RealtimeNoteService, RealtimeNoteStore], + providers: [RealtimeNoteService, RealtimeNoteStore], }) export class RealtimeNoteModule {} diff --git a/src/realtime/realtime-note/realtime-note.service.spec.ts b/src/realtime/realtime-note/realtime-note.service.spec.ts index 08142fb7a..6aab09dbf 100644 --- a/src/realtime/realtime-note/realtime-note.service.spec.ts +++ b/src/realtime/realtime-note/realtime-note.service.spec.ts @@ -3,26 +3,38 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ +import { SchedulerRegistry } from '@nestjs/schedule'; import { Mock } from 'ts-mockery'; +import { AppConfig } from '../../config/app.config'; +import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { Note } from '../../notes/note.entity'; import { Revision } from '../../revisions/revision.entity'; import { RevisionsService } from '../../revisions/revisions.service'; -import * as realtimeNoteModule from './realtime-note'; import { RealtimeNote } from './realtime-note'; +import { RealtimeNoteStore } from './realtime-note-store.service'; import { RealtimeNoteService } from './realtime-note.service'; +import { mockAwareness } from './test-utils/mock-awareness'; import { mockRealtimeNote } from './test-utils/mock-realtime-note'; -import { WebsocketAwareness } from './websocket-awareness'; +import { mockWebsocketDoc } from './test-utils/mock-websocket-doc'; +import { waitForOtherPromisesToFinish } from './test-utils/wait-for-other-promises-to-finish'; import { WebsocketDoc } from './websocket-doc'; describe('RealtimeNoteService', () => { - let realtimeNoteService: RealtimeNoteService; - let mockedNote: Note; - let mockedRealtimeNote: RealtimeNote; - let realtimeNoteConstructorSpy: jest.SpyInstance; - let revisionsService: RevisionsService; const mockedContent = 'mockedContent'; const mockedNoteId = 'mockedNoteId'; + let websocketDoc: WebsocketDoc; + let mockedNote: Note; + let mockedRealtimeNote: RealtimeNote; + let realtimeNoteService: RealtimeNoteService; + let revisionsService: RevisionsService; + let realtimeNoteStore: RealtimeNoteStore; + let consoleLoggerService: ConsoleLoggerService; + let mockedAppConfig: AppConfig; + let addIntervalSpy: jest.SpyInstance; + let setIntervalSpy: jest.SpyInstance; + let clearIntervalSpy: jest.SpyInstance; + let deleteIntervalSpy: jest.SpyInstance; function mockGetLatestRevision(latestRevisionExists: boolean) { jest @@ -42,62 +54,188 @@ describe('RealtimeNoteService', () => { jest.resetAllMocks(); jest.resetModules(); - revisionsService = Mock.of({ - getLatestRevision: jest.fn(), - }); - - realtimeNoteService = new RealtimeNoteService(revisionsService); - + websocketDoc = mockWebsocketDoc(); mockedNote = Mock.of({ id: mockedNoteId }); mockedRealtimeNote = mockRealtimeNote( - Mock.of(), - Mock.of(), + mockedNote, + websocketDoc, + mockAwareness(), + ); + + revisionsService = Mock.of({ + getLatestRevision: jest.fn(), + createRevision: jest.fn(), + }); + + consoleLoggerService = Mock.of({ + error: jest.fn(), + }); + realtimeNoteStore = Mock.of({ + find: jest.fn(), + create: jest.fn(), + getAllRealtimeNotes: jest.fn(), + }); + + mockedAppConfig = Mock.of({ persistInterval: 0 }); + + const schedulerRegistry = Mock.of({ + addInterval: jest.fn(), + deleteInterval: jest.fn(), + }); + + addIntervalSpy = jest.spyOn(schedulerRegistry, 'addInterval'); + deleteIntervalSpy = jest.spyOn(schedulerRegistry, 'deleteInterval'); + setIntervalSpy = jest.spyOn(global, 'setInterval'); + clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + + realtimeNoteService = new RealtimeNoteService( + revisionsService, + consoleLoggerService, + realtimeNoteStore, + schedulerRegistry, + mockedAppConfig, ); - realtimeNoteConstructorSpy = jest - .spyOn(realtimeNoteModule, 'RealtimeNote') - .mockReturnValue(mockedRealtimeNote); }); it("creates a new realtime note if it doesn't exist yet", async () => { mockGetLatestRevision(true); + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + mockedAppConfig.persistInterval = 0; + await expect( realtimeNoteService.getOrCreateRealtimeNote(mockedNote), ).resolves.toBe(mockedRealtimeNote); - expect(realtimeNoteConstructorSpy).toBeCalledWith( - mockedNoteId, - mockedContent, - ); - expect(realtimeNoteService.getRealtimeNote(mockedNoteId)).toBe( - mockedRealtimeNote, - ); + + expect(realtimeNoteStore.find).toBeCalledWith(mockedNoteId); + expect(realtimeNoteStore.create).toBeCalledWith(mockedNote, mockedContent); + expect(setIntervalSpy).not.toBeCalled(); + }); + + describe('with periodic timer', () => { + it('starts a timer if config has set an interval', async () => { + mockGetLatestRevision(true); + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + mockedAppConfig.persistInterval = 10; + + await realtimeNoteService.getOrCreateRealtimeNote(mockedNote); + + expect(setIntervalSpy).toBeCalledWith( + expect.any(Function), + 1000 * 60 * 10, + ); + expect(addIntervalSpy).toBeCalled(); + }); + + it('stops the timer if the realtime note gets destroyed', async () => { + mockGetLatestRevision(true); + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + mockedAppConfig.persistInterval = 10; + + await realtimeNoteService.getOrCreateRealtimeNote(mockedNote); + mockedRealtimeNote.emit('destroy'); + expect(deleteIntervalSpy).toBeCalled(); + expect(clearIntervalSpy).toBeCalled(); + }); }); it("fails if the requested note doesn't exist", async () => { mockGetLatestRevision(false); + + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + await expect( realtimeNoteService.getOrCreateRealtimeNote(mockedNote), ).rejects.toBe(`Revision for note mockedNoteId not found.`); - expect(realtimeNoteConstructorSpy).not.toBeCalled(); - expect(realtimeNoteService.getRealtimeNote(mockedNoteId)).toBeUndefined(); + expect(realtimeNoteStore.create).not.toBeCalled(); + expect(realtimeNoteStore.find).toBeCalledWith(mockedNoteId); }); it("doesn't create a new realtime note if there is already one", async () => { mockGetLatestRevision(true); + + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + await expect( realtimeNoteService.getOrCreateRealtimeNote(mockedNote), ).resolves.toBe(mockedRealtimeNote); + + jest + .spyOn(realtimeNoteStore, 'find') + .mockImplementation(() => mockedRealtimeNote); + await expect( realtimeNoteService.getOrCreateRealtimeNote(mockedNote), ).resolves.toBe(mockedRealtimeNote); - expect(realtimeNoteConstructorSpy).toBeCalledTimes(1); + expect(realtimeNoteStore.create).toBeCalledTimes(1); }); - it('deletes the realtime from the map if the realtime note is destroyed', async () => { + it('saves a realtime note if it gets destroyed', async () => { mockGetLatestRevision(true); - await expect( - realtimeNoteService.getOrCreateRealtimeNote(mockedNote), - ).resolves.toBe(mockedRealtimeNote); - mockedRealtimeNote.emit('destroy'); - expect(realtimeNoteService.getRealtimeNote(mockedNoteId)).toBeUndefined(); + const mockedCurrentContent = 'mockedCurrentContent'; + + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + jest + .spyOn(websocketDoc, 'getCurrentContent') + .mockReturnValue(mockedCurrentContent); + + await realtimeNoteService.getOrCreateRealtimeNote(mockedNote); + + const createRevisionSpy = jest + .spyOn(revisionsService, 'createRevision') + .mockImplementation(() => Promise.resolve(Mock.of())); + + mockedRealtimeNote.emit('beforeDestroy'); + expect(createRevisionSpy).toBeCalledWith(mockedNote, mockedCurrentContent); + }); + + it('logs errors that occur during saving on destroy', async () => { + mockGetLatestRevision(true); + const mockedCurrentContent = 'mockedCurrentContent'; + + jest.spyOn(realtimeNoteStore, 'find').mockImplementation(() => undefined); + jest + .spyOn(realtimeNoteStore, 'create') + .mockImplementation(() => mockedRealtimeNote); + jest + .spyOn(websocketDoc, 'getCurrentContent') + .mockReturnValue(mockedCurrentContent); + jest + .spyOn(revisionsService, 'createRevision') + .mockImplementation(() => Promise.reject('mocked error')); + + const logSpy = jest.spyOn(consoleLoggerService, 'error'); + + await realtimeNoteService.getOrCreateRealtimeNote(mockedNote); + mockedRealtimeNote.emit('beforeDestroy'); + + await waitForOtherPromisesToFinish(); + expect(logSpy).toBeCalled(); + }); + + it('destroys every realtime note on application shutdown', () => { + jest + .spyOn(realtimeNoteStore, 'getAllRealtimeNotes') + .mockReturnValue([mockedRealtimeNote]); + + const destroySpy = jest.spyOn(mockedRealtimeNote, 'destroy'); + + realtimeNoteService.beforeApplicationShutdown(); + + expect(destroySpy).toBeCalled(); }); }); diff --git a/src/realtime/realtime-note/realtime-note.service.ts b/src/realtime/realtime-note/realtime-note.service.ts index d63b5ac92..e3d223751 100644 --- a/src/realtime/realtime-note/realtime-note.service.ts +++ b/src/realtime/realtime-note/realtime-note.service.ts @@ -3,17 +3,47 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { Injectable } from '@nestjs/common'; +import { Optional } from '@mrdrogdrog/optional'; +import { BeforeApplicationShutdown, Inject, Injectable } from '@nestjs/common'; +import { SchedulerRegistry } from '@nestjs/schedule'; +import appConfiguration, { AppConfig } from '../../config/app.config'; +import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { Note } from '../../notes/note.entity'; import { RevisionsService } from '../../revisions/revisions.service'; import { RealtimeNote } from './realtime-note'; +import { RealtimeNoteStore } from './realtime-note-store.service'; @Injectable() -export class RealtimeNoteService { - constructor(private revisionsService: RevisionsService) {} +export class RealtimeNoteService implements BeforeApplicationShutdown { + constructor( + private revisionsService: RevisionsService, + private readonly logger: ConsoleLoggerService, + private realtimeNoteStore: RealtimeNoteStore, + private schedulerRegistry: SchedulerRegistry, + @Inject(appConfiguration.KEY) + private appConfig: AppConfig, + ) {} - private noteIdToRealtimeNote = new Map(); + beforeApplicationShutdown(): void { + this.realtimeNoteStore + .getAllRealtimeNotes() + .forEach((realtimeNote) => realtimeNote.destroy()); + } + + /** + * Reads the current content from the given {@link RealtimeNote} and creates a new {@link Revision} for the linked {@link Note}. + * + * @param realtimeNote The realtime note for which a revision should be created + */ + public saveRealtimeNote(realtimeNote: RealtimeNote): void { + this.revisionsService + .createRevision( + realtimeNote.getNote(), + realtimeNote.getYDoc().getCurrentContent(), + ) + .catch((reason) => this.logger.error(reason)); + } /** * Creates or reuses a {@link RealtimeNote} that is handling the real time editing of the {@link Note} which is identified by the given note id. @@ -23,13 +53,13 @@ export class RealtimeNoteService { */ public async getOrCreateRealtimeNote(note: Note): Promise { return ( - this.noteIdToRealtimeNote.get(note.id) ?? + this.realtimeNoteStore.find(note.id) ?? (await this.createNewRealtimeNote(note)) ); } /** - * Creates a new {@link RealtimeNote} for the given {@link Note} and memorizes it. + * Creates a new {@link RealtimeNote} for the given {@link Note}. * * @param note The note for which the realtime note should be created * @throws NotInDBError if note doesn't exist or has no revisions. @@ -38,20 +68,37 @@ export class RealtimeNoteService { private async createNewRealtimeNote(note: Note): Promise { const initialContent = (await this.revisionsService.getLatestRevision(note)) .content; - const realtimeNote = new RealtimeNote(note.id, initialContent); - realtimeNote.on('destroy', () => { - this.noteIdToRealtimeNote.delete(note.id); + const realtimeNote = this.realtimeNoteStore.create(note, initialContent); + realtimeNote.on('beforeDestroy', () => { + this.saveRealtimeNote(realtimeNote); }); - this.noteIdToRealtimeNote.set(note.id, realtimeNote); + this.startPersistTimer(realtimeNote); return realtimeNote; } /** - * Retrieves a {@link RealtimeNote} that is linked to the given {@link Note} id. - * @param noteId The id of the {@link Note} - * @return A {@link RealtimeNote} or {@code undefined} if no instance is existing. + * Starts a timer that persists the realtime note in a periodic interval depending on the {@link AppConfig}. + + * @param realtimeNote The realtime note for which the timer should be started */ - public getRealtimeNote(noteId: string): RealtimeNote | undefined { - return this.noteIdToRealtimeNote.get(noteId); + private startPersistTimer(realtimeNote: RealtimeNote): void { + Optional.of(this.appConfig.persistInterval) + .filter((value) => value > 0) + .ifPresent((persistInterval) => { + const intervalId = setInterval( + this.saveRealtimeNote.bind(this, realtimeNote), + persistInterval * 60 * 1000, + ); + this.schedulerRegistry.addInterval( + `periodic-persist-${realtimeNote.getNote().id}`, + intervalId, + ); + realtimeNote.on('destroy', () => { + clearInterval(intervalId); + this.schedulerRegistry.deleteInterval( + `periodic-persist-${realtimeNote.getNote().id}`, + ); + }); + }); } } diff --git a/src/realtime/realtime-note/realtime-note.spec.ts b/src/realtime/realtime-note/realtime-note.spec.ts index b4396d43b..4c31d36f5 100644 --- a/src/realtime/realtime-note/realtime-note.spec.ts +++ b/src/realtime/realtime-note/realtime-note.spec.ts @@ -3,6 +3,9 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ +import { Mock } from 'ts-mockery'; + +import { Note } from '../../notes/note.entity'; import { RealtimeNote } from './realtime-note'; import { mockAwareness } from './test-utils/mock-awareness'; import { mockConnection } from './test-utils/mock-connection'; @@ -15,6 +18,7 @@ import { WebsocketDoc } from './websocket-doc'; describe('realtime note', () => { let mockedDoc: WebsocketDoc; let mockedAwareness: WebsocketAwareness; + let mockedNote: Note; beforeEach(() => { jest.resetAllMocks(); @@ -27,6 +31,8 @@ describe('realtime note', () => { jest .spyOn(websocketAwarenessModule, 'WebsocketAwareness') .mockImplementation(() => mockedAwareness); + + mockedNote = Mock.of({ id: 'mock-note' }); }); afterAll(() => { @@ -34,8 +40,13 @@ describe('realtime note', () => { jest.resetModules(); }); + it('can return the given note', () => { + const sut = new RealtimeNote(mockedNote, 'nothing'); + expect(sut.getNote()).toBe(mockedNote); + }); + it('can connect and disconnect clients', () => { - const sut = new RealtimeNote('mock-note', 'nothing'); + const sut = new RealtimeNote(mockedNote, 'nothing'); const client1 = mockConnection(true); sut.addClient(client1); expect(sut.getConnections()).toStrictEqual([client1]); @@ -46,13 +57,13 @@ describe('realtime note', () => { }); it('creates a y-doc and y-awareness', () => { - const sut = new RealtimeNote('mock-note', 'nothing'); + const sut = new RealtimeNote(mockedNote, 'nothing'); expect(sut.getYDoc()).toBe(mockedDoc); expect(sut.getAwareness()).toBe(mockedAwareness); }); it('destroys y-doc and y-awareness on self-destruction', () => { - const sut = new RealtimeNote('mock-note', 'nothing'); + const sut = new RealtimeNote(mockedNote, 'nothing'); const docDestroy = jest.spyOn(mockedDoc, 'destroy'); const awarenessDestroy = jest.spyOn(mockedAwareness, 'destroy'); sut.destroy(); @@ -61,7 +72,7 @@ describe('realtime note', () => { }); it('emits destroy event on destruction', async () => { - const sut = new RealtimeNote('mock-note', 'nothing'); + const sut = new RealtimeNote(mockedNote, 'nothing'); const destroyPromise = new Promise((resolve) => { sut.once('destroy', () => { resolve(); @@ -72,7 +83,7 @@ describe('realtime note', () => { }); it("doesn't destroy a destroyed note", () => { - const sut = new RealtimeNote('mock-note', 'nothing'); + const sut = new RealtimeNote(mockedNote, 'nothing'); sut.destroy(); expect(() => sut.destroy()).toThrow(); }); diff --git a/src/realtime/realtime-note/realtime-note.ts b/src/realtime/realtime-note/realtime-note.ts index 0c6b54ef6..9820760ce 100644 --- a/src/realtime/realtime-note/realtime-note.ts +++ b/src/realtime/realtime-note/realtime-note.ts @@ -8,11 +8,13 @@ import { EventEmitter } from 'events'; import TypedEventEmitter, { EventMap } from 'typed-emitter'; import { Awareness } from 'y-protocols/awareness'; +import { Note } from '../../notes/note.entity'; import { WebsocketAwareness } from './websocket-awareness'; import { WebsocketConnection } from './websocket-connection'; import { WebsocketDoc } from './websocket-doc'; export type RealtimeNoteEvents = { + beforeDestroy: () => void; destroy: () => void; }; @@ -29,12 +31,12 @@ export class RealtimeNote extends (EventEmitter as TypedEventEmitterConstructor< private readonly clients = new Set(); private isClosing = false; - constructor(private readonly noteId: string, initialContent: string) { + constructor(private readonly note: Note, initialContent: string) { super(); - this.logger = new Logger(`${RealtimeNote.name} ${noteId}`); + this.logger = new Logger(`${RealtimeNote.name} ${note.id}`); this.websocketDoc = new WebsocketDoc(this, initialContent); this.websocketAwareness = new WebsocketAwareness(this); - this.logger.debug(`New realtime session for note ${noteId} created.`); + this.logger.debug(`New realtime session for note ${note.id} created.`); } /** @@ -76,6 +78,7 @@ export class RealtimeNote extends (EventEmitter as TypedEventEmitterConstructor< throw new Error('Note already destroyed'); } this.logger.debug('Destroying realtime note.'); + this.emit('beforeDestroy'); this.isClosing = true; this.websocketDoc.destroy(); this.websocketAwareness.destroy(); @@ -118,4 +121,13 @@ export class RealtimeNote extends (EventEmitter as TypedEventEmitterConstructor< public getAwareness(): Awareness { return this.websocketAwareness; } + + /** + * Get the {@link Note note} that is edited. + * + * @return the {@link Note note} + */ + public getNote(): Note { + return this.note; + } } diff --git a/src/realtime/realtime-note/test-utils/mock-realtime-note.ts b/src/realtime/realtime-note/test-utils/mock-realtime-note.ts index 0374ada40..0c035f94c 100644 --- a/src/realtime/realtime-note/test-utils/mock-realtime-note.ts +++ b/src/realtime/realtime-note/test-utils/mock-realtime-note.ts @@ -7,18 +7,26 @@ import { EventEmitter } from 'events'; import { Mock } from 'ts-mockery'; import TypedEmitter from 'typed-emitter'; +import { Note } from '../../../notes/note.entity'; import { RealtimeNote, RealtimeNoteEvents } from '../realtime-note'; import { WebsocketAwareness } from '../websocket-awareness'; import { WebsocketDoc } from '../websocket-doc'; +import { mockAwareness } from './mock-awareness'; +import { mockWebsocketDoc } from './mock-websocket-doc'; class MockRealtimeNote extends (EventEmitter as new () => TypedEmitter) { constructor( + private note: Note, private doc: WebsocketDoc, private awareness: WebsocketAwareness, ) { super(); } + public getNote(): Note { + return this.note; + } + public getYDoc(): WebsocketDoc { return this.doc; } @@ -30,6 +38,10 @@ class MockRealtimeNote extends (EventEmitter as new () => TypedEmitter TypedEmitter(new MockRealtimeNote(doc, awareness)); + return Mock.from( + new MockRealtimeNote( + note ?? Mock.of(), + doc ?? mockWebsocketDoc(), + awareness ?? mockAwareness(), + ), + ); } diff --git a/src/realtime/realtime-note/test-utils/mock-websocket-doc.ts b/src/realtime/realtime-note/test-utils/mock-websocket-doc.ts index 820e8de1c..1122c56f8 100644 --- a/src/realtime/realtime-note/test-utils/mock-websocket-doc.ts +++ b/src/realtime/realtime-note/test-utils/mock-websocket-doc.ts @@ -14,5 +14,6 @@ export function mockWebsocketDoc(): WebsocketDoc { return Mock.of({ on: jest.fn(), destroy: jest.fn(), + getCurrentContent: jest.fn(), }); } diff --git a/src/realtime/realtime-note/test-utils/wait-for-other-promises-to-finish.ts b/src/realtime/realtime-note/test-utils/wait-for-other-promises-to-finish.ts new file mode 100644 index 000000000..3886a1d8b --- /dev/null +++ b/src/realtime/realtime-note/test-utils/wait-for-other-promises-to-finish.ts @@ -0,0 +1,12 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/** + * Waits until all other pending promises are processed. + */ +export async function waitForOtherPromisesToFinish(): Promise { + return await new Promise((resolve) => process.nextTick(resolve)); +} diff --git a/src/realtime/realtime-note/websocket-connection.spec.ts b/src/realtime/realtime-note/websocket-connection.spec.ts index 23d9c816f..19d1bcea4 100644 --- a/src/realtime/realtime-note/websocket-connection.spec.ts +++ b/src/realtime/realtime-note/websocket-connection.spec.ts @@ -9,6 +9,7 @@ import { Mock } from 'ts-mockery'; import WebSocket from 'ws'; import * as yProtocolsAwarenessModule from 'y-protocols/awareness'; +import { Note } from '../../notes/note.entity'; import { User } from '../../users/user.entity'; import * as realtimeNoteModule from './realtime-note'; import { RealtimeNote } from './realtime-note'; @@ -38,7 +39,11 @@ describe('websocket connection', () => { jest.resetModules(); mockedDoc = mockWebsocketDoc(); mockedAwareness = mockAwareness(); - mockedRealtimeNote = mockRealtimeNote(mockedDoc, mockedAwareness); + mockedRealtimeNote = mockRealtimeNote( + Mock.of(), + mockedDoc, + mockedAwareness, + ); mockedWebsocket = Mock.of({}); mockedUser = Mock.of({}); mockedWebsocketTransporter = mockWebsocketTransporter(); diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts index e78590a88..6fc742cf3 100644 --- a/src/revisions/revisions.service.spec.ts +++ b/src/revisions/revisions.service.spec.ts @@ -6,6 +6,7 @@ import { ConfigModule } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { Mock } from 'ts-mockery'; import { Repository } from 'typeorm'; import { AuthToken } from '../auth/auth-token.entity'; @@ -99,7 +100,7 @@ describe('RevisionsService', () => { describe('getRevision', () => { it('returns a revision', async () => { - const note = {} as Note; + const note = Mock.of({}); const revision = Revision.create('', '', note) as Revision; jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revision); expect(await service.getRevision({} as Note, 1)).toEqual(revision); @@ -192,4 +193,48 @@ describe('RevisionsService', () => { expect(userInfo.anonymousUserCount).toEqual(2); }); }); + + describe('createRevision', () => { + it('creates a new revision', async () => { + const note = Mock.of({}); + const oldContent = 'old content\n'; + const newContent = 'new content\n'; + + const oldRevision = Mock.of({ content: oldContent }); + jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(oldRevision); + jest + .spyOn(revisionRepo, 'save') + .mockImplementation((revision) => + Promise.resolve(revision as Revision), + ); + + const createdRevision = await service.createRevision(note, newContent); + expect(createdRevision).not.toBeUndefined(); + expect(createdRevision?.content).toBe(newContent); + await expect(createdRevision?.note).resolves.toBe(note); + expect(createdRevision?.patch).toMatchInlineSnapshot(` + "Index: markdownContent + =================================================================== + --- markdownContent + +++ markdownContent + @@ -1,1 +1,1 @@ + -old content + +new content + " + `); + }); + + it("won't create a revision if content is unchanged", async () => { + const note = Mock.of({}); + const oldContent = 'old content\n'; + + const oldRevision = Mock.of({ content: oldContent }); + jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(oldRevision); + const saveSpy = jest.spyOn(revisionRepo, 'save').mockImplementation(); + + const createdRevision = await service.createRevision(note, oldContent); + expect(createdRevision).toBeUndefined(); + expect(saveSpy).not.toBeCalled(); + }); + }); }); diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts index 577c558b3..77bbaf0ae 100644 --- a/src/revisions/revisions.service.ts +++ b/src/revisions/revisions.service.ts @@ -1,10 +1,11 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; +import { createPatch } from 'diff'; import { Equal, Repository } from 'typeorm'; import { NotInDBError } from '../errors/errors'; @@ -51,10 +52,10 @@ export class RevisionsService { note: Equal(note), }, }); - const latestRevison = await this.getLatestRevision(note); + const latestRevision = await this.getLatestRevision(note); // get all revisions except the latest const oldRevisions = revisions.filter( - (item) => item.id !== latestRevison.id, + (item) => item.id !== latestRevision.id, ); // delete the old revisions return await this.revisionRepository.remove(oldRevisions); @@ -91,21 +92,6 @@ export class RevisionsService { return revision; } - async getFirstRevision(note: Note): Promise { - const revision = await this.revisionRepository.findOne({ - where: { - note: Equal(note), - }, - order: { - createdAt: 'ASC', - }, - }); - if (revision === null) { - throw new NotInDBError(`Revision for note ${note.id} not found.`); - } - return revision; - } - async getRevisionUserInfo(revision: Revision): Promise { // get a deduplicated list of all authors let authors = await Promise.all( @@ -156,14 +142,22 @@ export class RevisionsService { }; } - createRevision(content: string): Revision { - // TODO: Add previous revision - // TODO: Calculate patch + async createRevision( + note: Note, + newContent: string, + ): Promise { // TODO: Save metadata - return this.revisionRepository.create({ - content: content, - length: content.length, - patch: '', - }); + const latestRevision = await this.getLatestRevision(note); + const oldContent = latestRevision.content; + if (oldContent === newContent) { + return undefined; + } + const patch = createPatch( + 'markdownContent', + latestRevision.content, + newContent, + ); + const revision = Revision.create(newContent, patch, note); + return await this.revisionRepository.save(revision); } } diff --git a/yarn.lock b/yarn.lock index 8f6ae9935..f5b1bb480 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1659,6 +1659,13 @@ __metadata: languageName: node linkType: hard +"@types/diff@npm:5.0.2": + version: 5.0.2 + resolution: "@types/diff@npm:5.0.2" + checksum: 8fbc419b5aca33f494026bf5f70e026f76367689677ef114f9c078ac738d7dbe96e6dda3fd8290e4a7c35281e2b60b034e3d7e3c968b850cf06a21279e7ddcbe + languageName: node + linkType: hard + "@types/eslint-scope@npm:^3.7.3": version: 3.7.4 resolution: "@types/eslint-scope@npm:3.7.4" @@ -3959,6 +3966,13 @@ __metadata: languageName: node linkType: hard +"diff@npm:5.1.0": + version: 5.1.0 + resolution: "diff@npm:5.1.0" + checksum: c7bf0df7c9bfbe1cf8a678fd1b2137c4fb11be117a67bc18a0e03ae75105e8533dbfb1cda6b46beb3586ef5aed22143ef9d70713977d5fb1f9114e21455fba90 + languageName: node + linkType: hard + "diff@npm:^4.0.1": version: 4.0.2 resolution: "diff@npm:4.0.2" @@ -5347,6 +5361,7 @@ __metadata: "@types/cookie": 0.5.1 "@types/cookie-signature": 1.0.4 "@types/cron": 2.0.0 + "@types/diff": 5.0.2 "@types/express": 4.17.13 "@types/express-session": 1.17.5 "@types/jest": 28.1.6 @@ -5369,6 +5384,7 @@ __metadata: cli-color: 2.0.3 connect-typeorm: 2.0.0 cookie: 0.5.0 + diff: 5.1.0 eslint: 8.21.0 eslint-config-prettier: 8.5.0 eslint-plugin-import: 2.26.0