Fix v1.14.0 prod errors (#999)

* Fix bad reference to _player

* Fix duplicate key error for social_add

* Fix duplicate key error for coop_leaderboard

* Add user agent to ClientError log message

This would help to keep track of which errors are relevant to the latest 
client build and which can be ignored because they have already been 
fixed or are coming from an alternate client implementation.

server/gameconnection.py

@@ -288,7 +288,11 @@ async def handle_game_result(self, army: Any, result: Any):
             self._logger.warning("Invalid result for %s reported: %s", army, result)
         else:
             await self.game.add_result(
-                self.player.id, army, result_type, int(score), frozenset(metadata)
+                self.player.id,
+                army,
+                result_type,
+                int(score),
+                frozenset(metadata),
             )
 
     async def handle_operation_complete(
@@ -332,25 +336,26 @@ async def handle_operation_complete(
 
             # Each player in a co-op game will send the OperationComplete
             # message but we only need to perform this insert once
-            if not self.game.leaderboard_saved:
-                await conn.execute(
-                    coop_leaderboard.insert().values(
-                        mission=mission,
-                        gameuid=self.game.id,
-                        secondary=secondary,
-                        time=delta,
-                        player_count=len(self.game.players),
+            async with self.game.leaderboard_lock:
+                if not self.game.leaderboard_saved:
+                    await conn.execute(
+                        coop_leaderboard.insert().values(
+                            mission=mission,
+                            gameuid=self.game.id,
+                            secondary=secondary,
+                            time=delta,
+                            player_count=len(self.game.players),
+                        )
                     )
-                )
-                self.game.leaderboard_saved = True
+                    self.game.leaderboard_saved = True
 
     async def handle_json_stats(self, stats: str):
         try:
             self.game.report_army_stats(stats)
         except json.JSONDecodeError as e:
             self._logger.warning(
                 "Malformed game stats reported by %s: '...%s...'",
-                self._player.login,
+                self.player.login,
                 stats[e.pos-20:e.pos+20]
             )
 

server/games/coop.py

@@ -1,3 +1,5 @@
+import asyncio
+
 from .game import Game
 from .typedefs import FA, GameType, InitMode, ValidityState, Victory
 
@@ -19,6 +21,7 @@ def __init__(self, *args, **kwargs):
             "Difficulty": 3,
             "Expansion": "true"
         })
+        self.leaderboard_lock = asyncio.Lock()
         self.leaderboard_saved = False
 
     async def validate_game_mode_settings(self):

server/lobbyconnection.py

@@ -189,7 +189,11 @@ async def on_message_received(self, message):
             })
             await self.abort(e.message())
         except ClientError as e:
-            self._logger.warning("Client error: %s", e.message)
+            self._logger.warning(
+                "ClientError[%s]: %s",
+                self.user_agent,
+                e.message,
+            )
             await self.send({
                 "command": "notice",
                 "style": "error",
@@ -333,6 +337,9 @@ async def command_social_add(self, message):
         else:
             return
 
+        if subject_id in player_attr:
+            return
+
         async with self._db.acquire() as conn:
             await conn.execute(friends_and_foes.insert().values(
                 user_id=self.player.id,

tests/conftest.py

@@ -160,7 +160,7 @@ def opt(val):
         user=user,
         password=pw or "",
         port=port,
-        db=name
+        db=name,
     )
 
 
@@ -183,7 +183,7 @@ def opt(val):
             user=user,
             password=pw or "",
             port=port,
-            db=name
+            db=name,
         )
         await db.connect()
 
@@ -211,7 +211,7 @@ async def game(database, players):
 
 
 GAME_UID = 1
-COOP_GAME_UID = 1
+COOP_GAME_UID = 2
 
 
 @pytest.fixture

tests/unit_tests/test_gameconnection.py

@@ -475,6 +475,14 @@ async def test_json_stats_malformed(
     await game_connection.handle_action("JsonStats", ['{"stats": {}'])
 
 
+async def test_handle_json_stats_malformed(
+    real_game: Game,
+    game_connection: GameConnection,
+):
+    game_connection.game = real_game
+    await game_connection.handle_json_stats('{"stats": {}')
+
+
 async def test_handle_action_EnforceRating(
     game: Game,
     game_connection: GameConnection
@@ -519,9 +527,7 @@ async def test_handle_action_TeamkillHappened(
 
 
 async def test_handle_action_TeamkillHappened_AI(
-    game: Game,
     game_connection: GameConnection,
-    database
 ):
     # Should fail with a sql constraint error if this isn't handled correctly
     game_connection.abort = mock.AsyncMock()
@@ -667,13 +673,12 @@ async def test_handle_action_OperationComplete_duplicate(
         )
 
     with caplog.at_level(logging.ERROR):
-        await game_connection.handle_action(
-            "OperationComplete", [1, 1, time_taken]
-        )
-        caplog.clear()
-        await game_connection.handle_action(
-            "OperationComplete", [1, 1, time_taken]
-        )
+        await asyncio.gather(*(
+            game_connection.handle_action(
+                "OperationComplete", [1, 1, time_taken]
+            )
+            for _ in range(10)
+        ))
 
         assert not any(
             record.exc_info

tests/unit_tests/test_lobbyconnection.py

@@ -610,7 +610,7 @@ async def test_command_avatar_list(mocker, lobbyconnection: LobbyConnection):
     })
 
 
-async def test_command_avatar_select(mocker, database, lobbyconnection: LobbyConnection):
+async def test_command_avatar_select(database, lobbyconnection: LobbyConnection):
     lobbyconnection.player.id = 2  # Dostya test user
 
     await lobbyconnection.on_message_received({
@@ -656,6 +656,24 @@ async def test_command_social_add_friend(lobbyconnection, database):
     assert lobbyconnection.player.friends == {2}
 
 
+async def test_command_social_add_friend_idempotent(lobbyconnection, database):
+    lobbyconnection.player.id = 1
+
+    friends = await get_friends(lobbyconnection.player.id, database)
+    assert friends == []
+    assert lobbyconnection.player.friends == set()
+
+    for _ in range(5):
+        await lobbyconnection.command_social_add({
+            "command": "social_add",
+            "friend": 2
+        })
+
+    friends = await get_friends(lobbyconnection.player.id, database)
+    assert friends == [2]
+    assert lobbyconnection.player.friends == {2}
+
+
 async def test_command_social_remove_friend(lobbyconnection, database):
     lobbyconnection.player.id = 2
 
@@ -672,11 +690,19 @@ async def test_command_social_remove_friend(lobbyconnection, database):
     assert friends == []
     assert lobbyconnection.player.friends == set()
 
-    # Removing twice does nothing
-    await lobbyconnection.on_message_received({
-        "command": "social_remove",
-        "friend": 1
-    })
+
+async def test_command_social_remove_friend_idempotent(lobbyconnection, database):
+    lobbyconnection.player.id = 2
+
+    friends = await get_friends(lobbyconnection.player.id, database)
+    assert friends == [1]
+    lobbyconnection.player.friends = {1}
+
+    for _ in range(5):
+        await lobbyconnection.command_social_remove({
+            "command": "social_remove",
+            "friend": 1
+        })
 
     friends = await get_friends(lobbyconnection.player.id, database)
     assert friends == []