MediaService: Refactor saveFile

The function now expects a `Note` object instead of a noteId
and a `User` instead of a username to
make it more consistent with other functions.

Signed-off-by: David Mehren <git@herrmehren.de>

src/api/private/media/media.controller.ts

@@ -24,12 +24,18 @@ import { ConsoleLoggerService } from '../../../logger/console-logger.service';
 import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto';
 import { MediaService } from '../../../media/media.service';
 import { MulterFile } from '../../../media/multer-file.interface';
+import { Note } from '../../../notes/note.entity';
+import { NotesService } from '../../../notes/notes.service';
+import { User } from '../../../users/user.entity';
+import { UsersService } from '../../../users/users.service';
 
 @Controller('media')
 export class MediaController {
   constructor(
     private readonly logger: ConsoleLoggerService,
     private mediaService: MediaService,
+    private userService: UsersService,
+    private noteService: NotesService,
   ) {
     this.logger.setContext(MediaController.name);
   }
@@ -42,17 +48,15 @@ export class MediaController {
     @Headers('HedgeDoc-Note') noteId: string,
   ): Promise<MediaUploadUrlDto> {
     // ToDo: Get real userName
-    const username = 'hardcoded';
-    this.logger.debug(
-      `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`,
-      'uploadMedia',
-    );
+    const user: User = await this.userService.getUserByUsername('hardcoded');
     try {
-      const url = await this.mediaService.saveFile(
-        file.buffer,
-        username,
-        noteId,
+      // 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);
     } catch (e) {
       if (e instanceof ClientError || e instanceof NotInDBError) {

src/api/public/media/media.controller.ts

@@ -42,6 +42,8 @@ import { ConsoleLoggerService } from '../../../logger/console-logger.service';
 import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto';
 import { MediaService } from '../../../media/media.service';
 import { MulterFile } from '../../../media/multer-file.interface';
+import { Note } from '../../../notes/note.entity';
+import { NotesService } from '../../../notes/notes.service';
 import { User } from '../../../users/user.entity';
 import {
   forbiddenDescription,
@@ -58,6 +60,7 @@ export class MediaController {
   constructor(
     private readonly logger: ConsoleLoggerService,
     private mediaService: MediaService,
+    private noteService: NotesService,
   ) {
     this.logger.setContext(MediaController.name);
   }
@@ -93,17 +96,14 @@ export class MediaController {
     @UploadedFile() file: MulterFile,
     @Headers('HedgeDoc-Note') noteId: string,
   ): Promise<MediaUploadUrlDto> {
-    const username = user.userName;
-    this.logger.debug(
-      `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`,
-      'uploadMedia',
-    );
     try {
-      const url = await this.mediaService.saveFile(
-        file.buffer,
-        username,
-        noteId,
+      // 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);
     } catch (e) {
       if (e instanceof ClientError || e instanceof NotInDBError) {

src/media/media.service.spec.ts

@@ -98,10 +98,12 @@ describe('MediaService', () => {
   });
 
   describe('saveFile', () => {
+    let user: User;
+    let note: Note;
     beforeEach(() => {
-      const user = User.create('hardcoded', 'Testy') as User;
+      user = User.create('hardcoded', 'Testy') as User;
       const alias = 'alias';
-      const note = Note.create(user, alias);
+      note = Note.create(user, alias);
       jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user);
       jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note);
     });
@@ -126,22 +128,22 @@ describe('MediaService', () => {
             return [fileName, null];
           },
         );
-      const url = await service.saveFile(testImage, 'hardcoded', 'test');
+      const url = await service.saveFile(testImage, user, note);
       expect(url).toEqual(fileId);
     });
 
     describe('fails:', () => {
       it('MIME type not identifiable', async () => {
         await expect(
-          service.saveFile(Buffer.alloc(1), 'hardcoded', 'test'),
+          service.saveFile(Buffer.alloc(1), user, note),
         ).rejects.toThrow(ClientError);
       });
 
       it('MIME type not supported', async () => {
         const testText = await fs.readFile('test/public-api/fixtures/test.zip');
-        await expect(
-          service.saveFile(testText, 'hardcoded', 'test'),
-        ).rejects.toThrow(ClientError);
+        await expect(service.saveFile(testText, user, note)).rejects.toThrow(
+          ClientError,
+        );
       });
     });
   });

src/media/media.service.ts

@@ -69,24 +69,18 @@ export class MediaService {
    * @async
    * Save the given buffer to the configured MediaBackend and create a MediaUploadEntity to track where the file is, who uploaded it and to which note.
    * @param {Buffer} fileBuffer - the buffer of the file to save.
-   * @param {string} username - the username of the user who uploaded this file
-   * @param {string} noteId - the id or alias of the note which will be associated with the new file.
+   * @param {User} user - the user who uploaded this file
+   * @param {Note} note - the note which will be associated with the new file.
    * @return {string} the url of the saved file
    * @throws {ClientError} the MIME type of the file is not supported.
    * @throws {NotInDBError} - the note or user is not in the database
    * @throws {MediaBackendError} - there was an error saving the file
    */
-  async saveFile(
-    fileBuffer: Buffer,
-    username: string,
-    noteId: string,
-  ): Promise<string> {
+  async saveFile(fileBuffer: Buffer, user: User, note: Note): Promise<string> {
     this.logger.debug(
-      `Saving file for note '${noteId}' and user '${username}'`,
+      `Saving file for note '${note.id}' and user '${user.userName}'`,
       'saveFile',
     );
-    const note = await this.notesService.getNoteByIdOrAlias(noteId);
-    const user = await this.usersService.getUserByUsername(username);
     const fileTypeResult = await FileType.fromBuffer(fileBuffer);
     if (!fileTypeResult) {
       throw new ClientError('Could not detect file type.');