letro/letro-android
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

Fix sliding sync loop restarts due to expirations

Browse Source 
Both `NotifiableEventResolver` and `DefaultNotificationDrawerManager` were creating new Rust SDK Clients while processing notifications instead of reusing the already existing one.
This commit is contained in:
Jorge Martín
2023-07-17 08:05:12 +02:00
parent b9b3d6633b
commit d69dea6608
6 changed files with 96 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt
View File

@@ -37,7 +37,7 @@ import com.bumble.appyx.navmodel.backstack.operation.push
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.appnav.di.MatrixClientsHolder
import io.element.android.libraries.matrix.ui.di.MatrixClientsHolder
import io.element.android.appnav.intent.IntentResolver
import io.element.android.appnav.intent.ResolvedIntent
import io.element.android.appnav.root.RootPresenter

66
appnav/src/main/kotlin/io/element/android/appnav/di/MatrixClientsHolder.kt
View File

@@ -16,69 +16,3 @@
package io.element.android.appnav.di
import com.bumble.appyx.core.state.MutableSavedStateMap
import com.bumble.appyx.core.state.SavedStateMap
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
import io.element.android.libraries.matrix.api.core.SessionId
import kotlinx.coroutines.runBlocking
import timber.log.Timber
import java.util.concurrent.ConcurrentHashMap
import javax.inject.Inject
private const val SAVE_INSTANCE_KEY = "io.element.android.x.di.MatrixClientsHolder.SaveInstanceKey"
class MatrixClientsHolder @Inject constructor(private val authenticationService: MatrixAuthenticationService) {
private val sessionIdsToMatrixClient = ConcurrentHashMap<SessionId, MatrixClient>()
fun add(matrixClient: MatrixClient) {
sessionIdsToMatrixClient[matrixClient.sessionId] = matrixClient
}
fun removeAll() {
sessionIdsToMatrixClient.clear()
}
fun remove(sessionId: SessionId) {
sessionIdsToMatrixClient.remove(sessionId)
}
fun isEmpty(): Boolean = sessionIdsToMatrixClient.isEmpty()
fun knowSession(sessionId: SessionId): Boolean = sessionIdsToMatrixClient.containsKey(sessionId)
fun getOrNull(sessionId: SessionId): MatrixClient? {
return sessionIdsToMatrixClient[sessionId]
}
@Suppress("UNCHECKED_CAST")
fun restore(state: SavedStateMap?) {
Timber.d("Restore state")
if (state == null || sessionIdsToMatrixClient.isNotEmpty()) return Unit.also {
Timber.w("Restore with non-empty map")
}
val sessionIds = state[SAVE_INSTANCE_KEY] as? Array<SessionId>
Timber.d("Restore matrix session keys = ${sessionIds?.map { it.value }}")
if (sessionIds.isNullOrEmpty()) return
// Not ideal but should only happens in case of process recreation. This ensure we restore all the active sessions before restoring the node graphs.
runBlocking {
sessionIds.forEach { sessionId ->
Timber.d("Restore matrix session: $sessionId")
authenticationService.restoreSession(sessionId)
.onSuccess { matrixClient ->
add(matrixClient)
}
.onFailure {
Timber.e("Fail to restore session")
}
}
}
}
fun save(state: MutableSavedStateMap) {
val sessionKeys = sessionIdsToMatrixClient.keys.toTypedArray()
Timber.d("Save matrix session keys = ${sessionKeys.map { it.value }}")
state[SAVE_INSTANCE_KEY] = sessionKeys
}
}

1
changelog.d/880.bugfix Normal file
View File

@@ -0,0 +1 @@
Fix sliding sync loop restarts due to expirations.

87
libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/di/MatrixClientsHolder.kt Normal file
View File

@@ -0,0 +1,87 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.matrix.ui.di
import com.bumble.appyx.core.state.MutableSavedStateMap
import com.bumble.appyx.core.state.SavedStateMap
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
import io.element.android.libraries.matrix.api.core.SessionId
import kotlinx.coroutines.runBlocking
import timber.log.Timber
import java.util.concurrent.ConcurrentHashMap
import javax.inject.Inject
private const val SAVE_INSTANCE_KEY = "io.element.android.x.di.MatrixClientsHolder.SaveInstanceKey"
@SingleIn(AppScope::class)
class MatrixClientsHolder @Inject constructor(private val authenticationService: MatrixAuthenticationService) {
private val sessionIdsToMatrixClient = ConcurrentHashMap<SessionId, MatrixClient>()
fun add(matrixClient: MatrixClient) {
sessionIdsToMatrixClient[matrixClient.sessionId] = matrixClient
}
fun removeAll() {
sessionIdsToMatrixClient.clear()
}
fun remove(sessionId: SessionId) {
sessionIdsToMatrixClient.remove(sessionId)
}
fun isEmpty(): Boolean = sessionIdsToMatrixClient.isEmpty()
fun knowSession(sessionId: SessionId): Boolean = sessionIdsToMatrixClient.containsKey(sessionId)
fun getOrNull(sessionId: SessionId): MatrixClient? {
return sessionIdsToMatrixClient[sessionId]
}
@Suppress("UNCHECKED_CAST")
fun restore(state: SavedStateMap?) {
Timber.d("Restore state")
if (state == null || sessionIdsToMatrixClient.isNotEmpty()) return Unit.also {
Timber.w("Restore with non-empty map")
}
val sessionIds = state[SAVE_INSTANCE_KEY] as? Array<SessionId>
Timber.d("Restore matrix session keys = ${sessionIds?.map { it.value }}")
if (sessionIds.isNullOrEmpty()) return
// Not ideal but should only happens in case of process recreation. This ensure we restore all the active sessions before restoring the node graphs.
runBlocking {
sessionIds.forEach { sessionId ->
Timber.d("Restore matrix session: $sessionId")
authenticationService.restoreSession(sessionId)
.onSuccess { matrixClient ->
add(matrixClient)
}
.onFailure {
Timber.e("Fail to restore session")
}
}
}
}
fun save(state: MutableSavedStateMap) {
val sessionKeys = sessionIdsToMatrixClient.keys.toTypedArray()
Timber.d("Save matrix session keys = ${sessionKeys.map { it.value }}")
state[SAVE_INSTANCE_KEY] = sessionKeys
}
}

4
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt
View File

@@ -16,6 +16,7 @@
package io.element.android.libraries.push.impl.notifications
import io.element.android.libraries.matrix.ui.di.MatrixClientsHolder
import io.element.android.libraries.androidutils.throttler.FirstThrottler
import io.element.android.libraries.core.cache.CircularCache
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
@@ -60,6 +61,7 @@ class DefaultNotificationDrawerManager @Inject constructor(
private val dispatchers: CoroutineDispatchers,
private val buildMeta: BuildMeta,
private val matrixAuthenticationService: MatrixAuthenticationService,
private val matrixClientsHolder: MatrixClientsHolder,
) : NotificationDrawerManager {
/**
* Lazily initializes the NotificationState as we rely on having a current session in order to fetch the persisted queue of events.
@@ -255,7 +257,7 @@ class DefaultNotificationDrawerManager @Inject constructor(
val currentUser = tryOrNull(
onError = { Timber.e(it, "Unable to retrieve info for user ${sessionId.value}") },
operation = {
val client = matrixAuthenticationService.restoreSession(sessionId).getOrNull()
val client = matrixClientsHolder.getOrNull(sessionId)
// myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
val myUserDisplayName = client?.loadUserDisplayName()?.getOrNull() ?: sessionId.value

6
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt
View File

@@ -16,6 +16,7 @@
package io.element.android.libraries.push.impl.notifications
import io.element.android.libraries.matrix.ui.di.MatrixClientsHolder
import io.element.android.libraries.core.log.logger.LoggerTag
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
@@ -62,12 +63,13 @@ class NotifiableEventResolver @Inject constructor(
private val matrixAuthenticationService: MatrixAuthenticationService,
private val buildMeta: BuildMeta,
private val clock: SystemClock,
private val matrixClientsHolder: MatrixClientsHolder,
) {
suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): NotifiableEvent? {
// Restore session
val session = matrixAuthenticationService.restoreSession(sessionId).getOrNull() ?: return null
val notificationService = session.notificationService()
val client = matrixClientsHolder.getOrNull(sessionId) ?: return null
val notificationService = client.notificationService()
val notificationData = notificationService.getNotification(
userId = sessionId,
roomId = roomId,