Wire StreamClient into StreamVideoClient as coordinator owner#1738
Draft
aleksandar-apostolov wants to merge 6 commits into
Draft
Wire StreamClient into StreamVideoClient as coordinator owner#1738aleksandar-apostolov wants to merge 6 commits into
aleksandar-apostolov wants to merge 6 commits into
Conversation
Task 1 of Plan 02-02. Eagerly inject core v5 StreamClient into
StreamVideoClient's primary constructor (D-03), replace the four
coordinatorSocket.{events,state,errors,state} collectors with a single
StreamClientListener subscription that casts to VideoEvent and forwards to
the existing fireEvent(...) dispatch pipeline. connectAsync() delegates to
streamClient.connect(); cleanup() calls streamClient.disconnect() (bridged
via runBlocking) before scope.cancel(). Deletes setupGuestUser,
createGuestUser, guestUserJob, and the corresponding await() sites. Also
lands the minimum StreamVideoBuilder wiring (streamTokenProvider dispatch
on user.type + streamClient factory seam) so the two files compile as one
unit; Task 2 extends the builder tests separately.
Rule 3 blocker: Call.kt referenced clientImpl.guestUserJob?.await() at the
join call site. Replaced the await with a KDoc comment noting that
StreamTokenManager now single-flights the guest JWT transparently through
core's auth interceptor.
TODO(02-03): streamClientListener.onState currently no-ops; the
handleStreamState routing lands in Plan 02-03 alongside the ConnectionState
sealed-interface reshape.
Task 2 of Plan 02-02. New StreamVideoBuilderDispatchTest exercises the `when (user.type)` dispatch in StreamVideoBuilder.build() through the streamClientFactory seam: - Authenticated -> IntegrationStreamTokenProvider (D-05) - Guest -> GuestStreamTokenProvider (D-06, COORD-09) - Anonymous -> IllegalStateException with a D-07 reference - localCoordinatorAddress "10.0.2.2:3030" threads through into args.resolvedWssUrl as "ws://10.0.2.2:3030/video/connect" (COORD-08) The pre-existing StreamVideoBuilderTest stays @ignore'd (integration-style suite depending on network authData). The new lightweight class runs on the JVM with mockk Context + ConnectivityManager stubs.
Task 3 of Plan 02-02. Three surgical edits that finish unwiring the legacy coordinator socket from the live code path: 1. CoordinatorConnectionModule narrows its ConnectionModuleDeclaration generic to Unit and turns `socketConnection` into a `Unit = Unit` vestigial override. The CoordinatorSocketConnection field is deleted; the class file itself stays on disk for Phase 8 (COORD-07). 2. StreamLifecycleObserver.kt moved from `lifecycle/` to `lifecycle/legacy/` (Plan B in the plan text). Coordinator-side lifecycle observation now flows through core's StreamLifecycleMonitor per D-10 / PROC-07. Legacy consumers (Coordinator/SFU socket files kept for Phase 8) have their imports updated to `lifecycle.legacy.StreamLifecycleObserver`. SFU is Phase 3 territory; the moved file survives until then. 3. IntegrationTestBase updated to read the connection ID from streamClient.connectionState instead of the deleted socketConnection. Legacy coordinator test @ignore roster: - CoordinatorSocketTest.kt / CoordinatorSocketStateServiceTest.kt do NOT exist under `src/test/.../socket/coordinator/` on this branch — nothing to annotate. - TokenManagerImplTest.kt exercises TokenManagerImpl behaviours shared with SFU (still in-tree per RESEARCH line 1153). Preserved unannotated so SFU coverage is not lost. @ignore baseline unchanged at 21.
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
This was referenced Jul 1, 2026
Core's default StreamClient factory touches WifiManager during eager construction, which layoutlib's BridgeContext does not implement. All Compose snapshot tests were failing at setup with "AssertionError: Unsupported Service: wifi". Expose the streamClientFactory seam on StreamVideoBuilder via a public @InternalStreamVideoApi setter and have StreamPreviewDataUtils install a NoOpStreamClient before build().
Guest users (iOS loadGuestUserInfo parity): - GuestStreamTokenProvider now syncs the createGuest JWT into the legacy Retrofit auth path (CoordinatorAuthInterceptor) via onTokenIssued and flips the auth type anonymous->jwt, matching the legacy setupGuestUser sequence. Previously the token only reached core's WS path, leaving every guest REST call unauthenticated (401 loop). - Builder now auto-connects guests too (only anonymous never connects); core's token manager drives createGuest inside connect(). - Server-issued guest identity is adopted into ClientState.user through BindableWritableUserRepository, including iOS's name-preservation quirk for server-decorated display names. Anonymous users (D-07, iOS connectUser parity): - build() no longer throws; anonymous clients are REST-only with stream-auth-type anonymous restored. - connectAsync()/connectIfNotAlreadyConnected() fail fast without touching the network.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Closes AND-1282
Replaces our custom coordinator socket with
stream-android-core'sStreamClientas the owner of the coordinator WebSocket connection.StreamClientnow handles the socket lifecycle, token refresh, HTTP interceptors, and reconnection. Guest and authenticated user handling is unified through a single token-provider abstraction.Depends on #1737 (adapter bridging). Merge after parent.
Merge order: adapters (#1737) → this PR → ConnectionState rewrite (#1739).
Implementation
StreamVideoClientnow owns aStreamClientinstance.Constructed eagerly in the primary constructor and disposed in
cleanup(). Tests inject a mock via a constructor parameter — no moreLazy<>workarounds needed.Event flow now goes through core:
StreamClientfires WebSocket events, our listener forwards them to the existingfireEvent(...)pipeline. Existing subscribers keep working unchanged.StreamVideoBuilderpicks the right token provider based on user type.Regular users get
IntegrationStreamTokenProvider(wrapping the integration'sTokenProvider). Guest users getGuestStreamTokenProvider(which callscreateGuestinternally). Anonymous users get a REST-only client that never opens a coordinator socket; explicitconnectattempts fail fast.Guest and anonymous flows mirror the iOS SDK (
StreamVideo.loadGuestUserInfo/initialConnectIfRequired):createGuestinsideconnect().CoordinatorAuthInterceptor) the moment it is issued, and auth type flipsanonymous→jwt— REST and WS stay on one token until Phase 4 unifies HTTP through core.ClientState.user, including iOS's name-preservation quirk for decorated display names.createGuestagain, single-flighted byStreamTokenManager.Guest user machinery deleted.
setupGuestUser,createGuestUser,guestUserJob: Deferred<Unit>?, and everyapiCall { guestUserJob?.await() }synchronization site are gone. Guest token acquisition happens insideGuestStreamTokenProvider.loadToken(userId)— first invocation callscreateGuest, subsequent calls return the cached JWT via core'sStreamTokenManager(which provides single-flight dedup automatically). This removes the race conditions we've been patching in the guest flow over the last few weeks.Coordinator module surgery.
CoordinatorConnectionModule.socketConnectionfield is removed. The legacyCoordinatorSocketConnectionclass stays on disk (nothing references it), scheduled for deletion in a future cleanup PR.StreamLifecycleObservermoved tolifecycle/legacy/package.SfuSocketstill references it, so it can't be deleted yet — the SFU migration will remove it.Tests:
StreamVideoClientTestextended with assertions thatStreamClientis subscribed at init,connect()invoked onconnectAsync(),disconnect()invoked oncleanup().StreamVideoBuilderDispatchTestcovers the token-provider dispatch: Anonymous → REST-only provider (no throw), Authenticated → Integration provider, Guest → Guest provider,localCoordinatorAddress→ws://....GuestStreamTokenProviderTestcovers token sync into the REST auth path and the iOS name-preservation quirk;BindableWritableUserRepositoryTestcovers pre-bind buffering.StreamVideoClientTest.Testing
stream-video-android-core:testDebugUnitTestsuite green in ~2m30s.@Ignorecount unchanged at 21.spotlessCheckgreen.apiCheckshows zero public API diff — theConnectionStatebreaking change is in the follow-up PR.Heads-up for future test authors:
StreamVideoBuilder.build()constructs the connection module before the token-provider dispatch, and the module hitsgetSystemService(CONNECTIVITY_SERVICE) as ConnectivityManager— undermockk<Context>(relaxed = true)that throwsClassCastException. Tests using the builder in-process must stubCONNECTIVITY_SERVICE. SeeStreamVideoBuilderDispatchTest.builderFor(...)for the pattern.Intermediate-state note
On this PR alone,
ClientState.connectiononly updates viaConnectedEvent; disconnect transitions become visible when #1739 wireshandleStreamState. Merge the stack together.