diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index 1a7004a460e..4416be7eed7 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -1,5 +1,6 @@ import Foundation import Storage +import GRDB public protocol POSCatalogSyncCoordinatorProtocol { /// Performs a full catalog sync for the specified site @@ -17,11 +18,14 @@ public protocol POSCatalogSyncCoordinatorProtocol { public final class POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { private let fullSyncService: POSCatalogFullSyncServiceProtocol private let settingsStore: SiteSpecificAppSettingsStoreMethodsProtocol + private let grdbManager: GRDBManagerProtocol public init(fullSyncService: POSCatalogFullSyncServiceProtocol, - settingsStore: SiteSpecificAppSettingsStoreMethodsProtocol? = nil) { + settingsStore: SiteSpecificAppSettingsStoreMethodsProtocol? = nil, + grdbManager: GRDBManagerProtocol) { self.fullSyncService = fullSyncService self.settingsStore = settingsStore ?? SiteSpecificAppSettingsStoreMethods(fileStorage: PListFileStorage()) + self.grdbManager = grdbManager } public func performFullSync(for siteID: Int64) async throws { @@ -36,6 +40,11 @@ public final class POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol } public func shouldPerformFullSync(for siteID: Int64, maxAge: TimeInterval) -> Bool { + if !siteExistsInDatabase(siteID: siteID) { + DDLogInfo("📋 POSCatalogSyncCoordinator: Site \(siteID) not found in database, sync needed") + return true + } + guard let lastSyncDate = lastFullSyncDate(for: siteID) else { DDLogInfo("📋 POSCatalogSyncCoordinator: No previous sync found for site \(siteID), sync needed") return true @@ -58,4 +67,16 @@ public final class POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol private func lastFullSyncDate(for siteID: Int64) -> Date? { return settingsStore.getPOSLastFullSyncDate(for: siteID) } + + private func siteExistsInDatabase(siteID: Int64) -> Bool { + do { + return try grdbManager.databaseConnection.read { db in + return try PersistedSite.filter(key: siteID).fetchCount(db) > 0 + } + } catch { + DDLogError("⛔️ POSCatalogSyncCoordinator: Error checking if site \(siteID) exists in database: \(error)") + // On error, assume site exists to avoid unnecessary syncs + return true + } + } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index 1953d2ae700..5cf006ca67a 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -1,20 +1,23 @@ import Foundation import Testing @testable import Yosemite -import Storage +@testable import Storage struct POSCatalogSyncCoordinatorTests { private let mockSyncService: MockPOSCatalogFullSyncService private let mockSettingsStore: MockSiteSpecificAppSettingsStoreMethods + private let grdbManager: GRDBManager private let sut: POSCatalogSyncCoordinator private let sampleSiteID: Int64 = 134 - init() { + init() throws { self.mockSyncService = MockPOSCatalogFullSyncService() self.mockSettingsStore = MockSiteSpecificAppSettingsStoreMethods() + self.grdbManager = try GRDBManager() self.sut = POSCatalogSyncCoordinator( fullSyncService: mockSyncService, - settingsStore: mockSettingsStore + settingsStore: mockSettingsStore, + grdbManager: grdbManager ) } @@ -72,9 +75,23 @@ struct POSCatalogSyncCoordinatorTests { // MARK: - Should Sync Decision Tests - @Test func shouldPerformFullSync_returns_true_when_no_previous_sync() { - // Given - no previous sync date stored + @Test func shouldPerformFullSync_returns_true_when_site_is_not_in_database_with_no_sync_history() { + // Given - site doesn't exist in database AND has no sync history mockSettingsStore.storedDates = [:] + // Note: NOT creating site in database + + // When + let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + + // Then - should sync because site doesn't exist in database + #expect(shouldSync == true) + } + + @Test func shouldPerformFullSync_returns_true_when_site_is_in_database_with_no_previous_sync() 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 = [:] + try createSiteInDatabase(siteID: sampleSiteID) // When let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 3600) @@ -84,10 +101,11 @@ struct POSCatalogSyncCoordinatorTests { #expect(mockSettingsStore.getPOSLastFullSyncDateCallCount == 1) } - @Test func shouldPerformFullSync_returns_true_when_sync_is_stale() { - // Given - previous sync was 2 hours ago + @Test func shouldPerformFullSync_returns_true_when_sync_is_stale() throws { + // Given - previous sync was 2 hours ago, and site exists in database let twoHoursAgo = Date().addingTimeInterval(-2 * 60 * 60) mockSettingsStore.storedDates[sampleSiteID] = twoHoursAgo + try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 1 hour let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) @@ -96,10 +114,11 @@ struct POSCatalogSyncCoordinatorTests { #expect(shouldSync == true) } - @Test func shouldPerformFullSync_returns_false_when_sync_is_fresh() { - // Given - previous sync was 30 minutes ago + @Test func shouldPerformFullSync_returns_false_when_sync_is_fresh() throws { + // Given - previous sync was 30 minutes ago, and site exists in database let thirtyMinutesAgo = Date().addingTimeInterval(-30 * 60) mockSettingsStore.storedDates[sampleSiteID] = thirtyMinutesAgo + try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 1 hour let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) @@ -108,7 +127,7 @@ struct POSCatalogSyncCoordinatorTests { #expect(shouldSync == false) } - @Test func shouldPerformFullSync_handles_different_sites_independently() { + @Test func shouldPerformFullSync_handles_different_sites_independently() throws { // Given let siteA: Int64 = 123 let siteB: Int64 = 456 @@ -117,6 +136,10 @@ struct POSCatalogSyncCoordinatorTests { mockSettingsStore.storedDates[siteA] = oneHourAgo // Has previous sync // siteB has no previous sync + // Create both sites in database to test timing logic + try createSiteInDatabase(siteID: siteA) + try createSiteInDatabase(siteID: siteB) + // When let shouldSyncA = sut.shouldPerformFullSync(for: siteA, maxAge: 2 * 60 * 60) // 2 hours let shouldSyncB = sut.shouldPerformFullSync(for: siteB, maxAge: 2 * 60 * 60) // 2 hours @@ -126,10 +149,11 @@ struct POSCatalogSyncCoordinatorTests { #expect(shouldSyncB == true) // No previous sync } - @Test func shouldPerformFullSync_with_zero_maxAge_always_returns_true() { - // Given - previous sync was just now + @Test func shouldPerformFullSync_with_zero_maxAge_always_returns_true() throws { + // Given - previous sync was just now, and site exists in database let justNow = Date() mockSettingsStore.storedDates[sampleSiteID] = justNow + try createSiteInDatabase(siteID: sampleSiteID) // When - max age is 0 (always sync) let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 0) @@ -137,6 +161,43 @@ struct POSCatalogSyncCoordinatorTests { // Then #expect(shouldSync == true) } + + // MARK: - Database Check Tests + + @Test func shouldPerformFullSync_returns_true_when_site_not_in_database() { + // 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 + // Note: not creating site in database so it won't exist + + // When - max age is 1 hour (normally wouldn't sync) + let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + + // Then - should sync because site doesn't exist in database + #expect(shouldSync == true) + } + + @Test func shouldPerformFullSync_respects_time_when_site_exists_in_database() throws { + // Given - site exists in database with recent sync date + let recentSyncDate = Date().addingTimeInterval(-30 * 60) // 30 minutes ago + mockSettingsStore.storedDates[sampleSiteID] = recentSyncDate + try createSiteInDatabase(siteID: sampleSiteID) + + // When - max age is 1 hour + let shouldSync = sut.shouldPerformFullSync(for: sampleSiteID, maxAge: 60 * 60) + + // Then - should not sync because site exists and time hasn't passed + #expect(shouldSync == false) + } + + // MARK: - Helper Methods + + private func createSiteInDatabase(siteID: Int64) throws { + try grdbManager.databaseConnection.write { db in + let site = PersistedSite(id: siteID) + try site.insert(db) + } + } } // MARK: - Mock Services diff --git a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift index 2ac4e86158f..7d041a0d326 100644 --- a/WooCommerce/Classes/ViewRelated/MainTabBarController.swift +++ b/WooCommerce/Classes/ViewRelated/MainTabBarController.swift @@ -799,7 +799,7 @@ private extension MainTabBarController { return nil } - return POSCatalogSyncCoordinator(fullSyncService: fullSyncService) + return POSCatalogSyncCoordinator(fullSyncService: fullSyncService, grdbManager: ServiceLocator.grdbManager) } func triggerPOSCatalogSyncIfNeeded(for siteID: Int64) async {