diff --git a/README.md b/README.md index 234c228..93900e8 100644 --- a/README.md +++ b/README.md @@ -56,10 +56,7 @@ The config exposes the same operator-facing areas as MCXboxBroadcast: world type, and displayed MOTD data (joinability is always `joinable_by_friends`, matching MCXboxBroadcast) - gallery showcase image upload through `gallery.imagePath` -- friend sync automation and expiry settings, including last-seen history path - (stored as JSON at `friendSync.expiry.historyPath`, not Java's SQLite - database — operators migrating from MCXboxBroadcast start with a fresh - expiry history) +- friend sync automation and friend list cleanup (see below) - Slack/Discord-compatible webhook notifications - primary and sub-account token cache paths - optional HTTP proxy URL through `http.proxy` @@ -68,6 +65,36 @@ The config exposes the same operator-facing areas as MCXboxBroadcast: - relay mode through `relay.enabled`, which keeps players inside the NetherNet session instead of transferring them (see below). +### Friend list cleanup + +Xbox allows an account at most 1000 friends. Once the list is full, new players +can't add the bot. `friendSync.cleanup` keeps room for them: + +```yaml +friendSync: + cleanup: + inactiveDays: 15 # remove friends not seen for this many days; 0 = off + maxFriends: 950 # keep friends plus pending requests at or below this; 0 = off + interval: 1800 # seconds between inactive-friend checks + historyPath: cache/player_history.json +``` + +`maxFriends` is checked on every friend sync. When friends plus waiting friend +requests would go over it, the bot removes the friends it has seen least +recently to make exactly that much room, then accepts the waiting requests. It +never removes friends below `maxFriends`. A lower value leaves space for +requests that arrive between syncs. Removal ends the friendship +in both directions, so a removed player is not followed back. The bot's own +primary and sub-accounts are never removed. + +"Seen" means the player's last join, or when the bot first tracked them as a +friend. Each account keeps its own history in the JSON file at `historyPath`. +This is not Java's SQLite database, so operators migrating from MCXboxBroadcast +start with a fresh history. + +Configs from before `configVersion: 5` are migrated from `friendSync.expiry`. +They keep their inactivity setting, and `maxFriends` stays `0` until you set it. + ### Session recovery Signaling loss and repeated primary-session update failures share one recovery diff --git a/broadcaster.go b/broadcaster.go index 2386396..d079132 100644 --- a/broadcaster.go +++ b/broadcaster.go @@ -133,6 +133,18 @@ func New(conf Config) (*Broadcaster, error) { if err := conf.Relay.validate(); err != nil { return nil, err } + if conf.FriendSync != nil { + if err := conf.FriendSync.Cleanup.validate(); err != nil { + return nil, err + } + } + for _, account := range conf.SubAccounts { + if account.FriendSync != nil { + if err := account.FriendSync.Cleanup.validate(); err != nil { + return nil, fmt.Errorf("sub-account %q: %w", account.ID, err) + } + } + } mode, err := normalizeSignalingMode(conf.SignalingMode) if err != nil { return nil, err @@ -278,12 +290,14 @@ func (b *Broadcaster) Start(ctx context.Context) error { go client.Run(b.ctx, b.log) } go b.uploadGalleryWithTimeout() + b.primeFriendHistory() if b.conf.FriendSync != nil && hasSocialClient(b.conf.XBLClient) { b.debug("starting friend sync", "auto_follow", b.conf.FriendSync.AutoFollow, "auto_unfollow", b.conf.FriendSync.AutoUnfollow, "initial_invite", b.conf.FriendSync.InitialInvite, - "expiry_enabled", b.conf.FriendSync.ExpiryEnabled, + "cleanup_inactive_days", b.conf.FriendSync.Cleanup.InactiveDays, + "cleanup_max_friends", b.conf.FriendSync.Cleanup.MaxFriends, ) syncer := b.friendSyncer() syncer.Trigger = b.startSocialSubscription(b.conf.XBLClient, b.conf.FriendSync, b.log) @@ -314,19 +328,30 @@ func (b *Broadcaster) enabledSubAccounts() (accounts []*SubAccountConfig, duplic return accounts, duplicates } -// subAccountFriendSyncActive reports whether any enabled sub-account runs a -// friend syncer sharing the primary's history store. -func (b *Broadcaster) subAccountFriendSyncActive() bool { - accounts, _ := b.enabledSubAccounts() - for _, account := range accounts { - if !subAccountHasXBLCredentials(*account) { +// primeFriendHistory reads every own account's history once before any syncer +// writes, so a history file from before per-account entries migrates to all +// of them rather than only to the first account that writes. +func (b *Broadcaster) primeFriendHistory() { + if b.conf.FriendHistory == nil { + return + } + for _, xuid := range b.ownXUIDs() { + if xuid == "" { continue } - if account.FriendSync != nil || b.conf.FriendSync != nil { - return true + if _, err := b.conf.FriendHistory.LastSeen(b.ctx, xuid); err != nil { + b.log.Error("read player history", "xuid", xuid, "err", err) } } - return false +} + +// ownXUIDs returns the XUIDs of the primary and every configured sub-account. +func (b *Broadcaster) ownXUIDs() []string { + xuids := []string{b.primaryXUID()} + for _, account := range b.conf.SubAccounts { + xuids = append(xuids, accountXUID(account)) + } + return xuids } // startSubAccountFriendSync runs a friend syncer per enabled sub-account so @@ -346,13 +371,15 @@ func (b *Broadcaster) startSubAccountFriendSync() { continue } syncLog := b.log.With("sub_account", account.ID) - syncer := FriendSyncer{ - Client: b.friendClientFor(account.XBLClient), - Config: *conf, - History: b.conf.FriendHistory, - Notifier: b.conf.Notifier, - Trigger: b.startSocialSubscription(account.XBLClient, conf, syncLog), - Log: syncLog, + syncer := &FriendSyncer{ + Client: b.friendClientFor(account.XBLClient), + Config: *conf, + History: b.conf.FriendHistory, + Account: accountXUID(*account), + OwnAccounts: b.ownXUIDs(), + Notifier: b.conf.Notifier, + Trigger: b.startSocialSubscription(account.XBLClient, conf, syncLog), + Log: syncLog, } if conf.InitialInvite { syncer.Inviter = &subAccountInviter{b: b, id: account.ID} @@ -363,8 +390,7 @@ func (b *Broadcaster) startSubAccountFriendSync() { } // logSocialSummary logs the authenticated account and its friend usage at -// startup, mirroring MCXboxBroadcast's "N/2000 friends" line. The count comes -// from the friend list like MCXboxBroadcast; the social summary's +// startup. The count comes from the friend list; the social summary's // targetFollowingCount is unreliable for the caller's own profile. func (b *Broadcaster) logSocialSummary() { if !hasSocialClient(b.conf.XBLClient) { @@ -372,15 +398,26 @@ func (b *Broadcaster) logSocialSummary() { } ctx, cancel := context.WithTimeout(b.ctx, 15*time.Second) defer cancel() - friends, err := b.friendClientFor(b.conf.XBLClient).Friends(ctx) + people, err := b.friendClientFor(b.conf.XBLClient).Friends(ctx) if err != nil { b.debug("fetch friend list for summary", "err", err) return } + // Xbox caps the people an account follows; followers are unlimited. + friends, followers := 0, 0 + for _, p := range people { + if p.IsFollowedByCaller { + friends++ + } + if p.IsFollowingCaller { + followers++ + } + } b.info("authenticated to xbox live", "gamertag", b.hostNameFallback(), "xuid", b.primaryXUID(), - "friends", fmt.Sprintf("%d/2000", len(friends)), + "friends", fmt.Sprintf("%d/%d", friends, XboxFriendLimit), + "followers", followers, ) } @@ -415,18 +452,15 @@ func (b *Broadcaster) presenceClients() []PresenceClient { } // friendSyncer creates a FriendSyncer from the broadcaster's current config. -func (b *Broadcaster) friendSyncer() FriendSyncer { - syncer := FriendSyncer{ - Client: b.friendClientFor(b.conf.XBLClient), - Config: *b.conf.FriendSync, - History: b.conf.FriendHistory, - Notifier: b.conf.Notifier, - // Pruning compares the store against the primary's friend list only, - // so it must stay off while sub-account syncers share the store: - // people who only friended a sub-account would be pruned and re-seeded - // with a fresh expiry clock every pass. - PruneHistory: !b.subAccountFriendSyncActive(), - Log: b.log, +func (b *Broadcaster) friendSyncer() *FriendSyncer { + syncer := &FriendSyncer{ + Client: b.friendClientFor(b.conf.XBLClient), + Config: *b.conf.FriendSync, + History: b.conf.FriendHistory, + Account: b.primaryXUID(), + OwnAccounts: b.ownXUIDs(), + Notifier: b.conf.Notifier, + Log: b.log, } if b.conf.FriendSync.InitialInvite { syncer.Inviter = &broadcasterInviter{b: b} @@ -1434,8 +1468,8 @@ func (b *Broadcaster) transfer(conn transferConn) { b.log.Error("flush transfer", "xuid", id.XUID, "name", id.DisplayName, "err", err) return } - if recorder, ok := b.conf.FriendHistory.(HistoryRecorder); ok && id.XUID != "" { - if err := recorder.Seen(b.ctx, id.XUID, time.Now()); err != nil { + if b.conf.FriendHistory != nil && id.XUID != "" { + if err := b.conf.FriendHistory.Seen(b.ctx, id.XUID, time.Now()); err != nil { b.log.Error("record player history", "xuid", id.XUID, "err", err) } } diff --git a/broadcaster_test.go b/broadcaster_test.go index eba7bf1..6831b4f 100644 --- a/broadcaster_test.go +++ b/broadcaster_test.go @@ -226,22 +226,31 @@ func TestBroadcasterStartSubAccountsStopsQuietlyOnContextCancel(t *testing.T) { } } -func TestBroadcasterPrimarySyncerDisablesPruningWithSubAccountSyncers(t *testing.T) { +// Each syncer keys history by its own account and never removes any bot account. +func TestBroadcasterFriendSyncerKeysHistoryByAccount(t *testing.T) { conf := Config{ - XBLClient: &xsapi.Client{}, - XUID: "100", - FriendSync: &FriendSyncConfig{AutoFollow: true, ExpiryEnabled: true}, + XBLClient: &xsapi.Client{}, + XUID: "100", + FriendSync: &FriendSyncConfig{AutoFollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}, + SubAccounts: []SubAccountConfig{{ID: "sub", Enabled: true, XBLClient: &xsapi.Client{}, XUID: "200"}}, } - - solo := &Broadcaster{log: testBroadcasterLogger(), conf: conf} - if !solo.friendSyncer().PruneHistory { - t.Fatal("primary syncer should prune when it owns the history store alone") + syncer := (&Broadcaster{log: testBroadcasterLogger(), conf: conf}).friendSyncer() + if syncer.Account != "100" { + t.Fatalf("primary syncer account = %q, want 100", syncer.Account) } + if got := strings.Join(syncer.OwnAccounts, ","); got != "100,200" { + t.Fatalf("own accounts = %q, want 100,200", got) + } +} - conf.SubAccounts = []SubAccountConfig{{ID: "sub", Enabled: true, XBLClient: &xsapi.Client{}, XUID: "200"}} - shared := &Broadcaster{log: testBroadcasterLogger(), conf: conf} - if shared.friendSyncer().PruneHistory { - t.Fatal("primary syncer must not prune a history store shared with sub-account syncers") +func TestNewRejectsFriendLimitAboveXboxCap(t *testing.T) { + _, err := New(Config{ + XBLTokenSource: staticTokenSource{}, + Server: ServerInfo{Host: "127.0.0.1", Port: 19132}, + FriendSync: &FriendSyncConfig{Cleanup: FriendCleanupConfig{MaxFriends: XboxFriendLimit + 1}}, + }) + if err == nil { + t.Fatal("expected maxFriends above the Xbox friend limit to be rejected") } } diff --git a/config.example.yml b/config.example.yml index 1b17185..f31abcc 100644 --- a/config.example.yml +++ b/config.example.yml @@ -1,4 +1,4 @@ -configVersion: 4 +configVersion: 5 debugMode: false suppressSessionUpdateMessage: false @@ -32,10 +32,15 @@ friendSync: autoFollow: true autoUnfollow: true initialInvite: true - expiry: - enabled: true - days: 8 - check: 1800 + # Keeps room on the friend list (Xbox allows 1000). 0 turns a rule off. + cleanup: + # Remove friends not seen for this many days. + inactiveDays: 15 + # Keep friends plus pending requests at or below this, removing the least + # recently seen friends first. + maxFriends: 950 + # Seconds between inactive-friend checks. + interval: 1800 historyPath: cache/player_history.json notifications: diff --git a/config.go b/config.go index 4b1cd04..3b2676a 100644 --- a/config.go +++ b/config.go @@ -53,7 +53,8 @@ type Config struct { SuppressSessionUpdateMessage bool // FriendSync controls optional follower/friend synchronization. FriendSync *FriendSyncConfig - // FriendHistory records player activity for friend expiry. + // FriendHistory records when each account's friends were last seen, for + // FriendSync cleanup. Without it, cleanup is skipped. FriendHistory HistoryStore // SubAccounts contains additional accounts that publish independently owned // MPSD sessions for the same NetherNet listener. @@ -171,9 +172,33 @@ type FriendSyncConfig struct { AutoFollow bool AutoUnfollow bool InitialInvite bool - ExpiryEnabled bool - ExpiryDays int - ExpiryCheck time.Duration + Cleanup FriendCleanupConfig +} + +// FriendCleanupConfig removes friends so the list keeps room for new players. +// Each rule is off at zero, and the two can be combined. +type FriendCleanupConfig struct { + // InactiveDays removes friends not seen for this many days. + InactiveDays int + // MaxFriends keeps friends plus pending friend requests at or below this + // count by removing the least recently seen friends. At most XboxFriendLimit. + MaxFriends int + // Interval spaces inactive-friend checks. MaxFriends is enforced on every sync. + Interval time.Duration +} + +func (c FriendCleanupConfig) enabled() bool { + return c.InactiveDays > 0 || c.MaxFriends > 0 +} + +func (c FriendCleanupConfig) validate() error { + if c.InactiveDays < 0 { + return fmt.Errorf("friend cleanup inactive days must not be negative (got %d)", c.InactiveDays) + } + if c.MaxFriends < 0 || c.MaxFriends > XboxFriendLimit { + return fmt.Errorf("friend cleanup max friends must be between 0 and %d (got %d)", XboxFriendLimit, c.MaxFriends) + } + return nil } type SubAccountConfig struct { diff --git a/config_file.go b/config_file.go index 7babbee..9135edf 100644 --- a/config_file.go +++ b/config_file.go @@ -22,7 +22,7 @@ import ( "gopkg.in/yaml.v3" ) -const CurrentConfigVersion = 4 +const CurrentConfigVersion = 5 // exampleServerHost is the placeholder target in generated configs; the // broadcaster refuses to start until an operator replaces it. @@ -85,20 +85,27 @@ type SessionInfoFile struct { } type FriendFileConfig struct { - UpdateInterval int `yaml:"updateInterval" toml:"updateInterval"` - AutoFollow bool `yaml:"autoFollow" toml:"autoFollow"` - AutoUnfollow bool `yaml:"autoUnfollow" toml:"autoUnfollow"` - InitialInvite bool `yaml:"initialInvite" toml:"initialInvite"` - Expiry FriendExpiryFile `yaml:"expiry" toml:"expiry"` + UpdateInterval int `yaml:"updateInterval" toml:"updateInterval"` + AutoFollow bool `yaml:"autoFollow" toml:"autoFollow"` + AutoUnfollow bool `yaml:"autoUnfollow" toml:"autoUnfollow"` + InitialInvite bool `yaml:"initialInvite" toml:"initialInvite"` + Cleanup FriendCleanupFile `yaml:"cleanup" toml:"cleanup"` } -type FriendExpiryFile struct { - Enabled bool `yaml:"enabled" toml:"enabled"` - Days int `yaml:"days" toml:"days"` - Check int `yaml:"check" toml:"check"` - HistoryPath string `yaml:"historyPath" toml:"historyPath"` +// FriendCleanupFile is the file form of FriendCleanupConfig, with Interval in seconds. +type FriendCleanupFile struct { + InactiveDays int `yaml:"inactiveDays" toml:"inactiveDays"` + MaxFriends int `yaml:"maxFriends" toml:"maxFriends"` + Interval int `yaml:"interval" toml:"interval"` + HistoryPath string `yaml:"historyPath" toml:"historyPath"` } +const ( + defaultFriendInactiveDays = 15 + defaultFriendCleanupInterval = 1800 + defaultFriendHistoryPath = "cache/player_history.json" +) + type NotificationConfig struct { Enabled bool `yaml:"enabled" toml:"enabled"` WebhookURL string `yaml:"webhookUrl" toml:"webhookUrl"` @@ -162,11 +169,11 @@ func DefaultConfigFile() ConfigFile { AutoFollow: true, AutoUnfollow: true, InitialInvite: true, - Expiry: FriendExpiryFile{ - Enabled: true, - Days: 15, - Check: 1800, - HistoryPath: "cache/player_history.json", + Cleanup: FriendCleanupFile{ + InactiveDays: defaultFriendInactiveDays, + MaxFriends: 950, + Interval: defaultFriendCleanupInterval, + HistoryPath: defaultFriendHistoryPath, }, }, Notifications: NotificationConfig{}, @@ -243,7 +250,8 @@ func LoadConfigFile(path string) (ConfigFile, error) { cfg.Notes = append(cfg.Notes, notes...) loadedVersion := cfg.ConfigVersion cfg.migrate() - if loadedVersion != cfg.ConfigVersion { + // An unversioned file decodes at the current version, so also save when a step changed it. + if loadedVersion != cfg.ConfigVersion || len(notes) > 0 { if err := SaveConfigFile(path, cfg); err != nil { cfg.Notes = append(cfg.Notes, fmt.Sprintf("could not persist migrated config: %v", err)) } @@ -267,6 +275,7 @@ func SaveConfigFile(path string, cfg ConfigFile) error { // returns one operator note per change; no note means the document is unchanged. var configMigrations = map[int]func(doc map[string]any) []string{ 4: migrateDropSessionOverrides, + 5: migrateFriendExpiry, } // migrateDropSessionOverrides removes keys this broadcaster never honours or @@ -287,6 +296,64 @@ func migrateDropSessionOverrides(doc map[string]any) []string { return notes } +// migrateFriendExpiry turns friendSync.expiry into friendSync.cleanup the way +// the old loader read it. maxFriends stays off so existing deployments keep +// their behaviour until they opt in. +func migrateFriendExpiry(doc map[string]any) []string { + friendSync, _ := doc["friendSync"].(map[string]any) + if friendSync == nil { + friendSync = map[string]any{} + doc["friendSync"] = friendSync + } + if _, ok := friendSync["cleanup"]; ok { + if deleteConfigKey(friendSync, "expiry") { + return []string{"removed friendSync.expiry: friendSync.cleanup replaces it"} + } + return nil + } + expiry, _ := friendSync["expiry"].(map[string]any) + delete(friendSync, "expiry") + enabled := true + if v, ok := expiry["enabled"].(bool); ok { + enabled = v + } + days := configInt(expiry["days"], defaultFriendInactiveDays) + if days <= 0 { + days = defaultFriendInactiveDays + } + historyPath, _ := expiry["historyPath"].(string) + if historyPath == "" { + historyPath = defaultFriendHistoryPath + } + cleanup := map[string]any{ + "inactiveDays": 0, + "maxFriends": 0, + "interval": configInt(expiry["check"], defaultFriendCleanupInterval), + "historyPath": historyPath, + } + if enabled { + cleanup["inactiveDays"] = days + } + friendSync["cleanup"] = cleanup + return []string{fmt.Sprintf("migrated friendSync.expiry to friendSync.cleanup (inactiveDays %d); set friendSync.cleanup.maxFriends to keep room for new friends", cleanup["inactiveDays"])} +} + +// configInt reads a whole number decoded from YAML or TOML, or returns def. +func configInt(v any, def int) int { + switch n := v.(type) { + case int: + return n + case int64: + return int(n) + case uint64: + return int(n) + case float64: + return int(n) + default: + return def + } +} + // deleteConfigKey removes the value at the nested key path and reports whether it existed. func deleteConfigKey(doc map[string]any, path ...string) bool { for _, key := range path[:len(path)-1] { @@ -316,16 +383,16 @@ func (c *ConfigFile) migrate() { c.note("friendSync.updateInterval %d is below the 20 second minimum; using 20", c.FriendSync.UpdateInterval) c.FriendSync.UpdateInterval = 20 } - if c.FriendSync.Expiry.Days <= 0 { - c.note("friendSync.expiry.days %d is invalid; using 15", c.FriendSync.Expiry.Days) - c.FriendSync.Expiry.Days = 15 + cleanup := &c.FriendSync.Cleanup + if cleanup.Interval <= 0 { + c.note("friendSync.cleanup.interval %d is invalid; using %d", cleanup.Interval, defaultFriendCleanupInterval) + cleanup.Interval = defaultFriendCleanupInterval } - if c.FriendSync.Expiry.Check <= 0 { - c.note("friendSync.expiry.check %d is invalid; using 1800", c.FriendSync.Expiry.Check) - c.FriendSync.Expiry.Check = 1800 + if cleanup.HistoryPath == "" { + cleanup.HistoryPath = defaultFriendHistoryPath } - if c.FriendSync.Expiry.HistoryPath == "" { - c.FriendSync.Expiry.HistoryPath = "cache/player_history.json" + if cleanup.MaxFriends == XboxFriendLimit { + c.note("friendSync.cleanup.maxFriends %d is the Xbox friend limit, so new friend requests can fail until cleanup runs; a lower value keeps room for them", cleanup.MaxFriends) } if c.Gallery.ImagePath == "" { c.Gallery.ImagePath = "screenshot.jpg" @@ -378,8 +445,11 @@ func (c ConfigFile) RuntimeConfig(in RuntimeConfigInput) (Config, error) { SuppressSessionUpdateMessage: c.SuppressSessionUpdateMessage, FriendSync: c.FriendSync.runtime(), } - if c.FriendSync.Expiry.Enabled { - cfg.FriendHistory = NewFileHistoryStore(resolvePath(in.BaseDir, c.FriendSync.Expiry.HistoryPath)) + // Friend sync needs history even with cleanup off, to finish earlier removals. + if cfg.FriendSync != nil { + history := NewFileHistoryStore(resolvePath(in.BaseDir, c.FriendSync.Cleanup.HistoryPath)) + history.Log = in.Log + cfg.FriendHistory = history } if c.Relay.Enabled { cfg.Relay = &RelayConfig{} @@ -420,7 +490,12 @@ func (r ICEPortRangeFile) listenConfig() (nethernet.ListenConfig, error) { } func (f FriendFileConfig) runtime() *FriendSyncConfig { - if !f.AutoFollow && !f.AutoUnfollow && !f.Expiry.Enabled { + cleanup := FriendCleanupConfig{ + InactiveDays: f.Cleanup.InactiveDays, + MaxFriends: f.Cleanup.MaxFriends, + Interval: time.Duration(f.Cleanup.Interval) * time.Second, + } + if !f.AutoFollow && !f.AutoUnfollow && !cleanup.enabled() { return nil } return &FriendSyncConfig{ @@ -428,9 +503,7 @@ func (f FriendFileConfig) runtime() *FriendSyncConfig { AutoFollow: f.AutoFollow, AutoUnfollow: f.AutoUnfollow, InitialInvite: f.InitialInvite, - ExpiryEnabled: f.Expiry.Enabled, - ExpiryDays: f.Expiry.Days, - ExpiryCheck: time.Duration(f.Expiry.Check) * time.Second, + Cleanup: cleanup, } } diff --git a/config_file_test.go b/config_file_test.go index 6880916..d9fe50c 100644 --- a/config_file_test.go +++ b/config_file_test.go @@ -46,8 +46,8 @@ func TestLoadConfigFileCreatesDefaultsAndRefusesToRun(t *testing.T) { if cfg.Gallery.ImagePath != "screenshot.jpg" { t.Fatalf("unexpected image path %q", cfg.Gallery.ImagePath) } - if cfg.FriendSync.Expiry.HistoryPath != "cache/player_history.json" { - t.Fatalf("unexpected history path %q", cfg.FriendSync.Expiry.HistoryPath) + if want := (FriendCleanupFile{InactiveDays: 15, MaxFriends: 950, Interval: 1800, HistoryPath: "cache/player_history.json"}); cfg.FriendSync.Cleanup != want { + t.Fatalf("friend cleanup = %#v, want %#v", cfg.FriendSync.Cleanup, want) } } @@ -175,8 +175,8 @@ accounts: if cfg.FriendSync.UpdateInterval != 75 || cfg.FriendSync.AutoFollow || !cfg.FriendSync.AutoUnfollow || cfg.FriendSync.InitialInvite { t.Fatalf("canonical friendSync keys were not loaded: %#v", cfg.FriendSync) } - if cfg.FriendSync.Expiry.HistoryPath != "cache/upstream_history.json" || cfg.FriendSync.Expiry.Enabled { - t.Fatalf("canonical friendSync expiry keys were not loaded: %#v", cfg.FriendSync.Expiry) + if want := (FriendCleanupFile{Interval: 2400, HistoryPath: "cache/upstream_history.json"}); cfg.FriendSync.Cleanup != want { + t.Fatalf("friendSync expiry was not migrated: %#v, want %#v", cfg.FriendSync.Cleanup, want) } if cfg.Notifications.WebhookURL != "https://example.net/webhook" { t.Fatalf("canonical notification key was not loaded: %#v", cfg.Notifications) @@ -369,7 +369,7 @@ func TestConfigFileDisablesFriendSyncWhenNoActionsConfigured(t *testing.T) { cfg := DefaultConfigFile() cfg.FriendSync.AutoFollow = false cfg.FriendSync.AutoUnfollow = false - cfg.FriendSync.Expiry.Enabled = false + cfg.FriendSync.Cleanup = FriendCleanupFile{} runtime, err := cfg.RuntimeConfig(RuntimeConfigInput{ XBLTokenSource: staticTokenSource{}, @@ -382,6 +382,19 @@ func TestConfigFileDisablesFriendSyncWhenNoActionsConfigured(t *testing.T) { } } +// With cleanup off, friend sync still needs history to finish earlier removals. +func TestConfigFileKeepsFriendHistoryWithCleanupOff(t *testing.T) { + cfg := DefaultConfigFile() + cfg.FriendSync.Cleanup = FriendCleanupFile{HistoryPath: "cache/player_history.json"} + runtime, err := cfg.RuntimeConfig(RuntimeConfigInput{XBLTokenSource: staticTokenSource{}}) + if err != nil { + t.Fatal(err) + } + if runtime.FriendSync == nil || runtime.FriendHistory == nil { + t.Fatalf("friend sync = %#v history = %v, want both set", runtime.FriendSync, runtime.FriendHistory) + } +} + func TestHTTPConfigClientConfiguresProxyTransport(t *testing.T) { cfg := HTTPFileConfig{Proxy: "http://127.0.0.1:8080"} @@ -444,7 +457,119 @@ ip = "bedrock.test" if cfg.Session.UpdateInterval != 20 { t.Fatalf("expected interval clamp during migration, got %d", cfg.Session.UpdateInterval) } - if cfg.FriendSync.Expiry.HistoryPath != "cache/player_history.json" { - t.Fatalf("expected default history path, got %q", cfg.FriendSync.Expiry.HistoryPath) + if cfg.FriendSync.Cleanup.HistoryPath != "cache/player_history.json" { + t.Fatalf("expected default history path, got %q", cfg.FriendSync.Cleanup.HistoryPath) + } +} + +// Old expiry settings must keep working after an upgrade, with the new +// capacity limit left off until the operator opts in. +func TestLoadConfigFileMigratesFriendExpiry(t *testing.T) { + tests := []struct { + name string + file string + data string + want FriendCleanupFile + }{ + { + name: "enabled", + file: "config.yml", + data: "configVersion: 3\nfriendSync:\n expiry:\n enabled: true\n days: 7\n check: 600\n historyPath: cache/h.json\n", + want: FriendCleanupFile{InactiveDays: 7, Interval: 600, HistoryPath: "cache/h.json"}, + }, + { + name: "production v3", + file: "config.yml", + data: "configVersion: 3\ndebugMode: false\nfriendSync:\n updateInterval: 60\n autoFollow: true\n autoUnfollow: true\n initialInvite: true\n expiry:\n enabled: true\n days: 15\n check: 1800\n historyPath: cache/player_history.json\n", + want: FriendCleanupFile{InactiveDays: 15, Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "disabled", + file: "config.yml", + data: "configVersion: 3\nfriendSync:\n expiry:\n enabled: false\n days: 7\n", + want: FriendCleanupFile{Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "partial block keeps old defaults", + file: "config.yml", + data: "configVersion: 3\nfriendSync:\n expiry:\n days: 9\n", + want: FriendCleanupFile{InactiveDays: 9, Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "absent", + file: "config.yml", + data: "configVersion: 3\n", + want: FriendCleanupFile{InactiveDays: 15, Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "toml", + file: "config.toml", + data: "configVersion = 3\n[friendSync.expiry]\nenabled = true\ndays = 5\n", + want: FriendCleanupFile{InactiveDays: 5, Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "unversioned expiry block", + file: "config.yml", + data: "friendSync:\n expiry:\n enabled: true\n days: 4\n", + want: FriendCleanupFile{InactiveDays: 4, Interval: 1800, HistoryPath: "cache/player_history.json"}, + }, + { + name: "current version keeps cleanup", + file: "config.yml", + data: "configVersion: 4\nfriendSync:\n cleanup:\n inactiveDays: 3\n maxFriends: 900\n interval: 60\n", + want: FriendCleanupFile{InactiveDays: 3, MaxFriends: 900, Interval: 60, HistoryPath: "cache/player_history.json"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), tt.file) + if err := os.WriteFile(path, []byte(tt.data+testTarget(tt.file)), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := LoadConfigFile(path) + if err != nil { + t.Fatal(err) + } + if cfg.FriendSync.Cleanup != tt.want || cfg.ConfigVersion != CurrentConfigVersion { + t.Fatalf("version %d cleanup = %#v, want %d and %#v", cfg.ConfigVersion, cfg.FriendSync.Cleanup, CurrentConfigVersion, tt.want) + } + // The rewritten file must load to the same settings. + reloaded, err := LoadConfigFile(path) + if err != nil { + t.Fatal(err) + } + if reloaded.FriendSync.Cleanup != tt.want { + t.Fatalf("reloaded cleanup = %#v, want %#v", reloaded.FriendSync.Cleanup, tt.want) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "expiry") { + t.Fatalf("migrated file still has an expiry block:\n%s", data) + } + }) + } +} + +func TestLoadConfigFileNotesFriendLimitWithoutHeadroom(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(path, []byte("configVersion: 5\nfriendSync:\n cleanup:\n maxFriends: 1000\n"+testTarget(path)), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := LoadConfigFile(path) + if err != nil { + t.Fatal(err) + } + if !slices.ContainsFunc(cfg.Notes, func(note string) bool { return strings.Contains(note, "maxFriends 1000") }) { + t.Fatalf("notes = %q, want a maxFriends headroom note", cfg.Notes) + } +} + +// testTarget returns a session target block for path's format, since the example host is refused. +func testTarget(path string) string { + if strings.HasSuffix(path, ".toml") { + return "\n[session.sessionInfo]\nip = \"bedrock.test\"\n" } + return "session:\n sessionInfo:\n ip: bedrock.test\n" } diff --git a/constants.go b/constants.go index 5a0dfbc..b07b1d2 100644 --- a/constants.go +++ b/constants.go @@ -7,6 +7,9 @@ const ( TemplateName = "MinecraftLobby" TitleID = 896928775 + // XboxFriendLimit is the most friends Xbox Live allows one account. + XboxFriendLimit = 1000 + xboxLiveRelyingParty = "http://xboxlive.com" ) diff --git a/deployments/pterodactyl/egg-go-mcxboxbroadcast.json b/deployments/pterodactyl/egg-go-mcxboxbroadcast.json index 23e0637..e455dfb 100644 --- a/deployments/pterodactyl/egg-go-mcxboxbroadcast.json +++ b/deployments/pterodactyl/egg-go-mcxboxbroadcast.json @@ -22,7 +22,7 @@ }, "scripts": { "installation": { - "script": "#!/bin/ash\nset -eu\n\ncd /mnt/server\nmkdir -p cache\n\n# Emit a single-quoted YAML scalar so panel text cannot break the document.\nyaml_string() {\n printf \"'%s'\" \"$(printf '%s' \"$1\" | tr '\\r\\n' ' ' | sed \"s/'/''/g\")\"\n}\n\nif [ ! -f config.yml ]; then\n cat > config.yml < config.yml < initial invite sent + accepted map[string]acceptedFriendRequest // accepted, not yet on the friend list + rejected map[string]time.Time // XUID -> request retry allowed +} + +type acceptedFriendRequest struct { + person Person + at time.Time } -func (s *friendSyncRunState) options(now time.Time, expire bool) friendSyncOptions { +func (s *friendSyncRunState) options(now time.Time, cleanup bool) friendSyncOptions { return friendSyncOptions{ - expire: expire, - autoFollow: !now.Before(s.followRetryUntil) && !now.Before(s.autoFollowUntil), + cleanup: cleanup, + autoFollow: !now.Before(s.followRetryUntil), autoUnfollow: !now.Before(s.unfollowRetryUntil), + acceptLimit: s.room, } } @@ -100,38 +129,71 @@ func (s *friendSyncRunState) record(now time.Time, result friendSyncResult) { if result.unfollowRetryAfter > 0 { s.unfollowRetryUntil = now.Add(result.unfollowRetryAfter) } - if result.friendListFull { - s.autoFollowUntil = now.Add(friendListFullBackoff) + if result.scanned { + s.room = result.room } } -// friendSyncResult carries the rate-limit outcomes of a sync pass. +// forget drops expired memory and makes sure the maps exist. +func (s *friendSyncRunState) forget(now time.Time) { + if s.invited == nil { + s.invited = map[string]time.Time{} + s.accepted = map[string]acceptedFriendRequest{} + s.rejected = map[string]time.Time{} + } + for xuid, at := range s.invited { + if now.Sub(at) >= friendInviteMemory { + delete(s.invited, xuid) + } + } + for xuid, req := range s.accepted { + if now.Sub(req.at) >= friendAcceptConfirmWindow { + delete(s.accepted, xuid) + } + } + for xuid, until := range s.rejected { + if !now.Before(until) { + delete(s.rejected, xuid) + } + } +} + +// friendSyncResult carries the outcomes of a sync pass. type friendSyncResult struct { readRetryAfter time.Duration followRetryAfter time.Duration unfollowRetryAfter time.Duration friendListFull bool + requests FriendRequestResult // this pass's accept outcome + waiting int // incoming friend requests left pending + joined int // friends added this pass but not in its people snapshot + scanned bool // the friend list was read + room int // people the account can still follow } -func (s FriendSyncer) Sync(ctx context.Context) error { +// Sync runs one full pass, ignoring backoff from earlier passes. +func (s *FriendSyncer) Sync(ctx context.Context) error { _, err := s.syncWithOptions(ctx, friendSyncOptions{ - expire: true, + cleanup: true, autoFollow: true, autoUnfollow: true, + acceptLimit: XboxFriendLimit, }) return err } -func (s FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOptions) (friendSyncResult, error) { +func (s *FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOptions) (friendSyncResult, error) { var result friendSyncResult if s.Client == nil { return result, nil } + s.state.forget(time.Now()) if s.Config.InitialInvite && s.Inviter == nil { s.debug(ctx, "initial invite unavailable", "reason", "session inviter is not configured") } + // Accept requests before following anyone back. if s.Config.AutoFollow && opts.autoFollow { - s.acceptPending(ctx, &result) + s.acceptPending(ctx, opts, &result) if result.readRetryAfter > 0 { return result, nil } @@ -143,6 +205,11 @@ func (s FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOption result.readRetryAfter = retryDelay(err) return result, err } + result.scanned = true + s.settleRequests(ctx, people, opts, &result) + s.confirmAccepted(ctx, people) + own := s.ownAccounts() + removing := s.removing(ctx) stats := s.friendSyncStats(people, opts) s.debug(ctx, "friend sync scan", "people", stats.people, @@ -154,7 +221,7 @@ func (s FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOption "neither", stats.neither, "auto_follow_candidates", stats.autoFollowCandidates, "auto_unfollow_candidates", stats.autoUnfollowCandidates, - "expire", opts.expire, + "cleanup", opts.cleanup, ) if stats.autoFollowCandidates > 0 { s.debug(ctx, "adding friends", "count", stats.autoFollowCandidates) @@ -164,8 +231,7 @@ func (s FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOption } added := 0 removed := 0 - expiryCandidates := 0 - expiredFriends := 0 + unfollowed := make(map[string]struct{}) for _, p := range people { if err := ctx.Err(); err != nil { return result, err @@ -173,41 +239,38 @@ func (s FriendSyncer) syncWithOptions(ctx context.Context, opts friendSyncOption if isGuestXUID(p.XUID) { continue } - if s.Config.AutoFollow && opts.autoFollow && !result.followBlocked() && p.IsFollowingCaller && !p.IsFollowedByCaller { - if s.follow(ctx, p, &result) { - added++ + if _, ok := removing[p.XUID]; ok && p.IsFollowingCaller && !p.IsFollowedByCaller { + // A removed friend who still follows must not be followed back. + if opts.autoUnfollow && !result.unfollowBlocked() { + s.finishRemoval(ctx, p, &result) } + continue } - if s.Config.AutoUnfollow && opts.autoUnfollow && !result.unfollowBlocked() && !p.IsFollowingCaller && p.IsFollowedByCaller { - if s.unfollow(ctx, p, "", &result) { - removed++ + if s.Config.AutoFollow && opts.autoFollow && !result.followBlocked() && result.room > 0 && p.IsFollowingCaller && !p.IsFollowedByCaller { + if s.follow(ctx, p, opts, &result) { + added++ + result.joined++ + result.room-- } + } + if _, isOwn := own[p.XUID]; isOwn { continue } - if opts.expire && opts.autoUnfollow && !result.unfollowBlocked() && s.Config.ExpiryEnabled && p.IsFollowedByCaller && s.History != nil { - expiryResult := s.expire(ctx, p, &result) - if expiryResult.candidate { - expiryCandidates++ - } - if expiryResult.removed { + if s.Config.AutoUnfollow && opts.autoUnfollow && !result.unfollowBlocked() && !p.IsFollowingCaller && p.IsFollowedByCaller { + if s.unfollow(ctx, p, &result) { removed++ - expiredFriends++ + result.room++ + unfollowed[p.XUID] = struct{}{} } } } - if opts.expire && s.Config.ExpiryEnabled && s.PruneHistory { - s.pruneHistory(ctx, people) - } - if opts.expire && s.Config.ExpiryEnabled && s.History != nil { - s.debug(ctx, "friend sync expiry scan", - "mutual_expiry_candidates", expiryCandidates, - "expired_friends", expiredFriends, - ) - } + cleaned := s.cleanup(ctx, people, own, unfollowed, removing, opts, &result) + removed += cleaned + result.room += cleaned if stats.autoFollowCandidates > 0 { s.debug(ctx, "added friends", "count", added) } - if stats.autoUnfollowCandidates > 0 { + if stats.autoUnfollowCandidates > 0 || removed > 0 { s.debug(ctx, "removed friends", "count", removed) } return result, nil @@ -221,59 +284,152 @@ func (r friendSyncResult) unfollowBlocked() bool { return r.unfollowRetryAfter > 0 } -// acceptPending accepts incoming friend requests, sending initial invites for -// each accepted person when configured. -func (s FriendSyncer) acceptPending(ctx context.Context, result *friendSyncResult) { - accepter, ok := s.Client.(pendingFriendRequestAccepter) +// ownAccounts returns the XUIDs cleanup and auto-unfollow must never remove. +func (s *FriendSyncer) ownAccounts() map[string]struct{} { + own := make(map[string]struct{}, len(s.OwnAccounts)+1) + for _, xuid := range append([]string{s.Account}, s.OwnAccounts...) { + if xuid != "" { + own[xuid] = struct{}{} + } + } + return own +} + +// acceptPending accepts incoming friend requests. Accepted people are +// announced once they appear on the friend list; see confirmAccepted. +func (s *FriendSyncer) acceptPending(ctx context.Context, opts friendSyncOptions, result *friendSyncResult) { + accepter, ok := s.Client.(friendRequestAccepter) if !ok { return } + now := time.Now() + skip := func(xuid string) bool { + _, awaiting := s.state.accepted[xuid] + return awaiting || now.Before(s.state.rejected[xuid]) + } s.debug(ctx, "accepting pending friend requests") operationCtx, cancel := xboxOperationContext(ctx) - accepted, err := accepter.AcceptPendingFriendRequests(operationCtx) + requests, err := accepter.AcceptPendingFriendRequests(operationCtx, opts.acceptLimit, skip) cancel() - for _, p := range accepted { - s.info(ctx, "added friend", "xuid", p.XUID, "gamertag", p.Gamertag, "source", "pending_requests") + for _, p := range requests.Accepted { + s.state.accepted[p.XUID] = acceptedFriendRequest{person: p, at: now} + s.debug(ctx, "accepted friend request", "xuid", p.XUID, "gamertag", p.Gamertag) } - if s.Config.InitialInvite && s.Inviter != nil { - for _, p := range accepted { - s.sendInitialInvite(ctx, p, "pending_requests") + result.requests = requests + if err != nil { + var responseErr *xblsocial.ResponseError + isResponse := errors.As(err, &responseErr) + switch delay := retryDelay(err); { + case delay > 0 && isResponse && responseErr.Method == http.MethodGet: + // The pending list shares PeopleHub's read quota with Friends. + result.readRetryAfter = delay + case delay > 0: + result.followRetryAfter = delay + case isResponse && responseErr.StatusCode >= 400 && responseErr.StatusCode < 500: + // Xbox refused the request as a whole; don't repeat it every pass. + result.followRetryAfter = friendRequestRetryDelay } + s.warn(ctx, "accept pending friend requests", "err", err) } - if err != nil { - if delay := retryDelay(err); delay > 0 { - var responseErr *xblsocial.ResponseError - if errors.As(err, &responseErr) && responseErr.Method == http.MethodGet { - // The pending list shares PeopleHub's read quota with Friends. - result.readRetryAfter = delay - } else { - result.followRetryAfter = delay - } +} + +// settleRequests handles this pass's refused requests once the friend list is +// known. A full-list refusal counts as the account's own list being full only +// when the account is at the Xbox limit; otherwise it is the requester's list, +// and just that request is retried later. +func (s *FriendSyncer) settleRequests(ctx context.Context, people []Person, opts friendSyncOptions, result *friendSyncResult) { + requests := result.requests + // Only requests this pass tried to accept count; with auto-follow off none are. + result.waiting = requests.Waiting + following := make(map[string]struct{}, len(people)) + for _, p := range people { + if p.IsFollowedByCaller && !isGuestXUID(p.XUID) { + following[p.XUID] = struct{}{} + } + } + for _, p := range requests.Accepted { + if _, ok := following[p.XUID]; !ok { + result.joined++ + } + } + result.room = max(XboxFriendLimit-len(following)-result.joined, 0) + ownListFull := result.room == 0 + for _, rejected := range requests.Rejected { + if ownListFull && errors.Is(rejected.Err, xblsocial.ErrFriendListFull) { + result.friendListFull = true + continue + } + if s.rejectRequest(ctx, rejected, opts, result) { + result.waiting-- + } + } + // Warn once as the list fills, not on every pass it stays full. + if ownListFull && result.waiting > 0 && (result.friendListFull || s.state.room > 0) { + s.warn(ctx, "friend list full; friend requests wait for room", "friends", len(following), "waiting", result.waiting) + } +} + +// rejectRequest handles a request Xbox refused and reports whether it is no +// longer pending. Restricted requests are declined like restricted followers; +// others are retried after friendRequestRetryDelay. +func (s *FriendSyncer) rejectRequest(ctx context.Context, rejected RejectedFriendRequest, opts friendSyncOptions, result *friendSyncResult) bool { + p := rejected.Person + if errors.Is(rejected.Err, xblsocial.ErrFriendRestricted) && opts.autoUnfollow && !result.unfollowBlocked() { + operationCtx, cancel := xboxOperationContext(ctx) + err := s.Client.RemoveFriend(operationCtx, p.XUID) + cancel() + if err == nil { + s.warn(ctx, "declined friend request due to restrictions on their account", "xuid", p.XUID, "gamertag", p.Gamertag) + s.notify(ctx, "Declined a friend request from "+p.Gamertag+" ("+p.XUID+") due to restrictions on their account.") + return true + } + result.unfollowRetryAfter = max(result.unfollowRetryAfter, retryDelay(err)) + s.warn(ctx, "decline restricted friend request", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) + } + s.state.rejected[p.XUID] = time.Now().Add(friendRequestRetryDelay) + s.warn(ctx, "friend request not accepted; retrying later", "xuid", p.XUID, "gamertag", p.Gamertag, "retry_in", friendRequestRetryDelay, "err", rejected.Err) + return false +} + +// confirmAccepted announces accepted requests once the person is on the friend +// list. Xbox can report a request as accepted while it stays pending. +func (s *FriendSyncer) confirmAccepted(ctx context.Context, people []Person) { + if len(s.state.accepted) == 0 { + return + } + for _, p := range people { + req, ok := s.state.accepted[p.XUID] + if !ok || !p.IsFollowedByCaller { + continue } - s.logPendingFriendAcceptError(err) + delete(s.state.accepted, p.XUID) + s.forget(ctx, p.XUID) // a new friendship starts a fresh clock + s.info(ctx, "added friend", "xuid", p.XUID, "gamertag", req.person.Gamertag, "source", "pending_requests") + s.sendInitialInvite(ctx, req.person, "pending_requests") } } // follow follows p back and reports whether the friendship was established. -// Restricted accounts are force-unfollowed so they are not retried forever. -func (s FriendSyncer) follow(ctx context.Context, p Person, result *friendSyncResult) bool { +// Restricted accounts are dropped as followers so they are not retried forever. +func (s *FriendSyncer) follow(ctx context.Context, p Person, opts friendSyncOptions, result *friendSyncResult) bool { operationCtx, cancel := xboxOperationContext(ctx) err := s.Client.Follow(operationCtx, p.XUID) cancel() if err == nil { s.info(ctx, "added friend", "xuid", p.XUID, "gamertag", p.Gamertag) - if s.Config.InitialInvite && s.Inviter != nil { - s.sendInitialInvite(ctx, p, "auto_follow") - } + s.sendInitialInvite(ctx, p, "auto_follow") return true } s.logFriendSyncError("follow", p, err) s.debug(ctx, "failed to add friend", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) switch { case errors.Is(err, xblsocial.ErrFriendRestricted): - s.dropRestrictedFollower(ctx, p) + if opts.autoUnfollow && !result.unfollowBlocked() { + s.dropRestrictedFollower(ctx, p, result) + } case errors.Is(err, xblsocial.ErrFriendListFull): result.friendListFull = true + result.room = 0 // Xbox's count wins over the snapshot's default: if delay := retryDelay(err); delay > 0 { result.followRetryAfter = delay @@ -284,120 +440,277 @@ func (s FriendSyncer) follow(ctx context.Context, p Person, result *friendSyncRe // dropRestrictedFollower removes a privacy-restricted follower so the account // stops showing up as an auto-follow candidate on every pass. -func (s FriendSyncer) dropRestrictedFollower(ctx context.Context, p Person) { - unfollower, ok := s.Client.(forceUnfollower) - if !ok { - return - } +func (s *FriendSyncer) dropRestrictedFollower(ctx context.Context, p Person, result *friendSyncResult) { operationCtx, cancel := xboxOperationContext(ctx) - err := unfollower.ForceUnfollow(operationCtx, p.XUID) + err := s.Client.RemoveFollower(operationCtx, p.XUID) cancel() if err != nil { + result.unfollowRetryAfter = max(result.unfollowRetryAfter, retryDelay(err)) if s.Log != nil { - s.Log.Error("force unfollow restricted account", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) + s.Log.Error("remove restricted follower", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) } return } - if s.History != nil { - _ = s.History.Clear(ctx, p.XUID) - } + s.forget(ctx, p.XUID) s.warn(ctx, "removed friend due to restrictions on their account", "xuid", p.XUID, "gamertag", p.Gamertag) s.notify(ctx, "Removed "+p.Gamertag+" ("+p.XUID+") as a friend due to restrictions on their account.") } -// unfollow removes p and reports whether the removal succeeded. -func (s FriendSyncer) unfollow(ctx context.Context, p Person, reason string, result *friendSyncResult) bool { +// unfollow drops the account's follow of p, who no longer follows back. +func (s *FriendSyncer) unfollow(ctx context.Context, p Person, result *friendSyncResult) bool { operationCtx, cancel := xboxOperationContext(ctx) err := s.Client.Unfollow(operationCtx, p.XUID) cancel() if err != nil { s.debug(ctx, "failed to remove friend", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) - if delay := retryDelay(err); delay > 0 { - result.unfollowRetryAfter = delay + result.unfollowRetryAfter = max(result.unfollowRetryAfter, retryDelay(err)) + return false + } + s.info(ctx, "removed friend", "xuid", p.XUID, "gamertag", p.Gamertag) + s.forget(ctx, p.XUID) + return true +} + +// removeFriend ends the account's relationship with p and reports whether it +// ended. A follower is marked in History before anything changes on Xbox, so +// a crash or a failed follower removal leaves a later pass to finish the +// removal instead of following p back. +func (s *FriendSyncer) removeFriend(ctx context.Context, p Person, reason string, lastSeen time.Time, result *friendSyncResult) bool { + if p.IsFollowingCaller { + if err := s.History.MarkRemoving(ctx, s.Account, time.Now(), p.XUID); err != nil { + if s.Log != nil { + s.Log.Error("record pending friend removal", "xuid", p.XUID, "err", err) + } + return false + } + } + if err := s.endRelationship(ctx, p); err != nil { + s.debug(ctx, "failed to remove friend", "xuid", p.XUID, "gamertag", p.Gamertag, "reason", reason, "err", err) + result.unfollowRetryAfter = max(result.unfollowRetryAfter, retryDelay(err)) + if p.IsFollowingCaller { + s.restoreTracking(ctx, p.XUID, lastSeen) } return false } - if reason == "" { - s.info(ctx, "removed friend", "xuid", p.XUID, "gamertag", p.Gamertag) - } else { - s.info(ctx, "removed friend", "xuid", p.XUID, "gamertag", p.Gamertag, "reason", reason) + s.info(ctx, "removed friend", "xuid", p.XUID, "gamertag", p.Gamertag, "reason", reason, "last_seen", lastSeen) + if !p.IsFollowingCaller { + s.forget(ctx, p.XUID) + } else if !result.unfollowBlocked() { + s.finishRemoval(ctx, p, result) + } + return true +} + +// restoreTracking undoes a removal mark for a friend who was not removed. +func (s *FriendSyncer) restoreTracking(ctx context.Context, xuid string, lastSeen time.Time) { + s.forget(ctx, xuid) + if err := s.History.Track(ctx, s.Account, lastSeen, xuid); err != nil && s.Log != nil { + s.Log.Error("record player history", "xuid", xuid, "err", err) + } +} + +// endRelationship frees p's slot on the account's list: a one-way follow is +// unfollowed, and a friendship is ended, falling back to unfollowing when +// Xbox has no friendship record for the mutual follow. +func (s *FriendSyncer) endRelationship(ctx context.Context, p Person) error { + operationCtx, cancel := xboxOperationContext(ctx) + defer cancel() + if !p.IsFollowingCaller { + return s.Client.Unfollow(operationCtx, p.XUID) } - if s.History != nil { - _ = s.History.Clear(ctx, p.XUID) + err := s.Client.RemoveFriend(operationCtx, p.XUID) + if isNotFound(err) { + return s.Client.Unfollow(operationCtx, p.XUID) } + return err +} + +// finishRemoval drops a removed friend's follow of the account and reports +// whether it succeeded. +func (s *FriendSyncer) finishRemoval(ctx context.Context, p Person, result *friendSyncResult) bool { + operationCtx, cancel := xboxOperationContext(ctx) + err := s.Client.RemoveFollower(operationCtx, p.XUID) + cancel() + if err != nil { + s.debug(ctx, "failed to remove follower", "xuid", p.XUID, "gamertag", p.Gamertag, "err", err) + result.unfollowRetryAfter = max(result.unfollowRetryAfter, retryDelay(err)) + return false + } + s.forget(ctx, p.XUID) return true } -type friendExpiryResult struct { - candidate bool - removed bool +// removing returns the people this account is still finishing removing. It is +// read even with cleanup off, so an earlier removal is never followed back. +func (s *FriendSyncer) removing(ctx context.Context) map[string]time.Time { + if s.History == nil { + return nil + } + removing, err := s.History.Removing(ctx, s.Account) + if err != nil && s.Log != nil { + s.Log.Error("read pending friend removals", "err", err) + } + return removing +} + +// friendCleanupCandidate is a friend cleanup has chosen to remove. +type friendCleanupCandidate struct { + person Person + lastSeen time.Time + reason string } -// expire removes p when they have not been seen within the expiry window and -// reports whether p was an expiry candidate and whether a removal happened. -func (s FriendSyncer) expire(ctx context.Context, p Person, result *friendSyncResult) friendExpiryResult { - lastSeen, ok, err := s.History.LastSeen(ctx, p.XUID) +// cleanup removes inactive friends on cleanup passes and, on every pass, the +// least recently seen friends needed to keep the list within MaxFriends. It +// returns how many friends it removed. +func (s *FriendSyncer) cleanup(ctx context.Context, people []Person, own, unfollowed map[string]struct{}, removing map[string]time.Time, opts friendSyncOptions, result *friendSyncResult) int { + conf := s.Config.Cleanup + if s.History == nil || !conf.enabled() || (!opts.cleanup && conf.MaxFriends == 0) { + return 0 + } + now := time.Now() + lastSeen, err := s.History.LastSeen(ctx, s.Account) if err != nil { if s.Log != nil { - s.Log.Error("read player history", "xuid", p.XUID, "err", err) + s.Log.Error("read player history", "err", err) } - return friendExpiryResult{} + return 0 } - if !ok { - if recorder, ok := s.History.(HistoryRecorder); ok { - if err := recorder.Seen(ctx, p.XUID, time.Now()); err != nil && s.Log != nil { - s.Log.Error("record player history", "xuid", p.XUID, "err", err) + if lastSeen == nil { + lastSeen = map[string]time.Time{} + } + // Xbox caps the people the account follows, so that is what counts, + // including friends this pass added after the snapshot was taken. + count := result.joined + var friends []Person + var untracked []string + for _, p := range people { + if _, ok := unfollowed[p.XUID]; ok || !p.IsFollowedByCaller || isGuestXUID(p.XUID) { + continue + } + count++ + if _, ok := own[p.XUID]; ok { + continue + } + if _, ok := lastSeen[p.XUID]; !ok { + lastSeen[p.XUID] = now + untracked = append(untracked, p.XUID) + } + friends = append(friends, p) + } + if len(untracked) > 0 { + if err := s.History.Track(ctx, s.Account, now, untracked...); err != nil && s.Log != nil { + s.Log.Error("record player history", "err", err) + } + } + if opts.cleanup { + s.pruneHistory(ctx, lastSeen, removing, people, friends) + } + + slices.SortFunc(friends, func(a, b Person) int { + return cmp.Or(lastSeen[a.XUID].Compare(lastSeen[b.XUID]), cmp.Compare(a.XUID, b.XUID)) + }) + var candidates []friendCleanupCandidate + if opts.cleanup && conf.InactiveDays > 0 { + cutoff := now.Add(-time.Duration(conf.InactiveDays) * 24 * time.Hour) + for _, p := range friends { + if !lastSeen[p.XUID].Before(cutoff) { + break } + candidates = append(candidates, friendCleanupCandidate{p, lastSeen[p.XUID], "inactive"}) } - return friendExpiryResult{} } - expiryDays := s.Config.ExpiryDays - if expiryDays <= 0 { - expiryDays = 15 + inactive := len(candidates) + if conf.MaxFriends > 0 { + // Only ever down to MaxFriends: a full-list error alone never forces + // removals, so a mismatch with Xbox's count cannot drain the list. + need := count - inactive + result.waiting - conf.MaxFriends + for _, p := range friends[inactive:min(len(friends), inactive+max(need, 0))] { + candidates = append(candidates, friendCleanupCandidate{p, lastSeen[p.XUID], "over_capacity"}) + } } - if !lastSeen.Before(time.Now().Add(-time.Duration(expiryDays) * 24 * time.Hour)) { - return friendExpiryResult{} + s.debug(ctx, "friend cleanup scan", + "friends", count, + "waiting_requests", result.waiting, + "max_friends", conf.MaxFriends, + "inactive", inactive, + "over_capacity", len(candidates)-inactive, + "newly_tracked", len(untracked), + ) + if len(candidates) == 0 { + return 0 } - s.info(ctx, "removing inactive friend", "xuid", p.XUID, "gamertag", p.Gamertag, "last_seen", lastSeen) - return friendExpiryResult{ - candidate: true, - removed: s.unfollow(ctx, p, "inactive", result), + if !opts.autoUnfollow || result.unfollowBlocked() { + s.debug(ctx, "friend cleanup deferred by rate limit", "candidates", len(candidates)) + return 0 + } + removed := 0 + for _, c := range candidates { + if ctx.Err() != nil || result.unfollowBlocked() { + break + } + if s.removeFriend(ctx, c.person, c.reason, c.lastSeen, result) { + removed++ + } } + return removed } -// pruneHistory drops history entries for people who are no longer on the -// friend list so the store does not grow forever. -func (s FriendSyncer) pruneHistory(ctx context.Context, people []Person) { - lister, ok := s.History.(HistoryLister) - if !ok { - return - } - xuids, err := lister.XUIDs(ctx) - if err != nil { - if s.Log != nil { - s.Log.Error("list player history", "err", err) - } - return +// pruneHistory drops this account's history for people no longer its friends, +// and removal marks for people who no longer follow it. +func (s *FriendSyncer) pruneHistory(ctx context.Context, lastSeen, removing map[string]time.Time, people, friends []Person) { + current := make(map[string]struct{}, len(friends)) + for _, p := range friends { + current[p.XUID] = struct{}{} } - current := make(map[string]struct{}, len(people)) + followers := make(map[string]struct{}, len(people)) for _, p := range people { - current[p.XUID] = struct{}{} + if p.IsFollowingCaller { + followers[p.XUID] = struct{}{} + } } - for _, xuid := range xuids { - if _, ok := current[xuid]; ok { - continue + var gone []string + for xuid := range lastSeen { + if _, ok := current[xuid]; !ok { + gone = append(gone, xuid) + delete(lastSeen, xuid) } - if err := s.History.Clear(ctx, xuid); err != nil { - if s.Log != nil { - s.Log.Error("prune player history", "xuid", xuid, "err", err) - } - continue + } + for xuid := range removing { + // A mark for someone who no longer follows needs no finishing; one + // for a current friend is left by a removal that never reached Xbox. + _, follows := followers[xuid] + _, friend := current[xuid] + if !follows || friend { + gone = append(gone, xuid) } - s.debug(ctx, "pruned player history for ex-friend", "xuid", xuid) + } + if len(gone) > 0 { + s.forget(ctx, gone...) + s.debug(ctx, "pruned player history for ex-friends", "count", len(gone)) + } +} + +// forget drops xuids from this account's history. +func (s *FriendSyncer) forget(ctx context.Context, xuids ...string) { + if s.History == nil || len(xuids) == 0 { + return + } + if err := s.History.Forget(ctx, s.Account, xuids...); err != nil && s.Log != nil { + s.Log.Error("forget player history", "count", len(xuids), "err", err) } } -func (s FriendSyncer) sendInitialInvite(ctx context.Context, p Person, source string) { +// sendInitialInvite invites p to the session at most once per friendInviteMemory. +func (s *FriendSyncer) sendInitialInvite(ctx context.Context, p Person, source string) { + if !s.Config.InitialInvite || s.Inviter == nil { + return + } + if _, ok := s.state.invited[p.XUID]; ok { + s.debug(ctx, "skipping repeat initial invite", "xuid", p.XUID, "gamertag", p.Gamertag, "source", source) + return + } + s.state.invited[p.XUID] = time.Now() s.debug(ctx, "sending initial invite", "xuid", p.XUID, "gamertag", p.Gamertag, "source", source) operationCtx, cancel := xboxOperationContext(ctx) err := s.Inviter.Invite(operationCtx, p.XUID, strconv.FormatInt(TitleID, 10)) @@ -423,7 +736,7 @@ type friendSyncStats struct { autoUnfollowCandidates int } -func (s FriendSyncer) friendSyncStats(people []Person, opts friendSyncOptions) friendSyncStats { +func (s *FriendSyncer) friendSyncStats(people []Person, opts friendSyncOptions) friendSyncStats { stats := friendSyncStats{people: len(people)} for _, p := range people { if isGuestXUID(p.XUID) { @@ -455,7 +768,7 @@ func (s FriendSyncer) friendSyncStats(people []Person, opts friendSyncOptions) f return stats } -func (s FriendSyncer) logFriendSyncError(op string, p Person, err error) { +func (s *FriendSyncer) logFriendSyncError(op string, p Person, err error) { if s.Log == nil { return } @@ -467,13 +780,7 @@ func (s FriendSyncer) logFriendSyncError(op string, p Person, err error) { } } -func (s FriendSyncer) logPendingFriendAcceptError(err error) { - if s.Log != nil { - s.Log.Warn("accept pending friend requests", "err", err) - } -} - -func (s FriendSyncer) notify(ctx context.Context, message string) { +func (s *FriendSyncer) notify(ctx context.Context, message string) { if s.Notifier == nil { return } @@ -484,19 +791,19 @@ func (s FriendSyncer) notify(ctx context.Context, message string) { } } -func (s FriendSyncer) info(ctx context.Context, msg string, args ...any) { +func (s *FriendSyncer) info(ctx context.Context, msg string, args ...any) { if s.Log != nil { s.Log.InfoContext(ctx, msg, args...) } } -func (s FriendSyncer) warn(ctx context.Context, msg string, args ...any) { +func (s *FriendSyncer) warn(ctx context.Context, msg string, args ...any) { if s.Log != nil { s.Log.WarnContext(ctx, msg, args...) } } -func (s FriendSyncer) debug(ctx context.Context, msg string, args ...any) { +func (s *FriendSyncer) debug(ctx context.Context, msg string, args ...any) { if s.Log != nil { s.Log.DebugContext(ctx, msg, args...) } @@ -515,14 +822,14 @@ func retryDelay(err error) time.Duration { return 0 } -// Run combines events, polling, and expiry into spaced sync passes. -func (s FriendSyncer) Run(ctx context.Context) { +// Run combines events, polling, and cleanup into spaced sync passes. +func (s *FriendSyncer) Run(ctx context.Context) { interval := max(s.Config.UpdateInterval, friendSyncMinInterval) - expiryInterval := max(s.Config.ExpiryCheck, friendSyncMinInterval) - state := friendSyncRunState{} - nextPoll, nextExpiry := time.Now(), time.Now() + cleanupInterval := max(s.Config.Cleanup.Interval, friendSyncMinInterval) + cleanupEnabled := s.Config.Cleanup.enabled() + nextPoll, nextCleanup := time.Now(), time.Now() var nextScan time.Time - pending, expire := true, true + pending, cleanup := true, true timer := time.NewTimer(0) defer timer.Stop() for { @@ -542,26 +849,26 @@ func (s FriendSyncer) Run(ctx context.Context) { } now := time.Now() pending = pending || !now.Before(nextPoll) - if s.Config.ExpiryEnabled && !now.Before(nextExpiry) { - pending, expire = true, true + if cleanupEnabled && !now.Before(nextCleanup) { + pending, cleanup = true, true } if pending && !now.Before(nextScan) { - retryAfter := s.runSync(ctx, &state, expire) + retryAfter, again := s.runSync(ctx, cleanup) now = time.Now() nextScan = now.Add(max(friendSyncMinInterval, retryAfter)) nextPoll = now.Add(interval) - pending = retryAfter > 0 - if !pending { - if expire { - nextExpiry = now.Add(expiryInterval) + if retryAfter == 0 { + if cleanup { + nextCleanup = now.Add(cleanupInterval) } - expire = false + cleanup = false } + pending = retryAfter > 0 || again } nextWake := nextPoll - if s.Config.ExpiryEnabled && nextExpiry.Before(nextWake) { - nextWake = nextExpiry + if cleanupEnabled && nextCleanup.Before(nextWake) { + nextWake = nextCleanup } if pending || nextWake.Before(nextScan) { nextWake = nextScan @@ -570,19 +877,20 @@ func (s FriendSyncer) Run(ctx context.Context) { } } -// runSync updates mutation backoff and returns any delay needed before reading again. -func (s FriendSyncer) runSync(ctx context.Context, state *friendSyncRunState, expire bool) time.Duration { - if state == nil { - state = &friendSyncRunState{} - } - opts := state.options(time.Now(), expire) - s.debug(ctx, "friend sync tick", "expire", expire, "auto_follow", opts.autoFollow, "auto_unfollow", opts.autoUnfollow) +// runSync updates backoff and returns any delay needed before reading again, +// and whether another pass should follow as soon as spacing allows. +func (s *FriendSyncer) runSync(ctx context.Context, cleanup bool) (time.Duration, bool) { + now := time.Now() + opts := s.state.options(now, cleanup) + s.debug(ctx, "friend sync tick", "cleanup", cleanup, "auto_follow", opts.autoFollow, "auto_unfollow", opts.autoUnfollow) result, err := s.syncWithOptions(ctx, opts) - state.record(time.Now(), result) + s.state.record(time.Now(), result) if err != nil && s.Log != nil && !errors.Is(err, context.Canceled) { s.Log.Error("sync friends", "err", err) } - return result.readRetryAfter + // Requests were left for lack of room that the list now has. + again := result.requests.Deferred > 0 && result.room > 0 + return result.readRetryAfter, again } func isGuestXUID(xuid string) bool { diff --git a/friend_sync_rate_test.go b/friend_sync_rate_test.go index 6c0d374..9e77a0a 100644 --- a/friend_sync_rate_test.go +++ b/friend_sync_rate_test.go @@ -76,7 +76,7 @@ func TestFriendSyncReadRetryRunsWithoutAnotherTrigger(t *testing.T) { }) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - go (FriendSyncer{Client: client, Config: FriendSyncConfig{ + go (&FriendSyncer{Client: client, Config: FriendSyncConfig{ AutoFollow: true, UpdateInterval: time.Hour}}).Run(ctx) synctest.Wait() assertFriendRateReads(t, reads, 0, pendingRequestsURL) @@ -100,7 +100,7 @@ func TestFriendSyncTriggerSpacingStartsAfterPassCompletes(t *testing.T) { trigger := make(chan struct{}, 16) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - go (FriendSyncer{Client: client, Trigger: trigger, Config: FriendSyncConfig{ + go (&FriendSyncer{Client: client, Trigger: trigger, Config: FriendSyncConfig{ AutoFollow: true, UpdateInterval: time.Hour}}).Run(ctx) synctest.Wait() assertFriendRateReads(t, reads, 0, friendRateReadURLs...) @@ -131,10 +131,10 @@ func TestFriendSyncTriggerSpacingStartsAfterPassCompletes(t *testing.T) { }) } -func TestFriendSyncCoalescesPollExpiryAndTrigger(t *testing.T) { +func TestFriendSyncCoalescesPollCleanupAndTrigger(t *testing.T) { synctest.Test(t, func(t *testing.T) { passes := make(chan timedFriendRead, 16) - expiries := make(chan timedFriendRead, 16) + cleanups := make(chan timedFriendRead, 16) start := time.Now() client := &syncFriendClient{people: []Person{{XUID: "123", IsFollowedByCaller: true, IsFollowingCaller: true}}, accept: func(context.Context) ([]Person, error) { @@ -144,12 +144,12 @@ func TestFriendSyncCoalescesPollExpiryAndTrigger(t *testing.T) { trigger := make(chan struct{}, 16) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - go (FriendSyncer{Client: client, Trigger: trigger, History: &friendRateHistory{start, expiries}, - Config: FriendSyncConfig{AutoFollow: true, ExpiryEnabled: true, - UpdateInterval: 20 * time.Second, ExpiryCheck: 20 * time.Second}}).Run(ctx) + go (&FriendSyncer{Client: client, Trigger: trigger, History: &friendRateHistory{start, cleanups}, + Config: FriendSyncConfig{AutoFollow: true, UpdateInterval: 20 * time.Second, + Cleanup: FriendCleanupConfig{InactiveDays: 15, Interval: 20 * time.Second}}}).Run(ctx) synctest.Wait() assertFriendRateReads(t, passes, 0, "pass") - assertFriendRateReads(t, expiries, 0, "expiry") + assertFriendRateReads(t, cleanups, 0, "cleanup") time.Sleep(19 * time.Second) for range cap(trigger) { trigger <- struct{}{} @@ -160,7 +160,7 @@ func TestFriendSyncCoalescesPollExpiryAndTrigger(t *testing.T) { time.Sleep(at - time.Since(start)) synctest.Wait() assertFriendRateReads(t, passes, at, "pass") - assertFriendRateReads(t, expiries, at, "expiry") + assertFriendRateReads(t, cleanups, at, "cleanup") } }) } @@ -205,17 +205,24 @@ func assertFriendRateReads(t *testing.T, reads <-chan timedFriendRead, at time.D } } -// friendRateHistory records expiry scans while keeping the test friend active. +// friendRateHistory records cleanup scans while keeping the test friend active. type friendRateHistory struct { start time.Time reads chan<- timedFriendRead } -// LastSeen records that expiry ran and returns a recent visit so no removal is needed. -func (h *friendRateHistory) LastSeen(context.Context, string) (time.Time, bool, error) { - h.reads <- timedFriendRead{"expiry", time.Since(h.start)} - return time.Now(), true, nil +// LastSeen records that cleanup ran and returns a recent visit so no removal is needed. +func (h *friendRateHistory) LastSeen(context.Context, string) (map[string]time.Time, error) { + h.reads <- timedFriendRead{"cleanup", time.Since(h.start)} + return map[string]time.Time{"123": time.Now()}, nil } -// Clear satisfies HistoryStore; the active test friend never needs removal. -func (h *friendRateHistory) Clear(context.Context, string) error { return nil } +func (h *friendRateHistory) Track(context.Context, string, time.Time, ...string) error { return nil } +func (h *friendRateHistory) Seen(context.Context, string, time.Time) error { return nil } +func (h *friendRateHistory) Forget(context.Context, string, ...string) error { return nil } +func (h *friendRateHistory) MarkRemoving(context.Context, string, time.Time, ...string) error { + return nil +} +func (h *friendRateHistory) Removing(context.Context, string) (map[string]time.Time, error) { + return nil, nil +} diff --git a/friend_sync_test.go b/friend_sync_test.go index a853c38..62a88c6 100644 --- a/friend_sync_test.go +++ b/friend_sync_test.go @@ -3,11 +3,20 @@ package broadcaster import ( "bytes" "context" + "encoding/json" "errors" + "fmt" "log/slog" + "maps" + "net/http" + "path/filepath" + "regexp" + "slices" "strconv" "strings" + "sync" "testing" + "testing/synctest" "time" xblsocial "github.com/df-mc/go-xsapi/v2/social" @@ -17,6 +26,7 @@ func TestFriendSyncerAcceptsPendingIncomingRequests(t *testing.T) { var accepted bool var invited []string client := syncFriendClient{ + people: []Person{{XUID: "9", Gamertag: "Pending", IsFollowingCaller: true, IsFollowedByCaller: true}}, accept: func(context.Context) ([]Person, error) { accepted = true return []Person{{XUID: "9", Gamertag: "Pending"}}, nil @@ -51,13 +61,15 @@ func TestFriendSyncerBoundsXboxOperations(t *testing.T) { AutoFollow: true, AutoUnfollow: true, InitialInvite: true, + Cleanup: FriendCleanupConfig{MaxFriends: 1}, }, + History: newMemoryHistory(), Inviter: deadlineInviter{record: client.record}, } if err := syncer.Sync(context.Background()); err != nil { t.Fatal(err) } - for _, operation := range []string{"accept", "friends", "follow", "unfollow", "force_unfollow", "invite"} { + for _, operation := range []string{"accept", "friends", "follow", "unfollow", "remove_friend", "remove_follower", "invite"} { if !client.called[operation] { t.Fatalf("%s was not called", operation) } @@ -132,14 +144,11 @@ func TestFriendSyncerDebugLogsFriendSyncProgress(t *testing.T) { } } -func TestFriendSyncerDebugLogsExpiryCandidates(t *testing.T) { +func TestFriendSyncerDebugLogsCleanupScan(t *testing.T) { var log bytes.Buffer - history := &syncHistoryStore{ - lastSeen: map[string]time.Time{ - "stale": time.Now().Add(-16 * 24 * time.Hour), - "recent": time.Now(), - }, - } + history := newMemoryHistory() + history.set("me", "stale", time.Now().Add(-16*24*time.Hour)) + history.set("me", "recent", time.Now()) client := &syncFriendClient{ people: []Person{ {XUID: "stale", Gamertag: "Stale", IsFollowingCaller: true, IsFollowedByCaller: true}, @@ -149,10 +158,10 @@ func TestFriendSyncerDebugLogsExpiryCandidates(t *testing.T) { syncer := FriendSyncer{ Client: client, History: history, + Account: "me", Config: FriendSyncConfig{ - AutoUnfollow: true, - ExpiryEnabled: true, - ExpiryDays: 15, + AutoUnfollow: true, + Cleanup: FriendCleanupConfig{InactiveDays: 15}, }, Log: slog.New(slog.NewTextHandler(&log, &slog.HandlerOptions{Level: slog.LevelDebug})), } @@ -161,22 +170,27 @@ func TestFriendSyncerDebugLogsExpiryCandidates(t *testing.T) { } output := log.String() for _, want := range []string{ - `msg="friend sync expiry scan"`, - `mutual_expiry_candidates=1`, - `expired_friends=1`, + `msg="friend cleanup scan"`, + `inactive=1`, + `over_capacity=0`, + `msg="removed friend" xuid=stale gamertag=Stale reason=inactive`, } { if !strings.Contains(output, want) { t.Fatalf("debug log missing %q in:\n%s", want, output) } } - if client.removeCalls != 1 { - t.Fatalf("remove calls = %d, want 1", client.removeCalls) + if got := strings.Join(client.removedFriends, ","); got != "stale" { + t.Fatalf("removed friends = %q, want stale", got) } } func TestFriendSyncerDebugLogsPendingFriendAccepts(t *testing.T) { var log bytes.Buffer client := syncFriendClient{ + people: []Person{ + {XUID: "9", IsFollowingCaller: true, IsFollowedByCaller: true}, + {XUID: "10", IsFollowingCaller: true, IsFollowedByCaller: true}, + }, accept: func(context.Context) ([]Person, error) { return []Person{ {XUID: "9", Gamertag: "PendingOne"}, @@ -252,7 +266,7 @@ func TestFriendSyncerStopsAutoFollowPassWhenFriendListIsFull(t *testing.T) { }, Log: slog.New(slog.NewTextHandler(&log, nil)), } - result, err := syncer.syncWithOptions(context.Background(), friendSyncOptions{expire: true, autoFollow: true, autoUnfollow: true}) + result, err := syncer.syncWithOptions(context.Background(), friendSyncOptions{cleanup: true, autoFollow: true, autoUnfollow: true}) if err != nil { t.Fatalf("syncWithOptions() error = %v, want nil", err) } @@ -291,8 +305,8 @@ func TestFriendSyncerContinuesPastSingleFollowFailure(t *testing.T) { if client.followCalls != 2 { t.Fatalf("follow calls = %d, want 2 (continue past failure)", client.followCalls) } - if client.removeCalls != 1 { - t.Fatalf("remove calls = %d, want 1 (removals not aborted)", client.removeCalls) + if client.unfollowCalls != 1 { + t.Fatalf("unfollow calls = %d, want 1 (removals not aborted)", client.unfollowCalls) } } @@ -319,8 +333,8 @@ func TestFriendSyncerContinuesRemovalsWhenAddsAreRateLimited(t *testing.T) { if client.followCalls != 1 { t.Fatalf("follow calls = %d, want 1 (stop adds after rate limit)", client.followCalls) } - if client.removeCalls != 1 { - t.Fatalf("remove calls = %d, want 1 (removals use a separate limit)", client.removeCalls) + if client.unfollowCalls != 1 { + t.Fatalf("unfollow calls = %d, want 1 (removals use a separate limit)", client.unfollowCalls) } if result.followRetryAfter != 5*time.Second { t.Fatalf("follow retry-after = %s, want 5s", result.followRetryAfter) @@ -330,7 +344,7 @@ func TestFriendSyncerContinuesRemovalsWhenAddsAreRateLimited(t *testing.T) { } } -func TestFriendSyncerForceUnfollowsRestrictedAccounts(t *testing.T) { +func TestFriendSyncerDropsRestrictedFollowers(t *testing.T) { restrictedErr := &xblsocial.ResponseError{Code: 1049} var notified []string client := &syncFriendClient{ @@ -356,8 +370,8 @@ func TestFriendSyncerForceUnfollowsRestrictedAccounts(t *testing.T) { if err := syncer.Sync(context.Background()); err != nil { t.Fatalf("Sync() error = %v, want nil", err) } - if got := strings.Join(client.forceUnfollowed, ","); got != "1" { - t.Fatalf("force unfollowed = %q, want 1", got) + if got := strings.Join(client.removedFollowers, ","); got != "1" { + t.Fatalf("removed followers = %q, want 1", got) } if client.followCalls != 2 { t.Fatalf("follow calls = %d, want 2 (continue past restricted account)", client.followCalls) @@ -385,30 +399,31 @@ func TestFriendSyncerSkipsGuestXUIDsUnconditionally(t *testing.T) { } func TestFriendSyncerPrunesHistoryForExFriends(t *testing.T) { - history := &syncHistoryStore{ - lastSeen: map[string]time.Time{ - "1": time.Now(), - "gone": time.Now(), - }, - } + history := newMemoryHistory() + history.set("me", "1", time.Now()) + history.set("me", "gone", time.Now()) + history.set("other", "gone", time.Now()) client := &syncFriendClient{ people: []Person{{XUID: "1", IsFollowingCaller: true, IsFollowedByCaller: true}}, } syncer := FriendSyncer{ - Client: client, - Config: FriendSyncConfig{ExpiryEnabled: true, ExpiryDays: 15}, - History: history, - PruneHistory: true, + Client: client, + Config: FriendSyncConfig{Cleanup: FriendCleanupConfig{InactiveDays: 15}}, + History: history, + Account: "me", } if err := syncer.Sync(context.Background()); err != nil { t.Fatal(err) } - if _, ok := history.lastSeen["gone"]; ok { + if _, ok := history.get("me", "gone"); ok { t.Fatal("expected history entry for ex-friend to be pruned") } - if _, ok := history.lastSeen["1"]; !ok { + if _, ok := history.get("me", "1"); !ok { t.Fatal("expected history entry for current friend to remain") } + if _, ok := history.get("other", "gone"); !ok { + t.Fatal("pruning one account must not touch another account's history") + } } func TestFriendSyncRunStateTracksSeparateFollowAndUnfollowBackoff(t *testing.T) { @@ -430,14 +445,15 @@ func TestFriendSyncRunStateTracksSeparateFollowAndUnfollowBackoff(t *testing.T) } } -func TestFriendSyncRunStateSuppressesAutoFollowWhenFriendListIsFull(t *testing.T) { +// A pass that finds the list full stops the next pass from accepting. +func TestFriendSyncRunStateStopsAcceptsWhenFriendListIsFull(t *testing.T) { now := time.Unix(100, 0) - state := friendSyncRunState{} - state.record(now, friendSyncResult{friendListFull: true}) + state := friendSyncRunState{room: 5} + state.record(now, friendSyncResult{scanned: true, friendListFull: true}) opts := state.options(now.Add(time.Minute), true) - if opts.autoFollow { - t.Fatal("expected auto-follow suppressed after friend-list-full") + if opts.acceptLimit != 0 { + t.Fatalf("accept limit = %d, want 0 after friend-list-full", opts.acceptLimit) } if !opts.autoUnfollow { t.Fatal("expected auto-unfollow to keep running after friend-list-full") @@ -445,13 +461,64 @@ func TestFriendSyncRunStateSuppressesAutoFollowWhenFriendListIsFull(t *testing.T } type syncFriendClient struct { - people []Person - accept func(context.Context) ([]Person, error) - follow func(context.Context, string) error - unfollow func(context.Context, string) error - followCalls int - removeCalls int - forceUnfollowed []string + people []Person + accept func(context.Context) ([]Person, error) + follow func(context.Context, string) error + unfollow func(context.Context, string) error + removeFriend func(context.Context, string) error + removeFollower func(context.Context, string) error + followCalls int + unfollowCalls int + removedFriends []string + removedFollowers []string +} + +func (c *syncFriendClient) Friends(context.Context) ([]Person, error) { + return c.people, nil +} + +func (c *syncFriendClient) Follow(ctx context.Context, xuid string) error { + c.followCalls++ + if c.follow != nil { + return c.follow(ctx, xuid) + } + return nil +} + +func (c *syncFriendClient) Unfollow(ctx context.Context, xuid string) error { + c.unfollowCalls++ + if c.unfollow != nil { + return c.unfollow(ctx, xuid) + } + return nil +} + +func (c *syncFriendClient) RemoveFriend(ctx context.Context, xuid string) error { + if c.removeFriend != nil { + if err := c.removeFriend(ctx, xuid); err != nil { + return err + } + } + c.removedFriends = append(c.removedFriends, xuid) + return nil +} + +func (c *syncFriendClient) RemoveFollower(ctx context.Context, xuid string) error { + if c.removeFollower != nil { + if err := c.removeFollower(ctx, xuid); err != nil { + return err + } + } + c.removedFollowers = append(c.removedFollowers, xuid) + return nil +} + +func (c *syncFriendClient) AcceptPendingFriendRequests(ctx context.Context, _ int, _ func(string) bool) (FriendRequestResult, error) { + if c.accept == nil { + return FriendRequestResult{}, nil + } + accepted, err := c.accept(ctx) + return FriendRequestResult{Accepted: accepted}, err } type deadlineFriendClient struct { @@ -475,6 +542,8 @@ func (c *deadlineFriendClient) Friends(ctx context.Context) ([]Person, error) { {XUID: "follow", Gamertag: "Follow", IsFollowingCaller: true}, {XUID: "restricted", Gamertag: "Restricted", IsFollowingCaller: true}, {XUID: "unfollow", Gamertag: "Unfollow", IsFollowedByCaller: true}, + {XUID: "pending", Gamertag: "Pending", IsFollowingCaller: true, IsFollowedByCaller: true}, + {XUID: "old", Gamertag: "Old", IsFollowingCaller: true, IsFollowedByCaller: true}, }, nil } @@ -491,14 +560,19 @@ func (c *deadlineFriendClient) Unfollow(ctx context.Context, _ string) error { return nil } -func (c *deadlineFriendClient) ForceUnfollow(ctx context.Context, _ string) error { - c.record("force_unfollow", ctx) +func (c *deadlineFriendClient) RemoveFriend(ctx context.Context, _ string) error { + c.record("remove_friend", ctx) + return nil +} + +func (c *deadlineFriendClient) RemoveFollower(ctx context.Context, _ string) error { + c.record("remove_follower", ctx) return nil } -func (c *deadlineFriendClient) AcceptPendingFriendRequests(ctx context.Context) ([]Person, error) { +func (c *deadlineFriendClient) AcceptPendingFriendRequests(ctx context.Context, _ int, _ func(string) bool) (FriendRequestResult, error) { c.record("accept", ctx) - return []Person{{XUID: "pending", Gamertag: "Pending"}}, nil + return FriendRequestResult{Accepted: []Person{{XUID: "pending", Gamertag: "Pending"}}}, nil } type deadlineInviter struct { @@ -510,64 +584,92 @@ func (i deadlineInviter) Invite(ctx context.Context, _, _ string) error { return nil } -func (c *syncFriendClient) ForceUnfollow(_ context.Context, xuid string) error { - c.forceUnfollowed = append(c.forceUnfollowed, xuid) - return nil -} - type notifierFunc func(context.Context, string) error func (f notifierFunc) Notify(ctx context.Context, message string) error { return f(ctx, message) } -type syncHistoryStore struct { - lastSeen map[string]time.Time +// memoryHistory is an in-memory HistoryStore keyed by account. +type memoryHistory struct { + mu sync.Mutex + accounts map[string]map[string]time.Time + removing map[string]map[string]time.Time } -func (s *syncHistoryStore) LastSeen(_ context.Context, xuid string) (time.Time, bool, error) { - when, ok := s.lastSeen[xuid] - return when, ok, nil +func newMemoryHistory() *memoryHistory { + return &memoryHistory{accounts: map[string]map[string]time.Time{}, removing: map[string]map[string]time.Time{}} } -func (s *syncHistoryStore) Clear(_ context.Context, xuid string) error { - delete(s.lastSeen, xuid) +func (h *memoryHistory) MarkRemoving(_ context.Context, account string, when time.Time, xuids ...string) error { + h.mu.Lock() + defer h.mu.Unlock() + if h.removing[account] == nil { + h.removing[account] = map[string]time.Time{} + } + for _, xuid := range xuids { + delete(h.accounts[account], xuid) + h.removing[account][xuid] = when + } return nil } -func (s *syncHistoryStore) XUIDs(context.Context) ([]string, error) { - xuids := make([]string, 0, len(s.lastSeen)) - for xuid := range s.lastSeen { - xuids = append(xuids, xuid) +func (h *memoryHistory) Removing(_ context.Context, account string) (map[string]time.Time, error) { + h.mu.Lock() + defer h.mu.Unlock() + return maps.Clone(h.removing[account]), nil +} + +func (h *memoryHistory) set(account, xuid string, when time.Time) { + h.mu.Lock() + defer h.mu.Unlock() + if h.accounts[account] == nil { + h.accounts[account] = map[string]time.Time{} } - return xuids, nil + h.accounts[account][xuid] = when } -func (c *syncFriendClient) Friends(context.Context) ([]Person, error) { - return c.people, nil +func (h *memoryHistory) get(account, xuid string) (time.Time, bool) { + h.mu.Lock() + defer h.mu.Unlock() + when, ok := h.accounts[account][xuid] + return when, ok } -func (c *syncFriendClient) Follow(ctx context.Context, xuid string) error { - c.followCalls++ - if c.follow != nil { - return c.follow(ctx, xuid) +func (h *memoryHistory) LastSeen(_ context.Context, account string) (map[string]time.Time, error) { + h.mu.Lock() + defer h.mu.Unlock() + return maps.Clone(h.accounts[account]), nil +} + +func (h *memoryHistory) Track(_ context.Context, account string, when time.Time, xuids ...string) error { + for _, xuid := range xuids { + if _, ok := h.get(account, xuid); !ok { + h.set(account, xuid, when) + } } return nil } -func (c *syncFriendClient) Unfollow(ctx context.Context, xuid string) error { - c.removeCalls++ - if c.unfollow != nil { - return c.unfollow(ctx, xuid) +func (h *memoryHistory) Seen(_ context.Context, xuid string, when time.Time) error { + h.mu.Lock() + defer h.mu.Unlock() + for _, entries := range h.accounts { + if _, ok := entries[xuid]; ok { + entries[xuid] = when + } } return nil } -func (c *syncFriendClient) AcceptPendingFriendRequests(ctx context.Context) ([]Person, error) { - if c.accept != nil { - return c.accept(ctx) +func (h *memoryHistory) Forget(_ context.Context, account string, xuids ...string) error { + h.mu.Lock() + defer h.mu.Unlock() + for _, xuid := range xuids { + delete(h.accounts[account], xuid) + delete(h.removing[account], xuid) } - return nil, nil + return nil } type fakeSyncInviter struct { @@ -581,3 +683,534 @@ func (f fakeSyncInviter) Invite(_ context.Context, xuid, _ string) error { } return f.err } + +// fakeXbox models the Xbox people service behind FriendClient: who follows the +// account, whom it follows, and incoming friend requests. Ending a friendship +// leaves the other person's follow in place, as Xbox does. +type fakeXbox struct { + mu sync.Mutex + followers map[string]bool // they follow the account + following map[string]bool // the account follows them + pending map[string]bool // incoming friend requests + // limit makes bulk accepts fail with code 1028 once following reaches it. + limit int + // bulkAdd overrides how a bulk accept is answered. + bulkAdd func(xuids []string) *http.Response + // removeFollower optionally fails a follower removal. + removeFollower func(xuid string) *http.Response + bulkPosts int + follows []string +} + +func newFakeXbox() *fakeXbox { + return &fakeXbox{followers: map[string]bool{}, following: map[string]bool{}, pending: map[string]bool{}} +} + +// befriend makes each XUID a mutual friend. +func (f *fakeXbox) befriend(xuids ...string) { + for _, xuid := range xuids { + f.followers[xuid], f.following[xuid] = true, true + } +} + +var fakeXboxXUID = regexp.MustCompile(`xuid\((\d+)\)`) + +func (f *fakeXbox) peopleJSON(set map[string]bool) string { + people := []map[string]any{} + for _, xuid := range slices.Sorted(maps.Keys(set)) { + people = append(people, map[string]any{ + "xuid": xuid, "gamertag": "GT" + xuid, + "isFollowingCaller": f.followers[xuid], + "isFollowedByCaller": f.following[xuid], + }) + } + data, _ := json.Marshal(map[string]any{"people": people}) + return string(data) +} + +func (f *fakeXbox) client() FriendClient { + return FriendClient{Social: newTestSocialClient(&http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + resp := f.serve(req) + resp.Request = req + return resp, nil + })})} +} + +func (f *fakeXbox) serve(req *http.Request) *http.Response { + f.mu.Lock() + defer f.mu.Unlock() + path := req.URL.Path + xuid := "" + if m := fakeXboxXUID.FindStringSubmatch(path); m != nil { + xuid = m[1] + } + switch { + case req.Method == http.MethodGet && strings.HasSuffix(path, "/people/followers"): + return response(http.StatusOK, f.peopleJSON(f.followers)) + case req.Method == http.MethodGet && strings.HasSuffix(path, "/people/social"): + return response(http.StatusOK, f.peopleJSON(f.following)) + case req.Method == http.MethodGet && strings.Contains(path, "friendRequests(received)"): + return response(http.StatusOK, f.peopleJSON(f.pending)) + case req.Method == http.MethodPost && strings.HasPrefix(path, "/bulk/"): + f.bulkPosts++ + var body struct { + XUIDs []string `json:"xuids"` + } + _ = json.NewDecoder(req.Body).Decode(&body) + if f.bulkAdd != nil { + return f.bulkAdd(body.XUIDs) + } + if f.limit > 0 && len(f.following)+len(body.XUIDs) > f.limit { + return response(http.StatusBadRequest, `{"code":1028,"description":"The attempted People request was rejected because it would exceed the People list limit."}`) + } + for _, x := range body.XUIDs { + delete(f.pending, x) + f.befriend(x) + } + return updated(body.XUIDs) + case req.Method == http.MethodPut: + f.following[xuid] = true + f.follows = append(f.follows, xuid) + return response(http.StatusNoContent, "") + case req.Method == http.MethodDelete && strings.Contains(path, "/people/follower/"): + if f.removeFollower != nil { + if resp := f.removeFollower(xuid); resp != nil { + return resp + } + } + delete(f.followers, xuid) + return response(http.StatusNoContent, "") + case req.Method == http.MethodDelete && strings.Contains(path, "/friends/v2/"): + // Ends a friendship or a pending request, never a one-way follow. + if mutual := f.following[xuid] && f.followers[xuid]; !f.pending[xuid] && !mutual { + return response(http.StatusNotFound, "") + } + delete(f.following, xuid) + delete(f.pending, xuid) + return response(http.StatusOK, "") + case req.Method == http.MethodDelete: + delete(f.following, xuid) + return response(http.StatusNoContent, "") + } + return response(http.StatusNotFound, "") +} + +// Removed friends who still follow the account must not be followed back into the freed slot. +func TestFriendSyncDoesNotFollowBackRemovedFriends(t *testing.T) { + x := newFakeXbox() + x.befriend("42") + history := newMemoryHistory() + history.set("100", "42", time.Now().Add(-16*24*time.Hour)) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + for _, cleanup := range []bool{true, false, true} { + s.runSync(context.Background(), cleanup) + } + if len(x.follows) != 0 || x.followers["42"] || x.following["42"] { + t.Fatalf("follows=%v follower=%v following=%v, want the friendship gone both ways", x.follows, x.followers["42"], x.following["42"]) + } +} + +// A throttled follower removal is finished later and never turns into a follow-back. +func TestFriendSyncFinishesThrottledRemoval(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + x := newFakeXbox() + x.befriend("42") + throttled := false + x.removeFollower = func(string) *http.Response { + if throttled { + return nil + } + throttled = true + resp := response(http.StatusTooManyRequests, "") + resp.Header.Set("Retry-After", "30") + return resp + } + history := newMemoryHistory() + history.set("100", "42", time.Now().Add(-16*24*time.Hour)) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + s.runSync(context.Background(), true) + s.runSync(context.Background(), false) + if !x.followers["42"] || len(x.follows) != 0 { + t.Fatalf("during backoff follower=%v follows=%v, want follower kept and no follow-back", x.followers["42"], x.follows) + } + time.Sleep(30 * time.Second) + s.runSync(context.Background(), false) + if x.followers["42"] || len(x.follows) != 0 { + t.Fatalf("after backoff follower=%v follows=%v, want follower removed", x.followers["42"], x.follows) + } + }) +} + +// maxFriends leaves room for waiting requests by removing the least recently seen friends. +func TestFriendSyncKeepsFriendsWithinMaxFriends(t *testing.T) { + x := newFakeXbox() + x.befriend("1", "2", "3", "4") + history := newMemoryHistory() + for i, xuid := range []string{"3", "1", "4", "2"} { + history.set("100", xuid, time.Now().Add(-time.Duration(4-i)*time.Hour)) + } + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoUnfollow: true, Cleanup: FriendCleanupConfig{MaxFriends: 2}}} + s.runSync(context.Background(), false) + if got := strings.Join(slices.Sorted(maps.Keys(x.following)), ","); got != "2,4" { + t.Fatalf("following = %s, want 2,4 (the most recently seen)", got) + } + if _, ok := history.get("100", "3"); ok { + t.Fatal("removed friends should leave history") + } +} + +// A request Xbox refuses is retried later, not on every pass, and does not block the rest. +func TestFriendSyncRetriesRefusedRequestsLater(t *testing.T) { + x := newFakeXbox() + x.pending["1"], x.pending["2"], x.pending["3"] = true, true, true + x.bulkAdd = func(xuids []string) *http.Response { + if slices.Contains(xuids, "3") { + return response(http.StatusBadRequest, "") + } + for _, xuid := range xuids { + delete(x.pending, xuid) + x.befriend(xuid) + } + return updated(xuids) + } + s := &FriendSyncer{Client: x.client(), Config: FriendSyncConfig{AutoFollow: true}} + s.runSync(context.Background(), false) // learns the room + s.runSync(context.Background(), false) + posts := x.bulkPosts + s.runSync(context.Background(), false) + if !x.following["1"] || !x.following["2"] || x.bulkPosts != posts { + t.Fatalf("following=%v posts %d -> %d, want 1 and 2 accepted and 3 not retried yet", x.following, posts, x.bulkPosts) + } +} + +// Xbox can report a request as accepted while it stays pending: that is not a new friend. +func TestFriendSyncAnnouncesAcceptedFriendsOnce(t *testing.T) { + var log bytes.Buffer + var invites []string + x := newFakeXbox() + x.pending["7"] = true + x.bulkAdd = func([]string) *http.Response { return updated([]string{"7"}) } + s := &FriendSyncer{Client: x.client(), Config: FriendSyncConfig{AutoFollow: true, InitialInvite: true}, + Inviter: fakeSyncInviter{invite: func(xuid string) { invites = append(invites, xuid) }}, + Log: slog.New(slog.NewTextHandler(&log, nil))} + for range 3 { + s.runSync(context.Background(), false) + } + if len(invites) != 0 || strings.Contains(log.String(), "added friend") || x.bulkPosts != 1 { + t.Fatalf("invites=%v posts=%d log=%s, want nothing announced for a request still pending", invites, x.bulkPosts, log.String()) + } + + x.bulkAdd = nil + x.pending["8"] = true + for range 3 { + s.runSync(context.Background(), false) + } + if fmt.Sprint(invites) != "[8]" || strings.Count(log.String(), `msg="added friend"`) != 1 { + t.Fatalf("invites=%v log=%s, want one announcement and invite for 8", invites, log.String()) + } +} + +// Cleanup and auto-unfollow must never cut the links between the bot's own accounts. +func TestFriendSyncNeverRemovesOwnAccounts(t *testing.T) { + x := newFakeXbox() + x.befriend("200", "9") + x.following["201"] = true + history := newMemoryHistory() + history.set("100", "200", time.Now().Add(-30*24*time.Hour)) + history.set("100", "9", time.Now()) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", OwnAccounts: []string{"100", "200", "201"}, + Config: FriendSyncConfig{AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15, MaxFriends: 1}}} + s.runSync(context.Background(), true) + if !x.following["200"] || !x.following["201"] || x.following["9"] { + t.Fatalf("following = %v, want own accounts kept and 9 removed for capacity", x.following) + } +} + +// Restricted-follower removals share the removal rate limit and stop when it is hit. +func TestFriendSyncRestrictedRemovalHonoursRetryAfter(t *testing.T) { + var attempts int + client := &syncFriendClient{ + people: []Person{{XUID: "1", IsFollowingCaller: true}, {XUID: "2", IsFollowingCaller: true}}, + follow: func(context.Context, string) error { return &xblsocial.ResponseError{Code: 1049} }, + removeFollower: func(context.Context, string) error { + attempts++ + return &xblsocial.ResponseError{StatusCode: http.StatusTooManyRequests, RetryAfter: time.Minute} + }, + } + s := &FriendSyncer{Client: client, Config: FriendSyncConfig{AutoFollow: true}} + result, err := s.syncWithOptions(context.Background(), friendSyncOptions{autoFollow: true, autoUnfollow: true}) + if err != nil { + t.Fatal(err) + } + if attempts != 1 || result.unfollowRetryAfter != time.Minute { + t.Fatalf("attempts=%d retry=%s, want 1 attempt and a 1m backoff", attempts, result.unfollowRetryAfter) + } +} + +// A removal whose follower side failed must survive a restart, or the next process follows the player back. +func TestFriendSyncFinishesRemovalAfterRestart(t *testing.T) { + x := newFakeXbox() + x.befriend("42") + x.removeFollower = func(string) *http.Response { + resp := response(http.StatusTooManyRequests, "") + resp.Header.Set("Retry-After", "30") + return resp + } + path := filepath.Join(t.TempDir(), "player_history.json") + history := NewFileHistoryStore(path) + if err := history.Track(context.Background(), "100", time.Now().Add(-16*24*time.Hour), "42"); err != nil { + t.Fatal(err) + } + conf := FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}} + (&FriendSyncer{Client: x.client(), History: history, Account: "100", Config: conf}).runSync(context.Background(), true) + if x.following["42"] || !x.followers["42"] { + t.Fatalf("following=%v followers=%v, want friendship ended with the follower left", x.following, x.followers) + } + + x.removeFollower = nil + restarted := &FriendSyncer{Client: x.client(), History: NewFileHistoryStore(path), Account: "100", Config: conf} + restarted.runSync(context.Background(), false) + if len(x.follows) != 0 || x.followers["42"] { + t.Fatalf("after restart follows=%v follower=%v, want the removal finished and no follow-back", x.follows, x.followers["42"]) + } + if removing, _ := NewFileHistoryStore(path).Removing(context.Background(), "100"); len(removing) != 0 { + t.Fatalf("removal marks = %v, want cleared once the follower is gone", removing) + } +} + +// A removed player who sends a new request is a new friend with a fresh clock, not an inactive one. +func TestFriendSyncAcceptedRequestClearsRemoval(t *testing.T) { + x := newFakeXbox() + x.followers["42"] = true + x.pending["42"] = true + history := newMemoryHistory() + _ = history.MarkRemoving(context.Background(), "100", time.Now().Add(-20*24*time.Hour), "42") + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + s.runSync(context.Background(), true) + s.runSync(context.Background(), true) + if !x.following["42"] || !x.followers["42"] { + t.Fatalf("following=%v followers=%v, want 42 kept as a friend", x.following, x.followers) + } + if removing, _ := history.Removing(context.Background(), "100"); len(removing) != 0 { + t.Fatalf("removal marks = %v, want cleared by the new friendship", removing) + } +} + +// A one-way follow over maxFriends is unfollowed; ending a friendship would leave it holding the slot. +func TestFriendSyncCleanupUnfollowsOneWayFollows(t *testing.T) { + x := newFakeXbox() + x.befriend("1", "2") + x.following["3"] = true + history := newMemoryHistory() + history.set("100", "3", time.Now().Add(-2*time.Hour)) + history.set("100", "1", time.Now().Add(-time.Hour)) + history.set("100", "2", time.Now()) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, Cleanup: FriendCleanupConfig{MaxFriends: 2}}} + s.runSync(context.Background(), false) + if got := strings.Join(slices.Sorted(maps.Keys(x.following)), ","); got != "1,2" { + t.Fatalf("following = %s, want 1,2 with the one-way follow of 3 removed", got) + } + if _, ok := history.get("100", "3"); ok { + t.Fatal("history for the unfollowed person should be forgotten") + } +} + +// Turning cleanup off must not let a pending removal be followed back. +func TestFriendSyncFinishesPendingRemovalWithCleanupOff(t *testing.T) { + x := newFakeXbox() + x.followers["42"] = true + history := newMemoryHistory() + _ = history.MarkRemoving(context.Background(), "100", time.Now(), "42") + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true}} + s.runSync(context.Background(), false) + if len(x.follows) != 0 || x.followers["42"] { + t.Fatalf("follows=%v follower=%v, want the removal finished and no follow-back", x.follows, x.followers["42"]) + } + if removing, _ := history.Removing(context.Background(), "100"); len(removing) != 0 { + t.Fatalf("removal marks = %v, want cleared", removing) + } +} + +// befriendMany makes n numbered mutual friends, starting at XUID 1000. +func (f *fakeXbox) befriendMany(n int) { + for i := range n { + f.befriend(strconv.Itoa(1000 + i)) + } +} + +// At the Xbox limit, maxFriends makes room for a waiting request instead of failing every pass. +func TestFriendSyncMakesRoomWhenAcceptFindsListFull(t *testing.T) { + x := newFakeXbox() + x.limit = XboxFriendLimit + x.befriendMany(XboxFriendLimit) + x.pending["9"] = true + history := newMemoryHistory() + history.set("100", "1000", time.Now().Add(-3*24*time.Hour)) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{MaxFriends: XboxFriendLimit}}} + + if _, again := s.runSync(context.Background(), false); !again { + t.Fatal("expected another pass once room was made") + } + s.runSync(context.Background(), false) + if !x.following["9"] || x.following["1000"] || len(x.following) != XboxFriendLimit { + t.Fatalf("9 added=%v 1000 kept=%v size=%d, want 9 in place of 1000, the least recently seen", x.following["9"], x.following["1000"], len(x.following)) + } +} + +// At the Xbox limit without maxFriends, no pass posts an accept Xbox would refuse. +func TestFriendSyncSkipsAcceptsAtLimit(t *testing.T) { + x := newFakeXbox() + x.limit = XboxFriendLimit + x.befriendMany(XboxFriendLimit) + for i := range 3 * addFriendsBatchSize { + x.pending[strconv.Itoa(i)] = true + } + s := &FriendSyncer{Client: x.client(), Config: FriendSyncConfig{AutoFollow: true}} + for range 3 { + if _, again := s.runSync(context.Background(), false); again { + t.Fatal("a full list must not ask for another pass") + } + } + if x.bulkPosts != 0 { + t.Fatalf("bulk posts = %d, want none", x.bulkPosts) + } +} + +// Room left below the limit caps the accept, and the rest wait for the next pass. +func TestFriendSyncAcceptsOnlyIntoRoom(t *testing.T) { + x := newFakeXbox() + x.limit = XboxFriendLimit + x.befriendMany(XboxFriendLimit - 2) + x.pending["1"], x.pending["2"], x.pending["3"] = true, true, true + s := &FriendSyncer{Client: x.client(), Config: FriendSyncConfig{AutoFollow: true}} + if _, again := s.runSync(context.Background(), false); !again { + t.Fatal("expected another pass to accept into the room just found") + } + s.runSync(context.Background(), false) + if len(x.following) != XboxFriendLimit || len(x.pending) != 1 || x.bulkPosts != 1 { + t.Fatalf("following=%d pending=%d posts=%d, want 2 accepted in one post and 1 left waiting", len(x.following), len(x.pending), x.bulkPosts) + } +} + +// Below the Xbox limit a full-list refusal is the requester's list: retry that request later, remove no one. +func TestFriendSyncTreatsFullListBelowLimitAsRequesters(t *testing.T) { + x := newFakeXbox() + x.befriend("1", "2") + x.pending["9"], x.pending["10"] = true, true + x.bulkAdd = func(xuids []string) *http.Response { + if slices.Contains(xuids, "9") { + return response(http.StatusBadRequest, `{"code":1028,"description":"full"}`) + } + for _, xuid := range xuids { + delete(x.pending, xuid) + x.befriend(xuid) + } + return updated(xuids) + } + s := &FriendSyncer{Client: x.client(), History: newMemoryHistory(), Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{MaxFriends: 10}}} + s.runSync(context.Background(), false) // learns the room + s.runSync(context.Background(), false) + posts := x.bulkPosts + s.runSync(context.Background(), false) + if !x.following["10"] || !x.following["1"] || !x.following["2"] || x.bulkPosts != posts { + t.Fatalf("following=%v posts %d -> %d, want 10 accepted, nobody removed and 9 not retried yet", x.following, posts, x.bulkPosts) + } +} + +// Follow-backs in this pass count toward maxFriends before the next snapshot. +func TestFriendSyncCountsThisPassFollowsTowardMaxFriends(t *testing.T) { + x := newFakeXbox() + x.befriend("1", "2") + x.followers["3"], x.followers["4"] = true, true + history := newMemoryHistory() + history.set("100", "1", time.Now().Add(-time.Hour)) + history.set("100", "2", time.Now()) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{MaxFriends: 3}}} + s.runSync(context.Background(), false) + if len(x.following) != 3 || x.following["1"] { + t.Fatalf("following = %v, want 3 friends with 1, the least recently seen, removed", x.following) + } +} + +// The pending-removal mark must be saved before Xbox changes, and undone if the change fails. +func TestFriendSyncMarksRemovalBeforeRemoving(t *testing.T) { + history := newMemoryHistory() + old := time.Now().Add(-20 * 24 * time.Hour).Truncate(time.Second) + history.set("100", "42", old) + markedFirst, calls := false, 0 + client := &syncFriendClient{ + people: []Person{{XUID: "42", IsFollowingCaller: true, IsFollowedByCaller: true}}, + removeFriend: func(context.Context, string) error { + calls++ + removing, _ := history.Removing(context.Background(), "100") + _, markedFirst = removing["42"] + return &xblsocial.ResponseError{StatusCode: http.StatusInternalServerError} + }, + } + s := &FriendSyncer{Client: client, History: history, Account: "100", + Config: FriendSyncConfig{AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + s.runSync(context.Background(), true) + if calls != 1 || !markedFirst { + t.Fatalf("RemoveFriend calls=%d marked before=%v, want the mark saved first", calls, markedFirst) + } + removing, _ := history.Removing(context.Background(), "100") + if seen, ok := history.get("100", "42"); len(removing) != 0 || !ok || !seen.Equal(old) { + t.Fatalf("after a failed removal marks=%v seen=%v, want the mark undone and the old clock kept", removing, seen) + } +} + +// A mark left by a removal that never reached Xbox is dropped while the friendship stands. +func TestFriendSyncDropsMarkForCurrentFriend(t *testing.T) { + x := newFakeXbox() + x.befriend("42") + history := newMemoryHistory() + _ = history.MarkRemoving(context.Background(), "100", time.Now(), "42") + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + s.runSync(context.Background(), true) + if removing, _ := history.Removing(context.Background(), "100"); len(removing) != 0 || !x.following["42"] { + t.Fatalf("marks=%v following=%v, want the stale mark dropped and 42 kept", removing, x.following["42"]) + } +} + +// An account-wide refusal of a bulk accept backs off instead of repeating every pass. +func TestFriendSyncBacksOffAccountWideAcceptRefusal(t *testing.T) { + x := newFakeXbox() + x.pending["1"], x.pending["2"] = true, true + x.bulkAdd = func([]string) *http.Response { return response(http.StatusForbidden, "") } + s := &FriendSyncer{Client: x.client(), Config: FriendSyncConfig{AutoFollow: true}} + for range 3 { + s.runSync(context.Background(), false) + } + if x.bulkPosts != 1 || s.state.followRetryUntil.IsZero() { + t.Fatalf("bulk posts=%d backoff=%v, want one post then backoff", x.bulkPosts, s.state.followRetryUntil) + } +} + +// Room made by cleanup at the limit is used for waiting requests on the very next pass. +func TestFriendSyncCleanupMakesRoomForWaitingRequests(t *testing.T) { + x := newFakeXbox() + x.limit = XboxFriendLimit + x.befriendMany(XboxFriendLimit) + x.pending["9"] = true + history := newMemoryHistory() + history.set("100", "1000", time.Now().Add(-20*24*time.Hour)) + s := &FriendSyncer{Client: x.client(), History: history, Account: "100", + Config: FriendSyncConfig{AutoFollow: true, AutoUnfollow: true, Cleanup: FriendCleanupConfig{InactiveDays: 15}}} + if _, again := s.runSync(context.Background(), true); !again { + t.Fatal("expected another pass to accept waiting requests") + } + s.runSync(context.Background(), false) + if x.following["1000"] || !x.following["9"] { + t.Fatalf("1000 kept=%v 9 added=%v, want 1000 removed and 9 accepted", x.following["1000"], x.following["9"]) + } +} diff --git a/friends.go b/friends.go index 096372d..d5fe14c 100644 --- a/friends.go +++ b/friends.go @@ -3,7 +3,7 @@ package broadcaster import ( "context" "errors" - "fmt" + "net/http" xblsocial "github.com/df-mc/go-xsapi/v2/social" ) @@ -30,23 +30,33 @@ type Person struct { var friendListConfig = xblsocial.PeopleListConfig{Undecorated: true, ContractVersion: 5} // addFriendsBatchSize stays below Xbox Live's undocumented bulk-operation -// limit. Code 1050 responses are also split dynamically in case the service -// applies a lower limit to a particular account or request. +// limit. Rejected batches are also split dynamically; see acceptFriends. const addFriendsBatchSize = 50 -// AcceptFriendRequestsError reports a successful bulk accept response that -// still failed to update one or more pending friend requests. -type AcceptFriendRequestsError struct { - Failed []string +// FriendRequestResult reports what happened to incoming friend requests. +type FriendRequestResult struct { + // Accepted lists requests Xbox reported as accepted. + Accepted []Person + // Rejected lists requests Xbox refused, each with the reason. + Rejected []RejectedFriendRequest + // Waiting counts requests still pending afterwards, including skipped ones. + Waiting int + // Deferred counts requests left untried because of the limit. + Deferred int } -func (e *AcceptFriendRequestsError) Error() string { - return fmt.Sprintf("accept pending friend requests: failed to update %d users", len(e.Failed)) +// RejectedFriendRequest is an incoming friend request Xbox refused to accept. +// Err matches [xblsocial.ErrFriendRestricted] when the person's privacy or +// enforcement settings block the friendship, and [xblsocial.ErrFriendListFull] +// when either the account's or the requester's list is full. +type RejectedFriendRequest struct { + Person Person + Err error } -func (e *AcceptFriendRequestsError) FailedXUIDs() []string { - return append([]string(nil), e.Failed...) -} +// ErrFriendRequestNotAccepted is reported for a request a bulk accept listed +// as failed without giving a reason. +var ErrFriendRequestNotAccepted = errors.New("xbox did not accept the friend request") // Friends returns a merged view of people following the authenticated account // and people the authenticated account follows. @@ -63,82 +73,90 @@ func (c FriendClient) Friends(ctx context.Context) ([]Person, error) { return mergePeople(peopleFromSocialUsers(followers), peopleFromSocialUsers(following)), nil } -// AcceptPendingFriendRequests accepts incoming Xbox friend requests with -// bounded add calls and returns the people that Xbox reported as updated. -func (c FriendClient) AcceptPendingFriendRequests(ctx context.Context) ([]Person, error) { - socialClient := c.social() - pending, err := socialClient.People(ctx, xblsocial.PeopleListIncomingFriendRequests, xblsocial.PeopleListConfig{Undecorated: true}) +// AcceptPendingFriendRequests tries at most limit incoming Xbox friend requests +// in bounded batches, leaving requests for which skip reports true pending. It +// stops at the first rate limit or server error; refusals are narrowed down to +// the people they apply to and reported in Rejected. +func (c FriendClient) AcceptPendingFriendRequests(ctx context.Context, limit int, skip func(xuid string) bool) (FriendRequestResult, error) { + var result FriendRequestResult + pending, err := c.social().People(ctx, xblsocial.PeopleListIncomingFriendRequests, xblsocial.PeopleListConfig{Undecorated: true}) if err != nil { - return nil, err + return result, err } - xuids := make([]string, 0, len(pending)) byXUID := make(map[string]Person, len(pending)) + xuids := make([]string, 0, len(pending)) for _, user := range pending { - if user.XUID == "" { + if _, dup := byXUID[user.XUID]; user.XUID == "" || dup { continue } - xuids = append(xuids, user.XUID) byXUID[user.XUID] = personFromSocialUser(user) + if skip == nil || !skip(user.XUID) { + xuids = append(xuids, user.XUID) + } } - if len(xuids) == 0 { - return nil, nil + if len(xuids) > max(limit, 0) { + result.Deferred = len(xuids) - max(limit, 0) + xuids = xuids[:max(limit, 0)] } - - updatedXUIDs := make([]string, 0, len(xuids)) - var bulkErr error - for start := 0; start < len(xuids); start += addFriendsBatchSize { - end := min(start+addFriendsBatchSize, len(xuids)) - updated, err := addFriends(ctx, socialClient, xuids[start:end]) - updatedXUIDs = append(updatedXUIDs, updated...) - if err != nil { - bulkErr = err - break - } + for start := 0; start < len(xuids) && err == nil; start += addFriendsBatchSize { + err = c.acceptFriends(ctx, xuids[start:min(start+addFriendsBatchSize, len(xuids))], byXUID, &result) } - updated := make(map[string]struct{}, len(updatedXUIDs)) - accepted := make([]Person, 0, len(updatedXUIDs)) - for _, xuid := range updatedXUIDs { - if person, ok := byXUID[xuid]; ok { + result.Waiting = len(byXUID) - len(result.Accepted) + return result, err +} + +// acceptFriends accepts xuids, splitting a refused batch down to the people it +// applies to. It returns an error only when later batches should not be tried. +func (c FriendClient) acceptFriends(ctx context.Context, xuids []string, byXUID map[string]Person, result *FriendRequestResult) error { + bulk, err := c.social().AddFriends(ctx, xuids) + if err == nil { + updated := make(map[string]struct{}, len(bulk.Updated)) + for _, xuid := range bulk.Updated { + person, ok := byXUID[xuid] + if _, dup := updated[xuid]; !ok || dup { + continue + } updated[xuid] = struct{}{} - accepted = append(accepted, person) + result.Accepted = append(result.Accepted, person) } - } - if bulkErr != nil { - return accepted, bulkErr - } - var failed []string - for _, xuid := range xuids { - if _, ok := updated[xuid]; !ok { - failed = append(failed, xuid) + for _, xuid := range xuids { + if _, ok := updated[xuid]; !ok { + result.Rejected = append(result.Rejected, RejectedFriendRequest{Person: byXUID[xuid], Err: ErrFriendRequestNotAccepted}) + } } + return nil } - if len(failed) > 0 { - return accepted, &AcceptFriendRequestsError{Failed: failed} + // A full-list refusal may be one requester's list, so it is split like any other. + switch { + case !isRequestRefusal(err): + return err + case len(xuids) == 1: + result.Rejected = append(result.Rejected, RejectedFriendRequest{Person: byXUID[xuids[0]], Err: err}) + return nil } - return accepted, nil -} - -// addFriends retries Xbox's bulk-limit response with progressively smaller -// batches. Successful work from the first half is retained if the second half -// fails for an unrelated reason such as rate limiting. -func addFriends(ctx context.Context, client *xblsocial.Client, xuids []string) ([]string, error) { - result, err := client.AddFriends(ctx, xuids) - if err == nil || len(xuids) == 1 || !isBulkOperationLimit(err) { - return result.Updated, err - } - middle := len(xuids) / 2 - left, err := addFriends(ctx, client, xuids[:middle]) - if err != nil { - return left, err + if err := c.acceptFriends(ctx, xuids[:middle], byXUID, result); err != nil { + return err } - right, err := addFriends(ctx, client, xuids[middle:]) - return append(left, right...), err + return c.acceptFriends(ctx, xuids[middle:], byXUID, result) } -func isBulkOperationLimit(err error) bool { +// isRequestRefusal reports whether err may apply to only some people in a +// bulk request, or to its size: a 400 without a code, or a known per-person +// or bulk-limit code. Other client errors fail the whole request. +func isRequestRefusal(err error) bool { var responseErr *xblsocial.ResponseError - return errors.As(err, &responseErr) && responseErr.Code == 1050 + if !errors.As(err, &responseErr) { + return false + } + switch responseErr.Code { + case 0: + return responseErr.StatusCode == http.StatusBadRequest + case 1011, 1015, 1028, 1039, 1049, 1050: + return true + default: + return false + } } // Follow follows the XUID, which makes the user a friend when they also follow @@ -147,16 +165,36 @@ func (c FriendClient) Follow(ctx context.Context, xuid string) error { return c.social().Follow(ctx, xuid) } -// Unfollow removes the mutual follow relationship with xuid. +// Unfollow drops the authenticated account's follow of xuid. Xbox keeps the +// user's follow of the account, so use it only for people who do not follow back. func (c FriendClient) Unfollow(ctx context.Context, xuid string) error { return c.social().RemoveMutualFollow(ctx, xuid) } -// ForceUnfollow removes the follow relationship the user identified by xuid -// has towards the authenticated account. It is used to drop followers whose -// privacy or enforcement restrictions prevent a friendship. -func (c FriendClient) ForceUnfollow(ctx context.Context, xuid string) error { - return c.social().RemoveFollower(ctx, xuid) +// RemoveFriend ends the friendship with xuid, or declines their pending +// request, so a new request is needed to be friends again. It does not remove +// a one-way follow; a 404 means there was no friendship or request to end. +func (c FriendClient) RemoveFriend(ctx context.Context, xuid string) error { + return c.social().RemoveFriend(ctx, xuid) +} + +// RemoveFollower drops xuid's follow of the authenticated account. +func (c FriendClient) RemoveFollower(ctx context.Context, xuid string) error { + return ignoreNotFound(c.social().RemoveFollower(ctx, xuid)) +} + +// ignoreNotFound treats a missing relationship as already removed. +func ignoreNotFound(err error) error { + if isNotFound(err) { + return nil + } + return err +} + +// isNotFound reports whether err is a social 404. +func isNotFound(err error) bool { + var responseErr *xblsocial.ResponseError + return errors.As(err, &responseErr) && responseErr.StatusCode == http.StatusNotFound } func (c FriendClient) social() *xblsocial.Client { diff --git a/friends_test.go b/friends_test.go index 835a59a..dbbe8eb 100644 --- a/friends_test.go +++ b/friends_test.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "testing" "time" @@ -247,10 +248,11 @@ func TestFriendClientAcceptPendingFriendRequestsUsesAddFriends(t *testing.T) { return nil, nil })))} - accepted, err := client.AcceptPendingFriendRequests(context.Background()) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) if err != nil { t.Fatal(err) } + accepted := result.Accepted wantRequests := strings.Join([]string{ http.MethodGet + " " + pendingRequestsURL, http.MethodPost + " " + addFriendsURL, @@ -298,11 +300,11 @@ func TestFriendClientAcceptPendingFriendRequestsBatchesAdds(t *testing.T) { return nil, nil })})} - accepted, err := client.AcceptPendingFriendRequests(context.Background()) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) if err != nil { t.Fatal(err) } - if len(accepted) != len(pending) { + if accepted := result.Accepted; len(accepted) != len(pending) { t.Fatalf("accepted %d people, want %d", len(accepted), len(pending)) } if got := fmt.Sprint(batchSizes); got != "[50 1]" { @@ -339,12 +341,12 @@ func TestFriendClientAcceptPendingFriendRequestsSplitsLimitErrors(t *testing.T) return nil, nil })})} - accepted, err := client.AcceptPendingFriendRequests(context.Background()) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) if err != nil { t.Fatal(err) } - if len(accepted) != 4 { - t.Fatalf("accepted people = %#v, want all four", accepted) + if len(result.Accepted) != 4 { + t.Fatalf("accepted people = %#v, want all four", result.Accepted) } if got := fmt.Sprint(batchSizes); got != "[4 2 2]" { t.Fatalf("bulk batch sizes = %s, want [4 2 2]", got) @@ -367,7 +369,7 @@ func TestFriendClientAcceptPendingFriendRequestsReturnsRetryAfterError(t *testin return nil, nil })})} - _, err := client.AcceptPendingFriendRequests(context.Background()) + _, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) if err == nil { t.Fatal("expected retry-after error") } @@ -387,32 +389,162 @@ func TestFriendClientAcceptPendingFriendRequestsReportsFailedUpdates(t *testing. case http.MethodGet: return response(http.StatusOK, `{"people":[{"xuid":"1","gamertag":"One"},{"xuid":"2","gamertag":"Two"}]}`), nil case http.MethodPost: - return response(http.StatusOK, `{"updatedPeople":["1"]}`), nil + return response(http.StatusOK, `{"updatedPeople":["1"],"failedToUpdate":["2"]}`), nil default: t.Fatalf("unexpected request %s %s", req.Method, req.URL) } return nil, nil })})} - accepted, err := client.AcceptPendingFriendRequests(context.Background()) - if err == nil { - t.Fatal("expected failed updates error") + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) + if err != nil { + t.Fatal(err) + } + if len(result.Accepted) != 1 || result.Accepted[0].XUID != "1" { + t.Fatalf("accepted people = %#v, want xuid 1", result.Accepted) + } + if len(result.Rejected) != 1 || result.Rejected[0].Person.XUID != "2" || !errors.Is(result.Rejected[0].Err, ErrFriendRequestNotAccepted) { + t.Fatalf("rejected = %#v, want xuid 2 not accepted", result.Rejected) + } + if result.Waiting != 1 { + t.Fatalf("waiting = %d, want 1", result.Waiting) } - if len(accepted) != 1 || accepted[0].XUID != "1" { - t.Fatalf("accepted people = %#v, want xuid 1", accepted) +} + +// bulkAddServer answers pending-list reads and bulk adds from a callback, recording each batch. +func bulkAddServer(t *testing.T, pending []string, add func(xuids []string) *http.Response) (FriendClient, *[][]string) { + t.Helper() + people := make([]map[string]string, 0, len(pending)) + for _, xuid := range pending { + people = append(people, map[string]string{"xuid": xuid, "gamertag": "GT" + xuid}) + } + pendingBody, err := json.Marshal(map[string]any{"people": people}) + if err != nil { + t.Fatal(err) } - var acceptErr interface { - FailedXUIDs() []string + var batches [][]string + client := FriendClient{Social: newTestSocialClient(&http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + var resp *http.Response + switch req.Method { + case http.MethodGet: + resp = response(http.StatusOK, string(pendingBody)) + case http.MethodPost: + var body struct { + XUIDs []string `json:"xuids"` + } + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + t.Fatal(err) + } + batches = append(batches, body.XUIDs) + resp = add(body.XUIDs) + default: + t.Fatalf("unexpected request %s %s", req.Method, req.URL) + } + resp.Request = req + return resp, nil + })})} + return client, &batches +} + +func updated(xuids []string) *http.Response { + body, _ := json.Marshal(map[string]any{"updatedPeople": xuids}) + return response(http.StatusOK, string(body)) +} + +// One refused request must not block the rest of its batch or later batches. +func TestFriendClientAcceptIsolatesRefusedRequests(t *testing.T) { + xuids := make([]string, addFriendsBatchSize+2) + for i := range xuids { + xuids[i] = fmt.Sprint(i + 1) + } + client, batches := bulkAddServer(t, xuids, func(batch []string) *http.Response { + for _, xuid := range batch { + switch xuid { + case "3": + return response(http.StatusBadRequest, "Bad Request") + case "4": + return response(http.StatusForbidden, `{"code":1049,"description":"privacy"}`) + } + } + return updated(batch) + }) + + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) + if err != nil { + t.Fatal(err) } - if !errors.As(err, &acceptErr) { - t.Fatalf("expected accept failure error, got %T: %v", err, err) + if len(result.Accepted) != len(xuids)-2 { + t.Fatalf("accepted %d people, want %d", len(result.Accepted), len(xuids)-2) } - if got := strings.Join(acceptErr.FailedXUIDs(), ","); got != "2" { - t.Fatalf("failed xuids = %s, want 2", got) + rejected := map[string]error{} + for _, r := range result.Rejected { + rejected[r.Person.XUID] = r.Err + } + if len(rejected) != 2 || !strings.Contains(fmt.Sprint(rejected["3"]), "Bad Request") || !errors.Is(rejected["4"], xblsocial.ErrFriendRestricted) { + t.Fatalf("rejected = %v, want 3 with its body and 4 restricted", rejected) + } + if last := (*batches)[len(*batches)-1]; strings.Join(last, ",") != fmt.Sprintf("%d,%d", addFriendsBatchSize+1, addFriendsBatchSize+2) { + t.Fatalf("last batch = %v, want the second page of requests", last) + } +} + +// A full-list refusal may be one requester's list, so it is narrowed to the people it applies to. +func TestFriendClientAcceptSplitsFullListRefusals(t *testing.T) { + client, _ := bulkAddServer(t, []string{"1", "2", "3"}, func(batch []string) *http.Response { + if slices.Contains(batch, "2") { + return response(http.StatusBadRequest, `{"code":1028,"description":"full"}`) + } + return updated(batch) + }) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) + if err != nil { + t.Fatal(err) + } + if len(result.Accepted) != 2 || len(result.Rejected) != 1 || result.Rejected[0].Person.XUID != "2" || !errors.Is(result.Rejected[0].Err, xblsocial.ErrFriendListFull) { + t.Fatalf("result = %+v, want 1 and 3 accepted and 2 refused as list full", result) + } +} + +// Only per-person or bulk-size refusals are split; an account-wide one fails the request once. +func TestFriendClientAcceptDoesNotSplitAccountWideRefusals(t *testing.T) { + client, batches := bulkAddServer(t, []string{"1", "2", "3"}, func([]string) *http.Response { + return response(http.StatusForbidden, "") + }) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, nil) + if err == nil || len(*batches) != 1 || len(result.Rejected) != 0 || result.Waiting != 3 { + t.Fatalf("err=%v batches=%v result=%+v, want one failed request and nobody marked rejected", err, *batches, result) + } +} + +func TestFriendClientAcceptSkipsRequests(t *testing.T) { + client, batches := bulkAddServer(t, []string{"1", "2"}, updated) + result, err := client.AcceptPendingFriendRequests(context.Background(), XboxFriendLimit, func(xuid string) bool { return xuid == "1" }) + if err != nil { + t.Fatal(err) + } + if fmt.Sprint(*batches) != "[[2]]" || result.Waiting != 1 { + t.Fatalf("batches = %v waiting = %d, want [[2]] and 1", *batches, result.Waiting) + } +} + +func TestFriendClientRemoveFriendEndsFriendship(t *testing.T) { + var requests []string + client := FriendClient{Social: newTestSocialClient(&http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + requests = append(requests, req.Method+" "+req.URL.String()) + resp := response(http.StatusNotFound, "") + resp.Request = req + return resp, nil + })})} + // A 404 means no friendship was ended, so it must not pass as a freed slot. + if err := client.RemoveFriend(context.Background(), "123"); !isNotFound(err) { + t.Fatalf("RemoveFriend() error = %v, want the 404", err) + } + if want := "DELETE https://social.xboxlive.com/users/me/people/friends/v2/xuid(123)?deleteRelationships=friends"; strings.Join(requests, ",") != want { + t.Fatalf("requests = %v, want %s", requests, want) } } -func TestFriendClientForceUnfollowDeletesFollowerRelationship(t *testing.T) { +func TestFriendClientRemoveFollowerDeletesFollowerRelationship(t *testing.T) { called := false client := FriendClient{Social: newTestSocialClient( &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { @@ -426,7 +558,7 @@ func TestFriendClientForceUnfollowDeletesFollowerRelationship(t *testing.T) { return response(http.StatusNoContent, ""), nil })})} - if err := client.ForceUnfollow(context.Background(), "123"); err != nil { + if err := client.RemoveFollower(context.Background(), "123"); err != nil { t.Fatal(err) } if !called { diff --git a/gallery.go b/gallery.go index cef40dc..3f10d6e 100644 --- a/gallery.go +++ b/gallery.go @@ -144,16 +144,20 @@ func (g GalleryClient) Upload(ctx context.Context, imagePath string, featured bo return GalleryImage{}, err } defer f.Close() + stat, err := f.Stat() + if err != nil { + return GalleryImage{}, err + } req, err := g.request(ctx, http.MethodPost, galleryURL, f) if err != nil { return GalleryImage{}, err } + // Send a Content-Length instead of a chunked body. + req.ContentLength = stat.Size() req.Header.Set("Content-Type", "application/octet-stream") req.Header.Set("X-Ms-Showcased-Featured", fmt.Sprint(featured)) - if stat, err := f.Stat(); err == nil { - // UTC with milliseconds, matching Java's Instant.toString(). - req.Header.Set("X-Ms-Showcased-Timetaken", stat.ModTime().UTC().Format("2006-01-02T15:04:05.000Z")) - } + // UTC with milliseconds, matching Java's Instant.toString(). + req.Header.Set("X-Ms-Showcased-Timetaken", stat.ModTime().UTC().Format("2006-01-02T15:04:05.000Z")) resp, err := g.client().Do(req) if err != nil { return GalleryImage{}, err @@ -162,11 +166,11 @@ func (g GalleryClient) Upload(ctx context.Context, imagePath string, featured bo if resp.StatusCode != http.StatusAccepted { return GalleryImage{}, fmt.Errorf("%s %s: %s", req.Method, req.URL, resp.Status) } - var data galleryUploadResponse - if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { + var uploaded galleryUploadResponse + if err := json.NewDecoder(resp.Body).Decode(&uploaded); err != nil { return GalleryImage{}, err } - return data.Result, nil + return uploaded.Result, nil } func (g GalleryClient) Delete(ctx context.Context, imageID string) error { diff --git a/gallery_test.go b/gallery_test.go index caac984..4d6eb51 100644 --- a/gallery_test.go +++ b/gallery_test.go @@ -438,3 +438,23 @@ type galleryTokenSource struct { func (s galleryTokenSource) ServiceToken(context.Context) (*service.Token, error) { return &service.Token{AuthorizationHeader: s.authorization, ValidUntil: time.Now().Add(time.Hour)}, nil } + +// Uploads must carry a Content-Length rather than a chunked body. +func TestGalleryUploadSendsContentLength(t *testing.T) { + var gotLength int64 + var gotEncoding []string + path := filepath.Join(t.TempDir(), "a.png") + if err := os.WriteFile(path, []byte("0123456789"), 0o600); err != nil { + t.Fatal(err) + } + g := GalleryClient{TokenSource: galleryTokenSource{authorization: "Bearer t"}, Client: &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + gotLength, gotEncoding = req.ContentLength, req.TransferEncoding + return response(http.StatusAccepted, `{"result":{"id":"x"}}`), nil + })}} + if _, err := g.Upload(context.Background(), path, true); err != nil { + t.Fatal(err) + } + if gotLength != 10 || len(gotEncoding) != 0 { + t.Fatalf("ContentLength=%d TransferEncoding=%v, want 10 and none", gotLength, gotEncoding) + } +} diff --git a/integration_clients_test.go b/integration_clients_test.go index 8f41ea5..a92fbe9 100644 --- a/integration_clients_test.go +++ b/integration_clients_test.go @@ -8,7 +8,6 @@ import ( "io" "net" "net/http" - "path/filepath" "strings" "sync" "testing" @@ -239,61 +238,45 @@ func TestFriendSyncerSendsInitialInvite(t *testing.T) { } } -func TestFriendSyncerExpiresInactiveFriends(t *testing.T) { - var unfollowed bool +func TestFriendSyncerRemovesInactiveFriends(t *testing.T) { + var removed bool + history := newMemoryHistory() + history.set("me", "1", time.Now().Add(-48*time.Hour)) syncer := FriendSyncer{ Client: fakeFriendClient{ - people: []Person{{XUID: "1", Gamertag: "Old", IsFollowedByCaller: true}}, - unfollow: func(xuid string) { unfollowed = xuid == "1" }, + people: []Person{{XUID: "1", Gamertag: "Old", IsFollowingCaller: true, IsFollowedByCaller: true}}, + removeFriend: func(xuid string) { removed = xuid == "1" }, }, - History: fakeHistoryStore{seen: map[string]time.Time{"1": time.Now().Add(-48 * time.Hour)}}, - Config: FriendSyncConfig{ExpiryEnabled: true, ExpiryDays: 1}, + History: history, + Account: "me", + Config: FriendSyncConfig{Cleanup: FriendCleanupConfig{InactiveDays: 1}}, } if err := syncer.Sync(context.Background()); err != nil { t.Fatal(err) } - if !unfollowed { - t.Fatal("expected inactive friend to be unfollowed") + if !removed { + t.Fatal("expected inactive friend to be removed") } } -func TestFriendSyncerDefaultsExpiryDays(t *testing.T) { - var unfollowed bool +func TestFriendSyncerKeepsFriendsWhenInactiveDaysIsZero(t *testing.T) { + var removed bool + history := newMemoryHistory() + history.set("me", "1", time.Now().Add(-365*24*time.Hour)) syncer := FriendSyncer{ Client: fakeFriendClient{ - people: []Person{{XUID: "1", Gamertag: "Recent", IsFollowedByCaller: true}}, - unfollow: func(string) { unfollowed = true }, + people: []Person{{XUID: "1", Gamertag: "Old", IsFollowedByCaller: true}}, + removeFriend: func(string) { removed = true }, }, - History: fakeHistoryStore{seen: map[string]time.Time{"1": time.Now().Add(-24 * time.Hour)}}, - Config: FriendSyncConfig{ExpiryEnabled: true}, + History: history, + Account: "me", + Config: FriendSyncConfig{Cleanup: FriendCleanupConfig{MaxFriends: 10}}, } if err := syncer.Sync(context.Background()); err != nil { t.Fatal(err) } - if unfollowed { - t.Fatal("zero expiry days should default instead of pruning recent friends") - } -} - -func TestFileHistoryStoreRecordsAndClearsLastSeen(t *testing.T) { - store := NewFileHistoryStore(filepath.Join(t.TempDir(), "player_history.json")) - when := time.Now().Add(-time.Hour).Truncate(time.Second) - - if err := store.Seen(context.Background(), "1", when); err != nil { - t.Fatal(err) - } - lastSeen, ok, err := store.LastSeen(context.Background(), "1") - if err != nil { - t.Fatal(err) - } - if !ok || !lastSeen.Equal(when) { - t.Fatalf("last seen = %s, %v", lastSeen, ok) - } - if err := store.Clear(context.Background(), "1"); err != nil { - t.Fatal(err) - } - if _, ok, err := store.LastSeen(context.Background(), "1"); err != nil || ok { - t.Fatalf("expected cleared history, ok=%v err=%v", ok, err) + if removed { + t.Fatal("inactiveDays 0 must not remove friends for inactivity") } } @@ -714,9 +697,10 @@ func (s staticStatusProvider) RoomStatus() room.Status { } type fakeFriendClient struct { - people []Person - follow func(string) - unfollow func(string) + people []Person + follow func(string) + unfollow func(string) + removeFriend func(string) } func (f fakeFriendClient) Friends(context.Context) ([]Person, error) { return f.people, nil } @@ -732,6 +716,13 @@ func (f fakeFriendClient) Unfollow(_ context.Context, xuid string) error { } return nil } +func (f fakeFriendClient) RemoveFriend(_ context.Context, xuid string) error { + if f.removeFriend != nil { + f.removeFriend(xuid) + } + return nil +} +func (f fakeFriendClient) RemoveFollower(context.Context, string) error { return nil } type fakeInviter struct { invite func(string) @@ -742,20 +733,6 @@ func (f fakeInviter) Invite(_ context.Context, xuid, _ string) error { return nil } -type fakeHistoryStore struct { - seen map[string]time.Time -} - -func (f fakeHistoryStore) LastSeen(_ context.Context, xuid string) (time.Time, bool, error) { - t, ok := f.seen[xuid] - return t, ok, nil -} - -func (f fakeHistoryStore) Clear(_ context.Context, xuid string) error { - delete(f.seen, xuid) - return nil -} - type fakeNotifier struct { notify func(context.Context, string) } diff --git a/player_history.go b/player_history.go index 82224f7..4379e99 100644 --- a/player_history.go +++ b/player_history.go @@ -4,108 +4,226 @@ import ( "context" "encoding/json" "errors" + "fmt" + "log/slog" + "maps" "os" "sync" "time" ) +// FileHistoryStore is a HistoryStore kept in one JSON file, with entries keyed +// by account. The file is read once and then served from memory, so one +// process must own it. +// +// A flat file from older versions is copied to each account the first time +// that account is read, so read every account before the first write. type FileHistoryStore struct { Path string + // Log receives a warning when a corrupt file is moved aside. It may be nil. + Log *slog.Logger - mu sync.Mutex + mu sync.Mutex + loaded bool + dirty bool // memory holds changes a failed save did not write + accounts map[string]*accountHistory + legacy map[string]int64 // flat history from before entries were keyed by account +} + +// accountHistory is one account's part of the history file, in Unix seconds. +type accountHistory struct { + Seen map[string]int64 `json:"seen"` + Removing map[string]int64 `json:"removing,omitempty"` +} + +// historyFile is the on-disk layout of a FileHistoryStore. +type historyFile struct { + Accounts map[string]*accountHistory `json:"accounts"` } func NewFileHistoryStore(path string) *FileHistoryStore { return &FileHistoryStore{Path: path} } -func (s *FileHistoryStore) LastSeen(ctx context.Context, xuid string) (time.Time, bool, error) { - if err := ctxErr(ctx); err != nil { - return time.Time{}, false, err - } - s.mu.Lock() - defer s.mu.Unlock() +func (s *FileHistoryStore) LastSeen(ctx context.Context, account string) (map[string]time.Time, error) { + return s.read(ctx, account, func(h *accountHistory) map[string]int64 { return h.Seen }) +} - history, err := s.load() - if err != nil { - return time.Time{}, false, err - } - seconds, ok := history[xuid] - if !ok { - return time.Time{}, false, nil - } - return time.Unix(seconds, 0).UTC(), true, nil +func (s *FileHistoryStore) Removing(ctx context.Context, account string) (map[string]time.Time, error) { + return s.read(ctx, account, func(h *accountHistory) map[string]int64 { return h.Removing }) +} + +func (s *FileHistoryStore) Track(ctx context.Context, account string, when time.Time, xuids ...string) error { + return s.update(ctx, func() bool { + seen := s.account(account).Seen + changed := false + for _, xuid := range xuids { + if _, ok := seen[xuid]; !ok { + seen[xuid] = when.Unix() + changed = true + } + } + return changed + }) } func (s *FileHistoryStore) Seen(ctx context.Context, xuid string, when time.Time) error { - if err := ctxErr(ctx); err != nil { - return err - } - s.mu.Lock() - defer s.mu.Unlock() + return s.update(ctx, func() bool { + changed := false + for _, h := range s.accounts { + if _, ok := h.Seen[xuid]; ok { + h.Seen[xuid] = when.Unix() + changed = true + } + } + if _, ok := s.legacy[xuid]; ok { + s.legacy[xuid] = when.Unix() + } + return changed + }) +} - history, err := s.load() - if err != nil { - return err - } - history[xuid] = when.Unix() - return s.save(history) +func (s *FileHistoryStore) MarkRemoving(ctx context.Context, account string, when time.Time, xuids ...string) error { + return s.update(ctx, func() bool { + h := s.account(account) + if h.Removing == nil { + h.Removing = map[string]int64{} + } + for _, xuid := range xuids { + delete(h.Seen, xuid) + h.Removing[xuid] = when.Unix() + } + return len(xuids) > 0 + }) } -// XUIDs returns every XUID with a recorded last-seen time. -func (s *FileHistoryStore) XUIDs(ctx context.Context) ([]string, error) { +func (s *FileHistoryStore) Forget(ctx context.Context, account string, xuids ...string) error { + return s.update(ctx, func() bool { + h := s.account(account) + changed := false + for _, xuid := range xuids { + _, seen := h.Seen[xuid] + _, removing := h.Removing[xuid] + if seen || removing { + delete(h.Seen, xuid) + delete(h.Removing, xuid) + changed = true + } + } + return changed + }) +} + +// read returns one of account's maps as times. +func (s *FileHistoryStore) read(ctx context.Context, account string, pick func(*accountHistory) map[string]int64) (map[string]time.Time, error) { if err := ctxErr(ctx); err != nil { return nil, err } s.mu.Lock() defer s.mu.Unlock() - - history, err := s.load() - if err != nil { + if err := s.load(); err != nil { return nil, err } - xuids := make([]string, 0, len(history)) - for xuid := range history { - xuids = append(xuids, xuid) + entries := pick(s.account(account)) + out := make(map[string]time.Time, len(entries)) + for xuid, seconds := range entries { + out[xuid] = time.Unix(seconds, 0).UTC() } - return xuids, nil + return out, nil } -func (s *FileHistoryStore) Clear(ctx context.Context, xuid string) error { +// update applies change under the lock and saves when it reports a change or +// an earlier save failed, so a failed write is retried by the next update. +func (s *FileHistoryStore) update(ctx context.Context, change func() bool) error { if err := ctxErr(ctx); err != nil { return err } s.mu.Lock() defer s.mu.Unlock() - - history, err := s.load() - if err != nil { + if err := s.load(); err != nil { + return err + } + if !change() && !s.dirty { + return nil + } + s.dirty = true + if err := s.save(); err != nil { return err } - delete(history, xuid) - return s.save(history) + s.dirty = false + return nil } -func (s *FileHistoryStore) load() (map[string]int64, error) { +// account returns account's history, creating it from legacy history if needed. +func (s *FileHistoryStore) account(account string) *accountHistory { + h, ok := s.accounts[account] + if !ok { + h = &accountHistory{Seen: maps.Clone(s.legacy)} + s.accounts[account] = h + } + if h.Seen == nil { + h.Seen = map[string]int64{} + } + return h +} + +func (s *FileHistoryStore) load() error { + if s.loaded { + return nil + } data, err := os.ReadFile(s.Path) - if errors.Is(err, os.ErrNotExist) { - return map[string]int64{}, nil + if err != nil && !errors.Is(err, os.ErrNotExist) { + return err } - if err != nil { - return nil, err + s.accounts = map[string]*accountHistory{} + if len(data) > 0 { + if err := s.decode(data); err != nil { + s.quarantine(err) + } } - if len(data) == 0 { - return map[string]int64{}, nil + s.loaded = true + return nil +} + +// decode reads the account-keyed layout, or a flat XUID map from older versions. +func (s *FileHistoryStore) decode(data []byte) error { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err } - history := map[string]int64{} - if err := json.Unmarshal(data, &history); err != nil { - return nil, err + if _, ok := raw["accounts"]; ok { + var file historyFile + if err := json.Unmarshal(data, &file); err != nil { + return err + } + for account, h := range file.Accounts { + if h != nil { + s.accounts[account] = h + } + } + return nil + } + legacy := map[string]int64{} + if err := json.Unmarshal(data, &legacy); err != nil { + return err + } + s.legacy = legacy + return nil +} + +// quarantine moves an unreadable file aside so the store can start fresh. +func (s *FileHistoryStore) quarantine(cause error) { + s.accounts = map[string]*accountHistory{} + s.legacy = nil + dest := fmt.Sprintf("%s.corrupt-%d", s.Path, time.Now().Unix()) + err := os.Rename(s.Path, dest) + if s.Log != nil { + s.Log.Warn("player history is corrupt; starting fresh", "path", s.Path, "moved_to", dest, "err", cause, "rename_err", err) } - return history, nil } -func (s *FileHistoryStore) save(history map[string]int64) error { - data, err := json.MarshalIndent(history, "", " ") +func (s *FileHistoryStore) save() error { + data, err := json.MarshalIndent(historyFile{Accounts: s.accounts}, "", " ") if err != nil { return err } diff --git a/player_history_test.go b/player_history_test.go new file mode 100644 index 0000000..c703645 --- /dev/null +++ b/player_history_test.go @@ -0,0 +1,181 @@ +package broadcaster + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +func TestFileHistoryStoreTracksSeesAndForgets(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "player_history.json") + store := NewFileHistoryStore(path) + tracked := time.Now().Add(-time.Hour).Truncate(time.Second) + joined := tracked.Add(30 * time.Minute) + + if err := store.Track(ctx, "me", tracked, "1", "2"); err != nil { + t.Fatal(err) + } + if err := store.Track(ctx, "me", joined, "1"); err != nil { + t.Fatal(err) + } + if err := store.Seen(ctx, "2", joined); err != nil { + t.Fatal(err) + } + if err := store.Forget(ctx, "me", "1"); err != nil { + t.Fatal(err) + } + // A fresh store reads what the first one saved. + lastSeen, err := NewFileHistoryStore(path).LastSeen(ctx, "me") + if err != nil { + t.Fatal(err) + } + if _, ok := lastSeen["1"]; ok || len(lastSeen) != 1 || !lastSeen["2"].Equal(joined) { + t.Fatalf("last seen = %v, want only 2 at %s", lastSeen, joined) + } +} + +// One account's removals and pruning must not reset or delete another's entries. +func TestFileHistoryStoreKeepsAccountsSeparate(t *testing.T) { + ctx := context.Background() + store := NewFileHistoryStore(filepath.Join(t.TempDir(), "player_history.json")) + old := time.Now().Add(-10 * 24 * time.Hour).Truncate(time.Second) + for _, account := range []string{"primary", "sub"} { + if err := store.Track(ctx, account, old, "friend"); err != nil { + t.Fatal(err) + } + } + if err := store.Forget(ctx, "primary", "friend"); err != nil { + t.Fatal(err) + } + sub, err := store.LastSeen(ctx, "sub") + if err != nil { + t.Fatal(err) + } + if !sub["friend"].Equal(old) { + t.Fatalf("sub-account entry = %v, want untouched %s", sub["friend"], old) + } + // A join only refreshes accounts that still track the player. + joined := time.Now().Truncate(time.Second) + if err := store.Seen(ctx, "friend", joined); err != nil { + t.Fatal(err) + } + primary, _ := store.LastSeen(ctx, "primary") + sub, _ = store.LastSeen(ctx, "sub") + if _, ok := primary["friend"]; ok || !sub["friend"].Equal(joined) { + t.Fatalf("after join primary=%v sub=%v, want only sub refreshed", primary, sub) + } +} + +// History written before entries were keyed by account keeps every clock. +func TestFileHistoryStoreMigratesFlatHistory(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "player_history.json") + if err := os.WriteFile(path, []byte(`{"1": 1700000000, "2": 1700000100}`), 0o600); err != nil { + t.Fatal(err) + } + store := NewFileHistoryStore(path) + if err := store.Forget(ctx, "primary", "2"); err != nil { + t.Fatal(err) + } + for account, want := range map[string]int{"primary": 1, "sub": 2} { + lastSeen, err := store.LastSeen(ctx, account) + if err != nil { + t.Fatal(err) + } + if len(lastSeen) != want || lastSeen["1"].Unix() != 1700000000 { + t.Fatalf("%s history = %v, want %d entries keeping the old clock", account, lastSeen, want) + } + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"accounts"`) { + t.Fatalf("saved history is not keyed by account:\n%s", data) + } +} + +// A corrupt file is moved aside instead of failing every lookup forever. +func TestFileHistoryStoreQuarantinesCorruptFile(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + path := filepath.Join(dir, "player_history.json") + if err := os.WriteFile(path, []byte(`{"accounts": {`), 0o600); err != nil { + t.Fatal(err) + } + store := NewFileHistoryStore(path) + if lastSeen, err := store.LastSeen(ctx, "me"); err != nil || len(lastSeen) != 0 { + t.Fatalf("LastSeen() = %v, %v; want empty history", lastSeen, err) + } + if err := store.Track(ctx, "me", time.Now(), "1"); err != nil { + t.Fatal(err) + } + moved, err := filepath.Glob(path + ".corrupt-*") + if err != nil || len(moved) != 1 { + t.Fatalf("quarantined files = %v, %v; want one", moved, err) + } + if temps, _ := filepath.Glob(filepath.Join(dir, "*.tmp")); len(temps) != 0 { + t.Fatalf("temporary files left behind: %v", temps) + } +} + +// Every account read before the first write keeps the flat history after a restart. +func TestFileHistoryStoreMigratesFlatHistoryForEveryReadAccount(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "player_history.json") + if err := os.WriteFile(path, []byte(`{"1": 1700000000}`), 0o600); err != nil { + t.Fatal(err) + } + b := &Broadcaster{log: testBroadcasterLogger(), ctx: ctx, conf: Config{ + XUID: "primary", + FriendHistory: NewFileHistoryStore(path), + SubAccounts: []SubAccountConfig{{ID: "sub", Enabled: true, XUID: "sub"}}, + }} + b.primeFriendHistory() + if err := b.conf.FriendHistory.Track(ctx, "primary", time.Now(), "2"); err != nil { + t.Fatal(err) + } + sub, err := NewFileHistoryStore(path).LastSeen(ctx, "sub") + if err != nil { + t.Fatal(err) + } + if sub["1"].Unix() != 1700000000 { + t.Fatalf("sub-account history after restart = %v, want the migrated clock", sub) + } +} + +// A failed write must be retried by the next update, even one that changes nothing. +func TestFileHistoryStoreRetriesFailedSave(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("a read-only directory still accepts writes here") + } + ctx := context.Background() + dir := filepath.Join(t.TempDir(), "cache") + if err := os.Mkdir(dir, 0o500); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "player_history.json") + store := NewFileHistoryStore(path) + when := time.Now().Truncate(time.Second) + if err := store.Track(ctx, "me", when, "1"); err == nil { + t.Fatal("expected the save to fail in a read-only directory") + } + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := store.Track(ctx, "me", when.Add(time.Hour), "1"); err != nil { + t.Fatal(err) + } + lastSeen, err := NewFileHistoryStore(path).LastSeen(ctx, "me") + if err != nil { + t.Fatal(err) + } + if !lastSeen["1"].Equal(when) { + t.Fatalf("saved history = %v, want 1 tracked at %s", lastSeen, when) + } +} diff --git a/pterodactyl_test.go b/pterodactyl_test.go index 2160069..0a38177 100644 --- a/pterodactyl_test.go +++ b/pterodactyl_test.go @@ -105,6 +105,9 @@ func TestPterodactylArtifacts(t *testing.T) { t.Fatalf("installation script does not contain %q", want) } } + if want := fmt.Sprintf("configVersion: %d\n", CurrentConfigVersion); !strings.Contains(egg.Scripts.Installation.Script, want) { + t.Fatalf("installation script does not write %q", want) + } if strings.Contains(egg.Scripts.Installation.Script, "signalingMode:") { t.Fatal("installation script should rely on the default signaling mode") } diff --git a/relay.go b/relay.go index 5372f4b..e3e8209 100644 --- a/relay.go +++ b/relay.go @@ -180,8 +180,8 @@ func (b *Broadcaster) relay(conn relayClientConn) { } defer server.Close() - if recorder, ok := b.conf.FriendHistory.(HistoryRecorder); ok && id.XUID != "" { - if err := recorder.Seen(ctx, id.XUID, time.Now()); err != nil { + if b.conf.FriendHistory != nil && id.XUID != "" { + if err := b.conf.FriendHistory.Seen(ctx, id.XUID, time.Now()); err != nil { b.log.Error("record player history", "xuid", id.XUID, "err", err) } } diff --git a/social_subscription.go b/social_subscription.go index b139662..9a178a7 100644 --- a/social_subscription.go +++ b/social_subscription.go @@ -35,6 +35,12 @@ func (b *Broadcaster) startSocialSubscription(client *xsapi.Client, conf *Friend return b.subscribeSocial(client.Social(), log) } +// Backoff between social RTA subscribe attempts after a failure or a loss. +const ( + socialResubscribeMinDelay = 5 * time.Second + socialResubscribeMaxDelay = 5 * time.Minute +) + // subscribeSocial subscribes sub to the social RTA feed and returns a trigger // channel that fires on each event. The subscription's lifetime is bound to the // broadcaster: a dedicated goroutine unsubscribes once the broadcaster's context @@ -42,45 +48,87 @@ func (b *Broadcaster) startSocialSubscription(client *xsapi.Client, conf *Friend // caller-provided client (which the broadcaster does not close) from // accumulating stale handlers across broadcaster restarts. // -// The subscription is best-effort: on a subscribe failure or a lost subscription -// the periodic syncer remains the backstop, and the RTA connection re-subscribes -// automatically after a transient drop. +// A failed or lost subscription is retried with backoff; meanwhile the +// periodic syncer is the backstop. func (b *Broadcaster) subscribeSocial(sub socialSubscriber, log *slog.Logger) <-chan struct{} { // Buffered by one so bursts of events collapse into a single pending pass. trigger := make(chan struct{}, 1) - handler := friendRequestSubscriptionHandler{trigger: trigger, log: log} + lost := make(chan struct{}, 1) + handler := friendRequestSubscriptionHandler{trigger: trigger, lost: lost, log: log} b.socialWg.Add(1) go func() { defer b.socialWg.Done() - // Subscribe dials RTA lazily; running it here keeps a slow or failing - // dial off the start path. Bound setup because it holds the shared - // RTA subscription lock while waiting for an acknowledgment. - ctx, cancel := xboxOperationContext(b.ctx) - unsubscribe, err := sub.Subscribe(ctx, handler) - cancel() - if err != nil { - log.Warn("subscribe to social rta feed; friend requests will be accepted on the sync interval", "err", err) - return - } - log.Debug("subscribed to social rta feed for reactive friend sync") + delay := socialResubscribeMinDelay + resubscribing := false + for { + select { + case <-lost: // a stale loss from the previous registration + default: + } + // Subscribe dials RTA lazily; running it here keeps a slow or failing + // dial off the start path. Bound setup because it holds the shared + // RTA subscription lock while waiting for an acknowledgment. + ctx, cancel := xboxOperationContext(b.ctx) + unsubscribe, err := sub.Subscribe(ctx, handler) + cancel() + if err != nil { + if b.ctx.Err() != nil { + return + } + log.Warn("subscribe to social rta feed; friend requests will be accepted on the sync interval", "err", err, "retry_in", delay) + if !sleepContext(b.ctx, delay) { + return + } + delay = min(delay*2, socialResubscribeMaxDelay) + continue + } + log.Debug("subscribed to social rta feed for reactive friend sync") + if resubscribing { + handler.signal() // catch up on anything missed while unsubscribed + } + resubscribing = true - <-b.ctx.Done() - // b.ctx is done, so use a fresh context to release the subscription. - // The cleanup removes only this registration, so a shared client's other - // subscribers keep working. - ctx, cancel = context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - if err := unsubscribe(ctx); err != nil { - log.Debug("unsubscribe social rta feed", "err", err) + select { + case <-b.ctx.Done(): + case <-lost: + } + // Release this registration even when lost, so resubscribing does not + // leave a stale handler on a shared client. b.ctx may be done, so use a + // fresh context. + ctx, cancel = context.WithTimeout(context.Background(), 15*time.Second) + if err := unsubscribe(ctx); err != nil { + log.Debug("unsubscribe social rta feed", "err", err) + } + cancel() + if b.ctx.Err() != nil { + return + } + delay = socialResubscribeMinDelay + if !sleepContext(b.ctx, delay) { + return + } } }() return trigger } +// sleepContext waits for d and reports false if ctx ended first. +func sleepContext(ctx context.Context, d time.Duration) bool { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-timer.C: + return true + case <-ctx.Done(): + return false + } +} + // friendRequestSubscriptionHandler adapts go-xsapi's social RTA subscription to // the friend syncer. Events request a pass through its normal rate limits. type friendRequestSubscriptionHandler struct { trigger chan<- struct{} + lost chan<- struct{} log *slog.Logger } @@ -99,11 +147,14 @@ func (h friendRequestSubscriptionHandler) HandleSocialNotification(typ string, x h.signal() } -// HandleSubscriptionLost logs the loss. The periodic syncer continues to accept -// requests, and the RTA connection re-subscribes automatically after transient -// drops, so no action is taken here. +// HandleSubscriptionLost asks subscribeSocial to subscribe again. The periodic +// syncer keeps accepting requests meanwhile. func (h friendRequestSubscriptionHandler) HandleSubscriptionLost() { - h.log.Warn("social subscription lost; friend requests will be accepted on the sync interval until it is restored") + h.log.Warn("social subscription lost; resubscribing, friend requests will be accepted on the sync interval until then") + select { + case h.lost <- struct{}{}: + default: + } } // signal requests a sync pass without blocking. A full buffer means a pass is diff --git a/social_subscription_test.go b/social_subscription_test.go index 36816d8..b7ca05f 100644 --- a/social_subscription_test.go +++ b/social_subscription_test.go @@ -108,12 +108,17 @@ func TestFriendRequestSubscriptionHandlerCoalescesEvents(t *testing.T) { type fakeSocialSubscriber struct { subscribeErr error subscribed chan xblsocial.SubscriptionHandler + attempts atomic.Int32 cleanupCalls atomic.Int32 } // Subscribe records the handler and returns its cleanup or the configured error. func (f *fakeSocialSubscriber) Subscribe(_ context.Context, h xblsocial.SubscriptionHandler) (func(context.Context) error, error) { - f.subscribed <- h + f.attempts.Add(1) + select { + case f.subscribed <- h: + default: + } if f.subscribeErr != nil { return nil, f.subscribeErr } @@ -160,10 +165,11 @@ func TestSocialSubscriptionSetupTimesOut(t *testing.T) { default: t.Fatal("subscription setup did not time out") } - b.socialWg.Wait() if err := b.ctx.Err(); err != nil { t.Fatalf("setup timeout stopped the broadcaster: %v", err) } + b.cancel() + b.socialWg.Wait() }) } @@ -250,20 +256,20 @@ func TestReactiveFriendSyncPreservesMutationBackoff(t *testing.T) { defer cancel() go syncer.Run(ctx) synctest.Wait() - if accepts != 1 || client.removeCalls != 1 { - t.Fatalf("initial accepts=%d removals=%d, want 1 each", accepts, client.removeCalls) + if accepts != 1 || client.unfollowCalls != 1 { + t.Fatalf("initial accepts=%d removals=%d, want 1 each", accepts, client.unfollowCalls) } trigger <- struct{}{} time.Sleep(20 * time.Second) synctest.Wait() - if accepts != 1 || client.removeCalls != 2 { - t.Fatalf("during backoff accepts=%d removals=%d, want 1 and 2", accepts, client.removeCalls) + if accepts != 1 || client.unfollowCalls != 2 { + t.Fatalf("during backoff accepts=%d removals=%d, want 1 and 2", accepts, client.unfollowCalls) } time.Sleep(40 * time.Second) trigger <- struct{}{} synctest.Wait() - if accepts != 2 || client.removeCalls != 3 { - t.Fatalf("after backoff accepts=%d removals=%d, want 2 and 3", accepts, client.removeCalls) + if accepts != 2 || client.unfollowCalls != 3 { + t.Fatalf("after backoff accepts=%d removals=%d, want 2 and 3", accepts, client.unfollowCalls) } }) } @@ -296,6 +302,9 @@ func TestSocialSubscriptionFailureKeepsPolling(t *testing.T) { if accepts.Load() != 2 { t.Fatalf("polling accepts=%d after failed subscription, want 2", accepts.Load()) } + if got := sub.attempts.Load(); got < 2 { + t.Fatalf("subscribe attempts = %d after 20s, want retries", got) + } b.cancel() b.socialWg.Wait() if sub.cleanupCalls.Load() != 0 { @@ -303,3 +312,40 @@ func TestSocialSubscriptionFailureKeepsPolling(t *testing.T) { } }) } + +// A lost subscription must be replaced, releasing the old registration first. +func TestSubscribeSocialResubscribesAfterLoss(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + b := &Broadcaster{log: testBroadcasterLogger()} + b.ctx, b.cancel = context.WithCancel(t.Context()) + defer b.cancel() + sub := &fakeSocialSubscriber{subscribed: make(chan xblsocial.SubscriptionHandler, 1)} + trigger := b.subscribeSocial(sub, b.log) + synctest.Wait() + h := <-sub.subscribed + if len(trigger) != 0 { + t.Fatal("first subscription should not request an extra sync") + } + + h.HandleSubscriptionLost() + synctest.Wait() + if sub.cleanupCalls.Load() != 1 { + t.Fatalf("cleanup calls = %d, want the lost registration released", sub.cleanupCalls.Load()) + } + time.Sleep(socialResubscribeMinDelay) + synctest.Wait() + if got := sub.attempts.Load(); got != 2 { + t.Fatalf("subscribe attempts = %d, want 2", got) + } + select { + case <-trigger: + default: + t.Fatal("resubscribing should request a sync to catch up") + } + b.cancel() + b.socialWg.Wait() + if sub.cleanupCalls.Load() != 2 { + t.Fatalf("cleanup calls = %d, want 2 after shutdown", sub.cleanupCalls.Load()) + } + }) +}