diff --git a/CHANGELOG.md b/CHANGELOG.md index c81c38fd..804b1cad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,18 @@ accepts both, but the store flags the old spelling as deprecated mid-screen and mid-Vegas included, so the route now only starts the service when it is not running. `POST /display/on-demand/stop` reads `stop_service` as a boolean, so `"false"` no longer stops the service. +- On-demand works for a disabled plugin. The display only loads enabled + plugins, so "Preview on display" on a disabled plugin's config page (which + says the plugin will be enabled for the preview) failed with + `invalid-mode`. The display now loads the plugin live for the session, + without writing `enabled` to `config.json`, and unloads it when on-demand + is stopped, expires or moves to another plugin. A plugin that fails to + load reports on-demand status `error` with `load-failed`. A session + restored after a restart unloads its disabled plugin the same way; it used + to stay loaded until the next restart. +- A stop request now clears an on-demand error. After a failed request, + `/display/on-demand/status` kept reporting `status: error` for up to two + minutes even after a stop. - One hung plugin no longer stops every plugin from updating. The single update worker waited on each plugin's lock with no time limit, and the render thread holds that lock while it runs the plugin's display(); a diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 810d9307..78e990d1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -84,7 +84,11 @@ then normal rotation. - **On-demand.** A request from the web interface pins one plugin (or mode) for a duration. `_activate_on_demand()` / `_clear_on_demand()`; the session is saved under `display_on_demand_config` so it survives a - restart. It also keeps the display on during scheduled off hours. + restart. It also keeps the display on during scheduled off hours. A + request for a plugin that is disabled in config loads it live + (`_load_plugin_for_on_demand()`, `load_plugin(force_enabled=True)`) + without writing `config.json`; the main loop unloads it once on-demand + moves off it (`_release_on_demand_plugins()`). - **Live priority.** `_check_live_priority()` looks for a plugin whose `has_live_priority()` and `has_live_content()` are both true and switches to it, rotating between several live games. diff --git a/src/display_controller.py b/src/display_controller.py index e0a61083..9d61a333 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -29,7 +29,7 @@ import threading import types from collections import deque from contextlib import contextmanager -from typing import Dict, Any, List, Optional, Callable, Tuple +from typing import Dict, Any, List, Optional, Callable, Set, Tuple from datetime import datetime from concurrent.futures import ThreadPoolExecutor, as_completed # pylint: disable=no-name-in-module import pytz @@ -266,6 +266,10 @@ class DisplayController: self.on_demand_last_error: Optional[str] = None self.on_demand_last_event: Optional[str] = None self.on_demand_schedule_override = False + # Plugins that are disabled in config and loaded only because an + # on-demand request named them. The main loop unloads each one once + # on-demand has moved off it (_release_on_demand_plugins). + self._on_demand_loaded_plugins: Set[str] = set() self.rotation_resume_index: Optional[int] = None # Saved rotation position when a live-priority plugin preempts the # rotation, so it resumes where it left off (not after the live plugin) @@ -369,7 +373,11 @@ class DisplayController: """Load a single plugin and return result.""" plugin_load_start = time.time() try: - if self.plugin_manager.load_plugin(plugin_id): + if plugin_id in self._on_demand_loaded_plugins: + loaded = self.plugin_manager.load_plugin(plugin_id, force_enabled=True) + else: + loaded = self.plugin_manager.load_plugin(plugin_id) + if loaded: plugin_load_time = time.time() - plugin_load_start return { 'success': True, @@ -1486,8 +1494,13 @@ class DisplayController: On-demand still resumes on its saved mode; this only widens what gets loaded, so normal rotation has somewhere to return to when it ends. A plugin that is disabled in config but named by the on-demand request - is still enabled and added, since otherwise the mode being resumed - would have nothing behind it. + is still loaded, since otherwise the mode being resumed would have + nothing behind it. It is tracked as loaded for on-demand only, the + same as one loaded live by _activate_on_demand, so it is unloaded + when the session ends instead of staying loaded until the next + restart. Its config section is not touched: setting ``enabled`` in + self.config wrote into the dict config_manager caches and returns to + every later load_config() in this process. """ enabled_plugins = [p for p in discovered_plugins if self.config.get(p, {}).get('enabled', False)] @@ -1502,11 +1515,10 @@ class DisplayController: logger.warning("Falling back to normal mode (all enabled plugins)") return enabled_plugins - if not self.config.get(on_demand_plugin_id, {}).get('enabled', False): - logger.info("Temporarily enabling plugin '%s' for on-demand mode", on_demand_plugin_id) - self.config.setdefault(on_demand_plugin_id, {})['enabled'] = True - if on_demand_plugin_id not in enabled_plugins: - enabled_plugins.append(on_demand_plugin_id) + if on_demand_plugin_id not in enabled_plugins: + logger.info("Loading disabled plugin '%s' for on-demand mode only", on_demand_plugin_id) + self._on_demand_loaded_plugins.add(on_demand_plugin_id) + enabled_plugins.append(on_demand_plugin_id) # Restore on-demand state from the cached request so it resumes. self.on_demand_active = True @@ -1602,6 +1614,11 @@ class DisplayController: logger.debug("Stop request %s received but on-demand is not active", request_id) # Still update request_id to acknowledge the request self.on_demand_request_id = request_id + if self.on_demand_status == 'error': + # A failed request left status 'error' published, and + # without this the status route kept reporting it until + # the state aged out (120s) or another request came in. + self._clear_on_demand(reason='requested-stop') # Stop requests are deliberately exempt from the request_id/ # processed_id guards above, so that a second click stops a mode # that a race left running. Consuming the mailbox is therefore the @@ -1768,10 +1785,136 @@ class DisplayController: plugin_id, ordered_modes, self.on_demand_mode_index, ordered_modes[self.on_demand_mode_index] if ordered_modes else 'N/A') + def _load_plugin_for_on_demand(self, plugin_id: str) -> bool: + """Load an installed plugin that isn't running so on-demand can show it. + + This process only loads the plugins enabled in config, so a request + for a disabled one -- the config page's "Preview on display" button + offers it on every plugin -- failed with "invalid-mode" while the UI + said the plugin would be enabled for the session. Nothing did that + short of a restart, and restarts no longer happen on a request. + + Loads through the same path as a live enable (load_plugin, then + _register_loaded_plugin), with force_enabled so the instance runs + enabled while config.json keeps saying disabled. The plugin is + recorded in _on_demand_loaded_plugins, and the main loop unloads it + once on-demand moves off it (_release_on_demand_plugins). + + Returns False after publishing an error when the load fails. A + plugin that isn't installed returns True without loading anything: + the mode checks that follow report it as they always have. + """ + if self.plugin_manager is None: + return True + try: + known = self.plugin_manager.discovered_plugin_ids() + except AttributeError: + known = set(getattr(self.plugin_manager, 'plugin_manifests', ()) or ()) + if plugin_id not in known: + # Installed after this process scanned: the web process checked + # its own, fresher list before posting the request. + try: + known = set(self.plugin_manager.discover_plugins()) + except Exception: # pylint: disable=broad-except + logger.exception("On-demand: plugin discovery failed") + known = set() + if plugin_id not in known: + return True + + logger.info("On-demand: loading disabled plugin '%s' for this session only", plugin_id) + self._on_demand_loaded_plugins.add(plugin_id) + try: + loaded = self.plugin_manager.load_plugin(plugin_id, force_enabled=True) + if loaded: + modes = self._register_loaded_plugin(plugin_id) + logger.info("On-demand: loaded plugin '%s' (modes: %s)", plugin_id, modes) + except Exception: # pylint: disable=broad-except + logger.exception("On-demand: error loading plugin '%s'", plugin_id) + loaded = False + if not loaded: + # Stays in _on_demand_loaded_plugins so the main loop removes + # whatever part of it did get registered. + logger.error("On-demand: could not load plugin '%s'", plugin_id) + self._set_on_demand_error("load-failed") + return False + return True + + def _release_on_demand_plugins(self) -> None: + """Unload plugins loaded only for on-demand that it has moved off. + + Runs from the main loop, right after its own on-demand poll, not + where on-demand ends: a stop, an expiry or the next request is often + read from inside a render loop or a dwell sleep, where the plugin + being released may still be on the stack mid-display(). Unloading + goes through _unregister_plugin, as a live disable does, and nothing + is written to config.json. + + A plugin the user enabled in the meantime stays loaded and takes its + place in the rotation, which is what the reconcile that the enable + queued would have done. + """ + if self.plugin_manager is None: # plugin system failed after startup restore + self._on_demand_loaded_plugins.clear() + return + keep = self.on_demand_plugin_id if self.on_demand_active else None + releasable = [p for p in self._on_demand_loaded_plugins if p != keep] + if not releasable: + return + try: + config = self.config_service.get_config() + except Exception as e: # pylint: disable=broad-except + logger.warning("On-demand release: falling back to cached config: %s", e) + config = self.config + previous_mode = self.current_display_mode + for plugin_id in releasable: + self._on_demand_loaded_plugins.discard(plugin_id) + section = config.get(plugin_id) + if isinstance(section, dict) and section.get('enabled', False): + logger.info("On-demand: keeping plugin '%s' loaded; it was enabled " + "while on-demand showed it", plugin_id) + continue + if (plugin_id in self.plugin_display_modes + or self.plugin_manager.get_plugin(plugin_id) is not None): + logger.info("On-demand: unloading plugin '%s'; it is disabled in config", + plugin_id) + self._unregister_plugin(plugin_id) + if not self.on_demand_active: + # Only outside a session: rotation_resume_index points into + # available_modes until the session ends. + self._apply_plugin_rotation_order() + self._resync_mode_index_after_change(previous_mode) + if self.current_display_mode != previous_mode: + self.force_change = True + + def _rotation_index_outside_on_demand(self, start: int) -> Optional[int]: + """First index from `start` (wrapping) whose mode is not owned by a + plugin loaded only for on-demand, or None if every mode is. + + Ending a session must not resume the rotation onto the plugin that + is about to be unloaded. A live load appends that plugin's modes + after the saved resume index, but a session restored after a + restart has no saved index and its plugin was ordered in with the + rest -- the rotation resumed onto it, and a stop read during its own + screen changed nothing on the panel until that screen ended. + """ + if not self._on_demand_loaded_plugins: + return start + on_demand_only = {mode for plugin_id in self._on_demand_loaded_plugins + for mode in self.plugin_display_modes.get(plugin_id, [])} + count = len(self.available_modes) + for step in range(count): + index = (start + step) % count + if self.available_modes[index] not in on_demand_only: + return index + return None + def _activate_on_demand(self, request: Dict[str, Any]) -> None: """Activate on-demand mode for a specific plugin display.""" plugin_id = request.get('plugin_id') mode = request.get('mode') + if (plugin_id and plugin_id not in self.plugin_display_modes + and not self._load_plugin_for_on_demand(plugin_id)): + return resolved_mode = self._resolve_mode_for_plugin(plugin_id, mode) if not resolved_mode: @@ -1877,6 +2020,15 @@ class DisplayController: self.on_demand_last_event = 'stop-request-ignored' # Already idle self._publish_on_demand_state() return + if not self.on_demand_active and self.on_demand_status == 'error': + # _set_on_demand_error already ended any session and dropped + # rotation_resume_index; the full clear below would only move + # the rotation and force a redraw. Just drop the error. + self.on_demand_status = 'idle' + self.on_demand_last_error = None + self.on_demand_last_event = reason or 'cleared' + self._publish_on_demand_state() + return self._reset_on_demand_fields() self.on_demand_status = 'idle' @@ -1886,17 +2038,27 @@ class DisplayController: # Clear on-demand configuration from cache self.cache_manager.clear_cache('display_on_demand_config') - if self.rotation_resume_index is not None and self.available_modes: - self.current_mode_index = self.rotation_resume_index % len(self.available_modes) - self.current_display_mode = self.available_modes[self.current_mode_index] - logger.info("Resuming rotation from saved index %d: mode '%s'", - self.rotation_resume_index, self.current_display_mode) - elif self.available_modes: - # Default to first mode if no resume index - self.current_mode_index = self.current_mode_index % len(self.available_modes) - self.current_display_mode = self.available_modes[self.current_mode_index] - logger.info("Resuming rotation to mode '%s' (index %d)", - self.current_display_mode, self.current_mode_index) + if self.available_modes: + saved = self.rotation_resume_index + # Default to the current index if no resume index + start = saved if saved is not None else self.current_mode_index + index = self._rotation_index_outside_on_demand(start % len(self.available_modes)) + if index is None: + # Every mode belongs to a plugin loaded only for on-demand, + # which the main loop is about to unload; it then idles. + self.current_mode_index = 0 + self.current_display_mode = None + logger.info("No enabled mode to resume rotation to") + elif saved is not None: + self.current_mode_index = index + self.current_display_mode = self.available_modes[index] + logger.info("Resuming rotation from saved index %d: mode '%s'", + saved, self.current_display_mode) + else: + self.current_mode_index = index + self.current_display_mode = self.available_modes[index] + logger.info("Resuming rotation to mode '%s' (index %d)", + self.current_display_mode, self.current_mode_index) else: logger.warning("No available modes to resume rotation to") @@ -2093,6 +2255,14 @@ class DisplayController: # Handle on-demand commands before rendering self._poll_on_demand_requests() self._check_on_demand_expiration() + # Unload plugins loaded only to show them on-demand once it + # has moved off them. Here, where no display() is on the + # stack; one ended from inside a screen is caught here on + # the next pass. + if self._on_demand_loaded_plugins: + self._release_on_demand_plugins() + if not self.available_modes: + continue # it was all there was; idle as above self._tick_plugin_updates() # Clean up expired WiFi status messages @@ -3126,6 +3296,11 @@ class DisplayController: prepared = prepare(_pid, new_config) if callable(prepare) else None if isinstance(prepared, dict): new_config = prepared + if _pid in self._on_demand_loaded_plugins: + # Saved while on-demand shows it: config.json still + # says disabled, and on_config_change would switch + # the instance off mid-session. + new_config = {**new_config, 'enabled': True} # Runs on ConfigService's watcher thread. Under the # plugin's lock, so it cannot interleave with update() # on the worker or display() on the render thread; a diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index e9089afc..87ae4642 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -353,7 +353,7 @@ class PluginManager: return plugin_ids - def load_plugin(self, plugin_id: str) -> bool: + def load_plugin(self, plugin_id: str, force_enabled: bool = False) -> bool: """ Load a plugin by ID. @@ -367,6 +367,10 @@ class PluginManager: Args: plugin_id: Plugin identifier + force_enabled: Run the plugin enabled even though config.json has + it disabled. On-demand uses this to show a disabled plugin + (DisplayController._load_plugin_for_on_demand). Only the + instance's config says enabled; config.json is not written. Returns: True if loaded successfully, False otherwise @@ -433,6 +437,12 @@ class PluginManager: # (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) + if force_enabled: + # A copy: prepare_plugin_config can hand back the section from + # config_manager's cached config, and setting the flag there + # would read as enabled to everything else in this process. + config = dict(config) + config['enabled'] = True # Use PluginLoader to load plugin plugin_instance, _module = self.plugin_loader.load_plugin( diff --git a/test/conftest.py b/test/conftest.py index c04502e4..93ae4881 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -16,7 +16,54 @@ if str(project_root) not in sys.path: sys.path.insert(0, str(project_root)) +class _DisarmStartupReconciliation: + """Import hook: every ``web_interface.app`` this process builds starts disarmed. + + app.py wires itself to the checkout's real config/config.json and + plugin-repos/ at import, and its before_request hook launches startup + reconciliation on the first request any test sends. Reconciliation + reinstalls every configured plugin missing on disk from the live store, + so a full run downloaded basketball-scoreboard, calendar, + football-scoreboard, leaderboard and ledmatrix-stocks into the real + plugin-repos/ (not gitignored), minutes in, from a daemon thread no test + waits on. Setting ``_reconciliation_started`` is the app's own run-once + latch; doing it as the module finishes executing covers fixtures that + import the app lazily and send a request at once, and ``importlib.reload``. + StateReconciliation itself stays fully testable. + """ + + _MODULE = "web_interface.app" + + def find_spec(self, fullname, path, target=None): + if fullname != self._MODULE: + return None + import importlib.machinery + spec = importlib.machinery.PathFinder.find_spec(fullname, path, target) + if spec is None or spec.loader is None: + return spec + exec_module = spec.loader.exec_module + + def exec_disarmed(module): + exec_module(module) + module._reconciliation_started = True + + spec.loader.exec_module = exec_disarmed + return spec + + +_DISARM_HOOK = _DisarmStartupReconciliation() + + def pytest_configure(config): + sys.meta_path.insert(0, _DISARM_HOOK) + app_module = sys.modules.get(_DisarmStartupReconciliation._MODULE) + if app_module is not None: + app_module._reconciliation_started = True + + _point_emulator_at_raw_adapter(config) + + +def _point_emulator_at_raw_adapter(config): """Point the emulator at a per-process config that binds no socket. Six test modules set EMULATOR=true and build a real DisplayManager. The @@ -63,7 +110,9 @@ def pytest_configure(config): def pytest_unconfigure(config): - """Remove the throwaway emulator config written by pytest_configure.""" + """Undo pytest_configure: the import hook and the throwaway emulator config.""" + if _DISARM_HOOK in sys.meta_path: + sys.meta_path.remove(_DISARM_HOOK) tmp_dir = getattr(config, "_ledmatrix_emulator_tmp", None) if tmp_dir is not None: import shutil diff --git a/test/test_on_demand_disabled_plugin.py b/test/test_on_demand_disabled_plugin.py new file mode 100644 index 00000000..a7c52f80 --- /dev/null +++ b/test/test_on_demand_disabled_plugin.py @@ -0,0 +1,426 @@ +"""On-demand for a plugin that is installed but disabled in config. + +The display process only loads enabled plugins, so a request for a disabled +one -- "Preview on display" offers it on every plugin's config page, with a +note that the plugin will be enabled for the preview -- failed with +"invalid-mode". Nothing loaded it short of a restart, and the on-demand +route no longer restarts the service. + +The display now loads such a plugin live for the session (force_enabled, so +config.json keeps saying disabled) and the main loop unloads it once +on-demand moves off it: a stop, an expiry, or a request for another plugin. + +Also here: a stop sent after a failed request clears the error instead of +leaving status 'error' published until the state ages out. +""" + +import time +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system.plugin_manager import PluginManager +from src.plugin_system.plugin_state import PluginState + + +def _make_plugin(modes): + plugin = MagicMock() + plugin.modes = list(modes) + return plugin + + +@pytest.fixture +def controller(test_display_controller): + """An idle controller running 'clock', with 'preview-me' installed but disabled.""" + c = test_display_controller + clock = _make_plugin(['clock']) + preview = _make_plugin(['preview_a', 'preview_b']) + instances = {'clock': clock} + catalogue = {'clock': clock, 'preview-me': preview} + + def load_plugin(plugin_id, force_enabled=False): + instances[plugin_id] = catalogue[plugin_id] + return True + + def unload_plugin(plugin_id): + return instances.pop(plugin_id, None) is not None + + pm = c.plugin_manager + pm.discovered_plugin_ids.return_value = set(catalogue) + pm.discover_plugins.return_value = list(catalogue) + pm.plugin_manifests = {} + pm.load_plugin = MagicMock(side_effect=load_plugin) + pm.unload_plugin = MagicMock(side_effect=unload_plugin) + pm.get_plugin.side_effect = instances.get + + config = {'clock': {'enabled': True}, 'preview-me': {'enabled': False}} + c.config_service.get_config = lambda: config + c.config_manager.save_config = MagicMock() + c.cache_manager.set = MagicMock() + c.cache_manager.clear_cache = MagicMock() + + c._register_loaded_plugin('clock') + c.current_mode_index = 0 + c.current_display_mode = 'clock' + c.test_config = config + c.test_instances = instances + return c + + +def _start(c, plugin_id='preview-me', mode=None, **extra): + request = {'request_id': 'r-' + plugin_id, 'action': 'start', + 'plugin_id': plugin_id, 'mode': mode or plugin_id} + request.update(extra) + c._activate_on_demand(request) + + +class TestLoadingForOnDemand: + def test_a_disabled_plugin_is_loaded_and_shown(self, controller): + _start(controller) + + controller.plugin_manager.load_plugin.assert_called_once_with( + 'preview-me', force_enabled=True) + assert controller.on_demand_active is True + assert controller.on_demand_status == 'active' + assert controller.on_demand_plugin_id == 'preview-me' + assert controller.current_display_mode == 'preview_a' + assert controller.plugin_display_modes['preview-me'] == ['preview_a', 'preview_b'] + + def test_config_json_is_not_written(self, controller): + _start(controller) + + controller.config_manager.save_config.assert_not_called() + assert controller.test_config['preview-me'] == {'enabled': False} + + def test_a_requested_mode_is_honoured(self, controller): + _start(controller, mode='preview_b') + assert controller.current_display_mode == 'preview_b' + + def test_an_enabled_plugin_is_not_reloaded(self, controller): + _start(controller, plugin_id='clock') + + controller.plugin_manager.load_plugin.assert_not_called() + assert controller.on_demand_active is True + assert controller._on_demand_loaded_plugins == set() + + def test_a_plugin_that_is_not_installed_is_not_loaded(self, controller): + _start(controller, plugin_id='uninstalled') + + controller.plugin_manager.load_plugin.assert_not_called() + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'invalid-mode' + + def test_a_plugin_installed_after_startup_is_found_by_rescanning(self, controller): + controller.plugin_manager.discovered_plugin_ids.return_value = {'clock'} + + _start(controller) + + controller.plugin_manager.discover_plugins.assert_called() + assert controller.on_demand_active is True + + +class TestLoadFailures: + def test_a_failed_load_reports_load_failed(self, controller): + controller.plugin_manager.load_plugin = MagicMock(return_value=False) + + _start(controller) + + assert controller.on_demand_active is False + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'load-failed' + assert 'preview_a' not in controller.available_modes + published = controller.cache_manager.set.call_args_list[-1] + assert published.args[0] == 'display_on_demand_state' + assert published.args[1]['status'] == 'error' + assert published.args[1]['error'] == 'load-failed' + + def test_a_load_that_raises_reports_load_failed(self, controller): + controller.plugin_manager.load_plugin = MagicMock(side_effect=ImportError('no module')) + + _start(controller) + + assert controller.on_demand_status == 'error' + assert controller.on_demand_last_error == 'load-failed' + + def test_a_failed_load_leaves_the_rotation_alone(self, controller): + controller.plugin_manager.load_plugin = MagicMock(return_value=False) + + _start(controller) + controller._release_on_demand_plugins() + + assert controller.available_modes == ['clock'] + assert controller.current_display_mode == 'clock' + assert controller._on_demand_loaded_plugins == set() + controller.plugin_manager.unload_plugin.assert_not_called() + + def test_a_plugin_that_loads_but_has_no_modes_is_unloaded_again(self, controller): + """Registered, then the activation fails: the release removes it.""" + controller._on_demand_modes_for_plugin = MagicMock(return_value=[]) + + _start(controller) + assert controller.on_demand_last_error == 'no-modes' + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + + +class TestReleasingThePlugin: + def test_it_stays_loaded_while_on_demand_shows_it(self, controller): + _start(controller) + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_not_called() + assert 'preview_a' in controller.plugin_modes + + def test_a_stop_unloads_it_and_resumes_the_rotation(self, controller): + _start(controller) + controller._clear_on_demand(reason='requested-stop') + # Deferred to the main loop: the stop may be read mid-display(). + controller.plugin_manager.unload_plugin.assert_not_called() + + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + assert 'preview-me' not in controller.plugin_display_modes + assert 'preview_a' not in controller.plugin_modes + assert controller.current_display_mode == 'clock' + assert controller._on_demand_loaded_plugins == set() + assert controller.test_config['preview-me'] == {'enabled': False} + + def test_expiry_unloads_it(self, controller): + _start(controller, duration=30) + controller.on_demand_expires_at = time.time() - 1 + controller._check_on_demand_expiration() + controller._release_on_demand_plugins() + + assert controller.on_demand_last_event == 'expired' + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + + def test_a_request_for_another_plugin_unloads_it(self, controller): + _start(controller) + _start(controller, plugin_id='clock') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.on_demand_active is True + assert controller.on_demand_plugin_id == 'clock' + assert controller.current_display_mode == 'clock' + + def test_a_failed_request_that_ends_the_session_unloads_it(self, controller): + _start(controller) + _start(controller, plugin_id='uninstalled') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.current_display_mode == 'clock' + assert controller.force_change is True + + def test_a_plugin_enabled_during_the_session_stays_loaded(self, controller): + _start(controller) + controller.test_config['preview-me'] = {'enabled': True} + controller._clear_on_demand(reason='requested-stop') + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_not_called() + assert 'preview_a' in controller.available_modes + assert controller._on_demand_loaded_plugins == set() + + def test_the_main_loop_releases_right_after_its_own_poll(self, controller): + """A stop read by the main loop unloads before the next screen, not + one screen later. That poll runs with no display() on the stack.""" + import inspect + source = inspect.getsource(type(controller).run) + poll = source.index('self._check_on_demand_expiration()') + release = source.index('self._release_on_demand_plugins()') + render = source.index('self._tick_plugin_updates()') + assert poll < release < render + + def test_a_reconcile_that_runs_first_unloads_it_the_same_way(self, controller): + """A reconcile queued during the session runs at the top of the loop, + before the release: it removes the plugin itself (not in the enabled + set) and the release is then a no-op.""" + _start(controller) + controller._clear_on_demand(reason='requested-stop') + + controller._reconcile_enabled_plugins() + controller._release_on_demand_plugins() + + controller.plugin_manager.unload_plugin.assert_called_once_with('preview-me') + assert controller.available_modes == ['clock'] + assert controller._on_demand_loaded_plugins == set() + + def test_a_config_save_mid_session_keeps_the_instance_enabled(self, controller): + """on_config_change would otherwise read enabled: false and switch it off.""" + controller.config_service.subscribe = MagicMock() + _start(controller) + callback = controller._plugin_config_callbacks['preview-me'] + controller.plugin_manager.prepare_plugin_config = None + + callback({}, {'enabled': False, 'color': 'red'}) + + # The change goes through the manager's locked apply_config_change + # (which calls on_config_change under the plugin's lock). + plugin = controller.plugin_modes['preview_a'] + controller.plugin_manager.apply_config_change.assert_called_once_with( + 'preview-me', {'enabled': True, 'color': 'red'}, plugin_instance=plugin) + + +class TestRestoredSession: + """A restart during a session for a disabled plugin restores it the same way.""" + + def test_the_plugin_is_tracked_and_config_is_left_alone(self, test_display_controller): + c = test_display_controller + c.config.update({'clock': {'enabled': True}, 'disabled-one': {'enabled': False}}) + + selected = c._select_startup_plugins( + ['clock', 'disabled-one'], {'plugin_id': 'disabled-one', 'mode': 'x'}) + + assert 'disabled-one' in selected + assert c._on_demand_loaded_plugins == {'disabled-one'} + assert c.config['disabled-one']['enabled'] is False + + +class TestResumingAfterTheSession: + """Ending a session never resumes the rotation onto the plugin that is + about to be unloaded.""" + + def _restored_session(self, c, other_modes=('clock',)): + """As after a restart: no saved resume index, and the plugin's modes + ordered in ahead of the rest (load order is not deterministic).""" + c._on_demand_loaded_plugins.add('preview-me') + c.plugin_manager.load_plugin('preview-me', force_enabled=True) + c._register_loaded_plugin('preview-me') + c.available_modes = ['preview_a', 'preview_b'] + list(other_modes) + c.on_demand_active = True + c.on_demand_status = 'active' + c.on_demand_plugin_id = 'preview-me' + c.on_demand_modes = ['preview_a', 'preview_b'] + c.rotation_resume_index = None + c.current_mode_index = 0 + c.current_display_mode = 'preview_a' + + def test_a_restored_session_resumes_on_an_enabled_mode(self, controller): + self._restored_session(controller) + + controller._clear_on_demand(reason='requested-stop') + + assert controller.current_display_mode == 'clock' + controller._release_on_demand_plugins() + assert controller.available_modes == ['clock'] + assert controller.current_display_mode == 'clock' + + def test_with_nothing_else_enabled_the_display_goes_idle(self, controller): + controller._unregister_plugin('clock') + self._restored_session(controller, other_modes=()) + + controller._clear_on_demand(reason='requested-stop') + assert controller.current_display_mode is None + + controller._release_on_demand_plugins() + assert controller.available_modes == [] + assert controller.current_display_mode is None + + def test_a_saved_resume_index_is_still_used(self, controller): + c = controller + c.available_modes = ['clock', 'other'] + c.plugin_modes['other'] = MagicMock() + c.current_mode_index = 1 + c.current_display_mode = 'other' + + _start(c) + c._clear_on_demand(reason='requested-stop') + + assert c.current_display_mode == 'other' + + +class TestStopClearsAnError: + def _post_stop(self, c): + stop = {'request_id': 'S1', 'action': 'stop'} + c._last_on_demand_poll = None + c.cache_manager.get = MagicMock( + side_effect=lambda key, *a, **kw: + stop if key == 'display_on_demand_request' else None) + c.cache_manager.delete = MagicMock() + c._poll_on_demand_requests() + + def test_a_stop_after_a_failed_request_clears_the_error(self, controller): + _start(controller, plugin_id='uninstalled') + assert controller.on_demand_status == 'error' + + self._post_stop(controller) + + assert controller.on_demand_status == 'idle' + assert controller.on_demand_last_error is None + state = controller.cache_manager.set.call_args_list[-1].args[1] + assert state['status'] == 'idle' + assert state['error'] is None + + def test_clearing_the_error_leaves_the_rotation_alone(self, controller): + _start(controller, plugin_id='uninstalled') + controller.force_change = False + + self._post_stop(controller) + + assert controller.current_display_mode == 'clock' + assert controller.force_change is False + + def test_a_stop_while_idle_is_still_just_acknowledged(self, controller): + controller._clear_on_demand = MagicMock() + + self._post_stop(controller) + + assert controller.on_demand_status == 'idle' + assert controller.on_demand_request_id == 'S1' + controller._clear_on_demand.assert_not_called() + + +class TestForceEnabledLoad: + """PluginManager.load_plugin(force_enabled=True) runs the plugin enabled + without touching the config it read.""" + + class _Plugin: + def __init__(self, config): + self.config = config + self.enabled_calls = 0 + + def on_enable(self): + self.enabled_calls += 1 + + @pytest.fixture + def pm(self, tmp_path): + plugins_dir = tmp_path / 'plugins' + (plugins_dir / 'demo').mkdir(parents=True) + manager = PluginManager(plugins_dir=str(plugins_dir)) + manager.plugin_manifests['demo'] = {'id': 'demo', 'name': 'Demo'} + manager.schema_manager = MagicMock() + manager.schema_manager.get_schema_path.return_value = None + # Hand the section back as-is, as the fallback path can: the copy in + # load_plugin is what keeps the cached config clean. + manager.schema_manager.prepare_plugin_config.side_effect = ( + lambda pid, cfg, schema=None, changed_paths=None: cfg) + manager.plugin_loader = MagicMock() + manager.plugin_loader.find_plugin_directory.return_value = plugins_dir / 'demo' + manager.plugin_loader.load_plugin.side_effect = ( + lambda **kw: (self._Plugin(kw['config']), None)) + manager.config_manager = MagicMock() + manager.cached_config = {'demo': {'enabled': False, 'color': 'red'}} + manager.config_manager.load_config.return_value = manager.cached_config + return manager + + def test_a_disabled_plugin_loads_disabled_by_default(self, pm): + assert pm.load_plugin('demo') is True + assert pm.plugins['demo'].enabled_calls == 0 + assert pm.state_manager.get_state('demo') == PluginState.DISABLED + + def test_force_enabled_runs_it_enabled(self, pm): + assert pm.load_plugin('demo', force_enabled=True) is True + plugin = pm.plugins['demo'] + assert plugin.config == {'enabled': True, 'color': 'red'} + assert plugin.enabled_calls == 1 + assert pm.state_manager.get_state('demo') == PluginState.ENABLED + + def test_force_enabled_does_not_touch_the_cached_config(self, pm): + pm.load_plugin('demo', force_enabled=True) + assert pm.cached_config['demo'] == {'enabled': False, 'color': 'red'} diff --git a/test/test_on_demand_pinning_and_restart.py b/test/test_on_demand_pinning_and_restart.py index eb33f434..a82baa9a 100644 --- a/test/test_on_demand_pinning_and_restart.py +++ b/test/test_on_demand_pinning_and_restart.py @@ -164,12 +164,14 @@ class TestRestartDoesNotStarveTheOtherPlugins: assert controller.on_demand_mode == 'app_a' assert controller.on_demand_pinned is True - def test_a_disabled_on_demand_plugin_is_enabled_and_loaded(self, controller): - """Otherwise the mode being resumed has nothing behind it.""" + def test_a_disabled_on_demand_plugin_is_still_loaded(self, controller): + """Otherwise the mode being resumed has nothing behind it. It loads + for on-demand only; its config section is left disabled.""" selected = controller._select_startup_plugins( self.DISCOVERED, {'plugin_id': 'disabled-one', 'mode': 'x'}) assert 'disabled-one' in selected - assert controller.config['disabled-one']['enabled'] is True + assert controller._on_demand_loaded_plugins == {'disabled-one'} + assert controller.config['disabled-one']['enabled'] is False def test_an_unknown_on_demand_plugin_falls_back_to_normal(self, controller): selected = controller._select_startup_plugins( diff --git a/test/web_interface/test_startup_reconciliation_disarmed.py b/test/web_interface/test_startup_reconciliation_disarmed.py new file mode 100644 index 00000000..f18c3e5c --- /dev/null +++ b/test/web_interface/test_startup_reconciliation_disarmed.py @@ -0,0 +1,23 @@ +""" +No test may start the real app's startup reconciliation. + +web_interface/app.py reads the checkout's real config/config.json and +plugin-repos/ at import, and its first request launches a reconciliation +thread that reinstalls every configured-but-missing plugin from the live +store. A full suite run on a dev checkout used to leave whole plugins +untracked in plugin-repos/ that way. test/conftest.py disarms the run-once +latch on every import of the module; this pins that. +""" + +from unittest.mock import MagicMock, patch + + +def test_a_request_to_the_imported_app_launches_no_reconciliation(): + import web_interface.app as web_app + + assert web_app._reconciliation_started is True + + with patch.object(web_app, "threading", MagicMock()) as threading_mock: + web_app.app.test_client().get("/favicon.ico") + + threading_mock.Thread.assert_not_called() diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index 6cb2ca4c..40bb83a3 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -180,9 +180,9 @@ def start_on_demand_display(): if not resolved_plugin: return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 - # Note: On-demand can work with disabled plugins - the display controller - # will temporarily enable them during initialization if needed - # We don't block the request here, but log it for debugging + # On-demand works with disabled plugins: the running display loads one + # for the session and unloads it afterwards, leaving config.json alone + # (DisplayController._load_plugin_for_on_demand). Logged for debugging. if api_v3.config_manager and resolved_plugin: config = api_v3.config_manager.load_config() plugin_config = config.get(resolved_plugin, {})