BC-5714 - Remove async from busboy

ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2

@@ -14,6 +14,7 @@ metadata:
     nginx.ingress.kubernetes.io/http2-max-header-size: 96k
     nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k
     nginx.ingress.kubernetes.io/proxy-buffer-size: 96k
+    nginx.ingress.kubernetes.io/proxy-request-buffering: "off"
 {% if CLUSTER_ISSUER is defined %}
     cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }}
 {% endif %}

apps/server/src/modules/files-storage/controller/dto/file-storage.response.ts

@@ -14,6 +14,7 @@ export class FileRecordResponse {
 		this.creatorId = fileRecord.creatorId;
 		this.mimeType = fileRecord.mimeType;
 		this.parentType = fileRecord.parentType;
+		this.isUploading = fileRecord.isUploading;
 		this.deletedSince = fileRecord.deletedSince;
 		this.previewStatus = fileRecord.getPreviewStatus();
 	}
@@ -46,6 +47,9 @@ export class FileRecordResponse {
 	@ApiProperty({ enum: FileRecordParentType, enumName: 'FileRecordParentType' })
 	parentType: FileRecordParentType;
 
+	@ApiPropertyOptional()
+	isUploading?: boolean;
+
 	@ApiProperty({ enum: PreviewStatus, enumName: 'PreviewStatus' })
 	previewStatus: PreviewStatus;
 

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

@@ -127,6 +127,7 @@ export class FilesStorageController {
 	@ApiResponse({ status: 422, type: UnprocessableEntityException })
 	@ApiResponse({ status: 500, type: InternalServerErrorException })
 	@ApiHeader({ name: 'Range', required: false })
+	@ApiHeader({ name: 'If-None-Match', required: false })
 	@Get('/preview/:fileRecordId/:fileName')
 	@RequestTimeout(config().INCOMING_REQUEST_TIMEOUT)
 	async downloadPreview(

apps/server/src/modules/files-storage/entity/filerecord.entity.spec.ts

@@ -32,7 +32,7 @@ describe('FileRecord Entity', () => {
 			};
 		});
 
-		it('should provide the target id as entity id', () => {
+		it('should provide target id', () => {
 			const parentId = new ObjectId().toHexString();
 			const fileRecord = new FileRecord({
 				...props,
@@ -41,7 +41,7 @@ describe('FileRecord Entity', () => {
 			expect(fileRecord.parentId).toEqual(parentId);
 		});
 
-		it('should provide the creator id as entity id', () => {
+		it('should provide creator id', () => {
 			const creatorId = new ObjectId().toHexString();
 			const fileRecord = new FileRecord({
 				...props,
@@ -50,7 +50,7 @@ describe('FileRecord Entity', () => {
 			expect(fileRecord.creatorId).toEqual(creatorId);
 		});
 
-		it('should provide the school id as entity id', () => {
+		it('should provide school id', () => {
 			const schoolId = new ObjectId().toHexString();
 			const fileRecord = new FileRecord({
 				...props,
@@ -59,14 +59,23 @@ describe('FileRecord Entity', () => {
 			expect(fileRecord.schoolId).toEqual(schoolId);
 		});
 
-		it('should provide the isCopyFrom as entity id', () => {
+		it('should provide isCopyFrom', () => {
 			const isCopyFrom = new ObjectId().toHexString();
 			const fileRecord = new FileRecord({
 				...props,
 				isCopyFrom,
 			});
 			expect(fileRecord.isCopyFrom).toEqual(isCopyFrom);
 		});
+
+		it('should provide isUploading', () => {
+			const isUploading = true;
+			const fileRecord = new FileRecord({
+				...props,
+				isUploading,
+			});
+			expect(fileRecord.isUploading).toEqual(isUploading);
+		});
 	});
 
 	describe('when embedding the security status', () => {
@@ -852,4 +861,23 @@ describe('FileRecord Entity', () => {
 			});
 		});
 	});
+
+	describe('markAsLoaded is called', () => {
+		describe('WHEN isUploading is true', () => {
+			const setup = () => {
+				const isUploading = true;
+				const fileRecord = fileRecordFactory.build({ isUploading });
+
+				return { fileRecord, isUploading };
+			};
+
+			it('should set it to undefined', () => {
+				const { fileRecord } = setup();
+				expect(fileRecord.isUploading).toBe(true);
+				const result = fileRecord.markAsUploaded();
+
+				expect(result).toBe(undefined);
+			});
+		});
+	});
 });

apps/server/src/modules/files-storage/entity/filerecord.entity.ts

@@ -80,6 +80,7 @@ export interface FileRecordProperties {
 	schoolId: EntityId;
 	deletedSince?: Date;
 	isCopyFrom?: EntityId;
+	isUploading?: boolean;
 }
 
 interface ParentInfo {
@@ -120,6 +121,9 @@ export class FileRecord extends BaseEntityWithTimestamps {
 	@Enum()
 	parentType: FileRecordParentType;
 
+	@Property({ nullable: true })
+	isUploading?: boolean;
+
 	@Index()
 	@Property({ fieldName: 'parent' })
 	_parentId: ObjectId;
@@ -161,6 +165,7 @@ export class FileRecord extends BaseEntityWithTimestamps {
 		this.name = props.name;
 		this.mimeType = props.mimeType;
 		this.parentType = props.parentType;
+		this.isUploading = props.isUploading;
 		this._parentId = new ObjectId(props.parentId);
 		if (props.creatorId !== undefined) {
 			this._creatorId = new ObjectId(props.creatorId);
@@ -311,4 +316,8 @@ export class FileRecord extends BaseEntityWithTimestamps {
 	public removeCreatorId(): void {
 		this.creatorId = undefined;
 	}
+
+	public markAsUploaded(): void {
+		this.isUploading = undefined;
+	}
 }

apps/server/src/modules/files-storage/helper/file-record.ts

@@ -36,6 +36,7 @@ export function createFileRecord(
 		parentId: params.parentId,
 		creatorId: userId,
 		schoolId: params.schoolId,
+		isUploading: true,
 	});
 
 	return entity;

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

@@ -1,10 +1,10 @@
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
+import { AntivirusService } from '@infra/antivirus';
+import { S3ClientAdapter } from '@infra/s3-client';
 import { ObjectId } from '@mikro-orm/mongodb';
 import { BadRequestException } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { Test, TestingModule } from '@nestjs/testing';
-import { AntivirusService } from '@infra/antivirus';
-import { S3ClientAdapter } from '@infra/s3-client';
 import { fileRecordFactory, setupEntities } from '@shared/testing';
 import { readableStreamWithFileTypeFactory } from '@shared/testing/factory/readable-stream-with-file-type.factory';
 import { LegacyLogger } from '@src/core/logger';
@@ -157,6 +157,8 @@ describe('FilesStorageService upload methods', () => {
 					if (fr instanceof FileRecord && !fr._id) {
 						fr._id = new ObjectId();
 					}
+
+					return Promise.resolve();
 				});
 
 				return {
@@ -187,6 +189,37 @@ describe('FilesStorageService upload methods', () => {
 				expect(getFileRecordsOfParentSpy).toHaveBeenCalledWith(params.parentId);
 			});
 
+			it('should call fileRecordRepo.save in first call with isUploading: true', async () => {
+				const { params, file, userId } = setup();
+
+				// haveBeenCalledWith can't be use here because fileRecord is a reference and
+				// it will always compare the final state of the object
+				let param: FileRecord | undefined;
+
+				fileRecordRepo.save.mockReset();
+				fileRecordRepo.save.mockImplementationOnce(async (fr) => {
+					if (fr instanceof FileRecord && !fr._id) {
+						fr._id = new ObjectId();
+					}
+
+					param = JSON.parse(JSON.stringify(fr)) as FileRecord;
+
+					return Promise.resolve();
+				});
+
+				fileRecordRepo.save.mockImplementationOnce(async (fr) => {
+					if (fr instanceof FileRecord && !fr._id) {
+						fr._id = new ObjectId();
+					}
+
+					return Promise.resolve();
+				});
+
+				await service.uploadFile(userId, params, file);
+
+				expect(param).toMatchObject({ isUploading: true });
+			});
+
 			it('should call fileRecordRepo.save twice with correct params', async () => {
 				const { params, file, fileSize, userId, readableStreamWithFileType, expectedFileRecord } = setup();
 
@@ -201,6 +234,7 @@ describe('FilesStorageService upload methods', () => {
 						size: fileSize,
 						createdAt: expect.any(Date),
 						updatedAt: expect.any(Date),
+						isUploading: undefined,
 					})
 				);
 			});

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

@@ -180,12 +180,16 @@ export class FilesStorageService {
 			// The actual file size is set here because it is known only after the whole file is streamed.
 			fileRecord.size = await fileSizePromise;
 			this.throwErrorIfFileIsTooBig(fileRecord.size);
+
+			fileRecord.markAsUploaded();
+
 			await this.fileRecordRepo.save(fileRecord);
 
 			if (!useStreamToAntivirus || !fileRecord.isPreviewPossible()) {
 				await this.sendToAntivirus(fileRecord);
 			}
 		} catch (error) {
+			this.logger.error(`create file failed for fileRecordId ${fileRecord.id}`, error);
 			await this.storageClient.delete([filePath]);
 			await this.fileRecordRepo.delete(fileRecord);
 			throw error;

apps/server/src/modules/files-storage/uc/files-storage.uc.ts

@@ -1,3 +1,4 @@
+import { EntityManager, RequestContext } from '@mikro-orm/core';
 import { AuthorizationContext } from '@modules/authorization';
 import { AuthorizationReferenceService } from '@modules/authorization/domain';
 import { HttpService } from '@nestjs/axios';
@@ -36,7 +37,8 @@ export class FilesStorageUC {
 		private readonly authorizationReferenceService: AuthorizationReferenceService,
 		private readonly httpService: HttpService,
 		private readonly filesStorageService: FilesStorageService,
-		private readonly previewService: PreviewService
+		private readonly previewService: PreviewService,
+		private readonly em: EntityManager
 	) {
 		this.logger.setContext(FilesStorageUC.name);
 	}
@@ -63,18 +65,30 @@ export class FilesStorageUC {
 	private async uploadFileWithBusboy(userId: EntityId, params: FileRecordParams, req: Request): Promise<FileRecord> {
 		const promise = new Promise<FileRecord>((resolve, reject) => {
 			const bb = busboy({ headers: req.headers, defParamCharset: 'utf8' });
+			let promise2: Promise<FileRecord>;
 
-			// eslint-disable-next-line @typescript-eslint/no-misused-promises
-			bb.on('file', async (_name, file, info) => {
+			bb.on('file', (_name, file, info) => {
 				const fileDto = FileDtoBuilder.buildFromRequest(info, file);
 
-				try {
-					const record = await this.filesStorageService.uploadFile(userId, params, fileDto);
-					resolve(record);
-				} catch (error) {
-					req.unpipe(bb);
-					reject(error);
-				}
+				promise2 = RequestContext.createAsync(this.em, () => {
+					const record = this.filesStorageService.uploadFile(userId, params, fileDto);
+
+					return record;
+				});
+			});
+
+			bb.on('finish', () => {
+				promise2
+					.then((result) => {
+						resolve(result);
+						console.log('fileclose', result);
+						return result;
+					})
+					.catch((error) => {
+						this.logger.error({ message: 'could not upload file', error: error as Error });
+						req.unpipe(bb);
+						reject(error);
+					});
 			});
 
 			req.pipe(bb);

package.json

@@ -66,7 +66,7 @@
 		"nest:start:management:prod": "node dist/apps/server/apps/management.app",
 		"nest:start:files-storage": "nest start files-storage & nest start files-storage-amqp & nest start preview-generator-amqp",
 		"nest:start:files-storage:dev": "nest start files-storage --watch & nest start files-storage-amqp --watch & nest start preview-generator-amqp --watch",
-		"nest:start:files-storage:debug": "nest start files-storage --debug --watch & nest start files-storage-amqp --debug --watch  & nest start preview-generator-amqp --debug --watch",
+		"nest:start:files-storage:debug": "nest start files-storage --debug --watch",
 		"nest:start:files-storage:prod": "node dist/apps/server/apps/files-storage.app",
 		"nest:start:files-storage-amqp:prod": "node dist/apps/server/apps/files-storage-consumer.app",
 		"nest:start:preview-generator-amqp:prod": "node dist/apps/server/apps/preview-generator-consumer.app",