[gateway] draft SSE implementation #243
Conversation
WalkthroughThe changes introduce Server-Sent Events (SSE) support to the Android app. This includes adding OkHttp SSE dependencies, a new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant GatewayService
participant SSEForegroundService
participant SSEManager
participant Server
App->>GatewayService: start(context)
GatewayService->>SSEForegroundService: start(context)
SSEForegroundService->>SSEManager: connect()
SSEManager->>Server: Establish SSE connection
Server-->>SSEManager: Send event(s)
SSEManager-->>SSEForegroundService: onEvent callback
SSEForegroundService-->>GatewayService: (optional event handling)
App->>GatewayService: stop(context)
GatewayService->>SSEForegroundService: stop(context)
SSEForegroundService->>SSEManager: disconnect()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
app/src/main/java/me/capcom/smsgateway/helpers/SSEManager.kt (1)
20-23: Consider making timeout configurable and improve scope management.The hardcoded 30-second timeout might need to be configurable for different use cases. Also, consider using
SupervisorJob()for better error isolation.- private val client = OkHttpClient.Builder() - .readTimeout(30, TimeUnit.SECONDS) - .build() - private val scope = CoroutineScope(Dispatchers.IO + Job()) + private val client = OkHttpClient.Builder() + .readTimeout(30, TimeUnit.SECONDS) + .build() + private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/build.gradle(2 hunks)app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/me/capcom/smsgateway/helpers/SSEManager.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt(2 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt(1 hunks)
🔇 Additional comments (8)
app/build.gradle (2)
60-60: LGTM! Version variable properly defined.The OkHttp version variable is well-structured and follows the existing pattern.
84-85: OkHttp 4.10.0 & SSE added – no known CVEs found
Verified via OSS Index forpkg:maven/com.squareup.okhttp3/okhttp@4.10.0: no reported vulnerabilities. Please confirm this version is compatible with your project’s Android API levels (minSdk/targetSdk).• app/build.gradle (lines 84-85):
implementation("com.squareup.okhttp3:okhttp:$okhttp_version") implementation("com.squareup.okhttp3:okhttp-sse:$okhttp_version")app/src/main/java/me/capcom/smsgateway/modules/gateway/GatewayService.kt (3)
12-12: LGTM! Import properly added.The SSEForegroundService import follows the existing import pattern.
49-49: Good placement of SSE service startup.The SSE service is properly started after other workers, which ensures dependencies are initialized first.
57-57: Excellent service shutdown ordering.The SSE service is stopped before other workers, which is the correct order to prevent connection issues during shutdown.
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt (1)
79-93: Excellent service lifecycle management.The companion object methods properly handle Android API differences for starting foreground services and provide clean start/stop functionality.
app/src/main/java/me/capcom/smsgateway/helpers/SSEManager.kt (2)
34-82: Excellent SSE connection implementation.The connection logic properly handles authentication, event callbacks, and error scenarios. The event listener implementation covers all necessary cases.
92-105: Well-implemented reconnection strategy.The exponential backoff pattern with reasonable delay intervals provides good resilience against connection failures.
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt (1)
24-30: Consider adding error handling and state management.The lifecycle methods lack error handling and protection against multiple start/stop calls.
Consider adding basic state management and error handling:
+private var isStarted = false + fun start(context: Context) { + if (isStarted) return + try { eventsReceiver.start() + isStarted = true + } catch (e: Exception) { + Log.e("ReceiverService", "Failed to start events receiver", e) + throw e + } } fun stop(context: Context) { + if (!isStarted) return + try { eventsReceiver.stop() + isStarted = false + } catch (e: Exception) { + Log.e("ReceiverService", "Failed to stop events receiver", e) + } }app/src/main/java/me/capcom/smsgateway/modules/receiver/events/MessagesExportRequestedEvent.kt (1)
7-17: Consider using java.time API instead of DateThe
Dateclass is legacy and has various issues. Consider usingjava.time.Instantorjava.time.LocalDateTimefor better date/time handling.-import java.util.Date +import java.time.Instant class MessagesExportRequestedEvent( - val since: Date, - val until: Date, + val since: Instant, + val until: Instant, ) : AppEvent(NAME) { data class Payload( @SerializedName("since") - val since: Date, + val since: Instant, @SerializedName("until") - val until: Date, + val until: Instant, )app/src/main/java/me/capcom/smsgateway/services/PushService.kt (1)
31-44: Consider adding debug logging for received eventsFor better observability, consider logging the routed events.
override fun onMessageReceived(message: RemoteMessage) { try { Log.d(this.javaClass.name, message.data.toString()) val event = message.data["event"]?.let { ExternalEventType.valueOf(it) } ?: ExternalEventType.MessageEnqueued val data = message.data["data"] + + Log.d(this.javaClass.name, "Routing event: $event with data: $data") eventsRouter.route( ExternalEvent( type = event, data = data, ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/src/main/java/me/capcom/smsgateway/helpers/SSEManager.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/events/ExternalEvent.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/events/ExternalEventType.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/EventsReceiver.kt(3 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/events/MessageEnqueuedEvent.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/events/SettingsUpdatedEvent.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/events/WebhooksUpdatedEvent.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/gateway/workers/WebhooksUpdateWorker.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/orchestrator/EventsRouter.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/orchestrator/Module.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/orchestrator/OrchestratorService.kt(3 hunks)app/src/main/java/me/capcom/smsgateway/modules/push/events/PushMessageEnqueuedEvent.kt(0 hunks)app/src/main/java/me/capcom/smsgateway/modules/push/payloads/MessagesExportRequestedPayload.kt(0 hunks)app/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/modules/receiver/events/MessagesExportRequestedEvent.kt(1 hunks)app/src/main/java/me/capcom/smsgateway/services/PushService.kt(2 hunks)
💤 Files with no reviewable changes (2)
- app/src/main/java/me/capcom/smsgateway/modules/push/payloads/MessagesExportRequestedPayload.kt
- app/src/main/java/me/capcom/smsgateway/modules/push/events/PushMessageEnqueuedEvent.kt
✅ Files skipped from review due to trivial changes (6)
- app/src/main/java/me/capcom/smsgateway/modules/gateway/workers/WebhooksUpdateWorker.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/events/WebhooksUpdatedEvent.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/events/SettingsUpdatedEvent.kt
- app/src/main/java/me/capcom/smsgateway/modules/events/ExternalEvent.kt
- app/src/main/java/me/capcom/smsgateway/modules/events/ExternalEventType.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/events/MessageEnqueuedEvent.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/me/capcom/smsgateway/helpers/SSEManager.kt
- app/src/main/java/me/capcom/smsgateway/modules/gateway/services/SSEForegroundService.kt
🔇 Additional comments (9)
app/src/main/java/me/capcom/smsgateway/modules/orchestrator/Module.kt (1)
8-8: LGTM!The addition of
EventsRouteras a singleton follows the correct Koin DI pattern and aligns with the new event routing architecture.app/src/main/java/me/capcom/smsgateway/modules/orchestrator/OrchestratorService.kt (2)
21-21: LGTM!The addition of
ReceiverServiceas a dependency follows the correct constructor injection pattern.
58-58: LGTM!The service stop order follows the reverse startup sequence, which is the correct lifecycle management pattern.
app/src/main/java/me/capcom/smsgateway/modules/gateway/EventsReceiver.kt (4)
9-11: LGTM!The import updates correctly reflect the new event classes that replace the old push event architecture.
14-15: LGTM!The worker imports are correctly updated to match the new worker classes.
27-28: LGTM!The transition from
PushMessageEnqueuedEventtoMessageEnqueuedEventis correctly implemented while maintaining the same functional behavior.
61-81: LGTM!The new event collectors for
WebhooksUpdatedEventandSettingsUpdatedEventfollow the established patterns:
- Consistent logging and event handling structure
- Proper settings validation before processing
- Correct worker invocation with dependency injection
app/src/main/java/me/capcom/smsgateway/modules/receiver/ReceiverService.kt (1)
22-22: LGTM!Lazy initialization of
eventsReceiveris appropriate for performance optimization.app/src/main/java/me/capcom/smsgateway/modules/receiver/EventsReceiver.kt (1)
1-30: LGTM!The
EventsReceiverimplementation follows established patterns correctly:
- Proper async event handling with coroutineScope and launch
- Consistent logging structure matching other EventsReceiver classes
- Correct dependency injection using Koin
- Clean event delegation to the service layer
The inline use of
get()for context dependency is consistent with patterns used elsewhere in the codebase.
app/src/main/java/me/capcom/smsgateway/modules/orchestrator/EventsRouter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/orchestrator/EventsRouter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/orchestrator/EventsRouter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/orchestrator/OrchestratorService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/me/capcom/smsgateway/modules/receiver/events/MessagesExportRequestedEvent.kt
Show resolved
Hide resolved
Summary by CodeRabbit