Skip to content

Commit

Permalink
BC-5830 - remove creatorId from fileRecord entity (#4610)
Browse files Browse the repository at this point in the history
* remove creatorId from fileRecord entity

* add some test, adapter

* add tests to deletion-uc

* add test to filesStorageProducer

* add test to filesStorageClientService

* add test to filesStorageConsumer

* add more test

* fix testcases

* fix after review

* change in test after merge with main

* add export to deletionModule

* fixes in deletion and deletionApi modules
  • Loading branch information
WojciechGrancow authored Dec 18, 2023
1 parent 71cd058 commit 555c445
Show file tree
Hide file tree
Showing 20 changed files with 495 additions and 15 deletions.
3 changes: 2 additions & 1 deletion apps/server/src/infra/rabbitmq/exchange/files-storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
'REMOVE_CREATORID_OF_FILES' = 'remove-creatorId-of-files',
}

export enum ScanStatus {
Expand Down Expand Up @@ -51,7 +52,7 @@ export interface FileDO {
parentId: string;
securityCheckStatus: ScanStatus;
size: number;
creatorId: string;
creatorId?: string;
mimeType: string;
parentType: FileRecordParentType;
deletedSince?: Date;
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/modules/deletion/deletion-api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { RegistrationPinModule } from '@modules/registration-pin';
import { DeletionRequestsController } from './controller/deletion-requests.controller';
import { DeletionExecutionsController } from './controller/deletion-executions.controller';
import { DeletionRequestUc } from './uc';
import { FilesStorageClientModule } from '../files-storage-client';

@Module({
imports: [
Expand All @@ -33,6 +34,7 @@ import { DeletionRequestUc } from './uc';
AuthenticationModule,
RocketChatUserModule,
RegistrationPinModule,
FilesStorageClientModule,
RocketChatModule.forRoot({
uri: Configuration.get('ROCKET_CHAT_URI') as string,
adminId: Configuration.get('ROCKET_CHAT_ADMIN_ID') as string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const enum DeletionDomainModel {
COURSE = 'course',
DASHBOARD = 'dashboard',
FILE = 'file',
FILERECORDS = 'fileRecords',
LESSONS = 'lessons',
PSEUDONYMS = 'pseudonyms',
REGISTRATIONPIN = 'registrationPin',
Expand Down
22 changes: 21 additions & 1 deletion apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { RocketChatUser, RocketChatUserService, rocketChatUserFactory } from '@m
import { LegacyLogger } from '@src/core/logger';
import { ObjectId } from 'bson';
import { RegistrationPinService } from '@modules/registration-pin';
import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client';
import { DeletionDomainModel, DeletionStatusModel } from '../domain/types';
import { DeletionLogService } from '../services/deletion-log.service';
import { DeletionRequestService } from '../services';
Expand All @@ -40,6 +41,7 @@ describe(DeletionRequestUc.name, () => {
let rocketChatUserService: DeepMocked<RocketChatUserService>;
let rocketChatService: DeepMocked<RocketChatService>;
let registrationPinService: DeepMocked<RegistrationPinService>;
let filesStorageClientAdapterService: DeepMocked<FilesStorageClientAdapterService>;
let dashboardService: DeepMocked<DashboardService>;

beforeAll(async () => {
Expand Down Expand Up @@ -106,6 +108,10 @@ describe(DeletionRequestUc.name, () => {
provide: RegistrationPinService,
useValue: createMock<RegistrationPinService>(),
},
{
provide: FilesStorageClientAdapterService,
useValue: createMock<FilesStorageClientAdapterService>(),
},
{
provide: DashboardService,
useValue: createMock<DashboardService>(),
Expand All @@ -128,6 +134,7 @@ describe(DeletionRequestUc.name, () => {
rocketChatUserService = module.get(RocketChatUserService);
rocketChatService = module.get(RocketChatService);
registrationPinService = module.get(RegistrationPinService);
filesStorageClientAdapterService = module.get(FilesStorageClientAdapterService);
dashboardService = module.get(DashboardService);
await setupEntities();
});
Expand Down Expand Up @@ -205,6 +212,7 @@ describe(DeletionRequestUc.name, () => {
teamService.deleteUserDataFromTeams.mockResolvedValueOnce(2);
userService.deleteUser.mockResolvedValueOnce(1);
rocketChatUserService.deleteByUserId.mockResolvedValueOnce(1);
filesStorageClientAdapterService.removeCreatorIdFromFileRecords.mockResolvedValueOnce(5);
dashboardService.deleteDashboardByUserId.mockResolvedValueOnce(1);

return {
Expand Down Expand Up @@ -316,6 +324,18 @@ describe(DeletionRequestUc.name, () => {
expect(filesService.removeUserPermissionsToAnyFiles).toHaveBeenCalledWith(deletionRequestToExecute.targetRefId);
});

it('should call filesStorageClientAdapterService.removeCreatorIdFromFileRecords to remove cratorId to any files in fileRecords module', async () => {
const { deletionRequestToExecute } = setup();

deletionRequestService.findAllItemsToExecute.mockResolvedValueOnce([deletionRequestToExecute]);

await uc.executeDeletionRequests();

expect(filesStorageClientAdapterService.removeCreatorIdFromFileRecords).toHaveBeenCalledWith(
deletionRequestToExecute.targetRefId
);
});

it('should call lessonService.deleteUserDataFromLessons to delete users data in lesson module', async () => {
const { deletionRequestToExecute } = setup();

Expand Down Expand Up @@ -405,7 +425,7 @@ describe(DeletionRequestUc.name, () => {

await uc.executeDeletionRequests();

expect(deletionLogService.createDeletionLog).toHaveBeenCalledTimes(11);
expect(deletionLogService.createDeletionLog).toHaveBeenCalledTimes(12);
});
});

Expand Down
23 changes: 21 additions & 2 deletions apps/server/src/modules/deletion/uc/deletion-request.uc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ import { UserService } from '@modules/user';
import { Injectable } from '@nestjs/common';
import { EntityId } from '@shared/domain/types';
import { LegacyLogger } from '@src/core/logger';
import { DeletionLogStatisticBuilder, DeletionTargetRefBuilder, DeletionRequestLogResponseBuilder } from '../builder';
import { FilesStorageClientAdapterService } from '@modules/files-storage-client';
import { DeletionLogStatisticBuilder, DeletionRequestLogResponseBuilder, DeletionTargetRefBuilder } from '../builder';
import { DeletionRequestBodyProps, DeletionRequestLogResponse, DeletionRequestResponse } from '../controller/dto';
import { DeletionLogStatistic } from './interface/interfaces';
import { DeletionRequest, DeletionLog } from '../domain';
import { DeletionDomainModel, DeletionOperationModel, DeletionStatusModel } from '../domain/types';
import { DeletionRequestService, DeletionLogService } from '../services';
import { DeletionRequestBodyProps, DeletionRequestLogResponse, DeletionRequestResponse } from '../controller/dto';

@Injectable()
export class DeletionRequestUc {
Expand All @@ -37,6 +38,7 @@ export class DeletionRequestUc {
private readonly rocketChatService: RocketChatService,
private readonly logger: LegacyLogger,
private readonly registrationPinService: RegistrationPinService,
private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService,
private readonly dashboardService: DashboardService
) {
this.logger.setContext(DeletionRequestUc.name);
Expand Down Expand Up @@ -100,6 +102,7 @@ export class DeletionRequestUc {
this.removeUserFromCourseGroup(deletionRequest),
this.removeUserFromCourse(deletionRequest),
this.removeUsersFilesAndPermissions(deletionRequest),
this.removeUsersDataFromFileRecords(deletionRequest),
this.removeUserFromLessons(deletionRequest),
this.removeUsersPseudonyms(deletionRequest),
this.removeUserFromTeams(deletionRequest),
Expand Down Expand Up @@ -229,6 +232,22 @@ export class DeletionRequestUc {
);
}

private async removeUsersDataFromFileRecords(deletionRequest: DeletionRequest) {
this.logger.debug({ action: 'removeUsersDataFromFileRecords', deletionRequest });

const fileRecordsUpdated = await this.filesStorageClientAdapterService.removeCreatorIdFromFileRecords(
deletionRequest.targetRefId
);

await this.logDeletion(
deletionRequest,
DeletionDomainModel.FILERECORDS,
DeletionOperationModel.UPDATE,
fileRecordsUpdated,
0
);
}

private async removeUserFromLessons(deletionRequest: DeletionRequest) {
this.logger.debug({ action: 'removeUserFromLessons', deletionRequest });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,38 @@ describe('FilesStorageClientAdapterService', () => {
});
});
});

describe('removeCreatorIdFromFileRecords', () => {
describe('when creatorId is deleted successfully', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

return { creatorId };
};

it('Should call client.removeCreatorIdFromFileRecords', async () => {
const { creatorId } = setup();

await service.removeCreatorIdFromFileRecords(creatorId);

expect(client.removeCreatorIdFromFileRecords).toHaveBeenCalledWith(creatorId);
});
});

describe('when error is thrown', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

client.removeCreatorIdFromFileRecords.mockRejectedValue(new Error());

return { creatorId };
};

it('Should call error mapper if throw an error.', async () => {
const { creatorId } = setup();

await expect(service.removeCreatorIdFromFileRecords(creatorId)).rejects.toThrowError();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,10 @@ export class FilesStorageClientAdapterService {

return fileInfos;
}

async removeCreatorIdFromFileRecords(creatorId: EntityId): Promise<number> {
const response = await this.fileStorageMQProducer.removeCreatorIdFromFileRecords(creatorId);

return response.length;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,59 @@ describe('FilesStorageProducer', () => {
});
});
});

describe('removeCreatorIdFromFileRecords', () => {
describe('when valid parameter passed and amqpConnection return with error in response', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

amqpConnection.request.mockResolvedValue({ error: new Error() });
const spy = jest.spyOn(ErrorMapper, 'mapRpcErrorResponseToDomainError');

return { creatorId, spy };
};

it('should call error mapper and throw with error', async () => {
const { creatorId, spy } = setup();

await expect(service.removeCreatorIdFromFileRecords(creatorId)).rejects.toThrowError();
expect(spy).toBeCalled();
});
});

describe('when valid parameter passed and amqpConnection return with message', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

const message = [];
amqpConnection.request.mockResolvedValue({ message });

const expectedParams = {
exchange: FilesStorageExchange,
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
routingKey: FilesStorageEvents.REMOVE_CREATORID_OF_FILES,
payload: creatorId,
timeout,
};

return { creatorId, message, expectedParams };
};

it('should call the ampqConnection.', async () => {
const { creatorId, expectedParams } = setup();

await service.removeCreatorIdFromFileRecords(creatorId);

expect(amqpConnection.request).toHaveBeenCalledWith(expectedParams);
});

it('should return the response message.', async () => {
const { creatorId, message } = setup();

const res = await service.removeCreatorIdFromFileRecords(creatorId);

expect(res).toEqual(message);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,14 @@ export class FilesStorageProducer extends RpcMessageProducer {

return response;
}

async removeCreatorIdFromFileRecords(payload: EntityId): Promise<FileDO[]> {
this.logger.debug({ action: 'removeCreatorIdFromFileRecords:started', payload });
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
const response = await this.request<FileDO[]>(FilesStorageEvents.REMOVE_CREATORID_OF_FILES, payload);

this.logger.debug({ action: 'removeCreatorIdFromFileRecords:finished', payload });

return response;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class FileRecordResponse {
size: number;

@ApiProperty()
creatorId: string;
creatorId?: string;

@ApiProperty()
mimeType: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,51 @@ describe('FilesStorageConsumer', () => {
});
});
});

describe('removeCreatorIdFromFileRecords()', () => {
describe('WHEN valid file exists', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

const fileRecords = fileRecordFactory.buildList(3, { creatorId });
filesStorageService.getFileRecordsByCreatorId.mockResolvedValue([fileRecords, fileRecords.length]);

return { creatorId, fileRecords };
};

it('should call filesStorageService.getFileRecordsByCreatorId', async () => {
const { creatorId } = setup();

await service.removeCreatorIdFromFileRecords(creatorId);

expect(filesStorageService.getFileRecordsByCreatorId).toBeCalledWith(creatorId);
});

it('should call filesStorageService.removeCreatorIdFromFileRecords with params', async () => {
const { creatorId, fileRecords } = setup();

await service.removeCreatorIdFromFileRecords(creatorId);

expect(filesStorageService.removeCreatorIdFromFileRecords).toBeCalledWith(fileRecords);
});
});

describe('WHEN no file exists', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();

filesStorageService.getFileRecordsByCreatorId.mockResolvedValue([[], 0]);

return { creatorId };
};

it('should return RpcMessage with empty array', async () => {
const { creatorId } = setup();

const response = await service.removeCreatorIdFromFileRecords(creatorId);

expect(response).toStrictEqual({ message: [] });
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,22 @@ export class FilesStorageConsumer {

return { message: response.data };
}

@RabbitRPC({
exchange: FilesStorageExchange,
routingKey: FilesStorageEvents.REMOVE_CREATORID_OF_FILES,
queue: FilesStorageEvents.REMOVE_CREATORID_OF_FILES,
})
@UseRequestContext()
public async removeCreatorIdFromFileRecords(@RabbitPayload() payload: EntityId): Promise<RpcMessage<FileDO[]>> {
this.logger.debug({ action: 'removeCreatorIdFromFileRecords', payload });

const [fileRecords, total] = await this.filesStorageService.getFileRecordsByCreatorId(payload);
let updatedFileRecords = await this.filesStorageService.removeCreatorIdFromFileRecords(fileRecords);
updatedFileRecords = updatedFileRecords ?? [];

const response = FilesStorageMapper.mapToFileRecordListResponse(updatedFileRecords, total);

return { message: response.data };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -833,4 +833,23 @@ describe('FileRecord Entity', () => {
});
});
});

describe('removeCreatorId is called', () => {
describe('WHEN creatorId exists', () => {
const setup = () => {
const creatorId = new ObjectId().toHexString();
const fileRecord = fileRecordFactory.build({ creatorId });

return { fileRecord, creatorId };
};

it('should set it to undefined', () => {
const { fileRecord } = setup();

const result = fileRecord.removeCreatorId();

expect(result).toBe(undefined);
});
});
});
});
Loading

0 comments on commit 555c445

Please sign in to comment.