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)
pull/612/head
Morgan Pretty 3 years ago
parent 9859cf95a4
commit aed1b73185

@ -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()

@ -156,7 +156,7 @@ public extension OpenGroup {
imageId: nil,
imageData: nil,
userCount: 0,
infoUpdates: -1,
infoUpdates: 0,
sequenceNumber: 0,
inboxLatestMessageId: 0,
outboxLatestMessageId: 0

@ -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

@ -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<NoBody, Endpoint>(
server: server,
endpoint: .capabilities
),
responseType: Capabilities.self
),
// And the room info
BatchRequestInfo(
request: Request<NoBody, Endpoint>(
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<Capabilities>)?.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

@ -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

@ -182,7 +182,7 @@ extension OpenGroupAPI {
switch endpoint {
case .capabilities:
guard let responseData: BatchSubResponse<Capabilities> = endpointResponse.data as? BatchSubResponse<Capabilities>, 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<RoomPollInfo> = endpointResponse.data as? BatchSubResponse<RoomPollInfo>, 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<Message>]> = endpointResponse.data as? BatchSubResponse<[Failable<Message>]>, let responseBody: [Failable<Message>] = 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
}

@ -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)

@ -283,8 +283,10 @@ public class PagedDatabaseObserver<ObservedTable, T>: 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<ObservedTable, T>: 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<T, PagedType>: 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)
)
)
})

@ -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).")

Loading…
Cancel
Save