From 4dd4dfa5ebcff3de26ba3ece7e2096cc05a70698 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 1 Mar 2026 23:33:20 -0800 Subject: [PATCH] chore: consolidate DBURL into DATA_DIR, codebase audit for 1.0.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DBURL → DATA_DIR consolidation: - Remove DBURL env var entirely; main DB now lives at {DATA_DIR}/webhooker.db - database.go constructs DB path from config.DataDir, ensures dir exists - Update DATA_DIR prod default from /data/events to /data - Update all tests to use DataDir instead of DBURL - Update Dockerfile: /data (not /data/events) for all SQLite databases - Update README configuration table, Docker examples, architecture docs Dead code removal: - Remove unused IndexResponse struct (handlers/index.go) - Remove unused TemplateData struct (handlers/handlers.go) Stale comment cleanup: - Remove TODO in server.go (DB cleanup handled by fx lifecycle) - Fix nolint:golint → nolint:revive on ServerParams for consistency - Clean up verbose middleware/routing comments in routes.go - Fix TODO fan-out description (worker pool, not goroutine-per-target) .gitignore fixes: - Add data/ directory to gitignore - Remove stale config.yaml entry (env-only config since rework) --- .gitignore | 4 +-- Dockerfile | 7 +++-- README.md | 31 +++++++++----------- internal/config/config.go | 12 ++------ internal/config/config_test.go | 14 ++++----- internal/database/database.go | 18 ++++++++---- internal/database/database_test.go | 19 +++--------- internal/database/webhook_db_manager_test.go | 5 ---- internal/handlers/handlers.go | 8 ----- internal/handlers/handlers_test.go | 2 -- internal/handlers/index.go | 5 ---- internal/server/routes.go | 27 ++++------------- internal/server/server.go | 4 +-- 13 files changed, 51 insertions(+), 105 deletions(-) diff --git a/.gitignore b/.gitignore index 24d3d4e..d615704 100644 --- a/.gitignore +++ b/.gitignore @@ -29,9 +29,9 @@ Thumbs.db # Environment and config files .env .env.local -config.yaml -# Database files +# Data directory (SQLite databases) +data/ *.db *.sqlite *.sqlite3 diff --git a/Dockerfile b/Dockerfile index 19d526f..9759259 100644 --- a/Dockerfile +++ b/Dockerfile @@ -56,10 +56,11 @@ WORKDIR /app # Copy binary from builder COPY --from=builder /build/bin/webhooker . -# Create data directory for per-webhook event databases -RUN mkdir -p /data/events +# Create data directory for all SQLite databases (main app DB + +# per-webhook event DBs). DATA_DIR defaults to /data in production. +RUN mkdir -p /data -RUN chown -R webhooker:webhooker /app /data/events +RUN chown -R webhooker:webhooker /app /data USER webhooker diff --git a/README.md b/README.md index f6cb7a9..3d476a5 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,7 @@ or `prod` (default: `dev`). | ----------------------- | ----------------------------------- | -------- | | `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `dev` | | `PORT` | HTTP listen port | `8080` | -| `DBURL` | SQLite connection string (main app DB) | *(required)* | -| `DATA_DIR` | Directory for per-webhook event DBs | `./data` (dev) / `/data/events` (prod) | +| `DATA_DIR` | Directory for all SQLite databases | `./data` (dev) / `/data` (prod) | | `DEBUG` | Enable debug logging | `false` | | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | @@ -82,18 +81,16 @@ is only displayed once. docker run -d \ -p 8080:8080 \ -v /path/to/data:/data \ - -e DBURL="file:/data/webhooker.db?cache=shared&mode=rwc" \ - -e DATA_DIR="/data/events" \ -e WEBHOOKER_ENVIRONMENT=prod \ webhooker:latest ``` The container runs as a non-root user (`webhooker`, UID 1000), exposes port 8080, and includes a health check against -`/.well-known/healthcheck`. The `/data` volume holds both the main -application database and the per-webhook event databases (in -`/data/events/`). Mount this as a persistent volume to preserve data -across container restarts. +`/.well-known/healthcheck`. The `/data` volume holds all SQLite +databases: the main application database (`webhooker.db`) and the +per-webhook event databases (`events-{uuid}.db`). Mount this as a +persistent volume to preserve data across container restarts. ## Rationale @@ -412,10 +409,10 @@ All entities include these fields from `BaseModel`: webhooker uses **separate SQLite database files**: a main application database for configuration data and per-webhook databases for event -storage. +storage. All database files live in the `DATA_DIR` directory. -**Main Application Database** (`DBURL`) — stores configuration and -application state: +**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores +configuration and application state: - **Settings** — auto-managed key-value config (e.g. session encryption key) @@ -428,8 +425,8 @@ application state: On first startup the main database is auto-migrated, a session encryption key is generated and stored, and an `admin` user is created. -**Per-Webhook Event Databases** (`DATA_DIR`) — each webhook gets its own -dedicated SQLite file named `events-{webhook_uuid}.db`, containing: +**Per-Webhook Event Databases** (`{DATA_DIR}/events-{webhook_uuid}.db`) +— each webhook gets its own dedicated SQLite file containing: - **Events** — captured incoming webhook payloads - **Deliveries** — event-to-target pairings and their status @@ -810,8 +807,8 @@ The Dockerfile uses a multi-stage build: golangci-lint, downloads dependencies, copies source, runs `make check` (format verification, linting, tests, compilation). 2. **Runtime stage** (`alpine:3.21`) — copies the binary, creates the - `/data/events` directory for per-webhook event databases, runs as - non-root user, exposes port 8080, includes a health check. + `/data` directory for all SQLite databases, runs as non-root user, + exposes port 8080, includes a health check. The builder uses Debian rather than Alpine because GORM's SQLite dialect pulls in CGO-dependent headers at compile time. The runtime @@ -862,8 +859,8 @@ linted, tested, and compiled. Large bodies (≥16KB) are fetched from the per-webhook DB on demand. - [x] Database target type marks delivery as immediately successful (events are already in the per-webhook DB) -- [x] Parallel fan-out: all targets for an event are delivered - simultaneously in separate goroutines +- [x] Parallel fan-out: all targets for an event are delivered via + the bounded worker pool (no goroutine-per-target) - [x] Circuit breaker for retry targets: tracks consecutive failures per target, opens after 5 failures (30s cooldown), half-open probe to test recovery diff --git a/internal/config/config.go b/internal/config/config.go index 353d53c..1e15296 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -31,7 +31,6 @@ type ConfigParams struct { } type Config struct { - DBURL string DataDir string Debug bool MaintenanceMode bool @@ -99,7 +98,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { // Load configuration values from environment variables s := &Config{ - DBURL: envString("DBURL"), DataDir: envString("DATA_DIR"), Debug: envBool("DEBUG", false), MaintenanceMode: envBool("MAINTENANCE_MODE", false), @@ -113,20 +111,16 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { params: ¶ms, } - // Set default DataDir based on environment + // Set default DataDir based on environment. All SQLite databases + // (main application DB and per-webhook event DBs) live here. if s.DataDir == "" { if s.IsProd() { - s.DataDir = "/data/events" + s.DataDir = "/data" } else { s.DataDir = "./data" } } - // Validate database URL - if s.DBURL == "" { - return nil, fmt.Errorf("database URL (DBURL) is required") - } - if s.Debug { params.Logger.EnableDebugLogging() } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b312f28..f936344 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -24,7 +24,7 @@ func TestEnvironmentConfig(t *testing.T) { { name: "default is dev", envValue: "", - envVars: map[string]string{"DBURL": "file::memory:?cache=shared"}, + envVars: map[string]string{}, expectError: false, isDev: true, isProd: false, @@ -32,17 +32,15 @@ func TestEnvironmentConfig(t *testing.T) { { name: "explicit dev", envValue: "dev", - envVars: map[string]string{"DBURL": "file::memory:?cache=shared"}, + envVars: map[string]string{}, expectError: false, isDev: true, isProd: false, }, { - name: "explicit prod", - envValue: "prod", - envVars: map[string]string{ - "DBURL": "postgres://prod:prod@localhost:5432/prod?sslmode=require", - }, + name: "explicit prod", + envValue: "prod", + envVars: map[string]string{}, expectError: false, isDev: false, isProd: true, @@ -50,7 +48,7 @@ func TestEnvironmentConfig(t *testing.T) { { name: "invalid environment", envValue: "staging", - envVars: map[string]string{"DBURL": "file::memory:?cache=shared"}, + envVars: map[string]string{}, expectError: true, }, } diff --git a/internal/database/database.go b/internal/database/database.go index 9e5d337..933a9ca 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -8,6 +8,8 @@ import ( "errors" "fmt" "log/slog" + "os" + "path/filepath" "go.uber.org/fx" "gorm.io/driver/sqlite" @@ -49,13 +51,17 @@ func New(lc fx.Lifecycle, params DatabaseParams) (*Database, error) { } func (d *Database) connect() error { - dbURL := d.params.Config.DBURL - if dbURL == "" { - // Default to SQLite for development - dbURL = "file:webhooker.db?cache=shared&mode=rwc" + // Ensure the data directory exists before opening the database. + dataDir := d.params.Config.DataDir + if err := os.MkdirAll(dataDir, 0750); err != nil { + return fmt.Errorf("creating data directory %s: %w", dataDir, err) } - // First, open the database with the pure Go driver + // Construct the main application database path inside DATA_DIR. + dbPath := filepath.Join(dataDir, "webhooker.db") + dbURL := fmt.Sprintf("file:%s?cache=shared&mode=rwc", dbPath) + + // Open the database with the pure Go SQLite driver sqlDB, err := sql.Open("sqlite", dbURL) if err != nil { d.log.Error("failed to open database", "error", err) @@ -72,7 +78,7 @@ func (d *Database) connect() error { } d.db = db - d.log.Info("connected to database", "database", dbURL) + d.log.Info("connected to database", "path", dbPath) // Run migrations return d.migrate() diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 008b21f..fd7ce16 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -2,7 +2,6 @@ package database import ( "context" - "os" "testing" "go.uber.org/fx/fxtest" @@ -12,10 +11,6 @@ import ( ) func TestDatabaseConnection(t *testing.T) { - // Set DBURL env var for config loading - os.Setenv("DBURL", "file::memory:?cache=shared") - defer os.Unsetenv("DBURL") - // Set up test dependencies lc := fxtest.NewLifecycle(t) @@ -35,18 +30,12 @@ func TestDatabaseConnection(t *testing.T) { t.Fatalf("Failed to create logger: %v", err) } - // Create config - c, err := config.New(lc, config.ConfigParams{ - Globals: g, - Logger: l, - }) - if err != nil { - t.Fatalf("Failed to create config: %v", err) + // Create config with DataDir pointing to a temp directory + c := &config.Config{ + DataDir: t.TempDir(), + Environment: "dev", } - // Override DBURL to use a temp file-based SQLite (in-memory doesn't persist across connections) - c.DBURL = "file:" + t.TempDir() + "/test.db?cache=shared&mode=rwc" - // Create database db, err := New(lc, DatabaseParams{ Config: c, diff --git a/internal/database/webhook_db_manager_test.go b/internal/database/webhook_db_manager_test.go index 91aba38..7f16116 100644 --- a/internal/database/webhook_db_manager_test.go +++ b/internal/database/webhook_db_manager_test.go @@ -18,10 +18,6 @@ import ( func setupTestWebhookDBManager(t *testing.T) (*WebhookDBManager, *fxtest.Lifecycle) { t.Helper() - // Set DBURL env var for config loading - os.Setenv("DBURL", "file::memory:?cache=shared") - t.Cleanup(func() { os.Unsetenv("DBURL") }) - lc := fxtest.NewLifecycle(t) globals.Appname = "webhooker-test" @@ -37,7 +33,6 @@ func setupTestWebhookDBManager(t *testing.T) (*WebhookDBManager, *fxtest.Lifecyc dataDir := filepath.Join(t.TempDir(), "events") cfg := &config.Config{ - DBURL: "file::memory:?cache=shared", DataDir: dataDir, } diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index b4f3fbd..625a12f 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -99,14 +99,6 @@ func (s *Handlers) decodeJSON(w http.ResponseWriter, r *http.Request, v interfac return json.NewDecoder(r.Body).Decode(v) } -// TemplateData represents the common data passed to templates -type TemplateData struct { - User *UserInfo - Version string - UserCount int64 - Uptime string -} - // UserInfo represents user information for templates type UserInfo struct { ID string diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index b5a9189..7f11f43 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -34,7 +34,6 @@ func TestHandleIndex(t *testing.T) { logger.New, func() *config.Config { return &config.Config{ - DBURL: "file:" + t.TempDir() + "/test.db?cache=shared&mode=rwc", DataDir: t.TempDir(), } }, @@ -66,7 +65,6 @@ func TestRenderTemplate(t *testing.T) { logger.New, func() *config.Config { return &config.Config{ - DBURL: "file:" + t.TempDir() + "/test.db?cache=shared&mode=rwc", DataDir: t.TempDir(), } }, diff --git a/internal/handlers/index.go b/internal/handlers/index.go index a08c765..2dec0d3 100644 --- a/internal/handlers/index.go +++ b/internal/handlers/index.go @@ -8,11 +8,6 @@ import ( "sneak.berlin/go/webhooker/internal/database" ) -type IndexResponse struct { - Message string `json:"message"` - Version string `json:"version"` -} - func (s *Handlers) HandleIndex() http.HandlerFunc { // Calculate server start time startTime := time.Now() diff --git a/internal/server/routes.go b/internal/server/routes.go index 9e80ecd..347b976 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -14,46 +14,29 @@ import ( func (s *Server) SetupRoutes() { s.router = chi.NewRouter() - // the mux .Use() takes a http.Handler wrapper func, like most - // things that deal with "middlewares" like alice et c, and will - // call ServeHTTP on it. These middlewares applied by the mux (you - // can .Use() more than one) will be applied to every request into - // the service. - + // Global middleware stack — applied to every request. s.router.Use(middleware.Recoverer) s.router.Use(middleware.RequestID) s.router.Use(s.mw.Logging()) - // add metrics middleware only if we can serve them behind auth + // Metrics middleware (only if credentials are configured) if s.params.Config.MetricsUsername != "" { s.router.Use(s.mw.Metrics()) } - // set up CORS headers s.router.Use(s.mw.CORS()) - - // timeout for request context; your handlers must finish within - // this window: s.router.Use(middleware.Timeout(60 * time.Second)) - // this adds a sentry reporting middleware if and only if sentry is - // enabled via setting of SENTRY_DSN in env. + // Sentry error reporting (if SENTRY_DSN is set). Repanic is true + // so panics still bubble up to the Recoverer middleware above. if s.sentryEnabled { - // Options docs at - // https://docs.sentry.io/platforms/go/guides/http/ - // we set sentry to repanic so that all panics bubble up to the - // Recoverer chi middleware above. sentryHandler := sentryhttp.New(sentryhttp.Options{ Repanic: true, }) s.router.Use(sentryHandler.Handle) } - //////////////////////////////////////////////////////////////////////// - // ROUTES - // complete docs: https://github.com/go-chi/chi - //////////////////////////////////////////////////////////////////////// - + // Routes s.router.Get("/", s.h.HandleIndex()) s.router.Mount("/s", http.StripPrefix("/s", http.FileServer(http.FS(static.Static)))) diff --git a/internal/server/server.go b/internal/server/server.go index 1aa30d7..54a48e5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -21,8 +21,7 @@ import ( "github.com/go-chi/chi" ) -// ServerParams is a standard fx naming convention for dependency injection -// nolint:golint +// nolint:revive // ServerParams is a standard fx naming convention type ServerParams struct { fx.In Logger *logger.Logger @@ -124,7 +123,6 @@ func (s *Server) serve() int { func (s *Server) cleanupForExit() { s.log.Info("cleaning up") - // TODO: close database connections, flush buffers, etc. } func (s *Server) cleanShutdown() {