From fd02decf9f54d53f409367eaf78fb3b05ba29343 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 19 Jan 2024 15:51:14 +1100 Subject: [PATCH] Updated the PNModeVC to explicitly trigger the SyncPushTokensJob --- Session/Notifications/SyncPushTokensJob.swift | 32 ++++++++++++++++- Session/Onboarding/PNModeVC.swift | 36 +++++++++---------- .../PersistableRecord+Utilities.swift | 8 +++++ SessionUtilitiesKit/JobRunner/JobRunner.swift | 3 ++ 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/Session/Notifications/SyncPushTokensJob.swift b/Session/Notifications/SyncPushTokensJob.swift index 5062637ad..b55deb480 100644 --- a/Session/Notifications/SyncPushTokensJob.swift +++ b/Session/Notifications/SyncPushTokensJob.swift @@ -4,6 +4,7 @@ import Foundation import Combine import GRDB import SignalCoreKit +import SessionSnodeKit import SessionMessagingKit import SessionUtilitiesKit import SignalCoreKit @@ -13,6 +14,7 @@ public enum SyncPushTokensJob: JobExecutor { public static let requiresThreadId: Bool = false public static let requiresInteractionId: Bool = false private static let maxFrequency: TimeInterval = (12 * 60 * 60) + private static let maxRunFrequency: TimeInterval = 1 public static func run( _ job: Job, @@ -31,6 +33,27 @@ public enum SyncPushTokensJob: JobExecutor { return deferred(job, dependencies) } + /// Since this job can be dependant on network conditions it's possible for multiple jobs to run at the same time, while this shouldn't cause issues + /// it can result in multiple API calls getting made concurrently so to avoid this we defer the job as if the previous one was successful then the + /// `lastPushNotificationSync` value will prevent the subsequent call being made + guard + dependencies.jobRunner + .jobInfoFor(state: .running, variant: .syncPushTokens) + .filter({ key, info in key != job.id }) // Exclude this job + .isEmpty + else { + // Defer the job to run 'maxRunFrequency' from when this one ran (if we don't it'll try start + // it again immediately which is pointless) + let updatedJob: Job? = dependencies.storage.write { db in + try job + .with(nextRunTimestamp: dependencies.dateNow.timeIntervalSince1970 + maxRunFrequency) + .upserted(db) + } + + SNLog("[SyncPushTokensJob] Deferred due to in progress job") + return deferred(updatedJob ?? job, dependencies) + } + // Determine if the device has 'Fast Mode' (APNS) enabled let isUsingFullAPNs: Bool = UserDefaults.standard[.isUsingFullAPNs] @@ -82,9 +105,16 @@ public enum SyncPushTokensJob: JobExecutor { /// /// **Note:** Apple's documentation states that we should re-register for notifications on every launch: /// https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/HandlingRemoteNotifications.html#//apple_ref/doc/uid/TP40008194-CH6-SW1 - Logger.info("Re-registering for remote notifications.") + SNLog("[SyncPushTokensJob] Re-registering for remote notifications") PushRegistrationManager.shared.requestPushTokens() .flatMap { (pushToken: String, voipToken: String) -> AnyPublisher in + guard !OnionRequestAPI.paths.isEmpty else { + SNLog("[SyncPushTokensJob] OS subscription completed, skipping server subscription due to lack of paths") + return Just(()) + .setFailureType(to: Error.self) + .eraseToAnyPublisher() + } + /// For our `subscribe` endpoint we only want to call it if: /// • It's been longer than `SyncPushTokensJob.maxFrequency` since the last subscription; /// • The token has changed; or diff --git a/Session/Onboarding/PNModeVC.swift b/Session/Onboarding/PNModeVC.swift index c389e7b9b..33cdf58b3 100644 --- a/Session/Onboarding/PNModeVC.swift +++ b/Session/Onboarding/PNModeVC.swift @@ -141,16 +141,13 @@ final class PNModeVC: BaseVC, OptionViewDelegate { self.present(modal, animated: true) return } - UserDefaults.standard[.isUsingFullAPNs] = (selectedOptionView == apnsOptionView) + + let useAPNS: Bool = (selectedOptionView == apnsOptionView) + UserDefaults.standard[.isUsingFullAPNs] = useAPNS // If we are registering then we can just continue on guard flow != .register else { - self.flow.completeRegistration() - - // Go to the home screen - let homeVC: HomeVC = HomeVC() - self.navigationController?.setViewControllers([ homeVC ], animated: true) - return + return self.completeRegistration(useAPNS: useAPNS) } // Check if we already have a profile name (ie. profile retrieval completed while waiting on @@ -166,12 +163,7 @@ final class PNModeVC: BaseVC, OptionViewDelegate { guard existingProfileName?.isEmpty != false else { // If we have one then we can go straight to the home screen - self.flow.completeRegistration() - - // Go to the home screen - let homeVC: HomeVC = HomeVC() - self.navigationController?.setViewControllers([ homeVC ], animated: true) - return + return self.completeRegistration(useAPNS: useAPNS) } // If we don't have one then show a loading indicator and try to retrieve the existing name @@ -199,13 +191,21 @@ final class PNModeVC: BaseVC, OptionViewDelegate { } // Otherwise we are done and can go to the home screen - self?.flow.completeRegistration() - - // Go to the home screen - let homeVC: HomeVC = HomeVC() - self?.navigationController?.setViewControllers([ homeVC ], animated: true) + self?.completeRegistration(useAPNS: useAPNS) } ) } } + + private func completeRegistration(useAPNS: Bool) { + self.flow.completeRegistration() + + // Trigger the 'SyncPushTokensJob' directly as we don't want to wait for paths to build + // before requesting the permission from the user + if useAPNS { SyncPushTokensJob.run(uploadOnlyIfStale: false) } + + // Go to the home screen + let homeVC: HomeVC = HomeVC() + self.navigationController?.setViewControllers([ homeVC ], animated: true) + } } diff --git a/SessionUtilitiesKit/Database/Utilities/PersistableRecord+Utilities.swift b/SessionUtilitiesKit/Database/Utilities/PersistableRecord+Utilities.swift index a5fd86d1f..ba86dae43 100644 --- a/SessionUtilitiesKit/Database/Utilities/PersistableRecord+Utilities.swift +++ b/SessionUtilitiesKit/Database/Utilities/PersistableRecord+Utilities.swift @@ -46,6 +46,14 @@ public extension MutablePersistableRecord where Self: TableRecord & EncodableRec } } +public extension MutablePersistableRecord where Self: FetchableRecord { + @discardableResult func upserted(_ db: Database) throws -> Self { + var mutableSelf: Self = self + try mutableSelf.upsert(db) + return mutableSelf + } +} + // MARK: - MigrationSafeMutableRecord private class MigrationSafeRecord: MigrationSafeMutableRecord {} diff --git a/SessionUtilitiesKit/JobRunner/JobRunner.swift b/SessionUtilitiesKit/JobRunner/JobRunner.swift index 796aba7f9..dde276ceb 100644 --- a/SessionUtilitiesKit/JobRunner/JobRunner.swift +++ b/SessionUtilitiesKit/JobRunner/JobRunner.swift @@ -1154,6 +1154,9 @@ public final class JobQueue: Hashable { } return } + guard executionType == .concurrent || currentlyRunningJobIds.wrappedValue.isEmpty else { + return SNLog("[JobRunner] \(queueContext) Ignoring 'runNextJob' due to currently running job in serial queue") + } guard let (nextJob, numJobsRemaining): (Job, Int) = pendingJobsQueue.mutate({ queue in queue.popFirst().map { ($0, queue.count) } }) else { // If it's a serial queue, or there are no more jobs running then update the 'isRunning' flag if executionType != .concurrent || currentlyRunningJobIds.wrappedValue.isEmpty {