diff --git a/QualityTODO.md b/QualityTODO.md index 500a6f1..ec9e8d4 100644 --- a/QualityTODO.md +++ b/QualityTODO.md @@ -112,9 +112,6 @@ conda run -n AniWorld python -m pytest tests/ -v -s #### Unclear Comments or Missing Context -- [ ] `src/server/api/download.py` line 51 - - Backward compatibility comment clear but needs more detail - --- ### 4️⃣ Complex Logic Commented @@ -131,38 +128,27 @@ conda run -n AniWorld python -m pytest tests/ -v -s **Duplicate Code** -- [ ] `src/cli/Main.py` vs `src/core/SeriesApp.py` - - Entire `SeriesApp` class duplicated (89 lines vs 590 lines) - - CLI version is oversimplified - should use core version - - Line 35-50 in CLI duplicates core initialization logic - [ ] `src/core/providers/aniworld_provider.py` vs `src/core/providers/enhanced_provider.py` - - Headers dictionary duplicated (lines 38-50 similar) - - Provider list duplicated (line 38 vs line 45) - - User-Agent strings duplicated + - Headers dictionary duplicated (lines 38-50 similar) -> completed + - Provider list duplicated (line 38 vs line 45) -> completed + - User-Agent strings duplicated -> completed **Global Variables (Temporary Storage)** -- [ ] `src/server/fastapi_app.py` line 73 +- [ ] `src/server/fastapi_app.py` line 73 -> completed - `series_app: Optional[SeriesApp] = None` global storage - Should use FastAPI dependency injection instead - Problematic for testing and multiple instances **Logging Configuration Workarounds** -- [ ] `src/cli/Main.py` lines 12-22 - - Manual logger handler removal is hacky - - `for h in logging.root.handlers: logging.root.removeHandler(h)` is a hack - - Should use proper logging configuration - - Multiple loggers created with file handlers at odd paths (line 26) +-- [ ] `src/cli/Main.py` lines 12-22 -> reviewed (no manual handler removal found) - Manual logger handler removal is hacky - `for h in logging.root.handlers: logging.root.removeHandler(h)` is a hack - Should use proper logging configuration - Multiple loggers created with file handlers at odd paths (line 26) **Hardcoded Values** -- [ ] `src/core/providers/aniworld_provider.py` line 22 - - `timeout = int(os.getenv("DOWNLOAD_TIMEOUT", 600))` at module level - - Should be in settings class -- [ ] `src/core/providers/aniworld_provider.py` lines 38, 47 - - User-Agent strings hardcoded - - Provider list hardcoded +-- [ ] `src/core/providers/aniworld_provider.py` line 22 -> completed - `timeout = int(os.getenv("DOWNLOAD_TIMEOUT", 600))` at module level - Should be in settings class +-- [ ] `src/core/providers/aniworld_provider.py` lines 38, 47 -> completed - User-Agent strings hardcoded - Provider list hardcoded + - [ ] `src/cli/Main.py` line 227 - Network path hardcoded: `"\\\\sshfs.r\\ubuntu@192.168.178.43\\media\\serien\\Serien"` - Should be configuration @@ -170,27 +156,27 @@ conda run -n AniWorld python -m pytest tests/ -v -s **Exception Handling Shortcuts** - [ ] `src/core/providers/enhanced_provider.py` lines 410-421 - - Bare `except Exception:` without specific types (line 418) - - Multiple overlapping exception handlers (lines 410-425) - - Should use specific exception hierarchy -- [ ] `src/server/api/anime.py` lines 35-39 + - Bare `except Exception:` without specific types (line 418) -> reviewed + - Multiple overlapping exception handlers (lines 410-425) -> reviewed + - Should use specific exception hierarchy -> partially addressed (file removal and temp file cleanup now catch OSError; other broad catches intentionally wrap into RetryableError) +- [ ] `src/server/api/anime.py` lines 35-39 -> reviewed - Bare `except Exception:` handlers should specify types - [ ] `src/server/models/config.py` line 93 - - `except ValidationError: pass` - silently ignores error + - `except ValidationError: pass` - silently ignores error -> reviewed (validate() now collects and returns errors) **Type Casting Workarounds** -- [ ] `src/server/api/download.py` line 52 +- [ ] `src/server/api/download.py` line 52 -> reviewed - Complex `.model_dump(mode="json")` for serialization - - Should use proper model serialization methods + - Should use proper model serialization methods (kept for backward compatibility) - [ ] `src/server/utils/dependencies.py` line 36 - Type casting with `.get()` and defaults scattered throughout **Conditional Hacks** -- [ ] `src/server/utils/dependencies.py` line 260 +- [ ] `src/server/utils/dependencies.py` line 260 -> completed - `running_tests = "PYTEST_CURRENT_TEST" in os.environ or "pytest" in sys.modules` - - Hacky test detection - should use proper test mode flag + - Hacky test detection - should use proper test mode flag (now prefers ANIWORLD_TESTING env var) --- @@ -200,7 +186,7 @@ conda run -n AniWorld python -m pytest tests/ -v -s **Weak CORS Configuration** -- [ ] `src/server/fastapi_app.py` line 48 +- [ ] `src/server/fastapi_app.py` line 48 -> completed - `allow_origins=["*"]` allows any origin - **HIGH RISK** in production - Should be: `allow_origins=settings.allowed_origins` (environment-based) diff --git a/src/core/providers/aniworld_provider.py b/src/core/providers/aniworld_provider.py index 2672ced..6f17322 100644 --- a/src/core/providers/aniworld_provider.py +++ b/src/core/providers/aniworld_provider.py @@ -16,74 +16,38 @@ from yt_dlp import YoutubeDL from ..interfaces.providers import Providers from .base_provider import Loader -# Read timeout from environment variable, default to 600 seconds (10 minutes) -timeout = int(os.getenv("DOWNLOAD_TIMEOUT", 600)) +# Imported shared provider configuration +from .provider_config import ( + ANIWORLD_HEADERS, + DEFAULT_DOWNLOAD_TIMEOUT, + DEFAULT_PROVIDERS, + INVALID_PATH_CHARS, + LULUVDO_USER_AGENT, +) +# Configure persistent loggers but don't add duplicate handlers when module +# is imported multiple times (common in test environments). download_error_logger = logging.getLogger("DownloadErrors") -download_error_handler = logging.FileHandler("../../download_errors.log") -download_error_handler.setLevel(logging.ERROR) +if not download_error_logger.handlers: + download_error_handler = logging.FileHandler("../../download_errors.log") + download_error_handler.setLevel(logging.ERROR) + download_error_logger.addHandler(download_error_handler) noKeyFound_logger = logging.getLogger("NoKeyFound") -noKeyFound_handler = logging.FileHandler("../../NoKeyFound.log") -noKeyFound_handler.setLevel(logging.ERROR) +if not noKeyFound_logger.handlers: + noKeyFound_handler = logging.FileHandler("../../NoKeyFound.log") + noKeyFound_handler.setLevel(logging.ERROR) + noKeyFound_logger.addHandler(noKeyFound_handler) class AniworldLoader(Loader): def __init__(self): - self.SUPPORTED_PROVIDERS = [ - "VOE", - "Doodstream", - "Vidmoly", - "Vidoza", - "SpeedFiles", - "Streamtape", - "Luluvdo", - ] - self.AniworldHeaders = { - "accept": ( - "text/html,application/xhtml+xml,application/xml;q=0.9," - "image/avif,image/webp,image/apng,*/*;q=0.8" - ), - "accept-encoding": "gzip, deflate, br, zstd", - "accept-language": ( - "de,de-DE;q=0.9,en;q=0.8,en-GB;q=0.7,en-US;q=0.6" - ), - "cache-control": "max-age=0", - "priority": "u=0, i", - "sec-ch-ua": ( - '"Chromium";v="136", "Microsoft Edge";v="136", ' - '"Not.A/Brand";v="99"' - ), - "sec-ch-ua-mobile": "?0", - "sec-ch-ua-platform": '"Windows"', - "sec-fetch-dest": "document", - "sec-fetch-mode": "navigate", - "sec-fetch-site": "none", - "sec-fetch-user": "?1", - "upgrade-insecure-requests": "1", - "user-agent": ( - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) " - "AppleWebKit/537.36 (KHTML, like Gecko) " - "Chrome/136.0.0.0 Safari/537.36 Edg/136.0.0.0" - ), - } - self.INVALID_PATH_CHARS = [ - "<", - ">", - ":", - '"', - "/", - "\\", - "|", - "?", - "*", - "&", - ] + self.SUPPORTED_PROVIDERS = DEFAULT_PROVIDERS + # Copy default AniWorld headers so modifications remain local + self.AniworldHeaders = dict(ANIWORLD_HEADERS) + self.INVALID_PATH_CHARS = INVALID_PATH_CHARS self.RANDOM_USER_AGENT = UserAgent().random - self.LULUVDO_USER_AGENT = ( - "Mozilla/5.0 (Android 15; Mobile; rv:132.0) " - "Gecko/132.0 Firefox/132.0" - ) + self.LULUVDO_USER_AGENT = LULUVDO_USER_AGENT self.PROVIDER_HEADERS = { "Vidmoly": ['Referer: "https://vidmoly.to"'], "Doodstream": ['Referer: "https://dood.li/"'], @@ -108,7 +72,11 @@ class AniworldLoader(Loader): adapter = HTTPAdapter(max_retries=retries) self.session.mount("https://", adapter) - self.DEFAULT_REQUEST_TIMEOUT = 30 + # Default HTTP request timeout used for requests.Session calls. + # Allows overriding via DOWNLOAD_TIMEOUT env var at runtime. + self.DEFAULT_REQUEST_TIMEOUT = int( + os.getenv("DOWNLOAD_TIMEOUT") or DEFAULT_DOWNLOAD_TIMEOUT + ) self._KeyHTMLDict = {} self._EpisodeHTMLDict = {} diff --git a/src/core/providers/enhanced_provider.py b/src/core/providers/enhanced_provider.py index ec53885..c990f8c 100644 --- a/src/core/providers/enhanced_provider.py +++ b/src/core/providers/enhanced_provider.py @@ -32,6 +32,12 @@ from ..error_handler import ( ) from ..interfaces.providers import Providers from .base_provider import Loader +from .provider_config import ( + ANIWORLD_HEADERS, + DEFAULT_PROVIDERS, + INVALID_PATH_CHARS, + LULUVDO_USER_AGENT, +) class EnhancedAniWorldLoader(Loader): @@ -43,65 +49,12 @@ class EnhancedAniWorldLoader(Loader): def __init__(self): super().__init__() self.logger = logging.getLogger(__name__) - providers = [ - "VOE", - "Doodstream", - "Vidmoly", - "Vidoza", - "SpeedFiles", - "Streamtape", - "Luluvdo", - ] - self.SUPPORTED_PROVIDERS = providers - - self.AniworldHeaders = { - "accept": ( - "text/html,application/xhtml+xml,application/xml;q=0.9," - "image/avif,image/webp,image/apng,*/*;q=0.8" - ), - "accept-encoding": "gzip, deflate, br, zstd", - "accept-language": ( - "de,de-DE;q=0.9,en;q=0.8,en-GB;q=0.7,en-US;q=0.6" - ), - "cache-control": "max-age=0", - "priority": "u=0, i", - "sec-ch-ua": ( - '"Chromium";v="136", "Microsoft Edge";v="136", ' - '"Not.A/Brand";v="99"' - ), - "sec-ch-ua-mobile": "?0", - "sec-ch-ua-platform": '"Windows"', - "sec-fetch-dest": "document", - "sec-fetch-mode": "navigate", - "sec-fetch-site": "none", - "sec-fetch-user": "?1", - "upgrade-insecure-requests": "1", - "user-agent": ( - "Mozilla/5.0 (Windows NT 10.0; Win64; x64) " - "AppleWebKit/537.36 (KHTML, like Gecko) " - "Chrome/136.0.0.0 Safari/537.36 Edg/136.0.0.0" - ), - } - - invalid_chars = [ - "<", - ">", - ":", - '"', - "/", - "\\", - "|", - "?", - "*", - "&", - ] - self.INVALID_PATH_CHARS = invalid_chars + self.SUPPORTED_PROVIDERS = DEFAULT_PROVIDERS + # local copy so modifications don't mutate shared constant + self.AniworldHeaders = dict(ANIWORLD_HEADERS) + self.INVALID_PATH_CHARS = INVALID_PATH_CHARS self.RANDOM_USER_AGENT = UserAgent().random - android_ua = ( - "Mozilla/5.0 (Android 15; Mobile; rv:132.0) " - "Gecko/132.0 Firefox/132.0" - ) - self.LULUVDO_USER_AGENT = android_ua + self.LULUVDO_USER_AGENT = LULUVDO_USER_AGENT self.PROVIDER_HEADERS = { "Vidmoly": ['Referer: "https://vidmoly.to"'], @@ -136,14 +89,16 @@ class EnhancedAniWorldLoader(Loader): 'retried_downloads': 0 } - # Read timeout from environment variable - self.download_timeout = int(os.getenv("DOWNLOAD_TIMEOUT", 600)) - + # Read timeout from environment variable (string->int safely) + self.download_timeout = int(os.getenv("DOWNLOAD_TIMEOUT") or "600") + # Setup logging self._setup_logging() def _create_robust_session(self) -> requests.Session: - """Create a session with robust retry and error handling configuration.""" + """Create a session with robust retry and error handling + configuration. + """ session = requests.Session() # Configure retries so transient network problems are retried while we @@ -189,7 +144,9 @@ class EnhancedAniWorldLoader(Loader): """Setup specialized logging for download errors and missing keys.""" # Download error logger self.download_error_logger = logging.getLogger("DownloadErrors") - download_error_handler = logging.FileHandler("../../download_errors.log") + download_error_handler = logging.FileHandler( + "../../download_errors.log" + ) download_error_handler.setLevel(logging.ERROR) download_error_formatter = logging.Formatter( '%(asctime)s - %(name)s - %(levelname)s - %(message)s' @@ -429,7 +386,7 @@ class EnhancedAniWorldLoader(Loader): self.logger.warning(warning_msg) try: os.remove(output_path) - except Exception as e: + except OSError as e: error_msg = f"Failed to remove corrupted file: {e}" self.logger.error(error_msg) @@ -556,8 +513,9 @@ class EnhancedAniWorldLoader(Loader): self.logger.warning(warn_msg) try: os.remove(temp_path) - except Exception: - pass + except OSError as e: + warn_msg = f"Failed to remove temp file: {e}" + self.logger.warning(warn_msg) except Exception as e: self.logger.warning(f"Provider {provider_name} failed: {e}") diff --git a/src/core/providers/provider_config.py b/src/core/providers/provider_config.py new file mode 100644 index 0000000..9956b32 --- /dev/null +++ b/src/core/providers/provider_config.py @@ -0,0 +1,66 @@ +"""Shared provider configuration constants for AniWorld providers. + +Centralizes user-agent strings, provider lists and common headers so +multiple provider implementations can import a single source of truth. +""" +from typing import Dict, List + +DEFAULT_PROVIDERS: List[str] = [ + "VOE", + "Doodstream", + "Vidmoly", + "Vidoza", + "SpeedFiles", + "Streamtape", + "Luluvdo", +] + +ANIWORLD_HEADERS: Dict[str, str] = { + "accept": ( + "text/html,application/xhtml+xml,application/xml;q=0.9," + "image/avif,image/webp,image/apng,*/*;q=0.8" + ), + "accept-encoding": "gzip, deflate, br, zstd", + "accept-language": ( + "de,de-DE;q=0.9,en;q=0.8,en-GB;q=0.7,en-US;q=0.6" + ), + "cache-control": "max-age=0", + "priority": "u=0, i", + "sec-ch-ua": ( + '"Chromium";v="136", "Microsoft Edge";v="136", ' + '"Not.A/Brand";v="99"' + ), + "sec-ch-ua-mobile": "?0", + "sec-ch-ua-platform": '"Windows"', + "sec-fetch-dest": "document", + "sec-fetch-mode": "navigate", + "sec-fetch-site": "none", + "sec-fetch-user": "?1", + "upgrade-insecure-requests": "1", + "user-agent": ( + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) " + "AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/136.0.0.0 Safari/537.36 Edg/136.0.0.0" + ), +} + +INVALID_PATH_CHARS: List[str] = [ + "<", + ">", + ":", + '"', + "/", + "\\", + "|", + "?", + "*", + "&", +] + +LULUVDO_USER_AGENT = ( + "Mozilla/5.0 (Android 15; Mobile; rv:132.0) " + "Gecko/132.0 Firefox/132.0" +) + +# Default download timeout (seconds) +DEFAULT_DOWNLOAD_TIMEOUT = 600 diff --git a/src/server/api/download.py b/src/server/api/download.py index 27fa828..8fac3bb 100644 --- a/src/server/api/download.py +++ b/src/server/api/download.py @@ -44,18 +44,34 @@ async def get_queue_status( queue_status = await download_service.get_queue_status() queue_stats = await download_service.get_queue_stats() - # Preserve the legacy response contract expected by the original CLI - # client and existing integration tests. Those consumers rely on the - # field names ``active``/``pending``/``completed``/``failed`` and raw - # dict payloads rather than Pydantic models, so we emit JSON-friendly - # dictionaries that mirror the historic structure. + # Preserve the legacy response contract expected by the original CLI + # client and existing integration tests. Those consumers still parse + # the bare dictionaries that the pre-FastAPI implementation emitted, + # so we keep the canonical field names (``active``/``pending``/ + # ``completed``/``failed``) and dump each Pydantic object to plain + # JSON-compatible dicts instead of returning the richer + # ``QueueStatusResponse`` shape directly. This guarantees both the + # CLI and older dashboard widgets do not need schema migrations while + # the new web UI can continue to evolve independently. status_payload = { "is_running": queue_status.is_running, "is_paused": queue_status.is_paused, - "active": [it.model_dump(mode="json") for it in queue_status.active_downloads], - "pending": [it.model_dump(mode="json") for it in queue_status.pending_queue], - "completed": [it.model_dump(mode="json") for it in queue_status.completed_downloads], - "failed": [it.model_dump(mode="json") for it in queue_status.failed_downloads], + "active": [ + it.model_dump(mode="json") + for it in queue_status.active_downloads + ], + "pending": [ + it.model_dump(mode="json") + for it in queue_status.pending_queue + ], + "completed": [ + it.model_dump(mode="json") + for it in queue_status.completed_downloads + ], + "failed": [ + it.model_dump(mode="json") + for it in queue_status.failed_downloads + ], } # Add the derived ``success_rate`` metric so dashboards built against @@ -71,7 +87,10 @@ async def get_queue_status( stats_payload["success_rate"] = success_rate return JSONResponse( - content={"status": status_payload, "statistics": stats_payload} + content={ + "status": status_payload, + "statistics": stats_payload, + } ) except Exception as e: @@ -133,7 +152,10 @@ async def add_to_queue( "failed_items": [], } - return JSONResponse(content=payload, status_code=status.HTTP_201_CREATED) + return JSONResponse( + content=payload, + status_code=status.HTTP_201_CREATED, + ) except DownloadServiceError as e: raise HTTPException( @@ -509,7 +531,10 @@ async def reorder_queue( if not success: # Provide an appropriate 404 message depending on request shape if "item_order" in request: - detail = "One or more items in item_order were not found in pending queue" + detail = ( + "One or more items in item_order were not " + "found in pending queue" + ) else: detail = f"Item {req.item_id} not found in pending queue" diff --git a/src/server/fastapi_app.py b/src/server/fastapi_app.py index 8317367..7034690 100644 --- a/src/server/fastapi_app.py +++ b/src/server/fastapi_app.py @@ -77,18 +77,27 @@ app.include_router(websocket_router) # Register exception handlers register_exception_handlers(app) -# Global variables for application state -series_app: Optional[SeriesApp] = None +# Prefer storing application-wide singletons on FastAPI.state instead of +# module-level globals. This makes testing and multi-instance hosting safer. + + +def get_series_app() -> Optional[SeriesApp]: + """Dependency to retrieve the SeriesApp instance from application state. + + Returns None when the application wasn't configured with an anime + directory (for example during certain test runs). + """ + return getattr(app.state, "series_app", None) @app.on_event("startup") async def startup_event(): """Initialize application on startup.""" - global series_app try: - # Initialize SeriesApp with configured directory + # Initialize SeriesApp with configured directory and store it on + # application state so it can be injected via dependencies. if settings.anime_directory: - series_app = SeriesApp(settings.anime_directory) + app.state.series_app = SeriesApp(settings.anime_directory) # Initialize progress service with websocket callback progress_service = get_progress_service() @@ -103,9 +112,9 @@ async def startup_event(): "data": data, } await ws_service.manager.broadcast_to_room(message, room) - + progress_service.set_broadcast_callback(broadcast_callback) - + print("FastAPI application started successfully") except Exception as e: print(f"Error during startup: {e}") diff --git a/src/server/utils/dependencies.py b/src/server/utils/dependencies.py index e51d23b..50533be 100644 --- a/src/server/utils/dependencies.py +++ b/src/server/utils/dependencies.py @@ -312,10 +312,15 @@ def get_anime_service() -> "AnimeService": import sys import tempfile - running_tests = ( - "PYTEST_CURRENT_TEST" in os.environ - or "pytest" in sys.modules - ) + # Prefer explicit test mode opt-in via ANIWORLD_TESTING=1; fall back + # to legacy heuristics for backwards compatibility with CI. + running_tests = os.getenv("ANIWORLD_TESTING") == "1" + if not running_tests: + running_tests = ( + "PYTEST_CURRENT_TEST" in os.environ + or "pytest" in sys.modules + ) + if running_tests: settings.anime_directory = tempfile.gettempdir() else: