BC-6711 - Fix whiteboard issues (#4799)

* improve tldraw logic

* remove unused methods

* improve asset sync between tldraw board and file storage

* add additional guards to prevent multiple saves to db

* add periodic merge of tldraw updates in db

---------

Co-authored-by: Cedric Evers <[email protected]>

apps/server/src/infra/rabbitmq/exchange/files-storage.ts

@@ -8,7 +8,7 @@ export enum FilesStorageEvents {
 	'LIST_FILES_OF_PARENT' = 'list-files-of-parent',
 	'DELETE_FILES_OF_PARENT' = 'delete-files-of-parent',
 	'REMOVE_CREATORID_OF_FILES' = 'remove-creatorId-of-files',
-	'DELETE_ONE_FILE' = 'delete-one-file',
+	'DELETE_FILES' = 'delete-files',
 }
 
 export enum ScanStatus {

apps/server/src/modules/files-storage-client/service/files-storage-client.service.spec.ts

@@ -151,29 +151,31 @@ describe('FilesStorageClientAdapterService', () => {
 		});
 	});
 
-	describe('deleteOneFile', () => {
-		describe('when file is deleted successfully', () => {
+	describe('deleteFiles', () => {
+		describe('when files are deleted successfully', () => {
 			const setup = () => {
 				const recordId = new ObjectId().toHexString();
 
-				const spy = jest.spyOn(FilesStorageClientMapper, 'mapFileRecordResponseToFileDto').mockImplementation(() => {
-					return {
-						id: recordId,
-						name: 'file',
-						parentId: 'parentId',
-						parentType: FileRecordParentType.BoardNode,
-					};
-				});
+				const spy = jest
+					.spyOn(FilesStorageClientMapper, 'mapfileRecordListResponseToDomainFilesDto')
+					.mockImplementation(() => [
+						{
+							id: recordId,
+							name: 'file',
+							parentId: 'parentId',
+							parentType: FileRecordParentType.BoardNode,
+						},
+					]);
 
 				return { recordId, spy };
 			};
 
 			it('Should call all steps.', async () => {
 				const { recordId, spy } = setup();
 
-				await service.deleteOneFile(recordId);
+				await service.deleteFiles([recordId]);
 
-				expect(client.deleteOneFile).toHaveBeenCalledWith(recordId);
+				expect(client.deleteFiles).toHaveBeenCalledWith([recordId]);
 				expect(spy).toBeCalled();
 
 				spy.mockRestore();
@@ -184,15 +186,15 @@ describe('FilesStorageClientAdapterService', () => {
 			const setup = () => {
 				const recordId = new ObjectId().toHexString();
 
-				client.deleteOneFile.mockRejectedValue(new Error());
+				client.deleteFiles.mockRejectedValue(new Error());
 
 				return { recordId };
 			};
 
 			it('Should call error mapper if throw an error.', async () => {
 				const { recordId } = setup();
 
-				await expect(service.deleteOneFile(recordId)).rejects.toThrowError();
+				await expect(service.deleteFiles([recordId])).rejects.toThrowError();
 			});
 		});
 	});

apps/server/src/modules/files-storage-client/service/files-storage-client.service.ts

@@ -38,12 +38,12 @@ export class FilesStorageClientAdapterService {
 		return fileInfos;
 	}
 
-	async deleteOneFile(fileRecordId: EntityId): Promise<FileDto> {
-		const response = await this.fileStorageMQProducer.deleteOneFile(fileRecordId);
+	async deleteFiles(fileRecordIds: EntityId[]): Promise<FileDto[]> {
+		const response = await this.fileStorageMQProducer.deleteFiles(fileRecordIds);
 
-		const fileInfo = FilesStorageClientMapper.mapFileRecordResponseToFileDto(response);
+		const fileInfos = FilesStorageClientMapper.mapfileRecordListResponseToDomainFilesDto(response);
 
-		return fileInfo;
+		return fileInfos;
 	}
 
 	async removeCreatorIdFromFileRecords(creatorId: EntityId): Promise<DomainOperation> {

apps/server/src/modules/files-storage-client/service/files-storage.producer.spec.ts

@@ -239,7 +239,7 @@ describe('FilesStorageProducer', () => {
 		});
 	});
 
-	describe('deleteOneFile', () => {
+	describe('deleteFiles', () => {
 		describe('when valid parameter passed and amqpConnection return with error in response', () => {
 			const setup = () => {
 				const recordId = new ObjectId().toHexString();
@@ -253,7 +253,7 @@ describe('FilesStorageProducer', () => {
 			it('should call error mapper and throw with error', async () => {
 				const { recordId, spy } = setup();
 
-				await expect(service.deleteOneFile(recordId)).rejects.toThrowError();
+				await expect(service.deleteFiles([recordId])).rejects.toThrowError();
 				expect(spy).toBeCalled();
 			});
 		});
@@ -267,26 +267,25 @@ describe('FilesStorageProducer', () => {
 
 				const expectedParams = {
 					exchange: FilesStorageExchange,
-					routingKey: FilesStorageEvents.DELETE_ONE_FILE,
-					payload: recordId,
+					routingKey: FilesStorageEvents.DELETE_FILES,
+					payload: [recordId],
 					timeout,
 				};
-
 				return { recordId, message, expectedParams };
 			};
 
 			it('should call the ampqConnection.', async () => {
 				const { recordId, expectedParams } = setup();
 
-				await service.deleteOneFile(recordId);
+				await service.deleteFiles([recordId]);
 
 				expect(amqpConnection.request).toHaveBeenCalledWith(expectedParams);
 			});
 
 			it('should return the response message.', async () => {
 				const { recordId, message } = setup();
 
-				const res = await service.deleteOneFile(recordId);
+				const res = await service.deleteFiles([recordId]);
 
 				expect(res).toEqual(message);
 			});

apps/server/src/modules/files-storage-client/service/files-storage.producer.ts

@@ -51,11 +51,11 @@ export class FilesStorageProducer extends RpcMessageProducer {
 		return response;
 	}
 
-	async deleteOneFile(payload: EntityId): Promise<FileDO> {
-		this.logger.debug({ action: 'deleteOneFile:started', payload });
-		const response = await this.request<FileDO>(FilesStorageEvents.DELETE_ONE_FILE, payload);
+	async deleteFiles(payload: EntityId[]): Promise<FileDO[]> {
+		this.logger.debug({ action: 'deleteFiles:started', payload });
+		const response = await this.request<FileDO[]>(FilesStorageEvents.DELETE_FILES, payload);
 
-		this.logger.debug({ action: 'deleteOneFile:finished', payload });
+		this.logger.debug({ action: 'deleteFiles:finished', payload });
 
 		return response;
 	}

apps/server/src/modules/files-storage/controller/files-storage.consumer.spec.ts

@@ -211,7 +211,7 @@ describe('FilesStorageConsumer', () => {
 		});
 	});
 
-	describe('deleteOneFile()', () => {
+	describe('deleteFiles()', () => {
 		describe('WHEN valid file exists', () => {
 			const setup = () => {
 				const recordId = new ObjectId().toHexString();
@@ -222,22 +222,22 @@ describe('FilesStorageConsumer', () => {
 				return { recordId, fileRecord };
 			};
 
-			it('should call filesStorageService.deleteOneFile with params', async () => {
+			it('should call filesStorageService.deleteFiles with params', async () => {
 				const { recordId, fileRecord } = setup();
 
-				await service.deleteOneFile(recordId);
+				await service.deleteFiles([recordId]);
 
 				const result = [fileRecord];
 				expect(filesStorageService.getFileRecord).toBeCalledWith({ fileRecordId: recordId });
-				expect(filesStorageService.deleteFilesOfParent).toBeCalledWith(result);
+				expect(filesStorageService.delete).toBeCalledWith(result);
 			});
 
-			it('should return an instance of FileRecordResponse', async () => {
+			it('should return array instances of FileRecordResponse', async () => {
 				const { recordId } = setup();
 
-				const response = await service.deleteOneFile(recordId);
+				const response = await service.deleteFiles([recordId]);
 
-				expect(response.message).toBeInstanceOf(FileRecordResponse);
+				expect(response.message[0]).toBeInstanceOf(FileRecordResponse);
 			});
 		});
 
@@ -253,7 +253,7 @@ describe('FilesStorageConsumer', () => {
 			it('should throw', async () => {
 				const { recordId } = setup();
 
-				await expect(service.deleteOneFile(recordId)).rejects.toThrow('not found');
+				await expect(service.deleteFiles([recordId])).rejects.toThrow('not found');
 			});
 		});
 	});

apps/server/src/modules/files-storage/controller/files-storage.consumer.ts

@@ -1,6 +1,5 @@
 import { RabbitPayload, RabbitRPC } from '@golevelup/nestjs-rabbitmq';
-import { CopyFileDO, FileDO, FilesStorageEvents, FilesStorageExchange } from '@infra/rabbitmq';
-import { RpcMessage } from '@infra/rabbitmq/rpc-message';
+import { CopyFileDO, FileDO, FilesStorageEvents, FilesStorageExchange, RpcMessage } from '@infra/rabbitmq';
 import { MikroORM, UseRequestContext } from '@mikro-orm/core';
 import { Injectable } from '@nestjs/common';
 import { EntityId } from '@shared/domain/types';
@@ -75,21 +74,22 @@ export class FilesStorageConsumer {
 
 	@RabbitRPC({
 		exchange: FilesStorageExchange,
-		routingKey: FilesStorageEvents.DELETE_ONE_FILE,
-		queue: FilesStorageEvents.DELETE_ONE_FILE,
+		routingKey: FilesStorageEvents.DELETE_FILES,
+		queue: FilesStorageEvents.DELETE_FILES,
 	})
 	@UseRequestContext()
-	public async deleteOneFile(@RabbitPayload() payload: EntityId): Promise<RpcMessage<FileDO>> {
-		this.logger.debug({ action: 'deleteOneFile', payload });
+	public async deleteFiles(@RabbitPayload() payload: EntityId[]): Promise<RpcMessage<FileDO[]>> {
+		this.logger.debug({ action: 'deleteFiles', payload });
 
-		const fileRecord = await this.filesStorageService.getFileRecord({ fileRecordId: payload });
+		const promise = payload.map((fileRecordId) => this.filesStorageService.getFileRecord({ fileRecordId }));
+		const fileRecords = await Promise.all(promise);
 
-		await this.previewService.deletePreviews([fileRecord]);
-		await this.filesStorageService.deleteFilesOfParent([fileRecord]);
+		await this.previewService.deletePreviews(fileRecords);
+		await this.filesStorageService.delete(fileRecords);
 
-		const response = FilesStorageMapper.mapToFileRecordResponse(fileRecord);
+		const response = FilesStorageMapper.mapToFileRecordListResponse(fileRecords, fileRecords.length);
 
-		return { message: response };
+		return { message: response.data };
 	}
 
 	@RabbitRPC({

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

@@ -4,7 +4,7 @@ export interface TldrawConfig {
 	TLDRAW_DB_URL: string;
 	NEST_LOG_LEVEL: string;
 	INCOMING_REQUEST_TIMEOUT: number;
-	TLDRAW_DB_FLUSH_SIZE: string;
+	TLDRAW_DB_COMPRESS_THRESHOLD: string;
 	CONNECTION_STRING: string;
 	FEATURE_TLDRAW_ENABLED: boolean;
 	TLDRAW_PING_TIMEOUT: number;
@@ -24,7 +24,7 @@ const tldrawConfig = {
 	TLDRAW_DB_URL,
 	NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string,
 	INCOMING_REQUEST_TIMEOUT: Configuration.get('INCOMING_REQUEST_TIMEOUT_API') as number,
-	TLDRAW_DB_FLUSH_SIZE: Configuration.get('TLDRAW__DB_FLUSH_SIZE') as number,
+	TLDRAW_DB_COMPRESS_THRESHOLD: Configuration.get('TLDRAW__DB_COMPRESS_THRESHOLD') as number,
 	FEATURE_TLDRAW_ENABLED: Configuration.get('FEATURE_TLDRAW_ENABLED') as boolean,
 	CONNECTION_STRING: Configuration.get('TLDRAW_DB_URL') as string,
 	TLDRAW_PING_TIMEOUT: Configuration.get('TLDRAW__PING_TIMEOUT') as number,

apps/server/src/modules/tldraw/loggable/close-connection.loggable.spec.ts

@@ -4,7 +4,7 @@ describe('CloseConnectionLoggable', () => {
 	describe('getLogMessage', () => {
 		const setup = () => {
 			const error = new Error('test');
-			const loggable = new CloseConnectionLoggable(error);
+			const loggable = new CloseConnectionLoggable('functionName', error);
 
 			return { loggable, error };
 		};
@@ -15,8 +15,8 @@ describe('CloseConnectionLoggable', () => {
 			const message = loggable.getLogMessage();
 
 			expect(message).toEqual({
-				message: 'Close web socket connection error',
-				type: 'CLOSE_WEB_SOCKET_CONNECTION_ERROR',
+				message: 'Close web socket error in functionName',
+				type: 'CLOSE_WEB_SOCKET_ERROR',
 				error,
 			});
 		});

apps/server/src/modules/tldraw/loggable/close-connection.loggable.ts

@@ -3,16 +3,16 @@ import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from
 export class CloseConnectionLoggable implements Loggable {
 	private error: Error | undefined;
 
-	constructor(private readonly err: unknown) {
+	constructor(private readonly errorLocation: string, private readonly err: unknown) {
 		if (err instanceof Error) {
 			this.error = err;
 		}
 	}
 
 	getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
 		return {
-			message: `Close web socket connection error`,
-			type: `CLOSE_WEB_SOCKET_CONNECTION_ERROR`,
+			message: `Close web socket error in ${this.errorLocation}`,
+			type: `CLOSE_WEB_SOCKET_ERROR`,
 			error: this.error,
 		};
 	}

apps/server/src/modules/tldraw/loggable/file-storage-error.loggable.spec.ts

@@ -0,0 +1,24 @@
+import { FileStorageErrorLoggable } from './file-storage-error.loggable';
+
+describe('FileStorageErrorLoggable', () => {
+	describe('getLogMessage', () => {
+		const setup = () => {
+			const error = new Error('test');
+			const loggable = new FileStorageErrorLoggable('doc1', error);
+
+			return { loggable, error };
+		};
+
+		it('should return a loggable message', () => {
+			const { loggable, error } = setup();
+
+			const message = loggable.getLogMessage();
+
+			expect(message).toEqual({
+				message: 'Error in document doc1: assets could not be synchronized with file storage.',
+				type: 'FILE_STORAGE_GENERAL_ERROR',
+				error,
+			});
+		});
+	});
+});

apps/server/src/modules/tldraw/loggable/file-storage-error.loggable.ts

@@ -0,0 +1,19 @@
+import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
+
+export class FileStorageErrorLoggable implements Loggable {
+	private error: Error | undefined;
+
+	constructor(private readonly docName: string, private readonly err: unknown) {
+		if (err instanceof Error) {
+			this.error = err;
+		}
+	}
+
+	getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
+		return {
+			message: `Error in document ${this.docName}: assets could not be synchronized with file storage.`,
+			type: `FILE_STORAGE_GENERAL_ERROR`,
+			error: this.error,
+		};
+	}
+}

apps/server/src/modules/tldraw/loggable/index.ts

@@ -6,3 +6,4 @@ export * from './websocket-close-error.loggable';
 export * from './websocket-message-error.loggable';
 export * from './ws-shared-doc-error.loggable';
 export * from './close-connection.loggable';
+export * from './file-storage-error.loggable';

apps/server/src/modules/tldraw/loggable/websocket-error.loggable.spec.ts

@@ -14,7 +14,7 @@ describe('WebsocketErrorLoggable', () => {
 
 			const message = loggable.getLogMessage();
 
-			expect(message).toEqual({ message: 'Websocket error', error, type: 'WEBSOCKET_ERROR' });
+			expect(message).toEqual({ message: 'Websocket error event', error, type: 'WEBSOCKET_ERROR' });
 		});
 	});
 });

apps/server/src/modules/tldraw/loggable/websocket-error.loggable.ts

@@ -11,7 +11,7 @@ export class WebsocketErrorLoggable implements Loggable {
 
 	getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
 		return {
-			message: 'Websocket error',
+			message: 'Websocket error event',
 			type: 'WEBSOCKET_ERROR',
 			error: this.error,
 		};