From e1aedb36daed6d7c6b0364db7288aaf86b2dcd6b Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 2 Sep 2024 18:03:16 +1000 Subject: [PATCH] Fixed a few bugs found when testing strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fixed an issue where creating a legacy group could be blocked by the legacy PN subscription failing (was part of the synchronous request) • Fixed an issue where the code would incorrectly use profile data from incoming messages sent from the current user to update it's profile info • Fixed an issue where saving media would fail silently if the user had rejected the OS permission • Refactored a little code around profile changes to make things more readable --- Session.xcodeproj/project.pbxproj | 8 +- .../ConversationVC+Interaction.swift | 53 ++++--- .../SendMediaNavigationController.swift | 2 +- Session/Onboarding/DisplayNameScreen.swift | 2 +- Session/Settings/SettingsViewModel.swift | 40 ++--- Session/Utilities/Permissions.swift | 6 +- .../Jobs/Types/UpdateProfilePictureJob.swift | 3 +- .../LibSession+UserProfile.swift | 8 +- .../MessageReceiver+ClosedGroups.swift | 11 +- .../MessageReceiver+MessageRequests.swift | 6 +- .../MessageReceiver+VisibleMessages.swift | 10 +- .../MessageSender+ClosedGroups.swift | 65 +++++--- .../Models/LegacyPushServerResponse.swift | 2 +- .../Notifications/PushNotificationAPI.swift | 120 +++++++-------- .../Utilities/ProfileManager.swift | 143 ++++++++++-------- .../Utilities/ProfileManagerError.swift | 4 +- 16 files changed, 265 insertions(+), 218 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index 09b821a47..bce67090d 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -7673,7 +7673,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 478; + CURRENT_PROJECT_VERSION = 482; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -7710,7 +7710,7 @@ GCC_WARN_UNUSED_VARIABLE = YES; HEADER_SEARCH_PATHS = ""; IPHONEOS_DEPLOYMENT_TARGET = 13.0; - MARKETING_VERSION = 2.7.2; + MARKETING_VERSION = 2.7.3; ONLY_ACTIVE_ARCH = YES; OTHER_CFLAGS = ( "-fobjc-arc-exceptions", @@ -7751,7 +7751,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Distribution"; - CURRENT_PROJECT_VERSION = 478; + CURRENT_PROJECT_VERSION = 482; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; GCC_NO_COMMON_BLOCKS = YES; @@ -7783,7 +7783,7 @@ GCC_WARN_UNUSED_VARIABLE = YES; HEADER_SEARCH_PATHS = ""; IPHONEOS_DEPLOYMENT_TARGET = 13.0; - MARKETING_VERSION = 2.7.2; + MARKETING_VERSION = 2.7.3; ONLY_ACTIVE_ARCH = NO; OTHER_CFLAGS = ( "-DNS_BLOCK_ASSERTIONS=1", diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 521f6ea57..51cb1bc6d 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -298,7 +298,7 @@ extension ConversationVC: let threadId: String = self.viewModel.threadData.threadId let threadVariant: SessionThread.Variant = self.viewModel.threadData.threadVariant - Permissions.requestLibraryPermissionIfNeeded { [weak self, dependencies = viewModel.dependencies] in + Permissions.requestLibraryPermissionIfNeeded(isSavingMedia: false) { [weak self, dependencies = viewModel.dependencies] in DispatchQueue.main.async { let sendMediaNavController = SendMediaNavigationController.showingMediaLibraryFirst( threadId: threadId, @@ -2326,30 +2326,35 @@ extension ConversationVC: guard !mediaAttachments.isEmpty else { return } - mediaAttachments.forEach { attachment, originalFilePath in - PHPhotoLibrary.shared().performChanges( - { - if attachment.isImage || attachment.isAnimated { - PHAssetChangeRequest.creationRequestForAssetFromImage( - atFileURL: URL(fileURLWithPath: originalFilePath) - ) - } - else if attachment.isVideo { - PHAssetChangeRequest.creationRequestForAssetFromVideo( - atFileURL: URL(fileURLWithPath: originalFilePath) - ) - } - }, - completionHandler: { _, _ in } - ) - } - - // Send a 'media saved' notification if needed - guard self.viewModel.threadData.threadVariant == .contact, cellViewModel.variant == .standardIncoming else { - return + Permissions.requestLibraryPermissionIfNeeded( + isSavingMedia: true, + presentingViewController: self + ) { [weak self] in + mediaAttachments.forEach { attachment, originalFilePath in + PHPhotoLibrary.shared().performChanges( + { + if attachment.isImage || attachment.isAnimated { + PHAssetChangeRequest.creationRequestForAssetFromImage( + atFileURL: URL(fileURLWithPath: originalFilePath) + ) + } + else if attachment.isVideo { + PHAssetChangeRequest.creationRequestForAssetFromVideo( + atFileURL: URL(fileURLWithPath: originalFilePath) + ) + } + }, + completionHandler: { _, _ in } + ) + } + + // Send a 'media saved' notification if needed + guard self?.viewModel.threadData.threadVariant == .contact, cellViewModel.variant == .standardIncoming else { + return + } + + self?.sendDataExtraction(kind: .mediaSaved(timestamp: UInt64(cellViewModel.timestampMs))) } - - sendDataExtraction(kind: .mediaSaved(timestamp: UInt64(cellViewModel.timestampMs))) } func ban(_ cellViewModel: MessageViewModel, using dependencies: Dependencies) { diff --git a/Session/Media Viewing & Editing/SendMediaNavigationController.swift b/Session/Media Viewing & Editing/SendMediaNavigationController.swift index b83b23ca4..897c79148 100644 --- a/Session/Media Viewing & Editing/SendMediaNavigationController.swift +++ b/Session/Media Viewing & Editing/SendMediaNavigationController.swift @@ -154,7 +154,7 @@ class SendMediaNavigationController: UINavigationController { } private func didTapMediaLibraryModeButton() { - Permissions.requestLibraryPermissionIfNeeded { [weak self] in + Permissions.requestLibraryPermissionIfNeeded(isSavingMedia: false) { [weak self] in DispatchQueue.main.async { self?.fadeTo(viewControllers: ((self?.mediaLibraryViewController).map { [$0] } ?? [])) } diff --git a/Session/Onboarding/DisplayNameScreen.swift b/Session/Onboarding/DisplayNameScreen.swift index 92dd172bb..cd3392887 100644 --- a/Session/Onboarding/DisplayNameScreen.swift +++ b/Session/Onboarding/DisplayNameScreen.swift @@ -116,7 +116,7 @@ struct DisplayNameScreen: View { // Try to save the user name but ignore the result ProfileManager.updateLocal( queue: .global(qos: .default), - profileName: displayName, + displayNameUpdate: .currentUserUpdate(displayName), using: dependencies ) diff --git a/Session/Settings/SettingsViewModel.swift b/Session/Settings/SettingsViewModel.swift index 5c710fa34..eab23ed27 100644 --- a/Session/Settings/SettingsViewModel.swift +++ b/Session/Settings/SettingsViewModel.swift @@ -20,11 +20,8 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl private lazy var imagePickerHandler: ImagePickerHandler = ImagePickerHandler( onTransition: { [weak self] in self?.transitionToScreen($0, transitionType: $1) }, onImageDataPicked: { [weak self] resultImageData in - guard let oldDisplayName: String = self?.oldDisplayName else { return } - self?.updatedProfilePictureSelected( - name: oldDisplayName, - avatarUpdate: .uploadImageData(resultImageData) + displayPictureUpdate: .currentUserUploadImageData(resultImageData) ) } ) @@ -205,10 +202,7 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl self?.setIsEditing(false) self?.oldDisplayName = updatedNickname - self?.updateProfile( - name: updatedNickname, - avatarUpdate: .none - ) + self?.updateProfile(displayNameUpdate: .currentUserUpdate(updatedNickname)) } ] } @@ -527,8 +521,7 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl onConfirm: { modal in modal.close() }, onCancel: { [weak self] modal in self?.updateProfile( - name: existingDisplayName, - avatarUpdate: .remove, + displayPictureUpdate: .currentUserRemove, onComplete: { [weak modal] in modal?.close() } ) }, @@ -544,7 +537,7 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl self.transitionToScreen(modal, transitionType: .present) } - fileprivate func updatedProfilePictureSelected(name: String, avatarUpdate: ProfileManager.AvatarUpdate) { + fileprivate func updatedProfilePictureSelected(displayPictureUpdate: ProfileManager.DisplayPictureUpdate) { guard let info: ConfirmationModal.Info = self.editProfilePictureModalInfo else { return } self.editProfilePictureModal?.updateContent( @@ -552,8 +545,8 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl body: .image( placeholderData: UIImage(named: "profile_placeholder")?.pngData(), valueData: { - switch avatarUpdate { - case .uploadImageData(let imageData): return imageData + switch displayPictureUpdate { + case .currentUserUploadImageData(let imageData): return imageData default: return nil } }(), @@ -568,8 +561,7 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl confirmEnabled: true, onConfirm: { [weak self] modal in self?.updateProfile( - name: name, - avatarUpdate: avatarUpdate, + displayPictureUpdate: displayPictureUpdate, onComplete: { [weak modal] in modal?.close() } ) } @@ -578,7 +570,7 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl } private func showPhotoLibraryForAvatar() { - Permissions.requestLibraryPermissionIfNeeded { [weak self] in + Permissions.requestLibraryPermissionIfNeeded(isSavingMedia: false) { [weak self] in DispatchQueue.main.async { let picker: UIImagePickerController = UIImagePickerController() picker.sourceType = .photoLibrary @@ -591,15 +583,15 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl } fileprivate func updateProfile( - name: String, - avatarUpdate: ProfileManager.AvatarUpdate, + displayNameUpdate: ProfileManager.DisplayNameUpdate = .none, + displayPictureUpdate: ProfileManager.DisplayPictureUpdate = .none, onComplete: (() -> ())? = nil ) { let viewController = ModalActivityIndicatorViewController(canCancel: false) { [weak self] modalActivityIndicator in ProfileManager.updateLocal( queue: .global(qos: .default), - profileName: name, - avatarUpdate: avatarUpdate, + displayNameUpdate: displayNameUpdate, + displayPictureUpdate: displayPictureUpdate, success: { db in // Wait for the database transaction to complete before updating the UI db.afterNextTransactionNested { _ in @@ -614,8 +606,8 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl DispatchQueue.main.async { modalActivityIndicator.dismiss { let title: String = { - switch (avatarUpdate, error) { - case (.remove, _): return "update_profile_modal_remove_error_title".localized() + switch (displayPictureUpdate, error) { + case (.currentUserRemove, _): return "update_profile_modal_remove_error_title".localized() case (_, .avatarUploadMaxFileSizeExceeded): return "update_profile_modal_max_size_error_title".localized() @@ -623,8 +615,8 @@ class SettingsViewModel: SessionTableViewModel, NavigationItemSource, Navigatabl } }() let message: String? = { - switch (avatarUpdate, error) { - case (.remove, _): return nil + switch (displayPictureUpdate, error) { + case (.currentUserRemove, _): return nil case (_, .avatarUploadMaxFileSizeExceeded): return "update_profile_modal_max_size_error_message".localized() diff --git a/Session/Utilities/Permissions.swift b/Session/Utilities/Permissions.swift index f360cc792..4e711d628 100644 --- a/Session/Utilities/Permissions.swift +++ b/Session/Utilities/Permissions.swift @@ -96,12 +96,14 @@ public enum Permissions { } public static func requestLibraryPermissionIfNeeded( + isSavingMedia: Bool, presentingViewController: UIViewController? = nil, onAuthorized: @escaping () -> Void ) { let authorizationStatus: PHAuthorizationStatus if #available(iOS 14, *) { - authorizationStatus = PHPhotoLibrary.authorizationStatus(for: .readWrite) + let targetPermission: PHAccessLevel = (isSavingMedia ? .addOnly : .readWrite) + authorizationStatus = PHPhotoLibrary.authorizationStatus(for: targetPermission) if authorizationStatus == .notDetermined { // When the user chooses to select photos (which is the .limit status), // the PHPhotoUI will present the picker view on the top of the front view. @@ -113,7 +115,7 @@ public enum Permissions { // from showing when we request the photo library permission. SessionEnvironment.shared?.isRequestingPermission = true - PHPhotoLibrary.requestAuthorization(for: .readWrite) { status in + PHPhotoLibrary.requestAuthorization(for: targetPermission) { status in SessionEnvironment.shared?.isRequestingPermission = false if [ PHAuthorizationStatus.authorized, PHAuthorizationStatus.limited ].contains(status) { onAuthorized() diff --git a/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift b/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift index 21afcc6de..2af0e650a 100644 --- a/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift +++ b/SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift @@ -48,8 +48,7 @@ public enum UpdateProfilePictureJob: JobExecutor { ProfileManager.updateLocal( queue: queue, - profileName: profile.name, - avatarUpdate: (profilePictureData.map { .uploadImageData($0) } ?? .none), + displayPictureUpdate: (profilePictureData.map { .currentUserUploadImageData($0) } ?? .none), success: { db in // Need to call the 'success' closure asynchronously on the queue to prevent a reentrancy // issue as it will write to the database and this closure is already called within diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserProfile.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserProfile.swift index fcdd8084f..0eb8554e4 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserProfile.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserProfile.swift @@ -42,11 +42,11 @@ internal extension LibSession { try ProfileManager.updateProfileIfNeeded( db, publicKey: userPublicKey, - name: profileName, - avatarUpdate: { - guard let profilePictureUrl: String = profilePictureUrl else { return .remove } + displayNameUpdate: .currentUserUpdate(profileName), + displayPictureUpdate: { + guard let profilePictureUrl: String = profilePictureUrl else { return .currentUserRemove } - return .updateTo( + return .currentUserUpdateTo( url: profilePictureUrl, key: Data( libSessionVal: profilePic.key, diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift index 79116401b..9d00e67e5 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift @@ -241,8 +241,8 @@ extension MessageReceiver { // Resubscribe for group push notifications let currentUserPublicKey: String = getUserHexEncodedPublicKey(db) - PushNotificationAPI - .subscribeToLegacyGroups( + (try? PushNotificationAPI + .preparedSubscribeToLegacyGroups( currentUserPublicKey: currentUserPublicKey, legacyGroupIds: try ClosedGroup .select(.threadId) @@ -253,8 +253,11 @@ extension MessageReceiver { ) .asRequest(of: String.self) .fetchSet(db) - .inserting(groupPublicKey) // Insert the new key just to be sure - ) + .inserting(groupPublicKey), // Insert the new key just to be sure + using: dependencies + ))? + .send(using: dependencies) + .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies) .sinkUntilComplete() } diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift index 7664f03b9..8d51eddb9 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+MessageRequests.swift @@ -31,14 +31,14 @@ extension MessageReceiver { try ProfileManager.updateProfileIfNeeded( db, publicKey: senderId, - name: profile.displayName, - avatarUpdate: { + displayNameUpdate: .contactUpdate(profile.displayName), + displayPictureUpdate: { guard let profilePictureUrl: String = profile.profilePictureUrl, let profileKey: Data = profile.profileKey else { return .none } - return .updateTo( + return .contactUpdateTo( url: profilePictureUrl, key: profileKey, fileName: nil diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift index 4e832979a..74806d380 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift @@ -30,20 +30,20 @@ extension MessageReceiver { try ProfileManager.updateProfileIfNeeded( db, publicKey: sender, - name: profile.displayName, - blocksCommunityMessageRequests: profile.blocksCommunityMessageRequests, - avatarUpdate: { + displayNameUpdate: .contactUpdate(profile.displayName), + displayPictureUpdate: { guard let profilePictureUrl: String = profile.profilePictureUrl, let profileKey: Data = profile.profileKey - else { return .remove } + else { return .contactRemove } - return .updateTo( + return .contactUpdateTo( url: profilePictureUrl, key: profileKey, fileName: nil ) }(), + blocksCommunityMessageRequests: profile.blocksCommunityMessageRequests, sentTimestamp: messageSentTimestamp, using: dependencies ) diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift index 7eddabef2..7d06da940 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift @@ -7,6 +7,11 @@ import SessionUtilitiesKit import SessionSnodeKit extension MessageSender { + typealias CreateGroupDatabaseResult = ( + SessionThread, + [MessageSender.PreparedSendData], + Network.PreparedRequest? + ) public static var distributingKeyPairs: Atomic<[String: [ClosedGroupKeyPair]]> = Atomic([:]) public static func createClosedGroup( @@ -15,7 +20,7 @@ extension MessageSender { using dependencies: Dependencies = Dependencies() ) -> AnyPublisher { dependencies.storage - .writePublisher { db -> (String, SessionThread, [MessageSender.PreparedSendData], Set) in + .writePublisher { db -> CreateGroupDatabaseResult in // Generate the group's two keys guard let groupKeyPair: KeyPair = dependencies.crypto.generate(.x25519KeyPair()), @@ -108,42 +113,54 @@ extension MessageSender { using: dependencies ) } - let allActiveLegacyGroupIds: Set = try ClosedGroup - .select(.threadId) - .filter(!ClosedGroup.Columns.threadId.like("\(SessionId.Prefix.group.rawValue)%")) - .joining( - required: ClosedGroup.members - .filter(GroupMember.Columns.profileId == userPublicKey) - ) - .asRequest(of: String.self) - .fetchSet(db) - .inserting(groupPublicKey) // Insert the new key just to be sure - return (userPublicKey, thread, memberSendData, allActiveLegacyGroupIds) + // Prepare the notification subscription + var preparedNotificationSubscription: Network.PreparedRequest? + + if let token: String = dependencies.standardUserDefaults[.deviceToken] { + preparedNotificationSubscription = try? PushNotificationAPI + .preparedSubscribeToLegacyGroups( + token: token, + currentUserPublicKey: userPublicKey, + legacyGroupIds: try ClosedGroup + .select(.threadId) + .filter(!ClosedGroup.Columns.threadId.like("\(SessionId.Prefix.group.rawValue)%")) + .joining( + required: ClosedGroup.members + .filter(GroupMember.Columns.profileId == userPublicKey) + ) + .asRequest(of: String.self) + .fetchSet(db) + .inserting(groupPublicKey), // Insert the new key just to be sure, + using: dependencies + ) + } + + return (thread, memberSendData, preparedNotificationSubscription) } - .flatMap { userPublicKey, thread, memberSendData, allActiveLegacyGroupIds in + .flatMap { thread, memberSendData, preparedNotificationSubscription -> AnyPublisher<(SessionThread, Network.PreparedRequest?), Error> in Publishers .MergeMany( // Send a closed group update message to all members individually - memberSendData - .map { MessageSender.sendImmediate(data: $0, using: dependencies) } - .appending( - // Resubscribe to all legacy groups - PushNotificationAPI.subscribeToLegacyGroups( - currentUserPublicKey: userPublicKey, - legacyGroupIds: allActiveLegacyGroupIds - ) - ) + memberSendData.map { MessageSender.sendImmediate(data: $0, using: dependencies) } ) .collect() - .map { _ in thread } + .map { _ in (thread, preparedNotificationSubscription) } + .eraseToAnyPublisher() } .handleEvents( - receiveOutput: { thread in + receiveOutput: { thread, preparedNotificationSubscription in + // Subscribe for push notifications (if PNs are enabled) + preparedNotificationSubscription? + .send(using: dependencies) + .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies) + .sinkUntilComplete() + // Start polling ClosedGroupPoller.shared.startIfNeeded(for: thread.id, using: dependencies) } ) + .map { thread, _ -> SessionThread in thread } .eraseToAnyPublisher() } diff --git a/SessionMessagingKit/Sending & Receiving/Notifications/Models/LegacyPushServerResponse.swift b/SessionMessagingKit/Sending & Receiving/Notifications/Models/LegacyPushServerResponse.swift index dc8c77ff7..bd412f24e 100644 --- a/SessionMessagingKit/Sending & Receiving/Notifications/Models/LegacyPushServerResponse.swift +++ b/SessionMessagingKit/Sending & Receiving/Notifications/Models/LegacyPushServerResponse.swift @@ -2,7 +2,7 @@ import Foundation -extension PushNotificationAPI { +public extension PushNotificationAPI { struct LegacyPushServerResponse: Codable { let code: Int let message: String? diff --git a/SessionMessagingKit/Sending & Receiving/Notifications/PushNotificationAPI.swift b/SessionMessagingKit/Sending & Receiving/Notifications/PushNotificationAPI.swift index 54f67ee74..54266d2a9 100644 --- a/SessionMessagingKit/Sending & Receiving/Notifications/PushNotificationAPI.swift +++ b/SessionMessagingKit/Sending & Receiving/Notifications/PushNotificationAPI.swift @@ -31,6 +31,11 @@ public enum PushNotificationAPI { isForcedUpdate: Bool, using dependencies: Dependencies = Dependencies() ) -> AnyPublisher { + typealias SubscribeAllPreparedRequests = ( + SubscribeRequest, + String, + Network.PreparedRequest? + ) let hexEncodedToken: String = token.toHexString() let oldToken: String? = dependencies.standardUserDefaults[.deviceToken] let lastUploadTime: Double = dependencies.standardUserDefaults[.lastDeviceTokenUpload] @@ -52,7 +57,7 @@ public enum PushNotificationAPI { // TODO: Need to generate requests for each updated group as well return dependencies.storage - .readPublisher(using: dependencies) { db -> (SubscribeRequest, String, Set) in + .readPublisher(using: dependencies) { db -> SubscribeAllPreparedRequests in guard let userED25519KeyPair: KeyPair = Identity.fetchUserEd25519KeyPair(db) else { throw SnodeAPIError.noKeyPair } @@ -74,22 +79,30 @@ public enum PushNotificationAPI { ed25519PublicKey: userED25519KeyPair.publicKey, ed25519SecretKey: userED25519KeyPair.secretKey ) + let preparedLegacyGroupRequest = try PushNotificationAPI + .preparedSubscribeToLegacyGroups( + forced: true, + token: hexEncodedToken, + currentUserPublicKey: currentUserPublicKey, + legacyGroupIds: try ClosedGroup + .select(.threadId) + .filter(!ClosedGroup.Columns.threadId.like("\(SessionId.Prefix.group.rawValue)%")) + .joining( + required: ClosedGroup.members + .filter(GroupMember.Columns.profileId == currentUserPublicKey) + ) + .asRequest(of: String.self) + .fetchSet(db), + using: dependencies + ) return ( request, currentUserPublicKey, - try ClosedGroup - .select(.threadId) - .filter(!ClosedGroup.Columns.threadId.like("\(SessionId.Prefix.group.rawValue)%")) - .joining( - required: ClosedGroup.members - .filter(GroupMember.Columns.profileId == currentUserPublicKey) - ) - .asRequest(of: String.self) - .fetchSet(db) + preparedLegacyGroupRequest ) } - .tryFlatMap { request, currentUserPublicKey, legacyGroupIds -> AnyPublisher in + .tryFlatMap { request, currentUserPublicKey, legacyGroupRequest -> AnyPublisher in Publishers .MergeMany( [ @@ -126,14 +139,12 @@ public enum PushNotificationAPI { .map { _ in () } .eraseToAnyPublisher(), // FIXME: Remove this once legacy groups are deprecated - PushNotificationAPI.subscribeToLegacyGroups( - forced: true, - token: hexEncodedToken, - currentUserPublicKey: currentUserPublicKey, - legacyGroupIds: legacyGroupIds, - using: dependencies - ) + legacyGroupRequest? + .send(using: dependencies) + .map { _, _ in () } + .eraseToAnyPublisher() ] + .compactMap { $0 } ) .collect() .map { _ in () } @@ -281,61 +292,52 @@ public enum PushNotificationAPI { // MARK: - Legacy Groups // FIXME: Remove this once legacy groups are deprecated - public static func subscribeToLegacyGroups( + public static func preparedSubscribeToLegacyGroups( forced: Bool = false, token: String? = nil, currentUserPublicKey: String, legacyGroupIds: Set, - using dependencies: Dependencies = Dependencies() - ) -> AnyPublisher { + using dependencies: Dependencies + ) throws -> Network.PreparedRequest? { let isUsingFullAPNs = dependencies.standardUserDefaults[.isUsingFullAPNs] // Only continue if PNs are enabled and we have a device token guard + !legacyGroupIds.isEmpty, (forced || isUsingFullAPNs), let deviceToken: String = (token ?? dependencies.standardUserDefaults[.deviceToken]) - else { - return Just(()) - .setFailureType(to: Error.self) - .eraseToAnyPublisher() - } + else { return nil } - do { - return try PushNotificationAPI - .prepareRequest( - request: Request( - method: .post, - endpoint: .legacyGroupsOnlySubscribe, - body: LegacyGroupOnlyRequest( - token: deviceToken, - pubKey: currentUserPublicKey, - device: "ios", - legacyGroupPublicKeys: legacyGroupIds - ), - using: dependencies + return try PushNotificationAPI + .prepareRequest( + request: Request( + method: .post, + endpoint: .legacyGroupsOnlySubscribe, + body: LegacyGroupOnlyRequest( + token: deviceToken, + pubKey: currentUserPublicKey, + device: "ios", + legacyGroupPublicKeys: legacyGroupIds ), - responseType: LegacyPushServerResponse.self, using: dependencies - ) - .send(using: dependencies) - .retry(maxRetryCount, using: dependencies) - .handleEvents( - receiveOutput: { _, response in - guard response.code != 0 else { - return SNLog("Couldn't subscribe for legacy groups due to error: \(response.message ?? "nil").") - } - }, - receiveCompletion: { result in - switch result { - case .finished: break - case .failure: SNLog("Couldn't subscribe for legacy groups.") - } + ), + responseType: LegacyPushServerResponse.self, + retryCount: PushNotificationAPI.maxRetryCount, + using: dependencies + ) + .handleEvents( + receiveOutput: { _, response in + guard response.code != 0 else { + return Log.error("[PushNotificationAPI] Couldn't subscribe for legacy groups due to error: \(response.message ?? "nil").") } - ) - .map { _ in () } - .eraseToAnyPublisher() - } - catch { return Fail(error: error).eraseToAnyPublisher() } + }, + receiveCompletion: { result in + switch result { + case .finished: break + case .failure: Log.error("[PushNotificationAPI] Couldn't subscribe for legacy groups.") + } + } + ) } // FIXME: Remove this once legacy groups are deprecated diff --git a/SessionMessagingKit/Utilities/ProfileManager.swift b/SessionMessagingKit/Utilities/ProfileManager.swift index f492c8500..9261690d3 100644 --- a/SessionMessagingKit/Utilities/ProfileManager.swift +++ b/SessionMessagingKit/Utilities/ProfileManager.swift @@ -7,11 +7,21 @@ import GRDB import SessionUtilitiesKit public struct ProfileManager { - public enum AvatarUpdate { + public enum DisplayNameUpdate { case none - case remove - case uploadImageData(Data) - case updateTo(url: String, key: Data, fileName: String?) + case contactUpdate(String?) + case currentUserUpdate(String?) + } + + public enum DisplayPictureUpdate { + case none + + case contactRemove + case contactUpdateTo(url: String, key: Data, fileName: String?) + + case currentUserRemove + case currentUserUploadImageData(Data) + case currentUserUpdateTo(url: String, key: Data, fileName: String?) } // The max bytes for a user's profile name, encoded in UTF8. @@ -281,24 +291,27 @@ public struct ProfileManager { public static func updateLocal( queue: DispatchQueue, - profileName: String, - avatarUpdate: AvatarUpdate = .none, + displayNameUpdate: DisplayNameUpdate = .none, + displayPictureUpdate: DisplayPictureUpdate = .none, success: ((Database) throws -> ())? = nil, failure: ((ProfileManagerError) -> ())? = nil, using dependencies: Dependencies = Dependencies() ) { let userPublicKey: String = getUserHexEncodedPublicKey(using: dependencies) - let isRemovingAvatar: Bool = { - switch avatarUpdate { - case .remove: return true + let isRemovingDisplayPicture: Bool = { + switch displayPictureUpdate { + case .currentUserRemove: return true default: return false } }() - switch avatarUpdate { - case .none, .remove, .updateTo: + switch displayPictureUpdate { + case .contactRemove, .contactUpdateTo: + failure?(ProfileManagerError.invalidCall) + + case .none, .currentUserRemove, .currentUserUpdateTo: dependencies.storage.writeAsync { db in - if isRemovingAvatar { + if isRemovingDisplayPicture { let existingProfileUrl: String? = try Profile .filter(id: userPublicKey) .select(.profilePictureUrl) @@ -324,8 +337,8 @@ public struct ProfileManager { try ProfileManager.updateProfileIfNeeded( db, publicKey: userPublicKey, - name: profileName, - avatarUpdate: avatarUpdate, + displayNameUpdate: displayNameUpdate, + displayPictureUpdate: displayPictureUpdate, sentTimestamp: dependencies.dateNow.timeIntervalSince1970, using: dependencies ) @@ -334,7 +347,7 @@ public struct ProfileManager { try success?(db) } - case .uploadImageData(let data): + case .currentUserUploadImageData(let data): prepareAndUploadAvatarImage( queue: queue, imageData: data, @@ -343,8 +356,8 @@ public struct ProfileManager { try ProfileManager.updateProfileIfNeeded( db, publicKey: userPublicKey, - name: profileName, - avatarUpdate: .updateTo(url: downloadUrl, key: newProfileKey, fileName: fileName), + displayNameUpdate: displayNameUpdate, + displayPictureUpdate: .currentUserUpdateTo(url: downloadUrl, key: newProfileKey, fileName: fileName), sentTimestamp: dependencies.dateNow.timeIntervalSince1970, using: dependencies ) @@ -494,9 +507,9 @@ public struct ProfileManager { public static func updateProfileIfNeeded( _ db: Database, publicKey: String, - name: String?, + displayNameUpdate: DisplayNameUpdate = .none, + displayPictureUpdate: DisplayPictureUpdate, blocksCommunityMessageRequests: Bool? = nil, - avatarUpdate: AvatarUpdate, sentTimestamp: TimeInterval, calledFromConfigHandling: Bool = false, using dependencies: Dependencies @@ -506,15 +519,21 @@ public struct ProfileManager { var profileChanges: [ConfigColumnAssignment] = [] // Name - if let name: String = name, !name.isEmpty, name != profile.name { - if sentTimestamp > (profile.lastNameUpdate ?? 0) || (isCurrentUser && calledFromConfigHandling) { + // FIXME: This 'lastNameUpdate' approach is buggy - we should have a timestamp on the ConvoInfoVolatile + switch (displayNameUpdate, isCurrentUser, (sentTimestamp > (profile.lastNameUpdate ?? 0))) { + case (.none, _, _): break + case (.currentUserUpdate(let name), true, _), (.contactUpdate(let name), false, true): + guard let name: String = name, !name.isEmpty, name != profile.name else { break } + profileChanges.append(Profile.Columns.name.set(to: name)) profileChanges.append(Profile.Columns.lastNameUpdate.set(to: sentTimestamp)) - } + + // Don't want profiles in messages to modify the current users profile info so ignore those cases + default: break } - // Blocks community message requets flag - if let blocksCommunityMessageRequests: Bool = blocksCommunityMessageRequests, sentTimestamp > (profile.lastBlocksCommunityMessageRequests ?? 0) { + // Blocks community message requets flag (only update for other users) + if !isCurrentUser, let blocksCommunityMessageRequests: Bool = blocksCommunityMessageRequests, sentTimestamp > (profile.lastBlocksCommunityMessageRequests ?? 0) { profileChanges.append(Profile.Columns.blocksCommunityMessageRequests.set(to: blocksCommunityMessageRequests)) profileChanges.append(Profile.Columns.lastBlocksCommunityMessageRequests.set(to: sentTimestamp)) } @@ -522,43 +541,49 @@ public struct ProfileManager { // Profile picture & profile key var avatarNeedsDownload: Bool = false var targetAvatarUrl: String? = nil + let shouldUpdateAvatar: Bool = ( + (!isCurrentUser && (sentTimestamp > (profile.lastProfilePictureUpdate ?? 0))) || // Update other users + (isCurrentUser && calledFromConfigHandling) // Only update the current user via config messages + ) - if sentTimestamp > (profile.lastProfilePictureUpdate ?? 0) || (isCurrentUser && calledFromConfigHandling) { - switch avatarUpdate { - case .none: break - case .uploadImageData: preconditionFailure("Invalid options for this function") - - case .remove: - profileChanges.append(Profile.Columns.profilePictureUrl.set(to: nil)) - profileChanges.append(Profile.Columns.profileEncryptionKey.set(to: nil)) - profileChanges.append(Profile.Columns.profilePictureFileName.set(to: nil)) - profileChanges.append(Profile.Columns.lastProfilePictureUpdate.set(to: sentTimestamp)) - - case .updateTo(let url, let key, let fileName): - if url != profile.profilePictureUrl { - profileChanges.append(Profile.Columns.profilePictureUrl.set(to: url)) - avatarNeedsDownload = true - targetAvatarUrl = url - } - - if key != profile.profileEncryptionKey && key.count == ProfileManager.avatarAES256KeyByteLength { - profileChanges.append(Profile.Columns.profileEncryptionKey.set(to: key)) - } - - // Profile filename (this isn't synchronized between devices) - if let fileName: String = fileName { - profileChanges.append(Profile.Columns.profilePictureFileName.set(to: fileName)) - - // If we have already downloaded the image then no need to download it again - avatarNeedsDownload = ( - avatarNeedsDownload && - !ProfileManager.hasProfileImageData(with: fileName) - ) - } + switch (displayPictureUpdate, isCurrentUser) { + case (.none, _): break + case (.currentUserUploadImageData, _): preconditionFailure("Invalid options for this function") + + case (.contactRemove, false), (.currentUserRemove, true): + profileChanges.append(Profile.Columns.profilePictureUrl.set(to: nil)) + profileChanges.append(Profile.Columns.profileEncryptionKey.set(to: nil)) + profileChanges.append(Profile.Columns.profilePictureFileName.set(to: nil)) + profileChanges.append(Profile.Columns.lastProfilePictureUpdate.set(to: sentTimestamp)) + + case (.contactUpdateTo(let url, let key, let fileName), false), + (.currentUserUpdateTo(let url, let key, let fileName), true): + if url != profile.profilePictureUrl { + profileChanges.append(Profile.Columns.profilePictureUrl.set(to: url)) + avatarNeedsDownload = true + targetAvatarUrl = url + } + + if key != profile.profileEncryptionKey && key.count == ProfileManager.avatarAES256KeyByteLength { + profileChanges.append(Profile.Columns.profileEncryptionKey.set(to: key)) + } + + // Profile filename (this isn't synchronized between devices) + if let fileName: String = fileName { + profileChanges.append(Profile.Columns.profilePictureFileName.set(to: fileName)) - // Update the 'lastProfilePictureUpdate' timestamp for either external or local changes - profileChanges.append(Profile.Columns.lastProfilePictureUpdate.set(to: sentTimestamp)) - } + // If we have already downloaded the image then no need to download it again + avatarNeedsDownload = ( + avatarNeedsDownload && + !ProfileManager.hasProfileImageData(with: fileName) + ) + } + + // Update the 'lastProfilePictureUpdate' timestamp for either external or local changes + profileChanges.append(Profile.Columns.lastProfilePictureUpdate.set(to: sentTimestamp)) + + // Don't want profiles in messages to modify the current users profile info so ignore those cases + default: break } // Persist any changes diff --git a/SessionMessagingKit/Utilities/ProfileManagerError.swift b/SessionMessagingKit/Utilities/ProfileManagerError.swift index a522e492e..d709cfac9 100644 --- a/SessionMessagingKit/Utilities/ProfileManagerError.swift +++ b/SessionMessagingKit/Utilities/ProfileManagerError.swift @@ -1,4 +1,6 @@ // Copyright © 2022 Rangeproof Pty Ltd. All rights reserved. +// +// stringlint:disable import Foundation @@ -17,7 +19,7 @@ public enum ProfileManagerError: LocalizedError { case .avatarEncryptionFailed: return "Avatar encryption failed." case .avatarUploadFailed: return "Avatar upload failed." case .avatarUploadMaxFileSizeExceeded: return "Maximum file size exceeded." - case .invalidCall: return "Attempted to remove avatar using the wrong method." + case .invalidCall: return "Attempted to modify profile using the wrong method." } } }