181 lines
10 KiB
Markdown
181 lines
10 KiB
Markdown
# Implementation Plan: Job 917 — Category Icons
|
||
|
||
## 0. Status
|
||
|
||
**NOT IMPLEMENTED.** The repository contains zero PNG asset files anywhere under
|
||
`/repo` (confirmed via `find /repo -name "*.png"`). The database rows created by
|
||
`UpdateSystemCategoryKeys1700000000300` and
|
||
`RepairCategorySchemaAndSystemCategories1700000000400` reference
|
||
`/uploads/icons/{CRY,HW,MSC,PWN,REV,WEB}.png`, but nothing on bootstrap writes
|
||
those PNGs to `UPLOAD_DIR/icons`. `setup.sh` only `mkdir -p`s the directory; it
|
||
does not populate it. As a result `GET /uploads/icons/CRY.png` returns 404 and
|
||
the challenges board shows broken images.
|
||
|
||
The job must: generate the canonical six system-category PNG icons at
|
||
`UPLOAD_DIR/icons/{KEY}.png` on every startup, idempotently, so a fresh clone
|
||
that runs `setup.sh` and `npm start` has working category icons immediately.
|
||
|
||
## 1. Architectural Reconnaissance
|
||
|
||
- **Codebase style & conventions:** TypeScript strict, NestJS 10 modules, ESM
|
||
CommonJS hybrid, async/await everywhere. Migrations are forward-only classes
|
||
implementing `MigrationInterface`; service-layer logic in `*.service.ts`. A
|
||
recent precedent (Job 919 filesystem recovery) shows the codebase prefers
|
||
small, idempotent startup hooks in `DatabaseInitService` rather than changes
|
||
to individual migrations.
|
||
- **Data Layer:** TypeORM + `better-sqlite3`. DB at `DATABASE_PATH`
|
||
(default `./data/db.sqlite`). Upload files live at `UPLOAD_DIR`
|
||
(default `./data/uploads`) and are served by `app.use('/uploads',
|
||
express.static(uploadDir))` in `backend/src/main.ts:79`. Migrations are
|
||
registered in `backend/src/database/database.module.ts:40`.
|
||
- **Test Framework & Structure:** Jest with a multi-project config at
|
||
`tests/jest.config.js`. All backend specs live in `tests/backend/**.spec.ts`,
|
||
run via `npm test` (or `npm run test:backend`). All frontend specs live in
|
||
`tests/frontend/**.spec.ts`. New tests must follow the same layout — no
|
||
source-side spec files.
|
||
- **Required Tools & Dependencies:** No new dependencies are needed. `sharp`
|
||
(already a `backend` runtime dep at `^0.35.3`) provides PNG generation, and
|
||
the root `devDependencies` already pin `sharp` and `jest`. The existing
|
||
`admin-icon-normalize.spec.ts` test already verifies that the same sharp
|
||
pipeline produces a valid 128×128 PNG.
|
||
|
||
## 2. Impacted Files
|
||
|
||
- **To Modify:**
|
||
- `backend/src/database/database-init.service.ts` — call the new icon-seed
|
||
step after `runMigrations` so freshly migrated databases (and warm databases
|
||
that pre-date this job) both end up with canonical icons on disk.
|
||
- `setup.sh` — leave as-is (the directory is already created); no shell-side
|
||
asset copy is required because the generation will happen at startup.
|
||
|
||
- **To Create:**
|
||
- `backend/src/database/system-category-icons.ts` — small pure helper that
|
||
exports `CANONICAL_SYSTEM_ICON_KEYS` (the six canonical keys in fixed order:
|
||
`['CRY','HW','MSC','PWN','REV','WEB']`) and an async
|
||
`seedSystemCategoryIcons(uploadDir)` function. Re-importing this from the
|
||
migration that owns the canonical list would create a circular dependency
|
||
through `database.module.ts`, so the helper mirrors the canonical set
|
||
directly and references the migration file via comment for traceability.
|
||
It generates a deterministic 128×128 PNG for each key using `sharp` (solid
|
||
color background derived from a stable hash of the key, plus the 2–6-letter
|
||
abbreviation rendered in white) and writes
|
||
`<uploadDir>/icons/<KEY>.png` only if the file does not already exist
|
||
(idempotent — does not overwrite an admin-uploaded icon).
|
||
- `tests/backend/system-category-icons.spec.ts` — verifies that
|
||
`seedSystemCategoryIcons` (a) writes six PNGs into a temp
|
||
`icons/` directory, (b) writes files whose on-disk size is > 0 and whose
|
||
`sharp().metadata()` reports `format === 'png'`, `width === 128`,
|
||
`height === 128`, (c) is idempotent — running it twice does not change
|
||
file contents when the first run's bytes are preserved (assert by checking
|
||
`stat.mtimeMs` is unchanged or by capturing the buffer once and comparing),
|
||
(d) does not throw when the target directory is missing (it creates it),
|
||
and (e) covers exactly the canonical six keys (CRY, HW, MSC, PWN, REV,
|
||
WEB). One focused test per behavior above; no mocks of `sharp` (use a
|
||
real `os.tmpdir()`-based dir for the assertions).
|
||
|
||
## 3. Proposed Changes
|
||
|
||
1. **Helper module (`backend/src/database/system-category-icons.ts`):**
|
||
- `import * as fs from 'fs'` and `import * as path from 'path'` and
|
||
`import sharp from 'sharp'`.
|
||
- Export `const CANONICAL_SYSTEM_ICON_KEYS = ['CRY','HW','MSC','PWN','REV','WEB'] as const;`
|
||
with `export type CanonicalSystemIconKey = (typeof CANONICAL_SYSTEM_ICON_KEYS)[number];`
|
||
— the same six keys declared in
|
||
`1700000000400-RepairCategorySchemaAndSystemCategories.ts:13` as
|
||
`CANONICAL_SYSTEM_CATEGORIES`. The duplication is intentional to avoid a
|
||
migration → runtime import cycle; a code comment must point at the
|
||
migration file as the canonical source of truth so future jobs keep them
|
||
in sync.
|
||
- Define `export const SYSTEM_ICON_SIZE = 128` matching the existing
|
||
`uploads.controller.ts:88` resize target so generated icons look
|
||
consistent with admin-uploaded ones.
|
||
- Implement
|
||
`export async function seedSystemCategoryIcons(uploadDir: string): Promise<{ written: string[]; skipped: string[] }>`:
|
||
- Resolve `iconsDir = path.join(uploadDir, 'icons')`, create it
|
||
recursively if missing.
|
||
- For each `key` in `CANONICAL_SYSTEM_ICON_KEYS`, target file
|
||
`path.join(iconsDir, `${key}.png`)`.
|
||
- If the file already exists and is non-empty, push to `skipped`.
|
||
- Otherwise generate a 128×128 PNG via
|
||
`sharp({ create: { width, height, channels: 4, background: <color for key> } }).png().toBuffer()`
|
||
(color picked by a deterministic hash of `key` so each icon is visually
|
||
distinct but reproducible across machines/containers). Wrap in
|
||
`try/catch` and on failure `throw new Error('Failed to seed system
|
||
category icon ' + key + ': ' + err.message)`. On success `fs.writeFileSync`
|
||
the buffer to disk and push the absolute path to `written`.
|
||
- Return `{ written, skipped }`.
|
||
- Add a top-of-file comment noting that this helper must stay in sync with
|
||
`CANONICAL_SYSTEM_CATEGORIES` in
|
||
`1700000000400-RepairCategorySchemaAndSystemCategories.ts` and with the
|
||
`iconPath` URLs hard-coded in
|
||
`1700000000300-UpdateSystemCategoryKeys.ts:8-13` and
|
||
`1700000000400-RepairCategorySchemaAndSystemCategories.ts:19-54`.
|
||
|
||
2. **Bootstrap hook (`backend/src/database/database-init.service.ts`):**
|
||
- Add `import { seedSystemCategoryIcons } from './system-category-icons';`
|
||
near the existing top-of-file imports.
|
||
- In `init()`, after the `runMigrations` block and after `verifySeed()`,
|
||
call a new `await this.ensureSystemCategoryIcons()`. The call must run
|
||
on every startup (not gated on `pendingList.length > 0`) so warm
|
||
databases that pre-date this job also get the icons on next boot.
|
||
- Implement
|
||
`private async ensureSystemCategoryIcons(): Promise<void>`:
|
||
- `const uploadDir = path.resolve(this.config.get<string>('UPLOAD_DIR', './data/uploads'));`
|
||
- If `uploadDir === ':memory:'` or contains `:memory:` skip with a debug
|
||
log (matches how the module already short-circuits in-memory test DBs
|
||
in some places and avoids touching whatever a test has wired up).
|
||
- Otherwise `const result = await seedSystemCategoryIcons(uploadDir);` and
|
||
log: `System category icons: written=${result.written.length}
|
||
skipped=${result.skipped.length}`. A failure must `this.logger.warn`
|
||
and **not** abort startup (icons are recoverable and a missing icon
|
||
does not block HTTP traffic — mirroring the
|
||
`verifySeed` "best-effort" pattern at line 60).
|
||
|
||
3. **No changes to `setup.sh`.** The directory is already created there and
|
||
the runtime hook handles the asset creation. (Adding an out-of-band
|
||
`sharp` invocation in bash would require either bundling `sharp` into the
|
||
shell script or shelling out to `node -e` — both are heavier than the
|
||
in-process hook.)
|
||
|
||
4. **No changes to existing migrations.** The DB rows already reference the
|
||
right URLs; we only need the files to exist on disk.
|
||
|
||
## 4. Test Strategy
|
||
|
||
- **Target Unit Test File:** `tests/backend/system-category-icons.spec.ts`
|
||
(single file, ~30–50 lines, no test infrastructure beyond `os.tmpdir()`
|
||
and `fs.mkdtempSync`).
|
||
- **Mocking Strategy:** No mocks. Use real `sharp` (already a project dev
|
||
dep, validated by `tests/backend/admin-icon-normalize.spec.ts:1`) and a
|
||
fresh per-test temp directory under `os.tmpdir()` (e.g.
|
||
`fs.mkdtempSync(path.join(os.tmpdir(), 'hipctf-icons-'))`) that is removed
|
||
in `afterEach`. Asserting on real files via `fs.statSync` and
|
||
`sharp(buf).metadata()` is faster and more reliable than mocking the fs
|
||
module.
|
||
- **Cases (keep minimal, success-path only):**
|
||
1. `seedSystemCategoryIcons` writes six PNGs into a fresh temp dir and
|
||
each one is a valid 128×128 PNG with `format === 'png'`.
|
||
2. Calling it twice with the same dir is idempotent — the second call
|
||
reports all six as `skipped` and does not touch `mtimeMs` of the
|
||
existing files (this is the contract that protects admin-uploaded
|
||
icons: if an admin uploads a `CRY.png`, a later restart must not
|
||
overwrite it).
|
||
3. The helper creates the `icons/` subdirectory when the parent dir is
|
||
empty (covers the fresh-clone case the job explicitly calls out).
|
||
- All tests must run under `npm test` (single command from repo root) and
|
||
complete in well under a second on a warm cache. No UI, no visual
|
||
verification, no complex infrastructure.
|
||
|
||
## 5. Cross-cutting Notes
|
||
|
||
- The existing `admin-icon-normalize.spec.ts` already proves the `sharp`
|
||
pipeline at the size we need (128×128 PNG) and is the closest precedent for
|
||
the new spec — the new test should follow its style (real sharp, real
|
||
buffers, no jest mocks).
|
||
- Do **not** modify the canonical migration files. They are forward-only and
|
||
already correct; the bug is purely an asset-on-disk problem.
|
||
- Do **not** add new npm dependencies. `sharp` is sufficient.
|
||
- The `/data` persistent volume rule applies only if any test wants to share
|
||
state across runs — this job does not need to; per-test temp dirs are fine
|
||
and isolated.
|