Skip to content

Commit

Permalink
Restore async let in fetchMetadata
Browse files Browse the repository at this point in the history
  • Loading branch information
finestructure committed Dec 13, 2024
1 parent f7a9aa3 commit bb4873d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 30 deletions.
46 changes: 22 additions & 24 deletions Sources/App/Commands/Ingest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,22 @@ extension Ingestion {
static func ingest(client: Client, database: Database, package: Joined<Package, Repository>) async {
let result = await Result { () async throws(Ingestion.Error) -> Joined<Package, Repository> in
Current.logger().info("Ingesting \(package.package.url)")
let (metadata, license, readme) = try await fetchMetadata(client: client, package: package)

// Even though we have a `Joined<Package, Repository>` 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 {
try Github.parseOwnerName(url: package.model.url)
} rethrowing: { _ in
Ingestion.Error.invalidURL(packageId: package.model.id!, url: package.model.url)
}

let (metadata, license, readme) = try await run {
try await fetchMetadata(client: client, package: package.model, owner: owner, repository: repository)
} rethrowing: {
Ingestion.Error(packageId: package.model.id!,
underlyingError: .fetchMetadataFailed(owner: owner, name: repository, details: "\($0)"))
}
let repo = try await findOrCreateRepository(on: database, for: package)

let s3Readme: S3Readme?
Expand Down Expand Up @@ -233,32 +248,15 @@ extension Ingestion {
return repository.s3Readme
}
}
}

func fetchMetadata(client: Client, package: Joined<Package, Repository>) async throws(Ingestion.Error) -> (Github.Metadata, Github.License?, Github.Readme?) {
// Even though we get through a `Joined<Package, Repository>` as a parameter, it's
// we must not rely on `repository` 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 {
try Github.parseOwnerName(url: package.model.url)
} rethrowing: { _ in
Ingestion.Error.invalidURL(packageId: package.model.id!, url: package.model.url)
}

// sas 2024-12-13: This should be an `async let` as well but it doesn't compile right now with the
// typed throw. Reported as
// https://github.com/swiftlang/swift/issues/76169
// async let metadata = try await Current.fetchMetadata(client, owner, repository)
let metadata = try await run {
try await Current.fetchMetadata(client, owner, repository)
} rethrowing: {
Ingestion.Error(packageId: package.model.id!,
underlyingError: .fetchMetadataFailed(owner: owner, name: repository, details: "\($0)"))
}
async let license = await Current.fetchLicense(client, owner, repository)
async let readme = await Current.fetchReadme(client, owner, repository)
static func fetchMetadata(client: Client, package: Package, owner: String, repository: String) async throws -> (Github.Metadata, Github.License?, Github.Readme?) {
async let metadata = try await Current.fetchMetadata(client, owner, repository)
async let license = await Current.fetchLicense(client, owner, repository)
async let readme = await Current.fetchReadme(client, owner, repository)

return (metadata, await license, await readme)
return try await (metadata, license, readme)
}
}


Expand Down
9 changes: 3 additions & 6 deletions Tests/AppTests/IngestorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -623,11 +623,8 @@ class IngestorTests: AppTestCase {
func test_issue_761_no_license() async throws {
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/761
// setup
let pkg = try await {
let p = Package(url: "https://github.com/foo/1")
try await p.save(on: app.db)
return Joined<Package, Repository>(model: p)
}()
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 live fetch request for fetchLicense, whose behaviour we want to test ...
Expand All @@ -637,7 +634,7 @@ class IngestorTests: AppTestCase {
let client = MockClient { _, resp in resp.status = .notFound }

// MUT
let (_, license, _) = try await fetchMetadata(client: client, package: pkg)
let (_, license, _) = try await Ingestion.fetchMetadata(client: client, package: pkg, owner: "foo", repository: "1")

// validate
XCTAssertEqual(license, nil)
Expand Down

0 comments on commit bb4873d

Please sign in to comment.