Add support for PredefinedFilters for QueryChannels#6415
Add support for PredefinedFilters for QueryChannels#6415VelikovPetar wants to merge 16 commits intodevelopfrom
PredefinedFilters for QueryChannels#6415Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
|
DB Entities have been updated. Do we need to upgrade DB Version? |
SDK Size Comparison 📏
|
|
| return Filters.and(*filters.toTypedArray()) | ||
| } | ||
|
|
||
| @Suppress("SpreadOperator") |
There was a problem hiding this comment.
Maybe at some point we should consider introducing Filters overloads that take a collection, so we avoid these suppression and the intermediate array (since now we do collection -> array -> set). Not blocking at all for this PR.
There was a problem hiding this comment.
I would eventually love to migrate to the type-safe filters from core - they would also make this parser much simpler (or not simpler, but rather safer)
| * compatibility, we replace the held [_querySpec] instance instead of mutating it in place. | ||
| * `cids` and the predefined identity fields are carried over from the previous instance. | ||
| */ | ||
| fun applyResolvedSpec(filter: FilterObject, sort: QuerySorter<Channel>) { |
There was a problem hiding this comment.
In the docs we're saying that this is no op in the standard case, but that responsibility is currently on the callers. I'm a bit concerned that we might be accidentally call this for standard filters in the future.
Some ideas:
- Maybe we could we use a type specific to predefined filters here to make this un-callable for standard filters? -> although we don't yet have that type as far as I can see
- We could change the function name and update the documentation to say that this should only be called for predefined filters rather than implying that the function itself is no op
- We cold make it no op by adding an early return on
identifier !is QueryChannelsIdentifier.Predefined
There was a problem hiding this comment.
Hm good point, I actually re-did this part couple of times, so the docs are a bit outdated. I will revisit this, taking your feedback into consideration.
| val predefinedFilterName: String? = null, | ||
| val predefinedFilterValues: Map<String, Any>? = null, | ||
| val predefinedSortValues: Map<String, Any>? = null, |
There was a problem hiding this comment.
Is it ok that we define these as nullable but then they'll become empty maps when deserialized? Do you think it's worth to align the types/defaults?
There was a problem hiding this comment.
Aham I missed this part. Hmm, I will need to go over the flow - I think it shouldn't be an issue, but I will double check.
| * Note on shape: the predefined-filter fields ([predefinedFilterName], | ||
| * [predefinedFilterValues], [predefinedSortValues]) are declared as body-level `var` properties | ||
| * rather than primary-constructor parameters specifically to keep the primary `<init>(filter, | ||
| * querySort)` and the synthesized `copy(filter, querySort)` JVM signatures unchanged. |
There was a problem hiding this comment.
Could we instead introduce a secondary constructor + a copy function? Something like:
public data class QueryChannelsSpec(
val filter: FilterObject,
val querySort: QuerySorter<Channel>,
val cids: Set<String> = emptySet(),
val predefinedFilterName: String? = null,
val predefinedFilterValues: Map<String, Any>? = null,
val predefinedSortValues: Map<String, Any>? = null,
) {
public constructor(
filter: FilterObject,
querySort: QuerySorter<Channel>,
) : this(filter, querySort, emptySet(), null, null, null)
public fun copy(
filter: FilterObject = this.filter,
querySort: QuerySorter<Channel> = this.querySort,
): QueryChannelsSpec = QueryChannelsSpec(
filter = filter,
querySort = querySort,
cids = cids,
predefinedFilterName = predefinedFilterName,
predefinedFilterValues = predefinedFilterValues,
predefinedSortValues = predefinedSortValues,
)
}So we avoid the vars and we make sure equals and hashCode take all properties into account (at the moment, a predefined filter is treated as equal to a standard filter that has the same resolved filter/sort).
There was a problem hiding this comment.
Will look into it!
|
|
||
| public final class io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelFactory : androidx/lifecycle/ViewModelProvider$Factory { | ||
| public static final field $stable I | ||
| public fun <init> ()V |
There was a problem hiding this comment.
Is it ok that we lost this? I believe it only affects java callers, but would break them if they just call new ChannelListViewModelFactory()
There was a problem hiding this comment.
Hmm good point, let me see what we can do about this.
2 participants
Goal
Add support for server-side
PredefinedFilterstoQueryChannels. Apps can now run a channel query by referencing a filter/sort registered on the backend (by name, plus interpolation values) instead of building theFilterObject/QuerySorteron the client. This lets product/backend tweak filtering rules without an app release.You can check / update / create the predefined filters for the app "Stream Mobile SDK / Stream SDK - Android" - in the "Predefined Filters" section in the Beta Dashboard
Implementation
Public API
QueryChannelsRequestgainspredefinedFilter: String?,filterValues: Map<String, Any>?,sortValues: Map<String, Any>?. WhenpredefinedFilteris set, the client-sidefilter/querySortare ignored by the backend.QueryChannelsResultis a new wrapper aroundChatApi.queryChannelsthat exposes bothchannelsand the backend-resolvedPredefinedFilter(parsedname+FilterObject+QuerySorter<Channel>).ChannelListViewModel(Compose) andChannelListViewModel(UI Components) get a new constructor /ChannelListViewModelFactoryoverload takingpredefinedFilterName,filterValues,sortValues. In predefined mode,setFilters/setQuerySortare no-ops; channel search still narrows the displayed list.State / offline architecture
QueryChannelsIdentifiersealed type (Standard/Predefined) is the canonical key used byLogicRegistry,StateRegistry, the listener, and the offline repository. AQueryChannelsRequest/QueryChannelsSpecresolves to one or the other depending on whetherpredefinedFilteris present.QueryChannelsSpeckeeps its existing(filter, querySort)constructor (so JVM signatures stay stable) and addspredefinedFilterName/predefinedFilterValues/predefinedSortValuesasvars. For predefined queries, those three form the row's stable identity;filter/querySortcarry the currently resolved values.QueryChannelsMutableStatestarts predefined queries with placeholder filter/sort and applies the resolved spec viaapplyResolvedSpec(...)once the backend response arrives (or the row is rehydrated from disk).ChannelListViewModel.QueryMode(StandardvsPredefined) drives the two flows — pagination, refresh, search, and filter-flow wiring branch on it.Persistence
QueryChannelsEntitygainspredefinedFilterName/predefinedFilterValues/predefinedSortValues.QueryChannelsRepository.selectBy(identifier)replaces the previous filter-based lookup.104(destructive migration, in line with the rest of the SDK).Wire format
FilterDomainMappingnow supports a full round-trip (DTO ↔ domain) so the resolved predefined filter coming back from the server can be parsed into aFilterObjectand stored.Testing
QueryChannelsIdentifierderivation,MoshiChatApi,FilterDomainMapping(round trip),QuerySortByFieldround trip,DatabaseQueryChannelsRepository,QueryChannelsLogic/QueryChannelsDatabaseLogic/QueryChannelsListenerState,QueryChannelsMutableState,LogicRegistry, and bothChannelListViewModels.android_sample_filterso the feature can be exercised end-to-end on device.