diff --git a/api/client.py b/api/client.py index b262fb9..4ce5425 100644 --- a/api/client.py +++ b/api/client.py @@ -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: diff --git a/services/decision_service.py b/services/decision_service.py index e101d0e..11a36b5 100644 --- a/services/decision_service.py +++ b/services/decision_service.py @@ -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}" diff --git a/services/play_service.py b/services/play_service.py index 7b08bf6..cdaf293 100644 --- a/services/play_service.py +++ b/services/play_service.py @@ -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}" diff --git a/tests/test_api_client.py b/tests/test_api_client.py index b0b16cc..c2ca5b2 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -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, )