diff --git a/web/service/inbound.go b/web/service/inbound.go index da690bfa..35e04686 100644 --- a/web/service/inbound.go +++ b/web/service/inbound.go @@ -1879,9 +1879,16 @@ func (s *InboundService) addClientTraffic(tx *gorm.DB, traffics []*xray.ClientTr emails = append(emails, traffic.Email) } dbClientTraffics := make([]*xray.ClientTraffic, 0, len(traffics)) + // Match purely by email. client_traffics is email-keyed (one shared row per + // email regardless of how many inbounds the client is attached to), and these + // emails come from the local xray's report, so they always belong to a client + // attached to a local inbound. The old `inbound_id NOT IN (node inbounds)` + // filter dropped the local traffic of a client attached to both a node and the + // mother inbound whenever the node inbound happened to be attached first — its + // shared row then carried the node inbound's id (AddClientStat uses OnConflict + // DoNothing and never refreshes it), so the local poll skipped it entirely. err = tx.Model(xray.ClientTraffic{}). - Where("email IN (?) AND inbound_id NOT IN (?)", emails, - tx.Model(&model.Inbound{}).Select("id").Where("node_id IS NOT NULL")). + Where("email IN (?)", emails). Find(&dbClientTraffics).Error if err != nil { return err diff --git a/web/service/inbound_client_traffic_test.go b/web/service/inbound_client_traffic_test.go index f75c3f8a..5a2bbca2 100644 --- a/web/service/inbound_client_traffic_test.go +++ b/web/service/inbound_client_traffic_test.go @@ -10,14 +10,20 @@ import ( "github.com/mhsanaei/3x-ui/v3/xray" ) -// TestAddClientTraffic_MatchesDespiteStaleInboundId reproduces the production bug where -// client_traffics rows survive an inbound delete+recreate with a stale inbound_id (the -// shared-by-email row keeps the deleted inbound's id, and AddClientStat's OnConflict- -// DoNothing never refreshes it). The old `inbound_id IN (local inbounds)` filter dropped -// those rows, so local traffic and online status stopped updating. The fix matches by -// email and only excludes rows owned by a node inbound, so a stale local row is still -// updated while a genuine node-owned row is left untouched. -func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) { +// TestAddClientTraffic_MatchesByEmail covers two scenarios that share one fix: +// client_traffics is keyed by email (one shared row per email no matter how many +// inbounds the client is attached to), so local traffic must be applied by email +// regardless of which inbound_id the row happens to carry. +// +// - staleEmail: the row points at an inbound id that no longer exists (a deleted +// earlier incarnation, AddClientStat's OnConflict-DoNothing never refreshes it). +// - dualEmail: the client is attached to both a node inbound and the mother inbound, +// but the node inbound was attached first, so the shared row carries the node +// inbound's id (issue #4921). The old `inbound_id NOT IN (node inbounds)` filter +// dropped this client's local traffic, leaving it stuck at zero and offline. +// +// Both must have their local traffic counted. +func TestAddClientTraffic_MatchesByEmail(t *testing.T) { dbDir := t.TempDir() t.Setenv("XUI_DB_FOLDER", dbDir) if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil { @@ -27,11 +33,9 @@ func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) { db := database.GetDB() - const localEmail = "local-user" - const nodeEmail = "node-user" + const staleEmail = "stale-user" + const dualEmail = "dual-user" - // A local inbound exists, but the local client's traffic row points at an inbound id - // that no longer exists (a deleted earlier incarnation) — the stale-pointer scenario. localInbound := &model.Inbound{UserId: 1, Tag: "local-in", Enable: true, Port: 40001, Protocol: model.VLESS} if err := db.Create(localInbound).Error; err != nil { t.Fatalf("create local inbound: %v", err) @@ -42,39 +46,44 @@ func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) { t.Fatalf("create node inbound: %v", err) } - if err := db.Create(&xray.ClientTraffic{InboundId: 9999, Email: localEmail, Enable: true}).Error; err != nil { - t.Fatalf("create stale local client_traffics: %v", err) + if err := db.Create(&xray.ClientTraffic{InboundId: 9999, Email: staleEmail, Enable: true}).Error; err != nil { + t.Fatalf("create stale client_traffics: %v", err) } - if err := db.Create(&xray.ClientTraffic{InboundId: nodeInbound.Id, Email: nodeEmail, Enable: true}).Error; err != nil { - t.Fatalf("create node client_traffics: %v", err) + // Attached to both inbounds, but the node inbound won the OnConflict so the + // shared row is owned by the node inbound id. + if err := db.Create(&xray.ClientTraffic{InboundId: nodeInbound.Id, Email: dualEmail, Enable: true}).Error; err != nil { + t.Fatalf("create dual client_traffics: %v", err) } svc := InboundService{} err := svc.addClientTraffic(db, []*xray.ClientTraffic{ - {Email: localEmail, Up: 10, Down: 20}, - {Email: nodeEmail, Up: 30, Down: 40}, + {Email: staleEmail, Up: 10, Down: 20}, + {Email: dualEmail, Up: 30, Down: 40}, }) if err != nil { t.Fatalf("addClientTraffic: %v", err) } - var local xray.ClientTraffic - if err := db.Model(xray.ClientTraffic{}).Where("email = ?", localEmail).First(&local).Error; err != nil { - t.Fatalf("reload local row: %v", err) + var stale xray.ClientTraffic + if err := db.Model(xray.ClientTraffic{}).Where("email = ?", staleEmail).First(&stale).Error; err != nil { + t.Fatalf("reload stale row: %v", err) } - if local.Up != 10 || local.Down != 20 { - t.Errorf("stale-pointer local row not updated: up=%d down=%d, want 10/20", local.Up, local.Down) + if stale.Up != 10 || stale.Down != 20 { + t.Errorf("stale-pointer row not updated: up=%d down=%d, want 10/20", stale.Up, stale.Down) } - if local.LastOnline == 0 { - t.Errorf("stale-pointer local row LastOnline not set") + if stale.LastOnline == 0 { + t.Errorf("stale-pointer row LastOnline not set") } - var node xray.ClientTraffic - if err := db.Model(xray.ClientTraffic{}).Where("email = ?", nodeEmail).First(&node).Error; err != nil { - t.Fatalf("reload node row: %v", err) + var dual xray.ClientTraffic + if err := db.Model(xray.ClientTraffic{}).Where("email = ?", dualEmail).First(&dual).Error; err != nil { + t.Fatalf("reload dual row: %v", err) } - if node.Up != 0 || node.Down != 0 { - t.Errorf("node-owned row should not be touched by local traffic: up=%d down=%d, want 0/0", node.Up, node.Down) + if dual.Up != 30 || dual.Down != 40 { + t.Errorf("node-owned row not updated by local traffic (issue #4921): up=%d down=%d, want 30/40", dual.Up, dual.Down) + } + if dual.LastOnline == 0 { + t.Errorf("node-owned row LastOnline not set (client stayed offline)") } }