From 5e7b36d319d54920610af47d6b49871376754c40 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Feb 2021 22:49:07 +0100 Subject: [PATCH] Fix issue with progress --- .../DefaultInitialSyncProgressService.kt | 40 +++++++++---------- .../session/sync/CryptoSyncHandler.kt | 2 +- .../internal/session/sync/RoomSyncHandler.kt | 2 +- .../session/sync/SyncResponseHandler.kt | 12 +++--- .../sdk/internal/session/sync/SyncTask.kt | 19 +++++---- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultInitialSyncProgressService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultInitialSyncProgressService.kt index 9c816964db..d84a39f462 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultInitialSyncProgressService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultInitialSyncProgressService.kt @@ -36,7 +36,7 @@ internal class DefaultInitialSyncProgressService @Inject constructor( return status } - fun startTask(@StringRes nameRes: Int, totalProgress: Int, parentWeight: Float = 1f) { + fun startTask(@StringRes nameRes: Int, totalProgress: Int, parentWeight: Float) { // Create a rootTask, or add a child to the leaf if (rootTask == null) { rootTask = TaskInfo(nameRes, totalProgress) @@ -52,20 +52,20 @@ internal class DefaultInitialSyncProgressService @Inject constructor( currentLeaf.child = newTask } - reportProgress(0) + reportProgress(0F) } - fun reportProgress(progress: Int) { + fun reportProgress(progress: Float) { rootTask?.leaf()?.setProgress(progress) } fun endTask(nameRes: Int) { val endedTask = rootTask?.leaf() if (endedTask?.nameRes == nameRes) { - // close it - val parent = endedTask.parent - parent?.child = null - parent?.setProgress(endedTask.offset + (endedTask.totalProgress * endedTask.parentWeight).toInt()) + // Ensure the task progress is complete + endedTask.setProgress(endedTask.totalProgress.toFloat()) + // And close it + endedTask.parent?.child = null } if (endedTask?.parent == null) { status.postValue(InitialSyncProgressService.Status.Idle) @@ -81,9 +81,9 @@ internal class DefaultInitialSyncProgressService @Inject constructor( val totalProgress: Int, val parent: TaskInfo? = null, val parentWeight: Float = 1f, - val offset: Int = parent?.currentProgress ?: 0) { + val offset: Float = parent?.currentProgress ?: 0F) { var child: TaskInfo? = null - var currentProgress: Int = 0 + private var currentProgress: Float = 0F /** * Get the further child @@ -97,17 +97,18 @@ internal class DefaultInitialSyncProgressService @Inject constructor( } /** - * Set progress of the parent if any (which will post value), or post the value + * Set progress of this task and update the parent progress. Last parent will post value. */ - fun setProgress(progress: Int) { + fun setProgress(progress: Float) { + Timber.v("setProgress: $progress / $totalProgress") currentProgress = progress // val newProgress = Math.min(currentProgress + progress, totalProgress) if (parent != null) { - val parentProgress = (currentProgress * parentWeight).toInt() + val parentProgress = (currentProgress / totalProgress) * (parentWeight * parent.totalProgress) parent.setProgress(offset + parentProgress) } else { - Timber.v("--- ${stringProvider.getString(leaf().nameRes)}: $currentProgress") - status.postValue(InitialSyncProgressService.Status.Progressing(leaf().nameRes, currentProgress)) + Timber.v("--- ${stringProvider.getString(leaf().nameRes)}: ${currentProgress.toInt()}") + status.postValue(InitialSyncProgressService.Status.Progressing(leaf().nameRes, currentProgress.toInt())) } } } @@ -116,7 +117,7 @@ internal class DefaultInitialSyncProgressService @Inject constructor( internal inline fun reportSubtask(reporter: DefaultInitialSyncProgressService?, @StringRes nameRes: Int, totalProgress: Int, - parentWeight: Float = 1f, + parentWeight: Float, block: () -> T): T { reporter?.startTask(nameRes, totalProgress, parentWeight) return block().also { @@ -126,13 +127,12 @@ internal inline fun reportSubtask(reporter: DefaultInitialSyncProgressServic internal inline fun Map.mapWithProgress(reporter: DefaultInitialSyncProgressService?, taskId: Int, - weight: Float, + parentWeight: Float, transform: (Map.Entry) -> R): List { - val total = count().toFloat() - var current = 0 - reporter?.startTask(taskId, 100, weight) + var current = 0F + reporter?.startTask(taskId, count() + 1, parentWeight) return map { - reporter?.reportProgress((current / total * 100).toInt()) + reporter?.reportProgress(current) current++ transform.invoke(it) }.also { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/CryptoSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/CryptoSyncHandler.kt index fc476a3dd6..42477e1dd8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/CryptoSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/CryptoSyncHandler.kt @@ -38,7 +38,7 @@ internal class CryptoSyncHandler @Inject constructor(private val cryptoService: fun handleToDevice(toDevice: ToDeviceSyncResponse, initialSyncProgressService: DefaultInitialSyncProgressService? = null) { val total = toDevice.events?.size ?: 0 toDevice.events?.forEachIndexed { index, event -> - initialSyncProgressService?.reportProgress(((index / total.toFloat()) * 100).toInt()) + initialSyncProgressService?.reportProgress(index * 100F / total) // Decrypt event if necessary Timber.i("## CRYPTO | To device event from ${event.senderId} of type:${event.type}") decryptToDeviceEvent(event, null) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt index 979c2888d3..ef2212cdab 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt @@ -162,7 +162,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle ) } realm.insertOrUpdate(roomEntities) - reporter?.reportProgress(index + 1) + reporter?.reportProgress(index + 1F) } } } else { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt index a80b062427..c9376eb9f2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt @@ -51,13 +51,11 @@ internal class SyncResponseHandler @Inject constructor(@SessionDatabase private private val cryptoService: DefaultCryptoService, private val tokenStore: SyncTokenStore, private val processEventForPushTask: ProcessEventForPushTask, - private val pushRuleService: PushRuleService, - private val initialSyncProgressService: DefaultInitialSyncProgressService) { + private val pushRuleService: PushRuleService) { - suspend fun handleResponse(syncResponse: SyncResponse, fromToken: String?) { + suspend fun handleResponse(syncResponse: SyncResponse, fromToken: String?, reporter: DefaultInitialSyncProgressService?) { val isInitialSync = fromToken == null Timber.v("Start handling sync, is InitialSync: $isInitialSync") - val reporter = initialSyncProgressService.takeIf { isInitialSync } measureTimeMillis { if (!cryptoService.isStarted()) { @@ -85,7 +83,7 @@ internal class SyncResponseHandler @Inject constructor(@SessionDatabase private monarchy.awaitTransaction { realm -> measureTimeMillis { Timber.v("Handle rooms") - reportSubtask(reporter, R.string.initial_sync_start_importing_account_rooms, 100, 0.7f) { + reportSubtask(reporter, R.string.initial_sync_start_importing_account_rooms, 1, 0.7f) { if (syncResponse.rooms != null) { roomSyncHandler.handle(realm, syncResponse.rooms, isInitialSync, reporter) } @@ -95,7 +93,7 @@ internal class SyncResponseHandler @Inject constructor(@SessionDatabase private } measureTimeMillis { - reportSubtask(reporter, R.string.initial_sync_start_importing_account_groups, 100, 0.1f) { + reportSubtask(reporter, R.string.initial_sync_start_importing_account_groups, 1, 0.1f) { Timber.v("Handle groups") if (syncResponse.groups != null) { groupSyncHandler.handle(realm, syncResponse.groups, reporter) @@ -106,7 +104,7 @@ internal class SyncResponseHandler @Inject constructor(@SessionDatabase private } measureTimeMillis { - reportSubtask(reporter, R.string.initial_sync_start_importing_account_data, 100, 0.1f) { + reportSubtask(reporter, R.string.initial_sync_start_importing_account_data, 1, 0.1f) { Timber.v("Handle accountData") userAccountDataSyncHandler.handle(realm, syncResponse.accountData) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncTask.kt index dde3da48c6..ea06db7071 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncTask.kt @@ -91,7 +91,7 @@ internal class DefaultSyncTask @Inject constructor( // We might want to get the user information in parallel too userStore.createOrUpdate(userId) initialSyncProgressService.endAll() - initialSyncProgressService.startTask(R.string.initial_sync_start_importing_account, 100) + initialSyncProgressService.startTask(R.string.initial_sync_start_importing_account, 100, 1F) } // Maybe refresh the home server capabilities data we know getHomeServerCapabilitiesTask.execute(GetHomeServerCapabilitiesTask.Params(forceRefresh = false)) @@ -114,7 +114,7 @@ internal class DefaultSyncTask @Inject constructor( } logDuration("INIT_SYNC Database insertion") { - syncResponseHandler.handleResponse(syncResponse, token) + syncResponseHandler.handleResponse(syncResponse, token, initialSyncProgressService) } } } @@ -126,7 +126,7 @@ internal class DefaultSyncTask @Inject constructor( readTimeOut = readTimeOut ) } - syncResponseHandler.handleResponse(syncResponse, token) + syncResponseHandler.handleResponse(syncResponse, token, null) } Timber.v("Sync task finished on Thread: ${Thread.currentThread().name}") } @@ -138,17 +138,20 @@ internal class DefaultSyncTask @Inject constructor( if (workingFile.exists() && status >= InitialSyncStatus.STEP_DOWNLOADED) { // Go directly to the parse step Timber.v("INIT_SYNC file is already here") + reportSubtask(initialSyncProgressService, R.string.initial_sync_start_downloading, 1, 0.3f) { + // Empty task + } } else { initialSyncStatusRepository.setStep(InitialSyncStatus.STEP_DOWNLOADING) val syncResponse = logDuration("INIT_SYNC Perform server request") { - reportSubtask(initialSyncProgressService, R.string.initial_sync_start_server_computing, 0, 0.5f) { + reportSubtask(initialSyncProgressService, R.string.initial_sync_start_server_computing, 1, 0.2f) { getSyncResponse(requestParams, MAX_NUMBER_OF_RETRY_AFTER_TIMEOUT) } } if (syncResponse.isSuccessful) { logDuration("INIT_SYNC Download and save to file") { - reportSubtask(initialSyncProgressService, R.string.initial_sync_start_downloading, 0, 0.5f) { + reportSubtask(initialSyncProgressService, R.string.initial_sync_start_downloading, 1, 0.1f) { syncResponse.body()?.byteStream()?.use { inputStream -> workingFile.outputStream().use { outputStream -> inputStream.copyTo(outputStream) @@ -162,7 +165,9 @@ internal class DefaultSyncTask @Inject constructor( } initialSyncStatusRepository.setStep(InitialSyncStatus.STEP_DOWNLOADED) } - handleSyncFile(workingFile, initSyncStrategy) + reportSubtask(initialSyncProgressService, R.string.initial_sync_start_importing_account, 1, 0.7F) { + handleSyncFile(workingFile, initSyncStrategy) + } // Delete all files workingDir.deleteRecursively() @@ -199,7 +204,7 @@ internal class DefaultSyncTask @Inject constructor( Timber.v("INIT_SYNC $nbOfJoinedRooms rooms, $nbOfJoinedRoomsInFile stored into files") logDuration("INIT_SYNC Database insertion") { - syncResponseHandler.handleResponse(syncResponse, null) + syncResponseHandler.handleResponse(syncResponse, null, initialSyncProgressService) } initialSyncStatusRepository.setStep(InitialSyncStatus.STEP_SUCCESS) }