Skip to content

Commit

Permalink
Merge branch 'main' into N21-1497-delete-groups
Browse files Browse the repository at this point in the history
  • Loading branch information
@MarvinOehlerkingCap
MarvinOehlerkingCap authored Jan 9, 2024
2 parents e564a24 + e6193d3 commit ed113b6
Show file tree
Hide file tree
Showing 15 changed files with 256 additions and 70 deletions.
8 changes: 5 additions & 3 deletions apps/server/src/modules/deletion/uc/deletion-request.uc.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ describe(DeletionRequestUc.name, () => {
courseGroupService.deleteUserDataFromCourseGroup.mockResolvedValueOnce(2);
courseService.deleteUserDataFromCourse.mockResolvedValueOnce(2);
filesService.markFilesOwnedByUserForDeletion.mockResolvedValueOnce(2);
filesService.removeUserPermissionsToAnyFiles.mockResolvedValueOnce(2);
filesService.removeUserPermissionsOrCreatorReferenceToAnyFiles.mockResolvedValueOnce(2);
lessonService.deleteUserDataFromLessons.mockResolvedValueOnce(2);
pseudonymService.deleteByUserId.mockResolvedValueOnce(2);
teamService.deleteUserDataFromTeams.mockResolvedValueOnce(2);
Expand Down Expand Up @@ -336,7 +336,9 @@ describe(DeletionRequestUc.name, () => {

await uc.executeDeletionRequests();

expect(filesService.removeUserPermissionsToAnyFiles).toHaveBeenCalledWith(deletionRequestToExecute.targetRefId);
expect(filesService.removeUserPermissionsOrCreatorReferenceToAnyFiles).toHaveBeenCalledWith(
deletionRequestToExecute.targetRefId
);
});

it('should call filesStorageClientAdapterService.removeCreatorIdFromFileRecords to remove cratorId to any files in fileRecords module', async () => {
Expand Down Expand Up @@ -482,7 +484,7 @@ describe(DeletionRequestUc.name, () => {
courseGroupService.deleteUserDataFromCourseGroup.mockResolvedValueOnce(2);
courseService.deleteUserDataFromCourse.mockResolvedValueOnce(2);
filesService.markFilesOwnedByUserForDeletion.mockResolvedValueOnce(2);
filesService.removeUserPermissionsToAnyFiles.mockResolvedValueOnce(2);
filesService.removeUserPermissionsOrCreatorReferenceToAnyFiles.mockResolvedValueOnce(2);
lessonService.deleteUserDataFromLessons.mockResolvedValueOnce(2);
pseudonymService.deleteByUserId.mockResolvedValueOnce(2);
teamService.deleteUserDataFromTeams.mockResolvedValueOnce(2);
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/modules/deletion/uc/deletion-request.uc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,14 @@ export class DeletionRequestUc {
this.logger.debug({ action: 'removeUsersFilesAndPermissions', deletionRequest });

const filesDeleted: number = await this.filesService.markFilesOwnedByUserForDeletion(deletionRequest.targetRefId);
const filePermissionsUpdated: number = await this.filesService.removeUserPermissionsToAnyFiles(
const filesUpdated: number = await this.filesService.removeUserPermissionsOrCreatorReferenceToAnyFiles(
deletionRequest.targetRefId
);
await this.logDeletion(
deletionRequest,
DomainModel.FILE,
DeletionOperationModel.UPDATE,
filesDeleted + filePermissionsUpdated,
filesDeleted + filesUpdated,
0
);
}
Expand Down
44 changes: 44 additions & 0 deletions apps/server/src/modules/files/entity/file.entity.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,50 @@ describe(FileEntity.name, () => {
});
});

describe('removCreatorId', () => {
describe('when called on a file that contains matching creatorId', () => {
const setup = () => {
const file = fileEntityFactory.build({
ownerId: mainUserId,
creatorId: mainUserId,
});

const expectedFile = copyFile(file);
expectedFile._creatorId = undefined;

return { file, expectedFile };
};
it('should properly remove this creatorId', () => {
const { file, expectedFile } = setup();

file.removeCreatorId(mainUserId);

expect(file).toEqual(expectedFile);
});
});

describe("when called on a file that doesn't have any permission with given refId", () => {
const setup = () => {
const file = fileEntityFactory.build({
ownerId: mainUserId,
creatorId: mainUserId,
});

const originalFile = copyFile(file);

const randomUserId = new ObjectId().toHexString();
return { file, originalFile, randomUserId };
};
it('should not modify the file in any way (including the other present permissions)', () => {
const { file, originalFile, randomUserId } = setup();

file.removeCreatorId(randomUserId);

expect(file).toEqual(originalFile);
});
});
});

describe('markForDeletion', () => {
describe('when called on some typical file', () => {
it('should properly mark the file for deletion', () => {
Expand Down
16 changes: 11 additions & 5 deletions apps/server/src/modules/files/entity/file.entity.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface FileEntityProps {
parentId?: EntityId;
ownerId: EntityId;
refOwnerModel: FileOwnerModel;
creatorId: EntityId;
creatorId?: EntityId;
permissions: FilePermissionEntity[];
lockId?: EntityId;
versionKey?: number;
Expand Down Expand Up @@ -95,12 +95,12 @@ export class FileEntity extends BaseEntityWithTimestamps {
@Enum({ nullable: false })
refOwnerModel: FileOwnerModel;

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

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

@Embedded(() => FilePermissionEntity, { array: true, nullable: false })
Expand Down Expand Up @@ -141,6 +141,12 @@ export class FileEntity extends BaseEntityWithTimestamps {
return this.deleted && this.deletedAt !== undefined && !Number.isNaN(this.deletedAt.getTime());
}

public removeCreatorId(creatorId: EntityId): void {
if (creatorId === this._creatorId?.toHexString()) {
this._creatorId = undefined;
}
}

constructor(props: FileEntityProps) {
super();

Expand Down
53 changes: 46 additions & 7 deletions apps/server/src/modules/files/repo/files.repo.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,19 @@ describe(FilesRepo.name, () => {
});
});

describe('findByPermissionRefId', () => {
describe('when searching for a files to which the user with given userId has access', () => {
describe('findByPermissionRefIdOrCreatorId', () => {
describe('when searching for a files to which the user with given userId has access or is creator', () => {
const setup = async () => {
const mainUserId = new ObjectId().toHexString();
const otherUserId = new ObjectId().toHexString();

// Test files created, owned and accessible only by the other user.
const otherUserFileWithMainUserCreator = fileEntityFactory.build({
ownerId: otherUserId,
creatorId: mainUserId,
permissions: [filePermissionEntityFactory.build({ refId: otherUserId })],
});

const otherUserFile = fileEntityFactory.build({
ownerId: otherUserId,
creatorId: otherUserId,
Expand Down Expand Up @@ -268,9 +274,35 @@ describe(FilesRepo.name, () => {
permissions: [filePermissionEntityFactory.build({ refId: mainUserId })],
});

await em.persistAndFlush([otherUserFile, mainUserSharedFile, otherUserSharedFile, mainUserFile]);
await em.persistAndFlush([
otherUserFileWithMainUserCreator,
mainUserSharedFile,
otherUserSharedFile,
mainUserFile,
otherUserFile,
]);
em.clear();

const expectedOtherUserFileWithMainUserCreatorProps = {
id: otherUserFileWithMainUserCreator.id,
createdAt: otherUserFileWithMainUserCreator.createdAt,
updatedAt: otherUserFileWithMainUserCreator.updatedAt,
deleted: false,
isDirectory: false,
name: otherUserFileWithMainUserCreator.name,
size: otherUserFileWithMainUserCreator.size,
type: otherUserFileWithMainUserCreator.type,
storageFileName: otherUserFileWithMainUserCreator.storageFileName,
bucket: otherUserFileWithMainUserCreator.bucket,
thumbnail: otherUserFileWithMainUserCreator.thumbnail,
thumbnailRequestToken: otherUserFileWithMainUserCreator.thumbnailRequestToken,
securityCheck: otherUserFileWithMainUserCreator.securityCheck,
shareTokens: [],
refOwnerModel: otherUserFileWithMainUserCreator.refOwnerModel,
permissions: otherUserFileWithMainUserCreator.permissions,
versionKey: 0,
};

const expectedMainUserSharedFileProps = {
id: mainUserSharedFile.id,
createdAt: mainUserSharedFile.createdAt,
Expand Down Expand Up @@ -336,6 +368,8 @@ describe(FilesRepo.name, () => {
mainUserSharedFile,
otherUserSharedFile,
mainUserFile,
otherUserFileWithMainUserCreator,
expectedOtherUserFileWithMainUserCreatorProps,
expectedMainUserSharedFileProps,
expectedOtherUserSharedFileProps,
expectedMainUserFileProps,
Expand All @@ -349,18 +383,21 @@ describe(FilesRepo.name, () => {
mainUserSharedFile,
otherUserSharedFile,
mainUserFile,
otherUserFileWithMainUserCreator,
expectedMainUserSharedFileProps,
expectedOtherUserSharedFileProps,
expectedMainUserFileProps,
expectedOtherUserFileWithMainUserCreatorProps,
} = await setup();

const results = await repo.findByPermissionRefId(mainUserId);
const results = await repo.findByPermissionRefIdOrCreatorId(mainUserId);

expect(results).toHaveLength(3);
expect(results).toHaveLength(4);

// Verify explicit fields.
expect(results).toEqual(
expect.arrayContaining([
expect.objectContaining(expectedOtherUserFileWithMainUserCreatorProps),
expect.objectContaining(expectedMainUserSharedFileProps),
expect.objectContaining(expectedOtherUserSharedFileProps),
expect.objectContaining(expectedMainUserFileProps),
Expand All @@ -370,6 +407,7 @@ describe(FilesRepo.name, () => {
// Verify storage provider id.
expect(results.map((result) => result.storageProvider?.id)).toEqual(
expect.arrayContaining([
otherUserFileWithMainUserCreator.storageProvider?.id,
mainUserSharedFile.storageProvider?.id,
otherUserSharedFile.storageProvider?.id,
mainUserFile.storageProvider?.id,
Expand All @@ -384,6 +422,7 @@ describe(FilesRepo.name, () => {
// Verify implicit creatorId field.
expect(results.map((result) => result.creatorId)).toEqual(
expect.arrayContaining([
otherUserFileWithMainUserCreator.creatorId,
mainUserSharedFile.creatorId,
otherUserSharedFile.creatorId,
mainUserFile.creatorId,
Expand All @@ -397,7 +436,7 @@ describe(FilesRepo.name, () => {
await em.persistAndFlush([fileEntityFactory.build(), fileEntityFactory.build(), fileEntityFactory.build()]);
em.clear();

const results = await repo.findByPermissionRefId(new ObjectId().toHexString());
const results = await repo.findByPermissionRefIdOrCreatorId(new ObjectId().toHexString());

expect(results).toHaveLength(0);
});
Expand All @@ -407,7 +446,7 @@ describe(FilesRepo.name, () => {
it('should return an empty array', async () => {
const testPermissionRefId = new ObjectId().toHexString();

const results = await repo.findByPermissionRefId(testPermissionRefId);
const results = await repo.findByPermissionRefIdOrCreatorId(testPermissionRefId);

expect(results).toHaveLength(0);
});
Expand Down
23 changes: 18 additions & 5 deletions apps/server/src/modules/files/repo/files.repo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,28 @@ export class FilesRepo extends BaseRepo<FileEntity> {
return files as FileEntity[];
}

public async findByPermissionRefId(permissionRefId: EntityId): Promise<FileEntity[]> {
public async findByPermissionRefIdOrCreatorId(userId: EntityId): Promise<FileEntity[]> {
const refId = new ObjectId(userId);

const pipeline = [
{
$match: {
permissions: {
$elemMatch: {
refId: new ObjectId(permissionRefId),
$and: [
{
$or: [
{
permissions: {
$elemMatch: {
refId,
},
},
},
{ creator: refId },
],
},
},
{ deleted: false },
{ deletedAt: undefined },
],
},
},
];
Expand Down
Loading

0 comments on commit ed113b6

Please sign in to comment.