diff --git a/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 index 3eabd5d2659..fe377635efd 100644 --- a/ansible/roles/schulcloud-server-core/templates/api-files-ingress.yml.j2 +++ b/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 %} diff --git a/apps/server/src/modules/files-storage/controller/dto/file-storage.response.ts b/apps/server/src/modules/files-storage/controller/dto/file-storage.response.ts index dc595d62d9a..094589673a3 100644 --- a/apps/server/src/modules/files-storage/controller/dto/file-storage.response.ts +++ b/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; diff --git a/apps/server/src/modules/files-storage/controller/files-storage.controller.ts b/apps/server/src/modules/files-storage/controller/files-storage.controller.ts index 7269336c44c..8fc7bf17c60 100644 --- a/apps/server/src/modules/files-storage/controller/files-storage.controller.ts +++ b/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( diff --git a/apps/server/src/modules/files-storage/entity/filerecord.entity.spec.ts b/apps/server/src/modules/files-storage/entity/filerecord.entity.spec.ts index 13c17ddcad8..4caac8fafc6 100644 --- a/apps/server/src/modules/files-storage/entity/filerecord.entity.spec.ts +++ b/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,7 +59,7 @@ 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, @@ -67,6 +67,15 @@ describe('FileRecord Entity', () => { }); 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); + }); + }); + }); }); diff --git a/apps/server/src/modules/files-storage/entity/filerecord.entity.ts b/apps/server/src/modules/files-storage/entity/filerecord.entity.ts index 9278d93a4fa..ac615d15f65 100644 --- a/apps/server/src/modules/files-storage/entity/filerecord.entity.ts +++ b/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; + } } diff --git a/apps/server/src/modules/files-storage/helper/file-record.ts b/apps/server/src/modules/files-storage/helper/file-record.ts index ed291661735..80a73249441 100644 --- a/apps/server/src/modules/files-storage/helper/file-record.ts +++ b/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; diff --git a/apps/server/src/modules/files-storage/service/files-storage-upload.service.spec.ts b/apps/server/src/modules/files-storage/service/files-storage-upload.service.spec.ts index 765de1077bd..63049e0c587 100644 --- a/apps/server/src/modules/files-storage/service/files-storage-upload.service.spec.ts +++ b/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, }) ); }); diff --git a/apps/server/src/modules/files-storage/service/files-storage.service.ts b/apps/server/src/modules/files-storage/service/files-storage.service.ts index 449971b5542..2de2cd8a1d4 100644 --- a/apps/server/src/modules/files-storage/service/files-storage.service.ts +++ b/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; diff --git a/apps/server/src/modules/files-storage/uc/files-storage.uc.ts b/apps/server/src/modules/files-storage/uc/files-storage.uc.ts index a9fae9db118..4b3ed1cd7ff 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage.uc.ts +++ b/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 { const promise = new Promise((resolve, reject) => { const bb = busboy({ headers: req.headers, defParamCharset: 'utf8' }); + let promise2: Promise; - // 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); diff --git a/package.json b/package.json index 656edf9a2d4..7dbc59cfe9b 100644 --- a/package.json +++ b/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",