-
Notifications
You must be signed in to change notification settings - Fork 228
V5: Fix build warnings #3904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v5
Are you sure you want to change the base?
V5: Fix build warnings #3904
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 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 |
| var results: [NSManagedObject]? | ||
| nonisolated(unsafe) var modelMapping: [DatabaseId: NSManagedObjectID] = [:] | ||
| context.performAndWait { | ||
| results = modelClass.batchFetch(keyPath: keyPath, equalTo: values, context: context) | ||
| } | ||
| guard let results = results else { continue } | ||
|
|
||
| var modelMapping: [DatabaseId: NSManagedObjectID] = [:] | ||
| results.forEach { | ||
| if let id = modelClass.id(for: $0) { | ||
| modelMapping[id] = $0.objectID | ||
| let results = modelClass.batchFetch(keyPath: keyPath, equalTo: values, context: context) | ||
| results.forEach { | ||
| if let id = modelClass.id(for: $0) { | ||
| modelMapping[id] = $0.objectID | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was interesting because DTOs were leaking outside of context's queue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... Could this be causing this crash: #3905?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to fix this on v4 as well
| public let extraData: [String: RawJSON] | ||
| } | ||
|
|
||
| extension ChatThread: Hashable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it from UI
| context: self.database.backgroundReadOnlyContext, | ||
| fetchRequest: MessageDTO | ||
| .messagesPendingSendFetchRequest() | ||
| private lazy var observer = StateLayerDatabaseObserver<ListResult, MessageSendingQueue.SendRequest, MessageDTO>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to convert earlier, also makes sure DTOs don't escape from context's thread
Generated by 🚫 Danger |
| /// Returns max possible attachment size in bytes. | ||
| /// By default the value is taken from `CDNClient.maxAttachmentSize` type. | ||
| /// But it can be overridden by setting a value here. | ||
| @available(*, deprecated, message: "The max attachment size can now be set from the Stream's Dashboard App Settings. It supports setting a size limit per attachment type.") | ||
| public var maxAttachmentSize: Int64 { | ||
| // TODO: For v5 the maxAttachmentSize should be responsibility of the UI SDK. | ||
| // Since this is not even used in the StreamChat LLC SDK. | ||
| get { | ||
| if let overrideMaxAttachmentSize = self.overrideMaxAttachmentSize { | ||
| return overrideMaxAttachmentSize | ||
| } else if let customCDNClient = customCDNClient { | ||
| return type(of: customCDNClient).maxAttachmentSize | ||
| } else { | ||
| return StreamCDNClient.maxAttachmentSize | ||
| } | ||
| } | ||
| set { | ||
| overrideMaxAttachmentSize = newValue | ||
| } | ||
| } | ||
|
|
||
| /// Used to override the maxAttachmentSize, by setting the value in the config instead of relying on `CDNClient`. | ||
| private var overrideMaxAttachmentSize: Int64? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 🗑️
nuno-vieira
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ✅
SDK Size
|
SDK Performance
|
StreamChat XCSize
Show 391 more objects
|
StreamChatUI XCSize
Show 296 more objects
|
|



🔗 Issue Links
Fixes: IOS-1193
🎯 Goal
Fix build warnings in StreamChat and StreamChatUI since it is not great to have them for customers
📝 Summary
🛠 Implementation
☑️ Contributor Checklist
docs-contentrepo