Merge pull request #1130 from hedgedoc/history/transaction

This commit is contained in:
David Mehren 2021-05-02 18:15:05 +02:00 committed by GitHub
commit 7adbc72a5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 252 additions and 196 deletions

View file

@ -24,6 +24,12 @@ module.exports = {
assertFunctionNames: ['expect', 'request.**.expect'], assertFunctionNames: ['expect', 'request.**.expect'],
}, },
], ],
'jest/no-standalone-expect': [
'error',
{
additionalTestBlockFunctions: ['afterEach', 'beforeAll'],
},
],
}, },
}, },
], ],

View file

@ -10,7 +10,11 @@ import { LoggerModule } from '../../../../logger/logger.module';
import { UsersModule } from '../../../../users/users.module'; import { UsersModule } from '../../../../users/users.module';
import { HistoryModule } from '../../../../history/history.module'; import { HistoryModule } from '../../../../history/history.module';
import { NotesModule } from '../../../../notes/notes.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 { User } from '../../../../users/user.entity';
import { Note } from '../../../../notes/note.entity'; import { Note } from '../../../../notes/note.entity';
import { AuthToken } from '../../../../auth/auth-token.entity'; import { AuthToken } from '../../../../auth/auth-token.entity';
@ -41,8 +45,11 @@ describe('HistoryController', () => {
isGlobal: true, isGlobal: true,
load: [appConfigMock], load: [appConfigMock],
}), }),
TypeOrmModule.forRoot(),
], ],
}) })
.overrideProvider(getConnectionToken())
.useValue({})
.overrideProvider(getRepositoryToken(User)) .overrideProvider(getRepositoryToken(User))
.useValue({}) .useValue({})
.overrideProvider(getRepositoryToken(Note)) .overrideProvider(getRepositoryToken(Note))

View file

@ -5,6 +5,7 @@
*/ */
import { import {
BadRequestException,
Body, Body,
Controller, Controller,
Delete, Delete,
@ -16,9 +17,8 @@ import {
} from '@nestjs/common'; } from '@nestjs/common';
import { ApiTags } from '@nestjs/swagger'; import { ApiTags } from '@nestjs/swagger';
import { UsersService } from '../../../../users/users.service'; import { UsersService } from '../../../../users/users.service';
import { NotesService } from '../../../../notes/notes.service';
import { HistoryEntryDto } from '../../../../history/history-entry.dto'; 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 { HistoryEntryImportDto } from '../../../../history/history-entry-import.dto';
import { HistoryEntryUpdateDto } from '../../../../history/history-entry-update.dto'; import { HistoryEntryUpdateDto } from '../../../../history/history-entry-update.dto';
import { ConsoleLoggerService } from '../../../../logger/console-logger.service'; import { ConsoleLoggerService } from '../../../../logger/console-logger.service';
@ -31,7 +31,6 @@ export class HistoryController {
private readonly logger: ConsoleLoggerService, private readonly logger: ConsoleLoggerService,
private historyService: HistoryService, private historyService: HistoryService,
private userService: UsersService, private userService: UsersService,
private noteService: NotesService,
) { ) {
this.logger.setContext(HistoryController.name); this.logger.setContext(HistoryController.name);
} }
@ -60,21 +59,10 @@ export class HistoryController {
try { try {
// ToDo: use actual user here // ToDo: use actual user here
const user = await this.userService.getUserByUsername('hardcoded'); const user = await this.userService.getUserByUsername('hardcoded');
await this.historyService.deleteHistory(user); await this.historyService.setHistory(user, history);
for (const historyEntry of history) {
const note = await this.noteService.getNoteByIdOrAlias(
historyEntry.note,
);
await this.historyService.createOrUpdateHistoryEntry(
note,
user,
historyEntry.pinStatus,
historyEntry.lastVisited,
);
}
} catch (e) { } catch (e) {
if (e instanceof NotInDBError) { if (e instanceof NotInDBError || e instanceof ForbiddenIdError) {
throw new NotFoundException(e.message); throw new BadRequestException(e.message);
} }
throw e; throw e;
} }

View file

@ -7,7 +7,11 @@
import { Test, TestingModule } from '@nestjs/testing'; import { Test, TestingModule } from '@nestjs/testing';
import { NotesController } from './notes.controller'; import { NotesController } from './notes.controller';
import { NotesService } from '../../../notes/notes.service'; 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 { Note } from '../../../notes/note.entity';
import { Tag } from '../../../notes/tag.entity'; import { Tag } from '../../../notes/tag.entity';
import { RevisionsModule } from '../../../revisions/revisions.module'; import { RevisionsModule } from '../../../revisions/revisions.module';
@ -61,8 +65,11 @@ describe('NotesController', () => {
isGlobal: true, isGlobal: true,
load: [appConfigMock, mediaConfigMock], load: [appConfigMock, mediaConfigMock],
}), }),
TypeOrmModule.forRoot(),
], ],
}) })
.overrideProvider(getConnectionToken())
.useValue({})
.overrideProvider(getRepositoryToken(Note)) .overrideProvider(getRepositoryToken(Note))
.useValue({}) .useValue({})
.overrideProvider(getRepositoryToken(Revision)) .overrideProvider(getRepositoryToken(Revision))

View file

@ -5,7 +5,11 @@
*/ */
import { Test, TestingModule } from '@nestjs/testing'; 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 { HistoryModule } from '../../../history/history.module';
import { LoggerModule } from '../../../logger/logger.module'; import { LoggerModule } from '../../../logger/logger.module';
import { AuthorColor } from '../../../notes/author-color.entity'; import { AuthorColor } from '../../../notes/author-color.entity';
@ -45,8 +49,11 @@ describe('Me Controller', () => {
NotesModule, NotesModule,
LoggerModule, LoggerModule,
MediaModule, MediaModule,
TypeOrmModule.forRoot(),
], ],
}) })
.overrideProvider(getConnectionToken())
.useValue({})
.overrideProvider(getRepositoryToken(User)) .overrideProvider(getRepositoryToken(User))
.useValue({}) .useValue({})
.overrideProvider(getRepositoryToken(Note)) .overrideProvider(getRepositoryToken(Note))

View file

@ -5,7 +5,11 @@
*/ */
import { Test, TestingModule } from '@nestjs/testing'; 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 { LoggerModule } from '../../../logger/logger.module';
import { AuthorColor } from '../../../notes/author-color.entity'; import { AuthorColor } from '../../../notes/author-color.entity';
import { Note } from '../../../notes/note.entity'; import { Note } from '../../../notes/note.entity';
@ -61,8 +65,11 @@ describe('Notes Controller', () => {
isGlobal: true, isGlobal: true,
load: [appConfigMock, mediaConfigMock], load: [appConfigMock, mediaConfigMock],
}), }),
TypeOrmModule.forRoot(),
], ],
}) })
.overrideProvider(getConnectionToken())
.useValue({})
.overrideProvider(getRepositoryToken(Note)) .overrideProvider(getRepositoryToken(Note))
.useValue({}) .useValue({})
.overrideProvider(getRepositoryToken(Revision)) .overrideProvider(getRepositoryToken(Revision))

View file

@ -9,7 +9,7 @@ import { LoggerModule } from '../logger/logger.module';
import { HistoryService } from './history.service'; import { HistoryService } from './history.service';
import { UsersModule } from '../users/users.module'; import { UsersModule } from '../users/users.module';
import { NotesModule } from '../notes/notes.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 { Identity } from '../users/identity.entity';
import { User } from '../users/user.entity'; import { User } from '../users/user.entity';
import { AuthorColor } from '../notes/author-color.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 { Tag } from '../notes/tag.entity';
import { AuthToken } from '../auth/auth-token.entity'; import { AuthToken } from '../auth/auth-token.entity';
import { Revision } from '../revisions/revision.entity'; import { Revision } from '../revisions/revision.entity';
import { Repository } from 'typeorm'; import { Connection, Repository } from 'typeorm';
import { NotInDBError } from '../errors/errors'; import { NotInDBError } from '../errors/errors';
import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity';
import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity';
import { Group } from '../groups/group.entity'; import { Group } from '../groups/group.entity';
import { ConfigModule } from '@nestjs/config'; import { ConfigModule } from '@nestjs/config';
import appConfigMock from '../config/mock/app.config.mock'; import appConfigMock from '../config/mock/app.config.mock';
import { HistoryEntryImportDto } from './history-entry-import.dto';
describe('HistoryService', () => { describe('HistoryService', () => {
let service: HistoryService; let service: HistoryService;
let historyRepo: Repository<HistoryEntry>; let historyRepo: Repository<HistoryEntry>;
let connection;
let noteRepo: Repository<Note>; let noteRepo: Repository<Note>;
type MockConnection = {
transaction: () => void;
};
function mockConnection(): MockConnection {
return {
transaction: jest.fn(),
};
}
beforeEach(async () => { beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({ const module: TestingModule = await Test.createTestingModule({
providers: [ providers: [
HistoryService, HistoryService,
{
provide: getConnectionToken(),
useFactory: mockConnection,
},
{ {
provide: getRepositoryToken(HistoryEntry), provide: getRepositoryToken(HistoryEntry),
useClass: Repository, useClass: Repository,
@ -79,6 +95,7 @@ describe('HistoryService', () => {
historyRepo = module.get<Repository<HistoryEntry>>( historyRepo = module.get<Repository<HistoryEntry>>(
getRepositoryToken(HistoryEntry), getRepositoryToken(HistoryEntry),
); );
connection = module.get<Connection>(Connection);
noteRepo = module.get<Repository<Note>>(getRepositoryToken(Note)); noteRepo = module.get<Repository<Note>>(getRepositoryToken(Note));
}); });
@ -143,10 +160,8 @@ describe('HistoryService', () => {
describe('works', () => { describe('works', () => {
const user = {} as User; const user = {} as User;
const alias = 'alias'; 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)); 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, 'findOne').mockResolvedValueOnce(undefined);
jest jest
.spyOn(historyRepo, 'save') .spyOn(historyRepo, 'save')
@ -163,65 +178,7 @@ describe('HistoryService', () => {
expect(createHistoryEntry.pinStatus).toEqual(false); expect(createHistoryEntry.pinStatus).toEqual(false);
}); });
it('without an preexisting entry, with pinStatus and without lastVisited', async () => { it('with an preexisting entry', async () => {
jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined);
jest
.spyOn(historyRepo, 'save')
.mockImplementation(
async (entry: HistoryEntry): Promise<HistoryEntry> => 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<HistoryEntry> => 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<HistoryEntry> => 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 () => {
jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry);
jest jest
.spyOn(historyRepo, 'save') .spyOn(historyRepo, 'save')
@ -240,67 +197,6 @@ describe('HistoryService', () => {
historyEntry.updatedAt.getTime(), 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<HistoryEntry> => 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<HistoryEntry> => 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<HistoryEntry> => 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);
});
}); });
}); });
@ -431,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('toHistoryEntryDto', () => {
describe('works', () => { describe('works', () => {
it('with aliased note', async () => { it('with aliased note', async () => {

View file

@ -8,19 +8,22 @@ import { Injectable } from '@nestjs/common';
import { ConsoleLoggerService } from '../logger/console-logger.service'; import { ConsoleLoggerService } from '../logger/console-logger.service';
import { HistoryEntryUpdateDto } from './history-entry-update.dto'; import { HistoryEntryUpdateDto } from './history-entry-update.dto';
import { HistoryEntryDto } from './history-entry.dto'; import { HistoryEntryDto } from './history-entry.dto';
import { InjectRepository } from '@nestjs/typeorm'; import { InjectConnection, InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm'; import { Connection, Repository } from 'typeorm';
import { HistoryEntry } from './history-entry.entity'; import { HistoryEntry } from './history-entry.entity';
import { UsersService } from '../users/users.service'; import { UsersService } from '../users/users.service';
import { NotesService } from '../notes/notes.service'; import { NotesService } from '../notes/notes.service';
import { User } from '../users/user.entity'; import { User } from '../users/user.entity';
import { Note } from '../notes/note.entity'; import { Note } from '../notes/note.entity';
import { NotInDBError } from '../errors/errors'; import { NotInDBError } from '../errors/errors';
import { HistoryEntryImportDto } from './history-entry-import.dto';
@Injectable() @Injectable()
export class HistoryService { export class HistoryService {
constructor( constructor(
private readonly logger: ConsoleLoggerService, private readonly logger: ConsoleLoggerService,
@InjectConnection()
private connection: Connection,
@InjectRepository(HistoryEntry) @InjectRepository(HistoryEntry)
private historyEntryRepository: Repository<HistoryEntry>, private historyEntryRepository: Repository<HistoryEntry>,
private usersService: UsersService, private usersService: UsersService,
@ -80,25 +83,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. * 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 {Note} note - the note that the history entry belongs to
* @param {User} user - the user 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 * @return {HistoryEntry} the requested history entry
*/ */
async createOrUpdateHistoryEntry( async createOrUpdateHistoryEntry(
note: Note, note: Note,
user: User, user: User,
pinStatus?: boolean,
lastVisited?: Date,
): Promise<HistoryEntry> { ): Promise<HistoryEntry> {
let entry = await this.getEntryByNote(note, user); let entry = await this.getEntryByNote(note, user);
if (!entry) { if (!entry) {
entry = HistoryEntry.create(user, note); entry = HistoryEntry.create(user, note);
if (pinStatus !== undefined) {
entry.pinStatus = pinStatus;
}
if (lastVisited !== undefined) {
entry.updatedAt = lastVisited;
}
} else { } else {
entry.updatedAt = new Date(); entry.updatedAt = new Date();
} }
@ -158,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<void> {
await this.connection.transaction(async (manager) => {
const currentHistory = await manager.find<HistoryEntry>(HistoryEntry, {
where: { user: user },
relations: ['note', 'user'],
});
for (const entry of currentHistory) {
await manager.remove<HistoryEntry>(entry);
}
for (const historyEntry of history) {
this.notesService.checkNoteIdOrAlias(historyEntry.note);
const note = await manager.findOne<Note>(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<HistoryEntry>(entry);
}
});
}
/** /**
* Build HistoryEntryDto from a history entry. * Build HistoryEntryDto from a history entry.
* @param {HistoryEntry} entry - the history entry to use * @param {HistoryEntry} entry - the history entry to use

View file

@ -79,6 +79,8 @@ export class NotesService {
* @param {string=} alias - a optional alias the note should have * @param {string=} alias - a optional alias the note should have
* @param {User=} owner - the owner of the note * @param {User=} owner - the owner of the note
* @return {Note} the newly created 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( async createNote(
noteContent: string, noteContent: string,
@ -92,15 +94,7 @@ export class NotesService {
]); ]);
if (alias) { if (alias) {
newNote.alias = alias; newNote.alias = alias;
if (this.appConfig.forbiddenNoteIds.includes(alias)) { this.checkNoteIdOrAlias(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.`,
);
}
} }
if (owner) { if (owner) {
newNote.historyEntries = [HistoryEntry.create(owner)]; newNote.historyEntries = [HistoryEntry.create(owner)];
@ -154,6 +148,7 @@ export class NotesService {
* Get a note by either their id or alias. * Get a note by either their id or alias.
* @param {string} noteIdOrAlias - the notes id or alias * @param {string} noteIdOrAlias - the notes id or alias
* @return {Note} the note * @return {Note} the note
* @throws {ForbiddenIdError} the requested id or alias is forbidden
* @throws {NotInDBError} there is no note with this id or alias * @throws {NotInDBError} there is no note with this id or alias
*/ */
async getNoteByIdOrAlias(noteIdOrAlias: string): Promise<Note> { async getNoteByIdOrAlias(noteIdOrAlias: string): Promise<Note> {
@ -161,15 +156,7 @@ export class NotesService {
`Trying to find note '${noteIdOrAlias}'`, `Trying to find note '${noteIdOrAlias}'`,
'getNoteByIdOrAlias', 'getNoteByIdOrAlias',
); );
if (this.appConfig.forbiddenNoteIds.includes(noteIdOrAlias)) { this.checkNoteIdOrAlias(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.`,
);
}
const note = await this.noteRepository.findOne({ const note = await this.noteRepository.findOne({
where: [ where: [
{ {
@ -202,6 +189,23 @@ export class NotesService {
return note; 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 * @async
* Delete a note * Delete a note

View file

@ -5,7 +5,7 @@
*/ */
import { INestApplication } from '@nestjs/common'; import { INestApplication } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config'; import { ConfigModule, ConfigService } from '@nestjs/config';
import { Test } from '@nestjs/testing'; import { Test } from '@nestjs/testing';
import { TypeOrmModule } from '@nestjs/typeorm'; import { TypeOrmModule } from '@nestjs/typeorm';
import request from 'supertest'; import request from 'supertest';
@ -27,6 +27,7 @@ import { PrivateApiModule } from '../../src/api/private/private-api.module';
import { HistoryService } from '../../src/history/history.service'; import { HistoryService } from '../../src/history/history.service';
import { Note } from '../../src/notes/note.entity'; import { Note } from '../../src/notes/note.entity';
import { HistoryEntryImportDto } from '../../src/history/history-entry-import.dto'; import { HistoryEntryImportDto } from '../../src/history/history-entry-import.dto';
import { HistoryEntry } from '../../src/history/history-entry.entity';
describe('History', () => { describe('History', () => {
let app: INestApplication; let app: INestApplication;
@ -34,6 +35,7 @@ describe('History', () => {
let user: User; let user: User;
let note: Note; let note: Note;
let note2: Note; let note2: Note;
let forbiddenNoteId: string;
let content: string; let content: string;
beforeAll(async () => { beforeAll(async () => {
@ -66,6 +68,8 @@ describe('History', () => {
], ],
}).compile(); }).compile();
const config = moduleRef.get<ConfigService>(ConfigService);
forbiddenNoteId = config.get('appConfig').forbiddenNoteIds[0];
app = moduleRef.createNestApplication(); app = moduleRef.createNestApplication();
await app.init(); await app.init();
content = 'This is a test note.'; content = 'This is a test note.';
@ -99,8 +103,9 @@ describe('History', () => {
); );
}); });
it('POST /me/history', async () => { describe('POST /me/history', () => {
expect((await historyService.getEntriesByUser(user)).length).toEqual(1); it('works', async () => {
expect(await historyService.getEntriesByUser(user)).toHaveLength(1);
const pinStatus = true; const pinStatus = true;
const lastVisited = new Date('2020-12-01 12:23:34'); const lastVisited = new Date('2020-12-01 12:23:34');
const postEntryDto = new HistoryEntryImportDto(); const postEntryDto = new HistoryEntryImportDto();
@ -119,6 +124,56 @@ describe('History', () => {
expect(userEntries[0].pinStatus).toEqual(pinStatus); expect(userEntries[0].pinStatus).toEqual(pinStatus);
expect(userEntries[0].updatedAt).toEqual(lastVisited); 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 () => { it('DELETE /me/history', async () => {
expect((await historyService.getEntriesByUser(user)).length).toEqual(1); expect((await historyService.getEntriesByUser(user)).length).toEqual(1);