# Backend Hardening Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Make schema changes safe via ordered migrations, fix 17 confirmed defects, restructure business logic into `internal/` with port interfaces, and introduce a CI workflow. **Architecture:** Three sequential batches. Batch A builds the migration system and CI foundation. Batch B fixes bugs B1–B13 with reproduction tests. Batch C performs structural reorganization (ports/store split/media/upload/scanner) with contract tests and fake-based unit tests. Each batch keeps tests green throughout. **Tech Stack:** Go 1.26, gin, pgx/v5, go-redis/v9, PostgreSQL 16, Redis 7, GitHub Actions (Gitea Actions compatible), embedded SQL migrations with pg advisory lock. ## Global Constraints - Branch: `fix/backend-hardening` (already created). - CHANGELOG entries in `docs/CHANGELOG.md`: English line then Chinese line on consecutive lines, different entries separated by blank line. - `docs/README.md` and `docs/README_zh.md` stay content-equivalent; update both in same change. - API contract: zero changes except B6/B7/B8 error semantics (recorded in changelog). - Migration: self-hosted embedded SQL + `schema_migrations` + pg advisory lock. No down migrations. - Refactor: small consumer-side interfaces in `internal/ports`, hand-written fakes in `internal/ports/portsfake`, no DI framework, no mock generators. - CI: standard GitHub Actions syntax in `.github/workflows/`; Gitea Actions compatible. Runner-less until Gitea runner registered; local gate mandatory before each batch merge. - Local gate before each batch merge: dev compose PG+Redis up, then `go vet ./... && gofmt -l . && go test -p 1 -count=1 ./...` confirming 0 skip, plus `scripts/smoke.sh`. --- ## Part A — Migration System + CI + Quick Fixes ### Task 1: Migration System **Files:** - Create: `backend/internal/db/migrations/0001_baseline.sql` - Modify: `backend/internal/db/db.go` - Delete: `backend/internal/db/schema.sql` (after copying to migrations) **Interfaces:** - Consumes: `pgxpool.Pool` from `db.Connect` - Produces: `db.Migrate(ctx, pool) error` — same signature, different internals; `schema_migrations` table auto-created - [ ] **Step 1: Create baseline migration file** Copy current `backend/internal/db/schema.sql` content verbatim into `backend/internal/db/migrations/0001_baseline.sql`. Content is all `CREATE TABLE IF NOT EXISTS` and `CREATE INDEX IF NOT EXISTS` statements for users, libraries, books, reading_progress, bookmarks. - [ ] **Step 2: Rewrite `db.go` with migration system** Replace `backend/internal/db/db.go` entirely with the new migration system: ```go package db import ( "context" "embed" "fmt" "io/fs" "log" "regexp" "sort" "strings" "github.com/jackc/pgx/v5/pgxpool" ) //go:embed migrations var migrationsFS embed.FS const advisoryLockKey int64 = 0x424C4D49 // "BLMI" var migrationNameRe = regexp.MustCompile(`^\d{4}_[a-z0-9_]+\.sql$`) func Connect(ctx context.Context, url string) (*pgxpool.Pool, error) { cfg, err := pgxpool.ParseConfig(url) if err != nil { return nil, err } cfg.MaxConns = 10 return pgxpool.NewWithConfig(ctx, cfg) } func Migrate(ctx context.Context, p *pgxpool.Pool) error { // 1. Acquire advisory lock — serializes concurrent replicas. if _, err := p.Exec(ctx, "SELECT pg_advisory_lock($1)", advisoryLockKey); err != nil { return fmt.Errorf("advisory lock: %w", err) } defer func() { if _, err := p.Exec(ctx, "SELECT pg_advisory_unlock($1)", advisoryLockKey); err != nil { log.Printf("advisory unlock: %v", err) } }() // 2. Create tracking table. if _, err := p.Exec(ctx, `CREATE TABLE IF NOT EXISTS schema_migrations ( version BIGINT PRIMARY KEY, name TEXT NOT NULL, applied_at TIMESTAMPTZ NOT NULL DEFAULT now())`); err != nil { return fmt.Errorf("create schema_migrations: %w", err) } // 3. Read + validate embedded files. entries, err := fs.ReadDir(migrationsFS, "migrations") if err != nil { return fmt.Errorf("read migrations dir: %w", err) } var files []string for _, e := range entries { name := e.Name() if !migrationNameRe.MatchString(name) { panic(fmt.Sprintf("invalid migration filename: %q", name)) } files = append(files, name) } sort.Strings(files) // 4. Baseline: empty schema_migrations + books exists → mark 0001 applied. var count int if err := p.QueryRow(ctx, "SELECT count(*) FROM schema_migrations").Scan(&count); err != nil { return fmt.Errorf("count migrations: %w", err) } if count == 0 { var hasBooks bool if err := p.QueryRow(ctx, "SELECT to_regclass('books') IS NOT NULL").Scan(&hasBooks); err != nil { return fmt.Errorf("check books: %w", err) } if hasBooks && len(files) > 0 && strings.HasPrefix(files[0], "0001_") { if _, err := p.Exec(ctx, "INSERT INTO schema_migrations (version, name) VALUES ($1,$2)", 1, files[0]); err != nil { return fmt.Errorf("baseline: %w", err) } log.Printf("migration baseline: %s (existing db)", files[0]) files = files[1:] } } // 5. Load applied set. applied := map[int64]bool{} rows, err := p.Query(ctx, "SELECT version FROM schema_migrations") if err != nil { return fmt.Errorf("list applied: %w", err) } defer rows.Close() for rows.Next() { var v int64 if err := rows.Scan(&v); err != nil { return fmt.Errorf("scan: %w", err) } applied[v] = true } if err := rows.Err(); err != nil { return err } // 6. Apply pending, each in own transaction. for _, name := range files { v := parseVersion(name) if applied[v] { continue } sql, err := fs.ReadFile(migrationsFS, "migrations/"+name) if err != nil { return fmt.Errorf("read %s: %w", name, err) } tx, err := p.Begin(ctx) if err != nil { return err } if _, err := tx.Exec(ctx, string(sql)); err != nil { tx.Rollback(ctx) return fmt.Errorf("exec %s: %w", name, err) } if _, err := tx.Exec(ctx, "INSERT INTO schema_migrations (version, name) VALUES ($1,$2)", v, name); err != nil { tx.Rollback(ctx) return err } if err := tx.Commit(ctx); err != nil { return err } log.Printf("migration applied: %s", name) } return nil } func parseVersion(name string) int64 { parts := strings.SplitN(name, "_", 2) var v int64 fmt.Sscanf(parts[0], "%d", &v) return v } ``` - [ ] **Step 3: Delete old schema.sql** ```bash git rm backend/internal/db/schema.sql ``` - [ ] **Step 4: Verify build + tests** ```bash cd backend && go build ./... DATABASE_URL=postgres://booklib:booklib@localhost:5432/booklib?sslmode=disable \ go test -p 1 -count=1 ./... ``` - [ ] **Step 5: Verify baseline on existing DB** On a DB with old schema (no `schema_migrations`): 1. Start app → verify `SELECT * FROM schema_migrations` has one row (version=1). 2. Verify existing data untouched (`SELECT count(*) FROM books` same as before). 3. Verify no DDL re-executed (no errors in log). - [ ] **Step 6: Commit** ```bash git add backend/internal/db/migrations/0001_baseline.sql backend/internal/db/db.go git rm backend/internal/db/schema.sql git commit -m "feat(db): ordered migration system with advisory lock and baseline detection" ``` --- ### Task 2: CI Workflow **Files:** - Create: `.github/workflows/ci.yml` - [ ] **Step 1: Create workflow** ```yaml name: CI on: push: branches: [master, 'fix/**', 'feat/**'] pull_request: branches: [master] jobs: backend: runs-on: ubuntu-latest services: postgres: image: postgres:16 env: {POSTGRES_USER: booklib, POSTGRES_PASSWORD: booklib, POSTGRES_DB: booklib} ports: ['5432:5432'] options: --health-cmd pg_isready --health-interval 5s --health-timeout 3s --health-retries 10 redis: image: redis:7 ports: ['6379:6379'] options: --health-cmd "redis-cli ping" --health-interval 5s --health-timeout 3s --health-retries 10 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: {go-version-file: backend/go.mod} - run: | cd backend && OUT=$(gofmt -l .) && [ -z "$OUT" ] || { echo "gofmt:"; echo "$OUT"; exit 1; } - run: cd backend && go vet ./... - run: cd backend && go test -p 1 -count=1 ./... env: DATABASE_URL: postgres://booklib:booklib@localhost:5432/booklib?sslmode=disable REDIS_URL: redis://localhost:6379/0 frontend: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: {node-version: 20} - run: cd frontend && npm ci && npm run check ``` - [ ] **Step 2: Validate (optional)** ```bash go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/ci.yml ``` - [ ] **Step 3: Commit** ```bash git add .github/workflows/ci.yml git commit -m "ci: add GitHub Actions workflow (Gitea Actions compatible)" ``` --- ### Task 3: Fix B14 — log.Fatalf bypasses shutdown **Files:** - Modify: `backend/cmd/webui/main.go:43-55` - [ ] **Step 1: Replace serve goroutine + wait logic** Replace the block from `srv :=` through `<-ctx.Done()` in `main.go`: ```go serveErr := make(chan error, 1) srv := &http.Server{Addr: cfg.Addr, Handler: api.NewRouter(cfg, st, rdb, sc), ReadHeaderTimeout: 10 * time.Second} go func() { log.Printf("listening on %s", cfg.Addr) if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { serveErr <- err } close(serveErr) }() select { case <-ctx.Done(): case err := <-serveErr: if err != nil { log.Printf("serve: %v", err) } } stop() shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() if err := srv.Shutdown(shutdownCtx); err != nil { log.Printf("shutdown: %v", err) } ``` - [ ] **Step 2: Build + smoke** ```bash cd backend && go build ./cmd/webui && scripts/smoke.sh ``` - [ ] **Step 3: Commit** ```bash git add backend/cmd/webui/main.go git commit -m "fix(main): channel-based serve error instead of log.Fatalf in goroutine (B14)" ``` --- ### Task 4: Fix B15 — Config validation **Files:** - Modify: `backend/internal/config/config.go` - Modify: `backend/internal/config/config_test.go` - [ ] **Step 1: Add failing tests to config_test.go** ```go func TestLoadDatabaseURLRequired(t *testing.T) { t.Setenv("JWT_SECRET", "x") t.Setenv("DATABASE_URL", "") if _, err := Load(); err == nil { t.Fatal("empty DATABASE_URL must fail") } } func TestLoadDatabaseURLMalformed(t *testing.T) { t.Setenv("JWT_SECRET", "x") t.Setenv("DATABASE_URL", "not a url") if _, err := Load(); err == nil { t.Fatal("malformed DATABASE_URL must fail") } } ``` - [ ] **Step 2: Run to verify failure** ```bash cd backend && go test -run "TestLoadDatabase" -v ./internal/config/ ``` Expected: `TestLoadDatabaseURLRequired` FAIL (empty URL currently accepted). - [ ] **Step 3: Add validation in config.Load** Before the `return &Config{...}` in `Load()`, add: ```go dbURL := env("DATABASE_URL", "") if dbURL == "" { return nil, fmt.Errorf("DATABASE_URL is required") } if _, perr := pgxpool.ParseConfig(dbURL); perr != nil { return nil, fmt.Errorf("DATABASE_URL: %w", perr) } if env("REDIS_URL", "") == "" { log.Printf("redis disabled: rate-limit/scan-lock/page-cache off") } ``` Add imports: `"log"`, `"github.com/jackc/pgx/v5/pgxpool"`. - [ ] **Step 4: Run tests to verify pass** ```bash cd backend && go test -v ./internal/config/ ``` - [ ] **Step 5: Commit** ```bash git add backend/internal/config/ git commit -m "fix(config): validate DATABASE_URL, log redis disabled (B15)" ``` --- ### Task 5: Fix B17 — smoke.sh outdated root_path **Files:** - Modify: `scripts/smoke.sh` - [ ] **Step 1: Remove `root_path` from library creation JSON** Find the `curl` POST to `/libraries` (around line 32) and change: ```bash -d '{"name":"smoke","root_path":"/data/books/smoke-books"}' ``` to: ```bash -d '{"name":"smoke"}' ``` - [ ] **Step 2: Verify** ```bash scripts/smoke.sh ``` - [ ] **Step 3: Commit** ```bash git add scripts/smoke.sh git commit -m "fix(smoke): drop ignored root_path field, align with API contract (B17)" ``` --- ### Task 6: Batch A docs **Files:** - Modify: `docs/CHANGELOG.md` - Modify: `docs/README.md`, `docs/README_zh.md` - [ ] **Step 1: Prepend CHANGELOG entries** ``` ## [Unreleased] ### Changed - Add ordered migration system with schema_migrations tracking and pg advisory lock. Existing databases auto-baselined. - 新增有序迁移系统,通过 schema_migrations 表和 pg advisory lock 实现安全多副本 schema 演进,已有数据库自动基线化。 ### Fixed - Serve goroutine log.Fatalf replaced with channel-based shutdown (B14). - 服务 goroutine 的 log.Fatalf 改为 channel 通知,确保优雅关停(B14)。 - DATABASE_URL validated at startup; empty REDIS_URL logs clear message (B15). - 启动时校验 DATABASE_URL;空 REDIS_URL 记录明确日志(B15)。 - smoke.sh aligned with current API contract (B17). - smoke.sh 对齐当前 API 契约(B17)。 ### Added - CI workflow (.github/workflows/ci.yml) for GitHub/Gitea Actions. - CI 工作流,兼容 GitHub Actions 和 Gitea Actions。 ``` - [ ] **Step 2: Update README dual versions** Add "Schema Migration" section (how to add `NNNN_description.sql`, never modify applied files, no down migrations, baseline auto-upgrade). Add "CI" section (workflow location, Gitea/GitHub setup, local gate command). - [ ] **Step 3: Run local gate** ```bash cd backend && go vet ./... && gofmt -l . && \ DATABASE_URL=postgres://booklib:booklib@localhost:5432/booklib?sslmode=disable \ REDIS_URL=redis://localhost:6379/0 \ go test -p 1 -count=1 ./... ``` Confirm 0 skip. Then run `scripts/smoke.sh`. - [ ] **Step 4: Commit** ```bash git add docs/ git commit -m "docs: changelog + README for batch A (migrations, CI, B14/B15/B17)" ``` --- ## Part B — Bug Fixes B1–B13 Each bug: **write failing test → fix → verify → continue**. Tests use real PG/Redis (integration) until Part C converts applicable ones to fake-based unit tests. ### Task 7: B1 — Rate limiter permanent lockout **Files:** - Modify: `backend/internal/redispkg/redis.go` (IncrWindow) - Modify: `backend/internal/redispkg/redis_test.go` - [ ] **Step 1: Add test** ```go func TestIncrWindowSetsTTL(t *testing.T) { url := os.Getenv("REDIS_URL") if url == "" { t.Skip("REDIS_URL not set") } r := New(url) ctx := context.Background() key := "test:ttl:" + t.Name() r.c.Del(ctx, key) n := r.IncrWindow(ctx, key, 5*time.Second) if n != 1 { t.Fatalf("first = %d", n) } ttl, err := r.c.TTL(ctx, key).Result() if err != nil { t.Fatal(err) } if ttl <= 0 { t.Fatalf("TTL must be positive, got %v", ttl) } } ``` - [ ] **Step 2: Fix IncrWindow with Lua script** ```go var incrWindowScript = redis.NewScript(` local n = redis.call('INCR', KEYS[1]) if n == 1 then redis.call('EXPIRE', KEYS[1], ARGV[1]) end return n`) func (r *R) IncrWindow(ctx context.Context, key string, ttl time.Duration) int { if r.c == nil { return 1 } n, err := incrWindowScript.Run(ctx, r.c, []string{key}, int(ttl.Seconds())).Int() if err != nil { return 1 } return n } ``` - [ ] **Step 3: Run + commit** ```bash REDIS_URL=redis://localhost:6379/0 go test -v ./internal/redispkg/ git add backend/internal/redispkg/ && git commit -m "fix(redis): atomic INCR+EXPIRE via Lua (B1)" ``` --- ### Task 8: B2+B3 — Lock rand error + unlock context **Files:** - Modify: `backend/internal/redispkg/redis.go` (Lock method) - [ ] **Step 1: Fix token generation (B2)** In `Lock`, replace `rand.Read(b)` / `tok := hex.EncodeToString(b)` with: ```go b := make([]byte, 8) if _, err := rand.Read(b); err != nil { log.Printf("rand.Read: %v (degrading to no-lock)", err) return noop, true } tok := hex.EncodeToString(b) ``` - [ ] **Step 2: Fix unlock context (B3)** Replace the returned unlock closure's Eval call: ```go return func() { if err := r.c.Eval(context.WithoutCancel(ctx), "if redis.call('get',KEYS[1])==ARGV[1] then return redis.call('del',KEYS[1]) else return 0 end", []string{key}, tok).Err(); err != nil { log.Printf("unlock %s: %v", key, err) } }, true ``` - [ ] **Step 3: Run + commit** ```bash go test -v ./internal/redispkg/ git add backend/internal/redispkg/ && git commit -m "fix(redis): handle rand error + unlock survives ctx cancel (B2, B3)" ``` --- ### Task 9: B4 — Truncated part reported as received **Files:** - Modify: `backend/cmd/webui/handlers/uploads.go` (UploadPart method) - [ ] **Step 1: Fix part write to use tmp+rename** In `UploadPart`, replace the file-writing block (from `p := filepath.Join(dir, "parts", ...)` through the error handling): ```go p := filepath.Join(dir, "parts", strconv.FormatInt(idx, 10)) tmp := p + ".tmp" f, e := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) if e != nil { err(c, http.StatusInternalServerError, "internal", "create part") return } n, e := io.Copy(f, c.Request.Body) f.Close() if e != nil || n != hi-lo { os.Remove(tmp) var mbe *http.MaxBytesError code, msg := "too_large", "part size mismatch" if errors.As(e, &mbe) { msg = "part exceeds declared size" } err(c, http.StatusRequestEntityTooLarge, code, msg) return } if e := os.Rename(tmp, p); e != nil { os.Remove(tmp) err(c, http.StatusInternalServerError, "internal", "rename part") return } c.JSON(http.StatusAccepted, gin.H{"accepted": true}) ``` - [ ] **Step 2: Run + commit** ```bash go test -v ./cmd/webui/handlers/ -run Upload git add backend/cmd/webui/handlers/uploads.go && git commit -m "fix(upload): write part to .tmp then rename (B4)" ``` --- ### Task 10: B5 — DeleteUser last-admin TOCTOU **Files:** - Modify: `backend/internal/store/store.go` - Modify: `backend/cmd/webui/handlers/users.go` - [ ] **Step 1: Add test to store_test.go** ```go func TestDeleteUserLastAdmin(t *testing.T) { s := setup(t) ctx := context.Background() id, _ := s.CreateUser(ctx, "onlyadmin", "h", "admin") err := s.DeleteUser(ctx, id) if !errors.Is(err, ErrLastAdmin) { t.Fatalf("want ErrLastAdmin, got %v", err) } if _, err := s.GetUserByID(ctx, id); err != nil { t.Fatal("admin should still exist") } } func TestDeleteUserNonLastAdmin(t *testing.T) { s := setup(t) ctx := context.Background() id1, _ := s.CreateUser(ctx, "a1", "h", "admin") s.CreateUser(ctx, "a2", "h", "admin") if err := s.DeleteUser(ctx, id1); err != nil { t.Fatalf("non-last admin delete: %v", err) } } ``` - [ ] **Step 2: Run to verify failure** Expected: `ErrLastAdmin` undefined. - [ ] **Step 3: Add ErrLastAdmin + transactional DeleteUser to store.go** Add sentinel: ```go var ErrLastAdmin = errors.New("cannot delete the last admin") ``` Replace `DeleteUser`: ```go func (s *Store) DeleteUser(ctx context.Context, id int64) error { tx, err := s.P.Begin(ctx) if err != nil { return err } defer tx.Rollback(ctx) var role string if err := tx.QueryRow(ctx, "SELECT role FROM users WHERE id=$1 FOR UPDATE", id).Scan(&role); err != nil { return err } if role == "admin" { var n int if err := tx.QueryRow(ctx, "SELECT count(*) FROM users WHERE role='admin'").Scan(&n); err != nil { return err } if n <= 1 { return ErrLastAdmin } } if _, err := tx.Exec(ctx, "DELETE FROM users WHERE id=$1", id); err != nil { return err } return tx.Commit(ctx) } ``` Remove the old exported `CountAdmins` method. - [ ] **Step 4: Update users.go handler** Replace `DeleteUser` handler: ```go func (h *H) DeleteUser(c *gin.Context) { id, e := strconv.ParseInt(c.Param("id"), 10, 64) if e != nil { err(c, 400, "bad_request", "bad id"); return } if id == uid(c) { err(c, 400, "bad_request", "cannot delete yourself"); return } if e := h.st.DeleteUser(c, id); e != nil { if errors.Is(e, pgx.ErrNoRows) { err(c, 404, "not_found", "no such user"); return } if errors.Is(e, store.ErrLastAdmin) { err(c, 400, "bad_request", "cannot delete the last admin"); return } dbErr(c, e); return } c.Status(204) } ``` Remove the old `CountAdmins` + `GetUserByID` + role check block. Add `"booklib/internal/store"` import. - [ ] **Step 5: Run + commit** ```bash DATABASE_URL=... go test -v ./internal/store/ ./cmd/webui/handlers/ git add backend/internal/store/store.go backend/cmd/webui/handlers/users.go git commit -m "fix(store): transactional last-admin check in DeleteUser (B5)" ``` --- ### Task 11: B6+B7 — Error semantics fixes **Files:** - Modify: `backend/cmd/webui/handlers/libraries.go` (Upload method, B6) - Modify: `backend/cmd/webui/handlers/auth.go` (Me method, B7) - [ ] **Step 1: Fix B6 — Upload io.Copy error mapping** In `libraries.go` `Upload` method, replace the `io.Copy` error block: ```go if _, e := io.Copy(out, src); e != nil { out.Close() os.Remove(tmp) var mbe *http.MaxBytesError if errors.As(e, &mbe) { err(c, http.StatusRequestEntityTooLarge, "too_large", "file exceeds upload limit") return } err(c, http.StatusInternalServerError, "internal", "upload failed") return } ``` - [ ] **Step 2: Fix B7 — Me handler error mapping** In `auth.go` `Me` method, replace: ```go func (h *H) Me(c *gin.Context) { u, qerr := h.st.GetUserByID(c, uid(c)) if qerr != nil { if errors.Is(qerr, pgx.ErrNoRows) { err(c, http.StatusUnauthorized, "unauthorized", "no such user") return } dbErr(c, qerr) return } c.JSON(http.StatusOK, gin.H{"id": u.ID, "username": u.Username, "role": u.Role}) } ``` - [ ] **Step 3: Run + commit** ```bash go test -v ./cmd/webui/handlers/ git add backend/cmd/webui/handlers/libraries.go backend/cmd/webui/handlers/auth.go git commit -m "fix(handlers): correct error mapping for Upload (B6) and Me (B7)" ``` --- ### Task 12: B8 — Reserved library names **Files:** - Create: `backend/internal/media/reserved.go` - Create: `backend/internal/media/reserved_test.go` - Modify: `backend/cmd/webui/handlers/libraries.go` - [ ] **Step 1: Create reserved.go** ```go package media import "strings" var reservedNames = map[string]bool{ "cache": true, ".uploads": true, ".trash": true, } func IsReservedName(name string) bool { return reservedNames[strings.ToLower(name)] } ``` - [ ] **Step 2: Create reserved_test.go** ```go package media import "testing" func TestIsReservedName(t *testing.T) { for _, n := range []string{"cache", "Cache", "CACHE", ".uploads", ".Uploads", ".trash"} { if !IsReservedName(n) { t.Errorf("%q should be reserved", n) } } for _, n := range []string{"comics", "books", "my-library"} { if IsReservedName(n) { t.Errorf("%q should not be reserved", n) } } } ``` - [ ] **Step 3: Add check in CreateLibrary handler** In `libraries.go` `CreateLibrary`, after `SafeName` check, add: ```go if media.IsReservedName(safe) { err(c, http.StatusBadRequest, "bad_request", "reserved_name") return } ``` Import `"booklib/internal/media"`. - [ ] **Step 4: Run + commit** ```bash go test -v ./internal/media/ ./cmd/webui/handlers/ -run "Reserved\|CreateLibrary" git add backend/internal/media/ backend/cmd/webui/handlers/libraries.go git commit -m "fix(library): reject reserved names with 400 reserved_name (B8)" ``` --- ### Task 13: B9-① — Scan lock renewal **Files:** - Modify: `backend/internal/redispkg/redis.go` - Modify: `backend/internal/redispkg/redis_test.go` - Modify: `backend/internal/scanner/scanner.go` - [ ] **Step 1: Add test** ```go func TestScanLockRenewal(t *testing.T) { url := os.Getenv("REDIS_URL") if url == "" { t.Skip("REDIS_URL not set") } r := New(url) ctx, key := context.Background(), "test:scanlock:"+t.Name() unlock, ok := r.ScanLock(ctx, key, 2*time.Second) if !ok { t.Fatal("should acquire") } time.Sleep(3 * time.Second) // would expire without renewal _, ok2 := r.ScanLock(ctx, key, 2*time.Second) if ok2 { t.Fatal("should still be held") } unlock() } ``` - [ ] **Step 2: Add ScanLock to redis.go** ```go func (r *R) ScanLock(ctx context.Context, key string, ttl time.Duration) (func(), bool) { noop := func() {} if r.c == nil { return noop, true } b := make([]byte, 8) if _, err := rand.Read(b); err != nil { log.Printf("rand: %v", err); return noop, true } tok := hex.EncodeToString(b) ok, err := r.c.SetNX(ctx, key, tok, ttl).Result() if err != nil { log.Printf("scanlock %s: %v (proceeding)", key, err) return noop, true } if !ok { return noop, false } done := make(chan struct{}) go func() { tk := time.NewTicker(ttl / 2); defer tk.Stop() for { select { case <-done: return case <-tk.C: r.c.Eval(context.Background(), `if redis.call('get',KEYS[1])==ARGV[1] then return redis.call('expire',KEYS[1],ARGV[2]) end`, []string{key}, tok, int(ttl.Seconds())) } } }() return func() { close(done) r.c.Eval(context.WithoutCancel(ctx), "if redis.call('get',KEYS[1])==ARGV[1] then return redis.call('del',KEYS[1]) else return 0 end", []string{key}, tok) }, true } ``` - [ ] **Step 3: Update scanner to use ScanLock** In `scanner.go` `ScanLibrary`, change: ```go unlock, ok := s.rdb.Lock(ctx, fmt.Sprintf("scan:%d", lib.ID), 5*time.Minute) ``` to: ```go unlock, ok := s.rdb.ScanLock(ctx, fmt.Sprintf("scan:%d", lib.ID), 5*time.Minute) ``` - [ ] **Step 4: Run + commit** ```bash REDIS_URL=redis://localhost:6379/0 go test -v -timeout 10s ./internal/redispkg/ go test -v ./internal/scanner/ git add backend/internal/redispkg/ backend/internal/scanner/scanner.go git commit -m "fix(redis): ScanLock with auto-renewal for scanner (B9-①)" ``` --- ### Task 14: B10+B11 — Silent error logging fixes **Files:** - Modify: `backend/internal/scanner/scanner.go` (cover, add, update) - Modify: `backend/cmd/webui/handlers/content.go` (ServeCover) - [ ] **Step 1: Fix scanner add/update SetBookState (B10)** In both `add` and `update`, change: ```go s.st.SetBookState(ctx, id, "error", idxErr.Error()) ``` to: ```go if e := s.st.SetBookState(ctx, id, "error", idxErr.Error()); e != nil { log.Printf("scan: SetBookState %s: %v", rel, e) } ``` - [ ] **Step 2: Fix scanner cover write (B11)** Replace the cover write section: ```go tmp := filepath.Join(dir, "cover"+ext+".tmp") dst := filepath.Join(dir, "cover"+ext) if e := os.WriteFile(tmp, img, 0o644); e != nil { log.Printf("scan: cover tmp %s: %v", rel, e) os.Remove(tmp) return } if e := os.Rename(tmp, dst); e != nil { log.Printf("scan: cover rename %s: %v", rel, e) os.Remove(tmp) return } ``` - [ ] **Step 3: Fix ServeCover self-heal (B11)** In `content.go`, replace the self-heal write block: ```go tmp := fmt.Sprintf("%s.tmp-%d", dst, time.Now().UnixNano()) if e := os.WriteFile(tmp, img, 0o644); e != nil { log.Printf("serve: cover write: %v", e) os.Remove(tmp) } else if e := os.Rename(tmp, dst); e != nil { log.Printf("serve: cover rename: %v", e) os.Remove(tmp) } else { http.ServeFile(c.Writer, c.Request, dst) } ``` Add `"log"` import if missing. - [ ] **Step 4: Run + commit** ```bash go test -v ./internal/scanner/ ./cmd/webui/handlers/ git add backend/internal/scanner/scanner.go backend/cmd/webui/handlers/content.go git commit -m "fix: check cover write errors + log SetBookState failures (B10, B11)" ``` --- ### Task 15: B12 — uniquePath race **Files:** - Modify: `backend/cmd/webui/handlers/libraries.go` (Upload method) - [ ] **Step 1: Add retry loop in Upload handler** In `Upload`, replace the single `uniquePath` + `OpenFile` block with a retry: ```go var dst string var out *os.File for attempt := 0; attempt < 5; attempt++ { var e error dst, e = h.uniquePath(root, name) if e != nil { err(c, http.StatusForbidden, "forbidden", e.Error()); return } tmp := dst + ".upload-" + strconv.FormatInt(time.Now().UnixNano(), 36) out, e = os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) if e == nil { break } if !os.IsExist(e) { err(c, 500, "internal", "create tmp"); return } } if out == nil { err(c, http.StatusConflict, "conflict", "concurrent upload conflict") return } ``` - [ ] **Step 2: Run + commit** ```bash go test -v ./cmd/webui/handlers/ -run Upload git add backend/cmd/webui/handlers/libraries.go git commit -m "fix(upload): retry on O_EXCL collision (B12)" ``` --- ### Task 16: B13 — RowsAffected before err **Files:** - Modify: `backend/internal/store/store.go` (UpdateBookmarkNote, DeleteBookmark) - [ ] **Step 1: Fix both methods** ```go func (s *Store) UpdateBookmarkNote(ctx context.Context, userID, id int64, note string) (bool, error) { res, err := s.P.Exec(ctx, `UPDATE bookmarks SET note=$3 WHERE id=$1 AND user_id=$2`, id, userID, note) if err != nil { return false, err } return res.RowsAffected() > 0, nil } func (s *Store) DeleteBookmark(ctx context.Context, userID, id int64) (bool, error) { res, err := s.P.Exec(ctx, `DELETE FROM bookmarks WHERE id=$1 AND user_id=$2`, id, userID) if err != nil { return false, err } return res.RowsAffected() > 0, nil } ``` - [ ] **Step 2: Run + commit** ```bash go test -v ./internal/store/ git add backend/internal/store/store.go git commit -m "fix(store): check err before RowsAffected (B13)" ``` --- ### Task 17: B9-② — Scanner per-library single-flight **Files:** - Modify: `backend/internal/scanner/scanner.go` - [ ] **Step 1: Add sync.Map + scanOnce wrapper** Add field to Scanner: ```go flights sync.Map // map[int64]*sync.WaitGroup ``` Add method: ```go func (s *Scanner) scanOnce(ctx context.Context, lib store.Library) { wg := &sync.WaitGroup{} wg.Add(1) if existing, loaded := s.flights.LoadOrStore(lib.ID, wg); loaded { existing.(*sync.WaitGroup).Wait() return } defer func() { s.flights.Delete(lib.ID); wg.Done() }() s.ScanLibrary(ctx, lib) } ``` Update `Run` to call `s.scanOnce(ctx, l)` and `ScanLibraryByID` to call `s.scanOnce(ctx, lib)`. - [ ] **Step 2: Run + commit** ```bash go test -v ./internal/scanner/ git add backend/internal/scanner/scanner.go git commit -m "fix(scanner): per-library single-flight (B9-②)" ``` --- ### Task 18: Batch B docs + local gate **Files:** - Modify: `docs/CHANGELOG.md` - [ ] **Step 1: Prepend CHANGELOG entries for B1–B13** Add entries for each fix in the `[Unreleased]` section. Format per global constraints (English+Chinese, blank between entries). See spec §S2 for the full list. - [ ] **Step 2: Run full local gate** ```bash cd backend && go vet ./... && gofmt -l . DATABASE_URL=postgres://booklib:booklib@localhost:5432/booklib?sslmode=disable \ REDIS_URL=redis://localhost:6379/0 \ go test -p 1 -count=1 ./... scripts/smoke.sh ``` Confirm 0 skip. - [ ] **Step 3: Commit** ```bash git add docs/CHANGELOG.md git commit -m "docs: changelog for batch B (B1-B13)" ``` --- ## Part C — Structure Restructure + Interfaces ### Task 19: Define ports package **Files:** - Create: `backend/internal/ports/ports.go` - Create: `backend/internal/ports/errors.go` - [ ] **Step 1: Create ports.go** Define consumer-side interfaces. Value types use `store.*` (ports imports store for types only — one-way dependency): ```go package ports import ( "context" "time" "booklib/internal/store" ) type UserStore interface { CountUsers(ctx context.Context) (int, error) CreateUser(ctx context.Context, username, hash, role string) (int64, error) GetUserByName(ctx context.Context, username string) (store.User, error) GetUserByID(ctx context.Context, id int64) (store.User, error) ListUsers(ctx context.Context) ([]store.User, error) DeleteUser(ctx context.Context, id int64) error } type LibraryStore interface { CreateLibrary(ctx context.Context, name, root string) (int64, error) ListLibraries(ctx context.Context) ([]store.Library, error) GetLibrary(ctx context.Context, id int64) (store.Library, error) } type BookStore interface { InsertBook(ctx context.Context, libID int64, path, title, format string, size, modTS int64, pageCount int) (int64, error) GetBook(ctx context.Context, id int64) (store.Book, error) ListBookMeta(ctx context.Context, libID int64) (map[string]store.BookMeta, error) UpdateBookFile(ctx context.Context, id, size, modTS int64, pageCount int) error DeleteBookByPath(ctx context.Context, libID int64, path string) error DeleteBook(ctx context.Context, id int64) error SetBookState(ctx context.Context, id int64, state, msg string) error ListBooks(ctx context.Context, libID int64, q, prefix string, userID int64) ([]store.BookView, error) BookHashes(ctx context.Context) (map[int64][2]int64, error) } type ProgressStore interface { UpsertProgress(ctx context.Context, userID, libID int64, bookPath string, locator []byte, percent float64) error GetProgress(ctx context.Context, userID, libID int64, bookPath string) (store.Progress, error) ListProgress(ctx context.Context, userID int64) ([]store.Progress, error) } type BookmarkStore interface { InsertBookmark(ctx context.Context, userID, libID int64, bookPath string, locator []byte, percent float64, note string) (int64, error) ListBookmarks(ctx context.Context, userID, libID int64, bookPath string) ([]store.Bookmark, error) UpdateBookmarkNote(ctx context.Context, userID, id int64, note string) (bool, error) DeleteBookmark(ctx context.Context, userID, id int64) (bool, error) } type PageCache interface { Get(ctx context.Context, key string) (string, bool) Set(ctx context.Context, key, val string, ttl time.Duration) } type RateLimiter interface { IncrWindow(ctx context.Context, key string, ttl time.Duration) int } type ScanLocker interface { ScanLock(ctx context.Context, key string, ttl time.Duration) (func(), bool) } type Scanner interface { ScanLibraryByID(ctx context.Context, id int64) } ``` Note: `UploadSessions` and `Media` interfaces will be defined after `internal/upload` and `internal/media` are created (Tasks 22-23). - [ ] **Step 2: Create errors.go** ```go package ports import "booklib/internal/store" // Re-export sentinel errors so handlers use errors.Is via ports. var ErrLastAdmin = store.ErrLastAdmin // IsUniqueViolation consolidates the 3 copies of pg 23505 check. func IsUniqueViolation(err error) bool { return store.IsUniqueViolation(err) } ``` - [ ] **Step 3: Add IsUniqueViolation + ErrUniqueViolation to store** In `store.go`, add: ```go var ErrUniqueViolation = errors.New("unique violation") func IsUniqueViolation(err error) bool { var pgErr *pgconn.PgError return errors.As(err, &pgErr) && pgErr.Code == "23505" } ``` Update `ports/errors.go` to also re-export: ```go var ErrUniqueViolation = store.ErrUniqueViolation ``` - [ ] **Step 4: Verify build** ```bash cd backend && go build ./... ``` - [ ] **Step 5: Commit** ```bash git add backend/internal/ports/ backend/internal/store/store.go git commit -m "feat(ports): define consumer-side interfaces and shared errors" ``` --- ### Task 20: Split store package **Files:** - Create: `backend/internal/store/users.go` - Create: `backend/internal/store/libraries.go` - Create: `backend/internal/store/books.go` - Create: `backend/internal/store/progress.go` - Create: `backend/internal/store/bookmarks.go` - Modify: `backend/internal/store/store.go` (keep types + ctor + shared helpers only) - Modify: `backend/internal/store/store_test.go` (extract per-file tests) - [ ] **Step 1: Extract methods by aggregate** Move methods from `store.go` into per-aggregate files: - `users.go`: `CountUsers`, `CreateUser`, `GetUserByName`, `GetUserByID`, `ListUsers`, `DeleteUser`, `scanUser`, `CountAdmins` (make unexported: `countAdmins`) - `libraries.go`: `CreateLibrary`, `ListLibraries`, `GetLibrary` - `books.go`: `InsertBook`, `GetBook`, `ListBookMeta`, `UpdateBookFile`, `DeleteBookByPath`, `DeleteBook`, `SetBookState`, `ListBooks`, `BookHashes`. Delete `ListBookIDs` (dead code per spec). - `progress.go`: `UpsertProgress`, `GetProgress`, `ListProgress` - `bookmarks.go`: `InsertBookmark`, `ListBookmarks`, `UpdateBookmarkNote`, `DeleteBookmark` Each file: `package store`, receives methods on `*Store`. - [ ] **Step 2: Slim down store.go** Keep: type definitions (`Store`, `User`, `Library`, `Book`, `BookMeta`, `BookView`, `Progress`, `Bookmark`), `New()`, `IsUniqueViolation()`, `ErrLastAdmin`, column constants (`userCols`, `bookCols`). Make `P` field unexported (`p *pgxpool.Pool`). Update all method files to use `s.p` instead of `s.P`. Update `store_test.go` setup if it uses `s.P`. - [ ] **Step 3: Verify build + tests** ```bash cd backend && go build ./... && DATABASE_URL=... go test ./internal/store/ ``` - [ ] **Step 4: Commit** ```bash git add backend/internal/store/ git commit -m "refactor(store): split into per-aggregate files, unexport pool" ``` --- ### Task 21: Add bookfile utilities **Files:** - Modify: `backend/internal/bookfile/bookfile.go` (add Contains) - Create: `backend/internal/bookfile/reader.go` (OpenReaderAt) - [ ] **Step 1: Add Contains function** ```go // Contains reports whether child is inside parent (EvalSymlinks semantics). func Contains(parent, child string) bool { p, err := filepath.EvalSymlinks(parent) if err != nil { p = filepath.Clean(parent) } c, err := filepath.EvalSymlinks(child) if err != nil { c = filepath.Clean(child) } return c == p || strings.HasPrefix(c, p+string(os.PathSeparator)) } ``` - [ ] **Step 2: Add OpenReaderAt** Consolidate the 3 places that open a book file + stat + get ReaderAt: ```go // OpenReaderAt opens a book file and returns a ReaderAt + size. func OpenReaderAt(root, rel string) (*os.File, int64, error) { abs := filepath.Join(root, filepath.FromSlash(rel)) f, err := os.Open(abs) if err != nil { return nil, 0, err } st, err := f.Stat() if err != nil { f.Close(); return nil, 0, err } return f, st.Size(), nil } ``` - [ ] **Step 3: Add tests** ```go func TestContains(t *testing.T) { dir := t.TempDir() sub := filepath.Join(dir, "sub") os.MkdirAll(sub, 0o755) if !Contains(dir, sub) { t.Fatal("sub should be inside dir") } if Contains(sub, dir) { t.Fatal("dir should not be inside sub") } } ``` - [ ] **Step 4: Run + commit** ```bash go test -v ./internal/bookfile/ git add backend/internal/bookfile/ git commit -m "feat(bookfile): add Contains and OpenReaderAt utilities" ``` --- ### Task 22: Create internal/media package **Files:** - Modify: `backend/internal/media/reserved.go` (already exists from Task 12) - Create: `backend/internal/media/media.go` (Media struct + interface methods) - Create: `backend/internal/media/write.go` (WriteAtomic) - Create: `backend/internal/media/chapters.go` (ChaptersOf) - Create: `backend/internal/media/ports.go` (Media interface definition) - [ ] **Step 1: Create WriteAtomic** ```go package media import ( "log" "os" "path/filepath" "fmt" "time" ) // WriteAtomic writes data to dst via tmp+rename. Cleans tmp only on failure. func WriteAtomic(dir, name string, data []byte) error { if err := os.MkdirAll(dir, 0o755); err != nil { return err } tmp := filepath.Join(dir, fmt.Sprintf("%s.tmp-%d", name, time.Now().UnixNano())) if err := os.WriteFile(tmp, data, 0o644); err != nil { os.Remove(tmp) return err } dst := filepath.Join(dir, name) if err := os.Rename(tmp, dst); err != nil { os.Remove(tmp) return err } return nil } ``` - [ ] **Step 2: Move ChaptersOf from content.go** Copy `chaptersOf` and `cbzChapter` from `handlers/content.go` into `media/chapters.go` (exported as `ChaptersOf`). The handler will call `media.ChaptersOf(idx)`. - [ ] **Step 3: Move cache layout functions** Move `DirKey`, `CoverDir`, `PagesDir`, `SweepStale`, `Hash` from `bookfile/cache.go` and `bookfile/hash.go` into `media/`. Or keep them in `bookfile` and have `media` wrap them — whichever avoids a large move. Per spec, media is the "single source of truth" for cache layout. Move them to `media/cache.go`: ```go package media import "booklib/internal/bookfile" // Re-export cache layout from bookfile for backward compat. // Future: move implementations here. var DirKey = bookfile.DirKey var CoverDir = bookfile.CoverDir var PagesDir = bookfile.PagesDir var SweepStale = bookfile.SweepStale var Hash = bookfile.Hash ``` Actually simpler: just have Media methods call bookfile functions directly. No re-export needed. Media is the consumer; bookfile stays as the low-level utility package. - [ ] **Step 4: Create Media struct** ```go package media import ( "context" "booklib/internal/bookfile" "booklib/internal/config" "booklib/internal/redispkg" ) type M struct { cfg *config.Config rdb *redispkg.R } func New(cfg *config.Config, rdb *redispkg.R) *M { return &M{cfg: cfg, rdb: rdb} } ``` - [ ] **Step 5: Implement Media methods** Implement on `*M`: - `EnsureCover(ctx, bookID, format, size, modTS, root, rel)` — extracted from scanner.cover - `EnsurePage(ctx, bookID, size, modTS, root, rel, n)` — extracted from content.go Page handler - `ChaptersOf(idx)` — from step 2 - `PageIndex(ctx, bookID, size, modTS, root, rel)` — extracted from content.go pageIndex - `CacheBuster(size, modTS)` — returns `bookfile.Hash(size, modTS)` - [ ] **Step 6: Create Media interface in ports.go** Add to `ports/ports.go` (or `ports/media.go`): ```go type Media interface { EnsureCover(ctx context.Context, bookID int64, format string, size, modTS int64, root, rel string) error EnsurePage(ctx context.Context, bookID int64, size, modTS int64, root, rel string, n int) (string, error) ChaptersOf(idx []string) []Chapter PageIndex(ctx context.Context, bookID int64, size, modTS int64, root, rel string) ([]string, error) CacheBuster(size, modTS int64) string } ``` - [ ] **Step 7: Build + commit** ```bash go build ./... git add backend/internal/media/ backend/internal/ports/ git commit -m "feat(media): cache layout single source of truth + WriteAtomic + chapters" ``` --- ### Task 23: Create internal/upload package + B16 **Files:** - Create: `backend/internal/upload/upload.go` - Create: `backend/internal/upload/upload_test.go` - Create: `backend/internal/upload/ports.go` (UploadSessions interface) - Modify: `backend/internal/scanner/scanner.go` (add Sweep to ticker, B16) - [ ] **Step 1: Move upload domain logic** Extract from `handlers/uploads.go` into `internal/upload/upload.go`: - `uploadIDFor`, `uploadDir`, `chunkRange`, `numParts`, `uploadMeta`, `validUploadID` - `Init(libID, name, size, chunkSize)` → returns uploadID - `Status(uploadID)` → returns received part indices - `PutPart(uploadID, index, body)` → writes part with tmp+rename (B4 fix already applied) - `Complete(uploadID)` → assembles + renames to library root (uses `uniquePath`) - `Sweep()` → deletes expired sessions (B16: moved from request path to scanner ticker) - `UniquePath(root, name)` → shared between single-file and chunked upload The `upload` package takes `booksDir string` and `uploadMaxMB int64` in its constructor. - [ ] **Step 2: Define UploadSessions interface in ports** Add to `ports/ports.go`: ```go type UploadSessions interface { Init(ctx context.Context, libID int64, name string, size, chunkSize int64) (string, error) Status(ctx context.Context, uploadID string) ([]int64, error) PutPart(ctx context.Context, uploadID string, index int64, body io.Reader, maxSize int64) error Complete(ctx context.Context, uploadID string, root string) (string, error) Sweep(ctx context.Context) error UniquePath(root, name string) (string, error) } ``` Add to `ports/errors.go`: ```go var ( ErrTooLarge = errors.New("file too large") ErrIncomplete = errors.New("upload incomplete") ErrSizeMismatch = errors.New("size mismatch") ErrNotFound = errors.New("not found") ) ``` - [ ] **Step 3: Wire Sweep into scanner ticker (B16)** In `scanner.go` `Run` method, add sweep call each tick: ```go case <-t.C: h.sweepUploads() // B16: moved from request path libs, err := s.st.ListLibraries(ctx) ... ``` The scanner needs access to the upload package. Add `up *upload.U` field to Scanner. Or have main.go pass the sweep function. Simplest: scanner gets an `upload.Sweeper` interface: ```go type Sweeper interface { Sweep(ctx context.Context) error } ``` - [ ] **Step 4: Build + commit** ```bash go build ./... git add backend/internal/upload/ backend/internal/scanner/ backend/internal/ports/ git commit -m "feat(upload): extract upload subsystem + move sweep to scanner ticker (B16)" ``` --- ### Task 24: Refactor scanner **Files:** - Modify: `backend/internal/scanner/scanner.go` - [ ] **Step 1: Merge add/update into ingest** Consolidate the duplicated logic in `add` and `update`: ```go func (s *Scanner) ingest(ctx context.Context, libID, bookID int64, root, rel string, ds diskStat, isNew bool) { format := bookfile.FormatFromExt(filepath.Base(rel)) pageCount := 0 var idxErr error if format == "cbz" { idx, err := s.zipIndex(root, rel) pageCount = len(idx) idxErr = err if idxErr == nil && pageCount == 0 { idxErr = errors.New("no images in archive") } } if isNew { id, err := s.st.InsertBook(ctx, libID, rel, titleOf(rel), format, ds.size, ds.modTS, pageCount) if err != nil { log.Printf("scan: insert %s: %v", rel, err); return } bookID = id } else { if err := s.st.UpdateBookFile(ctx, bookID, ds.size, ds.modTS, pageCount); err != nil { log.Printf("scan: update %s: %v", rel, err); return } } if idxErr != nil { if e := s.st.SetBookState(ctx, bookID, "error", idxErr.Error()); e != nil { log.Printf("scan: SetBookState %s: %v", rel, e) } return } s.cover(ctx, bookID, root, rel, format, ds) } ``` - [ ] **Step 2: Use bookfile.OpenReaderAt** Replace `readCover` and `zipIndex` to use `bookfile.OpenReaderAt`: ```go func (s *Scanner) zipIndex(root, rel string) ([]string, error) { f, size, err := bookfile.OpenReaderAt(root, rel) if err != nil { return nil, err } defer f.Close() return bookfile.PageIndex(f, size) } ``` - [ ] **Step 3: Use bookfile.Contains** Replace the `inside` function call: ```go if !bookfile.Contains(s.cfg.BooksDir, root) { ... } ``` Remove the local `inside` function. - [ ] **Step 4: Use media.WriteAtomic in cover** Replace manual tmp+rename in `cover` with `media.WriteAtomic(dir, "cover"+ext, img)`. - [ ] **Step 5: Run + commit** ```bash go test -v ./internal/scanner/ git add backend/internal/scanner/ git commit -m "refactor(scanner): merge add/update into ingest, use shared utilities" ``` --- ### Task 25: Slim down handlers **Files:** - Modify: All `backend/cmd/webui/handlers/*.go` - Modify: `backend/cmd/webui/handlers/handlers.go` - [ ] **Step 1: Change H struct to use port interfaces** ```go type H struct { cfg *config.Config users ports.UserStore libs ports.LibraryStore books ports.BookStore progress ports.ProgressStore bookmarks ports.BookmarkStore cache ports.PageCache rl ports.RateLimiter scanner ports.Scanner media media.M // or ports.Media once interface finalized upload upload.U // or ports.UploadSessions } ``` Update `New()` to accept all interfaces. - [ ] **Step 2: Update all handler methods** Replace `h.st.Xxx(...)` with the appropriate port field. Deduplicate: - `getLibrary` and `getLibRow` → single `getLib(c, id)` helper - `strconv.ParseInt(c.Param("id"), ...)` → single `idParam(c)` helper - `isUnique` → `ports.IsUniqueViolation` - Path contains checks → `bookfile.Contains` - [ ] **Step 3: Update router.go to pass new H** Update `api/router.go` and `handlers.New()` signature to accept all dependencies. - [ ] **Step 4: Build + commit** ```bash go build ./... git add backend/cmd/webui/ git commit -m "refactor(handlers): use port interfaces, deduplicate helpers" ``` --- ### Task 26: Wire main.go assembly **Files:** - Modify: `backend/cmd/webui/main.go` - [ ] **Step 1: Update main.go to assemble all dependencies** ```go func main() { cfg, err := config.Load() if err != nil { log.Fatalf("config: %v", err) } // ... ctx, db, migrate as before ... st := store.New(p) seed.Admin(ctx, st, cfg.AdminUser, cfg.AdminPassword) rdb := redispkg.New(cfg.RedisURL) med := media.New(cfg, rdb) up := upload.New(cfg.BooksDir, cfg.UploadMaxMB) sc := scanner.New(st, cfg, rdb, med, up) // Wire router with port interfaces. // *store.Store satisfies UserStore, LibraryStore, etc. // *redispkg.R satisfies PageCache, RateLimiter, ScanLocker. r := api.NewRouter(cfg, st, st, st, st, st, rdb, rdb, sc, med, up) // ... serve as Task 3 pattern ... } ``` - [ ] **Step 2: Build + smoke** ```bash cd backend && go build ./cmd/webui && scripts/smoke.sh ``` - [ ] **Step 3: Commit** ```bash git add backend/cmd/webui/main.go git commit -m "refactor(main): assemble dependencies via port interfaces" ``` --- ### Task 27: Create portsfake + contract tests + handler unit tests **Files:** - Create: `backend/internal/ports/portsfake/fake.go` - Create: `backend/cmd/webui/handlers/users_unit_test.go` (and similar per handler) - Modify: `backend/cmd/webui/api/router_test.go` (contract test) - [ ] **Step 1: Create hand-written fakes** `portsfake/fake.go` implements all port interfaces with in-memory maps: ```go package portsfake type Users struct { m map[int64]store.User next int64 } func NewUsers() *Users { return &Users{m: map[int64]store.User{}, next: 1} } func (u *Users) CreateUser(_ context.Context, username, hash, role string) (int64, error) { for _, v := range u.m { if v.Username == username { return 0, ports.ErrUniqueViolation } } id := u.next; u.next++ u.m[id] = store.User{ID: id, Username: username, PasswordHash: hash, Role: role} return id, nil } // ... implement all interface methods ``` Similarly for Libraries, Books, Progress, Bookmarks fakes. And a simple in-memory PageCache, RateLimiter, ScanLocker. - [ ] **Step 2: Add router contract test** In `router_test.go`, pin the expected route table: ```go func TestRouterContract(t *testing.T) { r := NewRouter(/* fakes */) routes := r.Routes() expected := map[string]string{ "GET /api/healthz": "", "POST /api/auth/login": "", "GET /api/auth/me": "auth", "GET /api/users": "auth,admin", // ... all routes with middleware markers } // Assert exact match — any route change requires updating this test. } ``` - [ ] **Step 3: Add handler unit tests** Example `users_unit_test.go`: ```go func TestDeleteUserLastAdminReturns400(t *testing.T) { h := newTestH() // uses portsfake // Seed one admin h.users.CreateUser(ctx, "admin", "h", "admin") // Try to delete → should get 400 w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Params = gin.Params{{Key: "id", Value: "1"}} c.Set("uid", int64(999)) // different user h.DeleteUser(c) if w.Code != 400 { t.Fatalf("want 400, got %d", w.Code) } } ``` - [ ] **Step 4: Run all tests (unit + integration)** ```bash go test -p 1 -count=1 ./... ``` - [ ] **Step 5: Commit** ```bash git add backend/internal/ports/portsfake/ backend/cmd/webui/handlers/*_unit_test.go backend/cmd/webui/api/router_test.go git commit -m "test: add portsfake, router contract tests, handler unit tests" ``` --- ### Task 28: Consolidate isUnique + dedup helpers **Files:** - Modify: `backend/cmd/webui/handlers/users.go` (remove local `isUnique`) - Modify: `backend/internal/seed/seed.go` (use `ports.IsUniqueViolation`) - Modify: All handlers that duplicate error-check patterns - [ ] **Step 1: Replace all `isUnique` calls** Search for `isUnique(` and `pgconn.PgError` with code `23505` across the codebase. Replace with `ports.IsUniqueViolation(err)` or `store.IsUniqueViolation(err)`. Remove the local `isUnique` function from `handlers/users.go`. Update `seed.go` to use `store.IsUniqueViolation`. - [ ] **Step 2: Consolidate path validation** Replace the 3 different path-contains checks with `bookfile.Contains`: - `libraries.go` `libRoot` → `bookfile.Contains(h.cfg.BooksDir, root)` - `books.go` `hasPrefixDir` → `bookfile.Contains` - `scanner.go` `inside` → `bookfile.Contains` (already done in Task 24) Remove local helper functions. - [ ] **Step 3: Run + commit** ```bash go build ./... && go test -p 1 -count=1 ./... git add -A && git commit -m "refactor: consolidate isUnique and path validation helpers" ``` --- ### Task 29: Final docs + local gate **Files:** - Modify: `docs/CHANGELOG.md` - Modify: `docs/README.md`, `docs/README_zh.md` - [ ] **Step 1: Add CHANGELOG for Batch C** ``` ### Changed - Business logic restructured: handlers now consume small port interfaces, domain logic in internal/media and internal/upload. - 业务逻辑重构:handlers 消费小口径 port 接口,域逻辑下沉至 internal/media 和 internal/upload。 - Upload sweep moved from request path to scanner ticker cycle (B16). - 上传会话清理从请求路径移至扫描器定时周期(B16)。 ### Added - Router contract test pins expected route table; unauthorized changes fail CI. - 路由契约测试锁定预期路由表,未授权变更将导致 CI 失败。 - Hand-written in-memory fakes (portsfake) enable handler unit tests without PG/Redis. - 手写内存 fake(portsfake)实现无 PG/Redis 的 handler 单测。 ``` - [ ] **Step 2: Update READMEs** Document the new package structure (ports, media, upload). Explain the interface/fake testing approach. - [ ] **Step 3: Full local gate** ```bash cd backend && go vet ./... && gofmt -l . DATABASE_URL=postgres://booklib:booklib@localhost:5432/booklib?sslmode=disable \ REDIS_URL=redis://localhost:6379/0 \ go test -p 1 -count=1 -v ./... scripts/smoke.sh ``` Confirm: - 0 skip in test output - All 47+ original integration tests pass - New fake-based unit tests pass - Router contract test passes - smoke.sh passes - [ ] **Step 4: Commit** ```bash git add docs/ git commit -m "docs: changelog + README for batch C (restructure, contract tests)" ``` --- ## Verification Checklist After all 29 tasks: 1. `go vet ./...` — clean 2. `gofmt -l .` — empty output 3. `go test -p 1 -count=1 ./...` — all pass, 0 skip 4. `scripts/smoke.sh` — ALL SMOKE TESTS PASSED 5. `scripts/smoke-web.sh` — passes (if compose available) 6. Baseline test: old DB → new code → `schema_migrations` has `0001_baseline.sql` row 7. No `CountAdmins` exported (removed) 8. No `ListBookIDs` exported (removed, dead code) 9. No local `isUnique` functions outside `store.IsUniqueViolation` 10. No `inside` function in scanner (replaced by `bookfile.Contains`) 11. Router contract test pins all routes 12. All 17 bugs (B1–B17) have corresponding tests or smoke verification