Skip to content

Commit a583751

Browse files
committed
Code review updates
1 parent 254359d commit a583751

19 files changed

Lines changed: 196 additions & 137 deletions

.swiftlint.yml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ opt_in_rules:
1717
- contains_over_first_not_nil
1818
- contains_over_range_nil_comparison
1919
- convenience_type
20-
- discouraged_object_literal
2120
- discouraged_optional_boolean
2221
- empty_collection_literal
2322
- empty_count
@@ -45,22 +44,17 @@ opt_in_rules:
4544
- multiline_literal_brackets
4645
- multiline_parameters
4746
- multiline_parameters_brackets
48-
- nimble_operator
4947
- number_separator
50-
- object_literal
5148
- operator_usage_whitespace
5249
- optional_enum_case_matching
5350
- overridden_super_call
5451
- override_in_extension
5552
- pattern_matching_keywords
5653
- prefer_self_type_over_type_of_self
57-
- private_action
58-
- private_outlet
5954
- prohibited_super_call
6055
- reduce_into
6156
- redundant_nil_coalescing
6257
- required_enum_case
63-
- single_test_class
6458
- sorted_first_last
6559
- sorted_imports
6660
- static_operator
@@ -73,10 +67,6 @@ opt_in_rules:
7367
- vertical_parameter_alignment_on_call
7468
- vertical_whitespace_closing_braces
7569
- yoda_condition
76-
analyzer_rules:
77-
- unused_declaration
78-
- unused_import
79-
8070
# Rule configurations
8171
identifier_name:
8272
excluded:

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ The framework is built around two core protocols:
8888
**Source Structure** (`Sources/FTAPIKit/`):
8989
- Core protocols: `URLServer.swift`, `Endpoint.swift`
9090
- Request building: `URLRequestBuilder.swift`, `RequestConfiguring.swift`
91-
- Async execution: `URLServer+Async.swift`, `URLServer+Download.swift`
91+
- Async execution: `URLServer+Async.swift` (includes download)
9292
- Observers: `NetworkObserver.swift`
9393
- Utilities: `Coding.swift`, `URLQuery.swift`, `MultipartFormData.swift`, etc.
9494
- Error handling: `APIError.swift`, `APIError+Standard.swift`

Sources/FTAPIKit/MultipartFormData.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ struct MultipartFormData {
1111
self.boundary = boundary
1212
}
1313

14+
/// Returns the size of the temporary file containing the multipart body.
15+
///
16+
/// - Important: Must be called after ``inputStream()`` which writes the file.
17+
/// Returns `nil` if the file does not exist yet.
1418
var contentLength: Int64? {
1519
(try? FileManager.default.attributesOfItem(atPath: temporaryUrl.path)[.size] as? Int64)?.flatMap { $0 }
1620
}

Sources/FTAPIKit/NetworkObserver.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import Foundation
1212
/// 3. `didFail` is called additionally if the request processing fails (network, HTTP status, or decoding error)
1313
///
1414
/// - Note: Observers are strongly retained for the duration of a request to ensure lifecycle callbacks
15-
/// are always delivered.
15+
/// are always delivered. Observers must not hold strong references back to the server to avoid
16+
/// retain cycles.
1617
public protocol NetworkObserver: AnyObject, Sendable {
1718
associatedtype Context: Sendable
1819

Sources/FTAPIKit/RequestConfiguring.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,28 @@ public protocol RequestConfiguring: Sendable {
2323
/// - Throws: Any error that occurs during configuration (e.g., token refresh failure)
2424
func configure(_ request: inout URLRequest) async throws
2525
}
26+
27+
/// Composes multiple ``RequestConfiguring`` instances into a single configuration.
28+
///
29+
/// Configurations are applied in order, so later configurations can override
30+
/// headers set by earlier ones.
31+
///
32+
/// ```swift
33+
/// let config = CompositeRequestConfiguring([authConfig, tracingConfig])
34+
/// let data = try await server.call(data: endpoint, configuring: config)
35+
/// ```
36+
public struct CompositeRequestConfiguring: RequestConfiguring {
37+
private let configurations: [any RequestConfiguring]
38+
39+
/// Creates a composite configuration from multiple configurations.
40+
/// - Parameter configurations: Configurations to apply in order.
41+
public init(_ configurations: [any RequestConfiguring]) {
42+
self.configurations = configurations
43+
}
44+
45+
public func configure(_ request: inout URLRequest) async throws {
46+
for configuration in configurations {
47+
try await configuration.configure(&request)
48+
}
49+
}
50+
}

Sources/FTAPIKit/URLQuery.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ import Foundation
1414
/// "child[]": "Maggie"
1515
/// ]
1616
/// ```
17-
public struct URLQuery: ExpressibleByDictionaryLiteral {
17+
public struct URLQuery: ExpressibleByDictionaryLiteral, Sendable {
1818
/// Array of URL query items.
1919
public let items: [URLQueryItem]
2020

21-
init() {
21+
/// Creates an empty URL query.
22+
public init() {
2223
self.items = []
2324
}
2425

Sources/FTAPIKit/URLRequestBuilder.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ struct URLRequestBuilder<S: URLServer> {
2727
return request
2828
}
2929

30+
/// Populates the request body based on the endpoint's protocol conformance.
31+
///
32+
/// - Important: Case ordering matters. If an endpoint conforms to multiple body-providing
33+
/// protocols (e.g., both `DataEndpoint` and `EncodableEndpoint`), the first match wins.
3034
private func buildBody(to request: inout URLRequest) throws {
3135
switch endpoint {
3236
case let endpoint as DataEndpoint:

Sources/FTAPIKit/URLServer+Async.swift

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ public extension URLServer {
66
/// - Parameters:
77
/// - endpoint: The endpoint
88
/// - configuring: Optional request configuration to apply before sending
9-
/// - Throws: Throws an APIError if the request fails or server returns an error
9+
/// - Throws: Throws an ``APIError`` if the request fails or server returns an error,
10+
/// or an error from ``RequestConfiguring/configure(_:)`` if configuration fails.
1011
func call(endpoint: Endpoint, configuring: RequestConfiguring? = nil) async throws {
1112
_ = try await execute(endpoint: endpoint, configuring: configuring)
1213
}
@@ -15,7 +16,8 @@ public extension URLServer {
1516
/// - Parameters:
1617
/// - endpoint: The endpoint
1718
/// - configuring: Optional request configuration to apply before sending
18-
/// - Throws: Throws an APIError if the request fails or server returns an error
19+
/// - Throws: Throws an ``APIError`` if the request fails or server returns an error,
20+
/// or an error from ``RequestConfiguring/configure(_:)`` if configuration fails.
1921
/// - Returns: Plain data returned with the HTTP Response
2022
func call(data endpoint: Endpoint, configuring: RequestConfiguring? = nil) async throws -> Data {
2123
try await execute(endpoint: endpoint, configuring: configuring).data
@@ -25,7 +27,9 @@ public extension URLServer {
2527
/// - Parameters:
2628
/// - endpoint: The endpoint
2729
/// - configuring: Optional request configuration to apply before sending
28-
/// - Throws: Throws an APIError if the request fails, server returns an error, or decoding fails
30+
/// - Throws: Throws an ``APIError`` if the request fails or server returns an error.
31+
/// Throws a `DecodingError` directly if response decoding fails (decoding errors are not
32+
/// routed through ``URLServer/ErrorType``).
2933
/// - Returns: Instance of the required type
3034
func call<EP: ResponseEndpoint>(response endpoint: EP, configuring: RequestConfiguring? = nil) async throws -> EP.Response {
3135
let result = try await execute(endpoint: endpoint, configuring: configuring)
@@ -36,22 +40,48 @@ public extension URLServer {
3640
throw error
3741
}
3842
}
43+
44+
/// Downloads a file from the specified endpoint to a temporary location.
45+
/// - Parameters:
46+
/// - endpoint: The endpoint
47+
/// - configuring: Optional request configuration to apply before sending
48+
/// - Throws: Throws an ``APIError`` if the request fails or server returns an error,
49+
/// or an error from ``RequestConfiguring/configure(_:)`` if configuration fails.
50+
/// - Returns: The location of a temporary file where the server's response is stored.
51+
/// You must move this file or open it for reading before the async function returns. Otherwise, the file
52+
/// is deleted, and the data is lost.
53+
func download(endpoint: Endpoint, configuring: RequestConfiguring? = nil) async throws -> URL {
54+
let (urlRequest, observers) = try await prepareObservers(endpoint: endpoint, configuring: configuring)
55+
56+
let (localURL, response): (URL, URLResponse)
57+
do {
58+
(localURL, response) = try await urlSession.download(for: urlRequest)
59+
} catch {
60+
observers.forEach { $0.didFail(request: urlRequest, error: error) }
61+
throw error
62+
}
63+
64+
observers.forEach { $0.didReceiveResponse(for: urlRequest, response: response, data: nil) }
65+
try checkForError(data: nil, response: response, request: urlRequest, observers: observers)
66+
67+
return localURL
68+
}
3969
}
4070

4171
// MARK: - Private helpers
4272

4373
private struct ExecuteResult {
4474
let data: Data
4575
let request: URLRequest
46-
let observers: [AnyObserverToken]
76+
let observers: [BoundObserverContext]
4777
}
4878

4979
private extension URLServer {
5080

5181
/// Core execution method that builds the request, notifies observers, performs the network call,
5282
/// and handles errors.
5383
func execute(endpoint: Endpoint, configuring: RequestConfiguring?) async throws -> ExecuteResult {
54-
let (urlRequest, observers) = try await prepareRequest(endpoint: endpoint, configuring: configuring)
84+
let (urlRequest, observers) = try await prepareObservers(endpoint: endpoint, configuring: configuring)
5585

5686
let file = (endpoint as? UploadEndpoint)?.file
5787

@@ -72,20 +102,15 @@ private extension URLServer {
72102

73103
return ExecuteResult(data: data, request: urlRequest, observers: observers)
74104
}
75-
}
76-
77-
// MARK: - Shared helpers
78-
79-
extension URLServer {
80105

81-
/// Builds the URLRequest for the endpoint, applies optional configuration, and creates observer tokens.
82-
func prepareRequest(
106+
/// Builds the URLRequest for the endpoint, applies optional configuration, and creates observer contexts.
107+
func prepareObservers(
83108
endpoint: Endpoint,
84109
configuring: RequestConfiguring?
85-
) async throws -> (URLRequest, [AnyObserverToken]) {
110+
) async throws -> (URLRequest, [BoundObserverContext]) {
86111
var urlRequest = try await buildRequest(endpoint: endpoint)
87112
try await configuring?.configure(&urlRequest)
88-
let observers = networkObservers.map { AnyObserverToken(observer: $0, request: urlRequest) }
113+
let observers = networkObservers.map { BoundObserverContext(observer: $0, request: urlRequest) }
89114
return (urlRequest, observers)
90115
}
91116

@@ -94,7 +119,7 @@ extension URLServer {
94119
data: Data?,
95120
response: URLResponse,
96121
request: URLRequest,
97-
observers: [AnyObserverToken]
122+
observers: [BoundObserverContext]
98123
) throws {
99124
if let error = ErrorType(data: data, response: response, error: nil, decoding: decoding) {
100125
observers.forEach { $0.didFail(request: request, error: error) }
@@ -103,8 +128,13 @@ extension URLServer {
103128
}
104129
}
105130

106-
/// Type-erasing wrapper that captures an observer and its context from `willSendRequest`.
107-
final class AnyObserverToken: @unchecked Sendable {
131+
/// Captures an observer and its context from `willSendRequest`, binding the lifecycle callbacks
132+
/// for a single request. Created at request start and consumed before the call returns.
133+
///
134+
/// Marked `@unchecked Sendable` because the stored closures capture a `Sendable` observer
135+
/// and its `Sendable` context. Instances are created and consumed within a single async call
136+
/// and never shared across task boundaries.
137+
private final class BoundObserverContext: @unchecked Sendable {
108138
private let _didReceiveResponse: (URLRequest, URLResponse?, Data?) -> Void
109139
private let _didFail: (URLRequest, Error) -> Void
110140

Sources/FTAPIKit/URLServer+Download.swift

Lines changed: 0 additions & 29 deletions
This file was deleted.
Lines changed: 3 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import Foundation
2+
import FTAPIKit
23
import Testing
34

4-
@testable import FTAPIKit
5-
65
/// Tests demonstrating async buildRequest functionality addressing GitHub issue #105
76
@Suite
87
struct AsyncBuildRequestTests {
@@ -11,7 +10,7 @@ struct AsyncBuildRequestTests {
1110
func asyncBuildRequestWithDynamicHeaders() async throws {
1211
let server = DynamicHeaderServer()
1312
let data = try await server.call(data: GetEndpoint())
14-
let response = try JSONDecoder().decode(HTTPBinResponse.self, from: data)
13+
let response = try JSONDecoder().decode(HTTPBinHeadersResponse.self, from: data)
1514
#expect(response.headers["X-App-Version"] == "2.0.0")
1615
#expect(response.headers["X-Device-Id"] == "test-device-123")
1716
}
@@ -22,7 +21,7 @@ struct AsyncBuildRequestTests {
2221
let server = TokenRefreshServer(tokenManager: tokenManager)
2322
tokenManager.currentToken = "expired-token"
2423
let data = try await server.call(data: GetEndpoint())
25-
let response = try JSONDecoder().decode(HTTPBinResponse.self, from: data)
24+
let response = try JSONDecoder().decode(HTTPBinHeadersResponse.self, from: data)
2625
#expect(response.headers["Authorization"] == "Bearer refreshed-token-456")
2726
#expect(tokenManager.refreshCalled)
2827
}
@@ -67,40 +66,3 @@ private struct AppConfiguration {
6766
let appVersion: String
6867
let deviceId: String
6968
}
70-
71-
private final class MockTokenManager: @unchecked Sendable {
72-
private let lock = NSLock()
73-
private var _currentToken: String = "initial-token"
74-
private var _refreshCalled = false
75-
76-
var currentToken: String {
77-
get { lock.withLock { _currentToken } }
78-
set { lock.withLock { _currentToken = newValue } }
79-
}
80-
81-
var refreshCalled: Bool {
82-
lock.withLock { _refreshCalled }
83-
}
84-
85-
func refreshIfNeeded() async {
86-
try? await Task.sleep(nanoseconds: 10_000_000)
87-
lock.withLock {
88-
_refreshCalled = true
89-
_currentToken = "refreshed-token-456"
90-
}
91-
}
92-
}
93-
94-
private struct HTTPBinResponse: Decodable, Sendable {
95-
let headers: [String: String]
96-
97-
private enum CodingKeys: String, CodingKey {
98-
case headers
99-
}
100-
101-
init(from decoder: Decoder) throws {
102-
let container = try decoder.container(keyedBy: CodingKeys.self)
103-
let rawHeaders = try container.decode([String: String].self, forKey: .headers)
104-
self.headers = rawHeaders
105-
}
106-
}

0 commit comments

Comments
 (0)