Skip to content

Convert AccountService to suspend functions #2354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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.
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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> {
Expand Down