Skip to content

Merge ForkedFromResult and ForkedFromInfo #3388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2024
Merged

Merge ForkedFromResult and ForkedFromInfo #3388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 0 additions & 46 deletions Sources/App/Controllers/API/API+PackageController+ForkedFromResult.swift
View file
Open in desktop

This file was deleted.

52 changes: 8 additions & 44 deletions Sources/App/Controllers/API/API+PackageController+GetRoute+Model.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ extension API.PackageController.GetRoute {
preReleaseReference: App.Reference?,
fundingLinks: [FundingLink] = [],
swift6Readiness: Swift6Readiness?,
forkedFromResult: API.PackageController.ForkedFromResult?
forkedFromInfo: ForkedFromInfo?
) {
self.packageId = packageId
self.repositoryOwner = repositoryOwner
Expand Down Expand Up @@ -125,22 +125,7 @@ extension API.PackageController.GetRoute {
}()
self.fundingLinks = fundingLinks
self.swift6Readiness = swift6Readiness
if let forkedFromResult {
switch forkedFromResult {
case .fromSPI(let repo, let owner, let ownerName, let packageName):
self.forkedFromInfo = ForkedFromInfo.fromSPI(
packageName: self.title,
originalOwner: owner,
originalOwnerName: ownerName,
originalRepo: repo,
originalPackageName: packageName
)
case .fromGitHub(let url):
self.forkedFromInfo = ForkedFromInfo.fromGitHub(url: url)
}
} else {
self.forkedFromInfo = nil
}
self.forkedFromInfo = forkedFromInfo
}

init?(result: API.PackageController.PackageResult,
Expand All @@ -151,7 +136,7 @@ extension API.PackageController.GetRoute {
platformBuildInfo: BuildInfo<CompatibilityMatrix.PlatformCompatibility>?,
weightedKeywords: [WeightedKeyword] = [],
swift6Readiness: Swift6Readiness?,
forkedFromResult: API.PackageController.ForkedFromResult?) {
forkedFromInfo: ForkedFromInfo?) {
// we consider certain attributes as essential and return nil (raising .notFound)
let repository = result.repository
guard
Expand Down Expand Up @@ -197,7 +182,7 @@ extension API.PackageController.GetRoute {
preReleaseReference: result.preReleaseVersion?.reference,
fundingLinks: result.repository.fundingLinks,
swift6Readiness: swift6Readiness,
forkedFromResult: forkedFromResult
forkedFromInfo: forkedFromInfo
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model init now doesn't do any logic with ForkedFromInfo - it just assigns it. That makes the tests simpler, see below.

)

}
Expand Down Expand Up @@ -370,32 +355,11 @@ extension API.PackageController.GetRoute.Model {
}

enum ForkedFromInfo: Codable, Equatable {
case fromSPI(
packageName: String,
originalOwner: String,
originalOwnerName: String,
originalRepo: String,
originalPackageName: String
)
case fromSPI(originalOwner: String,
originalOwnerName: String,
originalRepo: String,
originalPackageName: String)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the packageName: String field was actually unused.

case fromGitHub(url: String)

var url: String {
switch self {
case .fromSPI(_, let originalOwner, _, let originalRepo, _):
return SiteURL.package(.value(originalOwner), .value(originalRepo), nil).relativeURL()
case .fromGitHub(let url):
return url
}
}

var ownerURL: String? {
switch self {
case .fromSPI(_, let owner, _, _, _):
return SiteURL.author(.value(owner)).relativeURL()
case .fromGitHub:
return nil
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved these two view related properties into an extension in the file under Views/PackageController/GetRoute.Model+ext.swift. That way, view related things don't distract from the basic definition and appear "closer" to where they're actually used.

}
}

Expand Down
50 changes: 32 additions & 18 deletions Sources/App/Controllers/API/API+PackageController+GetRoute.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ extension API.PackageController {
let packageResult = try await PackageResult.query(on: database,
owner: owner,
repository: repository)

let forkedFromResult = try? await self.fetchForkedFromResult(on: database,
repository: packageResult.repository)

async let weightedKeywords = WeightedKeyword.query(
on: database, keywords: packageResult.repository.keywords
)
Expand All @@ -49,6 +45,7 @@ extension API.PackageController {
async let buildInfo = API.PackageController.BuildInfo.query(on: database,
owner: owner,
repository: repository)
async let forkedFromInfo = forkedFromInfo(on: database, fork: packageResult.repository.forkedFrom)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're potentially running another query, using async let makes this concurrent.


guard
let model = try await Self.Model(
Expand All @@ -60,7 +57,7 @@ extension API.PackageController {
platformBuildInfo: buildInfo.platform,
weightedKeywords: weightedKeywords,
swift6Readiness: buildInfo.swift6Readiness,
forkedFromResult: forkedFromResult
forkedFromInfo: forkedFromInfo
),
let schema = API.PackageSchema(result: packageResult)
else {
Expand All @@ -69,19 +66,6 @@ extension API.PackageController {

return (model, schema)
}

static func fetchForkedFromResult(on database: Database, repository: Repository) async throws -> ForkedFromResult? {
if let forkedFrom = repository.forkedFrom {
switch forkedFrom {
case .parentId(let id):
let info = try await ForkedFromResult.query(on: database, packageId: id)
return info
case .parentURL(let url):
return .fromGitHub(url: url)
}
}
return nil
}
}
}

Expand All @@ -102,4 +86,34 @@ extension API.PackageController.GetRoute {
beta: links[1],
latest: links[2])
}

static func forkedFromInfo(on database: Database, fork: Fork?) async -> Model.ForkedFromInfo? {
guard let forkedFrom = fork else { return nil }
switch forkedFrom {
case let .parentId(id):
return await Model.ForkedFromInfo.query(on: database, packageId: id)
case let .parentURL(url):
return .fromGitHub(url: url)
}
}
}


extension API.PackageController.GetRoute.Model.ForkedFromInfo {
static func query(on database: Database, packageId: Package.Id) async -> Self? {
let model = try? await Joined3<Package, Repository, Version>
.query(on: database, packageId: packageId, version: .defaultBranch)
.first()

guard let repoName = model?.repository.name,
let ownerName = model?.repository.ownerName,
let owner = model?.repository.owner else {
return nil
}

return .fromSPI(originalOwner: owner,
originalOwnerName: ownerName,
originalRepo: repoName,
originalPackageName: model?.version.packageName ?? repoName)
}
}
2 changes: 1 addition & 1 deletion Sources/App/Controllers/API/Types+WithExample.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ extension API.PackageController.GetRoute.Model: WithExample {
releaseReference: .tag(1, 2, 3, "1.2.3"),
preReleaseReference: nil,
swift6Readiness: nil,
forkedFromResult: nil)
forkedFromInfo: nil)
}
}

Expand Down
23 changes: 22 additions & 1 deletion Sources/App/Views/PackageController/GetRoute.Model+ext.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ extension API.PackageController.GetRoute.Model {
repoURLNode,
.text(".")
)
case .fromSPI(_, _, let ownerName, _, let originalPackageName):
case .fromSPI(_, let ownerName, _, let originalPackageName):
let repoURLNode: Node<HTML.BodyContext> = .a(
.href(forkedFromInfo.url),
.text("\(originalPackageName)")
Expand Down Expand Up @@ -711,3 +711,24 @@ extension API.PackageController.GetRoute.Model.Swift6Readiness {
return lines.joined(separator: "\n")
}
}


extension API.PackageController.GetRoute.Model.ForkedFromInfo {
var url: String {
switch self {
case .fromSPI(let originalOwner, _, let originalRepo, _):
return SiteURL.package(.value(originalOwner), .value(originalRepo), nil).relativeURL()
case .fromGitHub(let url):
return url
}
}

var ownerURL: String? {
switch self {
case .fromSPI(let owner, _, _, _):
return SiteURL.author(.value(owner)).relativeURL()
case .fromGitHub:
return nil
}
}
}
Loading
Loading