MediaService: Change deleteFile

The former deleteFile was moved to the public apis media controller and the actual deletion functionality was moved in a separate function to be called on user deletion.

Signed-off-by: Philip Molares <philip.molares@udo.edu>

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

@@ -132,7 +132,23 @@ export class MediaController {
   ): Promise<void> {
     const username = req.user.userName;
     try {
-      await this.mediaService.deleteFile(filename, username);
+      this.logger.debug(
+        `Deleting '${filename}' for user '${username}'`,
+        'deleteFile',
+      );
+      const mediaUpload = await this.mediaService.findUploadByFilename(
+        filename,
+      );
+      if (mediaUpload.user.userName !== username) {
+        this.logger.warn(
+          `${username} tried to delete '${filename}', but is not the owner`,
+          'deleteFile',
+        );
+        throw new PermissionError(
+          `File '${filename}' is not owned by '${username}'`,
+        );
+      }
+      await this.mediaService.deleteFile(mediaUpload);
     } catch (e) {
       if (e instanceof PermissionError) {
         throw new UnauthorizedException(e.message);

src/media/media.service.spec.ts

@@ -24,7 +24,7 @@ import { BackendData, MediaUpload } from './media-upload.entity';
 import { MediaService } from './media.service';
 import { Repository } from 'typeorm';
 import { promises as fs } from 'fs';
-import { ClientError, NotInDBError, PermissionError } from '../errors/errors';
+import { ClientError, NotInDBError } from '../errors/errors';
 import { NoteGroupPermission } from '../permissions/note-group-permission.entity';
 import { NoteUserPermission } from '../permissions/note-user-permission.entity';
 import { Group } from '../groups/group.entity';
@@ -145,7 +145,6 @@ describe('MediaService', () => {
 
   describe('deleteFile', () => {
     it('works', async () => {
-      const testFileName = 'testFilename';
       const mockMediaUploadEntry = {
         id: 'testMediaUpload',
         backendData: 'testBackendData',
@@ -153,12 +152,9 @@ describe('MediaService', () => {
           userName: 'hardcoded',
         } as User,
       } as MediaUpload;
-      jest
-        .spyOn(mediaRepo, 'findOne')
-        .mockResolvedValueOnce(mockMediaUploadEntry);
       jest.spyOn(service.mediaBackend, 'deleteFile').mockImplementationOnce(
         async (fileName: string, backendData: BackendData): Promise<void> => {
-          expect(fileName).toEqual(testFileName);
+          expect(fileName).toEqual(mockMediaUploadEntry.id);
           expect(backendData).toEqual(mockMediaUploadEntry.backendData);
         },
       );
@@ -168,24 +164,7 @@ describe('MediaService', () => {
           expect(entry).toEqual(mockMediaUploadEntry);
           return entry;
         });
-      await service.deleteFile(testFileName, 'hardcoded');
+      await service.deleteFile(mockMediaUploadEntry);
-    });
-
-    it('fails: the mediaUpload is not owned by user', async () => {
-      const testFileName = 'testFilename';
-      const mockMediaUploadEntry = {
-        id: 'testMediaUpload',
-        backendData: 'testBackendData',
-        user: {
-          userName: 'not-hardcoded',
-        } as User,
-      } as MediaUpload;
-      jest
-        .spyOn(mediaRepo, 'findOne')
-        .mockResolvedValueOnce(mockMediaUploadEntry);
-      await expect(
-        service.deleteFile(testFileName, 'hardcoded'),
-      ).rejects.toThrow(PermissionError);
     });
   });
   describe('findUploadByFilename', () => {

src/media/media.service.ts

@@ -10,7 +10,7 @@ import { InjectRepository } from '@nestjs/typeorm';
 import * as FileType from 'file-type';
 import { Repository } from 'typeorm';
 import mediaConfiguration, { MediaConfig } from '../config/media.config';
-import { ClientError, NotInDBError, PermissionError } from '../errors/errors';
+import { ClientError, NotInDBError } from '../errors/errors';
 import { ConsoleLoggerService } from '../logger/console-logger.service';
 import { NotesService } from '../notes/notes.service';
 import { UsersService } from '../users/users.service';
@@ -113,30 +113,12 @@ export class MediaService {
 
   /**
    * @async
-   * Try to delete the file specified by the filename with the user specified by the username.
+   * Try to delete the specified file.
-   * @param {string} filename - the name of the file to delete.
+   * @param {MediaUpload} mediaUpload - the name of the file to delete.
-   * @param {string} username - the username of the user who uploaded this file
-   * @return {string} the url of the saved file
-   * @throws {PermissionError} the user is not permitted to delete this file.
-   * @throws {NotInDBError} - the file entry specified is not in the database
    * @throws {MediaBackendError} - there was an error deleting the file
    */
-  async deleteFile(filename: string, username: string): Promise<void> {
+  async deleteFile(mediaUpload: MediaUpload): Promise<void> {
-    this.logger.debug(
+    await this.mediaBackend.deleteFile(mediaUpload.id, mediaUpload.backendData);
-      `Deleting '${filename}' for user '${username}'`,
-      'deleteFile',
-    );
-    const mediaUpload = await this.findUploadByFilename(filename);
-    if (mediaUpload.user.userName !== username) {
-      this.logger.warn(
-        `${username} tried to delete '${filename}', but is not the owner`,
-        'deleteFile',
-      );
-      throw new PermissionError(
-        `File '${filename}' is not owned by '${username}'`,
-      );
-    }
-    await this.mediaBackend.deleteFile(filename, mediaUpload.backendData);
     await this.mediaUploadRepository.remove(mediaUpload);
   }