fix(packs): remove unfiltered pre-count in GET /packs (3 round-trips → 2)
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 <noreply@anthropic.com>
This commit is contained in:
parent
d83a4bdbb7
commit
7f7d9ffe1f
@ -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")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user