From b76fa91a3c80792aca118c94e2cb9b1744b2a69f Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 3 Feb 2021 21:15:39 +0100 Subject: [PATCH 1/6] History: Add HistoryEntry With this the backend now can hold a history entry. Also included in this commit are some minor changes to tests and services so they can still work. Signed-off-by: Philip Molares --- docs/content/dev/db-schema.plantuml | 10 ++++++ src/api/public/me/me.controller.spec.ts | 3 ++ src/auth/auth.service.spec.ts | 1 + src/history/history-entry.entity.ts | 47 +++++++++++++++++++++++++ src/notes/note.entity.ts | 3 ++ src/notes/notes.service.ts | 5 +++ src/users/user.entity.ts | 5 +-- 7 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 src/history/history-entry.entity.ts diff --git a/docs/content/dev/db-schema.plantuml b/docs/content/dev/db-schema.plantuml index ca8d0a3c6..b64bce37f 100644 --- a/docs/content/dev/db-schema.plantuml +++ b/docs/content/dev/db-schema.plantuml @@ -134,11 +134,20 @@ entity "media_upload" { *createdAt : date } +entity "history_entry" { + *noteId : uuid <> + *userId : uuid <> + -- + *pinStatus: boolean + *updatedAt: date +} + user "1" -- "0..*" note: owner user "1" -u- "1..*" identity user "1" - "1..*" auth_token: authTokens user "1" -l- "1..*" session user "1" - "0..*" media_upload +user "1" - "0..*" history_entry user "0..*" -- "0..*" note user "1" - "0..*" authorship @@ -149,6 +158,7 @@ revision "0..*" - "0..*" authorship media_upload "0..*" -- "1" note note "1" - "1..*" revision +note "1" - "0..*" history_entry note "0..*" -l- "0..*" tag note "0..*" -- "0..*" group diff --git a/src/api/public/me/me.controller.spec.ts b/src/api/public/me/me.controller.spec.ts index e3f262396..a71d73dcf 100644 --- a/src/api/public/me/me.controller.spec.ts +++ b/src/api/public/me/me.controller.spec.ts @@ -19,6 +19,7 @@ import { Identity } from '../../../users/identity.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; import { MeController } from './me.controller'; +import { HistoryEntry } from '../../../history/history-entry.entity'; describe('Me Controller', () => { let controller: MeController; @@ -44,6 +45,8 @@ describe('Me Controller', () => { .useValue({}) .overrideProvider(getRepositoryToken(Tag)) .useValue({}) + .overrideProvider(getRepositoryToken(HistoryEntry)) + .useValue({}) .compile(); controller = module.get(MeController); diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index b5a7e2c1d..a8ca9ffcb 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -28,6 +28,7 @@ describe('AuthService', () => { id: '1', identities: [], ownedNotes: [], + historyEntries: [], updatedAt: new Date(), userName: 'Testy', }; diff --git a/src/history/history-entry.entity.ts b/src/history/history-entry.entity.ts new file mode 100644 index 000000000..597e137b4 --- /dev/null +++ b/src/history/history-entry.entity.ts @@ -0,0 +1,47 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { + Column, + Entity, + ManyToOne, + UpdateDateColumn, +} from 'typeorm'; +import { User } from '../users/user.entity'; +import { Note } from '../notes/note.entity'; + +@Entity() +export class HistoryEntry { + @ManyToOne((_) => User, (user) => user.historyEntries, { + onDelete: 'CASCADE', + primary: true, + }) + user: User; + + @ManyToOne((_) => Note, (note) => note.historyEntries, { + onDelete: 'CASCADE', + primary: true, + }) + note: Note; + + @Column() + pinStatus: boolean; + + @UpdateDateColumn() + updatedAt: Date; + + // The optional note parameter is necessary for the createNote method in the NotesService, + // as we create the note then and don't need to add it to the HistoryEntry. + public static create(user: User, note?: Note): HistoryEntry { + const newHistoryEntry = new HistoryEntry(); + newHistoryEntry.user = user; + if (note) { + newHistoryEntry.note = note; + } + newHistoryEntry.pinStatus = false; + return newHistoryEntry; + } +} diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 27424dd95..ed2c2aae6 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -20,6 +20,7 @@ import { Revision } from '../revisions/revision.entity'; import { User } from '../users/user.entity'; import { AuthorColor } from './author-color.entity'; import { Tag } from './tag.entity'; +import { HistoryEntry } from '../history/history-entry.entity'; @Entity() export class Note { @@ -53,6 +54,8 @@ export class Note { revisions: Promise; @OneToMany((_) => AuthorColor, (authorColor) => authorColor.note) authorColors: AuthorColor[]; + @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) + historyEntries: HistoryEntry[]; @Column({ nullable: true, diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 0e3998987..8a377f482 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -21,6 +21,7 @@ import { import { NoteDto } from './note.dto'; import { Note } from './note.entity'; import { Tag } from './tag.entity'; +import { HistoryEntry } from '../history/history-entry.entity'; @Injectable() export class NotesService { @@ -46,6 +47,7 @@ export class NotesService { description: 'Very descriptive text.', userPermissions: [], groupPermissions: [], + historyEntries: [], tags: [], revisions: Promise.resolve([]), authorColors: [], @@ -69,6 +71,7 @@ export class NotesService { newNote.alias = alias; } if (owner) { + newNote.historyEntries = [HistoryEntry.create(owner)]; newNote.owner = owner; } return this.noteRepository.save(newNote); @@ -153,12 +156,14 @@ export class NotesService { id: '1', identities: [], ownedNotes: [], + historyEntries: [], updatedAt: new Date(), userName: 'Testy', }, description: 'Very descriptive text.', userPermissions: [], groupPermissions: [], + historyEntries: [], tags: [], revisions: Promise.resolve([]), authorColors: [], diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index cd0ab9c84..ba8016495 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -14,6 +14,7 @@ import { Column, OneToMany } from 'typeorm'; import { Note } from '../notes/note.entity'; import { AuthToken } from '../auth/auth-token.entity'; import { Identity } from './identity.entity'; +import { HistoryEntry } from '../history/history-entry.entity'; @Entity() export class User { @@ -51,8 +52,8 @@ export class User { @OneToMany((_) => Identity, (identity) => identity.user) identities: Identity[]; - // eslint-disable-next-line @typescript-eslint/no-empty-function - private constructor() {} + @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) + historyEntries: HistoryEntry[]; public static create( userName: string, From 7f1afc30c9d31f6b640ece0f46003639d68b6934 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 3 Feb 2021 21:22:55 +0100 Subject: [PATCH 2/6] History: Add history service and usage Add history service to allow for CRUD operations. Use history service in controllers to: 1. Allow manipulating of history entries 2. Guaranty the correct existence of history entries Signed-off-by: Philip Molares --- src/api/public/me/me.controller.ts | 40 ++++-- src/api/public/notes/notes.controller.ts | 37 +++--- src/history/history-entry.dto.ts | 28 +++- src/history/history-entry.entity.ts | 7 +- src/history/history.module.ts | 11 +- src/history/history.service.ts | 158 ++++++++++++----------- src/notes/notes.service.ts | 6 +- test/public-api/users.e2e-spec.ts | 4 +- 8 files changed, 173 insertions(+), 118 deletions(-) diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 4bbf8ccdb..2cb412f2b 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -17,15 +17,16 @@ import { Request, } from '@nestjs/common'; import { HistoryEntryUpdateDto } from '../../../history/history-entry-update.dto'; -import { HistoryEntryDto } from '../../../history/history-entry.dto'; import { HistoryService } from '../../../history/history.service'; import { ConsoleLoggerService } from '../../../logger/console-logger.service'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; import { NotesService } from '../../../notes/notes.service'; -import { UserInfoDto } from '../../../users/user-info.dto'; import { UsersService } from '../../../users/users.service'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; import { ApiSecurity } from '@nestjs/swagger'; +import { HistoryEntryDto } from '../../../history/history-entry.dto'; +import { UserInfoDto } from '../../../users/user-info.dto'; +import { NotInDBError } from '../../../errors/errors'; @ApiSecurity('token') @Controller('me') @@ -49,19 +50,37 @@ export class MeController { @UseGuards(TokenAuthGuard) @Get('history') - getUserHistory(@Request() req): HistoryEntryDto[] { - return this.historyService.getUserHistory(req.user.userName); + async getUserHistory(@Request() req): Promise { + const foundEntries = await this.historyService.getEntriesByUser(req.user); + return Promise.all( + foundEntries.map( + async (entry) => await this.historyService.toHistoryEntryDto(entry), + ), + ); } @UseGuards(TokenAuthGuard) @Put('history/:note') - updateHistoryEntry( + async updateHistoryEntry( @Request() req, @Param('note') note: string, @Body() entryUpdateDto: HistoryEntryUpdateDto, - ): HistoryEntryDto { + ): Promise { // ToDo: Check if user is allowed to pin this history entry - return this.historyService.updateHistoryEntry(note, entryUpdateDto); + try { + return this.historyService.toHistoryEntryDto( + await this.historyService.updateHistoryEntry( + note, + req.user, + entryUpdateDto, + ), + ); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + throw e; + } } @UseGuards(TokenAuthGuard) @@ -70,9 +89,12 @@ export class MeController { deleteHistoryEntry(@Request() req, @Param('note') note: string) { // ToDo: Check if user is allowed to delete note try { - return this.historyService.deleteHistoryEntry(note); + return this.historyService.deleteHistoryEntry(note, req.user); } catch (e) { - throw new NotFoundException(e.message); + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + throw e; } } diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index 4478356d5..b4f53e01c 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -28,6 +28,7 @@ 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 { HistoryService } from '../../../history/history.service'; import { NoteDto } from '../../../notes/note.dto'; import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto'; @@ -40,6 +41,7 @@ export class NotesController { private readonly logger: ConsoleLoggerService, private noteService: NotesService, private revisionsService: RevisionsService, + private historyService: HistoryService, ) { this.logger.setContext(NotesController.name); } @@ -57,6 +59,22 @@ export class NotesController { ); } + @UseGuards(TokenAuthGuard) + @Get(':noteIdOrAlias') + async getNote(@Request() req, @Param('noteIdOrAlias') noteIdOrAlias: string) { + // ToDo: check if user is allowed to view this note + try { + const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); + await this.historyService.createOrUpdateHistoryEntry(note, req.user); + return this.noteService.toNoteDto(note); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + throw e; + } + } + @UseGuards(TokenAuthGuard) @Post(':noteAlias') async createNamedNote( @@ -71,25 +89,6 @@ export class NotesController { ); } - @UseGuards(TokenAuthGuard) - @Get(':noteIdOrAlias') - async getNote( - @Request() req, - @Param('noteIdOrAlias') noteIdOrAlias: string, - ): Promise { - // ToDo: check if user is allowed to view this note - try { - return this.noteService.toNoteDto( - await this.noteService.getNoteByIdOrAlias(noteIdOrAlias), - ); - } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); - } - throw e; - } - } - @UseGuards(TokenAuthGuard) @Delete(':noteIdOrAlias') async deleteNote( diff --git a/src/history/history-entry.dto.ts b/src/history/history-entry.dto.ts index b990c92e4..2312c5a21 100644 --- a/src/history/history-entry.dto.ts +++ b/src/history/history-entry.dto.ts @@ -4,15 +4,33 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { IsBoolean, ValidateNested } from 'class-validator'; -import { NoteMetadataDto } from '../notes/note-metadata.dto'; +import { IsArray, IsBoolean, IsDate, IsString } from 'class-validator'; export class HistoryEntryDto { /** - * Metadata of this note + * ID or Alias of the note */ - @ValidateNested() - metadata: NoteMetadataDto; + @IsString() + identifier: string; + + /** + * Title of the note + * Does not contain any markup but might be empty + * @example "Shopping List" + */ + @IsString() + title: string; + + /** + * Datestring of the last time this note was updated + * @example "2020-12-01 12:23:34" + */ + @IsDate() + lastVisited: Date; + + @IsArray() + @IsString({ each: true }) + tags: string[]; /** * True if this note is pinned diff --git a/src/history/history-entry.entity.ts b/src/history/history-entry.entity.ts index 597e137b4..11e59888c 100644 --- a/src/history/history-entry.entity.ts +++ b/src/history/history-entry.entity.ts @@ -4,12 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { - Column, - Entity, - ManyToOne, - UpdateDateColumn, -} from 'typeorm'; +import { Column, Entity, ManyToOne, UpdateDateColumn } from 'typeorm'; import { User } from '../users/user.entity'; import { Note } from '../notes/note.entity'; diff --git a/src/history/history.module.ts b/src/history/history.module.ts index 507845309..410d50863 100644 --- a/src/history/history.module.ts +++ b/src/history/history.module.ts @@ -7,10 +7,19 @@ import { Module } from '@nestjs/common'; import { LoggerModule } from '../logger/logger.module'; import { HistoryService } from './history.service'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import { HistoryEntry } from './history-entry.entity'; +import { UsersModule } from '../users/users.module'; +import { NotesModule } from '../notes/notes.module'; @Module({ providers: [HistoryService], exports: [HistoryService], - imports: [LoggerModule], + imports: [ + LoggerModule, + TypeOrmModule.forFeature([HistoryEntry]), + UsersModule, + NotesModule, + ], }) export class HistoryModule {} diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 2a0c9d736..16343b16d 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -8,90 +8,98 @@ import { Injectable } from '@nestjs/common'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { HistoryEntryUpdateDto } from './history-entry-update.dto'; import { HistoryEntryDto } from './history-entry.dto'; +import { InjectRepository } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; +import { HistoryEntry } from './history-entry.enity'; +import { UsersService } from '../users/users.service'; +import { NotesService } from '../notes/notes.service'; +import { User } from '../users/user.entity'; +import { Note } from '../notes/note.entity'; +import { NotInDBError } from '../errors/errors'; @Injectable() export class HistoryService { - constructor(private readonly logger: ConsoleLoggerService) { + constructor( + private readonly logger: ConsoleLoggerService, + @InjectRepository(HistoryEntry) + private historyEntryRepository: Repository, + private usersService: UsersService, + private notesService: NotesService, + ) { this.logger.setContext(HistoryService.name); } - getUserHistory(username: string): HistoryEntryDto[] { - //TODO: Use the database - this.logger.warn('Using hardcoded data!'); - return [ - { - metadata: { - 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: [], - }, - tags: [], - title: 'Title!', - updateTime: new Date(), - updateUser: { - displayName: 'foo', - userName: 'fooUser', - email: 'foo@example.com', - photo: '', - }, - viewCount: 42, - }, - pinStatus: false, - }, - ]; + async getEntriesByUser(user: User): Promise { + return await this.historyEntryRepository.find({ + where: { user: user }, + relations: ['note'], + }); } - updateHistoryEntry( - noteId: string, - updateDto: HistoryEntryUpdateDto, - ): HistoryEntryDto { - //TODO: Use the database - this.logger.warn('Using hardcoded data!'); - return { - metadata: { - 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: [], - }, - tags: [], - title: 'Title!', - updateTime: new Date(), - updateUser: { - displayName: 'foo', - userName: 'fooUser', - email: 'foo@example.com', - photo: '', - }, - viewCount: 42, + private async getEntryByNoteIdOrAlias( + noteIdOrAlias: string, + user: User, + ): Promise { + const note = await this.notesService.getNoteByIdOrAlias(noteIdOrAlias); + return await this.getEntryByNote(note, user); + } + + private async getEntryByNote(note: Note, user: User): Promise { + return await this.historyEntryRepository.findOne({ + where: { + note: note, + user: user, }, - pinStatus: updateDto.pinStatus, + relations: ['note', 'user'], + }); + } + + async createOrUpdateHistoryEntry( + note: Note, + user: User, + ): Promise { + let entry = await this.getEntryByNote(note, user); + if (!entry) { + entry = HistoryEntry.create(user, note); + } else { + entry.updatedAt = new Date(); + } + return await this.historyEntryRepository.save(entry); + } + + async updateHistoryEntry( + noteIdOrAlias: string, + user: User, + updateDto: HistoryEntryUpdateDto, + ): Promise { + const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); + if (!entry) { + throw new NotInDBError( + `User '${user.userName}' has no HistoryEntry for Note with id '${noteIdOrAlias}'`, + ); + } + entry.pinStatus = updateDto.pinStatus; + return this.historyEntryRepository.save(entry); + } + + async deleteHistoryEntry(noteIdOrAlias: string, user: User): Promise { + const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); + if (!entry) { + throw new NotInDBError( + `User '${user.userName}' has no HistoryEntry for Note with id '${noteIdOrAlias}'`, + ); + } + await this.historyEntryRepository.remove(entry); + return; + } + + async toHistoryEntryDto(entry: HistoryEntry): Promise { + return { + identifier: entry.note.alias ? entry.note.alias : entry.note.id, + lastVisited: entry.updatedAt, + tags: this.notesService.toTagList(entry.note), + title: entry.note.title, + pinStatus: entry.pinStatus, }; } - - deleteHistoryEntry(note: string) { - //TODO: Use the database and actually do stuff - throw new Error('Not implemented'); - } } diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 8a377f482..aa03546ce 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -177,6 +177,10 @@ export class NotesService { return this.getCurrentContent(note); } + toTagList(note: Note): string[] { + return note.tags.map((tag) => tag.name); + } + async toNotePermissionsDto(note: Note): Promise { return { owner: this.usersService.toUserDto(note.owner), @@ -204,7 +208,7 @@ export class NotesService { ), // TODO: Extract into method permissions: await this.toNotePermissionsDto(note), - tags: note.tags.map((tag) => tag.name), + tags: this.toTagList(note), updateTime: (await this.getLatestRevision(note)).createdAt, // TODO: Get actual updateUser updateUser: { diff --git a/test/public-api/users.e2e-spec.ts b/test/public-api/users.e2e-spec.ts index fd4689c3d..4bcea119c 100644 --- a/test/public-api/users.e2e-spec.ts +++ b/test/public-api/users.e2e-spec.ts @@ -85,7 +85,7 @@ describe('Notes', () => { .delete('/me/history/test3') .expect(204); expect(response.body.content).toBeNull(); - const history = historyService.getUserHistory('testuser'); + const history = historyService.getEntriesByUser('testuser'); let historyEntry: HistoryEntryDto = null; for (const e of history) { if (e.metadata.alias === noteName) { @@ -106,7 +106,7 @@ describe('Notes', () => { .send(historyEntryUpdateDto) .expect(200); // TODO parameter is not used for now - const history = historyService.getUserHistory('testuser'); + const history = historyService.getEntriesByUser('testuser'); let historyEntry: HistoryEntryDto; for (const e of response.body.content) { if ((e).metadata.alias === noteName) { From 10ef4fcee15f9668cd77208c492bf4e8ab75d060 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 3 Feb 2021 21:46:36 +0100 Subject: [PATCH 3/6] History: Add unit and e2e test Add unit tests for history service Adapt relevant me e2e tests to work Signed-off-by: Philip Molares --- src/api/public/notes/notes.controller.spec.ts | 6 +- src/history/history.service.spec.ts | 253 +++++++++++++++++- src/history/history.service.ts | 2 +- test/public-api/users.e2e-spec.ts | 148 +++++----- 4 files changed, 339 insertions(+), 70 deletions(-) diff --git a/src/api/public/notes/notes.controller.spec.ts b/src/api/public/notes/notes.controller.spec.ts index 13ae993bc..6df9c73cb 100644 --- a/src/api/public/notes/notes.controller.spec.ts +++ b/src/api/public/notes/notes.controller.spec.ts @@ -19,6 +19,8 @@ import { Identity } from '../../../users/identity.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; import { NotesController } from './notes.controller'; +import { HistoryModule } from '../../../history/history.module'; +import { HistoryEntry } from '../../../history/history-entry.entity'; describe('Notes Controller', () => { let controller: NotesController; @@ -37,7 +39,7 @@ describe('Notes Controller', () => { useValue: {}, }, ], - imports: [RevisionsModule, UsersModule, LoggerModule], + imports: [RevisionsModule, UsersModule, LoggerModule, HistoryModule], }) .overrideProvider(getRepositoryToken(Note)) .useValue({}) @@ -57,6 +59,8 @@ describe('Notes Controller', () => { .useValue({}) .overrideProvider(getRepositoryToken(Tag)) .useValue({}) + .overrideProvider(getRepositoryToken(HistoryEntry)) + .useValue({}) .compile(); controller = module.get(NotesController); diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index ba6ba2b6e..ccf0ba04e 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -7,20 +7,267 @@ import { Test, TestingModule } from '@nestjs/testing'; import { LoggerModule } from '../logger/logger.module'; import { HistoryService } from './history.service'; +import { UsersModule } from '../users/users.module'; +import { NotesModule } from '../notes/notes.module'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { Identity } from '../users/identity.entity'; +import { User } from '../users/user.entity'; +import { AuthorColor } from '../notes/author-color.entity'; +import { Authorship } from '../revisions/authorship.entity'; +import { HistoryEntry } from './history-entry.entity'; +import { Note } from '../notes/note.entity'; +import { Tag } from '../notes/tag.entity'; +import { AuthToken } from '../auth/auth-token.entity'; +import { Revision } from '../revisions/revision.entity'; +import { Repository } from 'typeorm'; +import { NotInDBError } from '../errors/errors'; describe('HistoryService', () => { let service: HistoryService; + let historyRepo: Repository; + let noteRepo: Repository; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ - providers: [HistoryService], - imports: [LoggerModule], - }).compile(); + providers: [ + HistoryService, + { + provide: getRepositoryToken(HistoryEntry), + useClass: Repository, + }, + ], + imports: [LoggerModule, UsersModule, NotesModule], + }) + .overrideProvider(getRepositoryToken(User)) + .useValue({}) + .overrideProvider(getRepositoryToken(AuthToken)) + .useValue({}) + .overrideProvider(getRepositoryToken(Identity)) + .useValue({}) + .overrideProvider(getRepositoryToken(Authorship)) + .useValue({}) + .overrideProvider(getRepositoryToken(AuthorColor)) + .useValue({}) + .overrideProvider(getRepositoryToken(Revision)) + .useValue({}) + .overrideProvider(getRepositoryToken(Note)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(Tag)) + .useValue({}) + .compile(); service = module.get(HistoryService); + historyRepo = module.get>( + getRepositoryToken(HistoryEntry), + ); + noteRepo = module.get>(getRepositoryToken(Note)); }); it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('getEntriesByUser', () => { + describe('works', () => { + it('with an empty list', async () => { + jest.spyOn(historyRepo, 'find').mockResolvedValueOnce([]); + expect(await service.getEntriesByUser({} as User)).toEqual([]); + }); + + it('with an one element list', async () => { + const historyEntry = new HistoryEntry(); + jest.spyOn(historyRepo, 'find').mockResolvedValueOnce([historyEntry]); + expect(await service.getEntriesByUser({} as User)).toEqual([ + historyEntry, + ]); + }); + + it('with an multiple element list', async () => { + const historyEntry = new HistoryEntry(); + const historyEntry2 = new HistoryEntry(); + jest + .spyOn(historyRepo, 'find') + .mockResolvedValueOnce([historyEntry, historyEntry2]); + expect(await service.getEntriesByUser({} as User)).toEqual([ + historyEntry, + historyEntry2, + ]); + }); + }); + }); + + describe('createOrUpdateHistoryEntry', () => { + describe('works', () => { + it('without an preexisting entry', async () => { + const user = new User(); + const alias = 'alias'; + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); + jest + .spyOn(historyRepo, 'save') + .mockImplementation( + async (entry: HistoryEntry): Promise => entry, + ); + const createHistoryEntry = await service.createOrUpdateHistoryEntry( + Note.create(user, alias), + user, + ); + expect(createHistoryEntry.note.alias).toEqual(alias); + expect(createHistoryEntry.note.owner).toEqual(user); + expect(createHistoryEntry.user).toEqual(user); + expect(createHistoryEntry.pinStatus).toEqual(false); + }); + + it('with an preexisting entry', async () => { + const user = new User(); + const alias = 'alias'; + const historyEntry = HistoryEntry.create( + user, + Note.create(user, alias), + ); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); + jest + .spyOn(historyRepo, 'save') + .mockImplementation( + async (entry: HistoryEntry): Promise => entry, + ); + const createHistoryEntry = await service.createOrUpdateHistoryEntry( + Note.create(user, alias), + user, + ); + expect(createHistoryEntry.note.alias).toEqual(alias); + expect(createHistoryEntry.note.owner).toEqual(user); + expect(createHistoryEntry.user).toEqual(user); + expect(createHistoryEntry.pinStatus).toEqual(false); + expect(createHistoryEntry.updatedAt.getTime()).toBeGreaterThanOrEqual( + historyEntry.updatedAt.getTime(), + ); + }); + }); + }); + + describe('updateHistoryEntry', () => { + describe('works', () => { + it('with an entry', async () => { + const user = new User(); + const alias = 'alias'; + const note = Note.create(user, alias); + const historyEntry = HistoryEntry.create(user, note); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + jest + .spyOn(historyRepo, 'save') + .mockImplementation( + async (entry: HistoryEntry): Promise => entry, + ); + const updatedHistoryEntry = await service.updateHistoryEntry( + alias, + user, + { + pinStatus: true, + }, + ); + expect(updatedHistoryEntry.note.alias).toEqual(alias); + expect(updatedHistoryEntry.note.owner).toEqual(user); + expect(updatedHistoryEntry.user).toEqual(user); + expect(updatedHistoryEntry.pinStatus).toEqual(true); + }); + + it('without an entry', async () => { + const user = new User(); + const alias = 'alias'; + const note = Note.create(user, alias); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + try { + await service.updateHistoryEntry(alias, user, { + pinStatus: true, + }); + } catch (e) { + expect(e).toBeInstanceOf(NotInDBError); + } + }); + }); + }); + + describe('deleteHistoryEntry', () => { + describe('works', () => { + it('with an entry', async () => { + const user = new User(); + const alias = 'alias'; + const note = Note.create(user, alias); + const historyEntry = HistoryEntry.create(user, note); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + jest.spyOn(historyRepo, 'remove').mockImplementation( + async (entry: HistoryEntry): Promise => { + expect(entry).toEqual(historyEntry); + return entry; + }, + ); + await service.deleteHistoryEntry(alias, user); + }); + + it('without an entry', async () => { + const user = new User(); + const alias = 'alias'; + const note = Note.create(user, alias); + jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + try { + await service.deleteHistoryEntry(alias, user); + } catch (e) { + expect(e).toBeInstanceOf(NotInDBError); + } + }); + }); + }); + + describe('toHistoryEntryDto', () => { + describe('works', () => { + it('with aliased note', async () => { + const user = new User(); + const alias = 'alias'; + const title = 'title'; + const tags = ['tag1', 'tag2']; + const note = Note.create(user, alias); + note.title = title; + note.tags = tags.map((tag) => { + const newTag = new Tag(); + newTag.name = tag; + return newTag; + }); + const historyEntry = HistoryEntry.create(user, note); + historyEntry.pinStatus = true; + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const historyEntryDto = await service.toHistoryEntryDto(historyEntry); + expect(historyEntryDto.pinStatus).toEqual(true); + expect(historyEntryDto.identifier).toEqual(alias); + expect(historyEntryDto.tags).toEqual(tags); + expect(historyEntryDto.title).toEqual(title); + }); + + it('with regular note', async () => { + const user = new User(); + const title = 'title'; + const id = 'id'; + const tags = ['tag1', 'tag2']; + const note = Note.create(user); + note.title = title; + note.id = id; + note.tags = tags.map((tag) => { + const newTag = new Tag(); + newTag.name = tag; + return newTag; + }); + const historyEntry = HistoryEntry.create(user, note); + historyEntry.pinStatus = true; + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const historyEntryDto = await service.toHistoryEntryDto(historyEntry); + expect(historyEntryDto.pinStatus).toEqual(true); + expect(historyEntryDto.identifier).toEqual(id); + expect(historyEntryDto.tags).toEqual(tags); + expect(historyEntryDto.title).toEqual(title); + }); + }); + }); }); diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 16343b16d..84583220e 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -10,7 +10,7 @@ import { HistoryEntryUpdateDto } from './history-entry-update.dto'; import { HistoryEntryDto } from './history-entry.dto'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; -import { HistoryEntry } from './history-entry.enity'; +import { HistoryEntry } from './history-entry.entity'; import { UsersService } from '../users/users.service'; import { NotesService } from '../notes/notes.service'; import { User } from '../users/user.entity'; diff --git a/test/public-api/users.e2e-spec.ts b/test/public-api/users.e2e-spec.ts index 4bcea119c..4c406ccb4 100644 --- a/test/public-api/users.e2e-spec.ts +++ b/test/public-api/users.e2e-spec.ts @@ -7,28 +7,68 @@ import { INestApplication } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import * as request from 'supertest'; -import { AppModule } from '../../src/app.module'; import { UserInfoDto } from '../../src/users/user-info.dto'; import { HistoryService } from '../../src/history/history.service'; import { NotesService } from '../../src/notes/notes.service'; import { HistoryEntryUpdateDto } from '../../src/history/history-entry-update.dto'; import { HistoryEntryDto } from '../../src/history/history-entry.dto'; +import { HistoryEntry } from '../../src/history/history-entry.entity'; +import { UsersService } from '../../src/users/users.service'; +import { TokenAuthGuard } from '../../src/auth/token-auth.guard'; +import { MockAuthGuard } from '../../src/auth/mock-auth.guard'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import { PublicApiModule } from '../../src/api/public/public-api.module'; +import { NotesModule } from '../../src/notes/notes.module'; +import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { GroupsModule } from '../../src/groups/groups.module'; +import { LoggerModule } from '../../src/logger/logger.module'; +import { AuthModule } from '../../src/auth/auth.module'; +import { UsersModule } from '../../src/users/users.module'; +import { HistoryModule } from '../../src/history/history.module'; +import { ConfigModule } from '@nestjs/config'; +import mediaConfigMock from '../../src/config/media.config.mock'; +import { User } from '../../src/users/user.entity'; // TODO Tests have to be reworked using UserService functions describe('Notes', () => { let app: INestApplication; - //let usersService: UsersService; let historyService: HistoryService; let notesService: NotesService; + let user: User; beforeAll(async () => { const moduleRef = await Test.createTestingModule({ - imports: [AppModule], - }).compile(); - // TODO Create User and generateAPI Token or other Auth + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [mediaConfigMock], + }), + PublicApiModule, + NotesModule, + PermissionsModule, + GroupsModule, + TypeOrmModule.forRoot({ + type: 'sqlite', + database: './hedgedoc-e2e-me.sqlite', + autoLoadEntities: true, + synchronize: true, + dropSchema: true, + }), + LoggerModule, + AuthModule, + UsersModule, + HistoryModule, + ], + }) + .overrideGuard(TokenAuthGuard) + .useClass(MockAuthGuard) + .compile(); app = moduleRef.createNestApplication(); - //usersService = moduleRef.get(UsersService); + notesService = moduleRef.get(NotesService); + historyService = moduleRef.get(HistoryService); + const userService = moduleRef.get(UsersService); + user = await userService.createUser('hardcoded', 'Testy'); await app.init(); }); @@ -42,87 +82,65 @@ describe('Notes', () => { expect(response.body.content).toEqual(userInfo); }); - it.skip(`GET /me/history`, async () => { - // TODO user has to be chosen - /* TODO Note maybe not added to history by createNote, - use function from HistoryService instead - */ - await notesService.createNote('', 'testGetHistory'); + it(`GET /me/history`, async () => { + const noteName = 'testGetNoteHistory1'; + const note = await notesService.createNote('', noteName); + const createdHistoryEntry = await historyService.createOrUpdateHistoryEntry( + note, + user, + ); const response = await request(app.getHttpServer()) .get('/me/history') .expect('Content-Type', /json/) .expect(200); - let historyEntry: HistoryEntryDto; - for (const e of response.body.content) { - if ((e).metadata.alias === 'testGetHistory') { - historyEntry = e; + const history = response.body; + for (const historyEntry of history) { + if ((historyEntry).identifier === 'testGetHistory') { + expect(historyEntry).toEqual(createdHistoryEntry); } } - expect(historyEntry).toEqual(history); }); - it.skip(`GET /me/history/{note}`, async () => { - const noteName = 'testGetNoteHistory'; - /* TODO Note maybe not added to history by createNote, - use function from HistoryService instead - */ - await notesService.createNote('', noteName); - const response = await request(app.getHttpServer()) - .get('/me/history/' + noteName) - .expect('Content-Type', /json/) - .expect(200); - expect(response.body.metadata?.id).toBeDefined(); - return expect(response.body.metadata.alias).toEqual(noteName); - }); - - it.skip(`DELETE /me/history/{note}`, async () => { - const noteName = 'testDeleteNoteHistory'; - /* TODO Note maybe not added to history by createNote, - use function from HistoryService instead - */ - await notesService.createNote('This is a test note.', noteName); - const response = await request(app.getHttpServer()) - .delete('/me/history/test3') - .expect(204); - expect(response.body.content).toBeNull(); - const history = historyService.getEntriesByUser('testuser'); - let historyEntry: HistoryEntryDto = null; - for (const e of history) { - if (e.metadata.alias === noteName) { - historyEntry = e; - } - } - return expect(historyEntry).toBeNull(); - }); - - it.skip(`PUT /me/history/{note}`, async () => { - const noteName = 'testPutNoteHistory'; - // TODO use function from HistoryService to add an History Entry - await notesService.createNote('', noteName); + it(`PUT /me/history/{note}`, async () => { + const noteName = 'testGetNoteHistory2'; + const note = await notesService.createNote('', noteName); + await historyService.createOrUpdateHistoryEntry(note, user); const historyEntryUpdateDto = new HistoryEntryUpdateDto(); historyEntryUpdateDto.pinStatus = true; const response = await request(app.getHttpServer()) .put('/me/history/' + noteName) .send(historyEntryUpdateDto) .expect(200); - // TODO parameter is not used for now - const history = historyService.getEntriesByUser('testuser'); - let historyEntry: HistoryEntryDto; - for (const e of response.body.content) { - if ((e).metadata.alias === noteName) { - historyEntry = e; - } - } + const history = await historyService.getEntriesByUser(user); + let historyEntry: HistoryEntryDto = response.body; expect(historyEntry.pinStatus).toEqual(true); historyEntry = null; for (const e of history) { - if (e.metadata.alias === noteName) { - historyEntry = e; + if (e.note.alias === noteName) { + historyEntry = await historyService.toHistoryEntryDto(e); } } expect(historyEntry.pinStatus).toEqual(true); }); + it(`DELETE /me/history/{note}`, async () => { + const noteName = 'testGetNoteHistory3'; + const note = await notesService.createNote('', noteName); + await historyService.createOrUpdateHistoryEntry(note, user); + const response = await request(app.getHttpServer()) + .delete(`/me/history/${noteName}`) + .expect(204); + expect(response.body).toEqual({}); + const history = await historyService.getEntriesByUser(user); + let historyEntry: HistoryEntry = null; + for (const e of history) { + if ((e).note.alias === noteName) { + historyEntry = e; + } + } + return expect(historyEntry).toBeNull(); + }); + it.skip(`GET /me/notes/`, async () => { // TODO use function from HistoryService to add an History Entry await notesService.createNote('This is a test note.', 'test7'); From 1becc9b3d2e462ca8b3516335366589c2a2d5aa5 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 3 Feb 2021 21:49:39 +0100 Subject: [PATCH 4/6] Tests: Fix Mock Auth This makes it possible to create the user before the mock auth guard does it's magic. This is necessary for some test, where we need the user object before the api is called. Signed-off-by: Philip Molares --- src/auth/mock-auth.guard.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/auth/mock-auth.guard.ts b/src/auth/mock-auth.guard.ts index d4f1236b0..5f1e40c6b 100644 --- a/src/auth/mock-auth.guard.ts +++ b/src/auth/mock-auth.guard.ts @@ -16,7 +16,13 @@ export class MockAuthGuard { async canActivate(context: ExecutionContext) { const req = context.switchToHttp().getRequest(); if (!this.user) { - this.user = await this.usersService.createUser('hardcoded', 'Testy'); + // this assures that we can create the user 'hardcoded', if we need them before any calls are made or + // create them on the fly when the first call to the api is made + try { + this.user = await this.usersService.getUserByUsername('hardcoded'); + } catch (e) { + this.user = await this.usersService.createUser('hardcoded', 'Testy'); + } } req.user = this.user; return true; From 24ccb1fcd00f51fd7ea96e17e3441779895f2ef0 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 3 Feb 2021 21:51:30 +0100 Subject: [PATCH 5/6] Tests: Rename users.e2e-spec.ts to me.e2e-spec.ts As users.e2e-spec.ts tests the MeController this commit renames the test appropriately Signed-off-by: Philip Molares --- test/public-api/{users.e2e-spec.ts => me.e2e-spec.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/public-api/{users.e2e-spec.ts => me.e2e-spec.ts} (100%) diff --git a/test/public-api/users.e2e-spec.ts b/test/public-api/me.e2e-spec.ts similarity index 100% rename from test/public-api/users.e2e-spec.ts rename to test/public-api/me.e2e-spec.ts From 88ed1ec8baf3df21dc39566a5f7f87ba9039f40d Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 4 Feb 2021 13:44:08 +0100 Subject: [PATCH 6/6] Docs: Add api tags to group controller For a better structure of the autogenerated apidoc website tags are used. Each Controller get it's own tag and will be put in a separate section. See https://docs.nestjs.com/openapi/operations#tags Signed-off-by: Philip Molares --- src/api/private/tokens/tokens.controller.ts | 2 ++ src/api/public/me/me.controller.ts | 3 ++- src/api/public/media/media.controller.ts | 3 ++- src/api/public/monitoring/monitoring.controller.ts | 3 ++- src/api/public/notes/notes.controller.ts | 3 ++- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/api/private/tokens/tokens.controller.ts b/src/api/private/tokens/tokens.controller.ts index e5aaf8e30..651982b13 100644 --- a/src/api/private/tokens/tokens.controller.ts +++ b/src/api/private/tokens/tokens.controller.ts @@ -18,7 +18,9 @@ import { AuthService } from '../../../auth/auth.service'; import { TimestampMillis } from '../../../utils/timestamp'; import { AuthTokenDto } from '../../../auth/auth-token.dto'; import { AuthTokenWithSecretDto } from '../../../auth/auth-token-with-secret.dto'; +import { ApiTags } from '@nestjs/swagger'; +@ApiTags('tokens') @Controller('tokens') export class TokensController { constructor( diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 2cb412f2b..89e8eef28 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -23,11 +23,12 @@ import { NoteMetadataDto } from '../../../notes/note-metadata.dto'; import { NotesService } from '../../../notes/notes.service'; import { UsersService } from '../../../users/users.service'; import { TokenAuthGuard } from '../../../auth/token-auth.guard'; -import { ApiSecurity } from '@nestjs/swagger'; +import { ApiSecurity, ApiTags } from '@nestjs/swagger'; import { HistoryEntryDto } from '../../../history/history-entry.dto'; import { UserInfoDto } from '../../../users/user-info.dto'; import { NotInDBError } from '../../../errors/errors'; +@ApiTags('me') @ApiSecurity('token') @Controller('me') export class MeController { diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index c1c211063..436e87b4c 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -28,9 +28,10 @@ import { ConsoleLoggerService } from '../../../logger/console-logger.service'; 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 { ApiSecurity, ApiTags } from '@nestjs/swagger'; import { MediaUploadUrlDto } from '../../../media/media-upload-url.dto'; +@ApiTags('media') @ApiSecurity('token') @Controller('media') export class MediaController { diff --git a/src/api/public/monitoring/monitoring.controller.ts b/src/api/public/monitoring/monitoring.controller.ts index d90b81117..4d561f0a3 100644 --- a/src/api/public/monitoring/monitoring.controller.ts +++ b/src/api/public/monitoring/monitoring.controller.ts @@ -7,9 +7,10 @@ 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 { ApiSecurity, ApiTags } from '@nestjs/swagger'; import { ServerStatusDto } from '../../../monitoring/server-status.dto'; +@ApiTags('monitoring') @ApiSecurity('token') @Controller('monitoring') export class MonitoringController { diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index b4f53e01c..e17bf4e98 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -27,13 +27,14 @@ 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 { ApiSecurity, ApiTags } from '@nestjs/swagger'; import { HistoryService } from '../../../history/history.service'; 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'; +@ApiTags('notes') @ApiSecurity('token') @Controller('notes') export class NotesController {