From 056ab12e67c40ef27dceee9c2b2ae1061b92b116 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 11 Apr 2021 21:25:41 +0200 Subject: [PATCH 1/7] NotesService: Extract checkNoteIdOrAlias into own method To reuse this functionality in the history services setHistory method, it was extracted into its own exported function. Signed-off-by: Philip Molares --- src/notes/notes.service.ts | 40 +++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index a1b1ecfdf..582371d9d 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -79,6 +79,8 @@ export class NotesService { * @param {string=} alias - a optional alias the note should have * @param {User=} owner - the owner of the note * @return {Note} the newly created note + * @throws {AlreadyInDBError} a note with the requested id or alias already exists + * @throws {ForbiddenIdError} the requested id or alias is forbidden */ async createNote( noteContent: string, @@ -92,15 +94,7 @@ export class NotesService { ]); if (alias) { newNote.alias = alias; - if (this.appConfig.forbiddenNoteIds.includes(alias)) { - this.logger.debug( - `Creating a note with the alias '${alias}' is forbidden by the administrator.`, - 'createNote', - ); - throw new ForbiddenIdError( - `Creating a note with the alias '${alias}' is forbidden by the administrator.`, - ); - } + this.checkNoteIdOrAlias(alias); } if (owner) { newNote.historyEntries = [HistoryEntry.create(owner)]; @@ -154,6 +148,7 @@ export class NotesService { * Get a note by either their id or alias. * @param {string} noteIdOrAlias - the notes id or alias * @return {Note} the note + * @throws {ForbiddenIdError} the requested id or alias is forbidden * @throws {NotInDBError} there is no note with this id or alias */ async getNoteByIdOrAlias(noteIdOrAlias: string): Promise { @@ -161,15 +156,7 @@ export class NotesService { `Trying to find note '${noteIdOrAlias}'`, 'getNoteByIdOrAlias', ); - if (this.appConfig.forbiddenNoteIds.includes(noteIdOrAlias)) { - this.logger.debug( - `Accessing a note with the alias '${noteIdOrAlias}' is forbidden by the administrator.`, - 'getNoteByIdOrAlias', - ); - throw new ForbiddenIdError( - `Accessing a note with the alias '${noteIdOrAlias}' is forbidden by the administrator.`, - ); - } + this.checkNoteIdOrAlias(noteIdOrAlias); const note = await this.noteRepository.findOne({ where: [ { @@ -200,6 +187,23 @@ export class NotesService { return note; } + /** + * Check if the provided note id or alias is not forbidden + * @param noteIdOrAlias - the alias or id in question + * @throws {ForbiddenIdError} the requested id or alias is forbidden + */ + checkNoteIdOrAlias(noteIdOrAlias: string): void { + if (this.appConfig.forbiddenNoteIds.includes(noteIdOrAlias)) { + this.logger.debug( + `A note with the alias '${noteIdOrAlias}' is forbidden by the administrator.`, + 'checkNoteIdOrAlias', + ); + throw new ForbiddenIdError( + `A note with the alias '${noteIdOrAlias}' is forbidden by the administrator.`, + ); + } + } + /** * @async * Delete a note From 353f444f30abcd411e91bb1f4995fbe635f3f04e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 11 Apr 2021 21:28:53 +0200 Subject: [PATCH 2/7] HistoryService: Remove extra parameters from createOrUpdateHistoryEntry As the function is now only called with a user and a note and the previous extra parameters are now added into the transactional setHistory method, this is no longer necessary. Signed-off-by: Philip Molares --- src/history/history.service.spec.ts | 125 +--------------------------- src/history/history.service.ts | 10 --- 2 files changed, 2 insertions(+), 133 deletions(-) diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index 6cc7d947c..14ca1d82e 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -143,10 +143,8 @@ describe('HistoryService', () => { describe('works', () => { const user = {} as User; const alias = 'alias'; - const pinStatus = true; - const lastVisited = new Date('2020-12-01 12:23:34'); const historyEntry = HistoryEntry.create(user, Note.create(user, alias)); - it('without an preexisting entry, without pinStatus and without lastVisited', async () => { + it('without an preexisting entry', async () => { jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); jest .spyOn(historyRepo, 'save') @@ -163,65 +161,7 @@ describe('HistoryService', () => { expect(createHistoryEntry.pinStatus).toEqual(false); }); - it('without an preexisting entry, with pinStatus and without lastVisited', async () => { - 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, - pinStatus, - ); - expect(createHistoryEntry.note.alias).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); - expect(createHistoryEntry.user).toEqual(user); - expect(createHistoryEntry.pinStatus).toEqual(pinStatus); - }); - - it('without an preexisting entry, without pinStatus and with lastVisited', async () => { - 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, - undefined, - lastVisited, - ); - 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).toEqual(lastVisited); - }); - - it('without an preexisting entry, with pinStatus and with lastVisited', async () => { - 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, - pinStatus, - lastVisited, - ); - expect(createHistoryEntry.note.alias).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); - expect(createHistoryEntry.user).toEqual(user); - expect(createHistoryEntry.pinStatus).toEqual(pinStatus); - expect(createHistoryEntry.updatedAt).toEqual(lastVisited); - }); - - it('with an preexisting entry, without pinStatus and without lastVisited', async () => { + it('with an preexisting entry', async () => { jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); jest .spyOn(historyRepo, 'save') @@ -240,67 +180,6 @@ describe('HistoryService', () => { historyEntry.updatedAt.getTime(), ); }); - - it('with an preexisting entry, with pinStatus and without lastVisited', async () => { - 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, - pinStatus, - ); - expect(createHistoryEntry.note.alias).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); - expect(createHistoryEntry.user).toEqual(user); - expect(createHistoryEntry.pinStatus).not.toEqual(pinStatus); - expect(createHistoryEntry.updatedAt.getTime()).toBeGreaterThanOrEqual( - historyEntry.updatedAt.getTime(), - ); - }); - - it('with an preexisting entry, without pinStatus and with lastVisited', async () => { - 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, - undefined, - lastVisited, - ); - 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).not.toEqual(lastVisited); - }); - - it('with an preexisting entry, with pinStatus and with lastVisited', async () => { - 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, - pinStatus, - lastVisited, - ); - expect(createHistoryEntry.note.alias).toEqual(alias); - expect(createHistoryEntry.note.owner).toEqual(user); - expect(createHistoryEntry.user).toEqual(user); - expect(createHistoryEntry.pinStatus).not.toEqual(pinStatus); - expect(createHistoryEntry.updatedAt).not.toEqual(lastVisited); - }); }); }); diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 276af8190..d2793ac04 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -80,25 +80,15 @@ export class HistoryService { * Create or update a history entry by the user and note. If the entry is merely updated the updatedAt date is set to the current date. * @param {Note} note - the note that the history entry belongs to * @param {User} user - the user that the history entry belongs to - * @param {boolean} pinStatus - if the pinStatus of the history entry should be set - * @param {Date} lastVisited - the last time the associated note was accessed * @return {HistoryEntry} the requested history entry */ async createOrUpdateHistoryEntry( note: Note, user: User, - pinStatus?: boolean, - lastVisited?: Date, ): Promise { let entry = await this.getEntryByNote(note, user); if (!entry) { entry = HistoryEntry.create(user, note); - if (pinStatus !== undefined) { - entry.pinStatus = pinStatus; - } - if (lastVisited !== undefined) { - entry.updatedAt = lastVisited; - } } else { entry.updatedAt = new Date(); } From ea4c58c68f9a4a7b1f88d288672599eb0d100f0e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 11 Apr 2021 21:54:50 +0200 Subject: [PATCH 3/7] HistoryService: Add setHistory method This is the transactional reimplementation of the business logic of the history controllers setHistory method (of the private api). This should prevent the problem that the history gets deleted, but a later error in the handling of the list of HistoryEntryImportDto let's the call fail. See also: https://docs.nestjs.com/techniques/database#transactions Signed-off-by: Philip Molares --- .../private/me/history/history.controller.ts | 22 ++------ src/history/history.service.ts | 55 ++++++++++++++++++- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/api/private/me/history/history.controller.ts b/src/api/private/me/history/history.controller.ts index da00d7223..f442ccee0 100644 --- a/src/api/private/me/history/history.controller.ts +++ b/src/api/private/me/history/history.controller.ts @@ -5,6 +5,7 @@ */ import { + BadRequestException, Body, Controller, Delete, @@ -16,9 +17,8 @@ import { } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { UsersService } from '../../../../users/users.service'; -import { NotesService } from '../../../../notes/notes.service'; import { HistoryEntryDto } from '../../../../history/history-entry.dto'; -import { NotInDBError } from '../../../../errors/errors'; +import { ForbiddenIdError, NotInDBError } from '../../../../errors/errors'; import { HistoryEntryImportDto } from '../../../../history/history-entry-import.dto'; import { HistoryEntryUpdateDto } from '../../../../history/history-entry-update.dto'; import { ConsoleLoggerService } from '../../../../logger/console-logger.service'; @@ -31,7 +31,6 @@ export class HistoryController { private readonly logger: ConsoleLoggerService, private historyService: HistoryService, private userService: UsersService, - private noteService: NotesService, ) { this.logger.setContext(HistoryController.name); } @@ -60,21 +59,10 @@ export class HistoryController { try { // ToDo: use actual user here const user = await this.userService.getUserByUsername('hardcoded'); - await this.historyService.deleteHistory(user); - for (const historyEntry of history) { - const note = await this.noteService.getNoteByIdOrAlias( - historyEntry.note, - ); - await this.historyService.createOrUpdateHistoryEntry( - note, - user, - historyEntry.pinStatus, - historyEntry.lastVisited, - ); - } + await this.historyService.setHistory(user, history); } catch (e) { - if (e instanceof NotInDBError) { - throw new NotFoundException(e.message); + if (e instanceof NotInDBError || e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); } throw e; } diff --git a/src/history/history.service.ts b/src/history/history.service.ts index d2793ac04..a71efe9e5 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -8,19 +8,22 @@ 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 { InjectConnection, InjectRepository } from '@nestjs/typeorm'; +import { Connection, Repository } from 'typeorm'; import { HistoryEntry } from './history-entry.entity'; 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'; +import { HistoryEntryImportDto } from './history-entry-import.dto'; @Injectable() export class HistoryService { constructor( private readonly logger: ConsoleLoggerService, + @InjectConnection() + private connection: Connection, @InjectRepository(HistoryEntry) private historyEntryRepository: Repository, private usersService: UsersService, @@ -148,6 +151,54 @@ export class HistoryService { } } + /** + * @async + * Replace the user history with the provided history + * @param {User} user - the user that get's their history replaces + * @param {HistoryEntryImportDto[]} history + * @throws {ForbiddenIdError} one of the note ids or alias in the new history are forbidden + */ + async setHistory( + user: User, + history: HistoryEntryImportDto[], + ): Promise { + await this.connection.transaction(async (manager) => { + const currentHistory = await manager.find(HistoryEntry, { + where: { user: user }, + relations: ['note', 'user'], + }); + for (const entry of currentHistory) { + await manager.remove(entry); + } + for (const historyEntry of history) { + this.notesService.checkNoteIdOrAlias(historyEntry.note); + const note = await manager.findOne(Note, { + where: [ + { + id: historyEntry.note, + }, + { + alias: historyEntry.note, + }, + ], + }); + if (note === undefined) { + this.logger.debug( + `Could not find note '${historyEntry.note}'`, + 'setHistory', + ); + throw new NotInDBError( + `Note with id/alias '${historyEntry.note}' not found.`, + ); + } + const entry = HistoryEntry.create(user, note); + entry.pinStatus = historyEntry.pinStatus; + entry.updatedAt = historyEntry.lastVisited; + await manager.save(entry); + } + }); + } + /** * Build HistoryEntryDto from a history entry. * @param {HistoryEntry} entry - the history entry to use From f731d2d455ed32255137909cfd18e19c49c357a9 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 11 Apr 2021 22:06:36 +0200 Subject: [PATCH 4/7] HistoryService: Add test for setHistory Signed-off-by: Philip Molares --- src/history/history.service.spec.ts | 59 ++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index 14ca1d82e..e7fc8a48b 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -9,7 +9,7 @@ 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 { getConnectionToken, getRepositoryToken } from '@nestjs/typeorm'; import { Identity } from '../users/identity.entity'; import { User } from '../users/user.entity'; import { AuthorColor } from '../notes/author-color.entity'; @@ -19,23 +19,39 @@ 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 { Connection, Repository } from 'typeorm'; import { NotInDBError } from '../errors/errors'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Group } from '../groups/group.entity'; import { ConfigModule } from '@nestjs/config'; import appConfigMock from '../config/mock/app.config.mock'; +import { HistoryEntryImportDto } from './history-entry-import.dto'; describe('HistoryService', () => { let service: HistoryService; let historyRepo: Repository; + let connection; let noteRepo: Repository; + type MockConnection = { + transaction: () => void; + }; + + function mockConnection(): MockConnection { + return { + transaction: jest.fn(), + }; + } + beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ providers: [ HistoryService, + { + provide: getConnectionToken(), + useFactory: mockConnection, + }, { provide: getRepositoryToken(HistoryEntry), useClass: Repository, @@ -79,6 +95,7 @@ describe('HistoryService', () => { historyRepo = module.get>( getRepositoryToken(HistoryEntry), ); + connection = module.get(Connection); noteRepo = module.get>(getRepositoryToken(Note)); }); @@ -310,6 +327,44 @@ describe('HistoryService', () => { }); }); + describe('setHistory', () => { + it('works', async () => { + const user = {} as User; + const alias = 'alias'; + const note = Note.create(user, alias); + const historyEntry = HistoryEntry.create(user, note); + const historyEntryImport: HistoryEntryImportDto = { + lastVisited: new Date('2020-12-01 12:23:34'), + note: alias, + pinStatus: true, + }; + const newlyCreatedHistoryEntry: HistoryEntry = { + ...historyEntry, + pinStatus: historyEntryImport.pinStatus, + updatedAt: historyEntryImport.lastVisited, + }; + const mockedManager = { + find: jest.fn().mockResolvedValueOnce([historyEntry]), + findOne: jest.fn().mockResolvedValueOnce(note), + remove: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { + expect(entry.note.alias).toEqual(alias); + expect(entry.pinStatus).toEqual(false); + }), + save: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { + expect(entry.note.alias).toEqual(newlyCreatedHistoryEntry.note.alias); + expect(entry.pinStatus).toEqual(newlyCreatedHistoryEntry.pinStatus); + expect(entry.updatedAt).toEqual(newlyCreatedHistoryEntry.updatedAt); + }), + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + connection.transaction.mockImplementation((cb) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + cb(mockedManager); + }); + await service.setHistory(user, [historyEntryImport]); + }); + }); + describe('toHistoryEntryDto', () => { describe('works', () => { it('with aliased note', async () => { From f7e483da81b306bcb412362bfc57d212a2bd4b91 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 18 Apr 2021 14:02:55 +0200 Subject: [PATCH 5/7] ESLint: Add extra rules for tests Add afterEach and beforeAll as additional additionalTestBlockFunctions for jest/no-standalone-expect. This is necessary, because we use expects in those functions for the history service unit tests. Signed-off-by: Philip Molares --- .eslintrc.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 08b5558a0..2965aabf7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -24,6 +24,12 @@ module.exports = { assertFunctionNames: ['expect', 'request.**.expect'], }, ], + 'jest/no-standalone-expect': [ + 'error', + { + additionalTestBlockFunctions: ['afterEach', 'beforeAll'], + }, + ], }, }, ], From f967b34018a4bc9b2e4331375b651faca062588d Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 11 Apr 2021 22:06:41 +0200 Subject: [PATCH 6/7] ControllerTests: Add connection to controller tests Signed-off-by: Philip Molares --- src/api/private/me/history/history.controller.spec.ts | 9 ++++++++- src/api/private/notes/notes.controller.spec.ts | 9 ++++++++- src/api/public/me/me.controller.spec.ts | 9 ++++++++- src/api/public/notes/notes.controller.spec.ts | 9 ++++++++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/api/private/me/history/history.controller.spec.ts b/src/api/private/me/history/history.controller.spec.ts index caf73a292..2b5da6db2 100644 --- a/src/api/private/me/history/history.controller.spec.ts +++ b/src/api/private/me/history/history.controller.spec.ts @@ -10,7 +10,11 @@ import { LoggerModule } from '../../../../logger/logger.module'; import { UsersModule } from '../../../../users/users.module'; import { HistoryModule } from '../../../../history/history.module'; import { NotesModule } from '../../../../notes/notes.module'; -import { getRepositoryToken } from '@nestjs/typeorm'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; import { User } from '../../../../users/user.entity'; import { Note } from '../../../../notes/note.entity'; import { AuthToken } from '../../../../auth/auth-token.entity'; @@ -41,8 +45,11 @@ describe('HistoryController', () => { isGlobal: true, load: [appConfigMock], }), + TypeOrmModule.forRoot(), ], }) + .overrideProvider(getConnectionToken()) + .useValue({}) .overrideProvider(getRepositoryToken(User)) .useValue({}) .overrideProvider(getRepositoryToken(Note)) diff --git a/src/api/private/notes/notes.controller.spec.ts b/src/api/private/notes/notes.controller.spec.ts index d42903ca7..c1a3f8ad4 100644 --- a/src/api/private/notes/notes.controller.spec.ts +++ b/src/api/private/notes/notes.controller.spec.ts @@ -7,7 +7,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { NotesController } from './notes.controller'; import { NotesService } from '../../../notes/notes.service'; -import { getRepositoryToken } from '@nestjs/typeorm'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; import { Note } from '../../../notes/note.entity'; import { Tag } from '../../../notes/tag.entity'; import { RevisionsModule } from '../../../revisions/revisions.module'; @@ -61,8 +65,11 @@ describe('NotesController', () => { isGlobal: true, load: [appConfigMock, mediaConfigMock], }), + TypeOrmModule.forRoot(), ], }) + .overrideProvider(getConnectionToken()) + .useValue({}) .overrideProvider(getRepositoryToken(Note)) .useValue({}) .overrideProvider(getRepositoryToken(Revision)) diff --git a/src/api/public/me/me.controller.spec.ts b/src/api/public/me/me.controller.spec.ts index 0274ee1d8..bf61c10a6 100644 --- a/src/api/public/me/me.controller.spec.ts +++ b/src/api/public/me/me.controller.spec.ts @@ -5,7 +5,11 @@ */ import { Test, TestingModule } from '@nestjs/testing'; -import { getRepositoryToken } from '@nestjs/typeorm'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; import { HistoryModule } from '../../../history/history.module'; import { LoggerModule } from '../../../logger/logger.module'; import { AuthorColor } from '../../../notes/author-color.entity'; @@ -45,8 +49,11 @@ describe('Me Controller', () => { NotesModule, LoggerModule, MediaModule, + TypeOrmModule.forRoot(), ], }) + .overrideProvider(getConnectionToken()) + .useValue({}) .overrideProvider(getRepositoryToken(User)) .useValue({}) .overrideProvider(getRepositoryToken(Note)) diff --git a/src/api/public/notes/notes.controller.spec.ts b/src/api/public/notes/notes.controller.spec.ts index 789b5220a..867e36d9e 100644 --- a/src/api/public/notes/notes.controller.spec.ts +++ b/src/api/public/notes/notes.controller.spec.ts @@ -5,7 +5,11 @@ */ import { Test, TestingModule } from '@nestjs/testing'; -import { getRepositoryToken } from '@nestjs/typeorm'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; import { LoggerModule } from '../../../logger/logger.module'; import { AuthorColor } from '../../../notes/author-color.entity'; import { Note } from '../../../notes/note.entity'; @@ -61,8 +65,11 @@ describe('Notes Controller', () => { isGlobal: true, load: [appConfigMock, mediaConfigMock], }), + TypeOrmModule.forRoot(), ], }) + .overrideProvider(getConnectionToken()) + .useValue({}) .overrideProvider(getRepositoryToken(Note)) .useValue({}) .overrideProvider(getRepositoryToken(Revision)) From 2ba1be6ea377e6c4d8dc1b485a2697d62f82174e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Mon, 12 Apr 2021 10:55:16 +0200 Subject: [PATCH 7/7] PrivateHistoryE2E: Extend POST /me/history Test the correct behaviour in error cases Signed-off-by: Philip Molares --- test/private-api/history.e2e-spec.ts | 95 ++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 20 deletions(-) diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index 192c8964d..4b79be03e 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -5,7 +5,7 @@ */ import { INestApplication } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; +import { ConfigModule, ConfigService } from '@nestjs/config'; import { Test } from '@nestjs/testing'; import { TypeOrmModule } from '@nestjs/typeorm'; import * as request from 'supertest'; @@ -27,6 +27,7 @@ import { PrivateApiModule } from '../../src/api/private/private-api.module'; import { HistoryService } from '../../src/history/history.service'; import { Note } from '../../src/notes/note.entity'; import { HistoryEntryImportDto } from '../../src/history/history-entry-import.dto'; +import { HistoryEntry } from '../../src/history/history-entry.entity'; describe('History', () => { let app: INestApplication; @@ -34,6 +35,7 @@ describe('History', () => { let user: User; let note: Note; let note2: Note; + let forbiddenNoteId: string; let content: string; beforeAll(async () => { @@ -66,6 +68,8 @@ describe('History', () => { ], }).compile(); + const config = moduleRef.get(ConfigService); + forbiddenNoteId = config.get('appConfig').forbiddenNoteIds[0]; app = moduleRef.createNestApplication(); await app.init(); content = 'This is a test note.'; @@ -99,25 +103,76 @@ describe('History', () => { ); }); - it('POST /me/history', async () => { - expect((await historyService.getEntriesByUser(user)).length).toEqual(1); - const pinStatus = true; - const lastVisited = new Date('2020-12-01 12:23:34'); - const postEntryDto = new HistoryEntryImportDto(); - postEntryDto.note = note2.alias; - postEntryDto.pinStatus = pinStatus; - postEntryDto.lastVisited = lastVisited; - await request(app.getHttpServer()) - .post('/me/history') - .set('Content-Type', 'application/json') - .send(JSON.stringify({ history: [postEntryDto] })) - .expect(201); - const userEntries = await historyService.getEntriesByUser(user); - expect(userEntries.length).toEqual(1); - expect(userEntries[0].note.alias).toEqual(note2.alias); - expect(userEntries[0].user.userName).toEqual(user.userName); - expect(userEntries[0].pinStatus).toEqual(pinStatus); - expect(userEntries[0].updatedAt).toEqual(lastVisited); + describe('POST /me/history', () => { + it('works', async () => { + expect(await historyService.getEntriesByUser(user)).toHaveLength(1); + const pinStatus = true; + const lastVisited = new Date('2020-12-01 12:23:34'); + const postEntryDto = new HistoryEntryImportDto(); + postEntryDto.note = note2.alias; + postEntryDto.pinStatus = pinStatus; + postEntryDto.lastVisited = lastVisited; + await request(app.getHttpServer()) + .post('/me/history') + .set('Content-Type', 'application/json') + .send(JSON.stringify({ history: [postEntryDto] })) + .expect(201); + const userEntries = await historyService.getEntriesByUser(user); + expect(userEntries.length).toEqual(1); + expect(userEntries[0].note.alias).toEqual(note2.alias); + expect(userEntries[0].user.userName).toEqual(user.userName); + expect(userEntries[0].pinStatus).toEqual(pinStatus); + expect(userEntries[0].updatedAt).toEqual(lastVisited); + }); + describe('fails', () => { + let pinStatus: boolean; + let lastVisited: Date; + let postEntryDto: HistoryEntryImportDto; + let prevEntry: HistoryEntry; + beforeAll(async () => { + const previousHistory = await historyService.getEntriesByUser(user); + expect(previousHistory).toHaveLength(1); + prevEntry = previousHistory[0]; + pinStatus = !previousHistory[0].pinStatus; + lastVisited = new Date('2020-12-01 23:34:45'); + postEntryDto = new HistoryEntryImportDto(); + postEntryDto.note = note2.alias; + postEntryDto.pinStatus = pinStatus; + postEntryDto.lastVisited = lastVisited; + }); + it('with forbiddenId', async () => { + const brokenEntryDto = new HistoryEntryImportDto(); + brokenEntryDto.note = forbiddenNoteId; + brokenEntryDto.pinStatus = pinStatus; + brokenEntryDto.lastVisited = lastVisited; + await request(app.getHttpServer()) + .post('/me/history') + .set('Content-Type', 'application/json') + .send(JSON.stringify({ history: [brokenEntryDto] })) + .expect(400); + }); + it('with non-existing note', async () => { + const brokenEntryDto = new HistoryEntryImportDto(); + brokenEntryDto.note = 'i_dont_exist'; + brokenEntryDto.pinStatus = pinStatus; + brokenEntryDto.lastVisited = lastVisited; + await request(app.getHttpServer()) + .post('/me/history') + .set('Content-Type', 'application/json') + .send(JSON.stringify({ history: [brokenEntryDto] })) + .expect(400); + }); + afterEach(async () => { + const historyEntries = await historyService.getEntriesByUser(user); + expect(historyEntries).toHaveLength(1); + expect(historyEntries[0].note.alias).toEqual(prevEntry.note.alias); + expect(historyEntries[0].user.userName).toEqual( + prevEntry.user.userName, + ); + expect(historyEntries[0].pinStatus).toEqual(prevEntry.pinStatus); + expect(historyEntries[0].updatedAt).toEqual(prevEntry.updatedAt); + }); + }); }); it('DELETE /me/history', async () => {