diff --git a/internal/web/service/node.go b/internal/web/service/node.go index bafccd23c..d1e14d243 100644 --- a/internal/web/service/node.go +++ b/internal/web/service/node.go @@ -24,6 +24,8 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/util/netsafe" "github.com/mhsanaei/3x-ui/v3/internal/web/runtime" "github.com/mhsanaei/3x-ui/v3/internal/xray" + + "gorm.io/gorm" ) type HeartbeatPatch struct { @@ -439,6 +441,17 @@ func FilterNodeSnapshot(n *model.Node, snap *runtime.TrafficSnapshot) { func (s *NodeService) Delete(id int) error { db := database.GetDB() + // Refuse to delete a node that still owns inbounds: dropping the node row + // while inbounds keep its node_id leaves orphaned, dangling references that + // confuse node sync, subscriptions and cleanup. The operator must detach or + // remove those inbounds first. (DB-002) + var attached int64 + if err := db.Model(&model.Inbound{}).Where("node_id = ?", id).Count(&attached).Error; err != nil { + return err + } + if attached > 0 { + return common.NewError(fmt.Sprintf("cannot delete node: %d inbound(s) still attached to it; detach or delete them first", attached)) + } // Capture the node's guid before deleting the row so we can drop its per-node // IP attribution (NodeClientIp is keyed by guid, not node id). var guid string @@ -446,16 +459,22 @@ func (s *NodeService) Delete(id int) error { if err := db.Select("guid").Where("id = ?", id).First(&n).Error; err == nil { guid = n.Guid } - if err := db.Where("id = ?", id).Delete(model.Node{}).Error; err != nil { - return err - } - if err := db.Where("node_id = ?", id).Delete(&model.NodeClientTraffic{}).Error; err != nil { - return err - } - if guid != "" { - if err := db.Where("node_guid = ?", guid).Delete(&model.NodeClientIp{}).Error; err != nil { + // Delete the node row and its per-node child rows atomically. Remove the + // children (traffic baselines, IP attribution) before the parent node row so + // the ordering already matches a future ON DELETE constraint. Delete stays + // tolerant of a missing node row so it can still clean up orphaned baselines. + if err := db.Transaction(func(tx *gorm.DB) error { + if err := tx.Where("node_id = ?", id).Delete(&model.NodeClientTraffic{}).Error; err != nil { return err } + if guid != "" { + if err := tx.Where("node_guid = ?", guid).Delete(&model.NodeClientIp{}).Error; err != nil { + return err + } + } + return tx.Where("id = ?", id).Delete(&model.Node{}).Error + }); err != nil { + return err } if mgr := runtime.GetManager(); mgr != nil { mgr.InvalidateNode(id) diff --git a/internal/web/service/node_delete_orphan_test.go b/internal/web/service/node_delete_orphan_test.go new file mode 100644 index 000000000..11c2775db --- /dev/null +++ b/internal/web/service/node_delete_orphan_test.go @@ -0,0 +1,51 @@ +package service + +import ( + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// TestNodeDelete_BlocksWhenInboundsAttached guards DB-002: a node that still +// owns inbounds must not be deletable (which would orphan those inbounds with a +// dangling node_id), while a node with none deletes cleanly together with its +// traffic baselines. +func TestNodeDelete_BlocksWhenInboundsAttached(t *testing.T) { + db := initTrafficTestDB(t) + svc := &NodeService{} + + node := &model.Node{Name: "n1"} + if err := db.Create(node).Error; err != nil { + t.Fatalf("create node: %v", err) + } + createNodeInbound(t, db, node.Id, "n1-in-443", 443) + + // With an inbound attached, Delete must fail and leave node + inbound intact. + if err := svc.Delete(node.Id); err == nil { + t.Fatal("Delete should fail while an inbound is still attached") + } + var nodeCnt, ibCnt int64 + db.Model(&model.Node{}).Where("id = ?", node.Id).Count(&nodeCnt) + db.Model(&model.Inbound{}).Where("node_id = ?", node.Id).Count(&ibCnt) + if nodeCnt != 1 || ibCnt != 1 { + t.Fatalf("after blocked delete: node=%d inbound=%d, want 1/1", nodeCnt, ibCnt) + } + + // Detach the inbound and seed a traffic baseline; Delete now succeeds and + // cleans the baseline. + if err := db.Where("node_id = ?", node.Id).Delete(&model.Inbound{}).Error; err != nil { + t.Fatalf("detach inbound: %v", err) + } + if err := db.Create(&model.NodeClientTraffic{NodeId: node.Id, Email: "gone"}).Error; err != nil { + t.Fatalf("seed baseline: %v", err) + } + if err := svc.Delete(node.Id); err != nil { + t.Fatalf("Delete (no inbounds attached): %v", err) + } + var baseCnt int64 + db.Model(&model.Node{}).Where("id = ?", node.Id).Count(&nodeCnt) + db.Model(&model.NodeClientTraffic{}).Where("node_id = ?", node.Id).Count(&baseCnt) + if nodeCnt != 0 || baseCnt != 0 { + t.Fatalf("after delete: node=%d baseline=%d, want 0/0", nodeCnt, baseCnt) + } +}