From 294bd522bd03d288206b0a5167d401b20c3f4e85 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:56:00 -0400 Subject: [PATCH] fix(web): the installed-plugins list never waits on the registry download (#766) GET /plugins/installed called get_registry_info() per plugin. Despite the "no network call" comment, a cold or expired cache made that download plugins.json (10 s timeout, three attempts), and with no cached copy each plugin's lookup repeated it -- offline, every load waited out the timeouts. The route now reads the registry copy already in memory, however old, via get_cached_registry_info(). A missing or expired copy starts a single background refresh (backing off after an offline failure), so a later load gets update and verified badges. Store, install and update paths still fetch. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 12 ++ src/plugin_system/store_manager.py | 5 + src/plugin_system/store_registry.py | 60 +++++- test/test_api_v3_installed_display_modes.py | 2 +- test/test_api_v3_installed_plugin_icon.py | 2 +- test/test_installed_list_registry_offline.py | 188 ++++++++++++++++++ test/test_plugin_runtime_snapshot.py | 2 +- test/test_vegas_participation.py | 2 +- test/test_web_api.py | 6 +- test/test_web_plugin_dir_resolution.py | 2 +- .../test_web_process_runs_no_plugin_code.py | 1 + web_interface/blueprints/api_v3/plugins.py | 9 +- 12 files changed, 278 insertions(+), 13 deletions(-) create mode 100644 test/test_installed_list_registry_offline.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b9b9fde..a97d2f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,18 @@ accepts both, but the store flags the old spelling as deprecated changed frame, and the render loop writes it (`write_owed_snapshot()`) once the interval has passed. The cadence is unchanged, and nothing extra runs when no frame is owed. +- The installed-plugins list (`GET /api/v3/plugins/installed`) no longer + waits on GitHub. Its comment said the registry lookup made no network call, + but on a cold or expired cache `get_registry_info()` downloads plugins.json + (10 s timeout, three attempts), and with nothing cached to fall back on + every plugin's lookup repeated that: offline, 5 plugins took 11 s with DNS + failing and 2 plugins 65 s with the route black-holed, on every load. The + list now reads the registry copy already in memory, however old + (`get_cached_registry_info()`); with none yet it returns without update or + verified badges and starts one background refresh + (`refresh_registry_in_background()`, backing off for a minute after an + offline failure), so a later load has them. The store, install and update + paths still fetch as before. ### ESPN date-range fetches: fewer requests, fewer at once diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 80f60f67..aa46a9ef 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -138,6 +138,11 @@ class PluginStoreManager(_RegistryMixin, _InstallMixin, _UpdateMixin): # the registry cache expires. Only one thread fetches; others wait and # then get the result from the warm cache (double-checked locking). self._registry_fetch_lock = threading.Lock() + # refresh_registry_in_background: the one refresh thread, and when + # an offline one may be retried (see that method). + self._registry_refresh_lock = threading.Lock() + self._registry_refresh_thread: Optional[threading.Thread] = None + self._registry_refresh_retry_after = 0.0 # Per-plugin locks for _reinstall_with_rollback: the web UI runs # Flask with threaded=True, so two overlapping requests for the diff --git a/src/plugin_system/store_registry.py b/src/plugin_system/store_registry.py index 99c2e5ef..2270843c 100644 --- a/src/plugin_system/store_registry.py +++ b/src/plugin_system/store_registry.py @@ -7,6 +7,7 @@ methods reach shared state and helpers through ``self``. import json import requests +import threading import time from concurrent.futures import ThreadPoolExecutor from datetime import datetime @@ -985,10 +986,14 @@ class _RegistryMixin: def get_registry_info(self, plugin_id: str) -> Optional[Dict]: """ - Get plugin information from the registry cache only (no GitHub API calls). + Get plugin information from the registry (plugins.json). - Use this for lightweight lookups where only registry fields are needed - (e.g., verified status, latest_version). + Makes no GitHub API calls, but it does go through `fetch_registry`: + when the in-memory copy is missing or older than + ``registry_cache_timeout`` it downloads plugins.json, and with no + network that waits out the timeout and retries. A caller that must + not block on the network (the installed-plugins list) uses + `get_cached_registry_info` instead. Args: plugin_id: Plugin identifier @@ -999,3 +1004,52 @@ class _RegistryMixin: registry = self.fetch_registry() plugins = registry.get('plugins', []) or [] return self._match_registry_entry(plugins, plugin_id) + + def get_cached_registry_info(self, plugin_id: str) -> Optional[Dict]: + """The registry entry for ``plugin_id`` from the copy already in + memory, however old; never touches the network. + + None when no registry has been loaded yet, or the plugin isn't in it. + When the copy is missing or past ``registry_cache_timeout`` this + starts `refresh_registry_in_background`, so a later call has it. + """ + cache = getattr(self, 'registry_cache', None) + cache_time = getattr(self, 'registry_cache_time', None) + if (not cache or not cache_time + or (time.time() - cache_time) >= self.registry_cache_timeout): + self.refresh_registry_in_background() + plugins = cache.get('plugins') if isinstance(cache, dict) else None + if not isinstance(plugins, list): + return None + return self._match_registry_entry( + [p for p in plugins if isinstance(p, dict)], plugin_id) + + def refresh_registry_in_background(self) -> bool: + """Fetch the registry on a daemon thread; True when one was started. + + At most one runs at a time. After a fetch that left no registry in + memory (offline), no new one starts for ``_failure_backoff_seconds``, + so an offline Pi doesn't retry on every page load. + """ + with self._registry_refresh_lock: + running = self._registry_refresh_thread + if running is not None and running.is_alive(): + return False + if time.time() < self._registry_refresh_retry_after: + return False + thread = threading.Thread( + target=self._background_registry_refresh, + name='registry-refresh', daemon=True) + self._registry_refresh_thread = thread + thread.start() + return True + + def _background_registry_refresh(self) -> None: + try: + self.fetch_registry() + except Exception as e: # noqa: BLE001 - a background warm-up must not crash + self.logger.warning("Background registry refresh failed: %s", e) + if not getattr(self, 'registry_cache', None): + with self._registry_refresh_lock: + self._registry_refresh_retry_after = ( + time.time() + self._failure_backoff_seconds) diff --git a/test/test_api_v3_installed_display_modes.py b/test/test_api_v3_installed_display_modes.py index 439fc86f..7b15b6b4 100644 --- a/test/test_api_v3_installed_display_modes.py +++ b/test/test_api_v3_installed_display_modes.py @@ -29,7 +29,7 @@ def installed(api_v3_module, api_v3_client, tmp_path): api.plugin_catalog.plugins_dir = str(tmp_path) # no manifest on disk api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) api.plugin_catalog.get_plugin_display_modes = MagicMock(return_value=declared_modes) - api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.plugin_store_manager.get_cached_registry_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={}) response = api_v3_client.get('/api/v3/plugins/installed') assert response.status_code == 200 diff --git a/test/test_api_v3_installed_plugin_icon.py b/test/test_api_v3_installed_plugin_icon.py index 7f661a19..f56dc1bc 100644 --- a/test/test_api_v3_installed_plugin_icon.py +++ b/test/test_api_v3_installed_plugin_icon.py @@ -22,7 +22,7 @@ def installed(api_v3_module, api_v3_client, tmp_path): info.update(manifest_extra) api.plugin_catalog.plugins_dir = str(tmp_path) # no manifest on disk api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) - api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.plugin_store_manager.get_cached_registry_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={}) response = api_v3_client.get('/api/v3/plugins/installed') assert response.status_code == 200 diff --git a/test/test_installed_list_registry_offline.py b/test/test_installed_list_registry_offline.py new file mode 100644 index 00000000..6c35f5be --- /dev/null +++ b/test/test_installed_list_registry_offline.py @@ -0,0 +1,188 @@ +"""GET /api/v3/plugins/installed never waits on the network for registry data. + +The route used `get_registry_info`, which goes through `fetch_registry`: on a +cold (or expired) cache that downloads plugins.json from GitHub with a 10s +timeout and three attempts -- and, with no registry to fall back on, every +plugin's lookup repeated the whole cycle. The first plugin-list load after a +restart waited on GitHub, and offline it waited out every timeout. + +Now the route reads the registry copy already in memory, however old, and a +missing or expired copy only starts a background refresh. These tests block +the network at the socket layer (DNS lookups hang, then fail) and assert the +request returns quickly without a single network attempt on the request path. +""" + +import functools +import socket +import threading +import time +from unittest.mock import MagicMock + +import pytest +import requests + +from test._api_v3_test_helpers import ( # noqa: F401 - fixtures + api_v3_client, api_v3_module, +) +from src.plugin_system.store_manager import PluginStoreManager + +# How long a blocked lookup hangs before failing: long enough that a single +# one on the request path blows the response budget below. +HANG_SECONDS = 1.5 +FAST_SECONDS = 1.0 + +REGISTRY = {'plugins': [ + {'id': 'weather', 'name': 'Weather', 'verified': True, 'latest_version': '1.2.0'}, +]} + + +@pytest.fixture +def blocked_network(monkeypatch): + """Every DNS lookup hangs, then fails; records the thread it came from.""" + attempts = [] + + def hang_then_fail(host, *args, **kwargs): + attempts.append((host, threading.current_thread().name)) + time.sleep(HANG_SECONDS) + raise socket.gaierror(-3, 'Temporary failure in name resolution (blocked by test)') + + monkeypatch.setattr(socket, 'getaddrinfo', hang_then_fail) + return attempts + + +@pytest.fixture +def store(tmp_path, monkeypatch): + store = PluginStoreManager(plugins_dir=str(tmp_path / 'plugins')) + # One attempt, no pause between attempts, so a background refresh against + # the blocked network ends within the test (teardown joins it). + monkeypatch.setattr(store, '_http_get_with_retries', functools.partial( + PluginStoreManager._http_get_with_retries, store, max_retries=1)) + yield store + thread = getattr(store, '_registry_refresh_thread', None) + if thread is not None: + thread.join(timeout=30) + + +@pytest.fixture +def get_installed(api_v3_module, api_v3_client, store, tmp_path): + api = api_v3_module.api_v3 + api.plugin_store_manager = store + api.plugin_catalog.plugins_dir = str(tmp_path / 'plugins') + api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[ + {'id': 'weather', 'name': 'Weather', 'version': '1.0.0'}, + {'id': 'clock', 'name': 'Clock', 'version': '2.0.0'}, + ]) + api.plugin_catalog.get_plugin_display_modes = MagicMock(return_value=[]) + api.config_manager.load_config = MagicMock(return_value={}) + + def _get(): + start = time.perf_counter() + response = api_v3_client.get('/api/v3/plugins/installed') + elapsed = time.perf_counter() - start + assert response.status_code == 200 + plugins = {p['id']: p for p in response.get_json()['data']['plugins']} + return plugins, elapsed + return _get + + +def _request_path_attempts(attempts): + return [a for a in attempts if a[1] != 'registry-refresh'] + + +def test_a_cold_cache_offline_returns_fast_without_registry_info(get_installed, blocked_network): + plugins, elapsed = get_installed() + + assert _request_path_attempts(blocked_network) == [] + assert elapsed < FAST_SECONDS, f"installed list took {elapsed:.2f}s with the network blocked" + weather = plugins['weather'] + assert weather['latest_version'] == '' + assert weather['update_available'] is False + assert weather['verified'] is False + + +def test_a_stale_cache_is_used_as_is_without_a_fetch(get_installed, store, blocked_network): + store.registry_cache = REGISTRY + store.registry_cache_time = time.time() - store.registry_cache_timeout - 3600 + + plugins, elapsed = get_installed() + + assert _request_path_attempts(blocked_network) == [] + assert elapsed < FAST_SECONDS + weather = plugins['weather'] + assert weather['latest_version'] == '1.2.0' + assert weather['update_available'] is True + assert weather['verified'] is True + assert plugins['clock']['latest_version'] == '' + + +def test_a_fresh_cache_starts_no_refresh(get_installed, store, blocked_network): + store.registry_cache = REGISTRY + store.registry_cache_time = time.time() + + plugins, _ = get_installed() + + assert blocked_network == [] + assert store._registry_refresh_thread is None + assert plugins['weather']['update_available'] is True + + +def test_a_cold_cache_is_filled_in_the_background_for_the_next_load(get_installed, store, monkeypatch): + response = MagicMock() + response.json.return_value = REGISTRY + fetched_on = [] + + def fake_get(url, **kwargs): + fetched_on.append(threading.current_thread().name) + return response + + monkeypatch.setattr(store, '_http_get_with_retries', fake_get) + + first, _ = get_installed() + assert first['weather']['update_available'] is False + store._registry_refresh_thread.join(timeout=10) + + second, _ = get_installed() + assert second['weather']['latest_version'] == '1.2.0' + assert second['weather']['update_available'] is True + # One background fetch for the whole listing, none on the request path. + assert fetched_on == ['registry-refresh'] + + +def test_an_offline_background_refresh_backs_off(store, monkeypatch): + def offline(url, **kwargs): + raise requests.ConnectionError('blocked by test') + + monkeypatch.setattr(store, '_http_get_with_retries', offline) + + assert store.refresh_registry_in_background() is True + store._registry_refresh_thread.join(timeout=10) + assert store.registry_cache is None + # Offline: the next page load does not start another attempt straight away. + assert store.refresh_registry_in_background() is False + store._registry_refresh_retry_after = 0.0 + assert store.refresh_registry_in_background() is True + + +def test_only_one_background_refresh_runs_at_a_time(store, monkeypatch): + release = threading.Event() + + def slow(url, **kwargs): + release.wait(10) + raise requests.ConnectionError('blocked by test') + + monkeypatch.setattr(store, '_http_get_with_retries', slow) + try: + assert store.refresh_registry_in_background() is True + assert store.refresh_registry_in_background() is False + finally: + release.set() + + +def test_get_registry_info_still_fetches_for_the_store(store, monkeypatch): + """The store, install and update paths keep fetching a cold registry.""" + response = MagicMock() + response.json.return_value = REGISTRY + monkeypatch.setattr(store, '_http_get_with_retries', MagicMock(return_value=response)) + + assert store.get_registry_info('weather')['latest_version'] == '1.2.0' + store._http_get_with_retries.assert_called_once() diff --git a/test/test_plugin_runtime_snapshot.py b/test/test_plugin_runtime_snapshot.py index 8e476de1..30dda43c 100644 --- a/test/test_plugin_runtime_snapshot.py +++ b/test/test_plugin_runtime_snapshot.py @@ -423,7 +423,7 @@ def web_listing(api_v3_module, api_v3_client, shared_cache, tmp_path): # noqa: {"id": "clock", "name": "Clock", "version": "1.1.0"}, {"id": "weather", "name": "Weather", "version": "3.0.0"}, ]) - api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.plugin_store_manager.get_cached_registry_info = MagicMock(return_value=None) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={ "clock": {"enabled": True}, "weather": {"enabled": True}}) diff --git a/test/test_vegas_participation.py b/test/test_vegas_participation.py index 93949e06..a84fdff4 100644 --- a/test/test_vegas_participation.py +++ b/test/test_vegas_participation.py @@ -446,7 +446,7 @@ class TestInstalledPluginsApi: **(manifest_extra or {})} api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) api.plugin_catalog.get_plugin_directory = MagicMock(return_value=None) - api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.plugin_store_manager.get_cached_registry_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={'demo': config}) response = api_v3_client.get('/api/v3/plugins/installed') assert response.status_code == 200 diff --git a/test/test_web_api.py b/test/test_web_api.py index df881bd4..5b5a42f4 100644 --- a/test/test_web_api.py +++ b/test/test_web_api.py @@ -551,7 +551,7 @@ class TestPluginsAPI: mock_plugin_catalog.get_all_plugin_info.return_value = [ {'id': 'weather', 'name': 'Weather Plugin'} ] - api_v3.plugin_store_manager.get_registry_info.return_value = None + api_v3.plugin_store_manager.get_cached_registry_info.return_value = None response = client.get('/api/v3/plugins/installed') @@ -571,7 +571,7 @@ class TestPluginsAPI: {'id': 'weather', 'name': 'Weather', 'version': '1.0.0'} ] # Registry advertises a newer version than the installed one. - api_v3.plugin_store_manager.get_registry_info.return_value = { + api_v3.plugin_store_manager.get_cached_registry_info.return_value = { 'verified': True, 'latest_version': '1.2.0' } @@ -592,7 +592,7 @@ class TestPluginsAPI: mock_plugin_catalog.get_all_plugin_info.return_value = [ {'id': 'weather', 'name': 'Weather', 'version': '1.2.0'} ] - api_v3.plugin_store_manager.get_registry_info.return_value = { + api_v3.plugin_store_manager.get_cached_registry_info.return_value = { 'verified': True, 'latest_version': '1.2.0' } diff --git a/test/test_web_plugin_dir_resolution.py b/test/test_web_plugin_dir_resolution.py index 27b2188e..f210a5e9 100644 --- a/test/test_web_plugin_dir_resolution.py +++ b/test/test_web_plugin_dir_resolution.py @@ -59,7 +59,7 @@ class TestInstalledList: info = {"id": "demo", "name": "Demo", "version": "1.0.0", "description": "stale cached copy", "loaded": False} api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) - api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) + api.plugin_store_manager.get_cached_registry_info = MagicMock(return_value=None) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={}) diff --git a/test/web_interface/test_web_process_runs_no_plugin_code.py b/test/web_interface/test_web_process_runs_no_plugin_code.py index 5e993d9a..32ac40a0 100644 --- a/test/web_interface/test_web_process_runs_no_plugin_code.py +++ b/test/web_interface/test_web_process_runs_no_plugin_code.py @@ -128,6 +128,7 @@ class Web: store = api.plugin_store_manager store.plugins_dir = str(self.plugins_dir) store.get_registry_info.return_value = None + store.get_cached_registry_info.return_value = None store.get_plugin_info.return_value = None store._get_local_git_info.return_value = None store.install_plugin.return_value = True diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index 15c3df9d..1cd2e394 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -106,8 +106,13 @@ def get_installed_plugins(): plugin_config = {} enabled = bool(plugin_config.get('enabled', False)) - # Verified + latest published version from registry (no network call) - store_info = api_v3.plugin_store_manager.get_registry_info(plugin_id) + # Verified + latest published version from the registry copy already + # in memory. Never a fetch: on a cold cache get_registry_info would + # download plugins.json, and offline wait out its timeout and retries + # for every plugin. With no copy yet these are absent (no update or + # verified badge) and a background refresh fills them in for a + # later load. + store_info = api_v3.plugin_store_manager.get_cached_registry_info(plugin_id) verified = store_info.get('verified', False) if store_info else False latest_version = store_info.get('latest_version', '') if store_info else '' installed_version = plugin_info.get('version', '')