Skip to content

Commit 2d4ebb0

Browse files
jaclyncjaclync
authored
Merge pull request #5435 from woocommerce/feat/5364-network-publisher
Update `AccountRemote.loadSites` to use a Combine publisher in the Networking layer
2 parents c709f80 + a4200ec commit 2d4ebb0

File tree

9 files changed

+218
-14
lines changed

9 files changed

+218
-14
lines changed

‎Networking/Networking/Network/AlamofireNetwork.swift‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import Alamofire
34

@@ -68,6 +69,25 @@ public class AlamofireNetwork: Network {
6869
}
6970
}
7071

72+
/// Executes the specified Network Request. Upon completion, the payload or error will be emitted to the publisher.
73+
/// Only one value will be emitted and the request cannot be retried.
74+
///
75+
/// - Important:
76+
/// - Authentication Headers will be injected, based on the Network's Credentials.
77+
///
78+
/// - Parameter request: Request that should be performed.
79+
/// - Returns: A publisher that emits the result of the given request.
80+
public func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {
81+
let authenticated = AuthenticatedRequest(credentials: credentials, request: request)
82+
83+
return Future() { promise in
84+
Alamofire.request(authenticated).responseData { response in
85+
let result = response.result.toSwiftResult()
86+
promise(Swift.Result.success(result))
87+
}
88+
}.eraseToAnyPublisher()
89+
}
90+
7191
public func uploadMultipartFormData(multipartFormData: @escaping (MultipartFormData) -> Void,
7292
to request: URLRequestConvertible,
7393
completion: @escaping (Data?, Error?) -> Void) {

‎Networking/Networking/Network/MockNetwork.swift‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import Alamofire
34

@@ -81,6 +82,20 @@ class MockNetwork: Network {
8182
completion(.success(data))
8283
}
8384

85+
func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {
86+
requestsForResponseData.append(request)
87+
88+
if let error = error(for: request) {
89+
return Just<Swift.Result<Data, Error>>(.failure(error)).eraseToAnyPublisher()
90+
}
91+
92+
guard let name = filename(for: request), let data = Loader.contentsOf(name) else {
93+
return Just<Swift.Result<Data, Error>>(.failure(NetworkError.notFound)).eraseToAnyPublisher()
94+
}
95+
96+
return Just<Swift.Result<Data, Error>>(.success(data)).eraseToAnyPublisher()
97+
}
98+
8499
func uploadMultipartFormData(multipartFormData: @escaping (MultipartFormData) -> Void,
85100
to request: URLRequestConvertible,
86101
completion: @escaping (Data?, Error?) -> Void) {

‎Networking/Networking/Network/Network.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import Alamofire
34

@@ -43,6 +44,14 @@ public protocol Network {
4344
func responseData(for request: URLRequestConvertible,
4445
completion: @escaping (Swift.Result<Data, Error>) -> Void)
4546

47+
/// Executes the specified Network Request. Upon completion, the payload or error will be emitted to the publisher.
48+
///
49+
/// - Parameters:
50+
/// - request: Request that should be performed.
51+
///
52+
/// - Returns: A publisher that emits the result of the given request.
53+
func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never>
54+
4655
/// Executes the specified Network Request for file uploads. Upon completion, the payload will be sent back to the caller as a Data instance.
4756
///
4857
/// - Parameters:

‎Networking/Networking/Network/NullNetwork.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import Alamofire
34

@@ -16,6 +17,10 @@ public final class NullNetwork: Network {
1617

1718
}
1819

20+
public func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {
21+
Empty<Swift.Result<Data, Error>, Never>().eraseToAnyPublisher()
22+
}
23+
1924
public func uploadMultipartFormData(multipartFormData: @escaping (MultipartFormData) -> Void,
2025
to request: URLRequestConvertible,
2126
completion: @escaping (Data?, Error?) -> Void) { }

‎Networking/Networking/Remote/AccountRemote.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23

34
/// Account: Remote Endpoints
@@ -50,7 +51,7 @@ public class AccountRemote: Remote {
5051

5152
/// Loads the Sites collection associated with the WordPress.com User.
5253
///
53-
public func loadSites(completion: @escaping (Result<[Site], Error>) -> Void) {
54+
public func loadSites() -> AnyPublisher<Result<[Site], Error>, Never> {
5455
let path = "me/sites"
5556
let parameters = [
5657
"fields": "ID,name,description,URL,options,jetpack,jetpack_connection",
@@ -60,7 +61,7 @@ public class AccountRemote: Remote {
6061
let request = DotcomRequest(wordpressApiVersion: .mark1_1, method: .get, path: path, parameters: parameters)
6162
let mapper = SiteListMapper()
6263

63-
enqueue(request, mapper: mapper, completion: completion)
64+
return enqueue(request, mapper: mapper)
6465
}
6566

6667
/// Loads the site plan for the default site associated with the WordPress.com user.

‎Networking/Networking/Remote/Remote.swift‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import protocol Alamofire.URLRequestConvertible
34

@@ -129,6 +130,44 @@ public class Remote {
129130
}
130131
}
131132

133+
/// Returns a publisher that enqueues the specified Network Request on subscription and emits the result upon completion.
134+
///
135+
/// - Important:
136+
/// - Parsing will be performed by the Mapper.
137+
///
138+
/// - Parameters:
139+
/// - request: Request that should be performed.
140+
/// - mapper: Mapper entity that will be used to attempt to parse the Backend's Response.
141+
///
142+
/// - Returns: A publisher that emits result upon completion.
143+
func enqueue<M: Mapper>(_ request: URLRequestConvertible, mapper: M) -> AnyPublisher<Result<M.Output, Error>, Never> {
144+
network.responseDataPublisher(for: request)
145+
.map { (result: Result<Data, Error>) -> Result<M.Output, Error> in
146+
switch result {
147+
case .success(let data):
148+
if let dotcomError = DotcomValidator.error(from: data) {
149+
return .failure(dotcomError)
150+
}
151+
152+
do {
153+
let parsed = try mapper.map(response: data)
154+
return .success(parsed)
155+
} catch {
156+
DDLogError("<> Mapping Error: \(error)")
157+
return .failure(error)
158+
}
159+
case .failure(let error):
160+
return .failure(error)
161+
}
162+
}
163+
.handleEvents(receiveOutput: { [weak self] result in
164+
if let dotcomError = result.failure as? DotcomError {
165+
self?.dotcomErrorWasReceived(error: dotcomError, for: request)
166+
}
167+
})
168+
.eraseToAnyPublisher()
169+
}
170+
132171
/// Enqueues the specified Network Request for upload with multipart form data encoding.
133172
///
134173
/// - Important:

‎Networking/NetworkingTests/Remote/AccountRemoteTests.swift‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,24 @@
1+
import Combine
12
import XCTest
23
@testable import Networking
34

45

56
/// AccountRemote Unit Tests
67
///
7-
class AccountRemoteTests: XCTestCase {
8+
final class AccountRemoteTests: XCTestCase {
89

910
/// Dummy Network Wrapper
1011
///
11-
let network = MockNetwork()
12+
private let network = MockNetwork()
13+
14+
private var cancellables = Set<AnyCancellable>()
1215

1316
/// Repeat always!
1417
///
1518
override func setUp() {
19+
super.setUp()
1620
network.removeAllSimulatedResponses()
21+
cancellables = []
1722
}
1823

1924

@@ -52,4 +57,28 @@ class AccountRemoteTests: XCTestCase {
5257
// Then
5358
XCTAssertTrue(result.isSuccess)
5459
}
60+
61+
// MARK: - `loadSites`
62+
63+
func test_loadSites_emits_sites_in_response() throws {
64+
// Given
65+
let remote = AccountRemote(network: network)
66+
network.simulateResponse(requestUrlSuffix: "me/sites", filename: "sites")
67+
68+
// When
69+
let result = waitFor { promise in
70+
remote.loadSites().sink { result in
71+
promise(result)
72+
}.store(in: &self.cancellables)
73+
}
74+
75+
// Then
76+
XCTAssertTrue(result.isSuccess)
77+
let sites = try XCTUnwrap(result.get())
78+
// Sites in `sites.json` include one Jetpack CP site and one site with Jetpack-the-plugin.
79+
let jcpSites = sites.filter { $0.isJetpackCPConnected }
80+
let nonJCPSites = sites.filter { $0.isJetpackCPConnected == false }
81+
XCTAssertEqual(jcpSites.count, 1)
82+
XCTAssertEqual(nonJCPSites.count, 1)
83+
}
5584
}

‎Networking/NetworkingTests/Remote/RemoteTests.swift‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import XCTest
23
import Fakes
34

@@ -6,12 +7,18 @@ import Fakes
67

78
/// Remote UnitTests
89
///
9-
class RemoteTests: XCTestCase {
10+
final class RemoteTests: XCTestCase {
1011

1112
/// Sample Request
1213
///
1314
private let request = JetpackRequest(wooApiVersion: .mark3, method: .post, siteID: 123, path: "something", parameters: [:])
1415

16+
private var cancellables = Set<AnyCancellable>()
17+
18+
override func setUp() {
19+
super.setUp()
20+
cancellables = []
21+
}
1522

1623
/// Verifies that `enqueue:mapper:` properly wraps up the received request within an AuthenticatedRequest, with
1724
/// the remote credentials.
@@ -68,6 +75,36 @@ class RemoteTests: XCTestCase {
6875
XCTAssertEqual(receivedRequest.path, request.path)
6976
}
7077

78+
/// Verifies that `enqueuePublisher:` properly wraps up the received request within an AuthenticatedRequest, with
79+
/// the remote credentials.
80+
///
81+
func test_enqueuePublisher_wraps_up_request_into_authenticated_request_with_credentials() throws {
82+
// Given
83+
let network = MockNetwork()
84+
let mapper = DummyMapper()
85+
let remote = Remote(network: network)
86+
87+
// When
88+
let result = waitFor { promise in
89+
remote.enqueue(self.request, mapper: mapper).sink { result in
90+
promise(result)
91+
}.store(in: &self.cancellables)
92+
}
93+
94+
// Then
95+
let error = try XCTUnwrap(result.failure)
96+
guard case NetworkError.notFound = error,
97+
let receivedRequest = network.requestsForResponseData.first as? JetpackRequest
98+
else {
99+
XCTFail()
100+
return
101+
}
102+
103+
XCTAssertEqual(network.requestsForResponseData.count, 1)
104+
XCTAssertEqual(receivedRequest.method, request.method)
105+
XCTAssertEqual(receivedRequest.path, request.path)
106+
}
107+
71108
/// Verifies that `enqueue:mapper:` relays any received payload over to the Mapper.
72109
///
73110
func testEnqueueWithMapperProperlyRelaysReceivedPayloadToMapper() {
@@ -110,6 +147,28 @@ class RemoteTests: XCTestCase {
110147
XCTAssertNotNil(mapper.input)
111148
}
112149

150+
/// Verifies that `enqueuePublisher` relays any received payload over to the Mapper.
151+
///
152+
func test_enqueuePublisher_relays_received_payload_to_mapper() {
153+
// Given
154+
let network = MockNetwork()
155+
let mapper = DummyMapper()
156+
let remote = Remote(network: network)
157+
158+
network.simulateResponse(requestUrlSuffix: "something", filename: "order")
159+
160+
// When
161+
waitForExpectation { expectation in
162+
remote.enqueue(request, mapper: mapper).sink { _ in
163+
expectation.fulfill()
164+
}.store(in: &cancellables)
165+
}
166+
167+
// Then
168+
XCTAssertEqual(mapper.input, Loader.contentsOf("order"))
169+
XCTAssertNotNil(mapper.input)
170+
}
171+
113172
/// Verifies that `enqueue:` posts a `RemoteDidReceiveJetpackTimeoutError` Notification whenever the backend returns a
114173
/// Request Timeout error.
115174
///
@@ -180,6 +239,30 @@ class RemoteTests: XCTestCase {
180239
XCTAssertTrue(try XCTUnwrap(result).isFailure)
181240
XCTAssertTrue(try XCTUnwrap(result?.failure) is DotcomError)
182241
}
242+
243+
/// Verifies that `enqueuePublisher` posts a `RemoteDidReceiveJetpackTimeoutError` Notification whenever the backend returns a Request Timeout error.
244+
///
245+
func test_enqueuePublisher_posts_Jetpack_timeout_notification_when_the_response_contains_timeout_error() throws {
246+
// Given
247+
let network = MockNetwork()
248+
let mapper = DummyMapper()
249+
let remote = Remote(network: network)
250+
251+
network.simulateResponse(requestUrlSuffix: "something", filename: "timeout_error")
252+
253+
// When
254+
let expectationForNotification = expectation(forNotification: .RemoteDidReceiveJetpackTimeoutError, object: nil, handler: nil)
255+
let result: Result<Any, Error> = waitFor { promise in
256+
remote.enqueue(self.request, mapper: mapper).sink { result in
257+
promise(result)
258+
}.store(in: &self.cancellables)
259+
}
260+
wait(for: [expectationForNotification], timeout: Constants.expectationTimeout)
261+
262+
// Then
263+
XCTAssertTrue(result.isFailure)
264+
XCTAssertEqual(result.failure as? DotcomError, DotcomError.requestFailed)
265+
}
183266
}
184267

185268

‎Yosemite/Yosemite/Stores/AccountStore.swift‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import Combine
12
import Foundation
23
import Networking
34
import Storage
@@ -7,6 +8,7 @@ import Storage
78
//
89
public class AccountStore: Store {
910
private let remote: AccountRemote
11+
private var cancellables = Set<AnyCancellable>()
1012

1113
/// Shared private StorageType for use during synchronizeSites and synchronizeSitePlan processes
1214
///
@@ -107,16 +109,17 @@ private extension AccountStore {
107109
/// Synchronizes the WordPress.com sites associated with the Network's Auth Token.
108110
///
109111
func synchronizeSites(selectedSiteID: Int64?, onCompletion: @escaping (Result<Void, Error>) -> Void) {
110-
remote.loadSites { [weak self] result in
111-
switch result {
112-
case .success(let sites):
113-
self?.upsertStoredSitesInBackground(readOnlySites: sites, selectedSiteID: selectedSiteID) {
114-
onCompletion(.success(()))
112+
remote.loadSites()
113+
.sink { [weak self] result in
114+
switch result {
115+
case .success(let sites):
116+
self?.upsertStoredSitesInBackground(readOnlySites: sites, selectedSiteID: selectedSiteID) {
117+
onCompletion(.success(()))
118+
}
119+
case .failure(let error):
120+
onCompletion(.failure(error))
115121
}
116-
case .failure(let error):
117-
onCompletion(.failure(error))
118-
}
119-
}
122+
}.store(in: &cancellables)
120123
}
121124

122125
/// Loads the site plan for the default site.

0 commit comments

Comments
 (0)