-
Notifications
You must be signed in to change notification settings - Fork 121
[Local Catalog] Persist catalog downloads in the background #16342
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 6 commits
fabb1de
a93899e
be0cf24
5046bfd
f350297
db3f9d4
5d29062
06ae494
bc5c1bf
fa75864
d0558a2
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,55 @@ | ||
| import Foundation | ||
| import CocoaLumberjackSwift | ||
|
|
||
| /// Coordinates background catalog downloads, including handling app wake events. | ||
| public class BackgroundCatalogDownloadCoordinator { | ||
| private let backgroundDownloader: BackgroundDownloadProtocol | ||
|
|
||
| public init(backgroundDownloader: BackgroundDownloadProtocol = BackgroundDownloadService()) { | ||
| self.backgroundDownloader = backgroundDownloader | ||
| } | ||
|
|
||
| /// Handles a background URLSession wake event. | ||
| /// Called from AppDelegate when iOS wakes the app for a completed download. | ||
| /// - Parameters: | ||
| /// - sessionIdentifier: The session identifier from the callback | ||
| /// - completionHandler: Completion handler to call when processing is done | ||
| /// - parseHandler: Closure to parse and persist the downloaded file | ||
| public func handleBackgroundSessionEvent( | ||
| sessionIdentifier: String, | ||
| completionHandler: @escaping () -> Void, | ||
| parseHandler: @escaping (URL, Int64) async throws -> Void | ||
| ) async { | ||
| DDLogInfo("π£ Handling background session event for: \(sessionIdentifier)") | ||
|
|
||
| // Load the saved download state to know which site this is for | ||
| guard let state = BackgroundDownloadState.load(for: sessionIdentifier) else { | ||
| DDLogError("βοΈ No saved state found for background download session: \(sessionIdentifier)") | ||
| completionHandler() | ||
| return | ||
| } | ||
|
|
||
| // Reconnect to the background session and get the downloaded file | ||
| guard let fileURL = await backgroundDownloader.reconnectToSession(identifier: sessionIdentifier, | ||
| allowCellular: true, | ||
| completionHandler: completionHandler) else { | ||
| DDLogError("βοΈ Failed to reconnect to background download session") | ||
| BackgroundDownloadState.clear() | ||
| return | ||
| } | ||
|
|
||
| DDLogInfo("π£ Background download file ready at: \(fileURL.path)") | ||
|
|
||
| // Parse the catalog file in this background window (~30 seconds) | ||
| // TODO: WOOMOB-1677 - For very large catalogs, consider hybrid approach: try immediate parse, defer if timeout. | ||
| do { | ||
| try await parseHandler(fileURL, state.siteID) | ||
| DDLogInfo("β Background catalog processing completed successfully") | ||
| } catch { | ||
| DDLogError("βοΈ Failed to process catalog in background: \(error)") | ||
| } | ||
|
|
||
| // Clean up state | ||
| BackgroundDownloadState.clear() | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import Foundation | ||
|
|
||
| /// Persisted state for background catalog downloads. | ||
| /// Allows the app to resume processing downloads after being terminated. | ||
| public struct BackgroundDownloadState: Codable { | ||
| let sessionIdentifier: String | ||
| let siteID: Int64 | ||
|
|
||
| private static let userDefaultsKey = "com.woocommerce.pos.backgroundDownloadState" | ||
|
|
||
| /// Saves download state for later retrieval. | ||
| public static func save(_ state: BackgroundDownloadState) { | ||
| let encoder = JSONEncoder() | ||
| if let encoded = try? encoder.encode(state) { | ||
| UserDefaults.standard.set(encoded, forKey: userDefaultsKey) | ||
| } | ||
| } | ||
|
|
||
| /// Loads saved download state for a specific session identifier. | ||
| public static func load(for sessionIdentifier: String) -> BackgroundDownloadState? { | ||
| guard let data = UserDefaults.standard.data(forKey: userDefaultsKey), | ||
| let state = try? JSONDecoder().decode(BackgroundDownloadState.self, from: data), | ||
| state.sessionIdentifier == sessionIdentifier else { | ||
| return nil | ||
| } | ||
| return state | ||
| } | ||
|
|
||
| /// Clears saved download state. | ||
| public static func clear() { | ||
| UserDefaults.standard.removeObject(forKey: userDefaultsKey) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,13 +16,20 @@ final class POSSettingsLocalCatalogViewModel { | |
| private let catalogSettingsService: POSCatalogSettingsServiceProtocol | ||
| private let catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol | ||
| private let siteSettings: SiteSpecificAppSettingsStoreMethodsProtocol | ||
| private let syncStateModel: POSCatalogSyncStateModel | ||
| private let dateFormatter: RelativeDateTimeFormatter = { | ||
| let formatter = RelativeDateTimeFormatter() | ||
| formatter.dateTimeStyle = .named | ||
| formatter.unitsStyle = .full | ||
| return formatter | ||
| }() | ||
|
|
||
| private var syncStateObservationTask: Task<Void, Never>? | ||
|
|
||
| private var currentSyncState: POSCatalogSyncState { | ||
| syncStateModel.state[siteID] ?? .syncNeverDone(siteID: siteID) | ||
| } | ||
|
|
||
| var allowFullSyncOnCellular: Bool { | ||
| get { | ||
| siteSettings.getPOSLocalCatalogCellularDataAllowed(siteID: siteID) | ||
|
|
@@ -39,7 +46,15 @@ final class POSSettingsLocalCatalogViewModel { | |
| self.siteID = siteID | ||
| self.catalogSettingsService = catalogSettingsService | ||
| self.catalogSyncCoordinator = catalogSyncCoordinator | ||
| self.syncStateModel = catalogSyncCoordinator.fullSyncStateModel | ||
| self.siteSettings = siteSettings ?? SiteSpecificAppSettingsStoreMethods(fileStorage: PListFileStorage()) | ||
|
|
||
| // Observe sync state changes to update UI when sync completes in background | ||
| startObservingSyncState() | ||
| } | ||
|
|
||
| deinit { | ||
| syncStateObservationTask?.cancel() | ||
| } | ||
|
|
||
| @MainActor | ||
|
|
@@ -63,13 +78,56 @@ final class POSSettingsLocalCatalogViewModel { | |
| @MainActor | ||
| func refreshCatalog() async { | ||
| isRefreshingCatalog = true | ||
| defer { isRefreshingCatalog = false } | ||
|
|
||
| do { | ||
| try await catalogSyncCoordinator.performFullSync(for: siteID, regenerateCatalog: true) | ||
| // Sync completed synchronously - update UI | ||
| isRefreshingCatalog = false | ||
| await loadCatalogData() | ||
| } catch { | ||
| DDLogError("βοΈ POSSettingsLocalCatalog: Failed to refresh catalog: \(error)") | ||
| isRefreshingCatalog = false | ||
| } | ||
| } | ||
|
|
||
| /// Starts observing sync state changes to update UI when sync completes in background | ||
| private func startObservingSyncState() { | ||
| syncStateObservationTask = Task { @MainActor in | ||
| var previousState = currentSyncState | ||
|
|
||
| while !Task.isCancelled { | ||
| // Use withObservationTracking to detect changes | ||
| await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in | ||
| withObservationTracking { | ||
| // Access the observed property to register observation | ||
| _ = currentSyncState | ||
| } onChange: { | ||
| // When state changes, resume the continuation | ||
| continuation.resume() | ||
| } | ||
| } | ||
|
|
||
| // Check if state actually changed | ||
| let newState = currentSyncState | ||
| if newState != previousState { | ||
| switch newState { | ||
| case .syncCompleted, .syncFailed, .initialSyncFailed: | ||
| // Sync finished - clear the refreshing state if it was set | ||
| if isRefreshingCatalog { | ||
|
||
| isRefreshingCatalog = false | ||
| // Reload catalog data to show updated info | ||
| await loadCatalogData() | ||
| } | ||
| case .syncStarted, .initialSyncStarted: | ||
| // Sync is running - keep spinner active | ||
| break | ||
| case .syncNeverDone: | ||
| // No sync has been done | ||
| break | ||
| } | ||
| previousState = newState | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,13 @@ public protocol POSCatalogFullSyncServiceProtocol { | |
| /// - allowCellular: Should cellular data be used if required. | ||
| /// - Returns: The synced catalog containing products and variations | ||
| func startFullSync(for siteID: Int64, regenerateCatalog: Bool, allowCellular: Bool) async throws -> POSCatalog | ||
|
|
||
| /// Parses and persists a downloaded catalog file from a background download. | ||
| /// - Parameters: | ||
| /// - fileURL: Local file URL of the downloaded catalog | ||
| /// - siteID: Site ID for this catalog | ||
| /// - Returns: The parsed catalog | ||
| func parseAndPersistBackgroundDownload(fileURL: URL, siteID: Int64) async throws -> POSCatalog | ||
| } | ||
|
|
||
| /// POS catalog from full sync. | ||
|
|
@@ -200,4 +207,25 @@ private extension POSCatalogFullSyncService { | |
|
|
||
| throw POSCatalogSyncError.timeout | ||
| } | ||
|
|
||
| public func parseAndPersistBackgroundDownload(fileURL: URL, siteID: Int64) async throws -> POSCatalog { | ||
|
||
| DDLogInfo("π£ Parsing background catalog download for site \(siteID)") | ||
|
|
||
| let syncStartDate = Date.now | ||
| let catalogResponse = try await syncRemote.parseDownloadedCatalog(from: fileURL, siteID: siteID) | ||
|
|
||
| let catalog = POSCatalog( | ||
| products: catalogResponse.products, | ||
| variations: catalogResponse.variations, | ||
| syncDate: syncStartDate | ||
| ) | ||
|
|
||
| DDLogInfo("β Loaded \(catalog.products.count) products and \(catalog.variations.count) variations for siteID \(siteID)") | ||
|
|
||
| // Persist to database | ||
| try await persistenceService.replaceAllCatalogData(catalog, siteID: siteID) | ||
| DDLogInfo("β Persisted \(catalog.products.count) products and \(catalog.variations.count) variations to database for siteID \(siteID)") | ||
|
|
||
| return catalog | ||
| } | ||
| } | ||
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.
This is a bit tricky to wrap my head around, I have a question: When we setup the observation through
withCheckedContinuation, and then becomes inactive before reading the new state (let newState = currentSyncState)... could there be state changes that are missed before we loop back to the beginning of the loop?It was my understanding that when
.onChangefires we must callwithObservationTrackingagain to re-register, so the gap between these calls is not observed. But this could be just a miss-understanding on my part, since the signature does not return any cancellable/subscription object but is just a closure.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.
Yeah, it's a little odd. The while loop takes care of re-upping the observation tracking, but I've changed it to make that a little clearer.