diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index ba59d3c41..9e875dc4c 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -908,8 +908,9 @@ func (s *ClientService) bulkDelInboundClients( return res } - // Match by email — the client's stable identity (see Delete). Removes every - // entry carrying a wanted email, independent of credential drift. + // Match by email — the client's stable identity (see Delete). The link-derived + // set is deletion intent: an email already absent from settings is successful, + // while foundEmails tracks entries that still need settings-specific cleanup. wantedEmails := make(map[string]struct{}, len(emails)) for _, email := range emails { if records[email] == nil { @@ -939,12 +940,6 @@ func (s *ClientService) bulkDelInboundClients( newClients = append(newClients, client) } - for email := range wantedEmails { - if !foundEmails[email] { - res.perEmailSkipped[email] = "Client Not Found In Inbound" - } - } - db := database.GetDB() newClients = compactOrphans(db, newClients) if newClients == nil { @@ -953,7 +948,7 @@ func (s *ClientService) bulkDelInboundClients( settings["clients"] = newClients newSettings, err := json.MarshalIndent(settings, "", " ") if err != nil { - for email := range foundEmails { + for email := range wantedEmails { if _, skip := res.perEmailSkipped[email]; !skip { res.perEmailSkipped[email] = err.Error() } @@ -991,9 +986,8 @@ func (s *ClientService) bulkDelInboundClients( var sharedErr error sharedSet, sharedErr = inboundSvc.emailsUsedByOtherInbounds(foundList, inboundId) if sharedErr != nil { - for email := range foundEmails { + for email := range wantedEmails { res.perEmailSkipped[email] = sharedErr.Error() - delete(foundEmails, email) } return res } @@ -1046,7 +1040,7 @@ func (s *ClientService) bulkDelInboundClients( return nil }) if txErr != nil { - for email := range foundEmails { + for email := range wantedEmails { if _, skip := res.perEmailSkipped[email]; !skip { res.perEmailSkipped[email] = txErr.Error() } @@ -1071,12 +1065,21 @@ func (s *ClientService) bulkDelInboundClients( } } } - } else if len(foundEmails) <= nodeBulkPushThreshold { + } else { + dispatchEmails := make([]string, 0, len(wantedEmails)) + for email := range wantedEmails { + if _, skip := res.perEmailSkipped[email]; !skip { + dispatchEmails = append(dispatchEmails, email) + } + } + if len(dispatchEmails) > nodeBulkPushThreshold { + return res + } rt, push, _, perr := inboundSvc.nodePushPlan(oldInbound) if perr != nil { logger.Warning("BulkDelete: node runtime lookup after commit failed:", perr) } else if push { - for email := range foundEmails { + for _, email := range dispatchEmails { if err1 := rt.DeleteClient(context.Background(), email); err1 != nil { logger.Warning("Error in deleting client on", rt.Name(), ":", err1) } diff --git a/internal/web/service/node_bulk_dispatch_test.go b/internal/web/service/node_bulk_dispatch_test.go index 4381b1662..0ddc5c8ab 100644 --- a/internal/web/service/node_bulk_dispatch_test.go +++ b/internal/web/service/node_bulk_dispatch_test.go @@ -234,6 +234,9 @@ func TestNodeBulkDeleteDoesNotPushBeforeFailedCommit(t *testing.T) { if got := fake.deleteClient.Load() + fake.deleteUser.Load(); got != 0 { t.Fatalf("failed transaction pushed %d delete call(s) to the node, want 0", got) } + if isClientEmailTombstoned(client.Email) { + t.Fatal("failed bulk delete left a live tombstone") + } } func TestNodeBulkSmallDeleteRemovesWholeRemoteClient(t *testing.T) { @@ -241,6 +244,10 @@ func TestNodeBulkSmallDeleteRemovesWholeRemoteClient(t *testing.T) { nodeID, fake := setupNodeRuntime(t) client := model.Client{ID: uuid.NewString(), Email: "full-delete@x", Enable: true} nodeInbound(t, nodeID, 30024, []model.Client{client}) + var record model.ClientRecord + if err := database.GetDB().Where("email = ?", client.Email).First(&record).Error; err != nil { + t.Fatalf("load client record: %v", err) + } result, _, err := (&ClientService{}).BulkDelete(&InboundService{}, []string{client.Email}, true) if err != nil { @@ -255,6 +262,104 @@ func TestNodeBulkSmallDeleteRemovesWholeRemoteClient(t *testing.T) { if got := fake.deleteUser.Load(); got != 0 { t.Fatalf("remote DeleteUser detach calls = %d, want 0 for full deletion", got) } + var records, links int64 + if err := database.GetDB().Model(&model.ClientRecord{}).Where("email = ?", client.Email).Count(&records).Error; err != nil { + t.Fatalf("count client records: %v", err) + } + if err := database.GetDB().Model(&model.ClientInbound{}).Where("client_id = ?", record.Id).Count(&links).Error; err != nil { + t.Fatalf("count client links: %v", err) + } + if records != 0 || links != 0 { + t.Fatalf("bulk delete left records=%d links=%d, want 0/0", records, links) + } +} + +func TestNodeBulkDeleteTreatsMissingSettingsClientAsAlreadyDeleted(t *testing.T) { + setupBulkDB(t) + nodeID, fake := setupNodeRuntime(t) + client := model.Client{ID: uuid.NewString(), Email: "drifted-delete@x", Enable: true} + ib := nodeInbound(t, nodeID, 30025, []model.Client{client}) + + // Simulate a stale normalized link after the client has already disappeared + // from the inbound settings JSON. + if err := database.GetDB().Model(&model.Inbound{}).Where("id = ?", ib.Id). + Update("settings", clientsSettings(t, nil)).Error; err != nil { + t.Fatalf("drift inbound settings: %v", err) + } + + result, _, err := (&ClientService{}).BulkDelete(&InboundService{}, []string{client.Email}, true) + if err != nil { + t.Fatalf("BulkDelete: %v", err) + } + if result.Deleted != 1 || len(result.Skipped) != 0 { + t.Fatalf("BulkDelete result = %+v, want one deleted client", result) + } + if got := fake.deleteClient.Load(); got != 1 { + t.Fatalf("remote DeleteClient calls = %d, want 1", got) + } + var records, links int64 + if err := database.GetDB().Model(&model.ClientRecord{}).Where("email = ?", client.Email).Count(&records).Error; err != nil { + t.Fatalf("count client records: %v", err) + } + if err := database.GetDB().Model(&model.ClientInbound{}).Where("inbound_id = ?", ib.Id).Count(&links).Error; err != nil { + t.Fatalf("count client links: %v", err) + } + if records != 0 || links != 0 { + t.Fatalf("bulk delete left records=%d links=%d, want 0/0", records, links) + } + if !isClientEmailTombstoned(client.Email) { + t.Fatal("successful bulk delete withdrew the client tombstone") + } + t.Cleanup(func() { withdrawClientTombstones(client.Email) }) +} + +func TestNodeBulkDeleteCompletesAcrossPresentAndMissingSettings(t *testing.T) { + setupBulkDB(t) + nodeID, fake := setupNodeRuntime(t) + client := model.Client{ID: uuid.NewString(), Email: "mixed-delete@x", Enable: true} + drifted := nodeInbound(t, nodeID, 30026, []model.Client{client}) + nodeInbound(t, nodeID, 30027, []model.Client{client}) + + if err := database.GetDB().Model(&model.Inbound{}).Where("id = ?", drifted.Id). + Update("settings", clientsSettings(t, nil)).Error; err != nil { + t.Fatalf("drift inbound settings: %v", err) + } + + result, _, err := (&ClientService{}).BulkDelete(&InboundService{}, []string{client.Email}, true) + if err != nil { + t.Fatalf("BulkDelete: %v", err) + } + if result.Deleted != 1 || len(result.Skipped) != 0 { + t.Fatalf("BulkDelete result = %+v, want one deleted client", result) + } + if got := fake.deleteClient.Load(); got != 2 { + t.Fatalf("remote DeleteClient calls = %d, want one per node inbound", got) + } +} + +func TestNodeBulkDeleteMalformedSettingsWithdrawsTombstone(t *testing.T) { + setupBulkDB(t) + nodeID, fake := setupNodeRuntime(t) + client := model.Client{ID: uuid.NewString(), Email: "malformed-delete@x", Enable: true} + ib := nodeInbound(t, nodeID, 30028, []model.Client{client}) + if err := database.GetDB().Model(&model.Inbound{}).Where("id = ?", ib.Id). + Update("settings", `{"clients":[`).Error; err != nil { + t.Fatalf("break inbound settings: %v", err) + } + + result, _, err := (&ClientService{}).BulkDelete(&InboundService{}, []string{client.Email}, true) + if err != nil { + t.Fatalf("BulkDelete: %v", err) + } + if result.Deleted != 0 || len(result.Skipped) != 1 { + t.Fatalf("BulkDelete result = %+v, want one skipped client", result) + } + if got := fake.deleteClient.Load() + fake.deleteUser.Load(); got != 0 { + t.Fatalf("malformed settings pushed %d delete call(s) to the node, want 0", got) + } + if isClientEmailTombstoned(client.Email) { + t.Fatal("failed bulk delete left a live tombstone") + } } func TestNodeUpdateInboundClientNoopSkipsRuntimeAndDirty(t *testing.T) { @@ -351,8 +456,8 @@ func TestNodeBulk_LargeDeleteFoldsToDirty(t *testing.T) { t.Fatalf("BulkDelete: %v", err) } - if got := fake.deleteUser.Load(); got != 0 { - t.Fatalf("large delete streamed %d DeleteUser RPCs, want 0 (should fold to dirty)", got) + if got := fake.deleteClient.Load() + fake.deleteUser.Load(); got != 0 { + t.Fatalf("large delete streamed %d delete RPCs, want 0 (should fold to dirty)", got) } if _, _, dirty, _, err := (&NodeService{}).NodeSyncState(nodeID); err != nil { t.Fatalf("NodeSyncState: %v", err)