From ba27a601dd6a03ec468b0a629247e2d7ab9a41b2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 1 Apr 2021 11:44:53 +0200 Subject: [PATCH] Dominaezzz' review: remove Request class, just use executeRequest() --- .../internal/crypto/tasks/SendToDeviceTask.kt | 21 +++--- .../crypto/tasks/UploadSignaturesTask.kt | 11 ++-- .../android/sdk/internal/network/Request.kt | 65 ++++++++----------- .../room/membership/joining/InviteTask.kt | 14 ++-- .../session/room/read/SetReadMarkersTask.kt | 10 +-- .../session/room/timeline/PaginationTask.kt | 10 +-- 6 files changed, 61 insertions(+), 70 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt index fe6868968b..b3fb704131 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt +++ b/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, - { - cryptoApi.sendToDevice( - params.eventType, - params.transactionId ?: Random.nextInt(Integer.MAX_VALUE).toString(), - sendToDeviceBody - ) - }, - { - isRetryable = true - maxRetryCount = 3 - } - ) + isRetryable = true, + maxRetryCount = 3 + ) { + cryptoApi.sendToDevice( + params.eventType, + params.transactionId ?: Random.nextInt(Integer.MAX_VALUE).toString(), + sendToDeviceBody + ) + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/UploadSignaturesTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/UploadSignaturesTask.kt index bac886cafc..15cf017dc9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/UploadSignaturesTask.kt +++ b/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 - maxRetryCount = 10 - } - ) + isRetryable = true, + maxRetryCount = 10 + ) { + cryptoApi.uploadSignatures(params.signatures) + } if (response.failures?.isNotEmpty() == true) { throw Throwable(response.failures.toString()) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt index 9f3000ce18..9aa242aecf 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt +++ b/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 executeRequest(globalErrorReceiver: GlobalErrorReceiver?, - noinline requestBlock: suspend () -> DATA): DATA { - return executeRequest(globalErrorReceiver, requestBlock, {}) -} +/** + * 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 executeRequest(globalErrorReceiver: GlobalErrorReceiver?, + isRetryable: Boolean = false, + initialDelay: Long = 100L, + maxDelay: Long = 10_000L, + maxRetryCount: Int = Int.MAX_VALUE, + noinline requestBlock: suspend () -> DATA): DATA { + var currentRetryCount = 0 + var currentDelay = initialDelay -internal suspend inline fun executeRequest(globalErrorReceiver: GlobalErrorReceiver?, - noinline requestBlock: suspend () -> DATA, - initBlock: Request.() -> Unit): DATA { - return Request(globalErrorReceiver, requestBlock) - .apply(initBlock) - .execute() -} - -internal class Request( - 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() - } catch (exception: Throwable) { - throw when (exception) { - is KotlinNullPointerException -> IllegalStateException("The request returned a null body") - is HttpException -> exception.toFailure(globalErrorReceiver) - else -> exception - } + while (true) { + try { + return requestBlock() + } catch (throwable: Throwable) { + val exception = when (throwable) { + is KotlinNullPointerException -> IllegalStateException("The request returned a null body") + is HttpException -> throwable.toFailure(globalErrorReceiver) + 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( 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) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/joining/InviteTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/joining/InviteTask.kt index 0f141c344a..ab1299c34a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/joining/InviteTask.kt +++ b/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, - { roomAPI.invite(params.roomId, body) }, - { - isRetryable = true - maxRetryCount = 3 - } - ) + return executeRequest( + globalErrorReceiver, + isRetryable = true, + maxRetryCount = 3 + ) { + roomAPI.invite(params.roomId, body) + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/SetReadMarkersTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/SetReadMarkersTask.kt index 23a3f2d36c..aa1eaff119 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/read/SetReadMarkersTask.kt +++ b/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, - { roomAPI.sendReadMarker(params.roomId, markers) }, - { isRetryable = true } - ) + executeRequest( + globalErrorReceiver, + isRetryable = true + ) { + roomAPI.sendReadMarker(params.roomId, markers) + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/PaginationTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/PaginationTask.kt index b1ce5c1f26..ac46c9c9ff 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/PaginationTask.kt +++ b/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, - { roomAPI.getRoomMessagesFrom(params.roomId, params.from, params.direction.value, params.limit, filter) }, - { isRetryable = true } - ) + val chunk = executeRequest( + globalErrorReceiver, + isRetryable = true + ) { + roomAPI.getRoomMessagesFrom(params.roomId, params.from, params.direction.value, params.limit, filter) + } return tokenChunkEventPersistor.insertInDb(chunk, params.roomId, params.direction) } }