fix(conversationlist): Retry conversation sync

app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt

@@ -22,6 +22,15 @@ interface OfflineConversationsRepository {
      */
     val roomListFlow: Flow<List<ConversationModel>>
 
+    /**
+     * Emits when [getRooms] fails to sync with the server (e.g. a dropped/reset connection on a
+     * slow network) while there are no locally cached conversations to fall back on for that
+     * account, so the UI can tell the user why the list is empty instead of failing silently.
+     * A failed sync while conversations are already cached does not emit here, since
+     * [roomListFlow] already has data to show and the sync is a best-effort background refresh.
+     */
+    val syncErrorFlow: Flow<Throwable>
+
     /**
      * Stream of a single conversation, for use in each conversations settings.
      */

app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt

@@ -26,6 +26,7 @@ import com.nextcloud.talk.models.domain.ConversationModel
 import com.nextcloud.talk.utils.ApiUtils
 import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable
 import com.nextcloud.talk.utils.SpreedFeatures
+import com.nextcloud.talk.utils.withRetry
 import io.reactivex.Observer
 import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.disposables.Disposable
@@ -83,6 +84,10 @@ class OfflineFirstConversationsRepository @Inject constructor(
         get() = _conversationFlow
     private val _conversationFlow: MutableSharedFlow<ConversationModel> = MutableSharedFlow()
 
+    override val syncErrorFlow: Flow<Throwable>
+        get() = _syncErrorFlow
+    private val _syncErrorFlow: MutableSharedFlow<Throwable> = MutableSharedFlow()
+
     private val scope = CoroutineScope(Dispatchers.IO)
 
     sealed interface ConversationResult {
@@ -181,10 +186,16 @@ class OfflineFirstConversationsRepository @Inject constructor(
         val includeStatus = isUserStatusAvailable(user)
 
         try {
-            val conversationsList = network.getRooms(user, user.baseUrl!!, includeStatus)
-                .subscribeOn(Schedulers.io())
-                .observeOn(AndroidSchedulers.mainThread())
-                .blockingSingle()
+            val conversationsList = withRetry(
+                retries = NETWORK_FETCH_RETRIES,
+                initialDelayMillis = NETWORK_FETCH_RETRY_INITIAL_DELAY_MS,
+                maxDelayMillis = NETWORK_FETCH_RETRY_MAX_DELAY_MS
+            ) {
+                network.getRooms(user, user.baseUrl!!, includeStatus)
+                    .subscribeOn(Schedulers.io())
+                    .observeOn(AndroidSchedulers.mainThread())
+                    .blockingSingle()
+            }
 
             conversationsFromSync = conversationsList.map {
                 it.asEntity(user.id!!)
@@ -206,6 +217,10 @@ class OfflineFirstConversationsRepository @Inject constructor(
             scope.launch { catchUpRoomsWithNewMessages(user, roomsWithNewMessages) }
         } catch (e: Exception) {
             Log.e(TAG, "Something went wrong when fetching conversations", e)
+            val hasCachedConversations = dao.getConversationsForUser(user.id!!).first().isNotEmpty()
+            if (!hasCachedConversations) {
+                _syncErrorFlow.emit(e)
+            }
         }
         return conversationsFromSync
     }
@@ -340,5 +355,8 @@ class OfflineFirstConversationsRepository @Inject constructor(
         private const val CHAT_API_VERSION = 1
         private const val MAX_ROOMS_TO_CATCH_UP = 20
         private const val MAX_CONCURRENT_CATCH_UPS = 3
+        private const val NETWORK_FETCH_RETRIES = 3
+        private const val NETWORK_FETCH_RETRY_INITIAL_DELAY_MS = 1000L
+        private const val NETWORK_FETCH_RETRY_MAX_DELAY_MS = 8000L
     }
 }

app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt

@@ -58,6 +58,7 @@ import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOn
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.stateIn
@@ -150,6 +151,12 @@ class ConversationsListViewModel @Inject constructor(
             _getRoomsViewState.value = GetRoomsErrorState(it)
         }
 
+    init {
+        repository.syncErrorFlow
+            .onEach { throwable -> _getRoomsViewState.value = GetRoomsErrorState(throwable) }
+            .launchIn(viewModelScope)
+    }
+
     private val _isLoadingRooms = MutableStateFlow(true)
 
     /**

app/src/main/java/com/nextcloud/talk/ui/CharacterAvatar.kt

@@ -36,12 +36,7 @@ import com.nextcloud.talk.utils.ActorAvatar
  * character scales with the space the avatar is given, so callers only size the modifier.
  */
 @Composable
-fun CharacterAvatar(
-    character: String,
-    backgroundColor: Color,
-    textColor: Color,
-    modifier: Modifier = Modifier
-) {
+fun CharacterAvatar(character: String, backgroundColor: Color, textColor: Color, modifier: Modifier = Modifier) {
     BoxWithConstraints(modifier = modifier, contentAlignment = Alignment.Center) {
         // Sized to the shorter side and centered, so a circle stays a circle instead of being
         // stretched into a pill when the space the avatar is given is not square

app/src/main/java/com/nextcloud/talk/utils/CharacterAvatarUtils.kt

@@ -22,11 +22,8 @@ sealed interface ActorAvatar {
      * A character on a coloured circle: the first character of a guest's name, or the shell prompt
      * of a bot. Both colours are theme-aware resources, as the web client's are.
      */
-    data class Character(
-        val character: String,
-        @ColorRes val backgroundColor: Int,
-        @ColorRes val textColor: Int
-    ) : ActorAvatar
+    data class Character(val character: String, @ColorRes val backgroundColor: Int, @ColorRes val textColor: Int) :
+        ActorAvatar
 
     /**
      * The app's own icon, for the bots that ship their avatar with the app.

app/src/main/java/com/nextcloud/talk/utils/CoroutineUtils.kt

@@ -7,19 +7,38 @@
 
 package com.nextcloud.talk.utils
 
+import kotlinx.coroutines.delay
+import kotlin.math.min
+import kotlin.math.pow
+
 /**
- * Executes [block] and, if it throws, retries up to [retries] additional times.
- * Equivalent to RxJava's `.retry(retries)`.
+ * Executes [block] and, if it throws, retries up to [retries] additional times, waiting
+ * between attempts starting at [initialDelayMillis] and multiplying by [backoffFactor] after
+ * each failed attempt, capped at [maxDelayMillis].
+ * Equivalent to RxJava's `.retry(retries)`, with exponential backoff.
  * The last exception is rethrown if all attempts fail.
  */
-@Suppress("TooGenericExceptionCaught")
-suspend fun <T> withRetry(retries: Int = 1, block: suspend () -> T): T {
+@Suppress("TooGenericExceptionCaught", "LongParameterList")
+suspend fun <T> withRetry(
+    retries: Int = 1,
+    initialDelayMillis: Long = 0,
+    backoffFactor: Double = 2.0,
+    maxDelayMillis: Long = Long.MAX_VALUE,
+    block: suspend () -> T
+): T {
     var attempt = 0
     while (true) {
         try {
             return block()
         } catch (e: Exception) {
             if (attempt >= retries) throw e
+            if (initialDelayMillis > 0) {
+                val delayMillis = min(
+                    initialDelayMillis * backoffFactor.pow(attempt),
+                    maxDelayMillis.toDouble()
+                ).toLong()
+                delay(delayMillis)
+            }
             attempt++
         }
     }

app/src/test/java/com/nextcloud/talk/utils/CoroutineUtilsTest.kt

@@ -7,9 +7,11 @@
 
 package com.nextcloud.talk.utils
 
+import kotlinx.coroutines.test.currentTime
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertSame
+import org.junit.Assert.assertTrue
 import org.junit.Test
 
 @Suppress("TooGenericExceptionThrown")
@@ -76,4 +78,63 @@ class CoroutineUtilsTest {
             }
             assertEquals(1, callCount)
         }
+
+    @Test
+    fun `withRetry waits initialDelayMillis before the first retry`() =
+        runTest {
+            var callCount = 0
+            withRetry(retries = 1, initialDelayMillis = 1000) {
+                callCount++
+                if (callCount < 2) throw RuntimeException("transient error")
+                "success"
+            }
+            assertEquals(2, callCount)
+            assertEquals(1000, currentTime)
+        }
+
+    @Test
+    fun `withRetry does not wait when initialDelayMillis is zero`() =
+        runTest {
+            var callCount = 0
+            withRetry(retries = 2, initialDelayMillis = 0) {
+                callCount++
+                if (callCount < 3) throw RuntimeException("transient error")
+                "success"
+            }
+            assertEquals(3, callCount)
+            assertTrue(currentTime == 0L)
+        }
+
+    @Test
+    fun `withRetry backs off exponentially between attempts`() =
+        runTest {
+            var callCount = 0
+            withRetry(retries = 2, initialDelayMillis = 1000, backoffFactor = 2.0) {
+                callCount++
+                if (callCount < 3) throw RuntimeException("transient error")
+                "success"
+            }
+            assertEquals(3, callCount)
+            // 1000ms before the 2nd attempt, then 2000ms before the 3rd
+            assertEquals(3000, currentTime)
+        }
+
+    @Test
+    fun `withRetry caps the backoff delay at maxDelayMillis`() =
+        runTest {
+            var callCount = 0
+            withRetry(
+                retries = 3,
+                initialDelayMillis = 1000,
+                backoffFactor = 2.0,
+                maxDelayMillis = 1500
+            ) {
+                callCount++
+                if (callCount < 4) throw RuntimeException("transient error")
+                "success"
+            }
+            assertEquals(4, callCount)
+            // Uncapped would be 1000 + 2000 + 4000; capped at 1500 each time it exceeds it
+            assertEquals(1000 + 1500 + 1500, currentTime)
+        }
 }