Add support for PredefinedFilters for QueryChannels#6415
Open
VelikovPetar wants to merge 23 commits intodevelopfrom
Open
Add support for PredefinedFilters for QueryChannels#6415VelikovPetar wants to merge 23 commits intodevelopfrom
PredefinedFilters for QueryChannels#6415VelikovPetar wants to merge 23 commits intodevelopfrom
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
|
DB Entities have been updated. Do we need to upgrade DB Version? |
Contributor
SDK Size Comparison 📏
|
gpunto
reviewed
May 6, 2026
gpunto
approved these changes
May 7, 2026
gpunto
approved these changes
May 11, 2026
WalkthroughAdds predefined-filter channel queries and QueryChannelsResult, introduces QueryChannelsIdentifier across state/logic/cache, updates DB schema and converters, refactors client wiring, and adapts Compose/UI ViewModels, samples, and tests to the new identifier-based flow. ChangesPredefined channel queries end-to-end
Sequence Diagram(s)sequenceDiagram
participant UI
participant Client
participant Logic
participant Api
UI->>Client: queryChannels(request)
Client->>Logic: get by request.identifier
Logic->>Api: send QueryChannelsRequest
Api-->>Logic: QueryChannelsResult (channels, predefinedFilter?)
Logic-->>UI: channels (apply resolved spec when present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt (2)
127-134: 💤 Low value
normalizeValuedoes not recurse into nested maps.Lists are normalized recursively (line 132), but a
Mapvalue is returned as-is. In practice this is fine today because the only maps reachable here are operator maps (handled separately) — but if a future filter operator accepts a structured value (e.g., a sub-document like{"$nearby": {"lat": 1.0, "lng": 2.0}}), whole-number doubles inside that map would not be normalized toInt. Worth either adding a comment that the normalization is intentionally shallow w.r.t. maps, or recursing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt` around lines 127 - 134, The normalizeValue function currently handles Lists recursively but returns Map values unchanged, so nested maps containing whole-number Doubles won't get converted to Ints; update normalizeValue (in FilterDomainMapping.kt) to detect Map<*,*> values and recursively normalize their entries (e.g., map keys preserved, map values passed back into normalizeValue) so nested structures are normalized consistently, or add a clear comment in normalizeValue stating maps are intentionally not recursed; ensure you reference the normalizeValue function and the branch that handles List values when making the change.
86-95: ⚡ Quick winField name is added to the tracked set even when parsing of the field's value fails.
fields.add(fieldName)happens unconditionally at the top ofparseFieldFilter, before any of the bailout paths (operatorMap.isEmpty(), unknownopKey,$existswith a non-boolean value,$in/$ninwith a non-collection,$autocompletewith a non-string). When the function returnsnull,parseFilterMapskips that entry but the field name remains in the collected set, so callers can see fields that don't actually correspond to any produced filter clause. In the multi-key implicit-AND path (line 58) this means the fallback-sort logic could be steered by a field that ended up not being part of the parsed filter.Worth moving
fields.add(fieldName)to just before each successful return (or after thewhenresolves to a non-nullFilterObject).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt` around lines 86 - 95, The code currently calls fields.add(fieldName) unconditionally at the top of parseFieldFilter, which records field names even when parsing fails; change this so the field is only added when a valid FilterObject will be returned — move the fields.add(fieldName) call to immediately before each successful return of a FilterObject (e.g. before returning Filters.eq(...) for non-map values, and before each branch in the operatorMap handling that yields a non-null FilterObject such as $exists, $in/$nin, $autocomplete, or other operators), or alternatively add it once after the when/operatorMap resolution only if the resolved FilterObject is non-null; ensure no fields are added on early bailouts (operatorMap.isEmpty(), invalid types, unknown opKey) so parseFilterMap and fallback-sort logic only see fields that produced filters.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifier.kt (1)
54-58: ⚡ Quick winIdentity equality depends on the underlying value types in
filterValues/sortValues.
Map<String, Any>?equality is structural, but for non-primitiveAnyvalues it ultimately delegates to whateverequalsthe value type provides. If callers pass e.g. anArray<*>or a custom mutable wrapper as a value, two semantically identicalPredefinedidentifiers would compare unequal, which would silently spawn duplicateQueryChannelsLogic/state entries inLogicRegistry/StateRegistryand duplicate DB rows.Worth either:
- documenting on the property that values must have value-based
equals(e.g., primitives, strings, lists, maps), or- normalizing/validating the map at construction time (e.g., reject arrays or convert them to lists).
As per coding guidelines: "Document public APIs with KDoc, including thread expectations and state notes".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifier.kt` around lines 54 - 58, The Predefined data class (Predefined : QueryChannelsIdentifier) currently stores filterValues and sortValues as Map<String, Any>?, whose equality can be unstable for values like Array or custom mutable wrappers; update the class to (1) add KDoc on filterValues/sortValues documenting that values must be value-equality types (primitives, String, List, Map, etc.) and thread/state expectations, and (2) normalize/validate inputs in the constructor (or factory) of Predefined: convert Array instances to Lists and/or reject unsupported mutable wrappers by throwing IllegalArgumentException so identity comparisons are stable and you don't end up with duplicate QueryChannelsLogic/StateRegistry entries. Ensure references to Predefined, filterValues, sortValues, and QueryChannelsIdentifier are used so reviewers can locate the change.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifierTest.kt (1)
81-95: 💤 Low valueOptional: add a case distinguishing empty-map vs
nullpredefined values.The negative-equality cases cover same-name-different-values well. A nearby gotcha worth pinning down is
Predefined("p", emptyMap(), null)vsPredefined("p", null, null)— they are unequal underMap.equals, which means a caller that sometimes passesemptyMap()and sometimesnullwould inadvertently spawn two separate cache entries. A test that documents the chosen behavior (whichever way you go) would prevent regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifierTest.kt` around lines 81 - 95, Add a test that pins down behavior for empty-map vs null predefined values: in QueryChannelsIdentifierTest.kt add a test that constructs QueryChannelsIdentifier.Predefined("p", emptyMap(), null) and QueryChannelsIdentifier.Predefined("p", null, null) and then assert the expected relation (either assertEquals if you decide emptyMap() should be treated the same as null, or assertNotEquals if you want them distinct). Reference the Predefined constructor used elsewhere in this file and assert the chosen behavior so future changes to equality semantics are caught.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/query/QueryChannelsSpec.kt (1)
36-47: 💤 Low valueConsider not surfacing
cidsandpredefined*in the public KDoc summary.The doc on
predefinedFilterNamesays it "form[s] the spec's stable identity in the offline DB and must not change once assigned" — but the primary constructor still accepts these as mutable-by-construction params on adata class, soequals/hashCode/copywill treat differences as different specs. That is correct behavior; the doc just reads as if these fields had special immutability beyond the language guarantees. A short note that identity is enforced viaQueryChannelsIdentifier.Predefinedderived from these values (not by the spec instance equality alone) would resolve the ambiguity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/query/QueryChannelsSpec.kt` around lines 36 - 47, The public KDoc summary for QueryChannelsSpec currently presents cids and predefinedFilterName/predefined* as if they confer special immutability/identity beyond normal data class semantics; update the KDoc on QueryChannelsSpec to remove or de-emphasize cids and the predefined* fields from the top-line summary and add a concise note stating that the spec instance still uses normal data class equality/copy semantics and that stable identity for offline DB usage is enforced via QueryChannelsIdentifier.Predefined which is derived from predefinedFilterName and predefinedFilterValues (not by the QueryChannelsSpec instance itself).stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/QueryChannelsRequest.kt (1)
25-40: ⚡ Quick winKDoc could clarify thread/state expectations and
sortValuessemantics.For a public API in
stream-chat-android-clientthe guidelines call for documenting thread expectations and state notes. A few specific gaps:
predefinedFilteris described as an "ID", but the field used in the test/sample app reads like a name ("android_sample_filter"). Aligning wording (name vs ID) prevents user confusion.filterValues/sortValuessay "values to interpolate into the predefined ... template" — but nothing documents the expected map shape (string keys, any-typed leaves), what happens when these arenullvs empty, or whether both must be supplied together withpredefinedFilter.- A short note that
filter/querySortare still observed locally (e.g., used to seedQueryChannelsMutableStateplaceholders before the response resolves the predefined version) would help users who pass non-default values alongsidepredefinedFilter.Not blocking, but this is a public-API surface that downstream apps will read.
As per coding guidelines: "Document public APIs with KDoc, including thread expectations and state notes".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/QueryChannelsRequest.kt` around lines 25 - 40, Update the KDoc for QueryChannelsRequest to clarify semantics: change the wording for predefinedFilter to say "server-side predefined filter name/ID" (match examples like "android_sample_filter"); specify expected shapes and types for filterValues and sortValues (e.g., map of string keys to primitive/string/nullable values), and explain behavior when they are null vs empty and whether they must be provided when predefinedFilter is set; explicitly state that filter and querySort are ignored by the backend but still observed locally (used to seed QueryChannelsMutableState placeholders) until the server response arrives; and add a short thread/state note indicating which thread(s) these objects are safe to construct/consume (main-thread vs background) and any mutation expectations.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMappingTest.kt (1)
51-285: ⚡ Quick winConsider adding negative/edge-case coverage.
The parameterized cases cover the happy paths nicely. A few negative scenarios would harden the parser against future server changes since the input shape originates from the backend:
- Unknown operator key (e.g.,
{"x": {"$weirdOp": 1}}→ expectnull).$existswith a non-boolean value (e.g.,{"x": {"$exists": "yes"}}→ expectnull).$in/$ninwith a non-collection value (e.g.,{"x": {"$in": "messaging"}}→ expectnull).- Empty
$and/$or/$norlist (e.g.,{"$and": emptyList<Any>()}→ expectnull).- A logical-operator value that is not a list (e.g.,
{"$and": mapOf(...)}→ expectnull).These would also pin down the "field-name leaks on failed parse" behavior I flagged in
FilterDomainMapping.kt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMappingTest.kt` around lines 51 - 285, Add parameterized negative test cases to toFilterDomainWithFieldsArguments: include Arguments.of entries where the input map uses an unknown operator (e.g., mapOf("x" to mapOf("$weirdOp" to 1))), $exists with non-boolean (mapOf("x" to mapOf("$exists" to "yes"))), $in/$nin with non-collection (mapOf("x" to mapOf("$in" to "messaging"))), empty logical lists (mapOf("$and" to emptyList<Any>())), and logical operators whose value is not a list (mapOf("$and" to mapOf("type" to "messaging"))); for each of these assert the mapped Filter is null and the tracked field set is empty (match the existing Arguments.of null expectations in toFilterDomainWithFieldsArguments) so FilterDomainMapping.kt parsing failures do not leak field names.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/QueryChannelsRepository.kt (1)
36-41: ⚡ Quick winExpand
selectByKDoc with threading/state contract.The new public suspend API documents intent but not thread expectations or returned-state behavior for identifier variants. Please make that explicit in KDoc.
✍️ Suggested KDoc patch
/** * Selects a query spec by its identifier. * + * Threading: Safe to call from any coroutine context. + * State: Returns cached [QueryChannelsSpec] for both standard and predefined identifiers, + * or `null` if no matching entry exists. + * * `@param` identifier The query spec identifier. */ public suspend fun selectBy(identifier: QueryChannelsIdentifier): QueryChannelsSpec?As per coding guidelines, "Document public APIs with KDoc, including thread expectations and state notes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/QueryChannelsRepository.kt` around lines 36 - 41, Update the KDoc for the public suspend function selectBy in QueryChannelsRepository to explicitly state threading and state guarantees: note that selectBy is a suspend function and must be called from a coroutine (it is main-safe but may perform IO), describe whether it returns a snapshot copy or a live reference of QueryChannelsSpec (e.g., immutable snapshot vs. backing-store view), and document behavior for missing identifiers (returns null) and any concurrency guarantees (e.g., consistent view at call time, not automatically updated). Reference the QueryChannelsIdentifier parameter and QueryChannelsSpec return type in the KDoc so callers know the exact thread expectations and returned-state semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/StateRegistry.kt`:
- Around line 94-118: The current implementation seeds predefined queries with
Filters.neutral() (see QueryChannelsIdentifier.Predefined and
Filters.neutral()), which makes the query act like "match-all" until
applyResolvedSpec runs; instead, construct the QueryChannelsMutableState in an
unresolved state (e.g., pass null/absent initialFilter and initialSort or an
explicit unresolved flag) for QueryChannelsIdentifier.Predefined and update
QueryChannelsMutableState to treat that unresolved marker by ignoring
event-driven membership/channel updates until applyResolvedSpec resolves the
real filter/sort; ensure the constructor signature used here
(QueryChannelsMutableState(...)) accepts the unresolved marker or flag so
consumers can detect and skip processing until resolution.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt`:
- Around line 87-125: parseFieldFilter currently only uses
operatorMap.entries.first(), which silently drops additional operators; change
it to detect multi-operator maps and reject them: after casting to operatorMap
(the Map<String, Any> variable), add a check if (operatorMap.size > 1) return
null to explicitly fail on multi-operator objects, then proceed to read the
single operator as before (replace the current use of
operatorMap.entries.first() with a safe single-entry extraction). This uses the
existing parseFieldFilter and operatorMap identifiers so the logic fails fast
instead of silently dropping operators.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.kt`:
- Around line 1304-1308: The predefinedFilter deserialization is silently
degrading when response.predefined_filter exists but
response.predefined_filter.filter.toFilterDomainWithFields() or
response.predefined_filter.sort.toSortDomain() returns null; update the logic in
MoshiChatApi.kt so that if toFilterDomainWithFields() or toSortDomain() fails
(i.e., returns null) you treat that as an error case rather than as "absent" —
surface/propagate an error (or return a failing Result) instead of returning a
null predefinedFilter or applying defaultPredefinedFilterSort(filterFields).
Concretely, replace the return@let null behavior around predefinedFilter with
explicit error handling: detect the difference between a missing
predefined_filter and an unparseable spec, and raise or propagate a parsing
exception (or a failure result) when toFilterDomainWithFields() or
toSortDomain() fails so callers can handle the backend-resolved-but-unparseable
metadata instead of silently degrading it.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.kt`:
- Around line 277-301: fetchPage currently swallows failures by returning
emptyList(), which makes getRemoteCids treat transient errors as an empty final
page and allows updateOnlineChannels to remove still-existing channels; change
fetchPage to propagate errors (e.g., return Result<List<Channel>> or throw)
instead of emptyList() so callers can detect failure, update getRemoteCids to
stop and propagate that failure (e.g., return null or Result) when
client.queryChannelsInternal(request).await() yields Result.Failure, and change
updateOnlineChannels to skip computing cidsToRemove/removeChannels when
getRemoteCids signals failure; update usages of fetchPage, getRemoteCids, and
updateOnlineChannels accordingly to handle the propagated error path.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTestArguments.kt`:
- Around line 442-461: The test provider queryChannelsWithPredefinedFilterInput
currently doesn't include a predefined_filter in the successful
QueryChannelsResponse, so update the first Arguments success fixture to include
a populated predefined_filter on the QueryChannelsResponse (or on the
ChannelResponse as required by the DTO structure) so the predefined-filter
parsing path is exercised; locate the QueryChannelsResponse construction in
queryChannelsWithPredefinedFilterInput and add a predefined_filter value (use an
existing factory/helper like a Mother.* method or create a minimal
PredefinedFilter DTO instance) to the response payload, leaving the error case
unchanged.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.kt`:
- Around line 207-223: In QueryMode.Predefined the VM never stores the
server-resolved filter/sort causing searchMessagesForQuery and
loadMoreQueryMessages to fall back to defaultChannelListFilter (and channel
search to use optimizedChannelSearchFilter and drop server sort); fix by
capturing the resolved predefined filter and sort from the server response and
persisting them into filterFlow and querySortFlow (instead of leaving the
initial null/default) so all subsequent searches use the backend-resolved spec;
locate where the initial channel query response is handled (the code path that
currently applies optimizedChannelSearchFilter/defaultChannelListFilter) and
update it to set filterFlow.value and querySortFlow.value with the resolved
server filter/sort (set only when mode is QueryMode.Predefined and when a
resolved spec is received, or update on server-driven changes).
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelFactory.kt`:
- Around line 95-119: The predefined-filter secondary constructor of
ChannelListViewModelFactory (the public constructor that builds mode =
QueryMode.Predefined(...)) should be annotated with `@JvmOverloads` to provide
Java-friendly overloads like the primary constructor; add the `@JvmOverloads`
annotation to that constructor declaration so Java callers receive the same
convenient overload set as the other public constructor (keep the existing
parameter list: chatClient, predefinedFilterName, filterValues, sortValues,
channelLimit, memberLimit, messageLimit, chatEventHandlerFactory,
draftMessagesEnabled, messageSearchSort).
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.kt`:
- Around line 560-616: The search logic currently drops the predefined scope
(predefinedFilter, filterValues, sortValues) when handling SearchQuery.Channels,
causing broader backend queries; update the request-building path in
ChannelListViewModel (the code invoked by setSearchQuery / the
QueryChannelsRequest builder) to preserve the existing predefinedFilter,
filterValues and sortValues and to add the autocomplete/name filter as an
additional constraint (e.g., merge into the existing filter as an
AndFilterObject rather than replacing it), so search queries remain scoped to
the predefined filter and only add the name/member autocomplete constraints.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/QueryChannelsRequest.kt`:
- Around line 25-40: Update the KDoc for QueryChannelsRequest to clarify
semantics: change the wording for predefinedFilter to say "server-side
predefined filter name/ID" (match examples like "android_sample_filter");
specify expected shapes and types for filterValues and sortValues (e.g., map of
string keys to primitive/string/nullable values), and explain behavior when they
are null vs empty and whether they must be provided when predefinedFilter is
set; explicitly state that filter and querySort are ignored by the backend but
still observed locally (used to seed QueryChannelsMutableState placeholders)
until the server response arrives; and add a short thread/state note indicating
which thread(s) these objects are safe to construct/consume (main-thread vs
background) and any mutation expectations.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt`:
- Around line 127-134: The normalizeValue function currently handles Lists
recursively but returns Map values unchanged, so nested maps containing
whole-number Doubles won't get converted to Ints; update normalizeValue (in
FilterDomainMapping.kt) to detect Map<*,*> values and recursively normalize
their entries (e.g., map keys preserved, map values passed back into
normalizeValue) so nested structures are normalized consistently, or add a clear
comment in normalizeValue stating maps are intentionally not recursed; ensure
you reference the normalizeValue function and the branch that handles List
values when making the change.
- Around line 86-95: The code currently calls fields.add(fieldName)
unconditionally at the top of parseFieldFilter, which records field names even
when parsing fails; change this so the field is only added when a valid
FilterObject will be returned — move the fields.add(fieldName) call to
immediately before each successful return of a FilterObject (e.g. before
returning Filters.eq(...) for non-map values, and before each branch in the
operatorMap handling that yields a non-null FilterObject such as $exists,
$in/$nin, $autocomplete, or other operators), or alternatively add it once after
the when/operatorMap resolution only if the resolved FilterObject is non-null;
ensure no fields are added on early bailouts (operatorMap.isEmpty(), invalid
types, unknown opKey) so parseFilterMap and fallback-sort logic only see fields
that produced filters.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifier.kt`:
- Around line 54-58: The Predefined data class (Predefined :
QueryChannelsIdentifier) currently stores filterValues and sortValues as
Map<String, Any>?, whose equality can be unstable for values like Array or
custom mutable wrappers; update the class to (1) add KDoc on
filterValues/sortValues documenting that values must be value-equality types
(primitives, String, List, Map, etc.) and thread/state expectations, and (2)
normalize/validate inputs in the constructor (or factory) of Predefined: convert
Array instances to Lists and/or reject unsupported mutable wrappers by throwing
IllegalArgumentException so identity comparisons are stable and you don't end up
with duplicate QueryChannelsLogic/StateRegistry entries. Ensure references to
Predefined, filterValues, sortValues, and QueryChannelsIdentifier are used so
reviewers can locate the change.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/QueryChannelsRepository.kt`:
- Around line 36-41: Update the KDoc for the public suspend function selectBy in
QueryChannelsRepository to explicitly state threading and state guarantees: note
that selectBy is a suspend function and must be called from a coroutine (it is
main-safe but may perform IO), describe whether it returns a snapshot copy or a
live reference of QueryChannelsSpec (e.g., immutable snapshot vs. backing-store
view), and document behavior for missing identifiers (returns null) and any
concurrency guarantees (e.g., consistent view at call time, not automatically
updated). Reference the QueryChannelsIdentifier parameter and QueryChannelsSpec
return type in the KDoc so callers know the exact thread expectations and
returned-state semantics.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/query/QueryChannelsSpec.kt`:
- Around line 36-47: The public KDoc summary for QueryChannelsSpec currently
presents cids and predefinedFilterName/predefined* as if they confer special
immutability/identity beyond normal data class semantics; update the KDoc on
QueryChannelsSpec to remove or de-emphasize cids and the predefined* fields from
the top-line summary and add a concise note stating that the spec instance still
uses normal data class equality/copy semantics and that stable identity for
offline DB usage is enforced via QueryChannelsIdentifier.Predefined which is
derived from predefinedFilterName and predefinedFilterValues (not by the
QueryChannelsSpec instance itself).
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMappingTest.kt`:
- Around line 51-285: Add parameterized negative test cases to
toFilterDomainWithFieldsArguments: include Arguments.of entries where the input
map uses an unknown operator (e.g., mapOf("x" to mapOf("$weirdOp" to 1))),
$exists with non-boolean (mapOf("x" to mapOf("$exists" to "yes"))), $in/$nin
with non-collection (mapOf("x" to mapOf("$in" to "messaging"))), empty logical
lists (mapOf("$and" to emptyList<Any>())), and logical operators whose value is
not a list (mapOf("$and" to mapOf("type" to "messaging"))); for each of these
assert the mapped Filter is null and the tracked field set is empty (match the
existing Arguments.of null expectations in toFilterDomainWithFieldsArguments) so
FilterDomainMapping.kt parsing failures do not leak field names.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifierTest.kt`:
- Around line 81-95: Add a test that pins down behavior for empty-map vs null
predefined values: in QueryChannelsIdentifierTest.kt add a test that constructs
QueryChannelsIdentifier.Predefined("p", emptyMap(), null) and
QueryChannelsIdentifier.Predefined("p", null, null) and then assert the expected
relation (either assertEquals if you decide emptyMap() should be treated the
same as null, or assertNotEquals if you want them distinct). Reference the
Predefined constructor used elsewhere in this file and assert the chosen
behavior so future changes to equality semantics are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c8ba4d5e-552f-4897-84cc-e3bac5d89b3e
📒 Files selected for processing (63)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/ChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/internal/DistinctChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/internal/DistinctChatApiEnabler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/PredefinedFilter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/QueryChannelsRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/models/QueryChannelsResult.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/StateRegistry.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/requests/QueryChannelsRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/response/QueryChannelsResponse.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/internal/NullableMapConverter.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/queryChannels/internal/QueryChannelsEntity.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifier.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryChannelsListenerState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsDatabaseLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/internal/ChatClientStateCalls.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/internal/QueryChannelsMutableState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/sync/internal/SyncManager.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/QueryChannelsRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/noop/NoOpQueryChannelsRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/plugin/MessageDeliveredPlugin.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/plugin/listeners/QueryChannelsListener.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/query/QueryChannelsSpec.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientChannelApiTests.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/Mother.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTestArguments.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/DomainMappingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMappingTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/FilterObjectRoundTripTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/QuerySortByFieldRoundTripTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/database/converter/NullableMapConverterTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/domain/queryChannels/internal/QueryChannelsEntityConverterTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifierTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryChannelsListenerStateTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/internal/LogicRegistryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsDatabaseLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/internal/QueryChannelsMutableStateTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/plugin/MessageDeliveredPluginTest.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.ktstream-chat-android-ui-components-sample/src/main/kotlin/io/getstream/chat/ui/sample/feature/channel/list/ChannelListFragment.ktstream-chat-android-ui-components/api/stream-chat-android-ui-components.apistream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/channels/ChannelListViewModel.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/channels/ChannelListViewModelFactory.ktstream-chat-android-ui-components/src/test/kotlin/io/getstream/chat/android/ui/viewmodels/channels/ChannelListViewModelTest.kt
Comment on lines
+1304
to
+1308
| predefinedFilter = response.predefined_filter?.let { | ||
| val (filter, filterFields) = it.filter.toFilterDomainWithFields() | ||
| ?: return@let null | ||
| val sort = it.sort.toSortDomain() | ||
| ?: defaultPredefinedFilterSort(filterFields) |
There was a problem hiding this comment.
Don’t silently downgrade unsupported predefined metadata.
If toFilterDomainWithFields() or toSortDomain() can’t parse the backend-resolved spec, this code currently turns that into null or a fallback sort and still returns success. In predefined mode that resolved spec becomes the client’s source of truth, so masking parse failures here can persist the wrong filter/sort and break later refresh or pagination behavior. Please distinguish “field absent” from “field present but unparseable” and surface the latter instead of silently degrading it.
Possible direction
- predefinedFilter = response.predefined_filter?.let {
- val (filter, filterFields) = it.filter.toFilterDomainWithFields()
- ?: return@let null
- val sort = it.sort.toSortDomain()
- ?: defaultPredefinedFilterSort(filterFields)
+ predefinedFilter = response.predefined_filter?.let { predefined ->
+ val (filter, filterFields) = requireNotNull(predefined.filter.toFilterDomainWithFields()) {
+ "Unable to parse predefined filter `${predefined.name}`"
+ }
+ val sort = when {
+ predefined.sort == null -> defaultPredefinedFilterSort(filterFields)
+ else -> requireNotNull(predefined.sort.toSortDomain()) {
+ "Unable to parse predefined sort for `${predefined.name}`"
+ }
+ }
PredefinedFilter(
- name = it.name,
+ name = predefined.name,
filter = filter,
sort = sort,
)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.kt`
around lines 1304 - 1308, The predefinedFilter deserialization is silently
degrading when response.predefined_filter exists but
response.predefined_filter.filter.toFilterDomainWithFields() or
response.predefined_filter.sort.toSortDomain() returns null; update the logic in
MoshiChatApi.kt so that if toFilterDomainWithFields() or toSortDomain() fails
(i.e., returns null) you treat that as an error case rather than as "absent" —
surface/propagate an error (or return a failing Result) instead of returning a
null predefinedFilter or applying defaultPredefinedFilterSort(filterFields).
Concretely, replace the return@let null behavior around predefinedFilter with
explicit error handling: detect the difference between a missing
predefined_filter and an unparseable spec, and raise or propagate a parsing
exception (or a failure result) when toFilterDomainWithFields() or
toSortDomain() fails so callers can handle the backend-resolved-but-unparseable
metadata instead of silently degrading it.
Contributor
There was a problem hiding this comment.
The server-side MQ parser also accepts $q, $any, and $path_exists (with $search aliased to $q). FilterDomainMapping does not handle any of these. So this is not theoretical: any predefined filter registered with $q or $any will reach the silent-null path. Worth either teaching the parser these operators, or defining the contract when the resolved filter has a valid shape but uses an operator the parser does not know.
There was a problem hiding this comment.
🧩 Analysis chain
Good confirmation — let me check the current FilterDomainMapping implementation and available FilterObject subtypes to see what would be needed to support those operators.
🏁 Script executed:
#!/bin/bash
# Check the FilterDomainMapping implementation
fd FilterDomainMapping.kt --exec cat {}
echo "---"
# Check FilterObject hierarchy/subclasses to see what types exist
rg -n "class.*FilterObject\|data class.*Filter\b\|object.*Filter\b" --type=kotlin -A1 | grep -v "test\|Test" | head -80
echo "---"
# Check Filters.kt for available filter factory methods
fd Filters.kt --exec cat {}
Length of output: 12245
@andremion, confirmed — the analysis is spot on. Here's what the current code path shows:
In parseFieldFilter, any operator key not in the explicit when branches falls to else -> null. Since $q, $any, and $path_exists have no entries there and no corresponding FilterObject subtypes exist in the Filters/FilterObject model hierarchy, a predefined filter using any of these operators causes toFilterDomainWithFields() to return null, and MoshiChatApi then silently drops the entire predefinedFilter from the result.
There are two realistic paths forward:
Option A — Opaque passthrough (lower cost, handles future unknowns too)
Add an internal UnknownOperatorFilterObject(val fieldName: String, val raw: Map<String, Any>) that serializes back to the same raw map in toMap(). Unknown operator keys in parseFieldFilter would produce this type instead of null, and the client can still send the filter back to the server faithfully on refresh/pagination even if it can't introspect it locally.
Option B — Typed support for the missing operators (higher cost, better introspection)
Add FilterObject subtypes (e.g., QueryFilterObject for $q, AnyFilterObject for $any, PathExistsFilterObject for $path_exists) to the public Filters API, update toMap() for serialization, and teach FilterDomainMapping to parse them. This is a larger public API change but gives the client full visibility into the filter's semantics.
Given that the goal here is just to avoid silent data loss and keep pagination/refresh working, Option A is the pragmatic fix for this PR — it also acts as a safety net for any other operators added on the backend in the future. Option B can follow up as a dedicated feature.
Would you like me to open a follow-up GitHub issue to track the missing operator support ($q, $any, $path_exists) and the opaque passthrough contract?
✏️ Learnings added
Learnt from: andremion
Repo: GetStream/stream-chat-android PR: 6415
File: stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.kt:1304-1308
Timestamp: 2026-05-11T10:11:02.173Z
Learning: In GetStream/stream-chat-android, FilterDomainMapping (stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/FilterDomainMapping.kt) does not handle the backend MQ operators $q, $any, $path_exists (or $search as alias for $q). These operators are supported by the server-side MQ parser (utils/mq/operator.go). There are no corresponding FilterObject subtypes in the Filters/FilterObject hierarchy for these operators. Unknown operators fall to the `else -> null` branch in parseFieldFilter(), making toFilterDomainWithFields() return null and causing MoshiChatApi to silently drop the predefinedFilter. A recommended fix is to add an opaque passthrough FilterObject for unknown operators to preserve round-trip serialization.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: VelikovPetar
Repo: GetStream/stream-chat-android PR: 6152
File: stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelEventHandlerImpl.kt:194-212
Timestamp: 2026-02-17T13:56:11.136Z
Learning: In Kotlin code handling channel events, specifically MessageUpdatedEvent, prefer using server-provided data for replyTo rather than enriching it from local state as done in ChannelEventHandlerImpl. This should be treated as the source of truth. Ensure code comments document this design decision and that tests validate server data is used for replyTo. If there are any places that historically enriched replyTo from local state (e.g., ChannelEventHandlerLegacyImpl), consider aligning those implementations or adding a clear rationale in design docs to avoid future regressions.
Learnt from: andremion
Repo: GetStream/stream-chat-android PR: 6301
File: stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/channel/info/AddMembersViewAction.kt:19-25
Timestamp: 2026-03-30T09:19:45.796Z
Learning: In GetStream/stream-chat-android SDK modules that set a module-level Kotlin compiler argument (via build.gradle.kts) like `-opt-in=io.getstream.chat.android.core.internal.InternalStreamChatApi`, do not require per-file opt-in annotations. Specifically, don’t flag missing `file:OptIn(InternalStreamChatApi::class)` in `.kt` files within those modules when using `InternalStreamChatApi` types, since the opt-in is already applied at the module build level. Only flag missing per-file opt-in in modules that do NOT configure this compiler arg.
Learnt from: gpunto
Repo: GetStream/stream-chat-android PR: 6328
File: stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessagePositionHandler.kt:19-28
Timestamp: 2026-04-02T14:29:26.633Z
Learning: In Kotlin, if a `private const val` is declared inside a `companion object`, it is directly accessible within the same `companion object` body (where it’s declared) without any import. During review, don’t claim an import is required or that an import would cause a compile error for usage inside that same companion object in the same file; only flag missing/incorrect imports when code is referencing the constant from outside the companion object.
Contributor
Author
There was a problem hiding this comment.
The response is guaranteed to contain filter, but not sort (see specs). So I think we should fail on filter, but not on sort - and use the BE provided fallback.
Concerning the missing filter types: I think we should do this in a follow up: We currently don't support those in the upstream case as well (Filters/FilterObject), so parsing them currently doesn't really make sense.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
andremion
reviewed
May 11, 2026
Contributor
|
Two test gaps worth covering (no inline anchor):
The search-related and unknown-operator test gaps are already noted in CodeRabbit's threads on |
andremion
reviewed
May 11, 2026
andremion
approved these changes
May 11, 2026
Contributor
Author
Hi @andremion, concerning this remark:
Do you think we should harden this logic to also ensure proper round-trip parsing, to bypass the Moshi Double parsing limitation? |
|
Contributor
No, I wouldn't harden it. The current code is correct as long as identity always flows from the live user-provided maps, which it does. Hardening generateId to normalize values would add complexity for a problem that doesn't exist today. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.Summary by CodeRabbit
New Features
Bug Fixes & Improvements