feat: Remove global db.Dbpool with dependency injection (Phase 0)
- Add Database struct in internal/db/database.go with Pool, Ctx, and RunMigrations() - Update server.go to use Database struct with NewServerInstance() - Add backend.go with InitBackend(), BackendRepo(), BackendCtx(), BackendPool() - Update music.go and sync.go to use BackendRepo() and BackendCtx() instead of db.Dbpool/db.Ctx - Update token_handler.go to accept pool parameter - Update routes.go to use s.db.Pool for middleware - Update cmd/main.go to use NewServerInstance() and HTTPServer() - Update test_helpers.go to initialize backend with test database - Update test files to use backend.BackendPool() and backend.BackendCtx() Benefits: - Easier to mock database for unit tests - Follows Go best practices (dependency injection) - Better architecture with explicit dependencies - RunMigrations() replaces old Migrate_db() function Note: Global db.Dbpool and db.Ctx still exist in dbHelper.go for backward compatibility with test_helpers.go, but production code no longer uses them. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
This commit is contained in:
@@ -2,7 +2,6 @@ package server
|
||||
|
||||
import (
|
||||
"music-server/cmd/web"
|
||||
"music-server/internal/db"
|
||||
"music-server/internal/logging"
|
||||
"music-server/internal/server/middleware"
|
||||
"net/http"
|
||||
@@ -121,7 +120,7 @@ func (s *Server) RegisterRoutes() http.Handler {
|
||||
|
||||
// Protected endpoints - require valid token
|
||||
// Create token auth middleware with pool access
|
||||
tokenAuthMiddleware := middleware.TokenAuthMiddleware(db.Dbpool)
|
||||
tokenAuthMiddleware := middleware.TokenAuthMiddleware(s.db.Pool)
|
||||
|
||||
// Protected group with token authentication - will be used by VGMQ and Statistics API
|
||||
_ = apiV1.Group("", tokenAuthMiddleware)
|
||||
|
||||
+58
-25
@@ -6,6 +6,7 @@ import (
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"music-server/internal/backend"
|
||||
"music-server/internal/db"
|
||||
"music-server/internal/logging"
|
||||
"net/http"
|
||||
@@ -15,7 +16,9 @@ import (
|
||||
|
||||
type Server struct {
|
||||
port int
|
||||
db *db.Database
|
||||
tokenHandler *TokenHandler
|
||||
httpServer *http.Server
|
||||
}
|
||||
|
||||
var (
|
||||
@@ -30,7 +33,9 @@ var (
|
||||
logJSON = os.Getenv("LOG_JSON") == "true"
|
||||
)
|
||||
|
||||
func NewServer() *http.Server {
|
||||
// NewServerInstance creates a new Server instance with all dependencies initialized.
|
||||
// Use this for dependency injection and proper lifecycle management.
|
||||
func NewServerInstance() *Server {
|
||||
// Initialize logger
|
||||
if logLevel == "" {
|
||||
logLevel = "info"
|
||||
@@ -40,15 +45,45 @@ func NewServer() *http.Server {
|
||||
logger := logging.GetLogger()
|
||||
|
||||
port, _ := strconv.Atoi(os.Getenv("PORT"))
|
||||
|
||||
// Initialize token handler
|
||||
tokenHandler := NewTokenHandler()
|
||||
|
||||
NewServer := &Server{
|
||||
|
||||
// Validate required environment variables
|
||||
if host == "" || dbPort == "" || username == "" || password == "" || dbName == "" || musicPath == "" || charactersPath == "" {
|
||||
logging.GetLogger().Fatal("Invalid settings - missing required environment variables")
|
||||
}
|
||||
|
||||
// Create database instance
|
||||
database, err := db.NewDatabase(host, dbPort, username, password, dbName)
|
||||
if err != nil {
|
||||
logging.GetLogger().Fatal("Failed to initialize database", zap.String("error", err.Error()))
|
||||
}
|
||||
|
||||
// Run migrations using the new method
|
||||
if err := database.RunMigrations(); err != nil {
|
||||
logging.GetLogger().Fatal("Migration failed", zap.String("error", err.Error()))
|
||||
}
|
||||
|
||||
// Initialize backend package with database pool
|
||||
backend.InitBackend(database.Pool)
|
||||
|
||||
// Initialize token handler with database pool
|
||||
tokenHandler := NewTokenHandler(database.Pool)
|
||||
|
||||
// Create the server instance
|
||||
appServer := &Server{
|
||||
port: port,
|
||||
db: database,
|
||||
tokenHandler: tokenHandler,
|
||||
}
|
||||
|
||||
// Create the HTTP server
|
||||
appServer.httpServer = &http.Server{
|
||||
Addr: fmt.Sprintf(":%d", port),
|
||||
Handler: appServer.RegisterRoutes(),
|
||||
IdleTimeout: time.Minute,
|
||||
ReadTimeout: 10 * time.Second,
|
||||
WriteTimeout: 30 * time.Second,
|
||||
}
|
||||
|
||||
logger.Info("Starting server",
|
||||
zap.String("host", host),
|
||||
zap.String("dbPort", dbPort),
|
||||
@@ -61,23 +96,21 @@ func NewServer() *http.Server {
|
||||
zap.String("charactersPath", charactersPath),
|
||||
)
|
||||
|
||||
//conf.SetupDb()
|
||||
if host == "" || dbPort == "" || username == "" || password == "" || dbName == "" || musicPath == "" || charactersPath == "" {
|
||||
logging.GetLogger().Fatal("Invalid settings - missing required environment variables")
|
||||
}
|
||||
|
||||
db.Migrate_db(host, dbPort, username, password, dbName)
|
||||
|
||||
db.InitDB(host, dbPort, username, password, dbName)
|
||||
|
||||
// Declare Server config
|
||||
server := &http.Server{
|
||||
Addr: fmt.Sprintf(":%d", NewServer.port),
|
||||
Handler: NewServer.RegisterRoutes(),
|
||||
IdleTimeout: time.Minute,
|
||||
ReadTimeout: 10 * time.Second,
|
||||
WriteTimeout: 30 * time.Second,
|
||||
}
|
||||
|
||||
return server
|
||||
return appServer
|
||||
}
|
||||
|
||||
// HTTPServer returns the underlying http.Server for serving HTTP requests.
|
||||
func (s *Server) HTTPServer() *http.Server {
|
||||
return s.httpServer
|
||||
}
|
||||
|
||||
// DB returns the database instance for dependency injection.
|
||||
func (s *Server) DB() *db.Database {
|
||||
return s.db
|
||||
}
|
||||
|
||||
// NewServer creates a new HTTP server (deprecated, use NewServerInstance instead).
|
||||
// This function is kept for backward compatibility.
|
||||
func NewServer() *http.Server {
|
||||
return NewServerInstance().HTTPServer()
|
||||
}
|
||||
|
||||
@@ -75,8 +75,8 @@ func TestSyncPopulatesDatabase(t *testing.T) {
|
||||
db.TestClearDatabase(t)
|
||||
|
||||
// Before sync - should have no games
|
||||
repo := repository.New(db.Dbpool)
|
||||
gamesBefore, err := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
gamesBefore, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
beforeCount := len(gamesBefore)
|
||||
t.Logf("Games before sync: %d", beforeCount)
|
||||
@@ -92,7 +92,7 @@ func TestSyncPopulatesDatabase(t *testing.T) {
|
||||
}
|
||||
|
||||
// After sync - should have games
|
||||
gamesAfter, err := repo.FindAllGames(db.Ctx)
|
||||
gamesAfter, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
afterCount := len(gamesAfter)
|
||||
t.Logf("Games after sync: %d", afterCount)
|
||||
@@ -112,8 +112,8 @@ func TestSyncMakesDifference(t *testing.T) {
|
||||
db.TestClearDatabase(t)
|
||||
|
||||
// Before sync - should have no games
|
||||
repo := repository.New(db.Dbpool)
|
||||
gamesBefore, err := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
gamesBefore, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, 0, len(gamesBefore), "Should have no games before sync")
|
||||
|
||||
@@ -127,7 +127,7 @@ func TestSyncMakesDifference(t *testing.T) {
|
||||
}
|
||||
|
||||
// After sync - should have games
|
||||
gamesAfter, err := repo.FindAllGames(db.Ctx)
|
||||
gamesAfter, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, len(gamesAfter) > 0, "Should have games after sync")
|
||||
}
|
||||
@@ -199,8 +199,8 @@ func TestSyncGamesNewOnlyChanges(t *testing.T) {
|
||||
}
|
||||
|
||||
// Get initial count
|
||||
repo := repository.New(db.Dbpool)
|
||||
gamesBefore, _ := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
gamesBefore, _ := repo.FindAllGames(backend.BackendCtx())
|
||||
beforeCount := len(gamesBefore)
|
||||
|
||||
// Run incremental sync (should not change count if nothing changed)
|
||||
@@ -211,7 +211,7 @@ func TestSyncGamesNewOnlyChanges(t *testing.T) {
|
||||
time.Sleep(2 * time.Second)
|
||||
|
||||
// Count should be the same
|
||||
gamesAfter, _ := repo.FindAllGames(db.Ctx)
|
||||
gamesAfter, _ := repo.FindAllGames(backend.BackendCtx())
|
||||
afterCount := len(gamesAfter)
|
||||
|
||||
// Note: This might not be exactly equal due to timing, but should be close
|
||||
@@ -227,8 +227,8 @@ func TestResetGames(t *testing.T) {
|
||||
e := StartTestServer(t)
|
||||
|
||||
// First ensure we have data
|
||||
repo := repository.New(db.Dbpool)
|
||||
gamesBefore, _ := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
gamesBefore, _ := repo.FindAllGames(backend.BackendCtx())
|
||||
beforeCount := len(gamesBefore)
|
||||
|
||||
if beforeCount == 0 {
|
||||
@@ -238,7 +238,7 @@ func TestResetGames(t *testing.T) {
|
||||
t.Error("Sync did not complete within timeout")
|
||||
return
|
||||
}
|
||||
gamesBefore, _ = repo.FindAllGames(db.Ctx)
|
||||
gamesBefore, _ = repo.FindAllGames(backend.BackendCtx())
|
||||
beforeCount = len(gamesBefore)
|
||||
}
|
||||
|
||||
@@ -253,7 +253,7 @@ func TestResetGames(t *testing.T) {
|
||||
// Note: reset might take a moment to propagate
|
||||
time.Sleep(1 * time.Second)
|
||||
|
||||
gamesAfter, _ := repo.FindAllGames(db.Ctx)
|
||||
gamesAfter, _ := repo.FindAllGames(backend.BackendCtx())
|
||||
afterCount := len(gamesAfter)
|
||||
|
||||
t.Logf("Games after reset: %d", afterCount)
|
||||
@@ -281,8 +281,8 @@ func TestSyncGamesNewFull(t *testing.T) {
|
||||
}
|
||||
|
||||
// Verify database is populated
|
||||
repo := repository.New(db.Dbpool)
|
||||
games, err := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
games, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, len(games) > 0, "Database should be populated after full sync")
|
||||
t.Logf("Full sync populated %d games", len(games))
|
||||
|
||||
@@ -8,6 +8,9 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"music-server/internal/backend"
|
||||
"music-server/internal/db"
|
||||
|
||||
"github.com/labstack/echo/v5"
|
||||
)
|
||||
|
||||
@@ -45,8 +48,23 @@ func StartTestServer(t *testing.T) *echo.Echo {
|
||||
os.Setenv("LOG_JSON", "false")
|
||||
}
|
||||
|
||||
// Initialize database for tests
|
||||
db.TestSetupDB(t)
|
||||
|
||||
// Initialize backend with the global Dbpool
|
||||
// This ensures BackendRepo() and BackendCtx() are available
|
||||
if db.Dbpool != nil {
|
||||
backend.InitBackend(db.Dbpool)
|
||||
}
|
||||
|
||||
// Create a Server instance and get its routes
|
||||
s := &Server{}
|
||||
s := &Server{
|
||||
db: &db.Database{
|
||||
Pool: db.Dbpool,
|
||||
Ctx: db.Ctx,
|
||||
},
|
||||
tokenHandler: NewTokenHandler(db.Dbpool),
|
||||
}
|
||||
handler := s.RegisterRoutes()
|
||||
|
||||
// Wrap the http.Handler in an echo.Echo
|
||||
|
||||
@@ -7,10 +7,10 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"music-server/internal/db"
|
||||
"music-server/internal/db/repository"
|
||||
"music-server/internal/logging"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/labstack/echo/v5"
|
||||
"go.uber.org/zap"
|
||||
@@ -30,13 +30,13 @@ type TokenResponse struct {
|
||||
|
||||
// TokenHandler contains the database pool for token operations
|
||||
type TokenHandler struct {
|
||||
pool *repository.Queries
|
||||
pool *pgxpool.Pool
|
||||
}
|
||||
|
||||
// NewTokenHandler creates a new token handler with database pool
|
||||
func NewTokenHandler() *TokenHandler {
|
||||
func NewTokenHandler(pool *pgxpool.Pool) *TokenHandler {
|
||||
return &TokenHandler{
|
||||
pool: repository.New(db.Dbpool),
|
||||
pool: pool,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -84,7 +84,8 @@ func (h *TokenHandler) CreateTokenHandler(c *echo.Context) error {
|
||||
clientType := req.ClientType
|
||||
|
||||
// Store in database using sqlc-generated repository
|
||||
session, err := h.pool.CreateSession(c.Request().Context(), repository.CreateSessionParams{
|
||||
queries := repository.New(h.pool)
|
||||
session, err := queries.CreateSession(c.Request().Context(), repository.CreateSessionParams{
|
||||
Token: token,
|
||||
IpAddress: c.RealIP(),
|
||||
UserAgent: c.Request().UserAgent(),
|
||||
@@ -132,7 +133,8 @@ func (h *TokenHandler) DeleteTokenHandler(c *echo.Context) error {
|
||||
token := parts[1]
|
||||
|
||||
// Delete session using sqlc-generated repository
|
||||
err := h.pool.DeleteSession(c.Request().Context(), token)
|
||||
queries := repository.New(h.pool)
|
||||
err := queries.DeleteSession(c.Request().Context(), token)
|
||||
if err != nil {
|
||||
logging.GetLogger().Error("Failed to delete session", zap.String("error", err.Error()))
|
||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to invalidate token"})
|
||||
@@ -158,7 +160,8 @@ func (h *TokenHandler) CleanupExpiredSessionsHandler(c *echo.Context) error {
|
||||
// Verify token is valid first (using existing middleware)
|
||||
// The middleware will have already validated the token
|
||||
|
||||
err := h.pool.DeleteExpiredSessions(c.Request().Context())
|
||||
queries := repository.New(h.pool)
|
||||
err := queries.DeleteExpiredSessions(c.Request().Context())
|
||||
if err != nil {
|
||||
logging.GetLogger().Error("Failed to cleanup sessions", zap.String("error", err.Error()))
|
||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to cleanup sessions"})
|
||||
|
||||
@@ -15,8 +15,8 @@ import (
|
||||
|
||||
// ensureSyncRan ensures that sync has been run before testing music endpoints
|
||||
func ensureSyncRan(t *testing.T, e *echo.Echo) {
|
||||
repo := repository.New(db.Dbpool)
|
||||
games, err := repo.FindAllGames(db.Ctx)
|
||||
repo := repository.New(backend.BackendPool())
|
||||
games, err := repo.FindAllGames(backend.BackendCtx())
|
||||
assert.NoError(t, err)
|
||||
|
||||
if len(games) == 0 {
|
||||
|
||||
Reference in New Issue
Block a user