From 0d5b9dea000d56ea5b7e2980698087611c289bbc Mon Sep 17 00:00:00 2001 From: Yannick Bungers Date: Sat, 30 Jan 2021 00:06:38 +0100 Subject: [PATCH] Refactoring of controllers and service interfaces DTO should only be used for sending information to and from user. Services now have methods which return normal internal objects and methods which convert them to DTOs. This conversion is done in the controlers Signed-off-by: Yannick Bungers --- src/api/public/me/me.controller.ts | 7 +- src/api/public/media/media.controller.ts | 9 +- .../monitoring/monitoring.controller.ts | 4 +- src/api/public/notes/notes.controller.ts | 87 +++++---- src/media/media-upload-url.dto.ts | 14 ++ src/media/media.service.ts | 11 +- src/notes/notes.service.ts | 168 ++++++++---------- src/revisions/revisions.service.ts | 22 ++- 8 files changed, 175 insertions(+), 147 deletions(-) create mode 100644 src/media/media-upload-url.dto.ts diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 02e51e2af..6d35c9917 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -78,7 +78,10 @@ export class MeController { @UseGuards(TokenAuthGuard) @Get('notes') - getMyNotes(@Request() req): NoteMetadataDto[] { - return this.notesService.getUserNotes(req.user.userName); + async getMyNotes(@Request() req): Promise { + const notes = await this.notesService.getUserNotes(req.user) + return Promise.all( + notes.map(note => this.notesService.toNoteMetadataDto(note)) + ); } } diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index f8fd3bb45..0234c42db 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -29,6 +29,7 @@ import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { ApiSecurity } from '@nestjs/swagger'; +import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; @ApiSecurity('token') @Controller('media') @@ -47,7 +48,7 @@ export class MediaController { @Request() req, @UploadedFile() file: MulterFile, @Headers('HedgeDoc-Note') noteId: string, - ) { + ) : Promise { const username = req.user.userName; this.logger.debug( `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`, @@ -59,9 +60,7 @@ export class MediaController { username, noteId, ); - return { - link: url, - }; + return this.mediaService.toMediaUploadUrlDto(url) } catch (e) { if (e instanceof ClientError || e instanceof NotInDBError) { throw new BadRequestException(e.message); @@ -72,7 +71,7 @@ export class MediaController { @UseGuards(TokenAuthGuard) @Delete(':filename') - async deleteMedia(@Request() req, @Param('filename') filename: string) { + async deleteMedia(@Request() req, @Param('filename') filename: string) : Promise { const username = req.user.userName; try { await this.mediaService.deleteFile(filename, username); diff --git a/src/api/public/monitoring/monitoring.controller.ts b/src/api/public/monitoring/monitoring.controller.ts index f32e39701..c7cde2193 100644 --- a/src/api/public/monitoring/monitoring.controller.ts +++ b/src/api/public/monitoring/monitoring.controller.ts @@ -8,6 +8,7 @@ import { Controller, Get, UseGuards } from '@nestjs/common'; import { MonitoringService } from '../../../monitoring/monitoring.service'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { ApiSecurity } from '@nestjs/swagger'; +import { ServerStatusDto } from '../../../monitoring/server-status.dto'; @ApiSecurity('token') @Controller('monitoring') @@ -16,7 +17,8 @@ export class MonitoringController { @UseGuards(TokenAuthGuard) @Get() - getStatus() { + getStatus() : Promise { + // TODO: toServerStatusDto. return this.monitoringService.getServerStatus(); } diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 5a1782eb3..159e58ef9 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -19,12 +19,16 @@ import { } from '@nestjs/common'; import { NotInDBError } from '../../../errors/errors'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; -import { NotePermissionsUpdateDto } from '../../../notes/note-permissions.dto'; +import { NotePermissionsDto, NotePermissionsUpdateDto } from '../../../notes/note-permissions.dto'; import { NotesService } from '../../../notes/notes.service'; import { RevisionsService } from '../../../revisions/revisions.service'; import { MarkdownBody } from '../../utils/markdownbody-decorator'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { ApiSecurity } from '@nestjs/swagger'; +import { NoteDto } from '../../../notes/note.dto'; +import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; +import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto'; +import { RevisionDto } from '../../../revisions/revision.dto'; @ApiSecurity('token') @Controller('notes') @@ -39,18 +43,36 @@ export class NotesController { @UseGuards(TokenAuthGuard) @Post() - async createNote(@Request() req, @MarkdownBody() text: string) { + async createNote(@Request() req, @MarkdownBody() text: string): Promise { // ToDo: provide user for createNoteDto this.logger.debug('Got raw markdown:\n' + text); - return this.noteService.createNoteDto(text); + return this.noteService.toNoteDto( + await this.noteService.createNote(text, undefined, req.user) + ); + } + + @UseGuards(TokenAuthGuard) + @Post(':noteAlias') + async createNamedNote( + @Request() req, + @Param('noteAlias') noteAlias: string, + @MarkdownBody() text: string, + ): Promise { + // ToDo: check if user is allowed to view this note + this.logger.debug('Got raw markdown:\n' + text); + return this.noteService.toNoteDto( + await this.noteService.createNote(text, noteAlias, req.user) + ); } @UseGuards(TokenAuthGuard) @Get(':noteIdOrAlias') - async getNote(@Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string) { + async getNote(@Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string) : Promise { // ToDo: check if user is allowed to view this note try { - return await this.noteService.getNoteDtoByIdOrAlias(noteIdOrAlias); + return this.noteService.toNoteDto( + await this.noteService.getNoteByIdOrAlias(noteIdOrAlias) + ); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -59,24 +81,12 @@ export class NotesController { } } - @UseGuards(TokenAuthGuard) - @Post(':noteAlias') - async createNamedNote( - @Request() req, - @Param('noteAlias') noteAlias: string, - @MarkdownBody() text: string, - ) { - // ToDo: check if user is allowed to view this note - this.logger.debug('Got raw markdown:\n' + text); - return this.noteService.createNoteDto(text, noteAlias); - } - @UseGuards(TokenAuthGuard) @Delete(':noteIdOrAlias') async deleteNote( @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, - ) { + ): Promise { // ToDo: check if user is allowed to delete this note this.logger.debug('Deleting note: ' + noteIdOrAlias); try { @@ -97,11 +107,13 @@ export class NotesController { @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, @MarkdownBody() text: string, - ) { + ) : Promise { // ToDo: check if user is allowed to change this note this.logger.debug('Got raw markdown:\n' + text); try { - return await this.noteService.updateNoteByIdOrAlias(noteIdOrAlias, text); + return this.noteService.toNoteDto( + await this.noteService.updateNoteByIdOrAlias(noteIdOrAlias, text) + ); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -116,7 +128,7 @@ export class NotesController { async getNoteContent( @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, - ) { + ) : Promise { // ToDo: check if user is allowed to view this notes content try { return await this.noteService.getNoteContent(noteIdOrAlias); @@ -133,10 +145,12 @@ export class NotesController { async getNoteMetadata( @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, - ) { + ) : Promise { // ToDo: check if user is allowed to view this notes metadata try { - return await this.noteService.getNoteMetadata(noteIdOrAlias); + return this.noteService.toNoteMetadataDto( + await this.noteService.getNoteByIdOrAlias(noteIdOrAlias) + ); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -151,12 +165,14 @@ export class NotesController { @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, @Body() updateDto: NotePermissionsUpdateDto, - ) { + ) : Promise { // ToDo: check if user is allowed to view this notes permissions try { - return await this.noteService.updateNotePermissions( - noteIdOrAlias, - updateDto, + return this.noteService.toNotePermissionsDto( + await this.noteService.updateNotePermissions( + noteIdOrAlias, + updateDto, + ) ); } catch (e) { if (e instanceof NotInDBError) { @@ -171,12 +187,15 @@ export class NotesController { async getNoteRevisions( @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, - ) { + ) : Promise { // ToDo: check if user is allowed to view this notes revisions try { - return await this.revisionsService.getNoteRevisionMetadatas( + const revisions = await this.revisionsService.getAllRevisions( noteIdOrAlias, ); + return Promise.all( + revisions.map(revision => this.revisionsService.toRevisionMetadataDto(revision)) + ); } catch (e) { if (e instanceof NotInDBError) { throw new NotFoundException(e.message); @@ -191,12 +210,14 @@ export class NotesController { @Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string, @Param('revisionId') revisionId: number, - ) { + ) : Promise { // ToDo: check if user is allowed to view this notes revision try { - return await this.revisionsService.getNoteRevision( - noteIdOrAlias, - revisionId, + return this.revisionsService.toRevisionDto( + await this.revisionsService.getRevision( + noteIdOrAlias, + revisionId, + ) ); } catch (e) { if (e instanceof NotInDBError) { diff --git a/src/media/media-upload-url.dto.ts b/src/media/media-upload-url.dto.ts new file mode 100644 index 000000000..c4b869894 --- /dev/null +++ b/src/media/media-upload-url.dto.ts @@ -0,0 +1,14 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + + + +import { IsString } from 'class-validator'; + +export class MediaUploadUrlDto { + @IsString() + link: string +} diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 8b5dfdecb..4714281c4 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -18,6 +18,7 @@ import { BackendType } from './backends/backend-type.enum'; import { FilesystemBackend } from './backends/filesystem-backend'; import { MediaBackend } from './media-backend.interface'; import { MediaUpload } from './media-upload.entity'; +import { MediaUploadUrlDto } from './media-upload-url.dto'; @Injectable() export class MediaService { @@ -58,7 +59,7 @@ export class MediaService { return allowedTypes.includes(mimeType); } - public async saveFile(fileBuffer: Buffer, username: string, noteId: string) { + public async saveFile(fileBuffer: Buffer, username: string, noteId: string): Promise { this.logger.debug( `Saving file for note '${noteId}' and user '${username}'`, 'saveFile', @@ -88,7 +89,7 @@ export class MediaService { return url; } - public async deleteFile(filename: string, username: string) { + public async deleteFile(filename: string, username: string): Promise { this.logger.debug( `Deleting '${filename}' for user '${username}'`, 'deleteFile', @@ -132,4 +133,10 @@ export class MediaService { return this.moduleRef.get(FilesystemBackend); } } + + toMediaUploadUrlDto(url: string): MediaUploadUrlDto { + return { + link: url, + } + } } diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 8e1f11284..b810246f4 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -35,48 +35,26 @@ export class NotesService { this.logger.setContext(NotesService.name); } - getUserNotes(username: string): NoteMetadataDto[] { + getUserNotes(user: User): Note[] { this.logger.warn('Using hardcoded data!'); return [ { - alias: null, - createTime: new Date(), - description: 'Very descriptive text.', - editedBy: [], id: 'foobar-barfoo', - permissions: { - owner: { - displayName: 'foo', - userName: 'fooUser', - email: 'foo@example.com', - photo: '', - }, - sharedToUsers: [], - sharedToGroups: [], - }, + alias: null, + shortid: "abc", + owner: user, + description: 'Very descriptive text.', + userPermissions: [], + groupPermissions: [], tags: [], + revisions: Promise.resolve([]), + authorColors: [], title: 'Title!', - updateTime: new Date(), - updateUser: { - displayName: 'foo', - userName: 'fooUser', - email: 'foo@example.com', - photo: '', - }, - viewCount: 42, + viewcount: 42, }, ]; } - async createNoteDto( - noteContent: string, - alias?: NoteMetadataDto['alias'], - owner?: User, - ): Promise { - const note = await this.createNote(noteContent, alias, owner); - return this.toNoteDto(note); - } - async createNote( noteContent: string, alias?: NoteMetadataDto['alias'], @@ -96,7 +74,7 @@ export class NotesService { return this.noteRepository.save(newNote); } - async getCurrentContent(note: Note) { + async getCurrentContent(note: Note): Promise { return (await this.getLatestRevision(note)).content; } @@ -108,42 +86,6 @@ export class NotesService { return this.revisionsService.getFirstRevision(note.id); } - async getMetadata(note: Note): Promise { - return { - // TODO: Convert DB UUID to base64 - id: note.id, - alias: note.alias, - title: note.title, - createTime: (await this.getFirstRevision(note)).createdAt, - description: note.description, - editedBy: note.authorColors.map( - (authorColor) => authorColor.user.userName, - ), - // TODO: Extract into method - permissions: { - owner: this.usersService.toUserDto(note.owner), - sharedToUsers: note.userPermissions.map((noteUserPermission) => ({ - user: this.usersService.toUserDto(noteUserPermission.user), - canEdit: noteUserPermission.canEdit, - })), - sharedToGroups: note.groupPermissions.map((noteGroupPermission) => ({ - group: noteGroupPermission.group, - canEdit: noteGroupPermission.canEdit, - })), - }, - tags: note.tags.map((tag) => tag.name), - updateTime: (await this.getLatestRevision(note)).createdAt, - // TODO: Get actual updateUser - updateUser: { - displayName: 'Hardcoded User', - userName: 'hardcoded', - email: 'foo@example.com', - photo: '', - }, - viewCount: 42, - }; - } - async getNoteByIdOrAlias(noteIdOrAlias: string): Promise { this.logger.debug( `Trying to find note '${noteIdOrAlias}'`, @@ -178,45 +120,47 @@ export class NotesService { return note; } - async getNoteDtoByIdOrAlias(noteIdOrAlias: string): Promise { - const note = await this.getNoteByIdOrAlias(noteIdOrAlias); - return this.toNoteDto(note); - } - async deleteNoteByIdOrAlias(noteIdOrAlias: string) { const note = await this.getNoteByIdOrAlias(noteIdOrAlias); return await this.noteRepository.remove(note); } - async updateNoteByIdOrAlias(noteIdOrAlias: string, noteContent: string) { + async updateNoteByIdOrAlias(noteIdOrAlias: string, noteContent: string): Promise { const note = await this.getNoteByIdOrAlias(noteIdOrAlias); const revisions = await note.revisions; //TODO: Calculate patch revisions.push(Revision.create(noteContent, noteContent)); note.revisions = Promise.resolve(revisions); - await this.noteRepository.save(note); - return this.toNoteDto(note); - } - - async getNoteMetadata(noteIdOrAlias: string): Promise { - const note = await this.getNoteByIdOrAlias(noteIdOrAlias); - return this.getMetadata(note); + return this.noteRepository.save(note); } updateNotePermissions( noteIdOrAlias: string, newPermissions: NotePermissionsUpdateDto, - ): NotePermissionsDto { + ): Note { this.logger.warn('Using hardcoded data!'); return { - owner: { - displayName: 'foo', - userName: 'fooUser', - email: 'foo@example.com', - photo: '', - }, - sharedToUsers: [], - sharedToGroups: [], + id: 'foobar-barfoo', + alias: null, + shortid: "abc", + owner: { + authTokens: [], + createdAt: new Date(), + displayName: 'hardcoded', + id: '1', + identities: [], + ownedNotes: [], + updatedAt: new Date(), + userName: 'Testy', + }, + description: 'Very descriptive text.', + userPermissions: [], + groupPermissions: [], + tags: [], + revisions: Promise.resolve([]), + authorColors: [], + title: 'Title!', + viewcount: 42, }; } @@ -225,10 +169,50 @@ export class NotesService { return this.getCurrentContent(note); } + async toNotePermissionsDto(note: Note): Promise { + return { + owner: this.usersService.toUserDto(note.owner), + sharedToUsers: note.userPermissions.map((noteUserPermission) => ({ + user: this.usersService.toUserDto(noteUserPermission.user), + canEdit: noteUserPermission.canEdit, + })), + sharedToGroups: note.groupPermissions.map((noteGroupPermission) => ({ + group: noteGroupPermission.group, + canEdit: noteGroupPermission.canEdit, + })), + } + } + + async toNoteMetadataDto(note: Note): Promise { + return { + // TODO: Convert DB UUID to base64 + id: note.id, + alias: note.alias, + title: note.title, + createTime: (await this.getFirstRevision(note)).createdAt, + description: note.description, + editedBy: note.authorColors.map( + (authorColor) => authorColor.user.userName, + ), + // TODO: Extract into method + permissions: await this.toNotePermissionsDto(note), + tags: note.tags.map((tag) => tag.name), + updateTime: (await this.getLatestRevision(note)).createdAt, + // TODO: Get actual updateUser + updateUser: { + displayName: 'Hardcoded User', + userName: 'hardcoded', + email: 'foo@example.com', + photo: '', + }, + viewCount: 42, + }; + } + async toNoteDto(note: Note): Promise { return { content: await this.getCurrentContent(note), - metadata: await this.getMetadata(note), + metadata: await this.toNoteMetadataDto(note), editedByAtPosition: [], }; } diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts index 389fe422d..5052b0dc1 100644 --- a/src/revisions/revisions.service.ts +++ b/src/revisions/revisions.service.ts @@ -24,30 +24,28 @@ export class RevisionsService { this.logger.setContext(RevisionsService.name); } - async getNoteRevisionMetadatas( + async getAllRevisions( noteIdOrAlias: string, - ): Promise { + ): Promise { const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); - const revisions = await this.revisionRepository.find({ + return await this.revisionRepository.find({ where: { - note: note.id, + note: note, }, }); - return revisions.map((revision) => this.toMetadataDto(revision)); } - async getNoteRevision( + async getRevision( noteIdOrAlias: string, revisionId: number, - ): Promise { + ): Promise { const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); - const revision = await this.revisionRepository.findOne({ + return await this.revisionRepository.findOne({ where: { id: revisionId, note: note, }, }); - return this.toDto(revision); } getLatestRevision(noteId: string): Promise { @@ -73,7 +71,7 @@ export class RevisionsService { }); } - toMetadataDto(revision: Revision): RevisionMetadataDto { + toRevisionMetadataDto(revision: Revision): RevisionMetadataDto { return { id: revision.id, length: revision.length, @@ -81,7 +79,7 @@ export class RevisionsService { }; } - toDto(revision: Revision): RevisionDto { + toRevisionDto(revision: Revision): RevisionDto { return { id: revision.id, content: revision.content, @@ -90,7 +88,7 @@ export class RevisionsService { }; } - createRevision(content: string) { + createRevision(content: string) : Revision { // TODO: Add previous revision // TODO: Calculate patch // TODO: Save metadata