From 5c7ca5b579f24201456ddba7936dd340a088dc56 Mon Sep 17 00:00:00 2001 From: n0ctal <4c866w5fn9@privaterelay.appleid.com> Date: Tue, 18 Aug 2026 16:08:44 +0500 Subject: [PATCH] feat(clients): give each client its own traffic reset cycle (#6240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(clients): give each client its own traffic reset cycle Traffic reset is configured on the inbound, so every client sharing an inbound resets together. An operator running a monthly 1000GB plan and a weekly 200GB plan side by side has to press Reset Traffic by hand. Clients now carry the same trafficReset / trafficResetDay pair the inbound already has, with the same vocabulary and the same monthly due-day rule, and PeriodicTrafficResetJob makes a second pass over the clients whose own cycle matches the period it is running for. A client that leaves the field at never behaves exactly as before: only its inbound's schedule can reset it. The fields live on ClientRecord as well as in the inbound settings JSON, so an ordinary edit does not write the cycle back as empty, and an unknown period is rejected rather than coerced, since a coerced value would read as configured while no job would ever select the client. Cron expressions and a custom post-reset quota from the issue are left out: both are separate decisions, and neither has an inbound-level counterpart to stay consistent with. * fix(clients): make the per-client reset cycle editable and safe to run Review found three things wrong with the first cut, one of them mine and worse than the bug it replaced. The cycle could only be set at creation. ClientService.Update writes the columns directly only for a client with no inbounds; the normal path goes through SyncInbound and applyClientRecordMerge, which this change had not extended, so an edit updated the settings JSON while the clients column kept the old value and the job kept applying the old cycle. The earlier test passed because it asserted the value survived an unrelated edit, which it did precisely because nothing ever wrote it. Replaced with a test that changes the cycle and switches it off again. Avoiding the re-enable that ResetTrafficByEmail performs was wrong. Depletion disables clients.enable and the settings JSON as well as client_traffics.enable, so lifting only the quota gate left a depleted client out of the generated config with zeroed counters, which no longer match the depleted predicate: locked out permanently. The rule is now about cause, not state — a client the quota switched off is restored, one disabled below its quota was switched off by hand and is skipped. The bulk path also bypassed node propagation and the MTProto sidecar quota that ResetTrafficByEmail handles, so it silently did nothing on node-backed inbounds. Dropped in favour of the integrated path, whose needRestart is now collected and turned into a single SetToNeedRestart. Also adds the AutoMigrate NULL backfill, guards the merge so a stale node snapshot cannot erase a configured cycle, validates the bulk-create and import paths, normalizes the day the way the inbound path does, marks the fields omitempty so existing clients match the published contract, and shares one TRAFFIC_RESETS tuple between the three forms. * fix(clients): validate renew fields on the bulk and import paths too BulkCreate and ImportClients insert client records without going through Create, so the resetDay/resetMax checks added with the calendar renewal (#6239) and the renew cap (#6238) never ran there. An API caller could store resetDay 45 or a negative resetMax, values the renewal query then mishandles silently. Mirror Create's validation on both batch paths, next to the trafficReset check they already carry. --------- Co-authored-by: Sanaei --- frontend/public/openapi.json | 24 ++ frontend/src/generated/examples.ts | 4 + frontend/src/generated/schemas.ts | 24 ++ frontend/src/generated/types.ts | 4 + frontend/src/generated/zod.ts | 4 + .../src/pages/clients/ClientBulkAddModal.tsx | 26 ++- .../src/pages/clients/ClientFormModal.tsx | 36 ++- .../pages/inbounds/form/InboundFormModal.tsx | 3 +- frontend/src/schemas/client.ts | 6 + frontend/src/schemas/primitives/index.ts | 1 + .../src/schemas/primitives/traffic-reset.ts | 7 + internal/database/db.go | 19 ++ internal/database/model/model.go | 159 ++++++++------ .../job/periodic_traffic_reset_client_test.go | 206 ++++++++++++++++++ .../web/job/periodic_traffic_reset_job.go | 62 +++++- internal/web/service/client_bulk.go | 12 + internal/web/service/client_crud.go | 64 ++++++ internal/web/service/client_link.go | 8 + internal/web/service/client_portable.go | 12 + .../web/service/client_traffic_cycle_test.go | 154 +++++++++++++ 20 files changed, 761 insertions(+), 74 deletions(-) create mode 100644 frontend/src/schemas/primitives/traffic-reset.ts create mode 100644 internal/web/job/periodic_traffic_reset_client_test.go create mode 100644 internal/web/service/client_traffic_cycle_test.go diff --git a/frontend/public/openapi.json b/frontend/public/openapi.json index c80b1a262..14dc072c2 100644 --- a/frontend/public/openapi.json +++ b/frontend/public/openapi.json @@ -1157,6 +1157,22 @@ "format": "int64", "type": "integer" }, + "trafficReset": { + "description": "Per-client traffic reset cycle, independent of the inbound's own (#5497).", + "enum": [ + "never", + "hourly", + "daily", + "weekly", + "monthly" + ], + "type": "string" + }, + "trafficResetDay": { + "maximum": 31, + "minimum": 1, + "type": "integer" + }, "updated_at": { "description": "Last update timestamp", "format": "int64", @@ -1288,6 +1304,12 @@ "format": "int64", "type": "integer" }, + "trafficReset": { + "type": "string" + }, + "trafficResetDay": { + "type": "integer" + }, "updatedAt": { "format": "int64", "type": "integer" @@ -1324,6 +1346,8 @@ "subId", "tgId", "totalGB", + "trafficReset", + "trafficResetDay", "updatedAt", "uuid" ], diff --git a/frontend/src/generated/examples.ts b/frontend/src/generated/examples.ts index 5876b7a38..7e2cf8f26 100644 --- a/frontend/src/generated/examples.ts +++ b/frontend/src/generated/examples.ts @@ -266,6 +266,8 @@ export const EXAMPLES: Record = { "subId": "", "tgId": 0, "totalGB": 0, + "trafficReset": "never", + "trafficResetDay": 1, "updated_at": 0 }, "ClientInbound": { @@ -302,6 +304,8 @@ export const EXAMPLES: Record = { "subId": "", "tgId": 0, "totalGB": 0, + "trafficReset": "", + "trafficResetDay": 0, "updatedAt": 0, "uuid": "" }, diff --git a/frontend/src/generated/schemas.ts b/frontend/src/generated/schemas.ts index 00c1f5625..5fdc82510 100644 --- a/frontend/src/generated/schemas.ts +++ b/frontend/src/generated/schemas.ts @@ -1131,6 +1131,22 @@ export const SCHEMAS: Record = { "format": "int64", "type": "integer" }, + "trafficReset": { + "description": "Per-client traffic reset cycle, independent of the inbound's own (#5497).", + "enum": [ + "never", + "hourly", + "daily", + "weekly", + "monthly" + ], + "type": "string" + }, + "trafficResetDay": { + "maximum": 31, + "minimum": 1, + "type": "integer" + }, "updated_at": { "description": "Last update timestamp", "format": "int64", @@ -1262,6 +1278,12 @@ export const SCHEMAS: Record = { "format": "int64", "type": "integer" }, + "trafficReset": { + "type": "string" + }, + "trafficResetDay": { + "type": "integer" + }, "updatedAt": { "format": "int64", "type": "integer" @@ -1298,6 +1320,8 @@ export const SCHEMAS: Record = { "subId", "tgId", "totalGB", + "trafficReset", + "trafficResetDay", "updatedAt", "uuid" ], diff --git a/frontend/src/generated/types.ts b/frontend/src/generated/types.ts index 7e814bf6f..c86a570da 100644 --- a/frontend/src/generated/types.ts +++ b/frontend/src/generated/types.ts @@ -276,6 +276,8 @@ export interface Client { subId: string; tgId: number; totalGB: number; + trafficReset?: string; + trafficResetDay?: number; updated_at?: number; } @@ -314,6 +316,8 @@ export interface ClientRecord { subId: string; tgId: number; totalGB: number; + trafficReset: string; + trafficResetDay: number; updatedAt: number; uuid: string; } diff --git a/frontend/src/generated/zod.ts b/frontend/src/generated/zod.ts index 2da88a1a1..7a03ec69e 100644 --- a/frontend/src/generated/zod.ts +++ b/frontend/src/generated/zod.ts @@ -296,6 +296,8 @@ export const ClientSchema = z.object({ subId: z.string(), tgId: z.number().int(), totalGB: z.number().int(), + trafficReset: z.enum(['never', 'hourly', 'daily', 'weekly', 'monthly']).optional(), + trafficResetDay: z.number().int().min(1).max(31).optional(), updated_at: z.number().int().optional(), }); export type Client = z.infer; @@ -336,6 +338,8 @@ export const ClientRecordSchema = z.object({ subId: z.string(), tgId: z.number().int(), totalGB: z.number().int(), + trafficReset: z.string(), + trafficResetDay: z.number().int(), updatedAt: z.number().int(), uuid: z.string(), }); diff --git a/frontend/src/pages/clients/ClientBulkAddModal.tsx b/frontend/src/pages/clients/ClientBulkAddModal.tsx index eaf35ba04..218999206 100644 --- a/frontend/src/pages/clients/ClientBulkAddModal.tsx +++ b/frontend/src/pages/clients/ClientBulkAddModal.tsx @@ -8,7 +8,7 @@ import { FormProvider, useForm, useWatch } from 'react-hook-form'; import { RandomUtil, SizeFormatter } from '@/utils'; import { formatInboundLabel } from '@/lib/inbounds/label'; -import { TLS_FLOW_CONTROL } from '@/schemas/primitives'; +import { TLS_FLOW_CONTROL, TRAFFIC_RESETS } from '@/schemas/primitives'; import { DateTimePicker, SelectAllClearButtons } from '@/components/form'; import { FormField } from '@/components/form/rhf'; import { useClients, type InboundOption } from '@/hooks/useClients'; @@ -39,6 +39,8 @@ const EMPTY: ClientBulkAddFormValues = { reset: 0, resetDay: 0, resetMax: 0, + trafficReset: 'never' as const, + trafficResetDay: 1, inboundIds: [], }; @@ -69,6 +71,7 @@ export default function ClientBulkAddModal({ const expiryTime = useWatch({ control: methods.control, name: 'expiryTime' }); const subId = useWatch({ control: methods.control, name: 'subId' }); const limitIp = useWatch({ control: methods.control, name: 'limitIp' }); + const trafficReset = useWatch({ control: methods.control, name: 'trafficReset' }); const [delayedStart, setDelayedStart] = useState(false); const [saving, setSaving] = useState(false); const fail2ban = useFail2banStatusQuery(); @@ -180,6 +183,8 @@ export default function ClientBulkAddModal({ reset: Number(current.reset) || 0, resetDay: Number(current.resetDay) || 0, resetMax: Number(current.resetMax) || 0, + trafficReset: current.trafficReset || 'never', + trafficResetDay: Number(current.trafficResetDay) || 1, limitIp: Number(current.limitIp) || 0, limitHwid: Number(current.limitHwid) || 0, group: current.group, @@ -396,6 +401,25 @@ export default function ClientBulkAddModal({ > + + + ({ + value: r, + label: t(`pages.inbounds.periodicTrafficReset.${r}`), + }))} + /> + + + {trafficReset === 'monthly' && ( + + Number(v) || 1 }} + > + + + + )} diff --git a/frontend/src/pages/inbounds/form/InboundFormModal.tsx b/frontend/src/pages/inbounds/form/InboundFormModal.tsx index a050de5d8..34bca1da0 100644 --- a/frontend/src/pages/inbounds/form/InboundFormModal.tsx +++ b/frontend/src/pages/inbounds/form/InboundFormModal.tsx @@ -39,7 +39,7 @@ import { type InboundFormValues, } from '@/schemas/forms/inbound-form'; import { FormField, rhfZodValidate } from '@/components/form/rhf'; -import { Protocols } from '@/schemas/primitives'; +import { Protocols, TRAFFIC_RESETS } from '@/schemas/primitives'; import { SockoptStreamSettingsSchema } from '@/schemas/protocols/stream/sockopt'; import { HysteriaStreamSettingsSchema } from '@/schemas/protocols/stream/hysteria'; import { createHysteriaTlsSettingsWithDefaultCert } from '@/lib/xray/inbound-tls-defaults'; @@ -99,7 +99,6 @@ const labelWithHint = (label: string, hint: string) => ( ); const PROTOCOL_OPTIONS = Object.values(Protocols).map((p) => ({ value: p, label: p })); -const TRAFFIC_RESETS = ['never', 'hourly', 'daily', 'weekly', 'monthly'] as const; const SHARE_ADDR_STRATEGIES = ['node', 'listen', 'custom'] as const; const SHARE_ADDR_HOSTNAME_RE = /^[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?(\.[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?)*$/; diff --git a/frontend/src/schemas/client.ts b/frontend/src/schemas/client.ts index aeb891c0c..3dc4d5dc4 100644 --- a/frontend/src/schemas/client.ts +++ b/frontend/src/schemas/client.ts @@ -35,6 +35,8 @@ export const ClientRecordSchema = z.object({ reset: z.number().optional(), resetDay: z.number().optional(), resetMax: z.number().optional(), + trafficReset: z.string().optional(), + trafficResetDay: z.number().optional(), inboundIds: nullableNumberArray.optional(), traffic: ClientTrafficSchema.nullable().optional(), reverse: z.object({ tag: z.string().optional() }).loose().nullable().optional(), @@ -211,6 +213,8 @@ export const ClientFormSchema = z.object({ reset: z.number().int().min(0), resetDay: z.number().int().min(0).max(31), resetMax: z.number().int().min(0), + trafficReset: z.enum(['never', 'hourly', 'daily', 'weekly', 'monthly']), + trafficResetDay: z.number().int().min(1).max(31), limitIp: z.number().int().min(0), limitHwid: z.number().int().min(0), tgId: z.number().int().min(0), @@ -252,6 +256,8 @@ export const ClientBulkAddFormSchema = z.object({ reset: z.number().int().min(0), resetDay: z.number().int().min(0).max(31), resetMax: z.number().int().min(0), + trafficReset: z.enum(['never', 'hourly', 'daily', 'weekly', 'monthly']).optional(), + trafficResetDay: z.number().int().min(1).max(31).optional(), inboundIds: z.array(z.number()).min(1, 'pages.clients.selectInbound'), }); diff --git a/frontend/src/schemas/primitives/index.ts b/frontend/src/schemas/primitives/index.ts index 089e39a46..9bc230730 100644 --- a/frontend/src/schemas/primitives/index.ts +++ b/frontend/src/schemas/primitives/index.ts @@ -4,3 +4,4 @@ export * from './outbound-protocol'; export * from './sniffing'; export * from './flow'; export * from './options'; +export * from './traffic-reset'; diff --git a/frontend/src/schemas/primitives/traffic-reset.ts b/frontend/src/schemas/primitives/traffic-reset.ts new file mode 100644 index 000000000..d034eb43d --- /dev/null +++ b/frontend/src/schemas/primitives/traffic-reset.ts @@ -0,0 +1,7 @@ +/** + * The traffic reset cycles an inbound or a client may be put on. Shared so the + * inbound form, the client form and the bulk-add form cannot drift apart. + */ +export const TRAFFIC_RESETS = ['never', 'hourly', 'daily', 'weekly', 'monthly'] as const; + +export type TrafficResetCycle = (typeof TRAFFIC_RESETS)[number]; diff --git a/internal/database/db.go b/internal/database/db.go index 755f0a7fa..aee7f08ce 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -155,6 +155,9 @@ func initModels() error { if err := migrateTgIDIndex(); err != nil { return err } + if err := migrateClientTrafficResetColumns(); err != nil { + return err + } if err := migrateSyncOrphanColumns(); err != nil { return err } @@ -315,6 +318,22 @@ func rebuildInboundsWithoutInlineUniquePort() error { }) } +// AutoMigrate adds the columns; an older SQLite ALTER TABLE leaves them NULL, +// and a NULL traffic_reset fails every ClientRecord scan, not just the new query. +func migrateClientTrafficResetColumns() error { + if db.Migrator().HasColumn(&model.ClientRecord{}, "traffic_reset") { + if err := db.Exec("UPDATE clients SET traffic_reset = 'never' WHERE traffic_reset IS NULL").Error; err != nil { + return err + } + } + if db.Migrator().HasColumn(&model.ClientRecord{}, "traffic_reset_day") { + if err := db.Exec("UPDATE clients SET traffic_reset_day = 1 WHERE traffic_reset_day IS NULL").Error; err != nil { + return err + } + } + return nil +} + // AutoMigrate adds the column; this only backfills the NULLs an older SQLite // ALTER TABLE leaves behind, so the reaper's predicate never compares to NULL. func migrateSyncOrphanColumns() error { diff --git a/internal/database/model/model.go b/internal/database/model/model.go index 0cb382e90..c79e1a98f 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -896,40 +896,45 @@ type Client struct { Reset int `json:"reset" form:"reset"` // Reset period in days ResetDay int `json:"resetDay" form:"resetDay"` // Calendar renewal day 1-31, 0 = interval mode ResetMax int `json:"resetMax" form:"resetMax"` // Max auto-renew count, 0 = unlimited - CreatedAt int64 `json:"created_at,omitempty"` // Creation timestamp - UpdatedAt int64 `json:"updated_at,omitempty"` // Last update timestamp + // Per-client traffic reset cycle, independent of the inbound's own (#5497). + TrafficReset string `json:"trafficReset,omitempty" form:"trafficReset" validate:"omitempty,oneof=never hourly daily weekly monthly"` + TrafficResetDay int `json:"trafficResetDay,omitempty" form:"trafficResetDay" validate:"omitempty,gte=1,lte=31"` + CreatedAt int64 `json:"created_at,omitempty"` // Creation timestamp + UpdatedAt int64 `json:"updated_at,omitempty"` // Last update timestamp } type ClientRecord struct { - Id int `json:"id" gorm:"primaryKey;autoIncrement"` - Email string `json:"email" gorm:"uniqueIndex;not null"` - SubID string `json:"subId" gorm:"index;column:sub_id"` - UUID string `json:"uuid" gorm:"column:uuid"` - Password string `json:"password"` - Auth string `json:"auth"` - Flow string `json:"flow"` - Security string `json:"security"` - Reverse string `json:"reverse" gorm:"column:reverse"` - PrivateKey string `json:"privateKey" gorm:"column:wg_private_key"` - PublicKey string `json:"publicKey" gorm:"column:wg_public_key"` - AllowedIPs string `json:"allowedIPs" gorm:"column:wg_allowed_ips"` - PreSharedKey string `json:"preSharedKey" gorm:"column:wg_pre_shared_key"` - KeepAlive int `json:"keepAlive" gorm:"column:wg_keep_alive;default:0"` - Secret string `json:"secret" gorm:"column:secret"` - AdTag string `json:"adTag" gorm:"column:ad_tag;default:''"` - LimitIP int `json:"limitIp" gorm:"column:limit_ip"` - LimitHwid int `json:"limitHwid" gorm:"column:limit_hwid;default:0"` - TotalGB int64 `json:"totalGB" gorm:"column:total_gb"` - ExpiryTime int64 `json:"expiryTime" gorm:"column:expiry_time"` - Enable bool `json:"enable" gorm:"default:true"` - TgID int64 `json:"tgId" gorm:"column:tg_id;index:idx_clients_tg_id"` - Group string `json:"group" gorm:"column:group_name;default:'';index:idx_client_record_group"` - Comment string `json:"comment"` - Reset int `json:"reset" gorm:"default:0"` - ResetDay int `json:"resetDay" gorm:"column:reset_day;default:0"` - ResetMax int `json:"resetMax" gorm:"column:reset_max;default:0"` - CreatedAt int64 `json:"createdAt" gorm:"autoCreateTime:milli"` - UpdatedAt int64 `json:"updatedAt" gorm:"autoUpdateTime:milli"` + Id int `json:"id" gorm:"primaryKey;autoIncrement"` + Email string `json:"email" gorm:"uniqueIndex;not null"` + SubID string `json:"subId" gorm:"index;column:sub_id"` + UUID string `json:"uuid" gorm:"column:uuid"` + Password string `json:"password"` + Auth string `json:"auth"` + Flow string `json:"flow"` + Security string `json:"security"` + Reverse string `json:"reverse" gorm:"column:reverse"` + PrivateKey string `json:"privateKey" gorm:"column:wg_private_key"` + PublicKey string `json:"publicKey" gorm:"column:wg_public_key"` + AllowedIPs string `json:"allowedIPs" gorm:"column:wg_allowed_ips"` + PreSharedKey string `json:"preSharedKey" gorm:"column:wg_pre_shared_key"` + KeepAlive int `json:"keepAlive" gorm:"column:wg_keep_alive;default:0"` + Secret string `json:"secret" gorm:"column:secret"` + AdTag string `json:"adTag" gorm:"column:ad_tag;default:''"` + LimitIP int `json:"limitIp" gorm:"column:limit_ip"` + LimitHwid int `json:"limitHwid" gorm:"column:limit_hwid;default:0"` + TotalGB int64 `json:"totalGB" gorm:"column:total_gb"` + ExpiryTime int64 `json:"expiryTime" gorm:"column:expiry_time"` + Enable bool `json:"enable" gorm:"default:true"` + TgID int64 `json:"tgId" gorm:"column:tg_id;index:idx_clients_tg_id"` + Group string `json:"group" gorm:"column:group_name;default:'';index:idx_client_record_group"` + Comment string `json:"comment"` + Reset int `json:"reset" gorm:"default:0"` + ResetDay int `json:"resetDay" gorm:"column:reset_day;default:0"` + ResetMax int `json:"resetMax" gorm:"column:reset_max;default:0"` + TrafficReset string `json:"trafficReset" gorm:"column:traffic_reset;default:never;index:idx_clients_traffic_reset"` + TrafficResetDay int `json:"trafficResetDay" gorm:"column:traffic_reset_day;default:1"` + CreatedAt int64 `json:"createdAt" gorm:"autoCreateTime:milli"` + UpdatedAt int64 `json:"updatedAt" gorm:"autoUpdateTime:milli"` // Owned solely by the node-snapshot sweep, which soft-orphans instead of // deleting; orphans from any other cause stay at zero and are never reaped. SyncOrphanedAt int64 `json:"-" gorm:"column:sync_orphaned_at;default:0"` @@ -1094,25 +1099,27 @@ func (Host) TableName() string { return "hosts" } func (c *Client) ToRecord() *ClientRecord { rec := &ClientRecord{ - Email: c.Email, - SubID: c.SubID, - UUID: c.ID, - Password: c.Password, - Auth: c.Auth, - Flow: c.Flow, - Security: c.Security, - LimitIP: c.LimitIP, - TotalGB: c.TotalGB, - ExpiryTime: c.ExpiryTime, - Enable: c.Enable, - TgID: c.TgID, - Group: c.Group, - Comment: c.Comment, - Reset: c.Reset, - ResetDay: c.ResetDay, - ResetMax: c.ResetMax, - CreatedAt: c.CreatedAt, - UpdatedAt: c.UpdatedAt, + Email: c.Email, + SubID: c.SubID, + UUID: c.ID, + Password: c.Password, + Auth: c.Auth, + Flow: c.Flow, + Security: c.Security, + LimitIP: c.LimitIP, + TotalGB: c.TotalGB, + ExpiryTime: c.ExpiryTime, + Enable: c.Enable, + TgID: c.TgID, + Group: c.Group, + Comment: c.Comment, + Reset: c.Reset, + ResetDay: c.ResetDay, + ResetMax: c.ResetMax, + TrafficReset: c.TrafficReset, + TrafficResetDay: c.TrafficResetDay, + CreatedAt: c.CreatedAt, + UpdatedAt: c.UpdatedAt, PrivateKey: c.PrivateKey, PublicKey: c.PublicKey, @@ -1149,25 +1156,27 @@ func splitWireguardAllowedIPs(csv string) []string { func (r *ClientRecord) ToClient() *Client { c := &Client{ - ID: r.UUID, - Email: r.Email, - SubID: r.SubID, - Password: r.Password, - Auth: r.Auth, - Flow: r.Flow, - Security: r.Security, - LimitIP: r.LimitIP, - TotalGB: r.TotalGB, - ExpiryTime: r.ExpiryTime, - Enable: r.Enable, - TgID: r.TgID, - Group: r.Group, - Comment: r.Comment, - Reset: r.Reset, - ResetDay: r.ResetDay, - ResetMax: r.ResetMax, - CreatedAt: r.CreatedAt, - UpdatedAt: r.UpdatedAt, + ID: r.UUID, + Email: r.Email, + SubID: r.SubID, + Password: r.Password, + Auth: r.Auth, + Flow: r.Flow, + Security: r.Security, + LimitIP: r.LimitIP, + TotalGB: r.TotalGB, + ExpiryTime: r.ExpiryTime, + Enable: r.Enable, + TgID: r.TgID, + Group: r.Group, + Comment: r.Comment, + Reset: r.Reset, + ResetDay: r.ResetDay, + ResetMax: r.ResetMax, + TrafficReset: r.TrafficReset, + TrafficResetDay: r.TrafficResetDay, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, PrivateKey: r.PrivateKey, PublicKey: r.PublicKey, @@ -1326,6 +1335,18 @@ func MergeClientRecord(existing *ClientRecord, incoming *ClientRecord) []ClientM existing.ResetMax = incoming.ResetMax } } + if existing.TrafficReset != incoming.TrafficReset && incoming.TrafficReset != "" { + if incomingNewer || existing.TrafficReset == "" { + keep("trafficReset", existing.TrafficReset, incoming.TrafficReset, incoming.TrafficReset) + existing.TrafficReset = incoming.TrafficReset + } + } + if existing.TrafficResetDay != incoming.TrafficResetDay && incoming.TrafficResetDay != 0 { + if incomingNewer || existing.TrafficResetDay == 0 { + keep("trafficResetDay", existing.TrafficResetDay, incoming.TrafficResetDay, incoming.TrafficResetDay) + existing.TrafficResetDay = incoming.TrafficResetDay + } + } if existing.Reverse != incoming.Reverse && incoming.Reverse != "" { if incomingNewer || existing.Reverse == "" { keep("reverse", existing.Reverse, incoming.Reverse, incoming.Reverse) diff --git a/internal/web/job/periodic_traffic_reset_client_test.go b/internal/web/job/periodic_traffic_reset_client_test.go new file mode 100644 index 000000000..d9c3ded3b --- /dev/null +++ b/internal/web/job/periodic_traffic_reset_client_test.go @@ -0,0 +1,206 @@ +package job + +import ( + "encoding/json" + "path/filepath" + "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/xray" +) + +func initResetJobDB(t *testing.T) { + t.Helper() + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = database.CloseDB() }) +} + +type seededClient struct { + email string + cycle string + day int + recordEnable bool + quotaEnable bool + total int64 +} + +// seedClientOnCycle creates an inbound that never resets on its own, a client +// carrying its own cycle, and the client_inbounds link the reset path resolves +// through — without it every reset falls into the orphaned-client branch. +func seedClientOnCycle(t *testing.T, port int, c seededClient) { + t.Helper() + db := database.GetDB() + + client := model.Client{ + Email: c.email, ID: uuidFor(port), Enable: c.recordEnable, + TrafficReset: c.cycle, TrafficResetDay: c.day, + } + settings, err := json.Marshal(map[string]any{"clients": []model.Client{client}}) + if err != nil { + t.Fatalf("marshal settings: %v", err) + } + ib := model.Inbound{ + UserId: 1, Enable: true, Port: port, Protocol: model.VLESS, + Tag: "inbound-" + c.email, TrafficReset: "never", Settings: string(settings), + } + if err := db.Create(&ib).Error; err != nil { + t.Fatalf("create inbound: %v", err) + } + rec := model.ClientRecord{ + Email: c.email, UUID: client.ID, Enable: c.recordEnable, + TrafficReset: c.cycle, TrafficResetDay: c.day, + } + if err := db.Create(&rec).Error; err != nil { + t.Fatalf("create client record: %v", err) + } + // gorm skips a false bool on insert, so the column default:true wins; the + // disabled case has to be written back explicitly. + if err := db.Model(&model.ClientRecord{}).Where("id = ?", rec.Id). + Update("enable", c.recordEnable).Error; err != nil { + t.Fatalf("set record enable: %v", err) + } + if err := db.Create(&model.ClientInbound{ClientId: rec.Id, InboundId: ib.Id}).Error; err != nil { + t.Fatalf("link client to inbound: %v", err) + } + if err := db.Create(&xray.ClientTraffic{ + InboundId: ib.Id, Email: c.email, Enable: c.quotaEnable, Up: 500, Down: 700, Total: c.total, + }).Error; err != nil { + t.Fatalf("create traffic: %v", err) + } +} + +func uuidFor(port int) string { + return "00000000-0000-0000-0000-0000000" + string(rune('0'+port/10000%10)) + + string(rune('0'+port/1000%10)) + string(rune('0'+port/100%10)) + + string(rune('0'+port/10%10)) + string(rune('0'+port%10)) +} + +func trafficFor(t *testing.T, email string) xray.ClientTraffic { + t.Helper() + var row xray.ClientTraffic + if err := database.GetDB().Where("email = ?", email).First(&row).Error; err != nil { + t.Fatalf("read traffic for %s: %v", email, err) + } + return row +} + +func recordFor(t *testing.T, email string) model.ClientRecord { + t.Helper() + var rec model.ClientRecord + if err := database.GetDB().Where("email = ?", email).First(&rec).Error; err != nil { + t.Fatalf("read record for %s: %v", email, err) + } + return rec +} + +func TestPeriodicTrafficResetClients(t *testing.T) { + t.Run("resets a client on its own cycle inside a never-reset inbound", func(t *testing.T) { + initResetJobDB(t) + seedClientOnCycle(t, 41001, seededClient{email: "weekly@example.com", cycle: "weekly", day: 1, recordEnable: true, quotaEnable: true}) + seedClientOnCycle(t, 41002, seededClient{email: "monthly@example.com", cycle: "monthly", day: 1, recordEnable: true, quotaEnable: true}) + + NewPeriodicTrafficResetJob("weekly", time.UTC).Run() + + if row := trafficFor(t, "weekly@example.com"); row.Up != 0 || row.Down != 0 { + t.Fatalf("weekly client not reset by the weekly run: up=%d down=%d", row.Up, row.Down) + } + if row := trafficFor(t, "monthly@example.com"); row.Up != 500 || row.Down != 700 { + t.Fatalf("monthly client reset by the weekly run: up=%d down=%d", row.Up, row.Down) + } + }) + + t.Run("leaves a client with no cycle of its own alone", func(t *testing.T) { + initResetJobDB(t) + seedClientOnCycle(t, 41003, seededClient{email: "none@example.com", cycle: "never", day: 1, recordEnable: true, quotaEnable: true}) + + for _, period := range []Period{"hourly", "daily", "weekly", "monthly"} { + NewPeriodicTrafficResetJob(period, time.UTC).Run() + } + + if row := trafficFor(t, "none@example.com"); row.Up != 500 || row.Down != 700 { + t.Fatalf("client with trafficReset=never was reset: up=%d down=%d", row.Up, row.Down) + } + }) + + t.Run("monthly client waits for its own day", func(t *testing.T) { + initResetJobDB(t) + today := time.Now().In(time.UTC).Day() + otherDay := today%28 + 1 + seedClientOnCycle(t, 41004, seededClient{email: "due@example.com", cycle: "monthly", day: today, recordEnable: true, quotaEnable: true}) + seedClientOnCycle(t, 41005, seededClient{email: "notdue@example.com", cycle: "monthly", day: otherDay, recordEnable: true, quotaEnable: true}) + + NewPeriodicTrafficResetJob("monthly", time.UTC).Run() + + if row := trafficFor(t, "due@example.com"); row.Up != 0 || row.Down != 0 { + t.Fatalf("client due today was not reset: up=%d down=%d", row.Up, row.Down) + } + if row := trafficFor(t, "notdue@example.com"); row.Up != 500 || row.Down != 700 { + t.Fatalf("client due on another day was reset: up=%d down=%d", row.Up, row.Down) + } + }) + + t.Run("restores a client the quota switched off", func(t *testing.T) { + initResetJobDB(t) + // Depletion disables all three of client_traffics.enable, clients.enable + // and the settings JSON, so a reset that lifts only the first leaves the + // client out of the running core with nothing left to revisit it. + seedClientOnCycle(t, 41006, seededClient{ + email: "depleted@example.com", cycle: "daily", day: 1, + recordEnable: false, quotaEnable: false, total: 1000, + }) + + NewPeriodicTrafficResetJob("daily", time.UTC).Run() + + if row := trafficFor(t, "depleted@example.com"); !row.Enable { + t.Fatal("quota gate not lifted: the client cannot use its new allowance") + } + if rec := recordFor(t, "depleted@example.com"); !rec.Enable { + t.Fatal("clients.enable still false: GetXrayConfig skips the client, so it stays locked out for good") + } + if enabled := settingsEnableOf(t, 41006); !enabled { + t.Fatal("the inbound settings JSON still has the client disabled") + } + }) + + t.Run("leaves a client the operator switched off", func(t *testing.T) { + initResetJobDB(t) + // Disabled with usage below quota: nothing but a human did that. + seedClientOnCycle(t, 41007, seededClient{ + email: "banned@example.com", cycle: "daily", day: 1, + recordEnable: false, quotaEnable: true, total: 100000, + }) + + NewPeriodicTrafficResetJob("daily", time.UTC).Run() + + if rec := recordFor(t, "banned@example.com"); rec.Enable { + t.Fatal("an operator-disabled client was switched back on by a cron job") + } + if row := trafficFor(t, "banned@example.com"); row.Up != 500 || row.Down != 700 { + t.Fatalf("an operator-disabled client was reset anyway: up=%d down=%d", row.Up, row.Down) + } + }) +} + +func settingsEnableOf(t *testing.T, port int) bool { + t.Helper() + var stored model.Inbound + if err := database.GetDB().Where("port = ?", port).First(&stored).Error; err != nil { + t.Fatal(err) + } + var settings struct { + Clients []model.Client `json:"clients"` + } + if err := json.Unmarshal([]byte(stored.Settings), &settings); err != nil { + t.Fatalf("parse inbound settings: %v", err) + } + if len(settings.Clients) != 1 { + t.Fatalf("inbound holds %d clients, want 1", len(settings.Clients)) + } + return settings.Clients[0].Enable +} diff --git a/internal/web/job/periodic_traffic_reset_job.go b/internal/web/job/periodic_traffic_reset_job.go index b0564df4e..78a698153 100644 --- a/internal/web/job/periodic_traffic_reset_job.go +++ b/internal/web/job/periodic_traffic_reset_job.go @@ -14,6 +14,7 @@ type Period string type PeriodicTrafficResetJob struct { inboundService service.InboundService clientService service.ClientService + xrayService service.XrayService period Period location *time.Location } @@ -34,8 +35,14 @@ func monthlyResetDue(resetDay int, now time.Time) bool { return now.Day() == min(resetDay, lastDay) } -// Run resets traffic statistics for all inbounds that match the configured reset period. +// Run resets traffic statistics for all inbounds that match the configured reset +// period, then for the clients carrying that period on their own (#5497). func (j *PeriodicTrafficResetJob) Run() { + j.resetInboundsOnSchedule() + j.resetClientsOnTheirOwnCycle() +} + +func (j *PeriodicTrafficResetJob) resetInboundsOnSchedule() { inbounds, err := j.inboundService.GetInboundsByTrafficReset(string(j.period)) if err != nil { logger.Warning("Failed to get inbounds for traffic reset:", err) @@ -78,3 +85,56 @@ func (j *PeriodicTrafficResetJob) Run() { logger.Infof("Periodic traffic reset completed: %d inbounds reset", resetCount) } } + +// resetClientsOnTheirOwnCycle resets clients whose cycle is set individually. A +// client inside an inbound on the same cycle is reset twice, which is harmless. +func (j *PeriodicTrafficResetJob) resetClientsOnTheirOwnCycle() { + cycles, err := j.clientService.GetClientsByTrafficReset(string(j.period)) + if err != nil { + logger.Warning("Failed to get clients for traffic reset:", err) + return + } + + now := time.Now().In(j.location) + due := make([]service.ClientResetCycle, 0, len(cycles)) + for _, c := range cycles { + // Monthly clients come due on their own day, the rule the inbound-level + // schedule already follows. + if j.period == "monthly" && !monthlyResetDue(c.TrafficResetDay, now) { + continue + } + // A reset re-enables, which is right for a client the quota switched off + // and wrong for one an operator switched off by hand. + if !c.Enable && !c.Depleted() { + continue + } + due = append(due, c) + } + if len(due) == 0 { + return + } + logger.Infof("Running periodic traffic reset job for period: %s (%d matching clients)", j.period, len(due)) + + resetCount := 0 + needRestart := false + for _, c := range due { + // ResetTrafficByEmail rather than a bulk UPDATE: it is the path that also + // propagates to the client's node and clears the MTProto sidecar quota. + nr, resetErr := j.clientService.ResetTrafficByEmail(&j.inboundService, c.Email) + if resetErr != nil { + logger.Warning("Failed to reset traffic for client", c.Email, ":", resetErr) + continue + } + needRestart = needRestart || nr + resetCount++ + } + // Dropping this leaves a re-enabled client absent from the running core until + // something unrelated restarts it. + if needRestart { + j.xrayService.SetToNeedRestart() + } + + if resetCount > 0 { + logger.Infof("Periodic traffic reset completed: %d clients reset", resetCount) + } +} diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index 230ca05c5..cd04ab58a 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -1148,6 +1148,18 @@ func (s *ClientService) BulkCreate(inboundSvc *InboundService, payloads []Client skip(email, verr.Error()) continue } + if verr := validateClientResetDay(client.ResetDay); verr != nil { + skip(email, verr.Error()) + continue + } + if verr := validateClientResetMax(client.ResetMax); verr != nil { + skip(email, verr.Error()) + continue + } + if verr := validateClientTrafficReset(client.TrafficReset, client.TrafficResetDay); verr != nil { + skip(email, verr.Error()) + continue + } if len(payloads[i].InboundIds) == 0 { skip(email, "at least one inbound is required") continue diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index d7de748ab..b8bc6b113 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -43,6 +43,20 @@ func validateClientSubID(subID string) error { return nil } +// Rejected rather than coerced: an unknown cycle would leave the operator with +// a field that reads as configured while no job ever selects the client. +func validateClientTrafficReset(period string, day int) error { + switch period { + case "", "never", "hourly", "daily", "weekly", "monthly": + default: + return common.NewError("client trafficReset must be never, hourly, daily, weekly or monthly, got:", period) + } + if day < 0 || day > 31 { + return common.NewError("client trafficResetDay must be between 0 and 31, got:", day) + } + return nil +} + // Rejected rather than clamped: nextCalendarRenewal would silently move an // out-of-range day, and a negative one drops out of the renewal query entirely. func validateClientResetDay(day int) error { @@ -61,6 +75,46 @@ func validateClientResetMax(resetMax int) error { return nil } +// normalizeClientTrafficReset stores what the inbound path would store, so the +// day never reaches the DB as a 0 that three layers downstream each clamp to 1. +func normalizeClientTrafficReset(c *model.Client) { + if c.TrafficReset == "" { + c.TrafficReset = "never" + } + c.TrafficResetDay = normalizeTrafficResetDay(c.TrafficResetDay) +} + +// ClientResetCycle is the slice of a client the reset job needs: enough to know +// whether it is due, and whether its disable is the quota's doing or the operator's. +type ClientResetCycle struct { + Email string + TrafficResetDay int + Enable bool + Total int64 + Used int64 +} + +// Depleted reports a client the quota switched off. A reset restores that one; +// a client disabled below its quota was switched off by hand and stays off. +func (c ClientResetCycle) Depleted() bool { + return c.Total > 0 && c.Used >= c.Total +} + +// GetClientsByTrafficReset returns the clients whose own reset cycle matches the +// period, independent of the cycle configured on the inbounds they belong to. +func (s *ClientService) GetClientsByTrafficReset(period string) ([]ClientResetCycle, error) { + var cycles []ClientResetCycle + err := database.GetDB().Table("clients c"). + Select("c.email, c.traffic_reset_day, c.enable, COALESCE(ct.total, 0) AS total, COALESCE(ct.up, 0) + COALESCE(ct.down, 0) AS used"). + Joins("LEFT JOIN client_traffics ct ON ct.email = c.email"). + Where("c.traffic_reset = ?", period). + Scan(&cycles).Error + if err != nil { + return nil, err + } + return cycles, nil +} + func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreatePayload) (bool, error) { if payload == nil { return false, common.NewError("empty payload") @@ -81,6 +135,10 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate if err := validateClientResetMax(client.ResetMax); err != nil { return false, err } + if err := validateClientTrafficReset(client.TrafficReset, client.TrafficResetDay); err != nil { + return false, err + } + normalizeClientTrafficReset(&client) if len(payload.InboundIds) == 0 { return false, common.NewError("at least one inbound is required") } @@ -374,6 +432,10 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model if err := validateClientResetMax(updated.ResetMax); err != nil { return false, err } + if err := validateClientTrafficReset(updated.TrafficReset, updated.TrafficResetDay); err != nil { + return false, err + } + normalizeClientTrafficReset(&updated) if updated.SubID == "" { updated.SubID = existing.SubID } @@ -498,6 +560,8 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model "reset": merged.Reset, "reset_day": merged.ResetDay, "reset_max": merged.ResetMax, + "traffic_reset": merged.TrafficReset, + "traffic_reset_day": merged.TrafficResetDay, }).Error; err != nil { return needRestart, err } diff --git a/internal/web/service/client_link.go b/internal/web/service/client_link.go index ea7e81ccb..70eba4b7f 100644 --- a/internal/web/service/client_link.go +++ b/internal/web/service/client_link.go @@ -65,6 +65,14 @@ func applyClientRecordMerge(row *model.ClientRecord, incoming *model.ClientRecor row.Reset = incoming.Reset row.ResetDay = incoming.ResetDay row.ResetMax = incoming.ResetMax + // Guarded like Group and AdTag: a node snapshot rebuilt from settings that + // predate the cycle would otherwise silently erase it. + if incoming.TrafficReset != "" { + row.TrafficReset = incoming.TrafficReset + } + if incoming.TrafficResetDay > 0 { + row.TrafficResetDay = incoming.TrafficResetDay + } if incoming.CreatedAt > 0 && (row.CreatedAt == 0 || incoming.CreatedAt < row.CreatedAt) { row.CreatedAt = incoming.CreatedAt } diff --git a/internal/web/service/client_portable.go b/internal/web/service/client_portable.go index 1acd921f0..66abb5056 100644 --- a/internal/web/service/client_portable.go +++ b/internal/web/service/client_portable.go @@ -115,6 +115,18 @@ func (s *ClientService) ImportClients(inboundSvc *InboundService, items []Client skip(email, verr.Error()) continue } + if verr := validateClientResetDay(client.ResetDay); verr != nil { + skip(email, verr.Error()) + continue + } + if verr := validateClientResetMax(client.ResetMax); verr != nil { + skip(email, verr.Error()) + continue + } + if verr := validateClientTrafficReset(client.TrafficReset, client.TrafficResetDay); verr != nil { + skip(email, verr.Error()) + continue + } // An existing record (in the DB or just created from the attached set // above) always wins — import never clobbers a live client. diff --git a/internal/web/service/client_traffic_cycle_test.go b/internal/web/service/client_traffic_cycle_test.go new file mode 100644 index 000000000..aa9109125 --- /dev/null +++ b/internal/web/service/client_traffic_cycle_test.go @@ -0,0 +1,154 @@ +package service + +import ( + "encoding/json" + "testing" + "time" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// The cycle has to survive the clients table, not just the settings JSON: an +// ordinary edit rebuilds the client from the record and writes it back (#5497). +func TestClientEditKeepsTheTrafficResetCycle(t *testing.T) { + setupBulkDB(t) + svc := &InboundService{} + db := database.GetDB() + + clients := []model.Client{ + { + Email: "cyc@x", ID: "66666666-6666-6666-6666-666666666666", Enable: true, + TrafficReset: "monthly", TrafficResetDay: 15, + ExpiryTime: time.Now().Add(24 * time.Hour).UnixMilli(), + }, + } + ib := mkInbound(t, 30301, model.VLESS, clientsSettings(t, clients)) + if err := svc.clientService.SyncInbound(nil, ib.Id, clients); err != nil { + t.Fatalf("SyncInbound: %v", err) + } + + rec, err := svc.clientService.GetRecordByEmail(nil, "cyc@x") + if err != nil { + t.Fatalf("GetRecordByEmail: %v", err) + } + if rec.TrafficReset != "monthly" || rec.TrafficResetDay != 15 { + t.Fatalf("clients row holds %q/%d, want monthly/15", rec.TrafficReset, rec.TrafficResetDay) + } + + // What the edit dialog does: hydrate the record, change something else, save. + edited := rec.ToClient() + edited.Comment = "renamed" + if _, err := svc.clientService.Update(svc, rec.Id, *edited, rec.LimitHwid); err != nil { + t.Fatalf("Update: %v", err) + } + + var stored model.Inbound + if err := db.Where("id = ?", ib.Id).First(&stored).Error; err != nil { + t.Fatal(err) + } + var settings struct { + Clients []model.Client `json:"clients"` + } + if err := json.Unmarshal([]byte(stored.Settings), &settings); err != nil { + t.Fatalf("parse inbound settings: %v", err) + } + if len(settings.Clients) != 1 { + t.Fatalf("inbound holds %d clients, want 1", len(settings.Clients)) + } + if got := settings.Clients[0]; got.TrafficReset != "monthly" || got.TrafficResetDay != 15 { + t.Fatalf("inbound settings hold %q/%d after an unrelated edit, want monthly/15: the cycle was silently switched off", + got.TrafficReset, got.TrafficResetDay) + } + + rec, err = svc.clientService.GetRecordByEmail(nil, "cyc@x") + if err != nil { + t.Fatalf("GetRecordByEmail after edit: %v", err) + } + if rec.TrafficReset != "monthly" || rec.TrafficResetDay != 15 { + t.Fatalf("clients row holds %q/%d after an unrelated edit, want monthly/15", rec.TrafficReset, rec.TrafficResetDay) + } +} + +// The setting is useless if it can only be chosen once. This is the assertion +// the earlier "survives an unrelated edit" test could not make: that one passed +// precisely because nothing on the attached-inbound path ever wrote the column. +func TestClientEditChangesTheTrafficResetCycle(t *testing.T) { + setupBulkDB(t) + svc := &InboundService{} + + clients := []model.Client{ + { + Email: "chg@x", ID: "77777777-7777-7777-7777-777777777777", Enable: true, + TrafficReset: "weekly", TrafficResetDay: 1, + ExpiryTime: time.Now().Add(24 * time.Hour).UnixMilli(), + }, + } + ib := mkInbound(t, 30302, model.VLESS, clientsSettings(t, clients)) + if err := svc.clientService.SyncInbound(nil, ib.Id, clients); err != nil { + t.Fatalf("SyncInbound: %v", err) + } + + rec, err := svc.clientService.GetRecordByEmail(nil, "chg@x") + if err != nil { + t.Fatalf("GetRecordByEmail: %v", err) + } + + edited := rec.ToClient() + edited.TrafficReset = "monthly" + edited.TrafficResetDay = 9 + if _, err := svc.clientService.Update(svc, rec.Id, *edited, rec.LimitHwid); err != nil { + t.Fatalf("Update: %v", err) + } + + rec, err = svc.clientService.GetRecordByEmail(nil, "chg@x") + if err != nil { + t.Fatalf("GetRecordByEmail after edit: %v", err) + } + if rec.TrafficReset != "monthly" || rec.TrafficResetDay != 9 { + t.Fatalf("clients row holds %q/%d after the operator changed it to monthly/9: the job keeps applying the old cycle", + rec.TrafficReset, rec.TrafficResetDay) + } + + // Turning it off has to work too, and "never" is not an empty value. + edited = rec.ToClient() + edited.TrafficReset = "never" + if _, err := svc.clientService.Update(svc, rec.Id, *edited, rec.LimitHwid); err != nil { + t.Fatalf("Update to never: %v", err) + } + rec, err = svc.clientService.GetRecordByEmail(nil, "chg@x") + if err != nil { + t.Fatalf("GetRecordByEmail after disabling: %v", err) + } + if rec.TrafficReset != "never" { + t.Fatalf("clients row holds %q after the operator switched the cycle off", rec.TrafficReset) + } +} + +// An unknown cycle would leave a field that reads as configured while no job +// ever selects the client, so it is rejected instead of coerced. +func TestClientTrafficResetValidation(t *testing.T) { + for _, tc := range []struct { + name string + period string + day int + ok bool + }{ + {"unset", "", 0, true}, + {"never", "never", 1, true}, + {"monthly last day", "monthly", 31, true}, + {"unknown period", "fortnightly", 1, false}, + {"day past the month", "monthly", 32, false}, + {"negative day", "monthly", -1, false}, + } { + t.Run(tc.name, func(t *testing.T) { + err := validateClientTrafficReset(tc.period, tc.day) + if tc.ok && err != nil { + t.Errorf("validateClientTrafficReset(%q, %d) = %v, want accepted", tc.period, tc.day, err) + } + if !tc.ok && err == nil { + t.Errorf("validateClientTrafficReset(%q, %d) accepted, want rejected", tc.period, tc.day) + } + }) + } +}