diff --git a/scripts/dev_server.py b/scripts/dev_server.py index 7a7dd945..4372f14b 100644 --- a/scripts/dev_server.py +++ b/scripts/dev_server.py @@ -142,17 +142,18 @@ def find_plugin_dir(plugin_id: str) -> Optional[Path]: def load_config_defaults(plugin_dir: 'str | Path') -> Dict[str, Any]: - """Extract default values from config_schema.json.""" + """Extract default values from config_schema.json. + + The same extraction a device and the plugin harness use + (src/plugin_system/testing/loading.py), nested defaults included. + """ + from src.plugin_system.testing.loading import ( + load_config_defaults as _load_config_defaults, + ) schema_path = resolve_under(plugin_dir, 'config_schema.json') if schema_path is None or not schema_path.exists(): return {} - with open(schema_path, 'r') as f: - schema = json.load(f) - defaults: Dict[str, Any] = {} - for key, prop in schema.get('properties', {}).items(): - if 'default' in prop: - defaults[key] = prop['default'] - return defaults + return _load_config_defaults(schema_path.parent) # -------------------------------------------------------------------------- @@ -303,10 +304,13 @@ def _parse_render_request(data): with open(manifest_path, 'r') as f: manifest = json.load(f) - # Build config: schema defaults + user overrides - config = {'enabled': True} - config.update(load_config_defaults(trusted_dir)) - config.update(data.get('config', {})) + # Build config the way a device would: schema defaults under a forced + # enabled, with the user's overrides deep-merged on top + from src.plugin_system.testing.loading import build_config + overrides = data.get('config') or {} + if not isinstance(overrides, dict): + raise ValueError('config must be a JSON object') + config = build_config(trusted_dir, overrides) return trusted_dir, manifest, config, data.get('mock_data', {}), data.get('skip_update', False) diff --git a/src/display_controller.py b/src/display_controller.py index 0434528e..7f5d7a66 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -2976,6 +2976,14 @@ class DisplayController: _pid: str = plugin_id, _plugin: Any = plugin_instance) -> None: """Callback for plugin config changes.""" try: + # ConfigService hands over the raw config.json section. + # Prepare it as loading did (legacy booleans read as + # objects, schema defaults filled in), so the plugin gets + # the same shape it was constructed with. + prepare = getattr(self.plugin_manager, 'prepare_plugin_config', None) + prepared = prepare(_pid, new_config) if callable(prepare) else None + if isinstance(prepared, dict): + new_config = prepared _plugin.on_config_change(new_config) logger.debug("Plugin %s notified of config change", _pid) except Exception as e: diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index c587acc8..43879a82 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -22,7 +22,9 @@ from src.logging_config import get_logger from src.plugin_system.plugin_loader import PluginLoader from src.plugin_system.plugin_executor import PluginExecutor from src.plugin_system.plugin_state import PluginStateManager, PluginState -from src.plugin_system.schema_manager import SchemaManager, normalize_legacy_booleans +from src.plugin_system.schema_manager import ( + CORE_VEGAS_TUNING_KEYS, SchemaManager, normalize_legacy_booleans, +) from src.common.permission_utils import ( ensure_directory_permissions, get_plugin_dir_mode @@ -369,32 +371,11 @@ class PluginManager: f"The schema may be invalid. Please verify the schema file at: {schema_path}" ) - # A plugin that turned an on/off boolean into an {enabled, ...} - # object still finds the boolean in config.json until the user saves - # its settings form, which carries it over (plugin_config.html). - # Read it the same way here, before the defaults fill in the rest - # of the object and before schema validation, so the plugin doesn't - # start with a schema warning and a degraded flag. In memory only: - # config.json is written by saves, never by loading a plugin. - if schema: - upgraded: List[str] = [] - config = normalize_legacy_booleans(config, schema, upgraded) - if upgraded: - self.logger.info( - "Plugin %s: reading legacy boolean setting %s as " - "{\"enabled\": ...}; saving the plugin's settings " - "stores the new shape", - plugin_id, ", ".join(upgraded), - ) - - # Merge config with schema defaults to ensure all defaults are applied - try: - defaults = self.schema_manager.generate_default_config(plugin_id, use_cache=True) - config = self.schema_manager.merge_with_defaults(config, defaults) - self.logger.debug(f"Merged config with schema defaults for {plugin_id}") - except Exception as e: - self.logger.warning(f"Could not apply schema defaults for {plugin_id}: {e}") - # Continue with original config if defaults can't be applied + # Legacy booleans read as objects, then schema defaults: the same + # preparation saves, GET /plugins/config and hot reload apply + # (prepare_plugin_config). In memory only: config.json is written + # by saves, never by loading a plugin. + config = self.prepare_plugin_config(plugin_id, config, schema=schema) # Use PluginLoader to load plugin plugin_instance, module = self.plugin_loader.load_plugin( @@ -489,11 +470,55 @@ class PluginManager: #: #: Read by: ``vegas_mode/plugin_adapter.py`` (``vegas_width_pct``, #: ``vegas_overflow``) and ``base_plugin.py`` (``vegas_max_width_screens``). - CORE_OWNED_CONFIG_KEYS = frozenset({ - 'vegas_width_pct', - 'vegas_overflow', - 'vegas_max_width_screens', - }) + #: + #: The list itself lives with the other core-owned per-plugin properties in + #: ``schema_manager.CORE_PLUGIN_PROPERTIES``, which the web save path also + #: uses to keep these keys. + CORE_OWNED_CONFIG_KEYS = CORE_VEGAS_TUNING_KEYS + + def prepare_plugin_config(self, plugin_id: str, config: Any, + schema: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """The config a plugin runs with, built from its raw config.json section. + + A plugin that turned an on/off boolean into an ``{enabled, ...}`` + object still finds the boolean in config.json until its settings are + next saved; it is read as the object, and schema defaults fill in the + rest (``SchemaManager.prepare_plugin_config``). Used when loading a + plugin and on hot reload (DisplayController), so ``on_config_change`` + receives the same shape the plugin was constructed with. + + Never raises: on failure the legacy-boolean pass alone is applied, or + failing that the section is returned as it was. + """ + if schema is None: + try: + schema = self.schema_manager.load_schema(plugin_id) + except Exception as e: + self.logger.debug("Could not load schema for %s: %s", plugin_id, e) + schema = None + upgraded: List[str] = [] + try: + prepared = self.schema_manager.prepare_plugin_config( + plugin_id, config, schema=schema, changed_paths=upgraded) + self.logger.debug("Merged config with schema defaults for %s", plugin_id) + except Exception as e: + self.logger.warning("Could not apply schema defaults for %s: %s", plugin_id, e) + # Continue without defaults if they can't be applied + upgraded = [] + prepared = config if isinstance(config, dict) else {} + if schema: + try: + prepared = normalize_legacy_booleans(prepared, schema, upgraded) + except Exception: + pass + if upgraded: + self.logger.info( + "Plugin %s: reading legacy boolean setting %s as " + "{\"enabled\": ...}; saving the plugin's settings " + "stores the new shape", + plugin_id, ", ".join(upgraded), + ) + return prepared def _strip_core_owned_keys(self, config: Dict[str, Any]) -> Dict[str, Any]: """A shallow copy of ``config`` without the core's own tuning keys. diff --git a/src/plugin_system/schema_manager.py b/src/plugin_system/schema_manager.py index 2be222ef..d4eb70b8 100644 --- a/src/plugin_system/schema_manager.py +++ b/src/plugin_system/schema_manager.py @@ -95,6 +95,222 @@ def normalize_legacy_booleans(config: Any, schema: Any, return result +#: Per-plugin settings the **core** owns: it reads them out of each plugin's +#: config section, so they are allowed in every plugin's config whether or not +#: the plugin's schema declares them. The one list for validation, for the web +#: save filter and for the load-time checks -- a private copy is how JSON saves +#: came to drop ``skin`` and the ``vegas_*`` keys while the validator accepted +#: them. +#: +#: Values are the schema used when the plugin does not declare the property. +CORE_PLUGIN_PROPERTIES: Dict[str, Dict[str, Any]] = { + # Defaults match BasePlugin behavior: enabled=True, display_duration=15, + # live_priority=False. + "enabled": { + "type": "boolean", + "default": True, + "description": "Enable or disable this plugin" + }, + "display_duration": { + "type": "number", + "default": 15, + "minimum": 1, + "maximum": 300, + "description": "How long to display this plugin in seconds" + }, + "live_priority": { + "type": "boolean", + "default": False, + "description": "Enable live priority takeover when plugin has live content" + }, + # Skin selection (docs/SKIN_SYSTEM.md). Deliberately NOT an enum here: + # validation must keep passing when a configured skin gets uninstalled + # (rendering falls back to built-in). The install-dependent enum is + # injected only at serve time (inject_skin_selector) for the web UI + # dropdown. + "skin": { + "type": ["string", "object", "null"], + "description": "Visual skin id, or a per-mode mapping like {\"live\": \"my-skin\"}" + }, + "skin_options": { + "type": "object", + "description": "Options passed through to the selected skin" + }, + # Vegas tuning read by vegas_mode/plugin_adapter.py and base_plugin.py. + # Left untyped: the adapter validates them itself and ignores a bad + # value with a log line, so a stored one must never block a save. + "vegas_width_pct": { + "description": "Vegas mode: width of this plugin's card, as a percentage of the panel" + }, + "vegas_overflow": { + "description": "Vegas mode: 'rotate' or 'truncate' when this plugin's content overflows" + }, + "vegas_max_width_screens": { + "description": "Vegas mode: widest this plugin's card may be, in screens" + }, +} + +#: The keys of CORE_PLUGIN_PROPERTIES that are Vegas tuning rather than plugin +#: state. PluginManager strips these before its soft validation (see +#: PluginManager.CORE_OWNED_CONFIG_KEYS). +CORE_VEGAS_TUNING_KEYS = frozenset({ + 'vegas_width_pct', 'vegas_overflow', 'vegas_max_width_screens', +}) + + +def with_core_plugin_properties(schema: Dict[str, Any]) -> Dict[str, Any]: + """A deep copy of a plugin schema with CORE_PLUGIN_PROPERTIES allowed. + + Properties the plugin declares itself are left as declared. Core + properties are removed from ``required``: they are system-managed. + """ + enhanced = copy.deepcopy(schema) if isinstance(schema, dict) else {} + properties = enhanced.setdefault("properties", {}) + for name, definition in CORE_PLUGIN_PROPERTIES.items(): + if name not in properties: + properties[name] = copy.deepcopy(definition) + if "required" in enhanced: + enhanced["required"] = [field for field in enhanced["required"] + if field not in CORE_PLUGIN_PROPERTIES] + return enhanced + + +def extract_schema_defaults(schema: Dict[str, Any]) -> Dict[str, Any]: + """Default values of a JSON Schema's properties, recursively. + + A property's own ``default`` wins; otherwise a nested object contributes + its children's defaults, and an array contributes ``[]`` (or a one-item + list of its ``items`` default). This is what a device runs with, so the + dev tools use it too (src/plugin_system/testing/loading.py). + """ + defaults: Dict[str, Any] = {} + properties = schema.get('properties', {}) if isinstance(schema, dict) else {} + if not isinstance(properties, dict): + return defaults + + for key, prop_schema in properties.items(): + if not isinstance(prop_schema, dict): + continue + # If property has a default, use it + if 'default' in prop_schema: + defaults[key] = prop_schema['default'] + continue + + # Handle nested objects + if prop_schema.get('type') == 'object' and 'properties' in prop_schema: + nested_defaults = extract_schema_defaults(prop_schema) + if nested_defaults: + defaults[key] = nested_defaults + + # Handle arrays with object items + elif prop_schema.get('type') == 'array' and 'items' in prop_schema: + items_schema = prop_schema['items'] + if items_schema.get('type') == 'object' and 'properties' in items_schema: + # For arrays of objects, use empty array as default + # Individual objects will use their defaults when created + defaults[key] = [] + elif 'default' in items_schema: + # Array with default item value + defaults[key] = [items_schema['default']] + else: + # Empty array as default + defaults[key] = [] + + # For other types without defaults, don't add to defaults dict + # This allows plugins to handle missing values as needed + + return defaults + + +def plugin_config_defaults(schema: Optional[Dict[str, Any]]) -> Dict[str, Any]: + """Every default a plugin's config gets: the schema's plus the core ones. + + A plugin with no schema gets the minimal ``enabled: False, + display_duration: 15``. Device location is not applied here; that needs a + config manager (SchemaManager.generate_default_config). + """ + if not schema: + return { + 'enabled': False, + 'display_duration': 15 + } + + defaults = extract_schema_defaults(schema) + + # Ensure core properties have defaults (they may not be in the schema) + # These match BasePlugin behavior + for name in ('enabled', 'display_duration', 'live_priority'): + if name not in defaults: + defaults[name] = CORE_PLUGIN_PROPERTIES[name]['default'] + return defaults + + +def merge_config_defaults(config: Dict[str, Any], defaults: Dict[str, Any]) -> Dict[str, Any]: + """Merge configuration with defaults, preserving user values. + + Also replaces None values with defaults so a config never starts with + None where a default exists. Neither argument is mutated. + """ + merged = copy.deepcopy(defaults) + + def deep_merge(target: Dict[str, Any], source: Dict[str, Any], default_dict: Dict[str, Any]) -> None: + """Recursively merge source into target, replacing None with defaults.""" + for key, value in source.items(): + default_value = default_dict.get(key) + + if key in target and isinstance(target[key], dict) and isinstance(value, dict): + # Both are dicts, recursively merge + if isinstance(default_value, dict): + deep_merge(target[key], value, default_value) + else: + deep_merge(target[key], value, {}) + elif value is None and default_value is not None: + # Value is None and we have a default, use the default + target[key] = copy.deepcopy(default_value) if isinstance(default_value, (dict, list)) else default_value + else: + # Normal merge: user value takes precedence (copy if dict/list) + if isinstance(value, (dict, list)): + target[key] = copy.deepcopy(value) + else: + target[key] = value + + deep_merge(merged, config, defaults) + + # Final pass: replace any remaining None values at any level with defaults + def replace_none_with_defaults(target: Dict[str, Any], default_dict: Dict[str, Any]) -> None: + """Recursively replace None values with defaults.""" + for key in list(target.keys()): + value = target[key] + default_value = default_dict.get(key) + + if value is None and default_value is not None: + # Replace None with default + target[key] = copy.deepcopy(default_value) if isinstance(default_value, (dict, list)) else default_value + elif isinstance(value, dict) and isinstance(default_value, dict): + # Recursively process nested dicts + replace_none_with_defaults(value, default_value) + + replace_none_with_defaults(merged, defaults) + return merged + + +def prepare_plugin_config(config: Any, schema: Optional[Dict[str, Any]], + defaults: Dict[str, Any], + changed_paths: Optional[List[str]] = None) -> Dict[str, Any]: + """The config a plugin runs with, from its stored (or submitted) section. + + Legacy booleans are read as ``{"enabled": ...}`` objects + (normalize_legacy_booleans), then schema defaults fill in whatever is + missing. Loading a plugin, both config saves, GET /plugins/config, hot + reload and the dev tools all go through this, so a plugin sees the same + shape however its config reached it. + """ + config = config if isinstance(config, dict) else {} + if schema: + config = normalize_legacy_booleans(config, schema, changed_paths) + return merge_config_defaults(config, defaults) + + class SchemaManager: """ Manages plugin configuration schemas with caching and validation. @@ -264,57 +480,12 @@ class SchemaManager: def extract_defaults_from_schema(self, schema: Dict[str, Any], prefix: str = '') -> Dict[str, Any]: """ Recursively extract default values from a JSON Schema. - - Handles nested objects, arrays, and all schema types. - - Args: - schema: JSON Schema dictionary - prefix: Optional prefix for logging/debugging - - Returns: - Dictionary of default values + + See :func:`extract_schema_defaults`; ``prefix`` is accepted for + compatibility and unused. """ - defaults = {} - - # Handle schema with properties - properties = schema.get('properties', {}) - if not properties: - return defaults - - for key, prop_schema in properties.items(): - field_path = f"{prefix}.{key}" if prefix else key - - # If property has a default, use it - if 'default' in prop_schema: - defaults[key] = prop_schema['default'] - self.logger.debug(f"Found default for {field_path}: {prop_schema['default']}") - continue - - # Handle nested objects - if prop_schema.get('type') == 'object' and 'properties' in prop_schema: - nested_defaults = self.extract_defaults_from_schema(prop_schema, field_path) - if nested_defaults: - defaults[key] = nested_defaults - - # Handle arrays with object items - elif prop_schema.get('type') == 'array' and 'items' in prop_schema: - items_schema = prop_schema['items'] - if items_schema.get('type') == 'object' and 'properties' in items_schema: - # For arrays of objects, use empty array as default - # Individual objects will use their defaults when created - defaults[key] = [] - elif 'default' in items_schema: - # Array with default item value - defaults[key] = [items_schema['default']] - else: - # Empty array as default - defaults[key] = [] - - # For other types without defaults, don't add to defaults dict - # This allows plugins to handle missing values as needed - - return defaults - + return extract_schema_defaults(schema) + def get_device_location(self) -> Optional[Dict[str, Any]]: """ Return the device-wide ``location`` block from config.json, or None. @@ -393,31 +564,39 @@ class SchemaManager: schema = self.load_schema(plugin_id, use_cache=use_cache) if not schema: # Return minimal defaults if no schema - return { - 'enabled': False, - 'display_duration': 15 - } - - # Extract defaults from schema - defaults = self.extract_defaults_from_schema(schema) - - # Ensure core properties have defaults (they may not be in the schema) - # These match BasePlugin behavior - if 'enabled' not in defaults: - defaults['enabled'] = schema.get('properties', {}).get('enabled', {}).get('default', True) - - if 'display_duration' not in defaults: - defaults['display_duration'] = schema.get('properties', {}).get('display_duration', {}).get('default', 15) - - if 'live_priority' not in defaults: - defaults['live_priority'] = schema.get('properties', {}).get('live_priority', {}).get('default', False) - + return plugin_config_defaults(None) + + # Schema defaults plus the core properties' (they may not be in the + # schema) + defaults = plugin_config_defaults(schema) + # Cache the defaults *before* the device location is layered on, so a # later change to the device location is picked up by the next call. self._defaults_cache[plugin_id] = defaults.copy() return self.apply_device_location(defaults) + def prepare_plugin_config(self, plugin_id: str, config: Any, + schema: Optional[Dict[str, Any]] = None, + changed_paths: Optional[List[str]] = None) -> Dict[str, Any]: + """ + The config a plugin runs with: see :func:`prepare_plugin_config`. + + Args: + plugin_id: Plugin identifier + config: The plugin's stored or submitted config section + schema: The plugin's schema, when the caller already has it + changed_paths: Receives the dotted path of each legacy boolean + read as an object + + Returns: + A new dict; ``config`` is not mutated + """ + if schema is None: + schema = self.load_schema(plugin_id, use_cache=True) + defaults = self.generate_default_config(plugin_id, use_cache=True) + return prepare_plugin_config(config, schema, defaults, changed_paths) + def validate_config_against_schema(self, config: Dict[str, Any], schema: Dict[str, Any], plugin_id: Optional[str] = None) -> Tuple[bool, List[str]]: """ @@ -438,80 +617,18 @@ class SchemaManager: errors = [] try: - # Core plugin properties that should always be allowed - # These are handled by the base plugin system and should not cause validation failures - # Defaults match BasePlugin behavior: enabled=True, display_duration=15, live_priority=False - core_properties = { - "enabled": { - "type": "boolean", - "default": True, - "description": "Enable or disable this plugin" - }, - "display_duration": { - "type": "number", - "default": 15, - "minimum": 1, - "maximum": 300, - "description": "How long to display this plugin in seconds" - }, - "live_priority": { - "type": "boolean", - "default": False, - "description": "Enable live priority takeover when plugin has live content" - }, - # Skin selection (docs/SKIN_SYSTEM.md). Deliberately NOT an - # enum here: validation must keep passing when a configured - # skin gets uninstalled (rendering falls back to built-in). - # The install-dependent enum is injected only at serve time - # (inject_skin_selector) for the web UI dropdown. - "skin": { - "type": ["string", "object", "null"], - "description": "Visual skin id, or a per-mode mapping like {\"live\": \"my-skin\"}" - }, - "skin_options": { - "type": "object", - "description": "Options passed through to the selected skin" - } - } - - # Create a deep copy of the schema to modify (to avoid mutating the original) - enhanced_schema = copy.deepcopy(schema) - if "properties" not in enhanced_schema: - enhanced_schema["properties"] = {} - - # Inject core properties if they're not already defined in the schema - # This ensures core properties are always allowed even if not in the plugin's schema - properties_added = [] - for prop_name, prop_def in core_properties.items(): - if prop_name not in enhanced_schema["properties"]: - enhanced_schema["properties"][prop_name] = copy.deepcopy(prop_def) - properties_added.append(prop_name) - - # Log if we added any core properties (for debugging) - if properties_added and plugin_id: + # Core plugin properties (CORE_PLUGIN_PROPERTIES) are handled by + # the base plugin system and should not cause validation failures: + # they are allowed even when the plugin's schema doesn't declare + # them, and never required. + enhanced_schema = with_core_plugin_properties(schema) + if plugin_id: + declared = schema.get("properties", {}) if isinstance(schema, dict) else {} self.logger.debug( - f"Injected core properties into schema for {plugin_id}: {properties_added}" + "Injected core properties into schema for %s: %s", plugin_id, + [name for name in CORE_PLUGIN_PROPERTIES if name not in declared] ) - - # Remove core properties from required array (they're system-managed) - # Core properties should be allowed but not required for validation - if "required" in enhanced_schema: - core_prop_names = list(core_properties.keys()) - removed_from_required = [ - field for field in enhanced_schema["required"] - if field in core_prop_names - ] - enhanced_schema["required"] = [ - field for field in enhanced_schema["required"] - if field not in core_prop_names - ] - - # Log if we removed any core properties from required (for debugging) - if removed_from_required and plugin_id: - self.logger.debug( - f"Removed core properties from required array for {plugin_id}: {removed_from_required}" - ) - + # Create validator with enhanced schema validator = Draft7Validator(enhanced_schema) @@ -628,55 +745,15 @@ class SchemaManager: """ Merge configuration with defaults, preserving user values. Also replaces None values with defaults to ensure config never has None from the start. - + Args: config: User configuration defaults: Default values from schema - + Returns: Merged configuration with defaults applied where missing or None """ - merged = copy.deepcopy(defaults) - - def deep_merge(target: Dict[str, Any], source: Dict[str, Any], default_dict: Dict[str, Any]) -> None: - """Recursively merge source into target, replacing None with defaults.""" - for key, value in source.items(): - default_value = default_dict.get(key) - - if key in target and isinstance(target[key], dict) and isinstance(value, dict): - # Both are dicts, recursively merge - if isinstance(default_value, dict): - deep_merge(target[key], value, default_value) - else: - deep_merge(target[key], value, {}) - elif value is None and default_value is not None: - # Value is None and we have a default, use the default - target[key] = copy.deepcopy(default_value) if isinstance(default_value, (dict, list)) else default_value - else: - # Normal merge: user value takes precedence (copy if dict/list) - if isinstance(value, (dict, list)): - target[key] = copy.deepcopy(value) - else: - target[key] = value - - deep_merge(merged, config, defaults) - - # Final pass: replace any remaining None values at any level with defaults - def replace_none_with_defaults(target: Dict[str, Any], default_dict: Dict[str, Any]) -> None: - """Recursively replace None values with defaults.""" - for key in list(target.keys()): - value = target[key] - default_value = default_dict.get(key) - - if value is None and default_value is not None: - # Replace None with default - target[key] = copy.deepcopy(default_value) if isinstance(default_value, (dict, list)) else default_value - elif isinstance(value, dict) and isinstance(default_value, dict): - # Recursively process nested dicts - replace_none_with_defaults(value, default_value) - - replace_none_with_defaults(merged, defaults) - return merged + return merge_config_defaults(config, defaults) def detect_config_key_collisions( self, diff --git a/src/plugin_system/testing/harness.py b/src/plugin_system/testing/harness.py index 267774c2..90351542 100644 --- a/src/plugin_system/testing/harness.py +++ b/src/plugin_system/testing/harness.py @@ -27,7 +27,7 @@ from PIL import Image, ImageChops from src.logging_config import get_logger from .bounds_display_manager import BoundsCheckingDisplayManager -from .loading import load_config_defaults, load_manifest, merge_config +from .loading import build_config, load_manifest from .sizes import DEFAULT_TEST_SIZES, safe_mode_filename, size_label logger = get_logger("[Plugin Harness]") @@ -305,9 +305,7 @@ def render_plugin_matrix( manifest = load_manifest(plugin_dir) # Start from config_schema.json defaults so the plugin behaves like a real # install; explicit caller config still wins over a schema default. - config = merge_config( - merge_config({"enabled": True}, load_config_defaults(plugin_dir)), - config or {}) + config = build_config(plugin_dir, config) sizes = sizes or DEFAULT_TEST_SIZES results: List[RenderResult] = [] diff --git a/src/plugin_system/testing/loading.py b/src/plugin_system/testing/loading.py index a7992231..2a0dd8e0 100644 --- a/src/plugin_system/testing/loading.py +++ b/src/plugin_system/testing/loading.py @@ -42,29 +42,6 @@ def load_manifest(plugin_dir: Union[str, Path]) -> Dict[str, Any]: return json.load(f) -def _defaults_from_properties(properties: Dict[str, Any]) -> Dict[str, Any]: - """Defaults for one `properties` block, recursing into nested objects. - - An object property carries its defaults on its children, not on itself, so - reading only the top level dropped everything nested. That is most of the - fleet: config organised by league, or under customization/display_options, - lost 2,386 defaults across 37 of 44 plugins -- soccer-scoreboard alone lost - 539 of 565 -- and the harness rendered them with a config no install would - ever have. - """ - defaults: Dict[str, Any] = {} - for key, prop in (properties or {}).items(): - if not isinstance(prop, dict): - continue - if prop.get('type') == 'object' and isinstance(prop.get('properties'), dict): - nested = _defaults_from_properties(prop['properties']) - if nested: - defaults[key] = nested - elif 'default' in prop: - defaults[key] = prop['default'] - return defaults - - def merge_config(base: Dict[str, Any], override: Dict[str, Any]) -> Dict[str, Any]: """Deep-merge override onto base, without dropping sibling defaults. @@ -81,14 +58,45 @@ def merge_config(base: Dict[str, Any], override: Dict[str, Any]) -> Dict[str, An return merged -def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]: - """Extract default values from a plugin's config_schema.json (empty if none).""" +def load_schema(plugin_dir: Union[str, Path]) -> Optional[Dict[str, Any]]: + """A plugin's config_schema.json, or None when it has none.""" schema_path = Path(plugin_dir) / 'config_schema.json' if not schema_path.exists(): - return {} + return None with open(schema_path, 'r', encoding='utf-8') as f: - schema = json.load(f) - return _defaults_from_properties(schema.get('properties', {})) + return json.load(f) + + +def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]: + """Default values from a plugin's config_schema.json (empty if none). + + The device's own extraction (schema_manager.extract_schema_defaults), so a + harness run starts from the config an install would have: nested objects + contribute their children's defaults (organised-by-league configs lost + thousands of them when only the top level was read), and arrays without a + default start as []. + """ + from src.plugin_system.schema_manager import extract_schema_defaults + schema = load_schema(plugin_dir) + return extract_schema_defaults(schema) if schema else {} + + +def build_config(plugin_dir: Union[str, Path], + overrides: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """The config a device would give this plugin, with ``overrides`` applied. + + Starts from a forced ``enabled: True``, deep-merges ``overrides`` onto it + (merge_config), then prepares the result exactly as the device does when it + loads a plugin: legacy booleans read as objects, and schema plus core + defaults filled in (schema_manager.prepare_plugin_config). Used by the + harness, check_plugin, render_plugin and the dev preview server. + """ + from src.plugin_system.schema_manager import ( + plugin_config_defaults, prepare_plugin_config, + ) + schema = load_schema(plugin_dir) + requested = merge_config({"enabled": True}, overrides or {}) + return prepare_plugin_config(requested, schema, plugin_config_defaults(schema)) def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]: @@ -143,16 +151,14 @@ def build_full_config( Merge order: config_schema.json defaults, then a forced ``enabled: True``, then harness.json's config overlay, then the caller's explicit config -- - most specific wins. `enabled` is re-asserted *after* the schema defaults - so a plugin that reasonably ships `enabled: false` (e.g. a seasonal or - opt-in plugin) can't silently make every harness run test "disabled, do - nothing" by accident -- callers that genuinely want to test the disabled - path can still do so via `cli_config={"enabled": False}`. + most specific wins, and each layer deep-merges (a nested override such as + ``{"nhl": {"enabled": true}}`` keeps the other nhl defaults). `enabled` is + asserted over the schema defaults so a plugin that reasonably ships + `enabled: false` (e.g. a seasonal or opt-in plugin) can't silently make + every harness run test "disabled, do nothing" by accident -- callers that + genuinely want to test the disabled path can still do so via + `cli_config={"enabled": False}`. See build_config. """ spec = spec or {} - config: Dict[str, Any] = {} - config.update(load_config_defaults(plugin_dir)) - config["enabled"] = True - config.update(spec.get("config", {})) - config.update(cli_config or {}) - return config + overrides = merge_config(spec.get("config", {}) or {}, cli_config or {}) + return build_config(plugin_dir, overrides) diff --git a/test/test_plugin_config_preparation.py b/test/test_plugin_config_preparation.py new file mode 100644 index 00000000..e1ffb6bb --- /dev/null +++ b/test/test_plugin_config_preparation.py @@ -0,0 +1,205 @@ +"""One preparation for a plugin's config, wherever the config comes from. + +A plugin's stored section becomes the config it runs with through +schema_manager.prepare_plugin_config: legacy booleans (#588) read as +``{"enabled": ...}`` objects, then schema and core defaults filled in. Loading +did that; hot reload handed plugins the raw section instead (a legacy +``dynamic_duration: true`` came back as a boolean, turning dynamic duration +off), and the dev tools each built configs their own way: + +- dev_server read only top-level defaults and let a schema ``enabled: false`` + override its forced ``enabled: True``; +- check_plugin/render_plugin (build_full_config) merged overrides with + dict.update, so ``{"nhl": {"enabled": true}}`` dropped every other nhl + default; +- the harness and the device disagreed on object and array defaults. + +The web saves and GET are covered in +test/web_interface/test_plugin_config_json_saves.py. +""" + +import importlib.util +import json +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system.plugin_manager import PluginManager +from src.plugin_system.schema_manager import ( + CORE_PLUGIN_PROPERTIES, SchemaManager, extract_schema_defaults, +) +from src.plugin_system.testing import loading + +REPO = Path(__file__).resolve().parent.parent + +SCHEMA = { + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": False}, + "global": { + "type": "object", + "properties": { + "dynamic_duration": { + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": True}, + "max_duration_seconds": {"type": "integer", "default": 300}, + }, + }, + }, + }, + "nhl": { + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": False}, + "favorite_teams": {"type": "array", "default": ["TB"]}, + "show_records": {"type": "boolean", "default": True}, + }, + }, + "colors": { + "type": "object", + "default": {"text": [255, 255, 255]}, + "properties": {"text": {"type": "array", "default": [1, 2, 3]}}, + }, + "feeds": {"type": "array", "items": {"type": "string"}}, + }, +} + + +@pytest.fixture +def plugin_dir(tmp_path): + pdir = tmp_path / "plugins" / "demo" + pdir.mkdir(parents=True) + (pdir / "config_schema.json").write_text(json.dumps(SCHEMA)) + (pdir / "manifest.json").write_text(json.dumps({"id": "demo"})) + return pdir + + +@pytest.fixture +def plugin_manager(plugin_dir, tmp_path): + manager = PluginManager.__new__(PluginManager) # skip the heavy constructor + manager.logger = MagicMock() + manager.schema_manager = SchemaManager(plugins_dir=plugin_dir.parent, project_root=tmp_path) + return manager + + +class TestPluginManagerPreparation: + def test_legacy_boolean_and_defaults(self, plugin_manager): + prepared = plugin_manager.prepare_plugin_config( + "demo", {"enabled": True, "global": {"dynamic_duration": True}}) + assert prepared["global"]["dynamic_duration"] == { + "enabled": True, "max_duration_seconds": 300} + assert prepared["nhl"]["favorite_teams"] == ["TB"] + assert prepared["display_duration"] == 15 + + def test_does_not_mutate_the_raw_section(self, plugin_manager): + raw = {"global": {"dynamic_duration": True}} + plugin_manager.prepare_plugin_config("demo", raw) + assert raw == {"global": {"dynamic_duration": True}} + + def test_never_raises(self, plugin_manager): + plugin_manager.schema_manager = MagicMock() + plugin_manager.schema_manager.load_schema.return_value = SCHEMA + plugin_manager.schema_manager.prepare_plugin_config.side_effect = RuntimeError("boom") + prepared = plugin_manager.prepare_plugin_config("demo", {"global": {"dynamic_duration": False}}) + assert prepared["global"]["dynamic_duration"] == {"enabled": False} + + +class TestHotReload: + def test_on_config_change_gets_the_prepared_config(self, test_display_controller, plugin_manager): + controller = test_display_controller + plugin = MagicMock() + plugin.modes = ["demo"] + pm = controller.plugin_manager + pm.discover_plugins.return_value = ["demo"] + pm.load_plugin.return_value = True + pm.plugin_manifests = {} + pm.get_plugin.side_effect = lambda pid: plugin if pid == "demo" else None + pm.prepare_plugin_config.side_effect = plugin_manager.prepare_plugin_config + controller.config_service.get_config = lambda: {"demo": {"enabled": True}} + controller._reconcile_enabled_plugins() + + callback = controller._plugin_config_callbacks["demo"] + callback({"enabled": True}, {"enabled": True, "global": {"dynamic_duration": True}}) + + new_config = plugin.on_config_change.call_args[0][0] + assert new_config["global"]["dynamic_duration"] == { + "enabled": True, "max_duration_seconds": 300} + assert new_config["nhl"]["show_records"] is True + + def test_raw_section_still_delivered_without_a_preparer(self, test_display_controller): + controller = test_display_controller + plugin = MagicMock() + plugin.modes = ["demo"] + pm = controller.plugin_manager + pm.discover_plugins.return_value = ["demo"] + pm.load_plugin.return_value = True + pm.plugin_manifests = {} + pm.get_plugin.side_effect = lambda pid: plugin if pid == "demo" else None + pm.prepare_plugin_config.return_value = None + controller.config_service.get_config = lambda: {"demo": {"enabled": True}} + controller._reconcile_enabled_plugins() + + raw = {"enabled": False} + controller._plugin_config_callbacks["demo"]({}, raw) + plugin.on_config_change.assert_called_once_with(raw) + + +class TestDevToolsMatchTheDevice: + def test_harness_defaults_are_the_device_defaults(self, plugin_dir): + assert loading.load_config_defaults(plugin_dir) == extract_schema_defaults(SCHEMA) + defaults = loading.load_config_defaults(plugin_dir) + # An object's own default wins, as on a device; arrays start empty + assert defaults["colors"] == {"text": [255, 255, 255]} + assert defaults["feeds"] == [] + + def test_nested_override_keeps_sibling_defaults(self, plugin_dir): + config = loading.build_full_config(plugin_dir, cli_config={"nhl": {"enabled": True}}) + assert config["nhl"] == {"enabled": True, "favorite_teams": ["TB"], "show_records": True} + + def test_spec_and_cli_overrides_both_deep_merge(self, plugin_dir): + config = loading.build_full_config( + plugin_dir, spec={"config": {"nhl": {"show_records": False}}}, + cli_config={"nhl": {"enabled": True}}) + assert config["nhl"] == {"enabled": True, "favorite_teams": ["TB"], "show_records": False} + + def test_build_config_matches_the_device_load(self, plugin_dir, plugin_manager): + overrides = {"enabled": True, "global": {"dynamic_duration": False}} + assert loading.build_config(plugin_dir, overrides) == \ + plugin_manager.prepare_plugin_config("demo", overrides) + + def test_core_defaults_are_present(self, plugin_dir): + config = loading.build_full_config(plugin_dir) + assert config["enabled"] is True + assert config["display_duration"] == CORE_PLUGIN_PROPERTIES["display_duration"]["default"] + assert config["live_priority"] is False + + def test_render_plugin_matrix_config(self, plugin_dir, monkeypatch): + from src.plugin_system.testing import harness + seen = {} + + def fake_render_size(plugin_id, manifest, pdir, config, *args, **kwargs): + seen["config"] = config + return [] + + monkeypatch.setattr(harness, "_render_size", fake_render_size) + harness.render_plugin_matrix("demo", plugin_dir, config={"nhl": {"enabled": True}}, + sizes=[(64, 32)], run_update=False) + assert seen["config"]["enabled"] is True, "a schema enabled:false must not win" + assert seen["config"]["nhl"]["favorite_teams"] == ["TB"] + + def test_dev_server_render_config(self, plugin_dir, monkeypatch): + spec = importlib.util.spec_from_file_location("dev_server_under_test", + REPO / "scripts" / "dev_server.py") + dev_server = importlib.util.module_from_spec(spec) + spec.loader.exec_module(dev_server) + monkeypatch.setattr(dev_server, "find_plugin_dir", lambda pid: plugin_dir) + monkeypatch.setattr(dev_server, "_trusted_plugin_dir", lambda d: plugin_dir) + + _, _, config, _, _ = dev_server._parse_render_request( + {"plugin_id": "demo", "config": {"nhl": {"enabled": True}}}) + assert config["enabled"] is True, "a schema enabled:false must not win" + assert config["nhl"] == {"enabled": True, "favorite_teams": ["TB"], "show_records": True} + assert config["global"]["dynamic_duration"]["max_duration_seconds"] == 300 + assert dev_server.load_config_defaults(plugin_dir) == extract_schema_defaults(SCHEMA) diff --git a/test/web_interface/test_plugin_config_json_saves.py b/test/web_interface/test_plugin_config_json_saves.py new file mode 100644 index 00000000..5d0c50f6 --- /dev/null +++ b/test/web_interface/test_plugin_config_json_saves.py @@ -0,0 +1,258 @@ +"""JSON plugin-config saves store what the device would load, and only what was sent. + +Runs a real ConfigManager and SchemaManager over tmp_path, like +test_api_v3_secret_roundtrip.py, so assertions are on config.json itself. + +- POST /plugins/config with a partial ``config`` reset every unsent setting to + its schema default: the JSON path built on defaults, not the stored section. +- Legacy booleans (#588) were normalized only at load, so posting back what + GET /plugins/config returned failed validation. +- The JSON save's filter kept only enabled/display_duration/live_priority, so + a submitted skin, skin_options or vegas_* tuning key was silently dropped. +- Plugin sections posted to /config/main skipped all of that and were stored + verbatim, including values /plugins/config rejects. +""" + +import json +from unittest.mock import MagicMock + +import pytest +from flask import Flask + +from src.config_manager import ConfigManager +from src.plugin_system.schema_manager import SchemaManager +from web_interface.blueprints.api_v3 import api_v3 + +PLUGIN_ID = "demo" + +SCHEMA = { + "type": "object", + "additionalProperties": False, + "properties": { + "enabled": {"type": "boolean", "default": True}, + "city": {"type": "string", "default": "Dallas"}, + "update_interval": {"type": "integer", "default": 300, "minimum": 30}, + "api_key": {"type": "string", "x-secret": True, "default": ""}, + "display": { + "type": "object", + "additionalProperties": False, + "properties": { + "brightness": {"type": "integer", "default": 50}, + "show_icons": {"type": "boolean", "default": True}, + }, + }, + "global": { + "type": "object", + "additionalProperties": False, + "properties": { + "dynamic_duration": { + "type": "object", + "additionalProperties": False, + "properties": { + "enabled": {"type": "boolean", "default": True}, + "min_duration_seconds": {"type": "integer", "default": 30}, + }, + }, + }, + }, + }, +} + +STORED = { + "enabled": True, + "city": "Paris", + "update_interval": 600, + "display": {"brightness": 80, "show_icons": False}, + "global": {"dynamic_duration": {"enabled": False, "min_duration_seconds": 45}}, + "vegas_width_pct": 60, + "skin": "neon", +} + +_ATTRS = ('config_manager', 'plugin_manager', 'plugin_store_manager', + 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'operation_queue', 'operation_history', 'cache_manager') + + +@pytest.fixture +def env(tmp_path): + config_file = tmp_path / "config.json" + secrets_file = tmp_path / "config_secrets.json" + plugins_dir = tmp_path / "plugins" + plugin_dir = plugins_dir / PLUGIN_ID + plugin_dir.mkdir(parents=True) + (plugin_dir / "config_schema.json").write_text(json.dumps(SCHEMA)) + (plugin_dir / "manifest.json").write_text(json.dumps({"id": PLUGIN_ID})) + + sentinel = object() + originals = {name: getattr(api_v3, name, sentinel) for name in _ATTRS} + + config_manager = ConfigManager(config_path=str(config_file), + secrets_path=str(secrets_file)) + config_manager.template_path = str(tmp_path / "no-template.json") + plugin_manager = MagicMock() + plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}} + plugin_manager.plugins_dir = plugins_dir + plugin_manager.get_plugin.return_value = None + + for name in _ATTRS: + setattr(api_v3, name, MagicMock()) + api_v3.config_manager = config_manager + api_v3.schema_manager = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path) + api_v3.plugin_manager = plugin_manager + api_v3.operation_queue = None + + app = Flask(__name__) + app.config["TESTING"] = True + app.register_blueprint(api_v3, url_prefix="/api/v3") + + class Env: + client = app.test_client() + + @staticmethod + def store(section): + config_file.write_text(json.dumps({"timezone": "UTC", PLUGIN_ID: section})) + + @staticmethod + def main(): + return json.loads(config_file.read_text()) + + @staticmethod + def stored(): + return json.loads(config_file.read_text())[PLUGIN_ID] + + @staticmethod + def secrets(): + return json.loads(secrets_file.read_text()) if secrets_file.exists() else {} + + @staticmethod + def save(config): + return app.test_client().post("/api/v3/plugins/config", + json={"plugin_id": PLUGIN_ID, "config": config}) + + Env.store(json.loads(json.dumps(STORED))) + yield Env + + for name, original in originals.items(): + if original is sentinel: + if hasattr(api_v3, name): + delattr(api_v3, name) + else: + setattr(api_v3, name, original) + + +class TestPartialJsonSave: + def test_unsent_settings_keep_their_values(self, env): + resp = env.save({"enabled": True}) + assert resp.status_code == 200, resp.get_json() + stored = env.stored() + assert stored["city"] == "Paris" + assert stored["update_interval"] == 600 + assert stored["display"] == {"brightness": 80, "show_icons": False} + assert stored["global"]["dynamic_duration"] == {"enabled": False, "min_duration_seconds": 45} + + def test_a_nested_partial_keeps_its_siblings(self, env): + resp = env.save({"display": {"brightness": 20}}) + assert resp.status_code == 200, resp.get_json() + assert env.stored()["display"] == {"brightness": 20, "show_icons": False} + + def test_a_sent_setting_still_changes(self, env): + assert env.save({"city": "Lyon", "enabled": False}).status_code == 200 + stored = env.stored() + assert stored["city"] == "Lyon" and stored["enabled"] is False + + def test_an_invalid_sent_value_is_still_rejected(self, env): + resp = env.save({"update_interval": 5}) + assert resp.status_code == 400 + assert env.stored()["update_interval"] == 600 + + def test_config_must_be_an_object(self, env): + assert env.save(["city"]).status_code == 400 + + +class TestCoreOwnedKeysSurviveTheFilter: + def test_submitted_core_keys_are_stored(self, env): + env.store({"enabled": True, "city": "Paris"}) + resp = env.save({ + "vegas_width_pct": 50, "vegas_overflow": "truncate", + "vegas_max_width_screens": 2, "skin": "retro", + "skin_options": {"accent": "#ff0000"}, "live_priority": True, + }) + assert resp.status_code == 200, resp.get_json() + stored = env.stored() + assert stored["vegas_width_pct"] == 50 + assert stored["vegas_overflow"] == "truncate" + assert stored["vegas_max_width_screens"] == 2 + assert stored["skin"] == "retro" + assert stored["skin_options"] == {"accent": "#ff0000"} + assert stored["live_priority"] is True + + def test_stored_core_keys_survive_an_unrelated_save(self, env): + assert env.save({"city": "Nice"}).status_code == 200 + stored = env.stored() + assert stored["vegas_width_pct"] == 60 and stored["skin"] == "neon" + + def test_a_non_core_unknown_key_is_still_filtered(self, env): + assert env.save({"not_in_schema": 1}).status_code == 200 + assert "not_in_schema" not in env.stored() + + +class TestLegacyBooleans: + LEGACY = {"enabled": True, "city": "Paris", "global": {"dynamic_duration": True}} + + def test_get_returns_the_object_shape(self, env): + env.store(dict(self.LEGACY)) + data = env.client.get(f"/api/v3/plugins/config?plugin_id={PLUGIN_ID}").get_json()["data"] + assert data["global"]["dynamic_duration"] == {"enabled": True, "min_duration_seconds": 30} + + def test_get_output_posts_back(self, env): + env.store(dict(self.LEGACY)) + data = env.client.get(f"/api/v3/plugins/config?plugin_id={PLUGIN_ID}").get_json()["data"] + resp = env.save(data) + assert resp.status_code == 200, resp.get_json() + assert env.stored()["global"]["dynamic_duration"] == {"enabled": True, "min_duration_seconds": 30} + + def test_an_unrelated_json_save_upgrades_the_stored_boolean(self, env): + env.store(dict(self.LEGACY)) + resp = env.save({"city": "Lyon"}) + assert resp.status_code == 200, resp.get_json() + assert env.stored()["global"]["dynamic_duration"] == {"enabled": True, "min_duration_seconds": 30} + + def test_a_posted_legacy_boolean_is_read_as_the_object(self, env): + resp = env.save({"global": {"dynamic_duration": False}}) + assert resp.status_code == 200, resp.get_json() + assert env.stored()["global"]["dynamic_duration"]["enabled"] is False + + +class TestPluginSectionsInConfigMain: + def _post(self, env, body): + return env.client.post("/api/v3/config/main", json=body) + + def test_a_value_plugins_config_rejects_is_rejected_here_too(self, env): + resp = self._post(env, {PLUGIN_ID: {"update_interval": "abc"}}) + assert resp.status_code == 400 + assert env.stored()["update_interval"] == 600 + assert env.save({"update_interval": "abc"}).status_code == 400 + + def test_nothing_is_saved_when_a_plugin_section_fails(self, env): + resp = self._post(env, {"timezone": "Europe/Paris", PLUGIN_ID: {"update_interval": 1}}) + assert resp.status_code == 400 + assert env.main()["timezone"] == "UTC" + assert env.stored()["update_interval"] == 600 + + def test_partial_section_merges_and_keeps_core_keys(self, env): + resp = self._post(env, {PLUGIN_ID: {"city": "Lyon", "vegas_overflow": "rotate"}}) + assert resp.status_code == 200, resp.get_json() + stored = env.stored() + assert stored["city"] == "Lyon" + assert stored["display"] == {"brightness": 80, "show_icons": False} + assert stored["vegas_overflow"] == "rotate" + assert stored["vegas_width_pct"] == 60 and stored["skin"] == "neon" + + def test_secrets_still_go_to_the_secrets_file(self, env): + resp = self._post(env, {PLUGIN_ID: {"api_key": "s3cret"}}) + assert resp.status_code == 200, resp.get_json() + assert "api_key" not in env.stored() or env.stored()["api_key"] in ("", None) + assert env.secrets()[PLUGIN_ID]["api_key"] == "s3cret" + + def test_a_non_object_section_is_rejected(self, env): + assert self._post(env, {PLUGIN_ID: "on"}).status_code == 400 diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index 93ff7703..4b0a3374 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -1254,8 +1254,13 @@ def _set_missing_booleans_to_false(plugin_config, schema_props, form_keys, prefi ) def _enhance_schema_with_core_properties(schema): """ - Enhance schema with core plugin properties (enabled, display_duration, live_priority). - These properties are system-managed and should always be allowed even if not in the plugin's schema. + Enhance schema with the core-owned per-plugin properties. + + ``enabled``, ``display_duration``, ``live_priority``, ``skin``, + ``skin_options`` and the ``vegas_*`` tuning keys are system-managed and + always allowed, even when the plugin's schema doesn't declare them. The + list is ``schema_manager.CORE_PLUGIN_PROPERTIES``, the one validation uses, + so the save filter keeps exactly what validation accepts. Args: schema: The original JSON schema dict @@ -1263,44 +1268,31 @@ def _enhance_schema_with_core_properties(schema): Returns: Enhanced schema dict with core properties injected """ - import copy + from src.plugin_system.schema_manager import with_core_plugin_properties if not schema: return schema + return with_core_plugin_properties(schema) - # Core plugin properties that should always be allowed - # These match the definitions in SchemaManager.validate_config_against_schema() - core_properties = { - "enabled": { - "type": "boolean", - "default": True, - "description": "Enable or disable this plugin" - }, - "display_duration": { - "type": "number", - "default": 15, - "minimum": 1, - "maximum": 300, - "description": "How long to display this plugin in seconds" - }, - "live_priority": { - "type": "boolean", - "default": False, - "description": "Enable live priority takeover when plugin has live content" - } - } - # Create a deep copy of the schema to modify (to avoid mutating the original) - enhanced_schema = copy.deepcopy(schema) - if "properties" not in enhanced_schema: - enhanced_schema["properties"] = {} +def _prepared_plugin_config(plugin_id, raw_config): + """A plugin's config section as the plugin runs with it, for on_config_change. + + Loading a plugin reads legacy booleans as objects and fills in schema + defaults (PluginManager.prepare_plugin_config); a save's notification must + hand over the same shape. Falls back to the raw section. + """ + prepare = getattr(api_v3.plugin_manager, 'prepare_plugin_config', None) + if callable(prepare): + try: + prepared = prepare(plugin_id, raw_config) + if isinstance(prepared, dict): + return prepared + except Exception: + logger.debug("Could not prepare config for %s", plugin_id, exc_info=True) + return raw_config - # Inject core properties if they're not already defined in the schema - for prop_name, prop_def in core_properties.items(): - if prop_name not in enhanced_schema["properties"]: - enhanced_schema["properties"][prop_name] = copy.deepcopy(prop_def) - return enhanced_schema def _filter_config_by_schema(config, schema, prefix=''): """ Filter config to only include fields defined in the schema. diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index d7513284..896c6bd9 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -999,33 +999,34 @@ def save_main_config(): current_config['display']['display_durations'][mode_key] = int_value # Handle plugin configurations dynamically - # Any key that matches a plugin ID should be saved as plugin config - # This includes proper secret field handling from schema + # Any key that matches a plugin ID is that plugin's settings. They go + # through the same preparation as POST /plugins/config -- merged onto + # the stored section, legacy booleans and schema defaults applied, + # filtered, validated, secrets split out -- so this route can't store + # a config that one rejects (stored verbatim, it left the plugin + # flagged degraded at its next load). plugin_keys_to_remove = [] + plugin_secrets_updates = {} for key in data: # Check if this key is a plugin ID if api_v3.plugin_manager and key in api_v3.plugin_manager.plugin_manifests: plugin_id = key - plugin_config = data[key] + submitted_config = data[key] + if not isinstance(submitted_config, dict): + return error_response( + ErrorCode.VALIDATION_ERROR, + f"Settings for plugin '{plugin_id}' must be a JSON object", + status_code=400 + ) - # Load plugin schema to identify secret fields (same logic as save_plugin_config) - secret_fields = set() - if api_v3.plugin_manager: - plugins_dir = api_v3.plugin_manager.plugins_dir - else: - plugin_system_config = current_config.get('plugin_system', {}) - plugins_dir_name = plugin_system_config.get('plugins_directory', 'plugin-repos') - if os.path.isabs(plugins_dir_name): - plugins_dir = Path(plugins_dir_name) - else: - plugins_dir = PROJECT_ROOT / plugins_dir_name # plugin_id is already known to be a loaded plugin (the # membership test above), so this cannot currently traverse -- # but the path is built from a request key, and the guard and - # the join are far enough apart that a later edit could - # separate them. Build it through the shared helper instead. - schema_path = resolve_under(plugins_dir, plugin_id, 'config_schema.json') - + # the schema load are far enough apart that a later edit could + # separate them. Refuse rather than save without knowing which + # fields are secrets. + schema_path = resolve_under(api_v3.plugin_manager.plugins_dir, + plugin_id, 'config_schema.json') if schema_path is None: return error_response( ErrorCode.VALIDATION_ERROR, @@ -1033,77 +1034,48 @@ def save_main_config(): status_code=400 ) - if schema_path.exists(): - try: - with open(schema_path, 'r', encoding='utf-8') as f: - schema = json.load(f) - if 'properties' in schema: - secret_fields = find_secret_fields(schema['properties']) - except Exception as e: - logger.debug("Error reading schema for secret detection: %s", e) + schema_mgr = api_v3.schema_manager + if not schema_mgr: + return error_response( + ErrorCode.SYSTEM_ERROR, + 'Schema manager not initialized', + status_code=500 + ) + schema = schema_mgr.load_schema(plugin_id, use_cache=False) - # Separate secrets from regular config (same logic as save_plugin_config) - regular_config, secrets_config = separate_secrets(plugin_config, secret_fields) - # The config form renders secrets masked, so every save posts - # them back blank. Without this the blank is merged over the - # stored value and the credential is destroyed by the act of - # changing an unrelated setting. A blank means "unchanged". - secrets_config = remove_empty_secrets(secrets_config) - - # PRE-PROCESSING: Preserve 'enabled' state if not in regular_config - # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle - if 'enabled' not in regular_config: - try: - if plugin_id in current_config and 'enabled' in current_config[plugin_id]: - regular_config['enabled'] = current_config[plugin_id]['enabled'] - elif api_v3.plugin_manager: - # Fallback to plugin instance if config doesn't have it - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - regular_config['enabled'] = plugin_instance.enabled - # Final fallback: default to True if plugin is loaded (matches BasePlugin default) - if 'enabled' not in regular_config: - regular_config['enabled'] = True - except Exception as e: - logger.debug("Error preserving enabled state: %s", e) - # Default to True on error to avoid disabling plugins - regular_config['enabled'] = True - - # Get current secrets config - current_secrets = api_v3.config_manager.get_raw_file_content('secrets') + from web_interface.blueprints.api_v3.plugins import ( + _merge_onto_stored_plugin_config, _prepare_plugin_config_for_save, + ) + plugin_config = _merge_onto_stored_plugin_config( + plugin_id, submitted_config, current_config) + regular_config, secrets_config, error = _prepare_plugin_config_for_save( + plugin_id, plugin_config, schema, schema_mgr, is_json=True) + if error: + return error # Deep merge regular config into main config - if plugin_id not in current_config: - current_config[plugin_id] = {} - current_config[plugin_id] = deep_merge(current_config[plugin_id], regular_config) - - # Deep merge secrets into secrets config + stored_section = current_config.get(plugin_id) + current_config[plugin_id] = deep_merge( + stored_section if isinstance(stored_section, dict) else {}, regular_config) if secrets_config: - if plugin_id not in current_secrets: - current_secrets[plugin_id] = {} - # Lists merge by replacement, so deep_merge here wrote a - # blanked array straight over the stored credentials. - current_secrets[plugin_id] = merge_secrets( - current_secrets[plugin_id], secrets_config) - # Save secrets file - api_v3.config_manager.save_raw_file_content('secrets', current_secrets) + plugin_secrets_updates[plugin_id] = secrets_config # Mark for removal from data dict (already processed) plugin_keys_to_remove.append(key) - # Notify plugin of config change if loaded (with merged config including secrets) - try: - if api_v3.plugin_manager: - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - # Reload merged config (includes secrets) and pass the plugin-specific section - merged_config = api_v3.config_manager.load_config() - plugin_full_config = merged_config.get(plugin_id, {}) - if hasattr(plugin_instance, 'on_config_change'): - plugin_instance.on_config_change(plugin_full_config) - except Exception as hook_err: - # Don't fail the save if hook fails - logger.warning("on_config_change failed: %s", hook_err) + # Deep merge secrets into secrets config, once every plugin section + # has validated + if plugin_secrets_updates: + current_secrets = api_v3.config_manager.get_raw_file_content('secrets') + for plugin_id, secrets_config in plugin_secrets_updates.items(): + if plugin_id not in current_secrets: + current_secrets[plugin_id] = {} + # Lists merge by replacement, so deep_merge here wrote a + # blanked array straight over the stored credentials. + current_secrets[plugin_id] = merge_secrets( + current_secrets[plugin_id], secrets_config) + # Save secrets file + api_v3.config_manager.save_raw_file_content('secrets', current_secrets) # Remove processed plugin keys from data (they're already in current_config) for key in plugin_keys_to_remove: @@ -1152,6 +1124,19 @@ def save_main_config(): except ImportError: pass + # Notify saved plugins of their new config (with secrets merged), now + # that it is on disk. + for plugin_id in plugin_keys_to_remove: + try: + plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) + if plugin_instance and hasattr(plugin_instance, 'on_config_change'): + merged_config = api_v3.config_manager.load_config() + plugin_instance.on_config_change(_pkg._prepared_plugin_config( + plugin_id, merged_config.get(plugin_id, {}))) + except Exception as hook_err: + # Don't fail the save if hook fails + logger.warning("on_config_change failed: %s", hook_err) + message = 'Configuration saved successfully' # Switching automatic updates on finishes their setup, which needs # the display service to restart (web_interface/auto_update.py). diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index c9643e54..493c6b79 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -792,12 +792,16 @@ def get_plugin_config(): main_config = api_v3.config_manager.load_config() plugin_config = main_config.get(plugin_id, {}) - # Merge with defaults from schema so form shows default values for missing fields + # Merge with defaults from schema so form shows default values for + # missing fields, reading legacy booleans as objects first: what the + # plugin runs with, and what posts back through the JSON save schema_mgr = api_v3.schema_manager if schema_mgr: try: + from src.plugin_system.schema_manager import prepare_plugin_config defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) - plugin_config = schema_mgr.merge_with_defaults(plugin_config, defaults) + plugin_config = prepare_plugin_config( + plugin_config, schema_mgr.load_schema(plugin_id, use_cache=True), defaults) except Exception as e: # Log but don't fail - defaults merge is best effort import logging @@ -1725,7 +1729,14 @@ def save_plugin_config(): if error: return error plugin_id = data['plugin_id'] - plugin_config = data.get('config', {}) + submitted_config = data.get('config', {}) + if not isinstance(submitted_config, dict): + return error_response( + ErrorCode.INVALID_INPUT, + 'config must be a JSON object', + status_code=400 + ) + plugin_config = _merge_onto_stored_plugin_config(plugin_id, submitted_config) else: # Form data (HTMX submission) # plugin_id comes from query string, config from form fields @@ -2162,398 +2173,11 @@ def save_plugin_config(): if 'application/json' in content_type: schema = schema_mgr.load_schema(plugin_id, use_cache=False) - # JSON path: fix numeric-keyed dicts that should be arrays. - # JS dotToNested() converts feeds.custom_feeds.0.name → {'0': {name:...}} - # instead of [{name:...}]. The form-data path has fix_array_structures for this; - # mirror that logic here for JSON submissions. - if 'application/json' in content_type and schema and 'properties' in schema: - def _fix_json_arrays(cfg, props): - for k, ps in props.items(): - if not isinstance(cfg, dict) or k not in cfg: - continue - pt = ps.get('type') - val = cfg[k] - if pt == 'array': - items_schema = ps.get('items', {}) - item_type = items_schema.get('type') - if isinstance(val, dict): - keys = list(val.keys()) - if keys and all(str(x).isdigit() for x in keys): - sorted_keys = sorted(keys, key=lambda x: int(str(x))) - arr = [val[sk] for sk in sorted_keys] - if item_type in ('integer', 'number'): - converted = [] - for v in arr: - if isinstance(v, str): - try: - converted.append(int(v) if item_type == 'integer' else float(v)) - except (ValueError, TypeError, OverflowError): - converted.append(v) - else: - converted.append(v) - arr = converted - cfg[k] = arr - elif not keys: - cfg[k] = [] - # Recurse into each element when items are objects with properties, - # covering both freshly-converted and already-list values. - if item_type == 'object' and 'properties' in items_schema: - for elem in (cfg[k] if isinstance(cfg[k], list) else []): - if isinstance(elem, dict): - _fix_json_arrays(elem, items_schema['properties']) - elif pt == 'object' and 'properties' in ps and isinstance(val, dict): - _fix_json_arrays(val, ps['properties']) - _fix_json_arrays(plugin_config, schema['properties']) - - # PRE-PROCESSING: Preserve 'enabled' state if not in request - # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle - if 'enabled' not in plugin_config: - try: - current_config = api_v3.config_manager.load_config() - if plugin_id in current_config and 'enabled' in current_config[plugin_id]: - plugin_config['enabled'] = current_config[plugin_id]['enabled'] - # logger.debug(f"Preserving enabled state for {plugin_id}: {plugin_config['enabled']}") - elif api_v3.plugin_manager: - # Fallback to plugin instance if config doesn't have it - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - plugin_config['enabled'] = plugin_instance.enabled - # Final fallback: default to True if plugin is loaded (matches BasePlugin default) - if 'enabled' not in plugin_config: - plugin_config['enabled'] = True - except Exception as e: - logger.debug("Error preserving enabled state: %s", e) - # Default to True on error to avoid disabling plugins - plugin_config['enabled'] = True - - # Find secret fields (supports nested schemas and array-item secrets) - secret_fields = set() - - if schema and 'properties' in schema: - secret_fields = find_secret_fields(schema['properties']) - - # Apply defaults from schema to config BEFORE validation - # This ensures required fields with defaults are present before validation - # Store preserved enabled value before merge to protect it from defaults - preserved_enabled = None - if 'enabled' in plugin_config: - preserved_enabled = plugin_config['enabled'] - - if schema: - defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) - plugin_config = schema_mgr.merge_with_defaults(plugin_config, defaults) - - # After merging defaults, replace any None array values with their schema defaults. - # merge_with_defaults gives user config higher priority, so a None submitted by - # the client can survive the merge — this pass cleans those up. - def _fix_none_arrays(cfg, props): - for k, pschema in props.items(): - if pschema.get('type') == 'array': - if isinstance(cfg, dict) and (k not in cfg or cfg[k] is None): - cfg[k] = pschema.get('default', []) - elif pschema.get('type') == 'object' and 'properties' in pschema: - if isinstance(cfg, dict) and isinstance(cfg.get(k), dict): - _fix_none_arrays(cfg[k], pschema['properties']) - - if schema and 'properties' in schema and isinstance(plugin_config, dict): - _fix_none_arrays(plugin_config, schema['properties']) - - # Ensure enabled state is preserved after defaults merge - # Defaults should not overwrite an explicitly preserved enabled value - if preserved_enabled is not None: - # Restore preserved value if it was changed by defaults merge - if plugin_config.get('enabled') != preserved_enabled: - plugin_config['enabled'] = preserved_enabled - - # Normalize config data: convert string numbers to integers/floats where schema expects numbers - # This handles form data which sends everything as strings - def normalize_config_values(config, schema_props, prefix=''): - """Recursively normalize config values based on schema types""" - if not isinstance(config, dict) or not isinstance(schema_props, dict): - return config - - normalized = {} - for key, value in config.items(): - field_path = f"{prefix}.{key}" if prefix else key - - if key not in schema_props: - # Field not in schema, keep as-is (will be caught by additionalProperties check if needed) - normalized[key] = value - continue - - prop_schema = schema_props[key] - prop_type = prop_schema.get('type') - - # Handle union types (e.g., ["integer", "null"]) - if isinstance(prop_type, list): - # Check if null is allowed and value is empty/null - if 'null' in prop_type: - # Handle various representations of null/empty - if value is None: - normalized[key] = None - continue - elif isinstance(value, str): - # Strip whitespace and check for null representations - value_stripped = value.strip() - if value_stripped == '' or value_stripped.lower() in ('null', 'none', 'undefined'): - normalized[key] = None - continue - - # Try to normalize based on non-null types in the union - # Check integer first (more specific than number) - if 'integer' in prop_type: - if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '': - # Empty string with null allowed - already handled above, but double-check - if 'null' in prop_type: - normalized[key] = None - continue - try: - normalized[key] = int(value_stripped) - continue - except (ValueError, TypeError, OverflowError): - pass - elif isinstance(value, (int, float)): - normalized[key] = int(value) - continue - - # Check number (less specific, but handles floats) - if 'number' in prop_type: - if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '': - # Empty string with null allowed - already handled above, but double-check - if 'null' in prop_type: - normalized[key] = None - continue - try: - normalized[key] = float(value_stripped) - continue - except (ValueError, TypeError, OverflowError): - pass - elif isinstance(value, (int, float)): - normalized[key] = float(value) - continue - - # Check boolean - if 'boolean' in prop_type: - if isinstance(value, str): - normalized[key] = value.strip().lower() in ('true', '1', 'on', 'yes') - continue - - # If no conversion worked and null is allowed, try to set to None - # This handles cases where the value is an empty string or can't be converted - if 'null' in prop_type: - if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '' or value_stripped.lower() in ('null', 'none', 'undefined'): - normalized[key] = None - continue - # If it's already None, keep it - if value is None: - normalized[key] = None - continue - - # If no conversion worked, keep original value (will fail validation, but that's expected) - # Log a warning for debugging - logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}") - normalized[key] = value - continue - - if isinstance(value, dict) and prop_type == 'object' and 'properties' in prop_schema: - # Recursively normalize nested objects - normalized[key] = normalize_config_values(value, prop_schema['properties'], field_path) - elif isinstance(value, list) and prop_type == 'array' and 'items' in prop_schema: - # Normalize array items - items_schema = prop_schema['items'] - item_type = items_schema.get('type') - - # Handle union types in array items - if isinstance(item_type, list): - normalized_array = [] - for v in value: - # Check if null is allowed - if 'null' in item_type: - if v is None or v == '' or (isinstance(v, str) and v.lower() in ('null', 'none')): - normalized_array.append(None) - continue - - # Try to normalize based on non-null types - if 'integer' in item_type: - if isinstance(v, str): - try: - normalized_array.append(int(v)) - continue - except (ValueError, TypeError, OverflowError): - pass - elif isinstance(v, (int, float)): - normalized_array.append(int(v)) - continue - elif 'number' in item_type: - if isinstance(v, str): - try: - normalized_array.append(float(v)) - continue - except (ValueError, TypeError, OverflowError): - pass - elif isinstance(v, (int, float)): - normalized_array.append(float(v)) - continue - - # If no conversion worked, keep original value - normalized_array.append(v) - normalized[key] = normalized_array - elif item_type == 'integer': - # Convert string numbers to integers - normalized_array = [] - for v in value: - if isinstance(v, str): - try: - normalized_array.append(int(v)) - except (ValueError, TypeError, OverflowError): - normalized_array.append(v) - elif isinstance(v, (int, float)): - normalized_array.append(int(v)) - else: - normalized_array.append(v) - normalized[key] = normalized_array - elif item_type == 'number': - # Convert string numbers to floats - normalized_array = [] - for v in value: - if isinstance(v, str): - try: - normalized_array.append(float(v)) - except (ValueError, TypeError, OverflowError): - normalized_array.append(v) - else: - normalized_array.append(v) - normalized[key] = normalized_array - elif item_type == 'object' and 'properties' in items_schema: - # Recursively normalize array of objects - normalized_array = [] - for v in value: - if isinstance(v, dict): - normalized_array.append( - normalize_config_values(v, items_schema['properties'], f"{field_path}[]") - ) - else: - normalized_array.append(v) - normalized[key] = normalized_array - else: - normalized[key] = value - elif prop_type == 'integer': - # Convert string to integer - if isinstance(value, str): - try: - normalized[key] = int(value) - except (ValueError, TypeError, OverflowError): - normalized[key] = value - else: - normalized[key] = value - elif prop_type == 'number': - # Convert string to float - if isinstance(value, str): - try: - normalized[key] = float(value) - except (ValueError, TypeError, OverflowError): - normalized[key] = value - else: - normalized[key] = value - elif prop_type == 'boolean': - # Convert string booleans - if isinstance(value, str): - normalized[key] = value.lower() in ('true', '1', 'on', 'yes') - else: - normalized[key] = value - else: - normalized[key] = value - - return normalized - - # Normalize config before validation - if schema and 'properties' in schema: - plugin_config = normalize_config_values(plugin_config, schema['properties']) - - # Filter config to only include schema-defined fields (important when additionalProperties is false) - # Use enhanced schema with core properties to ensure core properties are preserved during filtering - if schema and 'properties' in schema: - enhanced_schema_for_filtering = _enhance_schema_with_core_properties(schema) - plugin_config = _filter_config_by_schema(plugin_config, enhanced_schema_for_filtering) - - # Debug logging for union type fields (temporary) - if 'rotation_settings' in plugin_config and 'random_seed' in plugin_config.get('rotation_settings', {}): - seed_value = plugin_config['rotation_settings']['random_seed'] - logger.debug(f"After normalization, random_seed value: {repr(seed_value)}, type: {type(seed_value)}") - - # Validate configuration against schema before saving - if schema: - # Log what we're validating for debugging - logger.info(f"Validating config for {plugin_id}") - # Only the shape. plugin_config still holds the submitted secret - # values at this point -- separate_secrets does not run until - # below -- so logging it wrote live credentials to the journal. - logger.info(f"Config keys being validated: {list(plugin_config.keys())}") - - # Get enhanced schema keys (including injected core properties) - # We need to create an enhanced schema to get the actual allowed keys - import copy - enhanced_schema = copy.deepcopy(schema) - if "properties" not in enhanced_schema: - enhanced_schema["properties"] = {} - - # Core properties that are always injected during validation - core_properties = ["enabled", "display_duration", "live_priority"] - for prop_name in core_properties: - if prop_name not in enhanced_schema["properties"]: - # Add placeholder to get the full list of allowed keys - enhanced_schema["properties"][prop_name] = {"type": "any"} - - is_valid, validation_errors = schema_mgr.validate_config_against_schema( - plugin_config, schema, plugin_id - ) - if not is_valid: - # Log validation errors for debugging - logger.error(f"Config validation failed for {plugin_id}") - logger.error(f"Validation errors: {validation_errors}") - # Keys only, for the same reason as above. - logger.error(f"Config keys that failed: {list(plugin_config.keys())}") - logger.error(f"Schema properties: {list(enhanced_schema.get('properties', {}).keys())}") - - # Also print to console for immediate visibility - import json - logger.warning("Config validation failed for plugin (see debug logs)") - - # Log raw form data if this was a form submission - if 'application/json' not in (request.content_type or ''): - form_data = request.form.to_dict() - return error_response( - ErrorCode.CONFIG_VALIDATION_FAILED, - 'Configuration validation failed', - details='; '.join(validation_errors) if validation_errors else 'Unknown validation error', - context={ - 'plugin_id': plugin_id, - 'validation_errors': validation_errors, - 'config_keys': list(plugin_config.keys()), - 'schema_keys': list(enhanced_schema.get('properties', {}).keys()) - }, - suggested_fixes=[ - 'Review validation errors above', - 'Check config against schema', - 'Verify all required fields are present' - ], - status_code=400 - ) - - # Separate secrets from regular config (handles nested configs and - # array-item secrets — see src/web_interface/secret_helpers.py) - regular_config, secrets_config = separate_secrets(plugin_config, secret_fields) - # The config form renders secrets masked, so every save posts - # them back blank. Without this the blank is merged over the - # stored value and the credential is destroyed by the act of - # changing an unrelated setting. A blank means "unchanged". - secrets_config = remove_empty_secrets(secrets_config) + regular_config, secrets_config, error = _prepare_plugin_config_for_save( + plugin_id, plugin_config, schema, schema_mgr, + is_json='application/json' in content_type) + if error: + return error # Get current configs current_config = api_v3.config_manager.load_config() @@ -2626,7 +2250,8 @@ def save_plugin_config(): if plugin_instance: # Reload merged config (includes secrets) and pass the plugin-specific section merged_config = api_v3.config_manager.load_config() - plugin_full_config = merged_config.get(plugin_id, {}) + plugin_full_config = _pkg._prepared_plugin_config( + plugin_id, merged_config.get(plugin_id, {})) if hasattr(plugin_instance, 'on_config_change'): plugin_instance.on_config_change(plugin_full_config) @@ -2677,6 +2302,419 @@ def save_plugin_config(): context=error.context, status_code=500 ) +def _merge_onto_stored_plugin_config(plugin_id, submitted_config, current_config=None): + """A JSON plugin-config body merged onto the plugin's stored section. + + A JSON body carries the settings being changed, as a form post does: the + rest keep their stored values. Built from defaults alone, a body such as + ``{"enabled": true}`` reset every unsent setting of the plugin. + ``current_config`` is the loaded main config when the caller has one. + """ + import copy + if current_config is None: + current_config = api_v3.config_manager.load_config() + stored = current_config.get(plugin_id) + base = copy.deepcopy(stored) if isinstance(stored, dict) else {} + return deep_merge(base, submitted_config) + + +def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr, is_json): + """Turn a submitted plugin config into what gets stored. + + Fixes array shapes, keeps the enabled state, reads legacy booleans and + applies schema defaults, normalizes value types, filters to the schema plus + the core-owned per-plugin properties, validates, and splits out secrets. + Shared by POST /plugins/config and plugin sections posted to + POST /config/main, so both store the same thing. + + Returns ``(regular_config, secrets_config, None)``, or + ``(None, None, error_response)`` when validation fails. + """ + # JSON path: fix numeric-keyed dicts that should be arrays. + # JS dotToNested() converts feeds.custom_feeds.0.name → {'0': {name:...}} + # instead of [{name:...}]. The form-data path has fix_array_structures for this; + # mirror that logic here for JSON submissions. + if is_json and schema and 'properties' in schema: + def _fix_json_arrays(cfg, props): + for k, ps in props.items(): + if not isinstance(cfg, dict) or k not in cfg: + continue + pt = ps.get('type') + val = cfg[k] + if pt == 'array': + items_schema = ps.get('items', {}) + item_type = items_schema.get('type') + if isinstance(val, dict): + keys = list(val.keys()) + if keys and all(str(x).isdigit() for x in keys): + sorted_keys = sorted(keys, key=lambda x: int(str(x))) + arr = [val[sk] for sk in sorted_keys] + if item_type in ('integer', 'number'): + converted = [] + for v in arr: + if isinstance(v, str): + try: + converted.append(int(v) if item_type == 'integer' else float(v)) + except (ValueError, TypeError, OverflowError): + converted.append(v) + else: + converted.append(v) + arr = converted + cfg[k] = arr + elif not keys: + cfg[k] = [] + # Recurse into each element when items are objects with properties, + # covering both freshly-converted and already-list values. + if item_type == 'object' and 'properties' in items_schema: + for elem in (cfg[k] if isinstance(cfg[k], list) else []): + if isinstance(elem, dict): + _fix_json_arrays(elem, items_schema['properties']) + elif pt == 'object' and 'properties' in ps and isinstance(val, dict): + _fix_json_arrays(val, ps['properties']) + _fix_json_arrays(plugin_config, schema['properties']) + + # PRE-PROCESSING: Preserve 'enabled' state if not in request + # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle + if 'enabled' not in plugin_config: + try: + current_config = api_v3.config_manager.load_config() + if plugin_id in current_config and 'enabled' in current_config[plugin_id]: + plugin_config['enabled'] = current_config[plugin_id]['enabled'] + # logger.debug(f"Preserving enabled state for {plugin_id}: {plugin_config['enabled']}") + elif api_v3.plugin_manager: + # Fallback to plugin instance if config doesn't have it + plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) + if plugin_instance: + plugin_config['enabled'] = plugin_instance.enabled + # Final fallback: default to True if plugin is loaded (matches BasePlugin default) + if 'enabled' not in plugin_config: + plugin_config['enabled'] = True + except Exception as e: + logger.debug("Error preserving enabled state: %s", e) + # Default to True on error to avoid disabling plugins + plugin_config['enabled'] = True + + # Find secret fields (supports nested schemas and array-item secrets) + secret_fields = set() + + if schema and 'properties' in schema: + secret_fields = find_secret_fields(schema['properties']) + + # Apply defaults from schema to config BEFORE validation + # This ensures required fields with defaults are present before validation + # Store preserved enabled value before merge to protect it from defaults + preserved_enabled = None + if 'enabled' in plugin_config: + preserved_enabled = plugin_config['enabled'] + + if schema: + # Legacy booleans read as {"enabled": ...} objects first (#588), the + # same preparation loading applies, so a config the device reads + # without complaint also saves. + from src.plugin_system.schema_manager import prepare_plugin_config + defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) + plugin_config = prepare_plugin_config(plugin_config, schema, defaults) + + # After merging defaults, replace any None array values with their schema defaults. + # merge_with_defaults gives user config higher priority, so a None submitted by + # the client can survive the merge — this pass cleans those up. + def _fix_none_arrays(cfg, props): + for k, pschema in props.items(): + if pschema.get('type') == 'array': + if isinstance(cfg, dict) and (k not in cfg or cfg[k] is None): + cfg[k] = pschema.get('default', []) + elif pschema.get('type') == 'object' and 'properties' in pschema: + if isinstance(cfg, dict) and isinstance(cfg.get(k), dict): + _fix_none_arrays(cfg[k], pschema['properties']) + + if schema and 'properties' in schema and isinstance(plugin_config, dict): + _fix_none_arrays(plugin_config, schema['properties']) + + # Ensure enabled state is preserved after defaults merge + # Defaults should not overwrite an explicitly preserved enabled value + if preserved_enabled is not None: + # Restore preserved value if it was changed by defaults merge + if plugin_config.get('enabled') != preserved_enabled: + plugin_config['enabled'] = preserved_enabled + + # Normalize config data: convert string numbers to integers/floats where schema expects numbers + # This handles form data which sends everything as strings + def normalize_config_values(config, schema_props, prefix=''): + """Recursively normalize config values based on schema types""" + if not isinstance(config, dict) or not isinstance(schema_props, dict): + return config + + normalized = {} + for key, value in config.items(): + field_path = f"{prefix}.{key}" if prefix else key + + if key not in schema_props: + # Field not in schema, keep as-is (will be caught by additionalProperties check if needed) + normalized[key] = value + continue + + prop_schema = schema_props[key] + prop_type = prop_schema.get('type') + + # Handle union types (e.g., ["integer", "null"]) + if isinstance(prop_type, list): + # Check if null is allowed and value is empty/null + if 'null' in prop_type: + # Handle various representations of null/empty + if value is None: + normalized[key] = None + continue + elif isinstance(value, str): + # Strip whitespace and check for null representations + value_stripped = value.strip() + if value_stripped == '' or value_stripped.lower() in ('null', 'none', 'undefined'): + normalized[key] = None + continue + + # Try to normalize based on non-null types in the union + # Check integer first (more specific than number) + if 'integer' in prop_type: + if isinstance(value, str): + value_stripped = value.strip() + if value_stripped == '': + # Empty string with null allowed - already handled above, but double-check + if 'null' in prop_type: + normalized[key] = None + continue + try: + normalized[key] = int(value_stripped) + continue + except (ValueError, TypeError, OverflowError): + pass + elif isinstance(value, (int, float)): + normalized[key] = int(value) + continue + + # Check number (less specific, but handles floats) + if 'number' in prop_type: + if isinstance(value, str): + value_stripped = value.strip() + if value_stripped == '': + # Empty string with null allowed - already handled above, but double-check + if 'null' in prop_type: + normalized[key] = None + continue + try: + normalized[key] = float(value_stripped) + continue + except (ValueError, TypeError, OverflowError): + pass + elif isinstance(value, (int, float)): + normalized[key] = float(value) + continue + + # Check boolean + if 'boolean' in prop_type: + if isinstance(value, str): + normalized[key] = value.strip().lower() in ('true', '1', 'on', 'yes') + continue + + # If no conversion worked and null is allowed, try to set to None + # This handles cases where the value is an empty string or can't be converted + if 'null' in prop_type: + if isinstance(value, str): + value_stripped = value.strip() + if value_stripped == '' or value_stripped.lower() in ('null', 'none', 'undefined'): + normalized[key] = None + continue + # If it's already None, keep it + if value is None: + normalized[key] = None + continue + + # If no conversion worked, keep original value (will fail validation, but that's expected) + # Log a warning for debugging + logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}") + normalized[key] = value + continue + + if isinstance(value, dict) and prop_type == 'object' and 'properties' in prop_schema: + # Recursively normalize nested objects + normalized[key] = normalize_config_values(value, prop_schema['properties'], field_path) + elif isinstance(value, list) and prop_type == 'array' and 'items' in prop_schema: + # Normalize array items + items_schema = prop_schema['items'] + item_type = items_schema.get('type') + + # Handle union types in array items + if isinstance(item_type, list): + normalized_array = [] + for v in value: + # Check if null is allowed + if 'null' in item_type: + if v is None or v == '' or (isinstance(v, str) and v.lower() in ('null', 'none')): + normalized_array.append(None) + continue + + # Try to normalize based on non-null types + if 'integer' in item_type: + if isinstance(v, str): + try: + normalized_array.append(int(v)) + continue + except (ValueError, TypeError, OverflowError): + pass + elif isinstance(v, (int, float)): + normalized_array.append(int(v)) + continue + elif 'number' in item_type: + if isinstance(v, str): + try: + normalized_array.append(float(v)) + continue + except (ValueError, TypeError, OverflowError): + pass + elif isinstance(v, (int, float)): + normalized_array.append(float(v)) + continue + + # If no conversion worked, keep original value + normalized_array.append(v) + normalized[key] = normalized_array + elif item_type == 'integer': + # Convert string numbers to integers + normalized_array = [] + for v in value: + if isinstance(v, str): + try: + normalized_array.append(int(v)) + except (ValueError, TypeError, OverflowError): + normalized_array.append(v) + elif isinstance(v, (int, float)): + normalized_array.append(int(v)) + else: + normalized_array.append(v) + normalized[key] = normalized_array + elif item_type == 'number': + # Convert string numbers to floats + normalized_array = [] + for v in value: + if isinstance(v, str): + try: + normalized_array.append(float(v)) + except (ValueError, TypeError, OverflowError): + normalized_array.append(v) + else: + normalized_array.append(v) + normalized[key] = normalized_array + elif item_type == 'object' and 'properties' in items_schema: + # Recursively normalize array of objects + normalized_array = [] + for v in value: + if isinstance(v, dict): + normalized_array.append( + normalize_config_values(v, items_schema['properties'], f"{field_path}[]") + ) + else: + normalized_array.append(v) + normalized[key] = normalized_array + else: + normalized[key] = value + elif prop_type == 'integer': + # Convert string to integer + if isinstance(value, str): + try: + normalized[key] = int(value) + except (ValueError, TypeError, OverflowError): + normalized[key] = value + else: + normalized[key] = value + elif prop_type == 'number': + # Convert string to float + if isinstance(value, str): + try: + normalized[key] = float(value) + except (ValueError, TypeError, OverflowError): + normalized[key] = value + else: + normalized[key] = value + elif prop_type == 'boolean': + # Convert string booleans + if isinstance(value, str): + normalized[key] = value.lower() in ('true', '1', 'on', 'yes') + else: + normalized[key] = value + else: + normalized[key] = value + + return normalized + + # Normalize config before validation + if schema and 'properties' in schema: + plugin_config = normalize_config_values(plugin_config, schema['properties']) + + # Filter config to only include schema-defined fields (important when additionalProperties is false) + # Use enhanced schema with core properties to ensure core properties are preserved during filtering + if schema and 'properties' in schema: + enhanced_schema_for_filtering = _enhance_schema_with_core_properties(schema) + plugin_config = _filter_config_by_schema(plugin_config, enhanced_schema_for_filtering) + + # Debug logging for union type fields (temporary) + if 'rotation_settings' in plugin_config and 'random_seed' in plugin_config.get('rotation_settings', {}): + seed_value = plugin_config['rotation_settings']['random_seed'] + logger.debug(f"After normalization, random_seed value: {repr(seed_value)}, type: {type(seed_value)}") + + # Validate configuration against schema before saving + if schema: + # Log what we're validating for debugging + logger.info(f"Validating config for {plugin_id}") + # Only the shape. plugin_config still holds the submitted secret + # values at this point -- separate_secrets does not run until + # below -- so logging it wrote live credentials to the journal. + logger.info(f"Config keys being validated: {list(plugin_config.keys())}") + + # Schema keys including the injected core properties, for the error + enhanced_schema = _enhance_schema_with_core_properties(schema) + + is_valid, validation_errors = schema_mgr.validate_config_against_schema( + plugin_config, schema, plugin_id + ) + if not is_valid: + # Log validation errors for debugging + logger.error(f"Config validation failed for {plugin_id}") + logger.error(f"Validation errors: {validation_errors}") + # Keys only, for the same reason as above. + logger.error(f"Config keys that failed: {list(plugin_config.keys())}") + logger.error(f"Schema properties: {list(enhanced_schema.get('properties', {}).keys())}") + + # Also print to console for immediate visibility + logger.warning("Config validation failed for plugin (see debug logs)") + + return None, None, error_response( + ErrorCode.CONFIG_VALIDATION_FAILED, + 'Configuration validation failed', + details='; '.join(validation_errors) if validation_errors else 'Unknown validation error', + context={ + 'plugin_id': plugin_id, + 'validation_errors': validation_errors, + 'config_keys': list(plugin_config.keys()), + 'schema_keys': list(enhanced_schema.get('properties', {}).keys()) + }, + suggested_fixes=[ + 'Review validation errors above', + 'Check config against schema', + 'Verify all required fields are present' + ], + status_code=400 + ) + + # Separate secrets from regular config (handles nested configs and + # array-item secrets — see src/web_interface/secret_helpers.py) + regular_config, secrets_config = separate_secrets(plugin_config, secret_fields) + # The config form renders secrets masked, so every save posts + # them back blank. Without this the blank is merged over the + # stored value and the credential is destroyed by the act of + # changing an unrelated setting. A blank means "unchanged". + secrets_config = remove_empty_secrets(secrets_config) + + return regular_config, secrets_config, None + + @api_v3.route('/plugins/schema', methods=['GET']) def get_plugin_schema(): """Get plugin configuration schema"""