From f6121b58e875e456fbd7306b76ae18f0dbaa8cc9 Mon Sep 17 00:00:00 2001
From: Philip Molares <philip.molares@udo.edu>
Date: Sat, 20 Mar 2021 18:58:59 +0100
Subject: [PATCH] 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 | 18 ++++++++++++++-
src/media/media.service.spec.ts | 27 +++--------------------
src/media/media.service.ts | 28 +++++-------------------
3 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts
index 27f960c23..1472088b0 100644
--- a/src/api/public/media/media.controller.ts
+++ b/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);
diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts
index 9ee8481ab..e7acf22b0 100644
--- a/src/media/media.service.spec.ts
+++ b/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');
- });
-
- 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);
+ await service.deleteFile(mockMediaUploadEntry);
});
});
describe('findUploadByFilename', () => {
diff --git a/src/media/media.service.ts b/src/media/media.service.ts
index 7cb1b36d6..b0a25442d 100644
--- a/src/media/media.service.ts
+++ b/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.
- * @param {string} filename - 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
+ * Try to delete the specified file.
+ * @param {MediaUpload} mediaUpload - the name of the file to delete.
* @throws {MediaBackendError} - there was an error deleting the file
*/
- async deleteFile(filename: string, username: string): Promise<void> {
- this.logger.debug(
- `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);
+ async deleteFile(mediaUpload: MediaUpload): Promise<void> {
+ await this.mediaBackend.deleteFile(mediaUpload.id, mediaUpload.backendData);
await this.mediaUploadRepository.remove(mediaUpload);
}