Skip to content

Commit

Permalink
remove creatorId from fileRecord entity
Browse files Browse the repository at this point in the history
  • Loading branch information
WojciechGrancow committed Dec 4, 2023
1 parent 399244f commit ffb4603
Show file tree
Hide file tree
Showing 9 changed files with 248 additions and 8 deletions.
2 changes: 1 addition & 1 deletion apps/server/src/infra/rabbitmq/exchange/files-storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface FileDO {
parentId: string;
securityCheckStatus: ScanStatus;
size: number;
creatorId: string;
creatorId?: string;
mimeType: string;
parentType: FileRecordParentType;
deletedSince?: Date;
Expand Down
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 @@ -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);
});
});
});
});
22 changes: 16 additions & 6 deletions apps/server/src/modules/files-storage/entity/filerecord.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export interface FileRecordProperties {
mimeType: string;
parentType: FileRecordParentType;
parentId: EntityId;
creatorId: EntityId;
creatorId?: EntityId;
schoolId: EntityId;
deletedSince?: Date;
isCopyFrom?: EntityId;
Expand Down Expand Up @@ -127,11 +127,15 @@ export class FileRecord extends BaseEntityWithTimestamps {
return this._parentId.toHexString();
}

@Property({ fieldName: 'creator' })
_creatorId: ObjectId;
@Property({ fieldName: 'creator', nullable: true })
_creatorId?: ObjectId;

get creatorId(): EntityId {
return this._creatorId.toHexString();
get creatorId(): EntityId | undefined {
return this._creatorId?.toHexString();
}

set creatorId(userId: EntityId | undefined) {
this._creatorId = userId !== undefined ? new ObjectId(userId) : undefined;
}

@Property({ fieldName: 'school' })
Expand All @@ -157,7 +161,9 @@ export class FileRecord extends BaseEntityWithTimestamps {
this.mimeType = props.mimeType;
this.parentType = props.parentType;
this._parentId = new ObjectId(props.parentId);
this._creatorId = new ObjectId(props.creatorId);
if (props.creatorId !== undefined) {
this._creatorId = new ObjectId(props.creatorId);
}
this._schoolId = new ObjectId(props.schoolId);
if (props.isCopyFrom) {
this._isCopyFrom = new ObjectId(props.isCopyFrom);
Expand Down Expand Up @@ -300,4 +306,8 @@ export class FileRecord extends BaseEntityWithTimestamps {

return filenameObj.name;
}

public removeCreatorId(): void {
this.creatorId = undefined;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,10 @@ export class FileRecordScope extends Scope<FileRecord> {

return this;
}

byCreatorId(creatorId: EntityId): FileRecordScope {
this.addQuery({ _creatorId: new ObjectId(creatorId) });

return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,32 @@ describe('FileRecordRepo', () => {
await expect(repo.findBySecurityCheckRequestToken(token)).rejects.toThrow();
});
});

describe('findByCreatorId', () => {
const setup = () => {
const creator1 = new ObjectId().toHexString();
const creator2 = new ObjectId().toHexString();
const fileRecords1 = fileRecordFactory.buildList(4, {
creatorId: creator1,
});
const fileRecords2 = fileRecordFactory.buildList(3, {
creatorId: creator2,
});

return { fileRecords1, fileRecords2, creator1 };
};

it('should only find searched creator', async () => {
const { fileRecords1, fileRecords2, creator1 } = setup();

await em.persistAndFlush([...fileRecords1, ...fileRecords2]);
em.clear();

const [results, count] = await repo.findByCreatorId(creator1);

expect(count).toEqual(4);
expect(results).toHaveLength(4);
expect(results.map((o) => o.creatorId)).toEqual([creator1, creator1, creator1, creator1]);
});
});
});
7 changes: 7 additions & 0 deletions apps/server/src/modules/files-storage/repo/filerecord.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ export class FileRecordRepo extends BaseRepo<FileRecord> {
return fileRecord;
}

async findByCreatorId(creatorId: EntityId): Promise<Counted<FileRecord[]>> {
const scope = new FileRecordScope().byCreatorId(creatorId);
const result = await this.findAndCount(scope);

return result;
}

private async findAndCount(
scope: FileRecordScope,
options?: IFindOptions<FileRecord>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
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 { AntivirusService } from '@infra/antivirus';
import { S3ClientAdapter } from '@infra/s3-client';
import { fileRecordFactory, setupEntities } from '@shared/testing';
import { LegacyLogger } from '@src/core/logger';
import { FileRecordParams } from '../controller/dto';
import { FileRecord, FileRecordParentType } from '../entity';
import { FILES_STORAGE_S3_CONNECTION } from '../files-storage.config';
import { FileRecordRepo } from '../repo';
import { FilesStorageService } from './files-storage.service';

const buildFileRecordsWithParams = () => {
const parentId = new ObjectId().toHexString();
const parentSchoolId = new ObjectId().toHexString();
const creatorId = new ObjectId().toHexString();

const fileRecords = [
fileRecordFactory.buildWithId({ parentId, schoolId: parentSchoolId, name: 'text.txt', creatorId }),
fileRecordFactory.buildWithId({ parentId, schoolId: parentSchoolId, name: 'text-two.txt', creatorId }),
fileRecordFactory.buildWithId({ parentId, schoolId: parentSchoolId, name: 'text-tree.txt', creatorId }),
];

const params: FileRecordParams = {
schoolId: parentSchoolId,
parentId,
parentType: FileRecordParentType.User,
};

return { params, fileRecords, parentId, creatorId };
};

describe('FilesStorageService delete methods', () => {
let module: TestingModule;
let service: FilesStorageService;
let fileRecordRepo: DeepMocked<FileRecordRepo>;
let storageClient: DeepMocked<S3ClientAdapter>;

Check failure on line 39 in apps/server/src/modules/files-storage/service/files-storage-remove-creator.service.spec.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'storageClient' is assigned a value but never used

beforeAll(async () => {
await setupEntities([FileRecord]);

module = await Test.createTestingModule({
providers: [
FilesStorageService,
{
provide: FILES_STORAGE_S3_CONNECTION,
useValue: createMock<S3ClientAdapter>(),
},
{
provide: FileRecordRepo,
useValue: createMock<FileRecordRepo>(),
},
{
provide: LegacyLogger,
useValue: createMock<LegacyLogger>(),
},
{
provide: AntivirusService,
useValue: createMock<AntivirusService>(),
},
{
provide: ConfigService,
useValue: createMock<ConfigService>(),
},
],
}).compile();

service = module.get(FilesStorageService);
storageClient = module.get(FILES_STORAGE_S3_CONNECTION);
fileRecordRepo = module.get(FileRecordRepo);
});

beforeEach(() => {
jest.resetAllMocks();
});

afterAll(async () => {
await module.close();
});

it('service should be defined', () => {
expect(service).toBeDefined();
});

describe('removeCreatorIdFromFileRecord is called', () => {
describe('WHEN valid files does not exist', () => {
const setup = () => {
const userId = new ObjectId().toHexString();
fileRecordRepo.findByCreatorId.mockResolvedValueOnce([[], 0]);

return { userId };
};

it('should not modify any filescall repo save with undefined creatorId', async () => {
const { userId } = setup();

const result = await service.removeCreatorIdFromFileRecord(userId);

expect(result).toEqual(0);

expect(fileRecordRepo.findByCreatorId).toBeCalledWith(userId);
expect(fileRecordRepo.save).not.toBeCalled();
});
});

describe('WHEN valid files exists', () => {
const setup = () => {
const { fileRecords, creatorId } = buildFileRecordsWithParams();

fileRecordRepo.findByCreatorId.mockResolvedValueOnce([fileRecords, fileRecords.length]);

return { fileRecords, creatorId };
};

it('should call repo save with undefined creatorId', async () => {
const { fileRecords, creatorId } = setup();

await service.removeCreatorIdFromFileRecord(creatorId);

expect(fileRecordRepo.save).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({ ...fileRecords[0], _creatorId: undefined }),
expect.objectContaining({ ...fileRecords[1], _creatorId: undefined }),
expect.objectContaining({ ...fileRecords[2], _creatorId: undefined }),
])
);
});

it('should getnumber of updated fileRecords', async () => {
const { creatorId } = setup();

const result = await service.removeCreatorIdFromFileRecord(creatorId);

expect(result).toEqual(3);
});
});

describe('WHEN repository throw an error', () => {
const setup = () => {
const { fileRecords } = buildFileRecordsWithParams();

fileRecordRepo.save.mockRejectedValueOnce(new Error('bla'));

return { fileRecords };
};

it('should pass the error', async () => {
const { fileRecords } = setup();

await expect(service.delete(fileRecords)).rejects.toThrow(new Error('bla'));
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,20 @@ export class FilesStorageService {
}
}

public async removeCreatorIdFromFileRecord(userId: EntityId): Promise<number> {
const [fileRecords] = await this.fileRecordRepo.findByCreatorId(userId);

if (fileRecords.length === 0) {
return 0;
}

fileRecords.forEach((entity) => entity.removeCreatorId());

await this.fileRecordRepo.save(fileRecords);

return fileRecords.length;
}

// restore
private async restoreFilesInFileStorageClient(fileRecords: FileRecord[]) {
const paths = getPaths(fileRecords);
Expand Down

0 comments on commit ffb4603

Please sign in to comment.