Merge branch 'develop' into fix_draft_sharesheet

2025-01-27 09:31:20 +01:00 · 2020-11-10 11:34:29 +00:00 · 2020-11-10 11:34:29 +00:00 · bbbaa60ddd
commit bbbaa60ddd
parent be3bc175bf dc17e5c3fa
10 changed files with 68 additions and 69 deletions
--- a/CHANGES.md
+++ b/CHANGES.md
@ -8,14 +8,15 @@ Improvements 🙌:
 - Open an existing DM instead of creating a new one (#2319)

 Bugfix 🐛:
- - Fix issue when updating the avatar of a room
 - Fix issue when restoring draft after sharing (#2287)
+ - Fix issue when updating the avatar of a room (new avatar vanishing)
+ - Discard change dialog displayed by mistake when avatar has been updated

 Translations 🗣:
 -

 SDK API changes ⚠️:
- -
+ - AccountService now exposes suspendable function instead of using MatrixCallback (#2354). Note: We will incrementally migrate all the SDK API in a near future.

 Build 🧱:
 -
--- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/ChangePasswordTest.kt
+++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/ChangePasswordTest.kt
@ -43,8 +43,8 @@ class ChangePasswordTest : InstrumentedTest {
        val session = commonTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(withInitialSync = false))

        // Change password
-        commonTestHelper.doSync<Unit> {
-            session.changePassword(TestConstants.PASSWORD, NEW_PASSWORD, it)
+        commonTestHelper.runBlockingTest {
+            session.changePassword(TestConstants.PASSWORD, NEW_PASSWORD)
        }

        // Try to login with the previous password, it will fail
--- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt
+++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt
@ -43,8 +43,8 @@ class DeactivateAccountTest : InstrumentedTest {
        val session = commonTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(withInitialSync = false))

        // Deactivate the account
-        commonTestHelper.doSync<Unit> {
-            session.deactivateAccount(TestConstants.PASSWORD, false, it)
+        commonTestHelper.runBlockingTest {
+            session.deactivateAccount(TestConstants.PASSWORD, false)
        }

        // Try to login on the previous account, it will fail (M_USER_DEACTIVATED)
--- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt
+++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt
@ -40,6 +40,7 @@ import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertTrue
@ -343,6 +344,14 @@ class CommonTestHelper(context: Context) {
        await(latch, timeout)
    }

+    fun <T> runBlockingTest(timeout: Long = TestConstants.timeOutMillis, block: suspend () -> T): T {
+        return runBlocking {
+            withTimeout(timeout) {
+                block()
+            }
+        }
+    }
+
    // Transform a method with a MatrixCallback to a synchronous method
    inline fun <reified T> doSync(timeout: Long? = TestConstants.timeOutMillis, block: (MatrixCallback<T>) -> Unit): T {
        val lock = CountDownLatch(1)
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/account/AccountService.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/account/AccountService.kt
@ -16,9 +16,6 @@

 package org.matrix.android.sdk.api.session.account

-import org.matrix.android.sdk.api.MatrixCallback
-import org.matrix.android.sdk.api.util.Cancelable
-
 /**
 * This interface defines methods to manage the account. It's implemented at the session level.
 */
@ -28,7 +25,7 @@ interface AccountService {
     * @param password Current password.
     * @param newPassword New password
     */
-    fun changePassword(password: String, newPassword: String, callback: MatrixCallback<Unit>): Cancelable
+    suspend fun changePassword(password: String, newPassword: String)

    /**
     * Deactivate the account.
@ -46,5 +43,5 @@ interface AccountService {
     * @param eraseAllData set to true to forget all messages that have been sent. Warning: this will cause future users to see
     * an incomplete view of conversations
     */
-    fun deactivateAccount(password: String, eraseAllData: Boolean, callback: MatrixCallback<Unit>): Cancelable
+    suspend fun deactivateAccount(password: String, eraseAllData: Boolean)
 }
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/account/DefaultAccountService.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/account/DefaultAccountService.kt
@ -16,30 +16,17 @@

 package org.matrix.android.sdk.internal.session.account

-import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.session.account.AccountService
-import org.matrix.android.sdk.api.util.Cancelable
-import org.matrix.android.sdk.internal.task.TaskExecutor
-import org.matrix.android.sdk.internal.task.configureWith
 import javax.inject.Inject

 internal class DefaultAccountService @Inject constructor(private val changePasswordTask: ChangePasswordTask,
-                                                         private val deactivateAccountTask: DeactivateAccountTask,
-                                                         private val taskExecutor: TaskExecutor) : AccountService {
+                                                         private val deactivateAccountTask: DeactivateAccountTask) : AccountService {

-    override fun changePassword(password: String, newPassword: String, callback: MatrixCallback<Unit>): Cancelable {
-        return changePasswordTask
-                .configureWith(ChangePasswordTask.Params(password, newPassword)) {
-                    this.callback = callback
-                }
-                .executeBy(taskExecutor)
+    override suspend fun changePassword(password: String, newPassword: String) {
+        changePasswordTask.execute(ChangePasswordTask.Params(password, newPassword))
    }

-    override fun deactivateAccount(password: String, eraseAllData: Boolean, callback: MatrixCallback<Unit>): Cancelable {
-        return deactivateAccountTask
-                .configureWith(DeactivateAccountTask.Params(password, eraseAllData)) {
-                    this.callback = callback
-                }
-                .executeBy(taskExecutor)
+    override suspend fun deactivateAccount(password: String, eraseAllData: Boolean) {
+        deactivateAccountTask.execute(DeactivateAccountTask.Params(password, eraseAllData))
    }
 }
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt
@ -35,6 +35,7 @@ import org.matrix.android.sdk.internal.task.TaskExecutor
 import org.matrix.android.sdk.internal.task.configureWith
 import org.matrix.android.sdk.internal.task.launchToCallback
 import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers
+import org.matrix.android.sdk.internal.util.awaitCallback

 internal class DefaultStateService @AssistedInject constructor(@Assisted private val roomId: String,
                                                               private val stateEventDataSource: StateEventDataSource,
@ -132,23 +133,23 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
    override fun updateAvatar(avatarUri: Uri, fileName: String, callback: MatrixCallback<Unit>): Cancelable {
        return taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) {
            val response = fileUploader.uploadFromUri(avatarUri, fileName, "image/jpeg")
-            sendStateEvent(
-                    eventType = EventType.STATE_ROOM_AVATAR,
-                    body = mapOf("url" to response.contentUri),
-                    callback = callback,
-                    stateKey = null
-            )
+            awaitCallback<Unit> {
+                sendStateEvent(
+                        eventType = EventType.STATE_ROOM_AVATAR,
+                        body = mapOf("url" to response.contentUri),
+                        callback = it,
+                        stateKey = null
+                )
+            }
        }
    }

    override fun deleteAvatar(callback: MatrixCallback<Unit>): Cancelable {
-        return taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) {
-            sendStateEvent(
-                    eventType = EventType.STATE_ROOM_AVATAR,
-                    body = emptyMap(),
-                    callback = callback,
-                    stateKey = null
-            )
-        }
+        return sendStateEvent(
+                eventType = EventType.STATE_ROOM_AVATAR,
+                body = emptyMap(),
+                callback = callback,
+                stateKey = null
+        )
    }
 }
--- a/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewModel.kt
@ -211,7 +211,10 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
                            postLoading(false)
                            setState {
                                deletePendingAvatar(this)
-                                copy(newHistoryVisibility = null)
+                                copy(
+                                        avatarAction = RoomSettingsViewState.AvatarAction.None,
+                                        newHistoryVisibility = null
+                                )
                            }
                            _viewEvents.post(RoomSettingsViewEvents.Success)
                        },
--- a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsGeneralFragment.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsGeneralFragment.kt
@ -27,6 +27,7 @@ import android.widget.ImageView
 import android.widget.Toast
 import androidx.appcompat.app.AlertDialog
 import androidx.core.view.isVisible
+import androidx.lifecycle.lifecycleScope
 import androidx.preference.EditTextPreference
 import androidx.preference.Preference
 import androidx.preference.PreferenceCategory
@ -451,28 +452,25 @@ class VectorSettingsGeneralFragment @Inject constructor(
                    val newPwd = newPasswordText.text.toString()

                    showPasswordLoadingView(true)
-                    session.changePassword(oldPwd, newPwd, object : MatrixCallback<Unit> {
-                        override fun onSuccess(data: Unit) {
-                            if (!isAdded) {
-                                return
-                            }
-                            showPasswordLoadingView(false)
+                    lifecycleScope.launch {
+                        val result = runCatching {
+                            session.changePassword(oldPwd, newPwd)
+                        }
+                        if (!isAdded) {
+                            return@launch
+                        }
+                        showPasswordLoadingView(false)
+                        result.fold({
                            dialog.dismiss()
                            activity.toast(R.string.settings_password_updated)
-                        }
-
-                        override fun onFailure(failure: Throwable) {
-                            if (!isAdded) {
-                                return
-                            }
-                            showPasswordLoadingView(false)
+                        }, { failure ->
                            if (failure.isInvalidPassword()) {
                                oldPasswordTil.error = getString(R.string.settings_fail_to_update_password_invalid_current_password)
                            } else {
                                oldPasswordTil.error = getString(R.string.settings_fail_to_update_password)
                            }
-                        }
-                    })
+                        })
+                    }
                }
            }
            dialog.show()
--- a/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
@ -15,6 +15,7 @@
 */
 package im.vector.app.features.settings.account.deactivation

+import androidx.lifecycle.viewModelScope
 import com.airbnb.mvrx.FragmentViewModelContext
 import com.airbnb.mvrx.MvRxState
 import com.airbnb.mvrx.MvRxViewModelFactory
@ -24,9 +25,10 @@ import com.squareup.inject.assisted.AssistedInject
 import im.vector.app.core.extensions.exhaustive
 import im.vector.app.core.platform.VectorViewModel
 import im.vector.app.core.platform.VectorViewModelAction
-import org.matrix.android.sdk.api.MatrixCallback
+import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.failure.isInvalidPassword
 import org.matrix.android.sdk.api.session.Session
+import java.lang.Exception

 data class DeactivateAccountViewState(
        val passwordShown: Boolean = false
@ -67,19 +69,20 @@ class DeactivateAccountViewModel @AssistedInject constructor(@Assisted private v

        _viewEvents.post(DeactivateAccountViewEvents.Loading())

-        session.deactivateAccount(action.password, action.eraseAllData, object : MatrixCallback<Unit> {
-            override fun onSuccess(data: Unit) {
-                _viewEvents.post(DeactivateAccountViewEvents.Done)
-            }
-
-            override fun onFailure(failure: Throwable) {
+        viewModelScope.launch {
+            val event = try {
+                session.deactivateAccount(action.password, action.eraseAllData)
+                DeactivateAccountViewEvents.Done
+            } catch (failure: Exception) {
                if (failure.isInvalidPassword()) {
-                    _viewEvents.post(DeactivateAccountViewEvents.InvalidPassword)
+                    DeactivateAccountViewEvents.InvalidPassword
                } else {
-                    _viewEvents.post(DeactivateAccountViewEvents.OtherFailure(failure))
+                    DeactivateAccountViewEvents.OtherFailure(failure)
                }
            }
-        })
+
+            _viewEvents.post(event)
+        }
    }

    companion object : MvRxViewModelFactory<DeactivateAccountViewModel, DeactivateAccountViewState> {