Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Foundation
import Storage
import GRDB

public protocol POSCatalogSyncCoordinatorProtocol {
/// Performs a full catalog sync for the specified site
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
}
}
}
Original file line number Diff line number Diff line change
@@ -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
)
}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -126,17 +149,55 @@ 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)

// 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
Expand Down
2 changes: 1 addition & 1 deletion WooCommerce/Classes/ViewRelated/MainTabBarController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down