Adopt Sendable throughout API to support -strict-concurrency=complete (…

…#22) ### Motivation When using `-strict-concurrency=targeted`, `ClientMiddleware` will emit warnings if an actor is conformed to it: ```swift import OpenAPIRuntime import Foundation actor Middleware: ClientMiddleware { func intercept( _ request: Request, baseURL: URL, operationID: String, /// Need to add `@Sendable` or the warnings don't go away even with this PR. Code-completion won't add it. next: @sendable (Request, URL) async throws -> Response ) async throws -> Response { try await next(request, baseURL) } } ``` The `intercept` function will emit the following warning with the current implementation: > Sendability of function types in instance method 'intercept(_:baseURL:operationID:next:)' does not match requirement in protocol 'ClientMiddleware' Repro package: https://github.com/MahdiBM/OAPIRClientWarningsRepro You can change between this PR and the main branch by commenting out the dependency declarations i've already put in the `Package.swift`. Then observe there are no warnings with this PR, unlike with the main branch. ### Modifications Generally adopt Sendable throughout the whole API to support -strict-concurrency=complete. For the most part, this means using @sendable for closures, and using `any Sendable` instead of `Any`. As an example for closures, currently we have: ```swift public protocol ClientMiddleware: Sendable { func intercept( _ request: Request, baseURL: URL, operationID: String, next: (Request, URL) async throws -> Response ) async throws -> Response } ``` With this PR we'll have: ```swift public protocol ClientMiddleware: Sendable { func intercept( _ request: Request, baseURL: URL, operationID: String, next: @sendable (Request, URL) async throws -> Response /// ~~~^ notice `@Sendable` ) async throws -> Response } ``` ### Result Safer Sendable code + being more ready for Swift 6 + no more warnings when using `-strict-concurrency=targeted` then conforming an actor to `ClientMiddleware`. ### Test Plan Adopted all tests to the changes and added the strict-concurrency flag to CI. --------- Co-authored-by: Si Beaumont <[email protected]> Co-authored-by: Si Beaumont <[email protected]>
apple · Jul 14, 2023 · 45b2420 · 45b2420
1 parent fbd56ae
commit 45b2420
Show file tree

Hide file tree

Showing 14 changed files with 76 additions and 49 deletions.
diff --git a/Sources/OpenAPIRuntime/Base/OpenAPIValue.swift b/Sources/OpenAPIRuntime/Base/OpenAPIValue.swift
@@ -52,7 +52,7 @@ public struct OpenAPIValueContainer: Codable, Equatable, Hashable, Sendable {
     /// - Parameter unvalidatedValue: A value of a JSON-compatible type,
     /// such as `String`, `[Any]`, and `[String: Any]`.
     /// - Throws: When the value is not supported.
-    public init(unvalidatedValue: Any? = nil) throws {
+    public init(unvalidatedValue: (any Sendable)? = nil) throws {
         try self.init(validatedValue: Self.tryCast(unvalidatedValue))
     }
 
@@ -62,14 +62,14 @@ public struct OpenAPIValueContainer: Codable, Equatable, Hashable, Sendable {
     /// - Parameter value: An untyped value.
     /// - Returns: A cast value if supported.
     /// - Throws: When the value is not supported.
-    static func tryCast(_ value: Any?) throws -> Sendable? {
+    static func tryCast(_ value: (any Sendable)?) throws -> Sendable? {
         guard let value = value else {
             return nil
         }
-        if let array = value as? [Any?] {
+        if let array = value as? [(any Sendable)?] {
             return try array.map(tryCast(_:))
         }
-        if let dictionary = value as? [String: Any?] {
+        if let dictionary = value as? [String: (any Sendable)?] {
             return try dictionary.mapValues(tryCast(_:))
         }
         if let value = tryCastPrimitiveType(value) {
@@ -87,7 +87,7 @@ public struct OpenAPIValueContainer: Codable, Equatable, Hashable, Sendable {
     /// Returns the specified value cast to a supported primitive type.
     /// - Parameter value: An untyped value.
     /// - Returns: A cast value if supported, nil otherwise.
-    static func tryCastPrimitiveType(_ value: Any) -> Sendable? {
+    static func tryCastPrimitiveType(_ value: any Sendable) -> (any Sendable)? {
         switch value {
         case is String, is Int, is Bool, is Double:
             return value
@@ -327,7 +327,7 @@ public struct OpenAPIObjectContainer: Codable, Equatable, Hashable, Sendable {
 
     public func encode(to encoder: Encoder) throws {
         var container = encoder.singleValueContainer()
-        try container.encode(value.mapValues(OpenAPIValueContainer.init))
+        try container.encode(value.mapValues(OpenAPIValueContainer.init(validatedValue:)))
     }
 
     // MARK: Equatable
@@ -430,7 +430,7 @@ public struct OpenAPIArrayContainer: Codable, Equatable, Hashable, Sendable {
 
     public func encode(to encoder: Encoder) throws {
         var container = encoder.singleValueContainer()
-        try container.encode(value.map(OpenAPIValueContainer.init))
+        try container.encode(value.map(OpenAPIValueContainer.init(validatedValue:)))
     }
 
     // MARK: Equatable

diff --git a/Sources/OpenAPIRuntime/Conversion/Converter.swift b/Sources/OpenAPIRuntime/Conversion/Converter.swift
@@ -14,6 +14,7 @@
 #if canImport(Darwin)
 import Foundation
 #else
+// `@preconcrrency` is for `JSONDecoder`/`JSONEncoder`.
 @preconcurrency import Foundation
 #endif
 

diff --git a/Sources/OpenAPIRuntime/Errors/ClientError.swift b/Sources/OpenAPIRuntime/Errors/ClientError.swift
@@ -11,7 +11,12 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 //===----------------------------------------------------------------------===//
+#if canImport(Darwin)
 import Foundation
+#else
+// `@preconcrrency` is for `URL`.
+@preconcurrency import Foundation
+#endif
 
 /// An error thrown by a client performing an OpenAPI operation.
 ///
@@ -24,7 +29,7 @@ public struct ClientError: Error {
     public var operationID: String
 
     /// The operation-specific Input value.
-    public var operationInput: Any
+    public var operationInput: any Sendable
 
     /// The HTTP request created during the operation.
     ///
@@ -56,7 +61,7 @@ public struct ClientError: Error {
     ///   - underlyingError: The underlying error that caused the operation to fail.
     public init(
         operationID: String,
-        operationInput: Any,
+        operationInput: any Sendable,
         request: Request? = nil,
         baseURL: URL? = nil,
         response: Response? = nil,

diff --git a/Sources/OpenAPIRuntime/Errors/ServerError.swift b/Sources/OpenAPIRuntime/Errors/ServerError.swift
@@ -27,12 +27,12 @@ public struct ServerError: Error {
     /// Operation-specific Input value.
     ///
     /// Is nil if error was thrown during request -> Input conversion.
-    public var operationInput: Any?
+    public var operationInput: (any Sendable)?
 
     /// Operation-specific Output value.
     ///
     /// Is nil if error was thrown before/during Output -> response conversion.
-    public var operationOutput: Any?
+    public var operationOutput: (any Sendable)?
 
     /// The underlying error that caused the operation to fail.
     public var underlyingError: Error
@@ -50,8 +50,8 @@ public struct ServerError: Error {
         operationID: String,
         request: Request,
         requestMetadata: ServerRequestMetadata,
-        operationInput: Any? = nil,
-        operationOutput: Any? = nil,
+        operationInput: (any Sendable)? = nil,
+        operationOutput: (any Sendable)? = nil,
         underlyingError: Error
     ) {
         self.operationID = operationID

diff --git a/Sources/OpenAPIRuntime/Interface/ClientTransport.swift b/Sources/OpenAPIRuntime/Interface/ClientTransport.swift
@@ -233,6 +233,6 @@ public protocol ClientMiddleware: Sendable {
         _ request: Request,
         baseURL: URL,
         operationID: String,
-        next: (Request, URL) async throws -> Response
+        next: @Sendable (Request, URL) async throws -> Response
     ) async throws -> Response
 }
diff --git a/Sources/OpenAPIRuntime/Interface/CurrencyTypes.swift b/Sources/OpenAPIRuntime/Interface/CurrencyTypes.swift
@@ -14,9 +14,42 @@
 #if canImport(Darwin)
 import Foundation
 #else
+// `@preconcrrency` is for `Data`/`URLQueryItem`.
 @preconcurrency import Foundation
 #endif
 
+/// A protected-by-locks storage for ``redactedHeaderFields``.
+private class RedactedHeadersStorage: @unchecked Sendable {
+    /// The underlying storage of ``redactedHeaderFields``,
+    /// protected by a lock.
+    private var _locked_redactedHeaderFields: Set<String> = HeaderField.defaultRedactedHeaderFields
+
+    /// The header fields to be redacted.
+    var redactedHeaderFields: Set<String> {
+        get {
+            lock.lock()
+            defer {
+                lock.unlock()
+            }
+            return _locked_redactedHeaderFields
+        }
+        set {
+            lock.lock()
+            defer {
+                lock.unlock()
+            }
+            _locked_redactedHeaderFields = newValue
+        }
+    }
+
+    /// The lock used for protecting access to `_locked_redactedHeaderFields`.
+    private let lock: NSLock = {
+        let lock = NSLock()
+        lock.name = "com.apple.swift-openapi-runtime.lock.redactedHeaderFields"
+        return lock
+    }()
+}
+
 /// A header field used in an HTTP request or response.
 public struct HeaderField: Equatable, Hashable, Sendable {
 
@@ -47,20 +80,12 @@ extension HeaderField {
     /// Use this to avoid leaking sensitive tokens into application logs.
     public static var redactedHeaderFields: Set<String> {
         set {
-            _lock_redactedHeaderFields.lock()
-            defer {
-                _lock_redactedHeaderFields.unlock()
-            }
             // Save lowercased versions of the header field names to make
             // membership checking O(1).
-            _locked_redactedHeaderFields = Set(newValue.map { $0.lowercased() })
+            redactedHeadersStorage.redactedHeaderFields = Set(newValue.map { $0.lowercased() })
         }
         get {
-            _lock_redactedHeaderFields.lock()
-            defer {
-                _lock_redactedHeaderFields.unlock()
-            }
-            return _locked_redactedHeaderFields
+            return redactedHeadersStorage.redactedHeaderFields
         }
     }
 
@@ -71,16 +96,7 @@ extension HeaderField {
         "set-cookie",
     ]
 
-    /// The lock used for protecting access to `_locked_redactedHeaderFields`.
-    private static let _lock_redactedHeaderFields: NSLock = {
-        let lock = NSLock()
-        lock.name = "com.apple.swift-openapi-runtime.lock.redactedHeaderFields"
-        return lock
-    }()
-
-    /// The underlying storage of ``HeaderField/redactedHeaderFields``,
-    /// protected by a lock.
-    private static var _locked_redactedHeaderFields: Set<String> = defaultRedactedHeaderFields
+    private static let redactedHeadersStorage = RedactedHeadersStorage()
 }
 
 /// Describes the HTTP method used in an OpenAPI operation.

diff --git a/Sources/OpenAPIRuntime/Interface/ServerTransport.swift b/Sources/OpenAPIRuntime/Interface/ServerTransport.swift
@@ -112,9 +112,7 @@ public protocol ServerTransport {
     ///   - queryItemNames: The names of query items to be extracted
     ///   from the request URL that matches the provided HTTP operation.
     func register(
-        _ handler: @Sendable @escaping (
-            Request, ServerRequestMetadata
-        ) async throws -> Response,
+        _ handler: @Sendable @escaping (Request, ServerRequestMetadata) async throws -> Response,
         method: HTTPMethod,
         path: [RouterPathComponent],
         queryItemNames: Set<String>
@@ -219,6 +217,6 @@ public protocol ServerMiddleware: Sendable {
         _ request: Request,
         metadata: ServerRequestMetadata,
         operationID: String,
-        next: (Request, ServerRequestMetadata) async throws -> Response
+        next: @Sendable (Request, ServerRequestMetadata) async throws -> Response
     ) async throws -> Response
 }
diff --git a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift
@@ -14,6 +14,7 @@
 #if canImport(Darwin)
 import Foundation
 #else
+// `@preconcrrency` is for `URL`.
 @preconcurrency import Foundation
 #endif
 
@@ -86,9 +87,10 @@ public struct UniversalClient: Sendable {
     public func send<OperationInput, OperationOutput>(
         input: OperationInput,
         forOperation operationID: String,
-        serializer: (OperationInput) throws -> Request,
-        deserializer: (Response) throws -> OperationOutput
-    ) async throws -> OperationOutput {
+        serializer: @Sendable (OperationInput) throws -> Request,
+        deserializer: @Sendable (Response) throws -> OperationOutput
+    ) async throws -> OperationOutput where OperationInput: Sendable, OperationOutput: Sendable {
+        @Sendable
         func wrappingErrors<R>(
             work: () async throws -> R,
             mapError: (Error) -> Error
@@ -121,7 +123,7 @@ public struct UniversalClient: Sendable {
             makeError(error: error)
         }
         let response: Response = try await wrappingErrors {
-            var next: (Request, URL) async throws -> Response = { (_request, _url) in
+            var next: @Sendable (Request, URL) async throws -> Response = { (_request, _url) in
                 try await wrappingErrors {
                     try await transport.send(
                         _request,

diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift
@@ -88,10 +88,11 @@ public struct UniversalServer<APIHandler: Sendable>: Sendable {
         request: Request,
         with metadata: ServerRequestMetadata,
         forOperation operationID: String,
-        using handlerMethod: @escaping (APIHandler) -> ((OperationInput) async throws -> OperationOutput),
-        deserializer: @escaping (Request, ServerRequestMetadata) throws -> OperationInput,
-        serializer: @escaping (OperationOutput, Request) throws -> Response
-    ) async throws -> Response {
+        using handlerMethod: @Sendable @escaping (APIHandler) -> ((OperationInput) async throws -> OperationOutput),
+        deserializer: @Sendable @escaping (Request, ServerRequestMetadata) throws -> OperationInput,
+        serializer: @Sendable @escaping (OperationOutput, Request) throws -> Response
+    ) async throws -> Response where OperationInput: Sendable, OperationOutput: Sendable {
+        @Sendable
         func wrappingErrors<R>(
             work: () async throws -> R,
             mapError: (Error) -> Error
@@ -102,6 +103,7 @@ public struct UniversalServer<APIHandler: Sendable>: Sendable {
                 throw mapError(error)
             }
         }
+        @Sendable
         func makeError(
             input: OperationInput? = nil,
             output: OperationOutput? = nil,
@@ -116,7 +118,7 @@ public struct UniversalServer<APIHandler: Sendable>: Sendable {
                 underlyingError: error
             )
         }
-        var next: (Request, ServerRequestMetadata) async throws -> Response = { _request, _metadata in
+        var next: @Sendable (Request, ServerRequestMetadata) async throws -> Response = { _request, _metadata in
             let input: OperationInput = try await wrappingErrors {
                 try deserializer(_request, _metadata)
             } mapError: { error in

diff --git a/docker/docker-compose.2204.58.yaml b/docker/docker-compose.2204.58.yaml
@@ -13,6 +13,7 @@ services:
     environment:
       - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
+      - STRICT_CONCURRENCY_ARG=-Xswiftc -strict-concurrency=complete
 
   shell:
     image: *image
diff --git a/docker/docker-compose.2204.59.yaml b/docker/docker-compose.2204.59.yaml
@@ -12,6 +12,7 @@ services:
     environment:
       - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
+      - STRICT_CONCURRENCY_ARG=-Xswiftc -strict-concurrency=complete
 
   shell:
     image: *image
diff --git a/docker/docker-compose.2204.main.yaml b/docker/docker-compose.2204.main.yaml
@@ -13,6 +13,7 @@ services:
     environment:
       - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors
       - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
+      - STRICT_CONCURRENCY_ARG=-Xswiftc -strict-concurrency=complete
 
   shell:
     image: *image
diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml
@@ -30,7 +30,7 @@ services:
 
   test:
     <<: *common
-    command: /bin/bash -xcl "swift $${SWIFT_TEST_VERB-test} $${WARN_AS_ERROR_ARG-} $${SANITIZER_ARG-} $${IMPORT_CHECK_ARG-}"
+    command: /bin/bash -xcl "swift $${SWIFT_TEST_VERB-test} $${WARN_AS_ERROR_ARG-} $${SANITIZER_ARG-} $${IMPORT_CHECK_ARG-} $${STRICT_CONCURRENCY_ARG-}"
 
   shell:
     <<: *common

diff --git a/scripts/run-integration-test.sh b/scripts/run-integration-test.sh
@@ -43,6 +43,6 @@ swift package --package-path "${INTEGRATION_TEST_PACKAGE_PATH}" \
     edit "${PACKAGE_NAME}" --path "${PACKAGE_PATH}"
 
 log "Building integration test package: ${INTEGRATION_TEST_PACKAGE_PATH}"
-swift build --package-path "${INTEGRATION_TEST_PACKAGE_PATH}"
+swift build --package-path "${INTEGRATION_TEST_PACKAGE_PATH}" -Xswiftc -strict-concurrency=complete
 
 log "✅ Successfully built integration test package."