From aed1b731857232cf976962edc6548db8e2d2f015 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Tue, 26 Jul 2022 11:36:32 +1000 Subject: [PATCH] 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).")