Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/App/Commands/Ingestion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ enum Ingestion {
}

let (metadata, license, readme) = try await run {
try await fetchMetadata(client: client, package: package.model, owner: owner, repository: repository)
try await fetchMetadata(package: package.model, owner: owner, repository: repository)
} rethrowing: {
Ingestion.Error(packageId: package.model.id!,
underlyingError: .fetchMetadataFailed(owner: owner, name: repository, details: "\($0)"))
Expand Down Expand Up @@ -256,7 +256,7 @@ enum Ingestion {
}


static func fetchMetadata(client: Client, package: Package, owner: String, repository: String) async throws(Github.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
static func fetchMetadata(package: Package, owner: String, repository: String) async throws(Github.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
@Dependency(\.environment) var environment
if environment.shouldFail(failureMode: .fetchMetadataFailed) {
throw Github.Error.requestFailed(.internalServerError)
Expand Down
2 changes: 0 additions & 2 deletions Sources/App/Core/AppEnvironment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ struct AppEnvironment: Sendable {
var fileManager: FileManager
var getStatusCount: @Sendable (_ client: Client, _ status: Gitlab.Builder.Status) async throws -> Int
var git: Git
var githubToken: @Sendable () -> String?
var gitlabApiToken: @Sendable () -> String?
var gitlabPipelineToken: @Sendable () -> String?
var gitlabPipelineLimit: @Sendable () -> Int
Expand Down Expand Up @@ -73,7 +72,6 @@ extension AppEnvironment {
maxPageCount: 5)
},
git: .live,
githubToken: { Environment.get("GITHUB_TOKEN") },
gitlabApiToken: { Environment.get("GITLAB_API_TOKEN") },
gitlabPipelineToken: { Environment.get("GITLAB_PIPELINE_TOKEN") },
gitlabPipelineLimit: {
Expand Down
8 changes: 6 additions & 2 deletions Sources/App/Core/Dependencies/GithubClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import Dependencies
import IssueReporting
import Vapor


// We currently cannot use @DependencyClient here due to
Expand All @@ -24,6 +25,7 @@ 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() }
var fetchReadme: @Sendable (_ owner: String, _ repository: String) async -> Github.Readme?
var token: @Sendable () -> String?
}


Expand All @@ -32,7 +34,8 @@ extension GithubClient: DependencyKey {
.init(
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) },
fetchReadme: { owner, repo in await Github.fetchReadme(owner: owner, repository: repo) }
fetchReadme: { owner, repo in await Github.fetchReadme(owner: owner, repository: repo) },
token: { Environment.get("GITHUB_TOKEN") }
)
}
}
Expand All @@ -43,7 +46,8 @@ extension GithubClient: TestDependencyKey {
.init(
fetchLicense: { _, _ in unimplemented("fetchLicense"); return nil },
fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() },
fetchReadme: { _, _ in unimplemented("fetchReadme"); return nil }
fetchReadme: { _, _ in unimplemented("fetchReadme"); return nil },
token: { unimplemented("token"); return nil }
)
}
}
Expand Down
14 changes: 10 additions & 4 deletions Sources/App/Core/Github.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,13 @@ extension Github {
}

static func fetch(url: String, headers: [(String, String)] = []) async throws -> (content: String, etag: String?) {
guard let token = Current.githubToken() else {
@Dependency(\.github) var github
@Dependency(\.httpClient) var httpClient

guard let token = github.token() else {
throw Error.missingToken
}

@Dependency(\.httpClient) var httpClient

let response = try await httpClient.get(url: url, headers: defaultHeaders(with: token).adding(contentsOf: headers))

Expand All @@ -114,7 +116,9 @@ extension Github {
}

static func fetchResource<T: Decodable>(_ type: T.Type, url: String) async throws -> T {
guard let token = Current.githubToken() else {
@Dependency(\.github) var github

guard let token = github.token() else {
throw Error.missingToken
}

Expand Down Expand Up @@ -176,7 +180,9 @@ extension Github {
}

static func fetchResource<T: Decodable>(_ type: T.Type, query: GraphQLQuery) async throws(Github.Error) -> T {
guard let token = Current.githubToken() else {
@Dependency(\.github) var github

guard let token = github.token() else {
throw Error.missingToken
}

Expand Down
34 changes: 10 additions & 24 deletions Tests/AppTests/GithubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ class GithubTests: AppTestCase {
}

func test_fetchResource() async throws {
Current.githubToken = { "secr3t" }
struct Response: Decodable, Equatable {
var data: Data
struct Data: Decodable, Equatable {
Expand All @@ -116,6 +115,7 @@ class GithubTests: AppTestCase {
let q = Github.GraphQLQuery(query: "query { viewer { login } }")

try await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in
.ok(body: #"{"data":{"viewer":{"login":"finestructure"}}}"#)
}
Expand All @@ -126,10 +126,10 @@ class GithubTests: AppTestCase {
}

func test_fetchMetadata() async throws {
Current.githubToken = { "secr3t" }
let iso8601 = ISO8601DateFormatter()

try await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in
try .ok(fixture: "github-graphql-resource.json")
}
Expand Down Expand Up @@ -181,9 +181,8 @@ class GithubTests: AppTestCase {
}

func test_fetchMetadata_badRequest() async throws {
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in .badRequest }
} operation: {
do {
Expand All @@ -200,10 +199,8 @@ class GithubTests: AppTestCase {
}

func test_fetchMetadata_badData() async throws {
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in .ok(body: "bad data") }
} operation: {
// MUT
Expand All @@ -225,10 +222,8 @@ class GithubTests: AppTestCase {

func test_fetchMetadata_rateLimiting_429() async throws {
// Github doesn't actually send a 429 when you hit the rate limit
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in .tooManyRequests }
} operation: {
// MUT
Expand Down Expand Up @@ -278,10 +273,8 @@ class GithubTests: AppTestCase {
// X-RateLimit-Limit: 60
// X-RateLimit-Remaining: 56
// Ensure we record it as a rate limit error and raise a Rollbar item
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.post = { @Sendable _, _, _ in
.init(status: .forbidden, headers: ["X-RateLimit-Remaining": "0"])
}
Expand Down Expand Up @@ -313,10 +306,8 @@ class GithubTests: AppTestCase {
}

func test_fetchLicense() async throws {
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.get = { @Sendable _, _ in
try .ok(fixture: "github-license-response.json")
}
Expand All @@ -331,10 +322,8 @@ class GithubTests: AppTestCase {

func test_fetchLicense_notFound() async throws {
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/761
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.get = { @Sendable _, _ in .notFound }
} operation: {
// MUT
Expand All @@ -346,10 +335,9 @@ class GithubTests: AppTestCase {
}

func test_fetchReadme() async throws {
// setup
Current.githubToken = { "secr3t" }
let requestCount = QueueIsolated(0)
await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.get = { @Sendable _, headers in
requestCount.increment()
switch headers[.accept] {
Expand Down Expand Up @@ -383,10 +371,8 @@ class GithubTests: AppTestCase {
}

func test_fetchReadme_notFound() async throws {
// setup
Current.githubToken = { "secr3t" }

await withDependencies {
$0.github.token = { "secr3t" }
$0.httpClient.get = { @Sendable _, headers in .notFound }
} operation: {
// MUT
Expand Down
17 changes: 12 additions & 5 deletions Tests/AppTests/IngestionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -636,20 +636,27 @@ class IngestionTests: AppTestCase {
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/761
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) }
$0.github.fetchLicense = GithubClient.liveValue.fetchLicense
// use mock for metadata request which we're not interested in ...
$0.github.fetchMetadata = { @Sendable _, _ in .init() }
$0.github.fetchReadme = { @Sendable _, _ in nil }
$0.github.token = { "token" }
$0.httpClient.get = { @Sendable url, _ in
if url.hasSuffix("/license") {
return .notFound
} else {
XCTFail("unexpected url \(url)")
struct TestError: Error { }
throw TestError()
}
}
} operation: {
// setup
let pkg = Package(url: "https://github.com/foo/1")
try await pkg.save(on: app.db)
// 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")
let (_, license, _) = try await Ingestion.fetchMetadata(package: pkg, owner: "foo", repository: "1")

// validate
XCTAssertEqual(license, nil)
Expand Down
1 change: 0 additions & 1 deletion Tests/AppTests/Mocks/AppEnvironment+mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ extension AppEnvironment {
fileManager: .mock,
getStatusCount: { _, _ in 100 },
git: .mock,
githubToken: { nil },
gitlabApiToken: { nil },
gitlabPipelineToken: { nil },
gitlabPipelineLimit: { Constants.defaultGitlabPipelineLimit },
Expand Down
Loading