Tweaks to notification extension

• Fixed an issue where the notification extension could end up in an invalid state if you delete and create a new account in rapid succession
• Fixed an issue where notification processing errors weren't getting handled correctly resulting in the notification extension timing out
• Stopped the notification extension from trying to download profile images
• Added commit hash to version info
• Tweaked the notification extension logic flow to be more straight forward

Session.xcodeproj/project.pbxproj

@@ -8042,7 +8042,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
-				CURRENT_PROJECT_VERSION = 452;
+				CURRENT_PROJECT_VERSION = 453;
 				ENABLE_BITCODE = NO;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;
 				ENABLE_TESTABILITY = YES;
@@ -8079,7 +8079,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				HEADER_SEARCH_PATHS = "";
 				IPHONEOS_DEPLOYMENT_TARGET = 13.0;
-				MARKETING_VERSION = 2.6.2;
+				MARKETING_VERSION = 2.6.3;
 				ONLY_ACTIVE_ARCH = YES;
 				OTHER_CFLAGS = (
 					"-fobjc-arc-exceptions",
@@ -8120,7 +8120,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Distribution";
-				CURRENT_PROJECT_VERSION = 452;
+				CURRENT_PROJECT_VERSION = 453;
 				ENABLE_BITCODE = NO;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;
 				GCC_NO_COMMON_BLOCKS = YES;
@@ -8152,7 +8152,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				HEADER_SEARCH_PATHS = "";
 				IPHONEOS_DEPLOYMENT_TARGET = 13.0;
-				MARKETING_VERSION = 2.6.2;
+				MARKETING_VERSION = 2.6.3;
 				ONLY_ACTIVE_ARCH = NO;
 				OTHER_CFLAGS = (
 					"-DNS_BLOCK_ASSERTIONS=1",

Session/Meta/SessionApp.swift

@@ -17,11 +17,7 @@ public struct SessionApp {
             .defaulting(to: "")
         let appVersion: String? = (Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String)
             .map { "App: \($0)\(buildNumber)" }
-        #if DEBUG
         let commitInfo: String? = (Bundle.main.infoDictionary?["GitCommitHash"] as? String).map { "Commit: \($0)" }
-        #else
-        let commitInfo: String? = nil
-        #endif
         
         let versionInfo: [String] = [
             "iOS \(UIDevice.current.systemVersion)",
@@ -119,9 +115,11 @@ public struct SessionApp {
         ProfileManager.resetProfileStorage()
         Attachment.resetAttachmentStorage()
         AppEnvironment.shared.notificationPresenter.clearAllNotifications()
-        Log.flush()
         
         onReset?()
+        Log.info("Data Reset Complete.")
+        Log.flush()
+
         exit(0)
     }
     

Session/Settings/Views/VersionFooterView.swift

@@ -33,12 +33,19 @@ class VersionFooterView: UIView {
         result.lineBreakMode = .byCharWrapping
         result.numberOfLines = 0
         
-        if
+        let infoDict = Bundle.main.infoDictionary
-            let version: String = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String,
+        let version: String = ((infoDict?["CFBundleShortVersionString"] as? String) ?? "0.0.0")   // stringlint:disable
-            let buildNumber: String = Bundle.main.infoDictionary?["CFBundleVersion"] as? String
+        let buildNumber: String? = (infoDict?["CFBundleShortVersionString"] as? String)           // stringlint:disable
-        {
+        let commitInfo: String? = (infoDict?["GitCommitHash"] as? String)                         // stringlint:disable
-            result.text = "Version \(version) (\(buildNumber))"
+        let buildInfo: String = [buildNumber, commitInfo]
-        }
+            .compactMap { $0 }
+            .joined(separator: " - ")
+        result.text = [
+            "Version \(version)",
+            (!buildInfo.isEmpty ? " (" : ""),
+            buildInfo,
+            (!buildInfo.isEmpty ? ")" : ""),
+        ].joined()
         
         return result
     }()

SessionMessagingKit/LibSession/Config Handling/LibSession+Contacts.swift

@@ -331,9 +331,15 @@ internal extension LibSession {
                     // Download the profile picture if needed (this can be triggered within
                     // database reads/writes so dispatch the download to a separate queue to
                     // prevent blocking)
+                    //
+                    // Note: Only trigger the avatar download if we are in the main app (don't
+                    // want the extensions to trigger this as it can clog up their networking)
                     if
+                        Singleton.hasAppContext &&
+                        Singleton.appContext.isMainApp && (
                             oldAvatarUrl != (updatedProfile.profilePictureUrl ?? "") ||
                             oldAvatarKey != (updatedProfile.profileEncryptionKey ?? Data(repeating: 0, count: ProfileManager.avatarAES256KeyByteLength))
+                        )
                     {
                         DispatchQueue.global(qos: .background).async {
                             ProfileManager.downloadAvatar(for: updatedProfile)

SessionMessagingKit/Messages/Message.swift

@@ -371,7 +371,7 @@ public extension Message {
         _ db: Database,
         data: Data,
         metadata: PushNotificationAPI.NotificationMetadata,
-        using dependencies: Dependencies = Dependencies()
+        using dependencies: Dependencies
     ) throws -> ProcessedMessage? {
         return try processRawReceivedMessage(
             db,

SessionMessagingKit/Sending & Receiving/Notifications/PushNotificationAPI.swift

@@ -382,7 +382,7 @@ public enum PushNotificationAPI {
     
     public static func processNotification(
         notificationContent: UNNotificationContent,
-        dependencies: Dependencies = Dependencies()
+        using dependencies: Dependencies
     ) -> (data: Data?, metadata: NotificationMetadata, result: ProcessResult) {
         // Make sure the notification is from the updated push server
         guard notificationContent.userInfo["spns"] != nil else {

SessionMessagingKit/Utilities/ProfileManager.swift

@@ -578,7 +578,8 @@ public struct ProfileManager {
         }
         
         // Download the profile picture if needed
-        guard avatarNeedsDownload else { return }
+        // FIXME: We don't want to trigger the download within the notification extension, as part of the groups rebuild this has been moved into a Job which won't be run so this logic can be removed
+        guard avatarNeedsDownload && Singleton.hasAppContext && Singleton.appContext.isMainApp else { return }
         
         let dedupeIdentifier: String = "AvatarDownload-\(publicKey)-\(targetAvatarUrl ?? "remove")" // stringlint:disable
         

SessionNotificationServiceExtension/NotificationServiceExtension.swift

@@ -12,6 +12,7 @@ import SignalCoreKit
 import SessionUtilitiesKit
 
 public final class NotificationServiceExtension: UNNotificationServiceExtension {
+    private let dependencies: Dependencies = Dependencies()
     private var didPerformSetup = false
     private var contentHandler: ((UNNotificationContent) -> Void)?
     private var request: UNNotificationRequest?
@@ -29,6 +30,9 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
         self.contentHandler = contentHandler
         self.request = request
         
+        // It's technically possible for 'completeSilently' to be called twice due to the NSE timeout so
+        self.hasCompleted.mutate { $0 = false }
+        
         // Abort if the main app is running
         guard !(UserDefaults.sharedLokiProject?[.isMainAppActive]).defaulting(to: false) else {
             Log.info("didReceive called while main app running.")
@@ -47,19 +51,31 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
             Singleton.setup(appContext: NotificationServiceExtensionContext())
         }
         
-        let isCallOngoing: Bool = (UserDefaults.sharedLokiProject?[.isCallOngoing])
-            .defaulting(to: false)
-
         // Perform main setup
         Storage.resumeDatabaseAccess()
-        DispatchQueue.main.sync { self.setUpIfNecessary() { } }
+        DispatchQueue.main.sync {
+            self.setUpIfNecessary() { [weak self] in
+                self?.handleNotification(notificationContent, isPerformingResetup: false)
+            }
+        }
+    }
     
-        // Handle the push notification
+    private func handleNotification(_ notificationContent: UNMutableNotificationContent, isPerformingResetup: Bool) {
-        Singleton.appReadiness.runNowOrWhenAppDidBecomeReady {
         let (maybeData, metadata, result) = PushNotificationAPI.processNotification(
-                notificationContent: notificationContent
+            notificationContent: notificationContent,
+            using: dependencies
         )
         
+        guard metadata.accountId == getUserHexEncodedPublicKey(using: dependencies) else {
+            guard !isPerformingResetup else {
+                Log.error("Received notification for an accountId that isn't the current user, resetup failed.")
+                return self.completeSilenty()
+            }
+            
+            Log.warn("Received notification for an accountId that isn't the current user, attempting to resetup.")
+            return self.forceResetup(notificationContent)
+        }
+        
         guard
             (result == .success || result == .legacySuccess),
             let data: Data = maybeData
@@ -73,19 +89,22 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                 // Just log if the notification was too long (a ~2k message should be able to fit so
                 // these will most commonly be call or config messages)
                 case .successTooLong:
-                        Log.info("Received too long notification for namespace: \(metadata.namespace).")
+                    Log.info("Received too long notification for namespace: \(metadata.namespace), dataLength: \(metadata.dataLength).")
                     return self.completeSilenty()
                     
                 case .legacyForceSilent, .failureNoContent: return self.completeSilenty()
             }
         }
         
+        let isCallOngoing: Bool = (UserDefaults.sharedLokiProject?[.isCallOngoing])
+            .defaulting(to: false)
+        
         // HACK: It is important to use write synchronously here to avoid a race condition
         // where the completeSilenty() is called before the local notification request
         // is added to notification center
-            Storage.shared.write { [weak self] db in
+        dependencies.storage.write { [weak self, dependencies] db in
             do {
-                    guard let processedMessage: ProcessedMessage = try Message.processRawReceivedMessageAsNotification(db, data: data, metadata: metadata) else {
+                guard let processedMessage: ProcessedMessage = try Message.processRawReceivedMessageAsNotification(db, data: data, metadata: metadata, using: dependencies) else {
                     throw NotificationError.messageProcessing
                 }
                 
@@ -117,7 +136,8 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                             db,
                             message: messageInfo.message,
                             threadId: threadId,
-                                threadVariant: threadVariant
+                            threadVariant: threadVariant,
+                            using: dependencies
                         )
                         
                         try MessageReceiver.handleCallMessage(
@@ -187,7 +207,8 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                             threadVariant: threadVariant,
                             message: messageInfo.message,
                             serverExpirationTimestamp: messageInfo.serverExpirationTimestamp,
-                                associatedWithProto: proto
+                            associatedWithProto: proto,
+                            using: dependencies
                         )
                 }
                 
@@ -215,7 +236,7 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                     }
                 }
                 
-                    db.afterNextTransactionNested(
+                db.afterNextTransaction(
                     onCommit: { _ in  handleError() },
                     onRollback: { _ in handleError() }
                 )
@@ -223,7 +244,6 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
             }
         }
     }
-    }
 
     // MARK: Setup
 
@@ -233,7 +253,7 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
         // The NSE will often re-use the same process, so if we're
         // already set up we want to do nothing; we're already ready
         // to process new messages.
-        guard !didPerformSetup else { return }
+        guard !didPerformSetup else { return completion() }
 
         Log.info("Performing setup.")
         didPerformSetup = true
@@ -257,7 +277,6 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                 
                 // Setup LibSession
                 LibSession.addLogger()
-                LibSession.createNetworkIfNeeded()
             },
             migrationsCompletion: { [weak self] result, needsConfigSync in
                 switch result {
@@ -278,16 +297,14 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
                         }
                         
                         DispatchQueue.main.async {
-                            self?.versionMigrationsDidComplete(needsConfigSync: needsConfigSync)
+                            self?.versionMigrationsDidComplete(needsConfigSync: needsConfigSync, completion: completion)
                         }
                 }
-                
-                completion()
             }
         )
     }
     
-    private func versionMigrationsDidComplete(needsConfigSync: Bool) {
+    private func versionMigrationsDidComplete(needsConfigSync: Bool, completion: @escaping () -> Void) {
         AssertIsOnMainThread()
         
         // If we need a config sync then trigger it now
@@ -297,28 +314,49 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
             }
         }
         
-        checkIsAppReady(migrationsCompleted: true)
-    }
-
-    private func checkIsAppReady(migrationsCompleted: Bool) {
-        AssertIsOnMainThread()
-
-        // Only mark the app as ready once.
-        guard !Singleton.appReadiness.isAppReady else { return }
-
         // App isn't ready until storage is ready AND all version migrations are complete.
-        guard Storage.shared.isValid && migrationsCompleted else {
+        guard Storage.shared.isValid else {
             Log.error("Storage invalid.")
-            self.completeSilenty()
+            return self.completeSilenty()
-            return
         }
         
-        SignalUtilitiesKit.Configuration.performMainSetup()
+        // If the app wasn't ready then mark it as ready now
-
+        if !Singleton.appReadiness.isAppReady {
             // Note that this does much more than set a flag; it will also run all deferred blocks.
             Singleton.appReadiness.setAppReady()
         }
         
+        completion()
+    }
+    
+    /// It's possible for the NotificationExtension to still have some kind of cached data from the old database after it's been deleted
+    /// when a new account is created shortly after, this results in weird errors when receiving PNs for the new account
+    ///
+    /// In order to avoid this situation we check to see whether the received PN is targetting the current user and, if not, we call this
+    /// method to force a resetup of the notification extension
+    ///
+    /// **Note:** We need to reconfigure the database here because if the database was deleted it's possible for the NotificationExtension
+    /// to somehow still have some form of access to the old one
+    private func forceResetup(_ notificationContent: UNMutableNotificationContent) {
+        Storage.reconfigureDatabase()
+        LibSession.clearMemoryState()
+        dependencies.caches.mutate(cache: .general) { $0.clearCachedUserPublicKey() }
+        
+        self.setUpIfNecessary() { [weak self] in
+            // If we had already done a setup then `libSession` won't have been re-setup so
+            // we need to do so now (this ensures it has the correct user keys as well)
+            Storage.shared.read { db in
+                LibSession.loadState(
+                    db,
+                    userPublicKey: getUserHexEncodedPublicKey(db),
+                    ed25519SecretKey: Identity.fetchUserEd25519KeyPair(db)?.secretKey
+                )
+            }
+            
+            self?.handleNotification(notificationContent, isPerformingResetup: true)
+        }
+    }
+    
     // MARK: Handle completion
     
     override public func serviceExtensionTimeWillExpire() {
@@ -434,5 +472,6 @@ public final class NotificationServiceExtension: UNNotificationServiceExtension
         let userInfo: [String: Any] = [ NotificationServiceExtension.isFromRemoteKey: true ]
         content.userInfo = userInfo
         contentHandler!(content)
+        hasCompleted.mutate { $0 = true }
     }
 }

SessionUtilitiesKit/Database/Storage.swift

@@ -428,7 +428,7 @@ open class Storage {
         Storage.shared.dbWriter = nil
         
         deleteDatabaseFiles()
-        try? deleteDbKeys()
+        do { try deleteDbKeys() } catch { Log.warn("Failed to delete database keys.") }
     }
     
     public static func reconfigureDatabase() {
@@ -444,9 +444,9 @@ open class Storage {
     }
     
     private static func deleteDatabaseFiles() {
-        OWSFileSystem.deleteFile(databasePath)
+        if !OWSFileSystem.deleteFile(databasePath) { Log.warn("Failed to delete database.") }
-        OWSFileSystem.deleteFile(databasePathShm)
+        if !OWSFileSystem.deleteFile(databasePathShm) { Log.warn("Failed to delete database-shm.") }
-        OWSFileSystem.deleteFile(databasePathWal)
+        if !OWSFileSystem.deleteFile(databasePathWal) { Log.warn("Failed to delete database-wal.") }
     }
     
     private static func deleteDbKeys() throws {

SessionUtilitiesKit/Database/Utilities/Database+Utilities.swift

@@ -135,21 +135,12 @@ fileprivate class TransactionHandler: TransactionObserver {
             }
         }
         catch {
-            SNLog("[Database] afterNextTransactionNested onCommit failed")
+            Log.warn("[Database] afterNextTransactionNested onCommit failed")
         }
     }
     
     func databaseDidRollback(_ db: Database) {
         TransactionHandler.registeredHandlers.mutate { $0.remove(identifier) }
-        
-        do {
-            try db.inTransaction {
         onRollback(db)
-                return .commit
-            }
-        }
-        catch {
-            SNLog("[Database] afterNextTransactionNested onRollback failed")
-        }
     }
 }

SessionUtilitiesKit/General/General.swift

@@ -9,6 +9,10 @@ public enum General {
     public class Cache: GeneralCacheType {
         public var encodedPublicKey: String? = nil
         public var recentReactionTimestamps: [Int64] = []
+        
+        public func clearCachedUserPublicKey() {
+            encodedPublicKey = nil
+        }
     }
 }
 
@@ -55,4 +59,6 @@ public protocol ImmutableGeneralCacheType: ImmutableCacheType {
 public protocol GeneralCacheType: ImmutableGeneralCacheType, MutableCacheType {
     var encodedPublicKey: String? { get set }
     var recentReactionTimestamps: [Int64] { get set }
+    
+    func clearCachedUserPublicKey()
 }