fix: use targeted trailing slashes instead of universal (hotfix)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m7s

Reverts universal trailing slash in _build_url which broke custom_commands
endpoints (401 on /execute/). Instead, add trailing slashes only to the
two batch POST endpoints (plays/, decisions/) that need them to avoid
307 redirects dropping request bodies.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-03-09 17:50:58 -05:00
parent 9379ba587a
commit f6a25aa16d
4 changed files with 61 additions and 69 deletions

View File

@ -88,10 +88,6 @@ 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:

View File

@ -3,6 +3,7 @@ Decision Service
Manages pitching decision operations for game submission.
"""
from typing import List, Dict, Any, Optional, Tuple
from utils.logging import get_contextual_logger
@ -16,17 +17,14 @@ class DecisionService:
def __init__(self):
"""Initialize decision service."""
self.logger = get_contextual_logger(f'{__name__}.DecisionService')
self.logger = get_contextual_logger(f"{__name__}.DecisionService")
self._get_client = get_global_client
async def get_client(self):
"""Get the API client."""
return await self._get_client()
async def create_decisions_batch(
self,
decisions: List[Dict[str, Any]]
) -> bool:
async def create_decisions_batch(self, decisions: List[Dict[str, Any]]) -> bool:
"""
POST batch of decisions to /decisions endpoint.
@ -42,8 +40,10 @@ class DecisionService:
try:
client = await self.get_client()
payload = {'decisions': decisions}
await client.post('decisions', payload)
payload = {"decisions": decisions}
# Trailing slash required: without it, the server returns a 307 redirect
# and aiohttp drops the POST body when following the redirect
await client.post("decisions/", payload)
self.logger.info(f"Created {len(decisions)} decisions")
return True
@ -70,7 +70,7 @@ class DecisionService:
"""
try:
client = await self.get_client()
await client.delete(f'decisions/game/{game_id}')
await client.delete(f"decisions/game/{game_id}")
self.logger.info(f"Deleted decisions for game {game_id}")
return True
@ -80,9 +80,10 @@ class DecisionService:
raise APIException(f"Failed to delete decisions: {e}")
async def find_winning_losing_pitchers(
self,
decisions_data: List[Dict[str, Any]]
) -> Tuple[Optional[Player], Optional[Player], Optional[Player], List[Player], List[Player]]:
self, decisions_data: List[Dict[str, Any]]
) -> Tuple[
Optional[Player], Optional[Player], Optional[Player], List[Player], List[Player]
]:
"""
Extract WP, LP, SV, Holds, Blown Saves from decisions list and fetch Player objects.
@ -110,17 +111,17 @@ class DecisionService:
# First pass: Extract IDs
for decision in decisions_data:
pitcher_id = int(decision.get('pitcher_id', 0))
pitcher_id = int(decision.get("pitcher_id", 0))
if int(decision.get('win', 0)) == 1:
if int(decision.get("win", 0)) == 1:
wp_id = pitcher_id
if int(decision.get('loss', 0)) == 1:
if int(decision.get("loss", 0)) == 1:
lp_id = pitcher_id
if int(decision.get('is_save', 0)) == 1:
if int(decision.get("is_save", 0)) == 1:
sv_id = pitcher_id
if int(decision.get('hold', 0)) == 1:
if int(decision.get("hold", 0)) == 1:
hold_ids.append(pitcher_id)
if int(decision.get('b_save', 0)) == 1:
if int(decision.get("b_save", 0)) == 1:
bsv_ids.append(pitcher_id)
# Second pass: Fetch Player objects
@ -154,9 +155,9 @@ class DecisionService:
"""
error_str = str(error)
if 'Player ID' in error_str and 'not found' in error_str:
if "Player ID" in error_str and "not found" in error_str:
return "Invalid pitcher ID in decision data."
elif 'Game ID' in error_str and 'not found' in error_str:
elif "Game ID" in error_str and "not found" in error_str:
return "Game not found for decisions."
else:
return f"Error submitting decisions: {error_str}"

View File

@ -3,6 +3,7 @@ Play Service
Manages play-by-play data operations for game submission.
"""
from typing import List, Dict, Any
from utils.logging import get_contextual_logger
@ -16,7 +17,7 @@ class PlayService:
def __init__(self):
"""Initialize play service."""
self.logger = get_contextual_logger(f'{__name__}.PlayService')
self.logger = get_contextual_logger(f"{__name__}.PlayService")
self._get_client = get_global_client
async def get_client(self):
@ -39,8 +40,10 @@ class PlayService:
try:
client = await self.get_client()
payload = {'plays': plays}
response = await client.post('plays', payload)
payload = {"plays": plays}
# Trailing slash required: without it, the server returns a 307 redirect
# and aiohttp drops the POST body when following the redirect
response = await client.post("plays/", payload)
self.logger.info(f"Created {len(plays)} plays")
return True
@ -68,7 +71,7 @@ class PlayService:
"""
try:
client = await self.get_client()
response = await client.delete(f'plays/game/{game_id}')
response = await client.delete(f"plays/game/{game_id}")
self.logger.info(f"Deleted plays for game {game_id}")
return True
@ -77,11 +80,7 @@ class PlayService:
self.logger.error(f"Failed to delete plays for game {game_id}: {e}")
raise APIException(f"Failed to delete plays: {e}")
async def get_top_plays_by_wpa(
self,
game_id: int,
limit: int = 3
) -> List[Play]:
async def get_top_plays_by_wpa(self, game_id: int, limit: int = 3) -> List[Play]:
"""
Get top plays by WPA (absolute value) for key plays display.
@ -95,19 +94,15 @@ class PlayService:
try:
client = await self.get_client()
params = [
('game_id', game_id),
('sort', 'wpa-desc'),
('limit', limit)
]
params = [("game_id", game_id), ("sort", "wpa-desc"), ("limit", limit)]
response = await client.get('plays', params=params)
response = await client.get("plays", params=params)
if not response or 'plays' not in response:
self.logger.info(f'No plays found for game ID {game_id}')
if not response or "plays" not in response:
self.logger.info(f"No plays found for game ID {game_id}")
return []
plays = [Play.from_api_data(p) for p in response['plays']]
plays = [Play.from_api_data(p) for p in response["plays"]]
self.logger.debug(f"Retrieved {len(plays)} top plays for game {game_id}")
return plays
@ -129,11 +124,11 @@ class PlayService:
error_str = str(error)
# Common error patterns
if 'Player ID' in error_str and 'not found' in error_str:
if "Player ID" in error_str and "not found" in error_str:
return "Invalid player ID in scorecard data. Please check player IDs."
elif 'Game ID' in error_str and 'not found' in error_str:
elif "Game ID" in error_str and "not found" in error_str:
return "Game not found in database. Please contact an admin."
elif 'validation' in error_str.lower():
elif "validation" in error_str.lower():
return f"Data validation error: {error_str}"
else:
return f"Error submitting plays: {error_str}"

View File

@ -40,7 +40,7 @@ class TestAPIClientWithAioresponses:
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,
)
@ -53,7 +53,7 @@ class TestAPIClientWithAioresponses:
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)
@ -63,7 +63,7 @@ class TestAPIClientWithAioresponses:
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")
@ -72,7 +72,7 @@ class TestAPIClientWithAioresponses:
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")
@ -82,7 +82,7 @@ class TestAPIClientWithAioresponses:
"""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",
)
@ -99,7 +99,7 @@ class TestAPIClientWithAioresponses:
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,
)
@ -118,7 +118,7 @@ class TestAPIClientWithAioresponses:
with aioresponses() as m:
m.post(
"https://api.example.com/v3/players/",
"https://api.example.com/v3/players",
payload=expected_response,
status=201,
)
@ -134,7 +134,7 @@ class TestAPIClientWithAioresponses:
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(
@ -150,7 +150,7 @@ class TestAPIClientWithAioresponses:
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,
)
@ -165,7 +165,7 @@ class TestAPIClientWithAioresponses:
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)
@ -175,7 +175,7 @@ class TestAPIClientWithAioresponses:
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)
@ -185,7 +185,7 @@ class TestAPIClientWithAioresponses:
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)
@ -195,7 +195,7 @@ class TestAPIClientWithAioresponses:
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)
@ -219,7 +219,7 @@ class TestAPIClientHelpers:
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,
)
@ -279,7 +279,7 @@ class TestIntegrationScenarios:
"pos_1": "C",
}
m.get(
"https://api.example.com/v3/players/1/",
"https://api.example.com/v3/players/1",
payload=player_data,
status=200,
)
@ -293,7 +293,7 @@ class TestIntegrationScenarios:
"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()
@ -338,7 +338,7 @@ class TestIntegrationScenarios:
}
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,
)
@ -357,14 +357,14 @@ class TestIntegrationScenarios:
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",
)
# 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,
)
@ -392,7 +392,7 @@ class TestIntegrationScenarios:
# 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,
)
@ -445,8 +445,8 @@ class TestAPIClientCoverageExtras:
# 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://", "").replace("//", "")
assert url == "https://api.example.com/v3/players"
assert "//" not in url.replace("https://", "")
@pytest.mark.asyncio
async def test_parameter_handling_edge_cases(self, mock_config):
@ -473,7 +473,7 @@ class TestAPIClientCoverageExtras:
# Test timeout using aioresponses exception parameter
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players/",
"https://api.example.com/v3/players",
exception=asyncio.TimeoutError("Request timed out"),
)
@ -493,7 +493,7 @@ class TestAPIClientCoverageExtras:
# Test generic exception
with aioresponses() as m:
m.get(
"https://api.example.com/v3/players/",
"https://api.example.com/v3/players",
exception=Exception("Generic error"),
)
@ -511,7 +511,7 @@ class TestAPIClientCoverageExtras:
# 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,
)