From d4e66dde141a81f92be014f118ad3edac624546e Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 5 Aug 2024 12:23:10 +1000 Subject: [PATCH] Fixed a few more issues found during testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fixed an issue where the conversation 'created' timestamp wasn't correctly getting set when creating a group • Fixed an issue where sending a message to 'Note to Self' may not show the conversation in the conversation list • Fixed an issue where sharing a message with an attachment might not include the attachment • Fixed an issue where list paging wouldn't work in some cases after values were inserted into the database after the currently loaded pages • Added some handling for invalid 'joinedAt' values for groups (seems like we can have an incorrect resolution) --- Session.xcodeproj/project.pbxproj | 4 +- .../ConversationVC+Interaction.swift | 10 ++++- .../Database/Models/SessionThread.swift | 12 ++++-- .../Jobs/Types/MessageSendJob.swift | 7 +--- .../Config Handling/LibSession+Shared.swift | 14 ++++--- .../LibSession+UserGroups.swift | 20 ++++++++-- .../MessageReceiver+ClosedGroups.swift | 8 +++- .../MessageSender+Convenience.swift | 26 ++++++++----- .../Sending & Receiving/MessageSender.swift | 39 ++++++++----------- .../Open Groups/OpenGroupManagerSpec.swift | 3 +- SessionShareExtension/ThreadPickerVC.swift | 19 ++++++--- ...eadDisappearingMessagesViewModelSpec.swift | 3 +- .../ThreadSettingsViewModelSpec.swift | 14 ++++--- .../Types/PagedDatabaseObserver.swift | 1 - 14 files changed, 113 insertions(+), 67 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index f8b99e4b0..7b6fcfb57 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -8109,7 +8109,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 466; + CURRENT_PROJECT_VERSION = 467; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -8187,7 +8187,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Distribution"; - CURRENT_PROJECT_VERSION = 466; + CURRENT_PROJECT_VERSION = 467; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; GCC_NO_COMMON_BLOCKS = YES; diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 068b9acb5..83cfde843 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -560,7 +560,11 @@ extension ConversationVC: if self?.viewModel.threadData.threadShouldBeVisible == false { _ = try SessionThread .filter(id: threadId) - .updateAllAndConfig(db, SessionThread.Columns.shouldBeVisible.set(to: true)) + .updateAllAndConfig( + db, + SessionThread.Columns.shouldBeVisible.set(to: true), + SessionThread.Columns.pinnedPriority.set(to: LibSession.visiblePriority) + ) } // Insert the interaction and associated it with the optimistically inserted message so @@ -1123,7 +1127,9 @@ extension ConversationVC: attachment.isMicrosoftDoc || attachment.contentType == OWSMimeTypeApplicationPdf { - + // FIXME: If given an invalid text file (eg with binary data) this hangs forever + // Note: I tried dispatching after a short delay, detecting that the new UI is invalid and dismissing it + // if so but the dismissal didn't work (we may have to wait on Apple to handle this one) let interactionController: UIDocumentInteractionController = UIDocumentInteractionController(url: fileUrl) interactionController.delegate = self interactionController.presentPreview(animated: true) diff --git a/SessionMessagingKit/Database/Models/SessionThread.swift b/SessionMessagingKit/Database/Models/SessionThread.swift index b55d160a4..2a6785c92 100644 --- a/SessionMessagingKit/Database/Models/SessionThread.swift +++ b/SessionMessagingKit/Database/Models/SessionThread.swift @@ -115,7 +115,7 @@ public struct SessionThread: Codable, Identifiable, Equatable, FetchableRecord, public init( id: String, variant: Variant, - creationDateTimestamp: TimeInterval = (TimeInterval(SnodeAPI.currentOffsetTimestampMs()) / 1000), + creationDateTimestamp: TimeInterval, shouldBeVisible: Bool = false, isPinned: Bool = false, messageDraft: String? = nil, @@ -158,12 +158,14 @@ public extension SessionThread { _ db: Database, id: ID, variant: Variant, + creationDateTimestamp: TimeInterval = (TimeInterval(SnodeAPI.currentOffsetTimestampMs()) / 1000), shouldBeVisible: Bool? ) throws -> SessionThread { guard let existingThread: SessionThread = try? fetchOne(db, id: id) else { return try SessionThread( id: id, variant: variant, + creationDateTimestamp: creationDateTimestamp, shouldBeVisible: (shouldBeVisible ?? false) ).saved(db) } @@ -187,8 +189,12 @@ public extension SessionThread { // would result in an infinite loop) return (try fetchOne(db, id: id)) .defaulting( - to: try SessionThread(id: id, variant: variant, shouldBeVisible: desiredVisibility) - .saved(db) + to: try SessionThread( + id: id, + variant: variant, + creationDateTimestamp: creationDateTimestamp, + shouldBeVisible: desiredVisibility + ).saved(db) ) } diff --git a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift index e10fb6919..df54e81b6 100644 --- a/SessionMessagingKit/Jobs/Types/MessageSendJob.swift +++ b/SessionMessagingKit/Jobs/Types/MessageSendJob.swift @@ -272,12 +272,9 @@ extension MessageSendJob { throw StorageError.decodingFailed } - let message: Message = try variant.decode(from: container, forKey: .message) - var destination: Message.Destination = try container.decode(Message.Destination.self, forKey: .destination) - self = Details( - destination: destination, - message: message + destination: try container.decode(Message.Destination.self, forKey: .destination), + message: try variant.decode(from: container, forKey: .message) ) } diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+Shared.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+Shared.swift index 498496242..5e64ba07c 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+Shared.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+Shared.swift @@ -9,6 +9,14 @@ import SessionUtilitiesKit // MARK: - Convenience +public extension LibSession { + /// A `0` `priority` value indicates visible, but not pinned + static let visiblePriority: Int32 = 0 + + /// A negative `priority` value indicates hidden + static let hiddenPriority: Int32 = -1 +} + internal extension LibSession { /// This is a buffer period within which we will process messages which would result in a config change, any message which would normally /// result in a config change which was sent before `lastConfigMessage.timestamp - configChangeBufferPeriod` will not @@ -34,12 +42,6 @@ internal extension LibSession { return !allColumnsThatTriggerConfigUpdate.isDisjoint(with: targetColumns) } - /// A `0` `priority` value indicates visible, but not pinned - static let visiblePriority: Int32 = 0 - - /// A negative `priority` value indicates hidden - static let hiddenPriority: Int32 = -1 - static func shouldBeVisible(priority: Int32) -> Bool { return (priority >= LibSession.visiblePriority) } diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift index 84e61ee89..746b42738 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift @@ -217,10 +217,24 @@ internal extension LibSession { let name: String = group.name, let lastKeyPair: ClosedGroupKeyPair = group.lastKeyPair, let members: [GroupMember] = group.groupMembers, - let updatedAdmins: Set = group.groupAdmins?.asSet(), - let joinedAt: Int64 = group.joinedAt + let updatedAdmins: Set = group.groupAdmins?.asSet() else { return } + // There were some bugs (somewhere) where the `joinedAt` valid could be in seconds, milliseconds + // or even microseconds so we need to try to detect this and convert it to proper seconds (if we don't + // have a value then we want it to be at the bottom of the list, so default to `0`) + let joinedAt: TimeInterval = { + guard let value: Int64 = group.joinedAt else { return 0 } + + if value > 9_000_000_000_000 { // Microseconds (after May 1973) + return (Double(value) / 1_000_000) + } else if value > 9_000_000_000 { // Milliseconds (after September 2001) + return (Double(value) / 1000) + } + + return TimeInterval(value) // Seconds + }() + if !existingLegacyGroupIds.contains(group.id) { // Add a new group if it doesn't already exist try MessageReceiver.handleNewClosedGroup( @@ -237,7 +251,7 @@ internal extension LibSession { .map { $0.profileId }, admins: updatedAdmins.map { $0.profileId }, expirationTimer: UInt32(group.disappearingConfig?.durationSeconds ?? 0), - formationTimestampMs: UInt64((group.joinedAt.map { $0 * 1000 } ?? latestConfigSentTimestampMs)), + formationTimestampMs: UInt64(joinedAt * 1000), calledFromConfigHandling: true, using: dependencies ) diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift index 7a9374ffd..ce51e65f4 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift @@ -153,7 +153,13 @@ extension MessageReceiver { // Create the group let thread: SessionThread = try SessionThread - .fetchOrCreate(db, id: groupPublicKey, variant: .legacyGroup, shouldBeVisible: true) + .fetchOrCreate( + db, + id: groupPublicKey, + variant: .legacyGroup, + creationDateTimestamp: (TimeInterval(formationTimestampMs) / 1000), + shouldBeVisible: true + ) let closedGroup: ClosedGroup = try ClosedGroup( threadId: groupPublicKey, name: name, diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift b/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift index 728c203a6..141828a5c 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender+Convenience.swift @@ -170,24 +170,32 @@ extension MessageSender { attachments .map { attachment -> AnyPublisher in attachment - .upload( - to: ( - openGroup.map { Attachment.Destination.openGroup($0) } ?? - .fileServer - ), - using: dependencies - ) + .upload(to: (openGroup.map { .openGroup($0) } ?? .fileServer), using: dependencies) } ) .collect() .eraseToAnyPublisher() } - .map { results -> PreparedSendData in + .flatMap { results -> AnyPublisher in // Once the attachments are processed then update the PreparedSendData with // the fileIds associated to the message let fileIds: [String] = results.compactMap { result -> String? in Attachment.fileId(for: result) } - return preparedSendData.with(fileIds: fileIds) + // We need to regenerate the 'PreparedSendData' because otherwise the `SnodeMessage` won't + // contain the attachment data + return dependencies.storage + .writePublisher { db in + try MessageSender.preparedSendData( + db, + message: preparedSendData.message, + to: preparedSendData.destination, + namespace: preparedSendData.destination.defaultNamespace, + interactionId: preparedSendData.interactionId, + using: dependencies + ) + } + .map { updatedData -> PreparedSendData in updatedData.with(fileIds: fileIds) } + .eraseToAnyPublisher() } .eraseToAnyPublisher() } diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender.swift b/SessionMessagingKit/Sending & Receiving/MessageSender.swift index 88951cd4e..66785fa69 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender.swift @@ -15,7 +15,7 @@ public final class MessageSender { let destination: Message.Destination let namespace: SnodeAPI.Namespace? - let message: Message? + let message: Message let interactionId: Int64? let totalAttachmentsUploaded: Int @@ -25,7 +25,7 @@ public final class MessageSender { private init( shouldSend: Bool, - message: Message?, + message: Message, destination: Message.Destination, namespace: SnodeAPI.Namespace?, interactionId: Int64?, @@ -621,17 +621,15 @@ public final class MessageSender { guard expectedAttachmentUploadCount == data.totalAttachmentsUploaded else { // Make sure to actually handle this as a failure (if we don't then the message // won't go into an error state correctly) - if let message: Message = data.message { - dependencies.storage.read { db in - MessageSender.handleFailedMessageSend( - db, - message: message, - destination: data.destination, - with: .attachmentsNotUploaded, - interactionId: data.interactionId, - using: dependencies - ) - } + dependencies.storage.read { db in + MessageSender.handleFailedMessageSend( + db, + message: data.message, + destination: data.destination, + with: .attachmentsNotUploaded, + interactionId: data.interactionId, + using: dependencies + ) } return Fail(error: MessageSenderError.attachmentsNotUploaded) @@ -657,7 +655,6 @@ public final class MessageSender { using dependencies: Dependencies ) -> AnyPublisher { guard - let message: Message = data.message, let namespace: SnodeAPI.Namespace = data.namespace, let snodeMessage: SnodeMessage = data.snodeMessage else { @@ -668,7 +665,7 @@ public final class MessageSender { return dependencies.network .send(.message(snodeMessage, in: namespace), using: dependencies) .flatMap { info, response -> AnyPublisher in - let updatedMessage: Message = message + let updatedMessage: Message = data.message updatedMessage.serverHash = response.hash // Only legacy groups need to manually trigger push notifications now so only create the job @@ -744,7 +741,7 @@ public final class MessageSender { dependencies.storage.read { db in MessageSender.handleFailedMessageSend( db, - message: message, + message: data.message, destination: data.destination, with: .other(error), interactionId: data.interactionId, @@ -765,7 +762,6 @@ public final class MessageSender { using dependencies: Dependencies ) -> AnyPublisher { guard - let message: Message = data.message, case .openGroup(let roomToken, let server, let whisperTo, let whisperMods, let fileIds) = data.destination, let plaintext: Data = data.plaintext else { @@ -791,7 +787,7 @@ public final class MessageSender { .flatMap { $0.send(using: dependencies) } .flatMap { (responseInfo, responseData) -> AnyPublisher in let serverTimestampMs: UInt64? = responseData.posted.map { UInt64(floor($0 * 1000)) } - let updatedMessage: Message = message + let updatedMessage: Message = data.message updatedMessage.openGroupServerMessageId = UInt64(responseData.id) return dependencies.storage.writePublisher { db in @@ -816,7 +812,7 @@ public final class MessageSender { dependencies.storage.read { db in MessageSender.handleFailedMessageSend( db, - message: message, + message: data.message, destination: data.destination, with: .other(error), interactionId: data.interactionId, @@ -834,7 +830,6 @@ public final class MessageSender { using dependencies: Dependencies ) -> AnyPublisher { guard - let message: Message = data.message, case .openGroupInbox(let server, _, let recipientBlindedPublicKey) = data.destination, let ciphertext: Data = data.ciphertext else { @@ -856,7 +851,7 @@ public final class MessageSender { } .flatMap { $0.send(using: dependencies) } .flatMap { (responseInfo, responseData) -> AnyPublisher in - let updatedMessage: Message = message + let updatedMessage: Message = data.message updatedMessage.openGroupServerMessageId = UInt64(responseData.id) return dependencies.storage.writePublisher { db in @@ -881,7 +876,7 @@ public final class MessageSender { dependencies.storage.read { db in MessageSender.handleFailedMessageSend( db, - message: message, + message: data.message, destination: data.destination, with: .other(error), interactionId: data.interactionId, diff --git a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift index d5b0f7e36..c055fc4c0 100644 --- a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift +++ b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift @@ -37,7 +37,8 @@ class OpenGroupManagerSpec: QuickSpec { ) @TestState var testGroupThread: SessionThread! = SessionThread( id: OpenGroup.idFor(roomToken: "testRoom", server: "testServer"), - variant: .community + variant: .community, + creationDateTimestamp: 0 ) @TestState var testOpenGroup: OpenGroup! = OpenGroup( server: "testServer", diff --git a/SessionShareExtension/ThreadPickerVC.swift b/SessionShareExtension/ThreadPickerVC.swift index 7dd57b1d2..bda9b6fad 100644 --- a/SessionShareExtension/ThreadPickerVC.swift +++ b/SessionShareExtension/ThreadPickerVC.swift @@ -249,13 +249,20 @@ final class ThreadPickerVC: UIViewController, UITableViewDataSource, UITableView .subscribe(on: DispatchQueue.global(qos: .userInitiated)) .flatMap { _ in dependencies.storage.writePublisher { db -> MessageSender.PreparedSendData in - guard - let threadVariant: SessionThread.Variant = try SessionThread + guard let thread: SessionThread = try SessionThread.fetchOne(db, id: threadId) else { + throw MessageSenderError.noThread + } + + // Update the thread to be visible (if it isn't already) + if !thread.shouldBeVisible || thread.pinnedPriority == LibSession.hiddenPriority { + _ = try SessionThread .filter(id: threadId) - .select(.variant) - .asRequest(of: SessionThread.Variant.self) - .fetchOne(db) - else { throw MessageSenderError.noThread } + .updateAllAndConfig( + db, + SessionThread.Columns.shouldBeVisible.set(to: true), + SessionThread.Columns.pinnedPriority.set(to: LibSession.visiblePriority) + ) + } // Create the interaction let interaction: Interaction = try Interaction( diff --git a/SessionTests/Conversations/Settings/ThreadDisappearingMessagesViewModelSpec.swift b/SessionTests/Conversations/Settings/ThreadDisappearingMessagesViewModelSpec.swift index edf5519a1..9306d9e13 100644 --- a/SessionTests/Conversations/Settings/ThreadDisappearingMessagesViewModelSpec.swift +++ b/SessionTests/Conversations/Settings/ThreadDisappearingMessagesViewModelSpec.swift @@ -30,7 +30,8 @@ class ThreadDisappearingMessagesSettingsViewModelSpec: QuickSpec { initialData: { db in try SessionThread( id: "TestId", - variant: .contact + variant: .contact, + creationDateTimestamp: 0 ).insert(db) }, using: dependencies diff --git a/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift b/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift index db68bd1d0..fff799427 100644 --- a/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift +++ b/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift @@ -41,7 +41,7 @@ class ThreadSettingsViewModelSpec: QuickSpec { data: Data(hex: TestConstants.publicKey) ).insert(db) - try SessionThread(id: "TestId",variant: .contact).insert(db) + try SessionThread(id: "TestId",variant: .contact, creationDateTimestamp: 0).insert(db) try Profile(id: "05\(TestConstants.publicKey)", name: "TestMe").insert(db) try Profile(id: "TestId", name: "TestUser").insert(db) }, @@ -142,7 +142,8 @@ class ThreadSettingsViewModelSpec: QuickSpec { try SessionThread( id: "05\(TestConstants.publicKey)", - variant: .contact + variant: .contact, + creationDateTimestamp: 0 ).insert(db) } @@ -306,7 +307,8 @@ class ThreadSettingsViewModelSpec: QuickSpec { try SessionThread( id: "TestId", - variant: .contact + variant: .contact, + creationDateTimestamp: 0 ).insert(db) } } @@ -439,7 +441,8 @@ class ThreadSettingsViewModelSpec: QuickSpec { try SessionThread( id: "TestId", - variant: .legacyGroup + variant: .legacyGroup, + creationDateTimestamp: 0 ).insert(db) } @@ -484,7 +487,8 @@ class ThreadSettingsViewModelSpec: QuickSpec { try SessionThread( id: "TestId", - variant: .community + variant: .community, + creationDateTimestamp: 0 ).insert(db) } diff --git a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift index 9b6f6dd3a..fbbc07333 100644 --- a/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift +++ b/SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift @@ -409,7 +409,6 @@ public class PagedDatabaseObserver: TransactionObserver where updatedPageInfo.totalCount + changesToQuery .filter { $0.kind == .insert } - .filter { validChangeRowIds.contains($0.rowId) } .count ) )