diff --git a/Networking/NetworkingTests/Extensions/URLRequestConvertible+PathTests.swift b/Networking/NetworkingTests/Extensions/URLRequestConvertible+PathTests.swift index 080ddc39932..386b5f6ab5e 100644 --- a/Networking/NetworkingTests/Extensions/URLRequestConvertible+PathTests.swift +++ b/Networking/NetworkingTests/Extensions/URLRequestConvertible+PathTests.swift @@ -12,10 +12,10 @@ final class URLRequestConvertible_PathTests: XCTestCase { // MARK: - `pathForAnalytics` // Example from `ProductsRemote.loadAllProducts`. - func test_pathForAnalytics_returns_path_of_JetpackRequest() throws { + func test_pathForAnalytics_returns_path_of_JetpackRequest() async throws { // Given let productsRemote = ProductsRemote(network: network) - productsRemote.loadAllProducts(for: 134, completion: { _ in }) + _ = try? await productsRemote.loadAllProducts(for: 134) // When let request = try XCTUnwrap(network.requestsForResponseData.first) diff --git a/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift b/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift index a8939351138..9fa16374ea7 100644 --- a/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift +++ b/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift @@ -256,39 +256,28 @@ final class ProductsRemoteTests: XCTestCase { /// Verifies that loadAllProducts properly parses the `products-load-all` sample response. /// - func testLoadAllProductsProperlyReturnsParsedProducts() { + func test_loadAllProducts_properly_returns_parsed_products() async throws { + // Given let remote = ProductsRemote(network: network) - let expectation = self.expectation(description: "Load All Products") - network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all") - remote.loadAllProducts(for: sampleSiteID) { result in - switch result { - case .success(let products): - XCTAssertEqual(products.count, 10) - default: - XCTFail("Unexpected result: \(result)") - } - expectation.fulfill() - } + // When + let products = try await remote.loadAllProducts(for: sampleSiteID) - wait(for: [expectation], timeout: Constants.expectationTimeout) + // Then + XCTAssertEqual(products.count, 10) } /// Verifies that loadAllProducts with `excludedProductIDs` makes a network request with the corresponding parameter. /// - func testLoadAllProductsWithExcludedIDsIncludesAnExcludeParamInNetworkRequest() throws { + func test_loadAllProducts_with_excluded_ids_includes_an_exclude_param_in_network_request() async throws { // Arrange let remote = ProductsRemote(network: network) network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all") let excludedProductIDs: [Int64] = [17, 671] // Action - waitForExpectation { expectation in - remote.loadAllProducts(for: sampleSiteID, excludedProductIDs: excludedProductIDs) { result in - expectation.fulfill() - } - } + _ = try await remote.loadAllProducts(for: sampleSiteID, excludedProductIDs: excludedProductIDs) // Assert let queryParameters = try XCTUnwrap(network.queryParameters) @@ -298,18 +287,14 @@ final class ProductsRemoteTests: XCTestCase { /// Verifies that loadAllProducts with `productIDs` makes a network request with the `include` parameter. /// - func test_loadAllProducts_with_productIDs_adds_a_include_param_in_network_request() throws { + func test_loadAllProducts_with_productIDs_adds_a_include_param_in_network_request() async throws { // Given let remote = ProductsRemote(network: network) network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all") let productIDs: [Int64] = [13, 61] // When - waitForExpectation { expectation in - remote.loadAllProducts(for: sampleSiteID, productIDs: productIDs) { result in - expectation.fulfill() - } - } + _ = try await remote.loadAllProducts(for: sampleSiteID, productIDs: productIDs) // Then let queryParameters = try XCTUnwrap(network.queryParameters) @@ -319,17 +304,13 @@ final class ProductsRemoteTests: XCTestCase { /// Verifies that loadAllProducts with empty `productIDs` makes a network request without the `include` parameter. /// - func test_loadAllProducts_with_empty_productIDs_does_not_add_include_param_in_network_request() throws { + func test_loadAllProducts_with_empty_productIDs_does_not_add_include_param_in_network_request() async throws { // Given let remote = ProductsRemote(network: network) network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all") // When - waitForExpectation { expectation in - remote.loadAllProducts(for: sampleSiteID, productIDs: []) { result in - expectation.fulfill() - } - } + _ = try await remote.loadAllProducts(for: sampleSiteID, productIDs: []) // Then let queryParameters = try XCTUnwrap(network.queryParameters) @@ -339,21 +320,16 @@ final class ProductsRemoteTests: XCTestCase { /// Verifies that loadAllProducts properly relays Networking Layer errors. /// - func test_loadAllProducts_properly_relays_netwoking_errors() { + func test_loadAllProducts_properly_relays_netwoking_errors() async { + // Given let remote = ProductsRemote(network: network) - let expectation = self.expectation(description: "Load all products returns error") - - remote.loadAllProducts(for: sampleSiteID) { result in - switch result { - case .failure: - break - default: - XCTFail("Unexpected result: \(result)") - } - expectation.fulfill() - } - wait(for: [expectation], timeout: Constants.expectationTimeout) + do { + _ = try await remote.loadAllProducts(for: sampleSiteID) + XCTFail("Expected error to be thrown") + } catch { + XCTAssertEqual(error as? NetworkError, .notFound()) + } } diff --git a/Networking/Sources/Extended/Remote/ProductsRemote.swift b/Networking/Sources/Extended/Remote/ProductsRemote.swift index 1c167070fbb..5fddc32cbc5 100644 --- a/Networking/Sources/Extended/Remote/ProductsRemote.swift +++ b/Networking/Sources/Extended/Remote/ProductsRemote.swift @@ -20,8 +20,7 @@ public protocol ProductsRemoteProtocol { orderBy: ProductsRemote.OrderKey, order: ProductsRemote.Order, productIDs: [Int64], - excludedProductIDs: [Int64], - completion: @escaping (Result<[Product], Error>) -> Void) + excludedProductIDs: [Int64]) async throws -> [Product] func searchProducts(for siteID: Int64, keyword: String, pageNumber: Int, @@ -179,8 +178,7 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol { orderBy: OrderKey = .name, order: Order = .ascending, productIDs: [Int64] = [], - excludedProductIDs: [Int64] = [], - completion: @escaping (Result<[Product], Error>) -> Void) { + excludedProductIDs: [Int64] = []) async throws -> [Product] { let stringOfExcludedProductIDs = excludedProductIDs.map { String($0) } .joined(separator: ",") let stringOfProductIDs: String = { @@ -211,9 +209,9 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol { let path = Path.products let request = JetpackRequest(wooApiVersion: .mark3, method: .get, siteID: siteID, path: path, parameters: parameters, availableAsRESTRequest: true) - let mapper = ProductListMapper(siteID: siteID) + let mapper = ListMapper(siteID: siteID) - enqueue(request, mapper: mapper, completion: completion) + return try await enqueue(request, mapper: mapper) } /// Retrieves products for the Point of Sale. Simple and variable products are loaded for WC version 9.6+, otherwise only simple products are loaded. diff --git a/Yosemite/Yosemite/Stores/ProductStore.swift b/Yosemite/Yosemite/Stores/ProductStore.swift index a68c1df7a88..82b5d10e8b2 100644 --- a/Yosemite/Yosemite/Stores/ProductStore.swift +++ b/Yosemite/Yosemite/Stores/ProductStore.swift @@ -90,18 +90,24 @@ public class ProductStore: Store { let excludedProductIDs, let shouldDeleteStoredProductsOnFirstPage, let onCompletion): - synchronizeProducts(siteID: siteID, - pageNumber: pageNumber, - pageSize: pageSize, - stockStatus: stockStatus, - productStatus: productStatus, - productType: productType, - productCategory: productCategory, - sortOrder: sortOrder, - productIDs: productIDs, - excludedProductIDs: excludedProductIDs, - shouldDeleteStoredProductsOnFirstPage: shouldDeleteStoredProductsOnFirstPage, - onCompletion: onCompletion) + Task { @MainActor in + do { + let hasNextPage = try await synchronizeProducts(siteID: siteID, + pageNumber: pageNumber, + pageSize: pageSize, + stockStatus: stockStatus, + productStatus: productStatus, + productType: productType, + productCategory: productCategory, + sortOrder: sortOrder, + productIDs: productIDs, + excludedProductIDs: excludedProductIDs, + shouldDeleteStoredProductsOnFirstPage: shouldDeleteStoredProductsOnFirstPage) + onCompletion(.success(hasNextPage)) + } catch { + onCompletion(.failure(error)) + } + } case .requestMissingProducts(let order, let onCompletion): requestMissingProducts(for: order, onCompletion: onCompletion) case .updateProduct(let product, let onCompletion): @@ -287,43 +293,35 @@ private extension ProductStore { sortOrder: ProductsSortOrder, productIDs: [Int64], excludedProductIDs: [Int64], - shouldDeleteStoredProductsOnFirstPage: Bool, - onCompletion: @escaping (Result) -> Void) { - remote.loadAllProducts(for: siteID, - context: nil, - pageNumber: pageNumber, - pageSize: pageSize, - stockStatus: stockStatus, - productStatus: productStatus, - productType: productType, - productCategory: productCategory, - orderBy: sortOrder.remoteOrderKey, - order: sortOrder.remoteOrder, - productIDs: productIDs, - excludedProductIDs: excludedProductIDs) { [weak self] result in - switch result { - case .failure(let error): - if let productType, - let error = error as? DotcomError, - case let .unknown(code, message) = error, - code == "rest_invalid_param", - message == "Invalid parameter(s): type", - ProductType.coreTypes.contains(productType) == false { - return onCompletion(.success(false)) - } - onCompletion(.failure(error)) - case .success(let products): - guard let self = self else { - return - } - let shouldDeleteExistingProducts = pageNumber == Default.firstPageNumber && shouldDeleteStoredProductsOnFirstPage - self.upsertStoredProductsInBackground(readOnlyProducts: products, - siteID: siteID, - shouldDeleteExistingProducts: shouldDeleteExistingProducts) { - let hasNextPage = products.count == pageSize - onCompletion(.success(hasNextPage)) - } - } + shouldDeleteStoredProductsOnFirstPage: Bool) async throws -> Bool { + do { + let products = try await remote.loadAllProducts(for: siteID, + context: nil, + pageNumber: pageNumber, + pageSize: pageSize, + stockStatus: stockStatus, + productStatus: productStatus, + productType: productType, + productCategory: productCategory, + orderBy: sortOrder.remoteOrderKey, + order: sortOrder.remoteOrder, + productIDs: productIDs, + excludedProductIDs: excludedProductIDs) + + let shouldDeleteExistingProducts = pageNumber == Default.firstPageNumber && shouldDeleteStoredProductsOnFirstPage + await upsertStoredProductsInBackground(readOnlyProducts: products, + siteID: siteID, + shouldDeleteExistingProducts: shouldDeleteExistingProducts) + let hasNextPage = products.count == pageSize + return hasNextPage + } catch let error as DotcomError where error == .unknown(code: "rest_invalid_param", message: "Invalid parameter(s): type") { + if let productType, + ProductType.coreTypes.contains(productType) == false { + return false + } + throw error + } catch { + throw error } } @@ -852,6 +850,23 @@ extension ProductStore { }, completion: onCompletion, on: .main) } + /// Updates (OR Inserts) the specified ReadOnly Product Entities *in a background thread* async. + /// Also deletes existing products if requested. + func upsertStoredProductsInBackground(readOnlyProducts: [Networking.Product], + siteID: Int64, + shouldDeleteExistingProducts: Bool = false) async { + await withCheckedContinuation { [weak self] continuation in + guard let self else { + return continuation.resume() + } + upsertStoredProductsInBackground(readOnlyProducts: readOnlyProducts, + siteID: siteID, + shouldDeleteExistingProducts: shouldDeleteExistingProducts) { + continuation.resume() + } + } + } + /// Updates (OR Inserts) the specified ReadOnly Product Entities *in a background thread*. /// Also deletes existing products if requested. /// `onCompletion` will be called on the main thread! @@ -861,7 +876,9 @@ extension ProductStore { shouldDeleteExistingProducts: Bool = false, onCompletion: @escaping () -> Void) { storageManager.performAndSave({ [weak self] storage in - guard let self else { return } + guard let self else { + return onCompletion() + } if shouldDeleteExistingProducts { storage.deleteProducts(siteID: siteID) } diff --git a/Yosemite/YosemiteTests/Mocks/Networking/Remote/MockProductsRemote.swift b/Yosemite/YosemiteTests/Mocks/Networking/Remote/MockProductsRemote.swift index 01802e6367f..8d9e4df2ebe 100644 --- a/Yosemite/YosemiteTests/Mocks/Networking/Remote/MockProductsRemote.swift +++ b/Yosemite/YosemiteTests/Mocks/Networking/Remote/MockProductsRemote.swift @@ -260,8 +260,7 @@ extension MockProductsRemote: ProductsRemoteProtocol { orderBy: ProductsRemote.OrderKey, order: ProductsRemote.Order, productIDs: [Int64], - excludedProductIDs: [Int64], - completion: @escaping (Result<[Product], Error>) -> Void) { + excludedProductIDs: [Int64]) async throws -> [Product] { synchronizeProductsTriggered = true synchronizeProductsWithStockStatus = stockStatus synchronizeProductsWithProductStatus = productStatus @@ -271,10 +270,16 @@ extension MockProductsRemote: ProductsRemoteProtocol { synchronizeProductsOrder = order synchronizeProductsProductIDs = productIDs synchronizeProductsExcludedProductIDs = excludedProductIDs - if let result = loadAllProductsResultsBySiteID[siteID] { - completion(result) - } else { + guard let result = loadAllProductsResultsBySiteID[siteID] else { XCTFail("\(String(describing: self)) Could not find Result for \(siteID)") + throw NetworkError.notFound() + } + + switch result { + case let .success(products): + return products + case let .failure(error): + throw error } } diff --git a/Yosemite/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift b/Yosemite/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift index 26ad2c1a0a5..608007b038e 100644 --- a/Yosemite/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift +++ b/Yosemite/YosemiteTests/Stores/ProductStore+FilterProductsTests.swift @@ -285,18 +285,21 @@ final class ProductStore_FilterProductsTests: XCTestCase { remote.whenLoadingAllProducts(siteID: sampleSiteID, thenReturn: .success([Product.fake()])) // When - let synchronizeAction = ProductAction.synchronizeProducts(siteID: sampleSiteID, - pageNumber: defaultPageNumber, - pageSize: defaultPageSize, - stockStatus: filteredStockStatus, - productStatus: filteredProductStatus, - productType: filteredProductType, - productCategory: filteredProductCategory, - sortOrder: filteredProductSortOrder, - productIDs: [1, 2], - excludedProductIDs: [30, 45], - onCompletion: { _ in }) - productStore.onAction(synchronizeAction) + waitFor { promise in + productStore.onAction(ProductAction.synchronizeProducts(siteID: self.sampleSiteID, + pageNumber: self.defaultPageNumber, + pageSize: self.defaultPageSize, + stockStatus: filteredStockStatus, + productStatus: filteredProductStatus, + productType: filteredProductType, + productCategory: filteredProductCategory, + sortOrder: filteredProductSortOrder, + productIDs: [1, 2], + excludedProductIDs: [30, 45], + onCompletion: { _ in + promise(()) + })) + } // Then XCTAssertTrue(remote.synchronizeProductsTriggered)