From a860b4937bb1ae31c8784ace7a0cd1c8c0c9ab93 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sat, 3 Sep 2022 13:50:16 +0100 Subject: [PATCH 01/11] increasing the amount of in memory room keys that are processed at once - 500 balanced memory with performance from my testing --- .../app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt index 3a559dd..9845899 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt @@ -86,7 +86,7 @@ class RoomKeyImporter( isExported = true, ) } - .chunked(50) + .chunked(500) .forEach { onChunk(it) } } roomIds.toList().ifEmpty { From d406586afaf4c1934df2f49b2c7bbf8e05582b05 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sat, 3 Sep 2022 13:51:14 +0100 Subject: [PATCH 02/11] performance room key inserts in transactions to help improve performance - adds explicit dispatcher contexts when interacting with the olm store --- .../app/dapk/st/olm/OlmPersistenceWrapper.kt | 4 + .../main/kotlin/app/dapk/st/olm/OlmStore.kt | 1 + .../main/kotlin/app/dapk/st/olm/OlmWrapper.kt | 15 ++- .../app/dapk/st/domain/OlmPersistence.kt | 110 ++++++++++++------ .../kotlin/app/dapk/st/domain/StoreModule.kt | 2 +- .../crypto/internal/DefaultCryptoService.kt | 9 +- tools/beta-release/app.js | 1 - 7 files changed, 94 insertions(+), 48 deletions(-) diff --git a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmPersistenceWrapper.kt b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmPersistenceWrapper.kt index c924186..1299244 100644 --- a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmPersistenceWrapper.kt +++ b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmPersistenceWrapper.kt @@ -47,6 +47,10 @@ class OlmPersistenceWrapper( olmPersistence.persist(sessionId, SerializedObject(inboundGroupSession.serialize())) } + override suspend fun transaction(action: suspend () -> Unit) { + olmPersistence.startTransaction { action() } + } + override suspend fun readInbound(sessionId: SessionId): OlmInboundGroupSession? { return olmPersistence.readInbound(sessionId)?.value?.deserialize() } diff --git a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmStore.kt b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmStore.kt index 175e3cd..2f22237 100644 --- a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmStore.kt +++ b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmStore.kt @@ -12,6 +12,7 @@ interface OlmStore { suspend fun read(): OlmAccount? suspend fun persist(olmAccount: OlmAccount) + suspend fun transaction(action: suspend () -> Unit) suspend fun readOutbound(roomId: RoomId): Pair? suspend fun persistOutbound(roomId: RoomId, creationTimestampUtc: Long, outboundGroupSession: OlmOutboundGroupSession) suspend fun persistSession(identity: Curve25519, sessionId: SessionId, olmSession: OlmSession) diff --git a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt index 6a5531a..6707492 100644 --- a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt +++ b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt @@ -46,13 +46,16 @@ class OlmWrapper( override suspend fun import(keys: List) { interactWithOlm() - keys.forEach { - val inBound = when (it.isExported) { - true -> OlmInboundGroupSession.importSession(it.sessionKey) - false -> OlmInboundGroupSession(it.sessionKey) + + olmStore.transaction { + keys.forEach { + val inBound = when (it.isExported) { + true -> OlmInboundGroupSession.importSession(it.sessionKey) + false -> OlmInboundGroupSession(it.sessionKey) + } + logger.crypto("import megolm ${it.sessionKey}") + olmStore.persist(it.sessionId, inBound) } - logger.crypto("import megolm ${it.sessionKey}") - olmStore.persist(it.sessionId, inBound) } } diff --git a/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt b/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt index 80d0a26..675e814 100644 --- a/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt +++ b/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt @@ -4,79 +4,113 @@ import app.dapk.db.DapkDb import app.dapk.db.model.DbCryptoAccount import app.dapk.db.model.DbCryptoMegolmInbound import app.dapk.db.model.DbCryptoMegolmOutbound +import app.dapk.st.core.CoroutineDispatchers +import app.dapk.st.core.withIoContext import app.dapk.st.matrix.common.CredentialsStore import app.dapk.st.matrix.common.Curve25519 import app.dapk.st.matrix.common.RoomId import app.dapk.st.matrix.common.SessionId +import com.squareup.sqldelight.TransactionWithoutReturn +import kotlinx.coroutines.withContext +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine class OlmPersistence( private val database: DapkDb, private val credentialsStore: CredentialsStore, + private val dispatchers: CoroutineDispatchers, ) { suspend fun read(): String? { - return database.cryptoQueries - .selectAccount(credentialsStore.credentials()!!.userId.value) - .executeAsOneOrNull() + return dispatchers.withIoContext { + database.cryptoQueries + .selectAccount(credentialsStore.credentials()!!.userId.value) + .executeAsOneOrNull() + } } suspend fun persist(olmAccount: SerializedObject) { - database.cryptoQueries.insertAccount( - DbCryptoAccount( - user_id = credentialsStore.credentials()!!.userId.value, - blob = olmAccount.value + dispatchers.withIoContext { + database.cryptoQueries.insertAccount( + DbCryptoAccount( + user_id = credentialsStore.credentials()!!.userId.value, + blob = olmAccount.value + ) ) - ) + } } suspend fun readOutbound(roomId: RoomId): Pair? { - return database.cryptoQueries - .selectMegolmOutbound(roomId.value) - .executeAsOneOrNull()?.let { - it.utcEpochMillis to it.blob - } + return dispatchers.withIoContext { + database.cryptoQueries + .selectMegolmOutbound(roomId.value) + .executeAsOneOrNull()?.let { + it.utcEpochMillis to it.blob + } + } } suspend fun persistOutbound(roomId: RoomId, creationTimestampUtc: Long, outboundGroupSession: SerializedObject) { - database.cryptoQueries.insertMegolmOutbound( - DbCryptoMegolmOutbound( - room_id = roomId.value, - blob = outboundGroupSession.value, - utcEpochMillis = creationTimestampUtc, + dispatchers.withIoContext { + database.cryptoQueries.insertMegolmOutbound( + DbCryptoMegolmOutbound( + room_id = roomId.value, + blob = outboundGroupSession.value, + utcEpochMillis = creationTimestampUtc, + ) ) - ) + } } suspend fun persistSession(identity: Curve25519, sessionId: SessionId, olmSession: SerializedObject) { - database.cryptoQueries.insertOlmSession( - identity_key = identity.value, - session_id = sessionId.value, - blob = olmSession.value, - ) + withContext(dispatchers.io) { + database.cryptoQueries.insertOlmSession( + identity_key = identity.value, + session_id = sessionId.value, + blob = olmSession.value, + ) + } } suspend fun readSessions(identities: List): List>? { - return database.cryptoQueries - .selectOlmSession(identities.map { it.value }) - .executeAsList() - .map { Curve25519(it.identity_key) to it.blob } - .takeIf { it.isNotEmpty() } + return withContext(dispatchers.io) { + database.cryptoQueries + .selectOlmSession(identities.map { it.value }) + .executeAsList() + .map { Curve25519(it.identity_key) to it.blob } + .takeIf { it.isNotEmpty() } + } + } + + suspend fun startTransaction(action: suspend TransactionWithoutReturn.() -> Unit) { + val transaction = suspendCoroutine { + database.cryptoQueries.transaction(false) { + it.resume(this) + } + } + dispatchers.withIoContext { + action(transaction) + } } suspend fun persist(sessionId: SessionId, inboundGroupSession: SerializedObject) { - database.cryptoQueries.insertMegolmInbound( - DbCryptoMegolmInbound( - session_id = sessionId.value, - blob = inboundGroupSession.value + withContext(dispatchers.io) { + database.cryptoQueries.insertMegolmInbound( + DbCryptoMegolmInbound( + session_id = sessionId.value, + blob = inboundGroupSession.value + ) ) - ) + } } suspend fun readInbound(sessionId: SessionId): SerializedObject? { - return database.cryptoQueries - .selectMegolmInbound(sessionId.value) - .executeAsOneOrNull() - ?.let { SerializedObject((it)) } + return withContext(dispatchers.io) { + database.cryptoQueries + .selectMegolmInbound(sessionId.value) + .executeAsOneOrNull() + ?.let { SerializedObject((it)) } + } } } diff --git a/domains/store/src/main/kotlin/app/dapk/st/domain/StoreModule.kt b/domains/store/src/main/kotlin/app/dapk/st/domain/StoreModule.kt index 15cbaf1..a90c276 100644 --- a/domains/store/src/main/kotlin/app/dapk/st/domain/StoreModule.kt +++ b/domains/store/src/main/kotlin/app/dapk/st/domain/StoreModule.kt @@ -39,7 +39,7 @@ class StoreModule( fun applicationStore() = ApplicationPreferences(preferences) - fun olmStore() = OlmPersistence(database, credentialsStore()) + fun olmStore() = OlmPersistence(database, credentialsStore(), coroutineDispatchers) fun knownDevicesStore() = DevicePersistence(database, KnownDevicesCache(), coroutineDispatchers) fun profileStore(): ProfileStore = ProfilePersistence(preferences) diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt index a0ecb89..4264d3f 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt @@ -1,5 +1,6 @@ package app.dapk.st.matrix.crypto.internal +import app.dapk.st.core.logP import app.dapk.st.matrix.common.* import app.dapk.st.matrix.crypto.Crypto import app.dapk.st.matrix.crypto.CryptoService @@ -48,8 +49,12 @@ internal class DefaultCryptoService( } override suspend fun InputStream.importRoomKeys(password: String): List { - return with(roomKeyImporter) { - importRoomKeys(password) { importRoomKeys(it) } + return logP("import room keys") { + with(roomKeyImporter) { + importRoomKeys(password) { + importRoomKeys(it) + } + } } } } diff --git a/tools/beta-release/app.js b/tools/beta-release/app.js index a139328..7015cfb 100644 --- a/tools/beta-release/app.js +++ b/tools/beta-release/app.js @@ -150,7 +150,6 @@ const incrementVersionFile = async (github, branchName) => { name: updatedVersionName, } - const encodedContentUpdate = Buffer.from(JSON.stringify(updatedVersionFile, null, 2)).toString('base64') await github.rest.repos.createOrUpdateFileContents({ owner: config.owner, From 4e3da03062e203aecc0366286e79af5ce9a0457a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 11:33:13 +0100 Subject: [PATCH 03/11] inverting log query to see most recent first --- .../src/main/sqldelight/app/dapk/db/model/EventLogger.sq | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/domains/store/src/main/sqldelight/app/dapk/db/model/EventLogger.sq b/domains/store/src/main/sqldelight/app/dapk/db/model/EventLogger.sq index f900c14..a52ec63 100644 --- a/domains/store/src/main/sqldelight/app/dapk/db/model/EventLogger.sq +++ b/domains/store/src/main/sqldelight/app/dapk/db/model/EventLogger.sq @@ -13,12 +13,14 @@ FROM dbEventLog; selectLatestByLog: SELECT id, tag, content, time(utcEpochSeconds,'unixepoch') FROM dbEventLog -WHERE logParent = ?; +WHERE logParent = ? +ORDER BY utcEpochSeconds DESC; selectLatestByLogFiltered: SELECT id, tag, content, time(utcEpochSeconds,'unixepoch') FROM dbEventLog -WHERE logParent = ? AND tag = ?; +WHERE logParent = ? AND tag = ? +ORDER BY utcEpochSeconds DESC; insert: INSERT INTO dbEventLog(tag, content, utcEpochSeconds, logParent) From d05d86a02d0491f98c1dd294b85e958fa41e1df2 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 11:59:08 +0100 Subject: [PATCH 04/11] providing dedicated scope instead of forcing transaction callback to suspend --- .../src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt | 1 - .../kotlin/app/dapk/st/domain/OlmPersistence.kt | 14 +++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt index 6707492..9f9826b 100644 --- a/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt +++ b/domains/olm/src/main/kotlin/app/dapk/st/olm/OlmWrapper.kt @@ -53,7 +53,6 @@ class OlmWrapper( true -> OlmInboundGroupSession.importSession(it.sessionKey) false -> OlmInboundGroupSession(it.sessionKey) } - logger.crypto("import megolm ${it.sessionKey}") olmStore.persist(it.sessionId, inBound) } } diff --git a/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt b/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt index 675e814..c01d535 100644 --- a/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt +++ b/domains/store/src/main/kotlin/app/dapk/st/domain/OlmPersistence.kt @@ -11,9 +11,9 @@ import app.dapk.st.matrix.common.Curve25519 import app.dapk.st.matrix.common.RoomId import app.dapk.st.matrix.common.SessionId import com.squareup.sqldelight.TransactionWithoutReturn +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import kotlin.coroutines.resume -import kotlin.coroutines.suspendCoroutine class OlmPersistence( private val database: DapkDb, @@ -83,13 +83,9 @@ class OlmPersistence( } suspend fun startTransaction(action: suspend TransactionWithoutReturn.() -> Unit) { - val transaction = suspendCoroutine { - database.cryptoQueries.transaction(false) { - it.resume(this) - } - } - dispatchers.withIoContext { - action(transaction) + val scope = CoroutineScope(dispatchers.io) + database.cryptoQueries.transaction { + scope.launch { action() } } } From 5b2243cfa917a9aee9a1dac0bcef465ec31aca02 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 12:43:03 +0100 Subject: [PATCH 05/11] providing progress during room key imports by switching to a flow --- core/src/main/kotlin/app/dapk/st/core/Lce.kt | 6 ++++ .../app/dapk/st/settings/SettingsScreen.kt | 18 +++++++--- .../app/dapk/st/settings/SettingsState.kt | 3 +- .../app/dapk/st/settings/SettingsViewModel.kt | 34 +++++++++++++------ .../dapk/st/matrix/crypto/CryptoService.kt | 10 ++++-- .../crypto/internal/DefaultCryptoService.kt | 3 +- .../matrix/crypto/internal/RoomKeyImporter.kt | 23 +++++++++---- 7 files changed, 73 insertions(+), 24 deletions(-) diff --git a/core/src/main/kotlin/app/dapk/st/core/Lce.kt b/core/src/main/kotlin/app/dapk/st/core/Lce.kt index bce0ff4..ebb3e14 100644 --- a/core/src/main/kotlin/app/dapk/st/core/Lce.kt +++ b/core/src/main/kotlin/app/dapk/st/core/Lce.kt @@ -6,3 +6,9 @@ sealed interface Lce { data class Content(val value: T) : Lce } +sealed interface LceWithProgress { + data class Loading(val progress: T) : LceWithProgress + data class Error(val cause: Throwable) : LceWithProgress + data class Content(val value: T) : LceWithProgress +} + diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt index 7b1862d..c88c682 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt @@ -36,6 +36,7 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.core.net.toUri import app.dapk.st.core.Lce +import app.dapk.st.core.LceWithProgress import app.dapk.st.core.StartObserving import app.dapk.st.core.components.CenteredLoading import app.dapk.st.core.components.Header @@ -136,10 +137,11 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is Lce.Content -> { + is LceWithProgress.Content -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Import success") + Text(text = "Successfully imported ${it.importProgress.value} keys") + Spacer(modifier = Modifier.height(12.dp)) Button(onClick = { navigator.navigate.upToHome() }) { Text(text = "Close".uppercase()) } @@ -147,7 +149,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is Lce.Error -> { + is LceWithProgress.Error -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { Text(text = "Import failed") @@ -158,7 +160,15 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is Lce.Loading -> CenteredLoading() + is LceWithProgress.Loading -> { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Column(horizontalAlignment = Alignment.CenterHorizontally) { + Text(text = "Imported ${it.importProgress.progress} keys") + Spacer(modifier = Modifier.height(12.dp)) + CircularProgressIndicator(Modifier.wrapContentSize()) + } + } + } } } } diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt index fa78752..6dc49ca 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt @@ -2,6 +2,7 @@ package app.dapk.st.settings import android.net.Uri import app.dapk.st.core.Lce +import app.dapk.st.core.LceWithProgress import app.dapk.st.design.components.Route import app.dapk.st.design.components.SpiderPage import app.dapk.st.push.Registrar @@ -15,7 +16,7 @@ internal sealed interface Page { object Security : Page data class ImportRoomKey( val selectedFile: NamedUri? = null, - val importProgress: Lce? = null, + val importProgress: LceWithProgress? = null, ) : Page data class PushProviders( diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt index 6644542..0114bac 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt @@ -2,11 +2,14 @@ package app.dapk.st.settings import android.content.ContentResolver import android.net.Uri +import android.util.Log import androidx.lifecycle.viewModelScope import app.dapk.st.core.Lce +import app.dapk.st.core.LceWithProgress import app.dapk.st.design.components.SpiderPage import app.dapk.st.domain.StoreCleaner import app.dapk.st.matrix.crypto.CryptoService +import app.dapk.st.matrix.crypto.ImportResult import app.dapk.st.matrix.sync.SyncService import app.dapk.st.push.PushTokenRegistrars import app.dapk.st.push.Registrar @@ -15,6 +18,8 @@ import app.dapk.st.settings.SettingsEvent.* import app.dapk.st.viewmodel.DapkViewModel import app.dapk.st.viewmodel.MutableStateFactory import app.dapk.st.viewmodel.defaultStateFactory +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch private const val PRIVACY_POLICY_URL = "https://ouchadam.github.io/small-talk/privacy/" @@ -120,17 +125,26 @@ internal class SettingsViewModel( } fun importFromFileKeys(file: Uri, passphrase: String) { - updatePageState { copy(importProgress = Lce.Loading()) } + updatePageState { copy(importProgress = LceWithProgress.Loading(0L)) } viewModelScope.launch { - kotlin.runCatching { - with(cryptoService) { - val roomsToRefresh = contentResolver.openInputStream(file)?.importRoomKeys(passphrase) - roomsToRefresh?.let { syncService.forceManualRefresh(roomsToRefresh) } - } - }.fold( - onSuccess = { updatePageState { copy(importProgress = Lce.Content(Unit)) } }, - onFailure = { updatePageState { copy(importProgress = Lce.Error(it)) } } - ) + with(cryptoService) { + contentResolver.openInputStream(file)?.importRoomKeys(passphrase) + ?.onEach { + when (it) { + is ImportResult.Error -> { + updatePageState { copy(importProgress = LceWithProgress.Error(it.cause)) } + } + is ImportResult.Update -> { + updatePageState { copy(importProgress = LceWithProgress.Loading(it.importedKeysCount)) } + } + is ImportResult.Success -> { + syncService.forceManualRefresh(it.roomIds.toList()) + updatePageState { copy(importProgress = LceWithProgress.Content(it.totalImportedKeysCount)) } + } + } + } + ?.launchIn(viewModelScope) + } } } diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt index 47bc545..bd79f71 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt @@ -18,7 +18,7 @@ interface CryptoService : MatrixService { suspend fun encrypt(roomId: RoomId, credentials: DeviceCredentials, messageJson: JsonString): Crypto.EncryptionResult suspend fun decrypt(encryptedPayload: EncryptedMessageContent): DecryptionResult suspend fun importRoomKeys(keys: List) - suspend fun InputStream.importRoomKeys(password: String): List + suspend fun InputStream.importRoomKeys(password: String): Flow suspend fun maybeCreateMoreKeys(serverKeyCount: ServerKeyCount) suspend fun updateOlmSession(userIds: List, syncToken: SyncToken?) @@ -159,4 +159,10 @@ fun MatrixServiceProvider.cryptoService(): CryptoService = this.getService(key = fun interface RoomMembersProvider { suspend fun userIdsForRoom(roomId: RoomId): List -} \ No newline at end of file +} + +sealed interface ImportResult { + data class Success(val roomIds: Set, val totalImportedKeysCount: Long) : ImportResult + data class Error(val cause: Throwable) : ImportResult + data class Update(val importedKeysCount: Long) : ImportResult +} diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt index 4264d3f..6293bb9 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt @@ -4,6 +4,7 @@ import app.dapk.st.core.logP import app.dapk.st.matrix.common.* import app.dapk.st.matrix.crypto.Crypto import app.dapk.st.matrix.crypto.CryptoService +import app.dapk.st.matrix.crypto.ImportResult import app.dapk.st.matrix.crypto.Verification import kotlinx.coroutines.flow.Flow import java.io.InputStream @@ -48,7 +49,7 @@ internal class DefaultCryptoService( verificationHandler.onUserVerificationAction(verificationAction) } - override suspend fun InputStream.importRoomKeys(password: String): List { + override suspend fun InputStream.importRoomKeys(password: String): Flow { return logP("import room keys") { with(roomKeyImporter) { importRoomKeys(password) { diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt index 9845899..e14f486 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt @@ -7,6 +7,8 @@ import app.dapk.st.matrix.common.AlgorithmName import app.dapk.st.matrix.common.RoomId import app.dapk.st.matrix.common.SessionId import app.dapk.st.matrix.common.SharedRoomKey +import app.dapk.st.matrix.crypto.ImportResult +import kotlinx.coroutines.flow.* import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json @@ -28,8 +30,10 @@ class RoomKeyImporter( private val dispatchers: CoroutineDispatchers, ) { - suspend fun InputStream.importRoomKeys(password: String, onChunk: suspend (List) -> Unit): List { - return dispatchers.withIoContext { + suspend fun InputStream.importRoomKeys(password: String, onChunk: suspend (List) -> Unit): Flow { + return flow { + var importedKeysCount = 0L + val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") var jsonSegment = "" @@ -87,13 +91,20 @@ class RoomKeyImporter( ) } .chunked(500) - .forEach { onChunk(it) } + .forEach { + onChunk(it) + importedKeysCount += it.size + emit(ImportResult.Update(importedKeysCount)) + } } - roomIds.toList().ifEmpty { - throw IOException("Found no rooms to import in the file") + + if (roomIds.isEmpty()) { + emit(ImportResult.Error(IOException("Found no rooms to import in the file"))) + } else { + emit(ImportResult.Success(roomIds, importedKeysCount)) } } - } + }.flowOn(dispatchers.io) } private fun Cipher.initialize(payload: ByteArray, passphrase: String) { From 6a3c594481e1f27352ecb54af26b91194c96a70e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 13:03:54 +0100 Subject: [PATCH 06/11] extracting the json accumulation to its own class --- .../matrix/crypto/internal/RoomKeyImporter.kt | 165 +++++++++--------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt index e14f486..2b68e8a 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt @@ -33,71 +33,48 @@ class RoomKeyImporter( suspend fun InputStream.importRoomKeys(password: String, onChunk: suspend (List) -> Unit): Flow { return flow { var importedKeysCount = 0L - + val roomIds = mutableSetOf() val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") - var jsonSegment = "" - - fun Sequence.accumulateJson() = this.mapNotNull { - val withLatest = jsonSegment + it - try { - when (val objectRange = withLatest.findClosingIndex()) { - null -> { - jsonSegment = withLatest - null - } - else -> { - val string = withLatest.substring(objectRange) - importJson.decodeFromString(ElementMegolmExportObject.serializer(), string).also { - jsonSegment = withLatest.replace(string, "").removePrefix(",") - } - } - } - } catch (error: Throwable) { - jsonSegment = withLatest - null - } - } - this@importRoomKeys.bufferedReader().use { - val roomIds = mutableSetOf() - it.useLines { sequence -> - sequence - .filterNot { it == HEADER_LINE || it == TRAILER_LINE || it.isEmpty() } - .chunked(2) - .withIndex() - .map { (index, it) -> - val line = it.joinToString(separator = "").replace("\n", "") - val toByteArray = base64.decode(line) - if (index == 0) { - decryptCipher.initialize(toByteArray, password) - toByteArray.copyOfRange(37, toByteArray.size).decrypt(decryptCipher).also { - if (!it.startsWith("[{")) { - throw IllegalArgumentException("Unable to decrypt, assumed invalid password") + with(JsonAccumulator()) { + it.useLines { sequence -> + sequence + .filterNot { it == HEADER_LINE || it == TRAILER_LINE || it.isEmpty() } + .chunked(2) + .withIndex() + .map { (index, it) -> + val line = it.joinToString(separator = "").replace("\n", "") + val toByteArray = base64.decode(line) + if (index == 0) { + decryptCipher.initialize(toByteArray, password) + toByteArray.copyOfRange(37, toByteArray.size).decrypt(decryptCipher).also { + if (!it.startsWith("[{")) { + throw IllegalArgumentException("Unable to decrypt, assumed invalid password") + } } + } else { + toByteArray.decrypt(decryptCipher) } - } else { - toByteArray.decrypt(decryptCipher) } - } - .accumulateJson() - .map { decoded -> - roomIds.add(decoded.roomId) - SharedRoomKey( - decoded.algorithmName, - decoded.roomId, - decoded.sessionId, - decoded.sessionKey, - isExported = true, - ) - } - .chunked(500) - .forEach { - onChunk(it) - importedKeysCount += it.size - emit(ImportResult.Update(importedKeysCount)) - } + .accumulateJson() + .map { decoded -> + roomIds.add(decoded.roomId) + SharedRoomKey( + decoded.algorithmName, + decoded.roomId, + decoded.sessionId, + decoded.sessionKey, + isExported = true, + ) + } + .chunked(500) + .forEach { + onChunk(it) + importedKeysCount += it.size + emit(ImportResult.Update(importedKeysCount)) + } + } } - if (roomIds.isEmpty()) { emit(ImportResult.Error(IOException("Found no rooms to import in the file"))) } else { @@ -160,28 +137,6 @@ class RoomKeyImporter( private fun Byte.toUnsignedInt() = toInt() and 0xff -private fun String.findClosingIndex(): IntRange? { - var opens = 0 - var openIndex = -1 - this.forEachIndexed { index, c -> - when { - c == '{' -> { - if (opens == 0) { - openIndex = index - } - opens++ - } - c == '}' -> { - opens-- - if (opens == 0) { - return IntRange(openIndex, index) - } - } - } - } - return null -} - @Serializable private data class ElementMegolmExportObject( @SerialName("room_id") val roomId: RoomId, @@ -189,3 +144,51 @@ private data class ElementMegolmExportObject( @SerialName("session_id") val sessionId: SessionId, @SerialName("algorithm") val algorithmName: AlgorithmName, ) + +private class JsonAccumulator { + + private var jsonSegment = "" + + fun Sequence.accumulateJson() = this.mapNotNull { + val withLatest = jsonSegment + it + try { + when (val objectRange = withLatest.findClosingIndex()) { + null -> { + jsonSegment = withLatest + null + } + else -> { + val string = withLatest.substring(objectRange) + importJson.decodeFromString(ElementMegolmExportObject.serializer(), string).also { + jsonSegment = withLatest.replace(string, "").removePrefix(",") + } + } + } + } catch (error: Throwable) { + jsonSegment = withLatest + null + } + } + + private fun String.findClosingIndex(): IntRange? { + var opens = 0 + var openIndex = -1 + this.forEachIndexed { index, c -> + when { + c == '{' -> { + if (opens == 0) { + openIndex = index + } + opens++ + } + c == '}' -> { + opens-- + if (opens == 0) { + return IntRange(openIndex, index) + } + } + } + } + return null + } +} \ No newline at end of file From 71af573d064b037168cfa3e92e888bf6239b438b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 13:36:48 +0100 Subject: [PATCH 07/11] using the import result directly in the UI, will allow separate error displays per reason --- .../app/dapk/st/settings/SettingsScreen.kt | 11 +- .../app/dapk/st/settings/SettingsState.kt | 3 +- .../app/dapk/st/settings/SettingsViewModel.kt | 9 +- .../dapk/st/matrix/crypto/CryptoService.kt | 10 +- .../matrix/crypto/internal/RoomKeyImporter.kt | 117 ++++++++++-------- 5 files changed, 89 insertions(+), 61 deletions(-) diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt index c88c682..5cb6e50 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt @@ -44,6 +44,7 @@ import app.dapk.st.design.components.SettingsTextRow import app.dapk.st.design.components.Spider import app.dapk.st.design.components.SpiderPage import app.dapk.st.design.components.TextRow +import app.dapk.st.matrix.crypto.ImportResult import app.dapk.st.navigator.Navigator import app.dapk.st.settings.SettingsEvent.* import app.dapk.st.settings.eventlogger.EventLogActivity @@ -137,10 +138,10 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is LceWithProgress.Content -> { + is ImportResult.Success -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Successfully imported ${it.importProgress.value} keys") + Text(text = "Successfully imported ${it.importProgress.totalImportedKeysCount} keys") Spacer(modifier = Modifier.height(12.dp)) Button(onClick = { navigator.navigate.upToHome() }) { Text(text = "Close".uppercase()) @@ -149,7 +150,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is LceWithProgress.Error -> { + is ImportResult.Error -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { Text(text = "Import failed") @@ -160,10 +161,10 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, } } - is LceWithProgress.Loading -> { + is ImportResult.Update -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Imported ${it.importProgress.progress} keys") + Text(text = "Imported ${it.importProgress.importedKeysCount} keys") Spacer(modifier = Modifier.height(12.dp)) CircularProgressIndicator(Modifier.wrapContentSize()) } diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt index 6dc49ca..ca3b67a 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsState.kt @@ -5,6 +5,7 @@ import app.dapk.st.core.Lce import app.dapk.st.core.LceWithProgress import app.dapk.st.design.components.Route import app.dapk.st.design.components.SpiderPage +import app.dapk.st.matrix.crypto.ImportResult import app.dapk.st.push.Registrar internal data class SettingsScreenState( @@ -16,7 +17,7 @@ internal sealed interface Page { object Security : Page data class ImportRoomKey( val selectedFile: NamedUri? = null, - val importProgress: LceWithProgress? = null, + val importProgress: ImportResult? = null, ) : Page data class PushProviders( diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt index 0114bac..4b68513 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt @@ -2,7 +2,6 @@ package app.dapk.st.settings import android.content.ContentResolver import android.net.Uri -import android.util.Log import androidx.lifecycle.viewModelScope import app.dapk.st.core.Lce import app.dapk.st.core.LceWithProgress @@ -125,21 +124,21 @@ internal class SettingsViewModel( } fun importFromFileKeys(file: Uri, passphrase: String) { - updatePageState { copy(importProgress = LceWithProgress.Loading(0L)) } + updatePageState { copy(importProgress = ImportResult.Update(0)) } viewModelScope.launch { with(cryptoService) { contentResolver.openInputStream(file)?.importRoomKeys(passphrase) ?.onEach { + updatePageState { copy(importProgress = it) } when (it) { is ImportResult.Error -> { - updatePageState { copy(importProgress = LceWithProgress.Error(it.cause)) } + // do nothing } is ImportResult.Update -> { - updatePageState { copy(importProgress = LceWithProgress.Loading(it.importedKeysCount)) } + // do nothing } is ImportResult.Success -> { syncService.forceManualRefresh(it.roomIds.toList()) - updatePageState { copy(importProgress = LceWithProgress.Content(it.totalImportedKeysCount)) } } } } diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt index bd79f71..39f3c98 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt @@ -163,6 +163,14 @@ fun interface RoomMembersProvider { sealed interface ImportResult { data class Success(val roomIds: Set, val totalImportedKeysCount: Long) : ImportResult - data class Error(val cause: Throwable) : ImportResult + data class Error(val cause: Type) : ImportResult { + + sealed interface Type { + data class Unknown(val cause: Throwable): Type + object NoKeysFound: Type + object UnexpectedDecryptionOutput: Type + } + + } data class Update(val importedKeysCount: Long) : ImportResult } diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt index 2b68e8a..24311a9 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/RoomKeyImporter.kt @@ -2,17 +2,18 @@ package app.dapk.st.matrix.crypto.internal import app.dapk.st.core.Base64 import app.dapk.st.core.CoroutineDispatchers -import app.dapk.st.core.withIoContext import app.dapk.st.matrix.common.AlgorithmName import app.dapk.st.matrix.common.RoomId import app.dapk.st.matrix.common.SessionId import app.dapk.st.matrix.common.SharedRoomKey import app.dapk.st.matrix.crypto.ImportResult -import kotlinx.coroutines.flow.* +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.FlowCollector +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json -import java.io.IOException import java.io.InputStream import java.nio.charset.Charset import javax.crypto.Cipher @@ -32,56 +33,72 @@ class RoomKeyImporter( suspend fun InputStream.importRoomKeys(password: String, onChunk: suspend (List) -> Unit): Flow { return flow { - var importedKeysCount = 0L - val roomIds = mutableSetOf() - val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") - this@importRoomKeys.bufferedReader().use { - with(JsonAccumulator()) { - it.useLines { sequence -> - sequence - .filterNot { it == HEADER_LINE || it == TRAILER_LINE || it.isEmpty() } - .chunked(2) - .withIndex() - .map { (index, it) -> - val line = it.joinToString(separator = "").replace("\n", "") - val toByteArray = base64.decode(line) - if (index == 0) { - decryptCipher.initialize(toByteArray, password) - toByteArray.copyOfRange(37, toByteArray.size).decrypt(decryptCipher).also { - if (!it.startsWith("[{")) { - throw IllegalArgumentException("Unable to decrypt, assumed invalid password") - } - } - } else { - toByteArray.decrypt(decryptCipher) - } - } - .accumulateJson() - .map { decoded -> - roomIds.add(decoded.roomId) - SharedRoomKey( - decoded.algorithmName, - decoded.roomId, - decoded.sessionId, - decoded.sessionKey, - isExported = true, - ) - } - .chunked(500) - .forEach { - onChunk(it) - importedKeysCount += it.size - emit(ImportResult.Update(importedKeysCount)) - } + runCatching { this@importRoomKeys.import(password, onChunk, this) } + .onFailure { + when (it) { + is ImportException -> emit(ImportResult.Error(it.type)) + else -> emit(ImportResult.Error(ImportResult.Error.Type.Unknown(it))) } } - if (roomIds.isEmpty()) { - emit(ImportResult.Error(IOException("Found no rooms to import in the file"))) - } else { - emit(ImportResult.Success(roomIds, importedKeysCount)) + }.flowOn(dispatchers.io) + } + + private suspend fun InputStream.import(password: String, onChunk: suspend (List) -> Unit, collector: FlowCollector) { + var importedKeysCount = 0L + val roomIds = mutableSetOf() + + this.bufferedReader().use { + with(JsonAccumulator()) { + it.useLines { sequence -> + sequence + .filterNot { it == HEADER_LINE || it == TRAILER_LINE || it.isEmpty() } + .chunked(5) + .decrypt(password) + .accumulateJson() + .map { decoded -> + roomIds.add(decoded.roomId) + SharedRoomKey( + decoded.algorithmName, + decoded.roomId, + decoded.sessionId, + decoded.sessionKey, + isExported = true, + ) + } + .chunked(500) + .forEach { + onChunk(it) + importedKeysCount += it.size + collector.emit(ImportResult.Update(importedKeysCount)) + } } } - }.flowOn(dispatchers.io) + when { + roomIds.isEmpty() -> collector.emit(ImportResult.Error(ImportResult.Error.Type.NoKeysFound)) + else -> collector.emit(ImportResult.Success(roomIds, importedKeysCount)) + } + } + } + + private fun Sequence>.decrypt(password: String): Sequence { + val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") + return this.withIndex().map { (index, it) -> + val line = it.joinToString(separator = "").replace("\n", "") + val toByteArray = base64.decode(line) + if (index == 0) { + decryptCipher.initialize(toByteArray, password) + toByteArray + .copyOfRange(37, toByteArray.size) + .decrypt(decryptCipher) + .also { + if (!it.startsWith("[{")) { + throw ImportException(ImportResult.Error.Type.UnexpectedDecryptionOutput) + } + } + } else { + toByteArray.decrypt(decryptCipher) + } + } } private fun Cipher.initialize(payload: ByteArray, passphrase: String) { @@ -145,6 +162,8 @@ private data class ElementMegolmExportObject( @SerialName("algorithm") val algorithmName: AlgorithmName, ) +private class ImportException(val type: ImportResult.Error.Type) : Throwable() + private class JsonAccumulator { private var jsonSegment = "" From 25c8ac07168c07456544ace96da971403767902d Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 13:44:45 +0100 Subject: [PATCH 08/11] improving key import messaging --- .../app/dapk/st/settings/SettingsScreen.kt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt index 5cb6e50..3f6c281 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt @@ -31,6 +31,7 @@ import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.PasswordVisualTransformation import androidx.compose.ui.text.input.VisualTransformation +import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp @@ -74,7 +75,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, PushProviders(viewModel, it) } item(Page.Routes.importRoomKeys) { - when (it.importProgress) { + when (val result = it.importProgress) { null -> { Box( modifier = Modifier.fillMaxSize(), @@ -141,7 +142,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, is ImportResult.Success -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Successfully imported ${it.importProgress.totalImportedKeysCount} keys") + Text(text = "Successfully imported ${result.totalImportedKeysCount} keys") Spacer(modifier = Modifier.height(12.dp)) Button(onClick = { navigator.navigate.upToHome() }) { Text(text = "Close".uppercase()) @@ -153,7 +154,14 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, is ImportResult.Error -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Import failed") + val message = when(val type = result.cause) { + ImportResult.Error.Type.NoKeysFound -> "No keys found in the file" + ImportResult.Error.Type.UnexpectedDecryptionOutput -> "Unable to decrypt file, double check your passphrase" + is ImportResult.Error.Type.Unknown -> "${type.cause::class.java.simpleName}: ${type.cause.message}" + } + + Text(text = "Import failed\n$message", textAlign = TextAlign.Center) + Spacer(modifier = Modifier.height(12.dp)) Button(onClick = { navigator.navigate.upToHome() }) { Text(text = "Close".uppercase()) } @@ -164,7 +172,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, is ImportResult.Update -> { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text(text = "Imported ${it.importProgress.importedKeysCount} keys") + Text(text = "Importing ${result.importedKeysCount} keys...") Spacer(modifier = Modifier.height(12.dp)) CircularProgressIndicator(Modifier.wrapContentSize()) } From 38dec9c80255974c22254c3ac799e7f39421d8b9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 13:50:04 +0100 Subject: [PATCH 09/11] updating smoke test to use flow result --- test-harness/src/test/kotlin/SmokeTest.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test-harness/src/test/kotlin/SmokeTest.kt b/test-harness/src/test/kotlin/SmokeTest.kt index d153626..8642059 100644 --- a/test-harness/src/test/kotlin/SmokeTest.kt +++ b/test-harness/src/test/kotlin/SmokeTest.kt @@ -4,11 +4,13 @@ import app.dapk.st.matrix.common.HomeServerUrl import app.dapk.st.matrix.common.RoomId import app.dapk.st.matrix.common.RoomMember import app.dapk.st.matrix.common.UserId +import app.dapk.st.matrix.crypto.ImportResult import app.dapk.st.matrix.crypto.Verification import app.dapk.st.matrix.crypto.cryptoService import app.dapk.st.matrix.room.roomService import app.dapk.st.matrix.sync.syncService import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo @@ -97,10 +99,13 @@ class SmokeTest { val stream = loadResourceStream("element-keys.txt") val result = with(cryptoService) { - stream.importRoomKeys(password = "aaaaaa") + stream.importRoomKeys(password = "aaaaaa").first { it is ImportResult.Success } } - result shouldBeEqualTo listOf(RoomId(value = "!qOSENTtFUuCEKJSVzl:matrix.org")) + result shouldBeEqualTo ImportResult.Success( + roomIds = setOf(RoomId(value = "!qOSENTtFUuCEKJSVzl:matrix.org")), + totalImportedKeysCount = 28, + ) } private fun testTextMessaging(isEncrypted: Boolean) = testAfterInitialSync { alice, bob -> From ef2a2c5624957d8a686d31eea3f14e0de8910f8c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 14:01:07 +0100 Subject: [PATCH 10/11] logging the room import time taken --- .../main/kotlin/app/dapk/st/core/HeliumLogger.kt | 14 ++++++++++++++ .../matrix/crypto/internal/DefaultCryptoService.kt | 10 ++++------ .../dapk/st/matrix/crypto/internal/OlmCrypto.kt | 1 - 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/core/src/main/kotlin/app/dapk/st/core/HeliumLogger.kt b/core/src/main/kotlin/app/dapk/st/core/HeliumLogger.kt index 9327337..d1846b9 100644 --- a/core/src/main/kotlin/app/dapk/st/core/HeliumLogger.kt +++ b/core/src/main/kotlin/app/dapk/st/core/HeliumLogger.kt @@ -1,5 +1,9 @@ package app.dapk.st.core +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.flow.onStart + enum class AppLogTag(val key: String) { NOTIFICATION("notification"), PERFORMANCE("performance"), @@ -26,3 +30,13 @@ suspend fun logP(area: String, block: suspend () -> T): T { log(AppLogTag.PERFORMANCE, "$area: took $timeTaken ms") } } + +fun Flow.logP(area: String): Flow { + var start = -1L + return this + .onStart { start = System.currentTimeMillis() } + .onCompletion { + val timeTaken = System.currentTimeMillis() - start + log(AppLogTag.PERFORMANCE, "$area: took $timeTaken ms") + } +} \ No newline at end of file diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt index 6293bb9..f5a64fe 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/DefaultCryptoService.kt @@ -50,12 +50,10 @@ internal class DefaultCryptoService( } override suspend fun InputStream.importRoomKeys(password: String): Flow { - return logP("import room keys") { - with(roomKeyImporter) { - importRoomKeys(password) { - importRoomKeys(it) - } - } + return with(roomKeyImporter) { + importRoomKeys(password) { + importRoomKeys(it) + }.logP("import room keys") } } } diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/OlmCrypto.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/OlmCrypto.kt index 39e95e5..d244cf6 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/OlmCrypto.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/internal/OlmCrypto.kt @@ -14,7 +14,6 @@ internal class OlmCrypto( ) { suspend fun importRoomKeys(keys: List) { - logger.crypto("import room keys : ${keys.size}") olm.import(keys) } From f973e01a4ec870c47607152ce9672db4df248a1a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sun, 4 Sep 2022 14:13:36 +0100 Subject: [PATCH 11/11] fixing broken settings view model tests and handling file opening errors --- .../app/dapk/st/settings/SettingsScreen.kt | 1 + .../app/dapk/st/settings/SettingsViewModel.kt | 39 +++++++++++-------- .../dapk/st/settings/SettingsViewModelTest.kt | 14 ++++--- .../dapk/st/matrix/crypto/CryptoService.kt | 1 + 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt index 3f6c281..9a25014 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsScreen.kt @@ -158,6 +158,7 @@ internal fun SettingsScreen(viewModel: SettingsViewModel, onSignOut: () -> Unit, ImportResult.Error.Type.NoKeysFound -> "No keys found in the file" ImportResult.Error.Type.UnexpectedDecryptionOutput -> "Unable to decrypt file, double check your passphrase" is ImportResult.Error.Type.Unknown -> "${type.cause::class.java.simpleName}: ${type.cause.message}" + ImportResult.Error.Type.UnableToOpenFile -> "Unable to open file" } Text(text = "Import failed\n$message", textAlign = TextAlign.Center) diff --git a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt index 4b68513..56dd839 100644 --- a/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt +++ b/features/settings/src/main/kotlin/app/dapk/st/settings/SettingsViewModel.kt @@ -4,7 +4,6 @@ import android.content.ContentResolver import android.net.Uri import androidx.lifecycle.viewModelScope import app.dapk.st.core.Lce -import app.dapk.st.core.LceWithProgress import app.dapk.st.design.components.SpiderPage import app.dapk.st.domain.StoreCleaner import app.dapk.st.matrix.crypto.CryptoService @@ -127,22 +126,30 @@ internal class SettingsViewModel( updatePageState { copy(importProgress = ImportResult.Update(0)) } viewModelScope.launch { with(cryptoService) { - contentResolver.openInputStream(file)?.importRoomKeys(passphrase) - ?.onEach { - updatePageState { copy(importProgress = it) } - when (it) { - is ImportResult.Error -> { - // do nothing - } - is ImportResult.Update -> { - // do nothing - } - is ImportResult.Success -> { - syncService.forceManualRefresh(it.roomIds.toList()) - } + runCatching { contentResolver.openInputStream(file)!! } + .fold( + onSuccess = { fileStream -> + fileStream.importRoomKeys(passphrase) + .onEach { + updatePageState { copy(importProgress = it) } + when (it) { + is ImportResult.Error -> { + // do nothing + } + is ImportResult.Update -> { + // do nothing + } + is ImportResult.Success -> { + syncService.forceManualRefresh(it.roomIds.toList()) + } + } + } + .launchIn(viewModelScope) + }, + onFailure = { + updatePageState { copy(importProgress = ImportResult.Error(ImportResult.Error.Type.UnableToOpenFile)) } } - } - ?.launchIn(viewModelScope) + ) } } } diff --git a/features/settings/src/test/kotlin/app/dapk/st/settings/SettingsViewModelTest.kt b/features/settings/src/test/kotlin/app/dapk/st/settings/SettingsViewModelTest.kt index 5fd5279..0a2067c 100644 --- a/features/settings/src/test/kotlin/app/dapk/st/settings/SettingsViewModelTest.kt +++ b/features/settings/src/test/kotlin/app/dapk/st/settings/SettingsViewModelTest.kt @@ -3,6 +3,7 @@ package app.dapk.st.settings import ViewModelTest import app.dapk.st.core.Lce import app.dapk.st.design.components.SpiderPage +import app.dapk.st.matrix.crypto.ImportResult import fake.* import fixture.FakeStoreCleaner import fixture.aRoomId @@ -10,6 +11,7 @@ import internalfake.FakeSettingsItemFactory import internalfake.FakeUriFilenameResolver import internalfixture.aImportRoomKeysPage import internalfixture.aSettingTextItem +import kotlinx.coroutines.flow.flowOf import org.junit.Test private const val APP_PRIVACY_POLICY_URL = "https://ouchadam.github.io/small-talk/privacy/" @@ -21,6 +23,8 @@ private val A_IMPORT_ROOM_KEYS_PAGE_WITH_SELECTION = aImportRoomKeysPage( state = Page.ImportRoomKey(selectedFile = NamedUri(A_FILENAME, A_URI.instance)) ) private val A_LIST_OF_ROOM_IDS = listOf(aRoomId()) +private val AN_IMPORT_SUCCESS = ImportResult.Success(A_LIST_OF_ROOM_IDS.toSet(), totalImportedKeysCount = 5) +private val AN_IMPORT_FILE_ERROR = ImportResult.Error(ImportResult.Error.Type.UnableToOpenFile) private val AN_INPUT_STREAM = FakeInputStream() private const val A_PASSPHRASE = "passphrase" private val AN_ERROR = RuntimeException() @@ -166,15 +170,15 @@ internal class SettingsViewModelTest { fun `given success when importing room keys, then emits progress`() = runViewModelTest { fakeSyncService.expectUnit { it.forceManualRefresh(A_LIST_OF_ROOM_IDS) } fakeContentResolver.givenFile(A_URI.instance).returns(AN_INPUT_STREAM.instance) - fakeCryptoService.givenImportKeys(AN_INPUT_STREAM.instance, A_PASSPHRASE).returns(A_LIST_OF_ROOM_IDS) + fakeCryptoService.givenImportKeys(AN_INPUT_STREAM.instance, A_PASSPHRASE).returns(flowOf(AN_IMPORT_SUCCESS)) viewModel .test(initialState = SettingsScreenState(A_IMPORT_ROOM_KEYS_PAGE_WITH_SELECTION)) .importFromFileKeys(A_URI.instance, A_PASSPHRASE) assertStates( - { copy(page = page.updateState { copy(importProgress = Lce.Loading()) }) }, - { copy(page = page.updateState { copy(importProgress = Lce.Content(Unit)) }) }, + { copy(page = page.updateState { copy(importProgress = ImportResult.Update(0L)) }) }, + { copy(page = page.updateState { copy(importProgress = AN_IMPORT_SUCCESS) }) }, ) assertNoEvents() verifyExpects() @@ -189,8 +193,8 @@ internal class SettingsViewModelTest { .importFromFileKeys(A_URI.instance, A_PASSPHRASE) assertStates( - { copy(page = page.updateState { copy(importProgress = Lce.Loading()) }) }, - { copy(page = page.updateState { copy(importProgress = Lce.Error(AN_ERROR)) }) }, + { copy(page = page.updateState { copy(importProgress = ImportResult.Update(0L)) }) }, + { copy(page = page.updateState { copy(importProgress = AN_IMPORT_FILE_ERROR) }) }, ) assertNoEvents() } diff --git a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt index 39f3c98..adb85c9 100644 --- a/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt +++ b/matrix/services/crypto/src/main/kotlin/app/dapk/st/matrix/crypto/CryptoService.kt @@ -169,6 +169,7 @@ sealed interface ImportResult { data class Unknown(val cause: Throwable): Type object NoKeysFound: Type object UnexpectedDecryptionOutput: Type + object UnableToOpenFile: Type } }