OpenAPI: Reuse generated AppSettings model in the public API layer#4159
Merged
Conversation
📝 WalkthroughWalkthroughThis PR renames and publicizes generated OpenAPI models (AppSettings, UploadConfig), switches ChatClient to consume the generated app payload directly, refactors AppSettings+Extensions.swift to extend the generated types with a deprecated sizeLimitInBytes property, and updates ComposerVC and test mocks for the new non-optional sizeLimit field. ChangesAppSettings/UploadConfig public model migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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 (1)
Sources/StreamChat/ChatClient.swift (1)
28-28: 🗄️ Data Integrity & Integration | 🔵 TrivialConfirm the struct→class change for
AppSettingsis documented as a breaking API change.
AppSettingsmoves from a struct to afinal class(per PR objectives). Even though it's immutable (letproperties), consumers that relied on value semantics (implicit copy-on-assign, potentialEquatable/Hashableusage, or@State/diffing based on struct equality) may behave differently now that this is a reference type. Recommend calling this out explicitly in release notes / SemVer bump.🤖 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/ChatClient.swift` at line 28, Document the AppSettings type change as a breaking API update in the release notes and SemVer bump guidance, since ChatClient.appSettings now exposes a final class instead of a struct. Call out the loss of value semantics and any behavior changes for consumers that relied on implicit copying, equality/hashability, or SwiftUI/state diffing, and reference AppSettings and ChatClient.appSettings explicitly so the change is easy to find.
🤖 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/Models/AppSettings`+Extensions.swift:
- Around line 13-18: The deprecated AppSettings.UploadConfig.sizeLimitInBytes
property currently always returns a value because sizeLimit is non-optional,
which breaks the previous “no limit configured” nil semantics. Update
sizeLimitInBytes to preserve the old behavior by returning nil when sizeLimit is
unset or non-positive (matching the existing sizeLimit <= 0 convention used
elsewhere, such as ComposerVC.maxAttachmentSize), and only return
Int64(sizeLimit) for real configured limits. Keep the change localized to
AppSettings.UploadConfig.sizeLimitInBytes so existing callers continue to branch
correctly on nil versus a valid limit.
- Around line 39-41: The early return in
AppSettings+Extensions.isAllowed(localURL:) lets extensionless files bypass the
configured upload validation entirely. Update this method so a missing
pathExtension does not immediately return true; instead, let the existing
extension and MIME-type allow/block logic run for extensionless URLs, or only
skip validation if the settings explicitly permit it. Use the
isAllowed(localURL:) flow and its related upload restriction checks to keep
dashboard-configured rules enforced.
---
Nitpick comments:
In `@Sources/StreamChat/ChatClient.swift`:
- Line 28: Document the AppSettings type change as a breaking API update in the
release notes and SemVer bump guidance, since ChatClient.appSettings now exposes
a final class instead of a struct. Call out the loss of value semantics and any
behavior changes for consumers that relied on implicit copying,
equality/hashability, or SwiftUI/state diffing, and reference AppSettings and
ChatClient.appSettings explicitly so the change is easy to find.
🪄 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: 298443d9-1e6f-4086-b667-8b5ffb7c6e06
⛔ Files ignored due to path filters (3)
Sources/StreamChat/Generated/OpenAPI/models/AppSettings.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/GetApplicationResponse.swiftis excluded by!**/generated/**Sources/StreamChat/Generated/OpenAPI/models/UploadConfig.swiftis excluded by!**/generated/**
📒 Files selected for processing (5)
Scripts/openapi_generate.shSources/StreamChat/ChatClient.swiftSources/StreamChat/Models/AppSettings+Extensions.swiftSources/StreamChatUI/Composer/ComposerVC.swiftTestTools/StreamChatTestTools/Mocks/StreamChat/ChatClient_Mock.swift
Comment on lines
+13
to
18
| extension AppSettings.UploadConfig { | ||
| /// The file size limit allowed in Bytes. | ||
| /// This value is configurable from Stream's Dashboard App Settings. | ||
| @available(*, deprecated, renamed: "sizeLimit") | ||
| public var sizeLimitInBytes: Int64? { Int64(sizeLimit) } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
sizeLimitInBytes can no longer represent "no limit configured".
sizeLimit (generated) is a non-optional Int, so Int64(sizeLimit) never returns nil. Given the codebase's own convention elsewhere (ComposerVC.maxAttachmentSize treats sizeLimit <= 0 as "use default"), the deprecated sizeLimitInBytes previously likely returned nil to signal "no limit configured." Any remaining caller (internal or third-party via this public deprecated API) branching on nil vs a value will now always take the "configured" branch, silently changing behavior for the 0/unset case.
🐛 Proposed fix to preserve nil semantics
`@available`(*, deprecated, renamed: "sizeLimit")
- public var sizeLimitInBytes: Int64? { Int64(sizeLimit) }
+ public var sizeLimitInBytes: Int64? { sizeLimit > 0 ? Int64(sizeLimit) : nil }📝 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.
Suggested change
| extension AppSettings.UploadConfig { | |
| /// The file size limit allowed in Bytes. | |
| /// This value is configurable from Stream's Dashboard App Settings. | |
| @available(*, deprecated, renamed: "sizeLimit") | |
| public var sizeLimitInBytes: Int64? { Int64(sizeLimit) } | |
| } | |
| extension AppSettings.UploadConfig { | |
| /// The file size limit allowed in Bytes. | |
| /// This value is configurable from Stream's Dashboard App Settings. | |
| `@available`(*, deprecated, renamed: "sizeLimit") | |
| public var sizeLimitInBytes: Int64? { sizeLimit > 0 ? Int64(sizeLimit) : nil } | |
| } |
🤖 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/Models/AppSettings`+Extensions.swift around lines 13 - 18,
The deprecated AppSettings.UploadConfig.sizeLimitInBytes property currently
always returns a value because sizeLimit is non-optional, which breaks the
previous “no limit configured” nil semantics. Update sizeLimitInBytes to
preserve the old behavior by returning nil when sizeLimit is unset or
non-positive (matching the existing sizeLimit <= 0 convention used elsewhere,
such as ComposerVC.maxAttachmentSize), and only return Int64(sizeLimit) for real
configured limits. Keep the change localized to
AppSettings.UploadConfig.sizeLimitInBytes so existing callers continue to branch
correctly on nil versus a valid limit.
Comment on lines
39
to
+41
| func isAllowed(localURL: URL) -> Bool { | ||
| guard !localURL.pathExtension.isEmpty else { return true } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file outline =="
ast-grep outline Sources/StreamChat/Models/AppSettings+Extensions.swift --view expanded || true
echo
echo "== target file excerpt =="
nl -ba Sources/StreamChat/Models/AppSettings+Extensions.swift | sed -n '1,220p'
echo
echo "== usages of isAllowed(localURL:) =="
rg -n "isAllowed\\(localURL:" -S Sources || true
echo
echo "== references to AttachmentFileType(ext: or mimeType checks =="
rg -n "AttachmentFileType\\(ext:|mimeType|allowedFileExtensions|allowedMimeTypes|upload validation|isAllowed\\(" Sources -S || trueRepository: GetStream/stream-chat-swift
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file excerpt =="
sed -n '1,180p' Sources/StreamChat/Models/AppSettings+Extensions.swift | cat -n
echo
echo "== usages of isAllowed(localURL:) =="
rg -n "isAllowed\\(localURL:" Sources -S || true
echo
echo "== nearby upload validation references =="
rg -n "allowedFileExtensions|allowedMimeTypes|isAllowed\\(|AttachmentFileType\\(|mimeType" Sources/StreamChat -S || trueRepository: GetStream/stream-chat-swift
Length of output: 11946
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AttachmentQueueUploader excerpt around validation =="
sed -n '150,210p' Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift | cat -n
echo
echo "== AttachmentFileType initializer and mimeType =="
sed -n '260,330p' Sources/StreamChat/Models/Attachments/AttachmentTypes.swift | cat -n
echo
echo "== AttachmentFileType ext-based usage =="
rg -n "init\\(ext:|AttachmentFileType\\(ext:" Sources/StreamChat/Models/Attachments/AttachmentTypes.swift Sources/StreamChat -S || trueRepository: GetStream/stream-chat-swift
Length of output: 6781
Extensionless uploads bypass configured upload validation Sources/StreamChat/Models/AppSettings+Extensions.swift:39
guard !localURL.pathExtension.isEmpty else { return true } skips both the extension and MIME-type allow/block checks, so a local file with no extension can bypass dashboard upload restrictions. If extensionless files should still honor those rules, remove this early return or gate it behind the config checks.
🤖 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/Models/AppSettings`+Extensions.swift around lines 39 - 41,
The early return in AppSettings+Extensions.isAllowed(localURL:) lets
extensionless files bypass the configured upload validation entirely. Update
this method so a missing pathExtension does not immediately return true;
instead, let the existing extension and MIME-type allow/block logic run for
extensionless URLs, or only skip validation if the settings explicitly permit
it. Use the isAllowed(localURL:) flow and its related upload restriction checks
to keep dashboard-configured rules enforced.
Collaborator
SDK Size
|
Collaborator
SDK Performance
|
Collaborator
StreamChat XCSize
|
Collaborator
StreamChatUI XCSize
|
|
Generated by 🚫 Danger |
Public Interface+ public final class AppSettings: Sendable, Codable, JSONEncodable
+
+ case asyncUrlEnrichEnabled = "async_url_enrich_enabled"
+ case autoTranslationEnabled = "auto_translation_enabled"
+ case fileUploadConfig = "file_upload_config"
+ case id
+ case imageUploadConfig = "image_upload_config"
+ case name
+ case placement
+
+
+ public let asyncUrlEnrichEnabled: Bool
+ public let autoTranslationEnabled: Bool
+ public let fileUploadConfig: UploadConfig
+ public let id: Int
+ public let imageUploadConfig: UploadConfig
+ public let name: String
+ public let placement: String
+ public final class UploadConfig: Sendable, Codable, JSONEncodable
+
+ case allowedFileExtensions = "allowed_file_extensions"
+ case allowedMimeTypes = "allowed_mime_types"
+ case blockedFileExtensions = "blocked_file_extensions"
+ case blockedMimeTypes = "blocked_mime_types"
+ case sizeLimit = "size_limit"
+
+
+ public let allowedFileExtensions: [String]
+ public let allowedMimeTypes: [String]
+ public let blockedFileExtensions: [String]
+ public let blockedMimeTypes: [String]
+ public let sizeLimit: Int
- public struct AppSettings: Sendable
-
- public let name: String
- public let fileUploadConfig: UploadConfig
- public let imageUploadConfig: UploadConfig
- public let autoTranslationEnabled: Bool
- public let asyncUrlEnrichEnabled: Bool
-
-
- public struct UploadConfig: Sendable
-
- public let allowedFileExtensions: [String]
- public let blockedFileExtensions: [String]
- public let allowedMimeTypes: [String]
- public let blockedMimeTypes: [String]
- public let sizeLimitInBytes: Int64?
|
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.
🔗 Issue Links
Related: IOS-1620
🎯 Goal
Reuse the OpenAPI-generated model for the public API layer instead of maintaining a duplicated hand-written
AppSettingsmodel with a mapping layer.📝 Summary
AppResponseFieldstoAppSettingsandFileUploadConfigtoUploadConfig, and generate both as public via a newpublicize_modelpost-processing step (the memberwise init andCodingKeysstay internal).AppSettingsstruct and itsasModel()mappings;ChatClient.loadAppSettingsnow returns the generated model directly.AppSettings.UploadConfigtypealias and the existing UTI/URL validation helpers are preserved inAppSettings+Extensions.swift;sizeLimitInBytesis kept as a computed property and deprecated in favor of the generatedsizeLimit.AppSettingsis now afinal class(previously a struct) and exposes newidandplacementproperties.🧪 Manual Testing Notes
Connect a user and confirm attachment upload validation in the composer still respects the dashboard upload configuration (allowed/blocked file types and size limits fetched from
/api/v2/app).☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit