From 9859cf95a48b320d39699daf3ad33b2a4a12d435 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 25 Jul 2022 17:03:09 +1000 Subject: [PATCH 1/4] Attempted to fix the notification & call reporting issues Fixed an issue where fileIds weren't correctly getting sent along with open group messages Fixed an issue where the screens could miss updates if the device was locked with the app in the foreground and then later unlocked after receiving notifications Added an optimisation to prevent attempting to send a message after it has been deleted Added logic to report fake calls if the code goes down an invalid code path when handling a call (to prevent Apple blocking the app) Delayed the core which clears notifications to increase the time the app has to handle interactions (just in case it was a race condition) --- .../Calls/Call Management/SessionCall.swift | 5 +- .../Call Management/SessionCallManager.swift | 14 +++- Session/Conversations/ConversationVC.swift | 11 ++- Session/Home/HomeVC.swift | 11 ++- .../MessageRequestsViewController.swift | 11 ++- .../MediaTileViewController.swift | 11 ++- Session/Meta/AppDelegate.swift | 4 +- .../PushRegistrationManager.swift | 74 +++++++++++-------- .../Database/Models/Attachment.swift | 25 +++++-- .../Jobs/Types/AttachmentDownloadJob.swift | 5 +- .../Jobs/Types/AttachmentUploadJob.swift | 2 +- .../Jobs/Types/MessageSendJob.swift | 32 ++++++-- .../Messages/Message+Destination.swift | 16 ++++ .../MessageSender+Convenience.swift | 20 +++-- .../Utilities/ProfileManager.swift | 5 +- .../Types/PagedDatabaseObserver.swift | 12 +++ 16 files changed, 189 insertions(+), 69 deletions(-) diff --git a/Session/Calls/Call Management/SessionCall.swift b/Session/Calls/Call Management/SessionCall.swift index 030860de8..1aef15a69 100644 --- a/Session/Calls/Call Management/SessionCall.swift +++ b/Session/Calls/Call Management/SessionCall.swift @@ -167,7 +167,10 @@ public final class SessionCall: CurrentCallProtocol, WebRTCSessionDelegate { } func reportIncomingCallIfNeeded(completion: @escaping (Error?) -> Void) { - guard case .answer = mode else { return } + guard case .answer = mode else { + SessionCallManager.reportFakeCall(info: "Call not in answer mode") + return + } setupTimeoutTimer() AppEnvironment.shared.callManager.reportIncomingCall(self, callerName: contactName) { error in diff --git a/Session/Calls/Call Management/SessionCallManager.swift b/Session/Calls/Call Management/SessionCallManager.swift index 30dbcdfa0..643268bc1 100644 --- a/Session/Calls/Call Management/SessionCallManager.swift +++ b/Session/Calls/Call Management/SessionCallManager.swift @@ -72,6 +72,16 @@ public final class SessionCallManager: NSObject, CallManagerProtocol { // MARK: - Report calls + public static func reportFakeCall(info: String) { + SessionCallManager.sharedProvider(useSystemCallLog: false) + .reportNewIncomingCall( + with: UUID(), + update: CXCallUpdate() + ) { _ in + SNLog("[Calls] Reported fake incoming call to CallKit due to: \(info)") + } + } + public func reportOutgoingCall(_ call: SessionCall) { AssertIsOnMainThread() UserDefaults.sharedLokiProject?.set(true, forKey: "isCallOngoing") @@ -109,7 +119,9 @@ public final class SessionCallManager: NSObject, CallManagerProtocol { UserDefaults.sharedLokiProject?.set(true, forKey: "isCallOngoing") completion(nil) } - } else { + } + else { + SessionCallManager.reportFakeCall(info: "No CXProvider instance") UserDefaults.sharedLokiProject?.set(true, forKey: "isCallOngoing") completion(nil) } diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index 31a7c770a..b06c8d0b2 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -450,7 +450,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers } @objc func applicationDidBecomeActive(_ notification: Notification) { - startObservingChanges() + startObservingChanges(didReturnFromBackground: true) recoverInputView() } @@ -460,7 +460,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers // MARK: - Updating - private func startObservingChanges() { + private func startObservingChanges(didReturnFromBackground: Bool = false) { // Start observing for data changes dataChangeObservable = Storage.shared.start( viewModel.observableThreadData, @@ -506,6 +506,13 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers self?.viewModel.onInteractionChange = { [weak self] updatedInteractionData in self?.handleInteractionUpdates(updatedInteractionData) } + + // Note: When returning from the background we could have received notifications but the + // PagedDatabaseObserver won't have them so we need to force a re-fetch of the current + // data to ensure everything is up to date + if didReturnFromBackground { + self?.viewModel.pagedDataObserver?.reload() + } } } ) diff --git a/Session/Home/HomeVC.swift b/Session/Home/HomeVC.swift index ec2ab0c32..f45a9ece4 100644 --- a/Session/Home/HomeVC.swift +++ b/Session/Home/HomeVC.swift @@ -239,7 +239,7 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve } @objc func applicationDidBecomeActive(_ notification: Notification) { - startObservingChanges() + startObservingChanges(didReturnFromBackground: true) } @objc func applicationDidResignActive(_ notification: Notification) { @@ -248,7 +248,7 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve // MARK: - Updating - private func startObservingChanges() { + private func startObservingChanges(didReturnFromBackground: Bool = false) { // Start observing for data changes dataChangeObservable = Storage.shared.start( viewModel.observableState, @@ -269,6 +269,13 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve self.viewModel.onThreadChange = { [weak self] updatedThreadData in self?.handleThreadUpdates(updatedThreadData) } + + // Note: When returning from the background we could have received notifications but the + // PagedDatabaseObserver won't have them so we need to force a re-fetch of the current + // data to ensure everything is up to date + if didReturnFromBackground { + self.viewModel.pagedDataObserver?.reload() + } } private func stopObservingChanges() { diff --git a/Session/Home/Message Requests/MessageRequestsViewController.swift b/Session/Home/Message Requests/MessageRequestsViewController.swift index afcbf8000..ba87a80c3 100644 --- a/Session/Home/Message Requests/MessageRequestsViewController.swift +++ b/Session/Home/Message Requests/MessageRequestsViewController.swift @@ -147,7 +147,7 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat } @objc func applicationDidBecomeActive(_ notification: Notification) { - startObservingChanges() + startObservingChanges(didReturnFromBackground: true) } @objc func applicationDidResignActive(_ notification: Notification) { @@ -186,10 +186,17 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat // MARK: - Updating - private func startObservingChanges() { + private func startObservingChanges(didReturnFromBackground: Bool = false) { self.viewModel.onThreadChange = { [weak self] updatedThreadData in self?.handleThreadUpdates(updatedThreadData) } + + // Note: When returning from the background we could have received notifications but the + // PagedDatabaseObserver won't have them so we need to force a re-fetch of the current + // data to ensure everything is up to date + if didReturnFromBackground { + self.viewModel.pagedDataObserver?.reload() + } } private func handleThreadUpdates(_ updatedData: [MessageRequestsViewModel.SectionModel], initialLoad: Bool = false) { diff --git a/Session/Media Viewing & Editing/MediaTileViewController.swift b/Session/Media Viewing & Editing/MediaTileViewController.swift index 45acdf317..1085b574a 100644 --- a/Session/Media Viewing & Editing/MediaTileViewController.swift +++ b/Session/Media Viewing & Editing/MediaTileViewController.swift @@ -171,7 +171,7 @@ public class MediaTileViewController: UIViewController, UICollectionViewDataSour } @objc func applicationDidBecomeActive(_ notification: Notification) { - startObservingChanges() + startObservingChanges(didReturnFromBackground: true) } @objc func applicationDidResignActive(_ notification: Notification) { @@ -243,11 +243,18 @@ public class MediaTileViewController: UIViewController, UICollectionViewDataSour } } - private func startObservingChanges() { + private func startObservingChanges(didReturnFromBackground: Bool = false) { // Start observing for data changes (will callback on the main thread) self.viewModel.onGalleryChange = { [weak self] updatedGalleryData in self?.handleUpdates(updatedGalleryData) } + + // Note: When returning from the background we could have received notifications but the + // PagedDatabaseObserver won't have them so we need to force a re-fetch of the current + // data to ensure everything is up to date + if didReturnFromBackground { + self.viewModel.pagedDataObserver?.reload() + } } private func stopObservingChanges() { diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index d58faa92f..84286342e 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -149,13 +149,13 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD /// no longer always called before `applicationDidBecomeActive` we need to trigger the "clear notifications" logic /// within the `runNowOrWhenAppDidBecomeReady` callback and dispatch to the next run loop to ensure it runs after /// the notification has actually been handled - DispatchQueue.main.async { [weak self] in + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { [weak self] in self?.clearAllNotificationsAndRestoreBadgeCount() } } // On every activation, clear old temp directories. - ClearOldTemporaryDirectories(); + ClearOldTemporaryDirectories() } func applicationWillResignActive(_ application: UIApplication) { diff --git a/Session/Notifications/PushRegistrationManager.swift b/Session/Notifications/PushRegistrationManager.swift index 9069858fc..1cd8d71cf 100644 --- a/Session/Notifications/PushRegistrationManager.swift +++ b/Session/Notifications/PushRegistrationManager.swift @@ -242,40 +242,52 @@ public enum PushRegistrationError: Error { owsAssertDebug(type == .voIP) let payload = payload.dictionaryPayload - if let uuid = payload["uuid"] as? String, let caller = payload["caller"] as? String, let timestampMs = payload["timestamp"] as? Int64 { - let call: SessionCall? = Storage.shared.write { db in - let messageInfo: CallMessage.MessageInfo = CallMessage.MessageInfo( - state: (caller == getUserHexEncodedPublicKey(db) ? - .outgoing : - .incoming - ) + guard + let uuid: String = payload["uuid"] as? String, + let caller: String = payload["caller"] as? String, + let timestampMs: Int64 = payload["timestamp"] as? Int64 + else { + SessionCallManager.reportFakeCall(info: "Missing payload data") + return + } + + let maybeCall: SessionCall? = Storage.shared.write { db in + let messageInfo: CallMessage.MessageInfo = CallMessage.MessageInfo( + state: (caller == getUserHexEncodedPublicKey(db) ? + .outgoing : + .incoming ) - - guard let messageInfoData: Data = try? JSONEncoder().encode(messageInfo) else { return nil } - - let call: SessionCall = SessionCall(db, for: caller, uuid: uuid, mode: .answer) - let thread: SessionThread = try SessionThread.fetchOrCreate(db, id: caller, variant: .contact) - - let interaction: Interaction = try Interaction( - messageUuid: uuid, - threadId: thread.id, - authorId: caller, - variant: .infoCall, - body: String(data: messageInfoData, encoding: .utf8), - timestampMs: timestampMs - ).inserted(db) - call.callInteractionId = interaction.id - - return call - } + ) - // NOTE: Just start 1-1 poller so that it won't wait for polling group messages - (UIApplication.shared.delegate as? AppDelegate)?.startPollersIfNeeded(shouldStartGroupPollers: false) + guard let messageInfoData: Data = try? JSONEncoder().encode(messageInfo) else { return nil } - call?.reportIncomingCallIfNeeded { error in - if let error = error { - SNLog("[Calls] Failed to report incoming call to CallKit due to error: \(error)") - } + let call: SessionCall = SessionCall(db, for: caller, uuid: uuid, mode: .answer) + let thread: SessionThread = try SessionThread.fetchOrCreate(db, id: caller, variant: .contact) + + let interaction: Interaction = try Interaction( + messageUuid: uuid, + threadId: thread.id, + authorId: caller, + variant: .infoCall, + body: String(data: messageInfoData, encoding: .utf8), + timestampMs: timestampMs + ).inserted(db) + call.callInteractionId = interaction.id + + return call + } + + guard let call: SessionCall = maybeCall else { + SessionCallManager.reportFakeCall(info: "Could not retrieve call from database") + return + } + + // NOTE: Just start 1-1 poller so that it won't wait for polling group messages + (UIApplication.shared.delegate as? AppDelegate)?.startPollersIfNeeded(shouldStartGroupPollers: false) + + call.reportIncomingCallIfNeeded { error in + if let error = error { + SNLog("[Calls] Failed to report incoming call to CallKit due to error: \(error)") } } } diff --git a/SessionMessagingKit/Database/Models/Attachment.swift b/SessionMessagingKit/Database/Models/Attachment.swift index ffe2a3bdd..a0d18d392 100644 --- a/SessionMessagingKit/Database/Models/Attachment.swift +++ b/SessionMessagingKit/Database/Models/Attachment.swift @@ -468,6 +468,7 @@ extension Attachment { public let attachmentId: String public let interactionId: Int64 public let state: Attachment.State + public let downloadUrl: String? } public static func stateInfo(authorId: String, state: State? = nil) -> SQLRequest { @@ -484,7 +485,8 @@ extension Attachment { SELECT DISTINCT \(attachment[.id]) AS attachmentId, \(interaction[.id]) AS interactionId, - \(attachment[.state]) AS state + \(attachment[.state]) AS state, + \(attachment[.downloadUrl]) AS downloadUrl FROM \(Attachment.self) @@ -529,7 +531,8 @@ extension Attachment { SELECT DISTINCT \(attachment[.id]) AS attachmentId, \(interaction[.id]) AS interactionId, - \(attachment[.state]) AS state + \(attachment[.state]) AS state, + \(attachment[.downloadUrl]) AS downloadUrl FROM \(Attachment.self) @@ -913,6 +916,16 @@ extension Attachment { return true } + + public static func fileId(for downloadUrl: String?) -> String? { + return downloadUrl + .map { urlString -> String? in + urlString + .split(separator: "/") + .last + .map { String($0) } + } + } } // MARK: - Upload @@ -923,14 +936,14 @@ extension Attachment { queue: DispatchQueue, using upload: (Database, Data) -> Promise, encrypt: Bool, - success: (() -> Void)?, + success: ((String?) -> Void)?, failure: ((Error) -> Void)? ) { // This can occur if an AttachmnetUploadJob was explicitly created for a message // dependant on the attachment being uploaded (in this case the attachment has // already been uploaded so just succeed) guard state != .uploaded else { - success?() + success?(Attachment.fileId(for: self.downloadUrl)) return } @@ -982,7 +995,7 @@ extension Attachment { return } - success?() + success?(Attachment.fileId(for: self.downloadUrl)) return } @@ -1073,7 +1086,7 @@ extension Attachment { return } - success?() + success?(fileId) } .catch(on: queue) { error in Storage.shared.write { db in diff --git a/SessionMessagingKit/Jobs/Types/AttachmentDownloadJob.swift b/SessionMessagingKit/Jobs/Types/AttachmentDownloadJob.swift index c420db071..6a1d4fc16 100644 --- a/SessionMessagingKit/Jobs/Types/AttachmentDownloadJob.swift +++ b/SessionMessagingKit/Jobs/Types/AttachmentDownloadJob.swift @@ -87,10 +87,7 @@ public enum AttachmentDownloadJob: JobExecutor { let downloadPromise: Promise = { guard let downloadUrl: String = attachment.downloadUrl, - let fileId: String = downloadUrl - .split(separator: "/") - .last - .map({ String($0) }) + let fileId: String = Attachment.fileId(for: downloadUrl) else { return Promise(error: AttachmentDownloadError.invalidUrl) } diff --git a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift index 18a058f4f..5be30e2f7 100644 --- a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift +++ b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift @@ -55,7 +55,7 @@ public enum AttachmentUploadJob: JobExecutor { .map { response -> String in response.id } }, encrypt: (openGroup == nil), - success: { success(job, false) }, + success: { _ in success(job, false) }, failure: { error in failure(job, error, false) } ) } diff --git a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift index bad9defe0..f7bbceb62 100644 --- a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift +++ b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift @@ -27,6 +27,10 @@ public enum MessageSendJob: JobExecutor { return } + // We need to include 'fileIds' when sending messages with attachments to Open Groups + // so extract them from any associated attachments + var messageFileIds: [String] = [] + if details.message is VisibleMessage { guard let jobId: Int64 = job.id, @@ -36,20 +40,30 @@ public enum MessageSendJob: JobExecutor { return } + // If the original interaction no longer exists then don't bother sending the message (ie. the + // message was deleted before it even got sent) + guard Storage.shared.read({ db in try Interaction.exists(db, id: interactionId) }) == true else { + failure(job, StorageError.objectNotFound, true) + return + } + // Check if there are any attachments associated to this message, and if so // upload them now // // Note: Normal attachments should be sent in a non-durable way but any // attachments for LinkPreviews and Quotes will be processed through this mechanism - let attachmentState: (shouldFail: Bool, shouldDefer: Bool)? = Storage.shared.write { db in + let attachmentState: (shouldFail: Bool, shouldDefer: Bool, fileIds: [String])? = Storage.shared.write { db in let allAttachmentStateInfo: [Attachment.StateInfo] = try Attachment .stateInfo(interactionId: interactionId) .fetchAll(db) + let maybeFileIds: [String?] = allAttachmentStateInfo + .map { Attachment.fileId(for: $0.downloadUrl) } + let fileIds: [String] = maybeFileIds.compactMap { $0 } // If there were failed attachments then this job should fail (can't send a // message which has associated attachments if the attachments fail to upload) guard !allAttachmentStateInfo.contains(where: { $0.state == .failedDownload }) else { - return (true, false) + return (true, false, fileIds) } // Create jobs for any pending (or failed) attachment jobs and insert them into the @@ -102,9 +116,13 @@ public enum MessageSendJob: JobExecutor { // If there were pending or uploading attachments then stop here (we want to // upload them first and then re-run this send job - the 'JobRunner.insert' // method will take care of this) + let isMissingFileIds: Bool = (maybeFileIds.count != fileIds.count) + let hasPendingUploads: Bool = allAttachmentStateInfo.contains(where: { $0.state != .uploaded }) + return ( - false, - allAttachmentStateInfo.contains(where: { $0.state != .uploaded }) + (isMissingFileIds && !hasPendingUploads), + hasPendingUploads, + fileIds ) } @@ -122,6 +140,9 @@ public enum MessageSendJob: JobExecutor { deferred(job) return } + + // Store the fileIds so they can be sent with the open group message content + messageFileIds = (attachmentState?.fileIds ?? []) } // Store the sentTimestamp from the message in case it fails due to a clockOutOfSync error @@ -135,7 +156,8 @@ public enum MessageSendJob: JobExecutor { try MessageSender.sendImmediate( db, message: details.message, - to: details.destination, + to: details.destination + .with(fileIds: messageFileIds), interactionId: job.interactionId ) } diff --git a/SessionMessagingKit/Messages/Message+Destination.swift b/SessionMessagingKit/Messages/Message+Destination.swift index a61a05344..e1eaad9bc 100644 --- a/SessionMessagingKit/Messages/Message+Destination.swift +++ b/SessionMessagingKit/Messages/Message+Destination.swift @@ -49,5 +49,21 @@ public extension Message { return .openGroup(roomToken: openGroup.roomToken, server: openGroup.server, fileIds: fileIds) } } + + func with(fileIds: [String]) -> Message.Destination { + // Only Open Group messages support receiving the 'fileIds' + switch self { + case .openGroup(let roomToken, let server, let whisperTo, let whisperMods, _): + return .openGroup( + roomToken: roomToken, + server: server, + whisperTo: whisperTo, + whisperMods: whisperMods, + fileIds: fileIds + ) + + default: return self + } + } } } diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift b/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift index 1a4640753..9942c3012 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift @@ -100,7 +100,7 @@ extension MessageSender { } public static func sendNonDurably(_ db: Database, message: Message, interactionId: Int64?, to destination: Message.Destination) -> Promise { - var attachmentUploadPromises: [Promise] = [Promise.value(())] + var attachmentUploadPromises: [Promise] = [Promise.value(nil)] // If we have an interactionId then check if it has any attachments and process them first if let interactionId: Int64 = interactionId { @@ -124,8 +124,8 @@ extension MessageSender { .filter(ids: attachmentStateInfo.map { $0.attachmentId }) .fetchAll(db)) .defaulting(to: []) - .map { attachment -> Promise in - let (promise, seal) = Promise.pending() + .map { attachment -> Promise in + let (promise, seal) = Promise.pending() attachment.upload( db, @@ -146,7 +146,7 @@ extension MessageSender { .map { response -> String in response.id } }, encrypt: (openGroup == nil), - success: { seal.fulfill(()) }, + success: { fileId in seal.fulfill(fileId) }, failure: { seal.reject($0) } ) @@ -167,10 +167,18 @@ extension MessageSender { if let error: Error = errors.first { return Promise(error: error) } return Storage.shared.writeAsync { db in - try MessageSender.sendImmediate( + let fileIds: [String] = results + .compactMap { result -> String? in + if case .fulfilled(let value) = result { return value } + + return nil + } + + return try MessageSender.sendImmediate( db, message: message, - to: destination, + to: destination + .with(fileIds: fileIds), interactionId: interactionId ) } diff --git a/SessionMessagingKit/Utilities/ProfileManager.swift b/SessionMessagingKit/Utilities/ProfileManager.swift index e163268a4..42c5fd0dd 100644 --- a/SessionMessagingKit/Utilities/ProfileManager.swift +++ b/SessionMessagingKit/Utilities/ProfileManager.swift @@ -150,10 +150,7 @@ public struct ProfileManager { return } guard - let fileId: String = profileUrlStringAtStart - .split(separator: "/") - .last - .map({ String($0) }), + let fileId: String = Attachment.fileId(for: profileUrlStringAtStart), let profileKeyAtStart: OWSAES256Key = profile.profileEncryptionKey, profileKeyAtStart.keyData.count > 0 else { diff --git a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift index 1317224f0..d8c60cc5a 100644 --- a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift +++ b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift @@ -477,6 +477,13 @@ public class PagedDatabaseObserver: TransactionObserver where cacheCurrentEndIndex, currentPageInfo.pageOffset ) + + case .reloadCurrent: + return ( + currentPageInfo.currentCount, + currentPageInfo.pageOffset, + currentPageInfo.pageOffset + ) } }() @@ -570,6 +577,10 @@ public class PagedDatabaseObserver: TransactionObserver where triggerUpdates() } + + public func reload() { + self.load(.reloadCurrent) + } } // MARK: - Convenience @@ -718,6 +729,7 @@ public enum PagedData { case pageBefore case pageAfter case untilInclusive(id: SQLExpression, padding: Int) + case reloadCurrent } public enum Target { From aed1b731857232cf976962edc6548db8e2d2f015 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Tue, 26 Jul 2022 11:36:32 +1000 Subject: [PATCH 2/4] Fixed a few additional issues uncovered Added a explicit "timeout" error to make debugging a little easier Added code to prevent the AttachmentUploadJob from continuing to try to upload if it's associated interaction has been deleted Updated the getDefaultRoomsIfNeeded to make an unauthenticated sequence all to get both capabilities and rooms (so we will know if the server is blinded and retrieve the room images using blinded auth) Fixed a bug where the notification badge wouldn't get cleared when removing data from a device Fixed a bug where adding an open group could start with an invalid 'infoUpdates' value resulting in invalid data getting retrieved Fixed a bug where under certain circumstances the PagedDatabaseObserver was filtering out updates (noticeable when restoring a device, would happen if the currentCount of content was smaller than the pageSize) --- Session/Settings/NukeDataModal.swift | 14 ++-- .../Database/Models/OpenGroup.swift | 2 +- .../Jobs/Types/AttachmentUploadJob.swift | 9 +++ .../Open Groups/OpenGroupAPI.swift | 66 +++++++++++++++++++ .../Open Groups/OpenGroupManager.swift | 24 +++++-- .../Pollers/OpenGroupPoller.swift | 8 +-- SessionUtilitiesKit/Database/Storage.swift | 6 ++ .../Types/PagedDatabaseObserver.swift | 18 +++-- SessionUtilitiesKit/Networking/HTTP.swift | 9 ++- 9 files changed, 133 insertions(+), 23 deletions(-) diff --git a/Session/Settings/NukeDataModal.swift b/Session/Settings/NukeDataModal.swift index 3f2eb92e5..4877e0e90 100644 --- a/Session/Settings/NukeDataModal.swift +++ b/Session/Settings/NukeDataModal.swift @@ -15,7 +15,7 @@ final class NukeDataModal: Modal { let result = UILabel() result.textColor = Colors.text result.font = .boldSystemFont(ofSize: Values.mediumFontSize) - result.text = NSLocalizedString("modal_clear_all_data_title", comment: "") + result.text = "modal_clear_all_data_title".localized() result.numberOfLines = 0 result.lineBreakMode = .byWordWrapping result.textAlignment = .center @@ -27,7 +27,7 @@ final class NukeDataModal: Modal { let result = UILabel() result.textColor = Colors.text.withAlphaComponent(Values.mediumOpacity) result.font = .systemFont(ofSize: Values.smallFontSize) - result.text = NSLocalizedString("modal_clear_all_data_explanation", comment: "") + result.text = "modal_clear_all_data_explanation".localized() result.numberOfLines = 0 result.textAlignment = .center result.lineBreakMode = .byWordWrapping @@ -44,7 +44,7 @@ final class NukeDataModal: Modal { } result.titleLabel!.font = .systemFont(ofSize: Values.smallFontSize) result.setTitleColor(isLightMode ? Colors.destructive : Colors.text, for: UIControl.State.normal) - result.setTitle(NSLocalizedString("TXT_DELETE_TITLE", comment: ""), for: UIControl.State.normal) + result.setTitle("TXT_DELETE_TITLE".localized(), for: UIControl.State.normal) result.addTarget(self, action: #selector(clearAllData), for: UIControl.Event.touchUpInside) return result @@ -66,7 +66,7 @@ final class NukeDataModal: Modal { result.backgroundColor = Colors.buttonBackground result.titleLabel!.font = .systemFont(ofSize: Values.smallFontSize) result.setTitleColor(Colors.text, for: UIControl.State.normal) - result.setTitle(NSLocalizedString("modal_clear_all_data_device_only_button_title", comment: ""), for: UIControl.State.normal) + result.setTitle("modal_clear_all_data_device_only_button_title".localized(), for: UIControl.State.normal) result.addTarget(self, action: #selector(clearDeviceOnly), for: UIControl.Event.touchUpInside) return result @@ -81,7 +81,7 @@ final class NukeDataModal: Modal { } result.titleLabel!.font = .systemFont(ofSize: Values.smallFontSize) result.setTitleColor(isLightMode ? Colors.destructive : Colors.text, for: UIControl.State.normal) - result.setTitle(NSLocalizedString("modal_clear_all_data_entire_account_button_title", comment: ""), for: UIControl.State.normal) + result.setTitle("modal_clear_all_data_entire_account_button_title".localized(), for: UIControl.State.normal) result.addTarget(self, action: #selector(clearEntireAccount), for: UIControl.Event.touchUpInside) return result @@ -211,6 +211,10 @@ final class NukeDataModal: Modal { PushNotificationAPI.unregister(data).retainUntilComplete() } + // Clear the app badge and notifications + AppEnvironment.shared.notificationPresenter.clearAllNotifications() + CurrentAppContext().setMainAppBadgeNumber(0) + // Clear out the user defaults UserDefaults.removeAll() diff --git a/SessionMessagingKit/Database/Models/OpenGroup.swift b/SessionMessagingKit/Database/Models/OpenGroup.swift index 069959b7c..fec7569ba 100644 --- a/SessionMessagingKit/Database/Models/OpenGroup.swift +++ b/SessionMessagingKit/Database/Models/OpenGroup.swift @@ -156,7 +156,7 @@ public extension OpenGroup { imageId: nil, imageData: nil, userCount: 0, - infoUpdates: -1, + infoUpdates: 0, sequenceNumber: 0, inboxLatestMessageId: 0, outboxLatestMessageId: 0 diff --git a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift index 5be30e2f7..ae538be47 100644 --- a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift +++ b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift @@ -34,6 +34,15 @@ public enum AttachmentUploadJob: JobExecutor { return } + // If the original interaction no longer exists then don't bother uploading the attachment (ie. the + // message was deleted before it even got sent) + if let interactionId: Int64 = job.interactionId { + guard Storage.shared.read({ db in try Interaction.exists(db, id: interactionId) }) == true else { + failure(job, StorageError.objectNotFound, true) + return + } + } + // Note: In the AttachmentUploadJob we intentionally don't provide our own db instance to prevent reentrancy // issues when the success/failure closures get called before the upload as the JobRunner will attempt to // update the state of the job immediately diff --git a/SessionMessagingKit/Open Groups/OpenGroupAPI.swift b/SessionMessagingKit/Open Groups/OpenGroupAPI.swift index 65190d397..60011301e 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupAPI.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupAPI.swift @@ -382,6 +382,72 @@ public enum OpenGroupAPI { } } + /// This is a convenience method which constructs a `/sequence` of the `capabilities` and `rooms` requests, refer to those + /// methods for the documented behaviour of each method + public static func capabilitiesAndRooms( + _ db: Database, + on server: String, + authenticated: Bool = true, + using dependencies: SMKDependencies = SMKDependencies() + ) -> Promise<(capabilities: (info: OnionRequestResponseInfoType, data: Capabilities), rooms: (info: OnionRequestResponseInfoType, data: [Room]))> { + let requestResponseType: [BatchRequestInfoType] = [ + // Get the latest capabilities for the server (in case it's a new server or the cached ones are stale) + BatchRequestInfo( + request: Request( + server: server, + endpoint: .capabilities + ), + responseType: Capabilities.self + ), + + // And the room info + BatchRequestInfo( + request: Request( + server: server, + endpoint: .rooms + ), + responseType: [Room].self + ) + ] + + return OpenGroupAPI + .sequence( + db, + server: server, + requests: requestResponseType, + authenticated: authenticated, + using: dependencies + ) + .map { (response: [Endpoint: (OnionRequestResponseInfoType, Codable?)]) -> (capabilities: (OnionRequestResponseInfoType, Capabilities), rooms: (OnionRequestResponseInfoType, [Room])) in + let maybeCapabilities: (info: OnionRequestResponseInfoType, data: Capabilities?)? = response[.capabilities] + .map { info, data in (info, (data as? BatchSubResponse)?.body) } + let maybeRoomResponse: (OnionRequestResponseInfoType, Codable?)? = response + .first(where: { key, _ in + switch key { + case .rooms: return true + default: return false + } + }) + .map { _, value in value } + let maybeRooms: (info: OnionRequestResponseInfoType, data: [Room]?)? = maybeRoomResponse + .map { info, data in (info, (data as? BatchSubResponse<[Room]>)?.body) } + + guard + let capabilitiesInfo: OnionRequestResponseInfoType = maybeCapabilities?.info, + let capabilities: Capabilities = maybeCapabilities?.data, + let roomsInfo: OnionRequestResponseInfoType = maybeRooms?.info, + let rooms: [Room] = maybeRooms?.data + else { + throw HTTP.Error.parsingFailed + } + + return ( + (capabilitiesInfo, capabilities), + (roomsInfo, rooms) + ) + } + } + // MARK: - Messages /// Posts a new message to a room diff --git a/SessionMessagingKit/Open Groups/OpenGroupManager.swift b/SessionMessagingKit/Open Groups/OpenGroupManager.swift index 3b4f5042b..5a2fb5c8a 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupManager.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupManager.swift @@ -775,17 +775,29 @@ public final class OpenGroupManager: NSObject { } let (promise, seal) = Promise<[OpenGroupAPI.Room]>.pending() - + // Try to retrieve the default rooms 8 times attempt(maxRetryCount: 8, recoveringOn: OpenGroupAPI.workQueue) { dependencies.storage.read { db in - OpenGroupAPI.rooms(db, server: OpenGroupAPI.defaultServer, using: dependencies) + OpenGroupAPI.capabilitiesAndRooms( + db, + on: OpenGroupAPI.defaultServer, + authenticated: false, + using: dependencies + ) } - .map { _, data in data } } - .done(on: OpenGroupAPI.workQueue) { items in + .done(on: OpenGroupAPI.workQueue) { response in dependencies.storage.writeAsync { db in - items + // Store the capabilities first + OpenGroupManager.handleCapabilities( + db, + capabilities: response.capabilities.data, + on: OpenGroupAPI.defaultServer + ) + + // Then the rooms + response.rooms.data .compactMap { room -> (String, String)? in // Try to insert an inactive version of the OpenGroup (use 'insert' rather than 'save' // as we want it to fail if the room already exists) @@ -825,7 +837,7 @@ public final class OpenGroupManager: NSObject { } } - seal.fulfill(items) + seal.fulfill(response.rooms.data) } .catch(on: OpenGroupAPI.workQueue) { error in dependencies.mutableCache.mutate { cache in diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift index 6ebedae5e..72f5126b7 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift @@ -182,7 +182,7 @@ extension OpenGroupAPI { switch endpoint { case .capabilities: guard let responseData: BatchSubResponse = endpointResponse.data as? BatchSubResponse, let responseBody: Capabilities = responseData.body else { - SNLog("Open group polling failed due to invalid data.") + SNLog("Open group polling failed due to invalid capability data.") return } @@ -194,7 +194,7 @@ extension OpenGroupAPI { case .roomPollInfo(let roomToken, _): guard let responseData: BatchSubResponse = endpointResponse.data as? BatchSubResponse, let responseBody: RoomPollInfo = responseData.body else { - SNLog("Open group polling failed due to invalid data.") + SNLog("Open group polling failed due to invalid room info data.") return } @@ -209,7 +209,7 @@ extension OpenGroupAPI { case .roomMessagesRecent(let roomToken), .roomMessagesBefore(let roomToken, _), .roomMessagesSince(let roomToken, _): guard let responseData: BatchSubResponse<[Failable]> = endpointResponse.data as? BatchSubResponse<[Failable]>, let responseBody: [Failable] = responseData.body else { - SNLog("Open group polling failed due to invalid data.") + SNLog("Open group polling failed due to invalid messages data.") return } let successfulMessages: [Message] = responseBody.compactMap { $0.value } @@ -231,7 +231,7 @@ extension OpenGroupAPI { case .inbox, .inboxSince, .outbox, .outboxSince: guard let responseData: BatchSubResponse<[DirectMessage]?> = endpointResponse.data as? BatchSubResponse<[DirectMessage]?>, !responseData.failedToParseBody else { - SNLog("Open group polling failed due to invalid data.") + SNLog("Open group polling failed due to invalid inbox/outbox data.") return } diff --git a/SessionUtilitiesKit/Database/Storage.swift b/SessionUtilitiesKit/Database/Storage.swift index 3cd783d74..3c1d3871f 100644 --- a/SessionUtilitiesKit/Database/Storage.swift +++ b/SessionUtilitiesKit/Database/Storage.swift @@ -193,6 +193,12 @@ public final class Storage { if !jobTableInfo.contains(where: { $0["name"] == "shouldSkipLaunchBecomeActive" }) { finalError = StorageError.devRemigrationRequired } + // Forcibly change any 'infoUpdates' on open groups from '-1' to '0' (-1 is invalid) + try? db.execute(literal: """ + UPDATE openGroup + SET infoUpdates = 0 + WHERE openGroup.infoUpdates = -1 + """) // TODO: Remove this once everyone has updated onComplete(finalError, needsConfigSync) diff --git a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift index d8c60cc5a..bdc17323e 100644 --- a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift +++ b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift @@ -283,8 +283,10 @@ public class PagedDatabaseObserver: TransactionObserver where let indexesAreSequential: Bool = (indexes.map { $0 - 1 }.dropFirst() == indexes.dropLast()) let hasOneValidIndex: Bool = indexInfo.contains(where: { info -> Bool in info.rowIndex >= updatedPageInfo.pageOffset && ( - info.rowIndex < updatedPageInfo.currentCount || - updatedPageInfo.currentCount == 0 + info.rowIndex < updatedPageInfo.currentCount || ( + updatedPageInfo.currentCount < updatedPageInfo.pageSize && + info.rowIndex <= (updatedPageInfo.pageOffset + updatedPageInfo.pageSize) + ) ) }) @@ -293,8 +295,10 @@ public class PagedDatabaseObserver: TransactionObserver where indexInfo .filter { info -> Bool in info.rowIndex >= updatedPageInfo.pageOffset && ( - info.rowIndex < updatedPageInfo.currentCount || - updatedPageInfo.currentCount == 0 + info.rowIndex < updatedPageInfo.currentCount || ( + updatedPageInfo.currentCount < updatedPageInfo.pageSize && + info.rowIndex <= (updatedPageInfo.pageOffset + updatedPageInfo.pageSize) + ) ) } .map { info -> Int64 in info.rowId } @@ -1102,8 +1106,10 @@ public class AssociatedRecord: ErasedAssociatedRecord where T: Fet /// commit - this will mean in some cases we cache data which is actually unrelated to the filtered paged data let hasOneValidIndex: Bool = pagedItemIndexes.contains(where: { info -> Bool in info.rowIndex >= pageInfo.pageOffset && ( - info.rowIndex < pageInfo.currentCount || - pageInfo.currentCount == 0 + info.rowIndex < pageInfo.currentCount || ( + pageInfo.currentCount < pageInfo.pageSize && + info.rowIndex <= (pageInfo.pageOffset + pageInfo.pageSize) + ) ) }) diff --git a/SessionUtilitiesKit/Networking/HTTP.swift b/SessionUtilitiesKit/Networking/HTTP.swift index 06c7b7f13..9e5946735 100644 --- a/SessionUtilitiesKit/Networking/HTTP.swift +++ b/SessionUtilitiesKit/Networking/HTTP.swift @@ -86,6 +86,7 @@ public enum HTTP { case invalidResponse case maxFileSizeExceeded case httpRequestFailed(statusCode: UInt, data: Data?) + case timeout public var errorDescription: String? { switch self { @@ -95,6 +96,7 @@ public enum HTTP { case .parsingFailed, .invalidResponse: return "Invalid response." case .maxFileSizeExceeded: return "Maximum file size exceeded." case .httpRequestFailed(let statusCode, _): return "HTTP request failed with status code: \(statusCode)." + case .timeout: return "The request timed out." } } } @@ -138,8 +140,13 @@ public enum HTTP { } else { SNLog("\(verb.rawValue) request to \(url) failed.") } + // Override the actual error so that we can correctly catch failed requests in sendOnionRequest(invoking:on:with:) - return seal.reject(Error.httpRequestFailed(statusCode: 0, data: nil)) + switch (error as? NSError)?.code { + case NSURLErrorTimedOut: return seal.reject(Error.timeout) + default: return seal.reject(Error.httpRequestFailed(statusCode: 0, data: nil)) + } + } if let error = error { SNLog("\(verb.rawValue) request to \(url) failed due to error: \(error).") From ae4999c3a78bfe7e90ac003c1fdd279a3b9f00dc Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 28 Jul 2022 15:36:56 +1000 Subject: [PATCH 3/4] Fixed a couple of crashes and a couple of other bugs Fixed a crash due to database re-entrancy Fixed an issue where interacting with a push notification wouldn't open the conversation in some cases Added code to prevent a user from being able to start a DM with a blinded id Updated some open group polling logs to be clearer --- Session.xcodeproj/project.pbxproj | 20 +------ Session/DMs/NewDMVC.swift | 54 +++++++++++-------- Session/Meta/AppDelegate.swift | 12 ++++- .../Jobs/Types/UpdateProfilePictureJob.swift | 9 +++- .../Pollers/OpenGroupPoller.swift | 10 +++- 5 files changed, 62 insertions(+), 43 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index e6a50a7d4..985a383ac 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -3070,8 +3070,6 @@ children = ( C3C2A5B0255385C700C340D1 /* Meta */, FD17D79D27F40CAA00122BE0 /* Database */, - FD17D7DF27F67BC400122BE0 /* Models */, - FD17D7D027F5795300122BE0 /* Types */, FDC438AF27BB158500C60D73 /* Models */, C3C2A5CD255385F300C340D1 /* Utilities */, C3C2A5B9255385ED00C340D1 /* Configuration.swift */, @@ -3558,20 +3556,6 @@ path = Models; sourceTree = ""; }; - FD17D7D027F5795300122BE0 /* Types */ = { - isa = PBXGroup; - children = ( - ); - path = Types; - sourceTree = ""; - }; - FD17D7DF27F67BC400122BE0 /* Models */ = { - isa = PBXGroup; - children = ( - ); - path = Models; - sourceTree = ""; - }; FD17D7E827F6A1B800122BE0 /* LegacyDatabase */ = { isa = PBXGroup; children = ( @@ -6830,7 +6814,7 @@ CODE_SIGN_ENTITLEMENTS = Session/Meta/Signal.entitlements; CODE_SIGN_IDENTITY = "iPhone Developer"; "CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 357; + CURRENT_PROJECT_VERSION = 360; DEVELOPMENT_TEAM = SUQ8J2PCT7; FRAMEWORK_SEARCH_PATHS = ( "$(inherited)", @@ -6902,7 +6886,7 @@ CODE_SIGN_ENTITLEMENTS = Session/Meta/Signal.entitlements; CODE_SIGN_IDENTITY = "iPhone Developer"; "CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 357; + CURRENT_PROJECT_VERSION = 360; DEVELOPMENT_TEAM = SUQ8J2PCT7; FRAMEWORK_SEARCH_PATHS = ( "$(inherited)", diff --git a/Session/DMs/NewDMVC.swift b/Session/DMs/NewDMVC.swift index 8767c8263..b55fbb3df 100644 --- a/Session/DMs/NewDMVC.swift +++ b/Session/DMs/NewDMVC.swift @@ -134,30 +134,42 @@ final class NewDMVC : BaseVC, UIPageViewControllerDataSource, UIPageViewControll } fileprivate func startNewDMIfPossible(with onsNameOrPublicKey: String) { - if ECKeyPair.isValidHexEncodedPublicKey(candidate: onsNameOrPublicKey) { + let maybeSessionId: SessionId? = SessionId(from: onsNameOrPublicKey) + + if ECKeyPair.isValidHexEncodedPublicKey(candidate: onsNameOrPublicKey) && maybeSessionId?.prefix == .standard { startNewDM(with: onsNameOrPublicKey) - } else { - // This could be an ONS name - ModalActivityIndicatorViewController.present(fromViewController: navigationController!, canCancel: false) { [weak self] modalActivityIndicator in - SnodeAPI.getSessionID(for: onsNameOrPublicKey).done { sessionID in - modalActivityIndicator.dismiss { - self?.startNewDM(with: sessionID) - } - }.catch { error in - modalActivityIndicator.dismiss { - var messageOrNil: String? - if let error = error as? SnodeAPIError { - switch error { - case .decryptionFailed, .hashingFailed, .validationFailed: - messageOrNil = error.errorDescription - default: break - } + return + } + + // This could be an ONS name + ModalActivityIndicatorViewController.present(fromViewController: navigationController!, canCancel: false) { [weak self] modalActivityIndicator in + SnodeAPI.getSessionID(for: onsNameOrPublicKey).done { sessionID in + modalActivityIndicator.dismiss { + self?.startNewDM(with: sessionID) + } + }.catch { error in + modalActivityIndicator.dismiss { + var messageOrNil: String? + if let error = error as? SnodeAPIError { + switch error { + case .decryptionFailed, .hashingFailed, .validationFailed: + messageOrNil = error.errorDescription + default: break } - let message = messageOrNil ?? "Please check the Session ID or ONS name and try again" - let alert = UIAlertController(title: "Error", message: message, preferredStyle: .alert) - alert.addAction(UIAlertAction(title: NSLocalizedString("BUTTON_OK", comment: ""), style: .default, handler: nil)) - self?.presentAlert(alert) } + let message: String = { + if let messageOrNil: String = messageOrNil { + return messageOrNil + } + + return (maybeSessionId?.prefix == .blinded ? + "You can only send messages to Blinded IDs from within an Open Group" : + "Please check the Session ID or ONS name and try again" + ) + }() + let alert = UIAlertController(title: "Error", message: message, preferredStyle: .alert) + alert.addAction(UIAlertAction(title: "BUTTON_OK".localized(), style: .default, handler: nil)) + self?.presentAlert(alert) } } } diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index 84286342e..7067ea3db 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -114,6 +114,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD return true } + func applicationWillEnterForeground(_ application: UIApplication) { + /// **Note:** We _shouldn't_ need to call this here but for some reason the OS doesn't seems to + /// be calling the `userNotificationCenter(_:,didReceive:withCompletionHandler:)` + /// method when the device is locked while the app is in the foreground (or if the user returns to the + /// springboard without swapping to another app) - adding this here in addition to the one in + /// `appDidFinishLaunching` seems to fix this odd behaviour (even though it doesn't match + /// Apple's documentation on the matter) + UNUserNotificationCenter.current().delegate = self + } + func applicationDidEnterBackground(_ application: UIApplication) { DDLog.flushLog() @@ -149,7 +159,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD /// no longer always called before `applicationDidBecomeActive` we need to trigger the "clear notifications" logic /// within the `runNowOrWhenAppDidBecomeReady` callback and dispatch to the next run loop to ensure it runs after /// the notification has actually been handled - DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { [weak self] in + DispatchQueue.main.async { [weak self] in self?.clearAllNotificationsAndRestoreBadgeCount() } } diff --git a/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift b/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift index 803ea34b9..260f150be 100644 --- a/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift +++ b/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift @@ -52,7 +52,14 @@ public enum UpdateProfilePictureJob: JobExecutor { image: nil, imageFilePath: profileFilePath, requiredSync: true, - success: { _, _ in success(job, false) }, + success: { _, _ in + // Need to call the 'success' closure asynchronously on the queue to prevent a reentrancy + // issue as it will write to the database and this closure is already called within + // another database write + queue.async { + success(job, false) + } + }, failure: { error in failure(job, error, false) } ) } diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift index 72f5126b7..c46938844 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift @@ -194,7 +194,10 @@ extension OpenGroupAPI { case .roomPollInfo(let roomToken, _): guard let responseData: BatchSubResponse = endpointResponse.data as? BatchSubResponse, let responseBody: RoomPollInfo = responseData.body else { - SNLog("Open group polling failed due to invalid room info data.") + switch (endpointResponse.data as? BatchSubResponse)?.code { + case 404: SNLog("Open group polling failed to retrieve info for unknown room '\(roomToken)'.") + default: SNLog("Open group polling failed due to invalid room info data.") + } return } @@ -209,7 +212,10 @@ extension OpenGroupAPI { case .roomMessagesRecent(let roomToken), .roomMessagesBefore(let roomToken, _), .roomMessagesSince(let roomToken, _): guard let responseData: BatchSubResponse<[Failable]> = endpointResponse.data as? BatchSubResponse<[Failable]>, let responseBody: [Failable] = responseData.body else { - SNLog("Open group polling failed due to invalid messages data.") + switch (endpointResponse.data as? BatchSubResponse<[Failable]>)?.code { + case 404: SNLog("Open group polling failed to retrieve messages for unknown room '\(roomToken)'.") + default: SNLog("Open group polling failed due to invalid messages data.") + } return } let successfulMessages: [Message] = responseBody.compactMap { $0.value } From c022f7cda23afbf8d1fe480a05f56c5de55834cd Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 28 Jul 2022 18:26:22 +1000 Subject: [PATCH 4/4] Added an exponential back-off to polling open groups when they fail to poll --- .../_001_InitialSetupMigration.swift | 3 + .../Database/Models/OpenGroup.swift | 14 +++- .../Pollers/OpenGroupPoller.swift | 66 +++++++++++++++++-- SessionUtilitiesKit/Database/Storage.swift | 8 +++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift b/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift index 0e18775b6..61747d7ea 100644 --- a/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift +++ b/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift @@ -140,6 +140,9 @@ enum _001_InitialSetupMigration: Migration { t.column(.sequenceNumber, .integer).notNull() t.column(.inboxLatestMessageId, .integer).notNull() t.column(.outboxLatestMessageId, .integer).notNull() + t.column(.pollFailureCount, .integer) + .notNull() + .defaults(to: 0) } /// Create a full-text search table synchronized with the OpenGroup table diff --git a/SessionMessagingKit/Database/Models/OpenGroup.swift b/SessionMessagingKit/Database/Models/OpenGroup.swift index fec7569ba..71912d32d 100644 --- a/SessionMessagingKit/Database/Models/OpenGroup.swift +++ b/SessionMessagingKit/Database/Models/OpenGroup.swift @@ -26,6 +26,7 @@ public struct OpenGroup: Codable, Identifiable, FetchableRecord, PersistableReco case sequenceNumber case inboxLatestMessageId case outboxLatestMessageId + case pollFailureCount } public var id: String { threadId } // Identifiable @@ -86,6 +87,9 @@ public struct OpenGroup: Codable, Identifiable, FetchableRecord, PersistableReco /// updated whenever this value changes) public let outboxLatestMessageId: Int64 + /// The number of times this room has failed to poll since the last successful poll + public let pollFailureCount: Int64 + // MARK: - Relationships public var thread: QueryInterfaceRequest { @@ -117,7 +121,8 @@ public struct OpenGroup: Codable, Identifiable, FetchableRecord, PersistableReco infoUpdates: Int64, sequenceNumber: Int64 = 0, inboxLatestMessageId: Int64 = 0, - outboxLatestMessageId: Int64 = 0 + outboxLatestMessageId: Int64 = 0, + pollFailureCount: Int64 = 0 ) { self.threadId = OpenGroup.idFor(roomToken: roomToken, server: server) self.server = server.lowercased() @@ -133,6 +138,7 @@ public struct OpenGroup: Codable, Identifiable, FetchableRecord, PersistableReco self.sequenceNumber = sequenceNumber self.inboxLatestMessageId = inboxLatestMessageId self.outboxLatestMessageId = outboxLatestMessageId + self.pollFailureCount = pollFailureCount } } @@ -159,7 +165,8 @@ public extension OpenGroup { infoUpdates: 0, sequenceNumber: 0, inboxLatestMessageId: 0, - outboxLatestMessageId: 0 + outboxLatestMessageId: 0, + pollFailureCount: 0 ) } @@ -192,7 +199,8 @@ extension OpenGroup: CustomStringConvertible, CustomDebugStringConvertible { "infoUpdates: \(infoUpdates)", "sequenceNumber: \(sequenceNumber)", "inboxLatestMessageId: \(inboxLatestMessageId)", - "outboxLatestMessageId: \(outboxLatestMessageId))" + "outboxLatestMessageId: \(outboxLatestMessageId)", + "pollFailureCount: \(pollFailureCount))" ].joined(separator: ", ") } } diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift index c46938844..52c2714fd 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift @@ -15,7 +15,8 @@ extension OpenGroupAPI { // MARK: - Settings - private static let pollInterval: TimeInterval = 4 + private static let minPollInterval: TimeInterval = 3 + private static let maxPollInterval: Double = (60 * 60) internal static let maxInactivityPeriod: Double = (14 * 24 * 60 * 60) // MARK: - Lifecycle @@ -28,10 +29,7 @@ extension OpenGroupAPI { guard !hasStarted else { return } hasStarted = true - timer = Timer.scheduledTimerOnMainThread(withTimeInterval: Poller.pollInterval, repeats: true) { _ in - self.poll(using: dependencies).retainUntilComplete() - } - poll(using: dependencies).retainUntilComplete() + pollRecursively(using: dependencies) } @objc public func stop() { @@ -41,6 +39,30 @@ extension OpenGroupAPI { // MARK: - Polling + private func pollRecursively(using dependencies: OpenGroupManager.OGMDependencies = OpenGroupManager.OGMDependencies()) { + guard hasStarted else { return } + + let minPollFailureCount: TimeInterval = Storage.shared + .read { db in + try OpenGroup + .filter(OpenGroup.Columns.server == server) + .select(min(OpenGroup.Columns.pollFailureCount)) + .asRequest(of: TimeInterval.self) + .fetchOne(db) + } + .defaulting(to: 0) + let nextPollInterval: TimeInterval = getInterval(for: minPollFailureCount, minInterval: Poller.minPollInterval, maxInterval: Poller.maxPollInterval) + + poll(using: dependencies).retainUntilComplete() + timer = Timer.scheduledTimerOnMainThread(withTimeInterval: nextPollInterval, repeats: false) { [weak self] timer in + timer.invalidate() + + Threading.pollerQueue.async { + self?.pollRecursively(using: dependencies) + } + } + } + @discardableResult public func poll(using dependencies: OpenGroupManager.OGMDependencies = OpenGroupManager.OGMDependencies()) -> Promise { return poll(isBackgroundPoll: false, isPostCapabilitiesRetry: false, using: dependencies) @@ -83,6 +105,14 @@ extension OpenGroupAPI { cache.timeSinceLastPoll[server] = Date().timeIntervalSince1970 UserDefaults.standard[.lastOpen] = Date() } + + // Reset the failure count + Storage.shared.writeAsync { db in + try OpenGroup + .filter(OpenGroup.Columns.server == server) + .updateAll(db, OpenGroup.Columns.pollFailureCount.set(to: 0)) + } + SNLog("Open group polling finished for \(server).") seal.fulfill(()) } @@ -97,7 +127,24 @@ extension OpenGroupAPI { ) .done(on: OpenGroupAPI.workQueue) { [weak self] didHandleError in if !didHandleError { - SNLog("Open group polling failed due to error: \(error).") + // Increase the failure count + let pollFailureCount: Int64 = Storage.shared + .read { db in + try OpenGroup + .filter(OpenGroup.Columns.server == server) + .select(max(OpenGroup.Columns.pollFailureCount)) + .asRequest(of: Int64.self) + .fetchOne(db) + } + .defaulting(to: 0) + + Storage.shared.writeAsync { db in + try OpenGroup + .filter(OpenGroup.Columns.server == server) + .updateAll(db, OpenGroup.Columns.pollFailureCount.set(to: (pollFailureCount + 1))) + } + + SNLog("Open group polling failed due to error: \(error). Setting failure count to \(pollFailureCount).") } self?.isPolling = false @@ -265,4 +312,11 @@ extension OpenGroupAPI { } } } + + // MARK: - Convenience + + fileprivate static func getInterval(for failureCount: TimeInterval, minInterval: TimeInterval, maxInterval: TimeInterval) -> TimeInterval { + // Arbitrary backoff factor... + return min(maxInterval, minInterval + pow(2, failureCount)) + } } diff --git a/SessionUtilitiesKit/Database/Storage.swift b/SessionUtilitiesKit/Database/Storage.swift index 3c1d3871f..92ba77ec0 100644 --- a/SessionUtilitiesKit/Database/Storage.swift +++ b/SessionUtilitiesKit/Database/Storage.swift @@ -200,6 +200,14 @@ public final class Storage { WHERE openGroup.infoUpdates = -1 """) // TODO: Remove this once everyone has updated + let openGroupTableInfo: [Row] = (try? Row.fetchAll(db, sql: "PRAGMA table_info(openGroup)")) + .defaulting(to: []) + if !openGroupTableInfo.contains(where: { $0["name"] == "pollFailureCount" }) { + try? db.execute(literal: """ + ALTER TABLE openGroup + ADD pollFailureCount INTEGER NOT NULL DEFAULT 0 + """) + } onComplete(finalError, needsConfigSync) }