From 7eb7a58d0cd8c5a70daefc45364661106cfdafed Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 28 Sep 2026 08:26:05 -0400 Subject: [PATCH] fix: web UI and src.common bugs (wifi wrong-password, plugin icon, starlark toggle, API caching, scroll/logo/font helpers) (#646) - wifi: keep the "wrong_password:" prefix through the restore/AP fallback so the UI's incorrect-password prompt fires again. - /plugins/installed returns the manifest's icon (string only). - /starlark/apps//toggle coerces `enabled` and delegates to _toggle_starlark_app (disk before memory, no KeyError, "false" is false). - /api/v3/ JSON GETs are sent Cache-Control: no-store; non-JSON keeps 5s. - ScrollHelper.set_scrolling_image converts non-RGB input (alpha onto black); create/set_scrolling_image reset last_update_time like reset_scroll. - LogoHelper backs off a failed download per path for MISSING_LOGO_RECHECK_SECONDS; cleared on invalidate/clear_cache. - refresh_placeholder_timestamp saves atomically. - FontManager.clear_cache / _clear_plugin_font_cache bump cache_generation. - Odds manager: per-game logs to DEBUG; JSON decode error caught before RequestException (same cooldown). - element_style mangled continuations; startup validator skips null plugin blocks and reuses the controller's discovery. - src/common/README lists frame_timing, json_body, render_gate. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 12 ++ docs/PLUGIN_CUSTOM_ICONS.md | 12 +- src/base_odds_manager.py | 21 ++-- src/common/README.md | 29 +++++ src/common/logo_helper.py | 15 ++- src/common/scroll_helper.py | 18 +++ src/display_controller.py | 12 +- src/element_style.py | 4 +- src/font_manager.py | 6 + src/logo_downloader.py | 5 +- src/startup_validator.py | 18 ++- src/wifi_manager.py | 19 ++- test/test_api_v3_installed_plugin_icon.py | 46 +++++++ test/test_base_odds_manager.py | 29 +++++ test/test_common_readme_lists_every_module.py | 21 ++++ ...t_display_controller_startup_validation.py | 19 ++- test/test_font_manager.py | 18 +++ test/test_logo_downloader.py | 12 ++ test/test_logo_helper_download_backoff.py | 89 ++++++++++++++ test/test_scroll_helper_image_entry.py | 87 +++++++++++++ test/test_startup_validator.py | 22 ++++ test/test_wifi_wrong_password_signal.py | 78 ++++++++++++ .../web_interface/test_api_json_not_cached.py | 51 ++++++++ .../test_starlark_app_toggle_route.py | 116 ++++++++++++++++++ web_interface/app.py | 12 +- web_interface/blueprints/api_v3/plugins.py | 3 + web_interface/blueprints/api_v3/starlark.py | 54 ++++---- 27 files changed, 764 insertions(+), 64 deletions(-) create mode 100644 test/test_api_v3_installed_plugin_icon.py create mode 100644 test/test_common_readme_lists_every_module.py create mode 100644 test/test_logo_helper_download_backoff.py create mode 100644 test/test_scroll_helper_image_entry.py create mode 100644 test/test_wifi_wrong_password_signal.py create mode 100644 test/web_interface/test_api_json_not_cached.py create mode 100644 test/web_interface/test_starlark_app_toggle_route.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6926c5b2..f1acf71c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,18 @@ accepts both, but the store flags the old spelling as deprecated - `check_system_compatibility.sh` no longer reports installed packages as missing. - A network failure fetching GitHub repo info logs a warning, not an error. +- Web UI and `src.common` fixes: + - A wrong Wi-Fi password is reported as one again ("Incorrect password for ..."); the fallback that restores the old network or brings up the setup AP was replacing the signal. + - Plugin tabs show the manifest's `icon`: `/api/v3/plugins/installed` now includes it. + - `POST /api/v3/starlark/apps//toggle` goes through the same code as `/plugins/toggle`: `"false"` disables, a failed save no longer leaves the running app out of step with disk, and a loaded app with no manifest entry no longer answers 500. + - `/api/v3/` JSON responses are sent `Cache-Control: no-store`, so a reload right after an install, toggle or Wi-Fi connect shows the new state. Non-JSON files served through the API keep the 5 s cache. + - `ScrollHelper.set_scrolling_image()` accepts RGBA, L and palette images (transparent pixels become black), and a new scrolling image no longer jumps ahead by the time the helper sat idle. + - `LogoHelper.load_logo_with_download()` waits an hour before retrying a download that failed for a missing logo, instead of retrying (with a 30 s timeout) on every call. + - Restamping a placeholder logo writes the file atomically. + - `FontManager.clear_cache()` and unregistering a plugin's fonts bump `cache_generation`, so cached layouts are rebuilt. + - The odds manager logs cache hits, misses and fetches at DEBUG, and a bad JSON body is logged as a parse error rather than a failed fetch. + - Startup plugin validation no longer gives up on a `null` plugin block, and plugins are discovered once at startup instead of twice. + - `src/common/README.md` lists `frame_timing`, `json_body` and `render_gate`. - Plugin system: - A plugin whose `on_enable()` raises is no longer left registered: the next load retries it instead of reporting "already loaded" for a plugin that never ran. - One plugin's `get_info()` raising no longer breaks the installed-plugins list; it is logged and shown with empty runtime info. diff --git a/docs/PLUGIN_CUSTOM_ICONS.md b/docs/PLUGIN_CUSTOM_ICONS.md index 0d7c761a..3c3dacdb 100644 --- a/docs/PLUGIN_CUSTOM_ICONS.md +++ b/docs/PLUGIN_CUSTOM_ICONS.md @@ -5,11 +5,9 @@ A plugin can name an icon for its tab in the web interface's second nav row (next to **Plugin Manager**) with the `icon` field in `manifest.json`. -> **Status:** the tab code honors `icon`, but `GET /api/v3/plugins/installed` -> (`web_interface/blueprints/api_v3/plugins.py`) does not currently include -> the manifest's `icon` in its response, so every tab shows the default -> puzzle piece. Setting `icon` is harmless and will take effect once the API -> passes it through again. +`GET /api/v3/plugins/installed` passes the manifest's `icon` through (a +non-string value comes back as `null`), and a plugin without one gets the +default puzzle piece. ## Font Awesome classes only @@ -55,8 +53,8 @@ With no `icon` (or an empty one) the tab shows `fas fa-puzzle-piece`. or misspelled class renders as a blank space. 2. Include the style prefix (`fas`, `far` or `fab`) as well as the icon class. -3. See the status note above: the icon is currently not passed through by - the API. +3. The manifest is re-read on each plugin list load; reload the page after + editing `icon`. ## Related Documentation diff --git a/src/base_odds_manager.py b/src/base_odds_manager.py index 548dcfba..c79d04e7 100644 --- a/src/base_odds_manager.py +++ b/src/base_odds_manager.py @@ -123,8 +123,10 @@ class BaseOddsManager: # Check cache first cached_data = self.cache_manager.get_with_auto_strategy(cache_key) + # Per-game chatter, logged on every update of every game on the + # slate: debug, not the journal. if cached_data: - self.logger.info(f"Using cached odds from ESPN for {cache_key}") + self.logger.debug(f"Using cached odds from ESPN for {cache_key}") return cached_data if time.monotonic() < self._skip_network_until: @@ -137,7 +139,7 @@ class BaseOddsManager: self._skip_network_until - time.monotonic()) return None - self.logger.info(f"Cache miss - fetching fresh odds from ESPN for {cache_key}") + self.logger.debug(f"Cache miss - fetching fresh odds from ESPN for {cache_key}") try: # Map league names to ESPN API format @@ -151,7 +153,7 @@ class BaseOddsManager: espn_league = league_mapping.get(league, league) url = f"{self.base_url}/{sport}/leagues/{espn_league}/events/{event_id}/competitions/{event_id}/odds" - self.logger.info(f"Requesting odds from URL: {url}") + self.logger.debug(f"Requesting odds from URL: {url}") response = self.session.get(url, timeout=self.request_timeout) response.raise_for_status() @@ -163,9 +165,9 @@ class BaseOddsManager: odds_data = self._extract_espn_data(raw_data) if odds_data: - self.logger.info(f"Successfully extracted odds data: {odds_data}") + self.logger.debug(f"Successfully extracted odds data: {odds_data}") self.cache_manager.set(cache_key, odds_data, ttl=interval) - self.logger.info(f"Saved odds data to cache for {cache_key} with TTL {interval}s") + self.logger.debug(f"Saved odds data to cache for {cache_key} with TTL {interval}s") else: self.logger.debug(f"No odds data available for {cache_key}") # Cache the absence too, so the game is not re-requested @@ -174,14 +176,19 @@ class BaseOddsManager: return odds_data + # Before RequestException: requests' JSONDecodeError subclasses it, so + # listed second this branch never ran and a bad body was reported as a + # failed fetch. It holds off like a failed fetch did, so only the + # message changes. + except (json.JSONDecodeError, requests.exceptions.JSONDecodeError): + self._skip_network_until = time.monotonic() + self._FAILURE_COOLDOWN + self.logger.error(f"Error decoding JSON response from ESPN API for {cache_key}.") except requests.exceptions.RequestException as e: self._skip_network_until = time.monotonic() + self._FAILURE_COOLDOWN self.logger.error( "Error fetching odds from ESPN API for %s: %s. Holding off on odds " "for %.0fs so a slate of games does not pay this timeout each.", cache_key, e, self._FAILURE_COOLDOWN) - except json.JSONDecodeError: - self.logger.error(f"Error decoding JSON response from ESPN API for {cache_key}.") return self.cache_manager.get_with_auto_strategy(cache_key) diff --git a/src/common/README.md b/src/common/README.md index 68c22404..0179b604 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -27,9 +27,12 @@ Rules for the package: | [`bdf_font`](#bdf_font) | Load and draw BDF bitmap fonts | Yes, if drawing BDF text directly | Unreleased | | [`espn_dates`](#espn_dates) | Fetch ESPN scoreboards across a date range | Yes (scoreboards) | 3.5.0 | | [`font_layout`](#font_layout) | Reproducible TrueType loading, crisp sizes | Yes | 3.4.0 | +| [`frame_timing`](#frame_timing) | Timing of every presented frame, stall watchdog | No, core-internal | n/a | +| [`json_body`](#json_body) | Parse a response body as JSON, with orjson if installed | Optional (large payloads) | Unreleased | | [`logo_helper`](#logo_helper) | Load, resize and cache team logos | Yes | — | | [`path_safety`](#path_safety) | Turn request-supplied names into safe paths | No, core-internal | n/a | | [`permission_utils`](#permission_utils) | File modes and shared-group ownership | Rarely | — | +| [`render_gate`](#render_gate) | Keep background Python off the GIL while the panel swaps | No, core-internal | n/a | | [`scroll_config`](#scroll_config) | Plugin scroll config → configured `ScrollHelper` | Yes (scrollers) | 3.4.0 | | [`scroll_helper`](#scroll_helper) | Pre-rendered horizontal scrolling | Yes | — | | [`snapshot_policy`](#snapshot_policy) | When to write the web preview frame | No, core-internal | n/a | @@ -107,6 +110,24 @@ the size a bundled face renders on whole pixels at. `resolve_asset_path()` resolves `assets/fonts/...` against the install root rather than the working directory. +### frame_timing + +[`frame_timing.py`](frame_timing.py). Core-internal. `DisplayManager` +records every presented frame in a `FrameTimingRecorder`, which writes +cumulative late-frame counters and histograms to `/dev/shm` for +`scripts/frame_soak.py` and `scripts/render_bench.py`. `StallWatchdog` logs +the stack of whatever holds up a scroll. See +[docs/SCROLL_PERFORMANCE.md](../../docs/SCROLL_PERFORMANCE.md). + +### json_body + +[`json_body.py`](json_body.py). `response_json(response)` is +`response.json()` parsed by orjson when it is installed, falling back to the +stdlib parser (and requests' own error) otherwise. For multi-MB payloads such +as a season schedule, where the parse holds the GIL and freezes the display. +A plugin that also runs on older cores should guard the import, as +`espn_dates` does. + ### logo_helper [`logo_helper.py`](logo_helper.py). `LogoHelper(display_width, @@ -134,6 +155,14 @@ let the root display service and the web user share files: already call these; a plugin needs them only when it creates its own files outside the cache. See [docs/PERMISSIONS.md](../../docs/PERMISSIONS.md). +### render_gate + +[`render_gate.py`](render_gate.py). Core-internal. `RenderGate` is opened by +the render thread around each vsync swap; a background thread inside +`gate.yielding()` (Vegas's prefetch) parks while the gate is closed, so the +render thread finds the GIL free when its refresh arrives. It never parks a +thread holding a guarded lock or inside logging, threading or import code. + ### scroll_config [`scroll_config.py`](scroll_config.py). `configure(scroll_helper, diff --git a/src/common/logo_helper.py b/src/common/logo_helper.py index 9ead2536..59a712d6 100644 --- a/src/common/logo_helper.py +++ b/src/common/logo_helper.py @@ -80,6 +80,11 @@ class LogoHelper: # Time-bounded rather than permanent so a logo that appears later (the # downloader writes them at runtime) is still picked up. self._missing_logos: Dict[str, float] = {} + + # Failed downloads by logo path. A logo that is absent (not a stale + # placeholder) has no on-disk timestamp to back off on, so without this + # every call retried the download -- up to a 30s timeout each time. + self._download_failures: Dict[str, float] = {} # Session for HTTP requests self.session = requests.Session() @@ -204,7 +209,12 @@ class LogoHelper: return self.load_logo(team_abbr, logo_path, max_width, max_height, scale) - # Download if URL provided and file doesn't exist + # Download if URL provided and file doesn't exist, unless the last + # attempt for this path failed recently. + failed_at = self._download_failures.get(str(logo_path)) + if (logo_url and failed_at is not None + and time.time() - failed_at < MISSING_LOGO_RECHECK_SECONDS): + logo_url = None if logo_url: try: self.logger.info(f"Downloading logo for {team_abbr} from {logo_url}") @@ -218,6 +228,7 @@ class LogoHelper: scale) except Exception as e: self.logger.error(f"Failed to download logo for {team_abbr}: {e}") + self._download_failures[str(logo_path)] = time.time() # The retry failed, so restart the back-off. The stale # placeholder is still on disk with its old timestamp, and # leaving it there means the next call retries immediately -- @@ -240,6 +251,7 @@ class LogoHelper: # leaving it would hide a logo we just downloaded. for key in [k for k in self._missing_logos if k.startswith(prefix)]: del self._missing_logos[key] + self._download_failures.pop(str(logo_path), None) @staticmethod def _refresh_stale_placeholder(logo_path: Path) -> None: @@ -332,6 +344,7 @@ class LogoHelper: self._logo_cache.clear() self._cache_order.clear() self._missing_logos.clear() + self._download_failures.clear() self.logger.debug("Logo cache cleared") def get_cache_stats(self) -> Dict[str, int]: diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index fe76942a..21f694b4 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -254,6 +254,9 @@ class ScrollHelper: now = time.time() self.scroll_start_time = now self.last_progress_log_time = now + # The position just went back to 0; the first update must not advance + # it by however long the helper sat idle (off-screen) before this. + self.last_update_time = now self.logger.info( "Dynamic duration target set to %ds (min=%ds, max=%ds, buffer=%.2f)", self.calculated_duration, @@ -776,6 +779,18 @@ class ScrollHelper: self.clear_cache() return + # Every frame is cut from cached_array with Image.frombytes('RGB', ...), + # which reads a 4-channel (RGBA) array as garbage and raises on a + # 1-channel (L) one. Transparent pixels go to black, the panel's + # background, rather than to whatever colour hides under the alpha. + if image.mode != 'RGB': + if 'A' in image.mode or 'transparency' in image.info: + rgba = image.convert('RGBA') + image = Image.new('RGB', rgba.size, (0, 0, 0)) + image.paste(rgba, (0, 0), rgba) + else: + image = image.convert('RGB') + # Set the cached image self.cached_image = image @@ -802,6 +817,9 @@ class ScrollHelper: self.scroll_start_time = now self.last_progress_log_time = now self.last_step_time = now # Initialize step timer for frame-based scrolling + # The position just went back to 0; the first update must not advance + # it by however long the helper sat idle before this image arrived. + self.last_update_time = now self.logger.debug("Set scrolling image: %dx%d, total_scroll_width=%d", image.width, image.height, self.total_scroll_width) diff --git a/src/display_controller.py b/src/display_controller.py index bcb4634a..bd2a5fa8 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -318,13 +318,19 @@ class DisplayController: except Exception as e: logger.warning("Could not enable plugin health/resource monitoring: %s", e) + # Discover plugins. Before the plugin checks so they can reuse the + # list: each discover_plugins() call rescans the plugins directory + # and logs every plugin again. + discovered_plugins = self.plugin_manager.discover_plugins() + logger.info("Discovered %d plugin(s)", len(discovered_plugins)) + # Only the plugin checks: validate_all() above has run the rest, # and running it again logged every config warning twice. try: from src.startup_validator import StartupValidator validator = StartupValidator(self.config_manager, self.plugin_manager, cache_manager=self.cache_manager) - validator._validate_plugins() + validator._validate_plugins(discovered_plugins=discovered_plugins) for warning in validator.warnings: logger.warning("Plugin validation warning: %s", warning) if validator.errors: @@ -333,10 +339,6 @@ class DisplayController: except Exception as e: logger.warning("Plugin validation could not be completed: %s", e) - # Discover plugins - discovered_plugins = self.plugin_manager.discover_plugins() - logger.info("Discovered %d plugin(s)", len(discovered_plugins)) - # Check for on-demand plugin filter from cache on_demand_config = self.cache_manager.get('display_on_demand_config', max_age=3600) enabled_plugins = self._select_startup_plugins(discovered_plugins, on_demand_config) diff --git a/src/element_style.py b/src/element_style.py index 15e1af4b..f4e47fc9 100644 --- a/src/element_style.py +++ b/src/element_style.py @@ -840,7 +840,7 @@ def defaults_from_schema(schema: Dict[str, Any]) -> Dict[str, Any]: defaults['align'] = align_spec['default'] scale_spec = spec.get('scale') if isinstance(scale_spec, dict) and 'default' in scale_spec: - layout.setdefault(element_key, {})['scale'] = scale_spec['default'] + layout.setdefault(element_key, {})['scale'] = scale_spec['default'] elif scale_spec is True: layout.setdefault(element_key, {})['scale'] = 1.0 if defaults: @@ -856,7 +856,7 @@ def defaults_from_schema(schema: Dict[str, Any]) -> Dict[str, Any]: continue scale_prop = (block.get('properties') or {}).get('scale') if isinstance(scale_prop, dict) and 'default' in scale_prop: - layout.setdefault(element_key, {})['scale'] = scale_prop['default'] + layout.setdefault(element_key, {})['scale'] = scale_prop['default'] for element_key, block in properties.items(): if element_key in ('layout', 'modes') or element_key in elements: continue diff --git a/src/font_manager.py b/src/font_manager.py index 38f2f173..ef5b8d8f 100644 --- a/src/font_manager.py +++ b/src/font_manager.py @@ -426,6 +426,9 @@ class FontManager: keys_to_remove = [key for key in self.font_cache.keys() if key.startswith(f"{plugin_id}::")] for key in keys_to_remove: del self.font_cache[key] + if keys_to_remove: + # Font objects someone may hold were dropped; see cache_generation. + self.cache_generation += 1 @deprecated("3.7.0") def get_plugin_fonts(self, plugin_id: str) -> List[str]: @@ -732,6 +735,9 @@ class FontManager: """Clear font and metrics cache.""" self.font_cache.clear() self.metrics_cache.clear() + # Holders of derived caches (layout fits, font usage) key off this; + # without the bump they kept serving results for the dropped fonts. + self.cache_generation += 1 logger.info("Font cache cleared") @deprecated("3.7.0", "read font_catalog") diff --git a/src/logo_downloader.py b/src/logo_downloader.py index c042188a..b7fa239a 100644 --- a/src/logo_downloader.py +++ b/src/logo_downloader.py @@ -235,7 +235,10 @@ def refresh_placeholder_timestamp(filepath: Path) -> bool: metadata = PngInfo() metadata.add_text(PLACEHOLDER_MARKER, str(time.time())) with Image.open(filepath) as img: - img.copy().save(filepath, "PNG", pnginfo=metadata) + image = img.copy() + # Atomically, like every other logo write: a renderer can open this + # file at any moment, and an in-place save exposes a truncated PNG. + save_png_atomically(image, filepath, pnginfo=metadata) return True except Exception: logger.debug("Could not refresh placeholder timestamp for %s", filepath, diff --git a/src/startup_validator.py b/src/startup_validator.py index 14c4c227..c3daf5f5 100644 --- a/src/startup_validator.py +++ b/src/startup_validator.py @@ -268,15 +268,21 @@ class StartupValidator: except Exception as e: self.warnings.append(f"Could not validate display configuration: {e}") - def _validate_plugins(self) -> None: - """Validate plugin configurations and dependencies.""" + def _validate_plugins(self, discovered_plugins=None) -> None: + """Validate plugin configurations and dependencies. + + ``discovered_plugins`` is a list the caller already got from + ``discover_plugins()``; passing it skips a second directory scan (and + its duplicate log lines) at startup. + """ if not self.plugin_manager: return try: # Get enabled plugins from config config = self.config_manager.get_config() - discovered_plugins = self.plugin_manager.discover_plugins() + if discovered_plugins is None: + discovered_plugins = self.plugin_manager.discover_plugins() # Check for enabled plugins that don't exist for plugin_id, plugin_config in config.items(): @@ -294,7 +300,11 @@ class StartupValidator: # Validate plugin configurations for plugin_id in discovered_plugins: - plugin_config = config.get(plugin_id, {}) + plugin_config = config.get(plugin_id) + # A null block ("my-plugin": null) is not an enabled plugin; + # .get() on it raised and abandoned every remaining check. + if not isinstance(plugin_config, dict): + continue if plugin_config.get('enabled', False): # Check if plugin can be loaded (without actually loading it) plugin_dir = self.plugin_manager.get_plugin_directory(plugin_id) diff --git a/src/wifi_manager.py b/src/wifi_manager.py index 86dde71f..88f3c420 100644 --- a/src/wifi_manager.py +++ b/src/wifi_manager.py @@ -1317,6 +1317,15 @@ class WiFiManager: if self.has_nmcli: success, message = self._connect_nmcli(ssid, password) + # Keep the "wrong_password:" prefix on whatever failure message + # the recovery below returns: the web UI keys its "incorrect + # password" prompt off it, and the restore/AP messages would + # otherwise erase it. + wrong_password = (not success and isinstance(message, str) + and message.startswith("wrong_password:")) + + def _fail(msg): + return False, (f"wrong_password: {msg}" if wrong_password else msg) # If connection failed, try to restore original connection if not success and original_connection and original_ssid: @@ -1327,18 +1336,18 @@ class WiFiManager: if restore_success: logger.info(f"Successfully restored original connection: {original_ssid}") self._show_led_message("Restored!", duration=3) - return False, f"Failed to connect to {ssid}, restored {original_ssid}" + return _fail(f"Failed to connect to {ssid}, restored {original_ssid}") else: logger.error(f"Failed to restore original connection: {original_ssid}") - return self._failsafe_ap( + return _fail(self._failsafe_ap( "Connection failed and restoration failed. AP mode enabled.", - "Connection failed, restoration failed, and AP mode failed") + "Connection failed, restoration failed, and AP mode failed")[1]) # If connection failed and no original connection to restore, enable AP mode elif not success: logger.warning(f"Connection to {ssid} failed and no original connection to restore") - return self._failsafe_ap("Connection failed. AP mode enabled.", - "Connection failed and AP mode failed") + return _fail(self._failsafe_ap("Connection failed. AP mode enabled.", + "Connection failed and AP mode failed")[1]) return success, message else: diff --git a/test/test_api_v3_installed_plugin_icon.py b/test/test_api_v3_installed_plugin_icon.py new file mode 100644 index 00000000..984a9160 --- /dev/null +++ b/test/test_api_v3_installed_plugin_icon.py @@ -0,0 +1,46 @@ +"""GET /api/v3/plugins/installed carries the manifest's `icon`. + +The tab nav sets ``iconEl.className = plugin.icon || 'fas fa-puzzle-piece'`` +(app-shell.js, app-early.js), but the route never included `icon`, so every +plugin tab showed the default puzzle piece. +""" + +from unittest.mock import MagicMock + +import pytest + +from test._api_v3_test_helpers import ( # noqa: F401 - fixtures + api_v3_client, api_v3_module, +) + + +@pytest.fixture +def installed(api_v3_module, api_v3_client, tmp_path): + def _get(manifest_extra): + api = api_v3_module.api_v3 + info = {'id': 'demo', 'name': 'Demo', 'version': '1.0.0', 'loaded': False} + info.update(manifest_extra) + api.plugin_manager.plugins_dir = str(tmp_path) # no manifest on disk + api.plugin_manager.get_all_plugin_info = MagicMock(return_value=[info]) + api.plugin_manager.get_plugin = MagicMock(return_value=None) + api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.config_manager.load_config = MagicMock(return_value={}) + response = api_v3_client.get('/api/v3/plugins/installed') + assert response.status_code == 200 + plugins = [p for p in response.get_json()['data']['plugins'] if p['id'] == 'demo'] + assert len(plugins) == 1 + return plugins[0] + return _get + + +def test_the_manifest_icon_is_passed_through(installed): + assert installed({'icon': 'fas fa-cloud-sun'})['icon'] == 'fas fa-cloud-sun' + + +def test_no_icon_comes_back_as_null(installed): + # The JS falls back to the puzzle piece on a falsy value. + assert installed({})['icon'] is None + + +def test_a_non_string_icon_is_not_passed_through(installed): + assert installed({'icon': {'class': 'fas fa-star'}})['icon'] is None diff --git a/test/test_base_odds_manager.py b/test/test_base_odds_manager.py index ad7cec4b..1fa15d18 100644 --- a/test/test_base_odds_manager.py +++ b/test/test_base_odds_manager.py @@ -13,6 +13,7 @@ its requests through a session so it can identify itself to ESPN, so patching the module-level requests.get would no longer intercept anything. """ +import logging from unittest.mock import MagicMock, patch import pytest @@ -161,6 +162,34 @@ class TestGetOdds: assert result == {'stale': True} assert cache_manager.get_with_auto_strategy.call_count == 2 + def test_a_bad_body_is_reported_as_a_parse_error( + self, manager, cache_manager, mock_get, caplog): + # requests' JSONDecodeError is a RequestException, so the JSON branch + # sat unreachable behind it and a bad body read as a failed fetch. + response = _make_response(None) + response.json.side_effect = requests.exceptions.JSONDecodeError('x', 'doc', 0) + mock_get.return_value = response + cache_manager.get_with_auto_strategy.side_effect = [None, {'stale': True}] + + with caplog.at_level(logging.ERROR, logger=manager.logger.name): + result = manager.get_odds('football', 'nfl', '401') + + assert result == {'stale': True} + messages = [r.getMessage() for r in caplog.records] + assert any('decoding JSON' in m for m in messages), messages + assert not any('Error fetching odds' in m for m in messages), messages + # The hold-off is unchanged: a bad body still backs off like a failed + # fetch did. + assert manager._skip_network_until > 0 + + def test_routine_fetches_log_nothing_at_info( + self, manager, cache_manager, mock_get, caplog): + with caplog.at_level(logging.INFO, logger=manager.logger.name): + manager.get_odds('football', 'nfl', '401') # miss + fetch + cache_manager.get_with_auto_strategy.return_value = {'spread': 1} + manager.get_odds('football', 'nfl', '401') # hit + assert [r for r in caplog.records if r.levelno == logging.INFO] == [] + # --------------------------------------------------------------------------- # _extract_espn_data diff --git a/test/test_common_readme_lists_every_module.py b/test/test_common_readme_lists_every_module.py new file mode 100644 index 00000000..35b2e410 --- /dev/null +++ b/test/test_common_readme_lists_every_module.py @@ -0,0 +1,21 @@ +"""src/common/README.md says it lists every module; keep it true. + +frame_timing, json_body and render_gate landed without rows, so the page a +plugin author reads to see what core offers was missing three modules. +""" + +from pathlib import Path + +COMMON = Path(__file__).resolve().parent.parent / "src" / "common" + + +def _modules(): + return sorted(p.stem for p in COMMON.glob("*.py") if p.stem != "__init__") + + +def test_every_module_has_a_table_row_and_a_section(): + readme = (COMMON / "README.md").read_text(encoding="utf-8") + missing_rows = [m for m in _modules() if f"| [`{m}`](#{m}) |" not in readme] + missing_sections = [m for m in _modules() if f"\n### {m}\n" not in readme] + assert not missing_rows, f"no summary-table row for: {missing_rows}" + assert not missing_sections, f"no '### ' section for: {missing_sections}" diff --git a/test/test_display_controller_startup_validation.py b/test/test_display_controller_startup_validation.py index d7941d7e..9df96e89 100644 --- a/test/test_display_controller_startup_validation.py +++ b/test/test_display_controller_startup_validation.py @@ -25,8 +25,9 @@ class RecordingValidator: self.warnings = ['config warning'] return True, [], list(self.warnings) - def _validate_plugins(self): + def _validate_plugins(self, discovered_plugins=None): RecordingValidator.calls.append(('plugins',)) + RecordingValidator.discovered = discovered_plugins self.warnings = ['plugin warning'] @@ -75,3 +76,19 @@ def test_each_check_runs_once(mock_config_manager, mock_display_manager, assert any('plugin warning' in r.getMessage() for r in caplog.records) finally: controller.cleanup() + + +def test_plugins_are_discovered_once(mock_config_manager, mock_display_manager, + mock_cache_manager, test_config_with_plugins, + emulator_mode): + """The plugin checks reuse the controller's discovery rather than rescanning + the plugins directory (and logging every plugin) a second time.""" + RecordingValidator.calls = [] + RecordingValidator.discovered = None + controller = _build_controller(mock_config_manager, mock_display_manager, + mock_cache_manager, test_config_with_plugins) + try: + assert controller.plugin_manager.discover_plugins.call_count == 1 + assert RecordingValidator.discovered == [] + finally: + controller.cleanup() diff --git a/test/test_font_manager.py b/test/test_font_manager.py index e9e80c7b..fb4ff923 100644 --- a/test/test_font_manager.py +++ b/test/test_font_manager.py @@ -137,6 +137,24 @@ class TestCacheLifecycle: assert fm.cache_generation == gen_before + 1 assert not fm.font_cache + def test_clear_cache_bumps_generation(self, fm): + # Layout contexts and font-usage results are keyed off + # cache_generation; clear_cache used to drop the fonts without + # telling them. + gen_before = fm.cache_generation + fm.clear_cache() + assert fm.cache_generation == gen_before + 1 + + def test_clearing_a_plugins_cached_fonts_bumps_generation(self, fm): + fm.font_cache["demo::tiny_8"] = object() + gen_before = fm.cache_generation + fm._clear_plugin_font_cache("demo") + assert "demo::tiny_8" not in fm.font_cache + assert fm.cache_generation == gen_before + 1 + # Nothing to drop, nothing to rebuild. + fm._clear_plugin_font_cache("demo") + assert fm.cache_generation == gen_before + 1 + class TestPluginFonts: """plugin:// sources resolve against the plugin's own directory, which diff --git a/test/test_logo_downloader.py b/test/test_logo_downloader.py index 581997bd..e659401c 100644 --- a/test/test_logo_downloader.py +++ b/test/test_logo_downloader.py @@ -356,6 +356,18 @@ class TestRefreshPlaceholderTimestamp: def test_missing_file_is_not_an_error(self, tmp_path): assert refresh_placeholder_timestamp(tmp_path / "nope.png") is False + def test_the_restamp_is_atomic(self, tmp_path): + # It used to save over the file in place, so a renderer opening the + # logo mid-write read a truncated PNG. A failed write must now leave + # the previous placeholder intact and no temp file behind. + assert LogoDownloader().create_placeholder_logo("COLL", str(tmp_path)) + path = tmp_path / "COLL.png" + before = path.read_bytes() + with patch("src.logo_downloader.os.replace", side_effect=OSError("disk full")): + assert refresh_placeholder_timestamp(path) is False + assert path.read_bytes() == before + assert sorted(p.name for p in tmp_path.iterdir()) == ["COLL.png"] + class TestFailurePaths: def test_a_team_without_logos_is_a_failed_download(self, tmp_path): diff --git a/test/test_logo_helper_download_backoff.py b/test/test_logo_helper_download_backoff.py new file mode 100644 index 00000000..f0528fd6 --- /dev/null +++ b/test/test_logo_helper_download_backoff.py @@ -0,0 +1,89 @@ +"""A failed logo download is not retried on every call. + +A logo that is simply absent (not a stale placeholder) had nothing to back +off on: every load_logo_with_download() call retried the download, each with +a 30s timeout, for as long as the host was down or the URL was dead. +""" + +import io +import logging +import time +from unittest.mock import MagicMock + +import pytest +import requests +from PIL import Image + +from src.common.logo_helper import MISSING_LOGO_RECHECK_SECONDS, LogoHelper + + +@pytest.fixture(autouse=True) +def _no_real_chmod(monkeypatch): + monkeypatch.setattr("src.common.logo_helper.ensure_directory_permissions", MagicMock()) + monkeypatch.setattr("src.logo_downloader.ensure_file_permissions", MagicMock()) + + +@pytest.fixture +def helper(): + h = LogoHelper(display_width=64, display_height=32, + logger=logging.getLogger("test.logo_backoff")) + h.session.get = MagicMock(side_effect=requests.ConnectionError("down")) + return h + + +def _png_response(): + buf = io.BytesIO() + Image.new("RGBA", (20, 20), (0, 128, 0, 255)).save(buf, "PNG") + data = buf.getvalue() + response = MagicMock() + response.__enter__.return_value = response + response.__exit__.return_value = False + response.raise_for_status = MagicMock() + response.headers = {"content-type": "image/png"} + response.iter_content = lambda *a, **k: iter([data]) + return response + + +def _load(helper, path): + return helper.load_logo_with_download("PHI", path, "http://x/logo.png", + max_width=20, max_height=20) + + +def test_a_second_call_does_not_retry(helper, tmp_path): + path = tmp_path / "PHI.png" + assert _load(helper, path) is not None # placeholder + assert _load(helper, path) is not None + assert helper.session.get.call_count == 1 + + +def test_it_retries_after_the_recheck_window(helper, tmp_path): + path = tmp_path / "PHI.png" + _load(helper, path) + helper._download_failures[str(path)] -= MISSING_LOGO_RECHECK_SECONDS + 1 + _load(helper, path) + assert helper.session.get.call_count == 2 + + +def test_other_paths_are_not_held_back(helper, tmp_path): + _load(helper, tmp_path / "PHI.png") + _load(helper, tmp_path / "DAL.png") + assert helper.session.get.call_count == 2 + + +def test_clear_cache_forgets_failures(helper, tmp_path): + path = tmp_path / "PHI.png" + _load(helper, path) + helper.clear_cache() + _load(helper, path) + assert helper.session.get.call_count == 2 + + +def test_invalidate_forgets_the_failure(helper, tmp_path): + path = tmp_path / "PHI.png" + _load(helper, path) + helper._invalidate_cached_logo("PHI", path) + helper.session.get = MagicMock(return_value=_png_response()) + logo = _load(helper, path) + helper.session.get.assert_called_once() + assert path.exists() + assert logo.getpixel((0, 0))[:3] == (0, 128, 0) diff --git a/test/test_scroll_helper_image_entry.py b/test/test_scroll_helper_image_entry.py new file mode 100644 index 00000000..9b1a38dd --- /dev/null +++ b/test/test_scroll_helper_image_entry.py @@ -0,0 +1,87 @@ +"""ScrollHelper: what a new scrolling image does on the way in. + +- set_scrolling_image cached a non-RGB image as-is, and every frame is cut + with Image.frombytes('RGB', ...): an RGBA strip came out as garbage and an + L strip raised. +- Only reset_scroll() reset last_update_time. In time-based mode the first + update after a new image (e.g. the plugin coming back on screen) advanced + the position by the whole idle gap, skipping the start of the content. +""" + +import time + +import pytest +from PIL import Image + +from src.common.scroll_helper import ScrollHelper + +W, H = 16, 8 + + +@pytest.fixture +def helper(): + return ScrollHelper(display_width=W, display_height=H) + + +class TestNonRgbImages: + def test_rgba_is_composited_onto_black(self, helper): + img = Image.new('RGBA', (40, H), (0, 0, 0, 0)) + for x in range(10): + for y in range(H): + img.putpixel((x, y), (255, 0, 0, 255)) + # Hidden colour under full transparency must not leak through. + img.putpixel((20, 0), (0, 255, 0, 0)) + helper.set_scrolling_image(img) + assert helper.cached_image.mode == 'RGB' + frame = helper.get_visible_portion() + assert frame.size == (W, H) + assert frame.getpixel((0, 0)) == (255, 0, 0) + assert frame.getpixel((12, 0)) == (0, 0, 0) + helper.scroll_position = 20.0 + assert helper.get_visible_portion().getpixel((0, 0)) == (0, 0, 0) + + def test_greyscale_does_not_raise(self, helper): + helper.set_scrolling_image(Image.new('L', (40, H), 200)) + frame = helper.get_visible_portion() + assert frame.getpixel((0, 0)) == (200, 200, 200) + + def test_palette_with_transparency(self, helper): + img = Image.new('P', (40, H), 1) + img.putpalette([0, 0, 0, 0, 0, 255] + [0] * 762) + img.info['transparency'] = 1 + helper.set_scrolling_image(img) + assert helper.get_visible_portion().getpixel((0, 0)) == (0, 0, 0) + + def test_an_rgb_image_is_kept_as_the_same_object(self, helper): + img = Image.new('RGB', (40, H), (1, 2, 3)) + helper.set_scrolling_image(img) + assert helper.cached_image is img + + +class TestNoJumpAfterIdle: + def _idle_then_new_image(self, helper, install): + helper.scroll_speed = 100.0 # px/s, time-based + install(helper) + helper.update_scroll_position() + # Off screen for 10 seconds, then a fresh image. + helper.last_update_time = time.time() - 10.0 + install(helper) + helper.update_scroll_position() + return helper.scroll_position + + def test_set_scrolling_image(self, helper): + pos = self._idle_then_new_image( + helper, lambda h: h.set_scrolling_image(Image.new('RGB', (5000, H)))) + assert pos < 50, f"jumped {pos}px on the first update of a new image" + + def test_create_scrolling_image(self, helper): + pos = self._idle_then_new_image( + helper, lambda h: h.create_scrolling_image([Image.new('RGB', (5000, H))])) + assert pos < 50, f"jumped {pos}px on the first update of a new image" + + def test_fixed_step_mode_still_steps_exactly(self, helper): + helper.set_pixels_per_frame(2) + helper.set_scrolling_image(Image.new('RGB', (5000, H))) + helper.update_scroll_position() + helper.update_scroll_position() + assert helper.scroll_position == 4.0 diff --git a/test/test_startup_validator.py b/test/test_startup_validator.py index 2bda887f..54096d21 100644 --- a/test/test_startup_validator.py +++ b/test/test_startup_validator.py @@ -242,6 +242,28 @@ class TestPlugins: assert is_valid is True assert errors == [] + def test_a_null_plugin_block_does_not_abandon_the_checks(self, good_cache, tmp_path): + # config.get(id, {}) returns None for "known": null, and .get() on it + # raised -- caught as "Could not validate plugins", skipping every + # plugin after it. + pm = MagicMock() + pm.discover_plugins.return_value = ['nulled', 'known'] + plugin_dir = tmp_path / "known" + plugin_dir.mkdir() # no manifest.json + pm.get_plugin_directory.return_value = str(plugin_dir) + + config = dict(GOOD_CONFIG, nulled=None, known={'enabled': True}) + validator = StartupValidator(make_config_manager(config), pm) + validator._validate_plugins() + assert not any('Could not validate plugins' in w for w in validator.warnings) + assert "Plugin 'known' manifest.json not found" in validator.errors + + def test_a_passed_discovery_is_reused(self, good_cache): + pm = MagicMock() + validator = StartupValidator(make_config_manager(dict(GOOD_CONFIG)), pm) + validator._validate_plugins(discovered_plugins=[]) + pm.discover_plugins.assert_not_called() + class TestIdempotence: """validate_all() resets error/warning state each run (the fixed bug).""" diff --git a/test/test_wifi_wrong_password_signal.py b/test/test_wifi_wrong_password_signal.py new file mode 100644 index 00000000..b3adb2ee --- /dev/null +++ b/test/test_wifi_wrong_password_signal.py @@ -0,0 +1,78 @@ +"""A rejected passphrase must reach the web UI as "wrong_password:". + +_connect_nmcli prefixes the message with "wrong_password:" and the api_v3 +wifi route turns that into ``error_type == 'wrong_password'``, which the +setup pages key their "incorrect password" prompt off. _connect_validated +used to replace the message on every failure branch (restore the old +network, or bring the setup AP up), so the prefix never survived. +""" + +import sys +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from src.wifi_manager import WiFiManager # noqa: E402 + + +def _manager(connected_ssid=None): + manager = WiFiManager.__new__(WiFiManager) # no __init__: no real host access + manager.has_nmcli = True + manager._wifi_interface = "wlan0" + manager.get_wifi_status = MagicMock(return_value=SimpleNamespace( + connected=bool(connected_ssid), ssid=connected_ssid, ip_address=None)) + manager._is_ap_mode_active = MagicMock(return_value=False) + manager._ensure_wifi_radio_enabled = MagicMock(return_value=True) + manager._show_led_message = MagicMock() + manager.disconnect_from_network = MagicMock(return_value=(True, "ok")) + manager._wait_for_device_idle = MagicMock(return_value=True) + manager.enable_ap_mode = MagicMock(return_value=(True, "AP up")) + return manager + + +def _run(manager, nmcli_result): + manager._connect_nmcli = MagicMock(return_value=nmcli_result) + nmcli_out = SimpleNamespace(returncode=0, stdout="GENERAL.CONNECTION:OldNet\n") + with patch("src.wifi_manager.subprocess.run", return_value=nmcli_out), \ + patch("src.wifi_manager.time.sleep"): + return manager._connect_validated("HomeNet", "hunter22") + + +WRONG = (False, "wrong_password: Secrets were required, but not provided") + + +class TestWrongPasswordSurvivesRecovery: + def test_when_the_setup_ap_comes_back_up(self): + ok, message = _run(_manager(), WRONG) + assert ok is False + assert message.startswith("wrong_password:") + + def test_when_the_setup_ap_fails_too(self): + manager = _manager() + manager.enable_ap_mode.return_value = (False, "hostapd missing") + ok, message = _run(manager, WRONG) + assert ok is False + assert message.startswith("wrong_password:") + assert "hostapd missing" in message + + @pytest.mark.parametrize("restored", [True, False]) + def test_when_the_previous_network_is_restored_or_not(self, restored): + manager = _manager(connected_ssid="OldNet") + manager._restore_original_connection = MagicMock(return_value=restored) + ok, message = _run(manager, WRONG) + assert ok is False + assert message.startswith("wrong_password:") + + +class TestOtherFailuresAreUnchanged: + def test_a_generic_failure_gets_no_prefix(self): + ok, message = _run(_manager(), (False, "Connection timed out")) + assert ok is False + assert message == "Connection failed. AP mode enabled." + + def test_success_passes_through(self): + assert _run(_manager(), (True, "Connected to HomeNet")) == (True, "Connected to HomeNet") diff --git a/test/web_interface/test_api_json_not_cached.py b/test/web_interface/test_api_json_not_cached.py new file mode 100644 index 00000000..0ff40858 --- /dev/null +++ b/test/web_interface/test_api_json_not_cached.py @@ -0,0 +1,51 @@ +"""/api/v3 JSON GETs must not be served from the browser cache. + +They carried ``Cache-Control: private, max-age=5``, so a fetch() made right +after an install, a toggle or a Wi-Fi connect could be answered with the copy +from before it. Non-JSON files served through the API keep the short cache. +""" + +from flask import Flask, Response, jsonify + +import pytest + + +@pytest.fixture +def client(): + import web_interface.app as web_app + # The real after_request hook on a throwaway app, so the header logic is + # tested without depending on any real endpoint's managers. + app = Flask(__name__) + app.after_request(web_app.add_security_headers) + app.add_url_rule('/api/v3/probe/json', 'probe_json', + lambda: jsonify({'status': 'success'})) + app.add_url_rule('/api/v3/probe/file', 'probe_file', + lambda: Response('body{}', mimetype='text/css')) + app.add_url_rule('/static/v3/probe.css', 'probe_static', + lambda: Response('body{}', mimetype='text/css')) + with app.test_client() as c: + yield c + + +def test_json_gets_are_not_stored(client): + resp = client.get('/api/v3/probe/json') + assert resp.headers['Cache-Control'] == 'no-store' + + +def test_non_json_files_keep_the_short_cache(client): + resp = client.get('/api/v3/probe/file') + assert resp.headers['Cache-Control'] == 'private, max-age=5, must-revalidate' + + +def test_static_assets_are_still_long_cached(client): + resp = client.get('/static/v3/probe.css') + assert 'immutable' in resp.headers.get('Cache-Control', '') + + +def test_the_real_app_marks_an_api_json_answer_no_store(): + import web_interface.app as web_app + web_app.app.config['TESTING'] = True + with web_app.app.test_client() as c: + resp = c.get('/api/v3/no-such-endpoint-for-cache-test') + assert resp.mimetype == 'application/json' + assert resp.headers['Cache-Control'] == 'no-store' diff --git a/test/web_interface/test_starlark_app_toggle_route.py b/test/web_interface/test_starlark_app_toggle_route.py new file mode 100644 index 00000000..bff7df2b --- /dev/null +++ b/test/web_interface/test_starlark_app_toggle_route.py @@ -0,0 +1,116 @@ +"""POST /api/v3/starlark/apps//toggle shares _toggle_starlark_app. + +The route carried its own copy of the toggle with the bugs the shared helper +had already fixed: it changed the loaded app before the disk write (so a +failed save left the two disagreeing), indexed ``manifest['apps'][app_id]`` +(a KeyError, answered as a 500, for a loaded app with no on-disk entry), and +stored ``"false"`` -- a truthy string -- as the new state. +""" + +import contextlib +from unittest.mock import MagicMock, patch + +import pytest + +PKG = 'web_interface.blueprints.api_v3' + + +@pytest.fixture +def client(): + from web_interface.app import app + app.config['TESTING'] = True + with app.test_client() as c: + yield c + + +def _loaded_plugin(enabled=True, save_ok=True, disk=None): + app = MagicMock() + app.manifest = {'enabled': enabled} + app.is_enabled.return_value = enabled + plugin = MagicMock() + plugin.apps = {'clock': app} + disk = {'apps': {}} if disk is None else disk + + def _update(fn): + if not save_ok: + return False + fn(disk) + return True + plugin._update_manifest_safe.side_effect = _update + return plugin, app, disk + + +def _post(client, body): + return client.post('/api/v3/starlark/apps/clock/toggle', json=body) + + +class TestLoadedApp: + def test_a_missing_disk_entry_is_created_not_a_500(self, client): + plugin, app, disk = _loaded_plugin(enabled=True) + with patch(f'{PKG}._get_starlark_plugin', return_value=plugin): + resp = _post(client, {'enabled': False}) + body = resp.get_json() + assert resp.status_code == 200, body + assert body == {'status': 'success', 'message': body['message'], 'enabled': False} + assert disk['apps']['clock']['enabled'] is False + assert app.manifest['enabled'] is False + + def test_a_failed_save_leaves_the_loaded_app_alone(self, client): + plugin, app, _ = _loaded_plugin(enabled=True, save_ok=False) + with patch(f'{PKG}._get_starlark_plugin', return_value=plugin): + resp = _post(client, {'enabled': False}) + assert resp.status_code == 500 + assert resp.get_json()['status'] == 'error' + assert app.manifest['enabled'] is True + + @pytest.mark.parametrize('sent', ['false', 'False', 0, '0']) + def test_a_false_string_or_zero_disables(self, client, sent): + plugin, app, disk = _loaded_plugin(enabled=True) + with patch(f'{PKG}._get_starlark_plugin', return_value=plugin): + resp = _post(client, {'enabled': sent}) + assert resp.get_json()['enabled'] is False + assert disk['apps']['clock']['enabled'] is False + + def test_an_unrecognised_value_is_refused(self, client): + plugin, app, disk = _loaded_plugin(enabled=True) + with patch(f'{PKG}._get_starlark_plugin', return_value=plugin): + resp = _post(client, {'enabled': 'maybe'}) + assert resp.status_code == 400 + assert disk == {'apps': {}} + + def test_no_enabled_flips_the_current_state(self, client): + plugin, app, disk = _loaded_plugin(enabled=True) + with patch(f'{PKG}._get_starlark_plugin', return_value=plugin): + resp = _post(client, {}) + assert resp.get_json()['enabled'] is False + + +class TestStandalone: + def _run(self, client, body, manifest): + written = {} + # The real lock is fcntl-based (Linux only); the toggle logic is what + # is under test here. + with patch(f'{PKG}._get_starlark_plugin', return_value=None), \ + patch(f'{PKG}._starlark_manifest_lock', contextlib.nullcontext), \ + patch(f'{PKG}._read_starlark_manifest', return_value=manifest), \ + patch(f'{PKG}._write_starlark_manifest', + side_effect=lambda m: written.update(m) or True): + resp = _post(client, body) + return resp, written + + def test_toggle_is_written(self, client): + resp, written = self._run(client, {'enabled': 'false'}, + {'apps': {'clock': {'enabled': True}}}) + assert resp.get_json()['enabled'] is False + assert written['apps']['clock']['enabled'] is False + + def test_no_enabled_flips_the_manifest_state(self, client): + resp, _ = self._run(client, {}, {'apps': {'clock': {'enabled': False}}}) + assert resp.get_json()['enabled'] is True + + @pytest.mark.parametrize('body', [{}, {'enabled': True}]) + def test_an_unknown_app_is_404(self, client, body): + resp, written = self._run(client, body, {'apps': {}}) + assert resp.status_code == 404 + assert 'clock' in resp.get_json()['message'] + assert written == {} diff --git a/web_interface/app.py b/web_interface/app.py index 535b055d..46266e31 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -570,10 +570,16 @@ def add_security_headers(response): response.headers['Cache-Control'] = 'public, max-age=31536000, immutable' response.headers['Expires'] = (datetime.now() + timedelta(days=365)).strftime('%a, %d %b %Y %H:%M:%S GMT') elif request.path.startswith('/api/v3/'): - # Short cache for API responses (5 seconds) to allow for quick updates - # but reduce server load for repeated requests if request.method == 'GET' and 'stream' not in request.path: - response.headers['Cache-Control'] = 'private, max-age=5, must-revalidate' + if response.mimetype == 'application/json': + # JSON is live state. A cached copy made a fetch() right after + # an install, toggle or Wi-Fi connect show the state from + # before it for up to the cache lifetime. + response.headers['Cache-Control'] = 'no-store' + else: + # Files served through the API (plugin web UI assets, backup + # downloads) keep the short cache. + response.headers['Cache-Control'] = 'private, max-age=5, must-revalidate' else: # No cache for HTML pages to ensure fresh content response.headers['Cache-Control'] = 'no-cache, no-store, must-revalidate' diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index e8682ed5..16d753f8 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -153,6 +153,9 @@ def get_installed_plugins(): 'category': plugin_info.get('category', 'General'), 'description': plugin_info.get('description', 'No description available'), 'tags': plugin_info.get('tags', []), + # The tab nav uses this as the element's Font Awesome class + # (app-shell.js / app-early.js); only a string can be one. + 'icon': plugin_info.get('icon') if isinstance(plugin_info.get('icon'), str) else None, 'enabled': enabled, 'verified': verified, 'loaded': plugin_info.get('loaded', False), diff --git a/web_interface/blueprints/api_v3/starlark.py b/web_interface/blueprints/api_v3/starlark.py index cdc21fa4..b5c39f74 100644 --- a/web_interface/blueprints/api_v3/starlark.py +++ b/web_interface/blueprints/api_v3/starlark.py @@ -21,6 +21,7 @@ from web_interface.blueprints.api_v3 import ( subprocess, tempfile, ) import web_interface.blueprints.api_v3 as _pkg +from web_interface.blueprints.api_v3.wifi import _parse_bool_ish # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. # Several are also called from helpers that live in __init__, so the @@ -547,36 +548,33 @@ def toggle_starlark_app(app_id): try: data = request.get_json(silent=True) or {} - starlark_plugin = _pkg._get_starlark_plugin() - if starlark_plugin: - app = starlark_plugin.apps.get(app_id) - if not app: - return jsonify({'status': 'error', 'message': f'App not found: {app_id}'}), 404 - enabled = data.get('enabled') - if enabled is None: + # The write itself goes through _toggle_starlark_app, the same helper + # /plugins/toggle uses: it saves to disk before touching the loaded + # app, and tolerates a loaded app with no on-disk entry. This route + # used to carry its own copy without either fix, and stored "false" + # (a truthy string) as-is. + enabled = data.get('enabled') + if enabled is None: + _, err = _validate_starlark_app_path(app_id) + if err: + return jsonify({'status': 'error', 'message': err}), 400 + # No explicit state: flip the current one. + starlark_plugin = _pkg._get_starlark_plugin() + app = starlark_plugin.apps.get(app_id) if starlark_plugin else None + if app is not None: enabled = not app.is_enabled() - app.manifest['enabled'] = enabled - # Use safe manifest update to prevent race conditions - def update_fn(manifest): - manifest['apps'][app_id]['enabled'] = enabled - starlark_plugin._update_manifest_safe(update_fn) - return jsonify({'status': 'success', 'message': f"App {'enabled' if enabled else 'disabled'}", 'enabled': enabled}) - - # Standalone: update manifest directly - with _starlark_manifest_lock(): - manifest = _pkg._read_starlark_manifest() - app_data = manifest.get('apps', {}).get(app_id) - if not app_data: - return jsonify({'status': 'error', 'message': f'App not found: {app_id}'}), 404 - - enabled = data.get('enabled') - if enabled is None: - enabled = not app_data.get('enabled', True) - app_data['enabled'] = enabled - if _pkg._write_starlark_manifest(manifest): - return jsonify({'status': 'success', 'message': f"App {'enabled' if enabled else 'disabled'}", 'enabled': enabled}) else: - return jsonify({'status': 'error', 'message': 'Failed to save'}), 500 + app_data = _pkg._read_starlark_manifest().get('apps', {}).get(app_id) + if not app_data: + return jsonify({'status': 'error', 'message': f'App not found: {app_id}'}), 404 + enabled = not app_data.get('enabled', True) + else: + enabled = _parse_bool_ish(enabled) + if enabled is None: + return jsonify({'status': 'error', + 'message': 'enabled must be true or false'}), 400 + + return _pkg._toggle_starlark_app(app_id, enabled) except Exception as e: logger.exception("[Starlark] toggle_starlark_app failed")