Merge pull request #3572 from SwiftPackageIndex/issue-3469-dependency-transition-13

Issue 3469 dependency transition 13

Author: finestructure

Sources/App/Commands/Ingestion.swift

@@ -258,12 +258,13 @@ 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 Current.fetchLicense(client, owner, repository)
+        async let license = await github.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 fetchLicense: @Sendable (_ client: Client, _ owner: String, _ repository: String) async -> Github.License?
     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
@@ -66,7 +65,6 @@ extension AppEnvironment {
     nonisolated(unsafe) static var logger: Logger!
 
     static let live = AppEnvironment(
-        fetchLicense: { client, owner, repo in await Github.fetchLicense(client:client, owner: owner, repository: repo) },
         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) },

Sources/App/Core/Dependencies/GithubClient.swift

@@ -0,0 +1,45 @@
+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+
+import Dependencies
+import DependenciesMacros
+
+
+@DependencyClient
+struct GithubClient {
+    var fetchLicense: @Sendable (_ owner: String, _ repository: String) async -> Github.License?
+}
+
+
+extension GithubClient: DependencyKey {
+    static var liveValue: Self {
+        .init(
+            fetchLicense: { owner, repo in await Github.fetchLicense(owner: owner, repository: repo) }
+        )
+    }
+}
+
+
+extension GithubClient: TestDependencyKey {
+    static var testValue: Self { Self() }
+}
+
+
+extension DependencyValues {
+    var github: GithubClient {
+        get { self[GithubClient.self] }
+        set { self[GithubClient.self] = newValue }
+    }
+}

Sources/App/Core/Dependencies/HTTPClient.swift

@@ -23,7 +23,9 @@ struct HTTPClient {
     typealias Request = Vapor.HTTPClient.Request
     typealias Response = Vapor.HTTPClient.Response
 
+    var get: @Sendable (_ url: String, _ headers: HTTPHeaders) async throws -> Response
     var post: @Sendable (_ url: String, _ headers: HTTPHeaders, _ body: Data?) async throws -> Response
+
     var fetchDocumentation: @Sendable (_ url: URI) async throws -> Response
     var fetchHTTPStatusCode: @Sendable (_ url: String) async throws -> HTTPStatus
     var mastodonPost: @Sendable (_ message: String) async throws -> Void
@@ -33,6 +35,10 @@ struct HTTPClient {
 extension HTTPClient: DependencyKey {
     static var liveValue: HTTPClient {
         .init(
+            get: { url, headers in
+                let req = try Request(url: url, method: .GET, headers: headers)
+                return try await Vapor.HTTPClient.shared.execute(request: req).get()
+            },
             post: { url, headers, body in
                 let req = try Request(url: url, method: .POST, headers: headers, body: body.map({.data($0)}))
                 return try await Vapor.HTTPClient.shared.execute(request: req).get()

Sources/App/Core/Github.swift

@@ -12,9 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import Vapor
-import SwiftSoup
+import Dependencies
 import S3Store
+import SwiftSoup
+import Vapor
 
 
 enum Github {
@@ -35,6 +36,7 @@ enum Github {
         return decoder
     }
 
+    @available(*, deprecated)
     static func rateLimit(response: ClientResponse) -> Int? {
         guard
             let header = response.headers.first(name: "X-RateLimit-Remaining"),
@@ -43,12 +45,27 @@ enum Github {
         return limit
     }
 
+    static func rateLimit(response: HTTPClient.Response) -> Int? {
+        guard
+            let header = response.headers.first(name: "X-RateLimit-Remaining"),
+            let limit = Int(header)
+        else { return nil }
+        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)
+        return response.status == .forbidden && limit == 0
+    }
+
     static func parseOwnerName(url: String) throws(Github.Error) -> (owner: String, name: String) {
         let parts = url
             .droppingGithubComPrefix
@@ -77,13 +94,22 @@ 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?) {
         guard let token = Current.githubToken() else {
             throw Error.missingToken
@@ -109,6 +135,7 @@ 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
@@ -128,9 +155,29 @@ extension Github {
         return try response.content.decode(T.self, using: decoder)
     }
 
-    static func fetchLicense(client: Client, owner: String, repository: String) async -> License? {
-        let uri = Github.apiUri(owner: owner, repository: repository, resource: .license)
-        return try? await Github.fetchResource(Github.License.self, client: client, uri: uri)
+    static func fetchResource<T: Decodable>(_ type: T.Type, url: String) async throws -> T {
+        guard let token = Current.githubToken() else {
+            throw Error.missingToken
+        }
+
+        @Dependency(\.httpClient) var httpClient
+
+        let response = try await httpClient.get(url: url, headers: defaultHeaders(with: token))
+
+        guard !isRateLimited(response) else {
+            Current.logger().critical("rate limited while fetching resource \(url)")
+            throw Error.requestFailed(.tooManyRequests)
+        }
+
+        guard response.status == .ok else { throw Error.requestFailed(response.status) }
+        guard let body = response.body else { throw Github.Error.noBody }
+
+        return try decoder.decode(T.self, from: body)
+    }
+
+    static func fetchLicense(owner: String, repository: String) async -> License? {
+        let url = Github.apiURL(owner: owner, repository: repository, resource: .license)
+        return try? await Github.fetchResource(Github.License.self, url: url)
     }
 
     static func fetchReadme(client: Client, owner: String, repository: String) async -> Readme? {
@@ -466,7 +513,7 @@ extension Github {
                 }
             }
         }
-        
+
         struct Parent: Decodable, Equatable {
             var url: String?
         }

Tests/AppTests/GithubTests.swift

@@ -261,28 +261,28 @@ class GithubTests: AppTestCase {
 
     func test_isRateLimited() throws {
         do {
-            let res = ClientResponse(status: .forbidden,
-                                     headers: .init([("X-RateLimit-Remaining", "0")]))
+            let res = HTTPClient.Response(status: .forbidden,
+                                          headers: .init([("X-RateLimit-Remaining", "0")]))
             XCTAssertTrue(Github.isRateLimited(res))
         }
         do {
-            let res = ClientResponse(status: .forbidden,
-                                     headers: .init([("x-ratelimit-remaining", "0")]))
+            let res = HTTPClient.Response(status: .forbidden,
+                                          headers: .init([("x-ratelimit-remaining", "0")]))
             XCTAssertTrue(Github.isRateLimited(res))
         }
         do {
-            let res = ClientResponse(status: .forbidden,
-                                     headers: .init([("X-RateLimit-Remaining", "1")]))
+            let res = HTTPClient.Response(status: .forbidden,
+                                          headers: .init([("X-RateLimit-Remaining", "1")]))
             XCTAssertFalse(Github.isRateLimited(res))
         }
         do {
-            let res = ClientResponse(status: .forbidden,
-                                     headers: .init([("unrelated", "0")]))
+            let res = HTTPClient.Response(status: .forbidden,
+                                          headers: .init([("unrelated", "0")]))
             XCTAssertFalse(Github.isRateLimited(res))
         }
         do {
-            let res = ClientResponse(status: .ok,
-                                     headers: .init([("X-RateLimit-Remaining", "0")]))
+            let res = HTTPClient.Response(status: .ok,
+                                          headers: .init([("X-RateLimit-Remaining", "0")]))
             XCTAssertFalse(Github.isRateLimited(res))
         }
     }
@@ -318,40 +318,44 @@ class GithubTests: AppTestCase {
         }
     }
 
-    func test_apiUri() throws {
-        XCTAssertEqual(Github.apiUri(owner: "foo", repository: "bar", resource: .license).string,
+    func test_apiURL() throws {
+        XCTAssertEqual(Github.apiURL(owner: "foo", repository: "bar", resource: .license),
                        "https://api.github.com/repos/foo/bar/license")
-        XCTAssertEqual(Github.apiUri(owner: "foo", repository: "bar", resource: .readme).string,
+        XCTAssertEqual(Github.apiURL(owner: "foo", repository: "bar", resource: .readme),
                        "https://api.github.com/repos/foo/bar/readme")
     }
 
     func test_fetchLicense() async throws {
         // setup
         Current.githubToken = { "secr3t" }
-        let data = try XCTUnwrap(try fixtureData(for: "github-license-response.json"))
-        let client = MockClient { _, resp in
-            resp.status = .ok
-            resp.body = makeBody(data)
-        }
 
-        // MUT
-        let res = await Github.fetchLicense(client: client, owner: "PSPDFKit", repository: "PSPDFKit-SP")
+        await withDependencies {
+            $0.httpClient.get = { @Sendable _, _ in
+                try .init(status: .ok, body: .fixture(named: "github-license-response.json"))
+            }
+        } operation: {
+            // MUT
+            let res = await Github.fetchLicense(owner: "PSPDFKit", repository: "PSPDFKit-SP")
 
-        // validate
-        XCTAssertEqual(res?.htmlUrl, "https://github.com/PSPDFKit/PSPDFKit-SP/blob/master/LICENSE")
+            // validate
+            XCTAssertEqual(res?.htmlUrl, "https://github.com/PSPDFKit/PSPDFKit-SP/blob/master/LICENSE")
+        }
     }
 
     func test_fetchLicense_notFound() async throws {
         // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/761
         // setup
         Current.githubToken = { "secr3t" }
-        let client = MockClient { _, resp in resp.status = .notFound }
 
-        // MUT
-        let res = await Github.fetchLicense(client: client, owner: "foo", repository: "bar")
+        await withDependencies {
+            $0.httpClient.get = { @Sendable _, _ in .notFound }
+        } operation: {
+            // MUT
+            let res = await Github.fetchLicense(owner: "foo", repository: "bar")
 
-        // validate
-        XCTAssertEqual(res, nil)
+            // validate
+            XCTAssertEqual(res, nil)
+        }
     }
 
     func test_fetchReadme() async throws {
@@ -488,3 +492,10 @@ class GithubTests: AppTestCase {
     }
 
 }
+
+
+private extension ByteBuffer {
+    static func fixture(named filename: String) throws -> Self {
+        .init(data: try fixtureData(for: filename))
+    }
+}