feat: persist notes on realtime note unload and on interval

The realtime note map has been moved into its own class
to separate the realtime note business logic from the storing logic.

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>
This commit is contained in:
Tilman Vatteroth 2022-07-22 23:05:38 +02:00 committed by Yannick Bungers
parent 49b4d2e070
commit 4746c30c26
22 changed files with 619 additions and 102 deletions

View file

@ -20,13 +20,14 @@ 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. |
| `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?

View file

@ -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",

View file

@ -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();
});
});
});

View file

@ -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,

View file

@ -15,5 +15,6 @@ export default registerAs(
rendererOrigin: 'md-renderer.example.com',
port: 3000,
loglevel: Loglevel.ERROR,
persistInterval: 10,
}),
);

View file

@ -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,

View file

@ -42,6 +42,13 @@ async function bootstrap(): Promise<void> {
// 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');

View file

@ -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<string> {
return (
this.realtimeNoteService
.getRealtimeNote(note.id)
?.getYDoc()
.getCurrentContent() ??
this.realtimeNoteStore.find(note.id)?.getYDoc().getCurrentContent() ??
(await this.revisionsService.getLatestRevision(note)).content
);
}

View file

@ -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<string, RealtimeNote>();
/**
* 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()];
}
}

View file

@ -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<Note>({ id: mockedNoteId });
mockedRealtimeNote = mockRealtimeNote(
mockedNote,
Mock.of<WebsocketDoc>(),
Mock.of<WebsocketAwareness>(),
);
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([]);
});
});

View file

@ -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 {}

View file

@ -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<RevisionsService>({
getLatestRevision: jest.fn(),
});
realtimeNoteService = new RealtimeNoteService(revisionsService);
websocketDoc = mockWebsocketDoc();
mockedNote = Mock.of<Note>({ id: mockedNoteId });
mockedRealtimeNote = mockRealtimeNote(
Mock.of<WebsocketDoc>(),
Mock.of<WebsocketAwareness>(),
mockedNote,
websocketDoc,
mockAwareness(),
);
revisionsService = Mock.of<RevisionsService>({
getLatestRevision: jest.fn(),
createRevision: jest.fn(),
});
consoleLoggerService = Mock.of<ConsoleLoggerService>({
error: jest.fn(),
});
realtimeNoteStore = Mock.of<RealtimeNoteStore>({
find: jest.fn(),
create: jest.fn(),
getAllRealtimeNotes: jest.fn(),
});
mockedAppConfig = Mock.of<AppConfig>({ persistInterval: 0 });
const schedulerRegistry = Mock.of<SchedulerRegistry>({
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<Revision>()));
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();
});
});

View file

@ -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<string, RealtimeNote>();
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<RealtimeNote> {
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<RealtimeNote> {
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}`,
);
});
});
}
}

View file

@ -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<Note>({ 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<void>((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();
});

View file

@ -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<WebsocketConnection>();
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;
}
}

View file

@ -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<RealtimeNoteEvents>) {
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<RealtimeN
public removeClient(): void {
//left blank for mock
}
public destroy(): void {
//left blank for mock
}
}
/**
@ -38,8 +50,15 @@ class MockRealtimeNote extends (EventEmitter as new () => TypedEmitter<RealtimeN
* @param awareness Defines the return value for `getAwareness`
*/
export function mockRealtimeNote(
doc: WebsocketDoc,
awareness: WebsocketAwareness,
note?: Note,
doc?: WebsocketDoc,
awareness?: WebsocketAwareness,
): RealtimeNote {
return Mock.from<RealtimeNote>(new MockRealtimeNote(doc, awareness));
return Mock.from<RealtimeNote>(
new MockRealtimeNote(
note ?? Mock.of<Note>(),
doc ?? mockWebsocketDoc(),
awareness ?? mockAwareness(),
),
);
}

View file

@ -14,5 +14,6 @@ export function mockWebsocketDoc(): WebsocketDoc {
return Mock.of<WebsocketDoc>({
on: jest.fn(),
destroy: jest.fn(),
getCurrentContent: jest.fn(),
});
}

View file

@ -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<void> {
return await new Promise((resolve) => process.nextTick(resolve));
}

View file

@ -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<Note>(),
mockedDoc,
mockedAwareness,
);
mockedWebsocket = Mock.of<WebSocket>({});
mockedUser = Mock.of<User>({});
mockedWebsocketTransporter = mockWebsocketTransporter();

View file

@ -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<Note>({});
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<Note>({});
const oldContent = 'old content\n';
const newContent = 'new content\n';
const oldRevision = Mock.of<Revision>({ 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<Note>({});
const oldContent = 'old content\n';
const oldRevision = Mock.of<Revision>({ 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();
});
});
});

View file

@ -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<Revision> {
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<RevisionUserInfo> {
// 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<Revision | undefined> {
// 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);
}
}

View file

@ -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