From de3d16f7f26b9b8edeb1b938d6d622505ba3b3cd Mon Sep 17 00:00:00 2001 From: RafaelKayumov Date: Fri, 5 Sep 2025 13:43:40 +0300 Subject: [PATCH 01/12] Add `CIABEligibilityChecker` placeholder --- .../Classes/CIAB/CIABAffectedFeature.swift | 22 ++++++++++ .../Classes/CIAB/CIABEligibilityChecker.swift | 44 +++++++++++++++++++ .../WooCommerce.xcodeproj/project.pbxproj | 16 +++++++ 3 files changed, 82 insertions(+) create mode 100644 WooCommerce/Classes/CIAB/CIABAffectedFeature.swift create mode 100644 WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift diff --git a/WooCommerce/Classes/CIAB/CIABAffectedFeature.swift b/WooCommerce/Classes/CIAB/CIABAffectedFeature.swift new file mode 100644 index 00000000000..1bd6fe1195c --- /dev/null +++ b/WooCommerce/Classes/CIAB/CIABAffectedFeature.swift @@ -0,0 +1,22 @@ +/// Describes the feature set affected by CIAB sites +/// By the moment of introduction the features aren't supported by CIAB sites + +/// periphery: ignore:all - Used through `.allCases` +enum CIABAffectedFeature: CaseIterable { + case blaze + case payments + case splitShipments + case groupedProducts + case variableProducts + case giftCardEditing + case productsStockDashboardCard +} + +extension CIABAffectedFeature { + /// Defines a collection of existing app features unsupported in CIAB sites + /// In case if a certain feature is reconsidered and decided to be supported in CIAB + /// just remove it from the collection + static var unsupportedFeatures: [CIABAffectedFeature] { + return CIABAffectedFeature.allCases + } +} diff --git a/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift new file mode 100644 index 00000000000..7f0b2990aba --- /dev/null +++ b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift @@ -0,0 +1,44 @@ +/// periphery: ignore:all - Will be used in upcoming PRs + +import Foundation +import Yosemite + +protocol CIABEligibilityCheckerProtocol { + var isCurrentSiteCIAB: Bool { get } + + func isSiteCIAB(_ site: Site) -> Bool + + func isFeatureSupportedForCurrentSite(_ feature: CIABAffectedFeature) -> Bool + func isFeatureSupported(_ feature: CIABAffectedFeature, for site: Site) -> Bool +} + +final class CIABEligibilityChecker { + private let stores: StoresManager + + init(stores: StoresManager = ServiceLocator.stores) { + self.stores = stores + } +} + +extension CIABEligibilityChecker: CIABEligibilityCheckerProtocol { + var isCurrentSiteCIAB: Bool { + /// Temp mocked value + return true + } + + func isSiteCIAB(_ site: Site) -> Bool { + /// Temp mocked value + return true + } + + func isFeatureSupportedForCurrentSite(_ feature: CIABAffectedFeature) -> Bool { + return !isCurrentSiteCIAB || !CIABAffectedFeature.unsupportedFeatures.contains(feature) + } + + func isFeatureSupported( + _ feature: CIABAffectedFeature, + for site: Site + ) -> Bool { + return !isSiteCIAB(site) || !CIABAffectedFeature.unsupportedFeatures.contains(feature) + } +} diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index d4695dc890f..3649e8afdc3 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -1243,6 +1243,8 @@ 2DB891692E27F6200001B175 /* OrderListCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB891682E27F61C0001B175 /* OrderListCellViewModel.swift */; }; 2DB8916B2E27F6D90001B175 /* OrderListCellViewModel+Localizations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB8916A2E27F6CE0001B175 /* OrderListCellViewModel+Localizations.swift */; }; 2DB8916E2E27F7840001B175 /* OrderListCellViewModel+Localizations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB8916A2E27F6CE0001B175 /* OrderListCellViewModel+Localizations.swift */; }; + 2DCB54FA2E6AE8E100621F90 /* CIABEligibilityChecker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DCB54F92E6AE8D800621F90 /* CIABEligibilityChecker.swift */; }; + 2DCB54FC2E6AFE6A00621F90 /* CIABAffectedFeature.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DCB54FB2E6AFE6900621F90 /* CIABAffectedFeature.swift */; }; 2DF0D1BC2E2907C100F8995C /* MarkOrderAsReadUseCase+Woo.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DB88DA32E27DD790001B175 /* MarkOrderAsReadUseCase+Woo.swift */; }; 310D1B482734919E001D55B4 /* InPersonPaymentsLiveSiteInTestModeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 310D1B472734919E001D55B4 /* InPersonPaymentsLiveSiteInTestModeView.swift */; }; 311237EE2714DA240033C44E /* CardPresentModalDisplayMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 311237ED2714DA240033C44E /* CardPresentModalDisplayMessage.swift */; }; @@ -4430,6 +4432,8 @@ 2DB891652E27F07E0001B175 /* Address+Shared.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Address+Shared.swift"; sourceTree = ""; }; 2DB891682E27F61C0001B175 /* OrderListCellViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OrderListCellViewModel.swift; sourceTree = ""; }; 2DB8916A2E27F6CE0001B175 /* OrderListCellViewModel+Localizations.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OrderListCellViewModel+Localizations.swift"; sourceTree = ""; }; + 2DCB54F92E6AE8D800621F90 /* CIABEligibilityChecker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CIABEligibilityChecker.swift; sourceTree = ""; }; + 2DCB54FB2E6AFE6900621F90 /* CIABAffectedFeature.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CIABAffectedFeature.swift; sourceTree = ""; }; 310D1B472734919E001D55B4 /* InPersonPaymentsLiveSiteInTestModeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InPersonPaymentsLiveSiteInTestModeView.swift; sourceTree = ""; }; 311237ED2714DA240033C44E /* CardPresentModalDisplayMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CardPresentModalDisplayMessage.swift; sourceTree = ""; }; 311D21E7264AEDB900102316 /* CardPresentModalScanningForReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CardPresentModalScanningForReader.swift; sourceTree = ""; }; @@ -9051,6 +9055,15 @@ path = Images; sourceTree = ""; }; + 2DCB54F82E6AE8C900621F90 /* CIAB */ = { + isa = PBXGroup; + children = ( + 2DCB54FB2E6AFE6900621F90 /* CIABAffectedFeature.swift */, + 2DCB54F92E6AE8D800621F90 /* CIABEligibilityChecker.swift */, + ); + path = CIAB; + sourceTree = ""; + }; 318853452639FE7F00F66A9C /* CardReadersV2 */ = { isa = PBXGroup; children = ( @@ -10870,6 +10883,7 @@ B56DB3F12049C0B800D4AA8E /* Classes */ = { isa = PBXGroup; children = ( + 2DCB54F82E6AE8C900621F90 /* CIAB */, DEB387932C2E7A540025256E /* GoogleAds */, 20AE33C32B0510AD00527B60 /* Destinations */, B9F3DAAB29BB714900DDD545 /* App Intents */, @@ -15342,6 +15356,7 @@ 029D025E2C231F2A00CB1E75 /* PointOfSaleCardPresentPaymentOptionalReaderUpdateInProgressView.swift in Sources */, B95A45E92A77AE2C0073A91F /* CustomerSelectorViewModel.swift in Sources */, 024DF3072372C18D006658FE /* AztecUIConfigurator.swift in Sources */, + 2DCB54FC2E6AFE6A00621F90 /* CIABAffectedFeature.swift in Sources */, EE1B07F32C81CB4B006D9769 /* BlazeLocalNotificationScheduler.swift in Sources */, DE02ABBE2B578D0E008E0AC4 /* CreditCardType.swift in Sources */, 020BE74823B05CF2007FE54C /* ProductInventoryEditableData.swift in Sources */, @@ -16235,6 +16250,7 @@ 029F29FA24D93E9E004751CA /* EditableProductModel.swift in Sources */, 027179E22C08817F0049F0BD /* CardPresentPaymentService.swift in Sources */, 20D3D4332C65E59B004CE6E3 /* OrdersRoute.swift in Sources */, + 2DCB54FA2E6AE8E100621F90 /* CIABEligibilityChecker.swift in Sources */, 31FC8CE927B476BA004B9456 /* CardReaderSettingsResultsControllers.swift in Sources */, 022266BC2AE7707000614F34 /* ConfigurableBundleItemViewModel.swift in Sources */, D449C52926DFBCCC00D75B02 /* WhatsNewHostingController.swift in Sources */, From d8ff294e4fd504cf9db7929b1a6588a18dddcb1d Mon Sep 17 00:00:00 2001 From: RafaelKayumov Date: Tue, 9 Sep 2025 15:06:13 +0300 Subject: [PATCH 02/12] Use temporary site name based logic to detect garden/ciab site --- .../Classes/CIAB/CIABEligibilityChecker.swift | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift index 7f0b2990aba..808d6469c78 100644 --- a/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift +++ b/WooCommerce/Classes/CIAB/CIABEligibilityChecker.swift @@ -22,13 +22,18 @@ final class CIABEligibilityChecker { extension CIABEligibilityChecker: CIABEligibilityCheckerProtocol { var isCurrentSiteCIAB: Bool { - /// Temp mocked value - return true + guard let currentSite = stores.sessionManager.defaultSite else { + return false + } + return isSiteCIAB(currentSite) } func isSiteCIAB(_ site: Site) -> Bool { - /// Temp mocked value - return true + /// Temp logic + /// If site name contains either `garden` or `ciab` then it's considered a CIAB site + return isCIABSupportedForBuildEnvironment && CIABUnlockingSiteNameSubstrings.allCases.contains { + site.name.lowercased().contains($0.rawValue) + } } func isFeatureSupportedForCurrentSite(_ feature: CIABAffectedFeature) -> Bool { @@ -42,3 +47,23 @@ extension CIABEligibilityChecker: CIABEligibilityCheckerProtocol { return !isSiteCIAB(site) || !CIABAffectedFeature.unsupportedFeatures.contains(feature) } } + +// MARK: - Temporary constants for CIAB identifying logic + +fileprivate extension CIABEligibilityChecker { + enum CIABUnlockingSiteNameSubstrings: String, CaseIterable { + case garden + case ciab + } +} + +// MARK: - Temporary environment checks + +import enum WooFoundationCore.BuildConfiguration + +private extension CIABEligibilityChecker { + var isCIABSupportedForBuildEnvironment: Bool { + let buildConfig = BuildConfiguration.current + return buildConfig == .localDeveloper || buildConfig == .alpha + } +} From 90d138e9c5cfba7f57f2fdc0e8ae55b1cc463932 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 16:16:00 +0100 Subject: [PATCH 03/12] Add full sync date to PersistedSite --- Modules/Sources/Fakes/Yosemite.generated.swift | 3 ++- .../Storage/GRDB/Migrations/V001InitialSchema.swift | 1 + Modules/Sources/Storage/GRDB/Model/PersistedSite.swift | 8 +++++++- .../Model/Copiable/Models+Copiable.generated.swift | 7 +++++-- .../Model/Storage/PersistedSite+Conversions.swift | 6 ++++-- Modules/Sources/Yosemite/PointOfSale/POSSite.swift | 4 +++- 6 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Modules/Sources/Fakes/Yosemite.generated.swift b/Modules/Sources/Fakes/Yosemite.generated.swift index a4a74615933..9f4ef148c6b 100644 --- a/Modules/Sources/Fakes/Yosemite.generated.swift +++ b/Modules/Sources/Fakes/Yosemite.generated.swift @@ -58,7 +58,8 @@ extension Yosemite.POSSite { public static func fake() -> Yosemite.POSSite { .init( siteID: .fake(), - lastIncrementalSyncDate: .fake() + lastIncrementalSyncDate: .fake(), + lastFullSyncDate: .fake() ) } } diff --git a/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift b/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift index c951cde5b67..46e179696ea 100644 --- a/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift +++ b/Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift @@ -20,6 +20,7 @@ struct V001InitialSchema { try db.create(table: "site") { siteTable in siteTable.primaryKey("id", .integer).notNull() siteTable.column("lastCatalogIncrementalSyncDate", .datetime) + siteTable.column("lastCatalogFullSyncDate", .datetime) } } diff --git a/Modules/Sources/Storage/GRDB/Model/PersistedSite.swift b/Modules/Sources/Storage/GRDB/Model/PersistedSite.swift index ee54b52e88c..c35d01559a9 100644 --- a/Modules/Sources/Storage/GRDB/Model/PersistedSite.swift +++ b/Modules/Sources/Storage/GRDB/Model/PersistedSite.swift @@ -7,11 +7,14 @@ public struct PersistedSite: Codable { public let id: Int64 // periphery:ignore - TODO: remove ignore when populating database public let lastCatalogIncrementalSyncDate: Date? + // periphery:ignore - TODO: remove ignore when populating database + public let lastCatalogFullSyncDate: Date? // periphery:ignore - TODO: remove ignore when populating database - public init(id: Int64, lastCatalogIncrementalSyncDate: Date? = nil) { + public init(id: Int64, lastCatalogIncrementalSyncDate: Date? = nil, lastCatalogFullSyncDate: Date? = nil) { self.id = id self.lastCatalogIncrementalSyncDate = lastCatalogIncrementalSyncDate + self.lastCatalogFullSyncDate = lastCatalogFullSyncDate } } @@ -24,6 +27,8 @@ extension PersistedSite: FetchableRecord, PersistableRecord { static let id = Column(CodingKeys.id) // periphery:ignore - TODO: remove ignore when populating database static let lastCatalogIncrementalSyncDate = Column(CodingKeys.lastCatalogIncrementalSyncDate) + // periphery:ignore - TODO: remove ignore when populating database + static let lastCatalogFullSyncDate = Column(CodingKeys.lastCatalogFullSyncDate) } } @@ -32,5 +37,6 @@ private extension PersistedSite { enum CodingKeys: String, CodingKey { case id case lastCatalogIncrementalSyncDate + case lastCatalogFullSyncDate } } diff --git a/Modules/Sources/Yosemite/Model/Copiable/Models+Copiable.generated.swift b/Modules/Sources/Yosemite/Model/Copiable/Models+Copiable.generated.swift index a83ba0a2084..f7639a5621f 100644 --- a/Modules/Sources/Yosemite/Model/Copiable/Models+Copiable.generated.swift +++ b/Modules/Sources/Yosemite/Model/Copiable/Models+Copiable.generated.swift @@ -96,14 +96,17 @@ extension Yosemite.POSSimpleProduct { extension Yosemite.POSSite { public func copy( siteID: CopiableProp = .copy, - lastIncrementalSyncDate: NullableCopiableProp = .copy + lastIncrementalSyncDate: NullableCopiableProp = .copy, + lastFullSyncDate: NullableCopiableProp = .copy ) -> Yosemite.POSSite { let siteID = siteID ?? self.siteID let lastIncrementalSyncDate = lastIncrementalSyncDate ?? self.lastIncrementalSyncDate + let lastFullSyncDate = lastFullSyncDate ?? self.lastFullSyncDate return Yosemite.POSSite( siteID: siteID, - lastIncrementalSyncDate: lastIncrementalSyncDate + lastIncrementalSyncDate: lastIncrementalSyncDate, + lastFullSyncDate: lastFullSyncDate ) } } diff --git a/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift b/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift index bb3034d94ed..b733f205d3e 100644 --- a/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift +++ b/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift @@ -5,14 +5,16 @@ extension PersistedSite { init(from posSite: POSSite) { self.init( id: posSite.siteID, - lastCatalogIncrementalSyncDate: posSite.lastIncrementalSyncDate + lastCatalogIncrementalSyncDate: posSite.lastIncrementalSyncDate, + lastCatalogFullSyncDate: posSite.lastFullSyncDate ) } func toPOSSite() -> POSSite { POSSite( siteID: id, - lastIncrementalSyncDate: lastCatalogIncrementalSyncDate + lastIncrementalSyncDate: lastCatalogIncrementalSyncDate, + lastFullSyncDate: lastCatalogFullSyncDate ) } } diff --git a/Modules/Sources/Yosemite/PointOfSale/POSSite.swift b/Modules/Sources/Yosemite/PointOfSale/POSSite.swift index 3eba4437d8c..c882cb1cfde 100644 --- a/Modules/Sources/Yosemite/PointOfSale/POSSite.swift +++ b/Modules/Sources/Yosemite/PointOfSale/POSSite.swift @@ -5,9 +5,11 @@ import Foundation public struct POSSite: Equatable, GeneratedCopiable, GeneratedFakeable { public let siteID: Int64 public let lastIncrementalSyncDate: Date? + public let lastFullSyncDate: Date? - public init(siteID: Int64, lastIncrementalSyncDate: Date? = nil) { + public init(siteID: Int64, lastIncrementalSyncDate: Date? = nil, lastFullSyncDate: Date? = nil) { self.siteID = siteID self.lastIncrementalSyncDate = lastIncrementalSyncDate + self.lastFullSyncDate = lastFullSyncDate } } From 7bb4af6b68ce4ba3e32641f878dc0c08235f20d4 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 15:28:54 +0100 Subject: [PATCH 04/12] Combine duplicated MockPOSCatalogPersistenceService --- .../MockPOSCatalogPersistenceService.swift | 66 +++++++++++++++++++ .../POS/POSCatalogFullSyncServiceTests.swift | 19 ------ ...OSCatalogIncrementalSyncServiceTests.swift | 30 --------- 3 files changed, 66 insertions(+), 49 deletions(-) create mode 100644 Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift new file mode 100644 index 00000000000..2afd6f5a3b2 --- /dev/null +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift @@ -0,0 +1,66 @@ +@testable import Yosemite +import Foundation + +final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { + // MARK: - persistIncrementalCatalogData tracking + private(set) var persistIncrementalCatalogDataCallCount = 0 + private(set) var persistIncrementalCatalogDataLastPersistedCatalog: POSCatalog? + var persistIncrementalCatalogDataError: Error? + + // MARK: - loadSite tracking + var loadSiteResult: Result = .success(nil) + private(set) var loadSiteCallCount = 0 + + // Track specific sites for multi-site tests + var siteResults: [Int64: POSSite] = [:] + + // MARK: - updateSite tracking + private(set) var updateSiteCallCount = 0 + private(set) var lastUpdatedSite: POSSite? + + // Internal storage for updated sites + private var storedSites: [Int64: POSSite] = [:] + + // MARK: - Protocol Implementation + + func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { + // Not used in current tests + } + + func persistIncrementalCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { + persistIncrementalCatalogDataCallCount += 1 + persistIncrementalCatalogDataLastPersistedCatalog = catalog + + if let error = persistIncrementalCatalogDataError { + throw error + } + } + + func loadSite(siteID: Int64) async throws -> POSSite? { + loadSiteCallCount += 1 + + // Check if we have a stored site from updateSite calls + if let storedSite = storedSites[siteID] { + return storedSite + } + + // Check if we have a specific result for this site + if let specificSite = siteResults[siteID] { + return specificSite + } + + // Fall back to configured result + switch loadSiteResult { + case .success(let site): + return site + case .failure(let error): + throw error + } + } + + func updateSite(_ site: POSSite) async throws { + updateSiteCallCount += 1 + lastUpdatedSite = site + storedSites[site.siteID] = site + } +} diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift index 5f46359bc52..e518ac9c993 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift @@ -170,22 +170,3 @@ struct POSCatalogFullSyncServiceTests { } } -// MARK: - Mock POSCatalogPersistenceService - -private final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { - private(set) var replaceAllCatalogDataCallCount = 0 - private(set) var lastPersistedCatalog: POSCatalog? - private(set) var lastPersistedSiteID: Int64? - - func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { - replaceAllCatalogDataCallCount += 1 - lastPersistedSiteID = siteID - lastPersistedCatalog = catalog - } - - func persistIncrementalCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws {} - - func loadSite(siteID: Int64) async throws -> POSSite? { nil } - - func updateSite(_ site: POSSite) async throws {} -} diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift index d2e436365e2..1838a830b07 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift @@ -174,33 +174,3 @@ struct POSCatalogIncrementalSyncServiceTests { #expect(site2ModifiedAfter == lastFullSyncDate) } } - -// MARK: - Mock Classes - -private final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { - private(set) var persistIncrementalCatalogDataCallCount = 0 - private(set) var persistIncrementalCatalogDataLastPersistedCatalog: POSCatalog? - private(set) var persistIncrementalCatalogDataLastPersistedSiteID: Int64? - var persistIncrementalCatalogDataError: Error? - - private var storedSites: [Int64: POSSite] = [:] - - func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws {} - - func persistIncrementalCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { - persistIncrementalCatalogDataCallCount += 1 - persistIncrementalCatalogDataLastPersistedSiteID = siteID - persistIncrementalCatalogDataLastPersistedCatalog = catalog - if let error = persistIncrementalCatalogDataError { - throw error - } - } - - func loadSite(siteID: Int64) async throws -> POSSite? { - storedSites[siteID] - } - - func updateSite(_ site: POSSite) async throws { - storedSites[site.siteID] = site - } -} From 3d9d4681cbca143cff7a3500fa31737c4f9e7e76 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 16:17:24 +0100 Subject: [PATCH 05/12] Use full sync date from site entity --- .../Tools/POS/POSCatalogSyncCoordinator.swift | 40 +++++++++++++++---- .../POS/POSCatalogSyncCoordinatorTests.swift | 37 ++++++++--------- 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index be349fa078c..675c59666be 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -22,17 +22,24 @@ public enum POSCatalogSyncError: Error, Equatable { public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { private let fullSyncService: POSCatalogFullSyncServiceProtocol - private let settingsStore: SiteSpecificAppSettingsStoreMethodsProtocol + private let persistenceService: POSCatalogPersistenceServiceProtocol private let grdbManager: GRDBManagerProtocol /// Tracks ongoing syncs by site ID to prevent duplicates private var ongoingSyncs: Set = [] public init(fullSyncService: POSCatalogFullSyncServiceProtocol, - settingsStore: SiteSpecificAppSettingsStoreMethodsProtocol? = nil, grdbManager: GRDBManagerProtocol) { self.fullSyncService = fullSyncService - self.settingsStore = settingsStore ?? SiteSpecificAppSettingsStoreMethods(fileStorage: PListFileStorage()) + self.persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager) + self.grdbManager = grdbManager + } + + init(fullSyncService: POSCatalogFullSyncServiceProtocol, + persistenceService: POSCatalogPersistenceServiceProtocol, + grdbManager: GRDBManagerProtocol) { + self.fullSyncService = fullSyncService + self.persistenceService = persistenceService self.grdbManager = grdbManager } @@ -54,7 +61,21 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { let catalog = try await fullSyncService.startFullSync(for: siteID) - settingsStore.setPOSLastFullSyncDate(Date(), for: siteID) + // Update the site with the full sync date + let currentSite = try await persistenceService.loadSite(siteID: siteID) + let updatedSite = POSSite( + siteID: siteID, + lastIncrementalSyncDate: currentSite?.lastIncrementalSyncDate, + lastFullSyncDate: Date() + ) + + do { + try await persistenceService.updateSite(updatedSite) + } catch POSCatalogPersistenceError.siteNotFound { + // Site doesn't exist yet, this could happen if sync coordinator is called before replaceAllCatalogData + // In this case, the full sync service should have handled the catalog persistence already + DDLogInfo("🟡 POSCatalogSyncCoordinator: Site \(siteID) not found during sync timestamp update - sync timestamp will be set during catalog persistence") + } DDLogInfo("✅ POSCatalogSyncCoordinator completed full sync for site \(siteID)") } @@ -65,7 +86,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { return true } - guard let lastSyncDate = lastFullSyncDate(for: siteID) else { + guard let lastSyncDate = await lastFullSyncDate(for: siteID) else { DDLogInfo("📋 POSCatalogSyncCoordinator: No previous sync found for site \(siteID), sync needed") return true } @@ -84,8 +105,13 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { // MARK: - Private - private func lastFullSyncDate(for siteID: Int64) -> Date? { - return settingsStore.getPOSLastFullSyncDate(for: siteID) + private func lastFullSyncDate(for siteID: Int64) async -> Date? { + do { + return try await persistenceService.loadSite(siteID: siteID)?.lastFullSyncDate + } catch { + DDLogError("⛔️ POSCatalogSyncCoordinator: Error loading site \(siteID) for full sync date: \(error)") + return nil + } } private func siteExistsInDatabase(siteID: Int64) -> Bool { diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index 5a8ff2ea4b9..5391fccb077 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -5,18 +5,18 @@ import Testing struct POSCatalogSyncCoordinatorTests { private let mockSyncService: MockPOSCatalogFullSyncService - private let mockSettingsStore: MockSiteSpecificAppSettingsStoreMethods + private let mockPersistenceService: MockPOSCatalogPersistenceService private let grdbManager: GRDBManager private let sut: POSCatalogSyncCoordinator private let sampleSiteID: Int64 = 134 init() throws { self.mockSyncService = MockPOSCatalogFullSyncService() - self.mockSettingsStore = MockSiteSpecificAppSettingsStoreMethods() + self.mockPersistenceService = MockPOSCatalogPersistenceService() self.grdbManager = try GRDBManager() self.sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, - settingsStore: mockSettingsStore, + persistenceService: mockPersistenceService, grdbManager: grdbManager ) } @@ -44,16 +44,17 @@ struct POSCatalogSyncCoordinatorTests { let beforeSync = Date() let expectedCatalog = POSCatalog(products: [], variations: []) mockSyncService.startFullSyncResult = .success(expectedCatalog) + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID)) // When try await sut.performFullSync(for: sampleSiteID) let afterSync = Date() // Then - #expect(mockSettingsStore.setPOSLastFullSyncDateCallCount == 1) - #expect(mockSettingsStore.lastSetSiteID == sampleSiteID) + #expect(mockPersistenceService.updateSiteCallCount == 1) + #expect(mockPersistenceService.lastUpdatedSite?.siteID == sampleSiteID) - let storedDate = mockSettingsStore.lastSetDate + let storedDate = mockPersistenceService.lastUpdatedSite?.lastFullSyncDate #expect(storedDate != nil) #expect(storedDate! >= beforeSync) #expect(storedDate! <= afterSync) @@ -70,14 +71,14 @@ struct POSCatalogSyncCoordinatorTests { } // Should not store timestamp on failure - #expect(mockSettingsStore.setPOSLastFullSyncDateCallCount == 0) + #expect(mockPersistenceService.updateSiteCallCount == 0) } // MARK: - Should Sync Decision Tests @Test func shouldPerformFullSync_returns_true_when_site_is_not_in_database_with_no_sync_history() async { // Given - site doesn't exist in database AND has no sync history - mockSettingsStore.storedDates = [:] + mockPersistenceService.loadSiteResult = .success(nil) // Note: NOT creating site in database // When @@ -90,7 +91,7 @@ struct POSCatalogSyncCoordinatorTests { @Test func shouldPerformFullSync_returns_true_when_site_is_in_database_with_no_previous_sync() async throws { // Given - no previous sync date stored, but site exists in database // This is much less likely to happen, but could help at a migration point - mockSettingsStore.storedDates = [:] + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: nil)) try createSiteInDatabase(siteID: sampleSiteID) // When @@ -98,13 +99,13 @@ struct POSCatalogSyncCoordinatorTests { // Then #expect(shouldSync == true) - #expect(mockSettingsStore.getPOSLastFullSyncDateCallCount == 1) + #expect(mockPersistenceService.loadSiteCallCount == 1) } @Test func shouldPerformFullSync_returns_true_when_sync_is_stale() async throws { // Given - previous sync was 2 hours ago, and site exists in database let twoHoursAgo = Date().addingTimeInterval(-2 * 60 * 60) - mockSettingsStore.storedDates[sampleSiteID] = twoHoursAgo + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: twoHoursAgo)) try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 1 hour @@ -117,7 +118,7 @@ struct POSCatalogSyncCoordinatorTests { @Test func shouldPerformFullSync_returns_false_when_sync_is_fresh() async throws { // Given - previous sync was 30 minutes ago, and site exists in database let thirtyMinutesAgo = Date().addingTimeInterval(-30 * 60) - mockSettingsStore.storedDates[sampleSiteID] = thirtyMinutesAgo + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: thirtyMinutesAgo)) try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 1 hour @@ -133,8 +134,9 @@ struct POSCatalogSyncCoordinatorTests { let siteB: Int64 = 456 let oneHourAgo = Date().addingTimeInterval(-60 * 60) - mockSettingsStore.storedDates[siteA] = oneHourAgo // Has previous sync - // siteB has no previous sync + // Setup mock to return different results for different sites + mockPersistenceService.siteResults[siteA] = POSSite(siteID: siteA, lastFullSyncDate: oneHourAgo) + mockPersistenceService.siteResults[siteB] = POSSite(siteID: siteB, lastFullSyncDate: nil) // Create both sites in database to test timing logic try createSiteInDatabase(siteID: siteA) @@ -152,7 +154,7 @@ struct POSCatalogSyncCoordinatorTests { @Test func shouldPerformFullSync_with_zero_maxAge_always_returns_true() async throws { // Given - previous sync was just now, and site exists in database let justNow = Date() - mockSettingsStore.storedDates[sampleSiteID] = justNow + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: justNow)) try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 0 (always sync) @@ -166,8 +168,7 @@ struct POSCatalogSyncCoordinatorTests { @Test func shouldPerformFullSync_returns_true_when_site_not_in_database() async { // Given - site does not exist in database, but has recent sync date - let recentSyncDate = Date().addingTimeInterval(-30 * 60) // 30 minutes ago - mockSettingsStore.storedDates[sampleSiteID] = recentSyncDate + mockPersistenceService.loadSiteResult = .success(nil) // Note: not creating site in database so it won't exist // When - max age is 1 hour (normally wouldn't sync) @@ -180,7 +181,7 @@ struct POSCatalogSyncCoordinatorTests { @Test func shouldPerformFullSync_respects_time_when_site_exists_in_database() async throws { // Given - site exists in database with recent sync date let recentSyncDate = Date().addingTimeInterval(-30 * 60) // 30 minutes ago - mockSettingsStore.storedDates[sampleSiteID] = recentSyncDate + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: recentSyncDate)) try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 1 hour From d54dd2e079c344ee0c5c371d2347590fb60feec6 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 16:17:59 +0100 Subject: [PATCH 06/12] Stop storing full sync date in settings --- .../Copiable/Models+Copiable.generated.swift | 7 +- .../Storage/Model/GeneralStoreSettings.swift | 11 +- .../SiteSpecificAppSettingsStoreMethods.swift | 18 --- ...kSiteSpecificAppSettingsStoreMethods.swift | 17 --- ...SpecificAppSettingsStoreMethodsTests.swift | 135 ------------------ 5 files changed, 4 insertions(+), 184 deletions(-) diff --git a/Modules/Sources/Storage/Model/Copiable/Models+Copiable.generated.swift b/Modules/Sources/Storage/Model/Copiable/Models+Copiable.generated.swift index 34327f624f0..5bf17686464 100644 --- a/Modules/Sources/Storage/Model/Copiable/Models+Copiable.generated.swift +++ b/Modules/Sources/Storage/Model/Copiable/Models+Copiable.generated.swift @@ -115,8 +115,7 @@ extension Storage.GeneralStoreSettings { lastSelectedOrderStatus: NullableCopiableProp = .copy, favoriteProductIDs: CopiableProp<[Int64]> = .copy, searchTermsByKey: CopiableProp<[String: [String]]> = .copy, - isPOSTabVisible: NullableCopiableProp = .copy, - posLastFullSyncDate: NullableCopiableProp = .copy + isPOSTabVisible: NullableCopiableProp = .copy ) -> Storage.GeneralStoreSettings { let storeID = storeID ?? self.storeID let isTelemetryAvailable = isTelemetryAvailable ?? self.isTelemetryAvailable @@ -138,7 +137,6 @@ extension Storage.GeneralStoreSettings { let favoriteProductIDs = favoriteProductIDs ?? self.favoriteProductIDs let searchTermsByKey = searchTermsByKey ?? self.searchTermsByKey let isPOSTabVisible = isPOSTabVisible ?? self.isPOSTabVisible - let posLastFullSyncDate = posLastFullSyncDate ?? self.posLastFullSyncDate return Storage.GeneralStoreSettings( storeID: storeID, @@ -160,8 +158,7 @@ extension Storage.GeneralStoreSettings { lastSelectedOrderStatus: lastSelectedOrderStatus, favoriteProductIDs: favoriteProductIDs, searchTermsByKey: searchTermsByKey, - isPOSTabVisible: isPOSTabVisible, - posLastFullSyncDate: posLastFullSyncDate + isPOSTabVisible: isPOSTabVisible ) } } diff --git a/Modules/Sources/Storage/Model/GeneralStoreSettings.swift b/Modules/Sources/Storage/Model/GeneralStoreSettings.swift index d0623517227..deefc73eb65 100644 --- a/Modules/Sources/Storage/Model/GeneralStoreSettings.swift +++ b/Modules/Sources/Storage/Model/GeneralStoreSettings.swift @@ -86,9 +86,6 @@ public struct GeneralStoreSettings: Codable, Equatable, GeneratedCopiable { /// public var isPOSTabVisible: Bool? - /// Last time a POS catalog full sync was completed for this store. - /// - public var posLastFullSyncDate: Date? public init(storeID: String? = nil, isTelemetryAvailable: Bool = false, @@ -109,8 +106,7 @@ public struct GeneralStoreSettings: Codable, Equatable, GeneratedCopiable { lastSelectedOrderStatus: String? = nil, favoriteProductIDs: [Int64] = [], searchTermsByKey: [String: [String]] = [:], - isPOSTabVisible: Bool? = nil, - posLastFullSyncDate: Date? = nil) { + isPOSTabVisible: Bool? = nil) { self.storeID = storeID self.isTelemetryAvailable = isTelemetryAvailable self.telemetryLastReportedTime = telemetryLastReportedTime @@ -131,7 +127,6 @@ public struct GeneralStoreSettings: Codable, Equatable, GeneratedCopiable { self.favoriteProductIDs = favoriteProductIDs self.searchTermsByKey = searchTermsByKey self.isPOSTabVisible = isPOSTabVisible - self.posLastFullSyncDate = posLastFullSyncDate } public func erasingSelectedTaxRateID() -> GeneralStoreSettings { @@ -153,8 +148,7 @@ public struct GeneralStoreSettings: Codable, Equatable, GeneratedCopiable { lastSelectedOrderStatus: lastSelectedOrderStatus, favoriteProductIDs: favoriteProductIDs, searchTermsByKey: searchTermsByKey, - isPOSTabVisible: isPOSTabVisible, - posLastFullSyncDate: posLastFullSyncDate) + isPOSTabVisible: isPOSTabVisible) } } @@ -189,7 +183,6 @@ extension GeneralStoreSettings { self.searchTermsByKey = try container.decodeIfPresent([String: [String]].self, forKey: .searchTermsByKey) ?? [:] self.isPOSTabVisible = try container.decodeIfPresent(Bool.self, forKey: .isPOSTabVisible) - self.posLastFullSyncDate = try container.decodeIfPresent(Date.self, forKey: .posLastFullSyncDate) // Decode new properties with `decodeIfPresent` and provide a default value if necessary. } diff --git a/Modules/Sources/Yosemite/Stores/Helpers/SiteSpecificAppSettingsStoreMethods.swift b/Modules/Sources/Yosemite/Stores/Helpers/SiteSpecificAppSettingsStoreMethods.swift index 976bf32141d..7fbe31065d4 100644 --- a/Modules/Sources/Yosemite/Stores/Helpers/SiteSpecificAppSettingsStoreMethods.swift +++ b/Modules/Sources/Yosemite/Stores/Helpers/SiteSpecificAppSettingsStoreMethods.swift @@ -11,10 +11,6 @@ public protocol SiteSpecificAppSettingsStoreMethodsProtocol { // Search history methods func getSearchTerms(for itemType: POSItemType, siteID: Int64) -> [String] func setSearchTerms(_ terms: [String], for itemType: POSItemType, siteID: Int64) - - // POS catalog sync timestamp methods - func getPOSLastFullSyncDate(for siteID: Int64) -> Date? - func setPOSLastFullSyncDate(_ date: Date?, for siteID: Int64) } /// Methods for managing site-specific app settings @@ -102,20 +98,6 @@ extension SiteSpecificAppSettingsStoreMethods { } } -// MARK: - POS Catalog Sync Timestamps -extension SiteSpecificAppSettingsStoreMethods { - func getPOSLastFullSyncDate(for siteID: Int64) -> Date? { - let storeSettings = getStoreSettings(for: siteID) - return storeSettings.posLastFullSyncDate - } - - func setPOSLastFullSyncDate(_ date: Date?, for siteID: Int64) { - let storeSettings = getStoreSettings(for: siteID) - let updatedSettings = storeSettings.copy(posLastFullSyncDate: date) - setStoreSettings(settings: updatedSettings, for: siteID) - } -} - // MARK: - Constants private enum Constants { static let generalStoreSettingsFileName = "general-store-settings.plist" diff --git a/Modules/Tests/YosemiteTests/Mocks/MockSiteSpecificAppSettingsStoreMethods.swift b/Modules/Tests/YosemiteTests/Mocks/MockSiteSpecificAppSettingsStoreMethods.swift index b1f794039de..e311398ce28 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockSiteSpecificAppSettingsStoreMethods.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockSiteSpecificAppSettingsStoreMethods.swift @@ -28,12 +28,6 @@ final class MockSiteSpecificAppSettingsStoreMethods: SiteSpecificAppSettingsStor var spySetSearchTermsSiteID: Int64? var mockSearchTerms: [POSItemType: [String]] = [:] - // POS sync timestamp properties - var storedDates: [Int64: Date] = [:] - private(set) var getPOSLastFullSyncDateCallCount = 0 - private(set) var setPOSLastFullSyncDateCallCount = 0 - private(set) var lastSetSiteID: Int64? - private(set) var lastSetDate: Date? func getStoreSettings(for siteID: Int64) -> GeneralStoreSettings { getStoreSettingsCalled = true @@ -92,15 +86,4 @@ final class MockSiteSpecificAppSettingsStoreMethods: SiteSpecificAppSettingsStor mockSearchTerms[itemType] = terms } - func getPOSLastFullSyncDate(for siteID: Int64) -> Date? { - getPOSLastFullSyncDateCallCount += 1 - return storedDates[siteID] - } - - func setPOSLastFullSyncDate(_ date: Date?, for siteID: Int64) { - setPOSLastFullSyncDateCallCount += 1 - lastSetSiteID = siteID - lastSetDate = date - storedDates[siteID] = date - } } diff --git a/Modules/Tests/YosemiteTests/Stores/Helpers/SiteSpecificAppSettingsStoreMethodsTests.swift b/Modules/Tests/YosemiteTests/Stores/Helpers/SiteSpecificAppSettingsStoreMethodsTests.swift index 9544b73ddee..164649dcab1 100644 --- a/Modules/Tests/YosemiteTests/Stores/Helpers/SiteSpecificAppSettingsStoreMethodsTests.swift +++ b/Modules/Tests/YosemiteTests/Stores/Helpers/SiteSpecificAppSettingsStoreMethodsTests.swift @@ -230,141 +230,6 @@ struct SiteSpecificAppSettingsStoreMethodsTests { #expect(retrievedCouponTerms == couponTerms) } - // MARK: - POS Last Full Sync Date Tests - - @Test func getPOSLastFullSyncDate_returns_nil_when_no_date_exists() { - // When - let syncDate = sut.getPOSLastFullSyncDate(for: siteID) - - // Then - #expect(syncDate == nil) - } - - @Test func getPOSLastFullSyncDate_returns_saved_date() throws { - // Given - let expectedDate = Date() - let storeSettings = GeneralStoreSettings(posLastFullSyncDate: expectedDate) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [siteID: storeSettings]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - // When - let syncDate = sut.getPOSLastFullSyncDate(for: siteID) - - // Then - #expect(syncDate == expectedDate) - } - - @Test func setPOSLastFullSyncDate_saves_date_successfully() throws { - // Given - let dateToSave = Date() - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [siteID: GeneralStoreSettings()]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - // When - sut.setPOSLastFullSyncDate(dateToSave, for: siteID) - - // Then - let savedData: GeneralStoreSettingsBySite = try fileStorage.data(for: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - #expect(savedData.storeSettingsBySite[siteID]?.posLastFullSyncDate == dateToSave) - } - - @Test func setPOSLastFullSyncDate_can_set_nil_date() throws { - // Given - let existingDate = Date() - let storeSettings = GeneralStoreSettings(posLastFullSyncDate: existingDate) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [siteID: storeSettings]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - // When - sut.setPOSLastFullSyncDate(nil, for: siteID) - - // Then - let savedData: GeneralStoreSettingsBySite = try fileStorage.data(for: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - #expect(savedData.storeSettingsBySite[siteID]?.posLastFullSyncDate == nil) - } - - @Test func setPOSLastFullSyncDate_preserves_other_settings() throws { - // Given - let existingStoreID = "existing-store" - let existingTerms = ["existing", "terms"] - let storeSettings = GeneralStoreSettings( - storeID: existingStoreID, - searchTermsByKey: ["product_search_terms": existingTerms] - ) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [siteID: storeSettings]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - let dateToSave = Date() - - // When - sut.setPOSLastFullSyncDate(dateToSave, for: siteID) - - // Then - let savedData: GeneralStoreSettingsBySite = try fileStorage.data(for: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - let savedSettings = savedData.storeSettingsBySite[siteID] - #expect(savedSettings?.posLastFullSyncDate == dateToSave) - #expect(savedSettings?.storeID == existingStoreID) - #expect(savedSettings?.searchTermsByKey["product_search_terms"] == existingTerms) - } - - @Test func setPOSLastFullSyncDate_preserves_dates_for_other_sites() throws { - // Given - let otherSiteID: Int64 = 456 - let otherSiteDate = Date().addingTimeInterval(-3600) - let otherSiteSettings = GeneralStoreSettings(posLastFullSyncDate: otherSiteDate) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [otherSiteID: otherSiteSettings]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - let newDate = Date() - - // When - sut.setPOSLastFullSyncDate(newDate, for: siteID) - - // Then - let savedData: GeneralStoreSettingsBySite = try fileStorage.data(for: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - #expect(savedData.storeSettingsBySite[siteID]?.posLastFullSyncDate == newDate) - #expect(savedData.storeSettingsBySite[otherSiteID]?.posLastFullSyncDate == otherSiteDate) - } - - @Test func getPOSLastFullSyncDate_handles_different_sites_independently() throws { - // Given - let siteA: Int64 = 123 - let siteB: Int64 = 456 - let dateA = Date() - let dateB = Date().addingTimeInterval(-3600) - - let storeSettingsA = GeneralStoreSettings(posLastFullSyncDate: dateA) - let storeSettingsB = GeneralStoreSettings(posLastFullSyncDate: dateB) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [ - siteA: storeSettingsA, - siteB: storeSettingsB - ]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - // When - let retrievedDateA = sut.getPOSLastFullSyncDate(for: siteA) - let retrievedDateB = sut.getPOSLastFullSyncDate(for: siteB) - - // Then - #expect(retrievedDateA == dateA) - #expect(retrievedDateB == dateB) - } - - @Test func resetStoreSettings_clears_pos_sync_date() throws { - // Given - let syncDate = Date() - let storeSettings = GeneralStoreSettings(posLastFullSyncDate: syncDate) - let existingData = GeneralStoreSettingsBySite(storeSettingsBySite: [siteID: storeSettings]) - try fileStorage.write(existingData, to: SiteSpecificAppSettingsStoreMethods.defaultGeneralStoreSettingsFileURL) - - // When - sut.resetStoreSettings() - - // Then - #expect(fileStorage.deleteIsHit == true) - let retrievedDate = sut.getPOSLastFullSyncDate(for: siteID) - #expect(retrievedDate == nil) - } } // MARK: - Mock FileStorage From a467350f21e8f4f1f64dfb4767a2e2175b8a3e9a Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 16:19:46 +0100 Subject: [PATCH 07/12] Lint fixes --- .../Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift | 3 ++- .../Mocks/MockPOSCatalogPersistenceService.swift | 8 ++++---- .../Tools/POS/POSCatalogFullSyncServiceTests.swift | 1 - 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index 675c59666be..b5ea1471ac6 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -35,6 +35,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { self.grdbManager = grdbManager } + //periphery:ignore - used for tests to inject persistence service init(fullSyncService: POSCatalogFullSyncServiceProtocol, persistenceService: POSCatalogPersistenceServiceProtocol, grdbManager: GRDBManagerProtocol) { @@ -68,7 +69,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { lastIncrementalSyncDate: currentSite?.lastIncrementalSyncDate, lastFullSyncDate: Date() ) - + do { try await persistenceService.updateSite(updatedSite) } catch POSCatalogPersistenceError.siteNotFound { diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift index 2afd6f5a3b2..c9140d2e5a0 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift @@ -10,19 +10,19 @@ final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtoc // MARK: - loadSite tracking var loadSiteResult: Result = .success(nil) private(set) var loadSiteCallCount = 0 - + // Track specific sites for multi-site tests var siteResults: [Int64: POSSite] = [:] - + // MARK: - updateSite tracking private(set) var updateSiteCallCount = 0 private(set) var lastUpdatedSite: POSSite? - + // Internal storage for updated sites private var storedSites: [Int64: POSSite] = [:] // MARK: - Protocol Implementation - + func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { // Not used in current tests } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift index e518ac9c993..1c57b8b43aa 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift @@ -169,4 +169,3 @@ struct POSCatalogFullSyncServiceTests { #expect(mockSyncRemote.loadProductVariationsCallCount == 5) } } - From 616957ee4f5f2dec1ef66203a795f21821ced617 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Thu, 11 Sep 2025 10:58:39 +0800 Subject: [PATCH 08/12] Add incremental sync support to `POSCatalogSyncCoordinator`. --- .../Tools/POS/POSCatalogSyncCoordinator.swift | 68 +++++++++++++++++-- ...MockPOSCatalogIncrementalSyncService.swift | 29 ++++++++ .../POS/POSCatalogSyncCoordinatorTests.swift | 3 + .../Classes/Yosemite/AuthenticatedState.swift | 4 +- 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index b5ea1471ac6..0254cd7cda3 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -14,6 +14,13 @@ public protocol POSCatalogSyncCoordinatorProtocol { /// - maxAge: Maximum age before a sync is considered stale /// - Returns: True if a sync should be performed func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) async -> Bool + + /// Performs an incremental sync if applicable based on sync conditions + /// - Parameters: + /// - siteID: The site ID to sync catalog for + /// - forceSync: Whether to bypass age checks and always sync + /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site + func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws } public enum POSCatalogSyncError: Error, Equatable { @@ -22,26 +29,38 @@ public enum POSCatalogSyncError: Error, Equatable { public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { private let fullSyncService: POSCatalogFullSyncServiceProtocol + private let incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol private let persistenceService: POSCatalogPersistenceServiceProtocol private let grdbManager: GRDBManagerProtocol + private let maxIncrementalSyncAge: TimeInterval - /// Tracks ongoing syncs by site ID to prevent duplicates + /// Tracks ongoing full syncs by site ID to prevent duplicates private var ongoingSyncs: Set = [] + /// Tracks ongoing incremental syncs by site ID to prevent duplicates + private var ongoingIncrementalSyncs: Set = [] public init(fullSyncService: POSCatalogFullSyncServiceProtocol, - grdbManager: GRDBManagerProtocol) { + incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol, + grdbManager: GRDBManagerProtocol, + maxIncrementalSyncAge: TimeInterval = 300) { self.fullSyncService = fullSyncService + self.incrementalSyncService = incrementalSyncService self.persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager) self.grdbManager = grdbManager + self.maxIncrementalSyncAge = maxIncrementalSyncAge } //periphery:ignore - used for tests to inject persistence service init(fullSyncService: POSCatalogFullSyncServiceProtocol, + incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol, persistenceService: POSCatalogPersistenceServiceProtocol, - grdbManager: GRDBManagerProtocol) { + grdbManager: GRDBManagerProtocol, + maxIncrementalSyncAge: TimeInterval = 300) { self.fullSyncService = fullSyncService + self.incrementalSyncService = incrementalSyncService self.persistenceService = persistenceService self.grdbManager = grdbManager + self.maxIncrementalSyncAge = maxIncrementalSyncAge } public func performFullSync(for siteID: Int64) async throws { @@ -60,7 +79,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { DDLogInfo("🔄 POSCatalogSyncCoordinator starting full sync for site \(siteID)") - let catalog = try await fullSyncService.startFullSync(for: siteID) + _ = try await fullSyncService.startFullSync(for: siteID) // Update the site with the full sync date let currentSite = try await persistenceService.loadSite(siteID: siteID) @@ -104,6 +123,38 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { return shouldSync } + public func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws { + if ongoingIncrementalSyncs.contains(siteID) { + DDLogInfo("⚠️ POSCatalogSyncCoordinator: Incremental sync already in progress for site \(siteID)") + throw POSCatalogSyncError.syncAlreadyInProgress(siteID: siteID) + } + + guard let lastFullSyncDate = await lastFullSyncDate(for: siteID) else { + DDLogInfo("📋 POSCatalogSyncCoordinator: No full sync performed yet for site \(siteID), skipping incremental sync") + return + } + + if !forceSync, let lastIncrementalSyncDate = await lastIncrementalSyncDate(for: siteID) { + let age = Date().timeIntervalSince(lastIncrementalSyncDate) + + if age <= maxIncrementalSyncAge { + return DDLogInfo("📋 POSCatalogSyncCoordinator: Last incremental sync for site \(siteID) was \(Int(age))s ago, sync not needed") + } + } + + ongoingIncrementalSyncs.insert(siteID) + + defer { + ongoingIncrementalSyncs.remove(siteID) + } + + DDLogInfo("🔄 POSCatalogSyncCoordinator starting incremental sync for site \(siteID)") + + try await incrementalSyncService.startIncrementalSync(for: siteID, lastFullSyncDate: lastFullSyncDate) + + DDLogInfo("✅ POSCatalogSyncCoordinator completed incremental sync for site \(siteID)") + } + // MARK: - Private private func lastFullSyncDate(for siteID: Int64) async -> Date? { @@ -115,6 +166,15 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { } } + private func lastIncrementalSyncDate(for siteID: Int64) async -> Date? { + do { + return try await persistenceService.loadSite(siteID: siteID)?.lastIncrementalSyncDate + } catch { + DDLogError("⛔️ POSCatalogSyncCoordinator: Error loading site \(siteID) for incremental sync date: \(error)") + return nil + } + } + private func siteExistsInDatabase(siteID: Int64) -> Bool { do { return try grdbManager.databaseConnection.read { db in diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift new file mode 100644 index 00000000000..f27cecc75d8 --- /dev/null +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift @@ -0,0 +1,29 @@ +import Foundation +@testable import Yosemite + +final class MockPOSCatalogIncrementalSyncService: POSCatalogIncrementalSyncServiceProtocol { + private(set) var startIncrementalSyncCallCount = 0 + private(set) var lastSiteID: Int64? + private(set) var lastFullSyncDate: Date? + + var shouldThrowError: Error? + + func startIncrementalSync(for siteID: Int64, lastFullSyncDate: Date) async throws { + startIncrementalSyncCallCount += 1 + self.lastSiteID = siteID + self.lastFullSyncDate = lastFullSyncDate + + if let error = shouldThrowError { + throw error + } + } +} + +extension MockPOSCatalogIncrementalSyncService { + func reset() { + startIncrementalSyncCallCount = 0 + lastSiteID = nil + lastFullSyncDate = nil + shouldThrowError = nil + } +} diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index 5391fccb077..9d998bf5cf8 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -5,6 +5,7 @@ import Testing struct POSCatalogSyncCoordinatorTests { private let mockSyncService: MockPOSCatalogFullSyncService + private let mockIncrementalSyncService: MockPOSCatalogIncrementalSyncService private let mockPersistenceService: MockPOSCatalogPersistenceService private let grdbManager: GRDBManager private let sut: POSCatalogSyncCoordinator @@ -12,10 +13,12 @@ struct POSCatalogSyncCoordinatorTests { init() throws { self.mockSyncService = MockPOSCatalogFullSyncService() + self.mockIncrementalSyncService = MockPOSCatalogIncrementalSyncService() self.mockPersistenceService = MockPOSCatalogPersistenceService() self.grdbManager = try GRDBManager() self.sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, + incrementalSyncService: mockIncrementalSyncService, persistenceService: mockPersistenceService, grdbManager: grdbManager ) diff --git a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift index f5710b4378b..5d7a75b1b31 100644 --- a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift +++ b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift @@ -142,9 +142,11 @@ class AuthenticatedState: StoresManagerState { // Initialize POS catalog sync coordinator if feature flag is enabled if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1), - let fullSyncService = POSCatalogFullSyncService(credentials: credentials, grdbManager: ServiceLocator.grdbManager) { + let fullSyncService = POSCatalogFullSyncService(credentials: credentials, grdbManager: ServiceLocator.grdbManager), + let incrementalSyncService = POSCatalogIncrementalSyncService(credentials: credentials, grdbManager: ServiceLocator.grdbManager) { posCatalogSyncCoordinator = POSCatalogSyncCoordinator( fullSyncService: fullSyncService, + incrementalSyncService: incrementalSyncService, grdbManager: ServiceLocator.grdbManager ) } else { From db3456e1fe3eb82417e0f5fdde46973cec7087e1 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Thu, 11 Sep 2025 11:55:16 +0800 Subject: [PATCH 09/12] POSCatalogSyncCoordinator: add test cases for `performIncrementalSyncIfApplicable`. --- ...MockPOSCatalogIncrementalSyncService.swift | 39 ++-- .../POS/POSCatalogSyncCoordinatorTests.swift | 196 ++++++++++++++++++ 2 files changed, 223 insertions(+), 12 deletions(-) diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift index f27cecc75d8..35678b9a282 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogIncrementalSyncService.swift @@ -2,28 +2,43 @@ import Foundation @testable import Yosemite final class MockPOSCatalogIncrementalSyncService: POSCatalogIncrementalSyncServiceProtocol { + var startIncrementalSyncResult: Result = .success(()) + private(set) var startIncrementalSyncCallCount = 0 - private(set) var lastSiteID: Int64? + private(set) var lastSyncSiteID: Int64? private(set) var lastFullSyncDate: Date? - - var shouldThrowError: Error? - + + private var syncContinuation: CheckedContinuation? + private var shouldBlockSync = false + func startIncrementalSync(for siteID: Int64, lastFullSyncDate: Date) async throws { startIncrementalSyncCallCount += 1 - self.lastSiteID = siteID + lastSyncSiteID = siteID self.lastFullSyncDate = lastFullSyncDate - - if let error = shouldThrowError { + + if shouldBlockSync { + await withCheckedContinuation { continuation in + syncContinuation = continuation + } + } + + switch startIncrementalSyncResult { + case .success: + return + case .failure(let error): throw error } } } extension MockPOSCatalogIncrementalSyncService { - func reset() { - startIncrementalSyncCallCount = 0 - lastSiteID = nil - lastFullSyncDate = nil - shouldThrowError = nil + func blockNextSync() { + shouldBlockSync = true + } + + func resumeBlockedSync() { + syncContinuation?.resume() + syncContinuation = nil + shouldBlockSync = false } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index 9d998bf5cf8..bbc624996db 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -260,6 +260,202 @@ struct POSCatalogSyncCoordinatorTests { try await sut.performFullSync(for: sampleSiteID) } + // MARK: - Incremental Sync Tests + + @Test(arguments: [true, false]) + func performIncrementalSyncIfApplicable_skips_sync_when_no_full_sync_performed(forceSync: Bool) async throws { + // Given + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: nil)) + + // When + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + + // Then + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) + } + + @Test func performIncrementalSyncIfApplicable_skips_sync_when_incremental_sync_is_within_max_age() async throws { + // Given + let maxAge: TimeInterval = 2 + let incrementalSyncDate = Date().addingTimeInterval(-(maxAge - 0.2)) // Just within max age + mockPersistenceService.loadSiteResult = .success(POSSite( + siteID: sampleSiteID, + lastIncrementalSyncDate: incrementalSyncDate, + lastFullSyncDate: Date().addingTimeInterval(-7200) + )) + let sut = POSCatalogSyncCoordinator( + fullSyncService: mockSyncService, + incrementalSyncService: mockIncrementalSyncService, + persistenceService: mockPersistenceService, + grdbManager: grdbManager, + maxIncrementalSyncAge: maxAge + ) + + // When + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + + // Then + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) + } + + @Test(arguments: [true, false]) + func performIncrementalSyncIfApplicable_performs_sync_when_incremental_sync_is_stale(forceSync: Bool) async throws { + // Given + let maxAge: TimeInterval = 2 + let incrementalSyncDate = Date().addingTimeInterval(-(maxAge + 0.2)) // Just above max age + let fullSyncDate = Date().addingTimeInterval(-3600) + mockPersistenceService.loadSiteResult = .success(POSSite( + siteID: sampleSiteID, + lastIncrementalSyncDate: incrementalSyncDate, + lastFullSyncDate: fullSyncDate + )) + let sut = POSCatalogSyncCoordinator( + fullSyncService: mockSyncService, + incrementalSyncService: mockIncrementalSyncService, + persistenceService: mockPersistenceService, + grdbManager: grdbManager, + maxIncrementalSyncAge: maxAge + ) + + // When + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + + // Then + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) + #expect(mockIncrementalSyncService.lastSyncSiteID == sampleSiteID) + #expect(mockIncrementalSyncService.lastFullSyncDate == fullSyncDate) + } + + @Test(arguments: [true, false]) + func performIncrementalSyncIfApplicable_performs_sync_when_no_incremental_sync_date(forceSync: Bool) async throws { + // Given + let fullSyncDate = Date().addingTimeInterval(-3600) + mockPersistenceService.loadSiteResult = .success(POSSite( + siteID: sampleSiteID, + lastIncrementalSyncDate: nil, + lastFullSyncDate: fullSyncDate + )) + + // When + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + + // Then + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) + #expect(mockIncrementalSyncService.lastSyncSiteID == sampleSiteID) + #expect(mockIncrementalSyncService.lastFullSyncDate == fullSyncDate) + } + + @Test func performIncrementalSyncIfApplicable_forceSync_bypasses_age_check() async throws { + // Given + let maxAge: TimeInterval = 2 + let incrementalSyncDate = Date().addingTimeInterval(-(maxAge - 0.2)) // Just within max age + let fullSyncDate = Date().addingTimeInterval(-3600) + mockPersistenceService.loadSiteResult = .success(POSSite( + siteID: sampleSiteID, + lastIncrementalSyncDate: incrementalSyncDate, + lastFullSyncDate: fullSyncDate + )) + let sut = POSCatalogSyncCoordinator( + fullSyncService: mockSyncService, + incrementalSyncService: mockIncrementalSyncService, + persistenceService: mockPersistenceService, + grdbManager: grdbManager, + maxIncrementalSyncAge: maxAge + ) + + // When + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: true) + + // Then + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) + #expect(mockIncrementalSyncService.lastSyncSiteID == sampleSiteID) + #expect(mockIncrementalSyncService.lastFullSyncDate == fullSyncDate) + } + + @Test func performIncrementalSyncIfApplicable_throws_error_when_sync_already_in_progress() async throws { + // Given + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-3600))) + mockIncrementalSyncService.blockNextSync() + + // Start first incremental sync (it will block) + let firstSyncTask = Task { + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + } + + // Give first sync a moment to start and get blocked + try await Task.sleep(nanoseconds: 10_000_000) // 10ms + + // When - try to start second incremental sync while first is blocked + do { + _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + #expect(Bool(false), "Should have thrown syncAlreadyInProgress error") + } catch let error as POSCatalogSyncError { + // Then + #expect(error == POSCatalogSyncError.syncAlreadyInProgress(siteID: sampleSiteID)) + } + + // Cleanup + mockIncrementalSyncService.resumeBlockedSync() + _ = try await firstSyncTask.value + } + + @Test func performIncrementalSyncIfApplicable_allows_concurrent_syncs_for_different_sites() async throws { + // Given + let siteA: Int64 = 123 + let siteB: Int64 = 456 + let fullSyncDate = Date().addingTimeInterval(-3600) + + mockPersistenceService.siteResults[siteA] = POSSite(siteID: siteA, lastFullSyncDate: fullSyncDate) + mockPersistenceService.siteResults[siteB] = POSSite(siteID: siteB, lastFullSyncDate: fullSyncDate) + + // When + async let syncA: () = sut.performIncrementalSyncIfApplicable(for: siteA, forceSync: false) + async let syncB: () = sut.performIncrementalSyncIfApplicable(for: siteB, forceSync: false) + + // Then + try await syncA + try await syncB + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 2) + } + + @Test func performIncrementalSyncIfApplicable_propagates_errors() async throws { + // Given + let fullSyncDate = Date().addingTimeInterval(-3600) + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate)) + + let expectedError = NSError(domain: "incremental_sync", code: 500, userInfo: [NSLocalizedDescriptionKey: "Incremental sync failed"]) + mockIncrementalSyncService.startIncrementalSyncResult = .failure(expectedError) + + // When/Then + await #expect(throws: expectedError) { + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + } + } + + @Test(arguments: [true, false]) + func performIncrementalSyncIfApplicable_incremental_tracking_cleaned_up_on_error(forceSync: Bool) async throws { + // Given + let fullSyncDate = Date().addingTimeInterval(-3600) + mockPersistenceService.loadSiteResult = .success(POSSite(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate)) + + let expectedError = NSError(domain: "test", code: 1, userInfo: nil) + mockIncrementalSyncService.startIncrementalSyncResult = .failure(expectedError) + + // When - incremental sync fails + do { + _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + #expect(Bool(false), "Should have thrown error") + } catch { + // Expected error + } + + // Then - subsequent incremental sync should be allowed + mockIncrementalSyncService.startIncrementalSyncResult = .success(()) + + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 2) + } + // MARK: - Helper Methods private func createSiteInDatabase(siteID: Int64) throws { From 45e760851689ef55500284c72274cc8ad0faffef Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Thu, 11 Sep 2025 14:01:47 +0800 Subject: [PATCH 10/12] Add periphery ignore comment to `performIncrementalSyncIfApplicable`. --- .../Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index 0254cd7cda3..a1b906f7213 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -20,6 +20,7 @@ public protocol POSCatalogSyncCoordinatorProtocol { /// - siteID: The site ID to sync catalog for /// - forceSync: Whether to bypass age checks and always sync /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site + //periphery:ignore - remove ignore comment when incremental sync is integrated with POS func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws } From 3cb66b6bd51ce030e50a1750fe9d13c75a3efe44 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Thu, 11 Sep 2025 11:59:41 +0100 Subject: [PATCH 11/12] Remove unused persistence service --- .../Tools/POS/POSCatalogSyncCoordinator.swift | 15 --------------- .../POS/POSCatalogSyncCoordinatorTests.swift | 8 +------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index ffdbd335be4..989a5fa6ae1 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -31,7 +31,6 @@ public enum POSCatalogSyncError: Error, Equatable { public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { private let fullSyncService: POSCatalogFullSyncServiceProtocol private let incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol - private let persistenceService: POSCatalogPersistenceServiceProtocol private let grdbManager: GRDBManagerProtocol private let maxIncrementalSyncAge: TimeInterval @@ -46,20 +45,6 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { maxIncrementalSyncAge: TimeInterval = 300) { self.fullSyncService = fullSyncService self.incrementalSyncService = incrementalSyncService - self.persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager) - self.grdbManager = grdbManager - self.maxIncrementalSyncAge = maxIncrementalSyncAge - } - - //periphery:ignore - used for tests to inject persistence service - init(fullSyncService: POSCatalogFullSyncServiceProtocol, - incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol, - persistenceService: POSCatalogPersistenceServiceProtocol, - grdbManager: GRDBManagerProtocol, - maxIncrementalSyncAge: TimeInterval = 300) { - self.fullSyncService = fullSyncService - self.incrementalSyncService = incrementalSyncService - self.persistenceService = persistenceService self.grdbManager = grdbManager self.maxIncrementalSyncAge = maxIncrementalSyncAge } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index ac25c08e3de..7cc1c0d6e0d 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -6,7 +6,6 @@ import Testing struct POSCatalogSyncCoordinatorTests { private let mockSyncService: MockPOSCatalogFullSyncService private let mockIncrementalSyncService: MockPOSCatalogIncrementalSyncService - private let mockPersistenceService: MockPOSCatalogPersistenceService private let grdbManager: GRDBManager private let sut: POSCatalogSyncCoordinator private let sampleSiteID: Int64 = 134 @@ -14,12 +13,10 @@ struct POSCatalogSyncCoordinatorTests { init() throws { self.mockSyncService = MockPOSCatalogFullSyncService() self.mockIncrementalSyncService = MockPOSCatalogIncrementalSyncService() - self.mockPersistenceService = MockPOSCatalogPersistenceService() self.grdbManager = try GRDBManager() self.sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, - persistenceService: mockPersistenceService, grdbManager: grdbManager ) } @@ -248,7 +245,6 @@ struct POSCatalogSyncCoordinatorTests { let sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, - persistenceService: mockPersistenceService, grdbManager: grdbManager, maxIncrementalSyncAge: maxAge ) @@ -271,7 +267,6 @@ struct POSCatalogSyncCoordinatorTests { let sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, - persistenceService: mockPersistenceService, grdbManager: grdbManager, maxIncrementalSyncAge: maxAge ) @@ -304,11 +299,10 @@ struct POSCatalogSyncCoordinatorTests { let incrementalSyncDate = Date().addingTimeInterval(-(maxAge - 0.2)) // Just within max age let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate, lastIncrementalSyncDate: incrementalSyncDate) - + let sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, - persistenceService: mockPersistenceService, grdbManager: grdbManager, maxIncrementalSyncAge: maxAge ) From c6aa59ad647fd66a525c4b6b8ec9c1dd19e2dcca Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Thu, 11 Sep 2025 12:09:15 +0100 Subject: [PATCH 12/12] Ignore --- .../Yosemite/Model/Storage/PersistedSite+Conversions.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift b/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift index b733f205d3e..80378e90466 100644 --- a/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift +++ b/Modules/Sources/Yosemite/Model/Storage/PersistedSite+Conversions.swift @@ -1,3 +1,4 @@ +// periphery:ignore:all import Foundation import Storage