From 3a81f38f0921b94fe35d2f9dc85ad1740e5a606d Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:35:49 -0400 Subject: [PATCH] fix(web): uniqueItems saves, /health count, Vegas order wipe; one list-repair helper (#638) * fix(web): drop repeats from uniqueItems lists before validating a plugin save dedup_unique_arrays lost its only caller in #330, so submitting a value a uniqueItems list already holds (a stock symbol saved once and posted again) failed the whole save with a validation error. _prepare_plugin_config_for_save runs it again just before validation, which covers both POST /plugins/config and plugin sections posted to /config/main. Co-Authored-By: Claude Opus 5.5 * fix(web): /health counts the discovered plugins and logs the checks it fails The plugin check counted plugin_manager.get_available_plugins(), which PluginManager does not have, behind a hasattr guard that made plugin_count 0 on every device. It now counts the discovered manifests, discovering first when nothing has been scanned yet. The config, plugin and hardware checks answered "see logs for details" without logging anything. Each now logs a warning with the traceback. Co-Authored-By: Claude Opus 5.5 * fix(web): store refresh no longer claims a commit-metadata refresh POST /plugins/store/refresh read fetch_commit_info (or fetch_latest_versions) only to append "(with refreshed commit metadata from GitHub)" to its message. It never fetched any: the route re-downloads the registry and nothing else. search_plugins takes the flag, but it reads commit info through its cache, so passing it on would not refresh anything either. The flag is ignored now and the message says what happened. Co-Authored-By: Claude Opus 5.5 * fix(web): refuse a malformed Vegas plugin order instead of clearing it A vegas_plugin_order or vegas_excluded_plugins value that was not JSON, or not a list, was stored as [] and the save answered 200, so a bad value wiped the saved order or exclusions. Both now answer 400 and save nothing, the way plugin_rotation_order already did; the three share one parser. A list that holds anything but plugin-id strings is refused as well. Co-Authored-By: Claude Opus 5.5 * fix(web): per-plugin health and metrics read the display service's latest GET /plugins/health/ and /plugins/metrics/ called get_health_summary and get_metrics_summary without force_reload, so they answered with whatever the web process read first and kept in memory, while the display service kept writing newer state. They now pass force_reload=True, as the list routes do. Co-Authored-By: Claude Opus 5.5 * fix(web): plugin config reset saves through the shared atomic save POST /plugins/config/reset called config_manager.save_config directly, so it took no backup, and a failed write escaped as an unhandled exception. It then handed on_config_change the raw stored section, not the prepared config a loaded plugin runs with. It now saves through _save_config_atomic with a backup, answers CONFIG_SAVE_FAILED when that fails, and notifies with _prepared_plugin_config, as POST /plugins/config does. POST /plugins/toggle carried its own copy of _save_config_atomic's save_config_atomic-or-save_config fallback; it calls the shared helper now. Co-Authored-By: Claude Opus 5.5 * fix(web): one reading and one "unavailable" for each system metric system_metrics.collect_system_metrics() promised None for a metric it could not read, but returned cpu_temp as 0 off a Pi, and the whole no-psutil fallback as zeros. GET /system/status measured the same numbers a second time with its own code, and answered None there. Now both come from collect_system_metrics(), and "unavailable" is None everywhere. /system/status keeps its 0.1s CPU sample and its 10s cache, and gains nothing it did not already send. Two differences: without psutil it answers 200 with null metrics instead of 503, and a disk it cannot stat is null instead of a 500. Co-Authored-By: Claude Opus 5.5 * fix(web): /display/current sends the snapshot as-is and logs a failed read GET /display/current PIL-decoded the preview snapshot and re-encoded it before base64-ing it, spending CPU on the Pi to send the same picture, and dropped any failure with `except Exception: pass`. The /stream/display SSE stream already passed the PNG's bytes straight through. Both now read through web_interface/display_preview.py and answer with the same payload. A missing snapshot is still a null image; any other read failure is logged as a warning. /health reads the snapshot path from the same module. Co-Authored-By: Claude Opus 5.5 * refactor(web): one helper puts a submitted plugin config's lists back The plugin-config save turned position-keyed dicts ({"0": ..., "1": ...}) back into lists in five copies: four in the form path's fix_array_structures (whose prefix branches never ran, since no caller passed one), and _fix_json_arrays on the JSON path. It then force-fixed the news plugin's feeds.custom_feeds by name, in case the generic pass had missed it. src/web_interface/config_arrays.coerce_array_shapes now does it for both paths, custom_feeds included. ensure_array_defaults duplicated _fix_none_arrays and is gone. In the same function: the union-type re-checks that the null handling above them made unreachable, the "(temporary)" random_seed debug log, and a commented-out log line are removed. A failed validation is logged once as a warning, not four ERROR lines and a WARNING. Element types are left to normalize_config_values, which already converted them for both paths. One difference: the form path no longer adds an empty {} for a nested object the post left out that has no defaults. Co-Authored-By: Claude Opus 5.5 * refactor(web): import at module top and log through the module logger The web_interface.cache imports in config.py and fonts.py were wrapped in `except ImportError` fallbacks. It is an in-repo module that imports nothing from the project, so it cannot fail to import; it is imported once at module top, as system.py now does. cache.py's docstring said blueprints import it lazily "to avoid circular imports"; it now says why that is unnecessary. Five logging.error calls in the dim-schedule GET and three logging.warning calls in plugins.py went to the root logger; they use the module logger. Function-local re-imports of json, os, shutil, logging and Path, all already imported by the module, are gone. The `import os` inside two except blocks of save_plugin_config also made os a local name for the whole function. execute_plugin_action's step-1 handler gets a comment saying why it stays: it looks like a copy of the blueprint handler, but without it a TimeoutExpired from the plugin's script would reach the route's own `except subprocess.TimeoutExpired` and be answered as a 408. Co-Authored-By: Claude Opus 5.5 * refactor(web): app.py loses dead CSRF and reconciliation state, comments fixed - csrf was always None, so `if csrf: csrf.exempt(...)` never ran, and its note that the api_v3 blueprint "is exempted above" named an exemption that does not exist. Both are gone; the reason there is no CSRF protection stays, shortened. - The SSE rate-limit comment called the default "tight" at 20 per minute. The default is 1000 per minute and the streams' 200 is the tighter one; the comment now says so. The limits are unchanged. - _reconciliation_done was written and never read. The docstring that explains why reconciliation runs once keeps its reason, in the present tense. - Removed: a dangling "import cache functions" comment with no import under it, a "security check ... within project_root" label on an existence check, the "(simplified version)" narration, and the note that no redirect route is needed. The preview loop's sleep comment no longer mentions a PIL encode that the loop does not do. Co-Authored-By: Claude Opus 5.5 * docs(web): api_v3 comments name the package __init__, not a _common module Every route module's docstring said the shared blueprint comes "from ._common", a module the package split never created; they name the package __init__. The PROJECT_ROOT comment described the path from _common.py; it now describes this package and keeps the incident it guards against. The "(corrected) in this commit" note in resolve_pull_command and the /health comment the split's mechanical time -> _pkg.time rewrite garbled ("Stamp the start _pkg.time") read correctly again. Co-Authored-By: Claude Opus 5.5 * refactor(web): drop hasattr checks for attributes PluginManager always has PluginManager.__init__ sets health_tracker and resource_monitor (to None until they are configured), so the seven hasattr(api_v3.plugin_manager, ...) guards in the health, metrics and limits routes were always true. The falsy checks that do the work stay. Co-Authored-By: Claude Opus 5.5 * refactor(web): pages_v3 dispatches partials from a dict with one error handler load_partial chose a loader through a fourteen-branch if/elif, and thirteen of the loaders then wrapped themselves in the same try/except, logging "Error loading partial" without saying which. The route now looks the name up in _PARTIAL_LOADERS and has the one handler, which logs the partial's name. The loaders just render. _load_tools_partial keeps its own messages. The search index's _partial_html already catches a loader that raises. serve_plugin_web_ui repeated _plugin_dir_for inline (containment plus the ledmatrix- prefix fallback); it calls it now. Also removed: the unused markupsafe.escape import, function-local json/Path re-imports, and unused exception bindings. Co-Authored-By: Claude Opus 5.5 * refactor(web): remove unused imports, locals and a try that cannot fail - get_error_aggregator was imported by the api_v3 package and used by no one; seven names config.py imported, and Path in misc.py and logging in plugins.py, likewise. - branch_info in install_plugin was built and never logged; test_config in /health was bound and never read (the load_config call is the check). - An f-string with no placeholders in the asset upload route. - _installed_plugin_ids wrapped list(manifests.keys()) in try/except; _discovered_plugin_manifests always returns a dict. Co-Authored-By: Claude Opus 5.5 * refactor(web): start.py logs its startup lines and drops unreachable branches The startup banner went to stdout with print(); it goes through a logger now, which the app import has already configured, so it reaches the journal with a level and timestamp like every other line. The "no addresses" branch is gone: get_local_ips() always returns at least "localhost". The except around app.run re-raised "only if it's not a client disconnection error" from inside the branch that had just established it was one, so that raise could not run. It is one check now, on a named tuple of the errnos, which the werkzeug log filter uses too. The comment on threaded=True counts three SSE endpoints, which is how many there are. Trailing whitespace is stripped. Co-Authored-By: Claude Opus 5.5 * refactor(web): save_main_config names its General fields once The General tab's field names were listed twice, once to detect a General form post and again, with four more, to keep the remaining-keys merge from storing them as top-level keys. GENERAL_FIELDS and _MAPPED_TOP_LEVEL_FIELDS hold them now, and the four per-section skip checks are one set. The comment on that merge said plugin configs are handled "here too", and "(including plugin keys)". Plugin sections are handled and removed from the body before it runs; the comment says so. Co-Authored-By: Claude Opus 5.5 * refactor(web): plugin directories come from the plugin manager only Six lookups fell back to PROJECT_ROOT/plugins/ when there was no plugin manager: GET /plugins/config's of-the-day data, POST /plugins/action, the plugin static-file route, the calendar credentials upload and the calendar OAuth routes. The loader never scans plugins/ (PluginManager.discover_plugins reads only the configured directory, plugin-repos by default), so what they found there was a plugin that never runs. _plugin_directory() asks the manager and answers None without one, which each route already reports as "not found". Co-Authored-By: Claude Opus 5.5 * docs(changelog): web-backend Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 10 + src/web_interface/config_arrays.py | 60 ++ test/test_api_v3_health.py | 90 +++ test/test_api_v3_plugin_health_single.py | 68 +++ test/test_api_v3_registry_endpoints.py | 12 +- test/test_api_v3_vegas_plugin_lists.py | 55 ++ test/test_display_preview.py | 79 +++ test/test_live_status_fields.py | 26 +- test/test_pages_v3_partials.py | 65 +++ test/test_system_status_available_memory.py | 23 +- .../test_api_v3_plugin_config_save.py | 210 +++++++ test/web_interface/test_config_arrays.py | 58 ++ .../web_interface/test_dedup_unique_arrays.py | 12 +- web_interface/app.py | 91 +-- web_interface/blueprints/api_v3/__init__.py | 56 +- web_interface/blueprints/api_v3/backup.py | 4 +- web_interface/blueprints/api_v3/config.py | 154 ++--- web_interface/blueprints/api_v3/display.py | 45 +- web_interface/blueprints/api_v3/fonts.py | 43 +- web_interface/blueprints/api_v3/misc.py | 33 +- web_interface/blueprints/api_v3/plugins.py | 536 +++--------------- web_interface/blueprints/api_v3/starlark.py | 6 +- web_interface/blueprints/api_v3/system.py | 104 +--- web_interface/blueprints/api_v3/wifi.py | 4 +- web_interface/blueprints/pages_v3.py | 451 ++++++--------- web_interface/cache.py | 4 +- web_interface/display_preview.py | 36 ++ web_interface/start.py | 69 +-- web_interface/system_metrics.py | 66 ++- 29 files changed, 1318 insertions(+), 1152 deletions(-) create mode 100644 src/web_interface/config_arrays.py create mode 100644 test/test_api_v3_health.py create mode 100644 test/test_api_v3_plugin_health_single.py create mode 100644 test/test_api_v3_vegas_plugin_lists.py create mode 100644 test/test_display_preview.py create mode 100644 test/test_pages_v3_partials.py create mode 100644 test/web_interface/test_api_v3_plugin_config_save.py create mode 100644 test/web_interface/test_config_arrays.py create mode 100644 web_interface/display_preview.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dbace593..2acd739a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,16 @@ accepts both, but the store flags the old spelling as deprecated - Backups record `src.__version__`. - Removed: `BackgroundDataService`'s `queue_size` stat and `clear_completed_requests()`. +- Web API fixes: + - A plugin save drops repeated entries in lists whose schema says `uniqueItems`, instead of failing validation. + - `/api/v3/health` reports the real plugin count. + - A malformed `vegas_plugin_order` or `vegas_excluded_plugins` is refused with a 400 and nothing is saved. It used to wipe the saved list. + - The per-plugin health and metrics routes return the display service's latest state. + - Resetting a plugin's config takes a backup first and reports a failed save. + - System metrics that can't be read are `null` everywhere: `cpu_temp` off a Pi, and every metric without psutil, where `/system/status` now answers 200 instead of 503. + - `/plugins/store/refresh` no longer claims a commit-metadata refresh it doesn't do. + - The plugin-config list repair code is in one place, `src/web_interface/config_arrays.py`. + - The web service (`ledmatrix-web`) logs through `src.logging_config` like the display service, so `journalctl -p err -u ledmatrix-web` works. Successful GET/HEAD/OPTIONS requests (the UI's polling) are logged at DEBUG instead of diff --git a/src/web_interface/config_arrays.py b/src/web_interface/config_arrays.py new file mode 100644 index 00000000..9dd5c32e --- /dev/null +++ b/src/web_interface/config_arrays.py @@ -0,0 +1,60 @@ +"""Putting a submitted plugin config's lists back into list shape. + +A list reaches a plugin-config save keyed by position more often than as a +list. The settings form posts one field per element (``feeds.custom_feeds.0.name``), +which ``_set_nested_value`` stores as ``{"0": {"name": ...}}``, and the JSON +path's dotToNested() in the browser builds the same dict. Validation expects +an array there, so the save converts them first. +""" +from typing import Any, Dict + + +def _is_index_dict(value: Any) -> bool: + """True for a dict keyed only by list positions ("0", "1", ...), or empty.""" + return isinstance(value, dict) and all(str(k).isdigit() for k in value) + + +def coerce_array_shapes(config: Dict[str, Any], schema_props: Dict[str, Any], + short_lists_take_default: bool = False) -> None: + """Turn position-keyed dicts into lists wherever the schema has an array. + + Walks ``config`` alongside the schema's ``properties``, in place: into + nested objects, and into the objects of an array's items. An empty dict + where an array belongs becomes ``[]``. + + ``short_lists_take_default`` is for form posts. A form draws a fixed-length + list (an RGB colour, say) as one input per element, and a blanked input + drops out of the parsed list; the schema default then stands in, as long as + it is itself long enough, instead of the save failing on ``minItems``. + + Element types are left alone: normalization after this converts numeric + strings to the numbers the schema asks for. + """ + if not isinstance(config, dict): + return + for key, prop_schema in schema_props.items(): + if key not in config or not isinstance(prop_schema, dict): + continue + prop_type = prop_schema.get('type') + value = config[key] + + if prop_type == 'array': + if _is_index_dict(value): + value = config[key] = [value[k] for k in sorted(value, key=lambda k: int(str(k)))] + if not isinstance(value, list): + continue + min_items = prop_schema.get('minItems') + default = prop_schema.get('default') + if (short_lists_take_default and min_items is not None + and len(value) < min_items + and isinstance(default, list) and len(default) >= min_items): + value = config[key] = list(default) + items_schema = prop_schema.get('items') + if (isinstance(items_schema, dict) and items_schema.get('type') == 'object' + and 'properties' in items_schema): + for element in value: + coerce_array_shapes(element, items_schema['properties'], + short_lists_take_default) + + elif prop_type == 'object' and 'properties' in prop_schema: + coerce_array_shapes(value, prop_schema['properties'], short_lists_take_default) diff --git a/test/test_api_v3_health.py b/test/test_api_v3_health.py new file mode 100644 index 00000000..d4921c67 --- /dev/null +++ b/test/test_api_v3_health.py @@ -0,0 +1,90 @@ +"""GET /api/v3/health: the plugin count is real, and a failed check is logged. + +The plugin check counted ``plugin_manager.get_available_plugins()``, which +PluginManager does not have; a hasattr guard turned that into a permanent 0. +Each check that fails answers "see logs for details", so it has to log. +""" + +import logging +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + +URL = "/api/v3/health" + + +@pytest.fixture(autouse=True) +def _no_systemctl(monkeypatch): + monkeypatch.setattr("web_interface.blueprints.api_v3.misc._get_display_service_status", + lambda: {"active": True}) + + +def _checks(client): + response = client.get(URL) + assert response.status_code == 200, response.get_json() + return response.get_json()["data"]["checks"] + + +def test_plugin_count_is_the_number_of_discovered_plugins(api_v3_client, api_v3_module): + api_v3_module.api_v3.plugin_manager.plugin_manifests = { + "clock": {"id": "clock"}, "weather": {"id": "weather"}, "stocks": {"id": "stocks"}, + } + + check = _checks(api_v3_client)["plugin_system"] + + assert check == {"status": "operational", "plugin_count": 3} + + +def test_plugin_count_discovers_when_nothing_is_discovered_yet(api_v3_client, api_v3_module): + pm = api_v3_module.api_v3.plugin_manager + pm.plugin_manifests = {} + + def discover(): + pm.plugin_manifests = {"clock": {"id": "clock"}} + pm.discover_plugins.side_effect = discover + + assert _checks(api_v3_client)["plugin_system"]["plugin_count"] == 1 + + +def test_a_failed_config_check_is_logged(api_v3_client, api_v3_module, caplog): + api_v3_module.api_v3.config_manager.load_config.side_effect = OSError("disk gone") + + with caplog.at_level(logging.WARNING): + check = _checks(api_v3_client)["config_file"] + + assert check["error"] == "see logs for details" + logged = [r for r in caplog.records if "config file" in r.getMessage()] + assert logged and logged[0].exc_info and "disk gone" in str(logged[0].exc_info[1]) + + +def test_a_failed_plugin_check_is_logged(api_v3_client, api_v3_module, caplog, monkeypatch): + def boom(): + raise RuntimeError("manifests unreadable") + monkeypatch.setattr("web_interface.blueprints.api_v3.misc._discovered_plugin_manifests", boom) + + with caplog.at_level(logging.WARNING): + check = _checks(api_v3_client)["plugin_system"] + + assert check["status"] == "error" + logged = [r for r in caplog.records if "count plugins" in r.getMessage()] + assert logged and logged[0].exc_info + + +def test_a_failed_hardware_check_is_logged(api_v3_client, api_v3_module, caplog, monkeypatch): + def getmtime(_path): + raise PermissionError("denied") + fake_os = SimpleNamespace(path=SimpleNamespace(exists=lambda _p: True, getmtime=getmtime)) + monkeypatch.setattr("web_interface.blueprints.api_v3.misc.os", fake_os) + + with caplog.at_level(logging.WARNING): + check = _checks(api_v3_client)["hardware"] + + assert check["status"] == "unknown" + logged = [r for r in caplog.records if "snapshot" in r.getMessage()] + assert logged and logged[0].exc_info diff --git a/test/test_api_v3_plugin_health_single.py b/test/test_api_v3_plugin_health_single.py new file mode 100644 index 00000000..455914ea --- /dev/null +++ b/test/test_api_v3_plugin_health_single.py @@ -0,0 +1,68 @@ +"""GET /plugins/health/ and /plugins/metrics/ read the display +service's latest state, not the web process's first snapshot. + +The display service writes health and metrics to the shared cache; the web +process only reads them. Its tracker and monitor keep what they read first in +memory, so without ``force_reload`` the per-plugin routes kept answering with +that first read while the list routes (which pass it) moved on. +""" + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from src.plugin_system.plugin_health import PluginHealthTracker # noqa: E402 +from src.plugin_system.resource_monitor import PluginResourceMonitor # noqa: E402 +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + + +class SharedCache: + """The on-disk cache both processes see, reduced to a dict.""" + + def __init__(self): + self.entries = {} + + def get(self, key, max_age=None, memory_ttl=None): + return self.entries.get(key) + + def set(self, key, value, *args, **kwargs): + self.entries[key] = value + + +@pytest.fixture +def shared_cache(api_v3_module): + cache = SharedCache() + pm = api_v3_module.api_v3.plugin_manager + pm.health_tracker = PluginHealthTracker(cache) + pm.resource_monitor = PluginResourceMonitor(cache) + return cache + + +def test_health_reflects_failures_recorded_after_the_first_read(api_v3_client, shared_cache): + first = api_v3_client.get("/api/v3/plugins/health/weather").get_json()["data"] + assert first["total_failures"] == 0 + + display_side = PluginHealthTracker(shared_cache) + display_side.record_failure("weather", RuntimeError("api down")) + display_side.record_failure("weather", RuntimeError("api down")) + + later = api_v3_client.get("/api/v3/plugins/health/weather").get_json()["data"] + assert later["total_failures"] == 2 + + +def test_metrics_reflect_calls_recorded_after_the_first_read(api_v3_client, shared_cache): + first = api_v3_client.get("/api/v3/plugins/metrics/weather").get_json()["data"] + assert first["call_count"] == 0 + + shared_cache.set("plugin_metrics:weather", { + "memory_mb": 12.5, "cpu_percent": 3.0, "execution_time": 0.2, + "call_count": 40, "total_execution_time": 8.0, + "max_execution_time": 0.5, "min_execution_time": 0.1, + "last_update_time": 1000.0, + }) + + later = api_v3_client.get("/api/v3/plugins/metrics/weather").get_json()["data"] + assert later["call_count"] == 40 diff --git a/test/test_api_v3_registry_endpoints.py b/test/test_api_v3_registry_endpoints.py index 85f4b377..b4e6dd1f 100644 --- a/test/test_api_v3_registry_endpoints.py +++ b/test/test_api_v3_registry_endpoints.py @@ -66,12 +66,16 @@ class TestRefreshPluginStore: assert response.status_code == 200 @pytest.mark.parametrize("key", ["fetch_commit_info", "fetch_latest_versions"]) - def test_either_commit_info_key_extends_the_message( + def test_commit_info_flag_claims_no_refresh_it_does_not_do( self, api_v3_client, api_v3_module, key): - # fetch_latest_versions is the older spelling; both must work. - api_v3_module.api_v3.plugin_store_manager.fetch_registry.return_value = {"plugins": []} + # The route only re-downloads the registry. It used to append "(with + # refreshed commit metadata from GitHub)" for either flag without + # fetching any. + store = api_v3_module.api_v3.plugin_store_manager + store.fetch_registry.return_value = {"plugins": [{"id": "a"}]} response = api_v3_client.post(self.URL, json={key: True}) - assert "commit metadata" in response.get_json()["message"] + assert response.get_json()["message"] == "Plugin store refreshed" + store.fetch_registry.assert_called_once_with(force_refresh=True) def test_message_stays_plain_without_the_flag(self, api_v3_client, api_v3_module): api_v3_module.api_v3.plugin_store_manager.fetch_registry.return_value = {"plugins": []} diff --git a/test/test_api_v3_vegas_plugin_lists.py b/test/test_api_v3_vegas_plugin_lists.py new file mode 100644 index 00000000..e5dd709c --- /dev/null +++ b/test/test_api_v3_vegas_plugin_lists.py @@ -0,0 +1,55 @@ +"""POST /config/main refuses a malformed Vegas plugin order or exclusion list. + +Both were parsed with ``except JSONDecodeError: ... = []``, so a bad value +cleared the saved list and answered 200. They now fail the save with a 400, +as plugin_rotation_order already did. +""" + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + +URL = "/api/v3/config/main" + + +@pytest.fixture +def saved(api_v3_module): + config = {"display": {"vegas_scroll": {"plugin_order": ["clock", "weather"], + "excluded_plugins": ["stocks"]}}} + cm = api_v3_module.api_v3.config_manager + cm.load_config.return_value = config + cm.save_config_atomic.return_value.status.value = 'success' + return cm + + +@pytest.mark.parametrize("field", ["vegas_plugin_order", "vegas_excluded_plugins"]) +@pytest.mark.parametrize("value", ["[not json", '{"a": 1}', "[1, 2]", 7]) +def test_malformed_list_is_refused_and_nothing_is_saved(api_v3_client, saved, field, value): + response = api_v3_client.post(URL, json={field: value}) + + assert response.status_code == 400, response.get_json() + assert field in response.get_json()["message"] + saved.save_config_atomic.assert_not_called() + saved.save_config.assert_not_called() + + +@pytest.mark.parametrize("value", ['["weather", "clock"]', ["weather", "clock"]]) +def test_json_text_or_array_is_stored(api_v3_client, saved, value): + response = api_v3_client.post(URL, json={"vegas_plugin_order": value}) + + assert response.status_code == 200, response.get_json() + stored = saved.save_config_atomic.call_args.args[0] + assert stored["display"]["vegas_scroll"]["plugin_order"] == ["weather", "clock"] + assert stored["display"]["vegas_scroll"]["excluded_plugins"] == ["stocks"] + + +def test_rotation_order_keeps_its_messages(api_v3_client, saved): + response = api_v3_client.post(URL, json={"plugin_rotation_order": "[oops"}) + + assert response.status_code == 400 + assert response.get_json()["message"] == "plugin_rotation_order must be valid JSON" diff --git a/test/test_display_preview.py b/test/test_display_preview.py new file mode 100644 index 00000000..aaaebfe0 --- /dev/null +++ b/test/test_display_preview.py @@ -0,0 +1,79 @@ +"""GET /api/v3/display/current passes the snapshot PNG through untouched. + +It used to PIL-decode the snapshot and re-encode it, which cost CPU on the Pi +for no change in the picture, and it swallowed any read failure with +``except Exception: pass``. It now sends the file's own bytes, the payload the +/stream/display SSE stream sends, and logs a failed read. +""" + +import base64 +import io +import logging +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +from flask import Flask +from PIL import Image, PngImagePlugin + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from web_interface import display_preview # noqa: E402 + + +@pytest.fixture +def client(monkeypatch): + from web_interface.blueprints.api_v3 import api_v3 + monkeypatch.setattr(api_v3, 'config_manager', MagicMock(), raising=False) + api_v3.config_manager.load_config.return_value = {} + app = Flask(__name__) + app.config['TESTING'] = True + app.register_blueprint(api_v3, url_prefix='/api/v3') + with app.test_client() as test_client: + yield test_client + + +@pytest.fixture +def snapshot(tmp_path, monkeypatch): + """A snapshot PNG carrying a text chunk, which a PIL re-encode drops.""" + info = PngImagePlugin.PngInfo() + info.add_text('written-by', 'display_manager') + buffer = io.BytesIO() + Image.new('RGB', (4, 2), (255, 0, 0)).save(buffer, format='PNG', pnginfo=info) + path = tmp_path / 'led_matrix_preview.png' + path.write_bytes(buffer.getvalue()) + monkeypatch.setattr(display_preview, 'SNAPSHOT_PATH', str(path)) + return path + + +def _image(client): + response = client.get('/api/v3/display/current') + assert response.status_code == 200 + return response.get_json()['data'] + + +def test_the_snapshot_bytes_are_sent_as_they_are(client, snapshot): + data = _image(client) + assert base64.b64decode(data['image']) == snapshot.read_bytes() + + +def test_route_and_stream_send_the_same_payload_keys(client, snapshot): + data = _image(client) + assert set(data) == set(display_preview.preview_payload(1, 1, None)) + + +def test_no_snapshot_is_a_null_image_without_a_warning(client, tmp_path, monkeypatch, caplog): + monkeypatch.setattr(display_preview, 'SNAPSHOT_PATH', str(tmp_path / 'missing.png')) + with caplog.at_level(logging.WARNING): + assert _image(client)['image'] is None + assert not [r for r in caplog.records if 'snapshot' in r.getMessage()] + + +def test_an_unreadable_snapshot_is_logged(client, snapshot, monkeypatch, caplog): + def denied(_path=None): + raise PermissionError('denied') + monkeypatch.setattr(display_preview, 'read_snapshot_base64', denied) + with caplog.at_level(logging.WARNING): + assert _image(client)['image'] is None + assert [r for r in caplog.records if 'snapshot' in r.getMessage() and r.exc_info] diff --git a/test/test_live_status_fields.py b/test/test_live_status_fields.py index 571683de..8f94f20f 100644 --- a/test/test_live_status_fields.py +++ b/test/test_live_status_fields.py @@ -31,8 +31,12 @@ import pytest PROJECT_ROOT = Path(__file__).parent.parent sys.path.insert(0, str(PROJECT_ROOT)) +from web_interface import system_metrics # noqa: E402 from web_interface.system_metrics import collect_system_metrics # noqa: E402 +MB = 1024 * 1024 +GB = 1024 * MB + @pytest.fixture def metrics(monkeypatch): @@ -45,10 +49,11 @@ def metrics(monkeypatch): lambda _p: (_ for _ in ()).throw(OSError("no such mount"))) else: monkeypatch.setattr(psutil, "disk_usage", - lambda _p: SimpleNamespace(percent=disk_percent)) + lambda _p: SimpleNamespace(percent=disk_percent, + total=32 * GB, used=4 * GB)) monkeypatch.setattr(psutil, "virtual_memory", - lambda: SimpleNamespace(percent=used_percent, - available=available_bytes)) + lambda: SimpleNamespace(percent=used_percent, total=1024 * MB, + used=600 * MB, available=available_bytes)) return collect_system_metrics() return _collect @@ -101,3 +106,18 @@ class TestEveryPredictiveFieldIsPresent: "memory_available_mb", "disk_used_percent"): assert field in m assert m["disk_used_percent"] is None + assert all(m[key] is None for key in m if key != "cpu_temp") + + +class TestUnavailableIsNull: + """One answer for "could not be read": None, never 0.""" + + def test_unreadable_temperature_is_null_not_zero_degrees(self, metrics, monkeypatch): + monkeypatch.setattr(system_metrics, "_THERMAL_ZONE", "/nonexistent/thermal/temp") + assert metrics()["cpu_temp"] is None + + def test_readable_temperature_is_degrees_c(self, metrics, monkeypatch, tmp_path): + zone = tmp_path / "temp" + zone.write_text("48312\n") + monkeypatch.setattr(system_metrics, "_THERMAL_ZONE", str(zone)) + assert metrics()["cpu_temp"] == 48.3 diff --git a/test/test_pages_v3_partials.py b/test/test_pages_v3_partials.py new file mode 100644 index 00000000..83fcf2ab --- /dev/null +++ b/test/test_pages_v3_partials.py @@ -0,0 +1,65 @@ +"""/partials/ dispatch and the plugin web_ui page's directory lookup.""" + +import logging +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +from flask import Flask + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from web_interface.blueprints import pages_v3 as module # noqa: E402 + + +@pytest.fixture +def client(tmp_path, monkeypatch): + plugin_manager = MagicMock() + plugin_manager.plugins_dir = tmp_path + monkeypatch.setattr(module.pages_v3, "plugin_manager", plugin_manager, raising=False) + monkeypatch.setattr(module.pages_v3, "config_manager", + MagicMock(load_config=lambda: {}), raising=False) + app = Flask(__name__, template_folder=str( + Path(module.__file__).resolve().parents[1] / "templates")) + app.register_blueprint(module.pages_v3) + return app.test_client() + + +def test_every_tab_the_old_if_chain_served_is_still_served(): + assert set(module._PARTIAL_LOADERS) == { + "overview", "general", "display", "durations", "schedule", "plugins", + "fonts", "logs", "raw-json", "backup-restore", "wifi", "cache", + "operation-history", "tools", + } + + +def test_an_unknown_partial_is_a_404(client): + response = client.get("/partials/nope") + assert response.status_code == 404 + + +def test_a_failing_loader_is_one_500_that_names_the_partial(client, monkeypatch, caplog): + def boom(): + raise RuntimeError("template exploded") + monkeypatch.setitem(module._PARTIAL_LOADERS, "logs", boom) + + with caplog.at_level(logging.ERROR): + response = client.get("/partials/logs") + + assert response.status_code == 500 + assert response.get_data(as_text=True) == "Error loading partial" + errors = [r for r in caplog.records if r.levelno >= logging.ERROR] + assert len(errors) == 1 + assert errors[0].getMessage() == "Error loading partial logs" + + +def test_web_ui_page_uses_the_ledmatrix_prefix_fallback(client, tmp_path): + web_ui = tmp_path / "ledmatrix-radar" / "web_ui" + web_ui.mkdir(parents=True) + (web_ui / "panel.html").write_text("

radar panel

", encoding="utf-8") + + response = client.get("/plugin-ui/radar/web-ui/panel.html") + + assert response.status_code == 200 + assert "radar panel" in response.get_data(as_text=True) diff --git a/test/test_system_status_available_memory.py b/test/test_system_status_available_memory.py index c08bde22..0113ea67 100644 --- a/test/test_system_status_available_memory.py +++ b/test/test_system_status_available_memory.py @@ -51,7 +51,7 @@ def _memory(total_mb, used_mb, available_mb): def _get_status(client, memory): # The endpoint caches for 10s; bypass so each case is measured fresh. - with patch("web_interface.cache.get_cached", return_value=None), \ + with patch("web_interface.blueprints.api_v3.system.get_cached", return_value=None), \ patch("psutil.virtual_memory", return_value=memory), \ patch("psutil.cpu_percent", return_value=5.0), \ patch("psutil.boot_time", return_value=0.0): @@ -90,3 +90,24 @@ def test_a_nearly_exhausted_board_reports_a_small_number(client): # to fork. The readout has to surface that rather than round it away. data = _get_status(client, _memory(total_mb=905, used_mb=800, available_mb=73)) assert data["memory_available_mb"] == pytest.approx(73, abs=0.5) + + +def test_status_and_live_stream_give_the_same_answer(client, monkeypatch): + """/system/status is built on collect_system_metrics(), so a metric has one + value, and "could not be read" is null in both.""" + from web_interface import system_metrics + monkeypatch.setattr(system_metrics, "_THERMAL_ZONE", "/nonexistent/thermal/temp") + memory = _memory(total_mb=905, used_mb=620, available_mb=284) + with patch("psutil.virtual_memory", return_value=memory), \ + patch("psutil.cpu_percent", return_value=5.0), \ + patch("psutil.boot_time", return_value=0.0), \ + patch("psutil.disk_usage", side_effect=OSError("no such mount")): + streamed = system_metrics.collect_system_metrics() + with patch("web_interface.blueprints.api_v3.system.get_cached", return_value=None): + status = json.loads(client.get("/api/v3/system/status").data)["data"] + + assert status["cpu_temp"] is None + assert status["disk_used_percent"] is None + for key in system_metrics.METRIC_KEYS: + if key != "uptime_seconds": # the two reads are a moment apart + assert status[key] == streamed[key], key diff --git a/test/web_interface/test_api_v3_plugin_config_save.py b/test/web_interface/test_api_v3_plugin_config_save.py new file mode 100644 index 00000000..c0de008c --- /dev/null +++ b/test/web_interface/test_api_v3_plugin_config_save.py @@ -0,0 +1,210 @@ +"""POST /plugins/config and POST /plugins/config/reset over a real +ConfigManager and SchemaManager. + +The save path reshapes what the browser posts before validating it, and these +tests pin the reshaping that validation depends on: repeats in a uniqueItems +list are dropped, and a list the form posted as numbered fields becomes a list +again. +""" + +import json +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent.parent)) + +from src.config_manager import ConfigManager # noqa: E402 +from src.plugin_system.schema_manager import SchemaManager # noqa: E402 +from test._api_v3_test_helpers import api_v3_module, build_app # noqa: E402,F401 + +PLUGIN_ID = "stocks" + +SCHEMA = { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": True}, + "stock_symbols": { + "type": "array", + "uniqueItems": True, + "items": {"type": "string"}, + "default": ["AAPL"], + }, + "refresh_seconds": {"type": "integer", "default": 60}, + "api_key": {"type": "string", "x-secret": True, "default": ""}, + }, +} + +# The shape of the news plugin's schema: a list of objects nested one level +# down. A form posts it as feeds.custom_feeds.0.name, feeds.custom_feeds.0.url, +# which lands as a dict keyed "0", "1", ... +NEWS_ID = "news" +NEWS_SCHEMA = { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": True}, + "feeds": { + "type": "object", + "properties": { + "enabled_feeds": { + "type": "array", "items": {"type": "string"}, "default": [], + }, + "custom_feeds": { + "type": "array", + "default": [], + "items": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "url": {"type": "string"}, + "enabled": {"type": "boolean", "default": True}, + }, + }, + }, + }, + }, + }, +} + + +@pytest.fixture +def env(tmp_path, api_v3_module): + """Real config and schema managers under tmp_path, on the blueprint.""" + plugins_dir = tmp_path / "plugins" + for plugin_id, schema in ((PLUGIN_ID, SCHEMA), (NEWS_ID, NEWS_SCHEMA)): + plugin_dir = plugins_dir / plugin_id + plugin_dir.mkdir(parents=True) + (plugin_dir / "config_schema.json").write_text(json.dumps(schema)) + (plugin_dir / "manifest.json").write_text(json.dumps( + {"id": plugin_id, "name": plugin_id, "version": "1.0.0"})) + + config_file = tmp_path / "config.json" + config_file.write_text(json.dumps( + {PLUGIN_ID: {"enabled": True, "stock_symbols": ["AAPL", "FNMA"]}})) + config_manager = ConfigManager( + config_path=str(config_file), + secrets_path=str(tmp_path / "config_secrets.json")) + config_manager.template_path = str(tmp_path / "no-template.json") + + api = api_v3_module.api_v3 + api.config_manager = config_manager + api.schema_manager = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path) + api.plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}, + NEWS_ID: {"id": NEWS_ID}} + api.plugin_manager.get_plugin.return_value = None + + class Env: + client = build_app(api).test_client() + plugin_manager = api.plugin_manager + + @staticmethod + def stored(plugin_id=PLUGIN_ID): + return json.loads(config_file.read_text())[plugin_id] + + Env.config_manager = config_manager + return Env + + +class TestUniqueItemsRepeats: + """A repeat in a uniqueItems list is dropped, not a failed save.""" + + def test_form_post_repeating_a_saved_symbol_saves(self, env): + response = env.client.post( + f"/api/v3/plugins/config?plugin_id={PLUGIN_ID}", + data={"stock_symbols": "AAPL, FNMA, FNMA"}) + + assert response.status_code == 200, response.get_json() + assert env.stored()["stock_symbols"] == ["AAPL", "FNMA"] + + def test_json_post_keeps_first_occurrence_order(self, env): + response = env.client.post("/api/v3/plugins/config", json={ + "plugin_id": PLUGIN_ID, + "config": {"stock_symbols": ["TSLA", "AAPL", "TSLA", "FNMA"]}, + }) + + assert response.status_code == 200, response.get_json() + assert env.stored()["stock_symbols"] == ["TSLA", "AAPL", "FNMA"] + + +class TestNumberedFieldsBecomeLists: + """The news plugin's custom feeds, posted one field per row.""" + + FEEDS = [{"name": "Local", "url": "https://example.com/local.xml", "enabled": True}, + {"name": "Tech", "url": "https://example.com/tech.xml", "enabled": False}] + + def test_form_post(self, env): + response = env.client.post(f"/api/v3/plugins/config?plugin_id={NEWS_ID}", data={ + "feeds.custom_feeds.0.name": "Local", + "feeds.custom_feeds.0.url": "https://example.com/local.xml", + "feeds.custom_feeds.0.enabled": "on", + "feeds.custom_feeds.1.name": "Tech", + "feeds.custom_feeds.1.url": "https://example.com/tech.xml", + }) + + assert response.status_code == 200, response.get_json() + assert env.stored(NEWS_ID)["feeds"]["custom_feeds"] == self.FEEDS + + def test_json_post(self, env): + response = env.client.post("/api/v3/plugins/config", json={ + "plugin_id": NEWS_ID, + "config": {"feeds": {"custom_feeds": {"1": self.FEEDS[1], "0": self.FEEDS[0]}}}, + }) + + assert response.status_code == 200, response.get_json() + assert env.stored(NEWS_ID)["feeds"]["custom_feeds"] == self.FEEDS + + +class TestReset: + """POST /plugins/config/reset saves the way every other plugin save does.""" + + def test_reset_saves_atomically_with_a_backup(self, env, monkeypatch): + cm = env.config_manager + calls = [] + real_atomic = cm.save_config_atomic + + def spy(config, create_backup=True, **kwargs): + calls.append(create_backup) + return real_atomic(config, create_backup=create_backup, **kwargs) + + def no_plain_save(_config): + raise AssertionError("reset bypassed the atomic save") + + monkeypatch.setattr(cm, "save_config_atomic", spy) + monkeypatch.setattr(cm, "save_config", no_plain_save) + + response = env.client.post("/api/v3/plugins/config/reset", + json={"plugin_id": PLUGIN_ID}) + + assert response.status_code == 200, response.get_json() + assert calls == [True] + assert env.stored()["stock_symbols"] == ["AAPL"] + + def test_reset_notifies_the_plugin_with_its_prepared_config(self, env): + plugin = MagicMock() + env.plugin_manager.get_plugin.return_value = plugin + env.plugin_manager.prepare_plugin_config.side_effect = ( + lambda _pid, raw: {**raw, "prepared": True}) + + response = env.client.post("/api/v3/plugins/config/reset", + json={"plugin_id": PLUGIN_ID}) + + assert response.status_code == 200, response.get_json() + handed_over = plugin.on_config_change.call_args.args[0] + assert handed_over["prepared"] is True + assert handed_over["stock_symbols"] == ["AAPL"] + + def test_a_failed_save_is_reported(self, env, monkeypatch): + failed = MagicMock(message="disk full") + failed.status.value = "failed" + monkeypatch.setattr(env.config_manager, "save_config_atomic", + MagicMock(return_value=failed)) + + response = env.client.post("/api/v3/plugins/config/reset", + json={"plugin_id": PLUGIN_ID}) + + assert response.status_code == 500 + assert "disk full" in response.get_json()["message"] diff --git a/test/web_interface/test_config_arrays.py b/test/web_interface/test_config_arrays.py new file mode 100644 index 00000000..dd617695 --- /dev/null +++ b/test/web_interface/test_config_arrays.py @@ -0,0 +1,58 @@ +"""coerce_array_shapes: position-keyed dicts back into lists before validation.""" + +from src.web_interface.config_arrays import coerce_array_shapes + +COLOR = {"type": "array", "items": {"type": "integer"}, + "minItems": 3, "maxItems": 3, "default": [255, 255, 255]} + + +def test_position_keys_become_a_list_in_numeric_order(): + config = {"tags": {"10": "k", "2": "c", "0": "a"}} + coerce_array_shapes(config, {"tags": {"type": "array"}}) + assert config["tags"] == ["a", "c", "k"] + + +def test_an_empty_dict_becomes_an_empty_list(): + config = {"tags": {}} + coerce_array_shapes(config, {"tags": {"type": "array"}}) + assert config["tags"] == [] + + +def test_a_dict_with_other_keys_is_left_for_validation_to_report(): + config = {"tags": {"0": "a", "name": "b"}} + coerce_array_shapes(config, {"tags": {"type": "array"}}) + assert config["tags"] == {"0": "a", "name": "b"} + + +def test_element_types_are_left_to_normalization(): + config = {"color": {"0": "1", "1": "2", "2": "3"}} + coerce_array_shapes(config, {"color": COLOR}) + assert config["color"] == ["1", "2", "3"] + + +def test_nested_objects_and_array_items_are_walked(): + schema = {"feeds": {"type": "object", "properties": { + "custom_feeds": {"type": "array", "items": {"type": "object", "properties": { + "tags": {"type": "array"}, + }}}, + }}} + config = {"feeds": {"custom_feeds": {"0": {"tags": {"0": "news"}}}}} + coerce_array_shapes(config, schema) + assert config == {"feeds": {"custom_feeds": [{"tags": ["news"]}]}} + + +def test_a_short_form_list_takes_the_default_only_when_asked(): + config = {"color": ["10", "20"]} + coerce_array_shapes(config, {"color": COLOR}) + assert config["color"] == ["10", "20"] + + coerce_array_shapes(config, {"color": COLOR}, short_lists_take_default=True) + assert config["color"] == [255, 255, 255] + assert config["color"] is not COLOR["default"] + + +def test_a_default_too_short_itself_is_not_used(): + schema = {"color": dict(COLOR, default=[0])} + config = {"color": ["10"]} + coerce_array_shapes(config, schema, short_lists_take_default=True) + assert config["color"] == ["10"] diff --git a/test/web_interface/test_dedup_unique_arrays.py b/test/web_interface/test_dedup_unique_arrays.py index 170f776c..787b4f2f 100644 --- a/test/web_interface/test_dedup_unique_arrays.py +++ b/test/web_interface/test_dedup_unique_arrays.py @@ -1,11 +1,9 @@ -"""Tests for dedup_unique_arrays used by save_plugin_config. +"""Tests for dedup_unique_arrays, which the plugin-config save path +(_prepare_plugin_config_for_save in api_v3/plugins.py) runs before validation. -Validates that arrays with uniqueItems: true in the JSON schema have -duplicates removed before validation, preventing spurious validation -failures when form merging introduces duplicate entries. - -Tests import the production function from src.web_interface.validators -to ensure they exercise the real code path. +Arrays with uniqueItems: true in the JSON schema lose their duplicates, so a +repeat introduced by form merging does not fail validation. +test_api_v3_plugin_config_save.py checks the same through the endpoint. """ diff --git a/web_interface/app.py b/web_interface/app.py index 2a8822fa..535b055d 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -46,6 +46,7 @@ _JOURNALCTL = shutil.which('journalctl') _SYSTEMCTL = shutil.which('systemctl') _VCGENCMD = shutil.which('vcgencmd') +from web_interface import display_preview from web_interface.system_metrics import collect_system_metrics # Create Flask app @@ -53,14 +54,10 @@ app = Flask(__name__) app.secret_key = os.urandom(24) config_manager = ConfigManager() -# CSRF protection disabled for local-only application -# CSRF is designed for internet-facing web apps to prevent cross-site request forgery. -# For a local-only Raspberry Pi application, the threat model is different: -# - If an attacker has network access to perform CSRF, they have other attack vectors -# - All API endpoints are programmatic (HTMX/fetch) and don't include CSRF tokens -# - Forms use HTMX which doesn't automatically include CSRF tokens -# If you need CSRF protection (e.g., exposing to internet), properly implement CSRF tokens in HTMX forms -csrf = None +# No CSRF protection: the UI is meant for the local network, where anyone who +# can forge a request can also send it directly, and neither the HTMX forms +# nor the fetch() calls carry a token. Exposing the UI beyond the LAN needs +# CSRF tokens added to both first. # Initialize rate limiting (prevent accidental abuse, not security) try: @@ -93,8 +90,6 @@ except ImportError: "'pip install flask-compress'." ) -# Import cache functions from separate module to avoid circular imports - # Initialize plugin managers - read plugins directory from config config = config_manager.load_config() plugin_system_config = config.get('plugin_system', {}) @@ -237,7 +232,6 @@ def serve_plugin_asset(plugin_id, filename): if assets_dir is None: return jsonify({'status': 'error', 'message': 'Invalid asset path'}), 403 - # Security check: ensure the assets directory exists and is within project_root if not assets_dir.exists() or not assets_dir.is_dir(): return jsonify({'status': 'error', 'message': 'Asset directory not found'}), 404 @@ -694,7 +688,7 @@ def system_status_generator(): 'power': _get_power_status() } yield status - except Exception as e: + except Exception: app.logger.error("SSE generator error", exc_info=True) yield {'error': 'An error occurred; see server logs'} time.sleep(10) # Update every 10 seconds (reduced frequency for better performance) @@ -702,9 +696,7 @@ def system_status_generator(): # Display preview generator for SSE def display_preview_generator(): """Generate display preview updates from snapshot file""" - import base64 - - snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path matches display_manager; only read here + snapshot_path = display_preview.SNAPSHOT_PATH # Viewer marker: this generator only runs while the broadcaster has # subscribers (it exits with no clients), so touching the marker each # loop tells the DISPLAY service a browser is actually watching — it @@ -739,20 +731,8 @@ def display_preview_generator(): # Only read if file is new or has been updated if last_modified is None or current_modified > last_modified: try: - # The snapshot is already a PNG, written atomically by - # the display service (tmp + os.replace in - # display_manager), so pass the raw bytes straight - # through instead of PIL-decoding and re-encoding — - # identical payload, much less CPU on the Pi. - with open(snapshot_path, 'rb') as f: - img_str = base64.b64encode(f.read()).decode('utf-8') - - preview_data = { - 'timestamp': time.time(), - 'width': width, - 'height': height, - 'image': img_str - } + preview_data = display_preview.preview_payload( + width, height, display_preview.read_snapshot_base64(snapshot_path)) last_modified = current_modified yield preview_data except OSError: @@ -760,27 +740,20 @@ def display_preview_generator(): # between mtime check and read); skip this update. app.logger.debug("Preview snapshot read failed; skipping frame", exc_info=True) else: - # No snapshot available - yield { - 'timestamp': time.time(), - 'width': width, - 'height': height, - 'image': None - } + yield display_preview.preview_payload(width, height, None) - except Exception as e: + except Exception: app.logger.error("SSE generator error", exc_info=True) yield {'error': 'An error occurred; see server logs'} - time.sleep(1.0) # Check once per second — halves PIL encode overhead vs 0.5s + time.sleep(1.0) # the snapshot is re-read only when its mtime changes # Logs generator for SSE def logs_generator(): """Generate log updates from journalctl""" while True: try: - # Get recent logs from journalctl (simplified version) - # Note: User should be in systemd-journal group to read logs without sudo + # Reading the journal without sudo needs the systemd-journal group. try: if not _JOURNALCTL: yield {'timestamp': time.time(), 'logs': 'journalctl not found; cannot read logs'} @@ -877,29 +850,19 @@ def stream_display(): def stream_logs(): return _sse_stream(_logs_broadcaster) -# Exempt SSE streams from CSRF and apply a generous rate limit. -# SSE connections are long-lived HTTP requests, not repeated API calls, so the -# tight "20 per minute" default would be exhausted quickly on reconnects. -if csrf: - csrf.exempt(stream_stats) - csrf.exempt(stream_display) - csrf.exempt(stream_logs) - # Note: api_v3 blueprint is exempted above after registration - +# Each SSE stream is one long-lived request, so only a (re)connect counts +# against a limit. The streams get their own 200 per minute, tighter than the +# 1000 per minute default, which bounds a client stuck reconnecting. if limiter: limiter.limit("200 per minute")(stream_stats) limiter.limit("200 per minute")(stream_display) limiter.limit("200 per minute")(stream_logs) -# The pages blueprint's index now serves '/' directly (see the un-prefixed -# blueprint registration above), so no redirect route is needed here. - @app.route('/favicon.ico') def favicon(): """Return 204 No Content for favicon to avoid 404 errors""" return '', 204 -_reconciliation_done = False _reconciliation_started = False import threading as _threading _reconciliation_lock = _threading.Lock() @@ -907,18 +870,13 @@ _reconciliation_lock = _threading.Lock() def _run_startup_reconciliation() -> None: """Run state reconciliation in background to auto-repair missing plugins. - Reconciliation runs exactly once per process lifetime, regardless of - whether every inconsistency could be auto-fixed. Previously, a failed - auto-repair (e.g. a config entry referencing a plugin that no longer - exists in the registry) would reset ``_reconciliation_started`` to False, - causing the ``@app.before_request`` hook to re-trigger reconciliation on - every single HTTP request — an infinite install-retry loop that pegged - the CPU and flooded the log. Unresolved issues are now left in place for - the user to address via the UI; the reconciler itself also caches - per-plugin unrecoverable failures internally so repeated reconcile calls - stay cheap. + Runs once per process, whether or not every inconsistency could be fixed: + ``_reconciliation_started`` is never reset. Resetting it after a failed + repair (a config entry naming a plugin the registry no longer has) made + the ``before_request`` hook rerun reconciliation on every request, an + install-retry loop that pegged the CPU and flooded the log. Unresolved + issues are left for the user to address in the UI. """ - global _reconciliation_done from src.logging_config import get_logger _logger = get_logger('reconciliation') @@ -980,11 +938,6 @@ def _run_startup_reconciliation() -> None: pass except Exception as e: _logger.error("[Reconciliation] Error: %s", e, exc_info=True) - finally: - # Always mark done — we do not want an unhandled exception (or an - # unresolved inconsistency) to cause the @before_request hook to - # retrigger reconciliation on every subsequent request. - _reconciliation_done = True # Run reconciliation in the background on first request @app.before_request diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index 4e5815df..46eade4f 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -49,7 +49,6 @@ from src.plugin_system.operation_types import OperationType from src.web_interface.validators import ( validate_file_upload ) -from src.error_aggregator import get_error_aggregator from src.common.permission_utils import install_requirements_file from src.common.path_safety import resolve_under from src.device_location import DeviceLocationResolver, apply_device_location @@ -95,13 +94,11 @@ def _scrub_git_remote_url(url: str) -> str: # `config_manager` used to resolve to a None that was never assigned, which # silently disabled the /health checks and made /display/current fall back # to a hardcoded 128x64. -# Get project root directory (web_interface/../..) -# web_interface/blueprints/api_v3/_common.py -> up four to the project root. -# This was three levels when everything lived in web_interface/blueprints/api_v3.py; -# the split moved the file one directory deeper and silently pointed PROJECT_ROOT -# at web_interface/ instead. Nothing failed at import -- it surfaced as routes -# 404ing and "installation script not found", because every path built from it -# was wrong. Asserted in test_api_v3_url_map.py so the next move cannot repeat it. +# The project root, three directories above this package. The split from a +# single api_v3.py moved this file one directory deeper, and a count left at +# the old depth pointed PROJECT_ROOT at web_interface/ without failing at +# import: routes 404ed and reported "installation script not found". +# test_api_v3_url_map.py asserts it so the next move cannot repeat that. PROJECT_ROOT = Path(__file__).resolve().parents[3] # System fonts that cannot be deleted (used by catalog API and delete endpoint) SYSTEM_FONTS = frozenset([ @@ -415,9 +412,9 @@ def resolve_pull_command(project_dir): is: first_time_install.sh chmods five scripts that git tracked as 644, so every machine that ran the installer carries five permanent mode changes and the update button reports "cannot pull with rebase: You have unstaged - changes". Those modes are corrected in this commit, but a user cannot pull - the correction while the pull is what is blocked, and any other local edit - would reproduce it anyway. Autostash reapplies the changes afterwards. + changes". The repository now tracks those modes, but a user cannot pull + that correction while the pull is what is blocked, and any other local + edit would reproduce it anyway. Autostash reapplies the changes afterwards. Returns ``(args, note, error)``. When ``origin/`` exists the pull is made explicit against it, so the update proceeds and the branch is @@ -594,12 +591,7 @@ def _installed_plugin_ids(): enumerate the installed plugins and read each one's persisted summary by ID instead of relying on the tracker's in-memory `get_all_*` view. """ - manifests = _discovered_plugin_manifests() - try: - return list(manifests.keys()) if manifests else [] - except Exception: - logger.debug('listing plugin_manifests failed while building plugin ids', exc_info=True) - return [] + return list(_discovered_plugin_manifests()) def _discovered_plugin_manifests(plugin_id=None, rescan=False): """The plugin manager's manifests, discovering plugins first if needed. @@ -741,8 +733,6 @@ def _parse_form_value(value): Parse a form value into the appropriate Python type. Handles booleans, numbers, JSON arrays/objects, and strings. """ - import json - if value is None: return None @@ -892,8 +882,6 @@ def _parse_form_value_with_schema(value, key_path, schema): Returns: Parsed value with correct type, or _SKIP_FIELD to indicate the field should not be set """ - import json - # Get the schema property for this field prop = _get_schema_property(schema, key_path) @@ -1394,16 +1382,24 @@ def _prune_credential_backups(plugin_dir: Path) -> None: # calendarList.list pages at 250 entries maximum. Ten pages is far past any # real account and exists only so a malformed nextPageToken cannot spin here. _CALENDAR_LIST_MAX_PAGES = 10 +def _plugin_directory(plugin_id: str) -> Optional[Path]: + """An installed plugin's directory, or None when it has none on disk. + + Only the plugin manager is asked, so no plugin manager means None. There + is no fallback to the legacy plugins/ directory: the loader never scans + it, so a plugin found only there is one that never runs. + """ + if not api_v3.plugin_manager: + return None + plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) + if not plugin_dir or not Path(plugin_dir).exists(): + return None + return Path(plugin_dir) + + def _calendar_plugin_dir() -> Optional[Path]: """Where the calendar plugin is installed, or None if it is not.""" - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory('calendar') - else: - plugin_dir = PROJECT_ROOT / 'plugins' / 'calendar' - if not plugin_dir: - return None - plugin_dir = Path(plugin_dir) - return plugin_dir if plugin_dir.exists() else None + return _plugin_directory('calendar') def _run_calendar_registration(plugin_dir: Path, stdin_payload: str): """Run the plugin's OAuth script and return the JSON object it prints. @@ -1901,8 +1897,6 @@ def _write_starlark_manifest(manifest: Dict[str, Any]) -> bool: return False def _install_star_file(app_id: str, star_file_path: str, metadata: Dict[str, Any], assets_dir: Optional[str] = None) -> bool: """Install a .star file and update the manifest (standalone, no plugin needed).""" - import shutil - import json app_dir, path_error = _validate_starlark_app_path(app_id) if path_error: logger.warning("Refusing to install %r: %s", app_id, path_error) diff --git a/web_interface/blueprints/api_v3/backup.py b/web_interface/blueprints/api_v3/backup.py index f15220ea..edc53865 100644 --- a/web_interface/blueprints/api_v3/backup.py +++ b/web_interface/blueprints/api_v3/backup.py @@ -1,7 +1,7 @@ """Backup creation, listing and restore. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( PROJECT_ROOT, Path, _coerce_to_bool, _safe_backup_path, api_v3, diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index 4cff4e51..00355af1 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -1,20 +1,20 @@ """Reading and writing configuration, including schedules. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( - ErrorCode, Optional, PROJECT_ROOT, Path, _coerce_to_bool, + ErrorCode, Optional, _coerce_to_bool, _redact_credentials, _validate_time_format, api_v3, deep_merge, - describe_exception, error_response, find_secret_fields, json, jsonify, - logger, logging, mask_all_secret_values, merge_secrets, os, - remove_empty_secrets, request, separate_secrets, strip_masked_values, - success_response, + describe_exception, error_response, json, jsonify, + logger, mask_all_secret_values, merge_secrets, + request, strip_masked_values, success_response, ) from src.common.path_safety import resolve_under from src.display_geometry import ORIENTATION_ROTATE_DEGREES from src.matrix_support import INT_SETTING_LIMITS, describe_range, library_refusals, refusal_message from src.pi5_matrix_support import is_raspberry_pi_5 +from web_interface.cache import invalidate_cache import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these @@ -27,6 +27,33 @@ import web_interface.blueprints.api_v3 as _pkg #: that a missing checkbox was unchecked, not merely left out of an API call. FORM_SECTION_FIELD = '__form_section' +#: Fields of the General tab. Any one of them in a /config/main post means the +#: General form was submitted, so its unchecked checkboxes read as False. +GENERAL_FIELDS = ('timezone', 'city', 'state', 'country', 'web_display_autostart', + 'plugins_directory', 'auto_update_enabled') + +#: Top-level fields save_main_config stores somewhere of its own (location, +#: plugin_system, ...), never as a config key of the same name. +_MAPPED_TOP_LEVEL_FIELDS = GENERAL_FIELDS + ( + 'auto_discover', 'auto_load_enabled', 'development_mode', 'target_fps') + + +def _plugin_id_list(raw, field_name): + """``(ids, None)`` for a list of plugin ids, or ``(None, message)``. + + The settings forms post these lists as JSON text in a hidden input; a JSON + client may send the array itself. Anything else is refused rather than + coerced: storing ``[]`` for a malformed value clears the saved order or + exclusions without a word. + """ + try: + parsed = json.loads(raw) if isinstance(raw, str) else raw + except (json.JSONDecodeError, TypeError, ValueError): + return None, f'{field_name} must be valid JSON' + if not isinstance(parsed, list) or not all(isinstance(p, str) for p in parsed): + return None, f'{field_name} must be a list of plugin-id strings' + return parsed, None + def _day_setting(data, day, flat_key, nested_key): """(present, value) of one per-day schedule setting in a POST body. @@ -203,16 +230,10 @@ def save_schedule_config(): status_code=500 ) - # Invalidate cache on config change - try: - from web_interface.cache import invalidate_cache - invalidate_cache() - except ImportError: - pass + invalidate_cache() return success_response(message='Schedule configuration saved successfully') except Exception as e: - import logging logger.error("Error saving schedule config", exc_info=True) return error_response( ErrorCode.CONFIG_SAVE_FAILED, @@ -223,11 +244,8 @@ def save_schedule_config(): @api_v3.route('/config/dim-schedule', methods=['GET']) def get_dim_schedule_config(): """Get current dim schedule configuration""" - import logging - import json - if not api_v3.config_manager: - logging.error("[DIM SCHEDULE] Config manager not initialized") + logger.error("[DIM SCHEDULE] Config manager not initialized") return error_response( ErrorCode.CONFIG_LOAD_FAILED, 'Config manager not initialized', @@ -247,28 +265,28 @@ def get_dim_schedule_config(): return success_response(data=dim_schedule_config) except FileNotFoundError as e: - logging.error(f"[DIM SCHEDULE] Config file not found: {e}", exc_info=True) + logger.error(f"[DIM SCHEDULE] Config file not found: {e}", exc_info=True) return error_response( ErrorCode.CONFIG_LOAD_FAILED, "Configuration file not found", status_code=500 ) except json.JSONDecodeError as e: - logging.error(f"[DIM SCHEDULE] Invalid JSON in config file: {e}", exc_info=True) + logger.error(f"[DIM SCHEDULE] Invalid JSON in config file: {e}", exc_info=True) return error_response( ErrorCode.CONFIG_LOAD_FAILED, "Configuration file contains invalid JSON", status_code=500 ) except (IOError, OSError) as e: - logging.error(f"[DIM SCHEDULE] Error reading config file: {e}", exc_info=True) + logger.error(f"[DIM SCHEDULE] Error reading config file: {e}", exc_info=True) return error_response( ErrorCode.CONFIG_LOAD_FAILED, "An error occurred; see logs for details", status_code=500, details=describe_exception(e) ) except Exception as e: - logging.error(f"[DIM SCHEDULE] Unexpected error loading config: {e}", exc_info=True) + logger.error(f"[DIM SCHEDULE] Unexpected error loading config: {e}", exc_info=True) return error_response( ErrorCode.CONFIG_LOAD_FAILED, "An error occurred; see logs for details", @@ -420,16 +438,10 @@ def save_dim_schedule_config(): status_code=500 ) - # Invalidate cache on config change - try: - from web_interface.cache import invalidate_cache - invalidate_cache() - except ImportError: - pass + invalidate_cache() return success_response(message='Dim schedule configuration saved successfully') except Exception as e: - import logging logger.error("Error saving dim schedule config", exc_info=True) return error_response( ErrorCode.CONFIG_SAVE_FAILED, @@ -490,11 +502,7 @@ def save_main_config(): current_config = api_v3.config_manager.load_config() was_auto_update_enabled = bool((current_config.get('auto_update') or {}).get('enabled')) - # Handle general settings - # Note: Checkboxes don't send data when unchecked, so we need to check if we're updating general settings - # If any general setting is present, we're updating the general tab - is_general_update = any(k in data for k in ['timezone', 'city', 'state', 'country', 'web_display_autostart', - 'plugins_directory', 'auto_update_enabled']) + is_general_update = any(k in data for k in GENERAL_FIELDS) if is_general_update: # For checkbox: if not present in data during a general *form* @@ -872,28 +880,13 @@ def save_main_config(): }), 400 vegas_config[config_key] = int_value - # Handle plugin order and exclusions (JSON arrays) - if 'vegas_plugin_order' in data: - try: - if isinstance(data['vegas_plugin_order'], str): - parsed = json.loads(data['vegas_plugin_order']) - else: - parsed = data['vegas_plugin_order'] - # Ensure result is a list - vegas_config['plugin_order'] = list(parsed) if isinstance(parsed, (list, tuple)) else [] - except (json.JSONDecodeError, TypeError, ValueError): - vegas_config['plugin_order'] = [] - - if 'vegas_excluded_plugins' in data: - try: - if isinstance(data['vegas_excluded_plugins'], str): - parsed = json.loads(data['vegas_excluded_plugins']) - else: - parsed = data['vegas_excluded_plugins'] - # Ensure result is a list - vegas_config['excluded_plugins'] = list(parsed) if isinstance(parsed, (list, tuple)) else [] - except (json.JSONDecodeError, TypeError, ValueError): - vegas_config['excluded_plugins'] = [] + for field_name, config_key in (('vegas_plugin_order', 'plugin_order'), + ('vegas_excluded_plugins', 'excluded_plugins')): + if field_name in data: + ids, id_error = _plugin_id_list(data[field_name], field_name) + if id_error: + return jsonify({'status': 'error', 'message': id_error}), 400 + vegas_config[config_key] = ids # Handle multi-display sync settings sync_fields = ["sync_role", "sync_port", "sync_follower_position"] @@ -921,19 +914,11 @@ def save_main_config(): return jsonify({"status": "error", "message": "sync_follower_position must be left or right"}), 400 current_config["sync"]["follower_position"] = pos_val - # Handle primary rotation order: must be a JSON array of plugin-id - # strings. Reject anything else with a 400 rather than silently - # coercing, so a buggy client can't clear or corrupt the saved order. if 'plugin_rotation_order' in data: - raw_order = data.pop('plugin_rotation_order') - try: - parsed = json.loads(raw_order) if isinstance(raw_order, str) else raw_order - except (json.JSONDecodeError, TypeError, ValueError): - return jsonify({'status': 'error', - 'message': 'plugin_rotation_order must be valid JSON'}), 400 - if not isinstance(parsed, list) or not all(isinstance(p, str) for p in parsed): - return jsonify({'status': 'error', - 'message': 'plugin_rotation_order must be a list of plugin-id strings'}), 400 + parsed, id_error = _plugin_id_list(data.pop('plugin_rotation_order'), + 'plugin_rotation_order') + if id_error: + return jsonify({'status': 'error', 'message': id_error}), 400 if 'display' not in current_config: current_config['display'] = {} current_config['display']['plugin_rotation_order'] = parsed @@ -1084,27 +1069,15 @@ def save_main_config(): for key in plugin_keys_to_remove: del data[key] - # Handle any remaining config keys - # System settings (timezone, city, etc.) are already handled above - # Plugin configs should use /api/v3/plugins/config endpoint, but we'll handle them here too for flexibility + # Whatever no section above claimed is stored as a top-level key, a + # dict merged onto the stored one. Plugin sections were handled and + # removed above. Form field names the sections above already stored + # elsewhere are skipped, or each would land as a top-level key too. + mapped_fields = set(_MAPPED_TOP_LEVEL_FIELDS).union( + display_fields, sync_fields, vegas_fields, double_sided_fields) for key in data: - # Skip system settings that are already handled above - if key in ['timezone', 'city', 'state', 'country', - 'web_display_autostart', 'auto_discover', - 'auto_load_enabled', 'development_mode', - 'plugins_directory', 'target_fps', 'auto_update_enabled']: + if key in mapped_fields: continue - # Skip fields that are already handled above in their own named sections. - # Without this, every form field name lands as a top-level config key too. - if key in display_fields: - continue - if key in sync_fields: - continue - if key in vegas_fields: - continue - if key in double_sided_fields: - continue - # For any remaining keys (including plugin keys), use deep merge to preserve existing settings if key in current_config and isinstance(current_config[key], dict) and isinstance(data[key], dict): # Deep merge to preserve existing settings current_config[key] = deep_merge(current_config[key], data[key]) @@ -1120,12 +1093,7 @@ def save_main_config(): status_code=500 ) - # Invalidate cache on config change - try: - from web_interface.cache import invalidate_cache - invalidate_cache() - except ImportError: - pass + invalidate_cache() # Notify saved plugins of their new config (with secrets merged), now # that it is on disk. diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index 68a0ab0d..0d99c0df 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -1,13 +1,14 @@ """Display control, on-demand playback and preview. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( _ensure_display_service_running, _get_display_service_status, _stop_display_service, api_v3, - jsonify, logger, os, request, uuid, + jsonify, logger, request, uuid, ) +from web_interface import display_preview import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. @@ -30,13 +31,11 @@ def _cache_manager(): @api_v3.route('/display/current', methods=['GET']) def get_display_current(): - """Get current display state""" - import base64 - from PIL import Image - import io - - snapshot_path = "/tmp/led_matrix_preview.png" + """The latest display preview, as the /stream/display SSE stream sends it. + ``data`` is ``{timestamp, width, height, image}``; ``image`` is the + snapshot PNG base64-encoded, or null when there is none to show. + """ # Get display dimensions from config: the logical size DisplayManager # renders at, so double-sided setups preview one screen from src.display_geometry import logical_size @@ -46,26 +45,16 @@ def get_display_current(): except Exception: width, height = logical_size({}) - # Try to read snapshot file - image_data = None - if os.path.exists(snapshot_path): - try: - with Image.open(snapshot_path) as img: - # Convert to PNG and encode as base64 - buffer = io.BytesIO() - img.save(buffer, format='PNG') - image_data = base64.b64encode(buffer.getvalue()).decode('utf-8') - except Exception as img_err: - # File might be being written or corrupted, return None - pass + try: + image = display_preview.read_snapshot_base64() + except FileNotFoundError: + image = None # the display service has not written one yet + except OSError: + logger.warning("Could not read the display preview snapshot", exc_info=True) + image = None - display_data = { - 'timestamp': _pkg.time.time(), - 'width': width, - 'height': height, - 'image': image_data # Base64 encoded image data or None if unavailable - } - return jsonify({'status': 'success', 'data': display_data}) + return jsonify({'status': 'success', + 'data': display_preview.preview_payload(width, height, image)}) @api_v3.route('/display/modes', methods=['GET']) def get_display_modes(): """Every display mode that can be requested on-demand, with its plugin. diff --git a/web_interface/blueprints/api_v3/fonts.py b/web_interface/blueprints/api_v3/fonts.py index ff52c744..2eb28a8a 100644 --- a/web_interface/blueprints/api_v3/fonts.py +++ b/web_interface/blueprints/api_v3/fonts.py @@ -1,12 +1,13 @@ """Font catalogue, upload, preview and deletion. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( PROJECT_ROOT, Path, Response, SYSTEM_FONTS, api_v3, jsonify, logger, os, re, request, validate_file_upload, ) +from web_interface.cache import delete_cached, get_cached, set_cached def _catalog_response(catalog): @@ -52,16 +53,9 @@ def _catalog_response(catalog): @api_v3.route('/fonts/catalog', methods=['GET']) def get_fonts_catalog(): """Get fonts catalog""" - # Check cache first (5 minute TTL) - try: - from web_interface.cache import get_cached, set_cached - cached_result = get_cached('fonts_catalog', ttl_seconds=300) - if cached_result is not None: - return _catalog_response(cached_result) - except ImportError: - # Cache not available, continue without caching - get_cached = None - set_cached = None + cached_result = get_cached('fonts_catalog', ttl_seconds=300) + if cached_result is not None: + return _catalog_response(cached_result) # Try to import freetype, but continue without it if unavailable try: @@ -148,12 +142,7 @@ def get_fonts_catalog(): 'metadata': metadata if metadata else None } - # Cache the result (5 minute TTL) if available - if set_cached: - try: - set_cached('fonts_catalog', catalog, ttl_seconds=300) - except Exception: - logger.error("[FontCatalog] Failed to cache fonts_catalog", exc_info=True) + set_cached('fonts_catalog', catalog, ttl_seconds=300) return _catalog_response(catalog) @api_v3.route('/fonts/tokens', methods=['GET']) @@ -229,14 +218,7 @@ def upload_font(): # Save the file font_file.save(str(filepath)) - # Clear font catalog cache - try: - from web_interface.cache import delete_cached - delete_cached('fonts_catalog') - except ImportError as e: - logger.warning("[FontUpload] Cache module not available: %s", e) - except Exception: - logger.error("[FontUpload] Failed to clear fonts_catalog cache", exc_info=True) + delete_cached('fonts_catalog') return jsonify({ 'status': 'success', @@ -453,14 +435,7 @@ def delete_font(font_family: str) -> tuple[Response, int] | Response: if not deleted: return jsonify({'status': 'error', 'message': f'Font not found: {font_family}'}), 404 - # Clear font catalog cache - try: - from web_interface.cache import delete_cached - delete_cached('fonts_catalog') - except ImportError as e: - logger.warning("[FontDelete] Cache module not available: %s", e) - except Exception: - logger.error("[FontDelete] Failed to clear fonts_catalog cache", exc_info=True) + delete_cached('fonts_catalog') return jsonify({ 'status': 'success', diff --git a/web_interface/blueprints/api_v3/misc.py b/web_interface/blueprints/api_v3/misc.py index 0f4df30e..e1fd7af4 100644 --- a/web_interface/blueprints/api_v3/misc.py +++ b/web_interface/blueprints/api_v3/misc.py @@ -1,12 +1,12 @@ """Routes with no larger group of their own: errors, integrations, cache, sync, logs, health and hardware. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( - _coerce_to_bool, - ErrorCode, Path, _JOURNALCTL, _MQTT_BRIDGE_CONFIG, _MQTT_BRIDGE_DEFAULTS, + _coerce_to_bool, _discovered_plugin_manifests, + ErrorCode, _JOURNALCTL, _MQTT_BRIDGE_CONFIG, _MQTT_BRIDGE_DEFAULTS, _MQTT_BRIDGE_DIR, _SUDO, _coerce_mqtt_bridge_value, _get_display_service_status, _mqtt_bridge_service_state, _read_mqtt_bridge_config, api_v3, contextlib, describe_exception, @@ -16,6 +16,7 @@ from web_interface.blueprints.api_v3 import ( from src.common.path_safety import safe_path_component from src.common import sync_manager as _sync from src import error_aggregator as _errors +from web_interface import display_preview import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. @@ -34,11 +35,9 @@ def get_health(): 'checks': {} } - # Check web interface service - # Stamp the start _pkg.time before measuring against it -- reading it with a - # fallback of _pkg.time.time() and only assigning afterwards made the very - # first call subtract two separate clock reads, reporting a small - # negative uptime. + # Stamp the start time before measuring against it: reading it with a + # fallback of time.time() and assigning it afterwards made the first + # call subtract two separate clock reads, a small negative uptime. if not hasattr(get_health, '_start_time'): get_health._start_time = _pkg.time.time() health_status['services']['web_interface'] = { @@ -56,7 +55,7 @@ def get_health(): # Check config file accessibility try: if api_v3.config_manager: - test_config = api_v3.config_manager.load_config() + api_v3.config_manager.load_config() health_status['checks']['config_file'] = { 'status': 'accessible', 'readable': True @@ -66,7 +65,8 @@ def get_health(): 'status': 'unknown', 'readable': False } - except Exception as e: + except Exception: + logger.warning("Health check could not read the config file", exc_info=True) health_status['checks']['config_file'] = { 'status': 'error', 'readable': False, @@ -76,8 +76,7 @@ def get_health(): # Check plugin system try: if api_v3.plugin_manager: - # Try to discover plugins (lightweight check) - plugin_count = len(api_v3.plugin_manager.get_available_plugins()) if hasattr(api_v3.plugin_manager, 'get_available_plugins') else 0 + plugin_count = len(_discovered_plugin_manifests()) health_status['checks']['plugin_system'] = { 'status': 'operational', 'plugin_count': plugin_count @@ -86,7 +85,8 @@ def get_health(): health_status['checks']['plugin_system'] = { 'status': 'not_initialized' } - except Exception as e: + except Exception: + logger.warning("Health check could not count plugins", exc_info=True) health_status['checks']['plugin_system'] = { 'status': 'error', 'error': 'see logs for details' @@ -94,7 +94,7 @@ def get_health(): # Check hardware connectivity (if display manager available) try: - snapshot_path = "/tmp/led_matrix_preview.png" + snapshot_path = display_preview.SNAPSHOT_PATH if os.path.exists(snapshot_path): # Check if snapshot is recent (updated in last 60 seconds) mtime = os.path.getmtime(snapshot_path) @@ -108,7 +108,8 @@ def get_health(): 'status': 'no_snapshot', 'note': 'Display service may not be running' } - except Exception as e: + except Exception: + logger.warning("Health check could not read the preview snapshot", exc_info=True) health_status['checks']['hardware'] = { 'status': 'unknown', 'error': 'see logs for details' diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index 24ecc362..a5ebefbf 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -1,7 +1,7 @@ """Plugin install, update, enable/disable, config and store routes. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( ErrorCode, OperationType, PROJECT_ROOT, Path, Response, @@ -9,13 +9,14 @@ from web_interface.blueprints.api_v3 import ( _do_transactional_uninstall, _enhance_schema_with_core_properties, _filter_config_by_schema, _get_plugin_version, _get_schema_property, _hidden_array_item_property, _installed_plugin_ids, _is_plugin_update_available, + _plugin_directory, _parse_form_value_with_schema, _prune_credential_backups, _run_calendar_registration, _schema_allows_null, _schema_type_is, _set_missing_booleans_to_false, _set_nested_value, _starlark_virtual_plugins, _toggle_starlark_app, api_v3, datetime, deep_merge, describe_exception, error_response, exception_error_response, - find_secret_fields, hashlib, json, jsonify, logger, logging, + find_secret_fields, hashlib, json, jsonify, logger, merge_secrets, os, redact_text, remove_empty_secrets, request, separate_secrets, shutil, stat, subprocess, success_response, tempfile, uuid, validate_request_json, @@ -23,6 +24,8 @@ from web_interface.blueprints.api_v3 import ( from src.common.path_safety import ( resolve_under, safe_path_component, safe_relative_parts, ) +from src.web_interface.config_arrays import coerce_array_shapes +from src.web_interface.validators import dedup_unique_arrays import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. @@ -36,9 +39,6 @@ def get_installed_plugins(): if not api_v3.plugin_manager or not api_v3.plugin_store_manager: return jsonify({'status': 'error', 'message': 'Plugin managers not initialized'}), 500 - import json - from pathlib import Path - # Re-discover plugins to ensure we have the latest list # This handles cases where plugins are added/removed after app startup api_v3.plugin_manager.discover_plugins() @@ -180,8 +180,7 @@ def get_plugin_health(): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: + if not api_v3.plugin_manager.health_tracker: return jsonify({ 'status': 'success', 'data': {}, @@ -216,15 +215,15 @@ def get_plugin_health_single(plugin_id): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: + if not api_v3.plugin_manager.health_tracker: return jsonify({ 'status': 'error', 'message': 'Health tracking not available' }), 503 - # Get health summary for specific plugin - health_summary = api_v3.plugin_manager.health_tracker.get_health_summary(plugin_id) + # force_reload for the same reason as the list route above. + health_summary = api_v3.plugin_manager.health_tracker.get_health_summary( + plugin_id, force_reload=True) return jsonify({ 'status': 'success', @@ -236,8 +235,7 @@ def reset_plugin_health(plugin_id): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: + if not api_v3.plugin_manager.health_tracker: return jsonify({ 'status': 'error', 'message': 'Health tracking not available' @@ -256,8 +254,7 @@ def get_plugin_metrics(): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: + if not api_v3.plugin_manager.resource_monitor: return jsonify({ 'status': 'success', 'data': {}, @@ -291,15 +288,15 @@ def get_plugin_metrics_single(plugin_id): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: + if not api_v3.plugin_manager.resource_monitor: return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' }), 503 - # Get metrics summary for specific plugin - metrics_summary = api_v3.plugin_manager.resource_monitor.get_metrics_summary(plugin_id) + # force_reload for the same reason as the list route above. + metrics_summary = api_v3.plugin_manager.resource_monitor.get_metrics_summary( + plugin_id, force_reload=True) return jsonify({ 'status': 'success', @@ -311,8 +308,7 @@ def reset_plugin_metrics(plugin_id): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: + if not api_v3.plugin_manager.resource_monitor: return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' @@ -331,8 +327,7 @@ def manage_plugin_limits(plugin_id): if not api_v3.plugin_manager: return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: + if not api_v3.plugin_manager.resource_monitor: return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' @@ -433,17 +428,13 @@ def toggle_plugin(): config[plugin_id] = {} config[plugin_id]['enabled'] = enabled - # Use atomic save if available - if hasattr(api_v3.config_manager, 'save_config_atomic'): - result = api_v3.config_manager.save_config_atomic(config, create_backup=True) - if result.status.value != 'success': - return error_response( - ErrorCode.CONFIG_SAVE_FAILED, - f"Failed to save configuration: {result.message}", - status_code=500 - ) - else: - api_v3.config_manager.save_config(config) + success, error_msg = _pkg._save_config_atomic(api_v3.config_manager, config, create_backup=True) + if not success: + return error_response( + ErrorCode.CONFIG_SAVE_FAILED, + f"Failed to save configuration: {error_msg}", + status_code=500 + ) # Update state manager if available if api_v3.plugin_state_manager: @@ -470,8 +461,7 @@ def toggle_plugin(): plugin.on_disable() except Exception as lifecycle_error: # Log the error but don't fail the toggle - config is already saved - import logging - logging.warning(f"Lifecycle method error for {plugin_id}: {lifecycle_error}", exc_info=True) + logger.warning("Lifecycle method error for %s: %s", plugin_id, lifecycle_error, exc_info=True) return success_response( message=f"Plugin {plugin_id} {'enabled' if enabled else 'disabled'} successfully" @@ -748,25 +738,16 @@ def get_plugin_config(): plugin_config, schema_mgr.load_schema(plugin_id, use_cache=True), defaults) except Exception as e: # Log but don't fail - defaults merge is best effort - import logging - logging.warning(f"Could not merge defaults for {plugin_id}: {e}") + logger.warning("Could not merge defaults for %s: %s", plugin_id, e) # Special handling for of-the-day plugin: populate uploaded_files and categories from disk if plugin_id == 'of-the-day' or plugin_id == 'ledmatrix-of-the-day': - # Get plugin directory - plugin_id in manifest is 'of-the-day', but directory is 'ledmatrix-of-the-day' - plugin_dir_name = 'ledmatrix-of-the-day' - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_dir_name) - # If not found, try with the plugin_id - if not plugin_dir or not Path(plugin_dir).exists(): - plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / plugin_dir_name - if not plugin_dir.exists(): - plugin_dir = PROJECT_ROOT / 'plugins' / plugin_id - - if plugin_dir and Path(plugin_dir).exists(): - data_dir = Path(plugin_dir) / 'of_the_day' + # The manifest id is 'of-the-day'; the directory is usually + # 'ledmatrix-of-the-day'. + plugin_dir = (_plugin_directory('ledmatrix-of-the-day') + or _plugin_directory(plugin_id)) + if plugin_dir: + data_dir = plugin_dir / 'of_the_day' if data_dir.exists(): # Scan for JSON files uploaded_files = [] @@ -934,7 +915,6 @@ def update_plugin(): if manifest_path.exists(): try: - import json with open(manifest_path, 'r', encoding='utf-8') as f: manifest = json.load(f) current_last_updated = manifest.get('last_updated') @@ -983,7 +963,6 @@ def update_plugin(): updated_version = current_version try: if manifest_path.exists(): - import json with open(manifest_path, 'r', encoding='utf-8') as f: manifest = json.load(f) updated_last_updated = manifest.get('last_updated', current_last_updated) @@ -1224,10 +1203,7 @@ def install_plugin(): return jsonify({'status': 'error', 'message': f"{plugin_id} is a {registry_entry.get('type')!r} entry, not a plugin"}), 400 - # Install the plugin - # Log the plugins directory being used for debugging plugins_dir = api_v3.plugin_store_manager.plugins_dir - branch_info = f" (branch: {branch})" if branch else "" logger.info("Installing plugin to directory: %s", plugins_dir) # Use operation queue if available @@ -1590,24 +1566,21 @@ def get_github_auth_status(): }) @api_v3.route('/plugins/store/refresh', methods=['POST']) def refresh_plugin_store(): - """Refresh plugin store repository""" + """Re-download the plugin registry, bypassing its cache. + + Takes no body. Answers ``{status, message, plugin_count}``, the count + being the registry's entries. Commit metadata is not refreshed here: the + store list fetches it per plugin when it is shown. + """ if not api_v3.plugin_store_manager: return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - data = request.get_json(silent=True) or {} - fetch_commit_info = data.get('fetch_commit_info', data.get('fetch_latest_versions', False)) - - # Force refresh the registry registry = api_v3.plugin_store_manager.fetch_registry(force_refresh=True) plugin_count = len(registry.get('plugins', [])) - message = 'Plugin store refreshed' - if fetch_commit_info: - message += ' (with refreshed commit metadata from GitHub)' - return jsonify({ 'status': 'success', - 'message': message, + 'message': 'Plugin store refreshed', 'plugin_count': plugin_count }) @api_v3.route('/plugins/config', methods=['POST']) @@ -1699,7 +1672,6 @@ def save_plugin_config(): # Process bracket notation fields and set directly in plugin_config # Use JSON encoding instead of comma-join to handle values containing commas - import json for base_path, values in bracket_array_fields.items(): # Get schema property to verify it's an array base_prop = _get_schema_property(schema, base_path) @@ -1797,254 +1769,11 @@ def save_plugin_config(): if parsed_value is not _SKIP_FIELD: _set_nested_value(plugin_config, key, parsed_value) - # Post-process: Fix array fields that might have been incorrectly structured - # This handles cases where array fields are stored as dicts (e.g., from indexed form fields) - def fix_array_structures(config_dict, schema_props, prefix=''): - """Recursively fix array structures (convert dicts with numeric keys to arrays, fix length issues)""" - for prop_key, prop_schema in schema_props.items(): - prop_type = prop_schema.get('type') - - if prop_type == 'array': - # Navigate to the field location - if prefix: - parent_parts = prefix.split('.') - parent = config_dict - for part in parent_parts: - if isinstance(parent, dict) and part in parent: - parent = parent[part] - else: - parent = None - break - - if parent is not None and isinstance(parent, dict) and prop_key in parent: - current_value = parent[prop_key] - # If it's a dict with numeric string keys, convert to array - if isinstance(current_value, dict) and not isinstance(current_value, list): - try: - # Check if all keys are numeric strings (array indices) - keys = [k for k in current_value.keys()] - if all(k.isdigit() for k in keys): - # Convert to sorted array by index - sorted_keys = sorted(keys, key=int) - array_value = [current_value[k] for k in sorted_keys] - # Convert array elements to correct types based on schema - items_schema = prop_schema.get('items', {}) - item_type = items_schema.get('type') - if item_type in ('number', 'integer'): - converted_array = [] - for v in array_value: - if isinstance(v, str): - try: - if item_type == 'integer': - converted_array.append(int(v)) - else: - converted_array.append(float(v)) - except (ValueError, TypeError, OverflowError): - converted_array.append(v) - else: - converted_array.append(v) - array_value = converted_array - parent[prop_key] = array_value - current_value = array_value # Update for length check below - except (ValueError, KeyError, TypeError): - # Conversion failed, check if we should use default - pass - - # If it's an array, ensure correct types and check minItems - if isinstance(current_value, list): - # First, ensure array elements are correct types - items_schema = prop_schema.get('items', {}) - item_type = items_schema.get('type') - if item_type in ('number', 'integer'): - converted_array = [] - for v in current_value: - if isinstance(v, str): - try: - if item_type == 'integer': - converted_array.append(int(v)) - else: - converted_array.append(float(v)) - except (ValueError, TypeError, OverflowError): - converted_array.append(v) - else: - converted_array.append(v) - parent[prop_key] = converted_array - current_value = converted_array - - # Then check minItems - min_items = prop_schema.get('minItems') - if min_items is not None and len(current_value) < min_items: - # Use default if available, otherwise keep as-is (validation will catch it) - default = prop_schema.get('default') - if default and isinstance(default, list) and len(default) >= min_items: - parent[prop_key] = default - else: - # Top-level field - if prop_key in config_dict: - current_value = config_dict[prop_key] - # If it's a dict with numeric string keys, convert to array - if isinstance(current_value, dict) and not isinstance(current_value, list): - try: - keys = list(current_value.keys()) - if keys and all(str(k).isdigit() for k in keys): - sorted_keys = sorted(keys, key=lambda x: int(str(x))) - array_value = [current_value[k] for k in sorted_keys] - # Convert array elements to correct types based on schema - items_schema = prop_schema.get('items', {}) - item_type = items_schema.get('type') - if item_type in ('number', 'integer'): - converted_array = [] - for v in array_value: - if isinstance(v, str): - try: - if item_type == 'integer': - converted_array.append(int(v)) - else: - converted_array.append(float(v)) - except (ValueError, TypeError, OverflowError): - converted_array.append(v) - else: - converted_array.append(v) - array_value = converted_array - config_dict[prop_key] = array_value - current_value = array_value # Update for length check below - except (ValueError, KeyError, TypeError) as e: - logger.debug(f"Failed to convert {prop_key} to array: {e}") - - # If it's an array, ensure correct types and check minItems - if isinstance(current_value, list): - # First, ensure array elements are correct types - items_schema = prop_schema.get('items', {}) - item_type = items_schema.get('type') - if item_type in ('number', 'integer'): - converted_array = [] - for v in current_value: - if isinstance(v, str): - try: - if item_type == 'integer': - converted_array.append(int(v)) - else: - converted_array.append(float(v)) - except (ValueError, TypeError, OverflowError): - converted_array.append(v) - else: - converted_array.append(v) - config_dict[prop_key] = converted_array - current_value = converted_array - - # Then check minItems - min_items = prop_schema.get('minItems') - if min_items is not None and len(current_value) < min_items: - default = prop_schema.get('default') - if default and isinstance(default, list) and len(default) >= min_items: - config_dict[prop_key] = default - - # Recurse into nested objects - elif prop_type == 'object' and 'properties' in prop_schema: - nested_prefix = f"{prefix}.{prop_key}" if prefix else prop_key - if prefix: - parent_parts = prefix.split('.') - parent = config_dict - for part in parent_parts: - if isinstance(parent, dict) and part in parent: - parent = parent[part] - else: - parent = None - break - nested_dict = parent.get(prop_key) if parent is not None and isinstance(parent, dict) else None - else: - nested_dict = config_dict.get(prop_key) - - if isinstance(nested_dict, dict): - # Pass no prefix: config_dict is already the navigated sub-dict, - # so path segments from the parent would mis-navigate it. - fix_array_structures(nested_dict, prop_schema['properties']) - - # Also ensure array fields that are None get converted to empty arrays - def ensure_array_defaults(config_dict, schema_props, prefix=''): - """Recursively ensure array fields have defaults if None""" - for prop_key, prop_schema in schema_props.items(): - prop_type = prop_schema.get('type') - - if prop_type == 'array': - if prefix: - parent_parts = prefix.split('.') - parent = config_dict - for part in parent_parts: - if isinstance(parent, dict) and part in parent: - parent = parent[part] - else: - parent = None - break - - if parent is not None and isinstance(parent, dict): - if prop_key not in parent or parent[prop_key] is None: - default = prop_schema.get('default', []) - parent[prop_key] = default if default else [] - else: - if prop_key not in config_dict or config_dict[prop_key] is None: - default = prop_schema.get('default', []) - config_dict[prop_key] = default if default else [] - - elif prop_type == 'object' and 'properties' in prop_schema: - nested_prefix = f"{prefix}.{prop_key}" if prefix else prop_key - if prefix: - parent_parts = prefix.split('.') - parent = config_dict - for part in parent_parts: - if isinstance(parent, dict) and part in parent: - parent = parent[part] - else: - parent = None - break - nested_dict = parent.get(prop_key) if parent is not None and isinstance(parent, dict) else None - else: - nested_dict = config_dict.get(prop_key) - - if nested_dict is None: - if prefix: - parent_parts = prefix.split('.') - parent = config_dict - for part in parent_parts: - if part not in parent: - parent[part] = {} - parent = parent[part] - if prop_key not in parent: - parent[prop_key] = {} - nested_dict = parent[prop_key] - else: - if prop_key not in config_dict: - config_dict[prop_key] = {} - nested_dict = config_dict[prop_key] - - if isinstance(nested_dict, dict): - # Pass no prefix: config_dict is already navigated. - ensure_array_defaults(nested_dict, prop_schema['properties']) - + # Before the booleans below: that walk replaces anything it + # expects to be a list and finds is not one. if schema and 'properties' in schema: - # First, fix any dict structures that should be arrays - # This must be called BEFORE validation to convert dicts with numeric keys to arrays - fix_array_structures(plugin_config, schema['properties']) - # Then, ensure None arrays get defaults - ensure_array_defaults(plugin_config, schema['properties']) - - # Debug: Log the structure after fixing - if 'feeds' in plugin_config and 'custom_feeds' in plugin_config.get('feeds', {}): - custom_feeds = plugin_config['feeds']['custom_feeds'] - logger.debug(f"After fix_array_structures: custom_feeds type={type(custom_feeds)}, value={custom_feeds}") - - # Force fix for feeds.custom_feeds if it's still a dict (fallback) - if 'feeds' in plugin_config: - feeds_config = plugin_config.get('feeds') or {} - if feeds_config and 'custom_feeds' in feeds_config and isinstance(feeds_config['custom_feeds'], dict): - custom_feeds_dict = feeds_config['custom_feeds'] - # Check if all keys are numeric - keys = list(custom_feeds_dict.keys()) - if keys and all(str(k).isdigit() for k in keys): - # Convert to array - sorted_keys = sorted(keys, key=lambda x: int(str(x))) - feeds_config['custom_feeds'] = [custom_feeds_dict[k] for k in sorted_keys] - logger.info(f"Force-converted feeds.custom_feeds from dict to array: {len(feeds_config['custom_feeds'])} items") + coerce_array_shapes(plugin_config, schema['properties'], + short_lists_take_default=True) # Fix unchecked boolean checkboxes: HTML checkboxes don't submit values # when unchecked, so the existing config value (potentially True) persists. @@ -2104,8 +1833,6 @@ def save_plugin_config(): try: api_v3.config_manager.save_raw_file_content('secrets', current_secrets) except PermissionError as e: - # Log the error with more details - import os secrets_path = api_v3.config_manager.secrets_path secrets_dir = os.path.dirname(secrets_path) if secrets_path else None @@ -2126,12 +1853,9 @@ def save_plugin_config(): f"Failed to save secrets configuration: Permission denied. Check file permissions on {secrets_path}", status_code=500 ) - except Exception as e: - # Log the error but don't fail the entire config save - import os + except Exception: secrets_path = api_v3.config_manager.secrets_path logger.error("Error saving secrets config for %s (path=%s)", plugin_id, secrets_path, exc_info=True) - # Return error response with more context return error_response( ErrorCode.CONFIG_SAVE_FAILED, "Failed to save secrets configuration; see logs for details", @@ -2177,8 +1901,7 @@ def save_plugin_config(): plugin_instance.on_disable() except Exception as lifecycle_error: # Log the error but don't fail the save - config is already saved - import logging - logging.warning(f"Lifecycle method error for {plugin_id}: {lifecycle_error}", exc_info=True) + logger.warning("Lifecycle method error for %s: %s", plugin_id, lifecycle_error, exc_info=True) except Exception as hook_err: # Do not fail the save if hook fails; just log logger.warning("on_config_change failed: %s", hook_err) @@ -2226,48 +1949,9 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr Returns ``(regular_config, secrets_config, None)``, or ``(None, None, error_response)`` when validation fails. """ - # JSON path: fix numeric-keyed dicts that should be arrays. - # JS dotToNested() converts feeds.custom_feeds.0.name → {'0': {name:...}} - # instead of [{name:...}]. The form-data path has fix_array_structures for this; - # mirror that logic here for JSON submissions. + # The form path has already done this, before its checkbox pass. if is_json and schema and 'properties' in schema: - def _fix_json_arrays(cfg, props): - for k, ps in props.items(): - if not isinstance(cfg, dict) or k not in cfg: - continue - pt = ps.get('type') - val = cfg[k] - if pt == 'array': - items_schema = ps.get('items', {}) - item_type = items_schema.get('type') - if isinstance(val, dict): - keys = list(val.keys()) - if keys and all(str(x).isdigit() for x in keys): - sorted_keys = sorted(keys, key=lambda x: int(str(x))) - arr = [val[sk] for sk in sorted_keys] - if item_type in ('integer', 'number'): - converted = [] - for v in arr: - if isinstance(v, str): - try: - converted.append(int(v) if item_type == 'integer' else float(v)) - except (ValueError, TypeError, OverflowError): - converted.append(v) - else: - converted.append(v) - arr = converted - cfg[k] = arr - elif not keys: - cfg[k] = [] - # Recurse into each element when items are objects with properties, - # covering both freshly-converted and already-list values. - if item_type == 'object' and 'properties' in items_schema: - for elem in (cfg[k] if isinstance(cfg[k], list) else []): - if isinstance(elem, dict): - _fix_json_arrays(elem, items_schema['properties']) - elif pt == 'object' and 'properties' in ps and isinstance(val, dict): - _fix_json_arrays(val, ps['properties']) - _fix_json_arrays(plugin_config, schema['properties']) + coerce_array_shapes(plugin_config, schema['properties']) # PRE-PROCESSING: Preserve 'enabled' state if not in request # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle @@ -2276,7 +1960,6 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr current_config = api_v3.config_manager.load_config() if plugin_id in current_config and 'enabled' in current_config[plugin_id]: plugin_config['enabled'] = current_config[plugin_id]['enabled'] - # logger.debug(f"Preserving enabled state for {plugin_id}: {plugin_config['enabled']}") elif api_v3.plugin_manager: # Fallback to plugin instance if config doesn't have it plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) @@ -2311,9 +1994,8 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) plugin_config = prepare_plugin_config(plugin_config, schema, defaults) - # After merging defaults, replace any None array values with their schema defaults. - # merge_with_defaults gives user config higher priority, so a None submitted by - # the client can survive the merge — this pass cleans those up. + # The defaults merge replaces a None only where the schema has a default, + # so an array the client sent as None, or left out, can still be one here. def _fix_none_arrays(cfg, props): for k, pschema in props.items(): if pschema.get('type') == 'array': @@ -2371,14 +2053,8 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr # Check integer first (more specific than number) if 'integer' in prop_type: if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '': - # Empty string with null allowed - already handled above, but double-check - if 'null' in prop_type: - normalized[key] = None - continue try: - normalized[key] = int(value_stripped) + normalized[key] = int(value.strip()) continue except (ValueError, TypeError, OverflowError): pass @@ -2389,14 +2065,8 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr # Check number (less specific, but handles floats) if 'number' in prop_type: if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '': - # Empty string with null allowed - already handled above, but double-check - if 'null' in prop_type: - normalized[key] = None - continue try: - normalized[key] = float(value_stripped) + normalized[key] = float(value.strip()) continue except (ValueError, TypeError, OverflowError): pass @@ -2410,21 +2080,7 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr normalized[key] = value.strip().lower() in ('true', '1', 'on', 'yes') continue - # If no conversion worked and null is allowed, try to set to None - # This handles cases where the value is an empty string or can't be converted - if 'null' in prop_type: - if isinstance(value, str): - value_stripped = value.strip() - if value_stripped == '' or value_stripped.lower() in ('null', 'none', 'undefined'): - normalized[key] = None - continue - # If it's already None, keep it - if value is None: - normalized[key] = None - continue - - # If no conversion worked, keep original value (will fail validation, but that's expected) - # Log a warning for debugging + # Nothing converted: keep the value for validation to report. logger.warning(f"Could not normalize field {field_path}: value={repr(value)}, type={type(value)}, schema_type={prop_type}") normalized[key] = value continue @@ -2550,37 +2206,24 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr enhanced_schema_for_filtering = _enhance_schema_with_core_properties(schema) plugin_config = _filter_config_by_schema(plugin_config, enhanced_schema_for_filtering) - # Debug logging for union type fields (temporary) - if 'rotation_settings' in plugin_config and 'random_seed' in plugin_config.get('rotation_settings', {}): - seed_value = plugin_config['rotation_settings']['random_seed'] - logger.debug(f"After normalization, random_seed value: {repr(seed_value)}, type: {type(seed_value)}") - - # Validate configuration against schema before saving + # A uniqueItems array can arrive with a repeat -- the form merges onto + # the stored list, so a stock symbol already saved and submitted again + # appears twice -- and validation would refuse the whole save for it. if schema: - # Log what we're validating for debugging - logger.info(f"Validating config for {plugin_id}") - # Only the shape. plugin_config still holds the submitted secret - # values at this point -- separate_secrets does not run until - # below -- so logging it wrote live credentials to the journal. - logger.info(f"Config keys being validated: {list(plugin_config.keys())}") - - # Schema keys including the injected core properties, for the error - enhanced_schema = _enhance_schema_with_core_properties(schema) + dedup_unique_arrays(plugin_config, schema) + if schema: is_valid, validation_errors = schema_mgr.validate_config_against_schema( plugin_config, schema, plugin_id ) if not is_valid: - # Log validation errors for debugging - logger.error(f"Config validation failed for {plugin_id}") - logger.error(f"Validation errors: {validation_errors}") - # Keys only, for the same reason as above. - logger.error(f"Config keys that failed: {list(plugin_config.keys())}") - logger.error(f"Schema properties: {list(enhanced_schema.get('properties', {}).keys())}") - - # Also print to console for immediate visibility - logger.warning("Config validation failed for plugin (see debug logs)") - + # Schema keys including the injected core properties, for the error + enhanced_schema = _enhance_schema_with_core_properties(schema) + # Keys, never values: plugin_config still holds the submitted + # secrets here (separate_secrets runs below), and logging it wrote + # live credentials to the journal. + logger.warning("Config validation failed for %s: %s (config keys: %s)", + plugin_id, validation_errors, list(plugin_config.keys())) return None, None, error_response( ErrorCode.CONFIG_VALIDATION_FAILED, 'Configuration validation failed', @@ -2704,8 +2347,13 @@ def reset_plugin_config(): # Replace all secrets with defaults current_secrets[plugin_id] = default_secrets - # Save updated configs - api_v3.config_manager.save_config(current_config) + success, error_msg = _pkg._save_config_atomic(api_v3.config_manager, current_config, create_backup=True) + if not success: + return error_response( + ErrorCode.CONFIG_SAVE_FAILED, + f"Failed to save configuration: {error_msg}", + status_code=500 + ) if default_secrets or not preserve_secrets: api_v3.config_manager.save_raw_file_content('secrets', current_secrets) @@ -2715,7 +2363,8 @@ def reset_plugin_config(): plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) if plugin_instance: merged_config = api_v3.config_manager.load_config() - plugin_full_config = merged_config.get(plugin_id, {}) + plugin_full_config = _pkg._prepared_plugin_config( + plugin_id, merged_config.get(plugin_id, {})) if hasattr(plugin_instance, 'on_config_change'): plugin_instance.on_config_change(plugin_full_config) except Exception as hook_err: @@ -2761,13 +2410,8 @@ def execute_plugin_action(): if plugin_id is None: return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 - # Get plugin directory - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / plugin_id - - if not plugin_dir or not Path(plugin_dir).exists(): + plugin_dir = _plugin_directory(plugin_id) + if not plugin_dir: return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 # Load manifest to get action definition @@ -2989,6 +2633,10 @@ sys.exit(proc.returncode) 'message': 'Could not generate authorization URL' }), 400 except Exception as e: + # Not a copy of the blueprint handler: without it, a + # TimeoutExpired from the plugin's script would reach + # this route's own `except subprocess.TimeoutExpired` + # and be answered as a 408 "Action timed out". logger.error("Error executing action step 1", exc_info=True) return jsonify({ 'status': 'error', @@ -3121,7 +2769,7 @@ def upload_plugin_asset(): if total_size + file_size > max_total_size: return jsonify({ 'status': 'error', - 'message': f'Upload would exceed 50MB total storage limit' + 'message': 'Upload would exceed 50MB total storage limit' }), 400 # Validate file is actually an image (check magic bytes) @@ -3222,13 +2870,8 @@ def serve_plugin_static(plugin_id, file_path): if not safe_parts: return jsonify({'status': 'error', 'message': 'Invalid file path'}), 400 - # Get plugin directory - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(safe_plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / safe_plugin_id - - if not plugin_dir or not Path(plugin_dir).exists(): + plugin_dir = _plugin_directory(safe_plugin_id) + if not plugin_dir: return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 # Containment is still checked after resolving: name validation cannot @@ -3300,14 +2943,8 @@ def upload_calendar_credentials(): 'message': 'File does not appear to be a valid Google OAuth credentials file' }), 400 - # Get plugin directory - plugin_id = 'calendar' - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / plugin_id - - if not plugin_dir or not Path(plugin_dir).exists(): + plugin_dir = _plugin_directory('calendar') + if not plugin_dir: return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 # Save file to plugin directory @@ -3316,7 +2953,6 @@ def upload_calendar_credentials(): # Backup existing file if it exists if credentials_path.exists(): backup_path = Path(plugin_dir) / f'credentials.json.backup.{int(_pkg.time.time())}' - import shutil shutil.copy2(credentials_path, backup_path) _prune_credential_backups(Path(plugin_dir)) diff --git a/web_interface/blueprints/api_v3/starlark.py b/web_interface/blueprints/api_v3/starlark.py index 08d592c7..cdc21fa4 100644 --- a/web_interface/blueprints/api_v3/starlark.py +++ b/web_interface/blueprints/api_v3/starlark.py @@ -1,7 +1,7 @@ """Starlark / Tronbyte app management routes. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ import signal import threading @@ -333,7 +333,6 @@ def uninstall_starlark_app(app_id): else: # Standalone: remove app dir and manifest entry. app_dir is the # path _validate_starlark_app_path checked, not a fresh join. - import shutil if app_dir.exists(): shutil.rmtree(app_dir) with _starlark_manifest_lock(): @@ -716,7 +715,6 @@ def install_from_tronbyte_repository(): success = _install_star_file(app_id, temp_path, install_metadata, assets_dir=temp_assets_dir) finally: # Clean up temp assets directory - import shutil try: shutil.rmtree(temp_assets_dir) except OSError: diff --git a/web_interface/blueprints/api_v3/system.py b/web_interface/blueprints/api_v3/system.py index 405c425f..ab3403a2 100644 --- a/web_interface/blueprints/api_v3/system.py +++ b/web_interface/blueprints/api_v3/system.py @@ -1,7 +1,7 @@ """Service control, updates, versions and system status. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( Any, Dict, PROJECT_ROOT, Path, _GIT, _UPDATE_CHECK_TTL, @@ -14,6 +14,8 @@ from web_interface.blueprints.api_v3 import ( ) import threading +from web_interface.cache import get_cached, set_cached +from web_interface.system_metrics import collect_system_metrics, format_uptime import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. @@ -23,91 +25,25 @@ import web_interface.blueprints.api_v3 as _pkg @api_v3.route('/system/status', methods=['GET']) def get_system_status(): - """Get system status""" - # Check cache first (10 second TTL for system status) - try: - from web_interface.cache import get_cached, set_cached - cached_result = get_cached('system_status', ttl_seconds=10) - if cached_result is not None: - return jsonify({'status': 'success', 'data': cached_result}) - except ImportError: - # Cache not available, continue without caching - get_cached = None - set_cached = None + """CPU, memory, disk, temperature and uptime, plus the display service state. - # Import psutil for system monitoring - try: - import psutil - except ImportError: - # Fallback if psutil not available - return jsonify({ - 'status': 'error', - 'message': 'psutil not available for system monitoring' - }), 503 + ``data`` carries every key of system_metrics.collect_system_metrics() -- + the numbers the live status stream sends -- and ``timestamp``, ``uptime`` + (formatted) and ``service_active``. A metric that cannot be read is null. + Cached for 10 seconds. + """ + cached_result = get_cached('system_status', ttl_seconds=10) + if cached_result is not None: + return jsonify({'status': 'success', 'data': cached_result}) - # Get system metrics using psutil - cpu_percent = psutil.cpu_percent(interval=0.1) # Short interval for responsiveness - memory = psutil.virtual_memory() - memory_percent = memory.percent - disk = psutil.disk_usage('/') - disk_percent = disk.percent - - # Calculate uptime - boot_time = psutil.boot_time() - uptime_seconds = _pkg.time.time() - boot_time - uptime_hours = uptime_seconds / 3600 - uptime_days = uptime_hours / 24 - - # Format uptime string - if uptime_days >= 1: - uptime_str = f"{int(uptime_days)}d {int(uptime_hours % 24)}h" - elif uptime_hours >= 1: - uptime_str = f"{int(uptime_hours)}h {int((uptime_seconds % 3600) / 60)}m" - else: - uptime_str = f"{int(uptime_seconds / 60)}m" - - # Get CPU temperature (Raspberry Pi) - cpu_temp = None - try: - temp_file = '/sys/class/thermal/thermal_zone0/temp' - if os.path.exists(temp_file): - with open(temp_file, 'r') as f: - temp_millidegrees = int(f.read().strip()) - cpu_temp = temp_millidegrees / 1000.0 # Convert to Celsius - except (IOError, ValueError, OSError): - # Temperature sensor not available or error reading - cpu_temp = None - - # Get display service status - service_status = _get_display_service_status() - - status = { - 'timestamp': _pkg.time.time(), - 'uptime': uptime_str, - 'uptime_seconds': int(uptime_seconds), - 'service_active': service_status.get('active', False), - 'cpu_percent': round(cpu_percent, 1), - 'memory_used_percent': round(memory_percent, 1), - 'memory_total_mb': round(memory.total / (1024 * 1024), 1), - 'memory_used_mb': round(memory.used / (1024 * 1024), 1), - # MemAvailable, not total-minus-used: it accounts for reclaimable - # page cache, so it is what actually predicts memory trouble. A - # board can read 70% "used" and be fine, or read the same and be - # about to fail fork(), and only this number tells them apart. - 'memory_available_mb': round(memory.available / (1024 * 1024), 1), - 'cpu_temp': round(cpu_temp, 1) if cpu_temp is not None else None, - 'disk_used_percent': round(disk_percent, 1), - 'disk_total_gb': round(disk.total / (1024 * 1024 * 1024), 1), - 'disk_used_gb': round(disk.used / (1024 * 1024 * 1024), 1) - } - - # Cache the result if available - if set_cached: - try: - set_cached('system_status', status, ttl_seconds=10) - except Exception: - pass # Cache write failed, but continue + # A short blocking sample: this may be the first cpu_percent call in the + # process, and a non-blocking first call has nothing to measure against. + status = collect_system_metrics(cpu_interval=0.1) + status['timestamp'] = _pkg.time.time() + status['uptime'] = format_uptime(status['uptime_seconds']) + status['service_active'] = _get_display_service_status().get('active', False) + set_cached('system_status', status, ttl_seconds=10) return jsonify({'status': 'success', 'data': status}) @api_v3.route('/system/version', methods=['GET']) def get_system_version(): diff --git a/web_interface/blueprints/api_v3/wifi.py b/web_interface/blueprints/api_v3/wifi.py index 7700ea82..f504a8a8 100644 --- a/web_interface/blueprints/api_v3/wifi.py +++ b/web_interface/blueprints/api_v3/wifi.py @@ -1,7 +1,7 @@ """Wi-Fi scanning, connection and status routes. -Routes decorate the shared `api_v3` Blueprint from ._common, so their -endpoint names are unchanged by living here. +Routes decorate the shared `api_v3` Blueprint from the package `__init__`, +so their endpoint names are unchanged by living here. """ import threading import time diff --git a/web_interface/blueprints/pages_v3.py b/web_interface/blueprints/pages_v3.py index e2b82c3d..3f55bc4c 100644 --- a/web_interface/blueprints/pages_v3.py +++ b/web_interface/blueprints/pages_v3.py @@ -1,6 +1,5 @@ from flask import Blueprint, Response, render_template, jsonify, url_for from jinja2 import TemplateNotFound -from markupsafe import escape from html.parser import HTMLParser import json import logging @@ -163,41 +162,13 @@ def index(): @pages_v3.route('/partials/') def load_partial(partial_name): - """Load HTMX partials dynamically""" + """One tab's HTML for HTMX, by the names in _PARTIAL_LOADERS; 404 otherwise.""" + loader = _PARTIAL_LOADERS.get(partial_name) + if loader is None: + return "Partial not found", 404 try: - # Map partial names to specific data loading - if partial_name == 'overview': - return _load_overview_partial() - elif partial_name == 'general': - return _load_general_partial() - elif partial_name == 'display': - return _load_display_partial() - elif partial_name == 'durations': - return _load_durations_partial() - elif partial_name == 'schedule': - return _load_schedule_partial() - elif partial_name == 'plugins': - return _load_plugins_partial() - elif partial_name == 'fonts': - return _load_fonts_partial() - elif partial_name == 'logs': - return _load_logs_partial() - elif partial_name == 'raw-json': - return _load_raw_json_partial() - elif partial_name == 'backup-restore': - return _load_backup_restore_partial() - elif partial_name == 'wifi': - return _load_wifi_partial() - elif partial_name == 'cache': - return _load_cache_partial() - elif partial_name == 'operation-history': - return _load_operation_history_partial() - elif partial_name == 'tools': - return _load_tools_partial() - else: - return "Partial not found", 404 - - except Exception as e: + return loader() + except Exception: logger.error("Error loading partial %s", partial_name, exc_info=True) return "Error loading partial", 500 @@ -297,19 +268,7 @@ def serve_plugin_web_ui(plugin_id, filename): return 'Plugin manager not available', 503, {'Content-Type': 'text/plain'} try: - _plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve() - - _plugin_dir = resolve_under(_plugins_base, safe_id) - if _plugin_dir is None: - return 'Forbidden', 403, {'Content-Type': 'text/plain'} - - # Mirror PluginManager's ledmatrix- prefix fallback. - if not _plugin_dir.exists(): - _alt = resolve_under(_plugins_base, f'ledmatrix-{safe_id}') - if _alt is not None: - _plugin_dir = _alt - - web_ui_path = resolve_under(_plugin_dir / 'web_ui', safe_fn) + web_ui_path = resolve_under(_plugin_dir_for(safe_id) / 'web_ui', safe_fn) if web_ui_path is None: return 'Forbidden', 403, {'Content-Type': 'text/plain'} @@ -362,10 +321,11 @@ def serve_plugin_web_ui(plugin_id, filename): def _plugin_dir_for(safe_id): - """Resolve a sanitised plugin id to its directory, or None. + """A sanitised plugin id's directory, which may not exist. - Mirrors serve_plugin_web_ui: containment-guarded against the configured - plugins directory, with PluginManager's ``ledmatrix-`` prefix fallback. + Contained under the configured plugins directory, with PluginManager's + ``ledmatrix-`` prefix fallback. Raises ValueError for an id that would + leave it; the routes answer that with a 403. """ plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve() plugin_dir = resolve_under(plugins_base, safe_id) @@ -479,45 +439,33 @@ def serve_plugin_widget(plugin_id, widget_name): def _load_overview_partial(): """Load overview partial with system stats""" - try: - if pages_v3.config_manager: - main_config = pages_v3.config_manager.load_config() - # This would be populated with real system stats via SSE - return render_template('v3/partials/overview.html', - main_config=main_config) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + if pages_v3.config_manager: + main_config = pages_v3.config_manager.load_config() + # This would be populated with real system stats via SSE + return render_template('v3/partials/overview.html', + main_config=main_config) def _load_general_partial(): """Load general settings partial""" - try: - if pages_v3.config_manager: - main_config = pages_v3.config_manager.load_config() - try: - from web_interface.auto_update import describe_status - auto_update_status = describe_status(main_config) - except Exception: - logger.debug("Could not read auto-update status", exc_info=True) - auto_update_status = None - return render_template('v3/partials/general.html', - main_config=main_config, - auto_update_status=auto_update_status) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + if pages_v3.config_manager: + main_config = pages_v3.config_manager.load_config() + try: + from web_interface.auto_update import describe_status + auto_update_status = describe_status(main_config) + except Exception: + logger.debug("Could not read auto-update status", exc_info=True) + auto_update_status = None + return render_template('v3/partials/general.html', + main_config=main_config, + auto_update_status=auto_update_status) def _load_display_partial(): """Load display settings partial""" - try: - if pages_v3.config_manager: - main_config = pages_v3.config_manager.load_config() - return render_template('v3/partials/display.html', - main_config=main_config, - is_pi5=is_raspberry_pi_5()) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + if pages_v3.config_manager: + main_config = pages_v3.config_manager.load_config() + return render_template('v3/partials/display.html', + main_config=main_config, + is_pi5=is_raspberry_pi_5()) def _plugin_default_duration(plugin_id, plugin_config): """Seconds a plugin shows each screen when the Rotation page sets none. @@ -553,199 +501,168 @@ def _load_durations_partial(): value overrides the plugin (see DisplayController._get_display_duration). Pre-filling every mode would pin them all on the first save. """ - try: - if pages_v3.config_manager: - main_config = pages_v3.config_manager.load_config() - duration_groups = [] - covered_keys = set() - if pages_v3.plugin_manager: - try: - pages_v3.plugin_manager.discover_plugins() - saved = (main_config.get('display', {}) or {}).get('display_durations', {}) or {} - infos = sorted(pages_v3.plugin_manager.get_all_plugin_info(), - key=lambda i: (i.get('name') or i.get('id') or '').lower()) - for info in infos: - pid = info.get('id') - if not pid or not (main_config.get(pid, {}) or {}).get('enabled', False): - continue - modes = pages_v3.plugin_manager.get_plugin_display_modes(pid) or [pid] - covered_keys.update(modes) - default = _plugin_default_duration(pid, main_config.get(pid, {}) or {}) - duration_groups.append({ - 'plugin_id': pid, - 'plugin_name': info.get('name') or pid, - 'modes': [{'key': m, 'value': saved.get(m, ''), 'default': default} - for m in modes], - }) - # Saved keys not owned by any enabled plugin (disabled or - # uninstalled plugins) stay visible rather than vanishing. - leftovers = [{'key': k, 'value': v} for k, v in saved.items() - if k not in covered_keys] - if leftovers: - duration_groups.append({ - 'plugin_id': '', - 'plugin_name': 'Other saved entries', - 'modes': leftovers, - }) - except Exception: - logger.warning("durations: could not enumerate plugin modes", exc_info=True) - return render_template('v3/partials/durations.html', - main_config=main_config, - duration_groups=duration_groups) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + if pages_v3.config_manager: + main_config = pages_v3.config_manager.load_config() + duration_groups = [] + covered_keys = set() + if pages_v3.plugin_manager: + try: + pages_v3.plugin_manager.discover_plugins() + saved = (main_config.get('display', {}) or {}).get('display_durations', {}) or {} + infos = sorted(pages_v3.plugin_manager.get_all_plugin_info(), + key=lambda i: (i.get('name') or i.get('id') or '').lower()) + for info in infos: + pid = info.get('id') + if not pid or not (main_config.get(pid, {}) or {}).get('enabled', False): + continue + modes = pages_v3.plugin_manager.get_plugin_display_modes(pid) or [pid] + covered_keys.update(modes) + default = _plugin_default_duration(pid, main_config.get(pid, {}) or {}) + duration_groups.append({ + 'plugin_id': pid, + 'plugin_name': info.get('name') or pid, + 'modes': [{'key': m, 'value': saved.get(m, ''), 'default': default} + for m in modes], + }) + # Saved keys not owned by any enabled plugin (disabled or + # uninstalled plugins) stay visible rather than vanishing. + leftovers = [{'key': k, 'value': v} for k, v in saved.items() + if k not in covered_keys] + if leftovers: + duration_groups.append({ + 'plugin_id': '', + 'plugin_name': 'Other saved entries', + 'modes': leftovers, + }) + except Exception: + logger.warning("durations: could not enumerate plugin modes", exc_info=True) + return render_template('v3/partials/durations.html', + main_config=main_config, + duration_groups=duration_groups) def _load_schedule_partial(): """Load schedule settings partial""" - try: - if pages_v3.config_manager: - main_config = pages_v3.config_manager.load_config() - schedule_config = main_config.get('schedule', {}) - dim_schedule_config = main_config.get('dim_schedule', {}) - # Get normal brightness for display in dim schedule UI - normal_brightness = main_config.get('display', {}).get('hardware', {}).get('brightness', 90) - return render_template('v3/partials/schedule.html', - schedule_config=schedule_config, - dim_schedule_config=dim_schedule_config, - normal_brightness=normal_brightness) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + if pages_v3.config_manager: + main_config = pages_v3.config_manager.load_config() + schedule_config = main_config.get('schedule', {}) + dim_schedule_config = main_config.get('dim_schedule', {}) + # Get normal brightness for display in dim schedule UI + normal_brightness = main_config.get('display', {}).get('hardware', {}).get('brightness', 90) + return render_template('v3/partials/schedule.html', + schedule_config=schedule_config, + dim_schedule_config=dim_schedule_config, + normal_brightness=normal_brightness) def _load_plugins_partial(): """Load plugins management partial""" - try: - import json - from pathlib import Path - - # Load plugin data from the plugin system - plugins_data = [] + # Load plugin data from the plugin system + plugins_data = [] - # Get installed plugins if managers are available - if pages_v3.plugin_manager and pages_v3.plugin_store_manager: - try: - # Get all installed plugin info - all_plugin_info = pages_v3.plugin_manager.get_all_plugin_info() + # Get installed plugins if managers are available + if pages_v3.plugin_manager and pages_v3.plugin_store_manager: + try: + # Get all installed plugin info + all_plugin_info = pages_v3.plugin_manager.get_all_plugin_info() - # Load config once before the loop (not per-plugin) - full_config = pages_v3.config_manager.load_config() if pages_v3.config_manager else {} + # Load config once before the loop (not per-plugin) + full_config = pages_v3.config_manager.load_config() if pages_v3.config_manager else {} - # Format for the web interface - for plugin_info in all_plugin_info: - plugin_id = plugin_info.get('id') + # Format for the web interface + for plugin_info in all_plugin_info: + plugin_id = plugin_info.get('id') - # Re-read manifest from disk to ensure we have the latest metadata - manifest_path = Path(pages_v3.plugin_manager.plugins_dir) / plugin_id / "manifest.json" - if manifest_path.exists(): - try: - with open(manifest_path, 'r', encoding='utf-8') as f: - fresh_manifest = json.load(f) - # Update plugin_info with fresh manifest data - plugin_info.update(fresh_manifest) - except Exception as e: - # If we can't read the fresh manifest, use the cached one - logger.warning("Could not read fresh manifest for plugin: %s", plugin_id) + # Re-read manifest from disk to ensure we have the latest metadata + manifest_path = Path(pages_v3.plugin_manager.plugins_dir) / plugin_id / "manifest.json" + if manifest_path.exists(): + try: + with open(manifest_path, 'r', encoding='utf-8') as f: + fresh_manifest = json.load(f) + # Update plugin_info with fresh manifest data + plugin_info.update(fresh_manifest) + except Exception: + # If we can't read the fresh manifest, use the cached one + logger.warning("Could not read fresh manifest for plugin: %s", plugin_id) - # Get enabled status from config (source of truth) - # Read from config file first, fall back to plugin instance if config doesn't have the key - enabled = None - if pages_v3.config_manager: - plugin_config = full_config.get(plugin_id, {}) - # Check if 'enabled' key exists in config (even if False) - if 'enabled' in plugin_config: - enabled = bool(plugin_config['enabled']) - - # Fallback to plugin instance if config doesn't have enabled key - if enabled is None: - plugin_instance = pages_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - enabled = plugin_instance.enabled - else: - # Default to True if no config key and plugin not loaded (matches BasePlugin default) - enabled = True + # Get enabled status from config (source of truth) + # Read from config file first, fall back to plugin instance if config doesn't have the key + enabled = None + if pages_v3.config_manager: + plugin_config = full_config.get(plugin_id, {}) + # Check if 'enabled' key exists in config (even if False) + if 'enabled' in plugin_config: + enabled = bool(plugin_config['enabled']) + + # Fallback to plugin instance if config doesn't have enabled key + if enabled is None: + plugin_instance = pages_v3.plugin_manager.get_plugin(plugin_id) + if plugin_instance: + enabled = plugin_instance.enabled + else: + # Default to True if no config key and plugin not loaded (matches BasePlugin default) + enabled = True - # Get verified status from store registry (no GitHub API calls needed) - store_info = pages_v3.plugin_store_manager.get_registry_info(plugin_id) - verified = store_info.get('verified', False) if store_info else False + # Get verified status from store registry (no GitHub API calls needed) + store_info = pages_v3.plugin_store_manager.get_registry_info(plugin_id) + verified = store_info.get('verified', False) if store_info else False - last_updated = plugin_info.get('last_updated') - last_commit = plugin_info.get('last_commit') or plugin_info.get('last_commit_sha') - branch = plugin_info.get('branch') + last_updated = plugin_info.get('last_updated') + last_commit = plugin_info.get('last_commit') or plugin_info.get('last_commit_sha') + branch = plugin_info.get('branch') - if store_info: - last_updated = last_updated or store_info.get('last_updated') or store_info.get('last_updated_iso') - last_commit = last_commit or store_info.get('last_commit') or store_info.get('last_commit_sha') - branch = branch or store_info.get('branch') or store_info.get('default_branch') + if store_info: + last_updated = last_updated or store_info.get('last_updated') or store_info.get('last_updated_iso') + last_commit = last_commit or store_info.get('last_commit') or store_info.get('last_commit_sha') + branch = branch or store_info.get('branch') or store_info.get('default_branch') - plugins_data.append({ - 'id': plugin_id, - 'name': plugin_info.get('name', plugin_id), - 'author': plugin_info.get('author', 'Unknown'), - 'category': plugin_info.get('category', 'General'), - 'description': plugin_info.get('description', 'No description available'), - 'tags': plugin_info.get('tags', []), - 'enabled': enabled, - 'verified': verified, - 'loaded': plugin_info.get('loaded', False), - 'last_updated': last_updated, - 'last_commit': last_commit, - 'branch': branch - }) - except Exception as e: - logger.error("Error loading plugin data", exc_info=True) + plugins_data.append({ + 'id': plugin_id, + 'name': plugin_info.get('name', plugin_id), + 'author': plugin_info.get('author', 'Unknown'), + 'category': plugin_info.get('category', 'General'), + 'description': plugin_info.get('description', 'No description available'), + 'tags': plugin_info.get('tags', []), + 'enabled': enabled, + 'verified': verified, + 'loaded': plugin_info.get('loaded', False), + 'last_updated': last_updated, + 'last_commit': last_commit, + 'branch': branch + }) + except Exception: + logger.error("Error loading plugin data", exc_info=True) - return render_template('v3/partials/plugins.html', - plugins=plugins_data) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/plugins.html', + plugins=plugins_data) def _load_fonts_partial(): """Load fonts management partial""" - try: - # This would load font data from the font system - fonts_data = {} # Placeholder for font data - return render_template('v3/partials/fonts.html', - fonts=fonts_data) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + # This would load font data from the font system + fonts_data = {} # Placeholder for font data + return render_template('v3/partials/fonts.html', + fonts=fonts_data) def _load_logs_partial(): """Load logs viewer partial""" - try: - return render_template('v3/partials/logs.html') - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/logs.html') def _load_raw_json_partial(): """Load raw JSON editor partial""" - try: - if pages_v3.config_manager: - main_config_data = pages_v3.config_manager.get_raw_file_content('main') - secrets_config_data = pages_v3.config_manager.get_raw_file_content('secrets') - main_config_json = json.dumps(main_config_data, indent=4) - secrets_config_json = json.dumps(secrets_config_data, indent=4) + if pages_v3.config_manager: + main_config_data = pages_v3.config_manager.get_raw_file_content('main') + secrets_config_data = pages_v3.config_manager.get_raw_file_content('secrets') + main_config_json = json.dumps(main_config_data, indent=4) + secrets_config_json = json.dumps(secrets_config_data, indent=4) - return render_template('v3/partials/raw_json.html', - main_config_json=main_config_json, - secrets_config_json=secrets_config_json, - main_config_path=pages_v3.config_manager.get_config_path(), - secrets_config_path=pages_v3.config_manager.get_secrets_path()) - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/raw_json.html', + main_config_json=main_config_json, + secrets_config_json=secrets_config_json, + main_config_path=pages_v3.config_manager.get_config_path(), + secrets_config_path=pages_v3.config_manager.get_secrets_path()) def _load_backup_restore_partial(): """Load backup & restore partial.""" - try: - return render_template('v3/partials/backup_restore.html') - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/backup_restore.html') @pages_v3.route('/setup') def captive_setup(): @@ -754,27 +671,15 @@ def captive_setup(): def _load_wifi_partial(): """Load WiFi setup partial""" - try: - return render_template('v3/partials/wifi.html') - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/wifi.html') def _load_cache_partial(): """Load cache management partial""" - try: - return render_template('v3/partials/cache.html') - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/cache.html') def _load_operation_history_partial(): """Load operation history partial""" - try: - return render_template('v3/partials/operation_history.html') - except Exception as e: - logger.error("Error loading partial", exc_info=True) - return "Error loading partial", 500 + return render_template('v3/partials/operation_history.html') def _load_tools_partial(): @@ -789,6 +694,24 @@ def _load_tools_partial(): return "[Pages V3][Tools] Failed to load due to a file system error. Check logs.", 500 +_PARTIAL_LOADERS = { + 'overview': _load_overview_partial, + 'general': _load_general_partial, + 'display': _load_display_partial, + 'durations': _load_durations_partial, + 'schedule': _load_schedule_partial, + 'plugins': _load_plugins_partial, + 'fonts': _load_fonts_partial, + 'logs': _load_logs_partial, + 'raw-json': _load_raw_json_partial, + 'backup-restore': _load_backup_restore_partial, + 'wifi': _load_wifi_partial, + 'cache': _load_cache_partial, + 'operation-history': _load_operation_history_partial, + 'tools': _load_tools_partial, +} + + def _load_plugin_config_partial(plugin_id): """ Load plugin configuration partial - server-side rendered form. @@ -960,7 +883,7 @@ def _load_plugin_config_partial(plugin_id): web_ui_actions=web_ui_actions ) - except Exception as e: + except Exception: logger.error("Error loading plugin config partial for %s", plugin_id, exc_info=True) return '
Error loading plugin config; see logs for details
', 500 @@ -1041,6 +964,6 @@ def _load_starlark_config_partial(app_id): last_render_time=None, ) - except Exception as e: + except Exception: logger.error("[Pages V3] Error loading starlark config for app", exc_info=True) return '
Error loading starlark config; see logs for details
', 500 diff --git a/web_interface/cache.py b/web_interface/cache.py index 977600a2..643826b1 100644 --- a/web_interface/cache.py +++ b/web_interface/cache.py @@ -6,8 +6,8 @@ seconds or minutes (the font catalog, the system-status snapshot, systemctl checks). It is per-process and in-memory only; data shared with the display service goes through ``src.cache_manager.CacheManager`` instead. -Separated from app.py to avoid circular imports: blueprints import the -module-level helpers below lazily, inside their request handlers. +Separate from app.py so the blueprints can import it without importing the +app; it imports nothing from the project, so they import it at module top. """ import threading import time diff --git a/web_interface/display_preview.py b/web_interface/display_preview.py new file mode 100644 index 00000000..47f3790a --- /dev/null +++ b/web_interface/display_preview.py @@ -0,0 +1,36 @@ +"""The display preview the web UI shows, read from the display service's snapshot. + +GET /api/v3/display/current and the /api/v3/stream/display SSE stream both +answer with preview_payload(). Free of Flask and app imports, like +system_metrics, so a blueprint can use it without constructing the app. +""" + +import base64 +import time +from typing import Any, Dict, Optional + +#: Where DisplayManager writes the snapshot. Written atomically (a temp file +#: and os.replace), so a read never sees half a PNG. +SNAPSHOT_PATH = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path shared with display_manager + + +def read_snapshot_base64(path: Optional[str] = None) -> str: + """The snapshot PNG's bytes, base64-encoded. + + The file already is a PNG, so it is passed through as it is: decoding and + re-encoding it produced the same image for more CPU on the Pi. Raises + OSError when there is no snapshot or it cannot be read. + """ + with open(path or SNAPSHOT_PATH, 'rb') as f: + return base64.b64encode(f.read()).decode('ascii') + + +def preview_payload(width: int, height: int, image: Optional[str]) -> Dict[str, Any]: + """``{timestamp, width, height, image}``; ``image`` is None when there is + no snapshot to show.""" + return { + 'timestamp': time.time(), + 'width': width, + 'height': height, + 'image': image, + } diff --git a/web_interface/start.py b/web_interface/start.py index f62ff086..7b37de78 100644 --- a/web_interface/start.py +++ b/web_interface/start.py @@ -11,10 +11,16 @@ import sys import logging from pathlib import Path +logger = logging.getLogger('web_interface.start') + +# No route to host, broken pipe, connection reset: a client went away. +_CLIENT_DISCONNECT_ERRNOS = (113, 32, 104) + + def get_local_ips(): """Get list of local IP addresses the service will be accessible on.""" ips = [] - + # Check if AP mode is active try: result = subprocess.run( @@ -27,7 +33,7 @@ def get_local_ips(): ips.append("192.168.4.1 (AP Mode)") except Exception: # nosec B110 - AP mode IP detection is non-critical startup info; systemctl may not exist pass - + # Get IPs from hostname -I try: result = subprocess.run( @@ -43,7 +49,7 @@ def get_local_ips(): ips.append(ip) except Exception: # nosec B110 - hostname -I output parsing; non-critical startup info pass - + # Fallback: try socket method if not ips: try: @@ -57,7 +63,7 @@ def get_local_ips(): s.close() except Exception: pass - + return ips if ips else ["localhost"] def main(): @@ -65,15 +71,15 @@ def main(): # Change to project root directory project_root = Path(__file__).parent.parent os.chdir(project_root) - + # Add to Python path sys.path.insert(0, str(project_root)) - + # Configure logging to suppress non-critical socket errors # These occur when clients disconnect and are harmless werkzeug_logger = logging.getLogger('werkzeug') original_log_exception = werkzeug_logger.error - + def log_exception_filtered(message, *args, **kwargs): """Filter out non-critical socket errors from werkzeug logs.""" if isinstance(message, str): @@ -90,51 +96,38 @@ def main(): if 'exc_info' in kwargs and kwargs['exc_info']: exc_type, exc_value, exc_tb = kwargs['exc_info'] if isinstance(exc_value, OSError): - # Suppress common non-critical socket errors - if exc_value.errno in (113, 32, 104): # No route to host, Broken pipe, Connection reset + if exc_value.errno in _CLIENT_DISCONNECT_ERRNOS: werkzeug_logger.debug(message, *args, **kwargs) return # Log everything else normally original_log_exception(message, *args, **kwargs) - + werkzeug_logger.error = log_exception_filtered - - # Import and run the Flask app + + # Importing the app also sets up logging, so the lines below reach the + # journal through it. from web_interface.app import app, start_auto_update_scheduler start_auto_update_scheduler() - print("Starting LED Matrix Web Interface V3...") - print("Web server binding to: 0.0.0.0:5000") - - # Get and display accessible IP addresses - ips = get_local_ips() - if ips: - print("Access the interface at:") - for ip in ips: - if "AP Mode" in ip: - print(" - http://192.168.4.1:5000 (AP Mode - connect to LEDMatrix-Setup WiFi)") - else: - print(f" - http://{ip}:5000") - else: - print(" - http://localhost:5000 (local only)") - print(" - http://:5000 (replace with your Pi's IP address)") - - # Run the web server with error handling for client disconnections + logger.info("Starting LED Matrix Web Interface V3, binding to 0.0.0.0:5000") + # get_local_ips() always returns at least "localhost". + logger.info("Access the interface at:") + for ip in get_local_ips(): + if "AP Mode" in ip: + logger.info(" - http://192.168.4.1:5000 (AP Mode - connect to LEDMatrix-Setup WiFi)") + else: + logger.info(" - http://%s:5000", ip) + try: # threaded=True is Flask's default since 1.0, but set it explicitly - # so it's self-documenting: the two /api/v3/stream/* SSE endpoints + # so it's self-documenting: the three /api/v3/stream/* SSE endpoints # hold long-lived connections and would starve other requests under # a single-threaded server. app.run(host='0.0.0.0', port=5000, debug=False, threaded=True) - except (OSError, BrokenPipeError) as e: - # Suppress non-critical socket errors (client disconnections) - if isinstance(e, OSError) and e.errno in (113, 32, 104): # No route to host, Broken pipe, Connection reset - werkzeug_logger.debug(f"Client disconnected: {e}", exc_info=True) - # Re-raise only if it's not a client disconnection error - if e.errno not in (113, 32, 104): - raise - else: + except OSError as e: + if e.errno not in _CLIENT_DISCONNECT_ERRNOS: raise + werkzeug_logger.debug("Client disconnected: %s", e, exc_info=True) if __name__ == '__main__': main() diff --git a/web_interface/system_metrics.py b/web_interface/system_metrics.py index 723bd85f..2d7ef585 100644 --- a/web_interface/system_metrics.py +++ b/web_interface/system_metrics.py @@ -1,4 +1,4 @@ -"""System metrics for the status stream. +"""System metrics for the status stream and GET /api/v3/system/status. Deliberately free of Flask and app imports: importing web_interface.app constructs the Flask application and a CacheManager, and the latter claims the @@ -11,9 +11,20 @@ the UI can render '--', because a confident wrong number is worse than a blank: on a card that was filling up. """ +import time from typing import Any, Dict, Optional _THERMAL_ZONE = '/sys/class/thermal/thermal_zone0/temp' +_MB = 1024 * 1024 +_GB = 1024 * 1024 * 1024 + +#: Every key collect_system_metrics() returns. +METRIC_KEYS = ( + 'cpu_percent', 'cpu_temp', + 'memory_used_percent', 'memory_total_mb', 'memory_used_mb', 'memory_available_mb', + 'disk_used_percent', 'disk_total_gb', 'disk_used_gb', + 'uptime_seconds', +) def _cpu_temp_c() -> Optional[float]: @@ -25,38 +36,53 @@ def _cpu_temp_c() -> Optional[float]: return None -def collect_system_metrics() -> Dict[str, Any]: +def collect_system_metrics(cpu_interval: Optional[float] = None) -> Dict[str, Any]: """Collect the numbers that predict trouble on a small board. + ``cpu_interval`` is passed to ``psutil.cpu_percent``. None does not block + and measures since the previous call (app startup primes it); a request + that may be the first call in its process passes a short interval instead. + ``memory_available_mb`` is MemAvailable rather than a used percentage: it accounts for reclaimable page cache, so it is what separates a board at 70% "used" that is fine from one at 70% that is about to fail fork(). A 1GB Pi can sit at either and only this number tells them apart. """ + metrics: Dict[str, Any] = dict.fromkeys(METRIC_KEYS) + metrics['cpu_temp'] = _cpu_temp_c() try: import psutil except ImportError: - return { - 'cpu_percent': 0, - 'memory_used_percent': 0, - 'memory_available_mb': None, - 'disk_used_percent': None, - 'cpu_temp': _cpu_temp_c() or 0, - } + return metrics + + metrics['cpu_percent'] = round(psutil.cpu_percent(interval=cpu_interval), 1) - # interval=None is non-blocking; app startup primes psutil's internal state. - cpu_percent = round(psutil.cpu_percent(interval=None), 1) memory = psutil.virtual_memory() + metrics['memory_used_percent'] = round(memory.percent, 1) + metrics['memory_total_mb'] = round(memory.total / _MB, 1) + metrics['memory_used_mb'] = round(memory.used / _MB, 1) + metrics['memory_available_mb'] = round(memory.available / _MB, 1) try: - disk_used_percent: Optional[float] = round(psutil.disk_usage('/').percent, 1) + disk = psutil.disk_usage('/') except OSError: - disk_used_percent = None + pass + else: + metrics['disk_used_percent'] = round(disk.percent, 1) + metrics['disk_total_gb'] = round(disk.total / _GB, 1) + metrics['disk_used_gb'] = round(disk.used / _GB, 1) - return { - 'cpu_percent': cpu_percent, - 'memory_used_percent': round(memory.percent, 1), - 'memory_available_mb': round(memory.available / (1024 * 1024), 1), - 'disk_used_percent': disk_used_percent, - 'cpu_temp': _cpu_temp_c() or 0, - } + metrics['uptime_seconds'] = int(time.time() - psutil.boot_time()) + return metrics + + +def format_uptime(uptime_seconds: Optional[float]) -> Optional[str]: + """'3d 4h', '5h 12m' or '42m'; None when the uptime is unknown.""" + if uptime_seconds is None: + return None + hours = uptime_seconds / 3600 + if hours >= 24: + return f"{int(hours / 24)}d {int(hours % 24)}h" + if hours >= 1: + return f"{int(hours)}h {int((uptime_seconds % 3600) / 60)}m" + return f"{int(uptime_seconds / 60)}m"