Dominaezzz' review: remove Request class, just use executeRequest()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt

@@ -48,17 +48,14 @@ internal class DefaultSendToDeviceTask @Inject constructor(
 
         return executeRequest(
                 globalErrorReceiver,
-                {
+                isRetryable = true,
+                maxRetryCount = 3
+        ) {
             cryptoApi.sendToDevice(
                     params.eventType,
                     params.transactionId ?: Random.nextInt(Integer.MAX_VALUE).toString(),
                     sendToDeviceBody
             )
-                },
-                {
-                    isRetryable = true
-                    maxRetryCount = 3
         }
-        )
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/UploadSignaturesTask.kt

@@ -37,12 +37,11 @@ internal class DefaultUploadSignaturesTask @Inject constructor(
         try {
             val response = executeRequest(
                     globalErrorReceiver,
-                    { cryptoApi.uploadSignatures(params.signatures) },
+                    isRetryable = true,
-                    {
-                        isRetryable = true
                     maxRetryCount = 10
+            ) {
+                cryptoApi.uploadSignatures(params.signatures)
             }
-            )
             if (response.failures?.isNotEmpty() == true) {
                 throw Throwable(response.failures.toString())
             }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt

@@ -25,44 +25,35 @@ import retrofit2.HttpException
 import timber.log.Timber
 import java.io.IOException
 
-// To use when there is no init block to provide
+/**
-internal suspend inline fun <DATA : Any> executeRequest(globalErrorReceiver: GlobalErrorReceiver?,
+ * Execute a request from the requestBlock and handle some of the Exception it could generate
+ *
+ * @param globalErrorReceiver will be use to notify error such as invalid token error. See [GlobalError]
+ * @param isRetryable if set to true, the request will be executed again in case of error, after a delay
+ * @param initialDelay the first delay to wait before a request is retried. Will be doubled after each retry
+ * @param maxDelay the max delay to wait before a retry
+ * @param maxRetryCount the max number of retries
+ * @param requestBlock a suspend lambda to perform the network request
+ */
+internal suspend inline fun <DATA> executeRequest(globalErrorReceiver: GlobalErrorReceiver?,
+                                                  isRetryable: Boolean = false,
+                                                  initialDelay: Long = 100L,
+                                                  maxDelay: Long = 10_000L,
+                                                  maxRetryCount: Int = Int.MAX_VALUE,
                                                   noinline requestBlock: suspend () -> DATA): DATA {
-    return executeRequest(globalErrorReceiver, requestBlock, {})
+    var currentRetryCount = 0
-}
+    var currentDelay = initialDelay
 
-internal suspend inline fun <DATA : Any> executeRequest(globalErrorReceiver: GlobalErrorReceiver?,
+    while (true) {
-                                                        noinline requestBlock: suspend () -> DATA,
-                                                        initBlock: Request<DATA>.() -> Unit): DATA {
-    return Request(globalErrorReceiver, requestBlock)
-            .apply(initBlock)
-            .execute()
-}
-
-internal class Request<DATA : Any>(
-        private val globalErrorReceiver: GlobalErrorReceiver?,
-        private val requestBlock: suspend () -> DATA
-) {
-
-    var isRetryable = false
-    var initialDelay: Long = 100L
-    var maxDelay: Long = 10_000L
-    var maxRetryCount = Int.MAX_VALUE
-    private var currentRetryCount = 0
-    private var currentDelay = initialDelay
-
-    suspend fun execute(): DATA {
-        return try {
         try {
-                requestBlock()
+            return requestBlock()
-            } catch (exception: Throwable) {
+        } catch (throwable: Throwable) {
-                throw when (exception) {
+            val exception = when (throwable) {
                 is KotlinNullPointerException -> IllegalStateException("The request returned a null body")
-                    is HttpException              -> exception.toFailure(globalErrorReceiver)
+                is HttpException              -> throwable.toFailure(globalErrorReceiver)
-                    else                          -> exception
+                else                          -> throwable
             }
-            }
+
-        } catch (exception: Throwable) {
             // Log some details about the request which has failed. This is less useful than before...
             // Timber.e("Exception when executing request ${apiCall.request().method} ${apiCall.request().url.toString().substringBefore("?")}")
             Timber.e("Exception when executing request")
@@ -79,7 +70,7 @@ internal class Request<DATA : Any>(
             if (isRetryable && currentRetryCount++ < maxRetryCount && exception.shouldBeRetried()) {
                 delay(currentDelay)
                 currentDelay = (currentDelay * 2L).coerceAtMost(maxDelay)
-                return execute()
+                // Try again (loop)
             } else {
                 throw when (exception) {
                     is IOException              -> Failure.NetworkConnection(exception)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/joining/InviteTask.kt

@@ -37,12 +37,12 @@ internal class DefaultInviteTask @Inject constructor(
 
     override suspend fun execute(params: InviteTask.Params) {
         val body = InviteBody(params.userId, params.reason)
-        return executeRequest(globalErrorReceiver,
+        return executeRequest(
-                { roomAPI.invite(params.roomId, body) },
+                globalErrorReceiver,
-                {
+                isRetryable = true,
-                    isRetryable = true
                 maxRetryCount = 3
+        ) {
+            roomAPI.invite(params.roomId, body)
         }
-        )
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/SetReadMarkersTask.kt

@@ -96,10 +96,12 @@ internal class DefaultSetReadMarkersTask @Inject constructor(
             updateDatabase(params.roomId, markers, shouldUpdateRoomSummary)
         }
         if (markers.isNotEmpty()) {
-            executeRequest(globalErrorReceiver,
+            executeRequest(
-                    { roomAPI.sendReadMarker(params.roomId, markers) },
+                    globalErrorReceiver,
-                    { isRetryable = true }
+                    isRetryable = true
-            )
+            ) {
+                roomAPI.sendReadMarker(params.roomId, markers)
+            }
         }
     }
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/PaginationTask.kt

@@ -42,10 +42,12 @@ internal class DefaultPaginationTask @Inject constructor(
 
     override suspend fun execute(params: PaginationTask.Params): TokenChunkEventPersistor.Result {
         val filter = filterRepository.getRoomFilter()
-        val chunk = executeRequest(globalErrorReceiver,
+        val chunk = executeRequest(
-                { roomAPI.getRoomMessagesFrom(params.roomId, params.from, params.direction.value, params.limit, filter) },
+                globalErrorReceiver,
-                { isRetryable = true }
+                isRetryable = true
-        )
+        ) {
+            roomAPI.getRoomMessagesFrom(params.roomId, params.from, params.direction.value, params.limit, filter)
+        }
         return tokenChunkEventPersistor.insertInDb(chunk, params.roomId, params.direction)
     }
 }