diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift index f9dc6b28f5e..77002f485e2 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift @@ -28,7 +28,6 @@ public struct POSCatalog { // periphery:ignore public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol { private let syncRemote: POSCatalogSyncRemoteProtocol - private let batchSize: Int private let persistenceService: POSCatalogPersistenceServiceProtocol private let batchedLoader: BatchedRequestLoader @@ -49,11 +48,15 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService) } - init(syncRemote: POSCatalogSyncRemoteProtocol, batchSize: Int, persistenceService: POSCatalogPersistenceServiceProtocol) { + init( + syncRemote: POSCatalogSyncRemoteProtocol, + batchSize: Int, + retryDelay: TimeInterval = 2.0, + persistenceService: POSCatalogPersistenceServiceProtocol + ) { self.syncRemote = syncRemote - self.batchSize = batchSize self.persistenceService = persistenceService - self.batchedLoader = BatchedRequestLoader(batchSize: batchSize) + self.batchedLoader = BatchedRequestLoader(batchSize: batchSize, retryDelay: retryDelay) } // MARK: - Protocol Conformance diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogIncrementalSyncService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogIncrementalSyncService.swift index edf8ad34ee1..a635781bc8e 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogIncrementalSyncService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogIncrementalSyncService.swift @@ -20,7 +20,6 @@ public protocol POSCatalogIncrementalSyncServiceProtocol { // periphery:ignore public final class POSCatalogIncrementalSyncService: POSCatalogIncrementalSyncServiceProtocol { private let syncRemote: POSCatalogSyncRemoteProtocol - private let batchSize: Int private let persistenceService: POSCatalogPersistenceServiceProtocol private let batchedLoader: BatchedRequestLoader @@ -41,11 +40,15 @@ public final class POSCatalogIncrementalSyncService: POSCatalogIncrementalSyncSe self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService) } - init(syncRemote: POSCatalogSyncRemoteProtocol, batchSize: Int, persistenceService: POSCatalogPersistenceServiceProtocol) { + init( + syncRemote: POSCatalogSyncRemoteProtocol, + batchSize: Int, + retryDelay: TimeInterval = 2.0, + persistenceService: POSCatalogPersistenceServiceProtocol + ) { self.syncRemote = syncRemote - self.batchSize = batchSize self.persistenceService = persistenceService - self.batchedLoader = BatchedRequestLoader(batchSize: batchSize) + self.batchedLoader = BatchedRequestLoader(batchSize: batchSize, retryDelay: retryDelay) } // MARK: - Protocol Conformance diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift index 64a126b93ac..203ddcec350 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift @@ -8,10 +8,10 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { private(set) var incrementalProductResults: [Int: Result, Error>] = [:] private(set) var incrementalVariationResults: [Int: Result, Error>] = [:] - private(set) var loadProductsCallCount = 0 - private(set) var loadProductVariationsCallCount = 0 - private(set) var loadIncrementalProductsCallCount = 0 - private(set) var loadIncrementalProductVariationsCallCount = 0 + let loadProductsCallCount = Counter() + let loadProductVariationsCallCount = Counter() + let loadIncrementalProductsCallCount = Counter() + let loadIncrementalProductVariationsCallCount = Counter() private(set) var lastIncrementalProductsModifiedAfter: Date? private(set) var lastIncrementalVariationsModifiedAfter: Date? @@ -67,7 +67,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { // MARK: - Protocol Methods - Incremental Sync func loadProducts(modifiedAfter: Date, siteID: Int64, pageNumber: Int) async throws -> PagedItems { - loadIncrementalProductsCallCount += 1 + await loadIncrementalProductsCallCount.increment() lastIncrementalProductsModifiedAfter = modifiedAfter if let result = incrementalProductResults[pageNumber] { @@ -82,7 +82,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { } func loadProductVariations(modifiedAfter: Date, siteID: Int64, pageNumber: Int) async throws -> PagedItems { - loadIncrementalProductVariationsCallCount += 1 + await loadIncrementalProductVariationsCallCount.increment() lastIncrementalVariationsModifiedAfter = modifiedAfter if let result = incrementalVariationResults[pageNumber] { @@ -99,7 +99,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { // MARK: - Protocol Methods - Full Sync func loadProducts(siteID: Int64, pageNumber: Int) async throws -> PagedItems { - loadProductsCallCount += 1 + await loadProductsCallCount.increment() if let result = productResults[pageNumber] { switch result { @@ -113,7 +113,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { } func loadProductVariations(siteID: Int64, pageNumber: Int) async throws -> PagedItems { - loadProductVariationsCallCount += 1 + await loadProductVariationsCallCount.increment() if let result = variationResults[pageNumber] { switch result { diff --git a/Modules/Tests/YosemiteTests/Tools/POS/BatchedRequestLoaderTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/BatchedRequestLoaderTests.swift index eed01d3b839..60ce01b56c3 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/BatchedRequestLoaderTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/BatchedRequestLoaderTests.swift @@ -11,10 +11,10 @@ struct BatchedRequestLoaderTests { // Given let sut = BatchedRequestLoader(batchSize: 2) let expectedItems = ["item1", "item2", "item3"] - var callCount = 0 + let callCount = Counter() let makeRequest: (Int) async throws -> PagedItems = { pageNumber in - callCount += 1 + await callCount.increment() #expect(pageNumber == 1 || pageNumber == 2) if pageNumber == 1 { return PagedItems(items: expectedItems, hasMorePages: false, totalItems: 3) @@ -28,7 +28,7 @@ struct BatchedRequestLoaderTests { // Then #expect(result == expectedItems) - #expect(callCount == 2) // batchSize = 2, so it fetches pages 1 and 2 + #expect(await callCount.value == 2) // batchSize = 2, so it fetches pages 1 and 2 } // MARK: - Retry Logic Tests @@ -43,13 +43,23 @@ struct BatchedRequestLoaderTests { errorEvaluator: mockErrorEvaluator ) - var attemptsByPage: [Int: Int] = [:] + let attemptCountForPage1 = Counter() + let attemptCountForPage2 = Counter() + let attemptCountForPage3 = Counter() let makeRequest: (Int) async throws -> PagedItems = { pageNumber in - attemptsByPage[pageNumber, default: 0] += 1 - - // Page 2 fails once, then succeeds - if pageNumber == 2 && attemptsByPage[pageNumber] == 1 { - throw URLError(.timedOut) + switch pageNumber { + case 1: + await attemptCountForPage1.increment() + case 2: + await attemptCountForPage2.increment() + // Page 2 fails once, then succeeds + if await attemptCountForPage2.value == 1 { + throw URLError(.timedOut) + } + case 3: + await attemptCountForPage3.increment() + default: + throw NSError(domain: "Invalid page number", code: 0) } return PagedItems(items: ["page \(pageNumber) success"], hasMorePages: false, totalItems: 1) @@ -60,9 +70,9 @@ struct BatchedRequestLoaderTests { // Then #expect(result == ["page 1 success", "page 2 success", "page 3 success"]) - #expect(attemptsByPage[1] == 1) // Page 1 succeeded immediately - #expect(attemptsByPage[2] == 2) // Page 2 failed once, then succeeded - #expect(attemptsByPage[3] == 1) // Page 3 succeeded immediately + #expect(await attemptCountForPage1.value == 1) // Page 1 succeeded immediately + #expect(await attemptCountForPage2.value == 2) // Page 2 failed once, then succeeded + #expect(await attemptCountForPage3.value == 1) // Page 3 succeeded immediately } @Test func loadAll_does_not_retry_on_non_retryable_error() async throws { @@ -75,9 +85,9 @@ struct BatchedRequestLoaderTests { errorEvaluator: mockErrorEvaluator ) - var attemptCount = 0 + let attemptCount = Counter() let makeRequest: (Int) async throws -> PagedItems = { pageNumber in - attemptCount += 1 + await attemptCount.increment() throw NetworkError.unacceptableStatusCode(statusCode: 401, response: nil) } @@ -85,7 +95,7 @@ struct BatchedRequestLoaderTests { await #expect(throws: NetworkError.self) { try await sut.loadAll(makeRequest: makeRequest) } - #expect(attemptCount == 2) // No retries for non-retryable error for both pages + #expect(await attemptCount.value == 2) // No retries for non-retryable error for both pages } @Test func loadAll_fails_after_max_retries() async throws { @@ -98,10 +108,10 @@ struct BatchedRequestLoaderTests { errorEvaluator: mockErrorEvaluator ) - var attemptCount = 0 + let attemptCount = Counter() let expectedError = URLError(.networkConnectionLost) let makeRequest: (Int) async throws -> PagedItems = { pageNumber in - attemptCount += 1 + await attemptCount.increment() throw expectedError } @@ -109,7 +119,7 @@ struct BatchedRequestLoaderTests { await #expect(throws: URLError.self) { try await sut.loadAll(makeRequest: makeRequest) } - #expect(attemptCount == 3) // maxRetries = 3 + #expect(await attemptCount.value == 3) // maxRetries = 3 } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/ConcurrencyUtilities/Counter.swift b/Modules/Tests/YosemiteTests/Tools/POS/ConcurrencyUtilities/Counter.swift new file mode 100644 index 00000000000..b78b51226e2 --- /dev/null +++ b/Modules/Tests/YosemiteTests/Tools/POS/ConcurrencyUtilities/Counter.swift @@ -0,0 +1,8 @@ +/// A simple counter actor to track increments in a thread-safe manner. +actor Counter { + var value = 0 + + func increment() { + value += 1 + } +} diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift index 8b271c3eb81..515ac3d0b75 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift @@ -33,8 +33,8 @@ struct POSCatalogFullSyncServiceTests { // Then #expect(result.products.count == expectedProducts.count) #expect(result.variations.count == expectedVariations.count) - #expect(mockSyncRemote.loadProductsCallCount == 2) // 1 batch of 2 requests - #expect(mockSyncRemote.loadProductVariationsCallCount == 2) // 1 batch of 2 requests + #expect(await mockSyncRemote.loadProductsCallCount.value == 2) // 1 batch of 2 requests + #expect(await mockSyncRemote.loadProductVariationsCallCount.value == 2) // 1 batch of 2 requests } @Test func startFullSync_handles_paginated_products_correctly() async throws { @@ -54,8 +54,8 @@ struct POSCatalogFullSyncServiceTests { // Then #expect(result.products.count == 3) - #expect(mockSyncRemote.loadProductsCallCount == 4) // 2 batches of 2 requests - #expect(mockSyncRemote.loadProductVariationsCallCount == 2) // 1 batch of 2 requests + #expect(await mockSyncRemote.loadProductsCallCount.value == 4) // 2 batches of 2 requests + #expect(await mockSyncRemote.loadProductVariationsCallCount.value == 2) // 1 batch of 2 requests } @Test func startFullSync_handles_paginated_variations_correctly() async throws { @@ -75,8 +75,8 @@ struct POSCatalogFullSyncServiceTests { // Then #expect(result.variations.count == 4) - #expect(mockSyncRemote.loadProductsCallCount == 2) // 1 batch of 2 requests - #expect(mockSyncRemote.loadProductVariationsCallCount == 4) // 2 batches of 2 requests + #expect(await mockSyncRemote.loadProductsCallCount.value == 2) // 1 batch of 2 requests + #expect(await mockSyncRemote.loadProductVariationsCallCount.value == 4) // 2 batches of 2 requests } @Test func startFullSync_stops_pagination_when_no_new_items_returned_and_hasMorePages_is_inaccurate() async throws { @@ -95,7 +95,7 @@ struct POSCatalogFullSyncServiceTests { // Then - Should stop after empty page #expect(result.products.count == 1) - #expect(mockSyncRemote.loadProductsCallCount == 4) // The results from the second batch are empty + #expect(await mockSyncRemote.loadProductsCallCount.value == 4) // The results from the second batch are empty } @Test func startFullSync_handles_batch_processing_correctly() async throws { @@ -116,13 +116,14 @@ struct POSCatalogFullSyncServiceTests { // Then #expect(result.products.count == 5) - #expect(mockSyncRemote.loadProductsCallCount == 6) + #expect(await mockSyncRemote.loadProductsCallCount.value == 6) } @Test func startFullSync_propagates_network_errors() async throws { // Given let expectedError = NSError(domain: "network", code: 500, userInfo: [NSLocalizedDescriptionKey: "Network error"]) mockSyncRemote.setProductResult(pageNumber: 1, result: .failure(expectedError)) + let sut = POSCatalogFullSyncService(syncRemote: mockSyncRemote, batchSize: 2, retryDelay: 0, persistenceService: mockPersistenceService) // When/Then await #expect(throws: expectedError) { @@ -176,7 +177,7 @@ struct POSCatalogFullSyncServiceTests { _ = try await service.startFullSync(for: sampleSiteID) // Then - #expect(mockSyncRemote.loadProductsCallCount == 5) - #expect(mockSyncRemote.loadProductVariationsCallCount == 5) + #expect(await mockSyncRemote.loadProductsCallCount.value == 5) + #expect(await mockSyncRemote.loadProductVariationsCallCount.value == 5) } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift index c7cd3cb56d0..a4434a76460 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogIncrementalSyncServiceTests.swift @@ -31,8 +31,8 @@ struct POSCatalogIncrementalSyncServiceTests { try await sut.startIncrementalSync(for: sampleSiteID, lastFullSyncDate: lastFullSyncDate, lastIncrementalSyncDate: nil) // Then - #expect(mockSyncRemote.loadIncrementalProductsCallCount == 2) - #expect(mockSyncRemote.loadIncrementalProductVariationsCallCount == 2) + #expect(await mockSyncRemote.loadIncrementalProductsCallCount.value == 2) + #expect(await mockSyncRemote.loadIncrementalProductVariationsCallCount.value == 2) #expect(mockSyncRemote.lastIncrementalProductsModifiedAfter == lastFullSyncDate) #expect(mockSyncRemote.lastIncrementalVariationsModifiedAfter == lastFullSyncDate) #expect(mockPersistenceService.persistIncrementalCatalogDataCallCount == 1) @@ -76,7 +76,7 @@ struct POSCatalogIncrementalSyncServiceTests { try await sut.startIncrementalSync(for: sampleSiteID, lastFullSyncDate: lastFullSyncDate, lastIncrementalSyncDate: nil) // Then - #expect(mockSyncRemote.loadIncrementalProductsCallCount == 4) + #expect(await mockSyncRemote.loadIncrementalProductsCallCount.value == 4) let persistedCatalog = try #require(mockPersistenceService.persistIncrementalCatalogDataLastPersistedCatalog) #expect(persistedCatalog.products.count == 3) } @@ -97,7 +97,7 @@ struct POSCatalogIncrementalSyncServiceTests { try await sut.startIncrementalSync(for: sampleSiteID, lastFullSyncDate: lastFullSyncDate, lastIncrementalSyncDate: nil) // Then - #expect(mockSyncRemote.loadIncrementalProductVariationsCallCount == 2) + #expect(await mockSyncRemote.loadIncrementalProductVariationsCallCount.value == 2) let persistedCatalog = try #require(mockPersistenceService.persistIncrementalCatalogDataLastPersistedCatalog) #expect(persistedCatalog.variations.count == 2) } @@ -108,6 +108,7 @@ struct POSCatalogIncrementalSyncServiceTests { // Given let lastFullSyncDate = Date(timeIntervalSince1970: 1000) let expectedError = NSError(domain: "test", code: 500, userInfo: nil) + let sut = POSCatalogIncrementalSyncService(syncRemote: mockSyncRemote, batchSize: 2, retryDelay: 0, persistenceService: mockPersistenceService) mockSyncRemote.setIncrementalProductResult(pageNumber: 1, result: .failure(expectedError)) mockSyncRemote.setIncrementalVariationResult(pageNumber: 1, result: .success(PagedItems(items: [], hasMorePages: false, totalItems: 0)))