From 8e31f3a39368389be11f487c26b6ce5563ac3cb7 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sat, 29 Jan 2022 18:57:44 +0100 Subject: [PATCH] refactor(api/private/media): return MediaUpload object instead of url This ensures the private POST /media API behaves in the same way as /me/media Signed-off-by: David Mehren --- src/api/private/media/media.controller.ts | 10 +++++----- src/api/public/media/media.controller.ts | 4 ++-- src/media/media.service.spec.ts | 4 ++-- src/media/media.service.ts | 9 ++++++--- test/private-api/me.e2e-spec.ts | 16 ++++++++++------ test/private-api/media.e2e-spec.ts | 6 +++--- test/private-api/notes.e2e-spec.ts | 16 ++++++++-------- test/public-api/me.e2e-spec.ts | 8 ++++---- test/public-api/media.e2e-spec.ts | 4 ++-- test/public-api/notes.e2e-spec.ts | 16 ++++++++-------- 10 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/api/private/media/media.controller.ts b/src/api/private/media/media.controller.ts index 815aed235..58c715b11 100644 --- a/src/api/private/media/media.controller.ts +++ b/src/api/private/media/media.controller.ts @@ -19,7 +19,7 @@ import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger'; import { PermissionError } from '../../../errors/errors'; import { SessionGuard } from '../../../identity/session.guard'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; -import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; +import { MediaUploadDto } from '../../../media/media-upload.dto'; import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; import { Note } from '../../../notes/note.entity'; @@ -63,7 +63,7 @@ export class MediaController { { code: 201, description: 'The file was uploaded successfully', - dto: MediaUploadUrlDto, + dto: MediaUploadDto, }, 400, 403, @@ -74,15 +74,15 @@ export class MediaController { @UploadedFile() file: MulterFile, @Headers('HedgeDoc-Note') noteId: string, @RequestUser() user: User, - ): Promise { + ): Promise { // TODO: Move getting the Note object into a decorator const note: Note = await this.noteService.getNoteByIdOrAlias(noteId); this.logger.debug( `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`, 'uploadMedia', ); - const url = await this.mediaService.saveFile(file.buffer, user, note); - return this.mediaService.toMediaUploadUrlDto(url); + const upload = await this.mediaService.saveFile(file.buffer, user, note); + return await this.mediaService.toMediaUploadDto(upload); } @Delete(':filename') diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index 01e0e6cc0..a7f9d4ad3 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -88,8 +88,8 @@ export class MediaController { `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`, 'uploadMedia', ); - const url = await this.mediaService.saveFile(file.buffer, user, note); - return this.mediaService.toMediaUploadUrlDto(url); + const upload = await this.mediaService.saveFile(file.buffer, user, note); + return this.mediaService.toMediaUploadUrlDto(upload.fileUrl); } @Delete(':filename') diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 1b68ed47d..9b10181fc 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -143,8 +143,8 @@ describe('MediaService', () => { return [fileName, null]; }, ); - const url = await service.saveFile(testImage, user, note); - expect(url).toEqual(fileId); + const upload = await service.saveFile(testImage, user, note); + expect(upload.fileUrl).toEqual(fileId); }); describe('fails:', () => { diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 23ade8417..b01339bf3 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -77,7 +77,11 @@ export class MediaService { * @throws {NotInDBError} - the note or user is not in the database * @throws {MediaBackendError} - there was an error saving the file */ - async saveFile(fileBuffer: Buffer, user: User, note: Note): Promise { + async saveFile( + fileBuffer: Buffer, + user: User, + note: Note, + ): Promise { this.logger.debug( `Saving file for note '${note.id}' and user '${user.username}'`, 'saveFile', @@ -102,8 +106,7 @@ export class MediaService { url, ); mediaUpload.backendData = backendData; - await this.mediaUploadRepository.save(mediaUpload); - return url; + return await this.mediaUploadRepository.save(mediaUpload); } /** diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts index 14801dc0b..f737afb56 100644 --- a/test/private-api/me.e2e-spec.ts +++ b/test/private-api/me.e2e-spec.ts @@ -69,16 +69,16 @@ describe('Me', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const imageUrls = []; imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note1), + (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note1), + (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note2), + (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note2), + (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl, ); const response = await agent @@ -112,12 +112,16 @@ describe('Me', () => { it('DELETE /me', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await testSetup.mediaService.saveFile(testImage, user, note1); + const upload = await testSetup.mediaService.saveFile( + testImage, + user, + note1, + ); const dbUser = await testSetup.userService.getUserByUsername('hardcoded'); expect(dbUser).toBeInstanceOf(User); const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser); expect(mediaUploads).toHaveLength(1); - expect(mediaUploads[0].fileUrl).toEqual(url0); + expect(mediaUploads[0].fileUrl).toEqual(upload.fileUrl); await agent.delete('/api/private/me').expect(204); await expect( testSetup.userService.getUserByUsername('hardcoded'), diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 4f9160d49..73b05d8d5 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/test/private-api/media.e2e-spec.ts @@ -64,7 +64,7 @@ describe('Media', () => { .set('HedgeDoc-Note', 'test_upload_media') .expect('Content-Type', /json/) .expect(201); - const path: string = uploadResponse.body.link; + const path: string = uploadResponse.body.url; const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const downloadResponse = await agent.get(path); expect(downloadResponse.body).toEqual(testImage); @@ -117,12 +117,12 @@ describe('Media', () => { 'test_delete_media', ); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); - const url = await testSetup.mediaService.saveFile( + const upload = await testSetup.mediaService.saveFile( testImage, user, testNote, ); - const filename = url.split('/').pop() || ''; + const filename = upload.fileUrl.split('/').pop() || ''; await agent.delete('/api/private/media/' + filename).expect(204); }); }); diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index 841652900..98da7ecf6 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/test/private-api/notes.e2e-spec.ts @@ -161,7 +161,7 @@ describe('Notes', () => { user, noteId, ); - const url = await testSetup.mediaService.saveFile( + const upload = await testSetup.mediaService.saveFile( testImage, user, note, @@ -182,7 +182,7 @@ describe('Notes', () => { await testSetup.mediaService.listUploadsByUser(user), ).toHaveLength(1); // Remove /upload/ from path as we just need the filename. - const fileName = url.replace('/uploads/', ''); + const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); await fs.rmdir(uploadPath); @@ -308,12 +308,12 @@ describe('Notes', () => { expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); - const url0 = await testSetup.mediaService.saveFile( + const upload0 = await testSetup.mediaService.saveFile( testImage, user, note1, ); - const url1 = await testSetup.mediaService.saveFile( + const upload1 = await testSetup.mediaService.saveFile( testImage, user, note2, @@ -324,10 +324,10 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); - expect(responseAfter.body[0].url).toEqual(url0); - expect(responseAfter.body[0].url).not.toEqual(url1); - for (const fileUrl of [url0, url1]) { - const fileName = fileUrl.replace('/uploads/', ''); + expect(responseAfter.body[0].url).toEqual(upload0.fileUrl); + expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl); + for (const upload of [upload0, upload1]) { + const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); } diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index a498f8ed8..7a149cc43 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -204,16 +204,16 @@ describe('Me', () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const imageUrls = []; imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note1), + (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note1), + (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note2), + (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl, ); imageUrls.push( - await testSetup.mediaService.saveFile(testImage, user, note2), + (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl, ); const response = await request(httpServer) diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index f68f11c6f..857b8ee8e 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -108,12 +108,12 @@ describe('Media', () => { it('DELETE /media/{filename}', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url = await testSetup.mediaService.saveFile( + const upload = await testSetup.mediaService.saveFile( testImage, user, testNote, ); - const filename = url.split('/').pop() || ''; + const filename = upload.fileUrl.split('/').pop() || ''; await request(testSetup.app.getHttpServer()) .delete('/api/v2/media/' + filename) .expect(204); diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 7c6554d6a..a3389eb21 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -159,7 +159,7 @@ describe('Notes', () => { user, noteId, ); - const url = await testSetup.mediaService.saveFile( + const upload = await testSetup.mediaService.saveFile( testImage, user, note, @@ -180,7 +180,7 @@ describe('Notes', () => { await testSetup.mediaService.listUploadsByUser(user), ).toHaveLength(1); // Remove /upload/ from path as we just need the filename. - const fileName = url.replace('/uploads/', ''); + const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); }); @@ -425,12 +425,12 @@ describe('Notes', () => { expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await testSetup.mediaService.saveFile( + const upload0 = await testSetup.mediaService.saveFile( testImage, user, note1, ); - const url1 = await testSetup.mediaService.saveFile( + const upload1 = await testSetup.mediaService.saveFile( testImage, user, note2, @@ -441,10 +441,10 @@ describe('Notes', () => { .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); - expect(responseAfter.body[0].url).toEqual(url0); - expect(responseAfter.body[0].url).not.toEqual(url1); - for (const fileUrl of [url0, url1]) { - const fileName = fileUrl.replace('/uploads/', ''); + expect(responseAfter.body[0].url).toEqual(upload0.fileUrl); + expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl); + for (const upload of [upload0, upload1]) { + const fileName = upload.fileUrl.replace('/uploads/', ''); // delete the file afterwards await fs.unlink(join(uploadPath, fileName)); }