From 8077406cba9833a42359a87f10a66f62b10211ef Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 12 May 2022 18:45:19 +0200 Subject: [PATCH] code review --- .../android/sdk/common/CryptoTestHelper.kt | 2 +- .../crypto/keysbackup/KeysBackupTest.kt | 3 --- .../crypto/keysbackup/KeysBackupTestHelper.kt | 2 +- .../crypto/crosssigning/CrossSigningOlm.kt | 2 +- .../keysbackup/DefaultKeysBackupService.kt | 1 - .../settings/KeysBackupManageActivity.kt | 5 ++--- .../settings/KeysBackupSettingsViewModel.kt | 20 ++++++++++++------- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt index f3bd9104c1..348841313b 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt @@ -231,7 +231,7 @@ class CryptoTestHelper(private val testHelper: CommonTestHelper) { return cryptoTestData } - private fun ensureEventReceived(roomId: String, eventId: String, session: Session, andCanDecrypt : Boolean) { + private fun ensureEventReceived(roomId: String, eventId: String, session: Session, andCanDecrypt: Boolean) { testHelper.waitWithLatch { latch -> testHelper.retryPeriodicallyWithLatch(latch) { val timeLineEvent = session.getRoom(roomId)?.timelineService()?.getTimelineEvent(eventId) 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 c2abbd39e6..b792d18822 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.Test import org.junit.runner.RunWith import org.junit.runners.MethodSorters @@ -63,7 +62,6 @@ class KeysBackupTest : InstrumentedTest { */ @Test fun roomKeysTest_testBackupStore_ok() { - val testHelper = CommonTestHelper(context()) val cryptoTestHelper = CryptoTestHelper(testHelper) @@ -105,7 +103,6 @@ class KeysBackupTest : InstrumentedTest { */ @Test fun prepareKeysBackupVersionTest() { - val testHelper = CommonTestHelper(context()) val cryptoTestHelper = CryptoTestHelper(testHelper) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt index 877d9a86ec..2220536e28 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt @@ -113,7 +113,7 @@ internal class KeysBackupTestHelper( keysBackup.createKeysBackupVersion(megolmBackupCreationInfo, it) } - Assert.assertNotNull("Key backup version should not be null",keysVersion.version) + Assert.assertNotNull("Key backup version should not be null", keysVersion.version) // Backup must be enable now Assert.assertTrue(keysBackup.isEnabled) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/CrossSigningOlm.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/CrossSigningOlm.kt index 311e0ba822..4fa355cd2a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/CrossSigningOlm.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/CrossSigningOlm.kt @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Matrix.org Foundation C.I.C. + * Copyright 2022 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. 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 0b54199f38..9754e1e7c2 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 @@ -347,7 +347,6 @@ internal class DefaultKeysBackupService @Inject constructor( override fun backupAllGroupSessions(progressListener: ProgressListener?, callback: MatrixCallback?) { - if (!isEnabled || backupOlmPkEncryption == null || keysBackupVersion == null) { callback?.onFailure(Throwable("Backup not enabled")) return diff --git a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupManageActivity.kt b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupManageActivity.kt index 6893e8e76f..e58746193b 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupManageActivity.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupManageActivity.kt @@ -98,9 +98,8 @@ class KeysBackupManageActivity : SimpleFragmentActivity() { is KeysBackupViewEvents.RequestStore4SSecret -> { secretStartForActivityResult.launch( SharedSecureStorageActivity.newWriteIntent( - this, - null, // default key - listOf(KEYBACKUP_SECRET_SSSS_NAME to it.recoveryKey) + context = this, + writeSecrets = listOf(KEYBACKUP_SECRET_SSSS_NAME to it.recoveryKey) ) ) } diff --git a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupSettingsViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupSettingsViewModel.kt index 3a76b5cdd8..b47b84af71 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupSettingsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupSettingsViewModel.kt @@ -156,14 +156,20 @@ class KeysBackupSettingsViewModel @AssistedInject constructor(@Assisted initialS suspend fun completeBackupCreation() { val info = pendingBackupCreationInfo ?: return - val version = awaitCallback { - session.cryptoService().keysBackupService().createKeysBackupVersion(info, it) - } - // Save it for gossiping - Timber.d("## BootstrapCrossSigningTask: Creating 4S - Save megolm backup key for gossiping") - session.cryptoService().keysBackupService().saveBackupRecoveryKey(info.recoveryKey, version = version.version) + try { + val version = awaitCallback { + session.cryptoService().keysBackupService().createKeysBackupVersion(info, it) + } + // Save it for gossiping + Timber.d("## BootstrapCrossSigningTask: Creating 4S - Save megolm backup key for gossiping") + session.cryptoService().keysBackupService().saveBackupRecoveryKey(info.recoveryKey, version = version.version) + } catch (failure: Throwable) { + // XXX mm... failed we should remove what we put in 4S, as it was not created? - // TODO catch, delete 4S account data + // for now just stay on the screen, user can retry, there is no api to delete account data + } finally { + pendingBackupCreationInfo = null + } } private fun deleteCurrentBackup() {