diff --git a/docs/guide/proxies-and-vpn.md b/docs/guide/proxies-and-vpn.md index 01eb3729..ce23b136 100644 --- a/docs/guide/proxies-and-vpn.md +++ b/docs/guide/proxies-and-vpn.md @@ -1069,8 +1069,10 @@ The returned proxy is HTTPS on **port 443**. methods: - `__init__(self, **kwargs)` receives its `proxy_providers` sub-dict splatted as - keyword arguments, so YAML keys map 1:1 to constructor parameters. Do authentication - and catalogue pre-fetching here. + keyword arguments, so YAML keys map 1:1 to constructor parameters. Check the + configuration here, but make no request: every `dl` run builds every configured + proxy provider, even for a run with no proxy. Read the server or location catalogue + through a `functools.cached_property`, so the first query fetches it. - `__repr__(self)` is a human summary, conventionally `"{n} Countries ({m} Servers)"`. - `get_proxy(self, query)` returns a Requests-compatible proxy URI, or `None` only when the query was valid but no proxy exists. Raise on genuine errors or bad queries. diff --git a/tests/core/test_proxy_dispatch.py b/tests/core/test_proxy_dispatch.py index ee985217..f262bb03 100644 --- a/tests/core/test_proxy_dispatch.py +++ b/tests/core/test_proxy_dispatch.py @@ -3,6 +3,7 @@ split, the named or bare provider lookup, and the line that reports which proxy from __future__ import annotations +import json import logging from typing import Iterator, Optional @@ -188,10 +189,29 @@ def test_bare_query_skips_a_provider_that_raises(caplog: pytest.LogCaptureFixtur pick_proxy([Broken()], "us") -def test_named_query_passes_the_provider_error_on() -> None: +def test_named_query_passes_the_provider_error_on_as_a_value_error() -> None: class Broken: def get_proxy(self, query: str) -> str: raise ConnectionError("api down") - with pytest.raises(ConnectionError, match="api down"): + with pytest.raises(ValueError, match="The proxy provider broken failed for us: api down") as raised: pick_proxy([Broken()], "us", "broken") + assert isinstance(raised.value.__cause__, ConnectionError) + + +def test_named_query_names_the_provider_for_a_bad_catalogue_response() -> None: + class Broken: + def get_proxy(self, query: str) -> str: + raise json.JSONDecodeError("Expecting value", "", 0) + + with pytest.raises(ValueError, match="The proxy provider broken failed for us: Expecting value"): + pick_proxy([Broken()], "us", "broken") + + +def test_named_query_passes_a_query_error_on_unchanged() -> None: + class Strict: + def get_proxy(self, query: str) -> str: + raise ValueError("no server for us150") + + with pytest.raises(ValueError, match="^no server for us150$"): + pick_proxy([Strict()], "us150", "strict") diff --git a/tests/core/test_proxy_loading.py b/tests/core/test_proxy_loading.py index 4ace7754..13eca957 100644 --- a/tests/core/test_proxy_loading.py +++ b/tests/core/test_proxy_loading.py @@ -4,6 +4,7 @@ by a bare query and reported by a query that names it.""" from __future__ import annotations import logging +from typing import Any import pytest import requests @@ -15,17 +16,14 @@ from unshackle.core.proxies.resolve import Unavailable, find_provider, load_prox pytestmark = pytest.mark.unit CREDENTIALS = {"username": "a" * 24, "password": "b" * 24} +LOGIN_CREDENTIALS = {"username": "someone@example.com", "password": "hunter2"} OTHERS = ("basic", "expressvpn", "protonvpn", "windscribevpn", "gluetun", "hola", "controld") @pytest.fixture(autouse=True) -def nordvpn_down_surfshark_up(monkeypatch: pytest.MonkeyPatch) -> None: - def refuse() -> list[dict]: - raise requests.ConnectionError("no route to api.nordvpn.com") - +def nordvpn_misconfigured_surfshark_up(monkeypatch: pytest.MonkeyPatch) -> None: clusters = [{"countryCode": "US", "connectionName": "us-dal.prod.surfshark.com", "location": "Dallas"}] - monkeypatch.setattr(config, "proxy_providers", {"nordvpn": CREDENTIALS, "surfsharkvpn": CREDENTIALS}) - monkeypatch.setattr(NordVPN, "get_countries", staticmethod(refuse)) + monkeypatch.setattr(config, "proxy_providers", {"nordvpn": LOGIN_CREDENTIALS, "surfsharkvpn": CREDENTIALS}) monkeypatch.setattr(SurfsharkVPN, "get_countries", staticmethod(lambda: clusters)) @@ -49,7 +47,7 @@ def test_query_naming_another_provider_still_works() -> None: @pytest.mark.parametrize("name", ["nordvpn", "NordVPN"]) def test_query_naming_the_failed_provider_reports_why(name: str) -> None: - with pytest.raises(ValueError, match="NordVPN could not load: no route to api.nordvpn.com"): + with pytest.raises(ValueError, match="NordVPN could not load: NordVPN needs its Service Credentials"): resolve_proxy(f"{name}:us", load_proxy_providers(OTHERS)) @@ -59,10 +57,29 @@ def test_query_naming_an_unknown_provider_lists_the_loaded_ones() -> None: def test_raise_errors_raises_the_load_failure() -> None: - with pytest.raises(requests.ConnectionError): + with pytest.raises(ValueError, match="Service Credentials"): load_proxy_providers(OTHERS, raise_errors=True) +def test_catalogue_is_fetched_on_first_use_and_a_failed_fetch_fails_only_its_query( + monkeypatch: pytest.MonkeyPatch, +) -> None: + calls = [] + + def refuse() -> list[dict]: + calls.append(1) + raise requests.ConnectionError("no route to api.nordvpn.com") + + monkeypatch.setattr(config, "proxy_providers", {"nordvpn": CREDENTIALS, "surfsharkvpn": CREDENTIALS}) + monkeypatch.setattr(NordVPN, "get_countries", staticmethod(refuse)) + nordvpn, surfshark = load_proxy_providers(OTHERS) + assert isinstance(nordvpn, NordVPN) and not calls + + assert resolve_proxy("us", [nordvpn, surfshark]).endswith("@us-dal.prod.surfshark.com:443") + with pytest.raises(ValueError, match="nordvpn failed for us: no route to api.nordvpn.com"): + resolve_proxy("nordvpn:us", [nordvpn, surfshark]) + + def test_bare_query_order_follows_config_key_order_with_controld_last(monkeypatch: pytest.MonkeyPatch) -> None: from unshackle.core.proxies import ControlD @@ -88,3 +105,71 @@ def test_rest_path_loads_windscribe_but_not_gluetun(monkeypatch: pytest.MonkeyPa assert any(isinstance(p, WindscribeVPN) for p in providers) assert not any(isinstance(p, Gluetun) or getattr(p, "name", None) == "Gluetun" for p in providers) assert resolve_proxy("windscribevpn:us", providers).endswith("@us-central-1.example:443") + + +def build_controld() -> Any: + from unshackle.core.proxies import ControlD + + return ControlD(token="t") + + +def build_hola(monkeypatch: pytest.MonkeyPatch) -> Any: + from unshackle.core import binaries + from unshackle.core.proxies import Hola + + monkeypatch.setattr(binaries, "HolaProxy", "hola-proxy") + return Hola() + + +@pytest.mark.parametrize( + ("name", "fetch", "attr"), + [ + ("NordVPN", "get_countries", "countries"), + ("WindscribeVPN", "get_countries", "countries"), + ("SurfsharkVPN", "get_countries", "countries"), + ("ControlD", "get_locations", "catalogue"), + ("Hola", "get_countries", "countries"), + ], +) +def test_construction_makes_no_request_and_the_first_read_fetches_once( + name: str, fetch: str, attr: str, monkeypatch: pytest.MonkeyPatch +) -> None: + from unshackle.core import proxies + + cls = getattr(proxies, name) + calls: list[int] = [] + + def counted(*args: Any) -> list[dict]: + calls.append(1) + return [{"country": "US"}] + + is_static = isinstance(vars(cls)[fetch], staticmethod) + monkeypatch.setattr(cls, fetch, staticmethod(counted) if is_static else counted) + if name == "ControlD": + provider = build_controld() + elif name == "Hola": + provider = build_hola(monkeypatch) + else: + provider = cls(**CREDENTIALS) + assert not calls + + assert getattr(provider, attr) == [{"country": "US"}] + getattr(provider, attr) + assert calls == [1] + + +def test_health_probe_reads_each_proxy_catalogue(monkeypatch: pytest.MonkeyPatch) -> None: + from unshackle.core.api.handlers import run_health_checks + + calls: list[int] = [] + + def countries() -> list[dict]: + calls.append(1) + return [{"serverCount": 3}] + + monkeypatch.setattr(config, "key_vaults", []) + monkeypatch.setattr(config, "proxy_providers", {"nordvpn": CREDENTIALS}) + monkeypatch.setattr(NordVPN, "get_countries", staticmethod(countries)) + detail = {c["id"]: c for c in run_health_checks()}["proxies"]["detail"] + assert calls == [1] + assert "NordVPN (1 Country (3 Servers))" in detail diff --git a/tests/core/test_services_lazy.py b/tests/core/test_services_lazy.py new file mode 100644 index 00000000..921944c7 --- /dev/null +++ b/tests/core/test_services_lazy.py @@ -0,0 +1,143 @@ +"""Services import on first use: a command pays only for the service it runs.""" + +from __future__ import annotations + +import logging +import sys +import threading +from pathlib import Path + +import click +import pytest +from aiohttp import web + +from unshackle.core import binaries, services +from unshackle.core.config import config + +pytestmark = pytest.mark.unit + +EXAMPLE_SOURCE = """\ +import click + + +class EXAMPLE: + ALIASES = ("ex",) + + @staticmethod + def get_binaries(): + return [{"name": "ExampleTool", "desc": "example dependency"}] + + @staticmethod + @click.command() + def cli(): + pass +""" + + +@pytest.fixture +def two_services(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + for tag, body in (("EXAMPLE", EXAMPLE_SOURCE), ("OTHER", "raise ImportError('boom')\n")): + (tmp_path / tag).mkdir() + (tmp_path / tag / "__init__.py").write_text(body) + monkeypatch.setattr(services, "SERVICES", [tmp_path / t / "__init__.py" for t in ("EXAMPLE", "OTHER")]) + monkeypatch.setattr(services, "MODULES", {}) + monkeypatch.setattr(services, "ALIASES", {}) + monkeypatch.setattr(services, "LOAD_ERRORS", []) + monkeypatch.setattr(services, "SHADOWED", []) + monkeypatch.setattr(services, "DIRTY_REPOS", []) + monkeypatch.setattr(services, "SUMMARY_LOGGED", False) + monkeypatch.setattr(binaries, "_registered_binaries", {}) + monkeypatch.setattr(binaries, "__binaries", dict(getattr(binaries, "__binaries"))) + yield + for name in ("EXAMPLE", "OTHER"): + sys.modules.pop(name, None) + + +def test_a_tag_imports_only_its_own_service(two_services: None) -> None: + assert services.Services.get_tag("example") == "EXAMPLE" + assert services.MODULES == {} + + assert services.Services.load("EXAMPLE").__name__ == "EXAMPLE" + assert list(services.MODULES) == ["EXAMPLE"] and not services.LOAD_ERRORS + assert [d["name"] for d in binaries.get_registered_dependencies()] == ["ExampleTool"] + + +def test_an_alias_imports_services_until_one_matches(two_services: None) -> None: + assert services.Services.get_tag("ex") == "EXAMPLE" + assert list(services.MODULES) == ["EXAMPLE"] + + +def test_a_failed_import_fails_only_its_own_tag_and_is_not_retried(two_services: None) -> None: + with pytest.raises(KeyError): + services.Services.load("OTHER") + assert len(services.LOAD_ERRORS) == 1 and services.LOAD_ERRORS[0].startswith("OTHER: ") + services.ensure_loaded("OTHER") + assert len(services.LOAD_ERRORS) == 1 + + services.check_load_errors("EXAMPLE") + with pytest.raises(click.ClickException, match="Failed to load 1 service"): + services.check_load_errors("OTHER") + + +def test_get_command_runs_a_good_service_beside_a_broken_one(two_services: None) -> None: + group = services.Services(name="dl") + ctx = click.Context(group) + with pytest.raises(click.ClickException, match="OTHER: failed to import"): + group.get_command(ctx, "OTHER") + assert group.get_command(ctx, "EXAMPLE").name == "EXAMPLE" + + +def test_get_tag_does_not_wait_for_a_reload_on_a_failed_tag(two_services: None) -> None: + services.ensure_loaded("EXAMPLE") + services.ensure_loaded("OTHER") + held, release = threading.Event(), threading.Event() + + def reload() -> None: + with services.RELOAD_LOCK: + held.set() + release.wait() + + holder = threading.Thread(target=reload) + holder.start() + try: + assert held.wait(5) + result: list[str] = [] + lookup = threading.Thread(target=lambda: result.append(services.Services.get_tag("nosuch"))) + lookup.start() + lookup.join(5) + assert result == ["nosuch"] + finally: + release.set() + holder.join() + + +def test_load_all_imports_every_service(two_services: None) -> None: + services.load_all() + assert list(services.MODULES) == ["EXAMPLE"] + assert services.ALIASES == {"EXAMPLE": ("ex",)} + assert services.failed_tags(services.LOAD_ERRORS) == {"OTHER"} + + +def test_env_dependencies_list_every_service_binary(two_services: None) -> None: + from unshackle.commands.env import get_dependencies + + assert "ExampleTool" in {dep["name"] for dep in get_dependencies()} + + +def test_serve_startup_imports_every_service(two_services: None, monkeypatch: pytest.MonkeyPatch) -> None: + from unshackle.commands.serve import _install_service_refresh + + monkeypatch.setattr(config, "serve", {}) + _install_service_refresh(web.Application()) + assert list(services.MODULES) == ["EXAMPLE"] + + +def test_a_run_of_one_tag_names_its_duplicate(two_services: None, caplog: pytest.LogCaptureFixture) -> None: + services.SHADOWED[:] = [ + "EXAMPLE: using /a/EXAMPLE, ignoring /b/EXAMPLE", + "OTHER: using /a/OTHER, ignoring /b/OTHER", + ] + with caplog.at_level(logging.INFO, logger="services"): + services.check_load_errors("EXAMPLE") + assert "EXAMPLE: 1 duplicate(s) ignored" in caplog.text + assert "OTHER" not in caplog.text diff --git a/unshackle/commands/dl.py b/unshackle/commands/dl.py index 863d795f..0fcbd091 100644 --- a/unshackle/commands/dl.py +++ b/unshackle/commands/dl.py @@ -607,6 +607,8 @@ class dl: Returns: Tuple of (fonts_attached_count, missing_fonts_list) """ + if not font_names: + return 0, [] system_fonts = get_system_fonts() font_count = 0 diff --git a/unshackle/commands/env.py b/unshackle/commands/env.py index 5df44cb4..c02e6006 100644 --- a/unshackle/commands/env.py +++ b/unshackle/commands/env.py @@ -13,7 +13,7 @@ from unshackle.core import binaries from unshackle.core.config import POSSIBLE_CONFIG_PATHS, config, config_path from unshackle.core.console import console, listing_table, print_wide from unshackle.core.constants import context_settings -from unshackle.core.services import Services +from unshackle.core.services import Services, load_all from unshackle.core.temp import TASK_PREFIX, is_stale @@ -134,6 +134,7 @@ def get_dependencies() -> list[dict]: }, ] + load_all() for reg in binaries.get_registered_dependencies(): deps.append( { diff --git a/unshackle/commands/serve.py b/unshackle/commands/serve.py index a1b9e7bb..cf0b5ccc 100644 --- a/unshackle/commands/serve.py +++ b/unshackle/commands/serve.py @@ -76,6 +76,7 @@ def _install_service_refresh(app: web.Application) -> None: from unshackle.core import services log = logging.getLogger("serve") + services.load_all() services.log_load_issues() services.record_loaded_commits() interval = int(config.serve.get("services_refresh_interval", 0) or 0) @@ -575,7 +576,11 @@ def serve( app["config"] = serve_config app["debug_api"] = debug_api - if not cdm_only: + if cdm_only: + from unshackle.core import services + + services.load_all() + else: _install_session_store(app) _install_service_refresh(app) _install_config_reload(app, no_key=no_key, serve_widevine=serve_widevine, serve_playready=serve_playready) diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 92a48f8d..668dd61e 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -3020,10 +3020,11 @@ def run_health_checks(checks: Optional[List[Dict[str, Any]]] = None) -> List[Dic checks.append(health_check("bad_keys", "bad content key flags", bad_keys_probe)) def proxy_probe() -> tuple[str, str]: + """Render each proxy provider, because the render fetches its catalogue from the provider.""" providers = initialize_proxy_providers(raise_errors=True, quiet=True) if not providers: return "ok", "none configured" - return "ok", ", ".join(type(p).__name__ for p in providers) + return "ok", ", ".join(f"{type(p).__name__} ({p!r})" for p in providers) if config.proxy_providers: checks.append(health_check("proxies", "proxy providers", proxy_probe, config_secrets(config.proxy_providers))) diff --git a/unshackle/core/proxies/controld.py b/unshackle/core/proxies/controld.py index 9611aa99..e7275860 100644 --- a/unshackle/core/proxies/controld.py +++ b/unshackle/core/proxies/controld.py @@ -9,6 +9,7 @@ import socket import socketserver import threading import time +from functools import cached_property from typing import Optional from urllib.parse import urlsplit @@ -361,7 +362,10 @@ class ControlD(Proxy): self.resolvers = dict(self.configured) self.last_profile: Optional[str] = None - self.catalogue = self.get_locations() + @cached_property + def catalogue(self) -> list[dict]: + """Fetched on first use, so a run that wants no proxy makes no request.""" + return self.get_locations() def __repr__(self) -> str: countries = len({x["country"] for x in self.catalogue}) @@ -377,7 +381,7 @@ class ControlD(Proxy): response.raise_for_status() return response.json()["body"]["proxies"] except Exception as e: - # never fail the command: all providers load up front, even for a run that wants no proxy + # never fail the command: a bare query falls through to the next proxy provider log.warning(f"Could not read Control D's locations: {e}") return [] diff --git a/unshackle/core/proxies/hola.py b/unshackle/core/proxies/hola.py index 6d81684b..f3ae958a 100644 --- a/unshackle/core/proxies/hola.py +++ b/unshackle/core/proxies/hola.py @@ -1,6 +1,7 @@ import random import re import subprocess +from functools import cached_property from typing import Optional from unshackle.core import binaries @@ -16,7 +17,10 @@ class Hola(Proxy): raise EnvironmentError("hola-proxy executable not found but is required for the Hola proxy provider.") self.binary = binaries.HolaProxy - self.countries = self.get_countries() + @cached_property + def countries(self) -> list[dict[str, str]]: + """Fetched on first use, so a run that wants no proxy makes no request.""" + return self.get_countries() def __repr__(self) -> str: countries = len(self.countries) diff --git a/unshackle/core/proxies/nordvpn.py b/unshackle/core/proxies/nordvpn.py index c6cfb4df..27d6290d 100644 --- a/unshackle/core/proxies/nordvpn.py +++ b/unshackle/core/proxies/nordvpn.py @@ -1,6 +1,7 @@ import json import random import re +from functools import cached_property from typing import Optional, Union import requests @@ -33,7 +34,10 @@ class NordVPN(Proxy): self.server_map = server_map or {} self.last_hostname: Optional[str] = None - self.countries = self.get_countries() + @cached_property + def countries(self) -> list[dict]: + """Fetched on first use, so a run that wants no proxy makes no request.""" + return self.get_countries() def __repr__(self) -> str: countries = len(self.countries) diff --git a/unshackle/core/proxies/proxy.py b/unshackle/core/proxies/proxy.py index 4986c492..abcaee47 100644 --- a/unshackle/core/proxies/proxy.py +++ b/unshackle/core/proxies/proxy.py @@ -12,7 +12,7 @@ F = TypeVar("F", bound=Callable[..., Any]) def cached_catalogue(fetch: F) -> F: """Keep a proxy provider's catalogue for ``CATALOGUE_TTL`` seconds; a failed (empty) answer is fetched again. - A server builds its proxy providers on every request, and each build fetches the catalogue. + A server builds its proxy providers on every request, and the first query on each one reads the catalogue. One config builds every provider, so a config reload clears the cache. """ @@ -33,9 +33,10 @@ class Proxy: @abstractmethod def __init__(self, **kwargs: Any) -> None: """ - The constructor initializes the Service using passed configuration data. + The constructor initialises the proxy provider from its configuration data. - Any authorization or pre-fetching of data should be done here. + Check the configuration here, but make no request: every run builds every configured proxy provider, + even a run with no proxy. Fetch a catalogue on first use, through a ``functools.cached_property``. """ @abstractmethod diff --git a/unshackle/core/proxies/resolve.py b/unshackle/core/proxies/resolve.py index 56bf4b90..0cbd588d 100644 --- a/unshackle/core/proxies/resolve.py +++ b/unshackle/core/proxies/resolve.py @@ -3,12 +3,14 @@ from __future__ import annotations import ipaddress +import json import logging import re from typing import Any, List, Optional from urllib.parse import urlparse import click +import requests from click.core import ParameterSource from unshackle.core.proxies.proxy import Proxy @@ -58,6 +60,7 @@ def load_proxy_providers(exclude: tuple[str, ...] = (), raise_errors: bool = Fal Each proxy provider loads on its own. One that fails is logged and kept as Unavailable, so a bare query skips it and a query that names it reports why. *raise_errors* raises the failure instead. + The first query fetches a catalogue, not this function. *exclude* holds lowercase class names to leave out. """ from unshackle.core import binaries @@ -106,7 +109,7 @@ def load_proxy_providers(exclude: tuple[str, ...] = (), raise_errors: bool = Fal if provider is None: continue if not quiet: - log.info(f"Loaded {name}: {provider}") + log.info(f"Loaded {name}") except Exception as e: if raise_errors: raise @@ -122,8 +125,7 @@ def initialize_proxy_providers(raise_errors: bool = False, quiet: bool = False) Load the proxy providers for the REST API and the remote service client. Gluetun is not offered on this path, because it starts a local Docker container. *quiet* drops the - per-provider summary lines: rendering a proxy provider asks some of them for their server catalogue - over the network, which a repeated caller must not pay for. + per-provider summary lines. """ proxy_providers = load_proxy_providers(("gluetun",), raise_errors, quiet) if not quiet and not proxy_providers: @@ -154,12 +156,17 @@ def pick_proxy(proxy_providers: List[Any], query: str, provider_name: Optional[s """ Get a proxy provider and its proxy URI: from the named one, or else from the first one that answers. - A named proxy provider's error goes to the caller. In a bare query, a proxy provider that fails is + A named proxy provider's error goes to the caller as a ValueError. In a bare query, a proxy provider that fails is logged and skipped, so a later one can still answer. Raises ValueError when no proxy provider answers. """ if provider_name: provider = find_provider(proxy_providers, provider_name) - uri = provider.get_proxy(query) + try: + uri = provider.get_proxy(query) + except Exception as e: + if isinstance(e, ValueError) and not isinstance(e, (json.JSONDecodeError, requests.RequestException)): + raise + raise ValueError(f"The proxy provider {provider_name} failed for {query}: {e}") from e if not uri: raise ValueError(f"The proxy provider {provider_name} had no proxy for {query}") return provider, uri diff --git a/unshackle/core/proxies/surfsharkvpn.py b/unshackle/core/proxies/surfsharkvpn.py index 1b6c715b..e8dcad88 100644 --- a/unshackle/core/proxies/surfsharkvpn.py +++ b/unshackle/core/proxies/surfsharkvpn.py @@ -6,7 +6,7 @@ import re import socket import ssl import threading -from functools import cache, partial +from functools import cache, cached_property, partial from typing import Optional import certifi @@ -119,7 +119,11 @@ class SurfsharkVPN(Proxy): self.server_map[COUNTRY_CODE_ALIASES.get(country, country) + sep + city] = server self.last_server: Optional[tuple[str, str]] = None - self.countries = self.get_countries() + + @cached_property + def countries(self) -> list[dict]: + """Fetched on first use, so a run that wants no proxy makes no request.""" + return self.get_countries() def __repr__(self) -> str: countries = len(set(x.get("country") for x in self.countries if x.get("country"))) diff --git a/unshackle/core/proxies/windscribevpn.py b/unshackle/core/proxies/windscribevpn.py index 1513f79f..1aa9f0fe 100644 --- a/unshackle/core/proxies/windscribevpn.py +++ b/unshackle/core/proxies/windscribevpn.py @@ -1,6 +1,7 @@ import json import random import re +from functools import cached_property from typing import Iterator, Optional import requests @@ -33,7 +34,10 @@ class WindscribeVPN(Proxy): self.server_map = server_map or {} self.last_hostname: Optional[str] = None - self.countries = self.get_countries() + @cached_property + def countries(self) -> list[dict]: + """Fetched on first use, so a run that wants no proxy makes no request.""" + return self.get_countries() def __repr__(self) -> str: countries = len(set(x.get("country_code") for x in self.countries if x.get("country_code"))) diff --git a/unshackle/core/services.py b/unshackle/core/services.py index aca57408..44f97352 100644 --- a/unshackle/core/services.py +++ b/unshackle/core/services.py @@ -108,24 +108,6 @@ def load_service(path: Path) -> type[Service]: ) from e -def load_services(paths: list[Path]) -> tuple[dict[str, type[Service]], list[str]]: - """Load every Service, returning the good ones plus a list of load errors. - - Importing this module must never raise: several commands import it, and Python - does not cache a failed import, so raising here would re-run and re-report for - every command. Instead we collect failures and let the caller surface them once, - cleanly, when it uses the services. - """ - modules: dict[str, type[Service]] = {} - errors: list[str] = [] - for path in paths: - try: - modules[path.parent.stem] = load_service(path) - except Exception as e: - errors.append(str(e)) - return modules, errors - - def register_service_binaries(modules: dict[str, type[Service]]) -> None: """Register custom binary dependencies declared by services via get_binaries().""" @@ -144,15 +126,46 @@ def register_service_binaries(modules: dict[str, type[Service]]) -> None: log.warning(f"Failed to register custom binaries for service {tag}: {e}") -MODULES, LOAD_ERRORS = load_services(SERVICES) -register_service_binaries(MODULES) - -ALIASES = {tag: getattr(module, "ALIASES", ()) for tag, module in MODULES.items()} +MODULES: dict[str, type[Service]] = {} +LOAD_ERRORS: list[str] = [] +ALIASES: dict[str, tuple[str, ...]] = {} PENDING: set[str] = set() PENDING_SINCE: dict[str, float] = {} LOADED_COMMITS: dict[str, str] = {} -RELOAD_LOCK = threading.Lock() +RELOAD_LOCK = threading.RLock() + + +def ensure_loaded(tag: str) -> type[Service] | None: + """Import one discovered service on first use; None when the tag is unknown or its import failed. + + Startup imports no service: a command that runs one service pays only for that one. + A failed import is kept in ``LOAD_ERRORS`` and is not tried again until a reload. + """ + service = MODULES.get(tag) + if service is not None or tag in failed_tags(LOAD_ERRORS): + return service + with RELOAD_LOCK: + if tag in MODULES or tag in failed_tags(LOAD_ERRORS): + return MODULES.get(tag) + path = next((path for path in SERVICES if path.parent.stem == tag), None) + if path is None: + return None + try: + service = load_service(path) + except Exception as e: + LOAD_ERRORS.append(str(e)) + return None + MODULES[tag] = service + ALIASES[tag] = getattr(service, "ALIASES", ()) + register_service_binaries({tag: service}) + return service + + +def load_all() -> None: + """Import every discovered service, for the callers that need all of them (help, serve, env).""" + for tag in Services.get_tags(): + ensure_loaded(tag) def record_loaded_commits(tags: Iterable[str] | None = None) -> None: @@ -320,7 +333,7 @@ def log_load_issues() -> None: log.error(f"Service skipped: {err}") -def check_load_errors() -> None: +def check_load_errors(tag: str | None = None) -> None: """Log a one-line load summary (once) and raise a single clean error on failures. unshackle calls this when it needs the services (listing/resolving), so Click @@ -328,23 +341,27 @@ def check_load_errors() -> None: command that imports this module. This function summarises the duplicate services (a tag that an earlier, higher-priority source also gives, and which unshackle ignores). The full list - naming the path used and the duplicate ignored - is - debug-only. + debug-only. With *tag*, only that service's load error counts, and the summary names + only a duplicate of that tag. """ if DIRTY_REPOS: joined = "\n".join(f" - {p}" for p in DIRTY_REPOS) raise click.ClickException("Service repo has local changes, not refreshing:\n" + joined) + errors = [err for err in LOAD_ERRORS if tag is None or err.startswith(f"{tag}: ")] + if errors: + joined = "\n".join(f" - {err}" for err in errors) + raise click.ClickException(f"Failed to load {len(errors)} service(s):\n{joined}") global SUMMARY_LOGGED - if not SUMMARY_LOGGED: - SUMMARY_LOGGED = True - summary = f"Loaded {len(MODULES)} services" - if SHADOWED: - summary += f" ({len(SHADOWED)} duplicate(s) ignored)" - log.info(summary) - for shadow in SHADOWED: - log.debug("%s", shadow) - if LOAD_ERRORS: - joined = "\n".join(f" - {err}" for err in LOAD_ERRORS) - raise click.ClickException(f"Failed to load {len(LOAD_ERRORS)} service(s):\n{joined}") + if SUMMARY_LOGGED: + return + SUMMARY_LOGGED = True + shadows = SHADOWED if tag is None else [shadow for shadow in SHADOWED if shadow.startswith(f"{tag}: ")] + if tag is None: + log.info(f"Loaded {len(MODULES)} services" + (f" ({len(SHADOWED)} duplicate(s) ignored)" if SHADOWED else "")) + elif shadows: + log.info(f"{tag}: {len(shadows)} duplicate(s) ignored") + for shadow in shadows: + log.debug("%s", shadow) ROUTING_PARAMS = ("remote", "server") @@ -395,12 +412,12 @@ class Services(click.Group): if remote_tag not in tags: tags.append(remote_tag) return tags + load_all() check_load_errors() return Services.get_tags() def get_command(self, ctx: click.Context, name: str) -> click.Command: """Load the Service and return the Click CLI method.""" - check_load_errors() tag = Services.get_tag(name) if Services.is_import(ctx): @@ -411,6 +428,8 @@ class Services(click.Group): if remote: return Services.make_remote_command(name, routing.get("server")) + ensure_loaded(tag) + check_load_errors(tag) try: service = Services.load(tag) except KeyError as e: @@ -608,6 +627,8 @@ class Services(click.Group): return tag for tag in tags: + if tag not in ALIASES: + ensure_loaded(tag) if any(value_lower == alias.lower() for alias in ALIASES.get(tag, ())): return tag @@ -616,7 +637,7 @@ class Services(click.Group): @staticmethod def load(tag: str) -> type[Service]: """Load a Service module by Service tag.""" - module = MODULES.get(tag) + module = ensure_loaded(tag) if module: return module diff --git a/unshackle/core/utilities.py b/unshackle/core/utilities.py index 218a2b72..51952659 100644 --- a/unshackle/core/utilities.py +++ b/unshackle/core/utilities.py @@ -897,9 +897,10 @@ def get_unix_fonts() -> dict[str, Path]: return fonts +@cache def get_system_fonts() -> dict[str, Path]: """ - Get system fonts as a mapping of font family names to font file paths. + Get system fonts as a mapping of font family names to font file paths, scanned once per process. On Windows: Uses registry to get font display names On Linux/macOS: Scans standard font directories and extracts family names using fontTools diff --git a/unshackle/core/utils/xml.py b/unshackle/core/utils/xml.py index 30eb83df..c5e033d8 100644 --- a/unshackle/core/utils/xml.py +++ b/unshackle/core/utils/xml.py @@ -5,20 +5,19 @@ from lxml.etree import ElementTree def load_xml(xml: Union[str, bytes]) -> ElementTree: - """Safely parse XML data to an ElementTree, without namespaces in tags.""" + """Safely parse XML data to an ElementTree, without namespaces in tags. + + The strip uses string operations, not etree.QName: on a 20 MB manifest the QName walk costs several times the parse. + """ if not isinstance(xml, bytes): xml = xml.encode("utf8") root = etree.fromstring(xml) - for elem in root.getiterator(): - if not hasattr(elem.tag, "find"): - # e.g. comment elements - continue - elem.tag = etree.QName(elem).localname - for name, value in elem.attrib.items(): - local_name = etree.QName(name).localname - if local_name == name: - continue - del elem.attrib[name] - elem.attrib[local_name] = value + for elem in root.iter(etree.Element): + tag = elem.tag + if tag[0] == "{": + elem.tag = tag.partition("}")[2] + attrib = elem.attrib + for name in [name for name in attrib if name[0] == "{"]: + attrib[name.partition("}")[2]] = attrib.pop(name) etree.cleanup_namespaces(root) return root