change: give the provider imports shared structure (#23) #138

Merged
mysticalsoap merged 6 commits from change/provider-base-class into trunk 2026-08-24 12:48:10 -04:00
Owner

Problem

#23: the four provider import routines duplicate one skeleton — allow-list IPs, fetch, parse, copy_certs, emit — with drifted error handling (~7 hand-rolled remove_temp_dir + failed.emit pairs) and dispatch via getattr(self, self.provider.lower())(), conflating a provider's display name with its type everywhere. #33's SRP work needs auth to be a strategy on one provider, and the recorded entry model — (display name, provider type, credentials, server list) — needs the name≠type split to become cheap now rather than retrofitted later.

Fix

Six commits, each standing alone:

  1. Provider routines become classes dispatched through a PROVIDERS registry; bodies moved verbatim, thread keeps the signals, temp dir and file helpers (also the tests' stubbing seams).
  2. import_servers() becomes the base template — allow API access, fetch(), the two anticipated failures (AuthFailure, RequestException), copy certs, emit. Message strings unchanged (template tests now pin them byte-exactly).
  3. Custom config-file import becomes CustomProvider via the registry fallback; its no-API path overrides import_servers whole.
  4. name≠type groundwork: optional type in the credentials dict (D-Bus a{ss} unchanged) defaulting to the name; dispatch, copy_certs' supported branch and the config-template lookup key off type; payloads and server entries gain a type field and drop the hardcoded provider literals; service timestamp/airvpn_key persistence follows the type. One-entry-per-type installs behave identically.
  5. config.PROVIDER_TYPES becomes the single source for the type list + gui metadata (credential placeholders, needs_key); gui placeholder map and both provider == "Airvpn" key-field checks become lookups; the cli's own drifted list literal dies.
  6. The providers split into their own package: aqomui/providers/<type>.py per provider, base + shared helpers + registry in the package root. update.py keeps AddServers and the thread-side file helpers (~180 lines); adding a provider is now adding a file, and #33's SRP lands in providers/protonvpn.py next to its only consumer.

Deliberately still name-keyed for 0.9.2 (the future multi-entry issue's follow-through): gui purge-on-reimport check, auto-update queue, provider combo, change_ovpn_config, the cli flow, and {type}_last timestamp granularity.

Verification

pytest — 329 passed (12 new: template error handling ×2, Airvpn single-emission, name≠type entry, registry/config consistency, plus the 36 characterization tests from #135 passing with only two mechanical seam adaptations). ruff + compileall gates clean. Diffed the &-message format strings against trunk: the per-provider literals collapse into the two template strings; runtime output pinned identical by tests. Live gui check still to do via packaging/arch/build-branch-and-install.sh: a custom .ovpn import (Windscribe live testing is deferred until that provider is actively supported; Proton stays broken until #33).

  • Airvpn's api helper caught network errors itself, emitted, and returned None — the caller then crashed and emitted a second, &-less "Sorry, something went wrong" that the gui parses as a success token (tries to open it as a filename). Airvpn now reports network failures once through the template; unexpected errors fall to GuardedThread's standard message. (The gui-side parser fragility is filed separately.)
  • Windscribe no longer misreports a malformed server list as an auth failure — only the credentials check guards AuthFailure.

🤖 Generated with Claude Code

## Problem #23: the four provider import routines duplicate one skeleton — allow-list IPs, fetch, parse, copy_certs, emit — with drifted error handling (~7 hand-rolled `remove_temp_dir` + `failed.emit` pairs) and dispatch via `getattr(self, self.provider.lower())()`, conflating a provider's display name with its type everywhere. #33's SRP work needs auth to be a strategy on one provider, and the recorded entry model — (display name, provider type, credentials, server list) — needs the name≠type split to become cheap now rather than retrofitted later. ## Fix Six commits, each standing alone: 1. Provider routines become classes dispatched through a `PROVIDERS` registry; bodies moved verbatim, thread keeps the signals, temp dir and file helpers (also the tests' stubbing seams). 2. `import_servers()` becomes the base template — allow API access, `fetch()`, the two anticipated failures (`AuthFailure`, `RequestException`), copy certs, emit. Message strings unchanged (template tests now pin them byte-exactly). 3. Custom config-file import becomes `CustomProvider` via the registry fallback; its no-API path overrides `import_servers` whole. 4. name≠type groundwork: optional `type` in the credentials dict (D-Bus `a{ss}` unchanged) defaulting to the name; dispatch, copy_certs' supported branch and the config-template lookup key off type; payloads and server entries gain a `type` field and drop the hardcoded provider literals; service timestamp/airvpn_key persistence follows the type. One-entry-per-type installs behave identically. 5. `config.PROVIDER_TYPES` becomes the single source for the type list + gui metadata (credential placeholders, needs_key); gui placeholder map and both `provider == "Airvpn"` key-field checks become lookups; the cli's own drifted list literal dies. 6. The providers split into their own package: `aqomui/providers/<type>.py` per provider, base + shared helpers + registry in the package root. update.py keeps AddServers and the thread-side file helpers (~180 lines); adding a provider is now adding a file, and #33's SRP lands in providers/protonvpn.py next to its only consumer. Deliberately still name-keyed for 0.9.2 (the future multi-entry issue's follow-through): gui purge-on-reimport check, auto-update queue, provider combo, `change_ovpn_config`, the cli flow, and `{type}_last` timestamp granularity. ## Verification `pytest` — 329 passed (12 new: template error handling ×2, Airvpn single-emission, name≠type entry, registry/config consistency, plus the 36 characterization tests from #135 passing with only two mechanical seam adaptations). `ruff` + compileall gates clean. Diffed the `&`-message format strings against trunk: the per-provider literals collapse into the two template strings; runtime output pinned identical by tests. Live gui check still to do via `packaging/arch/build-branch-and-install.sh`: a custom .ovpn import (Windscribe live testing is deferred until that provider is actively supported; Proton stays broken until #33). ## Related fixes (found along the way, technically out of scope) - Airvpn's api helper caught network errors itself, emitted, and returned None — the caller then crashed and emitted a second, `&`-less "Sorry, something went wrong" that the gui parses as a success token (tries to open it as a filename). Airvpn now reports network failures once through the template; unexpected errors fall to GuardedThread's standard message. (The gui-side parser fragility is filed separately.) - Windscribe no longer misreports a malformed server list as an auth failure — only the credentials check guards `AuthFailure`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Each supported provider becomes a class instantiated per import by
AddServers, dispatched through the PROVIDERS registry instead of
getattr(self, provider.lower()). Routine bodies move verbatim: the
Provider base aliases the thread's signals and temp path and delegates
the file helpers, so the bodies, the service's threading model and the
tests' stubbing seams (thread.copy_certs, thread.gen_wg_key) are all
unchanged. The instance survives as thread.provider_impl -- the seam for
anything that previously reached into per-provider state on the thread.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
import_servers becomes the base template -- allow API access, fetch,
handle the two anticipated failures, copy certs, emit -- and each
provider shrinks to a fetch() that returns its payload and raises
AuthFailure for rejected credentials. The seven hand-rolled
remove_temp_dir + failed.emit pairs collapse into the template; all
message strings are unchanged.

One deliberate fix rides along: Airvpn's api helper used to catch
network errors itself, emit, and return None, so the caller crashed and
emitted a second, &-less 'Sorry, something went wrong' the gui parses
as a success token. It now reports network failures once through the
template like every other provider, and unexpected errors fall to
GuardedThread's standard message. Windscribe similarly no longer
reports a malformed server list as an auth failure -- only the
credentials check guards AuthFailure now.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The registry's fallback replaces work()'s supported/custom branch, so
every import runs the same way: a Provider instantiated per import. The
custom path has no API to fetch from, so it overrides import_servers
whole rather than implementing fetch(), and its two failure emits go
through the base fail() helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Groundwork for the entry model in #23: (display name, provider type,
credentials, server list). The credentials dict takes an optional 'type'
(D-Bus a{ss} shape unchanged), defaulting to the name, so one-entry-
per-type installs behave identically. Dispatch, copy_certs' supported
branch and its config template lookup key off the type; the entry name
keeps owning every on-disk path. Payloads and server entries gain a
'type' field -- what later lets connect-time code key off server type
instead of name membership -- and the payload's hardcoded provider
literals become the entry name. The service's timestamp/airvpn_key
persistence follows the type.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
change: derive the supported-provider list from one table in config
All checks were successful
ci / test (pull_request) Successful in 34s
04dfe12b75
config.PROVIDER_TYPES carries the per-type gui metadata (credential
placeholders, whether a key/device field applies) and SUPPORTED_PROVIDERS
derives from it. The gui's placeholder map and both of its
provider == "Airvpn" key-field checks become table lookups; the cli
drops its own drifted copy of the list and derives its provider help
line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
change: split the providers into their own package
All checks were successful
ci / test (pull_request) Successful in 27s
5b5c80f3da
update.py was the import thread plus every provider's API knowledge in
one 1100-line file, growing ~150 lines per provider and about to grow a
few hundred more when #33 adds SRP to ProtonVPN. Each provider is
self-contained knowledge that accretes independently, so it gets a
module: aqomui/providers/<type>.py, with the base class, the shared
country/resolve helpers and the registry in the package root. update.py
keeps AddServers and the thread-side file helpers (~180 lines).

Adding a provider is now adding a file. The airvpn move drops three
imports its body never used (time, rsa, hashes); gzip/tarfile/logging
die with update.py's rewritten import block.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mysticalsoap force-pushed change/provider-base-class from 5b5c80f3da
All checks were successful
ci / test (pull_request) Successful in 27s
to 3bdc83440d
All checks were successful
ci / test (pull_request) Successful in 29s
ci / test (push) Successful in 34s
2026-08-24 12:20:04 -04:00
Compare
mysticalsoap deleted branch change/provider-base-class 2026-08-24 12:48:10 -04:00
Sign in to join this conversation.
No description provided.