Move fetchHTTPStatusCode to HTTPClient

SwiftPackageIndex · Dec 20, 2024 · 64defd4 · 64defd4
1 parent cce82f8
commit 64defd4
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 62 deletions.
diff --git a/Sources/App/Controllers/PackageController+routes.swift b/Sources/App/Controllers/PackageController+routes.swift
@@ -407,10 +407,11 @@ extension PackageController {
                     .query(on: db, owner: owner, repository: repository)
                 self = .packageAvailable(model, schema)
             } catch let error as AbortError where error.status == .notFound {
+                @Dependency(\.httpClient) var httpClient
                 // When the package is not in the index, we check if it's available on GitHub.
                 // We use try? to avoid raising internel server errors from exceptions raised
                 // from this call.
-                let status = try? await Current.fetchHTTPStatusCode("https://github.com/\(owner)/\(repository)")
+                let status = try? await httpClient.fetchHTTPStatusCode("https://github.com/\(owner)/\(repository)")
                 switch status {
                     case .some(.notFound):
                         // GitHub does not have the package

diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift
@@ -23,7 +23,6 @@ import FoundationNetworking
 
 
 struct AppEnvironment: Sendable {
-    var fetchHTTPStatusCode: @Sendable (_ url: String) async throws -> HTTPStatus
     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?
@@ -84,7 +83,6 @@ extension AppEnvironment {
     nonisolated(unsafe) static var logger: Logger!
 
     static let live = AppEnvironment(
-        fetchHTTPStatusCode: { url in try await Networking.fetchHTTPStatusCode(url) },
         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) },
@@ -152,24 +150,6 @@ extension AppEnvironment {
 }
 
 
-private enum Networking {
-    static func fetchHTTPStatusCode(_ url: String) async throws -> HTTPStatus {
-        var config = Vapor.HTTPClient.Configuration()
-        // We're forcing HTTP/1 due to a bug in Github's HEAD request handling
-        // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/1676
-        config.httpVersion = .http1Only
-        let client = Vapor.HTTPClient(eventLoopGroupProvider: .singleton, configuration: config)
-        return try await run {
-            var req = HTTPClientRequest(url: url)
-            req.method = .HEAD
-            return try await client.execute(req, timeout: .seconds(2)).status
-        } defer: {
-            try await client.shutdown()
-        }
-    }
-}
-
-
 struct FileManager: Sendable {
     var attributesOfItem: @Sendable (_ path: String) throws -> [FileAttributeKey : Any]
     var contentsOfDirectory: @Sendable (_ path: String) throws -> [String]

diff --git a/Sources/App/Core/Dependencies/HTTPClient.swift b/Sources/App/Core/Dependencies/HTTPClient.swift
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import AsyncHTTPClient
 import Dependencies
 import DependenciesMacros
 import Vapor
@@ -21,14 +22,29 @@ import Vapor
 struct HTTPClient {
     typealias Response = Vapor.HTTPClient.Response
 
-    var fetchDocumentation: @Sendable (_ url: URI) async throws -> Response
+    var fetchDocumentation: @Sendable (_ url: URI) async throws -> Response = { _ in XCTFail("fetchDocumentation"); return .ok }
+    var fetchHTTPStatusCode: @Sendable (_ url: String) async throws -> HTTPStatus = { _ in XCTFail("fetchHTTPStatusCode"); return .ok }
 }
 
 extension HTTPClient: DependencyKey {
     static var liveValue: HTTPClient {
         .init(
             fetchDocumentation: { url in
                 try await Vapor.HTTPClient.shared.get(url: url.string).get()
+            },
+            fetchHTTPStatusCode: { url in
+                var config = Vapor.HTTPClient.Configuration()
+                // We're forcing HTTP/1 due to a bug in Github's HEAD request handling
+                // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/1676
+                config.httpVersion = .http1Only
+                let client = Vapor.HTTPClient(eventLoopGroupProvider: .singleton, configuration: config)
+                return try await run {
+                    var req = HTTPClientRequest(url: url)
+                    req.method = .HEAD
+                    return try await client.execute(req, timeout: .seconds(2)).status
+                } defer: {
+                    try await client.shutdown()
+                }
             }
         )
     }

diff --git a/Tests/AppTests/Mocks/AppEnvironment+mock.swift b/Tests/AppTests/Mocks/AppEnvironment+mock.swift
@@ -22,7 +22,6 @@ import Vapor
 extension AppEnvironment {
     static func mock(eventLoop: EventLoop) -> Self {
         .init(
-            fetchHTTPStatusCode: { _ in .ok },
             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: []) },

diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift
@@ -44,9 +44,8 @@ class PackageController_routesTests: SnapshotTestCase {
     func test_show_checkingGitHubRepository_notFound() throws {
         try withDependencies {
             $0.environment.dbId = { nil }
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in .notFound }
         } operation: {
-            Current.fetchHTTPStatusCode = { _ in .notFound }
-
             // MUT
             try app.test(.GET, "/unknown/package") {
                 XCTAssertEqual($0.status, .notFound)
@@ -57,9 +56,8 @@ class PackageController_routesTests: SnapshotTestCase {
     func test_show_checkingGitHubRepository_found() throws {
         try withDependencies {
             $0.environment.dbId = { nil }
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in .ok }
         } operation: {
-            Current.fetchHTTPStatusCode = { _ in .ok }
-
             // MUT
             try app.test(.GET, "/unknown/package") {
                 XCTAssertEqual($0.status, .notFound)
@@ -72,9 +70,8 @@ class PackageController_routesTests: SnapshotTestCase {
         // fetchHTTPStatusCode fails
         try withDependencies {
             $0.environment.dbId = { nil }
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in throw FetchError() }
         } operation: {
-            Current.fetchHTTPStatusCode = { _ in throw FetchError() }
-
             // MUT
             try app.test(.GET, "/unknown/package") {
                 XCTAssertEqual($0.status, .notFound)
@@ -103,50 +100,53 @@ class PackageController_routesTests: SnapshotTestCase {
     }
 
     func test_ShowModel_packageMissing() async throws {
-        // setup
-        Current.fetchHTTPStatusCode = { _ in .ok }
-
-        // MUT
-        let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
+        try await withDependencies {
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in .ok }
+        } operation: {
+            // MUT
+            let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
 
-        // validate
-        switch model {
-            case .packageAvailable, .packageDoesNotExist:
-                XCTFail("expected package to be missing")
-            case .packageMissing:
-                break
+            // validate
+            switch model {
+                case .packageAvailable, .packageDoesNotExist:
+                    XCTFail("expected package to be missing")
+                case .packageMissing:
+                    break
+            }
         }
     }
 
     func test_ShowModel_packageDoesNotExist() async throws {
-        // setup
-        Current.fetchHTTPStatusCode = { _ in .notFound }
-
-        // MUT
-        let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
+        try await withDependencies {
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in .notFound }
+        } operation: {
+            // MUT
+            let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
 
-        // validate
-        switch model {
-            case .packageAvailable, .packageMissing:
-                XCTFail("expected package not to exist")
-            case .packageDoesNotExist:
-                break
+            // validate
+            switch model {
+                case .packageAvailable, .packageMissing:
+                    XCTFail("expected package not to exist")
+                case .packageDoesNotExist:
+                    break
+            }
         }
     }
 
     func test_ShowModel_fetchHTTPStatusCode_error() async throws {
-        // setup
-        Current.fetchHTTPStatusCode = { _ in throw FetchError() }
-
-        // MUT
-        let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
+        try await withDependencies {
+            $0.httpClient.fetchHTTPStatusCode = { @Sendable _ in throw FetchError() }
+        } operation: {
+            // MUT
+            let model = try await PackageController.ShowModel(db: app.db, owner: "owner", repository: "package")
 
-        // validate
-        switch model {
-            case .packageAvailable, .packageMissing:
-                XCTFail("expected package not to exist")
-            case .packageDoesNotExist:
-                break
+            // validate
+            switch model {
+                case .packageAvailable, .packageMissing:
+                    XCTFail("expected package not to exist")
+                case .packageDoesNotExist:
+                    break
+            }
         }
     }