-
Notifications
You must be signed in to change notification settings - Fork 121
[Local Catalog] Incremental sync: remote sync #16102
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
41c2992
Update catalog sync requests to be `availableAsRESTRequest`.
jaclync 23caef1
Refactor loadAllProducts/loadAllProductVariations with a generic load…
jaclync a802c6e
Create `POSCatalogIncrementalSyncService` with partial persistence te…
jaclync 2e27ade
Add/update test cases for product & relationships persistence.
jaclync c14d9bc
Add test cases for variation & relationships persistence.
jaclync 4e1690c
Remove MockPOSCatalogPersistenceService to keep variables per use case.
jaclync ebb3339
Merge branch 'trunk' into feat/WOOMOB-1097-incremental-sync-service
jaclync 68573ae
Revert persistence changes for incremental sync for a separate PR.
jaclync 9f8289f
Move `POSCatalogSyncRemoteProtocol` mock to a separate file for reuse.
jaclync 1dd2103
Recover parts of incremental sync service test cases.
jaclync File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
Modules/Sources/Yosemite/Tools/POS/BatchedRequestLoader.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import Foundation | ||
|
|
||
| /// Generic utility for loading paginated data with batch processing. | ||
| final class BatchedRequestLoader { | ||
| private let batchSize: Int | ||
|
|
||
| init(batchSize: Int) { | ||
| self.batchSize = batchSize | ||
| } | ||
|
|
||
| /// Loads all items using a paginated request function. | ||
| /// - Parameters: | ||
| /// - makeRequest: Function that takes a page number and returns PagedItems<T>. | ||
| /// - Returns: Array of all loaded items. | ||
| func loadAll<T>(makeRequest: @escaping (Int) async throws -> PagedItems<T>) async throws -> [T] { | ||
| var allItems: [T] = [] | ||
| var currentPage = 1 | ||
| var hasMorePages = true | ||
|
|
||
| while hasMorePages { | ||
| let pagesToFetch = Array(currentPage..<(currentPage + batchSize)) | ||
|
|
||
| let batchResults = try await withThrowingTaskGroup(of: PageResult<T>.self) { group in | ||
| for pageNumber in pagesToFetch { | ||
| group.addTask { | ||
| let result = try await makeRequest(pageNumber) | ||
| return PageResult(pageNumber: pageNumber, items: result) | ||
| } | ||
| } | ||
|
|
||
| var results: [PageResult<T>] = [] | ||
| for try await result in group { | ||
| results.append(result) | ||
| } | ||
| return results.sorted(by: { $0.pageNumber < $1.pageNumber }) | ||
| } | ||
|
|
||
| // Processes results in order and checks if there are more pages. | ||
| let newItems = batchResults.flatMap { $0.items.items } | ||
| allItems.append(contentsOf: newItems) | ||
|
|
||
| let highestPageResult = batchResults.last?.items | ||
| hasMorePages = (highestPageResult?.hasMorePages ?? false) && !newItems.isEmpty | ||
| currentPage += batchSize | ||
| } | ||
|
|
||
| return allItems | ||
| } | ||
| } | ||
|
|
||
| private struct PageResult<T> { | ||
| let pageNumber: Int | ||
| let items: PagedItems<T> | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
84 changes: 84 additions & 0 deletions
84
Modules/Sources/Yosemite/Tools/POS/POSCatalogIncrementalSyncService.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import Foundation | ||
| import protocol Networking.POSCatalogSyncRemoteProtocol | ||
| import class Networking.AlamofireNetwork | ||
| import class Networking.POSCatalogSyncRemote | ||
| import CocoaLumberjackSwift | ||
| import protocol Storage.GRDBManagerProtocol | ||
|
|
||
| // TODO - remove the periphery ignore comment when the service is integrated with POS. | ||
| // periphery:ignore | ||
| public protocol POSCatalogIncrementalSyncServiceProtocol { | ||
| /// Starts an incremental catalog sync process. | ||
| /// - Parameters: | ||
| /// - siteID: The site ID to sync catalog for. | ||
| /// - lastFullSyncDate: The date of the last full sync to use if no incremental sync date exists. | ||
| func startIncrementalSync(for siteID: Int64, lastFullSyncDate: Date) async throws | ||
| } | ||
|
|
||
| // TODO - remove the periphery ignore comment when the service is integrated with POS. | ||
| // periphery:ignore | ||
| public final class POSCatalogIncrementalSyncService: POSCatalogIncrementalSyncServiceProtocol { | ||
| private let syncRemote: POSCatalogSyncRemoteProtocol | ||
| private let batchSize: Int | ||
| private var lastIncrementalSyncDates: [Int64: Date] = [:] | ||
| private let batchedLoader: BatchedRequestLoader | ||
|
|
||
| public convenience init?(credentials: Credentials?, batchSize: Int = 1) { | ||
| guard let credentials else { | ||
| DDLogError("⛔️ Could not create POSCatalogIncrementalSyncService due missing credentials") | ||
| return nil | ||
| } | ||
| let network = AlamofireNetwork(credentials: credentials, ensuresSessionManagerIsInitialized: true) | ||
| let syncRemote = POSCatalogSyncRemote(network: network) | ||
| self.init(syncRemote: syncRemote, batchSize: batchSize) | ||
| } | ||
|
|
||
| init(syncRemote: POSCatalogSyncRemoteProtocol, batchSize: Int) { | ||
| self.syncRemote = syncRemote | ||
| self.batchSize = batchSize | ||
| self.batchedLoader = BatchedRequestLoader(batchSize: batchSize) | ||
| } | ||
|
|
||
| // MARK: - Protocol Conformance | ||
|
|
||
| public func startIncrementalSync(for siteID: Int64, lastFullSyncDate: Date) async throws { | ||
| let modifiedAfter = lastIncrementalSyncDates[siteID] ?? lastFullSyncDate | ||
|
|
||
| DDLogInfo("🔄 Starting incremental catalog sync for site ID: \(siteID), modifiedAfter: \(modifiedAfter)") | ||
|
|
||
| do { | ||
| let syncStartDate = Date() | ||
| let catalog = try await loadCatalog(for: siteID, modifiedAfter: modifiedAfter, syncRemote: syncRemote) | ||
| DDLogInfo("✅ Loaded \(catalog.products.count) products and \(catalog.variations.count) variations for siteID \(siteID)") | ||
|
|
||
| // TODO: WOOMOB-1298 - persist to database | ||
|
|
||
| // TODO: WOOMOB-1289 - replace with store settings persistence | ||
| lastIncrementalSyncDates[siteID] = syncStartDate | ||
| DDLogInfo("✅ Updated last incremental sync date to \(syncStartDate) for siteID \(siteID)") | ||
| } catch { | ||
| DDLogError("❌ Failed to sync and persist catalog incrementally: \(error)") | ||
| throw error | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Remote Loading | ||
|
|
||
| private extension POSCatalogIncrementalSyncService { | ||
| func loadCatalog(for siteID: Int64, modifiedAfter: Date, syncRemote: POSCatalogSyncRemoteProtocol) async throws -> POSCatalog { | ||
| async let productsTask = batchedLoader.loadAll( | ||
| makeRequest: { pageNumber in | ||
| try await syncRemote.loadProducts(modifiedAfter: modifiedAfter, siteID: siteID, pageNumber: pageNumber) | ||
| } | ||
| ) | ||
| async let variationsTask = batchedLoader.loadAll( | ||
| makeRequest: { pageNumber in | ||
| try await syncRemote.loadProductVariations(modifiedAfter: modifiedAfter, siteID: siteID, pageNumber: pageNumber) | ||
| } | ||
| ) | ||
|
|
||
| let (products, variations) = try await (productsTask, variationsTask) | ||
| return POSCatalog(products: products, variations: variations) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Adding
availableAsRESTRequest: trueas an oversight from the previous remote implementation.