From ecf3fee7094334ce9d47dbf31e6355dbf714a6b4 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoitm@matrix.org> Date: Mon, 18 May 2020 10:15:25 +0200 Subject: [PATCH] Integrate Valere's remarks - step 3: use viewModelScope in ViewModels --- .../riotx/core/platform/VectorViewModel.kt | 2 +- .../discovery/DiscoverySettingsViewModel.kt | 191 ++++++++---------- .../change/SetIdentityServerViewModel.kt | 77 +++---- .../features/terms/ReviewTermsViewModel.kt | 58 +++--- 4 files changed, 152 insertions(+), 176 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/core/platform/VectorViewModel.kt b/vector/src/main/java/im/vector/riotx/core/platform/VectorViewModel.kt index e82e8b3856..11cd9c485e 100644 --- a/vector/src/main/java/im/vector/riotx/core/platform/VectorViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/core/platform/VectorViewModel.kt @@ -30,7 +30,7 @@ import io.reactivex.Single abstract class VectorViewModel<S : MvRxState, VA : VectorViewModelAction, VE : VectorViewEvents>(initialState: S) : BaseMvRxViewModel<S>(initialState, false) { - interface Factory<S: MvRxState> { + interface Factory<S : MvRxState> { fun create(state: S): BaseMvRxViewModel<S> } diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt index d24a466036..7c5086afa7 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt @@ -15,6 +15,7 @@ */ package im.vector.riotx.features.discovery +import androidx.lifecycle.viewModelScope import com.airbnb.mvrx.Async import com.airbnb.mvrx.Fail import com.airbnb.mvrx.FragmentViewModelContext @@ -25,15 +26,16 @@ import com.airbnb.mvrx.Uninitialized import com.airbnb.mvrx.ViewModelContext import com.squareup.inject.assisted.Assisted import com.squareup.inject.assisted.AssistedInject -import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.identity.IdentityServiceError import im.vector.matrix.android.api.session.identity.IdentityServiceListener import im.vector.matrix.android.api.session.identity.SharedState import im.vector.matrix.android.api.session.identity.ThreePid +import im.vector.matrix.android.internal.util.awaitCallback import im.vector.matrix.rx.rx import im.vector.riotx.core.extensions.exhaustive import im.vector.riotx.core.platform.VectorViewModel +import kotlinx.coroutines.launch class DiscoverySettingsViewModel @AssistedInject constructor( @Assisted initialState: DiscoverySettingsState, @@ -104,72 +106,47 @@ class DiscoverySettingsViewModel @AssistedInject constructor( } private fun disconnectIdentityServer() { - setState { - copy( - identityServer = Loading() - ) + setState { copy(identityServer = Loading()) } + + viewModelScope.launch { + try { + awaitCallback<Unit> { session.identityService().disconnect(it) } + setState { copy(identityServer = Success(null)) } + } catch (failure: Throwable) { + setState { copy(identityServer = Fail(failure)) } + } } - - session.identityService().disconnect(object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { - setState { - copy( - identityServer = Success(null) - ) - } - } - - override fun onFailure(failure: Throwable) { - setState { - copy( - identityServer = Fail(failure) - ) - } - } - }) } private fun changeIdentityServer(action: DiscoverySettingsAction.ChangeIdentityServer) { - setState { - copy( - identityServer = Loading() - ) - } + setState { copy(identityServer = Loading()) } - session.identityService().setNewIdentityServer(action.url, object : MatrixCallback<String?> { - override fun onSuccess(data: String?) { - setState { - copy( - identityServer = Success(data) - ) + viewModelScope.launch { + try { + val data = awaitCallback<String?> { + session.identityService().setNewIdentityServer(action.url, it) } + setState { copy(identityServer = Success(data)) } retrieveBinding() + } catch (failure: Throwable) { + setState { copy(identityServer = Fail(failure)) } } - - override fun onFailure(failure: Throwable) { - setState { - copy( - identityServer = Fail(failure) - ) - } - } - }) + } } private fun shareThreePid(action: DiscoverySettingsAction.ShareThreePid) = withState { state -> if (state.identityServer() == null) return@withState changeThreePidState(action.threePid, Loading()) - identityService.startBindThreePid(action.threePid, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + awaitCallback<Unit> { identityService.startBindThreePid(action.threePid, it) } changeThreePidState(action.threePid, Success(SharedState.BINDING_IN_PROGRESS)) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) changeThreePidState(action.threePid, Fail(failure)) } - }) + } } private fun changeThreePidState(threePid: ThreePid, state: Async<SharedState>) { @@ -238,16 +215,15 @@ class DiscoverySettingsViewModel @AssistedInject constructor( if (state.emailList() == null) return@withState changeThreePidState(threePid, Loading()) - identityService.unbindThreePid(threePid, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + awaitCallback<Unit> { identityService.unbindThreePid(threePid, it) } changeThreePidState(threePid, Success(SharedState.NOT_SHARED)) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) changeThreePidState(threePid, Fail(failure)) } - }) + } } private fun revokeMsisdn(threePid: ThreePid.Msisdn) = withState { state -> @@ -255,29 +231,27 @@ class DiscoverySettingsViewModel @AssistedInject constructor( if (state.phoneNumbersList() == null) return@withState changeThreePidState(threePid, Loading()) - identityService.unbindThreePid(threePid, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + awaitCallback<Unit> { identityService.unbindThreePid(threePid, it) } changeThreePidState(threePid, Success(SharedState.NOT_SHARED)) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) changeThreePidState(threePid, Fail(failure)) } - }) + } } private fun cancelBinding(action: DiscoverySettingsAction.CancelBinding) { - identityService.cancelBindThreePid(action.threePid, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + awaitCallback<Unit> { identityService.cancelBindThreePid(action.threePid, it) } changeThreePidState(action.threePid, Success(SharedState.NOT_SHARED)) changeThreePidSubmitState(action.threePid, Uninitialized) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { // This could never fail } - }) + } } private fun startListenToIdentityManager() { @@ -305,32 +279,32 @@ class DiscoverySettingsViewModel @AssistedInject constructor( ) } - identityService.getShareStatus(threePids, - object : MatrixCallback<Map<ThreePid, SharedState>> { - override fun onSuccess(data: Map<ThreePid, SharedState>) { - setState { - copy( - emailList = Success(data.filter { it.key is ThreePid.Email }.toPidInfoList()), - phoneNumbersList = Success(data.filter { it.key is ThreePid.Msisdn }.toPidInfoList()), - termsNotSigned = false - ) - } - } + viewModelScope.launch { + try { + val data = awaitCallback<Map<ThreePid, SharedState>> { + identityService.getShareStatus(threePids, it) + } + setState { + copy( + emailList = Success(data.filter { it.key is ThreePid.Email }.toPidInfoList()), + phoneNumbersList = Success(data.filter { it.key is ThreePid.Msisdn }.toPidInfoList()), + termsNotSigned = false + ) + } + } catch (failure: Throwable) { + if (failure !is IdentityServiceError.TermsNotSignedException) { + _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) + } - override fun onFailure(failure: Throwable) { - if (failure !is IdentityServiceError.TermsNotSignedException) { - _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) - } - - setState { - copy( - emailList = Success(emails.map { PidInfo(it, Fail(failure)) }), - phoneNumbersList = Success(msisdns.map { PidInfo(it, Fail(failure)) }), - termsNotSigned = failure is IdentityServiceError.TermsNotSignedException - ) - } - } - }) + setState { + copy( + emailList = Success(emails.map { PidInfo(it, Fail(failure)) }), + phoneNumbersList = Success(msisdns.map { PidInfo(it, Fail(failure)) }), + termsNotSigned = failure is IdentityServiceError.TermsNotSignedException + ) + } + } + } } private fun Map<ThreePid, SharedState>.toPidInfoList(): List<PidInfo> { @@ -347,19 +321,17 @@ class DiscoverySettingsViewModel @AssistedInject constructor( changeThreePidSubmitState(action.threePid, Loading()) - identityService.submitValidationToken(action.threePid, - action.code, - object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { - changeThreePidSubmitState(action.threePid, Uninitialized) - finalizeBind3pid(DiscoverySettingsAction.FinalizeBind3pid(action.threePid), true) - } - - override fun onFailure(failure: Throwable) { - changeThreePidSubmitState(action.threePid, Fail(failure)) - } + viewModelScope.launch { + try { + awaitCallback<Unit> { + identityService.submitValidationToken(action.threePid, action.code, it) } - ) + changeThreePidSubmitState(action.threePid, Uninitialized) + finalizeBind3pid(DiscoverySettingsAction.FinalizeBind3pid(action.threePid), true) + } catch (failure: Throwable) { + changeThreePidSubmitState(action.threePid, Fail(failure)) + } + } } private fun finalizeBind3pid(action: DiscoverySettingsAction.FinalizeBind3pid, fromUser: Boolean) = withState { state -> @@ -374,13 +346,12 @@ class DiscoverySettingsViewModel @AssistedInject constructor( changeThreePidSubmitState(action.threePid, Loading()) - identityService.finalizeBindThreePid(threePid, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + awaitCallback<Unit> { identityService.finalizeBindThreePid(threePid, it) } changeThreePidSubmitState(action.threePid, Uninitialized) changeThreePidState(action.threePid, Success(SharedState.SHARED)) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { // If this is not from user (user did not click to "Continue", but this is a refresh when Fragment is resumed), do no display the error if (fromUser) { changeThreePidSubmitState(action.threePid, Fail(failure)) @@ -388,7 +359,7 @@ class DiscoverySettingsViewModel @AssistedInject constructor( changeThreePidSubmitState(action.threePid, Uninitialized) } } - }) + } } private fun refreshPendingEmailBindings() = withState { state -> diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt index 0aad3dd237..096623a75b 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt @@ -15,23 +15,25 @@ */ package im.vector.riotx.features.discovery.change +import androidx.lifecycle.viewModelScope import com.airbnb.mvrx.FragmentViewModelContext import com.airbnb.mvrx.MvRxViewModelFactory import com.airbnb.mvrx.ViewModelContext import com.squareup.inject.assisted.Assisted import com.squareup.inject.assisted.AssistedInject -import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.failure.Failure import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.identity.IdentityServiceError import im.vector.matrix.android.api.session.terms.GetTermsResponse import im.vector.matrix.android.api.session.terms.TermsService +import im.vector.matrix.android.internal.util.awaitCallback import im.vector.riotx.R import im.vector.riotx.core.di.HasScreenInjector import im.vector.riotx.core.extensions.exhaustive import im.vector.riotx.core.platform.VectorViewModel import im.vector.riotx.core.resources.StringProvider import im.vector.riotx.core.utils.ensureProtocol +import kotlinx.coroutines.launch class SetIdentityServerViewModel @AssistedInject constructor( @Assisted initialState: SetIdentityServerState, @@ -91,14 +93,15 @@ class SetIdentityServerViewModel @AssistedInject constructor( _viewEvents.post(SetIdentityServerViewEvents.Loading()) - // First ping the identity server v2 API - mxSession.identityService().isValidIdentityServer(baseUrl, object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { + viewModelScope.launch { + try { + // First ping the identity server v2 API + awaitCallback<Unit> { + mxSession.identityService().isValidIdentityServer(baseUrl, it) + } // Ok, next step checkTerms(baseUrl) - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { if (failure is IdentityServiceError.OutdatedIdentityServer) { _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.identity_server_error_outdated_identity_server)) } else { @@ -106,39 +109,37 @@ class SetIdentityServerViewModel @AssistedInject constructor( _viewEvents.post(SetIdentityServerViewEvents.OtherFailure(failure)) } } - }) + } } - private fun checkTerms(baseUrl: String) { - mxSession.getTerms(TermsService.ServiceType.IdentityService, - baseUrl, - object : MatrixCallback<GetTermsResponse> { - override fun onSuccess(data: GetTermsResponse) { - // has all been accepted? - val resp = data.serverResponse - val tos = resp.getLocalizedTerms(userLanguage) - if (tos.isEmpty()) { - // prompt do not define policy - _viewEvents.post(SetIdentityServerViewEvents.NoTerms) - } else { - val shouldPrompt = tos.any { !data.alreadyAcceptedTermUrls.contains(it.localizedUrl) } - if (shouldPrompt) { - _viewEvents.post(SetIdentityServerViewEvents.ShowTerms(baseUrl)) - } else { - _viewEvents.post(SetIdentityServerViewEvents.TermsAccepted) - } - } - } + private suspend fun checkTerms(baseUrl: String) { + try { + val data = awaitCallback<GetTermsResponse> { + mxSession.getTerms(TermsService.ServiceType.IdentityService, baseUrl, it) + } - override fun onFailure(failure: Throwable) { - if (failure is Failure.OtherServerError && failure.httpCode == 404) { - // 404: Same as NoTerms - _viewEvents.post(SetIdentityServerViewEvents.NoTerms) - } else { - _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.settings_discovery_bad_identity_server)) - _viewEvents.post(SetIdentityServerViewEvents.OtherFailure(failure)) - } - } - }) + // has all been accepted? + val resp = data.serverResponse + val tos = resp.getLocalizedTerms(userLanguage) + if (tos.isEmpty()) { + // prompt do not define policy + _viewEvents.post(SetIdentityServerViewEvents.NoTerms) + } else { + val shouldPrompt = tos.any { !data.alreadyAcceptedTermUrls.contains(it.localizedUrl) } + if (shouldPrompt) { + _viewEvents.post(SetIdentityServerViewEvents.ShowTerms(baseUrl)) + } else { + _viewEvents.post(SetIdentityServerViewEvents.TermsAccepted) + } + } + } catch (failure: Throwable) { + if (failure is Failure.OtherServerError && failure.httpCode == 404) { + // 404: Same as NoTerms + _viewEvents.post(SetIdentityServerViewEvents.NoTerms) + } else { + _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.settings_discovery_bad_identity_server)) + _viewEvents.post(SetIdentityServerViewEvents.OtherFailure(failure)) + } + } } } diff --git a/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsViewModel.kt index 90fd01e9e5..4c9a0d858c 100644 --- a/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsViewModel.kt @@ -15,6 +15,7 @@ */ package im.vector.riotx.features.terms +import androidx.lifecycle.viewModelScope import com.airbnb.mvrx.ActivityViewModelContext import com.airbnb.mvrx.Loading import com.airbnb.mvrx.MvRxViewModelFactory @@ -23,11 +24,12 @@ import com.airbnb.mvrx.Uninitialized import com.airbnb.mvrx.ViewModelContext import com.squareup.inject.assisted.Assisted import com.squareup.inject.assisted.AssistedInject -import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.terms.GetTermsResponse +import im.vector.matrix.android.internal.util.awaitCallback import im.vector.riotx.core.extensions.exhaustive import im.vector.riotx.core.platform.VectorViewModel +import kotlinx.coroutines.launch import timber.log.Timber class ReviewTermsViewModel @AssistedInject constructor( @@ -94,27 +96,28 @@ class ReviewTermsViewModel @AssistedInject constructor( val agreedUrls = acceptedTerms.map { it.url } - session.agreeToTerms( - termsArgs.type, - termsArgs.baseURL, - agreedUrls, - termsArgs.token, - object : MatrixCallback<Unit> { - override fun onSuccess(data: Unit) { - _viewEvents.post(ReviewTermsViewEvents.Success) - } - - override fun onFailure(failure: Throwable) { - Timber.e(failure, "Failed to agree to terms") - setState { - copy( - acceptingTerms = Uninitialized - ) - } - _viewEvents.post(ReviewTermsViewEvents.Failure(failure, false)) - } + viewModelScope.launch { + try { + awaitCallback<Unit> { + session.agreeToTerms( + termsArgs.type, + termsArgs.baseURL, + agreedUrls, + termsArgs.token, + it + ) } - ) + _viewEvents.post(ReviewTermsViewEvents.Success) + } catch (failure: Throwable) { + Timber.e(failure, "Failed to agree to terms") + setState { + copy( + acceptingTerms = Uninitialized + ) + } + _viewEvents.post(ReviewTermsViewEvents.Failure(failure, false)) + } + } } private fun loadTerms(action: ReviewTermsAction.LoadTerms) = withState { state -> @@ -126,8 +129,11 @@ class ReviewTermsViewModel @AssistedInject constructor( copy(termsList = Loading()) } - session.getTerms(termsArgs.type, termsArgs.baseURL, object : MatrixCallback<GetTermsResponse> { - override fun onSuccess(data: GetTermsResponse) { + viewModelScope.launch { + try { + val data = awaitCallback<GetTermsResponse> { + session.getTerms(termsArgs.type, termsArgs.baseURL, it) + } val terms = data.serverResponse.getLocalizedTerms(action.preferredLanguageCode).map { Term(it.localizedUrl ?: "", it.localizedName ?: "", @@ -141,9 +147,7 @@ class ReviewTermsViewModel @AssistedInject constructor( termsList = Success(terms) ) } - } - - override fun onFailure(failure: Throwable) { + } catch (failure: Throwable) { Timber.e(failure, "Failed to agree to terms") setState { copy( @@ -152,6 +156,6 @@ class ReviewTermsViewModel @AssistedInject constructor( } _viewEvents.post(ReviewTermsViewEvents.Failure(failure, true)) } - }) + } } }