OpenAPI: Migrate member endpoints#4173
Conversation
📝 WalkthroughWalkthroughThe PR migrates member listing and partial member updates to OpenAPI-generated request and response models, updates database persistence and offline endpoint handling, and revises controllers, workers, mocks, fixtures, and tests accordingly. ChangesMembers API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MemberList
participant ChannelMemberListUpdater
participant APIClient
participant DatabaseSession
MemberList->>ChannelMemberListUpdater: load(query)
ChannelMemberListUpdater->>APIClient: queryMembers(payload)
APIClient-->>ChannelMemberListUpdater: MembersResponse
ChannelMemberListUpdater->>DatabaseSession: saveMembers(response)
DatabaseSession-->>MemberList: updated member state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Generated by 🚫 Danger |
Public Interface🚀 No changes affecting the public interface. |
| memberUpdater.partialUpdate( | ||
| userId: userId, | ||
| in: cid, | ||
| updates: MemberUpdatePayload(extraData: extraData), |
There was a problem hiding this comment.
I did not touch public interfaces at the moment because there are 2 different arguments instead of single argument with request type - set and unset as arguments. Alternative is to add new functions which allow creating the UpdateMemberPartialRequest on the public layer. I think it is OK to keep this as is.
func partialUpdate(
extraData: [String: RawJSON]?,
unsetProperties: [String]? = nil,
completion: (@MainActor (Result<ChatChannelMember, Error>) -> Void)? = nil
) {
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelMemberUpdater_Mock.swift (2)
80-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
@Sendableto the overridden completion parameter.As per coding guidelines, maintain Swift 6.0 strict-concurrency compatibility. The base class signature requires the
completionclosure to be@Sendable, and omitting it here creates a strict-concurrency mismatch.♻️ Proposed refactor
request: UpdateMemberPartialRequest, - completion: `@escaping` ((Result<ChatChannelMember, Error>) -> Void) + completion: `@escaping` (`@Sendable` (Result<ChatChannelMember, Error>) -> Void) ) {🤖 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 `@TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelMemberUpdater_Mock.swift` around lines 80 - 82, The overridden completion parameter in ChannelMemberUpdater_Mock’s update-member method must match the base class’s strict-concurrency signature. Add `@Sendable` to the Result<ChatChannelMember, Error> completion closure while preserving the existing method behavior.Source: Coding guidelines
25-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
@Sendableto the completion closure to match the base class.As per coding guidelines, maintain Swift 6.0 strict-concurrency compatibility by using
Sendablewhere needed. TheChannelMemberUpdaterbase class defines the completion closure as@Sendable, so the mock should match it exactly.♻️ Proposed refactor
`@Atomic` var partialUpdate_request: UpdateMemberPartialRequest? - `@Atomic` var partialUpdate_completion: ((Result<ChatChannelMember, Error>) -> Void)? + `@Atomic` var partialUpdate_completion: (`@Sendable` (Result<ChatChannelMember, Error>) -> Void)?🤖 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 `@TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelMemberUpdater_Mock.swift` around lines 25 - 26, Update the partialUpdate_completion closure property in the ChannelMemberUpdater mock to include `@Sendable`, matching the completion closure declaration in the base ChannelMemberUpdater class while preserving its existing Result type and behavior.Source: Coding guidelines
🤖 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 `@Sources/StreamChat/Database/DTOs/MemberModelDTO.swift`:
- Around line 237-239: Update the member-list persistence flow around saveMember
so it pre-warms and reuses a cache mapping for all response.members, rather than
passing cache: nil for each member. Extend MembersResponse with a mapping
mechanism analogous to ChannelMemberListPayload.getPayloadToModelIdMappings,
then pass the resulting cache into each saveMember call while preserving the
existing compactMapLoggingError behavior.
In `@Sources/StreamChat/Query/Filter.swift`:
- Around line 678-679: Remove the documentation comment immediately above
Filter.toRawJSONDictionary() and leave the method implementation unchanged.
---
Nitpick comments:
In
`@TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelMemberUpdater_Mock.swift`:
- Around line 80-82: The overridden completion parameter in
ChannelMemberUpdater_Mock’s update-member method must match the base class’s
strict-concurrency signature. Add `@Sendable` to the Result<ChatChannelMember,
Error> completion closure while preserving the existing method behavior.
- Around line 25-26: Update the partialUpdate_completion closure property in the
ChannelMemberUpdater mock to include `@Sendable`, matching the completion closure
declaration in the base ChannelMemberUpdater class while preserving its existing
Result type and behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b7c266e-5a60-4f6a-9d5d-efb25865aa47
⛔ Files ignored due to path filters (8)
Sources/StreamChat/Generated/OpenAPI/APIs/DefaultEndpoints.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/ChannelMemberRequest.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/ChannelMemberResponse.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/MembersResponse.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/QueryMembersPayload.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/SortParamRequest.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/UpdateMemberPartialRequest.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/UpdateMemberPartialResponse.swiftis excluded by!**/generated/**
📒 Files selected for processing (31)
Scripts/openapi_generate.shSources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swiftSources/StreamChat/APIClient/Endpoints/MemberEndpoints.swiftSources/StreamChat/APIClient/Endpoints/Payloads/MemberUpdatePayload.swiftSources/StreamChat/APIClient/Endpoints/Requests/QueryPollsRequestBody.swiftSources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swiftSources/StreamChat/Controllers/MemberController/MemberController.swiftSources/StreamChat/Database/DTOs/MemberModelDTO.swiftSources/StreamChat/Database/DTOs/UserDTO.swiftSources/StreamChat/Database/DatabaseSession.swiftSources/StreamChat/Models/Member.swiftSources/StreamChat/Query/ChannelMemberListQuery.swiftSources/StreamChat/Query/Filter.swiftSources/StreamChat/Query/Sorting/ChannelMemberListSortingKey.swiftSources/StreamChat/Workers/ChannelMemberListUpdater.swiftSources/StreamChat/Workers/ChannelMemberUpdater.swiftTestTools/StreamChatTestTools/Assertions/AssertJSONEqual.swiftTestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swiftTestTools/StreamChatTestTools/Mocks/StreamChat/Database/DatabaseSession_Mock.swiftTestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelMemberUpdater_Mock.swiftTestTools/StreamChatTestTools/TestData/DummyData/ChannelMemberResponse+Dummy.swiftTestTools/StreamChatTestTools/TestData/FilterTestScope.swiftTests/StreamChatTests/APIClient/Endpoints/EndpointPath_Tests.swiftTests/StreamChatTests/APIClient/Endpoints/MemberEndpoints_Tests.swiftTests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swiftTests/StreamChatTests/Controllers/MemberController/MemberController_Tests.swiftTests/StreamChatTests/Query/Filter_Tests.swiftTests/StreamChatTests/StateLayer/Chat_Tests.swiftTests/StreamChatTests/StateLayer/MemberList_Tests.swiftTests/StreamChatTests/Workers/ChannelMemberListUpdater_Tests.swiftTests/StreamChatTests/Workers/ChannelMemberUpdater_Tests.swift
💤 Files with no reviewable changes (3)
- Sources/StreamChat/APIClient/Endpoints/Payloads/MemberUpdatePayload.swift
- Sources/StreamChat/APIClient/Endpoints/MemberEndpoints.swift
- Sources/StreamChat/APIClient/Endpoints/Requests/QueryPollsRequestBody.swift
| return response.members.compactMapLoggingError { | ||
| try saveMember(response: $0, channelId: channelId, query: query, cache: nil) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Pre-warm the cache to prevent N+1 queries.
Passing cache: nil bypasses Core Data pre-warming. When saving a paginated list of members, this will cause a separate fetch query for every individual member, leading to an N+1 performance regression.
Consider extending MembersResponse with a cache mapping mechanism analogous to getPayloadToModelIdMappings (used by ChannelMemberListPayload) to restore this database optimization.
🤖 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 `@Sources/StreamChat/Database/DTOs/MemberModelDTO.swift` around lines 237 -
239, Update the member-list persistence flow around saveMember so it pre-warms
and reuses a cache mapping for all response.members, rather than passing cache:
nil for each member. Extend MembersResponse with a mapping mechanism analogous
to ChannelMemberListPayload.getPayloadToModelIdMappings, then pass the resulting
cache into each saveMember call while preserving the existing
compactMapLoggingError behavior.
| /// Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies. | ||
| func toRawJSONDictionary() -> [String: RawJSON] { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove doc comment from internal method.
As per coding guidelines, do not add /// doc comments to internal, private, or test code. toRawJSONDictionary() is an internal method, so a standard comment should be used instead.
♻️ Proposed fix
- /// Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies.
+ // Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies.
func toRawJSONDictionary() -> [String: RawJSON] {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies. | |
| func toRawJSONDictionary() -> [String: RawJSON] { | |
| // Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies. | |
| func toRawJSONDictionary() -> [String: RawJSON] { |
🤖 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 `@Sources/StreamChat/Query/Filter.swift` around lines 678 - 679, Remove the
documentation comment immediately above Filter.toRawJSONDictionary() and leave
the method implementation unchanged.
Source: Coding guidelines
| func saveMember( | ||
| response: ChannelMemberResponse, | ||
| channelId: ChannelId, | ||
| query: ChannelMemberListQuery?, | ||
| cache: PreWarmedCache? | ||
| ) throws -> MemberDTO { |
There was a problem hiding this comment.
Decided to add additional save methods instead of converting new ChannelMemberResponse to MemberPayload just for reusing the save. At the end of the migration, saveMember with MemberPayload argument would get deleted. Felt cleaner to keep them separate. WDYT
SDK Size
|
|
|
||
| import Foundation | ||
|
|
||
| final class ChannelMemberResponse: Sendable, Codable, JSONEncodable { |
There was a problem hiding this comment.
I should consider making this public instead of ChatChannelMember public type. Is it possible?
There was a problem hiding this comment.
Not yet possible, these are too different at the moment. OpenAPI has UserResponse, not ChatUser. So ChatUser should be replaced first with OpenAPI type.
| } | ||
|
|
||
| /// Converts the filter to a `RawJSON` dictionary for OpenAPI request bodies. | ||
| func toRawJSONDictionary() -> [String: RawJSON] { |
There was a problem hiding this comment.
This is what OpenAPI models require, filter is fed in with [String: RawJSON]
let filterConditions: [String: RawJSON]
Alternative is going into generator and making it to accept Encodable, but then it has implications to other SDKs as well (Feeds) or it should be configurable on the generator level. WDYT?
This is what currently is used for encoding filters. Now we are moving toward using [String: RawJSON] which itself is encodable.
// pre this PR
extension Filter: Codable {
public func encode(to encoder: Encoder) throws {
if self.operator.isGroupOperator {
// Filters with group operators are encoded in the following form:
// { $<operator>: [ <filter 1>, <filter 2> ] }
try [self.operator: AnyEncodable(value)].encode(to: encoder)
return
} else if let key = self.key {
// Normal filters are encoded in the following form:
// { key: { $<operator>: <value> } }
try [key: [self.operator: AnyEncodable(value)]].encode(to: encoder)
return
} else {
throw EncodingError.invalidValue(
self,
EncodingError.Context(
codingPath: encoder.codingPath,
debugDescription: "Filter must have the `key` value when the operator is not a group operator."
)
)
}
}
StreamChat XCSize
Show 26 more objects
|
| case let value as ChannelId: | ||
| return .string(value.rawValue) | ||
| case let value as ChannelType: | ||
| return .string(value.rawValue) |
There was a problem hiding this comment.
These should probably be RawRepresentable
|
2 participants
🔗 Issue Links
Resolves: IOS-1829
🎯 Goal
Migrate the member query and partial member update endpoints from the hand-written v1 implementation to the OpenAPI-generated v2 endpoints and models.
📝 Summary
channelMembers→queryMembers(GET /api/v2/chat/members) andpartialMemberUpdate→updateMemberPartial(PATCH /api/v2/chat/channels/{type}/{id}/member) at the generated v2 operations, returning the generatedMembersResponse/ChannelMemberResponse/UpdateMemberPartialResponse.DatabaseSessionsave methods (saveUser(response:),saveMember(response:),saveMembers(response:)), alongside the existingMemberPayload/UserPayloadones which are still used by WebSocket events.MemberEndpointsandMemberUpdatePayload; callers now build the generatedUpdateMemberPartialRequestdirectly.ChannelMemberListQueryinto the generatedQueryMembersPayloadvia a newFilter.toRawJSONDictionary()helper.🛠 Implementation
MemberPayload/UserPayload, so those types remain; only the REST endpoints switch to the generated models. Public types (ChatChannelMember,MemberRole,MemberInfo) are unchanged.updateMemberPartialself-membership narrowing. The v2 path dropsuserId(channels/{type}/{id}/member); the public/workerpartialUpdate(...)signatures keepuserIdfor source compatibility but no longer send it, matching the v2 contract.Filter→[String: RawJSON]. PortedtoRawJSONDictionary()/FilterValue.rawJSON/MemberRole.filterRawValueand addedChannelMemberListSortingKey.remoteKey, soChannelMemberListQuery.asQueryMembersPayload()produces the generated request. This is the same conversion the wider OpenAPI migration branch uses.SortParamRequestcollision. The generatedSortParamRequestcollided with the hand-written poll one; renamed the poll-local precursor toPollsSortParamRequest.queryMembersserializes its request as apayloadquery-item string with non-deterministic key order, soCompareJSONEqualnow compares string values that are themselves JSON objects structurally (order-independent) — keeping fullAnyEndpointequality in endpoint tests. AddedFiltertests for the newtoRawJSONDictionaryconversion.🎨 Showcase
Not applicable — no UI changes.
🧪 Manual Testing Notes
Open debug menu and use show channel members item, members appear.
Try archiving a channel using the debug menu, channel appears under archived channel list query (button in the toolbar for channel list queries).
☑️ Contributor Checklist
docs-contentrepo