From a21f2fadf9ebe6ff9671639794352874edf5596c Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Wed, 28 Dec 2022 19:06:31 +0100 Subject: [PATCH] Remove rxjava from API calls used by AccountViewModel::changeRelationship() (#3008) * Remove rxjava from API calls used by AccountListFragment * Remove rxjava from API calls used by AccountViewModel::changeRelationship() The affected API functions are also called from - ReportViewModel.kt - SearchViewModel.kt - AccountListFragment.kt - SFragment.java - TimelineCases.kt so they have also been updated. This change requires bridging from Java code to Kotlin `suspend` functions, by creating wrappers for the `mute` and `block` functions that can be called from Java and create a coroutine scope. I've deliberately made this fairly ugly so that it sticks out and can be removed later. * Use "Throwable" type and name * Delete 46.json Not sure where this came from. * Emit log messages with the correct tag * Add another log tag, and lint * Move viewModelScope.launch in to changeRelationshop() --- .../components/account/AccountViewModel.kt | 77 +++++++++++-------- .../components/report/ReportViewModel.kt | 68 ++++++++-------- .../components/search/SearchViewModel.kt | 9 ++- .../tusky/fragment/AccountListFragment.kt | 48 ++++++------ .../keylesspalace/tusky/fragment/SFragment.kt | 10 ++- .../tusky/network/MastodonApi.kt | 32 ++++---- .../tusky/usecase/TimelineCases.kt | 43 +++++------ 7 files changed, 145 insertions(+), 142 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt index 664651eb..1b4aa7f0 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt @@ -2,6 +2,7 @@ package com.keylesspalace.tusky.components.account import android.util.Log import androidx.lifecycle.MutableLiveData +import androidx.lifecycle.viewModelScope import com.keylesspalace.tusky.appstore.BlockEvent import com.keylesspalace.tusky.appstore.DomainMuteEvent import com.keylesspalace.tusky.appstore.EventHub @@ -19,6 +20,7 @@ import com.keylesspalace.tusky.util.RxAwareViewModel import com.keylesspalace.tusky.util.Success import io.reactivex.rxjava3.core.Single import io.reactivex.rxjava3.disposables.Disposable +import kotlinx.coroutines.launch import retrofit2.Call import retrofit2.Callback import retrofit2.Response @@ -181,7 +183,11 @@ class AccountViewModel @Inject constructor( /** * @param parameter showReblogs if RelationShipAction.FOLLOW, notifications if MUTE */ - private fun changeRelationship(relationshipAction: RelationShipAction, parameter: Boolean? = null, duration: Int? = null) { + private fun changeRelationship( + relationshipAction: RelationShipAction, + parameter: Boolean? = null, + duration: Int? = null + ) = viewModelScope.launch { val relation = relationshipData.value?.data val account = accountData.value?.data val isMastodon = relationshipData.value?.data?.notifying != null @@ -216,40 +222,45 @@ class AccountViewModel @Inject constructor( relationshipData.postValue(Loading(newRelation)) } - when (relationshipAction) { - RelationShipAction.FOLLOW -> mastodonApi.followAccount(accountId, showReblogs = parameter ?: true) - RelationShipAction.UNFOLLOW -> mastodonApi.unfollowAccount(accountId) - RelationShipAction.BLOCK -> mastodonApi.blockAccount(accountId) - RelationShipAction.UNBLOCK -> mastodonApi.unblockAccount(accountId) - RelationShipAction.MUTE -> mastodonApi.muteAccount(accountId, parameter ?: true, duration) - RelationShipAction.UNMUTE -> mastodonApi.unmuteAccount(accountId) - RelationShipAction.SUBSCRIBE -> { - if (isMastodon) - mastodonApi.followAccount(accountId, notify = true) - else mastodonApi.subscribeAccount(accountId) - } - RelationShipAction.UNSUBSCRIBE -> { - if (isMastodon) - mastodonApi.followAccount(accountId, notify = false) - else mastodonApi.unsubscribeAccount(accountId) - } - }.subscribe( - { relationship -> - relationshipData.postValue(Success(relationship)) - - when (relationshipAction) { - RelationShipAction.UNFOLLOW -> eventHub.dispatch(UnfollowEvent(accountId)) - RelationShipAction.BLOCK -> eventHub.dispatch(BlockEvent(accountId)) - RelationShipAction.MUTE -> eventHub.dispatch(MuteEvent(accountId)) - else -> { - } + try { + val relationship = when (relationshipAction) { + RelationShipAction.FOLLOW -> mastodonApi.followAccount( + accountId, + showReblogs = parameter ?: true + ) + RelationShipAction.UNFOLLOW -> mastodonApi.unfollowAccount(accountId) + RelationShipAction.BLOCK -> mastodonApi.blockAccount(accountId) + RelationShipAction.UNBLOCK -> mastodonApi.unblockAccount(accountId) + RelationShipAction.MUTE -> mastodonApi.muteAccount( + accountId, + parameter ?: true, + duration + ) + RelationShipAction.UNMUTE -> mastodonApi.unmuteAccount(accountId) + RelationShipAction.SUBSCRIBE -> { + if (isMastodon) + mastodonApi.followAccount(accountId, notify = true) + else mastodonApi.subscribeAccount(accountId) + } + RelationShipAction.UNSUBSCRIBE -> { + if (isMastodon) + mastodonApi.followAccount(accountId, notify = false) + else mastodonApi.unsubscribeAccount(accountId) } - }, - { - relationshipData.postValue(Error(relation)) } - ) - .autoDispose() + + relationshipData.postValue(Success(relationship)) + + when (relationshipAction) { + RelationShipAction.UNFOLLOW -> eventHub.dispatch(UnfollowEvent(accountId)) + RelationShipAction.BLOCK -> eventHub.dispatch(BlockEvent(accountId)) + RelationShipAction.MUTE -> eventHub.dispatch(MuteEvent(accountId)) + else -> { + } + } + } catch (_: Throwable) { + relationshipData.postValue(Error(relation)) + } } fun noteChanged(newNote: String) { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt index 9f99da53..fe9215a4 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt @@ -154,52 +154,46 @@ class ReportViewModel @Inject constructor( fun toggleMute() { val alreadyMuted = muteStateMutable.value?.data == true - if (alreadyMuted) { - mastodonApi.unmuteAccount(accountId) - } else { - mastodonApi.muteAccount(accountId) - } - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe( - { relationship -> - val muting = relationship.muting - muteStateMutable.value = Success(muting) - if (muting) { - eventHub.dispatch(MuteEvent(accountId)) - } - }, - { error -> - muteStateMutable.value = Error(false, error.message) + viewModelScope.launch { + try { + val relationship = if (alreadyMuted) { + mastodonApi.unmuteAccount(accountId) + } else { + mastodonApi.muteAccount(accountId) } - ).autoDispose() + + val muting = relationship.muting + muteStateMutable.value = Success(muting) + if (muting) { + eventHub.dispatch(MuteEvent(accountId)) + } + } catch (t: Throwable) { + muteStateMutable.value = Error(false, t.message) + } + } muteStateMutable.value = Loading() } fun toggleBlock() { val alreadyBlocked = blockStateMutable.value?.data == true - if (alreadyBlocked) { - mastodonApi.unblockAccount(accountId) - } else { - mastodonApi.blockAccount(accountId) - } - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe( - { relationship -> - val blocking = relationship.blocking - blockStateMutable.value = Success(blocking) - if (blocking) { - eventHub.dispatch(BlockEvent(accountId)) - } - }, - { error -> - blockStateMutable.value = Error(false, error.message) + viewModelScope.launch { + try { + val relationship = if (alreadyBlocked) { + mastodonApi.unblockAccount(accountId) + } else { + mastodonApi.blockAccount(accountId) } - ) - .autoDispose() + val blocking = relationship.blocking + blockStateMutable.value = Success(blocking) + if (blocking) { + eventHub.dispatch(BlockEvent(accountId)) + } + } catch (t: Throwable) { + blockStateMutable.value = Error(false, t.message) + } + } blockStateMutable.value = Loading() } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt index 84a8b032..8207f83b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt @@ -32,6 +32,7 @@ import com.keylesspalace.tusky.util.toViewData import com.keylesspalace.tusky.viewdata.StatusViewData import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import io.reactivex.rxjava3.core.Single +import kotlinx.coroutines.launch import javax.inject.Inject class SearchViewModel @Inject constructor( @@ -169,7 +170,9 @@ class SearchViewModel @Inject constructor( } fun muteAccount(accountId: String, notifications: Boolean, duration: Int?) { - timelineCases.mute(accountId, notifications, duration) + viewModelScope.launch { + timelineCases.mute(accountId, notifications, duration) + } } fun pinAccount(status: Status, isPin: Boolean) { @@ -177,7 +180,9 @@ class SearchViewModel @Inject constructor( } fun blockAccount(accountId: String) { - timelineCases.block(accountId) + viewModelScope.launch { + timelineCases.block(accountId) + } } fun deleteStatus(id: String): Single { diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/AccountListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/AccountListFragment.kt index 9e57d2dc..19128eb7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/AccountListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/AccountListFragment.kt @@ -133,20 +133,18 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct } override fun onMute(mute: Boolean, id: String, position: Int, notifications: Boolean) { - if (!mute) { - api.unmuteAccount(id) - } else { - api.muteAccount(id, notifications) - } - .autoDispose(from(this)) - .subscribe( - { - onMuteSuccess(mute, id, position, notifications) - }, - { - onMuteFailure(mute, id, notifications) + lifecycleScope.launch { + try { + if (!mute) { + api.unmuteAccount(id) + } else { + api.muteAccount(id, notifications) } - ) + onMuteSuccess(mute, id, position, notifications) + } catch (_: Throwable) { + onMuteFailure(mute, id, notifications) + } + } } private fun onMuteSuccess(muted: Boolean, id: String, position: Int, notifications: Boolean) { @@ -181,20 +179,18 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct } override fun onBlock(block: Boolean, id: String, position: Int) { - if (!block) { - api.unblockAccount(id) - } else { - api.blockAccount(id) - } - .autoDispose(from(this)) - .subscribe( - { - onBlockSuccess(block, id, position) - }, - { - onBlockFailure(block, id) + lifecycleScope.launch { + try { + if (!block) { + api.unblockAccount(id) + } else { + api.blockAccount(id) } - ) + onBlockSuccess(block, id, position) + } catch (_: Throwable) { + onBlockFailure(block, id) + } + } } private fun onBlockSuccess(blocked: Boolean, id: String, position: Int) { diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt index 62d18e57..9b217f84 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt @@ -62,8 +62,6 @@ import com.keylesspalace.tusky.view.showMuteAccountDialog import com.keylesspalace.tusky.viewdata.AttachmentViewData import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import kotlinx.coroutines.launch -import java.lang.IllegalStateException -import java.util.LinkedHashSet import javax.inject.Inject /* Note from Andrew on Jan. 22, 2017: This class is a design problem for me, so I left it with an @@ -311,7 +309,9 @@ abstract class SFragment : Fragment(), Injectable { private fun onMute(accountId: String, accountUsername: String) { showMuteAccountDialog(this.requireActivity(), accountUsername) { notifications: Boolean?, duration: Int? -> - timelineCases.mute(accountId, notifications == true, duration) + lifecycleScope.launch { + timelineCases.mute(accountId, notifications == true, duration) + } } } @@ -319,7 +319,9 @@ abstract class SFragment : Fragment(), Injectable { AlertDialog.Builder(requireContext()) .setMessage(getString(R.string.dialog_block_warning, accountUsername)) .setPositiveButton(android.R.string.ok) { _: DialogInterface?, _: Int -> - timelineCases.block(accountId) + lifecycleScope.launch { + timelineCases.block(accountId) + } } .setNegativeButton(android.R.string.cancel, null) .show() diff --git a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt index fb0f7d80..2279778b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt +++ b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt @@ -359,39 +359,39 @@ interface MastodonApi { @FormUrlEncoded @POST("api/v1/accounts/{id}/follow") - fun followAccount( + suspend fun followAccount( @Path("id") accountId: String, @Field("reblogs") showReblogs: Boolean? = null, @Field("notify") notify: Boolean? = null - ): Single + ): Relationship @POST("api/v1/accounts/{id}/unfollow") - fun unfollowAccount( + suspend fun unfollowAccount( @Path("id") accountId: String - ): Single + ): Relationship @POST("api/v1/accounts/{id}/block") - fun blockAccount( + suspend fun blockAccount( @Path("id") accountId: String - ): Single + ): Relationship @POST("api/v1/accounts/{id}/unblock") - fun unblockAccount( + suspend fun unblockAccount( @Path("id") accountId: String - ): Single + ): Relationship @FormUrlEncoded @POST("api/v1/accounts/{id}/mute") - fun muteAccount( + suspend fun muteAccount( @Path("id") accountId: String, @Field("notifications") notifications: Boolean? = null, @Field("duration") duration: Int? = null - ): Single + ): Relationship @POST("api/v1/accounts/{id}/unmute") - fun unmuteAccount( + suspend fun unmuteAccount( @Path("id") accountId: String - ): Single + ): Relationship @GET("api/v1/accounts/relationships") fun relationships( @@ -399,14 +399,14 @@ interface MastodonApi { ): Single> @POST("api/v1/pleroma/accounts/{id}/subscribe") - fun subscribeAccount( + suspend fun subscribeAccount( @Path("id") accountId: String - ): Single + ): Relationship @POST("api/v1/pleroma/accounts/{id}/unsubscribe") - fun unsubscribeAccount( + suspend fun unsubscribeAccount( @Path("id") accountId: String - ): Single + ): Relationship @GET("api/v1/blocks") suspend fun blocks( diff --git a/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt b/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt index 570e1e39..3c527898 100644 --- a/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt +++ b/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt @@ -33,7 +33,6 @@ import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.util.getServerErrorMessage import io.reactivex.rxjava3.core.Single import io.reactivex.rxjava3.disposables.CompositeDisposable -import io.reactivex.rxjava3.kotlin.addTo import javax.inject.Inject /** @@ -95,30 +94,22 @@ class TimelineCases @Inject constructor( } } - fun mute(statusId: String, notifications: Boolean, duration: Int?) { - mastodonApi.muteAccount(statusId, notifications, duration) - .subscribe( - { - eventHub.dispatch(MuteEvent(statusId)) - }, - { t -> - Log.w("Failed to mute account", t) - } - ) - .addTo(cancelDisposable) + suspend fun mute(statusId: String, notifications: Boolean, duration: Int?) { + try { + mastodonApi.muteAccount(statusId, notifications, duration) + eventHub.dispatch(MuteEvent(statusId)) + } catch (t: Throwable) { + Log.w(TAG, "Failed to mute account", t) + } } - fun block(statusId: String) { - mastodonApi.blockAccount(statusId) - .subscribe( - { - eventHub.dispatch(BlockEvent(statusId)) - }, - { t -> - Log.w("Failed to block account", t) - } - ) - .addTo(cancelDisposable) + suspend fun block(statusId: String) { + try { + mastodonApi.blockAccount(statusId) + eventHub.dispatch(BlockEvent(statusId)) + } catch (t: Throwable) { + Log.w(TAG, "Failed to block account", t) + } } fun delete(statusId: String): Single { @@ -132,7 +123,7 @@ class TimelineCases @Inject constructor( // Replace with extension method if we use RxKotlin return (if (pin) mastodonApi.pinStatus(statusId) else mastodonApi.unpinStatus(statusId)) .doOnError { e -> - Log.w("Failed to change pin state", e) + Log.w(TAG, "Failed to change pin state", e) } .onErrorResumeNext(::convertError) .doAfterSuccess { @@ -153,6 +144,10 @@ class TimelineCases @Inject constructor( private fun convertError(e: Throwable): Single { return Single.error(TimelineError(e.getServerErrorMessage())) } + + companion object { + private const val TAG = "TimelineCases" + } } class TimelineError(message: String?) : RuntimeException(message)