Added a new upload endpoint #3788
Conversation
WalkthroughAdds a new standalone attachment upload API across models, clients, endpoints, and uploaders. Introduces StreamAttachment generic model, new CDN/client methods, new endpoint/path variant for standalone uploads, adjusts offline queuing, and updates tests and mocks. Refactors upload helper to accept endpoints. Project file updated to include a new mock. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as ChatClient
participant Uploader as AttachmentUploader
participant CDN as CDNClient
participant Net as Network
rect rgb(245,250,255)
note right of Client: Standalone upload (no channel context)
Client->>Uploader: uploadStandaloneAttachment(StreamAttachment, progress?)
Uploader->>CDN: uploadStandaloneAttachment(...)
CDN->>CDN: Build Endpoint.uploadAttachment(type)
CDN->>Net: multipart/form-data upload
Net-->>CDN: Response (success/error)
CDN-->>Uploader: Result<UploadedFile, Error>
Uploader-->>Client: Result<UploadedFile, Error>
end
alt Progress updates
CDN-->>Client: progress(Double)
end
opt Missing file
note over CDN: Return ClientError.Unknown()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Generated by 🚫 Danger |
Public Interface+ public struct StreamAttachment
+
+ public let type: AttachmentType
+ public var payload: Payload
+ public let downloadingState: AttachmentDownloadingState?
+ public let uploadingState: AttachmentUploadingState?
+
+
+ public init(type: AttachmentType,payload: Payload,downloadingState: AttachmentDownloadingState?,uploadingState: AttachmentUploadingState?)
public class StreamAttachmentUploader: AttachmentUploader
+ public func uploadStandaloneAttachment(_ attachment: StreamAttachment<Payload>,progress: ((Double) -> Void)?,completion: @escaping (Result<UploadedFile, Error>) -> Void)
public struct AttachmentUploadingState: Hashable
+
+
+ public init(localFileURL: URL,state: LocalAttachmentState,file: AttachmentFile)
public class ChatClient
+ public func upload(_ attachment: StreamAttachment<Payload>,progress: ((Double) -> Void)?,completion: @escaping (Result<UploadedFile, Error>) -> Void) |
SDK Size
|
SDK Performance
|
|
|
||
| import Foundation | ||
|
|
||
| public struct StreamAttachment<Payload> { |
There was a problem hiding this comment.
The Stream prefix could be a bit generic, and it does not give enough context on how this is used. What about this:
| public struct StreamAttachment<Payload> { | |
| /// An attachment that is not assigned to any message or channel. | |
| public struct StandaloneAttachment<Payload> { |
There was a problem hiding this comment.
Another option is to try to just reuse the API from ChannelUpdater.uploadFile(), and not expose any new type 🤔
There was a problem hiding this comment.
About the naming, I would actually prefer to keep StreamAttachment. The reason is that on other products, all attachments would be standalone - not tied to channels, posts, etc. And it matches the API one, which is just Attachment.
SDK Size
|
![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
♻️ Duplicate comments (1)
Sources/StreamChat/Models/Attachments/ChatMessageAttachment.swift (1)
7-38: Name discussion follow-up: provide a compatibility aliasThere was prior debate about naming. If you’d like to keep
StreamAttachmentbut still accommodate discoverability for “standalone” usage, add a public typealias:+/// An attachment that is not assigned to any message or channel. +public typealias StandaloneAttachment<Payload> = StreamAttachment<Payload>
🧹 Nitpick comments (26)
StreamChat.xcodeproj/project.pbxproj (1)
3797-3797: Optional: add fileEncoding for consistency with neighboring entries.Not required, but many Swift file references here include
fileEncoding = 4;. Keeping this consistent can reduce noisy diffs later.Apply this minimal change:
- 84BC99272E5C56590000FB87 /* StreamAttachment_Mock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StreamAttachment_Mock.swift; sourceTree = "<group>"; }; + 84BC99272E5C56590000FB87 /* StreamAttachment_Mock.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StreamAttachment_Mock.swift; sourceTree = "<group>"; };Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift (1)
39-41: Type-safety: prefer AttachmentType over String fortypeto prevent path typosEndpoint construction already maps
AttachmentTypeto"image"/"file". AcceptingAttachmentTypedirectly at the enum level removes a class of bugs where callers might pass arbitrary strings.Example:
- case uploadChannelAttachment(channelId: String, type: String) - case uploadAttachment(String) + case uploadChannelAttachment(channelId: String, type: AttachmentType) + case uploadAttachment(AttachmentType) @@ - case let .uploadChannelAttachment(channelId, type): return "channels/\(channelId)/\(type)" - case let .uploadAttachment(type): return "uploads/\(type)" + case let .uploadChannelAttachment(channelId, type): return "channels/\(channelId)/\(type == .image ? "image" : "file")" + case let .uploadAttachment(type): return "uploads/\(type == .image ? "image" : "file")"Also applies to: 129-131
Sources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swift (1)
14-19: Uploads are classified as NOT queued offline; verify intent for both channel-scoped and standalone uploadsBoth
.uploadChannelAttachmentand.uploadAttachmentcurrently returnfalseforshouldBeQueuedOffline. If the product expectation is to enqueue channel-scoped uploads for later execution when back online, this needs adjustment and tests. If uploads must never be queued (due to size or backend constraints), consider adding a comment explaining the rationale to avoid regressions.Recommend adding explicit tests in
EndpointPath_Teststo lock the intended behavior.Proposed tests are included in my comments on
EndpointPath_Tests.swift.Sources/StreamChat/Models/Attachments/ChatMessageAttachment.swift (2)
19-26: Doc comment mentions “message” on a standalone attachment; rephrase to avoid confusion
StreamAttachmentisn’t tied to a message, yet the doc states “Becomes nil when the message … is sent.” Consider wording that reflects standalone uploads.Suggested tweak:
- /// Becomes `nil` when the message with the current attachment is sent. + /// For message-bound flows, this may be set to `nil` after the message is sent. + /// For standalone uploads, this may be set to `nil` after the upload completes.
7-38: Consider conditional Sendable conformance for concurrency safetyIf the SDK is adopting Swift Concurrency, making attachments Sendable where the payload is Sendable helps avoid thread-safety warnings across async boundaries.
Example:
-public struct StreamAttachment<Payload> { +public struct StreamAttachment<Payload>: @unchecked Sendable where Payload: Sendable {You may need to audit internals (the URL/file types are already Sendable-compatible). If you prefer stricter guarantees, drop
@uncheckedand rely on compiler checks.TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift (1)
17-21: Avoid empty stub: call completion with a failure to prevent hangs when used in testsIf a test accidentally calls this method, the current no-op leaves the completion uninvoked, potentially hanging the test. Return a predictable failure instead.
public func uploadStandaloneAttachment<Payload>( _ attachment: StreamChat.StreamAttachment<Payload>, progress: ((Double) -> Void)?, completion: @escaping (Result<StreamChat.UploadedFile, any Error>) -> Void - ) {} + ) { + completion(.failure(NSError(domain: "CustomCDNClient", code: -1, userInfo: [NSLocalizedDescriptionKey: "Not implemented in CustomCDNClient"]))) + }Tests/StreamChatTests/APIClient/Endpoints/EndpointPath_Tests.swift (1)
98-157: Add coverage for both upload path variants (value + codable + offline-queue semantics)The new
.uploadAttachment(standalone) path isn’t covered, and the channel-scoped variant lacks explicit assertions forvalueandshouldBeQueuedOffline. Adding these tests will lock in intended behavior and protect against regressions.@@ func test_isProperlyEncodedAndDecoded() throws { @@ - assertResultEncodingAndDecoding(.pinnedMessages("channel_idq")) - assertResultEncodingAndDecoding(.uploadChannelAttachment(channelId: "channel_id", type: "file")) + assertResultEncodingAndDecoding(.pinnedMessages("channel_idq")) + // Channel-scoped upload + assertResultEncodingAndDecoding(.uploadChannelAttachment(channelId: "channel_id", type: "file")) + // Standalone upload (uploads/{type}) + assertResultEncodingAndDecoding(.uploadAttachment("file")) @@ } + + // MARK: - Uploads + + func test_uploadChannelAttachment_value_and_offlineQueue() { + XCTAssertEqual( + EndpointPath.uploadChannelAttachment(channelId: "chan/type=id", type: "file").value, + "channels/chan/type=id/file" + ) + XCTAssertFalse(EndpointPath.uploadChannelAttachment(channelId: "channel_id", type: "image").shouldBeQueuedOffline) + } + + func test_uploadAttachment_value_and_offlineQueue() { + XCTAssertEqual(EndpointPath.uploadAttachment("file").value, "uploads/file") + XCTAssertFalse(EndpointPath.uploadAttachment("image").shouldBeQueuedOffline) + }TestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swift (1)
1-4: Nit: filename typoThe filename is
EndpoinPath+Equatable.swift(missing “t”). Consider renaming toEndpointPath+Equatable.swiftfor consistency with the type name.Tests/StreamChatTests/APIClient/Endpoints/AttachmentEndpoints_Tests.swift (1)
25-38: Broaden coverage to include the new standalone upload pathThis test verifies the channel-scoped path. Please add a companion test that asserts
.uploadAttachment(type:)buildsuploads/{type}and does not require a connection id. Include all relevantAttachmentTypes (e.g.,.file,.voiceRecording) to lock the mapping.Here’s a test you can drop into this file:
+ func test_uploadStandaloneAttachment_buildsCorrectly() { + let testCases: [AttachmentType: String] = [ + .image: "image", + .video: "file", + .audio: "file", + .file: "file", + .voiceRecording: "file" + ] + + for (type, pathComponent) in testCases { + let expectedEndpoint: Endpoint<FileUploadPayload> = .init( + path: .uploadAttachment(type: pathComponent), + method: .post, + queryItems: nil, + requiresConnectionId: false, + body: nil + ) + + let endpoint: Endpoint<FileUploadPayload> = .uploadAttachment(type: type) + + XCTAssertEqual(AnyEndpoint(expectedEndpoint), AnyEndpoint(endpoint)) + XCTAssertEqual("uploads/\(pathComponent)", endpoint.path.value) + } + }Sources/StreamChat/ChatClient.swift (2)
626-645: Clarify progress semantics in the public API docsThe tests use
expectedProgress = 20, which implies the progress closure receives percentages (0...100). Please document the expected range explicitly to avoid ambiguity.Apply this doc tweak:
- /// - progress: the progress of the upload. + /// - progress: called with upload progress as a percentage in the range 0...100.
626-645: Add async/await overload for consistency with the rest of the APIMost
ChatClientAPIs offer async variants. Provide an async overload returningUploadedFileto match that pattern.Apply this addition below the existing method:
public func upload<Payload>( _ attachment: StreamAttachment<Payload>, progress: ((Double) -> Void)?, completion: @escaping (Result<UploadedFile, Error>) -> Void ) { apiClient.attachmentUploader.uploadStandaloneAttachment( attachment, progress: progress, completion: completion ) } + + /// Async variant of `upload` that returns the uploaded file. + @discardableResult + public func upload<Payload>( + _ attachment: StreamAttachment<Payload>, + progress: ((Double) -> Void)? = nil + ) async throws -> UploadedFile { + try await withCheckedThrowingContinuation { continuation in + upload(attachment, progress: progress) { result in + continuation.resume(with: result) + } + } + }Tests/StreamChatTests/APIClient/StreamAttachmentUploader_Tests.swift (2)
65-90: Avoidtry!and ensure the mock uses a local file URL used by the uploaderUse a safe unwrap for the test data and wire the mock’s
assetURLto the local file. This makes the test resilient if the implementation reads fromuploadingState.localFileURL.Apply this diff:
- let data = try! Data(contentsOf: expectedUrl) - let mockedAttachment = StreamAttachment.mock(payload: data) + guard let data = try? Data(contentsOf: expectedUrl) else { + return XCTFail("Failed to load test data from \(expectedUrl)") + } + let mockedAttachment = StreamAttachment.mock(payload: data, assetURL: expectedUrl)
92-113: Mirror the same safety in the error-path testSame reasoning as above.
Apply this diff:
- let expectedUrl = URL.localYodaImage - let data = try! Data(contentsOf: expectedUrl) - let mockedAttachment = StreamAttachment.mock(payload: data) + let expectedUrl = URL.localYodaImage + guard let data = try? Data(contentsOf: expectedUrl) else { + return XCTFail("Failed to load test data from \(expectedUrl)") + } + let mockedAttachment = StreamAttachment.mock(payload: data, assetURL: expectedUrl)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (1)
32-47: Ensure completion is always called to avoid hanging testsIf
uploadAttachmentResultis not set, tests can deadlock waiting for completion. Provide a fallback error completion. Also consider consistently delivering the progress and completion on the main queue.Apply this diff:
if let uploadAttachmentResult = uploadAttachmentResult { DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { completion(uploadAttachmentResult.map { UploadedFile(fileURL: $0) }) } - } + } else { + DispatchQueue.main.async { + completion(.failure(ClientError("uploadAttachmentResult not set in CDNClient_Spy"))) + } + }Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (1)
18-26: Prefer an explicit type mapping helper over inline ternary for future-proofingUsing a dedicated mapper makes it clearer how new AttachmentType cases should be routed (today: image -> "image", everything else -> "file") and avoids duplicating the logic in multiple places.
Apply this refactor:
extension Endpoint { - static func uploadAttachment(type: AttachmentType) -> Endpoint<FileUploadPayload> { - .init( - path: .uploadAttachment(type == .image ? "image" : "file"), - method: .post, - queryItems: nil, - requiresConnectionId: false, - body: nil - ) - } + static func uploadAttachment(type: AttachmentType) -> Endpoint<FileUploadPayload> { + .init( + path: .uploadAttachment(uploadPathComponent(for: type)), + method: .post, + queryItems: nil, + requiresConnectionId: false, + body: nil + ) + } + + // MARK: - Private + private static func uploadPathComponent(for type: AttachmentType) -> String { + switch type { + case .image: return "image" + default: return "file" + } + } }And similarly deduplicate the mapping in the channel-scoped variant:
- path: .uploadChannelAttachment(channelId: cid.apiPath, type: type == .image ? "image" : "file"), + path: .uploadChannelAttachment(channelId: cid.apiPath, type: uploadPathComponent(for: type)),TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (2)
32-36: Align completion type with the protocol (drop ‘any’ for consistency)The AttachmentUploader protocol uses Result<UploadedFile, Error>. Using Result<UploadedFile, any Error> here is needlessly inconsistent and may trigger existential warnings.
- func uploadStandaloneAttachment<Payload>( + func uploadStandaloneAttachment<Payload>( _ attachment: StreamAttachment<Payload>, progress: ((Double) -> Void)?, - completion: @escaping (Result<UploadedFile, any Error>) -> Void + completion: @escaping (Result<UploadedFile, Error>) -> Void ) {
43-46: Preserve thumbnailURL when mapping UploadedAttachment → UploadedFileWe’re currently dropping thumbnailURL in the spy. Keeping it mirrors production behavior and avoids false negatives in tests relying on thumbnails.
- completion(uploadAttachmentResult.map { UploadedFile(fileURL: $0.remoteURL )}) + completion(uploadAttachmentResult.map { UploadedFile(fileURL: $0.remoteURL, thumbnailURL: $0.thumbnailURL) })Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
19-30: Doc comment mismatches the actual result typeThe method returns UploadedFile (URLs), not “the uploaded attachment”. Adjust wording to avoid confusion for SDK users and contributors.
- /// Uploads a standalone attachment (not tied to message or channel), and returns the attachment with the remote information. + /// Uploads a standalone attachment (not tied to a message or channel) and returns the uploaded file information (URLs). @@ - /// - completion: The callback with the uploaded attachment. + /// - completion: The callback with the uploaded file information (file and optional thumbnail URLs).TestTools/StreamChatTestTools/Mocks/Models + Extensions/Attachments/StreamAttachment_Mock.swift (2)
10-19: Tighten the mock API: unused params and fixed type
- title and extraData aren’t used.
- The attachment type is hardcoded to .image, limiting reuse (e.g., file uploads).
Consider making type configurable and dropping unused params for clarity.
Proposed refactor:
- static func mock( - payload: Payload, - title: String = "Sample.png", - assetURL: URL = URL(string: "http://asset.url")!, - file: AttachmentFile = AttachmentFile(type: .png, size: 120, mimeType: "image/png"), - localState: LocalAttachmentState? = .uploaded, - localDownloadState: LocalAttachmentDownloadState? = nil, - uploadingState: AttachmentUploadingState? = nil, - extraData: [String: RawJSON]? = nil - ) -> Self { + static func mock( + payload: Payload, + type: AttachmentType = .image, + assetURL: URL = URL(string: "http://asset.url")!, + file: AttachmentFile = .init(type: .png, size: 120, mimeType: "image/png"), + localState: LocalAttachmentState? = .uploaded, + localDownloadState: LocalAttachmentDownloadState? = nil, + uploadingState: AttachmentUploadingState? = nil + ) -> Self {And set the type:
- type: .image, + type: type,If you prefer to keep backward compatibility, keep title/extraData but underscore them to silence “unused” warnings.
30-37: Default localState = .uploaded can mask missing uploadingState in testsDefaulting to .uploaded auto-creates an uploadingState, which may hide test setup issues. Consider defaulting localState to .pendingUpload to reflect typical pre-upload state. Optional; current tests pass explicit uploadingState anyway.
Tests/StreamChatTests/APIClient/StreamCDNClient_Tests.swift (3)
76-104: Test name/body mismatch: this validates standalone, not channel-scoped uploadsThe body calls client.uploadStandaloneAttachment but the name is test_uploadFileEncoderFailingToEncode. Rename for clarity.
-func test_uploadFileEncoderFailingToEncode() throws { +func test_standaloneUploadFileEncoderFailingToEncode() throws {
106-129: Test name/body mismatch (the other way around)This one calls client.uploadAttachment (channel-scoped) but is named test_uploadStandaloneFileEncoderFailingToEncode. Rename to match.
-func test_uploadStandaloneFileEncoderFailingToEncode() throws { +func test_uploadFileEncoderFailingToEncode() throws {
173-215: Success path for standalone uploads looks correct; fix misleading inline commentThe network and decoder stubs are exercised; final assertion checks decoder output. The comment at Line 198 says “assert the result is failure” but the test asserts success.
- // Create a request and assert the result is failure + // Create a request and wait for the completion; assert success mappingSources/StreamChat/APIClient/CDNClient/CDNClient.swift (3)
43-53: Public API addition for standalone uploads: clarify callback threading and ergonomicsThe new protocol API looks good. Two small nits to tighten it up:
- Document the callback threads for
progress/completion(they’re currently invoked from URLSession’s internal queue, not main). Callers often assume main-thread delivery for UI updates.- Consider adding a protocol extension overload that defaults
progresstonilfor nicer call sites.
129-149: Align error semantics and add context logging for early returns
- The standalone path returns
ClientError.Unknown()when the local file/uploadingStateis missing, while the channel-bound path returnsClientError.AttachmentUploading(...). Consider aligning these to a dedicated, descriptive error for “missing uploading state or unreadable local file” to avoid surprising clients and to simplify error handling across both flows.- Add a log entry before early-returning; this has proven invaluable when debugging user reports.
Proposed minimal change (adds logging, keeps current error type):
- guard - let uploadingState = attachment.uploadingState, - let fileData = try? Data(contentsOf: uploadingState.localFileURL) else { - return completion(.failure(ClientError.Unknown())) - } + guard let uploadingState = attachment.uploadingState else { + log.error("Standalone upload: missing uploadingState for attachment type=\(attachment.type)", subsystems: .httpRequests) + return completion(.failure(ClientError.Unknown())) + } + guard let fileData = try? Data(contentsOf: uploadingState.localFileURL) else { + log.error("Standalone upload: failed to read file at \(uploadingState.localFileURL)", subsystems: .httpRequests) + return completion(.failure(ClientError.Unknown())) + }Optional follow-ups:
- Perform an early size gate against
Self.maxAttachmentSizeusing file attributes before loading into memory to avoid reading very large files only to reject them. Return a specific “file too large” error if available inClientError.- If accurate upload progress is important, consider switching the helper to
URLSession.uploadTask(with:from:)or a streamingInputStreammultipart to get reliable upload progress (dataTask often reports download progress only).Would you like me to draft a small
ClientErrorvariant for standalone uploads and add unit tests that assert the behavior for missing state, unreadable file, and oversize files?
151-157: Remove unused generic from the private helper to avoid confusionThe helper always decodes
FileUploadPayloadand the genericResponsePayloadisn’t used elsewhere. Narrow the signature to the concrete payload to improve clarity and prevent accidental misuse.- private func uploadAttachment<ResponsePayload>( - endpoint: Endpoint<ResponsePayload>, + private func uploadAttachment( + endpoint: Endpoint<FileUploadPayload>, fileData: Data, uploadingState: AttachmentUploadingState, progress: ((Double) -> Void)? = nil, completion: @escaping (Result<UploadedFile, Error>) -> Void )Additionally, consider renaming this helper to
performUpload(or similar) to distinguish it from the two publicuploadAttachmentoverloads.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift(2 hunks)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift(2 hunks)Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift(1 hunks)Sources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swift(1 hunks)Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift(2 hunks)Sources/StreamChat/ChatClient.swift(1 hunks)Sources/StreamChat/Models/Attachments/ChatMessageAttachment.swift(2 hunks)StreamChat.xcodeproj/project.pbxproj(4 hunks)TestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swift(1 hunks)TestTools/StreamChatTestTools/Mocks/Models + Extensions/Attachments/StreamAttachment_Mock.swift(1 hunks)TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift(1 hunks)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift(1 hunks)TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift(1 hunks)Tests/StreamChatTests/APIClient/Endpoints/AttachmentEndpoints_Tests.swift(1 hunks)Tests/StreamChatTests/APIClient/Endpoints/EndpointPath_Tests.swift(1 hunks)Tests/StreamChatTests/APIClient/StreamAttachmentUploader_Tests.swift(1 hunks)Tests/StreamChatTests/APIClient/StreamCDNClient_Tests.swift(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
Tests/StreamChatTests/APIClient/StreamAttachmentUploader_Tests.swift (4)
TestTools/StreamChatTestTools/Mocks/Models + Extensions/Attachments/StreamAttachment_Mock.swift (1)
mock(10-38)Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (1)
uploadStandaloneAttachment(32-47)
TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (4)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (1)
uploadStandaloneAttachment(32-47)TestTools/StreamChatTestTools/SpyPattern/Spy/Spy.swift (2)
record(17-19)record(40-42)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (4)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (1)
uploadStandaloneAttachment(32-48)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (1)
uploadStandaloneAttachment(32-47)TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift (1)
uploadStandaloneAttachment(17-21)
Sources/StreamChat/ChatClient.swift (2)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (2)
upload(39-54)uploadStandaloneAttachment(56-66)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)
TestTools/StreamChatTestTools/Mocks/Models + Extensions/Attachments/StreamAttachment_Mock.swift (2)
TestTools/StreamChatTestTools/TestData/DummyData/MessageAttachmentPayload.swift (1)
file(72-86)TestTools/StreamChatTestTools/Extensions/Unique/URL+Unique.swift (1)
newTemporaryFileURL(14-16)
Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift (2)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (4)
uploadAttachment(57-70)uploadAttachment(93-106)uploadAttachment(108-127)uploadAttachment(151-216)Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (2)
uploadAttachment(8-16)uploadAttachment(18-26)
TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (5)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (1)
uploadStandaloneAttachment(32-48)TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift (1)
uploadStandaloneAttachment(17-21)TestTools/StreamChatTestTools/SpyPattern/Spy/Spy.swift (2)
record(17-19)record(40-42)
TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift (4)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
uploadStandaloneAttachment(129-149)TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (1)
uploadStandaloneAttachment(32-48)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (1)
uploadStandaloneAttachment(32-47)
Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (1)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (4)
uploadAttachment(57-70)uploadAttachment(93-106)uploadAttachment(108-127)uploadAttachment(151-216)
Sources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swift (2)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (4)
uploadAttachment(57-70)uploadAttachment(93-106)uploadAttachment(108-127)uploadAttachment(151-216)Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (2)
uploadAttachment(8-16)uploadAttachment(18-26)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (5)
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)TestTools/StreamChatTestTools/SpyPattern/Spy/AttachmentUploader_Spy.swift (1)
uploadStandaloneAttachment(32-48)TestTools/StreamChatTestTools/SpyPattern/Spy/CDNClient_Spy.swift (2)
uploadStandaloneAttachment(32-47)uploadAttachment(15-30)TestTools/StreamChatTestTools/TestData/CustomCDNClient.swift (2)
uploadStandaloneAttachment(17-21)uploadAttachment(11-15)Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (2)
uploadAttachment(8-16)uploadAttachment(18-26)
Tests/StreamChatTests/APIClient/StreamCDNClient_Tests.swift (4)
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (5)
uploadAttachment(57-70)uploadAttachment(93-106)uploadAttachment(108-127)uploadAttachment(151-216)uploadStandaloneAttachment(129-149)Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
uploadStandaloneAttachment(56-66)TestTools/StreamChatTestTools/Mocks/Models + Extensions/Attachments/StreamAttachment_Mock.swift (1)
mock(10-38)TestTools/StreamChatTestTools/Wait/WaitFor.swift (1)
waitFor(32-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Demo App + Example Apps
- GitHub Check: Test UI (Debug)
- GitHub Check: Test LLC (Debug)
🔇 Additional comments (9)
StreamChat.xcodeproj/project.pbxproj (3)
800-800: PBXBuildFile added for StreamAttachment_Mock.swift — looks correct.The build file references the matching PBXFileReference ID and is consistent with nearby entries. No issues spotted.
7007-7007: Good placement under Attachments group.Grouping the mock alongside other attachment mocks keeps the structure tidy and discoverable.
11399-11399: All good—StreamAttachment_Mock.swift only lives in the TestTools targetI inspected every PBXSourcesBuildPhase and found only one block (ID 793060E225778896005CF846) listing
StreamAttachment_Mock.swift, and that block contains nothing but test‐only mocks (e.g.ChatMessage_Mock.swift,WebSocketEngine_Mock.swift, etc.). There are no other Sources build phases—including the main StreamChat or StreamChatUI targets—that reference this file.Sources/StreamChat/APIClient/Endpoints/EndpointPath+OfflineRequest.swift (1)
14-19: Summary mismatch: code vs AI summaryThe AI summary claims channel uploads are queued offline (true branch). The code places both upload variants in the false branch. Treat the code as source of truth, but please confirm and correct the PR description to avoid confusion.
Sources/StreamChat/APIClient/Endpoints/AttachmentEndpoints.swift (1)
10-16: Channel-scoped endpoint update looks correctPath rename to uploadChannelAttachment and keeping requiresConnectionId: false align with how uploads work (no WS requirement). Payload and HTTP method are consistent with existing upload semantics.
Sources/StreamChat/APIClient/AttachmentUploader/AttachmentUploader.swift (1)
56-66: Delegation LGTMThin delegation to CDN client keeps roles clear and testable. No issues spotted.
Tests/StreamChatTests/APIClient/StreamCDNClient_Tests.swift (2)
43-74: Standalone encoder endpoint assertion: good coverageVerifies encoder receives the expected Endpoint for uploads/uploads/image. Solid.
262-310: Failure path for standalone uploads looks correctVerifies decoder receives the network error and maps to encoderError in the result. Good parity with channel-scoped failure test.
Sources/StreamChat/APIClient/CDNClient/CDNClient.swift (1)
118-127: Refactor to route through a shared helper using explicit endpoint — LGTMPassing an explicit
Endpoint<FileUploadPayload>improves readability and reuse across channel-bound and standalone paths. No issues spotted here.
| case uploadChannelAttachment(channelId: String, type: String) | ||
| case uploadAttachment(String) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Renaming the former .uploadAttachment(channelId:type:) breaks Codable compatibility; consider a backward-compatible strategy
Because EndpointPath is Codable, changing the case name and its payload shape (from two associated values to one) will fail to decode any previously persisted values (e.g., offline request queue, cached requests) that used the legacy case. This can silently drop queued work or crash decode paths after app upgrade.
Two viable approaches:
- Preferred (non-breaking for persisted data): keep the original case name for the channel-scoped upload and give the new standalone endpoint a distinct name, e.g.:
case uploadAttachment(channelId: String, type: String)(legacy semantics, unchanged)case uploadStandaloneAttachment(String)(new uploads/{type})
Update call-sites accordingly.
- Alternative: implement a custom
init(from:)that attempts to decode the legacy representation (uploadAttachmentwith a keyed object{channelId,type}) and map it to the newuploadChannelAttachment, while also supporting the new single-valueuploadAttachment(String)shape.
If you do intend this to be a breaking change, please call it out in the changelog and migration docs. Otherwise, consider the diff below to avoid breaking persisted payloads:
- case uploadChannelAttachment(channelId: String, type: String)
- case uploadAttachment(String)
+ // Keep legacy case name for channel-scoped uploads to retain Codable compatibility.
+ case uploadAttachment(channelId: String, type: String)
+ // New: standalone uploads (uploads/{type})
+ case uploadStandaloneAttachment(String)
@@
- case let .uploadChannelAttachment(channelId, type): return "channels/\(channelId)/\(type)"
- case let .uploadAttachment(type): return "uploads/\(type)"
+ case let .uploadAttachment(channelId, type): return "channels/\(channelId)/\(type)"
+ case let .uploadStandaloneAttachment(type): return "uploads/\(type)"Follow-ups outside this file: switch call-sites/tests from .uploadChannelAttachment to .uploadAttachment(channelId:type:) and rename references to the new standalone case as needed. I can generate a PR-wide patch if helpful.
Run this repo scan to see if EndpointPath values are persisted (to gauge the impact) and where to update call-sites:
Also applies to: 129-131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Look for EndpointPath encode/decode usage (persistence risk)
rg -nC2 -g '!**/DerivedData/**' -P 'JSONEncoder\.stream\.encode\(|JSONDecoder\.stream\.decode\('
# 2) Find references to uploadAttachment/uploadChannelAttachment to estimate blast radius
rg -nC2 -g '!**/DerivedData/**' -P '\bEndpointPath\.(uploadAttachment|uploadChannelAttachment)\b'
rg -nC2 -g '!**/DerivedData/**' -P '\.uploadAttachment\(' --type swift
# 3) Check for offline queue classes that may serialize endpoints
rg -nC3 -g '!**/DerivedData/**' -P 'Offline|Queue|Persistence|Store|Codable|Disk|background' --type swiftLength of output: 54439
Preserve Codable compatibility for queued endpoints in EndpointPath
The EndpointPath enum is serialized and deserialized by the offline requests queue (see OfflineRequestsRepository.swift:198–199 for JSONDecoder.stream.decode and 233–235 for JSONEncoder.stream.encode). Renaming the channel-scoped upload case and changing its associated values will break decoding of any endpoints persisted before this change, leading to silent failure or data loss in the offline queue.
Locations needing attention:
- Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift (lines 39–41)
Ensure the legacy.uploadAttachment(channelId:type:)case remains intact for backward compatibility. - Sources/StreamChat/Repositories/OfflineRequestsRepository.swift (around lines 198 & 233)
Confirm that decoding logic continues to expect the original case, or implement a custominit(from:)inEndpointPathto handle both legacy and new representations.
Suggested diff in EndpointPath.swift:
-Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift
- case uploadChannelAttachment(channelId: String, type: String)
- case uploadAttachment(String)
+ // Legacy: channel-scoped uploads (retain for Codable compatibility)
+ case uploadAttachment(channelId: String, type: String)
+ // New: standalone uploads (uploads/{type})
+ case uploadStandaloneAttachment(String)
@@
- case let .uploadChannelAttachment(channelId, type): return "channels/\(channelId)/\(type)"
- case let .uploadAttachment(type): return "uploads/\(type)"
+ case let .uploadAttachment(channelId, type): return "channels/\(channelId)/\(type)"
+ case let .uploadStandaloneAttachment(type): return "uploads/\(type)"Follow-ups:
- Update all call-sites and tests referencing
.uploadChannelAttachmentto use.uploadAttachment(channelId:type:). - Use
.uploadStandaloneAttachmentfor the new uploads endpoint.
📝 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.
| case uploadChannelAttachment(channelId: String, type: String) | |
| case uploadAttachment(String) | |
| // --- in Sources/StreamChat/APIClient/Endpoints/EndpointPath.swift --- | |
| // Replace the two existing cases: | |
| // | |
| // case uploadChannelAttachment(channelId: String, type: String) | |
| // case uploadAttachment(String) | |
| // | |
| // With these: | |
| // Legacy: channel-scoped uploads (retain for Codable compatibility) | |
| case uploadAttachment(channelId: String, type: String) | |
| // New: standalone uploads (uploads/{type}) | |
| case uploadStandaloneAttachment(String) | |
| // --- later in the same file, inside `var path: String { switch self { … } }` --- | |
| // Replace the two branches: | |
| // | |
| // case let .uploadChannelAttachment(channelId, type): return "channels/\(channelId)/\(type)" | |
| // case let .uploadAttachment(type): return "uploads/\(type)" | |
| // | |
| // With these: | |
| case let .uploadAttachment(channelId, type): | |
| return "channels/\(channelId)/\(type)" | |
| case let .uploadStandaloneAttachment(type): | |
| return "uploads/\(type)" |
| case let (.uploadChannelAttachment(channelId1, type1), .uploadChannelAttachment(channelId2, type2)): return channelId1 == channelId2 && | ||
| type1 == | ||
| type2 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add Equatable branch for the new .uploadAttachment(type:) path as well
You updated the renamed channel-scoped case, but the new standalone path (.uploadAttachment(type:)) is not covered. Any equality checks involving that case will currently return false. Please add a matching branch.
Apply this diff near the existing upload case:
case let (.uploadChannelAttachment(channelId1, type1), .uploadChannelAttachment(channelId2, type2)): return channelId1 == channelId2 &&
type1 ==
type2
+case let (.uploadAttachment(type1), .uploadAttachment(type2)): return type1 == type2Run this to confirm we have tests that rely on path equality for the new case (and to ensure the new branch is exercised):
🏁 Script executed:
#!/bin/bash
# Find usages of the standalone upload path in tests
rg -nP --type=swift -C2 '\.uploadAttachment\s*\(\s*type:' Tests TestTools Sources \
| sed -n '1,200p'
# Check Equatable extension covers standalone upload
rg -nP --type=swift 'extension\s+EndpointPath\s*:\s*Equatable' -A120 TestTools \
| rg -nP 'uploadAttachment'Length of output: 1143
🏁 Script executed:
#!/bin/bash
# Locate the Equatable extension file under TestTools
find TestTools -type f -name "*Equatable.swift"
# Display its first 200 lines if found
equiv_file=$(find TestTools -type f -name "*Equatable.swift" | head -n1)
if [ -n "$equiv_file" ]; then
echo "== Showing first 200 lines of $equiv_file =="
sed -n '1,200p' "$equiv_file"
echo "== Searching for uploadAttachment in $equiv_file =="
grep -n "uploadAttachment" "$equiv_file" || echo "No matches found in $equiv_file"
else
echo "No Equatable extension file found under TestTools"
fi
# Search across TestTools for any mentions of uploadAttachment
echo "== Searching TestTools for uploadAttachment usage =="
rg -n --type swift 'uploadAttachment' TestTools || echo "No uploadAttachment references in TestTools"Length of output: 5363
🏁 Script executed:
#!/bin/bash
# Inspect the EndpointPath Equatable extension for uploadAttachment handling
equiv="TestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swift"
if [ -f "$equiv" ]; then
echo "== File exists: $equiv =="
sed -n '1,200p' "$equiv"
echo "== Searching for uploadChannelAttachment =="
grep -n "uploadChannelAttachment" "$equiv" || echo "None"
echo "== Searching for uploadAttachment =="
grep -n "uploadAttachment" "$equiv" || echo "None"
else
echo "$equiv not found"
fiLength of output: 4316
Add missing Equatable branch for .uploadAttachment(type:)
Our inspection of TestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swift shows that while the .uploadChannelAttachment(channelId:type:) case is handled, there is no matching branch for the standalone .uploadAttachment(type:) path. As a result, any equality check between two .uploadAttachment cases will fall through to the default and return false, likely breaking tests or logic that compares upload endpoints.
Please add the following case immediately after the existing .uploadChannelAttachment branch:
case let (.uploadChannelAttachment(channelId1, type1), .uploadChannelAttachment(channelId2, type2)): return channelId1 == channelId2 &&
type1 ==
type2
+case let (.uploadAttachment(type1), .uploadAttachment(type2)): return type1 == type2This will ensure that equality between two .uploadAttachment endpoints correctly compares their type parameters.
📝 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.
| case let (.uploadChannelAttachment(channelId1, type1), .uploadChannelAttachment(channelId2, type2)): return channelId1 == channelId2 && | |
| type1 == | |
| type2 | |
| case let (.uploadChannelAttachment(channelId1, type1), .uploadChannelAttachment(channelId2, type2)): | |
| return channelId1 == channelId2 && | |
| type1 == type2 | |
| case let (.uploadAttachment(type1), .uploadAttachment(type2)): | |
| return type1 == type2 |
🤖 Prompt for AI Agents
In TestTools/StreamChatTestTools/Extensions/EndpoinPath+Equatable.swift around
lines 31 to 33, add an Equatable branch for the `.uploadAttachment(type:)` enum
case immediately after the existing `.uploadChannelAttachment(channelId:type:)`
branch; match two `.uploadAttachment` cases and return the comparison of their
`type` parameters (i.e., `type1 == type2`) so equality between two standalone
upload endpoints is handled correctly.
|
SDK Performance
|
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-927/ios-support-for-the-new-upload-endpoint.
🎯 Goal
Allow possibility to upload attachments without tying them to a channel/message.
📝 Summary
Provide bullet points with the most important changes in the codebase.
🛠 Implementation
Exposed the new endpoint.
🎨 Showcase
Add relevant screenshots and/or videos/gifs to easily see what this PR changes, if applicable.
🧪 Manual Testing Notes
Run this somewhere:
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit