Already in game matchmaking (#361)

* handle players already in game in matchmaking * 333 secure front game (#339) * update component event listener * secure game with toasts * Fixed PlayerManager memory not being shared by all workers by adding a redis cache * Fixed healthcheck number of retries pong-server-cache --------- Co-authored-by: Mururoahh <[email protected]> Co-authored-by: Tom Damerose <[email protected]>
tdameros · Mar 3, 2024 · 1e2366b · 1e2366b
1 parent 9093fe5
commit 1e2366b
Show file tree

Hide file tree

Showing 18 changed files with 106 additions and 43 deletions.
diff --git a/.github/workflows/pong-server.yaml b/.github/workflows/pong-server.yaml
@@ -60,6 +60,7 @@ jobs:
                   export PONG_GAME_SERVERS_MAX_PORT=42210
                   export GAME_SERVER_PATH=`pwd`/pong_server/src/game_server/
                   export PATH_TO_SSL_CERTS=`pwd`/ssl/certs/
+                  export DEBUG=True
 
                   cd pong_server/src/game_creator/
                   python3 manage.py test

diff --git a/docker-compose.yaml b/docker-compose.yaml
@@ -100,6 +100,10 @@ services:
         extends:
             file: pong_server/docker/docker-compose.yaml
             service: pong-server
+    pong-server-cache:
+        extends:
+            file: pong_server/docker/docker-compose.yaml
+            service: pong-server-cache
     pong-server-nginx:
         extends:
             file: pong_server/docker/docker-compose.yaml

diff --git a/matchmaking/src/error_message.py b/matchmaking/src/error_message.py
@@ -3,3 +3,4 @@
 GAME_CREATOR_CONNECT_ERROR = 'Could not connect to game-creator'
 GAME_CREATOR_CREATE_GAME_ERROR = 'Could not create game'
 ALREADY_IN_QUEUE = 'User is already in the queue'
+ALREADY_IN_GAME_ERROR = 'User is already in the game'
diff --git a/matchmaking/src/server.py b/matchmaking/src/server.py
@@ -45,12 +45,12 @@ async def connect(self, sid, environ, auth):
         logging.debug(f'New connection: {sid}')
         user, errors = authenticate_user(auth)
         if user is None:
-            logging.debug(str(errors))
+            logging.error(str(errors))
             raise socketio.exceptions.ConnectionRefusedError(str(errors))
         player = Player(sid, user['id'], user['elo'])
         error = self.matchmaking.add_player(player)
         if error is not None:
-            logging.debug(error)
+            logging.error(error)
             raise socketio.exceptions.ConnectionRefusedError(error)
         logging.debug(f'User added to the queue: {user}')
         return True
@@ -64,10 +64,9 @@ async def send_match_uri(self, player1: Player, player2: Player, data: Any) -> N
         await self.sio.emit('match', data, room=player1.sid)
         await self.sio.emit('match', data, room=player2.sid)
 
-    async def send_error(self, player_1: Player, player_2: Player, error_message: str) -> None:
+    async def send_error(self, player: Player, error_message: str) -> None:
         data = {'error': error_message}
-        await self.sio.emit('error', data, room=player_1.sid)
-        await self.sio.emit('error', data, room=player_2.sid)
+        await self.sio.emit('error', data, room=player.sid)
 
     async def send_match(self, player_1: Player, player_2: Player) -> None:
         data = {
@@ -83,27 +82,43 @@ async def send_match(self, player_1: Player, player_2: Player) -> None:
                 common_settings.GAME_CREATOR_CREATE_GAME_ENDPOINT,
                 data=json.dumps(data),
             )
+            logging.debug(f'response: {response.text}')
         except requests.exceptions.RequestException as e:
-            logging.debug(e)
+            logging.error(e)
             await self.disconnect_players(player_1, player_2, error.GAME_CREATOR_CONNECT_ERROR)
             return
-        if not response.ok:
-            logging.debug(f'Request failed: {response.text}')
+        if response.status_code == 409:
+            logging.error(f'Request failed: {response.text}')
+            body = response.json()
+            for player_id in body['players_already_in_a_game']:
+                if player_id == player_1.user_id:
+                    await self.disconnect_player(player_1, error.ALREADY_IN_GAME_ERROR)
+                elif player_id == player_2.user_id:
+                    await self.disconnect_player(player_2, error.ALREADY_IN_GAME_ERROR)
+            return
+        elif not response.ok:
+            logging.error(f'Request failed: {response.text}')
             if response.status_code != 503:
                 await self.disconnect_players(player_1, player_2, error.GAME_CREATOR_CREATE_GAME_ERROR)
             return
         await self.send_match_uri(player_1, player_2, response.json())
         await self.remove_players(player_1, player_2)
 
+    async def disconnect_player(self, player: Player, error_message: str) -> None:
+        await self.send_error(player, error_message)
+        await self.remove_player(player)
+
     async def disconnect_players(self, player_1: Player, player_2: Player, error_message: str) -> None:
-        await self.send_error(player_1, player_2, error_message)
-        await self.remove_players(player_1, player_2)
+        await self.disconnect_player(player_1, error_message)
+        await self.disconnect_player(player_2, error_message)
+
+    async def remove_player(self, player: Player) -> None:
+        self.matchmaking.remove_player(player.sid)
+        await self.sio.disconnect(player.sid)
 
     async def remove_players(self, player_1: Player, player_2: Player):
-        self.matchmaking.remove_player(player_1.sid)
-        self.matchmaking.remove_player(player_2.sid)
-        await self.sio.disconnect(player_1.sid)
-        await self.sio.disconnect(player_2.sid)
+        await self.remove_player(player_1)
+        await self.remove_player(player_2)
 
     async def start_matchmaking(self, app: web.Application) -> None:
         self.sio.start_background_task(self.matchmaking.routine)
diff --git a/pong_server/dev_game_creator.sh b/pong_server/dev_game_creator.sh
@@ -4,7 +4,6 @@
 export PONG_GAME_SERVERS_MIN_PORT=42200
 export PONG_GAME_SERVERS_MAX_PORT=42210
 export GAME_SERVER_PATH=~/git/transcendence/pong_server/src/game_server/
-
-./test_game_creator.sh || exit $?
+export DEBUG=False
 
 (cd src/game_creator/ && python3 manage.py runserver)
diff --git a/pong_server/doc/game_creator.md b/pong_server/doc/game_creator.md
@@ -44,7 +44,7 @@
 ## `/game_creator/remove_players_current_game/`
 
 <details>
- <summary><code>DELETE</code></summary>
+ <summary><code>POST</code></summary>
 
 ### Info
 > This should only be used by game_server

diff --git a/pong_server/docker/docker-compose.yaml b/pong_server/docker/docker-compose.yaml
@@ -41,9 +41,23 @@ services:
             - ssl_certs:/app/ssl
             - common_code:/app/common
         command: /app/run.sh
+        depends_on:
+            pong-server-cache:
+                condition: service_healthy
         healthcheck:
             test: [ "CMD-SHELL", "curl -kf https://localhost:8000/health/" ]
             interval: 5s
             timeout: 5s
             retries: 60
         restart: on-failure
+
+    pong-server-cache:
+        networks:
+            - pong_server
+        image: redis:7.2.4-bookworm
+        healthcheck:
+            test: [ "CMD-SHELL", "redis-cli", "--raw", "INCR", "ping" ]
+            interval: 5s
+            timeout: 5s
+            retries: 60
+        restart: on-failure
diff --git a/pong_server/docker/requirements.txt b/pong_server/docker/requirements.txt
@@ -1,11 +1,13 @@
 aiohttp==3.9.1
 aiosignal==1.3.1
 asgiref==3.7.2
+async-timeout==4.0.3
 attrs==23.1.0
 bidict==0.22.1
 colorlog==6.8.0
 Django==4.2.7
 django-cors-headers==4.3.1
+django-redis==3.1.6
 frozenlist==1.4.0
 gunicorn==21.2.0
 h11==0.14.0
@@ -15,6 +17,7 @@ numpy==1.26.2
 packaging==23.2
 python-engineio==4.8.0
 python-socketio==5.10.0
+redis==5.0.2
 simple-websocket==1.0.0
 sqlparse==0.4.4
 wsproto==1.2.0

diff --git a/pong_server/docker/run.sh b/pong_server/docker/run.sh
@@ -22,8 +22,6 @@ cp -r /app/common/ /app/src/game_creator/common
 rm -rf /app/src/game_server/common
 cp -r /app/common/ /app/src/game_server/common
 
-# Copy the .env to the game_creator folder
-cp /app/game_creator/.env /app/src/game_creator/.env
 
 # Run game_creator
 gunicorn -c /app/gunicorn.conf.py
diff --git a/pong_server/src/game_creator/api/PlayerManager.py b/pong_server/src/game_creator/api/PlayerManager.py
@@ -1,21 +1,24 @@
 from typing import Optional
 
+from django.core.cache import cache
 
-class PlayerManager(object):
-    #       dict[user_id, game_port]
-    _users: dict[int, int] = {}
 
+class PlayerManager(object):
     @staticmethod
     def add_players(players: list[Optional[int]], game_port: int) -> None:
         for player in players:
             if player is not None:
-                PlayerManager._users[player] = game_port
+                cache.set(player, game_port)
 
     @staticmethod
     def remove_players(players: list[int]) -> None:
         for player in players:
-            PlayerManager._users.pop(player, None)
+            cache.delete(player)
 
     @staticmethod
     def get_player_game_port(player: int) -> Optional[int]:
-        return PlayerManager._users.get(player)
+        return cache.get(player)
+
+    @staticmethod
+    def clear():
+        cache.clear()
diff --git a/pong_server/src/game_creator/api/tests/test_CreateGameView.py b/pong_server/src/game_creator/api/tests/test_CreateGameView.py
@@ -38,7 +38,7 @@ def post_request(self,
                 raise Exception(f'Failed to generate jwt: {errors}')
 
         if reset_player_manager:
-            PlayerManager._users = {}
+            PlayerManager.clear()
 
         response = self.client.post(
             url,

diff --git a/pong_server/src/game_creator/api/tests/test_GetMyGamePortView.py b/pong_server/src/game_creator/api/tests/test_GetMyGamePortView.py
@@ -31,7 +31,7 @@ def run_test(self,
 
     @patch('common.src.jwt_managers.UserAccessJWTDecoder.authenticate')
     def test_valid_request_player_in_game(self, mock_authenticate):
-        PlayerManager._users = {}
+        PlayerManager.clear()
         player_id = 1
         port = 4242
         PlayerManager.add_players([player_id], port)
@@ -42,7 +42,7 @@ def test_valid_request_player_in_game(self, mock_authenticate):
 
     @patch('common.src.jwt_managers.UserAccessJWTDecoder.authenticate')
     def test_valid_request_player_not_in_game(self, mock_authenticate):
-        PlayerManager._users = {}
+        PlayerManager.clear()
         payload = {'user_id': 1}
         mock_authenticate.return_value = (True, payload, None)
 

diff --git a/pong_server/src/game_creator/api/tests/test_PostGetPlayersGamePortView.py b/pong_server/src/game_creator/api/tests/test_PostGetPlayersGamePortView.py
@@ -43,7 +43,7 @@ def run_test(self,
         self.assertEqual(status, expected_status)
 
     def test_valid_request(self):
-        PlayerManager._users = {}
+        PlayerManager.clear()
         player_1_port = 4242
         player_2_port = 42322
         PlayerManager.add_players([1], player_1_port)

diff --git a/pong_server/src/game_creator/api/tests/test_RemovePlayersCurrentGameView.py b/pong_server/src/game_creator/api/tests/test_RemovePlayersCurrentGameView.py
@@ -9,21 +9,21 @@
 from common.src.jwt_managers import ServiceAccessJWT
 
 
-class DeleteRemovePlayersCurrentGameViewTest(TestCaseNoDatabase):
-    def delete_request(self,
-                       request_body,
-                       jwt: Optional[str],
-                       reset_player_manager: bool) -> (dict, int):
+class PostRemovePlayersCurrentGameViewTest(TestCaseNoDatabase):
+    def post_request(self,
+                     request_body,
+                     jwt: Optional[str],
+                     reset_player_manager: bool) -> (dict, int):
         url = reverse('remove_players_current_game')
         if jwt is None:
             success, jwt, errors = ServiceAccessJWT.generate_jwt()
             if not success:
                 raise Exception(f'Failed to generate jwt: {errors}')
 
         if reset_player_manager:
-            PlayerManager._users = {}
+            PlayerManager.clear()
 
-        response = self.client.delete(
+        response = self.client.post(
             url,
             json.dumps(request_body),
             content_type='application/json',
@@ -40,7 +40,7 @@ def run_test(self,
                  expected_status,
                  jwt: Optional[str] = None,
                  reset_player_manager: bool = True):
-        body, status = self.delete_request(request_body, jwt, reset_player_manager)
+        body, status = self.post_request(request_body, jwt, reset_player_manager)
 
         self.assertEqual(status, expected_status)
         self.assertEqual(body, expected_body)
@@ -54,6 +54,13 @@ def test_valid_request(self):
 
         self.assertEqual(PlayerManager.get_player_game_port(1), None)
 
+    def test_valid_request_no_players_are_in_a_game(self):
+        self.run_test({
+            'players': [1, 2],
+        }, {}, 204)
+
+        self.assertEqual(PlayerManager.get_player_game_port(1), None)
+
     def test_no_jwt(self):
         self.run_test({
             'players': [1, 4],

diff --git a/pong_server/src/game_creator/api/views/CreateGameView.py b/pong_server/src/game_creator/api/views/CreateGameView.py
@@ -29,8 +29,7 @@ def post(self, request):
 
             PlayerManager.add_players(players, port)
 
-            return JsonResponse({'port': port},
-                                status=201)
+            return JsonResponse({'port': port}, status=201)
 
         except JsonResponseException as json_response_exception:
             return json_response_exception.to_json_response()

diff --git a/pong_server/src/game_creator/api/views/RemovePlayersCurrentGameView.py b/pong_server/src/game_creator/api/views/RemovePlayersCurrentGameView.py
@@ -10,9 +10,9 @@
 
 
 @method_decorator(csrf_exempt, name='dispatch')
-@method_decorator(service_authentication(['DELETE']), name='dispatch')
+@method_decorator(service_authentication(['POST']), name='dispatch')
 class RemovePlayersCurrentGameView(View):
-    def delete(self, request):
+    def post(self, request):
         try:
             players: list[int] = get_player_list(request)
 

diff --git a/pong_server/src/game_creator/game_creator/settings.py b/pong_server/src/game_creator/game_creator/settings.py
@@ -29,7 +29,7 @@
 SECRET_KEY = os.getenv('GAME_CREATOR_SECRET_KEY')
 
 # SECURITY WARNING: don't run with debug turned on in production!
-DEBUG = False
+DEBUG = os.getenv('DEBUG') == 'True'
 
 ALLOWED_HOSTS = ['*']
 
@@ -77,6 +77,24 @@
 
 DATABASES = {}
 
+CACHE_TIMEOUT = 3600  # In seconds (1 hour)
+if DEBUG:
+    CACHES = {
+        'default': {
+            'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+            'LOCATION': 'unix:/tmp/memcached.sock',
+            'TIMEOUT': CACHE_TIMEOUT,
+        }
+    }
+else:
+    CACHES = {
+        'default': {
+            'BACKEND': 'django.core.cache.backends.redis.RedisCache',
+            'LOCATION': 'redis://pong-server-cache:6379/1',
+            'TIMEOUT': CACHE_TIMEOUT,
+        }
+    }
+
 # Password validation
 # https://docs.djangoproject.com/en/4.2/ref/settings/#auth-password-validators
 

diff --git a/pong_server/test_game_creator.sh b/pong_server/test_game_creator.sh
@@ -4,8 +4,9 @@
     export PONG_GAME_SERVERS_MAX_PORT=42210
     export GAME_SERVER_PATH=~/git/transcendence/pong_server/src/game_server/
     export PATH_TO_SSL_CERTS=~/git/transcendence/ssl/certs/
+    export DEBUG=True
 
-    cd src/game_creator/ &&
+    cd src/game_creator/
     python3 manage.py test
 )
 exit_code=$?