From 9a23439d04e6f257cb2db4ff33123c563906c8cf Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Wed, 28 Feb 2024 19:09:54 +0100 Subject: [PATCH] refactor: Remove duplicate strings from Filter contexts (#478) A filter's context (previously referred to as its `kind`) controls where the filter is applied. This was implemented as an enum with a specific property to control how it would serialise when @FormUrlEncoded, and with a @Json annotation for Moshi. In addition, the model objects kept the filter context in its string form throughout Pachli, requiring periodic conversion to/from the enum type, making the code more complicated. Fix this, by: 1. Converting the incoming JSON value to the enum type immediately, so the rest of the code uses the enum constants exclusively. 2. Implement a Retrofit converter that serialises the enum value when @FormUrlEncoded to the same string used in JSON serialisation --- .../java/app/pachli/StatusListActivity.kt | 23 ++++--- .../main/java/app/pachli/appstore/Events.kt | 4 +- .../components/filters/EditFilterActivity.kt | 23 ++++--- .../components/filters/EditFilterViewModel.kt | 37 +++++----- .../components/filters/FiltersAdapter.kt | 4 +- .../components/filters/FiltersViewModel.kt | 8 +-- .../notifications/NotificationsViewModel.kt | 7 +- .../timeline/viewmodel/TimelineViewModel.kt | 13 ++-- .../viewmodel/TrendingTagsViewModel.kt | 4 +- .../viewthread/ViewThreadViewModel.kt | 7 +- .../java/app/pachli/network/FilterModel.kt | 9 +-- app/src/test/java/app/pachli/FilterV1Test.kt | 17 ++--- .../pachli/core/network/di/NetworkModule.kt | 2 + .../json/EnumConstantConverterFactory.kt | 54 +++++++++++++++ .../app/pachli/core/network/model/Filter.kt | 46 ++----------- .../core/network/model/FilterContext.kt | 68 +++++++++++++++++++ .../app/pachli/core/network/model/FilterV1.kt | 14 +--- .../core/network/retrofit/MastodonApi.kt | 9 +-- .../json/EnumConstantConverterFactoryTest.kt | 26 +++++++ 19 files changed, 246 insertions(+), 129 deletions(-) create mode 100644 core/network/src/main/kotlin/app/pachli/core/network/json/EnumConstantConverterFactory.kt create mode 100644 core/network/src/main/kotlin/app/pachli/core/network/model/FilterContext.kt create mode 100644 core/network/src/test/kotlin/app/pachli/core/network/json/EnumConstantConverterFactoryTest.kt diff --git a/app/src/main/java/app/pachli/StatusListActivity.kt b/app/src/main/java/app/pachli/StatusListActivity.kt index b755c3261..a7baeaaee 100644 --- a/app/src/main/java/app/pachli/StatusListActivity.kt +++ b/app/src/main/java/app/pachli/StatusListActivity.kt @@ -33,6 +33,7 @@ import app.pachli.core.navigation.StatusListActivityIntent import app.pachli.core.network.ServerOperation.ORG_JOINMASTODON_FILTERS_CLIENT import app.pachli.core.network.ServerOperation.ORG_JOINMASTODON_FILTERS_SERVER import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterV1 import app.pachli.core.network.model.TimelineKind import app.pachli.databinding.ActivityStatuslistBinding @@ -243,7 +244,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton mastodonApi.getFilters().fold( { filters -> mutedFilter = filters.firstOrNull { filter -> - filter.context.contains(Filter.Kind.HOME.kind) && filter.keywords.any { + filter.contexts.contains(FilterContext.HOME) && filter.keywords.any { it.keyword == tagWithHash } } @@ -254,7 +255,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton mastodonApi.getFiltersV1().fold( { filters -> mutedFilterV1 = filters.firstOrNull { filter -> - tagWithHash == filter.phrase && filter.context.contains(FilterV1.HOME) + tagWithHash == filter.phrase && filter.contexts.contains(FilterContext.HOME) } updateTagMuteState(mutedFilterV1 != null) }, @@ -288,7 +289,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton lifecycleScope.launch { mastodonApi.createFilter( title = tagWithHash, - context = listOf(FilterV1.HOME), + context = listOf(FilterContext.HOME), filterAction = Filter.Action.WARN.action, expiresInSeconds = null, ).fold( @@ -296,7 +297,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton if (mastodonApi.addFilterKeyword(filterId = filter.id, keyword = tagWithHash, wholeWord = true).isSuccess) { mutedFilter = filter updateTagMuteState(true) - eventHub.dispatch(FilterChangedEvent(Filter.Kind.from(filter.context[0]))) + eventHub.dispatch(FilterChangedEvent(filter.contexts[0])) Snackbar.make(binding.root, getString(R.string.confirmation_hashtag_muted, hashtag), Snackbar.LENGTH_SHORT).show() } else { Snackbar.make(binding.root, getString(R.string.error_muting_hashtag_format, hashtag), Snackbar.LENGTH_SHORT).show() @@ -307,7 +308,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton if (throwable is HttpException && throwable.code() == 404) { mastodonApi.createFilterV1( tagWithHash, - listOf(FilterV1.HOME), + listOf(FilterContext.HOME), irreversible = false, wholeWord = true, expiresInSeconds = null, @@ -315,7 +316,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton { filter -> mutedFilterV1 = filter updateTagMuteState(true) - eventHub.dispatch(FilterChangedEvent(Filter.Kind.from(filter.context[0]))) + eventHub.dispatch(FilterChangedEvent(filter.contexts[0])) Snackbar.make(binding.root, getString(R.string.confirmation_hashtag_muted, hashtag), Snackbar.LENGTH_SHORT).show() }, { throwable -> @@ -340,23 +341,23 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton val result = if (mutedFilter != null) { val filter = mutedFilter!! - if (filter.context.size > 1) { + if (filter.contexts.size > 1) { // This filter exists in multiple contexts, just remove the home context mastodonApi.updateFilter( id = filter.id, - context = filter.context.filter { it != Filter.Kind.HOME.kind }, + context = filter.contexts.filter { it != FilterContext.HOME }, ) } else { mastodonApi.deleteFilter(filter.id) } } else if (mutedFilterV1 != null) { mutedFilterV1?.let { filter -> - if (filter.context.size > 1) { + if (filter.contexts.size > 1) { // This filter exists in multiple contexts, just remove the home context mastodonApi.updateFilterV1( id = filter.id, phrase = filter.phrase, - context = filter.context.filter { it != FilterV1.HOME }, + context = filter.contexts.filter { it != FilterContext.HOME }, irreversible = null, wholeWord = null, expiresInSeconds = null, @@ -373,7 +374,7 @@ class StatusListActivity : BottomSheetActivity(), AppBarLayoutHost, ActionButton { updateTagMuteState(false) Snackbar.make(binding.root, getString(R.string.confirmation_hashtag_unmuted, hashtag), Snackbar.LENGTH_SHORT).show() - eventHub.dispatch(FilterChangedEvent(Filter.Kind.HOME)) + eventHub.dispatch(FilterChangedEvent(FilterContext.HOME)) mutedFilterV1 = null mutedFilter = null }, diff --git a/app/src/main/java/app/pachli/appstore/Events.kt b/app/src/main/java/app/pachli/appstore/Events.kt index 7458368a2..5388782ff 100644 --- a/app/src/main/java/app/pachli/appstore/Events.kt +++ b/app/src/main/java/app/pachli/appstore/Events.kt @@ -2,7 +2,7 @@ package app.pachli.appstore import app.pachli.core.database.model.TabData import app.pachli.core.network.model.Account -import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.Poll import app.pachli.core.network.model.Status @@ -21,7 +21,7 @@ data class StatusComposedEvent(val status: Status) : Event data object StatusScheduledEvent : Event data class StatusEditedEvent(val originalId: String, val status: Status) : Event data class ProfileEditedEvent(val newProfileData: Account) : Event -data class FilterChangedEvent(val filterKind: Filter.Kind) : Event +data class FilterChangedEvent(val filterContext: FilterContext) : Event data class MainTabsChangedEvent(val newTabs: List) : Event data class PollVoteEvent(val statusId: String, val poll: Poll) : Event data class DomainMuteEvent(val instance: String) : Event diff --git a/app/src/main/java/app/pachli/components/filters/EditFilterActivity.kt b/app/src/main/java/app/pachli/components/filters/EditFilterActivity.kt index c92e60356..c5050826e 100644 --- a/app/src/main/java/app/pachli/components/filters/EditFilterActivity.kt +++ b/app/src/main/java/app/pachli/components/filters/EditFilterActivity.kt @@ -18,6 +18,7 @@ import app.pachli.core.common.extensions.viewBinding import app.pachli.core.common.extensions.visible import app.pachli.core.navigation.EditFilterActivityIntent import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterKeyword import app.pachli.core.network.retrofit.MastodonApi import app.pachli.databinding.ActivityEditFilterBinding @@ -48,20 +49,20 @@ class EditFilterActivity : BaseActivity() { private lateinit var filter: Filter private var originalFilter: Filter? = null - private lateinit var contextSwitches: Map + private lateinit var filterContextSwitches: Map override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) originalFilter = EditFilterActivityIntent.getFilter(intent) - filter = originalFilter ?: Filter("", "", listOf(), null, Filter.Action.WARN.action, listOf()) + filter = originalFilter ?: Filter("", "", listOf(), null, Filter.Action.WARN, listOf()) binding.apply { - contextSwitches = mapOf( - filterContextHome to Filter.Kind.HOME, - filterContextNotifications to Filter.Kind.NOTIFICATIONS, - filterContextPublic to Filter.Kind.PUBLIC, - filterContextThread to Filter.Kind.THREAD, - filterContextAccount to Filter.Kind.ACCOUNT, + filterContextSwitches = mapOf( + filterContextHome to FilterContext.HOME, + filterContextNotifications to FilterContext.NOTIFICATIONS, + filterContextPublic to FilterContext.PUBLIC, + filterContextThread to FilterContext.THREAD, + filterContextAccount to FilterContext.ACCOUNT, ) } @@ -90,9 +91,9 @@ class EditFilterActivity : BaseActivity() { } binding.filterDeleteButton.visible(originalFilter != null) - for (switch in contextSwitches.keys) { + for (switch in filterContextSwitches.keys) { switch.setOnCheckedChangeListener { _, isChecked -> - val context = contextSwitches[switch]!! + val context = filterContextSwitches[switch]!! if (isChecked) { viewModel.addContext(context) } else { @@ -156,7 +157,7 @@ class EditFilterActivity : BaseActivity() { } lifecycleScope.launch { viewModel.contexts.collect { contexts -> - for ((key, value) in contextSwitches) { + for ((key, value) in filterContextSwitches) { key.isChecked = contexts.contains(value) } } diff --git a/app/src/main/java/app/pachli/components/filters/EditFilterViewModel.kt b/app/src/main/java/app/pachli/components/filters/EditFilterViewModel.kt index 04b1750e9..c9611f761 100644 --- a/app/src/main/java/app/pachli/components/filters/EditFilterViewModel.kt +++ b/app/src/main/java/app/pachli/components/filters/EditFilterViewModel.kt @@ -6,6 +6,7 @@ import androidx.lifecycle.viewModelScope import app.pachli.appstore.EventHub import app.pachli.appstore.FilterChangedEvent import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterKeyword import app.pachli.core.network.retrofit.MastodonApi import at.connyduck.calladapter.networkresult.fold @@ -22,7 +23,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub val keywords = MutableStateFlow(listOf()) val action = MutableStateFlow(Filter.Action.WARN) val duration = MutableStateFlow(0) - val contexts = MutableStateFlow(listOf()) + val contexts = MutableStateFlow(listOf()) fun load(filter: Filter) { originalFilter = filter @@ -34,7 +35,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub } else { -1 } - contexts.value = filter.kinds + contexts.value = filter.contexts } fun addKeyword(keyword: FilterKeyword) { @@ -66,14 +67,14 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub this.action.value = action } - fun addContext(context: Filter.Kind) { - if (!contexts.value.contains(context)) { - contexts.value += context + fun addContext(filterContext: FilterContext) { + if (!contexts.value.contains(filterContext)) { + contexts.value += filterContext } } - fun removeContext(context: Filter.Kind) { - contexts.value = contexts.value.filter { it != context } + fun removeContext(filterContext: FilterContext) { + contexts.value = contexts.value.filter { it != filterContext } } fun validate(): Boolean { @@ -83,7 +84,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub } suspend fun saveChanges(context: Context): Boolean { - val contexts = contexts.value.map { it.kind } + val contexts = contexts.value val title = title.value val durationIndex = duration.value val action = action.value.action @@ -97,9 +98,9 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub // e.g., removing a filter from "home" still notifies anything showing // the home timeline, so the timeline can be refreshed. if (success) { - val originalKinds = originalFilter?.context?.map { Filter.Kind.from(it) } ?: emptyList() - val newKinds = contexts.map { Filter.Kind.from(it) } - (originalKinds + newKinds).distinct().forEach { + val originalContexts = originalFilter?.contexts ?: emptyList() + val newFilterContexts = contexts + (originalContexts + newFilterContexts).distinct().forEach { eventHub.dispatch(FilterChangedEvent(it)) } } @@ -107,7 +108,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub } } - private suspend fun createFilter(title: String, contexts: List, action: String, durationIndex: Int, context: Context): Boolean { + private suspend fun createFilter(title: String, contexts: List, action: String, durationIndex: Int, context: Context): Boolean { val expiresInSeconds = EditFilterActivity.getSecondsForDurationIndex(durationIndex, context) api.createFilter( title = title, @@ -131,7 +132,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub ) } - private suspend fun updateFilter(originalFilter: Filter, title: String, contexts: List, action: String, durationIndex: Int, context: Context): Boolean { + private suspend fun updateFilter(originalFilter: Filter, title: String, contexts: List, action: String, durationIndex: Int, context: Context): Boolean { val expiresInSeconds = EditFilterActivity.getSecondsForDurationIndex(durationIndex, context) api.updateFilter( id = originalFilter.id, @@ -167,18 +168,18 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub ) } - private suspend fun createFilterV1(context: List, expiresInSeconds: Int?): Boolean { + private suspend fun createFilterV1(contexts: List, expiresInSeconds: Int?): Boolean { return keywords.value.map { keyword -> - api.createFilterV1(keyword.keyword, context, false, keyword.wholeWord, expiresInSeconds) + api.createFilterV1(keyword.keyword, contexts, false, keyword.wholeWord, expiresInSeconds) }.none { it.isFailure } } - private suspend fun updateFilterV1(context: List, expiresInSeconds: Int?): Boolean { + private suspend fun updateFilterV1(contexts: List, expiresInSeconds: Int?): Boolean { val results = keywords.value.map { keyword -> if (originalFilter == null) { api.createFilterV1( phrase = keyword.keyword, - context = context, + context = contexts, irreversible = false, wholeWord = keyword.wholeWord, expiresInSeconds = expiresInSeconds, @@ -187,7 +188,7 @@ class EditFilterViewModel @Inject constructor(val api: MastodonApi, val eventHub api.updateFilterV1( id = originalFilter!!.id, phrase = keyword.keyword, - context = context, + context = contexts, irreversible = false, wholeWord = keyword.wholeWord, expiresInSeconds = expiresInSeconds, diff --git a/app/src/main/java/app/pachli/components/filters/FiltersAdapter.kt b/app/src/main/java/app/pachli/components/filters/FiltersAdapter.kt index 1c9c68536..b42062a3c 100644 --- a/app/src/main/java/app/pachli/components/filters/FiltersAdapter.kt +++ b/app/src/main/java/app/pachli/components/filters/FiltersAdapter.kt @@ -22,7 +22,7 @@ class FiltersAdapter(val listener: FiltersListener, val filters: List) : val binding = holder.binding val resources = binding.root.resources val actions = resources.getStringArray(R.array.filter_actions) - val contexts = resources.getStringArray(R.array.filter_contexts) + val filterContextNames = resources.getStringArray(R.array.filter_contexts) val filter = filters[position] val context = binding.root.context @@ -37,7 +37,7 @@ class FiltersAdapter(val listener: FiltersListener, val filters: List) : binding.textSecondary.text = context.getString( R.string.filter_description_format, actions.getOrNull(filter.action.ordinal - 1), - filter.context.map { contexts.getOrNull(Filter.Kind.from(it).ordinal) }.joinToString("/"), + filter.contexts.map { filterContextNames.getOrNull(it.ordinal) }.joinToString("/"), ) binding.delete.setOnClickListener { diff --git a/app/src/main/java/app/pachli/components/filters/FiltersViewModel.kt b/app/src/main/java/app/pachli/components/filters/FiltersViewModel.kt index e6ea4ea93..800fcd69c 100644 --- a/app/src/main/java/app/pachli/components/filters/FiltersViewModel.kt +++ b/app/src/main/java/app/pachli/components/filters/FiltersViewModel.kt @@ -70,8 +70,8 @@ class FiltersViewModel @Inject constructor( api.deleteFilter(filter.id).fold( { this@FiltersViewModel._state.value = State(this@FiltersViewModel._state.value.filters.filter { it.id != filter.id }, LoadingState.LOADED) - for (context in filter.context) { - eventHub.dispatch(FilterChangedEvent(Filter.Kind.from(context))) + for (context in filter.contexts) { + eventHub.dispatch(FilterChangedEvent(context)) } }, { throwable -> @@ -79,8 +79,8 @@ class FiltersViewModel @Inject constructor( api.deleteFilterV1(filter.id).fold( { this@FiltersViewModel._state.value = State(this@FiltersViewModel._state.value.filters.filter { it.id != filter.id }, LoadingState.LOADED) - filter.context.forEach { - eventHub.dispatch(FilterChangedEvent(Filter.Kind.from(it))) + filter.contexts.forEach { + eventHub.dispatch(FilterChangedEvent(it)) } }, { diff --git a/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt b/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt index da52e7c10..985982cd2 100644 --- a/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt @@ -35,6 +35,7 @@ import app.pachli.components.timeline.FiltersRepository import app.pachli.components.timeline.util.ifExpected import app.pachli.core.accounts.AccountManager import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.Notification import app.pachli.core.network.model.Poll import app.pachli.core.preferences.PrefKeys @@ -472,7 +473,7 @@ class NotificationsViewModel @Inject constructor( viewModelScope.launch { eventHub.events .filterIsInstance() - .filter { it.filterKind == Filter.Kind.NOTIFICATIONS } + .filter { it.filterContext == FilterContext.NOTIFICATIONS } .map { getFilters() repository.invalidate() @@ -538,8 +539,8 @@ class NotificationsViewModel @Inject constructor( private fun getFilters() = viewModelScope.launch { try { filterModel = when (val filters = filtersRepository.getFilters()) { - is FilterKind.V1 -> FilterModel(Filter.Kind.NOTIFICATIONS, filters.filters) - is FilterKind.V2 -> FilterModel(Filter.Kind.NOTIFICATIONS) + is FilterKind.V1 -> FilterModel(FilterContext.NOTIFICATIONS, filters.filters) + is FilterKind.V2 -> FilterModel(FilterContext.NOTIFICATIONS) } } catch (throwable: Throwable) { _uiErrorChannel.send(UiError.GetFilters(throwable)) diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/TimelineViewModel.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/TimelineViewModel.kt index 54d8bb1bd..0b7b792ad 100644 --- a/app/src/main/java/app/pachli/components/timeline/viewmodel/TimelineViewModel.kt +++ b/app/src/main/java/app/pachli/components/timeline/viewmodel/TimelineViewModel.kt @@ -46,6 +46,7 @@ import app.pachli.components.timeline.FiltersRepository import app.pachli.components.timeline.util.ifExpected import app.pachli.core.accounts.AccountManager import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.Poll import app.pachli.core.network.model.Status import app.pachli.core.network.model.TimelineKind @@ -521,7 +522,7 @@ abstract class TimelineViewModel( /** Updates the current set of filters if filter-related preferences change */ private fun updateFiltersFromPreferences() = eventHub.events .filterIsInstance() - .filter { filterContextMatchesKind(timelineKind, listOf(it.filterKind)) } + .filter { filterContextMatchesKind(timelineKind, listOf(it.filterContext)) } .map { getFilters() Timber.d("Reload because FilterChangedEvent") @@ -534,10 +535,10 @@ abstract class TimelineViewModel( viewModelScope.launch { Timber.d("getFilters()") try { - val filterKind = Filter.Kind.from(timelineKind) + val filterContext = FilterContext.from(timelineKind) filterModel = when (val filters = filtersRepository.getFilters()) { - is FilterKind.V1 -> FilterModel(filterKind, filters.filters) - is FilterKind.V2 -> FilterModel(filterKind) + is FilterKind.V1 -> FilterModel(filterContext, filters.filters) + is FilterKind.V2 -> FilterModel(filterContext) } } catch (throwable: Throwable) { Timber.d(throwable, "updateFilter(): Error fetching filters") @@ -635,9 +636,9 @@ abstract class TimelineViewModel( fun filterContextMatchesKind( timelineKind: TimelineKind, - filterContext: List, + filterContext: List, ): Boolean { - return filterContext.contains(Filter.Kind.from(timelineKind)) + return filterContext.contains(FilterContext.from(timelineKind)) } } } diff --git a/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt b/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt index ec145ef27..1dd0aed18 100644 --- a/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt +++ b/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt @@ -20,7 +20,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import app.pachli.appstore.EventHub import app.pachli.appstore.FilterChangedEvent -import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.TrendingTag import app.pachli.core.network.model.end import app.pachli.core.network.model.start @@ -96,7 +96,7 @@ class TrendingTagsViewModel @Inject constructor( TrendingTagsUiState(emptyList(), LoadingState.LOADED) } else { val homeFilters = deferredFilters.await().getOrNull()?.filter { filter -> - filter.context.contains(Filter.Kind.HOME.kind) + filter.contexts.contains(FilterContext.HOME) } val tags = tagResponse .filter { tag -> diff --git a/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt b/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt index 5029aefbb..be17f9330 100644 --- a/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt +++ b/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt @@ -38,6 +38,7 @@ import app.pachli.core.database.model.AccountEntity import app.pachli.core.database.model.TranslatedStatusEntity import app.pachli.core.database.model.TranslationState import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.Poll import app.pachli.core.network.model.Status import app.pachli.core.network.retrofit.MastodonApi @@ -112,7 +113,7 @@ class ViewThreadViewModel @Inject constructor( is StatusDeletedEvent -> handleStatusDeletedEvent(event) is StatusEditedEvent -> handleStatusEditedEvent(event) is FilterChangedEvent -> { - if (event.filterKind == Filter.Kind.THREAD) { + if (event.filterContext == FilterContext.THREAD) { loadFilters() } } @@ -527,8 +528,8 @@ class ViewThreadViewModel @Inject constructor( viewModelScope.launch { try { filterModel = when (val filters = filtersRepository.getFilters()) { - is FilterKind.V1 -> FilterModel(Filter.Kind.THREAD, filters.filters) - is FilterKind.V2 -> FilterModel(Filter.Kind.THREAD) + is FilterKind.V1 -> FilterModel(FilterContext.THREAD, filters.filters) + is FilterKind.V2 -> FilterModel(FilterContext.THREAD) } updateStatuses() } catch (_: Exception) { diff --git a/app/src/main/java/app/pachli/network/FilterModel.kt b/app/src/main/java/app/pachli/network/FilterModel.kt index 3d563a216..4a0afc170 100644 --- a/app/src/main/java/app/pachli/network/FilterModel.kt +++ b/app/src/main/java/app/pachli/network/FilterModel.kt @@ -1,6 +1,7 @@ package app.pachli.network import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterV1 import app.pachli.core.network.model.Status import app.pachli.core.network.parseAsMastodonHtml @@ -10,16 +11,16 @@ import java.util.regex.Pattern /** * Filter statuses using V1 or V2 filters. * - * Construct with [filterKind] that corresponds to the kind of timeline, and optionally the set + * Construct with [filterContext] that corresponds to the kind of timeline, and optionally the set * of v1 filters that should be applied. */ -class FilterModel constructor(private val filterKind: Filter.Kind, v1filters: List? = null) { +class FilterModel(private val filterContext: FilterContext, v1filters: List? = null) { /** Pattern to use when matching v1 filters against a status. Null if these are v2 filters */ private var pattern: Pattern? = null init { pattern = v1filters?.let { list -> - makeFilter(list.filter { it.context.contains(filterKind.kind) }) + makeFilter(list.filter { it.contexts.contains(filterContext) }) } } @@ -48,7 +49,7 @@ class FilterModel constructor(private val filterKind: Filter.Kind, v1filters: Li } val matchingKind = status.filtered?.filter { result -> - result.filter.kinds.contains(filterKind) + result.filter.contexts.contains(filterContext) } return if (matchingKind.isNullOrEmpty()) { diff --git a/app/src/test/java/app/pachli/FilterV1Test.kt b/app/src/test/java/app/pachli/FilterV1Test.kt index 1db651600..621c7c4c1 100644 --- a/app/src/test/java/app/pachli/FilterV1Test.kt +++ b/app/src/test/java/app/pachli/FilterV1Test.kt @@ -21,6 +21,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import app.pachli.components.filters.EditFilterActivity import app.pachli.core.network.model.Attachment import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterV1 import app.pachli.core.network.model.Poll import app.pachli.core.network.model.PollOption @@ -45,7 +46,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "badWord", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = null, irreversible = false, wholeWord = false, @@ -53,7 +54,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "badWholeWord", - context = listOf(FilterV1.HOME, FilterV1.PUBLIC), + contexts = listOf(FilterContext.HOME, FilterContext.PUBLIC), expiresAt = null, irreversible = false, wholeWord = true, @@ -61,7 +62,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "@twitter.com", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = null, irreversible = false, wholeWord = true, @@ -69,7 +70,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "#hashtag", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = null, irreversible = false, wholeWord = true, @@ -77,7 +78,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "expired", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = Date.from(Instant.now().minusSeconds(10)), irreversible = false, wholeWord = true, @@ -85,7 +86,7 @@ class FilterV1Test { FilterV1( id = "123", phrase = "unexpired", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = Date.from(Instant.now().plusSeconds(3600)), irreversible = false, wholeWord = true, @@ -93,14 +94,14 @@ class FilterV1Test { FilterV1( id = "123", phrase = "href", - context = listOf(FilterV1.HOME), + contexts = listOf(FilterContext.HOME), expiresAt = null, irreversible = false, wholeWord = false, ), ) - filterModel = FilterModel(Filter.Kind.HOME, filters) + filterModel = FilterModel(FilterContext.HOME, filters) } @Test diff --git a/core/network/src/main/kotlin/app/pachli/core/network/di/NetworkModule.kt b/core/network/src/main/kotlin/app/pachli/core/network/di/NetworkModule.kt index 30cea5824..e7e493278 100644 --- a/core/network/src/main/kotlin/app/pachli/core/network/di/NetworkModule.kt +++ b/core/network/src/main/kotlin/app/pachli/core/network/di/NetworkModule.kt @@ -24,6 +24,7 @@ import app.pachli.core.mastodon.model.MediaUploadApi import app.pachli.core.network.BuildConfig import app.pachli.core.network.json.BooleanIfNull import app.pachli.core.network.json.DefaultIfNull +import app.pachli.core.network.json.EnumConstantConverterFactory import app.pachli.core.network.json.Guarded import app.pachli.core.network.json.HasDefault import app.pachli.core.network.retrofit.InstanceSwitchAuthInterceptor @@ -127,6 +128,7 @@ object NetworkModule { ): Retrofit { return Retrofit.Builder().baseUrl("https://" + MastodonApi.PLACEHOLDER_DOMAIN) .client(httpClient) + .addConverterFactory(EnumConstantConverterFactory) .addConverterFactory(MoshiConverterFactory.create(moshi)) .addCallAdapterFactory(NetworkResultCallAdapterFactory.create()) .build() diff --git a/core/network/src/main/kotlin/app/pachli/core/network/json/EnumConstantConverterFactory.kt b/core/network/src/main/kotlin/app/pachli/core/network/json/EnumConstantConverterFactory.kt new file mode 100644 index 000000000..bfffcba96 --- /dev/null +++ b/core/network/src/main/kotlin/app/pachli/core/network/json/EnumConstantConverterFactory.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.core.network.json + +import com.squareup.moshi.Json +import java.lang.reflect.Type +import retrofit2.Converter +import retrofit2.Retrofit + +/** + * Retrofit [Converter.Factory] that converts enum constants to strings using the + * value of any `@Json(name = ...)` annotation on the constant, falling back to + * the enum's name if the annotation is not present. + * + * This ensures that the same string is used for an enum constant's value whether + * it is sent/received as JSON or sent as a [retrofit2.http.FormUrlEncoded] value. + * + * To install in Retrofit call `.addConverterFactory(EnumConstantConverterFactory)` + * on the Retrofit builder. + */ +object EnumConstantConverterFactory : Converter.Factory() { + object EnumConstantConverter : Converter, String> { + override fun convert(enum: Enum<*>): String { + return try { + enum.javaClass.getField(enum.name).getAnnotation(Json::class.java)?.name + } catch (_: Exception) { + null + } ?: enum.toString() + } + } + + override fun stringConverter( + type: Type, + annotations: Array, + retrofit: Retrofit, + ): Converter, String>? { + return if (type is Class<*> && type.isEnum) EnumConstantConverter else null + } +} diff --git a/core/network/src/main/kotlin/app/pachli/core/network/model/Filter.kt b/core/network/src/main/kotlin/app/pachli/core/network/model/Filter.kt index 344717f1a..41e12092f 100644 --- a/core/network/src/main/kotlin/app/pachli/core/network/model/Filter.kt +++ b/core/network/src/main/kotlin/app/pachli/core/network/model/Filter.kt @@ -13,9 +13,9 @@ import kotlinx.parcelize.Parcelize data class Filter( val id: String, val title: String, - val context: List, + @Json(name = "context") val contexts: List, @Json(name = "expires_at") val expiresAt: Date?, - @Json(name = "filter_action") val filterAction: String, + @Json(name = "filter_action") val action: Action, // This should not normally be empty. However, Mastodon does not include // this in a status' `filtered.filter` property (it's not null or empty, // it's missing) which breaks deserialisation. Patch this by ensuring it's @@ -26,48 +26,14 @@ data class Filter( ) : Parcelable { @HasDefault enum class Action(val action: String) { + @Json(name = "none") NONE("none"), + @Json(name = "warn") @Default WARN("warn"), + + @Json(name = "hide") HIDE("hide"), - ; - - companion object { - fun from(action: String): Action = entries.firstOrNull { it.action == action } ?: WARN - } } - - @HasDefault - enum class Kind(val kind: String) { - HOME("home"), - NOTIFICATIONS("notifications"), - - @Default - PUBLIC("public"), - THREAD("thread"), - ACCOUNT("account"), - ; - - companion object { - fun from(kind: String): Kind = entries.firstOrNull { it.kind == kind } ?: PUBLIC - - fun from(kind: TimelineKind): Kind = when (kind) { - is TimelineKind.Home, is TimelineKind.UserList -> HOME - is TimelineKind.PublicFederated, - is TimelineKind.PublicLocal, - is TimelineKind.Tag, - is TimelineKind.Favourites, - -> PUBLIC - is TimelineKind.User -> ACCOUNT - else -> PUBLIC - } - } - } - - val action: Action - get() = Action.from(filterAction) - - val kinds: List - get() = context.map { Kind.from(it) } } diff --git a/core/network/src/main/kotlin/app/pachli/core/network/model/FilterContext.kt b/core/network/src/main/kotlin/app/pachli/core/network/model/FilterContext.kt new file mode 100644 index 000000000..ada0f6006 --- /dev/null +++ b/core/network/src/main/kotlin/app/pachli/core/network/model/FilterContext.kt @@ -0,0 +1,68 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.core.network.model + +import app.pachli.core.network.json.Default +import app.pachli.core.network.json.HasDefault +import com.squareup.moshi.Json +import com.squareup.moshi.JsonClass + +/** + * The contexts in which a filter should be applied, for both a + * [v2](https://docs.joinmastodon.org/entities/Filter/#context) and + * [v1](https://docs.joinmastodon.org/entities/V1_Filter/#context) Mastodon + * filter. The API versions have identical contexts. + */ +@JsonClass(generateAdapter = false) +@HasDefault +enum class FilterContext { + /** Filter applies to home timeline and lists */ + @Json(name = "home") + HOME, + + /** Filter applies to notifications */ + @Json(name = "notifications") + NOTIFICATIONS, + + /** Filter applies to public timelines */ + @Default + @Json(name = "public") + PUBLIC, + + /** Filter applies to expanded thread */ + @Json(name = "thread") + THREAD, + + /** Filter applies when viewing a profile */ + @Json(name = "account") + ACCOUNT, + ; + + companion object { + fun from(kind: TimelineKind): FilterContext = when (kind) { + is TimelineKind.Home, is TimelineKind.UserList -> HOME + is TimelineKind.PublicFederated, + is TimelineKind.PublicLocal, + is TimelineKind.Tag, + is TimelineKind.Favourites, + -> PUBLIC + is TimelineKind.User -> ACCOUNT + else -> PUBLIC + } + } +} diff --git a/core/network/src/main/kotlin/app/pachli/core/network/model/FilterV1.kt b/core/network/src/main/kotlin/app/pachli/core/network/model/FilterV1.kt index 6227b8d99..c6ffb8e1b 100644 --- a/core/network/src/main/kotlin/app/pachli/core/network/model/FilterV1.kt +++ b/core/network/src/main/kotlin/app/pachli/core/network/model/FilterV1.kt @@ -24,19 +24,11 @@ import java.util.Date data class FilterV1( val id: String, val phrase: String, - val context: List, + @Json(name = "context") val contexts: List, @Json(name = "expires_at") val expiresAt: Date?, val irreversible: Boolean, @Json(name = "whole_word") val wholeWord: Boolean, ) { - companion object { - const val HOME = "home" - const val NOTIFICATIONS = "notifications" - const val PUBLIC = "public" - const val THREAD = "thread" - const val ACCOUNT = "account" - } - override fun hashCode(): Int { return id.hashCode() } @@ -53,9 +45,9 @@ data class FilterV1( return Filter( id = id, title = phrase, - context = context, + contexts = contexts, expiresAt = expiresAt, - filterAction = Filter.Action.WARN.action, + action = Filter.Action.WARN, keywords = listOf( FilterKeyword( id = id, diff --git a/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt b/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt index 2efebb10e..341830761 100644 --- a/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt +++ b/core/network/src/main/kotlin/app/pachli/core/network/retrofit/MastodonApi.kt @@ -25,6 +25,7 @@ import app.pachli.core.network.model.Conversation import app.pachli.core.network.model.DeletedStatus import app.pachli.core.network.model.Emoji import app.pachli.core.network.model.Filter +import app.pachli.core.network.model.FilterContext import app.pachli.core.network.model.FilterKeyword import app.pachli.core.network.model.FilterV1 import app.pachli.core.network.model.HashTag @@ -614,7 +615,7 @@ interface MastodonApi { @POST("api/v1/filters") suspend fun createFilterV1( @Field("phrase") phrase: String, - @Field("context[]") context: List, + @Field("context[]") context: List, @Field("irreversible") irreversible: Boolean?, @Field("whole_word") wholeWord: Boolean?, @Field("expires_in") expiresInSeconds: Int?, @@ -625,7 +626,7 @@ interface MastodonApi { suspend fun updateFilterV1( @Path("id") id: String, @Field("phrase") phrase: String, - @Field("context[]") context: List, + @Field("context[]") context: List, @Field("irreversible") irreversible: Boolean?, @Field("whole_word") wholeWord: Boolean?, @Field("expires_in") expiresInSeconds: Int?, @@ -640,7 +641,7 @@ interface MastodonApi { @POST("api/v2/filters") suspend fun createFilter( @Field("title") title: String, - @Field("context[]") context: List, + @Field("context[]") context: List, @Field("filter_action") filterAction: String, @Field("expires_in") expiresInSeconds: Int?, ): NetworkResult @@ -650,7 +651,7 @@ interface MastodonApi { suspend fun updateFilter( @Path("id") id: String, @Field("title") title: String? = null, - @Field("context[]") context: List? = null, + @Field("context[]") context: List? = null, @Field("filter_action") filterAction: String? = null, @Field("expires_in") expiresInSeconds: Int? = null, ): NetworkResult diff --git a/core/network/src/test/kotlin/app/pachli/core/network/json/EnumConstantConverterFactoryTest.kt b/core/network/src/test/kotlin/app/pachli/core/network/json/EnumConstantConverterFactoryTest.kt new file mode 100644 index 000000000..e8e2aa37f --- /dev/null +++ b/core/network/src/test/kotlin/app/pachli/core/network/json/EnumConstantConverterFactoryTest.kt @@ -0,0 +1,26 @@ +package app.pachli.core.network.json + +import com.google.common.truth.Truth.assertThat +import com.squareup.moshi.Json +import org.junit.Test + +class EnumConstantConverterFactoryTest { + enum class Enum { + @Json(name = "one") + ONE, + + TWO, + } + + private val converter = EnumConstantConverterFactory.EnumConstantConverter + + @Test + fun `Annotated enum constant uses annotation`() { + assertThat(converter.convert(Enum.ONE)).isEqualTo("one") + } + + @Test + fun `Unannotated enum constant uses constant name`() { + assertThat(converter.convert(Enum.TWO)).isEqualTo("TWO") + } +}