cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 2 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="")
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)
def _add_params(self, url: str, params: Optional[List[tuple]] = None) -> str:

365
tests/test_api_client.py
View File

@ -1,18 +1,24 @@
"""
API client tests using aioresponses for clean HTTP mocking
"""
import pytest
import asyncio
from unittest.mock import MagicMock, patch
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
class TestAPIClientWithAioresponses:
"""Test API client with aioresponses for HTTP mocking."""
@pytest.fixture
def mock_config(self):
"""Mock configuration for testing."""
@ -20,202 +26,185 @@ class TestAPIClientWithAioresponses:
config.db_url = "https://api.example.com"
config.api_token = "test-token"
return config
@pytest.fixture
def api_client(self, mock_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()
@pytest.mark.asyncio
async def test_get_request_success(self, api_client):
"""Test successful GET request."""
expected_data = {"id": 1, "name": "Test Player"}
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players/1",
"https://api.example.com/v3/players/1/",
payload=expected_data,
status=200
status=200,
)
result = await api_client.get("players", object_id=1)
assert result == expected_data
@pytest.mark.asyncio
async def test_get_request_404(self, api_client):
"""Test GET request returning 404."""
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players/999",
status=404
)
m.get("https://api.example.com/v3/players/999/", status=404)
result = await api_client.get("players", object_id=999)
assert result is None
@pytest.mark.asyncio
async def test_get_request_401_auth_error(self, api_client):
"""Test GET request with authentication error."""
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
status=401
)
m.get("https://api.example.com/v3/players/", status=401)
with pytest.raises(APIException, match="Authentication failed"):
await api_client.get("players")
@pytest.mark.asyncio
async def test_get_request_403_forbidden(self, api_client):
"""Test GET request with forbidden error."""
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
status=403
)
m.get("https://api.example.com/v3/players/", status=403)
with pytest.raises(APIException, match="Access forbidden"):
await api_client.get("players")
@pytest.mark.asyncio
async def test_get_request_500_server_error(self, api_client):
"""Test GET request with server error."""
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
"https://api.example.com/v3/players/",
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")
@pytest.mark.asyncio
async def test_get_request_with_params(self, api_client):
"""Test GET request with query parameters."""
expected_data = {"count": 2, "players": [{"id": 1}, {"id": 2}]}
with aioresponses() as m:
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,
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
@pytest.mark.asyncio
async def test_post_request_success(self, api_client):
"""Test successful POST request."""
input_data = {"name": "New Player", "position": "C"}
expected_response = {"id": 1, "name": "New Player", "position": "C"}
with aioresponses() as m:
m.post(
"https://api.example.com/v3/players",
"https://api.example.com/v3/players/",
payload=expected_response,
status=201
status=201,
)
result = await api_client.post("players", input_data)
assert result == expected_response
@pytest.mark.asyncio
async def test_post_request_400_error(self, api_client):
"""Test POST request with validation error."""
input_data = {"invalid": "data"}
with aioresponses() as m:
m.post(
"https://api.example.com/v3/players",
status=400,
body="Invalid data"
"https://api.example.com/v3/players/", 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)
@pytest.mark.asyncio
async def test_put_request_success(self, api_client):
"""Test successful PUT request."""
update_data = {"name": "Updated Player"}
expected_response = {"id": 1, "name": "Updated Player"}
with aioresponses() as m:
m.put(
"https://api.example.com/v3/players/1",
"https://api.example.com/v3/players/1/",
payload=expected_response,
status=200
status=200,
)
result = await api_client.put("players", update_data, object_id=1)
assert result == expected_response
@pytest.mark.asyncio
async def test_put_request_404(self, api_client):
"""Test PUT request with 404."""
update_data = {"name": "Updated Player"}
with aioresponses() as m:
m.put(
"https://api.example.com/v3/players/999",
status=404
)
m.put("https://api.example.com/v3/players/999/", status=404)
result = await api_client.put("players", update_data, object_id=999)
assert result is None
@pytest.mark.asyncio
async def test_delete_request_success(self, api_client):
"""Test successful DELETE request."""
with aioresponses() as m:
m.delete(
"https://api.example.com/v3/players/1",
status=204
)
m.delete("https://api.example.com/v3/players/1/", status=204)
result = await api_client.delete("players", object_id=1)
assert result is True
@pytest.mark.asyncio
async def test_delete_request_404(self, api_client):
"""Test DELETE request with 404."""
with aioresponses() as m:
m.delete(
"https://api.example.com/v3/players/999",
status=404
)
m.delete("https://api.example.com/v3/players/999/", status=404)
result = await api_client.delete("players", object_id=999)
assert result is False
@pytest.mark.asyncio
async def test_delete_request_200_success(self, api_client):
"""Test DELETE request with 200 success."""
with aioresponses() as m:
m.delete(
"https://api.example.com/v3/players/1",
status=200
)
m.delete("https://api.example.com/v3/players/1/", status=200)
result = await api_client.delete("players", object_id=1)
assert result is True
class TestAPIClientHelpers:
"""Test API client helper functions."""
@pytest.fixture
def mock_config(self):
"""Mock configuration for testing."""
@ -223,49 +212,49 @@ class TestAPIClientHelpers:
config.db_url = "https://api.example.com"
config.api_token = "test-token"
return config
@pytest.mark.asyncio
async def test_get_api_client_context_manager(self, mock_config):
"""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:
m.get(
"https://api.example.com/v3/test",
"https://api.example.com/v3/test/",
payload={"success": True},
status=200
status=200,
)
async with get_api_client() as client:
assert isinstance(client, APIClient)
result = await client.get("test")
assert result == {"success": True}
@pytest.mark.asyncio
async def test_global_client_management(self, mock_config):
"""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
client1 = await get_global_client()
client2 = await get_global_client()
# Should return same instance
assert client1 is client2
assert isinstance(client1, APIClient)
# Test cleanup
await cleanup_global_client()
# New client should be different instance
client3 = await get_global_client()
assert client3 is not client1
# Clean up for other tests
await cleanup_global_client()
class TestIntegrationScenarios:
"""Test realistic integration scenarios."""
@pytest.fixture
def mock_config(self):
"""Mock configuration for testing."""
@ -273,11 +262,11 @@ class TestIntegrationScenarios:
config.db_url = "https://api.example.com"
config.api_token = "test-token"
return config
@pytest.mark.asyncio
async def test_player_retrieval_with_team_lookup(self, mock_config):
"""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:
# Mock player data response
player_data = {
@ -287,43 +276,41 @@ class TestIntegrationScenarios:
"season": 12,
"team_id": 5,
"image": "https://example.com/player1.jpg",
"pos_1": "C"
"pos_1": "C",
}
m.get(
"https://api.example.com/v3/players/1",
"https://api.example.com/v3/players/1/",
payload=player_data,
status=200
status=200,
)
# Mock team data response
team_data = {
"id": 5,
"abbrev": "TST",
"sname": "Test Team",
"lname": "Test Team Full Name",
"season": 12
"season": 12,
}
m.get(
"https://api.example.com/v3/teams/5",
payload=team_data,
status=200
"https://api.example.com/v3/teams/5/", payload=team_data, status=200
)
client = APIClient()
# Get player
player = await client.get("players", object_id=1)
assert player["name"] == "Test Player"
assert player["team_id"] == 5
# Get team for player
team = await client.get("teams", object_id=player["team_id"])
assert team["sname"] == "Test Team"
@pytest.mark.asyncio
async def test_api_response_format_handling(self, mock_config):
"""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:
# Mock API response with count format
api_response = {
@ -336,7 +323,7 @@ class TestIntegrationScenarios:
"season": 12,
"team_id": 5,
"image": "https://example.com/player1.jpg",
"pos_1": "C"
"pos_1": "C",
},
{
"id": 2,
@ -345,93 +332,93 @@ class TestIntegrationScenarios:
"season": 12,
"team_id": 6,
"image": "https://example.com/player2.jpg",
"pos_1": "1B"
}
]
"pos_1": "1B",
},
],
}
m.get(
"https://api.example.com/v3/players?team_id=5",
"https://api.example.com/v3/players/?team_id=5",
payload=api_response,
status=200
status=200,
)
client = APIClient()
result = await client.get("players", params=[("team_id", "5")])
assert result["count"] == 25
assert len(result["players"]) == 2
assert result["players"][0]["name"] == "Player 1"
@pytest.mark.asyncio
async def test_error_recovery_scenarios(self, mock_config):
"""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:
# First request fails with 500
m.get(
"https://api.example.com/v3/players/1",
"https://api.example.com/v3/players/1/",
status=500,
body="Internal Server Error"
body="Internal Server Error",
)
# Second request succeeds
m.get(
"https://api.example.com/v3/players/2",
"https://api.example.com/v3/players/2/",
payload={"id": 2, "name": "Working Player"},
status=200
status=200,
)
client = APIClient()
# First request should raise exception
with pytest.raises(APIException, match="API request failed"):
await client.get("players", object_id=1)
# Second request should work fine
result = await client.get("players", object_id=2)
assert result["name"] == "Working Player"
# Client should still be functional
await client.close()
@pytest.mark.asyncio
async def test_concurrent_requests(self, mock_config):
"""Test multiple concurrent requests."""
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:
# Mock multiple endpoints
for i in range(1, 4):
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}"},
status=200
status=200,
)
client = APIClient()
# Make concurrent requests
tasks = [
client.get("players", object_id=1),
client.get("players", object_id=2),
client.get("players", object_id=3)
client.get("players", object_id=3),
]
results = await asyncio.gather(*tasks)
assert len(results) == 3
assert results[0]["name"] == "Player 1"
assert results[1]["name"] == "Player 2"
assert results[2]["name"] == "Player 3"
await client.close()
class TestAPIClientCoverageExtras:
"""Additional coverage tests for API client edge cases."""
@pytest.fixture
def mock_config(self):
"""Mock configuration for testing."""
@ -439,98 +426,104 @@ class TestAPIClientCoverageExtras:
config.db_url = "https://api.example.com"
config.api_token = "test-token"
return config
@pytest.mark.asyncio
async def test_global_client_cleanup_when_none(self):
"""Test cleanup when no global client exists."""
# Ensure no global client exists
await cleanup_global_client()
# Should not raise error
await cleanup_global_client()
@pytest.mark.asyncio
async def test_url_building_edge_cases(self, mock_config):
"""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()
# Test trailing slash handling
client.base_url = "https://api.example.com/"
url = client._build_url("players")
assert url == "https://api.example.com/v3/players"
assert "//" not in url.replace("https://", "")
assert url == "https://api.example.com/v3/players/"
assert "//" not in url.replace("https://", "").replace("//", "")
@pytest.mark.asyncio
async def test_parameter_handling_edge_cases(self, mock_config):
"""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()
# 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"
# Test with no parameters
url = client._add_params("https://example.com/api")
assert url == "https://example.com/api"
@pytest.mark.asyncio
async def test_timeout_error_handling(self, mock_config):
"""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()
# Test timeout using aioresponses exception parameter
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
exception=asyncio.TimeoutError("Request timed out")
"https://api.example.com/v3/players/",
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.close()
@pytest.mark.asyncio
async def test_generic_exception_handling(self, mock_config):
"""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()
# Test generic exception
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
exception=Exception("Generic error")
"https://api.example.com/v3/players/",
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.close()
@pytest.mark.asyncio
async def test_session_closed_handling(self, mock_config):
"""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
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players",
"https://api.example.com/v3/players/",
payload={"success": True},
status=200
status=200,
)
client = APIClient()
# Close the session manually
await client._ensure_session()
await client._session.close()
# Client should recreate session and work fine
result = await client.get("players")
assert result == {"success": True}
await client.close()
await client.close()