Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions Networking/Networking/Remote/AccountRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public protocol AccountRemoteProtocol {
func checkIfWooCommerceIsActive(for siteID: Int64) -> AnyPublisher<Result<Bool, Error>, Never>
func fetchWordPressSiteSettings(for siteID: Int64) -> AnyPublisher<Result<WordPressSiteSettings, Error>, Never>
func loadSitePlan(for siteID: Int64, completion: @escaping (Result<SitePlan, Error>) -> Void)
func loadUsernameSuggestions(from text: String) async -> Result<[String], Error>
func loadUsernameSuggestions(from text: String) async throws -> [String]

/// Creates a WPCOM account with the given email and password.
/// - Parameters:
Expand Down Expand Up @@ -131,17 +131,15 @@ public class AccountRemote: Remote, AccountRemoteProtocol {
enqueue(request, mapper: mapper, completion: completion)
}

public func loadUsernameSuggestions(from text: String) async -> Result<[String], Error> {
public func loadUsernameSuggestions(from text: String) async throws -> [String] {
let path = Path.usernameSuggestions
let parameters = [ParameterKey.name: text]
let request = DotcomRequest(wordpressApiVersion: .wpcomMark2, method: .get, path: path, parameters: parameters)
do {
let result: [String: [String]] = try await enqueue(request)
let suggestions = result["suggestions"] ?? []
return .success(suggestions)
} catch {
return .failure(error)
}

let result: [String: [String]] = try await enqueue(request)
let suggestions = result["suggestions"] ?? []

return suggestions
}

public func createAccount(email: String,
Expand Down
11 changes: 3 additions & 8 deletions Networking/Networking/Remote/DomainRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,21 @@ public protocol DomainRemoteProtocol {
/// Loads domain suggestions that are free (`*.wordpress.com` only) based on the query.
/// - Parameter query: What the domain suggestions are based on.
/// - Returns: The result of free domain suggestions.
func loadFreeDomainSuggestions(query: String) async -> Result<[FreeDomainSuggestion], Error>
func loadFreeDomainSuggestions(query: String) async throws -> [FreeDomainSuggestion]
}

/// Domain: Remote Endpoints
///
public class DomainRemote: Remote, DomainRemoteProtocol {
public func loadFreeDomainSuggestions(query: String) async -> Result<[FreeDomainSuggestion], Error> {
public func loadFreeDomainSuggestions(query: String) async throws -> [FreeDomainSuggestion] {
let path = Path.domainSuggestions
let parameters: [String: Any] = [
ParameterKey.query: query,
ParameterKey.quantity: Defaults.domainSuggestionsQuantity,
ParameterKey.wordPressDotComSubdomainsOnly: true
]
let request = DotcomRequest(wordpressApiVersion: .mark1_1, method: .get, path: path, parameters: parameters)
do {
let suggestions: [FreeDomainSuggestion] = try await enqueue(request)
return .success(suggestions)
} catch {
return .failure(error)
}
return try await enqueue(request)
}
}

Expand Down
12 changes: 6 additions & 6 deletions Networking/Networking/Remote/JustInTimeMessagesRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ public protocol JustInTimeMessagesRemoteProtocol {
func loadAllJustInTimeMessages(for siteID: Int64,
messagePath: JustInTimeMessagesRemote.MessagePath,
query: [String: String?]?,
locale: String?) async -> Result<[JustInTimeMessage], Error>
locale: String?) async throws -> [JustInTimeMessage]
func dismissJustInTimeMessage(for siteID: Int64,
messageID: String,
featureClass: String) async -> Result<Bool, Error>
featureClass: String) async throws -> Bool
}

/// Just In Time Messages: Remote endpoints
Expand All @@ -28,7 +28,7 @@ public final class JustInTimeMessagesRemote: Remote, JustInTimeMessagesRemotePro
public func loadAllJustInTimeMessages(for siteID: Int64,
messagePath: JustInTimeMessagesRemote.MessagePath,
query: [String: String?]?,
locale: String?) async -> Result<[JustInTimeMessage], Error> {
locale: String?) async throws -> [JustInTimeMessage] {
let request = JetpackRequest(wooApiVersion: .none,
method: .get,
siteID: siteID,
Expand All @@ -39,7 +39,7 @@ public final class JustInTimeMessagesRemote: Remote, JustInTimeMessagesRemotePro

let mapper = JustInTimeMessageListMapper(siteID: siteID)

return await enqueue(request, mapper: mapper)
return try await enqueue(request, mapper: mapper)
}

private func getParameters(messagePath: JustInTimeMessagesRemote.MessagePath,
Expand Down Expand Up @@ -88,7 +88,7 @@ public final class JustInTimeMessagesRemote: Remote, JustInTimeMessagesRemotePro
///
public func dismissJustInTimeMessage(for siteID: Int64,
messageID: String,
featureClass: String) async -> Result<Bool, Error> {
featureClass: String) async throws -> Bool {

let parameters = [ParameterKey.featureClass: featureClass,
ParameterKey.messageID: messageID]
Expand All @@ -99,7 +99,7 @@ public final class JustInTimeMessagesRemote: Remote, JustInTimeMessagesRemotePro
path: Path.jitm,
parameters: parameters)

return await enqueue(request, mapper: DataBoolMapper())
return try await enqueue(request, mapper: DataBoolMapper())
}
}

Expand Down
12 changes: 6 additions & 6 deletions Networking/Networking/Remote/Remote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ public class Remote: NSObject {
///
/// - Parameter request: Request that should be performed.
/// - Returns: The result from the JSON parsed response for the expected type.
func enqueue<M: Mapper>(_ request: Request, mapper: M) async -> Result<M.Output, Error> {
await withCheckedContinuation { continuation in
network.responseData(for: request) { [weak self] result in
func enqueue<M: Mapper>(_ request: Request, mapper: M) async throws -> M.Output {
try await withCheckedThrowingContinuation { continuation in
network.responseData(for: request) { [weak self] (result: Swift.Result<Data, Error>) in
guard let self else { return }

switch result {
Expand All @@ -212,14 +212,14 @@ public class Remote: NSObject {
let validator = request.responseDataValidator()
try validator.validate(data: data)
let parsed = try mapper.map(response: data)
continuation.resume(returning: .success(parsed))
continuation.resume(returning: parsed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest that we could replace this chunk of code using the existing helpers to convert a throwing call to a Result, and a Result to a continuation. After trying it, the code is similarly long and I believe less readable, so the current approach feels better 🤷🏽‍♂️

Leaving it here in case it helps for other future implementations:

continuation.resume(
    with: result.flatMap { data in
        Result {
            let validator = request.responseDataValidator()
            try validator.validate(data: data)
            return try mapper.map(response: data)
        }
    }.mapError({ error in
        DDLogError("<> Mapping Error: \(error)")
        self.handleResponseError(error: error, for: request)
        return error
    })
)

} catch {
DDLogError("<> Mapping Error: \(error)")
self.handleResponseError(error: error, for: request)
continuation.resume(returning: .failure(error))
continuation.resume(throwing: error)
}
case .failure(let error):
continuation.resume(returning: .failure(error))
continuation.resume(throwing: error)
}
}
}
Expand Down
16 changes: 5 additions & 11 deletions Networking/Networking/Remote/SiteRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ public protocol SiteRemoteProtocol {
/// - Parameters:
/// - name: The name of the site.
/// - domain: The domain selected for the site.
/// - Returns: The result of site creation.
func createSite(name: String,
domain: String) async -> Result<SiteCreationResponse, Error>
/// - Returns: The response with the site creation.
func createSite(name: String, domain: String) async throws -> SiteCreationResponse
}

/// Site: Remote Endpoints
Expand All @@ -24,12 +23,12 @@ public class SiteRemote: Remote, SiteRemoteProtocol {
}

public func createSite(name: String,
domain: String) async -> Result<SiteCreationResponse, Error> {
domain: String) async throws -> SiteCreationResponse {
let path = Path.siteCreation

// Domain input should be a `wordpress.com` subdomain.
guard let subdomainName = domain.split(separator: ".").first else {
return .failure(SiteCreationError.invalidDomain)
throw SiteCreationError.invalidDomain
}
let parameters: [String: Any] = [
"blog_name": subdomainName,
Expand All @@ -52,12 +51,7 @@ public class SiteRemote: Remote, SiteRemoteProtocol {
]
let request = DotcomRequest(wordpressApiVersion: .mark1_1, method: .post, path: path, parameters: parameters)

do {
let response: SiteCreationResponse = try await enqueue(request)
return .success(response)
} catch {
return .failure(error)
}
return try await enqueue(request)
}
}

Expand Down
11 changes: 3 additions & 8 deletions Networking/NetworkingTests/Remote/AccountRemoteTests.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Combine
import XCTest
import TestKit
@testable import Networking


Expand Down Expand Up @@ -151,23 +152,17 @@ final class AccountRemoteTests: XCTestCase {
network.simulateResponse(requestUrlSuffix: "username/suggestions", filename: "account-username-suggestions")

// When
let result = await remote.loadUsernameSuggestions(from: "woo")
let suggestions = try await remote.loadUsernameSuggestions(from: "woo")

// Then
let suggestions = try XCTUnwrap(result.get())
XCTAssertEqual(suggestions, ["woowriter", "woowoowoo", "woodaily"])
}

func test_loadUsernameSuggestions_returns_empty_suggestions_on_empty_response() async throws {
// Given
let remote = AccountRemote(network: network)

// When
let result = await remote.loadUsernameSuggestions(from: "woo")

// Then
let error = try XCTUnwrap(result.failure as? NetworkError)
XCTAssertEqual(error, .notFound)
await assertThrowsError({ _ = try await remote.loadUsernameSuggestions(from: "woo")}, errorAssert: { ($0 as? NetworkError) == .notFound })
}

// MARK: - `createAccount`
Expand Down
13 changes: 3 additions & 10 deletions Networking/NetworkingTests/Remote/DomainRemoteTests.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import XCTest
import TestKit
@testable import Networking

final class DomainRemoteTests: XCTestCase {
Expand All @@ -21,11 +22,9 @@ final class DomainRemoteTests: XCTestCase {
network.simulateResponse(requestUrlSuffix: "domains/suggestions", filename: "domain-suggestions")

// When
let result = await remote.loadFreeDomainSuggestions(query: "domain")
let suggestions = try await remote.loadFreeDomainSuggestions(query: "domain")

// Then
XCTAssertTrue(result.isSuccess)
let suggestions = try XCTUnwrap(result.get())
XCTAssertEqual(suggestions, [
.init(name: "domaintestingtips.wordpress.com", isFree: true),
.init(name: "domaintestingtoday.wordpress.com", isFree: true),
Expand All @@ -36,12 +35,6 @@ final class DomainRemoteTests: XCTestCase {
// Given
let remote = DomainRemote(network: network)

// When
let result = await remote.loadFreeDomainSuggestions(query: "domain")

// Then
XCTAssertTrue(result.isFailure)
let error = try XCTUnwrap(result.failure as? NetworkError)
XCTAssertEqual(error, .notFound)
await assertThrowsError({_ = try await remote.loadFreeDomainSuggestions(query: "domain")}, errorAssert: { ($0 as? NetworkError) == .notFound })
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
network.simulateResponse(requestUrlSuffix: "jetpack/v4/jitm", filename: "just-in-time-message-list")

// When
let result = await remote.loadAllJustInTimeMessages(
let justInTimeMessages = try await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -37,8 +37,6 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
locale: "en_US")

// Then
XCTAssert(result.isSuccess)
let justInTimeMessages = try XCTUnwrap(result.get())
assertEqual(1, justInTimeMessages.count)
}

Expand All @@ -49,7 +47,7 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
let remote = JustInTimeMessagesRemote(network: network)

// When
_ = await remote.loadAllJustInTimeMessages(
_ = try? await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -72,7 +70,7 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
network.simulateResponse(requestUrlSuffix: "jetpack/v4/jitm", filename: "just-in-time-message-list")

// When
let result = await remote.loadAllJustInTimeMessages(
let justInTimeMessages = try await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -82,7 +80,6 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
locale: "en_US")

// Then
let justInTimeMessages = try result.get()
assertEqual(sampleSiteID, justInTimeMessages.first?.siteID)
}

Expand All @@ -93,7 +90,7 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
let remote = JustInTimeMessagesRemote(network: network)

// When
_ = await remote.loadAllJustInTimeMessages(
_ = try? await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -116,31 +113,28 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
// Given
let remote = JustInTimeMessagesRemote(network: network)

let error = NetworkError.unacceptableStatusCode(statusCode: 403)
network.simulateError(requestUrlSuffix: "jetpack/v4/jitm", error: error)
let expectedError = NetworkError.unacceptableStatusCode(statusCode: 403)
network.simulateError(requestUrlSuffix: "jetpack/v4/jitm", error: expectedError)

// When
let result = await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
screen: "my_store",
hook: .adminNotices),
query: nil,
locale: "en_US")

// Then
XCTAssertTrue(result.isFailure)
let resultError = try XCTUnwrap(result.failure as? NetworkError)
assertEqual(error, resultError)
await assertThrowsError({
_ = try await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
screen: "my_store",
hook: .adminNotices),
query: nil,
locale: "en_US")
}, errorAssert: { ($0 as? NetworkError) == expectedError })
}

func test_test_loadAllJustInTimeMessages_uses_passed_locale_for_request() async throws {
// Given
let remote = JustInTimeMessagesRemote(network: network)

// When
_ = await remote.loadAllJustInTimeMessages(
_ = try? await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -162,7 +156,7 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
let remote = JustInTimeMessagesRemote(network: network)

// When
_ = await remote.loadAllJustInTimeMessages(
_ = try? await remote.loadAllJustInTimeMessages(
for: self.sampleSiteID,
messagePath: JustInTimeMessagesRemote.MessagePath(
app: .wooMobile,
Expand All @@ -180,7 +174,6 @@ final class JustInTimeMessagesRemoteTests: XCTestCase {
let queryItems = try XCTUnwrap(URLComponents(url: url, resolvingAgainstBaseURL: false)?.queryItems)
let queryJson = try XCTUnwrap(queryItems.first { $0.name == "query" }?.value)
assertThat(queryJson, contains: "\"query\":")
let parameters = request.parameters
let jitmQuery = try XCTUnwrap(request.parameters["query"] as? String)
// Individually check query items because dictionaries aren't ordered
assertThat(jitmQuery, contains: "platform=ios") // platform=ios
Expand Down
Loading