diff --git a/.claude/settings.json b/.claude/settings.json index ad11d51..93a440a 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,3 +1,13 @@ { - "model": "claude-sonnet-4-6" + "model": "claude-sonnet-4-6", + "permissions": { + "allow": [ + "Bash(gh issue:*)", + "Bash(gh auth:*)", + "Bash(uv run:*)", + "Bash(source .venv/bin/activate)", + "WebFetch(domain:github.com)", + "WebFetch(domain:api.github.com)" + ] + } } diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3523477..661cf7e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -40,7 +40,7 @@ tests/ — pytest integration tests - **Async**: All routes and service functions must be `async def`; use `AsyncSession` (never `Session`); use `aiosqlite` (never `sqlite3`); use SQLAlchemy 2.0 `select()` (never `session.query()`) - **API contract**: camelCase JSON via Pydantic `alias_generator=to_camel`; Python internals stay snake_case - **Models**: `PlayerRequestModel` (no `id`, used for POST/PUT) and `PlayerResponseModel` (includes `id: UUID`, used for GET/POST responses); never use the removed `PlayerModel` -- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for all CRUD operations. UUID v4 for API-created records; UUID v5 (deterministic) for migration-seeded records. `squad_number` is the natural key — human-readable, domain-meaningful, preferred lookup for external consumers +- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for GET by id only. UUID v4 for API-created records; UUID v5 (deterministic) for migration-seeded records. `squad_number` is the natural key — human-readable, domain-meaningful, used for all mutation endpoints (PUT, DELETE) and preferred for all external consumers - **Caching**: cache key `"players"` (hardcoded); clear on POST/PUT/DELETE; `X-Cache` header (HIT/MISS) - **Errors**: Catch specific exceptions with rollback in services; Pydantic validation returns 422 (not 400) - **Logging**: `logging` module only; never `print()` diff --git a/.gitignore b/.gitignore index cf6e796..aa7b685 100644 --- a/.gitignore +++ b/.gitignore @@ -160,6 +160,9 @@ dmypy.json # Cython debug symbols cython_debug/ +# Claude Code +.claude/settings.local.json + # PyCharm # JetBrains specific template is maintained in a separate JetBrains.gitignore that can # be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore diff --git a/.markdownlint.json b/.markdownlint.json new file mode 100644 index 0000000..ef592cf --- /dev/null +++ b/.markdownlint.json @@ -0,0 +1,6 @@ +{ + "MD013": false, + "MD024": { + "siblings_only": true + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a6c29..db7a7a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,12 @@ This project uses famous football coaches as release codenames, following an A-Z ## [Unreleased] +### Changed + +- **BREAKING**: `PUT /players/{player_id}` replaced by `PUT /players/squadnumber/{squad_number}` — mutation endpoints now use Squad Number (natural key) instead of UUID (surrogate key), consistent with `GET /players/squadnumber/{squad_number}` (#521) +- **BREAKING**: `DELETE /players/{player_id}` replaced by `DELETE /players/squadnumber/{squad_number}` — same rationale as above (#521) +- `update_async` and `delete_async` (UUID-based) replaced by `update_by_squad_number_async` and `delete_by_squad_number_async` in `services/player_service.py` (#521) + --- ## [1.1.0 - Bielsa] - 2026-03-02 diff --git a/README.md b/README.md index 3f26678..a4d70ee 100644 --- a/README.md +++ b/README.md @@ -184,8 +184,8 @@ Interactive API documentation is available via Swagger UI at `http://localhost:9 - `GET /players/{player_id}` — Get player by UUID (surrogate key) - `GET /players/squadnumber/{squad_number}` — Get player by squad number (natural key) - `POST /players/` — Create a new player -- `PUT /players/{player_id}` — Update an existing player -- `DELETE /players/{player_id}` — Remove a player +- `PUT /players/squadnumber/{squad_number}` — Update an existing player +- `DELETE /players/squadnumber/{squad_number}` — Remove a player - `GET /health` — Health check ### HTTP Requests diff --git a/rest/players.rest b/rest/players.rest index 2bfa0ac..8a06ea4 100644 --- a/rest/players.rest +++ b/rest/players.rest @@ -5,8 +5,8 @@ ### https://marketplace.visualstudio.com/items?itemName=humao.rest-client ### ### Key design note: -### squad_number = natural key (domain-meaningful, preferred for lookups) -### id (UUID) = surrogate key (internal, opaque, used for CRUD operations) +### squad_number = natural key (domain-meaningful, used for all CRUD operations) +### id (UUID) = surrogate key (internal, opaque, used for GET by id only) @baseUrl = http://localhost:9000 @@ -68,12 +68,12 @@ GET {{baseUrl}}/players/squadnumber/10 Accept: application/json # ------------------------------------------------------------------------------ -# PUT /players/{player_id} — Update -# Emiliano Martínez (squad 23): UUID v5, seeded by seed_001. +# PUT /players/squadnumber/{squad_number} — Update +# Emiliano Martínez (squad 23): seeded by seed_001. # ------------------------------------------------------------------------------ -### PUT /players/{player_id} — Update an existing Player -PUT {{baseUrl}}/players/b04965e6-a9bb-591f-8f8a-1adcb2c8dc39 +### PUT /players/squadnumber/{squad_number} — Update an existing Player +PUT {{baseUrl}}/players/squadnumber/23 Content-Type: application/json { @@ -90,15 +90,9 @@ Content-Type: application/json } # ------------------------------------------------------------------------------ -# DELETE /players/{player_id} — Delete -# Thiago Almada (squad 16): created by POST above. Since the UUID is generated -# at runtime, retrieve it first via squad number, then substitute it below. +# DELETE /players/squadnumber/{squad_number} — Delete +# Thiago Almada (squad 16): created by POST above. # ------------------------------------------------------------------------------ -### Step 1 — Look up Almada's UUID by squad number -GET {{baseUrl}}/players/squadnumber/16 -Accept: application/json - -### Step 2 — Delete Almada using the UUID returned above -# Replace {player_id} with the id field from the response above. -DELETE {{baseUrl}}/players/{player_id} +### DELETE /players/squadnumber/{squad_number} — Delete an existing Player +DELETE {{baseUrl}}/players/squadnumber/16 diff --git a/routes/player_route.py b/routes/player_route.py index 3a8d89c..4e50d66 100644 --- a/routes/player_route.py +++ b/routes/player_route.py @@ -15,11 +15,11 @@ (surrogate key, internal). - GET /players/squadnumber/{squad_number} : Retrieve Player by Squad Number (natural key, domain). -- PUT /players/{player_id} : Update an existing Player. -- DELETE /players/{player_id} : Delete an existing Player. +- PUT /players/squadnumber/{squad_number} : Update an existing Player. +- DELETE /players/squadnumber/{squad_number} : Delete an existing Player. """ -from typing import List +from typing import Annotated, List from uuid import UUID from fastapi import APIRouter, Body, Depends, HTTPException, status, Path, Response from sqlalchemy.ext.asyncio import AsyncSession @@ -34,6 +34,7 @@ CACHE_KEY = "players" CACHE_TTL = 600 # 10 minutes +SQUAD_NUMBER_TITLE = "The Squad Number of the Player" # POST ------------------------------------------------------------------------- @@ -46,8 +47,8 @@ tags=["Players"], ) async def post_async( - player_model: PlayerRequestModel = Body(...), - async_session: AsyncSession = Depends(generate_async_session), + player_model: Annotated[PlayerRequestModel, Body(...)], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to create a new player. @@ -89,7 +90,8 @@ async def post_async( tags=["Players"], ) async def get_all_async( - response: Response, async_session: AsyncSession = Depends(generate_async_session) + response: Response, + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to retrieve all players. @@ -117,8 +119,8 @@ async def get_all_async( tags=["Players"], ) async def get_by_id_async( - player_id: UUID = Path(..., title="The UUID of the Player"), - async_session: AsyncSession = Depends(generate_async_session), + player_id: Annotated[UUID, Path(..., title="The UUID of the Player")], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to retrieve a Player by its UUID. @@ -148,8 +150,8 @@ async def get_by_id_async( tags=["Players"], ) async def get_by_squad_number_async( - squad_number: int = Path(..., title="The Squad Number of the Player"), - async_session: AsyncSession = Depends(generate_async_session), + squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to retrieve a Player by its Squad Number. @@ -177,33 +179,37 @@ async def get_by_squad_number_async( @api_router.put( - "/players/{player_id}", + "/players/squadnumber/{squad_number}", status_code=status.HTTP_204_NO_CONTENT, summary="Updates an existing Player", tags=["Players"], ) async def put_async( - player_id: UUID = Path(..., title="The UUID of the Player"), - player_model: PlayerRequestModel = Body(...), - async_session: AsyncSession = Depends(generate_async_session), + squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)], + player_model: Annotated[PlayerRequestModel, Body(...)], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to entirely update an existing Player. Args: - player_id (UUID): The UUID of the Player to update. + squad_number (int): The Squad Number of the Player to update. player_model (PlayerRequestModel): The Pydantic model representing the Player to update. async_session (AsyncSession): The async version of a SQLAlchemy ORM session. Raises: - HTTPException: HTTP 404 Not Found error if the Player with the specified UUID - does not exist. + HTTPException: HTTP 404 Not Found error if the Player with the specified Squad + Number does not exist. """ - player = await player_service.retrieve_by_id_async(async_session, player_id) + player = await player_service.retrieve_by_squad_number_async( + async_session, squad_number + ) if not player: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) - updated = await player_service.update_async(async_session, player_id, player_model) + updated = await player_service.update_by_squad_number_async( + async_session, squad_number, player_model + ) if not updated: # pragma: no cover raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, @@ -216,30 +222,34 @@ async def put_async( @api_router.delete( - "/players/{player_id}", + "/players/squadnumber/{squad_number}", status_code=status.HTTP_204_NO_CONTENT, summary="Deletes an existing Player", tags=["Players"], ) async def delete_async( - player_id: UUID = Path(..., title="The UUID of the Player"), - async_session: AsyncSession = Depends(generate_async_session), + squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): """ Endpoint to delete an existing Player. Args: - player_id (UUID): The UUID of the Player to delete. + squad_number (int): The Squad Number of the Player to delete. async_session (AsyncSession): The async version of a SQLAlchemy ORM session. Raises: - HTTPException: HTTP 404 Not Found error if the Player with the specified UUID - does not exist. + HTTPException: HTTP 404 Not Found error if the Player with the specified Squad + Number does not exist. """ - player = await player_service.retrieve_by_id_async(async_session, player_id) + player = await player_service.retrieve_by_squad_number_async( + async_session, squad_number + ) if not player: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) - deleted = await player_service.delete_async(async_session, player_id) + deleted = await player_service.delete_by_squad_number_async( + async_session, squad_number + ) if not deleted: # pragma: no cover raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, diff --git a/services/player_service.py b/services/player_service.py index ade50e4..6df43ff 100644 --- a/services/player_service.py +++ b/services/player_service.py @@ -2,14 +2,14 @@ Async CRUD operations for Player entities using SQLAlchemy ORM. Functions: -- create_async : Add a new Player to the database. -- retrieve_all_async : Fetch all Player records. -- retrieve_by_id_async : Fetch a Player by its UUID - (surrogate key, internal). -- retrieve_by_squad_number_async : Fetch a Player by its Squad Number - (natural key, domain). -- update_async : Fully update an existing Player. -- delete_async : Remove a Player from the database. +- create_async : Add a new Player to the database. +- retrieve_all_async : Fetch all Player records. +- retrieve_by_id_async : Fetch a Player by its UUID + (surrogate key, internal). +- retrieve_by_squad_number_async : Fetch a Player by its Squad Number + (natural key, domain). +- update_by_squad_number_async : Fully update a Player by Squad Number. +- delete_by_squad_number_async : Remove a Player by Squad Number. Handles SQLAlchemy exceptions with transaction rollback and logs errors. """ @@ -117,30 +117,30 @@ async def retrieve_by_squad_number_async( # Update ----------------------------------------------------------------------- -async def update_async( - async_session: AsyncSession, player_id: UUID, player_model: PlayerRequestModel +async def update_by_squad_number_async( + async_session: AsyncSession, squad_number: int, player_model: PlayerRequestModel ) -> bool: """ - Updates (entirely) an existing Player in the database. + Updates (entirely) an existing Player identified by Squad Number. Args: async_session (AsyncSession): The async version of a SQLAlchemy ORM session. - player_id (UUID): The UUID of the Player to update. + squad_number (int): The Squad Number of the Player to update. player_model (PlayerRequestModel): The Pydantic model representing the Player to update. Returns: True if the Player was updated successfully, False otherwise. """ - player = await async_session.get(Player, player_id) + player = await retrieve_by_squad_number_async(async_session, squad_number) if player is None: # pragma: no cover - logger.error("Player not found for update: %s", player_id) + logger.error("Player not found for update: squad_number=%s", squad_number) return False player.first_name = player_model.first_name player.middle_name = player_model.middle_name player.last_name = player_model.last_name player.date_of_birth = player_model.date_of_birth - player.squad_number = player_model.squad_number + player.squad_number = squad_number player.position = player_model.position player.abbr_position = player_model.abbr_position player.team = player_model.team @@ -158,20 +158,25 @@ async def update_async( # Delete ----------------------------------------------------------------------- -async def delete_async(async_session: AsyncSession, player_id: UUID) -> bool: +async def delete_by_squad_number_async( + async_session: AsyncSession, squad_number: int +) -> bool: """ - Deletes an existing Player from the database. + Deletes an existing Player identified by Squad Number from the database. Args: async_session (AsyncSession): The async version of a SQLAlchemy ORM session. - player_id (UUID): The UUID of the Player to delete. + squad_number (int): The Squad Number of the Player to delete. Returns: True if the Player was deleted successfully, False otherwise. """ - player = await async_session.get(Player, player_id) - await async_session.delete(player) + player = await retrieve_by_squad_number_async(async_session, squad_number) + if player is None: # pragma: no cover + logger.error("Player not found for delete: squad_number=%s", squad_number) + return False try: + await async_session.delete(player) await async_session.commit() return True except SQLAlchemyError as error: # pragma: no cover diff --git a/tests/test_main.py b/tests/test_main.py index d27c669..6ac99be 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -7,8 +7,8 @@ - GET /players/{player_id} - GET /players/squadnumber/{squad_number} - POST /players/ -- PUT /players/{player_id} -- DELETE /players/{player_id} +- PUT /players/squadnumber/{squad_number} +- DELETE /players/squadnumber/{squad_number} Validates: - Status codes, response bodies, headers (e.g., X-Cache) @@ -191,66 +191,68 @@ def test_request_post_player_body_nonexistent_response_status_created(client): assert UUID(body["id"]).version == 4 # UUID v4 (API-created) -# PUT /players/{player_id} ----------------------------------------------------- +# PUT /players/squadnumber/{squad_number} -------------------------------------- -def test_request_put_player_id_existing_body_empty_response_status_unprocessable( +def test_request_put_player_squadnumber_existing_body_empty_response_status_unprocessable( client, ): - """PUT /players/{player_id} with empty body returns 422 Unprocessable Entity""" + """PUT /players/squadnumber/{squad_number} with empty body returns 422 Unprocessable Entity""" # Arrange - player_id = existing_player().id + squad_number = existing_player().squad_number # Act - response = client.put(PATH + str(player_id), json={}) + response = client.put(PATH + "squadnumber/" + str(squad_number), json={}) # Assert assert response.status_code == 422 -def test_request_put_player_id_unknown_response_status_not_found(client): - """PUT /players/{player_id} with unknown ID returns 404 Not Found""" +def test_request_put_player_squadnumber_unknown_response_status_not_found(client): + """PUT /players/squadnumber/{squad_number} with unknown number returns 404 Not Found""" # Arrange - player_id = unknown_player().id + squad_number = unknown_player().squad_number player = unknown_player() # Act - response = client.put(PATH + str(player_id), json=player.__dict__) + response = client.put( + PATH + "squadnumber/" + str(squad_number), json=player.__dict__ + ) # Assert assert response.status_code == 404 -def test_request_put_player_id_existing_response_status_no_content(client): - """PUT /players/{player_id} with existing ID returns 204 No Content""" +def test_request_put_player_squadnumber_existing_response_status_no_content(client): + """PUT /players/squadnumber/{squad_number} with existing number returns 204 No Content""" # Arrange - player_id = existing_player().id + squad_number = existing_player().squad_number player = existing_player() player.first_name = "Emiliano" player.middle_name = "" # Act - response = client.put(PATH + str(player_id), json=player.__dict__) + response = client.put( + PATH + "squadnumber/" + str(squad_number), json=player.__dict__ + ) # Assert assert response.status_code == 204 -# DELETE /players/{player_id} -------------------------------------------------- +# DELETE /players/squadnumber/{squad_number} ----------------------------------- -def test_request_delete_player_id_unknown_response_status_not_found(client): - """DELETE /players/{player_id} with unknown ID returns 404 Not Found""" +def test_request_delete_player_squadnumber_unknown_response_status_not_found(client): + """DELETE /players/squadnumber/{squad_number} with unknown number returns 404 Not Found""" # Arrange - player_id = unknown_player().id + squad_number = unknown_player().squad_number # Act - response = client.delete(PATH + str(player_id)) + response = client.delete(PATH + "squadnumber/" + str(squad_number)) # Assert assert response.status_code == 404 -def test_request_delete_player_id_existing_response_status_no_content(client): - """DELETE /players/{player_id} with existing UUID returns 204 No Content""" - # Arrange — create the player to be deleted, then resolve its UUID +def test_request_delete_player_squadnumber_existing_response_status_no_content(client): + """DELETE /players/squadnumber/{squad_number} with existing number returns 204 No Content""" + # Arrange — create the player to be deleted player = nonexistent_player() client.post(PATH, json=player.__dict__) - lookup_response = client.get(PATH + "squadnumber/" + str(player.squad_number)) - player_id = lookup_response.json()["id"] # Act - response = client.delete(PATH + str(player_id)) + response = client.delete(PATH + "squadnumber/" + str(player.squad_number)) # Assert assert response.status_code == 204