From 561eda848206116ecea0bb33db41114b2aa9fe87 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Tue, 10 Jan 2023 21:20:00 +0100 Subject: [PATCH] Remove rxjava from deletestatus API (#3041) * 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. * Remove rxjava from the deleteStatus call path * Emit log messages with the correct tag * Add another log tag, and lint * Use TAG in log messages now it's present * Lint * Move viewModelScope.launch in to changeRelationshop() * Use onSuccess/onFailure pair instead of fold * Return Deferred when deleting statuses --- .../components/search/SearchViewModel.kt | 26 ++++++------ .../fragments/SearchStatusesFragment.kt | 13 ++---- .../keylesspalace/tusky/fragment/SFragment.kt | 42 +++++++++---------- .../tusky/network/MastodonApi.kt | 4 +- .../tusky/usecase/TimelineCases.kt | 10 +++-- 5 files changed, 45 insertions(+), 50 deletions(-) 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 8207f83b..2a8154b9 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 @@ -20,6 +20,7 @@ import androidx.lifecycle.viewModelScope import androidx.paging.Pager import androidx.paging.PagingConfig import androidx.paging.cachedIn +import at.connyduck.calladapter.networkresult.NetworkResult import com.keylesspalace.tusky.components.search.adapter.SearchPagingSourceFactory import com.keylesspalace.tusky.db.AccountEntity import com.keylesspalace.tusky.db.AccountManager @@ -31,7 +32,8 @@ import com.keylesspalace.tusky.util.RxAwareViewModel 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.Deferred +import kotlinx.coroutines.async import kotlinx.coroutines.launch import javax.inject.Inject @@ -99,17 +101,13 @@ class SearchViewModel @Inject constructor( } fun removeItem(statusViewData: StatusViewData.Concrete) { - timelineCases.delete(statusViewData.id) - .subscribe( - { - if (loadedStatuses.remove(statusViewData)) - statusesPagingSourceFactory.invalidate() - }, - { err -> - Log.d(TAG, "Failed to delete status", err) + viewModelScope.launch { + if (timelineCases.delete(statusViewData.id).isSuccess) { + if (loadedStatuses.remove(statusViewData)) { + statusesPagingSourceFactory.invalidate() } - ) - .autoDispose() + } + } } fun expandedChange(statusViewData: StatusViewData.Concrete, expanded: Boolean) { @@ -185,8 +183,10 @@ class SearchViewModel @Inject constructor( } } - fun deleteStatus(id: String): Single { - return timelineCases.delete(id) + fun deleteStatusAsync(id: String): Deferred> { + return viewModelScope.async { + timelineCases.delete(id) + } } fun muteConversation(statusViewData: StatusViewData.Concrete, mute: Boolean) { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt index 103bcfc5..9973f3e8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt @@ -32,7 +32,6 @@ import androidx.appcompat.app.AlertDialog import androidx.appcompat.widget.PopupMenu import androidx.core.app.ActivityOptionsCompat import androidx.core.view.ViewCompat -import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.paging.PagingData import androidx.paging.PagingDataAdapter @@ -40,8 +39,6 @@ import androidx.preference.PreferenceManager import androidx.recyclerview.widget.DividerItemDecoration import androidx.recyclerview.widget.LinearLayoutManager import at.connyduck.calladapter.networkresult.fold -import autodispose2.androidx.lifecycle.AndroidLifecycleScopeProvider.from -import autodispose2.autoDispose import com.google.android.material.snackbar.Snackbar import com.keylesspalace.tusky.BaseActivity import com.keylesspalace.tusky.R @@ -63,7 +60,6 @@ import com.keylesspalace.tusky.util.openLink import com.keylesspalace.tusky.view.showMuteAccountDialog import com.keylesspalace.tusky.viewdata.AttachmentViewData import com.keylesspalace.tusky.viewdata.StatusViewData -import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch @@ -445,7 +441,7 @@ class SearchStatusesFragment : SearchFragment(), Status AlertDialog.Builder(it) .setMessage(R.string.dialog_delete_post_warning) .setPositiveButton(android.R.string.ok) { _, _ -> - viewModel.deleteStatus(id) + viewModel.deleteStatusAsync(id) removeItem(position) } .setNegativeButton(android.R.string.cancel, null) @@ -458,10 +454,8 @@ class SearchStatusesFragment : SearchFragment(), Status AlertDialog.Builder(it) .setMessage(R.string.dialog_redraft_post_warning) .setPositiveButton(android.R.string.ok) { _, _ -> - viewModel.deleteStatus(id) - .observeOn(AndroidSchedulers.mainThread()) - .autoDispose(from(this, Lifecycle.Event.ON_DESTROY)) - .subscribe( + lifecycleScope.launch { + viewModel.deleteStatusAsync(id).await().fold( { deletedStatus -> removeItem(position) @@ -492,6 +486,7 @@ class SearchStatusesFragment : SearchFragment(), Status Toast.makeText(context, R.string.error_generic, Toast.LENGTH_SHORT).show() } ) + } } .setNegativeButton(android.R.string.cancel, null) .show() 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 32808cce..b4284b39 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt @@ -364,18 +364,19 @@ abstract class SFragment : Fragment(), Injectable { AlertDialog.Builder(requireActivity()) .setMessage(R.string.dialog_delete_post_warning) .setPositiveButton(android.R.string.ok) { _: DialogInterface?, _: Int -> - timelineCases.delete(id) - .observeOn(AndroidSchedulers.mainThread()) - .to( - AutoDispose.autoDisposable( - AndroidLifecycleScopeProvider.from(this, Lifecycle.Event.ON_DESTROY) - ) - ) - .subscribe({ }) { error: Throwable? -> - Log.w("SFragment", "error deleting status", error) + lifecycleScope.launch { + val result = timelineCases.delete(id).exceptionOrNull() + if (result != null) { + Log.w("SFragment", "error deleting status", result) Toast.makeText(context, R.string.error_generic, Toast.LENGTH_SHORT).show() } - removeItem(position) + // XXX: Removes the item even if there was an error. This is probably not + // correct (see similar code in showConfirmEditDialog() which only + // removes the item if the timelineCases.delete() call succeeded. + // + // Either way, this logic should be in the view model. + removeItem(position) + } } .setNegativeButton(android.R.string.cancel, null) .show() @@ -388,14 +389,8 @@ abstract class SFragment : Fragment(), Injectable { AlertDialog.Builder(requireActivity()) .setMessage(R.string.dialog_redraft_post_warning) .setPositiveButton(android.R.string.ok) { _: DialogInterface?, _: Int -> - timelineCases.delete(id) - .observeOn(AndroidSchedulers.mainThread()) - .to( - AutoDispose.autoDisposable( - AndroidLifecycleScopeProvider.from(this, Lifecycle.Event.ON_DESTROY) - ) - ) - .subscribe( + lifecycleScope.launch { + timelineCases.delete(id).fold( { deletedStatus -> removeItem(position) val sourceStatus = if (deletedStatus.isEmpty()) { @@ -416,11 +411,14 @@ abstract class SFragment : Fragment(), Injectable { kind = ComposeActivity.ComposeKind.NEW ) startActivity(startIntent(requireContext(), composeOptions)) + }, + { error: Throwable? -> + Log.w("SFragment", "error deleting status", error) + Toast.makeText(context, R.string.error_generic, Toast.LENGTH_SHORT) + .show() } - ) { error: Throwable? -> - Log.w("SFragment", "error deleting status", error) - Toast.makeText(context, R.string.error_generic, Toast.LENGTH_SHORT).show() - } + ) + } } .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 c4e0425d..d21d3a86 100644 --- a/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt +++ b/app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt @@ -213,9 +213,9 @@ interface MastodonApi { ): Response> @DELETE("api/v1/statuses/{id}") - fun deleteStatus( + suspend fun deleteStatus( @Path("id") statusId: String - ): Single + ): NetworkResult @POST("api/v1/statuses/{id}/reblog") fun reblogStatus( 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 3c527898..45842f8e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt +++ b/app/src/main/java/com/keylesspalace/tusky/usecase/TimelineCases.kt @@ -16,6 +16,9 @@ package com.keylesspalace.tusky.usecase import android.util.Log +import at.connyduck.calladapter.networkresult.NetworkResult +import at.connyduck.calladapter.networkresult.onFailure +import at.connyduck.calladapter.networkresult.onSuccess import com.keylesspalace.tusky.appstore.BlockEvent import com.keylesspalace.tusky.appstore.BookmarkEvent import com.keylesspalace.tusky.appstore.EventHub @@ -112,11 +115,10 @@ class TimelineCases @Inject constructor( } } - fun delete(statusId: String): Single { + suspend fun delete(statusId: String): NetworkResult { return mastodonApi.deleteStatus(statusId) - .doAfterSuccess { - eventHub.dispatch(StatusDeletedEvent(statusId)) - } + .onSuccess { eventHub.dispatch(StatusDeletedEvent(statusId)) } + .onFailure { Log.w(TAG, "Failed to delete status", it) } } fun pin(statusId: String, pin: Boolean): Single {