feat(clients): give each client its own traffic reset cycle (#6240)

* 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 <ho3ein.sanaei@gmail.com>

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

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
 		}

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
 	}

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.

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