From c80b6c720eac03daf56ca757277bc6f56ad6df79 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 31 Mar 2023 12:09:04 +1100 Subject: [PATCH] Fixed the QA issues and a few other bugs Updated the convoInfoVolatile to only ever set `last_read` to the maximum between the current and updated values Fixed an issue where deleting the Note to Self and One-to-one conversations wouldn't reset the 'pinnedPriority' value Fixed an issue with updating legacy group members and losing admin status Fixed an issue where receiving a 'NEW' legacy group control message could revert legacy group changes Fixed a bug where the open group suggestion grid could have broken positioning depending on the number of items Fixed a bug where the UI wouldn't update correctly when network access was lost Fixed a fun bug where one-to-one conversations could reappear after deletion because a new snode was polled and the latest (locally deleted) message was received again Fixed some incorrect accessibility values --- Session/Conversations/ConversationVC.swift | 18 +++- .../Message Cells/VisibleMessageCell.swift | 8 +- .../Settings/ThreadSettingsViewModel.swift | 4 + .../GlobalSearchViewController.swift | 15 +++- Session/Home/HomeViewModel.swift | 45 ++-------- .../MessageRequestsViewController.swift | 10 ++- .../MessageRequestsViewModel.swift | 73 ++++++--------- .../GIFs/GifPickerViewController.swift | 2 +- .../Open Groups/OpenGroupSuggestionGrid.swift | 48 ++++++---- Session/Path/PathStatusView.swift | 17 ++-- Session/Path/PathVC.swift | 16 ++-- Session/Shared/Views/SessionCell.swift | 13 ++- .../Database/Models/ClosedGroup.swift | 2 +- .../Models/ControlMessageProcessRecord.swift | 14 ++- .../Database/Models/GroupMember.swift | 2 +- .../Database/Models/SessionThread.swift | 83 +++++++++++++++++ .../SessionUtil+Contacts.swift | 26 ++++-- .../SessionUtil+ConvoInfoVolatile.swift | 6 +- .../Config Handling/SessionUtil+Shared.swift | 88 +++++++++++++++---- .../SessionUtil+UserGroups.swift | 67 +++++++++----- .../SessionUtil+UserProfile.swift | 14 +++ .../MessageReceiver+MessageRequests.swift | 9 +- .../Utilities/SSKReachabilityManager.swift | 2 + 23 files changed, 398 insertions(+), 184 deletions(-) diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index 35cf9ea39..f25d8022f 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -555,7 +555,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers ) { Storage.shared.writeAsync { db in - _ = try SessionThread + _ = try SessionThread // Intentionally use `deleteAll` here instead of `deleteOrLeave` .filter(id: threadId) .deleteAll(db) } @@ -601,8 +601,18 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers }), let unblindedId: String = blindedLookup.sessionId else { - // If we don't have an unblinded id then something has gone very wrong so pop to the HomeVC - self?.navigationController?.popToRootViewController(animated: true) + // If we don't have an unblinded id then something has gone very wrong so pop to the + // nearest conversation list + let maybeTargetViewController: UIViewController? = self?.navigationController? + .viewControllers + .last(where: { ($0 as? SessionUtilRespondingViewController)?.isConversationList == true }) + + if let targetViewController: UIViewController = maybeTargetViewController { + self?.navigationController?.popToViewController(targetViewController, animated: true) + } + else { + self?.navigationController?.popToRootViewController(animated: true) + } return } @@ -1246,7 +1256,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers target: self, action: #selector(startCall) ) - callButton.accessibilityLabel = "Call button" + callButton.accessibilityLabel = "Call" callButton.isAccessibilityElement = true navigationItem.rightBarButtonItems = [settingsButtonItem, callButton] diff --git a/Session/Conversations/Message Cells/VisibleMessageCell.swift b/Session/Conversations/Message Cells/VisibleMessageCell.swift index 8b21a3d4d..8def1187d 100644 --- a/Session/Conversations/Message Cells/VisibleMessageCell.swift +++ b/Session/Conversations/Message Cells/VisibleMessageCell.swift @@ -301,9 +301,7 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate { lastSearchText: String? ) { self.viewModel = cellViewModel - self.bubbleView.accessibilityIdentifier = "Message Body" - self.bubbleView.isAccessibilityElement = true - self.bubbleView.accessibilityLabel = cellViewModel.body + // We want to add spacing between "clusters" of messages to indicate that time has // passed (even if there wasn't enough time to warrant showing a date header) let shouldAddTopInset: Bool = ( @@ -362,6 +360,10 @@ final class VisibleMessageCell: MessageCell, TappableLabelDelegate { lastSearchText: lastSearchText ) + bubbleView.accessibilityIdentifier = "Message Body" + bubbleView.accessibilityLabel = bodyTappableLabel?.attributedText?.string + bubbleView.isAccessibilityElement = true + // Author label authorLabelTopConstraint.constant = (shouldAddTopInset ? Values.mediumSpacing : 0) authorLabel.isHidden = (cellViewModel.senderName == nil) diff --git a/Session/Conversations/Settings/ThreadSettingsViewModel.swift b/Session/Conversations/Settings/ThreadSettingsViewModel.swift index cd0948d44..8452a7940 100644 --- a/Session/Conversations/Settings/ThreadSettingsViewModel.swift +++ b/Session/Conversations/Settings/ThreadSettingsViewModel.swift @@ -285,6 +285,10 @@ class ThreadSettingsViewModel: SessionTableViewModel // MARK: - SearchSection @@ -20,6 +20,15 @@ class GlobalSearchViewController: BaseVC, UITableViewDelegate, UITableViewDataSo case messages } + // MARK: - SessionUtilRespondingViewController + + let isConversationList: Bool = true + + func forceRefreshIfNeeded() { + // Need to do this as the 'GlobalSearchViewController' doesn't observe database changes + updateSearchResults(searchText: searchText, force: true) + } + // MARK: - Variables private lazy var defaultSearchResults: [SectionModel] = { @@ -152,7 +161,7 @@ class GlobalSearchViewController: BaseVC, UITableViewDelegate, UITableViewDataSo } } - private func updateSearchResults(searchText rawSearchText: String) { + private func updateSearchResults(searchText rawSearchText: String, force: Bool = false) { let searchText = rawSearchText.stripped guard searchText.count > 0 else { @@ -161,7 +170,7 @@ class GlobalSearchViewController: BaseVC, UITableViewDelegate, UITableViewDataSo reloadTableData() return } - guard lastSearchText != searchText else { return } + guard force || lastSearchText != searchText else { return } lastSearchText = searchText diff --git a/Session/Home/HomeViewModel.swift b/Session/Home/HomeViewModel.swift index 5f0158af5..3069bd9ed 100644 --- a/Session/Home/HomeViewModel.swift +++ b/Session/Home/HomeViewModel.swift @@ -368,44 +368,13 @@ public class HomeViewModel { public func deleteOrLeave(threadId: String, threadVariant: SessionThread.Variant) { Storage.shared.writeAsync { db in - switch threadVariant { - case .contact: - // We need to custom handle the 'Note to Self' conversation (it should just be - // hidden rather than deleted - guard threadId != getUserHexEncodedPublicKey(db) else { - _ = try Interaction - .filter(Interaction.Columns.threadId == threadId) - .deleteAll(db) - - _ = try SessionThread - .filter(id: threadId) - .updateAllAndConfig( - db, - SessionThread.Columns.shouldBeVisible.set(to: false) - ) - - return - } - - try SessionUtil - .hide(db, contactIds: [threadId]) - - case .legacyGroup, .group: - MessageSender - .leave(db, groupPublicKey: threadId) - .sinkUntilComplete() - - case .community: - OpenGroupManager.shared.delete( - db, - openGroupId: threadId, - calledFromConfigHandling: false - ) - } - - _ = try SessionThread - .filter(id: threadId) - .deleteAll(db) + try SessionThread.deleteOrLeave( + db, + threadId: threadId, + threadVariant: threadVariant, + shouldSendLeaveMessageForGroups: true, + calledFromConfigHandling: false + ) } } } diff --git a/Session/Home/Message Requests/MessageRequestsViewController.swift b/Session/Home/Message Requests/MessageRequestsViewController.swift index b9be340a0..b5dc1cb6a 100644 --- a/Session/Home/Message Requests/MessageRequestsViewController.swift +++ b/Session/Home/Message Requests/MessageRequestsViewController.swift @@ -7,7 +7,7 @@ import SessionUIKit import SessionMessagingKit import SignalUtilitiesKit -class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDataSource { +class MessageRequestsViewController: BaseVC, SessionUtilRespondingViewController, UITableViewDelegate, UITableViewDataSource { private static let loadingHeaderHeight: CGFloat = 40 private let viewModel: MessageRequestsViewModel = MessageRequestsViewModel() @@ -17,6 +17,10 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat private var isAutoLoadingNextPage: Bool = false private var viewHasAppeared: Bool = false + // MARK: - SessionUtilRespondingViewController + + let isConversationList: Bool = true + // MARK: - Intialization init() { @@ -466,7 +470,7 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat .filter { $0.threadVariant == .contact } .map { $0.threadId }) .defaulting(to: []) - let closedGroupThreadIds: [String] = (viewModel.threadData + let groupThreadIds: [String] = (viewModel.threadData .first { $0.model == .threads }? .elements .filter { $0.threadVariant == .legacyGroup || $0.threadVariant == .group } @@ -483,7 +487,7 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat ) { _ in MessageRequestsViewModel.clearAllRequests( contactThreadIds: contactThreadIds, - closedGroupThreadIds: closedGroupThreadIds + groupThreadIds: groupThreadIds ) }) alertVC.addAction(UIAlertAction(title: "TXT_CANCEL_TITLE".localized(), style: .cancel, handler: nil)) diff --git a/Session/Home/Message Requests/MessageRequestsViewModel.swift b/Session/Home/Message Requests/MessageRequestsViewModel.swift index 9a79a13e4..28e8dc00f 100644 --- a/Session/Home/Message Requests/MessageRequestsViewModel.swift +++ b/Session/Home/Message Requests/MessageRequestsViewModel.swift @@ -185,28 +185,13 @@ public class MessageRequestsViewModel { cancelStyle: .alert_text ) { _ in Storage.shared.write { db in - switch threadVariant { - case .contact: - try SessionUtil - .hide(db, contactIds: [threadId]) - - _ = try SessionThread - .filter(id: threadId) - .deleteAll(db) - - case .legacyGroup, .group: - try ClosedGroup.removeKeysAndUnsubscribe( - db, - threadId: threadId, - removeGroupData: true, - calledFromConfigHandling: false - ) - - // Trigger a config sync - ConfigurationSyncJob.enqueue(db, publicKey: getUserHexEncodedPublicKey(db)) - - default: break - } + try SessionThread.deleteOrLeave( + db, + threadId: threadId, + threadVariant: threadVariant, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: false + ) } completion?() @@ -250,14 +235,13 @@ public class MessageRequestsViewModel { Contact.Columns.didApproveMe.set(to: true) ) - // Sync the removal of the thread from other devices - try SessionUtil - .hide(db, contactIds: [threadId]) - - // Remove the thread - _ = try SessionThread - .filter(id: threadId) - .deleteAll(db) + try SessionThread.deleteOrLeave( + db, + threadId: threadId, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: false + ) }, completion: { _, _ in completion?() } ) @@ -269,24 +253,25 @@ public class MessageRequestsViewModel { static func clearAllRequests( contactThreadIds: [String], - closedGroupThreadIds: [String] + groupThreadIds: [String] ) { // Clear the requests Storage.shared.write { db in - // Sync the removal of the thread from other devices - try SessionUtil - .hide(db, contactIds: contactThreadIds) - - // Remove the threads - _ = try SessionThread - .filter(ids: contactThreadIds) - .deleteAll(db) - - // Remove the groups - try ClosedGroup.removeKeysAndUnsubscribe( + // Remove the one-to-one requests + try SessionThread.deleteOrLeave( db, - threadIds: closedGroupThreadIds, - removeGroupData: true, + threadIds: contactThreadIds, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: false + ) + + // Remove the group requests + try SessionThread.deleteOrLeave( + db, + threadIds: groupThreadIds, + threadVariant: .group, + shouldSendLeaveMessageForGroups: false, calledFromConfigHandling: false ) } diff --git a/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift b/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift index 244ba9fb7..6ee71df52 100644 --- a/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift +++ b/Session/Media Viewing & Editing/GIFs/GifPickerViewController.swift @@ -115,7 +115,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect createViews() - reachability = Reachability.forInternetConnection() + reachability = Environment.shared?.reachabilityManager.reachability NotificationCenter.default.addObserver( self, selector: #selector(reachabilityChanged), diff --git a/Session/Open Groups/OpenGroupSuggestionGrid.swift b/Session/Open Groups/OpenGroupSuggestionGrid.swift index 706df79f4..80b38ae0a 100644 --- a/Session/Open Groups/OpenGroupSuggestionGrid.swift +++ b/Session/Open Groups/OpenGroupSuggestionGrid.swift @@ -18,7 +18,7 @@ final class OpenGroupSuggestionGrid: UIView, UICollectionViewDataSource, UIColle private static let cellHeight: CGFloat = 40 private static let separatorWidth = Values.separatorThickness - private static let numHorizontalCells: CGFloat = (UIDevice.current.isIPad ? 4 : 2) + fileprivate static let numHorizontalCells: Int = (UIDevice.current.isIPad ? 4 : 2) private lazy var layout: LastRowCenteredLayout = { let result = LastRowCenteredLayout() @@ -157,7 +157,7 @@ final class OpenGroupSuggestionGrid: UIView, UICollectionViewDataSource, UIColle spinner.isHidden = true let roomCount: CGFloat = CGFloat(min(rooms.count, 8)) // Cap to a maximum of 8 (4 rows of 2) - let numRows: CGFloat = ceil(roomCount / OpenGroupSuggestionGrid.numHorizontalCells) + let numRows: CGFloat = ceil(roomCount / CGFloat(OpenGroupSuggestionGrid.numHorizontalCells)) let height: CGFloat = ((OpenGroupSuggestionGrid.cellHeight * numRows) + ((numRows - 1) * layout.minimumLineSpacing)) heightConstraint.constant = height collectionView.reloadData() @@ -172,18 +172,18 @@ final class OpenGroupSuggestionGrid: UIView, UICollectionViewDataSource, UIColle // MARK: - Layout func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize { - guard - indexPath.item == (collectionView.numberOfItems(inSection: indexPath.section) - 1) && - indexPath.item % 2 == 0 - else { - let cellWidth: CGFloat = ((maxWidth / OpenGroupSuggestionGrid.numHorizontalCells) - ((OpenGroupSuggestionGrid.numHorizontalCells - 1) * layout.minimumInteritemSpacing)) + let totalItems: Int = collectionView.numberOfItems(inSection: indexPath.section) + let itemsInFinalRow: Int = (totalItems % OpenGroupSuggestionGrid.numHorizontalCells) + + guard indexPath.item >= (totalItems - itemsInFinalRow) && itemsInFinalRow != 0 else { + let cellWidth: CGFloat = ((maxWidth / CGFloat(OpenGroupSuggestionGrid.numHorizontalCells)) - ((CGFloat(OpenGroupSuggestionGrid.numHorizontalCells) - 1) * layout.minimumInteritemSpacing)) return CGSize(width: cellWidth, height: OpenGroupSuggestionGrid.cellHeight) } - // If the last item is by itself then we want to make it wider + // If there isn't an even number of items then we want to calculate proper sizing return CGSize( - width: (Cell.calculatedWith(for: rooms[indexPath.item].name)), + width: Cell.calculatedWith(for: rooms[indexPath.item].name), height: OpenGroupSuggestionGrid.cellHeight ) } @@ -380,16 +380,30 @@ class LastRowCenteredLayout: UICollectionViewFlowLayout { }() guard - (elementAttributes?.count ?? 0) % 2 == 1, - let lastItemAttributes: UICollectionViewLayoutAttributes = elementAttributes?.last + let remainingItems: Int = elementAttributes.map({ $0.count % OpenGroupSuggestionGrid.numHorizontalCells }), + remainingItems != 0, + let lastItems: [UICollectionViewLayoutAttributes] = elementAttributes?.suffix(remainingItems), + !lastItems.isEmpty else { return elementAttributes } - lastItemAttributes.frame = CGRect( - x: ((targetViewWidth - lastItemAttributes.frame.size.width) / 2), - y: lastItemAttributes.frame.origin.y, - width: lastItemAttributes.frame.size.width, - height: lastItemAttributes.frame.size.height - ) + let totalItemWidth: CGFloat = lastItems + .map { $0.frame.size.width } + .reduce(0, +) + let lastRowWidth: CGFloat = (totalItemWidth + (CGFloat(lastItems.count - 1) * minimumInteritemSpacing)) + + // Offset the start width by half of the remaining space + var itemXPos: CGFloat = ((targetViewWidth - lastRowWidth) / 2) + + lastItems.forEach { item in + item.frame = CGRect( + x: itemXPos, + y: item.frame.origin.y, + width: item.frame.size.width, + height: item.frame.size.height + ) + + itemXPos += (item.frame.size.width + minimumInteritemSpacing) + } return elementAttributes } diff --git a/Session/Path/PathStatusView.swift b/Session/Path/PathStatusView.swift index c606b3268..a0fdb805d 100644 --- a/Session/Path/PathStatusView.swift +++ b/Session/Path/PathStatusView.swift @@ -4,6 +4,7 @@ import UIKit import Reachability import SessionUIKit import SessionSnodeKit +import SessionMessagingKit final class PathStatusView: UIView { enum Size { @@ -44,7 +45,7 @@ final class PathStatusView: UIView { // MARK: - Initialization private let size: Size - private let reachability: Reachability = Reachability.forInternetConnection() + private let reachability: Reachability? = Environment.shared?.reachabilityManager.reachability init(size: Size = .small) { self.size = size @@ -76,10 +77,10 @@ final class PathStatusView: UIView { self.set(.width, to: self.size.pointSize) self.set(.height, to: self.size.pointSize) - switch (reachability.isReachable(), OnionRequestAPI.paths.isEmpty) { - case (false, _): setStatus(to: .error) - case (true, true): setStatus(to: .connecting) - case (true, false): setStatus(to: .connected) + switch (reachability?.isReachable(), OnionRequestAPI.paths.isEmpty) { + case (.some(false), _), (nil, _): setStatus(to: .error) + case (.some(true), true): setStatus(to: .connecting) + case (.some(true), false): setStatus(to: .connected) } } @@ -124,7 +125,7 @@ final class PathStatusView: UIView { } @objc private func handleBuildingPathsNotification() { - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } @@ -133,7 +134,7 @@ final class PathStatusView: UIView { } @objc private func handlePathsBuiltNotification() { - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } @@ -147,7 +148,7 @@ final class PathStatusView: UIView { return } - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } diff --git a/Session/Path/PathVC.swift b/Session/Path/PathVC.swift index 1b641f43f..aa6c52fff 100644 --- a/Session/Path/PathVC.swift +++ b/Session/Path/PathVC.swift @@ -241,7 +241,7 @@ private final class LineView: UIView { private var dotViewWidthConstraint: NSLayoutConstraint! private var dotViewHeightConstraint: NSLayoutConstraint! private var dotViewAnimationTimer: Timer! - private let reachability: Reachability = Reachability.forInternetConnection() + private let reachability: Reachability? = Environment.shared?.reachabilityManager.reachability enum Location { case top, middle, bottom @@ -326,10 +326,10 @@ private final class LineView: UIView { } } - switch (reachability.isReachable(), OnionRequestAPI.paths.isEmpty) { - case (false, _): setStatus(to: .error) - case (true, true): setStatus(to: .connecting) - case (true, false): setStatus(to: .connected) + switch (reachability?.isReachable(), OnionRequestAPI.paths.isEmpty) { + case (.some(false), _), (nil, _): setStatus(to: .error) + case (.some(true), true): setStatus(to: .connecting) + case (.some(true), false): setStatus(to: .connected) } } @@ -380,7 +380,7 @@ private final class LineView: UIView { } @objc private func handleBuildingPathsNotification() { - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } @@ -389,7 +389,7 @@ private final class LineView: UIView { } @objc private func handlePathsBuiltNotification() { - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } @@ -403,7 +403,7 @@ private final class LineView: UIView { return } - guard reachability.isReachable() else { + guard reachability?.isReachable() == true else { setStatus(to: .error) return } diff --git a/Session/Shared/Views/SessionCell.swift b/Session/Shared/Views/SessionCell.swift index 5ad87cb99..90407d4c6 100644 --- a/Session/Shared/Views/SessionCell.swift +++ b/Session/Shared/Views/SessionCell.swift @@ -95,7 +95,7 @@ public class SessionCell: UITableViewCell { return result }() - private let titleLabel: SRCopyableLabel = { + fileprivate let titleLabel: SRCopyableLabel = { let result: SRCopyableLabel = SRCopyableLabel() result.translatesAutoresizingMaskIntoConstraints = false result.isUserInteractionEnabled = false @@ -586,7 +586,16 @@ public class SessionCell: UITableViewCell { extension CombineCompatible where Self: SessionCell { var textPublisher: AnyPublisher { - return self.titleTextField.publisher(for: .editingChanged) + return self.titleTextField.publisher(for: [.editingChanged, .editingDidEnd]) + .handleEvents( + receiveOutput: { [weak self] textField in + // When editing the text update the 'accessibilityLabel' of the cell to match + // the text + let targetText: String? = (textField.isEditing ? textField.text : self?.titleLabel.text) + self?.accessibilityLabel = (targetText ?? self?.accessibilityLabel) + } + ) + .filter { $0.isEditing } // Don't bother sending events for 'editingDidEnd' .map { textField -> String in (textField.text ?? "") } .eraseToAnyPublisher() } diff --git a/SessionMessagingKit/Database/Models/ClosedGroup.swift b/SessionMessagingKit/Database/Models/ClosedGroup.swift index e24dff75b..f2e4964ba 100644 --- a/SessionMessagingKit/Database/Models/ClosedGroup.swift +++ b/SessionMessagingKit/Database/Models/ClosedGroup.swift @@ -164,7 +164,7 @@ public extension ClosedGroup { // Remove the remaining group data if desired if removeGroupData { - try SessionThread + try SessionThread // Intentionally use `deleteAll` here as this gets triggered via `deleteOrLeave` .filter(ids: threadIds) .deleteAll(db) diff --git a/SessionMessagingKit/Database/Models/ControlMessageProcessRecord.swift b/SessionMessagingKit/Database/Models/ControlMessageProcessRecord.swift index 3890df232..be48d773a 100644 --- a/SessionMessagingKit/Database/Models/ControlMessageProcessRecord.swift +++ b/SessionMessagingKit/Database/Models/ControlMessageProcessRecord.swift @@ -41,6 +41,15 @@ public struct ControlMessageProcessRecord: Codable, FetchableRecord, Persistable case unsendRequest = 7 case messageRequestResponse = 8 case call = 9 + + /// Since we retrieve messages from all snodes in a swarm there is a fun issue where a user can delete a + /// one-to-one conversation (which removes all associated interactions) and then the poller checks a + /// different service node, if a previously processed message hadn't been processed yet for that specific + /// service node it results in the conversation re-appearing + /// + /// This `Variant` allows us to create a record which survives thread deletion to prevent a duplicate + /// message from being reprocessed + case visibleMessageDedupe = 10 } /// The id for the thread the control message is associated to @@ -68,10 +77,6 @@ public struct ControlMessageProcessRecord: Codable, FetchableRecord, Persistable message: Message, serverExpirationTimestamp: TimeInterval? ) { - // All `VisibleMessage` values will have an associated `Interaction` so just let - // the unique constraints on that table prevent duplicate messages - if message is VisibleMessage { return nil } - // Allow duplicates for UnsendRequest messages, if a user received an UnsendRequest // as a push notification the it wouldn't include a serverHash and, as a result, // wouldn't get deleted from the server - since the logic only runs if we find a @@ -113,6 +118,7 @@ public struct ControlMessageProcessRecord: Codable, FetchableRecord, Persistable case is UnsendRequest: return .unsendRequest case is MessageRequestResponse: return .messageRequestResponse case is CallMessage: return .call + case is VisibleMessage: return .visibleMessageDedupe default: preconditionFailure("[ControlMessageProcessRecord] Unsupported message type") } }() diff --git a/SessionMessagingKit/Database/Models/GroupMember.swift b/SessionMessagingKit/Database/Models/GroupMember.swift index 2ccd56d68..75fb605f0 100644 --- a/SessionMessagingKit/Database/Models/GroupMember.swift +++ b/SessionMessagingKit/Database/Models/GroupMember.swift @@ -4,7 +4,7 @@ import Foundation import GRDB import SessionUtilitiesKit -public struct GroupMember: Codable, Equatable, FetchableRecord, PersistableRecord, TableRecord, ColumnExpressible { +public struct GroupMember: Codable, Equatable, Hashable, FetchableRecord, PersistableRecord, TableRecord, ColumnExpressible { public static var databaseTableName: String { "groupMember" } internal static let openGroupForeignKey = ForeignKey([Columns.groupId], to: [OpenGroup.Columns.threadId]) internal static let closedGroupForeignKey = ForeignKey([Columns.groupId], to: [ClosedGroup.Columns.threadId]) diff --git a/SessionMessagingKit/Database/Models/SessionThread.swift b/SessionMessagingKit/Database/Models/SessionThread.swift index 507e96814..bedd17894 100644 --- a/SessionMessagingKit/Database/Models/SessionThread.swift +++ b/SessionMessagingKit/Database/Models/SessionThread.swift @@ -285,6 +285,89 @@ public extension SessionThread { ) """) } + + static func deleteOrLeave( + _ db: Database, + threadId: String, + threadVariant: Variant, + shouldSendLeaveMessageForGroups: Bool, + calledFromConfigHandling: Bool + ) throws { + try deleteOrLeave( + db, + threadIds: [threadId], + threadVariant: threadVariant, + shouldSendLeaveMessageForGroups: shouldSendLeaveMessageForGroups, + calledFromConfigHandling: calledFromConfigHandling + ) + } + + static func deleteOrLeave( + _ db: Database, + threadIds: [String], + threadVariant: Variant, + shouldSendLeaveMessageForGroups: Bool, + calledFromConfigHandling: Bool + ) throws { + let currentUserPublicKey: String = getUserHexEncodedPublicKey(db) + let remainingThreadIds: [String] = threadIds.filter { $0 != currentUserPublicKey } + + switch threadVariant { + case .contact: + // We need to custom handle the 'Note to Self' conversation (it should just be + // hidden rather than deleted + if threadIds.contains(currentUserPublicKey) { + _ = try Interaction + .filter(Interaction.Columns.threadId == currentUserPublicKey) + .deleteAll(db) + + _ = try SessionThread + .filter(id: currentUserPublicKey) + .updateAllAndConfig( + db, + SessionThread.Columns.pinnedPriority.set(to: 0), + SessionThread.Columns.shouldBeVisible.set(to: false) + ) + return + } + + // If this wasn't called from config handling then we need to hide the conversation + if !calledFromConfigHandling { + try SessionUtil + .hide(db, contactIds: threadIds) + } + + case .legacyGroup, .group: + if shouldSendLeaveMessageForGroups { + threadIds.forEach { threadId in + MessageSender + .leave(db, groupPublicKey: threadId) + .sinkUntilComplete() + } + } + else { + try ClosedGroup.removeKeysAndUnsubscribe( + db, + threadIds: threadIds, + removeGroupData: true, + calledFromConfigHandling: calledFromConfigHandling + ) + } + + case .community: + threadIds.forEach { threadId in + OpenGroupManager.shared.delete( + db, + openGroupId: threadId, + calledFromConfigHandling: calledFromConfigHandling + ) + } + } + + _ = try SessionThread + .filter(ids: remainingThreadIds) + .deleteAll(db) + } } // MARK: - Convenience diff --git a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Contacts.swift b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Contacts.swift index a48c82259..2700d1425 100644 --- a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Contacts.swift +++ b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Contacts.swift @@ -171,8 +171,13 @@ internal extension SessionUtil { SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: [contact.id]) try SessionThread - .filter(id: contact.id) - .deleteAll(db) + .deleteOrLeave( + db, + threadId: contact.id, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: true + ) case (true, false): try SessionThread( @@ -230,8 +235,13 @@ internal extension SessionUtil { // Delete the one-to-one conversations associated to the contact try SessionThread - .filter(ids: contactIdsToRemove) - .deleteAll(db) + .deleteOrLeave( + db, + threadIds: contactIdsToRemove, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: true + ) try SessionUtil.remove(db, volatileContactIds: contactIdsToRemove) } @@ -441,7 +451,13 @@ public extension SessionUtil { // Mark the contacts as hidden try SessionUtil.upsert( contactData: contactIds - .map { SyncedContactInfo(id: $0, hidden: true) }, + .map { + SyncedContactInfo( + id: $0, + hidden: true, + priority: 0 + ) + }, in: conf ) } diff --git a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift index 45854e1b3..41f0497af 100644 --- a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift +++ b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift @@ -199,7 +199,7 @@ internal extension SessionUtil { threadInfo.changes.forEach { change in switch change { case .lastReadTimestampMs(let lastReadMs): - oneToOne.last_read = lastReadMs + oneToOne.last_read = max(oneToOne.last_read, lastReadMs) case .markedAsUnread(let unread): oneToOne.unread = unread @@ -218,7 +218,7 @@ internal extension SessionUtil { threadInfo.changes.forEach { change in switch change { case .lastReadTimestampMs(let lastReadMs): - legacyGroup.last_read = lastReadMs + legacyGroup.last_read = max(legacyGroup.last_read, lastReadMs) case .markedAsUnread(let unread): legacyGroup.unread = unread @@ -246,7 +246,7 @@ internal extension SessionUtil { threadInfo.changes.forEach { change in switch change { case .lastReadTimestampMs(let lastReadMs): - community.last_read = lastReadMs + community.last_read = max(community.last_read, lastReadMs) case .markedAsUnread(let unread): community.unread = unread diff --git a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Shared.swift b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Shared.swift index 5f0bae6ea..c6775b7d1 100644 --- a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Shared.swift +++ b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+Shared.swift @@ -209,24 +209,74 @@ internal extension SessionUtil { // Extract the ones which will respond to SessionUtil changes let targetViewControllers: [any SessionUtilRespondingViewController] = navController .viewControllers - .compactMap({ $0 as? SessionUtilRespondingViewController }) + .compactMap { $0 as? SessionUtilRespondingViewController } + let presentedNavController: UINavigationController? = (navController.presentedViewController as? UINavigationController) + let presentedTargetViewControllers: [any SessionUtilRespondingViewController] = (presentedNavController? + .viewControllers + .compactMap { $0 as? SessionUtilRespondingViewController }) + .defaulting(to: []) // Make sure we have a conversation list and that one of the removed conversations are // in the nav hierarchy - guard - targetViewControllers.count > 1, - targetViewControllers.contains(where: { $0.isConversationList }), + let rootNavControllerNeedsPop: Bool = ( + targetViewControllers.count > 1 && + targetViewControllers.contains(where: { $0.isConversationList }) && targetViewControllers.contains(where: { $0.isConversation(in: removedThreadIds) }) - else { return } + ) + let presentedNavControllerNeedsPop: Bool = ( + presentedTargetViewControllers.count > 1 && + presentedTargetViewControllers.contains(where: { $0.isConversationList }) && + presentedTargetViewControllers.contains(where: { $0.isConversation(in: removedThreadIds) }) + ) - // Return to the root view controller as the removed conversation will be invalid - if navController.presentedViewController != nil { - navController.dismiss(animated: false) { - navController.popToRootViewController(animated: true) - } - } - else { - navController.popToRootViewController(animated: true) + // Force the UI to refresh if needed (most screens should do this automatically via database + // observation, but a couple of screens don't so need to be done manually) + targetViewControllers + .appending(contentsOf: presentedTargetViewControllers) + .filter { $0.isConversationList } + .forEach { $0.forceRefreshIfNeeded() } + + switch (rootNavControllerNeedsPop, presentedNavControllerNeedsPop) { + case (true, false): + // Return to the conversation list as the removed conversation will be invalid + guard + let targetViewController: UIViewController = navController.viewControllers + .last(where: { viewController in + ((viewController as? SessionUtilRespondingViewController)?.isConversationList) + .defaulting(to: false) + }) + else { return } + + if navController.presentedViewController != nil { + navController.dismiss(animated: false) { + navController.popToViewController(targetViewController, animated: true) + } + } + else { + navController.popToViewController(targetViewController, animated: true) + } + + case (false, true): + // Return to the conversation list as the removed conversation will be invalid + guard + let targetViewController: UIViewController = presentedNavController? + .viewControllers + .last(where: { viewController in + ((viewController as? SessionUtilRespondingViewController)?.isConversationList) + .defaulting(to: false) + }) + else { return } + + if presentedNavController?.presentedViewController != nil { + presentedNavController?.dismiss(animated: false) { + presentedNavController?.popToViewController(targetViewController, animated: true) + } + } + else { + presentedNavController?.popToViewController(targetViewController, animated: true) + } + + default: break } } } @@ -256,7 +306,12 @@ public extension SessionUtil { var cThreadId: [CChar] = threadId.cArray switch threadVariant { - case .contact: return contacts_get(conf, nil, &cThreadId) + case .contact: + var contact: contacts_contact = contacts_contact() + + guard contacts_get(conf, &contact, &cThreadId) else { return false } + + return !contact.hidden case .community: let maybeUrlInfo: OpenGroupUrlInfo? = Storage.shared @@ -267,8 +322,9 @@ public extension SessionUtil { var cBaseUrl: [CChar] = urlInfo.server.cArray var cRoom: [CChar] = urlInfo.roomToken.cArray + var community: ugroups_community_info = ugroups_community_info() - return user_groups_get_community(conf, nil, &cBaseUrl, &cRoom) + return user_groups_get_community(conf, &community, &cBaseUrl, &cRoom) case .legacyGroup: let groupInfo: UnsafeMutablePointer? = user_groups_get_legacy_group(conf, &cThreadId) @@ -331,10 +387,12 @@ public protocol SessionUtilRespondingViewController { var isConversationList: Bool { get } func isConversation(in threadIds: [String]) -> Bool + func forceRefreshIfNeeded() } public extension SessionUtilRespondingViewController { var isConversationList: Bool { false } func isConversation(in threadIds: [String]) -> Bool { return false } + func forceRefreshIfNeeded() {} } diff --git a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserGroups.swift b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserGroups.swift index bda053c0e..d62de5de6 100644 --- a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserGroups.swift +++ b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserGroups.swift @@ -88,7 +88,7 @@ internal extension SessionUtil { ), groupMembers: members .filter { _, isAdmin in !isAdmin } - .map { memberId, admin in + .map { memberId, _ in GroupMember( groupId: groupId, profileId: memberId, @@ -98,7 +98,7 @@ internal extension SessionUtil { }, groupAdmins: members .filter { _, isAdmin in isAdmin } - .map { memberId, admin in + .map { memberId, _ in GroupMember( groupId: groupId, profileId: memberId, @@ -171,13 +171,14 @@ internal extension SessionUtil { if !communityIdsToRemove.isEmpty { SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: Array(communityIdsToRemove)) - communityIdsToRemove.forEach { threadId in - OpenGroupManager.shared.delete( + try SessionThread + .deleteOrLeave( db, - openGroupId: threadId, + threadIds: Array(communityIdsToRemove), + threadVariant: .community, + shouldSendLeaveMessageForGroups: false, calledFromConfigHandling: true ) - } } // MARK: -- Handle Legacy Group Changes @@ -200,7 +201,7 @@ internal extension SessionUtil { let name: String = group.name, let lastKeyPair: ClosedGroupKeyPair = group.lastKeyPair, let members: [GroupMember] = group.groupMembers, - let updatedAdmins: [GroupMember] = group.groupAdmins + let updatedAdmins: Set = group.groupAdmins?.asSet() else { return } if !existingLegacyGroupIds.contains(group.id) { @@ -214,7 +215,8 @@ internal extension SessionUtil { secretKey: lastKeyPair.secretKey.bytes ), members: members - .appending(contentsOf: updatedAdmins) // Admins should also have 'standard' member entries + .asSet() + .inserting(contentsOf: updatedAdmins) // Admins should also have 'standard' member entries .map { $0.profileId }, admins: updatedAdmins.map { $0.profileId }, expirationTimer: UInt32(group.disappearingConfig?.durationSeconds ?? 0), @@ -253,7 +255,7 @@ internal extension SessionUtil { .saved(db) // Update the members - let updatedMembers: [GroupMember] = members + let updatedMembers: Set = members .appending( contentsOf: updatedAdmins.map { admin in GroupMember( @@ -264,10 +266,12 @@ internal extension SessionUtil { ) } ) + .asSet() if - let existingMembers: [GroupMember] = existingLegacyGroupMembers[group.id]? - .filter({ $0.role == .standard || $0.role == .zombie }), + let existingMembers: Set = existingLegacyGroupMembers[group.id]? + .filter({ $0.role == .standard || $0.role == .zombie }) + .asSet(), existingMembers != updatedMembers { // Add in any new members and remove any removed members @@ -288,8 +292,9 @@ internal extension SessionUtil { } if - let existingAdmins: [GroupMember] = existingLegacyGroupMembers[group.id]? - .filter({ $0.role == .admin }), + let existingAdmins: Set = existingLegacyGroupMembers[group.id]? + .filter({ $0.role == .admin }) + .asSet(), existingAdmins != updatedAdmins { // Add in any new admins and remove any removed admins @@ -326,12 +331,14 @@ internal extension SessionUtil { if !legacyGroupIdsToRemove.isEmpty { SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: Array(legacyGroupIdsToRemove)) - try ClosedGroup.removeKeysAndUnsubscribe( - db, - threadIds: Array(legacyGroupIdsToRemove), - removeGroupData: true, - calledFromConfigHandling: true - ) + try SessionThread + .deleteOrLeave( + db, + threadIds: Array(legacyGroupIdsToRemove), + threadVariant: .legacyGroup, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: true + ) } // MARK: -- Handle Group Changes @@ -364,8 +371,6 @@ internal extension SessionUtil { guard conf != nil else { throw SessionUtilError.nilConfigObject } guard !legacyGroups.isEmpty else { return } - // Since we are doing direct memory manipulation we are using an `Atomic` type which has - // blocking access in it's `mutate` closure legacyGroups .forEach { legacyGroup in var cGroupId: [CChar] = legacyGroup.id.cArray @@ -405,7 +410,12 @@ internal extension SessionUtil { }() if let groupMembers: [GroupMember] = legacyGroup.groupMembers { - let memberIds: Set = groupMembers.map { $0.profileId }.asSet() + // Need to make sure we remove any admins before adding them here otherwise we will + // overwrite the admin permission to be a standard user permission + let memberIds: Set = groupMembers + .map { $0.profileId } + .asSet() + .subtracting(legacyGroup.groupAdmins.defaulting(to: []).map { $0.profileId }.asSet()) let existingMemberIds: Set = Array(existingMembers .filter { _, isAdmin in !isAdmin } .keys) @@ -555,6 +565,19 @@ public extension SessionUtil { for: .userGroups, publicKey: getUserHexEncodedPublicKey(db) ) { conf in + guard conf != nil else { throw SessionUtilError.nilConfigObject } + + var cGroupId: [CChar] = groupPublicKey.cArray + let userGroup: UnsafeMutablePointer? = user_groups_get_legacy_group(conf, &cGroupId) + + // Need to make sure the group doesn't already exist (otherwise we will end up overriding the + // content which could revert newer changes since this can be triggered from other 'NEW' messages + // coming in from the legacy group swarm) + guard userGroup == nil else { + ugroups_legacy_group_free(userGroup) + return + } + try SessionUtil.upsert( legacyGroups: [ LegacyGroupInfo( diff --git a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserProfile.swift b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserProfile.swift index 938fb9a5b..9576b9509 100644 --- a/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserProfile.swift +++ b/SessionMessagingKit/LibSessionUtil/Config Handling/SessionUtil+UserProfile.swift @@ -98,6 +98,20 @@ internal extension SessionUtil { db, SessionThread.Columns.pinnedPriority.set(to: targetPriority) ) + + // If the 'Note to Self' conversation is hidden then we should trigger the proper + // `deleteOrLeave` behaviour (for 'Note to Self' this will leave the conversation + // but remove the associated interactions) + if targetHiddenState { + try SessionThread + .deleteOrLeave( + db, + threadId: userPublicKey, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: true + ) + } } // Create a contact for the current user if needed (also force-approve the current user diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift index 7cf96a268..7cd9330af 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift @@ -93,8 +93,13 @@ extension MessageReceiver { .updateAll(db, Interaction.Columns.threadId.set(to: unblindedThread.id)) _ = try SessionThread - .filter(id: blindedIdLookup.blindedId) - .deleteAll(db) + .deleteOrLeave( + db, + threadId: blindedIdLookup.blindedId, + threadVariant: .contact, + shouldSendLeaveMessageForGroups: false, + calledFromConfigHandling: false + ) } // Update the `didApproveMe` state of the sender diff --git a/SessionMessagingKit/Utilities/SSKReachabilityManager.swift b/SessionMessagingKit/Utilities/SSKReachabilityManager.swift index 9f6bea814..a68fa58ba 100644 --- a/SessionMessagingKit/Utilities/SSKReachabilityManager.swift +++ b/SessionMessagingKit/Utilities/SSKReachabilityManager.swift @@ -7,6 +7,8 @@ public enum ReachabilityType: Int { @objc public protocol SSKReachabilityManager { + var reachability: Reachability { get } + var observationContext: AnyObject { get } func setup()