Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use URLSession to send WP.com REST API requests #720

Merged
merged 9 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ _None._

### Internal Changes

- When enabled, `WordPressComRestApi` sends HTTP requests using URLSession instead of Alamofire. [#720]
- When enabled, `WrodPressOrgXMLRPCApi` sends HTTP requests using URLSession instead of Alamofire. [#719]

## 13.0.0
Expand Down
2 changes: 2 additions & 0 deletions WordPressKit/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ extension URLSession {
func perform<E: LocalizedError>(
request builder: HTTPRequestBuilder,
acceptableStatusCodes: [ClosedRange<Int>] = [200...299],
taskCreated: ((Int) -> Void)? = nil,
fulfilling parentProgress: Progress? = nil,
errorType: E.Type = E.self
) async -> WordPressAPIResult<HTTPAPIResponse<Data>, E> {
Expand Down Expand Up @@ -75,6 +76,7 @@ extension URLSession {
}

task.resume()
taskCreated?(task.taskIdentifier)

if let parentProgress, parentProgress.totalUnitCount > parentProgress.completedUnitCount {
let pending = parentProgress.totalUnitCount - parentProgress.completedUnitCount
Expand Down
152 changes: 136 additions & 16 deletions WordPressKit/WordPressComRestApi.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public enum ResponseType {

open class WordPressComRestApi: NSObject {

/// Use `URLSession` directly (instead of Alamofire) to send API requests.
public static var useURLSession = false

// MARK: Properties

@objc public static let ErrorKeyErrorCode = "WordPressComRestApiErrorCodeKey"
Expand Down Expand Up @@ -179,14 +182,14 @@ open class WordPressComRestApi: NSObject {
}

deinit {
for session in [urlSession, sessionManager.session, uploadSessionManager.session] {
for session in [urlSession, uploadURLSession, sessionManager.session, uploadSessionManager.session] {
session.finishTasksAndInvalidate()
}
}

/// Cancels all outgoing tasks asynchronously without invalidating the session.
public func cancelTasks() {
for session in [urlSession, sessionManager.session, uploadSessionManager.session] {
for session in [urlSession, uploadURLSession, sessionManager.session, uploadSessionManager.session] {
session.getAllTasks { tasks in
tasks.forEach({ $0.cancel() })
}
Expand All @@ -197,7 +200,7 @@ open class WordPressComRestApi: NSObject {
Cancels all ongoing taks and makes the session invalid so the object will not fullfil any more request
*/
@objc open func invalidateAndCancelTasks() {
for session in [urlSession, sessionManager.session, uploadSessionManager.session] {
for session in [urlSession, uploadURLSession, sessionManager.session, uploadSessionManager.session] {
session.invalidateAndCancel()
}
}
Expand Down Expand Up @@ -285,18 +288,49 @@ open class WordPressComRestApi: NSObject {
success: @escaping SuccessResponseBlock,
failure: @escaping FailureReponseBlock) -> Progress? {

return request(method: .get, urlString: URLString, parameters: parameters, encoding: URLEncoding.default, success: success, failure: failure)
guard WordPressComRestApi.useURLSession else {
return request(method: .get, urlString: URLString, parameters: parameters, encoding: URLEncoding.default, success: success, failure: failure)
}

let progress = Progress.discreteProgress(totalUnitCount: 100)

Task { @MainActor in
let result = await self.perform(.get, URLString: URLString, parameters: parameters, fulfilling: progress)

switch result {
case let .success(response):
success(response.body, response.response)
case let .failure(error):
failure(error.asNSError(), error.response)
}
}

return progress
}

open func GETData(_ URLString: String,
parameters: [String: AnyObject]?,
completion: @escaping (Swift.Result<(Data, HTTPURLResponse?), Error>) -> Void) {
guard WordPressComRestApi.useURLSession else {
dataRequest(method: .get,
urlString: URLString,
parameters: parameters,
encoding: URLEncoding.default,
completion: completion)
return
}

dataRequest(method: .get,
urlString: URLString,
parameters: parameters,
encoding: URLEncoding.default,
completion: completion)
Task { @MainActor in
let result: APIResult<Data> = await perform(.get, URLString: URLString, parameters: parameters)

completion(
result
.map { ($0.body, $0.response) }
// The completion expects a result with any Error.
// We need to "downcast" the WordPressComRestApiEndpointError error in the APIResult.
.mapError { error -> Error in error }
Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't obvious to me why this mapError was necessary.

What to you think of adding a line such as

Suggested change
.mapError { error -> Error in error }
// The completion expects a result with any Error.
// We need to "downcast" the WordPressComRestApiEndpointError error in the APIResult.
.mapError { error -> Error in error }

Also, using $0 would work, too. But, having the explicit function signature with Error might be clearer for the reader in this unusual scenario.

)
}
}

/**
Expand All @@ -315,8 +349,24 @@ open class WordPressComRestApi: NSObject {
parameters: [String: AnyObject]?,
success: @escaping SuccessResponseBlock,
failure: @escaping FailureReponseBlock) -> Progress? {
guard WordPressComRestApi.useURLSession else {
return request(method: .post, urlString: URLString, parameters: parameters, encoding: JSONEncoding.default, success: success, failure: failure)
}

return request(method: .post, urlString: URLString, parameters: parameters, encoding: JSONEncoding.default, success: success, failure: failure)
let progress = Progress.discreteProgress(totalUnitCount: 100)

Task { @MainActor in
let result = await self.perform(.post, URLString: URLString, parameters: parameters, fulfilling: progress)

switch result {
case let .success(response):
success(response.body, response.response)
case let .failure(error):
failure(error.asNSError(), error.response)
}
}

return progress
}

/**
Expand All @@ -340,6 +390,21 @@ open class WordPressComRestApi: NSObject {
requestEnqueued: RequestEnqueuedBlock? = nil,
success: @escaping SuccessResponseBlock,
failure: @escaping FailureReponseBlock) -> Progress? {
guard !WordPressComRestApi.useURLSession else {
let progress = Progress.discreteProgress(totalUnitCount: 100)

Task { @MainActor in
let result = await upload(URLString: URLString, parameters: parameters, fileParts: fileParts, requestEnqueued: requestEnqueued, fulfilling: progress)
switch result {
case let .success(response):
success(response.body, response.response)
case let .failure(error):
failure(error.asNSError(), error.response)
}
}

return progress
}

guard let URLString = buildRequestURLFor(path: URLString, parameters: parameters) else {
failure(Constants.buildRequestError, nil)
Expand Down Expand Up @@ -467,7 +532,17 @@ open class WordPressComRestApi: NSObject {
// MARK: - Async

private lazy var urlSession: URLSession = {
let configuration = URLSessionConfiguration.default
URLSession(configuration: sessionConfiguration(background: false))
}()

private lazy var uploadURLSession: URLSession = {
let configuration = sessionConfiguration(background: backgroundUploads)
configuration.sharedContainerIdentifier = self.sharedContainerIdentifier
return URLSession(configuration: configuration)
}()

private func sessionConfiguration(background: Bool) -> URLSessionConfiguration {
let configuration = background ? URLSessionConfiguration.background(withIdentifier: self.backgroundSessionIdentifier) : URLSessionConfiguration.default

var additionalHeaders: [String: AnyObject] = [:]
if let oAuthToken = self.oAuthToken {
Expand All @@ -479,8 +554,8 @@ open class WordPressComRestApi: NSObject {

configuration.httpAdditionalHeaders = additionalHeaders

return URLSession(configuration: configuration)
}()
return configuration
}

func perform(
_ method: HTTPRequestBuilder.Method,
Expand Down Expand Up @@ -536,10 +611,12 @@ open class WordPressComRestApi: NSObject {
private func perform<T>(
request: HTTPRequestBuilder,
fulfilling progress: Progress?,
decoder: @escaping (Data) throws -> T
decoder: @escaping (Data) throws -> T,
taskCreated: ((Int) -> Void)? = nil,
session: URLSession? = nil
) async -> APIResult<T> {
await self.urlSession
.perform(request: request, fulfilling: progress, errorType: WordPressComRestApiEndpointError.self)
await (session ?? self.urlSession)
.perform(request: request, taskCreated: taskCreated, fulfilling: progress, errorType: WordPressComRestApiEndpointError.self)
.mapSuccess { response -> HTTPAPIResponse<T> in
let object = try decoder(response.body)

Expand All @@ -564,6 +641,38 @@ open class WordPressComRestApi: NSObject {
}
}

public func upload(
URLString: String,
parameters: [String: AnyObject]?,
fileParts: [FilePart],
requestEnqueued: RequestEnqueuedBlock? = nil,
fulfilling progress: Progress? = nil
) async -> APIResult<AnyObject> {
let builder: HTTPRequestBuilder
do {
let form = try fileParts.map {
try MultipartFormField(fileAtPath: $0.url.path, name: $0.parameterName, filename: $0.filename, mimeType: $0.mimeType)
}
builder = try requestBuilder(URLString: URLString)
.method(.post)
.body(form: form)
} catch {
return .failure(.requestEncodingFailure(underlyingError: error))
}

return await perform(
request: builder.query(parameters ?? [:]),
fulfilling: progress,
decoder: { try JSONSerialization.jsonObject(with: $0) as AnyObject },
taskCreated: { taskID in
DispatchQueue.main.async {
requestEnqueued?(NSNumber(value: taskID))
}
},
session: uploadURLSession
)
}

}

// MARK: - FilePart
Expand Down Expand Up @@ -757,3 +866,14 @@ private extension CharacterSet {
return allowed
}()
}

private extension WordPressAPIError<WordPressComRestApiEndpointError> {
func asNSError() -> NSError {
// When encoutering `URLError`, return `URLError` to avoid potentially breaking existing error handling code in the apps.
if case let .connection(urlError) = self {
return urlError as NSError
}

return self as NSError
}
}
11 changes: 8 additions & 3 deletions WordPressKitTests/WordPressComRestApiTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ class WordPressComRestApiTests: XCTestCase {
XCTFail("This call should fail")
}, failure: { (error, _) in
expect.fulfill()
XCTAssert(error.domain == NSURLErrorDomain, "The error domain should be NSURLErrorDomain")
XCTAssert(error.code == NSURLErrorCancelled, "The error code should be NSURLErrorCancelled")
XCTAssertEqual(error.domain, NSURLErrorDomain, "The error domain should be NSURLErrorDomain")
XCTAssertEqual(error.code, NSURLErrorCancelled, "The error code should be NSURLErrorCancelled")
})
self.waitForExpectations(timeout: 2, handler: nil)
}
Expand Down Expand Up @@ -436,7 +436,12 @@ class WordPressComRestApiTests: XCTestCase {
},
failure: { error, _ in
complete.fulfill()
XCTAssertEqual(error.domain, "Alamofire.AFError")

if WordPressComRestApi.useURLSession {
XCTAssertTrue(error is WordPressAPIError<WordPressComRestApiEndpointError>)
} else {
XCTAssertEqual(error.domain, "Alamofire.AFError")
}
}
)

Expand Down