Issue 3469 dependency transition 13

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 @@
         return decoder
     }
 
+    @available(*, deprecated)
     static func rateLimit(response: ClientResponse) -> Int? {
         guard
             let header = response.headers.first(name: "X-RateLimit-Remaining"),
@@ -43,12 +45,27 @@
         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 @@
         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 @@
         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,16 +155,42 @@
         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
+        }
+
+        @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)
+    }
+
+    @available(*, deprecated)
     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 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? {
         let uri = Github.apiUri(owner: owner, repository: repository, resource: .readme)
 
         // Fetch readme html content
         let readme = try? await Github.fetch(client: client, uri: uri, headers: [
             ("Accept", "application/vnd.github.html+json")
         ])
         guard var html = readme?.content else { return nil }
@@ -147,7 +200,7 @@
             struct Response: Decodable {
                 var htmlUrl: String
             }
             return try? await Github.fetchResource(Response.self, client: client, uri: uri).htmlUrl
         }()
         guard let htmlUrl else { return nil }
 
@@ -466,7 +519,7 @@
                 }
             }
         }
-        
+
         struct Parent: Decodable, Equatable {
             var url: String?
         }

Tests/AppTests/IngestionTests.swift

@@ -36,6 +36,7 @@ class IngestionTests: AppTestCase {
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10))
@@ -64,30 +65,33 @@ class IngestionTests: AppTestCase {
 
     func test_ingest_continue_on_error() async throws {
         // Test completion of ingestion despite early error
-        // setup
-        let packages = try await savePackages(on: app.db, ["https://github.com/foo/1",
-                                                           "https://github.com/foo/2"], processingStage: .reconciliation)
-            .map(Joined<Package, Repository>.init(model:))
-        Current.fetchMetadata = { _, owner, repository throws(Github.Error) in
-            if owner == "foo" && repository == "1" {
-                throw Github.Error.requestFailed(.badRequest)
+        try await withDependencies {
+            $0.github.fetchLicense = { @Sendable _, _ in Github.License(htmlUrl: "license") }
+        } operation: {
+            // setup
+            let packages = try await savePackages(on: app.db, ["https://github.com/foo/1",
+                                                               "https://github.com/foo/2"], processingStage: .reconciliation)
+                .map(Joined<Package, Repository>.init(model:))
+            Current.fetchMetadata = { _, owner, repository throws(Github.Error) in
+                if owner == "foo" && repository == "1" {
+                    throw Github.Error.requestFailed(.badRequest)
+                }
+                return .mock(owner: owner, repository: repository)
             }
-            return .mock(owner: owner, repository: repository)
-        }
-        Current.fetchLicense = { _, _, _ in Github.License(htmlUrl: "license") }
 
-        // MUT
-        await Ingestion.ingest(client: app.client, database: app.db, packages: packages)
-
-        do {
-            // validate the second package's license is updated
-            let repo = try await Repository.query(on: app.db)
-                .filter(\.$name == "2")
-                .first()
-                .unwrap()
-            XCTAssertEqual(repo.licenseUrl, "license")
-            for pkg in try await Package.query(on: app.db).all() {
-                XCTAssertEqual(pkg.processingStage, .ingestion, "\(pkg.url) must be in ingestion")
+            // MUT
+            await Ingestion.ingest(client: app.client, database: app.db, packages: packages)
+
+            do {
+                // validate the second package's license is updated
+                let repo = try await Repository.query(on: app.db)
+                    .filter(\.$name == "2")
+                    .first()
+                    .unwrap()
+                XCTAssertEqual(repo.licenseUrl, "license")
+                for pkg in try await Package.query(on: app.db).all() {
+                    XCTAssertEqual(pkg.processingStage, .ingestion, "\(pkg.url) must be in ingestion")
+                }
             }
         }
     }
@@ -308,6 +312,7 @@ class IngestionTests: AppTestCase {
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(testUrls.count))
@@ -335,6 +340,7 @@ class IngestionTests: AppTestCase {
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10))
@@ -387,6 +393,7 @@ class IngestionTests: AppTestCase {
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10))
@@ -454,6 +461,7 @@ class IngestionTests: AppTestCase {
     func test_ingest_storeS3Readme() async throws {
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // setup
             let app = self.app!
@@ -571,6 +579,7 @@ class IngestionTests: AppTestCase {
 
         try await withDependencies {
             $0.date.now = .now
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(1))
@@ -601,6 +610,7 @@ class IngestionTests: AppTestCase {
         do { // first ingestion, no readme has been saved
             try await withDependencies {
                 $0.date.now = .now
+                $0.github.fetchLicense = { @Sendable _, _ in nil }
             } operation: {
                 // MUT
                 let app = self.app!
@@ -619,22 +629,25 @@ class IngestionTests: AppTestCase {
 
     func test_issue_761_no_license() async throws {
         // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/761
-        // setup
-        let pkg = Package(url: "https://github.com/foo/1")
-        try await pkg.save(on: app.db)
-        // use mock for metadata request which we're not interested in ...
-        Current.fetchMetadata = { _, _, _ in Github.Metadata() }
-        // and live fetch request for fetchLicense, whose behaviour we want to test ...
-        Current.fetchLicense = { client, owner, repo in await Github.fetchLicense(client: client, owner: owner, repository: repo) }
-        // and simulate its underlying request returning a 404 (by making all requests
-        // return a 404, but it's the only one we're sending)
-        let client = MockClient { _, resp in resp.status = .notFound }
+        try await withDependencies {
+            // use live fetch request for fetchLicense, whose behaviour we want to test ...
+            $0.github.fetchLicense = { @Sendable owner, repo in await Github.fetchLicense(owner: owner, repository: repo) }
+        } operation: {
+            // setup
+            let pkg = Package(url: "https://github.com/foo/1")
+            try await pkg.save(on: app.db)
+            // use mock for metadata request which we're not interested in ...
+            Current.fetchMetadata = { _, _, _ in Github.Metadata() }
+            // and simulate its underlying request returning a 404 (by making all requests
+            // return a 404, but it's the only one we're sending)
+            let client = MockClient { _, resp in resp.status = .notFound }
 
-        // MUT
-        let (_, license, _) = try await Ingestion.fetchMetadata(client: client, package: pkg, owner: "foo", repository: "1")
+            // MUT
+            let (_, license, _) = try await Ingestion.fetchMetadata(client: client, package: pkg, owner: "foo", repository: "1")
 
-        // validate
-        XCTAssertEqual(license, nil)
+            // validate
+            XCTAssertEqual(license, nil)
+        }
     }
 
     func test_migration076_updateRepositoryResetReadmes() async throws {

Tests/AppTests/MastodonTests.swift

@@ -25,6 +25,7 @@ final class MastodonTests: AppTestCase {
         let message = QueueIsolated<String?>(nil)
         try await withDependencies {
             $0.environment.allowSocialPosts = { true }
+            $0.github.fetchLicense = { @Sendable _, _ in nil }
             $0.httpClient.mastodonPost = { @Sendable msg in
                 if message.value == nil {
                     message.setValue(msg)

Tests/AppTests/Mocks/AppEnvironment+mock.swift

@@ -22,7 +22,6 @@ import Vapor
 extension AppEnvironment {
     static func mock(eventLoop: EventLoop) -> Self {
         .init(
-            fetchLicense: { _, _, _ in .init(htmlUrl: "https://github.com/foo/bar/blob/main/LICENSE") },
             fetchMetadata: { _, _, _ in .mock },
             fetchReadme: { _,  _, _ in .init(html: "readme html", htmlUrl: "readme html url", imagesToCache: []) },
             fetchS3Readme: { _, _, _ in "" },