Define StreamVideoClient and Call public interfaces#1641
Open
aleksandar-apostolov wants to merge 3 commits intodevelop-v2from
Open
Define StreamVideoClient and Call public interfaces#1641aleksandar-apostolov wants to merge 3 commits intodevelop-v2from
aleksandar-apostolov wants to merge 3 commits intodevelop-v2from
Conversation
- StreamVideoClient interface maps all current StreamVideo public methods - Call interface maps all public Call methods (excluding deprecated/internal) - Sub-interfaces: CameraManager, MicrophoneManager, SpeakerManager, ScreenShareManager - join() returns Result<Unit> instead of Result<RtcSession> - events exposed as SharedFlow (not MutableSharedFlow) - statsReport/statLatencyHistory exposed as StateFlow (not MutableStateFlow) - Internal methods excluded: fastReconnect, rejoin, migrate, handleEvent, fireEvent
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
aleksandar-apostolov
added
pr:breaking-change
aleksandar-apostolov
changed the title
WalkthroughNew public API interfaces are introduced for the Stream Video Android Core library, defining contracts for call lifecycle management, media device control (camera, microphone, speaker, screen share), event handling, and client-level operations including authentication and push device management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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. 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 (6)
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/StreamVideoClient.kt (1)
110-110: UselistOfinstead ofmutableListOffor default parameter.The default value uses
mutableListOf, but the parameter type isList<SortField>(immutable). This is inconsistent with line 87 wherelistOfis used. UsingmutableListOffor an immutable reference is misleading and incurs unnecessary overhead.♻️ Suggested fix
- sort: List<SortField> = mutableListOf(SortField.Desc("created_at")), + sort: List<SortField> = listOf(SortField.Desc("created_at")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/StreamVideoClient.kt` at line 110, Default parameter `sort: List<SortField> = mutableListOf(SortField.Desc("created_at"))` in StreamVideoClient is using mutableListOf while the parameter type is immutable List; replace the mutableListOf with listOf to match the declared type and avoid unnecessary mutable allocation (update the default to listOf(SortField.Desc("created_at")) for the `sort` parameter in the StreamVideoClient function/constructor).stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.kt (2)
253-253: UselistOfinstead ofmutableListOffor default parameter.Same issue as in
StreamVideoClient.queryMembers()— usingmutableListOffor an immutableListparameter is inconsistent and misleading.♻️ Suggested fix
- sort: List<SortField> = mutableListOf(SortField.Desc("created_at")), + sort: List<SortField> = listOf(SortField.Desc("created_at")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.kt` at line 253, The default parameter for sort uses mutableListOf but the parameter type is immutable List; update the default to use an immutable list (replace mutableListOf(...) with listOf(...)) where the signature declares sort: List<SortField> (e.g., the parameter using SortField.Desc("created_at") in Call.kt) so the default value matches the declared immutability.
162-172: Consider using a builder or options class forcreate()method.SonarCloud flagged this method for having 9 parameters, which exceeds the recommended limit of 7. High parameter counts reduce readability and make call sites error-prone.
Consider grouping related parameters into a
CreateCallOptionsdata class (similar to howjoin()usesCreateCallOptions), or use named parameters consistently at call sites to mitigate this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.kt` around lines 162 - 172, The create(...) method has too many parameters; introduce a CreateCallOptions data class to group related fields (e.g., memberIds, members: List<MemberRequest>?, custom: Map<String,Any>?, settings: CallSettingsRequest?, startsAt: OffsetDateTime?, team: String?, ring: Boolean = false, notify: Boolean = false, video: Boolean? = null) and change the suspend fun create(...) signature to accept a single options: CreateCallOptions parameter (or provide an overload that delegates to the new options-based signature to preserve compatibility); update all call sites to build and pass CreateCallOptions (mirror the pattern used by join()'s CreateCallOptions), preserve default values from the original parameters, and ensure return type Result<GetOrCreateCallResponse> remains unchanged.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/CameraManager.kt (1)
68-69: API inconsistency:listDevices()return type differs fromMicrophoneManager.
CameraManager.listDevices()returnsList<CameraDeviceWrapped>(snapshot), whileMicrophoneManager.listDevices()returnsStateFlow<List<StreamAudioDevice>>(reactive). This inconsistency may confuse API consumers expecting uniform behavior across device managers.Consider aligning the return types, or document the rationale if the difference is intentional (e.g., camera devices are typically static while audio devices may hotplug).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/CameraManager.kt` around lines 68 - 69, CameraManager.listDevices currently returns a snapshot List<CameraDeviceWrapped> which is inconsistent with MicrophoneManager.listDevices (StateFlow<List<StreamAudioDevice>>); to fix either (a) make CameraManager.listDevices return a StateFlow<List<CameraDeviceWrapped>> by backing it with a private MutableStateFlow updated on camera changes and exposing it as StateFlow, or (b) keep the snapshot but add a clearly named listDevicesSnapshot() and document the difference; update usages of CameraManager.listDevices and any public API docs to reflect the chosen approach (references: CameraManager.listDevices, MicrophoneManager.listDevices).stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/MicrophoneManager.kt (1)
61-62: Consider removing the redundantlistDevices()method.The
listDevices()method returnsStateFlow<List<StreamAudioDevice>>, which duplicates the functionality of thedevicesproperty defined on line 44. Having both may confuse API consumers about which to use.If both are needed for backward compatibility reasons, consider documenting the relationship between them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/MicrophoneManager.kt` around lines 61 - 62, The interface declares a redundant listDevices() function which duplicates the existing devices property; remove the duplicate by deleting the public fun listDevices(): StateFlow<List<StreamAudioDevice>> declaration from MicrophoneManager (and any implementations) so callers use the devices StateFlow instead, or if you must keep it for backward-compatibility, replace it with a simple delegating implementation annotated `@Deprecated` and with KDoc that points to the devices property (e.g., fun listDevices() = devices) and update implementations/tests accordingly.stream-video-android-core/api/stream-video-android-core.api (1)
9629-9633: Trim the duplicate device accessors onMicrophoneManager.Lines 9629-9633 expose both
getDevices()andlistDevices(), and both currently surface asStateFlow. If they represent the same data, this is redundant API you’ll have to support indefinitely. Keep one canonical accessor, or makelistDevices()a snapshot API likeCameraManager.listDevices().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/api/stream-video-android-core.api` around lines 9629 - 9633, MicrophoneManager currently exposes both getDevices() and listDevices() as StateFlow, which is redundant; pick a single canonical accessor and remove the duplicate or change listDevices() to a snapshot function. Update the API so MicrophoneManager only exposes getDevices(): StateFlow (or alternatively keep listDevices() as a non-flow snapshot like CameraManager.listDevices()), remove the other redundant declaration, and update any usages of getDevices()/listDevices() to the chosen symbol to avoid breaking callers. Ensure documentation/comments reflect the chosen canonical accessor and its behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stream-video-android-core/api/stream-video-android-core.api`:
- Around line 9493-9597: Public interfaces like Call and StreamVideoClient are
currently open for external implementation which prevents safe evolution; decide
whether external implementations are supported and either (a) mark them as
internal/sealed or annotate with `@RestrictTo`(LIBRARY_GROUP) (or equivalent) and
add a clear KDoc stating they are not for external implementation, or (b)
explicitly document in the API docs/KDoc that external implementations are
supported and will be maintained; additionally, in MicrophoneManager remove the
duplicated API surface by keeping either the devices property or the
listDevices() method (pick one), deprecate the other with a clear replacement in
KDoc/@Deprecated, and update any usages to the retained symbol (devices or
listDevices) to avoid duplication.
- Line 9639: The exported mangled method setAudioBitrateProfile-gIAlu-s
indicates a suspend function taking stream.video.sfu.models.AudioBitrateProfile
is leaking Kotlin JVM name mangling; to fix, add a
`@JvmName`("setAudioBitrateProfile") annotation to the suspend function
declaration (the suspend setAudioBitrateProfile(AudioBitrateProfile) method) so
the generated JVM signature is stable and uses the plain name for Java interop,
then recompile to verify the method name no longer exposes the mangled
identifier.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.kt`:
- Around line 440-443: The startHLS() and stopHLS() methods currently return
Result<Any>, which loses type safety; replace Result<Any> with concrete response
types (e.g., Result<HlsStartResponse> and Result<HlsStopResponse> or the
existing domain-specific response classes used elsewhere) and update their
declarations in Call.startHLS and Call.stopHLS, plus any callers and
implementations to construct and propagate the new typed Result instances;
ensure the new response classes include the fields the callers need (status,
error, metadata) or map existing internal responses to these types.
- Around line 421-430: The public API methods startRecording() /
startRecording(recordingType: RecordingType) and stopRecording() /
stopRecording(recordingType: RecordingType) currently return Result<Any>, which
erases the response type; update their signatures to return a concrete type
instead (e.g., Result<StartRecordingResponse> and Result<StopRecordingResponse>)
or Result<Unit> if there is no payload, and propagate that concrete type through
any callers/implementations (including the Call interface implementation and any
mappers/parsers) so consumers no longer need to cast from Any.
---
Nitpick comments:
In `@stream-video-android-core/api/stream-video-android-core.api`:
- Around line 9629-9633: MicrophoneManager currently exposes both getDevices()
and listDevices() as StateFlow, which is redundant; pick a single canonical
accessor and remove the duplicate or change listDevices() to a snapshot
function. Update the API so MicrophoneManager only exposes getDevices():
StateFlow (or alternatively keep listDevices() as a non-flow snapshot like
CameraManager.listDevices()), remove the other redundant declaration, and update
any usages of getDevices()/listDevices() to the chosen symbol to avoid breaking
callers. Ensure documentation/comments reflect the chosen canonical accessor and
its behavior.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.kt`:
- Line 253: The default parameter for sort uses mutableListOf but the parameter
type is immutable List; update the default to use an immutable list (replace
mutableListOf(...) with listOf(...)) where the signature declares sort:
List<SortField> (e.g., the parameter using SortField.Desc("created_at") in
Call.kt) so the default value matches the declared immutability.
- Around line 162-172: The create(...) method has too many parameters; introduce
a CreateCallOptions data class to group related fields (e.g., memberIds,
members: List<MemberRequest>?, custom: Map<String,Any>?, settings:
CallSettingsRequest?, startsAt: OffsetDateTime?, team: String?, ring: Boolean =
false, notify: Boolean = false, video: Boolean? = null) and change the suspend
fun create(...) signature to accept a single options: CreateCallOptions
parameter (or provide an overload that delegates to the new options-based
signature to preserve compatibility); update all call sites to build and pass
CreateCallOptions (mirror the pattern used by join()'s CreateCallOptions),
preserve default values from the original parameters, and ensure return type
Result<GetOrCreateCallResponse> remains unchanged.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/CameraManager.kt`:
- Around line 68-69: CameraManager.listDevices currently returns a snapshot
List<CameraDeviceWrapped> which is inconsistent with
MicrophoneManager.listDevices (StateFlow<List<StreamAudioDevice>>); to fix
either (a) make CameraManager.listDevices return a
StateFlow<List<CameraDeviceWrapped>> by backing it with a private
MutableStateFlow updated on camera changes and exposing it as StateFlow, or (b)
keep the snapshot but add a clearly named listDevicesSnapshot() and document the
difference; update usages of CameraManager.listDevices and any public API docs
to reflect the chosen approach (references: CameraManager.listDevices,
MicrophoneManager.listDevices).
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/MicrophoneManager.kt`:
- Around line 61-62: The interface declares a redundant listDevices() function
which duplicates the existing devices property; remove the duplicate by deleting
the public fun listDevices(): StateFlow<List<StreamAudioDevice>> declaration
from MicrophoneManager (and any implementations) so callers use the devices
StateFlow instead, or if you must keep it for backward-compatibility, replace it
with a simple delegating implementation annotated `@Deprecated` and with KDoc that
points to the devices property (e.g., fun listDevices() = devices) and update
implementations/tests accordingly.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/StreamVideoClient.kt`:
- Line 110: Default parameter `sort: List<SortField> =
mutableListOf(SortField.Desc("created_at"))` in StreamVideoClient is using
mutableListOf while the parameter type is immutable List; replace the
mutableListOf with listOf to match the declared type and avoid unnecessary
mutable allocation (update the default to listOf(SortField.Desc("created_at"))
for the `sort` parameter in the StreamVideoClient function/constructor).
🪄 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: 31998e60-5551-4870-990b-488bb0a7f21a
📒 Files selected for processing (7)
stream-video-android-core/api/stream-video-android-core.apistream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/Call.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/CameraManager.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/MicrophoneManager.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/ScreenShareManager.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/SpeakerManager.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/api/StreamVideoClient.kt
- Add KDoc clarifying interfaces are not for external implementation - Remove duplicate listDevices() from MicrophoneManager (devices property suffices) - Replace Result<Any> with Result<Unit> on recording and HLS methods
|
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
Define the public interface contracts for the V2 SDK migration. This is the first in a series of PRs migrating stream-video-android to use stream-android-core as its foundation.
This introduces
StreamVideoClientandCallas interfaces with sub-interfaces (CameraManager,MicrophoneManager,SpeakerManager,ScreenShareManager) followingstream-android-core'sapi/internalpackage pattern.Implementation
apipackage instream-video-android-corewith 6 interface filesStreamVideoClientinterface maps all currentStreamVideopublic methods (~17 methods)Callinterface maps currentCallclass (~60 methods) with 4 sub-interface accessors (camera,microphone,speaker,screenShare)by, cross-cutting ones (ScreenShare) implemented directly onCallImplTesting
compileDebugKotlinapiCheckSummary by CodeRabbit
Release Notes