diff --git a/Package.swift b/Package.swift index 2c3788214..70e2c2cf0 100644 --- a/Package.swift +++ b/Package.swift @@ -19,7 +19,7 @@ import PackageDescription let package = Package( name: "SPI-Server", platforms: [ - .macOS(.v13) + .macOS(.v15) ], products: [ .executable(name: "Run", targets: ["Run"]), diff --git a/Sources/App/Commands/Ingestion.swift b/Sources/App/Commands/Ingestion.swift index 14edba154..72dbf3f86 100644 --- a/Sources/App/Commands/Ingestion.swift +++ b/Sources/App/Commands/Ingestion.swift @@ -171,7 +171,7 @@ enum Ingestion { // Even though we have a `Joined` as a parameter, we must not rely // on `repository` for owner/name as it will be nil when a package is first ingested. // The only way to get `owner` and `repository` here is by parsing them from the URL. - let (owner, repository) = try await run { + let (owner, repository) = try run { if environment.shouldFail(failureMode: .invalidURL) { throw Github.Error.invalidURL(package.model.url) } @@ -258,13 +258,18 @@ 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 github.fetchLicense(owner, repository) + // Need to pull in github functions individually, because otherwise the `async let` will trigger a + // concurrency error if github gets used more than once: + // 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 + + async let metadata = try await fetchMetadata(owner, repository) + async let license = await fetchLicense(owner, repository) async let readme = await Current.fetchReadme(client, owner, repository) do { diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift index 519e430d6..1daa33b08 100644 --- a/Sources/App/Core/AppEnvironment.swift +++ b/Sources/App/Core/AppEnvironment.swift @@ -23,7 +23,6 @@ import FoundationNetworking struct AppEnvironment: Sendable { - 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 var fileManager: FileManager @@ -65,7 +64,6 @@ extension AppEnvironment { nonisolated(unsafe) static var logger: Logger! static let live = AppEnvironment( - 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) }, fileManager: .live, diff --git a/Sources/App/Core/Dependencies/GithubClient.swift b/Sources/App/Core/Dependencies/GithubClient.swift index 479074783..100b944f8 100644 --- a/Sources/App/Core/Dependencies/GithubClient.swift +++ b/Sources/App/Core/Dependencies/GithubClient.swift @@ -14,26 +14,35 @@ import Dependencies -import DependenciesMacros +import IssueReporting -@DependencyClient +// We currently cannot use @DependencyClient here due to +// https://github.com/pointfreeco/swift-dependencies/discussions/324 +//@DependencyClient 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() } } extension GithubClient: DependencyKey { static var liveValue: Self { .init( - fetchLicense: { owner, repo in await Github.fetchLicense(owner: owner, repository: repo) } + 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) } ) } } extension GithubClient: TestDependencyKey { - static var testValue: Self { Self() } + static var testValue: Self { + .init( + fetchLicense: { _, _ in unimplemented("fetchLicense"); return nil }, + fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() } + ) + } } diff --git a/Sources/App/Core/Dependencies/HTTPClient.swift b/Sources/App/Core/Dependencies/HTTPClient.swift index 532079ade..5d04c93a3 100644 --- a/Sources/App/Core/Dependencies/HTTPClient.swift +++ b/Sources/App/Core/Dependencies/HTTPClient.swift @@ -106,8 +106,9 @@ extension HTTPClient.Response { self.init(host: "host", status: status, version: version, headers: headers, body: body) } - static var ok: Self { .init(status: .ok) } - static var notFound: Self { .init(status: .notFound) } static var badRequest: Self { .init(status: .badRequest) } + static var notFound: Self { .init(status: .notFound) } + static var tooManyRequests: Self { .init(status: .tooManyRequests) } + static var ok: Self { .init(status: .ok) } } #endif diff --git a/Sources/App/Core/ErrorHandlingHelpers.swift b/Sources/App/Core/ErrorHandlingHelpers.swift index 6436b0fbf..15ddca02f 100644 --- a/Sources/App/Core/ErrorHandlingHelpers.swift +++ b/Sources/App/Core/ErrorHandlingHelpers.swift @@ -37,3 +37,15 @@ func run(_ operation: () async throws(E1) -> T, throw transform(error) } } + + +@discardableResult +func run(_ operation: () throws(E1) -> T, + rethrowing transform: (E1) -> E2) throws(E2) -> T { + do { + let result = try operation() + return result + } catch { + throw transform(error) + } +} diff --git a/Sources/App/Core/Github.swift b/Sources/App/Core/Github.swift index a63ceca83..92b140c8d 100644 --- a/Sources/App/Core/Github.swift +++ b/Sources/App/Core/Github.swift @@ -21,11 +21,12 @@ import Vapor enum Github { enum Error: Swift.Error { - case decodeContentFailed(URI, Swift.Error) + case decodeContentFailed(_ url: String, Swift.Error) + case encodeContentFailed(_ url: String, Swift.Error) case missingToken case noBody case invalidURL(String) - case postRequestFailed(URI, Swift.Error) + case postRequestFailed(_ url: String, Swift.Error) case requestFailed(HTTPStatus) } @@ -108,7 +109,7 @@ extension Github { 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 { @@ -211,12 +212,15 @@ 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(_ type: T.Type, client: Client, query: GraphQLQuery) async throws(Github.Error) -> T { guard let token = Current.githubToken() else { throw Error.missingToken @@ -228,7 +232,7 @@ extension Github { try $0.content.encode(query) } } catch { - throw .postRequestFailed(Self.graphQLApiUri, error) + throw .postRequestFailed(graphQLApiUri.string, error) } guard !isRateLimited(response) else { @@ -244,25 +248,57 @@ extension Github { do { return try response.content.decode(T.self, using: decoder) } catch { - throw .decodeContentFailed(Self.graphQLApiUri, error) + throw .decodeContentFailed(graphQLApiUri.string, error) + } + } + + static func fetchResource(_ type: T.Type, query: GraphQLQuery) async throws(Github.Error) -> T { + guard let token = Current.githubToken() else { + throw Error.missingToken + } + + @Dependency(\.httpClient) var httpClient + + let body = try run { + try JSONEncoder().encode(query) + } rethrowing: { + Error.encodeContentFailed(graphQLApiURL, $0) + } + + let response = try await run { + try await httpClient.post(url: graphQLApiURL, headers: defaultHeaders(with: token), body: body) + } rethrowing: { + Error.postRequestFailed(graphQLApiURL, $0) + } + + 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) + } + + guard let body = response.body else { throw Github.Error.noBody } + + return try run { + try decoder.decode(T.self, from: body) + } rethrowing: { + Error.decodeContentFailed(graphQLApiURL, $0) } } - static func fetchMetadata(client: Client, owner: String, repository: String) async throws(Github.Error) -> Metadata { + static func fetchMetadata(owner: String, repository: String) async throws(Github.Error) -> Metadata { struct Response: Decodable, Equatable { var data: T } return try await fetchResource(Response.self, - client: client, query: Metadata.query(owner: owner, repository: repository)) .data } - static func fetchMetadata(client: Client, packageUrl: String) async throws -> Metadata { - let (owner, name) = try parseOwnerName(url: packageUrl) - return try await fetchMetadata(client: client, owner: owner, repository: name) - } - } diff --git a/Tests/AppTests/ErrorReportingTests.swift b/Tests/AppTests/ErrorReportingTests.swift index a2e1fe703..13d7dd8c4 100644 --- a/Tests/AppTests/ErrorReportingTests.swift +++ b/Tests/AppTests/ErrorReportingTests.swift @@ -34,10 +34,10 @@ class ErrorReportingTests: AppTestCase { func test_Ingestion_error_reporting() async throws { // setup try await Package(id: .id0, url: "1", processingStage: .reconciliation).save(on: app.db) - Current.fetchMetadata = { _, _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") } try await withDependencies { $0.date.now = .now + $0.github.fetchMetadata = { @Sendable _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10)) diff --git a/Tests/AppTests/GithubTests.swift b/Tests/AppTests/GithubTests.swift index 6adeddc3b..c5cc41d07 100644 --- a/Tests/AppTests/GithubTests.swift +++ b/Tests/AppTests/GithubTests.swift @@ -104,10 +104,6 @@ class GithubTests: AppTestCase { func test_fetchResource() async throws { Current.githubToken = { "secr3t" } - let client = MockClient { _, resp in - resp.status = .ok - resp.body = makeBody("{\"data\":{\"viewer\":{\"login\":\"finestructure\"}}}") - } struct Response: Decodable, Equatable { var data: Data struct Data: Decodable, Equatable { @@ -118,96 +114,87 @@ class GithubTests: AppTestCase { } } let q = Github.GraphQLQuery(query: "query { viewer { login } }") - let res = try await Github.fetchResource(Response.self, client: client, query: q) - XCTAssertEqual(res, Response(data: .init(viewer: .init(login: "finestructure")))) + + try await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in + .ok(body: #"{"data":{"viewer":{"login":"finestructure"}}}"#) + } + } operation: { + let res = try await Github.fetchResource(Response.self, query: q) + XCTAssertEqual(res, Response(data: .init(viewer: .init(login: "finestructure")))) + } } func test_fetchMetadata() async throws { Current.githubToken = { "secr3t" } - let data = try XCTUnwrap(try fixtureData(for: "github-graphql-resource.json")) - let client = MockClient { _, resp in - resp.status = .ok - resp.body = makeBody(data) - } let iso8601 = ISO8601DateFormatter() - // MUT - let res = try await Github.fetchMetadata(client: client, - owner: "alamofire", - repository: "alamofire") - - // validation - XCTAssertEqual(res.repository?.closedIssues.nodes.first!.closedAt, - iso8601.date(from: "2020-07-17T16:27:10Z")) - XCTAssertEqual(res.repository?.closedPullRequests.nodes.first!.closedAt, - iso8601.date(from: "2021-05-28T15:50:17Z")) - XCTAssertEqual(res.repository?.forkCount, 6727) - XCTAssertEqual(res.repository?.fundingLinks, [ - .init(platform: .gitHub, url: "https://github.com/Alamofire"), - .init(platform: .lfxCrowdfunding, url: "https://crowdfunding.lfx.linuxfoundation.org/projects/alamofire"), - ]) - XCTAssertEqual(res.repository?.mergedPullRequests.nodes.first!.closedAt, - iso8601.date(from: "2021-06-07T22:47:01Z")) - XCTAssertEqual(res.repository?.name, "Alamofire") - XCTAssertEqual(res.repository?.owner.name, "Alamofire") - XCTAssertEqual(res.repository?.owner.login, "Alamofire") - XCTAssertEqual(res.repository?.owner.avatarUrl, "https://avatars.githubusercontent.com/u/7774181?v=4") - XCTAssertEqual(res.repository?.openIssues.totalCount, 30) - XCTAssertEqual(res.repository?.openPullRequests.totalCount, 6) - XCTAssertEqual(res.repository?.releases.nodes.count, 20) - XCTAssertEqual(res.repository?.releases.nodes.first, .some( - .init(description: "Released on 2020-04-21. All issues associated with this milestone can be found using this [filter](https://github.com/Alamofire/Alamofire/milestone/77?closed=1).\r\n\r\n#### Fixed\r\n- Change in multipart upload creation order.\r\n - Fixed by [Christian Noon](https://github.com/cnoon) in Pull Request [#3438](https://github.com/Alamofire/Alamofire/pull/3438).\r\n- Typo in Alamofire 5 migration guide.\r\n - Fixed by [DevYeom](https://github.com/DevYeom) in Pull Request [#3431](https://github.com/Alamofire/Alamofire/pull/3431).", - descriptionHTML: "

mock descriptionHTML", - isDraft: false, - publishedAt: iso8601.date(from: "2021-04-22T02:50:05Z")!, - tagName: "5.4.3", - url: "https://github.com/Alamofire/Alamofire/releases/tag/5.4.3") - )) - XCTAssertEqual(res.repository?.repositoryTopics.totalCount, 15) - XCTAssertEqual(res.repository?.repositoryTopics.nodes.first?.topic.name, - "networking") - XCTAssertEqual(res.repository?.stargazerCount, 35831) - XCTAssertEqual(res.repository?.isInOrganization, true) - XCTAssertEqual(res.repository?.homepageUrl, "https://swiftpackageindex.com/Alamofire/Alamofire") - // derived properties - XCTAssertEqual(res.repository?.lastIssueClosedAt, - iso8601.date(from: "2021-06-09T00:59:39Z")) - // merged date is latest - expect that one to be reported back - XCTAssertEqual(res.repository?.lastPullRequestClosedAt, - iso8601.date(from: "2021-06-07T22:47:01Z")) + try await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in + try .ok(fixture: "github-graphql-resource.json") + } + } operation: { + // MUT + let res = try await Github.fetchMetadata(owner: "alamofire", + repository: "alamofire") + + // validation + XCTAssertEqual(res.repository?.closedIssues.nodes.first!.closedAt, + iso8601.date(from: "2020-07-17T16:27:10Z")) + XCTAssertEqual(res.repository?.closedPullRequests.nodes.first!.closedAt, + iso8601.date(from: "2021-05-28T15:50:17Z")) + XCTAssertEqual(res.repository?.forkCount, 6727) + XCTAssertEqual(res.repository?.fundingLinks, [ + .init(platform: .gitHub, url: "https://github.com/Alamofire"), + .init(platform: .lfxCrowdfunding, url: "https://crowdfunding.lfx.linuxfoundation.org/projects/alamofire"), + ]) + XCTAssertEqual(res.repository?.mergedPullRequests.nodes.first!.closedAt, + iso8601.date(from: "2021-06-07T22:47:01Z")) + XCTAssertEqual(res.repository?.name, "Alamofire") + XCTAssertEqual(res.repository?.owner.name, "Alamofire") + XCTAssertEqual(res.repository?.owner.login, "Alamofire") + XCTAssertEqual(res.repository?.owner.avatarUrl, "https://avatars.githubusercontent.com/u/7774181?v=4") + XCTAssertEqual(res.repository?.openIssues.totalCount, 30) + XCTAssertEqual(res.repository?.openPullRequests.totalCount, 6) + XCTAssertEqual(res.repository?.releases.nodes.count, 20) + XCTAssertEqual(res.repository?.releases.nodes.first, .some( + .init(description: "Released on 2020-04-21. All issues associated with this milestone can be found using this [filter](https://github.com/Alamofire/Alamofire/milestone/77?closed=1).\r\n\r\n#### Fixed\r\n- Change in multipart upload creation order.\r\n - Fixed by [Christian Noon](https://github.com/cnoon) in Pull Request [#3438](https://github.com/Alamofire/Alamofire/pull/3438).\r\n- Typo in Alamofire 5 migration guide.\r\n - Fixed by [DevYeom](https://github.com/DevYeom) in Pull Request [#3431](https://github.com/Alamofire/Alamofire/pull/3431).", + descriptionHTML: "

mock descriptionHTML", + isDraft: false, + publishedAt: iso8601.date(from: "2021-04-22T02:50:05Z")!, + tagName: "5.4.3", + url: "https://github.com/Alamofire/Alamofire/releases/tag/5.4.3") + )) + XCTAssertEqual(res.repository?.repositoryTopics.totalCount, 15) + XCTAssertEqual(res.repository?.repositoryTopics.nodes.first?.topic.name, + "networking") + XCTAssertEqual(res.repository?.stargazerCount, 35831) + XCTAssertEqual(res.repository?.isInOrganization, true) + XCTAssertEqual(res.repository?.homepageUrl, "https://swiftpackageindex.com/Alamofire/Alamofire") + // derived properties + XCTAssertEqual(res.repository?.lastIssueClosedAt, + iso8601.date(from: "2021-06-09T00:59:39Z")) + // merged date is latest - expect that one to be reported back + XCTAssertEqual(res.repository?.lastPullRequestClosedAt, + iso8601.date(from: "2021-06-07T22:47:01Z")) + } } func test_fetchMetadata_badRequest() async throws { Current.githubToken = { "secr3t" } - let client = MockClient { _, resp in - resp.status = .badRequest - } - do { - _ = try await Github.fetchMetadata(client: client, - owner: "alamofire", - repository: "alamofire") - XCTFail("expected error to be thrown") - } catch { - guard case Github.Error.requestFailed(.badRequest) = error else { - XCTFail("unexpected error: \(error.localizedDescription)") - return - } - } - } - - func test_fetchMetadata_badUrl() async throws { - let pkg = Package(url: "https://foo/bar") - let client = MockClient { _, resp in - resp.status = .ok - } - do { - _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url) - XCTFail("expected error to be thrown") - } catch { - guard case Github.Error.invalidURL = error else { - XCTFail("unexpected error: \(error.localizedDescription)") - return + await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in .badRequest } + } operation: { + do { + _ = try await Github.fetchMetadata(owner: "alamofire", + repository: "alamofire") + XCTFail("expected error to be thrown") + } catch { + guard case Github.Error.requestFailed(.badRequest) = error else { + XCTFail("unexpected error: \(error.localizedDescription)") + return + } } } } @@ -215,25 +202,24 @@ class GithubTests: AppTestCase { func test_fetchMetadata_badData() async throws { // setup Current.githubToken = { "secr3t" } - let pkg = Package(url: "https://github.com/foo/bar") - let client = MockClient { _, resp in - resp.status = .ok - resp.body = makeBody("bad data") - } - // MUT - do { - _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url) - XCTFail("expected error to be thrown") - } catch let Github.Error.decodeContentFailed(uri, error) { - // validation - XCTAssertEqual(uri, "https://api.github.com/graphql") - guard case DecodingError.dataCorrupted = error else { - XCTFail("unexpected error: \(error.localizedDescription)") - return + await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in .ok(body: "bad data") } + } operation: { + // MUT + do { + _ = try await Github.fetchMetadata(owner: "foo", repository: "bar") + XCTFail("expected error to be thrown") + } catch let Github.Error.decodeContentFailed(uri, error) { + // validation + XCTAssertEqual(uri, "https://api.github.com/graphql") + guard case DecodingError.dataCorrupted = error else { + XCTFail("unexpected error: \(error.localizedDescription)") + return + } + } catch { + XCTFail("Unexpected error: \(error)") } - } catch { - XCTFail("Unexpected error: \(error)") } } @@ -241,20 +227,20 @@ class GithubTests: AppTestCase { // Github doesn't actually send a 429 when you hit the rate limit // setup Current.githubToken = { "secr3t" } - let pkg = Package(url: "https://github.com/foo/bar") - let client = MockClient { _, resp in - resp.status = .tooManyRequests - } - // MUT - do { - _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url) - XCTFail("expected error to be thrown") - } catch { - // validation - guard case Github.Error.requestFailed(.tooManyRequests) = error else { - XCTFail("unexpected error: \(error.localizedDescription)") - return + await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in .tooManyRequests } + } operation: { + // MUT + do { + _ = try await Github.fetchMetadata(owner: "foo", repository: "bar") + XCTFail("expected error to be thrown") + } catch { + // validation + guard case Github.Error.requestFailed(.tooManyRequests) = error else { + XCTFail("unexpected error: \(error.localizedDescription)") + return + } } } } @@ -294,26 +280,27 @@ class GithubTests: AppTestCase { // Ensure we record it as a rate limit error and raise a Rollbar item // setup Current.githubToken = { "secr3t" } - let pkg = Package(url: "https://github.com/foo/bar") - let client = MockClient { _, resp in - resp.status = .forbidden - resp.headers.add(name: "X-RateLimit-Remaining", value: "0") - } - // MUT - do { - _ = try await Github.fetchMetadata(client: client, packageUrl: pkg.url) - XCTFail("expected error to be thrown") - } catch { - // validation - logger.logs.withValue { logs in - XCTAssertEqual(logs, [ - .init(level: .critical, message: "rate limited while fetching resource Response") - ]) + await withDependencies { + $0.httpClient.post = { @Sendable _, _, _ in + .init(status: .forbidden, headers: ["X-RateLimit-Remaining": "0"]) } - guard case Github.Error.requestFailed(.tooManyRequests) = error else { - XCTFail("unexpected error: \(error.localizedDescription)") - return + } operation: { + // MUT + do { + _ = try await Github.fetchMetadata(owner: "foo", repository: "bar") + XCTFail("expected error to be thrown") + } catch { + // validation + logger.logs.withValue { logs in + XCTAssertEqual(logs, [ + .init(level: .critical, message: "rate limited while fetching resource Response") + ]) + } + guard case Github.Error.requestFailed(.tooManyRequests) = error else { + XCTFail("unexpected error: \(error.localizedDescription)") + return + } } } } @@ -331,7 +318,7 @@ class GithubTests: AppTestCase { await withDependencies { $0.httpClient.get = { @Sendable _, _ in - try .init(status: .ok, body: .fixture(named: "github-license-response.json")) + try .ok(fixture: "github-license-response.json") } } operation: { // MUT @@ -494,8 +481,12 @@ class GithubTests: AppTestCase { } -private extension ByteBuffer { - static func fixture(named filename: String) throws -> Self { - .init(data: try fixtureData(for: filename)) +private extension HTTPClient.Response { + static func ok(body: String) -> Self { + .init(status: .ok, body: .init(string: body)) + } + + static func ok(fixture: String) throws -> Self { + try .init(status: .ok, body: .init(data: fixtureData(for: fixture))) } } diff --git a/Tests/AppTests/IngestionTests.swift b/Tests/AppTests/IngestionTests.swift index f0cd97a02..021f668ad 100644 --- a/Tests/AppTests/IngestionTests.swift +++ b/Tests/AppTests/IngestionTests.swift @@ -26,7 +26,6 @@ class IngestionTests: AppTestCase { func test_ingest_basic() async throws { // setup - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } let packages = ["https://github.com/finestructure/Gala", "https://github.com/finestructure/Rester", "https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server"] @@ -37,6 +36,7 @@ class IngestionTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10)) @@ -67,17 +67,17 @@ class IngestionTests: AppTestCase { // Test completion of ingestion despite early error 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.init(model:)) - Current.fetchMetadata = { _, owner, repository throws(Github.Error) in + $0.github.fetchMetadata = { @Sendable owner, repository throws(Github.Error) in if owner == "foo" && repository == "1" { throw Github.Error.requestFailed(.badRequest) } return .mock(owner: owner, repository: repository) } + } operation: { + // setup + let packages = try await savePackages(on: app.db, ["https://github.com/foo/1", + "https://github.com/foo/2"], processingStage: .reconciliation) + .map(Joined.init(model:)) // MUT await Ingestion.ingest(client: app.client, database: app.db, packages: packages) @@ -306,13 +306,13 @@ class IngestionTests: AppTestCase { func test_partial_save_issue() async throws { // Test to ensure futures are properly waited for and get flushed to the db in full // setup - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } let packages = testUrls.map { Package(url: $0, processingStage: .reconciliation) } try await packages.save(on: app.db) try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(testUrls.count)) @@ -330,17 +330,17 @@ class IngestionTests: AppTestCase { "https://github.com/foo/2", "https://github.com/foo/3"] try await savePackages(on: app.db, urls.asURLs, processingStage: .reconciliation) - Current.fetchMetadata = { _, owner, repository throws(Github.Error) in - if owner == "foo" && repository == "2" { - throw Github.Error.requestFailed(.badRequest) - } - return .mock(owner: owner, repository: repository) - } let lastUpdate = Date() try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository throws(Github.Error) in + if owner == "foo" && repository == "2" { + throw Github.Error.requestFailed(.badRequest) + } + return .mock(owner: owner, repository: repository) + } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10)) @@ -371,29 +371,29 @@ class IngestionTests: AppTestCase { .save(on: app.db) try await Package(id: .id1, url: "https://github.com/foo/1", status: .ok, processingStage: .reconciliation) .save(on: app.db) - // Return identical metadata for both packages, same as a for instance a redirected - // package would after a rename / ownership change - Current.fetchMetadata = { _, _, _ in - Github.Metadata.init( - defaultBranch: "main", - forks: 0, - homepageUrl: nil, - isInOrganization: false, - issuesClosedAtDates: [], - license: .mit, - openIssues: 0, - parentUrl: nil, - openPullRequests: 0, - owner: "owner", - pullRequestsClosedAtDates: [], - name: "name", - stars: 0, - summary: "desc") - } try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + // Return identical metadata for both packages, same as a for instance a redirected + // package would after a rename / ownership change + $0.github.fetchMetadata = { @Sendable _, _ in + Github.Metadata.init( + defaultBranch: "main", + forks: 0, + homepageUrl: nil, + isInOrganization: false, + issuesClosedAtDates: [], + license: .mit, + openIssues: 0, + parentUrl: nil, + openPullRequests: 0, + owner: "owner", + pullRequestsClosedAtDates: [], + name: "name", + stars: 0, + summary: "desc") + } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10)) @@ -462,12 +462,12 @@ class IngestionTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } } operation: { // setup let app = self.app! let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation) try await pkg.save(on: app.db) - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } let fetchCalls = QueueIsolated(0) Current.fetchReadme = { _, _, _ in fetchCalls.increment() @@ -547,7 +547,6 @@ class IngestionTests: AppTestCase { let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation) try await pkg.save(on: app.db) - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } Current.storeS3Readme = { _, _, _ in "objectUrl" } Current.fetchReadme = { _, _, _ in return .init(etag: "etag", @@ -580,6 +579,7 @@ class IngestionTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(1)) @@ -594,7 +594,6 @@ class IngestionTests: AppTestCase { // setup let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation) try await pkg.save(on: app.db) - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } Current.fetchReadme = { _, _, _ in return .init(etag: "etag1", html: "readme html 1", @@ -611,6 +610,7 @@ class IngestionTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } } operation: { // MUT let app = self.app! @@ -632,12 +632,12 @@ class IngestionTests: AppTestCase { 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) } + // use mock for metadata request which we're not interested in ... + $0.github.fetchMetadata = { @Sendable _, _ in .init() } } 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 } diff --git a/Tests/AppTests/MastodonTests.swift b/Tests/AppTests/MastodonTests.swift index bd8445be1..74f74e7e4 100644 --- a/Tests/AppTests/MastodonTests.swift +++ b/Tests/AppTests/MastodonTests.swift @@ -26,6 +26,7 @@ final class MastodonTests: AppTestCase { try await withDependencies { $0.environment.allowSocialPosts = { true } $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } $0.httpClient.mastodonPost = { @Sendable msg in if message.value == nil { message.setValue(msg) @@ -36,8 +37,6 @@ final class MastodonTests: AppTestCase { } operation: { // setup let url = "https://github.com/foo/bar" - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } - Current.git.commitCount = { @Sendable _ in 12 } Current.git.firstCommitDate = { @Sendable _ in .t0 } Current.git.lastCommitDate = { @Sendable _ in .t2 } diff --git a/Tests/AppTests/Mocks/AppEnvironment+mock.swift b/Tests/AppTests/Mocks/AppEnvironment+mock.swift index 25dc04c94..c9b5e2b4f 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( - fetchMetadata: { _, _, _ in .mock }, fetchReadme: { _, _, _ in .init(html: "readme html", htmlUrl: "readme html url", imagesToCache: []) }, fetchS3Readme: { _, _, _ in "" }, fileManager: .mock, diff --git a/Tests/AppTests/PackageTests.swift b/Tests/AppTests/PackageTests.swift index 5cbf66869..3ad6539f6 100644 --- a/Tests/AppTests/PackageTests.swift +++ b/Tests/AppTests/PackageTests.swift @@ -312,12 +312,12 @@ final class PackageTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } $0.packageListRepository.fetchPackageList = { @Sendable _ in [url.url] } $0.packageListRepository.fetchPackageDenyList = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollections = { @Sendable _ in [] } } operation: { // setup - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } Current.git.commitCount = { @Sendable _ in 12 } Current.git.firstCommitDate = { @Sendable _ in Date(timeIntervalSince1970: 0) } Current.git.getTags = { @Sendable _ in [] } diff --git a/Tests/AppTests/PipelineTests.swift b/Tests/AppTests/PipelineTests.swift index 020e42308..ce27b2d66 100644 --- a/Tests/AppTests/PipelineTests.swift +++ b/Tests/AppTests/PipelineTests.swift @@ -162,6 +162,7 @@ class PipelineTests: AppTestCase { try await withDependencies { $0.date.now = .now $0.github.fetchLicense = { @Sendable _, _ in nil } + $0.github.fetchMetadata = { @Sendable owner, repository in .mock(owner: owner, repository: repository) } $0.packageListRepository.fetchPackageList = { @Sendable _ in urls.asURLs } $0.packageListRepository.fetchPackageDenyList = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollections = { @Sendable _ in [] } @@ -169,7 +170,6 @@ class PipelineTests: AppTestCase { } operation: { // Test pipeline pick-up end to end // setup - Current.fetchMetadata = { _, owner, repository in .mock(owner: owner, repository: repository) } Current.git.commitCount = { @Sendable _ in 12 } Current.git.firstCommitDate = { @Sendable _ in .t0 } Current.git.lastCommitDate = { @Sendable _ in .t1 }