From 73462a3045e2ebf66677e77ab7188cf831afd9c3 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 22 Nov 2019 20:04:11 +0100 Subject: [PATCH] Clean some coroutine code --- .../internal/auth/login/DefaultLoginWizard.kt | 33 +-- .../registration/DefaultRegistrationWizard.kt | 220 +++++++----------- .../internal/task/CoroutineToCallback.kt | 36 +++ .../android/internal/task/TaskExecutor.kt | 39 ++-- .../internal/util/CancelableCoroutine.kt | 4 + 5 files changed, 151 insertions(+), 181 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/CoroutineToCallback.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/login/DefaultLoginWizard.kt index 5ba1ae85ec..534e98f84c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/login/DefaultLoginWizard.kt @@ -35,13 +35,11 @@ import im.vector.matrix.android.internal.auth.data.ThreePidMedium import im.vector.matrix.android.internal.auth.registration.AddThreePidRegistrationParams import im.vector.matrix.android.internal.auth.registration.AddThreePidRegistrationResponse import im.vector.matrix.android.internal.auth.registration.RegisterAddThreePidTask -import im.vector.matrix.android.internal.extensions.foldToCallback import im.vector.matrix.android.internal.network.RetrofitFactory import im.vector.matrix.android.internal.network.executeRequest -import im.vector.matrix.android.internal.util.CancelableCoroutine +import im.vector.matrix.android.internal.task.launchToCallback import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import java.util.* @@ -73,18 +71,14 @@ internal class DefaultLoginWizard( password: String, deviceName: String, callback: MatrixCallback): Cancelable { - val job = GlobalScope.launch(coroutineDispatchers.main) { - val sessionOrFailure = runCatching { - loginInternal(login, password, deviceName) - } - sessionOrFailure.foldToCallback(callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + loginInternal(login, password, deviceName) } - return CancelableCoroutine(job) } private suspend fun loginInternal(login: String, password: String, - deviceName: String) = withContext(coroutineDispatchers.io) { + deviceName: String) = withContext(coroutineDispatchers.computation) { val loginParams = if (Patterns.EMAIL_ADDRESS.matcher(login).matches()) { PasswordLoginParams.thirdPartyIdentifier(ThreePidMedium.EMAIL, login, password, deviceName) } else { @@ -94,19 +88,14 @@ internal class DefaultLoginWizard( apiCall = authAPI.login(loginParams) } val sessionParams = SessionParams(credentials, homeServerConnectionConfig) - sessionParamsStore.save(sessionParams) sessionManager.getOrCreateSession(sessionParams) } override fun resetPassword(email: String, newPassword: String, callback: MatrixCallback): Cancelable { - val job = GlobalScope.launch(coroutineDispatchers.main) { - val result = runCatching { - resetPasswordInternal(email, newPassword) - } - result.foldToCallback(callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + resetPasswordInternal(email, newPassword) } - return CancelableCoroutine(job) } private suspend fun resetPasswordInternal(email: String, newPassword: String) { @@ -115,11 +104,9 @@ internal class DefaultLoginWizard( clientSecret, sendAttempt++ ) - val result = executeRequest { apiCall = authAPI.resetPassword(AddThreePidRegistrationParams.from(param)) } - resetPasswordData = ResetPasswordData(newPassword, result) } @@ -128,13 +115,9 @@ internal class DefaultLoginWizard( callback.onFailure(IllegalStateException("developer error, no reset password in progress")) return NoOpCancellable } - val job = GlobalScope.launch(coroutineDispatchers.main) { - val result = runCatching { - resetPasswordMailConfirmedInternal(safeResetPasswordData) - } - result.foldToCallback(callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + resetPasswordMailConfirmedInternal(safeResetPasswordData) } - return CancelableCoroutine(job) } private suspend fun resetPasswordMailConfirmedInternal(resetPasswordData: ResetPasswordData) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/DefaultRegistrationWizard.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/DefaultRegistrationWizard.kt index 356d1aa120..72a2206fb9 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/DefaultRegistrationWizard.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/DefaultRegistrationWizard.kt @@ -24,6 +24,7 @@ import im.vector.matrix.android.api.auth.registration.RegisterThreePid import im.vector.matrix.android.api.auth.registration.RegistrationResult import im.vector.matrix.android.api.auth.registration.RegistrationWizard import im.vector.matrix.android.api.failure.Failure +import im.vector.matrix.android.api.failure.Failure.RegistrationFlowError import im.vector.matrix.android.api.util.Cancelable import im.vector.matrix.android.api.util.NoOpCancellable import im.vector.matrix.android.internal.SessionManager @@ -31,11 +32,9 @@ import im.vector.matrix.android.internal.auth.AuthAPI import im.vector.matrix.android.internal.auth.SessionParamsStore import im.vector.matrix.android.internal.auth.data.LoginFlowTypes import im.vector.matrix.android.internal.network.RetrofitFactory -import im.vector.matrix.android.internal.util.CancelableCoroutine +import im.vector.matrix.android.internal.task.launchToCallback import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers -import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.delay -import kotlinx.coroutines.launch +import kotlinx.coroutines.* import okhttp3.OkHttpClient import java.util.* @@ -80,18 +79,24 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: } override fun getRegistrationFlow(callback: MatrixCallback): Cancelable { - return performRegistrationRequest(RegistrationParams(), callback) + val params = RegistrationParams() + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + performRegistrationRequest(params) + } } override fun createAccount(userName: String, password: String, initialDeviceDisplayName: String?, callback: MatrixCallback): Cancelable { - return performRegistrationRequest(RegistrationParams( + val params = RegistrationParams( username = userName, password = password, initialDeviceDisplayName = initialDeviceDisplayName - ), callback) + ) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + performRegistrationRequest(params) + } } override fun performReCaptcha(response: String, callback: MatrixCallback): Cancelable { @@ -99,11 +104,10 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) return NoOpCancellable } - - return performRegistrationRequest( - RegistrationParams( - auth = AuthParams.createForCaptcha(safeSession, response) - ), callback) + val params = RegistrationParams(auth = AuthParams.createForCaptcha(safeSession, response)) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + performRegistrationRequest(params) + } } override fun acceptTerms(callback: MatrixCallback): Cancelable { @@ -111,20 +115,17 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) return NoOpCancellable } - - return performRegistrationRequest( - RegistrationParams( - auth = AuthParams( - type = LoginFlowTypes.TERMS, - session = safeSession - ) - ), callback) + val params = RegistrationParams(auth = AuthParams(type = LoginFlowTypes.TERMS, session = safeSession)) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + performRegistrationRequest(params) + } } override fun addThreePid(threePid: RegisterThreePid, callback: MatrixCallback): Cancelable { currentThreePidData = null - - return sendThreePid(threePid, callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + sendThreePid(threePid) + } } override fun sendAgainThreePid(callback: MatrixCallback): Cancelable { @@ -132,54 +133,34 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) return NoOpCancellable } - - return sendThreePid(safeCurrentThreePid, callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + sendThreePid(safeCurrentThreePid) + } } - private fun sendThreePid(threePid: RegisterThreePid, callback: MatrixCallback): Cancelable { - val safeSession = currentSession ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } - - val job = GlobalScope.launch(coroutineDispatchers.main) { - runCatching { - registerAddThreePidTask.execute(RegisterAddThreePidTask.Params(threePid, clientSecret, sendAttempt++)) - } - .fold( - { - // Store data - currentThreePidData = ThreePidData( - threePid, - it, - RegistrationParams( - auth = if (threePid is RegisterThreePid.Email) { - AuthParams.createForEmailIdentity(safeSession, - ThreePidCredentials( - clientSecret = clientSecret, - sid = it.sid - ) - ) - } else { - AuthParams.createForMsisdnIdentity(safeSession, - ThreePidCredentials( - clientSecret = clientSecret, - sid = it.sid - ) - ) - } - )) - .also { threePidData -> - // and send the sid a first time - performRegistrationRequest(threePidData.registrationParams, callback) - } - }, - { - callback.onFailure(it) - } + private suspend fun sendThreePid(threePid: RegisterThreePid): RegistrationResult { + val safeSession = currentSession ?: throw IllegalStateException("developer error, call createAccount() method first") + val response = registerAddThreePidTask.execute(RegisterAddThreePidTask.Params(threePid, clientSecret, sendAttempt++)) + val params = RegistrationParams( + auth = if (threePid is RegisterThreePid.Email) { + AuthParams.createForEmailIdentity(safeSession, + ThreePidCredentials( + clientSecret = clientSecret, + sid = response.sid + ) ) - } - return CancelableCoroutine(job) + } else { + AuthParams.createForMsisdnIdentity(safeSession, + ThreePidCredentials( + clientSecret = clientSecret, + sid = response.sid + ) + ) + } + ) + // Store data + currentThreePidData = ThreePidData(threePid, response, params) + return performRegistrationRequest(params) } override fun checkIfEmailHasBeenValidated(delayMillis: Long, callback: MatrixCallback): Cancelable { @@ -187,53 +168,32 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: callback.onFailure(IllegalStateException("developer error, no pending three pid")) return NoOpCancellable } - - return performRegistrationRequest(safeParam, callback, delayMillis) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + performRegistrationRequest(safeParam) + } } override fun handleValidateThreePid(code: String, callback: MatrixCallback): Cancelable { - val safeParam = currentThreePidData?.registrationParams ?: run { - callback.onFailure(IllegalStateException("developer error, no pending three pid")) - return NoOpCancellable + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + validateThreePid(code) } + } - val safeCurrentData = currentThreePidData ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } - - val url = safeCurrentData.addThreePidRegistrationResponse.submitUrl ?: run { - callback.onFailure(IllegalStateException("Missing url the send the code")) - return NoOpCancellable - } - - val params = ValidationCodeBody( + private suspend fun validateThreePid(code: String): RegistrationResult { + val registrationParams = currentThreePidData?.registrationParams ?: throw IllegalStateException("developer error, no pending three pid") + val safeCurrentData = currentThreePidData ?: throw IllegalStateException("developer error, call createAccount() method first") + val url = safeCurrentData.addThreePidRegistrationResponse.submitUrl ?: throw IllegalStateException("Missing url the send the code") + val validationBody = ValidationCodeBody( clientSecret = clientSecret, sid = safeCurrentData.addThreePidRegistrationResponse.sid, code = code ) - - val job = GlobalScope.launch(coroutineDispatchers.main) { - runCatching { - validateCodeTask.execute(ValidateCodeTask.Params(url, params)) - } - .fold( - { - if (it.success == true) { - // The entered code is correct - // Same that validate email - performRegistrationRequest(safeParam, callback, 3_000) - } else { - // The code is not correct - callback.onFailure(Failure.SuccessError) - } - }, - { - callback.onFailure(it) - } - ) + val validationResponse = validateCodeTask.execute(ValidateCodeTask.Params(url, validationBody)) + if (validationResponse.success == true) { + return performRegistrationRequest(registrationParams, 3_000) + } else { + throw Failure.SuccessError } - return CancelableCoroutine(job) } override fun dummy(callback: MatrixCallback): Cancelable { @@ -241,43 +201,29 @@ internal class DefaultRegistrationWizard(private val homeServerConnectionConfig: callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) return NoOpCancellable } - - return performRegistrationRequest( - RegistrationParams( - auth = AuthParams( - type = LoginFlowTypes.DUMMY, - session = safeSession - ) - ), callback) + return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) { + val params = RegistrationParams(auth = AuthParams(type = LoginFlowTypes.DUMMY, session = safeSession)) + performRegistrationRequest(params) + } } - private fun performRegistrationRequest(registrationParams: RegistrationParams, - callback: MatrixCallback, - delayMillis: Long = 0): Cancelable { - val job = GlobalScope.launch(coroutineDispatchers.main) { - runCatching { - if (delayMillis > 0) delay(delayMillis) - registerTask.execute(RegisterTask.Params(registrationParams)) + private suspend fun performRegistrationRequest(registrationParams: RegistrationParams, + delayMillis: Long = 0): RegistrationResult { + delay(delayMillis) + val credentials = try { + registerTask.execute(RegisterTask.Params(registrationParams)) + } catch (exception: Throwable) { + if (exception is RegistrationFlowError) { + currentSession = exception.registrationFlowResponse.session + return RegistrationResult.FlowResponse(exception.registrationFlowResponse.toFlowResult()) + } else { + throw exception } - .fold( - { - val sessionParams = SessionParams(it, homeServerConnectionConfig) - sessionParamsStore.save(sessionParams) - val session = sessionManager.getOrCreateSession(sessionParams) - - callback.onSuccess(RegistrationResult.Success(session)) - }, - { - if (it is Failure.RegistrationFlowError) { - currentSession = it.registrationFlowResponse.session - callback.onSuccess(RegistrationResult.FlowResponse(it.registrationFlowResponse.toFlowResult())) - } else { - callback.onFailure(it) - } - } - ) } - return CancelableCoroutine(job) + val sessionParams = SessionParams(credentials, homeServerConnectionConfig) + sessionParamsStore.save(sessionParams) + val session = sessionManager.getOrCreateSession(sessionParams) + return RegistrationResult.Success(session) } private fun buildAuthAPI(): AuthAPI { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/CoroutineToCallback.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/CoroutineToCallback.kt new file mode 100644 index 0000000000..2d0457cb7c --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/CoroutineToCallback.kt @@ -0,0 +1,36 @@ +/* + * Copyright 2019 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.matrix.android.internal.task + +import im.vector.matrix.android.api.MatrixCallback +import im.vector.matrix.android.api.util.Cancelable +import im.vector.matrix.android.internal.extensions.foldToCallback +import im.vector.matrix.android.internal.util.toCancelable +import kotlinx.coroutines.* +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.EmptyCoroutineContext + +internal fun CoroutineScope.launchToCallback( + context: CoroutineContext = EmptyCoroutineContext, + callback: MatrixCallback, + block: suspend () -> T +): Cancelable = launch(context, CoroutineStart.DEFAULT) { + val result = runCatching { + block() + } + result.foldToCallback(callback) +}.toCancelable() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/TaskExecutor.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/TaskExecutor.kt index 14e546e0d6..d5392779d1 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/TaskExecutor.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/task/TaskExecutor.kt @@ -20,8 +20,8 @@ import im.vector.matrix.android.api.util.Cancelable import im.vector.matrix.android.internal.di.MatrixScope import im.vector.matrix.android.internal.extensions.foldToCallback import im.vector.matrix.android.internal.network.NetworkConnectivityChecker -import im.vector.matrix.android.internal.util.CancelableCoroutine import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers +import im.vector.matrix.android.internal.util.toCancelable import kotlinx.coroutines.* import timber.log.Timber import javax.inject.Inject @@ -34,27 +34,28 @@ internal class TaskExecutor @Inject constructor(private val coroutineDispatchers private val executorScope = CoroutineScope(SupervisorJob()) fun execute(task: ConfigurableTask): Cancelable { - val job = executorScope.launch(task.callbackThread.toDispatcher()) { - val resultOrFailure = runCatching { - withContext(task.executionThread.toDispatcher()) { - Timber.v("Enqueue task $task") - retry(task.retryCount) { - if (task.constraints.connectedToNetwork) { - Timber.v("Waiting network for $task") - networkConnectivityChecker.waitUntilConnected() + return executorScope + .launch(task.callbackThread.toDispatcher()) { + val resultOrFailure = runCatching { + withContext(task.executionThread.toDispatcher()) { + Timber.v("Enqueue task $task") + retry(task.retryCount) { + if (task.constraints.connectedToNetwork) { + Timber.v("Waiting network for $task") + networkConnectivityChecker.waitUntilConnected() + } + Timber.v("Execute task $task on ${Thread.currentThread().name}") + task.execute(task.params) + } } - Timber.v("Execute task $task on ${Thread.currentThread().name}") - task.execute(task.params) } + resultOrFailure + .onFailure { + Timber.d(it, "Task failed") + } + .foldToCallback(task.callback) } - } - resultOrFailure - .onFailure { - Timber.d(it, "Task failed") - } - .foldToCallback(task.callback) - } - return CancelableCoroutine(job) + .toCancelable() } fun cancelAll() = executorScope.coroutineContext.cancelChildren() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/CancelableCoroutine.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/CancelableCoroutine.kt index 71e2d3fdb2..c2f138e04c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/CancelableCoroutine.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/CancelableCoroutine.kt @@ -19,6 +19,10 @@ package im.vector.matrix.android.internal.util import im.vector.matrix.android.api.util.Cancelable import kotlinx.coroutines.Job +internal fun Job.toCancelable(): Cancelable { + return CancelableCoroutine(this) +} + internal class CancelableCoroutine(private val job: Job) : Cancelable { override fun cancel() {