Most tests opened a throwaway panel DB with database.InitDB, which runs the
full AutoMigrate + seed on an empty file every time: ~230ms, and ~850ms under
-race because GORM's reflection-heavy migration is what the detector slows
most. internal/web/service does this in ~550 of its 830 tests, so the CI race
job spent ~10 of its ~14.6 minutes re-migrating empty databases.
internal/database/dbtest.InitDB migrates once per test process, then hands
each test its own copy of that file (~130ms under -race) and registers the
CloseDB cleanup. The copy then goes through InitDB like a panel restart, so
every test still starts from the state a fresh install has. Tests that reopen
an existing file, migrate a hand-built legacy DB or target Postgres keep
calling database.InitDB.
Locally under -race: internal/web/service 626s (last CI run) -> 114s,
internal/sub 246s -> 35s.
* feat(clients): preserve traffic counters in portable export/import
ExportAll now attaches client_traffics up/down (plus resetCount and
last-seen fields) on each portable payload, and ImportClients restores
them only for newly created emails so skipped/existing clients keep
their live counters. Fixes#5858.
* fix(clients): restore imported traffic only onto rows the import created
Review of the portable-traffic export/import (#5858) found four defects:
- An orphan's restored row was hand-built, dropping reset_weekday and
forcing enable=true; a row kept by a keepTraffic delete kept the old
client's limits. depletedClientsClause then matched a weekly-renewing
over-quota orphan and DelDepleted deleted it. Orphan rows now go
through AddClientStat, whose upsert refreshes config and keeps counters,
so the unused traffic.total field is dropped from the export.
- Created clients were inferred from Skipped emails, so a duplicate email
in the file left the created copy with zero counters. bulkCreate now
reports which payloads inserted a record, and only those are restored.
- Each client took its own serialized-writer commit: 2000 clients spent
3.66s instead of 0.52s. Counters now apply in batched transactions
(0.51s).
- importClients discarded needRestart when the late restore step failed
after clients were committed; it now flags and notifies first, as
create already does.
The /clients/export and /clients/import API docs now describe traffic.
* fix(groups): keep imported traffic out of group totals
Group totals keep a deleted client's usage (#5675), and the portable
import restores that same usage onto the re-created client. Export,
delete, re-import therefore counted it twice in ListGroups, and a fresh
panel showed the migrated usage as consumption of its groups.
Restored counters are usage from before the import, so the import now
shifts each group's baseline up by what it restored, in the same
transaction. A group total no longer moves at import time; only traffic
consumed afterwards counts. The baseline shift reuses the #5675 helper,
now signed.
---------
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
63b46cd6 made a multi-inbound client op apply its inbounds concurrently and
stop aborting at the first failure, so an error can now come back together
with needRestart=true: the inbounds that succeeded committed real changes and
their Xray still needs the restart. That commit taught the two callers it
converted — create and attach — to read the flag before the error check.
d34ec97f then routed Update, Delete, Detach and DeleteByEmail's record-less
fallback through the same fanout but touched no caller, so on a master with
several nodes a partly-applied edit or delete returned (true, err) into a
handler that returned on err first. Xray was never flagged for the work that
landed and notifyClientsChanged never fired, so the running config kept
serving the pre-edit client set and every open panel showed stale rows until
something else happened to trigger a restart.
Read needRestart before the error check in update, delete and detach, and
broadcast on needRestart || err == nil — the same shape create and attach have
had since 63b46cd6. The predicate is a strict superset of the old err == nil,
and needRestart is only ever assigned after a runSerializedTx commit, so it
firing genuinely means something landed.
The three handlers are pinned by a new controller test each: one client on two
inbounds, the second one's settings JSON corrupted so the op commits on one and
fails on the other, asserting both the success:false response and the restart
flag. All three fail without the change.
The API docs for update, del and detach now describe the partial-application
contract, as add and attach already did. Detach ends at the fanout so every one
of its errors carries the inbound prefix; update and delete write the client
record afterwards, and a failure there is reported without one.
MHSanaei