Documentation & code review fixes

Author: plajdo

Sources/GoodNetworking/Interception/AuthenticationInterceptor.swift

@@ -7,6 +7,10 @@
 
 import Foundation
 
+/// Interceptor responsible for authentication of network requests.
+///
+/// Authentication interceptor requires an instance of ``Authenticator``, which handles
+/// the logic of refreshing the credential, checking its validity or caching.
 public final class AuthenticationInterceptor<AuthenticatorType: Authenticator>: Interceptor, @unchecked Sendable {
 
     private let authenticator: AuthenticatorType

Sources/GoodNetworking/Interception/Authenticator.swift

@@ -9,23 +9,108 @@ import Foundation
 
 // MARK: - Authenticator
 
+/// Authenticators provide the concrete implementation of authentication,
+/// credential management and refreshing of expired credentials for
+/// `AuthenticationInterceptor`.
 public protocol Authenticator: Sendable {
 
+    /// Persistent credential used to authorize requests.
+    ///
+    /// This is often a combination of access and refresh tokens,
+    /// an API key, an authentication string, passphrase, etc..
+    ///
+    /// Credentials conforming to ``RefreshableCredential`` can indicate
+    /// their validity and thus request a credential refresh early, resulting in better
+    /// networking performance.
     associatedtype Credential
-
+    
+    /// Return the latest available credential from storage (eg. cache).
+    /// - Returns: ``Credential`` if available or `nil`
     func getCredential() async -> Credential?
+    
+    /// Stores the new credential to storage (eg. cache).
+    ///
+    /// If the new credential is `nil`, this function MUST remove the
+    /// cached credential from storage. Subsequent calls to ``getCredential()``
+    /// must then return `nil`.
+    ///
+    /// - Parameter newCredential: New credential to store or `nil` if
+    /// credential should be deleted.
     func storeCredential(_ newCredential: Credential?) async
-
+    
+    /// Applies the credential to a URL request.
+    ///
+    /// When using HTTP with Bearer authorization, this function is responsible for
+    /// adding the `Authorization` header to the request.
+    ///
+    /// - Parameters:
+    ///   - credential: Credential to be added to the request
+    ///   - request: Request to modify
     func apply(credential: Credential, to request: inout URLRequest) async throws(NetworkError)
+    
+    /// Refreshes the expired or invalid credential.
+    ///
+    /// This function is responsible for refreshing the credential - the way this is done
+    /// is up to the implementation. Most often it will involve making a network call
+    /// to a backend authorization service.
+    ///
+    /// - note: Parameter `credential` may contain a valid credential, if the expiration
+    /// date is known and the credential is able to request a refresh. See ``RefreshableCredential``.
+    ///
+    /// - important: This session will block other requests while refresh is pending.
+    /// As a result, all potential refresh requests must be handled by another network session.
+    ///
+    /// - Parameter credential: Credential that needs to be refreshed.
+    /// - Returns: Refreshed and valid credential
+    /// - Throws: This function can throw a ``NetworkError``, if refreshing the credential fails, or
+    /// the credential cannot be refreshed.
     func refresh(credential: Credential) async throws(NetworkError) -> Credential
-    func didRequest(_ request: inout URLRequest, failDueToAuthenticationError: HTTPError) -> Bool
+    
+    /// Checks whether invalid response from the backend is an authentication error.
+    ///
+    /// The simplest implementation would be a check if the status code is `401 Unauthorized`.
+    /// This implementation, however, has a flaw. For example - in case the system gates access
+    /// to resources which require a certain level of authorization, it is possible that a resource is unavailable,
+    /// but the client can authenticate with a higher level of permissions to access it.
+    ///
+    /// - important: It is **highly recommended** to inspect the backend response and decide
+    /// whether the credential is invalid/expired, or the resource is not accessible to the current user
+    /// and the credential itself is correct.
+    ///
+    /// - Parameters:
+    ///   - request: Failed URL request
+    ///   - failDueToAuthenticationError: Remote error containing the backend response and status code
+    /// - Returns: `true` if it can be safely determined, that the failure occured due to invalid credential
+    func didRequest(_ request: inout URLRequest, failDueToAuthenticationError error: HTTPError) -> Bool
+    
+    /// Verifies if a request is authenticated with a given credential.
+    ///
+    /// The simplest implementation in a system using HTTP with Bearer authorization
+    /// would be checking if access token from ``Credential`` matches the token
+    /// in `Authorization` header.
+    ///
+    /// - Parameters:
+    ///   - request: URL request to verify
+    ///   - credential: Credential which is expected to authenticate the request
+    /// - Returns: `true` if request is authenticated using the `credential`, or `false` otherwise
+    /// (if the request is unauthenticated or the credentials do not match).
     func isRequest(_ request: inout URLRequest, authenticatedWith credential: Credential) -> Bool
+    
+    /// Notifies the user that credential could not be refreshed and failed due to an error.
+    ///
+    /// This function is often responsible for changing the app state to show an alert,
+    /// open the verification/login dialog, or cleaning up now invalid resources
+    /// (eg. authorized image cache).
+    ///
+    /// - Parameter error: Remote error containing the backend response and status code
+    /// when trying to refresh invalid credential, which could not be refreshed.
     func refresh(didFailDueToError error: HTTPError) async
 
 }
 
 // MARK: - No authenticator
 
+/// Empty implementation of authenticator for unauthorized network sessions.
 public final class NoAuthenticator: Authenticator {
 
     public typealias Credential = Void
@@ -43,8 +128,20 @@ public final class NoAuthenticator: Authenticator {
 
 // MARK: - Refreshable credential
 
+/// Denotes that a credential is refreshable and is capable of telling whether a refresh
+/// is required before making an invalid network call.
+///
+/// Conforming a `Credential` to this protocol is not required, however, it will
+/// improve the networking performance by refreshing the token early, before a failed
+/// API call.
 public protocol RefreshableCredential {
-
+    
+    /// This property should return `true` if the credential knows that a refresh
+    /// is required.
+    ///
+    /// In the simplest case it can contain logic comparing expiration date
+    /// with the current date minus a time delta (eg. 5 minutes before expiration).
+    ///
     var requiresRefresh: Bool { get }
 
 }

Sources/GoodNetworking/Interception/Interceptor.swift

@@ -11,15 +11,16 @@ import Foundation
 
 public protocol Interceptor: Adapter, Retrier {}
 
-// MARK: - No interceptor
+// MARK: - Default interceptor
 
-public final class NoInterceptor: Interceptor {
+public final class DefaultInterceptor: Interceptor {
 
     public init() {}
 
     public func adapt(urlRequest: inout URLRequest) async throws(NetworkError) {}
 
     public func retry(urlRequest: inout URLRequest, for session: NetworkSession, dueTo error: NetworkError) async throws(NetworkError) -> RetryResult {
+        #warning("TODO: better default retry logic (handle error type, HTTP method, ...")
         return .doNotRetry
     }
 

Sources/GoodNetworking/Interception/Retrier.swift

@@ -17,7 +17,7 @@ public protocol Retrier: Sendable {
 
 // MARK: - Retry result
 
-public enum RetryResult {
+public enum RetryResult: Sendable {
 
     case doNotRetry
     case retryAfter(TimeInterval)

Sources/GoodNetworking/Models/Endpoint.swift

@@ -9,7 +9,7 @@ import Foundation
 
 // MARK: - Endpoint
 
-/// `GREndpoint` protocol defines a set of requirements for an endpoint.
+/// `Endpoint` protocol defines a set of requirements for an endpoint.
 public protocol Endpoint {
 
     /// The path to be appended to the base URL.

Sources/GoodNetworking/Models/EndpointFactory.swift

@@ -23,7 +23,7 @@ public final class EndpointFactory: Endpoint {
         AutomaticEncoding.default
     }
 
-    init(path: URLConvertible) {
+    init(at path: URLConvertible) {
         self.path = path
     }
 
@@ -91,5 +91,5 @@ private extension EndpointFactory {
 }
 
 public func at(_ path: URLConvertible) -> EndpointFactory {
-    EndpointFactory(path: path)
+    EndpointFactory(at: path)
 }

Sources/GoodNetworking/Models/HTTPMethod.swift

@@ -9,6 +9,8 @@ import Foundation
 
 // MARK: - Method
 
+/// Enumeration of all HTTP methods as stated in
+/// [RFC9110 specification](https://httpwg.org/specs/rfc9110.html#rfc.section.9.3)
 @frozen public enum HTTPMethod: String {
 
     case get = "GET"

Sources/GoodNetworking/Models/Header.swift

@@ -9,6 +9,8 @@ import Foundation
 
 // MARK: - HTTPHeader
 
+/// HTTP headers are colon separated name-value pairs used for specifying request
+/// details, authentication and more.
 public struct HTTPHeader: Equatable, Hashable, HeaderConvertible {
 
     public let name: String
@@ -45,7 +47,11 @@ public struct HTTPHeader: Equatable, Hashable, HeaderConvertible {
         self.name = String(split[0])
         self.value = String(split[1])
     }
-
+    
+    /// Initialize HTTPHeader as a name-value pair.
+    /// - Parameters:
+    ///   - name: Name of the header (part before colon)
+    ///   - value: Value of the header (part after colon)
     public init(name: String, value: String) {
         self.name = name
         self.value = value
@@ -75,29 +81,68 @@ extension HTTPHeader: CustomStringConvertible {
 
 // MARK: - HTTPHeaders
 
-public struct HTTPHeaders: Equatable, Hashable, Sendable {
-
-    public var headers: [HTTPHeader]
-
+/// A collection of multiple headers. Can contain any entities convertible to
+/// `HTTPHeader` (`HeaderConvertible`). Final header names
+/// and values are resolved at the time the request is sent.
+public struct HTTPHeaders: Sendable {
+    
+    /// List of contained headers
+    public var headers: [any HeaderConvertible]
+    
+    /// Create collection of `HTTPHeader`-s from key-value dictionary mapped as
+    /// name-value header pairs.
+    /// - Parameter headers: Dictionary, where keys are header names
     public init(_ headers: [String: String]) {
         self.headers = headers.map(HTTPHeader.init).reduce(into: [], { $0.append($1) })
     }
 
+    /// Get the value of a header with name `name`
     public subscript(_ name: String) -> String? {
         value(for: name)
     }
-
+    
+    /// Resolves all headers to their final values and returns the value for header
+    /// with a given name.
+    ///
+    /// - important: This operation resolves all header values and can be expensive
+    ///
+    /// - Parameter name: Name of the header
+    /// - Returns: Value of the specified header
     public func value(for name: String) -> String? {
-        guard let index = headers.firstIndex(where: { $0.name == name }) else { return nil }
-        return headers[index].value
+        let resolved = resolve()
+        guard let index = resolved.firstIndex(where: { $0.name == name }) else { return nil }
+        return resolved[index].value
     }
 
-    public mutating func add(header: HTTPHeader) {
+    /// Appends a new entity to the header collection. Does not resolve
+    /// the header name or value.
+    /// - Parameter header: New header
+    public mutating func add(header: any HeaderConvertible) {
         headers.append(header)
     }
+    
+    /// Resolve all headers to their final values.
+    /// - Returns: Array of resolved headers as `HTTPHeader`-s
+    public func resolve() -> [HTTPHeader] {
+        headers.map { $0.resolveHeader() }
+    }
 
 }
 
+extension HTTPHeaders: Equatable, Hashable {
+    
+    nonisolated public static func == (lhs: Self, rhs: Self) -> Bool {
+        lhs.resolve() == rhs.resolve()
+    }
+    
+    nonisolated public func hash(into hasher: inout Hasher) {
+        for header in self.resolve() {
+            hasher.combine(header)
+        }
+    }
+    
+}
+
 extension HTTPHeaders: ExpressibleByDictionaryLiteral {
 
     public init(dictionaryLiteral elements: (String, String)...) {
@@ -109,14 +154,14 @@ extension HTTPHeaders: ExpressibleByDictionaryLiteral {
 extension HTTPHeaders: ExpressibleByArrayLiteral {
 
     public init(arrayLiteral elements: HeaderConvertible...) {
-        self.headers = elements.map { $0.resolveHeader() }
+        self.headers = elements
     }
 
 }
 
 extension HTTPHeaders: Sequence {
 
-    public func makeIterator() -> IndexingIterator<[HTTPHeader]> {
+    public func makeIterator() -> IndexingIterator<[any HeaderConvertible]> {
         headers.makeIterator()
     }
 
@@ -132,7 +177,7 @@ extension HTTPHeaders: Collection {
         headers.endIndex
     }
 
-    public subscript(position: Int) -> HTTPHeader {
+    public subscript(position: Int) -> any HeaderConvertible {
         headers[position]
     }
 
@@ -145,15 +190,19 @@ extension HTTPHeaders: Collection {
 extension HTTPHeaders: CustomStringConvertible {
 
     public var description: String {
-        headers.map(\.description).joined(separator: "\n")
+        self.resolve().map(\.description).joined(separator: "\n")
     }
 
 }
 
 // MARK: - HeaderConvertible
 
+/// Allows conforming entities to be converted to HTTPHeader
+/// for subsequent use in HTTP requests.
 public protocol HeaderConvertible: Sendable {
-
+    
+    /// Resolves the final name and value of the header
+    /// - Returns: Valid HTTP header name-value pair
     func resolveHeader() -> HTTPHeader
 
 }