refactor(clients): make clientHostRuleIds canonical, treat clientHostRuleId as deprecated alias

This commit is contained in:
Nethmina Yasas 2026-10-12 18:10:42 +05:30
parent 2986bcd841
commit 8a7c6a79c1
9 changed files with 316 additions and 25 deletions

View file

@ -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

View file

@ -1675,13 +1675,13 @@ export const SCHEMAS: Record<string, unknown> = {
"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

View file

@ -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),

View file

@ -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<typeof ClientFormModal>['save']) {
const qc = makeQC();
return (
<ThemeProvider>
<QueryClientProvider client={qc}>
<ClientFormModal
open
mode="edit"
client={CLIENT}
inbounds={INBOUNDS}
attachedIds={[1]}
save={save}
onOpenChange={() => {}}
/>
</QueryClientProvider>
</ThemeProvider>
);
}
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<string, unknown>;
expect(payload.clientHostRuleIds).toEqual([100]);
expect(payload.clientHostRuleId).toBe(100);
});
});

View file

@ -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)
}
}
})
}
}

View file

@ -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"`

View file

@ -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 {

View file

@ -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)
}
}

View file

@ -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 {