Code improvements according to PR comments in review.

apps/server/src/modules/tldraw/config.ts

@@ -1,30 +1,29 @@
 import { Configuration } from '@hpi-schul-cloud/commons';
-import { NodeEnvType } from '@src/modules/server';
 
 export interface TldrawConfig {
 	NEST_LOG_LEVEL: string;
 	INCOMING_REQUEST_TIMEOUT: number;
-	NODE_ENV: string;
 	TLDRAW_DB_COLLECTION_NAME: string;
 	TLDRAW_DB_FLUSH_SIZE: string;
 	TLDRAW_DB_MULTIPLE_COLLECTIONS: boolean;
 	CONNECTION_STRING: string;
 	FEATURE_TLDRAW_ENABLED: boolean;
 	TLDRAW_PING_TIMEOUT: number;
+	TLDRAW_GC_ENABLED: number;
 }
 
 const tldrawConnectionString: string = Configuration.get('TLDRAW_DB_URL') as string;
 
 const tldrawConfig = {
 	NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string,
 	INCOMING_REQUEST_TIMEOUT: Configuration.get('INCOMING_REQUEST_TIMEOUT_API') as number,
-	NODE_ENV: Configuration.get('NODE_ENV') as NodeEnvType,
 	TLDRAW_DB_COLLECTION_NAME: Configuration.get('TLDRAW__DB_COLLECTION_NAME') as string,
 	TLDRAW_DB_FLUSH_SIZE: Configuration.get('TLDRAW__DB_FLUSH_SIZE') as number,
 	TLDRAW_DB_MULTIPLE_COLLECTIONS: Configuration.get('TLDRAW__DB_MULTIPLE_COLLECTIONS') as boolean,
 	FEATURE_TLDRAW_ENABLED: Configuration.get('FEATURE_TLDRAW_ENABLED') as boolean,
 	CONNECTION_STRING: tldrawConnectionString,
 	TLDRAW_PING_TIMEOUT: Configuration.get('TLDRAW__PING_TIMEOUT') as number,
+	TLDRAW_GC_ENABLED: Configuration.get('TLDRAW__GC_ENABLED') as boolean,
 };
 
 export const SOCKET_PORT = Configuration.get('TLDRAW__SOCKET_PORT') as number;

apps/server/src/modules/tldraw/controller/api-test/tldraw.ws.api.spec.ts

@@ -6,34 +6,24 @@ import { INestApplication } from '@nestjs/common';
 import { CoreModule } from '@src/core';
 import { ConfigModule } from '@nestjs/config';
 import { createConfigModuleOptions } from '@src/config';
-import { config } from '@src/modules/tldraw/config';
-import { TldrawWsService } from '@src/modules/tldraw/service';
-import { TldrawBoardRepo } from '@src/modules/tldraw/repo';
+import { config } from '../../config';
+import { TldrawWsService } from '../../service';
+import { TldrawBoardRepo } from '../../repo';
 import { TldrawWs } from '../tldraw.ws';
+import { TestHelper } from '../../helper/test-helper';
 
 describe('WebSocketController (WsAdapter)', () => {
 	let app: INestApplication;
 	let gateway: TldrawWs;
 	let ws: WebSocket;
 
 	const gatewayPort = 3346;
-	const wsUrl = `ws://localhost:${gatewayPort}`;
+	const wsUrl = TestHelper.getWsUrl(gatewayPort);
 	const testMessage =
 		'AZQBAaCbuLANBIsBeyJ0ZFVzZXIiOnsiaWQiOiJkNGIxZThmYi0yMWUwLTQ3ZDAtMDI0Y' +
 		'S0zZGEwYjMzNjQ3MjIiLCJjb2xvciI6IiNGMDRGODgiLCJwb2ludCI6WzAsMF0sInNlbGVjdGVkSWRzIjpbXSwiYWN' +
 		'0aXZlU2hhcGVzIjpbXSwic2Vzc2lvbiI6ZmFsc2V9fQ==';
 
-	const setupWs = async (docName?: string) => {
-		if (docName) {
-			ws = new WebSocket(`${wsUrl}/${docName}`);
-		} else {
-			ws = new WebSocket(`${wsUrl}`);
-		}
-		await new Promise((resolve) => {
-			ws.on('open', resolve);
-		});
-	};
-
 	const delay = (ms: number) =>
 		new Promise((resolve) => {
 			setTimeout(resolve, ms);
@@ -70,7 +60,7 @@ describe('WebSocketController (WsAdapter)', () => {
 			const handleConnectionSpy = jest.spyOn(gateway, 'handleConnection');
 			jest.spyOn(Uint8Array.prototype, 'reduce').mockReturnValueOnce(1);
 
-			await setupWs('TEST');
+			ws = await TestHelper.setupWs(wsUrl, 'TEST');
 
 			const { buffer } = getMessage();
 
@@ -84,7 +74,7 @@ describe('WebSocketController (WsAdapter)', () => {
 			expect(handleConnectionSpy).toHaveBeenCalled();
 			expect(handleConnectionSpy).toHaveBeenCalledTimes(1);
 
-			await delay(2000);
+			await delay(50);
 			ws.close();
 		});
 
@@ -98,19 +88,16 @@ describe('WebSocketController (WsAdapter)', () => {
 				});
 			});
 
-			await delay(200);
+			await delay(50);
 			ws.close();
 		});
 	});
 
 	describe('when tldraw doc has multiple clients', () => {
 		const setup = async () => {
 			const handleConnectionSpy = jest.spyOn(gateway, 'handleConnection');
-			await setupWs('TEST2');
-			const ws2 = new WebSocket(`${wsUrl}/TEST2`);
-			await new Promise((resolve) => {
-				ws2.on('open', resolve);
-			});
+			ws = await TestHelper.setupWs(wsUrl, 'TEST2');
+			const ws2 = await TestHelper.setupWs(wsUrl, 'TEST2');
 
 			const { buffer } = getMessage();
 
@@ -129,17 +116,17 @@ describe('WebSocketController (WsAdapter)', () => {
 			expect(handleConnectionSpy).toHaveBeenCalled();
 			expect(handleConnectionSpy).toHaveBeenCalledTimes(2);
 
-			await delay(200);
+			await delay(50);
 			ws.close();
 			ws2.close();
-		}, 120000);
+		}, 2000);
 	});
 
 	describe('when tldraw is not correctly setup', () => {
 		const setup = async () => {
 			const handleConnectionSpy = jest.spyOn(gateway, 'handleConnection');
 
-			await setupWs();
+			ws = await TestHelper.setupWs(wsUrl);
 
 			return {
 				handleConnectionSpy,
@@ -156,7 +143,7 @@ describe('WebSocketController (WsAdapter)', () => {
 			expect(handleConnectionSpy).toHaveBeenCalled();
 			expect(handleConnectionSpy).toHaveBeenCalledTimes(1);
 
-			await delay(200);
+			await delay(50);
 			ws.close();
 		});
 	});

apps/server/src/modules/tldraw/controller/tldraw.ws.ts

@@ -1,9 +1,9 @@
 import { WebSocketGateway, WebSocketServer, OnGatewayInit, OnGatewayConnection } from '@nestjs/websockets';
 import { Server, WebSocket } from 'ws';
 import { ConfigService } from '@nestjs/config';
-import { TldrawConfig, SOCKET_PORT } from '@src/modules/tldraw/config';
-import { WsCloseCodeEnum } from '@src/modules/tldraw/types/ws-close-code-enum';
-import { TldrawWsService } from '@src/modules/tldraw/service';
+import { TldrawConfig, SOCKET_PORT } from '../config';
+import { WsCloseCodeEnum } from '../types/ws-close-code-enum';
+import { TldrawWsService } from '../service';
 
 @WebSocketGateway(SOCKET_PORT)
 export class TldrawWs implements OnGatewayInit, OnGatewayConnection {
@@ -22,7 +22,7 @@ export class TldrawWs implements OnGatewayInit, OnGatewayConnection {
 			this.tldrawWsService.setupWSConnection(client, docName);
 		} else {
 			client.close(
-				WsCloseCodeEnum.WS_CUSTOM_CLIENT_CLOSE_CODE,
+				WsCloseCodeEnum.WS_CLIENT_BAD_REQUEST_CODE,
 				'Document name is mandatory in url or Tldraw Tool is turned off.'
 			);
 		}
@@ -42,6 +42,7 @@ export class TldrawWs implements OnGatewayInit, OnGatewayConnection {
 	}
 
 	private getDocNameFromRequest(request: Request): string {
-		return request.url.slice(1).split('?')[0].replace('tldraw-server/', '');
+		const urlStripped = request.url.replace('/', '');
+		return urlStripped;
 	}
 }

apps/server/src/modules/tldraw/domain/ws-shared-doc.do.spec.ts

@@ -5,30 +5,22 @@ import { WsAdapter } from '@nestjs/platform-ws';
 import { CoreModule } from '@src/core';
 import { ConfigModule } from '@nestjs/config';
 import { createConfigModuleOptions } from '@src/config';
-import { config } from '@src/modules/tldraw/config';
-import { TldrawBoardRepo } from '@src/modules/tldraw/repo/tldraw-board.repo';
 import { createMock } from '@golevelup/ts-jest';
-import { WsSharedDocDo } from '@src/modules/tldraw/domain/ws-shared-doc.do';
-import { TldrawWsService } from '@src/modules/tldraw/service';
 import * as AwarenessProtocol from 'y-protocols/awareness';
+import { config } from '../config';
+import { TldrawBoardRepo } from '../repo/tldraw-board.repo';
+import { TldrawWsService } from '../service';
+import { WsSharedDocDo } from './ws-shared-doc.do';
 import { TldrawWs } from '../controller';
+import { TestHelper } from '../helper/test-helper';
 
-jest.mock('../utils', () => {
-	// eslint-disable-next-line @typescript-eslint/no-unsafe-return
-	return {
-		__esModule: true,
-		...jest.requireActual('../utils'),
-		calculateDiff: jest.fn(),
-	};
-});
-
-describe('TldrawBoardRepo', () => {
+describe('WsSharedDocDo', () => {
 	let app: INestApplication;
 	let ws: WebSocket;
 	let service: TldrawWsService;
 
 	const gatewayPort = 3346;
-	const wsUrl = `ws://localhost:${gatewayPort}`;
+	const wsUrl = TestHelper.getWsUrl(gatewayPort);
 
 	jest.useFakeTimers();
 
@@ -57,20 +49,9 @@ describe('TldrawBoardRepo', () => {
 		await app.close();
 	});
 
-	const setupWs = async (docName?: string) => {
-		if (docName) {
-			ws = new WebSocket(`${wsUrl}/${docName}`);
-		} else {
-			ws = new WebSocket(`${wsUrl}`);
-		}
-		await new Promise((resolve) => {
-			ws.on('open', resolve);
-		});
-	};
-
 	describe('when awareness change was called', () => {
 		const setup = async () => {
-			await setupWs();
+			ws = await TestHelper.setupWs(wsUrl);
 
 			class MockAwareness {
 				on = jest.fn();

apps/server/src/modules/tldraw/domain/ws-shared-doc.do.ts

@@ -2,12 +2,8 @@ import { Doc } from 'yjs';
 import WebSocket from 'ws';
 import { Awareness, encodeAwarenessUpdate } from 'y-protocols/awareness';
 import { encoding } from 'lib0';
-import { Configuration } from '@hpi-schul-cloud/commons';
-import { WSMessageType } from '@src/modules/tldraw/types/connection-enum';
-import { TldrawWsService } from '@src/modules/tldraw/service';
-
-// disable gc when using snapshots!
-const gcEnabled: boolean = Configuration.get('TLDRAW__GC_ENABLED') as boolean;
+import { WSMessageType } from '../types/connection-enum';
+import { TldrawWsService } from '../service';
 
 export class WsSharedDocDo extends Doc {
 	name: string;
@@ -19,8 +15,9 @@ export class WsSharedDocDo extends Doc {
 	/**
 	 * @param {string} name
 	 * @param {TldrawWsService} tldrawService
+	 * @param {boolean} gcEnabled
 	 */
-	constructor(name: string, private tldrawService: TldrawWsService) {
+	constructor(name: string, private tldrawService: TldrawWsService, gcEnabled = true) {
 		super({ gc: gcEnabled });
 		this.name = name;
 		this.conns = new Map();
@@ -35,15 +32,24 @@ export class WsSharedDocDo extends Doc {
 
 	/**
 	 * @param {{ added: Array<number>, updated: Array<number>, removed: Array<number> }} changes
-	 * @param {WebSocket | null} conn Origin is the connection that made the change
+	 * @param {WebSocket | null} wsConnection Origin is the connection that made the change
 	 */
 	public awarenessChangeHandler = (
 		{ added, updated, removed }: { added: Array<number>; updated: Array<number>; removed: Array<number> },
-		conn: WebSocket | null
+		wsConnection: WebSocket | null
 	) => {
+		const changedClients = this.manageClientsConnections({ added, updated, removed }, wsConnection);
+		const buff = this.prepareAwarenessMessage(changedClients);
+		this.sendAwarenessMessage(buff);
+	};
+
+	private manageClientsConnections(
+		{ added, updated, removed }: { added: Array<number>; updated: Array<number>; removed: Array<number> },
+		wsConnection: WebSocket | null
+	) {
 		const changedClients = added.concat(updated, removed);
-		if (conn !== null) {
-			const connControlledIDs = this.conns.get(conn) as Set<number>;
+		if (wsConnection !== null) {
+			const connControlledIDs = this.conns.get(wsConnection);
 			if (connControlledIDs !== undefined) {
 				added.forEach((clientID) => {
 					connControlledIDs.add(clientID);
@@ -53,13 +59,20 @@ export class WsSharedDocDo extends Doc {
 				});
 			}
 		}
-		// broadcast awareness update
+		return changedClients;
+	}
+
+	private prepareAwarenessMessage(changedClients: number[]) {
 		const encoder = encoding.createEncoder();
 		encoding.writeVarUint(encoder, WSMessageType.AWARENESS);
 		encoding.writeVarUint8Array(encoder, encodeAwarenessUpdate(this.awareness, changedClients));
-		const buff = encoding.toUint8Array(encoder);
+		const message = encoding.toUint8Array(encoder);
+		return message;
+	}
+
+	private sendAwarenessMessage(buff: Uint8Array) {
 		this.conns.forEach((_, c) => {
 			this.tldrawService.send(this, c, buff);
 		});
-	};
+	}
 }

apps/server/src/modules/tldraw/helper/test-helper.ts

@@ -0,0 +1,22 @@
+import WebSocket from 'ws';
+
+export class TestHelper {
+	public static getWsUrl = (gatewayPort: number): string => {
+		const wsUrl = `ws://localhost:${gatewayPort}`;
+		return wsUrl;
+	};
+
+	public static setupWs = async (wsUrl: string, docName?: string): Promise<WebSocket> => {
+		let ws: WebSocket;
+		if (docName) {
+			ws = new WebSocket(`${wsUrl}/${docName}`);
+		} else {
+			ws = new WebSocket(`${wsUrl}`);
+		}
+		await new Promise((resolve) => {
+			ws.on('open', resolve);
+		});
+
+		return ws;
+	};
+}