Fix PR #632: Improve metadata provider global enable/disable functionality

- Fix critical circular import between cwa_functions.py and search_metadata.py
- Add unified JSON parsing utility for metadata_providers_enabled setting
- Enhance error handling for null/empty values and malformed JSON
- Improve provider validation with proper attribute checks
- Add early return when no active providers available
- Standardize boolean logic across all provider enable/disable checks
- Remove code duplication across auto_metadata.py, metadata_helper.py, search_metadata.py

Author: crocodilestick

CONTRIBUTORS

@@ -1,7 +1,7 @@
 CONTRIBUTORS
 
 This file is automatically generated. DO NOT EDIT MANUALLY.
-Generated on: 2025-09-12T21:35:51.343565Z
+Generated on: 2025-09-12T21:51:34.133525Z
 
 Upstream project: https://github.com/janeczku/calibre-web
 Fork project (Calibre-Web Automated, since 2024): https://github.com/crocodilestick/calibre-web-automated
@@ -298,7 +298,7 @@ Copyright (C) 2024-2025 Calibre-Web Automated contributors
 - zhiyue (1 commits)
 # Fork Contributors (crocodilestick/calibre-web-automated)
 
-- crocodilestick (650 commits)
+- crocodilestick (654 commits)
 - jmarmstrong1207 (73 commits)
 - demitrix (30 commits)
 - sirwolfgang (22 commits)

cps/auto_metadata.py

@@ -81,29 +81,36 @@ def fetch_metadata_for_book(book_title: str, book_authors: str = "", user_id: Op
         provider_hierarchy = get_metadata_provider_hierarchy(cwa_settings)
 
         # Get global enabled map for providers
-        enabled_map_raw = cwa_settings.get('metadata_providers_enabled', '{}')
-        try:
-            if isinstance(enabled_map_raw, str):
-                s = enabled_map_raw.strip()
-                if s.startswith("'") and s.endswith("'"):
-                    s = s[1:-1]
-                enabled_map = json.loads(s or '{}')
-            elif isinstance(enabled_map_raw, dict):
-                enabled_map = enabled_map_raw
-            else:
-                enabled_map = {}
-        except Exception:
-            enabled_map = {}
+        from cps.cwa_functions import parse_metadata_providers_enabled, validate_and_cleanup_provider_enabled_map
+        enabled_map_raw = parse_metadata_providers_enabled(
+            cwa_settings.get('metadata_providers_enabled', '{}')
+        )
 
         # Get available metadata providers
-        available_providers = {provider.__id__: provider for provider in cl if provider.active}
+        available_providers = {
+            provider.__id__: provider 
+            for provider in cl 
+            if (provider.active and hasattr(provider, '__id__') and provider.__id__)
+        }
+        
+        # Early return if no providers available
+        if not available_providers:
+            log.warning("No active metadata providers available")
+            return None
+        
+        # Validate and cleanup the enabled map
+        enabled_map = validate_and_cleanup_provider_enabled_map(
+            enabled_map_raw, list(available_providers.keys())
+        )
 
         # Try providers in order of preference
         for provider_id in provider_hierarchy:
-            # Skip if globally disabled
-            if not bool(enabled_map.get(provider_id, True)):
+            # Check if explicitly disabled (default is enabled if not specified)
+            is_enabled = enabled_map.get(provider_id, True)
+            if not is_enabled:
                 log.debug(f"Provider {provider_id} is globally disabled")
                 continue
+                
             if provider_id not in available_providers:
                 log.debug(f"Provider {provider_id} not available or inactive")
                 continue

cps/cwa_functions.py

@@ -60,6 +60,81 @@
 ##                                                                            ##
 ##————————————————————————————————————————————————————————————————————————————##
 
+def parse_metadata_providers_enabled(raw_value):
+    """
+    Parse the metadata_providers_enabled setting from various formats into a dict.
+    
+    Args:
+        raw_value: The raw value from database/settings (str, dict, bytes, or None)
+        
+    Returns:
+        dict: Provider ID to enabled status mapping. Empty dict on error.
+    """
+    import json
+    
+    try:
+        # Handle None/null values
+        if raw_value is None:
+            return {}
+            
+        # Handle bytes (from some database drivers)
+        if isinstance(raw_value, bytes):
+            raw_value = raw_value.decode('utf-8', errors='ignore')
+        
+        # Handle string (most common case)
+        if isinstance(raw_value, str):
+            s = raw_value.strip()
+            # Handle empty strings
+            if not s:
+                return {}
+            # Strip surrounding single quotes if present from schema default
+            if s.startswith("'") and s.endswith("'"):
+                s = s[1:-1]
+            # Handle empty string after quote stripping
+            if not s:
+                return {}
+            data = json.loads(s)
+            return data if isinstance(data, dict) else {}
+        
+        # Handle dict (already parsed)
+        elif isinstance(raw_value, dict):
+            return raw_value
+        
+        # Unknown type, return empty dict
+        else:
+            return {}
+            
+    except (json.JSONDecodeError, ValueError, TypeError, AttributeError):
+        return {}
+
+def validate_and_cleanup_provider_enabled_map(enabled_map, available_provider_ids):
+    """
+    Validate and cleanup the provider enabled map.
+    
+    Args:
+        enabled_map (dict): Current provider enabled map
+        available_provider_ids (list): List of valid provider IDs
+        
+    Returns:
+        dict: Cleaned up enabled map with only valid providers
+    """
+    if not isinstance(enabled_map, dict):
+        return {}
+    
+    if not isinstance(available_provider_ids, (list, tuple, set)):
+        return {}
+    
+    # Keep only valid provider IDs and boolean values
+    cleaned_map = {}
+    for provider_id, enabled in enabled_map.items():
+        if (isinstance(provider_id, str) and 
+            provider_id.strip() and  # Non-empty string
+            provider_id in available_provider_ids):
+            # Convert to boolean, handling various truthy/falsy values
+            cleaned_map[provider_id] = bool(enabled)
+    
+    return cleaned_map
+
 @switch_theme.route("/cwa-switch-theme", methods=["GET", "POST"])
 @login_required_if_no_ano
 def cwa_switch_theme():
@@ -313,8 +388,13 @@ def set_cwa_settings():
                                 result[setting] = cwa_db.cwa_settings.get(setting, '["ibdb","google","dnb"]')
                         elif setting == 'metadata_providers_enabled':
                             # Validate dict mapping provider_id -> bool
-                            if isinstance(json_value, dict) and all(isinstance(k, str) and isinstance(v, bool) for k, v in json_value.items()):
-                                result[setting] = json.dumps(json_value)
+                            if isinstance(json_value, dict):
+                                # Just validate the basic structure - provider validation happens at runtime
+                                cleaned_map = {}
+                                for k, v in json_value.items():
+                                    if isinstance(k, str) and isinstance(v, bool):
+                                        cleaned_map[k] = v
+                                result[setting] = json.dumps(cleaned_map)
                             else:
                                 result[setting] = cwa_db.cwa_settings.get(setting, '{}')
                         else:

cps/metadata_helper.py

@@ -59,25 +59,17 @@ def fetch_and_apply_metadata(book_id: int, user_enabled: bool = False) -> bool:
             provider_hierarchy = ["google", "douban", "dnb", "ibdb", "comicvine"]
 
         # Global provider enablement map
-        enabled_map_raw = cwa_settings.get('metadata_providers_enabled', '{}')
-        try:
-            if isinstance(enabled_map_raw, str):
-                s = enabled_map_raw.strip()
-                if s.startswith("'") and s.endswith("'"):
-                    s = s[1:-1]
-                enabled_map = json.loads(s or '{}')
-            elif isinstance(enabled_map_raw, dict):
-                enabled_map = enabled_map_raw
-            else:
-                enabled_map = {}
-        except Exception:
-            enabled_map = {}
+        from cps.cwa_functions import parse_metadata_providers_enabled
+        enabled_map = parse_metadata_providers_enabled(
+            cwa_settings.get('metadata_providers_enabled', '{}')
+        )
 
         # Try each provider in order
         metadata_found = False
         for provider_id in provider_hierarchy:
-            # Skip if globally disabled
-            if not bool(enabled_map.get(provider_id, True)):
+            # Check if explicitly disabled (default is enabled if not specified)
+            is_enabled = enabled_map.get(provider_id, True)
+            if not is_enabled:
                 log.debug(f"Provider {provider_id} is globally disabled")
                 continue
             try:

cps/search_metadata.py

@@ -78,25 +78,20 @@ def _get_global_provider_enabled_map() -> dict:
         from cwa_db import CWA_DB  # type: ignore
         cwa_db = CWA_DB()
         settings = cwa_db.get_cwa_settings()
-        raw = settings.get('metadata_providers_enabled', '{}')
-        if isinstance(raw, bytes):
-            raw = raw.decode('utf-8', errors='ignore')
-        if isinstance(raw, str):
-            s = raw.strip()
-            # Strip surrounding single quotes if present from default schema value
-            if s.startswith("'") and s.endswith("'"):
-                s = s[1:-1]
-            try:
-                data = json.loads(s or '{}')
-            except Exception:
-                return {}
-            return data if isinstance(data, dict) else {}
-        elif isinstance(raw, dict):
-            return raw
-    except Exception:
-        # On any failure, treat as all enabled
+        
+        if not settings:
+            log.warning("Could not get CWA settings for provider enabled map")
+            return {}
+        
+        from cps.cwa_functions import parse_metadata_providers_enabled
+        return parse_metadata_providers_enabled(
+            settings.get('metadata_providers_enabled', '{}')
+        )
+    except Exception as e:
+        # On any failure, treat as all enabled (empty dict = all default to enabled)
+        log.warning(f"Error loading provider enabled map: {e}")
         return {}
-    return {}
+    # Remove redundant return
 
 @meta.route("/metadata/provider")
 @user_login_required