From facdc456cd7d3999a3becf1d19331e8f2ffa4742 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Thu, 18 Nov 2021 18:47:12 +0100 Subject: [PATCH] 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, }; }