Files
3x-ui/internal/web/service/inbound_migration_test.go
Mr. Nickson ff954ec48c fix: stop deleting client_traffics for detached-but-alive clients (#6110)
* fix: stop deleting client_traffics for detached-but-alive clients

MigrationRemoveOrphanedTraffics keyed "orphaned" off presence in some
inbound's settings.clients[] JSON, a definition that predates #4469's
standalone clients table. ClientService.Detach intentionally keeps a
client's traffic row when it drops its last inbound attachment (so it
can be re-attached later without losing stats/expiry), but that client
has no entry in any inbound's JSON anymore - so every x-ui migrate run
or backup restore deleted its traffic row anyway, even though the
client itself was untouched and still listed. Scope the query to the
clients table instead, which is the function's actual intent.

Separately, frontend/src/hooks/useClients.ts recomputed the clients
summary from the client_stats WS snapshot as soon as it arrived, even
when that snapshot held fewer rows than the server's own total (e.g.
exactly the gap above, or any other client with no client_traffics
row). The recompute can only bucket the clients it was given, so the
missing ones silently fell out of every bucket while the headline
total still counted them - the Ended/Disabled cards read 0 and their
hover lists were empty even though the table below listed those rows,
leaving the Filter drawer as the only way to reach them. Extracted the
decision into pickClientsSummary and added the guard: fall back to the
server summary (built from the clients table, always sums to total)
whenever the snapshot doesn't cover every client.

Fixes #6102.

* fix: union both keep-sets instead of replacing (review feedback)

Address the automated review on this PR: switching
MigrationRemoveOrphanedTraffics to key solely off the clients table
traded the original bug for a worse one. The one-shot ClientsTable
seeder (internal/database/db.go) skips a client it fails to unmarshal
and never retries, so a client still live in an inbound's
settings.clients[] JSON can have no clients row at all - the new
predicate deleted its traffic row too, and an empty clients table
would have emptied client_traffics outright. Union both keep-sets: a
row survives if it's referenced by either the clients table or any
inbound's JSON, and is removed only when it's in neither.

Log the delete's outcome instead of discarding it silently, since a
whole-table wipe would otherwise leave no trace.

Rewrote the migration test as a table of all four combinations, driven
through real ClientService calls (SyncInbound, Detach) rather than
hand-built rows wherever a real path produces the state, so it tracks
actual behavior instead of an assumption about it. Added the missing
case the review flagged: a client live in JSON only, with no clients
row, must survive.

Also stripped the // comments this PR had added - CLAUDE.md states
committed Go/TS carries none, which the review separately flagged.
2026-07-28 22:14:01 +02:00

277 lines
11 KiB
Go

package service
import (
"path/filepath"
"strings"
"testing"
"github.com/mhsanaei/3x-ui/v3/internal/database"
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
"github.com/mhsanaei/3x-ui/v3/internal/xray"
)
// TestMigrationRequirements_BackfillsClientTrafficsWithMultiDomainInbound guards the
// PostgreSQL fix where the externalProxy detection query (executed via .Scan) errored on
// json_extract and rolled back the whole transaction — including the client_traffics
// backfill at inbound.go:3093-3106, leaving clients with no traffic rows. A MultiDomain
// inbound is present so that query returns rows and the function runs to completion; both
// the backfill and the MultiDomain→ExternalProxy migration must then commit.
func TestMigrationRequirements_BackfillsClientTrafficsWithMultiDomainInbound(t *testing.T) {
dbDir := t.TempDir()
t.Setenv("XUI_DB_FOLDER", dbDir)
if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil {
t.Fatalf("InitDB: %v", err)
}
t.Cleanup(func() { _ = database.CloseDB() })
db := database.GetDB()
const backfillEmail = "needsbackfill@example.com"
const uid = "ce8d33df-3a64-4f10-8f9b-91c3a8e0c010"
// Inbound A: a client present only in settings.clients, with no client_traffics row.
clientInbound := &model.Inbound{
UserId: 1,
Tag: "a-tag",
Enable: true,
Port: 30001,
Protocol: model.VLESS,
Settings: `{"clients":[{"email":"` + backfillEmail + `","id":"` + uid + `","enable":true}]}`,
StreamSettings: `{"network":"tcp","security":"none"}`,
}
if err := db.Create(clientInbound).Error; err != nil {
t.Fatalf("create client inbound: %v", err)
}
// Inbound B: a legacy MultiDomain inbound whose tag carries the 0.0.0.0: prefix.
// Its presence makes the externalProxy query return rows, so the function does not
// early-return and reaches the tag-cleanup statement.
multiDomainInbound := &model.Inbound{
UserId: 1,
Tag: "inbound-0.0.0.0:30002",
Enable: true,
Port: 30002,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`,
}
if err := db.Create(multiDomainInbound).Error; err != nil {
t.Fatalf("create multidomain inbound: %v", err)
}
var before int64
if err := db.Model(xray.ClientTraffic{}).Count(&before).Error; err != nil {
t.Fatalf("count client_traffics before: %v", err)
}
if before != 0 {
t.Fatalf("expected no client_traffics before migration, got %d", before)
}
svc := InboundService{}
svc.MigrationRequirements()
// The backfill must have committed: the settings-only client now owns a row.
// Before the fix this was rolled back whenever the externalProxy detection query
// errored (it does on Postgres via json_extract), so the MultiDomain inbound below
// is deliberately present to make that query return rows and run to completion.
var ct xray.ClientTraffic
if err := db.Model(xray.ClientTraffic{}).Where("email = ?", backfillEmail).First(&ct).Error; err != nil {
t.Fatalf("client_traffics row not backfilled for %s: %v", backfillEmail, err)
}
// The MultiDomain→ExternalProxy migration must have committed too: the detection
// query ran (.Scan executes it) and the loop rewrote the inbound's streamSettings.
var refreshed model.Inbound
if err := db.First(&refreshed, multiDomainInbound.Id).Error; err != nil {
t.Fatalf("reload multidomain inbound: %v", err)
}
if !strings.Contains(refreshed.StreamSettings, "externalProxy") {
t.Errorf("MultiDomain migration did not commit; streamSettings = %q", refreshed.StreamSettings)
}
}
// TestMigrationRequirements_CleansLegacyZeroAddrTag guards the legacy tag cleanup that
// strips the auto-generated "0.0.0.0:" prefix. The inbound is MultiDomain TLS so the
// externalProxy detection query returns rows and the cleanup is reached (it early-returns
// at len(externalProxy)==0 otherwise). The cleanup must use tx.Exec, not tx.Raw, which
// only builds a non-SELECT statement without running it.
func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) {
dbDir := t.TempDir()
t.Setenv("XUI_DB_FOLDER", dbDir)
if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil {
t.Fatalf("InitDB: %v", err)
}
t.Cleanup(func() { _ = database.CloseDB() })
db := database.GetDB()
legacy := &model.Inbound{
UserId: 1,
Tag: "inbound-0.0.0.0:30002",
Enable: true,
Port: 30002,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`,
}
if err := db.Create(legacy).Error; err != nil {
t.Fatalf("create legacy inbound: %v", err)
}
svc := InboundService{}
svc.MigrationRequirements()
var got model.Inbound
if err := db.First(&got, legacy.Id).Error; err != nil {
t.Fatalf("reload inbound: %v", err)
}
if got.Tag != "inbound-30002" {
t.Fatalf("legacy 0.0.0.0: tag not stripped: got %q, want %q", got.Tag, "inbound-30002")
}
}
func TestMigrationRemoveOrphanedTraffics(t *testing.T) {
setupConflictDB(t)
db := database.GetDB()
clientSvc := &ClientService{}
inboundSvc := &InboundService{}
const attachedEmail = "attached@example.com"
attachedClient := model.Client{Email: attachedEmail, ID: "11111111-1111-1111-1111-111111111111", SubID: attachedEmail, Enable: true}
attachedIb := mkInbound(t, 30003, model.VLESS, clientsSettings(t, []model.Client{attachedClient}))
if err := clientSvc.SyncInbound(nil, attachedIb.Id, []model.Client{attachedClient}); err != nil {
t.Fatalf("seed attached client: %v", err)
}
mkTraffic(t, attachedIb.Id, attachedEmail, 0, 0, 0, 0, true)
const detachedEmail = "detached@example.com"
detachedClient := model.Client{Email: detachedEmail, ID: "22222222-2222-2222-2222-222222222222", SubID: detachedEmail, Enable: true}
detachedIb := mkInbound(t, 30004, model.VLESS, clientsSettings(t, []model.Client{detachedClient}))
if err := clientSvc.SyncInbound(nil, detachedIb.Id, []model.Client{detachedClient}); err != nil {
t.Fatalf("seed detached client: %v", err)
}
mkTraffic(t, detachedIb.Id, detachedEmail, 123, 456, 0, 0, true)
detachedRec := lookupClientRecord(t, detachedEmail)
if _, err := clientSvc.Detach(inboundSvc, detachedRec.Id, []int{detachedIb.Id}); err != nil {
t.Fatalf("Detach: %v", err)
}
const jsonOnlyEmail = "jsononly@example.com"
jsonOnlyClient := model.Client{Email: jsonOnlyEmail, ID: "33333333-3333-3333-3333-333333333333", SubID: jsonOnlyEmail, Enable: true}
jsonOnlyIb := mkInbound(t, 30005, model.VLESS, clientsSettings(t, []model.Client{jsonOnlyClient}))
mkTraffic(t, jsonOnlyIb.Id, jsonOnlyEmail, 0, 0, 0, 0, true)
const trulyOrphanedEmail = "deleted@example.com"
mkTraffic(t, attachedIb.Id, trulyOrphanedEmail, 0, 0, 0, 0, true)
inboundSvc.MigrationRemoveOrphanedTraffics()
cases := []struct {
name string
email string
want int64
}{
{"attached, in clients table and JSON", attachedEmail, 1},
{"detached-but-alive, in clients table only", detachedEmail, 1},
{"seeder-skipped-but-live, in JSON only", jsonOnlyEmail, 1},
{"truly orphaned, in neither", trulyOrphanedEmail, 0},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var got int64
if err := db.Model(xray.ClientTraffic{}).Where("email = ?", c.email).Count(&got).Error; err != nil {
t.Fatalf("count client_traffics for %s: %v", c.email, err)
}
if got != c.want {
t.Errorf("client_traffics count for %s: got %d, want %d", c.email, got, c.want)
}
})
}
}
func TestMigrationRequirements_NormalizesShareAddressFields(t *testing.T) {
setupConflictDB(t)
db := database.GetDB()
invalidStrategy := &model.Inbound{
UserId: 1,
Tag: "invalid-share-strategy",
Enable: true,
Port: 31001,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"network":"tcp","security":"none"}`,
}
paddedStrategy := &model.Inbound{
UserId: 1,
Tag: "padded-share-strategy",
Enable: true,
Port: 31002,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"network":"tcp","security":"none"}`,
}
invalidAddress := &model.Inbound{
UserId: 1,
Tag: "invalid-share-address",
Enable: true,
Port: 31003,
Protocol: model.VLESS,
Settings: `{"clients":[]}`,
StreamSettings: `{"network":"tcp","security":"none"}`,
}
if err := db.Create(invalidStrategy).Error; err != nil {
t.Fatalf("create invalid strategy inbound: %v", err)
}
if err := db.Create(paddedStrategy).Error; err != nil {
t.Fatalf("create padded strategy inbound: %v", err)
}
if err := db.Create(invalidAddress).Error; err != nil {
t.Fatalf("create invalid address inbound: %v", err)
}
if err := db.Model(&model.Inbound{}).Where("id = ?", invalidStrategy.Id).Updates(map[string]any{
"share_addr_strategy": " auto ",
"share_addr": " edge.example.com ",
}).Error; err != nil {
t.Fatalf("seed invalid share fields: %v", err)
}
if err := db.Model(&model.Inbound{}).Where("id = ?", paddedStrategy.Id).Updates(map[string]any{
"share_addr_strategy": " listen ",
"share_addr": " 10.0.0.1 ",
}).Error; err != nil {
t.Fatalf("seed padded share fields: %v", err)
}
if err := db.Model(&model.Inbound{}).Where("id = ?", invalidAddress.Id).Updates(map[string]any{
"share_addr_strategy": "custom",
"share_addr": "edge.example.com:8443",
}).Error; err != nil {
t.Fatalf("seed invalid address share fields: %v", err)
}
svc := InboundService{}
svc.MigrationRequirements()
var gotInvalid model.Inbound
if err := db.First(&gotInvalid, invalidStrategy.Id).Error; err != nil {
t.Fatalf("reload invalid strategy inbound: %v", err)
}
if gotInvalid.ShareAddrStrategy != "node" || gotInvalid.ShareAddr != "edge.example.com" {
t.Fatalf("invalid share fields = (%q, %q), want (node, edge.example.com)", gotInvalid.ShareAddrStrategy, gotInvalid.ShareAddr)
}
var gotPadded model.Inbound
if err := db.First(&gotPadded, paddedStrategy.Id).Error; err != nil {
t.Fatalf("reload padded strategy inbound: %v", err)
}
if gotPadded.ShareAddrStrategy != "listen" || gotPadded.ShareAddr != "10.0.0.1" {
t.Fatalf("padded share fields = (%q, %q), want (listen, 10.0.0.1)", gotPadded.ShareAddrStrategy, gotPadded.ShareAddr)
}
var gotInvalidAddress model.Inbound
if err := db.First(&gotInvalidAddress, invalidAddress.Id).Error; err != nil {
t.Fatalf("reload invalid address inbound: %v", err)
}
if gotInvalidAddress.ShareAddrStrategy != "node" || gotInvalidAddress.ShareAddr != "" {
t.Fatalf("invalid address share fields = (%q, %q), want (node, empty)", gotInvalidAddress.ShareAddrStrategy, gotInvalidAddress.ShareAddr)
}
}