Merge pull request #909 from hedgedoc/feature/media

src/media/media.service.spec.ts

@@ -20,11 +20,17 @@ import { Identity } from '../users/identity.entity';
 import { User } from '../users/user.entity';
 import { UsersModule } from '../users/users.module';
 import { FilesystemBackend } from './backends/filesystem-backend';
-import { MediaUpload } from './media-upload.entity';
+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';
 
 describe('MediaService', () => {
   let service: MediaService;
+  let noteRepo: Repository<Note>;
+  let userRepo: Repository<User>;
+  let mediaRepo: Repository<MediaUpload>;
 
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
@@ -32,7 +38,7 @@ describe('MediaService', () => {
         MediaService,
         {
           provide: getRepositoryToken(MediaUpload),
-          useValue: {},
+          useClass: Repository,
         },
         FilesystemBackend,
       ],
@@ -57,19 +63,154 @@ describe('MediaService', () => {
       .overrideProvider(getRepositoryToken(Identity))
       .useValue({})
       .overrideProvider(getRepositoryToken(Note))
-      .useValue({})
+      .useClass(Repository)
       .overrideProvider(getRepositoryToken(Revision))
       .useValue({})
       .overrideProvider(getRepositoryToken(User))
-      .useValue({})
+      .useClass(Repository)
       .overrideProvider(getRepositoryToken(Tag))
       .useValue({})
+      .overrideProvider(getRepositoryToken(MediaUpload))
+      .useClass(Repository)
       .compile();
 
     service = module.get<MediaService>(MediaService);
+    noteRepo = module.get<Repository<Note>>(getRepositoryToken(Note));
+    userRepo = module.get<Repository<User>>(getRepositoryToken(User));
+    mediaRepo = module.get<Repository<MediaUpload>>(
+      getRepositoryToken(MediaUpload),
+    );
   });
 
   it('should be defined', () => {
     expect(service).toBeDefined();
   });
+
+  describe('saveFile', () => {
+    beforeEach(async () => {
+      const user = User.create('hardcoded', 'Testy') as User;
+      const alias = 'alias';
+      const note = Note.create(user, alias);
+      jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user);
+      jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note);
+    });
+
+    it('works', async () => {
+      const testImage = await fs.readFile('test/public-api/fixtures/test.png');
+      let fileId = '';
+      jest
+        .spyOn(mediaRepo, 'save')
+        .mockImplementationOnce(async (entry: MediaUpload) => {
+          fileId = entry.id;
+          return entry;
+        });
+      jest.spyOn(service.mediaBackend, 'saveFile').mockImplementationOnce(
+        async (
+          buffer: Buffer,
+          fileName: string,
+        ): Promise<[string, BackendData]> => {
+          expect(buffer).toEqual(testImage);
+          return [fileName, null];
+        },
+      );
+      const url = await service.saveFile(testImage, 'hardcoded', 'test');
+      expect(url).toEqual(fileId);
+    });
+
+    describe('fails:', () => {
+      it('MIME type not identifiable', async () => {
+        try {
+          await service.saveFile(Buffer.alloc(1), 'hardcoded', 'test');
+        } catch (e) {
+          expect(e).toBeInstanceOf(ClientError);
+          expect(e.message).toContain('detect');
+        }
+      });
+
+      it('MIME type not supported', async () => {
+        try {
+          const testText = await fs.readFile(
+            'test/public-api/fixtures/test.zip',
+          );
+          await service.saveFile(testText, 'hardcoded', 'test');
+        } catch (e) {
+          expect(e).toBeInstanceOf(ClientError);
+          expect(e.message).not.toContain('detect');
+        }
+      });
+    });
+  });
+
+  describe('deleteFile', () => {
+    it('works', async () => {
+      const testFileName = 'testFilename';
+      const mockMediaUploadEntry = {
+        id: 'testMediaUpload',
+        backendData: 'testBackendData',
+        user: {
+          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(backendData).toEqual(mockMediaUploadEntry.backendData);
+        },
+      );
+      jest
+        .spyOn(mediaRepo, 'remove')
+        .mockImplementationOnce(async (entry, _) => {
+          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);
+      try {
+        await service.deleteFile(testFileName, 'hardcoded');
+      } catch (e) {
+        expect(e).toBeInstanceOf(PermissionError);
+      }
+    });
+  });
+  describe('findUploadByFilename', () => {
+    it('works', async () => {
+      const testFileName = 'testFilename';
+      const mockMediaUploadEntry = {
+        id: 'testMediaUpload',
+        backendData: 'testBackendData',
+        user: {
+          userName: 'hardcoded',
+        } as User,
+      } as MediaUpload;
+      jest
+        .spyOn(mediaRepo, 'findOne')
+        .mockResolvedValueOnce(mockMediaUploadEntry);
+      await service.findUploadByFilename(testFileName);
+    });
+    it("fails: can't find mediaUpload", async () => {
+      const testFileName = 'testFilename';
+      jest.spyOn(mediaRepo, 'findOne').mockResolvedValueOnce(undefined);
+      try {
+        await service.findUploadByFilename(testFileName);
+      } catch (e) {
+        expect(e).toBeInstanceOf(NotInDBError);
+      }
+    });
+  });
 });

src/media/media.service.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2020 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -59,7 +59,16 @@ export class MediaService {
     return allowedTypes.includes(mimeType);
   }
 
-  public async saveFile(
+  /**
+   * @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.
+   * @return {string} the url of the saved file
+   * @throws {ClientError} the MIME type of the file is not supported.
+   */
+  async saveFile(
     fileBuffer: Buffer,
     username: string,
     noteId: string,
@@ -93,7 +102,16 @@ export class MediaService {
     return url;
   }
 
-  public async deleteFile(filename: string, username: string): Promise<void> {
+  /**
+   * @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
+   */
+  async deleteFile(filename: string, username: string): Promise<void> {
     this.logger.debug(
       `Deleting '${filename}' for user '${username}'`,
       'deleteFile',
@@ -112,7 +130,14 @@ export class MediaService {
     await this.mediaUploadRepository.remove(mediaUpload);
   }
 
-  public async findUploadByFilename(filename: string): Promise<MediaUpload> {
+  /**
+   * @async
+   * Find a file entry by its filename.
+   * @param {string} filename - the name of the file entry to find
+   * @return {MediaUpload} the file entry, that was searched for
+   * @throws {NotInDBError} - the file entry specified is not in the database
+   */
+  async findUploadByFilename(filename: string): Promise<MediaUpload> {
     const mediaUpload = await this.mediaUploadRepository.findOne(filename, {
       relations: ['user'],
     });

test/public-api/fixtures/test.zip.license

@@ -0,0 +1,3 @@
+SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+
+SPDX-License-Identifier: CC0-1.0

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

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-import { ConfigModule } from '@nestjs/config';
+import { ConfigModule, ConfigService } from '@nestjs/config';
 import { NestExpressApplication } from '@nestjs/platform-express';
 import { Test } from '@nestjs/testing';
 import { TypeOrmModule } from '@nestjs/typeorm';
@@ -23,10 +23,12 @@ import { PermissionsModule } from '../../src/permissions/permissions.module';
 import { AuthModule } from '../../src/auth/auth.module';
 import { TokenAuthGuard } from '../../src/auth/token-auth.guard';
 import { MockAuthGuard } from '../../src/auth/mock-auth.guard';
+import { join } from 'path';
 
 describe('Notes', () => {
   let app: NestExpressApplication;
   let mediaService: MediaService;
+  let uploadPath: string;
 
   beforeAll(async () => {
     const moduleRef = await Test.createTestingModule({
@@ -54,8 +56,10 @@ describe('Notes', () => {
       .overrideGuard(TokenAuthGuard)
       .useClass(MockAuthGuard)
       .compile();
+    const config = moduleRef.get<ConfigService>(ConfigService);
+    uploadPath = config.get('mediaConfig').backend.filesystem.uploadPath;
     app = moduleRef.createNestApplication<NestExpressApplication>();
-    app.useStaticAssets('uploads', {
+    app.useStaticAssets(uploadPath, {
       prefix: '/uploads',
     });
     await app.init();
@@ -78,6 +82,10 @@ describe('Notes', () => {
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
     const downloadResponse = await request(app.getHttpServer()).get(path);
     expect(downloadResponse.body).toEqual(testImage);
+    // Remove /upload/ from path as we just need the filename.
+    const fileName = path.replace('/uploads/', '');
+    // delete the file afterwards
+    await fs.unlink(join(uploadPath, fileName));
   });
 
   it('DELETE /media/{filename}', async () => {
@@ -92,4 +100,9 @@ describe('Notes', () => {
       .delete('/media/' + filename)
       .expect(200);
   });
+
+  afterAll(async () => {
+    // Delete the upload folder
+    await fs.rmdir(uploadPath);
+  });
 });