From 7a8941db5cdc68d3c73e6ba8cdffd5b8f2f66cbc Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 1 Sep 2023 16:16:13 +1000 Subject: [PATCH] Fixed a couple of config handling bugs Fixed an bug where config messages could be processed in the wrong order Tweaked the behaviour or removing threads (this would cause issues with future config-based settings changes that live on the thread getting lost) --- Session/Conversations/ConversationVC.swift | 2 +- .../UIContextualAction+Utilities.swift | 2 +- .../Database/Models/SessionThread.swift | 32 ++++++--- .../SessionUtil+Contacts.swift | 71 ++++++++----------- .../SessionUtil+UserProfile.swift | 2 +- .../QueryInterfaceRequest+Utilities.swift | 15 ++-- .../SessionUtil/SessionUtil.swift | 1 + 7 files changed, 68 insertions(+), 57 deletions(-) diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index 523cdd884..f5f1ded92 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -574,7 +574,7 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers !SessionUtil.conversationInConfig( threadId: threadId, threadVariant: viewModel.threadData.threadVariant, - visibleOnly: true + visibleOnly: false ) { Storage.shared.writeAsync { db in diff --git a/Session/Utilities/UIContextualAction+Utilities.swift b/Session/Utilities/UIContextualAction+Utilities.swift index 85997e42a..3f26aa57c 100644 --- a/Session/Utilities/UIContextualAction+Utilities.swift +++ b/Session/Utilities/UIContextualAction+Utilities.swift @@ -164,7 +164,7 @@ public extension UIContextualAction { db, threadId: threadViewModel.threadId, threadVariant: threadViewModel.threadVariant, - groupLeaveType: .forced, + groupLeaveType: .silent, calledFromConfigHandling: false ) } diff --git a/SessionMessagingKit/Database/Models/SessionThread.swift b/SessionMessagingKit/Database/Models/SessionThread.swift index cb8f84fc3..83e996d33 100644 --- a/SessionMessagingKit/Database/Models/SessionThread.swift +++ b/SessionMessagingKit/Database/Models/SessionThread.swift @@ -296,17 +296,18 @@ public extension SessionThread { calledFromConfigHandling: Bool ) throws { let currentUserPublicKey: String = getUserHexEncodedPublicKey(db) - let remainingThreadIds: [String] = threadIds.filter { $0 != currentUserPublicKey } + let remainingThreadIds: Set = threadIds.asSet().removing(currentUserPublicKey) switch (threadVariant, groupLeaveType) { - case (.contact, _): + case (.contact, .standard), (.contact, .silent): + // Clear any interactions for the deleted thread + _ = try Interaction + .filter(threadIds.contains(Interaction.Columns.threadId)) + .deleteAll(db) + // We need to custom handle the 'Note to Self' conversation (it should just be - // hidden rather than deleted + // hidden locally rather than deleted) if threadIds.contains(currentUserPublicKey) { - _ = try Interaction - .filter(Interaction.Columns.threadId == currentUserPublicKey) - .deleteAll(db) - _ = try SessionThread .filter(id: currentUserPublicKey) .updateAllAndConfig( @@ -314,17 +315,28 @@ public extension SessionThread { SessionThread.Columns.pinnedPriority.set(to: 0), SessionThread.Columns.shouldBeVisible.set(to: false) ) - return } + // Update any other threads to be hidden (don't want to actually delete the thread + // record in case it's settings get changed while it's not visible) + _ = try SessionThread + .filter(id: remainingThreadIds) + .updateAllAndConfig( + db, + calledFromConfig: calledFromConfig, + SessionThread.Columns.pinnedPriority.set(to: SessionUtil.hiddenPriority), + SessionThread.Columns.shouldBeVisible.set(to: false) + ) + + case (.contact, .forced): // If this wasn't called from config handling then we need to hide the conversation if !calledFromConfigHandling { try SessionUtil - .hide(db, contactIds: threadIds) + .remove(db, contactIds: remainingThreadIds) } _ = try SessionThread - .filter(ids: remainingThreadIds) + .filter(id: remainingThreadIds) .deleteAll(db) case (.legacyGroup, .standard), (.group, .standard): diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift index b6a8b86f0..745373060 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift @@ -138,46 +138,37 @@ internal extension SessionUtil { let threadExists: Bool = (threadInfo != nil) let updatedShouldBeVisible: Bool = SessionUtil.shouldBeVisible(priority: data.priority) - switch (updatedShouldBeVisible, threadExists) { - case (false, true): - SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: [sessionId]) - - try SessionThread - .deleteOrLeave( - db, - threadId: sessionId, - threadVariant: .contact, - groupLeaveType: .forced, - calledFromConfigHandling: true - ) - - case (true, false): - try SessionThread( - id: sessionId, - variant: .contact, - creationDateTimestamp: data.created, - shouldBeVisible: true, - pinnedPriority: data.priority - ).save(db) - - case (true, true): - let changes: [ConfigColumnAssignment] = [ - (threadInfo?.shouldBeVisible == updatedShouldBeVisible ? nil : - SessionThread.Columns.shouldBeVisible.set(to: updatedShouldBeVisible) - ), - (threadInfo?.pinnedPriority == data.priority ? nil : - SessionThread.Columns.pinnedPriority.set(to: data.priority) - ) - ].compactMap { $0 } - - try SessionThread - .filter(id: sessionId) - .updateAll( // Handling a config update so don't use `updateAllAndConfig` - db, - changes - ) - - case (false, false): break + /// If we are hiding the conversation then kick the user from it if it's currently open + if !updatedShouldBeVisible { + SessionUtil.kickFromConversationUIIfNeeded(removedThreadIds: [sessionId]) + } + + /// Create the thread if it doesn't exist, otherwise just update it's state + if !threadExists { + try SessionThread( + id: sessionId, + variant: .contact, + creationDateTimestamp: data.created, + shouldBeVisible: updatedShouldBeVisible, + pinnedPriority: data.priority + ).save(db) + } + else { + let changes: [ConfigColumnAssignment] = [ + (threadInfo?.shouldBeVisible == updatedShouldBeVisible ? nil : + SessionThread.Columns.shouldBeVisible.set(to: updatedShouldBeVisible) + ), + (threadInfo?.pinnedPriority == data.priority ? nil : + SessionThread.Columns.pinnedPriority.set(to: data.priority) + ) + ].compactMap { $0 } + + try SessionThread + .filter(id: sessionId) + .updateAll( // Handling a config update so don't use `updateAllAndConfig` + db, + changes + ) } } diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserProfile.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserProfile.swift index 4416eee82..f4017cec0 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserProfile.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserProfile.swift @@ -113,7 +113,7 @@ internal extension SessionUtil { db, threadId: userPublicKey, threadVariant: .contact, - groupLeaveType: .forced, + groupLeaveType: .silent, calledFromConfigHandling: true ) } diff --git a/SessionMessagingKit/SessionUtil/Database/QueryInterfaceRequest+Utilities.swift b/SessionMessagingKit/SessionUtil/Database/QueryInterfaceRequest+Utilities.swift index a7285604e..8e3482a31 100644 --- a/SessionMessagingKit/SessionUtil/Database/QueryInterfaceRequest+Utilities.swift +++ b/SessionMessagingKit/SessionUtil/Database/QueryInterfaceRequest+Utilities.swift @@ -52,14 +52,16 @@ public extension QueryInterfaceRequest where RowDecoder: FetchableRecord & Table @discardableResult func updateAllAndConfig( _ db: Database, + calledFromConfig: Bool = false, _ assignments: ConfigColumnAssignment... ) throws -> Int { - return try updateAllAndConfig(db, assignments) + return try updateAllAndConfig(db, calledFromConfig: calledFromConfig, assignments) } @discardableResult func updateAllAndConfig( _ db: Database, + calledFromConfig: Bool = false, _ assignments: [ConfigColumnAssignment] ) throws -> Int { let targetAssignments: [ColumnAssignment] = assignments.map { $0.assignment } @@ -69,7 +71,7 @@ public extension QueryInterfaceRequest where RowDecoder: FetchableRecord & Table return try self.updateAll(db, targetAssignments) } - return try self.updateAndFetchAllAndUpdateConfig(db, assignments).count + return try self.updateAndFetchAllAndUpdateConfig(db, calledFromConfig: calledFromConfig, assignments).count } // MARK: -- updateAndFetchAll @@ -77,21 +79,26 @@ public extension QueryInterfaceRequest where RowDecoder: FetchableRecord & Table @discardableResult func updateAndFetchAllAndUpdateConfig( _ db: Database, + calledFromConfig: Bool = false, _ assignments: ConfigColumnAssignment... ) throws -> [RowDecoder] { - return try updateAndFetchAllAndUpdateConfig(db, assignments) + return try updateAndFetchAllAndUpdateConfig(db, calledFromConfig: calledFromConfig, assignments) } @discardableResult func updateAndFetchAllAndUpdateConfig( _ db: Database, + calledFromConfig: Bool = false, _ assignments: [ConfigColumnAssignment] ) throws -> [RowDecoder] { // First perform the actual updates let updatedData: [RowDecoder] = try self.updateAndFetchAll(db, assignments.map { $0.assignment }) // Then check if any of the changes could affect the config - guard SessionUtil.assignmentsRequireConfigUpdate(assignments) else { return updatedData } + guard + !calledFromConfig && + SessionUtil.assignmentsRequireConfigUpdate(assignments) + else { return updatedData } defer { // If we changed a column that requires a config update then we may as well automatically diff --git a/SessionMessagingKit/SessionUtil/SessionUtil.swift b/SessionMessagingKit/SessionUtil/SessionUtil.swift index a353e8ed1..363951df4 100644 --- a/SessionMessagingKit/SessionUtil/SessionUtil.swift +++ b/SessionMessagingKit/SessionUtil/SessionUtil.swift @@ -360,6 +360,7 @@ public enum SessionUtil { guard !publicKey.isEmpty else { throw MessageReceiverError.noThread } let groupedMessages: [ConfigDump.Variant: [SharedConfigMessage]] = messages + .sorted { lhs, rhs in lhs.seqNo < rhs.seqNo } .grouped(by: \.kind.configDumpVariant) let needsPush: Bool = try groupedMessages