Skip to content

Commit 2049253

Browse files
authored
Drop unused path parameter from URLRequest initializer (#321)
* refactor: drop unused path parameter from URLRequest initializer * refactor: inline make* factories — call RequestContext/ResponseMetadata initializers directly * refactor: RequestContext convenience init owns method/header normalization * fix: don't crash on cache-read race or unparseable split input - objectFromCache: a file vanishing between exists() and read (background sweep race) is now a cache miss, not a fatalError; inline the single-use URL.getData() helper that held the crash. - splitBaseURLAndRelativePath: return optional instead of fatalError on unparseable input (breaking: public API now returns an optional tuple).
1 parent 2306383 commit 2049253

6 files changed

Lines changed: 36 additions & 39 deletions

File tree

Sources/Networking/Helpers.swift

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ extension FileManager {
6161
}
6262

6363
extension URLRequest {
64-
init(url: URL, requestType: Networking.RequestType, path _: String, contentType: String?, responseType: Networking.ResponseType, authorizationHeaderValue: String?, token: String?, authorizationHeaderKey: String, headerFields: [String: String]?) {
64+
init(url: URL, requestType: Networking.RequestType, contentType: String?, responseType: Networking.ResponseType, authorizationHeaderValue: String?, token: String?, authorizationHeaderKey: String, headerFields: [String: String]?) {
6565
self = URLRequest(url: url)
6666
httpMethod = requestType.rawValue
6767

@@ -87,15 +87,6 @@ extension URLRequest {
8787
}
8888
}
8989

90-
extension URL {
91-
func getData() -> Data {
92-
let path = self.path
93-
guard let data = FileManager.default.contents(atPath: path) else { fatalError("Couldn't get image in destination url: \(self)") }
94-
95-
return data
96-
}
97-
}
98-
9990
extension HTTPURLResponse {
10091
convenience init(url: URL, headerFields: [String : String]? = nil, statusCode: Int) {
10192
self.init(url: url, statusCode: statusCode, httpVersion: nil, headerFields: headerFields)!

Sources/Networking/Networking+New.swift

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ extension Networking {
3535

3636
do {
3737
if let fakeRequest = try FakeRequest.find(ofType: requestType, forPath: path, in: fakeRequests) {
38-
let fakeContext = makeContext(id: requestID, method: requestType.rawValue, url: try? composedURL(with: path), headers: headerFields ?? [:])
38+
let fakeContext = RequestContext(id: requestID, requestType: requestType, url: try? composedURL(with: path), headers: headerFields)
3939
context = fakeContext
4040
emit(.started(fakeContext))
4141
let (fakeResult, fakeStatus, fakeBytes): (Result<T, NetworkingError>, Int, Int) = try await handleFakeRequest(fakeRequest, path: path, requestType: requestType)
@@ -44,7 +44,7 @@ extension Networking {
4444
byteCount = fakeBytes
4545
} else {
4646
let request = try createRequest(path: path, requestType: requestType, body: body, query: query)
47-
let requestContext = makeContext(id: requestID, method: requestType.rawValue, url: request.url, headers: request.allHTTPHeaderFields ?? [:])
47+
let requestContext = RequestContext(id: requestID, requestType: requestType, url: request.url, headers: request.allHTTPHeaderFields)
4848
context = requestContext
4949
emit(.started(requestContext))
5050

@@ -61,7 +61,7 @@ extension Networking {
6161
result = handleResponse(responseData: exchange.data, response: exchange.response, path: path)
6262
statusCode = exchange.response.statusCode
6363
byteCount = exchange.data.count
64-
responseMetadata = makeResponseMetadata(exchange.response, body: exchange.data)
64+
responseMetadata = ResponseMetadata(response: exchange.response, body: exchange.data)
6565
} else {
6666
let collector = MetricsCollector()
6767
let exchange = try await perform(request, collector: collector)
@@ -82,15 +82,15 @@ extension Networking {
8282
byteCount = responseData.count
8383
metrics = collector.metrics.flatMap(TransactionMetrics.init)
8484
if let httpResponse = response as? HTTPURLResponse {
85-
responseMetadata = makeResponseMetadata(httpResponse, body: responseData)
85+
responseMetadata = ResponseMetadata(response: httpResponse, body: responseData)
8686
}
8787
}
8888
}
8989
} catch {
9090
// A failure before the request context was built (e.g. URL building) still emits a paired
9191
// .started/.completed so observers see every request.
9292
if context == nil {
93-
let errorContext = makeContext(id: requestID, method: requestType.rawValue, url: nil, headers: headerFields ?? [:])
93+
let errorContext = RequestContext(id: requestID, requestType: requestType, url: nil, headers: headerFields)
9494
context = errorContext
9595
emit(.started(errorContext))
9696
}
@@ -199,15 +199,11 @@ extension Networking {
199199
// body encoding), so observers don't miss it.
200200
func emitPreflightFailure<T: Decodable>(_ requestType: RequestType, path: String, error: NetworkingError) -> Result<T, NetworkingError> {
201201
let requestID = UUID()
202-
let context = makeContext(id: requestID, method: requestType.rawValue, url: try? composedURL(with: path), headers: headerFields ?? [:])
202+
let context = RequestContext(id: requestID, requestType: requestType, url: try? composedURL(with: path), headers: headerFields)
203203
emit(.started(context))
204204
return complete(.failure(error), context: context, statusCode: nil, byteCount: 0, metrics: nil, duration: .zero)
205205
}
206206

207-
func makeContext(id: UUID, method: String, url: URL?, headers: [String: String]) -> RequestContext {
208-
RequestContext(id: id, method: method, url: url, headers: headers)
209-
}
210-
211207
// Persists status code and headers alongside the body so a cache hit reproduces real metadata.
212208
private struct CachedResponse: Codable {
213209
let statusCode: Int
@@ -274,7 +270,6 @@ extension Networking {
274270
var request = URLRequest(
275271
url: url,
276272
requestType: requestType,
277-
path: path,
278273
contentType: body.contentType(boundary: boundary),
279274
responseType: .json,
280275
authorizationHeaderValue: authorizationHeaderValue,
@@ -329,7 +324,7 @@ extension Networking {
329324
case .cancelled:
330325
return .failure(.cancelled)
331326
case .redirection, .clientError, .serverError, .unknown:
332-
let error = HTTPError(statusCode: statusCode, metadata: makeResponseMetadata(httpResponse, body: responseData))
327+
let error = HTTPError(statusCode: statusCode, metadata: ResponseMetadata(response: httpResponse, body: responseData))
333328
return .failure(.http(error))
334329
}
335330
}
@@ -347,7 +342,7 @@ extension Networking {
347342
let networkingJSON = JSONResponse(statusCode: httpResponse.statusCode, headers: headers, body: body)
348343
return .success(networkingJSON as! T)
349344
} catch let error as DecodingError {
350-
return .failure(.decoding(error, makeResponseMetadata(httpResponse, body: responseData)))
345+
return .failure(.decoding(error, ResponseMetadata(response: httpResponse, body: responseData)))
351346
} catch {
352347
return .failure(.invalidResponse) // JSONDecoder only throws DecodingError; unreachable.
353348
}
@@ -357,17 +352,13 @@ extension Networking {
357352
do {
358353
return .success(try decoder.decode(T.self, from: responseData))
359354
} catch let error as DecodingError {
360-
return .failure(.decoding(error, makeResponseMetadata(httpResponse, body: responseData)))
355+
return .failure(.decoding(error, ResponseMetadata(response: httpResponse, body: responseData)))
361356
} catch {
362357
return .failure(.invalidResponse) // JSONDecoder only throws DecodingError; unreachable.
363358
}
364359
}
365360
}
366361

367-
private func makeResponseMetadata(_ httpResponse: HTTPURLResponse, body: Data) -> ResponseMetadata {
368-
ResponseMetadata(response: httpResponse, body: body)
369-
}
370-
371362
private func bodySnippet(from data: Data, limit: Int = 512) -> String? {
372363
ResponseMetadata.bodySnippet(from: data, limit: limit)
373364
}

Sources/Networking/Networking+Private.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ extension Networking {
2121
return nil
2222
}
2323

24-
let returnedObject: Any? = responseType == .image ? Image(data: destinationURL.getData()) : destinationURL.getData()
24+
// The file can vanish between the exists() check above and this read — the background
25+
// sweep runs concurrently and doesn't share a lock with reads — so a read failure is a
26+
// cache miss, not a crash.
27+
guard let data = FileManager.default.contents(atPath: destinationURL.path) else { return nil }
28+
let returnedObject: Any? = responseType == .image ? Image(data: data) : data
2529
if let returnedObject {
2630
cache.setObject(returnedObject as AnyObject, forKey: key as AnyObject)
2731
// Re-warm: bump the file's mtime so an entry in active use never expires. Only happens
@@ -81,7 +85,7 @@ extension Networking {
8185
let requestID = UUID()
8286
let clock = ContinuousClock()
8387
let startInstant = clock.now
84-
let context = makeContext(id: requestID, method: requestType.rawValue, url: try? composedURL(with: path), headers: headerFields ?? [:])
88+
let context = RequestContext(id: requestID, requestType: requestType, url: try? composedURL(with: path), headers: headerFields)
8589
emit(.started(context))
8690

8791
let result: Result<T, NetworkingError>
@@ -127,7 +131,7 @@ extension Networking {
127131
let requestID = UUID()
128132
let clock = ContinuousClock()
129133
let startInstant = clock.now
130-
let context = makeContext(id: requestID, method: requestType.rawValue, url: try? composedURL(with: path), headers: headerFields ?? [:])
134+
let context = RequestContext(id: requestID, requestType: requestType, url: try? composedURL(with: path), headers: headerFields)
131135
emit(.started(context))
132136

133137
let result: Result<T, NetworkingError>
@@ -192,7 +196,7 @@ extension Networking {
192196
}
193197

194198
func requestData(_ requestType: RequestType, path: String, cachingLevel: CachingLevel, responseType: ResponseType) async throws -> (Data, HTTPURLResponse) {
195-
let request = URLRequest(url: try composedURL(with: path), requestType: requestType, path: path, contentType: nil, responseType: responseType, authorizationHeaderValue: authorizationHeaderValue, token: token, authorizationHeaderKey: authorizationHeaderKey, headerFields: headerFields)
199+
let request = URLRequest(url: try composedURL(with: path), requestType: requestType, contentType: nil, responseType: responseType, authorizationHeaderValue: authorizationHeaderValue, token: token, authorizationHeaderKey: authorizationHeaderKey, headerFields: headerFields)
196200

197201
// Route through the interceptor chain so retry/auth-refresh apply to downloads too.
198202
let exchange = try await perform(request)

Sources/Networking/Networking.swift

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,10 +289,12 @@ public actor Networking {
289289
return String(Int(byte) % shardCount, radix: 16)
290290
}
291291

292-
public static func splitBaseURLAndRelativePath(for path: String) -> (baseURL: String, relativePath: String) {
293-
guard let encodedPath = path.encodeUTF8() else { fatalError("Couldn't encode path to UTF8: \(path)") }
294-
guard let url = URL(string: encodedPath) else { fatalError("Path \(encodedPath) can't be converted to url") }
295-
guard let baseURLWithDash = URL(string: "/", relativeTo: url)?.absoluteURL.absoluteString else { fatalError("Can't find absolute url of url: \(url)") }
292+
public static func splitBaseURLAndRelativePath(for path: String) -> (baseURL: String, relativePath: String)? {
293+
guard let encodedPath = path.encodeUTF8(),
294+
let url = URL(string: encodedPath),
295+
let baseURLWithDash = URL(string: "/", relativeTo: url)?.absoluteURL.absoluteString else {
296+
return nil
297+
}
296298
let index = baseURLWithDash.index(before: baseURLWithDash.endIndex)
297299
let baseURL = String(baseURLWithDash[..<index])
298300
let relativePath = path.replacingOccurrences(of: baseURL, with: "")

Sources/Networking/NetworkingEvent.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ public struct RequestContext: Sendable {
2121
public let headers: [String: String]
2222
}
2323

24+
extension RequestContext {
25+
// Owns the two normalizations every call site repeated: the HTTP method string and a nil header
26+
// set. The URL stays a caller concern — it's resolved differently per site (best-effort
27+
// `composedURL` from a path, a built request's URL, or nil on a pre-flight failure).
28+
init(id: UUID, requestType: Networking.RequestType, url: URL?, headers: [String: String]?) {
29+
self.init(id: id, method: requestType.rawValue, url: url, headers: headers ?? [:])
30+
}
31+
}
32+
2433
public enum Outcome: Sendable {
2534
case success(statusCode: Int, byteCount: Int)
2635
case failure(NetworkingError)

Tests/NetworkingTests/NetworkingTests.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,12 @@ class NetworkingTests: XCTestCase {
173173
XCTAssertEqual(600.statusCodeType, Networking.StatusCodeType.unknown)
174174
}
175175

176-
func testSplitBaseURLAndRelativePath() {
177-
let (baseURL1, relativePath1) = Networking.splitBaseURLAndRelativePath(for: "https://rescuejuice.com/wp-content/uploads/2015/11/døgnvillburgere.jpg")
176+
func testSplitBaseURLAndRelativePath() throws {
177+
let (baseURL1, relativePath1) = try XCTUnwrap(Networking.splitBaseURLAndRelativePath(for: "https://rescuejuice.com/wp-content/uploads/2015/11/døgnvillburgere.jpg"))
178178
XCTAssertEqual(baseURL1, "https://rescuejuice.com")
179179
XCTAssertEqual(relativePath1, "/wp-content/uploads/2015/11/døgnvillburgere.jpg")
180180

181-
let (baseURL2, relativePath2) = Networking.splitBaseURLAndRelativePath(for: "http://example.com/basic-auth/user/passwd")
181+
let (baseURL2, relativePath2) = try XCTUnwrap(Networking.splitBaseURLAndRelativePath(for: "http://example.com/basic-auth/user/passwd"))
182182
XCTAssertEqual(baseURL2, "http://example.com")
183183
XCTAssertEqual(relativePath2, "/basic-auth/user/passwd")
184184
}

0 commit comments

Comments
 (0)