From a4a4bfff574f53239ab697858093174d091ac3dd Mon Sep 17 00:00:00 2001 From: lila Date: Tue, 28 Apr 2026 13:48:50 +0200 Subject: [PATCH 1/2] refactor: dependency injection for GameSessionStore via composition root --- apps/api/src/app.ts | 8 ++- apps/api/src/controllers/gameController.ts | 59 ++++++++--------- apps/api/src/routes/apiRouter.ts | 17 +++-- apps/api/src/routes/gameRouter.ts | 18 ++++-- apps/api/src/services/gameService.test.ts | 74 ++++++++++++++-------- apps/api/src/services/gameService.ts | 10 +-- 6 files changed, 107 insertions(+), 79 deletions(-) diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index 635a92a..47c51e6 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -4,7 +4,8 @@ import { toNodeHandler } from "better-auth/node"; import cors from "cors"; import helmet from "helmet"; import { auth } from "./lib/auth.js"; -import { apiRouter } from "./routes/apiRouter.js"; +import { createApiRouter } from "./routes/apiRouter.js"; +import { InMemoryGameSessionStore } from "./gameSessionStore/index.js"; import { errorHandler } from "./middleware/errorHandler.js"; import { authLimiter } from "./middleware/rateLimiters.js"; @@ -23,7 +24,10 @@ export function createApp() { app.use("/api/auth", authLimiter); app.all("/api/auth/*splat", toNodeHandler(auth)); app.use(express.json()); - app.use("/api/v1", apiRouter); + + const store = new InMemoryGameSessionStore(); + app.use("/api/v1", createApiRouter(store)); + app.use(errorHandler); return app; diff --git a/apps/api/src/controllers/gameController.ts b/apps/api/src/controllers/gameController.ts index fa43369..5ab8e75 100644 --- a/apps/api/src/controllers/gameController.ts +++ b/apps/api/src/controllers/gameController.ts @@ -2,41 +2,32 @@ import type { Request, Response, NextFunction } from "express"; import { GameRequestSchema, AnswerSubmissionSchema } from "@lila/shared"; import { createGameSession, evaluateAnswer } from "../services/gameService.js"; import { ValidationError } from "../errors/AppError.js"; +import type { GameSessionStore } from "../gameSessionStore/index.js"; -export const createGame = async ( - req: Request, - res: Response, - next: NextFunction, -) => { - try { - const gameSettings = GameRequestSchema.safeParse(req.body); - - if (!gameSettings.success) { - throw new ValidationError(gameSettings.error.message); +export const createGameController = (store: GameSessionStore) => ({ + createGame: async (req: Request, res: Response, next: NextFunction) => { + try { + const gameSettings = GameRequestSchema.safeParse(req.body); + if (!gameSettings.success) { + throw new ValidationError(gameSettings.error.message); + } + const gameQuestions = await createGameSession(gameSettings.data, store); + res.json({ success: true, data: gameQuestions }); + } catch (error) { + next(error); } + }, - const gameQuestions = await createGameSession(gameSettings.data); - res.json({ success: true, data: gameQuestions }); - } catch (error) { - next(error); - } -}; - -export const submitAnswer = async ( - req: Request, - res: Response, - next: NextFunction, -) => { - try { - const submission = AnswerSubmissionSchema.safeParse(req.body); - - if (!submission.success) { - throw new ValidationError(submission.error.message); + submitAnswer: async (req: Request, res: Response, next: NextFunction) => { + try { + const submission = AnswerSubmissionSchema.safeParse(req.body); + if (!submission.success) { + throw new ValidationError(submission.error.message); + } + const result = await evaluateAnswer(submission.data, store); + res.json({ success: true, data: result }); + } catch (error) { + next(error); } - - const result = await evaluateAnswer(submission.data); - res.json({ success: true, data: result }); - } catch (error) { - next(error); - } -}; + }, +}); diff --git a/apps/api/src/routes/apiRouter.ts b/apps/api/src/routes/apiRouter.ts index f5ebd01..a0ea8d7 100644 --- a/apps/api/src/routes/apiRouter.ts +++ b/apps/api/src/routes/apiRouter.ts @@ -1,11 +1,16 @@ import express from "express"; -import { Router } from "express"; +import type { Router } from "express"; import { healthRouter } from "./healthRouter.js"; -import { gameRouter } from "./gameRouter.js"; +import { createGameRouter } from "./gameRouter.js"; import { lobbyRouter } from "./lobbyRouter.js"; +import type { GameSessionStore } from "../gameSessionStore/index.js"; -export const apiRouter: Router = express.Router(); +export const createApiRouter = (store: GameSessionStore): Router => { + const router = express.Router(); -apiRouter.use("/health", healthRouter); -apiRouter.use("/game", gameRouter); -apiRouter.use("/lobbies", lobbyRouter); + router.use("/health", healthRouter); + router.use("/game", createGameRouter(store)); + router.use("/lobbies", lobbyRouter); + + return router; +}; diff --git a/apps/api/src/routes/gameRouter.ts b/apps/api/src/routes/gameRouter.ts index 850a146..08aeb26 100644 --- a/apps/api/src/routes/gameRouter.ts +++ b/apps/api/src/routes/gameRouter.ts @@ -1,13 +1,19 @@ import express from "express"; import type { Router } from "express"; -import { createGame, submitAnswer } from "../controllers/gameController.js"; +import { createGameController } from "../controllers/gameController.js"; import { requireAuth } from "../middleware/authMiddleware.js"; import { gameLimiter } from "../middleware/rateLimiters.js"; +import type { GameSessionStore } from "../gameSessionStore/index.js"; -export const gameRouter: Router = express.Router(); +export const createGameRouter = (store: GameSessionStore): Router => { + const router = express.Router(); + const controller = createGameController(store); -gameRouter.use(requireAuth); -gameRouter.use(gameLimiter); + router.use(requireAuth); + router.use(gameLimiter); -gameRouter.post("/start", createGame); -gameRouter.post("/answer", submitAnswer); + router.post("/start", controller.createGame); + router.post("/answer", controller.submitAnswer); + + return router; +}; diff --git a/apps/api/src/services/gameService.test.ts b/apps/api/src/services/gameService.test.ts index 2c7daf8..9299b06 100644 --- a/apps/api/src/services/gameService.test.ts +++ b/apps/api/src/services/gameService.test.ts @@ -5,6 +5,7 @@ vi.mock("@lila/db", () => ({ getGameTerms: vi.fn(), getDistractors: vi.fn() })); import { getGameTerms, getDistractors } from "@lila/db"; import { createGameSession, evaluateAnswer } from "./gameService.js"; +import { InMemoryGameSessionStore } from "../gameSessionStore/index.js"; const mockGetGameTerms = vi.mocked(getGameTerms); const mockGetDistractors = vi.mocked(getDistractors); @@ -35,15 +36,22 @@ beforeEach(() => { }); describe("createGameSession", () => { + + let store: InMemoryGameSessionStore; + + beforeEach(() => { + store = new InMemoryGameSessionStore(); + }); + it("returns a session with the correct number of questions", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); expect(session.sessionId).toBeDefined(); expect(session.questions).toHaveLength(3); }); it("each question has exactly 4 options", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); for (const question of session.questions) { expect(question.options).toHaveLength(4); @@ -51,14 +59,14 @@ describe("createGameSession", () => { }); it("each question has a unique questionId", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); const ids = session.questions.map((q) => q.questionId); expect(new Set(ids).size).toBe(ids.length); }); it("options have sequential optionIds 0-3", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); for (const question of session.questions) { const optionIds = question.options.map((o) => o.optionId); @@ -67,7 +75,7 @@ describe("createGameSession", () => { }); it("the correct answer is always among the options", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); for (let i = 0; i < session.questions.length; i++) { const question = session.questions[i]!; @@ -79,7 +87,7 @@ describe("createGameSession", () => { }); it("distractors are never the correct answer", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); for (let i = 0; i < session.questions.length; i++) { const question = session.questions[i]!; @@ -95,7 +103,7 @@ describe("createGameSession", () => { }); it("sets the prompt from the source text", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); expect(session.questions[0]!.prompt).toBe("dog"); expect(session.questions[1]!.prompt).toBe("cat"); @@ -103,14 +111,14 @@ describe("createGameSession", () => { }); it("passes gloss through (null or string)", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); expect(session.questions[0]!.gloss).toBeNull(); expect(session.questions[2]!.gloss).toBe("a building for living in"); }); it("calls getGameTerms with the correct arguments", async () => { - await createGameSession(validRequest); + await createGameSession(validRequest, store); expect(mockGetGameTerms).toHaveBeenCalledWith( "en", @@ -122,7 +130,7 @@ describe("createGameSession", () => { }); it("calls getDistractors once per question", async () => { - await createGameSession(validRequest); + await createGameSession(validRequest, store); expect(mockGetDistractors).toHaveBeenCalledTimes(3); }); @@ -130,24 +138,35 @@ describe("createGameSession", () => { it("propagates unexpected errors from getGameTerms", async () => { mockGetGameTerms.mockRejectedValue(new Error("connection refused")); - await expect(createGameSession(validRequest)).rejects.toThrow( + await expect(createGameSession(validRequest, store)).rejects.toThrow( "connection refused", ); }); }); describe("evaluateAnswer", () => { + + let store: InMemoryGameSessionStore; + + beforeEach(() => { + store = new InMemoryGameSessionStore(); + }); + + it("returns isCorrect: true when the correct option is selected", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); const question = session.questions[0]!; const correctText = fakeTerms[0]!.targetText; const correctOption = question.options.find((o) => o.text === correctText)!; - const result = await evaluateAnswer({ - sessionId: session.sessionId, - questionId: question.questionId, - selectedOptionId: correctOption.optionId, - }); + const result = await evaluateAnswer( + { + sessionId: session.sessionId, + questionId: question.questionId, + selectedOptionId: correctOption.optionId, + }, + store, + ); expect(result.isCorrect).toBe(true); expect(result.correctOptionId).toBe(correctOption.optionId); @@ -155,17 +174,20 @@ describe("evaluateAnswer", () => { }); it("returns isCorrect: false when a wrong option is selected", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); const question = session.questions[0]!; const correctText = fakeTerms[0]!.targetText; const correctOption = question.options.find((o) => o.text === correctText)!; const wrongOption = question.options.find((o) => o.text !== correctText)!; - const result = await evaluateAnswer({ - sessionId: session.sessionId, - questionId: question.questionId, - selectedOptionId: wrongOption.optionId, - }); + const result = await evaluateAnswer( + { + sessionId: session.sessionId, + questionId: question.questionId, + selectedOptionId: wrongOption.optionId, + }, + store, + ); expect(result.isCorrect).toBe(false); expect(result.correctOptionId).toBe(correctOption.optionId); @@ -179,13 +201,13 @@ describe("evaluateAnswer", () => { selectedOptionId: 0, }; - await expect(evaluateAnswer(submission)).rejects.toThrow( + await expect(evaluateAnswer(submission, store)).rejects.toThrow( "Game session not found", ); }); it("throws NotFoundError for a non-existent question", async () => { - const session = await createGameSession(validRequest); + const session = await createGameSession(validRequest, store); const submission: AnswerSubmission = { sessionId: session.sessionId, @@ -193,7 +215,7 @@ describe("evaluateAnswer", () => { selectedOptionId: 0, }; - await expect(evaluateAnswer(submission)).rejects.toThrow( + await expect(evaluateAnswer(submission, store)).rejects.toThrow( "Question not found", ); }); diff --git a/apps/api/src/services/gameService.ts b/apps/api/src/services/gameService.ts index 64f90f6..4611bec 100644 --- a/apps/api/src/services/gameService.ts +++ b/apps/api/src/services/gameService.ts @@ -8,14 +8,13 @@ import type { AnswerSubmission, AnswerResult, } from "@lila/shared"; -import { InMemoryGameSessionStore } from "../gameSessionStore/index.js"; +import type { GameSessionStore } from "../gameSessionStore/index.js"; import { NotFoundError } from "../errors/AppError.js"; import { shuffleArray } from "../lib/utils.js"; -const gameSessionStore = new InMemoryGameSessionStore(); - export const createGameSession = async ( request: GameRequest, + store: GameSessionStore, ): Promise => { const terms = await getGameTerms( request.source_language, @@ -60,15 +59,16 @@ export const createGameSession = async ( ); const sessionId = randomUUID(); - await gameSessionStore.create(sessionId, { answers: answerKey }); + await store.create(sessionId, { answers: answerKey }); return { sessionId, questions }; }; export const evaluateAnswer = async ( submission: AnswerSubmission, + store: GameSessionStore, ): Promise => { - const session = await gameSessionStore.get(submission.sessionId); + const session = await store.get(submission.sessionId); if (!session) { throw new NotFoundError(`Game session not found: ${submission.sessionId}`); From 54705943fa86bed2a8cf3c5d45562dfb521c4bd9 Mon Sep 17 00:00:00 2001 From: lila Date: Tue, 28 Apr 2026 13:50:56 +0200 Subject: [PATCH 2/2] adding ticket for refactor: dependency injection for GameSessionStore via composition root --- documentation/tickets/t00004.md | 110 ++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 documentation/tickets/t00004.md diff --git a/documentation/tickets/t00004.md b/documentation/tickets/t00004.md new file mode 100644 index 0000000..f7d5fea --- /dev/null +++ b/documentation/tickets/t00004.md @@ -0,0 +1,110 @@ +# ADR: Dependency injection for GameSessionStore via composition root + +## Status + +Accepted + +## Date + +2026-04-28 + +## Context + +`gameService.ts` had a module-level singleton: + +```ts +const gameSessionStore = new InMemoryGameSessionStore(); +``` + +This made the store invisible to anything outside the file. The `GameSessionStore` interface existed to make the store swappable — but the singleton made that impossible without editing the service itself. Tests shared the same instance across every test run, creating the potential for ghost sessions leaking between tests. The controller also briefly owned the singleton during an intermediate step, which violated the principle that controllers should only handle HTTP concerns. + +## Decision + +Adopt a composition root pattern. The store is created once in `createApp()` and passed down through factory functions: `createApiRouter(store)` → `createGameRouter(store)` → `createGameController(store)` → service calls. Neither the controller nor the service knows which implementation they're working with — they both see `GameSessionStore`. + +## Options considered + +### Option A — Composition root ✅ + +Convert routers and controllers to factory functions. Create the store in `createApp()` and pass it down. The store is created once, at the top, and injected through the call chain. + +Chosen because: clean separation of concerns, no layer below `createApp()` needs to know the concrete implementation, swapping to `ValKeyGameSessionStore` is a one-line change in `app.ts`, and tests get fresh isolated store instances. + +### Option B — Keep singleton in controller + +Leave the store as a module-level singleton in `gameController.ts`. Controllers own the store lifetime. + +Rejected because: controllers should only handle HTTP concerns. Owning infrastructure lifetime is not an HTTP concern. + +### Option C — DI framework (tsyringe, inversify) + +Use a proper dependency injection container. + +Rejected because: overkill for the current scale. The composition root pattern achieves the same result with zero dependencies and no magic. + +## Consequences + +- Swapping `InMemoryGameSessionStore` for `ValKeyGameSessionStore` requires editing one line in `app.ts` +- Tests create fresh `InMemoryGameSessionStore` instances per test — no shared state, no ghost sessions +- Routers and controllers are now factory functions instead of module-level singletons — slightly more verbose but explicitly testable +- `gameController.test.ts` uses `createApp()` which owns the store — controller tests remain integration-style and unaffected +- All layers below `createApp()` depend only on the `GameSessionStore` interface, never the concrete implementation + +## Affected files + +- `apps/api/src/app.ts` — creates the store, passes to `createApiRouter` +- `apps/api/src/routes/apiRouter.ts` — converted to `createApiRouter(store)` factory +- `apps/api/src/routes/gameRouter.ts` — converted to `createGameRouter(store)` factory +- `apps/api/src/controllers/gameController.ts` — converted to `createGameController(store)` factory +- `apps/api/src/services/gameService.ts` — `store` parameter added to both functions, singleton removed +- `apps/api/src/services/gameService.test.ts` — fresh store per describe block via `beforeEach` + +## References + +- [Composition root pattern](https://blog.ploeh.dk/2011/07/28/CompositionRoot/) + +--- + +## Setup guide / implementation notes + +1. `gameService.ts` — remove module-level singleton, add `store: GameSessionStore` parameter to `createGameSession` and `evaluateAnswer` + +2. `gameController.ts` — convert exported functions to a factory: + + ```ts + export const createGameController = (store: GameSessionStore) => ({ + createGame: async (req, res, next) => { ... }, + submitAnswer: async (req, res, next) => { ... }, + }); + ``` + +3. `gameRouter.ts` — convert to factory: + + ```ts + export const createGameRouter = (store: GameSessionStore): Router => { + const router = express.Router(); + const controller = createGameController(store); + router.post("/start", controller.createGame); + router.post("/answer", controller.submitAnswer); + return router; + }; + ``` + +4. `apiRouter.ts` — convert to factory: + + ```ts + export const createApiRouter = (store: GameSessionStore): Router => { + const router = express.Router(); + router.use("/game", createGameRouter(store)); + return router; + }; + ``` + +5. `app.ts` — create the store at the composition root: + + ```ts + const store = new InMemoryGameSessionStore(); + app.use("/api/v1", createApiRouter(store)); + ``` + +6. `gameService.test.ts` — add `let store: InMemoryGameSessionStore` to each `describe` block, reset in `beforeEach`, pass to every service call