Skip to content

Commit a3c6f33

Browse files
shikishiki
authored
Merge pull request #2278 from woocommerce/issue/2254-use-result-type-in-some-remotes
Use Result type for fetching a Note, Product, and ProductReview
2 parents e78136a + 5739ce9 commit a3c6f33

File tree

14 files changed

+176
-81
lines changed

14 files changed

+176
-81
lines changed

‎Networking/Networking/Remote/NotificationsRemote.swift‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import Foundation
2-
import Alamofire
32

43

54
/// Notifications: Remote Endpoints
@@ -13,7 +12,7 @@ public class NotificationsRemote: Remote {
1312
/// - pageSize: Number of hashes to retrieve.
1413
/// - completion: callback to be executed on completion.
1514
///
16-
public func loadNotes(noteIDs: [Int64]? = nil, pageSize: Int? = nil, completion: @escaping ([Note]?, Error?) -> Void) {
15+
public func loadNotes(noteIDs: [Int64]? = nil, pageSize: Int? = nil, completion: @escaping (Result<[Note], Error>) -> Void) {
1716
let request = requestForNotifications(fields: .all, noteIDs: noteIDs, pageSize: pageSize)
1817
let mapper = NoteListMapper()
1918

‎Networking/Networking/Remote/ProductReviewsRemote.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public final class ProductReviewsRemote: Remote {
4343
/// - reviewID: Identifier of the ProductReview.
4444
/// - completion: Closure to be executed upon completion.
4545
///
46-
public func loadProductReview(for siteID: Int64, reviewID: Int64, completion: @escaping (ProductReview?, Error?) -> Void) {
46+
public func loadProductReview(for siteID: Int64, reviewID: Int64, completion: @escaping (Result<ProductReview, Error>) -> Void) {
4747
let path = "\(Path.reviews)/\(reviewID)"
4848
let request = JetpackRequest(wooApiVersion: .mark3, method: .get, siteID: siteID, path: path, parameters: nil)
4949
let mapper = ProductReviewMapper(siteID: siteID)

‎Networking/Networking/Remote/ProductsRemote.swift‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import Foundation
2-
import Alamofire
32

43
/// Product: Remote Endpoints
54
///
@@ -83,7 +82,7 @@ public class ProductsRemote: Remote {
8382
/// - productID: Identifier of the Product.
8483
/// - completion: Closure to be executed upon completion.
8584
///
86-
public func loadProduct(for siteID: Int64, productID: Int64, completion: @escaping (Product?, Error?) -> Void) {
85+
public func loadProduct(for siteID: Int64, productID: Int64, completion: @escaping (Result<Product, Error>) -> Void) {
8786
let path = "\(Path.products)/\(productID)"
8887
let request = JetpackRequest(wooApiVersion: .mark3, method: .get, siteID: siteID, path: path, parameters: nil)
8988
let mapper = ProductMapper(siteID: siteID)

‎Networking/NetworkingTests/Remote/NotificationsRemoteTests.swift‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import XCTest
44

55
/// NotificationsRemote Tests
66
///
7-
class NotificationsRemoteTests: XCTestCase {
7+
final class NotificationsRemoteTests: XCTestCase {
88

99
/// Dummy Network Wrapper
1010
///
@@ -19,21 +19,28 @@ class NotificationsRemoteTests: XCTestCase {
1919

2020
/// Verifies that `loadNotes` properly returns all of the retrieved notifications.
2121
///
22-
func testLoadNotesProperlyParsesRemoteNotifications() {
22+
func testLoadNotesProperlyParsesRemoteNotifications() throws {
23+
// Given
2324
let remote = NotificationsRemote(network: network)
2425
let expectation = self.expectation(description: "Load Notifications")
2526

2627
network.simulateResponse(requestUrlSuffix: "notifications", filename: "notifications-load-all")
2728

28-
remote.loadNotes { (notes, error) in
29-
XCTAssertNotNil(notes)
30-
XCTAssertNil(error)
31-
XCTAssertEqual(notes?.count, 40)
32-
29+
// When
30+
var resultMaybe: Result<[Note], Error>?
31+
remote.loadNotes { aResult in
32+
resultMaybe = aResult
3333
expectation.fulfill()
3434
}
3535

3636
wait(for: [expectation], timeout: Constants.expectationTimeout)
37+
38+
// Then
39+
let result = try XCTUnwrap(resultMaybe)
40+
XCTAssertTrue(result.isSuccess)
41+
42+
let notes = try result.get()
43+
XCTAssertEqual(notes.count, 40)
3744
}
3845

3946
/// Verifies that `loadHashes` properly returns all of the retrieved hashes.

‎Networking/NetworkingTests/Remote/ProductReviewsRemoteTests.swift‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,34 +114,47 @@ final class ProductReviewsRemoteTests: XCTestCase {
114114

115115
/// Verifies that loadProductReview properly parses the `reviews-single` sample response.
116116
///
117-
func testLoadProductReviewProperlyReturnsParsedProductReviews() {
117+
func testLoadProductReviewProperlyReturnsParsedProductReviews() throws {
118+
// Given
118119
let remote = ProductReviewsRemote(network: network)
119120
let expectation = self.expectation(description: "Load Single Product Review")
120121

121122
network.simulateResponse(requestUrlSuffix: "products/reviews/\(sampleReviewID)", filename: "reviews-single")
122123

123-
remote.loadProductReview(for: sampleSiteID, reviewID: sampleReviewID) { review, error in
124-
XCTAssertNil(error)
125-
XCTAssertNotNil(review)
124+
// When
125+
var resultMaybe: Result<ProductReview, Error>?
126+
remote.loadProductReview(for: sampleSiteID, reviewID: sampleReviewID) { aResult in
127+
resultMaybe = aResult
126128
expectation.fulfill()
127129
}
128130

129131
wait(for: [expectation], timeout: Constants.expectationTimeout)
132+
133+
// Then
134+
let result = try XCTUnwrap(resultMaybe)
135+
XCTAssertTrue(result.isSuccess)
136+
XCTAssertNotNil(try result.get())
130137
}
131138

132139
/// Verifies that loadProductReview properly relays Networking Layer errors.
133140
///
134-
func testLoadProductReviewProperlyRelaysNetwokingErrors() {
141+
func testLoadProductReviewProperlyRelaysNetworkingErrors() throws {
142+
// Given
135143
let remote = ProductReviewsRemote(network: network)
136144
let expectation = self.expectation(description: "Load a single Product Review returns error")
137145

138-
remote.loadProductReview(for: sampleSiteID, reviewID: sampleReviewID) { review, error in
139-
XCTAssertNil(review)
140-
XCTAssertNotNil(error)
146+
// When
147+
var resultMaybe: Result<ProductReview, Error>?
148+
remote.loadProductReview(for: sampleSiteID, reviewID: sampleReviewID) { aResult in
149+
resultMaybe = aResult
141150
expectation.fulfill()
142151
}
143152

144153
wait(for: [expectation], timeout: Constants.expectationTimeout)
154+
155+
// Then
156+
let result = try XCTUnwrap(resultMaybe)
157+
XCTAssertTrue(result.isFailure)
145158
}
146159

147160

‎Networking/NetworkingTests/Remote/ProductsRemoteTests.swift‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,35 +65,49 @@ class ProductsRemoteTests: XCTestCase {
6565

6666
/// Verifies that loadProduct properly parses the `product` sample response.
6767
///
68-
func testLoadSingleProductProperlyReturnsParsedProduct() {
68+
func testLoadSingleProductProperlyReturnsParsedProduct() throws {
69+
// Given
6970
let remote = ProductsRemote(network: network)
7071
let expectation = self.expectation(description: "Load single product")
7172

7273
network.simulateResponse(requestUrlSuffix: "products/\(sampleProductID)", filename: "product")
7374

74-
remote.loadProduct(for: sampleSiteID, productID: sampleProductID) { product, error in
75-
XCTAssertNil(error)
76-
XCTAssertNotNil(product)
77-
XCTAssertEqual(product?.productID, self.sampleProductID)
75+
// When
76+
var resultMaybe: Result<Product, Error>?
77+
remote.loadProduct(for: sampleSiteID, productID: sampleProductID) { aResult in
78+
resultMaybe = aResult
7879
expectation.fulfill()
7980
}
8081

8182
wait(for: [expectation], timeout: Constants.expectationTimeout)
83+
84+
// Then
85+
let result = try XCTUnwrap(resultMaybe)
86+
XCTAssertTrue(result.isSuccess)
87+
88+
let product = try result.get()
89+
XCTAssertEqual(product.productID, sampleProductID)
8290
}
8391

8492
/// Verifies that loadProduct properly relays any Networking Layer errors.
8593
///
86-
func testLoadSingleProductProperlyRelaysNetwokingErrors() {
94+
func testLoadSingleProductProperlyRelaysNetwokingErrors() throws {
95+
// Given
8796
let remote = ProductsRemote(network: network)
8897
let expectation = self.expectation(description: "Load single product returns error")
8998

90-
remote.loadProduct(for: sampleSiteID, productID: sampleProductID) { product, error in
91-
XCTAssertNil(product)
92-
XCTAssertNotNil(error)
99+
// When
100+
var resultMaybe: Result<Product, Error>?
101+
remote.loadProduct(for: sampleSiteID, productID: sampleProductID) { aResult in
102+
resultMaybe = aResult
93103
expectation.fulfill()
94104
}
95105

96106
wait(for: [expectation], timeout: Constants.expectationTimeout)
107+
108+
// Then
109+
let result = try XCTUnwrap(resultMaybe)
110+
XCTAssertTrue(result.isFailure)
97111
}
98112

99113
// MARK: - Search Products

‎Networking/NetworkingTests/Testing/XCTestCase+Wait.swift‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,30 +14,11 @@ extension XCTestCase {
1414
/// }
1515
/// ```
1616
///
17-
public func waitForExpectation(description: String? = nil,
18-
timeout: TimeInterval,
19-
_ block: (XCTestExpectation) -> ()) {
17+
func waitForExpectation(description: String? = nil,
18+
timeout: TimeInterval = Constants.expectationTimeout,
19+
_ block: (XCTestExpectation) -> ()) {
2020
let exp = expectation(description: description ?? "")
2121
block(exp)
2222
wait(for: [exp], timeout: timeout)
2323
}
24-
25-
/// Creates an XCTestExpectation and waits for `block` to call `fulfill()`.
26-
///
27-
/// Example usage:
28-
///
29-
/// ```
30-
/// waitForExpectation { expectation in
31-
/// doSomethingInTheBackground {
32-
/// expectation.fulfill()
33-
/// }
34-
/// }
35-
/// ```
36-
///
37-
public func waitForExpectation(description: String? = nil,
38-
_ block: (XCTestExpectation) -> ()) {
39-
let exp = expectation(description: description ?? "")
40-
block(exp)
41-
wait(for: [exp], timeout: Constants.expectationTimeout)
42-
}
4324
}

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
4.3
22
-----
33
- Products: now the Product details can be edited and saved outside Products tab (e.g. from Order details or Top Performers).
4+
- [internal] Refactored some API calls for fetching a Note, Product, and Product Review.
45
- Products: we improved our VoiceOver support in Product Price settings
56
- In Settings > Experimental Features, a Products switch is now available for turning Products M2 features on and off for simple products (default off for beta testing). Products M2 features: update product images, product settings, viewing and sharing a product.
67

‎Yosemite/Yosemite.xcodeproj/project.pbxproj‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@
9090
45E18632237046CB009241F3 /* ShippingLine+ReadOnlyConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45E18631237046CB009241F3 /* ShippingLine+ReadOnlyConvertible.swift */; };
9191
45ED4F16239E939A004F1BE3 /* TaxClassStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45ED4F15239E939A004F1BE3 /* TaxClassStoreTests.swift */; };
9292
573B448B2424082B00E71ADC /* OrderStoreTests+FetchFilteredAndAllOrders.swift in Sources */ = {isa = PBXBuildFile; fileRef = 573B448A2424082B00E71ADC /* OrderStoreTests+FetchFilteredAndAllOrders.swift */; };
93+
57FDD6BB246B583200B8344B /* XCTestCase+Wait.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57FDD6BA246B583200B8344B /* XCTestCase+Wait.swift */; };
9394
741F34802195EA62005F5BD9 /* CommentAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 741F347F2195EA62005F5BD9 /* CommentAction.swift */; };
9495
741F34822195EA71005F5BD9 /* CommentStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 741F34812195EA71005F5BD9 /* CommentStore.swift */; };
9596
741F34842195F752005F5BD9 /* CommentStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 741F34832195F752005F5BD9 /* CommentStoreTests.swift */; };
@@ -311,6 +312,7 @@
311312
45E18631237046CB009241F3 /* ShippingLine+ReadOnlyConvertible.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ShippingLine+ReadOnlyConvertible.swift"; sourceTree = "<group>"; };
312313
45ED4F15239E939A004F1BE3 /* TaxClassStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaxClassStoreTests.swift; sourceTree = "<group>"; };
313314
573B448A2424082B00E71ADC /* OrderStoreTests+FetchFilteredAndAllOrders.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OrderStoreTests+FetchFilteredAndAllOrders.swift"; sourceTree = "<group>"; };
315+
57FDD6BA246B583200B8344B /* XCTestCase+Wait.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "XCTestCase+Wait.swift"; sourceTree = "<group>"; };
314316
585B973F61632665297738A3 /* Pods-Yosemite.release-alpha.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Yosemite.release-alpha.xcconfig"; path = "../Pods/Target Support Files/Pods-Yosemite/Pods-Yosemite.release-alpha.xcconfig"; sourceTree = "<group>"; };
315317
741F347F2195EA62005F5BD9 /* CommentAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommentAction.swift; sourceTree = "<group>"; };
316318
741F34812195EA71005F5BD9 /* CommentStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommentStore.swift; sourceTree = "<group>"; };
@@ -627,6 +629,14 @@
627629
path = Products;
628630
sourceTree = "<group>";
629631
};
632+
57FDD6B9246B580700B8344B /* Testing */ = {
633+
isa = PBXGroup;
634+
children = (
635+
57FDD6BA246B583200B8344B /* XCTestCase+Wait.swift */,
636+
);
637+
path = Testing;
638+
sourceTree = "<group>";
639+
};
630640
B52E002C2119E6C000700FDE /* Internal */ = {
631641
isa = PBXGroup;
632642
children = (
@@ -859,6 +869,7 @@
859869
B5C9DE022087FEC0006B910A /* YosemiteTests */ = {
860870
isa = PBXGroup;
861871
children = (
872+
57FDD6B9246B580700B8344B /* Testing */,
862873
B5BC71DA21139CF2005CF5AA /* Responses */,
863874
B5BC736C20D1AB3500B5B6FA /* Settings */,
864875
B5C9DE1D2087FF20006B910A /* Mockups */,
@@ -1308,6 +1319,7 @@
13081319
02124DAC24318D6B00980D74 /* Media+MediaTypeTests.swift in Sources */,
13091320
025CA2D0238F54E800B05C81 /* ProductShippingClassStoreTests.swift in Sources */,
13101321
74A7688E20D45ED400F9D437 /* OrderStoreTests.swift in Sources */,
1322+
57FDD6BB246B583200B8344B /* XCTestCase+Wait.swift in Sources */,
13111323
02FF056723DEB2180058E6E7 /* MediaStoreTests.swift in Sources */,
13121324
B5C9DE272087FF20006B910A /* MockupAcount.swift in Sources */,
13131325
02DA641B2313D6D200284168 /* AppSettingsStoreTests+StatsVersion.swift in Sources */,

‎Yosemite/Yosemite/Stores/NotificationStore.swift‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,18 @@ private extension NotificationStore {
102102
return
103103
}
104104

105-
remote.loadNotes(noteIDs: outdatedIDs, pageSize: Constants.maximumPageSize) { [weak self] (notes, error) in
106-
107-
guard let notes = notes else {
108-
onCompletion(error)
105+
remote.loadNotes(noteIDs: outdatedIDs, pageSize: Constants.maximumPageSize) { [weak self] result in
106+
guard let self = self else {
109107
return
110108
}
111109

112-
self?.updateLocalNotes(with: notes) {
113-
onCompletion(nil)
110+
switch result {
111+
case .failure(let error):
112+
onCompletion(error)
113+
case .success(let notes):
114+
self.updateLocalNotes(with: notes) {
115+
onCompletion(nil)
116+
}
114117
}
115118
}
116119
}
@@ -128,14 +131,14 @@ private extension NotificationStore {
128131
func synchronizeNotification(with noteID: Int64, onCompletion: @escaping (Note?, Error?) -> Void) {
129132
let remote = NotificationsRemote(network: network)
130133

131-
remote.loadNotes(noteIDs: [noteID]) { notes, error in
132-
guard let notes = notes else {
134+
remote.loadNotes(noteIDs: [noteID]) { result in
135+
switch result {
136+
case .failure(let error):
133137
onCompletion(nil, error)
134-
return
135-
}
136-
137-
self.updateLocalNotes(with: notes) {
138-
onCompletion(notes.first, nil)
138+
case .success(let notes):
139+
self.updateLocalNotes(with: notes) {
140+
onCompletion(notes.first, nil)
141+
}
139142
}
140143
}
141144
}

0 commit comments

Comments
 (0)