From 3af1afc53b78b56ada131437f399e69060a22a2f Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Fri, 12 Jun 2026 13:00:04 +0200 Subject: [PATCH] fix(inbound): avoid UNIQUE email constraint when importing inbounds that share clients Importing a second inbound whose clients overlap an already-imported inbound failed with "UNIQUE constraint failed: client_traffics.email". The import path carries exported ClientStats, and tx.Save(inbound) cascaded that has-many association as INSERTs whose ON CONFLICT targets only the primary key, so a shared email (already owning a row from the first import) tripped the global unique constraint. Omit the ClientStats association on save and insert the carried stats ourselves with the same OnConflict{email, DoNothing} guard AddClientStat already uses: new clients keep their imported counters, shared emails reuse the existing row. Then run an idempotent AddClientStat pass over all clients so any client present in settings but missing from the stats payload still gets a traffic row (else it would escape quota/expiry accounting), and propagate insert errors so the tx rolls back instead of committing a partial state. --- internal/web/service/inbound.go | 41 ++++-- .../inbound_import_shared_clients_test.go | 127 ++++++++++++++++++ 2 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 internal/web/service/inbound_import_shared_clients_test.go diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 58d719406..201071d4c 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -19,6 +19,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/xray" "gorm.io/gorm" + "gorm.io/gorm/clause" ) type InboundService struct { @@ -550,16 +551,40 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo } }() - err = tx.Save(inbound).Error - if err == nil { - if len(inbound.ClientStats) == 0 { - for _, client := range clients { - s.AddClientStat(tx, inbound.Id, &client) - } - } - } else { + // Omit the ClientStats has-many association: GORM's cascade would INSERT + // those rows with an ON CONFLICT target on the primary key only, which + // collides with the globally-unique client_traffics.email when an imported + // inbound carries clients that another inbound already created (e.g. + // importing two inbounds that share the same clients). We insert the stats + // ourselves below with the same email-conflict guard AddClientStat uses. + err = tx.Omit("ClientStats").Save(inbound).Error + if err != nil { return inbound, false, err } + // Imported stats first, so their traffic counters survive; emails that + // already own a (shared) row are skipped instead of tripping the unique + // constraint. + for i := range inbound.ClientStats { + if inbound.ClientStats[i].Email == "" { + continue + } + inbound.ClientStats[i].Id = 0 + inbound.ClientStats[i].InboundId = inbound.Id + if err = tx.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "email"}}, + DoNothing: true, + }).Create(&inbound.ClientStats[i]).Error; err != nil { + return inbound, false, err + } + } + // Then make sure every client has a stats row. AddClientStat is a no-op + // where one exists (including the rows just inserted), and fills the gap + // for clients an import payload didn't carry stats for. + for _, client := range clients { + if err = s.AddClientStat(tx, inbound.Id, &client); err != nil { + return inbound, false, err + } + } if err = s.clientService.SyncInbound(tx, inbound.Id, clients); err != nil { return inbound, false, err diff --git a/internal/web/service/inbound_import_shared_clients_test.go b/internal/web/service/inbound_import_shared_clients_test.go new file mode 100644 index 000000000..c5999cb40 --- /dev/null +++ b/internal/web/service/inbound_import_shared_clients_test.go @@ -0,0 +1,127 @@ +package service + +import ( + "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" +) + +// makeImportInbound builds an inbound shaped like the import payload: a clients +// JSON blob plus carried-over ClientStats (the exported traffic counters). The +// stats mirror what controller.importInbound feeds AddInbound after zeroing ids. +func makeImportInbound(tag string, port int, settings string, stats []xray.ClientTraffic) *model.Inbound { + for i := range stats { + stats[i].Id = 0 + stats[i].Enable = true + } + return &model.Inbound{ + UserId: 1, + Tag: tag, + Enable: true, + Listen: "0.0.0.0", + Port: port, + Protocol: model.VLESS, + StreamSettings: `{"network":"tcp"}`, + Settings: settings, + ClientStats: stats, + } +} + +// TestAddInbound_ImportTwoInboundsSharingClients reproduces the panel report: +// importing inbound #1 then inbound #2 when both carry the same clients (same +// email + subId) used to fail with "UNIQUE constraint failed: client_traffics.email". +// The shared email already owns a row from the first import, and the second +// inbound's ClientStats association tried to plain-INSERT it again. +func TestAddInbound_ImportTwoInboundsSharingClients(t *testing.T) { + setupConflictDB(t) + svc := &InboundService{} + + // Inbound #1: clients alice (shared) and bob (unique to #1). + settings1 := `{"clients":[` + + `{"id":"11111111-1111-1111-1111-111111111111","email":"alice","subId":"s-alice","enable":true},` + + `{"id":"22222222-2222-2222-2222-222222222222","email":"bob","subId":"s-bob","enable":true}` + + `],"decryption":"none","encryption":"none"}` + in1 := makeImportInbound("in-9101-tcp", 9101, settings1, []xray.ClientTraffic{ + {Email: "alice", Up: 100, Down: 200, Total: 1000}, + {Email: "bob", Up: 1, Down: 2, Total: 1000}, + }) + if _, _, err := svc.AddInbound(in1); err != nil { + t.Fatalf("import inbound #1: %v", err) + } + + // Inbound #2: clients alice (same email+subId as #1) and carol (unique to #2). + settings2 := `{"clients":[` + + `{"id":"11111111-1111-1111-1111-111111111111","email":"alice","subId":"s-alice","enable":true},` + + `{"id":"33333333-3333-3333-3333-333333333333","email":"carol","subId":"s-carol","enable":true}` + + `],"decryption":"none","encryption":"none"}` + in2 := makeImportInbound("in-9102-tcp", 9102, settings2, []xray.ClientTraffic{ + {Email: "alice", Up: 999, Down: 999, Total: 9999}, // would clobber the shared row if inserted + {Email: "carol", Up: 3, Down: 4, Total: 1000}, + }) + if _, _, err := svc.AddInbound(in2); err != nil { + t.Fatalf("import inbound #2 (the reported failure): %v", err) + } + + // One traffic row per distinct email — no duplicate "alice". + for _, tc := range []struct { + email string + want int64 + }{ + {"alice", 100}, // preserved from import #1, not clobbered by #2's 999 + {"bob", 1}, + {"carol", 3}, + } { + var rows []xray.ClientTraffic + if err := database.GetDB().Where("email = ?", tc.email).Find(&rows).Error; err != nil { + t.Fatalf("query %s: %v", tc.email, err) + } + if len(rows) != 1 { + t.Fatalf("email %q: got %d traffic rows, want exactly 1", tc.email, len(rows)) + } + if rows[0].Up != tc.want { + t.Fatalf("email %q: Up = %d, want %d (shared row should keep the first import's counters)", tc.email, rows[0].Up, tc.want) + } + } +} + +// TestAddInbound_ImportStatsMissingClientStillGetsTrafficRow covers an import +// payload whose clientStats doesn't cover every client in settings (older +// exports / hand-edited JSON): the uncovered client must still end up with a +// traffic row, or it would escape quota and expiry accounting. +func TestAddInbound_ImportStatsMissingClientStillGetsTrafficRow(t *testing.T) { + setupConflictDB(t) + svc := &InboundService{} + + settings := `{"clients":[` + + `{"id":"44444444-4444-4444-4444-444444444444","email":"dave","subId":"s-dave","enable":true,"totalGB":1000},` + + `{"id":"55555555-5555-5555-5555-555555555555","email":"erin","subId":"s-erin","enable":true,"totalGB":2000}` + + `],"decryption":"none","encryption":"none"}` + // Stats cover dave only; erin is missing. + in := makeImportInbound("in-9103-tcp", 9103, settings, []xray.ClientTraffic{ + {Email: "dave", Up: 7, Down: 8, Total: 1000}, + }) + if _, _, err := svc.AddInbound(in); err != nil { + t.Fatalf("import inbound: %v", err) + } + + var dave xray.ClientTraffic + if err := database.GetDB().Where("email = ?", "dave").First(&dave).Error; err != nil { + t.Fatalf("dave row: %v", err) + } + if dave.Up != 7 { + t.Fatalf("dave Up = %d, want 7 (imported counters preserved)", dave.Up) + } + + var erin xray.ClientTraffic + if err := database.GetDB().Where("email = ?", "erin").First(&erin).Error; err != nil { + t.Fatalf("erin must still get a traffic row despite missing from clientStats: %v", err) + } + if erin.Up != 0 || erin.Down != 0 { + t.Fatalf("erin counters = %d/%d, want zeroed", erin.Up, erin.Down) + } + if erin.Total != 2000 { + t.Fatalf("erin Total = %d, want 2000 (quota taken from client settings)", erin.Total) + } +}