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
3 changes: 2 additions & 1 deletion Sources/App/Commands/Ingestion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,11 @@ enum Ingestion {
// 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
@Dependency(\.github.fetchReadme) var fetchReadme

async let metadata = try await fetchMetadata(owner, repository)
async let license = await fetchLicense(owner, repository)
async let readme = await Current.fetchReadme(client, owner, repository)
async let readme = await fetchReadme(owner, repository)

do {
return try await (metadata, license, readme)
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 @@ -23,7 +23,6 @@ import FoundationNetworking


struct AppEnvironment: Sendable {
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
var getStatusCount: @Sendable (_ client: Client, _ status: Gitlab.Builder.Status) async throws -> Int
Expand Down Expand Up @@ -64,7 +63,6 @@ extension AppEnvironment {
nonisolated(unsafe) static var logger: Logger!

static let live = AppEnvironment(
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,
getStatusCount: { client, status in
Expand Down
7 changes: 5 additions & 2 deletions Sources/App/Core/Dependencies/GithubClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ import IssueReporting
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?
}


extension GithubClient: DependencyKey {
static var liveValue: Self {
.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) }
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) }
)
}
}
Expand All @@ -40,7 +42,8 @@ extension GithubClient: TestDependencyKey {
static var testValue: Self {
.init(
fetchLicense: { _, _ in unimplemented("fetchLicense"); return nil },
fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() }
fetchMetadata: { _, _ in unimplemented("fetchMetadata"); return .init() },
fetchReadme: { _, _ in unimplemented("fetchReadme"); return nil }
)
}
}
Expand Down
97 changes: 10 additions & 87 deletions Sources/App/Core/Github.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@ enum Github {
return decoder
}

@available(*, deprecated)
static func rateLimit(response: ClientResponse) -> Int? {
guard
let header = response.headers.first(name: "X-RateLimit-Remaining"),
let limit = Int(header)
else { return nil }
return limit
}

static func rateLimit(response: HTTPClient.Response) -> Int? {
guard
let header = response.headers.first(name: "X-RateLimit-Remaining"),
Expand All @@ -54,13 +45,6 @@ enum Github {
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)
Expand Down Expand Up @@ -95,36 +79,29 @@ extension Github {
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?) {
static func fetch(url: String, headers: [(String, String)] = []) async throws -> (content: String, etag: String?) {
guard let token = Current.githubToken() else {
throw Error.missingToken
}

let response = try await client.get(uri, headers: defaultHeaders(with: token).adding(contentsOf: headers))
@Dependency(\.httpClient) var httpClient

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

guard !isRateLimited(response) else {
Current.logger().critical("rate limited while fetching uri \(uri)")
Current.logger().critical("rate limited while fetching \(url)")
throw Error.requestFailed(.tooManyRequests)
}

guard response.status == .ok else {
Current.logger().warning("Github.fetch of '\(uri.path)' failed with status \(response.status)")
Current.logger().warning("Github.fetch of '\(url)' failed with status \(response.status)")
throw Error.requestFailed(response.status)
}

Expand All @@ -136,26 +113,6 @@ extension Github {
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
}

let response = try await client.get(uri, headers: defaultHeaders(with: token))

guard !isRateLimited(response) else {
Current.logger().critical("rate limited while fetching resource \(uri)")
throw Error.requestFailed(.tooManyRequests)
}

guard response.status == .ok else {
throw Error.requestFailed(response.status)
}

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
Expand All @@ -181,11 +138,11 @@ extension Github {
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)
static func fetchReadme(owner: String, repository: String) async -> Readme? {
let url = Github.apiURL(owner: owner, repository: repository, resource: .readme)

// Fetch readme html content
let readme = try? await Github.fetch(client: client, uri: uri, headers: [
let readme = try? await Github.fetch(url: url, headers: [
("Accept", "application/vnd.github.html+json")
])
guard var html = readme?.content else { return nil }
Expand All @@ -195,7 +152,7 @@ extension Github {
struct Response: Decodable {
var htmlUrl: String
}
return try? await Github.fetchResource(Response.self, client: client, uri: uri).htmlUrl
return try? await Github.fetchResource(Response.self, url: url).htmlUrl
}()
guard let htmlUrl else { return nil }

Expand All @@ -212,46 +169,12 @@ 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<T: Decodable>(_ type: T.Type, client: Client, query: GraphQLQuery) async throws(Github.Error) -> T {
guard let token = Current.githubToken() else {
throw Error.missingToken
}

let response: ClientResponse
do {
response = try await client.post(Self.graphQLApiUri, headers: defaultHeaders(with: token)) {
try $0.content.encode(query)
}
} catch {
throw .postRequestFailed(graphQLApiUri.string, error)
}

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)
}

do {
return try response.content.decode(T.self, using: decoder)
} catch {
throw .decodeContentFailed(graphQLApiUri.string, error)
}
}

static func fetchResource<T: Decodable>(_ type: T.Type, query: GraphQLQuery) async throws(Github.Error) -> T {
guard let token = Current.githubToken() else {
throw Error.missingToken
Expand Down
79 changes: 44 additions & 35 deletions Tests/AppTests/GithubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -349,48 +349,52 @@ class GithubTests: AppTestCase {
// setup
Current.githubToken = { "secr3t" }
let requestCount = QueueIsolated(0)
let client = MockClient { req, resp in
requestCount.increment()
switch req.headers[.accept] {
case ["application/vnd.github.html+json"]:
resp.status = .ok
resp.body = makeBody("readme html")
resp.headers.add(name: .eTag, value: "etag")
case []:
resp.status = .ok
struct Response: Encodable {
var htmlUrl: String
}
resp.body = makeBody(try! JSONEncoder().encode(Response(htmlUrl: "readme url")))
default:
XCTFail("unexpected accept header")
await withDependencies {
$0.httpClient.get = { @Sendable _, headers in
requestCount.increment()
switch headers[.accept] {
case ["application/vnd.github.html+json"]:
return .ok(body: "readme html", headers: ["ETag": "etag"])
case []:
struct Response: Encodable {
var htmlUrl: String
}
return try .ok(jsonEncode: Response(htmlUrl: "readme url"))
default:
XCTFail("unexpected accept header")
}
enum Error: Swift.Error { case unexpectedCodePath }
throw Error.unexpectedCodePath
}
}
} operation: {
// MUT
let res = await Github.fetchReadme(owner: "foo", repository: "bar")

// MUT
let res = await Github.fetchReadme(client: client, owner: "foo", repository: "bar")

// validate
XCTAssertEqual(requestCount.value, 2)
XCTAssertEqual(
res,
.init(etag: "etag",
html: "readme html",
htmlUrl: "readme url",
imagesToCache: [])
)
// validate
XCTAssertEqual(requestCount.value, 2)
XCTAssertEqual(
res,
.init(etag: "etag",
html: "readme html",
htmlUrl: "readme url",
imagesToCache: [])
)
}
}

func test_fetchReadme_notFound() async throws {
// setup
Current.githubToken = { "secr3t" }
let client = MockClient { _, resp in resp.status = .notFound }

// MUT
let res = await Github.fetchReadme(client: client, owner: "foo", repository: "bar")
await withDependencies {
$0.httpClient.get = { @Sendable _, headers in .notFound }
} operation: {
// MUT
let res = await Github.fetchReadme(owner: "foo", repository: "bar")

// validate
XCTAssertEqual(res, nil)
// validate
XCTAssertEqual(res, nil)
}
}

func test_extractImagesRequiringCaching() async throws {
Expand Down Expand Up @@ -482,11 +486,16 @@ class GithubTests: AppTestCase {


private extension HTTPClient.Response {
static func ok(body: String) -> Self {
.init(status: .ok, body: .init(string: body))
static func ok(body: String, headers: HTTPHeaders = .init()) -> Self {
.init(status: .ok, headers: headers, body: .init(string: body))
}

static func ok(fixture: String) throws -> Self {
try .init(status: .ok, body: .init(data: fixtureData(for: fixture)))
}

static func ok<T: Encodable>(jsonEncode value: T) throws -> Self {
let data = try JSONEncoder().encode(value)
return .init(status: .ok, body: .init(data: data))
}
}
Loading
Loading