cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: add trailing slashes to API URLs to prevent 307 redirects dropping POST bodies
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m13s
Details

Browse Source 
The FastAPI server returns 307 redirects for URLs without trailing slashes.
aiohttp follows these redirects but converts POST to GET, silently dropping
the request body. This caused play-by-play and decision data from
/submit-scorecard to never be persisted to the database despite the API
returning success.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-09 17:25:08 -05:00
parent b3b8cd9683
commit 9379ba587a
2 changed files with 183 additions and 186 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/client.py
View File

@ -88,6 +88,10 @@ class APIClient:
encoded_id = quote(str(object_id), safe="") encoded_id = quote(str(object_id), safe="")
path += f"/{encoded_id}" path += f"/{encoded_id}"
# Ensure trailing slash to prevent 307 redirects that drop POST bodies
if not path.endswith("/"):
path += "/"
return urljoin(self.base_url.rstrip("/") + "/", path) return urljoin(self.base_url.rstrip("/") + "/", path)
def _add_params(self, url: str, params: Optional[List[tuple]] = None) -> str: def _add_params(self, url: str, params: Optional[List[tuple]] = None) -> str:

173
tests/test_api_client.py
View File

@ -1,12 +1,18 @@
""" """
API client tests using aioresponses for clean HTTP mocking API client tests using aioresponses for clean HTTP mocking
""" """
import pytest import pytest
import asyncio import asyncio
from unittest.mock import MagicMock, patch from unittest.mock import MagicMock, patch
from aioresponses import aioresponses from aioresponses import aioresponses
from api.client import APIClient, get_api_client, get_global_client, cleanup_global_client from api.client import (
APIClient,
get_api_client,
get_global_client,
cleanup_global_client,
)
from exceptions import APIException from exceptions import APIException
@ -24,7 +30,7 @@ class TestAPIClientWithAioresponses:
@pytest.fixture @pytest.fixture
def api_client(self, mock_config): def api_client(self, mock_config):
"""Create API client with mocked config.""" """Create API client with mocked config."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
return APIClient() return APIClient()
@pytest.mark.asyncio @pytest.mark.asyncio
@ -34,9 +40,9 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players/1", "https://api.example.com/v3/players/1/",
payload=expected_data, payload=expected_data,
status=200 status=200,
) )
result = await api_client.get("players", object_id=1) result = await api_client.get("players", object_id=1)
@ -47,10 +53,7 @@ class TestAPIClientWithAioresponses:
async def test_get_request_404(self, api_client): async def test_get_request_404(self, api_client):
"""Test GET request returning 404.""" """Test GET request returning 404."""
with aioresponses() as m: with aioresponses() as m:
m.get( m.get("https://api.example.com/v3/players/999/", status=404)
"https://api.example.com/v3/players/999",
status=404
)
result = await api_client.get("players", object_id=999) result = await api_client.get("players", object_id=999)
@ -60,10 +63,7 @@ class TestAPIClientWithAioresponses:
async def test_get_request_401_auth_error(self, api_client): async def test_get_request_401_auth_error(self, api_client):
"""Test GET request with authentication error.""" """Test GET request with authentication error."""
with aioresponses() as m: with aioresponses() as m:
m.get( m.get("https://api.example.com/v3/players/", status=401)
"https://api.example.com/v3/players",
status=401
)
with pytest.raises(APIException, match="Authentication failed"): with pytest.raises(APIException, match="Authentication failed"):
await api_client.get("players") await api_client.get("players")
@ -72,10 +72,7 @@ class TestAPIClientWithAioresponses:
async def test_get_request_403_forbidden(self, api_client): async def test_get_request_403_forbidden(self, api_client):
"""Test GET request with forbidden error.""" """Test GET request with forbidden error."""
with aioresponses() as m: with aioresponses() as m:
m.get( m.get("https://api.example.com/v3/players/", status=403)
"https://api.example.com/v3/players",
status=403
)
with pytest.raises(APIException, match="Access forbidden"): with pytest.raises(APIException, match="Access forbidden"):
await api_client.get("players") await api_client.get("players")
@ -85,12 +82,14 @@ class TestAPIClientWithAioresponses:
"""Test GET request with server error.""" """Test GET request with server error."""
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/",
status=500, status=500,
body="Internal Server Error" body="Internal Server Error",
) )
with pytest.raises(APIException, match="API request failed with status 500"): with pytest.raises(
APIException, match="API request failed with status 500"
):
await api_client.get("players") await api_client.get("players")
@pytest.mark.asyncio @pytest.mark.asyncio
@ -100,12 +99,14 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players?team_id=5&season=12", "https://api.example.com/v3/players/?team_id=5&season=12",
payload=expected_data, payload=expected_data,
status=200 status=200,
) )
result = await api_client.get("players", params=[("team_id", "5"), ("season", "12")]) result = await api_client.get(
"players", params=[("team_id", "5"), ("season", "12")]
)
assert result == expected_data assert result == expected_data
@ -117,9 +118,9 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m: with aioresponses() as m:
m.post( m.post(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/",
payload=expected_response, payload=expected_response,
status=201 status=201,
) )
result = await api_client.post("players", input_data) result = await api_client.post("players", input_data)
@ -133,12 +134,12 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m: with aioresponses() as m:
m.post( m.post(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/", status=400, body="Invalid data"
status=400,
body="Invalid data"
) )
with pytest.raises(APIException, match="POST request failed with status 400"): with pytest.raises(
APIException, match="POST request failed with status 400"
):
await api_client.post("players", input_data) await api_client.post("players", input_data)
@pytest.mark.asyncio @pytest.mark.asyncio
@ -149,9 +150,9 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m: with aioresponses() as m:
m.put( m.put(
"https://api.example.com/v3/players/1", "https://api.example.com/v3/players/1/",
payload=expected_response, payload=expected_response,
status=200 status=200,
) )
result = await api_client.put("players", update_data, object_id=1) result = await api_client.put("players", update_data, object_id=1)
@ -164,10 +165,7 @@ class TestAPIClientWithAioresponses:
update_data = {"name": "Updated Player"} update_data = {"name": "Updated Player"}
with aioresponses() as m: with aioresponses() as m:
m.put( m.put("https://api.example.com/v3/players/999/", status=404)
"https://api.example.com/v3/players/999",
status=404
)
result = await api_client.put("players", update_data, object_id=999) result = await api_client.put("players", update_data, object_id=999)
@ -177,10 +175,7 @@ class TestAPIClientWithAioresponses:
async def test_delete_request_success(self, api_client): async def test_delete_request_success(self, api_client):
"""Test successful DELETE request.""" """Test successful DELETE request."""
with aioresponses() as m: with aioresponses() as m:
m.delete( m.delete("https://api.example.com/v3/players/1/", status=204)
"https://api.example.com/v3/players/1",
status=204
)
result = await api_client.delete("players", object_id=1) result = await api_client.delete("players", object_id=1)
@ -190,10 +185,7 @@ class TestAPIClientWithAioresponses:
async def test_delete_request_404(self, api_client): async def test_delete_request_404(self, api_client):
"""Test DELETE request with 404.""" """Test DELETE request with 404."""
with aioresponses() as m: with aioresponses() as m:
m.delete( m.delete("https://api.example.com/v3/players/999/", status=404)
"https://api.example.com/v3/players/999",
status=404
)
result = await api_client.delete("players", object_id=999) result = await api_client.delete("players", object_id=999)
@ -203,10 +195,7 @@ class TestAPIClientWithAioresponses:
async def test_delete_request_200_success(self, api_client): async def test_delete_request_200_success(self, api_client):
"""Test DELETE request with 200 success.""" """Test DELETE request with 200 success."""
with aioresponses() as m: with aioresponses() as m:
m.delete( m.delete("https://api.example.com/v3/players/1/", status=200)
"https://api.example.com/v3/players/1",
status=200
)
result = await api_client.delete("players", object_id=1) result = await api_client.delete("players", object_id=1)
@ -227,12 +216,12 @@ class TestAPIClientHelpers:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_api_client_context_manager(self, mock_config): async def test_get_api_client_context_manager(self, mock_config):
"""Test get_api_client context manager.""" """Test get_api_client context manager."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/test", "https://api.example.com/v3/test/",
payload={"success": True}, payload={"success": True},
status=200 status=200,
) )
async with get_api_client() as client: async with get_api_client() as client:
@ -243,7 +232,7 @@ class TestAPIClientHelpers:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_global_client_management(self, mock_config): async def test_global_client_management(self, mock_config):
"""Test global client getter and cleanup.""" """Test global client getter and cleanup."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
# Get global client # Get global client
client1 = await get_global_client() client1 = await get_global_client()
client2 = await get_global_client() client2 = await get_global_client()
@ -277,7 +266,7 @@ class TestIntegrationScenarios:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_player_retrieval_with_team_lookup(self, mock_config): async def test_player_retrieval_with_team_lookup(self, mock_config):
"""Test realistic scenario: get player with team data.""" """Test realistic scenario: get player with team data."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
with aioresponses() as m: with aioresponses() as m:
# Mock player data response # Mock player data response
player_data = { player_data = {
@ -287,12 +276,12 @@ class TestIntegrationScenarios:
"season": 12, "season": 12,
"team_id": 5, "team_id": 5,
"image": "https://example.com/player1.jpg", "image": "https://example.com/player1.jpg",
"pos_1": "C" "pos_1": "C",
} }
m.get( m.get(
"https://api.example.com/v3/players/1", "https://api.example.com/v3/players/1/",
payload=player_data, payload=player_data,
status=200 status=200,
) )
# Mock team data response # Mock team data response
@ -301,12 +290,10 @@ class TestIntegrationScenarios:
"abbrev": "TST", "abbrev": "TST",
"sname": "Test Team", "sname": "Test Team",
"lname": "Test Team Full Name", "lname": "Test Team Full Name",
"season": 12 "season": 12,
} }
m.get( m.get(
"https://api.example.com/v3/teams/5", "https://api.example.com/v3/teams/5/", payload=team_data, status=200
payload=team_data,
status=200
) )
client = APIClient() client = APIClient()
@ -323,7 +310,7 @@ class TestIntegrationScenarios:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_api_response_format_handling(self, mock_config): async def test_api_response_format_handling(self, mock_config):
"""Test handling of the API's count + list format.""" """Test handling of the API's count + list format."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
with aioresponses() as m: with aioresponses() as m:
# Mock API response with count format # Mock API response with count format
api_response = { api_response = {
@ -336,7 +323,7 @@ class TestIntegrationScenarios:
"season": 12, "season": 12,
"team_id": 5, "team_id": 5,
"image": "https://example.com/player1.jpg", "image": "https://example.com/player1.jpg",
"pos_1": "C" "pos_1": "C",
}, },
{ {
"id": 2, "id": 2,
@ -345,15 +332,15 @@ class TestIntegrationScenarios:
"season": 12, "season": 12,
"team_id": 6, "team_id": 6,
"image": "https://example.com/player2.jpg", "image": "https://example.com/player2.jpg",
"pos_1": "1B" "pos_1": "1B",
} },
] ],
} }
m.get( m.get(
"https://api.example.com/v3/players?team_id=5", "https://api.example.com/v3/players/?team_id=5",
payload=api_response, payload=api_response,
status=200 status=200,
) )
client = APIClient() client = APIClient()
@ -366,20 +353,20 @@ class TestIntegrationScenarios:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_error_recovery_scenarios(self, mock_config): async def test_error_recovery_scenarios(self, mock_config):
"""Test error handling and recovery.""" """Test error handling and recovery."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
with aioresponses() as m: with aioresponses() as m:
# First request fails with 500 # First request fails with 500
m.get( m.get(
"https://api.example.com/v3/players/1", "https://api.example.com/v3/players/1/",
status=500, status=500,
body="Internal Server Error" body="Internal Server Error",
) )
# Second request succeeds # Second request succeeds
m.get( m.get(
"https://api.example.com/v3/players/2", "https://api.example.com/v3/players/2/",
payload={"id": 2, "name": "Working Player"}, payload={"id": 2, "name": "Working Player"},
status=200 status=200,
) )
client = APIClient() client = APIClient()
@ -400,14 +387,14 @@ class TestIntegrationScenarios:
"""Test multiple concurrent requests.""" """Test multiple concurrent requests."""
import asyncio import asyncio
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
with aioresponses() as m: with aioresponses() as m:
# Mock multiple endpoints # Mock multiple endpoints
for i in range(1, 4): for i in range(1, 4):
m.get( m.get(
f"https://api.example.com/v3/players/{i}", f"https://api.example.com/v3/players/{i}/",
payload={"id": i, "name": f"Player {i}"}, payload={"id": i, "name": f"Player {i}"},
status=200 status=200,
) )
client = APIClient() client = APIClient()
@ -416,7 +403,7 @@ class TestIntegrationScenarios:
tasks = [ tasks = [
client.get("players", object_id=1), client.get("players", object_id=1),
client.get("players", object_id=2), client.get("players", object_id=2),
client.get("players", object_id=3) client.get("players", object_id=3),
] ]
results = await asyncio.gather(*tasks) results = await asyncio.gather(*tasks)
@ -452,23 +439,25 @@ class TestAPIClientCoverageExtras:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_url_building_edge_cases(self, mock_config): async def test_url_building_edge_cases(self, mock_config):
"""Test URL building with various edge cases.""" """Test URL building with various edge cases."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
client = APIClient() client = APIClient()
# Test trailing slash handling # Test trailing slash handling
client.base_url = "https://api.example.com/" client.base_url = "https://api.example.com/"
url = client._build_url("players") url = client._build_url("players")
assert url == "https://api.example.com/v3/players" assert url == "https://api.example.com/v3/players/"
assert "//" not in url.replace("https://", "") assert "//" not in url.replace("https://", "").replace("//", "")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_parameter_handling_edge_cases(self, mock_config): async def test_parameter_handling_edge_cases(self, mock_config):
"""Test parameter handling with various scenarios.""" """Test parameter handling with various scenarios."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
client = APIClient() client = APIClient()
# Test with existing query string # Test with existing query string
url = client._add_params("https://example.com/api?existing=true", [("new", "param")]) url = client._add_params(
"https://example.com/api?existing=true", [("new", "param")]
)
assert url == "https://example.com/api?existing=true&new=param" assert url == "https://example.com/api?existing=true&new=param"
# Test with no parameters # Test with no parameters
@ -478,17 +467,19 @@ class TestAPIClientCoverageExtras:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_timeout_error_handling(self, mock_config): async def test_timeout_error_handling(self, mock_config):
"""Test timeout error handling using aioresponses.""" """Test timeout error handling using aioresponses."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
client = APIClient() client = APIClient()
# Test timeout using aioresponses exception parameter # Test timeout using aioresponses exception parameter
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/",
exception=asyncio.TimeoutError("Request timed out") exception=asyncio.TimeoutError("Request timed out"),
) )
with pytest.raises(APIException, match="API call failed.*Request timed out"): with pytest.raises(
APIException, match="API call failed.*Request timed out"
):
await client.get("players") await client.get("players")
await client.close() await client.close()
@ -496,17 +487,19 @@ class TestAPIClientCoverageExtras:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_generic_exception_handling(self, mock_config): async def test_generic_exception_handling(self, mock_config):
"""Test generic exception handling.""" """Test generic exception handling."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
client = APIClient() client = APIClient()
# Test generic exception # Test generic exception
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/",
exception=Exception("Generic error") exception=Exception("Generic error"),
) )
with pytest.raises(APIException, match="API call failed.*Generic error"): with pytest.raises(
APIException, match="API call failed.*Generic error"
):
await client.get("players") await client.get("players")
await client.close() await client.close()
@ -514,13 +507,13 @@ class TestAPIClientCoverageExtras:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_session_closed_handling(self, mock_config): async def test_session_closed_handling(self, mock_config):
"""Test handling of closed session.""" """Test handling of closed session."""
with patch('api.client.get_config', return_value=mock_config): with patch("api.client.get_config", return_value=mock_config):
# Test that the client recreates session when needed # Test that the client recreates session when needed
with aioresponses() as m: with aioresponses() as m:
m.get( m.get(
"https://api.example.com/v3/players", "https://api.example.com/v3/players/",
payload={"success": True}, payload={"success": True},
status=200 status=200,
) )
client = APIClient() client = APIClient()