From 29ba25916cc3fcf675abb103e29e6b275d739304 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 5 Jan 2023 13:40:33 +1100 Subject: [PATCH] Fixed a few issues uncovered when testing the shared util feature flag Added code to handle an edge-case where an account could exist without a display name if crashing at the right time when onboarding/linking (now request a display name on start up) Fixed a crash which could occur when downloading new avatars Fixed a minor layout issue with the seed reminder view --- .../ConversationVC+Interaction.swift | 3 +-- Session/Meta/AppDelegate.swift | 15 +++++++++---- Session/Onboarding/Onboarding.swift | 9 ++++++++ Session/Onboarding/SeedReminderView.swift | 3 ++- Session/Open Groups/JoinOpenGroupVC.swift | 3 +-- .../Jobs/Types/ConfigurationSyncJob.swift | 8 ++++--- .../QueryInterfaceRequest+Utilities.swift | 6 ++++++ .../LibSessionUtil/SessionUtil.swift | 2 ++ .../Open Groups/OpenGroupAPI.swift | 2 -- .../Open Groups/OpenGroupManager.swift | 8 ++++--- ...essageReceiver+ConfigurationMessages.swift | 11 ++++++++-- .../Utilities/ProfileManager.swift | 21 ++++++++++++++++++- .../Open Groups/OpenGroupManagerSpec.swift | 4 ---- 13 files changed, 71 insertions(+), 24 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 446ef1c8d..a04745d20 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -1554,8 +1554,7 @@ extension ConversationVC: db, roomToken: room, server: server, - publicKey: publicKey, - isConfigMessage: false + publicKey: publicKey ) } .receive(on: DispatchQueue.main) diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index aa186b1a9..23e9894bb 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -405,10 +405,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD self.hasInitialRootViewController = true self.window?.rootViewController = StyledNavigationController( - rootViewController: (Identity.userExists() ? - HomeVC() : - LandingVC() - ) + rootViewController: { + guard Identity.userExists() else { return LandingVC() } + guard !Profile.fetchOrCreateCurrentUser().name.isEmpty else { + // If we have no display name then collect one (this can happen if the + // app crashed during onboarding which would leave the user in an invalid + // state with no display name) + return DisplayNameVC(flow: .register) + } + + return HomeVC() + }() ) UIViewController.attemptRotationToDeviceOrientation() diff --git a/Session/Onboarding/Onboarding.swift b/Session/Onboarding/Onboarding.swift index 0067fb033..948893aeb 100644 --- a/Session/Onboarding/Onboarding.swift +++ b/Session/Onboarding/Onboarding.swift @@ -9,6 +9,15 @@ import SessionMessagingKit enum Onboarding { private static let profileNameRetrievalPublisher: Atomic> = { + // FIXME: Remove this once `useSharedUtilForUserConfig` is permanent + guard Features.useSharedUtilForUserConfig else { + return Atomic( + Just(nil) + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + ) + } + let userPublicKey: String = getUserHexEncodedPublicKey() return Atomic( diff --git a/Session/Onboarding/SeedReminderView.swift b/Session/Onboarding/SeedReminderView.swift index 83b0034c0..5570cc578 100644 --- a/Session/Onboarding/SeedReminderView.swift +++ b/Session/Onboarding/SeedReminderView.swift @@ -82,10 +82,11 @@ final class SeedReminderView: UIView { // Set up button let button = SessionButton(style: .bordered, size: .small) + button.setContentCompressionResistancePriority(.required, for: .horizontal) button.accessibilityLabel = "Continue" button.isAccessibilityElement = true button.setTitle("continue_2".localized(), for: UIControl.State.normal) - button.set(.width, to: 96) + button.set(.width, greaterThanOrEqualTo: 96) button.addTarget(self, action: #selector(handleContinueButtonTapped), for: UIControl.Event.touchUpInside) // Set up content stack view diff --git a/Session/Open Groups/JoinOpenGroupVC.swift b/Session/Open Groups/JoinOpenGroupVC.swift index 648363152..f32320f16 100644 --- a/Session/Open Groups/JoinOpenGroupVC.swift +++ b/Session/Open Groups/JoinOpenGroupVC.swift @@ -174,8 +174,7 @@ final class JoinOpenGroupVC: BaseVC, UIPageViewControllerDataSource, UIPageViewC db, roomToken: roomToken, server: server, - publicKey: publicKey, - isConfigMessage: false + publicKey: publicKey ) } .receive(on: DispatchQueue.main) diff --git a/SessionMessagingKit/Jobs/Types/ConfigurationSyncJob.swift b/SessionMessagingKit/Jobs/Types/ConfigurationSyncJob.swift index 2b734fffc..563c34db2 100644 --- a/SessionMessagingKit/Jobs/Types/ConfigurationSyncJob.swift +++ b/SessionMessagingKit/Jobs/Types/ConfigurationSyncJob.swift @@ -265,11 +265,13 @@ public extension ConfigurationSyncJob { // FIXME: Remove this once `useSharedUtilForUserConfig` is permanent guard Features.useSharedUtilForUserConfig else { - // If we don't have a userKeyPair yet then there is no need to sync the configuration - // as the user doesn't exist yet (this will get triggered on the first launch of a - // fresh install due to the migrations getting run) + // If we don't have a userKeyPair (or name) yet then there is no need to sync the + // configuration as the user doesn't fully exist yet (this will get triggered on + // the first launch of a fresh install due to the migrations getting run and a few + // times during onboarding) guard Identity.userExists(db), + !Profile.fetchOrCreateCurrentUser(db).name.isEmpty, let legacyConfigMessage: Message = try? ConfigurationMessage.getCurrent(db) else { return } diff --git a/SessionMessagingKit/LibSessionUtil/Database/QueryInterfaceRequest+Utilities.swift b/SessionMessagingKit/LibSessionUtil/Database/QueryInterfaceRequest+Utilities.swift index 2caf9c03c..e186e0027 100644 --- a/SessionMessagingKit/LibSessionUtil/Database/QueryInterfaceRequest+Utilities.swift +++ b/SessionMessagingKit/LibSessionUtil/Database/QueryInterfaceRequest+Utilities.swift @@ -60,6 +60,12 @@ public extension QueryInterfaceRequest where RowDecoder: FetchableRecord & Table } } + // FIXME: Remove this once `useSharedUtilForUserConfig` is permanent + guard Features.useSharedUtilForUserConfig else { + return try self.updateAndFetchAll(db, assignments) + } + + // Update the config dump state where needed switch self { case is QueryInterfaceRequest: return try SessionUtil.updatingContacts(db, try updateAndFetchAll(db, assignments)) diff --git a/SessionMessagingKit/LibSessionUtil/SessionUtil.swift b/SessionMessagingKit/LibSessionUtil/SessionUtil.swift index d995a571f..6d95d9a5a 100644 --- a/SessionMessagingKit/LibSessionUtil/SessionUtil.swift +++ b/SessionMessagingKit/LibSessionUtil/SessionUtil.swift @@ -280,6 +280,8 @@ public enum SessionUtil { messages: [SharedConfigMessage], publicKey: String ) throws { + // FIXME: Remove this once `useSharedUtilForUserConfig` is permanent + guard Features.useSharedUtilForUserConfig else { return } guard !messages.isEmpty else { return } guard !publicKey.isEmpty else { throw MessageReceiverError.noThread } diff --git a/SessionMessagingKit/Open Groups/OpenGroupAPI.swift b/SessionMessagingKit/Open Groups/OpenGroupAPI.swift index ae249bfb3..46a96f7c8 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupAPI.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupAPI.swift @@ -946,7 +946,6 @@ public enum OpenGroupAPI { queue: OpenGroupAPI.workQueue ) ) -> AnyPublisher<(ResponseInfoType, Data), Error> { - print("Download File") return OpenGroupAPI .send( db, @@ -957,7 +956,6 @@ public enum OpenGroupAPI { using: dependencies ) .flatMap { responseInfo, maybeData -> AnyPublisher<(ResponseInfoType, Data), Error> in - print("Download File FlatMap") guard let data: Data = maybeData else { return Fail(error: HTTPError.parsingFailed) .eraseToAnyPublisher() diff --git a/SessionMessagingKit/Open Groups/OpenGroupManager.swift b/SessionMessagingKit/Open Groups/OpenGroupManager.swift index 200cc60ea..d8d87e567 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupManager.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupManager.swift @@ -207,12 +207,12 @@ public final class OpenGroupManager { roomToken: String, server: String, publicKey: String, - isConfigMessage: Bool, + calledFromConfigHandling: Bool = false, dependencies: OGMDependencies = OGMDependencies() ) -> AnyPublisher { // If we are currently polling for this server and already have a TSGroupThread for this room the do nothing if hasExistingOpenGroup(db, roomToken: roomToken, server: server, publicKey: publicKey, dependencies: dependencies) { - SNLog("Ignoring join open group attempt (already joined), user initiated: \(!isConfigMessage)") + SNLog("Ignoring join open group attempt (already joined), user initiated: \(!calledFromConfigHandling)") return Just(()) .setFailureType(to: Error.self) .eraseToAnyPublisher() @@ -276,7 +276,9 @@ public final class OpenGroupManager { Future { resolver in dependencies.storage.write { db in // Enqueue a config sync job (have a newly added open group to sync) - ConfigurationSyncJob.enqueue(db) + if !calledFromConfigHandling { + ConfigurationSyncJob.enqueue(db) + } // Store the capabilities first OpenGroupManager.handleCapabilities( diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ConfigurationMessages.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ConfigurationMessages.swift index 2e4203966..c3304e9ea 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ConfigurationMessages.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ConfigurationMessages.swift @@ -43,7 +43,8 @@ extension MessageReceiver { fileName: nil ) }(), - sentTimestamp: messageSentTimestamp + sentTimestamp: messageSentTimestamp, + calledFromConfigHandling: true ) // Create a contact for the current user if needed (also force-approve the current user @@ -197,7 +198,13 @@ extension MessageReceiver { for openGroupURL in message.openGroups { if let (room, server, publicKey) = OpenGroupManager.parseOpenGroup(from: openGroupURL) { OpenGroupManager.shared - .add(db, roomToken: room, server: server, publicKey: publicKey, isConfigMessage: true) + .add( + db, + roomToken: room, + server: server, + publicKey: publicKey, + calledFromConfigHandling: true + ) .sinkUntilComplete() } } diff --git a/SessionMessagingKit/Utilities/ProfileManager.swift b/SessionMessagingKit/Utilities/ProfileManager.swift index 9cf23b523..458ee0209 100644 --- a/SessionMessagingKit/Utilities/ProfileManager.swift +++ b/SessionMessagingKit/Utilities/ProfileManager.swift @@ -594,6 +594,25 @@ public struct ProfileManager { profileChanges ) } + // FIXME: Remove this once `useSharedUtilForUserConfig` is permanent + else if !Features.useSharedUtilForUserConfig { + // If we have a contact record for the profile (ie. it's a synced profile) then + // should should send an updated config message, otherwise we should just update + // the local state (the shared util has this logic build in to it's handling) + if (try? Contact.exists(db, id: publicKey)) == true { + try Profile + .filter(id: publicKey) + .updateAllAndConfig(db, profileChanges) + } + else { + try Profile + .filter(id: publicKey) + .updateAll( + db, + profileChanges + ) + } + } else { try Profile .filter(id: publicKey) @@ -606,7 +625,7 @@ public struct ProfileManager { db.afterNextTransaction { db in // Need to refetch to ensure the db changes have occurred - ProfileManager.downloadAvatar(for: Profile.fetchOrCreate(id: publicKey)) + ProfileManager.downloadAvatar(for: Profile.fetchOrCreate(db, id: publicKey)) } } } diff --git a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift index 26d5cc290..65e1a5a71 100644 --- a/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift +++ b/SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift @@ -822,7 +822,6 @@ class OpenGroupManagerSpec: QuickSpec { roomToken: "testRoom", server: "testServer", publicKey: TestConstants.serverPublicKey, - isConfigMessage: false, dependencies: dependencies ) } @@ -853,7 +852,6 @@ class OpenGroupManagerSpec: QuickSpec { roomToken: "testRoom", server: "testServer", publicKey: TestConstants.serverPublicKey, - isConfigMessage: false, dependencies: dependencies ) } @@ -892,7 +890,6 @@ class OpenGroupManagerSpec: QuickSpec { publicKey: TestConstants.serverPublicKey .replacingOccurrences(of: "c3", with: "00") .replacingOccurrences(of: "b3", with: "00"), - isConfigMessage: false, dependencies: dependencies ) } @@ -946,7 +943,6 @@ class OpenGroupManagerSpec: QuickSpec { roomToken: "testRoom", server: "testServer", publicKey: TestConstants.serverPublicKey, - isConfigMessage: false, dependencies: dependencies ) }