Skip to content

Commit 8630be3

Browse files
committed
Move remaining two top level functions into Ingestion namespace
1 parent 365c0af commit 8630be3

File tree

2 files changed

+118
-114
lines changed

2 files changed

+118
-114
lines changed

Sources/App/Commands/Ingestion.swift

Lines changed: 89 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ enum Ingestion {
116116
/// - mode: process a single `Package.Id` or a `limit` number of packages
117117
/// - Returns: future
118118
static func ingest(client: Client,
119-
database: Database,
120-
mode: SPICommand.Mode) async throws {
119+
database: Database,
120+
mode: SPICommand.Mode) async throws {
121121
let start = DispatchTime.now().uptimeNanoseconds
122122
defer { AppMetrics.ingestDurationSeconds?.time(since: start) }
123123

@@ -197,10 +197,10 @@ enum Ingestion {
197197
s3Readme = .error("\(error)")
198198
}
199199

200-
let fork = await getFork(on: database, parent: metadata.repository?.parent)
200+
let fork = await Ingestion.getFork(on: database, parent: metadata.repository?.parent)
201201

202202
try await run { () async throws(Ingestion.Error.UnderlyingError) in
203-
try await updateRepository(on: database, for: repo, metadata: metadata, licenseInfo: license, readmeInfo: readme, s3Readme: s3Readme, fork: fork)
203+
try await Ingestion.updateRepository(on: database, for: repo, metadata: metadata, licenseInfo: license, readmeInfo: readme, s3Readme: s3Readme, fork: fork)
204204
} rethrowing: {
205205
Ingestion.Error(packageId: package.model.id!, underlyingError: $0)
206206
}
@@ -281,108 +281,111 @@ enum Ingestion {
281281
// but let's play it safe and not risk a server crash, unlikely as it may be.
282282
}
283283
}
284-
}
285284

286285

287-
/// Insert or update `Repository` of given `Package` with given `Github.Metadata`.
288-
/// - Parameters:
289-
/// - database: `Database` object
290-
/// - package: package to update
291-
/// - metadata: `Github.Metadata` with data for update
292-
/// - Returns: future
293-
func updateRepository(on database: Database,
294-
for repository: Repository,
295-
metadata: Github.Metadata,
296-
licenseInfo: Github.License?,
297-
readmeInfo: Github.Readme?,
298-
s3Readme: S3Readme?,
299-
fork: Fork? = nil) async throws(Ingestion.Error.UnderlyingError) {
300-
@Dependency(\.environment) var environment
301-
if environment.shouldFail(failureMode: .noRepositoryMetadata) {
302-
throw .noRepositoryMetadata(owner: repository.owner, name: repository.name)
303-
}
304-
if environment.shouldFail(failureMode: .repositorySaveFailed) {
305-
throw .repositorySaveFailed(owner: repository.owner,
306-
name: repository.name,
307-
details: "TestError")
308-
}
309-
if environment.shouldFail(failureMode: .repositorySaveUniqueViolation) {
310-
throw .repositorySaveUniqueViolation(owner: repository.owner,
311-
name: repository.name,
312-
details: "TestError")
313-
}
314-
guard let repoMetadata = metadata.repository else {
315-
throw .noRepositoryMetadata(owner: repository.owner, name: repository.name)
316-
}
286+
/// Insert or update `Repository` of given `Package` with given `Github.Metadata`.
287+
/// - Parameters:
288+
/// - database: `Database` object
289+
/// - package: package to update
290+
/// - metadata: `Github.Metadata` with data for update
291+
/// - Returns: future
292+
static func updateRepository(on database: Database,
293+
for repository: Repository,
294+
metadata: Github.Metadata,
295+
licenseInfo: Github.License?,
296+
readmeInfo: Github.Readme?,
297+
s3Readme: S3Readme?,
298+
fork: Fork? = nil) async throws(Ingestion.Error.UnderlyingError) {
299+
@Dependency(\.environment) var environment
300+
if environment.shouldFail(failureMode: .noRepositoryMetadata) {
301+
throw .noRepositoryMetadata(owner: repository.owner, name: repository.name)
302+
}
303+
if environment.shouldFail(failureMode: .repositorySaveFailed) {
304+
throw .repositorySaveFailed(owner: repository.owner,
305+
name: repository.name,
306+
details: "TestError")
307+
}
308+
if environment.shouldFail(failureMode: .repositorySaveUniqueViolation) {
309+
throw .repositorySaveUniqueViolation(owner: repository.owner,
310+
name: repository.name,
311+
details: "TestError")
312+
}
313+
guard let repoMetadata = metadata.repository else {
314+
throw .noRepositoryMetadata(owner: repository.owner, name: repository.name)
315+
}
316+
317+
repository.defaultBranch = repoMetadata.defaultBranch
318+
repository.forks = repoMetadata.forkCount
319+
repository.fundingLinks = repoMetadata.fundingLinks?.compactMap(FundingLink.init(from:)) ?? []
320+
repository.hasSPIBadge = readmeInfo?.containsSPIBadge()
321+
repository.homepageUrl = repoMetadata.homepageUrl?.trimmed
322+
repository.isArchived = repoMetadata.isArchived
323+
repository.isInOrganization = repoMetadata.isInOrganization
324+
repository.keywords = Set(repoMetadata.topics.map { $0.lowercased() }).sorted()
325+
repository.lastIssueClosedAt = repoMetadata.lastIssueClosedAt
326+
repository.lastPullRequestClosedAt = repoMetadata.lastPullRequestClosedAt
327+
repository.license = .init(from: repoMetadata.licenseInfo)
328+
repository.licenseUrl = licenseInfo?.htmlUrl
329+
repository.name = repoMetadata.repositoryName
330+
repository.openIssues = repoMetadata.openIssues.totalCount
331+
repository.openPullRequests = repoMetadata.openPullRequests.totalCount
332+
repository.owner = repoMetadata.repositoryOwner
333+
repository.ownerName = repoMetadata.owner.name
334+
repository.ownerAvatarUrl = repoMetadata.owner.avatarUrl
335+
repository.s3Readme = s3Readme
336+
repository.readmeHtmlUrl = readmeInfo?.htmlUrl
337+
repository.releases = repoMetadata.releases.nodes.map(Release.init(from:))
338+
repository.stars = repoMetadata.stargazerCount
339+
repository.summary = repoMetadata.description
340+
repository.forkedFrom = fork
317341

318-
repository.defaultBranch = repoMetadata.defaultBranch
319-
repository.forks = repoMetadata.forkCount
320-
repository.fundingLinks = repoMetadata.fundingLinks?.compactMap(FundingLink.init(from:)) ?? []
321-
repository.hasSPIBadge = readmeInfo?.containsSPIBadge()
322-
repository.homepageUrl = repoMetadata.homepageUrl?.trimmed
323-
repository.isArchived = repoMetadata.isArchived
324-
repository.isInOrganization = repoMetadata.isInOrganization
325-
repository.keywords = Set(repoMetadata.topics.map { $0.lowercased() }).sorted()
326-
repository.lastIssueClosedAt = repoMetadata.lastIssueClosedAt
327-
repository.lastPullRequestClosedAt = repoMetadata.lastPullRequestClosedAt
328-
repository.license = .init(from: repoMetadata.licenseInfo)
329-
repository.licenseUrl = licenseInfo?.htmlUrl
330-
repository.name = repoMetadata.repositoryName
331-
repository.openIssues = repoMetadata.openIssues.totalCount
332-
repository.openPullRequests = repoMetadata.openPullRequests.totalCount
333-
repository.owner = repoMetadata.repositoryOwner
334-
repository.ownerName = repoMetadata.owner.name
335-
repository.ownerAvatarUrl = repoMetadata.owner.avatarUrl
336-
repository.s3Readme = s3Readme
337-
repository.readmeHtmlUrl = readmeInfo?.htmlUrl
338-
repository.releases = repoMetadata.releases.nodes.map(Release.init(from:))
339-
repository.stars = repoMetadata.stargazerCount
340-
repository.summary = repoMetadata.description
341-
repository.forkedFrom = fork
342-
343-
do {
344-
try await repository.save(on: database)
345-
} catch let error as PSQLError where error.isUniqueViolation {
346-
let details = error.serverInfo?[.message] ?? ""
347-
throw Ingestion.Error.UnderlyingError.repositorySaveUniqueViolation(owner: repository.owner,
348-
name: repository.name,
349-
details: details)
350-
} catch let error as PSQLError {
351-
let details = error.serverInfo?[.message] ?? ""
352-
throw Ingestion.Error.UnderlyingError.repositorySaveFailed(owner: repository.owner,
353-
name: repository.name,
354-
details: details)
355-
} catch {
356-
throw Ingestion.Error.UnderlyingError.repositorySaveFailed(owner: repository.owner,
357-
name: repository.name,
358-
details: "\(error)")
342+
do {
343+
try await repository.save(on: database)
344+
} catch let error as PSQLError where error.isUniqueViolation {
345+
let details = error.serverInfo?[.message] ?? ""
346+
throw Ingestion.Error.UnderlyingError.repositorySaveUniqueViolation(owner: repository.owner,
347+
name: repository.name,
348+
details: details)
349+
} catch let error as PSQLError {
350+
let details = error.serverInfo?[.message] ?? ""
351+
throw Ingestion.Error.UnderlyingError.repositorySaveFailed(owner: repository.owner,
352+
name: repository.name,
353+
details: details)
354+
} catch {
355+
throw Ingestion.Error.UnderlyingError.repositorySaveFailed(owner: repository.owner,
356+
name: repository.name,
357+
details: "\(error)")
358+
}
359359
}
360-
}
361360

362-
func getFork(on database: Database, parent: Github.Metadata.Parent?) async -> Fork? {
363-
guard let parentUrl = parent?.normalizedURL else { return nil }
361+
static func getFork(on database: Database, parent: Github.Metadata.Parent?) async -> Fork? {
362+
guard let parentUrl = parent?.normalizedURL else { return nil }
364363

365-
if let packageId = try? await Package.query(on: database)
366-
.filter(\.$url, .custom("ilike"), parentUrl)
367-
.first()?.id {
368-
return .parentId(id: packageId, fallbackURL: parentUrl)
369-
} else {
370-
return .parentURL(parentUrl)
364+
if let packageId = try? await Package.query(on: database)
365+
.filter(\.$url, .custom("ilike"), parentUrl)
366+
.first()?.id {
367+
return .parentId(id: packageId, fallbackURL: parentUrl)
368+
} else {
369+
return .parentURL(parentUrl)
370+
}
371371
}
372372
}
373373

374+
374375
// Helper to ensure the canonical source for these critical fields is the same in all the places where we need them
375376
private extension Github.Metadata {
376377
var repositoryOwner: String? { repository?.repositoryOwner }
377378
var repositoryName: String? { repository?.repositoryName }
378379
}
379380

381+
380382
// Helper to ensure the canonical source for these critical fields is the same in all the places where we need them
381383
private extension Github.Metadata.Repository {
382384
var repositoryOwner: String? { owner.login }
383385
var repositoryName: String? { name }
384386
}
385387

388+
386389
private extension Github.Metadata.Parent {
387390
// Returns a normalized version of the URL. Adding a `.git` if not present.
388391
var normalizedURL: String? {
@@ -394,6 +397,7 @@ private extension Github.Metadata.Parent {
394397
}
395398
}
396399

400+
397401
private extension Ingestion.Error {
398402
static func invalidURL(packageId: Package.Id, url: String) -> Self {
399403
Ingestion.Error(packageId: packageId, underlyingError: .invalidURL(url))

Tests/AppTests/IngestionTests.swift

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,12 @@ class IngestionTests: AppTestCase {
9797
let repo = Repository(packageId: try pkg.requireID())
9898

9999
// MUT
100-
try await updateRepository(on: app.db,
101-
for: repo,
102-
metadata: .mock(owner: "foo", repository: "bar"),
103-
licenseInfo: .init(htmlUrl: ""),
104-
readmeInfo: .init(html: "", htmlUrl: "", imagesToCache: []),
105-
s3Readme: nil)
100+
try await Ingestion.updateRepository(on: app.db,
101+
for: repo,
102+
metadata: .mock(owner: "foo", repository: "bar"),
103+
licenseInfo: .init(htmlUrl: ""),
104+
readmeInfo: .init(html: "", htmlUrl: "", imagesToCache: []),
105+
s3Readme: nil)
106106

107107
// validate
108108
do {
@@ -154,16 +154,16 @@ class IngestionTests: AppTestCase {
154154
summary: "package desc")
155155

156156
// MUT
157-
try await updateRepository(on: app.db,
158-
for: repo,
159-
metadata: md,
160-
licenseInfo: .init(htmlUrl: "license url"),
161-
readmeInfo: .init(etag: "etag",
162-
html: "readme html https://img.shields.io/endpoint?url=https%3A%2F%2Fswiftpackageindex.com",
163-
htmlUrl: "readme html url",
164-
imagesToCache: []),
165-
s3Readme: .cached(s3ObjectUrl: "url", githubEtag: "etag"),
166-
fork: .parentURL("https://github.com/foo/bar.git"))
157+
try await Ingestion.updateRepository(on: app.db,
158+
for: repo,
159+
metadata: md,
160+
licenseInfo: .init(htmlUrl: "license url"),
161+
readmeInfo: .init(etag: "etag",
162+
html: "readme html https://img.shields.io/endpoint?url=https%3A%2F%2Fswiftpackageindex.com",
163+
htmlUrl: "readme html url",
164+
imagesToCache: []),
165+
s3Readme: .cached(s3ObjectUrl: "url", githubEtag: "etag"),
166+
fork: .parentURL("https://github.com/foo/bar.git"))
167167

168168
// validate
169169
do {
@@ -229,14 +229,14 @@ class IngestionTests: AppTestCase {
229229
summary: "package desc")
230230

231231
// MUT
232-
try await updateRepository(on: app.db,
233-
for: repo,
234-
metadata: md,
235-
licenseInfo: .init(htmlUrl: "license url"),
236-
readmeInfo: .init(html: "readme html",
237-
htmlUrl: "readme html url",
238-
imagesToCache: []),
239-
s3Readme: nil)
232+
try await Ingestion.updateRepository(on: app.db,
233+
for: repo,
234+
metadata: md,
235+
licenseInfo: .init(htmlUrl: "license url"),
236+
readmeInfo: .init(html: "readme html",
237+
htmlUrl: "readme html url",
238+
imagesToCache: []),
239+
s3Readme: nil)
240240

241241
// validate
242242
do {
@@ -660,23 +660,23 @@ class IngestionTests: AppTestCase {
660660
try await Package(url: "https://github.com/bar/forked.git", processingStage: .analysis).save(on: app.db)
661661

662662
// test lookup when package is in the index
663-
let fork = await getFork(on: app.db, parent: .init(url: "https://github.com/foo/parent.git"))
663+
let fork = await Ingestion.getFork(on: app.db, parent: .init(url: "https://github.com/foo/parent.git"))
664664
XCTAssertEqual(fork, .parentId(id: .id0, fallbackURL: "https://github.com/foo/parent.git"))
665665

666666
// test lookup when package is in the index but with different case in URL
667-
let fork2 = await getFork(on: app.db, parent: .init(url: "https://github.com/Foo/Parent.git"))
667+
let fork2 = await Ingestion.getFork(on: app.db, parent: .init(url: "https://github.com/Foo/Parent.git"))
668668
XCTAssertEqual(fork2, .parentId(id: .id0, fallbackURL: "https://github.com/Foo/Parent.git"))
669669

670670
// test whem metadata repo url doesn't have `.git` at end
671-
let fork3 = await getFork(on: app.db, parent: .init(url: "https://github.com/Foo/Parent"))
671+
let fork3 = await Ingestion.getFork(on: app.db, parent: .init(url: "https://github.com/Foo/Parent"))
672672
XCTAssertEqual(fork3, .parentId(id: .id0, fallbackURL: "https://github.com/Foo/Parent.git"))
673673

674674
// test lookup when package is not in the index
675-
let fork4 = await getFork(on: app.db, parent: .init(url: "https://github.com/some/other.git"))
675+
let fork4 = await Ingestion.getFork(on: app.db, parent: .init(url: "https://github.com/some/other.git"))
676676
XCTAssertEqual(fork4, .parentURL("https://github.com/some/other.git"))
677677

678678
// test lookup when parent url is nil
679-
let fork5 = await getFork(on: app.db, parent: nil)
679+
let fork5 = await Ingestion.getFork(on: app.db, parent: nil)
680680
XCTAssertEqual(fork5, nil)
681681
}
682682
}

0 commit comments

Comments
 (0)