-
Notifications
You must be signed in to change notification settings - Fork 1.4k
refactor(voip): extract DDP layer into injectable interfaces (Phase 1) #7121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||
| package chat.rocket.reactnative.voip.credentials | ||||||||||||||||||
|
|
||||||||||||||||||
| import android.content.ContentResolver | ||||||||||||||||||
| import android.provider.Settings | ||||||||||||||||||
| import chat.rocket.reactnative.notification.Ejson | ||||||||||||||||||
|
|
||||||||||||||||||
| open class MMKVVoipCredentialsProvider( | ||||||||||||||||||
| private val contentResolver: ContentResolver, | ||||||||||||||||||
| private val host: String | ||||||||||||||||||
| ) : VoipCredentialsProvider { | ||||||||||||||||||
|
|
||||||||||||||||||
| protected open fun createEjson(): Ejson = | ||||||||||||||||||
| Ejson().apply { this.host = host } | ||||||||||||||||||
|
|
||||||||||||||||||
| protected open val ejson: Ejson by lazy { createEjson() } | ||||||||||||||||||
|
|
||||||||||||||||||
| override fun userId(): String? { | ||||||||||||||||||
| val userId = ejson.userId() | ||||||||||||||||||
| return userId.ifEmpty { null } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| override fun token(): String? { | ||||||||||||||||||
| val token = ejson.token() | ||||||||||||||||||
| return token.ifEmpty { null } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| override fun deviceId(): String = | ||||||||||||||||||
| Settings.Secure.getString(contentResolver, Settings.Secure.ANDROID_ID) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -type f -name "MMKVVoipCredentialsProvider.kt" | head -5Repository: RocketChat/Rocket.Chat.ReactNative Length of output: 177 🏁 Script executed: fd "MMKVVoipCredentialsProvider" --type fRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 277 🏁 Script executed: find . -type f -path "*voip/credentials*" | head -20Repository: RocketChat/Rocket.Chat.ReactNative Length of output: 377 🏁 Script executed: git ls-files | grep -i "voip\|credentials" | head -20Repository: RocketChat/Rocket.Chat.ReactNative Length of output: 1715 🏁 Script executed: git ls-files | head -30Repository: RocketChat/Rocket.Chat.ReactNative Length of output: 1103 🏁 Script executed: rg "MMKVVoipCredentialsProvider" --type kt -lRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 104 🏁 Script executed: rg "Settings\.Secure\.getString" -lRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 629 🏁 Script executed: ls -laRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 3421 🏁 Script executed: rg "fun deviceId" --type ktRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 104 🌐 Web query:
💡 Result:
The AOSP framework source for the same method also documents “or null if not present”. [2] So, in practice (and in Kotlin) you should treat it as nullable (i.e., 🏁 Script executed: cat -n android/app/src/main/java/chat/rocket/reactnative/voip/credentials/MMKVVoipCredentialsProvider.ktRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 1109 🏁 Script executed: cat -n android/app/src/test/java/chat/rocket/reactnative/voip/credentials/MMKVVoipCredentialsProviderTest.ktRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 4528 🏁 Script executed: cat -n android/app/src/main/java/chat/rocket/reactnative/voip/credentials/VoipCredentialsProvider.ktRepository: RocketChat/Rocket.Chat.ReactNative Length of output: 501 🏁 Script executed: rg "deviceId()" --context 3Repository: RocketChat/Rocket.Chat.ReactNative Length of output: 28327 Handle nullable return from
Suggested fixoverride fun deviceId(): String =
Settings.Secure.getString(contentResolver, Settings.Secure.ANDROID_ID)
+ ?.takeIf { it.isNotBlank() }
+ ?: ""📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package chat.rocket.reactnative.voip.credentials | ||
|
|
||
| /** | ||
| * Placeholder interface for Slice 3 implementation. | ||
| * Slice 1 requires this to exist so DdpClientFactory compiles. | ||
| * Slice 3 will provide the full implementation. | ||
| */ | ||
| interface VoipCredentialsProvider { | ||
| fun userId(): String? | ||
| fun token(): String? | ||
| fun deviceId(): String | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package chat.rocket.reactnative.voip.ddp | ||
|
|
||
| import org.json.JSONArray | ||
| import org.json.JSONObject | ||
|
|
||
| interface DdpClient { | ||
| var onCollectionMessage: ((JSONObject) -> Unit)? | ||
| fun connect(host: String, callback: (Boolean) -> Unit) | ||
| fun login(token: String, callback: (Boolean) -> Unit) | ||
| fun subscribe(name: String, params: JSONArray, callback: (Boolean) -> Unit) | ||
| fun callMethod(method: String, params: JSONArray, callback: (Boolean) -> Unit) | ||
| fun queueMethodCall(method: String, params: JSONArray, callback: (Boolean) -> Unit = {}) | ||
| fun hasQueuedMethodCalls(): Boolean | ||
| fun flushQueuedMethodCalls() | ||
| fun clearQueuedMethodCalls() | ||
| fun disconnect() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid a companion-scoped
Contextfor signaling state.Static flows like
DeclineReceiver.onReceive()andhandleAcceptAction()can run after a process restart, before anyVoipNotificationinstance executesinit. In that case the first accept/reject signal path dereferencescontext!!here and fails instead of sending the DDP message.🐛 Proposed fix
Then update the companion helpers to call
callSignalSender(context)instead of the global lazy instance.Also applies to: 139-180
🤖 Prompt for AI Agents