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 <git@innay.de>

src/api/public/me/me.controller.ts

@@ -78,7 +78,10 @@ export class MeController {
 
   @UseGuards(TokenAuthGuard)
   @Get('notes')
-  getMyNotes(@Request() req): NoteMetadataDto[] {
+  async getMyNotes(@Request() req): Promise<NoteMetadataDto[]> {
-    return this.notesService.getUserNotes(req.user.userName);
+    const notes = await this.notesService.getUserNotes(req.user)
+    return Promise.all(
+      notes.map(note => this.notesService.toNoteMetadataDto(note))
+    );
   }
 }

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<MediaUploadUrlDto> {
     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 {
+      return this.mediaService.toMediaUploadUrlDto(url)
-        link: 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<void> {
     const username = req.user.userName;
     try {
       await this.mediaService.deleteFile(filename, username);

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<ServerStatusDto> {
+    // TODO: toServerStatusDto.
     return this.monitoringService.getServerStatus();
   }
 

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<NoteDto> {
     // 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<NoteDto> {
+    // 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<NoteDto> {
     // 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<void> {
     // 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<NoteDto> {
     // 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<string> {
     // 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<NoteMetadataDto> {
     // 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<NotePermissionsDto> {
     // ToDo: check if user is allowed to view this notes permissions
     try {
-      return await this.noteService.updateNotePermissions(
+      return this.noteService.toNotePermissionsDto(
-        noteIdOrAlias,
+        await this.noteService.updateNotePermissions(
-        updateDto,
+          noteIdOrAlias,
+          updateDto,
+        )
       );
     } catch (e) {
       if (e instanceof NotInDBError) {
@@ -171,12 +187,15 @@ export class NotesController {
   async getNoteRevisions(
     @Request() req,
     @Param('noteIdOrAlias') noteIdOrAlias: string,
-  ) {
+  ) : Promise<RevisionMetadataDto[]> {
     // 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<RevisionDto> {
     // ToDo: check if user is allowed to view this notes revision
     try {
-      return await this.revisionsService.getNoteRevision(
+      return this.revisionsService.toRevisionDto(
-        noteIdOrAlias,
+        await this.revisionsService.getRevision(
-        revisionId,
+          noteIdOrAlias,
+          revisionId,
+        )
       );
     } catch (e) {
       if (e instanceof NotInDBError) {

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
+}

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<string> {
     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<void> {
     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,
+    }
+  }
 }

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: {
+        alias: null,
-          owner: {
+        shortid: "abc",
-            displayName: 'foo',
+        owner: user,
-            userName: 'fooUser',
+        description: 'Very descriptive text.',
-            email: 'foo@example.com',
+        userPermissions: [],
-            photo: '',
+        groupPermissions: [],
-          },
-          sharedToUsers: [],
-          sharedToGroups: [],
-        },
         tags: [],
+        revisions: Promise.resolve([]),
+        authorColors: [],
         title: 'Title!',
-        updateTime: new Date(),
+        viewcount: 42,
-        updateUser: {
-          displayName: 'foo',
-          userName: 'fooUser',
-          email: 'foo@example.com',
-          photo: '',
-        },
-        viewCount: 42,
       },
     ];
   }
 
-  async createNoteDto(
-    noteContent: string,
-    alias?: NoteMetadataDto['alias'],
-    owner?: User,
-  ): Promise<NoteDto> {
-    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<string> {
     return (await this.getLatestRevision(note)).content;
   }
 
@@ -108,42 +86,6 @@ export class NotesService {
     return this.revisionsService.getFirstRevision(note.id);
   }
 
-  async getMetadata(note: Note): Promise<NoteMetadataDto> {
-    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<Note> {
     this.logger.debug(
       `Trying to find note '${noteIdOrAlias}'`,
@@ -178,45 +120,47 @@ export class NotesService {
     return note;
   }
 
-  async getNoteDtoByIdOrAlias(noteIdOrAlias: string): Promise<NoteDto> {
-    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<Note> {
     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.noteRepository.save(note);
-    return this.toNoteDto(note);
-  }
-
-  async getNoteMetadata(noteIdOrAlias: string): Promise<NoteMetadataDto> {
-    const note = await this.getNoteByIdOrAlias(noteIdOrAlias);
-    return this.getMetadata(note);
   }
 
   updateNotePermissions(
     noteIdOrAlias: string,
     newPermissions: NotePermissionsUpdateDto,
-  ): NotePermissionsDto {
+  ): Note {
     this.logger.warn('Using hardcoded data!');
     return {
-      owner: {
+        id: 'foobar-barfoo',
-        displayName: 'foo',
+        alias: null,
-        userName: 'fooUser',
+        shortid: "abc",
-        email: 'foo@example.com',
+        owner: {
-        photo: '',
+          authTokens: [],
-      },
+          createdAt: new Date(),
-      sharedToUsers: [],
+          displayName: 'hardcoded',
-      sharedToGroups: [],
+          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<NotePermissionsDto> {
+    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<NoteMetadataDto> {
+    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<NoteDto> {
     return {
       content: await this.getCurrentContent(note),
-      metadata: await this.getMetadata(note),
+      metadata: await this.toNoteMetadataDto(note),
       editedByAtPosition: [],
     };
   }

src/revisions/revisions.service.ts

@@ -24,30 +24,28 @@ export class RevisionsService {
     this.logger.setContext(RevisionsService.name);
   }
 
-  async getNoteRevisionMetadatas(
+  async getAllRevisions(
     noteIdOrAlias: string,
-  ): Promise<RevisionMetadataDto[]> {
+  ): Promise<Revision[]> {
     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<RevisionDto> {
+  ): Promise<Revision> {
     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<Revision> {
@@ -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