refactor(store): check all discarded errors in sqlstore_test.go

Closes #74

internal/monitor/checker.go

@@ -15,6 +15,16 @@ import (
 	probing "github.com/prometheus-community/pro-bing"
 )
 
+const (
+	maxErrorLength       = 256
+	defaultAcceptedCodes = "200-299"
+	defaultHTTPStatusMin = 200
+	defaultHTTPStatusMax = 300
+	defaultTimeout       = 5 * time.Second
+	defaultDNSServer     = "1.1.1.1"
+	defaultDNSPort       = "53"
+)
+
 type CheckResult struct {
 	SiteID      int
 	Status      string // "UP", "DOWN", "SSL EXP"
@@ -90,7 +100,7 @@ func runHTTPCheck(site models.Site, strict, insecure *http.Client, globalInsecur
 
 	if err != nil {
 		result.Status = "DOWN"
-		result.ErrorReason = truncateError(err.Error(), 256)
+		result.ErrorReason = truncateError(err.Error(), maxErrorLength)
 		return result
 	}
 	defer resp.Body.Close()
@@ -100,7 +110,7 @@ func runHTTPCheck(site models.Site, strict, insecure *http.Client, globalInsecur
 		result.Status = "DOWN"
 		expected := site.AcceptedCodes
 		if expected == "" {
-			expected = "200-299"
+			expected = defaultAcceptedCodes
 		}
 		result.ErrorReason = fmt.Sprintf("HTTP %d (expected %s)", resp.StatusCode, expected)
 	}
@@ -160,7 +170,7 @@ func runPortCheck(site models.Site) CheckResult {
 	latency := time.Since(start)
 
 	if err != nil {
-		return CheckResult{SiteID: site.ID, Status: "DOWN", LatencyNs: latency.Nanoseconds(), ErrorReason: truncateError(err.Error(), 256)}
+		return CheckResult{SiteID: site.ID, Status: "DOWN", LatencyNs: latency.Nanoseconds(), ErrorReason: truncateError(err.Error(), maxErrorLength)}
 	}
 	_ = conn.Close()
 	return CheckResult{SiteID: site.ID, Status: "UP", LatencyNs: latency.Nanoseconds()}
@@ -174,10 +184,10 @@ func runDNSCheck(site models.Site) CheckResult {
 
 	server := site.DNSServer
 	if server == "" {
-		server = "1.1.1.1"
+		server = defaultDNSServer
 	}
 	if _, _, err := net.SplitHostPort(server); err != nil {
-		server = net.JoinHostPort(server, "53")
+		server = net.JoinHostPort(server, defaultDNSPort)
 	}
 
 	qtype := dns.TypeA
@@ -223,12 +233,12 @@ func siteTimeout(site models.Site) time.Duration {
 	if site.Timeout > 0 {
 		return time.Duration(site.Timeout) * time.Second
 	}
-	return 5 * time.Second
+	return defaultTimeout
 }
 
 func isCodeAccepted(code int, accepted string) bool {
 	if accepted == "" {
-		return code >= 200 && code < 300
+		return code >= defaultHTTPStatusMin && code < defaultHTTPStatusMax
 	}
 	for _, part := range strings.Split(accepted, ",") {
 		part = strings.TrimSpace(part)

internal/monitor/monitor.go

@@ -138,7 +138,7 @@ func (e *Engine) AddLog(msg string) {
 }
 
 func (e *Engine) InitLogs() {
-	logs, err := e.db.LoadLogs(100)
+	logs, err := e.db.LoadLogs(maxLogEntries)
 	if err != nil {
 		return
 	}

internal/store/sqlstore.go

@@ -152,10 +152,26 @@ func (s *SQLStore) UpdateSitePaused(id int, paused bool) error {
 }
 
 func (s *SQLStore) DeleteSite(id int) error {
-	_, err := s.db.Exec(s.q("DELETE FROM sites WHERE id=?"), id)
+	tx, err := s.db.Begin()
 	if err != nil {
 		return err
 	}
+	defer func() { _ = tx.Rollback() }()
+
+	for _, q := range []string{
+		"DELETE FROM maintenance_windows WHERE monitor_id = ?",
+		"DELETE FROM check_history WHERE site_id = ?",
+		"DELETE FROM state_changes WHERE site_id = ?",
+		"DELETE FROM sites WHERE id = ?",
+	} {
+		if _, err := tx.Exec(s.q(q), id); err != nil {
+			return err
+		}
+	}
+
+	if err := tx.Commit(); err != nil {
+		return err
+	}
 	s.dialect.ResetSequenceOnEmpty(s.db, "sites")
 	return nil
 }

internal/store/sqlstore_test.go

@@ -1,8 +1,10 @@
 package store
 
 import (
-	"gitea.lerkolabs.com/lerkolabs/uptop/internal/models"
 	"testing"
+	"time"
+
+	"gitea.lerkolabs.com/lerkolabs/uptop/internal/models"
 )
 
 func newTestStore(t *testing.T) *SQLStore {
@@ -48,7 +50,10 @@ func TestSiteCRUD(t *testing.T) {
 		t.Fatalf("UpdateSite: %v", err)
 	}
 
-	sites, _ = s.GetSites()
+	sites, err = s.GetSites()
+	if err != nil {
+		t.Fatalf("GetSites: %v", err)
+	}
 	if sites[0].Name != "Updated" {
 		t.Errorf("expected name 'Updated', got '%s'", sites[0].Name)
 	}
@@ -57,7 +62,10 @@ func TestSiteCRUD(t *testing.T) {
 		t.Fatalf("DeleteSite: %v", err)
 	}
 
-	sites, _ = s.GetSites()
+	sites, err = s.GetSites()
+	if err != nil {
+		t.Fatalf("GetSites: %v", err)
+	}
 	if len(sites) != 0 {
 		t.Fatalf("expected 0 sites after delete, got %d", len(sites))
 	}
@@ -96,7 +104,10 @@ func TestAlertCRUD(t *testing.T) {
 		t.Fatalf("UpdateAlert: %v", err)
 	}
 
-	a, _ = s.GetAlert(a.ID)
+	a, err = s.GetAlert(a.ID)
+	if err != nil {
+		t.Fatalf("GetAlert: %v", err)
+	}
 	if a.Type != "slack" {
 		t.Errorf("expected type 'slack', got '%s'", a.Type)
 	}
@@ -105,7 +116,10 @@ func TestAlertCRUD(t *testing.T) {
 		t.Fatalf("DeleteAlert: %v", err)
 	}
 
-	alerts, _ = s.GetAllAlerts()
+	alerts, err = s.GetAllAlerts()
+	if err != nil {
+		t.Fatalf("GetAllAlerts: %v", err)
+	}
 	if len(alerts) != 0 {
 		t.Fatalf("expected 0 alerts after delete, got %d", len(alerts))
 	}
@@ -133,7 +147,10 @@ func TestUserCRUD(t *testing.T) {
 		t.Fatalf("UpdateUser: %v", err)
 	}
 
-	users, _ = s.GetAllUsers()
+	users, err = s.GetAllUsers()
+	if err != nil {
+		t.Fatalf("GetAllUsers: %v", err)
+	}
 	if users[0].Username != "root" {
 		t.Errorf("expected username 'root', got '%s'", users[0].Username)
 	}
@@ -142,7 +159,10 @@ func TestUserCRUD(t *testing.T) {
 		t.Fatalf("DeleteUser: %v", err)
 	}
 
-	users, _ = s.GetAllUsers()
+	users, err = s.GetAllUsers()
+	if err != nil {
+		t.Fatalf("GetAllUsers: %v", err)
+	}
 	if len(users) != 0 {
 		t.Fatalf("expected 0 users after delete, got %d", len(users))
 	}
@@ -155,7 +175,10 @@ func TestPushTokenGeneration(t *testing.T) {
 		t.Fatalf("AddSite: %v", err)
 	}
 
-	sites, _ := s.GetSites()
+	sites, err := s.GetSites()
+	if err != nil {
+		t.Fatalf("GetSites: %v", err)
+	}
 	if len(sites) != 1 {
 		t.Fatalf("expected 1 site, got %d", len(sites))
 	}
@@ -170,9 +193,15 @@ func TestPushTokenGeneration(t *testing.T) {
 func TestImportExport(t *testing.T) {
 	s := newTestStore(t)
 
-	s.AddAlert("Test Alert", "webhook", map[string]string{"url": "https://example.com"})
-	s.AddSite(models.Site{Name: "Site1", URL: "https://example.com", Type: "http", Interval: 30})
-	s.AddUser("user1", "ssh-ed25519 KEY", "user")
+	if err := s.AddAlert("Test Alert", "webhook", map[string]string{"url": "https://example.com"}); err != nil {
+		t.Fatalf("AddAlert: %v", err)
+	}
+	if err := s.AddSite(models.Site{Name: "Site1", URL: "https://example.com", Type: "http", Interval: 30}); err != nil {
+		t.Fatalf("AddSite: %v", err)
+	}
+	if err := s.AddUser("user1", "ssh-ed25519 KEY", "user"); err != nil {
+		t.Fatalf("AddUser: %v", err)
+	}
 
 	backup, err := s.ExportData()
 	if err != nil {
@@ -187,9 +216,18 @@ func TestImportExport(t *testing.T) {
 		t.Fatalf("ImportData: %v", err)
 	}
 
-	sites, _ := s2.GetSites()
-	alerts, _ := s2.GetAllAlerts()
-	users, _ := s2.GetAllUsers()
+	sites, err := s2.GetSites()
+	if err != nil {
+		t.Fatalf("GetSites: %v", err)
+	}
+	alerts, err := s2.GetAllAlerts()
+	if err != nil {
+		t.Fatalf("GetAllAlerts: %v", err)
+	}
+	users, err := s2.GetAllUsers()
+	if err != nil {
+		t.Fatalf("GetAllUsers: %v", err)
+	}
 	if len(sites) != 1 || len(alerts) != 1 || len(users) != 1 {
 		t.Fatalf("import mismatch: %d sites, %d alerts, %d users", len(sites), len(alerts), len(users))
 	}
@@ -229,3 +267,51 @@ func TestCheckHistory(t *testing.T) {
 		t.Errorf("expected 1 up record for site 1, got %d", upCount)
 	}
 }
+
+func TestDeleteSiteCascade(t *testing.T) {
+	s := newTestStore(t)
+
+	site := models.Site{Name: "Cascade Test", URL: "https://example.com", Interval: 30}
+	if err := s.AddSite(site); err != nil {
+		t.Fatalf("AddSite: %v", err)
+	}
+	sites, _ := s.GetSites()
+	siteID := sites[0].ID
+
+	if err := s.SaveCheck(siteID, 1000, true); err != nil {
+		t.Fatalf("SaveCheck: %v", err)
+	}
+	if err := s.SaveStateChange(siteID, "UP", "DOWN", "timeout"); err != nil {
+		t.Fatalf("SaveStateChange: %v", err)
+	}
+	mw := models.MaintenanceWindow{
+		MonitorID: siteID,
+		Title:     "Test MW",
+		Type:      "maintenance",
+		StartTime: time.Now(),
+	}
+	if err := s.AddMaintenanceWindow(mw); err != nil {
+		t.Fatalf("AddMaintenanceWindow: %v", err)
+	}
+
+	if err := s.DeleteSite(siteID); err != nil {
+		t.Fatalf("DeleteSite: %v", err)
+	}
+
+	history, _ := s.LoadAllHistory(100)
+	if len(history[siteID]) != 0 {
+		t.Errorf("expected 0 check_history rows, got %d", len(history[siteID]))
+	}
+
+	changes, _ := s.GetStateChanges(siteID, 100)
+	if len(changes) != 0 {
+		t.Errorf("expected 0 state_changes rows, got %d", len(changes))
+	}
+
+	windows, _ := s.GetActiveMaintenanceWindows()
+	for _, w := range windows {
+		if w.MonitorID == siteID {
+			t.Errorf("orphaned maintenance window found: id=%d", w.ID)
+		}
+	}
+}