Merge pull request #3587 from SwiftPackageIndex/issue-3469-dependency-transition-15

Issue 3469 dependency transition 15

Author: finestructure

Sources/App/Commands/Ingestion.swift

@@ -267,10 +267,11 @@ enum Ingestion {
         //   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
+        @Dependency(\.github.fetchReadme) var fetchReadme
 
         async let metadata = try await fetchMetadata(owner, repository)
         async let license = await fetchLicense(owner, repository)
-        async let readme = await Current.fetchReadme(client, owner, repository)
+        async let readme = await fetchReadme(owner, repository)
 
         do {
             return try await (metadata, license, readme)

Sources/App/Core/AppEnvironment.swift

@@ -23,7 +23,6 @@ import FoundationNetworking
 
 
 struct AppEnvironment: Sendable {
-    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
     var getStatusCount: @Sendable (_ client: Client, _ status: Gitlab.Builder.Status) async throws -> Int
@@ -64,7 +63,6 @@ extension AppEnvironment {
     nonisolated(unsafe) static var logger: Logger!
 
     static let live = AppEnvironment(
-        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,
         getStatusCount: { client, status in

Sources/App/Core/Dependencies/GithubClient.swift

@@ -23,14 +23,16 @@ import IssueReporting
 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() }
+    var fetchReadme: @Sendable (_ owner: String, _ repository: String) async -> Github.Readme?
 }
 
 
 extension GithubClient: DependencyKey {
     static var liveValue: Self {
         .init(
             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) }
+            fetchMetadata: { owner, repo throws(Github.Error) in try await Github.fetchMetadata(owner: owner, repository: repo) },
+            fetchReadme: { owner, repo in await Github.fetchReadme(owner: owner, repository: repo) }
         )
     }
 }
@@ -40,7 +42,8 @@ extension GithubClient: TestDependencyKey {
     static var testValue: Self {
         .init(
             fetchLicense: { _, _ in unimplemented("fetchLicense"); return nil },
-            fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() }
+            fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() },
+            fetchReadme: { _, _ in unimplemented("fetchReadme"); return nil }
         )
     }
 }

Sources/App/Core/Github.swift

@@ -37,15 +37,6 @@ enum Github {
         return decoder
     }
 
-    @available(*, deprecated)
-    static func rateLimit(response: ClientResponse) -> Int? {
-        guard
-            let header = response.headers.first(name: "X-RateLimit-Remaining"),
-            let limit = Int(header)
-        else { return nil }
-        return limit
-    }
-
     static func rateLimit(response: HTTPClient.Response) -> Int? {
         guard
             let header = response.headers.first(name: "X-RateLimit-Remaining"),
@@ -54,13 +45,6 @@ enum Github {
         return limit
     }
 
-    @available(*, deprecated)
-    static func isRateLimited(_ response: ClientResponse) -> Bool {
-        guard let limit = rateLimit(response: response) else { return false }
-        AppMetrics.githubRateLimitRemainingCount?.set(limit)
-        return response.status == .forbidden && limit == 0
-    }
-
     static func isRateLimited(_ response: HTTPClient.Response) -> Bool {
         guard let limit = rateLimit(response: response) else { return false }
         AppMetrics.githubRateLimitRemainingCount?.set(limit)
@@ -95,36 +79,29 @@ extension Github {
         case readme
     }
 
-    @available(*, deprecated)
-    static func apiUri(owner: String, repository: String, resource: Resource)  -> URI {
-        switch resource {
-            case .license, .readme:
-                return URI(string: "https://api.github.com/repos/\(owner)/\(repository)/\(resource.rawValue)")
-        }
-    }
-
     static func apiURL(owner: String, repository: String, resource: Resource)  -> String {
         switch resource {
             case .license, .readme:
                 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?) {
+    static func fetch(url: String, headers: [(String, String)] = []) async throws -> (content: String, etag: String?) {
         guard let token = Current.githubToken() else {
             throw Error.missingToken
         }
 
-        let response = try await client.get(uri, headers: defaultHeaders(with: token).adding(contentsOf: headers))
+        @Dependency(\.httpClient) var httpClient
+
+        let response = try await httpClient.get(url: url, headers: defaultHeaders(with: token).adding(contentsOf: headers))
 
         guard !isRateLimited(response) else {
-            Current.logger().critical("rate limited while fetching uri \(uri)")
+            Current.logger().critical("rate limited while fetching \(url)")
             throw Error.requestFailed(.tooManyRequests)
         }
 
         guard response.status == .ok else {
-            Current.logger().warning("Github.fetch of '\(uri.path)' failed with status \(response.status)")
+            Current.logger().warning("Github.fetch of '\(url)' failed with status \(response.status)")
             throw Error.requestFailed(response.status)
         }
 
@@ -136,26 +113,6 @@ extension Github {
         return (body.asString(), response.headers.first(name: .eTag))
     }
 
-    @available(*, deprecated)
-    static func fetchResource<T: Decodable>(_ type: T.Type, client: Client, uri: URI) async throws -> T {
-        guard let token = Current.githubToken() else {
-            throw Error.missingToken
-        }
-
-        let response = try await client.get(uri, headers: defaultHeaders(with: token))
-
-        guard !isRateLimited(response) else {
-            Current.logger().critical("rate limited while fetching resource \(uri)")
-            throw Error.requestFailed(.tooManyRequests)
-        }
-
-        guard response.status == .ok else {
-            throw Error.requestFailed(response.status)
-        }
-
-        return try response.content.decode(T.self, using: decoder)
-    }
-
     static func fetchResource<T: Decodable>(_ type: T.Type, url: String) async throws -> T {
         guard let token = Current.githubToken() else {
             throw Error.missingToken
@@ -181,11 +138,11 @@ extension Github {
         return try? await Github.fetchResource(Github.License.self, url: url)
     }
 
-    static func fetchReadme(client: Client, owner: String, repository: String) async -> Readme? {
-        let uri = Github.apiUri(owner: owner, repository: repository, resource: .readme)
+    static func fetchReadme(owner: String, repository: String) async -> Readme? {
+        let url = Github.apiURL(owner: owner, repository: repository, resource: .readme)
 
         // Fetch readme html content
-        let readme = try? await Github.fetch(client: client, uri: uri, headers: [
+        let readme = try? await Github.fetch(url: url, headers: [
             ("Accept", "application/vnd.github.html+json")
         ])
         guard var html = readme?.content else { return nil }
@@ -195,7 +152,7 @@ extension Github {
             struct Response: Decodable {
                 var htmlUrl: String
             }
-            return try? await Github.fetchResource(Response.self, client: client, uri: uri).htmlUrl
+            return try? await Github.fetchResource(Response.self, url: url).htmlUrl
         }()
         guard let htmlUrl else { return nil }
 
@@ -212,46 +169,12 @@ 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
-        }
-
-        let response: ClientResponse
-        do {
-            response = try await client.post(Self.graphQLApiUri, headers: defaultHeaders(with: token)) {
-                try $0.content.encode(query)
-            }
-        } catch {
-            throw .postRequestFailed(graphQLApiUri.string, error)
-        }
-
-        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)
-        }
-
-        do {
-            return try response.content.decode(T.self, using: decoder)
-        } catch {
-            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

Tests/AppTests/GithubTests.swift

@@ -349,48 +349,52 @@ class GithubTests: AppTestCase {
         // setup
         Current.githubToken = { "secr3t" }
         let requestCount = QueueIsolated(0)
-        let client = MockClient { req, resp in
-            requestCount.increment()
-            switch req.headers[.accept] {
-                case ["application/vnd.github.html+json"]:
-                    resp.status = .ok
-                    resp.body = makeBody("readme html")
-                    resp.headers.add(name: .eTag, value: "etag")
-                case []:
-                    resp.status = .ok
-                    struct Response: Encodable {
-                        var htmlUrl: String
-                    }
-                    resp.body = makeBody(try! JSONEncoder().encode(Response(htmlUrl: "readme url")))
-                default:
-                    XCTFail("unexpected accept header")
+        await withDependencies {
+            $0.httpClient.get = { @Sendable _, headers in
+                requestCount.increment()
+                switch headers[.accept] {
+                    case ["application/vnd.github.html+json"]:
+                        return .ok(body: "readme html", headers: ["ETag": "etag"])
+                    case []:
+                        struct Response: Encodable {
+                            var htmlUrl: String
+                        }
+                        return try .ok(jsonEncode: Response(htmlUrl: "readme url"))
+                    default:
+                        XCTFail("unexpected accept header")
+                }
+                enum Error: Swift.Error { case unexpectedCodePath }
+                throw Error.unexpectedCodePath
             }
-        }
+        } operation: {
+            // MUT
+            let res = await Github.fetchReadme(owner: "foo", repository: "bar")
 
-        // MUT
-        let res = await Github.fetchReadme(client: client, owner: "foo", repository: "bar")
-
-        // validate
-        XCTAssertEqual(requestCount.value, 2)
-        XCTAssertEqual(
-            res,
-            .init(etag: "etag",
-                  html: "readme html",
-                  htmlUrl: "readme url",
-                  imagesToCache: [])
-        )
+            // validate
+            XCTAssertEqual(requestCount.value, 2)
+            XCTAssertEqual(
+                res,
+                .init(etag: "etag",
+                      html: "readme html",
+                      htmlUrl: "readme url",
+                      imagesToCache: [])
+            )
+        }
     }
 
     func test_fetchReadme_notFound() async throws {
         // setup
         Current.githubToken = { "secr3t" }
-        let client = MockClient { _, resp in resp.status = .notFound }
 
-        // MUT
-        let res = await Github.fetchReadme(client: client, owner: "foo", repository: "bar")
+        await withDependencies {
+            $0.httpClient.get = { @Sendable _, headers in .notFound }
+        } operation: {
+            // MUT
+            let res = await Github.fetchReadme(owner: "foo", repository: "bar")
 
-        // validate
-        XCTAssertEqual(res, nil)
+            // validate
+            XCTAssertEqual(res, nil)
+        }
     }
 
     func test_extractImagesRequiringCaching() async throws {
@@ -482,11 +486,16 @@ class GithubTests: AppTestCase {
 
 
 private extension HTTPClient.Response {
-    static func ok(body: String) -> Self {
-        .init(status: .ok, body: .init(string: body))
+    static func ok(body: String, headers: HTTPHeaders = .init()) -> Self {
+        .init(status: .ok, headers: headers, body: .init(string: body))
     }
 
     static func ok(fixture: String) throws -> Self {
         try .init(status: .ok, body: .init(data: fixtureData(for: fixture)))
     }
+
+    static func ok<T: Encodable>(jsonEncode value: T) throws -> Self {
+        let data = try JSONEncoder().encode(value)
+        return .init(status: .ok, body: .init(data: data))
+    }
 }