-
Notifications
You must be signed in to change notification settings - Fork 121
[Woo POS][Local Catalog] Add a periodic sync when the app is in the foreground #16246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
3b54605
a453b17
2896586
ae450dc
30c82b7
b015712
3be4a8b
2dd0372
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,213 @@ | ||
| import Foundation | ||
| import UIKit | ||
| import CocoaLumberjackSwift | ||
| import Yosemite | ||
| import Experiments | ||
|
|
||
| /// Periodically syncs POS catalog while the app is in the foreground. | ||
| /// Triggers `catalogCoordinator.performSmartSync()` every hour when active. | ||
| /// | ||
| final class ForegroundPOSCatalogSyncDispatcher { | ||
| private enum Constants { | ||
| static let syncInterval: TimeInterval = 60 * 60 // 1 hour | ||
| static let initialSyncDelay: TimeInterval = 5 // 5 seconds after becoming active | ||
| static let leeway: Int = 5 // 5 seconds leeway to give a system more flexibility in managing resources | ||
| } | ||
|
|
||
| private let interval: TimeInterval | ||
| private let notificationCenter: NotificationCenter | ||
| private let timerProvider: DispatchTimerProviding | ||
| private let featureFlagService: FeatureFlagService | ||
| private let stores: StoresManager | ||
| private let isAppActive: () -> Bool | ||
| private var observers: [NSObjectProtocol] = [] | ||
| private var timer: DispatchTimerProtocol? | ||
|
|
||
| private var isRunning: Bool { | ||
| syncSiteID != nil | ||
| } | ||
| private var syncSiteID: Int64? | ||
|
|
||
| init(interval: TimeInterval = Constants.syncInterval, | ||
| notificationCenter: NotificationCenter = .default, | ||
| timerProvider: DispatchTimerProviding = DefaultDispatchTimerProvider(), | ||
| featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, | ||
| stores: StoresManager = ServiceLocator.stores, | ||
| isAppActive: @escaping () -> Bool = { UIApplication.shared.applicationState == .active }) { | ||
| self.interval = interval | ||
| self.notificationCenter = notificationCenter | ||
| self.timerProvider = timerProvider | ||
| self.featureFlagService = featureFlagService | ||
| self.stores = stores | ||
| self.isAppActive = isAppActive | ||
| } | ||
|
|
||
| deinit { | ||
| stop() | ||
| } | ||
|
|
||
| func start() { | ||
| guard featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1) else { | ||
| return | ||
| } | ||
|
|
||
| if syncSiteID != stores.sessionManager.defaultStoreID { | ||
| DDLogInfo("🔄 ForegroundPOSCatalogSyncDispatcher: Site has changed, resetting the sync") | ||
| stop() | ||
| } | ||
|
|
||
| guard !isRunning else { return } | ||
|
|
||
| DDLogInfo("🔄 ForegroundPOSCatalogSyncDispatcher: Starting foreground sync dispatcher") | ||
|
|
||
| let activeObserver = notificationCenter.addObserver( | ||
| forName: UIApplication.didBecomeActiveNotification, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] _ in | ||
| self?.startTimer() | ||
| } | ||
|
|
||
| let backgroundObserver = notificationCenter.addObserver( | ||
| forName: UIApplication.didEnterBackgroundNotification, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] _ in | ||
| self?.stopTimer() | ||
| } | ||
|
|
||
| let defaultSiteObserver = notificationCenter.addObserver( | ||
| forName: .StoresManagerDidUpdateDefaultSite, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] _ in | ||
| self?.stop() | ||
| } | ||
|
|
||
| observers = [activeObserver, backgroundObserver, defaultSiteObserver] | ||
|
|
||
| if isAppActive() { | ||
| startTimer() | ||
| } | ||
|
|
||
| syncSiteID = stores.sessionManager.defaultStoreID | ||
| } | ||
|
|
||
| func stop() { | ||
| guard isRunning else { return } | ||
|
|
||
| DDLogInfo("🛑 ForegroundPOSCatalogSyncDispatcher: Stopping foreground sync dispatcher") | ||
| observers.forEach(notificationCenter.removeObserver) | ||
| observers.removeAll() | ||
| stopTimer() | ||
| syncSiteID = nil | ||
| } | ||
|
|
||
| private func startTimer() { | ||
| guard timer == nil else { return } | ||
|
|
||
| DDLogInfo("⏱️ ForegroundPOSCatalogSyncDispatcher: Starting timer (interval: \(Int(interval))s, initial delay: \(Int(Constants.initialSyncDelay))s)") | ||
|
|
||
| let queue = DispatchQueue(label: "com.automattic.woocommerce.posCatalogForegroundSync.timer", qos: .utility) | ||
| let timer = timerProvider.makeTimer(queue: queue) | ||
| timer.schedule(deadline: .now() + Constants.initialSyncDelay, repeating: interval, leeway: .seconds(Constants.leeway)) | ||
| timer.setEventHandler { [weak self] in | ||
| self?.performSync() | ||
| } | ||
| timer.resume() | ||
| self.timer = timer | ||
| } | ||
|
|
||
| private func stopTimer() { | ||
| guard timer != nil else { return } | ||
| DDLogInfo("⏱️ ForegroundPOSCatalogSyncDispatcher: Stopping timer") | ||
| timer?.cancel() | ||
| timer = nil | ||
| } | ||
|
|
||
| private func performSync() { | ||
| guard featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1) else { | ||
| DDLogInfo("📋 ForegroundPOSCatalogSyncDispatcher: Feature flag disabled, skipping sync") | ||
| stop() | ||
| return | ||
| } | ||
|
|
||
| guard let siteID = stores.sessionManager.defaultStoreID else { | ||
| DDLogInfo("📋 ForegroundPOSCatalogSyncDispatcher: No default store, skipping sync") | ||
| stop() | ||
| return | ||
| } | ||
|
|
||
| guard let coordinator = stores.posCatalogSyncCoordinator else { | ||
| DDLogInfo("📋 ForegroundPOSCatalogSyncDispatcher: Coordinator unavailable, skipping sync") | ||
| stop() | ||
| return | ||
| } | ||
|
|
||
| DDLogInfo("🔄 ForegroundPOSCatalogSyncDispatcher: Starting sync for site \(siteID)") | ||
|
|
||
| Task.detached(priority: .utility) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good use of detached and priority queue! |
||
| do { | ||
| try await coordinator.performSmartSync(for: siteID) | ||
| DDLogInfo("✅ ForegroundPOSCatalogSyncDispatcher: Sync completed for site \(siteID)") | ||
| } catch let error as POSCatalogSyncError { | ||
| switch error { | ||
| case .syncAlreadyInProgress: | ||
| DDLogInfo("ℹ️ ForegroundPOSCatalogSyncDispatcher: Sync already in progress for site \(siteID)") | ||
| case .negativeMaxAge: | ||
| DDLogError("⛔️ ForegroundPOSCatalogSyncDispatcher: Invalid max age for site \(siteID)") | ||
| } | ||
| } catch { | ||
| DDLogError("⛔️ ForegroundPOSCatalogSyncDispatcher: Sync failed for site \(siteID): \(error)") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Helpers | ||
|
|
||
| /// Simplified protocol for dispatch source timers, only exposing methods actually used. | ||
| protocol DispatchTimerProtocol { | ||
| func schedule(deadline: DispatchTime, repeating: Double, leeway: DispatchTimeInterval) | ||
| func setEventHandler(handler: (() -> Void)?) | ||
| func resume() | ||
| func cancel() | ||
| } | ||
|
|
||
| extension DispatchSourceTimer { | ||
| func asTimer() -> DispatchTimerProtocol { | ||
| DispatchTimerWrapper(timer: self) | ||
| } | ||
| } | ||
|
|
||
| private struct DispatchTimerWrapper: DispatchTimerProtocol { | ||
| let timer: DispatchSourceTimer | ||
|
|
||
| func schedule(deadline: DispatchTime, repeating: Double, leeway: DispatchTimeInterval) { | ||
| timer.schedule(deadline: deadline, repeating: repeating, leeway: leeway) | ||
| } | ||
|
|
||
| func setEventHandler(handler: (() -> Void)?) { | ||
| timer.setEventHandler(handler: handler) | ||
| } | ||
|
|
||
| func resume() { | ||
| timer.resume() | ||
| } | ||
|
|
||
| func cancel() { | ||
| timer.cancel() | ||
| } | ||
| } | ||
|
|
||
| /// Protocol for creating dispatch timers, enabling testability. | ||
| protocol DispatchTimerProviding { | ||
| func makeTimer(queue: DispatchQueue) -> DispatchTimerProtocol | ||
| } | ||
|
|
||
| /// Default implementation using system DispatchSource. | ||
| struct DefaultDispatchTimerProvider: DispatchTimerProviding { | ||
| func makeTimer(queue: DispatchQueue) -> DispatchTimerProtocol { | ||
| DispatchSource.makeTimerSource(queue: queue).asTimer() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,7 +151,7 @@ final class MainTabBarController: UITabBarController { | |
|
|
||
| private var posTabVisibilityChecker: POSTabVisibilityCheckerProtocol? | ||
| private var posEligibilityCheckTask: Task<Void, Never>? | ||
| private var posCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol? | ||
| private lazy var posSyncDispatcher = ForegroundPOSCatalogSyncDispatcher() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing this needs to be lazy because the POS tab visibility and eligibility? Otherwise we would be paying for initializing the sync dispatcher despite the merchant not being able to use POS. |
||
|
|
||
| /// periphery: ignore - keeping strong ref of the checker to keep its async task alive | ||
| private var bookingsEligibilityChecker: BookingsTabEligibilityCheckerProtocol? | ||
|
|
@@ -721,10 +721,8 @@ private extension MainTabBarController { | |
| updateTabViewControllers(isPOSTabVisible: isPOSTabVisible, isBookingsTabVisible: isBookingsTabVisible) | ||
| viewModel.loadHubMenuTabBadge() | ||
|
|
||
| // Trigger POS catalog sync if tab is visible and feature flag is enabled | ||
| if isPOSTabVisible, ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1) { | ||
| await triggerPOSCatalogSyncIfNeeded(for: siteID) | ||
| } | ||
| // Begin foreground synchronization if POS tab becomes visible | ||
| isPOSTabVisible ? posSyncDispatcher.start() : posSyncDispatcher.stop() | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -817,10 +815,6 @@ private extension MainTabBarController { | |
| navigateToContent: { _ in })] | ||
| } | ||
|
|
||
| // Configure POS catalog sync coordinator for local catalog syncing | ||
| // Get POS catalog sync coordinator (will be nil if feature flag disabled or not authenticated) | ||
| posCatalogSyncCoordinator = ServiceLocator.posCatalogSyncCoordinator | ||
|
|
||
| // Configure hub menu tab coordinator once per logged in session potentially with multiple sites. | ||
| if hubMenuTabCoordinator == nil { | ||
| let hubTabCoordinator = createHubMenuTabCoordinator() | ||
|
|
@@ -852,26 +846,6 @@ private extension MainTabBarController { | |
| OrdersSplitViewWrapperController(siteID: siteID) | ||
| } | ||
|
|
||
| func triggerPOSCatalogSyncIfNeeded(for siteID: Int64) async { | ||
| guard let coordinator = posCatalogSyncCoordinator else { | ||
| return | ||
| } | ||
|
|
||
| // Check if sync is needed (older than 24 hours) | ||
| let maxAge: TimeInterval = 24 * 60 * 60 | ||
|
|
||
| // Perform background sync | ||
| Task.detached { | ||
| do { | ||
| _ = try await coordinator.performFullSyncIfApplicable(for: siteID, maxAge: maxAge) | ||
| } catch POSCatalogSyncError.syncAlreadyInProgress { | ||
| DDLogInfo("ℹ️ POS catalog sync already in progress for site \(siteID), skipping") | ||
| } catch { | ||
| DDLogError("⚠️ POS catalog sync failed: \(error)") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func createBookingsViewController(siteID: Int64) -> UIViewController { | ||
| BookingsTabViewHostingController(siteID: siteID) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider if passing the
siteIDin the DDLog here would be useful: When I switched from a POS-eligible store to a non-eligible one the only notice logged was that the timer stopped. Perhaps not necessary as we would see the siteID changing from context of other events.