diff --git a/backend/src/app.ts b/backend/src/app.ts index 0c5e8892..07a84126 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -9,6 +9,7 @@ import { EnvVars, getNumericEnvVar } from './util/envvars.js'; import apiRouter from './routes/router.js'; import swaggerMiddleware from './swagger.js'; import swaggerUi from 'swagger-ui-express'; +import { errorHandler } from './middleware/error-handling/error-handler.js'; const logger: Logger = getLogger(); @@ -26,6 +27,8 @@ app.use('/api', apiRouter); // Swagger app.use('/api-docs', swaggerUi.serve, swaggerMiddleware); +app.use(errorHandler); + async function startServer() { await initORM(); diff --git a/backend/src/controllers/learning-objects.ts b/backend/src/controllers/learning-objects.ts index e9d0d727..53eb1ded 100644 --- a/backend/src/controllers/learning-objects.ts +++ b/backend/src/controllers/learning-objects.ts @@ -4,9 +4,9 @@ import { FilteredLearningObject, LearningObjectIdentifier, LearningPathIdentifie import learningObjectService from '../services/learning-objects/learning-object-service.js'; import { EnvVars, getEnvVar } from '../util/envvars.js'; import { Language } from '../entities/content/language.js'; -import { BadRequestException } from '../exceptions.js'; import attachmentService from '../services/learning-objects/attachment-service.js'; import { NotFoundError } from '@mikro-orm/core'; +import { BadRequestException } from '../exceptions/bad-request-exception.js'; function getLearningObjectIdentifierFromRequest(req: Request): LearningObjectIdentifier { if (!req.params.hruid) { diff --git a/backend/src/controllers/learning-paths.ts b/backend/src/controllers/learning-paths.ts index 37f92d91..04e44b59 100644 --- a/backend/src/controllers/learning-paths.ts +++ b/backend/src/controllers/learning-paths.ts @@ -2,13 +2,14 @@ import { Request, Response } from 'express'; import { themes } from '../data/themes.js'; import { FALLBACK_LANG } from '../config.js'; import learningPathService from '../services/learning-paths/learning-path-service.js'; -import { BadRequestException, NotFoundException } from '../exceptions.js'; import { Language } from '../entities/content/language.js'; import { PersonalizationTarget, personalizedForGroup, personalizedForStudent, } from '../services/learning-paths/learning-path-personalization-util.js'; +import { BadRequestException } from '../exceptions/bad-request-exception.js'; +import { NotFoundException } from '../exceptions/not-found-exception.js'; /** * Fetch learning paths based on query parameters. diff --git a/backend/src/data/dwengo-entity-repository.ts b/backend/src/data/dwengo-entity-repository.ts index 6538d6f5..ce894e4b 100644 --- a/backend/src/data/dwengo-entity-repository.ts +++ b/backend/src/data/dwengo-entity-repository.ts @@ -1,10 +1,12 @@ import { EntityRepository, FilterQuery } from '@mikro-orm/core'; +import { EntityAlreadyExistsException } from '../exceptions/entity-already-exists-exception.js'; export abstract class DwengoEntityRepository extends EntityRepository { - public async save(entity: T) { - const em = this.getEntityManager(); - em.persist(entity); - await em.flush(); + public async save(entity: T, options?: { preventOverwrite?: boolean }): Promise { + if (options?.preventOverwrite && (await this.findOne(entity))) { + throw new EntityAlreadyExistsException(`A ${this.getEntityName()} with this identifier already exists.`); + } + await this.getEntityManager().persistAndFlush(entity); } public async deleteWhere(query: FilterQuery) { const toDelete = await this.findOne(query); diff --git a/backend/src/entities/users/student.entity.ts b/backend/src/entities/users/student.entity.ts index da5b4367..58e82765 100644 --- a/backend/src/entities/users/student.entity.ts +++ b/backend/src/entities/users/student.entity.ts @@ -13,12 +13,4 @@ export class Student extends User { @ManyToMany(() => Group) groups!: Collection; - - constructor( - public username: string, - public firstName: string, - public lastName: string - ) { - super(); - } } diff --git a/backend/src/entities/users/teacher.entity.ts b/backend/src/entities/users/teacher.entity.ts index 8e22d1de..d53ca603 100644 --- a/backend/src/entities/users/teacher.entity.ts +++ b/backend/src/entities/users/teacher.entity.ts @@ -7,12 +7,4 @@ import { TeacherRepository } from '../../data/users/teacher-repository.js'; export class Teacher extends User { @ManyToMany(() => Class) classes!: Collection; - - constructor( - public username: string, - public firstName: string, - public lastName: string - ) { - super(); - } } diff --git a/backend/src/exceptions.ts b/backend/src/exceptions.ts deleted file mode 100644 index e93a6c93..00000000 --- a/backend/src/exceptions.ts +++ /dev/null @@ -1,42 +0,0 @@ -/** - * Exception for HTTP 400 Bad Request - */ -export class BadRequestException extends Error { - public status = 400; - - constructor(error: string) { - super(error); - } -} - -/** - * Exception for HTTP 401 Unauthorized - */ -export class UnauthorizedException extends Error { - status = 401; - constructor(message: string = 'Unauthorized') { - super(message); - } -} - -/** - * Exception for HTTP 403 Forbidden - */ -export class ForbiddenException extends Error { - status = 403; - - constructor(message: string = 'Forbidden') { - super(message); - } -} - -/** - * Exception for HTTP 404 Not Found - */ -export class NotFoundException extends Error { - public status = 404; - - constructor(error: string) { - super(error); - } -} diff --git a/backend/src/exceptions/bad-request-exception.ts b/backend/src/exceptions/bad-request-exception.ts new file mode 100644 index 00000000..f6672a62 --- /dev/null +++ b/backend/src/exceptions/bad-request-exception.ts @@ -0,0 +1,10 @@ +import { ExceptionWithHttpState } from './exception-with-http-state.js'; + +/** + * Exception for HTTP 400 Bad Request + */ +export class BadRequestException extends ExceptionWithHttpState { + constructor(error: string) { + super(400, error); + } +} diff --git a/backend/src/exceptions/conflict-exception.ts b/backend/src/exceptions/conflict-exception.ts new file mode 100644 index 00000000..ed1d0b24 --- /dev/null +++ b/backend/src/exceptions/conflict-exception.ts @@ -0,0 +1,12 @@ +import { ExceptionWithHttpState } from './exception-with-http-state.js'; + +/** + * Exception for HTTP 409 Conflict + */ +export class ConflictException extends ExceptionWithHttpState { + public status = 409; + + constructor(error: string) { + super(409, error); + } +} diff --git a/backend/src/exceptions/entity-already-exists-exception.ts b/backend/src/exceptions/entity-already-exists-exception.ts new file mode 100644 index 00000000..2769b814 --- /dev/null +++ b/backend/src/exceptions/entity-already-exists-exception.ts @@ -0,0 +1,7 @@ +import { ConflictException } from './conflict-exception.js'; + +export class EntityAlreadyExistsException extends ConflictException { + constructor(message: string) { + super(message); + } +} diff --git a/backend/src/exceptions/exception-with-http-state.ts b/backend/src/exceptions/exception-with-http-state.ts new file mode 100644 index 00000000..e5b9b9bd --- /dev/null +++ b/backend/src/exceptions/exception-with-http-state.ts @@ -0,0 +1,11 @@ +/** + * Exceptions which are associated with a HTTP error code. + */ +export abstract class ExceptionWithHttpState extends Error { + constructor( + public status: number, + public error: string + ) { + super(error); + } +} diff --git a/backend/src/exceptions/forbidden-exception.ts b/backend/src/exceptions/forbidden-exception.ts new file mode 100644 index 00000000..5712e0c8 --- /dev/null +++ b/backend/src/exceptions/forbidden-exception.ts @@ -0,0 +1,12 @@ +import { ExceptionWithHttpState } from './exception-with-http-state.js'; + +/** + * Exception for HTTP 403 Forbidden + */ +export class ForbiddenException extends ExceptionWithHttpState { + status = 403; + + constructor(message: string = 'Forbidden') { + super(403, message); + } +} diff --git a/backend/src/exceptions/not-found-exception.ts b/backend/src/exceptions/not-found-exception.ts new file mode 100644 index 00000000..a3e7d762 --- /dev/null +++ b/backend/src/exceptions/not-found-exception.ts @@ -0,0 +1,12 @@ +import { ExceptionWithHttpState } from './exception-with-http-state.js'; + +/** + * Exception for HTTP 404 Not Found + */ +export class NotFoundException extends ExceptionWithHttpState { + public status = 404; + + constructor(error: string) { + super(404, error); + } +} diff --git a/backend/src/exceptions/unauthorized-exception.ts b/backend/src/exceptions/unauthorized-exception.ts new file mode 100644 index 00000000..7ea9aca8 --- /dev/null +++ b/backend/src/exceptions/unauthorized-exception.ts @@ -0,0 +1,10 @@ +import { ExceptionWithHttpState } from './exception-with-http-state.js'; + +/** + * Exception for HTTP 401 Unauthorized + */ +export class UnauthorizedException extends ExceptionWithHttpState { + constructor(message: string = 'Unauthorized') { + super(401, message); + } +} diff --git a/backend/src/interfaces/student.ts b/backend/src/interfaces/student.ts index 079b355b..ecce8f89 100644 --- a/backend/src/interfaces/student.ts +++ b/backend/src/interfaces/student.ts @@ -1,4 +1,5 @@ import { Student } from '../entities/users/student.entity.js'; +import { getStudentRepository } from '../data/repositories.js'; export interface StudentDTO { id: string; @@ -23,7 +24,9 @@ export function mapToStudentDTO(student: Student): StudentDTO { } export function mapToStudent(studentData: StudentDTO): Student { - const student = new Student(studentData.username, studentData.firstName, studentData.lastName); - - return student; + return getStudentRepository().create({ + username: studentData.username, + firstName: studentData.firstName, + lastName: studentData.lastName, + }); } diff --git a/backend/src/interfaces/teacher.ts b/backend/src/interfaces/teacher.ts index 4dd6adb4..31b4723f 100644 --- a/backend/src/interfaces/teacher.ts +++ b/backend/src/interfaces/teacher.ts @@ -1,4 +1,5 @@ import { Teacher } from '../entities/users/teacher.entity.js'; +import { getTeacherRepository } from '../data/repositories.js'; export interface TeacherDTO { id: string; @@ -22,8 +23,10 @@ export function mapToTeacherDTO(teacher: Teacher): TeacherDTO { }; } -export function mapToTeacher(TeacherData: TeacherDTO): Teacher { - const teacher = new Teacher(TeacherData.username, TeacherData.firstName, TeacherData.lastName); - - return teacher; +export function mapToTeacher(teacherData: TeacherDTO): Teacher { + return getTeacherRepository().create({ + username: teacherData.username, + firstName: teacherData.firstName, + lastName: teacherData.lastName, + }); } diff --git a/backend/src/middleware/auth/auth.ts b/backend/src/middleware/auth/auth.ts index 5ff5a53c..4e651b4b 100644 --- a/backend/src/middleware/auth/auth.ts +++ b/backend/src/middleware/auth/auth.ts @@ -6,7 +6,8 @@ import * as express from 'express'; import * as jwt from 'jsonwebtoken'; import { AuthenticatedRequest } from './authenticated-request.js'; import { AuthenticationInfo } from './authentication-info.js'; -import { ForbiddenException, UnauthorizedException } from '../../exceptions.js'; +import { UnauthorizedException } from '../../exceptions/unauthorized-exception.js'; +import { ForbiddenException } from '../../exceptions/forbidden-exception.js'; const JWKS_CACHE = true; const JWKS_RATE_LIMIT = true; diff --git a/backend/src/middleware/error-handling/error-handler.ts b/backend/src/middleware/error-handling/error-handler.ts new file mode 100644 index 00000000..f2806938 --- /dev/null +++ b/backend/src/middleware/error-handling/error-handler.ts @@ -0,0 +1,15 @@ +import { NextFunction, Request, Response } from 'express'; +import { getLogger, Logger } from '../../logging/initalize.js'; +import { ExceptionWithHttpState } from '../../exceptions/exception-with-http-state.js'; + +const logger: Logger = getLogger(); + +export function errorHandler(err: unknown, req: Request, res: Response, _: NextFunction): void { + if (err instanceof ExceptionWithHttpState) { + logger.warn(`An error occurred while handling a request: ${err} (-> HTTP ${err.status})`); + res.status(err.status).json(err); + } else { + logger.error(`Unexpected error occurred while handing a request: ${JSON.stringify(err)}`); + res.status(500).json(err); + } +} diff --git a/backend/src/mikro-orm.config.ts b/backend/src/mikro-orm.config.ts index f2fb2bae..9e7800b0 100644 --- a/backend/src/mikro-orm.config.ts +++ b/backend/src/mikro-orm.config.ts @@ -49,6 +49,7 @@ function config(testingMode: boolean = false): Options { dbName: getEnvVar(EnvVars.DbName), subscribers: [new SqliteAutoincrementSubscriber()], entities: entities, + persistOnCreate: false, // Do not implicitly save entities when they are created via `create`. // EntitiesTs: entitiesTs, // Workaround: vitest: `TypeError: Unknown file extension ".ts"` (ERR_UNKNOWN_FILE_EXTENSION) @@ -65,6 +66,7 @@ function config(testingMode: boolean = false): Options { user: getEnvVar(EnvVars.DbUsername), password: getEnvVar(EnvVars.DbPassword), entities: entities, + persistOnCreate: false, // Do not implicitly save entities when they are created via `create`. // EntitiesTs: entitiesTs, // Logging diff --git a/backend/src/services/students.ts b/backend/src/services/students.ts index 4775c8a4..66327b6d 100644 --- a/backend/src/services/students.ts +++ b/backend/src/services/students.ts @@ -1,6 +1,4 @@ import { getClassRepository, getGroupRepository, getStudentRepository, getSubmissionRepository } from '../data/repositories.js'; -import { Class } from '../entities/classes/class.entity.js'; -import { Student } from '../entities/users/student.entity.js'; import { AssignmentDTO } from '../interfaces/assignment.js'; import { ClassDTO, mapToClassDTO } from '../interfaces/class.js'; import { GroupDTO, mapToGroupDTO, mapToGroupDTOId } from '../interfaces/group.js'; @@ -28,15 +26,9 @@ export async function getStudent(username: string): Promise { export async function createStudent(userData: StudentDTO): Promise { const studentRepository = getStudentRepository(); - try { - const newStudent = studentRepository.create(mapToStudent(userData)); - await studentRepository.save(newStudent); - - return mapToStudentDTO(newStudent); - } catch (e) { - console.log(e); - return null; - } + const newStudent = mapToStudent(userData); + await studentRepository.save(newStudent, { preventOverwrite: true }); + return mapToStudentDTO(newStudent); } export async function deleteStudent(username: string): Promise { @@ -87,9 +79,7 @@ export async function getStudentAssignments(username: string, full: boolean): Pr const classRepository = getClassRepository(); const classes = await classRepository.findByStudent(student); - const assignments = (await Promise.all(classes.map(async (cls) => await getAllAssignments(cls.classId!, full)))).flat(); - - return assignments; + return (await Promise.all(classes.map(async (cls) => await getAllAssignments(cls.classId!, full)))).flat(); } export async function getStudentGroups(username: string, full: boolean): Promise { diff --git a/backend/src/services/teachers.ts b/backend/src/services/teachers.ts index 77cea501..fced2b61 100644 --- a/backend/src/services/teachers.ts +++ b/backend/src/services/teachers.ts @@ -1,18 +1,9 @@ -import { - getClassRepository, - getLearningObjectRepository, - getQuestionRepository, - getStudentRepository, - getTeacherRepository, -} from '../data/repositories.js'; -import { Teacher } from '../entities/users/teacher.entity.js'; +import { getClassRepository, getLearningObjectRepository, getQuestionRepository, getTeacherRepository } from '../data/repositories.js'; import { ClassDTO, mapToClassDTO } from '../interfaces/class.js'; import { getClassStudents } from './classes.js'; import { StudentDTO } from '../interfaces/student.js'; import { mapToQuestionDTO, mapToQuestionId, QuestionDTO, QuestionId } from '../interfaces/question.js'; -import { mapToUser } from '../interfaces/user.js'; import { mapToTeacher, mapToTeacherDTO, TeacherDTO } from '../interfaces/teacher.js'; -import { teachersOnly } from '../middleware/auth/auth.js'; export async function getAllTeachers(full: boolean): Promise { const teacherRepository = getTeacherRepository(); @@ -34,15 +25,10 @@ export async function getTeacher(username: string): Promise { export async function createTeacher(userData: TeacherDTO): Promise { const teacherRepository = getTeacherRepository(); - try { - const newTeacher = teacherRepository.create(mapToTeacher(userData)); - await teacherRepository.save(newTeacher); + const newTeacher = mapToTeacher(userData); + await teacherRepository.save(newTeacher, { preventOverwrite: true }); - return mapToTeacherDTO(newTeacher); - } catch (e) { - console.log(e); - return null; - } + return mapToTeacherDTO(newTeacher); } export async function deleteTeacher(username: string): Promise { diff --git a/backend/tests/data/users/students.test.ts b/backend/tests/data/users/students.test.ts index 78800e1f..42822557 100644 --- a/backend/tests/data/users/students.test.ts +++ b/backend/tests/data/users/students.test.ts @@ -1,5 +1,4 @@ import { setupTestApp } from '../../setup-tests.js'; -import { Student } from '../../../src/entities/users/student.entity.js'; import { describe, it, expect, beforeAll } from 'vitest'; import { StudentRepository } from '../../../src/data/users/student-repository.js'; import { getStudentRepository } from '../../../src/data/repositories.js'; @@ -30,7 +29,7 @@ describe('StudentRepository', () => { }); it('should return the queried student after he was added', async () => { - await studentRepository.insert(new Student(username, firstName, lastName)); + await studentRepository.insert(studentRepository.create({ username, firstName, lastName })); const retrievedStudent = await studentRepository.findByUsername(username); expect(retrievedStudent).toBeTruthy(); diff --git a/backend/tests/data/users/teachers.test.ts b/backend/tests/data/users/teachers.test.ts index 0bd014a6..62ad6d81 100644 --- a/backend/tests/data/users/teachers.test.ts +++ b/backend/tests/data/users/teachers.test.ts @@ -2,7 +2,6 @@ import { describe, it, expect, beforeAll } from 'vitest'; import { TeacherRepository } from '../../../src/data/users/teacher-repository'; import { setupTestApp } from '../../setup-tests'; import { getTeacherRepository } from '../../../src/data/repositories'; -import { Teacher } from '../../../src/entities/users/teacher.entity'; const username = 'testteacher'; const firstName = 'John'; @@ -30,7 +29,7 @@ describe('TeacherRepository', () => { }); it('should return the queried teacher after he was added', async () => { - await teacherRepository.insert(new Teacher(username, firstName, lastName)); + await teacherRepository.insert(teacherRepository.create({ username, firstName, lastName })); const retrievedTeacher = await teacherRepository.findByUsername(username); expect(retrievedTeacher).toBeTruthy();