diff --git a/Modules/Sources/Experiments/DefaultFeatureFlagService.swift b/Modules/Sources/Experiments/DefaultFeatureFlagService.swift index 95f7d3abef0..217ca91c6f9 100644 --- a/Modules/Sources/Experiments/DefaultFeatureFlagService.swift +++ b/Modules/Sources/Experiments/DefaultFeatureFlagService.swift @@ -102,6 +102,8 @@ public struct DefaultFeatureFlagService: FeatureFlagService { return buildConfig == .localDeveloper || buildConfig == .alpha case .pointOfSaleSettingsCardReaderFlow: return buildConfig == .localDeveloper || buildConfig == .alpha + case .pointOfSaleCatalogAPI: + return false default: return true } diff --git a/Modules/Sources/Experiments/FeatureFlag.swift b/Modules/Sources/Experiments/FeatureFlag.swift index 620a0a55855..677f6ee9ada 100644 --- a/Modules/Sources/Experiments/FeatureFlag.swift +++ b/Modules/Sources/Experiments/FeatureFlag.swift @@ -211,4 +211,8 @@ public enum FeatureFlag: Int { /// Enables card reader connection flow within POS settings /// case pointOfSaleSettingsCardReaderFlow + + /// Enables using the catalog API endpoint for Point of Sale catalog full sync + /// + case pointOfSaleCatalogAPI } diff --git a/Modules/Sources/PointOfSale/Presentation/Settings/POSSettingsLocalCatalogViewModel.swift b/Modules/Sources/PointOfSale/Presentation/Settings/POSSettingsLocalCatalogViewModel.swift index d06de2705b7..f102a0e16a8 100644 --- a/Modules/Sources/PointOfSale/Presentation/Settings/POSSettingsLocalCatalogViewModel.swift +++ b/Modules/Sources/PointOfSale/Presentation/Settings/POSSettingsLocalCatalogViewModel.swift @@ -53,7 +53,7 @@ final class POSSettingsLocalCatalogViewModel { defer { isRefreshingCatalog = false } do { - try await catalogSyncCoordinator.performFullSync(for: siteID) + try await catalogSyncCoordinator.performFullSync(for: siteID, regenerateCatalog: true) await loadCatalogData() } catch { DDLogError("â›”ī¸ POSSettingsLocalCatalog: Failed to refresh catalog: \(error)") diff --git a/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift b/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift index 50904721b16..09edde4fab4 100644 --- a/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift +++ b/Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift @@ -615,7 +615,7 @@ final class POSPreviewCatalogSettingsService: POSCatalogSettingsServiceProtocol } final class POSPreviewCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { - func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, regenerateCatalog: Bool) async throws { // Simulates a full sync operation with a 1 second delay. try await Task.sleep(nanoseconds: 1_000_000_000) } diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift index 77002f485e2..fad9dd05d8a 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift @@ -5,14 +5,17 @@ import class Networking.POSCatalogSyncRemote import Storage import struct Combine.AnyPublisher import struct NetworkingCore.JetpackSite +import struct Networking.POSCatalogResponse // TODO - remove the periphery ignore comment when the catalog is integrated with POS. // periphery:ignore public protocol POSCatalogFullSyncServiceProtocol { /// Starts a full catalog sync process - /// - Parameter siteID: The site ID to sync catalog for + /// - Parameters: + /// - siteID: The site ID to sync catalog for + /// - regenerateCatalog: Whether to force the catalog generation /// - Returns: The synced catalog containing products and variations - func startFullSync(for siteID: Int64) async throws -> POSCatalog + func startFullSync(for siteID: Int64, regenerateCatalog: Bool) async throws -> POSCatalog } /// POS catalog from full sync. @@ -30,12 +33,14 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol private let syncRemote: POSCatalogSyncRemoteProtocol private let persistenceService: POSCatalogPersistenceServiceProtocol private let batchedLoader: BatchedRequestLoader + private let usesCatalogAPI: Bool public convenience init?(credentials: Credentials?, selectedSite: AnyPublisher, appPasswordSupportState: AnyPublisher, batchSize: Int = 2, - grdbManager: GRDBManagerProtocol) { + grdbManager: GRDBManagerProtocol, + usesCatalogAPI: Bool = false) { guard let credentials else { DDLogError("â›”ī¸ Could not create POSCatalogFullSyncService due missing credentials") return nil @@ -45,28 +50,35 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol appPasswordSupportState: appPasswordSupportState) let syncRemote = POSCatalogSyncRemote(network: network) let persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager) - self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService) + self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService, usesCatalogAPI: usesCatalogAPI) } init( syncRemote: POSCatalogSyncRemoteProtocol, batchSize: Int, retryDelay: TimeInterval = 2.0, - persistenceService: POSCatalogPersistenceServiceProtocol + persistenceService: POSCatalogPersistenceServiceProtocol, + usesCatalogAPI: Bool = false ) { self.syncRemote = syncRemote self.persistenceService = persistenceService self.batchedLoader = BatchedRequestLoader(batchSize: batchSize, retryDelay: retryDelay) + self.usesCatalogAPI = usesCatalogAPI } // MARK: - Protocol Conformance - public func startFullSync(for siteID: Int64) async throws -> POSCatalog { - DDLogInfo("🔄 Starting full catalog sync for site ID: \(siteID)") + public func startFullSync(for siteID: Int64, regenerateCatalog: Bool = false) async throws -> POSCatalog { + DDLogInfo("🔄 Starting full catalog sync for site ID: \(siteID) with regenerateCatalog: \(regenerateCatalog)") do { // Sync from network - let catalog = try await loadCatalog(for: siteID, syncRemote: syncRemote) + let catalog: POSCatalog + if usesCatalogAPI { + catalog = try await loadCatalogFromCatalogAPI(for: siteID, syncRemote: syncRemote, regenerateCatalog: regenerateCatalog) + } else { + catalog = try await loadCatalog(for: siteID, syncRemote: syncRemote) + } DDLogInfo("✅ Loaded \(catalog.products.count) products and \(catalog.variations.count) variations for siteID \(siteID)") // Persist to database @@ -102,4 +114,64 @@ private extension POSCatalogFullSyncService { return POSCatalog(products: products, variations: variations, syncDate: syncStartDate) } + func loadCatalogFromCatalogAPI(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol, regenerateCatalog: Bool) async throws -> POSCatalog { + let downloadStartTime = CFAbsoluteTimeGetCurrent() + let catalog = try await downloadCatalog(for: siteID, syncRemote: syncRemote, regenerateCatalog: regenerateCatalog) + let downloadTime = CFAbsoluteTimeGetCurrent() - downloadStartTime + DDLogInfo("đŸŸŖ Catalog download completed - Time: \(String(format: "%.2f", downloadTime))s") + + return .init(products: catalog.products, variations: catalog.variations, syncDate: .init()) + } +} + +private extension POSCatalogFullSyncService { + func downloadCatalog(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol, regenerateCatalog: Bool) async throws -> POSCatalogResponse { + DDLogInfo("đŸŸŖ Starting catalog request...") + + // 1. Requests catalog until download URL is available. + let response = try await syncRemote.requestCatalogGeneration(for: siteID, forceGeneration: regenerateCatalog) + let downloadURL: String? + if let url = response.downloadURL { + downloadURL = url + } else { + // 2. Polls for completion until download URL is available. + downloadURL = try await pollForCatalogCompletion(siteID: siteID, syncRemote: syncRemote) + } + + // 3. Downloads catalog using the provided URL. + guard let downloadURL else { + throw POSCatalogSyncError.invalidData + } + DDLogInfo("đŸŸŖ Catalog ready for download: \(downloadURL)") + return try await syncRemote.downloadCatalog(for: siteID, downloadURL: downloadURL) + } + + func pollForCatalogCompletion(siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol) async throws -> String { + // Each attempt is made 1 second after the last one completes. + let maxAttempts = 1000 + var attempts = 0 + + while attempts < maxAttempts { + let response = try await syncRemote.requestCatalogGeneration(for: siteID, forceGeneration: false) + + switch response.status { + case .complete: + guard let downloadURL = response.downloadURL else { + throw POSCatalogSyncError.invalidData + } + return downloadURL + case .pending, .processing: + // Only logs every 10th attempt to avoid flooding logs for large catalogs. + if attempts % 10 == 0 { + DDLogInfo("đŸŸŖ Catalog request \(response.status)... (attempt \(attempts + 1)/\(maxAttempts))") + } + try await Task.sleep(nanoseconds: 1_000_000_000) // 1 second + attempts += 1 + case .failed: + throw POSCatalogSyncError.generationFailed + } + } + + throw POSCatalogSyncError.timeout + } } diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift index 3f2df716726..e449b93a073 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift @@ -27,6 +27,8 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { DDLogInfo("💾 Persisting catalog with \(catalog.products.count) products and \(catalog.variations.count) variations") + let catalog = filterOrphanedVariations(from: catalog) + try await grdbManager.databaseConnection.write { db in DDLogInfo("đŸ—‘ī¸ Clearing catalog data for site \(siteID)") try PersistedSite.deleteOne(db, key: siteID) @@ -154,6 +156,22 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { } } +private extension POSCatalogPersistenceService { + /// Filters out variations whose parent products are not in the catalog. + /// This can happen when the API returns public variations but their parent products are not public. + func filterOrphanedVariations(from catalog: POSCatalog) -> POSCatalog { + let productIDs = catalog.products.map { $0.productID } + let variations = catalog.variations.filter { variation in + let parentExists = productIDs.contains { $0 == variation.productID } + if !parentExists { + DDLogWarn("Variation \(variation.productVariationID) references missing product \(variation.productID) - it will not be available in POS.") + } + return parentExists + } + return POSCatalog(products: catalog.products, variations: variations, syncDate: catalog.syncDate) + } +} + private extension POSCatalog { var productsToPersist: [PersistedProduct] { products.map { PersistedProduct(from: $0) } diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index 8bfb5c45d69..eeb846c44cf 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -9,8 +9,9 @@ public protocol POSCatalogSyncCoordinatorProtocol { /// - Parameters: /// - siteID: The site ID to sync catalog for /// - maxAge: Maximum age before a sync is considered stale + /// - regenerateCatalog: Whether to always generate a new catalog. If false, a cached catalog will be used if available. /// - Throws: POSCatalogSyncError.syncAlreadyInProgress if a sync is already running for this site - func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, regenerateCatalog: Bool) async throws /// Performs an incremental sync if applicable based on sync conditions /// - Parameters: @@ -37,8 +38,8 @@ public protocol POSCatalogSyncCoordinatorProtocol { } public extension POSCatalogSyncCoordinatorProtocol { - func performFullSync(for siteID: Int64) async throws { - try await performFullSyncIfApplicable(for: siteID, maxAge: .zero) + func performFullSync(for siteID: Int64, regenerateCatalog: Bool = false) async throws { + try await performFullSyncIfApplicable(for: siteID, maxAge: .zero, regenerateCatalog: regenerateCatalog) } func performIncrementalSync(for siteID: Int64) async throws { @@ -56,6 +57,9 @@ public extension POSCatalogSyncCoordinatorProtocol { public enum POSCatalogSyncError: Error, Equatable { case syncAlreadyInProgress(siteID: Int64) case negativeMaxAge + case invalidData + case timeout + case generationFailed case requestCancelled case shouldNotSync } @@ -85,7 +89,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { self.siteSettings = siteSettings ?? SiteSpecificAppSettingsStoreMethods(fileStorage: PListFileStorage()) } - public func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + public func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, regenerateCatalog: Bool) async throws { guard maxAge >= 0 else { throw POSCatalogSyncError.negativeMaxAge } @@ -104,7 +108,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { DDLogInfo("🔄 POSCatalogSyncCoordinator starting full sync for site \(siteID)") do { - _ = try await fullSyncService.startFullSync(for: siteID) + _ = try await fullSyncService.startFullSync(for: siteID, regenerateCatalog: regenerateCatalog) emitSyncState(.syncCompleted(siteID: siteID)) } catch AFError.explicitlyCancelled, is CancellationError { emitSyncState(.syncFailed(siteID: siteID, error: POSCatalogSyncError.requestCancelled)) @@ -127,7 +131,7 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { if Date().timeIntervalSince(lastFullSync) >= fullSyncMaxAge { DDLogInfo("🔄 POSCatalogSyncCoordinator: Performing full sync for site \(siteID) (last full sync: \(lastFullSyncUTC) UTC)") - try await performFullSyncIfApplicable(for: siteID, maxAge: fullSyncMaxAge) + try await performFullSyncIfApplicable(for: siteID, maxAge: fullSyncMaxAge, regenerateCatalog: false) } else { DDLogInfo("🔄 POSCatalogSyncCoordinator: Performing incremental sync for site \(siteID) (last full sync: \(lastFullSyncUTC) UTC)") try await performIncrementalSyncIfApplicable(for: siteID, maxAge: incrementalSyncMaxAge) @@ -341,7 +345,6 @@ public enum POSCatalogSyncState: Equatable { private extension POSCatalogSyncCoordinator { enum Constants { - static let defaultSizeLimitForPOSCatalog = 1000 static let maxDaysSinceLastOpened = 30 } diff --git a/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift b/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift index d0da6326193..c4c9095cfd1 100644 --- a/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift +++ b/Modules/Tests/PointOfSaleTests/Mocks/MockPOSCatalogSyncCoordinator.swift @@ -21,7 +21,7 @@ final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { var onPerformFullSyncCalled: (() -> Void)? - func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, regenerateCatalog: Bool) async throws { performFullSyncInvocationCount += 1 performFullSyncSiteID = siteID diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift index 1dcdb01760e..ff945a5cb81 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogPersistenceService.swift @@ -2,6 +2,11 @@ import Foundation final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { + // MARK: - replaceAllCatalogData tracking + private(set) var replaceAllCatalogDataCallCount = 0 + private(set) var replaceAllCatalogDataLastPersistedCatalog: POSCatalog? + var replaceAllCatalogDataError: Error? + // MARK: - persistIncrementalCatalogData tracking private(set) var persistIncrementalCatalogDataCallCount = 0 private(set) var persistIncrementalCatalogDataLastPersistedCatalog: POSCatalog? @@ -10,7 +15,12 @@ final class MockPOSCatalogPersistenceService: POSCatalogPersistenceServiceProtoc // MARK: - Protocol Implementation func replaceAllCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { - // Not used in current tests + replaceAllCatalogDataCallCount += 1 + replaceAllCatalogDataLastPersistedCatalog = catalog + + if let error = replaceAllCatalogDataError { + throw error + } } func persistIncrementalCatalogData(_ catalog: POSCatalog, siteID: Int64) async throws { diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift index 9051d6fd97f..5e560f2611d 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift @@ -8,6 +8,9 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { private(set) var incrementalProductResults: [Int: Result, Error>] = [:] private(set) var incrementalVariationResults: [Int: Result, Error>] = [:] + var catalogRequestResult: Result = .success(.init(status: .complete, downloadURL: "https://example.com/catalog.json")) + var catalogDownloadResult: Result = .success(.init(products: [], variations: [])) + let loadProductsCallCount = Counter() let loadProductVariationsCallCount = Counter() let loadIncrementalProductsCallCount = Counter() @@ -15,6 +18,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { private(set) var lastIncrementalProductsModifiedAfter: Date? private(set) var lastIncrementalVariationsModifiedAfter: Date? + private(set) var lastCatalogRequestForceGeneration: Bool? // Fallback result when no specific page result is configured private let fallbackResult = PagedItems(items: [] as [POSProduct], hasMorePages: false, totalItems: 0) @@ -129,11 +133,22 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { // MARK: - Protocol Methods - Catalog API func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool) async throws -> POSCatalogRequestResponse { - .init(status: .pending, downloadURL: nil) + lastCatalogRequestForceGeneration = forceGeneration + switch catalogRequestResult { + case .success(let response): + return response + case .failure(let error): + throw error + } } func downloadCatalog(for siteID: Int64, downloadURL: String) async throws -> POSCatalogResponse { - .init(products: [], variations: []) + switch catalogDownloadResult { + case .success(let response): + return response + case .failure(let error): + throw error + } } // MARK: - Protocol Methods - Catalog size diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift index 515ac3d0b75..38d130a34fd 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift @@ -180,4 +180,109 @@ struct POSCatalogFullSyncServiceTests { #expect(await mockSyncRemote.loadProductsCallCount.value == 5) #expect(await mockSyncRemote.loadProductVariationsCallCount.value == 5) } + + // MARK: - Catalog API Tests + + @Test func startFullSync_with_catalog_API_downloads_and_persists_catalog() async throws { + // Given + let expectedProduct = POSProduct.fake().copy(siteID: sampleSiteID, productID: 1) + let expectedVariation = POSProductVariation.fake().copy(siteID: sampleSiteID, productID: 1, productVariationID: 1) + + mockSyncRemote.catalogRequestResult = .success(.init(status: .complete, downloadURL: "https://example.com/catalog.json")) + mockSyncRemote.catalogDownloadResult = .success(.init(products: [expectedProduct], variations: [expectedVariation])) + + let sut = POSCatalogFullSyncService( + syncRemote: mockSyncRemote, + batchSize: 2, + persistenceService: mockPersistenceService, + usesCatalogAPI: true + ) + + // When + let result = try await sut.startFullSync(for: sampleSiteID) + + // Then + #expect(result.products.count == 1) + #expect(result.variations.count == 1) + #expect(mockPersistenceService.replaceAllCatalogDataCallCount == 1) + #expect(mockPersistenceService.replaceAllCatalogDataLastPersistedCatalog?.products.count == 1) + #expect(mockPersistenceService.replaceAllCatalogDataLastPersistedCatalog?.variations.count == 1) + } + + @Test func startFullSync_with_catalog_API_propagates_catalog_request_error() async throws { + // Given + let expectedError = NSError(domain: "catalog", code: 500, userInfo: [NSLocalizedDescriptionKey: "Catalog request failed"]) + mockSyncRemote.catalogRequestResult = .failure(expectedError) + + let sut = POSCatalogFullSyncService( + syncRemote: mockSyncRemote, + batchSize: 2, + persistenceService: mockPersistenceService, + usesCatalogAPI: true + ) + + // When/Then + await #expect(throws: expectedError) { + _ = try await sut.startFullSync(for: sampleSiteID) + } + } + + @Test func startFullSync_with_catalog_API_propagates_catalog_download_error() async throws { + // Given + let expectedError = NSError(domain: "catalog", code: 404, userInfo: [NSLocalizedDescriptionKey: "Catalog download failed"]) + mockSyncRemote.catalogRequestResult = .success(.init(status: .complete, downloadURL: "https://example.com/catalog.json")) + mockSyncRemote.catalogDownloadResult = .failure(expectedError) + + let sut = POSCatalogFullSyncService( + syncRemote: mockSyncRemote, + batchSize: 2, + persistenceService: mockPersistenceService, + usesCatalogAPI: true + ) + + // When/Then + await #expect(throws: expectedError) { + _ = try await sut.startFullSync(for: sampleSiteID) + } + } + + @Test func startFullSync_with_catalog_API_propagates_persistence_error() async throws { + // Given + let expectedError = NSError(domain: "persistence", code: 1000, userInfo: [NSLocalizedDescriptionKey: "Persistence failed"]) + mockSyncRemote.catalogRequestResult = .success(.init(status: .complete, downloadURL: "https://example.com/catalog.json")) + mockSyncRemote.catalogDownloadResult = .success(.init(products: [], variations: [])) + mockPersistenceService.replaceAllCatalogDataError = expectedError + + let sut = POSCatalogFullSyncService( + syncRemote: mockSyncRemote, + batchSize: 2, + persistenceService: mockPersistenceService, + usesCatalogAPI: true + ) + + // When/Then + await #expect(throws: expectedError) { + _ = try await sut.startFullSync(for: sampleSiteID) + } + } + + @Test(arguments: [true, false]) + func startFullSync_with_catalog_API_passes_regenerateCatalog_to_remote(regenerateCatalog: Bool) async throws { + // Given + mockSyncRemote.catalogRequestResult = .success(.init(status: .complete, downloadURL: "https://example.com/catalog.json")) + mockSyncRemote.catalogDownloadResult = .success(.init(products: [], variations: [])) + + let sut = POSCatalogFullSyncService( + syncRemote: mockSyncRemote, + batchSize: 2, + persistenceService: mockPersistenceService, + usesCatalogAPI: true + ) + + // When + _ = try await sut.startFullSync(for: sampleSiteID, regenerateCatalog: regenerateCatalog) + + // Then + #expect(mockSyncRemote.lastCatalogRequestForceGeneration == regenerateCatalog) + } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift index 5846459e11c..58b22c21a45 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift @@ -564,6 +564,42 @@ struct POSCatalogPersistenceServiceTests { #expect(site?.id == sampleSiteID) } } + + // MARK: - Orphaned Variation Filtering Tests + + @Test func replaceAllCatalogData_filters_out_variations_without_parent_products() async throws { + // Given + let product = POSProduct.fake().copy(siteID: sampleSiteID, productID: 10) + let validVariation = POSProductVariation.fake() + .copy(siteID: sampleSiteID, productID: 10, productVariationID: 1, image: ProductImage.fake().copy(imageID: 100)) + let orphanedVariation1 = POSProductVariation.fake() + .copy(siteID: sampleSiteID, productID: 20, productVariationID: 2, image: ProductImage.fake().copy(imageID: 200)) + let orphanedVariation2 = POSProductVariation.fake() + .copy(siteID: sampleSiteID, productID: 30, productVariationID: 3, image: ProductImage.fake().copy(imageID: 300)) + + let catalog = POSCatalog( + products: [product], + variations: [validVariation, orphanedVariation1, orphanedVariation2], + syncDate: .now + ) + + // When + try await sut.replaceAllCatalogData(catalog, siteID: sampleSiteID) + + // Then + try await db.read { db in + let variationCount = try PersistedProductVariation.fetchCount(db) + #expect(variationCount == 1) + let variationImageCount = try PersistedProductVariationImage.fetchCount(db) + #expect(variationImageCount == 1) + + let variation = try PersistedProductVariation.fetchOne(db) + #expect(variation?.id == 1) + #expect(variation?.productID == 10) + let variationImage = try PersistedProductVariationImage.fetchOne(db) + #expect(variationImage?.imageID == 100) + } + } } private extension POSCatalogPersistenceServiceTests { diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index 1b163f79d76..a36bbbb4ae8 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -592,7 +592,7 @@ final class MockPOSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol { private(set) var startFullSyncCallCount = 0 private(set) var lastSyncSiteID: Int64? - func startFullSync(for siteID: Int64) async throws -> POSCatalog { + func startFullSync(for siteID: Int64, regenerateCatalog: Bool) async throws -> POSCatalog { startFullSyncCallCount += 1 lastSyncSiteID = siteID @@ -858,3 +858,9 @@ extension POSCatalogSyncCoordinatorTests { #expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1) } } + +extension POSCatalogSyncCoordinator { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + try await performFullSyncIfApplicable(for: siteID, maxAge: maxAge, regenerateCatalog: false) + } +} diff --git a/WooCommerce/Classes/Tools/BackgroundTasks/ForegroundPOSCatalogSyncDispatcher.swift b/WooCommerce/Classes/Tools/BackgroundTasks/ForegroundPOSCatalogSyncDispatcher.swift index cb94d1a7086..9dba961d48f 100644 --- a/WooCommerce/Classes/Tools/BackgroundTasks/ForegroundPOSCatalogSyncDispatcher.swift +++ b/WooCommerce/Classes/Tools/BackgroundTasks/ForegroundPOSCatalogSyncDispatcher.swift @@ -159,6 +159,12 @@ final actor ForegroundPOSCatalogSyncDispatcher { DDLogInfo("â„šī¸ ForegroundPOSCatalogSyncDispatcher: Sync already in progress for site \(siteID)") case .negativeMaxAge: DDLogError("â›”ī¸ ForegroundPOSCatalogSyncDispatcher: Invalid max age for site \(siteID)") + case .invalidData: + DDLogError("â›”ī¸ ForegroundPOSCatalogSyncDispatcher: Invalid data encountered during sync for site \(siteID)") + case .timeout: + DDLogError("â›”ī¸ ForegroundPOSCatalogSyncDispatcher: Sync timed out for site \(siteID)") + case .generationFailed: + DDLogError("â›”ī¸ ForegroundPOSCatalogSyncDispatcher: Sync generation failed for site \(siteID)") case .requestCancelled: DDLogInfo("â„šī¸ ForegroundPOSCatalogSyncDispatcher: Sync request was cancelled for site \(siteID)") case .shouldNotSync: diff --git a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift index 612dfa85653..f6b101b488a 100644 --- a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift +++ b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift @@ -173,7 +173,8 @@ class AuthenticatedState: StoresManagerState { let fullSyncService = POSCatalogFullSyncService(credentials: credentials, selectedSite: site, appPasswordSupportState: appPasswordSupportState.eraseToAnyPublisher(), - grdbManager: ServiceLocator.grdbManager), + grdbManager: ServiceLocator.grdbManager, + usesCatalogAPI: ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleCatalogAPI)), let incrementalSyncService = POSCatalogIncrementalSyncService( credentials: credentials, selectedSite: site, diff --git a/WooCommerce/WooCommerceTests/Tools/ForegroundPOSCatalogSyncDispatcherTests.swift b/WooCommerce/WooCommerceTests/Tools/ForegroundPOSCatalogSyncDispatcherTests.swift index 1987a72bd13..6c85248ab79 100644 --- a/WooCommerce/WooCommerceTests/Tools/ForegroundPOSCatalogSyncDispatcherTests.swift +++ b/WooCommerce/WooCommerceTests/Tools/ForegroundPOSCatalogSyncDispatcherTests.swift @@ -284,7 +284,7 @@ private final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProt } } - func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval) async throws { + func performFullSyncIfApplicable(for siteID: Int64, maxAge: TimeInterval, regenerateCatalog: Bool) async throws { // Not used }