diff --git a/generated/schema.graphql b/generated/schema.graphql index 1b7c659c0..750bc4401 100644 --- a/generated/schema.graphql +++ b/generated/schema.graphql @@ -1601,6 +1601,11 @@ enum abandoned_matches_constraint { unique or primary key constraint on columns "id" """ abandoned_matches_pkey + + """ + unique or primary key constraint on columns "match_id", "steam_id" + """ + abandoned_matches_steam_id_match_id_key } """ diff --git a/generated/schema.ts b/generated/schema.ts index cf1294428..475b2be0f 100644 --- a/generated/schema.ts +++ b/generated/schema.ts @@ -1397,7 +1397,7 @@ export interface abandoned_matches_avg_fields { /** unique or primary key constraints on table "abandoned_matches" */ -export type abandoned_matches_constraint = 'abandoned_matches_pkey' +export type abandoned_matches_constraint = 'abandoned_matches_pkey' | 'abandoned_matches_steam_id_match_id_key' /** aggregate max on columns */ diff --git a/hasura/functions/tournaments/can_join_tournament.sql b/hasura/functions/tournaments/can_join_tournament.sql index eeccb9df3..ec5e4556c 100644 --- a/hasura/functions/tournaments/can_join_tournament.sql +++ b/hasura/functions/tournaments/can_join_tournament.sql @@ -77,16 +77,32 @@ CREATE OR REPLACE FUNCTION public.joined_tournament(tournament public.tournament LANGUAGE plpgsql STABLE AS $$ DECLARE - on_roster boolean; + _steam_id bigint := (hasura_session ->> 'x-hasura-user-id')::bigint; BEGIN - SELECT EXISTS ( + RETURN EXISTS ( SELECT 1 FROM tournament_team_roster ttr WHERE - tournament_id = tournament.id - AND player_steam_id = (hasura_session ->> 'x-hasura-user-id')::bigint - ) INTO on_roster; - - RETURN on_roster; + ttr.tournament_id = tournament.id + AND ttr.player_steam_id = _steam_id + ) OR EXISTS ( + -- An owner who fields a team without playing on it is still part of + -- the tournament. + SELECT 1 + FROM tournament_teams tt + WHERE + tt.tournament_id = tournament.id + AND tt.owner_steam_id = _steam_id + ) OR EXISTS ( + -- Nobody is on a roster until the draft runs, so in a free agent + -- tournament this is everyone who signed up. Drafted agents are on a + -- roster by then, and withdrawn ones have left the pool. + SELECT 1 + FROM tournament_free_agents tfa + WHERE + tfa.tournament_id = tournament.id + AND tfa.player_steam_id = _steam_id + AND tfa.status IN ('registered', 'waitlisted') + ); END; $$; \ No newline at end of file diff --git a/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/down.sql b/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/down.sql new file mode 100644 index 000000000..f6c04b270 --- /dev/null +++ b/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/down.sql @@ -0,0 +1,2 @@ +ALTER TABLE "public"."abandoned_matches" + DROP CONSTRAINT IF EXISTS "abandoned_matches_steam_id_match_id_key"; diff --git a/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/up.sql b/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/up.sql new file mode 100644 index 000000000..2a178aa26 --- /dev/null +++ b/hasura/migrations/default/1886000000000_abandoned_matches_unique_per_match/up.sql @@ -0,0 +1,27 @@ +-- One abandon per player per match. The plugin re-arms its disconnect timer on +-- every disconnect and on every map of a series, so the same leave can report +-- itself several times, and sanction_policy_occurrences() counts rows: each +-- duplicate moved the player a rung up the escalating cooldown ladder for a +-- single offense. +-- +-- match_id stays nullable (historical rows, and no-shows recorded before a +-- match exists), and postgres treats NULLs as distinct, so those rows are +-- unaffected by the constraint. +DELETE FROM public.abandoned_matches a + USING public.abandoned_matches b + WHERE a.match_id IS NOT NULL + AND a.match_id = b.match_id + AND a.steam_id = b.steam_id + AND (a.abandoned_at, a.id) > (b.abandoned_at, b.id); + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'abandoned_matches_steam_id_match_id_key' + ) THEN + ALTER TABLE "public"."abandoned_matches" + ADD CONSTRAINT "abandoned_matches_steam_id_match_id_key" + UNIQUE ("steam_id", "match_id"); + END IF; +END $$; diff --git a/hasura/triggers/team_roster.sql b/hasura/triggers/team_roster.sql index 08a925f19..d72c71a40 100644 --- a/hasura/triggers/team_roster.sql +++ b/hasura/triggers/team_roster.sql @@ -27,6 +27,34 @@ $$; DROP TRIGGER IF EXISTS tbi_team_roster ON public.team_roster; CREATE TRIGGER tbi_team_roster BEFORE INSERT ON public.team_roster FOR EACH ROW EXECUTE FUNCTION public.tbi_team_roster(); +-- The owner is a team's last line of authority: can_change_team_role and +-- can_remove_from_team both fall back to owner_steam_id, so an owner who walks +-- off the roster leaves a team that only a site admin can manage. Ownership has +-- to be handed over first. +-- +-- Deleting the team itself cascades to these rows, and by then the team row is +-- already gone, so that path finds no owner here and passes. +CREATE OR REPLACE FUNCTION public.tbd_team_roster() RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM teams t + WHERE t.id = OLD.team_id + AND t.owner_steam_id = OLD.player_steam_id + ) THEN + RAISE EXCEPTION USING ERRCODE = '22000', + MESSAGE = 'The team owner cannot leave the team; transfer ownership first'; + END IF; + + RETURN OLD; +END; +$$; + +DROP TRIGGER IF EXISTS tbd_team_roster ON public.team_roster; +CREATE TRIGGER tbd_team_roster BEFORE DELETE ON public.team_roster FOR EACH ROW EXECUTE FUNCTION public.tbd_team_roster(); + CREATE OR REPLACE FUNCTION public.tad_team_roster() RETURNS TRIGGER LANGUAGE plpgsql AS $$ diff --git a/src/chat/chat.service.spec.ts b/src/chat/chat.service.spec.ts index f65d3d719..57afaa901 100644 --- a/src/chat/chat.service.spec.ts +++ b/src/chat/chat.service.spec.ts @@ -49,11 +49,110 @@ describe("ChatService direct messages", () => { // Which matches this player belongs to, by id. let myMatches: string[]; + // The one tournament the fake knows about, and who is attached to it. + let tournament: { + organizers: string[]; + teamOwners: string[]; + roster: string[]; + freeAgents: Array<{ steam_id: string; status: string }>; + }; // Who the organizers' role gate admits. let staff: string[]; + // Answers the access query the way the database would, so the assertions are + // about who gets in rather than about the shape of the query. + const tournamentAdmits = (where: any) => + (where._or ?? []).some((branch: any) => { + if (branch.is_organizer) { + return tournament.organizers.includes(String(steamIdIn(branch))); + } + + if (branch.teams) { + return branch.teams._or.some((teamBranch: any) => { + const steamId = String(steamIdIn(teamBranch)); + return teamBranch.owner_steam_id + ? tournament.teamOwners.includes(steamId) + : tournament.roster.includes(steamId); + }); + } + + if (branch.free_agents) { + const steamId = String(branch.free_agents.player_steam_id._eq); + const statuses = branch.free_agents.status?._in ?? []; + + return tournament.freeAgents.some( + (freeAgent) => + freeAgent.steam_id === steamId && + statuses.includes(freeAgent.status), + ); + } + + return false; + }); + + // the steam id buried anywhere in one branch of the _or + const steamIdIn = (branch: any): string | undefined => { + if (typeof branch !== "object" || branch === null) { + return undefined; + } + + for (const [key, value] of Object.entries(branch)) { + if (key.endsWith("steam_id") && value?._eq !== undefined) { + return String(value._eq); + } + + const nested = Array.isArray(value) + ? value.map(steamIdIn).find(Boolean) + : steamIdIn(value); + + if (nested) { + return nested; + } + } + + return undefined; + }; + const hasuraService = { query: jest.fn(async (query: any) => { + if (query.tournaments) { + return { + tournaments: tournamentAdmits(query.tournaments.__args.where) + ? [{ id: "t-1" }] + : [], + }; + } + + if (query.tournaments_by_pk) { + return { + tournaments_by_pk: { + organizer_steam_id: tournament.organizers[0], + organizers: tournament.organizers + .slice(1) + .map((steam_id) => ({ steam_id })), + teams: [ + { + owner_steam_id: tournament.teamOwners[0], + roster: tournament.roster.map((player_steam_id) => ({ + player_steam_id, + })), + }, + ], + free_agents: tournament.freeAgents + .filter((freeAgent) => + ( + query.tournaments_by_pk.free_agents?.__args?.where?.status + ?._in ?? [] + ).includes(freeAgent.status), + ) + .map((freeAgent) => ({ + player_steam_id: freeAgent.steam_id, + status: freeAgent.status, + })), + }, + }; + } + if (query.matches_by_pk) { return myMatches.includes(query.matches_by_pk.__args.id) ? { @@ -107,6 +206,12 @@ describe("ChatService direct messages", () => { jest.clearAllMocks(); acceptedFriendships = [[ME, FRIEND]]; myMatches = ["m-1"]; + tournament = { + organizers: [STRANGER], + teamOwners: [], + roster: [], + freeAgents: [], + }; staff = []; role = "user"; queries = []; @@ -187,6 +292,102 @@ describe("ChatService direct messages", () => { }); }); + describe("tournament chat", () => { + const join = async (steamId: string) => { + await service.joinMatchLobby( + client(steamId), + ChatLobbyType.Tournament, + "t-1", + ); + return joined(); + }; + + it("lets a player on a tournament team roster in", async () => { + tournament.roster = [ME]; + + expect(await join(ME)).toBe(true); + }); + + it("lets a registered free agent in", async () => { + // in a free-agent tournament nobody is on a roster until the draft, so + // this is everyone who signed up + tournament.freeAgents = [{ steam_id: ME, status: "registered" }]; + + expect(await join(ME)).toBe(true); + }); + + it("lets a waitlisted free agent in", async () => { + tournament.freeAgents = [{ steam_id: ME, status: "waitlisted" }]; + + expect(await join(ME)).toBe(true); + }); + + it("keeps a withdrawn free agent out", async () => { + tournament.freeAgents = [{ steam_id: ME, status: "withdrawn" }]; + + expect(await join(ME)).toBe(false); + }); + + it("keeps an unrelated player out", async () => { + expect(await join(ME)).toBe(false); + }); + + // the message write is the awaited step; the broadcast after it is + // deliberately fire-and-forget + const posted = () => + redis.hset.mock.calls.some(([key]) => key === "chat_tournament_t-1"); + + it("stops a free agent who withdrew from posting", async () => { + // the room's membership lives in redis for 24h, so leaving the pool has + // to be re-checked when the message is sent, not only when joining + redis.hget.mockResolvedValue(JSON.stringify({ steam_id: ME })); + tournament.freeAgents = [{ steam_id: ME, status: "withdrawn" }]; + + await service.sendMessageToChat( + ChatLobbyType.Tournament, + "t-1", + { steam_id: ME, name: "Someone", role } as any, + "still here", + ); + + expect(posted()).toBe(false); + }); + + it("lets a registered free agent post", async () => { + redis.hget.mockResolvedValue(JSON.stringify({ steam_id: ME })); + tournament.freeAgents = [{ steam_id: ME, status: "registered" }]; + + await service.sendMessageToChat( + ChatLobbyType.Tournament, + "t-1", + { steam_id: ME, name: "Someone", role } as any, + "hello", + ); + + expect(posted()).toBe(true); + }); + + it("notifies free agents as well as rostered players", async () => { + tournament.organizers = [STRANGER]; + tournament.teamOwners = [FRIEND]; + tournament.roster = [FRIEND]; + tournament.freeAgents = [ + { steam_id: ME, status: "registered" }, + { steam_id: "76561198000000004", status: "withdrawn" }, + ]; + + const recipients = await service.getLobbyMemberSteamIds( + ChatLobbyType.Tournament, + "t-1", + ); + + expect(recipients).toContain(ME); + expect(recipients).toContain(FRIEND); + expect(recipients).toContain(STRANGER); + expect(recipients).not.toContain("76561198000000004"); + }); + }); + describe("rosters", () => { it("resolves both parties of a conversation", async () => { expect( diff --git a/src/chat/chat.service.ts b/src/chat/chat.service.ts index bc5432298..104915367 100644 --- a/src/chat/chat.service.ts +++ b/src/chat/chat.service.ts @@ -10,6 +10,7 @@ import { ChatLobbyType } from "./enums/ChatLobbyTypes"; import { e_notification_types_enum, e_player_roles_enum, + e_tournament_free_agent_statuses_enum, } from "generated/schema"; import { isRoleAbove, rolesAtOrAbove } from "src/utilities/isRoleAbove"; import { NotificationsService } from "src/notifications/notifications.service"; @@ -38,6 +39,11 @@ export class ChatService { private static readonly DEFAULT_TTL = 60 * 60; + // A drafted free agent is on a roster and gets in that way; withdrawn means + // they left the pool. + private static readonly TOURNAMENT_CHAT_FREE_AGENT_STATUSES: e_tournament_free_agent_statuses_enum[] = + ["registered", "waitlisted"]; + // Which setting governs which room's lifetime, and what it falls back to. // Read from system/ on boot and whenever a setting changes, so there is one // list to keep in step rather than a branch per type in three places. @@ -283,6 +289,18 @@ export class ChatService { ], }, }, + { + // Nobody is on a roster until the draft runs, so in a + // free agent tournament this is everyone who signed up. + free_agents: { + player_steam_id: { + _eq: user.steam_id, + }, + status: { + _in: ChatService.TOURNAMENT_CHAT_FREE_AGENT_STATUSES, + }, + }, + }, ], }, }, @@ -515,6 +533,16 @@ export class ChatService { ) { return; } + + // Room membership lives in redis for a day, so leaving the tournament - + // withdrawing from the free agent pool, or being dropped from a roster - + // has to be re-checked here rather than only at join time. + if ( + type === ChatLobbyType.Tournament && + !(await this.canAccessLobby(type, id, player)) + ) { + return; + } } const name = await this.redis.get( @@ -846,6 +874,16 @@ export class ChatService { owner_steam_id: true, roster: { player_steam_id: true }, }, + free_agents: { + __args: { + where: { + status: { + _in: ChatService.TOURNAMENT_CHAT_FREE_AGENT_STATUSES, + }, + }, + }, + player_steam_id: true, + }, }, }); @@ -862,6 +900,10 @@ export class ChatService { } } + for (const freeAgent of tournaments_by_pk?.free_agents ?? []) { + add(freeAgent.player_steam_id); + } + break; } case ChatLobbyType.Draft: { diff --git a/src/matches/events/MatchAbandoned.spec.ts b/src/matches/events/MatchAbandoned.spec.ts new file mode 100644 index 000000000..49852004c --- /dev/null +++ b/src/matches/events/MatchAbandoned.spec.ts @@ -0,0 +1,67 @@ +import { Logger } from "@nestjs/common"; +import MatchAbandoned from "./MatchAbandoned"; + +describe("MatchAbandoned", () => { + let processor: MatchAbandoned; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock }; + let affectedRows: number; + + beforeEach(() => { + affectedRows = 1; + + hasura = { + query: jest.fn(async () => ({ players_by_pk: { name: "keith" } })), + mutation: jest.fn(async () => ({ + insert_abandoned_matches: { affected_rows: affectedRows }, + })), + }; + notifications = { send: jest.fn() }; + + processor = new MatchAbandoned( + new Logger("MatchAbandonedTest"), + hasura as any, + {} as any, + {} as any, + notifications as any, + ); + processor.setData("11111111-1111-1111-1111-111111111111", { + steam_id: "76561198000000001", + }); + }); + + function insertArgs() { + return hasura.mutation.mock.calls[0][0].insert_abandoned_matches.__args; + } + + it("records the abandon against the match", async () => { + await processor.process(); + + expect(insertArgs().objects).toEqual([ + { + steam_id: "76561198000000001", + match_id: "11111111-1111-1111-1111-111111111111", + }, + ]); + expect(notifications.send).toHaveBeenCalledTimes(1); + }); + + it("ignores a repeat abandon for the same match", async () => { + // the plugin can report the same leave more than once, and every extra row + // would escalate the player's cooldown a rung + await processor.process(); + + expect(insertArgs().on_conflict).toEqual({ + constraint: "abandoned_matches_steam_id_match_id_key", + update_columns: [], + }); + }); + + it("does not notify admins twice for one abandon", async () => { + affectedRows = 0; + + await processor.process(); + + expect(notifications.send).not.toHaveBeenCalled(); + }); +}); diff --git a/src/matches/events/MatchAbandoned.ts b/src/matches/events/MatchAbandoned.ts index faf91d6a4..053bfea1f 100644 --- a/src/matches/events/MatchAbandoned.ts +++ b/src/matches/events/MatchAbandoned.ts @@ -5,18 +5,31 @@ export default class MatchAbandoned extends MatchEventProcessor<{ steam_id: string; }> { public async process() { - await this.hasura.mutation({ - insert_abandoned_matches_one: { + // The plugin re-arms its disconnect timer per reconnect and per map, so one + // leave can report itself several times. The cooldown counts rows, so a + // duplicate would escalate the ban for a single offense. + const { insert_abandoned_matches: inserted } = await this.hasura.mutation({ + insert_abandoned_matches: { __args: { - object: { - steam_id: this.data.steam_id, - match_id: this.matchId, + objects: [ + { + steam_id: this.data.steam_id, + match_id: this.matchId, + }, + ], + on_conflict: { + constraint: "abandoned_matches_steam_id_match_id_key", + update_columns: [], }, }, - __typename: true, + affected_rows: true, }, }); + if (!inserted?.affected_rows) { + return; + } + await this.notifyAdmins(); } diff --git a/src/matches/events/MatchMapStatusEvent.spec.ts b/src/matches/events/MatchMapStatusEvent.spec.ts index 9f719c1fb..1a8f405c5 100644 --- a/src/matches/events/MatchMapStatusEvent.spec.ts +++ b/src/matches/events/MatchMapStatusEvent.spec.ts @@ -126,4 +126,64 @@ describe("MatchMapStatusEvent", () => { }), ]); }); + + // The winner the server reports is cross-checked against the round score, + // because a wrong winner here decides the series. The plugin reports it first + // with WaitingForTV or UploadingDemo, so a map that stalls there would + // otherwise keep a value nobody checked. + describe("winner resolution", () => { + beforeEach(() => { + matchMaps = [{ id: "map-1", status: "Live" }]; + currentMatchMapId = "map-1"; + }); + + const winnerWritten = () => mapUpdates()[0]?.__args._set.winning_lineup_id; + + it.each(["Finished", "WaitingForTV", "UploadingDemo"])( + "overrides a wrong winner reported with %s", + async (status) => { + // lineup-1 won the map 13-7 + await process(status, "lineup-2"); + + expect(winnerWritten()).toBe("lineup-1"); + }, + ); + + it("keeps the reported winner of a surrendered map", async () => { + // the team that gives up is frequently the one ahead on rounds, so the + // score is not the authority here - the forfeit is + await process("Surrendered", "lineup-2"); + + expect(winnerWritten()).toBe("lineup-2"); + }); + + it("keeps the reported winner when the scores are tied", async () => { + hasura.query.mockImplementation(async (query: any) => { + if (query.match_map_rounds) { + return { + match_map_rounds: [{ lineup_1_score: 12, lineup_2_score: 12 }], + }; + } + return { + matches_by_pk: { + current_match_map_id: currentMatchMapId, + lineup_1_id: "lineup-1", + lineup_2_id: "lineup-2", + status: "Live", + match_maps: matchMaps, + }, + }; + }); + + await process("Finished", "lineup-2"); + + expect(winnerWritten()).toBe("lineup-2"); + }); + + it("leaves the winner alone for a status that carries none", async () => { + await process("Paused"); + + expect(winnerWritten()).toBeUndefined(); + }); + }); }); diff --git a/src/matches/events/MatchMapStatusEvent.ts b/src/matches/events/MatchMapStatusEvent.ts index 076d92b98..e373656bc 100644 --- a/src/matches/events/MatchMapStatusEvent.ts +++ b/src/matches/events/MatchMapStatusEvent.ts @@ -58,10 +58,20 @@ export default class MatchMapStatusEvent extends MatchEventProcessor<{ const isFinished = this.data.status === "Finished"; + // The plugin reports the winner when the map ends, which is WaitingForTV or + // UploadingDemo before it is ever Finished. A map that stalls there would + // otherwise keep a winner nobody checked against the round score. + // Surrendered is deliberately not in this list: its winner is the team that + // did not give up, which is frequently the team behind on rounds. + const carriesPlayedOutWinner = + isFinished || + this.data.status === "WaitingForTV" || + this.data.status === "UploadingDemo"; + let resolvedWinningLineupId: string | undefined = this.data.winning_lineup_id; - if (isFinished) { + if (carriesPlayedOutWinner) { const { match_map_rounds } = await this.hasura.query({ match_map_rounds: { __args: { diff --git a/src/matches/jobs/CancelExpiredMatches.spec.ts b/src/matches/jobs/CancelExpiredMatches.spec.ts index 56064d735..87c2d1ad6 100644 --- a/src/matches/jobs/CancelExpiredMatches.spec.ts +++ b/src/matches/jobs/CancelExpiredMatches.spec.ts @@ -314,6 +314,37 @@ describe("CancelExpiredMatches", () => { .flatMap((call: any) => call.__args.objects) .map((row: any) => row.steam_id); + it("records a no-show at most once per player per match", async () => { + tournamentMatches = [ + expiredTournamentMatch({ + is_tournament_match: false, + lineup_1: { + id: "lineup-1", + is_ready: false, + lineup_players: [{ steam_id: "no-show-a", is_connected: false }], + }, + lineup_2: { + id: "lineup-2", + is_ready: false, + lineup_players: [{ steam_id: "no-show-b", is_connected: false }], + }, + }), + ]; + + await job.process(); + + // a player can already carry an abandon for this match from the plugin, + // and the cooldown counts rows - without this the whole batch would throw + const [insert] = hasura.mutation.mock.calls + .map(([arg]: [any]) => arg?.insert_abandoned_matches) + .filter(Boolean); + + expect(insert.__args.on_conflict).toEqual({ + constraint: "abandoned_matches_steam_id_match_id_key", + update_columns: [], + }); + }); + it("penalises only the players who never connected", async () => { tournamentMatches = [ expiredTournamentMatch({ diff --git a/src/matches/jobs/CancelExpiredMatches.ts b/src/matches/jobs/CancelExpiredMatches.ts index 7a955d980..18e9b5456 100644 --- a/src/matches/jobs/CancelExpiredMatches.ts +++ b/src/matches/jobs/CancelExpiredMatches.ts @@ -329,6 +329,13 @@ export class CancelExpiredMatches extends WorkerHost { steam_id: lineupPlayer.steam_id, match_id: match.id, })), + // A player may already carry an abandon for this match from the + // game server. Without this the conflict would throw away the whole + // batch, penalising nobody. + on_conflict: { + constraint: "abandoned_matches_steam_id_match_id_key", + update_columns: [], + }, }, affected_rows: true, }, @@ -389,9 +396,9 @@ export class CancelExpiredMatches extends WorkerHost { // Same ordering as cancelMatch, and for the same reason: the matches // trigger clears cancels_at on the Forfeit/Finished transition and // getExpiredMatches requires cancels_at IS NOT NULL, so the match leaves - // the window and this runs exactly once. abandoned_matches has no unique - // constraint, so a second insert would silently double the escalating - // cooldown. + // the window and this runs exactly once. abandoned_matches is unique per + // (steam_id, match_id) as a backstop, so a second insert cannot double the + // escalating cooldown. await this.recordNoShows(match); } diff --git a/src/matchmaking/matchmake.integration.spec.ts b/src/matchmaking/matchmake.integration.spec.ts index 26dceb0f4..7f625d232 100644 --- a/src/matchmaking/matchmake.integration.spec.ts +++ b/src/matchmaking/matchmake.integration.spec.ts @@ -19,6 +19,7 @@ import { RedisManagerService } from "../redis/redis-manager/redis-manager.servic import { MatchmakingQueues } from "./enums/MatchmakingQueues"; import { FakeRedis } from "./testing/fakeRedis"; import { + getMatchmakingLobbyDetailsCacheKey, getMatchmakingQueueCacheKey, getMatchmakingRankCacheKey, } from "./utilities/cacheKeys"; @@ -50,6 +51,7 @@ describe("matchmaking (end to end)", () => { updateMatchStatus: jest.Mock; }; let hasura: { query: jest.Mock; mutation: jest.Mock }; + let lobbyService: Record; let queue: { add: jest.Mock; remove: jest.Mock }; beforeEach(async () => { @@ -62,7 +64,7 @@ describe("matchmaking (end to end)", () => { confirmationIds = []; lineupInserts = []; - const lobbyService = { + lobbyService = { getLobbyDetails: jest.fn(async (lobbyId: string) => { const lobby = lobbyStore.get(lobbyId); return lobby ? { ...lobby, players: [...lobby.players] } : null; @@ -91,6 +93,7 @@ describe("matchmaking (end to end)", () => { }), removeLobbyDetails: jest.fn(async (lobbyId: string) => { lobbyStore.delete(lobbyId); + await redis.del(getMatchmakingLobbyDetailsCacheKey(lobbyId)); }), removeConfirmationIdFromLobby: jest.fn(), }; @@ -176,6 +179,11 @@ describe("matchmaking (end to end)", () => { async function enqueue(lobbies: MatchmakingLobby[]) { for (const lobby of lobbies) { lobbyStore.set(lobby.lobbyId, lobby); + // the real getLobbyDetails reads this key, and the orphan sweep checks it + await redis.set( + getMatchmakingLobbyDetailsCacheKey(lobby.lobbyId), + JSON.stringify(lobby), + ); for (const region of lobby.regions) { await redis.zadd( getMatchmakingRankCacheKey(lobby.type, region), @@ -250,6 +258,93 @@ describe("matchmaking (end to end)", () => { // --- tests + describe("region stats", () => { + function lastRegionStats() { + const published = redis.published + .filter((entry) => entry.channel === "broadcast-message") + .map((entry) => JSON.parse(entry.message)) + .filter((message) => message.event === "matchmaking:region-stats"); + + return published.at(-1)?.data; + } + + it("counts queued players, not lobbies", async () => { + await enqueue([ + makeLobby("trio", [5000, 5000, 5000]), + makeLobby("solo", [5000]), + ]); + + await service.sendRegionStats(); + + const queued = lastRegionStats()["us-east"][COMPETITIVE]; + const players = queued.reduce( + (total: number, lobby: { players: number }) => total + lobby.players, + 0, + ); + + // a party of three is three people waiting for a game, not one + expect(players).toBe(4); + }); + + it("counts a multi region lobby once", async () => { + await enqueue([ + makeLobby("duo", [5000, 5000], { regions: ["us-east", "eu-west"] }), + ]); + + await service.sendRegionStats(); + + const stats = lastRegionStats(); + const east = stats["us-east"][COMPETITIVE]; + const west = stats["eu-west"][COMPETITIVE]; + + // the same lobby index in both regions is what lets the client dedupe it + expect(east[0].lobby).toBe(west[0].lobby); + expect(east[0].players).toBe(2); + }); + + it("drops a queue entry whose lobby details are gone", async () => { + await enqueue([makeLobby("ghost", [5000]), makeLobby("real", [5000])]); + + // a lobby can be left in the sorted set with no details behind it: a + // leave that lands between reading the details and the zadd of a rejoin + lobbyStore.delete("ghost"); + await redis.del(getMatchmakingLobbyDetailsCacheKey("ghost")); + + await service.sendRegionStats(); + + const queued = lastRegionStats()["us-east"][COMPETITIVE]; + expect(queued).toHaveLength(1); + + // and it is swept out of the queue rather than counted forever + expect( + redis.members(getMatchmakingQueueCacheKey(COMPETITIVE, "us-east")), + ).toEqual(["real"]); + }); + + it("keeps a lobby that rejoins while the stats are being built", async () => { + await enqueue([makeLobby("rejoiner", [5000])]); + + const details = lobbyService.getLobbyDetails as jest.Mock; + details.mockImplementationOnce(async (lobbyId: string) => { + // the lobby leaves and comes straight back while we are reading it + const rejoined = makeLobby("rejoiner", [5000]); + lobbyStore.set("rejoiner", rejoined); + await redis.set( + getMatchmakingLobbyDetailsCacheKey(lobbyId), + JSON.stringify(rejoined), + ); + return null; + }); + + await service.sendRegionStats(); + + // the sweep must not take the fresh entry down with the stale read + expect( + redis.members(getMatchmakingQueueCacheKey(COMPETITIVE, "us-east")), + ).toEqual(["rejoiner"]); + }); + }); + describe("queue state", () => { it("matches ten solo players and empties the queue", async () => { const lobbies = Array.from({ length: 10 }, (_, i) => @@ -694,6 +789,70 @@ describe("matchmaking (end to end)", () => { expect(matchAssistant.createMatchBasedOnType).toHaveBeenCalledTimes(1); }); + it("creates one match when the last players confirm at the same moment", async () => { + await enqueue(tenSolos()); + await service.matchmake(COMPETITIVE, "us-east"); + + const [confirmation] = confirmations; + const [confirmationId] = confirmationIds; + const players = [ + ...confirmation.team1.players, + ...confirmation.team2.players, + ]; + + for (const player of players.slice(0, 8)) { + await service.playerConfirmMatchmaking(confirmationId, player.steam_id); + } + + // the last two confirmations land together: both see a full lobby + await Promise.all( + players + .slice(8) + .map((player) => + service.playerConfirmMatchmaking(confirmationId, player.steam_id), + ), + ); + + expect(matchAssistant.createMatchBasedOnType).toHaveBeenCalledTimes(1); + expect(lineupInserts).toHaveLength(2); + }); + + it("does not create a second match when a player confirms twice", async () => { + await enqueue(tenSolos()); + await service.matchmake(COMPETITIVE, "us-east"); + + const { confirmation, confirmationId } = await confirmAll(); + + // the client re-sends matchmaking:confirm - the match already exists, so + // this must be a no-op rather than a second match for the same players + await service.playerConfirmMatchmaking( + confirmationId, + confirmation.team1.players[0].steam_id, + ); + + expect(matchAssistant.createMatchBasedOnType).toHaveBeenCalledTimes(1); + }); + + it("ignores a confirmation from a player who is not in the match", async () => { + await enqueue(tenSolos()); + await service.matchmake(COMPETITIVE, "us-east"); + + const [confirmation] = confirmations; + const [confirmationId] = confirmationIds; + const players = [ + ...confirmation.team1.players, + ...confirmation.team2.players, + ]; + + // nine of the ten ready up, and a stranger sends the tenth confirmation + for (const player of players.slice(0, 9)) { + await service.playerConfirmMatchmaking(confirmationId, player.steam_id); + } + await service.playerConfirmMatchmaking(confirmationId, "not-in-match"); + + expect(matchAssistant.createMatchBasedOnType).not.toHaveBeenCalled(); + }); + it("drops every lobby from the queue when nobody confirms", async () => { await enqueue(tenSolos()); await service.matchmake(COMPETITIVE, "us-east"); diff --git a/src/matchmaking/matchmake.service.ts b/src/matchmaking/matchmake.service.ts index 268fe2280..67ca74fbe 100644 --- a/src/matchmaking/matchmake.service.ts +++ b/src/matchmaking/matchmake.service.ts @@ -16,8 +16,10 @@ import { MatchAssistantService } from "src/matches/match-assistant/match-assista import { getMatchmakingQueueCacheKey, getMatchmakingConformationCacheKey, + getMatchmakingLobbyDetailsCacheKey, getMatchmakingRankCacheKey, } from "./utilities/cacheKeys"; +import { QueuedLobbyStat } from "./types/QueuedLobbyStat"; import { ExpectedPlayers } from "src/discord-bot/enums/ExpectedPlayers"; import { shuffleSplit } from "./utilities/shuffleSplit"; import { balanceTeams, canFillTeams } from "./utilities/balanceTeams"; @@ -111,11 +113,18 @@ export class MatchmakeService { const types: e_match_types_enum[] = ["Duel", "Wingman", "Competitive"]; const regionStats: Partial< - Record>> + Record>> > = {}; + const regionValues = regions.server_regions.map( + (region: { value: string }) => region.value, + ); + for (const type of types) { + // A lobby queued in several regions keeps one index across all of them, + // so the client can count it once. const lobbyIndexes = new Map(); + const lobbySizes = new Map(); for (const region of regions.server_regions) { const lobbyIds = await this.redis.zrange( @@ -124,15 +133,30 @@ export class MatchmakeService { -1, ); - const stats = (regionStats[region.value] ??= {}); - stats[type] = lobbyIds.map((lobbyId) => { + const queued: QueuedLobbyStat[] = []; + + for (const lobbyId of lobbyIds) { let index = lobbyIndexes.get(lobbyId); + if (index === undefined) { + const details = + await this.matchmakingLobbyService.getLobbyDetails(lobbyId); + + if (!details) { + await this.sweepOrphanedQueueEntry(lobbyId, type, regionValues); + continue; + } + index = lobbyIndexes.size; lobbyIndexes.set(lobbyId, index); + lobbySizes.set(lobbyId, details.players.length); } - return index; - }); + + queued.push({ lobby: index, players: lobbySizes.get(lobbyId) }); + } + + const stats = (regionStats[region.value] ??= {}); + stats[type] = queued; } } @@ -474,6 +498,49 @@ export class MatchmakeService { await this.redis.del(lockKey); } + // Removing the queue entry unconditionally would drop a lobby that left and + // rejoined between the details read and this call, so the details key is + // re-checked inside the script: if it is back, the entry is the fresh one. + private static readonly SWEEP_ORPHANED_ENTRY_SCRIPT = ` + if redis.call('EXISTS', KEYS[1]) == 1 then + return 0 + end + for i = 2, #KEYS do + redis.call('ZREM', KEYS[i], ARGV[1]) + end + return 1 + `; + + // A queue entry can outlive its details: addLobbyToQueue reads the details + // and then zadds, so a leave landing in between leaves an entry behind with + // nothing to read. Nothing expires the sorted sets, so it would sit in the + // queue - and in the queue counts - forever. + private async sweepOrphanedQueueEntry( + lobbyId: string, + type: e_match_types_enum, + regions: string[], + ) { + const keys = [getMatchmakingLobbyDetailsCacheKey(lobbyId)]; + + for (const region of regions) { + keys.push(getMatchmakingQueueCacheKey(type, region)); + keys.push(getMatchmakingRankCacheKey(type, region)); + } + + const swept = await this.redis.eval( + MatchmakeService.SWEEP_ORPHANED_ENTRY_SCRIPT, + keys.length, + ...keys, + lobbyId, + ); + + if (swept === 1) { + this.logger.warn( + `removed orphaned ${type} queue entry for lobby ${lobbyId}`, + ); + } + } + private static readonly CLAIM_LOBBY_SCRIPT = ` local acquired = redis.call('SET', KEYS[1], 1, 'EX', ARGV[2], 'NX') if not acquired then @@ -624,6 +691,8 @@ export class MatchmakeService { const confirmedKey = `${getMatchmakingConformationCacheKey(confirmationId)}:confirmed`; await this.redis.del(confirmedKey); + await this.redis.del(this.getMatchCreationClaimKey(confirmationId)); + await this.redis.del(getMatchmakingConformationCacheKey(confirmationId)); } @@ -726,13 +795,30 @@ export class MatchmakeService { confirmationId: string, steamId: string, ) { + const { lobbyIds, team1, team2, matchId } = + await this.getMatchConfirmationDetails(confirmationId); + + if (matchId) { + return; + } + + // An expired confirmation reads back as empty teams, so this also stops a + // late confirmation from creating a match with nobody in it. + const isPlaying = [...team1, ...team2].some( + (player) => player.steam_id === steamId, + ); + + if (!isPlaying) { + return; + } + await this.redis.hset( `${getMatchmakingConformationCacheKey(confirmationId)}:confirmed`, steamId, 1, ); - const { lobbyIds, team1, team2, confirmed } = + const { confirmed } = await this.getMatchConfirmationDetails(confirmationId); if (confirmed.length != team1.length + team2.length) { @@ -742,9 +828,28 @@ export class MatchmakeService { return; } + // Everyone is ready, but so is every other confirmation that arrived at the + // same time. Whoever claims this key creates the match; the rest return. + // The claim never expires on its own - it is dropped with the rest of the + // confirmation details, so a failed creation is cleaned up by the cancel + // job rather than by a second match. + const claimedCreation = await this.redis.set( + this.getMatchCreationClaimKey(confirmationId), + 1, + "NX", + ); + + if (!claimedCreation) { + return; + } + await this.createMatch(confirmationId); } + private getMatchCreationClaimKey(confirmationId: string) { + return `${getMatchmakingConformationCacheKey(confirmationId)}:creating`; + } + private async createMatch(confirmationId: string) { const { team1, team2, type, region, lobbyIds } = await this.getMatchConfirmationDetails(confirmationId); diff --git a/src/matchmaking/testing/fakeRedis.ts b/src/matchmaking/testing/fakeRedis.ts index e0ef6df6b..b209c160a 100644 --- a/src/matchmaking/testing/fakeRedis.ts +++ b/src/matchmaking/testing/fakeRedis.ts @@ -177,16 +177,33 @@ export class FakeRedis { } /** - * The redis EVAL command, not javascript eval. The lua source is ignored, not - * interpreted - this hardcodes the one script matchmaking runs - * (CLAIM_LOBBY_SCRIPT): SET NX the lock, and on success ZREM the lobby from - * every queue key passed in. Atomic here by virtue of being synchronous, - * which is the property the real script buys with lua. + * The redis EVAL command, not javascript eval. The lua source is not + * interpreted - this hardcodes the two scripts matchmaking runs, recognised + * by their first command, and both are atomic here by virtue of being + * synchronous, which is the property the real scripts buy with lua. + * + * CLAIM_LOBBY_SCRIPT: SET NX the lock, and on success ZREM the lobby from + * every queue key passed in. + * + * SWEEP_ORPHANED_ENTRY_SCRIPT: ZREM the lobby from every queue key passed in, + * but only while its details key is still missing. */ - async eval(_script: string, numKeys: number, ...args: unknown[]) { + async eval(script: string, numKeys: number, ...args: unknown[]) { const keys = args.slice(0, numKeys) as string[]; const [member, ttl] = args.slice(numKeys) as [string, number]; + if (script.includes("EXISTS")) { + if (this.has(keys[0])) { + return 0; + } + + for (const key of keys.slice(1)) { + await this.zrem(key, member); + } + + return 1; + } + const acquired = await this.set(keys[0], 1, "EX", ttl, "NX"); if (!acquired) { return 0; diff --git a/src/matchmaking/types/QueuedLobbyStat.ts b/src/matchmaking/types/QueuedLobbyStat.ts new file mode 100644 index 000000000..5e10d3e60 --- /dev/null +++ b/src/matchmaking/types/QueuedLobbyStat.ts @@ -0,0 +1,8 @@ +// One lobby waiting in a region's queue, as broadcast to clients. The index is +// stable across the regions a lobby queued for, so multi-region lobbies are +// counted once; players is the lobby's size, because the queue count people +// care about is how many are waiting, not how many parties are. +export interface QueuedLobbyStat { + lobby: number; + players: number; +} diff --git a/src/system/system.controller.spec.ts b/src/system/system.controller.spec.ts new file mode 100644 index 000000000..a5aed3247 --- /dev/null +++ b/src/system/system.controller.spec.ts @@ -0,0 +1,146 @@ +import { SystemController } from "./system.controller"; + +// Name registration is the one place a player writes their own display name +// without an admin in the loop, so the guards around it are what keep the +// approval flow from being optional. +describe("SystemController names", () => { + let controller: SystemController; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock; notifyPlayers: jest.Mock }; + let player: { name: string; name_registered: boolean } | null; + + const user = (steamId: string, role: string | null = "user") => + ({ steam_id: steamId, role }) as any; + + beforeEach(() => { + player = { name: "current", name_registered: false }; + + hasura = { + query: jest.fn(async (payload: any) => { + if (payload.players_by_pk) { + return { players_by_pk: player }; + } + return { notifications: [] as Array }; + }), + mutation: jest.fn(async () => ({})), + }; + + notifications = { send: jest.fn(), notifyPlayers: jest.fn() }; + + controller = new SystemController( + {} as any, + hasura as any, + notifications as any, + {} as any, + {} as any, + {} as any, + ); + }); + + function registeredName() { + const call = hasura.mutation.mock.calls.find( + ([payload]: [any]) => payload.update_players_by_pk, + ); + return call?.[0].update_players_by_pk.__args._set.name; + } + + describe("registerName", () => { + it("registers a name for a player who has not registered one", async () => { + await controller.registerName({ user: user("1"), name: "keith" }); + + expect(registeredName()).toBe("keith"); + expect( + hasura.mutation.mock.calls[0][0].update_players_by_pk.__args._set + .name_registered, + ).toBe(true); + }); + + it("refuses to re-register a name that is already registered", async () => { + player = { name: "current", name_registered: true }; + + // registerName skips the admin approval that requestNameChange requires, + // so a second call would be a self-serve rename + await expect( + controller.registerName({ user: user("1"), name: "somebody-else" }), + ).rejects.toThrow(); + + expect(hasura.mutation).not.toHaveBeenCalled(); + }); + + it("rejects a blank name", async () => { + await expect( + controller.registerName({ user: user("1"), name: " " }), + ).rejects.toThrow(); + + expect(hasura.mutation).not.toHaveBeenCalled(); + }); + + it("rejects a name that is too short or too long", async () => { + await expect( + controller.registerName({ user: user("1"), name: "ab" }), + ).rejects.toThrow(); + + await expect( + controller.registerName({ user: user("1"), name: "a".repeat(33) }), + ).rejects.toThrow(); + + expect(hasura.mutation).not.toHaveBeenCalled(); + }); + + it("stores the trimmed name", async () => { + await controller.registerName({ user: user("1"), name: " keith " }); + + expect(registeredName()).toBe("keith"); + }); + }); + + describe("requestNameChange", () => { + it("files the request against the player who asked for it", async () => { + await controller.requestNameChange({ + user: user("76561198000000001"), + steam_id: "76561198000000001", + name: "new name", + } as any); + + expect(notifications.send).toHaveBeenCalled(); + const [, notification] = notifications.send.mock.calls[0]; + expect(notification.entity_id).toBe("76561198000000001"); + }); + + it("ignores a steam id the caller does not own", async () => { + // the action takes steam_id from the client, so without this a player can + // file a rename for somebody else and have an admin approve it + await controller.requestNameChange({ + user: user("76561198000000001"), + steam_id: "76561198000000002", + name: "new name", + } as any); + + const [, notification] = notifications.send.mock.calls[0]; + expect(notification.entity_id).toBe("76561198000000001"); + }); + + it("lets an administrator file a request for another player", async () => { + await controller.requestNameChange({ + user: user("76561198000000001", "administrator"), + steam_id: "76561198000000002", + name: "new name", + } as any); + + const [, notification] = notifications.send.mock.calls[0]; + expect(notification.entity_id).toBe("76561198000000002"); + }); + + it("rejects a blank name", async () => { + await expect( + controller.requestNameChange({ + user: user("76561198000000001"), + steam_id: "76561198000000001", + name: " ", + } as any), + ).rejects.toThrow(); + + expect(notifications.send).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/system/system.controller.ts b/src/system/system.controller.ts index 4e48d6c32..8b27bc6ef 100644 --- a/src/system/system.controller.ts +++ b/src/system/system.controller.ts @@ -161,6 +161,30 @@ export class SystemController { @HasuraAction() public async registerName(data: { user: User; name: string }) { + const name = SystemController.validateName(data.name); + + const { players_by_pk: player } = await this.hasura.query({ + players_by_pk: { + __args: { + steam_id: data.user.steam_id, + }, + name_registered: true, + }, + }); + + if (!player) { + throw new Error("Player not found"); + } + + // Registration is the one rename that skips admin approval, so it is only + // available to a player who has never registered a name. Everyone else + // goes through requestNameChange. + if (player.name_registered) { + throw new Error( + "Your name is already registered, request a name change instead", + ); + } + await this.hasura.mutation({ update_players_by_pk: { __args: { @@ -168,7 +192,7 @@ export class SystemController { steam_id: data.user.steam_id, }, _set: { - name: data.name, + name, name_registered: true, }, }, @@ -181,6 +205,16 @@ export class SystemController { }; } + private static validateName(name: string) { + const trimmed = (name ?? "").trim(); + + if (trimmed.length < 3 || trimmed.length > 32) { + throw new Error("Name must be between 3 and 32 characters"); + } + + return trimmed; + } + @HasuraAction() public async approveNameChange(data: { name: string; steam_id: string }) { await this.hasura.mutation({ @@ -227,7 +261,19 @@ export class SystemController { } @HasuraAction() - public async requestNameChange(data: { name: string; steam_id: string }) { + public async requestNameChange(data: { + user: User; + name: string; + steam_id: string; + }) { + const name = SystemController.validateName(data.name); + + // steam_id comes from the client, so a player could otherwise file a + // rename for somebody else and have an admin approve it. + const steamId = isRoleAbove(data.user?.role, "administrator") + ? data.steam_id + : data.user.steam_id; + const { notifications } = await this.hasura.query({ notifications: { __args: { @@ -236,7 +282,7 @@ export class SystemController { _eq: "NameChangeRequest", }, entity_id: { - _eq: data.steam_id, + _eq: steamId, }, is_read: { _eq: false, @@ -254,7 +300,7 @@ export class SystemController { const { players_by_pk: player } = await this.hasura.query({ players_by_pk: { __args: { - steam_id: data.steam_id, + steam_id: steamId, }, name: true, }, @@ -267,10 +313,10 @@ export class SystemController { await this.notifications.send( "NameChangeRequest", { - message: `Player ${NotificationsService.escapeHtml(player.name)} has requested to change their name to ${NotificationsService.escapeHtml(data.name)}`, + message: `Player ${NotificationsService.escapeHtml(player.name)} has requested to change their name to ${NotificationsService.escapeHtml(name)}`, title: "Name Change Request", role: "administrator", - entity_id: data.steam_id, + entity_id: steamId, }, [ { @@ -279,8 +325,8 @@ export class SystemController { type: "mutation", action: "denyNameChange", variables: { - name: data.name, - steam_id: data.steam_id, + name, + steam_id: steamId, }, selection: { success: true, @@ -293,8 +339,8 @@ export class SystemController { type: "mutation", action: "approveNameChange", variables: { - name: data.name, - steam_id: data.steam_id, + name, + steam_id: steamId, }, selection: { success: true, diff --git a/test/sanctions-policy.spec.ts b/test/sanctions-policy.spec.ts index 11f5fd543..0a184676e 100644 --- a/test/sanctions-policy.spec.ts +++ b/test/sanctions-policy.spec.ts @@ -30,6 +30,7 @@ describe("sanctions policy (SQL-driven)", () => { beforeEach(async () => { await postgres.query("DELETE FROM tournaments"); + await postgres.query("DELETE FROM matches"); await postgres.query("DELETE FROM match_options"); await postgres.query("DELETE FROM abandoned_matches"); await postgres.query("DELETE FROM player_sanctions"); @@ -69,6 +70,16 @@ describe("sanctions policy (SQL-driven)", () => { [steamId, agoInterval], ); + // What MatchAbandoned does for a real match: the same event can arrive more + // than once, so the insert has to be idempotent per match. + const abandonMatch = (steamId: string, matchId: string) => + postgres.query( + `INSERT INTO abandoned_matches (steam_id, match_id) + VALUES ($1::bigint, $2::uuid) + ON CONFLICT DO NOTHING`, + [steamId, matchId], + ); + const createTournament = async ( start = "1 day", { type = "Competitive", checkIn = false } = {}, @@ -240,6 +251,69 @@ describe("sanctions policy (SQL-driven)", () => { } }); + it("counts one match once, however many abandon events it sends", async () => { + const player = await fx.player(); + const match = await fx.match({ regions: ["TestSanctions"] }); + + await abandonMatch(player, match.id); + + // the plugin re-arms its disconnect timer per map and per reconnect, so + // the same player leaving one match can report it more than once. Each + // extra row would move them a rung up the ladder for a single offense. + await abandonMatch(player, match.id); + + const [row] = await postgres.query>( + "SELECT COUNT(*) AS count FROM abandoned_matches WHERE steam_id = $1::bigint", + [player], + ); + expect(Number(row.count)).toBe(1); + + const [last] = await postgres.query>( + "SELECT MAX(abandoned_at) AS last_abandoned_at FROM abandoned_matches WHERE steam_id = $1::bigint", + [player], + ); + const cooldown = await matchmakingCooldown(player); + + expect(minutesBetween(last.last_abandoned_at, cooldown!)).toBe( + HARDCODED_LADDER[0], + ); + }); + + it("still counts abandons from different matches separately", async () => { + const player = await fx.player(); + const first = await fx.match({ regions: ["TestSanctions"] }); + const second = await fx.match({ regions: ["TestSanctions"] }); + + await abandonMatch(player, first.id); + await abandonMatch(player, second.id); + + const [last] = await postgres.query>( + "SELECT MAX(abandoned_at) AS last_abandoned_at FROM abandoned_matches WHERE steam_id = $1::bigint", + [player], + ); + const cooldown = await matchmakingCooldown(player); + + expect(minutesBetween(last.last_abandoned_at, cooldown!)).toBe( + HARDCODED_LADDER[1], + ); + }); + + it("keeps counting match-less abandons separately", async () => { + const player = await fx.player(); + + // historical rows, and no-shows recorded before a match exists, carry no + // match_id at all - those must not collapse into one another + await abandon(player); + await abandon(player); + + const [row] = await postgres.query>( + "SELECT COUNT(*) AS count FROM abandoned_matches WHERE steam_id = $1::bigint", + [player], + ); + + expect(Number(row.count)).toBe(2); + }); + it("clamps past the end of the ladder instead of escalating forever", async () => { const player = await fx.player(); diff --git a/test/team-rosters.spec.ts b/test/team-rosters.spec.ts index 2b91375ae..b70d6f6fa 100644 --- a/test/team-rosters.spec.ts +++ b/test/team-rosters.spec.ts @@ -153,16 +153,66 @@ describe("teams, rosters and lineup membership (SQL-driven)", () => { expect(await getTeamCaptain(teamId)).toBe(owner); }); - it("removing the owner-captain from the roster leaves the team captainless", async () => { + // The owner is the team's last line of authority: can_change_team_role and + // can_remove_from_team both fall back to owner_steam_id, so a team whose + // owner has walked off the roster can only be managed by a site admin. + it("refuses to drop the owner from their own roster", async () => { const owner = await seedPlayer(); const teamId = await createTeam(owner); + await expect( + postgres.query( + "DELETE FROM team_roster WHERE team_id = $1 AND player_steam_id = $2", + [teamId, owner], + ), + ).rejects.toThrow(/owner/i); + + const roster = await postgres.query>( + "SELECT player_steam_id FROM team_roster WHERE team_id = $1", + [teamId], + ); + expect(roster).toHaveLength(1); + }); + + it("lets the old owner leave once ownership is handed over", async () => { + const owner = await seedPlayer(); + const heir = await seedPlayer(); + const teamId = await createTeam(owner); + + await asUser(owner, "admin", (query) => + query( + "INSERT INTO team_roster (team_id, player_steam_id) VALUES ($1, $2)", + [teamId, heir], + ), + ); + await postgres.query("UPDATE teams SET owner_steam_id = $1 WHERE id = $2", [ + heir, + teamId, + ]); + await postgres.query( "DELETE FROM team_roster WHERE team_id = $1 AND player_steam_id = $2", [teamId, owner], ); - expect(await getTeamCaptain(teamId)).toBeNull(); + expect(await getTeamCaptain(teamId)).toBe(heir); + }); + + it("still lets the whole team be deleted", async () => { + const owner = await seedPlayer(); + const teamId = await createTeam(owner); + + // the roster rows go with it by cascade, and the owner guard must not + // turn that into an error + await expect( + postgres.query("DELETE FROM teams WHERE id = $1", [teamId]), + ).resolves.not.toThrow(); + + const roster = await postgres.query>( + "SELECT player_steam_id FROM team_roster WHERE team_id = $1", + [teamId], + ); + expect(roster).toHaveLength(0); }); }); diff --git a/test/tournament-check-in-and-free-agents.spec.ts b/test/tournament-check-in-and-free-agents.spec.ts index 21fde650a..f89f9cfc4 100644 --- a/test/tournament-check-in-and-free-agents.spec.ts +++ b/test/tournament-check-in-and-free-agents.spec.ts @@ -1040,6 +1040,92 @@ describe("tournament check-in, registration rules and free agents (SQL-driven)", }); }); + // joined_tournament is what puts the tournament's chat room in a player's + // sidebar, so anyone who can talk in that room has to answer true here. + describe("joined_tournament", () => { + const joined = async (tournamentId: string, steamId: string) => { + const rows = await runAsUser(postgres, steamId, "user", async (query) => + (await query( + `SELECT joined_tournament(t, json_build_object('x-hasura-user-id', $2::text)) AS joined + FROM tournaments t WHERE t.id = $1`, + [tournamentId, steamId], + )) as Array<{ joined: boolean }>, + ); + + return rows[0].joined; + }; + + it("counts a player on a tournament team roster", async () => { + const t = await createTournament(); + const [owner, player] = await fx.players(2); + await registerTeam(t.id, "Rostered", [owner, player]); + + expect(await joined(t.id, player)).toBe(true); + }); + + it("counts a registered free agent", async () => { + const t = await createTournament({ + columns: { registration_type: "free_agents" }, + }); + const player = await fx.player(); + await registerFreeAgent(t.id, player); + + // until the draft runs there is no roster to be on, so without this the + // signup sees no tournament chat at all + expect(await joined(t.id, player)).toBe(true); + }); + + it("counts a waitlisted free agent", async () => { + const t = await createTournament({ + columns: { registration_type: "free_agents" }, + }); + const player = await fx.player(); + await registerFreeAgent(t.id, player); + await postgres.query( + `UPDATE tournament_free_agents SET status = 'waitlisted' + WHERE tournament_id = $1 AND player_steam_id = $2`, + [t.id, player], + ); + + expect(await joined(t.id, player)).toBe(true); + }); + + it("drops a free agent who withdrew", async () => { + const t = await createTournament({ + columns: { registration_type: "free_agents" }, + }); + const player = await fx.player(); + await registerFreeAgent(t.id, player); + await postgres.query( + `UPDATE tournament_free_agents SET status = 'withdrawn' + WHERE tournament_id = $1 AND player_steam_id = $2`, + [t.id, player], + ); + + expect(await joined(t.id, player)).toBe(false); + }); + + it("counts a team owner who is not on their own roster", async () => { + const t = await createTournament(); + const [owner, player] = await fx.players(2); + await registerTeam(t.id, "Owned", [owner, player]); + await postgres.query( + `DELETE FROM tournament_team_roster + WHERE tournament_id = $1 AND player_steam_id = $2`, + [t.id, owner], + ); + + expect(await joined(t.id, owner)).toBe(true); + }); + + it("says no to an unrelated player", async () => { + const t = await createTournament(); + const stranger = await fx.player(); + + expect(await joined(t.id, stranger)).toBe(false); + }); + }); + describe("a free agent who also owns a team", () => { it("does not stall the close of registration", async () => { const t = await createTournament({ diff --git a/test/utility-insights.spec.ts b/test/utility-insights.spec.ts index 62462f59b..44a0559a7 100644 --- a/test/utility-insights.spec.ts +++ b/test/utility-insights.spec.ts @@ -57,7 +57,8 @@ describe("utility insights (SQL-driven)", () => { await postgres.query("DELETE FROM utility_lineups"); await postgres.query("DELETE FROM match_map_demos"); await postgres.query("DELETE FROM matches"); - await postgres.query("DELETE FROM team_roster"); + // deleting the team takes its roster with it, and an owner cannot be + // dropped from a roster while the team still exists await postgres.query("DELETE FROM teams"); await postgres.query("DELETE FROM players"); });