Skip to content

Commit d922118

Browse files
nanotaboadaclaude
andcommitted
feat(api): REST polish — cache fix, Location header, 409 detail (#530)
- Fix GET /players/ cache check: `if not players` → `if players is None` so empty collections are cached correctly - Add Location header to POST /players/ 201 response pointing to /players/squadnumber/{squad_number} per RFC 7231 §7.1.2 - Add detail message to POST /players/ 409 response - Add return type annotations to modified route handlers - Add [Unreleased] subsections to CHANGELOG.md Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 94c2115 commit d922118

File tree

3 files changed

+57
-4
lines changed

3 files changed

+57
-4
lines changed

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ This project uses famous football coaches as release codenames, following an A-Z
4242

4343
## [Unreleased]
4444

45+
### Added
46+
47+
### Changed
48+
49+
- `GET /players/` cache check changed from `if not players` to
50+
`if players is None` so that an empty collection is cached correctly
51+
instead of triggering a DB fetch on every request (#530)
52+
- `POST /players/` 409 response now includes a human-readable `detail`
53+
message: "A Player with this squad number already exists." (#530)
54+
55+
### Fixed
56+
57+
- `POST /players/` 201 response now includes a `Location` header
58+
pointing to the created resource at
59+
`/players/squadnumber/{squad_number}` per RFC 7231 §7.1.2 (#530)
60+
61+
### Removed
62+
4563
---
4664

4765
## [2.1.0 - Del Bosque] - 2026-03-31

routes/player_route.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@
4949
async def post_async(
5050
player_model: Annotated[PlayerRequestModel, Body(...)],
5151
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
52-
):
52+
response: Response,
53+
) -> PlayerResponseModel:
5354
"""
5455
Endpoint to create a new player.
5556
@@ -68,14 +69,18 @@ async def post_async(
6869
async_session, player_model.squad_number
6970
)
7071
if existing:
71-
raise HTTPException(status_code=status.HTTP_409_CONFLICT)
72+
raise HTTPException(
73+
status_code=status.HTTP_409_CONFLICT,
74+
detail="A Player with this squad number already exists.",
75+
)
7276
player = await player_service.create_async(async_session, player_model)
7377
if player is None: # pragma: no cover
7478
raise HTTPException(
7579
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
7680
detail="Failed to create the Player due to a database error.",
7781
)
7882
await simple_memory_cache.clear(CACHE_KEY)
83+
response.headers["Location"] = f"/players/squadnumber/{player.squad_number}"
7984
return player
8085

8186

@@ -92,7 +97,7 @@ async def post_async(
9297
async def get_all_async(
9398
response: Response,
9499
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
95-
):
100+
) -> List[PlayerResponseModel]:
96101
"""
97102
Endpoint to retrieve all players.
98103
@@ -104,7 +109,7 @@ async def get_all_async(
104109
"""
105110
players = await simple_memory_cache.get(CACHE_KEY)
106111
response.headers["X-Cache"] = "HIT"
107-
if not players:
112+
if players is None:
108113
players = await player_service.retrieve_all_async(async_session)
109114
await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
110115
response.headers["X-Cache"] = "MISS"

tests/test_main.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,18 @@ def test_request_post_player_body_existing_response_status_conflict(client):
178178
assert response.status_code == 409
179179

180180

181+
def test_request_post_player_body_existing_response_body_detail(client):
182+
"""POST /players/ with existing player returns 409 with detail message"""
183+
# Arrange
184+
player = existing_player()
185+
# Act
186+
response = client.post(PATH, json=player.__dict__)
187+
# Assert
188+
assert (
189+
response.json()["detail"] == "A Player with this squad number already exists."
190+
)
191+
192+
181193
def test_request_post_player_body_nonexistent_response_status_created(client):
182194
"""POST /players/ with nonexistent player returns 201 Created with a valid UUID"""
183195
# Arrange
@@ -275,3 +287,21 @@ def test_request_delete_player_squadnumber_existing_response_status_no_content(c
275287
response = client.delete(PATH + "squadnumber/" + str(player.squad_number))
276288
# Assert
277289
assert response.status_code == 204
290+
291+
292+
def test_request_post_player_body_nonexistent_response_header_location(client):
293+
"""POST /players/ with nonexistent player returns 201 with Location header"""
294+
# Arrange
295+
player = nonexistent_player()
296+
try:
297+
# Act
298+
response = client.post(PATH, json=player.__dict__)
299+
# Assert
300+
assert response.status_code == 201
301+
assert "Location" in response.headers
302+
assert (
303+
response.headers["Location"]
304+
== f"/players/squadnumber/{player.squad_number}"
305+
)
306+
finally:
307+
client.delete(PATH + "squadnumber/" + str(player.squad_number))

0 commit comments

Comments
 (0)