diff --git a/bin/evening_detective_server b/bin/evening_detective_server index 4832688..f5aac4b 100755 Binary files a/bin/evening_detective_server and b/bin/evening_detective_server differ diff --git a/internal/app/gameplay_errors.go b/internal/app/gameplay_errors.go index 9bf6156..99bed0f 100644 --- a/internal/app/gameplay_errors.go +++ b/internal/app/gameplay_errors.go @@ -16,5 +16,8 @@ func gameplayErrorStatus(err error, internalMessage string) error { if errors.Is(err, gameplay_service.ErrGameNotFound) { return status.Error(codes.NotFound, gameplay_service.ErrGameNotFound.Error()) } + if errors.Is(err, gameplay_service.ErrMovesNotAllowed) { + return status.Error(codes.FailedPrecondition, gameplay_service.ErrMovesNotAllowed.Error()) + } return status.Error(codes.Internal, internalMessage) } diff --git a/internal/app/gameplay_errors_test.go b/internal/app/gameplay_errors_test.go new file mode 100644 index 0000000..85da8b2 --- /dev/null +++ b/internal/app/gameplay_errors_test.go @@ -0,0 +1,22 @@ +package app + +import ( + "errors" + "fmt" + "testing" + + "evening_detective_server/internal/services/gameplay_service" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestGameplayMoveErrorIsSafe(t *testing.T) { + mapped := gameplayErrorStatus(fmt.Errorf("private detail: %w", gameplay_service.ErrMovesNotAllowed), "не удалось сделать ход") + if status.Code(mapped) != codes.FailedPrecondition || status.Convert(mapped).Message() != gameplay_service.ErrMovesNotAllowed.Error() { + t.Fatalf("unexpected move error: %v", mapped) + } + mapped = gameplayErrorStatus(errors.New("private database detail"), "не удалось сделать ход") + if status.Code(mapped) != codes.Internal || status.Convert(mapped).Message() != "не удалось сделать ход" { + t.Fatalf("unsafe internal error: %v", mapped) + } +} diff --git a/internal/repos/tickets_repo/accept_test.go b/internal/repos/tickets_repo/accept_test.go index 54c218f..3addd19 100644 --- a/internal/repos/tickets_repo/accept_test.go +++ b/internal/repos/tickets_repo/accept_test.go @@ -36,6 +36,8 @@ func TestAcceptIsAtomic(t *testing.T) { t.Fatal(err) } config.ConnConfig.RuntimeParams["search_path"] = schema + config.ConnConfig.RuntimeParams["application_name"] = schema + config.MaxConns = 10 pool, err := pgxpool.NewWithConfig(ctx, config) if err != nil { t.Fatal(err) @@ -67,6 +69,15 @@ func TestAcceptIsAtomic(t *testing.T) { t.Fatal(err) } + // Hold the row until every caller has read the active ticket and reached UPDATE. + lock, err := pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = lock.Rollback(ctx) }() + if _, err := lock.Exec(ctx, `SELECT id FROM tickets WHERE token_hash=$1 FOR UPDATE`, hash[:]); err != nil { + t.Fatal(err) + } repo := New(pool) results := make(chan error, 8) for i := 0; i < 8; i++ { @@ -76,6 +87,24 @@ func TestAcceptIsAtomic(t *testing.T) { }(i%2 + 1) } + deadline := time.Now().Add(5 * time.Second) + for { + var waiting int + if err := admin.QueryRow(ctx, `SELECT count(*) FROM pg_stat_activity WHERE application_name=$1 AND wait_event_type='Lock'`, schema).Scan(&waiting); err != nil { + t.Fatal(err) + } + if waiting == 8 { + break + } + if time.Now().After(deadline) { + t.Fatalf("only %d callers reached the concurrent UPDATE", waiting) + } + time.Sleep(10 * time.Millisecond) + } + if err := lock.Commit(ctx); err != nil { + t.Fatal(err) + } + successes := 0 for range 8 { if err := <-results; err == nil { diff --git a/internal/repos/tickets_repo/repo.go b/internal/repos/tickets_repo/repo.go index cabde6a..78bf600 100644 --- a/internal/repos/tickets_repo/repo.go +++ b/internal/repos/tickets_repo/repo.go @@ -108,7 +108,11 @@ func (r *Repo) Accept(ctx context.Context, tokenHash []byte, userID int) (*Info, return nil, err } if tag.RowsAffected() == 0 { - if status == "used" { + current, err := r.GetByHash(ctx, tokenHash) + if err != nil { + return nil, err + } + if current.Status == "used" { return nil, ErrTicketUsed } return nil, ErrGameFinished diff --git a/internal/services/game_service/lifecycle_test.go b/internal/services/game_service/lifecycle_test.go index d9890ad..f0a5a0e 100644 --- a/internal/services/game_service/lifecycle_test.go +++ b/internal/services/game_service/lifecycle_test.go @@ -107,6 +107,27 @@ func TestServicesLifecyclePostgres(t *testing.T) { if !strings.HasPrefix(string(pdf), "%PDF-") { t.Fatal("invalid QR PDF") } + for _, format := range []string{"online", "offline"} { + for _, state := range []string{"draft", "pause", "finish"} { + _, err := pool.Exec(ctx, `UPDATE games SET format=$1, status=$2 WHERE id=$3`, format, state, id) + check(err) + for _, viaMCP := range []bool{false, true} { + err := play.AddTeamAction(ctx, teamID, registered.Password, "p", viaMCP) + if !errors.Is(err, gameplay_service.ErrMovesNotAllowed) { + t.Fatalf("%s/%s MCP=%t accepted a move: %v", format, state, viaMCP, err) + } + } + } + } + _, err = pool.Exec(ctx, `UPDATE games SET format='online', status='draft' WHERE id=$1`, id) + check(err) + registered, err = teams.GetTeamByID(ctx, teamID) + check(err) + counts, err := actions.GetActionsCountByTeamIDs(ctx, []int{teamID}) + check(err) + if registered.ParticipationStatus != "pending" || counts[teamID] != 0 { + t.Fatal("rejected moves changed participation or progress") + } check(game.StartGame(ctx, id)) check(play.AddTeamAction(ctx, teamID, registered.Password, "p", true)) registered, err = teams.GetTeamByID(ctx, teamID) @@ -142,9 +163,15 @@ func TestServicesLifecyclePostgres(t *testing.T) { } check(answers.SubmitTeamAnswers(ctx, teamID, registered.Password, []answer_service.Answer{{QuestionCode: "q", Text: "Butler"}})) check(game.PauseGame(ctx, id)) + if err := play.AddTeamAction(ctx, teamID, registered.Password, "p", false); !errors.Is(err, gameplay_service.ErrMovesNotAllowed) { + t.Fatalf("paused move: %v", err) + } check(answers.SubmitTeamAnswers(ctx, teamID, registered.Password, []answer_service.Answer{{QuestionCode: "q", Text: "Butler"}})) check(game.PlayGame(ctx, id)) check(game.FinishGame(ctx, id)) + if err := play.AddTeamAction(ctx, teamID, registered.Password, "p", true); !errors.Is(err, gameplay_service.ErrMovesNotAllowed) { + t.Fatalf("finished MCP move: %v", err) + } if err := answers.SubmitTeamAnswers(ctx, teamID, registered.Password, nil); !errors.Is(err, answer_service.ErrGameFinished) { t.Fatalf("finished answers: %v", err) } diff --git a/internal/services/gameplay_service/service.go b/internal/services/gameplay_service/service.go index 9a326f0..986ac18 100644 --- a/internal/services/gameplay_service/service.go +++ b/internal/services/gameplay_service/service.go @@ -19,6 +19,8 @@ var ( ErrTeamNotFound = errors.New("команда не найдена") // ErrGameNotFound indicates that a game was not found. ErrGameNotFound = errors.New("игра не найдена") + // ErrMovesNotAllowed indicates that the game is not running. + ErrMovesNotAllowed = errors.New("ходы доступны только во время игры") ) // GameplayService handles team gameplay operations. @@ -110,6 +112,14 @@ func (s *GameplayService) AddTeamAction(ctx context.Context, teamID int, passwor return fmt.Errorf("%w: %v", ErrTeamNotFound, teams_repo.ErrTeamNotFound) } + game, err := s.gamesRepo.GetGameByID(ctx, team.GameID) + if err != nil { + return mapGameplayError(err) + } + if game.Status != "run" { + return ErrMovesNotAllowed + } + _, err = s.actionsRepo.AddAction(ctx, teamID, actionCode, viaMCP) if err != nil { return mapGameplayError(err) @@ -120,11 +130,6 @@ func (s *GameplayService) AddTeamAction(ctx context.Context, teamID int, passwor } } - game, err := s.gamesRepo.GetGameByID(ctx, team.GameID) - if err != nil { - return mapGameplayError(err) - } - actionDetails, err := s.actionsRepo.GetActionsByTeamID(ctx, teamID) if err != nil { return mapGameplayError(err)