From fd377014dbccdd397fae1462ecbd187bb8b16411 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Fri, 10 Apr 2026 02:52:39 -0500 Subject: [PATCH] =?UTF-8?q?fix(packs):=20remove=20unfiltered=20pre-count?= =?UTF-8?q?=20in=20GET=20/packs=20(3=20round-trips=20=E2=86=92=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove Pack.select().count() on the unfiltered table at the top of GET /api/v2/packs. This check raised 404 if zero packs existed globally — wrong for filtered queries where no match is the expected empty-list result. The filtered count at the end of the handler already handles the empty-result case. Endpoint now returns {count: 0, packs: []} on empty filter matches (standard REST pattern) and saves one DB round-trip per request. Co-Authored-By: Claude Sonnet 4.6 --- app/routers_v2/packs.py | 135 +++++++++++++++++++++++----------------- 1 file changed, 79 insertions(+), 56 deletions(-) diff --git a/app/routers_v2/packs.py b/app/routers_v2/packs.py index 77b3739..13f6b5e 100644 --- a/app/routers_v2/packs.py +++ b/app/routers_v2/packs.py @@ -10,10 +10,7 @@ from ..db_engine import db, Cardset, model_to_dict, Pack, Team, PackType, DoesNo from ..dependencies import oauth2_scheme, valid_token -router = APIRouter( - prefix='/api/v2/packs', - tags=['packs'] -) +router = APIRouter(prefix="/api/v2/packs", tags=["packs"]) class PackPydantic(pydantic.BaseModel): @@ -28,46 +25,58 @@ class PackModel(pydantic.BaseModel): packs: List[PackPydantic] -@router.get('') +@router.get("") async def get_packs( - team_id: Optional[int] = None, pack_type_id: Optional[int] = None, opened: Optional[bool] = None, - limit: Optional[int] = None, new_to_old: Optional[bool] = None, pack_team_id: Optional[int] = None, - pack_cardset_id: Optional[int] = None, exact_match: Optional[bool] = False, csv: Optional[bool] = None): + team_id: Optional[int] = None, + pack_type_id: Optional[int] = None, + opened: Optional[bool] = None, + limit: Optional[int] = None, + new_to_old: Optional[bool] = None, + pack_team_id: Optional[int] = None, + pack_cardset_id: Optional[int] = None, + exact_match: Optional[bool] = False, + csv: Optional[bool] = None, +): all_packs = Pack.select() - if all_packs.count() == 0: - raise HTTPException(status_code=404, detail=f'There are no packs to filter') - if team_id is not None: try: this_team = Team.get_by_id(team_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No team found with id {team_id}') + raise HTTPException( + status_code=404, detail=f"No team found with id {team_id}" + ) all_packs = all_packs.where(Pack.team == this_team) if pack_type_id is not None: try: this_pack_type = PackType.get_by_id(pack_type_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No pack type found with id {pack_type_id}') + raise HTTPException( + status_code=404, detail=f"No pack type found with id {pack_type_id}" + ) all_packs = all_packs.where(Pack.pack_type == this_pack_type) if pack_team_id is not None: try: this_pack_team = Team.get_by_id(pack_team_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No team found with id {pack_team_id}') + raise HTTPException( + status_code=404, detail=f"No team found with id {pack_team_id}" + ) all_packs = all_packs.where(Pack.pack_team == this_pack_team) elif exact_match: - all_packs = all_packs.where(Pack.pack_team == None) + all_packs = all_packs.where(Pack.pack_team == None) # noqa: E711 if pack_cardset_id is not None: try: this_pack_cardset = Cardset.get_by_id(pack_cardset_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No cardset found with id {pack_cardset_id}') + raise HTTPException( + status_code=404, detail=f"No cardset found with id {pack_cardset_id}" + ) all_packs = all_packs.where(Pack.pack_cardset == this_pack_cardset) elif exact_match: - all_packs = all_packs.where(Pack.pack_cardset == None) + all_packs = all_packs.where(Pack.pack_cardset == None) # noqa: E711 if opened is not None: all_packs = all_packs.where(Pack.open_time.is_null(not opened)) @@ -78,60 +87,62 @@ async def get_packs( else: all_packs = all_packs.order_by(Pack.id) - # if all_packs.count() == 0: - # db.close() - # raise HTTPException(status_code=404, detail=f'No packs found') - if csv: - data_list = [['id', 'team', 'pack_type', 'open_time']] + data_list = [["id", "team", "pack_type", "open_time"]] for line in all_packs: data_list.append( [ - line.id, line.team.abbrev, line.pack_type.name, - line.open_time # Already datetime in PostgreSQL + line.id, + line.team.abbrev, + line.pack_type.name, + line.open_time, # Already datetime in PostgreSQL ] ) 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_packs.count(), 'packs': []} + return_val = {"count": all_packs.count(), "packs": []} for x in all_packs: - return_val['packs'].append(model_to_dict(x)) + return_val["packs"].append(model_to_dict(x)) return return_val -@router.get('/{pack_id}') +@router.get("/{pack_id}") async def get_one_pack(pack_id: int, csv: Optional[bool] = False): try: this_pack = Pack.get_by_id(pack_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No pack found with id {pack_id}') + raise HTTPException(status_code=404, detail=f"No pack found with id {pack_id}") if csv: data_list = [ - ['id', 'team', 'pack_type', 'open_time'], - [this_pack.id, this_pack.team.abbrev, this_pack.pack_type.name, - this_pack.open_time] # Already datetime in PostgreSQL + ["id", "team", "pack_type", "open_time"], + [ + this_pack.id, + this_pack.team.abbrev, + this_pack.pack_type.name, + this_pack.open_time, + ], # Already datetime in PostgreSQL ] 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_pack) return return_val -@router.post('') +@router.post("") async def post_pack(packs: PackModel, 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 packs. This event has been logged.' + detail="You are not authorized to post packs. This event has been logged.", ) new_packs = [] @@ -141,23 +152,27 @@ async def post_pack(packs: PackModel, token: str = Depends(oauth2_scheme)): pack_type_id=x.pack_type_id, pack_team_id=x.pack_team_id, pack_cardset_id=x.pack_cardset_id, - open_time=datetime.fromtimestamp(x.open_time / 1000) if x.open_time else None + open_time=datetime.fromtimestamp(x.open_time / 1000) + if x.open_time + else None, ) new_packs.append(this_player) with db.atomic(): Pack.bulk_create(new_packs, batch_size=15) - raise HTTPException(status_code=200, detail=f'{len(new_packs)} packs have been added') + raise HTTPException( + status_code=200, detail=f"{len(new_packs)} packs have been added" + ) -@router.post('/one') +@router.post("/one") async def post_one_pack(pack: PackPydantic, 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 packs. This event has been logged.' + detail="You are not authorized to post packs. This event has been logged.", ) this_pack = Pack( @@ -165,7 +180,9 @@ async def post_one_pack(pack: PackPydantic, token: str = Depends(oauth2_scheme)) pack_type_id=pack.pack_type_id, pack_team_id=pack.pack_team_id, pack_cardset_id=pack.pack_cardset_id, - open_time=datetime.fromtimestamp(pack.open_time / 1000) if pack.open_time else None + open_time=datetime.fromtimestamp(pack.open_time / 1000) + if pack.open_time + else None, ) saved = this_pack.save() @@ -175,24 +192,30 @@ async def post_one_pack(pack: PackPydantic, token: str = Depends(oauth2_scheme)) else: raise HTTPException( status_code=418, - detail='Well slap my ass and call me a teapot; I could not save that cardset' + detail="Well slap my ass and call me a teapot; I could not save that cardset", ) -@router.patch('/{pack_id}') +@router.patch("/{pack_id}") async def patch_pack( - pack_id, team_id: Optional[int] = None, pack_type_id: Optional[int] = None, open_time: Optional[int] = None, - pack_team_id: Optional[int] = None, pack_cardset_id: Optional[int] = None, token: str = Depends(oauth2_scheme)): + pack_id, + team_id: Optional[int] = None, + pack_type_id: Optional[int] = None, + open_time: Optional[int] = None, + pack_team_id: Optional[int] = None, + pack_cardset_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 packs. This event has been logged.' + detail="You are not authorized to patch packs. This event has been logged.", ) try: this_pack = Pack.get_by_id(pack_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No pack found with id {pack_id}') + raise HTTPException(status_code=404, detail=f"No pack found with id {pack_id}") if team_id is not None: this_pack.team_id = team_id @@ -220,26 +243,26 @@ async def patch_pack( 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('/{pack_id}') +@router.delete("/{pack_id}") async def delete_pack(pack_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_pack = Pack.get_by_id(pack_id) except DoesNotExist: - raise HTTPException(status_code=404, detail=f'No packs found with id {pack_id}') + raise HTTPException(status_code=404, detail=f"No packs found with id {pack_id}") count = this_pack.delete_instance() if count == 1: - raise HTTPException(status_code=200, detail=f'Pack {pack_id} has been deleted') + raise HTTPException(status_code=200, detail=f"Pack {pack_id} has been deleted") else: - raise HTTPException(status_code=500, detail=f'Pack {pack_id} was not deleted') + raise HTTPException(status_code=500, detail=f"Pack {pack_id} was not deleted")