From 5ed369a6c70707543fd5ec9a13c79851fdfc5d6c Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 15 Feb 2026 22:13:50 +0900 Subject: refactor(backend): introduce DI interfaces for testability Replace concrete *db.Queries and *pgxpool.Pool dependencies with db.Querier and db.TxManager interfaces across all handlers, game hub, and auth. This enables unit testing with mocks. - Enable sqlc emit_interface to generate Querier interface - Add TxManager abstraction to encapsulate transactions - Convert auth package-level functions to Authenticator struct - Add TaskQueueInterface/TaskWorkerInterface for game.Hub - Add initial unit tests for game logic and API handlers Co-Authored-By: Claude Opus 4.6 --- backend/account/icon.go | 2 +- backend/admin/handler.go | 68 +++++++---------- backend/api/auth_middleware.go | 2 +- backend/api/handler.go | 57 +++++++-------- backend/api/handler_test.go | 130 +++++++++++++++++++++++++++++++++ backend/api/handler_wrapper.go | 7 +- backend/auth/auth.go | 76 ++++++++----------- backend/db/querier.go | 63 ++++++++++++++++ backend/db/txmanager.go | 40 ++++++++++ backend/game/hub.go | 70 +++++++++--------- backend/game/hub_test.go | 100 +++++++++++++++++++++++++ backend/gen/api/handler_wrapper_gen.go | 7 +- backend/gen/sqlc.yaml | 1 + backend/justfile | 3 +- backend/main.go | 9 ++- 15 files changed, 464 insertions(+), 171 deletions(-) create mode 100644 backend/api/handler_test.go create mode 100644 backend/db/querier.go create mode 100644 backend/db/txmanager.go create mode 100644 backend/game/hub_test.go (limited to 'backend') diff --git a/backend/account/icon.go b/backend/account/icon.go index 8a7ecd0..a4b7c5f 100644 --- a/backend/account/icon.go +++ b/backend/account/icon.go @@ -17,7 +17,7 @@ import ( func FetchIcon( ctx context.Context, - q *db.Queries, + q db.Querier, userID int, ) error { ctx, cancel := context.WithTimeout(ctx, 15*time.Second) diff --git a/backend/admin/handler.go b/backend/admin/handler.go index e8a7921..6ca0a3f 100644 --- a/backend/admin/handler.go +++ b/backend/admin/handler.go @@ -10,7 +10,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" "github.com/labstack/echo/v4" "albatross-2026-backend/account" @@ -22,13 +21,13 @@ import ( var jst = time.FixedZone("Asia/Tokyo", 9*60*60) type Handler struct { - q *db.Queries - pool *pgxpool.Pool + q db.Querier + txm db.TxManager conf *config.Config } -func NewHandler(q *db.Queries, pool *pgxpool.Pool, conf *config.Config) *Handler { - return &Handler{q: q, pool: pool, conf: conf} +func NewHandler(q db.Querier, txm db.TxManager, conf *config.Config) *Handler { + return &Handler{q: q, txm: txm, conf: conf} } func (h *Handler) newAdminMiddleware() echo.MiddlewareFunc { @@ -506,48 +505,35 @@ func (h *Handler) postGameEdit(c echo.Context) error { } ctx := c.Request().Context() - tx, err := h.pool.Begin(ctx) - if err != nil { - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - defer func() { - if err := tx.Rollback(ctx); err != nil && err != pgx.ErrTxClosed { - slog.Error("failed to rollback transaction", "error", err) + err = h.txm.RunInTx(ctx, func(qtx db.Querier) error { + if err := qtx.UpdateGame(ctx, db.UpdateGameParams{ + GameID: int32(gameID), + GameType: gameType, + IsPublic: isPublic, + DisplayName: displayName, + DurationSeconds: int32(durationSeconds), + StartedAt: changedStartedAt, + ProblemID: int32(problemID), + }); err != nil { + return err + } + if err := qtx.RemoveAllMainPlayers(ctx, int32(gameID)); err != nil { + return err + } + for _, userID := range mainPlayers { + if err := qtx.AddMainPlayer(ctx, db.AddMainPlayerParams{ + GameID: int32(gameID), + UserID: int32(userID), + }); err != nil { + return err + } } - }() - - qtx := h.q.WithTx(tx) - err = qtx.UpdateGame(ctx, db.UpdateGameParams{ - GameID: int32(gameID), - GameType: gameType, - IsPublic: isPublic, - DisplayName: displayName, - DurationSeconds: int32(durationSeconds), - StartedAt: changedStartedAt, - ProblemID: int32(problemID), + return nil }) if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) } - err = qtx.RemoveAllMainPlayers(ctx, int32(gameID)) - if err != nil { - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - for _, userID := range mainPlayers { - err = qtx.AddMainPlayer(ctx, db.AddMainPlayerParams{ - GameID: int32(gameID), - UserID: int32(userID), - }) - if err != nil { - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - } - - if err := tx.Commit(ctx); err != nil { - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - return c.Redirect(http.StatusSeeOther, h.conf.BasePath+"admin/games") } diff --git a/backend/api/auth_middleware.go b/backend/api/auth_middleware.go index d721f1d..0b0dfc8 100644 --- a/backend/api/auth_middleware.go +++ b/backend/api/auth_middleware.go @@ -12,7 +12,7 @@ import ( type sessionIDContextKey struct{} type userContextKey struct{} -func SessionCookieMiddleware(q *db.Queries) echo.MiddlewareFunc { +func SessionCookieMiddleware(q db.Querier) echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { cookie, err := c.Cookie("albatross_session") diff --git a/backend/api/handler.go b/backend/api/handler.go index 3fe7e3c..7efacf3 100644 --- a/backend/api/handler.go +++ b/backend/api/handler.go @@ -11,7 +11,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" "github.com/labstack/echo/v4" "github.com/oapi-codegen/nullable" @@ -20,10 +19,15 @@ import ( "albatross-2026-backend/db" ) +type AuthenticatorInterface interface { + Login(ctx context.Context, username, password string) (int, error) +} + type Handler struct { - q *db.Queries - pool *pgxpool.Pool + q db.Querier + txm db.TxManager hub GameHubInterface + auth AuthenticatorInterface conf *config.Config } @@ -47,7 +51,7 @@ func (r postLoginCookieResponse) VisitPostLoginResponse(w http.ResponseWriter) e func (h *Handler) PostLogin(ctx context.Context, request PostLoginRequestObject) (PostLoginResponseObject, error) { username := request.Body.Username password := request.Body.Password - userID, err := auth.Login(ctx, h.q, h.pool, username, password) + userID, err := h.auth.Login(ctx, username, password) if err != nil { slog.Error("login failed", "error", err) var msg string @@ -419,40 +423,29 @@ func (h *Handler) PostGamePlaySubmit(ctx context.Context, request PostGamePlaySu }, nil } - tx, err := h.pool.Begin(ctx) - if err != nil { - return nil, echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - defer func() { - if err := tx.Rollback(ctx); err != nil && err != pgx.ErrTxClosed { - slog.Error("failed to rollback transaction", "error", err) + var submissionID int32 + err = h.txm.RunInTx(ctx, func(qtx db.Querier) error { + if err := qtx.UpdateCodeAndStatus(ctx, db.UpdateCodeAndStatusParams{ + GameID: int32(gameID), + UserID: user.UserID, + Code: code, + Status: "running", + }); err != nil { + return err } - }() - - qtx := h.q.WithTx(tx) - err = qtx.UpdateCodeAndStatus(ctx, db.UpdateCodeAndStatusParams{ - GameID: int32(gameID), - UserID: user.UserID, - Code: code, - Status: "running", - }) - if err != nil { - return nil, echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - submissionID, err := qtx.CreateSubmission(ctx, db.CreateSubmissionParams{ - GameID: int32(gameID), - UserID: user.UserID, - Code: code, - CodeSize: int32(codeSize), + var err error + submissionID, err = qtx.CreateSubmission(ctx, db.CreateSubmissionParams{ + GameID: int32(gameID), + UserID: user.UserID, + Code: code, + CodeSize: int32(codeSize), + }) + return err }) if err != nil { return nil, echo.NewHTTPError(http.StatusInternalServerError, err.Error()) } - if err := tx.Commit(ctx); err != nil { - return nil, echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - err = h.hub.EnqueueTestTasks(ctx, int(submissionID), gameID, int(user.UserID), language, code) if err != nil { return nil, echo.NewHTTPError(http.StatusInternalServerError, err.Error()) diff --git a/backend/api/handler_test.go b/backend/api/handler_test.go new file mode 100644 index 0000000..47ad92c --- /dev/null +++ b/backend/api/handler_test.go @@ -0,0 +1,130 @@ +package api + +import ( + "context" + "errors" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "albatross-2026-backend/config" + "albatross-2026-backend/db" +) + +// mockQuerier implements db.Querier for testing. +type mockQuerier struct { + db.Querier + getGameByIDFunc func(ctx context.Context, gameID int32) (db.GetGameByIDRow, error) +} + +func (m *mockQuerier) GetGameByID(ctx context.Context, gameID int32) (db.GetGameByIDRow, error) { + if m.getGameByIDFunc != nil { + return m.getGameByIDFunc(ctx, gameID) + } + return db.GetGameByIDRow{}, pgx.ErrNoRows +} + +// mockTxManager implements db.TxManager for testing. +type mockTxManager struct{} + +func (m *mockTxManager) RunInTx(ctx context.Context, fn func(q db.Querier) error) error { + return fn(&mockQuerier{}) +} + +// mockGameHub implements GameHubInterface for testing. +type mockGameHub struct { + calcCodeSizeResult int + enqueueErr error +} + +func (m *mockGameHub) CalcCodeSize(_ string, _ string) int { + return m.calcCodeSizeResult +} + +func (m *mockGameHub) EnqueueTestTasks(_ context.Context, _, _, _ int, _, _ string) error { + return m.enqueueErr +} + +// mockAuthenticator implements AuthenticatorInterface for testing. +type mockAuthenticator struct { + loginResult int + loginErr error +} + +func (m *mockAuthenticator) Login(_ context.Context, _, _ string) (int, error) { + return m.loginResult, m.loginErr +} + +func TestPostGamePlaySubmit_GameNotFound(t *testing.T) { + h := Handler{ + q: &mockQuerier{}, + txm: &mockTxManager{}, + hub: &mockGameHub{}, + auth: &mockAuthenticator{}, + conf: &config.Config{}, + } + user := &db.User{UserID: 1} + resp, err := h.PostGamePlaySubmit(context.Background(), PostGamePlaySubmitRequestObject{ + GameID: 999, + Body: &PostGamePlaySubmitJSONRequestBody{Code: "test"}, + }, user) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := resp.(PostGamePlaySubmit404JSONResponse); !ok { + t.Errorf("expected 404 response, got %T", resp) + } +} + +func TestPostGamePlaySubmit_GameNotRunning(t *testing.T) { + h := Handler{ + q: &mockQuerier{ + getGameByIDFunc: func(_ context.Context, _ int32) (db.GetGameByIDRow, error) { + return db.GetGameByIDRow{ + GameID: 1, + Language: "php", + StartedAt: pgtype.Timestamp{ + Valid: false, + }, + }, nil + }, + }, + txm: &mockTxManager{}, + hub: &mockGameHub{calcCodeSizeResult: 10}, + auth: &mockAuthenticator{}, + conf: &config.Config{}, + } + user := &db.User{UserID: 1} + resp, err := h.PostGamePlaySubmit(context.Background(), PostGamePlaySubmitRequestObject{ + GameID: 1, + Body: &PostGamePlaySubmitJSONRequestBody{Code: "", + language: "php", + want: 6, // "echo1;" after stripping whitespace, "" + }, + { + name: "php with whitespace", + code: "", + language: "php", + want: 6, + }, + { + name: "non-php language", + code: "print(1)", + language: "swift", + want: 8, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := hub.CalcCodeSize(tt.code, tt.language) + if got != tt.want { + t.Errorf("CalcCodeSize(%q, %q) = %d, want %d", tt.code, tt.language, got, tt.want) + } + }) + } +} + +func TestIsTestcaseResultCorrect(t *testing.T) { + tests := []struct { + name string + expected string + actual string + want bool + }{ + { + name: "exact match", + expected: "hello", + actual: "hello", + want: true, + }, + { + name: "trailing newline ignored", + expected: "hello\n", + actual: "hello", + want: true, + }, + { + name: "CRLF normalized", + expected: "hello\r\n", + actual: "hello\n", + want: true, + }, + { + name: "mismatch", + expected: "hello", + actual: "world", + want: false, + }, + { + name: "multiline match", + expected: "line1\nline2", + actual: "line1\nline2\n", + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isTestcaseResultCorrect(tt.expected, tt.actual) + if got != tt.want { + t.Errorf("isTestcaseResultCorrect(%q, %q) = %v, want %v", tt.expected, tt.actual, got, tt.want) + } + }) + } +} diff --git a/backend/gen/api/handler_wrapper_gen.go b/backend/gen/api/handler_wrapper_gen.go index 1a948a7..e3e56c1 100644 --- a/backend/gen/api/handler_wrapper_gen.go +++ b/backend/gen/api/handler_wrapper_gen.go @@ -105,8 +105,6 @@ package api import ( "context" - "github.com/jackc/pgx/v5/pgxpool" - "albatross-2026-backend/config" "albatross-2026-backend/db" ) @@ -117,12 +115,13 @@ type HandlerWrapper struct { impl Handler } -func NewHandler(queries *db.Queries, pool *pgxpool.Pool, hub GameHubInterface, conf *config.Config) *HandlerWrapper { +func NewHandler(queries db.Querier, txm db.TxManager, hub GameHubInterface, auth AuthenticatorInterface, conf *config.Config) *HandlerWrapper { return &HandlerWrapper{ impl: Handler{ q: queries, - pool: pool, + txm: txm, hub: hub, + auth: auth, conf: conf, }, } diff --git a/backend/gen/sqlc.yaml b/backend/gen/sqlc.yaml index c56e44b..98dec89 100644 --- a/backend/gen/sqlc.yaml +++ b/backend/gen/sqlc.yaml @@ -9,3 +9,4 @@ sql: out: "../db" sql_package: "pgx/v5" emit_pointers_for_null_types: true + emit_interface: true diff --git a/backend/justfile b/backend/justfile index a74047f..1897cab 100644 --- a/backend/justfile +++ b/backend/justfile @@ -2,7 +2,8 @@ check: go build -o /dev/null ./... go tool golangci-lint run -ci: check +test: + go test ./... gen: go generate ./... diff --git a/backend/main.go b/backend/main.go index 01ed784..f936cf4 100644 --- a/backend/main.go +++ b/backend/main.go @@ -16,6 +16,7 @@ import ( "albatross-2026-backend/admin" "albatross-2026-backend/api" + "albatross-2026-backend/auth" "albatross-2026-backend/config" "albatross-2026-backend/db" "albatross-2026-backend/game" @@ -61,6 +62,8 @@ func main() { defer connPool.Close() queries := db.New(connPool) + txm := db.NewTxManager(connPool, queries) + authenticator := auth.NewAuthenticator(queries, txm) e := echo.New() e.Renderer = admin.NewRenderer() @@ -91,7 +94,7 @@ func main() { taskQueue := taskqueue.NewQueue("task-db:6379") workerServer := taskqueue.NewWorkerServer("task-db:6379") - gameHub := game.NewGameHub(queries, connPool, taskQueue, workerServer) + gameHub := game.NewGameHub(queries, txm, taskQueue, workerServer) loginRL := ratelimit.NewIPRateLimiter(rate.Every(time.Minute/5), 5) @@ -99,10 +102,10 @@ func main() { apiGroup.Use(ratelimit.LoginRateLimitMiddleware(loginRL)) apiGroup.Use(api.SessionCookieMiddleware(queries)) apiGroup.Use(oapimiddleware.OapiRequestValidator(openAPISpec)) - apiHandler := api.NewHandler(queries, connPool, gameHub, conf) + apiHandler := api.NewHandler(queries, txm, gameHub, authenticator, conf) api.RegisterHandlers(apiGroup, api.NewStrictHandler(apiHandler, nil)) - adminHandler := admin.NewHandler(queries, connPool, conf) + adminHandler := admin.NewHandler(queries, txm, conf) adminGroup := e.Group(conf.BasePath + "admin") adminGroup.Use(api.SessionCookieMiddleware(queries)) adminHandler.RegisterHandlers(adminGroup) -- cgit v1.3.1