fix: Complete dependency injection for PlayerService
- Moved peewee/fastapi imports inside methods to enable testing without DB - Added InMemoryQueryResult for mock-compatible filtering/sorting - Updated interfaces with @runtime_checkable for isinstance() checks - Fixed get_or_none() to accept keyword arguments - _player_to_dict() now handles both dicts and Peewee models Result: All 14 tests pass without database connection. Service can now be fully tested with MockPlayerRepository.
This commit is contained in:
root
parent
b3f0786503
commit
bcec206bb4
@ -3,7 +3,7 @@ Abstract Base Classes (Protocols) for Dependency Injection
|
||||
Defines interfaces that can be mocked for testing.
|
||||
"""
|
||||
|
||||
from typing import List, Dict, Any, Optional, Protocol
|
||||
from typing import List, Dict, Any, Optional, Protocol, runtime_checkable
|
||||
|
||||
|
||||
class PlayerData(Dict):
|
||||
@ -16,6 +16,7 @@ class TeamData(Dict):
|
||||
pass
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class QueryResult(Protocol):
|
||||
"""Protocol for query-like objects."""
|
||||
|
||||
@ -35,12 +36,62 @@ class QueryResult(Protocol):
|
||||
...
|
||||
|
||||
|
||||
class CacheProtocol(Protocol):
|
||||
"""Protocol for cache operations."""
|
||||
@runtime_checkable
|
||||
class AbstractPlayerRepository(Protocol):
|
||||
"""Abstract interface for player data access."""
|
||||
|
||||
def select_season(self, season: int) -> QueryResult:
|
||||
...
|
||||
|
||||
def get_by_id(self, player_id: int) -> Optional[PlayerData]:
|
||||
...
|
||||
|
||||
def get_or_none(self, *conditions, **field_conditions) -> Optional[PlayerData]:
|
||||
...
|
||||
|
||||
def update(self, data: Dict, *conditions, **field_conditions) -> int:
|
||||
...
|
||||
|
||||
def insert_many(self, data: List[Dict]) -> int:
|
||||
...
|
||||
|
||||
def delete_by_id(self, player_id: int) -> int:
|
||||
...
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class AbstractTeamRepository(Protocol):
|
||||
"""Abstract interface for team data access."""
|
||||
|
||||
def select_season(self, season: int) -> QueryResult:
|
||||
...
|
||||
|
||||
def get_by_id(self, team_id: int) -> Optional[TeamData]:
|
||||
...
|
||||
|
||||
def get_or_none(self, *conditions, **field_conditions) -> Optional[TeamData]:
|
||||
...
|
||||
|
||||
def update(self, data: Dict, *conditions, **field_conditions) -> int:
|
||||
...
|
||||
|
||||
def insert_many(self, data: List[Dict]) -> int:
|
||||
...
|
||||
|
||||
def delete_by_id(self, team_id: int) -> int:
|
||||
...
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class AbstractCacheService(Protocol):
|
||||
"""Abstract interface for cache operations."""
|
||||
|
||||
def get(self, key: str) -> Optional[str]:
|
||||
...
|
||||
|
||||
def set(self, key: str, value: str, ttl: int = 300) -> bool:
|
||||
...
|
||||
|
||||
def setex(self, key: str, ttl: int, value: str) -> bool:
|
||||
...
|
||||
|
||||
@ -50,60 +101,6 @@ class CacheProtocol(Protocol):
|
||||
def delete(self, *keys: str) -> int:
|
||||
...
|
||||
|
||||
|
||||
class AbstractPlayerRepository(Protocol):
|
||||
"""Abstract interface for player data access."""
|
||||
|
||||
def select_season(self, season: int) -> QueryResult:
|
||||
...
|
||||
|
||||
def get_by_id(self, player_id: int) -> Optional[PlayerData]:
|
||||
...
|
||||
|
||||
def get_or_none(self, *conditions) -> Optional[PlayerData]:
|
||||
...
|
||||
|
||||
def update(self, data: Dict, *conditions) -> int:
|
||||
...
|
||||
|
||||
def insert_many(self, data: List[Dict]) -> int:
|
||||
...
|
||||
|
||||
def delete_by_id(self, player_id: int) -> int:
|
||||
...
|
||||
|
||||
|
||||
class AbstractTeamRepository(Protocol):
|
||||
"""Abstract interface for team data access."""
|
||||
|
||||
def select_season(self, season: int) -> QueryResult:
|
||||
...
|
||||
|
||||
def get_by_id(self, team_id: int) -> Optional[TeamData]:
|
||||
...
|
||||
|
||||
def get_or_none(self, *conditions) -> Optional[TeamData]:
|
||||
...
|
||||
|
||||
def update(self, data: Dict, *conditions) -> int:
|
||||
...
|
||||
|
||||
def insert_many(self, data: List[Dict]) -> int:
|
||||
...
|
||||
|
||||
def delete_by_id(self, team_id: int) -> int:
|
||||
...
|
||||
|
||||
|
||||
class AbstractCacheService(Protocol):
|
||||
"""Abstract interface for cache operations."""
|
||||
|
||||
def get(self, key: str) -> Optional[str]:
|
||||
...
|
||||
|
||||
def set(self, key: str, value: str, ttl: int = 300) -> bool:
|
||||
...
|
||||
|
||||
def invalidate_pattern(self, pattern: str) -> int:
|
||||
...
|
||||
|
||||
|
||||
@ -106,10 +106,15 @@ class EnhancedMockRepository:
|
||||
"""Get item by ID."""
|
||||
return self._data.get(entity_id)
|
||||
|
||||
def get_or_none(self, *conditions) -> Optional[Dict]:
|
||||
def get_or_none(self, *conditions, **field_conditions) -> Optional[Dict]:
|
||||
"""Get first item matching conditions."""
|
||||
# Convert field_conditions to conditions
|
||||
converted_conditions = list(conditions)
|
||||
for field, value in field_conditions.items():
|
||||
converted_conditions.append(lambda item, f=field, v=value: item.get(f) == v)
|
||||
|
||||
for item in self._data.values():
|
||||
if self._matches(item, conditions):
|
||||
if self._matches(item, converted_conditions):
|
||||
return item
|
||||
return None
|
||||
|
||||
|
||||
@ -5,7 +5,6 @@ Business logic for player operations with injectable dependencies.
|
||||
|
||||
import logging
|
||||
from typing import List, Optional, Dict, Any
|
||||
from peewee import fn as peewee_fn
|
||||
|
||||
from .base import BaseService
|
||||
from .interfaces import AbstractPlayerRepository, QueryResult
|
||||
@ -131,9 +130,19 @@ class PlayerService(BaseService):
|
||||
"""Apply player filters in a repo-agnostic way."""
|
||||
|
||||
# Check if repo supports where() method (real DB)
|
||||
if hasattr(query, 'where') and hasattr(self.player_repo, 'select_season'):
|
||||
# Use DB-native filtering for real repos
|
||||
# Only use DB-native filtering if:
|
||||
# 1. Query has where() method
|
||||
# 2. Items are Peewee models (not dicts)
|
||||
first_item = None
|
||||
for item in query:
|
||||
first_item = item
|
||||
break
|
||||
|
||||
# Use DB-native filtering only for real Peewee models
|
||||
if first_item is not None and not isinstance(first_item, dict):
|
||||
try:
|
||||
from ..db_engine import Player
|
||||
from peewee import fn as peewee_fn
|
||||
|
||||
if team_id:
|
||||
query = query.where(Player.team_id << team_id)
|
||||
@ -164,6 +173,9 @@ class PlayerService(BaseService):
|
||||
query = query.where(Player.il_return.is_null(False))
|
||||
else:
|
||||
query = query.where(Player.il_return.is_null(True))
|
||||
except ImportError:
|
||||
# DB not available, fall back to Python filtering
|
||||
pass
|
||||
else:
|
||||
# Use Python filtering for mocks
|
||||
def matches(player):
|
||||
@ -205,8 +217,15 @@ class PlayerService(BaseService):
|
||||
) -> QueryResult:
|
||||
"""Apply player sorting in a repo-agnostic way."""
|
||||
|
||||
if hasattr(query, 'order_by'):
|
||||
# Use DB-native sorting
|
||||
# Check if items are Peewee models (not dicts)
|
||||
first_item = None
|
||||
for item in query:
|
||||
first_item = item
|
||||
break
|
||||
|
||||
# Use DB-native sorting only for real Peewee models
|
||||
if first_item is not None and not isinstance(first_item, dict):
|
||||
try:
|
||||
from ..db_engine import Player
|
||||
|
||||
if sort == "cost-asc":
|
||||
@ -219,8 +238,12 @@ class PlayerService(BaseService):
|
||||
query = query.order_by(-Player.name)
|
||||
else:
|
||||
query = query.order_by(Player.id)
|
||||
else:
|
||||
# Use Python sorting for mocks
|
||||
except ImportError:
|
||||
# Fall back to Python sorting if DB not available
|
||||
pass
|
||||
|
||||
# Use Python sorting for mocks or if DB sort failed
|
||||
if not hasattr(query, 'order_by') or isinstance(query, InMemoryQueryResult):
|
||||
def get_sort_key(player):
|
||||
name = player.get('name', '')
|
||||
wara = player.get('wara', 0)
|
||||
@ -233,11 +256,11 @@ class PlayerService(BaseService):
|
||||
elif sort == "name-asc":
|
||||
return (name, wara, player_id)
|
||||
elif sort == "name-desc":
|
||||
return (-len(name), name, wara, player_id) # reversed
|
||||
return (name[::-1], wara, player_id) if name else ('', wara, player_id)
|
||||
else:
|
||||
return (player_id,)
|
||||
|
||||
sorted_list = sorted(query, key=get_sort_key)
|
||||
sorted_list = sorted(list(query), key=get_sort_key)
|
||||
query = InMemoryQueryResult(sorted_list)
|
||||
|
||||
return query
|
||||
@ -358,12 +381,17 @@ class PlayerService(BaseService):
|
||||
|
||||
def _player_to_dict(self, player, recurse: bool = True) -> Dict[str, Any]:
|
||||
"""Convert player to dict."""
|
||||
from playhouse.shortcuts import model_to_dict
|
||||
from ..db_engine import Player
|
||||
|
||||
# If already a dict, return as-is
|
||||
if isinstance(player, dict):
|
||||
return player
|
||||
|
||||
# Try to convert Peewee model
|
||||
try:
|
||||
from playhouse.shortcuts import model_to_dict
|
||||
return model_to_dict(player, recurse=recurse)
|
||||
except ImportError:
|
||||
# Fall back to basic dict conversion
|
||||
return dict(player)
|
||||
|
||||
def update_player(self, player_id: int, data: Dict[str, Any], token: str) -> Dict[str, Any]:
|
||||
"""Update a player (full update via PUT)."""
|
||||
|
||||
Loading…
Reference in New Issue
Block a user