From 8a7c6a79c1951c9101fbf546145fa7d78b6b75f8 Mon Sep 17 00:00:00 2001 From: Nethmina Yasas <63988674+NethminaYasas@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:10:42 +0530 Subject: [PATCH] refactor(clients): make clientHostRuleIds canonical, treat clientHostRuleId as deprecated alias --- frontend/public/openapi.json | 4 +- frontend/src/generated/schemas.ts | 4 +- frontend/src/schemas/client.ts | 4 + .../test/client-host-list-payload.test.tsx | 73 ++++++++++++ .../model/client_host_normalize_test.go | 65 +++++++++++ internal/database/model/model.go | 44 ++++++- internal/web/service/client_crud.go | 25 ++-- .../web/service/client_host_binding_test.go | 107 ++++++++++++++++++ internal/web/service/client_link.go | 15 ++- 9 files changed, 316 insertions(+), 25 deletions(-) create mode 100644 frontend/src/test/client-host-list-payload.test.tsx create mode 100644 internal/database/model/client_host_normalize_test.go create mode 100644 internal/web/service/client_host_binding_test.go diff --git a/frontend/public/openapi.json b/frontend/public/openapi.json index 3aa755457..35596b510 100644 --- a/frontend/public/openapi.json +++ b/frontend/public/openapi.json @@ -1701,13 +1701,13 @@ "type": "string" }, "clientHostRuleId": { - "description": "ClientHostRuleId optionally binds one Host row to this client.\nNil or non-positive means no override (inbound rule applies).", + "description": "ClientHostRuleId is a deprecated read-only alias of ClientHostRuleIds[0].\nNil or non-positive means no override (inbound rule applies).", "example": 1, "nullable": true, "type": "integer" }, "clientHostRuleIds": { - "description": "ClientHostRuleIds optionally binds several Host rows to this client.\nUnioned with ClientHostRuleId; each bound host renders its own link.", + "description": "ClientHostRuleIds is the canonical per-client host binding; each id renders its own link.\nNil means omitted (preserve); empty means clear.", "example": [ 1, 2 diff --git a/frontend/src/generated/schemas.ts b/frontend/src/generated/schemas.ts index c4505a5c5..86d021155 100644 --- a/frontend/src/generated/schemas.ts +++ b/frontend/src/generated/schemas.ts @@ -1675,13 +1675,13 @@ export const SCHEMAS: Record = { "type": "string" }, "clientHostRuleId": { - "description": "ClientHostRuleId optionally binds one Host row to this client.\nNil or non-positive means no override (inbound rule applies).", + "description": "ClientHostRuleId is a deprecated read-only alias of ClientHostRuleIds[0].\nNil or non-positive means no override (inbound rule applies).", "example": 1, "nullable": true, "type": "integer" }, "clientHostRuleIds": { - "description": "ClientHostRuleIds optionally binds several Host rows to this client.\nUnioned with ClientHostRuleId; each bound host renders its own link.", + "description": "ClientHostRuleIds is the canonical per-client host binding; each id renders its own link.\nNil means omitted (preserve); empty means clear.", "example": [ 1, 2 diff --git a/frontend/src/schemas/client.ts b/frontend/src/schemas/client.ts index f3b914d3f..d9a9ff792 100644 --- a/frontend/src/schemas/client.ts +++ b/frontend/src/schemas/client.ts @@ -31,7 +31,9 @@ export const ClientRecordSchema = z auth: z.string().optional(), flow: z.string().optional(), security: z.string().optional(), + // Deprecated alias of clientHostRuleIds[0]; list is canonical. clientHostRuleId: z.number().int().nullable().optional(), + // Canonical binding; nil preserves, empty clears. clientHostRuleIds: z.array(z.number().int()).nullable().optional(), totalGB: z.number().optional(), expiryTime: z.number().optional(), @@ -325,7 +327,9 @@ export const ClientFormSchema = z.object({ auth: z.string(), flow: z.string(), security: z.string(), + // Deprecated alias of clientHostRuleIds[0]; list is canonical. clientHostRuleId: z.number().int().nullable().optional(), + // Canonical binding; nil preserves, empty clears. clientHostRuleIds: z.array(z.number().int()).nullable().optional(), reverseTag: z.string(), totalGB: z.number().min(0), diff --git a/frontend/src/test/client-host-list-payload.test.tsx b/frontend/src/test/client-host-list-payload.test.tsx new file mode 100644 index 000000000..89a34b228 --- /dev/null +++ b/frontend/src/test/client-host-list-payload.test.tsx @@ -0,0 +1,73 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { ComponentProps } from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { ThemeProvider } from '@/hooks/useTheme'; +import ClientFormModal from '@/pages/clients/ClientFormModal'; +import type { ClientRecord, InboundOption } from '@/hooks/useClients'; + +vi.mock('@/api/queries/useRawHostsQuery', () => ({ + useRawHostsQuery: () => ({ + hosts: [ + { id: 99, inboundId: 1, remark: 'cdn-a', address: 'a.example.com', isDisabled: false }, + { id: 100, inboundId: 1, remark: 'cdn-b', address: 'b.example.com', isDisabled: false }, + ], + loading: false, + fetched: true, + fetchError: '', + refetch: vi.fn(), + }), +})); + +function makeQC() { + return new QueryClient({ defaultOptions: { queries: { retry: false } } }); +} + +const INBOUNDS = [ + { id: 1, port: 443, protocol: 'vless', tag: 'in-1', remark: 'one', enable: true }, +] as unknown as InboundOption[]; + +// List-only payload; single mirrors the list for compatibility. +const CLIENT = { + email: 'list-only@example.com', + uuid: '11111111-1111-1111-1111-111111111111', + subId: 'subid123', + enable: true, + clientHostRuleId: null, + clientHostRuleIds: [100], +} as unknown as ClientRecord; + +function renderEdit(save: ComponentProps['save']) { + const qc = makeQC(); + return ( + + + {}} + /> + + + ); +} + +describe('ClientFormModal — list-only host payload', () => { + it('sends the list as the value with the single mirroring its first id', async () => { + const save = vi.fn().mockResolvedValue({ success: true }); + render(renderEdit(save)); + + const comment = await screen.findByLabelText('Comment'); + fireEvent.change(comment, { target: { value: 'touched' } }); + + fireEvent.click(await screen.findByRole('button', { name: /save/i })); + await waitFor(() => expect(save).toHaveBeenCalled()); + const payload = save.mock.calls[0][0] as Record; + expect(payload.clientHostRuleIds).toEqual([100]); + expect(payload.clientHostRuleId).toBe(100); + }); +}); diff --git a/internal/database/model/client_host_normalize_test.go b/internal/database/model/client_host_normalize_test.go new file mode 100644 index 000000000..d1875f7ba --- /dev/null +++ b/internal/database/model/client_host_normalize_test.go @@ -0,0 +1,65 @@ +package model + +import ( + "reflect" + "testing" +) + +func intPtr(v int) *int { return &v } + +func TestNormalizeClientHostBindings(t *testing.T) { + for _, tt := range []struct { + name string + single *int + list []int + wantSingle *int + wantList []int + wantListNil bool + }{ + {"nil preserves", nil, nil, nil, nil, true}, + {"empty clears", nil, []int{}, nil, []int{}, false}, + {"single-only migrates", intPtr(5), nil, intPtr(5), []int{5}, false}, + {"list-only mirrors first", nil, []int{2, 3}, intPtr(2), []int{2, 3}, false}, + {"conflict list wins", intPtr(9), []int{2, 3}, intPtr(2), []int{2, 3}, false}, + {"dedupe keeps order", nil, []int{3, 2, 3, 2}, intPtr(3), []int{3, 2}, false}, + {"non-positive dropped", nil, []int{0, -1, 2, 0, 3}, intPtr(2), []int{2, 3}, false}, + {"all non-positive clears", nil, []int{0, -2}, nil, []int{}, false}, + {"single non-positive normalizes", intPtr(0), nil, nil, nil, true}, + } { + t.Run(tt.name, func(t *testing.T) { + single := tt.single + var list []int + if tt.list != nil { + list = append([]int(nil), tt.list...) + if len(tt.list) == 0 { + list = []int{} + } + } + NormalizeClientHostBindings(&single, &list) + if tt.wantListNil { + if list != nil { + t.Fatalf("list = %v, want nil", list) + } + } else { + if list == nil { + t.Fatalf("list = nil, want %v", tt.wantList) + } + if !reflect.DeepEqual(list, tt.wantList) { + t.Fatalf("list = %v, want %v", list, tt.wantList) + } + } + if tt.wantSingle == nil { + if single != nil { + t.Fatalf("single = %v, want nil", *single) + } + } else { + if single == nil { + t.Fatalf("single = nil, want %d", *tt.wantSingle) + } + if *single != *tt.wantSingle { + t.Fatalf("single = %d, want %d", *single, *tt.wantSingle) + } + } + }) + } +} diff --git a/internal/database/model/model.go b/internal/database/model/model.go index ea6bf4248..9cb8e2726 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -928,11 +928,11 @@ type Client struct { PrivateKey string `json:"privateKey,omitempty"` PublicKey string `json:"publicKey,omitempty"` AllowedIPs []string `json:"allowedIPs,omitempty"` - // ClientHostRuleId optionally binds one Host row to this client. + // ClientHostRuleId is a deprecated read-only alias of ClientHostRuleIds[0]. // Nil or non-positive means no override (inbound rule applies). ClientHostRuleId *int `json:"clientHostRuleId,omitempty" example:"1"` - // ClientHostRuleIds optionally binds several Host rows to this client. - // Unioned with ClientHostRuleId; each bound host renders its own link. + // ClientHostRuleIds is the canonical per-client host binding; each id renders its own link. + // Nil means omitted (preserve); empty means clear. ClientHostRuleIds []int `json:"clientHostRuleIds,omitempty" example:"[1,2]"` // AllowedIPsByInbound optionally overrides AllowedIPs on a per-inbound // basis, keyed by inbound id. Lets one identity attached to both @@ -1397,6 +1397,44 @@ func EffectiveClientHostRuleIds(c Client) []int { return out } +// NormalizeClientHostBindings makes the list canonical; the single id is a read-only alias. +// Nil list preserves (migrating a lone single); non-nil list wins and mirrors its first id. +func NormalizeClientHostBindings(single **int, list *[]int) { + if single == nil || list == nil { + return + } + if *list != nil { + seen := make(map[int]struct{}, len(*list)) + canonical := make([]int, 0, len(*list)) + for _, id := range *list { + if id <= 0 { + continue + } + if _, dup := seen[id]; dup { + continue + } + seen[id] = struct{}{} + canonical = append(canonical, id) + } + *list = canonical + if len(canonical) > 0 { + v := canonical[0] + *single = &v + } else { + *single = nil + } + return + } + if *single != nil && **single > 0 { + v := **single + *list = []int{v} + return + } + if *single != nil && **single <= 0 { + *single = nil + } +} + type OutboundSubscription struct { Id int `json:"id" form:"id" gorm:"primaryKey;autoIncrement"` Remark string `json:"remark" form:"remark"` diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index 574a1e2be..7c85869d5 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -169,6 +169,8 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate return false, err } normalizeClientTrafficReset(&client) + // List is canonical; single is a deprecated alias migrated on write. + model.NormalizeClientHostBindings(&client.ClientHostRuleId, &client.ClientHostRuleIds) if len(payload.InboundIds) == 0 { return false, common.NewError("at least one inbound is required") } @@ -259,14 +261,13 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate // A re-created email is a live identity again: a delete tombstone left // standing makes the next node merge prune the new client's inbound links. withdrawClientTombstones(client.Email) - if client.ClientHostRuleId != nil { + // Canonical list plus its mirror; nil means omitted and preserves. + if client.ClientHostRuleIds != nil { if err := database.GetDB().Model(&model.ClientRecord{}). Where("email = ?", client.Email). UpdateColumn("client_host_rule_id", client.ClientHostRuleId).Error; err != nil { return needRestart, err } - } - if client.ClientHostRuleIds != nil { if err := database.GetDB().Model(&model.ClientRecord{}). Where("email = ?", client.Email). Update("client_host_rule_ids", model.EncodeClientHostRuleIds(client.ClientHostRuleIds)).Error; err != nil { @@ -663,6 +664,8 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model return false, err } normalizeClientTrafficReset(&updated) + // List is canonical; single is a deprecated alias migrated on write. + model.NormalizeClientHostBindings(&updated.ClientHostRuleId, &updated.ClientHostRuleIds) if updated.SubID == "" { updated.SubID = existing.SubID } @@ -862,17 +865,13 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model return needRestart, err } - // Same shape as the group write: SyncInbound preserves a stored binding, - // so the editor's round-tripped value (nil clears) is applied here. - if err := database.GetDB().Model(&model.ClientRecord{}). - Where("id = ?", id). - UpdateColumn("client_host_rule_id", updated.ClientHostRuleId).Error; err != nil { - return needRestart, err - } - - // Multi bindings round-trip like the single id, but nil (omitted) is - // distinct from empty (clear), so omitted preserves the stored list. + // List is canonical with its single mirror; nil preserves, empty clears. if updated.ClientHostRuleIds != nil { + if err := database.GetDB().Model(&model.ClientRecord{}). + Where("id = ?", id). + UpdateColumn("client_host_rule_id", updated.ClientHostRuleId).Error; err != nil { + return needRestart, err + } if err := database.GetDB().Model(&model.ClientRecord{}). Where("id = ?", id). Update("client_host_rule_ids", model.EncodeClientHostRuleIds(updated.ClientHostRuleIds)).Error; err != nil { diff --git a/internal/web/service/client_host_binding_test.go b/internal/web/service/client_host_binding_test.go new file mode 100644 index 000000000..7901ff135 --- /dev/null +++ b/internal/web/service/client_host_binding_test.go @@ -0,0 +1,107 @@ +package service + +import ( + "path/filepath" + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/dbtest" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +func setupHostBindingDB(t *testing.T) { + t.Helper() + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + dbtest.InitDB(t, filepath.Join(dbDir, "x-ui.db")) +} + +func TestUpdate_OmittedPreservesAndEmptyClearsHostBindings(t *testing.T) { + setupHostBindingDB(t) + svc := &ClientService{} + inboundSvc := &InboundService{} + + seven := 7 + rec := &model.ClientRecord{ + Email: "hostbind@x", + UUID: "11111111-1111-1111-1111-111111111111", + SubID: "hostbind@x", + Enable: true, + ClientHostRuleId: &seven, + ClientHostRuleIds: []int{7}, + } + if err := database.GetDB().Create(rec).Error; err != nil { + t.Fatalf("create record: %v", err) + } + + omitted := rec.ToClient() + omitted.ClientHostRuleId = nil + omitted.ClientHostRuleIds = nil + omitted.Comment = "touch" + if _, err := svc.Update(inboundSvc, rec.Id, *omitted, 0); err != nil { + t.Fatalf("Update omitted: %v", err) + } + got, err := svc.GetByID(rec.Id) + if err != nil { + t.Fatalf("GetByID: %v", err) + } + if got.ClientHostRuleIds == nil || len(got.ClientHostRuleIds) != 1 || got.ClientHostRuleIds[0] != 7 { + t.Fatalf("omitted must preserve list, got %v", got.ClientHostRuleIds) + } + if got.ClientHostRuleId == nil || *got.ClientHostRuleId != 7 { + t.Fatalf("omitted must preserve single, got %v", got.ClientHostRuleId) + } + + cleared := got.ToClient() + cleared.ClientHostRuleIds = []int{} + cleared.ClientHostRuleId = nil + if _, err := svc.Update(inboundSvc, rec.Id, *cleared, 0); err != nil { + t.Fatalf("Update clear: %v", err) + } + after, err := svc.GetByID(rec.Id) + if err != nil { + t.Fatalf("GetByID: %v", err) + } + if after.ClientHostRuleIds == nil { + t.Fatalf("empty must clear to non-nil empty, got nil") + } + if len(after.ClientHostRuleIds) != 0 { + t.Fatalf("empty must clear list, got %v", after.ClientHostRuleIds) + } + if after.ClientHostRuleId != nil { + t.Fatalf("empty must clear single, got %d", *after.ClientHostRuleId) + } +} + +func TestSyncInbound_SingleOnlyLegacyMigratesToList(t *testing.T) { + setupHostBindingDB(t) + db := database.GetDB() + + ib := &model.Inbound{Tag: "vless-host-migrate", Enable: true, Port: 20004, Protocol: model.VLESS} + if err := db.Create(ib).Error; err != nil { + t.Fatalf("create inbound: %v", err) + } + + svc := ClientService{} + hostID := 4242 + legacy := model.Client{ + Email: "legacy-single@x", + ID: "22222222-2222-2222-2222-222222222222", + Enable: true, + ClientHostRuleId: &hostID, + } + if err := svc.SyncInbound(nil, ib.Id, []model.Client{legacy}); err != nil { + t.Fatalf("SyncInbound legacy single: %v", err) + } + + var row model.ClientRecord + if err := db.Where("email = ?", "legacy-single@x").First(&row).Error; err != nil { + t.Fatalf("lookup record: %v", err) + } + if len(row.ClientHostRuleIds) != 1 || row.ClientHostRuleIds[0] != hostID { + t.Fatalf("single-only must migrate to list, got %v", row.ClientHostRuleIds) + } + if row.ClientHostRuleId == nil || *row.ClientHostRuleId != hostID { + t.Fatalf("single-only must mirror single, got %v", row.ClientHostRuleId) + } +} diff --git a/internal/web/service/client_link.go b/internal/web/service/client_link.go index 035675185..c93f70187 100644 --- a/internal/web/service/client_link.go +++ b/internal/web/service/client_link.go @@ -43,13 +43,16 @@ func applyClientRecordMerge(row *model.ClientRecord, incoming *model.ClientRecor if incoming.Security != "" { row.Security = incoming.Security } - // Guarded like Group: snapshots predating the binding carry nil and must - // not wipe it; clearing flows through Update's explicit write. - if incoming.ClientHostRuleId != nil { - row.ClientHostRuleId = incoming.ClientHostRuleId - } + // List is canonical; nil preserves, empty clears via Update's write. + model.NormalizeClientHostBindings(&incoming.ClientHostRuleId, &incoming.ClientHostRuleIds) if incoming.ClientHostRuleIds != nil { row.ClientHostRuleIds = model.CloneClientHostRuleIds(incoming.ClientHostRuleIds) + if incoming.ClientHostRuleId != nil { + v := *incoming.ClientHostRuleId + row.ClientHostRuleId = &v + } else { + row.ClientHostRuleId = nil + } } if incoming.Reverse != "" { row.Reverse = incoming.Reverse @@ -159,6 +162,8 @@ func (s *ClientService) syncInboundClients(tx *gorm.DB, inboundId int, clients [ // ToRecord copies the raw email; store the trimmed key this function // looks up by, or a padded email is inserted and never found again. incoming.Email = email + // List canonical for new rows too; merge path re-normalizes idempotently. + model.NormalizeClientHostBindings(&incoming.ClientHostRuleId, &incoming.ClientHostRuleIds) row, ok := existing[email] if !ok { if _, dup := pending[email]; !dup {