Add RingingCallJoinInterceptor hook for ringing call activation#1679
Open
rahul-lohra wants to merge 16 commits into
Open
Add RingingCallJoinInterceptor hook for ringing call activation#1679rahul-lohra wants to merge 16 commits into
rahul-lohra wants to merge 16 commits into
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
WalkthroughThis PR introduces a ChangesCall Join Interceptor Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateGate.kt (1)
176-179: 💤 Low valueConsider clarifying
clearAllJobssemantics.The method name
clearAllJobssuggests it cancels jobs, but it only resets references to null without cancellation. This is intentional (the calling coroutine is already active and will complete naturally), but the behavior may be unclear to future maintainers.💡 Consider adding a KDoc comment
+/** + * Resets job references without cancellation. + * Called when a job is already completing (e.g., after interceptor rejection). + */ fun clearAllJobs() { peerConnectionObserverJob.set(null) interceptorJob.set(null) }🤖 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/ActiveStateGate.kt` around lines 176 - 179, The clearAllJobs method currently only nulls the peerConnectionObserverJob and interceptorJob references rather than cancelling them, which can be confusing; add a KDoc above ActiveStateGate.clearAllJobs that clearly states it intentionally does not cancel jobs, explains that cancellation is handled by the caller/coroutine lifecycle, and documents that the method only clears the two fields peerConnectionObserverJob and interceptorJob to release references.demo-app/src/main/kotlin/io/getstream/video/android/DemoCallJoinInterceptor.kt (1)
34-34: ⚡ Quick winRemove unused constructor parameter.
The
callReadyToJoinFlowparameter is declared but never referenced in the implementation. Consider removing it to reduce confusion.♻️ Proposed fix
class DemoCallJoinInterceptor( - private val callReadyToJoinFlow: StateFlow<Boolean>, private val previousRingingStates: Set<RingingState>, ) : CallJoinInterceptor {🤖 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 `@demo-app/src/main/kotlin/io/getstream/video/android/DemoCallJoinInterceptor.kt` at line 34, The constructor parameter callReadyToJoinFlow in class DemoCallJoinInterceptor is unused; remove it from the primary constructor signature and eliminate any corresponding constructor argument at instantiation sites, and update any Kotlin imports or references accordingly so the class compiles without that StateFlow<Boolean> parameter; ensure you only modify the parameter list for DemoCallJoinInterceptor and its call sites, leaving the rest of the class (methods and members) unchanged.demo-app/src/main/kotlin/io/getstream/video/android/App.kt (1)
85-106: ⚡ Quick winUse a structured application scope for readiness observation.
This collector is launched from an ad-hoc
CoroutineScope(Dispatchers.Default). Prefer an owned app scope (SupervisorJob + Dispatchers.Default) so lifecycle/cancellation/error handling stays deterministic.As per coding guidelines "Keep concurrency deterministic—use structured coroutines and avoid global scope".
🤖 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 `@demo-app/src/main/kotlin/io/getstream/video/android/App.kt` around lines 85 - 106, The observeCallReadyToJoin() function currently launches an ad-hoc CoroutineScope(Dispatchers.Default); replace that with a structured, owned application scope (e.g., an appScope backed by SupervisorJob() + Dispatchers.Default) and launch the collector from that scope so cancellation/errors are propagated deterministically. Update observeCallReadyToJoin() to use the shared appScope when collecting StreamVideo.instanceState and when updating callerReadyToJoinFlow / calleeReadyToJoinFlow, and ensure the appScope is created once (SupervisorJob + Dispatchers.Default) at the application-level so lifecycle and error handling are consistent.demo-app/src/main/kotlin/io/getstream/video/android/CallActivity.kt (1)
90-104: ⚡ Quick winUse
lifecycleScopefor ringing observation in this Activity.This observer is launched from an ad-hoc scope. Binding it to
lifecycleScopekeeps cancellation deterministic without manual cleanup dependency onfinish().As per coding guidelines "Keep concurrency deterministic—use structured coroutines and avoid global scope".
🤖 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 `@demo-app/src/main/kotlin/io/getstream/video/android/CallActivity.kt` around lines 90 - 104, The observeRingingState function currently launches an ad-hoc CoroutineScope; replace that with the Activity's lifecycleScope so the observer is tied to the Activity lifecycle (use lifecycleScope.launch instead of CoroutineScope(Dispatchers.Default).launch) and keep using StreamVideo.instanceState, previousRingingStates, and observeRingingJob to manage the flow; you can remove or keep the explicit observeRingingJob?.cancel() if you prefer, but ensure the coroutine is created via lifecycleScope so it is cancelled deterministically with the Activity lifecycle.
🤖 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 `@demo-app/src/main/kotlin/io/getstream/video/android/CallActivity.kt`:
- Around line 64-66: The companion-level mutable flag USE_CALL_JOIN_INTERCEPTOR
should be removed and the interceptor behavior should be driven by an Intent
extra passed to CallActivity; change callers that start CallActivity to
putExtra("EXTRA_USE_CALL_JOIN_INTERCEPTOR", true/false) and inside CallActivity
(e.g., onCreate) read that extra into a private val (e.g.,
useCallJoinInterceptor) and use that instance field instead of the companion
var; if you need a default keep a private constant
DEFAULT_USE_CALL_JOIN_INTERCEPTOR and avoid mutable global state or
UPPER_SNAKE_CASE for non-constants.
In
`@demo-app/src/main/kotlin/io/getstream/video/android/DemoCallJoinInterceptor.kt`:
- Around line 50-52: The current use of previousRingingStates.first { it is
RingingState.Incoming || it is RingingState.Outgoing } can throw
NoSuchElementException if no matching state exists; replace with a safe lookup
(e.g., firstOrNull) and add defensive handling when null is returned so
isIncomingOrOutgoing is computed safely (handle the null case where appropriate:
treat as false, log, or short-circuit) — update the logic around the
isIncomingOrOutgoing variable and any downstream code that assumes a non-null
match.
In
`@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt`:
- Line 199: The Text composable in DirectCallJoinScreen.kt currently uses a
hardcoded label "Use Call Join Interceptor"; replace this literal with a
localized stringResource call (e.g.,
stringResource(R.string.use_call_join_interceptor)) in the Text invocation
inside DirectCallJoinScreen, and add the corresponding key/value ("Use Call Join
Interceptor") to your strings.xml (and localized variants as needed); ensure you
import androidx.compose.ui.res.stringResource and reference the new R.string
identifier from the Text call.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateGate.kt`:
- Around line 98-99: The debug log "ignored duplicate gate launch" is emitted
before the code actually checks for a duplicate; update ActiveStateGate
(launchGate) so the logger.d call happens only when the duplicate condition is
true: after checking if peerConnectionObserverJob.get()?.isActive == true, log
the message immediately before returning (or log conditioned on that same
boolean), instead of logging unconditionally before the check.
---
Nitpick comments:
In `@demo-app/src/main/kotlin/io/getstream/video/android/App.kt`:
- Around line 85-106: The observeCallReadyToJoin() function currently launches
an ad-hoc CoroutineScope(Dispatchers.Default); replace that with a structured,
owned application scope (e.g., an appScope backed by SupervisorJob() +
Dispatchers.Default) and launch the collector from that scope so
cancellation/errors are propagated deterministically. Update
observeCallReadyToJoin() to use the shared appScope when collecting
StreamVideo.instanceState and when updating callerReadyToJoinFlow /
calleeReadyToJoinFlow, and ensure the appScope is created once (SupervisorJob +
Dispatchers.Default) at the application-level so lifecycle and error handling
are consistent.
In `@demo-app/src/main/kotlin/io/getstream/video/android/CallActivity.kt`:
- Around line 90-104: The observeRingingState function currently launches an
ad-hoc CoroutineScope; replace that with the Activity's lifecycleScope so the
observer is tied to the Activity lifecycle (use lifecycleScope.launch instead of
CoroutineScope(Dispatchers.Default).launch) and keep using
StreamVideo.instanceState, previousRingingStates, and observeRingingJob to
manage the flow; you can remove or keep the explicit observeRingingJob?.cancel()
if you prefer, but ensure the coroutine is created via lifecycleScope so it is
cancelled deterministically with the Activity lifecycle.
In
`@demo-app/src/main/kotlin/io/getstream/video/android/DemoCallJoinInterceptor.kt`:
- Line 34: The constructor parameter callReadyToJoinFlow in class
DemoCallJoinInterceptor is unused; remove it from the primary constructor
signature and eliminate any corresponding constructor argument at instantiation
sites, and update any Kotlin imports or references accordingly so the class
compiles without that StateFlow<Boolean> parameter; ensure you only modify the
parameter list for DemoCallJoinInterceptor and its call sites, leaving the rest
of the class (methods and members) unchanged.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateGate.kt`:
- Around line 176-179: The clearAllJobs method currently only nulls the
peerConnectionObserverJob and interceptorJob references rather than cancelling
them, which can be confusing; add a KDoc above ActiveStateGate.clearAllJobs that
clearly states it intentionally does not cancel jobs, explains that cancellation
is handled by the caller/coroutine lifecycle, and documents that the method only
clears the two fields peerConnectionObserverJob and interceptorJob to release
references.
🪄 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: 22260e08-fbf5-4d67-ac6d-afa6c39f623f
⛔ Files ignored due to path filters (1)
stream-video-android-core/src/main/kotlin/io/getstream/android/video/generated/models/CustomVideoEvent.ktis excluded by!**/generated/**
📒 Files selected for processing (13)
demo-app/src/main/kotlin/io/getstream/video/android/App.ktdemo-app/src/main/kotlin/io/getstream/video/android/CallActivity.ktdemo-app/src/main/kotlin/io/getstream/video/android/DemoCallJoinInterceptor.ktdemo-app/src/main/kotlin/io/getstream/video/android/ui/DogfoodingNavHost.ktdemo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.ktstream-video-android-core/api/stream-video-android-core.apistream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateGate.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/Call.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallJoinInterceptor.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.ktstream-video-android-core/src/test/kotlin/io/getstream/video/android/core/ActiveStateGateTest.ktstream-video-android-ui-core/api/stream-video-android-ui-core.apistream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt
Contributor
SDK Size Comparison 📏
|
|
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
Add a join interception hook that lets apps delay or abort call entry after the join response has been applied locally, so participant readiness/status can be synchronized before the user is treated as fully in-call. Mirrors the iOS
CallJoinInterceptorAPI. GetStream/stream-video-swift#1108Implementation
The sdk providers a
CallJoinInterceptor interface. The integrators can pass their own instance ofCallJoinInterceptortoCall.join( callJoinInterceptor)This interface will run just before SDK transitions to [RingingState.Active]. You can throw
CallJoinInterceptionExceptionto abort the join operation.This interface will run a maximum of 5 seconds, after this the transition to
RingingState.Activewill happenFlow Diagram
Example Usage
CallActivity.kt,DemoCallJoinInterceptor.ktandApp.kt🎨 UI Changes
Demo App
Screen.Recording.2026-05-12.at.12.13.04.PM.mov
Testing
Change these timeouts
private const val PEER_CONNECTION_OBSERVER_TIMEOUT = 10_000L
private const val INTERCEPTOR_TIMEOUT_MS = 10_000L
Summary by CodeRabbit
Release Notes