Skip to content

Commit

Permalink
BC-2552 - Filestorage service refactoring (#3670)
Browse files Browse the repository at this point in the history
  • Loading branch information
bischofmax authored Nov 1, 2022
1 parent 0fbafee commit cef88d9
Show file tree
Hide file tree
Showing 35 changed files with 3,319 additions and 1,448 deletions.
1 change: 1 addition & 0 deletions apps/server/src/core/core.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ import { ValidationModule } from './validation';
*/
@Module({
imports: [LoggerModule, ErrorModule, ValidationModule, InterceptorModule],
exports: [LoggerModule],
})
export class CoreModule {}
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Body, Controller, Get, Param, Put, Req, StreamableFile } from '@nestjs/common';
import { ApiExcludeEndpoint, ApiTags } from '@nestjs/swagger';
import { ScanResultParams } from '@src/modules/files-storage/controller/dto';
import { Request } from 'express';
import { FilesStorageInternalActions } from '../files-storage.const';
import { FileRecordUC } from '../uc/file-record.uc';
import { FilesStorageUC } from '../uc/files-storage.uc';
import { FileRecordUC, FilesStorageUC } from '../uc';
import { ScanResultParams } from './dto';

@ApiTags('file-security')
@Controller()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import { EntityId, FileRecordParentType } from '@shared/domain';
import { courseFactory, fileRecordFactory, setupEntities } from '@shared/testing';
import { Logger } from '@src/core/logger';
import { FilesStorageService } from '../service/files-storage.service';
import { FilesStorageUC } from '../uc/files-storage.uc';
import { FileRecordResponse } from './dto';
import { FilesStorageConsumer } from './files-storage.consumer';

describe('FilesStorageConsumer', () => {
let module: TestingModule;
let filesStorageService: DeepMocked<FilesStorageService>;
let filesStorageUC: DeepMocked<FilesStorageUC>;
let service: FilesStorageConsumer;
let orm: MikroORM;

Expand All @@ -35,10 +33,6 @@ describe('FilesStorageConsumer', () => {
provide: FilesStorageService,
useValue: createMock<FilesStorageService>(),
},
{
provide: FilesStorageUC,
useValue: createMock<FilesStorageUC>(),
},
{
provide: Logger,
useValue: createMock<Logger>(),
Expand All @@ -51,7 +45,6 @@ describe('FilesStorageConsumer', () => {
}).compile();

filesStorageService = module.get(FilesStorageService);
filesStorageUC = module.get(FilesStorageUC);
service = module.get(FilesStorageConsumer);
});

Expand All @@ -73,7 +66,7 @@ describe('FilesStorageConsumer', () => {
},
};
await service.copyFilesOfParent(payload);
expect(filesStorageUC.copyFilesOfParent).toBeCalledWith(payload.userId, payload.source, {
expect(filesStorageService.copyFilesOfParent).toBeCalledWith(payload.userId, payload.source, {
target: payload.target,
});
});
Expand All @@ -94,7 +87,7 @@ describe('FilesStorageConsumer', () => {
},
};
const responseData = [{ id: '1', sourceId: '2', name: 'test.txt' }];
filesStorageUC.copyFilesOfParent.mockResolvedValue([responseData, responseData.length]);
filesStorageService.copyFilesOfParent.mockResolvedValue([responseData, responseData.length]);
const response = await service.copyFilesOfParent(payload);
expect(response.message).toEqual(responseData);
});
Expand All @@ -116,7 +109,7 @@ describe('FilesStorageConsumer', () => {
schoolId: targetCourse.school.id,
},
};
filesStorageUC.copyFilesOfParent.mockResolvedValue([[], 0]);
filesStorageService.copyFilesOfParent.mockResolvedValue([[], 0]);
const response = await service.copyFilesOfParent(payload);
expect(response.message).toEqual([]);
});
Expand All @@ -132,9 +125,9 @@ describe('FilesStorageConsumer', () => {
parentType: FileRecordParentType.Course,
schoolId,
};
filesStorageService.getFilesOfParent.mockResolvedValue([[], 0]);
filesStorageService.getFileRecordsOfParent.mockResolvedValue([[], 0]);
await service.getFilesOfParent(payload);
expect(filesStorageService.getFilesOfParent).toBeCalledWith(payload);
expect(filesStorageService.getFileRecordsOfParent).toBeCalledWith(payload);
});

it('should return array instances of FileRecordResponse', async () => {
Expand All @@ -146,7 +139,7 @@ describe('FilesStorageConsumer', () => {

const fileRecords = fileRecordFactory.buildList(3, payload);

filesStorageService.getFilesOfParent.mockResolvedValue([fileRecords, fileRecords.length]);
filesStorageService.getFileRecordsOfParent.mockResolvedValue([fileRecords, fileRecords.length]);
const response = await service.getFilesOfParent(payload);
expect(response.message[0]).toBeInstanceOf(FileRecordResponse);
});
Expand All @@ -159,7 +152,7 @@ describe('FilesStorageConsumer', () => {
parentType: FileRecordParentType.Course,
schoolId,
};
filesStorageService.getFilesOfParent.mockResolvedValue([[], 0]);
filesStorageService.getFileRecordsOfParent.mockResolvedValue([[], 0]);
const response = await service.getFilesOfParent(payload);
expect(response).toStrictEqual({ message: [] });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ import { Injectable } from '@nestjs/common';
import { RpcMessage } from '@shared/infra/rabbitmq/rpc-message';
import { Logger } from '@src/core/logger';
import { FilesStorageEvents, FilesStorageExchange, ICopyFileDO, IFileDO } from '@src/shared/infra/rabbitmq';
import { FilesStorageMapper } from '../mapper/file-record.mapper';
import { FilesStorageMapper } from '../mapper';
import { FilesStorageService } from '../service/files-storage.service';
import { FilesStorageUC } from '../uc/files-storage.uc';
import { CopyFilesOfParentPayload, FileRecordParams } from './dto';

@Injectable()
export class FilesStorageConsumer {
constructor(
private readonly filesStorageService: FilesStorageService,
private readonly filesStorageUC: FilesStorageUC,
private logger: Logger,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
private readonly orm: MikroORM // don't remove it, we need it for @UseRequestContext
Expand All @@ -33,7 +31,8 @@ export class FilesStorageConsumer {
this.logger.debug({ action: 'copyFilesOfParent', payload });

const { userId, source, target } = payload;
const [response] = await this.filesStorageUC.copyFilesOfParent(userId, source, { target });
const [response] = await this.filesStorageService.copyFilesOfParent(userId, source, { target });

return { message: response };
}

Expand All @@ -46,7 +45,7 @@ export class FilesStorageConsumer {
public async getFilesOfParent(@RabbitPayload() payload: FileRecordParams): Promise<RpcMessage<IFileDO[]>> {
this.logger.debug({ action: 'getFilesOfParent', payload });

const [fileRecords, total] = await this.filesStorageService.getFilesOfParent(payload);
const [fileRecords, total] = await this.filesStorageService.getFileRecordsOfParent(payload);
const response = FilesStorageMapper.mapToFileRecordListResponse(fileRecords, total);

return { message: response.data };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import { ICurrentUser } from '@shared/domain';
import { Authenticate, CurrentUser } from '@src/modules/authentication/decorator/auth.decorator';
import { Request } from 'express';
import { FilesStorageMapper } from '../mapper/file-record.mapper';
import { FileRecordUC } from '../uc/file-record.uc';
import { FilesStorageUC } from '../uc/files-storage.uc';
import { FileRecordUC, FilesStorageUC } from '../uc';
import {
CopyFileListResponse,
CopyFileParams,
Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/modules/files-storage/controller/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from './file-security.controller';
export * from './files-storage.controller';
export * from './files-storage.consumer';
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Module } from '@nestjs/common';
import { CoreModule } from '@src/core';
import { LoggerModule } from '@src/core/logger';
import { FilesStorageConsumer } from './controller/files-storage.consumer';
import { FilesStorageConsumer } from './controller';
import { FilesStorageModule } from './files-storage.module';

@Module({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { HttpModule } from '@nestjs/axios';
import { Module } from '@nestjs/common';
import { CoreModule } from '@src/core';
import { LoggerModule } from '@src/core/logger';
import { AuthModule } from '@src/modules/authentication';
import { AuthorizationModule } from '@src/modules/authorization';
import { FileSecurityController } from './controller/file-security.controller';
import { FilesStorageController } from './controller/files-storage.controller';
import { FileSecurityController, FilesStorageController } from './controller';
import { FilesStorageModule } from './files-storage.module';
import { FileRecordUC, FilesStorageUC } from './uc';

@Module({
imports: [AuthorizationModule, FilesStorageModule, AuthModule, CoreModule, LoggerModule],
imports: [AuthorizationModule, FilesStorageModule, AuthModule, CoreModule, HttpModule],
controllers: [FilesStorageController, FileSecurityController],
// providers: [FilesStorageUC, FileRecordUC],
providers: [FilesStorageUC, FileRecordUC],
})
export class FilesStorageApiModule {}
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,19 @@ import { CoreModule } from '@src/core';
import { LoggerModule } from '@src/core/logger';
import { AuthModule } from '@src/modules/authentication';
import { AuthorizationModule } from '@src/modules/authorization';
import { FileSecurityController } from './controller/file-security.controller';
import { FilesStorageConsumer } from './controller/files-storage.consumer';
import { FilesStorageController } from './controller/files-storage.controller';
import { FilesStorageModule } from './files-storage.module';
import { FilesStorageApiModule } from './files-storage-api.module';

const imports = [
FilesStorageModule,
FilesStorageApiModule,
MongoMemoryDatabaseModule.forRoot(),
RabbitMQWrapperTestModule,
AuthorizationModule,
AuthModule,
CoreModule,
LoggerModule,
];
const controllers = [FilesStorageController, FileSecurityController];
const providers = [FilesStorageConsumer];
const controllers = [];
const providers = [];
@Module({
imports,
controllers,
Expand Down
12 changes: 1 addition & 11 deletions apps/server/src/modules/files-storage/files-storage.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { S3Client } from '@aws-sdk/client-s3';
import { Configuration } from '@hpi-schul-cloud/commons';
import { Dictionary, IPrimaryKey } from '@mikro-orm/core';
import { MikroOrmModule, MikroOrmModuleSyncOptions } from '@mikro-orm/nestjs';
import { HttpModule } from '@nestjs/axios';
import { Module, NotFoundException } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';
import { ALL_ENTITIES } from '@shared/domain';
Expand All @@ -11,23 +10,17 @@ import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq/rabbitmq.module';
import { FileRecordRepo } from '@shared/repo';
import { DB_PASSWORD, DB_URL, DB_USERNAME } from '@src/config';
import { LoggerModule } from '@src/core/logger';
import { AuthorizationModule } from '@src/modules/authorization';
import { S3ClientAdapter } from './client/s3-client.adapter';
import { config, s3Config } from './files-storage.config';
import { FilesStorageHelper } from './helper';
import { S3Config } from './interface/config';
import { FilesStorageService } from './service/files-storage.service';
import { FileRecordUC } from './uc/file-record.uc';
import { FilesStorageUC } from './uc/files-storage.uc';

const imports = [
AuthorizationModule, // After refactoring, move to FilesStorageApiModule AuthorizationModule,
LoggerModule,
ConfigModule.forRoot({
isGlobal: true,
load: [config],
}),
HttpModule,
AntivirusModule.forRoot({
enabled: Configuration.get('ENABLE_FILE_SECURITY_CHECK') as boolean,
filesServiceBaseUrl: Configuration.get('FILES_STORAGE__SERVICE_BASE_URL') as string,
Expand All @@ -36,10 +29,7 @@ const imports = [
}),
];
const providers = [
FilesStorageUC, // After refactoring, move to FilesStorageApiModule FilesStorageUC, FileRecordUC
FileRecordUC,
FilesStorageService,
FilesStorageHelper,
{
provide: 'S3_Client',
useFactory: (configProvider: S3Config) => {
Expand Down Expand Up @@ -88,6 +78,6 @@ const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = {
}),
],
providers,
exports: [FilesStorageService, FilesStorageUC, FileRecordUC], // After refactoring, remove FilesStorageUC, FileRecordUC
exports: [FilesStorageService],
})
export class FilesStorageModule {}
Loading

0 comments on commit cef88d9

Please sign in to comment.