fix: Wrap persistent connection service in try Cherry-pick

wireapp · Oct 23, 2024 · 872013d · 872013d
1 parent 65a289c
commit 872013d
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 37 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt b/app/src/main/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCase.kt
@@ -24,17 +24,17 @@ import android.content.Intent
 import android.os.Build
 import com.wire.android.appLogger
 import com.wire.android.services.PersistentWebSocketService
+import com.wire.android.services.ServicesManager
 import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject
 import javax.inject.Singleton
 
 @Singleton
 class StartPersistentWebsocketIfNecessaryUseCase @Inject constructor(
-    @ApplicationContext private val appContext: Context,
+    private val servicesManager: ServicesManager,
     private val shouldStartPersistentWebSocketService: ShouldStartPersistentWebSocketServiceUseCase
 ) {
     suspend operator fun invoke() {
-        val persistentWebSocketServiceIntent = PersistentWebSocketService.newIntent(appContext)
         shouldStartPersistentWebSocketService().let {
             when (it) {
                 is ShouldStartPersistentWebSocketServiceUseCase.Result.Failure -> {
@@ -43,33 +43,17 @@ class StartPersistentWebsocketIfNecessaryUseCase @Inject constructor(
 
                 is ShouldStartPersistentWebSocketServiceUseCase.Result.Success -> {
                     if (it.shouldStartPersistentWebSocketService) {
-                        startForegroundService(persistentWebSocketServiceIntent)
+                        appLogger.i("${TAG}: Starting PersistentWebsocketService")
+                        servicesManager.startPersistentWebSocketService()
                     } else {
                         appLogger.i("${TAG}: Stopping PersistentWebsocketService, no user with persistent web socket enabled found")
-                        appContext.stopService(persistentWebSocketServiceIntent)
+                        servicesManager.stopPersistentWebSocketService()
                     }
                 }
             }
         }
     }
 
-    private fun startForegroundService(persistentWebSocketServiceIntent: Intent) {
-        when {
-            PersistentWebSocketService.isServiceStarted -> {
-                appLogger.i("${TAG}: PersistentWebsocketService already started, not starting again")
-            }
-
-            else -> {
-                appLogger.i("${TAG}: Starting PersistentWebsocketService")
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-                    appContext.startForegroundService(persistentWebSocketServiceIntent)
-                } else {
-                    appContext.startService(persistentWebSocketServiceIntent)
-                }
-            }
-        }
-    }
-
     companion object {
         const val TAG = "StartPersistentWebsocketIfNecessaryUseCase"
     }

diff --git a/app/src/main/kotlin/com/wire/android/services/PersistentWebSocketService.kt b/app/src/main/kotlin/com/wire/android/services/PersistentWebSocketService.kt
@@ -18,11 +18,13 @@
 
 package com.wire.android.services
 
+import android.app.ForegroundServiceStartNotAllowedException
 import android.app.Notification
 import android.app.Service
 import android.content.Context
 import android.content.Intent
 import android.content.pm.ServiceInfo
+import android.os.Build
 import android.os.IBinder
 import androidx.core.app.NotificationCompat
 import androidx.core.app.ServiceCompat
@@ -131,12 +133,29 @@ class PersistentWebSocketService : Service() {
             .setOngoing(true)
             .build()
 
-        ServiceCompat.startForeground(
-            this,
-            NotificationIds.PERSISTENT_NOTIFICATION_ID.ordinal,
-            notification,
-            ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE
-        )
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+            try {
+                ServiceCompat.startForeground(
+                    this,
+                    NotificationIds.PERSISTENT_NOTIFICATION_ID.ordinal,
+                    notification,
+                    ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE
+                )
+            } catch (e: ForegroundServiceStartNotAllowedException) {
+                // ForegroundServiceStartNotAllowedException may be thrown on restarting service from the background.
+                // this is the only suggested workaround from google for now.
+                // https://issuetracker.google.com/issues/307329994#comment86
+                appLogger.e("Failure while starting foreground: $e")
+                stopSelf()
+            }
+        } else {
+            ServiceCompat.startForeground(
+                this,
+                NotificationIds.PERSISTENT_NOTIFICATION_ID.ordinal,
+                notification,
+                ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE
+            )
+        }
     }
 
     override fun onDestroy() {

diff --git a/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt b/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt
@@ -102,11 +102,15 @@ class ServicesManager @Inject constructor(
 
     // Persistent WebSocket
     fun startPersistentWebSocketService() {
-        startService(PersistentWebSocketService.newIntent(context))
+        if (PersistentWebSocketService.isServiceStarted) {
+            appLogger.i("ServicesManager: PersistentWebsocketService already started, not starting again")
+        } else {
+            startService(PersistentWebSocketService.newIntent(context))
+        }
     }
 
     fun stopPersistentWebSocketService() {
-        stopService(PersistentWebSocketService::class)
+        stopService(PersistentWebSocketService.newIntent(context))
     }
 
     fun isPersistentWebSocketServiceRunning(): Boolean =
@@ -121,8 +125,9 @@ class ServicesManager @Inject constructor(
         }
     }
 
-    private fun stopService(serviceClass: KClass<out Service>) {
-        context.stopService(Intent(context, serviceClass.java))
+    private fun stopService(intent: Intent) {
+        appLogger.i("ServicesManager: stopping service for $intent")
+        context.stopService(intent)
     }
 
     companion object {

diff --git a/...rc/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt b/...rc/test/kotlin/com/wire/android/feature/StartPersistentWebsocketIfNecessaryUseCaseTest.kt
@@ -19,6 +19,7 @@ package com.wire.android.feature
 
 import android.content.ComponentName
 import android.content.Context
+import com.wire.android.services.ServicesManager
 import io.mockk.MockKAnnotations
 import io.mockk.coEvery
 import io.mockk.every
@@ -41,7 +42,7 @@ class StartPersistentWebsocketIfNecessaryUseCaseTest {
             sut.invoke()
 
             // then
-            verify(exactly = 1) { arrangement.applicationContext.startService(any()) }
+            verify(exactly = 1) { arrangement.servicesManager.startPersistentWebSocketService() }
         }
 
     @Test
@@ -56,7 +57,7 @@ class StartPersistentWebsocketIfNecessaryUseCaseTest {
             sut.invoke()
 
             // then
-            verify(exactly = 0) { arrangement.applicationContext.startService(any()) }
+            verify(exactly = 0) { arrangement.servicesManager.startPersistentWebSocketService() }
         }
 
     inner class Arrangement {
@@ -65,16 +66,16 @@ class StartPersistentWebsocketIfNecessaryUseCaseTest {
         lateinit var shouldStartPersistentWebSocketServiceUseCase: ShouldStartPersistentWebSocketServiceUseCase
 
         @MockK
-        lateinit var applicationContext: Context
+        lateinit var servicesManager: ServicesManager
 
         init {
             MockKAnnotations.init(this, relaxUnitFun = true)
-            every { applicationContext.startService(any()) } returns ComponentName.createRelative("dummy", "class")
-            every { applicationContext.stopService(any()) } returns true
+            every { servicesManager.startPersistentWebSocketService() } returns Unit
+            every { servicesManager.stopPersistentWebSocketService() } returns Unit
         }
 
         fun arrange() = this to StartPersistentWebsocketIfNecessaryUseCase(
-            applicationContext,
+            servicesManager,
             shouldStartPersistentWebSocketServiceUseCase
         )