Merge pull request #3575 from SwiftPackageIndex/issue-3469-dependency-transition-14

Issue 3469 dependency transition 14

Author: finestructure

Package.swift

@@ -19,7 +19,7 @@ import PackageDescription
 let package = Package(
     name: "SPI-Server",
     platforms: [
-        .macOS(.v13)
+        .macOS(.v15)
     ],
     products: [
         .executable(name: "Run", targets: ["Run"]),

Sources/App/Commands/Ingestion.swift

@@ -171,7 +171,7 @@ enum Ingestion {
             // Even though we have a `Joined<Package, Repository>` as a parameter, we must not rely
             // on `repository` for owner/name as it will be nil when a package is first ingested.
             // The only way to get `owner` and `repository` here is by parsing them from the URL.
-            let (owner, repository) = try await run {
+            let (owner, repository) = try run {
                 if environment.shouldFail(failureMode: .invalidURL) {
                     throw Github.Error.invalidURL(package.model.url)
                 }
@@ -258,13 +258,18 @@ enum Ingestion {
 
     static func fetchMetadata(client: Client, package: Package, owner: String, repository: String) async throws(Github.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
         @Dependency(\.environment) var environment
-        @Dependency(\.github) var github
         if environment.shouldFail(failureMode: .fetchMetadataFailed) {
             throw Github.Error.requestFailed(.internalServerError)
         }
 
-        async let metadata = try await Current.fetchMetadata(client, owner, repository)
-        async let license = await github.fetchLicense(owner, repository)
+        // Need to pull in github functions individually, because otherwise the `async let` will trigger a
+        // concurrency error if github gets used more than once:
+        //   Sending 'github' into async let risks causing data races between async let uses and local uses
+        @Dependency(\.github.fetchMetadata) var fetchMetadata
+        @Dependency(\.github.fetchLicense) var fetchLicense
+
+        async let metadata = try await fetchMetadata(owner, repository)
+        async let license = await fetchLicense(owner, repository)
         async let readme = await Current.fetchReadme(client, owner, repository)
 
         do {

Sources/App/Core/AppEnvironment.swift

@@ -23,7 +23,6 @@ import FoundationNetworking
 
 
 struct AppEnvironment: Sendable {
-    var fetchMetadata: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws(Github.Error) -> Github.Metadata
     var fetchReadme: @Sendable (_ client: Client, _ owner: String, _ repository: String) async -> Github.Readme?
     var fetchS3Readme: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws -> String
     var fileManager: FileManager
@@ -65,7 +64,6 @@ extension AppEnvironment {
     nonisolated(unsafe) static var logger: Logger!
 
     static let live = AppEnvironment(
-        fetchMetadata: { client, owner, repo throws(Github.Error) in try await Github.fetchMetadata(client:client, owner: owner, repository: repo) },
         fetchReadme: { client, owner, repo in await Github.fetchReadme(client:client, owner: owner, repository: repo) },
         fetchS3Readme: { client, owner, repo in try await S3Readme.fetchReadme(client:client, owner: owner, repository: repo) },
         fileManager: .live,

Sources/App/Core/Dependencies/GithubClient.swift

@@ -14,26 +14,35 @@
 
 
 import Dependencies
-import DependenciesMacros
+import IssueReporting
 
 
-@DependencyClient
+// We currently cannot use @DependencyClient here due to
+// https://github.com/pointfreeco/swift-dependencies/discussions/324
+//@DependencyClient
 struct GithubClient {
     var fetchLicense: @Sendable (_ owner: String, _ repository: String) async -> Github.License?
+    var fetchMetadata: @Sendable (_ owner: String, _ repository: String) async throws(Github.Error) -> Github.Metadata = { _,_ in reportIssue("fetchMetadata"); return .init() }
 }
 
 
 extension GithubClient: DependencyKey {
     static var liveValue: Self {
         .init(
-            fetchLicense: { owner, repo in await Github.fetchLicense(owner: owner, repository: repo) }
+            fetchLicense: { owner, repo in await Github.fetchLicense(owner: owner, repository: repo) },
+            fetchMetadata: { owner, repo throws(Github.Error) in try await Github.fetchMetadata(owner: owner, repository: repo) }
         )
     }
 }
 
 
 extension GithubClient: TestDependencyKey {
-    static var testValue: Self { Self() }
+    static var testValue: Self {
+        .init(
+            fetchLicense: { _, _ in unimplemented("fetchLicense"); return nil },
+            fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() }
+        )
+    }
 }
 
 

Sources/App/Core/Dependencies/HTTPClient.swift

@@ -106,8 +106,9 @@ extension HTTPClient.Response {
         self.init(host: "host", status: status, version: version, headers: headers, body: body)
     }
 
-    static var ok: Self { .init(status: .ok) }
-    static var notFound: Self { .init(status: .notFound) }
     static var badRequest: Self { .init(status: .badRequest) }
+    static var notFound: Self { .init(status: .notFound) }
+    static var tooManyRequests: Self { .init(status: .tooManyRequests) }
+    static var ok: Self { .init(status: .ok) }
 }
 #endif

Sources/App/Core/ErrorHandlingHelpers.swift

@@ -37,3 +37,15 @@ func run<T, E1: Error, E2: Error>(_ operation: () async throws(E1) -> T,
         throw transform(error)
     }
 }
+
+
+@discardableResult
+func run<T, E1: Error, E2: Error>(_ operation: () throws(E1) -> T,
+                                  rethrowing transform: (E1) -> E2) throws(E2) -> T {
+    do {
+        let result = try operation()
+        return result
+    } catch {
+        throw transform(error)
+    }
+}

Sources/App/Core/Github.swift

@@ -21,11 +21,12 @@ import Vapor
 enum Github {
 
     enum Error: Swift.Error {
-        case decodeContentFailed(URI, Swift.Error)
+        case decodeContentFailed(_ url: String, Swift.Error)
+        case encodeContentFailed(_ url: String, Swift.Error)
         case missingToken
         case noBody
         case invalidURL(String)
-        case postRequestFailed(URI, Swift.Error)
+        case postRequestFailed(_ url: String, Swift.Error)
         case requestFailed(HTTPStatus)
     }
 
@@ -108,7 +109,7 @@ extension Github {
                 return "https://api.github.com/repos/\(owner)/\(repository)/\(resource.rawValue)"
         }
     }
-    
+
     @available(*, deprecated)
     static func fetch(client: Client, uri: URI, headers: [(String, String)] = []) async throws -> (content: String, etag: String?) {
         guard let token = Current.githubToken() else {
@@ -211,12 +212,15 @@ extension Github {
 
 extension Github {
 
+    @available(*, deprecated)
     static let graphQLApiUri = URI(string: "https://api.github.com/graphql")
+    static let graphQLApiURL = "https://api.github.com/graphql"
 
     struct GraphQLQuery: Content {
         var query: String
     }
 
+    @available(*, deprecated)
     static func fetchResource<T: Decodable>(_ type: T.Type, client: Client, query: GraphQLQuery) async throws(Github.Error) -> T {
         guard let token = Current.githubToken() else {
             throw Error.missingToken
@@ -228,7 +232,7 @@ extension Github {
                 try $0.content.encode(query)
             }
         } catch {
-            throw .postRequestFailed(Self.graphQLApiUri, error)
+            throw .postRequestFailed(graphQLApiUri.string, error)
         }
 
         guard !isRateLimited(response) else {
@@ -244,25 +248,57 @@ extension Github {
         do {
             return try response.content.decode(T.self, using: decoder)
         } catch {
-            throw .decodeContentFailed(Self.graphQLApiUri, error)
+            throw .decodeContentFailed(graphQLApiUri.string, error)
+        }
+    }
+
+    static func fetchResource<T: Decodable>(_ type: T.Type, query: GraphQLQuery) async throws(Github.Error) -> T {
+        guard let token = Current.githubToken() else {
+            throw Error.missingToken
+        }
+
+        @Dependency(\.httpClient) var httpClient
+
+        let body = try run {
+            try JSONEncoder().encode(query)
+        } rethrowing: {
+            Error.encodeContentFailed(graphQLApiURL, $0)
+        }
+
+        let response = try await run {
+            try await httpClient.post(url: graphQLApiURL, headers: defaultHeaders(with: token), body: body)
+        } rethrowing: {
+            Error.postRequestFailed(graphQLApiURL, $0)
+        }
+
+        guard !isRateLimited(response) else {
+            Current.logger().critical("rate limited while fetching resource \(T.self)")
+            throw Error.requestFailed(.tooManyRequests)
+        }
+
+        guard response.status == .ok else {
+            Current.logger().warning("fetchResource<\(T.self)> request failed with status \(response.status)")
+            throw Error.requestFailed(response.status)
+        }
+
+        guard let body = response.body else { throw Github.Error.noBody }
+
+        return try run {
+            try decoder.decode(T.self, from: body)
+        } rethrowing: {
+            Error.decodeContentFailed(graphQLApiURL, $0)
         }
     }
 
-    static func fetchMetadata(client: Client, owner: String, repository: String) async throws(Github.Error) -> Metadata {
+    static func fetchMetadata(owner: String, repository: String) async throws(Github.Error) -> Metadata {
         struct Response<T: Decodable & Equatable>: Decodable, Equatable {
             var data: T
         }
         return try await fetchResource(Response<Metadata>.self,
-                                       client: client,
                                        query: Metadata.query(owner: owner, repository: repository))
         .data
     }
 
-    static func fetchMetadata(client: Client, packageUrl: String) async throws -> Metadata {
-        let (owner, name) = try parseOwnerName(url: packageUrl)
-        return try await fetchMetadata(client: client, owner: owner, repository: name)
-    }
-
 }
 
 

Tests/AppTests/ErrorReportingTests.swift

@@ -34,10 +34,10 @@ class ErrorReportingTests: AppTestCase {
     func test_Ingestion_error_reporting() async throws {
         // setup
         try await Package(id: .id0, url: "1", processingStage: .reconciliation).save(on: app.db)
-        Current.fetchMetadata = { _, _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") }
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchMetadata = { @Sendable _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10))