From 1d5ff376130cfe0e7414fbaed0b24bca6aee1b90 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 5 Oct 2023 19:42:46 +0200 Subject: [PATCH 01/35] BC-5170 - wip v1 --- .../schulcloud-server-core/tasks/main.yml | 6 ++ .../templates/amqp-preview-deployment.yml.j2 | 46 ++++++++++ .../apps/preview-generator-consumer.app.ts | 19 ++++ .../service/files-storage.producer.ts | 53 +++-------- .../modules/files-storage/controller/index.ts | 3 +- .../controller/preview-generator.consumer.ts | 32 +++++++ .../files-storage/files-storage.module.ts | 6 +- .../server/src/modules/files-storage/index.ts | 1 + .../files-storage/interface/interfaces.ts | 24 ++++- .../preview-generator-amqp.module.ts | 11 +++ .../service/preview-generator.service.ts | 54 ++++++++++++ .../files-storage/service/preview.producer.ts | 28 ++++++ .../files-storage/service/preview.service.ts | 88 ++++++++----------- .../files-storage/uc/files-storage.uc.ts | 2 +- .../preview-generator.module.ts | 0 .../infra/rabbitmq/exchange/files-storage.ts | 1 + .../server/src/shared/infra/rabbitmq/index.ts | 1 + .../infra/rabbitmq/rpc-message-producer.ts | 37 ++++++++ .../shared/infra/s3-client/interface/index.ts | 1 - nest-cli.json | 9 ++ package.json | 7 +- 21 files changed, 323 insertions(+), 106 deletions(-) create mode 100644 ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 create mode 100644 apps/server/src/apps/preview-generator-consumer.app.ts create mode 100644 apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts create mode 100644 apps/server/src/modules/files-storage/preview-generator-amqp.module.ts create mode 100644 apps/server/src/modules/files-storage/service/preview-generator.service.ts create mode 100644 apps/server/src/modules/files-storage/service/preview.producer.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.module.ts create mode 100644 apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index fff5b10197a..c8eac90aaa0 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -83,3 +83,9 @@ kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" template: amqp-files-deployment.yml.j2 + + - name: AMQPFilePreviewDeployment + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: amqp-preview-deployment.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 new file mode 100644 index 00000000000..bdcc28fd5be --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 @@ -0,0 +1,46 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: amqp-file-preview-deployment + namespace: {{ NAMESPACE }} + labels: + app: amqp-file-preview +spec: + replicas: {{ AMQP_FILE_PREVIEW_REPLICAS|default("1", true) }} + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + #maxUnavailable: 1 + revisionHistoryLimit: 4 + paused: false + selector: + matchLabels: + app: amqp-file-preview + template: + metadata: + labels: + app: amqp-file-preview + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 1000 + fsGroup: 1000 + runAsNonRoot: true + containers: + - name: amqp-file-preview + image: {{ SCHULCLOUD_SERVER_IMAGE }}:file-storage-{{ SCHULCLOUD_SERVER_IMAGE_TAG }} + imagePullPolicy: IfNotPresent + envFrom: + - configMapRef: + name: api-configmap + - secretRef: + name: api-secret + command: ['npm', 'run', 'nest:start:preview-generator-amqp:prod'] + resources: + limits: + cpu: {{ AMQP_FILE_PREVIEW_CPU_LIMITS|default("1000m", true) }} + memory: {{ AMQP_FILE_PREVIEW_MEMORY_LIMITS|default("2500Mi", true) }} + requests: + cpu: {{ AMQP_FILE_PREVIEW_CPU_REQUESTS|default("100m", true) }} + memory: {{ AMQP_FILE_PREVIEW_MEMORY_REQUESTS|default("50Mi", true) }} diff --git a/apps/server/src/apps/preview-generator-consumer.app.ts b/apps/server/src/apps/preview-generator-consumer.app.ts new file mode 100644 index 00000000000..f2abf69f5c9 --- /dev/null +++ b/apps/server/src/apps/preview-generator-consumer.app.ts @@ -0,0 +1,19 @@ +/* istanbul ignore file */ +/* eslint-disable no-console */ +import { NestFactory } from '@nestjs/core'; + +// register source-map-support for debugging +import { PreviewGeneratorAMQPModule } from '@src/modules/files-storage'; +import { install as sourceMapInstall } from 'source-map-support'; + +async function bootstrap() { + sourceMapInstall(); + + const nestApp = await NestFactory.createMicroservice(PreviewGeneratorAMQPModule); + await nestApp.init(); + + console.log('#############################################'); + console.log(`### Start Preview Generator AMQP Consumer ###`); + console.log('#############################################'); +} +void bootstrap(); diff --git a/apps/server/src/modules/files-storage-client/service/files-storage.producer.ts b/apps/server/src/modules/files-storage-client/service/files-storage.producer.ts index afd4f6365e1..ea049442df4 100644 --- a/apps/server/src/modules/files-storage-client/service/files-storage.producer.ts +++ b/apps/server/src/modules/files-storage-client/service/files-storage.producer.ts @@ -2,7 +2,6 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { EntityId } from '@shared/domain'; -import { RpcMessage } from '@shared/infra/rabbitmq/rpc-message'; import { LegacyLogger } from '@src/core/logger'; import { FilesStorageEvents, @@ -11,75 +10,45 @@ import { ICopyFilesOfParentParams, IFileDO, IFileRecordParams, + RpcMessageProducer, } from '@src/shared/infra/rabbitmq'; import { IFilesStorageClientConfig } from '../interfaces'; -import { ErrorMapper } from '../mapper/error.mapper'; @Injectable() -export class FilesStorageProducer { - private readonly timeout = 0; - +export class FilesStorageProducer extends RpcMessageProducer { constructor( + protected readonly amqpConnection: AmqpConnection, private readonly logger: LegacyLogger, - private readonly amqpConnection: AmqpConnection, - private readonly configService: ConfigService + protected readonly configService: ConfigService ) { + super(amqpConnection, FilesStorageExchange, configService.get('INCOMING_REQUEST_TIMEOUT_COPY_API')); this.logger.setContext(FilesStorageProducer.name); - this.timeout = this.configService.get('INCOMING_REQUEST_TIMEOUT_COPY_API'); } async copyFilesOfParent(payload: ICopyFilesOfParentParams): Promise { this.logger.debug({ action: 'copyFilesOfParent:started', payload }); - const response = await this.amqpConnection.request>( - this.createRequest(FilesStorageEvents.COPY_FILES_OF_PARENT, payload) - ); + const response = await this.request(FilesStorageEvents.COPY_FILES_OF_PARENT, payload); this.logger.debug({ action: 'copyFilesOfParent:finished', payload }); - this.checkError(response); - return response.message || []; + return response; } async listFilesOfParent(payload: IFileRecordParams): Promise { this.logger.debug({ action: 'listFilesOfParent:started', payload }); - const response = await this.amqpConnection.request>( - this.createRequest(FilesStorageEvents.LIST_FILES_OF_PARENT, payload) - ); + const response = await this.request(FilesStorageEvents.LIST_FILES_OF_PARENT, payload); this.logger.debug({ action: 'listFilesOfParent:finished', payload }); - this.checkError(response); - return response.message || []; + return response; } async deleteFilesOfParent(payload: EntityId): Promise { this.logger.debug({ action: 'deleteFilesOfParent:started', payload }); - const response = await this.amqpConnection.request>( - this.createRequest(FilesStorageEvents.DELETE_FILES_OF_PARENT, payload) - ); + const response = await this.request(FilesStorageEvents.DELETE_FILES_OF_PARENT, payload); this.logger.debug({ action: 'deleteFilesOfParent:finished', payload }); - this.checkError(response); - return response.message || []; - } - - // need to be fixed with https://ticketsystem.dbildungscloud.de/browse/BC-2984 - // mapRpcErrorResponseToDomainError should also removed with this ticket - private checkError(response: RpcMessage) { - const { error } = response; - if (error) { - const domainError = ErrorMapper.mapRpcErrorResponseToDomainError(error); - throw domainError; - } - } - - private createRequest(event: FilesStorageEvents, payload: IFileRecordParams | ICopyFilesOfParentParams | EntityId) { - return { - exchange: FilesStorageExchange, - routingKey: event, - payload, - timeout: this.timeout, - }; + return response; } } diff --git a/apps/server/src/modules/files-storage/controller/index.ts b/apps/server/src/modules/files-storage/controller/index.ts index ffd18a5db36..9300d186f61 100644 --- a/apps/server/src/modules/files-storage/controller/index.ts +++ b/apps/server/src/modules/files-storage/controller/index.ts @@ -1,3 +1,4 @@ export * from './file-security.controller'; -export * from './files-storage.controller'; export * from './files-storage.consumer'; +export * from './files-storage.controller'; +export * from './preview-generator.consumer'; diff --git a/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts b/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts new file mode 100644 index 00000000000..bb703b3acba --- /dev/null +++ b/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts @@ -0,0 +1,32 @@ +import { RabbitPayload, RabbitRPC } from '@golevelup/nestjs-rabbitmq'; +import { MikroORM, UseRequestContext } from '@mikro-orm/core'; +import { Injectable } from '@nestjs/common'; +import { LegacyLogger } from '@src/core/logger'; +import { FilesStorageEvents, FilesStorageExchange } from '@src/shared/infra/rabbitmq'; +import { PreviewFileOptions } from '../interface'; +import { PreviewGeneratorService } from '../service/preview-generator.service'; + +@Injectable() +export class PreviewGeneratorConsumer { + constructor( + private readonly previewGeneratorService: PreviewGeneratorService, + private logger: LegacyLogger, // eslint-disable-next-line @typescript-eslint/no-unused-vars + private readonly orm: MikroORM // don't remove it, we need it for @UseRequestContext + ) { + this.logger.setContext(PreviewGeneratorConsumer.name); + } + + @RabbitRPC({ + exchange: FilesStorageExchange, + routingKey: FilesStorageEvents.GENERATE_PREVIEW, + queue: FilesStorageEvents.GENERATE_PREVIEW, + }) + @UseRequestContext() + public async copyFilesOfParent(@RabbitPayload() payload: PreviewFileOptions) { + this.logger.debug({ action: 'generate preview', payload }); + + const response = await this.previewGeneratorService.generatePreview(payload); + + return { message: response }; + } +} diff --git a/apps/server/src/modules/files-storage/files-storage.module.ts b/apps/server/src/modules/files-storage/files-storage.module.ts index 29caa14e8c8..df10305f38b 100644 --- a/apps/server/src/modules/files-storage/files-storage.module.ts +++ b/apps/server/src/modules/files-storage/files-storage.module.ts @@ -13,6 +13,8 @@ import { FileRecord, FileRecordSecurityCheck } from './entity'; import { config, s3Config } from './files-storage.config'; import { FileRecordRepo } from './repo'; import { FilesStorageService } from './service/files-storage.service'; +import { PreviewGeneratorService } from './service/preview-generator.service'; +import { PreviewProducer } from './service/preview.producer'; import { PreviewService } from './service/preview.service'; const imports = [ @@ -26,7 +28,7 @@ const imports = [ }), S3ClientModule.register([s3Config]), ]; -const providers = [FilesStorageService, PreviewService, FileRecordRepo]; +const providers = [FilesStorageService, PreviewService, FileRecordRepo, PreviewGeneratorService, PreviewProducer]; const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = { findOneOrFailHandler: (entityName: string, where: Dictionary | IPrimaryKey) => @@ -51,6 +53,6 @@ const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = { }), ], providers, - exports: [FilesStorageService, PreviewService], + exports: [FilesStorageService, PreviewService, PreviewGeneratorService], }) export class FilesStorageModule {} diff --git a/apps/server/src/modules/files-storage/index.ts b/apps/server/src/modules/files-storage/index.ts index c22e4c2be98..e171dd0887e 100644 --- a/apps/server/src/modules/files-storage/index.ts +++ b/apps/server/src/modules/files-storage/index.ts @@ -3,3 +3,4 @@ export * from './files-storage-api.module'; export * from './files-storage-test.module'; // @deprecated remove after move api tests to modules export * from './files-storage.config'; export * from './files-storage.const'; +export * from './preview-generator-amqp.module'; diff --git a/apps/server/src/modules/files-storage/interface/interfaces.ts b/apps/server/src/modules/files-storage/interface/interfaces.ts index 047c943e55a..354e5682203 100644 --- a/apps/server/src/modules/files-storage/interface/interfaces.ts +++ b/apps/server/src/modules/files-storage/interface/interfaces.ts @@ -1,6 +1,7 @@ import { Readable } from 'stream'; -import type { DownloadFileParams, PreviewParams } from '../controller/dto'; +import type { PreviewParams } from '../controller/dto'; import { FileRecord } from '../entity'; +import { PreviewWidth } from './preview-width.enum'; export interface GetFileResponse { data: Readable; @@ -13,9 +14,26 @@ export interface GetFileResponse { export interface PreviewFileParams { fileRecord: FileRecord; - downloadParams: DownloadFileParams; previewParams: PreviewParams; hash: string; - filePath: string; + originFilePath: string; + previewFilePath: string; + format: string; bytesRange?: string; } + +export interface PreviewOptions { + format: string; + width?: PreviewWidth; +} + +export interface PreviewFileOptions { + originFilePath: string; + previewFilePath: string; + previewOptions: PreviewOptions; +} + +export interface PreviewResponseMessage { + previewFilePath: string; + status: boolean; +} diff --git a/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts b/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts new file mode 100644 index 00000000000..b325c040b34 --- /dev/null +++ b/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts @@ -0,0 +1,11 @@ +import { Module } from '@nestjs/common'; +import { CoreModule } from '@src/core'; +import { LoggerModule } from '@src/core/logger'; +import { PreviewGeneratorConsumer } from './controller/preview-generator.consumer'; +import { FilesStorageModule } from './files-storage.module'; + +@Module({ + imports: [FilesStorageModule, CoreModule, LoggerModule], + providers: [PreviewGeneratorConsumer], +}) +export class PreviewGeneratorAMQPModule {} diff --git a/apps/server/src/modules/files-storage/service/preview-generator.service.ts b/apps/server/src/modules/files-storage/service/preview-generator.service.ts new file mode 100644 index 00000000000..6c04bcd6708 --- /dev/null +++ b/apps/server/src/modules/files-storage/service/preview-generator.service.ts @@ -0,0 +1,54 @@ +import { Inject, Injectable } from '@nestjs/common'; +import { GetFile, S3ClientAdapter } from '@shared/infra/s3-client'; +import { LegacyLogger } from '@src/core/logger'; +import { subClass } from 'gm'; +import { PassThrough } from 'stream'; +import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; +import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from '../interface'; + +@Injectable() +export class PreviewGeneratorService { + constructor( + @Inject(FILES_STORAGE_S3_CONNECTION) private readonly storageClient: S3ClientAdapter, + private logger: LegacyLogger + ) { + this.logger.setContext(PreviewGeneratorService.name); + } + + public async generatePreview(params: PreviewFileOptions): Promise { + const { originFilePath, previewFilePath, previewOptions } = params; + + const original = await this.downloadOriginFile(originFilePath); + const preview = this.resizeAndConvert(original, previewOptions); + + const file = { data: preview, mimeType: previewOptions.format }; + await this.storageClient.create(previewFilePath, file); + + return { + previewFilePath, + status: true, + }; + } + + private async downloadOriginFile(pathToFile: string): Promise { + const file = await this.storageClient.get(pathToFile); + + return file; + } + + private resizeAndConvert(original: GetFile, previewParams: PreviewOptions): PassThrough { + const { format } = previewParams; + const im = subClass({ imageMagick: '7+' }); + + const preview = im(original.data); + const { width } = previewParams; + + if (width) { + preview.resize(width, undefined, '>'); + } + + const result = preview.stream(format); + + return result; + } +} diff --git a/apps/server/src/modules/files-storage/service/preview.producer.ts b/apps/server/src/modules/files-storage/service/preview.producer.ts new file mode 100644 index 00000000000..2d6d6b56e55 --- /dev/null +++ b/apps/server/src/modules/files-storage/service/preview.producer.ts @@ -0,0 +1,28 @@ +import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; +import { Injectable } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { FilesStorageEvents, FilesStorageExchange, RpcMessageProducer } from '@shared/infra/rabbitmq'; +import { LegacyLogger } from '@src/core/logger'; +import { IFileStorageConfig } from '../files-storage.config'; +import { PreviewFileOptions, PreviewResponseMessage } from '../interface'; + +@Injectable() +export class PreviewProducer extends RpcMessageProducer { + constructor( + protected readonly amqpConnection: AmqpConnection, + private readonly logger: LegacyLogger, + protected readonly configService: ConfigService + ) { + super(amqpConnection, FilesStorageExchange, configService.get('INCOMING_REQUEST_TIMEOUT')); + this.logger.setContext(PreviewProducer.name); + } + + async generate(payload: PreviewFileOptions): Promise { + this.logger.debug({ action: 'generate:started', payload }); + const response = await this.request(FilesStorageEvents.GENERATE_PREVIEW, payload); + + this.logger.debug({ action: 'generate:finished', payload }); + + return response; + } +} diff --git a/apps/server/src/modules/files-storage/service/preview.service.ts b/apps/server/src/modules/files-storage/service/preview.service.ts index 5ca2093351b..d51f0361e5f 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.ts @@ -1,49 +1,56 @@ import { Inject, Injectable, NotFoundException, UnprocessableEntityException } from '@nestjs/common'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { LegacyLogger } from '@src/core/logger'; -import { subClass } from 'gm'; -import { PassThrough } from 'stream'; -import { DownloadFileParams, PreviewParams } from '../controller/dto'; +import { PreviewParams } from '../controller/dto'; import { FileRecord, PreviewStatus } from '../entity'; import { ErrorType } from '../error'; import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; -import { createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; -import { GetFileResponse, PreviewFileParams } from '../interface'; +import { createPath, createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; +import { GetFileResponse, PreviewFileOptions, PreviewFileParams } from '../interface'; import { PreviewOutputMimeTypes } from '../interface/preview-output-mime-types.enum'; -import { FileDtoBuilder, FileResponseBuilder } from '../mapper'; -import { FilesStorageService } from './files-storage.service'; +import { FileResponseBuilder } from '../mapper'; +import { PreviewProducer } from './preview.producer'; @Injectable() export class PreviewService { constructor( @Inject(FILES_STORAGE_S3_CONNECTION) private readonly storageClient: S3ClientAdapter, - private readonly fileStorageService: FilesStorageService, - private logger: LegacyLogger + private logger: LegacyLogger, + private readonly previewProducer: PreviewProducer ) { this.logger.setContext(PreviewService.name); } public async getPreview( fileRecord: FileRecord, - downloadParams: DownloadFileParams, previewParams: PreviewParams, bytesRange?: string ): Promise { this.checkIfPreviewPossible(fileRecord); - const hash = createPreviewNameHash(fileRecord.id, previewParams); - const filePath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const { schoolId, id, mimeType } = fileRecord; + const originFilePath = createPath(schoolId, id); + const format = this.getFormat(previewParams.outputFormat ?? mimeType); - let response: GetFileResponse; + const hash = createPreviewNameHash(id, previewParams); + const previewFilePath = createPreviewFilePath(schoolId, hash, id); - const previewFileParams = { fileRecord, downloadParams, previewParams, hash, filePath, bytesRange }; + const previewFileParams = { + fileRecord, + previewParams, + hash, + previewFilePath, + originFilePath, + format, + bytesRange, + }; if (previewParams.forceUpdate) { - response = await this.generatePreview(previewFileParams); - } else { - response = await this.tryGetPreviewOrGenerate(previewFileParams); + await this.generatePreview(previewFileParams); } + const response = await this.tryGetPreviewOrGenerate(previewFileParams); + return response; } @@ -78,56 +85,31 @@ export class PreviewService { throw error; } - file = await this.generatePreview(params); + await this.generatePreview(params); + file = await this.getPreviewFile(params); } return file; } private async getPreviewFile(params: PreviewFileParams): Promise { - const { fileRecord, filePath, bytesRange, previewParams } = params; + const { fileRecord, previewFilePath, bytesRange, previewParams } = params; const name = this.getPreviewName(fileRecord, previewParams.outputFormat); - const file = await this.storageClient.get(filePath, bytesRange); + const file = await this.storageClient.get(previewFilePath, bytesRange); const response = FileResponseBuilder.build(file, name); return response; } - private async generatePreview(params: PreviewFileParams): Promise { - const { fileRecord, downloadParams, previewParams, hash, filePath, bytesRange } = params; - - const original = await this.fileStorageService.download(fileRecord, downloadParams, bytesRange); - const preview = this.resizeAndConvert(original, fileRecord, previewParams); - - const format = previewParams.outputFormat ?? fileRecord.mimeType; - const fileDto = FileDtoBuilder.build(hash, preview, format); - await this.storageClient.create(filePath, fileDto); - - const response = await this.getPreviewFile(params); - - return response; - } - - private resizeAndConvert( - original: GetFileResponse, - fileRecord: FileRecord, - previewParams: PreviewParams - ): PassThrough { - const mimeType = previewParams.outputFormat ?? fileRecord.mimeType; - const format = this.getFormat(mimeType); - const im = subClass({ imageMagick: '7+' }); - - const preview = im(original.data, fileRecord.name); - const { width } = previewParams; - - if (width) { - preview.resize(width, undefined, '>'); - } - - const result = preview.stream(format); + private async generatePreview(params: PreviewFileParams): Promise { + const payload: PreviewFileOptions = { + originFilePath: params.originFilePath, + previewFilePath: params.previewFilePath, + previewOptions: { width: params.previewParams.width, format: params.format }, + }; - return result; + await this.previewProducer.generate(payload); } private getFormat(mimeType: string): string { 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 fa6a27202de..26ad93bf310 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 @@ -153,7 +153,7 @@ export class FilesStorageUC { await this.checkPermission(userId, parentType, parentId, FileStorageAuthorizationContext.read); - const result = this.previewService.getPreview(fileRecord, params, previewParams, bytesRange); + const result = this.previewService.getPreview(fileRecord, previewParams, bytesRange); return result; } diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.module.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts b/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts index 080bcfebae5..edead16b836 100644 --- a/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts +++ b/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts @@ -7,6 +7,7 @@ export enum FilesStorageEvents { 'COPY_FILES_OF_PARENT' = 'copy-files-of-parent', 'LIST_FILES_OF_PARENT' = 'list-files-of-parent', 'DELETE_FILES_OF_PARENT' = 'delete-files-of-parent', + 'GENERATE_PREVIEW' = 'generate-preview', } export enum ScanStatus { diff --git a/apps/server/src/shared/infra/rabbitmq/index.ts b/apps/server/src/shared/infra/rabbitmq/index.ts index 0183c37b284..0b4a36d5824 100644 --- a/apps/server/src/shared/infra/rabbitmq/index.ts +++ b/apps/server/src/shared/infra/rabbitmq/index.ts @@ -1,3 +1,4 @@ export * from './exchange'; export * from './rabbitmq.module'; export * from './rpc-message'; +export * from './rpc-message-producer'; diff --git a/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts new file mode 100644 index 00000000000..db938aad770 --- /dev/null +++ b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts @@ -0,0 +1,37 @@ +import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; +import { ErrorMapper } from '@src/modules/files-storage-client/mapper'; +import { RpcMessage } from './rpc-message'; + +export abstract class RpcMessageProducer { + constructor( + protected readonly amqpConnection: AmqpConnection, + protected readonly exchange: string, + protected readonly timeout: number + ) {} + + protected async request(event: string, payload: unknown) { + const response = await this.amqpConnection.request>(this.createRequest(event, payload)); + + this.checkError(response); + return response.message || []; + } + + // need to be fixed with https://ticketsystem.dbildungscloud.de/browse/BC-2984 + // mapRpcErrorResponseToDomainError should also removed with this ticket + protected checkError(response: RpcMessage) { + const { error } = response; + if (error) { + const domainError = ErrorMapper.mapRpcErrorResponseToDomainError(error); + throw domainError; + } + } + + protected createRequest(event: string, payload: unknown) { + return { + exchange: this.exchange, + routingKey: event, + payload, + timeout: this.timeout, + }; + } +} diff --git a/apps/server/src/shared/infra/s3-client/interface/index.ts b/apps/server/src/shared/infra/s3-client/interface/index.ts index dc4b76ad922..ad6ed9c81da 100644 --- a/apps/server/src/shared/infra/s3-client/interface/index.ts +++ b/apps/server/src/shared/infra/s3-client/interface/index.ts @@ -24,6 +24,5 @@ export interface CopyFiles { export interface File { data: Readable; - name: string; mimeType: string; } diff --git a/nest-cli.json b/nest-cli.json index c92fcacf1cf..73dea03c093 100644 --- a/nest-cli.json +++ b/nest-cli.json @@ -63,6 +63,15 @@ "tsConfigPath": "apps/server/tsconfig.app.json" } }, + "preview-generator-amqp": { + "type": "application", + "root": "apps/server", + "entryFile": "apps/preview-generator-consumer.app", + "sourceRoot": "apps/server/src", + "compilerOptions": { + "tsConfigPath": "apps/server/tsconfig.app.json" + } + }, "fwu-learning-contents": { "type": "application", "root": "apps/server", diff --git a/package.json b/package.json index 99a99b681d4..b4b064d6613 100644 --- a/package.json +++ b/package.json @@ -61,11 +61,12 @@ "nest:start:management:dev": "nest start management --watch", "nest:start:management:debug": "nest start management --debug --watch", "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:files-storage:dev": "nest start files-storage --watch & nest start files-storage-amqp --watch", - "nest:start:files-storage:debug": "nest start files-storage --debug --watch & nest start files-storage-amqp --debug --watch", + "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: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", "nest:start:fwu-learning-contents": "nest start fwu-learning-contents", "nest:start:fwu-learning-contents:debug": "nest start fwu-learning-contents --debug --watch", "nest:start:fwu-learning-contents:prod": "node dist/apps/server/apps/fwu-learning-contents.app", From 46203c992b8ee19615cfef529e0adcf9201ebd00 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 5 Oct 2023 20:44:56 +0200 Subject: [PATCH 02/35] BC-5170 - wip v2 --- .../apps/preview-generator-consumer.app.ts | 2 - .../modules/files-storage/controller/index.ts | 1 - .../controller/preview-generator.consumer.ts | 32 -------------- .../files-preview-amqp.module.ts | 8 ++++ .../files-storage/files-storage.module.ts | 8 ++-- .../server/src/modules/files-storage/index.ts | 3 +- .../files-storage/interface/interfaces.ts | 17 -------- .../preview-generator-amqp.module.ts | 11 ----- .../files-storage/service/preview.service.ts | 6 +-- .../shared/infra/preview-generator/index.ts | 4 ++ .../preview-generator/interface/index.ts | 15 +++++++ .../preview-consumer.module.ts | 11 +++++ .../preview-generator-consumer.module.ts | 42 +++++++++++++++++++ .../preview-generator-producer.module.ts | 12 ++++++ .../preview-generator.consumer.ts | 26 ++++++++++++ .../preview-generator.module.ts | 0 .../preview-generator.service.ts | 10 ++--- .../preview-generator}/preview.producer.ts | 11 +++-- .../infra/rabbitmq/exchange/files-preview.ts | 7 ++++ .../infra/rabbitmq/exchange/files-storage.ts | 1 - .../shared/infra/rabbitmq/exchange/index.ts | 1 + .../shared/infra/rabbitmq/rabbitmq.module.ts | 5 +++ 22 files changed, 147 insertions(+), 86 deletions(-) delete mode 100644 apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts create mode 100644 apps/server/src/modules/files-storage/files-preview-amqp.module.ts delete mode 100644 apps/server/src/modules/files-storage/preview-generator-amqp.module.ts create mode 100644 apps/server/src/shared/infra/preview-generator/index.ts create mode 100644 apps/server/src/shared/infra/preview-generator/interface/index.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts delete mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.module.ts rename apps/server/src/{modules/files-storage/service => shared/infra/preview-generator}/preview-generator.service.ts (82%) rename apps/server/src/{modules/files-storage/service => shared/infra/preview-generator}/preview.producer.ts (67%) create mode 100644 apps/server/src/shared/infra/rabbitmq/exchange/files-preview.ts diff --git a/apps/server/src/apps/preview-generator-consumer.app.ts b/apps/server/src/apps/preview-generator-consumer.app.ts index f2abf69f5c9..17f94fbf4dd 100644 --- a/apps/server/src/apps/preview-generator-consumer.app.ts +++ b/apps/server/src/apps/preview-generator-consumer.app.ts @@ -1,8 +1,6 @@ /* istanbul ignore file */ /* eslint-disable no-console */ import { NestFactory } from '@nestjs/core'; - -// register source-map-support for debugging import { PreviewGeneratorAMQPModule } from '@src/modules/files-storage'; import { install as sourceMapInstall } from 'source-map-support'; diff --git a/apps/server/src/modules/files-storage/controller/index.ts b/apps/server/src/modules/files-storage/controller/index.ts index 9300d186f61..7aa61d2c93b 100644 --- a/apps/server/src/modules/files-storage/controller/index.ts +++ b/apps/server/src/modules/files-storage/controller/index.ts @@ -1,4 +1,3 @@ export * from './file-security.controller'; export * from './files-storage.consumer'; export * from './files-storage.controller'; -export * from './preview-generator.consumer'; diff --git a/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts b/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts deleted file mode 100644 index bb703b3acba..00000000000 --- a/apps/server/src/modules/files-storage/controller/preview-generator.consumer.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { RabbitPayload, RabbitRPC } from '@golevelup/nestjs-rabbitmq'; -import { MikroORM, UseRequestContext } from '@mikro-orm/core'; -import { Injectable } from '@nestjs/common'; -import { LegacyLogger } from '@src/core/logger'; -import { FilesStorageEvents, FilesStorageExchange } from '@src/shared/infra/rabbitmq'; -import { PreviewFileOptions } from '../interface'; -import { PreviewGeneratorService } from '../service/preview-generator.service'; - -@Injectable() -export class PreviewGeneratorConsumer { - constructor( - private readonly previewGeneratorService: PreviewGeneratorService, - private logger: LegacyLogger, // eslint-disable-next-line @typescript-eslint/no-unused-vars - private readonly orm: MikroORM // don't remove it, we need it for @UseRequestContext - ) { - this.logger.setContext(PreviewGeneratorConsumer.name); - } - - @RabbitRPC({ - exchange: FilesStorageExchange, - routingKey: FilesStorageEvents.GENERATE_PREVIEW, - queue: FilesStorageEvents.GENERATE_PREVIEW, - }) - @UseRequestContext() - public async copyFilesOfParent(@RabbitPayload() payload: PreviewFileOptions) { - this.logger.debug({ action: 'generate preview', payload }); - - const response = await this.previewGeneratorService.generatePreview(payload); - - return { message: response }; - } -} diff --git a/apps/server/src/modules/files-storage/files-preview-amqp.module.ts b/apps/server/src/modules/files-storage/files-preview-amqp.module.ts new file mode 100644 index 00000000000..4080d512e0a --- /dev/null +++ b/apps/server/src/modules/files-storage/files-preview-amqp.module.ts @@ -0,0 +1,8 @@ +import { Module } from '@nestjs/common'; +import { PreviewGeneratorConsumerModule } from '@shared/infra/preview-generator'; +import { s3Config } from './files-storage.config'; + +@Module({ + imports: [PreviewGeneratorConsumerModule.register(s3Config)], +}) +export class PreviewGeneratorAMQPModule {} diff --git a/apps/server/src/modules/files-storage/files-storage.module.ts b/apps/server/src/modules/files-storage/files-storage.module.ts index df10305f38b..9d8bb08c7a4 100644 --- a/apps/server/src/modules/files-storage/files-storage.module.ts +++ b/apps/server/src/modules/files-storage/files-storage.module.ts @@ -5,6 +5,7 @@ import { Module, NotFoundException } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; import { ALL_ENTITIES } from '@shared/domain'; import { AntivirusModule } from '@shared/infra/antivirus/antivirus.module'; +import { PreviewGeneratorProducerModule } from '@shared/infra/preview-generator'; import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq/rabbitmq.module'; import { S3ClientModule } from '@shared/infra/s3-client'; import { DB_PASSWORD, DB_URL, DB_USERNAME, createConfigModuleOptions } from '@src/config'; @@ -13,8 +14,6 @@ import { FileRecord, FileRecordSecurityCheck } from './entity'; import { config, s3Config } from './files-storage.config'; import { FileRecordRepo } from './repo'; import { FilesStorageService } from './service/files-storage.service'; -import { PreviewGeneratorService } from './service/preview-generator.service'; -import { PreviewProducer } from './service/preview.producer'; import { PreviewService } from './service/preview.service'; const imports = [ @@ -27,8 +26,9 @@ const imports = [ routingKey: Configuration.get('ANTIVIRUS_ROUTING_KEY') as string, }), S3ClientModule.register([s3Config]), + PreviewGeneratorProducerModule, ]; -const providers = [FilesStorageService, PreviewService, FileRecordRepo, PreviewGeneratorService, PreviewProducer]; +const providers = [FilesStorageService, PreviewService, FileRecordRepo]; const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = { findOneOrFailHandler: (entityName: string, where: Dictionary | IPrimaryKey) => @@ -53,6 +53,6 @@ const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = { }), ], providers, - exports: [FilesStorageService, PreviewService, PreviewGeneratorService], + exports: [FilesStorageService, PreviewService], }) export class FilesStorageModule {} diff --git a/apps/server/src/modules/files-storage/index.ts b/apps/server/src/modules/files-storage/index.ts index e171dd0887e..a1e2e9fdd19 100644 --- a/apps/server/src/modules/files-storage/index.ts +++ b/apps/server/src/modules/files-storage/index.ts @@ -1,6 +1,5 @@ +export * from './files-preview-amqp.module'; export * from './files-storage-amqp.module'; export * from './files-storage-api.module'; -export * from './files-storage-test.module'; // @deprecated remove after move api tests to modules export * from './files-storage.config'; export * from './files-storage.const'; -export * from './preview-generator-amqp.module'; diff --git a/apps/server/src/modules/files-storage/interface/interfaces.ts b/apps/server/src/modules/files-storage/interface/interfaces.ts index 354e5682203..2f288f9133b 100644 --- a/apps/server/src/modules/files-storage/interface/interfaces.ts +++ b/apps/server/src/modules/files-storage/interface/interfaces.ts @@ -1,7 +1,6 @@ import { Readable } from 'stream'; import type { PreviewParams } from '../controller/dto'; import { FileRecord } from '../entity'; -import { PreviewWidth } from './preview-width.enum'; export interface GetFileResponse { data: Readable; @@ -21,19 +20,3 @@ export interface PreviewFileParams { format: string; bytesRange?: string; } - -export interface PreviewOptions { - format: string; - width?: PreviewWidth; -} - -export interface PreviewFileOptions { - originFilePath: string; - previewFilePath: string; - previewOptions: PreviewOptions; -} - -export interface PreviewResponseMessage { - previewFilePath: string; - status: boolean; -} diff --git a/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts b/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts deleted file mode 100644 index b325c040b34..00000000000 --- a/apps/server/src/modules/files-storage/preview-generator-amqp.module.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Module } from '@nestjs/common'; -import { CoreModule } from '@src/core'; -import { LoggerModule } from '@src/core/logger'; -import { PreviewGeneratorConsumer } from './controller/preview-generator.consumer'; -import { FilesStorageModule } from './files-storage.module'; - -@Module({ - imports: [FilesStorageModule, CoreModule, LoggerModule], - providers: [PreviewGeneratorConsumer], -}) -export class PreviewGeneratorAMQPModule {} diff --git a/apps/server/src/modules/files-storage/service/preview.service.ts b/apps/server/src/modules/files-storage/service/preview.service.ts index d51f0361e5f..86d9e9c6b07 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.ts @@ -1,4 +1,5 @@ import { Inject, Injectable, NotFoundException, UnprocessableEntityException } from '@nestjs/common'; +import { PreviewProducer } from '@shared/infra/preview-generator'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { LegacyLogger } from '@src/core/logger'; import { PreviewParams } from '../controller/dto'; @@ -6,10 +7,9 @@ import { FileRecord, PreviewStatus } from '../entity'; import { ErrorType } from '../error'; import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; import { createPath, createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; -import { GetFileResponse, PreviewFileOptions, PreviewFileParams } from '../interface'; +import { GetFileResponse, PreviewFileParams } from '../interface'; import { PreviewOutputMimeTypes } from '../interface/preview-output-mime-types.enum'; import { FileResponseBuilder } from '../mapper'; -import { PreviewProducer } from './preview.producer'; @Injectable() export class PreviewService { @@ -103,7 +103,7 @@ export class PreviewService { } private async generatePreview(params: PreviewFileParams): Promise { - const payload: PreviewFileOptions = { + const payload = { originFilePath: params.originFilePath, previewFilePath: params.previewFilePath, previewOptions: { width: params.previewParams.width, format: params.format }, diff --git a/apps/server/src/shared/infra/preview-generator/index.ts b/apps/server/src/shared/infra/preview-generator/index.ts new file mode 100644 index 00000000000..570b38cc9bd --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/index.ts @@ -0,0 +1,4 @@ +export * from './interface'; +export * from './preview-generator-consumer.module'; +export * from './preview-generator-producer.module'; +export * from './preview.producer'; diff --git a/apps/server/src/shared/infra/preview-generator/interface/index.ts b/apps/server/src/shared/infra/preview-generator/interface/index.ts new file mode 100644 index 00000000000..92ab2808151 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/interface/index.ts @@ -0,0 +1,15 @@ +export interface PreviewOptions { + format: string; + width?: number; +} + +export interface PreviewFileOptions { + originFilePath: string; + previewFilePath: string; + previewOptions: PreviewOptions; +} + +export interface PreviewResponseMessage { + previewFilePath: string; + status: boolean; +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts new file mode 100644 index 00000000000..e008b88851e --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts @@ -0,0 +1,11 @@ +import { Module } from '@nestjs/common'; +import { LoggerModule } from '@src/core/logger'; +import { PreviewGeneratorConsumer } from './preview-generator.consumer'; +import { PreviewGeneratorService } from './preview-generator.service'; + +const providers = [PreviewGeneratorConsumer, PreviewGeneratorService]; +@Module({ + imports: [LoggerModule], + providers, +}) +export class PreviewGeneratorConsumerModule {} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts new file mode 100644 index 00000000000..20e78a52e19 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts @@ -0,0 +1,42 @@ +import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { DynamicModule, Module } from '@nestjs/common'; +import { ConfigModule } from '@nestjs/config'; +import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq'; +import { S3ClientAdapter, S3ClientModule, S3Config } from '@shared/infra/s3-client'; +import { createConfigModuleOptions } from '@src/config'; +import { LegacyLogger, LoggerModule } from '@src/core/logger'; +import { PreviewGeneratorConsumer } from './preview-generator.consumer'; +import { PreviewGeneratorService } from './preview-generator.service'; + +const serverConfig = { + NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, + INCOMING_REQUEST_TIMEOUT: Configuration.get('FILES_STORAGE__INCOMING_REQUEST_TIMEOUT') as number, +}; + +export const config = () => serverConfig; + +@Module({}) +export class PreviewGeneratorConsumerModule { + static register(s3Config: S3Config): DynamicModule { + const providers = [ + { + provide: PreviewGeneratorService, + useFactory: (logger: LegacyLogger, storageClient: S3ClientAdapter) => + new PreviewGeneratorService(storageClient, logger), + inject: [LegacyLogger, s3Config.connectionName], + }, + PreviewGeneratorConsumer, + ]; + + return { + module: PreviewGeneratorConsumerModule, + imports: [ + LoggerModule, + S3ClientModule.register([s3Config]), + RabbitMQWrapperModule, + ConfigModule.forRoot(createConfigModuleOptions(config)), + ], + providers, + }; + } +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts new file mode 100644 index 00000000000..d91ec75c8f9 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts @@ -0,0 +1,12 @@ +import { Module } from '@nestjs/common'; +import { LoggerModule } from '@src/core/logger'; +import { RabbitMQWrapperModule } from '../rabbitmq'; +import { PreviewProducer } from './preview.producer'; + +const providers = [PreviewProducer]; +@Module({ + imports: [LoggerModule, RabbitMQWrapperModule], + providers, + exports: providers, +}) +export class PreviewGeneratorProducerModule {} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts new file mode 100644 index 00000000000..b8fafe123ed --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts @@ -0,0 +1,26 @@ +import { RabbitPayload, RabbitRPC } from '@golevelup/nestjs-rabbitmq'; +import { Injectable } from '@nestjs/common'; +import { LegacyLogger } from '@src/core/logger'; +import { FilesPreviewEvents, FilesPreviewExchange } from '@src/shared/infra/rabbitmq'; +import { PreviewFileOptions } from './interface'; +import { PreviewGeneratorService } from './preview-generator.service'; + +@Injectable() +export class PreviewGeneratorConsumer { + constructor(private readonly previewGeneratorService: PreviewGeneratorService, private logger: LegacyLogger) { + this.logger.setContext(PreviewGeneratorConsumer.name); + } + + @RabbitRPC({ + exchange: FilesPreviewExchange, + routingKey: FilesPreviewEvents.GENERATE_PREVIEW, + queue: FilesPreviewEvents.GENERATE_PREVIEW, + }) + public async copyFilesOfParent(@RabbitPayload() payload: PreviewFileOptions) { + this.logger.debug({ action: 'generate preview', payload }); + + const response = await this.previewGeneratorService.generatePreview(payload); + + return { message: response }; + } +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.module.ts deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/apps/server/src/modules/files-storage/service/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts similarity index 82% rename from apps/server/src/modules/files-storage/service/preview-generator.service.ts rename to apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index 6c04bcd6708..2aff5e5794b 100644 --- a/apps/server/src/modules/files-storage/service/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -1,17 +1,13 @@ -import { Inject, Injectable } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { GetFile, S3ClientAdapter } from '@shared/infra/s3-client'; import { LegacyLogger } from '@src/core/logger'; import { subClass } from 'gm'; import { PassThrough } from 'stream'; -import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; -import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from '../interface'; +import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from './interface'; @Injectable() export class PreviewGeneratorService { - constructor( - @Inject(FILES_STORAGE_S3_CONNECTION) private readonly storageClient: S3ClientAdapter, - private logger: LegacyLogger - ) { + constructor(private readonly storageClient: S3ClientAdapter, private logger: LegacyLogger) { this.logger.setContext(PreviewGeneratorService.name); } diff --git a/apps/server/src/modules/files-storage/service/preview.producer.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.ts similarity index 67% rename from apps/server/src/modules/files-storage/service/preview.producer.ts rename to apps/server/src/shared/infra/preview-generator/preview.producer.ts index 2d6d6b56e55..748e821fd4f 100644 --- a/apps/server/src/modules/files-storage/service/preview.producer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.ts @@ -1,25 +1,24 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; -import { FilesStorageEvents, FilesStorageExchange, RpcMessageProducer } from '@shared/infra/rabbitmq'; +import { FilesPreviewEvents, FilesPreviewExchange, RpcMessageProducer } from '@shared/infra/rabbitmq'; import { LegacyLogger } from '@src/core/logger'; -import { IFileStorageConfig } from '../files-storage.config'; -import { PreviewFileOptions, PreviewResponseMessage } from '../interface'; +import { PreviewFileOptions, PreviewResponseMessage } from './interface'; @Injectable() export class PreviewProducer extends RpcMessageProducer { constructor( protected readonly amqpConnection: AmqpConnection, private readonly logger: LegacyLogger, - protected readonly configService: ConfigService + protected readonly configService: ConfigService ) { - super(amqpConnection, FilesStorageExchange, configService.get('INCOMING_REQUEST_TIMEOUT')); + super(amqpConnection, FilesPreviewExchange, configService.get('INCOMING_REQUEST_TIMEOUT')); this.logger.setContext(PreviewProducer.name); } async generate(payload: PreviewFileOptions): Promise { this.logger.debug({ action: 'generate:started', payload }); - const response = await this.request(FilesStorageEvents.GENERATE_PREVIEW, payload); + const response = await this.request(FilesPreviewEvents.GENERATE_PREVIEW, payload); this.logger.debug({ action: 'generate:finished', payload }); diff --git a/apps/server/src/shared/infra/rabbitmq/exchange/files-preview.ts b/apps/server/src/shared/infra/rabbitmq/exchange/files-preview.ts new file mode 100644 index 00000000000..0bab7491e07 --- /dev/null +++ b/apps/server/src/shared/infra/rabbitmq/exchange/files-preview.ts @@ -0,0 +1,7 @@ +import { Configuration } from '@hpi-schul-cloud/commons/lib'; + +export const FilesPreviewExchange = Configuration.get('FILES_STORAGE__EXCHANGE') as string; + +export enum FilesPreviewEvents { + 'GENERATE_PREVIEW' = 'generate-preview', +} diff --git a/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts b/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts index edead16b836..080bcfebae5 100644 --- a/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts +++ b/apps/server/src/shared/infra/rabbitmq/exchange/files-storage.ts @@ -7,7 +7,6 @@ export enum FilesStorageEvents { 'COPY_FILES_OF_PARENT' = 'copy-files-of-parent', 'LIST_FILES_OF_PARENT' = 'list-files-of-parent', 'DELETE_FILES_OF_PARENT' = 'delete-files-of-parent', - 'GENERATE_PREVIEW' = 'generate-preview', } export enum ScanStatus { diff --git a/apps/server/src/shared/infra/rabbitmq/exchange/index.ts b/apps/server/src/shared/infra/rabbitmq/exchange/index.ts index f0d84c9e1a1..0cf6bd00d13 100644 --- a/apps/server/src/shared/infra/rabbitmq/exchange/index.ts +++ b/apps/server/src/shared/infra/rabbitmq/exchange/index.ts @@ -1 +1,2 @@ +export * from './files-preview'; export * from './files-storage'; diff --git a/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts b/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts index 2d73162e6e3..572c5356f91 100644 --- a/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts +++ b/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts @@ -2,6 +2,7 @@ import { AmqpConnectionManager, RabbitMQModule } from '@golevelup/nestjs-rabbitm import { Configuration } from '@hpi-schul-cloud/commons'; import { Global, Module, OnModuleDestroy } from '@nestjs/common'; import { FilesStorageExchange } from './exchange'; +import { FilesPreviewExchange } from './exchange/files-preview'; /** * https://www.npmjs.com/package/@golevelup/nestjs-rabbitmq#usage @@ -28,6 +29,10 @@ const imports = [ name: FilesStorageExchange, type: 'direct', }, + { + name: FilesPreviewExchange, + type: 'direct', + }, ], uri: Configuration.get('RABBITMQ_URI') as string, }), From 2c737160781feed41ccce4a534c770f7d5f0488a Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Mon, 9 Oct 2023 12:20:00 +0200 Subject: [PATCH 03/35] Adjust tests in file storage service and uc --- .../service/preview.service.spec.ts | 767 +++++++----------- .../files-storage-download-preview.uc.spec.ts | 7 +- 2 files changed, 293 insertions(+), 481 deletions(-) diff --git a/apps/server/src/modules/files-storage/service/preview.service.spec.ts b/apps/server/src/modules/files-storage/service/preview.service.spec.ts index ed4592b97da..685b54f52a3 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.spec.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.spec.ts @@ -2,6 +2,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { NotFoundException, UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; +import { PreviewProducer } from '@shared/infra/preview-generator'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; @@ -10,11 +11,11 @@ import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType, ScanStatus } from '../entity'; import { ErrorType } from '../error'; import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; -import { createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; +import { createPath, createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; import { TestHelper } from '../helper/test-helper'; import { PreviewWidth } from '../interface'; import { PreviewOutputMimeTypes } from '../interface/preview-output-mime-types.enum'; -import { FileDtoBuilder, FileResponseBuilder } from '../mapper'; +import { FileResponseBuilder } from '../mapper'; import { FilesStorageService } from './files-storage.service'; import { PreviewService } from './preview.service'; @@ -62,8 +63,8 @@ const defaultPreviewParamsWithWidth = { describe('PreviewService', () => { let module: TestingModule; let previewService: PreviewService; - let fileStorageService: DeepMocked; let s3ClientAdapter: DeepMocked; + let previewProducer: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -83,514 +84,369 @@ describe('PreviewService', () => { provide: LegacyLogger, useValue: createMock(), }, + { provide: PreviewProducer, useValue: createMock() }, ], }).compile(); previewService = module.get(PreviewService); - fileStorageService = module.get(FilesStorageService); s3ClientAdapter = module.get(FILES_STORAGE_S3_CONNECTION); - }); - - beforeEach(() => { - jest.resetAllMocks(); + previewProducer = module.get(PreviewProducer); }); afterAll(async () => { await module.close(); }); + afterEach(() => { + jest.resetAllMocks(); + }); + describe('getPreview is called', () => { describe('WHEN preview is possbile', () => { describe('WHEN forceUpdate is true', () => { - describe('WHEN width and outputformat are not set', () => { - describe('WHEN download of original and preview file is successfull', () => { - const setup = () => { - const bytesRange = 'bytes=0-100'; - const orignalMimeType = 'image/png'; - const format = orignalMimeType.split('/')[1]; - const { fileRecord } = buildFileRecordWithParams(orignalMimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { forceUpdate: true }; - - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - - const previewFile = TestHelper.createFile(); - s3ClientAdapter.get.mockResolvedValueOnce(previewFile); - - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; - const name = `${fileNameWithoutExtension}.${format}`; - const previewFileResponse = FileResponseBuilder.build(previewFile, name); - - const hash = createPreviewNameHash(fileRecord.id, {}); - const previewFileDto = FileDtoBuilder.build(hash, previewFile.data, orignalMimeType); - const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); - streamMock.mockClear(); - streamMock.mockReturnValueOnce(previewFileDto.data); - - return { - bytesRange, - fileRecord, - downloadParams, - previewParams, - format, - previewFileDto, - previewPath, - previewFileResponse, - }; + describe('WHEN first get of preview file is successfull', () => { + const setup = () => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: true, }; - - it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange); - - expect(fileStorageService.download).toHaveBeenCalledWith(fileRecord, downloadParams, bytesRange); - }); - - it('calls image magicks stream method', async () => { - const { fileRecord, downloadParams, previewParams, format } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(streamMock).toHaveBeenCalledWith(format); - expect(streamMock).toHaveBeenCalledTimes(1); - }); - - it('calls S3ClientAdapters create method', async () => { - const { fileRecord, downloadParams, previewParams, previewFileDto, previewPath } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(s3ClientAdapter.create).toHaveBeenCalledWith(previewPath, previewFileDto); - expect(s3ClientAdapter.create).toHaveBeenCalledTimes(1); - }); - - it('calls S3ClientAdapters get method', async () => { - const { fileRecord, downloadParams, previewParams, previewPath } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); - expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); - }); - - it('returns preview file response', async () => { - const { fileRecord, downloadParams, previewParams, previewFileResponse } = setup(); - - const response = await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(response).toEqual(previewFileResponse); - }); - }); - - describe('WHEN download of original file throws error', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { forceUpdate: true }; - - const error = new Error('testError'); - fileStorageService.download.mockRejectedValueOnce(error); - - return { fileRecord, downloadParams, previewParams, error }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + s3ClientAdapter.get.mockResolvedValueOnce(previewFile); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, }; + }; - it('passes error', async () => { - const { fileRecord, downloadParams, previewParams, error } = setup(); - - await expect(previewService.getPreview(fileRecord, downloadParams, previewParams)).rejects.toThrowError( - error - ); - }); - }); - - describe('WHEN create of preview file throws error', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { forceUpdate: true }; - - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - - const error = new Error('testError'); - s3ClientAdapter.create.mockRejectedValueOnce(error); - - return { fileRecord, downloadParams, previewParams, error }; - }; + it('calls previewProducer.generate with correct params', async () => { + const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = setup(); - it('passes error', async () => { - const { fileRecord, downloadParams, previewParams, error } = setup(); + await previewService.getPreview(fileRecord, previewParams, bytesRange); - await expect(previewService.getPreview(fileRecord, downloadParams, previewParams)).rejects.toThrowError( - error - ); + expect(previewProducer.generate).toHaveBeenCalledWith({ + originFilePath: originPath, + previewFilePath: previewPath, + previewOptions: { width: previewParams.width, format }, }); }); - describe('WHEN get of preview file throws error', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { forceUpdate: true }; + it('calls S3ClientAdapters get method', async () => { + const { fileRecord, previewParams, previewPath } = setup(); - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); + await previewService.getPreview(fileRecord, previewParams); - const error = new Error('testError'); - s3ClientAdapter.get.mockRejectedValueOnce(error); + expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); + expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); + }); - return { fileRecord, downloadParams, previewParams, error }; - }; + it('returns preview file response', async () => { + const { fileRecord, previewParams, previewFileResponse } = setup(); - it('passes error', async () => { - const { fileRecord, downloadParams, previewParams, error } = setup(); + const response = await previewService.getPreview(fileRecord, previewParams); - await expect(previewService.getPreview(fileRecord, downloadParams, previewParams)).rejects.toThrowError( - error - ); - }); + expect(response).toEqual(previewFileResponse); }); }); - describe('WHEN width and outputFormat are set', () => { - describe('WHEN download of original and preview file is successfull', () => { - const setup = () => { - const bytesRange = 'bytes=0-100'; - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { - ...defaultPreviewParamsWithWidth, - forceUpdate: true, - }; - const format = previewParams.outputFormat.split('/')[1]; - - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - - const previewFile = TestHelper.createFile(); - s3ClientAdapter.get.mockResolvedValueOnce(previewFile); - - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; - const name = `${fileNameWithoutExtension}.${format}`; - const previewFileResponse = FileResponseBuilder.build(previewFile, name); - - const hash = createPreviewNameHash(fileRecord.id, previewParams); - const previewFileDto = FileDtoBuilder.build(hash, previewFile.data, previewParams.outputFormat); - const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); - - streamMock.mockClear(); - streamMock.mockReturnValueOnce(previewFileDto.data); - - resizeMock.mockClear(); - - return { - bytesRange, - fileRecord, - downloadParams, - previewParams, - format, - previewFileDto, - previewPath, - previewFileResponse, - }; + describe('WHEN first get of preview file throws error and second is successfull', () => { + const setup = (error: Error | NotFoundException) => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: true, }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + s3ClientAdapter.get.mockRejectedValueOnce(error).mockResolvedValueOnce(previewFile); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, + }; + }; - it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange); - - expect(fileStorageService.download).toHaveBeenCalledWith(fileRecord, downloadParams, bytesRange); - }); - - it('calls image magicks resize method', async () => { - const { fileRecord, downloadParams, previewParams } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(resizeMock).toHaveBeenCalledWith(previewParams.width, undefined, '>'); - expect(resizeMock).toHaveBeenCalledTimes(1); - }); - - it('calls image magicks stream method', async () => { - const { fileRecord, downloadParams, previewParams, format } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(streamMock).toHaveBeenCalledWith(format); - expect(streamMock).toHaveBeenCalledTimes(1); - }); - - it('calls S3ClientAdapters create method', async () => { - const { fileRecord, downloadParams, previewParams, previewFileDto, previewPath } = setup(); + describe('WHEN error is a NotFoundException', () => { + it('calls previewProducer.generate with correct params', async () => { + const notFoundException = new NotFoundException(); + const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = + setup(notFoundException); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams, bytesRange); - expect(s3ClientAdapter.create).toHaveBeenCalledWith(previewPath, previewFileDto); - expect(s3ClientAdapter.create).toHaveBeenCalledTimes(1); + expect(previewProducer.generate).toHaveBeenCalledWith({ + originFilePath: originPath, + previewFilePath: previewPath, + previewOptions: { width: previewParams.width, format }, + }); + expect(previewProducer.generate).toHaveBeenCalledTimes(2); }); it('calls S3ClientAdapters get method', async () => { - const { fileRecord, downloadParams, previewParams, previewPath } = setup(); + const notFoundException = new NotFoundException(); + const { fileRecord, previewParams, previewPath } = setup(notFoundException); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); - expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); + expect(s3ClientAdapter.get).toHaveBeenCalledTimes(2); }); + }); - it('returns preview file response', async () => { - const { fileRecord, downloadParams, previewParams, previewFileResponse } = setup(); - - const response = await previewService.getPreview(fileRecord, downloadParams, previewParams); + describe('WHEN error is other error', () => { + it('should pass error', async () => { + const error = new Error('testError'); + const { fileRecord, previewParams } = setup(error); - expect(response).toEqual(previewFileResponse); + await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(error); }); }); + }); - describe('WHEN download of original file throws error', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { ...defaultPreviewParams, forceUpdate: true }; - - const error = new Error('testError'); - fileStorageService.download.mockRejectedValueOnce(error); - - return { fileRecord, downloadParams, previewParams, error }; + describe('WHEN both gets of preview file throw error', () => { + const setup = () => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: true, + }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + const notFoundException = new NotFoundException(); + s3ClientAdapter.get.mockRejectedValueOnce(notFoundException).mockRejectedValueOnce(notFoundException); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, }; + }; - it('passes error', async () => { - const { fileRecord, downloadParams, previewParams, error } = setup(); + it('should pass error', async () => { + const { fileRecord, previewParams } = setup(); - await expect(previewService.getPreview(fileRecord, downloadParams, previewParams)).rejects.toThrowError( - error - ); - }); + await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(); }); }); }); describe('WHEN forceUpdate is false', () => { - describe('WHEN width and outputFormat are set', () => { - describe('WHEN S3ClientAdapter get returns already stored preview file', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { - ...defaultPreviewParamsWithWidth, - }; - const format = previewParams.outputFormat.split('/')[1]; - - const previewFile = TestHelper.createFile(); - s3ClientAdapter.get.mockResolvedValueOnce(previewFile); - - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; - const name = `${fileNameWithoutExtension}.${format}`; - const previewFileResponse = FileResponseBuilder.build(previewFile, name); - - const hash = createPreviewNameHash(fileRecord.id, previewParams); - const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); - - resizeMock.mockClear(); - streamMock.mockClear(); - - return { - fileRecord, - downloadParams, - previewParams, - previewPath, - previewFileResponse, - }; + describe('WHEN first get of preview file is successfull', () => { + const setup = () => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: false, + }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + s3ClientAdapter.get.mockResolvedValueOnce(previewFile); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, }; + }; - it('calls S3ClientAdapters get method', async () => { - const { fileRecord, downloadParams, previewParams, previewPath } = setup(); + it('calls S3ClientAdapters get method', async () => { + const { fileRecord, previewParams, previewPath } = setup(); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams); - expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); - expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); - }); + expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); + expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); + }); - it('returns preview file response', async () => { - const { fileRecord, downloadParams, previewParams, previewFileResponse } = setup(); + it('returns preview file response', async () => { + const { fileRecord, previewParams, previewFileResponse } = setup(); - const response = await previewService.getPreview(fileRecord, downloadParams, previewParams); + const response = await previewService.getPreview(fileRecord, previewParams); - expect(response).toEqual(previewFileResponse); - }); + expect(response).toEqual(previewFileResponse); + }); - it('does not call image magicks resize and stream method', async () => { - const { fileRecord, downloadParams, previewParams } = setup(); + it('does not call generate', async () => { + const { fileRecord, previewParams, bytesRange } = setup(); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams, bytesRange); - expect(resizeMock).not.toHaveBeenCalled(); - expect(streamMock).not.toHaveBeenCalled(); - }); + expect(previewProducer.generate).toHaveBeenCalledTimes(0); }); + }); - describe('WHEN S3ClientAdapter get throws NotFoundException', () => { - const setup = () => { - const bytesRange = 'bytes=0-100'; - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { - ...defaultPreviewParamsWithWidth, - }; - const format = previewParams.outputFormat.split('/')[1]; - - const error = new NotFoundException(); - s3ClientAdapter.get.mockRejectedValueOnce(error); - - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - - const previewFile = TestHelper.createFile(); - s3ClientAdapter.get.mockResolvedValueOnce(previewFile); - - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; - const name = `${fileNameWithoutExtension}.${format}`; - const previewFileResponse = FileResponseBuilder.build(previewFile, name); - - const hash = createPreviewNameHash(fileRecord.id, previewParams); - const previewFileDto = FileDtoBuilder.build(hash, previewFile.data, previewParams.outputFormat); - const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); - - streamMock.mockClear(); - streamMock.mockReturnValueOnce(previewFileDto.data); - - resizeMock.mockClear(); - - return { - bytesRange, - fileRecord, - downloadParams, - previewParams, - format, - previewFileDto, - previewPath, - previewFileResponse, - }; + describe('WHEN first get of preview file throws error and second is successfull', () => { + const setup = (error: Error | NotFoundException) => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: false, }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + s3ClientAdapter.get.mockRejectedValueOnce(error).mockResolvedValueOnce(previewFile); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, + }; + }; - it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange); - - expect(fileStorageService.download).toHaveBeenCalledWith(fileRecord, downloadParams, bytesRange); - }); - - it('calls image magicks resize method', async () => { - const { fileRecord, downloadParams, previewParams } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(resizeMock).toHaveBeenCalledWith(previewParams.width, undefined, '>'); - expect(resizeMock).toHaveBeenCalledTimes(1); - }); - - it('calls image magicks stream method', async () => { - const { fileRecord, downloadParams, previewParams, format } = setup(); - - await previewService.getPreview(fileRecord, downloadParams, previewParams); - - expect(streamMock).toHaveBeenCalledWith(format); - expect(streamMock).toHaveBeenCalledTimes(1); - }); - - it('calls S3ClientAdapters create method', async () => { - const { fileRecord, downloadParams, previewParams, previewFileDto, previewPath } = setup(); + describe('WHEN error is a NotFoundException', () => { + it('calls previewProducer.generate with correct params', async () => { + const notFoundException = new NotFoundException(); + const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = + setup(notFoundException); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams, bytesRange); - expect(s3ClientAdapter.create).toHaveBeenCalledWith(previewPath, previewFileDto); - expect(s3ClientAdapter.create).toHaveBeenCalledTimes(1); + expect(previewProducer.generate).toHaveBeenCalledWith({ + originFilePath: originPath, + previewFilePath: previewPath, + previewOptions: { width: previewParams.width, format }, + }); + expect(previewProducer.generate).toHaveBeenCalledTimes(1); }); it('calls S3ClientAdapters get method', async () => { - const { fileRecord, downloadParams, previewParams, previewPath } = setup(); + const notFoundException = new NotFoundException(); + const { fileRecord, previewParams, previewPath } = setup(notFoundException); - await previewService.getPreview(fileRecord, downloadParams, previewParams); + await previewService.getPreview(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); expect(s3ClientAdapter.get).toHaveBeenCalledTimes(2); }); + }); - it('returns preview file response', async () => { - const { fileRecord, downloadParams, previewParams, previewFileResponse } = setup(); - - const response = await previewService.getPreview(fileRecord, downloadParams, previewParams); + describe('WHEN error is other error', () => { + it('should pass error', async () => { + const error = new Error('testError'); + const { fileRecord, previewParams } = setup(error); - expect(response).toEqual(previewFileResponse); + await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(error); }); }); + }); - describe('WHEN S3ClientAdapter get throws other than NotFoundException', () => { - const setup = () => { - const mimeType = 'image/png'; - const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { - ...defaultPreviewParamsWithWidth, - }; - const format = previewParams.outputFormat.split('/')[1]; - - const error = new Error('testError'); - s3ClientAdapter.get.mockRejectedValueOnce(error); - - return { - fileRecord, - downloadParams, - previewParams, - format, - error, - }; + describe('WHEN both gets of preview file throw error', () => { + const setup = () => { + const bytesRange = 'bytes=0-100'; + const mimeType = 'image/png'; + const { fileRecord } = buildFileRecordWithParams(mimeType); + const previewParams = { + ...defaultPreviewParamsWithWidth, + forceUpdate: true, }; + const format = previewParams.outputFormat.split('/')[1]; + + const previewFile = TestHelper.createFile(); + const notFoundException = new NotFoundException(); + s3ClientAdapter.get.mockRejectedValueOnce(notFoundException).mockRejectedValueOnce(notFoundException); + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const name = `${fileNameWithoutExtension}.${format}`; + const previewFileResponse = FileResponseBuilder.build(previewFile, name); + + const hash = createPreviewNameHash(fileRecord.id, previewParams); + const previewPath = createPreviewFilePath(fileRecord.getSchoolId(), hash, fileRecord.id); + const originPath = createPath(fileRecord.getSchoolId(), fileRecord.id); + + return { + bytesRange, + fileRecord, + previewParams, + format, + previewPath, + originPath, + previewFileResponse, + }; + }; - it('passes error', async () => { - const { fileRecord, downloadParams, previewParams, error } = setup(); + it('should pass error', async () => { + const { fileRecord, previewParams } = setup(); - await expect(previewService.getPreview(fileRecord, downloadParams, previewParams)).rejects.toThrow(error); - }); + await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(); }); }); }); @@ -603,33 +459,23 @@ describe('PreviewService', () => { const mimeType = 'application/zip'; const format = mimeType.split('/')[1]; const { fileRecord } = buildFileRecordWithParams(mimeType); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; const previewParams = { ...defaultPreviewParams, forceUpdate: true }; - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - const error = new UnprocessableEntityException(ErrorType.PREVIEW_NOT_POSSIBLE); return { bytesRange, fileRecord, - downloadParams, previewParams, format, error, }; }; - it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange, error } = setup(); + it('passes error', async () => { + const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect( - previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange) - ).rejects.toThrowError(error); + await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -639,33 +485,23 @@ describe('PreviewService', () => { const mimeType = 'image/png'; const format = mimeType.split('/')[1]; const { fileRecord } = buildFileRecordWithParams(mimeType, ScanStatus.PENDING); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; const previewParams = { ...defaultPreviewParams, forceUpdate: true }; - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - const error = new UnprocessableEntityException(ErrorType.PREVIEW_NOT_POSSIBLE); return { bytesRange, fileRecord, - downloadParams, previewParams, format, error, }; }; - it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange, error } = setup(); + it('passes error', async () => { + const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect( - previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange) - ).rejects.toThrowError(error); + await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -675,21 +511,14 @@ describe('PreviewService', () => { const mimeType = 'image/png'; const format = mimeType.split('/')[1]; const { fileRecord } = buildFileRecordWithParams(mimeType, ScanStatus.ERROR); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; - const previewParams = { ...defaultPreviewParams, forceUpdate: true }; - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); + const previewParams = { ...defaultPreviewParams, forceUpdate: true }; const error = new UnprocessableEntityException(ErrorType.PREVIEW_NOT_POSSIBLE); return { bytesRange, fileRecord, - downloadParams, previewParams, format, error, @@ -697,11 +526,9 @@ describe('PreviewService', () => { }; it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange, error } = setup(); + const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect( - previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange) - ).rejects.toThrowError(error); + await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -711,21 +538,13 @@ describe('PreviewService', () => { const mimeType = 'image/png'; const format = mimeType.split('/')[1]; const { fileRecord } = buildFileRecordWithParams(mimeType, ScanStatus.BLOCKED); - const downloadParams = { - fileRecordId: fileRecord.id, - fileName: fileRecord.name, - }; const previewParams = { ...defaultPreviewParams, forceUpdate: true }; - const originalFileResponse = TestHelper.createFileResponse(); - fileStorageService.download.mockResolvedValueOnce(originalFileResponse); - const error = new UnprocessableEntityException(ErrorType.PREVIEW_NOT_POSSIBLE); return { bytesRange, fileRecord, - downloadParams, previewParams, format, error, @@ -733,11 +552,9 @@ describe('PreviewService', () => { }; it('calls download with correct params', async () => { - const { fileRecord, downloadParams, previewParams, bytesRange, error } = setup(); + const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect( - previewService.getPreview(fileRecord, downloadParams, previewParams, bytesRange) - ).rejects.toThrowError(error); + await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); }); diff --git a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts index 2d62feaa08e..d0bd2c16bcf 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts @@ -129,12 +129,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.downloadPreview(userId, fileDownloadParams, previewParams); - expect(previewService.getPreview).toHaveBeenCalledWith( - fileRecord, - fileDownloadParams, - previewParams, - undefined - ); + expect(previewService.getPreview).toHaveBeenCalledWith(fileRecord, previewParams, undefined); }); it('should call checkPermission with correct params', async () => { From 8b7c957399354b517c076979e8d2edf9f782d9c5 Mon Sep 17 00:00:00 2001 From: Max Bischof Date: Mon, 9 Oct 2023 13:17:50 +0200 Subject: [PATCH 04/35] Fix imports of testmodules --- .../controller/api-test/files-security.api.spec.ts | 4 ++-- .../controller/api-test/files-storage-copy-files.api.spec.ts | 3 ++- .../api-test/files-storage-delete-files.api.spec.ts | 3 ++- .../api-test/files-storage-download-upload.api.spec.ts | 3 ++- .../controller/api-test/files-storage-list-files.api.spec.ts | 2 +- .../controller/api-test/files-storage-preview.api.spec.ts | 3 ++- .../controller/api-test/files-storage-rename-file.api.spec.ts | 4 ++-- .../api-test/files-storage-restore-files.api.spec.ts | 3 ++- 8 files changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts index 956697a11d4..421a95bccc4 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts @@ -3,7 +3,6 @@ import { ExecutionContext, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ApiValidationError } from '@shared/common'; import { Permission } from '@shared/domain'; -import { ICurrentUser } from '@src/modules/authentication'; import { cleanupCollections, fileRecordFactory, @@ -12,12 +11,13 @@ import { schoolFactory, userFactory, } from '@shared/testing'; +import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FilesStorageTestModule } from '@src/modules/files-storage'; import { FileRecordListResponse, ScanResultParams } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecord, FileRecordParentType } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; const baseRouteName = '/file-security'; const scanResult: ScanResultParams = { virus_detected: false }; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts index a0b0c63a8a6..3ce6b597187 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts @@ -17,7 +17,7 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION, FilesStorageTestModule } from '@src/modules/files-storage'; +import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { CopyFileParams, CopyFilesOfParentParams, @@ -28,6 +28,7 @@ import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/copy'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts index de360aa60f9..34a9dfdefda 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts @@ -16,12 +16,13 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION, FilesStorageTestModule } from '@src/modules/files-storage'; +import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/delete'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts index 9ec672dfd36..e1893975886 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts @@ -9,13 +9,14 @@ import { S3ClientAdapter } from '@shared/infra/s3-client'; import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION, FilesStorageTestModule } from '@src/modules/files-storage'; +import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecord } from '../../entity'; import { ErrorType } from '../../error'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { TestHelper } from '../../helper/test-helper'; import { availableParentTypes } from './mocks'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts index 6cc436fa7f6..ebc773983a4 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts @@ -13,11 +13,11 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FilesStorageTestModule } from '@src/modules/files-storage'; import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/list'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts index d1aedc97f9e..6467fea68e6 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts @@ -9,13 +9,14 @@ import { S3ClientAdapter } from '@shared/infra/s3-client'; import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION, FilesStorageTestModule } from '@src/modules/files-storage'; +import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecord, ScanStatus } from '../../entity'; import { ErrorType } from '../../error'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { TestHelper } from '../../helper/test-helper'; import { PreviewWidth } from '../../interface'; import { PreviewOutputMimeTypes } from '../../interface/preview-output-mime-types.enum'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts index 2b3cb75f55d..f34be06c8ac 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts @@ -3,7 +3,6 @@ import { ExecutionContext, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ApiValidationError } from '@shared/common'; import { Permission } from '@shared/domain'; -import { ICurrentUser } from '@src/modules/authentication'; import { cleanupCollections, fileRecordFactory, @@ -12,12 +11,13 @@ import { schoolFactory, userFactory, } from '@shared/testing'; +import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FilesStorageTestModule } from '@src/modules/files-storage'; import { FileRecordResponse, RenameFileParams } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecord, FileRecordParentType } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; const baseRouteName = '/file/rename/'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts index 603221b6651..c6ce1d02c2a 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts @@ -16,12 +16,13 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION, FilesStorageTestModule } from '@src/modules/files-storage'; +import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; +import { FilesStorageTestModule } from '../../files-storage-test.module'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/restore'; From 13a53c3ed5cf6b86b07e3a24524d96dccc2e9b40 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 9 Oct 2023 14:23:39 +0200 Subject: [PATCH 05/35] BC-5170 - fix api tests --- .../api-test/files-storage-preview.api.spec.ts | 12 +++++------- .../files-storage/service/files-storage.service.ts | 2 +- .../src/modules/files-storage/uc/files-storage.uc.ts | 2 ++ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts index 6467fea68e6..8e957188b3a 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts @@ -5,6 +5,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ApiValidationError } from '@shared/common'; import { EntityId, Permission } from '@shared/domain'; import { AntivirusService } from '@shared/infra/antivirus/antivirus.service'; +import { PreviewProducer } from '@shared/infra/preview-generator'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; @@ -102,6 +103,8 @@ describe('File Controller (API) - preview', () => { }) .overrideProvider(AntivirusService) .useValue(createMock()) + .overrideProvider(PreviewProducer) + .useValue(createMock()) .overrideProvider(FILES_STORAGE_S3_CONNECTION) .useValue(createMock()) .overrideGuard(JwtAuthGuard) @@ -326,9 +329,8 @@ describe('File Controller (API) - preview', () => { const { result: uploadedFile } = await api.postUploadFile(uploadPath); await setScanStatus(uploadedFile.id, ScanStatus.VERIFIED); - const originalFile = TestHelper.createFile(); const previewFile = TestHelper.createFile('bytes 0-3/4'); - s3ClientAdapter.get.mockResolvedValueOnce(originalFile).mockResolvedValueOnce(previewFile); + s3ClientAdapter.get.mockResolvedValueOnce(previewFile); return { uploadedFile }; }; @@ -371,12 +373,8 @@ describe('File Controller (API) - preview', () => { await setScanStatus(uploadedFile.id, ScanStatus.VERIFIED); const error = new NotFoundException(); - const originalFile = TestHelper.createFile(); const previewFile = TestHelper.createFile('bytes 0-3/4'); - s3ClientAdapter.get - .mockRejectedValueOnce(error) - .mockResolvedValueOnce(originalFile) - .mockResolvedValueOnce(previewFile); + s3ClientAdapter.get.mockRejectedValueOnce(error).mockResolvedValueOnce(previewFile); return { uploadedFile }; }; 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 2b6bc4b179a..bfb4de90472 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 @@ -229,7 +229,7 @@ export class FilesStorageService { } // download - private checkFileName(fileRecord: FileRecord, params: DownloadFileParams): void | NotFoundException { + public checkFileName(fileRecord: FileRecord, params: DownloadFileParams): void | NotFoundException { if (!fileRecord.hasName(params.fileName)) { this.logger.debug(`could not find file with id: ${fileRecord.id} by filename`); throw new NotFoundException(ErrorType.FILE_NOT_FOUND); 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 26ad93bf310..39a836422d2 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 @@ -153,6 +153,8 @@ export class FilesStorageUC { await this.checkPermission(userId, parentType, parentId, FileStorageAuthorizationContext.read); + this.filesStorageService.checkFileName(fileRecord, params); + const result = this.previewService.getPreview(fileRecord, previewParams, bytesRange); return result; From ec15838ea7a6c786f926b1643df0f829272390e7 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 9 Oct 2023 14:25:31 +0200 Subject: [PATCH 06/35] BC-5170 - add test + code review --- .../files-storage-client/mapper/index.ts | 3 +- .../service/files-storage.producer.spec.ts | 3 +- .../preview-generator/interface/index.ts | 16 +- .../preview-generator/interface/preview.ts | 15 ++ .../preview-consumer.module.ts | 11 -- .../preview-generator.consumer.spec.ts | 80 ++++++++++ .../preview-generator.consumer.ts | 2 +- .../preview-generator.service.spec.ts | 151 ++++++++++++++++++ .../preview-generator.service.ts | 3 +- .../preview.producer.spec.ts | 123 ++++++++++++++ .../preview-generator/preview.producer.ts | 4 +- .../infra/rabbitmq}/error.mapper.spec.ts | 0 .../infra/rabbitmq}/error.mapper.ts | 0 .../server/src/shared/infra/rabbitmq/index.ts | 1 + .../shared/infra/rabbitmq/rabbitmq.module.ts | 3 +- .../infra/rabbitmq/rpc-message-producer.ts | 4 +- 16 files changed, 380 insertions(+), 39 deletions(-) create mode 100644 apps/server/src/shared/infra/preview-generator/interface/preview.ts delete mode 100644 apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts rename apps/server/src/{modules/files-storage-client/mapper => shared/infra/rabbitmq}/error.mapper.spec.ts (100%) rename apps/server/src/{modules/files-storage-client/mapper => shared/infra/rabbitmq}/error.mapper.ts (100%) diff --git a/apps/server/src/modules/files-storage-client/mapper/index.ts b/apps/server/src/modules/files-storage-client/mapper/index.ts index 06f40e707e1..006cf64b627 100644 --- a/apps/server/src/modules/files-storage-client/mapper/index.ts +++ b/apps/server/src/modules/files-storage-client/mapper/index.ts @@ -1,4 +1,3 @@ -export * from './error.mapper'; +export * from './copy-files-of-parent-param.builder'; export * from './files-storage-client.mapper'; export * from './files-storage-param.builder'; -export * from './copy-files-of-parent-param.builder'; diff --git a/apps/server/src/modules/files-storage-client/service/files-storage.producer.spec.ts b/apps/server/src/modules/files-storage-client/service/files-storage.producer.spec.ts index 926f860c736..7e4ed5a1c83 100644 --- a/apps/server/src/modules/files-storage-client/service/files-storage.producer.spec.ts +++ b/apps/server/src/modules/files-storage-client/service/files-storage.producer.spec.ts @@ -3,10 +3,9 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { FileRecordParentType, FilesStorageEvents, FilesStorageExchange } from '@shared/infra/rabbitmq'; +import { ErrorMapper, FileRecordParentType, FilesStorageEvents, FilesStorageExchange } from '@shared/infra/rabbitmq'; import { setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { ErrorMapper } from '../mapper'; import { FilesStorageProducer } from './files-storage.producer'; describe('FilesStorageProducer', () => { diff --git a/apps/server/src/shared/infra/preview-generator/interface/index.ts b/apps/server/src/shared/infra/preview-generator/interface/index.ts index 92ab2808151..37aae418ee2 100644 --- a/apps/server/src/shared/infra/preview-generator/interface/index.ts +++ b/apps/server/src/shared/infra/preview-generator/interface/index.ts @@ -1,15 +1 @@ -export interface PreviewOptions { - format: string; - width?: number; -} - -export interface PreviewFileOptions { - originFilePath: string; - previewFilePath: string; - previewOptions: PreviewOptions; -} - -export interface PreviewResponseMessage { - previewFilePath: string; - status: boolean; -} +export * from './preview'; diff --git a/apps/server/src/shared/infra/preview-generator/interface/preview.ts b/apps/server/src/shared/infra/preview-generator/interface/preview.ts new file mode 100644 index 00000000000..92ab2808151 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/interface/preview.ts @@ -0,0 +1,15 @@ +export interface PreviewOptions { + format: string; + width?: number; +} + +export interface PreviewFileOptions { + originFilePath: string; + previewFilePath: string; + previewOptions: PreviewOptions; +} + +export interface PreviewResponseMessage { + previewFilePath: string; + status: boolean; +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts deleted file mode 100644 index e008b88851e..00000000000 --- a/apps/server/src/shared/infra/preview-generator/preview-consumer.module.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Module } from '@nestjs/common'; -import { LoggerModule } from '@src/core/logger'; -import { PreviewGeneratorConsumer } from './preview-generator.consumer'; -import { PreviewGeneratorService } from './preview-generator.service'; - -const providers = [PreviewGeneratorConsumer, PreviewGeneratorService]; -@Module({ - imports: [LoggerModule], - providers, -}) -export class PreviewGeneratorConsumerModule {} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts new file mode 100644 index 00000000000..0b36b813840 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts @@ -0,0 +1,80 @@ +import { DeepMocked, createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { LegacyLogger } from '@src/core/logger'; +import { PreviewFileOptions, PreviewResponseMessage } from './interface'; +import { PreviewGeneratorConsumer } from './preview-generator.consumer'; +import { PreviewGeneratorService } from './preview-generator.service'; + +describe('PreviewGeneratorConsumer', () => { + let module: TestingModule; + let previewGeneratorService: DeepMocked; + let service: PreviewGeneratorConsumer; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + PreviewGeneratorConsumer, + { + provide: PreviewGeneratorService, + useValue: createMock(), + }, + { + provide: LegacyLogger, + useValue: createMock(), + }, + ], + }).compile(); + + previewGeneratorService = module.get(PreviewGeneratorService); + service = module.get(PreviewGeneratorConsumer); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + afterAll(async () => { + await module.close(); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('generatePreview()', () => { + const setup = () => { + const payload: PreviewFileOptions = { + originFilePath: 'file/test.jpeg', + previewFilePath: 'preview/text.webp', + previewOptions: { + format: 'webp', + width: 500, + }, + }; + + const response: PreviewResponseMessage = { + previewFilePath: payload.previewFilePath, + status: true, + }; + previewGeneratorService.generatePreview.mockResolvedValueOnce(response); + + return { payload, response }; + }; + + it('should call previewGeneratorService.generatePreview with payload', async () => { + const { payload } = setup(); + + await service.generatePreview(payload); + + expect(previewGeneratorService.generatePreview).toBeCalledWith(payload); + }); + + it('should return expected value', async () => { + const { payload, response } = setup(); + + const result = await service.generatePreview(payload); + + expect(result).toEqual({ message: response }); + }); + }); +}); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts index b8fafe123ed..9da92cf87c5 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts @@ -16,7 +16,7 @@ export class PreviewGeneratorConsumer { routingKey: FilesPreviewEvents.GENERATE_PREVIEW, queue: FilesPreviewEvents.GENERATE_PREVIEW, }) - public async copyFilesOfParent(@RabbitPayload() payload: PreviewFileOptions) { + public async generatePreview(@RabbitPayload() payload: PreviewFileOptions) { this.logger.debug({ action: 'generate preview', payload }); const response = await this.previewGeneratorService.generatePreview(payload); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts new file mode 100644 index 00000000000..d8aac92c1b1 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts @@ -0,0 +1,151 @@ +import { DeepMocked, createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { GetFile, S3ClientAdapter } from '@shared/infra/s3-client'; +import { LegacyLogger } from '@src/core/logger'; +import { Readable } from 'node:stream'; +import { PreviewGeneratorService } from './preview-generator.service'; + +const streamMock = jest.fn(); +const resizeMock = jest.fn(); +const imageMagickMock = () => { + return { stream: streamMock, resize: resizeMock, data: Readable.from('text') }; +}; +jest.mock('gm', () => { + return { + subClass: () => imageMagickMock, + }; +}); + +const createFile = (contentRange?: string): GetFile => { + const text = 'testText'; + const readable = Readable.from(text); + + const fileResponse = { + data: readable, + contentType: 'image/jpeg', + contentLength: text.length, + contentRange, + etag: 'testTag', + }; + + return fileResponse; +}; + +describe('PreviewGeneratorService', () => { + let module: TestingModule; + let service: PreviewGeneratorService; + let s3ClientAdapter: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + PreviewGeneratorService, + { + provide: S3ClientAdapter, + useValue: createMock(), + }, + { + provide: LegacyLogger, + useValue: createMock(), + }, + ], + }).compile(); + + service = module.get(PreviewGeneratorService); + s3ClientAdapter = module.get(S3ClientAdapter); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('generatePreview', () => { + const setup = (width = 500) => { + const params = { + originFilePath: 'file/test.jpeg', + previewFilePath: 'preview/text.webp', + previewOptions: { + format: 'webp', + width, + }, + }; + const originFile = createFile(); + s3ClientAdapter.get.mockResolvedValueOnce(originFile); + + const data = Readable.from('text'); + streamMock.mockReturnValueOnce(data); + + const expectedFileData = { + data, + mimeType: params.previewOptions.format, + }; + + return { params, originFile, expectedFileData }; + }; + + describe('WHEN download of original and preview file is successful', () => { + it('should call storageClient get method with originFilePath', async () => { + const { params } = setup(); + + await service.generatePreview(params); + + expect(s3ClientAdapter.get).toBeCalledWith(params.originFilePath); + }); + + it('should call imagemagicks resize method', async () => { + const { params } = setup(); + + await service.generatePreview(params); + + expect(resizeMock).toHaveBeenCalledWith(params.previewOptions.width, undefined, '>'); + expect(resizeMock).toHaveBeenCalledTimes(1); + }); + + it('should call imagemagicks stream method', async () => { + const { params } = setup(); + + await service.generatePreview(params); + + expect(streamMock).toHaveBeenCalledWith(params.previewOptions.format); + expect(streamMock).toHaveBeenCalledTimes(1); + }); + + it('should call S3ClientAdapters create method', async () => { + const { params, expectedFileData } = setup(); + + await service.generatePreview(params); + + expect(s3ClientAdapter.create).toHaveBeenCalledWith(params.previewFilePath, expectedFileData); + expect(s3ClientAdapter.create).toHaveBeenCalledTimes(1); + }); + + it('should should return values', async () => { + const { params } = setup(); + const expectedValue = { previewFilePath: params.previewFilePath, status: true }; + + const result = await service.generatePreview(params); + + expect(result).toEqual(expectedValue); + }); + }); + + describe('WHEN previewParams.width not set', () => { + it('should not call imagemagicks resize method', async () => { + const { params } = setup(0); + + await service.generatePreview(params); + + expect(resizeMock).not.toHaveBeenCalledWith(params.previewOptions.width, undefined, '>'); + expect(resizeMock).not.toHaveBeenCalledTimes(1); + }); + }); + }); +}); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index 2aff5e5794b..ec42e138e39 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -33,11 +33,10 @@ export class PreviewGeneratorService { } private resizeAndConvert(original: GetFile, previewParams: PreviewOptions): PassThrough { - const { format } = previewParams; + const { format, width } = previewParams; const im = subClass({ imageMagick: '7+' }); const preview = im(original.data); - const { width } = previewParams; if (width) { preview.resize(width, undefined, '>'); diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts new file mode 100644 index 00000000000..b00db206d1a --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts @@ -0,0 +1,123 @@ +import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ConfigService } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { setupEntities } from '@shared/testing'; +import { LegacyLogger } from '@src/core/logger'; +import { ErrorMapper, FilesPreviewEvents, FilesPreviewExchange } from '../rabbitmq'; +import { PreviewFileOptions } from './interface'; +import { PreviewProducer } from './preview.producer'; + +describe('PreviewProducer', () => { + let module: TestingModule; + let service: PreviewProducer; + let configService: DeepMocked; + let amqpConnection: DeepMocked; + + const timeout = 10000; + + beforeAll(async () => { + await setupEntities(); + module = await Test.createTestingModule({ + providers: [ + PreviewProducer, + { + provide: LegacyLogger, + useValue: createMock(), + }, + { + provide: AmqpConnection, + useValue: createMock(), + }, + { + provide: ConfigService, + useValue: createMock(), + }, + ], + }).compile(); + + service = module.get(PreviewProducer); + amqpConnection = module.get(AmqpConnection); + configService = module.get(ConfigService); + configService.get.mockReturnValue(timeout); + }); + + afterAll(async () => { + await module.close(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('generate', () => { + describe('when valid params are passed and amqp connection return with a message', () => { + const setup = () => { + const params: PreviewFileOptions = { + originFilePath: 'file/test.jpeg', + previewFilePath: 'preview/text.webp', + previewOptions: { + format: 'webp', + width: 500, + }, + }; + + const message = []; + amqpConnection.request.mockResolvedValueOnce({ message }); + + const expectedParams = { + exchange: FilesPreviewExchange, + routingKey: FilesPreviewEvents.GENERATE_PREVIEW, + payload: params, + timeout, + }; + + return { params, expectedParams, message }; + }; + + it('should call the ampqConnection.', async () => { + const { params, expectedParams } = setup(); + + await service.generate(params); + + expect(amqpConnection.request).toHaveBeenCalledWith(expectedParams); + }); + + it('should return the response message.', async () => { + const { params, message } = setup(); + + const res = await service.generate(params); + + expect(res).toEqual(message); + }); + }); + + describe('when amqpConnection return with error in response', () => { + const setup = () => { + const params: PreviewFileOptions = { + originFilePath: 'file/test.jpeg', + previewFilePath: 'preview/text.webp', + previewOptions: { + format: 'webp', + width: 500, + }, + }; + + amqpConnection.request.mockResolvedValueOnce({ error: new Error() }); + const spy = jest.spyOn(ErrorMapper, 'mapRpcErrorResponseToDomainError'); + + return { params, spy }; + }; + + it('should call error mapper and throw with error', async () => { + const { params, spy } = setup(); + + await expect(service.generate(params)).rejects.toThrowError(); + expect(spy).toBeCalled(); + }); + }); + }); +}); diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.ts index 748e821fd4f..841e48e12a6 100644 --- a/apps/server/src/shared/infra/preview-generator/preview.producer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.ts @@ -16,9 +16,9 @@ export class PreviewProducer extends RpcMessageProducer { this.logger.setContext(PreviewProducer.name); } - async generate(payload: PreviewFileOptions): Promise { + async generate(payload: PreviewFileOptions): Promise { this.logger.debug({ action: 'generate:started', payload }); - const response = await this.request(FilesPreviewEvents.GENERATE_PREVIEW, payload); + const response = await this.request(FilesPreviewEvents.GENERATE_PREVIEW, payload); this.logger.debug({ action: 'generate:finished', payload }); diff --git a/apps/server/src/modules/files-storage-client/mapper/error.mapper.spec.ts b/apps/server/src/shared/infra/rabbitmq/error.mapper.spec.ts similarity index 100% rename from apps/server/src/modules/files-storage-client/mapper/error.mapper.spec.ts rename to apps/server/src/shared/infra/rabbitmq/error.mapper.spec.ts diff --git a/apps/server/src/modules/files-storage-client/mapper/error.mapper.ts b/apps/server/src/shared/infra/rabbitmq/error.mapper.ts similarity index 100% rename from apps/server/src/modules/files-storage-client/mapper/error.mapper.ts rename to apps/server/src/shared/infra/rabbitmq/error.mapper.ts diff --git a/apps/server/src/shared/infra/rabbitmq/index.ts b/apps/server/src/shared/infra/rabbitmq/index.ts index 0b4a36d5824..99f5887b9a8 100644 --- a/apps/server/src/shared/infra/rabbitmq/index.ts +++ b/apps/server/src/shared/infra/rabbitmq/index.ts @@ -1,3 +1,4 @@ +export * from './error.mapper'; export * from './exchange'; export * from './rabbitmq.module'; export * from './rpc-message'; diff --git a/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts b/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts index 572c5356f91..946b642e779 100644 --- a/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts +++ b/apps/server/src/shared/infra/rabbitmq/rabbitmq.module.ts @@ -1,8 +1,7 @@ import { AmqpConnectionManager, RabbitMQModule } from '@golevelup/nestjs-rabbitmq'; import { Configuration } from '@hpi-schul-cloud/commons'; import { Global, Module, OnModuleDestroy } from '@nestjs/common'; -import { FilesStorageExchange } from './exchange'; -import { FilesPreviewExchange } from './exchange/files-preview'; +import { FilesPreviewExchange, FilesStorageExchange } from './exchange'; /** * https://www.npmjs.com/package/@golevelup/nestjs-rabbitmq#usage diff --git a/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts index db938aad770..8a239b2cbcd 100644 --- a/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts +++ b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.ts @@ -1,5 +1,5 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; -import { ErrorMapper } from '@src/modules/files-storage-client/mapper'; +import { ErrorMapper } from './error.mapper'; import { RpcMessage } from './rpc-message'; export abstract class RpcMessageProducer { @@ -13,7 +13,7 @@ export abstract class RpcMessageProducer { const response = await this.amqpConnection.request>(this.createRequest(event, payload)); this.checkError(response); - return response.message || []; + return response.message; } // need to be fixed with https://ticketsystem.dbildungscloud.de/browse/BC-2984 From c8999a44c4668dfa6a765ec820788053b67f92ab Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Wed, 11 Oct 2023 16:57:19 +0200 Subject: [PATCH 07/35] BC-5170 devops code review --- .github/workflows/push.yml | 18 +++++++++--------- .github/workflows/tag.yml | 8 ++++---- ...rfile.filestorage => Dockerfile.filepreview | 0 .../templates/amqp-preview-deployment.yml.j2 | 2 +- .../templates/api-files-deployment.yml.j2 | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) rename Dockerfile.filestorage => Dockerfile.filepreview (100%) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 94b3514822f..c042be2c2a9 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -68,36 +68,36 @@ jobs: tags: ghcr.io/${{ github.repository }}:${{ needs.branch_meta.outputs.sha }} labels: ${{ steps.docker_meta_img.outputs.labels }} - - name: Docker meta Service Name (file storage) + - name: Docker meta Service Name (file preview) id: docker_meta_img_file_storage uses: docker/metadata-action@v4 with: images: ghcr.io/${{ github.repository }} tags: | - type=ref,event=branch,enable=false,priority=600,prefix=file-storage- - type=sha,enable=true,priority=600,prefix=file-storage- + type=ref,event=branch,enable=false,priority=600,prefix=file-preview- + type=sha,enable=true,priority=600,prefix=file-preview- labels: | org.opencontainers.image.title=schulcloud-file-storage - - name: test image exists (file storage) + - name: test image exists (file preview) run: | - echo "IMAGE_EXISTS=$(docker manifest inspect ghcr.io/${{ github.repository }}:file-storage-${{ needs.branch_meta.outputs.sha }} > /dev/null && echo 1 || echo 0)" >> $GITHUB_ENV + echo "IMAGE_EXISTS=$(docker manifest inspect ghcr.io/${{ github.repository }}:file-preview-${{ needs.branch_meta.outputs.sha }} > /dev/null && echo 1 || echo 0)" >> $GITHUB_ENV - - name: Set up Docker Buildx (file storage) + - name: Set up Docker Buildx (file preview) if: ${{ env.IMAGE_EXISTS == 0 }} uses: docker/setup-buildx-action@v2 - - name: Build and push ${{ github.repository }} (file storage) + - name: Build and push ${{ github.repository }} (file preview) if: ${{ env.IMAGE_EXISTS == 0 }} uses: docker/build-push-action@v4 with: build-args: | BASE_IMAGE=ghcr.io/${{ github.repository }}:${{ needs.branch_meta.outputs.sha }} context: . - file: ./Dockerfile.filestorage + file: ./Dockerfile.filepreview platforms: linux/amd64 push: true - tags: ghcr.io/${{ github.repository }}:file-storage-${{ needs.branch_meta.outputs.sha }} + tags: ghcr.io/${{ github.repository }}:file-preview-${{ needs.branch_meta.outputs.sha }} labels: | ${{ steps.docker_meta_img_file_storage.outputs.labels }} diff --git a/.github/workflows/tag.yml b/.github/workflows/tag.yml index fa62df4b0ab..8f484116ca8 100644 --- a/.github/workflows/tag.yml +++ b/.github/workflows/tag.yml @@ -48,14 +48,14 @@ jobs: tags: ${{ steps.docker_meta_img_hub.outputs.tags }} labels: ${{ steps.docker_meta_img_hub.outputs.labels }} - - name: Docker meta Service Name for docker hub (file storage) + - name: Docker meta Service Name for docker hub (file preview) id: docker_meta_img_hub_file_storage uses: docker/metadata-action@v4 with: images: docker.io/schulcloud/schulcloud-server, quay.io/schulcloudverbund/schulcloud-server tags: | - type=semver,pattern={{version}},prefix=file-storage-,onlatest=false - type=semver,pattern={{major}}.{{minor}},prefix=file-storage-,onlatest=false + type=semver,pattern={{version}},prefix=file-preview-,onlatest=false + type=semver,pattern={{major}}.{{minor}},prefix=file-preview-,onlatest=false labels: | org.opencontainers.image.title=schulcloud-file-storage - name: Build and push ${{ github.repository }} (file-storage) @@ -64,7 +64,7 @@ jobs: build-args: | BASE_IMAGE=quay.io/schulcloudverbund/schulcloud-server:${{ github.ref_name }} context: . - file: ./Dockerfile.filestorage + file: ./Dockerfile.filepreview platforms: linux/amd64 push: true tags: ${{ steps.docker_meta_img_hub_file_storage.outputs.tags }} diff --git a/Dockerfile.filestorage b/Dockerfile.filepreview similarity index 100% rename from Dockerfile.filestorage rename to Dockerfile.filepreview diff --git a/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 index bdcc28fd5be..ead975a37a5 100644 --- a/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 @@ -29,7 +29,7 @@ spec: runAsNonRoot: true containers: - name: amqp-file-preview - image: {{ SCHULCLOUD_SERVER_IMAGE }}:file-storage-{{ SCHULCLOUD_SERVER_IMAGE_TAG }} + image: {{ SCHULCLOUD_SERVER_IMAGE }}:file-preview-{{ SCHULCLOUD_SERVER_IMAGE_TAG }} imagePullPolicy: IfNotPresent envFrom: - configMapRef: diff --git a/ansible/roles/schulcloud-server-core/templates/api-files-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-files-deployment.yml.j2 index 7f5a5b7e50b..727b5a9f4f0 100644 --- a/ansible/roles/schulcloud-server-core/templates/api-files-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/api-files-deployment.yml.j2 @@ -29,7 +29,7 @@ spec: runAsNonRoot: true containers: - name: api-files - image: {{ SCHULCLOUD_SERVER_IMAGE }}:file-storage-{{ SCHULCLOUD_SERVER_IMAGE_TAG }} + image: {{ SCHULCLOUD_SERVER_IMAGE }}:{{ SCHULCLOUD_SERVER_IMAGE_TAG }} imagePullPolicy: IfNotPresent ports: - containerPort: 4444 From 37bfd659b4b8e64d6350daf095ebcdb8325f5c55 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 08:50:51 +0200 Subject: [PATCH 08/35] BC-5170 - code review --- apps/server/src/apps/files-storage-consumer.app.ts | 2 +- apps/server/src/apps/files-storage.app.ts | 3 ++- apps/server/src/apps/preview-generator-consumer.app.ts | 2 +- .../controller/api-test/files-security.api.spec.ts | 2 +- .../api-test/files-storage-copy-files.api.spec.ts | 7 +------ .../api-test/files-storage-delete-files.api.spec.ts | 2 +- .../api-test/files-storage-download-upload.api.spec.ts | 4 ++-- .../api-test/files-storage-list-files.api.spec.ts | 2 +- .../controller/api-test/files-storage-preview.api.spec.ts | 4 ++-- .../api-test/files-storage-rename-file.api.spec.ts | 2 +- .../api-test/files-storage-restore-files.api.spec.ts | 4 ++-- apps/server/src/modules/files-storage/index.ts | 7 ++----- apps/server/src/modules/index.ts | 3 +-- .../preview-generator/preview-generator-producer.module.ts | 5 ++--- 14 files changed, 20 insertions(+), 29 deletions(-) diff --git a/apps/server/src/apps/files-storage-consumer.app.ts b/apps/server/src/apps/files-storage-consumer.app.ts index 777176e1176..4708c6c640b 100644 --- a/apps/server/src/apps/files-storage-consumer.app.ts +++ b/apps/server/src/apps/files-storage-consumer.app.ts @@ -3,7 +3,7 @@ import { NestFactory } from '@nestjs/core'; // register source-map-support for debugging -import { FilesStorageAMQPModule } from '@src/modules/files-storage'; +import { FilesStorageAMQPModule } from '@src/modules/files-storage/files-storage-amqp.module'; import { install as sourceMapInstall } from 'source-map-support'; async function bootstrap() { diff --git a/apps/server/src/apps/files-storage.app.ts b/apps/server/src/apps/files-storage.app.ts index e10a2b3a162..2a09bc3a5ec 100644 --- a/apps/server/src/apps/files-storage.app.ts +++ b/apps/server/src/apps/files-storage.app.ts @@ -10,7 +10,8 @@ import { install as sourceMapInstall } from 'source-map-support'; // application imports import { SwaggerDocumentOptions } from '@nestjs/swagger'; import { LegacyLogger } from '@src/core/logger'; -import { API_VERSION_PATH, FilesStorageApiModule } from '@src/modules/files-storage'; +import { FilesStorageApiModule } from '@src/modules/files-storage/files-storage-api.module'; +import { API_VERSION_PATH } from '@src/modules/files-storage/files-storage.const'; import { enableOpenApiDocs } from '@src/shared/controller/swagger'; async function bootstrap() { diff --git a/apps/server/src/apps/preview-generator-consumer.app.ts b/apps/server/src/apps/preview-generator-consumer.app.ts index 17f94fbf4dd..e802776bfd1 100644 --- a/apps/server/src/apps/preview-generator-consumer.app.ts +++ b/apps/server/src/apps/preview-generator-consumer.app.ts @@ -1,7 +1,7 @@ /* istanbul ignore file */ /* eslint-disable no-console */ import { NestFactory } from '@nestjs/core'; -import { PreviewGeneratorAMQPModule } from '@src/modules/files-storage'; +import { PreviewGeneratorAMQPModule } from '@src/modules/files-storage/files-preview-amqp.module'; import { install as sourceMapInstall } from 'source-map-support'; async function bootstrap() { diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts index 421a95bccc4..36947d7c454 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-security.api.spec.ts @@ -13,11 +13,11 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FileRecordListResponse, ScanResultParams } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecord, FileRecordParentType } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FileRecordListResponse, ScanResultParams } from '../dto'; const baseRouteName = '/file-security'; const scanResult: ScanResultParams = { virus_detected: false }; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts index 93dfa06cfc7..ece0175ad35 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts @@ -18,17 +18,12 @@ import { import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; -import { - CopyFileParams, - CopyFilesOfParentParams, - FileRecordListResponse, - FileRecordResponse, -} from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { CopyFileParams, CopyFilesOfParentParams, FileRecordListResponse, FileRecordResponse } from '../dto'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/copy'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts index 57ea97e626e..b22470b75c3 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-delete-files.api.spec.ts @@ -16,13 +16,13 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FILES_STORAGE_S3_CONNECTION } from '../../files-storage.config'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/delete'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts index d232825d044..fd475609ffb 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-download-upload.api.spec.ts @@ -9,15 +9,15 @@ import { S3ClientAdapter } from '@shared/infra/s3-client'; import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; -import { FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecord } from '../../entity'; import { ErrorType } from '../../error'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FILES_STORAGE_S3_CONNECTION } from '../../files-storage.config'; import { TestHelper } from '../../helper/test-helper'; +import { FileRecordResponse } from '../dto'; import { availableParentTypes } from './mocks'; jest.mock('file-type-cjs/file-type-cjs-index', () => { diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts index ebc773983a4..ea3f4d27e37 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-list-files.api.spec.ts @@ -13,11 +13,11 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FileRecordListResponse, FileRecordResponse } from '../dto'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/list'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts index ea0b3821a52..9ce9bff2108 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-preview.api.spec.ts @@ -10,17 +10,17 @@ import { S3ClientAdapter } from '@shared/infra/s3-client'; import { cleanupCollections, mapUserToCurrentUser, roleFactory, schoolFactory, userFactory } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; -import { FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecord, ScanStatus } from '../../entity'; import { ErrorType } from '../../error'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FILES_STORAGE_S3_CONNECTION } from '../../files-storage.config'; import { TestHelper } from '../../helper/test-helper'; import { PreviewWidth } from '../../interface'; import { PreviewOutputMimeTypes } from '../../interface/preview-output-mime-types.enum'; +import { FileRecordResponse } from '../dto'; jest.mock('file-type-cjs/file-type-cjs-index', () => { return { diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts index f34be06c8ac..f1e701d1b6a 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-rename-file.api.spec.ts @@ -13,11 +13,11 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FileRecordResponse, RenameFileParams } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import request from 'supertest'; import { FileRecord, FileRecordParentType } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FileRecordResponse, RenameFileParams } from '../dto'; const baseRouteName = '/file/rename/'; diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts index 56aa2278642..7e3109fefc2 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-restore-files.api.spec.ts @@ -16,13 +16,13 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; -import { FileRecordListResponse, FileRecordResponse } from '@src/modules/files-storage/controller/dto'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType, PreviewStatus } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FILES_STORAGE_S3_CONNECTION } from '../../files-storage.config'; +import { FileRecordListResponse, FileRecordResponse } from '../dto'; import { availableParentTypes } from './mocks'; const baseRouteName = '/file/restore'; diff --git a/apps/server/src/modules/files-storage/index.ts b/apps/server/src/modules/files-storage/index.ts index a1e2e9fdd19..6ee0883938f 100644 --- a/apps/server/src/modules/files-storage/index.ts +++ b/apps/server/src/modules/files-storage/index.ts @@ -1,5 +1,2 @@ -export * from './files-preview-amqp.module'; -export * from './files-storage-amqp.module'; -export * from './files-storage-api.module'; -export * from './files-storage.config'; -export * from './files-storage.const'; +// this module has no exports +// it is an isolated module, it cannot be used in other modules diff --git a/apps/server/src/modules/index.ts b/apps/server/src/modules/index.ts index 111a64d9f33..9bd7dd43d5b 100644 --- a/apps/server/src/modules/index.ts +++ b/apps/server/src/modules/index.ts @@ -3,10 +3,10 @@ export * from './authentication'; export * from './authorization'; export * from './board'; export * from './collaborative-storage'; -export * from './files-storage'; export * from './files-storage-client'; export * from './fwu-learning-contents'; export * from './learnroom'; +export * from './legacy-school'; export * from './lesson'; export * from './news'; export * from './oauth'; @@ -14,7 +14,6 @@ export * from './oauth-provider'; export * from './provisioning'; export * from './rocketchat'; export * from './role'; -export * from './legacy-school'; export * from './sharing'; export * from './system'; export * from './task'; diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts index d91ec75c8f9..d3f65299657 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator-producer.module.ts @@ -3,10 +3,9 @@ import { LoggerModule } from '@src/core/logger'; import { RabbitMQWrapperModule } from '../rabbitmq'; import { PreviewProducer } from './preview.producer'; -const providers = [PreviewProducer]; @Module({ imports: [LoggerModule, RabbitMQWrapperModule], - providers, - exports: providers, + providers: [PreviewProducer], + exports: [PreviewProducer], }) export class PreviewGeneratorProducerModule {} From 75cb61c84ad95f4d14730a19f02dbe1b5ad06b8b Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 09:29:36 +0200 Subject: [PATCH 09/35] BC-5170 - rename method --- .../service/preview.service.spec.ts | 38 +++++++++---------- .../files-storage/service/preview.service.ts | 2 +- .../files-storage-download-preview.uc.spec.ts | 6 +-- .../files-storage/uc/files-storage.uc.ts | 2 +- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/apps/server/src/modules/files-storage/service/preview.service.spec.ts b/apps/server/src/modules/files-storage/service/preview.service.spec.ts index 685b54f52a3..3101aa156eb 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.spec.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.spec.ts @@ -101,7 +101,7 @@ describe('PreviewService', () => { jest.resetAllMocks(); }); - describe('getPreview is called', () => { + describe('download is called', () => { describe('WHEN preview is possbile', () => { describe('WHEN forceUpdate is true', () => { describe('WHEN first get of preview file is successfull', () => { @@ -140,7 +140,7 @@ describe('PreviewService', () => { it('calls previewProducer.generate with correct params', async () => { const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = setup(); - await previewService.getPreview(fileRecord, previewParams, bytesRange); + await previewService.download(fileRecord, previewParams, bytesRange); expect(previewProducer.generate).toHaveBeenCalledWith({ originFilePath: originPath, @@ -152,7 +152,7 @@ describe('PreviewService', () => { it('calls S3ClientAdapters get method', async () => { const { fileRecord, previewParams, previewPath } = setup(); - await previewService.getPreview(fileRecord, previewParams); + await previewService.download(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); @@ -161,7 +161,7 @@ describe('PreviewService', () => { it('returns preview file response', async () => { const { fileRecord, previewParams, previewFileResponse } = setup(); - const response = await previewService.getPreview(fileRecord, previewParams); + const response = await previewService.download(fileRecord, previewParams); expect(response).toEqual(previewFileResponse); }); @@ -206,7 +206,7 @@ describe('PreviewService', () => { const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = setup(notFoundException); - await previewService.getPreview(fileRecord, previewParams, bytesRange); + await previewService.download(fileRecord, previewParams, bytesRange); expect(previewProducer.generate).toHaveBeenCalledWith({ originFilePath: originPath, @@ -220,7 +220,7 @@ describe('PreviewService', () => { const notFoundException = new NotFoundException(); const { fileRecord, previewParams, previewPath } = setup(notFoundException); - await previewService.getPreview(fileRecord, previewParams); + await previewService.download(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); expect(s3ClientAdapter.get).toHaveBeenCalledTimes(2); @@ -232,7 +232,7 @@ describe('PreviewService', () => { const error = new Error('testError'); const { fileRecord, previewParams } = setup(error); - await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(error); + await expect(previewService.download(fileRecord, previewParams)).rejects.toThrow(error); }); }); }); @@ -274,7 +274,7 @@ describe('PreviewService', () => { it('should pass error', async () => { const { fileRecord, previewParams } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(); + await expect(previewService.download(fileRecord, previewParams)).rejects.toThrow(); }); }); }); @@ -316,7 +316,7 @@ describe('PreviewService', () => { it('calls S3ClientAdapters get method', async () => { const { fileRecord, previewParams, previewPath } = setup(); - await previewService.getPreview(fileRecord, previewParams); + await previewService.download(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); expect(s3ClientAdapter.get).toHaveBeenCalledTimes(1); @@ -325,7 +325,7 @@ describe('PreviewService', () => { it('returns preview file response', async () => { const { fileRecord, previewParams, previewFileResponse } = setup(); - const response = await previewService.getPreview(fileRecord, previewParams); + const response = await previewService.download(fileRecord, previewParams); expect(response).toEqual(previewFileResponse); }); @@ -333,7 +333,7 @@ describe('PreviewService', () => { it('does not call generate', async () => { const { fileRecord, previewParams, bytesRange } = setup(); - await previewService.getPreview(fileRecord, previewParams, bytesRange); + await previewService.download(fileRecord, previewParams, bytesRange); expect(previewProducer.generate).toHaveBeenCalledTimes(0); }); @@ -378,7 +378,7 @@ describe('PreviewService', () => { const { fileRecord, previewParams, bytesRange, originPath, previewPath, format } = setup(notFoundException); - await previewService.getPreview(fileRecord, previewParams, bytesRange); + await previewService.download(fileRecord, previewParams, bytesRange); expect(previewProducer.generate).toHaveBeenCalledWith({ originFilePath: originPath, @@ -392,7 +392,7 @@ describe('PreviewService', () => { const notFoundException = new NotFoundException(); const { fileRecord, previewParams, previewPath } = setup(notFoundException); - await previewService.getPreview(fileRecord, previewParams); + await previewService.download(fileRecord, previewParams); expect(s3ClientAdapter.get).toHaveBeenCalledWith(previewPath, undefined); expect(s3ClientAdapter.get).toHaveBeenCalledTimes(2); @@ -404,7 +404,7 @@ describe('PreviewService', () => { const error = new Error('testError'); const { fileRecord, previewParams } = setup(error); - await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(error); + await expect(previewService.download(fileRecord, previewParams)).rejects.toThrow(error); }); }); }); @@ -446,7 +446,7 @@ describe('PreviewService', () => { it('should pass error', async () => { const { fileRecord, previewParams } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams)).rejects.toThrow(); + await expect(previewService.download(fileRecord, previewParams)).rejects.toThrow(); }); }); }); @@ -475,7 +475,7 @@ describe('PreviewService', () => { it('passes error', async () => { const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); + await expect(previewService.download(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -501,7 +501,7 @@ describe('PreviewService', () => { it('passes error', async () => { const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); + await expect(previewService.download(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -528,7 +528,7 @@ describe('PreviewService', () => { it('calls download with correct params', async () => { const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); + await expect(previewService.download(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); @@ -554,7 +554,7 @@ describe('PreviewService', () => { it('calls download with correct params', async () => { const { fileRecord, previewParams, bytesRange, error } = setup(); - await expect(previewService.getPreview(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); + await expect(previewService.download(fileRecord, previewParams, bytesRange)).rejects.toThrowError(error); }); }); }); diff --git a/apps/server/src/modules/files-storage/service/preview.service.ts b/apps/server/src/modules/files-storage/service/preview.service.ts index 86d9e9c6b07..8e5a1470784 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.ts @@ -21,7 +21,7 @@ export class PreviewService { this.logger.setContext(PreviewService.name); } - public async getPreview( + public async download( fileRecord: FileRecord, previewParams: PreviewParams, bytesRange?: string diff --git a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts index 81a4e5cb2dc..72edacd96ed 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts @@ -111,7 +111,7 @@ describe('FilesStorageUC', () => { const previewFileResponse = TestHelper.createFileResponse(); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - previewService.getPreview.mockResolvedValueOnce(previewFileResponse); + previewService.download.mockResolvedValueOnce(previewFileResponse); return { fileDownloadParams, previewParams, userId, fileRecord, singleFileParams, previewFileResponse }; }; @@ -129,7 +129,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.downloadPreview(userId, fileDownloadParams, previewParams); - expect(previewService.getPreview).toHaveBeenCalledWith(fileRecord, previewParams, undefined); + expect(previewService.download).toHaveBeenCalledWith(fileRecord, previewParams, undefined); }); it('should call checkPermission with correct params', async () => { @@ -206,7 +206,7 @@ describe('FilesStorageUC', () => { filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); const error = new Error('test'); - previewService.getPreview.mockRejectedValueOnce(error); + previewService.download.mockRejectedValueOnce(error); return { fileDownloadParams, previewParams, userId, 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 39a836422d2..8c5c2b96de9 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 @@ -155,7 +155,7 @@ export class FilesStorageUC { this.filesStorageService.checkFileName(fileRecord, params); - const result = this.previewService.getPreview(fileRecord, previewParams, bytesRange); + const result = this.previewService.download(fileRecord, previewParams, bytesRange); return result; } From 7565814a622e660e54fd67e3d6e9c0e42433d1da Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 09:41:42 +0200 Subject: [PATCH 10/35] BC-5170 - change logger --- .../loggable/preview-actions.loggable.ts | 19 +++++++++++++++++++ .../preview-generator-consumer.module.ts | 6 +++--- .../preview-generator.consumer.spec.ts | 6 +++--- .../preview-generator.consumer.ts | 7 ++++--- .../preview-generator.service.spec.ts | 6 +++--- .../preview-generator.service.ts | 8 ++++++-- .../preview.producer.spec.ts | 6 +++--- .../preview-generator/preview.producer.ts | 13 ++++++++----- 8 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.ts diff --git a/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.ts b/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.ts new file mode 100644 index 00000000000..e98f21d09be --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.ts @@ -0,0 +1,19 @@ +import { LogMessage, Loggable } from '@src/core/logger'; +import { PreviewFileOptions } from '../interface'; + +export class PreviewActionsLoggable implements Loggable { + constructor(private readonly message: string, private readonly payload: PreviewFileOptions) {} + + getLogMessage(): LogMessage { + const { originFilePath, previewFilePath, previewOptions } = this.payload; + return { + message: this.message, + data: { + originFilePath, + previewFilePath, + format: previewOptions.format, + width: previewOptions.width, + }, + }; + } +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts index 20e78a52e19..ac72bec2938 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts @@ -4,7 +4,7 @@ import { ConfigModule } from '@nestjs/config'; import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq'; import { S3ClientAdapter, S3ClientModule, S3Config } from '@shared/infra/s3-client'; import { createConfigModuleOptions } from '@src/config'; -import { LegacyLogger, LoggerModule } from '@src/core/logger'; +import { Logger, LoggerModule } from '@src/core/logger'; import { PreviewGeneratorConsumer } from './preview-generator.consumer'; import { PreviewGeneratorService } from './preview-generator.service'; @@ -21,9 +21,9 @@ export class PreviewGeneratorConsumerModule { const providers = [ { provide: PreviewGeneratorService, - useFactory: (logger: LegacyLogger, storageClient: S3ClientAdapter) => + useFactory: (logger: Logger, storageClient: S3ClientAdapter) => new PreviewGeneratorService(storageClient, logger), - inject: [LegacyLogger, s3Config.connectionName], + inject: [Logger, s3Config.connectionName], }, PreviewGeneratorConsumer, ]; diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts index 0b36b813840..b1a24a30a57 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.spec.ts @@ -1,6 +1,6 @@ import { DeepMocked, createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { PreviewFileOptions, PreviewResponseMessage } from './interface'; import { PreviewGeneratorConsumer } from './preview-generator.consumer'; import { PreviewGeneratorService } from './preview-generator.service'; @@ -19,8 +19,8 @@ describe('PreviewGeneratorConsumer', () => { useValue: createMock(), }, { - provide: LegacyLogger, - useValue: createMock(), + provide: Logger, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts index 9da92cf87c5..8fc08d261f3 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.consumer.ts @@ -1,13 +1,14 @@ import { RabbitPayload, RabbitRPC } from '@golevelup/nestjs-rabbitmq'; import { Injectable } from '@nestjs/common'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { FilesPreviewEvents, FilesPreviewExchange } from '@src/shared/infra/rabbitmq'; import { PreviewFileOptions } from './interface'; +import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; import { PreviewGeneratorService } from './preview-generator.service'; @Injectable() export class PreviewGeneratorConsumer { - constructor(private readonly previewGeneratorService: PreviewGeneratorService, private logger: LegacyLogger) { + constructor(private readonly previewGeneratorService: PreviewGeneratorService, private logger: Logger) { this.logger.setContext(PreviewGeneratorConsumer.name); } @@ -17,7 +18,7 @@ export class PreviewGeneratorConsumer { queue: FilesPreviewEvents.GENERATE_PREVIEW, }) public async generatePreview(@RabbitPayload() payload: PreviewFileOptions) { - this.logger.debug({ action: 'generate preview', payload }); + this.logger.debug(new PreviewActionsLoggable('PreviewGeneratorConsumer.generatePreview', payload)); const response = await this.previewGeneratorService.generatePreview(payload); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts index d8aac92c1b1..b8eeea612f5 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.spec.ts @@ -1,7 +1,7 @@ import { DeepMocked, createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; import { GetFile, S3ClientAdapter } from '@shared/infra/s3-client'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { Readable } from 'node:stream'; import { PreviewGeneratorService } from './preview-generator.service'; @@ -45,8 +45,8 @@ describe('PreviewGeneratorService', () => { useValue: createMock(), }, { - provide: LegacyLogger, - useValue: createMock(), + provide: Logger, + useValue: createMock(), }, ], }).compile(); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index ec42e138e39..fb24a4832fd 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -1,17 +1,19 @@ import { Injectable } from '@nestjs/common'; import { GetFile, S3ClientAdapter } from '@shared/infra/s3-client'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { subClass } from 'gm'; import { PassThrough } from 'stream'; import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from './interface'; +import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; @Injectable() export class PreviewGeneratorService { - constructor(private readonly storageClient: S3ClientAdapter, private logger: LegacyLogger) { + constructor(private readonly storageClient: S3ClientAdapter, private logger: Logger) { this.logger.setContext(PreviewGeneratorService.name); } public async generatePreview(params: PreviewFileOptions): Promise { + this.logger.debug(new PreviewActionsLoggable('PreviewGeneratorService.generatePreview:start', params)); const { originFilePath, previewFilePath, previewOptions } = params; const original = await this.downloadOriginFile(originFilePath); @@ -20,6 +22,8 @@ export class PreviewGeneratorService { const file = { data: preview, mimeType: previewOptions.format }; await this.storageClient.create(previewFilePath, file); + this.logger.debug(new PreviewActionsLoggable('PreviewGeneratorService.generatePreview:end', params)); + return { previewFilePath, status: true, diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts index b00db206d1a..24a33c1fd07 100644 --- a/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts @@ -3,7 +3,7 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { setupEntities } from '@shared/testing'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { ErrorMapper, FilesPreviewEvents, FilesPreviewExchange } from '../rabbitmq'; import { PreviewFileOptions } from './interface'; import { PreviewProducer } from './preview.producer'; @@ -22,8 +22,8 @@ describe('PreviewProducer', () => { providers: [ PreviewProducer, { - provide: LegacyLogger, - useValue: createMock(), + provide: Logger, + useValue: createMock(), }, { provide: AmqpConnection, diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.ts index 841e48e12a6..bbf8287f3f2 100644 --- a/apps/server/src/shared/infra/preview-generator/preview.producer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.ts @@ -2,25 +2,28 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { FilesPreviewEvents, FilesPreviewExchange, RpcMessageProducer } from '@shared/infra/rabbitmq'; -import { LegacyLogger } from '@src/core/logger'; +import { Logger } from '@src/core/logger'; import { PreviewFileOptions, PreviewResponseMessage } from './interface'; +import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; @Injectable() export class PreviewProducer extends RpcMessageProducer { constructor( protected readonly amqpConnection: AmqpConnection, - private readonly logger: LegacyLogger, + private readonly logger: Logger, protected readonly configService: ConfigService ) { - super(amqpConnection, FilesPreviewExchange, configService.get('INCOMING_REQUEST_TIMEOUT')); + const timeout = configService.get('INCOMING_REQUEST_TIMEOUT'); + + super(amqpConnection, FilesPreviewExchange, timeout); this.logger.setContext(PreviewProducer.name); } async generate(payload: PreviewFileOptions): Promise { - this.logger.debug({ action: 'generate:started', payload }); + this.logger.debug(new PreviewActionsLoggable('PreviewProducer.generate:started', payload)); const response = await this.request(FilesPreviewEvents.GENERATE_PREVIEW, payload); - this.logger.debug({ action: 'generate:finished', payload }); + this.logger.debug(new PreviewActionsLoggable('PreviewProducer.generate:finished', payload)); return response; } From d609e9bc68bbd3755254781711274950769f9494 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 10:13:24 +0200 Subject: [PATCH 11/35] BC-5170 - add configmap --- .../schulcloud-server-core/tasks/main.yml | 18 ++++++++++++++++-- .../preview-geneartor-configmap.yml.j2 | 8 ++++++++ ....j2 => preview-generator-deployment.yml.j2} | 14 +++++++------- .../preview-generator-onepassword.yml.j2 | 9 +++++++++ 4 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 rename ansible/roles/schulcloud-server-core/templates/{amqp-preview-deployment.yml.j2 => preview-generator-deployment.yml.j2} (82%) create mode 100644 ansible/roles/schulcloud-server-core/templates/preview-generator-onepassword.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index c8eac90aaa0..957be25f770 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -84,8 +84,22 @@ namespace: "{{ NAMESPACE }}" template: amqp-files-deployment.yml.j2 - - name: AMQPFilePreviewDeployment + - name: PreviewGeneratorDeployment kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" - template: amqp-preview-deployment.yml.j2 + template: preview-generator-deployment.yml.j2 + + - name: PreviewGeneratorConfigmap + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: preview-generator-configmap.yml.j2 + apply: yes + + - name: PreviewGenerator Secret by 1Password + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: preview-generator-onepassword.yml.j2 + when: ONEPASSWORD_OPERATOR is defined and ONEPASSWORD_OPERATOR|bool diff --git a/ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 new file mode 100644 index 00000000000..2ddad2fd911 --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: preview-geneartor-configmap + namespace: {{ NAMESPACE }} + labels: + app: preview-generator +data: diff --git a/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 similarity index 82% rename from ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 rename to ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 index ead975a37a5..0c16be66532 100644 --- a/ansible/roles/schulcloud-server-core/templates/amqp-preview-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 @@ -1,10 +1,10 @@ apiVersion: apps/v1 kind: Deployment metadata: - name: amqp-file-preview-deployment + name: preview-generator-deployment namespace: {{ NAMESPACE }} labels: - app: amqp-file-preview + app: preview-generator spec: replicas: {{ AMQP_FILE_PREVIEW_REPLICAS|default("1", true) }} strategy: @@ -16,11 +16,11 @@ spec: paused: false selector: matchLabels: - app: amqp-file-preview + app: preview-generator template: metadata: labels: - app: amqp-file-preview + app: preview-generator spec: securityContext: runAsUser: 1000 @@ -28,14 +28,14 @@ spec: fsGroup: 1000 runAsNonRoot: true containers: - - name: amqp-file-preview + - name: preview-generator image: {{ SCHULCLOUD_SERVER_IMAGE }}:file-preview-{{ SCHULCLOUD_SERVER_IMAGE_TAG }} imagePullPolicy: IfNotPresent envFrom: - configMapRef: - name: api-configmap + name: preview-generator-configmap - secretRef: - name: api-secret + name: preview-generator-secret command: ['npm', 'run', 'nest:start:preview-generator-amqp:prod'] resources: limits: diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-onepassword.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-onepassword.yml.j2 new file mode 100644 index 00000000000..51d979e27e0 --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-onepassword.yml.j2 @@ -0,0 +1,9 @@ +apiVersion: onepassword.com/v1 +kind: OnePasswordItem +metadata: + name: preview-generator-secret + namespace: {{ NAMESPACE }} + labels: + app: preview-generator +spec: + itemPath: "vaults/{{ ONEPASSWORD_OPERATOR_VAULT }}/items/preview-generator" From 1f4ab96bb776bccea70c15cb788e81fca81867d6 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 10:53:59 +0200 Subject: [PATCH 12/35] fixup! BC-5170 - add configmap --- ...eartor-configmap.yml.j2 => preview-generator-configmap.yml.j2} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ansible/roles/schulcloud-server-core/templates/{preview-geneartor-configmap.yml.j2 => preview-generator-configmap.yml.j2} (100%) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 similarity index 100% rename from ansible/roles/schulcloud-server-core/templates/preview-geneartor-configmap.yml.j2 rename to ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 From 6bad34805115d2b5e7895f3ddb8200c7f35713b0 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 11:27:51 +0200 Subject: [PATCH 13/35] fixup! fixup! BC-5170 - add configmap --- .../templates/preview-generator-configmap.yml.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 index 2ddad2fd911..457a5e47364 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-configmap.yml.j2 @@ -1,7 +1,7 @@ apiVersion: v1 kind: ConfigMap metadata: - name: preview-geneartor-configmap + name: preview-generator-configmap namespace: {{ NAMESPACE }} labels: app: preview-generator From 1c63d61a443c831609da1fa214c7e16513cb6e6e Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 11:43:47 +0200 Subject: [PATCH 14/35] BC-5170 - remove required envs --- config/default.schema.json | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/config/default.schema.json b/config/default.schema.json index 5aba0e9aad8..d1a2c83457e 100644 --- a/config/default.schema.json +++ b/config/default.schema.json @@ -1339,14 +1339,7 @@ } } }, - "required": [ - "TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION", - "STUDENT_TEAM_CREATION", - "BLOCK_DISPOSABLE_EMAIL_DOMAINS", - "HOST", - "ACTIVATION_LINK_PERIOD_OF_VALIDITY_SECONDS", - "AES_KEY" - ], + "required": [], "allOf": [ { "$ref": "#/definitions/FEATURE_ES_MERLIN_ENABLED", From c8271052bc9f6fd6ddaa8e3e8505848a43382778 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 12:09:32 +0200 Subject: [PATCH 15/35] BC-5170 - add scaled object --- .../schulcloud-server-core/tasks/main.yml | 6 +++ .../preview-generator-scaled-object.yml.j2 | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 957be25f770..fdda78688a1 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -103,3 +103,9 @@ namespace: "{{ NAMESPACE }}" template: preview-generator-onepassword.yml.j2 when: ONEPASSWORD_OPERATOR is defined and ONEPASSWORD_OPERATOR|bool + + - name: PreviewGeneratorScaledObject + kubernetes.core.k8s: + kubeconfig: ~/.kube/config + namespace: "{{ NAMESPACE }}" + template: preview-generator-scaled-object.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 new file mode 100644 index 00000000000..b0c2db8108f --- /dev/null +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 @@ -0,0 +1,43 @@ +apiVersion: onepassword.com/v1 +kind: OnePasswordItem +metadata: + name: keda-secret + namespace: {{ NAMESPACE }} + labels: + app: keda +spec: + itemPath: "vaults/{{ ONEPASSWORD_OPERATOR_VAULT }}/items/keda" +--- +apiVersion: keda.sh/v1alpha1 +kind: TriggerAuthentication +metadata: + name: keda-trigger-auth-rabbitmq-conn + namespace: {{ NAMESPACE }} +spec: + secretTargetRef: + - parameter: host + name: keda-secret + key: amqp-url +--- +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: rabbitmq-scaledobject + namespace: {{ NAMESPACE }} +spec: + scaleTargetRef: + name: preview-generator-deployment + pollingInterval: 1 + cooldownPeriod: 300 + idleReplicaCount: 0 + minReplicaCount: 0 + maxReplicaCount: 1 + triggers: + - type: rabbitmq + metadata: + protocol: amqp + queueName: generate-preview + mode: QueueLength + value: "1" + authenticationRef: + name: keda-trigger-auth-rabbitmq-conn \ No newline at end of file From 9bda5ae79bef058605e21588666b7a6f9b170b7f Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Fri, 13 Oct 2023 15:44:59 +0200 Subject: [PATCH 16/35] BC-5170 code review --- .../infra/preview-generator/preview-generator.service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index fb24a4832fd..7af1f2412c1 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -6,6 +6,8 @@ import { PassThrough } from 'stream'; import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from './interface'; import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; +const imageMagick = subClass({ imageMagick: '7+' }); + @Injectable() export class PreviewGeneratorService { constructor(private readonly storageClient: S3ClientAdapter, private logger: Logger) { @@ -38,9 +40,8 @@ export class PreviewGeneratorService { private resizeAndConvert(original: GetFile, previewParams: PreviewOptions): PassThrough { const { format, width } = previewParams; - const im = subClass({ imageMagick: '7+' }); - const preview = im(original.data); + const preview = imageMagick(original.data); if (width) { preview.resize(width, undefined, '>'); From 24d14408d736b9a8b49566cb551f592a8fb98322 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Sat, 14 Oct 2023 13:37:47 +0200 Subject: [PATCH 17/35] BC-5170 - code review --- .../files-storage-copy-files.api.spec.ts | 2 +- .../files-preview-amqp.module.ts | 4 ++-- .../files-storage/files-storage.config.ts | 8 ++++++-- .../service/preview.service.spec.ts | 12 ----------- .../interface/preview-consumer-config.ts | 11 ++++++++++ .../preview-generator-consumer.module.ts | 20 +++++++------------ .../preview-generator/preview.producer.ts | 3 ++- 7 files changed, 29 insertions(+), 31 deletions(-) create mode 100644 apps/server/src/shared/infra/preview-generator/interface/preview-consumer-config.ts diff --git a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts index ece0175ad35..1f00b5f4229 100644 --- a/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts +++ b/apps/server/src/modules/files-storage/controller/api-test/files-storage-copy-files.api.spec.ts @@ -17,12 +17,12 @@ import { } from '@shared/testing'; import { ICurrentUser } from '@src/modules/authentication'; import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; -import { FILES_STORAGE_S3_CONNECTION } from '@src/modules/files-storage'; import { Request } from 'express'; import FileType from 'file-type-cjs/file-type-cjs-index'; import request from 'supertest'; import { FileRecordParentType } from '../../entity'; import { FilesStorageTestModule } from '../../files-storage-test.module'; +import { FILES_STORAGE_S3_CONNECTION } from '../../files-storage.config'; import { CopyFileParams, CopyFilesOfParentParams, FileRecordListResponse, FileRecordResponse } from '../dto'; import { availableParentTypes } from './mocks'; diff --git a/apps/server/src/modules/files-storage/files-preview-amqp.module.ts b/apps/server/src/modules/files-storage/files-preview-amqp.module.ts index 4080d512e0a..411a26e76d6 100644 --- a/apps/server/src/modules/files-storage/files-preview-amqp.module.ts +++ b/apps/server/src/modules/files-storage/files-preview-amqp.module.ts @@ -1,8 +1,8 @@ import { Module } from '@nestjs/common'; import { PreviewGeneratorConsumerModule } from '@shared/infra/preview-generator'; -import { s3Config } from './files-storage.config'; +import { defaultConfig, s3Config } from './files-storage.config'; @Module({ - imports: [PreviewGeneratorConsumerModule.register(s3Config)], + imports: [PreviewGeneratorConsumerModule.register({ storageConfig: s3Config, serverConfig: defaultConfig })], }) export class PreviewGeneratorAMQPModule {} diff --git a/apps/server/src/modules/files-storage/files-storage.config.ts b/apps/server/src/modules/files-storage/files-storage.config.ts index 7dc01e9f4e1..7fac8ded763 100644 --- a/apps/server/src/modules/files-storage/files-storage.config.ts +++ b/apps/server/src/modules/files-storage/files-storage.config.ts @@ -9,13 +9,17 @@ export interface IFileStorageConfig extends ICoreModuleConfig { USE_STREAM_TO_ANTIVIRUS: boolean; } -const fileStorageConfig: IFileStorageConfig = { +export const defaultConfig = { + NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, INCOMING_REQUEST_TIMEOUT: Configuration.get('FILES_STORAGE__INCOMING_REQUEST_TIMEOUT') as number, +}; + +const fileStorageConfig: IFileStorageConfig = { INCOMING_REQUEST_TIMEOUT_COPY_API: Configuration.get('INCOMING_REQUEST_TIMEOUT_COPY_API') as number, MAX_FILE_SIZE: Configuration.get('FILES_STORAGE__MAX_FILE_SIZE') as number, MAX_SECURITY_CHECK_FILE_SIZE: Configuration.get('FILES_STORAGE__MAX_FILE_SIZE') as number, - NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, USE_STREAM_TO_ANTIVIRUS: Configuration.get('FILES_STORAGE__USE_STREAM_TO_ANTIVIRUS') as boolean, + ...defaultConfig, }; // The configurations lookup diff --git a/apps/server/src/modules/files-storage/service/preview.service.spec.ts b/apps/server/src/modules/files-storage/service/preview.service.spec.ts index 3101aa156eb..3dd334a4da2 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.spec.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.spec.ts @@ -6,7 +6,6 @@ import { PreviewProducer } from '@shared/infra/preview-generator'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { Readable } from 'stream'; import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType, ScanStatus } from '../entity'; import { ErrorType } from '../error'; @@ -19,17 +18,6 @@ import { FileResponseBuilder } from '../mapper'; import { FilesStorageService } from './files-storage.service'; import { PreviewService } from './preview.service'; -const streamMock = jest.fn(); -const resizeMock = jest.fn(); -const imageMagickMock = () => { - return { stream: streamMock, resize: resizeMock, data: Readable.from('text') }; -}; -jest.mock('gm', () => { - return { - subClass: () => imageMagickMock, - }; -}); - const buildFileRecordWithParams = (mimeType: string, scanStatus?: ScanStatus) => { const parentId = new ObjectId().toHexString(); const parentSchoolId = new ObjectId().toHexString(); diff --git a/apps/server/src/shared/infra/preview-generator/interface/preview-consumer-config.ts b/apps/server/src/shared/infra/preview-generator/interface/preview-consumer-config.ts new file mode 100644 index 00000000000..2924fc945bc --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/interface/preview-consumer-config.ts @@ -0,0 +1,11 @@ +import { S3Config } from '@shared/infra/s3-client'; + +export interface PreviewModuleConfig { + NEST_LOG_LEVEL: string; + INCOMING_REQUEST_TIMEOUT: number; +} + +export interface PreviewConfig { + storageConfig: S3Config; + serverConfig: PreviewModuleConfig; +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts index ac72bec2938..9d352b81d9d 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator-consumer.module.ts @@ -1,29 +1,23 @@ -import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { DynamicModule, Module } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq'; -import { S3ClientAdapter, S3ClientModule, S3Config } from '@shared/infra/s3-client'; +import { S3ClientAdapter, S3ClientModule } from '@shared/infra/s3-client'; import { createConfigModuleOptions } from '@src/config'; import { Logger, LoggerModule } from '@src/core/logger'; +import { PreviewConfig } from './interface/preview-consumer-config'; import { PreviewGeneratorConsumer } from './preview-generator.consumer'; import { PreviewGeneratorService } from './preview-generator.service'; -const serverConfig = { - NEST_LOG_LEVEL: Configuration.get('NEST_LOG_LEVEL') as string, - INCOMING_REQUEST_TIMEOUT: Configuration.get('FILES_STORAGE__INCOMING_REQUEST_TIMEOUT') as number, -}; - -export const config = () => serverConfig; - @Module({}) export class PreviewGeneratorConsumerModule { - static register(s3Config: S3Config): DynamicModule { + static register(config: PreviewConfig): DynamicModule { + const { storageConfig, serverConfig } = config; const providers = [ { provide: PreviewGeneratorService, useFactory: (logger: Logger, storageClient: S3ClientAdapter) => new PreviewGeneratorService(storageClient, logger), - inject: [Logger, s3Config.connectionName], + inject: [Logger, storageConfig.connectionName], }, PreviewGeneratorConsumer, ]; @@ -32,9 +26,9 @@ export class PreviewGeneratorConsumerModule { module: PreviewGeneratorConsumerModule, imports: [ LoggerModule, - S3ClientModule.register([s3Config]), + S3ClientModule.register([storageConfig]), RabbitMQWrapperModule, - ConfigModule.forRoot(createConfigModuleOptions(config)), + ConfigModule.forRoot(createConfigModuleOptions(() => serverConfig)), ], providers, }; diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.ts index bbf8287f3f2..602e2503185 100644 --- a/apps/server/src/shared/infra/preview-generator/preview.producer.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.ts @@ -4,6 +4,7 @@ import { ConfigService } from '@nestjs/config'; import { FilesPreviewEvents, FilesPreviewExchange, RpcMessageProducer } from '@shared/infra/rabbitmq'; import { Logger } from '@src/core/logger'; import { PreviewFileOptions, PreviewResponseMessage } from './interface'; +import { PreviewModuleConfig } from './interface/preview-consumer-config'; import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; @Injectable() @@ -11,7 +12,7 @@ export class PreviewProducer extends RpcMessageProducer { constructor( protected readonly amqpConnection: AmqpConnection, private readonly logger: Logger, - protected readonly configService: ConfigService + protected readonly configService: ConfigService ) { const timeout = configService.get('INCOMING_REQUEST_TIMEOUT'); From 25da0bdc5119c99d1c4c9f8dfce6f9a12290d594 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 23 Oct 2023 13:31:49 +0200 Subject: [PATCH 18/35] BC-5170 - fix tests --- .../infra/preview-generator/preview-generator.service.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index 7af1f2412c1..78d47c0aede 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -6,10 +6,10 @@ import { PassThrough } from 'stream'; import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from './interface'; import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; -const imageMagick = subClass({ imageMagick: '7+' }); - @Injectable() export class PreviewGeneratorService { + private imageMagick = subClass({ imageMagick: '7+' }); + constructor(private readonly storageClient: S3ClientAdapter, private logger: Logger) { this.logger.setContext(PreviewGeneratorService.name); } @@ -41,7 +41,7 @@ export class PreviewGeneratorService { private resizeAndConvert(original: GetFile, previewParams: PreviewOptions): PassThrough { const { format, width } = previewParams; - const preview = imageMagick(original.data); + const preview = this.imageMagick(original.data); if (width) { preview.resize(width, undefined, '>'); From cebb08b0f075eabc5829996f3dee99c78c6b4298 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Mon, 23 Oct 2023 18:30:03 +0200 Subject: [PATCH 19/35] BC-5170 - code review --- .../preview.producer.spec.ts | 19 ++- .../rabbitmq/rpc-message-producer.spec.ts | 130 ++++++++++++++++++ .../src/shared/infra/rabbitmq/rpc-message.ts | 2 +- 3 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 apps/server/src/shared/infra/rabbitmq/rpc-message-producer.spec.ts diff --git a/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts index 24a33c1fd07..47adea158a6 100644 --- a/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts +++ b/apps/server/src/shared/infra/preview-generator/preview.producer.spec.ts @@ -1,5 +1,6 @@ import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { InternalServerErrorException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { setupEntities } from '@shared/testing'; @@ -14,8 +15,6 @@ describe('PreviewProducer', () => { let configService: DeepMocked; let amqpConnection: DeepMocked; - const timeout = 10000; - beforeAll(async () => { await setupEntities(); module = await Test.createTestingModule({ @@ -39,7 +38,6 @@ describe('PreviewProducer', () => { service = module.get(PreviewProducer); amqpConnection = module.get(AmqpConnection); configService = module.get(ConfigService); - configService.get.mockReturnValue(timeout); }); afterAll(async () => { @@ -56,6 +54,8 @@ describe('PreviewProducer', () => { describe('generate', () => { describe('when valid params are passed and amqp connection return with a message', () => { const setup = () => { + const timeout = 10000; + const params: PreviewFileOptions = { originFilePath: 'file/test.jpeg', previewFilePath: 'preview/text.webp', @@ -67,6 +67,7 @@ describe('PreviewProducer', () => { const message = []; amqpConnection.request.mockResolvedValueOnce({ message }); + configService.get.mockReturnValue(timeout); const expectedParams = { exchange: FilesPreviewExchange, @@ -106,16 +107,20 @@ describe('PreviewProducer', () => { }, }; - amqpConnection.request.mockResolvedValueOnce({ error: new Error() }); + const error = new Error('An error from called service'); + + amqpConnection.request.mockResolvedValueOnce({ error }); const spy = jest.spyOn(ErrorMapper, 'mapRpcErrorResponseToDomainError'); - return { params, spy }; + return { params, spy, error }; }; it('should call error mapper and throw with error', async () => { - const { params, spy } = setup(); + const { params, spy, error } = setup(); - await expect(service.generate(params)).rejects.toThrowError(); + await expect(service.generate(params)).rejects.toThrowError( + new InternalServerErrorException(null, { cause: error }) + ); expect(spy).toBeCalled(); }); }); diff --git a/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.spec.ts b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.spec.ts new file mode 100644 index 00000000000..b2e94ea676b --- /dev/null +++ b/apps/server/src/shared/infra/rabbitmq/rpc-message-producer.spec.ts @@ -0,0 +1,130 @@ +import { AmqpConnection } from '@golevelup/nestjs-rabbitmq'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ErrorMapper, RpcMessageProducer } from '.'; + +interface TestPayload { + value: boolean; +} + +interface TestResponse { + value: boolean; +} + +const TestEvent = 'test-event'; +const TestExchange = 'test-exchange'; +const timeout = 1000; + +class RpcMessageProducerImp extends RpcMessageProducer { + constructor(protected readonly amqpConnection: AmqpConnection) { + super(amqpConnection, TestExchange, timeout); + } + + async testRequest(payload: TestPayload): Promise { + const response = await this.request(TestEvent, payload); + + return response; + } +} + +describe('RpcMessageProducer', () => { + let service: RpcMessageProducerImp; + let amqpConnection: DeepMocked; + + beforeAll(() => { + amqpConnection = createMock(); + + service = new RpcMessageProducerImp(amqpConnection); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('generate', () => { + describe('when valid params are passed and amqp connection return with a message', () => { + const setup = () => { + const params: TestPayload = { + value: true, + }; + + const message = []; + amqpConnection.request.mockResolvedValueOnce({ message }); + + const expectedParams = { + exchange: TestExchange, + routingKey: TestEvent, + payload: params, + timeout, + }; + + return { params, expectedParams, message }; + }; + + it('should call the ampqConnection.', async () => { + const { params, expectedParams } = setup(); + + await service.testRequest(params); + + expect(amqpConnection.request).toHaveBeenCalledWith(expectedParams); + }); + + it('should return the response message.', async () => { + const { params, message } = setup(); + + const res = await service.testRequest(params); + + expect(res).toEqual(message); + }); + }); + + describe('when amqpConnection return with error in response', () => { + const setup = () => { + const params: TestPayload = { + value: true, + }; + + const error = new Error('An error from called service'); + + amqpConnection.request.mockResolvedValueOnce({ error }); + const spy = jest.spyOn(ErrorMapper, 'mapRpcErrorResponseToDomainError'); + + return { params, spy, error }; + }; + + it('should call error mapper and throw with error', async () => { + const { params, spy, error } = setup(); + + await expect(service.testRequest(params)).rejects.toThrowError( + ErrorMapper.mapRpcErrorResponseToDomainError(error) + ); + expect(spy).toBeCalled(); + }); + }); + + describe('when amqpConnection throw an error', () => { + const setup = () => { + const params: TestPayload = { + value: true, + }; + + const error = new Error('An error from called service'); + + amqpConnection.request.mockRejectedValueOnce(error); + const spy = jest.spyOn(ErrorMapper, 'mapRpcErrorResponseToDomainError'); + + return { params, spy, error }; + }; + + it('should call error mapper and throw with error', async () => { + const { params, spy, error } = setup(); + + await expect(service.testRequest(params)).rejects.toThrowError(error); + expect(spy).not.toBeCalled(); + }); + }); + }); +}); diff --git a/apps/server/src/shared/infra/rabbitmq/rpc-message.ts b/apps/server/src/shared/infra/rabbitmq/rpc-message.ts index 0d512e73b4a..c7e0e7de41f 100644 --- a/apps/server/src/shared/infra/rabbitmq/rpc-message.ts +++ b/apps/server/src/shared/infra/rabbitmq/rpc-message.ts @@ -1,6 +1,6 @@ export interface IError extends Error { status?: number; - message: never; + message: string; } export interface RpcMessage { message: T; From 711d1d6becf804b6567f3b79ef411841256eb2a4 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Tue, 24 Oct 2023 09:21:59 +0200 Subject: [PATCH 20/35] BC-5170 - fix tests --- .../controller/api-test/task-copy-timeout.api.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts b/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts index 446a0fab032..7f9e9a5269f 100644 --- a/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts +++ b/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts @@ -1,9 +1,10 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig'; import { EntityManager } from '@mikro-orm/mongodb'; +import { ICurrentUser } from '@modules/authentication'; +import { JwtAuthGuard } from '@modules/authentication/guard/jwt-auth.guard'; import { ExecutionContext, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { ICurrentUser } from '@modules/authentication'; import { cleanupCollections, courseFactory, @@ -12,14 +13,15 @@ import { taskFactory, userFactory, } from '@shared/testing'; -import { JwtAuthGuard } from '@modules/authentication/guard/jwt-auth.guard'; import { Request } from 'express'; import request from 'supertest'; Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); Configuration.set('INCOMING_REQUEST_TIMEOUT_COPY_API', 1); // eslint-disable-next-line import/first +import { createMock } from '@golevelup/ts-jest'; import { ServerTestModule } from '@modules/server/server.module'; +import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client'; // This needs to be in a separate test file because of the above configuration. // When we find a way to mock the config, it should be moved alongside the other API tests. @@ -42,6 +44,8 @@ describe('Task copy (API)', () => { return true; }, }) + .overrideProvider(FilesStorageClientAdapterService) + .useValue(createMock()) .compile(); app = moduleFixture.createNestApplication(); From 5e05e10acde290eda0cc9daa73df804fbbfbaaf9 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Tue, 24 Oct 2023 09:32:36 +0200 Subject: [PATCH 21/35] BC-5170 - fix lintner --- .../task/controller/api-test/task-copy-timeout.api.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts b/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts index 7f9e9a5269f..4a810eeb0a8 100644 --- a/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts +++ b/apps/server/src/modules/task/controller/api-test/task-copy-timeout.api.spec.ts @@ -1,8 +1,10 @@ +import { createMock } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig'; import { EntityManager } from '@mikro-orm/mongodb'; import { ICurrentUser } from '@modules/authentication'; import { JwtAuthGuard } from '@modules/authentication/guard/jwt-auth.guard'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { ExecutionContext, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -19,9 +21,7 @@ import request from 'supertest'; Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); Configuration.set('INCOMING_REQUEST_TIMEOUT_COPY_API', 1); // eslint-disable-next-line import/first -import { createMock } from '@golevelup/ts-jest'; import { ServerTestModule } from '@modules/server/server.module'; -import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client'; // This needs to be in a separate test file because of the above configuration. // When we find a way to mock the config, it should be moved alongside the other API tests. From 1623cd5274766fb587bc1aad9ffb581216d0f914 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Tue, 24 Oct 2023 17:59:45 +0200 Subject: [PATCH 22/35] BC-5170 - change keda config --- .../schulcloud-server-core/tasks/main.yml | 3 ++ .../preview-generator-scaled-object.yml.j2 | 30 +++---------------- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index fdda78688a1..e5801f36b66 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -109,3 +109,6 @@ kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" template: preview-generator-scaled-object.yml.j2 + when: + - KEDA_ENABLED is defined and KEDA_ENABLED|bool + - SCALED_PREVIEW_GENERATOR_ENABLED is defined and SCALED_PREVIEW_GENERATOR_ENABLED|bool diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 index b0c2db8108f..446ece1f571 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 @@ -1,23 +1,3 @@ -apiVersion: onepassword.com/v1 -kind: OnePasswordItem -metadata: - name: keda-secret - namespace: {{ NAMESPACE }} - labels: - app: keda -spec: - itemPath: "vaults/{{ ONEPASSWORD_OPERATOR_VAULT }}/items/keda" ---- -apiVersion: keda.sh/v1alpha1 -kind: TriggerAuthentication -metadata: - name: keda-trigger-auth-rabbitmq-conn - namespace: {{ NAMESPACE }} -spec: - secretTargetRef: - - parameter: host - name: keda-secret - key: amqp-url --- apiVersion: keda.sh/v1alpha1 kind: ScaledObject @@ -27,11 +7,9 @@ metadata: spec: scaleTargetRef: name: preview-generator-deployment - pollingInterval: 1 - cooldownPeriod: 300 - idleReplicaCount: 0 - minReplicaCount: 0 - maxReplicaCount: 1 + idleReplicaCount: 1 + minReplicaCount: 1 + maxReplicaCount: 5 triggers: - type: rabbitmq metadata: @@ -40,4 +18,4 @@ spec: mode: QueueLength value: "1" authenticationRef: - name: keda-trigger-auth-rabbitmq-conn \ No newline at end of file + name: rabbitmq-trigger-auth From 36e4137959a9f44eaaa4e7ded80fb2cca223f817 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Tue, 24 Oct 2023 19:40:50 +0200 Subject: [PATCH 23/35] BC-5170 - code review --- .../controller/files-storage.consumer.spec.ts | 21 +++++-- .../controller/files-storage.consumer.ts | 8 ++- .../controller/files-storage.controller.ts | 6 +- .../src/modules/files-storage/dto/file.dto.ts | 3 +- .../files-storage-delete.service.spec.ts | 62 +++---------------- .../service/files-storage.service.ts | 8 +-- .../service/preview.service.spec.ts | 6 +- .../files-storage/service/preview.service.ts | 20 +++--- .../uc/files-storage-delete.uc.spec.ts | 12 ++-- .../files-storage/uc/files-storage.uc.ts | 9 +-- 10 files changed, 62 insertions(+), 93 deletions(-) diff --git a/apps/server/src/modules/files-storage/controller/files-storage.consumer.spec.ts b/apps/server/src/modules/files-storage/controller/files-storage.consumer.spec.ts index 124f486dc68..81fd2884347 100644 --- a/apps/server/src/modules/files-storage/controller/files-storage.consumer.spec.ts +++ b/apps/server/src/modules/files-storage/controller/files-storage.consumer.spec.ts @@ -7,6 +7,7 @@ import { courseFactory, fileRecordFactory, setupEntities } from '@shared/testing import { LegacyLogger } from '@src/core/logger'; import { FileRecord, FileRecordParentType } from '../entity'; import { FilesStorageService } from '../service/files-storage.service'; +import { PreviewService } from '../service/preview.service'; import { FileRecordResponse } from './dto'; import { FilesStorageConsumer } from './files-storage.consumer'; @@ -33,6 +34,10 @@ describe('FilesStorageConsumer', () => { provide: FilesStorageService, useValue: createMock(), }, + { + provide: PreviewService, + useValue: createMock(), + }, { provide: LegacyLogger, useValue: createMock(), @@ -165,9 +170,9 @@ describe('FilesStorageConsumer', () => { const parentId = new ObjectId().toHexString(); const fileRecords = fileRecordFactory.buildList(3); - filesStorageService.deleteFilesOfParent.mockResolvedValue([fileRecords, fileRecords.length]); + filesStorageService.getFileRecordsOfParent.mockResolvedValue([fileRecords, fileRecords.length]); - return { parentId }; + return { parentId, fileRecords }; }; it('should call filesStorageService.deleteFilesOfParent with params', async () => { @@ -175,7 +180,15 @@ describe('FilesStorageConsumer', () => { await service.deleteFilesOfParent(parentId); - expect(filesStorageService.deleteFilesOfParent).toBeCalledWith(parentId); + expect(filesStorageService.getFileRecordsOfParent).toBeCalledWith(parentId); + }); + + it('should call filesStorageService.deleteFilesOfParent with params', async () => { + const { parentId, fileRecords } = setup(); + + await service.deleteFilesOfParent(parentId); + + expect(filesStorageService.deleteFilesOfParent).toBeCalledWith(fileRecords); }); it('should return array instances of FileRecordResponse', async () => { @@ -191,7 +204,7 @@ describe('FilesStorageConsumer', () => { const setup = () => { const parentId = new ObjectId().toHexString(); - filesStorageService.deleteFilesOfParent.mockResolvedValue([[], 0]); + filesStorageService.getFileRecordsOfParent.mockResolvedValue([[], 0]); return { parentId }; }; diff --git a/apps/server/src/modules/files-storage/controller/files-storage.consumer.ts b/apps/server/src/modules/files-storage/controller/files-storage.consumer.ts index e7dee3c6b0d..aabefa60f16 100644 --- a/apps/server/src/modules/files-storage/controller/files-storage.consumer.ts +++ b/apps/server/src/modules/files-storage/controller/files-storage.consumer.ts @@ -7,12 +7,14 @@ import { LegacyLogger } from '@src/core/logger'; import { FilesStorageEvents, FilesStorageExchange, ICopyFileDO, IFileDO } from '@src/shared/infra/rabbitmq'; import { FilesStorageMapper } from '../mapper'; import { FilesStorageService } from '../service/files-storage.service'; +import { PreviewService } from '../service/preview.service'; import { CopyFilesOfParentPayload, FileRecordParams } from './dto'; @Injectable() export class FilesStorageConsumer { constructor( private readonly filesStorageService: FilesStorageService, + private readonly previewService: PreviewService, private logger: LegacyLogger, // eslint-disable-next-line @typescript-eslint/no-unused-vars private readonly orm: MikroORM // don't remove it, we need it for @UseRequestContext @@ -61,7 +63,11 @@ export class FilesStorageConsumer { public async deleteFilesOfParent(@RabbitPayload() payload: EntityId): Promise> { this.logger.debug({ action: 'deleteFilesOfParent', payload }); - const [fileRecords, total] = await this.filesStorageService.deleteFilesOfParent(payload); + const [fileRecords, total] = await this.filesStorageService.getFileRecordsOfParent(payload); + + await this.previewService.deletePreviews(fileRecords); + await this.filesStorageService.deleteFilesOfParent(fileRecords); + const response = FilesStorageMapper.mapToFileRecordListResponse(fileRecords, total); return { message: response.data }; 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 564919670e4..736d69e3e29 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 @@ -1,3 +1,4 @@ +import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; import { BadRequestException, Body, @@ -22,10 +23,10 @@ import { UseInterceptors, } from '@nestjs/common'; import { ApiConsumes, ApiHeader, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; -import { ApiValidationError, RequestLoggingInterceptor } from '@shared/common'; +import { ApiValidationError, RequestLoggingInterceptor, RequestTimeout } from '@shared/common'; import { PaginationParams } from '@shared/controller'; -import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; import { Request, Response } from 'express'; +import { config } from '../files-storage.config'; import { GetFileResponse } from '../interface'; import { FilesStorageMapper } from '../mapper'; import { FileRecordMapper } from '../mapper/file-record.mapper'; @@ -126,6 +127,7 @@ export class FilesStorageController { @ApiResponse({ status: 500, type: InternalServerErrorException }) @ApiHeader({ name: 'Range', required: false }) @Get('/preview/:fileRecordId/:fileName') + @RequestTimeout(config().INCOMING_REQUEST_TIMEOUT) async downloadPreview( @Param() params: DownloadFileParams, @CurrentUser() currentUser: ICurrentUser, diff --git a/apps/server/src/modules/files-storage/dto/file.dto.ts b/apps/server/src/modules/files-storage/dto/file.dto.ts index 540e35bb7e6..9668ac3af72 100644 --- a/apps/server/src/modules/files-storage/dto/file.dto.ts +++ b/apps/server/src/modules/files-storage/dto/file.dto.ts @@ -1,6 +1,7 @@ +import { File } from '@shared/infra/s3-client'; import { Readable } from 'stream'; -export class FileDto { +export class FileDto implements File { constructor(file: FileDto) { this.name = file.name; this.data = file.data; diff --git a/apps/server/src/modules/files-storage/service/files-storage-delete.service.spec.ts b/apps/server/src/modules/files-storage/service/files-storage-delete.service.spec.ts index cd76b564b31..3705f93b51c 100644 --- a/apps/server/src/modules/files-storage/service/files-storage-delete.service.spec.ts +++ b/apps/server/src/modules/files-storage/service/files-storage-delete.service.spec.ts @@ -171,35 +171,13 @@ describe('FilesStorageService delete methods', () => { return { parentId, fileRecords }; }; - it('should call findBySchoolIdAndParentId once with correct params', async () => { - const { parentId } = setup(); - - await service.deleteFilesOfParent(parentId); - - expect(fileRecordRepo.findByParentId).toHaveBeenNthCalledWith(1, parentId); - }); - it('should call delete with correct params', async () => { - const { parentId, fileRecords } = setup(); + const { fileRecords } = setup(); - await service.deleteFilesOfParent(parentId); + await service.deleteFilesOfParent(fileRecords); expect(service.delete).toHaveBeenCalledWith(fileRecords); }); - - it('should return file records and count', async () => { - const { parentId, fileRecords } = setup(); - - const responseData = await service.deleteFilesOfParent(parentId); - expect(responseData[0]).toEqual( - expect.arrayContaining([ - expect.objectContaining({ ...fileRecords[0] }), - expect.objectContaining({ ...fileRecords[1] }), - expect.objectContaining({ ...fileRecords[2] }), - ]) - ); - expect(responseData[1]).toEqual(fileRecords.length); - }); }); describe('WHEN no files exists', () => { @@ -215,43 +193,17 @@ describe('FilesStorageService delete methods', () => { const { parentId } = params; spy = jest.spyOn(service, 'delete'); - fileRecordRepo.findByParentId.mockResolvedValueOnce([fileRecords, fileRecords.length]); - return { parentId }; + return { parentId, fileRecords }; }; it('should not call delete', async () => { - const { parentId } = setup(); + const { fileRecords } = setup(); - await service.deleteFilesOfParent(parentId); + await service.deleteFilesOfParent(fileRecords); expect(service.delete).toHaveBeenCalledTimes(0); }); - - it('should return empty counted type', async () => { - const { parentId } = setup(); - - const result = await service.deleteFilesOfParent(parentId); - - expect(result).toEqual([[], 0]); - }); - }); - - describe('WHEN repository throw an error', () => { - const setup = () => { - const { params } = buildFileRecordsWithParams(); - const { parentId } = params; - - fileRecordRepo.findByParentId.mockRejectedValueOnce(new Error('bla')); - - return { parentId }; - }; - - it('should pass the error', async () => { - const { parentId } = setup(); - - await expect(service.deleteFilesOfParent(parentId)).rejects.toThrow(new Error('bla')); - }); }); describe('WHEN service.delete throw an error', () => { @@ -272,9 +224,9 @@ describe('FilesStorageService delete methods', () => { }; it('should pass the error', async () => { - const { parentId } = setup(); + const { fileRecords } = setup(); - await expect(service.deleteFilesOfParent(parentId)).rejects.toThrow(new Error('bla')); + await expect(service.deleteFilesOfParent(fileRecords)).rejects.toThrow(new Error('bla')); }); }); }); 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 0b851ac8de0..209f1804d3e 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 @@ -304,14 +304,10 @@ export class FilesStorageService { await this.deleteWithRollbackByError(fileRecords); } - public async deleteFilesOfParent(parentId: EntityId): Promise> { - const [fileRecords, count] = await this.getFileRecordsOfParent(parentId); - - if (count > 0) { + public async deleteFilesOfParent(fileRecords: FileRecord[]): Promise { + if (fileRecords.length > 0) { await this.delete(fileRecords); } - - return [fileRecords, count]; } // restore diff --git a/apps/server/src/modules/files-storage/service/preview.service.spec.ts b/apps/server/src/modules/files-storage/service/preview.service.spec.ts index 3dd334a4da2..f02f48aee21 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.spec.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.spec.ts @@ -594,10 +594,10 @@ describe('PreviewService', () => { }; }; - it('does not pass error', async () => { - const { fileRecord } = setup(); + it('should throw error', async () => { + const { fileRecord, error } = setup(); - await previewService.deletePreviews([fileRecord]); + await expect(previewService.deletePreviews([fileRecord])).rejects.toThrowError(error); }); }); }); diff --git a/apps/server/src/modules/files-storage/service/preview.service.ts b/apps/server/src/modules/files-storage/service/preview.service.ts index 8e5a1470784..2d56ec91b0d 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.ts @@ -45,27 +45,17 @@ export class PreviewService { bytesRange, }; - if (previewParams.forceUpdate) { - await this.generatePreview(previewFileParams); - } - const response = await this.tryGetPreviewOrGenerate(previewFileParams); return response; } public async deletePreviews(fileRecords: FileRecord[]): Promise { - try { - const paths = fileRecords.map((fileRecord) => - createPreviewDirectoryPath(fileRecord.getSchoolId(), fileRecord.id) - ); + const paths = fileRecords.map((fileRecord) => createPreviewDirectoryPath(fileRecord.getSchoolId(), fileRecord.id)); - const promises = paths.map((path) => this.storageClient.deleteDirectory(path)); + const promises = paths.map((path) => this.storageClient.deleteDirectory(path)); - await Promise.all(promises); - } catch (error) { - this.logger.warn(error); - } + await Promise.all(promises); } private checkIfPreviewPossible(fileRecord: FileRecord): void | UnprocessableEntityException { @@ -79,6 +69,10 @@ export class PreviewService { let file: GetFileResponse; try { + if (params.previewParams.forceUpdate) { + await this.generatePreview(params); + } + file = await this.getPreviewFile(params); } catch (error) { if (!(error instanceof NotFoundException)) { diff --git a/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts index a1aaf0342ee..b12006367aa 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts @@ -1,5 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; +import { AuthorizationReferenceService } from '@modules/authorization/domain'; import { HttpService } from '@nestjs/axios'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -8,7 +9,6 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationReferenceService } from '@modules/authorization/domain'; import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -123,7 +123,7 @@ describe('FilesStorageUC delete methods', () => { const mockedResult = [[fileRecord], 0] as Counted; authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); - filesStorageService.deleteFilesOfParent.mockResolvedValueOnce(mockedResult); + filesStorageService.getFileRecordsOfParent.mockResolvedValueOnce(mockedResult); return { params, userId, mockedResult, requestParams, fileRecord }; }; @@ -143,11 +143,11 @@ describe('FilesStorageUC delete methods', () => { }); it('should call service with correct params', async () => { - const { requestParams, userId } = setup(); + const { requestParams, userId, fileRecord } = setup(); await filesStorageUC.deleteFilesOfParent(userId, requestParams); - expect(filesStorageService.deleteFilesOfParent).toHaveBeenCalledWith(requestParams.parentId); + expect(filesStorageService.deleteFilesOfParent).toHaveBeenCalledWith([fileRecord]); }); it('should call deletePreviews', async () => { @@ -189,10 +189,14 @@ describe('FilesStorageUC delete methods', () => { describe('WHEN service throws error', () => { const setup = () => { + const { fileRecords } = buildFileRecordsWithParams(); + + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); const { requestParams, userId } = createParams(); const error = new Error('test'); authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); + filesStorageService.getFileRecordsOfParent.mockResolvedValueOnce([fileRecords, fileRecords.length]); filesStorageService.deleteFilesOfParent.mockRejectedValueOnce(error); return { requestParams, userId, 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 41387b6394a..47eb7c79104 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,9 +1,9 @@ +import { AuthorizationContext } from '@modules/authorization'; +import { AuthorizationReferenceService } from '@modules/authorization/domain'; import { HttpService } from '@nestjs/axios'; import { Injectable, NotFoundException } from '@nestjs/common'; import { Counted, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationContext } from '@modules/authorization'; -import { AuthorizationReferenceService } from '@modules/authorization/domain'; import { AxiosRequestConfig, AxiosResponse } from 'axios'; import busboy from 'busboy'; import { Request } from 'express'; @@ -164,8 +164,9 @@ export class FilesStorageUC { // delete public async deleteFilesOfParent(userId: EntityId, params: FileRecordParams): Promise> { await this.checkPermission(userId, params.parentType, params.parentId, FileStorageAuthorizationContext.delete); - const [fileRecords, count] = await this.filesStorageService.deleteFilesOfParent(params.parentId); + const [fileRecords, count] = await this.filesStorageService.getFileRecordsOfParent(params.parentId); await this.previewService.deletePreviews(fileRecords); + await this.filesStorageService.deleteFilesOfParent(fileRecords); return [fileRecords, count]; } @@ -175,8 +176,8 @@ export class FilesStorageUC { const { parentType, parentId } = fileRecord.getParentInfo(); await this.checkPermission(userId, parentType, parentId, FileStorageAuthorizationContext.delete); - await this.filesStorageService.delete([fileRecord]); await this.previewService.deletePreviews([fileRecord]); + await this.filesStorageService.delete([fileRecord]); return fileRecord; } From da6cd8135cabc96cb21fe4a6414cf1cc8549ffcf Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:10:25 +0200 Subject: [PATCH 24/35] Update ansible/roles/schulcloud-server-core/tasks/main.yml Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- ansible/roles/schulcloud-server-core/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index e5801f36b66..784fba4c299 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -84,7 +84,7 @@ namespace: "{{ NAMESPACE }}" template: amqp-files-deployment.yml.j2 - - name: PreviewGeneratorDeployment + - name: Preview Generator Deployment kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" From 588e0226435e2ba03efa66c7cb6d0d304d625c35 Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:10:36 +0200 Subject: [PATCH 25/35] Update ansible/roles/schulcloud-server-core/tasks/main.yml Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- ansible/roles/schulcloud-server-core/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 784fba4c299..18f3e356cdf 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -90,7 +90,7 @@ namespace: "{{ NAMESPACE }}" template: preview-generator-deployment.yml.j2 - - name: PreviewGeneratorConfigmap + - name: preview generator configmap kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" From 634db781de82e7ed7962e5846e9a6a55c7484b8c Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:10:49 +0200 Subject: [PATCH 26/35] Update ansible/roles/schulcloud-server-core/tasks/main.yml Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- ansible/roles/schulcloud-server-core/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 18f3e356cdf..646424d505c 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -97,7 +97,7 @@ template: preview-generator-configmap.yml.j2 apply: yes - - name: PreviewGenerator Secret by 1Password + - name: preview generator Secret by 1Password kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" From 26563ed6abfdad3a49dde47a693f5421068db645 Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:11:02 +0200 Subject: [PATCH 27/35] Update ansible/roles/schulcloud-server-core/tasks/main.yml Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- ansible/roles/schulcloud-server-core/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 646424d505c..7f1bbeeecfe 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -104,7 +104,7 @@ template: preview-generator-onepassword.yml.j2 when: ONEPASSWORD_OPERATOR is defined and ONEPASSWORD_OPERATOR|bool - - name: PreviewGeneratorScaledObject + - name: preview generator scaled object kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" From 6966569b0048c649598faa0cd225268bb5a26c4f Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:11:52 +0200 Subject: [PATCH 28/35] Update ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- .../templates/preview-generator-scaled-object.yml.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 index 446ece1f571..f8b1fd84b24 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 @@ -2,7 +2,7 @@ apiVersion: keda.sh/v1alpha1 kind: ScaledObject metadata: - name: rabbitmq-scaledobject + name: preview-generator-rabbitmq-scaledobject namespace: {{ NAMESPACE }} spec: scaleTargetRef: From bfc70c96ae74787f6f7298586c8326dad91097d5 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Wed, 25 Oct 2023 16:09:06 +0200 Subject: [PATCH 29/35] BC-5170 - code review --- .../files-storage/helper/file-record.spec.ts | 38 +++++++++++- .../files-storage/helper/file-record.ts | 19 ++++++ .../src/modules/files-storage/mapper/index.ts | 1 + .../mapper/preview.builder.spec.ts | 61 +++++++++++++++++++ .../files-storage/mapper/preview.builder.ts | 47 ++++++++++++++ .../files-storage/service/preview.service.ts | 48 ++------------- .../loggable/preview-actions.loggable.spec.ts | 37 +++++++++++ .../preview-generator.builder.spec.ts | 28 +++++++++ .../preview-generator.builder.ts | 16 +++++ .../preview-generator.service.ts | 4 +- 10 files changed, 254 insertions(+), 45 deletions(-) create mode 100644 apps/server/src/modules/files-storage/mapper/preview.builder.spec.ts create mode 100644 apps/server/src/modules/files-storage/mapper/preview.builder.ts create mode 100644 apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.spec.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.builder.spec.ts create mode 100644 apps/server/src/shared/infra/preview-generator/preview-generator.builder.ts diff --git a/apps/server/src/modules/files-storage/helper/file-record.spec.ts b/apps/server/src/modules/files-storage/helper/file-record.spec.ts index 6d975ebe21b..defdf545f8e 100644 --- a/apps/server/src/modules/files-storage/helper/file-record.spec.ts +++ b/apps/server/src/modules/files-storage/helper/file-record.spec.ts @@ -1,8 +1,9 @@ import { EntityId } from '@shared/domain'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { ObjectId } from 'bson'; -import { createFileRecord, markForDelete, unmarkForDelete } from '.'; +import { createFileRecord, getFormat, getPreviewName, markForDelete, unmarkForDelete } from '.'; import { FileRecord } from '../entity'; +import { PreviewOutputMimeTypes } from '../interface'; describe('File Record Helper', () => { const setupFileRecords = () => { @@ -88,4 +89,39 @@ describe('File Record Helper', () => { expect(newFileRecord).toEqual(expect.any(FileRecord)); }); }); + + describe('getFormat is called', () => { + it('should return format', () => { + const mimeType = 'image/jpeg'; + + const result = getFormat(mimeType); + + expect(result).toEqual('jpeg'); + }); + }); + + describe('getPreviewName is called', () => { + const setup = () => { + const fileRecord = fileRecordFactory.buildWithId(); + const outputFormat = PreviewOutputMimeTypes.IMAGE_WEBP; + + return { fileRecord, outputFormat }; + }; + + it('should return origin file name', () => { + const { fileRecord } = setup(); + + const result = getPreviewName(fileRecord, undefined); + + expect(result).toEqual(fileRecord.name); + }); + + it('should return preview name', () => { + const { fileRecord, outputFormat } = setup(); + + const result = getPreviewName(fileRecord, outputFormat); + + expect(result).toEqual(`${fileRecord.name.split('.')[0]}.webp`); + }); + }); }); 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 c0984063420..6a0d5b39e5f 100644 --- a/apps/server/src/modules/files-storage/helper/file-record.ts +++ b/apps/server/src/modules/files-storage/helper/file-record.ts @@ -1,5 +1,6 @@ import { FileRecordParams } from '../controller/dto'; import { FileRecord } from '../entity'; +import { PreviewOutputMimeTypes } from '../interface'; export function markForDelete(fileRecords: FileRecord[]): FileRecord[] { const markedFileRecords = fileRecords.map((fileRecord) => { @@ -38,3 +39,21 @@ export function createFileRecord( return entity; } + +export function getFormat(mimeType: string): string { + const format = mimeType.split('/')[1]; + + return format; +} + +export function getPreviewName(fileRecord: FileRecord, outputFormat?: PreviewOutputMimeTypes): string { + if (!outputFormat) { + return fileRecord.name; + } + + const fileNameWithoutExtension = fileRecord.name.split('.')[0]; + const format = getFormat(outputFormat); + const name = `${fileNameWithoutExtension}.${format}`; + + return name; +} diff --git a/apps/server/src/modules/files-storage/mapper/index.ts b/apps/server/src/modules/files-storage/mapper/index.ts index 556e7508929..bc8af7f7f05 100644 --- a/apps/server/src/modules/files-storage/mapper/index.ts +++ b/apps/server/src/modules/files-storage/mapper/index.ts @@ -3,3 +3,4 @@ export * from './file-dto.builder'; export * from './file-record.mapper'; export * from './file-response.builder'; export * from './files-storage.mapper'; +export * from './preview.builder'; diff --git a/apps/server/src/modules/files-storage/mapper/preview.builder.spec.ts b/apps/server/src/modules/files-storage/mapper/preview.builder.spec.ts new file mode 100644 index 00000000000..1a3cc843f86 --- /dev/null +++ b/apps/server/src/modules/files-storage/mapper/preview.builder.spec.ts @@ -0,0 +1,61 @@ +import { fileRecordFactory } from '@shared/testing'; +import { PreviewOutputMimeTypes } from '../interface'; +import { PreviewBuilder } from './preview.builder'; + +describe('PreviewBuilder', () => { + describe('buildParams is called', () => { + const setup = () => { + const fileRecord = fileRecordFactory.buildWithId(); + const previewParams = { outputFormat: PreviewOutputMimeTypes.IMAGE_WEBP }; + const bytesRange = 'bytes=0-100'; + + const expectedResponse = { + fileRecord, + previewParams, + hash: expect.any(String), + previewFilePath: expect.any(String), + originFilePath: expect.any(String), + format: expect.any(String), + bytesRange, + }; + + return { fileRecord, previewParams, bytesRange, expectedResponse }; + }; + + it('should return preview file params', () => { + const { fileRecord, previewParams, bytesRange, expectedResponse } = setup(); + + const result = PreviewBuilder.buildParams(fileRecord, previewParams, bytesRange); + + expect(result).toEqual(expectedResponse); + }); + }); + + describe('buildPayload is called', () => { + const setup = () => { + const fileRecord = fileRecordFactory.buildWithId(); + const previewParams = { outputFormat: PreviewOutputMimeTypes.IMAGE_WEBP }; + const bytesRange = 'bytes=0-100'; + const previewFileParams = PreviewBuilder.buildParams(fileRecord, previewParams, bytesRange); + + const expectedResponse = { + originFilePath: previewFileParams.originFilePath, + previewFilePath: previewFileParams.previewFilePath, + previewOptions: { + format: previewFileParams.format, + width: previewFileParams.previewParams.width, + }, + }; + + return { previewFileParams, expectedResponse }; + }; + + it('should return preview payload', () => { + const { previewFileParams, expectedResponse } = setup(); + + const result = PreviewBuilder.buildPayload(previewFileParams); + + expect(result).toEqual(expectedResponse); + }); + }); +}); diff --git a/apps/server/src/modules/files-storage/mapper/preview.builder.ts b/apps/server/src/modules/files-storage/mapper/preview.builder.ts new file mode 100644 index 00000000000..83a16448a98 --- /dev/null +++ b/apps/server/src/modules/files-storage/mapper/preview.builder.ts @@ -0,0 +1,47 @@ +import { PreviewFileOptions } from '@shared/infra/preview-generator'; +import { PreviewParams } from '../controller/dto'; +import { FileRecord } from '../entity'; +import { createPath, createPreviewFilePath, createPreviewNameHash, getFormat } from '../helper'; +import { PreviewFileParams } from '../interface'; + +export class PreviewBuilder { + public static buildParams( + fileRecord: FileRecord, + previewParams: PreviewParams, + bytesRange: string | undefined + ): PreviewFileParams { + const { schoolId, id, mimeType } = fileRecord; + const originFilePath = createPath(schoolId, id); + const format = getFormat(previewParams.outputFormat ?? mimeType); + + const hash = createPreviewNameHash(id, previewParams); + const previewFilePath = createPreviewFilePath(schoolId, hash, id); + + const previewFileParams = { + fileRecord, + previewParams, + hash, + previewFilePath, + originFilePath, + format, + bytesRange, + }; + + return previewFileParams; + } + + public static buildPayload(params: PreviewFileParams): PreviewFileOptions { + const { originFilePath, previewFilePath, previewParams, format } = params; + + const payload = { + originFilePath, + previewFilePath, + previewOptions: { + format, + width: previewParams.width, + }, + }; + + return payload; + } +} diff --git a/apps/server/src/modules/files-storage/service/preview.service.ts b/apps/server/src/modules/files-storage/service/preview.service.ts index 2d56ec91b0d..e27fbc0645a 100644 --- a/apps/server/src/modules/files-storage/service/preview.service.ts +++ b/apps/server/src/modules/files-storage/service/preview.service.ts @@ -6,10 +6,9 @@ import { PreviewParams } from '../controller/dto'; import { FileRecord, PreviewStatus } from '../entity'; import { ErrorType } from '../error'; import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config'; -import { createPath, createPreviewDirectoryPath, createPreviewFilePath, createPreviewNameHash } from '../helper'; +import { createPreviewDirectoryPath, getPreviewName } from '../helper'; import { GetFileResponse, PreviewFileParams } from '../interface'; -import { PreviewOutputMimeTypes } from '../interface/preview-output-mime-types.enum'; -import { FileResponseBuilder } from '../mapper'; +import { FileResponseBuilder, PreviewBuilder } from '../mapper'; @Injectable() export class PreviewService { @@ -28,22 +27,7 @@ export class PreviewService { ): Promise { this.checkIfPreviewPossible(fileRecord); - const { schoolId, id, mimeType } = fileRecord; - const originFilePath = createPath(schoolId, id); - const format = this.getFormat(previewParams.outputFormat ?? mimeType); - - const hash = createPreviewNameHash(id, previewParams); - const previewFilePath = createPreviewFilePath(schoolId, hash, id); - - const previewFileParams = { - fileRecord, - previewParams, - hash, - previewFilePath, - originFilePath, - format, - bytesRange, - }; + const previewFileParams = PreviewBuilder.buildParams(fileRecord, previewParams, bytesRange); const response = await this.tryGetPreviewOrGenerate(previewFileParams); @@ -88,7 +72,7 @@ export class PreviewService { private async getPreviewFile(params: PreviewFileParams): Promise { const { fileRecord, previewFilePath, bytesRange, previewParams } = params; - const name = this.getPreviewName(fileRecord, previewParams.outputFormat); + const name = getPreviewName(fileRecord, previewParams.outputFormat); const file = await this.storageClient.get(previewFilePath, bytesRange); const response = FileResponseBuilder.build(file, name); @@ -97,30 +81,8 @@ export class PreviewService { } private async generatePreview(params: PreviewFileParams): Promise { - const payload = { - originFilePath: params.originFilePath, - previewFilePath: params.previewFilePath, - previewOptions: { width: params.previewParams.width, format: params.format }, - }; + const payload = PreviewBuilder.buildPayload(params); await this.previewProducer.generate(payload); } - - private getFormat(mimeType: string): string { - const format = mimeType.split('/')[1]; - - return format; - } - - private getPreviewName(fileRecord: FileRecord, outputFormat?: PreviewOutputMimeTypes): string { - if (!outputFormat) { - return fileRecord.name; - } - - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; - const format = this.getFormat(outputFormat); - const name = `${fileNameWithoutExtension}.${format}`; - - return name; - } } diff --git a/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.spec.ts b/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.spec.ts new file mode 100644 index 00000000000..04d56a991f1 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/loggable/preview-actions.loggable.spec.ts @@ -0,0 +1,37 @@ +import { PreviewActionsLoggable } from './preview-actions.loggable'; + +describe('PreviewActionsLoggable', () => { + describe('getLogMessage is called', () => { + const setup = () => { + const message = 'message'; + const payload = { + originFilePath: 'originFilePath', + previewFilePath: 'previewFilePath', + previewOptions: { + format: 'webp', + width: 100, + }, + }; + + const expectedResponse = { + message, + data: { + originFilePath: payload.originFilePath, + previewFilePath: payload.previewFilePath, + format: payload.previewOptions.format, + width: payload.previewOptions.width, + }, + }; + + return { message, payload, expectedResponse }; + }; + + it('should return log message', () => { + const { message, payload, expectedResponse } = setup(); + + const result = new PreviewActionsLoggable(message, payload).getLogMessage(); + + expect(result).toEqual(expectedResponse); + }); + }); +}); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.builder.spec.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.builder.spec.ts new file mode 100644 index 00000000000..2caaad7abe8 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.builder.spec.ts @@ -0,0 +1,28 @@ +import { PassThrough } from 'stream'; +import { PreviewGeneratorBuilder } from './preview-generator.builder'; + +describe('PreviewGeneratorBuilder', () => { + describe('buildFile is called', () => { + const setup = () => { + const preview = new PassThrough(); + const previewOptions = { + format: 'webp', + }; + + const expectedResponse = { + data: preview, + mimeType: previewOptions.format, + }; + + return { preview, previewOptions, expectedResponse }; + }; + + it('should return preview file', () => { + const { preview, previewOptions, expectedResponse } = setup(); + + const result = PreviewGeneratorBuilder.buildFile(preview, previewOptions); + + expect(result).toEqual(expectedResponse); + }); + }); +}); diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.builder.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.builder.ts new file mode 100644 index 00000000000..4c5561ed089 --- /dev/null +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.builder.ts @@ -0,0 +1,16 @@ +import { File } from '@shared/infra/s3-client'; +import { PassThrough } from 'stream'; +import { PreviewOptions } from './interface'; + +export class PreviewGeneratorBuilder { + public static buildFile(preview: PassThrough, previewOptions: PreviewOptions): File { + const { format } = previewOptions; + + const file = { + data: preview, + mimeType: format, + }; + + return file; + } +} diff --git a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts index 78d47c0aede..72dac25f076 100644 --- a/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts +++ b/apps/server/src/shared/infra/preview-generator/preview-generator.service.ts @@ -5,6 +5,7 @@ import { subClass } from 'gm'; import { PassThrough } from 'stream'; import { PreviewFileOptions, PreviewOptions, PreviewResponseMessage } from './interface'; import { PreviewActionsLoggable } from './loggable/preview-actions.loggable'; +import { PreviewGeneratorBuilder } from './preview-generator.builder'; @Injectable() export class PreviewGeneratorService { @@ -21,7 +22,8 @@ export class PreviewGeneratorService { const original = await this.downloadOriginFile(originFilePath); const preview = this.resizeAndConvert(original, previewOptions); - const file = { data: preview, mimeType: previewOptions.format }; + const file = PreviewGeneratorBuilder.buildFile(preview, params.previewOptions); + await this.storageClient.create(previewFilePath, file); this.logger.debug(new PreviewActionsLoggable('PreviewGeneratorService.generatePreview:end', params)); From bebde5c5a03ed8ab6d4dcd98698af96d2444b780 Mon Sep 17 00:00:00 2001 From: Sergej Hoffmann <97111299+SevenWaysDP@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:18:59 +0200 Subject: [PATCH 30/35] Update ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 Co-authored-by: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> --- .../templates/preview-generator-deployment.yml.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 index 0c16be66532..51d87b88755 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-deployment.yml.j2 @@ -39,8 +39,8 @@ spec: command: ['npm', 'run', 'nest:start:preview-generator-amqp:prod'] resources: limits: - cpu: {{ AMQP_FILE_PREVIEW_CPU_LIMITS|default("1000m", true) }} - memory: {{ AMQP_FILE_PREVIEW_MEMORY_LIMITS|default("2500Mi", true) }} + cpu: {{ AMQP_FILE_PREVIEW_CPU_LIMITS|default("4000m", true) }} + memory: {{ AMQP_FILE_PREVIEW_MEMORY_LIMITS|default("4000Mi", true) }} requests: cpu: {{ AMQP_FILE_PREVIEW_CPU_REQUESTS|default("100m", true) }} - memory: {{ AMQP_FILE_PREVIEW_MEMORY_REQUESTS|default("50Mi", true) }} + memory: {{ AMQP_FILE_PREVIEW_MEMORY_REQUESTS|default("250Mi", true) }} From 44e0eaf7eb86ebb344562e885ec88fac66dca9da Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Wed, 25 Oct 2023 16:20:02 +0200 Subject: [PATCH 31/35] BC-5170 - add label --- .../templates/preview-generator-scaled-object.yml.j2 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 index f8b1fd84b24..795f189eff2 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 @@ -4,6 +4,8 @@ kind: ScaledObject metadata: name: preview-generator-rabbitmq-scaledobject namespace: {{ NAMESPACE }} + labels: + app: preview-generator spec: scaleTargetRef: name: preview-generator-deployment From 4ebdb1d546e7a3b99a740048b5868dc9966752ca Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 26 Oct 2023 10:00:03 +0200 Subject: [PATCH 32/35] BC-5170 - add configs --- .../templates/preview-generator-scaled-object.yml.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 index 795f189eff2..2f8f9091b8e 100644 --- a/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 +++ b/ansible/roles/schulcloud-server-core/templates/preview-generator-scaled-object.yml.j2 @@ -9,9 +9,9 @@ metadata: spec: scaleTargetRef: name: preview-generator-deployment - idleReplicaCount: 1 - minReplicaCount: 1 - maxReplicaCount: 5 + idleReplicaCount: {{ AMQP_FILE_PREVIEW_IDLE_REPLICA_COUNT|default("1", true) }} + minReplicaCount: {{ AMQP_FILE_PREVIEW_MIN_REPLICA_COUNT|default("1", true) }} + maxReplicaCount: {{ AMQP_FILE_PREVIEW_MAX_REPLICA_COUNT|default("5", true) }} triggers: - type: rabbitmq metadata: From 96989b288659de11d6fc06b87b1fcd7a1705259c Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 26 Oct 2023 12:42:59 +0200 Subject: [PATCH 33/35] BC-5170 - code review --- .../entity/filerecord.entity.spec.ts | 34 +++++++++++++++++++ .../files-storage/entity/filerecord.entity.ts | 6 ++++ .../files-storage/helper/file-record.spec.ts | 8 ++++- .../files-storage/helper/file-record.ts | 14 +++++--- 4 files changed, 57 insertions(+), 5 deletions(-) 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 f07d88d85fd..d508b264605 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 @@ -783,4 +783,38 @@ describe('FileRecord Entity', () => { }); }); }); + + describe('fileNameWithoutExtension is called', () => { + describe('WHEN file name has extension', () => { + const setup = () => { + const fileRecord = fileRecordFactory.build({ name: 'file-name.jpg' }); + + return { fileRecord }; + }; + + it('should return the correct file name without extension', () => { + const { fileRecord } = setup(); + + const result = fileRecord.fileNameWithoutExtension; + + expect(result).toEqual('file-name'); + }); + }); + + describe('WHEN file name has not extension', () => { + const setup = () => { + const fileRecord = fileRecordFactory.build({ name: 'file-name' }); + + return { fileRecord }; + }; + + it('should return the correct file name without extension', () => { + const { fileRecord } = setup(); + + const result = fileRecord.fileNameWithoutExtension; + + expect(result).toEqual('file-name'); + }); + }); + }); }); 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 a87789d30fc..19e147469d7 100644 --- a/apps/server/src/modules/files-storage/entity/filerecord.entity.ts +++ b/apps/server/src/modules/files-storage/entity/filerecord.entity.ts @@ -293,4 +293,10 @@ export class FileRecord extends BaseEntityWithTimestamps { return PreviewStatus.PREVIEW_NOT_POSSIBLE_SCAN_STATUS_ERROR; } + + public get fileNameWithoutExtension(): string { + const fileNameWithoutExtension = this.name.split('.')[0]; + + return fileNameWithoutExtension; + } } diff --git a/apps/server/src/modules/files-storage/helper/file-record.spec.ts b/apps/server/src/modules/files-storage/helper/file-record.spec.ts index defdf545f8e..a999b599936 100644 --- a/apps/server/src/modules/files-storage/helper/file-record.spec.ts +++ b/apps/server/src/modules/files-storage/helper/file-record.spec.ts @@ -98,6 +98,12 @@ describe('File Record Helper', () => { expect(result).toEqual('jpeg'); }); + + it('should throw error', () => { + const mimeType = 'image'; + + expect(() => getFormat(mimeType)).toThrowError(`could not get format from mime type: ${mimeType}`); + }); }); describe('getPreviewName is called', () => { @@ -116,7 +122,7 @@ describe('File Record Helper', () => { expect(result).toEqual(fileRecord.name); }); - it('should return preview name', () => { + it('should return preview name with format', () => { const { fileRecord, outputFormat } = setup(); const result = getPreviewName(fileRecord, outputFormat); 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 6a0d5b39e5f..ed291661735 100644 --- a/apps/server/src/modules/files-storage/helper/file-record.ts +++ b/apps/server/src/modules/files-storage/helper/file-record.ts @@ -1,3 +1,4 @@ +import { InternalServerErrorException } from '@nestjs/common'; import { FileRecordParams } from '../controller/dto'; import { FileRecord } from '../entity'; import { PreviewOutputMimeTypes } from '../interface'; @@ -43,17 +44,22 @@ export function createFileRecord( export function getFormat(mimeType: string): string { const format = mimeType.split('/')[1]; + if (!format) { + throw new InternalServerErrorException(`could not get format from mime type: ${mimeType}`); + } + return format; } export function getPreviewName(fileRecord: FileRecord, outputFormat?: PreviewOutputMimeTypes): string { + const { fileNameWithoutExtension, name } = fileRecord; + if (!outputFormat) { - return fileRecord.name; + return name; } - const fileNameWithoutExtension = fileRecord.name.split('.')[0]; const format = getFormat(outputFormat); - const name = `${fileNameWithoutExtension}.${format}`; + const previewFileName = `${fileNameWithoutExtension}.${format}`; - return name; + return previewFileName; } From a5f0fb0f10f1ebcdc76a8f72b4125efb2a177723 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 26 Oct 2023 14:17:42 +0200 Subject: [PATCH 34/35] BC-5170 - change logic for file name --- .../entity/filerecord.entity.spec.ts | 16 ++++++++++++++++ .../files-storage/entity/filerecord.entity.ts | 5 +++-- 2 files changed, 19 insertions(+), 2 deletions(-) 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 d508b264605..ba484fcf0ad 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 @@ -816,5 +816,21 @@ describe('FileRecord Entity', () => { expect(result).toEqual('file-name'); }); }); + + describe('WHEN file name starts with dot', () => { + const setup = () => { + const fileRecord = fileRecordFactory.build({ name: '.file-name.jpg' }); + + return { fileRecord }; + }; + + it('should return the correct file name without extension', () => { + const { fileRecord } = setup(); + + const result = fileRecord.fileNameWithoutExtension; + + expect(result).toEqual('.file-name'); + }); + }); }); }); 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 19e147469d7..26f2807924c 100644 --- a/apps/server/src/modules/files-storage/entity/filerecord.entity.ts +++ b/apps/server/src/modules/files-storage/entity/filerecord.entity.ts @@ -2,6 +2,7 @@ import { Embeddable, Embedded, Entity, Enum, Index, Property } from '@mikro-orm/ import { ObjectId } from '@mikro-orm/mongodb'; import { BadRequestException } from '@nestjs/common'; import { BaseEntityWithTimestamps, EntityId } from '@shared/domain'; +import path from 'path'; import { v4 as uuid } from 'uuid'; import { ErrorType } from '../error'; import { PreviewInputMimeTypes } from '../interface/preview-input-mime-types.enum'; @@ -295,8 +296,8 @@ export class FileRecord extends BaseEntityWithTimestamps { } public get fileNameWithoutExtension(): string { - const fileNameWithoutExtension = this.name.split('.')[0]; + const filenameObj = path.parse(this.name); - return fileNameWithoutExtension; + return filenameObj.name; } } From 5ea34cc37db578aa2cbd3c6d2c161033b5130466 Mon Sep 17 00:00:00 2001 From: SevenWaysDP Date: Thu, 26 Oct 2023 14:31:04 +0200 Subject: [PATCH 35/35] BC-5170 - because Cedric wishs ;o) --- .../modules/files-storage/entity/filerecord.entity.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 ba484fcf0ad..f497ff39a6c 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 @@ -819,7 +819,7 @@ describe('FileRecord Entity', () => { describe('WHEN file name starts with dot', () => { const setup = () => { - const fileRecord = fileRecordFactory.build({ name: '.file-name.jpg' }); + const fileRecord = fileRecordFactory.build({ name: '.bild.123.jpg' }); return { fileRecord }; }; @@ -829,7 +829,7 @@ describe('FileRecord Entity', () => { const result = fileRecord.fileNameWithoutExtension; - expect(result).toEqual('.file-name'); + expect(result).toEqual('.bild.123'); }); }); });