From b26626e454c34390ef2bbf274786f3fa31023649 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 5 Nov 2025 12:00:58 +0000 Subject: [PATCH 1/4] Toggle cellular for full sync on catalog endpoint --- .../Remote/POSCatalogSyncRemote.swift | 22 ++++---- .../Tools/POS/POSCatalogFullSyncService.swift | 32 ++++++++--- .../Tools/POS/POSCatalogSyncCoordinator.swift | 5 +- .../Remote/POSCatalogSyncRemoteTests.swift | 23 ++++++-- .../Mocks/MockPOSCatalogSyncRemote.swift | 4 +- .../POS/POSCatalogFullSyncServiceTests.swift | 44 +++++++++++---- .../POS/POSCatalogSyncCoordinatorTests.swift | 55 ++++++++++++++++++- 7 files changed, 146 insertions(+), 39 deletions(-) diff --git a/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift b/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift index ce48de11dcb..8dfb87bb1a2 100644 --- a/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift +++ b/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift @@ -39,9 +39,11 @@ public protocol POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to download catalog for. /// - downloadURL: Download URL of the catalog file. + /// - allowCellular: Should cellular data be used if required. /// - Returns: List of products and variations in the POS catalog. - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync - func downloadCatalog(for siteID: Int64, downloadURL: String) async throws -> POSCatalogResponse + func downloadCatalog(for siteID: Int64, + downloadURL: String, + allowCellular: Bool) async throws -> POSCatalogResponse /// Loads POS products for full sync. /// @@ -87,7 +89,6 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - pageNumber: Page number for pagination. /// - Returns: Paginated list of POS products. /// - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public func loadProducts(modifiedAfter: Date, siteID: Int64, pageNumber: Int) async throws -> PagedItems { let path = Path.products @@ -120,7 +121,6 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - pageNumber: Page number for pagination. /// - Returns: Paginated list of POS product variations. /// - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public func loadProductVariations(modifiedAfter: Date, siteID: Int64, pageNumber: Int) async throws -> PagedItems { let path = Path.variations let parameters = [ @@ -153,7 +153,6 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - siteID: Site ID to generate catalog for. /// - Returns: Catalog job response with job ID. /// - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool) async throws -> POSCatalogRequestResponse { let path = "products/catalog" let parameters: [String: Any] = [ @@ -176,14 +175,17 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to download catalog for. /// - downloadURL: Download URL of the catalog file. + /// - allowCellular: Should cellular data be used if required. /// - Returns: List of products and variations in the POS catalog. - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync - public func downloadCatalog(for siteID: Int64, downloadURL: String) async throws -> POSCatalogResponse { + public func downloadCatalog(for siteID: Int64, + downloadURL: String, + allowCellular: Bool) async throws -> POSCatalogResponse { // TODO: WOOMOB-1173 - move download task to the background using `URLSessionConfiguration.background` guard let url = URL(string: downloadURL) else { throw NetworkError.invalidURL } - let request = URLRequest(url: url) + var request = URLRequest(url: url) + request.allowsCellularAccess = allowCellular let mapper = ListMapper(siteID: siteID) let items = try await enqueue(request, mapper: mapper) let variationProductTypeKey = "variation" @@ -200,7 +202,6 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - pageNumber: Page number for pagination. /// - Returns: Paginated list of POS products. /// - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public func loadProducts(siteID: Int64, pageNumber: Int) async throws -> PagedItems { let path = Path.products let parameters = [ @@ -230,7 +231,6 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - pageNumber: Page number for pagination. /// - Returns: Paginated list of POS product variations. /// - // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public func loadProductVariations(siteID: Int64, pageNumber: Int) async throws -> PagedItems { let path = Path.variations let parameters = [ @@ -331,7 +331,6 @@ private extension POSCatalogSyncRemote { // MARK: - Response Models /// Response from catalog generation request. -// periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public struct POSCatalogRequestResponse: Decodable { /// Current status of the catalog generation job. public let status: POSCatalogStatus @@ -353,7 +352,6 @@ public enum POSCatalogStatus: String, Decodable { } /// POS catalog from download. -// periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync public struct POSCatalogResponse { public let products: [POSProduct] public let variations: [POSProductVariation] diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift index fad9dd05d8a..da56ef77858 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift @@ -14,8 +14,9 @@ public protocol POSCatalogFullSyncServiceProtocol { /// - Parameters: /// - siteID: The site ID to sync catalog for /// - regenerateCatalog: Whether to force the catalog generation + /// - allowCellular: Should cellular data be used if required. /// - Returns: The synced catalog containing products and variations - func startFullSync(for siteID: Int64, regenerateCatalog: Bool) async throws -> POSCatalog + func startFullSync(for siteID: Int64, regenerateCatalog: Bool, allowCellular: Bool) async throws -> POSCatalog } /// POS catalog from full sync. @@ -68,14 +69,20 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol // MARK: - Protocol Conformance - public func startFullSync(for siteID: Int64, regenerateCatalog: Bool = false) async throws -> POSCatalog { - DDLogInfo("🔄 Starting full catalog sync for site ID: \(siteID) with regenerateCatalog: \(regenerateCatalog)") + public func startFullSync(for siteID: Int64, + regenerateCatalog: Bool = false, + allowCellular: Bool) async throws -> POSCatalog { + DDLogInfo("🔄 Starting full catalog sync for site ID: \(siteID) with regenerateCatalog: \(regenerateCatalog) " + + "and allowCellular: \(allowCellular)") do { // Sync from network let catalog: POSCatalog if usesCatalogAPI { - catalog = try await loadCatalogFromCatalogAPI(for: siteID, syncRemote: syncRemote, regenerateCatalog: regenerateCatalog) + catalog = try await loadCatalogFromCatalogAPI(for: siteID, + syncRemote: syncRemote, + regenerateCatalog: regenerateCatalog, + allowCellular: allowCellular) } else { catalog = try await loadCatalog(for: siteID, syncRemote: syncRemote) } @@ -114,9 +121,15 @@ private extension POSCatalogFullSyncService { return POSCatalog(products: products, variations: variations, syncDate: syncStartDate) } - func loadCatalogFromCatalogAPI(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol, regenerateCatalog: Bool) async throws -> POSCatalog { + func loadCatalogFromCatalogAPI(for siteID: Int64, + syncRemote: POSCatalogSyncRemoteProtocol, + regenerateCatalog: Bool, + allowCellular: Bool) async throws -> POSCatalog { let downloadStartTime = CFAbsoluteTimeGetCurrent() - let catalog = try await downloadCatalog(for: siteID, syncRemote: syncRemote, regenerateCatalog: regenerateCatalog) + let catalog = try await downloadCatalog(for: siteID, + syncRemote: syncRemote, + regenerateCatalog: regenerateCatalog, + allowCellular: allowCellular) let downloadTime = CFAbsoluteTimeGetCurrent() - downloadStartTime DDLogInfo("🟣 Catalog download completed - Time: \(String(format: "%.2f", downloadTime))s") @@ -125,7 +138,10 @@ private extension POSCatalogFullSyncService { } private extension POSCatalogFullSyncService { - func downloadCatalog(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol, regenerateCatalog: Bool) async throws -> POSCatalogResponse { + func downloadCatalog(for siteID: Int64, + syncRemote: POSCatalogSyncRemoteProtocol, + regenerateCatalog: Bool, + allowCellular: Bool) async throws -> POSCatalogResponse { DDLogInfo("🟣 Starting catalog request...") // 1. Requests catalog until download URL is available. @@ -143,7 +159,7 @@ private extension POSCatalogFullSyncService { throw POSCatalogSyncError.invalidData } DDLogInfo("🟣 Catalog ready for download: \(downloadURL)") - return try await syncRemote.downloadCatalog(for: siteID, downloadURL: downloadURL) + return try await syncRemote.downloadCatalog(for: siteID, downloadURL: downloadURL, allowCellular: allowCellular) } func pollForCatalogCompletion(siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol) async throws -> String { diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift index 16cfcdfdc72..960195e008b 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift @@ -117,10 +117,13 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol { emitSyncState(isFirstSync ? .initialSyncStarted(siteID: siteID) : .syncStarted(siteID: siteID)) + let allowCellular = isFirstSync || siteSettings.getPOSLocalCatalogCellularDataAllowed(siteID: siteID) DDLogInfo("🔄 POSCatalogSyncCoordinator starting full sync for site \(siteID)") do { - _ = try await fullSyncService.startFullSync(for: siteID, regenerateCatalog: regenerateCatalog) + _ = try await fullSyncService.startFullSync(for: siteID, + regenerateCatalog: regenerateCatalog, + allowCellular: allowCellular) emitSyncState(.syncCompleted(siteID: siteID)) } catch AFError.explicitlyCancelled, is CancellationError { if isFirstSync { diff --git a/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift index 1db4277e553..7c0cc4b7478 100644 --- a/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift @@ -638,7 +638,7 @@ struct POSCatalogSyncRemoteTests { // When network.simulateResponse(requestUrlSuffix: "", filename: "pos-catalog-download-mixed") - let catalog = try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL) + let catalog = try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL, allowCellular: true) // Then #expect(catalog.products.count == 2) @@ -700,7 +700,7 @@ struct POSCatalogSyncRemoteTests { // When network.simulateResponse(requestUrlSuffix: "", filename: "empty-data-array") - let catalog = try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL) + let catalog = try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL, allowCellular: true) // Then #expect(catalog.products.count == 0) @@ -714,7 +714,7 @@ struct POSCatalogSyncRemoteTests { // When/Then await #expect(throws: NetworkError.invalidURL) { - try await remote.downloadCatalog(for: sampleSiteID, downloadURL: emptyURL) + try await remote.downloadCatalog(for: sampleSiteID, downloadURL: emptyURL, allowCellular: true) } } @@ -725,7 +725,22 @@ struct POSCatalogSyncRemoteTests { // When/Then await #expect(throws: NetworkError.notFound()) { - try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL) + try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL, allowCellular: true) } } + + @Test(arguments: [true, false]) + func downloadCatalog_sets_allowsCellularAccess_on_request(allowCellular: Bool) async throws { + // Given + let remote = POSCatalogSyncRemote(network: network) + let downloadURL = "https://example.com/catalog.json" + network.simulateResponse(requestUrlSuffix: "", filename: "empty-data-array") + + // When + _ = try await remote.downloadCatalog(for: sampleSiteID, downloadURL: downloadURL, allowCellular: allowCellular) + + // Then + let urlRequest = try #require(network.requestsForResponseData.last as? URLRequest) + #expect(urlRequest.allowsCellularAccess == allowCellular) + } } diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift index 5e560f2611d..8754c3684c0 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift @@ -19,6 +19,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { private(set) var lastIncrementalProductsModifiedAfter: Date? private(set) var lastIncrementalVariationsModifiedAfter: Date? private(set) var lastCatalogRequestForceGeneration: Bool? + private(set) var lastCatalogDownloadAllowCellular: Bool? // Fallback result when no specific page result is configured private let fallbackResult = PagedItems(items: [] as [POSProduct], hasMorePages: false, totalItems: 0) @@ -142,7 +143,8 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { } } - func downloadCatalog(for siteID: Int64, downloadURL: String) async throws -> POSCatalogResponse { + func downloadCatalog(for siteID: Int64, downloadURL: String, allowCellular: Bool) async throws -> POSCatalogResponse { + lastCatalogDownloadAllowCellular = allowCellular switch catalogDownloadResult { case .success(let response): return response diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift index 38d130a34fd..8ef0163f285 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogFullSyncServiceTests.swift @@ -28,7 +28,7 @@ struct POSCatalogFullSyncServiceTests { mockSyncRemote.setVariationResult(pageNumber: 1, result: .success(PagedItems(items: expectedVariations, hasMorePages: false, totalItems: 0))) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(result.products.count == expectedProducts.count) @@ -50,7 +50,7 @@ struct POSCatalogFullSyncServiceTests { ]) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(result.products.count == 3) @@ -71,7 +71,7 @@ struct POSCatalogFullSyncServiceTests { ]) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(result.variations.count == 4) @@ -91,7 +91,7 @@ struct POSCatalogFullSyncServiceTests { mockSyncRemote.setVariationResult(pageNumber: 1, result: .success(PagedItems(items: [], hasMorePages: false, totalItems: 0))) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then - Should stop after empty page #expect(result.products.count == 1) @@ -112,7 +112,7 @@ struct POSCatalogFullSyncServiceTests { mockSyncRemote.setVariationResult(pageNumber: 1, result: .success(PagedItems(items: [], hasMorePages: false, totalItems: 0))) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(result.products.count == 5) @@ -127,7 +127,7 @@ struct POSCatalogFullSyncServiceTests { // When/Then await #expect(throws: expectedError) { - _ = try await sut.startFullSync(for: sampleSiteID) + _ = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) } } @@ -174,7 +174,7 @@ struct POSCatalogFullSyncServiceTests { let service = POSCatalogFullSyncService(syncRemote: mockSyncRemote, batchSize: customBatchSize, persistenceService: mockPersistenceService) - _ = try await service.startFullSync(for: sampleSiteID) + _ = try await service.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(await mockSyncRemote.loadProductsCallCount.value == 5) @@ -199,7 +199,7 @@ struct POSCatalogFullSyncServiceTests { ) // When - let result = try await sut.startFullSync(for: sampleSiteID) + let result = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) // Then #expect(result.products.count == 1) @@ -223,7 +223,7 @@ struct POSCatalogFullSyncServiceTests { // When/Then await #expect(throws: expectedError) { - _ = try await sut.startFullSync(for: sampleSiteID) + _ = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) } } @@ -242,7 +242,7 @@ struct POSCatalogFullSyncServiceTests { // When/Then await #expect(throws: expectedError) { - _ = try await sut.startFullSync(for: sampleSiteID) + _ = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) } } @@ -262,7 +262,7 @@ struct POSCatalogFullSyncServiceTests { // When/Then await #expect(throws: expectedError) { - _ = try await sut.startFullSync(for: sampleSiteID) + _ = try await sut.startFullSync(for: sampleSiteID, allowCellular: true) } } @@ -280,9 +280,29 @@ struct POSCatalogFullSyncServiceTests { ) // When - _ = try await sut.startFullSync(for: sampleSiteID, regenerateCatalog: regenerateCatalog) + _ = try await sut.startFullSync(for: sampleSiteID, regenerateCatalog: regenerateCatalog, allowCellular: true) // Then #expect(mockSyncRemote.lastCatalogRequestForceGeneration == regenerateCatalog) } + + @Test(arguments: [true, false]) + func startFullSync_with_catalog_API_passes_allowCellular_to_downloadCatalog(allowCellular: 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: false, allowCellular: allowCellular) + + // Then + #expect(mockSyncRemote.lastCatalogDownloadAllowCellular == allowCellular) + } } diff --git a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift index db5c81899da..7e1c1ee83af 100644 --- a/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift +++ b/Modules/Tests/YosemiteTests/Tools/POS/POSCatalogSyncCoordinatorTests.swift @@ -594,10 +594,12 @@ final class MockPOSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol { private(set) var startFullSyncCallCount = 0 private(set) var lastSyncSiteID: Int64? + private(set) var lastAllowCellular: Bool? - func startFullSync(for siteID: Int64, regenerateCatalog: Bool) async throws -> POSCatalog { + func startFullSync(for siteID: Int64, regenerateCatalog: Bool, allowCellular: Bool) async throws -> POSCatalog { startFullSyncCallCount += 1 lastSyncSiteID = siteID + lastAllowCellular = allowCellular // If we should block, wait for continuation to be resumed if shouldBlockSync { @@ -935,6 +937,57 @@ extension POSCatalogSyncCoordinatorTests { // Then - past threshold should be stale #expect(isStale == true) } + + // MARK: - Cellular Data Tests + + @Test func performFullSync_allows_cellular_for_first_sync() async throws { + // Given - No previous sync (first sync) + mockSiteSettings.mockPOSLocalCatalogCellularDataAllowed = false + + // When + try await sut.performFullSync(for: sampleSiteID) + + // Then - Should allow cellular for first sync regardless of setting + #expect(mockSyncService.lastAllowCellular == true) + } + + @Test func performFullSync_respects_cellular_setting_for_subsequent_syncs_when_true() async throws { + // Given - Previous sync exists + try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-2 * 60 * 60)) + mockSiteSettings.mockPOSLocalCatalogCellularDataAllowed = true + + // When + try await sut.performFullSync(for: sampleSiteID, regenerateCatalog: true) + + // Then - Should use the setting value + #expect(mockSyncService.lastAllowCellular == true) + #expect(mockSiteSettings.getPOSLocalCatalogCellularDataAllowedCalled == true) + } + + @Test func performFullSync_respects_cellular_setting_for_subsequent_syncs_when_false() async throws { + // Given - Previous sync exists + try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: Date().addingTimeInterval(-2 * 60 * 60)) + mockSiteSettings.mockPOSLocalCatalogCellularDataAllowed = false + + // When + try await sut.performFullSync(for: sampleSiteID, regenerateCatalog: true) + + // Then - Should use the setting value + #expect(mockSyncService.lastAllowCellular == false) + #expect(mockSiteSettings.getPOSLocalCatalogCellularDataAllowedCalled == true) + } + + @Test func performFullSync_allows_cellular_for_first_sync_even_when_setting_is_false() async throws { + // Given - No previous sync AND setting is explicitly false + mockSiteSettings.mockPOSLocalCatalogCellularDataAllowed = false + + // When + try await sut.performFullSync(for: sampleSiteID) + + // Then - Should allow cellular for first sync, overriding the setting + #expect(mockSyncService.lastAllowCellular == true) + // Setting should not be checked for first sync (it's overridden) + } } extension POSCatalogSyncCoordinator { From f9d6100d820391328a60a48aec2c288884701315 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 5 Nov 2025 12:49:19 +0000 Subject: [PATCH 2/4] Enable setting cellular access on API requests --- .../Requests/JetpackRequest.swift | 19 +++- .../NetworkingCore/Requests/RESTRequest.swift | 39 +++++++-- .../Requests/JetpackRequestTests.swift | 66 ++++++++++++++ .../Requests/RESTRequestTests.swift | 86 +++++++++++++++++++ 4 files changed, 199 insertions(+), 11 deletions(-) diff --git a/Modules/Sources/NetworkingCore/Requests/JetpackRequest.swift b/Modules/Sources/NetworkingCore/Requests/JetpackRequest.swift index 116a87de3f3..621851d639b 100644 --- a/Modules/Sources/NetworkingCore/Requests/JetpackRequest.swift +++ b/Modules/Sources/NetworkingCore/Requests/JetpackRequest.swift @@ -38,6 +38,10 @@ public struct JetpackRequest: Request, RESTRequestConvertible { /// private let availableAsRESTRequest: Bool + /// Whether this request should allow cellular access. + /// + private let allowsCellularAccess: Bool + /// Designated Initializer. /// @@ -48,6 +52,7 @@ public struct JetpackRequest: Request, RESTRequestConvertible { /// - path: RPC that should be called. /// - parameters: Collection of Key/Value parameters, to be forwarded to the Jetpack Connected site. /// - availableAsRESTRequest: Whether the request should be transformed to a REST request if application password is available. + /// - allowsCellularAccess: Whether the request should allow cellular data access. /// public init(wooApiVersion: WooAPIVersion, method: HTTPMethod, @@ -55,7 +60,8 @@ public struct JetpackRequest: Request, RESTRequestConvertible { locale: String? = nil, path: String, parameters: [String: Any]? = nil, - availableAsRESTRequest: Bool = false) { + availableAsRESTRequest: Bool = false, + allowsCellularAccess: Bool = true) { if [.mark1, .mark2].contains(wooApiVersion) { DDLogWarn("⚠️ You are using an older version of the Woo REST API: \(wooApiVersion.rawValue), for path: \(path)") } @@ -66,6 +72,7 @@ public struct JetpackRequest: Request, RESTRequestConvertible { self.path = path self.parameters = parameters ?? [:] self.availableAsRESTRequest = availableAsRESTRequest + self.allowsCellularAccess = allowsCellularAccess } @@ -73,7 +80,8 @@ public struct JetpackRequest: Request, RESTRequestConvertible { /// public func asURLRequest() throws -> URLRequest { let dotcomEndpoint = DotcomRequest(wordpressApiVersion: JetpackRequest.wordpressApiVersion, method: dotcomMethod, path: dotcomPath) - let dotcomRequest = try dotcomEndpoint.asURLRequest() + var dotcomRequest = try dotcomEndpoint.asURLRequest() + dotcomRequest.allowsCellularAccess = allowsCellularAccess return try dotcomEncoder.encode(dotcomRequest, with: dotcomParams) } @@ -86,7 +94,12 @@ public struct JetpackRequest: Request, RESTRequestConvertible { guard availableAsRESTRequest else { return nil } - return RESTRequest(siteURL: siteURL, wooApiVersion: wooApiVersion, method: method, path: path, parameters: parameters) + return RESTRequest(siteURL: siteURL, + wooApiVersion: wooApiVersion, + method: method, + path: path, + parameters: parameters, + allowsCellularAccess: allowsCellularAccess) } } diff --git a/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift b/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift index 2e8f28cfadf..b0fb84ad38c 100644 --- a/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift +++ b/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift @@ -24,16 +24,22 @@ public struct RESTRequest: Request { /// let parameters: [String: Any]? + /// Whether this request should allow cellular access. + /// + let allowsCellularAccess: Bool + private init(siteURL: String, apiVersionPath: String?, method: HTTPMethod, path: String, - parameters: [String: Any]? = nil) { + parameters: [String: Any]? = nil, + allowsCellularAccess: Bool = true) { self.siteURL = siteURL self.apiVersionPath = apiVersionPath self.method = method self.path = path self.parameters = parameters + self.allowsCellularAccess = allowsCellularAccess } /// - Parameters: @@ -41,12 +47,14 @@ public struct RESTRequest: Request { /// - method: HTTP Method we should use. /// - path: path to the target endpoint. /// - parameters: Collection of String parameters to be passed over to our target endpoint. + /// - allowsCellularAccess: Whether the request should allow cellular data access. /// public init(siteURL: String, method: HTTPMethod, path: String, - parameters: [String: Any]? = nil) { - self.init(siteURL: siteURL, apiVersionPath: nil, method: method, path: path, parameters: parameters) + parameters: [String: Any]? = nil, + allowsCellularAccess: Bool = true) { + self.init(siteURL: siteURL, apiVersionPath: nil, method: method, path: path, parameters: parameters, allowsCellularAccess: allowsCellularAccess) } /// - Parameters: @@ -55,13 +63,20 @@ public struct RESTRequest: Request { /// - method: HTTP Method we should use. /// - path: path to the target endpoint. /// - parameters: Collection of String parameters to be passed over to our target endpoint. + /// - allowsCellularAccess: Whether the request should allow cellular data access. /// init(siteURL: String, wooApiVersion: WooAPIVersion, method: HTTPMethod, path: String, - parameters: [String: Any]? = nil) { - self.init(siteURL: siteURL, apiVersionPath: wooApiVersion.path, method: method, path: path, parameters: parameters) + parameters: [String: Any]? = nil, + allowsCellularAccess: Bool = true) { + self.init(siteURL: siteURL, + apiVersionPath: wooApiVersion.path, + method: method, + path: path, + parameters: parameters, + allowsCellularAccess: allowsCellularAccess) } /// - Parameters: @@ -70,13 +85,20 @@ public struct RESTRequest: Request { /// - method: HTTP Method we should use. /// - path: path to the target endpoint. /// - parameters: Collection of String parameters to be passed over to our target endpoint. + /// - allowsCellularAccess: Whether the request should allow cellular data access. /// init(siteURL: String, wordpressApiVersion: WordPressAPIVersion, method: HTTPMethod, path: String, - parameters: [String: Any]? = nil) { - self.init(siteURL: siteURL, apiVersionPath: wordpressApiVersion.path, method: method, path: path, parameters: parameters) + parameters: [String: Any]? = nil, + allowsCellularAccess: Bool = true) { + self.init(siteURL: siteURL, + apiVersionPath: wordpressApiVersion.path, + method: method, + path: path, + parameters: parameters, + allowsCellularAccess: allowsCellularAccess) } /// Returns a URLRequest instance representing the current REST API Request. @@ -87,7 +109,8 @@ public struct RESTRequest: Request { .map { $0.trimSlashes() } .filter { $0.isEmpty == false } let url = try components.joined(separator: "/").asURL() - let request = try URLRequest(url: url, method: method) + var request = try URLRequest(url: url, method: method) + request.allowsCellularAccess = allowsCellularAccess switch method { case .post, .put: return try JSONEncoding.default.encode(request, with: parameters) diff --git a/Modules/Tests/NetworkingTests/Requests/JetpackRequestTests.swift b/Modules/Tests/NetworkingTests/Requests/JetpackRequestTests.swift index 679f51aa598..02025e212e7 100644 --- a/Modules/Tests/NetworkingTests/Requests/JetpackRequestTests.swift +++ b/Modules/Tests/NetworkingTests/Requests/JetpackRequestTests.swift @@ -161,6 +161,72 @@ final class JetpackRequestTests: XCTestCase { // Then XCTAssertNil(request.asRESTRequest(with: sampleSiteAddress)) } + + // MARK: - allowsCellularAccess Tests + + func test_request_with_allowsCellularAccess_true_sets_URLRequest_property() throws { + // Given + let request = JetpackRequest(wooApiVersion: .mark3, + method: .get, + siteID: sampleSiteID, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: true) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertTrue(urlRequest.allowsCellularAccess) + } + + func test_request_with_allowsCellularAccess_false_sets_URLRequest_property() throws { + // Given + let request = JetpackRequest(wooApiVersion: .mark3, + method: .get, + siteID: sampleSiteID, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: false) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertFalse(urlRequest.allowsCellularAccess) + } + + func test_request_defaults_to_allowsCellularAccess_true() throws { + // Given - no explicit allowsCellularAccess parameter + let request = JetpackRequest(wooApiVersion: .mark3, + method: .get, + siteID: sampleSiteID, + path: sampleRPC, + parameters: sampleParameters) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertTrue(urlRequest.allowsCellularAccess) + } + + func test_converting_to_RESTRequest_preserves_allowsCellularAccess() throws { + // Given + let request = JetpackRequest(wooApiVersion: .mark3, + method: .post, + siteID: sampleSiteID, + path: sampleRPC, + parameters: sampleParameters, + availableAsRESTRequest: true, + allowsCellularAccess: false) + + // When + let restRequest = try XCTUnwrap(request.asRESTRequest(with: sampleSiteAddress)) + + // Then + XCTAssertFalse(restRequest.allowsCellularAccess) + } } diff --git a/Modules/Tests/NetworkingTests/Requests/RESTRequestTests.swift b/Modules/Tests/NetworkingTests/Requests/RESTRequestTests.swift index c4e1215cc52..e8992fa95b4 100644 --- a/Modules/Tests/NetworkingTests/Requests/RESTRequestTests.swift +++ b/Modules/Tests/NetworkingTests/Requests/RESTRequestTests.swift @@ -146,4 +146,90 @@ final class RESTRequestTests: XCTestCase { XCTAssertNotNil(urlRequest.httpBody) } } + + // MARK: - allowsCellularAccess Tests + + func test_request_with_allowsCellularAccess_true_sets_URLRequest_property() throws { + // Given + let request = RESTRequest(siteURL: sampleSiteAddress, + wooApiVersion: sampleWooApiVersion, + method: .get, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: true) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertTrue(urlRequest.allowsCellularAccess) + } + + func test_request_with_allowsCellularAccess_false_sets_URLRequest_property() throws { + // Given + let request = RESTRequest(siteURL: sampleSiteAddress, + wooApiVersion: sampleWooApiVersion, + method: .get, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: false) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertFalse(urlRequest.allowsCellularAccess) + } + + func test_request_defaults_to_allowsCellularAccess_true() throws { + // Given - no explicit allowsCellularAccess parameter + let request = RESTRequest(siteURL: sampleSiteAddress, + wooApiVersion: sampleWooApiVersion, + method: .get, + path: sampleRPC, + parameters: sampleParameters) + + // When + let urlRequest = try request.asURLRequest() + + // Then + XCTAssertTrue(urlRequest.allowsCellularAccess) + } + + func test_request_with_allowsCellularAccess_works_for_all_initializers() throws { + // Given - Test all three initializers + + // 1. Simple initializer + let simpleRequest = RESTRequest(siteURL: sampleSiteAddress, + method: .get, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: false) + + // 2. WooApiVersion initializer + let wooRequest = RESTRequest(siteURL: sampleSiteAddress, + wooApiVersion: sampleWooApiVersion, + method: .get, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: false) + + // 3. WordPressApiVersion initializer + let wpRequest = RESTRequest(siteURL: sampleSiteAddress, + wordpressApiVersion: .wpMark2, + method: .get, + path: sampleRPC, + parameters: sampleParameters, + allowsCellularAccess: false) + + // When/Then + let simpleURLRequest = try simpleRequest.asURLRequest() + XCTAssertFalse(simpleURLRequest.allowsCellularAccess) + + let wooURLRequest = try wooRequest.asURLRequest() + XCTAssertFalse(wooURLRequest.allowsCellularAccess) + + let wpURLRequest = try wpRequest.asURLRequest() + XCTAssertFalse(wpURLRequest.allowsCellularAccess) + } } From 5df989cb49b1ef06f0c99362b1135d1940653087 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 5 Nov 2025 12:50:26 +0000 Subject: [PATCH 3/4] Make full catalog syncs respect cellular setting --- .../Remote/POSCatalogSyncRemote.swift | 28 +++++--- .../Tools/POS/POSCatalogFullSyncService.swift | 22 +++--- .../Remote/POSCatalogSyncRemoteTests.swift | 69 ++++++++++++++++--- .../Mocks/MockPOSCatalogSyncRemote.swift | 7 +- 4 files changed, 95 insertions(+), 31 deletions(-) diff --git a/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift b/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift index 8dfb87bb1a2..31901636e8f 100644 --- a/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift +++ b/Modules/Sources/Networking/Remote/POSCatalogSyncRemote.swift @@ -30,10 +30,11 @@ public protocol POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to generate catalog for. /// - forceGeneration: Whether to always generate a catalog. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Catalog job response with job ID. /// // periphery:ignore - TODO - remove this periphery ignore comment when this endpoint is integrated with catalog sync - func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool) async throws -> POSCatalogRequestResponse + func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool, allowCellular: Bool) async throws -> POSCatalogRequestResponse /// Downloads the generated catalog at the specified download URL. /// - Parameters: @@ -50,16 +51,18 @@ public protocol POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to load products from. /// - pageNumber: Page number for pagination. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Paginated list of POS products. - func loadProducts(siteID: Int64, pageNumber: Int) async throws -> PagedItems + func loadProducts(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems /// Loads POS product variations for full sync. /// /// - Parameters: /// - siteID: Site ID to load variations from. /// - pageNumber: Page number for pagination. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Paginated list of POS product variations. - func loadProductVariations(siteID: Int64, pageNumber: Int) async throws -> PagedItems + func loadProductVariations(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems /// Gets the total count of products for the specified site. /// @@ -151,9 +154,11 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// /// - Parameters: /// - siteID: Site ID to generate catalog for. + /// - forceGeneration: Whether to always generate a catalog. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Catalog job response with job ID. /// - public func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool) async throws -> POSCatalogRequestResponse { + public func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool, allowCellular: Bool) async throws -> POSCatalogRequestResponse { let path = "products/catalog" let parameters: [String: Any] = [ ParameterKey.fullSyncFields: POSProduct.requestFields, @@ -165,7 +170,8 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { siteID: siteID, path: path, parameters: parameters, - availableAsRESTRequest: true + availableAsRESTRequest: true, + allowsCellularAccess: allowCellular ) let mapper = SingleItemMapper(siteID: siteID) return try await enqueue(request, mapper: mapper) @@ -200,9 +206,10 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to load products from. /// - pageNumber: Page number for pagination. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Paginated list of POS products. /// - public func loadProducts(siteID: Int64, pageNumber: Int) async throws -> PagedItems { + public func loadProducts(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems { let path = Path.products let parameters = [ ParameterKey.page: String(pageNumber), @@ -216,7 +223,8 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { siteID: siteID, path: path, parameters: parameters, - availableAsRESTRequest: true + availableAsRESTRequest: true, + allowsCellularAccess: allowCellular ) let mapper = ListMapper(siteID: siteID) let (products, responseHeaders) = try await enqueueWithResponseHeaders(request, mapper: mapper) @@ -229,9 +237,10 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { /// - Parameters: /// - siteID: Site ID to load variations from. /// - pageNumber: Page number for pagination. + /// - allowCellular: Should cellular data be used if required. /// - Returns: Paginated list of POS product variations. /// - public func loadProductVariations(siteID: Int64, pageNumber: Int) async throws -> PagedItems { + public func loadProductVariations(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems { let path = Path.variations let parameters = [ ParameterKey.page: String(pageNumber), @@ -245,7 +254,8 @@ public class POSCatalogSyncRemote: Remote, POSCatalogSyncRemoteProtocol { siteID: siteID, path: path, parameters: parameters, - availableAsRESTRequest: true + availableAsRESTRequest: true, + allowsCellularAccess: allowCellular ) let mapper = ListMapper(siteID: siteID) let (variations, responseHeaders) = try await enqueueWithResponseHeaders(request, mapper: mapper) diff --git a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift index da56ef77858..19bbce27427 100644 --- a/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift +++ b/Modules/Sources/Yosemite/Tools/POS/POSCatalogFullSyncService.swift @@ -84,7 +84,7 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol regenerateCatalog: regenerateCatalog, allowCellular: allowCellular) } else { - catalog = try await loadCatalog(for: siteID, syncRemote: syncRemote) + catalog = try await loadCatalog(for: siteID, syncRemote: syncRemote, allowCellular: allowCellular) } DDLogInfo("✅ Loaded \(catalog.products.count) products and \(catalog.variations.count) variations for siteID \(siteID)") @@ -103,17 +103,17 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol // MARK: - Remote Loading private extension POSCatalogFullSyncService { - func loadCatalog(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol) async throws -> POSCatalog { + func loadCatalog(for siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol, allowCellular: Bool) async throws -> POSCatalog { let syncStartDate = Date.now // Loads products and variations in batches in parallel. async let productsTask = batchedLoader.loadAll( makeRequest: { pageNumber in - try await syncRemote.loadProducts(siteID: siteID, pageNumber: pageNumber) + try await syncRemote.loadProducts(siteID: siteID, pageNumber: pageNumber, allowCellular: allowCellular) } ) async let variationsTask = batchedLoader.loadAll( makeRequest: { pageNumber in - try await syncRemote.loadProductVariations(siteID: siteID, pageNumber: pageNumber) + try await syncRemote.loadProductVariations(siteID: siteID, pageNumber: pageNumber, allowCellular: allowCellular) } ) @@ -145,13 +145,15 @@ private extension POSCatalogFullSyncService { DDLogInfo("🟣 Starting catalog request...") // 1. Requests catalog until download URL is available. - let response = try await syncRemote.requestCatalogGeneration(for: siteID, forceGeneration: regenerateCatalog) + let response = try await syncRemote.requestCatalogGeneration(for: siteID, forceGeneration: regenerateCatalog, allowCellular: allowCellular) 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) + downloadURL = try await pollForCatalogCompletion(siteID: siteID, + syncRemote: syncRemote, + allowCellular: allowCellular) } // 3. Downloads catalog using the provided URL. @@ -162,13 +164,17 @@ private extension POSCatalogFullSyncService { return try await syncRemote.downloadCatalog(for: siteID, downloadURL: downloadURL, allowCellular: allowCellular) } - func pollForCatalogCompletion(siteID: Int64, syncRemote: POSCatalogSyncRemoteProtocol) async throws -> String { + func pollForCatalogCompletion(siteID: Int64, + syncRemote: POSCatalogSyncRemoteProtocol, + allowCellular: Bool) 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) + let response = try await syncRemote.requestCatalogGeneration(for: siteID, + forceGeneration: false, + allowCellular: allowCellular) switch response.status { case .complete: diff --git a/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift b/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift index 7c0cc4b7478..664b05ae153 100644 --- a/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift +++ b/Modules/Tests/NetworkingTests/Remote/POSCatalogSyncRemoteTests.swift @@ -290,7 +290,7 @@ struct POSCatalogSyncRemoteTests { network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array") // When loading page 1 - let pagedProducts = try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1) + let pagedProducts = try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) // Then there are more pages #expect(pagedProducts.hasMorePages == true) @@ -304,7 +304,7 @@ struct POSCatalogSyncRemoteTests { let pageNumber = 2 // When - _ = try? await remote.loadProducts(siteID: sampleSiteID, pageNumber: pageNumber) + _ = try? await remote.loadProducts(siteID: sampleSiteID, pageNumber: pageNumber, allowCellular: true) // Then let queryParametersDictionary = try #require(network.queryParametersDictionary as? [String: any Hashable]) @@ -321,7 +321,7 @@ struct POSCatalogSyncRemoteTests { // When network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-pos") - let pagedProducts = try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1) + let pagedProducts = try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) // Then #expect(pagedProducts.items.count == expectedProductsCount) @@ -338,7 +338,7 @@ struct POSCatalogSyncRemoteTests { // When/Then await #expect(throws: NetworkError.notFound()) { - try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1) + try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) } } @@ -350,7 +350,7 @@ struct POSCatalogSyncRemoteTests { let pageNumber = 3 // When - _ = try? await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: pageNumber) + _ = try? await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: pageNumber, allowCellular: true) // Then let queryParametersDictionary = try #require(network.queryParametersDictionary as? [String: any Hashable]) @@ -366,7 +366,7 @@ struct POSCatalogSyncRemoteTests { // When network.simulateResponse(requestUrlSuffix: "variations", filename: "product-variations-load-pos") - let pagedVariations = try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1) + let pagedVariations = try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) // Then #expect(pagedVariations.items.count == 1) @@ -383,7 +383,7 @@ struct POSCatalogSyncRemoteTests { // When/Then await #expect(throws: NetworkError.notFound()) { - try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1) + try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) } } @@ -394,7 +394,7 @@ struct POSCatalogSyncRemoteTests { network.simulateResponse(requestUrlSuffix: "variations", filename: "empty-data-array") // When loading page 1 - let pagedVariations = try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1) + let pagedVariations = try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1, allowCellular: true) // Then there are more pages #expect(pagedVariations.hasMorePages == true) @@ -598,7 +598,7 @@ struct POSCatalogSyncRemoteTests { let remote = POSCatalogSyncRemote(network: network) // When - _ = try? await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false) + _ = try? await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false, allowCellular: true) // Then let queryParametersDictionary = try #require(network.queryParametersDictionary as? [String: any Hashable]) @@ -612,7 +612,7 @@ struct POSCatalogSyncRemoteTests { // When network.simulateResponse(requestUrlSuffix: "catalog", filename: "pos-catalog-generation") - let response = try await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false) + let response = try await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false, allowCellular: true) // Then #expect(response.status == .complete) @@ -625,7 +625,7 @@ struct POSCatalogSyncRemoteTests { // When/Then await #expect(throws: NetworkError.notFound()) { - try await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false) + try await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false, allowCellular: true) } } @@ -743,4 +743,51 @@ struct POSCatalogSyncRemoteTests { let urlRequest = try #require(network.requestsForResponseData.last as? URLRequest) #expect(urlRequest.allowsCellularAccess == allowCellular) } + + // MARK: - Full Sync allowCellular Tests + + @Test(arguments: [true, false]) + func requestCatalogGeneration_sets_allowsCellularAccess_on_request(allowCellular: Bool) async throws { + // Given + let remote = POSCatalogSyncRemote(network: network) + network.simulateResponse(requestUrlSuffix: "catalog", filename: "pos-catalog-generation") + + // When + _ = try await remote.requestCatalogGeneration(for: sampleSiteID, forceGeneration: false, allowCellular: allowCellular) + + // Then + let jetpackRequest = try #require(network.requestsForResponseData.last as? JetpackRequest) + let urlRequest = try jetpackRequest.asURLRequest() + #expect(urlRequest.allowsCellularAccess == allowCellular) + } + + @Test(arguments: [true, false]) + func loadProducts_fullSync_sets_allowsCellularAccess_on_request(allowCellular: Bool) async throws { + // Given + let remote = POSCatalogSyncRemote(network: network) + network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array") + + // When + _ = try await remote.loadProducts(siteID: sampleSiteID, pageNumber: 1, allowCellular: allowCellular) + + // Then + let jetpackRequest = try #require(network.requestsForResponseData.last as? JetpackRequest) + let urlRequest = try jetpackRequest.asURLRequest() + #expect(urlRequest.allowsCellularAccess == allowCellular) + } + + @Test(arguments: [true, false]) + func loadProductVariations_fullSync_sets_allowsCellularAccess_on_request(allowCellular: Bool) async throws { + // Given + let remote = POSCatalogSyncRemote(network: network) + network.simulateResponse(requestUrlSuffix: "variations", filename: "empty-data-array") + + // When + _ = try await remote.loadProductVariations(siteID: sampleSiteID, pageNumber: 1, allowCellular: allowCellular) + + // Then + let jetpackRequest = try #require(network.requestsForResponseData.last as? JetpackRequest) + let urlRequest = try jetpackRequest.asURLRequest() + #expect(urlRequest.allowsCellularAccess == allowCellular) + } } diff --git a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift index 8754c3684c0..805ece37cc6 100644 --- a/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift +++ b/Modules/Tests/YosemiteTests/Mocks/MockPOSCatalogSyncRemote.swift @@ -103,7 +103,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { // MARK: - Protocol Methods - Full Sync - func loadProducts(siteID: Int64, pageNumber: Int) async throws -> PagedItems { + func loadProducts(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems { await loadProductsCallCount.increment() if let result = productResults[pageNumber] { @@ -117,7 +117,7 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { return fallbackResult } - func loadProductVariations(siteID: Int64, pageNumber: Int) async throws -> PagedItems { + func loadProductVariations(siteID: Int64, pageNumber: Int, allowCellular: Bool) async throws -> PagedItems { await loadProductVariationsCallCount.increment() if let result = variationResults[pageNumber] { @@ -133,8 +133,9 @@ final class MockPOSCatalogSyncRemote: POSCatalogSyncRemoteProtocol { // MARK: - Protocol Methods - Catalog API - func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool) async throws -> POSCatalogRequestResponse { + func requestCatalogGeneration(for siteID: Int64, forceGeneration: Bool, allowCellular: Bool) async throws -> POSCatalogRequestResponse { lastCatalogRequestForceGeneration = forceGeneration + lastCatalogDownloadAllowCellular = allowCellular switch catalogRequestResult { case .success(let response): return response From 4b644ab7a3e4ced89913b573cf799324ed079536 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 5 Nov 2025 13:56:21 +0000 Subject: [PATCH 4/4] Periphery ignore --- Modules/Sources/NetworkingCore/Requests/RESTRequest.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift b/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift index b0fb84ad38c..a7f45c38b23 100644 --- a/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift +++ b/Modules/Sources/NetworkingCore/Requests/RESTRequest.swift @@ -87,6 +87,7 @@ public struct RESTRequest: Request { /// - parameters: Collection of String parameters to be passed over to our target endpoint. /// - allowsCellularAccess: Whether the request should allow cellular data access. /// + // periphery:ignore - we include the cellular parameter for all inits init(siteURL: String, wordpressApiVersion: WordPressAPIVersion, method: HTTPMethod,