diff --git a/server/ladder_service/violation_service.py b/server/ladder_service/violation_service.py index 4787b8159..2ef1a9030 100644 --- a/server/ladder_service/violation_service.py +++ b/server/ladder_service/violation_service.py @@ -63,24 +63,25 @@ class ViolationService(Service): """ def __init__(self): - self.violations: dict[Player, Violation] = {} + # We store a reference to the original `Player` object for logging only + self._violations: dict[int, tuple[Player, Violation]] = {} async def initialize(self): self._cleanup_task = at_interval(5, func=self.clear_expired) def clear_expired(self): now = datetime_now() - for player, violation in list(self.violations.items()): + for player, violation in list(self._violations.values()): if violation.is_expired(now): self._clear_violation(player) def register_violations(self, players: list[Player]): now = datetime_now() for player in players: - violation = self.violations.get(player) + violation = self.get_violation(player) if violation is None or violation.is_expired(now): violation = Violation(time=now) - self.violations[player] = violation + self.set_violation(player, violation) else: violation.register() @@ -109,7 +110,7 @@ def get_violations(self, players: list[Player]) -> dict[Player, Violation]: now = datetime_now() result = {} for player in players: - violation = self.violations.get(player) + violation = self.get_violation(player) if not violation: continue elif violation.get_ban_expiration() > now: @@ -119,11 +120,18 @@ def get_violations(self, players: list[Player]) -> dict[Player, Violation]: return result + def get_violation(self, player: Player) -> Optional[Violation]: + _, violation = self._violations.get(player.id, (None, None)) + return violation + + def set_violation(self, player: Player, violation: Violation): + self._violations[player.id] = (player, violation) + def _clear_violation(self, player: Player): - violation = self.violations.get(player) + violation = self.get_violation(player) self._logger.debug( "Cleared violation for player %s: %s", player.login, violation ) - del self.violations[player] + del self._violations[player.id] diff --git a/tests/integration_tests/test_matchmaker.py b/tests/integration_tests/test_matchmaker.py index 89b762184..e2ff15ffb 100644 --- a/tests/integration_tests/test_matchmaker.py +++ b/tests/integration_tests/test_matchmaker.py @@ -2,7 +2,6 @@ import math import re from collections import deque -from datetime import datetime, timezone import pytest from sqlalchemy import select @@ -24,8 +23,7 @@ queue_players_for_matchmaking, queue_temp_players_for_matchmaking, read_until_launched, - send_player_options, - start_search + send_player_options ) @@ -568,105 +566,3 @@ async def test_search_info_messages(lobby_server): with pytest.raises(asyncio.TimeoutError): await read_until_command(proto, "search_info", timeout=5) - - -@fast_forward(360) -async def test_failed_start_ban_guest(mocker, lobby_server): - mock_now = mocker.patch( - "server.ladder_service.violation_service.datetime_now", - return_value=datetime(2022, 2, 5, tzinfo=timezone.utc) - ) - _, host, guest_id, guest = await queue_players_for_matchmaking(lobby_server) - - # The player that queued last will be the host - async def launch_game_and_timeout_guest(): - await read_until_command(host, "game_launch") - await open_fa(host) - await read_until_command(host, "game_info") - - await read_until_command(guest, "game_launch") - await read_until_command(guest, "match_cancelled", timeout=120) - await read_until_command(host, "match_cancelled") - await host.send_message({ - "command": "GameState", - "target": "game", - "args": ["Ended"] - }) - - await launch_game_and_timeout_guest() - - # Second time searching there is no ban - await start_search(host) - await start_search(guest) - await launch_game_and_timeout_guest() - - # Third time searching there is a short ban - await guest.send_message({ - "command": "game_matchmaking", - "state": "start", - "queue_name": "ladder1v1" - }) - - msg = await read_until_command(guest, "search_timeout") - assert msg == { - "command": "search_timeout", - "timeouts": [{ - "player": guest_id, - "expires_at": "2022-02-05T00:10:00+00:00" - }] - } - - mock_now.return_value = datetime(2022, 2, 5, 0, 10, tzinfo=timezone.utc) - await asyncio.sleep(1) - - # Third successful search - await start_search(host) - await start_search(guest) - await launch_game_and_timeout_guest() - - # Fourth time searching there is a long ban - await guest.send_message({ - "command": "game_matchmaking", - "state": "start", - "queue_name": "ladder1v1" - }) - - msg = await read_until_command(guest, "search_timeout") - assert msg == { - "command": "search_timeout", - "timeouts": [{ - "player": guest_id, - "expires_at": "2022-02-05T00:40:00+00:00" - }] - } - - mock_now.return_value = datetime(2022, 2, 5, 0, 40, tzinfo=timezone.utc) - await asyncio.sleep(1) - - # Fourth successful search - await start_search(host) - await start_search(guest) - await launch_game_and_timeout_guest() - - # Fifth time searching there is a long ban - await guest.send_message({ - "command": "game_matchmaking", - "state": "start", - "queue_name": "ladder1v1" - }) - - msg = await read_until_command(guest, "search_timeout") - assert msg == { - "command": "search_timeout", - "timeouts": [{ - "player": guest_id, - "expires_at": "2022-02-05T01:10:00+00:00" - }] - } - - msg = await read_until_command(guest, "notice") - assert msg == { - "command": "notice", - "style": "info", - "text": "Player ladder2 is timed out for 30 minutes" - } diff --git a/tests/integration_tests/test_matchmaker_violations.py b/tests/integration_tests/test_matchmaker_violations.py new file mode 100644 index 000000000..542df8452 --- /dev/null +++ b/tests/integration_tests/test_matchmaker_violations.py @@ -0,0 +1,222 @@ +import asyncio +from datetime import datetime, timezone + +from tests.utils import fast_forward + +from .conftest import connect_and_sign_in, read_until_command +from .test_game import open_fa, queue_players_for_matchmaking, start_search +from .test_parties import accept_party_invite, invite_to_party + + +@fast_forward(360) +async def test_violation_for_guest_timeout(mocker, lobby_server): + mock_now = mocker.patch( + "server.ladder_service.violation_service.datetime_now", + return_value=datetime(2022, 2, 5, tzinfo=timezone.utc) + ) + _, host, guest_id, guest = await queue_players_for_matchmaking(lobby_server) + + # The player that queued last will be the host + async def launch_game_and_timeout_guest(): + await read_until_command(host, "game_launch") + await open_fa(host) + await read_until_command(host, "game_info") + + await read_until_command(guest, "game_launch") + await read_until_command(guest, "match_cancelled", timeout=120) + await read_until_command(host, "match_cancelled") + await host.send_message({ + "command": "GameState", + "target": "game", + "args": ["Ended"] + }) + + await launch_game_and_timeout_guest() + + # Second time searching there is no ban + await start_search(host) + await start_search(guest) + await launch_game_and_timeout_guest() + + # Third time searching there is a short ban + await guest.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(guest, "search_timeout") + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": guest_id, + "expires_at": "2022-02-05T00:10:00+00:00" + }] + } + + mock_now.return_value = datetime(2022, 2, 5, 0, 10, tzinfo=timezone.utc) + await asyncio.sleep(1) + + # Third successful search + await start_search(host) + await start_search(guest) + await launch_game_and_timeout_guest() + + # Fourth time searching there is a long ban + await guest.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(guest, "search_timeout") + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": guest_id, + "expires_at": "2022-02-05T00:40:00+00:00" + }] + } + + mock_now.return_value = datetime(2022, 2, 5, 0, 40, tzinfo=timezone.utc) + await asyncio.sleep(1) + + # Fourth successful search + await start_search(host) + await start_search(guest) + await launch_game_and_timeout_guest() + + # Fifth time searching there is a long ban + await guest.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(guest, "search_timeout") + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": guest_id, + "expires_at": "2022-02-05T01:10:00+00:00" + }] + } + + msg = await read_until_command(guest, "notice") + assert msg == { + "command": "notice", + "style": "info", + "text": "Player ladder2 is timed out for 30 minutes" + } + + +@fast_forward(360) +async def test_violation_persisted_across_logins(mocker, lobby_server): + mocker.patch( + "server.ladder_service.violation_service.datetime_now", + return_value=datetime(2022, 2, 5, tzinfo=timezone.utc) + ) + host_id, host, _, guest = await queue_players_for_matchmaking(lobby_server) + + await read_until_command(host, "match_cancelled", timeout=120) + await read_until_command(guest, "match_cancelled", timeout=10) + + # Second time searching there is no ban + await start_search(host) + await start_search(guest) + await read_until_command(host, "match_cancelled", timeout=120) + await read_until_command(guest, "match_cancelled", timeout=10) + + # Third time searching there is a short ban + await host.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(host, "search_timeout", timeout=10) + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": host_id, + "expires_at": "2022-02-05T00:10:00+00:00" + }] + } + + await host.close() + + _, _, host = await connect_and_sign_in( + ("ladder1", "ladder1"), + lobby_server + ) + + # Player should still be banned after re-logging + await host.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(host, "search_timeout", timeout=10) + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": host_id, + "expires_at": "2022-02-05T00:10:00+00:00" + }] + } + + +@fast_forward(360) +async def test_violation_persisted_across_parties(mocker, lobby_server): + mocker.patch( + "server.ladder_service.violation_service.datetime_now", + return_value=datetime(2022, 2, 5, tzinfo=timezone.utc) + ) + host_id, host, guest_id, guest = await queue_players_for_matchmaking(lobby_server) + + await read_until_command(host, "match_cancelled", timeout=120) + await read_until_command(guest, "match_cancelled", timeout=10) + + # Second time searching there is no ban + await start_search(host) + await start_search(guest) + await read_until_command(host, "match_cancelled", timeout=120) + await read_until_command(guest, "match_cancelled", timeout=10) + + # Third time searching there is a short ban + await host.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "ladder1v1" + }) + + msg = await read_until_command(host, "search_timeout", timeout=10) + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": host_id, + "expires_at": "2022-02-05T00:10:00+00:00" + }] + } + + await invite_to_party(guest, host_id) + await read_until_command(host, "party_invite", timeout=10) + await accept_party_invite(host, guest_id) + await read_until_command(guest, "update_party", timeout=10) + + # Guest should not be able to queue when player in their party has a ban + await guest.send_message({ + "command": "game_matchmaking", + "state": "start", + "queue_name": "tmm2v2" + }) + + msg = await read_until_command(host, "search_timeout", timeout=10) + assert msg == { + "command": "search_timeout", + "timeouts": [{ + "player": host_id, + "expires_at": "2022-02-05T00:10:00+00:00" + }] + } diff --git a/tests/unit_tests/test_violation_service.py b/tests/unit_tests/test_violation_service.py index df55afc12..c338faf2c 100644 --- a/tests/unit_tests/test_violation_service.py +++ b/tests/unit_tests/test_violation_service.py @@ -60,12 +60,13 @@ def test_violation_clear_expired( v1 = Violation(time=NOW - timedelta(hours=1)) v2 = Violation(time=NOW) - violation_service.violations[p1] = v1 - violation_service.violations[p2] = v2 + violation_service.set_violation(p1, v1) + violation_service.set_violation(p2, v2) violation_service.clear_expired() - assert violation_service.violations == {p2: v2} + assert violation_service.get_violation(p1) is None + assert violation_service.get_violation(p2) == v2 def test_register_violation( @@ -89,7 +90,21 @@ def test_get_violations_clears_expired( player_factory ): p1 = player_factory("Test3", player_id=1) + v1 = Violation(time=NOW - timedelta(hours=1)) - violation_service.violations[p1] = Violation(time=NOW - timedelta(hours=1)) + violation_service.set_violation(p1, v1) violation_service.get_violations([p1]) - assert violation_service.violations == {} + assert violation_service.get_violation(p1) is None + + +def test_violation_tracked_by_player_id( + violation_service: ViolationService, + player_factory +): + p1 = player_factory("Test", player_id=1) + p2 = player_factory("Test_Renamed", player_id=1) + + v1 = Violation(time=NOW - timedelta(hours=1)) + + violation_service.set_violation(p1, v1) + assert violation_service.get_violation(p2) == v1