Add Game id column to journal (#66)

* Add game id column to league score journal

* Remove constraint that was not enforced in the database

* Fix isort

* Fix integration test

* Use NewType

* Fix isort

migrations/V018__add_game_id_column_to_journal.sql

@@ -0,0 +1,2 @@
+ALTER table league_score_journal
+ADD COLUMN `game_id` INTEGER NOT NULL AFTER id;

service/db/models.py

@@ -76,7 +76,8 @@
     "league_score_journal",
     metadata,
     Column("id", Integer, primary_key=True),
-    Column("login_id", Integer, ForeignKey("login.id")),
+    Column("game_id", Integer),
+    Column("login_id", Integer),
     Column("league_season_id", Integer, ForeignKey("league_season.id")),
     Column("subdivision_id_before", Integer, ForeignKey("league_season_division_subdivision.id")),
     Column("subdivision_id_after", Integer, ForeignKey("league_season_division_subdivision.id")),

service/league_service/league_service.py

@@ -19,8 +19,9 @@
 from service.metrics import league_service_backlog
 
 from .league_rater import LeagueRater
-from .typedefs import (InvalidScoreError, League, LeagueDivision,
-                       LeagueRatingRequest, LeagueScore, ServiceNotReadyError)
+from .typedefs import (GameID, InvalidScoreError, League, LeagueDivision,
+                       LeagueRatingRequest, LeagueScore, PlayerID,
+                       ServiceNotReadyError)
 
 
 @with_logger
@@ -142,11 +143,11 @@ async def _rate_single_league(self, league: League, request: LeagueRatingRequest
             league, old_score, request.outcome, request.rating
         )
         await self._persist_score(
-            request.player_id, league.current_season_id, old_score, new_score
+            request.game_id, request.player_id, league.current_season_id, old_score, new_score
         )
-        await self._broadcast_score_change(request.player_id, league, new_score)
+        await self._broadcast_score_change(request.game_id, request.player_id, league, new_score)
 
-    async def _load_score(self, player_id: int, league: League) -> LeagueScore:
+    async def _load_score(self, player_id: PlayerID, league: League) -> LeagueScore:
         async with self._db.acquire() as conn:
             sql = select([league_season_score]).where(
                 and_(
@@ -167,7 +168,7 @@ async def _load_score(self, player_id: int, league: League) -> LeagueScore:
             row[league_season_score.c.returning_player],
         )
 
-    async def is_returning_player(self, player_id: int, rating_type: str) -> bool:
+    async def is_returning_player(self, player_id: PlayerID, rating_type: str) -> bool:
         async with self._db.acquire() as conn:
             sql = (
                 select([league_season_score])
@@ -189,7 +190,7 @@ async def is_returning_player(self, player_id: int, rating_type: str) -> bool:
             return True
 
     async def _persist_score(
-        self, player_id: int, season_id: int, old_score: LeagueScore, new_score: LeagueScore
+        self, game_id: GameID, player_id: PlayerID, season_id: int, old_score: LeagueScore, new_score: LeagueScore
     ):
         async with self._db.acquire() as conn:
             # TODO this asserts that the passed season_id matches the
@@ -222,6 +223,7 @@ async def _persist_score(
             journal_insert_sql = (
                 insert(league_score_journal)
                 .values(
+                    game_id=game_id,
                     login_id=player_id,
                     league_season_id=season_id,
                     subdivision_id_before=old_score.division_id,
@@ -253,7 +255,7 @@ async def _persist_score(
             await conn.execute(score_insert_sql)
 
     async def _broadcast_score_change(
-        self, player_id, league: League, new_score: LeagueScore
+        self, game_id: GameID, player_id: PlayerID, league: League, new_score: LeagueScore
     ):
         pass
 

service/league_service/typedefs.py

@@ -1,5 +1,5 @@
 from enum import Enum
-from typing import Callable, Dict, List, NamedTuple, Optional, Tuple
+from typing import Callable, Dict, List, NamedTuple, NewType, Optional, Tuple
 
 from ..decorators import with_logger
 
@@ -22,7 +22,8 @@ class InvalidScoreError(LeagueServiceError):
     pass
 
 
-PlayerID = int
+GameID = NewType("GameId", int)
+PlayerID = NewType("PlayerID", int)
 RatingType = str  # e.g. "ladder_1v1"
 Rating = Tuple[float, float]
 
@@ -114,6 +115,7 @@ class LeagueRatingRequest(NamedTuple):
     Includes a callback to acknowledge processing.
     """
 
+    game_id: GameID
     player_id: PlayerID
     rating_type: RatingType
     rating: Rating
@@ -123,6 +125,7 @@ class LeagueRatingRequest(NamedTuple):
     @classmethod
     def from_rating_change_dict(cls, message: Dict):
         return cls(
+            message["game_id"],
             message["player_id"],
             message["rating_type"],
             (message["new_rating_mean"], message["new_rating_deviation"]),

tests/integration_tests/test_league_request_rating.py

@@ -19,6 +19,7 @@ async def test_rate_new_player(league_service):
     new_player_id = 50
     rating_type = "global"
     rating_change_message = {
+        "game_id": 22,
         "player_id": new_player_id,
         "rating_type": rating_type,
         "new_rating_mean": 1200,
@@ -40,6 +41,7 @@ async def test_rate_returning_player(league_service):
     returning_player_id = 2
     rating_type = "global"
     rating_change_message = {
+        "game_id": 22,
         "player_id": returning_player_id,
         "rating_type": rating_type,
         "new_rating_mean": 1200,
@@ -63,6 +65,7 @@ async def test_rate_new_player_twice(league_service):
 
     for _ in range(2):
         rating_change_message = {
+            "game_id": 22,
             "player_id": new_player_id,
             "rating_type": rating_type,
             "new_rating_mean": 1200,
@@ -85,6 +88,7 @@ async def test_rate_new_player_until_placement(league_service):
 
     for _ in range(10):
         rating_change_message = {
+            "game_id": 22,
             "player_id": new_player_id,
             "rating_type": rating_type,
             "new_rating_mean": 1200,
@@ -110,6 +114,7 @@ async def test_rate_returning_player_until_placement(league_service):
 
     for _ in range(3):
         rating_change_message = {
+            "game_id": 22,
             "player_id": new_player_id,
             "rating_type": rating_type,
             "new_rating_mean": 1200,

tests/unit_tests/test_league_service.py

@@ -167,6 +167,7 @@ async def test_load_score_league_does_not_exist(league_service):
 
 
 async def test_persist_score_new_player(league_service, database):
+    game_id = 1
     new_player_id = 5
     season_id = 2
     division_id = 3
@@ -176,7 +177,7 @@ async def test_persist_score_new_player(league_service, database):
     old_score = LeagueScore(division_id, score, game_count, returning)
     new_score = LeagueScore(division_id, score - 1, game_count + 1, returning)
 
-    await league_service._persist_score(new_player_id, season_id, old_score, new_score)
+    await league_service._persist_score(game_id, new_player_id, season_id, old_score, new_score)
 
     league = league_service._leagues_by_rating_type["global"][0]
     loaded_score = await league_service._load_score(new_player_id, league)
@@ -187,6 +188,7 @@ async def test_persist_score_new_player(league_service, database):
         rows = await result.fetchall()
         assert len(rows) == 1
         for row in rows:
+            assert row["game_id"] == 1
             assert row["login_id"] == 5
             assert row["league_season_id"] == 2
             assert row["subdivision_id_before"] == 3
@@ -197,6 +199,7 @@ async def test_persist_score_new_player(league_service, database):
 
 
 async def test_persist_score_old_player(league_service, database):
+    game_id = 10
     old_player_id = 1
     season_id = 2
     division_id = 3
@@ -206,7 +209,7 @@ async def test_persist_score_old_player(league_service, database):
     old_score = LeagueScore(division_id, score, game_count, returning)
     new_score = LeagueScore(division_id, score - 1, game_count + 1, returning)
 
-    await league_service._persist_score(old_player_id, season_id, old_score, new_score)
+    await league_service._persist_score(game_id, old_player_id, season_id, old_score, new_score)
 
     league = league_service._leagues_by_rating_type["global"][0]
     loaded_score = await league_service._load_score(old_player_id, league)
@@ -217,6 +220,7 @@ async def test_persist_score_old_player(league_service, database):
         rows = await result.fetchall()
         assert len(rows) == 1
         for row in rows:
+            assert row["game_id"] == 10
             assert row["login_id"] == 1
             assert row["league_season_id"] == 2
             assert row["subdivision_id_before"] == 3
@@ -227,6 +231,7 @@ async def test_persist_score_old_player(league_service, database):
 
 
 async def test_persist_score_season_id_mismatch(league_service):
+    game_id = 10
     player_id = 1
     wrong_season_id = 1
     division_id = 3
@@ -237,10 +242,11 @@ async def test_persist_score_season_id_mismatch(league_service):
     new_score = LeagueScore(division_id, score - 1, game_count + 1, returning)
 
     with pytest.raises(InvalidScoreError):
-        await league_service._persist_score(player_id, wrong_season_id, old_score, new_score)
+        await league_service._persist_score(game_id, player_id, wrong_season_id, old_score, new_score)
 
 
 async def test_persist_score_division_without_score(league_service):
+    game_id = 10
     player_id = 1
     season_id = 2
     division_id = 3
@@ -251,4 +257,4 @@ async def test_persist_score_division_without_score(league_service):
     new_score = LeagueScore(division_id, no_score, game_count + 1, returning)
 
     with pytest.raises(InvalidScoreError):
-        await league_service._persist_score(player_id, season_id, old_score, new_score)
+        await league_service._persist_score(game_id, player_id, season_id, old_score, new_score)