code review

element-hq · May 12, 2022 · 8077406 · 8077406
1 parent 5a323db
commit 8077406
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 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
@@ -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/.../src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt b/.../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/...ndroidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt b/...ndroidTest/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/...roid/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/CrossSigningOlm.kt b/...roid/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/...c/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt b/...c/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<Unit>?) {
-
         if (!isEnabled || backupOlmPkEncryption == null || keysBackupVersion == null) {
             callback?.onFailure(Throwable("Backup not enabled"))
             return

diff --git a/...c/main/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupManageActivity.kt b/...c/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/...ain/java/im/vector/app/features/crypto/keysbackup/settings/KeysBackupSettingsViewModel.kt b/...ain/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<KeysVersion> {
-            session.cryptoService().keysBackupService().createKeysBackupVersion(info, it)
+        try {
+            val version = awaitCallback<KeysVersion> {
+                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?
+
+            // for now just stay on the screen, user can retry, there is no api to delete account data
+        } finally {
+            pendingBackupCreationInfo = null
         }
-        // Save it for gossiping
-        Timber.d("## BootstrapCrossSigningTask: Creating 4S - Save megolm backup key for gossiping")
-        session.cryptoService().keysBackupService().saveBackupRecoveryKey(info.recoveryKey, version = version.version)
-
-        // TODO catch, delete 4S account data
     }
 
     private fun deleteCurrentBackup() {