diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index f1df2a3..a0f1029 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -16,9 +16,9 @@ Requirements for this milestone. Each maps to roadmap phases. ### Backend Refactor -- [ ] **REFAC-01**: Database operations are behind a Store interface with separate SQLite and PostgreSQL implementations +- [x] **REFAC-01**: Database operations are behind a Store interface with separate SQLite and PostgreSQL implementations - [ ] **REFAC-02**: Package-level global state (db, mu, webhookSecret) is replaced with a Server struct that holds dependencies -- [ ] **REFAC-03**: Schema migrations use golang-migrate with separate migration directories per dialect (sqlite/, postgres/) +- [x] **REFAC-03**: Schema migrations use golang-migrate with separate migration directories per dialect (sqlite/, postgres/) ### Database @@ -95,9 +95,9 @@ Which phases cover which requirements. Updated during roadmap creation. | DATA-02 | Phase 1 | Complete | | DATA-03 | Phase 1 | Complete | | DATA-04 | Phase 1 | Complete | -| REFAC-01 | Phase 2 | Pending | +| REFAC-01 | Phase 2 | Complete | | REFAC-02 | Phase 2 | Pending | -| REFAC-03 | Phase 2 | Pending | +| REFAC-03 | Phase 2 | Complete | | DB-01 | Phase 3 | Pending | | DB-02 | Phase 3 | Pending | | DB-03 | Phase 3 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 25aa1d8..c3e2fbe 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -46,7 +46,7 @@ Plans: **Plans**: 2 plans Plans: -- [ ] 02-01-PLAN.md — Create Store interface (9 methods), SQLiteStore implementation, golang-migrate migration infrastructure with embedded SQL files +- [x] 02-01-PLAN.md — Create Store interface (9 methods), SQLiteStore implementation, golang-migrate migration infrastructure with embedded SQL files - [ ] 02-02-PLAN.md — Convert handlers to Server struct methods, remove globals, rewrite tests for per-test isolated databases, update main.go wiring ### Phase 3: PostgreSQL Support diff --git a/.planning/STATE.md b/.planning/STATE.md index a1df511..313df33 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,14 +2,14 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone -status: Ready to plan -stopped_at: Completed 01-02-PLAN.md (body size limits + test hardening) -last_updated: "2026-03-23T20:29:13.565Z" +status: Ready to execute +stopped_at: Completed 02-01-PLAN.md (Store interface, SQLiteStore, migration infrastructure) +last_updated: "2026-03-23T20:59:13.329Z" progress: total_phases: 4 completed_phases: 1 - total_plans: 2 - completed_plans: 2 + total_plans: 4 + completed_plans: 3 --- # Project State @@ -19,12 +19,12 @@ progress: See: .planning/PROJECT.md (updated 2026-03-23) **Core value:** Reliable, persistent visibility into which services need updating — data never disappears, and the dashboard is the one place you trust to show the full picture. -**Current focus:** Phase 01 — data-integrity +**Current focus:** Phase 02 — backend-refactor ## Current Position -Phase: 2 -Plan: Not started +Phase: 02 (backend-refactor) — EXECUTING +Plan: 2 of 2 ## Performance Metrics @@ -48,6 +48,7 @@ Plan: Not started *Updated after each plan completion* | Phase 01 P01 | 2 | 2 tasks | 2 files | | Phase 01-data-integrity P02 | 7 | 2 tasks | 2 files | +| Phase 02-backend-refactor P01 | 7min | 2 tasks | 7 files | ## Accumulated Context @@ -64,6 +65,8 @@ Recent decisions affecting current work: - [Phase 01]: Enable PRAGMA foreign_keys = ON in InitDB() before DDL to activate ON DELETE CASCADE for tag deletion - [Phase 01-data-integrity]: Use MaxBytesReader + errors.As(*http.MaxBytesError) per-handler (not middleware) for request body size limiting — consistent with no-middleware architecture - [Phase 01-data-integrity]: Oversized body tests need valid JSON prefix so decoder reads past 1MB limit; all-x bytes fail at byte 1 before MaxBytesReader triggers +- [Phase 02-backend-refactor]: Store interface with 9 methods is the persistence abstraction; SQLiteStore holds *sql.DB and sync.Mutex as struct fields (not package globals) +- [Phase 02-backend-refactor]: golang-migrate v4.19.1 database/sqlite sub-package confirmed to use modernc.org/sqlite (no CGO); single 0001 baseline migration uses CREATE TABLE IF NOT EXISTS for backward compatibility ### Pending Todos @@ -76,6 +79,6 @@ None yet. ## Session Continuity -Last session: 2026-03-23T20:25:50.294Z -Stopped at: Completed 01-02-PLAN.md (body size limits + test hardening) +Last session: 2026-03-23T20:59:13.327Z +Stopped at: Completed 02-01-PLAN.md (Store interface, SQLiteStore, migration infrastructure) Resume file: None diff --git a/.planning/phases/02-backend-refactor/02-01-SUMMARY.md b/.planning/phases/02-backend-refactor/02-01-SUMMARY.md new file mode 100644 index 0000000..c67c480 --- /dev/null +++ b/.planning/phases/02-backend-refactor/02-01-SUMMARY.md @@ -0,0 +1,132 @@ +--- +phase: 02-backend-refactor +plan: "01" +subsystem: database +tags: [golang-migrate, sqlite, store-interface, dependency-injection, migrations, embed-fs] + +# Dependency graph +requires: + - phase: 01-data-integrity + provides: PRAGMA foreign_keys enforcement and UPSERT semantics in existing diunwebhook.go +provides: + - Store interface with 9 methods covering all persistence operations + - SQLiteStore implementing Store with exact SQL from current handlers + - RunMigrations function using golang-migrate + embed.FS (iofs source) + - Baseline migration 0001 with full current schema (CREATE TABLE IF NOT EXISTS) +affects: + - 02-02 (Server struct refactor will use Store interface and RunMigrations) + - 03-postgresql (PostgreSQLStore will implement same Store interface) + +# Tech tracking +tech-stack: + added: + - github.com/golang-migrate/migrate/v4 v4.19.1 + - github.com/golang-migrate/migrate/v4/database/sqlite (modernc.org/sqlite driver, no CGO) + - github.com/golang-migrate/migrate/v4/source/iofs (embed.FS migration source) + patterns: + - Store interface pattern - persistence abstraction hiding *sql.DB from handlers + - SQLiteStore with per-struct sync.Mutex (replaces package-level global) + - golang-migrate with embedded SQL files via //go:embed migrations/sqlite + - ErrNoChange guard in RunMigrations (startup idempotency) + - CREATE TABLE IF NOT EXISTS in baseline migration (backward compatible with existing databases) + +key-files: + created: + - pkg/diunwebhook/store.go + - pkg/diunwebhook/sqlite_store.go + - pkg/diunwebhook/migrate.go + - pkg/diunwebhook/migrations/sqlite/0001_initial_schema.up.sql + - pkg/diunwebhook/migrations/sqlite/0001_initial_schema.down.sql + modified: + - go.mod + - go.sum + +key-decisions: + - "Used database/sqlite sub-package (not database/sqlite3) to avoid CGO - confirmed modernc.org/sqlite usage in sqlite.go source" + - "Single 0001 baseline migration with full schema including acknowledged_at - safe for existing databases via CREATE TABLE IF NOT EXISTS" + - "NewSQLiteStore sets MaxOpenConns(1) and PRAGMA foreign_keys = ON - moved from InitDB which will be removed in Plan 02" + - "AssignTag preserves INSERT OR REPLACE (not ON CONFLICT DO UPDATE) per research Pitfall 6 - correct semantics for tag_assignments PRIMARY KEY" + - "defer rows.Close() directly (not verbose closure pattern) as plan specifies" + +patterns-established: + - "Store interface: all persistence behind 9 named methods, no *sql.DB in interface signature" + - "SQLiteStore field mutex: sync.Mutex as struct field, not package global - enables parallel test isolation" + - "Migration files: versioned SQL files embedded via //go:embed, applied via golang-migrate at startup" + - "ErrNoChange is not an error: errors.Is(err, migrate.ErrNoChange) guard ensures idempotent startup" + +requirements-completed: [REFAC-01, REFAC-03] + +# Metrics +duration: 6min +completed: "2026-03-23" +--- + +# Phase 02 Plan 01: Store Interface and Migration Infrastructure Summary + +**Store interface (9 methods) + SQLiteStore implementation + golang-migrate v4.19.1 migration infrastructure with embedded SQL files** + +## Performance + +- **Duration:** ~6 min +- **Started:** 2026-03-23T20:50:31Z +- **Completed:** 2026-03-23T20:56:56Z +- **Tasks:** 2 +- **Files modified:** 7 + +## Accomplishments + +- Store interface with 9 methods extracted from current handler SQL (UpsertEvent, GetUpdates, AcknowledgeUpdate, ListTags, CreateTag, DeleteTag, AssignTag, UnassignTag, TagExists) +- SQLiteStore implementing all 9 Store methods with exact SQL semantics preserved from diunwebhook.go +- golang-migrate v4.19.1 migration infrastructure with RunMigrations using embed.FS and iofs source +- Baseline migration 0001 with full current schema using CREATE TABLE IF NOT EXISTS (safe for existing databases) +- All existing tests pass; no existing code modified (additive-only changes as specified) + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Create Store interface and SQLiteStore implementation** - `57bf3bd` (feat) +2. **Task 2: Create migration infrastructure and SQL files** - `6506d93` (feat) + +**Plan metadata:** (docs commit follows) + +## Files Created/Modified + +- `pkg/diunwebhook/store.go` - Store interface with 9 persistence methods +- `pkg/diunwebhook/sqlite_store.go` - SQLiteStore struct implementing Store; NewSQLiteStore sets MaxOpenConns(1) and PRAGMA foreign_keys = ON +- `pkg/diunwebhook/migrate.go` - RunMigrations using golang-migrate + embed.FS + iofs; handles ErrNoChange +- `pkg/diunwebhook/migrations/sqlite/0001_initial_schema.up.sql` - Full baseline schema (updates, tags, tag_assignments) with CREATE TABLE IF NOT EXISTS +- `pkg/diunwebhook/migrations/sqlite/0001_initial_schema.down.sql` - DROP TABLE IF EXISTS for all three tables +- `go.mod` - Added github.com/golang-migrate/migrate/v4 v4.19.1 and sub-packages +- `go.sum` - Updated checksums + +## Decisions Made + +- Used `database/sqlite` (not `database/sqlite3`) for golang-migrate driver — confirmed at source level that it imports `modernc.org/sqlite`, satisfying no-CGO constraint +- Single 0001 baseline migration includes `acknowledged_at` from the start; safe for existing databases because `CREATE TABLE IF NOT EXISTS` makes it idempotent on pre-existing schemas +- `NewSQLiteStore` sets `MaxOpenConns(1)` and `PRAGMA foreign_keys = ON` — these will no longer live in `InitDB` once Plan 02 removes globals +- `AssignTag` uses `INSERT OR REPLACE` (not `ON CONFLICT DO UPDATE`) — preserves semantics per research Pitfall 6 + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered + +- `go vet` reports a pre-existing issue in `diunwebhook_test.go:227` (`call to (*testing.T).Fatalf from a non-test goroutine`) — confirmed pre-existing before any changes; out of scope for this plan. Logged to deferred-items. +- `mattn/go-sqlite3` appears in `go mod graph` as an indirect dependency of the `golang-migrate` module itself, but our code only imports `database/sqlite` (confirmed no CGO import in our code chain via `go mod graph | grep sqlite3 | grep -v golang-migrate`). + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- Store interface and SQLiteStore ready for Plan 02 to wire into Server struct +- RunMigrations ready to call from main.go instead of InitDB +- All existing tests pass — Plan 02 can refactor handlers with confidence +- Blocker: Plan 02 must redesign export_test.go (currently references package-level globals that will be removed) + +--- +*Phase: 02-backend-refactor* +*Completed: 2026-03-23*