Skip to content

Wire StreamClient into StreamVideoClient as coordinator owner#1738

Merged
aleksandar-apostolov merged 17 commits into
develop-v2from
gsd/phase-2-02-streamclient-rewrite
Jul 13, 2026
Merged

Wire StreamClient into StreamVideoClient as coordinator owner#1738
aleksandar-apostolov merged 17 commits into
develop-v2from
gsd/phase-2-02-streamclient-rewrite

Conversation

@aleksandar-apostolov

@aleksandar-apostolov aleksandar-apostolov commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Goal

Closes AND-1282

Replaces our custom coordinator socket with stream-android-core's StreamClient as the owner of the coordinator WebSocket connection. StreamClient now 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

StreamVideoClient now owns a StreamClient instance.
Constructed eagerly in the primary constructor and disposed in cleanup(). Tests inject a mock via a constructor parameter — no more Lazy<> workarounds needed.

Event flow now goes through core: StreamClient fires WebSocket events, our listener forwards them to the existing fireEvent(...) pipeline. Existing subscribers keep working unchanged.

StreamVideoBuilder picks the right token provider based on user type.
Regular users get IntegrationStreamTokenProvider (wrapping the integration's TokenProvider). Guest users get GuestStreamTokenProvider (which calls createGuest internally). Anonymous users get a REST-only client that never opens a coordinator socket; explicit connect attempts fail fast.

Guest and anonymous flows mirror the iOS SDK (StreamVideo.loadGuestUserInfo / initialConnectIfRequired):

  • Guests auto-connect at build time like authenticated users; core's token manager drives createGuest inside connect().
  • The guest JWT is synced into the legacy Retrofit auth path (CoordinatorAuthInterceptor) the moment it is issued, and auth type flips anonymousjwt — REST and WS stay on one token until HTTP is unified through core in a later PR.
  • The server-issued guest identity is adopted into ClientState.user, including iOS's name-preservation quirk for decorated display names.
  • Guest token refresh = createGuest again, single-flighted by StreamTokenManager.

Guest user machinery deleted.
setupGuestUser, createGuestUser, guestUserJob: Deferred<Unit>?, and every apiCall { guestUserJob?.await() } synchronization site are gone. Guest token acquisition happens inside GuestStreamTokenProvider.loadToken(userId) — first invocation calls createGuest, subsequent calls return the cached JWT via core's StreamTokenManager (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.socketConnection field is removed. The legacy CoordinatorSocketConnection class stays on disk (nothing references it), scheduled for deletion in a future cleanup PR.

StreamLifecycleObserver moved to lifecycle/legacy/ package.
SfuSocket still references it, so it can't be deleted yet — the SFU migration will remove it.

Tests:

  • StreamVideoClientTest extended with assertions that StreamClient is subscribed at init, connect() invoked on connectAsync(), disconnect() invoked on cleanup().
  • New StreamVideoBuilderDispatchTest covers the token-provider dispatch: Anonymous → REST-only provider (no throw), Authenticated → Integration provider, Guest → Guest provider, localCoordinatorAddressws://....
  • GuestStreamTokenProviderTest covers token sync into the REST auth path and the iOS name-preservation quirk; BindableWritableUserRepositoryTest covers pre-bind buffering.
  • Anonymous fail-fast covered in StreamVideoClientTest.
  • The four unit tests introduced in the parent PR are all passing now.

Testing

  • Whole-project compile passes (all consumer modules still build).
  • Full stream-video-android-core:testDebugUnitTest suite green in ~2m30s. @Ignore count unchanged at 21.
  • spotlessCheck green.
  • BCV apiCheck shows zero public API diff — the ConnectionState breaking 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 hits getSystemService(CONNECTIVITY_SERVICE) as ConnectivityManager — under mockk<Context>(relaxed = true) that throws ClassCastException. Tests using the builder in-process must stub CONNECTIVITY_SERVICE. See StreamVideoBuilderDispatchTest.builderFor(...) for the pattern.

Intermediate-state note

On this PR alone, ClientState.connection only updates via ConnectedEvent; disconnect transitions become visible when #1739 wires handleStreamState. Merge the stack together.

Summary by CodeRabbit

  • New Features
    • Improved guest-user sign-in, including smoother identity adoption and token handling.
    • Added clearer support for authenticated, guest, and anonymous connection behavior.
    • Added an option to provide a custom underlying client for advanced integrations and testing.
  • Bug Fixes
    • Improved connection recovery, event delivery, cleanup, and token refresh behavior.
    • Anonymous users no longer attempt unsupported real-time connections.
  • Tests
    • Expanded coverage for authentication, connection states, guest identity handling, and lifecycle behavior.

@aleksandar-apostolov aleksandar-apostolov added the pr:new-feature Adds new functionality label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled, or the PR is bot-authored.
  • An issue is linked (Linear ticket or GitHub issue), or the PR is bot-authored.

🎉 Great job! This PR is ready for review.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

SDK Size Comparison 📏

SDK Before After Difference Status
stream-video-android-core 12.05 MB 12.35 MB 0.30 MB 🟡
stream-video-android-ui-xml 5.68 MB 5.73 MB 0.05 MB 🟢
stream-video-android-ui-compose 6.28 MB 5.86 MB -0.43 MB 🚀

@aleksandar-apostolov aleksandar-apostolov force-pushed the gsd/phase-2-02-streamclient-rewrite branch 2 times, most recently from 97f4003 to fa2eeb2 Compare July 7, 2026 16:29
@aleksandar-apostolov aleksandar-apostolov force-pushed the gsd/phase-2-02-streamclient-rewrite branch from c761587 to 6ecf46f Compare July 9, 2026 08:39
Base automatically changed from gsd/phase-2-01-adapters to develop-v2 July 10, 2026 07:48
Eagerly inject core v5 StreamClient into StreamVideoClient's primary
constructor, 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; builder tests are extended
separately.

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: streamClientListener.onState currently no-ops; the handleStreamState
routing lands alongside the ConnectionState sealed-interface reshape.
New StreamVideoBuilderDispatchTest exercises the `when (user.type)`
dispatch in StreamVideoBuilder.build() through the streamClientFactory
seam:

- Authenticated -> IntegrationStreamTokenProvider
- Guest -> GuestStreamTokenProvider
- Anonymous -> IllegalStateException on token load
- localCoordinatorAddress "10.0.2.2:3030" threads through into
  args.resolvedWssUrl as "ws://10.0.2.2:3030/video/connect"

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.
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 until the legacy socket interfaces
   are removed later in v2.

2. StreamLifecycleObserver.kt moved from `lifecycle/` to `lifecycle/legacy/`.
   Coordinator-side lifecycle observation now flows through core's
   StreamLifecycleMonitor. Legacy consumers (Coordinator/SFU socket files
   kept for their own later migration) have their imports updated to
   `lifecycle.legacy.StreamLifecycleObserver`. The SFU migrates separately;
   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
  the SFU (still in-tree). Preserved unannotated so SFU coverage is not
  lost.

@ignore baseline unchanged at 21.
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 (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.
Forked test JVMs do not inherit org.gradle.jvmargs and run on
Gradle's 512m default. The suite (Robolectric framework sandbox,
MockK inline instrumentation, OkHttp mock servers in a single
reused executor) now exceeds that on CI: the executor died with
OutOfMemoryError across OkHttp/Robolectric/test-worker threads.
The earlier intermittent 'Can't instantiate proxy' MockK failures
were the same memory pressure surfacing sooner.
2g still produced OutOfMemoryError on CI (7 occurrences across
OkHttp/Robolectric/test-worker threads in a single run). The single
reused executor accumulates Robolectric sandboxes and MockK inline
recordings across the whole suite; 3g gives the headroom the runner
needs.
3g still hit OutOfMemoryError on CI while the same suite passes
locally under coverage - CI runs the root :testCoverage with
org.gradle.parallel=true, so core's executor competes with Paparazzi
and every other module's test JVM for runner memory. Growing the
heap worsens the overcommit. Bound the executor's accumulation
(Robolectric sandboxes, MockK recordings) by forking every 200
tests and return to 2g.
Every successful StreamVideoBuilder.build() ends in
StreamVideo.install(client); install() replaces the previous
singleton without cleaning it up, so each test leaked a full client
(live ClientScope, OkHttp pools, notification manager) for the
executor's lifetime. Four leaked clients per fork plus kover
instrumentation exhausted the CI executor heap regardless of size -
the same reason StreamVideoBuilderTest and ClientAndAuthTest run in
the single-fork isolatedTest task.

removeClient() cleans up the retained client; unmockkAll() drops the
MockK inline recordings.
The core unit-test executor dies with OutOfMemoryError only on CI
(512m, 2g, 3g, fork-cycling, and test teardown all failed to stop
it) while the same suite passes locally under coverage. Write the
heap dump and GC log into the reports directory so the
unit-tests-results artifact carries them; temporary until the
retainer is identified.
The shared DispatcherRule installed one virtual-time dispatcher as both
Main and DispatcherProvider.IO. Integration tests await responses from
the live coordinator; while a runTest body is suspended on a real
network round trip, the test scheduler fast-forwards virtual time, and
core StreamClient's health monitor — a delay-driven loop on IO that
reschedules itself unconditionally — spins millions of iterations, each
logging at the client's DEBUG level. Robolectric's ShadowLog statically
buffers every android.util.Log line, so the executor heap fills with
LogItems (2 GB / 95% of heap, 5.6M items in the CI heap dump) and the
fork dies with OOM. CI-only because the coordinator answers in
milliseconds locally, bounding the loop.

DispatcherRule now takes a separate ioDispatcher and IntegrationTestBase
overrides it to real Dispatchers.IO: virtual time stays on Main for
determinism, IO runs on the same clock as the network the tests talk to.
Health pings return to real intervals — a handful per test.
With integration-test IO on real dispatchers, CallState work (participant
flow propagation, the speaking-while-muted reset job) no longer runs on
the virtual clock, so advanceTimeBy/instant reads race it. Await the
expected state with a real-time timeout instead (awaitRealTime helper on
IntegrationTestBase).
The executor OOM is resolved (ShadowLog flood from virtual-time IO in
integration tests); the heap-dump/GC-log flags served their purpose and
forkEvery(200) never triggered (the suite has fewer than 200 classes).
…order

Same instant-read race as the sibling sorting test: updateParticipant
propagates through the call scope (real IO in integration tests), so a
fixed delay plus an instant value read is racy on slow runners.
…m coverage

Sonar's new-code gate (80%) failed at 70.3% almost entirely because the
moved lifecycle/legacy/StreamLifecycleObserver.kt counts as uncovered new
code; exclude the quarantined package (it is deleted once the remaining
legacy socket consumers migrate). Cover the genuinely new lines: the
StreamClientListener event/error dispatch, the connect branch of
connectIfNotAlreadyConnected, AnonymousStreamTokenProvider.loadToken's
fail-fast, and ClientState.updateUser.
@aleksandar-apostolov aleksandar-apostolov force-pushed the gsd/phase-2-02-streamclient-rewrite branch from 11e6329 to d03a473 Compare July 10, 2026 07:50
@sonarqubecloud

Copy link
Copy Markdown

@aleksandar-apostolov aleksandar-apostolov marked this pull request as ready for review July 10, 2026 10:51
@aleksandar-apostolov aleksandar-apostolov requested a review from a team as a code owner July 10, 2026 10:52
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The PR moves coordinator connection management into StreamClient, adds user-type-specific token providers and guest identity propagation, updates client lifecycle handling and tests, and provides injectable/no-op client factories for tests and preview data.

Changes

StreamClient coordinator migration

Layer / File(s) Summary
Token providers and builder wiring
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/{ClientState.kt,StreamVideoBuilder.kt}, .../socket/coordinator/v2/*, stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/*
Token providers are selected by user type, guest tokens update legacy authentication and user state, and StreamClient construction becomes injectable.
StreamClient connection lifecycle
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt, .../Call.kt, .../internal/module/CoordinatorConnectionModule.kt, .../lifecycle/legacy/*, .../socket/*, stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/StreamVideoClientTest.kt
Connection, events, cleanup, reconnection, and connection IDs use StreamClient instead of the coordinator socket module.
Test runtime and asynchronous validation
stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/*, .../CallStateTest.kt, stream-video-android-core/build.gradle.kts
Tests gain configurable I/O dispatching, real-time waiting, updated connection-state setup, and a 2 GB unit-test heap limit.
Preview and build support
stream-video-android-previewdata/src/main/kotlin/io/getstream/video/android/mock/*, .../StreamPreviewDataUtils.kt, build.gradle.kts
Preview initialization injects a no-op StreamClient, and legacy lifecycle sources are added to Sonar coverage exclusions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Builder
  participant StreamVideoClient
  participant StreamClient
  participant ClientState
  Builder->>StreamClient: construct via factory
  Builder->>StreamVideoClient: inject StreamClient
  Builder->>ClientState: bind guest identity updates
  StreamVideoClient->>StreamClient: subscribe and connect
  StreamClient-->>StreamVideoClient: connection state and video events
Loading

Possibly related PRs

Suggested labels: pr:improvement

Suggested reviewers: rahul-lohra, PratimMallick

Poem

I’m a rabbit watching tokens flow,
Through sockets bright and states aglow.
Guest names settle, tests hop fast,
Preview clients sleep at last.
Carrots for every green build! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: wiring StreamClient into StreamVideoClient as the coordinator owner.
Description check ✅ Passed The description covers the goal, implementation, and testing in detail; the missing UI/checklist/gif sections are acceptable for this non-visual change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gsd/phase-2-02-streamclient-rewrite

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt (1)

232-236: 📐 Maintainability & Code Quality | 🔵 Trivial

The empty onState with the deferred TODO is intentional and tracked for the ConnectionState rewrite in #1739. I can open a tracking issue to ensure handleStreamState(state) wiring isn't lost — let me know.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt`
around lines 232 - 236, Keep the intentionally empty onState implementation
unchanged for now, preserving the TODO documenting replacement with
this@StreamVideoClient.state.handleStreamState(state) after the ConnectionState
rewrite. Do not add interim state-handling logic; track the follow-up wiring
separately if needed.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt`:
- Around line 132-138: Move the testSessionId assignment in IntegrationTestBase
from before connection establishment to immediately after
connectAsync().await(); read the connected connectionId from
streamClient.connectionState only after the connection completes so
waitForConnectionId() does not retain an initial null value.

---

Nitpick comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt`:
- Around line 232-236: Keep the intentionally empty onState implementation
unchanged for now, preserving the TODO documenting replacement with
this@StreamVideoClient.state.handleStreamState(state) after the ConnectionState
rewrite. Do not add interim state-handling logic; track the follow-up wiring
separately if needed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 581095bc-d265-4731-adce-a3278715664f

📥 Commits

Reviewing files that changed from the base of the PR and between c665172 and d03a473.

📒 Files selected for processing (26)
  • build.gradle.kts
  • stream-video-android-core/build.gradle.kts
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/Call.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoBuilder.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/internal/module/CoordinatorConnectionModule.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/lifecycle/legacy/StreamLifecycleObserver.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/coordinator/CoordinatorSocket.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/coordinator/CoordinatorSocketConnection.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/coordinator/v2/AnonymousStreamTokenProvider.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/coordinator/v2/GuestStreamTokenProvider.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/sfu/SfuSocket.kt
  • stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/socket/sfu/SfuSocketConnection.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/BindableWritableUserRepositoryTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/CallStateTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/ClientStateUpdateUserTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/StreamVideoBuilderDispatchTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/StreamVideoClientTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/DispatcherRule.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/TestBase.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/socket/coordinator/v2/AnonymousStreamTokenProviderTest.kt
  • stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/socket/coordinator/v2/GuestStreamTokenProviderTest.kt
  • stream-video-android-previewdata/src/main/kotlin/io/getstream/video/android/mock/NoOpStreamClient.kt
  • stream-video-android-previewdata/src/main/kotlin/io/getstream/video/android/mock/StreamPreviewDataUtils.kt

Comment on lines 132 to +138
clientImpl.testSessionId = runBlocking {
(client as StreamVideoClient).coordinatorConnectionModule.socketConnection.connectionId().value
// Derive the WS connectionId from core's StreamClient state (StreamClient
// replaced the coordinator-socket connectionId() flow). Null before Connected.
(
clientImpl.streamClient.connectionState.value
as? io.getstream.android.core.api.model.connection.StreamConnectionState.Connected
)?.connectionId

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map relevant test base file and related state usage.
git ls-files 'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt' \
             'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/*' \
             'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/**/IntegrationTestState*' \
             'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/**/IntegrationTest*' \
             | sed 's#^`#FILE` #'

echo '--- IntegrationTestBase outline ---'
ast-grep outline stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt --view expanded || true

echo '--- nearby lines ---'
nl -ba stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt | sed -n '110,180p'

echo '--- search for IntegrationTestState and testSessionId ---'
rg -n "IntegrationTestState|testSessionId|connectAsync\(\)\.await|connectionState\.value" stream-video-android-core/src/test/kotlin/io/getstream/video/android/core -S

Repository: GetStream/stream-video-android

Length of output: 2149


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read any related state/test setup files if present.
for f in \
  $(git ls-files 'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/**/IntegrationTestState*.kt' \
                'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/**/IntegrationTest*.kt' \
                'stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/**/Test*.kt' | head -n 20); do
  echo "--- $f ---"
  nl -ba "$f" | sed -n '1,240p'
done

Repository: GetStream/stream-video-android

Length of output: 319


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant test base and shared test-state files.
echo '--- IntegrationTestBase.kt ---'
nl -ba stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt | sed -n '120,190p'

echo '--- files mentioning IntegrationTestState/testSessionId/connectAsync ---'
rg -n "IntegrationTestState|testSessionId|connectAsync\(\)\.await|connectionState\.value|connectionId" stream-video-android-core/src/test/kotlin/io/getstream/video/android/core -S

Repository: GetStream/stream-video-android

Length of output: 239


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt

echo '--- lines 60-170 ---'
sed -n '60,170p' "$FILE"

echo '--- lines 170-260 ---'
sed -n '170,260p' "$FILE"

echo '--- all occurrences of IntegrationTestState / testSessionId in file ---'
grep -nE 'IntegrationTestState|testSessionId|connectAsync\(\)\.await|connectionState\.value|connectionId' "$FILE" || true

Repository: GetStream/stream-video-android

Length of output: 8743


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- references to testSessionId in repository ---'
rg -n "testSessionId" . -S

echo '--- references to connectionState / Connected in stream-video-android-core ---'
rg -n "connectionState\.value|StreamConnectionState\.Connected|connectionId" stream-video-android-core -S

Repository: GetStream/stream-video-android

Length of output: 10548


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt

python3 - <<'PY'
from pathlib import Path
p = Path("stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt")
text = p.read_text()
start = text.index("        if (IntegrationTestState.client == null) {")
end = text.index("            IntegrationTestState.client = client")
print(text[start:end+len("            IntegrationTestState.client = client")])
PY

Repository: GetStream/stream-video-android

Length of output: 1593


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact initialization/connection sequence in IntegrationTestBase.
python3 - <<'PY'
from pathlib import Path
text = Path("stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt").read_text().splitlines()
for i in range(124, 156):
    print(f"{i+1}: {text[i]}")
PY

Repository: GetStream/stream-video-android

Length of output: 1987


Set testSessionId after connecting. The cached client keeps the initial null snapshot here, so waitForConnectionId() can fall back to null later. Move this assignment after connectAsync().await().

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@stream-video-android-core/src/test/kotlin/io/getstream/video/android/core/base/IntegrationTestBase.kt`
around lines 132 - 138, Move the testSessionId assignment in IntegrationTestBase
from before connection establishment to immediately after
connectAsync().await(); read the connected connectionId from
streamClient.connectionState only after the connection completes so
waitForConnectionId() does not retain an initial null value.

@rahul-lohra rahul-lohra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@aleksandar-apostolov aleksandar-apostolov merged commit 17a70c6 into develop-v2 Jul 13, 2026
27 of 30 checks passed
@aleksandar-apostolov aleksandar-apostolov deleted the gsd/phase-2-02-streamclient-rewrite branch July 13, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:new-feature Adds new functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants