diff --git a/app/features/sendouq-match/SQMatchRepository.server.test.ts b/app/features/sendouq-match/SQMatchRepository.server.test.ts index 4018de171..5b36bdfab 100644 --- a/app/features/sendouq-match/SQMatchRepository.server.test.ts +++ b/app/features/sendouq-match/SQMatchRepository.server.test.ts @@ -460,6 +460,58 @@ describe("finalizeMatch", () => { playerResults.map(() => matchSeason.nth), ); }); + + // Demonstrates a bug: reportMapWinner checks the match lock on a snapshot read + // outside the finalizing transaction, so two teammates confirming the score at + // the same time both finalize. Rating/stat changes get applied twice. + test("concurrent score confirmations finalize the match only once", async () => { + const setup = await setupMatch(); + + let reportedCount = 0; + let result = await SQMatchRepository.reportMapWinner({ + matchId: setup.match.id, + winnerId: setup.alphaGroupId, + reportedByUserId: setup.alphaMembers[0].id, + reportedCount, + }); + while (result.status === "MAP_REPORTED") { + reportedCount++; + result = await SQMatchRepository.reportMapWinner({ + matchId: setup.match.id, + winnerId: setup.alphaGroupId, + reportedByUserId: setup.alphaMembers[0].id, + reportedCount, + }); + } + expect(result.status).toBe("MATCH_REPORTED"); + + const skillsBeforeConfirm = await fetchSkills(setup.match.id); + + const [first, second] = await Promise.all([ + SQMatchRepository.reportMapWinner({ + matchId: setup.match.id, + winnerId: setup.alphaGroupId, + reportedByUserId: setup.bravoMembers[0].id, + reportedCount: reportedCount + 1, + }), + SQMatchRepository.reportMapWinner({ + matchId: setup.match.id, + winnerId: setup.alphaGroupId, + reportedByUserId: setup.bravoMembers[1].id, + reportedCount: reportedCount + 1, + }), + ]); + + const finalizedCount = [first, second].filter( + (r) => r.status === "MATCH_FINALIZED", + ).length; + expect(finalizedCount).toBe(1); + + const skillsAfterConfirm = await fetchSkills(setup.match.id); + const skillsFromThisFinalization = + skillsAfterConfirm.length - skillsBeforeConfirm.length; + expect(skillsFromThisFinalization).toBe(FULL_GROUP_SIZE * 2 + 2); + }); }); describe("undoMatchReport", () => { diff --git a/app/features/sendouq-match/SQMatchRepository.server.ts b/app/features/sendouq-match/SQMatchRepository.server.ts index 372dcbdb2..ae6b3b2b7 100644 --- a/app/features/sendouq-match/SQMatchRepository.server.ts +++ b/app/features/sendouq-match/SQMatchRepository.server.ts @@ -1179,7 +1179,7 @@ async function handleMatchConfirmation({ .filter((m) => m.winnerGroupId !== null) .map((m) => (m.winnerGroupId === match.groupAlpha.id ? "ALPHA" : "BRAVO")); - await finalizeMatch({ + const finalized = await finalizeMatch({ match, members, winners, @@ -1190,6 +1190,10 @@ async function handleMatchConfirmation({ SQGroupRepository.setAsInactive(groupToDeactivate, trx), }); + if (!finalized) { + return { status: "ALREADY_LOCKED" }; + } + return { status: "MATCH_FINALIZED" }; } @@ -1225,7 +1229,7 @@ async function handleStaffFinalization({ winnerId === match.groupAlpha.id ? "ALPHA" : "BRAVO", ]; - await finalizeMatch({ + const finalized = await finalizeMatch({ match, members, winners, @@ -1247,6 +1251,10 @@ async function handleStaffFinalization({ }, }); + if (!finalized) { + return { status: "ALREADY_LOCKED" }; + } + return { status: "MATCH_FINALIZED" }; } @@ -1289,7 +1297,10 @@ async function finalizeMatch({ loserGroupId, }); - await db.transaction().execute(async (trx) => { + return db.transaction().execute(async (trx) => { + const { isLocked, confirmedAt } = await findLockState(match.id, trx); + if (isLocked || confirmedAt) return false; + if (preFinalize) await preFinalize(trx); await trx .updateTable("GroupMatch") @@ -1322,9 +1333,27 @@ async function finalizeMatch({ }, trx, ); + + return true; }); } +/** Lock state read inside the finalizing transaction so concurrent confirmations can't both finalize. */ +function findLockState(matchId: number, trx: Transaction) { + return trx + .selectFrom("GroupMatch") + .select(({ exists, selectFrom }) => [ + "GroupMatch.confirmedAt", + exists( + selectFrom("Skill") + .select("Skill.id") + .where("Skill.groupMatchId", "=", matchId), + ).as("isLocked"), + ]) + .where("GroupMatch.id", "=", matchId) + .executeTakeFirstOrThrow(); +} + /** Matches created before the given cutoff whose score was never confirmed and that no cancellation has locked. */ export function findUnfinishedMatchesCreatedBefore(cutoff: Date) { return db @@ -1394,7 +1423,7 @@ export async function resolveUnfinishedMatch( .filter((m) => m.winnerGroupId !== null) .map((m) => (m.winnerGroupId === match.groupAlpha.id ? "ALPHA" : "BRAVO")); - await finalizeMatch({ + const finalized = await finalizeMatch({ match, members: buildMembers(match), winners, @@ -1407,6 +1436,10 @@ export async function resolveUnfinishedMatch( }, }); + if (!finalized) { + return { status: "ALREADY_LOCKED" }; + } + return { status: "CONFIRMED" }; }