fix: Don't crash due to Filters/ServerRepository race condition (#837)

The `canFilter()` implementation could crash if `server` (marked
`lateinit`) hadn't been initialised at the point of use.

Fix this by removing it and adjusting the two callers to use the
`filters` flow and take appropriate action on error.
This commit is contained in:
Nik Clayton 2024-07-25 18:43:34 +02:00 committed by GitHub
parent 01831474dc
commit b1d5cb548f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 47 additions and 20 deletions

View File

@ -33,6 +33,7 @@ import app.pachli.core.common.util.unsafeLazy
import app.pachli.core.data.model.Filter import app.pachli.core.data.model.Filter
import app.pachli.core.data.model.NewFilterKeyword import app.pachli.core.data.model.NewFilterKeyword
import app.pachli.core.data.repository.FilterEdit import app.pachli.core.data.repository.FilterEdit
import app.pachli.core.data.repository.FiltersError
import app.pachli.core.data.repository.FiltersRepository import app.pachli.core.data.repository.FiltersRepository
import app.pachli.core.data.repository.NewFilter import app.pachli.core.data.repository.NewFilter
import app.pachli.core.model.Timeline import app.pachli.core.model.Timeline
@ -132,7 +133,7 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc
unmuteTagItem = menu.findItem(R.id.action_unmute_hashtag) unmuteTagItem = menu.findItem(R.id.action_unmute_hashtag)
followTagItem?.isVisible = tagEntity.following == false followTagItem?.isVisible = tagEntity.following == false
unfollowTagItem?.isVisible = tagEntity.following == true unfollowTagItem?.isVisible = tagEntity.following == true
updateMuteTagMenuItems() updateMuteTagMenuItems(tag)
}, },
{ {
Timber.w(it, "Failed to query tag #%s", tag) Timber.w(it, "Failed to query tag #%s", tag)
@ -230,18 +231,10 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc
} }
/** /**
* Determine if the current hashtag is muted, and update the UI state accordingly. * Determine if the given hashtag is muted, and update the UI state accordingly.
*/ */
private fun updateMuteTagMenuItems() { private fun updateMuteTagMenuItems(tag: String) {
val tagWithHash = hashtag?.let { "#$it" } ?: return val tagWithHash = "#$tag"
// If the server can't filter then it's impossible to mute hashtags, so disable
// the functionality.
if (!filtersRepository.canFilter()) {
muteTagItem?.isVisible = false
unmuteTagItem?.isVisible = false
return
}
muteTagItem?.isVisible = true muteTagItem?.isVisible = true
muteTagItem?.isEnabled = false muteTagItem?.isEnabled = false
@ -256,6 +249,14 @@ class TimelineActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButtonAc
} }
updateTagMuteState(mutedFilter != null) updateTagMuteState(mutedFilter != null)
} }
result.onFailure { error ->
// If the server can't filter then it's impossible to mute hashtags,
// so disable the functionality.
if (error is FiltersError.ServerDoesNotFilter) {
muteTagItem?.isVisible = false
unmuteTagItem?.isVisible = false
}
}
} }
} }
} }

View File

@ -19,7 +19,12 @@ package app.pachli.components.preference
import android.content.Intent import android.content.Intent
import android.os.Build import android.os.Build
import android.os.Bundle import android.os.Bundle
import android.view.View
import androidx.annotation.DrawableRes import androidx.annotation.DrawableRes
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.preference.Preference
import androidx.preference.PreferenceFragmentCompat import androidx.preference.PreferenceFragmentCompat
import app.pachli.BuildConfig import app.pachli.BuildConfig
import app.pachli.R import app.pachli.R
@ -55,11 +60,13 @@ import app.pachli.util.getInitialLanguages
import app.pachli.util.getLocaleList import app.pachli.util.getLocaleList
import app.pachli.util.getPachliDisplayName import app.pachli.util.getPachliDisplayName
import app.pachli.util.iconRes import app.pachli.util.iconRes
import com.github.michaelbull.result.Ok
import com.google.android.material.snackbar.Snackbar import com.google.android.material.snackbar.Snackbar
import com.mikepenz.iconics.IconicsDrawable import com.mikepenz.iconics.IconicsDrawable
import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial
import dagger.hilt.android.AndroidEntryPoint import dagger.hilt.android.AndroidEntryPoint
import javax.inject.Inject import javax.inject.Inject
import kotlinx.coroutines.launch
import retrofit2.Call import retrofit2.Call
import retrofit2.Callback import retrofit2.Callback
import retrofit2.Response import retrofit2.Response
@ -84,6 +91,28 @@ class AccountPreferencesFragment : PreferenceFragmentCompat() {
private val iconSize by unsafeLazy { resources.getDimensionPixelSize(DR.dimen.preference_icon_size) } private val iconSize by unsafeLazy { resources.getDimensionPixelSize(DR.dimen.preference_icon_size) }
/**
* The filter preference.
*
* Is enabled/disabled at runtime.
*/
private lateinit var filterPreference: Preference
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
// Enable/disable the filter preference based on info from
// FiltersRespository. filterPreferences is safe to access here,
// it was populated in onCreatePreferences, called by onCreate
// before onViewCreated is called.
filtersRepository.filters.collect { filters ->
filterPreference.isEnabled = filters is Ok
}
}
}
return super.onViewCreated(view, savedInstanceState)
}
override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
val context = requireContext() val context = requireContext()
makePreferenceScreen { makePreferenceScreen {
@ -158,7 +187,7 @@ class AccountPreferencesFragment : PreferenceFragmentCompat() {
} }
} }
preference { filterPreference = preference {
setTitle(R.string.pref_title_timeline_filters) setTitle(R.string.pref_title_timeline_filters)
setIcon(R.drawable.ic_filter_24dp) setIcon(R.drawable.ic_filter_24dp)
setOnPreferenceClickListener { setOnPreferenceClickListener {
@ -166,8 +195,9 @@ class AccountPreferencesFragment : PreferenceFragmentCompat() {
activity?.startActivityWithTransition(intent, TransitionKind.SLIDE_FROM_END) activity?.startActivityWithTransition(intent, TransitionKind.SLIDE_FROM_END)
true true
} }
isEnabled = filtersRepository.canFilter() setSummaryProvider {
if (!isEnabled) summary = context.getString(R.string.pref_summary_timeline_filters) if (it.isEnabled) "" else context.getString(R.string.pref_summary_timeline_filters)
}
} }
preferenceCategory(R.string.pref_publishing) { preferenceCategory(R.string.pref_publishing) {

View File

@ -40,7 +40,6 @@ import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.Result import com.github.michaelbull.result.Result
import com.github.michaelbull.result.andThen import com.github.michaelbull.result.andThen
import com.github.michaelbull.result.coroutines.binding.binding import com.github.michaelbull.result.coroutines.binding.binding
import com.github.michaelbull.result.get
import com.github.michaelbull.result.map import com.github.michaelbull.result.map
import com.github.michaelbull.result.mapError import com.github.michaelbull.result.mapError
import com.github.michaelbull.result.mapResult import com.github.michaelbull.result.mapResult
@ -171,7 +170,7 @@ data class Filters(
class FiltersRepository @Inject constructor( class FiltersRepository @Inject constructor(
@ApplicationScope private val externalScope: CoroutineScope, @ApplicationScope private val externalScope: CoroutineScope,
private val mastodonApi: MastodonApi, private val mastodonApi: MastodonApi,
private val serverRepository: ServerRepository, serverRepository: ServerRepository,
) { ) {
/** Flow where emissions trigger fresh loads from the server. */ /** Flow where emissions trigger fresh loads from the server. */
private val reload = MutableSharedFlow<Unit>(replay = 1).apply { tryEmit(Unit) } private val reload = MutableSharedFlow<Unit>(replay = 1).apply { tryEmit(Unit) }
@ -197,9 +196,6 @@ class FiltersRepository @Inject constructor(
suspend fun reload() = reload.emit(Unit) suspend fun reload() = reload.emit(Unit)
/** @return True if the user's server can filter, false otherwise. */
fun canFilter() = server.get()?.let { it.canFilterV1() || it.canFilterV2() } ?: false
/** Get a specific filter from the server, by [filterId]. */ /** Get a specific filter from the server, by [filterId]. */
suspend fun getFilter(filterId: String): Result<Filter, FiltersError> = binding { suspend fun getFilter(filterId: String): Result<Filter, FiltersError> = binding {
val server = server.bind() val server = server.bind()