Async nonblocking fileio (#597)

soto-project · Mar 30, 2024 · 8383e7b · 8383e7b
1 parent fb597d5
commit 8383e7b
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 80 deletions.
diff --git a/Package.swift b/Package.swift
@@ -29,12 +29,12 @@ let package = Package(
         .library(name: "SotoSignerV4", targets: ["SotoSignerV4"]),
     ],
     dependencies: [
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.63.0"),
         .package(url: "https://github.com/apple/swift-atomics.git", from: "1.1.0"),
         .package(url: "https://github.com/apple/swift-crypto.git", "1.0.0"..<"4.0.0"),
         .package(url: "https://github.com/apple/swift-distributed-tracing.git", from: "1.0.1"),
         .package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"),
         .package(url: "https://github.com/apple/swift-metrics.git", "1.0.0"..<"3.0.0"),
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.42.0"),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.7.2"),
         .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.13.1"),
         .package(url: "https://github.com/swift-server/async-http-client.git", from: "1.19.0"),

diff --git a/Sources/SotoCore/AWSClient.swift b/Sources/SotoCore/AWSClient.swift
@@ -48,8 +48,6 @@ public final class AWSClient: Sendable {
     public let httpClient: HTTPClient
     /// Keeps a record of how we obtained the HTTP client
     let httpClientProvider: HTTPClientProvider
-    /// EventLoopGroup used by AWSClient
-    public var eventLoopGroup: EventLoopGroup { return self.httpClient.eventLoopGroup }
     /// Logger used for non-request based output
     let clientLogger: Logger
     /// client options

diff --git a/Sources/SotoCore/AWSService.swift b/Sources/SotoCore/AWSService.swift
@@ -39,8 +39,6 @@ extension AWSService {
     public var region: Region { return config.region }
     /// The url to use in requests
     public var endpoint: String { return config.endpoint }
-    /// The EventLoopGroup service is using
-    public var eventLoopGroup: EventLoopGroup { return client.eventLoopGroup }
 
     /// Return new version of Service with edited parameters
     /// - Parameters:

diff --git a/Sources/SotoCore/Credential/ConfigFileCredentialProvider.swift b/Sources/SotoCore/Credential/ConfigFileCredentialProvider.swift
@@ -15,6 +15,7 @@
 import Logging
 import NIOConcurrencyHelpers
 import NIOCore
+import NIOPosix
 import SotoSignerV4
 
 final class ConfigFileCredentialProvider: CredentialProviderSelector {
@@ -60,14 +61,15 @@ final class ConfigFileCredentialProvider: CredentialProviderSelector {
         configFilePath: String,
         for profile: String,
         context: CredentialProviderFactory.Context,
-        endpoint: String?
+        endpoint: String?,
+        threadPool: NIOThreadPool = .singleton
     ) async throws -> CredentialProvider {
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
             credentialsFilePath: credentialsFilePath,
             configFilePath: configFilePath,
             profile: profile,
-            context: context
-        ).get()
+            threadPool: threadPool
+        )
         return try self.credentialProvider(from: sharedCredentials, context: context, endpoint: endpoint)
     }
 

diff --git a/Sources/SotoCore/Credential/ConfigFileLoader.swift b/Sources/SotoCore/Credential/ConfigFileLoader.swift
@@ -94,36 +94,31 @@ enum ConfigFileLoader {
         credentialsFilePath: String,
         configFilePath: String,
         profile: String,
-        context: CredentialProviderFactory.Context
-    ) -> EventLoopFuture<SharedCredentials> {
-        let threadPool = NIOThreadPool(numberOfThreads: 1)
-        threadPool.start()
+        threadPool: NIOThreadPool = .singleton
+    ) async throws -> SharedCredentials {
         let fileIO = NonBlockingFileIO(threadPool: threadPool)
-
-        // Load credentials file
-        return self.loadFile(path: credentialsFilePath, on: context.httpClient.eventLoopGroup.any(), using: fileIO)
-            .flatMap { credentialsByteBuffer in
-                // Load profile config file
-                return self.loadFile(path: configFilePath, on: context.httpClient.eventLoopGroup.any(), using: fileIO)
-                    .map {
-                        (credentialsByteBuffer, $0)
-                    }
-                    .flatMapError { _ in
-                        // Recover from error if profile config file does not exist
-                        context.httpClient.eventLoopGroup.any().makeSucceededFuture((credentialsByteBuffer, nil))
-                    }
-            }
-            .flatMapErrorThrowing { _ in
-                // Throw `.noProvider` error if credential file cannot be loaded
-                throw CredentialProviderError.noProvider
-            }
-            .flatMapThrowing { credentialsByteBuffer, configByteBuffer in
-                return try self.parseSharedCredentials(from: credentialsByteBuffer, configByteBuffer: configByteBuffer, for: profile)
-            }
-            .always { _ in
-                // shutdown the threadpool async
-                threadPool.shutdownGracefully { _ in }
-            }
+        let credentialsByteBuffer: ByteBuffer
+        do {
+            // Load credentials file
+            credentialsByteBuffer = try await self.loadFile(
+                path: credentialsFilePath,
+                fileIO: fileIO
+            )
+        } catch {
+            // Throw `.noProvider` error if credential file cannot be loaded
+            throw CredentialProviderError.noProvider
+        }
+        let configByteBuffer: ByteBuffer?
+        do {
+            // Load profile config file
+            configByteBuffer = try await self.loadFile(
+                path: configFilePath,
+                fileIO: fileIO
+            )
+        } catch {
+            configByteBuffer = nil
+        }
+        return try self.parseSharedCredentials(from: credentialsByteBuffer, configByteBuffer: configByteBuffer, for: profile)
     }
 
     /// Load a file from disk without blocking the current thread
@@ -132,17 +127,11 @@ enum ConfigFileLoader {
     ///   - eventLoop: event loop to run everything on
     ///   - fileIO: non-blocking file IO
     /// - Returns: Event loop future with file contents in a byte-buffer
-    static func loadFile(path: String, on eventLoop: EventLoop, using fileIO: NonBlockingFileIO) -> EventLoopFuture<ByteBuffer> {
+    static func loadFile(path: String, fileIO: NonBlockingFileIO) async throws -> ByteBuffer {
         let path = self.expandTildeInFilePath(path)
-
-        return fileIO.openFile(path: path, eventLoop: eventLoop)
-            .flatMap { handle, region in
-                fileIO.read(fileRegion: region, allocator: ByteBufferAllocator(), eventLoop: eventLoop).and(value: handle)
-            }
-            .flatMapThrowing { byteBuffer, handle in
-                try handle.close()
-                return byteBuffer
-            }
+        return try await fileIO.withFileRegion(path: path) { fileRegion in
+            try await fileIO.read(fileHandle: fileRegion.fileHandle, byteCount: fileRegion.readableBytes, allocator: ByteBufferAllocator())
+        }
     }
 
     // MARK: - Byte Buffer parsing (INIParser)

diff --git a/Tests/SotoCoreTests/Concurrency/ExpiringValueTests.swift b/Tests/SotoCoreTests/Concurrency/ExpiringValueTests.swift
@@ -48,6 +48,7 @@ final class ExpiringValueTests: XCTestCase {
             return (1, Date())
         }
         await Task.yield()
+        await Task.yield()
         // test it return current value
         XCTAssertEqual(value, 0)
         // test it kicked off a task

diff --git a/Tests/SotoCoreTests/Credential/ConfigFileCredentialProviderTests.swift b/Tests/SotoCoreTests/Credential/ConfigFileCredentialProviderTests.swift
@@ -172,9 +172,8 @@ class ConfigFileCredentialProviderTests: XCTestCase {
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
             credentialsFilePath: filename,
             configFilePath: "/dev/null",
-            profile: profile,
-            context: context
-        ).get()
+            profile: profile
+        )
 
         switch sharedCredentials {
         case .assumeRole(let aRoleArn, _, _, let sourceCredentialProvider):

diff --git a/Tests/SotoCoreTests/Credential/ConfigFileLoaderTests.swift b/Tests/SotoCoreTests/Credential/ConfigFileLoaderTests.swift
@@ -45,7 +45,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -55,9 +55,8 @@ class ConfigFileLoadersTests: XCTestCase {
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
             credentialsFilePath: credentialsPath,
             configFilePath: "/dev/null",
-            profile: profile,
-            context: context
-        ).get()
+            profile: profile
+        )
 
         switch sharedCredentials {
         case .staticCredential(let credentials):
@@ -102,9 +101,8 @@ class ConfigFileLoadersTests: XCTestCase {
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
             credentialsFilePath: credentialsPath,
             configFilePath: configPath,
-            profile: profile,
-            context: context
-        ).get()
+            profile: profile
+        )
 
         switch sharedCredentials {
         case .assumeRole(let aRoleArn, let aSessionName, let region, let sourceCredentialProvider):
@@ -139,9 +137,8 @@ class ConfigFileLoadersTests: XCTestCase {
         let sharedCredentials = try await ConfigFileLoader.loadSharedCredentials(
             credentialsFilePath: credentialsPath,
             configFilePath: "non-existing-file-path",
-            profile: profile,
-            context: context
-        ).get()
+            profile: profile
+        )
 
         switch sharedCredentials {
         case .assumeRole(let aRoleArn, _, _, let source):
@@ -164,7 +161,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -175,9 +172,8 @@ class ConfigFileLoadersTests: XCTestCase {
             _ = try await ConfigFileLoader.loadSharedCredentials(
                 credentialsFilePath: credentialsPath,
                 configFilePath: "/dev/null",
-                profile: profile,
-                context: context
-            ).get()
+                profile: profile
+            )
         } catch ConfigFileLoader.ConfigFileError.missingAccessKeyId {
             // Pass
         } catch {
@@ -194,7 +190,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -205,9 +201,8 @@ class ConfigFileLoadersTests: XCTestCase {
             _ = try await ConfigFileLoader.loadSharedCredentials(
                 credentialsFilePath: credentialsPath,
                 configFilePath: "/dev/null",
-                profile: profile,
-                context: context
-            ).get()
+                profile: profile
+            )
         } catch ConfigFileLoader.ConfigFileError.missingSecretAccessKey {
             // Pass
         } catch {
@@ -229,7 +224,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -240,9 +235,8 @@ class ConfigFileLoadersTests: XCTestCase {
             _ = try await ConfigFileLoader.loadSharedCredentials(
                 credentialsFilePath: credentialsPath,
                 configFilePath: "/dev/null",
-                profile: profile,
-                context: context
-            ).get()
+                profile: profile
+            )
         } catch ConfigFileLoader.ConfigFileError.missingAccessKeyId {
             // Pass
         } catch {
@@ -264,7 +258,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -275,9 +269,8 @@ class ConfigFileLoadersTests: XCTestCase {
             _ = try await ConfigFileLoader.loadSharedCredentials(
                 credentialsFilePath: credentialsPath,
                 configFilePath: "/dev/null",
-                profile: profile,
-                context: context
-            ).get()
+                profile: profile
+            )
         } catch ConfigFileLoader.ConfigFileError.missingSecretAccessKey {
             // Pass
         } catch {
@@ -294,7 +287,7 @@ class ConfigFileLoadersTests: XCTestCase {
         """
 
         let credentialsPath = try save(content: credentialsFile, prefix: #function)
-        let (context, httpClient) = try makeContext()
+        let (_, httpClient) = try makeContext()
 
         defer {
             try? FileManager.default.removeItem(atPath: credentialsPath)
@@ -305,9 +298,8 @@ class ConfigFileLoadersTests: XCTestCase {
             _ = try await ConfigFileLoader.loadSharedCredentials(
                 credentialsFilePath: credentialsPath,
                 configFilePath: "/dev/null",
-                profile: profile,
-                context: context
-            ).get()
+                profile: profile
+            )
         } catch ConfigFileLoader.ConfigFileError.invalidCredentialFile {
             // Pass
         } catch {