Skip to content

Commit

Permalink
Improve controller architecture (#1292)
Browse files Browse the repository at this point in the history
* Create abstract class `SingletonController` with `path` and `tags` properties
* Fix `Singleton` class
* Harmonize API and endpoint paths

Signed-off-by: Marvin A. Ruder <signed@mruder.dev>
  • Loading branch information
marvinruder authored May 14, 2024
1 parent 16c8ecc commit af4655c
Show file tree
Hide file tree
Showing 88 changed files with 782 additions and 844 deletions.
57 changes: 26 additions & 31 deletions packages/backend/src/controllers/AccountController.live.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
baseURL,
sessionEndpointPath,
accountEndpointPath,
accountAvatarEndpointSuffix,
} from "@rating-tracker/commons";
import { baseURL, sessionAPIPath, accountAPIPath, accountAvatarEndpointSuffix } from "@rating-tracker/commons";

import type { LiveTestSuite } from "../../test/liveTestHelpers";
import { expectRouteToBePrivate, supertest } from "../../test/liveTestHelpers";
Expand All @@ -15,7 +10,7 @@ export const tests: LiveTestSuite = [];
tests.push({
testName: "returns empty object when no user is logged in",
testFunction: async () => {
const res = await supertest.get(`${baseURL}${accountEndpointPath}`);
const res = await supertest.get(`${baseURL}${accountAPIPath}`);
expect(res.status).toBe(200);
expect(Object.keys(res.body)).toHaveLength(0);
},
Expand All @@ -24,7 +19,7 @@ tests.push({
tests.push({
testName: "provides current user’s information",
testFunction: async () => {
const res = await supertest.get(`${baseURL}${accountEndpointPath}`).set("Cookie", ["authToken=exampleSessionID"]);
const res = await supertest.get(`${baseURL}${accountAPIPath}`).set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(Object.keys(res.body).length).toBeGreaterThan(0);
expect(res.body.email).toBe("jane.doe@example.com");
Expand All @@ -40,9 +35,9 @@ tests.push({
tests.push({
testName: "provides current user’s avatar",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`);
const res = await supertest
.get(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.get(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(res.headers["content-type"]).toBe("image/jpeg");
Expand All @@ -53,18 +48,18 @@ tests.push({
tests.push({
testName: "validates the phone number",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}`, supertest.patch);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}`, supertest.patch);
let res = await supertest
.patch(`${baseURL}${accountEndpointPath}?phone=987654321`)
.patch(`${baseURL}${accountAPIPath}?phone=987654321`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(400);
res = await supertest
.patch(`${baseURL}${accountEndpointPath}?phone=+1%20234%20567%2D8900`)
.patch(`${baseURL}${accountAPIPath}?phone=+1%20234%20567%2D8900`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(400);

// Check that no changes were applied
res = await supertest.get(`${baseURL}${accountEndpointPath}`).set("Cookie", ["authToken=exampleSessionID"]);
res = await supertest.get(`${baseURL}${accountAPIPath}`).set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(res.body.phone).toBe("+123456789");
},
Expand All @@ -73,14 +68,14 @@ tests.push({
tests.push({
testName: "[unsafe] updates current user’s email address",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}`, supertest.patch);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}`, supertest.patch);
let res = await supertest
.patch(`${baseURL}${accountEndpointPath}?email=jane.doe.2%40example%2Ecom`)
.patch(`${baseURL}${accountAPIPath}?email=jane.doe.2%40example%2Ecom`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(204);

// Check that the changes were applied
res = await supertest.get(`${baseURL}${accountEndpointPath}`).set("Cookie", ["authToken=exampleSessionID"]);
res = await supertest.get(`${baseURL}${accountAPIPath}`).set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(res.body.email).toBe("jane.doe.2@example.com");
},
Expand All @@ -89,14 +84,14 @@ tests.push({
tests.push({
testName: "[unsafe] updates current user’s information without email address",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}`, supertest.patch);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}`, supertest.patch);
let res = await supertest
.patch(`${baseURL}${accountEndpointPath}?name=Jane%20Doe%20II%2E&phone=%2B987654321`)
.patch(`${baseURL}${accountAPIPath}?name=Jane%20Doe%20II%2E&phone=%2B987654321`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(204);

// Check that the changes were applied
res = await supertest.get(`${baseURL}${accountEndpointPath}`).set("Cookie", ["authToken=exampleSessionID"]);
res = await supertest.get(`${baseURL}${accountAPIPath}`).set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(res.body.email).toBe("jane.doe@example.com");
expect(res.body.name).toBe("Jane Doe II.");
Expand All @@ -108,29 +103,29 @@ tests.push({
testName: "[unsafe] updates current user’s avatar",
testFunction: async () => {
await expectRouteToBePrivate(
`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`,
`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`,
supertest.put,
"image/avif",
);

// Only certain media types are allowed
let res = await supertest
.put(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.put(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Content-Type", "image/png")
.send("Another fancy avatar image")
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(415);

res = await supertest
.put(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.put(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Content-Type", "image/avif")
.send("Another fancy avatar image")
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(201);

// Check that the changes were applied
res = await supertest
.get(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.get(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(200);
expect(res.headers["content-type"]).toBe("image/avif");
Expand All @@ -142,7 +137,7 @@ tests.push({
testName: "disallows changing own access rights",
testFunction: async () => {
const res = await supertest
.patch(`${baseURL}${accountEndpointPath}?accessRights=255`)
.patch(`${baseURL}${accountAPIPath}?accessRights=255`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(400);
},
Expand All @@ -151,29 +146,29 @@ tests.push({
tests.push({
testName: "[unsafe] deletes the current user",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}`, supertest.delete);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}`, supertest.delete);
let res = await supertest
.delete(`${baseURL}${accountEndpointPath}`)
.delete(`${baseURL}${accountAPIPath}`)
.set("Cookie", ["authToken=anotherExampleSessionID"]);

// Check that the user was deleted
res = await supertest.head(`${baseURL}${sessionEndpointPath}`).set("Cookie", ["authToken=anotherExampleSessionID"]);
res = await supertest.head(`${baseURL}${sessionAPIPath}`).set("Cookie", ["authToken=anotherExampleSessionID"]);
expect(res.status).toBe(401);
},
});

tests.push({
testName: "[unsafe] deletes the current user’s avatar",
testFunction: async () => {
await expectRouteToBePrivate(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`, supertest.delete);
await expectRouteToBePrivate(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`, supertest.delete);
let res = await supertest
.delete(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.delete(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(204);

// Check that the avatar was deleted
res = await supertest
.get(`${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`)
.get(`${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`)
.set("Cookie", ["authToken=exampleSessionID"]);
expect(res.status).toBe(404);
},
Expand Down
37 changes: 14 additions & 23 deletions packages/backend/src/controllers/AccountController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { User } from "@rating-tracker/commons";
import { GENERAL_ACCESS, accountAvatarEndpointSuffix, accountEndpointPath, baseURL } from "@rating-tracker/commons";
import { GENERAL_ACCESS, accountAvatarEndpointSuffix, accountAPIPath, baseURL } from "@rating-tracker/commons";
import type { Request, RequestHandler, Response } from "express";
import express from "express";

Expand All @@ -11,35 +11,36 @@ import { created, noContent, okAvatar, okUser } from "../openapi/responses/succe
import { updateSession } from "../redis/repositories/sessionRepository";
import APIError from "../utils/APIError";
import Endpoint from "../utils/Endpoint";
import Singleton from "../utils/Singleton";

import SingletonController from "./SingletonController";

/**
* This class is responsible for handling a user’s own account information.
*/
class AccountController extends Singleton {
class AccountController extends SingletonController {
path = accountAPIPath;
tags = ["Account API"];

/**
* Returns the current user fetched during session validation. If no user is logged in, an empty object is returned.
* @param _ The request.
* @param res The response.
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "getAccount",
summary: "Get the current user",
description:
"Returns the current user fetched during session validation. " +
"If no user is logged in, an empty object is returned.",
responses: { "200": okUser },
},
method: "get",
path: accountEndpointPath,
path: "",
accessRights: 0,
})
get: RequestHandler = (_: Request, res: Response) => {
const user: User = { ...res.locals.user };
if (user?.avatar?.startsWith("data:"))
user.avatar = `${baseURL}${accountEndpointPath}${accountAvatarEndpointSuffix}`;
if (user?.avatar?.startsWith("data:")) user.avatar = `${baseURL}${accountAPIPath}${accountAvatarEndpointSuffix}`;
res.status(200).json(user).end();
};

Expand All @@ -50,8 +51,6 @@ class AccountController extends Singleton {
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "getAvatar",
summary: "Get the avatar of the current user",
description: "Returns the avatar of the current user fetched during session validation.",
parameters: [
Expand All @@ -72,7 +71,7 @@ class AccountController extends Singleton {
},
},
method: "get",
path: accountEndpointPath + accountAvatarEndpointSuffix,
path: accountAvatarEndpointSuffix,
accessRights: GENERAL_ACCESS,
})
getAvatar: RequestHandler = async (_: Request, res: Response) => {
Expand All @@ -95,8 +94,6 @@ class AccountController extends Singleton {
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "updateAccount",
summary: "Update the current user",
description: "Updates the current user in the database.",
parameters: [
Expand All @@ -108,7 +105,7 @@ class AccountController extends Singleton {
responses: { "204": noContent, "401": unauthorized },
},
method: "patch",
path: accountEndpointPath,
path: "",
accessRights: GENERAL_ACCESS,
})
patch: RequestHandler = async (req: Request, res: Response) => {
Expand Down Expand Up @@ -137,15 +134,13 @@ class AccountController extends Singleton {
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "createAvatar",
summary: "Create an avatar for the current user",
description: "Creates an avatar for the current user in the database.",
requestBody: { required: true, content: { "image/avif": { schema: { type: "string", format: "binary" } } } },
responses: { "201": created, "401": unauthorized, "415": unsupportedMediaType },
},
method: "put",
path: accountEndpointPath + accountAvatarEndpointSuffix,
path: accountAvatarEndpointSuffix,
accessRights: GENERAL_ACCESS,
bodyParser: express.raw({ type: ["image/avif"], limit: "1mb" }),
})
Expand All @@ -172,14 +167,12 @@ class AccountController extends Singleton {
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "deleteAccount",
summary: "Delete the current user",
description: "Deletes the current user from the database.",
responses: { "204": noContent, "401": unauthorized },
},
method: "delete",
path: accountEndpointPath,
path: "",
accessRights: GENERAL_ACCESS,
})
delete: RequestHandler = async (_: Request, res: Response) => {
Expand All @@ -194,14 +187,12 @@ class AccountController extends Singleton {
*/
@Endpoint({
spec: {
tags: ["Account API"],
operationId: "deleteAvatar",
summary: "Delete the avatar of the current user",
description: "Deletes the avatar of the current user from the database.",
responses: { "204": noContent, "401": unauthorized },
},
method: "delete",
path: accountEndpointPath + accountAvatarEndpointSuffix,
path: accountAvatarEndpointSuffix,
accessRights: GENERAL_ACCESS,
})
deleteAvatar: RequestHandler = async (_: Request, res: Response) => {
Expand Down
Loading

0 comments on commit af4655c

Please sign in to comment.