Fix race condition in voting system

[immortal71]

Description

Fixes #2288

This PR addresses a critical race condition in the voting system that could lead to duplicate votes and data corruption when multiple requests are processed concurrently.

Problem

The current voting implementation has a check-then-act pattern that is vulnerable to race conditions:

1. Check if vote exists in memory/preloaded data
2. Insert or delete based on check result

When concurrent requests are processed, both can pass the check simultaneously and create duplicate votes, violating data integrity.

Solution

Database Layer

• Added unique index on votes(player_id, dealt_card_id) to enforce constraint at database level
• Added unique index on continue_votes(player_id, game_id) (already existed)

Application Layer

• Updated toggle_vote to use Repo.get_by with on_conflict: :nothing for atomic insert operations
• Updated toggle_continue_vote to query database instead of relying on preloaded data
• Replaced unsafe String.to_integer with safe Integer.parse to prevent crashes on invalid input
• Replaced IO.puts with proper Logger.debug/Logger.warning calls

Testing

• Added race condition test for duplicate vote prevention
• Added race condition test for duplicate continue vote prevention
• Both tests verify that database constraints prevent duplicates

Changes

• priv/repo/migrations/20260218120000_add_unique_constraint_to_votes.exs - New migration
• lib/copi_web/live/player_live/show.ex - Fixed race conditions in event handlers
• test/copi_web/live/player_live_test.exs - Added race condition tests

Testing

All tests pass with proper database constraints ensuring data integrity even under concurrent load.

[Copilot]

Pull request overview

This PR addresses a critical race condition in the voting system that could lead to duplicate votes when multiple concurrent requests are processed. The fix implements defense-in-depth by adding database-level unique constraints and updating the application layer to use atomic operations with conflict handling.

Changes:

• Added unique index on votes(player_id, dealt_card_id) to enforce database-level constraint
• Refactored vote toggle handlers to use Repo.get_by with on_conflict: :nothing for safer concurrent operations
• Replaced unsafe String.to_integer with Integer.parse for input validation
• Replaced IO.puts with proper Logger.debug/Logger.warning calls

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
copi.owasp.org/priv/repo/migrations/20260218120000_add_unique_constraint_to_votes.exs	Adds database unique constraint on votes table to prevent duplicate (player_id, dealt_card_id) pairs
copi.owasp.org/lib/copi_web/live/player_live/show.ex	Refactors voting handlers to query database directly, use on_conflict handling, safe input parsing, and proper logging
copi.owasp.org/test/copi_web/live/player_live_test.exs	Adds tests verifying database constraints prevent duplicate votes and continue votes

Comments suppressed due to low confidence (1)

copi.owasp.org/lib/copi_web/live/player_live/show.ex:17

• The Vote schema's changeset should include a unique_constraint to match the database constraint and provide meaningful error messages. Currently, ContinueVote has this constraint in its changeset (line 17 of continue_vote.ex) but Vote does not. Add: |> unique_constraint([:player_id, :dealt_card_id], name: :votes_player_dealt_card_unique_index) to the changeset function.

  alias Copi.Cornucopia.ContinueVote

  @impl true
  def mount(_params, _session, socket) do
    {:ok, socket}
  end

[immortal71]

I have implemented copilot suggestions, @sydseter is this good to go ?

[sydseter]

@immortal71 I am not sure. It’s a will take me some time to look deeper into this. I am going to leave this open for now and come back to it sometime next week.

[sydseter]

Besides this pull-request, I haven’t had any reports about this. That doesn’t mean it doesn’t or can’t happen. It’s just that I want to be extra careful making changes here.

[immortal71]

@sydseter so should i further work on this or not or wait for your review ?

[immortal71]

@sydseter do u have any issues I can work on ? ~ something big heavy task which I can do and keep me bg ?
light will ask work if there is no work to do