diff --git a/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift b/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift index d22b7a6bbe6..6f4a012f368 100644 --- a/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift +++ b/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift @@ -609,16 +609,12 @@ final class POSPreviewCatalogSettingsService: POSCatalogSettingsServiceProtocol } final class POSPreviewCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { - func performFullSync(for siteID: Int64) async throws { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { // Simulates a full sync operation with a 1 second delay. try await Task.sleep(nanoseconds: 1_000_000_000) } - func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) async -> Bool { - true - } - - func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws { + func performIncrementalSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { // Simulates an incremental sync operation with a 0.5 second delay. try await Task.sleep(nanoseconds: 500_000_000) } diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index afcf920502e..a1b94d8311b 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -4,36 +4,41 @@ import Storage import GRDB public protocol POSCatalogSyncCoordinatorProtocol { - /// Performs a full catalog sync for the specified site - /// - Parameter siteID: The site ID to sync catalog for - /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site - func performFullSync(for siteID: Int64) async throws - - /// Determines if a full sync should be performed based on the age of the last sync + /// Performs a full catalog sync if applicable for the specified site /// - Parameters: - /// - siteID: The site ID to check + /// - siteID: The site ID to sync catalog for /// - 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 + /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws /// 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 + /// - maxAge: Maximum age before a sync is considered stale /// - 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 + func performIncrementalSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws +} + +public extension POSCatalogSyncCoordinatorProtocol { + func performFullSync(for siteID: Int64) async throws { + try await performFullSyncIfApplicable(for: siteID, maxAge: .zero) + } + + func performIncrementalSync(for siteID: Int64) async throws { + try await performIncrementalSyncIfApplicable(for: siteID, maxAge: .zero) + } } public enum POSCatalogSyncError: Error, Equatable { case syncAlreadyInProgress(siteID: Int64) + case negativeMaxAge } public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { private let fullSyncService: POSCatalogFullSyncServiceProtocol private let incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol private let grdbManager: GRDBManagerProtocol - private let maxIncrementalSyncAge: TimeInterval private let catalogSizeLimit: Int private let catalogSizeChecker: POSCatalogSizeCheckerProtocol @@ -45,18 +50,24 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { public init(fullSyncService: POSCatalogFullSyncServiceProtocol, incrementalSyncService: POSCatalogIncrementalSyncServiceProtocol, grdbManager: GRDBManagerProtocol, - maxIncrementalSyncAge: TimeInterval = 300, catalogSizeLimit: Int? = nil, catalogSizeChecker: POSCatalogSizeCheckerProtocol) { self.fullSyncService = fullSyncService self.incrementalSyncService = incrementalSyncService self.grdbManager = grdbManager - self.maxIncrementalSyncAge = maxIncrementalSyncAge self.catalogSizeLimit = catalogSizeLimit ?? Constants.defaultSizeLimitForPOSCatalog self.catalogSizeChecker = catalogSizeChecker } - public func performFullSync(for siteID: Int64) async throws { + public func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + guard maxAge >= 0 else { + throw POSCatalogSyncError.negativeMaxAge + } + + guard await shouldPerformFullSync(for: siteID, maxAge: maxAge) else { + return + } + if ongoingSyncs.contains(siteID) { DDLogInfo("⚠ī¸ POSCatalogSyncCoordinator: Sync already in progress for site \(siteID)") throw POSCatalogSyncError.syncAlreadyInProgress(siteID: siteID) @@ -82,8 +93,8 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { /// - siteID: The site ID to check /// - maxAge: Maximum age before a sync is considered stale /// - Returns: True if a sync should be performed - public func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) async -> Bool { - return await shouldPerformFullSync(for: siteID, maxAge: maxAge, maxCatalogSize: catalogSizeLimit) + private func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) async -> Bool { + await shouldPerformFullSync(for: siteID, maxAge: maxAge, maxCatalogSize: catalogSizeLimit) } private func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval, maxCatalogSize: Int) async -> Bool { @@ -118,33 +129,28 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { /// 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 + /// - maxAge: Maximum age before a sync is considered stale /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site - public func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws { - try await performIncrementalSyncIfApplicable(for: siteID, forceSync: forceSync, maxCatalogSize: catalogSizeLimit) + public func performIncrementalSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + try await performIncrementalSyncIfApplicable(for: siteID, maxAge: maxAge, maxCatalogSize: catalogSizeLimit) } - private func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool, maxCatalogSize: Int) async throws { - if ongoingIncrementalSyncs.contains(siteID) { - DDLogInfo("⚠ī¸ POSCatalogSyncCoordinator: Incremental sync already in progress for site \(siteID)") - throw POSCatalogSyncError.syncAlreadyInProgress(siteID: siteID) + private func performIncrementalSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, maxCatalogSize: Int) async throws { + guard maxAge >= 0 else { + throw POSCatalogSyncError.negativeMaxAge } - guard await isCatalogSizeWithinLimit(for: siteID, maxCatalogSize: maxCatalogSize) else { + guard await shouldPerformIncrementalSync(for: siteID, maxAge: maxAge, maxCatalogSize: maxCatalogSize) else { return } - guard let lastFullSyncDate = await lastFullSyncDate(for: siteID) else { - DDLogInfo("📋 POSCatalogSyncCoordinator: No full sync performed yet for site \(siteID), skipping incremental sync") - return + if ongoingIncrementalSyncs.contains(siteID) { + DDLogInfo("⚠ī¸ POSCatalogSyncCoordinator: Incremental sync already in progress for site \(siteID)") + throw POSCatalogSyncError.syncAlreadyInProgress(siteID: siteID) } - 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") - } + guard let lastFullSyncDate = await lastFullSyncDate(for: siteID) else { + return } ongoingIncrementalSyncs.insert(siteID) @@ -162,6 +168,28 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { DDLogInfo("✅ POSCatalogSyncCoordinator completed incremental sync for site \(siteID)") } + private func shouldPerformIncrementalSync(for siteID: Int64, maxAge: TimeInterval, maxCatalogSize: Int) async -> Bool { + guard await isCatalogSizeWithinLimit(for: siteID, maxCatalogSize: maxCatalogSize) else { + return false + } + + guard await lastFullSyncDate(for: siteID) != nil else { + DDLogInfo("📋 POSCatalogSyncCoordinator: No full sync performed yet for site \(siteID), skipping incremental sync") + return false + } + + if maxAge > 0, let lastIncrementalSyncDate = await lastIncrementalSyncDate(for: siteID) { + let age = Date().timeIntervalSince(lastIncrementalSyncDate) + + if age <= maxAge { + DDLogInfo("📋 POSCatalogSyncCoordinator: Last incremental sync for site \(siteID) was \(Int(age))s ago, sync not needed") + return false + } + } + + return true + } + // MARK: - Private /// Checks if the catalog size is within the specified sync limit diff --git a/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift b/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift index 37bc800cc9a..eed86e0d9ad 100644 --- a/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift +++ b/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift @@ -7,11 +7,7 @@ final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { var performFullSyncResult: Result = .success(()) var shouldDelayResponse = false - func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) async -> Bool { - true - } - - func performFullSync(for siteID: Int64) async throws { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { if shouldDelayResponse { try await Task.sleep(for: .milliseconds(100)) } @@ -27,5 +23,5 @@ final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { } } - func performIncrementalSyncIfApplicable(for siteID: Int64, forceSync: Bool) async throws {} + func performIncrementalSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws {} } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index d33a84844e6..5705805f6f6 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -10,6 +10,7 @@ struct POSCatalogSyncCoordinatorTests { private let mockCatalogSizeChecker: MockPOSCatalogSizeChecker private let sut: POSCatalogSyncCoordinator private let sampleSiteID: Int64 = 134 + private let sampleMaxAge: TimeInterval = 60 * 60 init() throws { self.mockSyncService = MockPOSCatalogFullSyncService() @@ -56,53 +57,65 @@ struct POSCatalogSyncCoordinatorTests { // MARK: - Should Sync Decision Tests - @Test func shouldPerformFullSync_returns_true_when_site_is_not_in_database_with_no_sync_history() async { + @Test func performFullSyncIfApplicable_throws_error_when_max_age_is_negative() async throws { + // Given + try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-30 * 60)) + + // When/Then + await #expect(throws: POSCatalogSyncError.negativeMaxAge) { + let _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: -0.01) + } + + #expect(mockSyncService.startFullSyncCallCount == 0) + } + + @Test func performFullSyncIfApplicable_starts_sync_when_site_is_not_in_database_with_no_sync_history() async throws { // Given - site does not exist in database // Note: not creating site in database so it won't exist // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSync(for: sampleSiteID) // Then - should sync because site doesn't exist in database - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) } - @Test func shouldPerformFullSync_returns_true_when_site_has_no_previous_sync() async throws { + @Test func performFullSyncIfApplicable_starts_sync_when_site_has_no_previous_sync() async throws { // Given - site exists in database but has no previous sync date try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 3600) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) } - @Test func shouldPerformFullSync_returns_true_when_sync_is_stale() async throws { + @Test func performFullSyncIfApplicable_starts_sync_when_sync_is_stale() async throws { // Given - previous sync was 2 hours ago - let twoHoursAgo = Date().addingTimeInterval(-2 * 60 * 60) + let twoHoursAgo = Date().addingTimeInterval(-2 * sampleMaxAge) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: twoHoursAgo) // When - max age is 1 hour - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) } - @Test func shouldPerformFullSync_returns_false_when_sync_is_fresh() async throws { + @Test func performFullSyncIfApplicable_skips_sync_when_sync_is_fresh() async throws { // Given - previous sync was 30 minutes ago let thirtyMinutesAgo = Date().addingTimeInterval(-30 * 60) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: thirtyMinutesAgo) // When - max age is 1 hour - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == false) + #expect(mockSyncService.startFullSyncCallCount == 0) } - @Test func shouldPerformFullSync_handles_different_sites_independently() async throws { + @Test func performFullSyncIfApplicable_handles_different_sites_independently() async throws { // Given let siteA: Int64 = 123 let siteB: Int64 = 456 @@ -113,122 +126,122 @@ struct POSCatalogSyncCoordinatorTests { try createSiteInDatabase(siteID: siteB, lastFullSyncDate: nil) // When - let shouldSyncA = await sut.shouldPerformFullSync(for: siteA, maxAge: 2 * 60 * 60) // 2 hours - let shouldSyncB = await sut.shouldPerformFullSync(for: siteB, maxAge: 2 * 60 * 60) // 2 hours + let _ = try await sut.performFullSyncIfApplicable(for: siteA, maxAge: 2 * sampleMaxAge) + let _ = try await sut.performFullSyncIfApplicable(for: siteB, maxAge: 2 * sampleMaxAge) // Then - #expect(shouldSyncA == false) // Recent sync exists - #expect(shouldSyncB == true) // No previous sync + #expect(mockSyncService.startFullSyncCallCount == 1) + #expect(mockSyncService.lastSyncSiteID == siteB) } - @Test func shouldPerformFullSync_with_zero_maxAge_always_returns_true() async throws { + @Test func performFullSyncIfApplicable_with_zero_maxAge_skips_age_check() async throws { // Given - previous sync was just now let justNow = Date() try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: justNow) // When - max age is 0 (always sync) - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 0) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: 0) // Then - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) } // MARK: - Catalog Size Check Tests - @Test func shouldPerformFullSync_returns_false_when_catalog_size_exceeds_limit() async throws { + @Test func performFullSyncIfApplicable_skips_sync_when_catalog_size_exceeds_limit() async throws { // Given - catalog size is above the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 800, variationCount: 300)) // 1100 total try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == false) + #expect(mockSyncService.startFullSyncCallCount == 0) #expect(mockCatalogSizeChecker.checkCatalogSizeCallCount == 1) #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test func shouldPerformFullSync_returns_true_when_catalog_size_is_at_limit() async throws { + @Test func performFullSyncIfApplicable_starts_sync_when_catalog_size_is_at_limit() async throws { // Given - catalog size is exactly at the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 600, variationCount: 400)) // 1000 total try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) #expect(mockCatalogSizeChecker.checkCatalogSizeCallCount == 1) #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test func shouldPerformFullSync_returns_true_when_catalog_size_is_under_limit() async throws { + @Test func performFullSyncIfApplicable_starts_sync_when_catalog_size_is_under_limit() async throws { // Given - catalog size is below the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 300, variationCount: 200)) // 500 total try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) #expect(mockCatalogSizeChecker.checkCatalogSizeCallCount == 1) #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test func shouldPerformFullSync_returns_false_when_catalog_size_check_fails() async throws { + @Test func performFullSyncIfApplicable_skips_sync_when_catalog_size_check_fails() async throws { // Given - catalog size check throws an error let sizeCheckError = NSError(domain: "size_check", code: 500, userInfo: [NSLocalizedDescriptionKey: "Network error"]) mockCatalogSizeChecker.checkCatalogSizeResult = .failure(sizeCheckError) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - #expect(shouldSync == false) + #expect(mockSyncService.startFullSyncCallCount == 0) #expect(mockCatalogSizeChecker.checkCatalogSizeCallCount == 1) #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test func shouldPerformFullSync_respects_time_only_when_catalog_size_is_acceptable() async throws { + @Test func performFullSyncIfApplicable_respects_time_only_when_catalog_size_is_acceptable() async throws { // Given - catalog size is acceptable but sync is recent mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 200, variationCount: 100)) // 300 total let thirtyMinutesAgo = Date().addingTimeInterval(-30 * 60) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: thirtyMinutesAgo) // When - max age is 1 hour - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - should not sync because time hasn't passed yet - #expect(shouldSync == false) + #expect(mockSyncService.startFullSyncCallCount == 0) #expect(mockCatalogSizeChecker.checkCatalogSizeCallCount == 1) } // MARK: - Database Check Tests - @Test func shouldPerformFullSync_returns_true_when_site_not_in_database() async { + @Test func performFullSyncIfApplicable_starts_sync_when_site_not_in_database() async throws { // Given - site does not exist in database, but has recent sync date // Note: not creating site in database so it won't exist // When - max age is 1 hour (normally wouldn't sync) - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - should sync because site doesn't exist in database - #expect(shouldSync == true) + #expect(mockSyncService.startFullSyncCallCount == 1) } - @Test func shouldPerformFullSync_respects_time_when_site_exists_in_database() async throws { + @Test func performFullSyncIfApplicable_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 try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: recentSyncDate) // When - max age is 1 hour - let shouldSync = await sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + let _ = try await sut.performFullSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) // Then - should not sync because site exists and time hasn't passed - #expect(shouldSync == false) + #expect(mockSyncService.startFullSyncCallCount == 0) } // MARK: - Sync Tracking Tests @@ -299,13 +312,25 @@ struct POSCatalogSyncCoordinatorTests { // MARK: - Incremental Sync Tests - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_skips_sync_when_no_full_sync_performed(forceSync: Bool) async throws { + @Test func performIncrementalSyncIfApplicable_throws_error_when_max_age_is_negative() async throws { + // Given + try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-30 * 60)) + + // When/Then + await #expect(throws: POSCatalogSyncError.negativeMaxAge) { + let _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: -0.01) + } + + #expect(mockSyncService.startFullSyncCallCount == 0) + } + + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_skips_sync_when_no_full_sync_performed(maxAge: TimeInterval) async throws { // Given - site exists but has no full sync date try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: nil) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) @@ -322,21 +347,19 @@ struct POSCatalogSyncCoordinatorTests { fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, grdbManager: grdbManager, - maxIncrementalSyncAge: maxAge, catalogSizeChecker: mockCatalogSizeChecker ) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_performs_sync_when_incremental_sync_is_stale(forceSync: Bool) async throws { + @Test(arguments: [.zero, 2]) + func performIncrementalSyncIfApplicable_performs_sync_when_incremental_sync_is_stale(maxAge: TimeInterval) async throws { // Given - let maxAge: TimeInterval = 2 let incrementalSyncDate = Date().addingTimeInterval(-(maxAge + 0.2)) // Just above max age let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate, lastIncrementalSyncDate: incrementalSyncDate) @@ -345,36 +368,34 @@ struct POSCatalogSyncCoordinatorTests { fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, grdbManager: grdbManager, - maxIncrementalSyncAge: maxAge, catalogSizeChecker: mockCatalogSizeChecker ) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) #expect(mockIncrementalSyncService.lastSyncSiteID == sampleSiteID) } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_performs_sync_when_no_incremental_sync_date(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_performs_sync_when_no_incremental_sync_date(maxAge: TimeInterval) async throws { // Given let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate, lastIncrementalSyncDate: nil) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) #expect(mockIncrementalSyncService.lastSyncSiteID == sampleSiteID) } - @Test func performIncrementalSyncIfApplicable_forceSync_bypasses_age_check() async throws { + @Test func performIncrementalSync_bypasses_age_check() async throws { // Given - let maxAge: TimeInterval = 2 - let incrementalSyncDate = Date().addingTimeInterval(-(maxAge - 0.2)) // Just within max age + let incrementalSyncDate = Date().addingTimeInterval(-5000) // A long time since the last incremental sync let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate, lastIncrementalSyncDate: incrementalSyncDate) @@ -382,12 +403,11 @@ struct POSCatalogSyncCoordinatorTests { fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, grdbManager: grdbManager, - maxIncrementalSyncAge: maxAge, catalogSizeChecker: mockCatalogSizeChecker ) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: true) + try await sut.performIncrementalSync(for: sampleSiteID) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) @@ -402,7 +422,7 @@ struct POSCatalogSyncCoordinatorTests { // Start first incremental sync (it will block) let firstSyncTask = Task { - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) } // Give first sync a moment to start and get blocked @@ -410,7 +430,7 @@ struct POSCatalogSyncCoordinatorTests { // When - try to start second incremental sync while first is blocked do { - _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) #expect(Bool(false), "Should have thrown syncAlreadyInProgress error") } catch let error as POSCatalogSyncError { // Then @@ -432,8 +452,8 @@ struct POSCatalogSyncCoordinatorTests { try createSiteInDatabase(siteID: siteB, lastFullSyncDate: fullSyncDate) // When - async let syncA: () = sut.performIncrementalSyncIfApplicable(for: siteA, forceSync: false) - async let syncB: () = sut.performIncrementalSyncIfApplicable(for: siteB, forceSync: false) + async let syncA: () = sut.performIncrementalSyncIfApplicable(for: siteA, maxAge: sampleMaxAge) + async let syncB: () = sut.performIncrementalSyncIfApplicable(for: siteB, maxAge: sampleMaxAge) // Then try await syncA @@ -451,12 +471,12 @@ struct POSCatalogSyncCoordinatorTests { // When/Then await #expect(throws: expectedError) { - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) } } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_incremental_tracking_cleaned_up_on_error(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_incremental_tracking_cleaned_up_on_error(maxAge: TimeInterval) async throws { // Given let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate) @@ -466,7 +486,7 @@ struct POSCatalogSyncCoordinatorTests { // When - incremental sync fails do { - _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + _ = try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) #expect(Bool(false), "Should have thrown error") } catch { // Expected error @@ -475,21 +495,21 @@ struct POSCatalogSyncCoordinatorTests { // Then - subsequent incremental sync should be allowed mockIncrementalSyncService.startIncrementalSyncResult = .success(()) - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: sampleMaxAge) #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 2) } // MARK: - Incremental Sync Catalog Size Tests - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_skips_sync_when_catalog_size_exceeds_limit(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_skips_sync_when_catalog_size_exceeds_limit(maxAge: TimeInterval) async throws { // Given - catalog size is above the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 700, variationCount: 400)) // 1100 total let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) @@ -497,15 +517,15 @@ struct POSCatalogSyncCoordinatorTests { #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_performs_sync_when_catalog_size_is_at_limit(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_performs_sync_when_catalog_size_is_at_limit(maxAge: TimeInterval) async throws { // Given - catalog size is exactly at the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 500, variationCount: 500)) // 1000 total let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) @@ -513,15 +533,15 @@ struct POSCatalogSyncCoordinatorTests { #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_performs_sync_when_catalog_size_is_under_limit(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_performs_sync_when_catalog_size_is_under_limit(maxAge: TimeInterval) async throws { // Given - catalog size is below the 1000 item limit mockCatalogSizeChecker.checkCatalogSizeResult = .success(POSCatalogSize(productCount: 200, variationCount: 150)) // 350 total let fullSyncDate = Date().addingTimeInterval(-3600) try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) @@ -529,8 +549,8 @@ struct POSCatalogSyncCoordinatorTests { #expect(mockCatalogSizeChecker.lastCheckedSiteID == sampleSiteID) } - @Test(arguments: [true, false]) - func performIncrementalSyncIfApplicable_skips_sync_when_catalog_size_check_fails(forceSync: Bool) async throws { + @Test(arguments: [.zero, 60 * 60]) + func performIncrementalSyncIfApplicable_skips_sync_when_catalog_size_check_fails(maxAge: TimeInterval) async throws { // Given - catalog size check throws an error let sizeCheckError = NSError(domain: "size_check", code: 500, userInfo: [NSLocalizedDescriptionKey: "Network error"]) mockCatalogSizeChecker.checkCatalogSizeResult = .failure(sizeCheckError) @@ -538,7 +558,7 @@ struct POSCatalogSyncCoordinatorTests { try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: fullSyncDate) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: forceSync) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then - should skip sync when size check fails #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) @@ -558,12 +578,11 @@ struct POSCatalogSyncCoordinatorTests { fullSyncService: mockSyncService, incrementalSyncService: mockIncrementalSyncService, grdbManager: grdbManager, - maxIncrementalSyncAge: maxAge, catalogSizeChecker: mockCatalogSizeChecker ) // When - try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, forceSync: false) + try await sut.performIncrementalSyncIfApplicable(for: sampleSiteID, maxAge: maxAge) // Then - should skip sync due to size limit, regardless of age #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 0) diff --git a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift index 6fd64a45932..237d986542c 100644 --- a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift +++ b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift @@ -859,14 +859,11 @@ private extension MainTabBarController { // Check if sync is needed (older than 24 hours) let maxAge: TimeInterval = 24 * 60 * 60 - guard await coordinator.shouldPerformFullSync(for: siteID, maxAge: maxAge) else { - return - } // Perform background sync Task.detached { do { - _ = try await coordinator.performFullSync(for: siteID) + _ = try await coordinator.performFullSyncIfApplicable(for: siteID, maxAge: maxAge) } catch POSCatalogSyncError.syncAlreadyInProgress { DDLogInfo("ℹī¸ POS catalog sync already in progress for site \(siteID), skipping") } catch {