Skip to content

Commit

Permalink
refactor: Replace the different network response types with ApiResult (
Browse files Browse the repository at this point in the history
…#1261)

Previous code used five (!) different types for the network response.

Some used Retrofit's `Response`. This provides access to the headers.

Some used `NetworkResult`. This did not provide access to the headers,
but did provide some higher-order functions (e.g., `fold`) for operating
on the results.

One used a raw `Map`.

One used a raw `Call`.

The rest had been converted to `ApiResult`, a `Result<V, ApiError>`.
This provides the higher-order functions, provides the headers, and
is exception-free, so is the correct type to use.

This PR completes the work of cutting over to `ApiResult`. The return
values are changed and the calling code is adjusted to use the new
functions as appropriate.
  • Loading branch information
nikclayton authored Feb 5, 2025
1 parent f5d42c8 commit 91d577c
Show file tree
Hide file tree
Showing 124 changed files with 1,063 additions and 1,282 deletions.
4 changes: 2 additions & 2 deletions app/src/fdroid/kotlin/app/pachli/di/UpdateCheckModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

package app.pachli.di

import app.pachli.core.network.retrofit.apiresult.ApiResultCallAdapterFactory
import app.pachli.updatecheck.FdroidService
import at.connyduck.calladapter.networkresult.NetworkResultCallAdapterFactory
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
Expand All @@ -40,7 +40,7 @@ object UpdateCheckModule {
.baseUrl("https://f-droid.org")
.client(httpClient)
.addConverterFactory(MoshiConverterFactory.create())
.addCallAdapterFactory(NetworkResultCallAdapterFactory.create())
.addCallAdapterFactory(ApiResultCallAdapterFactory.create())
.build()
.create()
}
4 changes: 2 additions & 2 deletions app/src/fdroid/kotlin/app/pachli/updatecheck/FdroidService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package app.pachli.updatecheck

import androidx.annotation.Keep
import at.connyduck.calladapter.networkresult.NetworkResult
import app.pachli.core.network.retrofit.apiresult.ApiResult
import com.squareup.moshi.JsonClass
import retrofit2.http.GET
import retrofit2.http.Path
Expand All @@ -42,5 +42,5 @@ interface FdroidService {
@GET("/api/v1/packages/{package}")
suspend fun getPackage(
@Path("package") pkg: String,
): NetworkResult<FdroidPackage>
): ApiResult<FdroidPackage>
}
3 changes: 2 additions & 1 deletion app/src/fdroid/kotlin/app/pachli/updatecheck/UpdateCheck.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import android.content.Intent
import android.net.Uri
import app.pachli.BuildConfig
import app.pachli.core.preferences.SharedPreferencesRepository
import com.github.michaelbull.result.get
import javax.inject.Inject
import javax.inject.Singleton

Expand All @@ -34,7 +35,7 @@ class UpdateCheck @Inject constructor(
}

override suspend fun remoteFetchLatestVersionCode(): Int? {
val fdroidPackage = fdroidService.getPackage(BuildConfig.APPLICATION_ID).getOrNull() ?: return null
val fdroidPackage = fdroidService.getPackage(BuildConfig.APPLICATION_ID).get()?.body ?: return null

// `packages` is a list of all packages that have been built and are available.
//
Expand Down
4 changes: 2 additions & 2 deletions app/src/github/kotlin/app/pachli/di/UpdateCheckModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

package app.pachli.di

import app.pachli.core.network.retrofit.apiresult.ApiResultCallAdapterFactory
import app.pachli.updatecheck.GitHubService
import at.connyduck.calladapter.networkresult.NetworkResultCallAdapterFactory
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
Expand All @@ -40,7 +40,7 @@ object UpdateCheckModule {
.baseUrl("https://api.github.com")
.client(httpClient)
.addConverterFactory(MoshiConverterFactory.create())
.addCallAdapterFactory(NetworkResultCallAdapterFactory.create())
.addCallAdapterFactory(ApiResultCallAdapterFactory.create())
.build()
.create()
}
4 changes: 2 additions & 2 deletions app/src/github/kotlin/app/pachli/updatecheck/GithubService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package app.pachli.updatecheck

import androidx.annotation.Keep
import at.connyduck.calladapter.networkresult.NetworkResult
import app.pachli.core.network.retrofit.apiresult.ApiResult
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
import retrofit2.http.GET
Expand Down Expand Up @@ -47,5 +47,5 @@ interface GitHubService {
suspend fun getLatestRelease(
@Path("owner") owner: String,
@Path("repo") repo: String,
): NetworkResult<GitHubRelease>
): ApiResult<GitHubRelease>
}
3 changes: 2 additions & 1 deletion app/src/github/kotlin/app/pachli/updatecheck/UpdateCheck.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package app.pachli.updatecheck
import android.content.Intent
import android.net.Uri
import app.pachli.core.preferences.SharedPreferencesRepository
import com.github.michaelbull.result.get
import javax.inject.Inject

class UpdateCheck @Inject constructor(
Expand All @@ -33,7 +34,7 @@ class UpdateCheck @Inject constructor(
}

override suspend fun remoteFetchLatestVersionCode(): Int? {
val release = gitHubService.getLatestRelease("pachli", "pachli-android").getOrNull() ?: return null
val release = gitHubService.getLatestRelease("pachli", "pachli-android").get()?.body ?: return null
for (asset in release.assets) {
if (asset.contentType != "application/vnd.android.package-archive") continue
return versionCodeExtractor.find(asset.name)?.groups?.get(1)?.value?.toIntOrNull() ?: continue
Expand Down
63 changes: 27 additions & 36 deletions app/src/main/java/app/pachli/TimelineActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import app.pachli.core.navigation.pachliAccountId
import app.pachli.databinding.ActivityTimelineBinding
import app.pachli.interfaces.ActionButtonActivity
import app.pachli.interfaces.AppBarLayoutHost
import at.connyduck.calladapter.networkresult.fold
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import com.google.android.material.appbar.AppBarLayout
Expand Down Expand Up @@ -135,21 +134,19 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc

hashtag?.let { tag ->
lifecycleScope.launch {
mastodonApi.tag(tag).fold(
{ tagEntity ->
menuInflater.inflate(R.menu.view_hashtag_toolbar, menu)
followTagItem = menu.findItem(R.id.action_follow_hashtag)
unfollowTagItem = menu.findItem(R.id.action_unfollow_hashtag)
muteTagItem = menu.findItem(R.id.action_mute_hashtag)
unmuteTagItem = menu.findItem(R.id.action_unmute_hashtag)
followTagItem?.isVisible = tagEntity.following == false
unfollowTagItem?.isVisible = tagEntity.following == true
updateMuteTagMenuItems(tag)
},
{
Timber.w(it, "Failed to query tag #%s", tag)
},
)
mastodonApi.tag(tag).onSuccess {
val tagEntity = it.body
menuInflater.inflate(R.menu.view_hashtag_toolbar, menu)
followTagItem = menu.findItem(R.id.action_follow_hashtag)
unfollowTagItem = menu.findItem(R.id.action_unfollow_hashtag)
muteTagItem = menu.findItem(R.id.action_mute_hashtag)
unmuteTagItem = menu.findItem(R.id.action_unmute_hashtag)
followTagItem?.isVisible = tagEntity.following == false
unfollowTagItem?.isVisible = tagEntity.following == true
updateMuteTagMenuItems(tag)
}.onFailure {
Timber.w("Failed to query tag #%s: %s", tag, it)
}
}
}

Expand Down Expand Up @@ -205,16 +202,13 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc
val tag = hashtag
if (tag != null) {
lifecycleScope.launch {
mastodonApi.followTag(tag).fold(
{
followTagItem?.isVisible = false
unfollowTagItem?.isVisible = true
},
{
Snackbar.make(binding.root, getString(R.string.error_following_hashtag_format, tag), Snackbar.LENGTH_SHORT).show()
Timber.e(it, "Failed to follow #%s", tag)
},
)
mastodonApi.followTag(tag).onSuccess {
followTagItem?.isVisible = false
unfollowTagItem?.isVisible = true
}.onFailure {
Snackbar.make(binding.root, getString(R.string.error_following_hashtag_format, tag), Snackbar.LENGTH_SHORT).show()
Timber.e("Failed to follow #%s: %s", tag, it)
}
}
}

Expand All @@ -225,16 +219,13 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc
val tag = hashtag
if (tag != null) {
lifecycleScope.launch {
mastodonApi.unfollowTag(tag).fold(
{
followTagItem?.isVisible = true
unfollowTagItem?.isVisible = false
},
{
Snackbar.make(binding.root, getString(R.string.error_unfollowing_hashtag_format, tag), Snackbar.LENGTH_SHORT).show()
Timber.e(it, "Failed to unfollow #%s", tag)
},
)
mastodonApi.unfollowTag(tag).onSuccess {
followTagItem?.isVisible = true
unfollowTagItem?.isVisible = false
}.onFailure {
Snackbar.make(binding.root, getString(R.string.error_unfollowing_hashtag_format, tag), Snackbar.LENGTH_SHORT).show()
Timber.e("Failed to unfollow #%s: %s", tag, it)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class AccountViewModel @AssistedInject constructor(

isFromOwnDomain = domain == activeAccount.domain
}
.onFailure { t ->
Timber.w("failed obtaining account: %s", t)
accountData.postValue(Error(cause = t.throwable))
.onFailure {
Timber.w("failed obtaining account: %s", it)
accountData.postValue(Error(cause = it.throwable))
isDataLoading = false
isRefreshing.postValue(false)
}
Expand All @@ -100,13 +100,13 @@ class AccountViewModel @AssistedInject constructor(

viewModelScope.launch {
mastodonApi.relationships(listOf(accountId))
.onSuccess { response ->
val relationships = response.body
.onSuccess {
val relationships = it.body
relationshipData.postValue(if (relationships.isNotEmpty()) Success(relationships[0]) else Error())
}
.onFailure { t ->
Timber.w("failed obtaining relationships: %s", t)
relationshipData.postValue(Error(cause = t.throwable))
.onFailure {
Timber.w("failed obtaining relationships: %s", it)
relationshipData.postValue(Error(cause = it.throwable))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import androidx.paging.RemoteMediator
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.navigation.AttachmentViewData
import app.pachli.core.network.retrofit.MastodonApi
import com.github.michaelbull.result.getOrElse
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.ensureActive
import retrofit2.HttpException

@OptIn(ExperimentalPagingApi::class)
class AccountMediaRemoteMediator(
Expand Down Expand Up @@ -53,13 +53,9 @@ class AccountMediaRemoteMediator(
return MediatorResult.Success(endOfPaginationReached = false)
}
}
}

val statuses = statusResponse.body()
if (!statusResponse.isSuccessful || statuses == null) {
return MediatorResult.Error(HttpException(statusResponse))
}
}.getOrElse { return MediatorResult.Error(it.throwable) }

val statuses = statusResponse.body
val attachments = statuses.flatMap { status ->
AttachmentViewData.list(status, activeAccount.alwaysShowSensitiveMedia)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ import app.pachli.databinding.FragmentAccountListBinding
import app.pachli.interfaces.AccountActionListener
import app.pachli.interfaces.AppBarLayoutHost
import app.pachli.view.EndlessOnScrollListener
import at.connyduck.calladapter.networkresult.fold
import com.github.michaelbull.result.getOrElse
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
Expand Down Expand Up @@ -273,19 +272,12 @@ class AccountListFragment :
api.authorizeFollowRequest(accountId)
} else {
api.rejectFollowRequest(accountId)
}.fold(
{
onRespondToFollowRequestSuccess(position)
},
{ throwable ->
val verb = if (accept) {
"accept"
} else {
"reject"
}
Timber.e(throwable, "Failed to %s accountId %s", verb, accountId)
},
)
}.onSuccess {
onRespondToFollowRequestSuccess(position)
}.onFailure { error ->
val verb = if (accept) "accept" else "reject"
Timber.e("Failed to %s accountId %s: %s", verb, accountId, error.fmt(requireContext()))
}
}
}

Expand Down
Loading

0 comments on commit 91d577c

Please sign in to comment.