diff --git a/Sources/App/Controllers/PackageController+routes.swift b/Sources/App/Controllers/PackageController+routes.swift index a12df5449..7e7829b2e 100644 --- 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 index 196451658..610b8631d 100644 --- 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 index 87eae4bc4..0a7e06630 100644 --- 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,7 +22,8 @@ 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 { @@ -29,6 +31,20 @@ extension HTTPClient: DependencyKey { .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 index b42abfd85..b9fc8efe9 100644 --- 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 index dd577c415..eed792c9e 100644 --- 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 + } } }