refactor: quality refactor across all subsystems #3

2026-05-15T14:07:03Z

lerko commented

2026-05-15 14:07:03 +00:00

Summary

Full codebase quality refactor against pillars: readability, maintainability, correctness, simplicity, DRY, SOLID, performance, robustness, testability.

Phase 1: Correctness fixes

XSS fix in import error responses
template.Must at package init (was ignoring parse error)
defer resp.Body.Close() in all alert providers
Shared HTTP clients with per-site context timeout
AcceptedCodes validation (was hardcoded >= 400)
HTTP method support (was always GET)
defer tx.Rollback() in ImportData
ListenAndServe error handling

Phase 2: Store deduplication

Dialect interface abstracts SQLite/Postgres differences
Single SQLStore implementation (was 95% duplicated across 2 files)
sqlite.go: 286 → 83 lines, postgres.go: 266 → 78 lines

Phase 3: Error returns

All Store interface methods return errors
All rows.Scan() errors checked
~25 caller sites updated with error handling

Phase 4: Dependency injection

Removed store.Get()/SetGlobal() global singleton
Store threaded explicitly via constructors/parameters

Phase 5: TUI DRY

Shared table rendering helper (was duplicated across 3 tabs)
Cursor bounds clamp in refreshData
Tab click handler off-by-one fix

Phase 6: Alert DRY

HTTPProvider with PayloadFunc (Discord/Slack/Webhook deduplicated)
HTTP status code checking (4xx/5xx now return errors)

Phase 7: Engine struct + shutdown + tests

Engine struct encapsulates all monitor state
Context-based graceful shutdown (no more leaked goroutines)
Token index for O(1) push heartbeat lookup
UpdateSiteConfig: struct copy preserving runtime state
First test suite: 12 tests (store CRUD, alert providers)

Test plan

go build ./... passes
go vet ./... passes
go test ./... — 12 tests pass
Functional test with -demo flag — DB created, data seeded
Manual TUI verification (SSH + direct)
Verify push heartbeat endpoint
Test with PostgreSQL backend

## Summary Full codebase quality refactor against pillars: readability, maintainability, correctness, simplicity, DRY, SOLID, performance, robustness, testability. ### Phase 1: Correctness fixes - XSS fix in import error responses - template.Must at package init (was ignoring parse error) - defer resp.Body.Close() in all alert providers - Shared HTTP clients with per-site context timeout - AcceptedCodes validation (was hardcoded >= 400) - HTTP method support (was always GET) - defer tx.Rollback() in ImportData - ListenAndServe error handling ### Phase 2: Store deduplication - Dialect interface abstracts SQLite/Postgres differences - Single SQLStore implementation (was 95% duplicated across 2 files) - sqlite.go: 286 → 83 lines, postgres.go: 266 → 78 lines ### Phase 3: Error returns - All Store interface methods return errors - All rows.Scan() errors checked - ~25 caller sites updated with error handling ### Phase 4: Dependency injection - Removed store.Get()/SetGlobal() global singleton - Store threaded explicitly via constructors/parameters ### Phase 5: TUI DRY - Shared table rendering helper (was duplicated across 3 tabs) - Cursor bounds clamp in refreshData - Tab click handler off-by-one fix ### Phase 6: Alert DRY - HTTPProvider with PayloadFunc (Discord/Slack/Webhook deduplicated) - HTTP status code checking (4xx/5xx now return errors) ### Phase 7: Engine struct + shutdown + tests - Engine struct encapsulates all monitor state - Context-based graceful shutdown (no more leaked goroutines) - Token index for O(1) push heartbeat lookup - UpdateSiteConfig: struct copy preserving runtime state - First test suite: 12 tests (store CRUD, alert providers) ## Test plan - [x] `go build ./...` passes - [x] `go vet ./...` passes - [x] `go test ./...` — 12 tests pass - [x] Functional test with `-demo` flag — DB created, data seeded - [ ] Manual TUI verification (SSH + direct) - [ ] Verify push heartbeat endpoint - [ ] Test with PostgreSQL backend

lerko added 7 commits 2026-05-15 14:07:03 +00:00

fix(core): correctness and robustness fixes across all subsystems 4d5116644f

- Move status page template to package-level template.Must (panic on
  parse error at init instead of nil deref at runtime)
- Fix XSS in import error responses (log detail server-side, return
  generic message to client)
- Handle ListenAndServe errors in HTTP and SSH servers
- Use defer resp.Body.Close() in all alert providers, check
  json.Marshal errors
- Share HTTP clients across checks instead of creating per-request
- Use http.NewRequestWithContext for per-site timeout control
- Support HTTP method field (was always GET despite DB storing method)
- Implement AcceptedCodes validation (was hardcoded >= 400 despite DB
  storing accepted code ranges)
- Add defer tx.Rollback() to ImportData for transaction safety

refactor(store): unify SQLite and Postgres into dialect-based SQLStore ab75f61c6b

Extract shared SQLStore with Dialect interface for the ~5% that
differs between backends (DDL, placeholders, sequence resets).

- New dialect.go: Dialect interface + placeholder rewriter (? → $N)
- New sqlstore.go: single implementation of all 19 Store methods
- sqlite.go: reduced from 286 to 83 lines (SQLiteDialect only)
- postgres.go: reduced from 266 to 78 lines (PostgresDialect only)
- main.go: use NewSQLiteStore/NewPostgresStore constructors

Zero CRUD logic duplication. Every future schema change written once.

refactor(store): add error returns to all Store interface methods d4f4012c8a

Every Store method now returns an error. Callers handle errors
gracefully — TUI logs to event log, server returns HTTP 500,
monitor engine logs and retries. All rows.Scan() errors are now
checked in sqlstore.go instead of silently appending corrupt data.

- GetSites, GetAllAlerts, GetAllUsers return ([]T, error)
- GetAlert returns (AlertConfig, error) instead of (AlertConfig, bool)
- AddSite, UpdateSite, DeleteSite, etc. all return error
- SaveCheck, LoadAllHistory, ExportData return error
- ~25 caller sites updated across tui, server, monitor, main

refactor(core): remove store global singleton, thread store explicitly a6bb9a7aff

Remove store.Get()/SetGlobal()/Current. Store is now passed explicitly
to all consumers via constructor parameters and function arguments.

- TUI Model holds store field, set via InitialModel(isAdmin, store)
- monitor.StartEngine(s) and InitHistoryFromStore(s) accept store
- server.Start(cfg, s) closes over store in HTTP handlers
- main.go threads store to SSH server, TUI, monitor, server
- isKeyAllowed receives store as parameter

No more hidden dependency on package-level mutable state in store pkg.
Monitor package still uses package-level state (LiveState, etc.) — will
be encapsulated into Engine struct in Phase 7.

refactor(alert): extract shared HTTPProvider for webhook-based alerts d6f33a4d1f

Discord, Slack, and Webhook providers now use a single HTTPProvider
struct with a PayloadFunc for the only part that differs. Centralizes
response body handling and adds HTTP status code checking (4xx/5xx
now return errors instead of being silently ignored).

Email and Ntfy keep separate implementations (different protocols).
Adding a new HTTP-based alert provider is now a one-line PayloadFunc.

refactor(tui): extract shared table rendering, fix cursor bounds 0e6dc774cb

- New table_helpers.go with renderTable() and shared styles
- Remove 4 duplicated style blocks (header/cell/selected/border)
  from tab_alerts.go and tab_users.go
- All 3 tab views now use renderTable() for offset/end calc,
  selected row highlighting, and table construction
- Sites tab keeps siteGroupStyle via StyleOverride callback
- Clamp cursor to list length at end of refreshData() to prevent
  index-out-of-bounds after concurrent list changes
- Fix off-by-one in tab click handler (i <= maxTabs → i < tabCount)

refactor(monitor): encapsulate engine state, add graceful shutdown and tests f023e38fdc

Replace all monitor package-level mutable state with Engine struct.
All state (liveState, logStore, histories, tokenIndex, HTTP clients)
is now encapsulated in Engine, created via NewEngine(store).

Key changes:
- Engine struct holds all monitor state with proper mutex protection
- Engine.Start(ctx) and monitorRoutine respect context cancellation
  for graceful shutdown — no more leaked goroutines
- cluster.runFollowerLoop also respects context for clean exit
- Token index (map[string]int) for O(1) push heartbeat lookup,
  replacing O(n) linear scan through LiveState
- UpdateSiteConfig preserves 8 runtime fields instead of copying
  17 config fields individually
- triggerAlert goroutines get 30s timeout context
- All consumers (TUI, server, cluster, main) receive *Engine via
  constructor/parameter — no package-level state access
- main.go creates context.WithCancel, passes to engine and cluster

First test suite: 12 tests across store and alert packages
- Store: CRUD for sites/alerts/users, push token generation,
  import/export round-trip, check history persistence
- Alert: Discord/Slack/Webhook payload format, HTTP 4xx error
  propagation, Ntfy headers, unknown provider returns nil

lerko added 3 commits 2026-05-15 15:35:04 +00:00

feat(alert): add Telegram, PagerDuty, Pushover, Gotify providers 52a54f9c5c

Expand alert provider count from 5 to 9. All new providers use
the shared HTTPProvider with closure-based payload functions.
Includes TUI form support and tests for each provider.

feat(metrics): add Prometheus /metrics endpoint b7b8aa6f03

Zero-dependency Prometheus text exposition format. Exposes monitor
up/down, latency, status code, check timestamps, pause state,
SSL cert expiry, and check counters — all from in-memory state.

Merge pull request 'feat(metrics): add Prometheus /metrics endpoint' (#5 ) from feat/prometheus-metrics into feat/next 079270274f

Reviewed-on: lerko/uptime#5

lerko closed this pull request

2026-05-15 15:35:52 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lerkolabs/uptop#3