Merge pull request #1094 from hedgedoc/notes/deleteMedia

src/api/private/notes/notes.controller.ts

@@ -6,6 +6,7 @@
 
 import {
   BadRequestException,
+  Body,
   Controller,
   Delete,
   Get,
@@ -33,6 +34,7 @@ import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto';
 import { RevisionDto } from '../../../revisions/revision.dto';
 import { RevisionsService } from '../../../revisions/revisions.service';
 import { MarkdownBody } from '../../utils/markdownbody-decorator';
+import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto';
 
 @Controller('notes')
 export class NotesController {
@@ -140,6 +142,7 @@ export class NotesController {
   @HttpCode(204)
   async deleteNote(
     @Param('noteIdOrAlias') noteIdOrAlias: string,
+    @Body() noteMediaDeletionDto: NoteMediaDeletionDto,
   ): Promise<void> {
     try {
       // ToDo: use actual user here
@@ -148,6 +151,14 @@ export class NotesController {
       if (!this.permissionsService.isOwner(user, note)) {
         throw new UnauthorizedException('Deleting note denied!');
       }
+      const mediaUploads = await this.mediaService.listUploadsByNote(note);
+      for (const mediaUpload of mediaUploads) {
+        if (!noteMediaDeletionDto.keepMedia) {
+          await this.mediaService.deleteFile(mediaUpload);
+        } else {
+          await this.mediaService.removeNoteFromMediaUpload(mediaUpload);
+        }
+      }
       this.logger.debug('Deleting note: ' + noteIdOrAlias, 'deleteNote');
       await this.noteService.deleteNote(note);
       this.logger.debug('Successfully deleted ' + noteIdOrAlias, 'deleteNote');

src/api/public/notes/notes.controller.ts

@@ -62,6 +62,7 @@ import {
 } from '../../utils/descriptions';
 import { MediaUploadDto } from '../../../media/media-upload.dto';
 import { MediaService } from '../../../media/media.service';
+import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto';
 
 @ApiTags('notes')
 @ApiSecurity('token')
@@ -172,12 +173,21 @@ export class NotesController {
   async deleteNote(
     @Req() req: Request,
     @Param('noteIdOrAlias') noteIdOrAlias: string,
+    @Body() noteMediaDeletionDto: NoteMediaDeletionDto,
   ): Promise<void> {
     try {
       const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias);
       if (!this.permissionsService.isOwner(req.user, note)) {
         throw new UnauthorizedException('Deleting note denied!');
       }
+      const mediaUploads = await this.mediaService.listUploadsByNote(note);
+      for (const mediaUpload of mediaUploads) {
+        if (!noteMediaDeletionDto.keepMedia) {
+          await this.mediaService.deleteFile(mediaUpload);
+        } else {
+          await this.mediaService.removeNoteFromMediaUpload(mediaUpload);
+        }
+      }
       this.logger.debug('Deleting note: ' + noteIdOrAlias, 'deleteNote');
       await this.noteService.deleteNote(note);
       this.logger.debug('Successfully deleted ' + noteIdOrAlias, 'deleteNote');

src/media/media-upload.entity.ts

@@ -24,7 +24,7 @@ export class MediaUpload {
   id: string;
 
   @ManyToOne((_) => Note, (note) => note.mediaUploads, {
-    nullable: false,
+    nullable: true,
   })
   note: Note;
 

src/media/media.service.spec.ts

@@ -265,4 +265,27 @@ describe('MediaService', () => {
       });
     });
   });
+
+  describe('removeNoteFromMediaUpload', () => {
+    it('works', async () => {
+      const mockMediaUploadEntry = {
+        id: 'testMediaUpload',
+        backendData: 'testBackendData',
+        note: {
+          alias: 'test',
+        } as Note,
+        user: {
+          userName: 'hardcoded',
+        } as User,
+      } as MediaUpload;
+      jest
+        .spyOn(mediaRepo, 'save')
+        .mockImplementationOnce(async (entry: MediaUpload) => {
+          expect(entry.note).toBeNull();
+          return entry;
+        });
+      await service.removeNoteFromMediaUpload(mockMediaUploadEntry);
+      expect(mediaRepo.save).toHaveBeenCalled();
+    });
+  });
 });

src/media/media.service.ts

@@ -176,6 +176,20 @@ export class MediaService {
     return mediaUploads;
   }
 
+  /**
+   * @async
+   * Set the note of a mediaUpload to null
+   * @param {MediaUpload} mediaUpload - the media upload to be changed
+   */
+  async removeNoteFromMediaUpload(mediaUpload: MediaUpload): Promise<void> {
+    this.logger.debug(
+      'Setting note to null for mediaUpload: ' + mediaUpload.id,
+      'removeNoteFromMediaUpload',
+    );
+    mediaUpload.note = null;
+    await this.mediaUploadRepository.save(mediaUpload);
+  }
+
   private chooseBackendType(): BackendType {
     switch (this.mediaConfig.backend.use) {
       case 'filesystem':

src/notes/note.media-deletion.dto.ts

@@ -0,0 +1,18 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+import { IsBoolean } from 'class-validator';
+import { ApiProperty } from '@nestjs/swagger';
+
+export class NoteMediaDeletionDto {
+  /**
+   * Should the associated mediaUploads be keept
+   * @default false
+   * @example false
+   */
+  @IsBoolean()
+  @ApiProperty()
+  keepMedia: boolean;
+}

test/private-api/notes.e2e-spec.ts

@@ -38,6 +38,7 @@ describe('Notes', () => {
   let content: string;
   let forbiddenNoteId: string;
   let uploadPath: string;
+  let testImage: Buffer;
 
   beforeAll(async () => {
     const moduleRef = await Test.createTestingModule({
@@ -80,6 +81,7 @@ describe('Notes', () => {
     user = await userService.createUser('hardcoded', 'Testy');
     user2 = await userService.createUser('hardcoded2', 'Max Mustermann');
     content = 'This is a test note.';
+    testImage = await fs.readFile('test/public-api/fixtures/test.png');
   });
 
   it('POST /notes', async () => {
@@ -152,12 +154,49 @@ describe('Notes', () => {
   });
 
   describe('DELETE /notes/{note}', () => {
-    it('works with an existing alias', async () => {
+    describe('works', () => {
-      await notesService.createNote(content, 'test3', user);
+      it('with an existing alias and keepMedia false', async () => {
-      await request(app.getHttpServer()).delete('/notes/test3').expect(204);
+        const noteId = 'test3';
-      await expect(notesService.getNoteByIdOrAlias('test3')).rejects.toEqual(
+        await notesService.createNote(content, noteId, user);
-        new NotInDBError("Note with id/alias 'test3' not found."),
+        await mediaService.saveFile(testImage, user.userName, noteId);
-      );
+        await request(app.getHttpServer())
+          .delete(`/notes/${noteId}`)
+          .set('Content-Type', 'application/json')
+          .send({
+            keepMedia: false,
+          })
+          .expect(204);
+        await expect(notesService.getNoteByIdOrAlias(noteId)).rejects.toEqual(
+          new NotInDBError(`Note with id/alias '${noteId}' not found.`),
+        );
+        expect(await mediaService.listUploadsByUser(user)).toHaveLength(0);
+        await fs.rmdir(uploadPath);
+      });
+      it('with an existing alias and keepMedia true', async () => {
+        const noteId = 'test3a';
+        await notesService.createNote(content, noteId, user);
+        const url = await mediaService.saveFile(
+          testImage,
+          user.userName,
+          noteId,
+        );
+        await request(app.getHttpServer())
+          .delete(`/notes/${noteId}`)
+          .set('Content-Type', 'application/json')
+          .send({
+            keepMedia: true,
+          })
+          .expect(204);
+        await expect(notesService.getNoteByIdOrAlias(noteId)).rejects.toEqual(
+          new NotInDBError(`Note with id/alias '${noteId}' not found.`),
+        );
+        expect(await mediaService.listUploadsByUser(user)).toHaveLength(1);
+        // Remove /upload/ from path as we just need the filename.
+        const fileName = url.replace('/uploads/', '');
+        // delete the file afterwards
+        await fs.unlink(join(uploadPath, fileName));
+        await fs.rmdir(uploadPath);
+      });
     });
     it('fails with a forbidden alias', async () => {
       await request(app.getHttpServer())

test/public-api/notes.e2e-spec.ts

@@ -38,6 +38,7 @@ describe('Notes', () => {
   let content: string;
   let forbiddenNoteId: string;
   let uploadPath: string;
+  let testImage: Buffer;
 
   beforeAll(async () => {
     const moduleRef = await Test.createTestingModule({
@@ -77,6 +78,7 @@ describe('Notes', () => {
     user = await userService.createUser('hardcoded', 'Testy');
     user2 = await userService.createUser('hardcoded2', 'Max Mustermann');
     content = 'This is a test note.';
+    testImage = await fs.readFile('test/public-api/fixtures/test.png');
   });
 
   it('POST /notes', async () => {
@@ -149,12 +151,47 @@ describe('Notes', () => {
   });
 
   describe('DELETE /notes/{note}', () => {
-    it('works with an existing alias', async () => {
+    describe('works', () => {
-      await notesService.createNote(content, 'test3', user);
+      it('with an existing alias and keepMedia false', async () => {
-      await request(app.getHttpServer()).delete('/notes/test3').expect(204);
+        const noteId = 'test3';
-      await expect(notesService.getNoteByIdOrAlias('test3')).rejects.toEqual(
+        await notesService.createNote(content, noteId, user);
-        new NotInDBError("Note with id/alias 'test3' not found."),
+        await mediaService.saveFile(testImage, user.userName, noteId);
-      );
+        await request(app.getHttpServer())
+          .delete(`/notes/${noteId}`)
+          .set('Content-Type', 'application/json')
+          .send({
+            keepMedia: false,
+          })
+          .expect(204);
+        await expect(notesService.getNoteByIdOrAlias(noteId)).rejects.toEqual(
+          new NotInDBError(`Note with id/alias '${noteId}' not found.`),
+        );
+        expect(await mediaService.listUploadsByUser(user)).toHaveLength(0);
+      });
+      it('with an existing alias and keepMedia true', async () => {
+        const noteId = 'test3a';
+        await notesService.createNote(content, noteId, user);
+        const url = await mediaService.saveFile(
+          testImage,
+          user.userName,
+          noteId,
+        );
+        await request(app.getHttpServer())
+          .delete(`/notes/${noteId}`)
+          .set('Content-Type', 'application/json')
+          .send({
+            keepMedia: true,
+          })
+          .expect(204);
+        await expect(notesService.getNoteByIdOrAlias(noteId)).rejects.toEqual(
+          new NotInDBError(`Note with id/alias '${noteId}' not found.`),
+        );
+        expect(await mediaService.listUploadsByUser(user)).toHaveLength(1);
+        // Remove /upload/ from path as we just need the filename.
+        const fileName = url.replace('/uploads/', '');
+        // delete the file afterwards
+        await fs.unlink(join(uploadPath, fileName));
+      });
     });
     it('works with an existing alias with permissions', async () => {
       const note = await notesService.createNote(content, 'test3', user);