Fix-review-feedback

backend/cmd/server/main.go

@@ -24,10 +24,11 @@ import (
 
 	"github.com/gin-contrib/cors"
 
+	"github.com/paularynty/transcendence/auth-service-go/internal/dependency"
 	"github.com/paularynty/transcendence/auth-service-go/internal/middleware"
 )
 
-func SetupRouter(logger *slog.Logger) *gin.Engine {
+func SetupRouter(dep *dependency.Dependency) *gin.Engine {
 	r := gin.New()
 
 	logConfig := sloggin.Config{
@@ -59,39 +60,40 @@ func SetupRouter(logger *slog.Logger) *gin.Engine {
 	r.Use(rateLimiter.RateLimit())
 
 	r.Use(middleware.PanicHandler())
-	r.Use(sloggin.NewWithConfig(logger, logConfig))
+	r.Use(sloggin.NewWithConfig(dep.Logger, logConfig))
 	r.Use(middleware.ErrorHandler())
 
 	return r
 }
 
+func initDependency() *dependency.Dependency {
+	logger := util.GetLogger(slog.LevelInfo)
+	cfg := config.LoadConfigFromEnv()
+	myDB := db.GetDB(cfg.DbAddress, logger)
+	redis := db.GetRedis(cfg.RedisURL, cfg, logger)
+
+	return dependency.NewDependency(cfg, myDB, redis, logger)
+}
+
 // @title Auth Service API
 // @version 1.0
-// @description Auth service for Transcendence
+// @description Auth service
 // @BasePath /api
 func main() {
 	// config
 	_ = godotenv.Load()
 
-	config.LoadConfig()
-
-	// logger
-	util.InitLogger(slog.LevelInfo)
+	// init dependency
+	dep := initDependency()
+	defer db.CloseDB(dep.DB, dep.Logger)
+	defer db.CloseRedis(dep.Redis, dep.Logger)
 
 	// validator
 	dto.InitValidator()
 
-	// database
-	db.ConnectDB(config.Cfg.DbAddress)
-	defer db.CloseDB()
-
-	db.ConnectRedis(config.Cfg.RedisURL)
-	defer db.CloseRedis()
-
 	// router
-	r := SetupRouter(util.Logger)
-	routers.UsersRouter(r.Group("/api/users"))
-	routers.DevRouter(r.Group("/api/dev"))
+	r := SetupRouter(dep)
+	routers.UsersRouter(r.Group("/api/users"), dep)
 
 	// Health check
 	r.GET("/api/ping", func(c *gin.Context) {
@@ -104,7 +106,7 @@ func main() {
 	r.GET("/api/docs/*any", ginSwagger.WrapHandler(swaggerfiles.Handler))
 
 	if err := r.Run(":3003"); err != nil {
-		util.Logger.Error("failed to start server", "err", err)
+		dep.Logger.Error("failed to start server", "err", err)
 		os.Exit(1)
 	}
 }

backend/internal/config/config.go

@@ -22,8 +22,6 @@ type Config struct {
 	UserTokenAbsoluteExpiry int
 }
 
-var Cfg *Config
-
 func getEnvStrOrDefault(key string, defaultValue string) string {
 	value := os.Getenv(key)
 
@@ -34,6 +32,16 @@ func getEnvStrOrDefault(key string, defaultValue string) string {
 	return value
 }
 
+func getEnvStrOrPanic(key string) string {
+	value := os.Getenv(key)
+
+	if value == "" {
+		panic("environment variable " + key + " is required but not set")
+	}
+
+	return value
+}
+
 func getEnvIntOrDefault(key string, defaultValue int) int {
 	strValue := os.Getenv(key)
 
@@ -45,15 +53,15 @@ func getEnvIntOrDefault(key string, defaultValue int) int {
 	return intValue
 }
 
-func LoadConfig() {
-	Cfg = &Config{
+func LoadConfigFromEnv() *Config {
+	return &Config{
 		GinMode:                 getEnvStrOrDefault("GIN_MODE", "debug"),
 		DbAddress:               getEnvStrOrDefault("DB_ADDRESS", "data/auth_service_db.sqlite"),
-		JwtSecret:               getEnvStrOrDefault("JWT_SECRET", "test-secret"),
+		JwtSecret:               getEnvStrOrPanic("JWT_SECRET"),
 		UserTokenExpiry:         getEnvIntOrDefault("USER_TOKEN_EXPIRY", 3600),
 		OauthStateTokenExpiry:   getEnvIntOrDefault("OAUTH_STATE_TOKEN_EXPIRY", 600),
-		GoogleClientId:          getEnvStrOrDefault("GOOGLE_CLIENT_ID", "test-google-client-id"),
-		GoogleClientSecret:      getEnvStrOrDefault("GOOGLE_CLIENT_SECRET", "test-google-client-secret"),
+		GoogleClientId:          getEnvStrOrPanic("GOOGLE_CLIENT_ID"),
+		GoogleClientSecret:      getEnvStrOrPanic("GOOGLE_CLIENT_SECRET"),
 		GoogleRedirectUri:       getEnvStrOrDefault("GOOGLE_REDIRECT_URI", "test-google-redirect-uri"),
 		FrontendUrl:             getEnvStrOrDefault("FRONTEND_URL", "http://localhost:5173"),
 		TwoFaUrlPrefix:          getEnvStrOrDefault("TWO_FA_URL_PREFIX", "otpauth://totp/Transcendence?secret="),

backend/internal/config/config_test.go

@@ -0,0 +1,95 @@
+package config
+
+import (
+	"testing"
+)
+
+func assertPanics(t *testing.T, fn func(), name string) {
+	t.Helper()
+	defer func() {
+		if r := recover(); r == nil {
+			t.Fatalf("expected panic for %s", name)
+		}
+	}()
+	fn()
+}
+
+func assertNotPanics(t *testing.T, fn func(), name string) {
+	t.Helper()
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("unexpected panic for %s: %v", name, r)
+		}
+	}()
+	fn()
+}
+
+func TestGetEnvStrOrDefault(t *testing.T) {
+	t.Setenv("TEST_STR", "")
+	if got := getEnvStrOrDefault("TEST_STR", "fallback"); got != "fallback" {
+		t.Fatalf("expected default value, got %q", got)
+	}
+
+	t.Setenv("TEST_STR", "value")
+	if got := getEnvStrOrDefault("TEST_STR", "fallback"); got != "value" {
+		t.Fatalf("expected env value, got %q", got)
+	}
+}
+
+func TestGetEnvStrOrPanic(t *testing.T) {
+	t.Setenv("TEST_PANIC", "")
+	assertPanics(t, func() {
+		_ = getEnvStrOrPanic("TEST_PANIC")
+	}, "empty env")
+
+	t.Setenv("TEST_PANIC", "value")
+	assertNotPanics(t, func() {
+		if got := getEnvStrOrPanic("TEST_PANIC"); got != "value" {
+			t.Fatalf("expected env value, got %q", got)
+		}
+	}, "set env")
+}
+
+func TestGetEnvIntOrDefault(t *testing.T) {
+	t.Setenv("TEST_INT", "")
+	if got := getEnvIntOrDefault("TEST_INT", 7); got != 7 {
+		t.Fatalf("expected default value, got %d", got)
+	}
+
+	t.Setenv("TEST_INT", "42")
+	if got := getEnvIntOrDefault("TEST_INT", 7); got != 42 {
+		t.Fatalf("expected env value, got %d", got)
+	}
+
+	t.Setenv("TEST_INT", "not-an-int")
+	if got := getEnvIntOrDefault("TEST_INT", 7); got != 7 {
+		t.Fatalf("expected default value for invalid int, got %d", got)
+	}
+}
+
+func TestLoadConfigFromEnv_PanicsOnMissingRequired(t *testing.T) {
+	t.Setenv("JWT_SECRET", "jwt")
+	t.Setenv("GOOGLE_CLIENT_ID", "client")
+	t.Setenv("GOOGLE_CLIENT_SECRET", "secret")
+
+	assertNotPanics(t, func() {
+		_ = LoadConfigFromEnv()
+	}, "all required set")
+
+	t.Setenv("JWT_SECRET", "")
+	assertPanics(t, func() {
+		_ = LoadConfigFromEnv()
+	}, "JWT_SECRET unset")
+
+	t.Setenv("JWT_SECRET", "jwt")
+	t.Setenv("GOOGLE_CLIENT_ID", "")
+	assertPanics(t, func() {
+		_ = LoadConfigFromEnv()
+	}, "GOOGLE_CLIENT_ID unset")
+
+	t.Setenv("GOOGLE_CLIENT_ID", "client")
+	t.Setenv("GOOGLE_CLIENT_SECRET", "")
+	assertPanics(t, func() {
+		_ = LoadConfigFromEnv()
+	}, "GOOGLE_CLIENT_SECRET unset")
+}

backend/internal/db/db.go

@@ -2,57 +2,55 @@ package db
 
 import (
 	"context"
+	"log/slog"
 
 	"gorm.io/driver/sqlite"
 	"gorm.io/gorm"
-
-	"github.com/paularynty/transcendence/auth-service-go/internal/util"
 )
 
-var DB *gorm.DB
-
-func ConnectDB(dbName string) {
+func GetDB(dbName string, logger *slog.Logger) *gorm.DB {
 	var err error
-	DB, err = gorm.Open(sqlite.Open(dbName), &gorm.Config{TranslateError: true})
+	db, err := gorm.Open(sqlite.Open(dbName), &gorm.Config{TranslateError: true})
 
 	if err != nil {
 		panic("failed to connect to db: " + dbName)
 	}
 
-	DB.Exec("PRAGMA foreign_keys = ON")
+	db.Exec("PRAGMA foreign_keys = ON")
 
 	for _, model := range []any{
 		&User{},
 		&Friend{},
 		&Token{},
 		&HeartBeat{},
 	} {
-		if err := DB.AutoMigrate(model); err != nil {
+		if err := db.AutoMigrate(model); err != nil {
 			panic("failed to migrate model: " + err.Error())
 		}
 	}
 
-	util.Logger.Info("connected to db")
+	logger.Info("connected to db")
+
+	return db
 }
 
-func CloseDB() {
-	sqlDB, err := DB.DB()
+func CloseDB(db *gorm.DB, logger *slog.Logger) {
+	sqlDB, err := db.DB()
 	if err != nil {
-		util.Logger.Error("failed to get db instance", "err", err)
+		logger.Error("failed to get db instance", "err", err)
 		return
 	}
 
 	if err := sqlDB.Close(); err != nil {
-		util.Logger.Error("failed to close db", "err", err)
+		logger.Error("failed to close db", "err", err)
 		return
 	}
 
-	util.Logger.Info("db connection closed")
+	logger.Info("db connection closed")
 }
 
-func ResetDB() {
-	util.Logger.Warn("resetting db...")
-
+func ResetDB(db *gorm.DB, logger *slog.Logger) {
+	logger.Warn("resetting db...")
 	ctx := context.Background()
 	tables := []string{
 		"heart_beats",
@@ -62,11 +60,11 @@ func ResetDB() {
 	}
 
 	for _, table := range tables {
-		err := gorm.G[any](DB).Exec(ctx, "DELETE FROM "+table)
+		err := gorm.G[any](db).Exec(ctx, "DELETE FROM "+table)
 		if err != nil {
-			util.Logger.Error("failed to reset table", table, err.Error())
+			logger.Error("failed to reset table", table, err.Error())
 		}
 	}
 
-	util.Logger.Info("db is reset")
+	logger.Info("db is reset")
 }

backend/internal/db/redis.go

@@ -2,18 +2,16 @@ package db
 
 import (
 	"context"
+	"log/slog"
 
 	"github.com/paularynty/transcendence/auth-service-go/internal/config"
-	"github.com/paularynty/transcendence/auth-service-go/internal/util"
 	"github.com/redis/go-redis/v9"
 )
 
-var Redis *redis.Client
-
-func ConnectRedis(redisURL string) {
-	if !config.Cfg.IsRedisEnabled {
-		util.Logger.Info("redis is disabled by config")
-		return
+func GetRedis(redisURL string, cfg *config.Config, logger *slog.Logger) *redis.Client {
+	if !cfg.IsRedisEnabled {
+		logger.Info("redis is disabled by config")
+		return nil
 	}
 
 	opt, err := redis.ParseURL(redisURL)
@@ -31,20 +29,20 @@ func ConnectRedis(redisURL string) {
 		panic("failed to connect to redis: " + err.Error())
 	}
 
-	Redis = client
+	logger.Info("connected to redis")
 
-	util.Logger.Info("connected to redis")
+	return client
 }
 
-func CloseRedis() {
-	if Redis == nil {
+func CloseRedis(client *redis.Client, logger *slog.Logger) {
+	if client == nil {
 		return
 	}
 
-	err := Redis.Close()
+	err := client.Close()
 	if err != nil {
-		util.Logger.Error("failed to close redis connection", "error", err)
+		logger.Error("failed to close redis connection", "error", err)
 	} else {
-		util.Logger.Info("redis connection closed")
+		logger.Info("redis connection closed")
 	}
 }

backend/internal/dependency/dependency.go

@@ -0,0 +1,24 @@
+package dependency
+
+import (
+	"github.com/paularynty/transcendence/auth-service-go/internal/config"
+	"github.com/redis/go-redis/v9"
+	"gorm.io/gorm"
+	"log/slog"
+)
+
+type Dependency struct {
+	Cfg    *config.Config
+	DB     *gorm.DB
+	Redis  *redis.Client
+	Logger *slog.Logger
+}
+
+func NewDependency(cfg *config.Config, db *gorm.DB, redis *redis.Client, logger *slog.Logger) *Dependency {
+	return &Dependency{
+		Cfg:    cfg,
+		DB:     db,
+		Redis:  redis,
+		Logger: logger,
+	}
+}