refactor(api/private/media): return MediaUpload object instead of url

This ensures the private POST /media API behaves in the same way as /me/media

Signed-off-by: David Mehren <git@herrmehren.de>
This commit is contained in:
David Mehren 2022-01-29 18:57:44 +01:00
parent 4f10e17d40
commit 8e31f3a393
10 changed files with 50 additions and 43 deletions

View file

@ -19,7 +19,7 @@ import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger';
import { PermissionError } from '../../../errors/errors'; import { PermissionError } from '../../../errors/errors';
import { SessionGuard } from '../../../identity/session.guard'; import { SessionGuard } from '../../../identity/session.guard';
import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service';
import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; import { MediaUploadDto } from '../../../media/media-upload.dto';
import { MediaService } from '../../../media/media.service'; import { MediaService } from '../../../media/media.service';
import { MulterFile } from '../../../media/multer-file.interface'; import { MulterFile } from '../../../media/multer-file.interface';
import { Note } from '../../../notes/note.entity'; import { Note } from '../../../notes/note.entity';
@ -63,7 +63,7 @@ export class MediaController {
{ {
code: 201, code: 201,
description: 'The file was uploaded successfully', description: 'The file was uploaded successfully',
dto: MediaUploadUrlDto, dto: MediaUploadDto,
}, },
400, 400,
403, 403,
@ -74,15 +74,15 @@ export class MediaController {
@UploadedFile() file: MulterFile, @UploadedFile() file: MulterFile,
@Headers('HedgeDoc-Note') noteId: string, @Headers('HedgeDoc-Note') noteId: string,
@RequestUser() user: User, @RequestUser() user: User,
): Promise<MediaUploadUrlDto> { ): Promise<MediaUploadDto> {
// TODO: Move getting the Note object into a decorator // TODO: Move getting the Note object into a decorator
const note: Note = await this.noteService.getNoteByIdOrAlias(noteId); const note: Note = await this.noteService.getNoteByIdOrAlias(noteId);
this.logger.debug( this.logger.debug(
`Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`, `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`,
'uploadMedia', 'uploadMedia',
); );
const url = await this.mediaService.saveFile(file.buffer, user, note); const upload = await this.mediaService.saveFile(file.buffer, user, note);
return this.mediaService.toMediaUploadUrlDto(url); return await this.mediaService.toMediaUploadDto(upload);
} }
@Delete(':filename') @Delete(':filename')

View file

@ -88,8 +88,8 @@ export class MediaController {
`Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`, `Recieved filename '${file.originalname}' for note '${noteId}' from user '${user.username}'`,
'uploadMedia', 'uploadMedia',
); );
const url = await this.mediaService.saveFile(file.buffer, user, note); const upload = await this.mediaService.saveFile(file.buffer, user, note);
return this.mediaService.toMediaUploadUrlDto(url); return this.mediaService.toMediaUploadUrlDto(upload.fileUrl);
} }
@Delete(':filename') @Delete(':filename')

View file

@ -143,8 +143,8 @@ describe('MediaService', () => {
return [fileName, null]; return [fileName, null];
}, },
); );
const url = await service.saveFile(testImage, user, note); const upload = await service.saveFile(testImage, user, note);
expect(url).toEqual(fileId); expect(upload.fileUrl).toEqual(fileId);
}); });
describe('fails:', () => { describe('fails:', () => {

View file

@ -77,7 +77,11 @@ export class MediaService {
* @throws {NotInDBError} - the note or user is not in the database * @throws {NotInDBError} - the note or user is not in the database
* @throws {MediaBackendError} - there was an error saving the file * @throws {MediaBackendError} - there was an error saving the file
*/ */
async saveFile(fileBuffer: Buffer, user: User, note: Note): Promise<string> { async saveFile(
fileBuffer: Buffer,
user: User,
note: Note,
): Promise<MediaUpload> {
this.logger.debug( this.logger.debug(
`Saving file for note '${note.id}' and user '${user.username}'`, `Saving file for note '${note.id}' and user '${user.username}'`,
'saveFile', 'saveFile',
@ -102,8 +106,7 @@ export class MediaService {
url, url,
); );
mediaUpload.backendData = backendData; mediaUpload.backendData = backendData;
await this.mediaUploadRepository.save(mediaUpload); return await this.mediaUploadRepository.save(mediaUpload);
return url;
} }
/** /**

View file

@ -69,16 +69,16 @@ describe('Me', () => {
const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const testImage = await fs.readFile('test/public-api/fixtures/test.png');
const imageUrls = []; const imageUrls = [];
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note1), (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note1), (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note2), (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note2), (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
); );
const response = await agent const response = await agent
@ -112,12 +112,16 @@ describe('Me', () => {
it('DELETE /me', async () => { it('DELETE /me', async () => {
const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const testImage = await fs.readFile('test/public-api/fixtures/test.png');
const url0 = await testSetup.mediaService.saveFile(testImage, user, note1); const upload = await testSetup.mediaService.saveFile(
testImage,
user,
note1,
);
const dbUser = await testSetup.userService.getUserByUsername('hardcoded'); const dbUser = await testSetup.userService.getUserByUsername('hardcoded');
expect(dbUser).toBeInstanceOf(User); expect(dbUser).toBeInstanceOf(User);
const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser); const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser);
expect(mediaUploads).toHaveLength(1); expect(mediaUploads).toHaveLength(1);
expect(mediaUploads[0].fileUrl).toEqual(url0); expect(mediaUploads[0].fileUrl).toEqual(upload.fileUrl);
await agent.delete('/api/private/me').expect(204); await agent.delete('/api/private/me').expect(204);
await expect( await expect(
testSetup.userService.getUserByUsername('hardcoded'), testSetup.userService.getUserByUsername('hardcoded'),

View file

@ -64,7 +64,7 @@ describe('Media', () => {
.set('HedgeDoc-Note', 'test_upload_media') .set('HedgeDoc-Note', 'test_upload_media')
.expect('Content-Type', /json/) .expect('Content-Type', /json/)
.expect(201); .expect(201);
const path: string = uploadResponse.body.link; const path: string = uploadResponse.body.url;
const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const testImage = await fs.readFile('test/private-api/fixtures/test.png');
const downloadResponse = await agent.get(path); const downloadResponse = await agent.get(path);
expect(downloadResponse.body).toEqual(testImage); expect(downloadResponse.body).toEqual(testImage);
@ -117,12 +117,12 @@ describe('Media', () => {
'test_delete_media', 'test_delete_media',
); );
const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const testImage = await fs.readFile('test/private-api/fixtures/test.png');
const url = await testSetup.mediaService.saveFile( const upload = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
testNote, testNote,
); );
const filename = url.split('/').pop() || ''; const filename = upload.fileUrl.split('/').pop() || '';
await agent.delete('/api/private/media/' + filename).expect(204); await agent.delete('/api/private/media/' + filename).expect(204);
}); });
}); });

View file

@ -161,7 +161,7 @@ describe('Notes', () => {
user, user,
noteId, noteId,
); );
const url = await testSetup.mediaService.saveFile( const upload = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note, note,
@ -182,7 +182,7 @@ describe('Notes', () => {
await testSetup.mediaService.listUploadsByUser(user), await testSetup.mediaService.listUploadsByUser(user),
).toHaveLength(1); ).toHaveLength(1);
// Remove /upload/ from path as we just need the filename. // Remove /upload/ from path as we just need the filename.
const fileName = url.replace('/uploads/', ''); const fileName = upload.fileUrl.replace('/uploads/', '');
// delete the file afterwards // delete the file afterwards
await fs.unlink(join(uploadPath, fileName)); await fs.unlink(join(uploadPath, fileName));
await fs.rmdir(uploadPath); await fs.rmdir(uploadPath);
@ -308,12 +308,12 @@ describe('Notes', () => {
expect(response.body).toHaveLength(0); expect(response.body).toHaveLength(0);
const testImage = await fs.readFile('test/private-api/fixtures/test.png'); const testImage = await fs.readFile('test/private-api/fixtures/test.png');
const url0 = await testSetup.mediaService.saveFile( const upload0 = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note1, note1,
); );
const url1 = await testSetup.mediaService.saveFile( const upload1 = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note2, note2,
@ -324,10 +324,10 @@ describe('Notes', () => {
.expect('Content-Type', /json/) .expect('Content-Type', /json/)
.expect(200); .expect(200);
expect(responseAfter.body).toHaveLength(1); expect(responseAfter.body).toHaveLength(1);
expect(responseAfter.body[0].url).toEqual(url0); expect(responseAfter.body[0].url).toEqual(upload0.fileUrl);
expect(responseAfter.body[0].url).not.toEqual(url1); expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl);
for (const fileUrl of [url0, url1]) { for (const upload of [upload0, upload1]) {
const fileName = fileUrl.replace('/uploads/', ''); const fileName = upload.fileUrl.replace('/uploads/', '');
// delete the file afterwards // delete the file afterwards
await fs.unlink(join(uploadPath, fileName)); await fs.unlink(join(uploadPath, fileName));
} }

View file

@ -204,16 +204,16 @@ describe('Me', () => {
const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const testImage = await fs.readFile('test/public-api/fixtures/test.png');
const imageUrls = []; const imageUrls = [];
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note1), (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note1), (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note2), (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
); );
imageUrls.push( imageUrls.push(
await testSetup.mediaService.saveFile(testImage, user, note2), (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
); );
const response = await request(httpServer) const response = await request(httpServer)

View file

@ -108,12 +108,12 @@ describe('Media', () => {
it('DELETE /media/{filename}', async () => { it('DELETE /media/{filename}', async () => {
const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const testImage = await fs.readFile('test/public-api/fixtures/test.png');
const url = await testSetup.mediaService.saveFile( const upload = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
testNote, testNote,
); );
const filename = url.split('/').pop() || ''; const filename = upload.fileUrl.split('/').pop() || '';
await request(testSetup.app.getHttpServer()) await request(testSetup.app.getHttpServer())
.delete('/api/v2/media/' + filename) .delete('/api/v2/media/' + filename)
.expect(204); .expect(204);

View file

@ -159,7 +159,7 @@ describe('Notes', () => {
user, user,
noteId, noteId,
); );
const url = await testSetup.mediaService.saveFile( const upload = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note, note,
@ -180,7 +180,7 @@ describe('Notes', () => {
await testSetup.mediaService.listUploadsByUser(user), await testSetup.mediaService.listUploadsByUser(user),
).toHaveLength(1); ).toHaveLength(1);
// Remove /upload/ from path as we just need the filename. // Remove /upload/ from path as we just need the filename.
const fileName = url.replace('/uploads/', ''); const fileName = upload.fileUrl.replace('/uploads/', '');
// delete the file afterwards // delete the file afterwards
await fs.unlink(join(uploadPath, fileName)); await fs.unlink(join(uploadPath, fileName));
}); });
@ -425,12 +425,12 @@ describe('Notes', () => {
expect(response.body).toHaveLength(0); expect(response.body).toHaveLength(0);
const testImage = await fs.readFile('test/public-api/fixtures/test.png'); const testImage = await fs.readFile('test/public-api/fixtures/test.png');
const url0 = await testSetup.mediaService.saveFile( const upload0 = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note1, note1,
); );
const url1 = await testSetup.mediaService.saveFile( const upload1 = await testSetup.mediaService.saveFile(
testImage, testImage,
user, user,
note2, note2,
@ -441,10 +441,10 @@ describe('Notes', () => {
.expect('Content-Type', /json/) .expect('Content-Type', /json/)
.expect(200); .expect(200);
expect(responseAfter.body).toHaveLength(1); expect(responseAfter.body).toHaveLength(1);
expect(responseAfter.body[0].url).toEqual(url0); expect(responseAfter.body[0].url).toEqual(upload0.fileUrl);
expect(responseAfter.body[0].url).not.toEqual(url1); expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl);
for (const fileUrl of [url0, url1]) { for (const upload of [upload0, upload1]) {
const fileName = fileUrl.replace('/uploads/', ''); const fileName = upload.fileUrl.replace('/uploads/', '');
// delete the file afterwards // delete the file afterwards
await fs.unlink(join(uploadPath, fileName)); await fs.unlink(join(uploadPath, fileName));
} }