diff --git a/cwa_book_downloader/api/websocket.py b/cwa_book_downloader/api/websocket.py index c0e656f5..32b3f43d 100644 --- a/cwa_book_downloader/api/websocket.py +++ b/cwa_book_downloader/api/websocket.py @@ -28,29 +28,17 @@ class WebSocketManager: logger.info("WebSocket manager initialized") def register_on_first_connect(self, callback: Callable[[], None]): - """Register a callback to be called when the first client connects. - - This is useful for warming up resources (like the Cloudflare bypasser) - when a user starts using the web UI. - """ + """Register a callback for when the first client connects.""" self._on_first_connect_callbacks.append(callback) logger.debug(f"Registered on_first_connect callback: {callback.__name__}") def register_on_all_disconnect(self, callback: Callable[[], None]): - """Register a callback to be called when all clients disconnect. - - This can be used to trigger cleanup or resource release. - """ + """Register a callback for when all clients disconnect.""" self._on_all_disconnect_callbacks.append(callback) logger.debug(f"Registered on_all_disconnect callback: {callback.__name__}") def request_warmup_on_next_connect(self): - """Request that warmup callbacks be triggered on the next client connect. - - This is used when resources (like the Cloudflare bypasser) shut down due to - inactivity while clients are still connected. The next connect event should - trigger warmup even though it's not technically the "first" connection. - """ + """Request warmup callbacks on the next client connect (e.g., after idle shutdown).""" with self._connection_lock: self._needs_rewarm = True logger.debug("Warmup requested for next client connect") @@ -165,15 +153,7 @@ class WebSocketManager: message: str, phase: str = 'searching' ): - """Broadcast search status update for a release source search. - - Args: - source: Release source name (e.g., 'irc', 'direct_download') - provider: Metadata provider name (e.g., 'hardcover') - book_id: Book ID from the metadata provider - message: Human-readable status message - phase: Search phase ('connecting', 'searching', 'downloading', 'parsing', 'complete', 'error') - """ + """Broadcast search status update for a release source search.""" if not self.is_enabled(): return diff --git a/cwa_book_downloader/bypass/external_bypasser.py b/cwa_book_downloader/bypass/external_bypasser.py index 14d321ec..d11e5363 100644 --- a/cwa_book_downloader/bypass/external_bypasser.py +++ b/cwa_book_downloader/bypass/external_bypasser.py @@ -28,14 +28,7 @@ BACKOFF_CAP = 10.0 def _fetch_via_bypasser(target_url: str) -> Optional[str]: - """Make a single request to the external bypasser service. - - Args: - target_url: The URL to fetch through the bypasser - - Returns: - HTML content if successful, None otherwise - """ + """Make a single request to the external bypasser service. Returns HTML or None.""" bypasser_url = config.get("EXT_BYPASSER_URL", "http://flaresolverr:8191") bypasser_path = config.get("EXT_BYPASSER_PATH", "/v1") bypasser_timeout = config.get("EXT_BYPASSER_TIMEOUT", 60000) @@ -105,21 +98,7 @@ def get_bypassed_page( selector: Optional["network.AAMirrorSelector"] = None, cancel_flag: Optional[Event] = None ) -> Optional[str]: - """Fetch HTML content from a URL using an external Cloudflare bypasser service. - - Retries with exponential backoff and mirror/DNS rotation on failure. - - Args: - url: Target URL to fetch - selector: Mirror selector for AA URL rewriting and rotation - cancel_flag: Optional threading Event to signal cancellation - - Returns: - HTML content if successful, None otherwise - - Raises: - BypassCancelledException: If cancel_flag is set during operation - """ + """Fetch HTML via external bypasser with retries and mirror rotation.""" from cwa_book_downloader.download import network as network_module sel = selector or network_module.AAMirrorSelector() diff --git a/cwa_book_downloader/bypass/internal_bypasser.py b/cwa_book_downloader/bypass/internal_bypasser.py index d0747480..f341fc17 100644 --- a/cwa_book_downloader/bypass/internal_bypasser.py +++ b/cwa_book_downloader/bypass/internal_bypasser.py @@ -268,11 +268,7 @@ def _has_cloudflare_patterns(body: str, url: str) -> bool: return "cf-" in body or "cloudflare" in url.lower() or "/cdn-cgi/" in url def _detect_challenge_type(sb) -> str: - """Detect what type of challenge we're facing. - - Returns: - str: 'cloudflare', 'ddos_guard', or 'none' if no challenge detected - """ + """Detect challenge type: 'cloudflare', 'ddos_guard', or 'none'.""" try: title, body, current_url = _get_page_info(sb) diff --git a/cwa_book_downloader/config/env.py b/cwa_book_downloader/config/env.py index d9c91bcf..c2225d86 100644 --- a/cwa_book_downloader/config/env.py +++ b/cwa_book_downloader/config/env.py @@ -1,4 +1,4 @@ -"""Environment variable parsing. No local dependencies - import first.""" +"""Bootstrap environment variables. No local dependencies - import first.""" import json import os @@ -7,17 +7,12 @@ from pathlib import Path def string_to_bool(s: str) -> bool: + """Convert string to boolean.""" return s.lower() in ["true", "yes", "1", "y"] def _read_debug_from_config() -> bool: - """ - Read DEBUG setting directly from config JSON file. - - This is called at import time before the config singleton is available. - Priority: ENV var > config file > default (False) - """ - # Check env var first (takes priority) + """Read DEBUG from env var or config file (import-time safe).""" env_debug = os.environ.get("DEBUG") if env_debug is not None: return string_to_bool(env_debug) @@ -38,20 +33,18 @@ def _read_debug_from_config() -> bool: return False -# Authentication and session settings -SESSION_COOKIE_SECURE_ENV = os.getenv("SESSION_COOKIE_SECURE", "false") +def _is_sqlite_file(path: Path) -> bool: + """Check if a file is a valid SQLite database by reading magic bytes.""" + try: + with open(path, "rb") as f: + header = f.read(16) + return header[:16] == b"SQLite format 3\x00" + except (OSError, PermissionError): + return False + def _resolve_cwa_db_path() -> Path | None: - """ - Resolve the Calibre-Web database path. - - Priority: - 1. CWA_DB_PATH env var (backwards compatibility) - 2. Default path /auth/app.db if it exists and is a valid SQLite file - - Returns None if no valid database is found. - """ - # Check env var first (backwards compatibility) + """Resolve CWA database path from env var or default location.""" env_path = os.getenv("CWA_DB_PATH") if env_path: path = Path(env_path) @@ -66,99 +59,6 @@ def _resolve_cwa_db_path() -> Path | None: return None -def _is_sqlite_file(path: Path) -> bool: - """Check if a file is a valid SQLite database by reading magic bytes.""" - try: - with open(path, "rb") as f: - header = f.read(16) - return header[:16] == b"SQLite format 3\x00" - except (OSError, PermissionError): - return False - - -CWA_DB_PATH = _resolve_cwa_db_path() -CONFIG_DIR = Path(os.getenv("CONFIG_DIR", "/config")) -LOG_ROOT = Path(os.getenv("LOG_ROOT", "/var/log/")) -LOG_DIR = LOG_ROOT / "cwa-book-downloader" -TMP_DIR = Path(os.getenv("TMP_DIR", "/tmp/cwa-book-downloader")) -INGEST_DIR = Path(os.getenv("INGEST_DIR", "/cwa-book-ingest")) - -STATUS_TIMEOUT = int(os.getenv("STATUS_TIMEOUT", "3600")) -USE_BOOK_TITLE = string_to_bool(os.getenv("USE_BOOK_TITLE", "false")) -MAX_RETRY = int(os.getenv("MAX_RETRY", "10")) -DEFAULT_SLEEP = int(os.getenv("DEFAULT_SLEEP", "5")) -USE_CF_BYPASS = string_to_bool(os.getenv("USE_CF_BYPASS", "true")) -HTTP_PROXY = os.getenv("HTTP_PROXY", "").strip() -HTTPS_PROXY = os.getenv("HTTPS_PROXY", "").strip() -AA_DONATOR_KEY = os.getenv("AA_DONATOR_KEY", "").strip() -_AA_BASE_URL = os.getenv("AA_BASE_URL", "auto").strip() -_AA_ADDITIONAL_URLS = os.getenv("AA_ADDITIONAL_URLS", "").strip() -_SUPPORTED_FORMATS = os.getenv("SUPPORTED_FORMATS", "epub,mobi,azw3,fb2,djvu,cbz,cbr").lower() -_SUPPORTED_AUDIOBOOK_FORMATS = os.getenv("SUPPORTED_AUDIOBOOK_FORMATS", "m4b,mp3").lower() -_BOOK_LANGUAGE = os.getenv("BOOK_LANGUAGE", "en").lower() -_CUSTOM_SCRIPT = os.getenv("CUSTOM_SCRIPT", "").strip() -FLASK_HOST = os.getenv("FLASK_HOST", "0.0.0.0") -FLASK_PORT = int(os.getenv("FLASK_PORT", "8084")) -DEBUG = _read_debug_from_config() -# Debug: skip specific download sources for testing fallback chains -# Comma-separated values: aa-fast, aa-slow-nowait, aa-slow-wait, libgen, zlib, welib -_DEBUG_SKIP_SOURCES_RAW = os.getenv("DEBUG_SKIP_SOURCES", "").strip().lower() -DEBUG_SKIP_SOURCES = set(s.strip() for s in _DEBUG_SKIP_SOURCES_RAW.split(",") if s.strip()) - -# Legacy welib settings - replaced by SOURCE_PRIORITY OrderableListField -# Kept for migration: if set, used to build initial SOURCE_PRIORITY config -_LEGACY_PRIORITIZE_WELIB = string_to_bool(os.getenv("PRIORITIZE_WELIB", "false")) -_LEGACY_ALLOW_USE_WELIB = string_to_bool(os.getenv("ALLOW_USE_WELIB", "true")) - -# Version information from Docker build -BUILD_VERSION = os.getenv("BUILD_VERSION", "N/A") -RELEASE_VERSION = os.getenv("RELEASE_VERSION", "N/A") - -# Log level is derived from DEBUG - no separate LOG_LEVEL setting -LOG_LEVEL = "DEBUG" if DEBUG else "INFO" -ENABLE_LOGGING = string_to_bool(os.getenv("ENABLE_LOGGING", "true")) -MAIN_LOOP_SLEEP_TIME = int(os.getenv("MAIN_LOOP_SLEEP_TIME", "5")) -MAX_CONCURRENT_DOWNLOADS = int(os.getenv("MAX_CONCURRENT_DOWNLOADS", "3")) -DOWNLOAD_PROGRESS_UPDATE_INTERVAL = int(os.getenv("DOWNLOAD_PROGRESS_UPDATE_INTERVAL", "1")) -DOCKERMODE = string_to_bool(os.getenv("DOCKERMODE", "false")) -_CUSTOM_DNS = os.getenv("CUSTOM_DNS", "auto").strip() -USE_DOH = string_to_bool(os.getenv("USE_DOH", "true")) -BYPASS_RELEASE_INACTIVE_MIN = int(os.getenv("BYPASS_RELEASE_INACTIVE_MIN", "5")) -BYPASS_WARMUP_ON_CONNECT = string_to_bool(os.getenv("BYPASS_WARMUP_ON_CONNECT", "true")) - -# Logging settings -LOG_FILE = LOG_DIR / "cwa-book-downloader.log" - -USING_EXTERNAL_BYPASSER = string_to_bool(os.getenv("USING_EXTERNAL_BYPASSER", "false")) -if USING_EXTERNAL_BYPASSER: - EXT_BYPASSER_URL = os.getenv("EXT_BYPASSER_URL", "http://flaresolverr:8191").strip() - EXT_BYPASSER_PATH = os.getenv("EXT_BYPASSER_PATH", "/v1").strip() - EXT_BYPASSER_TIMEOUT = int(os.getenv("EXT_BYPASSER_TIMEOUT", "60000")) - -USING_TOR = string_to_bool(os.getenv("USING_TOR", "false")) -# If using Tor, we don't need to set custom DNS, use DOH, or proxy -if USING_TOR: - _CUSTOM_DNS = "" - USE_DOH = False - HTTP_PROXY = "" - HTTPS_PROXY = "" - -# Detect Tor variant (has tor binary installed) -TOR_VARIANT_AVAILABLE = shutil.which("tor") is not None - -# Calibre-Web URL for navigation button -CALIBRE_WEB_URL = os.getenv("CALIBRE_WEB_URL", "").strip() - -# Metadata provider settings (Stage 2) -# Set to "hardcover" or "openlibrary" to enable metadata-first search mode -METADATA_PROVIDER = os.getenv("METADATA_PROVIDER", "").strip().lower() -HARDCOVER_API_KEY = os.getenv("HARDCOVER_API_KEY", "").strip() - -# Cache TTL settings (in seconds) -METADATA_CACHE_SEARCH_TTL = int(os.getenv("METADATA_CACHE_SEARCH_TTL", "300")) # 5 minutes -METADATA_CACHE_BOOK_TTL = int(os.getenv("METADATA_CACHE_BOOK_TTL", "600")) # 10 minutes - -# Cover image cache settings def _is_config_dir_writable() -> bool: """Check if the config directory exists and is writable.""" try: @@ -173,20 +73,81 @@ def _is_config_dir_writable() -> bool: def is_covers_cache_enabled() -> bool: - """Check if cover caching is enabled (dynamic, respects settings changes). - - Cache is only enabled if: - 1. The COVERS_CACHE_ENABLED setting is true - 2. The config directory is writable - """ + """Check if cover caching is enabled (requires setting + writable config dir).""" from cwa_book_downloader.core.config import config setting_enabled = config.get("COVERS_CACHE_ENABLED", True) return setting_enabled and _is_config_dir_writable() -# Legacy static value - use is_covers_cache_enabled() for dynamic checks -_COVERS_CACHE_ENABLED_ENV = string_to_bool(os.getenv("COVERS_CACHE_ENABLED", "true")) -COVERS_CACHE_ENABLED = _COVERS_CACHE_ENABLED_ENV and _is_config_dir_writable() -COVERS_CACHE_DIR = CONFIG_DIR / "covers" -COVERS_CACHE_TTL = int(os.getenv("COVERS_CACHE_TTL", "0")) # 0 = forever (covers are static) -COVERS_CACHE_MAX_SIZE_MB = int(os.getenv("COVERS_CACHE_MAX_SIZE_MB", "500")) +# ============================================================================= +# Bootstrap paths - needed before settings registry is available +# ============================================================================= + +CONFIG_DIR = Path(os.getenv("CONFIG_DIR", "/config")) +LOG_ROOT = Path(os.getenv("LOG_ROOT", "/var/log/")) +LOG_DIR = LOG_ROOT / "cwa-book-downloader" +LOG_FILE = LOG_DIR / "cwa-book-downloader.log" +TMP_DIR = Path(os.getenv("TMP_DIR", "/tmp/cwa-book-downloader")) +INGEST_DIR = Path(os.getenv("INGEST_DIR", "/books")) + + +# ============================================================================= +# Logger configuration - needed before settings registry is available +# ============================================================================= + +DEBUG = _read_debug_from_config() +LOG_LEVEL = "DEBUG" if DEBUG else "INFO" +ENABLE_LOGGING = string_to_bool(os.getenv("ENABLE_LOGGING", "true")) + + +# ============================================================================= +# Flask configuration - needed before app starts +# ============================================================================= + +FLASK_HOST = os.getenv("FLASK_HOST", "0.0.0.0") +FLASK_PORT = int(os.getenv("FLASK_PORT", "8084")) + + +# ============================================================================= +# Authentication +# ============================================================================= + +SESSION_COOKIE_SECURE_ENV = os.getenv("SESSION_COOKIE_SECURE", "false") +CWA_DB_PATH = _resolve_cwa_db_path() + + +# ============================================================================= +# Version information from Docker build +# ============================================================================= + +BUILD_VERSION = os.getenv("BUILD_VERSION", "N/A") +RELEASE_VERSION = os.getenv("RELEASE_VERSION", "N/A") + + +# ============================================================================= +# Capability detection - runtime checks, not user-configurable +# ============================================================================= + +DOCKERMODE = string_to_bool(os.getenv("DOCKERMODE", "false")) +TOR_VARIANT_AVAILABLE = shutil.which("tor") is not None +USING_TOR = string_to_bool(os.getenv("USING_TOR", "false")) + + +# ============================================================================= +# Debug/development settings +# ============================================================================= + +# Debug: skip specific download sources for testing fallback chains +# Comma-separated values: aa-fast, aa-slow-nowait, aa-slow-wait, libgen, zlib, welib +_DEBUG_SKIP_SOURCES_RAW = os.getenv("DEBUG_SKIP_SOURCES", "").strip().lower() +DEBUG_SKIP_SOURCES = set(s.strip() for s in _DEBUG_SKIP_SOURCES_RAW.split(",") if s.strip()) + + +# ============================================================================= +# Legacy migration support - will be removed in future version +# ============================================================================= + +# Legacy welib settings - replaced by SOURCE_PRIORITY OrderableListField +# Kept for migration: if set, used to build initial SOURCE_PRIORITY config +_LEGACY_PRIORITIZE_WELIB = string_to_bool(os.getenv("PRIORITIZE_WELIB", "false")) +_LEGACY_ALLOW_USE_WELIB = string_to_bool(os.getenv("ALLOW_USE_WELIB", "true")) diff --git a/cwa_book_downloader/config/security.py b/cwa_book_downloader/config/security.py index 6f288e54..2f78618c 100644 --- a/cwa_book_downloader/config/security.py +++ b/cwa_book_downloader/config/security.py @@ -20,15 +20,14 @@ logger = setup_logger(__name__) def _clear_builtin_credentials() -> Dict[str, Any]: """Clear built-in credentials to allow public access.""" + import json + from cwa_book_downloader.core.settings_registry import _get_config_file_path, _ensure_config_dir + try: config = load_config_file("security") config.pop("BUILTIN_USERNAME", None) config.pop("BUILTIN_PASSWORD_HASH", None) - # Save the cleared config - from cwa_book_downloader.core.settings_registry import _get_config_file_path, _ensure_config_dir - import json - _ensure_config_dir("security") config_path = _get_config_file_path("security") with open(config_path, 'w') as f: @@ -102,9 +101,8 @@ def _on_save_security(values: Dict[str, Any]) -> Dict[str, Any]: def security_settings(): """Security and authentication settings.""" from cwa_book_downloader.config.env import CWA_DB_PATH - import os - cwa_db_available = CWA_DB_PATH and os.path.exists(CWA_DB_PATH) + cwa_db_available = CWA_DB_PATH is not None and CWA_DB_PATH.exists() fields = [ TextField( diff --git a/cwa_book_downloader/config/settings.py b/cwa_book_downloader/config/settings.py index 0ffe4cab..9f580546 100644 --- a/cwa_book_downloader/config/settings.py +++ b/cwa_book_downloader/config/settings.py @@ -9,23 +9,11 @@ from cwa_book_downloader.core.logger import setup_logger logger = setup_logger(__name__) -# Log configuration values at DEBUG level, filtering out module imports and functions -logger.debug("Environment configuration:") -for key, value in env.__dict__.items(): - # Skip private attributes, modules, types, and callables (functions) - if key.startswith('_'): - continue - if isinstance(value, type) or callable(value): - continue - # Don't log module objects (they have __name__ attribute) - if hasattr(value, '__name__') and hasattr(value, '__file__'): - continue - # Redact sensitive values - if key == "AA_DONATOR_KEY" and isinstance(value, str) and value.strip(): - value = "REDACTED" - if key == "HARDCOVER_API_KEY" and isinstance(value, str) and value.strip(): - value = "REDACTED" - logger.debug(f" {key}: {value}") +# Log bootstrap configuration values at DEBUG level +logger.debug("Bootstrap configuration:") +for key in ['CONFIG_DIR', 'LOG_DIR', 'TMP_DIR', 'INGEST_DIR', 'DEBUG', 'DOCKERMODE']: + if hasattr(env, key): + logger.debug(f" {key}: {getattr(env, key)}") # Load supported book languages from data file # Path is relative to the package root, not this file @@ -52,47 +40,20 @@ logger.debug(f"CROSS_FILE_SYSTEM: {CROSS_FILE_SYSTEM}") CUSTOM_DNS: list[str] = [] DOH_SERVER: str = "" -# Warn about external bypasser DNS limitations -if env.USING_EXTERNAL_BYPASSER and env.USE_CF_BYPASS: - logger.warning( - "Using external bypasser (FlareSolverr). Note: FlareSolverr uses its own DNS resolution, " - "not this application's custom DNS settings. If you experience DNS-related blocks, " - "configure DNS at the Docker/system level for your FlareSolverr container, " - "or consider using the internal bypasser which integrates with the app's DNS system." - ) +# Recording directory for debugging internal cloudflare bypasser +RECORDING_DIR = env.LOG_DIR / "recording" -# Anna's Archive settings -AA_BASE_URL = env._AA_BASE_URL -AA_AVAILABLE_URLS = ["https://annas-archive.org", "https://annas-archive.se", "https://annas-archive.li"] -AA_AVAILABLE_URLS.extend(env._AA_ADDITIONAL_URLS.split(",")) -AA_AVAILABLE_URLS = [url.strip() for url in AA_AVAILABLE_URLS if url.strip()] -# File format settings -SUPPORTED_FORMATS = env._SUPPORTED_FORMATS.split(",") -logger.debug(f"SUPPORTED_FORMATS: {SUPPORTED_FORMATS}") -SUPPORTED_AUDIOBOOK_FORMATS = env._SUPPORTED_AUDIOBOOK_FORMATS.split(",") -logger.debug(f"SUPPORTED_AUDIOBOOK_FORMATS: {SUPPORTED_AUDIOBOOK_FORMATS}") - -# Complex language processing logic kept in config.py -BOOK_LANGUAGE = env._BOOK_LANGUAGE.split(',') -BOOK_LANGUAGE = [l for l in BOOK_LANGUAGE if l in [lang['code'] for lang in _SUPPORTED_BOOK_LANGUAGE]] -if len(BOOK_LANGUAGE) == 0: - BOOK_LANGUAGE = ['en'] - -# Custom script settings with validation logic -CUSTOM_SCRIPT = env._CUSTOM_SCRIPT -if CUSTOM_SCRIPT: - if not os.path.exists(CUSTOM_SCRIPT): - logger.warn(f"CUSTOM_SCRIPT {CUSTOM_SCRIPT} does not exist") - CUSTOM_SCRIPT = "" - elif not os.access(CUSTOM_SCRIPT, os.X_OK): - logger.warn(f"CUSTOM_SCRIPT {CUSTOM_SCRIPT} is not executable") - CUSTOM_SCRIPT = "" - -# Debugging settings -if not env.USING_EXTERNAL_BYPASSER: - # Recording directory for debugging internal cloudflare bypasser - RECORDING_DIR = env.LOG_DIR / "recording" +def _log_external_bypasser_warning() -> None: + """Log warning about external bypasser DNS limitations (called after config is available).""" + from cwa_book_downloader.core.config import config + if config.get("USING_EXTERNAL_BYPASSER", False) and config.get("USE_CF_BYPASS", True): + logger.warning( + "Using external bypasser (FlareSolverr). Note: FlareSolverr uses its own DNS resolution, " + "not this application's custom DNS settings. If you experience DNS-related blocks, " + "configure DNS at the Docker/system level for your FlareSolverr container, " + "or consider using the internal bypasser which integrates with the app's DNS system." + ) from cwa_book_downloader.core.settings_registry import ( @@ -182,9 +143,7 @@ def _get_metadata_provider_options(): def _get_metadata_provider_options_with_none(): """Build metadata provider options with a 'Use main provider' option first.""" - options = [{"value": "", "label": "Use book provider"}] - options.extend(_get_metadata_provider_options()) - return options + return [{"value": "", "label": "Use book provider"}] + _get_metadata_provider_options() def _get_release_source_options(): @@ -492,7 +451,7 @@ def download_settings(): key="DESTINATION", label="Destination", description="Directory where downloaded files are saved.", - default="/cwa-book-ingest", + default="/books", required=True, ), SelectField( @@ -789,49 +748,49 @@ def download_source_settings(): TextField( key="AA_CONTENT_TYPE_DIR_FICTION", label="Fiction Books", - placeholder="/cwa-book-ingest/fiction", + placeholder="/books/fiction", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_NON_FICTION", label="Non-Fiction Books", - placeholder="/cwa-book-ingest/non-fiction", + placeholder="/books/non-fiction", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_UNKNOWN", label="Unknown Books", - placeholder="/cwa-book-ingest/unknown", + placeholder="/books/unknown", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_MAGAZINE", label="Magazines", - placeholder="/cwa-book-ingest/magazines", + placeholder="/books/magazines", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_COMIC", label="Comic Books", - placeholder="/cwa-book-ingest/comics", + placeholder="/books/comics", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_STANDARDS", label="Standards Documents", - placeholder="/cwa-book-ingest/standards", + placeholder="/books/standards", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_MUSICAL_SCORE", label="Musical Scores", - placeholder="/cwa-book-ingest/scores", + placeholder="/books/scores", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), TextField( key="AA_CONTENT_TYPE_DIR_OTHER", label="Other", - placeholder="/cwa-book-ingest/other", + placeholder="/books/other", show_when={"field": "AA_CONTENT_TYPE_ROUTING", "value": True}, ), ] diff --git a/cwa_book_downloader/core/cache.py b/cwa_book_downloader/core/cache.py index 58e91ee0..f36d8f57 100644 --- a/cwa_book_downloader/core/cache.py +++ b/cwa_book_downloader/core/cache.py @@ -24,24 +24,13 @@ class CacheService: """Thread-safe in-memory cache with TTL support.""" def __init__(self, max_size: int = 1000): - """Initialize cache service. - - Args: - max_size: Maximum number of entries before oldest are evicted. - """ + """Initialize cache with max_size entries before eviction.""" self._cache: Dict[str, CacheEntry] = {} self._lock = threading.Lock() self._max_size = max_size def get(self, key: str) -> Optional[Any]: - """Get cached value if not expired. - - Args: - key: Cache key to retrieve. - - Returns: - Cached value or None if not found/expired. - """ + """Get cached value if not expired.""" with self._lock: entry = self._cache.get(key) if entry is None: @@ -54,13 +43,7 @@ class CacheService: return entry.value def set(self, key: str, value: Any, ttl: int) -> None: - """Cache value with TTL. - - Args: - key: Cache key. - value: Value to cache. - ttl: Time to live in seconds. - """ + """Cache value with TTL in seconds.""" with self._lock: # Evict oldest entries if at capacity if len(self._cache) >= self._max_size: @@ -72,14 +55,7 @@ class CacheService: ) def invalidate(self, key: str) -> bool: - """Remove specific cache entry. - - Args: - key: Cache key to remove. - - Returns: - True if entry was removed, False if not found. - """ + """Remove specific cache entry. Returns True if found.""" with self._lock: if key in self._cache: del self._cache[key] @@ -92,11 +68,7 @@ class CacheService: self._cache.clear() def cleanup_expired(self) -> int: - """Remove all expired entries. - - Returns: - Number of entries removed. - """ + """Remove all expired entries. Returns count removed.""" with self._lock: now = time.time() expired_keys = [ @@ -108,10 +80,7 @@ class CacheService: return len(expired_keys) def _evict_oldest(self) -> None: - """Evict oldest entries (by expiration time) to make room. - - Called with lock held. - """ + """Evict ~10% of oldest entries. Called with lock held.""" if not self._cache: return @@ -126,11 +95,7 @@ class CacheService: del self._cache[key] def stats(self) -> Dict[str, int]: - """Get cache statistics. - - Returns: - Dict with size and max_size. - """ + """Get cache statistics (size, max_size).""" with self._lock: return { "size": len(self._cache), @@ -148,15 +113,7 @@ def get_metadata_cache() -> CacheService: def cache_key(*args, **kwargs) -> str: - """Generate cache key from arguments. - - Args: - *args: Positional arguments to include in key. - **kwargs: Keyword arguments to include in key. - - Returns: - String cache key. - """ + """Generate cache key from arguments.""" parts = [str(arg) for arg in args] parts.extend(f"{k}={v}" for k, v in sorted(kwargs.items())) return ":".join(parts) @@ -168,18 +125,7 @@ def cacheable( ttl_default: int = 300, key_prefix: str = "" ): - """Decorator for caching function results. - - Args: - ttl: Static time to live in seconds (use this OR ttl_key, not both). - ttl_key: Config key to read TTL from (e.g., "METADATA_CACHE_SEARCH_TTL"). - ttl_default: Default TTL if ttl_key not found in config. - key_prefix: Optional prefix for cache keys. - - Examples: - @cacheable(ttl=300, key_prefix="hardcover:search") # Static TTL - @cacheable(ttl_key="METADATA_CACHE_SEARCH_TTL", key_prefix="hardcover:search") # Dynamic TTL - """ + """Decorator for caching function results. Use ttl (static) or ttl_key (from config).""" def decorator(func: Callable[..., T]) -> Callable[..., T]: @wraps(func) def wrapper(*args, **kwargs) -> T: diff --git a/cwa_book_downloader/core/config.py b/cwa_book_downloader/core/config.py index dc1be74c..d2bd6135 100644 --- a/cwa_book_downloader/core/config.py +++ b/cwa_book_downloader/core/config.py @@ -65,11 +65,12 @@ class Config: def _load_settings(self) -> None: """Load all settings from the registry.""" - # Ensure all plugin settings are registered before loading - # This handles cases where config is accessed before plugins are imported + # Ensure all settings modules are imported before loading + # This handles cases where config is accessed before settings are registered try: - import cwa_book_downloader.release_sources # noqa: F401 - import cwa_book_downloader.metadata_providers # noqa: F401 + import cwa_book_downloader.config.settings # noqa: F401 - main app settings + import cwa_book_downloader.release_sources # noqa: F401 - plugin settings + import cwa_book_downloader.metadata_providers # noqa: F401 - plugin settings except ImportError: pass diff --git a/cwa_book_downloader/core/image_cache.py b/cwa_book_downloader/core/image_cache.py index e6e5f783..da496fe1 100644 --- a/cwa_book_downloader/core/image_cache.py +++ b/cwa_book_downloader/core/image_cache.py @@ -92,10 +92,13 @@ class ImageCacheService: def _load_index(self) -> None: """Load cache index from disk.""" + if not self.index_path.exists(): + self._index = {} + return + try: - if self.index_path.exists(): - with open(self.index_path, 'r') as f: - self._index = json.load(f) + with open(self.index_path, 'r') as f: + self._index = json.load(f) except (json.JSONDecodeError, IOError): self._index = {} @@ -179,9 +182,7 @@ class ImageCacheService: """Check if a cache entry is expired.""" if self.ttl_seconds == 0: return False - - cached_at = entry.get('cached_at', 0) - return (time.time() - cached_at) > self.ttl_seconds + return (time.time() - entry.get('cached_at', 0)) > self.ttl_seconds def _is_negative_expired(self, entry: Dict[str, Any]) -> bool: """Check if a negative cache entry is expired. @@ -193,12 +194,8 @@ class ImageCacheService: return False cached_at = entry.get('cached_at', 0) - - # Transient failures (timeouts, connection errors) use shorter TTL - if entry.get('transient', False): - return (time.time() - cached_at) > TRANSIENT_CACHE_TTL - - return (time.time() - cached_at) > NEGATIVE_CACHE_TTL + ttl = TRANSIENT_CACHE_TTL if entry.get('transient', False) else NEGATIVE_CACHE_TTL + return (time.time() - cached_at) > ttl def _calculate_total_size(self) -> int: """Calculate total size of cached images.""" @@ -255,14 +252,13 @@ class ImageCacheService: with self._lock: entry = self._index.get(cache_id) + # Try reloading from disk if not found (handles multiprocess case) if not entry: - # Try reloading from disk (handles multiprocess case) self._load_index() entry = self._index.get(cache_id) - - if not entry: - self._misses += 1 - return None + if not entry: + self._misses += 1 + return None # Check for negative cache (failed fetch) if entry.get('negative', False): @@ -526,10 +522,8 @@ class ImageCacheService: self.put_negative(cache_id, transient=True) return None except requests.exceptions.HTTPError as e: - if e.response is not None and e.response.status_code == 404: - self.put_negative(cache_id) - else: - self.put_negative(cache_id, transient=True) + is_404 = e.response is not None and e.response.status_code == 404 + self.put_negative(cache_id, transient=not is_404) return None except Exception: return None diff --git a/cwa_book_downloader/core/logger.py b/cwa_book_downloader/core/logger.py index fadd9ef2..3d86cd48 100644 --- a/cwa_book_downloader/core/logger.py +++ b/cwa_book_downloader/core/logger.py @@ -72,17 +72,7 @@ def setup_logger(name: str, log_file: Path = LOG_FILE) -> CustomLogger: # Create logger as CustomLogger instance logger = CustomLogger(name) - log_level = logging.INFO - if LOG_LEVEL == "DEBUG": - log_level = logging.DEBUG - elif LOG_LEVEL == "INFO": - log_level = logging.INFO - elif LOG_LEVEL == "WARNING": - log_level = logging.WARNING - elif LOG_LEVEL == "ERROR": - log_level = logging.ERROR - elif LOG_LEVEL == "CRITICAL": - log_level = logging.CRITICAL + log_level = getattr(logging, LOG_LEVEL, logging.INFO) logger.setLevel(log_level) formatter = logging.Formatter( diff --git a/cwa_book_downloader/core/models.py b/cwa_book_downloader/core/models.py index 2fb2924d..573c1fa4 100644 --- a/cwa_book_downloader/core/models.py +++ b/cwa_book_downloader/core/models.py @@ -146,12 +146,14 @@ class BookInfo: """ # Resolve format if needed if not self.format: - for url in (self.download_urls[0] if self.download_urls else None, fallback_url): - if url: - ext = url.split(".")[-1].lower() - if ext and len(ext) <= 5 and ext.isalnum(): - self.format = ext - break + urls = [self.download_urls[0]] if self.download_urls else [] + if fallback_url: + urls.append(fallback_url) + for url in urls: + ext = url.split(".")[-1].lower() + if ext and len(ext) <= 5 and ext.isalnum(): + self.format = ext + break return build_filename(self.title, self.author, self.year, self.format) diff --git a/cwa_book_downloader/core/naming.py b/cwa_book_downloader/core/naming.py index 496a5193..5d154998 100644 --- a/cwa_book_downloader/core/naming.py +++ b/cwa_book_downloader/core/naming.py @@ -32,20 +32,19 @@ def _sanitize(name: str, max_length: int = 245) -> str: def sanitize_filename(name: str, max_length: int = 245) -> str: - """Sanitize a string for use as a filename.""" + """Sanitize a string for use as a filename or path component.""" return _sanitize(name, max_length) -def sanitize_path_component(name: str, max_length: int = 245) -> str: - """Sanitize a string for use as a path component.""" - return _sanitize(name, max_length) +# Alias for backwards compatibility +sanitize_path_component = sanitize_filename def format_series_position(position: Optional[Union[int, float]]) -> str: if position is None: return "" - # Check if it's effectively an integer + # Display as integer if whole number if isinstance(position, float) and position.is_integer(): return str(int(position)) @@ -110,11 +109,7 @@ def parse_naming_template( return "" # Sanitize the value - # If suffix contains a slash, this is meant to be a folder component - if '/' in suffix: - value = sanitize_path_component(value) - else: - value = sanitize_filename(value) + value = sanitize_filename(value) return f"{prefix}{value}{suffix}" diff --git a/cwa_book_downloader/core/queue.py b/cwa_book_downloader/core/queue.py index faa93645..7f75d17f 100644 --- a/cwa_book_downloader/core/queue.py +++ b/cwa_book_downloader/core/queue.py @@ -12,11 +12,7 @@ from cwa_book_downloader.core.models import QueueStatus, QueueItem, DownloadTask class BookQueue: - """Thread-safe download queue manager with priority support and cancellation. - - Stores DownloadTask objects which are source-agnostic download descriptors. - Works with both Direct Download and Universal modes. - """ + """Thread-safe download queue manager with priority support and cancellation.""" def __init__(self) -> None: self._queue: queue.PriorityQueue[QueueItem] = queue.PriorityQueue() @@ -33,14 +29,7 @@ class BookQueue: return timedelta(seconds=app_config.get("STATUS_TIMEOUT", 3600)) def add(self, task: DownloadTask) -> bool: - """Add a download task to the queue. - - Args: - task: The download task to queue (includes task_id, priority, etc.) - - Returns: - True if added successfully, False if already exists - """ + """Add a download task to the queue. Returns False if already exists.""" with self._lock: task_id = task.task_id @@ -59,11 +48,7 @@ class BookQueue: return True def get_next(self) -> Optional[Tuple[str, Event]]: - """Get next task ID from queue with cancellation flag. - - Returns: - Tuple of (task_id, cancel_flag) or None if queue is empty - """ + """Get next task ID from queue with cancellation flag.""" # Use iterative approach to avoid stack overflow if many items are cancelled while True: try: @@ -85,14 +70,7 @@ class BookQueue: return None def get_task(self, task_id: str) -> Optional[DownloadTask]: - """Get a task by its ID. - - Args: - task_id: The task identifier - - Returns: - The DownloadTask if found, None otherwise - """ + """Get a task by its ID.""" with self._lock: return self._task_data.get(task_id) @@ -171,14 +149,7 @@ class BookQueue: return sorted(queue_items, key=lambda x: (x['priority'], x['added_time'])) def cancel_download(self, task_id: str) -> bool: - """Cancel a download or clear a completed/errored item. - - Args: - task_id: Task identifier to cancel or clear - - Returns: - bool: True if cancellation/clearing was successful - """ + """Cancel a download or clear a completed/errored item.""" with self._lock: current_status = self._status.get(task_id) @@ -205,15 +176,7 @@ class BookQueue: return False def set_priority(self, task_id: str, new_priority: int) -> bool: - """Change the priority of a queued task. - - Args: - task_id: Task identifier - new_priority: New priority level (lower = higher priority) - - Returns: - bool: True if priority was successfully changed - """ + """Change the priority of a queued task (lower = higher priority).""" with self._lock: if task_id not in self._status or self._status[task_id] != QueueStatus.QUEUED: return False @@ -245,14 +208,7 @@ class BookQueue: return found def reorder_queue(self, task_priorities: Dict[str, int]) -> bool: - """Bulk reorder queue by setting new priorities. - - Args: - task_priorities: Dict mapping task_id to new priority - - Returns: - bool: True if reordering was successful - """ + """Bulk reorder queue by mapping task_id to new priority.""" with self._lock: # Extract all items from queue all_items = [] @@ -283,39 +239,18 @@ class BookQueue: return list(self._active_downloads.keys()) def has_pending_work(self) -> bool: - """Check if there are any active downloads or queued items. - - This is useful for determining if the bypasser should stay active - even when the UI is closed. - - Returns: - bool: True if there are active downloads or queued items - """ + """Check if there are any active downloads or queued items.""" with self._lock: - # Check for active downloads if self._active_downloads: return True - - # Check for queued items (excluding cancelled ones) - for task_id, status in self._status.items(): - if status == QueueStatus.QUEUED: - return True - - return False + return any(status == QueueStatus.QUEUED for status in self._status.values()) def clear_completed(self) -> int: - """Remove all completed, errored, or cancelled tasks from tracking. - - Returns: - int: Number of tasks removed - """ + """Remove all completed, errored, or cancelled tasks from tracking.""" + terminal_statuses = {QueueStatus.COMPLETE, QueueStatus.DONE, QueueStatus.AVAILABLE, QueueStatus.ERROR, QueueStatus.CANCELLED} with self._lock: - to_remove = [] - for task_id, status in self._status.items(): - if status in [QueueStatus.COMPLETE, QueueStatus.DONE, QueueStatus.AVAILABLE, QueueStatus.ERROR, QueueStatus.CANCELLED]: - to_remove.append(task_id) + to_remove = [task_id for task_id, status in self._status.items() if status in terminal_statuses] - removed_count = len(to_remove) for task_id in to_remove: self._status.pop(task_id, None) self._status_timestamps.pop(task_id, None) @@ -323,14 +258,13 @@ class BookQueue: self._cancel_flags.pop(task_id, None) self._active_downloads.pop(task_id, None) - return removed_count + return len(to_remove) def refresh(self) -> None: """Remove any tasks that are done downloading or have stale status.""" + terminal_statuses = {QueueStatus.COMPLETE, QueueStatus.DONE, QueueStatus.ERROR, QueueStatus.AVAILABLE, QueueStatus.CANCELLED} with self._lock: current_time = datetime.now() - - # Create a list of items to remove to avoid modifying dict during iteration to_remove = [] for task_id, status in self._status.items(): @@ -338,28 +272,25 @@ class BookQueue: if not task: continue - path = task.download_path - if path and not Path(path).exists(): + # Clear stale download paths + if task.download_path and not Path(task.download_path).exists(): task.download_path = None - path = None - # Check for completed downloads - if status == QueueStatus.AVAILABLE: - if not path: - self._update_status(task_id, QueueStatus.DONE) + # Mark available downloads as done if file is gone + if status == QueueStatus.AVAILABLE and not task.download_path: + self._update_status(task_id, QueueStatus.DONE) # Check for stale status entries last_update = self._status_timestamps.get(task_id) if last_update and (current_time - last_update) > self._status_timeout: - if status in [QueueStatus.COMPLETE, QueueStatus.DONE, QueueStatus.ERROR, QueueStatus.AVAILABLE, QueueStatus.CANCELLED]: + if status in terminal_statuses: to_remove.append(task_id) # Remove stale entries for task_id in to_remove: - del self._status[task_id] - del self._status_timestamps[task_id] - if task_id in self._task_data: - del self._task_data[task_id] + self._status.pop(task_id, None) + self._status_timestamps.pop(task_id, None) + self._task_data.pop(task_id, None) # Global instance of BookQueue book_queue = BookQueue() diff --git a/cwa_book_downloader/core/settings_registry.py b/cwa_book_downloader/core/settings_registry.py index 969f2b7c..5a86fb31 100644 --- a/cwa_book_downloader/core/settings_registry.py +++ b/cwa_book_downloader/core/settings_registry.py @@ -240,9 +240,7 @@ def _get_config_file_path(tab_name: str) -> Path: # Core settings tabs share the main settings.json file if tab_name in ("general", "search_mode"): return config_dir / "settings.json" - else: - plugins_dir = config_dir / "plugins" - return plugins_dir / f"{tab_name}.json" + return config_dir / "plugins" / f"{tab_name}.json" def _ensure_config_dir(tab_name: str) -> None: @@ -463,12 +461,9 @@ def is_value_from_env(field: SettingsField) -> bool: if isinstance(field, (ActionButton, HeadingField)): return False # UI-only settings never come from ENV (env_supported=False) - # Default to True for backwards compatibility - env_supported = getattr(field, 'env_supported', True) - if env_supported is False: + if not getattr(field, 'env_supported', True): return False - env_var_name = field.get_env_var_name() - return env_var_name in os.environ + return field.get_env_var_name() in os.environ def serialize_field(field: SettingsField, tab_name: str, include_value: bool = True) -> Dict[str, Any]: @@ -511,19 +506,12 @@ def serialize_field(field: SettingsField, tab_name: str, include_value: bool = T "requiresRestart": getattr(field, 'requires_restart', False), } - # Add conditional visibility if specified - show_when = getattr(field, 'show_when', None) - if show_when: - result["showWhen"] = show_when - - # Add conditional disable if specified - disabled_when = getattr(field, 'disabled_when', None) - if disabled_when: - result["disabledWhen"] = disabled_when - - # Add universal_only flag if set - universal_only = getattr(field, 'universal_only', False) - if universal_only: + # Add optional properties if set + if getattr(field, 'show_when', None): + result["showWhen"] = field.show_when + if getattr(field, 'disabled_when', None): + result["disabledWhen"] = field.disabled_when + if getattr(field, 'universal_only', False): result["universalOnly"] = True # Add type-specific properties diff --git a/cwa_book_downloader/core/utils.py b/cwa_book_downloader/core/utils.py index 7ae4a363..6a18e2bb 100644 --- a/cwa_book_downloader/core/utils.py +++ b/cwa_book_downloader/core/utils.py @@ -1,14 +1,15 @@ -""" -Shared utility functions for the CWA Book Downloader. - -Provides common helper functions used across the application. -""" +"""Shared utility functions for the CWA Book Downloader.""" import base64 from pathlib import Path from typing import Optional +def is_audiobook(content_type: Optional[str]) -> bool: + """Check if content type indicates an audiobook.""" + return bool(content_type and "audiobook" in content_type.lower()) + + CONTENT_TYPES = [ "book (fiction)", "book (non-fiction)", @@ -50,14 +51,7 @@ _LEGACY_CONTENT_TYPE_TO_CONFIG_KEY = { def get_destination(is_audiobook: bool = False) -> Path: - """Get the base destination directory. - - Args: - is_audiobook: If True, returns audiobook destination (with fallback to books destination) - - Returns: - Path to the destination directory - """ + """Get base destination directory. Audiobooks fall back to main destination.""" from cwa_book_downloader.core.config import config if is_audiobook: @@ -68,63 +62,40 @@ def get_destination(is_audiobook: bool = False) -> Path: # Main destination (also fallback for audiobooks) # Check new setting first, then legacy INGEST_DIR - destination = config.get("DESTINATION", "") or config.get("INGEST_DIR", "/cwa-book-ingest") + destination = config.get("DESTINATION", "") or config.get("INGEST_DIR", "/books") return Path(destination) def get_aa_content_type_dir(content_type: Optional[str] = None) -> Optional[Path]: - """Get override directory for Anna's Archive content-type routing. - - Only returns a path if AA_CONTENT_TYPE_ROUTING is enabled AND - a custom directory is configured for the given content type. - - Args: - content_type: The AA content type (e.g., "book (fiction)", "magazine") - - Returns: - Path to the override directory if configured, None otherwise - """ + """Get override directory for AA content-type routing if configured.""" from cwa_book_downloader.core.config import config - # Check if content-type routing is enabled - if not config.get("AA_CONTENT_TYPE_ROUTING", False): - # Also check legacy setting for backwards compatibility - if not config.get("USE_CONTENT_TYPE_DIRECTORIES", False): - return None + # Check if content-type routing is enabled (new or legacy setting) + if not config.get("AA_CONTENT_TYPE_ROUTING", False) and not config.get("USE_CONTENT_TYPE_DIRECTORIES", False): + return None if not content_type: return None - # Normalize content type for lookup content_type_lower = content_type.lower().strip() - # Try new AA-specific config keys first - config_key = _AA_CONTENT_TYPE_TO_CONFIG_KEY.get(content_type_lower) - if config_key: - custom_dir = config.get(config_key, "") - if custom_dir: - return Path(custom_dir) - - # Fall back to legacy config keys for backwards compatibility - legacy_key = _LEGACY_CONTENT_TYPE_TO_CONFIG_KEY.get(content_type_lower) - if legacy_key: - custom_dir = config.get(legacy_key, "") - if custom_dir: - return Path(custom_dir) + # Try new AA-specific config keys first, then legacy keys + for mapping in (_AA_CONTENT_TYPE_TO_CONFIG_KEY, _LEGACY_CONTENT_TYPE_TO_CONFIG_KEY): + config_key = mapping.get(content_type_lower) + if config_key: + custom_dir = config.get(config_key, "") + if custom_dir: + return Path(custom_dir) return None def get_ingest_dir(content_type: Optional[str] = None) -> Path: - """Get the ingest directory for a content type, falling back to default. - - DEPRECATED: Use get_destination() and get_aa_content_type_dir() instead. - Kept for backwards compatibility during migration. - """ + """DEPRECATED: Use get_destination() and get_aa_content_type_dir() instead.""" from cwa_book_downloader.core.config import config # Check new DESTINATION setting first, then legacy INGEST_DIR - default_ingest_dir = Path(config.get("DESTINATION", "") or config.get("INGEST_DIR", "/cwa-book-ingest")) + default_ingest_dir = Path(config.get("DESTINATION", "") or config.get("INGEST_DIR", "/books")) if not content_type: return default_ingest_dir @@ -138,20 +109,7 @@ def get_ingest_dir(content_type: Optional[str] = None) -> Path: def transform_cover_url(cover_url: Optional[str], cache_id: str) -> Optional[str]: - """ - Transform an external cover URL to a local proxy URL when caching is enabled. - - When cover caching is enabled, external cover image URLs are transformed - to local proxy URLs that cache the images on first access. This reduces - external requests and provides a consistent caching layer. - - Args: - cover_url: Original cover URL (external or already local) - cache_id: Unique identifier for the cache entry (e.g., "provider_bookid") - - Returns: - Transformed URL if caching enabled and URL is external, otherwise original URL - """ + """Transform external cover URL to local proxy URL when caching is enabled.""" if not cover_url: return cover_url diff --git a/cwa_book_downloader/download/archive.py b/cwa_book_downloader/download/archive.py index 6b90f7a7..282e90f9 100644 --- a/cwa_book_downloader/download/archive.py +++ b/cwa_book_downloader/download/archive.py @@ -10,6 +10,8 @@ from typing import List, Optional, Tuple from cwa_book_downloader.core.logger import setup_logger from cwa_book_downloader.core.config import config from cwa_book_downloader.core.naming import parse_naming_template, sanitize_filename +from cwa_book_downloader.core.utils import is_audiobook as check_audiobook +from cwa_book_downloader.download.fs import atomic_write, atomic_move logger = setup_logger(__name__) @@ -76,6 +78,8 @@ def _get_template(is_audiobook: bool, organization_mode: str) -> str: template = config.get(legacy_key, "") if not template: + if organization_mode == "organize": + return "{Author}/{Title} ({Year})" return "{Author} - {Title} ({Year})" return template @@ -83,8 +87,7 @@ def _get_template(is_audiobook: bool, organization_mode: str) -> str: def _build_filename_from_task(task, extension: str, organization_mode: str) -> str: """Build a filename from task metadata using the configured template.""" - content_type = task.content_type.lower() if task.content_type else "" - is_audiobook = "audiobook" in content_type + is_audiobook = check_audiobook(task.content_type) template = _get_template(is_audiobook, organization_mode) metadata = { @@ -138,7 +141,7 @@ def is_archive(file_path: Path) -> bool: def _is_supported_file(file_path: Path, content_type: Optional[str] = None) -> bool: """Check if file matches user's supported formats setting based on content type.""" ext = file_path.suffix.lower().lstrip(".") - if content_type and content_type.lower() == "audiobook": + if check_audiobook(content_type): supported_formats = _get_supported_audiobook_formats() else: supported_formats = _get_supported_formats() @@ -156,19 +159,8 @@ def _filter_files( extracted_files: List[Path], content_type: Optional[str] = None, ) -> Tuple[List[Path], List[Path], List[Path]]: - """ - Filter extracted files based on content type. - - For audiobooks: filters to audio formats using SUPPORTED_AUDIOBOOK_FORMATS - For books: filters to book formats using SUPPORTED_FORMATS - - Returns: - Tuple of (matched_files, rejected_format_files, other_files) - - matched_files: Match user's supported formats for this content type - - rejected_format_files: Valid formats for this type but not enabled by user - - other_files: Unrelated files (images, html, etc) - """ - is_audiobook = content_type and content_type.lower() == "audiobook" + """Filter files by content type. Returns (matched, rejected_format, other).""" + is_audiobook = check_audiobook(content_type) known_extensions = ALL_AUDIO_EXTENSIONS if is_audiobook else ALL_EBOOK_EXTENSIONS matched_files = [] @@ -191,30 +183,7 @@ def extract_archive( output_dir: Path, content_type: Optional[str] = None, ) -> Tuple[List[Path], List[str], List[Path]]: - """ - Extract files from an archive based on content type. - - Extracts all files, then filters based on content type: - - Audiobooks: keeps files matching SUPPORTED_AUDIOBOOK_FORMATS - - Books: keeps files matching SUPPORTED_FORMATS - Non-matching files (HTML, images, etc.) are deleted. - - Args: - archive_path: Path to the archive file - output_dir: Directory to extract files to - content_type: Content type (e.g., "audiobook") to determine which formats to keep - - Returns: - Tuple of (matched_files, warnings, rejected_files) - - matched_files: Paths to extracted files matching supported formats - - warnings: List of warning messages - - rejected_files: Files that were rejected (format not enabled) - - Raises: - ArchiveExtractionError: If extraction fails - PasswordProtectedError: If archive requires password - CorruptedArchiveError: If archive is corrupted - """ + """Extract archive and filter by content type. Returns (matched, warnings, rejected).""" suffix = archive_path.suffix.lower().lstrip(".") if suffix == "zip": @@ -224,7 +193,7 @@ def extract_archive( else: raise ArchiveExtractionError(f"Unsupported archive format: {suffix}") - is_audiobook = content_type and content_type.lower() == "audiobook" + is_audiobook = check_audiobook(content_type) file_type_label = "audiobook" if is_audiobook else "book" # Filter files based on content type @@ -256,14 +225,46 @@ def extract_archive( return matched_files, warnings, rejected_files -def _extract_zip( - archive_path: Path, - output_dir: Path, -) -> Tuple[List[Path], List[str]]: - """Extract files from a ZIP archive.""" +def _extract_files_from_archive(archive, output_dir: Path) -> List[Path]: + """Extract files from ZipFile or RarFile to output_dir with security checks.""" extracted_files = [] - warnings = [] + for info in archive.infolist(): + if info.is_dir(): + continue + + # Use only filename, strip directory path (security: prevent path traversal) + filename = Path(info.filename).name + if not filename: + continue + + # Security: reject filenames with null bytes or path separators + # Check both / and \ since archives may be created on different OSes + if "\x00" in filename or "/" in filename or "\\" in filename: + logger.warning(f"Skipping suspicious filename in archive: {info.filename!r}") + continue + + # Extract to output_dir with flat structure + target_path = output_dir / filename + + # Security: verify resolved path stays within output directory (defense-in-depth) + try: + target_path.resolve().relative_to(output_dir.resolve()) + except ValueError: + logger.warning(f"Path traversal attempt blocked: {info.filename!r}") + continue + + with archive.open(info) as src: + data = src.read() + final_path = atomic_write(target_path, data) + extracted_files.append(final_path) + logger.debug(f"Extracted: {filename}") + + return extracted_files + + +def _extract_zip(archive_path: Path, output_dir: Path) -> Tuple[List[Path], List[str]]: + """Extract files from a ZIP archive.""" try: with zipfile.ZipFile(archive_path, "r") as zf: # Check for password protection @@ -276,45 +277,19 @@ def _extract_zip( if bad_file: raise CorruptedArchiveError(f"Corrupted file in archive: {bad_file}") - # Extract all files - for info in zf.infolist(): - if info.is_dir(): - continue - - # Use only filename, strip directory path (security: prevent path traversal) - filename = Path(info.filename).name - if not filename: - continue - - # Extract to output_dir with flat structure - target_path = output_dir / filename - target_path = _handle_duplicate_filename(target_path) - - with zf.open(info) as src, open(target_path, "wb") as dst: - dst.write(src.read()) - - extracted_files.append(target_path) - logger.debug(f"Extracted: {filename}") + return _extract_files_from_archive(zf, output_dir), [] except zipfile.BadZipFile as e: raise CorruptedArchiveError(f"Invalid or corrupted ZIP: {e}") except PermissionError as e: raise ArchiveExtractionError(f"Permission denied: {e}") - return extracted_files, warnings - -def _extract_rar( - archive_path: Path, - output_dir: Path, -) -> Tuple[List[Path], List[str]]: +def _extract_rar(archive_path: Path, output_dir: Path) -> Tuple[List[Path], List[str]]: """Extract files from a RAR archive.""" if not RAR_AVAILABLE: raise ArchiveExtractionError("RAR extraction not available - rarfile library not installed") - extracted_files = [] - warnings = [] - try: with rarfile.RarFile(archive_path, "r") as rf: # Check for password protection @@ -324,25 +299,7 @@ def _extract_rar( # Test archive integrity rf.testrar() - # Extract all files - for info in rf.infolist(): - if info.is_dir(): - continue - - # Use only filename, strip directory path (security: prevent path traversal) - filename = Path(info.filename).name - if not filename: - continue - - # Extract to output_dir with flat structure - target_path = output_dir / filename - target_path = _handle_duplicate_filename(target_path) - - with rf.open(info) as src, open(target_path, "wb") as dst: - dst.write(src.read()) - - extracted_files.append(target_path) - logger.debug(f"Extracted: {filename}") + return _extract_files_from_archive(rf, output_dir), [] except rarfile.BadRarFile as e: raise CorruptedArchiveError(f"Invalid or corrupted RAR: {e}") @@ -351,25 +308,6 @@ def _extract_rar( except PermissionError as e: raise ArchiveExtractionError(f"Permission denied: {e}") - return extracted_files, warnings - - -def _handle_duplicate_filename(target_path: Path) -> Path: - """Handle duplicate filenames by appending counter.""" - if not target_path.exists(): - return target_path - - base = target_path.stem - ext = target_path.suffix - parent = target_path.parent - counter = 1 - - while target_path.exists(): - target_path = parent / f"{base}_{counter}{ext}" - counter += 1 - - return target_path - @dataclass class ArchiveResult: @@ -388,27 +326,10 @@ def process_archive( archive_id: str, task: Optional["DownloadTask"] = None, ) -> ArchiveResult: - """ - Process an archive file: extract, filter to supported files, move to ingest. - - This is the main entry point for archive handling, usable by any download handler. - Filters files based on content type: - - Audiobooks: keeps files matching SUPPORTED_AUDIOBOOK_FORMATS - - Books: keeps files matching SUPPORTED_FORMATS - - Args: - archive_path: Path to the downloaded archive file - temp_dir: Base temp directory for extraction (e.g., TMP_DIR) - ingest_dir: Final destination directory for files - archive_id: Unique identifier for temp directory naming - task: Optional download task for filename generation and content type - - Returns: - ArchiveResult with success status, final paths, and status message - """ + """Extract archive, filter to supported formats, move to ingest directory.""" extract_dir = temp_dir / f"extract_{archive_id}" content_type = task.content_type if task else None - is_audiobook = content_type and content_type.lower() == "audiobook" + is_audiobook = check_audiobook(content_type) file_type_label = "audiobook" if is_audiobook else "book" try: @@ -456,7 +377,7 @@ def process_archive( final_paths = [] # Determine file organization mode - is_audiobook = task and task.content_type and "audiobook" in task.content_type.lower() + is_audiobook = check_audiobook(task.content_type) if task else False organization_mode = _get_file_organization(is_audiobook) if task else "none" for extracted_file in extracted_files: @@ -472,9 +393,8 @@ def process_archive( else: filename = extracted_file.name - final_path = ingest_dir / filename - final_path = _handle_duplicate_filename(final_path) - shutil.move(str(extracted_file), str(final_path)) + dest_path = ingest_dir / filename + final_path = atomic_move(extracted_file, dest_path) final_paths.append(final_path) logger.debug(f"Moved to ingest: {final_path.name}") diff --git a/cwa_book_downloader/download/fs.py b/cwa_book_downloader/download/fs.py new file mode 100644 index 00000000..03605080 --- /dev/null +++ b/cwa_book_downloader/download/fs.py @@ -0,0 +1,191 @@ +"""Atomic filesystem operations for concurrent-safe file handling. + +These utilities handle file collisions atomically, avoiding TOCTOU race conditions +when multiple workers may try to write to the same path simultaneously. +""" + +import errno +import os +import shutil +from pathlib import Path + +from cwa_book_downloader.core.logger import setup_logger + +logger = setup_logger(__name__) + + +def atomic_write(dest_path: Path, data: bytes, max_attempts: int = 100) -> Path: + """Write data to a file with atomic collision detection. + + If the destination already exists, retries with counter suffix (_1, _2, etc.) + until a unique path is found. + + Args: + dest_path: Desired destination path + data: Bytes to write + max_attempts: Maximum collision retries before raising error + + Returns: + Path where file was actually written (may differ from dest_path) + + Raises: + RuntimeError: If no unique path found after max_attempts + """ + base = dest_path.stem + ext = dest_path.suffix + parent = dest_path.parent + + for attempt in range(max_attempts): + try_path = dest_path if attempt == 0 else parent / f"{base}_{attempt}{ext}" + try: + # O_CREAT | O_EXCL fails atomically if file exists + fd = os.open(str(try_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + try: + os.write(fd, data) + finally: + os.close(fd) + if attempt > 0: + logger.info(f"File collision resolved: {try_path.name}") + return try_path + except FileExistsError: + continue + + raise RuntimeError(f"Could not write file after {max_attempts} attempts: {dest_path}") + + +def atomic_move(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: + """Move a file with collision detection. + + Uses os.rename() for same-filesystem moves (atomic, triggers inotify events), + falls back to exclusive create + shutil.move for cross-filesystem moves. + + Note: We use os.rename() instead of hardlink+unlink because os.rename() + triggers proper inotify IN_MOVED_TO events that file watchers (like Calibre's + auto-add) rely on to detect new files. + + Args: + source_path: Source file to move + dest_path: Desired destination path + max_attempts: Maximum collision retries before raising error + + Returns: + Path where file was actually moved (may differ from dest_path) + + Raises: + RuntimeError: If no unique path found after max_attempts + """ + base = dest_path.stem + ext = dest_path.suffix + parent = dest_path.parent + + for attempt in range(max_attempts): + try_path = dest_path if attempt == 0 else parent / f"{base}_{attempt}{ext}" + + # Check for existing file (os.rename would overwrite on Unix) + if try_path.exists(): + continue + + try: + # os.rename is atomic on same filesystem and triggers inotify events + os.rename(str(source_path), str(try_path)) + if attempt > 0: + logger.info(f"File collision resolved: {try_path.name}") + return try_path + except FileExistsError: + # Race condition: file created between exists() check and rename() + continue + except OSError as e: + # Cross-filesystem - fall back to exclusive create + move + if e.errno != errno.EXDEV: + raise + try: + fd = os.open(str(try_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.close(fd) + try: + shutil.move(str(source_path), str(try_path)) + if attempt > 0: + logger.info(f"File collision resolved: {try_path.name}") + return try_path + except Exception: + try_path.unlink(missing_ok=True) + raise + except FileExistsError: + continue + + raise RuntimeError(f"Could not move file after {max_attempts} attempts: {dest_path}") + + +def atomic_hardlink(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: + """Create a hardlink with atomic collision detection. + + Args: + source_path: Source file to link from + dest_path: Desired destination path for the link + max_attempts: Maximum collision retries before raising error + + Returns: + Path where link was actually created (may differ from dest_path) + + Raises: + RuntimeError: If no unique path found after max_attempts + """ + base = dest_path.stem + ext = dest_path.suffix + parent = dest_path.parent + + for attempt in range(max_attempts): + try_path = dest_path if attempt == 0 else parent / f"{base}_{attempt}{ext}" + try: + os.link(str(source_path), str(try_path)) + if attempt > 0: + logger.info(f"File collision resolved: {try_path.name}") + return try_path + except FileExistsError: + continue + + raise RuntimeError(f"Could not create hardlink after {max_attempts} attempts: {dest_path}") + + +def atomic_copy(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: + """Copy a file with atomic collision detection. + + Uses exclusive create to claim destination, then copies via temp file + to avoid partial files on failure. + + Args: + source_path: Source file to copy + dest_path: Desired destination path + max_attempts: Maximum collision retries before raising error + + Returns: + Path where file was actually copied (may differ from dest_path) + + Raises: + RuntimeError: If no unique path found after max_attempts + """ + base = dest_path.stem + ext = dest_path.suffix + parent = dest_path.parent + + for attempt in range(max_attempts): + try_path = dest_path if attempt == 0 else parent / f"{base}_{attempt}{ext}" + try: + # Atomically claim the destination by creating an exclusive file + fd = os.open(str(try_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.close(fd) + # Copy to temp file first, then replace to avoid partial files + temp_path = try_path.parent / f".{try_path.name}.tmp" + try: + shutil.copy2(str(source_path), str(temp_path)) + temp_path.replace(try_path) + if attempt > 0: + logger.info(f"File collision resolved: {try_path.name}") + return try_path + except Exception: + try_path.unlink(missing_ok=True) + temp_path.unlink(missing_ok=True) + raise + except FileExistsError: + continue + + raise RuntimeError(f"Could not copy file after {max_attempts} attempts: {dest_path}") diff --git a/cwa_book_downloader/download/http.py b/cwa_book_downloader/download/http.py index 7641dcaa..e54e756f 100644 --- a/cwa_book_downloader/download/http.py +++ b/cwa_book_downloader/download/http.py @@ -15,6 +15,8 @@ from cwa_book_downloader.download.network import get_proxies from cwa_book_downloader.core.config import config as app_config from cwa_book_downloader.core.logger import setup_logger +logger = setup_logger(__name__) + # Bypasser modules are imported lazily to support dynamic selection based on config _internal_bypasser = None _external_bypasser = None @@ -65,8 +67,7 @@ def get_bypassed_page(url, selector=None, cancel_flag=None): """Wrapper that delegates to the appropriate bypasser based on config.""" if _is_using_external_bypasser(): return _get_external_bypasser().get_bypassed_page(url, selector, cancel_flag) - else: - return _get_internal_bypasser().get_bypassed_page(url, selector, cancel_flag) + return _get_internal_bypasser().get_bypassed_page(url, selector, cancel_flag) def get_cf_cookies_for_domain(domain): @@ -84,7 +85,24 @@ def get_cf_user_agent_for_domain(domain): return None return _get_internal_bypasser().get_cf_user_agent_for_domain(domain) -logger = setup_logger(__name__) + +def _apply_cf_bypass(url: str, headers: dict) -> dict: + """Apply CF bypass cookies and user agent if available. + + Modifies headers in-place with the stored user agent (if available). + Returns cookies dict to use with the request. + """ + if not _is_cf_bypass_enabled(): + return {} + + parsed = urlparse(url) + hostname = parsed.hostname or "" + cookies = get_cf_cookies_for_domain(hostname) + stored_ua = get_cf_user_agent_for_domain(hostname) + if stored_ua: + headers['User-Agent'] = stored_ua + return cookies + # Network settings REQUEST_TIMEOUT = (5, 10) # (connect, read) @@ -131,7 +149,7 @@ def _is_retryable_error(e: Exception) -> bool: if isinstance(e, CONNECTION_ERRORS): return True status = _get_status_code(e) - return status in RETRYABLE_CODES if status else False + return status is not None and status in RETRYABLE_CODES def _try_rotation(original_url: str, current_url: str, selector: network.AAMirrorSelector) -> Optional[str]: @@ -180,15 +198,8 @@ def html_get_page( logger.debug(f"GET: {current_url}") # Try with CF cookies/UA if available (from previous bypass) - cookies = {} headers = {} - if _is_cf_bypass_enabled(): - parsed = urlparse(current_url) - hostname = parsed.hostname or "" - cookies = get_cf_cookies_for_domain(hostname) - stored_ua = get_cf_user_agent_for_domain(hostname) - if stored_ua: - headers['User-Agent'] = stored_ua + cookies = _apply_cf_bypass(current_url, headers) response = requests.get(current_url, proxies=get_proxies(), timeout=REQUEST_TIMEOUT, cookies=cookies, headers=headers) response.raise_for_status() time.sleep(1) @@ -271,20 +282,7 @@ def download_url( logger.info(f"Downloading: {current_url} (attempt {attempt + 1}/{MAX_DOWNLOAD_RETRIES})") # Try with CF cookies/UA if available - cookies = {} - if _is_cf_bypass_enabled(): - parsed = urlparse(current_url) - hostname = parsed.hostname or "" - cookies = get_cf_cookies_for_domain(hostname) - # Use stored UA - Cloudflare ties cf_clearance to the UA that solved the challenge - stored_ua = get_cf_user_agent_for_domain(hostname) - if stored_ua: - headers['User-Agent'] = stored_ua - logger.debug(f"Using stored UA for {hostname}") - else: - logger.debug(f"No stored UA available for {hostname}") - if cookies: - logger.debug(f"Using {len(cookies)} cookies for {hostname}: {list(cookies.keys())}") + cookies = _apply_cf_bypass(current_url, headers) response = requests.get(current_url, stream=True, proxies=get_proxies(), timeout=REQUEST_TIMEOUT, cookies=cookies, headers=headers) response.raise_for_status() @@ -392,15 +390,8 @@ def _try_resume( try: # Try with CF cookies/UA if available - cookies = {} resume_headers = {**(base_headers or DOWNLOAD_HEADERS), 'Range': f'bytes={start_byte}-'} - if _is_cf_bypass_enabled(): - parsed = urlparse(url) - hostname = parsed.hostname or "" - cookies = get_cf_cookies_for_domain(hostname) - stored_ua = get_cf_user_agent_for_domain(hostname) - if stored_ua: - resume_headers['User-Agent'] = stored_ua + cookies = _apply_cf_bypass(url, resume_headers) response = requests.get( url, stream=True, proxies=get_proxies(), timeout=REQUEST_TIMEOUT, headers=resume_headers, cookies=cookies diff --git a/cwa_book_downloader/download/network.py b/cwa_book_downloader/download/network.py index 511e70b9..91b36795 100644 --- a/cwa_book_downloader/download/network.py +++ b/cwa_book_downloader/download/network.py @@ -1,6 +1,5 @@ """DNS rotation, mirror selection, and network utilities.""" -import os import requests import urllib.request from typing import Sequence, Tuple, Any, Union, cast, List, Optional, Callable @@ -8,12 +7,9 @@ import socket import dns.resolver from socket import AddressFamily, SocketKind import urllib.parse -import ssl import ipaddress from cwa_book_downloader.core.logger import setup_logger -from cwa_book_downloader.config.settings import AA_BASE_URL, AA_AVAILABLE_URLS -from cwa_book_downloader.config import settings as config from cwa_book_downloader.core.config import config as app_config from datetime import datetime, timedelta @@ -107,13 +103,6 @@ def _notify_dns_rotation(provider_name: str, servers: List[str], doh_url: str) - except Exception as e: logger.warning(f"DNS rotation callback {callback.__name__} failed: {e}") -def _agent_debug_log(code: str, source: str, reason: str, meta: Optional[dict] = None) -> None: - """Lightweight debug hook for automated runs; safe no-op on failure.""" - try: - logger.debug(f"[agent] code={code} source={source} reason={reason} meta={meta or {}}") - except Exception as exc: - # Avoid raising inside debug logger - logger.debug(f"[agent] log failed: {exc}") def _load_state(): """Return current in-memory network state (no disk persistence).""" @@ -133,7 +122,8 @@ def _save_state(aa_url=None, dns_provider=None): # AA URL failover state _current_aa_url_index = 0 -_aa_urls = AA_AVAILABLE_URLS.copy() +_aa_urls: List[str] = [] # Initialized lazily in _initialize_aa_state() +_aa_base_url: str = "" # Current active AA URL def _ensure_initialized() -> None: """Lazy guard so runtime setup happens once and late calls still work.""" @@ -234,34 +224,17 @@ def _decode_port(port: Union[str, bytes, int, None]) -> int: """Convert port to integer, handling various input types.""" if port is None: return 0 - if isinstance(port, (str, bytes)): - return int(port) return int(port) def _is_local_address(host_str: str) -> bool: """Check if an address is local or private and should bypass custom DNS.""" - # Localhost checks - if (host_str == 'localhost' or - host_str.startswith('127.') or - host_str == '::1' or - host_str == '0.0.0.0'): + if host_str == 'localhost': return True - - # IPv4 private ranges (RFC 1918) - if (host_str.startswith('10.') or - (host_str.startswith('172.') and - len(host_str.split('.')) > 1 and - 16 <= int(host_str.split('.')[1]) <= 31) or - host_str.startswith('192.168.')): - return True - - # IPv6 private ranges - if (host_str.startswith('fc') or - host_str.startswith('fd') or # Unique local addresses (fc00::/7) - host_str.startswith('fe80:')): # Link-local addresses (fe80::/10) - return True - - return False + try: + addr = ipaddress.ip_address(host_str) + return addr.is_private or addr.is_loopback or addr.is_link_local + except ValueError: + return False def _is_ip_address(host_str: str) -> bool: """Check if a string is a valid IP address (IPv4 or IPv6).""" @@ -642,8 +615,8 @@ def switch_dns_provider() -> bool: name, servers, doh = DNS_PROVIDERS[_current_dns_index] CUSTOM_DNS = servers DOH_SERVER = doh - config.CUSTOM_DNS = servers - config.DOH_SERVER = doh + app_config.CUSTOM_DNS = servers + app_config.DOH_SERVER = doh logger.warning(f"Switched DNS provider to: {name} (using DoH)") _save_state(dns_provider=name) @@ -672,20 +645,20 @@ def rotate_dns_and_reset_aa() -> bool: """ Switch DNS provider (auto mode) and reset AA URL list to the first entry. Returns True if DNS switched; False if no providers left or not in auto mode. - + Note: This function can be called during initialization, so we must NOT call _ensure_initialized() here to avoid recursive init loops. """ if not rotate_dns_provider(): return False # Reset AA URL to first available auto option if using auto AA - global AA_BASE_URL, _current_aa_url_index - if AA_BASE_URL == "auto" or AA_BASE_URL in _aa_urls: + global _aa_base_url, _current_aa_url_index + configured_url = app_config.get("AA_BASE_URL", "auto") + if configured_url == "auto" or _aa_base_url in _aa_urls: _current_aa_url_index = 0 - AA_BASE_URL = _aa_urls[0] - config.AA_BASE_URL = AA_BASE_URL - logger.info(f"After DNS switch, resetting AA URL to: {AA_BASE_URL}") - _save_state(aa_url=AA_BASE_URL) + _aa_base_url = _aa_urls[0] if _aa_urls else "https://annas-archive.org" + logger.info(f"After DNS switch, resetting AA URL to: {_aa_base_url}") + _save_state(aa_url=_aa_base_url) return True def set_dns_provider(provider: str, manual_servers: list[str] | None = None, use_doh: bool | None = None) -> bool: @@ -715,8 +688,8 @@ def set_dns_provider(provider: str, manual_servers: list[str] | None = None, use _dns_exhausted_logged = False CUSTOM_DNS = [] DOH_SERVER = "" - config.CUSTOM_DNS = [] - config.DOH_SERVER = "" + app_config.CUSTOM_DNS = [] + app_config.DOH_SERVER = "" # Restore original system getaddrinfo socket.getaddrinfo = original_getaddrinfo logger.info("DNS set to system mode (using OS default resolver)") @@ -730,8 +703,8 @@ def set_dns_provider(provider: str, manual_servers: list[str] | None = None, use _dns_exhausted_logged = False CUSTOM_DNS = [] DOH_SERVER = "" - config.CUSTOM_DNS = [] - config.DOH_SERVER = "" + app_config.CUSTOM_DNS = [] + app_config.DOH_SERVER = "" logger.info("DNS set to auto mode (system DNS, will rotate on failure with DoH)") init_dns_resolvers() _notify_dns_rotation("auto", [], "") @@ -744,8 +717,8 @@ def set_dns_provider(provider: str, manual_servers: list[str] | None = None, use _current_dns_index = -1 # Not using preset providers CUSTOM_DNS = manual_servers DOH_SERVER = "" # No DoH for manual servers - config.CUSTOM_DNS = manual_servers - config.DOH_SERVER = "" + app_config.CUSTOM_DNS = manual_servers + app_config.DOH_SERVER = "" logger.info(f"DNS set to manual servers: {manual_servers}") init_dns_resolvers() _notify_dns_rotation("manual", manual_servers, "") @@ -759,8 +732,8 @@ def set_dns_provider(provider: str, manual_servers: list[str] | None = None, use CUSTOM_DNS = servers # Only set DoH server if DoH is enabled DOH_SERVER = doh if doh_enabled else "" - config.CUSTOM_DNS = servers - config.DOH_SERVER = DOH_SERVER + app_config.CUSTOM_DNS = servers + app_config.DOH_SERVER = DOH_SERVER doh_status = "DoH enabled" if doh_enabled else "standard DNS" logger.info(f"DNS set to: {name} ({doh_status})") _save_state(dns_provider=name) @@ -781,14 +754,14 @@ def init_dns_resolvers(): name, servers, doh = DNS_PROVIDERS[_current_dns_index] CUSTOM_DNS = servers DOH_SERVER = doh - config.CUSTOM_DNS = servers - config.DOH_SERVER = doh + app_config.CUSTOM_DNS = servers + app_config.DOH_SERVER = doh logger.info(f"Using DNS provider: {name} (DoH enabled)") else: CUSTOM_DNS = [] DOH_SERVER = "" - config.CUSTOM_DNS = [] - config.DOH_SERVER = "" + app_config.CUSTOM_DNS = [] + app_config.DOH_SERVER = "" logger.debug("Using system DNS (auto mode - will switch on failure)") socket.getaddrinfo = cast(Any, create_system_failover_getaddrinfo()) return @@ -837,13 +810,29 @@ def _looks_like_ip(s: str) -> bool: # Simple heuristic: contains only digits, dots, and colons return s.replace(".", "").replace(":", "").isdigit() +def _build_aa_urls() -> List[str]: + """Build list of available AA URLs from config.""" + urls = ["https://annas-archive.org", "https://annas-archive.se", "https://annas-archive.li"] + additional = app_config.get("AA_ADDITIONAL_URLS", "") + if additional: + urls.extend(u.strip() for u in additional.split(",") if u.strip()) + return urls + + def _initialize_aa_state() -> None: """Restore or probe AA URL state.""" - global AA_BASE_URL, _current_aa_url_index - if AA_BASE_URL == "auto": + global _aa_base_url, _current_aa_url_index, _aa_urls + + # Build URL list from config + _aa_urls = _build_aa_urls() + + # Get configured base URL from config + configured_url = app_config.get("AA_BASE_URL", "auto") + + if configured_url == "auto": if state.get('aa_base_url') and state['aa_base_url'] in _aa_urls: _current_aa_url_index = _aa_urls.index(state['aa_base_url']) - AA_BASE_URL = state['aa_base_url'] + _aa_base_url = state['aa_base_url'] else: logger.debug(f"AA_BASE_URL: auto, checking available urls {_aa_urls}") for i, url in enumerate(_aa_urls): @@ -851,21 +840,22 @@ def _initialize_aa_state() -> None: response = requests.get(url, proxies=get_proxies(), timeout=3) if response.status_code == 200: _current_aa_url_index = i - AA_BASE_URL = url - _save_state(aa_url=AA_BASE_URL) + _aa_base_url = url + _save_state(aa_url=_aa_base_url) break except Exception: pass - if AA_BASE_URL == "auto": - AA_BASE_URL = _aa_urls[0] + if not _aa_base_url or _aa_base_url == "auto": + _aa_base_url = _aa_urls[0] _current_aa_url_index = 0 - elif AA_BASE_URL not in _aa_urls: - logger.info(f"AA_BASE_URL set to custom value {AA_BASE_URL}; skipping auto-switch") + elif configured_url not in _aa_urls: + logger.info(f"AA_BASE_URL set to custom value {configured_url}; skipping auto-switch") + _aa_base_url = configured_url else: - _current_aa_url_index = _aa_urls.index(AA_BASE_URL) + _current_aa_url_index = _aa_urls.index(configured_url) + _aa_base_url = configured_url - config.AA_BASE_URL = AA_BASE_URL - logger.info(f"AA_BASE_URL: {AA_BASE_URL}") + logger.info(f"AA_BASE_URL: {_aa_base_url}") def init_dns(force: bool = False) -> None: """Initialize DNS state and resolvers using set_dns_provider() for consistency.""" @@ -939,7 +929,7 @@ def init(force: bool = False) -> None: if _initialized and not force: return # Do the work first, then set flag to prevent race conditions - # where another thread sees _initialized=True but AA_BASE_URL is still "auto" + # where another thread sees _initialized=True but _aa_base_url is still empty try: init_dns(force=force) init_aa(force=force) @@ -952,7 +942,7 @@ def init(force: bool = False) -> None: def get_aa_base_url(): """Get current AA base URL.""" _ensure_initialized() - return AA_BASE_URL + return _aa_base_url def get_available_aa_urls(): """Get list of configured AA URLs (copy).""" @@ -962,14 +952,13 @@ def get_available_aa_urls(): def set_aa_url_index(new_index: int) -> bool: """Set AA base URL by index in available list; returns True if applied.""" _ensure_initialized() - global AA_BASE_URL, _current_aa_url_index + global _aa_base_url, _current_aa_url_index if new_index < 0 or new_index >= len(_aa_urls): return False _current_aa_url_index = new_index - AA_BASE_URL = _aa_urls[_current_aa_url_index] - config.AA_BASE_URL = AA_BASE_URL - logger.info(f"Set AA URL to: {AA_BASE_URL}") - _save_state(aa_url=AA_BASE_URL) + _aa_base_url = _aa_urls[_current_aa_url_index] + logger.info(f"Set AA URL to: {_aa_base_url}") + _save_state(aa_url=_aa_base_url) return True class AAMirrorSelector: diff --git a/cwa_book_downloader/download/orchestrator.py b/cwa_book_downloader/download/orchestrator.py index c3717475..fb2792b9 100644 --- a/cwa_book_downloader/download/orchestrator.py +++ b/cwa_book_downloader/download/orchestrator.py @@ -1,23 +1,7 @@ """Download queue orchestration and worker management. -## Download Architecture - -All downloads follow a two-stage process: - -1. **Staging (TMP_DIR)**: Handlers download/copy files to a temp staging area. - - Direct downloads: Downloaded directly to staging - - Torrent downloads: Copied from torrent client's completed folder to staging - - NZB downloads: Moved from NZB client's completed folder to staging - -2. **Ingest (INGEST_DIR)**: Orchestrator moves staged files to the final location. - - Archive extraction (RAR/ZIP) happens here - - Custom scripts run here - - Final move to ingest folder - -This ensures: -- Handlers don't need to know about ingest folder logic -- Archive handling works uniformly for all sources -- Single point of control for what enters the ingest folder +Two-stage architecture: handlers stage to TMP_DIR, orchestrator moves to INGEST_DIR +with archive extraction and custom script support. """ import hashlib @@ -36,9 +20,16 @@ from cwa_book_downloader.release_sources import direct_download from cwa_book_downloader.release_sources.direct_download import SearchUnavailable from cwa_book_downloader.core.config import config from cwa_book_downloader.config.env import TMP_DIR -from cwa_book_downloader.core.utils import get_ingest_dir, get_destination, get_aa_content_type_dir +from cwa_book_downloader.core.utils import get_ingest_dir, get_destination, get_aa_content_type_dir, is_audiobook as check_audiobook, transform_cover_url from cwa_book_downloader.core.naming import build_library_path, same_filesystem, assign_part_numbers, parse_naming_template, sanitize_filename -from cwa_book_downloader.download.archive import is_archive, process_archive +from cwa_book_downloader.download.archive import ( + is_archive, + process_archive, + _get_file_organization, + _get_template, + _get_supported_formats as _get_book_formats, + _get_supported_audiobook_formats, +) from cwa_book_downloader.release_sources import get_handler, get_source_display_name from cwa_book_downloader.core.logger import setup_logger from cwa_book_downloader.core.models import BookInfo, DownloadTask, QueueStatus, SearchFilters, SearchMode @@ -54,25 +45,13 @@ logger = setup_logger(__name__) # The orchestrator handles moving staged files to the ingest folder. def get_staging_dir() -> Path: - """Get the staging directory for downloads. - - All handlers should stage their downloads here. The orchestrator - handles moving staged files to the final ingest location. - """ + """Get the staging directory for downloads.""" TMP_DIR.mkdir(parents=True, exist_ok=True) return TMP_DIR def get_staging_path(task_id: str, extension: str) -> Path: - """Get a staging path for a download. - - Args: - task_id: Unique task identifier - extension: File extension (e.g., 'epub', 'zip') - - Returns: - Path in staging directory for this download - """ + """Get a staging path for a download.""" staging_dir = get_staging_dir() # Hash task_id in case it contains invalid filename chars (e.g., Prowlarr URLs) safe_id = hashlib.md5(task_id.encode()).hexdigest()[:16] @@ -80,19 +59,7 @@ def get_staging_path(task_id: str, extension: str) -> Path: def stage_file(source_path: Path, task_id: str, copy: bool = False) -> Path: - """Stage a file for ingest processing. - - Use this when a download client has completed a download and the file - needs to be staged for orchestrator processing. - - Args: - source_path: Path to the completed download - task_id: Unique task identifier - copy: If True, copy the file (for torrents). If False, move it. - - Returns: - Path to the staged file - """ + """Stage a file for ingest processing. Use copy=True for torrents to preserve seeding.""" staging_dir = get_staging_dir() # Stage with original filename, add counter suffix if collision staged_path = staging_dir / source_path.name @@ -112,85 +79,8 @@ def stage_file(source_path: Path, task_id: str, copy: bool = False) -> Path: return staged_path -# ============================================================================= -# File Organization Helpers -# ============================================================================= - - -def _get_file_organization(is_audiobook: bool) -> str: - """Get the file organization mode for the content type. - - Returns: - One of: "none", "rename", "organize" - """ - key = "FILE_ORGANIZATION_AUDIOBOOK" if is_audiobook else "FILE_ORGANIZATION" - mode = config.get(key, "rename") - - # Handle legacy settings migration - if mode not in ("none", "rename", "organize"): - # Check legacy PROCESSING_MODE - legacy_key = "PROCESSING_MODE_AUDIOBOOK" if is_audiobook else "PROCESSING_MODE" - legacy_mode = config.get(legacy_key, "ingest") - if legacy_mode == "library": - return "organize" - # Check legacy USE_BOOK_TITLE for ingest mode - if config.get("USE_BOOK_TITLE", True): - return "rename" - return "none" - - return mode - - -def _get_template(is_audiobook: bool, organization_mode: str) -> str: - """Get the template for the content type and organization mode. - - Returns: - Template string - """ - # Build the key based on content type and organization mode - if is_audiobook: - if organization_mode == "organize": - key = "TEMPLATE_AUDIOBOOK_ORGANIZE" - else: - key = "TEMPLATE_AUDIOBOOK_RENAME" - else: - if organization_mode == "organize": - key = "TEMPLATE_ORGANIZE" - else: - key = "TEMPLATE_RENAME" - - template = config.get(key, "") - - # Try legacy keys if new setting is empty - if not template: - # Try old unified TEMPLATE key - legacy_key = "TEMPLATE_AUDIOBOOK" if is_audiobook else "TEMPLATE" - template = config.get(legacy_key, "") - - if not template: - # Try even older LIBRARY_TEMPLATE key - legacy_key = "LIBRARY_TEMPLATE_AUDIOBOOK" if is_audiobook else "LIBRARY_TEMPLATE" - template = config.get(legacy_key, "") - - # Use sensible default if still empty - if not template: - if organization_mode == "organize": - return "{Author}/{Title} ({Year})" - else: - return "{Author} - {Title} ({Year})" - - return template - - def _should_hardlink(task: DownloadTask) -> bool: - """Determine if a download should be hardlinked instead of copied. - - Hardlinking only applies to torrent downloads (Prowlarr source). - Uses per-content-type settings. - - Returns: - True if hardlinking should be used - """ + """Check if download should be hardlinked (Prowlarr torrents only).""" # Only Prowlarr downloads (torrents) can be hardlinked if task.source != "prowlarr": return False @@ -200,7 +90,7 @@ def _should_hardlink(task: DownloadTask) -> bool: return False # Check per-content-type setting - is_audiobook = task.content_type and "audiobook" in task.content_type.lower() + is_audiobook = check_audiobook(task.content_type) key = "HARDLINK_TORRENTS_AUDIOBOOK" if is_audiobook else "HARDLINK_TORRENTS" # Check new setting first, then legacy TORRENT_HARDLINK @@ -213,30 +103,13 @@ def _should_hardlink(task: DownloadTask) -> bool: def _should_extract_archives(task: DownloadTask) -> bool: - """Determine if archives should be extracted for this download. - - Archives are NOT extracted when hardlinking is enabled (to preserve torrent seeding). - """ - if _should_hardlink(task): - return False - return True + """Check if archives should be extracted (disabled when hardlinking).""" + return not _should_hardlink(task) def _get_final_destination(task: DownloadTask) -> Path: - """Get the final destination directory for a download. - - Handles: - - Per-content-type destinations (books vs audiobooks) - - AA content-type routing override (for Direct mode downloads) - - Returns: - Path to the destination directory - """ - content_type = task.content_type.lower() if task.content_type else "" - is_audiobook = "audiobook" in content_type - - # Get base destination - base_dest = get_destination(is_audiobook) + """Get final destination directory, with content-type routing support.""" + is_audiobook = check_audiobook(task.content_type) # For Anna's Archive (direct_download), check for content-type routing override if task.source == "direct_download" and not is_audiobook: @@ -244,7 +117,7 @@ def _get_final_destination(task: DownloadTask) -> Path: if override: return override - return base_dest + return get_destination(is_audiobook) def _build_metadata_dict(task: DownloadTask) -> dict: @@ -261,32 +134,19 @@ def _build_metadata_dict(task: DownloadTask) -> dict: def _get_supported_formats(content_type: str = None) -> List[str]: """Get current supported formats from config singleton based on content type.""" - if content_type and content_type.lower() == "audiobook": - formats = config.get("SUPPORTED_AUDIOBOOK_FORMATS", ["m4b", "mp3"]) - else: - formats = config.get("SUPPORTED_FORMATS", ["epub", "mobi", "azw3", "fb2", "djvu", "cbz", "cbr"]) - # Handle both list (from MultiSelectField) and comma-separated string (legacy/env) - if isinstance(formats, str): - return [fmt.strip().lower() for fmt in formats.split(",") if fmt.strip()] - return [fmt.lower() for fmt in formats] + if check_audiobook(content_type): + return _get_supported_audiobook_formats() + return _get_book_formats() def _find_book_files_in_directory(directory: Path, content_type: str = None) -> Tuple[List[Path], List[Path]]: - """Find all book files in a directory matching supported formats. - - Args: - directory: Directory to search recursively - content_type: Content type to determine format list (e.g., "audiobook") - - Returns: - Tuple of (matching book files, rejected files with unsupported extensions) - """ + """Find book files matching supported formats. Returns (matches, rejected).""" book_files = [] rejected_files = [] supported_formats = _get_supported_formats(content_type) supported_exts = {f".{fmt}" for fmt in supported_formats} - is_audiobook = content_type and content_type.lower() == "audiobook" + is_audiobook = check_audiobook(content_type) if is_audiobook: trackable_exts = {'.m4b', '.mp3', '.m4a', '.flac', '.ogg', '.wma', '.aac', '.wav'} else: @@ -307,19 +167,7 @@ def process_directory( ingest_dir: Path, task: DownloadTask, ) -> Tuple[List[Path], Optional[str]]: - """Process a staged directory: find book files, handle archives, move to ingest. - - For multi-file torrent/usenet downloads. If book files exist, moves them directly. - If only archives exist, extracts them to find book files inside. - - Args: - directory: Staged directory containing downloaded files - ingest_dir: Final destination directory for book files - task: Download task for filename generation - - Returns: - Tuple of (list of final paths, error message if failed) - """ + """Process staged directory: find book files, extract archives, move to ingest.""" try: content_type = task.content_type book_files, rejected_files = _find_book_files_in_directory(directory, content_type) @@ -385,8 +233,7 @@ def process_directory( # Move each book file to destination final_paths = [] - content_type = task.content_type.lower() if task.content_type else "" - is_audiobook = "audiobook" in content_type + is_audiobook = check_audiobook(task.content_type) organization_mode = _get_file_organization(is_audiobook) for book_file in book_files: @@ -428,11 +275,14 @@ def process_directory( # WebSocket manager (initialized by app.py) +# Track whether WebSocket is available for status reporting +WEBSOCKET_AVAILABLE = True try: from cwa_book_downloader.api.websocket import ws_manager except ImportError: - logger.warning("WebSocket unavailable - real-time updates disabled") + logger.error("WebSocket unavailable - real-time updates disabled") ws_manager = None + WEBSOCKET_AVAILABLE = False # Progress update throttling - track last broadcast time per book _progress_last_broadcast: Dict[str, float] = {} @@ -443,15 +293,7 @@ _last_activity: Dict[str, float] = {} STALL_TIMEOUT = 300 # 5 minutes without progress/status update = stalled def search_books(query: str, filters: SearchFilters) -> List[Dict[str, Any]]: - """Search for books matching the query. - - Args: - query: Search term - filters: Search filters object - - Returns: - List[Dict]: List of book information dictionaries - """ + """Search for books matching the query.""" try: books = direct_download.search_books(query, filters) return [_book_info_to_dict(book) for book in books] @@ -462,17 +304,7 @@ def search_books(query: str, filters: SearchFilters) -> List[Dict[str, Any]]: raise def get_book_info(book_id: str) -> Optional[Dict[str, Any]]: - """Get detailed information for a specific book. - - Args: - book_id: Book identifier - - Returns: - Optional[Dict]: Book information dictionary if found, None if not found - - Raises: - Exception: If there's an error fetching the book info - """ + """Get detailed information for a specific book.""" try: book = direct_download.get_book_info(book_id) return _book_info_to_dict(book) @@ -480,25 +312,14 @@ def get_book_info(book_id: str) -> Optional[Dict[str, Any]]: logger.error_trace(f"Error getting book info: {e}") raise -def queue_book(book_id: str, priority: int = 0, source: str = "direct_download") -> bool: - """Add a book to the download queue with specified priority. - - Fetches display info and creates a DownloadTask. The handler will fetch - the full book details (including download URLs) when processing. - - Args: - book_id: Book identifier (e.g., AA MD5 hash) - priority: Priority level (lower number = higher priority) - source: Release source handler to use (default: direct_download) - - Returns: - bool: True if book was successfully queued - """ +def queue_book(book_id: str, priority: int = 0, source: str = "direct_download") -> Tuple[bool, Optional[str]]: + """Add a book to the download queue. Returns (success, error_message).""" try: book_info = direct_download.get_book_info(book_id, fetch_download_count=False) if not book_info: - logger.warning(f"Could not fetch book info for {book_id}") - return False + error_msg = f"Could not fetch book info for {book_id}" + logger.warning(error_msg) + return False, error_msg # Create a source-agnostic download task task = DownloadTask( @@ -516,7 +337,7 @@ def queue_book(book_id: str, priority: int = 0, source: str = "direct_download") if not book_queue.add(task): logger.info(f"Book already in queue: {book_info.title}") - return False + return False, "Book is already in the download queue" logger.info(f"Book queued with priority {priority}: {book_info.title}") @@ -524,28 +345,19 @@ def queue_book(book_id: str, priority: int = 0, source: str = "direct_download") if ws_manager: ws_manager.broadcast_status_update(queue_status()) - return True + return True, None + except SearchUnavailable as e: + error_msg = f"Search service unavailable: {e}" + logger.warning(error_msg) + return False, error_msg except Exception as e: - logger.error_trace(f"Error queueing book: {e}") - return False + error_msg = f"Error queueing book: {e}" + logger.error_trace(error_msg) + return False, error_msg -def queue_release(release_data: dict, priority: int = 0) -> bool: - """Add a release to the download queue. - - This is used when downloading from the ReleaseModal where we already have - all the release data from the search - no need to re-fetch. - - Creates a DownloadTask directly from the release data. The handler will - fetch full details when processing. - - Args: - release_data: Release dictionary with source, source_id, title, format, etc. - priority: Priority level (lower number = higher priority) - - Returns: - bool: True if release was successfully queued - """ +def queue_release(release_data: dict, priority: int = 0) -> Tuple[bool, Optional[str]]: + """Add a release to the download queue. Returns (success, error_message).""" try: source = release_data.get('source', 'direct_download') extra = release_data.get('extra', {}) @@ -581,7 +393,7 @@ def queue_release(release_data: dict, priority: int = 0) -> bool: if not book_queue.add(task): logger.info(f"Release already in queue: {task.title}") - return False + return False, "Release is already in the download queue" logger.info(f"Release queued with priority {priority}: {task.title}") @@ -589,28 +401,29 @@ def queue_release(release_data: dict, priority: int = 0) -> bool: if ws_manager: ws_manager.broadcast_status_update(queue_status()) - return True + return True, None except ValueError as e: # Handler not found for this source - logger.warning(f"Unknown release source: {e}") - return False + error_msg = f"Unknown release source: {e}" + logger.warning(error_msg) + return False, error_msg + except KeyError as e: + error_msg = f"Missing required field in release data: {e}" + logger.warning(error_msg) + return False, error_msg except Exception as e: - logger.error_trace(f"Error queueing release: {e}") - return False + error_msg = f"Error queueing release: {e}" + logger.error_trace(error_msg) + return False, error_msg def queue_status() -> Dict[str, Dict[str, Any]]: - """Get current status of the download queue. - - Returns: - Dict: Queue status organized by status type with serialized task data - """ + """Get current status of the download queue.""" status = book_queue.get_status() for _, tasks in status.items(): for _, task in tasks.items(): - if task.download_path: - if not os.path.exists(task.download_path): - task.download_path = None + if task.download_path and not os.path.exists(task.download_path): + task.download_path = None # Convert Enum keys to strings and DownloadTask objects to dicts for JSON serialization return { @@ -622,14 +435,7 @@ def queue_status() -> Dict[str, Dict[str, Any]]: } def get_book_data(task_id: str) -> Tuple[Optional[bytes], Optional[DownloadTask]]: - """Get downloaded file data for a specific task. - - Args: - task_id: Task identifier - - Returns: - Tuple[Optional[bytes], Optional[DownloadTask]]: File data if available, and the task - """ + """Get downloaded file data for a specific task.""" task = None try: task = book_queue.get_task(task_id) @@ -649,12 +455,7 @@ def get_book_data(task_id: str) -> Tuple[Optional[bytes], Optional[DownloadTask] return None, task def _book_info_to_dict(book: BookInfo) -> Dict[str, Any]: - """Convert BookInfo object to dictionary representation. - - Transforms external preview URLs to local proxy URLs when cover caching is enabled. - """ - from cwa_book_downloader.core.utils import transform_cover_url - + """Convert BookInfo to dict, transforming cover URLs for caching.""" result = { key: value for key, value in book.__dict__.items() if value is not None @@ -668,14 +469,7 @@ def _book_info_to_dict(book: BookInfo) -> Dict[str, Any]: def _task_to_dict(task: DownloadTask) -> Dict[str, Any]: - """Convert DownloadTask object to dictionary representation. - - Maps DownloadTask fields to the format expected by the frontend, - maintaining compatibility with the previous BookInfo-based format. - Transforms external preview URLs to local proxy URLs when cover caching is enabled. - """ - from cwa_book_downloader.core.utils import transform_cover_url - + """Convert DownloadTask to dict for frontend, transforming cover URLs.""" # Transform external preview URLs to local proxy URLs preview = transform_cover_url(task.preview, task.task_id) @@ -699,19 +493,7 @@ def _task_to_dict(task: DownloadTask) -> Dict[str, Any]: def _download_task(task_id: str, cancel_flag: Event) -> Optional[str]: - """Download a task with cancellation support. - - Delegates to the appropriate handler based on the task's source. - Handlers return a temp file path, orchestrator handles post-processing - (archive extraction, moving to ingest) uniformly for all sources. - - Args: - task_id: Task identifier - cancel_flag: Threading event to signal cancellation - - Returns: - str: Path to the downloaded file if successful, None otherwise - """ + """Download a task via appropriate handler, then post-process to ingest.""" try: # Check for cancellation before starting if cancel_flag.is_set(): @@ -766,6 +548,11 @@ def _download_task(task_id: str, cancel_flag: Event) -> Optional[str]: logger.info(f"Download cancelled during error handling: {task_id}") else: logger.error_trace(f"Error downloading: {e}") + # Update task status so user sees the failure + task = book_queue.get_task(task_id) + if task: + book_queue.update_status(task_id, QueueStatus.ERROR) + book_queue.update_status_message(task_id, f"Download failed: {type(e).__name__}") return None @@ -774,16 +561,8 @@ def _process_organize_mode( task: DownloadTask, status_callback, ) -> Optional[str]: - """Process a download with file organization (organize mode with folders). - - Organizes files into folders based on template (e.g., "{Author}/{Series/}{Title}"). - Supports hardlinking for torrent downloads when enabled. - - Returns: - Path to organized file if successful, None if failed - """ - content_type = task.content_type.lower() if task.content_type else "" - is_audiobook = "audiobook" in content_type + """Organize files into library folders using template. Supports hardlinking.""" + is_audiobook = check_audiobook(task.content_type) # Get destination and template destination = _get_final_destination(task) @@ -810,32 +589,25 @@ def _process_organize_mode( # Determine if we should use hardlinking use_hardlink = False - hardlink_source = None + source = temp_file if _should_hardlink(task): hardlink_source = Path(task.original_download_path) - if hardlink_source.exists(): - # Check same filesystem (required for hardlinks) - if same_filesystem(hardlink_source, destination): - use_hardlink = True - else: - logger.warning( - f"Cannot hardlink: {hardlink_source} and {destination} are on different filesystems. " - "Falling back to copy. To fix: ensure torrent client downloads to same filesystem as destination." - ) - status_callback("resolving", "Cannot hardlink (different filesystems), using copy") + if hardlink_source.exists() and same_filesystem(hardlink_source, destination): + use_hardlink = True + source = hardlink_source + elif hardlink_source.exists(): + logger.warning( + f"Cannot hardlink: {hardlink_source} and {destination} are on different filesystems. " + "Falling back to copy. To fix: ensure torrent client downloads to same filesystem as destination." + ) + status_callback("resolving", "Cannot hardlink (different filesystems), using copy") # Build metadata dict for template metadata = _build_metadata_dict(task) try: - if use_hardlink: - status_callback("resolving", "Creating hardlinks") - else: - status_callback("resolving", "Organizing files") - - # Use torrent client path for hardlinks, staging path for moves - source = hardlink_source if use_hardlink else temp_file + status_callback("resolving", "Creating hardlinks" if use_hardlink else "Organizing files") if source.is_dir(): return _transfer_directory_to_library( @@ -865,108 +637,34 @@ def _is_torrent_source(source_path: Path, task: DownloadTask) -> bool: return False -def _get_unique_path(dest_path: Path) -> Path: - """Return a unique path by appending counter if file already exists. - - Note: Has TOCTOU race. Use _atomic_hardlink/_atomic_move for concurrent safety. - """ - if not dest_path.exists(): - return dest_path - - base = dest_path.stem - ext = dest_path.suffix +def _stage_torrent_path(source: Path) -> Path: + """Copy torrent source to staging directory to preserve seeding.""" + staging_dir = get_staging_dir() + staged_path = staging_dir / source.name counter = 1 - while dest_path.exists(): - dest_path = dest_path.parent / f"{base}_{counter}{ext}" - counter += 1 - logger.info(f"File already exists, saving as: {dest_path.name}") - return dest_path + if source.is_dir(): + while staged_path.exists(): + staged_path = staging_dir / f"{source.name}_{counter}" + counter += 1 + shutil.copytree(str(source), str(staged_path)) + else: + while staged_path.exists(): + staged_path = staging_dir / f"{source.stem}_{counter}{source.suffix}" + counter += 1 + shutil.copy2(str(source), str(staged_path)) + + logger.debug(f"Staged torrent {'directory' if source.is_dir() else 'file'}: {staged_path.name}") + return staged_path -def _atomic_hardlink(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: - """Create a hardlink with atomic collision detection. Retries with counter suffix on collision.""" - base = dest_path.stem - ext = dest_path.suffix - - for attempt in range(max_attempts): - try_path = dest_path if attempt == 0 else dest_path.parent / f"{base}_{attempt}{ext}" - try: - os.link(str(source_path), str(try_path)) - if attempt > 0: - logger.info(f"File collision resolved: {try_path.name}") - return try_path - except FileExistsError: - continue - - raise RuntimeError(f"Could not create hardlink after {max_attempts} attempts: {dest_path}") - - -def _atomic_copy(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: - """Copy a file with atomic collision detection. Retries with counter suffix on collision.""" - base = dest_path.stem - ext = dest_path.suffix - - for attempt in range(max_attempts): - try_path = dest_path if attempt == 0 else dest_path.parent / f"{base}_{attempt}{ext}" - try: - # Atomically claim the destination by creating an exclusive file - fd = os.open(str(try_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) - os.close(fd) - try: - # Copy to temp file first, then replace to avoid partial files - temp_path = try_path.parent / f".{try_path.name}.tmp" - shutil.copy2(str(source_path), str(temp_path)) - temp_path.replace(try_path) - if attempt > 0: - logger.info(f"File collision resolved: {try_path.name}") - return try_path - except Exception: - try_path.unlink(missing_ok=True) - temp_path.unlink(missing_ok=True) if 'temp_path' in locals() else None - raise - except FileExistsError: - continue - - raise RuntimeError(f"Could not copy file after {max_attempts} attempts: {dest_path}") - - -def _atomic_move(source_path: Path, dest_path: Path, max_attempts: int = 100) -> Path: - """Move a file with atomic collision detection. Retries with counter suffix on collision.""" - import errno - - base = dest_path.stem - ext = dest_path.suffix - - for attempt in range(max_attempts): - try_path = dest_path if attempt == 0 else dest_path.parent / f"{base}_{attempt}{ext}" - try: - os.link(str(source_path), str(try_path)) - source_path.unlink() - if attempt > 0: - logger.info(f"File collision resolved: {try_path.name}") - return try_path - except FileExistsError: - continue - except OSError as e: - # Cross-filesystem - fall back to exclusive create - if e.errno not in (errno.EXDEV, errno.EMLINK): - raise - try: - fd = os.open(str(try_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) - os.close(fd) - try: - shutil.move(str(source_path), str(try_path)) - if attempt > 0: - logger.info(f"File collision resolved: {try_path.name}") - return try_path - except Exception: - try_path.unlink(missing_ok=True) - raise - except FileExistsError: - continue - - raise RuntimeError(f"Could not move file after {max_attempts} attempts: {dest_path}") +# Import atomic file operations from shared module +# Re-exported here for backwards compatibility with existing tests/imports +from cwa_book_downloader.download.fs import ( + atomic_hardlink as _atomic_hardlink, + atomic_copy as _atomic_copy, + atomic_move as _atomic_move, +) def _cleanup_staged_files(temp_file: Path, source_dir: Optional[Path] = None) -> None: @@ -1023,7 +721,7 @@ def _transfer_file_to_library( if use_hardlink: _cleanup_staged_files(temp_file) - status_callback("complete", "Complete (library mode)") + status_callback("complete", "Complete") return str(final_path) @@ -1100,8 +798,8 @@ def _transfer_directory_to_library( elif not is_torrent: _cleanup_staged_files(temp_file, source_dir) - count_msg = f" ({len(transferred_paths)} files," if len(transferred_paths) > 1 else " (" - status_callback("complete", f"Complete{count_msg} library mode)") + message = f"Complete ({len(transferred_paths)} files)" if len(transferred_paths) > 1 else "Complete" + status_callback("complete", message) return str(transferred_paths[0]) @@ -1112,25 +810,8 @@ def _post_process_download( cancel_flag: Event, status_callback, ) -> Optional[str]: - """Post-process a downloaded file based on file organization settings. - - This runs uniformly for all download sources, ensuring consistent behavior. - Handles three organization modes: - - "none": Keep original filename, move to destination - - "rename": Apply template to filename, move to destination - - "organize": Apply template with folders, move to destination - - Args: - temp_file: Path to downloaded file in temp directory - task: Download task with metadata - cancel_flag: Cancellation event - status_callback: Callback for status updates - - Returns: - Final path in destination directory, or None on failure - """ - content_type = task.content_type.lower() if task.content_type else "" - is_audiobook = "audiobook" in content_type + """Post-process download: extract archives, apply naming template, move to destination.""" + is_audiobook = check_audiobook(task.content_type) # Validate search_mode if task.search_mode is None: @@ -1150,7 +831,11 @@ def _post_process_download( if result is not None: return result # If organize mode fails, fall through to flat mode - logger.warning("Organize mode failed, falling back to flat destination") + logger.warning( + f"Organize mode failed for '{task.title}', falling back to flat destination. " + "Check destination folder permissions and ensure the path is writable." + ) + status_callback("resolving", "Organization failed, using flat destination") # Ensure destination exists os.makedirs(destination, exist_ok=True) @@ -1159,26 +844,7 @@ def _post_process_download( # (Torrent handler returns original path, not staged copy) if _is_torrent_source(temp_file, task) and not _should_hardlink(task): status_callback("resolving", "Staging torrent files") - staging_dir = get_staging_dir() - - if temp_file.is_dir(): - staged_path = staging_dir / temp_file.name - counter = 1 - while staged_path.exists(): - staged_path = staging_dir / f"{temp_file.name}_{counter}" - counter += 1 - shutil.copytree(str(temp_file), str(staged_path)) - logger.debug(f"Staged torrent directory: {staged_path.name}") - else: - staged_path = staging_dir / temp_file.name - counter = 1 - while staged_path.exists(): - staged_path = staging_dir / f"{temp_file.stem}_{counter}{temp_file.suffix}" - counter += 1 - shutil.copy2(str(temp_file), str(staged_path)) - logger.debug(f"Staged torrent file: {staged_path.name}") - - temp_file = staged_path + temp_file = _stage_torrent_path(temp_file) # Handle archive extraction (RAR/ZIP) - only if not hardlinking if is_archive(temp_file) and _should_extract_archives(task): @@ -1215,17 +881,14 @@ def _post_process_download( status_callback("error", error) return None - if final_paths: - if len(final_paths) == 1: - message = "Complete" - else: - message = f"Complete ({len(final_paths)} files)" - status_callback("complete", message) - return str(final_paths[0]) - else: + if not final_paths: status_callback("error", "No book files found") return None + message = "Complete" if len(final_paths) == 1 else f"Complete ({len(final_paths)} files)" + status_callback("complete", message) + return str(final_paths[0]) + # Non-archive: run custom script if configured, then move to destination if config.CUSTOM_SCRIPT: logger.info(f"Running custom script: {config.CUSTOM_SCRIPT}") @@ -1297,14 +960,7 @@ def _post_process_download( return str(final_path) def update_download_progress(book_id: str, progress: float) -> None: - """Update download progress with throttled WebSocket broadcasts. - - Progress is always stored in the queue, but WebSocket broadcasts are - throttled to avoid flooding clients with updates. Broadcasts occur: - - At most once per DOWNLOAD_PROGRESS_UPDATE_INTERVAL seconds - - Always at 0% (start) and 100% (complete) - - On significant progress jumps (>10%) - """ + """Update download progress with throttled WebSocket broadcasts.""" book_queue.update_progress(book_id, progress) # Track activity for stall detection @@ -1339,13 +995,7 @@ def update_download_progress(book_id: str, progress: float) -> None: ws_manager.broadcast_download_progress(book_id, progress, 'downloading') def update_download_status(book_id: str, status: str, message: Optional[str] = None) -> None: - """Update download status with optional detailed message. - - Args: - book_id: Book identifier - status: Status string (e.g., 'resolving', 'downloading') - message: Optional detailed status message for UI display - """ + """Update download status with optional message for UI display.""" # Map string status to QueueStatus enum status_map = { 'queued': QueueStatus.QUEUED, @@ -1375,14 +1025,7 @@ def update_download_status(book_id: str, status: str, message: Optional[str] = N ws_manager.broadcast_status_update(queue_status()) def cancel_download(book_id: str) -> bool: - """Cancel a download. - - Args: - book_id: Book identifier to cancel - - Returns: - bool: True if cancellation was successful - """ + """Cancel a download.""" result = book_queue.cancel_download(book_id) # Broadcast status update via WebSocket @@ -1392,26 +1035,11 @@ def cancel_download(book_id: str) -> bool: return result def set_book_priority(book_id: str, priority: int) -> bool: - """Set priority for a queued book. - - Args: - book_id: Book identifier - priority: New priority level (lower = higher priority) - - Returns: - bool: True if priority was successfully changed - """ + """Set priority for a queued book (lower = higher priority).""" return book_queue.set_priority(book_id, priority) def reorder_queue(book_priorities: Dict[str, int]) -> bool: - """Bulk reorder queue. - - Args: - book_priorities: Dict mapping book_id to new priority - - Returns: - bool: True if reordering was successful - """ + """Bulk reorder queue by mapping book_id to new priority.""" return book_queue.reorder_queue(book_priorities) def get_queue_order() -> List[Dict[str, Any]]: @@ -1539,11 +1167,7 @@ _started = False def start() -> None: - """Start the download coordinator thread. - - This should be called once during application startup. - Calling multiple times is safe - subsequent calls are no-ops. - """ + """Start the download coordinator thread. Safe to call multiple times.""" global _coordinator_thread, _started if _started: diff --git a/cwa_book_downloader/main.py b/cwa_book_downloader/main.py index dc95e420..8b71e4a9 100644 --- a/cwa_book_downloader/main.py +++ b/cwa_book_downloader/main.py @@ -135,6 +135,15 @@ def clear_failed_logins(username: str) -> None: logger.debug(f"Cleared failed login attempts for user: {username}") +def get_client_ip() -> str: + """Extract client IP address from request, handling reverse proxy forwarding.""" + ip_address = request.headers.get('X-Forwarded-For', request.remote_addr) or 'unknown' + # X-Forwarded-For can contain multiple IPs, take the first one + if ',' in ip_address: + ip_address = ip_address.split(',')[0].strip() + return ip_address + + def get_auth_mode() -> str: """Determine which authentication mode is active. @@ -171,44 +180,34 @@ if DEBUG: }) # Custom log filter to exclude routine status endpoint polling and WebSocket noise -class StatusEndpointFilter(logging.Filter): - """Filter out routine status endpoint requests and WebSocket upgrade errors to reduce log noise.""" - def filter(self, record): - if hasattr(record, 'getMessage'): - message = record.getMessage() - # Exclude GET /api/status requests (polling noise) - if 'GET /api/status' in message: - return False - # Exclude WebSocket upgrade errors (benign - falls back to polling) - if 'write() before start_response' in message: - return False - # Exclude the Error on request line that precedes WebSocket errors - if 'Error on request:' in message and record.levelno == logging.ERROR: - return False - return True +class LogNoiseFilter(logging.Filter): + """Filter out routine status endpoint requests and WebSocket upgrade errors to reduce log noise. - -class WebSocketErrorFilter(logging.Filter): - """Filter out WebSocket upgrade errors that occur in Werkzeug dev server. - - These errors are benign - Flask-SocketIO automatically falls back to polling transport. + WebSocket upgrade errors are benign - Flask-SocketIO automatically falls back to polling transport. The error occurs because Werkzeug's built-in server doesn't fully support WebSocket upgrades. """ def filter(self, record): - # Filter out the AssertionError traceback for WebSocket upgrades + message = record.getMessage() if hasattr(record, 'getMessage') else str(record.msg) + + # Exclude GET /api/status requests (polling noise) + if 'GET /api/status' in message: + return False + + # Exclude WebSocket upgrade errors (benign - falls back to polling) + if 'write() before start_response' in message: + return False + + # Exclude the Error on request line that precedes WebSocket errors if record.levelno == logging.ERROR: - message = record.getMessage() if hasattr(record, 'getMessage') else str(record.msg) - # Filter out the full traceback that includes the WebSocket assertion error - if 'write() before start_response' in message: + if 'Error on request:' in message: return False - # Also filter the "Error on request" header that precedes it + # Filter WebSocket-related AssertionError tracebacks if hasattr(record, 'exc_info') and record.exc_info: - exc_type = record.exc_info[0] + exc_type, exc_value = record.exc_info[0], record.exc_info[1] if exc_type and exc_type.__name__ == 'AssertionError': - # Check if it's the WebSocket-related assertion - exc_value = record.exc_info[1] if exc_value and 'write() before start_response' in str(exc_value): return False + return True # Flask logger @@ -218,17 +217,15 @@ app.logger.setLevel(logger.level) werkzeug_logger = logging.getLogger('werkzeug') werkzeug_logger.handlers = logger.handlers werkzeug_logger.setLevel(logger.level) -# Add filters to suppress routine status endpoint polling logs and WebSocket upgrade errors -werkzeug_logger.addFilter(StatusEndpointFilter()) -werkzeug_logger.addFilter(WebSocketErrorFilter()) +# Add filter to suppress routine status endpoint polling logs and WebSocket upgrade errors +werkzeug_logger.addFilter(LogNoiseFilter()) # Set up authentication defaults # The secret key will reset every time we restart, which will # require users to authenticate again +from cwa_book_downloader.config.env import SESSION_COOKIE_SECURE_ENV, string_to_bool -# Session cookie security - set to 'true' if exclusively using HTTPS -session_cookie_secure_env = os.getenv('SESSION_COOKIE_SECURE', 'false').lower() -SESSION_COOKIE_SECURE = session_cookie_secure_env in ['true', 'yes', '1'] +SESSION_COOKIE_SECURE = string_to_bool(SESSION_COOKIE_SECURE_ENV) app.config.update( SECRET_KEY = os.urandom(64), @@ -238,7 +235,7 @@ app.config.update( PERMANENT_SESSION_LIFETIME = 604800 # 7 days in seconds ) -logger.info(f"Session cookie secure setting: {SESSION_COOKIE_SECURE} (from env: {session_cookie_secure_env})") +logger.info(f"Session cookie secure setting: {SESSION_COOKIE_SECURE} (from env: {SESSION_COOKIE_SECURE_ENV})") def login_required(f): @wraps(f) @@ -304,10 +301,12 @@ if not app_config.get("USING_EXTERNAL_BYPASSER", False): if DEBUG: import subprocess + if app_config.get("USING_EXTERNAL_BYPASSER", False): - STOP_GUI = lambda: None + _stop_gui = lambda: None else: - from cwa_book_downloader.bypass.internal_bypasser import _reset_driver as STOP_GUI + from cwa_book_downloader.bypass.internal_bypasser import _reset_driver as _stop_gui + @app.route('/api/debug', methods=['GET']) @login_required def debug() -> Union[Response, Tuple[Response, int]]: @@ -317,9 +316,8 @@ if DEBUG: And then return it to the user """ try: - # Run the debug script logger.info("Debug endpoint called, stopping GUI and generating debug info...") - STOP_GUI() + _stop_gui() time.sleep(1) result = subprocess.run(['/app/genDebug.sh'], capture_output=True, text=True, check=True) if result.returncode != 0: @@ -329,9 +327,8 @@ if DEBUG: if not os.path.exists(debug_file_path): logger.error(f"Debug zip file not found at: {debug_file_path}") return jsonify({"error": "Failed to generate debug information"}), 500 - + logger.info(f"Sending debug file: {debug_file_path}") - # Return the file to the user return send_file( debug_file_path, mimetype='application/zip', @@ -345,7 +342,6 @@ if DEBUG: logger.error_trace(f"Debug endpoint error: {e}") return jsonify({"error": str(e)}), 500 -if DEBUG: @app.route('/api/restart', methods=['GET']) @login_required def restart() -> Union[Response, Tuple[Response, int]]: @@ -441,10 +437,10 @@ def api_download() -> Union[Response, Tuple[Response, int]]: try: priority = int(request.args.get('priority', 0)) - success = backend.queue_book(book_id, priority) + success, error_msg = backend.queue_book(book_id, priority) if success: return jsonify({"status": "queued", "priority": priority}) - return jsonify({"error": "Failed to queue book"}), 500 + return jsonify({"error": error_msg or "Failed to queue book"}), 500 except Exception as e: logger.error_trace(f"Download error: {e}") return jsonify({"error": str(e)}), 500 @@ -479,22 +475,16 @@ def api_download_release() -> Union[Response, Tuple[Response, int]]: return jsonify({"error": "source_id is required"}), 400 priority = data.get('priority', 0) - success = backend.queue_release(data, priority) + success, error_msg = backend.queue_release(data, priority) if success: return jsonify({"status": "queued", "priority": priority}) - return jsonify({"error": "Failed to queue release"}), 500 + return jsonify({"error": error_msg or "Failed to queue release"}), 500 except Exception as e: logger.error_trace(f"Release download error: {e}") return jsonify({"error": str(e)}), 500 -def _is_settings_enabled() -> bool: - """Check if the config directory is mounted and writable.""" - from cwa_book_downloader.config.env import _is_config_dir_writable - return _is_config_dir_writable() - - @app.route('/api/config', methods=['GET']) @login_required def api_config() -> Union[Response, Tuple[Response, int]]: @@ -510,6 +500,7 @@ def api_config() -> Union[Response, Tuple[Response, int]]: get_provider_search_fields, get_provider_default_sort, ) + from cwa_book_downloader.config.env import _is_config_dir_writable config = { "calibre_web_url": app_config.get("CALIBRE_WEB_URL", ""), @@ -526,7 +517,7 @@ def api_config() -> Union[Response, Tuple[Response, int]]: "default_release_source": app_config.get("DEFAULT_RELEASE_SOURCE", "direct_download"), "auto_open_downloads_sidebar": app_config.get("AUTO_OPEN_DOWNLOADS_SIDEBAR", True), "download_to_browser": app_config.get("DOWNLOAD_TO_BROWSER", False), - "settings_enabled": _is_settings_enabled(), + "settings_enabled": _is_config_dir_writable(), # Default sort orders "default_sort": app_config.get("AA_DEFAULT_SORT", "relevance"), # For direct mode (Anna's Archive) "metadata_default_sort": get_provider_default_sort(), # For universal mode @@ -541,11 +532,17 @@ def api_health() -> Union[Response, Tuple[Response, int]]: """ Health check endpoint for container orchestration. No authentication required. - + Returns: - flask.Response: JSON with status "ok". + flask.Response: JSON with status "ok" and optional degraded features. """ - return jsonify({"status": "ok"}) + response = {"status": "ok"} + + # Report degraded features + if not backend.WEBSOCKET_AVAILABLE: + response["degraded"] = {"websocket": "WebSocket unavailable - real-time updates disabled"} + + return jsonify(response) @app.route('/api/status', methods=['GET']) @login_required @@ -874,12 +871,7 @@ def api_login() -> Union[Response, Tuple[Response, int]]: from cwa_book_downloader.core.settings_registry import load_config_file try: - # Get client IP address (handles reverse proxy forwarding) - ip_address = request.headers.get('X-Forwarded-For', request.remote_addr) - if ip_address and ',' in ip_address: - # X-Forwarded-For can contain multiple IPs, take the first one - ip_address = ip_address.split(',')[0].strip() - + ip_address = get_client_ip() data = request.get_json() if not data: return jsonify({"error": "No data provided"}), 400 @@ -983,11 +975,7 @@ def api_logout() -> Union[Response, Tuple[Response, int]]: flask.Response: JSON with success status. """ try: - # Get client IP address (handles reverse proxy forwarding) - ip_address = request.headers.get('X-Forwarded-For', request.remote_addr) - if ip_address and ',' in ip_address: - ip_address = ip_address.split(',')[0].strip() - + ip_address = get_client_ip() username = session.get('user_id', 'unknown') session.clear() logger.info(f"Logout successful for user '{username}' from IP {ip_address}") diff --git a/cwa_book_downloader/metadata_providers/__init__.py b/cwa_book_downloader/metadata_providers/__init__.py index da2b2073..f0726dc5 100644 --- a/cwa_book_downloader/metadata_providers/__init__.py +++ b/cwa_book_downloader/metadata_providers/__init__.py @@ -1,9 +1,9 @@ """Metadata provider plugin system - base classes and registry.""" from abc import ABC, abstractmethod -from dataclasses import dataclass, field, asdict +from dataclasses import dataclass, field from enum import Enum -from typing import List, Optional, Dict, Type, Literal, Any, Union +from typing import Any, Dict, List, Optional, Type, Union class SearchType(str, Enum): @@ -79,24 +79,12 @@ class CheckboxSearchField: SearchField = Union[TextSearchField, NumberSearchField, SelectSearchField, CheckboxSearchField] -def _get_field_type_name(search_field: SearchField) -> str: - """Get the type name for a search field.""" - return search_field.__class__.__name__ - - def serialize_search_field(search_field: SearchField) -> Dict[str, Any]: - """Serialize a search field for API response. - - Args: - search_field: The search field definition. - - Returns: - Dict representation for frontend. - """ + """Serialize a search field to dict for API response.""" result: Dict[str, Any] = { "key": search_field.key, "label": search_field.label, - "type": _get_field_type_name(search_field), + "type": search_field.__class__.__name__, "placeholder": getattr(search_field, 'placeholder', ''), "description": getattr(search_field, 'description', ''), } @@ -116,11 +104,7 @@ def serialize_search_field(search_field: SearchField) -> Dict[str, Any]: @dataclass class MetadataSearchOptions: - """Options for metadata search queries. - - Provides an abstracted interface that works across all metadata providers. - Providers map these options to their specific API parameters. - """ + """Options for metadata search queries across all providers.""" query: str search_type: SearchType = SearchType.GENERAL language: Optional[str] = None # ISO 639-1 code (e.g., "en", "fr") @@ -132,11 +116,7 @@ class MetadataSearchOptions: @dataclass class DisplayField: - """A display field for metadata cards. - - Providers can populate these to show provider-specific metadata - like ratings, page counts, reader counts, etc. - """ + """A display field for metadata cards (ratings, page counts, etc.).""" label: str # e.g., "Rating", "Pages", "Readers" value: str # e.g., "4.5", "496", "8,041" icon: Optional[str] = None # Icon name: "star", "book", "users", "editions" @@ -208,19 +188,7 @@ class MetadataProvider(ABC): @abstractmethod def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: - """Search for books using the provided options. - - Args: - options: Search options including query, type, language, sort, pagination. - - Returns: - List of BookMetadata matching the search criteria. - - Note: - - If search_type is ISBN, this delegates to search_by_isbn() - - Unsupported sort orders fall back to RELEVANCE - - Language filtering is best-effort (not all providers support it) - """ + """Search for books using the provided options.""" pass @abstractmethod @@ -239,17 +207,7 @@ class MetadataProvider(ABC): pass def search_paginated(self, options: MetadataSearchOptions) -> SearchResult: - """Search for books and return results with pagination info. - - Default implementation calls search() and estimates has_more. - Providers should override this to return accurate pagination info. - - Args: - options: Search options including query, type, language, sort, pagination. - - Returns: - SearchResult with books and pagination info. - """ + """Search with pagination info. Override for accurate pagination.""" books = self.search(options) # Heuristic: if we got exactly limit results, there might be more has_more = len(books) >= options.limit @@ -309,18 +267,7 @@ def list_providers() -> List[dict]: def get_provider_kwargs(provider_name: str) -> Dict: - """Get provider-specific initialization kwargs based on configuration. - - Looks up the provider's registered kwargs factory and calls it to get - the configuration. Each provider registers its own factory via - @register_provider_kwargs decorator. - - Args: - provider_name: Name of the provider. - - Returns: - Dict of kwargs to pass to provider constructor. - """ + """Get provider-specific initialization kwargs from registered factory.""" factory = _PROVIDER_KWARGS_FACTORIES.get(provider_name) if factory: return factory() @@ -328,29 +275,12 @@ def get_provider_kwargs(provider_name: str) -> Dict: def is_provider_registered(provider_name: str) -> bool: - """Check if a provider is registered. - - Args: - provider_name: Name of the provider. - - Returns: - True if provider is registered, False otherwise. - """ + """Check if a provider is registered.""" return provider_name in _PROVIDERS def is_provider_enabled(provider_name: str) -> bool: - """Check if a provider is enabled in settings. - - Each provider has an enabled flag (e.g., HARDCOVER_ENABLED, OPENLIBRARY_ENABLED) - that must be explicitly set to True for the provider to be used. - - Args: - provider_name: Name of the provider. - - Returns: - True if provider is enabled, False otherwise. - """ + """Check if a provider is enabled in settings.""" from cwa_book_downloader.core.config import config as app_config # Refresh config to get latest settings @@ -362,31 +292,12 @@ def is_provider_enabled(provider_name: str) -> bool: def get_enabled_providers() -> List[str]: - """Get list of all enabled provider names. - - Returns: - List of enabled provider names. - """ - enabled = [] - for name in _PROVIDERS: - if is_provider_enabled(name): - enabled.append(name) - return enabled + """Get list of all enabled provider names.""" + return [name for name in _PROVIDERS if is_provider_enabled(name)] def get_configured_provider(content_type: str = "ebook") -> Optional[MetadataProvider]: - """Get the currently configured metadata provider, if any. - - Uses the METADATA_PROVIDER config setting to determine which provider - to instantiate. For audiobook content type, uses METADATA_PROVIDER_AUDIOBOOK - if configured, otherwise falls back to METADATA_PROVIDER. - - Args: - content_type: Content type - "ebook" or "audiobook" (default: "ebook") - - Returns: - MetadataProvider instance or None. - """ + """Get the currently configured metadata provider for the content type.""" from cwa_book_downloader.core.config import config as app_config # Refresh config to ensure we have the latest saved settings @@ -422,16 +333,7 @@ def _get_configured_provider_name() -> str: def get_provider_sort_options(provider_name: Optional[str] = None) -> List[Dict[str, str]]: - """Get sort options for a metadata provider. - - Returns a list of {value, label} dicts suitable for frontend dropdowns. - - Args: - provider_name: Provider name. If None, uses configured provider. - - Returns: - List of sort option dicts, or default [relevance] if provider not found. - """ + """Get sort options for a metadata provider as {value, label} dicts.""" if provider_name is None: provider_name = _get_configured_provider_name() @@ -448,16 +350,7 @@ def get_provider_sort_options(provider_name: Optional[str] = None) -> List[Dict[ def get_provider_search_fields(provider_name: Optional[str] = None) -> List[Dict[str, Any]]: - """Get search fields for a metadata provider. - - Returns a list of serialized search field dicts suitable for frontend rendering. - - Args: - provider_name: Provider name. If None, uses configured provider. - - Returns: - List of search field dicts, or empty list if provider not found. - """ + """Get search fields for a metadata provider as serialized dicts.""" if provider_name is None: provider_name = _get_configured_provider_name() @@ -471,16 +364,7 @@ def get_provider_search_fields(provider_name: Optional[str] = None) -> List[Dict def get_provider_default_sort(provider_name: Optional[str] = None) -> str: - """Get the default sort order for a metadata provider. - - Reads from the provider-specific config setting (e.g., HARDCOVER_DEFAULT_SORT). - - Args: - provider_name: Provider name. If None, uses configured provider. - - Returns: - Default sort value string, or "relevance" if not configured. - """ + """Get the default sort order for a metadata provider.""" from cwa_book_downloader.core.config import config as app_config if provider_name is None: diff --git a/cwa_book_downloader/metadata_providers/googlebooks.py b/cwa_book_downloader/metadata_providers/googlebooks.py index ee620628..d17e4b4a 100644 --- a/cwa_book_downloader/metadata_providers/googlebooks.py +++ b/cwa_book_downloader/metadata_providers/googlebooks.py @@ -53,15 +53,7 @@ def _googlebooks_kwargs() -> Dict[str, Any]: @register_provider("googlebooks") class GoogleBooksProvider(MetadataProvider): - """Google Books metadata provider using REST API. - - Attributes: - name: Internal provider identifier. - display_name: Human-readable name for UI. - requires_auth: True - requires API key. - supported_sorts: Only RELEVANCE and NEWEST supported. - search_fields: Author and title search fields. - """ + """Google Books metadata provider using REST API.""" name = "googlebooks" display_name = "Google Books" @@ -81,11 +73,7 @@ class GoogleBooksProvider(MetadataProvider): ] def __init__(self, api_key: Optional[str] = None): - """Initialize provider with API key. - - Args: - api_key: Google Books API key. If not provided, reads from config. - """ + """Initialize provider with optional API key (falls back to config).""" self.api_key = api_key or app_config.get("GOOGLEBOOKS_API_KEY", "") self.session = requests.Session() @@ -94,14 +82,7 @@ class GoogleBooksProvider(MetadataProvider): return bool(self.api_key) def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: - """Search for books using Google Books API. - - Args: - options: Search options (query, type, sort, pagination, fields). - - Returns: - List of BookMetadata objects matching the search. - """ + """Search for books using Google Books API.""" if not self.api_key: logger.warning("Google Books API key not configured") return [] @@ -127,40 +108,29 @@ class GoogleBooksProvider(MetadataProvider): def _search_cached( self, cache_key: str, options: MetadataSearchOptions ) -> List[BookMetadata]: - """Cached search implementation. - - Args: - cache_key: Cache key for this search (includes all options). - options: Search options. - - Returns: - List of BookMetadata objects. - """ + """Cached search implementation.""" # Build query string with Google Books operators author_value = options.fields.get("author", "").strip() title_value = options.fields.get("title", "").strip() query_parts = [] - if author_value and not title_value: - # Author-only search - query_parts.append(f"inauthor:{author_value}") - elif title_value and not author_value: - # Title-only search + # Add field-specific operators + if title_value: query_parts.append(f"intitle:{title_value}") - elif author_value and title_value: - # Both provided - combine - query_parts.append(f"intitle:{title_value}") - query_parts.append(f"inauthor:{author_value}") elif options.search_type == SearchType.TITLE: query_parts.append(f"intitle:{options.query}") + + if author_value: + query_parts.append(f"inauthor:{author_value}") elif options.search_type == SearchType.AUTHOR: query_parts.append(f"inauthor:{options.query}") - else: - # General search + + # Fall back to general search if no specific fields + if not query_parts: query_parts.append(options.query) - query = "+".join(query_parts) if query_parts else options.query + query = "+".join(query_parts) # Build request params params: Dict[str, Any] = { @@ -205,14 +175,7 @@ class GoogleBooksProvider(MetadataProvider): key_prefix="googlebooks:book", ) def get_book(self, book_id: str) -> Optional[BookMetadata]: - """Get book details by Google Books volume ID. - - Args: - book_id: Google Books volume ID. - - Returns: - BookMetadata or None if not found. - """ + """Get book details by Google Books volume ID.""" try: result = self._make_request(f"/volumes/{book_id}", {}) if not result: @@ -230,14 +193,7 @@ class GoogleBooksProvider(MetadataProvider): key_prefix="googlebooks:isbn", ) def search_by_isbn(self, isbn: str) -> Optional[BookMetadata]: - """Search for a book by ISBN. - - Args: - isbn: ISBN-10 or ISBN-13. - - Returns: - BookMetadata or None if not found. - """ + """Search for a book by ISBN-10 or ISBN-13.""" # Clean ISBN (remove hyphens and spaces) clean_isbn = isbn.replace("-", "").replace(" ", "").strip() @@ -266,15 +222,7 @@ class GoogleBooksProvider(MetadataProvider): def _make_request( self, endpoint: str, params: Dict[str, Any] ) -> Optional[Dict[str, Any]]: - """Make authenticated API request. - - Args: - endpoint: API endpoint path (e.g., "/volumes"). - params: Query parameters. - - Returns: - Response JSON or None on error. - """ + """Make authenticated API request to endpoint.""" if not self.api_key: logger.warning("Google Books API key not configured") return None @@ -313,14 +261,7 @@ class GoogleBooksProvider(MetadataProvider): return None def _parse_volume(self, volume: Dict[str, Any]) -> Optional[BookMetadata]: - """Parse a volume object into BookMetadata. - - Args: - volume: Volume data from Google Books API. - - Returns: - BookMetadata or None if parsing fails. - """ + """Parse a volume object into BookMetadata.""" try: volume_id = volume.get("id") volume_info = volume.get("volumeInfo", {}) @@ -419,14 +360,7 @@ class GoogleBooksProvider(MetadataProvider): def _test_googlebooks_connection(current_values: Dict[str, Any] = None) -> Dict[str, Any]: - """Test the Google Books API connection using current form values. - - Args: - current_values: Current unsaved form values from the UI. - - Returns: - Dict with 'success' bool and 'message' string. - """ + """Test the Google Books API connection using current form values.""" current_values = current_values or {} # Use current form values first, fall back to saved config diff --git a/cwa_book_downloader/metadata_providers/hardcover.py b/cwa_book_downloader/metadata_providers/hardcover.py index e6542147..3ea755c2 100644 --- a/cwa_book_downloader/metadata_providers/hardcover.py +++ b/cwa_book_downloader/metadata_providers/hardcover.py @@ -135,11 +135,7 @@ class HardcoverProvider(MetadataProvider): ] def __init__(self, api_key: Optional[str] = None): - """Initialize provider with API key. - - Args: - api_key: Hardcover API key. If not provided, uses config singleton. - """ + """Initialize provider with optional API key (falls back to config).""" raw_key = api_key or app_config.get("HARDCOVER_API_KEY", "") # Strip "Bearer " prefix if user pasted the full auth header from Hardcover self.api_key = raw_key.removeprefix("Bearer ").strip() if raw_key else "" @@ -154,26 +150,32 @@ class HardcoverProvider(MetadataProvider): """Check if provider is configured with an API key.""" return bool(self.api_key) - def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: - """Search for books using Hardcover's search API. + def _build_search_params( + self, default_query: str, author: str, title: str, series: str + ) -> tuple[str, Optional[str], Optional[str]]: + """Build search query, fields, and weights based on provided values. - Args: - options: Search options (query, type, sort, pagination, fields). - - Returns: - List of BookMetadata objects. + Returns (query, fields, weights) tuple. Fields/weights are None for general search. """ + if series and not author and not title: + return series, "series_names", "1" + if author and not title and not series: + return author, "author_names", "1" + if title and not author and not series: + return title, "title,alternative_titles", "5,1" + if author and title and not series: + return f"{title} {author}", "title,alternative_titles,author_names", "5,1,3" + if series: + query = " ".join(p for p in [series, title, author] if p) + return query, "series_names,title,alternative_titles,author_names", "5,3,1,2" + return default_query, None, None + + def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: + """Search for books using Hardcover's search API.""" return self.search_paginated(options).books def search_paginated(self, options: MetadataSearchOptions) -> SearchResult: - """Search for books with pagination info. - - Args: - options: Search options (query, type, sort, pagination, fields). - - Returns: - SearchResult with books and pagination info. - """ + """Search for books with pagination info.""" if not self.api_key: logger.warning("Hardcover API key not configured") return SearchResult(books=[], page=options.page, total_found=0, has_more=False) @@ -193,36 +195,17 @@ class HardcoverProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_SEARCH_TTL", ttl_default=300, key_prefix="hardcover:search") def _search_cached(self, cache_key: str, options: MetadataSearchOptions) -> SearchResult: - """Cached search implementation. - - Args: - cache_key: Cache key (used by decorator). - options: Search options. - - Returns: - SearchResult with books and pagination info. - """ + """Cached search implementation.""" # Determine query and fields based on custom search fields # Note: Hardcover API requires 'weights' when using 'fields' parameter author_value = options.fields.get("author", "").strip() title_value = options.fields.get("title", "").strip() series_value = options.fields.get("series", "").strip() - if series_value and not author_value and not title_value: - query, search_fields, search_weights = series_value, "series_names", "1" - elif author_value and not title_value and not series_value: - query, search_fields, search_weights = author_value, "author_names", "1" - elif title_value and not author_value and not series_value: - query, search_fields, search_weights = title_value, "title,alternative_titles", "5,1" - elif author_value and title_value and not series_value: - query = f"{title_value} {author_value}" - search_fields, search_weights = "title,alternative_titles,author_names", "5,1,3" - elif series_value: - query = " ".join(p for p in [series_value, title_value, author_value] if p) - search_fields, search_weights = "series_names,title,alternative_titles,author_names", "5,3,1,2" - else: - query, search_fields, search_weights = options.query, None, None - + # Build query and field configuration based on which fields are provided + query, search_fields, search_weights = self._build_search_params( + options.query, author_value, title_value, series_value + ) # Build GraphQL query - include fields/weights parameters only when needed if search_fields: @@ -256,7 +239,6 @@ class HardcoverProvider(MetadataProvider): variables["fields"] = search_fields variables["weights"] = search_weights - try: result = self._execute_query(graphql_query, variables) if not result: @@ -314,15 +296,7 @@ class HardcoverProvider(MetadataProvider): return SearchResult(books=[], page=options.page, total_found=0, has_more=False) def _apply_series_ordering(self, books: List[BookMetadata], series_name: str) -> List[BookMetadata]: - """Filter books to exact series match and sort by series position. - - Args: - books: List of books from search results. - series_name: The series name to match. - - Returns: - Filtered and sorted list of books. - """ + """Filter books to exact series match and sort by series position.""" series_name_lower = series_name.lower() books_with_position = [] @@ -353,14 +327,7 @@ class HardcoverProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_BOOK_TTL", ttl_default=600, key_prefix="hardcover:book") def get_book(self, book_id: str) -> Optional[BookMetadata]: - """Get book details by Hardcover ID. - - Args: - book_id: Hardcover book ID. - - Returns: - BookMetadata or None if not found. - """ + """Get book details by Hardcover ID.""" if not self.api_key: logger.warning("Hardcover API key not configured") return None @@ -433,14 +400,7 @@ class HardcoverProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_BOOK_TTL", ttl_default=600, key_prefix="hardcover:isbn") def search_by_isbn(self, isbn: str) -> Optional[BookMetadata]: - """Search for a book by ISBN. - - Args: - isbn: ISBN-10 or ISBN-13. - - Returns: - BookMetadata or None if not found. - """ + """Search for a book by ISBN-10 or ISBN-13.""" if not self.api_key: logger.warning("Hardcover API key not configured") return None @@ -510,15 +470,7 @@ class HardcoverProvider(MetadataProvider): return None def _execute_query(self, query: str, variables: Dict[str, Any]) -> Optional[Dict]: - """Execute a GraphQL query. - - Args: - query: GraphQL query string. - variables: Query variables. - - Returns: - Response data dict or None on error. - """ + """Execute a GraphQL query and return data or None on error.""" try: response = self.session.post( HARDCOVER_API_URL, @@ -549,14 +501,7 @@ class HardcoverProvider(MetadataProvider): return None def _parse_search_result(self, item: Dict) -> Optional[BookMetadata]: - """Parse a search result item into BookMetadata. - - Args: - item: Search result item dict. - - Returns: - BookMetadata or None if parsing fails. - """ + """Parse a search result item into BookMetadata.""" try: book_id = item.get("id") or item.get("document", {}).get("id") title = item.get("title") or item.get("document", {}).get("title") @@ -633,14 +578,7 @@ class HardcoverProvider(MetadataProvider): return None def _parse_book(self, book: Dict) -> BookMetadata: - """Parse a book object into BookMetadata. - - Args: - book: Book data dict from GraphQL response. - - Returns: - BookMetadata object. - """ + """Parse a book object into BookMetadata.""" # Extract authors - try contributions first (filtered), fall back to cached_contributors authors = [] contributions = book.get("contributions") or [] diff --git a/cwa_book_downloader/metadata_providers/openlibrary.py b/cwa_book_downloader/metadata_providers/openlibrary.py index bad0027f..4b7a672f 100644 --- a/cwa_book_downloader/metadata_providers/openlibrary.py +++ b/cwa_book_downloader/metadata_providers/openlibrary.py @@ -1,9 +1,10 @@ """Open Library metadata provider. No API key required, rate limited.""" +import re import time import threading from collections import deque -from typing import Any, Deque, Dict, List, Optional, Union +from typing import Any, Deque, Dict, List, Optional import requests @@ -42,23 +43,14 @@ class RateLimiter: """Simple sliding window rate limiter.""" def __init__(self, max_requests: int, window_seconds: int): - """Initialize rate limiter. - - Args: - max_requests: Maximum requests allowed in the window. - window_seconds: Time window in seconds. - """ + """Initialize rate limiter with max requests per time window.""" self.max_requests = max_requests self.window_seconds = window_seconds self.timestamps: Deque[float] = deque() self.lock = threading.Lock() def wait_if_needed(self) -> None: - """Block until a request is allowed. - - Thread-safe implementation that calculates wait time with lock held, - then sleeps without holding the lock to avoid blocking other threads. - """ + """Block until a request is allowed (thread-safe).""" wait_time = 0 # Calculate wait time with lock held @@ -139,14 +131,7 @@ class OpenLibraryProvider(MetadataProvider): return True def search(self, options: MetadataSearchOptions) -> List[BookMetadata]: - """Search for books using Open Library's search API. - - Args: - options: Search options (query, type, sort, language, pagination, fields). - - Returns: - List of BookMetadata objects. - """ + """Search for books using Open Library's search API.""" # Handle ISBN search separately if options.search_type == SearchType.ISBN: result = self.search_by_isbn(options.query) @@ -159,15 +144,7 @@ class OpenLibraryProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_SEARCH_TTL", ttl_default=300, key_prefix="openlibrary:search") def _search_cached(self, cache_key: str, options: MetadataSearchOptions) -> List[BookMetadata]: - """Cached search implementation. - - Args: - cache_key: Cache key (used by decorator). - options: Search options. - - Returns: - List of BookMetadata objects. - """ + """Cached search implementation.""" _rate_limiter.wait_if_needed() # Build query params @@ -240,14 +217,7 @@ class OpenLibraryProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_BOOK_TTL", ttl_default=600, key_prefix="openlibrary:book") def get_book(self, book_id: str) -> Optional[BookMetadata]: - """Get book details by Open Library work ID. - - Args: - book_id: Open Library work ID (e.g., "OL12345W"). - - Returns: - BookMetadata or None if not found. - """ + """Get book details by Open Library work ID (e.g., 'OL12345W').""" _rate_limiter.wait_if_needed() # Normalize the book_id format @@ -281,14 +251,7 @@ class OpenLibraryProvider(MetadataProvider): @cacheable(ttl_key="METADATA_CACHE_BOOK_TTL", ttl_default=600, key_prefix="openlibrary:isbn") def search_by_isbn(self, isbn: str) -> Optional[BookMetadata]: - """Search for a book by ISBN. - - Args: - isbn: ISBN-10 or ISBN-13. - - Returns: - BookMetadata or None if not found. - """ + """Search for a book by ISBN-10 or ISBN-13.""" # Clean ISBN clean_isbn = isbn.replace("-", "").strip() @@ -343,14 +306,7 @@ class OpenLibraryProvider(MetadataProvider): return None def _parse_search_doc(self, doc: dict) -> Optional[BookMetadata]: - """Parse a search document into BookMetadata. - - Args: - doc: Search result document from Open Library. - - Returns: - BookMetadata or None if parsing fails. - """ + """Parse a search document into BookMetadata.""" try: # Extract work ID from key key = doc.get("key", "") @@ -364,17 +320,10 @@ class OpenLibraryProvider(MetadataProvider): if not isinstance(authors, list): authors = [authors] if authors else [] - # Get ISBNs + # Get ISBNs - find first ISBN-10 and ISBN-13 isbns = doc.get("isbn", []) - isbn_10 = None - isbn_13 = None - for isbn in isbns: - if len(isbn) == 10 and not isbn_10: - isbn_10 = isbn - elif len(isbn) == 13 and not isbn_13: - isbn_13 = isbn - if isbn_10 and isbn_13: - break + isbn_10 = next((i for i in isbns if len(i) == 10), None) + isbn_13 = next((i for i in isbns if len(i) == 13), None) # Get cover URL cover_id = doc.get("cover_i") @@ -426,15 +375,7 @@ class OpenLibraryProvider(MetadataProvider): return None def _parse_work(self, work: dict, work_id: str) -> Optional[BookMetadata]: - """Parse a work object into BookMetadata. - - Args: - work: Work data from Open Library API. - work_id: The work ID. - - Returns: - BookMetadata or None if parsing fails. - """ + """Parse a work object into BookMetadata.""" try: title = work.get("title") if not title: @@ -484,15 +425,7 @@ class OpenLibraryProvider(MetadataProvider): return None def _parse_edition(self, edition: dict, isbn: str) -> Optional[BookMetadata]: - """Parse an edition object into BookMetadata (fallback for ISBN lookup). - - Args: - edition: Edition data from Open Library API. - isbn: The ISBN used for lookup. - - Returns: - BookMetadata or None if parsing fails. - """ + """Parse an edition object into BookMetadata (fallback for ISBN lookup).""" try: title = edition.get("title") if not title: @@ -524,7 +457,6 @@ class OpenLibraryProvider(MetadataProvider): publish_date = edition.get("publish_date", "") if publish_date: # Try to extract year from various formats - import re year_match = re.search(r'\b(19|20)\d{2}\b', publish_date) if year_match: publish_year = int(year_match.group()) @@ -547,14 +479,7 @@ class OpenLibraryProvider(MetadataProvider): return None def _get_author_name(self, author_key: str) -> Optional[str]: - """Get author name from author key. - - Args: - author_key: Open Library author key (e.g., "/authors/OL123A"). - - Returns: - Author name or None. - """ + """Get author name from author key (e.g., '/authors/OL123A').""" _rate_limiter.wait_if_needed() try: diff --git a/cwa_book_downloader/release_sources/__init__.py b/cwa_book_downloader/release_sources/__init__.py index 81510a75..7c443376 100644 --- a/cwa_book_downloader/release_sources/__init__.py +++ b/cwa_book_downloader/release_sources/__init__.py @@ -40,13 +40,7 @@ class Release: @dataclass class DownloadProgress: - """Progress update structure. - - DEPRECATED: This class is deprecated and will be removed. - The new DownloadHandler.download() uses simpler callbacks: - - progress_callback(float) for progress percentage - - status_callback(str, Optional[str]) for status and message - """ + """DEPRECATED: Use progress_callback and status_callback instead.""" status: str # "queued", "resolving", "downloading", "complete", "failed" progress: float # 0-100 status_message: Optional[str] = None @@ -227,18 +221,7 @@ class ReleaseSource(ABC): languages: Optional[List[str]] = None, content_type: str = "ebook" ) -> List[Release]: - """Search for releases of a book. - - Args: - book: Book metadata from provider - expand_search: If True, use broader search (e.g., title+author instead of ISBN). - Not all sources support this - they may ignore it. - languages: Optional list of language codes to filter by. - If provided, overrides book.language and default settings. - Not all sources support this - they may ignore it. - content_type: Content type - "ebook" or "audiobook" (default: "ebook"). - Sources may use this to adjust search categories/filters. - """ + """Search for releases of a book.""" pass @abstractmethod @@ -248,41 +231,13 @@ class ReleaseSource(ABC): @classmethod def get_column_config(cls) -> ReleaseColumnConfig: - """Get the column configuration for this source's release list UI. - - Override this method in subclasses to provide custom columns. - Default implementation returns standard columns (language, format, size). - """ + """Get column configuration for release list UI. Override for custom columns.""" return _default_column_config() class DownloadHandler(ABC): - """Interface for executing downloads from a source. - - ## Staging Architecture - - Handlers are responsible for getting files into the STAGING directory (TMP_DIR). - The orchestrator handles all post-processing and moving to the INGEST directory. - - This means handlers should: - 1. Download/retrieve the file to the staging directory - 2. Return the path to the staged file - 3. NOT move files to the ingest folder (orchestrator does this) - - Examples by source type: - - **Direct downloads**: Download directly to staging dir - - **Torrents**: Copy completed file from torrent client to staging (keep seeding) - - **Usenet**: Move completed file from NZB client to staging - - Use the staging helpers from orchestrator: - - `get_staging_dir()` - Get the staging directory path - - `get_staging_path(task_id, ext)` - Get a staging path for a task - - `stage_file(source, task_id, copy=False)` - Stage a file (copy or move) - - The orchestrator then handles: - - Archive extraction (RAR/ZIP) - - Custom script execution - - Moving to the final ingest folder + """Interface for executing downloads. Handlers stage files to TMP_DIR; + orchestrator handles post-processing and move to INGEST_DIR. """ @abstractmethod @@ -293,22 +248,7 @@ class DownloadHandler(ABC): progress_callback: Callable[[float], None], status_callback: Callable[[str, Optional[str]], None] ) -> Optional[str]: - """ - Execute download and return path to STAGED file. - - Handlers should download/copy files to the staging directory (TMP_DIR), - NOT directly to the ingest folder. The orchestrator handles post-processing - (archive extraction, custom scripts) and final move to ingest. - - Args: - task: The download task with task_id and display info - cancel_flag: Event to check for cancellation - progress_callback: Called with progress percentage (0-100) - status_callback: Called with (status, message) for status updates - - Returns: - Path to staged file (in TMP_DIR) if successful, None otherwise - """ + """Execute download and return path to staged file in TMP_DIR.""" pass @abstractmethod @@ -354,11 +294,7 @@ def get_handler(name: str) -> DownloadHandler: def list_available_sources() -> List[dict]: - """For frontend - list all registered sources with their status. - - Returns all sources (not just available ones) so the frontend can show - appropriate UI for disabled/unconfigured sources instead of hiding them. - """ + """List all registered sources with their availability status.""" result = [] for name, src_class in _SOURCES.items(): instance = src_class() diff --git a/cwa_book_downloader/release_sources/direct_download.py b/cwa_book_downloader/release_sources/direct_download.py index dffe7c2d..378a9400 100644 --- a/cwa_book_downloader/release_sources/direct_download.py +++ b/cwa_book_downloader/release_sources/direct_download.py @@ -2,12 +2,11 @@ import itertools import json -import os import re import time from pathlib import Path from threading import Event -from typing import Callable, Dict, List, Optional, Tuple +from typing import Callable, Dict, List, Optional from urllib.parse import quote from bs4 import BeautifulSoup, NavigableString, Tag @@ -84,29 +83,16 @@ def _is_source_enabled(source_id: str) -> bool: return False -def _get_enabled_source_order() -> List[str]: - """Get ordered list of enabled source IDs.""" - return [ - item["id"] - for item in _get_source_priority() - if item.get("enabled", True) - ] +_SIZE_UNIT_PATTERN = re.compile(r'(kb|mb|gb|tb)', re.IGNORECASE) -def _get_source_position(source_id: str) -> int: - """Get the position of a source in the priority list (lower = higher priority). - - Returns 999 if source not found or disabled. - """ - for i, item in enumerate(_get_source_priority()): - if item["id"] == source_id and item.get("enabled", True): - return i - return 999 +def _normalize_size(size_str: str) -> str: + """Normalize size string by uppercasing units (e.g., '5.2 mb' -> '5.2 MB').""" + return _SIZE_UNIT_PATTERN.sub(lambda m: m.group(1).upper(), size_str.strip()) class SearchUnavailable(Exception): """Raised when Anna's Archive cannot be reached via any mirror/DNS.""" - pass def search_books(query: str, filters: SearchFilters) -> List[BookInfo]: @@ -148,11 +134,9 @@ def search_books(query: str, filters: SearchFilters) -> List[BookInfo]: index = 1 for filter_type, filter_values in vars(filters).items(): - if (filter_type == "author" or filter_type == "title") and filter_values: + if filter_type in ("author", "title") and filter_values: for value in filter_values: - filters_query += ( - f"&termtype_{index}={filter_type}&termval_{index}={quote(value)}" - ) + filters_query += f"&termtype_{index}={filter_type}&termval_{index}={quote(value)}" index += 1 selector = network.AAMirrorSelector() @@ -272,26 +256,25 @@ def _parse_book_info_page(soup: BeautifulSoup, book_id: str, fetch_download_coun data = soup.find_all("div", {"class": "main-inner"})[0].find_next("div") divs = list(data.children) - slow_urls_no_waitlist: List[str] = [] - slow_urls_with_waitlist: List[str] = [] - - def _append_unique(lst: List[str], href: str) -> None: - if href and href not in lst: - lst.append(href) + slow_urls_no_waitlist: set[str] = set() + slow_urls_with_waitlist: set[str] = set() for anchor in soup.find_all("a"): try: text = anchor.text.strip().lower() href = anchor.get("href", "") + if not href: + continue + next_text = "" if anchor.next and anchor.next.next: next_text = getattr(anchor.next.next, 'text', str(anchor.next.next)).strip().lower() if text.startswith("slow partner server") and "waitlist" in next_text: if "no waitlist" in next_text: - _append_unique(slow_urls_no_waitlist, href) + slow_urls_no_waitlist.add(href) else: - _append_unique(slow_urls_with_waitlist, href) + slow_urls_with_waitlist.add(href) except Exception: pass @@ -331,9 +314,8 @@ def _parse_book_info_page(soup: BeautifulSoup, book_id: str, fetch_download_coun for f in _details: if format == "" and f.strip().lower() in config.SUPPORTED_FORMATS: format = f.strip().lower() - if size == "" and any(u in f.strip().lower() for u in ["mb", "kb", "gb"]): - # Preserve original case but uppercase the unit (e.g., "5.2 mb" -> "5.2 MB") - size = re.sub(r'(kb|mb|gb|tb)', lambda m: m.group(1).upper(), f.strip(), flags=re.IGNORECASE) + if size == "" and any(u in f.strip().lower() for u in ("mb", "kb", "gb")): + size = _normalize_size(f) if content == "": for ct in CONTENT_TYPES: if ct in f.strip().lower(): @@ -345,8 +327,7 @@ def _parse_book_info_page(soup: BeautifulSoup, book_id: str, fetch_download_coun if format == "" and stripped and " " not in stripped: format = stripped if size == "" and "." in stripped: - # Uppercase any size units - size = re.sub(r'(kb|mb|gb|tb)', lambda m: m.group(1).upper(), f.strip(), flags=re.IGNORECASE) + size = _normalize_size(f) book_title = _find_in_divs(divs, "🔍")[0].strip("🔍").strip() @@ -456,43 +437,28 @@ def _extract_book_description(soup: BeautifulSoup) -> Optional[str]: def _extract_book_metadata(metadata_divs) -> Dict[str, List[str]]: """Extract metadata from book info divs.""" - info: Dict[str, List[str]] = {} + info: Dict[str, set[str]] = {} - # Process the first set of metadata sub_datas = metadata_divs.find_all("div")[0] - sub_datas = list(sub_datas.children) - for sub_data in sub_datas: + for sub_data in sub_datas.children: if sub_data.text.strip() == "": continue - sub_data = list(sub_data.children) - key = sub_data[0].text.strip() - value = sub_data[1].text.strip() + children = list(sub_data.children) + key = children[0].text.strip() + value = children[1].text.strip() if key not in info: info[key] = set() info[key].add(value) - # make set into list - for key, value in info.items(): - info[key] = list(value) - - # Filter relevant metadata - relevant_prefixes = [ - "ISBN-", - "ALTERNATIVE", - "ASIN", - "Goodreads", - "Language", - "Year", - ] + relevant_prefixes = ("isbn-", "alternative", "asin", "goodreads", "language", "year") return { - k.strip(): v + k.strip(): list(v) for k, v in info.items() - if any(k.lower().startswith(prefix.lower()) for prefix in relevant_prefixes) - and "filename" not in k.lower() + if k.lower().startswith(relevant_prefixes) and "filename" not in k.lower() } -def _get_source_info(link: str) -> Tuple[str, str]: +def _get_source_info(link: str) -> tuple[str, str]: """Get source label and friendly name for a download link. Args: @@ -514,41 +480,32 @@ def _get_source_info(link: str) -> Tuple[str, str]: return "unknown", "Mirror" -def _label_source(link: str) -> str: - """Get lightweight source tag for logging/metrics.""" - return _get_source_info(link)[0] - - def _friendly_source_name(link: str) -> str: """Get user-friendly name for a download source.""" return _get_source_info(link)[1] +def _group_urls_by_source(urls: List[str], urls_by_source: Dict[str, List[str]]) -> None: + """Group URLs into urls_by_source dict by their source type.""" + for url in urls: + source_type = _url_source_types.get(url) + if source_type: + urls_by_source.setdefault(source_type, []).append(url) + + def _fetch_aa_page_urls(book_info: BookInfo, urls_by_source: Dict[str, List[str]]) -> None: """Fetch and parse AA page, populating urls_by_source dict. Groups existing book_info.download_urls by source type. If book_info has no URLs, fetches the AA page fresh. """ - # If book_info already has URLs, group them by source type if book_info.download_urls: - for url in book_info.download_urls: - source_type = _url_source_types.get(url) - if source_type: - if source_type not in urls_by_source: - urls_by_source[source_type] = [] - urls_by_source[source_type].append(url) + _group_urls_by_source(book_info.download_urls, urls_by_source) return - # Otherwise fetch the page fresh try: fresh_book_info = get_book_info(book_info.id, fetch_download_count=False) - for url in fresh_book_info.download_urls: - source_type = _url_source_types.get(url) - if source_type: - if source_type not in urls_by_source: - urls_by_source[source_type] = [] - urls_by_source[source_type].append(url) + _group_urls_by_source(fresh_book_info.download_urls, urls_by_source) except Exception as e: logger.warning(f"Failed to fetch AA page: {e}") @@ -560,7 +517,6 @@ def _get_urls_for_source( cancel_flag: Optional[Event], status_callback: Optional[Callable[[str, Optional[str]], None]], urls_by_source: Dict[str, List[str]], - aa_page_fetched: bool ) -> List[str]: """Get URLs for a specific source, fetching lazily if needed.""" # AA Fast - generate URL dynamically @@ -585,7 +541,7 @@ def _get_urls_for_source( # AA page sources - fetch AA page if not already done if source_id in _AA_PAGE_SOURCES: - if not aa_page_fetched and not urls_by_source: + if not urls_by_source: if status_callback: status_callback("resolving", "Fetching download sources") _fetch_aa_page_urls(book_info, urls_by_source) @@ -686,7 +642,6 @@ def _download_book( selector = network.AAMirrorSelector() source_failures: dict[str, int] = {} urls_by_source: dict[str, list[str]] = {} - aa_page_fetched = False url_attempt_counter = 0 # Get enabled sources in priority order @@ -716,13 +671,9 @@ def _download_book( # Get URLs for this source (lazy-loads as needed) urls_to_try = _get_urls_for_source( source_id, book_info, selector, cancel_flag, status_callback, - urls_by_source, aa_page_fetched + urls_by_source, ) - # Track if we fetched AA page - if source_id in _AA_PAGE_SOURCES and not aa_page_fetched: - aa_page_fetched = bool(urls_by_source) - if not urls_to_try: continue @@ -841,7 +792,7 @@ def _extract_slow_download_url( # The URL appears as plain text in http://... for span in soup.find_all("span", class_=lambda c: c and "bg-gray-200" in c): text = span.get_text(strip=True) - if text.startswith("http://") or text.startswith("https://"): + if text.startswith(("http://", "https://")): return text # Try "copy this URL" pattern (legacy) @@ -875,14 +826,8 @@ def _extract_slow_download_url( logger.info(f"AA waitlist: {sleep_time}s for {title}") # Live countdown with status updates - remaining = sleep_time - while remaining > 0: - # Format countdown message with source context - if source_context: - wait_msg = f"{source_context} - Waiting {remaining}s" - else: - wait_msg = f"Waiting {remaining}s" - + for remaining in range(sleep_time, 0, -1): + wait_msg = f"{source_context} - Waiting {remaining}s" if source_context else f"Waiting {remaining}s" if status_callback: status_callback("resolving", wait_msg) @@ -891,8 +836,6 @@ def _extract_slow_download_url( logger.info(f"Cancelled wait for {title}") return "" - remaining -= 1 - # After countdown, update status and re-fetch if status_callback and source_context: status_callback("resolving", f"{source_context} - Fetching") diff --git a/cwa_book_downloader/release_sources/irc/client.py b/cwa_book_downloader/release_sources/irc/client.py index 52d4de15..de49ef44 100644 --- a/cwa_book_downloader/release_sources/irc/client.py +++ b/cwa_book_downloader/release_sources/irc/client.py @@ -72,24 +72,7 @@ class IRCConnectionError(IRCError): class IRCClient: - """Minimal IRC client for IRC Highway ebook searches. - - Designed for per-request connections - connect, do operation, disconnect. - Not intended for long-lived connections. - - Usage: - client = IRCClient(nick="mybot") - client.connect() - client.join_channel("ebooks") - client.send_message("#ebooks", "@search harry potter") - - for msg in client.read_messages(): - if msg.event == IRCEvent.SEARCH_RESULT: - offer = parse_dcc_send(msg.raw) - break - - client.disconnect() - """ + """Minimal IRC client for per-request ebook searches on IRC Highway.""" def __init__( self, @@ -119,16 +102,7 @@ class IRCClient: return f"cwa_{suffix}" def connect(self) -> None: - """Connect to IRC server and authenticate. - - Connection sequence: - 1. TCP/TLS connect - 2. Send USER and NICK - 3. Wait for server welcome messages - - Raises: - IRCConnectionError: If connection fails - """ + """Connect to IRC server, send USER/NICK, and wait for welcome.""" logger.info(f"Connecting to {self.server}:{self.port} (TLS={self.use_tls})") try: @@ -179,14 +153,7 @@ class IRCClient: logger.info("Disconnected from IRC") def join_channel(self, channel: str, wait_for_join: bool = True) -> None: - """Join an IRC channel. - - Args: - channel: Channel name without # prefix - wait_for_join: If True, wait for server confirmation (366 message) - - Also captures the channel's user list to track online servers. - """ + """Join an IRC channel (without # prefix) and capture online servers.""" self._send(f"JOIN #{channel}") logger.debug(f"Sent JOIN #{channel}") @@ -228,41 +195,20 @@ class IRCClient: logger.warning(f"Joined #{channel} (no confirmation received)") def send_message(self, target: str, message: str) -> None: - """Send a PRIVMSG to a channel or user. - - Args: - target: Channel (with #) or user nick - message: Message content - """ + """Send a PRIVMSG to a channel or user.""" self._send(f"PRIVMSG {target} :{message}") logger.debug(f"Sent to {target}: {message[:50]}...") def send_notice(self, target: str, message: str) -> None: - """Send a NOTICE to a user. - - Args: - target: User nick - message: Notice content - """ + """Send a NOTICE to a user.""" self._send(f"NOTICE {target} :{message}") def request_names(self, channel: str) -> None: - """Request user list for a channel. - - Args: - channel: Channel name without # prefix - """ + """Request user list for a channel (without # prefix).""" self._send(f"NAMES #{channel}") def _parse_names_list(self, names_data: str) -> None: - """Parse NAMES list and extract elevated users (download servers). - - IRC NAMES reply format (353): - :server 353 nick = #channel :@user1 +user2 user3 ... - - Users with prefixes (~, &, @, %, +) are elevated (ops/voice). - These are the download bots/servers. - """ + """Parse 353 NAMES reply and extract elevated users (download servers).""" # Extract the trailing part after the last colon (the actual names) if ' :' in names_data: names_part = names_data.split(' :')[-1] @@ -270,9 +216,6 @@ class IRCClient: names_part = names_data for name in names_part.split(): - if not name: - continue - # Check if user has an elevated prefix if name[0] in ELEVATED_PREFIXES: # Strip the prefix to get the actual nick @@ -288,11 +231,7 @@ class IRCClient: self._socket.sendall(data) def _recv_lines(self) -> Iterator[str]: - """Receive and yield complete IRC lines. - - IRC messages are delimited by \\r\\n. We buffer partial - reads and yield complete lines as they arrive. - """ + """Receive and yield complete CRLF-delimited IRC lines.""" while True: # Check if we have a complete line in buffer while '\r\n' in self._buffer: @@ -344,11 +283,7 @@ class IRCClient: return msg def _classify_event(self, msg: IRCMessage) -> IRCEvent: - """Classify message into event type. - - Uses simple string containment checks for robustness - rather than strict IRC protocol parsing. - """ + """Classify message into event type using string containment checks.""" raw = msg.raw trailing = msg.trailing or "" @@ -370,9 +305,7 @@ class IRCClient: return IRCEvent.MATCHES_FOUND # User list (RPL_NAMREPLY and RPL_ENDOFNAMES) - if msg.command == "353": - return IRCEvent.SERVER_LIST - if msg.command == "366": + if msg.command in ("353", "366"): return IRCEvent.SERVER_LIST # Server PING @@ -401,14 +334,7 @@ class IRCClient: logger.debug(f"Sent VERSION to {sender}") def read_messages(self, auto_handle: bool = True) -> Iterator[IRCMessage]: - """Read and yield IRC messages. - - Args: - auto_handle: If True, automatically handle PING and VERSION - - Yields: - IRCMessage objects for each received message - """ + """Read and yield IRC messages, optionally auto-handling PING/VERSION.""" for line in self._recv_lines(): msg = self._parse_message(line) @@ -429,15 +355,7 @@ class IRCClient: timeout: float = 60.0, result_type: bool = False, ) -> Optional[DCCOffer]: - """Wait for a DCC SEND offer. - - Args: - timeout: Maximum time to wait in seconds - result_type: If True, wait for SEARCH_RESULT; else BOOK_RESULT - - Returns: - DCCOffer if received, None if timeout - """ + """Wait for a DCC SEND offer. Returns None on timeout or no results.""" target_event = IRCEvent.SEARCH_RESULT if result_type else IRCEvent.BOOK_RESULT start = time.time() @@ -459,12 +377,12 @@ class IRCClient: if msg.event == IRCEvent.NO_RESULTS: logger.info("Server reports no results") return None - if msg.event == IRCEvent.BAD_SERVER: + elif msg.event == IRCEvent.BAD_SERVER: logger.warning("Server unavailable") return None - if msg.event == IRCEvent.SEARCH_ACCEPTED: + elif msg.event == IRCEvent.SEARCH_ACCEPTED: logger.info("Search accepted, waiting for results...") - if msg.event == IRCEvent.MATCHES_FOUND: + elif msg.event == IRCEvent.MATCHES_FOUND: # Extract count from "returned X matches" if msg.trailing and "returned" in msg.trailing: try: diff --git a/cwa_book_downloader/release_sources/irc/dcc.py b/cwa_book_downloader/release_sources/irc/dcc.py index 295c9924..087a7dce 100644 --- a/cwa_book_downloader/release_sources/irc/dcc.py +++ b/cwa_book_downloader/release_sources/irc/dcc.py @@ -60,29 +60,13 @@ class DCCConnectionError(DCCError): def int_to_ip(ip_int: int) -> str: - """Convert 32-bit integer to dotted IP notation. - - DCC protocol sends IP addresses as 32-bit unsigned integers - in network byte order (big-endian). - - Example: 2760158537 -> "164.132.173.73" - """ + """Convert 32-bit integer (DCC format) to dotted IP notation.""" packed = struct.pack('>I', ip_int) return '.'.join(str(b) for b in packed) def parse_dcc_send(text: str) -> DCCOffer: - """Parse a DCC SEND message into a DCCOffer. - - Args: - text: Full IRC message containing DCC SEND - - Returns: - DCCOffer with filename, ip, port, size - - Raises: - DCCParseError: If message doesn't match expected format - """ + """Parse a DCC SEND message into a DCCOffer. Raises DCCParseError on failure.""" match = DCC_REGEX.search(text) if not match: raise DCCParseError(f"Invalid DCC SEND format: {text[:100]}") @@ -107,23 +91,7 @@ def download_dcc( cancel_flag: Optional[Event] = None, timeout: float = 30.0, ) -> None: - """Download file via DCC protocol. - - Uses a custom read loop with 4096-byte buffer which is faster than - Python's shutil.copyfileobj since DCC servers don't properly signal EOF. - - Args: - offer: Parsed DCC offer with connection details - dest_path: Where to save the file - progress_callback: Called with percentage (0-100) during download - cancel_flag: If set, abort the download - timeout: Socket timeout in seconds - - Raises: - DCCConnectionError: Failed to connect to sender - DCCSizeError: Downloaded bytes != expected size - DCCError: Other socket/IO errors - """ + """Download file via DCC protocol to dest_path. Raises DCCError on failure.""" logger.info(f"DCC connecting to {offer.ip}:{offer.port} for {offer.filename}") try: diff --git a/cwa_book_downloader/release_sources/irc/handler.py b/cwa_book_downloader/release_sources/irc/handler.py index 3808417d..e574d9ac 100644 --- a/cwa_book_downloader/release_sources/irc/handler.py +++ b/cwa_book_downloader/release_sources/irc/handler.py @@ -29,20 +29,7 @@ class IRCDownloadHandler(DownloadHandler): progress_callback: Callable[[float], None], status_callback: Callable[[str, Optional[str]], None], ) -> Optional[str]: - """Download a book via IRC DCC. - - The task.task_id contains the full IRC request string, - e.g., "!ServerName Author - Title.epub ::INFO:: 2.5MB" - - Args: - task: Download task with IRC request info - cancel_flag: Set to cancel download - progress_callback: Report progress 0-100 - status_callback: Report status messages - - Returns: - Path to downloaded file, or None on failure - """ + """Download a book via IRC DCC. task.task_id contains the IRC request string.""" download_request = task.task_id logger.info(f"IRC download: {download_request[:60]}...") @@ -135,10 +122,6 @@ class IRCDownloadHandler(DownloadHandler): return None def cancel(self, task_id: str) -> bool: - """Cancel an in-progress download. - - Note: Actual cancellation is handled via the cancel_flag in download(). - This method is for cleanup if the cancel_flag mechanism fails. - """ + """Cancel an in-progress download (cleanup if cancel_flag fails).""" logger.debug(f"Cancel requested for IRC task: {task_id}") return True diff --git a/cwa_book_downloader/release_sources/irc/parser.py b/cwa_book_downloader/release_sources/irc/parser.py index 9cd15c4a..cf851182 100644 --- a/cwa_book_downloader/release_sources/irc/parser.py +++ b/cwa_book_downloader/release_sources/irc/parser.py @@ -7,7 +7,7 @@ import re import zipfile from dataclasses import dataclass from pathlib import Path -from typing import List, Optional +from typing import Optional from cwa_book_downloader.core.config import config from cwa_book_downloader.core.logger import setup_logger @@ -29,12 +29,12 @@ ALL_RECOGNIZED_FORMATS = { } -def _get_supported_formats() -> List[str]: +def _get_supported_formats() -> set[str]: """Get user's configured supported formats from settings.""" formats = config.get("SUPPORTED_FORMATS", ["epub", "mobi", "azw3", "fb2", "djvu", "cbz", "cbr"]) if isinstance(formats, str): - return [fmt.strip().lower() for fmt in formats.split(",") if fmt.strip()] - return [fmt.lower() for fmt in formats] + return {fmt.strip().lower() for fmt in formats.split(",") if fmt.strip()} + return {fmt.lower() for fmt in formats} # Regex to parse result lines # Format: !Server Author - Title.format ::INFO:: size @@ -75,14 +75,7 @@ class SearchResult: def parse_result_line(line: str) -> Optional[SearchResult]: - """Parse a single search result line. - - Args: - line: Raw line from search results file - - Returns: - SearchResult if parseable, None otherwise - """ + """Parse a single search result line. Returns None if unparseable.""" line = line.strip() # Must start with ! @@ -148,16 +141,9 @@ def parse_result_line(line: str) -> Optional[SearchResult]: def parse_results_file(content: str) -> list[SearchResult]: - """Parse a search results file. - - Args: - content: Full file content - - Returns: - List of parsed SearchResult objects - """ + """Parse a search results file into SearchResult objects.""" results = [] - supported = set(_get_supported_formats()) + supported = _get_supported_formats() for line in content.splitlines(): result = parse_result_line(line) @@ -171,16 +157,7 @@ def parse_results_file(content: str) -> list[SearchResult]: def extract_results_from_zip(zip_path: Path) -> str: - """Extract and return content from a search results ZIP. - - Search results are sent as ZIP files containing a single text file. - - Args: - zip_path: Path to downloaded ZIP file - - Returns: - Text content of the results file - """ + """Extract and return text content from a search results ZIP.""" with zipfile.ZipFile(zip_path, 'r') as zf: # Should contain exactly one text file names = zf.namelist() diff --git a/cwa_book_downloader/release_sources/irc/source.py b/cwa_book_downloader/release_sources/irc/source.py index 07f6491b..e03c1bd5 100644 --- a/cwa_book_downloader/release_sources/irc/source.py +++ b/cwa_book_downloader/release_sources/irc/source.py @@ -70,7 +70,7 @@ class IRCReleaseSource(ReleaseSource): def __init__(self): # Track online servers from most recent search - self._online_servers: Optional[list[str]] = None + self._online_servers: Optional[set[str]] = None @classmethod def is_available(cls) -> bool: @@ -78,11 +78,7 @@ class IRCReleaseSource(ReleaseSource): return config.get("IRC_ENABLED", False) def get_column_config(self) -> ReleaseColumnConfig: - """Configure UI columns for IRC results. - - Includes online_servers from the most recent search, allowing - the frontend to show status indicators for each server. - """ + """Configure UI columns for IRC results.""" return ReleaseColumnConfig( columns=[ ColumnSchema( @@ -124,17 +120,7 @@ class IRCReleaseSource(ReleaseSource): languages: Optional[List[str]] = None, content_type: str = "ebook" ) -> List[Release]: - """Search IRC Highway for books matching metadata. - - Args: - book: Book metadata (title, authors, etc.) - expand_search: Ignored - IRC always uses title+author search - languages: Ignored - IRC doesn't support language filtering - content_type: Ignored - IRC doesn't differentiate content types - - Returns: - List of matching releases - """ + """Search IRC Highway for books matching metadata.""" # Build search query query = self._build_query(book) if not query: @@ -241,15 +227,9 @@ class IRCReleaseSource(ReleaseSource): } def _convert_to_releases(self, results: List[SearchResult]) -> List[Release]: - """Convert parsed results to Release objects. - - Results are sorted by: - 1. Online status (online servers first) - 2. Format priority (epub > mobi > azw3 > ...) - 3. Server name (alphabetically) - """ + """Convert parsed results to Release objects, sorted by online/format/server.""" releases = [] - online_servers = self._online_servers or set() + online_servers = self._online_servers if self._online_servers else set() for result in results: release = Release( @@ -287,10 +267,7 @@ class IRCReleaseSource(ReleaseSource): @staticmethod def _parse_size(size_str: str) -> Optional[int]: - """Parse human-readable size to bytes. - - Handles formats like: 1.2MB, 1.2M, 500KB, 500K, 1GB, 1G, etc. - """ + """Parse human-readable size (e.g., '1.2MB', '500K') to bytes.""" if not size_str: return None diff --git a/cwa_book_downloader/release_sources/prowlarr/api.py b/cwa_book_downloader/release_sources/prowlarr/api.py index 601f1d03..bbcbb0d4 100644 --- a/cwa_book_downloader/release_sources/prowlarr/api.py +++ b/cwa_book_downloader/release_sources/prowlarr/api.py @@ -1,14 +1,7 @@ -""" -Prowlarr API client. - -Handles communication with the Prowlarr API for: -- Connection testing -- Indexer listing -- Book search -""" +"""Prowlarr API client for connection testing, indexer listing, and search.""" from typing import Any, Dict, List, Optional, Tuple -from urllib.parse import urlencode, urljoin +from urllib.parse import urljoin import requests @@ -21,14 +14,6 @@ class ProwlarrClient: """Client for interacting with the Prowlarr API.""" def __init__(self, url: str, api_key: str, timeout: int = 30): - """ - Initialize the Prowlarr client. - - Args: - url: Base URL of the Prowlarr instance (e.g., http://prowlarr:9696) - api_key: Prowlarr API key - timeout: Request timeout in seconds - """ self.base_url = url.rstrip("/") self.api_key = api_key self.timeout = timeout @@ -45,22 +30,7 @@ class ProwlarrClient: params: Optional[Dict[str, Any]] = None, json_data: Optional[Dict[str, Any]] = None, ) -> Any: - """ - Make an API request to Prowlarr. - - Args: - method: HTTP method (GET, POST, etc.) - endpoint: API endpoint (e.g., /api/v1/search) - params: Query parameters - json_data: JSON body data - - Returns: - Parsed JSON response - - Raises: - requests.RequestException: On network errors - ValueError: On invalid JSON response - """ + """Make an API request to Prowlarr. Returns parsed JSON response.""" url = urljoin(self.base_url, endpoint) logger.debug(f"Prowlarr API: {method} {url}") @@ -94,12 +64,7 @@ class ProwlarrClient: raise def test_connection(self) -> Tuple[bool, str]: - """ - Test the connection to Prowlarr. - - Returns: - Tuple of (success: bool, message: str) - """ + """Test connection to Prowlarr. Returns (success, message).""" logger.info(f"Testing Prowlarr connection to: {self.base_url}") try: data = self._request("GET", "/api/v1/system/status") @@ -117,12 +82,7 @@ class ProwlarrClient: return False, f"Connection failed: {str(e)}" def get_indexers(self) -> List[Dict[str, Any]]: - """ - Get all configured indexers. - - Returns: - List of indexer configurations with id, name, protocol, enabled status - """ + """Get all configured indexers.""" try: indexers = self._request("GET", "/api/v1/indexer") return indexers @@ -131,12 +91,7 @@ class ProwlarrClient: return [] def get_enabled_indexers(self) -> List[Dict[str, Any]]: - """ - Get enabled indexers with book-related info. - - Returns: - List of enabled indexers with simplified structure - """ + """Get enabled indexers with book capability info.""" indexers = self.get_indexers() result = [] @@ -144,20 +99,9 @@ class ProwlarrClient: if not idx.get("enable", False): continue - # Check for book categories - capabilities = idx.get("capabilities", {}) - categories = capabilities.get("categories", []) - has_books = False - - for cat in categories: - cat_id = cat.get("id", 0) - if 7000 <= cat_id <= 7999: - has_books = True - break - for subcat in cat.get("subCategories", []): - if 7000 <= subcat.get("id", 0) <= 7999: - has_books = True - break + # Check for book categories (7000-7999 range) + categories = idx.get("capabilities", {}).get("categories", []) + has_books = self._has_book_categories(categories) result.append({ "id": idx.get("id"), @@ -168,6 +112,17 @@ class ProwlarrClient: return result + def _has_book_categories(self, categories: List[Dict[str, Any]]) -> bool: + """Check if any category or subcategory is in the book range (7000-7999).""" + for cat in categories: + cat_id = cat.get("id", 0) + if 7000 <= cat_id <= 7999: + return True + for subcat in cat.get("subCategories", []): + if 7000 <= subcat.get("id", 0) <= 7999: + return True + return False + def search( self, query: str, @@ -175,33 +130,19 @@ class ProwlarrClient: categories: Optional[List[int]] = None, limit: int = 100, ) -> List[Dict[str, Any]]: - """ - Search for releases via Prowlarr. - - Args: - query: Search query - indexer_ids: Specific indexer IDs to search (required for targeted search) - categories: Category IDs to filter by (optional) - limit: Maximum number of results to return (default: 100) - - Returns: - List of search results - """ + """Search for releases via Prowlarr.""" if not query: return [] - params = {"query": query, "limit": limit} + params: Dict[str, Any] = {"query": query, "limit": limit} if indexer_ids: params["indexerIds"] = indexer_ids if categories: params["categories"] = categories - endpoint = f"/api/v1/search?{urlencode(params, doseq=True)}" - try: - results = self._request("GET", endpoint) + results = self._request("GET", "/api/v1/search", params=params) return results if isinstance(results, list) else [] - except Exception as e: logger.error(f"Prowlarr search failed: {e}") return [] diff --git a/cwa_book_downloader/release_sources/prowlarr/clients/nzbget.py b/cwa_book_downloader/release_sources/prowlarr/clients/nzbget.py index c715e844..19bd7332 100644 --- a/cwa_book_downloader/release_sources/prowlarr/clients/nzbget.py +++ b/cwa_book_downloader/release_sources/prowlarr/clients/nzbget.py @@ -4,6 +4,7 @@ NZBGet download client for Prowlarr integration. Uses NZBGet's JSON-RPC API directly via requests (no external dependency). """ +import json from typing import Any, Optional, Tuple import requests @@ -60,8 +61,6 @@ class NZBGetClient(DownloadClient): """ rpc_url = f"{self.url}/jsonrpc" - # Build JSON-RPC 2.0 request - import json payload = json.dumps({ "jsonrpc": "2.0", "id": 1, diff --git a/cwa_book_downloader/release_sources/prowlarr/clients/qbittorrent.py b/cwa_book_downloader/release_sources/prowlarr/clients/qbittorrent.py index 4adda82e..62956c75 100644 --- a/cwa_book_downloader/release_sources/prowlarr/clients/qbittorrent.py +++ b/cwa_book_downloader/release_sources/prowlarr/clients/qbittorrent.py @@ -1,6 +1,7 @@ """qBittorrent download client for Prowlarr integration.""" import time +from types import SimpleNamespace from typing import List, Optional, Tuple from cwa_book_downloader.core.config import config @@ -69,13 +70,7 @@ class QBittorrentClient(DownloadClient): ) response.raise_for_status() torrents = response.json() - - class TorrentInfo: - def __init__(self, data): - for key, value in data.items(): - setattr(self, key, value) - - return [TorrentInfo(t) for t in torrents] + return [SimpleNamespace(**t) for t in torrents] except requests.exceptions.HTTPError as e: if e.response is not None and e.response.status_code == 403: logger.warning("qBittorrent auth failed - check credentials") diff --git a/cwa_book_downloader/release_sources/prowlarr/clients/sabnzbd.py b/cwa_book_downloader/release_sources/prowlarr/clients/sabnzbd.py index 4ff4036e..583ed5bd 100644 --- a/cwa_book_downloader/release_sources/prowlarr/clients/sabnzbd.py +++ b/cwa_book_downloader/release_sources/prowlarr/clients/sabnzbd.py @@ -19,6 +19,49 @@ from cwa_book_downloader.release_sources.prowlarr.clients import ( logger = setup_logger(__name__) +def _parse_eta(eta_str: str) -> Optional[int]: + """Parse SABnzbd ETA string (format: 'H:MM:SS') to seconds.""" + if not eta_str or eta_str == "0:00:00": + return None + try: + parts = eta_str.split(":") + if len(parts) == 3: + return int(parts[0]) * 3600 + int(parts[1]) * 60 + int(parts[2]) + except (ValueError, IndexError): + pass + return None + + +def _parse_speed(slot: dict) -> Optional[int]: + """Parse download speed from SABnzbd slot data, returning bytes/sec.""" + # Prefer kbpersec field (more reliable numeric value) + kbpersec_str = slot.get("kbpersec", "") + if kbpersec_str: + try: + return int(float(kbpersec_str) * 1024) + except (ValueError, TypeError): + pass + + # Fall back to human-readable speed field + speed_str = slot.get("speed", "") + if not speed_str: + return None + + try: + speed_parts = speed_str.split() + if len(speed_parts) < 2: + return None + speed_val = float(speed_parts[0]) + unit = speed_parts[1].upper() + multipliers = {"K": 1024, "M": 1024**2, "G": 1024**3} + for prefix, mult in multipliers.items(): + if prefix in unit: + return int(speed_val * mult) + return int(speed_val) + except (ValueError, IndexError): + return None + + @register_client("usenet") class SABnzbdClient(DownloadClient): """SABnzbd download client using REST API.""" @@ -179,59 +222,14 @@ class SABnzbdClient(DownloadClient): } state = status_mapping.get(status_text, "downloading") - # Parse ETA (format: "0:01:23" or empty) - eta_str = slot.get("timeleft", "") - eta_seconds = None - if eta_str and eta_str != "0:00:00": - try: - parts = eta_str.split(":") - if len(parts) == 3: - eta_seconds = ( - int(parts[0]) * 3600 - + int(parts[1]) * 60 - + int(parts[2]) - ) - except (ValueError, IndexError): - pass # ETA display is optional - - # Parse speed - prefer kbpersec field (more reliable numeric value) - download_speed = None - kbpersec_str = slot.get("kbpersec", "") - if kbpersec_str: - try: - kbpersec = float(kbpersec_str) - download_speed = int(kbpersec * 1024) # Convert KB/s to bytes/s - except (ValueError, TypeError): - pass # Speed display is optional - - # Fall back to human-readable speed field if kbpersec not available - if download_speed is None: - speed_str = slot.get("speed", "") - if speed_str: - try: - speed_parts = speed_str.split() - if len(speed_parts) >= 2: - speed_val = float(speed_parts[0]) - unit = speed_parts[1].upper() - if "K" in unit: - download_speed = int(speed_val * 1024) - elif "M" in unit: - download_speed = int(speed_val * 1024 * 1024) - elif "G" in unit: - download_speed = int(speed_val * 1024 * 1024 * 1024) - else: - download_speed = int(speed_val) - except (ValueError, IndexError): - pass # Speed display is optional - return DownloadStatus( progress=percentage, state=state, message=status_text.lower().replace("_", " ").title(), complete=False, file_path=None, - download_speed=download_speed, - eta=eta_seconds, + download_speed=_parse_speed(slot), + eta=_parse_eta(slot.get("timeleft", "")), ) # Not in queue, check history diff --git a/cwa_book_downloader/release_sources/prowlarr/handler.py b/cwa_book_downloader/release_sources/prowlarr/handler.py index 3e9a7452..4147a2b6 100644 --- a/cwa_book_downloader/release_sources/prowlarr/handler.py +++ b/cwa_book_downloader/release_sources/prowlarr/handler.py @@ -1,10 +1,4 @@ -""" -Prowlarr download handler. - -Handles downloads from Prowlarr via external download clients. -Supported torrent clients: qBittorrent, Transmission, Deluge. -Supported usenet clients: NZBGet, SABnzbd. -""" +"""Prowlarr download handler - executes downloads via torrent/usenet clients.""" import shutil from pathlib import Path @@ -14,6 +8,7 @@ from typing import Callable, Optional from cwa_book_downloader.core.config import config from cwa_book_downloader.core.logger import setup_logger from cwa_book_downloader.core.models import DownloadTask +from cwa_book_downloader.core.utils import is_audiobook from cwa_book_downloader.release_sources import DownloadHandler, register_handler from cwa_book_downloader.release_sources.prowlarr.cache import get_release, remove_release from cwa_book_downloader.release_sources.prowlarr.clients import ( @@ -34,31 +29,38 @@ class ProwlarrHandler(DownloadHandler): """Handler for Prowlarr downloads via configured torrent or usenet client.""" def _get_category_for_task(self, client, task: DownloadTask) -> Optional[str]: - """Get the appropriate category based on content type. - - Returns the audiobook-specific category if configured and the task is an audiobook, - otherwise returns None to let the client use its default category. - """ - is_audiobook = task.content_type and "audiobook" in task.content_type.lower() - - if not is_audiobook: + """Get audiobook category if configured and applicable, else None for default.""" + if not is_audiobook(task.content_type): return None # Client-specific audiobook category config keys - audiobook_key = { + audiobook_keys = { "qbittorrent": "QBITTORRENT_CATEGORY_AUDIOBOOK", "transmission": "TRANSMISSION_CATEGORY_AUDIOBOOK", "deluge": "DELUGE_CATEGORY_AUDIOBOOK", "nzbget": "NZBGET_CATEGORY_AUDIOBOOK", "sabnzbd": "SABNZBD_CATEGORY_AUDIOBOOK", - }.get(client.name) + } + audiobook_key = audiobook_keys.get(client.name) + return config.get(audiobook_key, "") or None if audiobook_key else None - if audiobook_key: - audiobook_cat = config.get(audiobook_key, "") - if audiobook_cat: - return audiobook_cat + def _build_progress_message(self, status) -> str: + """Build a progress message from download status.""" + msg = f"{status.progress:.0f}%" - return None # Let client use its default + if status.download_speed and status.download_speed > 0: + speed_mb = status.download_speed / 1024 / 1024 + msg += f" ({speed_mb:.1f} MB/s)" + + if status.eta and status.eta > 0: + if status.eta < 60: + msg += f" - {status.eta}s left" + elif status.eta < 3600: + msg += f" - {status.eta // 60}m left" + else: + msg += f" - {status.eta // 3600}h {(status.eta % 3600) // 60}m left" + + return msg def download( self, @@ -67,18 +69,7 @@ class ProwlarrHandler(DownloadHandler): progress_callback: Callable[[float], None], status_callback: Callable[[str, Optional[str]], None], ) -> Optional[str]: - """ - Execute a Prowlarr download. - - Args: - task: Download task with task_id (Prowlarr source_id/GUID) - cancel_flag: Event to check for cancellation - progress_callback: Called with progress percentage (0-100) - status_callback: Called with (status, message) for status updates - - Returns: - Path to downloaded file if successful, None otherwise - """ + """Execute download via configured torrent/usenet client. Returns file path or None.""" try: # Look up the cached release prowlarr_result = get_release(task.task_id) @@ -205,24 +196,8 @@ class ProwlarrHandler(DownloadHandler): client.remove(download_id, delete_files=True) return None - # Build status message - # If client provided a specific message (e.g., "Stalled", "Fetching metadata"), - # use that. Otherwise, build a progress message. - if status.message: - msg = status.message - else: - msg = f"{status.progress:.0f}%" - if status.download_speed and status.download_speed > 0: - speed_mb = status.download_speed / 1024 / 1024 - msg += f" ({speed_mb:.1f} MB/s)" - if status.eta and status.eta > 0: - if status.eta < 60: - msg += f" - {status.eta}s left" - elif status.eta < 3600: - msg += f" - {status.eta // 60}m left" - else: - msg += f" - {status.eta // 3600}h {(status.eta % 3600) // 60}m left" - + # Build status message - use client message if provided, else build progress + msg = status.message or self._build_progress_message(status) status_callback("downloading", msg) # Wait for next poll (interruptible by cancel) @@ -271,14 +246,7 @@ class ProwlarrHandler(DownloadHandler): task: DownloadTask, status_callback: Callable[[str, Optional[str]], None], ) -> Optional[str]: - """Handle completed download - stage or return path for orchestrator. - - For torrents: returns original path directly (no copy). Orchestrator - will hardlink or copy as needed, avoiding unnecessary staging of - large files like audiobooks. - - For usenet: stages to temp directory based on config. - """ + """Handle completed download. Torrents return original path; usenet stages to temp.""" try: # For torrents, skip staging - return original path directly # Orchestrator will hardlink (library mode) or copy (ingest mode) as needed @@ -321,12 +289,7 @@ class ProwlarrHandler(DownloadHandler): return None def cancel(self, task_id: str) -> bool: - """ - Cancel an in-progress download. - - Note: Actual cancellation is handled via the cancel_flag in download(). - This method is for cleanup if the cancel_flag mechanism fails. - """ + """Cancel download and clean up cache. Primary cancellation is via cancel_flag.""" logger.debug(f"Cancel requested for Prowlarr task: {task_id}") # Remove from cache if present remove_release(task_id) diff --git a/cwa_book_downloader/release_sources/prowlarr/source.py b/cwa_book_downloader/release_sources/prowlarr/source.py index d0e33e81..3b3f48b5 100644 --- a/cwa_book_downloader/release_sources/prowlarr/source.py +++ b/cwa_book_downloader/release_sources/prowlarr/source.py @@ -1,9 +1,4 @@ -""" -Prowlarr release source implementation. - -Implements the ReleaseSource interface to search Prowlarr indexers -for book releases (torrents and usenet). -""" +"""Prowlarr release source - searches indexers for book releases (torrents/usenet).""" import re from typing import List, Optional @@ -60,53 +55,27 @@ ALL_BOOK_FORMATS = AUDIOBOOK_FORMATS + EBOOK_FORMATS def _extract_format(title: str) -> Optional[str]: - """ - Extract format from release title with smart parsing. - - Supports both ebook formats (epub, mobi, etc.) and audiobook formats (m4b, mp3, etc.). - - Priority: - 1. File extension at end of title or in quotes (e.g., ".azw3", ".m4b") - 2. Format keyword in brackets/parentheses (e.g., "[EPUB]", "(MP3)") - 3. Format as standalone word (not part of another word) - """ + """Extract ebook/audiobook format from release title (extension, bracketed, or standalone).""" title_lower = title.lower() - # 1. Look for file extensions (most reliable) - pattern: .format at word boundary or end - # This catches ".azw3", ".epub", ".m4b", ".mp3", etc. - for fmt in ALL_BOOK_FORMATS: - # Match .format at end of string or followed by non-alphanumeric - pattern = rf'\.{fmt}(?:["\'\s\]\)]|$)' - if re.search(pattern, title_lower): - return fmt + # Pattern priority: file extension > bracketed > standalone word + # Use %s placeholder since {fmt} conflicts with regex syntax + pattern_templates = [ + r'\.%s(?:["\'\s\]\)]|$)', # .format at end or followed by delimiter + r'[\[\(\{]%s[\]\)\}]', # [EPUB], (PDF), {mobi} + r'\b%s\b', # standalone word + ] - # 2. Look for format in brackets/parentheses (common in release names) - # e.g., "[EPUB]", "(PDF)", "{mobi}", "[M4B]", "(MP3)" - for fmt in ALL_BOOK_FORMATS: - pattern = rf'[\[\(\{{]{fmt}[\]\)\}}]' - if re.search(pattern, title_lower): - return fmt - - # 3. Look for format as standalone word (not part of another word) - # e.g., "epub" but not "republic", "m4b" but not "m4b123" - for fmt in ALL_BOOK_FORMATS: - # Match format as whole word - pattern = rf'\b{fmt}\b' - if re.search(pattern, title_lower): - return fmt + for template in pattern_templates: + for fmt in ALL_BOOK_FORMATS: + if re.search(template % fmt, title_lower): + return fmt return None def _extract_language(title: str) -> Optional[str]: - """ - Extract language from release title. - - Common patterns: - - [German], (French), {Spanish} - - German, French, etc. as standalone words - - Language codes like [DE], [FR], [ES] - """ + """Extract language code from release title (e.g., [German] -> 'de').""" title_lower = title.lower() # Common language names and their codes @@ -139,87 +108,50 @@ EBOOK_CATEGORY_IDS = {7000, 7020} # 7000 = Books, 7020 = Books/Ebook def _detect_content_type_from_categories(categories: list, fallback: str = "book") -> str: - """ - Detect content type from Prowlarr category IDs. - - Prowlarr returns categories as a list of dicts with 'id' and 'name' keys, - or sometimes just category IDs directly. - - Args: - categories: List of category objects from Prowlarr result - fallback: Content type to use if categories don't indicate a specific type - - Returns: - "audiobook" if audiobook categories detected, "book" otherwise - """ + """Detect content type from Prowlarr category IDs. Returns 'audiobook' or 'book'.""" # Normalize fallback - convert "ebook" to "book" for display consistency - if fallback == "ebook": - fallback = "book" + normalized_fallback = "book" if fallback == "ebook" else fallback if not categories: - return fallback + return normalized_fallback # Extract category IDs from the nested structure - cat_ids = set() - for cat in categories: - if isinstance(cat, dict): - cat_id = cat.get("id") - if cat_id is not None: - cat_ids.add(cat_id) - elif isinstance(cat, int): - cat_ids.add(cat) + cat_ids = { + cat.get("id") if isinstance(cat, dict) else cat + for cat in categories + if (isinstance(cat, dict) and cat.get("id") is not None) or isinstance(cat, int) + } - # Check for audiobook categories first (more specific) + # Check for audiobook categories first (more specific), then ebook if cat_ids & AUDIOBOOK_CATEGORY_IDS: return "audiobook" - - # Check for ebook categories if cat_ids & EBOOK_CATEGORY_IDS: return "book" - # Fall back to normalized content_type if no recognized categories - return fallback + return normalized_fallback def _prowlarr_result_to_release(result: dict, search_content_type: str = "ebook") -> Release: - """ - Convert a Prowlarr search result to a Release object. - - Uses structured fields from Prowlarr when available: - - protocol: Direct from Prowlarr - - fileName: For format detection (more reliable than title) - - categories: To detect content type (audiobook vs ebook) - - grabs: Download count - - Args: - result: Raw Prowlarr API result - search_content_type: Content type from search, used as fallback if - categories don't indicate a specific type - """ + """Convert a Prowlarr API result to a Release object.""" title = result.get("title", "Unknown") size_bytes = result.get("size") - download_url = result.get("downloadUrl") or result.get("magnetUrl") - info_url = result.get("infoUrl") or result.get("guid") indexer = result.get("indexer", "Unknown") protocol = get_protocol_display(result) seeders = result.get("seeders") leechers = result.get("leechers") + categories = result.get("categories", []) + is_torrent = protocol == "torrent" + # Format peers display string: "seeders / leechers" - peers_display = f"{seeders} / {leechers}" if (seeders is not None and leechers is not None) else None - grabs = result.get("grabs") + peers_display = ( + f"{seeders} / {leechers}" + if is_torrent and seeders is not None and leechers is not None + else None + ) # For format detection, prefer fileName over title (often cleaner) file_name = result.get("fileName", "") - format_detected = _extract_format(file_name) if file_name else None - if not format_detected: - format_detected = _extract_format(title) - - # Extract language from title (Prowlarr doesn't provide this structured) - language = _extract_language(title) - - # Detect content type from categories (per-result), with search type as fallback - categories = result.get("categories", []) - content_type = _detect_content_type_from_categories(categories, search_content_type) + format_detected = _extract_format(file_name) if file_name else _extract_format(title) # Build the source_id from GUID or generate from indexer + title source_id = result.get("guid") or f"{indexer}:{hash(title)}" @@ -232,34 +164,29 @@ def _prowlarr_result_to_release(result: dict, search_content_type: str = "ebook" source_id=source_id, title=title, format=format_detected, - language=language, + language=_extract_language(title), size=_parse_size(size_bytes), size_bytes=size_bytes, - download_url=download_url, - info_url=info_url, + download_url=result.get("downloadUrl") or result.get("magnetUrl"), + info_url=result.get("infoUrl") or result.get("guid"), protocol=protocol, indexer=indexer, - seeders=seeders if protocol == "torrent" else None, - peers=peers_display if protocol == "torrent" else None, - content_type=content_type, # Detected per-result from categories + seeders=seeders if is_torrent else None, + peers=peers_display, + content_type=_detect_content_type_from_categories(categories, search_content_type), extra={ "publish_date": result.get("publishDate"), "categories": categories, "indexer_id": result.get("indexerId"), "files": result.get("files"), - "grabs": grabs, + "grabs": result.get("grabs"), }, ) @register_source("prowlarr") class ProwlarrSource(ReleaseSource): - """ - Prowlarr release source. - - Searches Prowlarr indexers for book releases (torrents and usenet). - Supports both ebooks (category 7000) and audiobooks (category 3030). - """ + """Prowlarr release source for ebooks and audiobooks.""" name = "prowlarr" display_name = "Prowlarr" @@ -371,21 +298,7 @@ class ProwlarrSource(ReleaseSource): languages: Optional[List[str]] = None, content_type: str = "ebook" ) -> List[Release]: - """ - Search Prowlarr for releases matching the book. - - Makes separate API calls for each selected indexer to ensure - all indexers are properly queried regardless of their capabilities. - - Args: - book: Book metadata to search for - expand_search: If True, skip category filtering (broader search) - languages: Ignored - Prowlarr doesn't support language filtering - content_type: "ebook" or "audiobook" - determines search categories - - Returns: - List of Release objects - """ + """Search Prowlarr indexers for releases matching the book.""" client = self._get_client() if not client: logger.warning("Prowlarr not configured - skipping search") @@ -422,43 +335,33 @@ class ProwlarrSource(ReleaseSource): # Get search categories based on content type # Audiobooks use 3030 (Audio/Audiobook), ebooks use 7000 (Books) - if content_type == "audiobook": - search_categories = [3030] # Audio/Audiobook category - else: - search_categories = [7000] # Books category - - if expand_search: - # Expand search: search all indexers without category filtering - categories = None - self.last_search_type = "expanded" - else: - categories = search_categories - self.last_search_type = "categories" + search_categories = [3030] if content_type == "audiobook" else [7000] + categories = None if expand_search else search_categories + self.last_search_type = "expanded" if expand_search else "categories" logger.debug(f"Searching Prowlarr: query='{query}', indexers={indexer_ids}, categories={categories}") - all_results = [] - try: + def search_indexers(cats: Optional[List[int]]) -> List[dict]: + """Search all indexers with given categories, collecting results.""" + results = [] for indexer_id in indexer_ids: try: - raw_results = client.search(query=query, indexer_ids=[indexer_id], categories=categories) - if raw_results: - all_results.extend(raw_results) + raw = client.search(query=query, indexer_ids=[indexer_id], categories=cats) + if raw: + results.extend(raw) except Exception as e: logger.warning(f"Search failed for indexer {indexer_id}: {e}") + return results - # Auto-expand: if no results with categories and auto-expand enabled, retry without categories + all_results = [] + try: + all_results = search_indexers(categories) + + # Auto-expand: if no results with categories and auto-expand enabled, retry without auto_expand_enabled = config.get("PROWLARR_AUTO_EXPAND", False) - logger.debug(f"Auto-expand check: no_results={not all_results}, has_categories={bool(categories)}, auto_expand_enabled={auto_expand_enabled}") if not all_results and categories and auto_expand_enabled: logger.info("Prowlarr: no results with category filter, auto-expanding search") - for indexer_id in indexer_ids: - try: - raw_results = client.search(query=query, indexer_ids=[indexer_id], categories=None) - if raw_results: - all_results.extend(raw_results) - except Exception as e: - logger.warning(f"Expanded search failed for indexer {indexer_id}: {e}") + all_results = search_indexers(None) self.last_search_type = "expanded" results = [_prowlarr_result_to_release(r, content_type) for r in all_results] diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 3d120d68..b81619d4 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -12,9 +12,9 @@ services: DEBUG: true volumes: - ./.local/config:/config - - ./.local/ingest:/cwa-book-ingest + - ./.local/books:/books - ./.local/log:/var/log/cwa-book-downloader - ./.local/tmp:/tmp/cwa-book-downloader - ./cwa_book_downloader:/app/cwa_book_downloader:ro - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads diff --git a/docker-compose.extbp.dev.yml b/docker-compose.extbp.dev.yml index a2cad3c2..7c28adbc 100644 --- a/docker-compose.extbp.dev.yml +++ b/docker-compose.extbp.dev.yml @@ -15,11 +15,11 @@ services: EXT_BYPASSER_TIMEOUT: 60000 volumes: - ./.local/config:/config - - ./.local/ingest:/cwa-book-ingest + - ./.local/books:/books - ./.local/log:/var/log/cwa-book-downloader - ./.local/tmp:/tmp/cwa-book-downloader - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads flaresolverr: image: ghcr.io/flaresolverr/flaresolverr:latest diff --git a/docker-compose.extbp.yml b/docker-compose.extbp.yml index 7da38d7f..b245e5e1 100644 --- a/docker-compose.extbp.yml +++ b/docker-compose.extbp.yml @@ -11,10 +11,10 @@ services: - 8084:8084 restart: unless-stopped volumes: - - /path/to/ingest:/cwa-book-ingest # Book ingest directory + - /path/to/books:/books # Book destination directory - /path/to/config:/config # App configuration - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads flaresolverr: image: ghcr.io/flaresolverr/flaresolverr:latest diff --git a/docker-compose.test-clients.yml b/docker-compose.test-clients.yml index 33c97cb6..4759df4d 100644 --- a/docker-compose.test-clients.yml +++ b/docker-compose.test-clients.yml @@ -43,8 +43,8 @@ services: # Config and state - ./.local/test-clients/cwabd/config:/config - ./.local/test-clients/cwabd/log:/var/log/cwa-book-downloader - # Ingest directory (where completed books go) - - ./.local/test-clients/ingest:/cwa-book-ingest + # Book destination directory (where completed books go) + - ./.local/test-clients/books:/books # Staging directory - ./.local/test-clients/tmp:/tmp/cwa-book-downloader # CRITICAL: Mount client download directories so cwabd can access completed files diff --git a/docker-compose.tor.dev.yml b/docker-compose.tor.dev.yml index 02b91a64..eb676eb0 100644 --- a/docker-compose.tor.dev.yml +++ b/docker-compose.tor.dev.yml @@ -12,8 +12,8 @@ services: DEBUG: true volumes: - ./.local/config:/config - - ./.local/ingest:/cwa-book-ingest + - ./.local/books:/books - ./.local/log:/var/log/cwa-book-downloader - ./.local/tmp:/tmp/cwa-book-downloader - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads diff --git a/docker-compose.tor.yml b/docker-compose.tor.yml index 1b8871cb..d2396525 100644 --- a/docker-compose.tor.yml +++ b/docker-compose.tor.yml @@ -15,7 +15,7 @@ services: - 8084:8084 restart: unless-stopped volumes: - - /path/to/ingest:/cwa-book-ingest # Book ingest directory + - /path/to/books:/books # Book destination directory - /path/to/config:/config # App configuration - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads diff --git a/docker-compose.yml b/docker-compose.yml index fee005b6..bc8802ec 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,7 +10,7 @@ services: - 8084:8084 restart: unless-stopped volumes: - - /path/to/ingest:/cwa-book-ingest # Book ingest directory + - /path/to/books:/books # Book destination directory - /path/to/config:/config # App configuration - # Download client mount (must match your torrent/usenet client's volume) - # - /path/to/downloads:/downloads + # Download client mount - path must match your torrent/usenet client's volume exactly + # - /path/to/downloads:/path/to/downloads diff --git a/entrypoint.sh b/entrypoint.sh index d9e9929d..e409ac16 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -203,6 +203,11 @@ fi echo "Running command: '$command' as '$USERNAME' (debug=$is_debug)" +# Set umask for file permissions (default: 0022 = files 644, dirs 755) +UMASK_VALUE=${UMASK:-0022} +echo "Setting umask to $UMASK_VALUE" +umask $UMASK_VALUE + # Stop logging exec 1>&3 2>&4 exec 3>&- 4>&- diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index af34b764..81838335 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -608,8 +608,8 @@ function App() { /> {isInitialState && !featureNoticeDismissed && ( -