felhom.eu/hub/internal/store/pragma_test.go

package store

import (
	"database/sql"
	"io"
	"log"
	"os"
	"path/filepath"
	"sync"
	"testing"
	"time"
)

// R-172 — the connection pragmas must actually be APPLIED, not merely requested.
//
// THE DEFECT THESE PIN. The DSN read `?_journal_mode=WAL&_busy_timeout=5000` — mattn/go-sqlite3
// syntax — while the driver is modernc.org/sqlite, which ignores unknown parameters WITHOUT AN
// ERROR. The hub therefore ran in rollback-journal mode with busy_timeout=0 for its entire life,
// and nothing said so. Any test that asserted the DSN STRING would have passed throughout.
//
// So every assertion below reads the value back from the DATABASE.

// newPragmaStore is a sibling of host_test.go's newTestStore that also returns the DB PATH, because
// two of the checks below are about the files SQLite creates beside it.
func newPragmaStore(t *testing.T) (*Store, string) {
	t.Helper()
	path := filepath.Join(t.TempDir(), "hub.db")
	s, err := New(path, log.New(io.Discard, "", 0))
	if err != nil {
		t.Fatalf("New: %v", err)
	}
	t.Cleanup(func() { s.Close() })
	return s, path
}

func TestStorePragmasAreActuallyApplied(t *testing.T) {
	// RED-PROOF: restore the old DSN (`?_journal_mode=WAL&_busy_timeout=5000`) and this fails with
	// journal_mode=delete, busy_timeout=0 — i.e. it reproduces the shipped bug exactly.
	// Demonstrated in hub REPORT.md.
	s, _ := newPragmaStore(t)

	var journal string
	if err := s.db.QueryRow("PRAGMA journal_mode").Scan(&journal); err != nil {
		t.Fatalf("read journal_mode: %v", err)
	}
	if journal != "wal" {
		t.Fatalf("journal_mode = %q, want \"wal\" — in rollback-journal mode a reader blocks a writer, "+
			"so rendering an operator page can 500 a host report (R-172)", journal)
	}

	var busy int
	if err := s.db.QueryRow("PRAGMA busy_timeout").Scan(&busy); err != nil {
		t.Fatalf("read busy_timeout: %v", err)
	}
	if busy < 5000 {
		t.Fatalf("busy_timeout = %d, want >= 5000 — without it SQLite returns SQLITE_BUSY immediately "+
			"instead of waiting, and the hub turns that into an HTTP 500", busy)
	}
}

func TestStoreWALFilesExistWhileOpen(t *testing.T) {
	// The on-disk observable that EXPOSED the bug in production, pinned as a test: in WAL mode the
	// `-wal` and `-shm` files must exist beside an open database. Their absence on the live hub is
	// what proved the pragma was never applied, so it is the check to keep.
	s, path := newPragmaStore(t)

	// A write guarantees the WAL is materialised rather than merely configured.
	if _, err := s.db.Exec(`CREATE TABLE IF NOT EXISTS r172_probe (k TEXT)`); err != nil {
		t.Fatalf("probe write: %v", err)
	}
	if _, err := s.db.Exec(`INSERT INTO r172_probe (k) VALUES ('x')`); err != nil {
		t.Fatalf("probe insert: %v", err)
	}

	for _, suffix := range []string{"-wal", "-shm"} {
		if _, err := os.Stat(path + suffix); err != nil {
			t.Fatalf("%s is missing beside an OPEN database (%v) — this is exactly the signature that "+
				"proved the live hub was NOT in WAL mode", filepath.Base(path+suffix), err)
		}
	}
}

func TestStoreReaderDoesNotBlockWriter(t *testing.T) {
	// THE CONSEQUENCE, not the mechanism. A held READ transaction — what rendering an operator page
	// does — must not make a concurrent write fail. In rollback-journal mode it does, and that is the
	// whole of R-172: `Failed to save host-report …: database is locked (5) (SQLITE_BUSY)`.
	s, _ := newPragmaStore(t)
	if _, err := s.db.Exec(`CREATE TABLE IF NOT EXISTS r172_probe (k TEXT)`); err != nil {
		t.Fatalf("setup: %v", err)
	}
	if _, err := s.db.Exec(`INSERT INTO r172_probe (k) VALUES ('seed')`); err != nil {
		t.Fatalf("seed: %v", err)
	}

	// Hold a read open for the whole write.
	rows, err := s.db.Query(`SELECT k FROM r172_probe`)
	if err != nil {
		t.Fatalf("open read: %v", err)
	}
	defer rows.Close()
	if !rows.Next() {
		t.Fatal("expected a seeded row")
	}

	done := make(chan error, 1)
	go func() {
		_, err := s.db.Exec(`INSERT INTO r172_probe (k) VALUES ('concurrent')`)
		done <- err
	}()

	select {
	case err := <-done:
		if err != nil {
			t.Fatalf("a write FAILED while a read was open: %v — this is the live 500 (R-172)", err)
		}
	case <-time.After(15 * time.Second):
		t.Fatal("a write BLOCKED indefinitely while a read was open")
	}
}

func TestStoreConcurrentWritersDoNotReturnBusy(t *testing.T) {
	// Writers still serialise under WAL; busy_timeout is what turns that into a WAIT rather than an
	// error. Several concurrent writers must all succeed — the host report, the event save and a UI
	// action genuinely do overlap on this hub.
	s, _ := newPragmaStore(t)
	if _, err := s.db.Exec(`CREATE TABLE IF NOT EXISTS r172_probe (k TEXT)`); err != nil {
		t.Fatalf("setup: %v", err)
	}

	const writers = 8
	var wg sync.WaitGroup
	errs := make(chan error, writers)
	for i := 0; i < writers; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			if _, err := s.db.Exec(`INSERT INTO r172_probe (k) VALUES ('w')`); err != nil {
				errs <- err
			}
		}()
	}
	wg.Wait()
	close(errs)
	for err := range errs {
		t.Fatalf("a concurrent writer returned an error instead of waiting: %v", err)
	}
}

func TestStoreTransactionUpgradeDoesNotReturnBusySnapshot(t *testing.T) {
	// `_txlock=immediate` is the parameter that is easy to leave out, and WAL + busy_timeout alone
	// would not cover this. With a DEFERRED transaction (database/sql's default), a tx that reads and
	// then writes must upgrade its lock, and a failed upgrade is SQLITE_BUSY_SNAPSHOT — which
	// busy_timeout does NOT retry. This store has 10+ db.Begin() sites and they are all write paths.
	//
	// RED-PROOF: drop `&_txlock=immediate` from sqliteDSNParams and this test becomes able to fail
	// (it is inherently racy without it, which is precisely the point — an un-retryable error that
	// appears only under contention is the worst kind to ship). Demonstrated in hub REPORT.md.
	s, _ := newPragmaStore(t)
	if _, err := s.db.Exec(`CREATE TABLE IF NOT EXISTS r172_probe (k TEXT)`); err != nil {
		t.Fatalf("setup: %v", err)
	}
	if _, err := s.db.Exec(`INSERT INTO r172_probe (k) VALUES ('seed')`); err != nil {
		t.Fatalf("seed: %v", err)
	}

	// Two read-then-write transactions racing is the upgrade shape.
	const txs = 6
	var wg sync.WaitGroup
	errs := make(chan error, txs)
	for i := 0; i < txs; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			tx, err := s.db.Begin()
			if err != nil {
				errs <- err
				return
			}
			var n int
			if err := tx.QueryRow(`SELECT COUNT(*) FROM r172_probe`).Scan(&n); err != nil {
				tx.Rollback()
				errs <- err
				return
			}
			if _, err := tx.Exec(`INSERT INTO r172_probe (k) VALUES ('tx')`); err != nil {
				tx.Rollback()
				errs <- err
				return
			}
			if err := tx.Commit(); err != nil {
				errs <- err
			}
		}()
	}
	wg.Wait()
	close(errs)
	for err := range errs {
		t.Fatalf("a read-then-write transaction failed under contention: %v — this is the "+
			"SQLITE_BUSY_SNAPSHOT that busy_timeout cannot retry, and _txlock=immediate prevents", err)
	}
}

// TestSQLiteDriverIgnoresMattnStyleParams is the regression guard for the ROOT CAUSE, not the symptom.
//
// It documents, executably, that the old DSN syntax is silently ignored by this driver — so anyone
// who "tidies" the pragmas back to the more familiar mattn form gets a failing test instead of a
// hub that quietly reverts to rollback-journal mode for another few months.
func TestSQLiteDriverIgnoresMattnStyleParams(t *testing.T) {
	dir := t.TempDir()
	path := filepath.Join(dir, "mattnstyle.db")

	db, err := sql.Open("sqlite", path+"?_journal_mode=WAL&_busy_timeout=5000")
	if err != nil {
		t.Fatalf("open: %v", err)
	}
	defer db.Close()

	var journal string
	if err := db.QueryRow("PRAGMA journal_mode").Scan(&journal); err != nil {
		t.Fatalf("read journal_mode: %v", err)
	}
	if journal == "wal" {
		t.Skip("this driver now honours mattn-style parameters — the R-172 trap is gone; simplify " +
			"sqliteDSNParams and delete this test")
	}
	if journal != "delete" {
		t.Fatalf("journal_mode = %q, expected the driver to IGNORE the mattn-style parameter and leave "+
			"the default; if this changed, re-read sqliteDSNParams", journal)
	}
}