Skip to content

Commit 8317f7c

Browse files
jaclyncjaclync
authored
Update ProductsRemote.loadAllProducts to call async/await enqueue method for response parsing to be in a background thread (#15743)
2 parents dd582a6 + 40268f3 commit 8317f7c

File tree

6 files changed

+118
-119
lines changed

6 files changed

+118
-119
lines changed

‎Networking/NetworkingTests/Extensions/URLRequestConvertible+PathTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ final class URLRequestConvertible_PathTests: XCTestCase {
1212
// MARK: - `pathForAnalytics`
1313

1414
// Example from `ProductsRemote.loadAllProducts`.
15-
func test_pathForAnalytics_returns_path_of_JetpackRequest() throws {
15+
func test_pathForAnalytics_returns_path_of_JetpackRequest() async throws {
1616
// Given
1717
let productsRemote = ProductsRemote(network: network)
18-
productsRemote.loadAllProducts(for: 134, completion: { _ in })
18+
_ = try? await productsRemote.loadAllProducts(for: 134)
1919

2020
// When
2121
let request = try XCTUnwrap(network.requestsForResponseData.first)

‎Networking/NetworkingTests/Remote/ProductsRemoteTests.swift‎

Lines changed: 20 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -256,39 +256,28 @@ final class ProductsRemoteTests: XCTestCase {
256256

257257
/// Verifies that loadAllProducts properly parses the `products-load-all` sample response.
258258
///
259-
func testLoadAllProductsProperlyReturnsParsedProducts() {
259+
func test_loadAllProducts_properly_returns_parsed_products() async throws {
260+
// Given
260261
let remote = ProductsRemote(network: network)
261-
let expectation = self.expectation(description: "Load All Products")
262-
263262
network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all")
264263

265-
remote.loadAllProducts(for: sampleSiteID) { result in
266-
switch result {
267-
case .success(let products):
268-
XCTAssertEqual(products.count, 10)
269-
default:
270-
XCTFail("Unexpected result: \(result)")
271-
}
272-
expectation.fulfill()
273-
}
264+
// When
265+
let products = try await remote.loadAllProducts(for: sampleSiteID)
274266

275-
wait(for: [expectation], timeout: Constants.expectationTimeout)
267+
// Then
268+
XCTAssertEqual(products.count, 10)
276269
}
277270

278271
/// Verifies that loadAllProducts with `excludedProductIDs` makes a network request with the corresponding parameter.
279272
///
280-
func testLoadAllProductsWithExcludedIDsIncludesAnExcludeParamInNetworkRequest() throws {
273+
func test_loadAllProducts_with_excluded_ids_includes_an_exclude_param_in_network_request() async throws {
281274
// Arrange
282275
let remote = ProductsRemote(network: network)
283276
network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all")
284277
let excludedProductIDs: [Int64] = [17, 671]
285278

286279
// Action
287-
waitForExpectation { expectation in
288-
remote.loadAllProducts(for: sampleSiteID, excludedProductIDs: excludedProductIDs) { result in
289-
expectation.fulfill()
290-
}
291-
}
280+
_ = try await remote.loadAllProducts(for: sampleSiteID, excludedProductIDs: excludedProductIDs)
292281

293282
// Assert
294283
let queryParameters = try XCTUnwrap(network.queryParameters)
@@ -298,18 +287,14 @@ final class ProductsRemoteTests: XCTestCase {
298287

299288
/// Verifies that loadAllProducts with `productIDs` makes a network request with the `include` parameter.
300289
///
301-
func test_loadAllProducts_with_productIDs_adds_a_include_param_in_network_request() throws {
290+
func test_loadAllProducts_with_productIDs_adds_a_include_param_in_network_request() async throws {
302291
// Given
303292
let remote = ProductsRemote(network: network)
304293
network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all")
305294
let productIDs: [Int64] = [13, 61]
306295

307296
// When
308-
waitForExpectation { expectation in
309-
remote.loadAllProducts(for: sampleSiteID, productIDs: productIDs) { result in
310-
expectation.fulfill()
311-
}
312-
}
297+
_ = try await remote.loadAllProducts(for: sampleSiteID, productIDs: productIDs)
313298

314299
// Then
315300
let queryParameters = try XCTUnwrap(network.queryParameters)
@@ -319,17 +304,13 @@ final class ProductsRemoteTests: XCTestCase {
319304

320305
/// Verifies that loadAllProducts with empty `productIDs` makes a network request without the `include` parameter.
321306
///
322-
func test_loadAllProducts_with_empty_productIDs_does_not_add_include_param_in_network_request() throws {
307+
func test_loadAllProducts_with_empty_productIDs_does_not_add_include_param_in_network_request() async throws {
323308
// Given
324309
let remote = ProductsRemote(network: network)
325310
network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all")
326311

327312
// When
328-
waitForExpectation { expectation in
329-
remote.loadAllProducts(for: sampleSiteID, productIDs: []) { result in
330-
expectation.fulfill()
331-
}
332-
}
313+
_ = try await remote.loadAllProducts(for: sampleSiteID, productIDs: [])
333314

334315
// Then
335316
let queryParameters = try XCTUnwrap(network.queryParameters)
@@ -339,21 +320,16 @@ final class ProductsRemoteTests: XCTestCase {
339320

340321
/// Verifies that loadAllProducts properly relays Networking Layer errors.
341322
///
342-
func test_loadAllProducts_properly_relays_netwoking_errors() {
323+
func test_loadAllProducts_properly_relays_netwoking_errors() async {
324+
// Given
343325
let remote = ProductsRemote(network: network)
344-
let expectation = self.expectation(description: "Load all products returns error")
345-
346-
remote.loadAllProducts(for: sampleSiteID) { result in
347-
switch result {
348-
case .failure:
349-
break
350-
default:
351-
XCTFail("Unexpected result: \(result)")
352-
}
353-
expectation.fulfill()
354-
}
355326

356-
wait(for: [expectation], timeout: Constants.expectationTimeout)
327+
do {
328+
_ = try await remote.loadAllProducts(for: sampleSiteID)
329+
XCTFail("Expected error to be thrown")
330+
} catch {
331+
XCTAssertEqual(error as? NetworkError, .notFound())
332+
}
357333
}
358334

359335

‎Networking/Sources/Extended/Remote/ProductsRemote.swift‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ public protocol ProductsRemoteProtocol {
2020
orderBy: ProductsRemote.OrderKey,
2121
order: ProductsRemote.Order,
2222
productIDs: [Int64],
23-
excludedProductIDs: [Int64],
24-
completion: @escaping (Result<[Product], Error>) -> Void)
23+
excludedProductIDs: [Int64]) async throws -> [Product]
2524
func searchProducts(for siteID: Int64,
2625
keyword: String,
2726
pageNumber: Int,
@@ -179,8 +178,7 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
179178
orderBy: OrderKey = .name,
180179
order: Order = .ascending,
181180
productIDs: [Int64] = [],
182-
excludedProductIDs: [Int64] = [],
183-
completion: @escaping (Result<[Product], Error>) -> Void) {
181+
excludedProductIDs: [Int64] = []) async throws -> [Product] {
184182
let stringOfExcludedProductIDs = excludedProductIDs.map { String($0) }
185183
.joined(separator: ",")
186184
let stringOfProductIDs: String = {
@@ -211,9 +209,9 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
211209

212210
let path = Path.products
213211
let request = JetpackRequest(wooApiVersion: .mark3, method: .get, siteID: siteID, path: path, parameters: parameters, availableAsRESTRequest: true)
214-
let mapper = ProductListMapper(siteID: siteID)
212+
let mapper = ListMapper<Product>(siteID: siteID)
215213

216-
enqueue(request, mapper: mapper, completion: completion)
214+
return try await enqueue(request, mapper: mapper)
217215
}
218216

219217
/// Retrieves products for the Point of Sale. Simple and variable products are loaded for WC version 9.6+, otherwise only simple products are loaded.

‎Yosemite/Yosemite/Stores/ProductStore.swift‎

Lines changed: 67 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -90,18 +90,24 @@ public class ProductStore: Store {
9090
let excludedProductIDs,
9191
let shouldDeleteStoredProductsOnFirstPage,
9292
let onCompletion):
93-
synchronizeProducts(siteID: siteID,
94-
pageNumber: pageNumber,
95-
pageSize: pageSize,
96-
stockStatus: stockStatus,
97-
productStatus: productStatus,
98-
productType: productType,
99-
productCategory: productCategory,
100-
sortOrder: sortOrder,
101-
productIDs: productIDs,
102-
excludedProductIDs: excludedProductIDs,
103-
shouldDeleteStoredProductsOnFirstPage: shouldDeleteStoredProductsOnFirstPage,
104-
onCompletion: onCompletion)
93+
Task { @MainActor in
94+
do {
95+
let hasNextPage = try await synchronizeProducts(siteID: siteID,
96+
pageNumber: pageNumber,
97+
pageSize: pageSize,
98+
stockStatus: stockStatus,
99+
productStatus: productStatus,
100+
productType: productType,
101+
productCategory: productCategory,
102+
sortOrder: sortOrder,
103+
productIDs: productIDs,
104+
excludedProductIDs: excludedProductIDs,
105+
shouldDeleteStoredProductsOnFirstPage: shouldDeleteStoredProductsOnFirstPage)
106+
onCompletion(.success(hasNextPage))
107+
} catch {
108+
onCompletion(.failure(error))
109+
}
110+
}
105111
case .requestMissingProducts(let order, let onCompletion):
106112
requestMissingProducts(for: order, onCompletion: onCompletion)
107113
case .updateProduct(let product, let onCompletion):
@@ -287,43 +293,35 @@ private extension ProductStore {
287293
sortOrder: ProductsSortOrder,
288294
productIDs: [Int64],
289295
excludedProductIDs: [Int64],
290-
shouldDeleteStoredProductsOnFirstPage: Bool,
291-
onCompletion: @escaping (Result<Bool, Error>) -> Void) {
292-
remote.loadAllProducts(for: siteID,
293-
context: nil,
294-
pageNumber: pageNumber,
295-
pageSize: pageSize,
296-
stockStatus: stockStatus,
297-
productStatus: productStatus,
298-
productType: productType,
299-
productCategory: productCategory,
300-
orderBy: sortOrder.remoteOrderKey,
301-
order: sortOrder.remoteOrder,
302-
productIDs: productIDs,
303-
excludedProductIDs: excludedProductIDs) { [weak self] result in
304-
switch result {
305-
case .failure(let error):
306-
if let productType,
307-
let error = error as? DotcomError,
308-
case let .unknown(code, message) = error,
309-
code == "rest_invalid_param",
310-
message == "Invalid parameter(s): type",
311-
ProductType.coreTypes.contains(productType) == false {
312-
return onCompletion(.success(false))
313-
}
314-
onCompletion(.failure(error))
315-
case .success(let products):
316-
guard let self = self else {
317-
return
318-
}
319-
let shouldDeleteExistingProducts = pageNumber == Default.firstPageNumber && shouldDeleteStoredProductsOnFirstPage
320-
self.upsertStoredProductsInBackground(readOnlyProducts: products,
321-
siteID: siteID,
322-
shouldDeleteExistingProducts: shouldDeleteExistingProducts) {
323-
let hasNextPage = products.count == pageSize
324-
onCompletion(.success(hasNextPage))
325-
}
326-
}
296+
shouldDeleteStoredProductsOnFirstPage: Bool) async throws -> Bool {
297+
do {
298+
let products = try await remote.loadAllProducts(for: siteID,
299+
context: nil,
300+
pageNumber: pageNumber,
301+
pageSize: pageSize,
302+
stockStatus: stockStatus,
303+
productStatus: productStatus,
304+
productType: productType,
305+
productCategory: productCategory,
306+
orderBy: sortOrder.remoteOrderKey,
307+
order: sortOrder.remoteOrder,
308+
productIDs: productIDs,
309+
excludedProductIDs: excludedProductIDs)
310+
311+
let shouldDeleteExistingProducts = pageNumber == Default.firstPageNumber && shouldDeleteStoredProductsOnFirstPage
312+
await upsertStoredProductsInBackground(readOnlyProducts: products,
313+
siteID: siteID,
314+
shouldDeleteExistingProducts: shouldDeleteExistingProducts)
315+
let hasNextPage = products.count == pageSize
316+
return hasNextPage
317+
} catch let error as DotcomError where error == .unknown(code: "rest_invalid_param", message: "Invalid parameter(s): type") {
318+
if let productType,
319+
ProductType.coreTypes.contains(productType) == false {
320+
return false
321+
}
322+
throw error
323+
} catch {
324+
throw error
327325
}
328326
}
329327

@@ -852,6 +850,23 @@ extension ProductStore {
852850
}, completion: onCompletion, on: .main)
853851
}
854852

853+
/// Updates (OR Inserts) the specified ReadOnly Product Entities *in a background thread* async.
854+
/// Also deletes existing products if requested.
855+
func upsertStoredProductsInBackground(readOnlyProducts: [Networking.Product],
856+
siteID: Int64,
857+
shouldDeleteExistingProducts: Bool = false) async {
858+
await withCheckedContinuation { [weak self] continuation in
859+
guard let self else {
860+
return continuation.resume()
861+
}
862+
upsertStoredProductsInBackground(readOnlyProducts: readOnlyProducts,
863+
siteID: siteID,
864+
shouldDeleteExistingProducts: shouldDeleteExistingProducts) {
865+
continuation.resume()
866+
}
867+
}
868+
}
869+
855870
/// Updates (OR Inserts) the specified ReadOnly Product Entities *in a background thread*.
856871
/// Also deletes existing products if requested.
857872
/// `onCompletion` will be called on the main thread!
@@ -861,7 +876,9 @@ extension ProductStore {
861876
shouldDeleteExistingProducts: Bool = false,
862877
onCompletion: @escaping () -> Void) {
863878
storageManager.performAndSave({ [weak self] storage in
864-
guard let self else { return }
879+
guard let self else {
880+
return onCompletion()
881+
}
865882
if shouldDeleteExistingProducts {
866883
storage.deleteProducts(siteID: siteID)
867884
}

‎Yosemite/YosemiteTests/Mocks/Networking/Remote/MockProductsRemote.swift‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,7 @@ extension MockProductsRemote: ProductsRemoteProtocol {
260260
orderBy: ProductsRemote.OrderKey,
261261
order: ProductsRemote.Order,
262262
productIDs: [Int64],
263-
excludedProductIDs: [Int64],
264-
completion: @escaping (Result<[Product], Error>) -> Void) {
263+
excludedProductIDs: [Int64]) async throws -> [Product] {
265264
synchronizeProductsTriggered = true
266265
synchronizeProductsWithStockStatus = stockStatus
267266
synchronizeProductsWithProductStatus = productStatus
@@ -271,10 +270,16 @@ extension MockProductsRemote: ProductsRemoteProtocol {
271270
synchronizeProductsOrder = order
272271
synchronizeProductsProductIDs = productIDs
273272
synchronizeProductsExcludedProductIDs = excludedProductIDs
274-
if let result = loadAllProductsResultsBySiteID[siteID] {
275-
completion(result)
276-
} else {
273+
guard let result = loadAllProductsResultsBySiteID[siteID] else {
277274
XCTFail("\(String(describing: self)) Could not find Result for \(siteID)")
275+
throw NetworkError.notFound()
276+
}
277+
278+
switch result {
279+
case let .success(products):
280+
return products
281+
case let .failure(error):
282+
throw error
278283
}
279284
}
280285

0 commit comments

Comments
 (0)