1886 lines
53 KiB
Markdown
1886 lines
53 KiB
Markdown
# 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
|