diff --git a/internal/modules/stats/startup_mongo_test.go b/internal/modules/stats/startup_mongo_test.go index a9fabcc..b049bb0 100644 --- a/internal/modules/stats/startup_mongo_test.go +++ b/internal/modules/stats/startup_mongo_test.go @@ -138,3 +138,8 @@ func setupMongoStatsTest(t *testing.T) (context.Context, storage.Collection, sto provider := storage.NewMongoProvider(db) return ctx, provider.Collection("stats"), provider.Collection(systemstate.CollectionName) } + +func TestInc_MongoUsernameMoveClearsOldHolder(t *testing.T) { + _, statsColl, _ := setupMongoStatsTest(t) + assertUsernameMoveClearsOldHolder(t, statsColl) +} diff --git a/internal/modules/stats/stats_test.go b/internal/modules/stats/stats_test.go index 5da96be..a7abed4 100644 --- a/internal/modules/stats/stats_test.go +++ b/internal/modules/stats/stats_test.go @@ -497,3 +497,36 @@ func tail(s string) string { } return s[len(s)-n:] } + +// assertUsernameMoveClearsOldHolder checks that when a Telegram username moves +// to another account, the previous holder's rows drop it, so /stats user +// resolves to the new owner alone. +func assertUsernameMoveClearsOldHolder(t *testing.T, coll storage.Collection) { + t.Helper() + ctx := context.Background() + c := newCounter(coll) + docs := storage.Typed[usageEntry](coll) + + c.Inc(ctx, "ping", updateFrom(42, "bob")) + c.Inc(ctx, "wordle", updateFrom(42, "bob")) + c.Inc(ctx, "lol", updateFrom(77, "bob")) + + old, _, err := docs.Get(ctx, usageKey("ping", 42)) + if err != nil { + t.Fatalf("ping:42: %v", err) + } + if old.Username != "" || old.N != 1 { + t.Errorf("old holder row = %+v, want username cleared and count kept", old) + } + rows, found, err := c.store.CommandsByUser(ctx, "bob", 10) + if err != nil || !found { + t.Fatalf("CommandsByUser(bob) found=%v err=%v", found, err) + } + if len(rows) != 1 || rows[0].display != "/lol" { + t.Errorf("CommandsByUser(bob) = %+v, want only the new holder's /lol", rows) + } +} + +func TestInc_UsernameMoveClearsOldHolder(t *testing.T) { + assertUsernameMoveClearsOldHolder(t, storage.NewMemoryProvider().Collection("stats")) +} diff --git a/internal/modules/stats/usage_store.go b/internal/modules/stats/usage_store.go index 1913011..401b7df 100644 --- a/internal/modules/stats/usage_store.go +++ b/internal/modules/stats/usage_store.go @@ -106,16 +106,25 @@ func (s *docUsageStore) Increment(ctx context.Context, cmd string, user usageUse return nil } +// refreshUsernameLocked rewrites the username on every row of user.ID, so a +// Telegram username change carries the user's whole history with it and +// /stats user finds all of it. It also clears that username from +// other users' rows: Telegram usernames can move to a new account, and a stale +// copy would make lookups by name ambiguous. The caller must hold s.mu. func (s *docUsageStore) refreshUsernameLocked(ctx context.Context, user usageUser) error { entries, err := s.loadEntriesLocked(ctx) if err != nil { return err } for _, e := range entries { - if e.UserID != user.ID || e.Username == user.Username { + switch { + case e.UserID == user.ID && e.Username != user.Username: + e.Username = user.Username + case e.UserID != user.ID && user.Username != "" && e.Username == user.Username: + e.Username = "" + default: continue } - e.Username = user.Username if err := s.docs.Put(ctx, usageKey(e.Cmd, e.UserID), e); err != nil { return fmt.Errorf("stats refresh username %d: %w", user.ID, err) } @@ -255,6 +264,11 @@ type mongoUsageStore struct { coll *mongo.Collection } +// Increment upserts the row with $inc, so concurrent increments need no lock. +// Unsetting "deleted" revives a soft-deleted row that is being used again; the +// retired command is filtered out before that can happen. A follow-up +// UpdateMany propagates a username change to the user's other rows, and a +// second one clears the username from any other account that held it before. func (s *mongoUsageStore) Increment(ctx context.Context, cmd string, user usageUser, hasUser bool) error { if isRetiredCommand(cmd) { return nil @@ -301,6 +315,21 @@ func (s *mongoUsageStore) Increment(ctx context.Context, cmd string, user usageU ); err != nil { return fmt.Errorf("mongo stats refresh username %d: %w", user.ID, err) } + if user.Username == "" { + return nil + } + // The username may have moved from another account; drop the stale copy so + // lookups by name resolve to one user. + if _, err := s.coll.UpdateMany(ctx, + bson.M{"uid": bson.M{"$ne": user.ID}, "user": user.Username}, + bson.M{ + "$unset": bson.M{"user": ""}, + "$inc": bson.M{"version": int64(1)}, + "$currentDate": bson.M{"updatedAt": true}, + }, + ); err != nil { + return fmt.Errorf("mongo stats clear moved username %s: %w", user.Username, err) + } return nil }