From 63ffc083e4f7c4aacba658c20832873997a7d29b Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Mon, 28 Sep 2026 01:59:33 +0200 Subject: [PATCH] fix(clients): stop client ops from reverting a renewal committed mid-op Invariant: an operation on an inbound's clients writes back only what it changed, onto the settings as committed when it writes. Every client op (add, edit, delete, bulk adjust/detach/delete/set-enable) read the inbound outside the serial traffic writer and then tx.Save'd the whole row inside it. A traffic tick that committed in between - auto-renew re-enabling a client, the delayed-start conversion, a node adoption - was overwritten with the stale copy. A renewed neighbour ended up enable=false with its old expiry in settings while client_traffics said enabled, so the next runtime rebuild dropped a healthy client nobody had touched. The bulk ops then ran a full SyncInbound from those stale settings, copying enable=false into the client record too. Each site now commits through commitInboundClientSettings: inside the serialized tx it three-way merges the op's edit (read -> output, per client and per field) onto the committed settings and updates only the settings column, which also stops the stale up/down/enable inbound columns being written back. advancePushedInbound now records what the per-client push delivered rather than the merged settings, so the node's reconcile-skip fingerprint cannot claim a renewal it never received. --- internal/web/service/client_bulk.go | 10 +- internal/web/service/client_inbound_apply.go | 26 +- .../web/service/inbound_settings_commit.go | 141 ++++++++ .../inbound_settings_stale_write_test.go | 336 ++++++++++++++++++ 4 files changed, 497 insertions(+), 16 deletions(-) create mode 100644 internal/web/service/inbound_settings_commit.go create mode 100644 internal/web/service/inbound_settings_stale_write_test.go diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index e1855b7bc..f48fd02e9 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -795,6 +795,7 @@ func (s *ClientService) bulkAdjustInboundClients( } return res } + prevSettings := oldInbound.Settings oldInbound.Settings = string(newSettings) // A flow change rewrites the user's xray config, which the lightweight @@ -807,7 +808,7 @@ func (s *ClientService) bulkAdjustInboundClients( // Serialize against the traffic poll to avoid the cross-transaction // lock-order deadlock on inbounds/client_records (runSerializedTx). txErr := runSerializedTx(func(tx *gorm.DB) error { - if err := tx.Save(oldInbound).Error; err != nil { + if err := commitInboundClientSettings(tx, oldInbound, prevSettings); err != nil { return err } finalClients, gcErr := inboundSvc.GetClients(oldInbound) @@ -1112,6 +1113,7 @@ func (s *ClientService) bulkDelInboundClients( } return res } + prevSettings := oldInbound.Settings oldInbound.Settings = string(newSettings) foundList := make([]string, 0, len(foundEmails)) @@ -1181,7 +1183,7 @@ func (s *ClientService) bulkDelInboundClients( // Serialize against the traffic poll to avoid the cross-transaction // lock-order deadlock on inbounds/client_records (runSerializedTx). txErr := runSerializedTx(func(tx *gorm.DB) error { - if err := tx.Save(oldInbound).Error; err != nil { + if err := commitInboundClientSettings(tx, oldInbound, prevSettings); err != nil { return err } finalClients, err := inboundSvc.GetClients(oldInbound) @@ -1780,7 +1782,7 @@ func (s *ClientService) bulkSetEnableInboundClients(inboundSvc *InboundService, } txErr := runSerializedTx(func(tx *gorm.DB) error { - if e := tx.Save(oldInbound).Error; e != nil { + if e := commitInboundClientSettings(tx, oldInbound, prevSettings); e != nil { return e } finalClients, gcErr := inboundSvc.GetClients(oldInbound) @@ -1850,7 +1852,7 @@ func (s *ClientService) bulkSetEnableInboundClients(inboundSvc *InboundService, } } if !pushFailed { - advancePushedInbound(rt, prevSettings, oldInbound) + advancePushedInbound(rt, prevSettings, string(newSettings), oldInbound) } } diff --git a/internal/web/service/client_inbound_apply.go b/internal/web/service/client_inbound_apply.go index 9598d4653..fc242e207 100644 --- a/internal/web/service/client_inbound_apply.go +++ b/internal/web/service/client_inbound_apply.go @@ -39,16 +39,18 @@ func sameClientConfigExceptUpdatedAt(a, b map[string]any) bool { return aerr == nil && berr == nil && string(an) == string(bn) } -// advancePushedInbound advances the node's reconcile-skip fingerprint from the -// pre-edit settings to the saved ones after every per-client push succeeded. -func advancePushedInbound(rt runtime.Runtime, prevSettings string, ib *model.Inbound) { +// advancePushedInbound advances the node's reconcile-skip fingerprint to what the +// per-client pushes delivered, not the saved settings a traffic tick may have extended. +func advancePushedInbound(rt runtime.Runtime, prevSettings, pushedSettings string, ib *model.Inbound) { rem, ok := rt.(*runtime.Remote) if !ok { return } prev := *ib prev.Settings = prevSettings - rem.AdvancePushedInbound(&prev, ib) + pushed := *ib + pushed.Settings = pushedSettings + rem.AdvancePushedInbound(&prev, &pushed) } // delInboundClients removes several clients from a single inbound in one pass: @@ -184,7 +186,7 @@ func (s *ClientService) delInboundClients(inboundSvc *InboundService, inboundId } } } - if e := tx.Save(oldInbound).Error; e != nil { + if e := commitInboundClientSettings(tx, oldInbound, prevSettings); e != nil { return e } detached := make([]string, 0, len(targets)) @@ -249,7 +251,7 @@ func (s *ClientService) delInboundClients(inboundSvc *InboundService, inboundId } } if nodePush && !nodePushFailed { - advancePushedInbound(nodeRt, prevSettings, oldInbound) + advancePushedInbound(nodeRt, prevSettings, string(newSettings), oldInbound) } return needRestart, nil @@ -563,7 +565,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model return e } } - if e := tx.Save(oldInbound).Error; e != nil { + if e := commitInboundClientSettings(tx, oldInbound, prevSettings); e != nil { return e } if err := s.ApplyInboundClientDelta(tx, oldInbound.Id, addedClients, nil); err != nil { @@ -648,7 +650,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model } } if push { - advancePushedInbound(rt, prevSettings, oldInbound) + advancePushedInbound(rt, prevSettings, string(newSettings), oldInbound) } } @@ -980,7 +982,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo } } - if e := tx.Save(oldInbound).Error; e != nil { + if e := commitInboundClientSettings(tx, oldInbound, prevSettings); e != nil { return e } // Rename the client record in the same transaction as the settings JSON @@ -1074,7 +1076,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo if err1 != nil { logger.Warning("Error in updating client on", rt.Name(), ":", err1) } else { - advancePushedInbound(rt, prevSettings, oldInbound) + advancePushedInbound(rt, prevSettings, string(newSettings), oldInbound) } } } else { @@ -1184,7 +1186,7 @@ func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inbo return e } } - if e := tx.Save(oldInbound).Error; e != nil { + if e := commitInboundClientSettings(tx, oldInbound, prevSettings); e != nil { return e } if err := s.ApplyInboundClientDelta(tx, inboundId, nil, []string{email}); err != nil { @@ -1248,7 +1250,7 @@ func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inbo if err1 != nil { logger.Warning("Error in deleting client on", rt.Name(), ":", err1) } else { - advancePushedInbound(rt, prevSettings, oldInbound) + advancePushedInbound(rt, prevSettings, string(newSettings), oldInbound) } } } diff --git a/internal/web/service/inbound_settings_commit.go b/internal/web/service/inbound_settings_commit.go new file mode 100644 index 000000000..9ab1badd4 --- /dev/null +++ b/internal/web/service/inbound_settings_commit.go @@ -0,0 +1,141 @@ +package service + +import ( + "encoding/json" + "reflect" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" + + "gorm.io/gorm" +) + +// commitInboundClientSettings writes a client op's edit (base → ib.Settings) onto +// the settings committed now: a traffic tick after the op's read must survive. +func commitInboundClientSettings(tx *gorm.DB, ib *model.Inbound, base string) error { + var current []string + if err := tx.Model(&model.Inbound{}).Where("id = ?", ib.Id).Pluck("settings", ¤t).Error; err != nil { + return err + } + if len(current) == 1 && current[0] != base { + merged, err := rebaseClientSettings(base, ib.Settings, current[0]) + if err != nil { + return err + } + ib.Settings = merged + } + return tx.Model(&model.Inbound{}).Where("id = ?", ib.Id).Update("settings", ib.Settings).Error +} + +// rebaseClientSettings three-way merges settings JSON: every key and client field +// ours left as base had it takes current's value; clients are matched by email. +func rebaseClientSettings(base, ours, current string) (string, error) { + var baseM, oursM, curM map[string]any + for _, p := range []struct { + raw string + dst *map[string]any + }{{base, &baseM}, {ours, &oursM}, {current, &curM}} { + if err := json.Unmarshal([]byte(p.raw), p.dst); err != nil { + return "", err + } + } + out := mergeFields(baseM, oursM, curM) + baseClients, _ := baseM["clients"].([]any) + oursClients, _ := oursM["clients"].([]any) + curClients, _ := curM["clients"].([]any) + if _, has := oursM["clients"]; has { + out["clients"] = mergeClientLists(baseClients, oursClients, curClients) + } + b, err := json.MarshalIndent(out, "", " ") + if err != nil { + return "", err + } + return string(b), nil +} + +func mergeFields(base, ours, current map[string]any) map[string]any { + out := make(map[string]any, len(current)+len(ours)) + for k, v := range current { + out[k] = v + } + keys := make(map[string]struct{}, len(base)+len(ours)) + for k := range base { + keys[k] = struct{}{} + } + for k := range ours { + keys[k] = struct{}{} + } + for k := range keys { + bv, inBase := base[k] + ov, inOurs := ours[k] + if inBase == inOurs && reflect.DeepEqual(bv, ov) { + continue + } + if inOurs { + out[k] = ov + } else { + delete(out, k) + } + } + return out +} + +func clientEntryEmail(entry any) (map[string]any, string) { + m, ok := entry.(map[string]any) + if !ok { + return nil, "" + } + email, _ := m["email"].(string) + return m, email +} + +func indexClientsByEmail(list []any) map[string]map[string]any { + out := make(map[string]map[string]any, len(list)) + for _, entry := range list { + if m, email := clientEntryEmail(entry); email != "" { + out[email] = m + } + } + return out +} + +// mergeClientLists keeps ours' order. A client ours removed stays removed; one a +// concurrent writer added or removed keeps that change unless ours edited it. +func mergeClientLists(base, ours, current []any) []any { + baseBy := indexClientsByEmail(base) + curBy := indexClientsByEmail(current) + out := make([]any, 0, len(ours)+len(current)) + placed := make(map[string]struct{}, len(ours)) + for _, entry := range ours { + o, email := clientEntryEmail(entry) + if email == "" { + out = append(out, entry) + continue + } + placed[email] = struct{}{} + b, inBase := baseBy[email] + c, inCur := curBy[email] + switch { + case !inBase: + out = append(out, o) + case !inCur: + if !reflect.DeepEqual(b, o) { + out = append(out, o) + } + default: + out = append(out, mergeFields(b, o, c)) + } + } + for _, entry := range current { + c, email := clientEntryEmail(entry) + if email == "" { + continue + } + if _, done := placed[email]; done { + continue + } + if _, inBase := baseBy[email]; !inBase { + out = append(out, c) + } + } + return out +} diff --git a/internal/web/service/inbound_settings_stale_write_test.go b/internal/web/service/inbound_settings_stale_write_test.go new file mode 100644 index 000000000..17d71a494 --- /dev/null +++ b/internal/web/service/inbound_settings_stale_write_test.go @@ -0,0 +1,336 @@ +package service + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "strconv" + "testing" + "time" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" + "github.com/mhsanaei/3x-ui/v3/internal/web/runtime" + "github.com/mhsanaei/3x-ui/v3/internal/xray" + + "gorm.io/gorm" +) + +// commitTickBetweenReadAndWrite parks the serial writer, lets op read the +// inbound and queue its transaction, then commits tick ahead of that transaction. +func commitTickBetweenReadAndWrite(t *testing.T, tick func(tx *gorm.DB) error, op func()) { + t.Helper() + resetTrafficWriterForTest(t) + StartTrafficWriter() + + parked := make(chan struct{}) + release := make(chan struct{}) + tickErr := make(chan error, 1) + go func() { + tickErr <- submitTrafficWrite(func() error { + close(parked) + <-release + return database.GetDB().Transaction(tick) + }) + }() + <-parked + + opDone := make(chan struct{}) + go func() { + defer close(opDone) + op() + }() + waitTrafficWriterQueued(t) + close(release) + if err := <-tickErr; err != nil { + t.Fatalf("tick: %v", err) + } + <-opDone +} + +// seedRenewableNeighbour builds an inbound holding a healthy client X and a +// quota-disabled client Y whose auto-renew is due, as the traffic job sees them. +func seedRenewableNeighbour(t *testing.T, port int, nodeID *int) *model.Inbound { + t.Helper() + past := time.Now().Add(-time.Hour).UnixMilli() + future := time.Now().Add(30 * 24 * time.Hour).UnixMilli() + clients := []model.Client{ + {Email: "x@stale", ID: "aaaaaaaa-0000-0000-0000-00000000000a", SubID: "sub-x", Enable: true, ExpiryTime: future}, + {Email: "y@stale", ID: "aaaaaaaa-0000-0000-0000-00000000000b", SubID: "sub-y", Enable: false, Reset: 30, ExpiryTime: past, TotalGB: 1000}, + } + ib := &model.Inbound{ + Tag: "stale-" + strconv.Itoa(port), Enable: true, Port: port, Protocol: model.VLESS, + Settings: clientsSettings(t, clients), NodeID: nodeID, + } + if err := database.GetDB().Create(ib).Error; err != nil { + t.Fatalf("create inbound: %v", err) + } + if err := (&ClientService{}).SyncInbound(nil, ib.Id, clients); err != nil { + t.Fatalf("SyncInbound: %v", err) + } + rows := []xray.ClientTraffic{ + {InboundId: ib.Id, Email: "x@stale", Enable: true, ExpiryTime: future}, + {InboundId: ib.Id, Email: "y@stale", Enable: false, Up: 600, Down: 400, Total: 1000, Reset: 30, ExpiryTime: past}, + } + if err := database.GetDB().Create(&rows).Error; err != nil { + t.Fatalf("seed client_traffics: %v", err) + } + return ib +} + +func autoRenewTick(tx *gorm.DB) error { + _, _, err := (&InboundService{}).autoRenewClients(tx, newTrafficMutationBatch()) + return err +} + +// renewYTick writes the renewal autoRenewClients would commit for y@stale; it +// skips clients hosted only on a node, so the node case applies it directly. +func renewYTick(inboundId int) func(tx *gorm.DB) error { + return func(tx *gorm.DB) error { + var ib model.Inbound + if err := tx.First(&ib, inboundId).Error; err != nil { + return err + } + var settings map[string]any + if err := json.Unmarshal([]byte(ib.Settings), &settings); err != nil { + return err + } + for _, c := range settings["clients"].([]any) { + if m := c.(map[string]any); m["email"] == "y@stale" { + m["enable"] = true + m["expiryTime"] = time.Now().Add(30 * 24 * time.Hour).UnixMilli() + } + } + b, err := json.MarshalIndent(settings, "", " ") + if err != nil { + return err + } + return tx.Model(&model.Inbound{}).Where("id = ?", inboundId).Update("settings", string(b)).Error + } +} + +func settingsClient(t *testing.T, inboundId int, email string) (model.Client, bool) { + t.Helper() + ib, err := (&InboundService{}).GetInbound(inboundId) + if err != nil { + t.Fatalf("GetInbound: %v", err) + } + clients, err := (&InboundService{}).GetClients(ib) + if err != nil { + t.Fatalf("GetClients: %v", err) + } + for _, c := range clients { + if c.Email == email { + return c, true + } + } + return model.Client{}, false +} + +func requireNeighbourRenewed(t *testing.T, inboundId int) model.Client { + t.Helper() + y, ok := settingsClient(t, inboundId, "y@stale") + if !ok { + t.Fatal("neighbour y@stale missing from settings") + } + if now := time.Now().UnixMilli(); !y.Enable || y.ExpiryTime <= now { + t.Fatalf("renewed neighbour rolled back in settings: enable=%v expiryTime=%d (now %d)", y.Enable, y.ExpiryTime, now) + } + return y +} + +type staleClientOp struct { + name string + // advancesNodeFingerprint: on a node inbound the op pushes per client and + // then advances the reconcile-skip fingerprint. + advancesNodeFingerprint bool + run func(t *testing.T, ib *model.Inbound) error +} + +var staleClientOps = []staleClientOp{ + {"edit", true, func(t *testing.T, ib *model.Inbound) error { + rec := lookupClientRecord(t, "x@stale") + edited := rec.ToClient() + edited.Comment = "edited" + _, err := (&ClientService{}).UpdateInboundClient(&InboundService{}, &model.Inbound{ + Id: ib.Id, Settings: clientsSettings(t, []model.Client{*edited}), + }, "x@stale") + return err + }}, + {"add", true, func(t *testing.T, ib *model.Inbound) error { + _, err := (&ClientService{}).AddInboundClient(&InboundService{}, &model.Inbound{ + Id: ib.Id, Settings: clientsSettings(t, []model.Client{{Email: "z@stale", ID: "aaaaaaaa-0000-0000-0000-00000000000c", Enable: true}}), + }) + return err + }}, + {"delete", true, func(t *testing.T, ib *model.Inbound) error { + _, err := (&ClientService{}).DelInboundClientByEmail(&InboundService{}, ib.Id, "x@stale", false, true) + return err + }}, + {"bulk detach", true, func(t *testing.T, ib *model.Inbound) error { + _, _, err := (&ClientService{}).BulkDetach(&InboundService{}, []string{"x@stale"}, []int{ib.Id}) + return err + }}, + {"bulk adjust", false, func(t *testing.T, ib *model.Inbound) error { + _, _, err := (&ClientService{}).BulkAdjust(&InboundService{}, []string{"x@stale"}, 1, 0, "", nil, "") + return err + }}, + {"bulk delete", false, func(t *testing.T, ib *model.Inbound) error { + _, _, err := (&ClientService{}).BulkDelete(&InboundService{}, []string{"x@stale"}, false) + return err + }}, + {"bulk set enable", true, func(t *testing.T, ib *model.Inbound) error { + _, _, err := (&ClientService{}).BulkSetEnable(&InboundService{}, []string{"x@stale"}, false) + return err + }}, +} + +// Each client op reads the inbound before queueing its write; a renewal the +// traffic writer commits in between must not be reverted to enable=false. +func TestClientOpsKeepNeighbourRenewedMidOp(t *testing.T) { + for i, op := range staleClientOps { + t.Run(op.name, func(t *testing.T) { + setupBulkDB(t) + ib := seedRenewableNeighbour(t, 23101+i, nil) + commitTickBetweenReadAndWrite(t, autoRenewTick, func() { + if err := op.run(t, ib); err != nil { + t.Errorf("%s: %v", op.name, err) + } + }) + requireNeighbourRenewed(t, ib.Id) + }) + } +} + +// An op on the renewed client itself keeps the fields it did not change. +func TestBulkAdjustOnRenewedClientKeepsRenewal(t *testing.T) { + setupBulkDB(t) + ib := seedRenewableNeighbour(t, 23120, nil) + commitTickBetweenReadAndWrite(t, autoRenewTick, func() { + if _, _, err := (&ClientService{}).BulkAdjust(&InboundService{}, []string{"y@stale"}, 0, 500, "", nil, ""); err != nil { + t.Errorf("BulkAdjust: %v", err) + } + }) + if y := requireNeighbourRenewed(t, ib.Id); y.TotalGB != 1500 { + t.Fatalf("y@stale totalGB = %d, want 1500 (the adjust itself was lost)", y.TotalGB) + } +} + +// The node got only the per-client push, so the skip fingerprint must not claim +// it also holds the renewal the traffic writer committed mid-op. +func TestNodeClientOpsMidRenewalStillReconcileRenewal(t *testing.T) { + for i, op := range staleClientOps { + if !op.advancesNodeFingerprint { + continue + } + t.Run(op.name, func(t *testing.T) { + setupBulkDB(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + })) + t.Cleanup(srv.Close) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parse url: %v", err) + } + port, _ := strconv.Atoi(u.Port()) + node := &model.Node{ + Name: "stale-node", Scheme: "http", Address: u.Hostname(), Port: port, BasePath: "/", + ApiToken: "tok", Enable: true, Status: "online", AllowPrivateAddress: true, + } + if err := database.GetDB().Create(node).Error; err != nil { + t.Fatalf("create node: %v", err) + } + remote := runtime.NewRemote(node, nil) + useTestRuntimeManager(t).SetRuntimeOverride(node.Id, remote) + + ib := seedRenewableNeighbour(t, 23131+i, &node.Id) + remote.AdoptInboundAlias(ib, runtime.RemoteInboundOption{Id: 7, Tag: ib.Tag}) + + commitTickBetweenReadAndWrite(t, renewYTick(ib.Id), func() { + if err := op.run(t, ib); err != nil { + t.Errorf("%s: %v", op.name, err) + } + }) + requireNeighbourRenewed(t, ib.Id) + + saved, err := (&InboundService{}).GetInbound(ib.Id) + if err != nil { + t.Fatalf("GetInbound: %v", err) + } + pushed, err := remote.ReconcileInbound(context.Background(), saved, true) + if err != nil { + t.Fatalf("ReconcileInbound: %v", err) + } + if !pushed { + t.Fatal("reconcile skipped the inbound: the node never receives y@stale's renewal") + } + }) + } +} + +func TestRebaseClientSettings(t *testing.T) { + const base = `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}` + cases := []struct { + name, ours, current, want string + }{ + { + name: "untouched client takes the committed version", + ours: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":2},{"email":"b","enable":false,"expiryTime":1}]}`, + current: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":true,"expiryTime":9}]}`, + want: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":2},{"email":"b","enable":true,"expiryTime":9}]}`, + }, + { + name: "edited client keeps committed changes to fields the op left alone", + ours: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1,"comment":"x"}]}`, + current: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":true,"expiryTime":9}]}`, + want: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":true,"expiryTime":9,"comment":"x"}]}`, + }, + { + name: "client the op removed stays removed", + ours: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1}]}`, + current: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":true,"expiryTime":9}]}`, + want: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1}]}`, + }, + { + name: "client committed after the read is kept", + ours: `{"decryption":"none","clients":[{"email":"a","enable":false,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}`, + current: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1},{"email":"c"}]}`, + want: `{"decryption":"none","clients":[{"email":"a","enable":false,"totalGB":1},{"email":"b","enable":false,"expiryTime":1},{"email":"c"}]}`, + }, + { + name: "untouched client removed after the read stays removed", + ours: `{"decryption":"none","clients":[{"email":"a","enable":false,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}`, + current: `{"decryption":"none","clients":[{"email":"a","enable":true,"totalGB":1}]}`, + want: `{"decryption":"none","clients":[{"email":"a","enable":false,"totalGB":1}]}`, + }, + { + name: "top-level key follows whichever side changed it", + ours: `{"decryption":"none","testseed":[1],"clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}`, + current: `{"decryption":"mlkem","clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}`, + want: `{"decryption":"mlkem","testseed":[1],"clients":[{"email":"a","enable":true,"totalGB":1},{"email":"b","enable":false,"expiryTime":1}]}`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := rebaseClientSettings(base, tc.ours, tc.current) + if err != nil { + t.Fatalf("rebaseClientSettings: %v", err) + } + var gotV, wantV any + if err := json.Unmarshal([]byte(got), &gotV); err != nil { + t.Fatalf("unmarshal got: %v", err) + } + if err := json.Unmarshal([]byte(tc.want), &wantV); err != nil { + t.Fatalf("unmarshal want: %v", err) + } + if !reflect.DeepEqual(gotV, wantV) { + t.Fatalf("rebase = %s\nwant %s", got, tc.want) + } + }) + } +}