diff --git a/.gitignore b/.gitignore index b7faf40..853cd4f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,17 @@ __pycache__/ # C extensions *.so +# Bot-specific files +.last_command_hash +logs/ +*.log +*.json + +# Claude files & directories +.claude/ +CLAUDE.md +.env* + # Distribution / packaging .Python build/ diff --git a/api/__init__.py b/api/__init__.py new file mode 100644 index 0000000..4c9b54c --- /dev/null +++ b/api/__init__.py @@ -0,0 +1,8 @@ +""" +API client layer for Discord Bot v2.0 + +HTTP client for communicating with the database API. +""" +from .client import APIClient, get_api_client, get_global_client, cleanup_global_client + +__all__ = ['APIClient', 'get_api_client', 'get_global_client', 'cleanup_global_client'] \ No newline at end of file diff --git a/api/client.py b/api/client.py new file mode 100644 index 0000000..a5db7d2 --- /dev/null +++ b/api/client.py @@ -0,0 +1,424 @@ +""" +API client for Discord Bot v2.0 + +Modern aiohttp-based HTTP client for communicating with the database API. +Provides connection pooling, proper error handling, and session management. +""" +import aiohttp +import logging +from typing import Optional, List, Dict, Any, Union +from urllib.parse import urljoin +from contextlib import asynccontextmanager + +from config import get_config +from exceptions import APIException + +logger = logging.getLogger(f'{__name__}.APIClient') + + +class APIClient: + """ + Async HTTP client for SBA database API communication. + + Features: + - Connection pooling with proper session management + - Bearer token authentication + - Standardized v3 API usage + - Comprehensive error handling + - Debug logging with response truncation + """ + + def __init__(self, base_url: Optional[str] = None, api_token: Optional[str] = None): + """ + Initialize API client with configuration. + + Args: + base_url: Override default database URL from config + api_token: Override default API token from config + + Raises: + ValueError: If required configuration is missing + """ + config = get_config() + self.base_url = base_url or config.db_url + self.api_token = api_token or config.api_token + self._session: Optional[aiohttp.ClientSession] = None + + if not self.base_url: + raise ValueError("DB_URL must be configured") + if not self.api_token: + raise ValueError("API_TOKEN must be configured") + + logger.debug(f"APIClient initialized with base_url: {self.base_url}") + + @property + def headers(self) -> Dict[str, str]: + """Get headers with authentication and content type.""" + return { + 'Authorization': f'Bearer {self.api_token}', + 'Content-Type': 'application/json', + 'User-Agent': 'SBA-Discord-Bot-v2/1.0' + } + + def _build_url(self, endpoint: str, api_version: int = 3, object_id: Optional[int] = None) -> str: + """ + Build complete API URL from components. + + Args: + endpoint: API endpoint path + api_version: API version number (default: 3) + object_id: Optional object ID to append + + Returns: + Complete URL for API request + """ + # Handle already complete URLs + if endpoint.startswith(('http://', 'https://')) or '/api/' in endpoint: + return endpoint + + path = f"v{api_version}/{endpoint}" + if object_id is not None: + path += f"/{object_id}" + + return urljoin(self.base_url.rstrip('/') + '/', path) + + def _add_params(self, url: str, params: Optional[List[tuple]] = None) -> str: + """ + Add query parameters to URL. + + Args: + url: Base URL + params: List of (key, value) tuples + + Returns: + URL with query parameters appended + """ + if not params: + return url + + param_str = "&".join(f"{key}={value}" for key, value in params) + separator = "&" if "?" in url else "?" + return f"{url}{separator}{param_str}" + + async def _ensure_session(self) -> None: + """Ensure aiohttp session exists and is not closed.""" + if self._session is None or self._session.closed: + connector = aiohttp.TCPConnector( + limit=100, # Total connection pool size + limit_per_host=30, # Connections per host + ttl_dns_cache=300, # DNS cache TTL + use_dns_cache=True + ) + + timeout = aiohttp.ClientTimeout(total=30, connect=10) + + self._session = aiohttp.ClientSession( + headers=self.headers, + connector=connector, + timeout=timeout + ) + + logger.debug("Created new aiohttp session with connection pooling") + + async def get( + self, + endpoint: str, + object_id: Optional[int] = None, + params: Optional[List[tuple]] = None, + api_version: int = 3, + timeout: Optional[int] = None + ) -> Optional[Dict[str, Any]]: + """ + Make GET request to API. + + Args: + endpoint: API endpoint + object_id: Optional object ID + params: Query parameters + api_version: API version (default: 3) + timeout: Request timeout override + + Returns: + JSON response data or None for 404 + + Raises: + APIException: For HTTP errors or network issues + """ + url = self._build_url(endpoint, api_version, object_id) + url = self._add_params(url, params) + + await self._ensure_session() + + try: + logger.debug(f"GET: {endpoint} id: {object_id} params: {params}") + + request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None + + async with self._session.get(url, timeout=request_timeout) as response: + if response.status == 404: + logger.warning(f"Resource not found: {url}") + return None + elif response.status == 401: + logger.error(f"Authentication failed for: {url}") + raise APIException("Authentication failed - check API token") + elif response.status == 403: + logger.error(f"Access forbidden for: {url}") + raise APIException("Access forbidden - insufficient permissions") + elif response.status >= 400: + error_text = await response.text() + logger.error(f"API error {response.status}: {url} - {error_text}") + raise APIException(f"API request failed with status {response.status}: {error_text}") + + data = await response.json() + + # Truncate response for logging + data_str = str(data) + if len(data_str) > 1200: + log_data = data_str[:1200] + "..." + else: + log_data = data_str + logger.debug(f"Response: {log_data}") + + return data + + except aiohttp.ClientError as e: + logger.error(f"HTTP client error for {url}: {e}") + raise APIException(f"Network error: {e}") + except Exception as e: + logger.error(f"Unexpected error in GET {url}: {e}") + raise APIException(f"API call failed: {e}") + + async def post( + self, + endpoint: str, + data: Dict[str, Any], + api_version: int = 3, + timeout: Optional[int] = None + ) -> Optional[Dict[str, Any]]: + """ + Make POST request to API. + + Args: + endpoint: API endpoint + data: Request payload + api_version: API version (default: 3) + timeout: Request timeout override + + Returns: + JSON response data + + Raises: + APIException: For HTTP errors or network issues + """ + url = self._build_url(endpoint, api_version) + + await self._ensure_session() + + try: + logger.debug(f"POST: {endpoint} data: {data}") + + request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None + + async with self._session.post(url, json=data, timeout=request_timeout) as response: + if response.status == 401: + logger.error(f"Authentication failed for POST: {url}") + raise APIException("Authentication failed - check API token") + elif response.status == 403: + logger.error(f"Access forbidden for POST: {url}") + raise APIException("Access forbidden - insufficient permissions") + elif response.status not in [200, 201]: + error_text = await response.text() + logger.error(f"POST error {response.status}: {url} - {error_text}") + raise APIException(f"POST request failed with status {response.status}: {error_text}") + + result = await response.json() + + # Truncate response for logging + result_str = str(result) + if len(result_str) > 1200: + log_result = result_str[:1200] + "..." + else: + log_result = result_str + logger.debug(f"POST Response: {log_result}") + + return result + + except aiohttp.ClientError as e: + logger.error(f"HTTP client error for POST {url}: {e}") + raise APIException(f"Network error: {e}") + except Exception as e: + logger.error(f"Unexpected error in POST {url}: {e}") + raise APIException(f"POST failed: {e}") + + async def put( + self, + endpoint: str, + data: Dict[str, Any], + object_id: Optional[int] = None, + api_version: int = 3, + timeout: Optional[int] = None + ) -> Optional[Dict[str, Any]]: + """ + Make PUT request to API. + + Args: + endpoint: API endpoint + data: Request payload + object_id: Optional object ID + api_version: API version (default: 3) + timeout: Request timeout override + + Returns: + JSON response data + + Raises: + APIException: For HTTP errors or network issues + """ + url = self._build_url(endpoint, api_version, object_id) + + await self._ensure_session() + + try: + logger.debug(f"PUT: {endpoint} id: {object_id} data: {data}") + + request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None + + async with self._session.put(url, json=data, timeout=request_timeout) as response: + if response.status == 401: + logger.error(f"Authentication failed for PUT: {url}") + raise APIException("Authentication failed - check API token") + elif response.status == 403: + logger.error(f"Access forbidden for PUT: {url}") + raise APIException("Access forbidden - insufficient permissions") + elif response.status == 404: + logger.warning(f"Resource not found for PUT: {url}") + return None + elif response.status not in [200, 201]: + error_text = await response.text() + logger.error(f"PUT error {response.status}: {url} - {error_text}") + raise APIException(f"PUT request failed with status {response.status}: {error_text}") + + result = await response.json() + logger.debug(f"PUT Response: {str(result)[:1200]}{'...' if len(str(result)) > 1200 else ''}") + return result + + except aiohttp.ClientError as e: + logger.error(f"HTTP client error for PUT {url}: {e}") + raise APIException(f"Network error: {e}") + except Exception as e: + logger.error(f"Unexpected error in PUT {url}: {e}") + raise APIException(f"PUT failed: {e}") + + async def delete( + self, + endpoint: str, + object_id: Optional[int] = None, + api_version: int = 3, + timeout: Optional[int] = None + ) -> bool: + """ + Make DELETE request to API. + + Args: + endpoint: API endpoint + object_id: Optional object ID + api_version: API version (default: 3) + timeout: Request timeout override + + Returns: + True if deletion successful, False if resource not found + + Raises: + APIException: For HTTP errors or network issues + """ + url = self._build_url(endpoint, api_version, object_id) + + await self._ensure_session() + + try: + logger.debug(f"DELETE: {endpoint} id: {object_id}") + + request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None + + async with self._session.delete(url, timeout=request_timeout) as response: + if response.status == 401: + logger.error(f"Authentication failed for DELETE: {url}") + raise APIException("Authentication failed - check API token") + elif response.status == 403: + logger.error(f"Access forbidden for DELETE: {url}") + raise APIException("Access forbidden - insufficient permissions") + elif response.status == 404: + logger.warning(f"Resource not found for DELETE: {url}") + return False + elif response.status not in [200, 204]: + error_text = await response.text() + logger.error(f"DELETE error {response.status}: {url} - {error_text}") + raise APIException(f"DELETE request failed with status {response.status}: {error_text}") + + logger.debug(f"DELETE successful: {url}") + return True + + except aiohttp.ClientError as e: + logger.error(f"HTTP client error for DELETE {url}: {e}") + raise APIException(f"Network error: {e}") + except Exception as e: + logger.error(f"Unexpected error in DELETE {url}: {e}") + raise APIException(f"DELETE failed: {e}") + + async def close(self) -> None: + """Close the HTTP session and clean up resources.""" + if self._session and not self._session.closed: + await self._session.close() + logger.debug("Closed aiohttp session") + + async def __aenter__(self): + """Async context manager entry.""" + await self._ensure_session() + return self + + async def __aexit__(self, exc_type, exc_val, exc_tb): + """Async context manager exit with cleanup.""" + await self.close() + + +@asynccontextmanager +async def get_api_client() -> APIClient: + """ + Get API client as async context manager. + + Usage: + async with get_api_client() as client: + data = await client.get('players') + """ + client = APIClient() + try: + yield client + finally: + await client.close() + + +# Global API client instance for reuse +_global_client: Optional[APIClient] = None + + +async def get_global_client() -> APIClient: + """ + Get global API client instance with automatic session management. + + Returns: + Shared APIClient instance + """ + global _global_client + if _global_client is None: + _global_client = APIClient() + + await _global_client._ensure_session() + return _global_client + + +async def cleanup_global_client() -> None: + """Clean up global API client. Call during bot shutdown.""" + global _global_client + if _global_client: + await _global_client.close() + _global_client = None \ No newline at end of file diff --git a/bot.py b/bot.py new file mode 100644 index 0000000..6812ae0 --- /dev/null +++ b/bot.py @@ -0,0 +1,369 @@ +""" +Discord Bot v2.0 - Main Entry Point + +Modern discord.py bot with application commands and proper error handling. +""" +import asyncio +import hashlib +import json +import logging +import os +from logging.handlers import RotatingFileHandler + +import discord +from discord.ext import commands + +from config import get_config +from exceptions import BotException +from api.client import get_global_client, cleanup_global_client + + +def setup_logging(): + """Configure hybrid logging: human-readable console + structured JSON files.""" + from utils.logging import JSONFormatter + + # Create logs directory if it doesn't exist + os.makedirs('logs', exist_ok=True) + + # Configure root logger + config = get_config() + logger = logging.getLogger('discord_bot_v2') + logger.setLevel(getattr(logging, config.log_level.upper())) + + # Console handler - human readable for development + console_handler = logging.StreamHandler() + console_formatter = logging.Formatter( + '%(asctime)s - %(name)s - %(levelname)s - %(message)s' + ) + console_handler.setFormatter(console_formatter) + logger.addHandler(console_handler) + + # Traditional file handler - human readable with debug info + file_handler = RotatingFileHandler( + 'logs/discord_bot_v2.log', + maxBytes=5 * 1024 * 1024, # 5MB + backupCount=5 + ) + file_formatter = logging.Formatter( + '%(asctime)s - %(name)s - %(levelname)s - %(funcName)s:%(lineno)d - %(message)s' + ) + file_handler.setFormatter(file_formatter) + logger.addHandler(file_handler) + + # JSON file handler - structured logging for analysis + json_handler = RotatingFileHandler( + 'logs/discord_bot_v2.json', + maxBytes=5 * 1024 * 1024, # 5MB + backupCount=5 + ) + json_handler.setFormatter(JSONFormatter()) + logger.addHandler(json_handler) + + # Apply to all loggers (not just root) + root_logger = logging.getLogger() + root_logger.setLevel(getattr(logging, config.log_level.upper())) + + # Add handlers to root logger so all child loggers inherit them + if not root_logger.handlers: # Avoid duplicate handlers + root_logger.addHandler(console_handler) + root_logger.addHandler(file_handler) + root_logger.addHandler(json_handler) + + return logger + + +class SBABot(commands.Bot): + """Custom bot class for SBA league management.""" + + def __init__(self): + # Configure intents + intents = discord.Intents.default() + intents.message_content = True # For legacy commands if needed + intents.members = True # For member management + + super().__init__( + command_prefix='!', # Legacy prefix, primarily using slash commands + intents=intents, + description="SBA League Management Bot v2.0" + ) + + self.logger = logging.getLogger('discord_bot_v2') + + async def setup_hook(self): + """Called when the bot is starting up.""" + self.logger.info("Setting up bot...") + + # Load command packages + await self._load_command_packages() + + # Smart command syncing: auto-sync in development if changes detected + config = get_config() + if config.is_development: + if await self._should_sync_commands(): + self.logger.info("Development mode: changes detected, syncing commands...") + await self._sync_commands() + await self._save_command_hash() + else: + self.logger.info("Development mode: no command changes detected, skipping sync") + else: + self.logger.info("Production mode: commands loaded but not auto-synced") + self.logger.info("Use /sync command to manually sync when needed") + + async def _load_command_packages(self): + """Load all command packages with resilient error handling.""" + from commands.players import setup_players + + # Define command packages to load + command_packages = [ + ("players", setup_players), + # Future packages: + # ("teams", setup_teams), + # ("league", setup_league), + # ("admin", setup_admin), + ] + + total_successful = 0 + total_failed = 0 + + for package_name, setup_func in command_packages: + try: + self.logger.info(f"Loading {package_name} commands...") + successful, failed, failed_modules = await setup_func(self) + total_successful += successful + total_failed += failed + + if failed == 0: + self.logger.info(f"✅ {package_name} commands loaded successfully ({successful} cogs)") + else: + self.logger.warning(f"⚠️ {package_name} commands partially loaded: {successful} successful, {failed} failed") + + except Exception as e: + self.logger.error(f"❌ Failed to load {package_name} package: {e}", exc_info=True) + total_failed += 1 + + # Log overall summary + if total_failed == 0: + self.logger.info(f"🎉 All command packages loaded successfully ({total_successful} total cogs)") + else: + self.logger.warning(f"⚠️ Command loading completed with issues: {total_successful} successful, {total_failed} failed") + + async def _should_sync_commands(self) -> bool: + """Check if commands have changed since last sync.""" + try: + # Create hash of current command tree + commands_data = [] + for cmd in self.tree.get_commands(): + # Include relevant command data for comparison + cmd_dict = { + 'name': cmd.name, + 'description': cmd.description, + 'parameters': [ + { + 'name': param.name, + 'description': param.description, + 'required': param.required, + 'type': str(param.type) + } for param in cmd.parameters + ] if hasattr(cmd, 'parameters') else [] + } + commands_data.append(cmd_dict) + + # Sort for consistent hashing + commands_data.sort(key=lambda x: x['name']) + current_hash = hashlib.md5( + json.dumps(commands_data, sort_keys=True).encode() + ).hexdigest() + + # Compare with stored hash + hash_file = '.last_command_hash' + if os.path.exists(hash_file): + with open(hash_file, 'r') as f: + last_hash = f.read().strip() + return current_hash != last_hash + else: + # No previous hash = first run, should sync + return True + + except Exception as e: + self.logger.warning(f"Error checking command hash: {e}") + # If we can't determine changes, err on the side of syncing + return True + + async def _save_command_hash(self): + """Save current command hash for future comparison.""" + try: + # Create hash of current command tree (same logic as _should_sync_commands) + commands_data = [] + for cmd in self.tree.get_commands(): + cmd_dict = { + 'name': cmd.name, + 'description': cmd.description, + 'parameters': [ + { + 'name': param.name, + 'description': param.description, + 'required': param.required, + 'type': str(param.type) + } for param in cmd.parameters + ] if hasattr(cmd, 'parameters') else [] + } + commands_data.append(cmd_dict) + + commands_data.sort(key=lambda x: x['name']) + current_hash = hashlib.md5( + json.dumps(commands_data, sort_keys=True).encode() + ).hexdigest() + + # Save hash to file + with open('.last_command_hash', 'w') as f: + f.write(current_hash) + + except Exception as e: + self.logger.warning(f"Error saving command hash: {e}") + + async def _sync_commands(self): + """Internal method to sync commands.""" + config = get_config() + if config.guild_id: + guild = discord.Object(id=config.guild_id) + self.tree.copy_global_to(guild=guild) + synced = await self.tree.sync(guild=guild) + self.logger.info(f"Synced {len(synced)} commands to guild {config.guild_id}") + else: + synced = await self.tree.sync() + self.logger.info(f"Synced {len(synced)} commands globally") + + async def on_ready(self): + """Called when the bot is ready.""" + self.logger.info(f"Bot ready! Logged in as {self.user}") + self.logger.info(f"Connected to {len(self.guilds)} guilds") + + # Set activity status + activity = discord.Activity( + type=discord.ActivityType.watching, + name="SBA League Management" + ) + await self.change_presence(activity=activity) + + async def on_error(self, event_method: str, /, *args, **kwargs): + """Global error handler for events.""" + self.logger.error(f"Error in event {event_method}", exc_info=True) + + +# Create global bot instance +bot = SBABot() + + +@bot.tree.command(name="health", description="Check bot and API health status") +async def health_command(interaction: discord.Interaction): + """Health check command to verify bot and API connectivity.""" + logger = logging.getLogger('discord_bot_v2') + + try: + # Check API connectivity + api_status = "✅ Connected" + try: + client = await get_global_client() + # Test API with a simple request + result = await client.get('current') + if result: + api_status = "✅ Connected" + else: + api_status = "⚠️ API returned no data" + except Exception as e: + logger.error(f"API health check failed: {e}") + api_status = f"❌ Error: {str(e)}" + + # Bot health info + bot_uptime = discord.utils.utcnow() - bot.user.created_at if bot.user else None + guild_count = len(bot.guilds) + + # Create health status embed + embed = discord.Embed( + title="🏥 Bot Health Check", + color=discord.Color.green(), + timestamp=discord.utils.utcnow() + ) + + embed.add_field(name="Bot Status", value="✅ Online", inline=True) + embed.add_field(name="API Status", value=api_status, inline=True) + embed.add_field(name="Guilds", value=str(guild_count), inline=True) + embed.add_field(name="Latency", value=f"{bot.latency*1000:.1f}ms", inline=True) + + if bot.user: + embed.set_footer(text=f"Bot: {bot.user.name}", icon_url=bot.user.display_avatar.url) + + await interaction.response.send_message(embed=embed, ephemeral=True) + + except Exception as e: + logger.error(f"Health check command error: {e}", exc_info=True) + await interaction.response.send_message( + f"❌ Health check failed: {str(e)}", + ephemeral=True + ) + + +@bot.tree.error +async def on_app_command_error(interaction: discord.Interaction, error: discord.app_commands.AppCommandError): + """Global error handler for application commands.""" + logger = logging.getLogger('discord_bot_v2') + + # Handle specific error types + if isinstance(error, discord.app_commands.CommandOnCooldown): + await interaction.response.send_message( + f"⏰ Command on cooldown. Try again in {error.retry_after:.1f} seconds.", + ephemeral=True + ) + elif isinstance(error, discord.app_commands.MissingPermissions): + await interaction.response.send_message( + "❌ You don't have permission to use this command.", + ephemeral=True + ) + elif isinstance(error, discord.app_commands.CommandNotFound): + await interaction.response.send_message( + "❌ Command not found. Use `/help` to see available commands.", + ephemeral=True + ) + elif isinstance(error, BotException): + # Our custom exceptions - show user-friendly message + await interaction.response.send_message( + f"❌ {str(error)}", + ephemeral=True + ) + else: + # Unexpected errors - log and show generic message + logger.error(f"Unhandled command error: {error}", exc_info=True) + + message = "❌ An unexpected error occurred. Please try again." + config = get_config() + if config.is_development: + message += f"\n\nDevelopment error: {str(error)}" + + if interaction.response.is_done(): + await interaction.followup.send(message, ephemeral=True) + else: + await interaction.response.send_message(message, ephemeral=True) + + +async def main(): + """Main entry point.""" + logger = setup_logging() + + config = get_config() + logger.info("Starting Discord Bot v2.0") + logger.info(f"Environment: {config.environment}") + logger.info(f"Guild ID: {config.guild_id}") + + try: + await bot.start(config.bot_token) + except KeyboardInterrupt: + logger.info("Received keyboard interrupt, shutting down...") + except Exception as e: + logger.error(f"Fatal error: {e}", exc_info=True) + finally: + await cleanup_global_client() + await bot.close() + + +if __name__ == "__main__": + asyncio.run(main()) \ No newline at end of file diff --git a/commands/README.md b/commands/README.md new file mode 100644 index 0000000..b475dd4 --- /dev/null +++ b/commands/README.md @@ -0,0 +1,451 @@ +# Commands Package Documentation +**Discord Bot v2.0 - Scalable Command Architecture** + +This document outlines the command architecture, patterns, and best practices established for the SBA Discord Bot v2.0. + +## 📁 Architecture Overview + +### **Package Structure** +``` +commands/ +├── README.md # This documentation +├── __init__.py # Future: Global command utilities +└── players/ # Player-related commands + ├── __init__.py # Package setup with resilient loading + └── info.py # Player information commands +``` + +### **Future Expansion (Phase 2+)** +``` +commands/ +├── README.md +├── __init__.py +├── players/ # ✅ COMPLETED +│ ├── __init__.py +│ ├── info.py # /player command +│ ├── search.py # /player-search, /player-lookup +│ ├── stats.py # /player-stats, /player-compare +│ └── rankings.py # /player-rankings, /leaderboard +├── teams/ # 🔄 PLANNED +│ ├── __init__.py +│ ├── roster.py # /team-roster, /team-depth +│ ├── stats.py # /team-stats, /team-leaders +│ └── schedule.py # /team-schedule, /team-record +├── league/ # 🔄 PLANNED +│ ├── __init__.py +│ ├── standings.py # /standings, /playoff-race +│ ├── schedule.py # /schedule, /scores +│ └── leaders.py # /leaders, /awards +├── draft/ # 🔄 PLANNED +│ ├── __init__.py +│ ├── picks.py # /draft-pick, /draft-order +│ ├── board.py # /draft-board, /draft-list +│ └── timer.py # /draft-status, /draft-timer +├── transactions/ # 🔄 PLANNED +│ ├── __init__.py +│ ├── trades.py # /trade, /trade-history +│ ├── waivers.py # /waivers, /free-agents +│ └── history.py # /transaction-history +├── admin/ # 🔄 PLANNED +│ ├── __init__.py +│ ├── league.py # /admin-season, /admin-week +│ ├── draft.py # /admin-draft, /admin-timer +│ └── system.py # /health, /sync-commands +└── utils/ # 🔄 PLANNED + ├── __init__.py + ├── dice.py # /roll, /dice + └── fun.py # Fun/misc commands +``` + +## 🏗️ Design Principles + +### **1. Single Responsibility** +- Each file handles 2-4 closely related commands +- Clear logical grouping by domain (players, teams, etc.) +- Focused functionality reduces complexity + +### **2. Resilient Loading** +- One failed cog doesn't break the entire package +- Loop-based loading with comprehensive error handling +- Clear logging for debugging and monitoring + +### **3. Scalable Architecture** +- Easy to add new packages and cogs +- Consistent patterns across all command groups +- Future-proof structure for bot growth + +### **4. Modern Discord.py Patterns** +- Application commands (slash commands) only +- Proper error handling with user-friendly messages +- Async/await throughout +- Type hints and comprehensive documentation + +## 🔧 Implementation Patterns + +### **Command Package Structure** + +#### **Individual Command File (e.g., `players/info.py`)** +```python +""" +Player Information Commands + +Implements slash commands for displaying player information and statistics. +""" +import logging +from typing import Optional + +import discord +from discord.ext import commands + +from services.player_service import player_service +from exceptions import BotException + +logger = logging.getLogger(f'{__name__}.PlayerInfoCommands') + + +class PlayerInfoCommands(commands.Cog): + """Player information and statistics command handlers.""" + + def __init__(self, bot: commands.Bot): + self.bot = bot + + @discord.app_commands.command( + name="player", + description="Display player information and statistics" + ) + @discord.app_commands.describe( + name="Player name to search for", + season="Season to show stats for (defaults to current season)" + ) + async def player_info( + self, + interaction: discord.Interaction, + name: str, + season: Optional[int] = None + ): + """Display player card with statistics.""" + try: + # Always defer for potentially slow API calls + await interaction.response.defer() + + # Command implementation here + # Use logger for error logging + # Create Discord embeds for responses + + except Exception as e: + logger.error(f"Player info command error: {e}", exc_info=True) + error_msg = "❌ Error retrieving player information." + + if interaction.response.is_done(): + await interaction.followup.send(error_msg, ephemeral=True) + else: + await interaction.response.send_message(error_msg, ephemeral=True) + + +async def setup(bot: commands.Bot): + """Load the player info commands cog.""" + await bot.add_cog(PlayerInfoCommands(bot)) +``` + +#### **Package __init__.py with Resilient Loading** +```python +""" +Player Commands Package + +This package contains all player-related Discord commands organized into focused modules. +""" +import logging +from discord.ext import commands + +from .info import PlayerInfoCommands +# Future imports: +# from .search import PlayerSearchCommands +# from .stats import PlayerStatsCommands + +logger = logging.getLogger(__name__) + + +async def setup_players(bot: commands.Bot): + """ + Setup all player command modules. + + Returns: + tuple: (successful_count, failed_count, failed_modules) + """ + # Define all player command cogs to load + player_cogs = [ + ("PlayerInfoCommands", PlayerInfoCommands), + # Future cogs: + # ("PlayerSearchCommands", PlayerSearchCommands), + # ("PlayerStatsCommands", PlayerStatsCommands), + ] + + successful = 0 + failed = 0 + failed_modules = [] + + for cog_name, cog_class in player_cogs: + try: + await bot.add_cog(cog_class(bot)) + logger.info(f"✅ Loaded {cog_name}") + successful += 1 + except Exception as e: + logger.error(f"❌ Failed to load {cog_name}: {e}", exc_info=True) + failed += 1 + failed_modules.append(cog_name) + + # Log summary + if failed == 0: + logger.info(f"🎉 All {successful} player command modules loaded successfully") + else: + logger.warning(f"⚠️ Player commands loaded with issues: {successful} successful, {failed} failed") + + return successful, failed, failed_modules + + +# Export the setup function for easy importing +__all__ = ['setup_players', 'PlayerInfoCommands'] +``` + +## 🔄 Smart Command Syncing + +### **Hash-Based Change Detection** +The bot implements smart command syncing that only updates Discord when commands actually change: + +**Development Mode:** +- Automatically detects command changes using SHA-256 hashing +- Only syncs when changes are detected +- Saves hash to `.last_command_hash` for comparison +- Prevents unnecessary Discord API calls + +**Production Mode:** +- No automatic syncing +- Commands must be manually synced using `/sync` command +- Prevents accidental command updates in production + +### **How It Works** +1. **Hash Generation**: Creates hash of command names, descriptions, and parameters +2. **Comparison**: Compares current hash with stored hash from `.last_command_hash` +3. **Conditional Sync**: Only syncs if hashes differ or no previous hash exists +4. **Hash Storage**: Saves new hash after successful sync + +### **Benefits** +- ✅ **API Efficiency**: Avoids Discord rate limits +- ✅ **Development Speed**: Fast restarts when no command changes +- ✅ **Production Safety**: No accidental command updates +- ✅ **Consistency**: Commands stay consistent across restarts + +## 🚀 Bot Integration + +### **Command Loading in bot.py** +```python +async def setup_hook(self): + """Called when the bot is starting up.""" + # Load command packages + await self._load_command_packages() + + # Smart command syncing: auto-sync in development if changes detected + config = get_config() + if config.is_development: + if await self._should_sync_commands(): + self.logger.info("Development mode: changes detected, syncing commands...") + await self._sync_commands() + await self._save_command_hash() + else: + self.logger.info("Development mode: no command changes detected, skipping sync") + else: + self.logger.info("Production mode: commands loaded but not auto-synced") + +async def _load_command_packages(self): + """Load all command packages with resilient error handling.""" + from commands.players import setup_players + + # Define command packages to load + command_packages = [ + ("players", setup_players), + # Future packages: + # ("teams", setup_teams), + # ("league", setup_league), + ] + + # Loop-based loading with error isolation + for package_name, setup_func in command_packages: + try: + successful, failed, failed_modules = await setup_func(self) + # Log results + except Exception as e: + self.logger.error(f"❌ Failed to load {package_name} package: {e}") +``` + +## 📋 Development Guidelines + +### **Adding New Command Packages** + +#### **1. Create Package Structure** +```bash +mkdir commands/teams +touch commands/teams/__init__.py +touch commands/teams/roster.py +``` + +#### **2. Implement Command Module** +- Follow the pattern from `players/info.py` +- Use module-level logger: `logger = logging.getLogger(f'{__name__}.ClassName')` +- Always defer responses: `await interaction.response.defer()` +- Comprehensive error handling with user-friendly messages +- Type hints and docstrings + +#### **3. Create Package Setup Function** +- Follow the pattern from `players/__init__.py` +- Use loop-based cog loading with error isolation +- Return tuple: `(successful, failed, failed_modules)` +- Comprehensive logging with emojis for quick scanning + +#### **4. Register in Bot** +- Add import to `_load_command_packages()` in `bot.py` +- Add to `command_packages` list +- Test in development environment + +### **Adding Commands to Existing Packages** + +#### **1. Create New Command Module** +```python +# commands/players/search.py +class PlayerSearchCommands(commands.Cog): + # Implementation + pass + +async def setup(bot: commands.Bot): + await bot.add_cog(PlayerSearchCommands(bot)) +``` + +#### **2. Update Package __init__.py** +```python +from .search import PlayerSearchCommands + +# Add to player_cogs list +player_cogs = [ + ("PlayerInfoCommands", PlayerInfoCommands), + ("PlayerSearchCommands", PlayerSearchCommands), # New cog +] +``` + +#### **3. Test Import Structure** +```python +# Verify imports work +from commands.players import setup_players +from commands.players.search import PlayerSearchCommands +``` + +## 🎯 Best Practices + +### **Command Implementation** +1. **Always defer responses** for API calls: `await interaction.response.defer()` +2. **Use ephemeral responses** for errors: `ephemeral=True` +3. **Comprehensive error handling** with try/except blocks +4. **User-friendly error messages** with emojis +5. **Proper logging** with context and stack traces +6. **Type hints** on all parameters and return values +7. **Descriptive docstrings** for commands and methods + +### **Package Organization** +1. **2-4 commands per file** maximum +2. **Logical grouping** by functionality/domain +3. **Consistent naming** patterns across packages +4. **Module-level logging** for clean, consistent logs +5. **Loop-based cog loading** for error resilience +6. **Comprehensive return values** from setup functions + +### **Error Handling** +1. **Package-level isolation** - one failed cog doesn't break the package +2. **Clear error logging** with stack traces for debugging +3. **User-friendly messages** that don't expose internal errors +4. **Graceful degradation** when possible +5. **Metric reporting** for monitoring (success/failure counts) + +## 📊 Monitoring & Metrics + +### **Startup Logging** +The command loading system provides comprehensive metrics: + +``` +INFO - Loading players commands... +INFO - ✅ Loaded PlayerInfoCommands +INFO - 🎉 All 1 player command modules loaded successfully +INFO - ✅ players commands loaded successfully (1 cogs) +INFO - 🎉 All command packages loaded successfully (1 total cogs) +``` + +### **Error Scenarios** +``` +ERROR - ❌ Failed to load PlayerInfoCommands: +WARNING - ⚠️ Player commands loaded with issues: 0 successful, 1 failed +WARNING - Failed modules: PlayerInfoCommands +``` + +### **Command Sync Logging** +``` +INFO - Development mode: changes detected, syncing commands... +INFO - Synced 1 commands to guild 123456789 +``` + +or + +``` +INFO - Development mode: no command changes detected, skipping sync +``` + +## 🔧 Troubleshooting + +### **Common Issues** + +#### **Import Errors** +- Check that `__init__.py` files exist in all packages +- Verify cog class names match imports +- Ensure service dependencies are available + +#### **Command Not Loading** +- Check logs for specific error messages +- Verify cog is added to the package's cog list +- Test individual module imports in Python REPL + +#### **Commands Not Syncing** +- Check if running in development mode (`config.is_development`) +- Verify `.last_command_hash` file permissions +- Use manual `/sync` command for troubleshooting +- Check Discord API rate limits + +#### **Performance Issues** +- Monitor command loading times in logs +- Check for unnecessary API calls during startup +- Verify hash-based sync is working correctly + +### **Debugging Tips** +1. **Use the logs** - comprehensive logging shows exactly what's happening +2. **Test imports individually** - isolate package/module issues +3. **Check hash file** - verify command change detection is working +4. **Monitor Discord API** - watch for rate limiting or errors +5. **Use development mode** - auto-sync helps debug command issues + +## 🚦 Future Enhancements + +### **Planned Features** +- **Command Groups**: Discord.py command groups for better organization (`/player info`, `/player stats`) +- **Permission Decorators**: Role-based command restrictions per package +- **Dynamic Loading**: Hot-reload commands without bot restart +- **Usage Metrics**: Command usage tracking and analytics +- **Rate Limiting**: Per-command rate limiting for resource management + +### **Architecture Improvements** +- **Shared Utilities**: Common embed builders, decorators, helpers +- **Configuration**: Per-package configuration and feature flags +- **Testing**: Automated testing for command packages +- **Documentation**: Auto-generated command documentation +- **Monitoring**: Health checks and performance metrics per package + +This architecture provides a solid foundation for scaling the Discord bot while maintaining code quality, reliability, and developer productivity. + +--- + +**Last Updated:** Phase 2.1 - Command Package Conversion +**Next Review:** After implementing teams/ and league/ packages \ No newline at end of file diff --git a/commands/__init__.py b/commands/__init__.py new file mode 100644 index 0000000..ae7fa85 --- /dev/null +++ b/commands/__init__.py @@ -0,0 +1,5 @@ +""" +Discord command handlers for Bot v2.0 + +Modern slash command implementations with proper error handling. +""" \ No newline at end of file diff --git a/commands/players/__init__.py b/commands/players/__init__.py new file mode 100644 index 0000000..78a77b9 --- /dev/null +++ b/commands/players/__init__.py @@ -0,0 +1,60 @@ +""" +Player Commands Package + +This package contains all player-related Discord commands organized into focused modules: +- info.py: Player information and statistics display +""" +import logging +from discord.ext import commands + +from .info import PlayerInfoCommands + +logger = logging.getLogger(__name__) + + +async def setup_players(bot: commands.Bot): + """ + Setup all player command modules. + + Args: + bot: The Discord bot instance + + Returns: + tuple: (successful_count, failed_count, failed_modules) + """ + # Define all player command cogs to load + player_cogs = [ + ("PlayerInfoCommands", PlayerInfoCommands), + # Future player command modules: + # ("PlayerSearchCommands", PlayerSearchCommands), + # ("PlayerStatsCommands", PlayerStatsCommands), + # ("PlayerCompareCommands", PlayerCompareCommands), + ] + + successful = 0 + failed = 0 + failed_modules = [] + + for cog_name, cog_class in player_cogs: + try: + await bot.add_cog(cog_class(bot)) + logger.info(f"✅ Loaded {cog_name}") + successful += 1 + except Exception as e: + logger.error(f"❌ Failed to load {cog_name}: {e}", exc_info=True) + failed += 1 + failed_modules.append(cog_name) + + # Log summary + if failed == 0: + logger.info(f"🎉 All {successful} player command modules loaded successfully") + else: + logger.warning(f"⚠️ Player commands loaded with issues: {successful} successful, {failed} failed") + if failed_modules: + logger.warning(f"Failed modules: {', '.join(failed_modules)}") + + return successful, failed, failed_modules + + +# Export the setup function for easy importing +__all__ = ['setup_players', 'PlayerInfoCommands'] \ No newline at end of file diff --git a/commands/players/info.py b/commands/players/info.py new file mode 100644 index 0000000..a2ec99e --- /dev/null +++ b/commands/players/info.py @@ -0,0 +1,179 @@ +""" +Player Information Commands + +Implements slash commands for displaying player information and statistics. +""" +from typing import Optional + +import discord +from discord.ext import commands + +from services.player_service import player_service +from exceptions import BotException +from utils.logging import get_contextual_logger, set_discord_context +from constants import SBA_CURRENT_SEASON + + +class PlayerInfoCommands(commands.Cog): + """Player information and statistics command handlers.""" + + def __init__(self, bot: commands.Bot): + self.bot = bot + self.logger = get_contextual_logger(f'{__name__}.PlayerInfoCommands') + + @discord.app_commands.command( + name="player", + description="Display player information and statistics" + ) + @discord.app_commands.describe( + name="Player name to search for", + season="Season to show stats for (defaults to current season)" + ) + async def player_info( + self, + interaction: discord.Interaction, + name: str, + season: Optional[int] = None + ): + """Display player card with statistics.""" + # Set up logging context for this command + set_discord_context( + interaction=interaction, + command="/player", + player_name=name, + season=season + ) + + # Start operation timing and tracing + trace_id = self.logger.start_operation("player_info_command") + + try: + self.logger.info("Player info command started") + + # Defer response for potentially slow API calls + await interaction.response.defer() + self.logger.debug("Response deferred") + + # Search for player by name (use season parameter or default to current) + search_season = season or SBA_CURRENT_SEASON + self.logger.debug("Starting player search", api_call="get_players_by_name", season=search_season) + players = await player_service.get_players_by_name(name, search_season) + self.logger.info("Player search completed", players_found=len(players), season=search_season) + + if not players: + self.logger.warning("No players found for search", search_term=name) + await interaction.followup.send( + f"❌ No players found matching '{name}'.", + ephemeral=True + ) + return + + # If multiple players, try exact match first + player = None + if len(players) == 1: + player = players[0] + self.logger.debug("Single player found", player_id=player.id, player_name=player.name) + else: + self.logger.debug("Multiple players found, attempting exact match", candidate_count=len(players)) + + # Try exact match + for p in players: + if p.name.lower() == name.lower(): + player = p + self.logger.debug("Exact match found", player_id=player.id, player_name=player.name) + break + + if not player: + # Show multiple options + candidate_names = [p.name for p in players[:10]] + self.logger.info("Multiple candidates found, requiring user clarification", + candidates=candidate_names) + + player_list = "\n".join([f"• {p.name} ({p.primary_position})" for p in players[:10]]) + await interaction.followup.send( + f"🔍 Multiple players found for '{name}':\n{player_list}\n\nPlease be more specific.", + ephemeral=True + ) + return + + # Get player with team information + self.logger.debug("Fetching player with team information", + player_id=player.id, + api_call="get_player_with_team") + + player_with_team = await player_service.get_player_with_team(player.id) + if not player_with_team: + self.logger.warning("Failed to get player with team, using basic player data") + player_with_team = player # Fallback to player without team + else: + team_info = f"{player_with_team.team.abbrev}" if hasattr(player_with_team, 'team') and player_with_team.team else "No team" + self.logger.debug("Player with team information retrieved", team=team_info) + + # Create player embed + self.logger.debug("Creating Discord embed") + embed = discord.Embed( + title=f"🏟️ {player_with_team.name}", + color=discord.Color.blue(), + timestamp=discord.utils.utcnow() + ) + + # Basic info + embed.add_field( + name="Position", + value=player_with_team.primary_position, + inline=True + ) + + if hasattr(player_with_team, 'team') and player_with_team.team: + embed.add_field( + name="Team", + value=f"{player_with_team.team.abbrev} - {player_with_team.team.sname}", + inline=True + ) + + embed.add_field( + name="WARA", + value=f"{player_with_team.wara:.1f}", + inline=True + ) + + season_text = season or player_with_team.season + embed.add_field( + name="Season", + value=str(season_text), + inline=True + ) + + # All positions if multiple + if len(player_with_team.positions) > 1: + embed.add_field( + name="All Positions", + value=", ".join(player_with_team.positions), + inline=True + ) + + # Player image if available + if player_with_team.image: + embed.set_thumbnail(url=player_with_team.image) + self.logger.debug("Player image added to embed", image_url=player_with_team.image) + + embed.set_footer(text=f"Player ID: {player_with_team.id}") + + await interaction.followup.send(embed=embed) + self.logger.info("Player info command completed successfully", + final_player_id=player_with_team.id, + final_player_name=player_with_team.name) + + except Exception as e: + self.logger.error("Player info command failed", error=e) + error_msg = "❌ Error retrieving player information." + + if interaction.response.is_done(): + await interaction.followup.send(error_msg, ephemeral=True) + else: + await interaction.response.send_message(error_msg, ephemeral=True) + + +async def setup(bot: commands.Bot): + """Load the player info commands cog.""" + await bot.add_cog(PlayerInfoCommands(bot)) \ No newline at end of file diff --git a/config.py b/config.py new file mode 100644 index 0000000..49023c4 --- /dev/null +++ b/config.py @@ -0,0 +1,57 @@ +""" +Configuration management for Discord Bot v2.0 +""" +import os +from typing import Optional +from pydantic_settings import BaseSettings +from pydantic import ConfigDict + + +class BotConfig(BaseSettings): + """Application configuration with environment variable support.""" + + # Discord settings + bot_token: str + guild_id: int + + # Database API settings + api_token: str + db_url: str + + # League settings + sba_season: int = 12 + pd_season: int = 9 + fa_lock_week: int = 14 + sba_color: str = "a6ce39" + + # Application settings + log_level: str = "INFO" + environment: str = "development" + testing: bool = False + + model_config = ConfigDict( + env_file=".env", + case_sensitive=False, + extra="ignore" # Ignore extra environment variables + ) + + @property + def is_development(self) -> bool: + """Check if running in development mode.""" + return self.environment.lower() == "development" + + @property + def is_testing(self) -> bool: + """Check if running in test mode.""" + return self.testing + + +# Global configuration instance - lazily initialized to avoid import-time errors +_config = None + +def get_config() -> BotConfig: + """Get the global configuration instance.""" + global _config + if _config is None: + _config = BotConfig() + return _config \ No newline at end of file diff --git a/constants.py b/constants.py new file mode 100644 index 0000000..3b48c90 --- /dev/null +++ b/constants.py @@ -0,0 +1,34 @@ +""" +Application constants for Discord Bot v2.0 +""" + +# Discord Limits +DISCORD_EMBED_LIMIT = 6000 +DISCORD_FIELD_VALUE_LIMIT = 1024 +DISCORD_EMBED_DESCRIPTION_LIMIT = 4096 + +# League Constants +WEEKS_PER_SEASON = 18 +GAMES_PER_WEEK = 4 +MODERN_STATS_START_SEASON = 8 + +# Current Season Constants +SBA_CURRENT_SEASON = 12 +PD_CURRENT_SEASON = 9 + +# API Constants +API_VERSION = "v3" +DEFAULT_TIMEOUT = 10 +MAX_RETRIES = 3 + +# Baseball Positions +PITCHER_POSITIONS = {"SP", "RP", "P"} +POSITION_FIELDERS = {"C", "1B", "2B", "3B", "SS", "LF", "CF", "RF", "OF", "DH"} +ALL_POSITIONS = PITCHER_POSITIONS | POSITION_FIELDERS + +# Draft Constants +DEFAULT_PICK_MINUTES = 10 +DRAFT_ROUNDS = 25 + +# Special Team IDs +FREE_AGENT_TEAM_ID = 31 # Generic free agent team ID (same per season) \ No newline at end of file diff --git a/exceptions.py b/exceptions.py new file mode 100644 index 0000000..bbda2be --- /dev/null +++ b/exceptions.py @@ -0,0 +1,41 @@ +""" +Custom exceptions for Discord Bot v2.0 + +Uses modern error handling patterns with discord.py's built-in error handling. +No decorators - rely on global error handlers and explicit try/catch blocks. +""" + + +class BotException(Exception): + """Base exception for all bot-related errors.""" + pass + + +class APIException(BotException): + """Exception for API-related errors.""" + pass + + +class PlayerNotFoundError(BotException): + """Raised when a requested player cannot be found.""" + pass + + +class TeamNotFoundError(BotException): + """Raised when a requested team cannot be found.""" + pass + + +class DraftException(BotException): + """Exception for draft-related errors.""" + pass + + +class ValidationException(BotException): + """Exception for data validation errors.""" + pass + + +class ConfigurationException(BotException): + """Exception for configuration-related errors.""" + pass \ No newline at end of file diff --git a/models/__init__.py b/models/__init__.py new file mode 100644 index 0000000..19847a8 --- /dev/null +++ b/models/__init__.py @@ -0,0 +1,23 @@ +""" +Data models for Discord Bot v2.0 + +Clean Pydantic models with proper validation and type safety. +""" + +from models.base import SBABaseModel +from models.team import Team +from models.player import Player +from models.current import Current +from models.draft_pick import DraftPick +from models.draft_data import DraftData +from models.draft_list import DraftList + +__all__ = [ + 'SBABaseModel', + 'Team', + 'Player', + 'Current', + 'DraftPick', + 'DraftData', + 'DraftList', +] \ No newline at end of file diff --git a/models/base.py b/models/base.py new file mode 100644 index 0000000..fc3c769 --- /dev/null +++ b/models/base.py @@ -0,0 +1,40 @@ +""" +Base model for all SBA entities + +Provides common functionality for data validation, serialization, and API interaction. +""" +from pydantic import BaseModel +from typing import Optional, Dict, Any +from datetime import datetime + + +class SBABaseModel(BaseModel): + """Base model for all SBA entities with common functionality.""" + + model_config = { + "validate_assignment": True, + "use_enum_values": True, + "arbitrary_types_allowed": True, + "json_encoders": { + datetime: lambda v: v.isoformat() if v else None + } + } + + id: Optional[int] = None + created_at: Optional[datetime] = None + updated_at: Optional[datetime] = None + + def __repr__(self): + fields = ', '.join(f'{k}={v}' for k, v in self.model_dump(exclude_none=True).items()) + return f"{self.__class__.__name__}({fields})" + + def to_dict(self, exclude_none: bool = True) -> Dict[str, Any]: + """Convert model to dictionary, optionally excluding None values.""" + return self.model_dump(exclude_none=exclude_none) + + @classmethod + def from_api_data(cls, data: Dict[str, Any]): + """Create model instance from API response data.""" + if not data: + raise ValueError(f"Cannot create {cls.__name__} from empty data") + return cls(**data) \ No newline at end of file diff --git a/models/current.py b/models/current.py new file mode 100644 index 0000000..fb3aa88 --- /dev/null +++ b/models/current.py @@ -0,0 +1,42 @@ +""" +Current league state model + +Represents the current state of the league including week, season, and settings. +""" +from pydantic import Field, field_validator + +from models.base import SBABaseModel + + +class Current(SBABaseModel): + """Model representing current league state and settings.""" + + week: int = Field(69, description="Current week number") + season: int = Field(69, description="Current season number") + freeze: bool = Field(True, description="Whether league is frozen") + bet_week: str = Field('sheets', description="Betting week identifier") + trade_deadline: int = Field(1, description="Trade deadline week") + pick_trade_start: int = Field(69, description="Draft pick trading start week") + pick_trade_end: int = Field(420, description="Draft pick trading end week") + playoffs_begin: int = Field(420, description="Week when playoffs begin") + + @field_validator("bet_week", mode="before") + @classmethod + def cast_bet_week_to_string(cls, v): + """Ensure bet_week is always a string.""" + return str(v) if v is not None else 'sheets' + + @property + def is_offseason(self) -> bool: + """Check if league is currently in offseason.""" + return self.week > 18 + + @property + def is_playoffs(self) -> bool: + """Check if league is currently in playoffs.""" + return self.week >= self.playoffs_begin + + @property + def can_trade_picks(self) -> bool: + """Check if draft pick trading is currently allowed.""" + return self.pick_trade_start <= self.week <= self.pick_trade_end \ No newline at end of file diff --git a/models/draft_data.py b/models/draft_data.py new file mode 100644 index 0000000..ddb295e --- /dev/null +++ b/models/draft_data.py @@ -0,0 +1,45 @@ +""" +Draft configuration and state model + +Represents the current draft settings and timer state. +""" +from typing import Optional +from datetime import datetime +from pydantic import Field, field_validator + +from models.base import SBABaseModel + + +class DraftData(SBABaseModel): + """Draft configuration and state model.""" + + currentpick: int = Field(0, description="Current pick number in progress") + timer: bool = Field(False, description="Whether draft timer is active") + pick_deadline: Optional[datetime] = Field(None, description="Deadline for current pick") + result_channel_id: int = Field(..., description="Discord channel ID for draft results") + ping_channel_id: int = Field(..., description="Discord channel ID for draft pings") + pick_minutes: int = Field(1, description="Minutes allowed per pick") + + @field_validator("result_channel_id", "ping_channel_id", mode="before") + @classmethod + def cast_channel_ids_to_int(cls, v): + """Ensure channel IDs are integers.""" + if isinstance(v, str): + return int(v) + return v + + @property + def is_draft_active(self) -> bool: + """Check if the draft is currently active.""" + return self.timer + + @property + def is_pick_expired(self) -> bool: + """Check if the current pick deadline has passed.""" + if not self.pick_deadline: + return False + return datetime.now() > self.pick_deadline + + def __str__(self): + status = "Active" if self.is_draft_active else "Inactive" + return f"Draft {status}: Pick {self.currentpick} ({self.pick_minutes}min timer)" \ No newline at end of file diff --git a/models/draft_list.py b/models/draft_list.py new file mode 100644 index 0000000..e2364fd --- /dev/null +++ b/models/draft_list.py @@ -0,0 +1,34 @@ +""" +Draft preference list model + +Represents team draft board rankings and preferences. +""" +from typing import Optional +from pydantic import Field + +from models.base import SBABaseModel +from models.team import Team +from models.player import Player + + +class DraftList(SBABaseModel): + """Draft preference list entry for a team.""" + + season: int = Field(..., description="Draft season") + team_id: int = Field(..., description="Team ID that owns this list entry") + rank: int = Field(..., description="Ranking of player on team's draft board") + player_id: int = Field(..., description="Player ID on the draft board") + + # Related objects (populated when needed) + team: Optional[Team] = Field(None, description="Team object (populated when needed)") + player: Optional[Player] = Field(None, description="Player object (populated when needed)") + + @property + def is_top_ranked(self) -> bool: + """Check if this is the team's top-ranked available player.""" + return self.rank == 1 + + def __str__(self): + team_str = self.team.abbrev if self.team else f"Team {self.team_id}" + player_str = self.player.name if self.player else f"Player {self.player_id}" + return f"{team_str} Draft Board #{self.rank}: {player_str}" \ No newline at end of file diff --git a/models/draft_pick.py b/models/draft_pick.py new file mode 100644 index 0000000..3a39dc5 --- /dev/null +++ b/models/draft_pick.py @@ -0,0 +1,47 @@ +""" +Draft pick model + +Represents individual draft picks with team and player relationships. +""" +from typing import Optional +from pydantic import Field + +from models.base import SBABaseModel +from models.team import Team +from models.player import Player + + +class DraftPick(SBABaseModel): + """Draft pick model representing a single draft selection.""" + + season: int = Field(..., description="Draft season") + overall: int = Field(..., description="Overall pick number") + round: int = Field(..., description="Draft round") + + # Team relationships - IDs are required, objects are optional + origowner_id: int = Field(..., description="Original owning team ID") + origowner: Optional[Team] = Field(None, description="Original owning team (populated when needed)") + + owner_id: Optional[int] = Field(None, description="Current owning team ID") + owner: Optional[Team] = Field(None, description="Current owning team (populated when needed)") + + # Player selection + player_id: Optional[int] = Field(None, description="Selected player ID") + player: Optional[Player] = Field(None, description="Selected player (populated when needed)") + + @property + def is_traded(self) -> bool: + """Check if this pick has been traded.""" + return self.origowner_id != self.owner_id + + @property + def is_selected(self) -> bool: + """Check if a player has been selected with this pick.""" + return self.player_id is not None + + def __str__(self): + team_str = f"({self.owner.abbrev})" if self.owner else f"(Team {self.owner_id})" + if self.is_selected and self.player: + return f"Pick {self.overall}: {self.player.name} {team_str}" + else: + return f"Pick {self.overall}: Available {team_str}" \ No newline at end of file diff --git a/models/player.py b/models/player.py new file mode 100644 index 0000000..c992a76 --- /dev/null +++ b/models/player.py @@ -0,0 +1,105 @@ +""" +Player model for SBA players + +Represents a player with team relationships and position information. +""" +from typing import Optional, List +from pydantic import Field + +from models.base import SBABaseModel +from models.team import Team + + +class Player(SBABaseModel): + """Player model representing an SBA player.""" + + name: str = Field(..., description="Player full name") + wara: float = Field(..., description="Wins Above Replacement Average") + season: int = Field(..., description="Season number") + + # Team relationship + team_id: int = Field(..., description="Team ID this player belongs to") + team: Optional[Team] = Field(None, description="Team object (populated when needed)") + + # Images and media + image: str = Field(..., description="Primary player image URL") + image2: Optional[str] = Field(None, description="Secondary player image URL") + vanity_card: Optional[str] = Field(None, description="Custom vanity card URL") + headshot: Optional[str] = Field(None, description="Player headshot URL") + + # Positions (up to 8 positions) + pos_1: str = Field(..., description="Primary position") + pos_2: Optional[str] = None + pos_3: Optional[str] = None + pos_4: Optional[str] = None + pos_5: Optional[str] = None + pos_6: Optional[str] = None + pos_7: Optional[str] = None + pos_8: Optional[str] = None + + # Injury and status information + pitcher_injury: Optional[int] = Field(None, description="Pitcher injury rating") + injury_rating: Optional[str] = Field(None, description="General injury rating") + il_return: Optional[str] = Field(None, description="Injured list return date") + demotion_week: Optional[int] = Field(None, description="Week of demotion") + + # Game tracking + last_game: Optional[str] = Field(None, description="Last game played") + last_game2: Optional[str] = Field(None, description="Second to last game played") + + # External identifiers + strat_code: Optional[str] = Field(None, description="Strat-o-matic code") + bbref_id: Optional[str] = Field(None, description="Baseball Reference ID") + sbaplayer_id: Optional[int] = Field(None, description="SBA player ID") + + @property + def positions(self) -> List[str]: + """Return list of all positions this player can play.""" + positions = [] + for i in range(1, 9): + pos = getattr(self, f'pos_{i}', None) + if pos: + positions.append(pos) + return positions + + @property + def primary_position(self) -> str: + """Return the player's primary position.""" + return self.pos_1 + + @classmethod + def from_api_data(cls, data: dict) -> 'Player': + """ + Create Player instance from API data, handling nested team structure. + + The API returns team data as a nested object, but our model expects + both team_id (int) and team (optional Team object). + """ + # Make a copy to avoid modifying original data + player_data = data.copy() + + # Handle nested team structure + if 'team' in player_data and isinstance(player_data['team'], dict): + team_data = player_data['team'] + # Extract team_id from nested team object + player_data['team_id'] = team_data.get('id') + # Keep team object for optional population + if team_data.get('id'): + from models.team import Team + player_data['team'] = Team.from_api_data(team_data) + + # Handle nested sbaplayer_id structure (API sometimes returns object instead of int) + if 'sbaplayer_id' in player_data and isinstance(player_data['sbaplayer_id'], dict): + sba_data = player_data['sbaplayer_id'] + # Extract ID from nested object, or set to None if no valid ID + player_data['sbaplayer_id'] = sba_data.get('id') if sba_data.get('id') else None + + return super().from_api_data(player_data) + + @property + def is_pitcher(self) -> bool: + """Check if player is a pitcher.""" + return self.pos_1 in ['SP', 'RP', 'P'] + + def __str__(self): + return f"{self.name} ({self.primary_position})" \ No newline at end of file diff --git a/models/team.py b/models/team.py new file mode 100644 index 0000000..24f7379 --- /dev/null +++ b/models/team.py @@ -0,0 +1,34 @@ +""" +Team model for SBA teams + +Represents a team in the league with all associated metadata. +""" +from typing import Optional +from pydantic import Field + +from models.base import SBABaseModel + + +class Team(SBABaseModel): + """Team model representing an SBA team.""" + + abbrev: str = Field(..., description="Team abbreviation (e.g., 'NYY')") + sname: str = Field(..., description="Short team name") + lname: str = Field(..., description="Long team name") + season: int = Field(..., description="Season number") + + # Manager information + gmid: Optional[int] = Field(None, description="Primary general manager ID") + gmid2: Optional[int] = Field(None, description="Secondary general manager ID") + manager1_id: Optional[int] = Field(None, description="Primary manager ID") + manager2_id: Optional[int] = Field(None, description="Secondary manager ID") + + # Team metadata + division_id: Optional[int] = Field(None, description="Division ID") + stadium: Optional[str] = Field(None, description="Home stadium name") + thumbnail: Optional[str] = Field(None, description="Team thumbnail URL") + color: Optional[str] = Field(None, description="Primary team color") + dice_color: Optional[str] = Field(None, description="Dice rolling color") + + def __str__(self): + return f"{self.abbrev} - {self.lname}" \ No newline at end of file diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..c2c916e --- /dev/null +++ b/requirements.txt @@ -0,0 +1,19 @@ +# Core Framework +discord.py>=2.3.0 +pydantic>=2.0.0 +pydantic-settings>=2.0.0 +aiohttp>=3.8.0 + +# Utilities +python-dotenv>=1.0.0 + +# Development & Testing +pytest>=7.0.0 +pytest-asyncio>=0.21.0 +pytest-mock>=3.10.0 +aioresponses>=0.7.4 +black>=23.0.0 +ruff>=0.1.0 + +# Optional Dependencies +# pygsheets>=4.0.0 # For Google Sheets integration if needed \ No newline at end of file diff --git a/services/__init__.py b/services/__init__.py new file mode 100644 index 0000000..c96b4e4 --- /dev/null +++ b/services/__init__.py @@ -0,0 +1,16 @@ +""" +Business logic services for Discord Bot v2.0 + +Service layer providing clean interfaces to data operations. +""" + +from .team_service import TeamService, team_service +from .player_service import PlayerService, player_service + +# Wire services together for dependency injection +player_service._team_service = team_service + +__all__ = [ + 'TeamService', 'team_service', + 'PlayerService', 'player_service' +] \ No newline at end of file diff --git a/services/base_service.py b/services/base_service.py new file mode 100644 index 0000000..21da8ec --- /dev/null +++ b/services/base_service.py @@ -0,0 +1,352 @@ +""" +Base service class for Discord Bot v2.0 + +Provides common CRUD operations and error handling for all data services. +""" +import logging +from typing import Optional, Type, TypeVar, Generic, Dict, Any, List, Tuple + +from api.client import get_global_client, APIClient +from models.base import SBABaseModel +from exceptions import APIException + +logger = logging.getLogger(f'{__name__}.BaseService') + +T = TypeVar('T', bound=SBABaseModel) + + +class BaseService(Generic[T]): + """ + Base service class providing common CRUD operations for SBA models. + + Features: + - Generic type support for any SBABaseModel subclass + - Automatic model validation and conversion + - Standardized error handling + - API response format handling (count + list format) + - Connection management via global client + """ + + def __init__(self, + model_class: Type[T], + endpoint: str, + client: Optional[APIClient] = None): + """ + Initialize base service. + + Args: + model_class: Pydantic model class for this service + endpoint: API endpoint path (e.g., 'players', 'teams') + client: Optional API client override (uses global client by default) + """ + self.model_class = model_class + self.endpoint = endpoint + self._client = client + self._cached_client: Optional[APIClient] = None + + logger.debug(f"Initialized {self.__class__.__name__} for {model_class.__name__} at endpoint '{endpoint}'") + + async def get_client(self) -> APIClient: + """ + Get API client instance with caching to reduce async overhead. + + Returns: + APIClient instance (cached after first access) + """ + if self._client: + return self._client + + # Cache the global client to avoid repeated async calls + if self._cached_client is None: + self._cached_client = await get_global_client() + + return self._cached_client + + async def get_by_id(self, object_id: int) -> Optional[T]: + """ + Get single object by ID. + + Args: + object_id: Unique identifier for the object + + Returns: + Model instance or None if not found + + Raises: + APIException: For API errors + ValueError: For invalid data + """ + try: + client = await self.get_client() + data = await client.get(self.endpoint, object_id=object_id) + + if not data: + logger.debug(f"{self.model_class.__name__} {object_id} not found") + return None + + model = self.model_class.from_api_data(data) + logger.debug(f"Retrieved {self.model_class.__name__} {object_id}: {model}") + return model + + except APIException: + logger.error(f"API error retrieving {self.model_class.__name__} {object_id}") + raise + except Exception as e: + logger.error(f"Error retrieving {self.model_class.__name__} {object_id}: {e}") + raise APIException(f"Failed to retrieve {self.model_class.__name__}: {e}") + + async def get_all(self, params: Optional[List[tuple]] = None) -> Tuple[List[T], int]: + """ + Get all objects with optional query parameters. + + Args: + params: Query parameters as list of (key, value) tuples + + Returns: + Tuple of (list of model instances, total count) + + Raises: + APIException: For API errors + """ + try: + client = await self.get_client() + data = await client.get(self.endpoint, params=params) + + if not data: + logger.debug(f"No {self.model_class.__name__} objects found") + return [], 0 + + # Handle API response format: {'count': int, '': [...]} + items, count = self._extract_items_and_count_from_response(data) + + models = [self.model_class.from_api_data(item) for item in items] + logger.debug(f"Retrieved {len(models)} of {count} {self.model_class.__name__} objects") + return models, count + + except APIException: + logger.error(f"API error retrieving {self.model_class.__name__} list") + raise + except Exception as e: + logger.error(f"Error retrieving {self.model_class.__name__} list: {e}") + raise APIException(f"Failed to retrieve {self.model_class.__name__} list: {e}") + + async def get_all_items(self, params: Optional[List[tuple]] = None) -> List[T]: + """ + Get all objects (convenience method that only returns the list). + + Args: + params: Query parameters as list of (key, value) tuples + + Returns: + List of model instances + """ + items, _ = await self.get_all(params=params) + return items + + async def create(self, model_data: Dict[str, Any]) -> Optional[T]: + """ + Create new object from data dictionary. + + Args: + model_data: Dictionary of model fields + + Returns: + Created model instance or None + + Raises: + APIException: For API errors + ValueError: For invalid data + """ + try: + client = await self.get_client() + response = await client.post(self.endpoint, model_data) + + if not response: + logger.warning(f"No response from {self.model_class.__name__} creation") + return None + + model = self.model_class.from_api_data(response) + logger.debug(f"Created {self.model_class.__name__}: {model}") + return model + + except APIException: + logger.error(f"API error creating {self.model_class.__name__}") + raise + except Exception as e: + logger.error(f"Error creating {self.model_class.__name__}: {e}") + raise APIException(f"Failed to create {self.model_class.__name__}: {e}") + + async def create_from_model(self, model: T) -> Optional[T]: + """ + Create new object from model instance. + + Args: + model: Model instance to create + + Returns: + Created model instance or None + """ + return await self.create(model.to_dict(exclude_none=True)) + + async def update(self, object_id: int, model_data: Dict[str, Any]) -> Optional[T]: + """ + Update existing object. + + Args: + object_id: ID of object to update + model_data: Dictionary of fields to update + + Returns: + Updated model instance or None if not found + + Raises: + APIException: For API errors + """ + try: + client = await self.get_client() + response = await client.put(self.endpoint, model_data, object_id=object_id) + + if not response: + logger.debug(f"{self.model_class.__name__} {object_id} not found for update") + return None + + model = self.model_class.from_api_data(response) + logger.debug(f"Updated {self.model_class.__name__} {object_id}: {model}") + return model + + except APIException: + logger.error(f"API error updating {self.model_class.__name__} {object_id}") + raise + except Exception as e: + logger.error(f"Error updating {self.model_class.__name__} {object_id}: {e}") + raise APIException(f"Failed to update {self.model_class.__name__}: {e}") + + async def update_from_model(self, model: T) -> Optional[T]: + """ + Update object from model instance. + + Args: + model: Model instance to update (must have ID) + + Returns: + Updated model instance or None + + Raises: + ValueError: If model has no ID + """ + if not model.id: + raise ValueError(f"Cannot update {self.model_class.__name__} without ID") + + return await self.update(model.id, model.to_dict(exclude_none=True)) + + async def delete(self, object_id: int) -> bool: + """ + Delete object by ID. + + Args: + object_id: ID of object to delete + + Returns: + True if deleted, False if not found + + Raises: + APIException: For API errors + """ + try: + client = await self.get_client() + success = await client.delete(self.endpoint, object_id=object_id) + + if success: + logger.debug(f"Deleted {self.model_class.__name__} {object_id}") + else: + logger.debug(f"{self.model_class.__name__} {object_id} not found for deletion") + + return success + + except APIException: + logger.error(f"API error deleting {self.model_class.__name__} {object_id}") + raise + except Exception as e: + logger.error(f"Error deleting {self.model_class.__name__} {object_id}: {e}") + raise APIException(f"Failed to delete {self.model_class.__name__}: {e}") + + async def search(self, query: str, **kwargs) -> List[T]: + """ + Search for objects by query string. + + Args: + query: Search query + **kwargs: Additional search parameters + + Returns: + List of matching model instances + """ + params = [('q', query)] + params.extend(kwargs.items()) + + return await self.get_all_items(params=params) + + async def get_by_field(self, field: str, value: Any) -> List[T]: + """ + Get objects by specific field value. + + Args: + field: Field name to search + value: Field value to match + + Returns: + List of matching model instances + """ + params = [(field, str(value))] + return await self.get_all_items(params=params) + + async def count(self, params: Optional[List[tuple]] = None) -> int: + """ + Get count of objects matching parameters. + + Args: + params: Query parameters + + Returns: + Number of matching objects (from API count field) + """ + _, count = await self.get_all(params=params) + return count + + def _extract_items_and_count_from_response(self, data: Any) -> Tuple[List[Dict[str, Any]], int]: + """ + Extract items list and count from API response with optimized parsing. + + Expected format: {'count': int, '': [...]} + Single object format: {'id': 1, 'name': '...'} + + Args: + data: API response data + + Returns: + Tuple of (items list, total count) + """ + if isinstance(data, list): + return data, len(data) + + if not isinstance(data, dict): + logger.warning(f"Unexpected response format for {self.model_class.__name__}: {type(data)}") + return [], 0 + + # Single pass through the response dict - get count first + count = data.get('count', 0) + + # Priority order for finding items list (most common first) + field_candidates = [self.endpoint, 'items', 'data', 'results'] + for field_name in field_candidates: + if field_name in data and isinstance(data[field_name], list): + return data[field_name], count or len(data[field_name]) + + # Single object response (check for common identifying fields) + if any(key in data for key in ['id', 'name', 'abbrev']): + return [data], 1 + + return [], count + + def __repr__(self) -> str: + return f"{self.__class__.__name__}(model={self.model_class.__name__}, endpoint='{self.endpoint}')" \ No newline at end of file diff --git a/services/player_service.py b/services/player_service.py new file mode 100644 index 0000000..81f59df --- /dev/null +++ b/services/player_service.py @@ -0,0 +1,284 @@ +""" +Player service for Discord Bot v2.0 + +Handles player-related operations with team population and search functionality. +""" +import logging +from typing import Optional, List, TYPE_CHECKING + +from services.base_service import BaseService +from models.player import Player +from models.team import Team +from constants import FREE_AGENT_TEAM_ID, SBA_CURRENT_SEASON +from exceptions import APIException + +if TYPE_CHECKING: + from services.team_service import TeamService + +logger = logging.getLogger(f'{__name__}.PlayerService') + + +class PlayerService(BaseService[Player]): + """ + Service for player-related operations. + + Features: + - Player retrieval with team population + - Team roster queries + - Name-based search with exact matching + - Season-specific filtering + - Free agent handling via constants + """ + + def __init__(self, team_service: Optional['TeamService'] = None): + """Initialize player service.""" + super().__init__(Player, 'players') + self._team_service = team_service + logger.debug("PlayerService initialized") + + async def get_player(self, player_id: int) -> Optional[Player]: + """ + Get player by ID with error handling. + + Args: + player_id: Unique player identifier + + Returns: + Player instance or None if not found + """ + try: + return await self.get_by_id(player_id) + except APIException: + logger.error(f"Failed to get player {player_id}") + return None + except Exception as e: + logger.error(f"Unexpected error getting player {player_id}: {e}") + return None + + async def get_player_with_team(self, player_id: int) -> Optional[Player]: + """ + Get player with team information populated. + + Args: + player_id: Unique player identifier + + Returns: + Player instance with team data or None if not found + """ + try: + player = await self.get_player(player_id) + if not player: + return None + + # Populate team information if team_id exists and TeamService is available + if player.team_id and self._team_service: + team = await self._team_service.get_team(player.team_id) + if team: + player.team = team + logger.debug(f"Populated team data via TeamService for player {player_id}: {team.sname}") + # Fallback to direct API call + elif player.team_id: + client = await self.get_client() + team_data = await client.get('teams', object_id=player.team_id) + if team_data: + player.team = Team.from_api_data(team_data) + logger.debug(f"Populated team data via API for player {player_id}: {player.team.sname}") + + return player + + except Exception as e: + logger.error(f"Error getting player with team {player_id}: {e}") + return None + + async def get_players_by_team(self, team_id: int, season: int) -> List[Player]: + """ + Get all players for a specific team. + + Args: + team_id: Team identifier + season: Season number (required) + + Returns: + List of players on the team + """ + try: + params = [ + ('season', str(season)), + ('team_id', str(team_id)) + ] + + players = await self.get_all_items(params=params) + logger.debug(f"Retrieved {len(players)} players for team {team_id} in season {season}") + return players + + except Exception as e: + logger.error(f"Failed to get players for team {team_id}: {e}") + return [] + + async def get_players_by_name(self, name: str, season: int) -> List[Player]: + """ + Search for players by name (partial match). + + Args: + name: Player name or partial name + season: Season number (required) + + Returns: + List of matching players + """ + try: + params = [ + ('season', str(season)), + ('name', name) + ] + + players = await self.get_all_items(params=params) + logger.debug(f"Found {len(players)} players matching '{name}' in season {season}") + return players + + except Exception as e: + logger.error(f"Failed to search players by name '{name}': {e}") + return [] + + async def get_player_by_name_exact(self, name: str, season: int) -> Optional[Player]: + """ + Get player by exact name match (case-insensitive). + + Args: + name: Exact player name + season: Season number (required) + + Returns: + Player instance or None if not found + """ + try: + players = await self.get_players_by_name(name, season) + + # Look for exact case-insensitive match + name_lower = name.lower() + for player in players: + if player.name.lower() == name_lower: + logger.debug(f"Found exact match for '{name}': {player.name}") + return player + + logger.debug(f"No exact match found for '{name}'") + return None + + except Exception as e: + logger.error(f"Error finding exact player match for '{name}': {e}") + return None + + async def search_players_fuzzy(self, query: str, limit: int = 10) -> List[Player]: + """ + Fuzzy search for players by name with limit. + + Args: + query: Search query + limit: Maximum results to return + + Returns: + List of matching players (up to limit) + """ + try: + players = await self.search(query) + + # Sort by relevance (exact matches first, then partial) + query_lower = query.lower() + exact_matches = [] + partial_matches = [] + + for player in players: + name_lower = player.name.lower() + if name_lower == query_lower: + exact_matches.append(player) + elif query_lower in name_lower: + partial_matches.append(player) + + # Combine and limit results + results = exact_matches + partial_matches + limited_results = results[:limit] + + logger.debug(f"Fuzzy search '{query}' returned {len(limited_results)} of {len(results)} matches") + return limited_results + + except Exception as e: + logger.error(f"Error in fuzzy search for '{query}': {e}") + return [] + + + async def get_free_agents(self, season: int) -> List[Player]: + """ + Get all free agent players. + + Args: + season: Season number (required) + + Returns: + List of free agent players + """ + try: + params = [('team_id', FREE_AGENT_TEAM_ID), ('season', str(season))] + + players = await self.get_all_items(params=params) + logger.debug(f"Retrieved {len(players)} free agents") + return players + + except Exception as e: + logger.error(f"Failed to get free agents: {e}") + return [] + + async def is_free_agent(self, player: Player) -> bool: + """ + Check if a player is a free agent. + + Args: + player: Player instance to check + + Returns: + True if player is a free agent + """ + return player.team_id == FREE_AGENT_TEAM_ID + + async def get_players_by_position(self, position: str, season: int) -> List[Player]: + """ + Get players by position. + + Args: + position: Player position (e.g., 'C', '1B', 'OF') + season: Season number (required) + + Returns: + List of players at the position + """ + try: + params = [('position', position), ('season', str(season))] + + players = await self.get_all_items(params=params) + logger.debug(f"Retrieved {len(players)} players at position {position}") + return players + + except Exception as e: + logger.error(f"Failed to get players by position {position}: {e}") + return [] + + + async def update_player(self, player_id: int, updates: dict) -> Optional[Player]: + """ + Update player information. + + Args: + player_id: Player ID to update + updates: Dictionary of fields to update + + Returns: + Updated player instance or None + """ + try: + return await self.update(player_id, updates) + except Exception as e: + logger.error(f"Failed to update player {player_id}: {e}") + return None + + +# Global service instance - will be properly initialized in __init__.py +player_service = PlayerService() \ No newline at end of file diff --git a/services/team_service.py b/services/team_service.py new file mode 100644 index 0000000..f8bd20a --- /dev/null +++ b/services/team_service.py @@ -0,0 +1,266 @@ +""" +Team service for Discord Bot v2.0 + +Handles team-related operations with roster management and league queries. +""" +import logging +from typing import Optional, List, Dict, Any + +from services.base_service import BaseService +from models.team import Team +from constants import SBA_CURRENT_SEASON +from exceptions import APIException + +logger = logging.getLogger(f'{__name__}.TeamService') + + +class TeamService(BaseService[Team]): + """ + Service for team-related operations. + + Features: + - Team retrieval by ID, abbreviation, and season + - Manager-based team queries + - Division and league organization + - Roster management with position counts and player lists + - Season-specific team data + - Standings integration + """ + + def __init__(self): + """Initialize team service.""" + super().__init__(Team, 'teams') + logger.debug("TeamService initialized") + + async def get_team(self, team_id: int) -> Optional[Team]: + """ + Get team by ID with error handling. + + Args: + team_id: Unique team identifier + + Returns: + Team instance or None if not found + """ + try: + return await self.get_by_id(team_id) + except APIException: + logger.error(f"Failed to get team {team_id}") + return None + except Exception as e: + logger.error(f"Unexpected error getting team {team_id}: {e}") + return None + + async def get_team_by_abbrev(self, abbrev: str, season: Optional[int] = None) -> Optional[Team]: + """ + Get team by abbreviation for a specific season. + + Args: + abbrev: Team abbreviation (e.g., 'NYY', 'BOS') + season: Season number (defaults to current season) + + Returns: + Team instance or None if not found + """ + try: + season = season or SBA_CURRENT_SEASON + params = [ + ('abbrev', abbrev.upper()), + ('season', str(season)) + ] + + teams = await self.get_all_items(params=params) + + if teams: + team = teams[0] # Should be unique per season + logger.debug(f"Found team {abbrev} for season {season}: {team.lname}") + return team + + logger.debug(f"No team found for abbreviation '{abbrev}' in season {season}") + return None + + except Exception as e: + logger.error(f"Error getting team by abbreviation '{abbrev}': {e}") + return None + + async def get_teams_by_season(self, season: int) -> List[Team]: + """ + Get all teams for a specific season. + + Args: + season: Season number + + Returns: + List of teams in the season + """ + try: + params = [('season', str(season))] + + teams = await self.get_all_items(params=params) + logger.debug(f"Retrieved {len(teams)} teams for season {season}") + return teams + + except Exception as e: + logger.error(f"Failed to get teams for season {season}: {e}") + return [] + + async def get_teams_by_manager(self, manager_id: int, season: Optional[int] = None) -> List[Team]: + """ + Get teams managed by a specific manager. + + Uses 'manager_id' query parameter which supports multiple manager matching. + + Args: + manager_id: Manager identifier + season: Season number (optional) + + Returns: + List of teams managed by the manager + """ + try: + params = [('manager_id', str(manager_id))] + + if season: + params.append(('season', str(season))) + + teams = await self.get_all_items(params=params) + logger.debug(f"Found {len(teams)} teams for manager {manager_id}") + return teams + + except Exception as e: + logger.error(f"Failed to get teams for manager {manager_id}: {e}") + return [] + + async def get_teams_by_division(self, division_id: int, season: int) -> List[Team]: + """ + Get teams in a specific division for a season. + + Args: + division_id: Division identifier + season: Season number + + Returns: + List of teams in the division + """ + try: + params = [ + ('division_id', str(division_id)), + ('season', str(season)) + ] + + teams = await self.get_all_items(params=params) + logger.debug(f"Retrieved {len(teams)} teams for division {division_id} in season {season}") + return teams + + except Exception as e: + logger.error(f"Failed to get teams for division {division_id}: {e}") + return [] + + async def get_team_roster(self, team_id: int, roster_type: str = 'current') -> Optional[Dict[str, Any]]: + """ + Get the roster for a team with position counts and player lists. + + Returns roster data with active, shortil (minor league), and longil (injured list) + rosters. Each roster contains position counts and players sorted by descending WARa. + + Args: + team_id: Team identifier + roster_type: 'current' or 'next' roster + + Returns: + Dictionary with roster structure: + { + 'active': { + 'C': 0, '1B': 0, '2B': 0, '3B': 0, 'SS': 0, 'LF': 0, 'CF': 0, 'RF': 0, 'DH': 0, + 'SP': 0, 'RP': 0, 'CP': 0, 'WARa': 0, + 'players': [] + }, + 'shortil': { ... }, + 'longil': { ... } + } + """ + try: + client = await self.get_client() + data = await client.get(f'teams/{team_id}/roster/{roster_type}') + + if data: + logger.debug(f"Retrieved {roster_type} roster for team {team_id}") + return data + + logger.debug(f"No roster data found for team {team_id}") + return None + + except Exception as e: + logger.error(f"Failed to get roster for team {team_id}: {e}") + return None + + async def update_team(self, team_id: int, updates: dict) -> Optional[Team]: + """ + Update team information. + + Args: + team_id: Team ID to update + updates: Dictionary of fields to update + + Returns: + Updated team instance or None + """ + try: + return await self.update(team_id, updates) + except Exception as e: + logger.error(f"Failed to update team {team_id}: {e}") + return None + + async def get_team_standings_position(self, team_id: int, season: int) -> Optional[dict]: + """ + Get team's standings information. + + Calls /standings/team/{team_id} endpoint which returns a Standings object. + + Args: + team_id: Team identifier + season: Season number + + Returns: + Standings object data for the team + """ + try: + client = await self.get_client() + data = await client.get(f'standings/team/{team_id}', params=[('season', str(season))]) + + if data: + logger.debug(f"Retrieved standings for team {team_id}") + return data + + return None + + except Exception as e: + logger.error(f"Failed to get standings for team {team_id}: {e}") + return None + + async def is_valid_team_abbrev(self, abbrev: str, season: Optional[int] = None) -> bool: + """ + Check if a team abbreviation is valid for a season. + + Args: + abbrev: Team abbreviation to validate + season: Season number (defaults to current) + + Returns: + True if the abbreviation is valid + """ + team = await self.get_team_by_abbrev(abbrev, season) + return team is not None + + async def get_current_season_teams(self) -> List[Team]: + """ + Get all teams for the current season. + + Returns: + List of teams in current season + """ + return await self.get_teams_by_season(SBA_CURRENT_SEASON) + + +# Global service instance +team_service = TeamService() \ No newline at end of file diff --git a/test_real_data.py b/test_real_data.py new file mode 100644 index 0000000..de7772c --- /dev/null +++ b/test_real_data.py @@ -0,0 +1,307 @@ +#!/usr/bin/env python3 +""" +Real Data Testing Script + +Safely test services with real cloud database data (READ-ONLY operations only). +Uses structured logging to demonstrate contextual information with real data. +""" +import asyncio +import os +import sys +from pathlib import Path + +# Load testing environment +os.environ.setdefault('BOT_TOKEN', 'dummy_token') +os.environ.setdefault('GUILD_ID', '123456789') +os.environ.setdefault('API_TOKEN', 'Tp3aO3jhYve5NJF1IqOmJTmk') +os.environ.setdefault('DB_URL', 'https://sbadev.manticorum.com/api') +os.environ.setdefault('LOG_LEVEL', 'DEBUG') +os.environ.setdefault('ENVIRONMENT', 'testing') +os.environ.setdefault('TESTING', 'true') + +from services.player_service import player_service +from utils.logging import get_contextual_logger, set_discord_context +from api.client import cleanup_global_client +from constants import SBA_CURRENT_SEASON + +logger = get_contextual_logger('test_real_data') + + +class MockInteraction: + """Mock Discord interaction for testing context.""" + def __init__(self, user_id="999888777", guild_id="111222333"): + self.user = MockUser(user_id) + self.guild = MockGuild(guild_id) + self.channel = MockChannel() + +class MockUser: + def __init__(self, user_id): + self.id = int(user_id) + +class MockGuild: + def __init__(self, guild_id): + self.id = int(guild_id) + self.name = "SBA Test Guild" + +class MockChannel: + def __init__(self): + self.id = 444555666 + + +async def test_player_search(): + """Test player search with real data.""" + print("🔍 Testing Player Search...") + + # Set up logging context + mock_interaction = MockInteraction() + set_discord_context( + interaction=mock_interaction, + command="/player", + test_type="player_search" + ) + + trace_id = logger.start_operation("real_data_test_player_search") + + try: + # Test 1: Search for a common name (should find multiple) + logger.info("Testing search for common player name") + players = await player_service.get_players_by_name("Smith", SBA_CURRENT_SEASON) + logger.info("Common name search completed", + search_term="Smith", + results_found=len(players)) + + if players: + print(f" ✅ Found {len(players)} players with 'Smith' in name") + for i, player in enumerate(players[:3]): # Show first 3 + print(f" {i+1}. {player.name} ({player.primary_position}) - Season {player.season}") + else: + print(" ⚠️ No players found with 'Smith' - unusual for baseball!") + + # Test 2: Search for specific player (exact match) + logger.info("Testing search for specific player") + players = await player_service.get_players_by_name("Mike Trout", SBA_CURRENT_SEASON) + logger.info("Specific player search completed", + search_term="Mike Trout", + results_found=len(players)) + + if players: + player = players[0] + print(f" ✅ Found Mike Trout: {player.name} (WARA: {player.wara})") + + # Get with team info + logger.debug("Testing get_player_with_team", player_id=player.id) + player_with_team = await player_service.get_player_with_team(player.id) + if player_with_team and hasattr(player_with_team, 'team') and player_with_team.team: + print(f" Team: {player_with_team.team.abbrev} - {player_with_team.team.sname}") + logger.info("Player with team retrieved successfully", + player_name=player_with_team.name, + team_abbrev=player_with_team.team.abbrev) + else: + print(" Team: Not found or no team association") + logger.warning("Player team information not available") + else: + print(" ❌ Mike Trout not found - checking if database has current players") + + # Test 3: Get player by ID (if we found any players) + if players: + test_player = players[0] + logger.info("Testing get_by_id", player_id=test_player.id) + player_by_id = await player_service.get_by_id(test_player.id) + + if player_by_id: + print(f" ✅ Retrieved by ID: {player_by_id.name} (ID: {player_by_id.id})") + logger.info("Get by ID successful", + player_id=player_by_id.id, + player_name=player_by_id.name) + else: + print(f" ❌ Failed to retrieve player ID {test_player.id}") + logger.error("Get by ID failed", player_id=test_player.id) + + return True + + except Exception as e: + logger.error("Player search test failed", error=e) + print(f" ❌ Error: {e}") + return False + + +async def test_player_service_methods(): + """Test various player service methods.""" + print("🔧 Testing Player Service Methods...") + + set_discord_context( + command="/test-service-methods", + test_type="service_methods" + ) + + trace_id = logger.start_operation("test_service_methods") + + try: + # Test get_all with limit (need to include season) + from constants import SBA_CURRENT_SEASON + logger.info("Testing get_all with limit") + players, total_count = await player_service.get_all(params=[ + ('season', str(SBA_CURRENT_SEASON)), + ('limit', '10') + ]) + + print(f" ✅ Retrieved {len(players)} of {total_count} total players") + logger.info("Get all players completed", + retrieved_count=len(players), + total_count=total_count, + limit=10, + season=SBA_CURRENT_SEASON) + + if players: + print(" Sample players:") + for i, player in enumerate(players[:3]): + print(f" {i+1}. {player.name} ({player.primary_position}) - WARA: {player.wara}") + + # Test search by position (if we have players) + if players: + test_position = players[0].primary_position + logger.info("Testing position search", position=test_position) + position_players = await player_service.get_players_by_position(test_position, SBA_CURRENT_SEASON) + + print(f" ✅ Found {len(position_players)} players at position {test_position}") + logger.info("Position search completed", + position=test_position, + players_found=len(position_players)) + + return True + + except Exception as e: + logger.error("Service methods test failed", error=e) + print(f" ❌ Error: {e}") + return False + + +async def test_api_connectivity(): + """Test basic API connectivity.""" + print("🌐 Testing API Connectivity...") + + set_discord_context( + command="/test-api", + test_type="connectivity" + ) + + trace_id = logger.start_operation("test_api_connectivity") + + try: + from api.client import get_global_client + + logger.info("Testing basic API connection") + client = await get_global_client() + + # Test current endpoint (usually lightweight) + logger.debug("Making API call to current endpoint") + current_data = await client.get('current') + + if current_data: + print(" ✅ API connection successful") + logger.info("API connectivity test passed", + endpoint='current', + response_received=True) + + # Show some basic info about the league + if isinstance(current_data, dict): + season = current_data.get('season', 'Unknown') + week = current_data.get('week', 'Unknown') + print(f" Current season: {season}, Week: {week}") + logger.info("Current league info retrieved", + season=season, + week=week) + else: + print(" ⚠️ API connected but returned no data") + logger.warning("API connection successful but no data returned") + + return True + + except Exception as e: + logger.error("API connectivity test failed", error=e) + print(f" ❌ API Error: {e}") + return False + + +async def main(): + """Run all real data tests.""" + print("🧪 Testing Discord Bot v2.0 with Real Cloud Database") + print("=" * 60) + print(f"🌐 API URL: https://sbadev.manticorum.com/api") + print(f"📝 Logging: Check logs/discord_bot_v2.json for structured output") + print() + + # Initialize logging + import logging + from logging.handlers import RotatingFileHandler + from utils.logging import JSONFormatter + + os.makedirs('logs', exist_ok=True) + + # Set up logging + root_logger = logging.getLogger() + root_logger.setLevel(logging.DEBUG) + + # Console handler + console_handler = logging.StreamHandler() + console_formatter = logging.Formatter( + '%(asctime)s - %(name)s - %(levelname)s - %(message)s' + ) + console_handler.setFormatter(console_formatter) + + # JSON file handler for structured logging + json_handler = RotatingFileHandler('logs/discord_bot_v2.json', maxBytes=2*1024*1024, backupCount=3) + json_handler.setFormatter(JSONFormatter()) + + root_logger.addHandler(console_handler) + root_logger.addHandler(json_handler) + + # Run tests + tests = [ + ("API Connectivity", test_api_connectivity), + ("Player Search", test_player_search), + ("Player Service Methods", test_player_service_methods), + ] + + passed = 0 + failed = 0 + + for test_name, test_func in tests: + try: + print(f"\n📋 {test_name}") + print("-" * 40) + success = await test_func() + if success: + passed += 1 + print(f"✅ {test_name} PASSED") + else: + failed += 1 + print(f"❌ {test_name} FAILED") + except Exception as e: + failed += 1 + print(f"❌ {test_name} CRASHED: {e}") + + print("\n" + "=" * 60) + print(f"📊 Test Results: {passed} passed, {failed} failed") + + if failed == 0: + print("🎉 All tests passed! Services work with real data!") + else: + print("⚠️ Some tests failed. Check logs for details.") + + print(f"\n📁 Structured logs available at: logs/discord_bot_v2.json") + print(" Use jq to query: jq '.context.test_type' logs/discord_bot_v2.json") + + # Cleanup + await cleanup_global_client() + + return failed == 0 + + +if __name__ == "__main__": + try: + success = asyncio.run(main()) + sys.exit(0 if success else 1) + except KeyboardInterrupt: + print("\n🛑 Testing interrupted by user") + sys.exit(1) \ No newline at end of file diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..7113328 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,293 @@ +# Testing Guide for Discord Bot v2.0 + +This document provides guidance on testing strategies, patterns, and lessons learned during the development of the Discord Bot v2.0 test suite. + +## Test Structure Overview + +``` +tests/ +├── README.md # This guide +├── __init__.py # Test package +├── fixtures/ # Test data fixtures +├── test_config.py # Configuration tests +├── test_constants.py # Constants tests +├── test_exceptions.py # Exception handling tests +├── test_models.py # Pydantic model tests +├── test_services.py # Service layer tests (25 tests) +└── test_api_client_with_aioresponses.py # API client HTTP tests (19 tests) +``` + +**Total Coverage**: 44 comprehensive tests covering all core functionality. + +## Key Testing Patterns + +### 1. HTTP Testing with aioresponses + +**✅ Recommended Approach:** +```python +from aioresponses import aioresponses + +@pytest.mark.asyncio +async def test_api_request(api_client): + with aioresponses() as m: + m.get( + "https://api.example.com/v3/players/1", + payload={"id": 1, "name": "Test Player"}, + status=200 + ) + + result = await api_client.get("players", object_id=1) + assert result["name"] == "Test Player" +``` + +**❌ Avoid Complex AsyncMock:** +We initially tried mocking aiohttp's async context managers manually with AsyncMock, which led to complex, brittle tests that failed due to coroutine protocol issues. + +### 2. Service Layer Testing + +**✅ Complete Model Data:** +Always provide complete model data that satisfies Pydantic validation: + +```python +def create_player_data(self, player_id: int, name: str, **kwargs): + """Create complete player data for testing.""" + base_data = { + 'id': player_id, + 'name': name, + 'wara': 2.5, # Required field + 'season': 12, # Required field + 'team_id': team_id, # Required field + 'image': f'https://example.com/player{player_id}.jpg', # Required field + 'pos_1': position, # Required field + } + base_data.update(kwargs) + return base_data +``` + +**❌ Partial Model Data:** +Providing incomplete data leads to Pydantic validation errors that are hard to debug. + +### 3. API Response Format Testing + +Our API returns responses in this format: +```json +{ + "count": 25, + "players": [...] +} +``` + +**✅ Test Both Formats:** +```python +# Test the count + list format +mock_data = { + "count": 2, + "players": [player1_data, player2_data] +} + +# Test single object format (for get_by_id) +mock_data = player1_data +``` + +## Lessons Learned + +### 1. aiohttp Testing Complexity + +**Problem**: Manually mocking aiohttp's async context managers is extremely complex and error-prone. + +**Solution**: Use `aioresponses` library specifically designed for this purpose. + +**Code Example**: +```bash +pip install aioresponses>=0.7.4 +``` + +```python +# Clean, readable, reliable +with aioresponses() as m: + m.get("https://api.example.com/endpoint", payload=expected_data) + result = await client.get("endpoint") +``` + +### 2. Pydantic Model Validation in Tests + +**Problem**: Our models have many required fields. Partial test data causes validation errors. + +**Solution**: Create helper functions that generate complete, valid model data. + +**Pattern**: +```python +def create_model_data(self, id: int, name: str, **overrides): + """Create complete model data with all required fields.""" + base_data = { + # All required fields with sensible defaults + 'id': id, + 'name': name, + 'required_field1': 'default_value', + 'required_field2': 42, + } + base_data.update(overrides) + return base_data +``` + +### 3. Async Context Manager Mocking + +**Problem**: This doesn't work reliably: +```python +# ❌ Brittle and complex +mock_session.get.return_value.__aenter__ = AsyncMock(return_value=mock_response) +mock_session.get.return_value.__aexit__ = AsyncMock(return_value=None) +``` + +**Solution**: Use specialized libraries or patch at higher levels: +```python +# ✅ Clean with aioresponses +with aioresponses() as m: + m.get("url", payload=data) + # Test the actual HTTP call +``` + +### 4. Service Layer Mocking Strategy + +**✅ Mock at the Client Level:** +```python +@pytest.fixture +def player_service_instance(self, mock_client): + service = PlayerService() + service._client = mock_client # Inject mock client + return service +``` + +This allows testing service logic while controlling API responses. + +### 5. Global Instance Testing + +**Pattern for Singleton Services:** +```python +def test_global_service_independence(): + service1 = PlayerService() + service2 = PlayerService() + + # Should be different instances + assert service1 is not service2 + # But same configuration + assert service1.endpoint == service2.endpoint +``` + +## Testing Anti-Patterns to Avoid + +### 1. ❌ Incomplete Test Data +```python +# This will fail Pydantic validation +mock_data = {'id': 1, 'name': 'Test'} # Missing required fields +``` + +### 2. ❌ Complex Manual Mocking +```python +# Avoid complex AsyncMock setups for HTTP clients +mock_response = AsyncMock() +mock_response.__aenter__ = AsyncMock(...) # Too complex +``` + +### 3. ❌ Testing Implementation Details +```python +# Don't test internal method calls +assert mock_client.get.call_count == 2 # Brittle +# Instead test behavior +assert len(result) == 2 # What matters to users +``` + +### 4. ❌ Mixing Test Concerns +```python +# Don't test multiple unrelated things in one test +def test_everything(): # Too broad + # Test HTTP client + # Test service logic + # Test model validation + # All in one test - hard to debug +``` + +## Best Practices Summary + +### ✅ Do: +1. **Use aioresponses** for HTTP client testing +2. **Create complete model data** with helper functions +3. **Test behavior, not implementation** details +4. **Mock at appropriate levels** (client level for services) +5. **Use realistic data** that matches actual API responses +6. **Test error scenarios** as thoroughly as happy paths +7. **Keep tests focused** on single responsibilities + +### ❌ Don't: +1. **Manually mock async context managers** - use specialized tools +2. **Use partial model data** - always provide complete valid data +3. **Test implementation details** - focus on behavior +4. **Mix multiple concerns** in single tests +5. **Ignore error paths** - test failure scenarios +6. **Skip integration scenarios** - test realistic workflows + +## Running Tests + +```bash +# Run all tests +pytest + +# Run specific test files +pytest tests/test_services.py +pytest tests/test_api_client_with_aioresponses.py + +# Run with coverage +pytest --cov=api --cov=services + +# Run with verbose output +pytest -v + +# Run specific test patterns +pytest -k "test_player" -v +``` + +## Adding New Tests + +### For New API Endpoints: +1. Add aioresponses-based tests in `test_api_client_with_aioresponses.py` +2. Follow existing patterns for success/error scenarios + +### For New Services: +1. Add service tests in `test_services.py` +2. Create helper functions for complete model data +3. Mock at the client level, not HTTP level + +### For New Models: +1. Add model tests in `test_models.py` +2. Test validation, serialization, and edge cases +3. Use `from_api_data()` pattern for realistic data + +## Dependencies + +Core testing dependencies in `requirements.txt`: +``` +pytest>=7.0.0 +pytest-asyncio>=0.21.0 +pytest-mock>=3.10.0 +aioresponses>=0.7.4 # Essential for HTTP testing +``` + +## Troubleshooting Common Issues + +### "coroutine object does not support async context manager" +- **Cause**: Manually mocking aiohttp async context managers +- **Solution**: Use aioresponses instead of manual mocking + +### "ValidationError: Field required" +- **Cause**: Incomplete test data for Pydantic models +- **Solution**: Use helper functions that provide all required fields + +### "AssertionError: Regex pattern did not match" +- **Cause**: Exception message doesn't match expected pattern +- **Solution**: Check actual error message and adjust test expectations + +### Tests hanging or timing out +- **Cause**: Unclosed aiohttp sessions or improper async handling +- **Solution**: Ensure proper session cleanup and use async context managers + +This guide should help maintain high-quality, reliable tests as the project grows! \ No newline at end of file diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..8a55a23 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,5 @@ +""" +Test suite for Discord Bot v2.0 + +Comprehensive tests for models, services, and commands. +""" \ No newline at end of file diff --git a/tests/test_api_client.py b/tests/test_api_client.py new file mode 100644 index 0000000..4ac2473 --- /dev/null +++ b/tests/test_api_client.py @@ -0,0 +1,475 @@ +""" +API client tests using aioresponses for clean HTTP mocking +""" +import pytest +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 exceptions import APIException + + +class TestAPIClientWithAioresponses: + """Test API client with aioresponses for HTTP mocking.""" + + @pytest.fixture + def mock_config(self): + """Mock configuration for testing.""" + config = MagicMock() + 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): + 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", + payload=expected_data, + 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 + ) + + 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 + ) + + 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 + ) + + 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", + status=500, + body="Internal Server Error" + ) + + 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", + payload=expected_data, + status=200 + ) + + 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", + payload=expected_response, + 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" + ) + + 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", + payload=expected_response, + 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 + ) + + 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 + ) + + 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 + ) + + 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 + ) + + 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.""" + config = MagicMock() + 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 aioresponses() as m: + m.get( + "https://api.example.com/v3/test", + payload={"success": True}, + 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): + # 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.""" + config = MagicMock() + 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 aioresponses() as m: + # Mock player data response + player_data = { + "id": 1, + "name": "Test Player", + "wara": 2.5, + "season": 12, + "team_id": 5, + "image": "https://example.com/player1.jpg", + "pos_1": "C" + } + m.get( + "https://api.example.com/v3/players/1", + payload=player_data, + status=200 + ) + + # Mock team data response + team_data = { + "id": 5, + "abbrev": "TST", + "sname": "Test Team", + "lname": "Test Team Full Name", + "season": 12 + } + m.get( + "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 aioresponses() as m: + # Mock API response with count format + api_response = { + "count": 25, + "players": [ + { + "id": 1, + "name": "Player 1", + "wara": 2.5, + "season": 12, + "team_id": 5, + "image": "https://example.com/player1.jpg", + "pos_1": "C" + }, + { + "id": 2, + "name": "Player 2", + "wara": 1.8, + "season": 12, + "team_id": 6, + "image": "https://example.com/player2.jpg", + "pos_1": "1B" + } + ] + } + + m.get( + "https://api.example.com/v3/players?team_id=5", + payload=api_response, + 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 aioresponses() as m: + # First request fails with 500 + m.get( + "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", + payload={"id": 2, "name": "Working Player"}, + 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 aioresponses() as m: + # Mock multiple endpoints + for i in range(1, 4): + m.get( + f"https://api.example.com/v3/players/{i}", + payload={"id": i, "name": f"Player {i}"}, + 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) + ] + + 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.""" + config = MagicMock() + 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): + 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://", "") + + @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): + client = APIClient() + + # Test with existing query string + 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" \ No newline at end of file diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..e9554af --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,241 @@ +""" +Tests for configuration management + +Ensures configuration loading, validation, and environment handling work correctly. +""" +import os +import pytest +from unittest.mock import patch + +from config import BotConfig +from exceptions import ConfigurationException + + +class TestBotConfig: + """Test configuration loading and validation.""" + + def test_config_loads_required_fields(self): + """Test that config loads all required fields from environment.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + config = BotConfig() + assert config.bot_token == 'test_bot_token' + assert config.guild_id == 123456789 + assert config.api_token == 'test_api_token' + assert config.db_url == 'https://api.example.com' + + def test_config_has_default_values(self): + """Test that config provides sensible defaults.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + config = BotConfig() + assert config.sba_season == 12 + assert config.pd_season == 9 + assert config.fa_lock_week == 14 + assert config.sba_color == "a6ce39" + assert config.log_level == "INFO" + assert config.environment == "development" + assert config.testing is False + + def test_config_overrides_defaults_from_env(self): + """Test that environment variables override default values.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'SBA_SEASON': '15', + 'LOG_LEVEL': 'DEBUG', + 'ENVIRONMENT': 'production', + 'TESTING': 'true' + }): + config = BotConfig() + assert config.sba_season == 15 + assert config.log_level == "DEBUG" + assert config.environment == "production" + assert config.testing is True + + def test_config_ignores_extra_env_vars(self): + """Test that extra environment variables are ignored.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'RANDOM_EXTRA_VAR': 'should_be_ignored', + 'ANOTHER_RANDOM_VAR': 'also_ignored' + }): + # Should not raise validation error + config = BotConfig() + assert config.bot_token == 'test_bot_token' + + # Extra vars should not be accessible + assert not hasattr(config, 'random_extra_var') + assert not hasattr(config, 'another_random_var') + + def test_config_converts_string_to_int(self): + """Test that guild_id is properly converted from string to int.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '987654321', # String input + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + config = BotConfig() + assert config.guild_id == 987654321 + assert isinstance(config.guild_id, int) + + def test_config_converts_string_to_bool(self): + """Test that boolean fields are properly converted.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'TESTING': 'false' + }): + config = BotConfig() + assert config.testing is False + assert isinstance(config.testing, bool) + + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'TESTING': '1' + }): + config = BotConfig() + assert config.testing is True + + def test_config_case_insensitive(self): + """Test that environment variables are case insensitive.""" + with patch.dict(os.environ, { + 'bot_token': 'test_bot_token', # lowercase + 'GUILD_ID': '123456789', # uppercase + 'Api_Token': 'test_api_token', # mixed case + 'db_url': 'https://api.example.com' + }): + config = BotConfig() + assert config.bot_token == 'test_bot_token' + assert config.api_token == 'test_api_token' + assert config.db_url == 'https://api.example.com' + + def test_is_development_property(self): + """Test the is_development property.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'ENVIRONMENT': 'development' + }): + config = BotConfig() + assert config.is_development is True + + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'ENVIRONMENT': 'production' + }): + config = BotConfig() + assert config.is_development is False + + def test_is_testing_property(self): + """Test the is_testing property.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'TESTING': 'true' + }): + config = BotConfig() + assert config.is_testing is True + + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com', + 'TESTING': 'false' + }): + config = BotConfig() + assert config.is_testing is False + + +class TestConfigValidation: + """Test configuration validation and error handling.""" + + def test_missing_required_field_raises_error(self): + """Test that missing required fields raise validation errors.""" + # Missing BOT_TOKEN + with patch.dict(os.environ, { + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }, clear=True): + with pytest.raises(Exception): # Pydantic ValidationError + BotConfig() + + # Missing GUILD_ID + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }, clear=True): + with pytest.raises(Exception): # Pydantic ValidationError + BotConfig() + + def test_invalid_guild_id_raises_error(self): + """Test that invalid guild_id values raise validation errors.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': 'not_a_number', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + with pytest.raises(Exception): # Pydantic ValidationError + BotConfig() + + def test_empty_required_field_is_allowed(self): + """Test that empty required fields are allowed (Pydantic default behavior).""" + with patch.dict(os.environ, { + 'BOT_TOKEN': '', # Empty string + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + # Should not raise - Pydantic allows empty strings by default + config = BotConfig() + assert config.bot_token == '' + + +@pytest.fixture +def valid_config(): + """Provide a valid configuration for testing.""" + with patch.dict(os.environ, { + 'BOT_TOKEN': 'test_bot_token', + 'GUILD_ID': '123456789', + 'API_TOKEN': 'test_api_token', + 'DB_URL': 'https://api.example.com' + }): + return BotConfig() + + +def test_config_fixture(valid_config): + """Test that the valid_config fixture works correctly.""" + assert valid_config.bot_token == 'test_bot_token' + assert valid_config.guild_id == 123456789 + assert valid_config.api_token == 'test_api_token' + assert valid_config.db_url == 'https://api.example.com' \ No newline at end of file diff --git a/tests/test_constants.py b/tests/test_constants.py new file mode 100644 index 0000000..7c4b483 --- /dev/null +++ b/tests/test_constants.py @@ -0,0 +1,155 @@ +""" +Tests for application constants + +Validates that constants have sensible values. +""" +import pytest + +from constants import ( + DISCORD_EMBED_LIMIT, + DISCORD_FIELD_VALUE_LIMIT, + DISCORD_EMBED_DESCRIPTION_LIMIT, + WEEKS_PER_SEASON, + GAMES_PER_WEEK, + MODERN_STATS_START_SEASON, + API_VERSION, + DEFAULT_TIMEOUT, + MAX_RETRIES, + PITCHER_POSITIONS, + POSITION_FIELDERS, + ALL_POSITIONS, + DEFAULT_PICK_MINUTES, + DRAFT_ROUNDS +) + + +class TestDiscordLimits: + """Test Discord API limits are reasonable.""" + + def test_discord_limits_are_positive(self): + """Test that all Discord limits are positive integers.""" + assert DISCORD_EMBED_LIMIT > 0 + assert DISCORD_FIELD_VALUE_LIMIT > 0 + assert DISCORD_EMBED_DESCRIPTION_LIMIT > 0 + + assert isinstance(DISCORD_EMBED_LIMIT, int) + assert isinstance(DISCORD_FIELD_VALUE_LIMIT, int) + assert isinstance(DISCORD_EMBED_DESCRIPTION_LIMIT, int) + + def test_discord_limits_hierarchy(self): + """Test that Discord limits have sensible relationships.""" + # Description should be larger than field values + assert DISCORD_EMBED_DESCRIPTION_LIMIT > DISCORD_FIELD_VALUE_LIMIT + + # Total embed limit should be larger than description limit + assert DISCORD_EMBED_LIMIT > DISCORD_EMBED_DESCRIPTION_LIMIT + + +class TestLeagueConstants: + """Test league-specific constants.""" + + def test_league_constants_are_positive(self): + """Test that league constants are positive.""" + assert WEEKS_PER_SEASON > 0 + assert GAMES_PER_WEEK > 0 + assert MODERN_STATS_START_SEASON > 0 + + assert isinstance(WEEKS_PER_SEASON, int) + assert isinstance(GAMES_PER_WEEK, int) + assert isinstance(MODERN_STATS_START_SEASON, int) + + def test_league_constants_are_reasonable(self): + """Test that league constants have reasonable values.""" + # Baseball season should be reasonable length + assert 10 <= WEEKS_PER_SEASON <= 30 + + # Games per week should be reasonable + assert 1 <= GAMES_PER_WEEK <= 7 + + # Modern stats era should be reasonable + assert 1 <= MODERN_STATS_START_SEASON <= 20 + + +class TestAPIConstants: + """Test API-related constants.""" + + def test_api_version_format(self): + """Test that API version is properly formatted.""" + assert isinstance(API_VERSION, str) + assert API_VERSION.startswith("v") + assert API_VERSION[1:].isdigit() # Should be like "v3" + + def test_timeout_and_retry_values(self): + """Test that timeout and retry values are reasonable.""" + assert DEFAULT_TIMEOUT > 0 + assert MAX_RETRIES > 0 + + assert isinstance(DEFAULT_TIMEOUT, int) + assert isinstance(MAX_RETRIES, int) + + # Should be reasonable values + assert 1 <= DEFAULT_TIMEOUT <= 60 # 1-60 seconds + assert 1 <= MAX_RETRIES <= 10 # 1-10 retries + + +class TestPositionConstants: + """Test baseball position constants.""" + + def test_position_sets_are_sets(self): + """Test that position constants are sets.""" + assert isinstance(PITCHER_POSITIONS, set) + assert isinstance(POSITION_FIELDERS, set) + assert isinstance(ALL_POSITIONS, set) + + def test_position_sets_not_empty(self): + """Test that position sets are not empty.""" + assert len(PITCHER_POSITIONS) > 0 + assert len(POSITION_FIELDERS) > 0 + assert len(ALL_POSITIONS) > 0 + + def test_position_sets_no_overlap(self): + """Test that pitcher and fielder positions don't overlap.""" + overlap = PITCHER_POSITIONS & POSITION_FIELDERS + assert len(overlap) == 0, f"Found overlapping positions: {overlap}" + + def test_all_positions_is_union(self): + """Test that ALL_POSITIONS is the union of pitcher and fielder positions.""" + expected_all = PITCHER_POSITIONS | POSITION_FIELDERS + assert ALL_POSITIONS == expected_all + + def test_position_values_are_strings(self): + """Test that all position values are strings.""" + for position in ALL_POSITIONS: + assert isinstance(position, str) + assert len(position) > 0 # Not empty strings + + def test_common_positions_exist(self): + """Test that common baseball positions exist.""" + # Common pitcher positions + assert "SP" in PITCHER_POSITIONS or "P" in PITCHER_POSITIONS + assert "RP" in PITCHER_POSITIONS or "P" in PITCHER_POSITIONS + + # Common fielder positions + common_fielders = {"C", "1B", "2B", "3B", "SS", "LF", "CF", "RF"} + found_fielders = common_fielders & POSITION_FIELDERS + assert len(found_fielders) > 0, "No common fielder positions found" + + +class TestDraftConstants: + """Test draft-related constants.""" + + def test_draft_constants_are_positive(self): + """Test that draft constants are positive.""" + assert DEFAULT_PICK_MINUTES > 0 + assert DRAFT_ROUNDS > 0 + + assert isinstance(DEFAULT_PICK_MINUTES, int) + assert isinstance(DRAFT_ROUNDS, int) + + def test_draft_constants_are_reasonable(self): + """Test that draft constants have reasonable values.""" + # Pick minutes should be reasonable + assert 1 <= DEFAULT_PICK_MINUTES <= 60 + + # Draft rounds should be reasonable for fantasy baseball + assert 10 <= DRAFT_ROUNDS <= 50 \ No newline at end of file diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 0000000..a400a79 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,121 @@ +""" +Tests for custom exceptions + +Ensures exception hierarchy and behavior work correctly. +""" +import pytest + +from exceptions import ( + BotException, + APIException, + PlayerNotFoundError, + TeamNotFoundError, + DraftException, + ValidationException, + ConfigurationException +) + + +class TestExceptionHierarchy: + """Test that exceptions inherit correctly.""" + + def test_all_exceptions_inherit_from_bot_exception(self): + """Test that all custom exceptions inherit from BotException.""" + assert issubclass(APIException, BotException) + assert issubclass(PlayerNotFoundError, BotException) + assert issubclass(TeamNotFoundError, BotException) + assert issubclass(DraftException, BotException) + assert issubclass(ValidationException, BotException) + assert issubclass(ConfigurationException, BotException) + + def test_bot_exception_inherits_from_exception(self): + """Test that BotException inherits from built-in Exception.""" + assert issubclass(BotException, Exception) + + def test_exceptions_can_be_instantiated(self): + """Test that all exceptions can be created with messages.""" + message = "Test error message" + + exceptions = [ + BotException(message), + APIException(message), + PlayerNotFoundError(message), + TeamNotFoundError(message), + DraftException(message), + ValidationException(message), + ConfigurationException(message) + ] + + for exc in exceptions: + assert str(exc) == message + assert isinstance(exc, Exception) + + def test_exceptions_can_be_raised_and_caught(self): + """Test that exceptions can be raised and caught properly.""" + # Test specific exception catching + with pytest.raises(PlayerNotFoundError): + raise PlayerNotFoundError("Player not found") + + # Test catching by parent class + with pytest.raises(BotException): + raise APIException("API error") + + # Test catching by base Exception + with pytest.raises(Exception): + raise ValidationException("Validation error") + + +class TestExceptionMessages: + """Test exception message handling.""" + + def test_exceptions_with_no_message(self): + """Test that exceptions work without explicit messages.""" + exc = APIException() + assert isinstance(exc, APIException) + # Should not raise when converted to string + str(exc) + + def test_exceptions_preserve_message(self): + """Test that exception messages are preserved correctly.""" + message = "Detailed error description" + exc = PlayerNotFoundError(message) + assert str(exc) == message + + def test_exceptions_with_formatting(self): + """Test exceptions with formatted messages.""" + player_name = "John Doe" + exc = PlayerNotFoundError(f"Player '{player_name}' was not found") + assert "John Doe" in str(exc) + assert "not found" in str(exc) + + +class TestSpecificExceptions: + """Test specific exception use cases.""" + + def test_player_not_found_error(self): + """Test PlayerNotFoundError for missing players.""" + with pytest.raises(PlayerNotFoundError) as exc_info: + raise PlayerNotFoundError("Player 'Mike Trout' not found") + + assert "Mike Trout" in str(exc_info.value) + + def test_team_not_found_error(self): + """Test TeamNotFoundError for missing teams.""" + with pytest.raises(TeamNotFoundError) as exc_info: + raise TeamNotFoundError("Team 'LAA' not found") + + assert "LAA" in str(exc_info.value) + + def test_api_exception(self): + """Test APIException for API-related errors.""" + with pytest.raises(APIException) as exc_info: + raise APIException("Database connection failed") + + assert "Database connection failed" in str(exc_info.value) + + def test_draft_exception(self): + """Test DraftException for draft-related errors.""" + with pytest.raises(DraftException) as exc_info: + raise DraftException("Draft is not currently active") + + assert "Draft is not currently active" in str(exc_info.value) \ No newline at end of file diff --git a/tests/test_models.py b/tests/test_models.py new file mode 100644 index 0000000..25ded1f --- /dev/null +++ b/tests/test_models.py @@ -0,0 +1,416 @@ +""" +Tests for SBA data models + +Validates model creation, validation, and business logic. +""" +import pytest +from datetime import datetime + +from models import Team, Player, Current, DraftPick, DraftData, DraftList + + +class TestSBABaseModel: + """Test base model functionality.""" + + def test_model_creation_with_api_data(self): + """Test creating models from API data.""" + team_data = { + 'id': 1, + 'abbrev': 'NYY', + 'sname': 'Yankees', + 'lname': 'New York Yankees', + 'season': 12 + } + + team = Team.from_api_data(team_data) + assert team.id == 1 + assert team.abbrev == 'NYY' + assert team.lname == 'New York Yankees' + + def test_to_dict_functionality(self): + """Test model to dictionary conversion.""" + team = Team(abbrev='LAA', sname='Angels', lname='Los Angeles Angels', season=12) + + team_dict = team.to_dict() + assert 'abbrev' in team_dict + assert team_dict['abbrev'] == 'LAA' + assert team_dict['lname'] == 'Los Angeles Angels' + + def test_model_repr(self): + """Test model string representation.""" + team = Team(abbrev='BOS', sname='Red Sox', lname='Boston Red Sox', season=12) + repr_str = repr(team) + assert 'Team(' in repr_str + assert 'abbrev=BOS' in repr_str + + +class TestTeamModel: + """Test Team model functionality.""" + + def test_team_creation_minimal(self): + """Test team creation with minimal required fields.""" + team = Team( + abbrev='HOU', + sname='Astros', + lname='Houston Astros', + season=12 + ) + + assert team.abbrev == 'HOU' + assert team.sname == 'Astros' + assert team.lname == 'Houston Astros' + assert team.season == 12 + + def test_team_creation_with_optional_fields(self): + """Test team creation with optional fields.""" + team = Team( + abbrev='SF', + sname='Giants', + lname='San Francisco Giants', + season=12, + gmid=100, + division_id=1, + stadium='Oracle Park', + color='FF8C00' + ) + + assert team.gmid == 100 + assert team.division_id == 1 + assert team.stadium == 'Oracle Park' + assert team.color == 'FF8C00' + + def test_team_str_representation(self): + """Test team string representation.""" + team = Team(abbrev='SD', sname='Padres', lname='San Diego Padres', season=12) + assert str(team) == 'SD - San Diego Padres' + + +class TestPlayerModel: + """Test Player model functionality.""" + + def test_player_creation(self): + """Test player creation with required fields.""" + player = Player( + name='Mike Trout', + wara=8.5, + season=12, + team_id=1, + image='trout.jpg', + pos_1='CF' + ) + + assert player.name == 'Mike Trout' + assert player.wara == 8.5 + assert player.team_id == 1 + assert player.pos_1 == 'CF' + + def test_player_positions_property(self): + """Test player positions property.""" + player = Player( + name='Shohei Ohtani', + wara=9.0, + season=12, + team_id=1, + image='ohtani.jpg', + pos_1='SP', + pos_2='DH', + pos_3='RF' + ) + + positions = player.positions + assert len(positions) == 3 + assert 'SP' in positions + assert 'DH' in positions + assert 'RF' in positions + + def test_player_primary_position(self): + """Test primary position property.""" + player = Player( + name='Mookie Betts', + wara=7.2, + season=12, + team_id=1, + image='betts.jpg', + pos_1='RF', + pos_2='2B' + ) + + assert player.primary_position == 'RF' + + def test_player_is_pitcher(self): + """Test is_pitcher property.""" + pitcher = Player( + name='Gerrit Cole', + wara=6.8, + season=12, + team_id=1, + image='cole.jpg', + pos_1='SP' + ) + + position_player = Player( + name='Aaron Judge', + wara=8.1, + season=12, + team_id=1, + image='judge.jpg', + pos_1='RF' + ) + + assert pitcher.is_pitcher is True + assert position_player.is_pitcher is False + + def test_player_str_representation(self): + """Test player string representation.""" + player = Player( + name='Ronald Acuna Jr.', + wara=8.8, + season=12, + team_id=1, + image='acuna.jpg', + pos_1='OF' + ) + + assert str(player) == 'Ronald Acuna Jr. (OF)' + + +class TestCurrentModel: + """Test Current league state model.""" + + def test_current_default_values(self): + """Test current model with default values.""" + current = Current() + + assert current.week == 69 + assert current.season == 69 + assert current.freeze is True + assert current.bet_week == 'sheets' + + def test_current_with_custom_values(self): + """Test current model with custom values.""" + current = Current( + week=15, + season=12, + freeze=False, + trade_deadline=14, + playoffs_begin=19 + ) + + assert current.week == 15 + assert current.season == 12 + assert current.freeze is False + + def test_current_properties(self): + """Test current model properties.""" + # Regular season + current = Current(week=10, playoffs_begin=19) + assert current.is_offseason is False + assert current.is_playoffs is False + + # Playoffs + current = Current(week=20, playoffs_begin=19) + assert current.is_offseason is True + assert current.is_playoffs is True + + # Pick trading + current = Current(week=15, pick_trade_start=10, pick_trade_end=20) + assert current.can_trade_picks is True + + +class TestDraftPickModel: + """Test DraftPick model functionality.""" + + def test_draft_pick_creation(self): + """Test draft pick creation.""" + pick = DraftPick( + season=12, + overall=1, + round=1, + origowner_id=1, + owner_id=1 + ) + + assert pick.season == 12 + assert pick.overall == 1 + assert pick.origowner_id == 1 + assert pick.owner_id == 1 + + def test_draft_pick_properties(self): + """Test draft pick properties.""" + # Not traded, not selected + pick = DraftPick( + season=12, + overall=5, + round=1, + origowner_id=1, + owner_id=1 + ) + + assert pick.is_traded is False + assert pick.is_selected is False + + # Traded pick + traded_pick = DraftPick( + season=12, + overall=10, + round=1, + origowner_id=1, + owner_id=2 + ) + + assert traded_pick.is_traded is True + + # Selected pick + selected_pick = DraftPick( + season=12, + overall=15, + round=1, + origowner_id=1, + owner_id=1, + player_id=100 + ) + + assert selected_pick.is_selected is True + + +class TestDraftDataModel: + """Test DraftData model functionality.""" + + def test_draft_data_creation(self): + """Test draft data creation.""" + draft_data = DraftData( + result_channel_id=123456789, + ping_channel_id=987654321, + pick_minutes=10 + ) + + assert draft_data.result_channel_id == 123456789 + assert draft_data.ping_channel_id == 987654321 + assert draft_data.pick_minutes == 10 + + def test_draft_data_properties(self): + """Test draft data properties.""" + # Inactive draft + draft_data = DraftData( + result_channel_id=123, + ping_channel_id=456, + timer=False + ) + + assert draft_data.is_draft_active is False + + # Active draft + active_draft = DraftData( + result_channel_id=123, + ping_channel_id=456, + timer=True + ) + + assert active_draft.is_draft_active is True + + +class TestDraftListModel: + """Test DraftList model functionality.""" + + def test_draft_list_creation(self): + """Test draft list creation.""" + draft_entry = DraftList( + season=12, + team_id=1, + rank=1, + player_id=100 + ) + + assert draft_entry.season == 12 + assert draft_entry.team_id == 1 + assert draft_entry.rank == 1 + assert draft_entry.player_id == 100 + + def test_draft_list_top_ranked_property(self): + """Test top ranked property.""" + top_pick = DraftList( + season=12, + team_id=1, + rank=1, + player_id=100 + ) + + lower_pick = DraftList( + season=12, + team_id=1, + rank=5, + player_id=200 + ) + + assert top_pick.is_top_ranked is True + assert lower_pick.is_top_ranked is False + + +class TestModelCoverageExtras: + """Additional model coverage tests.""" + + def test_base_model_from_api_data_validation(self): + """Test from_api_data with various edge cases.""" + from models.base import SBABaseModel + + # Test with empty data raises ValueError + with pytest.raises(ValueError, match="Cannot create SBABaseModel from empty data"): + SBABaseModel.from_api_data({}) + + # Test with None raises ValueError + with pytest.raises(ValueError, match="Cannot create SBABaseModel from empty data"): + SBABaseModel.from_api_data(None) + + def test_player_positions_comprehensive(self): + """Test player positions property with all position variations.""" + player_data = { + 'name': 'Multi-Position Player', + 'wara': 3.0, + 'season': 12, + 'team_id': 5, + 'image': 'https://example.com/player.jpg', + 'pos_1': 'C', + 'pos_2': '1B', + 'pos_3': '3B', + 'pos_4': None, # Test None handling + 'pos_5': 'DH', + 'pos_6': 'OF', + 'pos_7': None, # Another None + 'pos_8': 'SS' + } + player = Player.from_api_data(player_data) + + positions = player.positions + assert 'C' in positions + assert '1B' in positions + assert '3B' in positions + assert 'DH' in positions + assert 'OF' in positions + assert 'SS' in positions + assert len(positions) == 6 # Should exclude None values + assert None not in positions + + def test_player_is_pitcher_variations(self): + """Test is_pitcher property with different positions.""" + test_cases = [ + ('SP', True), # Starting pitcher + ('RP', True), # Relief pitcher + ('P', True), # Generic pitcher + ('C', False), # Catcher + ('1B', False), # First base + ('OF', False), # Outfield + ('DH', False), # Designated hitter + ] + + for position, expected in test_cases: + player_data = { + 'name': f'Test {position}', + 'wara': 2.0, + 'season': 12, + 'team_id': 5, + 'image': 'https://example.com/player.jpg', + 'pos_1': position, + } + player = Player.from_api_data(player_data) + assert player.is_pitcher == expected, f"Position {position} should return {expected}" + assert player.primary_position == position \ No newline at end of file diff --git a/tests/test_services_base_service.py b/tests/test_services_base_service.py new file mode 100644 index 0000000..5bf2d80 --- /dev/null +++ b/tests/test_services_base_service.py @@ -0,0 +1,275 @@ +""" +Tests for BaseService functionality +""" +import pytest +from unittest.mock import AsyncMock + +from services.base_service import BaseService +from models.base import SBABaseModel +from exceptions import APIException + + +class MockModel(SBABaseModel): + """Mock model for testing BaseService.""" + id: int + name: str + value: int = 100 + + +class TestBaseService: + """Test BaseService functionality.""" + + @pytest.fixture + def mock_client(self): + """Mock API client.""" + client = AsyncMock() + return client + + @pytest.fixture + def base_service(self, mock_client): + """Create BaseService instance for testing.""" + service = BaseService(MockModel, 'mocks', client=mock_client) + return service + + @pytest.mark.asyncio + async def test_init(self): + """Test service initialization.""" + service = BaseService(MockModel, 'test_endpoint') + assert service.model_class == MockModel + assert service.endpoint == 'test_endpoint' + assert service._client is None + + @pytest.mark.asyncio + async def test_get_by_id_success(self, base_service, mock_client): + """Test successful get_by_id.""" + mock_data = {'id': 1, 'name': 'Test', 'value': 200} + mock_client.get.return_value = mock_data + + result = await base_service.get_by_id(1) + + assert isinstance(result, MockModel) + assert result.id == 1 + assert result.name == 'Test' + assert result.value == 200 + mock_client.get.assert_called_once_with('mocks', object_id=1) + + @pytest.mark.asyncio + async def test_get_by_id_not_found(self, base_service, mock_client): + """Test get_by_id when object not found.""" + mock_client.get.return_value = None + + result = await base_service.get_by_id(999) + + assert result is None + mock_client.get.assert_called_once_with('mocks', object_id=999) + + @pytest.mark.asyncio + async def test_get_all_with_count(self, base_service, mock_client): + """Test get_all with count response format.""" + mock_data = { + 'count': 2, + 'mocks': [ + {'id': 1, 'name': 'Test1', 'value': 100}, + {'id': 2, 'name': 'Test2', 'value': 200} + ] + } + mock_client.get.return_value = mock_data + + result, count = await base_service.get_all() + + assert len(result) == 2 + assert count == 2 + assert all(isinstance(item, MockModel) for item in result) + mock_client.get.assert_called_once_with('mocks', params=None) + + @pytest.mark.asyncio + async def test_get_all_items_convenience(self, base_service, mock_client): + """Test get_all_items convenience method.""" + mock_data = { + 'count': 1, + 'mocks': [{'id': 1, 'name': 'Test', 'value': 100}] + } + mock_client.get.return_value = mock_data + + result = await base_service.get_all_items() + + assert len(result) == 1 + assert isinstance(result[0], MockModel) + + @pytest.mark.asyncio + async def test_create_success(self, base_service, mock_client): + """Test successful object creation.""" + input_data = {'name': 'New Item', 'value': 300} + response_data = {'id': 3, 'name': 'New Item', 'value': 300} + mock_client.post.return_value = response_data + + result = await base_service.create(input_data) + + assert isinstance(result, MockModel) + assert result.id == 3 + assert result.name == 'New Item' + mock_client.post.assert_called_once_with('mocks', input_data) + + @pytest.mark.asyncio + async def test_update_success(self, base_service, mock_client): + """Test successful object update.""" + update_data = {'name': 'Updated'} + response_data = {'id': 1, 'name': 'Updated', 'value': 100} + mock_client.put.return_value = response_data + + result = await base_service.update(1, update_data) + + assert isinstance(result, MockModel) + assert result.name == 'Updated' + mock_client.put.assert_called_once_with('mocks', update_data, object_id=1) + + @pytest.mark.asyncio + async def test_delete_success(self, base_service, mock_client): + """Test successful object deletion.""" + mock_client.delete.return_value = True + + result = await base_service.delete(1) + + assert result is True + mock_client.delete.assert_called_once_with('mocks', object_id=1) + + @pytest.mark.asyncio + async def test_search(self, base_service, mock_client): + """Test search functionality.""" + mock_data = { + 'count': 1, + 'mocks': [{'id': 1, 'name': 'Searchable', 'value': 100}] + } + mock_client.get.return_value = mock_data + + result = await base_service.search('Searchable') + + assert len(result) == 1 + mock_client.get.assert_called_once_with('mocks', params=[('q', 'Searchable')]) + + @pytest.mark.asyncio + async def test_get_by_field(self, base_service, mock_client): + """Test get_by_field functionality.""" + mock_data = { + 'count': 1, + 'mocks': [{'id': 1, 'name': 'Test', 'value': 100}] + } + mock_client.get.return_value = mock_data + + result = await base_service.get_by_field('name', 'Test') + + assert len(result) == 1 + mock_client.get.assert_called_once_with('mocks', params=[('name', 'Test')]) + + def test_extract_items_and_count_standard_format(self, base_service): + """Test response parsing for standard format.""" + data = { + 'count': 3, + 'mocks': [ + {'id': 1, 'name': 'Test1'}, + {'id': 2, 'name': 'Test2'}, + {'id': 3, 'name': 'Test3'} + ] + } + + items, count = base_service._extract_items_and_count_from_response(data) + + assert len(items) == 3 + assert count == 3 + assert items[0]['name'] == 'Test1' + + def test_extract_items_and_count_single_object(self, base_service): + """Test response parsing for single object.""" + data = {'id': 1, 'name': 'Single'} + + items, count = base_service._extract_items_and_count_from_response(data) + + assert len(items) == 1 + assert count == 1 + assert items[0] == data + + def test_extract_items_and_count_direct_list(self, base_service): + """Test response parsing for direct list.""" + data = [ + {'id': 1, 'name': 'Test1'}, + {'id': 2, 'name': 'Test2'} + ] + + items, count = base_service._extract_items_and_count_from_response(data) + + assert len(items) == 2 + assert count == 2 + + +class TestBaseServiceExtras: + """Additional coverage tests for BaseService edge cases.""" + + @pytest.mark.asyncio + async def test_base_service_additional_methods(self): + """Test additional BaseService methods for coverage.""" + from services.base_service import BaseService + from models.base import SBABaseModel + + class TestModel(SBABaseModel): + name: str + value: int = 100 + + mock_client = AsyncMock() + service = BaseService(TestModel, 'test', client=mock_client) + + # Test search with kwargs + mock_client.get.return_value = {'count': 1, 'test': [{'name': 'Test', 'value': 100}]} + result = await service.search('query', season=12, active=True) + expected_params = [('q', 'query'), ('season', 12), ('active', True)] + mock_client.get.assert_called_once_with('test', params=expected_params) + + # Test count method + mock_client.reset_mock() + mock_client.get.return_value = {'count': 42, 'test': []} + count = await service.count(params=[('active', 'true')]) + assert count == 42 + + # Test update_from_model with ID + mock_client.reset_mock() + model = TestModel(id=1, name="Updated", value=300) + mock_client.put.return_value = {"id": 1, "name": "Updated", "value": 300} + result = await service.update_from_model(model) + assert result.name == "Updated" + + # Test update_from_model without ID + model_no_id = TestModel(name="Test") + with pytest.raises(ValueError, match="Cannot update TestModel without ID"): + await service.update_from_model(model_no_id) + + def test_base_service_response_parsing_edge_cases(self): + """Test edge cases in response parsing.""" + from services.base_service import BaseService + from models.base import SBABaseModel + + class TestModel(SBABaseModel): + name: str + + service = BaseService(TestModel, 'test') + + # Test with 'items' field + data = {'count': 2, 'items': [{'name': 'Item1'}, {'name': 'Item2'}]} + items, count = service._extract_items_and_count_from_response(data) + assert len(items) == 2 + assert count == 2 + + # Test with 'data' field + data = {'count': 1, 'data': [{'name': 'DataItem'}]} + items, count = service._extract_items_and_count_from_response(data) + assert len(items) == 1 + assert count == 1 + + # Test with count but no recognizable list field + data = {'count': 5, 'unknown_field': [{'name': 'Item'}]} + items, count = service._extract_items_and_count_from_response(data) + assert len(items) == 0 + assert count == 5 + + # Test with unexpected data type + items, count = service._extract_items_and_count_from_response("unexpected") + assert len(items) == 0 + assert count == 0 \ No newline at end of file diff --git a/tests/test_services_integration.py b/tests/test_services_integration.py new file mode 100644 index 0000000..572e89e --- /dev/null +++ b/tests/test_services_integration.py @@ -0,0 +1,65 @@ +""" +Tests for service integration and global service instances +""" +import pytest + +from services.player_service import PlayerService, player_service +from services.team_service import TeamService, team_service +from models.player import Player +from models.team import Team + + +class TestGlobalServiceInstances: + """Test global service instances and their integration.""" + + def test_player_service_global(self): + """Test global player service instance.""" + assert isinstance(player_service, PlayerService) + assert player_service.model_class == Player + assert player_service.endpoint == 'players' + + def test_team_service_global(self): + """Test global team service instance.""" + assert isinstance(team_service, TeamService) + assert team_service.model_class == Team + assert team_service.endpoint == 'teams' + + def test_service_wiring(self): + """Test that services are properly wired together.""" + # PlayerService should have TeamService injected + assert player_service._team_service is not None + assert isinstance(player_service._team_service, TeamService) + assert player_service._team_service is team_service + + @pytest.mark.asyncio + async def test_service_independence(self): + """Test that service instances are independent.""" + player_service1 = PlayerService() + player_service2 = PlayerService() + team_service1 = TeamService() + team_service2 = TeamService() + + # Should be different instances + assert player_service1 is not player_service2 + assert team_service1 is not team_service2 + + # But same configuration + assert player_service1.model_class == player_service2.model_class + assert player_service1.endpoint == player_service2.endpoint + assert team_service1.model_class == team_service2.model_class + assert team_service1.endpoint == team_service2.endpoint + + def test_service_imports_work(self): + """Test that service imports work from the main services module.""" + from services import player_service, team_service, PlayerService, TeamService + + # Should be able to import both services and their classes + assert isinstance(player_service, PlayerService) + assert isinstance(team_service, TeamService) + + # Should be the same instances as imported directly + from services.player_service import player_service as direct_player_service + from services.team_service import team_service as direct_team_service + + assert player_service is direct_player_service + assert team_service is direct_team_service \ No newline at end of file diff --git a/tests/test_services_player_service.py b/tests/test_services_player_service.py new file mode 100644 index 0000000..cf6b3aa --- /dev/null +++ b/tests/test_services_player_service.py @@ -0,0 +1,274 @@ +""" +Tests for PlayerService functionality +""" +import pytest +from unittest.mock import AsyncMock + +from services.player_service import PlayerService, player_service +from models.player import Player +from constants import FREE_AGENT_TEAM_ID +from exceptions import APIException + + +class TestPlayerService: + """Test PlayerService functionality.""" + + @pytest.fixture + def mock_client(self): + """Mock API client.""" + client = AsyncMock() + return client + + @pytest.fixture + def player_service_instance(self, mock_client): + """Create PlayerService instance with mocked client.""" + service = PlayerService() + service._client = mock_client + return service + + def create_player_data(self, player_id: int, name: str, team_id: int = 5, position: str = 'C', **kwargs): + """Create complete player data for testing.""" + base_data = { + 'id': player_id, + 'name': name, + 'wara': 2.5, + 'season': 12, + 'team_id': team_id, + 'image': f'https://example.com/player{player_id}.jpg', + 'pos_1': position, + } + base_data.update(kwargs) + return base_data + + @pytest.mark.asyncio + async def test_get_player_success(self, player_service_instance, mock_client): + """Test successful player retrieval.""" + mock_data = self.create_player_data(1, 'Test Player', pos_2='1B') + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_player(1) + + assert isinstance(result, Player) + assert result.name == 'Test Player' + assert result.wara == 2.5 + assert result.season == 12 + assert result.primary_position == 'C' + mock_client.get.assert_called_once_with('players', object_id=1) + + @pytest.mark.asyncio + async def test_get_player_with_team(self, player_service_instance, mock_client): + """Test player retrieval with team population.""" + player_data = self.create_player_data(1, 'Test Player', team_id=5) + team_data = { + 'id': 5, + 'abbrev': 'TST', + 'sname': 'Test Team', + 'lname': 'Test Team Long Name', + 'season': 12 + } + + # Mock the get calls + mock_client.get.side_effect = [player_data, team_data] + + result = await player_service_instance.get_player_with_team(1) + + assert isinstance(result, Player) + assert result.name == 'Test Player' + assert result.team is not None + assert result.team.sname == 'Test Team' + + # Should call get twice: once for player, once for team + assert mock_client.get.call_count == 2 + + @pytest.mark.asyncio + async def test_get_players_by_team(self, player_service_instance, mock_client): + """Test getting players by team.""" + mock_data = { + 'count': 2, + 'players': [ + self.create_player_data(1, 'Player1', team_id=5), + self.create_player_data(2, 'Player2', team_id=5) + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_players_by_team(5, season=12) + + assert len(result) == 2 + assert all(isinstance(p, Player) for p in result) + mock_client.get.assert_called_once_with('players', params=[('season', '12'), ('team_id', '5')]) + + @pytest.mark.asyncio + async def test_get_players_by_name(self, player_service_instance, mock_client): + """Test searching players by name.""" + mock_data = { + 'count': 1, + 'players': [ + self.create_player_data(1, 'John Smith', team_id=5) + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_players_by_name('John', season=12) + + assert len(result) == 1 + assert result[0].name == 'John Smith' + mock_client.get.assert_called_once_with('players', params=[('season', '12'), ('name', 'John')]) + + @pytest.mark.asyncio + async def test_get_player_by_name_exact(self, player_service_instance, mock_client): + """Test exact name matching.""" + mock_data = { + 'count': 2, + 'players': [ + self.create_player_data(1, 'John Smith', team_id=5), + self.create_player_data(2, 'John Doe', team_id=6) + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_player_by_name_exact('John Smith', season=12) + + assert result is not None + assert result.name == 'John Smith' + assert result.id == 1 + + @pytest.mark.asyncio + async def test_get_free_agents(self, player_service_instance, mock_client): + """Test getting free agents.""" + mock_data = { + 'count': 2, + 'players': [ + self.create_player_data(1, 'Free Agent 1', team_id=FREE_AGENT_TEAM_ID), + self.create_player_data(2, 'Free Agent 2', team_id=FREE_AGENT_TEAM_ID) + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_free_agents(season=12) + + assert len(result) == 2 + assert all(p.team_id == FREE_AGENT_TEAM_ID for p in result) + mock_client.get.assert_called_once_with('players', params=[('team_id', FREE_AGENT_TEAM_ID), ('season', '12')]) + + @pytest.mark.asyncio + async def test_is_free_agent(self, player_service_instance): + """Test free agent checking.""" + # Create test players with all required fields + free_agent_data = self.create_player_data(1, 'Free Agent', team_id=FREE_AGENT_TEAM_ID) + regular_player_data = self.create_player_data(2, 'Regular Player', team_id=5) + + free_agent = Player.from_api_data(free_agent_data) + regular_player = Player.from_api_data(regular_player_data) + + assert await player_service_instance.is_free_agent(free_agent) is True + assert await player_service_instance.is_free_agent(regular_player) is False + + @pytest.mark.asyncio + async def test_search_players_fuzzy(self, player_service_instance, mock_client): + """Test fuzzy search with relevance sorting.""" + mock_data = { + 'count': 3, + 'players': [ + self.create_player_data(1, 'John Smith', team_id=5), # partial match + self.create_player_data(2, 'John', team_id=6), # exact match + self.create_player_data(3, 'Johnny Doe', team_id=7) # partial match + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.search_players_fuzzy('John', limit=2) + + # Should return exact match first, then partial matches, limited to 2 + assert len(result) == 2 + assert result[0].name == 'John' # exact match first + mock_client.get.assert_called_once_with('players', params=[('q', 'John')]) + + @pytest.mark.asyncio + async def test_get_players_by_position(self, player_service_instance, mock_client): + """Test getting players by position.""" + mock_data = { + 'count': 2, + 'players': [ + self.create_player_data(1, 'Catcher 1', position='C', team_id=5), + self.create_player_data(2, 'Catcher 2', position='C', team_id=6) + ] + } + mock_client.get.return_value = mock_data + + result = await player_service_instance.get_players_by_position('C', season=12) + + assert len(result) == 2 + assert all(p.primary_position == 'C' for p in result) + mock_client.get.assert_called_once_with('players', params=[('position', 'C'), ('season', '12')]) + + @pytest.mark.asyncio + async def test_error_handling(self, player_service_instance, mock_client): + """Test error handling in service methods.""" + mock_client.get.side_effect = APIException("API Error") + + # Should return None/empty list on errors, not raise + result = await player_service_instance.get_player(1) + assert result is None + + result = await player_service_instance.get_players_by_team(5, season=12) + assert result == [] + + +class TestPlayerServiceExtras: + """Additional coverage tests for PlayerService edge cases.""" + + def create_player_data(self, player_id: int, name: str, team_id: int = 5, position: str = 'C', **kwargs): + """Create complete player data for testing.""" + base_data = { + 'id': player_id, + 'name': name, + 'wara': 2.5, + 'season': 12, + 'team_id': team_id, + 'image': f'https://example.com/player{player_id}.jpg', + 'pos_1': position, + } + base_data.update(kwargs) + return base_data + + @pytest.mark.asyncio + async def test_player_service_additional_methods(self): + """Test additional PlayerService methods for coverage.""" + from services.player_service import PlayerService + from constants import FREE_AGENT_TEAM_ID + + mock_client = AsyncMock() + player_service = PlayerService() + player_service._client = mock_client + + # Test additional functionality + mock_client.get.return_value = { + 'count': 1, + 'players': [self.create_player_data(1, 'Test Player')] + } + + result = await player_service.get_players_by_name('Test', season=12) + assert len(result) == 1 + + +class TestGlobalPlayerServiceInstance: + """Test global player service instance.""" + + def test_player_service_global(self): + """Test global player service instance.""" + assert isinstance(player_service, PlayerService) + assert player_service.model_class == Player + assert player_service.endpoint == 'players' + + @pytest.mark.asyncio + async def test_service_independence(self): + """Test that service instances are independent.""" + service1 = PlayerService() + service2 = PlayerService() + + # Should be different instances + assert service1 is not service2 + # But same configuration + assert service1.model_class == service2.model_class + assert service1.endpoint == service2.endpoint \ No newline at end of file diff --git a/tests/test_services_team_service.py b/tests/test_services_team_service.py new file mode 100644 index 0000000..24d0161 --- /dev/null +++ b/tests/test_services_team_service.py @@ -0,0 +1,332 @@ +""" +Tests for TeamService functionality +""" +import pytest +from unittest.mock import AsyncMock + +from services.team_service import TeamService, team_service +from models.team import Team +from constants import SBA_CURRENT_SEASON +from exceptions import APIException + + +class TestTeamService: + """Test TeamService functionality.""" + + @pytest.fixture + def mock_client(self): + """Mock API client.""" + client = AsyncMock() + return client + + @pytest.fixture + def team_service_instance(self, mock_client): + """Create TeamService instance with mocked client.""" + service = TeamService() + service._client = mock_client + return service + + def create_team_data(self, team_id: int, abbrev: str, season: int = 12, **kwargs): + """Create complete team data for testing.""" + base_data = { + 'id': team_id, + 'abbrev': abbrev, + 'sname': f'{abbrev} Team', + 'lname': f'{abbrev} Long Team Name', + 'season': season, + 'gmid': 101, + 'division_id': 1, + } + base_data.update(kwargs) + return base_data + + @pytest.mark.asyncio + async def test_get_team_success(self, team_service_instance, mock_client): + """Test successful team retrieval.""" + mock_data = self.create_team_data(1, 'TST', stadium='Test Stadium') + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_team(1) + + assert isinstance(result, Team) + assert result.abbrev == 'TST' + assert result.sname == 'TST Team' + assert result.season == 12 + mock_client.get.assert_called_once_with('teams', object_id=1) + + @pytest.mark.asyncio + async def test_get_team_by_abbrev_success(self, team_service_instance, mock_client): + """Test getting team by abbreviation.""" + mock_data = { + 'count': 1, + 'teams': [self.create_team_data(1, 'NYY', season=12)] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_team_by_abbrev('nyy', season=12) + + assert isinstance(result, Team) + assert result.abbrev == 'NYY' + mock_client.get.assert_called_once_with('teams', params=[('abbrev', 'NYY'), ('season', '12')]) + + @pytest.mark.asyncio + async def test_get_team_by_abbrev_not_found(self, team_service_instance, mock_client): + """Test getting team by abbreviation when not found.""" + mock_data = {'count': 0, 'teams': []} + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_team_by_abbrev('XXX', season=12) + + assert result is None + + @pytest.mark.asyncio + async def test_get_teams_by_season(self, team_service_instance, mock_client): + """Test getting all teams for a season.""" + mock_data = { + 'count': 3, + 'teams': [ + self.create_team_data(1, 'TEA', season=12), + self.create_team_data(2, 'TEB', season=12), + self.create_team_data(3, 'TEC', season=12) + ] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_teams_by_season(12) + + assert len(result) == 3 + assert all(isinstance(team, Team) for team in result) + assert all(team.season == 12 for team in result) + mock_client.get.assert_called_once_with('teams', params=[('season', '12')]) + + @pytest.mark.asyncio + async def test_get_teams_by_manager(self, team_service_instance, mock_client): + """Test getting teams by manager.""" + mock_data = { + 'count': 2, + 'teams': [ + self.create_team_data(1, 'TEA', manager1_id=101, season=12), + self.create_team_data(2, 'TEB', manager2_id=101, season=12) + ] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_teams_by_manager(101, season=12) + + assert len(result) == 2 + assert all(isinstance(team, Team) for team in result) + mock_client.get.assert_called_once_with('teams', params=[('manager_id', '101'), ('season', '12')]) + + @pytest.mark.asyncio + async def test_get_teams_by_division(self, team_service_instance, mock_client): + """Test getting teams by division.""" + mock_data = { + 'count': 4, + 'teams': [ + self.create_team_data(1, 'TEA', division_id=1, season=12), + self.create_team_data(2, 'TEB', division_id=1, season=12), + self.create_team_data(3, 'TEC', division_id=1, season=12), + self.create_team_data(4, 'TED', division_id=1, season=12) + ] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_teams_by_division(1, season=12) + + assert len(result) == 4 + assert all(isinstance(team, Team) for team in result) + mock_client.get.assert_called_once_with('teams', params=[('division_id', '1'), ('season', '12')]) + + @pytest.mark.asyncio + async def test_get_team_roster(self, team_service_instance, mock_client): + """Test getting team roster with position counts.""" + mock_roster_data = { + 'active': { + 'C': 2, '1B': 1, '2B': 1, '3B': 1, 'SS': 1, 'LF': 1, 'CF': 1, 'RF': 1, 'DH': 1, + 'SP': 5, 'RP': 8, 'CP': 2, 'WARa': 45.2, + 'players': [ + {'id': 1, 'name': 'Player 1', 'wara': 5.2}, + {'id': 2, 'name': 'Player 2', 'wara': 4.8} + ] + }, + 'shortil': { + 'C': 0, '1B': 0, '2B': 0, '3B': 0, 'SS': 0, 'LF': 0, 'CF': 0, 'RF': 0, 'DH': 0, + 'SP': 2, 'RP': 3, 'CP': 0, 'WARa': 8.5, + 'players': [ + {'id': 3, 'name': 'Minor Player 1', 'wara': 2.1} + ] + }, + 'longil': { + 'C': 0, '1B': 0, '2B': 0, '3B': 0, 'SS': 0, 'LF': 0, 'CF': 0, 'RF': 0, 'DH': 0, + 'SP': 0, 'RP': 1, 'CP': 0, 'WARa': 1.2, + 'players': [ + {'id': 4, 'name': 'Injured Player', 'wara': 1.2} + ] + } + } + mock_client.get.return_value = mock_roster_data + + result = await team_service_instance.get_team_roster(1, 'current') + + assert result is not None + assert 'active' in result + assert 'shortil' in result + assert 'longil' in result + assert result['active']['C'] == 2 + assert result['active']['SP'] == 5 + assert len(result['active']['players']) == 2 + mock_client.get.assert_called_once_with('teams/1/roster/current') + + @pytest.mark.asyncio + async def test_get_team_roster_next_week(self, team_service_instance, mock_client): + """Test getting next week's roster.""" + mock_roster_data = { + 'active': {'C': 1, 'SP': 5, 'players': []}, + 'shortil': {'C': 0, 'SP': 0, 'players': []}, + 'longil': {'C': 0, 'SP': 0, 'players': []} + } + mock_client.get.return_value = mock_roster_data + + result = await team_service_instance.get_team_roster(1, 'next') + + assert result is not None + mock_client.get.assert_called_once_with('teams/1/roster/next') + + @pytest.mark.asyncio + async def test_get_team_standings_position(self, team_service_instance, mock_client): + """Test getting team standings information.""" + mock_standings_data = { + 'id': 1, + 'team_id': 1, + 'wins': 45, + 'losses': 27, + 'run_diff': 125, + 'div_gb': None, + 'wc_gb': None, + 'home_wins': 25, + 'home_losses': 11, + 'away_wins': 20, + 'away_losses': 16 + } + mock_client.get.return_value = mock_standings_data + + result = await team_service_instance.get_team_standings_position(1, season=12) + + assert result is not None + assert result['wins'] == 45 + assert result['losses'] == 27 + assert result['run_diff'] == 125 + mock_client.get.assert_called_once_with('standings/team/1', params=[('season', '12')]) + + @pytest.mark.asyncio + async def test_update_team(self, team_service_instance, mock_client): + """Test team update functionality.""" + update_data = {'stadium': 'New Stadium', 'color': '#FF0000'} + response_data = self.create_team_data(1, 'TST', stadium='New Stadium', color='#FF0000') + mock_client.put.return_value = response_data + + result = await team_service_instance.update_team(1, update_data) + + assert isinstance(result, Team) + assert result.stadium == 'New Stadium' + assert result.color == '#FF0000' + mock_client.put.assert_called_once_with('teams', update_data, object_id=1) + + @pytest.mark.asyncio + async def test_is_valid_team_abbrev(self, team_service_instance, mock_client): + """Test team abbreviation validation.""" + # Mock successful lookup + mock_data = { + 'count': 1, + 'teams': [self.create_team_data(1, 'TST', season=12)] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.is_valid_team_abbrev('TST', season=12) + + assert result is True + + # Mock failed lookup + mock_client.get.return_value = {'count': 0, 'teams': []} + + result = await team_service_instance.is_valid_team_abbrev('XXX', season=12) + + assert result is False + + @pytest.mark.asyncio + async def test_get_current_season_teams(self, team_service_instance, mock_client): + """Test getting current season teams.""" + mock_data = { + 'count': 2, + 'teams': [ + self.create_team_data(1, 'TEA', season=SBA_CURRENT_SEASON), + self.create_team_data(2, 'TEB', season=SBA_CURRENT_SEASON) + ] + } + mock_client.get.return_value = mock_data + + result = await team_service_instance.get_current_season_teams() + + assert len(result) == 2 + assert all(team.season == SBA_CURRENT_SEASON for team in result) + mock_client.get.assert_called_once_with('teams', params=[('season', str(SBA_CURRENT_SEASON))]) + + @pytest.mark.asyncio + async def test_error_handling(self, team_service_instance, mock_client): + """Test error handling in team service methods.""" + mock_client.get.side_effect = APIException("API Error") + + # Should return None/empty list on errors, not raise + result = await team_service_instance.get_team(1) + assert result is None + + result = await team_service_instance.get_teams_by_season(12) + assert result == [] + + result = await team_service_instance.get_teams_by_manager(101) + assert result == [] + + result = await team_service_instance.get_team_roster(1) + assert result is None + + result = await team_service_instance.get_team_standings_position(1, 12) + assert result is None + + @pytest.mark.asyncio + async def test_abbrev_case_insensitive(self, team_service_instance, mock_client): + """Test that abbreviation lookup is case insensitive.""" + mock_data = { + 'count': 1, + 'teams': [self.create_team_data(1, 'NYY', season=12)] + } + mock_client.get.return_value = mock_data + + # Test with lowercase input + result = await team_service_instance.get_team_by_abbrev('nyy', season=12) + + assert result is not None + assert result.abbrev == 'NYY' + # Should call with uppercase + mock_client.get.assert_called_once_with('teams', params=[('abbrev', 'NYY'), ('season', '12')]) + + +class TestGlobalTeamServiceInstance: + """Test global team service instance.""" + + def test_team_service_global(self): + """Test global team service instance.""" + assert isinstance(team_service, TeamService) + assert team_service.model_class == Team + assert team_service.endpoint == 'teams' + + @pytest.mark.asyncio + async def test_service_independence(self): + """Test that service instances are independent.""" + service1 = TeamService() + service2 = TeamService() + + # Should be different instances + assert service1 is not service2 + # But same configuration + assert service1.model_class == service2.model_class + assert service1.endpoint == service2.endpoint \ No newline at end of file diff --git a/utils/README.md b/utils/README.md new file mode 100644 index 0000000..f0e124a --- /dev/null +++ b/utils/README.md @@ -0,0 +1,530 @@ +# Utils Package Documentation +**Discord Bot v2.0 - Utility Functions and Helpers** + +This package contains utility functions, helpers, and shared components used throughout the Discord bot application. + +## 📋 Table of Contents + +1. [**Structured Logging**](#-structured-logging) - Contextual logging with Discord integration +2. [**Future Utilities**](#-future-utilities) - Planned utility modules + +--- + +## 🔍 Structured Logging + +**Location:** `utils/logging.py` +**Purpose:** Provides hybrid logging system with contextual information for Discord bot debugging and monitoring. + +### **Quick Start** + +```python +from utils.logging import get_contextual_logger, set_discord_context + +class YourCommandCog(commands.Cog): + def __init__(self, bot): + self.bot = bot + self.logger = get_contextual_logger(f'{__name__}.YourCommandCog') + + async def your_command(self, interaction: discord.Interaction, param: str): + # Set Discord context for all subsequent log entries + set_discord_context( + interaction=interaction, + command="/your-command", + param_value=param + ) + + # Start operation timing and get trace ID + trace_id = self.logger.start_operation("your_command_operation") + + try: + self.logger.info("Command started") + + # Your command logic here + result = await some_api_call(param) + self.logger.debug("API call completed", result_count=len(result)) + + self.logger.info("Command completed successfully") + + except Exception as e: + self.logger.error("Command failed", error=e) + raise +``` + +### **Key Features** + +#### **🎯 Contextual Information** +Every log entry automatically includes: +- **Discord Context**: User ID, guild ID, guild name, channel ID +- **Command Context**: Command name, parameters +- **Operation Context**: Trace ID, operation name, execution duration +- **Custom Fields**: Additional context via keyword arguments + +#### **⏱️ Automatic Timing** +```python +trace_id = self.logger.start_operation("complex_operation") +# ... do work ... +self.logger.info("Operation completed") # Automatically includes duration_ms +``` + +#### **🔗 Request Tracing** +Track a single request through all log entries using trace IDs: +```bash +# Find all logs for a specific request +jq '.context.trace_id == "abc12345"' logs/discord_bot_v2.json +``` + +#### **📤 Hybrid Output** +- **Console**: Human-readable for development +- **Traditional File** (`discord_bot_v2.log`): Human-readable with debug info +- **JSON File** (`discord_bot_v2.json`): Structured for analysis + +### **API Reference** + +#### **Core Functions** + +**`get_contextual_logger(logger_name: str) -> ContextualLogger`** +```python +# Get a logger instance for your module +logger = get_contextual_logger(f'{__name__}.MyClass') +``` + +**`set_discord_context(interaction=None, user_id=None, guild_id=None, **kwargs)`** +```python +# Set context from Discord interaction (recommended) +set_discord_context(interaction=interaction, command="/player", player_name="Mike Trout") + +# Or set context manually +set_discord_context(user_id="123456", guild_id="987654", custom_field="value") +``` + +**`clear_context()`** +```python +# Clear the current logging context (usually not needed) +clear_context() +``` + +#### **ContextualLogger Methods** + +**`start_operation(operation_name: str = None) -> str`** +```python +# Start timing and get trace ID +trace_id = logger.start_operation("player_search") +``` + +**`info(message: str, **kwargs)`** +```python +logger.info("Player found", player_id=123, team_name="Yankees") +``` + +**`debug(message: str, **kwargs)`** +```python +logger.debug("API call started", endpoint="players/search", timeout=30) +``` + +**`warning(message: str, **kwargs)`** +```python +logger.warning("Multiple players found", candidates=["Player A", "Player B"]) +``` + +**`error(message: str, error: Exception = None, **kwargs)`** +```python +# With exception +logger.error("API call failed", error=e, retry_count=3) + +# Without exception +logger.error("Validation failed", field="player_name", value="invalid") +``` + +**`exception(message: str, **kwargs)`** +```python +# Automatically captures current exception +try: + risky_operation() +except: + logger.exception("Unexpected error in operation", operation_id=123) +``` + +### **Output Examples** + +#### **Console Output (Development)** +``` +2025-08-14 14:32:15,123 - commands.players.info.PlayerInfoCommands - INFO - Player info command started +2025-08-14 14:32:16,456 - commands.players.info.PlayerInfoCommands - DEBUG - Starting player search +2025-08-14 14:32:18,789 - commands.players.info.PlayerInfoCommands - INFO - Command completed successfully +``` + +#### **JSON Output (Monitoring & Analysis)** +```json +{ + "timestamp": "2025-08-14T14:32:15.123Z", + "level": "INFO", + "logger": "commands.players.info.PlayerInfoCommands", + "message": "Player info command started", + "function": "player_info", + "line": 50, + "context": { + "user_id": "123456789", + "guild_id": "987654321", + "guild_name": "SBA League", + "channel_id": "555666777", + "command": "/player", + "player_name": "Mike Trout", + "season": 12, + "trace_id": "abc12345", + "operation": "player_info_command" + }, + "extra": { + "duration_ms": 0 + } +} +``` + +#### **Error Output with Exception** +```json +{ + "timestamp": "2025-08-14T14:32:18.789Z", + "level": "ERROR", + "logger": "commands.players.info.PlayerInfoCommands", + "message": "API call failed", + "function": "player_info", + "line": 125, + "exception": { + "type": "APITimeout", + "message": "Request timed out after 30s", + "traceback": "Traceback (most recent call last):\n File ..." + }, + "context": { + "user_id": "123456789", + "guild_id": "987654321", + "command": "/player", + "player_name": "Mike Trout", + "trace_id": "abc12345" + }, + "extra": { + "duration_ms": 30000, + "retry_count": 3, + "endpoint": "players/search" + } +} +``` + +### **Advanced Usage Patterns** + +#### **API Call Logging** +```python +async def fetch_player_data(self, player_name: str): + self.logger.debug("API call started", + api_endpoint="players/search", + search_term=player_name, + timeout_ms=30000) + + try: + result = await api_client.get("players", params=[("name", player_name)]) + self.logger.info("API call successful", + results_found=len(result) if result else 0, + response_size_kb=len(str(result)) // 1024) + return result + + except TimeoutError as e: + self.logger.error("API timeout", + error=e, + endpoint="players/search", + search_term=player_name) + raise +``` + +#### **Performance Monitoring** +```python +async def complex_operation(self, data): + trace_id = self.logger.start_operation("complex_operation") + + # Step 1 + self.logger.debug("Processing step 1", step="validation") + validate_data(data) + + # Step 2 + self.logger.debug("Processing step 2", step="transformation") + processed = transform_data(data) + + # Step 3 + self.logger.debug("Processing step 3", step="persistence") + result = await save_data(processed) + + self.logger.info("Complex operation completed", + input_size=len(data), + output_size=len(result), + steps_completed=3) + + # Final log automatically includes total duration_ms +``` + +#### **Error Context Enrichment** +```python +async def handle_player_command(self, interaction, player_name): + set_discord_context( + interaction=interaction, + command="/player", + player_name=player_name, + # Add additional context that helps debugging + user_permissions=interaction.user.guild_permissions.administrator, + guild_member_count=len(interaction.guild.members), + request_timestamp=discord.utils.utcnow().isoformat() + ) + + try: + # Command logic + pass + except Exception as e: + # Error logs will include all the above context automatically + self.logger.error("Player command failed", + error=e, + # Additional error-specific context + error_code="PLAYER_NOT_FOUND", + suggestion="Try using the full player name") + raise +``` + +### **Querying JSON Logs** + +#### **Using jq for Analysis** + +**Find all errors:** +```bash +jq 'select(.level == "ERROR")' logs/discord_bot_v2.json +``` + +**Find slow operations (>5 seconds):** +```bash +jq 'select(.extra.duration_ms > 5000)' logs/discord_bot_v2.json +``` + +**Track a specific user's activity:** +```bash +jq 'select(.context.user_id == "123456789")' logs/discord_bot_v2.json +``` + +**Find API timeout errors:** +```bash +jq 'select(.exception.type == "APITimeout")' logs/discord_bot_v2.json +``` + +**Get error summary by type:** +```bash +jq -r 'select(.level == "ERROR") | .exception.type' logs/discord_bot_v2.json | sort | uniq -c +``` + +**Trace a complete request:** +```bash +jq 'select(.context.trace_id == "abc12345")' logs/discord_bot_v2.json | jq -s 'sort_by(.timestamp)' +``` + +#### **Performance Analysis** + +**Average command execution time:** +```bash +jq -r 'select(.message == "Command completed successfully") | .extra.duration_ms' logs/discord_bot_v2.json | awk '{sum+=$1; n++} END {print sum/n}' +``` + +**Most active users:** +```bash +jq -r '.context.user_id' logs/discord_bot_v2.json | sort | uniq -c | sort -nr | head -10 +``` + +**Command usage statistics:** +```bash +jq -r '.context.command' logs/discord_bot_v2.json | sort | uniq -c | sort -nr +``` + +### **Best Practices** + +#### **✅ Do:** +1. **Always set Discord context** at the start of command handlers +2. **Use start_operation()** for timing critical operations +3. **Include relevant context** in log messages via keyword arguments +4. **Log at appropriate levels** (debug for detailed flow, info for milestones, warning for recoverable issues, error for failures) +5. **Include error context** when logging exceptions + +#### **❌ Don't:** +1. **Don't log sensitive information** (passwords, tokens, personal data) +2. **Don't over-log in tight loops** (use sampling or conditional logging) +3. **Don't use string formatting in log messages** (use keyword arguments instead) +4. **Don't forget to handle exceptions** in logging code itself + +#### **Performance Considerations** +- JSON serialization adds minimal overhead (~1-2ms per log entry) +- Context variables are async-safe and thread-local +- Log rotation prevents disk space issues +- Structured queries are much faster than grep on large files + +### **Troubleshooting** + +#### **Common Issues** + +**Logs not appearing:** +- Check log level configuration in environment +- Verify logs/ directory permissions +- Ensure handlers are properly configured + +**JSON serialization errors:** +- Avoid logging complex objects directly +- Convert objects to strings or dicts before logging +- The JSONFormatter handles most common types automatically + +**Context not appearing in logs:** +- Ensure `set_discord_context()` is called before logging +- Context is tied to the current async task +- Check that context is not cleared prematurely + +**Performance issues:** +- Monitor log file sizes and rotation +- Consider reducing log level in production +- Use sampling for high-frequency operations + +--- + +## 🚀 Future Utilities + +Additional utility modules planned for future implementation: + +### **Discord Helpers** (Planned) +- Embed builders and formatters +- Permission checking decorators +- User mention and role utilities +- Message pagination helpers + +### **API Utilities** (Planned) +- Rate limiting decorators +- Response caching mechanisms +- Retry logic with exponential backoff +- Request validation helpers + +### **Data Processing** (Planned) +- CSV/JSON export utilities +- Statistical calculation helpers +- Date/time formatting for baseball seasons +- Text processing and search utilities + +### **Testing Utilities** (Planned) +- Mock Discord objects for testing +- Fixture generators for common test data +- Assertion helpers for Discord responses +- Test database setup and teardown + +--- + +## 📚 Usage Examples by Module + +### **Logging Integration in Commands** + +```python +# commands/teams/roster.py +from utils.logging import get_contextual_logger, set_discord_context + +class TeamRosterCommands(commands.Cog): + def __init__(self, bot): + self.bot = bot + self.logger = get_contextual_logger(f'{__name__}.TeamRosterCommands') + + @discord.app_commands.command(name="roster") + async def team_roster(self, interaction, team_name: str, season: int = None): + set_discord_context( + interaction=interaction, + command="/roster", + team_name=team_name, + season=season + ) + + trace_id = self.logger.start_operation("team_roster_command") + + try: + self.logger.info("Team roster command started") + + # Command implementation + team = await team_service.find_team(team_name) + self.logger.debug("Team found", team_id=team.id, team_abbreviation=team.abbrev) + + players = await team_service.get_roster(team.id, season) + self.logger.info("Roster retrieved", player_count=len(players)) + + # Create and send response + embed = create_roster_embed(team, players) + await interaction.followup.send(embed=embed) + + self.logger.info("Team roster command completed") + + except TeamNotFoundError as e: + self.logger.warning("Team not found", search_term=team_name) + await interaction.followup.send(f"❌ Team '{team_name}' not found", ephemeral=True) + + except Exception as e: + self.logger.error("Team roster command failed", error=e) + await interaction.followup.send("❌ Error retrieving team roster", ephemeral=True) +``` + +### **Service Layer Logging** + +```python +# services/team_service.py +from utils.logging import get_contextual_logger + +class TeamService(BaseService[Team]): + def __init__(self): + super().__init__(Team, 'teams') + self.logger = get_contextual_logger(f'{__name__}.TeamService') + + async def find_team(self, team_name: str) -> Team: + self.logger.debug("Starting team search", search_term=team_name) + + # Try exact match first + teams = await self.get_by_field('name', team_name) + if len(teams) == 1: + self.logger.debug("Exact team match found", team_id=teams[0].id) + return teams[0] + + # Try abbreviation match + teams = await self.get_by_field('abbrev', team_name.upper()) + if len(teams) == 1: + self.logger.debug("Team abbreviation match found", team_id=teams[0].id) + return teams[0] + + # Try fuzzy search + all_teams = await self.get_all_items() + matches = [t for t in all_teams if team_name.lower() in t.name.lower()] + + if len(matches) == 0: + self.logger.warning("No team matches found", search_term=team_name) + raise TeamNotFoundError(f"No team found matching '{team_name}'") + elif len(matches) > 1: + match_names = [t.name for t in matches] + self.logger.warning("Multiple team matches found", + search_term=team_name, + matches=match_names) + raise MultipleTeamsFoundError(f"Multiple teams found: {', '.join(match_names)}") + + self.logger.debug("Fuzzy team match found", team_id=matches[0].id) + return matches[0] +``` + +--- + +## 📁 File Structure + +``` +utils/ +├── README.md # This documentation +├── __init__.py # Package initialization +└── logging.py # Structured logging implementation + +# Future files: +├── discord_helpers.py # Discord utility functions +├── api_utils.py # API helper functions +├── data_processing.py # Data manipulation utilities +└── testing.py # Testing helper functions +``` + +--- + +**Last Updated:** Phase 2.1 - Structured Logging Implementation +**Next Update:** When additional utility modules are added + +For questions or improvements to the logging system, check the implementation in `utils/logging.py` or refer to the JSON log outputs in `logs/discord_bot_v2.json`. \ No newline at end of file diff --git a/utils/__init__.py b/utils/__init__.py new file mode 100644 index 0000000..d77dd99 --- /dev/null +++ b/utils/__init__.py @@ -0,0 +1,5 @@ +""" +Utility functions for Discord Bot v2.0 + +Helper functions, formatters, and validation utilities. +""" \ No newline at end of file diff --git a/utils/logging.py b/utils/logging.py new file mode 100644 index 0000000..8e69ac4 --- /dev/null +++ b/utils/logging.py @@ -0,0 +1,247 @@ +""" +Enhanced Logging Utilities + +Provides structured logging with contextual information for Discord bot debugging. +Implements hybrid approach: human-readable console + structured JSON files. +""" +import contextvars +import json +import logging +import time +import uuid +from datetime import datetime +from typing import Dict, Any, Optional, Union + +# Context variable for request tracking across async calls +log_context: contextvars.ContextVar[Dict[str, Any]] = contextvars.ContextVar('log_context', default={}) + +logger = logging.getLogger(f'{__name__}.logging_utils') + +JSONValue = Union[ + str, + int, + float, + bool, + None, + dict[str, Any], # nested object + list[Any] # arrays +] + + +class JSONFormatter(logging.Formatter): + """Custom JSON formatter for structured file logging.""" + + def format(self, record) -> str: + """Format log record as JSON with context information.""" + # Base log object + log_obj: dict[str, JSONValue] = { + 'timestamp': datetime.now().isoformat() + 'Z', + 'level': record.levelname, + 'logger': record.name, + 'message': record.getMessage() + } + + # Add function/line info if available + if hasattr(record, 'funcName') and record.funcName: + log_obj['function'] = record.funcName + if hasattr(record, 'lineno') and record.lineno: + log_obj['line'] = record.lineno + + # Add exception info if present + if record.exc_info: + log_obj['exception'] = { + 'type': record.exc_info[0].__name__ if record.exc_info[0] else 'Unknown', + 'message': str(record.exc_info[1]) if record.exc_info[1] else 'No message', + 'traceback': self.formatException(record.exc_info) + } + + # Add context from contextvars + context = log_context.get({}) + if context: + log_obj['context'] = context.copy() + + # Add custom fields from extra parameter + excluded_keys = { + 'name', 'msg', 'args', 'levelname', 'levelno', 'pathname', + 'filename', 'module', 'lineno', 'funcName', 'created', + 'msecs', 'relativeCreated', 'thread', 'threadName', + 'processName', 'process', 'getMessage', 'exc_info', + 'exc_text', 'stack_info' + } + + extra_data = {} + for key, value in record.__dict__.items(): + if key not in excluded_keys: + # Ensure JSON serializable + try: + json.dumps(value) + extra_data[key] = value + except (TypeError, ValueError): + extra_data[key] = str(value) + + if extra_data: + log_obj['extra'] = extra_data + + return json.dumps(log_obj, ensure_ascii=False) + + +class ContextualLogger: + """ + Logger wrapper that provides contextual information and structured logging. + + Automatically includes Discord context (user, guild, command) in all log messages. + """ + + def __init__(self, logger_name: str): + """ + Initialize contextual logger. + + Args: + logger_name: Name for the underlying logger + """ + self.logger = logging.getLogger(logger_name) + self._start_time: Optional[float] = None + + def start_operation(self, operation_name: Optional[str] = None) -> str: + """ + Start timing an operation and generate a trace ID. + + Args: + operation_name: Optional name for the operation being tracked + + Returns: + Generated trace ID for this operation + """ + self._start_time = time.time() + trace_id = str(uuid.uuid4())[:8] + + # Add trace_id to context + current_context = log_context.get({}) + current_context['trace_id'] = trace_id + if operation_name: + current_context['operation'] = operation_name + log_context.set(current_context) + + return trace_id + + def _get_duration_ms(self) -> Optional[int]: + """Get operation duration in milliseconds if start_operation was called.""" + if self._start_time: + return int((time.time() - self._start_time) * 1000) + return None + + def debug(self, message: str, **kwargs): + """Log debug message with context.""" + duration = self._get_duration_ms() + if duration is not None: + kwargs['duration_ms'] = duration + self.logger.debug(message, extra=kwargs) + + def info(self, message: str, **kwargs): + """Log info message with context.""" + duration = self._get_duration_ms() + if duration is not None: + kwargs['duration_ms'] = duration + self.logger.info(message, extra=kwargs) + + def warning(self, message: str, **kwargs): + """Log warning message with context.""" + duration = self._get_duration_ms() + if duration is not None: + kwargs['duration_ms'] = duration + self.logger.warning(message, extra=kwargs) + + def error(self, message: str, error: Optional[Exception] = None, **kwargs): + """ + Log error message with context and exception information. + + Args: + message: Error message + error: Optional exception object + **kwargs: Additional context + """ + duration = self._get_duration_ms() + if duration is not None: + kwargs['duration_ms'] = duration + + if error: + kwargs['error'] = { + 'type': type(error).__name__, + 'message': str(error) + } + self.logger.error(message, exc_info=True, extra=kwargs) + else: + self.logger.error(message, extra=kwargs) + + def exception(self, message: str, **kwargs): + """Log exception with full traceback and context.""" + duration = self._get_duration_ms() + if duration is not None: + kwargs['duration_ms'] = duration + self.logger.exception(message, extra=kwargs) + + +def set_discord_context( + interaction: Optional[Any] = None, + user_id: Optional[Union[str, int]] = None, + guild_id: Optional[Union[str, int]] = None, + channel_id: Optional[Union[str, int]] = None, + command: Optional[str] = None, + **additional_context +): + """ + Set Discord-specific context for logging. + + Args: + interaction: Discord interaction object (will extract user/guild/channel) + user_id: Discord user ID + guild_id: Discord guild ID + channel_id: Discord channel ID + command: Command name (e.g., '/player') + **additional_context: Any additional context to include + """ + context = log_context.get({}).copy() + + # Extract from interaction if provided + if interaction: + context['user_id'] = str(interaction.user.id) + if interaction.guild: + context['guild_id'] = str(interaction.guild.id) + context['guild_name'] = interaction.guild.name + if interaction.channel: + context['channel_id'] = str(interaction.channel.id) + if hasattr(interaction, 'command') and interaction.command: + context['command'] = f"/{interaction.command.name}" + + # Override with explicit parameters + if user_id: + context['user_id'] = str(user_id) + if guild_id: + context['guild_id'] = str(guild_id) + if channel_id: + context['channel_id'] = str(channel_id) + if command: + context['command'] = command + + # Add any additional context + context.update(additional_context) + + log_context.set(context) + + +def clear_context(): + """Clear the current logging context.""" + log_context.set({}) + + +def get_contextual_logger(logger_name: str) -> ContextualLogger: + """ + Get a contextual logger instance. + + Args: + logger_name: Name for the logger (typically __name__) + + Returns: + ContextualLogger instance + """ + return ContextualLogger(logger_name) \ No newline at end of file diff --git a/views/__init__.py b/views/__init__.py new file mode 100644 index 0000000..fa0d2fc --- /dev/null +++ b/views/__init__.py @@ -0,0 +1,5 @@ +""" +Discord UI components for Bot v2.0 + +Interactive views, buttons, modals, and select menus. +""" \ No newline at end of file