From 2273fda48dcb4521fff660b29365d4f4787460d6 Mon Sep 17 00:00:00 2001 From: m5r Date: Fri, 23 Oct 2026 08:50:01 +0200 Subject: [PATCH] web: a manual blocklist refresh publishes once, since refreshAll already ends in its own reload --- CHANGELOG.md | 6 ++++++ specs/milestone-8.md | 4 ++-- src/web/handlers/blocklists.zig | 15 ++++++--------- src/web/web_integration_test.zig | 26 ++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e922bd5..e915c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to nxdns are recorded here. The format follows [Keep a Chang Sections are written by hand. Nothing here is generated from commit messages: the point of the file is to say what changed for an operator, which a commit subject rarely does. +## [Unreleased] + +### Fixed + +- **Updating the blocklists by hand builds them once.** "Update now" in the web interface, or `POST /api/blocklists/update`, rebuilt and published the blocklists a second time right after the refresh had already done so, which cost a second full build and advanced the snapshot generation twice. The scheduled refresh was not affected. + ## [0.0.25] - 2026-10-22 ### Fixed diff --git a/specs/milestone-8.md b/specs/milestone-8.md index 30e2db7..96ad630 100644 --- a/specs/milestone-8.md +++ b/specs/milestone-8.md @@ -25,7 +25,7 @@ Ground truth: PLAN.md:610-612 (phase text), :537-550 (endpoints), §3.11/§12.1/ > contract in this ruling is unchanged. 10. **Repo layer**: every list row the API serves carries its row id; each mutated resource gains `getX(db, id)`, `updateX(db, id, item)`, `deleteX(db, id)` (strict: 0 rows touched → error.NotFound), written in the house repo idiom with prepared statements. Existing import-path functions stay frozen. 11. **`GET /api/queries`**: keyset pagination `?limit` (default 100, max 1000) + `?before=`, ordered id DESC; filters `domain=` (substring), `client=` (exact), `blocked=` (bool), `since=`/`until=` (unix seconds). Response `{"queries":[...], "next_before": |null}`. Row fields: id, ts, domain, client_ip, qtype, blocked, block_reason, response_time_us, cache_hit, upstream. -12. **Live vs restart**: mutations to rules, blocklists, groups, group-sources, clients and client-prefixes call `Manager.reload(io)` and take effect live (the handler's next `acquire` sees the new snapshot). Local records and forward zones ALSO apply live: a new `src/server/local_tables.zig` RCU holder (acquire/release + swap, mirroring the manager's pattern at small scale) owns `{records, zones}`; `Handler` reads through it instead of `*const` fields; the local-records/forward-zones handlers rebuild and swap. Upstream mutations and everything in `/api/settings` are restart-required. `POST /api/blocklists/update` = `Manager.refreshAll` then reload, 202 with the status snapshot. +12. **Live vs restart**: mutations to rules, blocklists, groups, group-sources, clients and client-prefixes call `Manager.reload(io)` and take effect live (the handler's next `acquire` sees the new snapshot). Local records and forward zones ALSO apply live: a new `src/server/local_tables.zig` RCU holder (acquire/release + swap, mirroring the manager's pattern at small scale) owns `{records, zones}`; `Handler` reads through it instead of `*const` fields; the local-records/forward-zones handlers rebuild and swap. Upstream mutations and everything in `/api/settings` are restart-required. `POST /api/blocklists/update` = `Manager.refreshAll`, which ends in its own reload, 202 with the status snapshot. Amended 2026-10-23: this ruling first said "`Manager.refreshAll` then reload", but milestone 5 already made `refreshAll` end in one reload, so every manual refresh built and published the same snapshot twice. 13. **Stats grammar**: `period=1h|24h|7d|30d` (else 400). `/api/stats` returns totals {queries, blocked, cached, clients (distinct), avg_response_time_us} for the period. `/api/stats/timeseries` buckets: 1h→60×1m, 24h→48×30m, 7d→168×1h, 30d→120×6h; UTC; each bucket {ts, queries, blocked, cached}. SQL aggregates over query_log on the web task's own read connection. 14. **`GET /api/lookup?domain=&group_id=`** (group optional, default group when absent): normalizes, then reports the full pipeline view: `{domain, group_id, local_records: bool, forward_zone: |null, blocked, reason, matched, source_url|null, safe_search_rewrite: |null}` via `snapshot.evaluate` + records/zones lookups. Null snapshot → 503 `{"error":"no snapshot loaded"}`. 15. **Pause API**: `GET /api/pause` → `{"paused": bool, "until": |null}` (null while unpaused OR indefinite — disambiguated by `paused`). `POST /api/pause` body `{"paused": true, "duration_seconds": }` or `{"paused": false}`. @@ -178,7 +178,7 @@ Every handler is split into a pure core plus a thin `handle`, so the decisions a Owns: `src/web/handlers/` `groups.zig`, `blocklists.zig`, `rules.zig`, `local.zig`, `clients.zig`, `settings.zig`, `pause.zig`, `auth.zig` (login/logout), plus `src/server/local_tables.zig` (new, ruling 12) and the `src/server/handler.zig` edit to read records/zones through it (second mechanical handler change; coordinate: W5 already edits handler.zig — W7 runs AFTER W5 lands; sequence inside wave 2). - local_tables.zig: `LocalTables { lock: std.Io.RwLock, records, zones }` with `acquire/release` (shared) and `swap(io, new_records, new_zones)` (exclusive; frees the old under the lock after swap — no reader can hold across queries since acquire/release brackets each query). Handler: acquire in handle, release on every exit (same single defer discipline as the snapshot). -- Handlers: rulings 9, 12, 15, 16, 17-18 behaviors; every mutation validates (shared validators — reuse validate.zig section rules by constructing the candidate row set; where validate.zig only does whole-config, extract the per-section check it already has — validate.zig is NOT in your ownership: if extraction is needed, report it and use a local check this milestone), writes via W2 repos on state's config connection, then reload/rebuild per ruling 12. blocklists/update → refreshAll + reload, 202 + status. settings PUT: merge, validate whole config, write keys, clear sessions on hash change. +- Handlers: rulings 9, 12, 15, 16, 17-18 behaviors; every mutation validates (shared validators — reuse validate.zig section rules by constructing the candidate row set; where validate.zig only does whole-config, extract the per-section check it already has — validate.zig is NOT in your ownership: if extraction is needed, report it and use a local check this milestone), writes via W2 repos on state's config connection, then reload/rebuild per ruling 12. blocklists/update → refreshAll (it reloads itself), 202 + status. settings PUT: merge, validate whole config, write keys, clear sessions on hash change. - Tests: per handler with in-memory dbs and a real Manager where cheap, else assert repo effects + reload-called flag via a seam (a `reload_fn` pointer on WebState set by W9; tests inject a counter). Acceptance: fmt/ast clean; temp-root green; phase7 integration still green. diff --git a/src/web/handlers/blocklists.zig b/src/web/handlers/blocklists.zig index 60ea8e1..da3e574 100644 --- a/src/web/handlers/blocklists.zig +++ b/src/web/handlers/blocklists.zig @@ -4,9 +4,9 @@ //! an operator edits, and the counters the refresh writes ride along in the //! read shape so the UI can show a list's size next to its url (ruling 9). //! -//! `POST /api/blocklists/update` runs `Manager.refreshAll` and then the reload -//! seam, and answers 202 with the status of every source (ruling 12). The -//! refresh downloads and compiles before the response is written: 202 is +//! `POST /api/blocklists/update` runs `Manager.refreshAll`, which publishes its +//! own snapshot, and answers 202 with the status of every source (ruling 12). +//! The refresh downloads and compiles before the response is written: 202 is //! "accepted and done as far as this connection is concerned", and the status //! table in the body is what tells the operator which sources actually landed. //! @@ -153,11 +153,9 @@ fn pruneFiles(state: *server.WebState, io: std.Io) void { }; } -/// Refreshes every enabled source, then applies the result (ruling 12). -/// -/// `refreshAll` already ends in the manager's own reload; the seam is called -/// too, because it is how the composition root learns that a change landed and -/// the only reload a test can observe. +/// Refreshes every enabled source (ruling 12). The reload seam is not called: +/// `refreshAll` ends in the manager's own reload, so a second one would build +/// and publish the same snapshot again. fn applyRefresh(state: *server.WebState, io: std.Io, out: []manager_mod.SourceStatus) union(enum) { statuses: usize, fail: Failure, @@ -176,7 +174,6 @@ fn applyRefresh(state: *server.WebState, io: std.Io, out: []manager_mod.SourceSt return .{ .fail = .{ .internal = error.Unexpected } }; }, }; - if (mutations.reload(state, io)) |failure| return .{ .fail = failure }; return .{ .statuses = manager.statusSnapshot(io, out) }; } diff --git a/src/web/web_integration_test.zig b/src/web/web_integration_test.zig index 443796f..3cacb04 100644 --- a/src/web/web_integration_test.zig +++ b/src/web/web_integration_test.zig @@ -3212,6 +3212,32 @@ test "W10 a rule mutation reloads the snapshot and the change is live" { try bounded(env.io(), default_budget, mutationReloads, .{ env.io(), env }); } +fn refreshPublishesOnce(io: std.Io, env: *Env) anyerror!void { + const before = try env.generation(); + + var conn: Conn = undefined; + try conn.connect(io, env.addr); + defer conn.close(io); + + var body_buf: [4096]u8 = undefined; + try conn.request("POST", "/api/blocklists/update", null, null); + const response = try conn.receive(&body_buf); + try testing.expectEqual(@as(u16, 202), response.status); + try testing.expectEqualStrings("{\"sources\":[]}", response.body); + + try testing.expectEqual(before + 1, try env.generation()); +} + +test "W10 a manual refresh publishes exactly one generation" { + if (!build_options.integration) return error.SkipZigTest; + + const gpa = testing.allocator; + var env = try Env.create(gpa, .{}); + defer env.destroy(); + + try bounded(env.io(), default_budget, refreshPublishesOnce, .{ env.io(), env }); +} + // --------------------------------------------------------------------------- // deleting a source takes its compiled files with it (m13 ruling F-f) // ---------------------------------------------------------------------------