diff --git a/changelog.d/6585.bugfix b/changelog.d/6585.bugfix new file mode 100644 index 00000000000..63bf5a0af63 --- /dev/null +++ b/changelog.d/6585.bugfix @@ -0,0 +1 @@ +Fix backup saving several times the same keys diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt index e160938721f..2439119f01c 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt @@ -24,7 +24,6 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.FixMethodOrder -import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -56,7 +55,6 @@ import java.util.concurrent.CountDownLatch @RunWith(AndroidJUnit4::class) @FixMethodOrder(MethodSorters.JVM) @LargeTest -@Ignore class KeysBackupTest : InstrumentedTest { @get:Rule val rule = RetryTestRule(3) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/InboundGroupSessionStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/InboundGroupSessionStore.kt index ab7cbb74b11..39dfb721490 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/InboundGroupSessionStore.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/InboundGroupSessionStore.kt @@ -21,13 +21,10 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import org.matrix.android.sdk.api.MatrixCoroutineDispatchers -import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.logger.LoggerTag import org.matrix.android.sdk.internal.crypto.model.MXInboundMegolmSessionWrapper import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import timber.log.Timber -import java.util.Timer -import java.util.TimerTask import javax.inject.Inject internal data class InboundGroupSessionHolder( @@ -57,18 +54,13 @@ internal class InboundGroupSessionStore @Inject constructor( if (oldValue != null) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { Timber.tag(loggerTag.value).v("## Inbound: entryRemoved ${oldValue.wrapper.roomId}-${oldValue.wrapper.senderKey}") - store.storeInboundGroupSessions(listOf(oldValue).map { it.wrapper }) + // store.storeInboundGroupSessions(listOf(oldValue).map { it.wrapper }) oldValue.wrapper.session.releaseSession() } } } } - private val timer = Timer() - private var timerTask: TimerTask? = null - - private val dirtySession = mutableListOf() - @Synchronized fun clear() { sessionCache.evictAll() @@ -90,7 +82,6 @@ internal class InboundGroupSessionStore @Inject constructor( @Synchronized fun replaceGroupSession(old: InboundGroupSessionHolder, new: InboundGroupSessionHolder, sessionId: String, senderKey: String) { Timber.tag(loggerTag.value).v("## Replacing outdated session ${old.wrapper.roomId}-${old.wrapper.senderKey}") - dirtySession.remove(old) store.removeInboundGroupSession(sessionId, senderKey) sessionCache.remove(CacheKey(sessionId, senderKey)) @@ -107,33 +98,14 @@ internal class InboundGroupSessionStore @Inject constructor( private fun internalStoreGroupSession(holder: InboundGroupSessionHolder, sessionId: String, senderKey: String) { Timber.tag(loggerTag.value).v("## Inbound: getInboundGroupSession mark as dirty ${holder.wrapper.roomId}-${holder.wrapper.senderKey}") - // We want to batch this a bit for performances - dirtySession.add(holder) if (sessionCache[CacheKey(sessionId, senderKey)] == null) { // first time seen, put it in memory cache while waiting for batch insert // If it's already known, no need to update cache it's already there sessionCache.put(CacheKey(sessionId, senderKey), holder) } - - timerTask?.cancel() - timerTask = object : TimerTask() { - override fun run() { - batchSave() - } - } - timer.schedule(timerTask!!, 300) - } - - @Synchronized - private fun batchSave() { - val toSave = mutableListOf().apply { addAll(dirtySession) } - dirtySession.clear() cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - Timber.tag(loggerTag.value).v("## Inbound: getInboundGroupSession batching save of ${toSave.size}") - tryOrNull { - store.storeInboundGroupSessions(toSave.map { it.wrapper }) - } + store.storeInboundGroupSessions(listOf(holder.wrapper)) } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index c4a6488258b..a4575cf5934 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -806,7 +806,6 @@ internal class MXOlmDevice @Inject constructor( } replayAttackMap[messageIndexKey] = eventId } - inboundGroupSessionStore.storeInBoundGroupSession(sessionHolder, sessionId, senderKey) val payload = try { val adapter = MoshiProvider.providesMoshi().adapter(JSON_DICT_PARAMETERIZED_TYPE) val payloadString = convertFromUTF8(decryptResult.mDecryptedMessage) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt index 49cf60d0518..8691c087798 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt @@ -1349,6 +1349,8 @@ internal class DefaultKeysBackupService @Inject constructor( // Mark keys as backed up cryptoStore.markBackupDoneForInboundGroupSessions(olmInboundGroupSessionWrappers) + // we can release the sessions now + olmInboundGroupSessionWrappers.onEach { it.session.releaseSession() } if (olmInboundGroupSessionWrappers.size < KEY_BACKUP_SEND_KEYS_MAX_COUNT) { Timber.v("backupKeys: All keys have been backed up") diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt index 20ca357d1ab..f5468634cbf 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt @@ -763,11 +763,17 @@ internal class RealmCryptoStore @Inject constructor( // } ?: false val key = OlmInboundGroupSessionEntity.createPrimaryKey(sessionIdentifier, wrapper.sessionData.senderKey) + val existing = realm.where() + .equalTo(OlmInboundGroupSessionEntityFields.PRIMARY_KEY, key) + .findFirst() + val realmOlmInboundGroupSession = OlmInboundGroupSessionEntity().apply { primaryKey = key store(wrapper) + backedUp = existing?.backedUp ?: false } - Timber.i("## CRYPTO | shouldShareHistory: ${wrapper.sessionData.sharedHistory} for $key") + + Timber.v("## CRYPTO | shouldShareHistory: ${wrapper.sessionData.sharedHistory} for $key") realm.insertOrUpdate(realmOlmInboundGroupSession) } }