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"