From b104fd190a82351b5f11b91a9f3bccc7db3ec5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:50:55 +0200 Subject: [PATCH 01/10] Refactor state handling during the disconnect period. Introduce StateRecoveryStrategy to provide extra control for integrators. ReplayEvents - default, works as previously which is grab missed events and replay them one by one. BatchStateUpdate - applies all missed events without raising events, will raise a single StateRecovered events once all are processed. Disabled - skips recovery and allows integrator to handle lost state handling --- .../Core/Configs/IStreamClientConfig.cs | 8 + .../Core/Configs/StateRecoveryStrategy.cs | 64 +++ .../Configs/StateRecoveryStrategy.cs.meta | 2 + .../Core/Configs/StreamClientConfig.cs | 2 + .../StreamChat/Core/IStreamChatClient.cs | 27 ++ .../LowLevelClient/HistorySyncApplyResult.cs | 22 + .../HistorySyncApplyResult.cs.meta | 2 + .../StreamChatLowLevelClient.cs | 227 ++++++++-- .../StreamStateRecoveredEventArgs.cs | 48 ++ .../StreamStateRecoveredEventArgs.cs.meta | 2 + .../Core/State/StreamStatefulModelBase.cs | 11 + .../Core/StatefulModels/StreamChannel.cs | 112 +++-- .../Core/StatefulModels/StreamMessage.cs | 15 +- .../Core/StatefulModels/StreamThread.cs | 12 +- .../StreamChat/Core/StreamChatClient.cs | 422 +++++++++++++++++- .../StateSync/StateRecoveryClientTests.cs | 367 +++++++++++++++ .../StateRecoveryClientTests.cs.meta | 2 + .../StateSync/StateRecoveryLowLevelTests.cs | 326 ++++++++++++++ .../StateRecoveryLowLevelTests.cs.meta | 2 + 19 files changed, 1600 insertions(+), 73 deletions(-) create mode 100644 Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs create mode 100644 Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs.meta create mode 100644 Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs create mode 100644 Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs.meta create mode 100644 Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs create mode 100644 Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs.meta create mode 100644 Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs create mode 100644 Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs.meta create mode 100644 Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs create mode 100644 Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs.meta diff --git a/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs b/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs index 69887678..054bafcd 100644 --- a/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs +++ b/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs @@ -36,5 +36,13 @@ public interface IStreamClientConfig /// Does not change server history. See . /// MessageCacheWindow DefaultMessageCacheWindow { get; set; } + + /// + /// How the client restores local state after the websocket reconnects. Defaults to + /// , which preserves the per-event + /// callback behaviour of earlier SDK versions. See + /// for when to pick each option. + /// + StateRecoveryStrategy StateRecoveryStrategy { get; set; } } } \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs new file mode 100644 index 00000000..4029c10a --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs @@ -0,0 +1,64 @@ +namespace StreamChat.Core.Configs +{ + /// + /// How restores local state after the websocket reconnects. + /// Set through . + /// + /// + /// Regardless of the strategy, a reconnect always drops the server-side watches that were + /// established before the disconnect. and + /// re-establish them; leaves that to you. + /// + public enum StateRecoveryStrategy + { + /// + /// Default, and the behaviour of every SDK version before this option existed. + /// + /// The client calls /sync for the channels it was watching and replays each missed + /// event through the normal event pipeline, so every per-event callback + /// (, + /// , and so on) fires exactly as it + /// would for a live event. It then re-queries and re-watches those channels unconditionally, + /// which is new: previously a failed or skipped /sync left the channels stale and + /// unwatched for the rest of the connection. + /// + /// Choose this when your UI is driven by per-event callbacks. The cost is that a long outage + /// on a busy channel replays up to ~1000 events in a single frame, which is visible as a + /// hitch on mobile. + /// + ReplayEvents = 0, + + /// + /// Same recovery pipeline as , but the /sync events are + /// applied to local state without raising the per-event callbacks whose effect is observable + /// in model state afterwards. Subscribe to and + /// rebuild from and friends instead. + /// + /// Callbacks that carry information the SDK cannot reconstruct from state are still raised + /// per event: , + /// , and the local-user membership and invite + /// notifications. + /// + /// Choose this when a long outage causes a frame hitch on resume. This is the cheapest + /// recovery the SDK offers. + /// + BatchStateUpdate = 1, + + /// + /// The SDK performs no recovery after a reconnect: no /sync, no re-query, no re-watch, + /// and no . Local state is left exactly as it + /// was and is left untouched, so it remains + /// the list of what you were watching before the drop. + /// + /// Choose this only if you own recovery. Subscribe to + /// , and on the transition to + /// re-hydrate and re-watch yourself with + /// QueryChannelsAsync(new[] { ChannelFilter.Cid.In(cids) }, limit: 30) - that single + /// call both refreshes state and re-establishes the watches. Note that + /// is a no-op for a channel whose + /// is still true, which is the + /// case here, so use the query. + /// + Disabled = 2, + } +} diff --git a/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs.meta b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs.meta new file mode 100644 index 00000000..5ce3c44b --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: deb96db72983f9e4ca61a28cb444a7be \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Core/Configs/StreamClientConfig.cs b/Assets/Plugins/StreamChat/Core/Configs/StreamClientConfig.cs index 54b698f9..a48396d2 100644 --- a/Assets/Plugins/StreamChat/Core/Configs/StreamClientConfig.cs +++ b/Assets/Plugins/StreamChat/Core/Configs/StreamClientConfig.cs @@ -12,5 +12,7 @@ public class StreamClientConfig : IStreamClientConfig public bool OptimisticMessageInsert { get; set; } = true; public MessageCacheWindow DefaultMessageCacheWindow { get; set; } = null; + + public StateRecoveryStrategy StateRecoveryStrategy { get; set; } = Configs.StateRecoveryStrategy.ReplayEvents; } } \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs b/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs index e72e016b..e3c44f99 100644 --- a/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs @@ -33,6 +33,27 @@ public interface IStreamChatClient : IDisposable, IStreamChatClientEventsListene /// event Action Disconnected; + /// + /// Raised once after reconnect recovery finishes, on every path: full success, partial + /// success, and a reconnect where there was nothing to recover. Not raised on the initial + /// login, and not raised when + /// is + /// . + /// + /// When it fires, the channels in + /// have fresh state and live watches + /// again. Anything in is + /// still stale and no longer watched. + /// + /// This is the signal to rebuild from state after an outage. It is required rather than + /// merely convenient under + /// , where the per-event callbacks + /// are suppressed during recovery, and it is worth handling under + /// too, because the re-query that + /// follows the event replay merges channel state without raising per-message callbacks. + /// + event StateRecoveredHandler StateRecovered; + /// /// Event fired when connection state with Stream Chat server has changed /// @@ -129,6 +150,12 @@ public interface IStreamChatClient : IDisposable, IStreamChatClientEventsListene /// methods may not be watched - check on a specific /// channel to know its state. /// + /// + /// Emptied when the connection drops, because the server drops every watch with it, and + /// repopulated by reconnect recovery. If you enumerate this to decide what to restore + /// yourself, read it before the disconnect or use + /// , which leaves it untouched. + /// /// IReadOnlyList WatchedChannels { get; } diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs new file mode 100644 index 00000000..591cb5fb --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs @@ -0,0 +1,22 @@ +using System; + +namespace StreamChat.Core.LowLevelClient +{ + /// + /// Outcome of one silent /sync history batch. See + /// . + /// + internal sealed class HistorySyncApplyResult + { + /// + /// created_at of the newest event that was applied successfully, or null when + /// nothing was applied. This is what the /sync watermark advances to. + /// + public DateTimeOffset? MaxAppliedCreatedAt { get; set; } + + /// + /// Events that threw while being applied. They are skipped, not retried within the batch. + /// + public int FailedEventCount { get; set; } + } +} diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs.meta b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs.meta new file mode 100644 index 00000000..f131c2ae --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 59eb11f6ed758d54fa5b796cd4f8a5c7 \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs index 45a29261..a96f7e43 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs @@ -425,7 +425,7 @@ public void Update(float deltaTime) #if STREAM_DEBUG_ENABLED _logs.Info(_authCredentials.UserId + " WS message: " + msg); #endif - HandleNewWebsocketMessage(msg); + HandleNewWebsocketMessage(msg, isLiveEvent: true); } } @@ -443,44 +443,172 @@ public void SetReconnectStrategySettings(ReconnectStrategy reconnectStrategy, fl public async Task FetchAndProcessEventsSinceLastReceivedEvent(IEnumerable channelCids) { - if (!channelCids.Any() || !_disconnectionLastEventReceivedAt.HasValue) + var response = await TrySyncHistoryAsync(channelCids); + ReplayHistoryEvents(response?.Events); + } + + /// + /// The /sync endpoint counts events summed across every requested cid against a + /// server-side ceiling of roughly 1000 and refuses the whole request once it is exceeded, so + /// asking about fewer channels makes a successful catch-up more likely. Swift and Android both + /// cap the request at 100 cids; this matches them. + /// + internal const int MaxSyncChannelCids = 100; + + /// + /// Best-effort /sync for up to channels. Returns + /// null when catch-up is skipped, which happens when there is no sync point to catch up + /// from or when the sync point is older than the 30 days the server accepts. + /// + internal async Task TrySyncHistoryAsync(IEnumerable channelCids) + { + if (channelCids == null || !_disconnectionLastEventReceivedAt.HasValue) { - return; + return null; } var lastEventReceivedAt = _disconnectionLastEventReceivedAt.Value; - // Check if less than 30 days. Past that the server rejects LastSyncAt, so this only - // skips a request that was certain to fail — it is not a recovery path. The SDK has no - // re-hydrate fallback of its own, so bridging a gap this large is the consumer's job. - TimeSpan diff = _timeService.Now - lastEventReceivedAt; - if (diff.TotalDays > 30) + if ((_timeService.Now - lastEventReceivedAt).TotalDays > 30) + { + return null; + } + + // Released only once the request has completed: the request body is serialized from this + // list, and an auth retry re-serializes it after the first attempt has already awaited. + using (new ListPoolScope(out var cids)) + { + foreach (var cid in channelCids) + { + if (cids.Count == MaxSyncChannelCids) + { + break; + } + + cids.Add(cid); + } + + if (cids.Count == 0) + { + return null; + } + + return await ChannelApi.SyncAsync(new SyncRequest + { + ChannelCids = cids, + LastSyncAt = lastEventReceivedAt, + Watch = true, + + // Lets the caller tell "the server will never return this channel again" apart from + // "the query happened to omit it", so recovery can stop retrying channels that were + // deleted or that the local user lost access to while offline. + WithInaccessibleCids = true, + }); + } + } + + /// + /// Replay history events through the live event pipeline, so every per-event public callback + /// fires exactly as it would for a real-time event. This is + /// and the behaviour of + /// . + /// + internal void ReplayHistoryEvents(IEnumerable events) + { + if (events == null) { return; } - //StreamTodo: according to Android SDK there's an error if there are > 1000 events + foreach (var e in events) + { + // Each event is isolated by the try/catch inside the registered handler, so one + // malformed event cannot abandon the rest of the replay. + HandleNewWebsocketMessage(SerializeHistoryEvent(e)); + } + } - var response = await ChannelApi.SyncAsync(new SyncRequest + /// + /// Apply history events to local state without raising the per-event public callbacks whose + /// effect is observable in model state afterwards. This is + /// . Mirrors Android's + /// isFromHistorySync and Swift's postNotifications: false. + /// + internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) + { + var result = new HistorySyncApplyResult(); + if (events == null) { - ChannelCids = channelCids.ToList(), - LastSyncAt = lastEventReceivedAt, - Watch = true, - }); + return result; + } + + _isApplyingHistoryEvents = true; + _historyMaxAppliedCreatedAt = null; - if (response.Events.Count == 0) + try + { + foreach (var e in events) + { + try + { + HandleNewWebsocketMessage(SerializeHistoryEvent(e)); + } + catch (Exception ex) + { + // Only count and log. Abandoning the batch would leave state half-applied, + // and the watermark below only ever advances to the newest event that was + // actually applied, so a partial batch is retried on the next reconnect. + result.FailedEventCount++; + _logs.Exception(ex); + } + } + } + finally + { + result.MaxAppliedCreatedAt = _historyMaxAppliedCreatedAt; + _historyMaxAppliedCreatedAt = null; + _isApplyingHistoryEvents = false; + + if (result.MaxAppliedCreatedAt.HasValue) + { + TryAdvanceLastEventReceivedAt(result.MaxAppliedCreatedAt.Value, HistorySyncWatermarkSource); + } + } + + return result; + } + + /// + /// True while is running. + /// + internal bool IsApplyingHistoryEvents => _isApplyingHistoryEvents; + + private const string HistorySyncWatermarkSource = "history.sync"; + + // The batch advances the watermark once, at the end, and only to the newest event it managed + // to apply. Advancing per event would let a throwing event in the middle leave a watermark + // claiming a catch-up that did not happen. + private void RecordHistoryWatermark(DateTimeOffset createdAt) + { + if (createdAt == DateTimeOffset.MinValue) { return; } - foreach (var e in response.Events) + if (!_historyMaxAppliedCreatedAt.HasValue || createdAt > _historyMaxAppliedCreatedAt.Value) { - // StreamTodo: check if we can not serialized this again. Investigate adding a custom EventsJsonConverter that would populate the list as serialized strings - var serializedMsg = _serializer.Serialize(e); + _historyMaxAppliedCreatedAt = createdAt; + } + } - //StreamTodo: try block? - HandleNewWebsocketMessage(serializedMsg); + private string SerializeHistoryEvent(object e) + { + if (e is string serialized) + { + return serialized; } + + return _serializer.Serialize(e); } public void Dispose() @@ -615,6 +743,9 @@ internal async Task ConnectUserAsync(string apiKey, string u /// private DateTimeOffset? _disconnectionLastEventReceivedAt; + private bool _isApplyingHistoryEvents; + private DateTimeOffset? _historyMaxAppliedCreatedAt; + private async Task RefreshAuthTokenFromProvider() { #if STREAM_DEBUG_ENABLED @@ -996,13 +1127,34 @@ private void RegisterEventType(string key, #endif var eventObj = DeserializeEvent(serializedContent, out var dto); postprocess?.Invoke(dto); - TryAdvanceLastEventReceivedAt(eventObj.CreatedAt, key); - handler?.Invoke(eventObj, dto); + + if (_isApplyingHistoryEvents) + { + RecordHistoryWatermark(eventObj.CreatedAt); + } + else + { + TryAdvanceLastEventReceivedAt(eventObj.CreatedAt, key); + + // The low-level client is event-only, so it has no state a consumer could read + // after a silent batch. Suppressing its callbacks keeps a low-level subscriber + // consistent with the stateful client's BatchStateUpdate contract. + handler?.Invoke(eventObj, dto); + } + + // Always applied - this is what mutates local state. internalHandler?.Invoke(dto); } catch (Exception e) { _logs.Exception(e); + + // A silent batch counts its failures and advances the watermark only to the newest + // event it applied, so it needs to see the throw. Live events stay isolated here. + if (_isApplyingHistoryEvents) + { + throw; + } } }); } @@ -1025,7 +1177,7 @@ private TEvent DeserializeEvent(string content, out TDto dto) return response; } - private void HandleNewWebsocketMessage(string msg) + private void HandleNewWebsocketMessage(string msg, bool isLiveEvent = false) { const string ErrorKey = "error"; @@ -1046,7 +1198,16 @@ private void HandleNewWebsocketMessage(string msg) return; } - if (EventReceived != null) + // Stamp liveness here rather than from the health check handler: the handler runs after + // every consumer callback registered ahead of it, so a slow consumer could push the gap + // past HealthCheckMaxWaitingTime and make the client disconnect itself. Only events that + // came off the live socket count - a health check replayed from /sync proves nothing. + if (isLiveEvent && type == WSEventType.HealthCheck) + { + _lastHealthCheckReceivedTime = _timeService.Time; + } + + if (EventReceived != null && !_isApplyingHistoryEvents) { var time = DateTime.Now.TimeOfDay.ToString(@"hh\:mm\:ss"); EventReceived.Invoke($"{time} - Event received: {type}"); @@ -1081,8 +1242,22 @@ private bool TryHandleCustomChannelEvent(string serializedContent, string eventT try { var dto = _serializer.Deserialize(serializedContent); - TryAdvanceLastEventReceivedAt(dto.CreatedAt, eventType); + if (_isApplyingHistoryEvents) + { + RecordHistoryWatermark(dto.CreatedAt); + } + else + { + TryAdvanceLastEventReceivedAt(dto.CreatedAt, eventType); + } + + // Custom events are the one category with no representation in local state, so a + // consumer cannot reconstruct them from IStreamChannel after a silent batch. They are + // therefore delivered per event even during history sync - dropping them would be + // silent data loss, and deferring them into the recovery signal would arrive after + // the re-query and out of chronological order. The reference SDKs discard custom + // events from /sync entirely; an app porting between SDKs must not rely on this. var evt = new EventCustom(); ((ILoadableFrom)evt).LoadFromDto(dto); CustomEventReceived?.Invoke(evt); @@ -1133,8 +1308,6 @@ private void PingHealthCheck() private void HandleHealthCheckEvent(EventHealthCheck healthCheckEvent, HealthCheckEventInternalDTO dto) { - _lastHealthCheckReceivedTime = _timeService.Time; - if (ConnectionState == ConnectionState.Connecting) { OnConnectionConfirmed(healthCheckEvent, dto); diff --git a/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs new file mode 100644 index 00000000..b683b6d4 --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs @@ -0,0 +1,48 @@ +using System; +using System.Collections.Generic; +using StreamChat.Core.StatefulModels; + +namespace StreamChat.Core.Responses +{ + /// + /// Payload for . + /// + public sealed class StreamStateRecoveredEventArgs + { + public StreamStateRecoveredEventArgs(IReadOnlyList channels, + IReadOnlyList unrecoveredChannelCids) + { + Channels = channels ?? Array.Empty(); + UnrecoveredChannelCids = unrecoveredChannelCids ?? Array.Empty(); + } + + /// + /// Channels whose state was refreshed and whose watch was re-established. Their + /// and other collections are up to date as of the + /// moment this event is raised. + /// + /// Note that the recovery query returns the channel's latest page of messages and merges it + /// into what was already loaded. If more messages arrived during the outage than fit in one + /// page, the list contains the pre-disconnect messages followed by the latest page with a + /// hole in between, and cannot reach into + /// that hole because it pages back from the oldest loaded message. + /// + public IReadOnlyList Channels { get; } + + /// + /// Channels that were being watched before the disconnect but could not be recovered - the + /// server no longer returns them (deleted, or the local user lost access while offline), or + /// every attempt to re-query them failed. Their local state is still stale and they are no + /// longer watched, so they will not receive realtime updates. + /// + /// Empty on a fully successful recovery. Use it to tear down or flag the corresponding UI + /// rather than leaving it silently frozen. + /// + public IReadOnlyList UnrecoveredChannelCids { get; } + + /// + /// true when every channel that was being watched before the disconnect was recovered. + /// + public bool IsComplete => UnrecoveredChannelCids.Count == 0; + } +} diff --git a/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs.meta b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs.meta new file mode 100644 index 00000000..ed989ec5 --- /dev/null +++ b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 343e17afd03f6814b9154f5e482fbfcb \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs b/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs index bc7e6e67..676c162e 100644 --- a/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs +++ b/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs @@ -51,6 +51,17 @@ internal StreamStatefulModelBase(string uniqueId, ICacheRepository Repository { get; } + /// + /// True while a /sync history batch is being applied under + /// . State mutations still run; + /// public per-event notifications must not, when their effect is observable in model state once + /// the batch finishes. Consumers observe instead. + /// + /// Notifications carrying information the SDK cannot reconstruct from state - custom events, + /// channel deletion, local-user membership and invite notifications - are raised regardless. + /// + protected bool IsSilentHistorySync => Client.IsApplyingHistorySync; + protected void LoadAdditionalProperties(Dictionary additionalProperties) { //StreamTodo: investigate if there's a case we don't want to clear here diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs index a1bba234..c1054cfb 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs @@ -116,7 +116,7 @@ public bool Hidden get => _hidden; internal set { - if (TrySet(ref _hidden, value)) + if (TrySet(ref _hidden, value) && !IsSilentHistorySync) { VisibilityChanged?.Invoke(this, Hidden); } @@ -148,7 +148,10 @@ internal set } _muted = value; - MuteChanged?.Invoke(this, value); + if (!IsSilentHistorySync) + { + MuteChanged?.Invoke(this, value); + } } } @@ -873,7 +876,10 @@ internal void HandleMessageUpdatedEvent(MessageUpdatedEventInternalDTO dto) } message.TryUpdateFromDto(dto.Message, Cache); - MessageUpdated?.Invoke(this, message); + if (!IsSilentHistorySync) + { + MessageUpdated?.Invoke(this, message); + } } internal void HandleMessageDeletedEvent(MessageDeletedEventInternalDTO dto) @@ -898,7 +904,10 @@ internal void HandleMessageDeletedEvent(MessageDeletedEventInternalDTO dto) message.InternalHandleSoftDelete(); } - MessageDeleted?.Invoke(this, message, isHardDelete); + if (!IsSilentHistorySync) + { + MessageDeleted?.Invoke(this, message, isHardDelete); + } } internal void HandleChannelUpdatedEvent(ChannelUpdatedEventInternalDTO eventDto) @@ -908,7 +917,10 @@ internal void HandleChannelUpdatedEvent(ChannelUpdatedEventInternalDTO eventDto) UpdateChannelFieldsFromDtoOverwrite(eventDto.Channel, Cache); MemberCount = eventDto.ChannelMemberCount; - Updated?.Invoke(this); + if (!IsSilentHistorySync) + { + Updated?.Invoke(this); + } } internal void HandleChannelTruncatedEvent(ChannelTruncatedEventInternalDTO eventDto) @@ -933,8 +945,11 @@ internal void InternalAddMember(StreamChannelMember member) } _members.Add(member); - MemberAdded?.Invoke(this, member); - MembersChanged?.Invoke(this, member, OperationType.Added); + if (!IsSilentHistorySync) + { + MemberAdded?.Invoke(this, member); + MembersChanged?.Invoke(this, member, OperationType.Added); + } } internal void InternalRemoveMember(StreamChannelMember member) @@ -945,8 +960,11 @@ internal void InternalRemoveMember(StreamChannelMember member) } _members.Remove(member); - MemberRemoved?.Invoke(this, member); - MembersChanged?.Invoke(this, member, OperationType.Removed); + if (!IsSilentHistorySync) + { + MemberRemoved?.Invoke(this, member); + MembersChanged?.Invoke(this, member, OperationType.Removed); + } } internal void InternalUpdateMember(StreamChannelMember member) @@ -956,8 +974,11 @@ internal void InternalUpdateMember(StreamChannelMember member) _members.Add(member); } - MemberUpdated?.Invoke(this, member); - MembersChanged?.Invoke(this, member, OperationType.Updated); + if (!IsSilentHistorySync) + { + MemberUpdated?.Invoke(this, member); + MembersChanged?.Invoke(this, member, OperationType.Updated); + } } protected override StreamChannel Self => this; @@ -1037,37 +1058,42 @@ private bool InternalAppendOrUpdateMessage(MessageInternalDTO dto, out StreamMes _messages.Sort(MessageCreatedAtComparer.Instance); } - MessageReceived?.Invoke(this, streamMessage); + if (!IsSilentHistorySync) + { + MessageReceived?.Invoke(this, streamMessage); + } - // Trim after MessageReceived so a message is never removed from cache before it is received. + // Trim after MessageReceived so a message is never removed from cache before it is + // received. Trimming still runs during a silent history batch: it only ever removes the + // oldest contiguous prefix, which the batch is appending newer messages ahead of, and + // skipping it would let a batch push a windowed channel past its MaxMessages. TrimMessageCacheIfNeeded(); return true; } + /// + /// A query response merges through UpdateFromDto, which appends to + /// without going through + /// and therefore never trims. Reconnect recovery calls this after its merge. + /// + internal void InternalTrimMessageCache() => TrimMessageCacheIfNeeded(); + //StreamTodo: This deleteBeforeCreatedAt date is the date of event, it does not equal the passed TruncatedAt //Therefore the only way to detect partial truncate in the past would be to query the history private void InternalTruncateMessages(DateTimeOffset? deleteBeforeCreatedAt = null, MessageInternalDTO systemMessageDto = null) { - if (deleteBeforeCreatedAt.HasValue) + for (int i = _messages.Count - 1; i >= 0; i--) { - for (int i = _messages.Count - 1; i >= 0; i--) + var msg = _messages[i]; + if (deleteBeforeCreatedAt.HasValue && msg.CreatedAt >= deleteBeforeCreatedAt) { - var msg = _messages[i]; - if (msg.CreatedAt < deleteBeforeCreatedAt) - { - _messages.RemoveAt(i); - Cache.Messages.Remove(msg); - } - } - } - else - { - for (int i = _messages.Count - 1; i >= 0; i--) - { - _messages.RemoveAt(i); - Cache.Messages.Remove(_messages[i]); + continue; } + + _messages.RemoveAt(i); + _pinnedMessages.Remove(msg); + Cache.Messages.Remove(msg); } if (systemMessageDto != null) @@ -1075,7 +1101,10 @@ private void InternalTruncateMessages(DateTimeOffset? deleteBeforeCreatedAt = nu InternalAppendOrUpdateMessage(systemMessageDto, out _); } - Truncated?.Invoke(this); + if (!IsSilentHistorySync) + { + Truncated?.Invoke(this); + } } private void TrimMessageCacheIfNeeded() @@ -1380,17 +1409,34 @@ internal void InternalHandleCustomEvent(CustomEventInternalDTO dto) var customEvent = new StreamCustomEvent(dto.Type, user, dto.CreatedAt, new StreamCustomData(custom, Serializer)); + // Deliberately not gated on IsSilentHistorySync: a custom event has no representation in + // channel state, so suppressing it would lose the payload with no way to recover it. CustomEventReceived?.Invoke(this, customEvent); } internal void InternalNotifyReactionReceived(StreamMessage message, StreamReaction reaction) - => ReactionAdded?.Invoke(this, message, reaction); + { + if (!IsSilentHistorySync) + { + ReactionAdded?.Invoke(this, message, reaction); + } + } internal void InternalNotifyReactionUpdated(StreamMessage message, StreamReaction reaction) - => ReactionUpdated?.Invoke(this, message, reaction); + { + if (!IsSilentHistorySync) + { + ReactionUpdated?.Invoke(this, message, reaction); + } + } public void InternalNotifyReactionDeleted(StreamMessage message, StreamReaction reaction) - => ReactionRemoved?.Invoke(this, message, reaction); + { + if (!IsSilentHistorySync) + { + ReactionRemoved?.Invoke(this, message, reaction); + } + } //StreamTodo: implement some timeout for typing users in case we dont' receive, this could be configurable private readonly List _typingUsers = new List(); diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamMessage.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamMessage.cs index 5e5e4382..32ecdd36 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamMessage.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamMessage.cs @@ -409,7 +409,10 @@ internal void HandleReactionNewEvent(ReactionNewEventInternalDTO eventDto, Strea //StreamTodo: verify if this how we should update the message + what about events for customer to get notified Cache.TryCreateOrUpdate(eventDto.Message); - ReactionAdded?.Invoke(channel, this, reaction); + if (!IsSilentHistorySync) + { + ReactionAdded?.Invoke(channel, this, reaction); + } } internal void HandleReactionUpdatedEvent(ReactionUpdatedEventInternalDTO eventDto, StreamChannel channel, StreamReaction reaction) @@ -422,7 +425,10 @@ internal void HandleReactionUpdatedEvent(ReactionUpdatedEventInternalDTO eventDt eventDto.Message.OwnReactions = null; Cache.TryCreateOrUpdate(eventDto.Message); - ReactionUpdated?.Invoke(channel, this, reaction); + if (!IsSilentHistorySync) + { + ReactionUpdated?.Invoke(channel, this, reaction); + } } internal void HandleReactionDeletedEvent(ReactionDeletedEventInternalDTO eventDto, StreamChannel channel, StreamReaction reaction) @@ -435,7 +441,10 @@ internal void HandleReactionDeletedEvent(ReactionDeletedEventInternalDTO eventDt eventDto.Message.OwnReactions = null; Cache.TryCreateOrUpdate(eventDto.Message); - ReactionRemoved?.Invoke(channel, this, reaction); + if (!IsSilentHistorySync) + { + ReactionRemoved?.Invoke(channel, this, reaction); + } } protected override StreamMessage Self => this; diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs index f562e38a..b659dd81 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs @@ -427,7 +427,10 @@ internal void HandleMarkReadByUser(string userId, DateTimeOffset createdAt) } } - ReadStateChanged?.Invoke(this); + if (!IsSilentHistorySync) + { + ReadStateChanged?.Invoke(this); + } } // Mirrors Android's Thread.markAsUnreadByUser. notification.mark_unread carries no @@ -452,7 +455,10 @@ internal void HandleMarkUnreadByUser(string userId, DateTimeOffset? lastReadAt) } } - ReadStateChanged?.Invoke(this); + if (!IsSilentHistorySync) + { + ReadStateChanged?.Invoke(this); + } } protected override string InternalUniqueId @@ -521,7 +527,7 @@ private void IncrementUnreadForOtherReaders(StreamMessage reply) } } - if (anyChanged) + if (anyChanged && !IsSilentHistorySync) { ReadStateChanged?.Invoke(this); } diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index cec74b01..d753d87e 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -67,6 +67,9 @@ namespace StreamChat.Core /// public delegate void ChannelMemberRemovedHandler(IStreamChannel channel, IStreamChannelMember member); + /// + public delegate void StateRecoveredHandler(StreamStateRecoveredEventArgs eventArgs); + /// public sealed class StreamChatClient : IStreamChatClient { @@ -90,6 +93,8 @@ public sealed class StreamChatClient : IStreamChatClient public event StreamThreadChangeHandler ThreadTracked; public event StreamThreadChangeHandler ThreadUntracked; + public event StateRecoveredHandler StateRecovered; + public const int QueryUsersLimitMaxValue = 30; public const int QueryUsersOffsetMaxValue = 1000; @@ -222,6 +227,11 @@ var ownUserDto public Task DisconnectUserAsync() { TryCancelWaitingForUserConnection(); + + // Ends the session, so the next Connected transition is a fresh login rather than a + // reconnect and must not run recovery or raise StateRecovered. + _hasConnectedBefore = false; + return InternalLowLevelClient.DisconnectAsync(permanent: true); } @@ -967,6 +977,43 @@ internal Task RefreshChannelState(string cid) private readonly StreamPollsApi _pollsApi; private readonly List _watchedChannels = new List(); + /// + /// Cids that were being watched when the connection dropped, most recently active first. + /// Reconnect recovery restores state and watches from this, not from + /// , which is cleared on the disconnect. + /// + private readonly List _recoveryChannelCids = new List(); + + /// + /// Cids the /sync response reported as inaccessible - deleted, or no longer readable by + /// the local user. Never re-queried again for the lifetime of the client. + /// + private readonly HashSet _inaccessibleCids = new HashSet(); + + private int _recoveryGeneration; + private bool _hasConnectedBefore; + + /// + /// Recovering more channels than this would mean an unbounded number of sequential queries on + /// every reconnect, which invites rate limiting. Matches the /sync cid cap so both + /// halves of recovery cover the same set. JS and Android both cap lower, at 30. + /// + internal const int MaxRecoveredChannels = StreamChatLowLevelClient.MaxSyncChannelCids; + + /// + /// QueryChannelsAsync asserts a limit of at most 30, so a longer recovery set has to be + /// chunked. Same chunk size as . + /// + private const int MaxChannelsPerRecoveryQuery = 30; + + // Ties are broken arbitrarily - List.Sort is unstable - which only matters for channels with + // an equal LastMessageAt, where there is no meaningful "more recently active" anyway. + private static readonly Comparison ByLastMessageAtDescending = (a, b) + => (b.LastMessageAt ?? DateTimeOffset.MinValue).CompareTo(a.LastMessageAt ?? DateTimeOffset.MinValue); + + /// + internal bool IsApplyingHistorySync => InternalLowLevelClient.IsApplyingHistoryEvents; + private TaskCompletionSource _connectUserTaskSource; private CancellationToken _connectUserCancellationToken; private CancellationTokenSource _connectUserCancellationTokenSource; @@ -1041,7 +1088,16 @@ private void MarkChannelWatched(StreamChannel channel) // after the server confirms the unwatch. Idempotent. internal void InternalMarkChannelUnwatched(StreamChannel channel) { - if (channel == null || !channel.IsWatched) + if (channel == null) + { + return; + } + + // Drop it from the recovery snapshot as well, or an unwatch performed while disconnected + // would be undone by the next reconnect re-watching it. + _recoveryChannelCids.Remove(channel.Cid); + + if (!channel.IsWatched) { return; } @@ -1050,7 +1106,11 @@ internal void InternalMarkChannelUnwatched(StreamChannel channel) _watchedChannels.Remove(channel); } - private void OnChannelLeftCache(StreamChannel channel) => _watchedChannels.Remove(channel); + private void OnChannelLeftCache(StreamChannel channel) + { + _watchedChannels.Remove(channel); + _recoveryChannelCids.Remove(channel.Cid); + } private void TryCancelWaitingForUserConnection() { @@ -1143,20 +1203,341 @@ private void OnConnected(HealthCheckEventInternalDTO dto) RestoreStateLostDuringDisconnect().LogIfFailed(); } - private Task RestoreStateLostDuringDisconnect() + /// + /// Watches are bound to a websocket connection, and a reconnect always gets a new one, so a + /// reconnected client is watching nothing until something re-watches for it. Without this the + /// channels stay in local state but stop receiving events - the chat looks alive and silently + /// never updates again. + /// + /// This holds no matter how briefly the socket was down. The handshake payload + /// (ConnectPayload) carries only the user and token plus + /// server_determines_connection_id; there is no session or resume token, so the client + /// cannot ask the server to continue a previous connection, and the server mints a fresh + /// connection_id that every subsequent request is then tagged with. Do not confuse this + /// with the server-side health check grace period: that governs when the server notices a + /// silently dropped socket, which affects presence and the cleanup of the stale watcher entry. + /// It does not hand the old connection's watches to the new one - if anything a fast reconnect + /// is the worse case, because for a while the channel counts you as a watcher twice while the + /// connection you are actually reading receives nothing. + /// + /// Runs after every reconnect, in this order: + /// + /// 1. /sync catch-up, best effort. This is what makes a short outage recover with no + /// hole in the message list. It has to run first because a replayed channel.truncated + /// wipes the local message list, and doing that after step 2 would discard the page step 2 + /// just fetched. + /// 2. Re-query and re-watch, unconditionally, whatever step 1 did. One query per 30 cids, with + /// State and Watch set, so a single request both re-hydrates and re-watches. + /// 3. Raise once. + /// + /// Steps 1 and 2 are individually fault-tolerant: a failure in one channel or one request must + /// not abandon the others, because this is the only recovery this reconnect gets. + /// + private async Task RestoreStateLostDuringDisconnect() { - if (!WatchedChannels.Any()) + // A fresh login is not a recovery: there is no prior state to restore and no consumer + // expects a recovery signal for it. Anything left in the snapshot belongs to the previous + // session, and possibly to a different user, so drop it. + if (!_hasConnectedBefore) { - return Task.CompletedTask; + _hasConnectedBefore = true; + _recoveryChannelCids.Clear(); + _inaccessibleCids.Clear(); + return; + } + + if (InternalLowLevelClient.Config.StateRecoveryStrategy == StateRecoveryStrategy.Disabled) + { + return; + } + + var generation = ++_recoveryGeneration; + + // Pooled - never leaves this method and the steps below only read it. The two collections + // handed to StateRecovered are not pooled, because subscribers keep them for as long as + // they like. + using (new ListPoolScope(out var recoverSet)) + { + FillRecoverySet(recoverSet); + + var refreshedChannels = new List(); + + try + { + if (recoverSet.Count > 0) + { + await TryCatchUpWithHistoryAsync(recoverSet, generation); + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + + await RehydrateAndRewatchChannelsAsync(recoverSet, generation, refreshedChannels); + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + } + } + catch (Exception e) + { + // Defence in depth - every step already handles its own failures. Whatever happened, + // the consumer still gets told that recovery finished and which channels are stale. + _logs.Exception(e); + } + + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + + var unrecovered = new List(); + + using (new HashSetPoolScope(out var recovered)) + { + for (var i = 0; i < refreshedChannels.Count; i++) + { + recovered.Add(refreshedChannels[i].Cid); + } + + for (var i = 0; i < recoverSet.Count; i++) + { + if (!recovered.Contains(recoverSet[i])) + { + unrecovered.Add(recoverSet[i]); + } + } + } + + if (unrecovered.Count > 0) + { + _logs.Warning( + $"Reconnect recovery could not restore {unrecovered.Count} channel(s): {string.Join(", ", unrecovered)}. " + + "Their local state is stale and they are no longer watched. See " + + nameof(StreamStateRecoveredEventArgs) + "." + nameof(StreamStateRecoveredEventArgs.UnrecoveredChannelCids)); + } + + StateRecovered?.Invoke(new StreamStateRecoveredEventArgs(refreshedChannels, unrecovered)); + } + } + + /// + /// Copy the most recently active cids from the snapshot + /// captured on the disconnect into . + /// + private void FillRecoverySet(List recoverSet) + { + if (_recoveryChannelCids.Count > MaxRecoveredChannels) + { + _logs.Warning( + $"{_recoveryChannelCids.Count} channels were being watched when the connection dropped, but reconnect " + + $"recovery restores at most {MaxRecoveredChannels}. The {MaxRecoveredChannels} most recently active are " + + "recovered; the rest keep stale state and are not re-watched. Watch fewer channels concurrently, or set " + + nameof(IStreamClientConfig) + "." + nameof(IStreamClientConfig.StateRecoveryStrategy) + " to " + + nameof(StateRecoveryStrategy.Disabled) + " and recover them yourself."); + } + + var count = Math.Min(_recoveryChannelCids.Count, MaxRecoveredChannels); + for (var i = 0; i < count; i++) + { + recoverSet.Add(_recoveryChannelCids[i]); + } + } + + private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, int generation) + { + try + { + var response = await InternalLowLevelClient.TrySyncHistoryAsync(recoverSet); + + // null means the catch-up was skipped: no sync point, or one older than the 30 days + // the server accepts. Both used to return before any recovery ran; now step 2 still + // runs, which is the whole point of making it unconditional. + if (response?.Events == null || response.Events.Count == 0) + { + return; + } + + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + + if (response.InaccessibleCids != null) + { + // The server is telling us these will never come back. Recording them keeps the + // re-query from asking about deleted channels and stops us reporting them as a + // recovery failure every reconnect. + foreach (var cid in response.InaccessibleCids) + { + _inaccessibleCids.Add(cid); + } + } + + if (InternalLowLevelClient.Config.StateRecoveryStrategy == StateRecoveryStrategy.BatchStateUpdate) + { + InternalLowLevelClient.ApplyHistoryEvents(response.Events); + } + else + { + InternalLowLevelClient.ReplayHistoryEvents(response.Events); + } + } + catch (StreamApiException ex) when (ex.IsInputError()) + { + // HTTP 400 / code 4, "too many events to sync". The server counts events summed across + // every requested cid against a ceiling of roughly 1000 and refuses the whole request, + // so this is the normal outcome of a long outage on busy channels, not an anomaly. + // The re-query below is the fallback and recovers the same state minus the events that + // did not fit in the latest page. + _logs.Warning("The /sync catch-up was refused because too many events accumulated during the outage. " + + "Recovering channel state with a re-query instead. " + ex.Message); + } + catch (Exception ex) + { + _logs.Warning("The /sync catch-up failed. Recovering channel state with a re-query instead. " + + ex.Message); + } + } + + /// + /// Re-hydrate and re-watch in one request per cids. + /// + /// + /// Unlike Android, this does not follow up with a per-channel re-watch for cids the query did + /// not return. Android needs that because it recovers through the customer's own channel-list + /// queries, which need not cover every active cid; this queries the recovery set by cid, so it + /// is exhaustive by construction. A cid the query omits is one the server will not return at + /// all - deleted, or no longer readable - and the only per-channel watch primitive available + /// is get-or-create, which would recreate a channel that was deleted while we were offline. + /// Such cids are reported through + /// instead. + /// + private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recoverSet, int generation, + List refreshed) + { + var sort = ChannelSort.OrderByDescending(ChannelSortFieldName.LastMessageAt); + + for (var i = 0; i < recoverSet.Count; i += MaxChannelsPerRecoveryQuery) + { + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + + // Released only once the query has completed: the filter holds this list and the + // request body is serialized from it. + using (new ListPoolScope(out var chunk)) + { + var chunkEnd = Math.Min(i + MaxChannelsPerRecoveryQuery, recoverSet.Count); + for (var j = i; j < chunkEnd; j++) + { + if (!_inaccessibleCids.Contains(recoverSet[j])) + { + chunk.Add(recoverSet[j]); + } + } + + if (chunk.Count == 0) + { + continue; + } + + var filters = new IFieldFilterRule[] + { + ChannelFilter.Cid.In(chunk), + }; + + IEnumerable channels; + try + { + channels = await QueryChannelsAsync(filters, sort, limit: chunk.Count); + } + catch (Exception e) + { + // One failed chunk (a rate limit part-way through a long watch list, a channel + // torn down while offline) must not cost the remaining chunks their recovery - + // there is no later retry this connection. + _logs.Warning($"Recovery query failed for {chunk.Count} channel(s). Continuing with the rest. " + + e.Message); + continue; + } + + if (!IsRecoveryGenerationCurrent(generation)) + { + return; + } + + foreach (var channel in channels) + { + // The query merge path goes through UpdateFromDto, which does not trim, so a + // recovery merge can push Messages past MessageCacheWindow.MaxMessages. + ((StreamChannel)channel).InternalTrimMessageCache(); + refreshed.Add(channel); + } + } + } + } + + private bool IsRecoveryGenerationCurrent(int generation) => generation == _recoveryGeneration; + + /// + /// Capture what was being watched when the connection dropped, then stop claiming those + /// watches: the server has dropped them, so would + /// otherwise report watches that no longer exist. Recovery restores both from the snapshot. + /// + private void SnapshotRecoverySetAndClearWatches() + { + // A reconnect attempt that fails transitions Connecting -> Disconnected again, and by then + // the watch list is already empty. Overwriting the snapshot at that point would throw away + // the only record of what needs recovering, and the reconnect that eventually succeeds + // would restore nothing at all - which is exactly the flaky-mobile-network case. + if (_watchedChannels.Count == 0) + { + return; + } + + _recoveryChannelCids.Clear(); + + using (new ListPoolScope(out var ordered)) + { + ordered.AddRange(_watchedChannels); + ordered.Sort(ByLastMessageAtDescending); + + for (var i = 0; i < ordered.Count; i++) + { + _recoveryChannelCids.Add(ordered[i].Cid); + } + } + + for (var i = 0; i < _watchedChannels.Count; i++) + { + ((StreamChannel)_watchedChannels[i]).IsWatched = false; } - return LowLevelClient.FetchAndProcessEventsSinceLastReceivedEvent(WatchedChannels.Select(c => c.Cid)); + _watchedChannels.Clear(); } private void OnDisconnected() => Disconnected?.Invoke(); private void OnConnectionStateChanged(ConnectionState previous, ConnectionState current) - => ConnectionStateChanged?.Invoke(previous, current); + { + if (current == ConnectionState.Disconnected) + { + // Supersede any recovery still in flight before its responses can land on top of the + // state the next recovery is about to fetch. Some channel fields (read state, members, + // pinned messages) are replaced wholesale by a query response rather than merged, so a + // late response is not merely redundant, it can overwrite newer state. + _recoveryGeneration++; + + if (InternalLowLevelClient.Config.StateRecoveryStrategy != StateRecoveryStrategy.Disabled) + { + SnapshotRecoverySetAndClearWatches(); + } + } + + ConnectionStateChanged?.Invoke(previous, current); + } private void OnMessageDeleted(MessageDeletedEventInternalDTO eventMessageDeleted) { @@ -1606,16 +1987,29 @@ var reaction } } + // Who is currently watching is live presence, like typing: replaying it would leave watchers + // listed who left during the outage. The recovery query returns the authoritative watcher set. private void OnUserWatchingStop(UserWatchingStopEventInternalDTO eventDto) { + if (IsApplyingHistorySync) + { + return; + } + if (_cache.Channels.TryGet(eventDto.Cid, out var streamChannel)) { streamChannel.InternalHandleUserWatchingStop(eventDto); } } + /// private void OnUserWatchingStart(UserWatchingStartEventInternalDTO eventDto) { + if (IsApplyingHistorySync) + { + return; + } + if (_cache.Channels.TryGet(eventDto.Cid, out var streamChannel)) { streamChannel.InternalHandleUserWatchingStartEvent(eventDto); @@ -1655,14 +2049,28 @@ private void OnUserPresenceChanged(UserPresenceChangedEventInternalDTO eventDto) private void OnTypingStopped(TypingStopEventInternalDTO eventDto) { + // Typing is live presence with no meaning in a history replay, and applying it is not + // merely redundant but wrong: a typing.start whose matching typing.stop fell outside the + // synced window would leave a user typing forever. Skipped entirely, state included. + if (IsApplyingHistorySync) + { + return; + } + if (_cache.Channels.TryGet(eventDto.Cid, out var streamChannel)) { streamChannel.InternalHandleTypingStopped(eventDto); } } + /// private void OnTypingStarted(TypingStartEventInternalDTO eventDto) { + if (IsApplyingHistorySync) + { + return; + } + if (_cache.Channels.TryGet(eventDto.Cid, out var streamChannel)) { streamChannel.InternalHandleTypingStarted(eventDto); diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs new file mode 100644 index 00000000..9fe9f9a3 --- /dev/null +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -0,0 +1,367 @@ +#if STREAM_TESTS_ENABLED +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using NSubstitute; +using NUnit.Framework; +using StreamChat.Core; +using StreamChat.Core.Configs; +using StreamChat.Core.InternalDTO.Responses; +using StreamChat.Core.LowLevelClient; +using StreamChat.Core.Responses; +using StreamChat.Core.State.Caches; +using StreamChat.Core.StatefulModels; +using StreamChat.Libs.AppInfo; +using StreamChat.Libs.Auth; +using StreamChat.Libs.ChatInstanceRunner; +using StreamChat.Libs.Http; +using StreamChat.Libs.Logs; +using StreamChat.Libs.NetworkMonitors; +using StreamChat.Libs.Serialization; +using StreamChat.Libs.Time; +using StreamChat.Libs.Websockets; + +namespace StreamChat.Tests.StateSync.Unit +{ + /// + /// Unit tests for reconnect state recovery on , driven entirely + /// through mocked transports so connection state transitions can be sequenced exactly. + /// + internal class StateRecoveryClientTests + { + [SetUp] + public void Up() + { + _authCredentials = new AuthCredentials("api123", "user123", "token123"); + _mockWebsocketClient = Substitute.For(); + _mockHttpClient = Substitute.For(); + _mockTimeService = Substitute.For(); + _mockNetworkMonitor = Substitute.For(); + _mockApplicationInfo = Substitute.For(); + _mockLogs = Substitute.For(); + _config = new StreamClientConfig(); + + _mockWebsocketClient.ConnectAsync(Arg.Any()).Returns(Task.CompletedTask); + _mockWebsocketClient.DisconnectAsync(Arg.Any(), + Arg.Any()).Returns(Task.CompletedTask); + + _mockWebsocketClient.TryDequeueMessage(out Arg.Any()).Returns(arg => + { + if (_pendingWebsocketMessages.Count == 0) + { + return false; + } + + arg[0] = _pendingWebsocketMessages.Dequeue(); + return true; + }); + + RespondWith(SyncEndpoint, "{\"events\":[]}"); + RespondWith(QueryChannelsEndpoint, "{\"channels\":[]}"); + + _client = (StreamChatClient)StreamChatClient.CreateClientWithCustomDependencies(_mockWebsocketClient, + _mockHttpClient, new NewtonsoftJsonSerializer(), _mockTimeService, _mockNetworkMonitor, + _mockApplicationInfo, _mockLogs, _config); + } + + [TearDown] + public void TearDown() + { + _client.Dispose(); + _client = null; + _pendingWebsocketMessages.Clear(); + _recoveredEvents.Clear(); + } + + [Test] + public void when_first_connect_expect_no_recovery_and_no_state_recovered_event() + { + Connect(); + + Assert.AreEqual(0, _recoveredEvents.Count, + "A fresh login is not a recovery - firing StateRecovered here would make it useless as a signal."); + AssertQueryChannelsCallCount(0); + } + + [Test] + public void when_reconnected_expect_channels_requeried_even_though_sync_was_skipped() + { + Connect(); + WatchChannel("messaging:a"); + WatchChannel("messaging:b"); + + DropConnection(); + Reconnect(); + + // The whole point of #227/#232: the re-query is what re-establishes the watches, so it has + // to run whether or not the /sync catch-up did anything. Here it was skipped outright, + // because the health check carried no created_at so there is no sync point. + AssertQueryChannelsCallCount(1); + Assert.AreEqual(1, _recoveredEvents.Count); + } + + [Test] + public void when_connection_drops_expect_watches_released_and_snapshot_taken() + { + Connect(); + var channel = WatchChannel("messaging:a"); + + DropConnection(); + + // The server dropped the watch, so continuing to report it would be a lie - and would make + // IStreamChannel.WatchAsync a silent no-op for a channel that is not actually watched. + Assert.IsFalse(channel.IsWatched); + Assert.AreEqual(0, _client.WatchedChannels.Count); + Assert.AreEqual(new[] { "messaging:a" }, RecoverySnapshot()); + } + + [Test] + public void when_reconnect_attempt_fails_expect_recovery_snapshot_preserved() + { + Connect(); + WatchChannel("messaging:a"); + WatchChannel("messaging:b"); + + DropConnection(); + FailReconnectAttempt(); + FailReconnectAttempt(); + + // A failed attempt transitions Connecting -> Disconnected with an already-empty watch list. + // Re-snapshotting there would discard the only record of what needs recovering, and the + // attempt that eventually succeeds would restore nothing - the flaky-mobile-network case. + Assert.AreEqual(new[] { "messaging:a", "messaging:b" }, RecoverySnapshot().OrderBy(_ => _).ToArray()); + + Reconnect(); + AssertQueryChannelsCallCount(1); + } + + [Test] + public void when_channels_cannot_be_recovered_expect_them_reported_as_unrecovered() + { + Connect(); + WatchChannel("messaging:a"); + + DropConnection(); + Reconnect(); + + // The mocked query returns no channels, which is what the server does for a channel that + // was deleted or that the local user lost access to while offline. + Assert.AreEqual(1, _recoveredEvents.Count); + Assert.AreEqual(0, _recoveredEvents[0].Channels.Count); + Assert.AreEqual(new[] { "messaging:a" }, _recoveredEvents[0].UnrecoveredChannelCids.ToArray()); + Assert.IsFalse(_recoveredEvents[0].IsComplete); + } + + [Test] + public void when_recovery_query_fails_expect_remaining_chunks_still_queried() + { + Connect(); + for (var i = 0; i < 40; i++) + { + WatchChannel($"messaging:channel-{i:D2}"); + } + + var callCount = 0; + _mockHttpClient + .SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), Arg.Any()) + .Returns(_ => + { + callCount++; + return callCount == 1 + ? new HttpResponse(false, 429, "{\"code\":9,\"message\":\"rate limited\"}", null, null) + : new HttpResponse(true, 200, "{\"channels\":[]}", null, null); + }); + + DropConnection(); + Reconnect(); + + // 40 cids is two chunks of 30 and 10. There is no later retry within a connection, so a + // failed chunk must not cost the remaining chunks their recovery. + Assert.AreEqual(2, callCount); + Assert.AreEqual(1, _recoveredEvents.Count); + } + + [Test] + public void when_recovery_set_exceeds_cap_expect_only_capped_channels_queried() + { + Connect(); + for (var i = 0; i < StreamChatClient.MaxRecoveredChannels + 25; i++) + { + WatchChannel($"messaging:channel-{i:D3}"); + } + + DropConnection(); + Reconnect(); + + // Capped at 100, chunked by 30 -> 4 requests. Uncapped this would be 5, and would keep + // growing with the watch list on every single reconnect. + AssertQueryChannelsCallCount(4); + } + + [Test] + public void when_strategy_is_disabled_expect_no_recovery_and_watches_left_untouched() + { + _config.StateRecoveryStrategy = StateRecoveryStrategy.Disabled; + + Connect(); + var channel = WatchChannel("messaging:a"); + + DropConnection(); + Reconnect(); + + AssertQueryChannelsCallCount(0); + Assert.AreEqual(0, _recoveredEvents.Count); + + // Disabled means the SDK does nothing, so WatchedChannels stays the record of what the + // consumer was watching and is theirs to recover from. + Assert.IsTrue(channel.IsWatched); + Assert.AreEqual(1, _client.WatchedChannels.Count); + } + + [Test] + public void when_user_disconnects_and_connects_again_expect_no_recovery_of_previous_session() + { + Connect(); + WatchChannel("messaging:a"); + + _client.DisconnectUserAsync().GetAwaiter().GetResult(); + DropConnection(); + + Connect(); + + // A new login must not recover, or re-watch, channels belonging to the session that ended - + // possibly for a different user. + AssertQueryChannelsCallCount(0); + Assert.AreEqual(0, _recoveredEvents.Count); + } + + [Test] + public void when_channel_unwatched_while_disconnected_expect_it_not_recovered() + { + Connect(); + var channel = WatchChannel("messaging:a"); + WatchChannel("messaging:b"); + + DropConnection(); + InvokeMarkChannelUnwatched(channel); + Reconnect(); + + Assert.AreEqual(new[] { "messaging:b" }, RecoverySnapshot()); + } + + private const string SyncEndpoint = "/sync"; + private const string QueryChannelsEndpoint = "/channels"; + + private void RespondWith(string endpointSuffix, string json) + { + _mockHttpClient + .SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(endpointSuffix)), Arg.Any()) + .Returns(new HttpResponse(true, 200, json, null, null)); + } + + private void AssertQueryChannelsCallCount(int expected) + { + _mockHttpClient.Received(expected).SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), Arg.Any()); + } + + private void Connect() + { + _client.StateRecovered -= OnStateRecovered; + _client.StateRecovered += OnStateRecovered; + + var connectTask = _client.ConnectUserAsync(_authCredentials); + _pendingWebsocketMessages.Enqueue(HealthCheckJson); + Update(); + + Assert.IsTrue(connectTask.IsCompleted, "Expected the mocked health check to complete the connect."); + Assert.AreEqual(ConnectionState.Connected, _client.ConnectionState); + } + + private void Reconnect() + { + _client.InternalLowLevelClient.Connect(); + _pendingWebsocketMessages.Enqueue(HealthCheckJson); + Update(); + + Assert.AreEqual(ConnectionState.Connected, _client.ConnectionState); + } + + private void DropConnection() + { + _mockWebsocketClient.Disconnected += Raise.Event(); + Update(); + + Assert.AreEqual(ConnectionState.Disconnected, _client.ConnectionState); + } + + private void FailReconnectAttempt() + { + _client.InternalLowLevelClient.Connect(); + Assert.AreEqual(ConnectionState.Connecting, _client.ConnectionState); + + _mockWebsocketClient.ConnectionFailed += Raise.Event(); + Update(); + + Assert.AreEqual(ConnectionState.Disconnected, _client.ConnectionState); + } + + private void Update() => ((IStreamChatClientEventsListener)_client).Update(); + + private void OnStateRecovered(StreamStateRecoveredEventArgs args) => _recoveredEvents.Add(args); + + private IStreamChannel WatchChannel(string cid) + { + var separatorIndex = cid.IndexOf(':'); + var channel = _client.InternalCache.TryCreateOrUpdate(new ChannelResponseInternalDTO + { + Cid = cid, + Type = cid.Substring(0, separatorIndex), + Id = cid.Substring(separatorIndex + 1), + }); + + InvokePrivate("MarkChannelWatched", channel); + return channel; + } + + private void InvokeMarkChannelUnwatched(IStreamChannel channel) + => InvokePrivate("InternalMarkChannelUnwatched", channel); + + private void InvokePrivate(string methodName, object argument) + { + var method = typeof(StreamChatClient).GetMethod(methodName, + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull(method, $"Expected {methodName} to exist."); + method.Invoke(_client, new[] { argument }); + } + + private string[] RecoverySnapshot() + { + var field = typeof(StreamChatClient).GetField("_recoveryChannelCids", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull(field, "Expected _recoveryChannelCids to exist."); + return ((List)field.GetValue(_client)).ToArray(); + } + + private const string HealthCheckJson = "{\"connection_id\":\"fakeId\",\"type\":\"health.check\"}"; + + private readonly Queue _pendingWebsocketMessages = new Queue(); + private readonly List _recoveredEvents = + new List(); + + private StreamChatClient _client; + private StreamClientConfig _config; + private AuthCredentials _authCredentials; + private IWebsocketClient _mockWebsocketClient; + private IApplicationInfo _mockApplicationInfo; + private ILogs _mockLogs; + private ITimeService _mockTimeService; + private INetworkMonitor _mockNetworkMonitor; + private IHttpClient _mockHttpClient; + } +} +#endif diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs.meta b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs.meta new file mode 100644 index 00000000..7ae2eba4 --- /dev/null +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 533cd95f3d407dc42934ec9bb226a9e0 \ No newline at end of file diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs new file mode 100644 index 00000000..31466051 --- /dev/null +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs @@ -0,0 +1,326 @@ +#if STREAM_TESTS_ENABLED +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using NSubstitute; +using NUnit.Framework; +using StreamChat.Core.Configs; +using StreamChat.Core.LowLevelClient; +using StreamChat.Libs.AppInfo; +using StreamChat.Libs.Auth; +using StreamChat.Libs.Http; +using StreamChat.Libs.Logs; +using StreamChat.Libs.NetworkMonitors; +using StreamChat.Libs.Serialization; +using StreamChat.Libs.Time; +using StreamChat.Libs.Websockets; + +namespace StreamChat.Tests.StateSync.Unit +{ + /// + /// Unit tests for the two history application modes on - + /// and + /// - and for the /sync request shape. + /// + internal class StateRecoveryLowLevelTests + { + [SetUp] + public void Up() + { + _authCredentials = new AuthCredentials("api123", "token123", "user123"); + _mockWebsocketClient = Substitute.For(); + _mockHttpClient = Substitute.For(); + _serializer = new NewtonsoftJsonSerializer(); + _mockTimeService = Substitute.For(); + _mockNetworkMonitor = Substitute.For(); + _mockApplicationInfo = Substitute.For(); + _mockLogs = Substitute.For(); + _mockStreamClientConfig = Substitute.For(); + + _lowLevelClient = CreateClient(); + _lowLevelClient.Update(0.1f); + + _mockHttpClient + .SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), Arg.Any(), Arg.Any()) + .Returns(new HttpResponse(true, 200, "{\"events\":[]}", null, null)); + } + + [TearDown] + public void TearDown() + { + for (var i = _clientsToDispose.Count - 1; i >= 0; i--) + { + _clientsToDispose[i].Dispose(); + } + + _clientsToDispose.Clear(); + _lowLevelClient = null; + } + + [Test] + public void when_sync_requested_with_more_than_100_cids_expect_only_100_sent() + { + var now = new DateTimeOffset(2026, 8, 10, 12, 0, 0, TimeSpan.Zero); + _mockTimeService.Now.Returns(now); + SetDisconnectionLastEventReceivedAt(_lowLevelClient, now.AddHours(-1)); + + var cids = Enumerable.Range(0, 150).Select(i => $"messaging:channel-{i}").ToList(); + + _lowLevelClient.TrySyncHistoryAsync(cids).GetAwaiter().GetResult(); + + _mockHttpClient.Received(1).SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith("/sync")), + Arg.Is(body => CountSyncCids(body) == StreamChatLowLevelClient.MaxSyncChannelCids)); + } + + [Test] + public void when_sync_requested_expect_inaccessible_cids_asked_for() + { + var now = new DateTimeOffset(2026, 8, 10, 12, 0, 0, TimeSpan.Zero); + _mockTimeService.Now.Returns(now); + SetDisconnectionLastEventReceivedAt(_lowLevelClient, now.AddHours(-1)); + + _lowLevelClient.TrySyncHistoryAsync(new[] { "messaging:a" }).GetAwaiter().GetResult(); + + // Without this the response cannot distinguish a deleted channel from one the query + // happened to omit, and recovery would keep retrying it forever. + _mockHttpClient.Received(1).SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith("/sync")), + Arg.Is(body => GetBoolMember(body, "WithInaccessibleCids") == true)); + } + + [Test] + public void when_history_batch_applied_expect_no_public_message_received() + { + var received = 0; + _lowLevelClient.MessageReceived += _ => received++; + + var result = _lowLevelClient.ApplyHistoryEvents(new List { MessageNewJson("msg-1", NewestCreatedAt) }); + + Assert.AreEqual(0, received, "A silent history batch must not raise public per-event callbacks."); + Assert.AreEqual(0, result.FailedEventCount); + Assert.AreEqual(NewestCreatedAt, result.MaxAppliedCreatedAt); + } + + [Test] + public void when_history_batch_replayed_expect_public_message_received() + { + var received = 0; + _lowLevelClient.MessageReceived += _ => received++; + + _lowLevelClient.ReplayHistoryEvents(new List { MessageNewJson("msg-1", NewestCreatedAt) }); + + Assert.AreEqual(1, received, + "ReplayEvents is the default strategy and must keep raising per-event callbacks for back-compat."); + } + + [Test] + public void when_history_batch_contains_custom_event_expect_it_delivered_per_event() + { + var received = new List(); + _lowLevelClient.CustomEventReceived += e => received.Add(e.Type); + + _lowLevelClient.ApplyHistoryEvents(new List + { + CustomEventJson("game.state", NewestCreatedAt), + }); + + // Custom events have no representation in local state, so suppressing them would lose the + // payload with no way for a consumer to recover it. + Assert.AreEqual(new[] { "game.state" }, received.ToArray()); + } + + [Test] + public void when_history_batch_applied_expect_watermark_advanced_once_to_newest_applied_event() + { + var client = CreateClient(); + + client.ApplyHistoryEvents(new List + { + MessageNewJson("msg-1", NewestCreatedAt.AddMinutes(-10)), + MessageNewJson("msg-2", NewestCreatedAt), + MessageNewJson("msg-3", NewestCreatedAt.AddMinutes(-5)), + }); + + Assert.AreEqual(NewestCreatedAt, GetLastEventReceivedAt(client), + "The batch must advance the watermark exactly once, to its newest event."); + } + + [Test] + public void when_history_batch_contains_malformed_event_expect_remaining_events_still_applied() + { + var client = CreateClient(); + var newest = NewestCreatedAt; + + var result = client.ApplyHistoryEvents(new List + { + MessageNewJson("msg-1", newest.AddMinutes(-10)), + $"{{\"type\":\"message.new\",\"cid\":\"messaging:test\",\"created_at\":\"{newest.AddMinutes(-1):O}\",\"message\":\"not-an-object\"}}", + MessageNewJson("msg-3", newest.AddMinutes(-5)), + }); + + Assert.AreEqual(1, result.FailedEventCount); + + // The watermark must not claim the failed event was applied, or the next reconnect would + // never ask for it again. + Assert.AreEqual(newest.AddMinutes(-5), result.MaxAppliedCreatedAt); + Assert.AreEqual(newest.AddMinutes(-5), GetLastEventReceivedAt(client)); + } + + [Test] + public void when_history_event_older_than_watermark_expect_watermark_not_regressed() + { + var client = CreateClient(); + SetLastEventReceivedAt(client, NewestCreatedAt); + + client.ApplyHistoryEvents(new List { MessageNewJson("msg-1", NewestCreatedAt.AddDays(-1)) }); + + Assert.AreEqual(NewestCreatedAt, GetLastEventReceivedAt(client)); + } + + [Test] + public void when_health_check_arrives_on_live_socket_expect_liveness_stamped_before_handlers() + { + var client = CreateClientWithMessages(HealthCheckJson()); + client.Connect(); + client.Update(0.2f); + + _mockTimeService.Time.Returns(12f); + EnqueueMessages(HealthCheckJson()); + client.Update(0.2f); + + Assert.AreEqual(12f, GetLastHealthCheckReceivedTime(client), + "Liveness must be stamped when the health check is read, not after consumer handlers run."); + } + + [Test] + public void when_health_check_arrives_from_history_replay_expect_liveness_not_stamped() + { + var client = CreateClientWithMessages(HealthCheckJson()); + client.Connect(); + client.Update(0.2f); + + var stampedOnConnect = GetLastHealthCheckReceivedTime(client); + + _mockTimeService.Time.Returns(99f); + client.ReplayHistoryEvents(new List { HealthCheckJson() }); + + Assert.AreEqual(stampedOnConnect, GetLastHealthCheckReceivedTime(client), + "A replayed health check proves nothing about the current socket and must not extend liveness."); + } + + private static readonly DateTimeOffset NewestCreatedAt = + new DateTimeOffset(2026, 8, 10, 11, 0, 0, TimeSpan.Zero); + + private const string TestCid = "messaging:test"; + + private static string MessageNewJson(string messageId, DateTimeOffset createdAt) + => $"{{\"type\":\"message.new\",\"cid\":\"{TestCid}\",\"created_at\":\"{createdAt:O}\"," + + $"\"message\":{{\"id\":\"{messageId}\",\"text\":\"hi\",\"created_at\":\"{createdAt:O}\"," + + $"\"updated_at\":\"{createdAt:O}\",\"user\":{{\"id\":\"user-1\"}}}}}}"; + + private static string CustomEventJson(string type, DateTimeOffset createdAt) + => $"{{\"type\":\"{type}\",\"cid\":\"{TestCid}\",\"created_at\":\"{createdAt:O}\"," + + "\"user\":{\"id\":\"user-1\"}}"; + + private static string HealthCheckJson() + => "{\"connection_id\":\"fakeId\",\"type\":\"health.check\"}"; + + private static int CountSyncCids(object requestBody) + { + var list = GetMember(requestBody, "ChannelCids") as System.Collections.IList; + return list?.Count ?? -1; + } + + private static bool? GetBoolMember(object requestBody, string name) => GetMember(requestBody, name) as bool?; + + private static object GetMember(object requestBody, string name) + { + const BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; + + var property = requestBody.GetType().GetProperty(name, flags); + if (property != null) + { + return property.GetValue(requestBody); + } + + return requestBody.GetType().GetField(name, flags)?.GetValue(requestBody); + } + + private static void SetDisconnectionLastEventReceivedAt(StreamChatLowLevelClient client, DateTimeOffset value) + => GetPrivateField("_disconnectionLastEventReceivedAt").SetValue(client, (DateTimeOffset?)value); + + private static void SetLastEventReceivedAt(StreamChatLowLevelClient client, DateTimeOffset value) + => GetPrivateField("_lastEventReceivedAt").SetValue(client, (DateTimeOffset?)value); + + private static DateTimeOffset? GetLastEventReceivedAt(StreamChatLowLevelClient client) + => (DateTimeOffset?)GetPrivateField("_lastEventReceivedAt").GetValue(client); + + private static float GetLastHealthCheckReceivedTime(StreamChatLowLevelClient client) + => (float)GetPrivateField("_lastHealthCheckReceivedTime").GetValue(client); + + private static FieldInfo GetPrivateField(string name) + { + var field = typeof(StreamChatLowLevelClient).GetField(name, + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull(field, $"Expected {name} field to exist."); + return field; + } + + private StreamChatLowLevelClient CreateClient() + { + var client = new StreamChatLowLevelClient(_authCredentials, _mockWebsocketClient, _mockHttpClient, + _serializer, _mockTimeService, _mockNetworkMonitor, _mockApplicationInfo, _mockLogs, + _mockStreamClientConfig); + + _clientsToDispose.Add(client); + return client; + } + + private StreamChatLowLevelClient CreateClientWithMessages(params string[] websocketMessages) + { + var client = CreateClient(); + _mockWebsocketClient.ConnectAsync(Arg.Any()).Returns(System.Threading.Tasks.Task.CompletedTask); + + _mockWebsocketClient.TryDequeueMessage(out Arg.Any()).Returns(arg => + { + if (_pendingWebsocketMessages.Count == 0) + { + return false; + } + + arg[0] = _pendingWebsocketMessages.Dequeue(); + return true; + }); + + EnqueueMessages(websocketMessages); + return client; + } + + private void EnqueueMessages(params string[] websocketMessages) + { + foreach (var message in websocketMessages) + { + _pendingWebsocketMessages.Enqueue(message); + } + } + + private readonly List _clientsToDispose = new List(); + private readonly Queue _pendingWebsocketMessages = new Queue(); + + private StreamChatLowLevelClient _lowLevelClient; + private AuthCredentials _authCredentials; + private IWebsocketClient _mockWebsocketClient; + private IApplicationInfo _mockApplicationInfo; + private ILogs _mockLogs; + private ISerializer _serializer; + private ITimeService _mockTimeService; + private INetworkMonitor _mockNetworkMonitor; + private IHttpClient _mockHttpClient; + private IStreamClientConfig _mockStreamClientConfig; + } +} +#endif diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs.meta b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs.meta new file mode 100644 index 00000000..f6c190db --- /dev/null +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: b019eca0edff8ba44971000de05399a6 \ No newline at end of file From f17d2af949e19980afcd6deee09ec0955415bd0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:22:59 +0200 Subject: [PATCH 02/10] Recovery used to apply a query result (and mark channels watched) before checking whether that reconnect was still current, so a late response from an old connection could overwrite newer members/read/pinned. Apply and `MarkChannelWatched` now run only after the generation check, so a stale in-flight query is discarded. --- .../StreamChat/Core/StreamChatClient.cs | 105 +++++++++-------- .../StateSync/StateRecoveryClientTests.cs | 108 ++++++++++++++++++ 2 files changed, 165 insertions(+), 48 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index d753d87e..04f8f8a9 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -296,28 +296,7 @@ public async Task> QueryChannelsAsync(IEnumerable _.GenerateFilterEntry()).ToDictionary(x => x.Key, x => x.Value), - Limit = limit, - MemberLimit = null, - MessageLimit = null, - Offset = offset, - Presence = true, - - /* - * StreamTodo: Allowing to sort query can potentially lead to mixed sorting in WatchedChannels - * But there seems no other choice because its too limiting to force only a global sorting for channels - * e.g. user may want to show channels in multiple ways with different sorting which would not work with global only sorting - */ - Sort = sort?.ToSortParamRequestList(), - State = true, - Watch = true, - }; - - var channelsResponseDto - = await InternalLowLevelClient.InternalChannelApi.QueryChannelsAsync(requestBodyDto); + var channelsResponseDto = await FetchQueryChannelsResponseAsync(filters, sort, limit, offset); if (channelsResponseDto.Channels == null || channelsResponseDto.Channels.Count == 0) { return Enumerable.Empty(); @@ -342,28 +321,8 @@ public async Task> QueryChannelsAsync(IDictionary x.Key, x => x.Value), - Limit = limit, - MemberLimit = null, - MessageLimit = null, - Offset = offset, - Presence = true, - - /* - * StreamTodo: Allowing to sort query can potentially lead to mixed sorting in WatchedChannels - * But there seems no other choice because its too limiting to force only a global sorting for channels - * e.g. user may want to show channels in multiple ways with different sorting which would not work with global only sorting - */ - Sort = sort?.ToSortParamRequestList(), - State = true, - Watch = true, - }; - - var channelsResponseDto - = await InternalLowLevelClient.InternalChannelApi.QueryChannelsAsync(requestBodyDto); + var channelsResponseDto = await InternalLowLevelClient.InternalChannelApi.QueryChannelsAsync( + CreateQueryChannelsRequest(filters, sort, limit, offset)); if (channelsResponseDto.Channels == null || channelsResponseDto.Channels.Count == 0) { return Enumerable.Empty(); @@ -1448,10 +1407,13 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove ChannelFilter.Cid.In(chunk), }; - IEnumerable channels; + QueryChannelsResponseInternalDTO response; try { - channels = await QueryChannelsAsync(filters, sort, limit: chunk.Count); + // Fetch only. Public QueryChannelsAsync applies immediately (members/read/pinned + // are replaced, watches marked). A request started on an old connection can still + // succeed after a reconnect, so apply must wait for the generation check below. + response = await FetchQueryChannelsResponseAsync(filters, sort, chunk.Count); } catch (Exception e) { @@ -1468,17 +1430,64 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove return; } - foreach (var channel in channels) + if (response?.Channels == null) + { + continue; + } + + foreach (var channelDto in response.Channels) { + var channel = _cache.TryCreateOrUpdate(channelDto); + if (channel == null) + { + continue; + } + + MarkChannelWatched(channel); // The query merge path goes through UpdateFromDto, which does not trim, so a // recovery merge can push Messages past MessageCacheWindow.MaxMessages. - ((StreamChannel)channel).InternalTrimMessageCache(); + channel.InternalTrimMessageCache(); refreshed.Add(channel); } } } } + /// + /// Same request as public + /// but does not touch _cache or _watchedChannels. Recovery uses this so a stale + /// in-flight response cannot overwrite newer state or mark watches for a dropped connection. + /// + private Task FetchQueryChannelsResponseAsync( + IEnumerable filters, ChannelSortObject sort, int limit, int offset = 0) + { + var filterConditions = filters?.Select(_ => _.GenerateFilterEntry()).ToDictionary(x => x.Key, x => x.Value); + return InternalLowLevelClient.InternalChannelApi.QueryChannelsAsync( + CreateQueryChannelsRequest(filterConditions, sort, limit, offset)); + } + + //StreamTodo: Perhaps MessageLimit and MemberLimit should be configurable + /* + * StreamTodo: Allowing to sort query can potentially lead to mixed sorting in WatchedChannels + * But there seems no other choice because its too limiting to force only a global sorting for channels + * e.g. user may want to show channels in multiple ways with different sorting which would not work with global only sorting + */ + private static QueryChannelsRequestInternalDTO CreateQueryChannelsRequest( + IDictionary filterConditions, ChannelSortObject sort, int limit, int offset) + => new QueryChannelsRequestInternalDTO + { + FilterConditions = filterConditions as Dictionary + ?? filterConditions?.ToDictionary(x => x.Key, x => x.Value), + Limit = limit, + MemberLimit = null, + MessageLimit = null, + Offset = offset, + Presence = true, + Sort = sort?.ToSortParamRequestList(), + State = true, + Watch = true, + }; + private bool IsRecoveryGenerationCurrent(int generation) => generation == _recoveryGeneration; /// diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index 9fe9f9a3..5601f728 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -1,9 +1,11 @@ #if STREAM_TESTS_ENABLED using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Threading.Tasks; +using UnityEngine.TestTools; using NSubstitute; using NUnit.Framework; using StreamChat.Core; @@ -252,6 +254,77 @@ public void when_channel_unwatched_while_disconnected_expect_it_not_recovered() Assert.AreEqual(new[] { "messaging:b" }, RecoverySnapshot()); } + [Test] + public void when_recovery_query_returns_channel_expect_watch_restored_and_state_recovered() + { + Connect(); + var channel = WatchChannel("messaging:a"); + + RespondWith(QueryChannelsEndpoint, QueryChannelsJson("messaging:a", "restored")); + + DropConnection(); + Reconnect(); + + Assert.IsTrue(channel.IsWatched); + Assert.AreEqual(1, _client.WatchedChannels.Count); + Assert.AreSame(channel, _client.WatchedChannels.Single()); + Assert.AreEqual("restored", channel.Name); + Assert.AreEqual(1, _recoveredEvents.Count); + Assert.AreSame(channel, _recoveredEvents[0].Channels.Single()); + Assert.IsTrue(_recoveredEvents[0].IsComplete); + } + + [UnityTest] + public IEnumerator when_stale_recovery_query_completes_after_newer_recovery_expect_stale_result_not_applied() + { + Connect(); + var channel = WatchChannel("messaging:a"); + + var staleQuery = new TaskCompletionSource(); + var currentQuery = new TaskCompletionSource(); + HoldQueryChannelsResponses(staleQuery.Task, currentQuery.Task); + + DropConnection(); + Reconnect(); + + Assert.AreEqual(0, _recoveredEvents.Count, + "The first recovery must still be waiting on its query."); + Assert.IsFalse(channel.IsWatched); + Assert.AreEqual(0, _client.WatchedChannels.Count); + + // Increments generation. Snapshot is kept because watches were already cleared. + DropConnection(); + Reconnect(); + + Assert.AreEqual(0, _recoveredEvents.Count); + + currentQuery.SetResult(QueryChannelsHttpResponse("messaging:a", "fresh-B")); + yield return WaitUntil(() => channel.Name == "fresh-B", + "Current-generation recovery did not apply the fresh query payload."); + + Assert.IsTrue(channel.IsWatched); + Assert.AreEqual(1, _client.WatchedChannels.Count); + Assert.AreSame(channel, _client.WatchedChannels.Single()); + Assert.AreEqual(1, _recoveredEvents.Count); + Assert.AreSame(channel, _recoveredEvents[0].Channels.Single()); + Assert.IsTrue(_recoveredEvents[0].IsComplete); + + staleQuery.SetResult(QueryChannelsHttpResponse("messaging:a", "stale-A")); + // Drain posted continuations so a missing generation gate would have applied by now. + for (var i = 0; i < 5; i++) + { + Update(); + yield return null; + } + + Assert.AreEqual("fresh-B", channel.Name, + "A late query from a superseded recovery must not replace channel state."); + Assert.IsTrue(channel.IsWatched); + Assert.AreEqual(1, _client.WatchedChannels.Count); + Assert.AreEqual(1, _recoveredEvents.Count, + "The stale recovery must not raise StateRecovered after a newer one already did."); + } + private const string SyncEndpoint = "/sync"; private const string QueryChannelsEndpoint = "/channels"; @@ -263,6 +336,41 @@ private void RespondWith(string endpointSuffix, string json) .Returns(new HttpResponse(true, 200, json, null, null)); } + private void HoldQueryChannelsResponses(params Task[] responses) + { + var queue = new Queue>(responses); + _mockHttpClient + .SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), Arg.Any()) + .Returns(_ => queue.Dequeue()); + } + + private static HttpResponse QueryChannelsHttpResponse(string cid, string name) + => new HttpResponse(true, 200, QueryChannelsJson(cid, name), null, null); + + private static string QueryChannelsJson(string cid, string name) + { + var separatorIndex = cid.IndexOf(':'); + var type = cid.Substring(0, separatorIndex); + var id = cid.Substring(separatorIndex + 1); + return "{\"channels\":[{\"channel\":{" + + $"\"cid\":\"{cid}\",\"id\":\"{id}\",\"type\":\"{type}\",\"name\":\"{name}\"" + + "},\"messages\":[],\"members\":[]}]}"; + } + + private IEnumerator WaitUntil(Func condition, string message, int maxFrames = 30) + { + var frames = 0; + while (!condition() && frames < maxFrames) + { + Update(); + yield return null; + frames++; + } + + Assert.IsTrue(condition(), message); + } + private void AssertQueryChannelsCallCount(int expected) { _mockHttpClient.Received(expected).SendHttpRequestAsync(Arg.Is(HttpMethodType.Post), From f66ce3bed4b89a80731c662412f3c9770048bc76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:32:11 +0200 Subject: [PATCH 03/10] Fix edge where _inaccessibleCids would not be updated because /sync returned no events. But it could still contain info about a channel CID to which the user lost access --- .../StreamChat/Core/StreamChatClient.cs | 13 +++-- .../StateSync/StateRecoveryClientTests.cs | 48 +++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index 04f8f8a9..a087c040 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -1311,8 +1311,9 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, // null means the catch-up was skipped: no sync point, or one older than the 30 days // the server accepts. Both used to return before any recovery ran; now step 2 still - // runs, which is the whole point of making it unconditional. - if (response?.Events == null || response.Events.Count == 0) + // runs, which is the whole point of making it unconditional. That is not the same as + // the server naming inaccessible cids - do not treat a skip as a deletion. + if (response == null) { return; } @@ -1326,13 +1327,19 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, { // The server is telling us these will never come back. Recording them keeps the // re-query from asking about deleted channels and stops us reporting them as a - // recovery failure every reconnect. + // recovery failure every reconnect. Must run even when Events is empty - a + // deleted channel can be the only thing /sync has to say. foreach (var cid in response.InaccessibleCids) { _inaccessibleCids.Add(cid); } } + if (response.Events == null || response.Events.Count == 0) + { + return; + } + if (InternalLowLevelClient.Config.StateRecoveryStrategy == StateRecoveryStrategy.BatchStateUpdate) { InternalLowLevelClient.ApplyHistoryEvents(response.Events); diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index 5601f728..e3632d98 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -254,6 +254,40 @@ public void when_channel_unwatched_while_disconnected_expect_it_not_recovered() Assert.AreEqual(new[] { "messaging:b" }, RecoverySnapshot()); } + [Test] + public void when_sync_returns_empty_events_with_inaccessible_cids_expect_those_cids_not_requeried() + { + Connect(); + WatchChannel("messaging:gone"); + var ok = WatchChannel("messaging:ok"); + + var now = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + _mockTimeService.Now.Returns(now); + + DropConnection(); + // Health checks in these tests carry no created_at, so disconnect would otherwise leave + // no sync point and skip /sync entirely. Seed one after the drop so the copy on + // Disconnected cannot wipe it. + SetDisconnectionLastEventReceivedAt(_client.InternalLowLevelClient, now.AddHours(-1)); + + RespondWith(SyncEndpoint, "{\"events\":[],\"inaccessible_cids\":[\"messaging:gone\"]}"); + RespondWith(QueryChannelsEndpoint, QueryChannelsJson("messaging:ok", "ok")); + + Reconnect(); + + _mockHttpClient.Received(1).SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), + Arg.Is(body => RequestBodyContains(body, "messaging:ok") + && !RequestBodyContains(body, "messaging:gone"))); + + Assert.AreEqual(1, _recoveredEvents.Count); + Assert.AreSame(ok, _recoveredEvents[0].Channels.Single()); + Assert.AreEqual(new[] { "messaging:gone" }, _recoveredEvents[0].UnrecoveredChannelCids.ToArray()); + Assert.IsFalse(_recoveredEvents[0].IsComplete); + Assert.IsTrue(ok.IsWatched); + } + [Test] public void when_recovery_query_returns_channel_expect_watch_restored_and_state_recovered() { @@ -455,6 +489,20 @@ private string[] RecoverySnapshot() return ((List)field.GetValue(_client)).ToArray(); } + private static void SetDisconnectionLastEventReceivedAt(StreamChatLowLevelClient client, DateTimeOffset value) + { + var field = typeof(StreamChatLowLevelClient).GetField("_disconnectionLastEventReceivedAt", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull(field, "Expected _disconnectionLastEventReceivedAt to exist."); + field.SetValue(client, (DateTimeOffset?)value); + } + + private static bool RequestBodyContains(object requestBody, string value) + { + var json = requestBody as string ?? requestBody?.ToString() ?? string.Empty; + return json.IndexOf(value, StringComparison.Ordinal) >= 0; + } + private const string HealthCheckJson = "{\"connection_id\":\"fakeId\",\"type\":\"health.check\"}"; private readonly Queue _pendingWebsocketMessages = new Queue(); From 021bc9c5b722809c67770ae4e0cfb6227fef4496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:39:15 +0200 Subject: [PATCH 04/10] Fix MessagesRemovedFromCache being called during silent batch state update --- .../Core/StatefulModels/StreamChannel.cs | 11 +++- .../StateSync/StateRecoveryClientTests.cs | 55 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs index c1054cfb..ff2bb789 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs @@ -1146,7 +1146,9 @@ private void TrimMessageCacheIfNeeded() var handler = MessagesRemovedFromCache; // Not pooled - this is handed to subscribers, so the SDK does not control its lifetime. - var removed = handler == null ? null : new List(removeCount); + // Skip the allocation during a silent history batch: the callback is suppressed and a + // 1000-event /sync can trim repeatedly. + var removed = (handler == null || IsSilentHistorySync) ? null : new List(removeCount); using (new ListPoolScope(out var tempUntrackCandidates)) using (new HashSetPoolScope(out var tempPinnedMessageIds)) @@ -1172,7 +1174,12 @@ private void TrimMessageCacheIfNeeded() } // Raised after the pooled buffers are returned so subscribers can trim or send safely. - handler?.Invoke(this, removed); + // BatchStateUpdate already rebuilt from Messages on StateRecovered. Firing this + // during the batch would only destroy old rows that the rebuild is about to replace. + if (!IsSilentHistorySync) + { + handler?.Invoke(this, removed); + } } // Live messages are still appended while paused, so a channel that is never resumed keeps diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index e3632d98..a3c932d6 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -359,9 +359,64 @@ public IEnumerator when_stale_recovery_query_completes_after_newer_recovery_expe "The stale recovery must not raise StateRecovered after a newer one already did."); } + [Test] + public void when_silent_history_batch_trims_expect_messages_removed_from_cache_not_raised() + { + Connect(); + var channel = WatchChannel("messaging:a"); + channel.OverrideMessageCacheWindow(SmallWindow); + + var removedCount = 0; + channel.MessagesRemovedFromCache += (_, __) => removedCount++; + + _client.InternalLowLevelClient.ApplyHistoryEvents(MessageNewEvents("messaging:a", count: 7)); + + Assert.AreEqual(0, removedCount, + "BatchStateUpdate must not fire MessagesRemovedFromCache; the UI rebuilds on StateRecovered."); + Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count, + "Trim must still run during a silent batch so a 1000-event /sync cannot blow past MaxMessages."); + } + + [Test] + public void when_history_replay_trims_expect_messages_removed_from_cache_raised() + { + Connect(); + var channel = WatchChannel("messaging:a"); + channel.OverrideMessageCacheWindow(SmallWindow); + + var removedCount = 0; + channel.MessagesRemovedFromCache += (_, __) => removedCount++; + + _client.InternalLowLevelClient.ReplayHistoryEvents(MessageNewEvents("messaging:a", count: 7)); + + Assert.AreEqual(1, removedCount, + "ReplayEvents is the default and must keep raising MessagesRemovedFromCache for back-compat."); + Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count); + } + private const string SyncEndpoint = "/sync"; private const string QueryChannelsEndpoint = "/channels"; + // Same window as MessageCacheWindowTests: 7 messages exceed MaxMessages and trim down to 3. + private static readonly MessageCacheWindow SmallWindow = new MessageCacheWindow(6, 3); + + private static IEnumerable MessageNewEvents(string cid, int count) + { + var start = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + var events = new List(count); + for (var i = 0; i < count; i++) + { + events.Add(MessageNewJson(cid, $"msg-{i}", start.AddSeconds(i))); + } + + return events; + } + + private static string MessageNewJson(string cid, string messageId, DateTimeOffset createdAt) + => $"{{\"type\":\"message.new\",\"cid\":\"{cid}\",\"created_at\":\"{createdAt:O}\"," + + $"\"message\":{{\"id\":\"{messageId}\",\"text\":\"hi\",\"created_at\":\"{createdAt:O}\"," + + $"\"updated_at\":\"{createdAt:O}\",\"user\":{{\"id\":\"user-1\"}}}}}}"; + private void RespondWith(string endpointSuffix, string json) { _mockHttpClient From e28234b8d77d9c191ea3770a39614544387ddefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:15:22 +0200 Subject: [PATCH 05/10] Improve comments --- .../Core/Configs/IStreamClientConfig.cs | 7 +- .../Core/Configs/StateRecoveryStrategy.cs | 66 +++--- .../StreamChat/Core/IStreamChatClient.cs | 33 ++- .../LowLevelClient/HistorySyncApplyResult.cs | 9 +- .../StreamChatLowLevelClient.cs | 80 +++---- .../StreamStateRecoveredEventArgs.cs | 28 ++- .../Core/State/StreamStatefulModelBase.cs | 12 +- .../Core/StatefulModels/StreamChannel.cs | 23 +- .../StreamChat/Core/StreamChatClient.cs | 199 ++++++------------ 9 files changed, 176 insertions(+), 281 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs b/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs index 054bafcd..2af70921 100644 --- a/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs +++ b/Assets/Plugins/StreamChat/Core/Configs/IStreamClientConfig.cs @@ -38,10 +38,9 @@ public interface IStreamClientConfig MessageCacheWindow DefaultMessageCacheWindow { get; set; } /// - /// How the client restores local state after the websocket reconnects. Defaults to - /// , which preserves the per-event - /// callback behaviour of earlier SDK versions. See - /// for when to pick each option. + /// How the client restores local state after the websocket reconnects. + /// Default is . + /// See for the other options. /// StateRecoveryStrategy StateRecoveryStrategy { get; set; } } diff --git a/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs index 4029c10a..f10497cd 100644 --- a/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs +++ b/Assets/Plugins/StreamChat/Core/Configs/StateRecoveryStrategy.cs @@ -2,62 +2,56 @@ namespace StreamChat.Core.Configs { /// /// How restores local state after the websocket reconnects. - /// Set through . + /// Set this on . /// /// - /// Regardless of the strategy, a reconnect always drops the server-side watches that were - /// established before the disconnect. and - /// re-establish them; leaves that to you. + /// After a reconnect, the server always drops the old watches. + /// and start watching those channels again. + /// does not - you must do that yourself. /// public enum StateRecoveryStrategy { /// - /// Default, and the behaviour of every SDK version before this option existed. + /// Default. /// - /// The client calls /sync for the channels it was watching and replays each missed - /// event through the normal event pipeline, so every per-event callback + /// Missed events are replayed as normal events /// (, - /// , and so on) fires exactly as it - /// would for a live event. It then re-queries and re-watches those channels unconditionally, - /// which is new: previously a failed or skipped /sync left the channels stale and - /// unwatched for the rest of the connection. + /// , and so on). + /// The client also refreshes those channels and starts watching them again. /// - /// Choose this when your UI is driven by per-event callbacks. The cost is that a long outage - /// on a busy channel replays up to ~1000 events in a single frame, which is visible as a - /// hitch on mobile. + /// Use this if your UI updates from per-event callbacks. + /// After a long disconnect on a busy channel this can replay many events at once + /// and cause a hitch. /// ReplayEvents = 0, /// - /// Same recovery pipeline as , but the /sync events are - /// applied to local state without raising the per-event callbacks whose effect is observable - /// in model state afterwards. Subscribe to and - /// rebuild from and friends instead. + /// Same recovery as , but missed events update local state + /// without raising per-event callbacks. Listen to + /// and rebuild your UI from channel state + /// (, and similar). /// - /// Callbacks that carry information the SDK cannot reconstruct from state are still raised - /// per event: , - /// , and the local-user membership and invite - /// notifications. + /// Some events are still raised because they are not stored in channel state: + /// , + /// , and membership or invite notifications. /// - /// Choose this when a long outage causes a frame hitch on resume. This is the cheapest - /// recovery the SDK offers. + /// Use this if a long disconnect causes a hitch when the app resumes. /// BatchStateUpdate = 1, /// - /// The SDK performs no recovery after a reconnect: no /sync, no re-query, no re-watch, - /// and no . Local state is left exactly as it - /// was and is left untouched, so it remains - /// the list of what you were watching before the drop. + /// The SDK does not restore state after a reconnect. + /// It does not refresh channels, does not start watching them again, + /// and does not raise . + /// Local state and stay as they were + /// before the disconnect. /// - /// Choose this only if you own recovery. Subscribe to - /// , and on the transition to - /// re-hydrate and re-watch yourself with - /// QueryChannelsAsync(new[] { ChannelFilter.Cid.In(cids) }, limit: 30) - that single - /// call both refreshes state and re-establishes the watches. Note that - /// is a no-op for a channel whose - /// is still true, which is the - /// case here, so use the query. + /// Use this only if you handle recovery yourself. When the connection is + /// again, query the channels you need. + /// Example: QueryChannelsAsync(new[] { ChannelFilter.Cid.In(cids) }, limit: 30). + /// That call refreshes state and starts watching. + /// Do not use here: + /// is still true, so WatchAsync does nothing. /// Disabled = 2, } diff --git a/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs b/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs index e3c44f99..63eabffe 100644 --- a/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/IStreamChatClient.cs @@ -34,23 +34,20 @@ public interface IStreamChatClient : IDisposable, IStreamChatClientEventsListene event Action Disconnected; /// - /// Raised once after reconnect recovery finishes, on every path: full success, partial - /// success, and a reconnect where there was nothing to recover. Not raised on the initial - /// login, and not raised when - /// is + /// Raised once when reconnect recovery is done. + /// This includes full success, partial success, and cases with nothing to recover. + /// Not raised on the first login. + /// Not raised when is /// . /// - /// When it fires, the channels in - /// have fresh state and live watches - /// again. Anything in is - /// still stale and no longer watched. + /// Channels in have fresh state and are watched again. + /// Cids in are still stale and not watched. /// - /// This is the signal to rebuild from state after an outage. It is required rather than - /// merely convenient under - /// , where the per-event callbacks - /// are suppressed during recovery, and it is worth handling under - /// too, because the re-query that - /// follows the event replay merges channel state without raising per-message callbacks. + /// Use this to rebuild your UI from channel state after a disconnect. + /// You need this for , + /// because per-event callbacks do not fire during recovery. + /// It is also useful for , + /// because some state updates after reconnect do not raise per-message callbacks. /// event StateRecoveredHandler StateRecovered; @@ -151,10 +148,10 @@ public interface IStreamChatClient : IDisposable, IStreamChatClientEventsListene /// channel to know its state. /// /// - /// Emptied when the connection drops, because the server drops every watch with it, and - /// repopulated by reconnect recovery. If you enumerate this to decide what to restore - /// yourself, read it before the disconnect or use - /// , which leaves it untouched. + /// This list is cleared when the connection drops, because the server drops every watch. + /// Recovery fills it again. + /// If you read this list to restore watches yourself, do it before the disconnect, + /// or use so the list is not cleared. /// /// IReadOnlyList WatchedChannels { get; } diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs index 591cb5fb..30285713 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs @@ -3,19 +3,18 @@ namespace StreamChat.Core.LowLevelClient { /// - /// Outcome of one silent /sync history batch. See - /// . + /// Result of applying one /sync history batch. /// internal sealed class HistorySyncApplyResult { /// - /// created_at of the newest event that was applied successfully, or null when - /// nothing was applied. This is what the /sync watermark advances to. + /// created_at of the newest event that applied, or null. + /// The /sync watermark advances to this. /// public DateTimeOffset? MaxAppliedCreatedAt { get; set; } /// - /// Events that threw while being applied. They are skipped, not retried within the batch. + /// Events that threw while applying. Skipped, not retried in this batch. /// public int FailedEventCount { get; set; } } diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs index a96f7e43..f96379f4 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs @@ -448,17 +448,14 @@ public async Task FetchAndProcessEventsSinceLastReceivedEvent(IEnumerable - /// The /sync endpoint counts events summed across every requested cid against a - /// server-side ceiling of roughly 1000 and refuses the whole request once it is exceeded, so - /// asking about fewer channels makes a successful catch-up more likely. Swift and Android both - /// cap the request at 100 cids; this matches them. + /// /sync counts events across all requested cids. The server rejects the request + /// at about 1000 events. Fewer cids makes success more likely. Matches Swift and Android (100). /// internal const int MaxSyncChannelCids = 100; /// - /// Best-effort /sync for up to channels. Returns - /// null when catch-up is skipped, which happens when there is no sync point to catch up - /// from or when the sync point is older than the 30 days the server accepts. + /// Best-effort /sync for up to channels. + /// Returns null when there is no sync point, or when it is older than 30 days. /// internal async Task TrySyncHistoryAsync(IEnumerable channelCids) { @@ -474,44 +471,39 @@ public async Task FetchAndProcessEventsSinceLastReceivedEvent(IEnumerable(out var cids)) + using (new ListPoolScope(out var tempCids)) { foreach (var cid in channelCids) { - if (cids.Count == MaxSyncChannelCids) + if (tempCids.Count == MaxSyncChannelCids) { break; } - cids.Add(cid); + tempCids.Add(cid); } - if (cids.Count == 0) + if (tempCids.Count == 0) { return null; } return await ChannelApi.SyncAsync(new SyncRequest { - ChannelCids = cids, + ChannelCids = tempCids, LastSyncAt = lastEventReceivedAt, Watch = true, - // Lets the caller tell "the server will never return this channel again" apart from - // "the query happened to omit it", so recovery can stop retrying channels that were - // deleted or that the local user lost access to while offline. + // Lets recovery skip channels the server will never return again + // (deleted, or the user lost access while offline). WithInaccessibleCids = true, }); } } /// - /// Replay history events through the live event pipeline, so every per-event public callback - /// fires exactly as it would for a real-time event. This is - /// and the behaviour of - /// . + /// Replay history events as live events, so public callbacks fire the same way. + /// Used by . /// internal void ReplayHistoryEvents(IEnumerable events) { @@ -522,17 +514,15 @@ internal void ReplayHistoryEvents(IEnumerable events) foreach (var e in events) { - // Each event is isolated by the try/catch inside the registered handler, so one - // malformed event cannot abandon the rest of the replay. + // Isolated in the handler so one bad event does not stop the rest. HandleNewWebsocketMessage(SerializeHistoryEvent(e)); } } /// - /// Apply history events to local state without raising the per-event public callbacks whose - /// effect is observable in model state afterwards. This is - /// . Mirrors Android's - /// isFromHistorySync and Swift's postNotifications: false. + /// Apply history events to local state without raising per-event public callbacks. + /// Used by . + /// Matches Android isFromHistorySync and Swift postNotifications: false. /// internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) { @@ -555,9 +545,8 @@ internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) } catch (Exception ex) { - // Only count and log. Abandoning the batch would leave state half-applied, - // and the watermark below only ever advances to the newest event that was - // actually applied, so a partial batch is retried on the next reconnect. + // Do not abort the batch. The watermark only moves to events that applied, + // so a failed event is retried on the next reconnect. result.FailedEventCount++; _logs.Exception(ex); } @@ -585,9 +574,8 @@ internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) private const string HistorySyncWatermarkSource = "history.sync"; - // The batch advances the watermark once, at the end, and only to the newest event it managed - // to apply. Advancing per event would let a throwing event in the middle leave a watermark - // claiming a catch-up that did not happen. + // Advance the watermark once at the end, to the newest applied event. + // Advancing per event would skip a failed event on the next reconnect. private void RecordHistoryWatermark(DateTimeOffset createdAt) { if (createdAt == DateTimeOffset.MinValue) @@ -1136,21 +1124,19 @@ private void RegisterEventType(string key, { TryAdvanceLastEventReceivedAt(eventObj.CreatedAt, key); - // The low-level client is event-only, so it has no state a consumer could read - // after a silent batch. Suppressing its callbacks keeps a low-level subscriber - // consistent with the stateful client's BatchStateUpdate contract. + // The low-level client has no state to read after a silent batch. + // Skip its public callbacks so they match BatchStateUpdate. handler?.Invoke(eventObj, dto); } - // Always applied - this is what mutates local state. + // Updates local state even when public callbacks are skipped. internalHandler?.Invoke(dto); } catch (Exception e) { _logs.Exception(e); - // A silent batch counts its failures and advances the watermark only to the newest - // event it applied, so it needs to see the throw. Live events stay isolated here. + // ApplyHistoryEvents counts failures and needs the throw. Live events stay isolated here. if (_isApplyingHistoryEvents) { throw; @@ -1198,10 +1184,9 @@ private void HandleNewWebsocketMessage(string msg, bool isLiveEvent = false) return; } - // Stamp liveness here rather than from the health check handler: the handler runs after - // every consumer callback registered ahead of it, so a slow consumer could push the gap - // past HealthCheckMaxWaitingTime and make the client disconnect itself. Only events that - // came off the live socket count - a health check replayed from /sync proves nothing. + // Stamp here, not in the health-check handler. That handler runs after other + // callbacks; a slow one can miss the timeout and drop the connection. + // Only live socket events count. A /sync replay does not prove the socket is alive. if (isLiveEvent && type == WSEventType.HealthCheck) { _lastHealthCheckReceivedTime = _timeService.Time; @@ -1252,12 +1237,9 @@ private bool TryHandleCustomChannelEvent(string serializedContent, string eventT TryAdvanceLastEventReceivedAt(dto.CreatedAt, eventType); } - // Custom events are the one category with no representation in local state, so a - // consumer cannot reconstruct them from IStreamChannel after a silent batch. They are - // therefore delivered per event even during history sync - dropping them would be - // silent data loss, and deferring them into the recovery signal would arrive after - // the re-query and out of chronological order. The reference SDKs discard custom - // events from /sync entirely; an app porting between SDKs must not rely on this. + // Always raise. Custom events are not stored in channel state, so skipping them + // would lose the payload. JS, Swift, and Android drop custom events from /sync; + // an app that ports between SDKs must not rely on receiving them. var evt = new EventCustom(); ((ILoadableFrom)evt).LoadFromDto(dto); CustomEventReceived?.Invoke(evt); diff --git a/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs index b683b6d4..af528352 100644 --- a/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs +++ b/Assets/Plugins/StreamChat/Core/Responses/StreamStateRecoveredEventArgs.cs @@ -5,7 +5,7 @@ namespace StreamChat.Core.Responses { /// - /// Payload for . + /// Data for . /// public sealed class StreamStateRecoveredEventArgs { @@ -17,31 +17,27 @@ public StreamStateRecoveredEventArgs(IReadOnlyList channels, } /// - /// Channels whose state was refreshed and whose watch was re-established. Their - /// and other collections are up to date as of the - /// moment this event is raised. + /// Channels that were refreshed and are watched again. + /// Their and other collections are up to date when this event is raised. /// - /// Note that the recovery query returns the channel's latest page of messages and merges it - /// into what was already loaded. If more messages arrived during the outage than fit in one - /// page, the list contains the pre-disconnect messages followed by the latest page with a - /// hole in between, and cannot reach into - /// that hole because it pages back from the oldest loaded message. + /// Recovery adds the newest messages to what was already loaded. + /// If many messages arrived while you were disconnected, there can be a gap in the list. + /// cannot fill that gap, + /// because it loads from the oldest loaded message. /// public IReadOnlyList Channels { get; } /// - /// Channels that were being watched before the disconnect but could not be recovered - the - /// server no longer returns them (deleted, or the local user lost access while offline), or - /// every attempt to re-query them failed. Their local state is still stale and they are no - /// longer watched, so they will not receive realtime updates. + /// Channels that were watched before the disconnect but could not be recovered. + /// The server no longer returns them (deleted, or the user lost access), or the client could not refresh them. + /// Their local state is still stale and they are not watched. /// - /// Empty on a fully successful recovery. Use it to tear down or flag the corresponding UI - /// rather than leaving it silently frozen. + /// Empty when recovery fully succeeds. Use this to close or mark the related UI. /// public IReadOnlyList UnrecoveredChannelCids { get; } /// - /// true when every channel that was being watched before the disconnect was recovered. + /// True when every watched channel from before the disconnect was recovered. /// public bool IsComplete => UnrecoveredChannelCids.Count == 0; } diff --git a/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs b/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs index 676c162e..a46c027a 100644 --- a/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs +++ b/Assets/Plugins/StreamChat/Core/State/StreamStatefulModelBase.cs @@ -52,13 +52,13 @@ internal StreamStatefulModelBase(string uniqueId, ICacheRepository Repository { get; } /// - /// True while a /sync history batch is being applied under - /// . State mutations still run; - /// public per-event notifications must not, when their effect is observable in model state once - /// the batch finishes. Consumers observe instead. + /// True while a /sync batch is applied with + /// . + /// State still updates. Do not raise per-event callbacks when that change + /// is already visible in state. Use instead. /// - /// Notifications carrying information the SDK cannot reconstruct from state - custom events, - /// channel deletion, local-user membership and invite notifications - are raised regardless. + /// Still raise callbacks that are not stored in state: custom events, + /// channel deleted, and local-user membership or invite notifications. /// protected bool IsSilentHistorySync => Client.IsApplyingHistorySync; diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs index ff2bb789..4ef031b4 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs @@ -1063,18 +1063,14 @@ private bool InternalAppendOrUpdateMessage(MessageInternalDTO dto, out StreamMes MessageReceived?.Invoke(this, streamMessage); } - // Trim after MessageReceived so a message is never removed from cache before it is - // received. Trimming still runs during a silent history batch: it only ever removes the - // oldest contiguous prefix, which the batch is appending newer messages ahead of, and - // skipping it would let a batch push a windowed channel past its MaxMessages. + // Trim after MessageReceived so the callback sees the message first. + // Still trim during a silent batch, or a long /sync can exceed MaxMessages. TrimMessageCacheIfNeeded(); return true; } /// - /// A query response merges through UpdateFromDto, which appends to - /// without going through - /// and therefore never trims. Reconnect recovery calls this after its merge. + /// Query merge appends messages and does not trim. Recovery calls this after merge. /// internal void InternalTrimMessageCache() => TrimMessageCacheIfNeeded(); @@ -1145,9 +1141,8 @@ private void TrimMessageCacheIfNeeded() var handler = MessagesRemovedFromCache; - // Not pooled - this is handed to subscribers, so the SDK does not control its lifetime. - // Skip the allocation during a silent history batch: the callback is suppressed and a - // 1000-event /sync can trim repeatedly. + // Not pooled: subscribers keep this list. Skip it during a silent batch + // (callback is not raised, and a long /sync can trim many times). var removed = (handler == null || IsSilentHistorySync) ? null : new List(removeCount); using (new ListPoolScope(out var tempUntrackCandidates)) @@ -1173,9 +1168,8 @@ private void TrimMessageCacheIfNeeded() Cache.Messages.RemoveMany(tempUntrackCandidates); } - // Raised after the pooled buffers are returned so subscribers can trim or send safely. - // BatchStateUpdate already rebuilt from Messages on StateRecovered. Firing this - // during the batch would only destroy old rows that the rebuild is about to replace. + // After pooled lists are returned, so handlers can allocate safely. + // Do not raise during BatchStateUpdate: StateRecovered already rebuilds from Messages. if (!IsSilentHistorySync) { handler?.Invoke(this, removed); @@ -1416,8 +1410,7 @@ internal void InternalHandleCustomEvent(CustomEventInternalDTO dto) var customEvent = new StreamCustomEvent(dto.Type, user, dto.CreatedAt, new StreamCustomData(custom, Serializer)); - // Deliberately not gated on IsSilentHistorySync: a custom event has no representation in - // channel state, so suppressing it would lose the payload with no way to recover it. + // Always raise. Custom events are not stored in channel state, so skipping them would lose the payload. CustomEventReceived?.Invoke(this, customEvent); } diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index a087c040..f262a8ce 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -53,8 +53,6 @@ namespace StreamChat.Core /// public delegate void ChannelDeleteHandler(string channelCid, string channelId, ChannelType channelType); - //StreamTodo: Handle restoring state after lost connection - public delegate void ChannelInviteHandler(IStreamChannel channel, IStreamUser invitee); /// @@ -228,8 +226,7 @@ public Task DisconnectUserAsync() { TryCancelWaitingForUserConnection(); - // Ends the session, so the next Connected transition is a fresh login rather than a - // reconnect and must not run recovery or raise StateRecovered. + // End the session so the next Connected is a new login, not a reconnect. _hasConnectedBefore = false; return InternalLowLevelClient.DisconnectAsync(permanent: true); @@ -937,15 +934,13 @@ internal Task RefreshChannelState(string cid) private readonly List _watchedChannels = new List(); /// - /// Cids that were being watched when the connection dropped, most recently active first. - /// Reconnect recovery restores state and watches from this, not from - /// , which is cleared on the disconnect. + /// Cids watched when the connection dropped, newest activity first. + /// Recovery uses this because is cleared on disconnect. /// private readonly List _recoveryChannelCids = new List(); /// - /// Cids the /sync response reported as inaccessible - deleted, or no longer readable by - /// the local user. Never re-queried again for the lifetime of the client. + /// Cids /sync reported as inaccessible. Never queried again for this client. /// private readonly HashSet _inaccessibleCids = new HashSet(); @@ -953,20 +948,17 @@ internal Task RefreshChannelState(string cid) private bool _hasConnectedBefore; /// - /// Recovering more channels than this would mean an unbounded number of sequential queries on - /// every reconnect, which invites rate limiting. Matches the /sync cid cap so both - /// halves of recovery cover the same set. JS and Android both cap lower, at 30. + /// Cap so reconnect does not run an unbounded number of queries. + /// Same cap as /sync. JS and Android cap at 30. /// internal const int MaxRecoveredChannels = StreamChatLowLevelClient.MaxSyncChannelCids; /// - /// QueryChannelsAsync asserts a limit of at most 30, so a longer recovery set has to be - /// chunked. Same chunk size as . + /// QueryChannelsAsync allows at most 30 channels per request. /// private const int MaxChannelsPerRecoveryQuery = 30; - // Ties are broken arbitrarily - List.Sort is unstable - which only matters for channels with - // an equal LastMessageAt, where there is no meaningful "more recently active" anyway. + // List.Sort is unstable. Ties only happen when LastMessageAt is equal. private static readonly Comparison ByLastMessageAtDescending = (a, b) => (b.LastMessageAt ?? DateTimeOffset.MinValue).CompareTo(a.LastMessageAt ?? DateTimeOffset.MinValue); @@ -1028,10 +1020,8 @@ private void InternalDeleteChannel(StreamChannel channel) ChannelDeleted?.Invoke(channel.Cid, channel.Id, channel.Type); } - // Flip IsWatched=true and add to _watchedChannels. Call from every path that issued - // Watch=true to the server. Channels that land in the cache via non-watching paths - // (search hits, threads with Watch=false, ban-info / mute payloads) stay IsWatched=false. - // Idempotent: a no-op when the channel is already watched. + // Call from every path that sent Watch=true to the server. + // Search hits, threads with Watch=false, and similar cache-only paths stay unwatched. private void MarkChannelWatched(StreamChannel channel) { if (channel == null || channel.IsWatched) @@ -1043,8 +1033,7 @@ private void MarkChannelWatched(StreamChannel channel) _watchedChannels.Add(channel); } - // Counterpart to MarkChannelWatched. Called from StreamChannel.StopWatchingAsync - // after the server confirms the unwatch. Idempotent. + // Called from StreamChannel.StopWatchingAsync after the server confirms. internal void InternalMarkChannelUnwatched(StreamChannel channel) { if (channel == null) @@ -1052,8 +1041,8 @@ internal void InternalMarkChannelUnwatched(StreamChannel channel) return; } - // Drop it from the recovery snapshot as well, or an unwatch performed while disconnected - // would be undone by the next reconnect re-watching it. + // Also remove it from the reconnect recovery list. If the user stopped watching + // while disconnected, recovery would otherwise start watching this channel again. _recoveryChannelCids.Remove(channel.Cid); if (!channel.IsWatched) @@ -1163,40 +1152,19 @@ private void OnConnected(HealthCheckEventInternalDTO dto) } /// - /// Watches are bound to a websocket connection, and a reconnect always gets a new one, so a - /// reconnected client is watching nothing until something re-watches for it. Without this the - /// channels stay in local state but stop receiving events - the chat looks alive and silently - /// never updates again. - /// - /// This holds no matter how briefly the socket was down. The handshake payload - /// (ConnectPayload) carries only the user and token plus - /// server_determines_connection_id; there is no session or resume token, so the client - /// cannot ask the server to continue a previous connection, and the server mints a fresh - /// connection_id that every subsequent request is then tagged with. Do not confuse this - /// with the server-side health check grace period: that governs when the server notices a - /// silently dropped socket, which affects presence and the cleanup of the stale watcher entry. - /// It does not hand the old connection's watches to the new one - if anything a fast reconnect - /// is the worse case, because for a while the channel counts you as a watcher twice while the - /// connection you are actually reading receives nothing. - /// - /// Runs after every reconnect, in this order: + /// After reconnect the new websocket has no watches. Without this, channels stay + /// in local state but stop receiving events. /// - /// 1. /sync catch-up, best effort. This is what makes a short outage recover with no - /// hole in the message list. It has to run first because a replayed channel.truncated - /// wipes the local message list, and doing that after step 2 would discard the page step 2 - /// just fetched. - /// 2. Re-query and re-watch, unconditionally, whatever step 1 did. One query per 30 cids, with - /// State and Watch set, so a single request both re-hydrates and re-watches. + /// 1. /sync catch-up (best effort). Must run first: a replayed channel.truncated + /// would wipe messages fetched in step 2. + /// 2. Re-query and re-watch, even if step 1 failed or was skipped. /// 3. Raise once. /// - /// Steps 1 and 2 are individually fault-tolerant: a failure in one channel or one request must - /// not abandon the others, because this is the only recovery this reconnect gets. + /// A failure in one channel or request must not stop the others. /// private async Task RestoreStateLostDuringDisconnect() { - // A fresh login is not a recovery: there is no prior state to restore and no consumer - // expects a recovery signal for it. Anything left in the snapshot belongs to the previous - // session, and possibly to a different user, so drop it. + // First login is not a recovery. Clear any leftover snapshot from a previous session. if (!_hasConnectedBefore) { _hasConnectedBefore = true; @@ -1212,26 +1180,23 @@ private async Task RestoreStateLostDuringDisconnect() var generation = ++_recoveryGeneration; - // Pooled - never leaves this method and the steps below only read it. The two collections - // handed to StateRecovered are not pooled, because subscribers keep them for as long as - // they like. - using (new ListPoolScope(out var recoverSet)) + using (new ListPoolScope(out var tempRecoverSet)) { - FillRecoverySet(recoverSet); + FillRecoverySet(tempRecoverSet); var refreshedChannels = new List(); try { - if (recoverSet.Count > 0) + if (tempRecoverSet.Count > 0) { - await TryCatchUpWithHistoryAsync(recoverSet, generation); + await TryCatchUpWithHistoryAsync(tempRecoverSet, generation); if (!IsRecoveryGenerationCurrent(generation)) { return; } - await RehydrateAndRewatchChannelsAsync(recoverSet, generation, refreshedChannels); + await RehydrateAndRewatchChannelsAsync(tempRecoverSet, generation, refreshedChannels); if (!IsRecoveryGenerationCurrent(generation)) { return; @@ -1240,8 +1205,8 @@ private async Task RestoreStateLostDuringDisconnect() } catch (Exception e) { - // Defence in depth - every step already handles its own failures. Whatever happened, - // the consumer still gets told that recovery finished and which channels are stale. + // Each step already handles its own errors. Still raise StateRecovered so the + // app knows recovery finished and which channels are stale. _logs.Exception(e); } @@ -1252,18 +1217,18 @@ private async Task RestoreStateLostDuringDisconnect() var unrecovered = new List(); - using (new HashSetPoolScope(out var recovered)) + using (new HashSetPoolScope(out var tempRecovered)) { for (var i = 0; i < refreshedChannels.Count; i++) { - recovered.Add(refreshedChannels[i].Cid); + tempRecovered.Add(refreshedChannels[i].Cid); } - for (var i = 0; i < recoverSet.Count; i++) + for (var i = 0; i < tempRecoverSet.Count; i++) { - if (!recovered.Contains(recoverSet[i])) + if (!tempRecovered.Contains(tempRecoverSet[i])) { - unrecovered.Add(recoverSet[i]); + unrecovered.Add(tempRecoverSet[i]); } } } @@ -1281,8 +1246,7 @@ private async Task RestoreStateLostDuringDisconnect() } /// - /// Copy the most recently active cids from the snapshot - /// captured on the disconnect into . + /// Copy up to cids from the disconnect snapshot. /// private void FillRecoverySet(List recoverSet) { @@ -1309,10 +1273,7 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, { var response = await InternalLowLevelClient.TrySyncHistoryAsync(recoverSet); - // null means the catch-up was skipped: no sync point, or one older than the 30 days - // the server accepts. Both used to return before any recovery ran; now step 2 still - // runs, which is the whole point of making it unconditional. That is not the same as - // the server naming inaccessible cids - do not treat a skip as a deletion. + // No sync point, or older than 30 days. Step 2 still runs. This is not a deletion. if (response == null) { return; @@ -1325,10 +1286,8 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, if (response.InaccessibleCids != null) { - // The server is telling us these will never come back. Recording them keeps the - // re-query from asking about deleted channels and stops us reporting them as a - // recovery failure every reconnect. Must run even when Events is empty - a - // deleted channel can be the only thing /sync has to say. + // These channels will never come back. Record them so we do not query them again. + // Must run even when Events is empty. foreach (var cid in response.InaccessibleCids) { _inaccessibleCids.Add(cid); @@ -1351,11 +1310,7 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, } catch (StreamApiException ex) when (ex.IsInputError()) { - // HTTP 400 / code 4, "too many events to sync". The server counts events summed across - // every requested cid against a ceiling of roughly 1000 and refuses the whole request, - // so this is the normal outcome of a long outage on busy channels, not an anomaly. - // The re-query below is the fallback and recovers the same state minus the events that - // did not fit in the latest page. + // Too many events to sync. The re-query below recovers state without those events. _logs.Warning("The /sync catch-up was refused because too many events accumulated during the outage. " + "Recovering channel state with a re-query instead. " + ex.Message); } @@ -1368,17 +1323,11 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, /// /// Re-hydrate and re-watch in one request per cids. + /// Unlike Android, we do not re-watch omitted cids one by one. This query is by cid, + /// so a missing cid is deleted or no longer readable. A get-or-create watch would + /// recreate a deleted channel. Those cids go to + /// . /// - /// - /// Unlike Android, this does not follow up with a per-channel re-watch for cids the query did - /// not return. Android needs that because it recovers through the customer's own channel-list - /// queries, which need not cover every active cid; this queries the recovery set by cid, so it - /// is exhaustive by construction. A cid the query omits is one the server will not return at - /// all - deleted, or no longer readable - and the only per-channel watch primitive available - /// is get-or-create, which would recreate a channel that was deleted while we were offline. - /// Such cids are reported through - /// instead. - /// private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recoverSet, int generation, List refreshed) { @@ -1391,43 +1340,38 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove return; } - // Released only once the query has completed: the filter holds this list and the - // request body is serialized from it. - using (new ListPoolScope(out var chunk)) + using (new ListPoolScope(out var tempChunk)) { var chunkEnd = Math.Min(i + MaxChannelsPerRecoveryQuery, recoverSet.Count); for (var j = i; j < chunkEnd; j++) { if (!_inaccessibleCids.Contains(recoverSet[j])) { - chunk.Add(recoverSet[j]); + tempChunk.Add(recoverSet[j]); } } - if (chunk.Count == 0) + if (tempChunk.Count == 0) { continue; } var filters = new IFieldFilterRule[] { - ChannelFilter.Cid.In(chunk), + ChannelFilter.Cid.In(tempChunk), }; QueryChannelsResponseInternalDTO response; try { - // Fetch only. Public QueryChannelsAsync applies immediately (members/read/pinned - // are replaced, watches marked). A request started on an old connection can still - // succeed after a reconnect, so apply must wait for the generation check below. - response = await FetchQueryChannelsResponseAsync(filters, sort, chunk.Count); + // Fetch only. Apply after the generation check so a late response from an old + // connection cannot overwrite newer members, read, or pinned messages. + response = await FetchQueryChannelsResponseAsync(filters, sort, tempChunk.Count); } catch (Exception e) { - // One failed chunk (a rate limit part-way through a long watch list, a channel - // torn down while offline) must not cost the remaining chunks their recovery - - // there is no later retry this connection. - _logs.Warning($"Recovery query failed for {chunk.Count} channel(s). Continuing with the rest. " + + // Do not skip the rest. There is no later retry on this connection. + _logs.Warning($"Recovery query failed for {tempChunk.Count} channel(s). Continuing with the rest. " + e.Message); continue; } @@ -1451,8 +1395,7 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove } MarkChannelWatched(channel); - // The query merge path goes through UpdateFromDto, which does not trim, so a - // recovery merge can push Messages past MessageCacheWindow.MaxMessages. + // Query merge does not trim, so Messages can exceed MessageCacheWindow. channel.InternalTrimMessageCache(); refreshed.Add(channel); } @@ -1461,9 +1404,8 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove } /// - /// Same request as public - /// but does not touch _cache or _watchedChannels. Recovery uses this so a stale - /// in-flight response cannot overwrite newer state or mark watches for a dropped connection. + /// Same request as public QueryChannelsAsync, but does not update cache or watches. + /// A late response from an old connection must not overwrite newer state. /// private Task FetchQueryChannelsResponseAsync( IEnumerable filters, ChannelSortObject sort, int limit, int offset = 0) @@ -1498,16 +1440,12 @@ private static QueryChannelsRequestInternalDTO CreateQueryChannelsRequest( private bool IsRecoveryGenerationCurrent(int generation) => generation == _recoveryGeneration; /// - /// Capture what was being watched when the connection dropped, then stop claiming those - /// watches: the server has dropped them, so would - /// otherwise report watches that no longer exist. Recovery restores both from the snapshot. + /// Save watched cids, then mark them unwatched. The server already dropped the watches. /// private void SnapshotRecoverySetAndClearWatches() { - // A reconnect attempt that fails transitions Connecting -> Disconnected again, and by then - // the watch list is already empty. Overwriting the snapshot at that point would throw away - // the only record of what needs recovering, and the reconnect that eventually succeeds - // would restore nothing at all - which is exactly the flaky-mobile-network case. + // A failed reconnect also hits Disconnected with an empty watch list. + // Do not overwrite the snapshot, or the next success recovers nothing. if (_watchedChannels.Count == 0) { return; @@ -1515,14 +1453,14 @@ private void SnapshotRecoverySetAndClearWatches() _recoveryChannelCids.Clear(); - using (new ListPoolScope(out var ordered)) + using (new ListPoolScope(out var tempOrdered)) { - ordered.AddRange(_watchedChannels); - ordered.Sort(ByLastMessageAtDescending); + tempOrdered.AddRange(_watchedChannels); + tempOrdered.Sort(ByLastMessageAtDescending); - for (var i = 0; i < ordered.Count; i++) + for (var i = 0; i < tempOrdered.Count; i++) { - _recoveryChannelCids.Add(ordered[i].Cid); + _recoveryChannelCids.Add(tempOrdered[i].Cid); } } @@ -1540,10 +1478,8 @@ private void OnConnectionStateChanged(ConnectionState previous, ConnectionState { if (current == ConnectionState.Disconnected) { - // Supersede any recovery still in flight before its responses can land on top of the - // state the next recovery is about to fetch. Some channel fields (read state, members, - // pinned messages) are replaced wholesale by a query response rather than merged, so a - // late response is not merely redundant, it can overwrite newer state. + // Ignore in-flight recovery. A late query can replace members, read state, + // and pinned messages with older data. _recoveryGeneration++; if (InternalLowLevelClient.Config.StateRecoveryStrategy != StateRecoveryStrategy.Disabled) @@ -2003,8 +1939,8 @@ var reaction } } - // Who is currently watching is live presence, like typing: replaying it would leave watchers - // listed who left during the outage. The recovery query returns the authoritative watcher set. + // Watcher lists are live presence. Replaying them would show people who left during the outage. + // The recovery query returns the current watcher set. private void OnUserWatchingStop(UserWatchingStopEventInternalDTO eventDto) { if (IsApplyingHistorySync) @@ -2065,9 +2001,8 @@ private void OnUserPresenceChanged(UserPresenceChangedEventInternalDTO eventDto) private void OnTypingStopped(TypingStopEventInternalDTO eventDto) { - // Typing is live presence with no meaning in a history replay, and applying it is not - // merely redundant but wrong: a typing.start whose matching typing.stop fell outside the - // synced window would leave a user typing forever. Skipped entirely, state included. + // Typing is live presence. A typing.start without its matching typing.stop would leave + // a user typing forever. Skip the whole event, including state. if (IsApplyingHistorySync) { return; From 5b31bfd2c28b30bf594aaa10d0b58b1d3e62febc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:25:40 +0200 Subject: [PATCH 06/10] Fix some events still being triggered during silent batch update --- .../Core/StatefulModels/StreamChannel.cs | 26 ++- .../Core/StatefulModels/StreamPoll.cs | 13 +- .../Core/StatefulModels/StreamThread.cs | 26 ++- .../Core/StatefulModels/StreamUser.cs | 2 +- .../StateSync/StateRecoveryClientTests.cs | 180 ++++++++++++++++++ 5 files changed, 230 insertions(+), 17 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs index 4ef031b4..6406721c 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamChannel.cs @@ -1334,7 +1334,10 @@ internal void InternalHandleUserWatchingStartEvent(UserWatchingStartEventInterna { WatcherCount += 1; _watchers.Add(user); - WatcherAdded?.Invoke(this, user); + if (!IsSilentHistorySync) + { + WatcherAdded?.Invoke(this, user); + } } } @@ -1351,7 +1354,11 @@ internal void InternalHandleUserWatchingStop(UserWatchingStopEventInternalDTO ev { var user = Cache.TryCreateOrUpdate(eventDto.User, out var wasCreated); _watchers.RemoveAt(i); - WatcherRemoved?.Invoke(this, user); + if (!IsSilentHistorySync) + { + WatcherRemoved?.Invoke(this, user); + } + return; } } @@ -1368,8 +1375,12 @@ internal void InternalHandleTypingStopped(TypingStopEventInternalDTO eventDto) if (_typingUsers[i].Id == eventDto.User.Id) { _typingUsers.RemoveAt(i); - UserStoppedTyping?.Invoke(this, user); - TypingUsersChanged?.Invoke(this); + if (!IsSilentHistorySync) + { + UserStoppedTyping?.Invoke(this, user); + TypingUsersChanged?.Invoke(this); + } + return; } } @@ -1384,8 +1395,11 @@ internal void InternalHandleTypingStarted(TypingStartEventInternalDTO eventDto) if (!_typingUsers.ContainsNoAlloc(user)) { _typingUsers.Add(user); - UserStartedTyping?.Invoke(this, user); - TypingUsersChanged?.Invoke(this); + if (!IsSilentHistorySync) + { + UserStartedTyping?.Invoke(this, user); + TypingUsersChanged?.Invoke(this); + } } } diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamPoll.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamPoll.cs index df4864ae..de7a8516 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamPoll.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamPoll.cs @@ -55,7 +55,7 @@ private set _isClosed = value; - if (value == true) + if (value && !IsSilentHistorySync) { Closed?.Invoke(this); } @@ -275,7 +275,10 @@ void IUpdateableFrom.UpdateFromDto(Poll LoadAdditionalProperties(dto.Custom); - Updated?.Invoke(this); + if (!IsSilentHistorySync) + { + Updated?.Invoke(this); + } } internal void HandlePollClosedEvent(PollClosedEventInternalDTO dto) @@ -292,7 +295,7 @@ internal void HandlePollVoteCastedEvent(PollVoteCastedEventInternalDTO dto) { this.TryUpdateFromDto(dto.Poll, Cache); - if (dto.PollVote != null) + if (dto.PollVote != null && !IsSilentHistorySync) { var vote = new StreamPollVote().TryLoadFromDto(dto.PollVote, Cache); VoteCasted?.Invoke(this, vote); @@ -303,7 +306,7 @@ internal void HandlePollVoteChangedEvent(PollVoteChangedEventInternalDTO dto) { this.TryUpdateFromDto(dto.Poll, Cache); - if (dto.PollVote != null) + if (dto.PollVote != null && !IsSilentHistorySync) { var vote = new StreamPollVote().TryLoadFromDto(dto.PollVote, Cache); VoteChanged?.Invoke(this, vote); @@ -314,7 +317,7 @@ internal void HandlePollVoteRemovedEvent(PollVoteRemovedEventInternalDTO dto) { this.TryUpdateFromDto(dto.Poll, Cache); - if (dto.PollVote != null) + if (dto.PollVote != null && !IsSilentHistorySync) { var vote = new StreamPollVote().TryLoadFromDto(dto.PollVote, Cache); VoteRemoved?.Invoke(this, vote); diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs index b659dd81..2d478614 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamThread.cs @@ -195,7 +195,10 @@ void IUpdateableFrom.UpdateFromDto // and stream-chat-js (constructCustomDataObject collects non-reserved top-level fields). LoadAdditionalProperties(dto.AdditionalProperties); - Updated?.Invoke(this); + if (!IsSilentHistorySync) + { + Updated?.Invoke(this); + } } // ChannelStateResponse[Fields]InternalDTO carries threads as ThreadStateInternalDTO @@ -259,7 +262,10 @@ void IUpdateableFrom3.UpdateFromDto( // and stream-chat-js (constructCustomDataObject collects non-reserved top-level fields). LoadAdditionalProperties(dto.AdditionalProperties); - Updated?.Invoke(this); + if (!IsSilentHistorySync) + { + Updated?.Invoke(this); + } } void IUpdateableFrom2.UpdateFromDto( @@ -307,7 +313,10 @@ void IUpdateableFrom2.UpdateFromDto( // and stream-chat-js (constructCustomDataObject collects non-reserved top-level fields). LoadAdditionalProperties(dto.AdditionalProperties); - Updated?.Invoke(this); + if (!IsSilentHistorySync) + { + Updated?.Invoke(this); + } } internal StreamThread(string uniqueId, ICacheRepository repository, @@ -332,7 +341,11 @@ internal void HandleNewReply(IStreamMessage reply) var isInsert = !_latestReplies.Contains(streamReply); if (!isInsert) { - ReplyReceived?.Invoke(this, reply); + if (!IsSilentHistorySync) + { + ReplyReceived?.Invoke(this, reply); + } + return; } @@ -355,7 +368,10 @@ internal void HandleNewReply(IStreamMessage reply) UpsertReplySenderAsParticipant(streamReply); IncrementUnreadForOtherReaders(streamReply); - ReplyReceived?.Invoke(this, reply); + if (!IsSilentHistorySync) + { + ReplyReceived?.Invoke(this, reply); + } } internal void HandleReplyDeleted(string messageId, bool isHardDelete) diff --git a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamUser.cs b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamUser.cs index edf721ea..8b209dd7 100644 --- a/Assets/Plugins/StreamChat/Core/StatefulModels/StreamUser.cs +++ b/Assets/Plugins/StreamChat/Core/StatefulModels/StreamUser.cs @@ -48,7 +48,7 @@ private set var prev = _online; _online = value; - if (prev != value) + if (prev != value && !IsSilentHistorySync) { PresenceChanged?.Invoke(this, Online, LastActive); } diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index a3c932d6..2e728642 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -10,6 +10,7 @@ using NUnit.Framework; using StreamChat.Core; using StreamChat.Core.Configs; +using StreamChat.Core.InternalDTO.Models; using StreamChat.Core.InternalDTO.Responses; using StreamChat.Core.LowLevelClient; using StreamChat.Core.Responses; @@ -394,6 +395,130 @@ public void when_history_replay_trims_expect_messages_removed_from_cache_raised( Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count); } + [Test] + public void when_silent_history_batch_adds_thread_reply_expect_reply_received_not_raised() + { + Connect(); + WatchChannel("messaging:a"); + var thread = TrackThread("messaging:a", "parent-1"); + + var replyCount = 0; + thread.ReplyReceived += (_, __) => replyCount++; + + _client.InternalLowLevelClient.ApplyHistoryEvents(new[] + { + ThreadReplyJson("messaging:a", "parent-1", "reply-1"), + }); + + Assert.AreEqual(0, replyCount, + "BatchStateUpdate must not fire ReplyReceived; the UI rebuilds on StateRecovered."); + Assert.AreEqual(1, thread.LatestReplies.Count); + Assert.AreEqual("reply-1", thread.LatestReplies[0].Id); + } + + [Test] + public void when_history_replay_adds_thread_reply_expect_reply_received_raised() + { + Connect(); + WatchChannel("messaging:a"); + var thread = TrackThread("messaging:a", "parent-1"); + + var replyCount = 0; + thread.ReplyReceived += (_, __) => replyCount++; + + _client.InternalLowLevelClient.ReplayHistoryEvents(new[] + { + ThreadReplyJson("messaging:a", "parent-1", "reply-1"), + }); + + Assert.AreEqual(1, replyCount, + "ReplayEvents is the default and must keep raising ReplyReceived for back-compat."); + Assert.AreEqual(1, thread.LatestReplies.Count); + Assert.AreEqual("reply-1", thread.LatestReplies[0].Id); + } + + [Test] + public void when_silent_history_batch_changes_presence_expect_presence_changed_not_raised() + { + Connect(); + var user = TrackUser("other-user", online: false); + + var presenceCount = 0; + user.PresenceChanged += (_, __, ___) => presenceCount++; + + _client.InternalLowLevelClient.ApplyHistoryEvents(new[] + { + PresenceChangedJson("other-user", online: true), + }); + + Assert.AreEqual(0, presenceCount, + "BatchStateUpdate must not fire PresenceChanged; the UI rebuilds on StateRecovered."); + Assert.IsTrue(user.Online); + } + + [Test] + public void when_history_replay_changes_presence_expect_presence_changed_raised() + { + Connect(); + var user = TrackUser("other-user", online: false); + + var presenceCount = 0; + user.PresenceChanged += (_, __, ___) => presenceCount++; + + _client.InternalLowLevelClient.ReplayHistoryEvents(new[] + { + PresenceChangedJson("other-user", online: true), + }); + + Assert.AreEqual(1, presenceCount, + "ReplayEvents is the default and must keep raising PresenceChanged for back-compat."); + Assert.IsTrue(user.Online); + } + + [Test] + public void when_silent_history_batch_closes_poll_expect_closed_not_raised() + { + Connect(); + WatchChannel("messaging:a"); + var poll = TrackPoll("poll-1"); + + var closedCount = 0; + var updatedCount = 0; + poll.Closed += _ => closedCount++; + poll.Updated += _ => updatedCount++; + + _client.InternalLowLevelClient.ApplyHistoryEvents(new[] + { + PollClosedJson("messaging:a", "poll-1"), + }); + + Assert.AreEqual(0, closedCount, + "BatchStateUpdate must not fire Closed; the UI rebuilds on StateRecovered."); + Assert.AreEqual(0, updatedCount, + "BatchStateUpdate must not fire Updated during a silent poll DTO apply."); + Assert.IsTrue(poll.IsClosed); + } + + [Test] + public void when_history_replay_closes_poll_expect_closed_raised() + { + Connect(); + WatchChannel("messaging:a"); + var poll = TrackPoll("poll-1"); + + var closedCount = 0; + poll.Closed += _ => closedCount++; + + _client.InternalLowLevelClient.ReplayHistoryEvents(new[] + { + PollClosedJson("messaging:a", "poll-1"), + }); + + Assert.AreEqual(1, closedCount, + "ReplayEvents is the default and must keep raising Closed for back-compat."); + Assert.IsTrue(poll.IsClosed); + } + private const string SyncEndpoint = "/sync"; private const string QueryChannelsEndpoint = "/channels"; @@ -417,6 +542,61 @@ private static string MessageNewJson(string cid, string messageId, DateTimeOffse $"\"message\":{{\"id\":\"{messageId}\",\"text\":\"hi\",\"created_at\":\"{createdAt:O}\"," + $"\"updated_at\":\"{createdAt:O}\",\"user\":{{\"id\":\"user-1\"}}}}}}"; + private static string ThreadReplyJson(string cid, string parentId, string replyId) + { + var createdAt = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + return $"{{\"type\":\"message.new\",\"cid\":\"{cid}\",\"created_at\":\"{createdAt:O}\"," + + $"\"message\":{{\"id\":\"{replyId}\",\"parent_id\":\"{parentId}\",\"text\":\"reply\"," + + $"\"created_at\":\"{createdAt:O}\",\"updated_at\":\"{createdAt:O}\"," + + $"\"user\":{{\"id\":\"user-1\"}}}}}}"; + } + + private static string PresenceChangedJson(string userId, bool online) + { + var createdAt = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + var onlineJson = online ? "true" : "false"; + return $"{{\"type\":\"user.presence.changed\",\"created_at\":\"{createdAt:O}\"," + + $"\"user\":{{\"id\":\"{userId}\",\"online\":{onlineJson}}}}}"; + } + + private static string PollClosedJson(string cid, string pollId) + { + var createdAt = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + return $"{{\"type\":\"poll.closed\",\"cid\":\"{cid}\",\"created_at\":\"{createdAt:O}\"," + + $"\"poll\":{{\"id\":\"{pollId}\",\"name\":\"q\",\"is_closed\":true,\"vote_count\":0," + + $"\"voting_visibility\":\"public\"}}}}"; + } + + private IStreamThread TrackThread(string cid, string parentMessageId) + => _client.InternalCache.TryCreateOrUpdate(new ThreadStateInternalDTO + { + ParentMessageId = parentMessageId, + ChannelCid = cid, + ReplyCount = 0, + ParentMessage = new MessageInternalDTO + { + Id = parentMessageId, + Text = "parent", + User = new UserObjectInternalDTO { Id = "user-1" }, + }, + }); + + private IStreamUser TrackUser(string userId, bool online) + => _client.InternalCache.TryCreateOrUpdate(new UserObjectInternalDTO + { + Id = userId, + Online = online, + }); + + private IStreamPoll TrackPoll(string pollId) + => _client.InternalCache.TryCreateOrUpdate(new PollResponseDataInternalDTO + { + Id = pollId, + Name = "q", + IsClosed = false, + VoteCount = 0, + }); + private void RespondWith(string endpointSuffix, string json) { _mockHttpClient From 243b0abb092a303c30a75f0dd8c8e67f967ea0a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:37:14 +0200 Subject: [PATCH 07/10] Add more tests --- .../StateSync/StateRecoveryClientTests.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index 2e728642..5f00d633 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -519,6 +519,82 @@ public void when_history_replay_closes_poll_expect_closed_raised() Assert.IsTrue(poll.IsClosed); } + [Test] + public void when_silent_recovery_syncs_message_new_expect_channel_message_received_not_raised() + { + _config.StateRecoveryStrategy = StateRecoveryStrategy.BatchStateUpdate; + + Connect(); + var channel = WatchChannel("messaging:a"); + RespondWith(QueryChannelsEndpoint, QueryChannelsJson("messaging:a", "a")); + + var receivedCount = 0; + channel.MessageReceived += (_, __) => receivedCount++; + + ReconnectWithSync(SyncEventsJson(MessageNewJson("messaging:a", "msg-offline", + new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero)))); + + Assert.AreEqual(0, receivedCount, + "BatchStateUpdate must not fire IStreamChannel.MessageReceived; rebuild on StateRecovered."); + Assert.AreEqual(1, channel.Messages.Count); + Assert.AreEqual("msg-offline", channel.Messages[0].Id); + Assert.AreEqual(1, _recoveredEvents.Count); + } + + [Test] + public void when_replay_recovery_syncs_message_new_expect_channel_message_received_raised() + { + Connect(); + var channel = WatchChannel("messaging:a"); + RespondWith(QueryChannelsEndpoint, QueryChannelsJson("messaging:a", "a")); + + var receivedCount = 0; + channel.MessageReceived += (_, __) => receivedCount++; + + ReconnectWithSync(SyncEventsJson(MessageNewJson("messaging:a", "msg-offline", + new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero)))); + + Assert.AreEqual(1, receivedCount, + "ReplayEvents is the default and must keep raising IStreamChannel.MessageReceived for back-compat."); + Assert.AreEqual(1, channel.Messages.Count); + Assert.AreEqual("msg-offline", channel.Messages[0].Id); + Assert.AreEqual(1, _recoveredEvents.Count); + } + + [Test] + public void when_silent_history_batch_contains_custom_event_expect_channel_custom_event_received() + { + Connect(); + var channel = WatchChannel("messaging:a"); + + string receivedType = null; + channel.CustomEventReceived += (_, evt) => receivedType = evt.Type; + + _client.InternalLowLevelClient.ApplyHistoryEvents(new[] + { + CustomEventJson("messaging:a", "game.state"), + }); + + Assert.AreEqual("game.state", receivedType, + "Custom events have no state representation, so ApplyHistoryEvents must still deliver them."); + } + + [Test] + public void when_recovery_query_sent_expect_watch_and_state_true() + { + Connect(); + WatchChannel("messaging:a"); + + DropConnection(); + Reconnect(); + + _mockHttpClient.Received().SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), + Arg.Is(body => RequestHasJsonBool(body, "watch", true) + && RequestHasJsonBool(body, "state", true))); + } + private const string SyncEndpoint = "/sync"; private const string QueryChannelsEndpoint = "/channels"; @@ -567,6 +643,30 @@ private static string PollClosedJson(string cid, string pollId) $"\"voting_visibility\":\"public\"}}}}"; } + private static string CustomEventJson(string cid, string type) + { + var createdAt = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + return $"{{\"type\":\"{type}\",\"cid\":\"{cid}\",\"created_at\":\"{createdAt:O}\"," + + "\"user\":{\"id\":\"user-1\"}}"; + } + + private static string SyncEventsJson(params string[] events) + => "{\"events\":[" + string.Join(",", events) + "]}"; + + private void ReconnectWithSync(string syncJson) + { + var now = new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero); + _mockTimeService.Now.Returns(now); + + DropConnection(); + // Health checks in these tests carry no created_at, so disconnect would otherwise leave + // no sync point and skip /sync entirely. Seed one after the drop so the copy on + // Disconnected cannot wipe it. + SetDisconnectionLastEventReceivedAt(_client.InternalLowLevelClient, now.AddHours(-1)); + RespondWith(SyncEndpoint, syncJson); + Reconnect(); + } + private IStreamThread TrackThread(string cid, string parentMessageId) => _client.InternalCache.TryCreateOrUpdate(new ThreadStateInternalDTO { @@ -738,6 +838,13 @@ private static bool RequestBodyContains(object requestBody, string value) return json.IndexOf(value, StringComparison.Ordinal) >= 0; } + private static bool RequestHasJsonBool(object requestBody, string property, bool value) + { + var json = requestBody as string ?? requestBody?.ToString() ?? string.Empty; + var needle = "\"" + property + "\":" + (value ? "true" : "false"); + return json.IndexOf(needle, StringComparison.Ordinal) >= 0; + } + private const string HealthCheckJson = "{\"connection_id\":\"fakeId\",\"type\":\"health.check\"}"; private readonly Queue _pendingWebsocketMessages = new Queue(); From 56e6691696b8d5d06dd293da83f845440717a1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Mon, 24 Aug 2026 21:21:13 +0200 Subject: [PATCH 08/10] remove uncecessary internal serialization step + review fixes --- .../LowLevelClient/HistorySyncApplyResult.cs | 2 +- .../StreamChatLowLevelClient.cs | 192 ++++++++++++++---- .../StreamChat/Core/StreamChatClient.cs | 34 ++-- .../StateSync/StateRecoveryLowLevelTests.cs | 64 ++++++ 4 files changed, 231 insertions(+), 61 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs index 30285713..a1a4419c 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/HistorySyncApplyResult.cs @@ -9,7 +9,7 @@ internal sealed class HistorySyncApplyResult { /// /// created_at of the newest event that applied, or null. - /// The /sync watermark advances to this. + /// The /sync last_sync_at cursor advances to this. /// public DateTimeOffset? MaxAppliedCreatedAt { get; set; } diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs index f96379f4..e52261a5 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs @@ -515,7 +515,7 @@ internal void ReplayHistoryEvents(IEnumerable events) foreach (var e in events) { // Isolated in the handler so one bad event does not stop the rest. - HandleNewWebsocketMessage(SerializeHistoryEvent(e)); + HandleNewWebsocketMessage(e); } } @@ -541,12 +541,12 @@ internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) { try { - HandleNewWebsocketMessage(SerializeHistoryEvent(e)); + HandleNewWebsocketMessage(e); } catch (Exception ex) { - // Do not abort the batch. The watermark only moves to events that applied, - // so a failed event is retried on the next reconnect. + // Do not abort the batch. The /sync last_sync_at cursor only moves to + // events that applied, so a failed event is retried on the next reconnect. result.FailedEventCount++; _logs.Exception(ex); } @@ -574,9 +574,10 @@ internal HistorySyncApplyResult ApplyHistoryEvents(IEnumerable events) private const string HistorySyncWatermarkSource = "history.sync"; - // Advance the watermark once at the end, to the newest applied event. + // Track the newest created_at applied in this batch. + // _lastEventReceivedAt (the /sync last_sync_at cursor) is advanced once at the end. // Advancing per event would skip a failed event on the next reconnect. - private void RecordHistoryWatermark(DateTimeOffset createdAt) + private void TrackMaxAppliedCreatedAt(DateTimeOffset createdAt) { if (createdAt == DateTimeOffset.MinValue) { @@ -589,16 +590,6 @@ private void RecordHistoryWatermark(DateTimeOffset createdAt) } } - private string SerializeHistoryEvent(object e) - { - if (e is string serialized) - { - return serialized; - } - - return _serializer.Serialize(e); - } - public void Dispose() { ConnectionState = ConnectionState.Closing; @@ -693,8 +684,8 @@ internal async Task ConnectUserAsync(string apiKey, string u private readonly IStreamClientConfig _config; private readonly ReconnectScheduler _reconnectScheduler; - private readonly Dictionary> _eventKeyToHandler = - new Dictionary>(); + private readonly Dictionary> _eventKeyToHandler = + new Dictionary>(); private readonly object _websocketConnectionFailedFlagLock = new object(); private readonly object _websocketDisconnectedFlagLock = new object(); @@ -1102,7 +1093,7 @@ private void RegisterEventType(string key, return; } - _eventKeyToHandler.Add(key, serializedContent => + _eventKeyToHandler.Add(key, payload => { try { @@ -1110,15 +1101,15 @@ private void RegisterEventType(string key, var ignoreKeys = new[] { WSEventType.HealthCheck }; if (!ignoreKeys.Contains(key)) { - _logs.Warning("WS event received KEY: " + key + " CONTENT: " + serializedContent); + _logs.Warning("WS event received KEY: " + key + " CONTENT: " + payload); } #endif - var eventObj = DeserializeEvent(serializedContent, out var dto); + var eventObj = DeserializeEvent(payload, out var dto); postprocess?.Invoke(dto); if (_isApplyingHistoryEvents) { - RecordHistoryWatermark(eventObj.CreatedAt); + TrackMaxAppliedCreatedAt(eventObj.CreatedAt); } else { @@ -1145,45 +1136,122 @@ private void RegisterEventType(string key, }); } - private TEvent DeserializeEvent(string content, out TDto dto) + private TEvent DeserializeEvent(object payload, out TDto dto) where TEvent : ILoadableFrom, new() { + dto = DeserializePayload(payload); + + var response = new TEvent(); + response.LoadFromDto(dto); + + return response; + } + + private TDto DeserializePayload(object payload) + { + if (payload is string content) + { + try + { + return _serializer.Deserialize(content); + } + catch (Exception e) + { + throw new StreamDeserializationException(content, typeof(TDto), e); + } + } + try { - dto = _serializer.Deserialize(content); + var converted = _serializer.TryConvertTo(payload); + if (converted != null) + { + return converted; + } } catch (Exception e) { - throw new StreamDeserializationException(content, typeof(TDto), e); + throw new StreamDeserializationException(PayloadToLog(payload), typeof(TDto), e); } - var response = new TEvent(); - response.LoadFromDto(dto); + // Unknown object shape. Same as the old serialize-then-deserialize path. + try + { + return _serializer.Deserialize(_serializer.Serialize(payload)); + } + catch (Exception e) + { + throw new StreamDeserializationException(PayloadToLog(payload), typeof(TDto), e); + } + } - return response; + private void HandleNewWebsocketMessage(object payload, bool isLiveEvent = false) + { + if (payload is string json) + { + HandleNewWebsocketMessage(json, isLiveEvent); + return; + } + + if (payload == null) + { + HandleNewWebsocketMessage(_serializer.Serialize(payload), isLiveEvent); + return; + } + + WebsocketEventEnvelopePeek peek; + try + { + peek = _serializer.TryConvertTo(payload); + } + catch (Exception) + { + HandleNewWebsocketMessage(_serializer.Serialize(payload), isLiveEvent); + return; + } + + if (peek == null || (peek.Error == null && string.IsNullOrEmpty(peek.Type))) + { + HandleNewWebsocketMessage(_serializer.Serialize(payload), isLiveEvent); + return; + } + + DispatchWebsocketPayload(payload, peek.Error, peek.Type, isLiveEvent); } private void HandleNewWebsocketMessage(string msg, bool isLiveEvent = false) { const string ErrorKey = "error"; - if (_serializer.TryPeekValue(msg, ErrorKey, out var apiError)) + APIError apiError = null; + if (_serializer.TryPeekValue(msg, ErrorKey, out var peekedError)) { - _errorSb.Length = 0; - apiError.AppendFullLog(_errorSb); - - _logs.Error($"{nameof(APIError)} returned: {_errorSb}"); - return; + apiError = peekedError; } const string TypeKey = "type"; - if (!_serializer.TryPeekValue(msg, TypeKey, out var type)) + string type = null; + if (apiError == null && !_serializer.TryPeekValue(msg, TypeKey, out type)) { _logs.Error($"Failed to find `{TypeKey}` in msg: " + msg); return; } + DispatchWebsocketPayload(msg, apiError, type, isLiveEvent); + } + + private void DispatchWebsocketPayload(object payload, APIError apiError, string type, bool isLiveEvent) + { + if (apiError != null) + { + _errorSb.Length = 0; + apiError.AppendFullLog(_errorSb); + + _logs.Error($"{nameof(APIError)} returned: {_errorSb}"); + return; + } + // Stamp here, not in the health-check handler. That handler runs after other // callbacks; a slow one can miss the timeout and drop the connection. // Only live socket events count. A /sync replay does not prove the socket is alive. @@ -1200,37 +1268,36 @@ private void HandleNewWebsocketMessage(string msg, bool isLiveEvent = false) if (!_eventKeyToHandler.TryGetValue(type, out var handler)) { - if (TryHandleCustomChannelEvent(msg, type)) + if (TryHandleCustomChannelEvent(payload, type)) { return; } if (_config.LogLevel.IsDebugEnabled()) { - _logs.Warning($"No message handler registered for `{type}`. Message not handled: " + msg); + _logs.Warning($"No message handler registered for `{type}`. Message not handled: " + payload); } return; } - handler(msg); + handler(payload); } - private bool TryHandleCustomChannelEvent(string serializedContent, string eventType) + private bool TryHandleCustomChannelEvent(object payload, string eventType) { - if (!_serializer.TryPeekValue(serializedContent, "cid", out var cid) - || string.IsNullOrEmpty(cid)) + if (!TryGetEventCid(payload, out var cid) || string.IsNullOrEmpty(cid)) { return false; } try { - var dto = _serializer.Deserialize(serializedContent); + var dto = DeserializePayload(payload); if (_isApplyingHistoryEvents) { - RecordHistoryWatermark(dto.CreatedAt); + TrackMaxAppliedCreatedAt(dto.CreatedAt); } else { @@ -1253,6 +1320,45 @@ private bool TryHandleCustomChannelEvent(string serializedContent, string eventT } } + private bool TryGetEventCid(object payload, out string cid) + { + if (payload is string json) + { + return _serializer.TryPeekValue(json, "cid", out cid); + } + + try + { + var peek = _serializer.TryConvertTo(payload); + cid = peek?.Cid; + return !string.IsNullOrEmpty(cid); + } + catch (Exception) + { + var json = _serializer.Serialize(payload); + return _serializer.TryPeekValue(json, "cid", out cid); + } + } + + private static string PayloadToLog(object payload) + => payload as string ?? payload?.ToString() ?? string.Empty; + + /// + /// Enough of a websocket /sync event to dispatch: type, cid, and error. + /// Used so history events can be applied from JObject without a string round-trip. + /// + private class WebsocketEventEnvelopePeek + { + [Newtonsoft.Json.JsonProperty("type")] + public string Type { get; set; } + + [Newtonsoft.Json.JsonProperty("cid")] + public string Cid { get; set; } + + [Newtonsoft.Json.JsonProperty("error")] + public APIError Error { get; set; } + } + private void UpdateHealthCheck() { if (ConnectionState != ConnectionState.Connected) diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index f262a8ce..6aa57071 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -1180,23 +1180,23 @@ private async Task RestoreStateLostDuringDisconnect() var generation = ++_recoveryGeneration; - using (new ListPoolScope(out var tempRecoverSet)) + using (new ListPoolScope(out var tempRecoveryChannelCids)) { - FillRecoverySet(tempRecoverSet); + FillRecoveryChannelCids(tempRecoveryChannelCids); var refreshedChannels = new List(); try { - if (tempRecoverSet.Count > 0) + if (tempRecoveryChannelCids.Count > 0) { - await TryCatchUpWithHistoryAsync(tempRecoverSet, generation); + await TryCatchUpWithHistoryAsync(tempRecoveryChannelCids, generation); if (!IsRecoveryGenerationCurrent(generation)) { return; } - await RehydrateAndRewatchChannelsAsync(tempRecoverSet, generation, refreshedChannels); + await RehydrateAndRewatchChannelsAsync(tempRecoveryChannelCids, generation, refreshedChannels); if (!IsRecoveryGenerationCurrent(generation)) { return; @@ -1224,11 +1224,11 @@ private async Task RestoreStateLostDuringDisconnect() tempRecovered.Add(refreshedChannels[i].Cid); } - for (var i = 0; i < tempRecoverSet.Count; i++) + for (var i = 0; i < tempRecoveryChannelCids.Count; i++) { - if (!tempRecovered.Contains(tempRecoverSet[i])) + if (!tempRecovered.Contains(tempRecoveryChannelCids[i])) { - unrecovered.Add(tempRecoverSet[i]); + unrecovered.Add(tempRecoveryChannelCids[i]); } } } @@ -1248,7 +1248,7 @@ private async Task RestoreStateLostDuringDisconnect() /// /// Copy up to cids from the disconnect snapshot. /// - private void FillRecoverySet(List recoverSet) + private void FillRecoveryChannelCids(List recoveryChannelCids) { if (_recoveryChannelCids.Count > MaxRecoveredChannels) { @@ -1263,15 +1263,15 @@ private void FillRecoverySet(List recoverSet) var count = Math.Min(_recoveryChannelCids.Count, MaxRecoveredChannels); for (var i = 0; i < count; i++) { - recoverSet.Add(_recoveryChannelCids[i]); + recoveryChannelCids.Add(_recoveryChannelCids[i]); } } - private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, int generation) + private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoveryChannelCids, int generation) { try { - var response = await InternalLowLevelClient.TrySyncHistoryAsync(recoverSet); + var response = await InternalLowLevelClient.TrySyncHistoryAsync(recoveryChannelCids); // No sync point, or older than 30 days. Step 2 still runs. This is not a deletion. if (response == null) @@ -1328,12 +1328,12 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoverSet, /// recreate a deleted channel. Those cids go to /// . /// - private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recoverSet, int generation, + private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recoveryChannelCids, int generation, List refreshed) { var sort = ChannelSort.OrderByDescending(ChannelSortFieldName.LastMessageAt); - for (var i = 0; i < recoverSet.Count; i += MaxChannelsPerRecoveryQuery) + for (var i = 0; i < recoveryChannelCids.Count; i += MaxChannelsPerRecoveryQuery) { if (!IsRecoveryGenerationCurrent(generation)) { @@ -1342,12 +1342,12 @@ private async Task RehydrateAndRewatchChannelsAsync(IReadOnlyList recove using (new ListPoolScope(out var tempChunk)) { - var chunkEnd = Math.Min(i + MaxChannelsPerRecoveryQuery, recoverSet.Count); + var chunkEnd = Math.Min(i + MaxChannelsPerRecoveryQuery, recoveryChannelCids.Count); for (var j = i; j < chunkEnd; j++) { - if (!_inaccessibleCids.Contains(recoverSet[j])) + if (!_inaccessibleCids.Contains(recoveryChannelCids[j])) { - tempChunk.Add(recoverSet[j]); + tempChunk.Add(recoveryChannelCids[j]); } } diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs index 31466051..f0d72319 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs @@ -181,6 +181,70 @@ public void when_history_event_older_than_watermark_expect_watermark_not_regress Assert.AreEqual(NewestCreatedAt, GetLastEventReceivedAt(client)); } + [Test] + public void when_history_event_is_parsed_object_expect_applied_like_json_string() + { + var json = MessageNewJson("msg-1", NewestCreatedAt); + var parsed = _serializer.DeserializeObject(json); + + var received = 0; + _lowLevelClient.MessageReceived += _ => received++; + + var fromObject = _lowLevelClient.ApplyHistoryEvents(new List { parsed }); + var fromString = CreateClient().ApplyHistoryEvents(new List { json }); + + Assert.AreEqual(0, received, "Parsed /sync objects must take the silent path, same as JSON strings."); + Assert.AreEqual(0, fromObject.FailedEventCount); + Assert.AreEqual(fromString.MaxAppliedCreatedAt, fromObject.MaxAppliedCreatedAt, + "JObject /sync events must advance last_sync_at the same way JSON strings do."); + Assert.AreEqual(NewestCreatedAt, fromObject.MaxAppliedCreatedAt); + } + + [Test] + public void when_history_custom_event_is_parsed_object_expect_delivered() + { + var received = new List(); + _lowLevelClient.CustomEventReceived += e => received.Add(e.Type); + + var parsed = _serializer.DeserializeObject(CustomEventJson("game.state", NewestCreatedAt)); + _lowLevelClient.ApplyHistoryEvents(new List { parsed }); + + Assert.AreEqual(new[] { "game.state" }, received.ToArray()); + } + + [Test] + public void when_history_replay_parsed_object_expect_public_message_received() + { + var received = 0; + _lowLevelClient.MessageReceived += _ => received++; + + var parsed = _serializer.DeserializeObject(MessageNewJson("msg-1", NewestCreatedAt)); + _lowLevelClient.ReplayHistoryEvents(new List { parsed }); + + Assert.AreEqual(1, received, + "ReplayEvents must raise public callbacks for parsed /sync objects, same as JSON strings."); + } + + [Test] + public void when_history_batch_contains_malformed_parsed_object_expect_remaining_events_still_applied() + { + var client = CreateClient(); + var newest = NewestCreatedAt; + var malformed = _serializer.DeserializeObject( + $"{{\"type\":\"message.new\",\"cid\":\"messaging:test\",\"created_at\":\"{newest.AddMinutes(-1):O}\",\"message\":\"not-an-object\"}}"); + + var result = client.ApplyHistoryEvents(new List + { + MessageNewJson("msg-1", newest.AddMinutes(-10)), + malformed, + MessageNewJson("msg-3", newest.AddMinutes(-5)), + }); + + Assert.AreEqual(1, result.FailedEventCount); + Assert.AreEqual(newest.AddMinutes(-5), result.MaxAppliedCreatedAt); + Assert.AreEqual(newest.AddMinutes(-5), GetLastEventReceivedAt(client)); + } + [Test] public void when_health_check_arrives_on_live_socket_expect_liveness_stamped_before_handlers() { From 06cff003802d38515b7ef371d433ea00e88e537f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:44:37 +0200 Subject: [PATCH 09/10] fix compilation issue --- .../Core/LowLevelClient/StreamChatLowLevelClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs index e52261a5..1afb9459 100644 --- a/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs +++ b/Assets/Plugins/StreamChat/Core/LowLevelClient/StreamChatLowLevelClient.cs @@ -1335,8 +1335,8 @@ private bool TryGetEventCid(object payload, out string cid) } catch (Exception) { - var json = _serializer.Serialize(payload); - return _serializer.TryPeekValue(json, "cid", out cid); + var serialized = _serializer.Serialize(payload); + return _serializer.TryPeekValue(serialized, "cid", out cid); } } From 0c8d1133375b18e8cb83efd8592c5a35a6ccaac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sierpi=C5=84ski?= <33436839+sierpinskid@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:33:26 +0200 Subject: [PATCH 10/10] pr review --- .../StreamChat/Core/StreamChatClient.cs | 4 +- .../StateSync/StateRecoveryClientTests.cs | 131 ++++++------------ .../StateSync/StateRecoveryLowLevelTests.cs | 87 +----------- 3 files changed, 51 insertions(+), 171 deletions(-) diff --git a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs index 6aa57071..eb22abc0 100644 --- a/Assets/Plugins/StreamChat/Core/StreamChatClient.cs +++ b/Assets/Plugins/StreamChat/Core/StreamChatClient.cs @@ -1273,7 +1273,9 @@ private async Task TryCatchUpWithHistoryAsync(IReadOnlyList recoveryChan { var response = await InternalLowLevelClient.TrySyncHistoryAsync(recoveryChannelCids); - // No sync point, or older than 30 days. Step 2 still runs. This is not a deletion. + // null = we never called /sync (no _lastEventReceivedAt yet, or the gap is older than 30 days). + // Channels are still valid; RehydrateAndRewatchChannelsAsync recovers them. + // Contrast inaccessible_cids below: those are gone and must not be queried. if (response == null) { return; diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs index 5f00d633..5ff2bac4 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryClientTests.cs @@ -98,30 +98,31 @@ public void when_reconnected_expect_channels_requeried_even_though_sync_was_skip DropConnection(); Reconnect(); - // The whole point of #227/#232: the re-query is what re-establishes the watches, so it has - // to run whether or not the /sync catch-up did anything. Here it was skipped outright, - // because the health check carried no created_at so there is no sync point. + // /sync is skipped here (no last_sync_at). Recovery must still re-query so the + // new socket watches the same channels again. AssertQueryChannelsCallCount(1); Assert.AreEqual(1, _recoveredEvents.Count); + _mockHttpClient.Received().SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), + Arg.Is(body => RequestHasJsonBool(body, "watch", true) + && RequestHasJsonBool(body, "state", true))); } [Test] - public void when_connection_drops_expect_watches_released_and_snapshot_taken() + public void when_connection_drops_expect_watches_released() { Connect(); var channel = WatchChannel("messaging:a"); DropConnection(); - // The server dropped the watch, so continuing to report it would be a lie - and would make - // IStreamChannel.WatchAsync a silent no-op for a channel that is not actually watched. Assert.IsFalse(channel.IsWatched); Assert.AreEqual(0, _client.WatchedChannels.Count); - Assert.AreEqual(new[] { "messaging:a" }, RecoverySnapshot()); } [Test] - public void when_reconnect_attempt_fails_expect_recovery_snapshot_preserved() + public void when_reconnect_attempt_fails_expect_channels_still_recovered() { Connect(); WatchChannel("messaging:a"); @@ -130,14 +131,16 @@ public void when_reconnect_attempt_fails_expect_recovery_snapshot_preserved() DropConnection(); FailReconnectAttempt(); FailReconnectAttempt(); - - // A failed attempt transitions Connecting -> Disconnected with an already-empty watch list. - // Re-snapshotting there would discard the only record of what needs recovering, and the - // attempt that eventually succeeds would restore nothing - the flaky-mobile-network case. - Assert.AreEqual(new[] { "messaging:a", "messaging:b" }, RecoverySnapshot().OrderBy(_ => _).ToArray()); - Reconnect(); + + // Failed Connecting -> Disconnected must not forget the channels that were + // watched before the outage. The attempt that succeeds still recovers them. AssertQueryChannelsCallCount(1); + _mockHttpClient.Received().SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), + Arg.Is(body => RequestBodyContains(body, "messaging:a") + && RequestBodyContains(body, "messaging:b"))); } [Test] @@ -149,8 +152,6 @@ public void when_channels_cannot_be_recovered_expect_them_reported_as_unrecovere DropConnection(); Reconnect(); - // The mocked query returns no channels, which is what the server does for a channel that - // was deleted or that the local user lost access to while offline. Assert.AreEqual(1, _recoveredEvents.Count); Assert.AreEqual(0, _recoveredEvents[0].Channels.Count); Assert.AreEqual(new[] { "messaging:a" }, _recoveredEvents[0].UnrecoveredChannelCids.ToArray()); @@ -181,10 +182,10 @@ public void when_recovery_query_fails_expect_remaining_chunks_still_queried() DropConnection(); Reconnect(); - // 40 cids is two chunks of 30 and 10. There is no later retry within a connection, so a - // failed chunk must not cost the remaining chunks their recovery. Assert.AreEqual(2, callCount); Assert.AreEqual(1, _recoveredEvents.Count); + Assert.AreEqual(40, _recoveredEvents[0].UnrecoveredChannelCids.Count); + Assert.IsFalse(_recoveredEvents[0].IsComplete); } [Test] @@ -199,8 +200,6 @@ public void when_recovery_set_exceeds_cap_expect_only_capped_channels_queried() DropConnection(); Reconnect(); - // Capped at 100, chunked by 30 -> 4 requests. Uncapped this would be 5, and would keep - // growing with the watch list on every single reconnect. AssertQueryChannelsCallCount(4); } @@ -217,9 +216,6 @@ public void when_strategy_is_disabled_expect_no_recovery_and_watches_left_untouc AssertQueryChannelsCallCount(0); Assert.AreEqual(0, _recoveredEvents.Count); - - // Disabled means the SDK does nothing, so WatchedChannels stays the record of what the - // consumer was watching and is theirs to recover from. Assert.IsTrue(channel.IsWatched); Assert.AreEqual(1, _client.WatchedChannels.Count); } @@ -235,8 +231,6 @@ public void when_user_disconnects_and_connects_again_expect_no_recovery_of_previ Connect(); - // A new login must not recover, or re-watch, channels belonging to the session that ended - - // possibly for a different user. AssertQueryChannelsCallCount(0); Assert.AreEqual(0, _recoveredEvents.Count); } @@ -252,7 +246,11 @@ public void when_channel_unwatched_while_disconnected_expect_it_not_recovered() InvokeMarkChannelUnwatched(channel); Reconnect(); - Assert.AreEqual(new[] { "messaging:b" }, RecoverySnapshot()); + _mockHttpClient.Received(1).SendHttpRequestAsync( + Arg.Is(HttpMethodType.Post), + Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), + Arg.Is(body => RequestBodyContains(body, "messaging:b") + && !RequestBodyContains(body, "messaging:a"))); } [Test] @@ -266,9 +264,6 @@ public void when_sync_returns_empty_events_with_inaccessible_cids_expect_those_c _mockTimeService.Now.Returns(now); DropConnection(); - // Health checks in these tests carry no created_at, so disconnect would otherwise leave - // no sync point and skip /sync entirely. Seed one after the drop so the copy on - // Disconnected cannot wipe it. SetDisconnectionLastEventReceivedAt(_client.InternalLowLevelClient, now.AddHours(-1)); RespondWith(SyncEndpoint, "{\"events\":[],\"inaccessible_cids\":[\"messaging:gone\"]}"); @@ -327,7 +322,6 @@ public IEnumerator when_stale_recovery_query_completes_after_newer_recovery_expe Assert.IsFalse(channel.IsWatched); Assert.AreEqual(0, _client.WatchedChannels.Count); - // Increments generation. Snapshot is kept because watches were already cleared. DropConnection(); Reconnect(); @@ -345,7 +339,6 @@ public IEnumerator when_stale_recovery_query_completes_after_newer_recovery_expe Assert.IsTrue(_recoveredEvents[0].IsComplete); staleQuery.SetResult(QueryChannelsHttpResponse("messaging:a", "stale-A")); - // Drain posted continuations so a missing generation gate would have applied by now. for (var i = 0; i < 5; i++) { Update(); @@ -372,10 +365,8 @@ public void when_silent_history_batch_trims_expect_messages_removed_from_cache_n _client.InternalLowLevelClient.ApplyHistoryEvents(MessageNewEvents("messaging:a", count: 7)); - Assert.AreEqual(0, removedCount, - "BatchStateUpdate must not fire MessagesRemovedFromCache; the UI rebuilds on StateRecovered."); - Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count, - "Trim must still run during a silent batch so a 1000-event /sync cannot blow past MaxMessages."); + Assert.AreEqual(0, removedCount); + Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count); } [Test] @@ -390,8 +381,7 @@ public void when_history_replay_trims_expect_messages_removed_from_cache_raised( _client.InternalLowLevelClient.ReplayHistoryEvents(MessageNewEvents("messaging:a", count: 7)); - Assert.AreEqual(1, removedCount, - "ReplayEvents is the default and must keep raising MessagesRemovedFromCache for back-compat."); + Assert.AreEqual(1, removedCount); Assert.AreEqual(SmallWindow.MaxMessages - SmallWindow.DiscardBatchSize, channel.Messages.Count); } @@ -410,8 +400,7 @@ public void when_silent_history_batch_adds_thread_reply_expect_reply_received_no ThreadReplyJson("messaging:a", "parent-1", "reply-1"), }); - Assert.AreEqual(0, replyCount, - "BatchStateUpdate must not fire ReplyReceived; the UI rebuilds on StateRecovered."); + Assert.AreEqual(0, replyCount); Assert.AreEqual(1, thread.LatestReplies.Count); Assert.AreEqual("reply-1", thread.LatestReplies[0].Id); } @@ -431,8 +420,7 @@ public void when_history_replay_adds_thread_reply_expect_reply_received_raised() ThreadReplyJson("messaging:a", "parent-1", "reply-1"), }); - Assert.AreEqual(1, replyCount, - "ReplayEvents is the default and must keep raising ReplyReceived for back-compat."); + Assert.AreEqual(1, replyCount); Assert.AreEqual(1, thread.LatestReplies.Count); Assert.AreEqual("reply-1", thread.LatestReplies[0].Id); } @@ -451,8 +439,7 @@ public void when_silent_history_batch_changes_presence_expect_presence_changed_n PresenceChangedJson("other-user", online: true), }); - Assert.AreEqual(0, presenceCount, - "BatchStateUpdate must not fire PresenceChanged; the UI rebuilds on StateRecovered."); + Assert.AreEqual(0, presenceCount); Assert.IsTrue(user.Online); } @@ -470,8 +457,7 @@ public void when_history_replay_changes_presence_expect_presence_changed_raised( PresenceChangedJson("other-user", online: true), }); - Assert.AreEqual(1, presenceCount, - "ReplayEvents is the default and must keep raising PresenceChanged for back-compat."); + Assert.AreEqual(1, presenceCount); Assert.IsTrue(user.Online); } @@ -492,10 +478,8 @@ public void when_silent_history_batch_closes_poll_expect_closed_not_raised() PollClosedJson("messaging:a", "poll-1"), }); - Assert.AreEqual(0, closedCount, - "BatchStateUpdate must not fire Closed; the UI rebuilds on StateRecovered."); - Assert.AreEqual(0, updatedCount, - "BatchStateUpdate must not fire Updated during a silent poll DTO apply."); + Assert.AreEqual(0, closedCount); + Assert.AreEqual(0, updatedCount); Assert.IsTrue(poll.IsClosed); } @@ -514,8 +498,7 @@ public void when_history_replay_closes_poll_expect_closed_raised() PollClosedJson("messaging:a", "poll-1"), }); - Assert.AreEqual(1, closedCount, - "ReplayEvents is the default and must keep raising Closed for back-compat."); + Assert.AreEqual(1, closedCount); Assert.IsTrue(poll.IsClosed); } @@ -534,8 +517,7 @@ public void when_silent_recovery_syncs_message_new_expect_channel_message_receiv ReconnectWithSync(SyncEventsJson(MessageNewJson("messaging:a", "msg-offline", new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero)))); - Assert.AreEqual(0, receivedCount, - "BatchStateUpdate must not fire IStreamChannel.MessageReceived; rebuild on StateRecovered."); + Assert.AreEqual(0, receivedCount); Assert.AreEqual(1, channel.Messages.Count); Assert.AreEqual("msg-offline", channel.Messages[0].Id); Assert.AreEqual(1, _recoveredEvents.Count); @@ -554,45 +536,27 @@ public void when_replay_recovery_syncs_message_new_expect_channel_message_receiv ReconnectWithSync(SyncEventsJson(MessageNewJson("messaging:a", "msg-offline", new DateTimeOffset(2026, 8, 24, 12, 0, 0, TimeSpan.Zero)))); - Assert.AreEqual(1, receivedCount, - "ReplayEvents is the default and must keep raising IStreamChannel.MessageReceived for back-compat."); + Assert.AreEqual(1, receivedCount); Assert.AreEqual(1, channel.Messages.Count); Assert.AreEqual("msg-offline", channel.Messages[0].Id); Assert.AreEqual(1, _recoveredEvents.Count); } [Test] - public void when_silent_history_batch_contains_custom_event_expect_channel_custom_event_received() + public void when_silent_recovery_syncs_custom_event_expect_channel_custom_event_received() { + _config.StateRecoveryStrategy = StateRecoveryStrategy.BatchStateUpdate; + Connect(); var channel = WatchChannel("messaging:a"); + RespondWith(QueryChannelsEndpoint, QueryChannelsJson("messaging:a", "a")); string receivedType = null; channel.CustomEventReceived += (_, evt) => receivedType = evt.Type; - _client.InternalLowLevelClient.ApplyHistoryEvents(new[] - { - CustomEventJson("messaging:a", "game.state"), - }); - - Assert.AreEqual("game.state", receivedType, - "Custom events have no state representation, so ApplyHistoryEvents must still deliver them."); - } + ReconnectWithSync(SyncEventsJson(CustomEventJson("messaging:a", "game.state"))); - [Test] - public void when_recovery_query_sent_expect_watch_and_state_true() - { - Connect(); - WatchChannel("messaging:a"); - - DropConnection(); - Reconnect(); - - _mockHttpClient.Received().SendHttpRequestAsync( - Arg.Is(HttpMethodType.Post), - Arg.Is(uri => uri.AbsolutePath.EndsWith(QueryChannelsEndpoint)), - Arg.Is(body => RequestHasJsonBool(body, "watch", true) - && RequestHasJsonBool(body, "state", true))); + Assert.AreEqual("game.state", receivedType); } private const string SyncEndpoint = "/sync"; @@ -659,9 +623,6 @@ private void ReconnectWithSync(string syncJson) _mockTimeService.Now.Returns(now); DropConnection(); - // Health checks in these tests carry no created_at, so disconnect would otherwise leave - // no sync point and skip /sync entirely. Seed one after the drop so the copy on - // Disconnected cannot wipe it. SetDisconnectionLastEventReceivedAt(_client.InternalLowLevelClient, now.AddHours(-1)); RespondWith(SyncEndpoint, syncJson); Reconnect(); @@ -816,14 +777,6 @@ private void InvokePrivate(string methodName, object argument) method.Invoke(_client, new[] { argument }); } - private string[] RecoverySnapshot() - { - var field = typeof(StreamChatClient).GetField("_recoveryChannelCids", - BindingFlags.Instance | BindingFlags.NonPublic); - Assert.IsNotNull(field, "Expected _recoveryChannelCids to exist."); - return ((List)field.GetValue(_client)).ToArray(); - } - private static void SetDisconnectionLastEventReceivedAt(StreamChatLowLevelClient client, DateTimeOffset value) { var field = typeof(StreamChatLowLevelClient).GetField("_disconnectionLastEventReceivedAt", diff --git a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs index f0d72319..51b89097 100644 --- a/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs +++ b/Assets/Plugins/StreamChat/Tests/StateSync/StateRecoveryLowLevelTests.cs @@ -84,8 +84,6 @@ public void when_sync_requested_expect_inaccessible_cids_asked_for() _lowLevelClient.TrySyncHistoryAsync(new[] { "messaging:a" }).GetAwaiter().GetResult(); - // Without this the response cannot distinguish a deleted channel from one the query - // happened to omit, and recovery would keep retrying it forever. _mockHttpClient.Received(1).SendHttpRequestAsync( Arg.Is(HttpMethodType.Post), Arg.Is(uri => uri.AbsolutePath.EndsWith("/sync")), @@ -100,7 +98,7 @@ public void when_history_batch_applied_expect_no_public_message_received() var result = _lowLevelClient.ApplyHistoryEvents(new List { MessageNewJson("msg-1", NewestCreatedAt) }); - Assert.AreEqual(0, received, "A silent history batch must not raise public per-event callbacks."); + Assert.AreEqual(0, received); Assert.AreEqual(0, result.FailedEventCount); Assert.AreEqual(NewestCreatedAt, result.MaxAppliedCreatedAt); } @@ -113,8 +111,7 @@ public void when_history_batch_replayed_expect_public_message_received() _lowLevelClient.ReplayHistoryEvents(new List { MessageNewJson("msg-1", NewestCreatedAt) }); - Assert.AreEqual(1, received, - "ReplayEvents is the default strategy and must keep raising per-event callbacks for back-compat."); + Assert.AreEqual(1, received); } [Test] @@ -128,8 +125,6 @@ public void when_history_batch_contains_custom_event_expect_it_delivered_per_eve CustomEventJson("game.state", NewestCreatedAt), }); - // Custom events have no representation in local state, so suppressing them would lose the - // payload with no way for a consumer to recover it. Assert.AreEqual(new[] { "game.state" }, received.ToArray()); } @@ -145,8 +140,7 @@ public void when_history_batch_applied_expect_watermark_advanced_once_to_newest_ MessageNewJson("msg-3", NewestCreatedAt.AddMinutes(-5)), }); - Assert.AreEqual(NewestCreatedAt, GetLastEventReceivedAt(client), - "The batch must advance the watermark exactly once, to its newest event."); + Assert.AreEqual(NewestCreatedAt, GetLastEventReceivedAt(client)); } [Test] @@ -163,9 +157,6 @@ public void when_history_batch_contains_malformed_event_expect_remaining_events_ }); Assert.AreEqual(1, result.FailedEventCount); - - // The watermark must not claim the failed event was applied, or the next reconnect would - // never ask for it again. Assert.AreEqual(newest.AddMinutes(-5), result.MaxAppliedCreatedAt); Assert.AreEqual(newest.AddMinutes(-5), GetLastEventReceivedAt(client)); } @@ -182,71 +173,7 @@ public void when_history_event_older_than_watermark_expect_watermark_not_regress } [Test] - public void when_history_event_is_parsed_object_expect_applied_like_json_string() - { - var json = MessageNewJson("msg-1", NewestCreatedAt); - var parsed = _serializer.DeserializeObject(json); - - var received = 0; - _lowLevelClient.MessageReceived += _ => received++; - - var fromObject = _lowLevelClient.ApplyHistoryEvents(new List { parsed }); - var fromString = CreateClient().ApplyHistoryEvents(new List { json }); - - Assert.AreEqual(0, received, "Parsed /sync objects must take the silent path, same as JSON strings."); - Assert.AreEqual(0, fromObject.FailedEventCount); - Assert.AreEqual(fromString.MaxAppliedCreatedAt, fromObject.MaxAppliedCreatedAt, - "JObject /sync events must advance last_sync_at the same way JSON strings do."); - Assert.AreEqual(NewestCreatedAt, fromObject.MaxAppliedCreatedAt); - } - - [Test] - public void when_history_custom_event_is_parsed_object_expect_delivered() - { - var received = new List(); - _lowLevelClient.CustomEventReceived += e => received.Add(e.Type); - - var parsed = _serializer.DeserializeObject(CustomEventJson("game.state", NewestCreatedAt)); - _lowLevelClient.ApplyHistoryEvents(new List { parsed }); - - Assert.AreEqual(new[] { "game.state" }, received.ToArray()); - } - - [Test] - public void when_history_replay_parsed_object_expect_public_message_received() - { - var received = 0; - _lowLevelClient.MessageReceived += _ => received++; - - var parsed = _serializer.DeserializeObject(MessageNewJson("msg-1", NewestCreatedAt)); - _lowLevelClient.ReplayHistoryEvents(new List { parsed }); - - Assert.AreEqual(1, received, - "ReplayEvents must raise public callbacks for parsed /sync objects, same as JSON strings."); - } - - [Test] - public void when_history_batch_contains_malformed_parsed_object_expect_remaining_events_still_applied() - { - var client = CreateClient(); - var newest = NewestCreatedAt; - var malformed = _serializer.DeserializeObject( - $"{{\"type\":\"message.new\",\"cid\":\"messaging:test\",\"created_at\":\"{newest.AddMinutes(-1):O}\",\"message\":\"not-an-object\"}}"); - - var result = client.ApplyHistoryEvents(new List - { - MessageNewJson("msg-1", newest.AddMinutes(-10)), - malformed, - MessageNewJson("msg-3", newest.AddMinutes(-5)), - }); - - Assert.AreEqual(1, result.FailedEventCount); - Assert.AreEqual(newest.AddMinutes(-5), result.MaxAppliedCreatedAt); - Assert.AreEqual(newest.AddMinutes(-5), GetLastEventReceivedAt(client)); - } - - [Test] - public void when_health_check_arrives_on_live_socket_expect_liveness_stamped_before_handlers() + public void when_health_check_arrives_on_live_socket_expect_liveness_stamped() { var client = CreateClientWithMessages(HealthCheckJson()); client.Connect(); @@ -256,8 +183,7 @@ public void when_health_check_arrives_on_live_socket_expect_liveness_stamped_bef EnqueueMessages(HealthCheckJson()); client.Update(0.2f); - Assert.AreEqual(12f, GetLastHealthCheckReceivedTime(client), - "Liveness must be stamped when the health check is read, not after consumer handlers run."); + Assert.AreEqual(12f, GetLastHealthCheckReceivedTime(client)); } [Test] @@ -272,8 +198,7 @@ public void when_health_check_arrives_from_history_replay_expect_liveness_not_st _mockTimeService.Time.Returns(99f); client.ReplayHistoryEvents(new List { HealthCheckJson() }); - Assert.AreEqual(stampedOnConnect, GetLastHealthCheckReceivedTime(client), - "A replayed health check proves nothing about the current socket and must not extend liveness."); + Assert.AreEqual(stampedOnConnect, GetLastHealthCheckReceivedTime(client)); } private static readonly DateTimeOffset NewestCreatedAt =