From 0c5fdf42013f6f76dce8393d85bf5ebe01b6f9e0 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Tue, 16 Nov 2021 19:05:28 +0100 Subject: [PATCH] refactor(group): lazy-load relations Signed-off-by: David Mehren --- src/api/private/notes/notes.controller.ts | 8 +- src/api/public/notes/notes.controller.ts | 17 ++-- src/groups/group.entity.ts | 4 +- src/permissions/permissions.service.spec.ts | 96 ++++++++++----------- src/permissions/permissions.service.ts | 14 +-- 5 files changed, 64 insertions(+), 75 deletions(-) diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index 527f32bbe..5a43b713f 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/src/api/private/notes/notes.controller.ts @@ -14,14 +14,10 @@ import { Param, Post, UseGuards, - UseInterceptors, + UseInterceptors } from '@nestjs/common'; -import { - AlreadyInDBError, - ForbiddenIdError, - NotInDBError, -} from '../../../errors/errors'; +import { AlreadyInDBError, ForbiddenIdError, NotInDBError } from '../../../errors/errors'; import { HistoryService } from '../../../history/history.service'; import { SessionGuard } from '../../../identity/session.guard'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index d1a1b4649..99980c9bc 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -16,7 +16,7 @@ import { Post, Put, UseGuards, - UseInterceptors, + UseInterceptors } from '@nestjs/common'; import { ApiCreatedResponse, @@ -26,24 +26,17 @@ import { ApiProduces, ApiSecurity, ApiTags, - ApiUnauthorizedResponse, + ApiUnauthorizedResponse } from '@nestjs/swagger'; import { TokenAuthGuard } from '../../../auth/token.strategy'; -import { - AlreadyInDBError, - ForbiddenIdError, - NotInDBError, -} from '../../../errors/errors'; +import { AlreadyInDBError, ForbiddenIdError, NotInDBError } from '../../../errors/errors'; import { HistoryService } from '../../../history/history.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { MediaUploadDto } from '../../../media/media-upload.dto'; import { MediaService } from '../../../media/media.service'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; -import { - NotePermissionsDto, - NotePermissionsUpdateDto, -} from '../../../notes/note-permissions.dto'; +import { NotePermissionsDto, NotePermissionsUpdateDto } from '../../../notes/note-permissions.dto'; import { NoteDto } from '../../../notes/note.dto'; import { Note } from '../../../notes/note.entity'; import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto'; @@ -57,7 +50,7 @@ import { User } from '../../../users/user.entity'; import { forbiddenDescription, successfullyDeletedDescription, - unauthorizedDescription, + unauthorizedDescription } from '../../utils/descriptions'; import { FullApi } from '../../utils/fullapi-decorator'; import { GetNoteInterceptor } from '../../utils/get-note.interceptor'; diff --git a/src/groups/group.entity.ts b/src/groups/group.entity.ts index f274f90c5..d4abf660a 100644 --- a/src/groups/group.entity.ts +++ b/src/groups/group.entity.ts @@ -38,7 +38,7 @@ export class Group { eager: true, }) @JoinTable() - members: User[]; + members: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -52,7 +52,7 @@ export class Group { newGroup.name = name; newGroup.displayName = displayName; newGroup.special = special; // this attribute should only be true for the two special groups - newGroup.members = []; + newGroup.members = Promise.resolve([]); return newGroup; } } diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 981be252e..3bf1833d7 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -170,79 +170,79 @@ describe('PermissionsService', () => { const notes = createNoteUserPermissionNotes(); describe('mayRead works with', () => { - it('Owner', () => { + it('Owner', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayRead(user1, notes[0])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[7])).toBeFalsy(); + expect(await permissionsService.mayRead(user1, notes[0])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[7])).toBeFalsy(); }); - it('userPermission read', () => { + it('userPermission read', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayRead(user1, notes[1])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[2])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[3])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[1])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[2])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[3])).toBeTruthy(); }); - it('userPermission write', () => { + it('userPermission write', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayRead(user1, notes[4])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[5])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[6])).toBeTruthy(); - expect(permissionsService.mayRead(user1, notes[7])).toBeFalsy(); + expect(await permissionsService.mayRead(user1, notes[4])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[5])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[6])).toBeTruthy(); + expect(await permissionsService.mayRead(user1, notes[7])).toBeFalsy(); }); describe('guest permission', () => { - it('CREATE_ALIAS', () => { + it('CREATE_ALIAS', async () => { permissionsService.guestPermission = GuestPermission.CREATE_ALIAS; - expect(permissionsService.mayRead(null, notes[8])).toBeTruthy(); + expect(await permissionsService.mayRead(null, notes[8])).toBeTruthy(); }); - it('CREATE', () => { + it('CREATE', async () => { permissionsService.guestPermission = GuestPermission.CREATE; - expect(permissionsService.mayRead(null, notes[8])).toBeTruthy(); + expect(await permissionsService.mayRead(null, notes[8])).toBeTruthy(); }); - it('WRITE', () => { + it('WRITE', async () => { permissionsService.guestPermission = GuestPermission.WRITE; - expect(permissionsService.mayRead(null, notes[8])).toBeTruthy(); + expect(await permissionsService.mayRead(null, notes[8])).toBeTruthy(); }); - it('READ', () => { + it('READ', async () => { permissionsService.guestPermission = GuestPermission.READ; - expect(permissionsService.mayRead(null, notes[8])).toBeTruthy(); + expect(await permissionsService.mayRead(null, notes[8])).toBeTruthy(); }); }); }); describe('mayWrite works with', () => { - it('Owner', () => { + it('Owner', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayWrite(user1, notes[0])).toBeTruthy(); - expect(permissionsService.mayWrite(user1, notes[7])).toBeFalsy(); + expect(await permissionsService.mayWrite(user1, notes[0])).toBeTruthy(); + expect(await permissionsService.mayWrite(user1, notes[7])).toBeFalsy(); }); - it('userPermission read', () => { + it('userPermission read', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayWrite(user1, notes[1])).toBeFalsy(); - expect(permissionsService.mayWrite(user1, notes[2])).toBeFalsy(); - expect(permissionsService.mayWrite(user1, notes[3])).toBeFalsy(); + expect(await permissionsService.mayWrite(user1, notes[1])).toBeFalsy(); + expect(await permissionsService.mayWrite(user1, notes[2])).toBeFalsy(); + expect(await permissionsService.mayWrite(user1, notes[3])).toBeFalsy(); }); - it('userPermission write', () => { + it('userPermission write', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.mayWrite(user1, notes[4])).toBeTruthy(); - expect(permissionsService.mayWrite(user1, notes[5])).toBeTruthy(); - expect(permissionsService.mayWrite(user1, notes[6])).toBeTruthy(); - expect(permissionsService.mayWrite(user1, notes[7])).toBeFalsy(); + expect(await permissionsService.mayWrite(user1, notes[4])).toBeTruthy(); + expect(await permissionsService.mayWrite(user1, notes[5])).toBeTruthy(); + expect(await permissionsService.mayWrite(user1, notes[6])).toBeTruthy(); + expect(await permissionsService.mayWrite(user1, notes[7])).toBeFalsy(); }); describe('guest permission', () => { - it('CREATE_ALIAS', () => { + it('CREATE_ALIAS', async () => { permissionsService.guestPermission = GuestPermission.CREATE_ALIAS; - expect(permissionsService.mayWrite(null, notes[9])).toBeTruthy(); + expect(await permissionsService.mayWrite(null, notes[9])).toBeTruthy(); }); - it('CREATE', () => { + it('CREATE', async () => { permissionsService.guestPermission = GuestPermission.CREATE; - expect(permissionsService.mayWrite(null, notes[9])).toBeTruthy(); + expect(await permissionsService.mayWrite(null, notes[9])).toBeTruthy(); }); - it('WRITE', () => { + it('WRITE', async () => { permissionsService.guestPermission = GuestPermission.WRITE; - expect(permissionsService.mayWrite(null, notes[9])).toBeTruthy(); + expect(await permissionsService.mayWrite(null, notes[9])).toBeTruthy(); }); - it('READ', () => { + it('READ', async () => { permissionsService.guestPermission = GuestPermission.READ; - expect(permissionsService.mayWrite(null, notes[9])).toBeFalsy(); + expect(await permissionsService.mayWrite(null, notes[9])).toBeFalsy(); }); }); }); @@ -277,11 +277,11 @@ describe('PermissionsService', () => { result[SpecialGroup.LOGGED_IN] = loggedIn; const user1group = Group.create('user1group', 'user1group', false) as Group; - user1group.members = [user1]; + user1group.members = Promise.resolve([user1]); result['user1group'] = user1group; const user2group = Group.create('user2group', 'user2group', false) as Group; - user2group.members = [user2]; + user2group.members = Promise.resolve([user2]); result['user2group'] = user2group; const user1and2group = Group.create( @@ -289,7 +289,7 @@ describe('PermissionsService', () => { 'user1and2group', false, ) as Group; - user1and2group.members = [user1, user2]; + user1and2group.members = Promise.resolve([user1, user2]); result['user1and2group'] = user1and2group; const user2and1group = Group.create( @@ -297,7 +297,7 @@ describe('PermissionsService', () => { 'user2and1group', false, ) as Group; - user2and1group.members = [user2, user1]; + user2and1group.members = Promise.resolve([user2, user1]); result['user2and1group'] = user2and1group; return result; @@ -506,15 +506,15 @@ describe('PermissionsService', () => { for (const perm of permission.permissions) { permissionString += ` ${perm.group.name}:${String(perm.canEdit)}`; } - it(`mayWrite - test #${i}:${permissionString}`, () => { + it(`mayWrite - test #${i}:${permissionString}`, async () => { permissionsService.guestPermission = guestPermission; - expect(permissionsService.mayWrite(user1, note)).toEqual( + expect(await permissionsService.mayWrite(user1, note)).toEqual( permission.allowsWrite, ); }); - it(`mayRead - test #${i}:${permissionString}`, () => { + it(`mayRead - test #${i}:${permissionString}`, async () => { permissionsService.guestPermission = guestPermission; - expect(permissionsService.mayRead(user1, note)).toEqual( + expect(await permissionsService.mayRead(user1, note)).toEqual( permission.allowsRead, ); }); diff --git a/src/permissions/permissions.service.ts b/src/permissions/permissions.service.ts index c7ef792b3..5b059ac93 100644 --- a/src/permissions/permissions.service.ts +++ b/src/permissions/permissions.service.ts @@ -21,24 +21,24 @@ export enum GuestPermission { @Injectable() export class PermissionsService { public guestPermission: GuestPermission; // TODO change to configOption - mayRead(user: User | null, note: Note): boolean { + async mayRead(user: User | null, note: Note): Promise { if (this.isOwner(user, note)) return true; if (this.hasPermissionUser(user, note, false)) return true; // noinspection RedundantIfStatementJS - if (this.hasPermissionGroup(user, note, false)) return true; + if (await this.hasPermissionGroup(user, note, false)) return true; return false; } - mayWrite(user: User | null, note: Note): boolean { + async mayWrite(user: User | null, note: Note): Promise { if (this.isOwner(user, note)) return true; if (this.hasPermissionUser(user, note, true)) return true; // noinspection RedundantIfStatementJS - if (this.hasPermissionGroup(user, note, true)) return true; + if (await this.hasPermissionGroup(user, note, true)) return true; return false; } @@ -83,11 +83,11 @@ export class PermissionsService { return false; } - private hasPermissionGroup( + private async hasPermissionGroup( user: User | null, note: Note, wantEdit: boolean, - ): boolean { + ): Promise { // TODO: Get real config value let guestsAllowed = false; switch (this.guestPermission) { @@ -116,7 +116,7 @@ export class PermissionsService { } else { // Handle normal groups if (user) { - for (const member of groupPermission.group.members) { + for (const member of await groupPermission.group.members) { if (member.id === user.id) return true; } }