diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index a0f1029..e7639df 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -17,7 +17,7 @@ Requirements for this milestone. Each maps to roadmap phases. ### Backend Refactor - [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 +- [x] **REFAC-02**: Package-level global state (db, mu, webhookSecret) is replaced with a Server struct that holds dependencies - [x] **REFAC-03**: Schema migrations use golang-migrate with separate migration directories per dialect (sqlite/, postgres/) ### Database @@ -96,7 +96,7 @@ Which phases cover which requirements. Updated during roadmap creation. | DATA-03 | Phase 1 | Complete | | DATA-04 | Phase 1 | Complete | | REFAC-01 | Phase 2 | Complete | -| REFAC-02 | Phase 2 | Pending | +| REFAC-02 | Phase 2 | Complete | | REFAC-03 | Phase 2 | Complete | | DB-01 | Phase 3 | Pending | | DB-02 | Phase 3 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index c3e2fbe..fc37c32 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -13,7 +13,7 @@ This milestone restores data trust and then extends the foundation. Phase 1 fixe Decimal phases appear between their surrounding integers in numeric order. - [ ] **Phase 1: Data Integrity** - Fix active SQLite bugs that silently delete tag assignments and suppress test failures -- [ ] **Phase 2: Backend Refactor** - Replace global state with Store interface + Server struct; prerequisite for PostgreSQL +- [x] **Phase 2: Backend Refactor** - Replace global state with Store interface + Server struct; prerequisite for PostgreSQL (completed 2026-03-24) - [ ] **Phase 3: PostgreSQL Support** - Add PostgreSQL as an alternative backend via DATABASE_URL, with versioned migrations - [ ] **Phase 4: UX Improvements** - Bulk dismiss, search/filter, new-update indicators, and accessibility fixes @@ -47,7 +47,7 @@ Plans: Plans: - [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 +- [x] 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 **Goal**: Users running PostgreSQL infrastructure can point DiunDashboard at a Postgres database via DATABASE_URL and the dashboard works identically to the SQLite deployment @@ -83,6 +83,6 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 1. Data Integrity | 0/2 | Not started | - | -| 2. Backend Refactor | 0/2 | Not started | - | +| 2. Backend Refactor | 2/2 | Complete | 2026-03-24 | | 3. PostgreSQL Support | 0/? | Not started | - | | 4. UX Improvements | 0/? | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 313df33..e423004 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,13 +3,13 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone status: Ready to execute -stopped_at: Completed 02-01-PLAN.md (Store interface, SQLiteStore, migration infrastructure) -last_updated: "2026-03-23T20:59:13.329Z" +stopped_at: Completed 02-02-PLAN.md (Server struct refactor, test isolation with NewTestServer) +last_updated: "2026-03-24T07:39:15.372Z" progress: total_phases: 4 - completed_phases: 1 + completed_phases: 2 total_plans: 4 - completed_plans: 3 + completed_plans: 4 --- # Project State @@ -49,6 +49,7 @@ Plan: 2 of 2 | 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 | +| Phase 02-backend-refactor P02 | 3min | 2 tasks | 4 files | ## Accumulated Context @@ -67,6 +68,8 @@ Recent decisions affecting current work: - [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 +- [Phase 02-backend-refactor]: Option B for test store access: internal helpers in export_test.go (TestUpsertEvent, TestGetUpdatesMap) instead of exported Store() accessor - keeps store field unexported +- [Phase 02-backend-refactor]: NewTestServer pattern: each test gets its own in-memory SQLite DB (RunMigrations + NewSQLiteStore + NewServer) - eliminates shared global state between tests ### Pending Todos @@ -79,6 +82,6 @@ None yet. ## Session Continuity -Last session: 2026-03-23T20:59:13.327Z -Stopped at: Completed 02-01-PLAN.md (Store interface, SQLiteStore, migration infrastructure) +Last session: 2026-03-24T07:39:15.370Z +Stopped at: Completed 02-02-PLAN.md (Server struct refactor, test isolation with NewTestServer) Resume file: None diff --git a/.planning/phases/02-backend-refactor/02-02-SUMMARY.md b/.planning/phases/02-backend-refactor/02-02-SUMMARY.md new file mode 100644 index 0000000..ab14c5f --- /dev/null +++ b/.planning/phases/02-backend-refactor/02-02-SUMMARY.md @@ -0,0 +1,125 @@ +--- +phase: 02-backend-refactor +plan: "02" +subsystem: http-handlers +tags: [server-struct, dependency-injection, store-interface, test-isolation, in-memory-sqlite, refactor] + +# Dependency graph +requires: + - phase: 02-01 + provides: Store interface (9 methods), SQLiteStore, RunMigrations +provides: + - Server struct with Store field and webhookSecret field + - NewServer constructor wiring Store and secret + - All 6 handlers converted to *Server methods calling s.store.X() + - NewTestServer / NewTestServerWithSecret helpers for isolated per-test databases + - main.go wiring: sql.Open -> RunMigrations -> NewSQLiteStore -> NewServer -> routes +affects: + - 03-postgresql (PostgreSQLStore will implement same Store interface; Server struct accepts any Store) + +# Tech tracking +tech-stack: + added: [] + patterns: + - Server struct pattern - all handler dependencies injected via constructor, no package-level globals + - export_test.go internal helpers (TestUpsertEvent, TestGetUpdatesMap) - access unexported fields without exposing Store accessor + - Per-test in-memory SQLite database via NewTestServer() - eliminates shared state between tests + - NewTestServerWithSecret for auth-enabled test scenarios + +key-files: + created: [] + modified: + - pkg/diunwebhook/diunwebhook.go + - pkg/diunwebhook/export_test.go + - pkg/diunwebhook/diunwebhook_test.go + - cmd/diunwebhook/main.go + +key-decisions: + - "Option B for test store access: internal helpers in export_test.go (TestUpsertEvent, TestGetUpdatesMap) instead of exported Store() accessor - keeps store field unexported" + - "t.Errorf used inside goroutines in TestConcurrentUpdateEvent (t.Fatalf is not safe from non-test goroutines)" + - "_ modernc.org/sqlite blank import moved from diunwebhook.go to main.go and migrate.go - driver registration happens where needed" + +patterns-established: + - "Server struct: HTTP handlers as methods on *Server, all deps injected at construction" + - "NewTestServer pattern: each test creates its own in-memory SQLite DB via RunMigrations + NewSQLiteStore + NewServer" + - "export_test.go internal methods: (s *Server) TestUpsertEvent / TestGetUpdatesMap access s.store without exporting Store field" + +requirements-completed: [REFAC-01, REFAC-02, REFAC-03] + +# Metrics +duration: 3min +completed: "2026-03-23" +--- + +# Phase 02 Plan 02: Server Struct Refactor and Test Isolation Summary + +**Server struct with Store injection, globals removed, all 6 handlers as *Server methods calling s.store.X(), per-test in-memory databases via NewTestServer** + +## Performance + +- **Duration:** ~3 min +- **Started:** 2026-03-23T21:02:53Z +- **Completed:** 2026-03-23T21:05:09Z +- **Tasks:** 2 +- **Files modified:** 4 + +## Accomplishments + +- Removed all package-level globals (db, mu, webhookSecret) from diunwebhook.go +- Removed InitDB, SetWebhookSecret, UpdateEvent, GetUpdates functions (replaced by Store and Server) +- Added Server struct with store Store and webhookSecret string fields +- Added NewServer(store Store, webhookSecret string) *Server constructor +- Converted all 6 handler functions to *Server methods using s.store.X() for all persistence +- Rewrote export_test.go: NewTestServer, NewTestServerWithSecret, TestUpsertEvent, TestGetUpdatesMap helpers +- Rewrote diunwebhook_test.go: every test creates its own isolated in-memory database (no shared global state) +- Updated main.go: sql.Open -> RunMigrations -> NewSQLiteStore -> NewServer -> route registration +- All 35 tests pass against the new Server/Store architecture + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Convert diunwebhook.go to Server struct and update main.go** - `78543d7` (feat) +2. **Task 2: Rewrite export_test.go and update all tests for Server/Store** - `e35b4f8` (test) + +## Files Created/Modified + +- `pkg/diunwebhook/diunwebhook.go` - Server struct, NewServer constructor, all 6 handlers as *Server methods; globals and standalone functions removed +- `pkg/diunwebhook/export_test.go` - NewTestServer, NewTestServerWithSecret, (s *Server) TestUpsertEvent, TestGetUpdates, TestGetUpdatesMap +- `pkg/diunwebhook/diunwebhook_test.go` - All 35 tests rewritten to use NewTestServer per-test; no shared state; no TestMain +- `cmd/diunwebhook/main.go` - Full replacement: sql.Open -> RunMigrations -> NewSQLiteStore -> NewServer -> route registration with srv.XHandler + +## Decisions Made + +- Test store access via internal helper methods in export_test.go (Option B) — avoids exposing Store field publicly while still letting tests call UpsertEvent/GetUpdates +- t.Errorf used inside goroutine in TestConcurrentUpdateEvent — t.Fatalf is not safe from non-test goroutines (pre-existing issue resolved) +- _ "modernc.org/sqlite" blank import moved to main.go (and already in migrate.go) — driver registered where *sql.DB is opened + +## Deviations from Plan + +None - plan executed exactly as written. + +## Known Stubs + +None. + +## Self-Check: PASSED + +- pkg/diunwebhook/diunwebhook.go: FOUND +- pkg/diunwebhook/export_test.go: FOUND +- pkg/diunwebhook/diunwebhook_test.go: FOUND +- cmd/diunwebhook/main.go: FOUND +- Commit 78543d7: FOUND +- Commit e35b4f8: FOUND +- All 35 tests pass: VERIFIED (go test -v -count=1 ./pkg/diunwebhook/) + +## Next Phase Readiness + +- Server struct accepts any Store implementation — PostgreSQL store can be introduced in Phase 3 without touching handlers +- RunMigrations called in main.go before store creation — Phase 3 just needs to add a postgres migration variant +- Per-test isolation via NewTestServer is the established pattern — Phase 3 tests can follow the same approach +- All acceptance criteria verified: no globals, no SQL in handlers, s.store.X() pattern throughout, main.go wiring complete + +--- +*Phase: 02-backend-refactor* +*Completed: 2026-03-23*