From 1ac99e92a6ce025dac9106a973430a5b90153ea9 Mon Sep 17 00:00:00 2001 From: Dominic Fischer Date: Thu, 24 Oct 2019 14:58:11 +0100 Subject: [PATCH] Light refactoring. Signed-off-by: Dominic Fischer --- .../DefaultSasVerificationService.kt | 114 ++++++++---------- .../room/send/LocalEchoEventFactory.kt | 2 +- .../im/vector/riotx/core/files/FileSaver.kt | 26 ++-- .../im/vector/riotx/core/images/ImageTools.kt | 25 ++-- .../im/vector/riotx/core/intent/Filename.kt | 21 +--- .../features/crypto/keys/KeysExporter.kt | 32 ++--- .../features/crypto/keys/KeysImporter.kt | 37 ++---- .../setup/KeysBackupSetupStep3Fragment.kt | 4 +- .../action/MessageActionsViewModel.kt | 3 +- 9 files changed, 103 insertions(+), 161 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt index 3115eafc45..e0cd47e0e0 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt @@ -43,6 +43,7 @@ import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import timber.log.Timber +import java.lang.Exception import java.util.UUID import javax.inject.Inject import kotlin.collections.HashMap @@ -166,72 +167,59 @@ internal class DefaultSasVerificationService @Inject constructor(private val cre return } // Download device keys prior to everything - checkKeysAreDownloaded( - otherUserId!!, - startReq, - success = { - Timber.v("## SAS onStartRequestReceived ${startReq.transactionID!!}") - val tid = startReq.transactionID!! - val existing = getExistingTransaction(otherUserId, tid) - val existingTxs = getExistingTransactionsForUser(otherUserId) - if (existing != null) { - // should cancel both! - Timber.v("## SAS onStartRequestReceived - Request exist with same if ${startReq.transactionID!!}") - existing.cancel(CancelCode.UnexpectedMessage) - cancelTransaction(tid, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) - } else if (existingTxs?.isEmpty() == false) { - Timber.v("## SAS onStartRequestReceived - There is already a transaction with this user ${startReq.transactionID!!}") - // Multiple keyshares between two devices: any two devices may only have at most one key verification in flight at a time. - existingTxs.forEach { - it.cancel(CancelCode.UnexpectedMessage) - } - cancelTransaction(tid, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) - } else { - // Ok we can create - if (KeyVerificationStart.VERIF_METHOD_SAS == startReq.method) { - Timber.v("## SAS onStartRequestReceived - request accepted ${startReq.transactionID!!}") - val tx = IncomingSASVerificationTransaction( - this, - setDeviceVerificationAction, - credentials, - cryptoStore, - sendToDeviceTask, - taskExecutor, - myDeviceInfoHolder.get().myDevice.fingerprint()!!, - startReq.transactionID!!, - otherUserId) - addTransaction(tx) - tx.acceptToDeviceEvent(otherUserId, startReq) - } else { - Timber.e("## SAS onStartRequestReceived - unknown method ${startReq.method}") - cancelTransaction(tid, otherUserId, startReq.fromDevice - ?: event.getSenderKey()!!, CancelCode.UnknownMethod) - } - } - }, - error = { - cancelTransaction(startReq.transactionID!!, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) - }) + if (checkKeysAreDownloaded(otherUserId!!, startReq) != null) { + Timber.v("## SAS onStartRequestReceived ${startReq.transactionID!!}") + val tid = startReq.transactionID!! + val existing = getExistingTransaction(otherUserId, tid) + val existingTxs = getExistingTransactionsForUser(otherUserId) + if (existing != null) { + // should cancel both! + Timber.v("## SAS onStartRequestReceived - Request exist with same if ${startReq.transactionID!!}") + existing.cancel(CancelCode.UnexpectedMessage) + cancelTransaction(tid, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) + } else if (existingTxs?.isEmpty() == false) { + Timber.v("## SAS onStartRequestReceived - There is already a transaction with this user ${startReq.transactionID!!}") + // Multiple keyshares between two devices: any two devices may only have at most one key verification in flight at a time. + existingTxs.forEach { + it.cancel(CancelCode.UnexpectedMessage) + } + cancelTransaction(tid, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) + } else { + // Ok we can create + if (KeyVerificationStart.VERIF_METHOD_SAS == startReq.method) { + Timber.v("## SAS onStartRequestReceived - request accepted ${startReq.transactionID!!}") + val tx = IncomingSASVerificationTransaction( + this, + setDeviceVerificationAction, + credentials, + cryptoStore, + sendToDeviceTask, + taskExecutor, + myDeviceInfoHolder.get().myDevice.fingerprint()!!, + startReq.transactionID!!, + otherUserId) + addTransaction(tx) + tx.acceptToDeviceEvent(otherUserId, startReq) + } else { + Timber.e("## SAS onStartRequestReceived - unknown method ${startReq.method}") + cancelTransaction(tid, otherUserId, startReq.fromDevice + ?: event.getSenderKey()!!, CancelCode.UnknownMethod) + } + } + } else { + cancelTransaction(startReq.transactionID!!, otherUserId, startReq.fromDevice!!, CancelCode.UnexpectedMessage) + } } private suspend fun checkKeysAreDownloaded(otherUserId: String, - startReq: KeyVerificationStart, - success: (MXUsersDevicesMap) -> Unit, - error: () -> Unit) { - runCatching { - deviceListManager.downloadKeys(listOf(otherUserId), true) - }.fold( - { - if (it.getUserDeviceIds(otherUserId)?.contains(startReq.fromDevice) == true) { - success(it) - } else { - error() - } - }, - { - error() - } - ) + startReq: KeyVerificationStart): MXUsersDevicesMap? { + return try { + val keys = deviceListManager.downloadKeys(listOf(otherUserId), true) + val deviceIds = keys.getUserDeviceIds(otherUserId) ?: return null + keys.takeIf { deviceIds.contains(startReq.fromDevice) } + } catch (e: Exception) { + null + } } private suspend fun onCancelReceived(event: Event) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt index 49c813ece6..a9406c8bff 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt @@ -39,7 +39,7 @@ import im.vector.matrix.android.internal.session.room.RoomSummaryUpdater import im.vector.matrix.android.internal.util.StringProvider import org.commonmark.parser.Parser import org.commonmark.renderer.html.HtmlRenderer -import java.util.* +import java.util.UUID import javax.inject.Inject /** diff --git a/vector/src/main/java/im/vector/riotx/core/files/FileSaver.kt b/vector/src/main/java/im/vector/riotx/core/files/FileSaver.kt index c1f58306a4..677f7894e8 100644 --- a/vector/src/main/java/im/vector/riotx/core/files/FileSaver.kt +++ b/vector/src/main/java/im/vector/riotx/core/files/FileSaver.kt @@ -30,15 +30,10 @@ import java.io.File */ @WorkerThread fun writeToFile(str: String, file: File): Try { - return Try { - val sink = file.sink() - - val bufferedSink = sink.buffer() - - bufferedSink.writeString(str, Charsets.UTF_8) - - bufferedSink.close() - sink.close() + return Try { + file.sink().buffer().use { + it.writeString(str, Charsets.UTF_8) + } } } @@ -47,15 +42,10 @@ fun writeToFile(str: String, file: File): Try { */ @WorkerThread fun writeToFile(data: ByteArray, file: File): Try { - return Try { - val sink = file.sink() - - val bufferedSink = sink.buffer() - - bufferedSink.write(data) - - bufferedSink.close() - sink.close() + return Try { + file.sink().buffer().use { + it.write(data) + } } } diff --git a/vector/src/main/java/im/vector/riotx/core/images/ImageTools.kt b/vector/src/main/java/im/vector/riotx/core/images/ImageTools.kt index b6ae2be20b..84cba7392f 100644 --- a/vector/src/main/java/im/vector/riotx/core/images/ImageTools.kt +++ b/vector/src/main/java/im/vector/riotx/core/images/ImageTools.kt @@ -17,7 +17,6 @@ package im.vector.riotx.core.images import android.content.Context -import android.database.Cursor import android.net.Uri import android.provider.MediaStore import androidx.exifinterface.media.ExifInterface @@ -37,26 +36,24 @@ class ImageTools @Inject constructor(private val context: Context) { if (uri.scheme == "content") { val proj = arrayOf(MediaStore.Images.Media.DATA) - var cursor: Cursor? = null try { - cursor = context.contentResolver.query(uri, proj, null, null, null) - if (cursor != null && cursor.count > 0) { - cursor.moveToFirst() - val idxData = cursor.getColumnIndexOrThrow(MediaStore.Images.Media.DATA) - val path = cursor.getString(idxData) - if (path.isNullOrBlank()) { - Timber.w("Cannot find path in media db for uri $uri") - return orientation + val cursor = context.contentResolver.query(uri, proj, null, null, null) + cursor?.use { + if (it.moveToFirst()) { + val idxData = it.getColumnIndexOrThrow(MediaStore.Images.Media.DATA) + val path = it.getString(idxData) + if (path.isNullOrBlank()) { + Timber.w("Cannot find path in media db for uri $uri") + return orientation + } + val exif = ExifInterface(path) + orientation = exif.getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_UNDEFINED) } - val exif = ExifInterface(path) - orientation = exif.getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_UNDEFINED) } } catch (e: Exception) { // eg SecurityException from com.google.android.apps.photos.content.GooglePhotosImageProvider URIs // eg IOException from trying to parse the returned path as a file when it is an http uri. Timber.e(e, "Cannot get orientation for bitmap") - } finally { - cursor?.close() } } else if (uri.scheme == "file") { try { diff --git a/vector/src/main/java/im/vector/riotx/core/intent/Filename.kt b/vector/src/main/java/im/vector/riotx/core/intent/Filename.kt index 9e9f0ae508..2b6740f62f 100644 --- a/vector/src/main/java/im/vector/riotx/core/intent/Filename.kt +++ b/vector/src/main/java/im/vector/riotx/core/intent/Filename.kt @@ -17,28 +17,17 @@ package im.vector.riotx.core.intent import android.content.Context -import android.database.Cursor import android.net.Uri import android.provider.OpenableColumns fun getFilenameFromUri(context: Context?, uri: Uri): String? { - var result: String? = null if (context != null && uri.scheme == "content") { - val cursor: Cursor? = context.contentResolver.query(uri, null, null, null, null) - try { - if (cursor != null && cursor.moveToFirst()) { - result = cursor.getString(cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)) + val cursor = context.contentResolver.query(uri, null, null, null, null) + cursor?.use { + if (it.moveToFirst()) { + return it.getString(it.getColumnIndex(OpenableColumns.DISPLAY_NAME)) } - } finally { - cursor?.close() } } - if (result == null) { - result = uri.path - val cut = result?.lastIndexOf('/') ?: -1 - if (cut != -1) { - result = result?.substring(cut + 1) - } - } - return result + return uri.path?.substringAfterLast('/') } diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysExporter.kt b/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysExporter.kt index 54e3a34744..9642c2d8c6 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysExporter.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysExporter.kt @@ -18,10 +18,10 @@ package im.vector.riotx.features.crypto.keys import android.content.Context import android.os.Environment -import arrow.core.Try import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.internal.extensions.foldToCallback +import im.vector.matrix.android.internal.util.awaitCallback import im.vector.riotx.core.files.addEntryToDownloadManager import im.vector.riotx.core.files.writeToFile import kotlinx.coroutines.Dispatchers @@ -36,28 +36,20 @@ class KeysExporter(private val session: Session) { * Export keys and return the file path with the callback */ fun export(context: Context, password: String, callback: MatrixCallback) { - session.exportRoomKeys(password, object : MatrixCallback { - override fun onSuccess(data: ByteArray) { - GlobalScope.launch(Dispatchers.Main) { - withContext(Dispatchers.IO) { - Try { - val parentDir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) - val file = File(parentDir, "riotx-keys-" + System.currentTimeMillis() + ".txt") + GlobalScope.launch(Dispatchers.Main) { + runCatching { + val data = awaitCallback { session.exportRoomKeys(password, it) } + withContext(Dispatchers.IO) { + val parentDir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + val file = File(parentDir, "riotx-keys-" + System.currentTimeMillis() + ".txt") - writeToFile(data, file) + writeToFile(data, file) - addEntryToDownloadManager(context, file, "text/plain") + addEntryToDownloadManager(context, file, "text/plain") - file.absolutePath - } - } - .foldToCallback(callback) + file.absolutePath } - } - - override fun onFailure(failure: Throwable) { - callback.onFailure(failure) - } - }) + }.foldToCallback(callback) + } } } diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysImporter.kt b/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysImporter.kt index 74b2a86bc1..b60e25af04 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysImporter.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/keys/KeysImporter.kt @@ -18,10 +18,11 @@ package im.vector.riotx.features.crypto.keys import android.content.Context import android.net.Uri -import arrow.core.Try import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.internal.crypto.model.ImportRoomKeysResult +import im.vector.matrix.android.internal.extensions.foldToCallback +import im.vector.matrix.android.internal.util.awaitCallback import im.vector.riotx.core.intent.getMimeTypeFromUri import im.vector.riotx.core.resources.openResource import kotlinx.coroutines.Dispatchers @@ -41,8 +42,8 @@ class KeysImporter(private val session: Session) { password: String, callback: MatrixCallback) { GlobalScope.launch(Dispatchers.Main) { - withContext(Dispatchers.IO) { - Try { + runCatching { + withContext(Dispatchers.IO) { val resource = openResource(context, uri, mimetype ?: getMimeTypeFromUri(context, uri)) if (resource?.mContentStream == null) { @@ -51,33 +52,17 @@ class KeysImporter(private val session: Session) { val data: ByteArray try { - data = ByteArray(resource.mContentStream!!.available()) - resource.mContentStream!!.read(data) - resource.mContentStream!!.close() - - data + data = resource.mContentStream!!.use { it.readBytes() } } catch (e: Exception) { - try { - resource.mContentStream!!.close() - } catch (e2: Exception) { - Timber.e(e2, "## importKeys()") - } - + Timber.e(e, "## importKeys()") throw e } + + awaitCallback { + session.importRoomKeys(data, password, null, it) + } } - } - .fold( - { - callback.onFailure(it) - }, - { byteArray -> - session.importRoomKeys(byteArray, - password, - null, - callback) - } - ) + }.foldToCallback(callback) } } } diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt b/vector/src/main/java/im/vector/riotx/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt index 7b61ca2c0f..a5cc0510da 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt @@ -170,8 +170,8 @@ class KeysBackupSetupStep3Fragment : VectorBaseFragment() { private fun exportRecoveryKeyToFile(data: String) { GlobalScope.launch(Dispatchers.Main) { - withContext(Dispatchers.IO) { - Try { + Try { + withContext(Dispatchers.IO) { val parentDir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) val file = File(parentDir, "recovery-key-" + System.currentTimeMillis() + ".txt") diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 135496264d..63a4919763 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -42,7 +42,8 @@ import im.vector.riotx.features.home.room.detail.timeline.format.NoticeEventForm import im.vector.riotx.features.home.room.detail.timeline.item.MessageInformationData import im.vector.riotx.features.html.EventHtmlRenderer import java.text.SimpleDateFormat -import java.util.* +import java.util.Date +import java.util.Locale /** * Quick reactions state