From 552d06f18239fe39984a2b410367d9339fd5a84d Mon Sep 17 00:00:00 2001 From: David Mehren Date: Tue, 16 Nov 2021 18:39:52 +0100 Subject: [PATCH 01/14] refactor(auth-token): lazy-load relations Signed-off-by: David Mehren --- src/auth/auth-token.entity.ts | 4 ++-- src/auth/auth.service.spec.ts | 16 ++++++++-------- src/auth/auth.service.ts | 6 +++++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/auth/auth-token.entity.ts b/src/auth/auth-token.entity.ts index 80d9da025..668956813 100644 --- a/src/auth/auth-token.entity.ts +++ b/src/auth/auth-token.entity.ts @@ -24,7 +24,7 @@ export class AuthToken { @ManyToOne((_) => User, (user) => user.authTokens, { onDelete: 'CASCADE', // This deletes the AuthToken, when the associated User is deleted }) - user: User; + user: Promise; @Column() label: string; @@ -53,7 +53,7 @@ export class AuthToken { ): Omit { const newToken = new AuthToken(); newToken.keyId = keyId; - newToken.user = user; + newToken.user = Promise.resolve(user); newToken.label = label; newToken.accessTokenHash = accessToken; newToken.validUntil = validUntil; diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 21a39d752..813017000 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -89,7 +89,7 @@ describe('AuthService', () => { const accessTokenHash = await hashPassword(token); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, - user: user, + user: Promise.resolve(user), accessTokenHash: accessTokenHash, }); const authTokenFromCall = await service.getAuthTokenAndValidate( @@ -98,7 +98,7 @@ describe('AuthService', () => { ); expect(authTokenFromCall).toEqual({ ...authToken, - user: user, + user: Promise.resolve(user), accessTokenHash: accessTokenHash, }); }); @@ -112,7 +112,7 @@ describe('AuthService', () => { it('AuthToken has wrong hash', async () => { jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, - user: user, + user: Promise.resolve(user), accessTokenHash: 'the wrong hash', }); await expect( @@ -123,7 +123,7 @@ describe('AuthService', () => { const accessTokenHash = await hashPassword(token); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, - user: user, + user: Promise.resolve(user), accessTokenHash: accessTokenHash, validUntil: new Date(1549312452000), }); @@ -138,7 +138,7 @@ describe('AuthService', () => { it('works', async () => { jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, - user: user, + user: Promise.resolve(user), lastUsed: new Date(1549312452000), }); jest @@ -170,7 +170,7 @@ describe('AuthService', () => { }); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValue({ ...authToken, - user: user, + user: Promise.resolve(user), accessTokenHash: accessTokenHash, }); jest @@ -204,14 +204,14 @@ describe('AuthService', () => { it('works', async () => { jest.spyOn(authTokenRepo, 'findOne').mockResolvedValue({ ...authToken, - user: user, + user: Promise.resolve(user), }); jest .spyOn(authTokenRepo, 'remove') .mockImplementationOnce(async (token, __): Promise => { expect(token).toEqual({ ...authToken, - user: user, + user: Promise.resolve(user), }); return authToken; }); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 789e9b7fb..90bb67094 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -53,7 +53,11 @@ export class AuthService { } const accessToken = await this.getAuthTokenAndValidate(keyId, secret); await this.setLastUsedToken(keyId); - return await this.usersService.getUserByUsername(accessToken.user.username); + return await this.usersService.getUserByUsername( + ( + await accessToken.user + ).username, + ); } async createTokenForUser( From 244e3f76ea7f9e1d3dd03f7569a98f4aca5105e1 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Tue, 16 Nov 2021 18:45:40 +0100 Subject: [PATCH 02/14] refactor(author): lazy-load relations Signed-off-by: David Mehren --- src/authors/author.entity.ts | 12 ++++++------ src/notes/notes.service.spec.ts | 4 ++-- src/notes/notes.service.ts | 2 +- src/seed.ts | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/authors/author.entity.ts b/src/authors/author.entity.ts index bc24af958..de46461f1 100644 --- a/src/authors/author.entity.ts +++ b/src/authors/author.entity.ts @@ -39,21 +39,21 @@ export class Author { * Only contains sessions for anonymous users, which don't have a user set */ @OneToMany(() => Session, (session) => session.author) - sessions: Session[]; + sessions: Promise; /** * User that this author corresponds to * Only set when the user was identified (by a browser session) as a registered user at edit-time */ @ManyToOne(() => User, (user) => user.authors, { nullable: true }) - user: User | null; + user: Promise; /** * List of edits that this author created * All edits must belong to the same note */ @OneToMany(() => Edit, (edit) => edit.author) - edits: Edit[]; + edits: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -61,9 +61,9 @@ export class Author { public static create(color: number): Omit { const newAuthor = new Author(); newAuthor.color = color; - newAuthor.sessions = []; - newAuthor.user = null; - newAuthor.edits = []; + newAuthor.sessions = Promise.resolve([]); + newAuthor.user = Promise.resolve(null); + newAuthor.edits = Promise.resolve([]); return newAuthor; } } diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 3be7f998f..686646ecd 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -703,7 +703,7 @@ describe('NotesService', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; const author = Author.create(1); - author.user = user; + author.user = Promise.resolve(user); const group = Group.create('testGroup', 'testGroup', false) as Group; const content = 'testContent'; jest @@ -798,7 +798,7 @@ describe('NotesService', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; const author = Author.create(1); - author.user = user; + author.user = Promise.resolve(user); const otherUser = User.create('other hardcoded', 'Testy2') as User; otherUser.username = 'other hardcoded user'; const group = Group.create('testGroup', 'testGroup', false) as Group; diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 87f799ed2..05f8d2efc 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -343,7 +343,7 @@ export class NotesService { if (lastRevision && lastRevision.edits) { // Sort the last Revisions Edits by their updatedAt Date to get the latest one // the user of that Edit is the updateUser - return lastRevision.edits.sort( + return await lastRevision.edits.sort( (a, b) => b.updatedAt.getTime() - a.updatedAt.getTime(), )[0].author.user; } diff --git a/src/seed.ts b/src/seed.ts index 0f89ce1f5..c280cacde 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -67,7 +67,7 @@ createConnection({ const identity = Identity.create(user, ProviderType.LOCAL, false); identity.passwordHash = await hashPassword(password); connection.manager.create(Identity, identity); - author.user = user; + author.user = Promise.resolve(user); const revision = Revision.create( 'This is a test note', 'This is a test note', From 8eabfbc0a5f1fe49a93208776b38e59a0a62c660 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Tue, 16 Nov 2021 19:05:28 +0100 Subject: [PATCH 03/14] 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; } } From 8aae5cb57445cf02c813553e71c9fdcf3d90ec13 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Thu, 18 Nov 2021 18:08:29 +0100 Subject: [PATCH 04/14] docs(history-entry): document why we can't lazy-load Signed-off-by: David Mehren --- src/history/history-entry.entity.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/history/history-entry.entity.ts b/src/history/history-entry.entity.ts index 053822312..481c6b9ea 100644 --- a/src/history/history-entry.entity.ts +++ b/src/history/history-entry.entity.ts @@ -10,6 +10,13 @@ import { User } from '../users/user.entity'; @Entity() export class HistoryEntry { + /** + * The `user` and `note` properties cannot be converted to promises + * (to lazy-load them), as TypeORM gets confused about lazy composite + * primary keys. + * See https://github.com/typeorm/typeorm/issues/6908 + */ + @ManyToOne((_) => User, (user) => user.historyEntries, { onDelete: 'CASCADE', primary: true, From facdc456cd7d3999a3becf1d19331e8f2ffa4742 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Thu, 18 Nov 2021 18:47:12 +0100 Subject: [PATCH 05/14] refactor(media-upload): lazy-load relations Signed-off-by: David Mehren --- src/api/private/me/me.controller.ts | 4 ++- src/api/private/media/media.controller.ts | 2 +- src/api/private/notes/notes.controller.ts | 4 ++- src/api/public/me/me.controller.ts | 4 ++- src/api/public/media/media.controller.ts | 2 +- src/api/public/notes/notes.controller.ts | 4 ++- src/media/media-upload.entity.ts | 8 +++--- src/media/media.service.spec.ts | 33 ++++++++++++----------- src/media/media.service.ts | 8 +++--- 9 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/api/private/me/me.controller.ts b/src/api/private/me/me.controller.ts index f59af2164..6f79306d0 100644 --- a/src/api/private/me/me.controller.ts +++ b/src/api/private/me/me.controller.ts @@ -40,7 +40,9 @@ export class MeController { @Get('media') async getMyMedia(@RequestUser() user: User): Promise { const media = await this.mediaService.listUploadsByUser(user); - return media.map((media) => this.mediaService.toMediaUploadDto(media)); + return await Promise.all( + media.map((media) => this.mediaService.toMediaUploadDto(media)), + ); } @Delete() diff --git a/src/api/private/media/media.controller.ts b/src/api/private/media/media.controller.ts index fc6a6720e..4daf956f9 100644 --- a/src/api/private/media/media.controller.ts +++ b/src/api/private/media/media.controller.ts @@ -131,7 +131,7 @@ export class MediaController { const mediaUpload = await this.mediaService.findUploadByFilename( filename, ); - if (mediaUpload.user.username !== username) { + if ((await mediaUpload.user).username !== username) { this.logger.warn( `${username} tried to delete '${filename}', but is not the owner`, 'deleteMedia', diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index 5a43b713f..fd02e839b 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/src/api/private/notes/notes.controller.ts @@ -72,7 +72,9 @@ export class NotesController { @UseGuards(PermissionsGuard) async getNotesMedia(@RequestNote() note: Note): Promise { const media = await this.mediaService.listUploadsByNote(note); - return media.map((media) => this.mediaService.toMediaUploadDto(media)); + return await Promise.all( + media.map((media) => this.mediaService.toMediaUploadDto(media)), + ); } @Post() diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 059e681c7..1e1d6c908 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -188,6 +188,8 @@ export class MeController { @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getMyMedia(@RequestUser() user: User): Promise { const media = await this.mediaService.listUploadsByUser(user); - return media.map((media) => this.mediaService.toMediaUploadDto(media)); + return await Promise.all( + media.map((media) => this.mediaService.toMediaUploadDto(media)), + ); } } diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index c495a561e..d7db8b3e2 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -136,7 +136,7 @@ export class MediaController { const mediaUpload = await this.mediaService.findUploadByFilename( filename, ); - if (mediaUpload.user.username !== username) { + if ((await mediaUpload.user).username !== username) { this.logger.warn( `${username} tried to delete '${filename}', but is not the owner`, 'deleteMedia', diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 99980c9bc..28b2bd837 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -298,6 +298,8 @@ export class NotesController { @RequestNote() note: Note, ): Promise { const media = await this.mediaService.listUploadsByNote(note); - return media.map((media) => this.mediaService.toMediaUploadDto(media)); + return await Promise.all( + media.map((media) => this.mediaService.toMediaUploadDto(media)), + ); } } diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index 7a6d71e13..2e2051ca6 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -25,12 +25,12 @@ export class MediaUpload { @ManyToOne((_) => Note, (note) => note.mediaUploads, { nullable: true, }) - note: Note | null; + note: Promise; @ManyToOne((_) => User, (user) => user.mediaUploads, { nullable: false, }) - user: User; + user: Promise; @Column({ nullable: false, @@ -72,8 +72,8 @@ export class MediaUpload { ): Omit { const upload = new MediaUpload(); upload.id = id; - upload.note = note; - upload.user = user; + upload.note = Promise.resolve(note); + upload.user = Promise.resolve(user); upload.backendType = backendType; upload.backendData = null; upload.fileUrl = fileUrl; diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index a22e6c374..3260a1ef6 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -167,9 +167,9 @@ describe('MediaService', () => { const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', - user: { + user: Promise.resolve({ username: 'hardcoded', - } as User, + } as User), } as MediaUpload; jest .spyOn(service.mediaBackend, 'deleteFile') @@ -196,15 +196,15 @@ describe('MediaService', () => { const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: backendData, - user: { + user: Promise.resolve({ username: username, - } as User, + } as User), } as MediaUpload; jest .spyOn(mediaRepo, 'findOne') .mockResolvedValueOnce(mockMediaUploadEntry); const mediaUpload = await service.findUploadByFilename(testFileName); - expect(mediaUpload.user.username).toEqual(username); + expect((await mediaUpload.user).username).toEqual(username); expect(mediaUpload.backendData).toEqual(backendData); }); it("fails: can't find mediaUpload", async () => { @@ -218,13 +218,14 @@ describe('MediaService', () => { describe('listUploadsByUser', () => { describe('works', () => { + const username = 'hardcoded'; it('with one upload from user', async () => { const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', - user: { - username: 'hardcoded', - } as User, + user: Promise.resolve({ + username: username, + } as User), } as MediaUpload; jest .spyOn(mediaRepo, 'find') @@ -237,14 +238,14 @@ describe('MediaService', () => { it('without uploads from user', async () => { jest.spyOn(mediaRepo, 'find').mockResolvedValueOnce([]); const mediaList = await service.listUploadsByUser({ - username: 'hardcoded', + username: username, } as User); expect(mediaList).toEqual([]); }); it('with error (undefined as return value of find)', async () => { jest.spyOn(mediaRepo, 'find').mockResolvedValueOnce(undefined); const mediaList = await service.listUploadsByUser({ - username: 'hardcoded', + username: username, } as User); expect(mediaList).toEqual([]); }); @@ -257,9 +258,9 @@ describe('MediaService', () => { const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', - note: { + note: Promise.resolve({ id: '123', - } as Note, + } as Note), } as MediaUpload; jest .spyOn(mediaRepo, 'find') @@ -294,15 +295,15 @@ describe('MediaService', () => { const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', - note: mockNote, - user: { + note: Promise.resolve(mockNote), + user: Promise.resolve({ username: 'hardcoded', - } as User, + } as User), } as MediaUpload; jest .spyOn(mediaRepo, 'save') .mockImplementationOnce(async (entry: MediaUpload) => { - expect(entry.note).toBeNull(); + expect(await entry.note).toBeNull(); return entry; }); await service.removeNoteFromMediaUpload(mockMediaUploadEntry); diff --git a/src/media/media.service.ts b/src/media/media.service.ts index df8004aca..23ade8417 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -181,7 +181,7 @@ export class MediaService { 'Setting note to null for mediaUpload: ' + mediaUpload.id, 'removeNoteFromMediaUpload', ); - mediaUpload.note = null; + mediaUpload.note = Promise.resolve(null); await this.mediaUploadRepository.save(mediaUpload); } @@ -219,12 +219,12 @@ export class MediaService { } } - toMediaUploadDto(mediaUpload: MediaUpload): MediaUploadDto { + async toMediaUploadDto(mediaUpload: MediaUpload): Promise { return { url: mediaUpload.fileUrl, - noteId: mediaUpload.note?.id ?? null, + noteId: (await mediaUpload.note)?.id ?? null, createdAt: mediaUpload.createdAt, - username: mediaUpload.user.username, + username: (await mediaUpload.user).username, }; } From 9e608c75d30d7e999d1ddace2fbe53a1e1d7b981 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Thu, 18 Nov 2021 18:53:39 +0100 Subject: [PATCH 06/14] refactor(alias): lazy-load relations Signed-off-by: David Mehren --- src/notes/alias.entity.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/notes/alias.entity.ts b/src/notes/alias.entity.ts index 70a633f09..16d52e5c6 100644 --- a/src/notes/alias.entity.ts +++ b/src/notes/alias.entity.ts @@ -49,7 +49,7 @@ export class Alias { @ManyToOne((_) => Note, (note) => note.aliases, { onDelete: 'CASCADE', // This deletes the Alias, when the associated Note is deleted }) - note: Note; + note: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -58,7 +58,7 @@ export class Alias { const alias = new Alias(); alias.name = name; alias.primary = primary; - alias.note = note; + alias.note = Promise.resolve(note); return alias; } } From 3c0c11e3d4546596799917755a32a2a4425b95a7 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Tue, 30 Nov 2021 16:46:07 +0100 Subject: [PATCH 07/14] refactor(note): lazy-load relations Signed-off-by: David Mehren --- src/api/private/alias/alias.controller.ts | 6 +- .../private/me/history/history.controller.ts | 8 +- src/api/public/alias/alias.controller.ts | 6 +- src/api/public/me/me.controller.ts | 4 +- src/api/public/notes/notes.controller.ts | 19 +- src/history/history.service.spec.ts | 44 ++-- src/history/history.service.ts | 6 +- src/history/utils.spec.ts | 22 +- src/history/utils.ts | 7 +- src/media/media.service.spec.ts | 4 +- src/notes/alias.service.spec.ts | 43 ++-- src/notes/alias.service.ts | 18 +- src/notes/note.entity.ts | 30 +-- src/notes/notes.service.spec.ts | 204 +++++++++--------- src/notes/notes.service.ts | 43 ++-- src/notes/utils.spec.ts | 12 +- src/notes/utils.ts | 4 +- src/permissions/permissions.service.spec.ts | 52 ++--- src/permissions/permissions.service.ts | 23 +- src/seed.ts | 10 +- test/private-api/history.e2e-spec.ts | 28 +-- test/public-api/me.e2e-spec.ts | 24 ++- test/public-api/notes.e2e-spec.ts | 10 +- 23 files changed, 343 insertions(+), 284 deletions(-) diff --git a/src/api/private/alias/alias.controller.ts b/src/api/private/alias/alias.controller.ts index d32cc21cf..704143853 100644 --- a/src/api/private/alias/alias.controller.ts +++ b/src/api/private/alias/alias.controller.ts @@ -56,7 +56,7 @@ export class AliasController { const note = await this.noteService.getNoteByIdOrAlias( newAliasDto.noteIdOrAlias, ); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } const updatedAlias = await this.aliasService.addAlias( @@ -88,7 +88,7 @@ export class AliasController { } try { const note = await this.noteService.getNoteByIdOrAlias(alias); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } const updatedAlias = await this.aliasService.makeAliasPrimary( @@ -115,7 +115,7 @@ export class AliasController { ): Promise { try { const note = await this.noteService.getNoteByIdOrAlias(alias); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } await this.aliasService.removeAlias(note, alias); diff --git a/src/api/private/me/history/history.controller.ts b/src/api/private/me/history/history.controller.ts index 6b3ebe9b1..5c2b684dc 100644 --- a/src/api/private/me/history/history.controller.ts +++ b/src/api/private/me/history/history.controller.ts @@ -45,8 +45,10 @@ export class HistoryController { async getHistory(@RequestUser() user: User): Promise { try { const foundEntries = await this.historyService.getEntriesByUser(user); - return foundEntries.map((entry) => - this.historyService.toHistoryEntryDto(entry), + return await Promise.all( + foundEntries.map((entry) => + this.historyService.toHistoryEntryDto(entry), + ), ); } catch (e) { if (e instanceof NotInDBError) { @@ -96,7 +98,7 @@ export class HistoryController { user, entryUpdateDto, ); - return this.historyService.toHistoryEntryDto(newEntry); + return await this.historyService.toHistoryEntryDto(newEntry); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); diff --git a/src/api/public/alias/alias.controller.ts b/src/api/public/alias/alias.controller.ts index 35bf80676..c3faa10db 100644 --- a/src/api/public/alias/alias.controller.ts +++ b/src/api/public/alias/alias.controller.ts @@ -69,7 +69,7 @@ export class AliasController { const note = await this.noteService.getNoteByIdOrAlias( newAliasDto.noteIdOrAlias, ); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } const updatedAlias = await this.aliasService.addAlias( @@ -107,7 +107,7 @@ export class AliasController { } try { const note = await this.noteService.getNoteByIdOrAlias(alias); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } const updatedAlias = await this.aliasService.makeAliasPrimary( @@ -139,7 +139,7 @@ export class AliasController { ): Promise { try { const note = await this.noteService.getNoteByIdOrAlias(alias); - if (!this.permissionsService.isOwner(user, note)) { + if (!(await this.permissionsService.isOwner(user, note))) { throw new UnauthorizedException('Reading note denied!'); } await this.aliasService.removeAlias(note, alias); diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 1e1d6c908..bb9c33089 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -101,7 +101,7 @@ export class MeController { ): Promise { try { const foundEntry = await this.historyService.getEntryByNote(note, user); - return this.historyService.toHistoryEntryDto(foundEntry); + return await this.historyService.toHistoryEntryDto(foundEntry); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -126,7 +126,7 @@ export class MeController { ): Promise { // ToDo: Check if user is allowed to pin this history entry try { - return this.historyService.toHistoryEntryDto( + return await this.historyService.toHistoryEntryDto( await this.historyService.updateHistoryEntry( note, user, diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 28b2bd837..351d42f8d 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,17 +26,24 @@ 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'; @@ -50,7 +57,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'; @@ -230,7 +237,7 @@ export class NotesController { @RequestNote() note: Note, @Body() updateDto: NotePermissionsUpdateDto, ): Promise { - return this.noteService.toNotePermissionsDto( + return await this.noteService.toNotePermissionsDto( await this.noteService.updateNotePermissions(note, updateDto), ); } diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index bf4a02f68..84df3999f 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -159,9 +159,9 @@ describe('HistoryService', () => { Note.create(user, alias) as Note, user, ); - expect(createHistoryEntry.note.aliases).toHaveLength(1); - expect(createHistoryEntry.note.aliases[0].name).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); + expect(await createHistoryEntry.note.aliases).toHaveLength(1); + expect((await createHistoryEntry.note.aliases)[0].name).toEqual(alias); + expect(await createHistoryEntry.note.owner).toEqual(user); expect(createHistoryEntry.user).toEqual(user); expect(createHistoryEntry.pinStatus).toEqual(false); }); @@ -177,9 +177,9 @@ describe('HistoryService', () => { Note.create(user, alias) as Note, user, ); - expect(createHistoryEntry.note.aliases).toHaveLength(1); - expect(createHistoryEntry.note.aliases[0].name).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); + expect(await createHistoryEntry.note.aliases).toHaveLength(1); + expect((await createHistoryEntry.note.aliases)[0].name).toEqual(alias); + expect(await createHistoryEntry.note.owner).toEqual(user); expect(createHistoryEntry.user).toEqual(user); expect(createHistoryEntry.pinStatus).toEqual(false); expect(createHistoryEntry.updatedAt.getTime()).toBeGreaterThanOrEqual( @@ -223,9 +223,9 @@ describe('HistoryService', () => { pinStatus: true, }, ); - expect(updatedHistoryEntry.note.aliases).toHaveLength(1); - expect(updatedHistoryEntry.note.aliases[0].name).toEqual(alias); - expect(updatedHistoryEntry.note.owner).toEqual(user); + expect(await updatedHistoryEntry.note.aliases).toHaveLength(1); + expect((await updatedHistoryEntry.note.aliases)[0].name).toEqual(alias); + expect(await updatedHistoryEntry.note.owner).toEqual(user); expect(updatedHistoryEntry.user).toEqual(user); expect(updatedHistoryEntry.pinStatus).toEqual(true); }); @@ -371,11 +371,13 @@ describe('HistoryService', () => { const mockedManager = { find: jest.fn().mockResolvedValueOnce([historyEntry]), createQueryBuilder: () => createQueryBuilder, - remove: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { - expect(entry.note.aliases).toHaveLength(1); - expect(entry.note.aliases[0].name).toEqual(alias); - expect(entry.pinStatus).toEqual(false); - }), + remove: jest + .fn() + .mockImplementationOnce(async (entry: HistoryEntry) => { + expect(await entry.note.aliases).toHaveLength(1); + expect((await entry.note.aliases)[0].name).toEqual(alias); + expect(entry.pinStatus).toEqual(false); + }), save: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { expect(entry.note.aliases).toEqual( newlyCreatedHistoryEntry.note.aliases, @@ -402,11 +404,13 @@ describe('HistoryService', () => { const tags = ['tag1', 'tag2']; const note = Note.create(user, alias) as Note; note.title = title; - note.tags = tags.map((tag) => { - const newTag = new Tag(); - newTag.name = tag; - return newTag; - }); + note.tags = Promise.resolve( + tags.map((tag) => { + const newTag = new Tag(); + newTag.name = tag; + return newTag; + }), + ); const historyEntry = HistoryEntry.create(user, note) as HistoryEntry; historyEntry.pinStatus = true; const createQueryBuilder = { @@ -420,7 +424,7 @@ describe('HistoryService', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore .mockImplementation(() => createQueryBuilder); - const historyEntryDto = service.toHistoryEntryDto(historyEntry); + const historyEntryDto = await service.toHistoryEntryDto(historyEntry); expect(historyEntryDto.pinStatus).toEqual(true); expect(historyEntryDto.identifier).toEqual(alias); expect(historyEntryDto.tags).toEqual(tags); diff --git a/src/history/history.service.ts b/src/history/history.service.ts index a9b478b34..c4081d3bc 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -186,11 +186,11 @@ export class HistoryService { * @param {HistoryEntry} entry - the history entry to use * @return {HistoryEntryDto} the built HistoryEntryDto */ - toHistoryEntryDto(entry: HistoryEntry): HistoryEntryDto { + async toHistoryEntryDto(entry: HistoryEntry): Promise { return { - identifier: getIdentifier(entry), + identifier: await getIdentifier(entry), lastVisited: entry.updatedAt, - tags: this.notesService.toTagList(entry.note), + tags: await this.notesService.toTagList(entry.note), title: entry.note.title ?? '', pinStatus: entry.pinStatus, }; diff --git a/src/history/utils.spec.ts b/src/history/utils.spec.ts index 582d7d403..d65fc617b 100644 --- a/src/history/utils.spec.ts +++ b/src/history/utils.spec.ts @@ -18,19 +18,19 @@ describe('getIdentifier', () => { note = Note.create(user, alias) as Note; entry = HistoryEntry.create(user, note) as HistoryEntry; }); - it('returns the publicId if there are no aliases', () => { - note.aliases = undefined as unknown as Alias[]; - expect(getIdentifier(entry)).toEqual(note.publicId); + it('returns the publicId if there are no aliases', async () => { + note.aliases = Promise.resolve(undefined as unknown as Alias[]); + expect(await getIdentifier(entry)).toEqual(note.publicId); }); - it('returns the publicId, if the alias array is empty', () => { - note.aliases = []; - expect(getIdentifier(entry)).toEqual(note.publicId); + it('returns the publicId, if the alias array is empty', async () => { + note.aliases = Promise.resolve([]); + expect(await getIdentifier(entry)).toEqual(note.publicId); }); - it('returns the publicId, if the only alias is not primary', () => { - note.aliases[0].primary = false; - expect(getIdentifier(entry)).toEqual(note.publicId); + it('returns the publicId, if the only alias is not primary', async () => { + (await note.aliases)[0].primary = false; + expect(await getIdentifier(entry)).toEqual(note.publicId); }); - it('returns the primary alias, if one exists', () => { - expect(getIdentifier(entry)).toEqual(note.aliases[0].name); + it('returns the primary alias, if one exists', async () => { + expect(await getIdentifier(entry)).toEqual((await note.aliases)[0].name); }); }); diff --git a/src/history/utils.ts b/src/history/utils.ts index ea49cee63..5a11718d3 100644 --- a/src/history/utils.ts +++ b/src/history/utils.ts @@ -6,11 +6,12 @@ import { getPrimaryAlias } from '../notes/utils'; import { HistoryEntry } from './history-entry.entity'; -export function getIdentifier(entry: HistoryEntry): string { - if (!entry.note.aliases || entry.note.aliases.length === 0) { +export async function getIdentifier(entry: HistoryEntry): Promise { + const aliases = await entry.note.aliases; + if (!aliases || aliases.length === 0) { return entry.note.publicId; } - const primaryAlias = getPrimaryAlias(entry.note); + const primaryAlias = await getPrimaryAlias(entry.note); if (primaryAlias === undefined) { return entry.note.publicId; } diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 3260a1ef6..0ce609ebd 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -291,7 +291,9 @@ describe('MediaService', () => { describe('removeNoteFromMediaUpload', () => { it('works', async () => { const mockNote = {} as Note; - mockNote.aliases = [Alias.create('test', mockNote, true) as Alias]; + mockNote.aliases = Promise.resolve([ + Alias.create('test', mockNote, true) as Alias, + ]); const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', diff --git a/src/notes/alias.service.spec.ts b/src/notes/alias.service.spec.ts index 218b04c30..80cbd1a4a 100644 --- a/src/notes/alias.service.spec.ts +++ b/src/notes/alias.service.spec.ts @@ -158,8 +158,11 @@ describe('AliasService', () => { const alias2 = 'testAlias2'; const user = User.create('hardcoded', 'Testy') as User; describe('removes one alias correctly', () => { - const note = Note.create(user, alias) as Note; - note.aliases.push(Alias.create(alias2, note, false) as Alias); + let note: Note; + beforeAll(async () => { + note = Note.create(user, alias) as Note; + (await note.aliases).push(Alias.create(alias2, note, false) as Alias); + }); it('with two aliases', async () => { jest .spyOn(noteRepo, 'save') @@ -170,9 +173,10 @@ describe('AliasService', () => { async (alias: Alias): Promise => alias, ); const savedNote = await service.removeAlias(note, alias2); - expect(savedNote.aliases).toHaveLength(1); - expect(savedNote.aliases[0].name).toEqual(alias); - expect(savedNote.aliases[0].primary).toBeTruthy(); + const aliases = await savedNote.aliases; + expect(aliases).toHaveLength(1); + expect(aliases[0].name).toEqual(alias); + expect(aliases[0].primary).toBeTruthy(); }); it('with one alias, that is primary', async () => { jest @@ -184,12 +188,15 @@ describe('AliasService', () => { async (alias: Alias): Promise => alias, ); const savedNote = await service.removeAlias(note, alias); - expect(savedNote.aliases).toHaveLength(0); + expect(await savedNote.aliases).toHaveLength(0); }); }); describe('does not remove one alias', () => { - const note = Note.create(user, alias) as Note; - note.aliases.push(Alias.create(alias2, note, false) as Alias); + let note: Note; + beforeEach(async () => { + note = Note.create(user, alias) as Note; + (await note.aliases).push(Alias.create(alias2, note, false) as Alias); + }); it('if the alias is unknown', async () => { await expect(service.removeAlias(note, 'non existent')).rejects.toThrow( NotInDBError, @@ -206,10 +213,18 @@ describe('AliasService', () => { describe('makeAliasPrimary', () => { const user = User.create('hardcoded', 'Testy') as User; const aliasName = 'testAlias'; - const note = Note.create(user, aliasName) as Note; - const alias = Alias.create(aliasName, note, true) as Alias; - const alias2 = Alias.create('testAlias2', note, false) as Alias; - note.aliases.push(alias2); + let note: Note; + let alias: Alias; + let alias2: Alias; + beforeEach(async () => { + note = Note.create(user, aliasName) as Note; + alias = Alias.create(aliasName, note, true) as Alias; + alias2 = Alias.create('testAlias2', note, false) as Alias; + (await note.aliases).push( + Alias.create('testAlias2', note, false) as Alias, + ); + }); + it('mark the alias as primary', async () => { jest .spyOn(aliasRepo, 'findOne') @@ -224,10 +239,10 @@ describe('AliasService', () => { where: () => createQueryBuilder, orWhere: () => createQueryBuilder, setParameter: () => createQueryBuilder, - getOne: () => { + getOne: async () => { return { ...note, - aliases: note.aliases.map((anAlias) => { + aliases: (await note.aliases).map((anAlias) => { if (anAlias.primary) { anAlias.primary = false; } diff --git a/src/notes/alias.service.ts b/src/notes/alias.service.ts index 96f8c0d55..179541c7e 100644 --- a/src/notes/alias.service.ts +++ b/src/notes/alias.service.ts @@ -62,13 +62,13 @@ export class AliasService { ); } let newAlias; - if (note.aliases.length === 0) { + if ((await note.aliases).length === 0) { // the first alias is automatically made the primary alias newAlias = Alias.create(alias, note, true); } else { newAlias = Alias.create(alias, note, false); } - note.aliases.push(newAlias as Alias); + (await note.aliases).push(newAlias as Alias); await this.noteRepository.save(note); return newAlias as Alias; @@ -87,7 +87,7 @@ export class AliasService { let oldPrimaryId = ''; let newPrimaryId = ''; - for (const anAlias of note.aliases) { + for (const anAlias of await note.aliases) { // found old primary if (anAlias.primary) { oldPrimaryId = anAlias.id; @@ -134,9 +134,9 @@ export class AliasService { * @throws {PrimaryAliasDeletionForbiddenError} the primary alias can only be deleted if it's the only alias */ async removeAlias(note: Note, alias: string): Promise { - const primaryAlias = getPrimaryAlias(note); + const primaryAlias = await getPrimaryAlias(note); - if (primaryAlias === alias && note.aliases.length !== 1) { + if (primaryAlias === alias && (await note.aliases).length !== 1) { this.logger.debug( `The alias '${alias}' is the primary alias, which can only be removed if it's the only alias.`, 'removeAlias', @@ -146,10 +146,10 @@ export class AliasService { ); } - const filteredAliases = note.aliases.filter( + const filteredAliases = (await note.aliases).filter( (anAlias) => anAlias.name !== alias, ); - if (note.aliases.length === filteredAliases.length) { + if ((await note.aliases).length === filteredAliases.length) { this.logger.debug( `The alias '${alias}' is not used by this note or is the primary alias, which can't be removed.`, 'removeAlias', @@ -158,13 +158,13 @@ export class AliasService { `The alias '${alias}' is not used by this note or is the primary alias, which can't be removed.`, ); } - const aliasToDelete = note.aliases.find( + const aliasToDelete = (await note.aliases).find( (anAlias) => anAlias.name === alias, ); if (aliasToDelete !== undefined) { await this.aliasRepository.remove(aliasToDelete); } - note.aliases = filteredAliases; + note.aliases = Promise.resolve(filteredAliases); return await this.noteRepository.save(note); } diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 42e697672..f099c2820 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -36,21 +36,21 @@ export class Note { (alias) => alias.note, { cascade: true }, // This ensures that embedded Aliases are automatically saved to the database ) - aliases: Alias[]; + aliases: Promise; @OneToMany( (_) => NoteGroupPermission, (groupPermission) => groupPermission.note, { cascade: true }, // This ensures that embedded NoteGroupPermissions are automatically saved to the database ) - groupPermissions: NoteGroupPermission[]; + groupPermissions: Promise; @OneToMany( (_) => NoteUserPermission, (userPermission) => userPermission.note, { cascade: true }, // This ensures that embedded NoteUserPermission are automatically saved to the database ) - userPermissions: NoteUserPermission[]; + userPermissions: Promise; @Column({ nullable: false, @@ -62,16 +62,16 @@ export class Note { onDelete: 'CASCADE', // This deletes the Note, when the associated User is deleted nullable: true, }) - owner: User | null; + owner: Promise; @OneToMany((_) => Revision, (revision) => revision.note, { cascade: true }) revisions: Promise; @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) - historyEntries: HistoryEntry[]; + historyEntries: Promise; @OneToMany((_) => MediaUpload, (mediaUpload) => mediaUpload.note) - mediaUploads: MediaUpload[]; + mediaUploads: Promise; @Column({ nullable: true, @@ -87,7 +87,7 @@ export class Note { @ManyToMany((_) => Tag, (tag) => tag.notes, { eager: true, cascade: true }) @JoinTable() - tags: Tag[]; + tags: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -101,18 +101,18 @@ export class Note { const newNote = new Note(); newNote.publicId = generatePublicId(); newNote.aliases = alias - ? [Alias.create(alias, newNote, true) as Alias] - : []; - newNote.userPermissions = []; - newNote.groupPermissions = []; + ? Promise.resolve([Alias.create(alias, newNote, true) as Alias]) + : Promise.resolve([]); + newNote.userPermissions = Promise.resolve([]); + newNote.groupPermissions = Promise.resolve([]); newNote.viewCount = 0; - newNote.owner = owner; + newNote.owner = Promise.resolve(owner); newNote.revisions = Promise.resolve([]); - newNote.historyEntries = []; - newNote.mediaUploads = []; + newNote.historyEntries = Promise.resolve([]); + newNote.mediaUploads = Promise.resolve([]); newNote.description = null; newNote.title = null; - newNote.tags = []; + newNote.tags = Promise.resolve([]); return newNote; } } diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 686646ecd..d4a5154e6 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -168,51 +168,51 @@ describe('NotesService', () => { const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toHaveLength(0); - expect(newNote.userPermissions).toHaveLength(0); - expect(newNote.groupPermissions).toHaveLength(0); - expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toBeNull(); - expect(newNote.aliases).toHaveLength(0); + expect(await newNote.historyEntries).toHaveLength(0); + expect(await newNote.userPermissions).toHaveLength(0); + expect(await newNote.groupPermissions).toHaveLength(0); + expect(await newNote.tags).toHaveLength(0); + expect(await newNote.owner).toBeNull(); + expect(await newNote.aliases).toHaveLength(0); }); it('without alias, with owner', async () => { const newNote = await service.createNote(content, user); const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toHaveLength(1); - expect(newNote.historyEntries[0].user).toEqual(user); - expect(newNote.userPermissions).toHaveLength(0); - expect(newNote.groupPermissions).toHaveLength(0); - expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toEqual(user); - expect(newNote.aliases).toHaveLength(0); + expect(await newNote.historyEntries).toHaveLength(1); + expect((await newNote.historyEntries)[0].user).toEqual(user); + expect(await newNote.userPermissions).toHaveLength(0); + expect(await newNote.groupPermissions).toHaveLength(0); + expect(await newNote.tags).toHaveLength(0); + expect(await newNote.owner).toEqual(user); + expect(await newNote.aliases).toHaveLength(0); }); it('with alias, without owner', async () => { const newNote = await service.createNote(content, null, alias); const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toHaveLength(0); - expect(newNote.userPermissions).toHaveLength(0); - expect(newNote.groupPermissions).toHaveLength(0); - expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toBeNull(); - expect(newNote.aliases).toHaveLength(1); + expect(await newNote.historyEntries).toHaveLength(0); + expect(await newNote.userPermissions).toHaveLength(0); + expect(await newNote.groupPermissions).toHaveLength(0); + expect(await newNote.tags).toHaveLength(0); + expect(await newNote.owner).toBeNull(); + expect(await newNote.aliases).toHaveLength(1); }); it('with alias, with owner', async () => { const newNote = await service.createNote(content, user, alias); const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); - expect(newNote.historyEntries).toHaveLength(1); - expect(newNote.historyEntries[0].user).toEqual(user); - expect(newNote.userPermissions).toHaveLength(0); - expect(newNote.groupPermissions).toHaveLength(0); - expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toEqual(user); - expect(newNote.aliases).toHaveLength(1); - expect(newNote.aliases[0].name).toEqual(alias); + expect(await newNote.historyEntries).toHaveLength(1); + expect((await newNote.historyEntries)[0].user).toEqual(user); + expect(await newNote.userPermissions).toHaveLength(0); + expect(await newNote.groupPermissions).toHaveLength(0); + expect(await newNote.tags).toHaveLength(0); + expect(await newNote.owner).toEqual(user); + expect(await newNote.aliases).toHaveLength(1); + expect((await newNote.aliases)[0].name).toEqual(alias); }); }); describe('fails:', () => { @@ -379,8 +379,8 @@ describe('NotesService', () => { sharedToUsers: [], sharedToGroups: [], }); - expect(savedNote.userPermissions).toHaveLength(0); - expect(savedNote.groupPermissions).toHaveLength(0); + expect(await savedNote.userPermissions).toHaveLength(0); + expect(await savedNote.groupPermissions).toHaveLength(0); }); it('with empty GroupPermissions and with new UserPermissions', async () => { jest @@ -393,24 +393,24 @@ describe('NotesService', () => { sharedToUsers: [userPermissionUpdate], sharedToGroups: [], }); - expect(savedNote.userPermissions).toHaveLength(1); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect(await savedNote.userPermissions).toHaveLength(1); + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions).toHaveLength(0); + expect(await savedNote.groupPermissions).toHaveLength(0); }); it('with empty GroupPermissions and with existing UserPermissions', async () => { const noteWithPreexistingPermissions: Note = { ...note }; - noteWithPreexistingPermissions.userPermissions = [ + noteWithPreexistingPermissions.userPermissions = Promise.resolve([ { note: noteWithPreexistingPermissions, user: user, canEdit: !userPermissionUpdate.canEdit, }, - ]; + ]); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -421,14 +421,14 @@ describe('NotesService', () => { sharedToUsers: [userPermissionUpdate], sharedToGroups: [], }); - expect(savedNote.userPermissions).toHaveLength(1); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect(await savedNote.userPermissions).toHaveLength(1); + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions).toHaveLength(0); + expect(await savedNote.groupPermissions).toHaveLength(0); }); it('with new GroupPermissions and with empty UserPermissions', async () => { jest @@ -441,11 +441,11 @@ describe('NotesService', () => { sharedToUsers: [], sharedToGroups: [groupPermissionUpate], }); - expect(savedNote.userPermissions).toHaveLength(0); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect(await savedNote.userPermissions).toHaveLength(0); + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); @@ -461,28 +461,28 @@ describe('NotesService', () => { sharedToUsers: [userPermissionUpdate], sharedToGroups: [groupPermissionUpate], }); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); it('with new GroupPermissions and with existing UserPermissions', async () => { const noteWithUserPermission: Note = { ...note }; - noteWithUserPermission.userPermissions = [ + noteWithUserPermission.userPermissions = Promise.resolve([ { note: noteWithUserPermission, user: user, canEdit: !userPermissionUpdate.canEdit, }, - ]; + ]); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -497,28 +497,28 @@ describe('NotesService', () => { sharedToGroups: [groupPermissionUpate], }, ); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); it('with existing GroupPermissions and with empty UserPermissions', async () => { const noteWithPreexistingPermissions: Note = { ...note }; - noteWithPreexistingPermissions.groupPermissions = [ + noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { note: noteWithPreexistingPermissions, group: group, canEdit: !groupPermissionUpate.canEdit, }, - ]; + ]); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); jest .spyOn(noteRepo, 'save') @@ -532,23 +532,23 @@ describe('NotesService', () => { sharedToGroups: [groupPermissionUpate], }, ); - expect(savedNote.userPermissions).toHaveLength(0); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect(await savedNote.userPermissions).toHaveLength(0); + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); it('with existing GroupPermissions and with new UserPermissions', async () => { const noteWithPreexistingPermissions: Note = { ...note }; - noteWithPreexistingPermissions.groupPermissions = [ + noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { note: noteWithPreexistingPermissions, group: group, canEdit: !groupPermissionUpate.canEdit, }, - ]; + ]); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -563,35 +563,35 @@ describe('NotesService', () => { sharedToGroups: [groupPermissionUpate], }, ); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); it('with existing GroupPermissions and with existing UserPermissions', async () => { const noteWithPreexistingPermissions: Note = { ...note }; - noteWithPreexistingPermissions.groupPermissions = [ + noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { note: noteWithPreexistingPermissions, group: group, canEdit: !groupPermissionUpate.canEdit, }, - ]; - noteWithPreexistingPermissions.userPermissions = [ + ]); + noteWithPreexistingPermissions.userPermissions = Promise.resolve([ { note: noteWithPreexistingPermissions, user: user, canEdit: !userPermissionUpdate.canEdit, }, - ]; + ]); jest .spyOn(noteRepo, 'save') .mockImplementationOnce(async (entry: Note) => { @@ -606,16 +606,16 @@ describe('NotesService', () => { sharedToGroups: [groupPermissionUpate], }, ); - expect(savedNote.userPermissions[0].user.username).toEqual( + expect((await savedNote.userPermissions)[0].user.username).toEqual( userPermissionUpdate.username, ); - expect(savedNote.userPermissions[0].canEdit).toEqual( + expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect(savedNote.groupPermissions[0].group.name).toEqual( + expect((await savedNote.groupPermissions)[0].group.name).toEqual( groupPermissionUpate.groupname, ); - expect(savedNote.groupPermissions[0].canEdit).toEqual( + expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpate.canEdit, ); }); @@ -653,16 +653,16 @@ describe('NotesService', () => { describe('toTagList', () => { it('works', async () => { const note = {} as Note; - note.tags = [ + note.tags = Promise.resolve([ { id: 1, name: 'testTag', notes: [note], }, - ]; - const tagList = service.toTagList(note); + ]); + const tagList = await service.toTagList(note); expect(tagList).toHaveLength(1); - expect(tagList[0]).toEqual(note.tags[0].name); + expect(tagList[0]).toEqual((await note.tags)[0].name); }); }); @@ -671,21 +671,21 @@ describe('NotesService', () => { const user = User.create('hardcoded', 'Testy') as User; const group = Group.create('testGroup', 'testGroup', false) as Group; const note = Note.create(user) as Note; - note.userPermissions = [ + note.userPermissions = Promise.resolve([ { note: note, user: user, canEdit: true, }, - ]; - note.groupPermissions = [ + ]); + note.groupPermissions = Promise.resolve([ { note: note, group: group, canEdit: true, }, - ]; - const permissions = service.toNotePermissionsDto(note); + ]); + const permissions = await service.toNotePermissionsDto(note); expect(permissions.owner).not.toEqual(null); expect(permissions.owner?.username).toEqual(user.username); expect(permissions.sharedToUsers).toHaveLength(1); @@ -740,36 +740,38 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.aliases = [Alias.create('testAlias', note, true) as Alias]; + note.aliases = Promise.resolve([ + Alias.create('testAlias', note, true) as Alias, + ]); note.title = 'testTitle'; note.description = 'testDescription'; - note.owner = user; - note.userPermissions = [ + note.owner = Promise.resolve(user); + note.userPermissions = Promise.resolve([ { note: note, user: user, canEdit: true, }, - ]; - note.groupPermissions = [ + ]); + note.groupPermissions = Promise.resolve([ { note: note, group: group, canEdit: true, }, - ]; - note.tags = [ + ]); + note.tags = Promise.resolve([ { id: 1, name: 'testTag', notes: [note], }, - ]; + ]); note.viewCount = 1337; const metadataDto = await service.toNoteMetadataDto(note); expect(metadataDto.id).toEqual(note.publicId); expect(metadataDto.aliases).toHaveLength(1); - expect(metadataDto.aliases[0]).toEqual(note.aliases[0].name); + expect(metadataDto.aliases[0]).toEqual((await note.aliases)[0].name); expect(metadataDto.title).toEqual(note.title); expect(metadataDto.createTime).toEqual(revisions[0].createdAt); expect(metadataDto.description).toEqual(note.description); @@ -787,7 +789,7 @@ describe('NotesService', () => { ).toEqual(group.displayName); expect(metadataDto.permissions.sharedToGroups[0].canEdit).toEqual(true); expect(metadataDto.tags).toHaveLength(1); - expect(metadataDto.tags[0]).toEqual(note.tags[0].name); + expect(metadataDto.tags[0]).toEqual((await note.tags)[0].name); expect(metadataDto.updateTime).toEqual(revisions[0].createdAt); expect(metadataDto.updateUser.username).toEqual(user.username); expect(metadataDto.viewCount).toEqual(note.viewCount); @@ -840,36 +842,38 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.aliases = [Alias.create('testAlias', note, true) as Alias]; + note.aliases = Promise.resolve([ + Alias.create('testAlias', note, true) as Alias, + ]); note.title = 'testTitle'; note.description = 'testDescription'; - note.owner = user; - note.userPermissions = [ + note.owner = Promise.resolve(user); + note.userPermissions = Promise.resolve([ { note: note, user: user, canEdit: true, }, - ]; - note.groupPermissions = [ + ]); + note.groupPermissions = Promise.resolve([ { note: note, group: group, canEdit: true, }, - ]; - note.tags = [ + ]); + note.tags = Promise.resolve([ { id: 1, name: 'testTag', notes: [note], }, - ]; + ]); note.viewCount = 1337; const noteDto = await service.toNoteDto(note); expect(noteDto.metadata.id).toEqual(note.publicId); expect(noteDto.metadata.aliases).toHaveLength(1); - expect(noteDto.metadata.aliases[0]).toEqual(note.aliases[0].name); + expect(noteDto.metadata.aliases[0]).toEqual((await note.aliases)[0].name); expect(noteDto.metadata.title).toEqual(note.title); expect(noteDto.metadata.createTime).toEqual(revisions[0].createdAt); expect(noteDto.metadata.description).toEqual(note.description); @@ -893,7 +897,7 @@ describe('NotesService', () => { true, ); expect(noteDto.metadata.tags).toHaveLength(1); - expect(noteDto.metadata.tags[0]).toEqual(note.tags[0].name); + expect(noteDto.metadata.tags[0]).toEqual((await note.tags)[0].name); expect(noteDto.metadata.updateTime).toEqual(revisions[0].createdAt); expect(noteDto.metadata.updateUser.username).toEqual(user.username); expect(noteDto.metadata.viewCount).toEqual(note.viewCount); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 05f8d2efc..51a24d07e 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -100,9 +100,9 @@ export class NotesService { Revision.create(noteContent, noteContent, newNote as Note) as Revision, ]); if (owner) { - newNote.historyEntries = [ + newNote.historyEntries = Promise.resolve([ HistoryEntry.create(owner, newNote as Note) as HistoryEntry, - ]; + ]); } try { return await this.noteRepository.save(newNote); @@ -262,8 +262,8 @@ export class NotesService { //TODO: Calculate patch revisions.push(Revision.create(noteContent, noteContent, note) as Revision); note.revisions = Promise.resolve(revisions); - note.userPermissions = []; - note.groupPermissions = []; + note.userPermissions = Promise.resolve([]); + note.groupPermissions = Promise.resolve([]); return await this.noteRepository.save(note); } @@ -298,8 +298,8 @@ export class NotesService { ); } - note.userPermissions = []; - note.groupPermissions = []; + note.userPermissions = Promise.resolve([]); + note.groupPermissions = Promise.resolve([]); // Create new userPermissions for (const newUserPermission of newPermissions.sharedToUsers) { @@ -312,7 +312,7 @@ export class NotesService { newUserPermission.canEdit, ); createdPermission.note = note; - note.userPermissions.push(createdPermission); + (await note.userPermissions).push(createdPermission); } // Create groupPermissions @@ -326,7 +326,7 @@ export class NotesService { newGroupPermission.canEdit, ); createdPermission.note = note; - note.groupPermissions.push(createdPermission); + (await note.groupPermissions).push(createdPermission); } return await this.noteRepository.save(note); @@ -348,7 +348,7 @@ export class NotesService { )[0].author.user; } // If there are no Edits, the owner is the updateUser - return note.owner; + return await note.owner; } /** @@ -356,8 +356,8 @@ export class NotesService { * @param {Note} note - the note to use * @return {string[]} string array of tags names */ - toTagList(note: Note): string[] { - return note.tags.map((tag) => tag.name); + async toTagList(note: Note): Promise { + return (await note.tags).map((tag) => tag.name); } /** @@ -365,14 +365,17 @@ export class NotesService { * @param {Note} note - the note to use * @return {NotePermissionsDto} the built NotePermissionDto */ - toNotePermissionsDto(note: Note): NotePermissionsDto { + async toNotePermissionsDto(note: Note): Promise { + const owner = await note.owner; + const userPermissions = await note.userPermissions; + const groupPermissions = await note.groupPermissions; return { - owner: note.owner ? this.usersService.toUserDto(note.owner) : null, - sharedToUsers: note.userPermissions.map((noteUserPermission) => ({ + owner: owner ? this.usersService.toUserDto(owner) : null, + sharedToUsers: userPermissions.map((noteUserPermission) => ({ user: this.usersService.toUserDto(noteUserPermission.user), canEdit: noteUserPermission.canEdit, })), - sharedToGroups: note.groupPermissions.map((noteGroupPermission) => ({ + sharedToGroups: groupPermissions.map((noteGroupPermission) => ({ group: this.groupsService.toGroupDto(noteGroupPermission.group), canEdit: noteGroupPermission.canEdit, })), @@ -389,14 +392,16 @@ export class NotesService { const updateUser = await this.calculateUpdateUser(note); return { id: note.publicId, - aliases: note.aliases.map((alias) => alias.name), - primaryAlias: getPrimaryAlias(note) ?? null, + aliases: await Promise.all( + (await note.aliases).map((alias) => alias.name), + ), + primaryAlias: (await getPrimaryAlias(note)) ?? null, title: note.title ?? '', createTime: (await this.getFirstRevision(note)).createdAt, description: note.description ?? '', editedBy: (await this.getAuthorUsers(note)).map((user) => user.username), - permissions: this.toNotePermissionsDto(note), - tags: this.toTagList(note), + permissions: await this.toNotePermissionsDto(note), + tags: await this.toTagList(note), updateTime: (await this.getLatestRevision(note)).createdAt, updateUser: updateUser ? this.usersService.toUserDto(updateUser) : null, viewCount: note.viewCount, diff --git a/src/notes/utils.spec.ts b/src/notes/utils.spec.ts index c84f6cc76..38c9ff35b 100644 --- a/src/notes/utils.spec.ts +++ b/src/notes/utils.spec.ts @@ -29,12 +29,12 @@ describe('getPrimaryAlias', () => { const user = User.create('hardcoded', 'Testy') as User; note = Note.create(user, alias) as Note; }); - it('finds correct primary alias', () => { - note.aliases.push(Alias.create('annother', note, false) as Alias); - expect(getPrimaryAlias(note)).toEqual(alias); + it('finds correct primary alias', async () => { + (await note.aliases).push(Alias.create('annother', note, false) as Alias); + expect(await getPrimaryAlias(note)).toEqual(alias); }); - it('returns undefined if there is no alias', () => { - note.aliases[0].primary = false; - expect(getPrimaryAlias(note)).toEqual(undefined); + it('returns undefined if there is no alias', async () => { + (await note.aliases)[0].primary = false; + expect(await getPrimaryAlias(note)).toEqual(undefined); }); }); diff --git a/src/notes/utils.ts b/src/notes/utils.ts index 7f6229fe7..ed87d1cfc 100644 --- a/src/notes/utils.ts +++ b/src/notes/utils.ts @@ -22,8 +22,8 @@ export function generatePublicId(): string { * Extract the primary alias from a aliases of a note * @param {Note} note - the note from which the primary alias should be extracted */ -export function getPrimaryAlias(note: Note): string | undefined { - const listWithPrimaryAlias = note.aliases.filter( +export async function getPrimaryAlias(note: Note): Promise { + const listWithPrimaryAlias = (await note.aliases).filter( (alias: Alias) => alias.primary, ); if (listWithPrimaryAlias.length !== 1) { diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 3bf1833d7..0e60e2eb6 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -30,6 +30,7 @@ import { GuestPermission, PermissionsService } from './permissions.service'; describe('PermissionsService', () => { let permissionsService: PermissionsService; + let notes: Note[]; beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -73,6 +74,7 @@ describe('PermissionsService', () => { .useValue({}) .compile(); permissionsService = module.get(PermissionsService); + notes = await createNoteUserPermissionNotes(); }); // The two users we test with: @@ -87,16 +89,16 @@ describe('PermissionsService', () => { function createNote(owner: User): Note { const note = {} as Note; - note.userPermissions = []; - note.groupPermissions = []; - note.owner = owner; + note.userPermissions = Promise.resolve([]); + note.groupPermissions = Promise.resolve([]); + note.owner = Promise.resolve(owner); return note; } /* * Creates the permission objects for UserPermission for two users with write and with out write permission */ - function createNoteUserPermissionNotes(): Note[] { + async function createNoteUserPermissionNotes(): Promise { const note0 = createNote(user1); const note1 = createNote(user2); const note2 = createNote(user2); @@ -116,23 +118,23 @@ describe('PermissionsService', () => { noteUserPermission4.user = user2; noteUserPermission4.canEdit = true; - note1.userPermissions.push(noteUserPermission1); + (await note1.userPermissions).push(noteUserPermission1); - note2.userPermissions.push(noteUserPermission1); - note2.userPermissions.push(noteUserPermission2); + (await note2.userPermissions).push(noteUserPermission1); + (await note2.userPermissions).push(noteUserPermission2); - note3.userPermissions.push(noteUserPermission2); - note3.userPermissions.push(noteUserPermission1); + (await note3.userPermissions).push(noteUserPermission2); + (await note3.userPermissions).push(noteUserPermission1); - note4.userPermissions.push(noteUserPermission3); + (await note4.userPermissions).push(noteUserPermission3); - note5.userPermissions.push(noteUserPermission3); - note5.userPermissions.push(noteUserPermission4); + (await note5.userPermissions).push(noteUserPermission3); + (await note5.userPermissions).push(noteUserPermission4); - note6.userPermissions.push(noteUserPermission4); - note6.userPermissions.push(noteUserPermission3); + (await note6.userPermissions).push(noteUserPermission4); + (await note6.userPermissions).push(noteUserPermission3); - note7.userPermissions.push(noteUserPermission2); + (await note7.userPermissions).push(noteUserPermission2); const everybody = {} as Group; everybody.name = SpecialGroup.EVERYONE; @@ -143,7 +145,9 @@ describe('PermissionsService', () => { noteGroupPermissionRead.group = everybody; noteGroupPermissionRead.canEdit = false; noteGroupPermissionRead.note = noteEverybodyRead; - noteEverybodyRead.groupPermissions = [noteGroupPermissionRead]; + noteEverybodyRead.groupPermissions = Promise.resolve([ + noteGroupPermissionRead, + ]); const noteEverybodyWrite = createNote(user1); @@ -151,7 +155,9 @@ describe('PermissionsService', () => { noteGroupPermissionWrite.group = everybody; noteGroupPermissionWrite.canEdit = true; noteGroupPermissionWrite.note = noteEverybodyWrite; - noteEverybodyWrite.groupPermissions = [noteGroupPermissionWrite]; + noteEverybodyWrite.groupPermissions = Promise.resolve([ + noteGroupPermissionWrite, + ]); return [ note0, @@ -167,8 +173,6 @@ describe('PermissionsService', () => { ]; } - const notes = createNoteUserPermissionNotes(); - describe('mayRead works with', () => { it('Owner', async () => { permissionsService.guestPermission = GuestPermission.DENY; @@ -501,7 +505,7 @@ describe('PermissionsService', () => { let i = 0; for (const permission of permissions) { const note = createNote(user2); - note.groupPermissions = permission.permissions; + note.groupPermissions = Promise.resolve(permission.permissions); let permissionString = ''; for (const perm of permission.permissions) { permissionString += ` ${perm.group.name}:${String(perm.canEdit)}`; @@ -550,13 +554,13 @@ describe('PermissionsService', () => { }); describe('isOwner works', () => { - it('for positive case', () => { + it('for positive case', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.isOwner(user1, notes[0])).toBeTruthy(); + expect(await permissionsService.isOwner(user1, notes[0])).toBeTruthy(); }); - it('for negative case', () => { + it('for negative case', async () => { permissionsService.guestPermission = GuestPermission.DENY; - expect(permissionsService.isOwner(user1, notes[1])).toBeFalsy(); + expect(await permissionsService.isOwner(user1, notes[1])).toBeFalsy(); }); }); }); diff --git a/src/permissions/permissions.service.ts b/src/permissions/permissions.service.ts index 5b059ac93..5f3cfd25e 100644 --- a/src/permissions/permissions.service.ts +++ b/src/permissions/permissions.service.ts @@ -22,9 +22,9 @@ export enum GuestPermission { export class PermissionsService { public guestPermission: GuestPermission; // TODO change to configOption async mayRead(user: User | null, note: Note): Promise { - if (this.isOwner(user, note)) return true; + if (await this.isOwner(user, note)) return true; - if (this.hasPermissionUser(user, note, false)) return true; + if (await this.hasPermissionUser(user, note, false)) return true; // noinspection RedundantIfStatementJS if (await this.hasPermissionGroup(user, note, false)) return true; @@ -33,9 +33,9 @@ export class PermissionsService { } async mayWrite(user: User | null, note: Note): Promise { - if (this.isOwner(user, note)) return true; + if (await this.isOwner(user, note)) return true; - if (this.hasPermissionUser(user, note, true)) return true; + if (await this.hasPermissionUser(user, note, true)) return true; // noinspection RedundantIfStatementJS if (await this.hasPermissionGroup(user, note, true)) return true; @@ -58,21 +58,22 @@ export class PermissionsService { return false; } - isOwner(user: User | null, note: Note): boolean { + async isOwner(user: User | null, note: Note): Promise { if (!user) return false; - if (!note.owner) return false; - return note.owner.id === user.id; + const owner = await note.owner; + if (!owner) return false; + return owner.id === user.id; } - private hasPermissionUser( + private async hasPermissionUser( user: User | null, note: Note, wantEdit: boolean, - ): boolean { + ): Promise { if (!user) { return false; } - for (const userPermission of note.userPermissions) { + for (const userPermission of await note.userPermissions) { if ( userPermission.user.id === user.id && (userPermission.canEdit || !wantEdit) @@ -99,7 +100,7 @@ export class PermissionsService { case GuestPermission.READ: guestsAllowed = !wantEdit; } - for (const groupPermission of note.groupPermissions) { + for (const groupPermission of await note.groupPermissions) { if (groupPermission.canEdit || !wantEdit) { // Handle special groups if (groupPermission.group.special) { diff --git a/src/seed.ts b/src/seed.ts index c280cacde..edd56a769 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -76,8 +76,8 @@ createConnection({ const edit = Edit.create(author, 1, 42) as Edit; revision.edits = [edit]; notes[i].revisions = Promise.all([revision]); - notes[i].userPermissions = []; - notes[i].groupPermissions = []; + notes[i].userPermissions = Promise.resolve([]); + notes[i].groupPermissions = Promise.resolve([]); user.ownedNotes = [notes[i]]; await connection.manager.save([ notes[i], @@ -99,7 +99,7 @@ createConnection({ throw new Error('Could not find freshly seeded notes. Aborting.'); } for (const note of foundNotes) { - if (!note.aliases[0]) { + if (!(await note.aliases)[0]) { throw new Error( 'Could not find alias of freshly seeded notes. Aborting.', ); @@ -111,7 +111,7 @@ createConnection({ ); } for (const note of foundNotes) { - console.log(`Created Note '${note.aliases[0].name ?? ''}'`); + console.log(`Created Note '${(await note.aliases)[0].name ?? ''}'`); } for (const user of foundUsers) { for (const note of foundNotes) { @@ -119,7 +119,7 @@ createConnection({ await connection.manager.save(historyEntry); console.log( `Created HistoryEntry for user '${user.username}' and note '${ - note.aliases[0].name ?? '' + (await note.aliases)[0].name ?? '' }'`, ); } diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index de07bee27..0da6cf547 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -69,7 +69,7 @@ describe('History', () => { note, user, ); - const entryDto = testSetup.historyService.toHistoryEntryDto(entry); + const entryDto = await testSetup.historyService.toHistoryEntryDto(entry); const response = await agent .get('/api/private/me/history') .expect('Content-Type', /json/) @@ -92,7 +92,7 @@ describe('History', () => { const pinStatus = true; const lastVisited = new Date('2020-12-01 12:23:34'); const postEntryDto = new HistoryEntryImportDto(); - postEntryDto.note = note2.aliases.filter( + postEntryDto.note = (await note2.aliases).filter( (alias) => alias.primary, )[0].name; postEntryDto.pinStatus = pinStatus; @@ -104,13 +104,15 @@ describe('History', () => { .expect(201); const userEntries = await testSetup.historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); - expect(userEntries[0].note.aliases[0].name).toEqual( - note2.aliases[0].name, + expect((await userEntries[0].note.aliases)[0].name).toEqual( + (await note2.aliases)[0].name, ); - expect(userEntries[0].note.aliases[0].primary).toEqual( - note2.aliases[0].primary, + expect((await userEntries[0].note.aliases)[0].primary).toEqual( + (await note2.aliases)[0].primary, + ); + expect((await userEntries[0].note.aliases)[0].id).toEqual( + (await note2.aliases)[0].id, ); - expect(userEntries[0].note.aliases[0].id).toEqual(note2.aliases[0].id); expect(userEntries[0].user.username).toEqual(user.username); expect(userEntries[0].pinStatus).toEqual(pinStatus); expect(userEntries[0].updatedAt).toEqual(lastVisited); @@ -129,7 +131,7 @@ describe('History', () => { pinStatus = !previousHistory[0].pinStatus; lastVisited = new Date('2020-12-01 23:34:45'); postEntryDto = new HistoryEntryImportDto(); - postEntryDto.note = note2.aliases.filter( + postEntryDto.note = (await note2.aliases).filter( (alias) => alias.primary, )[0].name; postEntryDto.pinStatus = pinStatus; @@ -188,7 +190,8 @@ describe('History', () => { user, ); expect(entry.pinStatus).toBeFalsy(); - const alias = entry.note.aliases.filter((alias) => alias.primary)[0].name; + const alias = (await entry.note.aliases).filter((alias) => alias.primary)[0] + .name; await agent .put(`/api/private/me/history/${alias || 'undefined'}`) .send({ pinStatus: true }) @@ -201,15 +204,16 @@ describe('History', () => { it('DELETE /me/history/:note', async () => { const entry = await historyService.updateHistoryEntryTimestamp(note2, user); - const alias = entry.note.aliases.filter((alias) => alias.primary)[0].name; + const alias = (await entry.note.aliases).filter((alias) => alias.primary)[0] + .name; const entry2 = await historyService.updateHistoryEntryTimestamp(note, user); - const entryDto = historyService.toHistoryEntryDto(entry2); + const entryDto = await historyService.toHistoryEntryDto(entry2); await agent .delete(`/api/private/me/history/${alias || 'undefined'}`) .expect(200); const userEntries = await historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); - const userEntryDto = historyService.toHistoryEntryDto(userEntries[0]); + const userEntryDto = await historyService.toHistoryEntryDto(userEntries[0]); expect(userEntryDto.identifier).toEqual(entryDto.identifier); expect(userEntryDto.title).toEqual(entryDto.title); expect(userEntryDto.tags).toEqual(entryDto.tags); diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index 2e02fc6aa..3e947933b 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -51,8 +51,9 @@ describe('Me', () => { .expect(200); const history: HistoryEntryDto[] = response.body; expect(history.length).toEqual(1); - const historyDto = - testSetup.historyService.toHistoryEntryDto(createdHistoryEntry); + const historyDto = await testSetup.historyService.toHistoryEntryDto( + createdHistoryEntry, + ); for (const historyEntry of history) { expect(historyEntry.identifier).toEqual(historyDto.identifier); expect(historyEntry.title).toEqual(historyDto.title); @@ -75,8 +76,9 @@ describe('Me', () => { .expect('Content-Type', /json/) .expect(200); const historyEntry: HistoryEntryDto = response.body; - const historyEntryDto = - testSetup.historyService.toHistoryEntryDto(createdHistoryEntry); + const historyEntryDto = await testSetup.historyService.toHistoryEntryDto( + createdHistoryEntry, + ); expect(historyEntry.identifier).toEqual(historyEntryDto.identifier); expect(historyEntry.title).toEqual(historyEntryDto.title); expect(historyEntry.tags).toEqual(historyEntryDto.tags); @@ -109,8 +111,12 @@ describe('Me', () => { expect(historyEntry.pinStatus).toEqual(true); let theEntry: HistoryEntryDto; for (const entry of history) { - if (entry.note.aliases.find((element) => element.name === noteName)) { - theEntry = testSetup.historyService.toHistoryEntryDto(entry); + if ( + (await entry.note.aliases).find( + (element) => element.name === noteName, + ) + ) { + theEntry = await testSetup.historyService.toHistoryEntryDto(entry); } } expect(theEntry.pinStatus).toEqual(true); @@ -134,7 +140,11 @@ describe('Me', () => { expect(response.body).toEqual({}); const history = await testSetup.historyService.getEntriesByUser(user); for (const entry of history) { - if (entry.note.aliases.find((element) => element.name === noteName)) { + if ( + (await entry.note.aliases).find( + (element) => element.name === noteName, + ) + ) { throw new Error('Deleted history entry still in history'); } } diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index bc8093e6d..d5dd441bc 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -200,16 +200,16 @@ describe('Notes', () => { updateNotePermission, ); const updatedNote = await testSetup.notesService.getNoteByIdOrAlias( - note.aliases.filter((alias) => alias.primary)[0].name, + (await note.aliases).filter((alias) => alias.primary)[0].name, ); - expect(updatedNote.userPermissions).toHaveLength(1); - expect(updatedNote.userPermissions[0].canEdit).toEqual( + expect(await updatedNote.userPermissions).toHaveLength(1); + expect((await updatedNote.userPermissions)[0].canEdit).toEqual( updateNotePermission.sharedToUsers[0].canEdit, ); - expect(updatedNote.userPermissions[0].user.username).toEqual( + expect((await updatedNote.userPermissions)[0].user.username).toEqual( user.username, ); - expect(updatedNote.groupPermissions).toHaveLength(0); + expect(await updatedNote.groupPermissions).toHaveLength(0); await request(testSetup.app.getHttpServer()) .delete('/api/v2/notes/test3') .expect(204); From f164e8408121ac41157656e5fc21fd842135495b Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 19:06:05 +0100 Subject: [PATCH 08/14] refactor(tag): lazy-load relations Signed-off-by: David Mehren --- src/notes/notes.service.spec.ts | 6 +++--- src/notes/tag.entity.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index d4a5154e6..0e38c41e7 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -657,7 +657,7 @@ describe('NotesService', () => { { id: 1, name: 'testTag', - notes: [note], + notes: Promise.resolve([note]), }, ]); const tagList = await service.toTagList(note); @@ -764,7 +764,7 @@ describe('NotesService', () => { { id: 1, name: 'testTag', - notes: [note], + notes: Promise.resolve([note]), }, ]); note.viewCount = 1337; @@ -866,7 +866,7 @@ describe('NotesService', () => { { id: 1, name: 'testTag', - notes: [note], + notes: Promise.resolve([note]), }, ]); note.viewCount = 1337; diff --git a/src/notes/tag.entity.ts b/src/notes/tag.entity.ts index 04f787b31..f054cdca5 100644 --- a/src/notes/tag.entity.ts +++ b/src/notes/tag.entity.ts @@ -18,5 +18,5 @@ export class Tag { name: string; @ManyToMany((_) => Note, (note) => note.tags) - notes: Note[]; + notes: Promise; } From c5c7307552f301329977ca1615445d615faa83a2 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 19:32:31 +0100 Subject: [PATCH 09/14] docs(permissions): document why we can't lazy-load Signed-off-by: David Mehren --- src/permissions/note-group-permission.entity.ts | 7 +++++++ src/permissions/note-user-permission.entity.ts | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/permissions/note-group-permission.entity.ts b/src/permissions/note-group-permission.entity.ts index 2c0eb779b..52e18e662 100644 --- a/src/permissions/note-group-permission.entity.ts +++ b/src/permissions/note-group-permission.entity.ts @@ -10,6 +10,13 @@ import { Note } from '../notes/note.entity'; @Entity() export class NoteGroupPermission { + /** + * The `group` and `note` properties cannot be converted to promises + * (to lazy-load them), as TypeORM gets confused about lazy composite + * primary keys. + * See https://github.com/typeorm/typeorm/issues/6908 + */ + @ManyToOne((_) => Group, { primary: true, onDelete: 'CASCADE', // This deletes the NoteGroupPermission, when the associated Group is deleted diff --git a/src/permissions/note-user-permission.entity.ts b/src/permissions/note-user-permission.entity.ts index c0e128f9f..b60593a4b 100644 --- a/src/permissions/note-user-permission.entity.ts +++ b/src/permissions/note-user-permission.entity.ts @@ -10,6 +10,13 @@ import { User } from '../users/user.entity'; @Entity() export class NoteUserPermission { + /** + * The `user` and `note` properties cannot be converted to promises + * (to lazy-load them), as TypeORM gets confused about lazy composite + * primary keys. + * See https://github.com/typeorm/typeorm/issues/6908 + */ + @ManyToOne((_) => User, { primary: true, onDelete: 'CASCADE', // This deletes the NoteUserPermission, when the associated Note is deleted From e73bd7c030b69c2d9562f80da35746b4228d3780 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 19:58:01 +0100 Subject: [PATCH 10/14] refactor(edit): lazy-load relations Signed-off-by: David Mehren --- src/api/private/notes/notes.controller.ts | 8 ++++++-- src/api/utils/permissions.guard.ts | 6 +++--- src/notes/notes.service.spec.ts | 16 ++++++++-------- src/notes/notes.service.ts | 8 +++++--- src/revisions/edit.entity.ts | 8 ++++---- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/api/private/notes/notes.controller.ts b/src/api/private/notes/notes.controller.ts index fd02e839b..1c9997530 100644 --- a/src/api/private/notes/notes.controller.ts +++ b/src/api/private/notes/notes.controller.ts @@ -14,10 +14,14 @@ 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/utils/permissions.guard.ts b/src/api/utils/permissions.guard.ts index bdbe95ee5..96116db00 100644 --- a/src/api/utils/permissions.guard.ts +++ b/src/api/utils/permissions.guard.ts @@ -55,11 +55,11 @@ export class PermissionsGuard implements CanActivate { const note = await getNote(this.noteService, noteIdOrAlias); switch (permissions[0]) { case Permission.READ: - return this.permissionsService.mayRead(user, note); + return await this.permissionsService.mayRead(user, note); case Permission.WRITE: - return this.permissionsService.mayWrite(user, note); + return await this.permissionsService.mayWrite(user, note); case Permission.OWNER: - return this.permissionsService.isOwner(user, note); + return await this.permissionsService.isOwner(user, note); } return false; } diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 0e38c41e7..741bdb5f4 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -713,18 +713,18 @@ describe('NotesService', () => { const revisions = await note.revisions; revisions[0].edits = [ { - revisions: revisions, + revisions: Promise.resolve(revisions), startPos: 0, endPos: 1, updatedAt: new Date(1549312452000), - author: author, + author: Promise.resolve(author), } as Edit, { - revisions: revisions, + revisions: Promise.resolve(revisions), startPos: 0, endPos: 1, updatedAt: new Date(1549312452001), - author: author, + author: Promise.resolve(author), } as Edit, ]; revisions[0].createdAt = new Date(1549312452000); @@ -812,18 +812,18 @@ describe('NotesService', () => { const revisions = await note.revisions; revisions[0].edits = [ { - revisions: revisions, + revisions: Promise.resolve(revisions), startPos: 0, endPos: 1, updatedAt: new Date(1549312452000), - author: author, + author: Promise.resolve(author), } as Edit, { - revisions: revisions, + revisions: Promise.resolve(revisions), startPos: 0, endPos: 1, updatedAt: new Date(1549312452001), - author: author, + author: Promise.resolve(author), } as Edit, ]; revisions[0].createdAt = new Date(1549312452000); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 51a24d07e..16d98e5cb 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -343,9 +343,11 @@ export class NotesService { if (lastRevision && lastRevision.edits) { // Sort the last Revisions Edits by their updatedAt Date to get the latest one // the user of that Edit is the updateUser - return await lastRevision.edits.sort( - (a, b) => b.updatedAt.getTime() - a.updatedAt.getTime(), - )[0].author.user; + return await ( + await lastRevision.edits.sort( + (a, b) => b.updatedAt.getTime() - a.updatedAt.getTime(), + )[0].author + ).user; } // If there are no Edits, the owner is the updateUser return await note.owner; diff --git a/src/revisions/edit.entity.ts b/src/revisions/edit.entity.ts index ac2a188fe..349e6d54f 100644 --- a/src/revisions/edit.entity.ts +++ b/src/revisions/edit.entity.ts @@ -28,13 +28,13 @@ export class Edit { * Revisions this edit appears in */ @ManyToMany((_) => Revision, (revision) => revision.edits) - revisions: Revision[]; + revisions: Promise; /** * Author that created the change */ @ManyToOne(() => Author, (author) => author.edits) - author: Author; + author: Promise; @Column() startPos: number; @@ -57,8 +57,8 @@ export class Edit { endPos: number, ): Omit { const newEdit = new Edit(); - newEdit.revisions = []; - newEdit.author = author; + newEdit.revisions = Promise.resolve([]); + newEdit.author = Promise.resolve(author); newEdit.startPos = startPos; newEdit.endPos = endPos; return newEdit; From 2da6faa4b42dcf1fa19751559fcb11619acd00e6 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 21:38:36 +0100 Subject: [PATCH 11/14] refactor(revision): lazy-load relations Signed-off-by: David Mehren --- src/notes/notes.service.spec.ts | 8 ++++---- src/notes/notes.service.ts | 5 +++-- src/revisions/revision.entity.ts | 8 ++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 741bdb5f4..76ec8e1bc 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -711,7 +711,7 @@ describe('NotesService', () => { .mockImplementation(async (note: Note): Promise => note); const note = await service.createNote(content, null); const revisions = await note.revisions; - revisions[0].edits = [ + revisions[0].edits = Promise.resolve([ { revisions: Promise.resolve(revisions), startPos: 0, @@ -726,7 +726,7 @@ describe('NotesService', () => { updatedAt: new Date(1549312452001), author: Promise.resolve(author), } as Edit, - ]; + ]); revisions[0].createdAt = new Date(1549312452000); jest.spyOn(revisionRepo, 'findOne').mockResolvedValue(revisions[0]); const createQueryBuilder = { @@ -810,7 +810,7 @@ describe('NotesService', () => { .mockImplementation(async (note: Note): Promise => note); const note = await service.createNote(content, null); const revisions = await note.revisions; - revisions[0].edits = [ + revisions[0].edits = Promise.resolve([ { revisions: Promise.resolve(revisions), startPos: 0, @@ -825,7 +825,7 @@ describe('NotesService', () => { updatedAt: new Date(1549312452001), author: Promise.resolve(author), } as Edit, - ]; + ]); revisions[0].createdAt = new Date(1549312452000); jest .spyOn(revisionRepo, 'findOne') diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 16d98e5cb..0eee31428 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -340,11 +340,12 @@ export class NotesService { */ async calculateUpdateUser(note: Note): Promise { const lastRevision = await this.getLatestRevision(note); - if (lastRevision && lastRevision.edits) { + const edits = await lastRevision.edits; + if (edits.length > 0) { // Sort the last Revisions Edits by their updatedAt Date to get the latest one // the user of that Edit is the updateUser return await ( - await lastRevision.edits.sort( + await edits.sort( (a, b) => b.updatedAt.getTime() - a.updatedAt.getTime(), )[0].author ).user; diff --git a/src/revisions/revision.entity.ts b/src/revisions/revision.entity.ts index 749a6e494..9858889ef 100644 --- a/src/revisions/revision.entity.ts +++ b/src/revisions/revision.entity.ts @@ -58,14 +58,14 @@ export class Revision { * Note this revision belongs to. */ @ManyToOne((_) => Note, (note) => note.revisions, { onDelete: 'CASCADE' }) - note: Note; + note: Promise; /** * All edit objects which are used in the revision. */ @ManyToMany((_) => Edit, (edit) => edit.revisions) @JoinTable() - edits: Edit[]; + edits: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -79,8 +79,8 @@ export class Revision { newRevision.patch = patch; newRevision.content = content; newRevision.length = content.length; - newRevision.note = note; - newRevision.edits = []; + newRevision.note = Promise.resolve(note); + newRevision.edits = Promise.resolve([]); return newRevision; } } From 4483d2b898f6459e12524c54fb4bf906777e2fcb Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 21:49:29 +0100 Subject: [PATCH 12/14] refactor(session): lazy-load relations Signed-off-by: David Mehren --- src/seed.ts | 2 +- src/users/session.entity.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/seed.ts b/src/seed.ts index edd56a769..8931919c6 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -74,7 +74,7 @@ createConnection({ notes[i], ) as Revision; const edit = Edit.create(author, 1, 42) as Edit; - revision.edits = [edit]; + revision.edits = Promise.resolve([edit]); notes[i].revisions = Promise.all([revision]); notes[i].userPermissions = Promise.resolve([]); notes[i].groupPermissions = Promise.resolve([]); diff --git a/src/users/session.entity.ts b/src/users/session.entity.ts index b8e466ef8..f4dc6a51f 100644 --- a/src/users/session.entity.ts +++ b/src/users/session.entity.ts @@ -21,5 +21,5 @@ export class Session implements ISession { public json = ''; @ManyToOne(() => Author, (author) => author.sessions) - author: Author; + author: Promise; } From 977ed4b9fafe3a4966c7402b516d4dc1a18201ca Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 22:03:41 +0100 Subject: [PATCH 13/14] refactor(user): lazy-load relations Signed-off-by: David Mehren --- src/auth/auth.service.spec.ts | 4 ++-- src/auth/auth.service.ts | 4 ++-- src/seed.ts | 2 +- src/users/user.entity.ts | 24 ++++++++++++------------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 813017000..f3f425f39 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -166,7 +166,7 @@ describe('AuthService', () => { const accessTokenHash = await hashPassword(token); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce({ ...user, - authTokens: [authToken], + authTokens: Promise.resolve([authToken]), }); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValue({ ...authToken, @@ -183,7 +183,7 @@ describe('AuthService', () => { ); expect(userByToken).toEqual({ ...user, - authTokens: [authToken], + authTokens: Promise.resolve([authToken]), }); }); describe('fails:', () => { diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 90bb67094..0d61cf390 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -65,9 +65,9 @@ export class AuthService { identifier: string, validUntil: TimestampMillis, ): Promise { - user.authTokens = await this.getTokensByUser(user); + user.authTokens = this.getTokensByUser(user); - if (user.authTokens.length >= 200) { + if ((await user.authTokens).length >= 200) { // This is a very high ceiling unlikely to hinder legitimate usage, // but should prevent possible attack vectors throw new TooManyTokensError( diff --git a/src/seed.ts b/src/seed.ts index 8931919c6..d601c378f 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -78,7 +78,7 @@ createConnection({ notes[i].revisions = Promise.all([revision]); notes[i].userPermissions = Promise.resolve([]); notes[i].groupPermissions = Promise.resolve([]); - user.ownedNotes = [notes[i]]; + user.ownedNotes = Promise.resolve([notes[i]]); await connection.manager.save([ notes[i], user, diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index e965d63fb..4500ce2c8 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -53,25 +53,25 @@ export class User { email: string | null; @OneToMany((_) => Note, (note) => note.owner) - ownedNotes: Note[]; + ownedNotes: Promise; @OneToMany((_) => AuthToken, (authToken) => authToken.user) - authTokens: AuthToken[]; + authTokens: Promise; @OneToMany((_) => Identity, (identity) => identity.user) identities: Promise; @ManyToMany((_) => Group, (group) => group.members) - groups: Group[]; + groups: Promise; @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) - historyEntries: HistoryEntry[]; + historyEntries: Promise; @OneToMany((_) => MediaUpload, (mediaUpload) => mediaUpload.user) - mediaUploads: MediaUpload[]; + mediaUploads: Promise; @OneToMany(() => Author, (author) => author.user) - authors: Author[]; + authors: Promise; // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} @@ -85,13 +85,13 @@ export class User { newUser.displayName = displayName; newUser.photo = null; newUser.email = null; - newUser.ownedNotes = []; - newUser.authTokens = []; + newUser.ownedNotes = Promise.resolve([]); + newUser.authTokens = Promise.resolve([]); newUser.identities = Promise.resolve([]); - newUser.groups = []; - newUser.historyEntries = []; - newUser.mediaUploads = []; - newUser.authors = []; + newUser.groups = Promise.resolve([]); + newUser.historyEntries = Promise.resolve([]); + newUser.mediaUploads = Promise.resolve([]); + newUser.authors = Promise.resolve([]); return newUser; } } From e21b5e695d445daaa74b04d953cf08d9cfa33b67 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 5 Dec 2021 22:10:59 +0100 Subject: [PATCH 14/14] refactor(identity): lazy-load relations Signed-off-by: David Mehren --- src/identity/identity.entity.ts | 4 ++-- src/identity/identity.service.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/identity/identity.entity.ts b/src/identity/identity.entity.ts index 73d0964ee..529dd16e2 100644 --- a/src/identity/identity.entity.ts +++ b/src/identity/identity.entity.ts @@ -32,7 +32,7 @@ export class Identity { @ManyToOne((_) => User, (user) => user.identities, { onDelete: 'CASCADE', // This deletes the Identity, when the associated User is deleted }) - user: User; + user: Promise; /** * The ProviderType of the identity @@ -103,7 +103,7 @@ export class Identity { syncSource: boolean, ): Omit { const newIdentity = new Identity(); - newIdentity.user = user; + newIdentity.user = Promise.resolve(user); newIdentity.providerType = providerType; newIdentity.providerName = null; newIdentity.syncSource = syncSource; diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts index c7c44ac41..c93b4b797 100644 --- a/src/identity/identity.service.spec.ts +++ b/src/identity/identity.service.spec.ts @@ -60,7 +60,7 @@ describe('IdentityService', () => { await checkPassword(password, identity.passwordHash ?? '').then( (result) => expect(result).toBeTruthy(), ); - expect(identity.user).toEqual(user); + expect(await identity.user).toEqual(user); }); }); @@ -83,7 +83,7 @@ describe('IdentityService', () => { await checkPassword(newPassword, identity.passwordHash ?? '').then( (result) => expect(result).toBeTruthy(), ); - expect(identity.user).toEqual(user); + expect(await identity.user).toEqual(user); }); it('fails, when user has no local identity', async () => { user.identities = Promise.resolve([]);