From 977c2051ed6e78775fcb2a7025468120e0fe3862 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 10 May 2023 17:27:36 +1000 Subject: [PATCH] Fixed a few bugs uncovered with further testing Added some more logs to libSession build script and tweaked the stdout location Added shadow threads to the GarbageCollectionJob Changed the seed node retries to 2 because it's likely we will swap to another seed node pretty quickly which could resolve the issue Fixed a bug where the user could get kicked from a draft conversation if they get a contacts update before sending a message Fixed a bug where message status or media message download statuses would trigger the conversation to jump to the bottom --- Scripts/build_libSession_util.sh | 29 +++++++++++++++---- Session/Conversations/ConversationVC.swift | 26 +++++++++++------ Session/Shared/SessionTableViewModel.swift | 4 --- .../Jobs/Types/GarbageCollectionJob.swift | 29 +++++++++++++++++++ SessionMessagingKit/Messages/Message.swift | 22 ++++++++++++++ .../Open Groups/OpenGroupManager.swift | 6 ++++ .../Sending & Receiving/MessageSender.swift | 12 ++------ .../SessionUtil+Contacts.swift | 25 ++++++++++++++-- .../SessionThreadViewModel.swift | 2 -- SessionSnodeKit/Networking/SnodeAPI.swift | 2 +- 10 files changed, 124 insertions(+), 33 deletions(-) diff --git a/Scripts/build_libSession_util.sh b/Scripts/build_libSession_util.sh index cfd934ac6..bb3cb671b 100755 --- a/Scripts/build_libSession_util.sh +++ b/Scripts/build_libSession_util.sh @@ -25,19 +25,32 @@ # Need to set the path or we won't find cmake PATH=${PATH}:/usr/local/bin:/opt/homebrew/bin:/sbin/md5 -# Direct the output to a log file -exec > "${TARGET_BUILD_DIR}/libsession_util_output.log" 2>&1 +# Ensure the build directory exists (in case we need it before XCode creates it) +mkdir -p "${TARGET_BUILD_DIR}" # Remove any old build errors rm -rf "${TARGET_BUILD_DIR}/libsession_util_error.log" # First ensure cmake is installed (store the error in a log and exit with a success status - xcode will output the error) +echo "info: Validating build requirements" + if ! which cmake > /dev/null; then - echo "error: cmake is required to build, please install (can install via homebrew with 'brew install cmake')." > "${TARGET_BUILD_DIR}/error.log" + touch "${TARGET_BUILD_DIR}/libsession_util_error.log" + echo "error: cmake is required to build, please install (can install via homebrew with 'brew install cmake')." + echo "error: cmake is required to build, please install (can install via homebrew with 'brew install cmake')." > "${TARGET_BUILD_DIR}/libsession_util_error.log" exit 0 fi +if [ ! -d "${SRCROOT}/LibSession-Util" ] || [ ! -d "${SRCROOT}/LibSession-Util/src" ]; then + touch "${TARGET_BUILD_DIR}/libsession_util_error.log" + echo "error: Need to fetch LibSession-Util submodule." + echo "error: Need to fetch LibSession-Util submodule." > "${TARGET_BUILD_DIR}/libsession_util_error.log" + exit 1 +fi + # Generate a hash of the libSession-util source files and check if they differ from the last hash +echo "info: Checking for changes to source" + NEW_SOURCE_HASH=$(find "${SRCROOT}/LibSession-Util/src" -type f -exec md5 {} + | awk '{print $NF}' | sort | md5 | awk '{print $NF}') NEW_HEADER_HASH=$(find "${SRCROOT}/LibSession-Util/include" -type f -exec md5 {} + | awk '{print $NF}' | sort | md5 | awk '{print $NF}') @@ -62,13 +75,15 @@ if [ "${NEW_SOURCE_HASH}" != "${OLD_SOURCE_HASH}" ] || [ "${NEW_HEADER_HASH}" != rm -rf "${TARGET_BUILD_DIR}/libsession-util.a" rm -rf "${TARGET_BUILD_DIR}/libsession-util.xcframework" rm -rf "${BUILD_DIR}/libsession-util.xcframework" - + # Trigger the new build cd "${SRCROOT}/LibSession-Util" result=$(./utils/ios.sh "libsession-util" false) if [ $? -ne 0 ]; then - echo "error: Failed to build libsession-util (See details in '${TARGET_BUILD_DIR}/pre-action-output.log')." > "${TARGET_BUILD_DIR}/error.log" + touch "${TARGET_BUILD_DIR}/libsession_util_error.log" + echo "error: Failed to build libsession-util (See details in '${TARGET_BUILD_DIR}/pre-action-output.log')." + echo "error: Failed to build libsession-util (See details in '${TARGET_BUILD_DIR}/pre-action-output.log')." > "${TARGET_BUILD_DIR}/libsession_util_error.log" exit 0 fi @@ -76,6 +91,10 @@ if [ "${NEW_SOURCE_HASH}" != "${OLD_SOURCE_HASH}" ] || [ "${NEW_HEADER_HASH}" != echo "${NEW_SOURCE_HASH}" > "${TARGET_BUILD_DIR}/libsession_util_source_hash.log" echo "${NEW_HEADER_HASH}" > "${TARGET_BUILD_DIR}/libsession_util_header_hash.log" echo "${ARCHS[*]}" > "${TARGET_BUILD_DIR}/libsession_util_archs.log" + echo "" + echo "info: Build complete" +else + echo "info: Build is up-to-date" fi # Move the target-specific libSession-util build to the parent build directory (so XCode can have a reference to a single build) diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index e9b28fa65..9932102b8 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -734,6 +734,11 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers initialIsBlocked: (viewModel.threadData.threadIsBlocked == true) ) + messageRequestDescriptionLabel.text = (updatedThreadData.threadRequiresApproval == false ? + "MESSAGE_REQUESTS_INFO".localized() : + "MESSAGE_REQUEST_PENDING_APPROVAL_INFO".localized() + ) + let messageRequestsViewWasVisible: Bool = ( messageRequestStackView.isHidden == false ) @@ -865,15 +870,18 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers self.viewModel.updateInteractionData(updatedData) self.tableView.reloadData() - // We need to dispatch to the next run loop because it seems trying to scroll immediately after - // triggering a 'reloadData' doesn't work - DispatchQueue.main.async { [weak self] in - self?.scrollToBottom(isAnimated: false) - - // Note: The scroll button alpha won't get set correctly in this case so we forcibly set it to - // have an alpha of 0 to stop it appearing buggy - self?.scrollButton.alpha = 0 - self?.unreadCountView.alpha = 0 + // If we just sent a message then we want to jump to the bottom of the conversation instantly + if didSendMessageBeforeUpdate { + // We need to dispatch to the next run loop because it seems trying to scroll immediately after + // triggering a 'reloadData' doesn't work + DispatchQueue.main.async { [weak self] in + self?.scrollToBottom(isAnimated: false) + + // Note: The scroll button alpha won't get set correctly in this case so we forcibly set it to + // have an alpha of 0 to stop it appearing buggy + self?.scrollButton.alpha = 0 + self?.unreadCountView.alpha = 0 + } } return } diff --git a/Session/Shared/SessionTableViewModel.swift b/Session/Shared/SessionTableViewModel.swift index 9c97082da..cfd772876 100644 --- a/Session/Shared/SessionTableViewModel.swift +++ b/Session/Shared/SessionTableViewModel.swift @@ -41,10 +41,6 @@ class SessionTableViewModel { Just(nil).eraseToAnyPublisher() } - open var settingsData: [SectionModel] { preconditionFailure("abstract class - override in subclass") } - open var observableSettingsData: ObservableData { - preconditionFailure("abstract class - override in subclass") - } open var footerView: AnyPublisher { Just(nil).eraseToAnyPublisher() } open var footerButtonInfo: AnyPublisher { Just(nil).eraseToAnyPublisher() diff --git a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift index f5022cf36..5649183fe 100644 --- a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift +++ b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift @@ -296,6 +296,34 @@ public enum GarbageCollectionJob: JobExecutor { .filter(PendingReadReceipt.Columns.serverExpirationTimestamp <= timestampNow) .deleteAll(db) } + + if finalTypesToCollect.contains(.shadowThreads) { + // Shadow threads are thread records which were created to start a conversation that + // didn't actually get turned into conversations (ie. the app was closed or crashed + // before the user sent a message) + let thread: TypedTableAlias = TypedTableAlias() + let contact: TypedTableAlias = TypedTableAlias() + let openGroup: TypedTableAlias = TypedTableAlias() + let closedGroup: TypedTableAlias = TypedTableAlias() + + try db.execute(literal: """ + DELETE FROM \(SessionThread.self) + WHERE \(Column.rowID) IN ( + SELECT \(thread.alias[Column.rowID]) + FROM \(SessionThread.self) + LEFT JOIN \(Contact.self) ON \(contact[.id]) = \(thread[.id]) + LEFT JOIN \(OpenGroup.self) ON \(openGroup[.threadId]) = \(thread[.id]) + LEFT JOIN \(ClosedGroup.self) ON \(closedGroup[.threadId]) = \(thread[.id]) + WHERE ( + \(contact[.id]) IS NULL AND + \(openGroup[.threadId]) IS NULL AND + \(closedGroup[.threadId]) IS NULL AND + \(thread[.shouldBeVisible]) = false AND + \(SQL("\(thread[.id]) != \(getUserHexEncodedPublicKey(db))")) + ) + ) + """) + } }, completion: { _, _ in // Dispatch async so we can swap from the write queue to a read one (we are done writing) @@ -450,6 +478,7 @@ extension GarbageCollectionJob { case orphanedAttachmentFiles case orphanedProfileAvatars case expiredPendingReadReceipts + case shadowThreads } public struct Details: Codable { diff --git a/SessionMessagingKit/Messages/Message.swift b/SessionMessagingKit/Messages/Message.swift index 715023e30..97d6198e9 100644 --- a/SessionMessagingKit/Messages/Message.swift +++ b/SessionMessagingKit/Messages/Message.swift @@ -206,6 +206,28 @@ public extension Message { } } + static func threadId(forMessage message: Message, destination: Message.Destination) -> String { + switch destination { + case .contact(let publicKey): + // Extract the 'syncTarget' value if there is one + let maybeSyncTarget: String? + + switch message { + case let message as VisibleMessage: maybeSyncTarget = message.syncTarget + case let message as ExpirationTimerUpdate: maybeSyncTarget = message.syncTarget + default: maybeSyncTarget = nil + } + + return (maybeSyncTarget ?? publicKey) + + case .closedGroup(let groupPublicKey): return groupPublicKey + case .openGroup(let roomToken, let server, _, _, _): + return OpenGroup.idFor(roomToken: roomToken, server: server) + + case .openGroupInbox(_, _, let blindedPublicKey): return blindedPublicKey + } + } + static func processRawReceivedMessage( _ db: Database, rawMessage: SnodeReceivedMessage diff --git a/SessionMessagingKit/Open Groups/OpenGroupManager.swift b/SessionMessagingKit/Open Groups/OpenGroupManager.swift index 3f33981da..3716460df 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupManager.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupManager.swift @@ -382,6 +382,12 @@ public final class OpenGroupManager { .filter(id: openGroupId) .deleteAll(db) + // Remove any MessageProcessRecord entries (we will want to reprocess all OpenGroup messages + // if they get re-added) + _ = try? ControlMessageProcessRecord + .filter(ControlMessageProcessRecord.Columns.threadId == openGroupId) + .deleteAll(db) + // Remove the open group (no foreign key to the thread so it won't auto-delete) if server?.lowercased() != OpenGroupAPI.defaultServer.lowercased() { _ = try? OpenGroup diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender.swift b/SessionMessagingKit/Sending & Receiving/MessageSender.swift index 0f620b89f..1da13d3ec 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender.swift @@ -961,16 +961,8 @@ public final class MessageSender { } } - let threadId: String = { - switch destination { - case .contact(let publicKey): return publicKey - case .closedGroup(let groupPublicKey): return groupPublicKey - case .openGroup(let roomToken, let server, _, _, _): - return OpenGroup.idFor(roomToken: roomToken, server: server) - - case .openGroupInbox(_, _, let blindedPublicKey): return blindedPublicKey - } - }() + // Extract the threadId from the message + let threadId: String = Message.threadId(forMessage: message, destination: destination) // Prevent ControlMessages from being handled multiple times if not supported try? ControlMessageProcessRecord( diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift index 283afe901..c4465cdeb 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift @@ -210,7 +210,7 @@ internal extension SessionUtil { } } - // Delete any contact/thread records which aren't in the config message + /// Delete any contact/thread records which aren't in the config message let syncedContactIds: [String] = targetContactData .map { $0.key } .appending(userPublicKey) @@ -226,7 +226,28 @@ internal extension SessionUtil { .select(.id) .asRequest(of: String.self) .fetchAll(db) - let combinedIds: [String] = contactIdsToRemove.appending(contentsOf: threadIdsToRemove) + + /// When the user opens a brand new conversation this creates a "draft conversation" which has a hidden thread but no + /// contact record, when we receive a contact update this "draft conversation" would be included in the + /// `threadIdsToRemove` which would result in the user getting kicked from the screen and the thread removed, we + /// want to avoid this (as it's essentially a bug) so find any conversations in this state and remove them from the list that + /// will be pruned + let threadT: TypedTableAlias = TypedTableAlias() + let contactT: TypedTableAlias = TypedTableAlias() + let draftConversationIds: [String] = try SQLRequest(""" + SELECT \(threadT[.id]) + FROM \(SessionThread.self) + LEFT JOIN \(Contact.self) ON \(contactT[.id]) = \(threadT[.id]) + WHERE ( + \(SQL("\(threadT[.id]) IN \(threadIdsToRemove)")) AND + \(contactT[.id]) IS NULL + ) + """).fetchAll(db) + + /// Consolidate the ids which should be removed + let combinedIds: [String] = contactIdsToRemove + .appending(contentsOf: threadIdsToRemove) + .filter { !draftConversationIds.contains($0) } if !combinedIds.isEmpty { SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: combinedIds) diff --git a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift index 817ff6582..c70d8388d 100644 --- a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift +++ b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift @@ -784,7 +784,6 @@ public extension SessionThreadViewModel { static func homeFilterSQL(userPublicKey: String) -> SQL { let thread: TypedTableAlias = TypedTableAlias() let contact: TypedTableAlias = TypedTableAlias() - let interaction: TypedTableAlias = TypedTableAlias() return """ \(thread[.shouldBeVisible]) = true AND ( @@ -848,7 +847,6 @@ public extension SessionThreadViewModel { let interaction: TypedTableAlias = TypedTableAlias() let aggregateInteractionLiteral: SQL = SQL(stringLiteral: "aggregateInteraction") - let timestampMsColumnLiteral: SQL = SQL(stringLiteral: Interaction.Columns.timestampMs.name) let closedGroupUserCountTableLiteral: SQL = SQL(stringLiteral: "\(ViewModel.closedGroupUserCountString)_table") let groupMemberGroupIdColumnLiteral: SQL = SQL(stringLiteral: GroupMember.Columns.groupId.name) let profileIdColumnLiteral: SQL = SQL(stringLiteral: Profile.Columns.id.name) diff --git a/SessionSnodeKit/Networking/SnodeAPI.swift b/SessionSnodeKit/Networking/SnodeAPI.swift index 58d33734e..39cf4d59e 100644 --- a/SessionSnodeKit/Networking/SnodeAPI.swift +++ b/SessionSnodeKit/Networking/SnodeAPI.swift @@ -1105,7 +1105,7 @@ public final class SnodeAPI { .compactMap { $0.value } .asSet() } - .retry(4) + .retry(2) .handleEvents( receiveCompletion: { result in switch result {