Skip to content

Commit 4545d08

Browse files
author
immortal71
committed
fix: wrap card dealing in Ecto.Multi transaction for atomicity
Replaces Repo.insert! loop with Ecto.Multi to ensure all card inserts and the game start update succeed or roll back together. Also adds minimum 3-player validation before starting a game. Addresses ASVS V2.3.3 - database transactions at the business logic level. Closes #2343
1 parent 668eb28 commit 4545d08

4 files changed

Lines changed: 573 additions & 219 deletions

File tree

copi.owasp.org/lib/copi_web/live/game_live/show.ex

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ defmodule CopiWeb.GameLive.Show do
3131
case Want.integer(params["round"], min: 1, max: current_round, default: current_round) do
3232
{:ok, requested_round} ->
3333
{:noreply, socket |> assign(:game, game) |> assign(:requested_round, requested_round)}
34+
3435
{:error, _reason} ->
3536
{:noreply, redirect(socket, to: "/error")}
3637
end
@@ -73,29 +74,33 @@ defmodule CopiWeb.GameLive.Show do
7374
players = game.players
7475
player_count = length(players)
7576

76-
# Deal cards to players in round-robin fashion
77-
all_cards
78-
|> Enum.with_index()
79-
|> Enum.each(fn {card, i} ->
80-
Copi.Repo.insert!(%DealtCard{
81-
card_id: card.id,
82-
player_id: Enum.at(players, rem(i, player_count)).id
83-
})
84-
end)
85-
86-
# Update game with start time and handle potential errors
87-
case Copi.Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)}) do
88-
{:ok, updated_game} ->
77+
# Build transaction with all card dealing operations and game update.
78+
# Ecto.Multi ensures atomicity: either all operations succeed or all are rolled back.
79+
multi =
80+
all_cards
81+
|> Enum.with_index()
82+
|> Enum.reduce(Ecto.Multi.new(), fn {card, i}, multi ->
83+
Ecto.Multi.insert(multi, {:deal_card, i}, %DealtCard{
84+
card_id: card.id,
85+
player_id: Enum.at(players, rem(i, player_count)).id
86+
})
87+
end)
88+
|> Ecto.Multi.run(:start_game, fn _repo, _changes ->
89+
Copi.Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})
90+
end)
91+
92+
# Execute transaction: all cards dealt and game started, or nothing happens.
93+
case Copi.Repo.transaction(multi) do
94+
{:ok, %{start_game: updated_game}} ->
8995
CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
9096
{:noreply, assign(socket, :game, updated_game)}
9197

92-
{:error, _changeset} ->
93-
# If update fails, reload game and show error
94-
{:ok, reloaded_game} = Game.find(game.id)
98+
{:error, _failed_operation, _failed_value, _changes_so_far} ->
99+
# Transaction rolled back, game state unchanged.
95100
{:noreply,
96101
socket
97-
|> put_flash(:error, "Failed to start game. Please try again.")
98-
|> assign(:game, reloaded_game)}
102+
|> put_flash(:error, "Failed to start game due to a system error. Please try again.")
103+
|> assign(:game, game)}
99104
end
100105
end
101106
end

0 commit comments

Comments
 (0)