docs(01-01): complete UPSERT + FK enforcement plan
- Create 01-01-SUMMARY.md documenting both bug fixes and test addition - Advance plan counter to 2/2 in STATE.md - Record decisions and metrics in STATE.md - Update ROADMAP.md plan progress (1/2 summaries) - Mark requirements DATA-01 and DATA-02 complete
This commit is contained in:
@@ -9,8 +9,8 @@ Requirements for this milestone. Each maps to roadmap phases.
|
|||||||
|
|
||||||
### Data Integrity
|
### Data Integrity
|
||||||
|
|
||||||
- [ ] **DATA-01**: Webhook events use proper UPSERT (ON CONFLICT DO UPDATE) instead of INSERT OR REPLACE, preserving tag assignments when an image receives a new event
|
- [x] **DATA-01**: Webhook events use proper UPSERT (ON CONFLICT DO UPDATE) instead of INSERT OR REPLACE, preserving tag assignments when an image receives a new event
|
||||||
- [ ] **DATA-02**: SQLite foreign key enforcement is enabled (PRAGMA foreign_keys = ON) so tag deletion properly cascades to tag assignments
|
- [x] **DATA-02**: SQLite foreign key enforcement is enabled (PRAGMA foreign_keys = ON) so tag deletion properly cascades to tag assignments
|
||||||
- [ ] **DATA-03**: Webhook and API endpoints enforce request body size limits (e.g., 1MB) to prevent OOM from oversized payloads
|
- [ ] **DATA-03**: Webhook and API endpoints enforce request body size limits (e.g., 1MB) to prevent OOM from oversized payloads
|
||||||
- [ ] **DATA-04**: Test error handling uses t.Fatal instead of silent returns, so test failures are never swallowed
|
- [ ] **DATA-04**: Test error handling uses t.Fatal instead of silent returns, so test failures are never swallowed
|
||||||
|
|
||||||
@@ -91,8 +91,8 @@ Which phases cover which requirements. Updated during roadmap creation.
|
|||||||
|
|
||||||
| Requirement | Phase | Status |
|
| Requirement | Phase | Status |
|
||||||
|-------------|-------|--------|
|
|-------------|-------|--------|
|
||||||
| DATA-01 | Phase 1 | Pending |
|
| DATA-01 | Phase 1 | Complete |
|
||||||
| DATA-02 | Phase 1 | Pending |
|
| DATA-02 | Phase 1 | Complete |
|
||||||
| DATA-03 | Phase 1 | Pending |
|
| DATA-03 | Phase 1 | Pending |
|
||||||
| DATA-04 | Phase 1 | Pending |
|
| DATA-04 | Phase 1 | Pending |
|
||||||
| REFAC-01 | Phase 2 | Pending |
|
| REFAC-01 | Phase 2 | Pending |
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ Decimal phases appear between their surrounding integers in numeric order.
|
|||||||
**Plans**: 2 plans
|
**Plans**: 2 plans
|
||||||
|
|
||||||
Plans:
|
Plans:
|
||||||
- [ ] 01-01-PLAN.md — Fix INSERT OR REPLACE → UPSERT in UpdateEvent(); enable PRAGMA foreign_keys = ON in InitDB(); add regression test
|
- [x] 01-01-PLAN.md — Fix INSERT OR REPLACE → UPSERT in UpdateEvent(); enable PRAGMA foreign_keys = ON in InitDB(); add regression test
|
||||||
- [ ] 01-02-PLAN.md — Add http.MaxBytesReader body limits to 3 handlers (413 on oversized); replace 6 silent test returns with t.Fatalf
|
- [ ] 01-02-PLAN.md — Add http.MaxBytesReader body limits to 3 handlers (413 on oversized); replace 6 silent test returns with t.Fatalf
|
||||||
|
|
||||||
### Phase 2: Backend Refactor
|
### Phase 2: Backend Refactor
|
||||||
|
|||||||
@@ -1,3 +1,17 @@
|
|||||||
|
---
|
||||||
|
gsd_state_version: 1.0
|
||||||
|
milestone: v1.0
|
||||||
|
milestone_name: milestone
|
||||||
|
status: Ready to execute
|
||||||
|
stopped_at: Completed 01-01-PLAN.md (UPSERT + FK enforcement)
|
||||||
|
last_updated: "2026-03-23T20:16:27.362Z"
|
||||||
|
progress:
|
||||||
|
total_phases: 4
|
||||||
|
completed_phases: 0
|
||||||
|
total_plans: 2
|
||||||
|
completed_plans: 1
|
||||||
|
---
|
||||||
|
|
||||||
# Project State
|
# Project State
|
||||||
|
|
||||||
## Project Reference
|
## Project Reference
|
||||||
@@ -5,20 +19,17 @@
|
|||||||
See: .planning/PROJECT.md (updated 2026-03-23)
|
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.
|
**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 1 — Data Integrity
|
**Current focus:** Phase 01 — data-integrity
|
||||||
|
|
||||||
## Current Position
|
## Current Position
|
||||||
|
|
||||||
Phase: 1 of 4 (Data Integrity)
|
Phase: 01 (data-integrity) — EXECUTING
|
||||||
Plan: 0 of ? in current phase
|
Plan: 2 of 2
|
||||||
Status: Ready to plan
|
|
||||||
Last activity: 2026-03-23 — Roadmap created; ready to begin Phase 1 planning
|
|
||||||
|
|
||||||
Progress: [░░░░░░░░░░] 0%
|
|
||||||
|
|
||||||
## Performance Metrics
|
## Performance Metrics
|
||||||
|
|
||||||
**Velocity:**
|
**Velocity:**
|
||||||
|
|
||||||
- Total plans completed: 0
|
- Total plans completed: 0
|
||||||
- Average duration: —
|
- Average duration: —
|
||||||
- Total execution time: —
|
- Total execution time: —
|
||||||
@@ -30,10 +41,12 @@ Progress: [░░░░░░░░░░] 0%
|
|||||||
| - | - | - | - |
|
| - | - | - | - |
|
||||||
|
|
||||||
**Recent Trend:**
|
**Recent Trend:**
|
||||||
|
|
||||||
- Last 5 plans: —
|
- Last 5 plans: —
|
||||||
- Trend: —
|
- Trend: —
|
||||||
|
|
||||||
*Updated after each plan completion*
|
*Updated after each plan completion*
|
||||||
|
| Phase 01 P01 | 2 | 2 tasks | 2 files |
|
||||||
|
|
||||||
## Accumulated Context
|
## Accumulated Context
|
||||||
|
|
||||||
@@ -46,6 +59,8 @@ Recent decisions affecting current work:
|
|||||||
- Backend refactor must be behavior-neutral — all existing tests must pass before PostgreSQL is introduced
|
- Backend refactor must be behavior-neutral — all existing tests must pass before PostgreSQL is introduced
|
||||||
- No ORM or query builder — raw SQL per store implementation; 8 operations across 3 tables is too small to justify a dependency
|
- No ORM or query builder — raw SQL per store implementation; 8 operations across 3 tables is too small to justify a dependency
|
||||||
- `DATABASE_URL` present activates PostgreSQL; absent falls back to SQLite with `DB_PATH` — no separate `DB_DRIVER` variable
|
- `DATABASE_URL` present activates PostgreSQL; absent falls back to SQLite with `DB_PATH` — no separate `DB_DRIVER` variable
|
||||||
|
- [Phase 01]: Use named-column UPSERT (ON CONFLICT DO UPDATE) to preserve tag_assignments child rows on re-insert
|
||||||
|
- [Phase 01]: Enable PRAGMA foreign_keys = ON in InitDB() before DDL to activate ON DELETE CASCADE for tag deletion
|
||||||
|
|
||||||
### Pending Todos
|
### Pending Todos
|
||||||
|
|
||||||
@@ -58,6 +73,6 @@ None yet.
|
|||||||
|
|
||||||
## Session Continuity
|
## Session Continuity
|
||||||
|
|
||||||
Last session: 2026-03-23
|
Last session: 2026-03-23T20:16:27.360Z
|
||||||
Stopped at: Roadmap created; STATE.md initialized
|
Stopped at: Completed 01-01-PLAN.md (UPSERT + FK enforcement)
|
||||||
Resume file: None
|
Resume file: None
|
||||||
|
|||||||
81
.planning/phases/01-data-integrity/01-01-SUMMARY.md
Normal file
81
.planning/phases/01-data-integrity/01-01-SUMMARY.md
Normal file
@@ -0,0 +1,81 @@
|
|||||||
|
---
|
||||||
|
phase: 01-data-integrity
|
||||||
|
plan: "01"
|
||||||
|
subsystem: backend/storage
|
||||||
|
tags: [sqlite, bug-fix, data-integrity, upsert, foreign-keys]
|
||||||
|
dependency_graph:
|
||||||
|
requires: []
|
||||||
|
provides: [DATA-01, DATA-02]
|
||||||
|
affects: [pkg/diunwebhook/diunwebhook.go]
|
||||||
|
tech_stack:
|
||||||
|
added: []
|
||||||
|
patterns: [SQLite UPSERT (ON CONFLICT DO UPDATE), PRAGMA foreign_keys = ON]
|
||||||
|
key_files:
|
||||||
|
created: []
|
||||||
|
modified:
|
||||||
|
- pkg/diunwebhook/diunwebhook.go
|
||||||
|
- pkg/diunwebhook/diunwebhook_test.go
|
||||||
|
decisions:
|
||||||
|
- "Use named-column INSERT with ON CONFLICT(image) DO UPDATE SET (UPSERT) instead of INSERT OR REPLACE to preserve tag_assignments child rows"
|
||||||
|
- "Enable PRAGMA foreign_keys = ON immediately after SetMaxOpenConns(1) so all connections (single-connection pool) enforce FK constraints"
|
||||||
|
metrics:
|
||||||
|
duration: "2 minutes"
|
||||||
|
completed_date: "2026-03-23"
|
||||||
|
tasks_completed: 2
|
||||||
|
files_modified: 2
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 01 Plan 01: Fix SQLite Data-Destruction Bugs (UPSERT + FK Enforcement) Summary
|
||||||
|
|
||||||
|
**One-liner:** SQLite UPSERT replaces INSERT OR REPLACE to preserve tag_assignments on re-insert, and PRAGMA foreign_keys = ON enables ON DELETE CASCADE for tag deletion.
|
||||||
|
|
||||||
|
## What Was Built
|
||||||
|
|
||||||
|
Fixed two silent data-destruction bugs in the SQLite persistence layer:
|
||||||
|
|
||||||
|
**Bug DATA-01 (INSERT OR REPLACE destroying tags):** SQLite's `INSERT OR REPLACE` is implemented as DELETE + INSERT. The DELETE fired `ON DELETE CASCADE` on `tag_assignments.image`, silently removing the tag assignment every time a new DIUN event arrived for an already-tagged image. Fixed by replacing the statement with a proper UPSERT (`INSERT INTO ... ON CONFLICT(image) DO UPDATE SET`) that only updates the non-key columns, leaving `tag_assignments` untouched.
|
||||||
|
|
||||||
|
**Bug DATA-02 (FK enforcement disabled):** SQLite disables foreign key enforcement by default. `PRAGMA foreign_keys = ON` was never executed, so `ON DELETE CASCADE` on `tag_assignments.tag_id` did not fire when a tag was deleted. Fixed by executing the pragma immediately after `db.SetMaxOpenConns(1)` in `InitDB()`, before any DDL statements.
|
||||||
|
|
||||||
|
## Tasks Completed
|
||||||
|
|
||||||
|
| Task | Name | Commit | Files |
|
||||||
|
|------|------|--------|-------|
|
||||||
|
| 1 | Replace INSERT OR REPLACE with UPSERT + add PRAGMA FK enforcement | 7edbaad | pkg/diunwebhook/diunwebhook.go |
|
||||||
|
| 2 | Add regression test TestUpdateEvent_PreservesTagOnUpsert | e2d388c | pkg/diunwebhook/diunwebhook_test.go |
|
||||||
|
|
||||||
|
## Decisions Made
|
||||||
|
|
||||||
|
1. **Named-column UPSERT over positional INSERT OR REPLACE:** The UPSERT explicitly lists 15 columns and their `excluded.*` counterparts in the DO UPDATE SET clause, making column mapping unambiguous and safe for future schema additions.
|
||||||
|
|
||||||
|
2. **acknowledged_at hardcoded NULL in UPSERT:** The UPSERT sets `acknowledged_at = NULL` unconditionally in both the INSERT and the ON CONFLICT update clause. This ensures a new event always resets the acknowledged state, matching the pre-existing behavior and test expectations.
|
||||||
|
|
||||||
|
3. **PRAGMA placement before DDL:** The FK pragma is placed before all CREATE TABLE statements to ensure the enforcement is active when foreign key relationships are first defined, not just at query time.
|
||||||
|
|
||||||
|
## Deviations from Plan
|
||||||
|
|
||||||
|
None — plan executed exactly as written.
|
||||||
|
|
||||||
|
## Verification Results
|
||||||
|
|
||||||
|
- `grep -c "INSERT OR REPLACE INTO updates" pkg/diunwebhook/diunwebhook.go` → `0` (confirmed)
|
||||||
|
- `grep -c "PRAGMA foreign_keys = ON" pkg/diunwebhook/diunwebhook.go` → `1` (confirmed)
|
||||||
|
- `grep -c "ON CONFLICT(image) DO UPDATE SET" pkg/diunwebhook/diunwebhook.go` → `1` (confirmed)
|
||||||
|
- Full test suite: 29 tests pass, 0 failures, coverage 63.6%
|
||||||
|
- `TestDismissHandler_ReappearsAfterNewWebhook` — PASS
|
||||||
|
- `TestDeleteTagHandler_CascadesAssignment` — PASS
|
||||||
|
- `TestUpdateEvent_PreservesTagOnUpsert` — PASS (new regression test)
|
||||||
|
|
||||||
|
## Known Stubs
|
||||||
|
|
||||||
|
None.
|
||||||
|
|
||||||
|
## Self-Check: PASSED
|
||||||
|
|
||||||
|
Files exist:
|
||||||
|
- FOUND: pkg/diunwebhook/diunwebhook.go (modified)
|
||||||
|
- FOUND: pkg/diunwebhook/diunwebhook_test.go (modified, contains TestUpdateEvent_PreservesTagOnUpsert)
|
||||||
|
|
||||||
|
Commits exist:
|
||||||
|
- FOUND: 7edbaad — fix(01-01): replace INSERT OR REPLACE with UPSERT and enable FK enforcement
|
||||||
|
- FOUND: e2d388c — test(01-01): add TestUpdateEvent_PreservesTagOnUpsert regression test
|
||||||
Reference in New Issue
Block a user