mysticalsoap/aqomui
Watch
1
0
Fork
You've already forked aqomui
 0
Code Issues 38 Pull requests Releases 3 Packages Wiki Activity Actions

Move gui/cli JSON persistence into config #173

Merged
mysticalsoap merged 5 commits from json-persistence into trunk 2026-08-27 16:49:10 -04:00
Conversation 0 Commits 5 Files changed 8 +214 -210
mysticalsoap commented 2026-08-27 13:00:11 -04:00
Owner
Copy link

Problem

The #83 seam-2 move (#126): the gui and cli each carried their own load_json copy plus a dozen inline json.dump sites for the five state files. Found surveying for #23 and filed as #139: del_provider popped the provider's entry from protocol_dict but wrote only server.json, so the deleted protocol entry came back from disk on the next start. The cli's delete_provider path turned out to have the same hole. On top of that, resources/default_config.json duplicated config.default_settings as a stale subset (missing no_dnsmasq, dns_off, bypass_networks).

Fix

  • config.load_json/config.save_json plus per-file path constants (SERVER_FILE, PROTOCOL_FILE, PROFILE_FILE, BYPASS_APPS_FILE, LAST_SERVER_FILE, CONFIG_FILE) and JSON_FILE_LIST now live in config, which already owned the service side of the same job. config stays Qt-free per the #83 placement rule, so the cli keeps importing it. Both load_json copies and every inline dump site are gone; load_config reuses the shared helper. The server.json/protocol.json call sites pass to the #125 catalog when that lands.
  • Both delete paths now save protocol.json after popping the provider's entry (#139).
  • default_config.json is deleted (also from setup.py and the PKGBUILD); the cli's get_config goes through load_config like every other consumer, which also lets enable/disable set keys a stale config.json doesn't carry. load_config deep-copies defaulted values so appending to a defaulted bypass_networks can't rewrite the template.

Edge notes, detailed in the commit bodies: the cli's arguments() locals named config had to become conf (module shadowing), and load_config no longer aliases default_settings on fallback.

Verification

  • Full suite passes (397). New tests/test_config.py covers round-trip, missing-file and corrupt-file behavior (the #162 log-level contract moved there from the gui tests), load_config's defaults merge, and that defaulted values don't alias the template.
  • New TestDelProvider regression tests assert both halves of a deletion survive a restart, including a provider with no protocol entry.
  • Import smoke test of both entry points and the CI lint gate (ruff check --select E9,F63,F7,F82 .) pass.
  • Verified live via build-branch-and-install.sh: package no longer ships default_config.json; protocol flips, favourites and provider deletion persist correctly (#139 scenario reproduced with an override-protocol entry on a custom provider — entry removed on delete, no resurrection after relaunch); --show-options shows the merged defaults; --disable ping / --enable ping round-trip through cfgupdate and persist — the first successful run of these flags ever; gui connects record last_server.json correctly across consecutive servers with the hop key preserved.

Related fixes (found along the way, technically out of scope)

  • applyoptions was broken since upstream: it called self.dbus_call, a method AqomuiCli never defined, and load_firewall without the stage argument the D-Bus signature requires — so --enable/--disable/--set-alt-dns had never worked. Both repaired; surfaced by the first live test of those flags during verification.
  • setup.py declared the license via a deprecated trove classifier, warning twice on every wheel build; now an SPDX expression (GPL-3.0-or-later). Surfaced by the verification build.

Closes #126. Closes #139.

🤖 Generated with Claude Code

## Problem The #83 seam-2 move (#126): the gui and cli each carried their own `load_json` copy plus a dozen inline `json.dump` sites for the five state files. Found surveying for #23 and filed as #139: `del_provider` popped the provider's entry from `protocol_dict` but wrote only `server.json`, so the deleted protocol entry came back from disk on the next start. The cli's `delete_provider` path turned out to have the same hole. On top of that, `resources/default_config.json` duplicated `config.default_settings` as a stale subset (missing `no_dnsmasq`, `dns_off`, `bypass_networks`). ## Fix - `config.load_json`/`config.save_json` plus per-file path constants (`SERVER_FILE`, `PROTOCOL_FILE`, `PROFILE_FILE`, `BYPASS_APPS_FILE`, `LAST_SERVER_FILE`, `CONFIG_FILE`) and `JSON_FILE_LIST` now live in config, which already owned the service side of the same job. config stays Qt-free per the #83 placement rule, so the cli keeps importing it. Both `load_json` copies and every inline dump site are gone; `load_config` reuses the shared helper. The `server.json`/`protocol.json` call sites pass to the #125 catalog when that lands. - Both delete paths now save `protocol.json` after popping the provider's entry (#139). - `default_config.json` is deleted (also from setup.py and the PKGBUILD); the cli's `get_config` goes through `load_config` like every other consumer, which also lets `enable`/`disable` set keys a stale `config.json` doesn't carry. `load_config` deep-copies defaulted values so appending to a defaulted `bypass_networks` can't rewrite the template. Edge notes, detailed in the commit bodies: the cli's `arguments()` locals named `config` had to become `conf` (module shadowing), and `load_config` no longer aliases `default_settings` on fallback. ## Verification - Full suite passes (397). New `tests/test_config.py` covers round-trip, missing-file and corrupt-file behavior (the #162 log-level contract moved there from the gui tests), `load_config`'s defaults merge, and that defaulted values don't alias the template. - New `TestDelProvider` regression tests assert both halves of a deletion survive a restart, including a provider with no protocol entry. - Import smoke test of both entry points and the CI lint gate (`ruff check --select E9,F63,F7,F82 .`) pass. - Verified live via `build-branch-and-install.sh`: package no longer ships `default_config.json`; protocol flips, favourites and provider deletion persist correctly (#139 scenario reproduced with an override-protocol entry on a custom provider — entry removed on delete, no resurrection after relaunch); `--show-options` shows the merged defaults; `--disable ping` / `--enable ping` round-trip through cfgupdate and persist — the first successful run of these flags ever; gui connects record `last_server.json` correctly across consecutive servers with the hop key preserved. ## Related fixes (found along the way, technically out of scope) - `applyoptions` was broken since upstream: it called `self.dbus_call`, a method `AqomuiCli` never defined, and `load_firewall` without the stage argument the D-Bus signature requires — so `--enable`/`--disable`/`--set-alt-dns` had never worked. Both repaired; surfaced by the first live test of those flags during verification. - setup.py declared the license via a deprecated trove classifier, warning twice on every wheel build; now an SPDX expression (`GPL-3.0-or-later`). Surfaced by the verification build. Closes #126. Closes #139. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
The #83 seam-2 move (#126): AqomuiGui.load_json, its verbatim
AqomuiCli copy, JSON_FILE_LIST and every inline json.dump of the
five state files become config.load_json/save_json plus per-file
path constants. config stays Qt-free, so the cli keeps importing
it; the server.json/protocol.json call sites pass to the #125
catalog when that lands.

The cli's arguments() locals named config had to become conf: once
the function body references the config module, a local of the
same name makes every module reference an UnboundLocalError.

Two deliberate behavior changes at the edges: load_config no
longer aliases default_settings on fallback, so mutating the live
settings can't rewrite the template it falls back on; and the
cli's get_config now reaches default_config.json on a malformed
config.json too, where the old copy only caught the missing-file
case and crashed on parse errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: persist protocol.json when a provider is deleted
All checks were successful
ci / test (pull_request) Successful in 26s
Details
aa60f62f18
 
del_provider popped the provider's entry from protocol_dict but
wrote only server.json, so the popped entry came back from disk on
the next start and lingered until an unrelated import rewrote
protocol.json (#139). The cli's delete_provider path had the same
hole.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
change: drop default_config.json for the in-code defaults
All checks were successful
ci / test (pull_request) Successful in 26s
Details
309377ce5c
 
The shipped file was a stale subset of config.default_settings
(missing no_dnsmasq, dns_off and bypass_networks) and its only
consumer was the cli's get_config fallback. get_config now goes
through load_config like every other consumer, which also lets
enable/disable set keys a config.json written before their
introduction doesn't carry.

load_config deep-copies defaulted values while merging: the
bypass_networks list was handed out by reference, so appending to
a defaulted settings dict silently rewrote the template every
later fallback starts from.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: applyoptions called a dbus_call helper the cli never had
All checks were successful
ci / test (pull_request) Successful in 45s
Details
35d7ca1fa2
 
--enable, --disable and --set-alt-dns all funnel into applyoptions,
whose first line called self.dbus_call -- a method AqomuiCli has
never defined, here or upstream. Every other service call in the
file goes through the aqomui_service proxy directly; cfgupdate now
does too. Surfaced by the first live test of these flags, during
PR #173 verification.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
change: declare the license as an SPDX expression
All checks were successful
ci / test (pull_request) Successful in 25s
Details
fbb3f09328
 
setuptools deprecated trove license classifiers in favor of SPDX
license expressions and warns twice on every wheel build. Same
license, new spelling. Surfaced during PR #173's verification
build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mysticalsoap force-pushed json-persistence from fbb3f09328
All checks were successful
ci / test (pull_request) Successful in 25s
Details
to 1c69b09c84
All checks were successful
ci / test (pull_request) Successful in 25s
Details
2026-08-27 16:17:18 -04:00 Compare
mysticalsoap deleted branch json-persistence 2026-08-27 16:49:10 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
architecture
Structural change, not a local fix

  
bug
Defect in existing behaviour

  
cleanup
Tidying with no behaviour change

  
docs
Documentation and repo standards

  
packaging
distro packages and their build tooling

  
security
Vulnerability or hardening

  
testing
Test coverage or infrastructure

  
upstream
Needs action on someone else's project

  
ux
Interface behaviour and clarity

No labels
architecture
bug
cleanup
docs
packaging
security
testing
upstream
ux
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
mysticalsoap/aqomui!173
No description provided.