Move to stream-android-core 5.0.2 and handle aggregated socket events - #213
Conversation
Core 5.0.0 replaced the socket session's StreamBatcher with StreamEventAggregator, so StreamComponentProvider no longer takes a `batcher`. Leave `eventAggregator` unset: core builds one from the event parser it already holds, tuned by the aggregation values on socketConfig. The aggregator changes the dispatch contract. Under a traffic spike core delivers one StreamAggregatedEvent wrapping N events instead of N separate calls, and FeedsClientImpl only accepted WSEvent — every event in a spike would have been logged as an error and dropped. Unwrap the batch and replay it in arrival order.
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe Android core version changes to 5.0.2. Client creation uses core event aggregation defaults. ChangesEvent dispatch flow
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This updates core event aggregation and replays each aggregated socket event through the existing feed dispatch path. The covered event cases preserve normal dispatch behavior, with no current merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant StreamClientListener
participant FeedsClientImpl
participant feedsEventsSubscriptionManager
StreamClientListener->>FeedsClientImpl: Deliver StreamAggregatedEvent
FeedsClientImpl->>FeedsClientImpl: Replay contained events in arrival order
FeedsClientImpl->>feedsEventsSubscriptionManager: Dispatch each WSEvent
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit sees events arrive Comment |
|
🚀 Available in v0.10.0 |
3 participants
Goal
Closes AND-1506 — feeds was pinned to core 4.0.0 while core shipped 5.0.2.
Implementation
streamAndroidCore4.0.0 → 5.0.2.StreamBatcherwithStreamEventAggregator, soStreamComponentProviderno longer takes abatcher.eventAggregatoris left unset: core builds one from the event parser it already holds, at the aggregation defaults onsocketConfig(threshold 50, 500 ms window). The old batcher'sbatchSize=10 / 100 ms / 1000 mswas deliberately not mapped across — batching and spike-aggregation are different mechanisms, so the constants don't carry meaning between them. Aggregation stays at core's defaults: the goal here is preserving the dispatch contract, and how feeds wants to handle spikes is a separate decision it can take later.StreamAggregatedEventwrapping N events rather than N separate calls.FeedsClientImpl.onEventaccepted onlyWSEvent, so on 5.x every event in a spike would have gone down the error branch and been dropped. It now unwraps the batch and replays it in arrival order, leaving the single-event path untouched.StreamBatcheritself is unchanged in 5.0.2, soFeedOwnValuesRepositoryImpl's own use of it is unaffected.Testing
FeedsClientImplTestcases: an aggregated batch dispatches every event it holds, a foreign payload mid-batch doesn't stop the rest, and single events still dispatch once. All three were confirmed to fail with the unwrap removed.spotlessCheck,apiCheck,testDebugUnitTest,lintandassembleDebugpass locally.apiCheckis clean — the change is entirely internal, no public API surface moved.Checklist
Summary by CodeRabbit
Bug Fixes
Maintenance