Update OrdersRemote with async/throws network methods to parse orders response in a background thread (#15982)

Author: jaclync

Modules/Sources/NetworkingCore/Remote/OrdersRemote.swift

@@ -25,7 +25,8 @@ public class OrdersRemote: Remote {
     ///     - createdVia: If given, limit response to orders created via the specified source (e.g. "pos-rest-api" for Point of Sale).
     ///     - pageNumber: Number of page that should be retrieved.
     ///     - pageSize: Number of Orders to be retrieved per page.
-    ///     - completion: Closure to be executed upon completion.
+    /// - Returns: Array of orders.
+    /// - Throws: Network or parsing errors.
     ///
     public func loadAllOrders(for siteID: Int64,
                               statuses: [String]? = nil,
@@ -36,8 +37,7 @@ public class OrdersRemote: Remote {
                               productID: Int64? = nil,
                               createdVia: String? = nil,
                               pageNumber: Int = Defaults.pageNumber,
-                              pageSize: Int = Defaults.pageSize,
-                              completion: @escaping (Result<[Order], Error>) -> Void) {
+                              pageSize: Int = Defaults.pageSize) async throws -> [Order] {
         let utcDateFormatter = DateFormatter.Defaults.iso8601
 
         let statusesString: String? = statuses?.isEmpty == true ? Defaults.statusAny : statuses?.joined(separator: ",")
@@ -84,7 +84,7 @@ public class OrdersRemote: Remote {
                                      availableAsRESTRequest: true)
         let mapper = OrderListMapper(siteID: siteID)
 
-        enqueue(request, mapper: mapper, completion: completion)
+        return try await enqueue(request, mapper: mapper)
     }
 
     /// Retrieves a specific `Order`
@@ -138,13 +138,13 @@ public class OrdersRemote: Remote {
     ///     - keyword: Search string that should be matched by the orders.
     ///     - pageNumber: Number of page that should be retrieved.
     ///     - pageSize: Number of Orders to be retrieved per page.
-    ///     - completion: Closure to be executed upon completion.
+    /// - Returns: Array of orders matching the search criteria.
+    /// - Throws: Network or parsing errors.
     ///
     public func searchOrders(for siteID: Int64,
                              keyword: String,
                              pageNumber: Int = Defaults.pageNumber,
-                             pageSize: Int = Defaults.pageSize,
-                             completion: @escaping ([Order]?, Error?) -> Void) {
+                             pageSize: Int = Defaults.pageSize) async throws -> [Order] {
         let parameters = [
             ParameterKeys.keyword: keyword,
             ParameterKeys.page: String(pageNumber),
@@ -162,7 +162,7 @@ public class OrdersRemote: Remote {
                                      availableAsRESTRequest: true)
         let mapper = OrderListMapper(siteID: siteID)
 
-        enqueue(request, mapper: mapper, completion: completion)
+        return try await enqueue(request, mapper: mapper)
     }
 
     /// Creates an order using the specified fields of a given order

Modules/Sources/Yosemite/Stores/OrderStore.swift

@@ -113,14 +113,14 @@ private extension OrderStore {
     /// Searches all of the orders that contain a given Keyword.
     ///
     func searchOrders(siteID: Int64, keyword: String, pageNumber: Int, pageSize: Int, onCompletion: @escaping (Error?) -> Void) {
-        remote.searchOrders(for: siteID, keyword: keyword, pageNumber: pageNumber, pageSize: pageSize) { [weak self] (orders, error) in
-            guard let orders = orders else {
+        Task { @MainActor in
+            do {
+                let orders = try await remote.searchOrders(for: siteID, keyword: keyword, pageNumber: pageNumber, pageSize: pageSize)
+                upsertSearchResultsInBackground(for: siteID, keyword: keyword, readOnlyOrders: orders) {
+                    onCompletion(nil)
+                }
+            } catch {
                 onCompletion(error)
-                return
-            }
-
-            self?.upsertSearchResultsInBackground(for: siteID, keyword: keyword, readOnlyOrders: orders) {
-                onCompletion(nil)
             }
         }
     }
@@ -155,18 +155,19 @@ private extension OrderStore {
 
         let pageNumber = OrdersRemote.Defaults.pageNumber
         let startTime = Date()
-        self.remote.loadAllOrders(for: siteID,
-                                  statuses: statuses,
-                                  after: after,
-                                  before: before,
-                                  modifiedAfter: modifiedAfter,
-                                  customerID: customerID,
-                                  productID: productID,
-                                  createdVia: createdVia,
-                                  pageNumber: pageNumber,
-                                  pageSize: pageSize) { [weak self] result in
-            switch result {
-            case .success(let orders):
+        Task { @MainActor [weak self] in
+            do {
+                let orders = try await self?.remote.loadAllOrders(for: siteID,
+                                                                  statuses: statuses,
+                                                                  after: after,
+                                                                  before: before,
+                                                                  modifiedAfter: modifiedAfter,
+                                                                  customerID: customerID,
+                                                                  productID: productID,
+                                                                  createdVia: createdVia,
+                                                                  pageNumber: pageNumber,
+                                                                  pageSize: pageSize) ?? []
+                let result: Result<[Order], Error> = .success(orders)
                 switch writeStrategy {
                 case .doNotSave:
                     onCompletion(Date().timeIntervalSince(startTime), result)
@@ -176,7 +177,8 @@ private extension OrderStore {
                         onCompletion(Date().timeIntervalSince(startTime), result)
                     }
                 }
-            case .failure:
+            } catch {
+                let result: Result<[Order], Error> = .failure(error)
                 onCompletion(Date().timeIntervalSince(startTime), result)
             }
         }
@@ -196,22 +198,22 @@ private extension OrderStore {
                            pageSize: Int,
                            onCompletion: @escaping (TimeInterval, Error?) -> Void) {
         let startTime = Date()
-        remote.loadAllOrders(for: siteID,
-                             statuses: statuses,
-                             after: after,
-                             before: before,
-                             modifiedAfter: modifiedAfter,
-                             customerID: customerID,
-                             productID: productID,
-                             createdVia: createdVia,
-                             pageNumber: pageNumber,
-                             pageSize: pageSize) { [weak self] result in
-            switch result {
-            case .success(let orders):
-                self?.upsertStoredOrdersInBackground(readOnlyOrders: orders) {
+        Task { @MainActor in
+            do {
+                let orders = try await remote.loadAllOrders(for: siteID,
+                                                            statuses: statuses,
+                                                            after: after,
+                                                            before: before,
+                                                            modifiedAfter: modifiedAfter,
+                                                            customerID: customerID,
+                                                            productID: productID,
+                                                            createdVia: createdVia,
+                                                            pageNumber: pageNumber,
+                                                            pageSize: pageSize)
+                upsertStoredOrdersInBackground(readOnlyOrders: orders) {
                     onCompletion(Date().timeIntervalSince(startTime), nil)
                 }
-            case .failure(let error):
+            } catch {
                 onCompletion(Date().timeIntervalSince(startTime), error)
             }
         }
@@ -228,11 +230,11 @@ private extension OrderStore {
         }
 
         // If there are no locally stored orders, then check remote.
-        remote.loadAllOrders(for: siteID, pageNumber: Default.firstPageNumber, pageSize: 1) { result in
-            switch result {
-            case .success(let orders):
+        Task { @MainActor in
+            do {
+                let orders = try await remote.loadAllOrders(for: siteID, pageNumber: Default.firstPageNumber, pageSize: 1)
                 onCompletion(.success(orders.isEmpty == false))
-            case .failure(let error):
+            } catch {
                 onCompletion(.failure(error))
             }
         }

Modules/Tests/NetworkingTests/Remote/OrdersRemoteTests.swift

@@ -60,57 +60,42 @@ final class OrdersRemoteTests: XCTestCase {
 
     /// Verifies that loadAllOrders properly parses the `orders-load-all` sample response.
     ///
-    func testLoadAllOrdersProperlyReturnsParsedOrders() throws {
+    func test_loadAllOrders_properly_returns_parsed_orders() async throws {
         // Given
         let remote = OrdersRemote(network: network)
 
         network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
 
         // When
-        var result: Result<[Order], Error>?
-        waitForExpectation { expectation in
-            remote.loadAllOrders(for: sampleSiteID) { aResult in
-                result = aResult
-                expectation.fulfill()
-            }
-        }
+        let orders = try await remote.loadAllOrders(for: sampleSiteID)
 
         // Then
-        let orders = try XCTUnwrap(result?.get())
         XCTAssert(orders.count == 4)
     }
 
     /// Verifies that loadAllOrders properly relays Networking Layer errors.
     ///
-    func testLoadAllOrdersProperlyRelaysNetworkingErrors() throws {
+    func test_loadAllOrders_properly_relays_networking_errors() async throws {
         // Given
         let remote = OrdersRemote(network: network)
 
-        // When
-        var result: Result<[Order], Error>?
-        waitForExpectation { expectation in
-            remote.loadAllOrders(for: sampleSiteID) { aResult in
-                result = aResult
-                expectation.fulfill()
-            }
+        // When & Then
+        do {
+            _ = try await remote.loadAllOrders(for: sampleSiteID)
+            XCTFail("Expected error to be thrown")
+        } catch {
+            XCTAssertEqual(error as? NetworkError, .notFound(response: nil))
         }
-
-        // Then
-        XCTAssertTrue(try XCTUnwrap(result).isFailure)
     }
 
-    func test_loadAllOrders_includes_modifiedAfter_parameter_when_provided() {
+    func test_loadAllOrders_includes_modifiedAfter_parameter_when_provided() async throws {
         // Given
         let remote = OrdersRemote(network: network)
         let modifiedAfter = Date()
         network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
 
         // When
-        _ = waitFor { promise in
-            remote.loadAllOrders(for: self.sampleSiteID, modifiedAfter: modifiedAfter) { result in
-                promise(result)
-            }
-        }
+        _ = try await remote.loadAllOrders(for: sampleSiteID, modifiedAfter: modifiedAfter)
 
         // Then
         guard let queryParameters = network.queryParameters else {
@@ -123,18 +108,14 @@ final class OrdersRemoteTests: XCTestCase {
         XCTAssertTrue(queryParameters.contains(expectedParam), "Expected to have param: \(expectedParam)")
     }
 
-    func test_loadAllOrders_includes_customer_parameter_when_provided() {
+    func test_loadAllOrders_includes_customer_parameter_when_provided() async throws {
         // Given
         let remote = OrdersRemote(network: network)
         network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
         let expectedCustomerID: Int64 = 123
 
         // When
-        _ = waitFor { promise in
-            remote.loadAllOrders(for: self.sampleSiteID, customerID: expectedCustomerID) { result in
-                promise(result)
-            }
-        }
+        _ = try await remote.loadAllOrders(for: sampleSiteID, customerID: expectedCustomerID)
 
         // Then
         guard let queryParameters = network.queryParameters else {
@@ -146,18 +127,14 @@ final class OrdersRemoteTests: XCTestCase {
         XCTAssertTrue(queryParameters.contains(expectedParam), "Expected to have param: \(expectedParam)")
     }
 
-    func test_loadAllOrders_includes_product_parameter_when_provided() {
+    func test_loadAllOrders_includes_product_parameter_when_provided() async throws {
         // Given
         let remote = OrdersRemote(network: network)
         network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
         let expectedProductID: Int64 = 13
 
         // When
-        _ = waitFor { promise in
-            remote.loadAllOrders(for: self.sampleSiteID, productID: expectedProductID) { result in
-                promise(result)
-            }
-        }
+        _ = try await remote.loadAllOrders(for: sampleSiteID, productID: expectedProductID)
 
         // Then
         guard let queryParameters = network.queryParameters else {
@@ -243,35 +220,33 @@ final class OrdersRemoteTests: XCTestCase {
 
     /// Verifies that searchOrders properly parses the `orders-load-all` sample response.
     ///
-    func testSearchOrdersProperlyReturnsParsedOrders() {
+    func test_searchOrders_properly_returns_parsed_orders() async throws {
+        // Given
         let remote = OrdersRemote(network: network)
-        let expectation = self.expectation(description: "Load All Orders")
 
         network.simulateResponse(requestUrlSuffix: "orders", filename: "orders-load-all")
 
-        remote.searchOrders(for: sampleSiteID, keyword: String()) { (orders, error) in
-            XCTAssertNil(error)
-            XCTAssertNotNil(orders)
-            XCTAssert(orders!.count == 4)
-            expectation.fulfill()
-        }
+        // When
+        let orders = try await remote.searchOrders(for: sampleSiteID, keyword: String())
 
-        wait(for: [expectation], timeout: Constants.expectationTimeout)
+        // Then
+        XCTAssert(orders.count == 4)
     }
 
     /// Verifies that searchOrders properly relays Networking Layer errors.
     ///
-    func testSearchOrdersProperlyRelaysNetworkingErrors() {
+    func test_searchOrders_properly_relays_networking_error() async throws {
+        // Given
         let remote = OrdersRemote(network: network)
-        let expectation = self.expectation(description: "Load All Orders")
 
-        remote.searchOrders(for: sampleSiteID, keyword: String()) { (orders, error) in
-            XCTAssertNil(orders)
-            XCTAssertNotNil(error)
-            expectation.fulfill()
+        do {
+            // When
+            _ = try await remote.searchOrders(for: sampleSiteID, keyword: String())
+            XCTFail("Expected error to be thrown")
+        } catch {
+            // Then
+            XCTAssertEqual(error as? NetworkError, .notFound(response: nil))
         }
-
-        wait(for: [expectation], timeout: Constants.expectationTimeout)
     }
 
 

WooCommerce/Classes/ViewRelated/Connectivity Tool/ConnectivityToolViewModel.swift

@@ -177,20 +177,13 @@ final class ConnectivityToolViewModel {
     /// Test fetching the site orders by actually fetching orders.
     ///
     func testFetchingOrders() async -> ConnectivityToolCard.State {
-        await withCheckedContinuation { continuation in
-            orderRemote?.loadAllOrders(for: siteID) { [weak self] result in
-                guard let self else { return }
-
-                switch result {
-                case .success:
-                    DDLogInfo("Connectivity Tool: ✅ Site Orders")
-                case .failure(let error):
-                    DDLogError("Connectivity Tool: ❌ Site Orders\n\(error)")
-                }
-
-                let state = self.stateForSiteResult(result)
-                continuation.resume(returning: state)
-            }
+        do {
+            _ = try await orderRemote?.loadAllOrders(for: siteID)
+            DDLogInfo("Connectivity Tool: ✅ Site Orders")
+            return stateForSiteResult(Result<[Order], Error>.success([]))
+        } catch {
+            DDLogError("Connectivity Tool: ❌ Site Orders\n\(error)")
+            return stateForSiteResult(Result<[Order], Error>.failure(error))
         }
     }
 

WooCommerce/Woo Watch App/Orders/OrdersDataService.swift

@@ -17,13 +17,9 @@ final class OrdersDataService {
         ordersRemote = OrdersRemote(network: network)
     }
 
-    /// Async wrapper that fetches orders for a store ID.
+    /// Fetches orders for a store ID.
     ///
     func loadAllOrders(for storeID: Int64, pageNumber: Int, pageSize: Int) async throws -> [Order] {
-        try await withCheckedThrowingContinuation { continuation in
-            ordersRemote.loadAllOrders(for: storeID, pageNumber: pageNumber, pageSize: pageSize) { result in
-                continuation.resume(with: result)
-            }
-        }
+        try await ordersRemote.loadAllOrders(for: storeID, pageNumber: pageNumber, pageSize: pageSize)
     }
 }