Skip to content

Commit b1b9a85

Browse files
jkmasseljkmassel
committed
Address HTTPClient feedback
1 parent 6aa25bc commit b1b9a85

File tree

5 files changed

+60
-68
lines changed

5 files changed

+60
-68
lines changed

‎ios/Sources/GutenbergKit/Sources/EditorHTTPClient.swift‎

Lines changed: 39 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import OSLog
33

44
/// A protocol for making authenticated HTTP requests to the WordPress REST API.
55
public protocol EditorHTTPClientProtocol: Sendable {
6-
func GET(url: URL) async throws -> (Data, HTTPURLResponse)
7-
func OPTIONS(url: URL) async throws -> (Data, HTTPURLResponse)
8-
func download(url: URL) async throws -> (URL, HTTPURLResponse)
6+
func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse)
7+
func download(_ urlRequest: URLRequest) async throws -> (URL, HTTPURLResponse)
98
}
109

1110
/// A delegate for observing HTTP requests made by the editor.
@@ -40,65 +39,58 @@ public actor EditorHTTPClient: EditorHTTPClientProtocol {
4039
private let urlSession: URLSession
4140
private let authHeader: String
4241
private let delegate: EditorHTTPClientDelegate?
43-
42+
private let requestTimeout: TimeInterval
43+
4444
public init(
4545
urlSession: URLSession,
4646
authHeader: String,
47-
delegate: EditorHTTPClientDelegate? = nil
47+
delegate: EditorHTTPClientDelegate? = nil,
48+
requestTimeout: TimeInterval = 60 // `URLRequest` default
4849
) {
4950
self.urlSession = urlSession
5051
self.authHeader = authHeader
5152
self.delegate = delegate
53+
self.requestTimeout = requestTimeout
5254
}
53-
54-
public func GET(url: URL) async throws -> (Data, HTTPURLResponse) {
55-
var request = URLRequest(url: url)
56-
request.httpMethod = "GET"
57-
return try await self.perform(request: request)
58-
}
59-
60-
public func OPTIONS(url: URL) async throws -> (Data, HTTPURLResponse) {
61-
var request = URLRequest(url: url)
62-
request.httpMethod = "OPTIONS"
63-
return try await self.perform(request: request)
64-
}
65-
66-
public func download(url: URL) async throws -> (URL, HTTPURLResponse) {
67-
var request = URLRequest(url: url)
68-
request.addValue(self.authHeader, forHTTPHeaderField: "Authorization")
69-
70-
let (url, response) = try await self.urlSession.download(for: request)
71-
72-
let httpResponse = response as! HTTPURLResponse
73-
74-
guard 200...299 ~= httpResponse.statusCode else {
75-
throw ClientError.downloadFailed(statusCode: httpResponse.statusCode)
76-
}
77-
78-
return (url, response as! HTTPURLResponse)
79-
}
80-
81-
private func perform(request: URLRequest) async throws -> (Data, HTTPURLResponse) {
82-
var signedRequest = request
83-
signedRequest.setValue(self.authHeader, forHTTPHeaderField: "Authorization")
84-
signedRequest.timeoutInterval = 60
85-
86-
let (data, response) = try await self.urlSession.data(for: signedRequest)
87-
self.delegate?.didPerformRequest(signedRequest, response: response, data: data)
88-
55+
56+
public func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) {
57+
var mutableRequest = urlRequest
58+
mutableRequest.setValue(self.authHeader, forHTTPHeaderField: "Authorization")
59+
mutableRequest.timeoutInterval = self.requestTimeout
60+
61+
let (data, response) = try await self.urlSession.data(for: mutableRequest)
62+
self.delegate?.didPerformRequest(mutableRequest, response: response, data: data)
63+
8964
let httpResponse = response as! HTTPURLResponse
90-
65+
9166
guard 200...299 ~= httpResponse.statusCode else {
92-
Logger.http.error("📡 HTTP error fetching \(request.url!.absoluteString): \(httpResponse.statusCode)")
93-
67+
Logger.http.error("📡 HTTP error fetching \(mutableRequest.url!.absoluteString): \(httpResponse.statusCode)")
68+
9469
if let wpError = try? JSONDecoder().decode(WPError.self, from: data) {
9570
throw ClientError.wpError(wpError)
9671
}
97-
72+
9873
throw ClientError.unknown(response: data, statusCode: httpResponse.statusCode)
9974
}
100-
75+
10176
return (data, httpResponse)
10277
}
103-
78+
79+
public func download(_ urlRequest: URLRequest) async throws -> (URL, HTTPURLResponse) {
80+
var mutableRequest = urlRequest
81+
mutableRequest.addValue(self.authHeader, forHTTPHeaderField: "Authorization")
82+
mutableRequest.timeoutInterval = self.requestTimeout
83+
84+
let (url, response) = try await self.urlSession.download(for: mutableRequest)
85+
86+
let httpResponse = response as! HTTPURLResponse
87+
88+
guard 200...299 ~= httpResponse.statusCode else {
89+
Logger.http.error("📡 HTTP error fetching \(mutableRequest.url!.absoluteString): \(httpResponse.statusCode)")
90+
91+
throw ClientError.downloadFailed(statusCode: httpResponse.statusCode)
92+
}
93+
94+
return (url, response as! HTTPURLResponse)
95+
}
10496
}

‎ios/Sources/GutenbergKit/Sources/Extensions/Foundation.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ extension URLRequest {
7878
/// - Parameters:
7979
/// - url: The URL for the request.
8080
/// - method: The HTTP method to use.
81-
init(url: URL, method: EditorHttpMethod) {
81+
init(method: EditorHttpMethod, url: URL) {
8282
var request = URLRequest(url: url)
8383
request.httpMethod = method.rawValue
8484
self = request

‎ios/Sources/GutenbergKit/Sources/RESTAPIRepository.swift‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public struct RESTAPIRepository: Sendable {
4444
// MARK: Post
4545
@discardableResult
4646
public func fetchPost(id: Int) async throws -> EditorURLResponse {
47-
let response = try await self.httpClient.GET(url: buildPostUrl(id: id))
47+
let request = URLRequest(method: .GET, url: self.buildPostUrl(id: id))
48+
let response = try await self.httpClient.perform(request)
4849
return EditorURLResponse(response)
4950
}
5051

@@ -68,7 +69,9 @@ public struct RESTAPIRepository: Sendable {
6869
return .undefined
6970
}
7071

71-
let response = try await self.httpClient.GET(url: editorSettingsUrl)
72+
let request = URLRequest(method: .GET, url: editorSettingsUrl)
73+
let response = try await self.httpClient.perform(request)
74+
7275
let editorSettings = EditorSettings(data: response.0)
7376

7477
let urlResponse = EditorURLResponse((try JSONEncoder().encode(editorSettings), response.1))
@@ -87,7 +90,7 @@ public struct RESTAPIRepository: Sendable {
8790
// MARK: GET Post Type
8891
@discardableResult
8992
package func fetchPostType(for type: String) async throws -> EditorURLResponse {
90-
try await GET(url: buildPostTypeUrl(type: type))
93+
try await self.perform(method: .GET, url: self.buildPostTypeUrl(type: type))
9194
}
9295

9396
package func readPostType(for type: String) throws -> EditorURLResponse? {
@@ -105,7 +108,7 @@ public struct RESTAPIRepository: Sendable {
105108
// MARK: GET Active Theme
106109
@discardableResult
107110
package func fetchActiveTheme() async throws -> EditorURLResponse {
108-
try await GET(url: self.activeThemeUrl)
111+
try await self.perform(method: .GET, url: self.activeThemeUrl)
109112
}
110113

111114
package func readActiveTheme() throws -> EditorURLResponse? {
@@ -115,7 +118,7 @@ public struct RESTAPIRepository: Sendable {
115118
// MARK: OPTIONS Settings
116119
@discardableResult
117120
package func fetchSettingsOptions() async throws -> EditorURLResponse {
118-
try await OPTIONS(url: self.siteSettingsUrl)
121+
try await self.perform(method: .OPTIONS, url: self.siteSettingsUrl)
119122
}
120123

121124
package func readSettingsOptions() throws -> EditorURLResponse? {
@@ -125,24 +128,19 @@ public struct RESTAPIRepository: Sendable {
125128
// MARK: Post Types
126129
@discardableResult
127130
package func fetchPostTypes() async throws -> EditorURLResponse {
128-
try await self.GET(url: self.postTypesUrl)
131+
try await self.perform(method: .GET, url: self.postTypesUrl)
129132
}
130133

131134
package func readPostTypes() throws -> EditorURLResponse? {
132135
try self.cache.response(for: self.postTypesUrl, httpMethod: .GET)
133136
}
134137

135-
private func GET(url: URL) async throws -> EditorURLResponse {
136-
let response = try await self.httpClient.GET(url: url)
138+
private func perform(method: EditorHttpMethod, url: URL) async throws -> EditorURLResponse {
139+
let request = URLRequest(method: method, url: url)
140+
let response = try await self.httpClient.perform(request)
137141
let urlResponse = EditorURLResponse(response)
138-
try self.cache.store(urlResponse, for: url, httpMethod: .GET)
142+
try self.cache.store(urlResponse, for: url, httpMethod: method)
139143
return urlResponse
140144
}
141145

142-
private func OPTIONS(url: URL) async throws -> EditorURLResponse {
143-
let response = try await self.httpClient.OPTIONS(url: url)
144-
let urlResponse = EditorURLResponse(response)
145-
try self.cache.store(urlResponse, for: url, httpMethod: .OPTIONS)
146-
return urlResponse
147-
}
148146
}

‎ios/Sources/GutenbergKit/Sources/Stores/EditorAssetLibrary.swift‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ public actor EditorAssetLibrary {
3838
package func fetchManifest(cachePolicy: CachePolicy = .useExisting) async throws
3939
-> LocalEditorAssetManifest {
4040
guard configuration.shouldUsePlugins else { return .empty }
41-
let data = try await httpClient.GET(url: self.editorAssetsUrl(for: self.configuration)).0
41+
let data = try await httpClient.perform(
42+
URLRequest(method: .GET, url: self.editorAssetsUrl(for: self.configuration))
43+
).0
4244
let remoteManifest = try RemoteEditorAssetManifest(data: data)
4345

4446
if case .useExisting = cachePolicy,
@@ -146,7 +148,7 @@ public actor EditorAssetLibrary {
146148
///
147149
private func fetchAsset(url: URL, into bundle: EditorAssetBundle) async throws -> URL {
148150
let tempUrl = try await logExecutionTime("Downloading \(url.lastPathComponent)") {
149-
try await httpClient.download(url: url).0
151+
try await httpClient.download(URLRequest(method: .GET, url: url)).0
150152
}
151153

152154
let destinationPath = bundle.bundleRoot.appending(path: url.path(percentEncoded: false))

‎ios/Sources/GutenbergKit/Sources/Stores/EditorURLCache.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public struct EditorURLCache: Sendable {
5555
storagePolicy: .allowed
5656
)
5757

58-
self.cache.storeCachedResponse(response, for: URLRequest(url: url, method: httpMethod))
58+
self.cache.storeCachedResponse(response, for: URLRequest(method: httpMethod, url: url))
5959
Thread.sleep(forTimeInterval: 0.05) // Hack to make `URLCache` work
6060
}
6161

@@ -100,7 +100,7 @@ public struct EditorURLCache: Sendable {
100100
storagePolicy: .allowed
101101
)
102102

103-
self.cache.storeCachedResponse(response, for: URLRequest(url: url, method: httpMethod))
103+
self.cache.storeCachedResponse(response, for: URLRequest(method: httpMethod, url: url))
104104
Thread.sleep(forTimeInterval: 0.05) // Hack to make `URLCache` work
105105
}
106106

@@ -137,7 +137,7 @@ public struct EditorURLCache: Sendable {
137137
) throws -> EditorURLResponse? {
138138
performanceMonitor.measure { () -> EditorURLResponse? in
139139
guard
140-
let response = self.cache.cachedResponse(for: URLRequest(url: url, method: httpMethod)),
140+
let response = self.cache.cachedResponse(for: URLRequest(method: httpMethod, url: url)),
141141
let storageDate = response.userInfo?["storageDate"] as? Date,
142142
self.cachePolicy.allowsResponseWith(date: storageDate, currentDate: currentDate)
143143
else {

0 commit comments

Comments
 (0)