Merge pull request 'fix: batch Paperdex lookups to avoid N+1 queries (#17)' (#53) from ai/paper-dynasty-database#17 into next-release

Reviewed-on: #53
Reviewed-by: Claude <cal.corum+openclaw@gmail.com>

app/routers_v2/cards.py

@@ -7,11 +7,7 @@ from pandas import DataFrame
 from ..db_engine import db, Card, model_to_dict, Team, Player, Pack, Paperdex, CARDSETS, DoesNotExist
 from ..dependencies import oauth2_scheme, valid_token
 
-
-router = APIRouter(
-    prefix='/api/v2/cards',
-    tags=['cards']
-)
+router = APIRouter(prefix="/api/v2/cards", tags=["cards"])
 
 
 class CardPydantic(pydantic.BaseModel):
@@ -26,12 +22,20 @@ class CardModel(pydantic.BaseModel):
     cards: List[CardPydantic]
 
 
-@router.get('')
+@router.get("")
 async def get_cards(
-        player_id: Optional[int] = None, team_id: Optional[int] = None, pack_id: Optional[int] = None,
-        value: Optional[int] = None, min_value: Optional[int] = None, max_value: Optional[int] = None, variant: Optional[int] = None,
-        order_by: Optional[str] = None, limit: Optional[int] = None, dupes: Optional[bool] = None,
-        csv: Optional[bool] = None):
+    player_id: Optional[int] = None,
+    team_id: Optional[int] = None,
+    pack_id: Optional[int] = None,
+    value: Optional[int] = None,
+    min_value: Optional[int] = None,
+    max_value: Optional[int] = None,
+    variant: Optional[int] = None,
+    order_by: Optional[str] = None,
+    limit: Optional[int] = None,
+    dupes: Optional[bool] = None,
+    csv: Optional[bool] = None,
+):
     all_cards = Card.select()
 
     # if all_cards.count() == 0:
@@ -65,7 +69,7 @@ async def get_cards(
     if max_value is not None:
         all_cards = all_cards.where(Card.value <= max_value)
     if order_by is not None:
-        if order_by.lower() == 'new':
+        if order_by.lower() == "new":
             all_cards = all_cards.order_by(-Card.id)
     else:
         all_cards = all_cards.order_by(Card.id)
@@ -73,8 +77,10 @@ async def get_cards(
         all_cards = all_cards.limit(limit)
     if dupes:
         if team_id is None:
-            raise HTTPException(status_code=400, detail='Dupe checking must include a team_id')
-        logging.debug(f'dupe check')
+            raise HTTPException(
+                status_code=400, detail="Dupe checking must include a team_id"
+            )
+        logging.debug(f"dupe check")
         p_query = Card.select(Card.player).where(Card.team_id == team_id)
         seen = set()
         dupes = []
@@ -90,38 +96,52 @@ async def get_cards(
     #     raise HTTPException(status_code=404, detail=f'No cards found')
 
     if csv:
-        data_list = [['id', 'player', 'cardset', 'rarity', 'team', 'pack', 'value']]  #, 'variant']]
+        data_list = [
+            ["id", "player", "cardset", "rarity", "team", "pack", "value"]
+        ]  # , 'variant']]
         for line in all_cards:
             data_list.append(
                 [
-                    line.id, line.player.p_name, line.player.cardset, line.player.rarity, line.team.abbrev, line.pack,
+                    line.id,
+                    line.player.p_name,
+                    line.player.cardset,
+                    line.player.rarity,
+                    line.team.abbrev,
+                    line.pack,
                     line.value,  # line.variant
                 ]
             )
         return_val = DataFrame(data_list).to_csv(header=False, index=False)
 
-        return Response(content=return_val, media_type='text/csv')
+        return Response(content=return_val, media_type="text/csv")
 
     else:
-        return_val = {'count': all_cards.count(), 'cards': []}
-        for x in all_cards:
+        card_list = list(all_cards)
+        player_ids = [c.player_id for c in card_list if c.player_id is not None]
+        dex_by_player = {}
+        if player_ids:
+            for row in Paperdex.select().where(Paperdex.player_id << player_ids):
+                dex_by_player.setdefault(row.player_id, []).append(row)
+        return_val = {"count": len(card_list), "cards": []}
+        for x in card_list:
 
             this_record = model_to_dict(x)
-            logging.debug(f'this_record: {this_record}')
+            logging.debug(f"this_record: {this_record}")
 
-            this_dex = Paperdex.select().where(Paperdex.player == x)
-            this_record['player']['paperdex'] = {'count': this_dex.count(), 'paperdex': []}
-            for y in this_dex:
-                this_record['player']['paperdex']['paperdex'].append(model_to_dict(y, recurse=False))
+            entries = dex_by_player.get(x.player_id, [])
+            this_record["player"]["paperdex"] = {
+                "count": len(entries),
+                "paperdex": [model_to_dict(y, recurse=False) for y in entries],
+            }
 
-            return_val['cards'].append(this_record)
+            return_val["cards"].append(this_record)
 
             # return_val['cards'].append(model_to_dict(x))
 
         return return_val
 
 
-@router.get('/{card_id}')
+@router.get("/{card_id}")
 async def v1_cards_get_one(card_id, csv: Optional[bool] = False):
     try:
         this_card = Card.get_by_id(card_id)
@@ -130,25 +150,31 @@ async def v1_cards_get_one(card_id, csv: Optional[bool] = False):
 
     if csv:
         data_list = [
-            ['id', 'player', 'team', 'pack', 'value'],
-            [this_card.id, this_card.player, this_card.team.abbrev, this_card.pack, this_card.value]
+            ["id", "player", "team", "pack", "value"],
+            [
+                this_card.id,
+                this_card.player,
+                this_card.team.abbrev,
+                this_card.pack,
+                this_card.value,
+            ],
         ]
         return_val = DataFrame(data_list).to_csv(header=False, index=False)
 
-        return Response(content=return_val, media_type='text/csv')
+        return Response(content=return_val, media_type="text/csv")
 
     else:
         return_val = model_to_dict(this_card)
         return return_val
 
 
-@router.post('')
+@router.post("")
 async def v1_cards_post(cards: CardModel, token: str = Depends(oauth2_scheme)):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to post cards. This event has been logged.'
+            detail="You are not authorized to post cards. This event has been logged.",
         )
     last_card = Card.select(Card.id).order_by(-Card.id).limit(1)
     lc_id = last_card[0].id
@@ -157,7 +183,7 @@ async def v1_cards_post(cards: CardModel, token: str = Depends(oauth2_scheme)):
     player_ids = []
     inc_dex = True
     this_team = Team.get_by_id(cards.cards[0].team_id)
-    if this_team.is_ai or 'Gauntlet' in this_team.abbrev:
+    if this_team.is_ai or "Gauntlet" in this_team.abbrev:
         inc_dex = False
 
     # new_dex = []
@@ -177,11 +203,15 @@ async def v1_cards_post(cards: CardModel, token: str = Depends(oauth2_scheme)):
 
     with db.atomic():
         Card.bulk_create(new_cards, batch_size=15)
-    cost_query = Player.update(cost=Player.cost + 1).where(Player.player_id << player_ids)
+    cost_query = Player.update(cost=Player.cost + 1).where(
+        Player.player_id << player_ids
+    )
     cost_query.execute()
     # sheets.post_new_cards(SHEETS_AUTH, lc_id)
 
-    raise HTTPException(status_code=200, detail=f'{len(new_cards)} cards have been added')
+    raise HTTPException(
+        status_code=200, detail=f"{len(new_cards)} cards have been added"
+    )
 
 
 # @router.post('/ai-update')
@@ -198,21 +228,27 @@ async def v1_cards_post(cards: CardModel, token: str = Depends(oauth2_scheme)):
 #     raise HTTPException(status_code=200, detail=f'Just sent AI cards to sheets')
 
 
-@router.post('/legal-check/{rarity_name}')
+@router.post("/legal-check/{rarity_name}")
 async def v1_cards_legal_check(
-        rarity_name: str, card_id: list = Query(default=None), token: str = Depends(oauth2_scheme)):
+    rarity_name: str,
+    card_id: list = Query(default=None),
+    token: str = Depends(oauth2_scheme),
+):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
-        raise HTTPException(
-            status_code=401,
-            detail='Unauthorized'
-        )
+        logging.warning("Bad Token: [REDACTED]")
+        raise HTTPException(status_code=401, detail="Unauthorized")
     if rarity_name not in CARDSETS.keys():
-        return f'Rarity name {rarity_name} not a valid check'
+        return f"Rarity name {rarity_name} not a valid check"
 
     # Handle case where card_id is passed as a stringified list
-    if card_id and len(card_id) == 1 and isinstance(card_id[0], str) and card_id[0].startswith('['):
+    if (
+        card_id
+        and len(card_id) == 1
+        and isinstance(card_id[0], str)
+        and card_id[0].startswith("[")
+    ):
         import ast
+
         try:
             card_id = [int(x) for x in ast.literal_eval(card_id[0])]
         except (ValueError, SyntaxError):
@@ -222,48 +258,51 @@ async def v1_cards_legal_check(
     all_cards = Card.select().where(Card.id << card_id)
 
     for x in all_cards:
-        if x.player.cardset_id not in CARDSETS[rarity_name]['human']:
+        if x.player.cardset_id not in CARDSETS[rarity_name]["human"]:
             if x.player.p_name in x.player.description:
                 bad_cards.append(x.player.description)
             else:
-                bad_cards.append(f'{x.player.description} {x.player.p_name}')
+                bad_cards.append(f"{x.player.description} {x.player.p_name}")
 
-    return {'count': len(bad_cards), 'bad_cards': bad_cards}
+    return {"count": len(bad_cards), "bad_cards": bad_cards}
 
 
-@router.post('/post-update/{starting_id}')
+@router.post("/post-update/{starting_id}")
 async def v1_cards_post_update(starting_id: int, token: str = Depends(oauth2_scheme)):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to update card lists. This event has been logged.'
+            detail="You are not authorized to update card lists. This event has been logged.",
         )
 
     # sheets.post_new_cards(SHEETS_AUTH, starting_id)
-    raise HTTPException(status_code=200, detail=f'Just sent cards to sheets starting at ID {starting_id}')
+    raise HTTPException(
+        status_code=200,
+        detail=f"Just sent cards to sheets starting at ID {starting_id}",
+    )
 
 
-@router.post('/post-delete')
+@router.post("/post-delete")
 async def v1_cards_post_delete(del_ids: str, token: str = Depends(oauth2_scheme)):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to delete card lists. This event has been logged.'
+            detail="You are not authorized to delete card lists. This event has been logged.",
         )
 
-    logging.info(f'del_ids: {del_ids} / type: {type(del_ids)}')
+    logging.info(f"del_ids: {del_ids} / type: {type(del_ids)}")
     # sheets.post_deletion(SHEETS_AUTH, del_ids.split(','))
 
 
-@router.post('/wipe-team/{team_id}')
+@router.post("/wipe-team/{team_id}")
 async def v1_cards_wipe_team(team_id: int, token: str = Depends(oauth2_scheme)):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to wipe teams. This event has been logged.'
+            detail="You are not authorized to wipe teams. This event has been logged.",
         )
 
     try:
@@ -273,19 +312,27 @@ async def v1_cards_wipe_team(team_id: int, token: str = Depends(oauth2_scheme)):
         raise HTTPException(status_code=404, detail=f'Team {team_id} not found')
 
     t_query = Card.update(team=None).where(Card.team == this_team).execute()
-    return f'Wiped {t_query} cards'
+    return f"Wiped {t_query} cards"
 
 
-@router.patch('/{card_id}')
+@router.patch("/{card_id}")
 async def v1_cards_patch(
-        card_id, player_id: Optional[int] = None, team_id: Optional[int] = None, pack_id: Optional[int] = None,
-        value: Optional[int] = None, variant: Optional[int] = None, roster1_id: Optional[int] = None, roster2_id: Optional[int] = None,
-        roster3_id: Optional[int] = None, token: str = Depends(oauth2_scheme)):
+    card_id,
+    player_id: Optional[int] = None,
+    team_id: Optional[int] = None,
+    pack_id: Optional[int] = None,
+    value: Optional[int] = None,
+    variant: Optional[int] = None,
+    roster1_id: Optional[int] = None,
+    roster2_id: Optional[int] = None,
+    roster3_id: Optional[int] = None,
+    token: str = Depends(oauth2_scheme),
+):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to patch cards. This event has been logged.'
+            detail="You are not authorized to patch cards. This event has been logged.",
         )
     try:
         this_card = Card.get_by_id(card_id)
@@ -318,17 +365,17 @@ async def v1_cards_patch(
     else:
         raise HTTPException(
             status_code=418,
-            detail='Well slap my ass and call me a teapot; I could not save that rarity'
+            detail="Well slap my ass and call me a teapot; I could not save that rarity",
         )
 
 
-@router.delete('/{card_id}')
+@router.delete("/{card_id}")
 async def v1_cards_delete(card_id, token: str = Depends(oauth2_scheme)):
     if not valid_token(token):
-        logging.warning('Bad Token: [REDACTED]')
+        logging.warning("Bad Token: [REDACTED]")
         raise HTTPException(
             status_code=401,
-            detail='You are not authorized to delete packs. This event has been logged.'
+            detail="You are not authorized to delete packs. This event has been logged.",
         )
     try:
         this_card = Card.get_by_id(card_id)
@@ -338,6 +385,6 @@ async def v1_cards_delete(card_id, token: str = Depends(oauth2_scheme)):
     count = this_card.delete_instance()
 
     if count == 1:
-        raise HTTPException(status_code=200, detail=f'Card {card_id} has been deleted')
+        raise HTTPException(status_code=200, detail=f"Card {card_id} has been deleted")
     else:
-        raise HTTPException(status_code=500, detail=f'Card {card_id} was not deleted')
+        raise HTTPException(status_code=500, detail=f"Card {card_id} was not deleted")

app/routers_v2/players.py

@@ -144,7 +144,7 @@ async def get_players(
 ):
     all_players = Player.select()
     if all_players.count() == 0:
-        raise HTTPException(status_code=404, detail=f"There are no players to filter")
+        raise HTTPException(status_code=404, detail="There are no players to filter")
 
     if name is not None:
         all_players = all_players.where(fn.Lower(Player.p_name) == name.lower())
@@ -294,16 +294,21 @@ async def get_players(
 
     else:
         return_val = {"count": len(final_players), "players": []}
+        dex_by_player = {}
+        if inc_dex:
+            player_ids = [p.player_id for p in final_players]
+            if player_ids:
+                for row in Paperdex.select().where(Paperdex.player_id << player_ids):
+                    dex_by_player.setdefault(row.player_id, []).append(row)
         for x in final_players:
             this_record = model_to_dict(x, recurse=not (flat or short_output))
 
             if inc_dex:
-                this_dex = Paperdex.select().where(Paperdex.player == x)
-                this_record["paperdex"] = {"count": this_dex.count(), "paperdex": []}
-                for y in this_dex:
-                    this_record["paperdex"]["paperdex"].append(
-                        model_to_dict(y, recurse=False)
-                    )
+                entries = dex_by_player.get(x.player_id, [])
+                this_record["paperdex"] = {
+                    "count": len(entries),
+                    "paperdex": [model_to_dict(y, recurse=False) for y in entries],
+                }
 
             if inc_keys and (flat or short_output):
                 if this_record["mlbplayer"] is not None:
@@ -471,16 +476,21 @@ async def get_random_player(
         return Response(content=return_val, media_type="text/csv")
 
     else:
+        final_players = list(final_players)
         return_val = {"count": len(final_players), "players": []}
+        player_ids = [p.player_id for p in final_players]
+        dex_by_player = {}
+        if player_ids:
+            for row in Paperdex.select().where(Paperdex.player_id << player_ids):
+                dex_by_player.setdefault(row.player_id, []).append(row)
         for x in final_players:
             this_record = model_to_dict(x)
 
-            this_dex = Paperdex.select().where(Paperdex.player == x)
-            this_record["paperdex"] = {"count": this_dex.count(), "paperdex": []}
-            for y in this_dex:
-                this_record["paperdex"]["paperdex"].append(
-                    model_to_dict(y, recurse=False)
-                )
+            entries = dex_by_player.get(x.player_id, [])
+            this_record["paperdex"] = {
+                "count": len(entries),
+                "paperdex": [model_to_dict(y, recurse=False) for y in entries],
+            }
 
             return_val["players"].append(this_record)
             # return_val['players'].append(model_to_dict(x))
@@ -674,9 +684,6 @@ async def get_batter_card(
         )
 
     headers = {"Cache-Control": "public, max-age=86400"}
-    filename = (
-        f"{this_player.description} {this_player.p_name} {card_type} {d}-v{variant}"
-    )
     if (
         os.path.isfile(
             f"storage/cards/cardset-{this_player.cardset.id}/{card_type}/{player_id}-{d}-v{variant}.png"